-
Notifications
You must be signed in to change notification settings - Fork 37
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
Feature/fix cred chain #20
Conversation
* Credential chain validation will now try, in order: - static variables - environment variables - shared credentials file - AWS SDK session-derived credentials (including shared config file) - ECS and EC2 metadata agents * Previous behavior ignored the config file when running in ECS/EC2, because those credentials would prematurely satisfy the contract. This prevented the use of config file options, in particular credentials_source = EcsContainer or EC2InstanceMetadata
How's the progress on this getting merged in? I'm trying to launch a terraform job via cci using fargate, and I am running into this issue. |
@ian-axelrod I've been using this workaround in the interim: first |
Yup, that is what I ended up doing after I posted the question. It works fine, but is of course not ideal :). Cheers! |
My setup is really hurting due to #19 , is there any work-around for that? Can that fix be fast-tracked at all? Anything we can do to at all? This fix has been sitting for 2 months already... |
This is currently affecting us too, would be awesome to get this in ASAP. Can we have some eyes on this? @radeksimko @aeschright |
Thank you for submitting this, @nkupton! Please note that I will be looking to pull this in after #32, hopefully today or tomorrow, since that introduces the necessary unit testing that verifies the bug. 👍 This in a potentially breaking change for a lot of practitioners who may be dependent on the (unfortunate) current preference of EC2 Metadata over ECS Credentials, so it will only be included in Terraform AWS Provider v3.0.0 and Terraform S3 Backend (bundled with Terraform CLI) v0.13.0, both of which are releasing in the next few weeks. |
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.
Looks good and fixes so many issues. Thanks so much, @nkupton 🚀
Proposal to fix issues like:
#7
hashicorp/terraform-provider-aws#5018
The recent introduction of session-derived credentials from the AWS SDK was a huge improvement to searching the supported methods of loading credentials. However that code path was only being exercised on error, after validation of the original credential chain. In ECS and EC2 environments, this would never produce an error, so long as the metadata service returned its default credentials. The end result was to ignore the contents of any ~/.aws/config file, which is an important method of introducing other options (assume_role, etc). The root cause is also well documented in #7
To fix, I have moved the ECS and EC2 metadata credentials fetching to the end of the validation chain. This allows the proper use of credentials_source = EcsContainer or EC2InstanceMetadata in earlier paths for loading credentials, and eliminates need to use the skip_metadata_api_check workaround when in EC2.
I've also included a fix for #19
Fix order for credentials chain, building upon Dirk Avery's work.
Add missing profile #19