-
Notifications
You must be signed in to change notification settings - Fork 31
fix: update env provider to skip terminal resolver registration if asume role creds are configured #766
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
base: develop
Are you sure you want to change the base?
fix: update env provider to skip terminal resolver registration if asume role creds are configured #766
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "bugfix", | ||
| "description": "Updated the Environment credentials provider to skip registering a terminal resolver when a configured profile assumes a role using `credential_source = Environment`." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,16 @@ | |
|
|
||
| from smithy_core.interfaces.identity import Identity | ||
|
|
||
| from ....config import load_config | ||
| from ...components import AWSCredentialsIdentity | ||
| from ...environment import EnvironmentCredentialsResolver | ||
| from ..ordering import Standard, StandardProvider | ||
| from ..provider import ChainSetup | ||
|
|
||
| _ACCESS_KEY_ID = "AWS_ACCESS_KEY_ID" | ||
| _SECRET_ACCESS_KEY = "AWS_SECRET_ACCESS_KEY" # noqa: S105 | ||
| _ROLE_ARN = "role_arn" | ||
| _CREDENTIAL_SOURCE = "credential_source" | ||
|
|
||
|
|
||
| class EnvironmentCredentialsProvider: | ||
|
|
@@ -38,4 +41,15 @@ async def setup( | |
| if not os.getenv(_ACCESS_KEY_ID) or not os.getenv(_SECRET_ACCESS_KEY): | ||
| return | ||
|
|
||
| # Skip environment provider if a profile is explicitly provided and | ||
| # that profile configures assume role credentials with 'Environment' as the | ||
| # credential source | ||
| profile_name = setup.profile_name | ||
| if profile_name: | ||
| config = setup.config_file or await load_config() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We aren't caching the config here which means we'll end up doing duplicate work. We need to ensure that we're always only reading the config file only once. Let's also keep this in mind when we integrate with @ubaskota's config resolver work. Also, might want to make this a method on setup or some other class so we can just reuse loading caching logic. |
||
| role_arn = config.get(profile_name, _ROLE_ARN) | ||
| credential_source = config.get(profile_name, _CREDENTIAL_SOURCE) | ||
| if role_arn is not None and credential_source == "Environment": | ||
| return | ||
|
|
||
| setup.add_terminal_resolver(EnvironmentCredentialsResolver()) | ||
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'm just now realizing that when a customer attempts to use the assume role provider and doesn't install
aws-credentials-sts, they don't get a helpful error message. They just get:Not sure if this is another gap in the SEP or a miss on our end. Can you look into this?