Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicit option for using iam profile for authentication #107

Merged
merged 14 commits into from
Apr 10, 2015
Merged
25 changes: 17 additions & 8 deletions lib/kitchen/driver/ec2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ module Driver
#
# @author Fletcher Nichol <[email protected]>
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'
Expand All @@ -42,13 +41,14 @@ 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'] || 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'] \
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to break this across multiple lines you don't need the \ - just leave the || at the end of the line

|| 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]
driver.default_aws_session_token
end
default_config :aws_ssh_key_id do |driver|
ENV['AWS_SSH_KEY_ID']
Expand Down Expand Up @@ -98,10 +98,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
fetch_credentials(use_iam_profile: true)
rescue RuntimeError, NoMethodError => e
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do you expect the NoMethodError to come from?

Copy link
Author

Choose a reason for hiding this comment

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

fetch_credentials calls super when it fails, so since we don't have a super for it to fail over to we need to catch NoMethodError, I think I made a note in the commit message.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still have the same question about NoMethodError - why is it needed? After including the ServiceMethods, you shouldn't ever encounter a NoMethodError. Or am I missing something?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

NM, I was able to reproduce the error from https://github.com/test-kitchen/kitchen-ec2/pull/104/files locally and I see why you have the NoMethodError check

debug("fetch_credentials failed with exception #{e.message}:#{e.backtrace.join("\n")}")
{}
end
Expand Down Expand Up @@ -167,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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the user required to specify the same access key / secret as returned by the iam_creds query? If they have to do that, why not just use the provided access key / secret for authentication?

I thought the primary point of the iam_creds was they allowed the EC2 node acting as a provisioner to provision nodes using the local credentials.

Copy link
Author

Choose a reason for hiding this comment

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

Actually I'm just checking that config[:aws_secret_access_key] et all had fallen back to the iam_creds statement so that we know it would be logical to return the iam_creds[:aws_session_token] since our other config values are from iam.

ENV['AWS_SESSION_TOKEN'] || ENV['AWS_TOKEN']
end
end

private

def connection
Expand Down
44 changes: 44 additions & 0 deletions spec/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,50 @@

end

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 :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)

expect(driver)
.to receive(:iam_creds)

expect(driver.send(:config)[:aws_session_token]).to eq(iam_creds[:aws_session_token])
end
end
end

describe '#block_device_mappings' do
let(:connection) { double(Fog::Compute) }
let(:image) { double('Image', :root_device_name => 'name') }
Expand Down