Sourcing service config from the environment. #3493

Velfi · 2024-03-15T19:27:38Z

Motivation and Context

Description

This PR adds a new feature: the ability to source service-specific config from the environment.
This is only supported when creating a service config from an SdkConfig. I've posted a guide to our discussions board.

This also adds support for setting an endpoint URL in environment config.

Testing

I have written several tests ensuring config is extracted with the correct precedence.

Checklist

I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…config

github-actions · 2024-03-15T19:45:42Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-19T15:57:23Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-19T16:36:10Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-19T17:20:11Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-19T21:30:53Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-20T16:54:09Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

jdisanti

Nice work on this!

aws/rust-runtime/aws-runtime/src/env_config.rs

jdisanti · 2024-03-20T22:46:54Z

aws/rust-runtime/aws-runtime/src/env_config.rs

+        let profile_value = match (profiles, self.profile_key.as_ref()) {
+            (Some(profiles), Some(profile_key)) => {
+                // Check for a service-specific profile key first
+                let service_config = get_service_config_from_profile(


I think for the S3 Express properties, the service config lookup needs to be explicitly turned off. Otherwise, this would end up working:

[default] services = foo [services foo] s3 = s3_disable_express_session_auth = true

Which is unspecified behavior.

I'll add a test ensuring that this isn't the case.

aws/rust-runtime/aws-runtime/src/profile.rs

aws/rust-runtime/aws-runtime/src/profile/parse.rs

jdisanti · 2024-03-20T22:59:45Z

aws/rust-runtime/aws-runtime/src/profile/profile_file.rs

+
+/// A single profile file within a [`ProfileFiles`] file set.
+#[derive(Clone)]
+pub(crate) enum ProfileFile {


Should really consider renaming these things since they're not profile files, but rather, shared config files.

I did a big rename of these and related types. Let me know what you think.

For ease of review, I kept using the same type names as the old ones in aws-config by using the deprecated type aliases. I can switch them to use the new names, I just thought this might make it easier to understand what's actually changed and what's basically the same after the move+rename

aws/rust-runtime/aws-runtime/src/profile/profile_set.rs

aws/rust-runtime/aws-runtime/src/profile/section.rs

aws/rust-runtime/aws-types/src/sdk_config.rs

Co-authored-by: John DiSanti <[email protected]>

…config

Velfi · 2024-03-25T19:56:21Z

I still have at least one test to fix. Otherwise, this is nearly complete.

github-actions · 2024-03-25T20:17:19Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-26T16:25:21Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

…config

github-actions · 2024-03-28T14:31:06Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-28T14:51:33Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-28T17:57:16Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-03-28T18:19:53Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

…config

github-actions · 2024-03-28T20:55:06Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

ysaito1001

Looks great, and consider this approval from me. Just want to check the status of this comment?

aws/rust-runtime/aws-runtime/src/env_config/file.rs

CHANGELOG.next.toml

Co-authored-by: ysaito1001 <[email protected]>

github-actions · 2024-03-29T14:57:04Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

…3873) ## Motivation and Context   awslabs/aws-sdk-rust#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](#3568), [2](#3493), [3](#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 awslabs/aws-sdk-rust#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._

…overrides (#3873) ## Motivation and Context   #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](smithy-lang/smithy-rs#3568), [2](smithy-lang/smithy-rs#3493), [3](smithy-lang/smithy-rs#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 #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._

Velfi added 2 commits March 12, 2024 11:31

add support for service env config

2339db5

Merge remote-tracking branch 'origin/main' into zhessler-service-env-…

02485ac

…config

Velfi added 5 commits March 19, 2024 10:16

update tests

1c39d00

Merge branch 'main' into zhessler-service-env-config

2d574ae

move test JSONs

6aee5d9

fix clippy lints

d847bc8

fix codegen attribute

4504106

fix integration test import

5746e52

fix usage of fs_util

f1ba0c0

Velfi added 2 commits March 19, 2024 16:11

add support for s3 express env config

d98aa86

Merge branch 'main' into zhessler-service-env-config

699e44a

remove comment

0e4e3be

jdisanti reviewed Mar 20, 2024

View reviewed changes

Velfi and others added 7 commits March 25, 2024 07:36

Update aws/rust-runtime/aws-types/src/sdk_config.rs

8fdcb11

Co-authored-by: John DiSanti <[email protected]>

Update aws/rust-runtime/aws-runtime/src/profile/section.rs

5bc1fa1

Co-authored-by: John DiSanti <[email protected]>

Merge remote-tracking branch 'origin/main' into zhessler-service-env-…

28c2453

…config

big rename of env config types

28102c3

add missing doc comment

fca10de

remove unnecessary async

25fea3c

Merge branch 'main' into zhessler-service-env-config

d29330b

appease the versioner

fc1c32c

Merge remote-tracking branch 'origin/main' into zhessler-service-env-…

e98f476

…config

PR comments update

02d7ae9

Velfi marked this pull request as ready for review March 28, 2024 14:38

Velfi requested review from a team as code owners March 28, 2024 14:38

Velfi added 2 commits March 28, 2024 12:37

undo change to parse_property_line (old one was better)

d7744fa

Merge branch 'main' into zhessler-service-env-config

480ab45

remove unnecessary s3 env config test

ebfab85

Velfi added 2 commits March 28, 2024 15:31

add changelog entry

590c000

Merge remote-tracking branch 'origin/main' into zhessler-service-env-…

62f21dc

…config

ysaito1001 reviewed Mar 28, 2024

View reviewed changes

aws/rust-runtime/aws-runtime/src/env_config/file.rs Outdated Show resolved Hide resolved

ysaito1001 reviewed Mar 28, 2024

View reviewed changes

CHANGELOG.next.toml Outdated Show resolved Hide resolved

Velfi and others added 3 commits March 29, 2024 09:37

remove duplicated changelog entry

631a3e8

Update aws/rust-runtime/aws-runtime/src/env_config/file.rs

a8e3ac3

Co-authored-by: ysaito1001 <[email protected]>

Merge branch 'main' into zhessler-service-env-config

455dc51

ysaito1001 approved these changes Mar 29, 2024

View reviewed changes

Velfi added this pull request to the merge queue Apr 1, 2024

Merged via the queue into main with commit 5f7113f Apr 1, 2024
41 of 42 checks passed

Velfi deleted the zhessler-service-env-config branch April 1, 2024 15:16

Velfi mentioned this pull request Apr 10, 2024

re-add support for sourcing endpoint URLs from service-specific env config #3568

Merged

aajtodd mentioned this pull request Oct 10, 2024

fix default credential chain not respecting endpoint URL overrides #3873

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sourcing service config from the environment. #3493

Sourcing service config from the environment. #3493

Velfi commented Mar 15, 2024 •

edited

Loading

github-actions bot commented Mar 15, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 20, 2024

jdisanti left a comment

jdisanti Mar 20, 2024

Velfi Mar 25, 2024

jdisanti Mar 20, 2024

Velfi Mar 25, 2024

Velfi commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024

github-actions bot commented Mar 26, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

ysaito1001 left a comment

github-actions bot commented Mar 29, 2024

Sourcing service config from the environment. #3493

Sourcing service config from the environment. #3493

Conversation

Velfi commented Mar 15, 2024 • edited Loading

Motivation and Context

Description

Testing

Checklist

github-actions bot commented Mar 15, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

github-actions bot commented Mar 20, 2024

jdisanti left a comment

Choose a reason for hiding this comment

jdisanti Mar 20, 2024

Choose a reason for hiding this comment

Velfi Mar 25, 2024

Choose a reason for hiding this comment

jdisanti Mar 20, 2024

Choose a reason for hiding this comment

Velfi Mar 25, 2024

Choose a reason for hiding this comment

Velfi commented Mar 25, 2024 • edited Loading

github-actions bot commented Mar 25, 2024

github-actions bot commented Mar 26, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

ysaito1001 left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 29, 2024

Velfi commented Mar 15, 2024 •

edited

Loading

Velfi commented Mar 25, 2024 •

edited

Loading