From 69d4f41c14b6f9ff9e5dc4e94735971538d8ec24 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Tue, 7 Apr 2015 02:42:11 -0400 Subject: [PATCH 01/14] Add explicit option for using iam profile for authentication * Add 'use_iam_profile' config option, default to false * Have #iam_creds return an empty hash unless use_iam_profile is true * Call iam_creds as driver.iam_creds in `default_config` blocks * Rescue fron NoMethodError in iam_creds `Fog::AWS::CredentialFetcher::ServiceMethods::fetch_credentials` will call super when it fails. Because there is no superclass method this will throw a NoMethodError * Write unit tests for #iam_creds --- lib/kitchen/driver/ec2.rb | 15 ++++++++------- spec/create_spec.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index db2e9327..bfb3fc49 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -29,7 +29,6 @@ module Driver # # @author Fletcher Nichol class Ec2 < Kitchen::Driver::SSHBase - extend Fog::AWS::CredentialFetcher::ServiceMethods default_config :region, 'us-east-1' default_config :availability_zone, 'us-east-1b' @@ -41,14 +40,16 @@ class Ec2 < Kitchen::Driver::SSHBase default_config :private_ip_address, nil default_config :iam_profile_name, nil default_config :price, nil + default_config :use_iam_profile, false default_config :aws_access_key_id do |driver| - ENV['AWS_ACCESS_KEY'] || ENV['AWS_ACCESS_KEY_ID'] || iam_creds[:aws_access_key_id] + ENV['AWS_ACCESS_KEY'] || ENV['AWS_ACCESS_KEY_ID'] || driver.iam_creds[:aws_access_key_id] end default_config :aws_secret_access_key do |driver| - ENV['AWS_SECRET_KEY'] || ENV['AWS_SECRET_ACCESS_KEY'] || iam_creds[:aws_secret_access_key] + ENV['AWS_SECRET_KEY'] || ENV['AWS_SECRET_ACCESS_KEY'] \ + || driver.iam_creds[:aws_secret_access_key] end default_config :aws_session_token do |driver| - ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] || iam_creds[:aws_session_token] + ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] || driver.iam_creds[:aws_session_token] end default_config :aws_ssh_key_id do |driver| ENV['AWS_SSH_KEY_ID'] @@ -98,10 +99,10 @@ class Ec2 < Kitchen::Driver::SSHBase end end - def self.iam_creds + def iam_creds @iam_creds ||= begin - fetch_credentials(use_iam_profile:true) - rescue RuntimeError => e + config[:use_iam_profile] ? fetch_credentials(use_iam_profile: true) : {} + rescue RuntimeError, NoMethodError => e debug("fetch_credentials failed with exception #{e.message}:#{e.backtrace.join("\n")}") {} end diff --git a/spec/create_spec.rb b/spec/create_spec.rb index fcebf72f..6866a13f 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -128,6 +128,37 @@ end + describe '#iam_creds' do + context 'when use_iam_profile is not set' do + it 'returns an empty hash' do + expect(driver.iam_creds).to eq({}) + end + end + + context 'when use_iam_profile is set to true' do + let(:credentials) do + { + aws_access_key_id: 'secret', + aws_secret_access_key: 'moarsecret', + aws_session_token: 'randomsecret' + } + end + + it 'calls fetch_credentials' do + config[:use_iam_profile] = true + + allow(driver) + .to receive(:fetch_credentials).and_return(credentials) + + expect(driver) + .to receive(:fetch_credentials) + .with(use_iam_profile: true) + + expect(driver.iam_creds).to eq(credentials) + end + end + end + describe '#block_device_mappings' do let(:connection) { double(Fog::Compute) } let(:image) { double('Image', :root_device_name => 'name') } From 4b12e1b5e587a9c26cf37884d8485dc5cf683af3 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Tue, 7 Apr 2015 12:16:30 -0400 Subject: [PATCH 02/14] Remove need for :use_iam_profile autodetect if using iam instead * Remove :use_iam_profile config setting * Update tests to reflect autodetection vs use of :use_iam_profile * Add `#default_aws_session_token` method to handle logic for setting default value of config[:aws_session_token] Since the :use_iam_profile parameter is confusing instead autodetect if `iam_creds` should be used for setting config[:aws_session_token] by checking if other config settings match the values returned from `iam_creds`. If the config settings match then it's a safe bet that iam_creds should be used for setting config[:aws_session_token]. --- lib/kitchen/driver/ec2.rb | 14 +++++++++--- spec/create_spec.rb | 45 ++++++++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index bfb3fc49..484f9e9e 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -40,7 +40,6 @@ class Ec2 < Kitchen::Driver::SSHBase default_config :private_ip_address, nil default_config :iam_profile_name, nil default_config :price, nil - default_config :use_iam_profile, false default_config :aws_access_key_id do |driver| ENV['AWS_ACCESS_KEY'] || ENV['AWS_ACCESS_KEY_ID'] || driver.iam_creds[:aws_access_key_id] end @@ -49,7 +48,7 @@ class Ec2 < Kitchen::Driver::SSHBase || driver.iam_creds[:aws_secret_access_key] end default_config :aws_session_token do |driver| - ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] || driver.iam_creds[:aws_session_token] + driver.default_aws_session_token end default_config :aws_ssh_key_id do |driver| ENV['AWS_SSH_KEY_ID'] @@ -101,7 +100,7 @@ class Ec2 < Kitchen::Driver::SSHBase def iam_creds @iam_creds ||= begin - config[:use_iam_profile] ? fetch_credentials(use_iam_profile: true) : {} + fetch_credentials(use_iam_profile: true) rescue RuntimeError, NoMethodError => e debug("fetch_credentials failed with exception #{e.message}:#{e.backtrace.join("\n")}") {} @@ -168,6 +167,15 @@ def default_public_ip_association !!config[:subnet_id] end + def default_aws_session_token + if config[:aws_secret_access_key] == iam_creds[:aws_secret_access_key] \ + && config[:aws_access_key_id] == iam_creds[:aws_access_key_id] + iam_creds[:aws_session_token] + else + ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] + end + end + private def connection diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 6866a13f..bcf4f5bb 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -128,14 +128,38 @@ end - describe '#iam_creds' do - context 'when use_iam_profile is not set' do - it 'returns an empty hash' do - expect(driver.iam_creds).to eq({}) + context 'When autodetecting an iam role' do + let(:iam_creds) do + { + aws_access_key_id: 'secret', + aws_secret_access_key: 'moarsecret', + aws_session_token: 'randomsecret' + } + end + + context 'when :aws_secret_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token] based on iam_creds' do + config[:aws_secret_access_key] = 'adifferentsecret' + + allow(driver) + .to receive(:iam_creds).and_return(iam_creds) + + expect(driver.send(:config)[:aws_session_token]).to be_nil + end + end + + context 'when :aws_access_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token] based on iam_creds' do + config[:aws_access_key_id] = 'adifferentsecret' + + allow(driver) + .to receive(:iam_creds).and_return(iam_creds) + + expect(driver.send(:config)[:aws_session_token]).to be_nil end end - context 'when use_iam_profile is set to true' do + context 'when :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do let(:credentials) do { aws_access_key_id: 'secret', @@ -144,17 +168,14 @@ } end - it 'calls fetch_credentials' do - config[:use_iam_profile] = true - + it 'uses :aws_session_token from iam_creds' do allow(driver) - .to receive(:fetch_credentials).and_return(credentials) + .to receive(:iam_creds).and_return(iam_creds) expect(driver) - .to receive(:fetch_credentials) - .with(use_iam_profile: true) + .to receive(:iam_creds) - expect(driver.iam_creds).to eq(credentials) + expect(driver.send(:config)[:aws_session_token]).to eq(iam_creds[:aws_session_token]) end end end From adc082dc90e5f829c97911320effd8037fb5b3d6 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Tue, 7 Apr 2015 12:26:20 -0400 Subject: [PATCH 03/14] Use include not extend for `Fog::AWS::CredentialFetcher::ServiceMethods` --- lib/kitchen/driver/ec2.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index 484f9e9e..a51b0d2e 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -29,7 +29,7 @@ module Driver # # @author Fletcher Nichol class Ec2 < Kitchen::Driver::SSHBase - extend Fog::AWS::CredentialFetcher::ServiceMethods + include Fog::AWS::CredentialFetcher::ServiceMethods default_config :region, 'us-east-1' default_config :availability_zone, 'us-east-1b' default_config :flavor_id, 'm1.small' From 389522c697e1f175942299fcf38a334e585fcadd Mon Sep 17 00:00:00 2001 From: James Awesome Date: Tue, 7 Apr 2015 13:39:23 -0400 Subject: [PATCH 04/14] Remove unused let() block from tests --- spec/create_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index bcf4f5bb..d31ca6c7 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -160,14 +160,6 @@ end context 'when :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do - let(:credentials) do - { - aws_access_key_id: 'secret', - aws_secret_access_key: 'moarsecret', - aws_session_token: 'randomsecret' - } - end - it 'uses :aws_session_token from iam_creds' do allow(driver) .to receive(:iam_creds).and_return(iam_creds) From aaaa892fb0c69b72d5200b305b5c210a39c10ec1 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Wed, 8 Apr 2015 16:13:15 -0400 Subject: [PATCH 05/14] Stylefix, remove unnecessary "\" --- lib/kitchen/driver/ec2.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index a51b0d2e..e8df7af0 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -44,8 +44,8 @@ class Ec2 < Kitchen::Driver::SSHBase ENV['AWS_ACCESS_KEY'] || ENV['AWS_ACCESS_KEY_ID'] || driver.iam_creds[:aws_access_key_id] end default_config :aws_secret_access_key do |driver| - ENV['AWS_SECRET_KEY'] || ENV['AWS_SECRET_ACCESS_KEY'] \ - || driver.iam_creds[:aws_secret_access_key] + ENV['AWS_SECRET_KEY'] || ENV['AWS_SECRET_ACCESS_KEY'] || + driver.iam_creds[:aws_secret_access_key] end default_config :aws_session_token do |driver| driver.default_aws_session_token From 194587993b135df557f3d5b068234c40e1dade4a Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 15:17:57 -0400 Subject: [PATCH 06/14] Update tests for #iam_creds --- lib/kitchen/driver/ec2.rb | 25 ++++++------ spec/create_spec.rb | 85 +++++++++++++++++++++++++++------------ 2 files changed, 72 insertions(+), 38 deletions(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index e8df7af0..ea2950e3 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -41,14 +41,16 @@ class Ec2 < Kitchen::Driver::SSHBase default_config :iam_profile_name, nil default_config :price, nil default_config :aws_access_key_id do |driver| - ENV['AWS_ACCESS_KEY'] || ENV['AWS_ACCESS_KEY_ID'] || driver.iam_creds[:aws_access_key_id] + ENV['AWS_ACCESS_KEY'] || ENV['AWS_ACCESS_KEY_ID'] || + driver.iam_creds[:aws_access_key_id] end default_config :aws_secret_access_key do |driver| ENV['AWS_SECRET_KEY'] || ENV['AWS_SECRET_ACCESS_KEY'] || driver.iam_creds[:aws_secret_access_key] end default_config :aws_session_token do |driver| - driver.default_aws_session_token + ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] || + driver.iam_creds[:aws_session_token] end default_config :aws_ssh_key_id do |driver| ENV['AWS_SSH_KEY_ID'] @@ -98,10 +100,18 @@ class Ec2 < Kitchen::Driver::SSHBase end end + # First we check the existence of the metadata host. Only fetch_credentials + # if we can find the host. Ping logic taken from + # http://stackoverflow.com/questions/7519159/ruby-ping-pingecho-missing def iam_creds + require 'net/http' + require 'timeout' @iam_creds ||= begin + timeout(5) do + Net::HTTP.get(URI.parse('http://169.254.169.254')) + end fetch_credentials(use_iam_profile: true) - rescue RuntimeError, NoMethodError => e + rescue Errno::EHOSTUNREACH, Timeout::Error, NoMethodError, ::StandardError => e debug("fetch_credentials failed with exception #{e.message}:#{e.backtrace.join("\n")}") {} end @@ -167,15 +177,6 @@ def default_public_ip_association !!config[:subnet_id] end - def default_aws_session_token - if config[:aws_secret_access_key] == iam_creds[:aws_secret_access_key] \ - && config[:aws_access_key_id] == iam_creds[:aws_access_key_id] - iam_creds[:aws_session_token] - else - ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] - end - end - private def connection diff --git a/spec/create_spec.rb b/spec/create_spec.rb index d31ca6c7..09673966 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -128,46 +128,79 @@ end - context 'When autodetecting an iam role' do + describe '#iam_creds' do let(:iam_creds) do { - aws_access_key_id: 'secret', - aws_secret_access_key: 'moarsecret', - aws_session_token: 'randomsecret' + aws_access_key_id: 'siam_creds_access_keu', + aws_secret_access_key: 'iam_creds_secret_access_key', + aws_session_token: 'iam_creds_session_token' } end - context 'when :aws_secret_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token] based on iam_creds' do - config[:aws_secret_access_key] = 'adifferentsecret' + context 'when a metadata service is available' do + before do + allow(Net::HTTP).to receive(:get).with(URI.parse('http://169.254.169.254')).and_return(true) + end + + context 'and #fetch_credentials returns valid iam credentials' do + context 'when :aws_secret_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token] based on iam_creds' do + config[:aws_secret_access_key] = 'adifferentsecret' + allow(driver).to receive(:fetch_credentials).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end + end - allow(driver) - .to receive(:iam_creds).and_return(iam_creds) + context 'when :aws_access_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token] based on iam_creds' do + allow(driver).to receive(:fetch_credentials).and_return(iam_creds) + config[:aws_access_key_id] = 'adifferentsecret' + expect(driver.send(:config)[:aws_session_token]).to be_nil + end + end - expect(driver.send(:config)[:aws_session_token]).to be_nil + context 'when :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do + it 'uses :aws_session_token from iam_creds' do + allow(driver).to receive(:fetch_credentials).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to eq(iam_creds[:aws_session_token]) + end + end end - end - - context 'when :aws_access_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token] based on iam_creds' do - config[:aws_access_key_id] = 'adifferentsecret' - allow(driver) - .to receive(:iam_creds).and_return(iam_creds) + context 'when #fetch_credentials fails with NoMethodError' do + it 'returns an empty hash' do + allow(driver).to receive(:fetch_credentials).and_raise(NoMethodError) + expect(driver.iam_creds).to eq({}) + end + end - expect(driver.send(:config)[:aws_session_token]).to be_nil + context 'when #fetch_credentials fails with StandardError' do + it 'returns an empty hash' do + allow(driver).to receive(:fetch_credentials).and_raise(StandardError) + expect(driver.iam_creds).to eq({}) + end end - end - context 'when :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do - it 'uses :aws_session_token from iam_creds' do - allow(driver) - .to receive(:iam_creds).and_return(iam_creds) + context 'when #fetch_credentials fails with Errno::EHOSTUNREACH' do + it 'returns an empty hash' do + allow(driver).to receive(:fetch_credentials).and_raise(Errno::EHOSTUNREACH) + expect(driver.iam_creds).to eq({}) + end + end - expect(driver) - .to receive(:iam_creds) + context 'when #fetch_credentials fails with Timeout::Error' do + it 'returns an empty hash' do + allow(driver).to receive(:fetch_credentials).and_raise(Timeout::Error) + expect(driver.iam_creds).to eq({}) + end + end + end - expect(driver.send(:config)[:aws_session_token]).to eq(iam_creds[:aws_session_token]) + context 'when a metadata service is not available' do + it 'will not call #fetch_credentials' do + allow(Net::HTTP).to receive(:get) + .with(URI.parse('http://169.254.169.254')).and_return(false) + expect(driver).to_not receive(:fetch_credentials) end end end From 98535265ed130e1aea10e38a4e215ec22dda6dd6 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 15:20:07 -0400 Subject: [PATCH 07/14] Ensure raising top namespace StandardError from rspec-mock --- spec/create_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 09673966..e9f08f81 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -176,7 +176,7 @@ context 'when #fetch_credentials fails with StandardError' do it 'returns an empty hash' do - allow(driver).to receive(:fetch_credentials).and_raise(StandardError) + allow(driver).to receive(:fetch_credentials).and_raise(::StandardError) expect(driver.iam_creds).to eq({}) end end From 770557eb2fc00507918b4752920bdda8053b78c4 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 16:15:04 -0400 Subject: [PATCH 08/14] Better test name spacing, correct false positive --- spec/create_spec.rb | 65 ++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index e9f08f81..4e20db1b 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -41,6 +41,14 @@ Kitchen::Driver::Ec2.new(config) end + let(:iam_creds) do + { + aws_access_key_id: 'siam_creds_access_keu', + aws_secret_access_key: 'iam_creds_secret_access_key', + aws_session_token: 'iam_creds_session_token' + } + end + before do instance allow(driver).to receive(:create_server).and_return(server) @@ -128,42 +136,43 @@ end - describe '#iam_creds' do - let(:iam_creds) do - { - aws_access_key_id: 'siam_creds_access_keu', - aws_secret_access_key: 'iam_creds_secret_access_key', - aws_session_token: 'iam_creds_session_token' - } + context 'When #iam_creds returns values but they should not be used' do + context 'because :aws_access_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token]' do + config[:aws_access_key_id] = 'adifferentkey' + allow(driver).to receive(:iam_creds).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end + end + + context 'because :aws_secret_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token]' do + config[:aws_secret_access_key] = 'adifferentsecret' + allow(driver).to receive(:iam_creds).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end + end + + context 'because :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do + it 'does not set config[:aws_session_token]' do + config[:aws_access_key_id] = 'adifferentkey' + config[:aws_secret_access_key] = 'adifferentsecret' + allow(driver).to receive(:iam_creds).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end end + end + describe '#iam_creds' do context 'when a metadata service is available' do before do allow(Net::HTTP).to receive(:get).with(URI.parse('http://169.254.169.254')).and_return(true) end context 'and #fetch_credentials returns valid iam credentials' do - context 'when :aws_secret_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token] based on iam_creds' do - config[:aws_secret_access_key] = 'adifferentsecret' - allow(driver).to receive(:fetch_credentials).and_return(iam_creds) - expect(driver.send(:config)[:aws_session_token]).to be_nil - end - end - - context 'when :aws_access_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token] based on iam_creds' do - allow(driver).to receive(:fetch_credentials).and_return(iam_creds) - config[:aws_access_key_id] = 'adifferentsecret' - expect(driver.send(:config)[:aws_session_token]).to be_nil - end - end - - context 'when :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do - it 'uses :aws_session_token from iam_creds' do - allow(driver).to receive(:fetch_credentials).and_return(iam_creds) - expect(driver.send(:config)[:aws_session_token]).to eq(iam_creds[:aws_session_token]) - end + it '#iam_creds retuns the iam credentials from fetch_credentials' do + allow(driver).to receive(:fetch_credentials).and_return(iam_creds) + expect(driver.send(:iam_creds)).to eq(iam_creds) end end From f13c0735c664df580728645bdb2664df0abb268b Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 16:57:19 -0400 Subject: [PATCH 09/14] Add tests for when iam_creds should be used --- spec/create_spec.rb | 64 ++++++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 4e20db1b..63c25978 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -43,7 +43,7 @@ let(:iam_creds) do { - aws_access_key_id: 'siam_creds_access_keu', + aws_access_key_id: 'iam_creds_access_key', aws_secret_access_key: 'iam_creds_secret_access_key', aws_session_token: 'iam_creds_session_token' } @@ -136,29 +136,57 @@ end - context 'When #iam_creds returns values but they should not be used' do - context 'because :aws_access_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token]' do - config[:aws_access_key_id] = 'adifferentkey' - allow(driver).to receive(:iam_creds).and_return(iam_creds) - expect(driver.send(:config)[:aws_session_token]).to be_nil + context 'When #iam_creds returns values' do + context 'but they should not be used' do + context 'because :aws_access_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token]' do + config[:aws_access_key_id] = 'adifferentkey' + allow(driver).to receive(:iam_creds).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end end - end - context 'because :aws_secret_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token]' do - config[:aws_secret_access_key] = 'adifferentsecret' - allow(driver).to receive(:iam_creds).and_return(iam_creds) - expect(driver.send(:config)[:aws_session_token]).to be_nil + context 'because :aws_secret_key_id is not set via iam_creds' do + it 'does not set config[:aws_session_token]' do + config[:aws_secret_access_key] = 'adifferentsecret' + allow(driver).to receive(:iam_creds).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end + end + + context 'because :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do + it 'does not set config[:aws_session_token]' do + config[:aws_access_key_id] = 'adifferentkey' + config[:aws_secret_access_key] = 'adifferentsecret' + allow(driver).to receive(:iam_creds).and_return(iam_creds) + expect(driver.send(:config)[:aws_session_token]).to be_nil + end end end - context 'because :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do - it 'does not set config[:aws_session_token]' do - config[:aws_access_key_id] = 'adifferentkey' - config[:aws_secret_access_key] = 'adifferentsecret' + context 'and they should be used' do + let(:config) do + { + aws_ssh_key_id: 'larry', + user_data: nil + } + end + + before do + allow(ENV).to receive(:[]).and_return(nil) allow(driver).to receive(:iam_creds).and_return(iam_creds) - expect(driver.send(:config)[:aws_session_token]).to be_nil + end + + it 'uses :aws_access_key_id from iam_creds' do + expect(driver.send(:config)[:aws_access_key_id]).to eq(iam_creds[:aws_access_key_id]) + end + + it 'uses :aws_secret_key_id from iam_creds' do + expect(driver.send(:config)[:aws_secret_key_id]).to eq(iam_creds[:aws_secret_key_id]) + end + + it 'uses :aws_session_token from iam_creds' do + expect(driver.send(:config)[:aws_session_token]).to eq(iam_creds[:aws_session_token]) end end end From d352cb44373246ce64a673bdfb69c06ccae9675a Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 17:11:35 -0400 Subject: [PATCH 10/14] Add even MORE tests --- spec/create_spec.rb | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 63c25978..9cc746d4 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -138,28 +138,44 @@ context 'When #iam_creds returns values' do context 'but they should not be used' do - context 'because :aws_access_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token]' do - config[:aws_access_key_id] = 'adifferentkey' - allow(driver).to receive(:iam_creds).and_return(iam_creds) + let(:config) do + { + aws_ssh_key_id: 'larry', + user_data: nil + } + end + + before do + allow(ENV).to receive(:[]).and_return(nil) + allow(driver).to receive(:iam_creds).and_return(iam_creds) + end + + context 'because :aws_access_key_id is explicitly set' do + before { config[:aws_access_key_id] = 'adifferentkey' } + it 'does not override :aws_access_key_id' do + expect(driver.send(:config)[:aws_access_key_id]).to eq('adifferentkey') + end + + it 'does not set :aws_session_token' do expect(driver.send(:config)[:aws_session_token]).to be_nil end end - context 'because :aws_secret_key_id is not set via iam_creds' do - it 'does not set config[:aws_session_token]' do - config[:aws_secret_access_key] = 'adifferentsecret' - allow(driver).to receive(:iam_creds).and_return(iam_creds) + context 'because :aws_access_key_id is explicitly set' do + before { config[:aws_secret_access_key] = 'adifferentsecret' } + it 'does not override :aws_secret_access_key' do + expect(driver.send(:config)[:aws_secret_access_key]).to eq('adifferentsecret') + end + + it 'does not set :aws_session_token' do expect(driver.send(:config)[:aws_session_token]).to be_nil end end - context 'because :aws_secret_key_id and :aws_access_key_id are set via iam_creds' do - it 'does not set config[:aws_session_token]' do - config[:aws_access_key_id] = 'adifferentkey' - config[:aws_secret_access_key] = 'adifferentsecret' - allow(driver).to receive(:iam_creds).and_return(iam_creds) - expect(driver.send(:config)[:aws_session_token]).to be_nil + context 'because :aws_session_token is explicitly set' do + before { config[:aws_session_token] = 'adifferentsessiontoken' } + it 'does not override :aws_session_token' do + expect(driver.send(:config)[:aws_session_token]).to eq('adifferentsessiontoken') end end end From 40ce055eec8df8aa4d01fb214eaee7856374419e Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 17:35:34 -0400 Subject: [PATCH 11/14] Since autodetecting iam is all or nothing update specs to reflect that --- spec/create_spec.rb | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 9cc746d4..2985600a 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -156,8 +156,14 @@ expect(driver.send(:config)[:aws_access_key_id]).to eq('adifferentkey') end - it 'does not set :aws_session_token' do - expect(driver.send(:config)[:aws_session_token]).to be_nil + it 'does not set :aws_secret_access_key via #iam_creds' do + expect(driver.send(:config)[:aws_secret_access_key]) + .to_not eq(iam_creds[:aws_secret_access_key]) + end + + it 'does not set :aws_session_token via #iam_creds' do + expect(driver.send(:config)[:aws_session_token]) + .to_not eq(iam_creds[:aws_session_token]) end end @@ -167,8 +173,14 @@ expect(driver.send(:config)[:aws_secret_access_key]).to eq('adifferentsecret') end - it 'does not set :aws_session_token' do - expect(driver.send(:config)[:aws_session_token]).to be_nil + it 'does not set :aws_access_key_id via #iam_creds' do + expect(driver.send(:config)[:aws_access_key_id]) + .to_not eq(iam_creds[:aws_access_key_id]) + end + + it 'does not set :aws_session_token via #iam_creds' do + expect(driver.send(:config)[:aws_session_token]) + .to_not eq(iam_creds[:aws_session_token]) end end @@ -177,6 +189,16 @@ it 'does not override :aws_session_token' do expect(driver.send(:config)[:aws_session_token]).to eq('adifferentsessiontoken') end + + it 'does not set :aws_access_key_id via #iam_creds' do + expect(driver.send(:config)[:aws_access_key_id]) + .to_not eq(iam_creds[:aws_access_key_id]) + end + + it 'does not set :aws_secret_access_key via #iam_creds' do + expect(driver.send(:config)[:aws_secret_access_key]) + .to_not eq(iam_creds[:aws_secret_access_key]) + end end end From e1d5ae431620e9b825448f3f03903508a7ab47e9 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 18:40:39 -0400 Subject: [PATCH 12/14] Remove unessecary tests and make all tests pass * only set :aws_session_token via iam_creds in the event that :aws_access_key_id and :aws_secret_access_key are BOTH already set via iam_creds * prefer environment over iam_creds when setting :aws_session_token I think this is the best I can do. All unprevented combinations of autodetectable credentials via iam_creds and user defined settings should simply fail authentication. --- lib/kitchen/driver/ec2.rb | 13 +++++++++++-- spec/create_spec.rb | 29 +++++------------------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index ea2950e3..d55679d1 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -49,8 +49,7 @@ class Ec2 < Kitchen::Driver::SSHBase driver.iam_creds[:aws_secret_access_key] end default_config :aws_session_token do |driver| - ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] || - driver.iam_creds[:aws_session_token] + driver.default_aws_session_token end default_config :aws_ssh_key_id do |driver| ENV['AWS_SSH_KEY_ID'] @@ -177,6 +176,16 @@ def default_public_ip_association !!config[:subnet_id] end + def default_aws_session_token + env = ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN'] + if config[:aws_secret_access_key] == iam_creds[:aws_secret_access_key] \ + && config[:aws_access_key_id] == iam_creds[:aws_access_key_id] + env || iam_creds[:aws_session_token] + else + env + end + end + private def connection diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 2985600a..53e4dd36 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -148,57 +148,38 @@ before do allow(ENV).to receive(:[]).and_return(nil) allow(driver).to receive(:iam_creds).and_return(iam_creds) + allow(Net::HTTP).to receive(:get).with(URI.parse('http://169.254.169.254')).and_return(true) end - context 'because :aws_access_key_id is explicitly set' do + context 'because :aws_access_key_id is set but not via #iam_creds' do before { config[:aws_access_key_id] = 'adifferentkey' } it 'does not override :aws_access_key_id' do expect(driver.send(:config)[:aws_access_key_id]).to eq('adifferentkey') end - it 'does not set :aws_secret_access_key via #iam_creds' do - expect(driver.send(:config)[:aws_secret_access_key]) - .to_not eq(iam_creds[:aws_secret_access_key]) - end - it 'does not set :aws_session_token via #iam_creds' do expect(driver.send(:config)[:aws_session_token]) .to_not eq(iam_creds[:aws_session_token]) end end - context 'because :aws_access_key_id is explicitly set' do + context 'because :aws_access_key_id is set but not via #iam_creds' do before { config[:aws_secret_access_key] = 'adifferentsecret' } it 'does not override :aws_secret_access_key' do expect(driver.send(:config)[:aws_secret_access_key]).to eq('adifferentsecret') end - it 'does not set :aws_access_key_id via #iam_creds' do - expect(driver.send(:config)[:aws_access_key_id]) - .to_not eq(iam_creds[:aws_access_key_id]) - end - it 'does not set :aws_session_token via #iam_creds' do expect(driver.send(:config)[:aws_session_token]) .to_not eq(iam_creds[:aws_session_token]) end end - context 'because :aws_session_token is explicitly set' do + context 'because :aws_session_token is set but not via #iam_creds' do before { config[:aws_session_token] = 'adifferentsessiontoken' } it 'does not override :aws_session_token' do expect(driver.send(:config)[:aws_session_token]).to eq('adifferentsessiontoken') end - - it 'does not set :aws_access_key_id via #iam_creds' do - expect(driver.send(:config)[:aws_access_key_id]) - .to_not eq(iam_creds[:aws_access_key_id]) - end - - it 'does not set :aws_secret_access_key via #iam_creds' do - expect(driver.send(:config)[:aws_secret_access_key]) - .to_not eq(iam_creds[:aws_secret_access_key]) - end end end @@ -249,7 +230,7 @@ end end - context 'when #fetch_credentials fails with StandardError' do + context 'when #fetch_credentials fails with ::StandardError' do it 'returns an empty hash' do allow(driver).to receive(:fetch_credentials).and_raise(::StandardError) expect(driver.iam_creds).to eq({}) From 16302ab634cea1adeedeeb3f831501e884452018 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Thu, 9 Apr 2015 18:58:10 -0400 Subject: [PATCH 13/14] Adjust tests to pass in travis --- spec/create_spec.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index 53e4dd36..bf190122 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -138,13 +138,6 @@ context 'When #iam_creds returns values' do context 'but they should not be used' do - let(:config) do - { - aws_ssh_key_id: 'larry', - user_data: nil - } - end - before do allow(ENV).to receive(:[]).and_return(nil) allow(driver).to receive(:iam_creds).and_return(iam_creds) @@ -152,9 +145,8 @@ end context 'because :aws_access_key_id is set but not via #iam_creds' do - before { config[:aws_access_key_id] = 'adifferentkey' } it 'does not override :aws_access_key_id' do - expect(driver.send(:config)[:aws_access_key_id]).to eq('adifferentkey') + expect(driver.send(:config)[:aws_access_key_id]).to eq(config[:aws_access_key_id]) end it 'does not set :aws_session_token via #iam_creds' do @@ -164,9 +156,8 @@ end context 'because :aws_access_key_id is set but not via #iam_creds' do - before { config[:aws_secret_access_key] = 'adifferentsecret' } it 'does not override :aws_secret_access_key' do - expect(driver.send(:config)[:aws_secret_access_key]).to eq('adifferentsecret') + expect(driver.send(:config)[:aws_secret_access_key]).to eq(config[:aws_secret_access_key]) end it 'does not set :aws_session_token via #iam_creds' do From ac3d1c4e1f8b5a0399f7cafa16c6a312835bd4c7 Mon Sep 17 00:00:00 2001 From: James Awesome Date: Fri, 10 Apr 2015 08:08:48 -0400 Subject: [PATCH 14/14] Correct order of stubbing to allow iam_creds to work in test suite --- spec/create_spec.rb | 72 +++++++++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 19 deletions(-) diff --git a/spec/create_spec.rb b/spec/create_spec.rb index bf190122..47a38df3 100644 --- a/spec/create_spec.rb +++ b/spec/create_spec.rb @@ -49,14 +49,13 @@ } end + context 'Interface is set in config' do before do instance allow(driver).to receive(:create_server).and_return(server) allow(driver).to receive(:wait_for_sshd) end - context 'Interface is set in config' do - it 'derives hostname from DNS when specified in the .kitchen.yml' do config[:interface] = 'dns' driver.create(state) @@ -83,6 +82,11 @@ end context 'Interface is derived automatically' do + before do + instance + allow(driver).to receive(:create_server).and_return(server) + allow(driver).to receive(:wait_for_sshd) + end let(:server) do double(:id => "123", @@ -122,6 +126,11 @@ end context 'user_data implementation is working' do + before do + instance + allow(driver).to receive(:create_server).and_return(server) + allow(driver).to receive(:wait_for_sshd) + end it 'user_data is not defined' do driver.create(state) @@ -137,16 +146,28 @@ end context 'When #iam_creds returns values' do - context 'but they should not be used' do - before do - allow(ENV).to receive(:[]).and_return(nil) - allow(driver).to receive(:iam_creds).and_return(iam_creds) - allow(Net::HTTP).to receive(:get).with(URI.parse('http://169.254.169.254')).and_return(true) - end + let(:config) do + { + aws_ssh_key_id: 'larry', + user_data: nil + } + end + + before do + allow(ENV).to receive(:[]).and_return(nil) + allow(driver).to receive(:iam_creds).and_return(iam_creds) + instance + end + context 'but they should not be used' do context 'because :aws_access_key_id is set but not via #iam_creds' do + let(:aws_access_key_id) { 'secret' } + before do + config[:aws_access_key_id] = aws_access_key_id + end + it 'does not override :aws_access_key_id' do - expect(driver.send(:config)[:aws_access_key_id]).to eq(config[:aws_access_key_id]) + expect(driver.send(:config)[:aws_access_key_id]).to eq(aws_access_key_id) end it 'does not set :aws_session_token via #iam_creds' do @@ -155,9 +176,15 @@ end end - context 'because :aws_access_key_id is set but not via #iam_creds' do + context 'because :aws_secret_access_key is set but not via #iam_creds' do + let(:aws_secret_access_key) { 'moarsecret' } + + before do + config[:aws_secret_access_key] = aws_secret_access_key + end + it 'does not override :aws_secret_access_key' do - expect(driver.send(:config)[:aws_secret_access_key]).to eq(config[:aws_secret_access_key]) + expect(driver.send(:config)[:aws_secret_access_key]).to eq(aws_secret_access_key) end it 'does not set :aws_session_token via #iam_creds' do @@ -167,7 +194,10 @@ end context 'because :aws_session_token is set but not via #iam_creds' do - before { config[:aws_session_token] = 'adifferentsessiontoken' } + let(:aws_session_token) { 'adifferentsessiontoken' } + before do + config[:aws_session_token] = aws_session_token + end it 'does not override :aws_session_token' do expect(driver.send(:config)[:aws_session_token]).to eq('adifferentsessiontoken') end @@ -182,11 +212,6 @@ } end - before do - allow(ENV).to receive(:[]).and_return(nil) - allow(driver).to receive(:iam_creds).and_return(iam_creds) - end - it 'uses :aws_access_key_id from iam_creds' do expect(driver.send(:config)[:aws_access_key_id]).to eq(iam_creds[:aws_access_key_id]) end @@ -204,19 +229,20 @@ describe '#iam_creds' do context 'when a metadata service is available' do before do - allow(Net::HTTP).to receive(:get).with(URI.parse('http://169.254.169.254')).and_return(true) + allow(Net::HTTP).to receive(:get).and_return(true) end context 'and #fetch_credentials returns valid iam credentials' do it '#iam_creds retuns the iam credentials from fetch_credentials' do allow(driver).to receive(:fetch_credentials).and_return(iam_creds) - expect(driver.send(:iam_creds)).to eq(iam_creds) + expect(driver.iam_creds).to eq(iam_creds) end end context 'when #fetch_credentials fails with NoMethodError' do it 'returns an empty hash' do allow(driver).to receive(:fetch_credentials).and_raise(NoMethodError) + expect { driver.fetch_credentials }.to raise_error(NoMethodError) expect(driver.iam_creds).to eq({}) end end @@ -224,6 +250,7 @@ context 'when #fetch_credentials fails with ::StandardError' do it 'returns an empty hash' do allow(driver).to receive(:fetch_credentials).and_raise(::StandardError) + expect { driver.fetch_credentials }.to raise_error(::StandardError) expect(driver.iam_creds).to eq({}) end end @@ -231,6 +258,7 @@ context 'when #fetch_credentials fails with Errno::EHOSTUNREACH' do it 'returns an empty hash' do allow(driver).to receive(:fetch_credentials).and_raise(Errno::EHOSTUNREACH) + expect { driver.fetch_credentials }.to raise_error(Errno::EHOSTUNREACH) expect(driver.iam_creds).to eq({}) end end @@ -238,6 +266,7 @@ context 'when #fetch_credentials fails with Timeout::Error' do it 'returns an empty hash' do allow(driver).to receive(:fetch_credentials).and_raise(Timeout::Error) + expect { driver.fetch_credentials }.to raise_error(Timeout::Error) expect(driver.iam_creds).to eq({}) end end @@ -253,6 +282,11 @@ end describe '#block_device_mappings' do + before do + instance + allow(driver).to receive(:create_server).and_return(server) + allow(driver).to receive(:wait_for_sshd) + end let(:connection) { double(Fog::Compute) } let(:image) { double('Image', :root_device_name => 'name') } before do