-
Notifications
You must be signed in to change notification settings - Fork 112
AWS_PROFILE Env Var Support #15
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
Conversation
Following convention, if the command line arguments does not specify a profile, we check the AWS_PROFILE. The sdk session options is also initialised with SharedConfigEnable to support shared configurations from the (~/.aws/config). https://github.com/aws/aws-sdk-go#configuring-credentials
Following convention, if the command line arguments does not specify a profile, we check the AWS_PROFILE. The sdk session options is also initialised with SharedConfigEnable to support shared configurations from the (~/.aws/config). https://github.com/aws/aws-sdk-go#configuring-credentials
Codecov Report
@@ Coverage Diff @@
## master #15 +/- ##
=======================================
Coverage 89.60% 89.60%
=======================================
Files 7 7
Lines 779 779
=======================================
Hits 698 698
Misses 53 53
Partials 28 28 Continue to review full report at Codecov.
|
bwagner5
left a comment
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.
remove unneeded custom region fill in for profiles
|
|
||
| if profileName != nil { | ||
| sessOpts.Profile = *profileName | ||
| if sessOpts.Config.Region == nil { |
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.
can you delete this block as well? I'm fairly certain this block is no longer needed since the SDK is now doing the work of populating a region in the right precedence.
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.
you are right; Just tested without the block, and read the https://docs.aws.amazon.com/sdk-for-go/api/aws/session/#SharedConfigState carefully; Indeed not needed.
By setting up the sharedConfigState to enabled the SDK does take care of the AWS_PROFILE population: https://docs.aws.amazon.com/sdk-for-go/api/aws/session/#SharedConfigState
bwagner5
left a comment
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.
lgtm! thanks! 🥳
*Issue #14
Description of changes:
There is currently no testing around the profile flag/env-var, I've done manual testing and verified rest of test cases work. Happy to take pointers on the testing side.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.