Skip to content

Commit

Permalink
Render smoke tests for services without support for account ID routing (
Browse files Browse the repository at this point in the history
#3808)

## Motivation and Context
Follow-up on #3799

## Description
This PR updates the smoke test rendering process for services whose
models specify the `smokeTests` trait
([example](https://github.com/awslabs/aws-sdk-rust/blob/c349073b345a576d18ab7729afc7b75ae273d2ac/aws-models/sns.json#L3392-L3404)).
Previously, `SmokeTestsDecorator` skipped rendering when
`useAccountIdRouting` was set to true in the vendor parameters, which is
the default for `AwsVendorParams`. Even though the Rust SDK does not
currently support the account ID routing, it is safe to render these
tests. This is because existing smoke tests do not rely on this feature,
and when account ID routing is used, the Rust SDK will fall back to
other means to sourcing the identity.

This PR includes a minor fix: it uses `aws_config` to load default
configurations. `config::Builder::new()` would have no credentials
provider chain available, which would then result in test failures at
runtime.
```
---- test_list_certificates_success stdout ----
thread 'test_list_certificates_success' panicked at sdk/acm/tests/smoketests.rs:22:9:
request should succeed: DispatchFailure(DispatchFailure { source: ConnectorError { kind: Other(None), source: NoMatchingAuthSchemeError(ExploredList { items: [ExploredAuthOption { scheme_id: AuthSchemeId { scheme_id: "sigv4" }, result: NoIdentityResolver }], truncated: false }), connection: Unknown } })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

By bringing `aws-config`, the `SmokeTestsDecoratorTest` no longer works,
since the AWS runtime crates required by `aws-config` conflict with
those generated by `awsSdkIntegrationTest` (a known limitation). For
example:
```
error: package collision in the lockfile: packages aws-credential-types v1.2.0 (smithy-rs/aws/rust-runtime/aws-credential-types) and aws-credential-types v1.2.0 (smithy-rs/aws/sdk/build/aws-sdk/sdk/aws-credential-types) are different, but only one can be written to lockfile unambiguously
```
Therefore, we removed `SmokeTestsDecoratorTest.kt` _only_ in this PR to
ship the feature, but are planning to restore it in the next PR by
directly testing `SmokeTestsInstantiator` without using
`awsSdkIntegrationTest`.

## Testing
- [x] Existing tests in CI
- [x] Executed and passed smoke tests for services that support them in
our release pipeline

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
ysaito1001 authored Aug 28, 2024
1 parent 740edcc commit 069ec70
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.docs
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.PublicImportSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
import software.amazon.smithy.rust.codegen.core.smithy.generators.Instantiator
Expand Down Expand Up @@ -50,12 +51,6 @@ class SmokeTestsDecorator : ClientCodegenDecorator {
logger.warning("skipping smoketest `${smokeTestCase.id}` with unsupported vendorParam `sigv4aRegionSet`")
return false
}
// TODO(https://github.com/smithy-lang/smithy-rs/issues/3776) Once Account ID routing is supported,
// update the vendorParams setter and remove this check.
if (vendorParams.useAccountIdRouting()) {
logger.warning("skipping smoketest `${smokeTestCase.id}` with unsupported vendorParam `useAccountIdRouting`")
return false
}
}
AwsSmokeTestModel.getS3VendorParams(smokeTestCase)?.orNull()?.let { s3VendorParams ->
if (s3VendorParams.useGlobalEndpoint()) {
Expand Down Expand Up @@ -138,7 +133,7 @@ class SmokeTestsBuilderKindBehavior(val codegenContext: CodegenContext) : Instan
}

class SmokeTestsInstantiator(private val codegenContext: ClientCodegenContext) : Instantiator(
codegenContext.symbolProvider,
PublicImportSymbolProvider(codegenContext.symbolProvider, codegenContext.moduleUseName()),
codegenContext.model,
codegenContext.runtimeConfig,
SmokeTestsBuilderKindBehavior(codegenContext),
Expand All @@ -147,9 +142,16 @@ class SmokeTestsInstantiator(private val codegenContext: ClientCodegenContext) :
writer: RustWriter,
testCase: SmokeTestCase,
) {
writer.rust("let conf = config::Builder::new()")
writer.rust(
"let config = #{T}::load_defaults(config::BehaviorVersion::latest()).await;",
AwsCargoDependency.awsConfig(codegenContext.runtimeConfig).toType(),
)
writer.rust("let conf = config::Config::from(&config).to_builder()")
writer.indent()
writer.rust(".behavior_version(config::BehaviorVersion::latest())")

// TODO(https://github.com/smithy-lang/smithy-rs/issues/3776) Once Account ID routing is supported,
// reflect the config setting here, especially to disable it if needed, as it is enabled by default in
// `AwsVendorParams`.

val vendorParams = AwsSmokeTestModel.getAwsVendorParams(testCase)
vendorParams.orNull()?.let { params ->
Expand Down

This file was deleted.

0 comments on commit 069ec70

Please sign in to comment.