From c4fbd8910062236daf3b0f19ac926e3e9af5e6c3 Mon Sep 17 00:00:00 2001 From: Brian Dwyer Date: Thu, 10 Oct 2019 22:18:16 -0400 Subject: [PATCH 1/4] Ensure we only ever search for Security Groups within a subnet's VPC Signed-off-by: Brian Dwyer --- lib/kitchen/driver/aws/instance_generator.rb | 45 ++++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/kitchen/driver/aws/instance_generator.rb b/lib/kitchen/driver/aws/instance_generator.rb index c84f1ac1..a30505a9 100644 --- a/lib/kitchen/driver/aws/instance_generator.rb +++ b/lib/kitchen/driver/aws/instance_generator.rb @@ -43,25 +43,26 @@ def initialize(config, ec2, logger) # can be passed in null, others need to be ommitted if they are null def ec2_instance_data # rubocop:disable Metrics/MethodLength, Metrics/AbcSize # Support for looking up security group id and subnet id using tags. - + vpc_id = nil + client = ::Aws::EC2::Client.new(region: config[:region]) if config[:subnet_id].nil? && config[:subnet_filter] - config[:subnet_id] = ::Aws::EC2::Client - .new(region: config[:region]).describe_subnets( - filters: [ - { - name: "tag:#{config[:subnet_filter][:tag]}", - values: [config[:subnet_filter][:value]], - }, - ] - )[0][0].subnet_id - - if config[:subnet_id].nil? - raise "The subnet tagged '#{config[:subnet_filter][:tag]}\ - #{config[:subnet_filter][:value]}' does not exist!" - end + subnets = client.describe_subnets( + filters: [ + { + name: "tag:#{config[:subnet_filter][:tag]}", + values: [config[:subnet_filter][:value]], + }, + ] + ).subnets + raise "The subnet tagged '#{config[:subnet_filter][:tag]}:#{config[:subnet_filter][:value]}' does not exist!" unless subnets.any? + + vpc_id = subnets[0].vpc_id + config[:subnet_id] = subnets[0].subnet_id end if config[:security_group_ids].nil? && config[:security_group_filter] + # => Grab the VPC in the case a Subnet ID rather than Filter was set + vpc_id ||= client.describe_subnets(subnet_ids: [config[:subnet_id]]).subnets[0].vpc_id security_groups = [] filters = [config[:security_group_filter]].flatten filters.each do |sg_filter| @@ -72,6 +73,10 @@ def ec2_instance_data # rubocop:disable Metrics/MethodLength, Metrics/AbcSize name: "group-name", values: [sg_filter[:name]], }, + { + name: "vpc-id", + values: [vpc_id], + }, ] end @@ -81,13 +86,17 @@ def ec2_instance_data # rubocop:disable Metrics/MethodLength, Metrics/AbcSize name: "tag:#{sg_filter[:tag]}", values: [sg_filter[:value]], }, + { + name: "vpc-id", + values: [vpc_id], + }, ] end - security_group = ::Aws::EC2::Client.new(region: config[:region]).describe_security_groups(r)[0][0] + security_group = client.describe_security_groups(r).security_groups - if security_group - security_groups.push(security_group.group_id) + if security_group.any? + security_groups.push(security_group[0].group_id) else raise "A Security Group matching the following filter could not be found:\n#{sg_filter}" end From 0716c87da038b6f6b94833e2b8415b1c15583417 Mon Sep 17 00:00:00 2001 From: Brian Dwyer Date: Fri, 11 Oct 2019 01:36:44 -0400 Subject: [PATCH 2/4] Fix up tests Signed-off-by: Brian Dwyer --- lib/kitchen/driver/ec2.rb | 1 + .../driver/aws/instance_generator_spec.rb | 30 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index d2fa839b..1b2fbed5 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -788,6 +788,7 @@ def create_security_group(state) elsif config[:subnet_filter] subnets = ec2.client.describe_subnets(filters: [{ name: "tag:#{config[:subnet_filter][:tag]}", values: [config[:subnet_filter][:value]] }]).subnets raise "Subnets with tag '#{config[:subnet_filter][:tag]}=#{config[:subnet_filter][:value]}' not found during security group creation" if subnets.empty? + subnets.first.vpc_id else # Try to check for a default VPC. diff --git a/spec/kitchen/driver/aws/instance_generator_spec.rb b/spec/kitchen/driver/aws/instance_generator_spec.rb index fae75f7e..66c0c00d 100644 --- a/spec/kitchen/driver/aws/instance_generator_spec.rb +++ b/spec/kitchen/driver/aws/instance_generator_spec.rb @@ -25,7 +25,7 @@ describe Kitchen::Driver::Aws::InstanceGenerator do - let(:config) { {} } + let(:config) { { region: "us-east-1" } } let(:resource) { instance_double(Aws::EC2::Resource) } let(:ec2) { instance_double(Kitchen::Driver::Aws::Client, resource: resource) } let(:logger) { instance_double(Logger) } @@ -76,6 +76,7 @@ subnets: [ { subnet_id: "s-123", + vpc_id: "vpc-456", tags: [{ key: "foo", value: "bar" }], }, ] @@ -105,6 +106,7 @@ context "when populated with minimum requirements" do let(:config) do { + region: "us-east-1", instance_type: "micro", ebs_optimized: true, image_id: "ami-123", @@ -128,6 +130,7 @@ context "when populated with ssh key" do let(:config) do { + region: "us-east-1", instance_type: "micro", ebs_optimized: true, image_id: "ami-123", @@ -204,6 +207,10 @@ name: "tag:foo", values: ["bar"], }, + { + name: "vpc-id", + values: ["vpc-456"], + }, ] ).and_return(ec2_stub.describe_security_groups) expect(generator.ec2_instance_data[:security_group_ids]).to eq(["sg-123"]) @@ -212,6 +219,16 @@ context "when provided a non existing security_group tag filter" do ec2_stub_whithout_security_group = Aws::EC2::Client.new(stub_responses: true) + ec2_stub_whithout_security_group.stub_responses( + :describe_subnets, + subnets: [ + { + subnet_id: "s-123", + vpc_id: "vpc-456", + tags: [{ key: "foo", value: "bar" }], + }, + ] + ) let(:config) do { @@ -237,6 +254,10 @@ name: "tag:foo", values: ["bar"], }, + { + name: "vpc-id", + values: ["vpc-456"], + }, ] ).and_return(ec2_stub_whithout_security_group.describe_security_groups) @@ -249,6 +270,7 @@ context "when passed an empty block_device_mappings" do let(:config) do { + region: "us-east-1", instance_type: "micro", ebs_optimized: true, image_id: "ami-123", @@ -416,6 +438,7 @@ context "when subnet_id is provided" do let(:config) do { + region: "us-east-1", subnet_id: "s-456", } end @@ -435,6 +458,7 @@ context "when associate_public_ip is provided" do let(:config) do { + region: "us-east-1", associate_public_ip: true, } end @@ -458,6 +482,7 @@ context "and subnet is provided" do let(:config) do { + region: "us-east-1", associate_public_ip: true, subnet_id: "s-456", } @@ -483,6 +508,7 @@ context "and security_group_ids is provided" do let(:config) do { + region: "us-east-1", associate_public_ip: true, security_group_ids: ["sg-789"], } @@ -522,6 +548,7 @@ context "and private_ip_address is provided" do let(:config) do { + region: "us-east-1", associate_public_ip: true, private_ip_address: "0.0.0.0", } @@ -548,6 +575,7 @@ context "when provided the maximum config" do let(:config) do { + region: "eu-west-1", availability_zone: "eu-west-1a", instance_type: "micro", ebs_optimized: true, From 357259c1d50ea5a5073f6f57a2844ac665db52b0 Mon Sep 17 00:00:00 2001 From: Brian Dwyer Date: Fri, 11 Oct 2019 02:01:34 -0400 Subject: [PATCH 3/4] Stop masking spot instance provisioning errors Signed-off-by: Brian Dwyer --- lib/kitchen/driver/ec2.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index 1b2fbed5..5325bb34 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -454,15 +454,16 @@ def submit_spots(state) expanded = [] end + errs = [] configs.each do |conf| begin @config = conf return submit_spot(state) - rescue + rescue => e + errs.append(e) end end - - raise "Could not create a spot" + raise ["Could not create a spot instance:", errs].flatten.join("\n") end def submit_spot(state) From 11ee420cc5aef47348c7211af8476c9b67212b66 Mon Sep 17 00:00:00 2001 From: Brian Dwyer Date: Tue, 17 Dec 2019 12:57:09 -0500 Subject: [PATCH 4/4] Support adding multiple security groups from a single search --- lib/kitchen/driver/aws/instance_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kitchen/driver/aws/instance_generator.rb b/lib/kitchen/driver/aws/instance_generator.rb index a30505a9..d13955d1 100644 --- a/lib/kitchen/driver/aws/instance_generator.rb +++ b/lib/kitchen/driver/aws/instance_generator.rb @@ -96,7 +96,7 @@ def ec2_instance_data # rubocop:disable Metrics/MethodLength, Metrics/AbcSize security_group = client.describe_security_groups(r).security_groups if security_group.any? - security_groups.push(security_group[0].group_id) + security_group.each { |sg| security_groups.push(sg.group_id) } else raise "A Security Group matching the following filter could not be found:\n#{sg_filter}" end