Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

feat: Filtering imported security groups by IDs #410

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+148 −13
Open

feat: Filtering imported security groups by IDs #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/terraforming.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
require "terraforming/util"
require "terraforming/version"

require "terraforming/cli"
require "terraforming/resource/alb"
require "terraforming/resource/auto_scaling_group"
require "terraforming/resource/cloud_watch_alarm"
Expand Down Expand Up @@ -66,3 +65,4 @@
require "terraforming/resource/vpn_gateway"
require "terraforming/resource/sns_topic"
require "terraforming/resource/sns_topic_subscription"
require "terraforming/cli"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to move this to the bottom of this file in order to avoid NameError: uninitialized constant Terraforming::Resource at the line 4 of lib/terraforming/cli.rb.

30 changes: 25 additions & 5 deletions lib/terraforming/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
module Terraforming
class CLI < Thor
OPTIONS_AVAILABLE_TO_SUBCOMMANDS = [
Terraforming::Resource::SecurityGroup::AVAILABLE_OPTIONS,
].reduce(:concat).freeze

class_option :merge, type: :string, desc: "tfstate file to merge"
class_option :overwrite, type: :boolean, desc: "Overwrite existing tfstate"
class_option :tfstate, type: :boolean, desc: "Generate tfstate"
Expand Down Expand Up @@ -191,6 +195,7 @@ def s3
end

desc "sg", "Security Group"
method_option :"group-ids", type: :array, desc: "Filter exported security groups by IDs"
def sg
execute(Terraforming::Resource::SecurityGroup, options)
end
Expand Down Expand Up @@ -242,7 +247,13 @@ def configure_aws(options)

def execute(klass, options)
configure_aws(options)
result = options[:tfstate] ? tfstate(klass, options[:merge]) : tf(klass)

subcommand_options = options.select { |k, v| OPTIONS_AVAILABLE_TO_SUBCOMMANDS.include? k }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind if I blindly passed all the options as subcommand_options here?
In that case we can simplify the implementation by removing OPTIONS_AVAILABLE_TO_SUBCOMMANDS and its relevant code.

result = if options[:tfstate]
tfstate(klass, options[:merge], subcommand_options)
else
tf(klass, subcommand_options)
end

if options[:tfstate] && options[:merge] && options[:overwrite]
open(options[:merge], "w+") do |f|
Expand All @@ -254,14 +265,23 @@ def execute(klass, options)
end
end

def tf(klass)
klass.tf
def tf(klass, options={})
if options.empty?
klass.tf
else
klass.tf(options)
end
end

def tfstate(klass, tfstate_path)
def tfstate(klass, tfstate_path, options={})
tfstate = tfstate_path ? MultiJson.load(open(tfstate_path).read) : tfstate_skeleton
tfstate["serial"] = tfstate["serial"] + 1
tfstate["modules"][0]["resources"] = tfstate["modules"][0]["resources"].merge(klass.tfstate)
tfstate_addition = if options.empty?
klass.tfstate
else
klass.tfstate(options)
end
tfstate["modules"][0]["resources"] = tfstate["modules"][0]["resources"].merge(tfstate_addition)
MultiJson.encode(tfstate, pretty: true)
end

Expand Down
36 changes: 30 additions & 6 deletions lib/terraforming/resource/security_group.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,35 @@ module Resource
class SecurityGroup
include Terraforming::Util

def self.tf(client: Aws::EC2::Client.new)
self.new(client).tf
module Options
GROUP_IDS = 'group-ids'
end

def self.tfstate(client: Aws::EC2::Client.new)
self.new(client).tfstate
AVAILABLE_OPTIONS = [
Options::GROUP_IDS,
].freeze

def self.tf(options={})
opts = apply_defaults_to_options(options)
self
.new(opts[:client], opts)
.tf
end

def self.tfstate(options={})
opts = apply_defaults_to_options(options)
self.new(opts[:client], opts).tfstate
end

def self.apply_defaults_to_options(options)
options.dup.tap { |o|
o[:client] ||= Aws::EC2::Client.new
}
end

def initialize(client)
def initialize(client, options)
@client = client
@group_ids = options[Options::GROUP_IDS]
end

def tf
Expand Down Expand Up @@ -159,7 +178,12 @@ def self_referenced_permission?(security_group, permission)
end

def security_groups
@client.describe_security_groups.map(&:security_groups).flatten
description = if @group_ids
@client.describe_security_groups(group_ids: @group_ids)
else
@client.describe_security_groups
end
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to make this block conditional instead of doing @client.describea_security_groups(opts) not to break huge number of test cases :)

I can make it look something like:

opts = {}.tap { |o|
  o.[:group_ids] ||= @group_ids if @group_ids
}
@client.describe_security_groups(opts)

but I prefer separating it into another PR to ease your review.

description.map(&:security_groups).flatten
end

def security_groups_in(permission, security_group)
Expand Down
12 changes: 12 additions & 0 deletions spec/lib/terraforming/cli_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,18 @@ module Terraforming
let(:command) { :sg }

it_behaves_like "CLI examples"

context "with --group-ids" do
it "should export partial tf" do
expect(klass).to receive(:tf).with({"group-ids" => %w| sg-1234abcd |})
described_class.new.invoke(command, [], {:"group-ids" => %w| sg-1234abcd |})
end

it "should export partial tfstate" do
expect(klass).to receive(:tfstate).with({"group-ids" => %w| sg-1234abcd |})
described_class.new.invoke(command, [], {:tfstate => true, :"group-ids" => %w| sg-1234abcd |})
end
end
end

describe "sqs" do
Expand Down
81 changes: 80 additions & 1 deletion spec/lib/terraforming/resource/security_group_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,17 @@ module Resource
end

before do
client.stub_responses(:describe_security_groups, security_groups: security_groups)
client.stub_responses(:describe_security_groups, -> (context) {
group_ids = context.params[:group_ids]
groups = if group_ids
security_groups.select { |sg|
group_ids.include? sg[:group_id]
}
else
security_groups
end
{ security_groups: groups }
})
end

describe ".tf" do
Expand Down Expand Up @@ -282,6 +292,36 @@ module Resource
EOS
end

context "with --group-ids" do
it "should generate partial tf" do
expect(described_class.tf({:client => client, "group-ids" => %w| sg-1234abcd |})).to eq <<-EOS
resource "aws_security_group" "hoge" {
name = "hoge"
description = "Group for hoge"
vpc_id = ""
ingress {
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
}
ingress {
from_port = 22
to_port = 22
protocol = "tcp"
security_groups = ["987654321012/piyo"]
self = true
}
}
EOS
end
end
end

describe ".tfstate" do
Expand Down Expand Up @@ -405,6 +445,45 @@ module Resource
},
})
end

context "with --group-ids" do
it "should generate partial tfstate" do
expect(described_class.tfstate({:client => client, "group-ids" => %w| sg-1234abcd |})).to eq({
"aws_security_group.hoge" => {
"type" => "aws_security_group",
"primary" => {
"id" => "sg-1234abcd",
"attributes" => {
"description" => "Group for hoge",
"id" => "sg-1234abcd",
"name" => "hoge",
"owner_id" => "012345678901",
"vpc_id" => "",
"tags.#" => "0",
"egress.#" => "0",
"ingress.#" => "2",
"ingress.2541437006.from_port" => "22",
"ingress.2541437006.to_port" => "22",
"ingress.2541437006.protocol" => "tcp",
"ingress.2541437006.cidr_blocks.#" => "1",
"ingress.2541437006.prefix_list_ids.#" => "0",
"ingress.2541437006.security_groups.#" => "0",
"ingress.2541437006.self" => "false",
"ingress.2541437006.cidr_blocks.0" => "0.0.0.0/0",
"ingress.3232230010.from_port" => "22",
"ingress.3232230010.to_port" => "22",
"ingress.3232230010.protocol" => "tcp",
"ingress.3232230010.cidr_blocks.#" => "0",
"ingress.3232230010.prefix_list_ids.#" => "0",
"ingress.3232230010.security_groups.#" => "1",
"ingress.3232230010.self" => "true",
"ingress.3232230010.security_groups.1889292513" => "987654321012/piyo",
}
}
},
})
end
end
end
end
end
Expand Down