Resolves #2171: Honor 'use_aws_shared_config_files' for process… #2172
Conversation
…ased CredentialProvider For the credential resolver whereby the default ~/.aws/config and ~/.aws/credentials are evaluated for access credentials, the config variable 'use_aws_shared_config_files' is considered. However, for the process-based credential resolver, checking the same $HOME directory shared config files, it is not. This adds the same conditional check to ensure those files are not considered for either method where configuration is explicitly supplied indicating they should not be.
Codecov Report
@@ Coverage Diff @@
## master #2172 +/- ##
============================================
- Coverage 93.00% 92.99% -0.01%
- Complexity 4021 4023 +2
============================================
Files 222 222
Lines 10848 10850 +2
============================================
+ Hits 10089 10090 +1
- Misses 759 760 +1
Continue to review full report at Codecov.
|
The default ordering is supposed to be: 1) env 2) web identity 3) profile config 4) Container/Instance The process type is a relative new add. However, it was put below ECS. In the Java SDK, all profile processing (and case logic) is together in the search order. This makes that match. See Also: https://github.com/aws/aws-sdk-java/blob/1.11.918/aws-java-sdk-core/src/main/java/com/amazonaws/auth/profile/ProfilesConfigFile.java#L207-L216
The default ordering is supposed to be: 1) env 2) web identity 3) profile config 4) Container/Instance ECS deployments and instance profiles are easily detectable via an environment variable. If the environment variable is present, add the ECS provider to the chain; otherwise, add the instance profile provider. See Also: https://github.com/aws/aws-sdk-java/blob/1.11.918/aws-java-sdk-core/src/main/java/com/amazonaws/auth/EC2ContainerCredentialsProviderWrapper.java#L58-L64
|
I've pushed a couple more commits to order this according to the documentation:
The ordering in the existing file is blended a bit. I've made the comment block and the code match the documentation. In addition, this ordering now matches the Java SDK for comparison The profile types are all lumped together: And then, there's a toggle between ECS and instance profile "either/or" versus "both": |
|
@cuppett Thanks for your PR, I agree this is a better order than we had before- could you create a changelog file? I also will close/re-open this to retrigger the github actions since this was pushed while we were in the process of migrating from Travis. |
Done. Thanks! |
Double checked and raw file shows them all in good order. The markup view with line breaks a little confusing how it interrupts some of the sentences. I've got each then, then, then on separate lines to let future adds and changes be easier to see and review; hope that helps! |
|
@cuppett I apologize for my confusion- I think I misread and the documentation is missing one step: the SSO provider. I saw the note about the CLI's default profile and my mind jumped to SSO. |
No worries, getting these right super important. |
|
Does this PR need flipped back to approved? |
Comment stanza to match up with the terminology used throughout the code.
|
Looks good! Going to wait for all the github actions to pass and will merge ASAP. Thank you for your contribution |
…-based CredentialProvider
Issue #, if available:
Resolves #2171
For the credential resolver whereby the default ~/.aws/config and ~/.aws/credentials are evaluated for access credentials, the config variable 'use_aws_shared_config_files' is considered. However, for the process-based credential resolver, checking the same $HOME directory shared config files, it is not. This adds the same conditional check to ensure those files are not considered for either method where configuration is explicitly supplied indicating they should not be.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.