From edab8cfb4913979374e461c96d31a3c939f66ebc Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 29 Nov 2023 10:01:51 -0500 Subject: [PATCH 1/5] Fix runtime crate versions (#3260) ## Motivation and Context When the stable/unstable crate versions were split, this caused a regression in determining the crate version during codegen. This enhances our crate-version forwarding code to publish all the crate versions. For impact, see codegen-client-test. ## Description - Change the build artifact to be a JSON blob will all required versions. ## Testing - [ ] Audit diff ## Checklist - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [ ] 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._ --- .../smithy/rustsdk/AwsCargoDependency.kt | 2 +- buildSrc/src/main/kotlin/CrateSet.kt | 2 + .../config/ServiceConfigGenerator.kt | 1 + codegen-core/build.gradle.kts | 37 +++++- .../smithy/rust/codegen/core/Version.kt | 38 ++++-- .../rust/codegen/core/smithy/RuntimeType.kt | 20 +-- .../smithy/rust/codegen/core/VersionTest.kt | 25 +--- .../codegen/core/smithy/RuntimeTypeTest.kt | 118 ++---------------- .../generators/PythonServerModuleGenerator.kt | 2 +- 9 files changed, 94 insertions(+), 151 deletions(-) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCargoDependency.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCargoDependency.kt index 4141c7e0b1..c803040f7d 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCargoDependency.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCargoDependency.kt @@ -10,7 +10,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.crateLocation fun RuntimeConfig.awsRuntimeCrate(name: String, features: Set = setOf()): CargoDependency = - CargoDependency(name, awsRoot().crateLocation(null), features = features) + CargoDependency(name, awsRoot().crateLocation(name), features = features) object AwsCargoDependency { fun awsConfig(runtimeConfig: RuntimeConfig) = runtimeConfig.awsRuntimeCrate("aws-config") diff --git a/buildSrc/src/main/kotlin/CrateSet.kt b/buildSrc/src/main/kotlin/CrateSet.kt index 72eb37d3fb..765503e5a1 100644 --- a/buildSrc/src/main/kotlin/CrateSet.kt +++ b/buildSrc/src/main/kotlin/CrateSet.kt @@ -79,4 +79,6 @@ object CrateSet { ) val ENTIRE_SMITHY_RUNTIME = (AWS_SDK_SMITHY_RUNTIME + SERVER_SMITHY_RUNTIME).toSortedSet(compareBy { it.name }) + + val ALL_CRATES = AWS_SDK_RUNTIME + ENTIRE_SMITHY_RUNTIME } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt index e94480e05a..26f4cd7a63 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt @@ -476,6 +476,7 @@ class ServiceConfigGenerator( docs("Apply test defaults to the builder") rustBlock("pub fn apply_test_defaults(&mut self) -> &mut Self") { customizations.forEach { it.section(ServiceConfig.DefaultForTests("self"))(this) } + rustTemplate("self.behavior_version = #{Some}(crate::config::BehaviorVersion::latest());", *preludeScope) rust("self") } diff --git a/codegen-core/build.gradle.kts b/codegen-core/build.gradle.kts index 556088f8a0..0213e423a5 100644 --- a/codegen-core/build.gradle.kts +++ b/codegen-core/build.gradle.kts @@ -53,18 +53,47 @@ fun gitCommitHash(): String { } val generateSmithyRuntimeCrateVersion by tasks.registering { + fun kv(key: String, value: String) = "\"$key\": \"$value\"" // generate the version of the runtime to use as a resource. // this keeps us from having to manually change version numbers in multiple places val resourcesDir = "$buildDir/resources/main/software/amazon/smithy/rust/codegen/core" val versionFile = file("$resourcesDir/runtime-crate-version.txt") outputs.file(versionFile) - val crateVersion = project.properties["smithy.rs.runtime.crate.version"].toString() - inputs.property("crateVersion", crateVersion) + val stableCrateVersion = project.properties["smithy.rs.runtime.crate.stable.version"].toString() + val unstableCrateVersion = project.properties["smithy.rs.runtime.crate.unstable.version"].toString() + inputs.property("crateVersion", stableCrateVersion) // version format must be in sync with `software.amazon.smithy.rust.codegen.core.Version` - val version = "$crateVersion\n${gitCommitHash()}" + val version = StringBuilder().append("{") + version.append(kv("githash", gitCommitHash())).append(",") + version.append(kv("stableVersion", stableCrateVersion)).append(",") + version.append(kv("unstableVersion", unstableCrateVersion)).append(",") + // hack for internal build + val smithyStableCrates = listOf( + // AWS crates + "aws-config", + "aws-credential-types", + "aws-runtime", + "aws-runtime-api", + "aws-sigv4", + "aws-types", + + // smithy crates + "aws-smithy-async", + "aws-smithy-runtime-api", + "aws-smithy-runtime", + "aws-smithy-types", + ) + + val runtimeCrates = + smithyStableCrates.joinToString(separator = ",", prefix = "{", postfix = "}") { crate -> + kv(crate, stableCrateVersion) + } + + version.append(""""runtimeCrates": $runtimeCrates""").append("}") + sourceSets.main.get().output.dir(resourcesDir) doLast { - versionFile.writeText(version) + versionFile.writeText(version.toString()) } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/Version.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/Version.kt index 633d92c227..071f0a2089 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/Version.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/Version.kt @@ -6,29 +6,49 @@ package software.amazon.smithy.rust.codegen.core import software.amazon.smithy.codegen.core.CodegenException +import software.amazon.smithy.model.node.Node // generated as part of the build, see codegen-core/build.gradle.kts private const val VERSION_FILENAME = "runtime-crate-version.txt" -data class Version(val fullVersion: String, val crateVersion: String) { +data class Version( + val fullVersion: String, + val stableCrateVersion: String, + val unstableCrateVersion: String, + val crates: Map, +) { companion object { // Version must be in the "{smithy_rs_version}\n{git_commit_hash}" format fun parse(content: String): Version { - val lines = content.lines() - if (lines.size != 2) { - throw IllegalArgumentException("Invalid version format, it should contain `2` lines but contains `${lines.size}` line(s)") - } - return Version(lines.joinToString("-"), lines.first()) + val node = Node.parse(content).expectObjectNode() + val githash = node.expectStringMember("githash").value + val stableVersion = node.expectStringMember("stableVersion").value + val unstableVersion = node.expectStringMember("unstableVersion").value + return Version( + "$stableVersion-$githash", + stableCrateVersion = stableVersion, + unstableCrateVersion = unstableVersion, + node.expectObjectMember("runtimeCrates").members.map { + it.key.value to it.value.expectStringNode().value + }.toMap(), + ) } // Returns full version in the "{smithy_rs_version}-{git_commit_hash}" format fun fullVersion(): String = fromDefaultResource().fullVersion - fun crateVersion(): String = - fromDefaultResource().crateVersion + fun stableCrateVersion(): String = + fromDefaultResource().stableCrateVersion - private fun fromDefaultResource(): Version = parse( + fun unstableCrateVersion(): String = + fromDefaultResource().unstableCrateVersion + + fun crateVersion(crate: String): String { + val version = fromDefaultResource() + return version.crates[crate] ?: version.unstableCrateVersion + } + fun fromDefaultResource(): Version = parse( Version::class.java.getResource(VERSION_FILENAME)?.readText() ?: throw CodegenException("$VERSION_FILENAME does not exist"), ) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt index 3a12fbec76..ea8cbc9db1 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt @@ -40,19 +40,21 @@ data class RuntimeCrateLocation(val path: String?, val versions: CrateVersionMap } } -fun RuntimeCrateLocation.crateLocation(crateName: String?): DependencyLocation { - val version = crateName.let { versions.map[crateName] } ?: versions.map[DEFAULT_KEY] +fun RuntimeCrateLocation.crateLocation(crateName: String): DependencyLocation { + val version = crateName.let { + versions.map[crateName] + } ?: Version.crateVersion(crateName) return when (this.path) { // CratesIo needs an exact version. However, for local runtime crates we do not // provide a detected version unless the user explicitly sets one via the `versions` map. - null -> CratesIo(version ?: defaultRuntimeCrateVersion()) - else -> Local(this.path, version) + null -> CratesIo(version) + else -> Local(this.path) } } fun defaultRuntimeCrateVersion(): String { try { - return Version.crateVersion() + return Version.stableCrateVersion() } catch (ex: Exception) { throw CodegenException("failed to get crate version which sets the default client-runtime version", ex) } @@ -94,10 +96,6 @@ data class RuntimeConfig( } } - val crateSrcPrefix: String = cratePrefix.replace("-", "_") - - fun runtimeCratesPath(): String? = runtimeCrateLocation.path - fun smithyRuntimeCrate( runtimeCrateName: String, optional: Boolean = false, @@ -324,7 +322,9 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null) fun smithyQuery(runtimeConfig: RuntimeConfig) = CargoDependency.smithyQuery(runtimeConfig).toType() fun smithyRuntime(runtimeConfig: RuntimeConfig) = CargoDependency.smithyRuntime(runtimeConfig).toType() fun smithyRuntimeApi(runtimeConfig: RuntimeConfig) = CargoDependency.smithyRuntimeApi(runtimeConfig).toType() - fun smithyRuntimeApiClient(runtimeConfig: RuntimeConfig) = CargoDependency.smithyRuntimeApiClient(runtimeConfig).toType() + fun smithyRuntimeApiClient(runtimeConfig: RuntimeConfig) = + CargoDependency.smithyRuntimeApiClient(runtimeConfig).toType() + fun smithyTypes(runtimeConfig: RuntimeConfig) = CargoDependency.smithyTypes(runtimeConfig).toType() fun smithyXml(runtimeConfig: RuntimeConfig) = CargoDependency.smithyXml(runtimeConfig).toType() private fun smithyProtocolTest(runtimeConfig: RuntimeConfig) = diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/VersionTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/VersionTest.kt index 9c4cde5f5c..2147dc857c 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/VersionTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/VersionTest.kt @@ -21,7 +21,7 @@ class VersionTest { ) { val version = Version.parse(content) version.fullVersion shouldBe fullVersion - version.crateVersion shouldBe crateVersion + version.stableCrateVersion shouldBe crateVersion } @ParameterizedTest() @@ -36,30 +36,15 @@ class VersionTest { @JvmStatic fun versionProvider() = listOf( Arguments.of( - "0.47.0\n0198d26096eb1af510ce24766c921ffc5e4c191e", - "0.47.0-0198d26096eb1af510ce24766c921ffc5e4c191e", - "0.47.0", + """{ "stableVersion": "1.0.1", "unstableVersion": "0.60.1","githash": "0198d26096eb1af510ce24766c921ffc5e4c191e", "runtimeCrates": {} }""", + "1.0.1-0198d26096eb1af510ce24766c921ffc5e4c191e", + "1.0.1", ), Arguments.of( - "release-2022-08-04\ndb48039065bec890ef387385773b37154b555b14", + """{ "unstableVersion": "0.60.1", "stableVersion": "release-2022-08-04", "githash": "db48039065bec890ef387385773b37154b555b14", "runtimeCrates": {} }""", "release-2022-08-04-db48039065bec890ef387385773b37154b555b14", "release-2022-08-04", ), - Arguments.of( - "0.30.0-alpha\na1dbbe2947de3c8bbbef9446eb442e298f83f200", - "0.30.0-alpha-a1dbbe2947de3c8bbbef9446eb442e298f83f200", - "0.30.0-alpha", - ), - Arguments.of( - "0.6-rc1.cargo\nc281800a185b34600b05f8b501a0322074184123", - "0.6-rc1.cargo-c281800a185b34600b05f8b501a0322074184123", - "0.6-rc1.cargo", - ), - Arguments.of( - "0.27.0-alpha.1\n643f2ee", - "0.27.0-alpha.1-643f2ee", - "0.27.0-alpha.1", - ), ) @JvmStatic diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeTypeTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeTypeTest.kt index 1ae310088d..b3dec08b7c 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeTypeTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeTypeTest.kt @@ -11,8 +11,8 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource import software.amazon.smithy.model.node.Node +import software.amazon.smithy.rust.codegen.core.Version import software.amazon.smithy.rust.codegen.core.rustlang.CratesIo -import software.amazon.smithy.rust.codegen.core.rustlang.DependencyLocation import software.amazon.smithy.rust.codegen.core.rustlang.Local import java.util.Optional @@ -35,22 +35,20 @@ class RuntimeTypesTest { cfg.runtimeCrateLocation shouldBe RuntimeCrateLocation(null, CrateVersionMap(mapOf())) } - @ParameterizedTest - @MethodSource("runtimeCrateLocationProvider") - fun `runtimeCrateLocation provides dependency location`( - path: String?, - versions: CrateVersionMap, - crateName: String?, - expectedDependencyLocation: DependencyLocation, - ) { - val crateLoc = RuntimeCrateLocation(path, versions) - val depLoc = crateLoc.crateLocation(crateName) - depLoc shouldBe expectedDependencyLocation + @Test + fun `runtimeCrateLocation provides dependency location`() { + val crateLoc = RuntimeCrateLocation("/foo", CrateVersionMap(mapOf("aws-smithy-runtime-api" to "999.999"))) + crateLoc.crateLocation("aws-smithy-runtime") shouldBe Local("/foo", null) + crateLoc.crateLocation("aws-smithy-runtime-api") shouldBe Local("/foo", null) + crateLoc.crateLocation("aws-smithy-http") shouldBe Local("/foo", null) + + val crateLocVersioned = RuntimeCrateLocation(null, CrateVersionMap(mapOf("aws-smithy-runtime-api" to "999.999"))) + crateLocVersioned.crateLocation("aws-smithy-runtime") shouldBe CratesIo(Version.stableCrateVersion()) + crateLocVersioned.crateLocation("aws-smithy-runtime-api") shouldBe CratesIo("999.999") + crateLocVersioned.crateLocation("aws-smithy-http") shouldBe CratesIo(Version.unstableCrateVersion()) } companion object { - @JvmStatic - private val defaultVersion = defaultRuntimeCrateVersion() @JvmStatic fun runtimeConfigProvider() = listOf( @@ -90,97 +88,5 @@ class RuntimeTypesTest { RuntimeCrateLocation("/path", CrateVersionMap(mapOf("a" to "1.0", "b" to "2.0"))), ), ) - - @JvmStatic - fun runtimeCrateLocationProvider() = listOf( - // If user specifies `relativePath` in `runtimeConfig`, then that always takes precedence over versions. - Arguments.of( - "/path", - mapOf(), - null, - Local("/path"), - ), - Arguments.of( - "/path", - mapOf("a" to "1.0", "b" to "2.0"), - null, - Local("/path"), - ), - Arguments.of( - "/path", - mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"), - null, - Local("/path", "0.1"), - ), - - // User does not specify the versions object. - // The version number of the code-generator should be used as the version for all runtime crates. - Arguments.of( - null, - mapOf(), - null, - CratesIo(defaultVersion), - ), - Arguments.of( - null, - mapOf(), - "a", - CratesIo(defaultVersion), - ), - - // User specifies versions object, setting explicit version numbers for some runtime crates. - // Then the rest of the runtime crates use the code-generator's version as their version. - Arguments.of( - null, - mapOf("a" to "1.0", "b" to "2.0"), - null, - CratesIo(defaultVersion), - ), - Arguments.of( - null, - mapOf("a" to "1.0", "b" to "2.0"), - "a", - CratesIo("1.0"), - ), - Arguments.of( - null, - mapOf("a" to "1.0", "b" to "2.0"), - "b", - CratesIo("2.0"), - ), - Arguments.of( - null, - mapOf("a" to "1.0", "b" to "2.0"), - "c", - CratesIo(defaultVersion), - ), - - // User specifies versions object, setting DEFAULT and setting version numbers for some runtime crates. - // Then the specified version in DEFAULT is used for all runtime crates, except for those where the user specified a value for in the map. - Arguments.of( - null, - mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"), - null, - CratesIo("0.1"), - ), - Arguments.of( - null, - mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"), - "a", - CratesIo("1.0"), - ), - Arguments.of( - null, - mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"), - "b", - CratesIo("2.0"), - ), - Arguments.of( - null, - mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"), - "c", - CratesIo("0.1"), - ), - ) } } diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt index 577999a724..cbec0b8551 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt @@ -237,7 +237,7 @@ class PythonServerModuleGenerator( // Render the codegeneration version as module attribute. private fun RustWriter.renderCodegenVersion() { - rust("""m.add("CODEGEN_VERSION", "${Version.crateVersion()}")?;""") + rust("""m.add("CODEGEN_VERSION", "${Version.stableCrateVersion()}")?;""") } // Convert to symbol and check the namespace to figure out where they should be imported from. From 76ae89ae0bf8fd54629e31b1fbd8bba2510bdead Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Wed, 29 Nov 2023 18:08:50 -0600 Subject: [PATCH 2/5] Fix running semver-checks on `aws-config` (#3272) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation and Context Fixes https://github.com/smithy-lang/smithy-rs/issues/3265 ## Description To check semver, `semver checks` needs to compile two versions of a crate, i.e. baseline and current. Our CI failed to compile `aws-config` for a baseline version. The following diagram shows what was missing to cause compilation failure: ``` ┌───────────┐ │ smithy-rs │ └─────┬─────┘ │ │ │ ┌────────┐ └─────┤ target │ └───┬────┘ │ ┌───────────────┐ └────────┤ semver_checks │ └───────┬───────┘ │ │ │ ┌──────────┐ └───────┤ git-base │ └────┬─────┘ │ │ ┌──────────────┐ ├────────┤ │ │ └──────┬───────┘ │ │ │ │ │ │ ┌───────┐ │ ├───────┤ aws │ │ │ └───┬───┘ │ │ │ │ │ │ ┌──────────────┐ │ │ ├───────┤ rust-runtime │ │ │ │ └──────┬───────┘ │ │ │ │ │ │ │ │ ┌────────────┐ │ │ │ └───────┤ aws-config │ ◄*************** │ │ │ └────────────┘ * │ │ │ * * │ │ │ * * │ │ │ *****depends*on**** * │ │ │ ▼ * │ │ │ * │ │ │ ┌─────┐ * │ │ └───────┤ sdk │ (with no "build" directory) * │ │ └─────┘ * │ │ * │ │ * │ │ ┌────────────────────┐ depends on │ └──────┤ tmp-codegen-diff │ * │ └─────────┬──────────┘ * │ │ * │ │ ┌─────────┐ * │ └───────┤ aws-sdk │ * │ └────┬────┘ * │ │ ┌─────┐ * │ └─────┤ sdk │ * │ └─────┘ * │ * │ * │ ┌───────────────────────────────────────┐ * └───────────┤ local-aws_config-0_0_0_smithy_rs_head │***************************** └───────────────────────────────────────┘ ``` `local-aws_config-0_0_0_smithy_rs_head` under the `git-base` directory is a special crate created by `semver-checks` for a baseline version of `aws-config` and its `Cargo.toml` depends on `target/semver_checks/git-base//aws/rust-runtime/aws-config`. However, that `aws-config` in turn depends upon crates in `target/semver_checks/git-base//aws/sdk/build/` (as shown [here](https://github.com/smithy-lang/smithy-rs/blob/main/aws/rust-runtime/aws-config/Cargo.toml#L23-L33)), which does not exist in a baseline branch. When `semver-checks.py` [creates a baseline branch](https://github.com/smithy-lang/smithy-rs/blob/3d0cb5c3b179d5ed1d14531882657b481c4469f0/tools/ci-scripts/codegen-diff/semver-checks.py#L31) to prepare for running `cargo semver-checks`, it [moves aws/sdk/build to tmp-codegen-diff](https://github.com/smithy-lang/smithy-rs/blob/3d0cb5c3b179d5ed1d14531882657b481c4469f0/tools/ci-scripts/codegen-diff/diff_lib.py#L59), leaving nothing behind in `aws/sdk/build/`. The fix, therefore, is to `cp -r aws/sdk/build/aws-sdk` instead of `mv aws/sdk/build/aws-sdk` when preparing a baseline branch. The issue is specific to `aws-config`, probably because that's the only runtime crate that depends on those in `aws/sdk/build`. ## Testing Verified a clean run for `cargo semver-checks` in [an investigation branch](https://github.com/smithy-lang/smithy-rs/actions/runs/7035082815/job/19144676499#step:4:1101). ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- tools/ci-scripts/codegen-diff/diff_lib.py | 15 +++++++++++---- tools/ci-scripts/codegen-diff/semver-checks.py | 4 +--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/tools/ci-scripts/codegen-diff/diff_lib.py b/tools/ci-scripts/codegen-diff/diff_lib.py index d39bbaba17..6d632c15a3 100644 --- a/tools/ci-scripts/codegen-diff/diff_lib.py +++ b/tools/ci-scripts/codegen-diff/diff_lib.py @@ -26,7 +26,7 @@ def running_in_docker_build(): return os.environ.get("SMITHY_RS_DOCKER_BUILD_IMAGE") == "1" -def checkout_commit_and_generate(revision_sha, branch_name, targets=None): +def checkout_commit_and_generate(revision_sha, branch_name, targets=None, preserve_aws_sdk_build=False): if running_in_docker_build(): eprint(f"Fetching base revision {revision_sha} from GitHub...") run(f"git fetch --no-tags --progress --no-recurse-submodules --depth=1 origin {revision_sha}") @@ -34,10 +34,10 @@ def checkout_commit_and_generate(revision_sha, branch_name, targets=None): # Generate code for HEAD eprint(f"Creating temporary branch {branch_name} with generated code for {revision_sha}") run(f"git checkout {revision_sha} -B {branch_name}") - generate_and_commit_generated_code(revision_sha, targets) + generate_and_commit_generated_code(revision_sha, targets, preserve_aws_sdk_build) -def generate_and_commit_generated_code(revision_sha, targets=None): +def generate_and_commit_generated_code(revision_sha, targets=None, preserve_aws_sdk_build=False): targets = targets or [ target_codegen_client, target_codegen_server, @@ -56,7 +56,11 @@ def generate_and_commit_generated_code(revision_sha, targets=None): get_cmd_output(f"rm -rf {OUTPUT_PATH}") get_cmd_output(f"mkdir {OUTPUT_PATH}") if target_aws_sdk in targets: - get_cmd_output(f"mv aws/sdk/build/aws-sdk {OUTPUT_PATH}/") + # Compiling aws-config for semver checks baseline requires build artifacts to exist under aws/sdk/build + if preserve_aws_sdk_build: + get_cmd_output(f"cp -r aws/sdk/build/aws-sdk {OUTPUT_PATH}/") + else: + get_cmd_output(f"mv aws/sdk/build/aws-sdk {OUTPUT_PATH}/") for target in [target_codegen_client, target_codegen_server]: if target in targets: get_cmd_output(f"mv {target}/build/smithyprojections/{target} {OUTPUT_PATH}/") @@ -89,6 +93,9 @@ def generate_and_commit_generated_code(revision_sha, targets=None): f"xargs rm -f", shell=True) get_cmd_output(f"git add -f {OUTPUT_PATH}") + if preserve_aws_sdk_build: + get_cmd_output(f"git add -f aws/sdk/build") + get_cmd_output(f"git -c 'user.name=GitHub Action (generated code preview)' " f"-c 'user.name={COMMIT_AUTHOR_NAME}' " f"-c 'user.email={COMMIT_AUTHOR_EMAIL}' " diff --git a/tools/ci-scripts/codegen-diff/semver-checks.py b/tools/ci-scripts/codegen-diff/semver-checks.py index 25d36ffb34..1632242fc0 100755 --- a/tools/ci-scripts/codegen-diff/semver-checks.py +++ b/tools/ci-scripts/codegen-diff/semver-checks.py @@ -28,7 +28,7 @@ def main(skip_generation=False): if not skip_generation: checkout_commit_and_generate(head_commit_sha, CURRENT_BRANCH, targets=['aws:sdk']) - checkout_commit_and_generate(base_commit_sha, BASE_BRANCH, targets=['aws:sdk']) + checkout_commit_and_generate(base_commit_sha, BASE_BRANCH, targets=['aws:sdk'], preserve_aws_sdk_build=True) get_cmd_output(f'git checkout {CURRENT_BRANCH}') sdk_directory = os.path.join(OUTPUT_PATH, 'aws-sdk', 'sdk') os.chdir(sdk_directory) @@ -36,8 +36,6 @@ def main(skip_generation=False): failures = [] deny_list = [ # add crate names here to exclude them from the semver checks - # TODO(https://github.com/smithy-lang/smithy-rs/issues/3265) - 'aws-config' ] for path in list(os.listdir())[:10]: eprint(f'checking {path}...', end='') From 81fc83ee6ea1b98fcfa6b1bccaf2dd8db0748adb Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 1 Dec 2023 11:20:45 -0600 Subject: [PATCH 3/5] Fix `config::Builder::set_credentials_provider` to override what's previsouly set (#3278) ## Motivation and Context Fixes https://github.com/awslabs/aws-sdk-rust/issues/973 ## Description Prior to the PR, if a customer explicitly passed a credentials provider to a client's config `Builder::set_credentials_provider`, what's passed did not override a credentials provider previously set ([actual use case](https://github.com/awslabs/aws-sdk-rust/issues/973#issuecomment-1827224049)). While in general, we recommend customers single-source a credentials provider through [aws_config::ConfigLoader::credentials_provider](https://docs.rs/aws-config/1.0.1/aws_config/struct.ConfigLoader.html#method.credentials_provider), we should eliminate the said footgun in case they directly pass a credentials provider to a client's config `Builder`. The PR reverts test signature updates in https://github.com/smithy-lang/smithy-rs/pull/3156 (in hindsight, having to update test signatures in that PR was a sign of regression). ## Testing Added a Kotlin test to `CredentialProviderConfigTest.kt` to verify the fix ## 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 --- CHANGELOG.next.toml | 20 ++++- .../smithy/rustsdk/CredentialProviders.kt | 9 ++- .../rustsdk/CredentialProviderConfigTest.kt | 69 ++++++++++++++++- .../kms/tests/integration.rs | 2 +- .../qldbsession/tests/integration.rs | 2 +- .../s3/tests/naughty-string-metadata.rs | 2 +- .../s3/tests/normalize-uri-path.rs | 2 +- .../query-strings-are-correctly-encoded.rs | 2 +- .../integration-tests/s3/tests/signing-it.rs | 2 +- .../s3control/tests/signing-it.rs | 6 +- .../customizations/HttpAuthDecorator.kt | 8 +- .../aws-smithy-runtime-api/Cargo.toml | 2 +- .../src/client/runtime_components.rs | 76 ++++++++++++++++++- .../src/client/orchestrator/operation.rs | 2 +- 14 files changed, 181 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fc4c4c2578..6f90ff174b 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -9,4 +9,22 @@ # message = "Fix typos in module documentation for generated crates" # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} -# author = "rcoh" \ No newline at end of file +# author = "rcoh" + +[[aws-sdk-rust]] +message = "Fix `config::Builder::set_credentials_provider` to override a credentials provider previously set." +references = ["aws-sdk-rust#973", "smithy-rs#3278"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "ysaito1001" + +[[aws-sdk-rust]] +message = "`config::Config::credentials_provider` has been broken since `release-2023-11-15` and is now marked as `deprecated` explicitly." +references = ["smithy-rs#3251", "smithy-rs#3278"] +meta = { "breaking" = false, "tada" = false, "bug" = false } +author = "ysaito1001" + +[[smithy-rs]] +message = "`RuntimeComponentsBuilder::push_identity_resolver` is now deprecated since it does not replace the existing identity resolver of a given auth scheme ID. Use `RuntimeComponentsBuilder::set_identity_resolver` instead." +references = ["smithy-rs#3278"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" } +author = "ysaito1001" diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt index 9405f8a2fe..331babe611 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt @@ -81,9 +81,10 @@ class CredentialProviderConfig(private val codegenContext: ClientCodegenContext) ServiceConfig.ConfigImpl -> { rustTemplate( """ - /// Returns the credentials provider for this service + /// This function was intended to be removed, and has been broken since release-2023-11-15 as it always returns a `None`. Do not use. + ##[deprecated(note = "This function was intended to be removed, and has been broken since release-2023-11-15 as it always returns a `None`. Do not use.")] pub fn credentials_provider(&self) -> Option<#{SharedCredentialsProvider}> { - self.config.load::<#{SharedCredentialsProvider}>().cloned() + #{None} } """, *codegenScope, @@ -118,13 +119,13 @@ class CredentialProviderConfig(private val codegenContext: ClientCodegenContext) if (codegenContext.serviceShape.supportedAuthSchemes().contains("sigv4a")) { featureGateBlock("sigv4a") { rustTemplate( - "self.runtime_components.push_identity_resolver(#{SIGV4A_SCHEME_ID}, credentials_provider.clone());", + "self.runtime_components.set_identity_resolver(#{SIGV4A_SCHEME_ID}, credentials_provider.clone());", *codegenScope, ) } } rustTemplate( - "self.runtime_components.push_identity_resolver(#{SIGV4_SCHEME_ID}, credentials_provider);", + "self.runtime_components.set_identity_resolver(#{SIGV4_SCHEME_ID}, credentials_provider);", *codegenScope, ) } diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialProviderConfigTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialProviderConfigTest.kt index 63a4f820ba..aec0806d06 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialProviderConfigTest.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialProviderConfigTest.kt @@ -30,7 +30,7 @@ internal class CredentialProviderConfigTest { val moduleName = ctx.moduleUseName() rustTemplate( """ - let (http_client, _rx) = #{capture_request}(None); + let (http_client, _rx) = #{capture_request}(#{None}); let client_config = $moduleName::Config::builder() .http_client(http_client) .build(); @@ -62,4 +62,71 @@ internal class CredentialProviderConfigTest { } } } + + @Test + fun `configuring credentials provider on builder should replace what was previously set`() { + awsSdkIntegrationTest(SdkCodegenIntegrationTest.model) { ctx, rustCrate -> + val rc = ctx.runtimeConfig + val codegenScope = arrayOf( + *RuntimeType.preludeScope, + "capture_request" to RuntimeType.captureRequest(rc), + "Credentials" to AwsRuntimeType.awsCredentialTypesTestUtil(rc) + .resolve("Credentials"), + "Region" to AwsRuntimeType.awsTypes(rc).resolve("region::Region"), + "SdkConfig" to AwsRuntimeType.awsTypes(rc).resolve("sdk_config::SdkConfig"), + "SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(rc) + .resolve("provider::SharedCredentialsProvider"), + ) + rustCrate.integrationTest("credentials_provider") { + // per https://github.com/awslabs/aws-sdk-rust/issues/973 + tokioTest("configuring_credentials_provider_on_builder_should_replace_what_was_previously_set") { + val moduleName = ctx.moduleUseName() + rustTemplate( + """ + let (http_client, rx) = #{capture_request}(#{None}); + + let replace_me = #{Credentials}::new( + "replace_me", + "replace_me", + #{None}, + #{None}, + "replace_me", + ); + let sdk_config = #{SdkConfig}::builder() + .credentials_provider( + #{SharedCredentialsProvider}::new(replace_me), + ) + .region(#{Region}::new("us-west-2")) + .build(); + + let expected = #{Credentials}::new( + "expected_credential", + "expected_credential", + #{None}, + #{None}, + "expected_credential", + ); + let conf = $moduleName::config::Builder::from(&sdk_config) + .http_client(http_client) + .credentials_provider(expected) + .build(); + + let client = $moduleName::Client::from_conf(conf); + + let _ = client + .some_operation() + .send() + .await + .expect("success"); + + let req = rx.expect_request(); + let auth_header = req.headers().get("AUTHORIZATION").unwrap(); + assert!(auth_header.contains("expected_credential"), "{auth_header}"); + """, + *codegenScope, + ) + } + } + } + } } diff --git a/aws/sdk/integration-tests/kms/tests/integration.rs b/aws/sdk/integration-tests/kms/tests/integration.rs index 6cba0de93d..ed534f61bd 100644 --- a/aws/sdk/integration-tests/kms/tests/integration.rs +++ b/aws/sdk/integration-tests/kms/tests/integration.rs @@ -52,7 +52,7 @@ async fn generate_random() { .header("content-type", "application/x-amz-json-1.1") .header("x-amz-target", "TrentService.GenerateRandom") .header("content-length", "20") - .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/kms/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-security-token;x-amz-target;x-amz-user-agent, Signature=703f72fe50c310e3ee1a7a106df947b980cb91bc8bad7a4a603b057096603aed") + .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/kms/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-target;x-amz-user-agent, Signature=53dcf70f6f852cb576185dcabef5aaa3d068704cf1b7ea7dc644efeaa46674d7") .header("x-amz-date", "20090213T233130Z") .header("user-agent", "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0") .header("x-amz-user-agent", "aws-sdk-rust/0.123.test api/test-service/0.123 os/windows/XPSP3 lang/rust/1.50.0") diff --git a/aws/sdk/integration-tests/qldbsession/tests/integration.rs b/aws/sdk/integration-tests/qldbsession/tests/integration.rs index fbc8b0a00f..6cdb32d1f1 100644 --- a/aws/sdk/integration-tests/qldbsession/tests/integration.rs +++ b/aws/sdk/integration-tests/qldbsession/tests/integration.rs @@ -20,7 +20,7 @@ async fn signv4_use_correct_service_name() { .header("content-type", "application/x-amz-json-1.0") .header("x-amz-target", "QLDBSession.SendCommand") .header("content-length", "49") - .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/qldb/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-security-token;x-amz-target;x-amz-user-agent, Signature=e8d50282fa369adf05f33a5b32e3ce2a7582edc902312c59de311001a97426d9") + .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/qldb/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-target;x-amz-user-agent, Signature=9a07c60550504d015fb9a2b0f1b175a4d906651f9dd4ee44bebb32a802d03815") // qldbsession uses the signing name 'qldb' in signature _________________________^^^^ .header("x-amz-date", "20090213T233130Z") .header("user-agent", "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0") diff --git a/aws/sdk/integration-tests/s3/tests/naughty-string-metadata.rs b/aws/sdk/integration-tests/s3/tests/naughty-string-metadata.rs index f139a28b0f..3b6bd97002 100644 --- a/aws/sdk/integration-tests/s3/tests/naughty-string-metadata.rs +++ b/aws/sdk/integration-tests/s3/tests/naughty-string-metadata.rs @@ -88,7 +88,7 @@ async fn test_s3_signer_with_naughty_string_metadata() { // This is a snapshot test taken from a known working test result let snapshot_signature = - "Signature=733dba2f1ca3c9a39f4eef3a6750a71eff00297cd765408ad3cef5dcdc44d642"; + "Signature=a5115604df66219874a9e5a8eab4c9f7a28c992ab2d918037a285756c019f3b2"; assert!( auth_header .contains(snapshot_signature), "authorization header signature did not match expected signature: got {}, expected it to contain {}", diff --git a/aws/sdk/integration-tests/s3/tests/normalize-uri-path.rs b/aws/sdk/integration-tests/s3/tests/normalize-uri-path.rs index 367aa19682..11ee0f7b09 100644 --- a/aws/sdk/integration-tests/s3/tests/normalize-uri-path.rs +++ b/aws/sdk/integration-tests/s3/tests/normalize-uri-path.rs @@ -41,7 +41,7 @@ async fn test_operation_should_not_normalize_uri_path() { let expected_uri = "https://test-bucket-ad7c9f01-7f7b-4669-b550-75cc6d4df0f1.s3.us-east-1.amazonaws.com/a/.././b.txt?x-id=PutObject"; assert_eq!(actual_uri, expected_uri); - let expected_sig = "Signature=404fb9502378c8f46fb83544848c42d29d55610a14b4bed9577542e49e549d08"; + let expected_sig = "Signature=2ac540538c84dc2616d92fb51d4fc6146ccd9ccc1ee85f518a1a686c5ef97b86"; assert!( actual_auth.contains(expected_sig), "authorization header signature did not match expected signature: expected {} but not found in {}", diff --git a/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs b/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs index 563ba7076d..53560e3beb 100644 --- a/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs +++ b/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs @@ -45,7 +45,7 @@ async fn test_s3_signer_query_string_with_all_valid_chars() { // This is a snapshot test taken from a known working test result let snapshot_signature = - "Signature=740feb1de3968a643e68fb1a17c415d98dd6a1cc28782fb1ef6157586548c747"; + "Signature=9a931d20606f93fa4e5553602866a9b5ccac2cd42b54ae5a4b17e4614fb443ce"; assert!( auth_header .contains(snapshot_signature), diff --git a/aws/sdk/integration-tests/s3/tests/signing-it.rs b/aws/sdk/integration-tests/s3/tests/signing-it.rs index d0106aaded..450f4ba43f 100644 --- a/aws/sdk/integration-tests/s3/tests/signing-it.rs +++ b/aws/sdk/integration-tests/s3/tests/signing-it.rs @@ -15,7 +15,7 @@ use aws_smithy_types::body::SdkBody; async fn test_signer() { let http_client = StaticReplayClient::new(vec![ReplayEvent::new( http::Request::builder() - .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-security-token;x-amz-user-agent, Signature=d8ea22461a59cc1cbeb01fa093423ffafcb7695197ba2409b477216a4be2c104") + .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-user-agent, Signature=27e3f59ec3cffaa10e4f1c92112e8fb62d468a04cd32be39e68215f830404dbb") .uri("https://test-bucket.s3.us-east-1.amazonaws.com/?list-type=2&prefix=prefix~") .body(SdkBody::empty()) .unwrap(), diff --git a/aws/sdk/integration-tests/s3control/tests/signing-it.rs b/aws/sdk/integration-tests/s3control/tests/signing-it.rs index 7b06289a1c..6d258496b1 100644 --- a/aws/sdk/integration-tests/s3control/tests/signing-it.rs +++ b/aws/sdk/integration-tests/s3control/tests/signing-it.rs @@ -13,10 +13,10 @@ use aws_smithy_types::body::SdkBody; async fn test_signer() { let http_client = StaticReplayClient::new(vec![ReplayEvent::new( http::Request::builder() - .header("authorization", + .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/s3/aws4_request, \ - SignedHeaders=host;x-amz-account-id;x-amz-content-sha256;x-amz-date;x-amz-security-token;x-amz-user-agent, \ - Signature=01a71226e959c7b0b998adf26fa266f9c3612df57a60b187d549822e86d90667") + SignedHeaders=host;x-amz-account-id;x-amz-content-sha256;x-amz-date;x-amz-user-agent, \ + Signature=0102a74cb220f8445c4efada17660572ff813e07b524032ec831e8c2514be903") .uri("https://test-bucket.s3-control.us-east-1.amazonaws.com/v20180820/accesspoint") .body(SdkBody::empty()) .unwrap(), diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt index ca526b8650..1f11f99164 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt @@ -220,7 +220,7 @@ private class HttpAuthConfigCustomization( /// Sets an API key resolver will be used for authentication. pub fn api_key_resolver(mut self, api_key_resolver: impl #{ResolveIdentity} + 'static) -> Self { - self.runtime_components.push_identity_resolver( + self.runtime_components.set_identity_resolver( #{HTTP_API_KEY_AUTH_SCHEME_ID}, #{SharedIdentityResolver}::new(api_key_resolver) ); @@ -240,7 +240,7 @@ private class HttpAuthConfigCustomization( /// Sets a bearer token provider that will be used for HTTP bearer auth. pub fn bearer_token_resolver(mut self, bearer_token_resolver: impl #{ResolveIdentity} + 'static) -> Self { - self.runtime_components.push_identity_resolver( + self.runtime_components.set_identity_resolver( #{HTTP_BEARER_AUTH_SCHEME_ID}, #{SharedIdentityResolver}::new(bearer_token_resolver) ); @@ -260,7 +260,7 @@ private class HttpAuthConfigCustomization( /// Sets a login resolver that will be used for HTTP basic auth. pub fn basic_auth_login_resolver(mut self, basic_auth_resolver: impl #{ResolveIdentity} + 'static) -> Self { - self.runtime_components.push_identity_resolver( + self.runtime_components.set_identity_resolver( #{HTTP_BASIC_AUTH_SCHEME_ID}, #{SharedIdentityResolver}::new(basic_auth_resolver) ); @@ -280,7 +280,7 @@ private class HttpAuthConfigCustomization( /// Sets a login resolver that will be used for HTTP digest auth. pub fn digest_auth_login_resolver(mut self, digest_auth_resolver: impl #{ResolveIdentity} + 'static) -> Self { - self.runtime_components.push_identity_resolver( + self.runtime_components.set_identity_resolver( #{HTTP_DIGEST_AUTH_SCHEME_ID}, #{SharedIdentityResolver}::new(digest_auth_resolver) ); diff --git a/rust-runtime/aws-smithy-runtime-api/Cargo.toml b/rust-runtime/aws-smithy-runtime-api/Cargo.toml index 3e7415448b..93f88c0cd6 100644 --- a/rust-runtime/aws-smithy-runtime-api/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime-api/Cargo.toml @@ -28,7 +28,7 @@ zeroize = { version = "1", optional = true } [dev-dependencies] proptest = "1" -tokio = { version = "1.25", features = ["rt", "macros"] } +tokio = { version = "1.25", features = ["macros", "rt", "rt-multi-thread"] } [package.metadata.docs.rs] all-features = true diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs index 0b0c58c321..2f94c58543 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs @@ -570,7 +570,11 @@ impl RuntimeComponentsBuilder { self } - /// Adds an identity resolver. + /// This method is broken since it does not replace an existing identity resolver of the given auth scheme ID. + /// Use `set_identity_resolver` instead. + #[deprecated( + note = "This method is broken since it does not replace an existing identity resolver of the given auth scheme ID. Use `set_identity_resolver` instead." + )] pub fn push_identity_resolver( &mut self, scheme_id: AuthSchemeId, @@ -583,13 +587,40 @@ impl RuntimeComponentsBuilder { self } + /// Sets the identity resolver for a given `scheme_id`. + /// + /// If there is already an identity resolver for that `scheme_id`, this method will replace + /// the existing one with the passed-in `identity_resolver`. + pub fn set_identity_resolver( + &mut self, + scheme_id: AuthSchemeId, + identity_resolver: impl ResolveIdentity + 'static, + ) -> &mut Self { + let tracked = Tracked::new( + self.builder_name, + ConfiguredIdentityResolver::new(scheme_id, identity_resolver.into_shared()), + ); + + if let Some(s) = self + .identity_resolvers + .iter_mut() + .find(|s| s.value.scheme_id() == scheme_id) + { + *s = tracked; + } else { + self.identity_resolvers.push(tracked); + } + + self + } + /// Adds an identity resolver. pub fn with_identity_resolver( mut self, scheme_id: AuthSchemeId, identity_resolver: impl ResolveIdentity + 'static, ) -> Self { - self.push_identity_resolver(scheme_id, identity_resolver); + self.set_identity_resolver(scheme_id, identity_resolver); self } @@ -1144,4 +1175,45 @@ mod tests { fn building_test_builder_should_not_panic() { let _ = RuntimeComponentsBuilder::for_tests().build(); // should not panic } + + #[test] + fn set_identity_resolver_should_replace_existing_resolver_for_given_auth_scheme() { + use crate::client::auth::AuthSchemeId; + use crate::client::identity::{Identity, IdentityFuture, ResolveIdentity}; + use crate::client::runtime_components::{GetIdentityResolver, RuntimeComponents}; + use aws_smithy_types::config_bag::ConfigBag; + use tokio::runtime::Runtime; + + #[derive(Debug)] + struct AnotherFakeIdentityResolver; + impl ResolveIdentity for AnotherFakeIdentityResolver { + fn resolve_identity<'a>( + &'a self, + _: &'a RuntimeComponents, + _: &'a ConfigBag, + ) -> IdentityFuture<'a> { + IdentityFuture::ready(Ok(Identity::new("doesntmatter", None))) + } + } + + // Set a different `IdentityResolver` for the `fake` auth scheme already configured in + // a test runtime components builder + let rc = RuntimeComponentsBuilder::for_tests() + .with_identity_resolver(AuthSchemeId::new("fake"), AnotherFakeIdentityResolver) + .build() + .expect("should build RuntimeComponents"); + + let resolver = rc + .identity_resolver(AuthSchemeId::new("fake")) + .expect("identity resolver should be found"); + + let identity = Runtime::new().unwrap().block_on(async { + resolver + .resolve_identity(&rc, &ConfigBag::base()) + .await + .expect("identity should be resolved") + }); + + assert_eq!(Some(&"doesntmatter"), identity.data::<&str>()); + } } diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs index 5ae8ab04d8..bd875c72e6 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs @@ -255,7 +255,7 @@ impl OperationBuilder { .push_auth_scheme(SharedAuthScheme::new(NoAuthScheme::default())); self.runtime_components .set_identity_cache(Some(IdentityCache::no_cache())); - self.runtime_components.push_identity_resolver( + self.runtime_components.set_identity_resolver( NO_AUTH_SCHEME_ID, SharedIdentityResolver::new(NoAuthIdentityResolver::new()), ); From 529b3f03e2b945ea2e5e879183ccfd8e74b7377c Mon Sep 17 00:00:00 2001 From: AWS SDK Rust Bot Date: Fri, 1 Dec 2023 18:50:52 +0000 Subject: [PATCH 4/5] Upgrade the smithy-rs runtime crates version to 1.0.3 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index f62062154b..ed4fedc5e4 100644 --- a/gradle.properties +++ b/gradle.properties @@ -12,7 +12,7 @@ rust.msrv=1.70.0 org.gradle.jvmargs=-Xmx1024M # Version number to use for the generated stable runtime crates -smithy.rs.runtime.crate.stable.version=1.0.2 +smithy.rs.runtime.crate.stable.version=1.0.3 # Version number to use for the generated unstable runtime crates smithy.rs.runtime.crate.unstable.version=0.60.0 From cd6baa940c2bd00a11c378891136c73238c02a0b Mon Sep 17 00:00:00 2001 From: AWS SDK Rust Bot Date: Fri, 1 Dec 2023 18:53:05 +0000 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 6 ++ CHANGELOG.next.toml | 20 +----- aws/SDK_CHANGELOG.next.json | 125 ++++++++---------------------------- 3 files changed, 35 insertions(+), 116 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5512c0f7a..cd4fe2a3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ +December 1st, 2023 +================== +**New this release:** +- (client, [smithy-rs#3278](https://github.com/smithy-lang/smithy-rs/issues/3278)) `RuntimeComponentsBuilder::push_identity_resolver` is now deprecated since it does not replace the existing identity resolver of a given auth scheme ID. Use `RuntimeComponentsBuilder::set_identity_resolver` instead. + + November 27th, 2023 =================== **New this release:** diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 6f90ff174b..fc4c4c2578 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -9,22 +9,4 @@ # message = "Fix typos in module documentation for generated crates" # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} -# author = "rcoh" - -[[aws-sdk-rust]] -message = "Fix `config::Builder::set_credentials_provider` to override a credentials provider previously set." -references = ["aws-sdk-rust#973", "smithy-rs#3278"] -meta = { "breaking" = false, "tada" = false, "bug" = true } -author = "ysaito1001" - -[[aws-sdk-rust]] -message = "`config::Config::credentials_provider` has been broken since `release-2023-11-15` and is now marked as `deprecated` explicitly." -references = ["smithy-rs#3251", "smithy-rs#3278"] -meta = { "breaking" = false, "tada" = false, "bug" = false } -author = "ysaito1001" - -[[smithy-rs]] -message = "`RuntimeComponentsBuilder::push_identity_resolver` is now deprecated since it does not replace the existing identity resolver of a given auth scheme ID. Use `RuntimeComponentsBuilder::set_identity_resolver` instead." -references = ["smithy-rs#3278"] -meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" } -author = "ysaito1001" +# author = "rcoh" \ No newline at end of file diff --git a/aws/SDK_CHANGELOG.next.json b/aws/SDK_CHANGELOG.next.json index 7b3cbbdd85..8823e0a91a 100644 --- a/aws/SDK_CHANGELOG.next.json +++ b/aws/SDK_CHANGELOG.next.json @@ -6,21 +6,7 @@ "smithy-rs": [], "aws-sdk-rust": [ { - "message": "Add configurable stalled-stream protection for downloads.\n\nWhen making HTTP calls,\nit's possible for a connection to 'stall out' and emit no more data due to server-side issues.\nIn the event this happens, it's desirable for the stream to error out as quickly as possible.\nWhile timeouts can protect you from this issue, they aren't adaptive to the amount of data\nbeing sent and so must be configured specifically for each use case. When enabled, stalled-stream\nprotection will ensure that bad streams error out quickly, regardless of the amount of data being\ndownloaded.\n\nProtection is enabled by default for all clients but can be configured or disabled.\nSee [this discussion](https://github.com/awslabs/aws-sdk-rust/discussions/956) for more details.\n", - "meta": { - "bug": false, - "breaking": true, - "tada": true - }, - "author": "Velfi", - "references": [ - "smithy-rs#3202" - ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", - "age": 5 - }, - { - "message": "Make certain types for EMR Serverless optional. Previously, they defaulted to 0, but this created invalid requests.", + "message": "Make properties of S3Control PublicAccessBlockConfiguration optional. Previously, they defaulted to false, but this created invalid requests.", "meta": { "bug": true, "breaking": true, @@ -28,13 +14,13 @@ }, "author": "milesziemer", "references": [ - "smithy-rs#3217" + "smithy-rs#3246" ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", + "since-commit": "e155c3048b9989fe406ef575d461ea01dfaf294c", "age": 5 }, { - "message": "Prevent multiplication overflow in backoff computation", + "message": "Allow `--` to be used in bucket names for S3", "meta": { "bug": true, "breaking": false, @@ -42,70 +28,28 @@ }, "author": "rcoh", "references": [ - "smithy-rs#3229", - "aws-sdk-rust#960" - ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", - "age": 5 - }, - { - "message": "Make some types for various services optional. Previously, they defaulted to 0, but this created invalid requests.", - "meta": { - "bug": true, - "breaking": true, - "tada": false - }, - "author": "milesziemer", - "references": [ - "smithy-rs#3228" - ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", - "age": 5 - }, - { - "message": "Types/functions that were deprecated in previous releases were removed. Unfortunately, some of these deprecations\nwere ignored by the Rust compiler (we found out later that `#[deprecated]` on `pub use` doesn't work). See\nthe [deprecations removal list](https://github.com/smithy-lang/smithy-rs/discussions/3223) for more details.\n", - "meta": { - "bug": false, - "breaking": true, - "tada": false - }, - "author": "jdisanti", - "references": [ - "smithy-rs#3222" - ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", - "age": 5 - }, - { - "message": "Add `Display` impl for `DateTime`.", - "meta": { - "bug": false, - "breaking": false, - "tada": true - }, - "author": "HakanVardarr", - "references": [ - "smithy-rs#3183" + "smithy-rs#3253" ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", - "age": 5 + "since-commit": "48e3c95a3f10eebd5a637f8e7670c4232cdabbe4", + "age": 4 }, { - "message": "Types/functions that were previously `#[doc(hidden)]` in `aws-config`, `aws-inlineable`, `aws-types`, and the SDK crates are now visible. For those that are not intended to be used directly, they are called out in their docs as such.", + "message": "Retry additional classes of H2 errors (H2 GoAway & H2 ResetStream)", "meta": { "bug": false, "breaking": false, "tada": false }, - "author": "ysaito1001", + "author": "rcoh", "references": [ - "smithy-rs#3226" + "aws-sdk-rust#738", + "aws-sdk-rust#858" ], - "since-commit": "f66f9246bccc376462ef47aec5707569fca214f5", - "age": 5 + "since-commit": "88970ba88ef45266aade152c7c1da8e90b24c0d7", + "age": 2 }, { - "message": "Make properties of S3Control PublicAccessBlockConfiguration optional. Previously, they defaulted to false, but this created invalid requests.", + "message": "Make some properties for IoT types optional. Previously, they defaulted to false, but that isn't how the service actual works.", "meta": { "bug": true, "breaking": true, @@ -113,52 +57,39 @@ }, "author": "milesziemer", "references": [ - "smithy-rs#3246" + "smithy-rs#3256" ], - "since-commit": "e155c3048b9989fe406ef575d461ea01dfaf294c", - "age": 4 + "since-commit": "88970ba88ef45266aade152c7c1da8e90b24c0d7", + "age": 2 }, { - "message": "Allow `--` to be used in bucket names for S3", + "message": "Fix `config::Builder::set_credentials_provider` to override a credentials provider previously set.", "meta": { "bug": true, "breaking": false, "tada": false }, - "author": "rcoh", + "author": "ysaito1001", "references": [ - "smithy-rs#3253" + "aws-sdk-rust#973", + "smithy-rs#3278" ], - "since-commit": "48e3c95a3f10eebd5a637f8e7670c4232cdabbe4", - "age": 3 + "since-commit": "529b3f03e2b945ea2e5e879183ccfd8e74b7377c", + "age": 1 }, { - "message": "Retry additional classes of H2 errors (H2 GoAway & H2 ResetStream)", + "message": "`config::Config::credentials_provider` has been broken since `release-2023-11-15` and is now marked as `deprecated` explicitly.", "meta": { "bug": false, "breaking": false, "tada": false }, - "author": "rcoh", - "references": [ - "aws-sdk-rust#738", - "aws-sdk-rust#858" - ], - "since-commit": "88970ba88ef45266aade152c7c1da8e90b24c0d7", - "age": 1 - }, - { - "message": "Make some properties for IoT types optional. Previously, they defaulted to false, but that isn't how the service actual works.", - "meta": { - "bug": true, - "breaking": true, - "tada": false - }, - "author": "milesziemer", + "author": "ysaito1001", "references": [ - "smithy-rs#3256" + "smithy-rs#3251", + "smithy-rs#3278" ], - "since-commit": "88970ba88ef45266aade152c7c1da8e90b24c0d7", + "since-commit": "529b3f03e2b945ea2e5e879183ccfd8e74b7377c", "age": 1 } ],