-
Notifications
You must be signed in to change notification settings - Fork 30
Prioritize assume role credentials and deprecate built-in network providers #773
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
Changes from all commits
5469b53
36edf46
a179559
a112dd6
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": "breaking", | ||
| "description": "Updated the credential chain precedence so assume role credentials are resolved before session and static profile keys." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "enhancement", | ||
| "description": "Updated profile session and static key providers to defer when the selected profile declares an assume-role configuration." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "enhancement", | ||
| "description": "Deprecated the built-in IMDS and container credentials resolvers in favor of the resolvers provided by the `aws-credentials-imds` and `aws-credentials-http` packages." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "enhancement", | ||
| "description": "Updated environment credentials provider to defer when `profile_name` is passed to `IdentityChain.create()`." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| _SECRET_ACCESS_KEY = "aws_secret_access_key" # noqa: S105 | ||
| _SESSION_TOKEN = "aws_session_token" # noqa: S105 | ||
| _ACCOUNT_ID = "aws_account_id" | ||
| _ROLE_ARN = "role_arn" | ||
|
|
||
|
|
||
| class ProfileSessionCredentialsProvider: | ||
|
|
@@ -36,6 +37,9 @@ async def setup(self, identity_type: type[Identity], setup: ChainSetup) -> None: | |
| if config_file is None or profile_name is None: | ||
| return | ||
|
|
||
| if config_file.get(profile_name, _ROLE_ARN) is not None: | ||
|
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. [The comment applies to line 81 as well] With this deferment, consider a user whose profile looks like: [default]
aws_access_key_id = AKIA...
aws_secret_access_key = ...
role_arn = arn:aws:iam::123:role/Foo
source_profile = defaultbut who doesn't have Is this a gap we should address?
Contributor
Author
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. I am aware of this gap. However adding it to this PR expands its scope. This PR focuses on behavioral changes we want to get in before our next release. The module suggestions improves customer experience but doesn't change behavior. I already have a follow up PR to add better module suggestions which will add profile based provider suggestions to |
||
| return | ||
|
|
||
| access_key_id = config_file.get(profile_name, _ACCESS_KEY_ID) | ||
| secret_access_key = config_file.get(profile_name, _SECRET_ACCESS_KEY) | ||
| session_token = config_file.get(profile_name, _SESSION_TOKEN) | ||
|
|
@@ -74,6 +78,9 @@ async def setup(self, identity_type: type[Identity], setup: ChainSetup) -> None: | |
| if config_file is None or profile_name is None: | ||
| return | ||
|
|
||
| if config_file.get(profile_name, _ROLE_ARN) is not None: | ||
| return | ||
|
|
||
| access_key_id = config_file.get(profile_name, _ACCESS_KEY_ID) | ||
| secret_access_key = config_file.get(profile_name, _SECRET_ACCESS_KEY) | ||
| if access_key_id is None or secret_access_key is None: | ||
|
|
||
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.
PR #771 passes
config_context.profile_nameintoIdentityChain.create(), but it looks likeSharedConfigContext.profile_nameis neverNone? It falls back to AWS_PROFILE, then default.smithy-python/packages/smithy-aws-core/src/smithy_aws_core/config/context.py
Lines 184 to 201 in c0150aa
So once both PRs land, a user with just
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEYset andno profile configured still gets the environment provider skipped.
Since the context already tracks
profile_origin, should #771 pass the profile only when it came from the explicit argument?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.
Good catch. I'll update #771 to only set the profile if its origin is an explicit override.