Skip to content
Unverified Commit 42751e5d authored by Aaron Todd's avatar Aaron Todd Committed by GitHub
Browse files

fix default credential chain not respecting endpoint URL overrides (#3873)

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
https://github.com/awslabs/aws-sdk-rust/issues/1193

## Description
This PR fixes a customer reported bug where the default chain doesn't
respect `AWS_ENDPOINT_URL`/`AWS_ENDPOINT_URL_<SERVICE>` environment
variables or the equivalents in AWS shared config (`~/.aws/config`).

This fix is a little nuanced and frankly gross but there isn't a better
option that I can see right now that isn't way more invasive. The crux
of the issue is that when we implemented support for this feature
([1](https://github.com/smithy-lang/smithy-rs/pull/3568),
[2](https://github.com/smithy-lang/smithy-rs/pull/3493),
[3](https://github.com/smithy-lang/smithy-rs/pull/3488)) we really only
made it work for clients created via
[`ConfigLoader::load()`](https://github.com/smithy-lang/smithy-rs/blob/release-2024-10-09/aws/rust-runtime/aws-config/src/lib.rs#L871).
Internally the default chain credential provider constructs `STS` and
`SSO` clients but it does so using
[`ProviderConfig`](https://github.com/smithy-lang/smithy-rs/blob/release-2024-10-09/aws/rust-runtime/aws-config/src/provider_config.rs#L36)
by mapping this to `SdkConfig` via
[`ProviderConfig::client_config()`](https://github.com/smithy-lang/smithy-rs/blob/release-2024-10-09/aws/rust-runtime/aws-config/src/provider_config.rs#L199).
This conversion is used in several places and it doesn't take any of the
required logic into account to setup
[`EnvServiceConfig`](https://github.com/smithy-lang/smithy-rs/blob/release-2024-10-09/aws/rust-runtime/aws-config/src/lib.rs#L859-L862)
which is what generated SDK's ultimately use to figure out the endpoint
URL from either environment/profile ([example
client](https://github.com/awslabs/aws-sdk-rust/blob/release-2024-10-09/sdk/sts/src/config.rs#L1214-L1221)
which ultimately ends up in `EnvServiceConfig`
[here](https://github.com/smithy-lang/smithy-rs/blob/release-2024-10-09/aws/rust-runtime/aws-config/src/env_service_config.rs#L18)).

The fix applied here is nuanced in that we update the conversion to
provide a `EnvServiceConfig` but it relies on the profile to have been
parsed already or else you'll get an empty/default profile. This
generally works for the profile provider since the first thing we do is
load the profile but in isolation it may not work as expected. I've
added tests for STS to cover all cases but SSO credentials and token
providers do NOT currently respect shared config endpoint URL keys.
Fixing this is possible but involved since we require an `async` context
to ensure a profile is loaded already and in many places where we
construct `SdkConfig` from `ProviderConfig` we are in non async
function.

## Testing
Tested repro + additional integration tests

## Future
This does _not_ fix https://github.com/awslabs/aws-sdk-rust/issues/1194
which was discovered as a bug/gap. Fixing it would be outside the scope
of this PR.

SSO/token provider is instantiated sometimes before we have parsed a
profile. This PR definitely fixes the STS provider for all configuration
scenarios but the SSO related client usage may still have some edge
cases when configured via profiles since we often instantiate them
before parsing a profile. When we surveyed other SDKs there were several
that failed to respect these variables and haven't received issues
around this which leads me to believe this isn't likely a problem in
practice (most likely due to SSO being used in local development most
often where redirecting that endpoint doesn't make much sense anyway).

## Checklist
- [X] For changes to the AWS SDK, generated SDK code, or SDK runtime
crates, I have created a changelog entry Markdown file in the
`.changelog` directory, specifying "aws-sdk-rust" in the `applies_to`
key.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 4b66264d
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment