-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Shared Configuration Support #1178
Conversation
This change adds the `Aws::SharedConfig` file, which parses both the Shared Credentials file and the Shared Config file. From that, functions are provided to support: * Credentials from shared credentials or shared config. * Default region selection from shared config. * Assume role credentials from shared config. Shared config support is toggled behind the `AWS_SDK_LOAD_CONFIG` environment variable. Assume role credentials support is working, but not yet wired in to any default credential provider chain. I'd also still like to test edge cases, as CLI configuration supports many more values than the shared credential file.
@@ -51,4 +51,5 @@ end | |||
|
|||
group :repl do | |||
gem 'pry' | |||
gem 'byebug' |
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 will want to remove byebug from the Gemfile and gitignore.
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.
I've removed it from the Gemfile, I still think it's fine in the .gitignore
file unless we would intend on tracking some sort of byebug history in the repo at any point.
This doesn't appear to support any of the other possible shared configuration values from |
Brings this change up to spec for credentials, assume role, and region support. Includes code review feedback to this point, and relevant test improvements. Still to be done is the addition of the Amazon Simple Storage Service specific configuration values, and the wiring of Assume Role to the credential provider chain.
Is there an ETA on this getting finished and merged? We have a large environment (including several ruby tools) which is moving to role assumption for our core architecture. |
Adds `:assume_role_credentials` to default credential provider chain, used only if the `AWS_SDK_LOAD_CONFIG` environment variable is set. Improves usability of the default assume role behavior, and creates a credential provider chain test to ensure resolution ordering.
@temujin9 This last commit is the end of the planning/development cycle. Once we finish reviewing and ensuring that the test harness is solid (since this touches a lot of code that's run by just about every single client construction) we're intending to release. |
@awood45 Thanks for the follow-up. Can I ask you ping here when it does release? |
When we release this, you'll definitely see a note here and it will be a minor version bump, for e.g. to |
enabled = ENV["AWS_SDK_LOAD_CONFIG"] ? true : false | ||
@shared_config ||= SharedConfig.new(config_enabled: true) | ||
end | ||
|
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.
Minor nitpick. I try to limit the top-level utility functions. A simple change would be to keep this function inside the Aws::SharedConfig
module. You could name the method something like .default_config
, e.g Aws::SharedConfig.default_config
.
I finished going though this today. I have some additional feedback from simply a code-organization perspective that should affect functionality. I can go over this with you in person. If you add |
Also fixes bad fallback condition for Assume Role.
I've marked the |
Ship it! |
Shipment received. Thanks, guys. |
This change adds the
Aws::SharedConfig
file, which parses both theShared Credentials file and the Shared Config file. From that, functions
are provided to support:
Shared config support is toggled behind the
AWS_SDK_LOAD_CONFIG
environment variable.
Assume role credentials support is working, but not yet wired in to any
default credential provider chain. I'd also still like to test edge
cases, as CLI configuration supports many more values than the shared
credential file.