-
Notifications
You must be signed in to change notification settings - Fork 647
feat: Filtering imported security groups by IDs #410
base: master
Are you sure you want to change the base?
Conversation
| require "terraforming/resource/vpn_gateway" | ||
| require "terraforming/resource/sns_topic" | ||
| require "terraforming/resource/sns_topic_subscription" | ||
| require "terraforming/cli" |
There was a problem hiding this comment.
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.
| configure_aws(options) | ||
| result = options[:tfstate] ? tfstate(klass, options[:merge]) : tf(klass) | ||
|
|
||
| subcommand_options = options.select { |k, v| OPTIONS_AVAILABLE_TO_SUBCOMMANDS.include? k } |
There was a problem hiding this comment.
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.
| @client.describe_security_groups(group_ids: @group_ids) | ||
| else | ||
| @client.describe_security_groups | ||
| end |
There was a problem hiding this comment.
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.
38a6a15 to
e632fed
Compare
`terraforming sg` now accepts one or more security groups via `--group-ids sg-12345 sg-234456`. This limits the tf output to include only the two security groups. Similarly, `terraform sg --tfstate --group-ids sg-12345` limits the tfstate output to include only the security group. An expected use-case to this flag is to gradually migrate hundreds of your security groups under the control of terraform, without worrying about the huge tf/tfstate diff on initial import. Run `terraforming help sg` to see the description of the flag: ``` bundle exec bin/terraforming help sg Usage: terraforming sg Options: [--group-ids=one two three] # Filter exported security groups by IDs [--merge=MERGE] # tfstate file to merge [--overwrite], [--no-overwrite] # Overwrite existing tfstate [--tfstate], [--no-tfstate] # Generate tfstate [--profile=PROFILE] # AWS credentials profile [--region=REGION] # AWS region [--assume=ASSUME] # Role ARN to assume [--use-bundled-cert], [--no-use-bundled-cert] # Use the bundled CA certificate from AWS SDK Security Group ```
e632fed to
f3200ef
Compare
terraforming sgnow accepts one or more security groups via--group-ids sg-12345 sg-234456.This limits the tf output to include only the two security groups.
Similarly,
terraform sg --tfstate --group-ids sg-12345limits the tfstate output to include only the security group.An expected use-case to this flag is to gradually migrate hundreds of your security groups under the control of terraform, without worrying about the huge tf/tfstate diff on initial import.
Run
terraforming help sgto see the description of the flag: