-
Notifications
You must be signed in to change notification settings - Fork 202
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 support for IAM role credentials #104
Conversation
noooo, line's too long... great |
Yes :) passes the tests finaly |
@fnichol do you have any comments? |
After a bit of throughout, I've reordered the precedence of aim_role_credentials. |
👍 |
1 similar comment
👍 |
Now using this on our jenkins server. |
Why do you prefer this PR over #68 ? I made a comment in that PR that would apply here too. You also should not be using an |
Thanks for reply @tyler-ball This PR builds on top of #68. Unfortuantely @daanemanz deleted his fork so I couldn't contribute to original PR. Where should include go? just above the iam_cred request? |
I'll integrate your comments from the other PR. |
@Igorshp When you include the module you add The include can go on the line directly below the |
@tyler-ball i've implemented your comments into the PR. The iam_creds method had to be come static though or it woudn't be allowed to be called during class initialisation. Any preference of extend/include? Extend works well right now and as you said doesn't require full namespace prefix, include still does (even when calling from method). |
As mentioned in test-kitchen#55. This change will add support for using IAM temporary credentials when creating EC2 instances. Currently the credentials will have to be set in the environment, while an EC2 instance that's set up with an IAM profile can fetch its temporary credentials from the metadata server.
tweak for PR by daanemanz test-kitchen#68 he has since deleted his fork, so I can't submit a PR to him.
using aim_role should be a fallback, not a priority. This will allow a node with aim role to still use custom keys if needed
hooray for passing checks!
rebased on master.... |
@iam_creds ||= begin | ||
fetch_credentials(use_iam_profile:true) | ||
rescue | ||
debug("fetch_credentials failed with exception #{e.message}:#{e.backtrace.join("\n")}") |
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.
The var e
doesn't exist here because you haven't declared it. The rescue line should look like rescue RuntimeError => e
I had 1 more comment, and it looks like Travis is failing with Tailor code style errors. Fix those 2 things and this should be good to merge! |
👍 |
@tyler-ball bah, totally skipped over that. Thanks for picking it up! |
👍 |
Add support for IAM role credentials
Thanks for this PR! |
Thanks for the merge @tyler-ball! |
When using this on a local machine I am experiencing an issue when the 'kitchen converge' is always crashing because the fetch_credentials() can't get the AWS metadata from 169.254.169.254. Because of this it makes kitchen-ec2 unusable if it is not ran on an EC2 instance.
Is this happening because self.iam_creds is always calling fetch_credentials() even if is not needed? |
@otanner how are you running kitchen-ec2 plugin localy? |
Hi guys, I'm having the same problem here. I'm using ChefDK's (0.4.0) Test-Kitchen just by running "kitchen converge" - and I can repeat this behaviour by upgrading to latest 0.8.1-dev as well. |
@Igorshp I am setting the driver name to ec2 and putting the needed AWS configuration in place (api keys, region, etc). With the kitchen-ec2 0.8.0 the same configuration works. I'm running ChefDK's Test-Kitchen on OS X. |
@otanner can you check your kitchen logs and see what is the error raised by Fog? The |
@tyler-ball @Igorshp Here is an unsuccessful run from kitchen.log. Notice the one minute delay between starting the kitchen and getting the error as it waits for the network timeout. So even if the rescue would (which it apparently does not) work and catch the error in the end it would cause unnecessary one minute delay on each run. Can there be a bug in Fog as well because the code falls back with 'super' but the parent class doesn't have a suitable method to run?
|
This is a patch for PR by @daanemanz - #68
The problem was that fetch_credentials does not exist in the context and would always fall back to rescue block.
The original issue is #55