Skip to content

Commit

Permalink
Ensure identity resolver exists when a credentials provider is given …
Browse files Browse the repository at this point in the history
…only at operation level (#3021)

## Motivation and Context
Fixes awslabs/aws-sdk-rust#901

## Description
When a credentials provider is specified _only_ at the operation level
(but not at the service config level), the code in the above PR fails on
request dispatch, saying `NoMatchingAuthScheme`. This occurs today
because if we [do not set a credentials provider at the service config
level](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/kms/src/config.rs#L757-L769),
we will [not set the identity resolver for
sigv4](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/kms/src/config.rs#L811-L818).
The same goes for configuring a `SigningRegion` when it is only supplied
at the operation level.

This PR fixes the said issue so that `config_override` sets
- the identity resolver for sigv4 when a credentials provider is
supplied only at the operation config level
- a `SigningRegion` when a `Region` is given only at the operation level

## Testing
Added a Kotlin integ test
`test_specifying_credentials_provider_only_at_operation_level_should_work`
based on the customer reported PR.

## Checklist
- [x] 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._

---------

Co-authored-by: John DiSanti <[email protected]>
  • Loading branch information
ysaito1001 and jdisanti authored Oct 4, 2023
1 parent cd09fd2 commit 40aaa1e
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 57 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ message = "Fix code generation for union members with the `@httpPayload` trait."
references = ["smithy-rs#2969", "smithy-rs#1896"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all" }
author = "jdisanti"

[[aws-sdk-rust]]
message = "Fix exclusively setting the credentials provider and/or region at operation config-override time. It's now possible to set the region or credentials when an operation is sent (via `.config_override()`), rather than at client-creation time."
references = ["smithy-rs#3021", "aws-sdk-rust#901"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,17 @@ class CredentialCacheConfig(codegenContext: ClientCodegenContext) : ConfigCustom
private val codegenScope = arrayOf(
*preludeScope,
"CredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig).resolve("cache::CredentialsCache"),
"CredentialsIdentityResolver" to AwsRuntimeType.awsRuntime(runtimeConfig)
.resolve("identity::credentials::CredentialsIdentityResolver"),
"DefaultProvider" to defaultProvider(),
"SIGV4_SCHEME_ID" to AwsRuntimeType.awsRuntime(runtimeConfig).resolve("auth::sigv4::SCHEME_ID"),
"SharedAsyncSleep" to RuntimeType.smithyAsync(runtimeConfig).resolve("rt::sleep::SharedAsyncSleep"),
"SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig).resolve("cache::SharedCredentialsCache"),
"SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig).resolve("provider::SharedCredentialsProvider"),
"SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig)
.resolve("cache::SharedCredentialsCache"),
"SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig)
.resolve("provider::SharedCredentialsProvider"),
"SharedIdentityResolver" to RuntimeType.smithyRuntimeApi(runtimeConfig)
.resolve("client::identity::SharedIdentityResolver"),
)

override fun section(section: ServiceConfig) = writable {
Expand Down Expand Up @@ -209,7 +216,13 @@ class CredentialCacheConfig(codegenContext: ClientCodegenContext) : ConfigCustom
#{Some}(credentials_cache),
#{Some}(credentials_provider),
) => {
resolver.config_mut().store_put(credentials_cache.create_cache(credentials_provider));
let credentials_cache = credentials_cache.create_cache(credentials_provider);
resolver.runtime_components_mut().push_identity_resolver(
#{SIGV4_SCHEME_ID},
#{SharedIdentityResolver}::new(
#{CredentialsIdentityResolver}::new(credentials_cache),
),
);
}
}
""",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,19 @@ class SigV4SigningConfig(
)
}
}
is ServiceConfig.OperationConfigOverride -> {
if (runtimeMode.generateOrchestrator) {
rustTemplate(
"""
resolver.config_mut()
.load::<#{Region}>()
.cloned()
.map(|r| resolver.config_mut().store_put(#{SigningRegion}::from(r)));
""",
*codegenScope,
)
}
}

else -> emptySection
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ internal class CredentialCacheConfigTest {
namespace com.example
use aws.protocols#awsJson1_0
use aws.api#service
use aws.auth#sigv4
use smithy.rules#endpointRuleSet
@service(sdkId: "Some Value")
@awsJson1_0
@sigv4(name: "dontcare")
@auth([sigv4])
@endpointRuleSet({
"version": "1.0",
"rules": [{
Expand All @@ -39,7 +42,6 @@ internal class CredentialCacheConfigTest {
version: "1"
}
@optionalAuth
operation SayHello { input: TestInput }
structure TestInput {
foo: String,
Expand All @@ -56,14 +58,11 @@ internal class CredentialCacheConfigTest {
.resolve("Credentials"),
"CredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig)
.resolve("cache::CredentialsCache"),
"ProvideCachedCredentials" to AwsRuntimeType.awsCredentialTypes(runtimeConfig)
.resolve("cache::ProvideCachedCredentials"),
"Region" to AwsRuntimeType.awsTypes(runtimeConfig).resolve("region::Region"),
"RuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig)
.resolve("client::runtime_plugin::RuntimePlugin"),
"SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig)
.resolve("cache::SharedCredentialsCache"),
"SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig)
.resolve("provider::SharedCredentialsProvider"),
)
rustCrate.testModule {
unitTest(
Expand Down Expand Up @@ -114,74 +113,55 @@ internal class CredentialCacheConfigTest {
)
}

tokioTest("test_overriding_cache_and_provider_leads_to_shared_credentials_cache_in_layer") {
unitTest("test_not_overriding_cache_and_provider_leads_to_no_shared_credentials_cache_in_layer") {
rustTemplate(
"""
use #{ProvideCachedCredentials};
use #{RuntimePlugin};
let client_config = crate::config::Config::builder()
.credentials_provider(#{Credentials}::for_tests())
.build();
let client_config_layer = client_config.config;
// make sure test credentials are set in the client config level
assert_eq!(#{Credentials}::for_tests(),
client_config_layer
.load::<#{SharedCredentialsCache}>()
.unwrap()
.provide_cached_credentials()
.await
.unwrap()
);
let credentials = #{Credentials}::new(
"test",
"test",
#{None},
#{None},
"test",
);
let config_override = crate::config::Config::builder()
.credentials_cache(#{CredentialsCache}::lazy())
.credentials_provider(credentials.clone());
let client_config = crate::config::Config::builder().build();
let config_override = crate::config::Config::builder();
let sut = crate::config::ConfigOverrideRuntimePlugin::new(
config_override,
client_config_layer,
client_config.config,
&client_config.runtime_components,
);
let sut_layer = sut.config().unwrap();
// make sure `.provide_cached_credentials` returns credentials set through `config_override`
assert_eq!(credentials,
sut_layer
assert!(sut_layer
.load::<#{SharedCredentialsCache}>()
.unwrap()
.provide_cached_credentials()
.await
.unwrap()
);
.is_none());
""",
*codegenScope,
)
}

unitTest("test_not_overriding_cache_and_provider_leads_to_no_shared_credentials_cache_in_layer") {
tokioTest("test_specifying_credentials_provider_only_at_operation_level_should_work") {
// per https://github.com/awslabs/aws-sdk-rust/issues/901
rustTemplate(
"""
use #{RuntimePlugin};
let client_config = crate::config::Config::builder().build();
let config_override = crate::config::Config::builder();
let sut = crate::config::ConfigOverrideRuntimePlugin::new(
config_override,
client_config.config,
&client_config.runtime_components,
let client = crate::client::Client::from_conf(client_config);
let credentials = #{Credentials}::new(
"test",
"test",
#{None},
#{None},
"test",
);
let sut_layer = sut.config().unwrap();
assert!(sut_layer
.load::<#{SharedCredentialsCache}>()
.is_none());
let operation_config_override = crate::config::Config::builder()
.credentials_cache(#{CredentialsCache}::no_caching())
.credentials_provider(credentials.clone())
.region(#{Region}::new("us-west-2"));
let _ = client
.say_hello()
.customize()
.await
.unwrap()
.config_override(operation_config_override)
.send()
.await
.expect("success");
""",
*codegenScope,
)
Expand Down

0 comments on commit 40aaa1e

Please sign in to comment.