Address post-merge feedback on smithy-rs#3827

buildSrc/src/main/kotlin/CrateSet.kt

@@ -39,6 +39,8 @@ object CrateSet {
         }
     }
 
+    // If we make changes to `AWS_SDK_RUNTIME`, also update the list in
+    // https://github.com/smithy-lang/smithy-rs/blob/main/tools/ci-build/sdk-lockfiles/src/audit.rs#L22
     val AWS_SDK_RUNTIME =
         listOf(
             "aws-config",
@@ -79,13 +81,16 @@ object CrateSet {
 
     val AWS_SDK_SMITHY_RUNTIME = SMITHY_RUNTIME_COMMON
 
-    val SERVER_SMITHY_RUNTIME =
-        SMITHY_RUNTIME_COMMON +
-            listOf(
-                Crate("aws-smithy-http-server", UNSTABLE_VERSION_PROP_NAME),
-                Crate("aws-smithy-http-server-python", UNSTABLE_VERSION_PROP_NAME),
-                Crate("aws-smithy-http-server-typescript", UNSTABLE_VERSION_PROP_NAME),
-            )
+    // If we make changes to `SERVER_SPECIFIC_SMITHY_RUNTIME`, also update the list in
+    // https://github.com/smithy-lang/smithy-rs/blob/main/tools/ci-build/sdk-lockfiles/src/audit.rs#L38
+    private val SERVER_SPECIFIC_SMITHY_RUNTIME =
+        listOf(
+            Crate("aws-smithy-http-server", UNSTABLE_VERSION_PROP_NAME),
+            Crate("aws-smithy-http-server-python", UNSTABLE_VERSION_PROP_NAME),
+            Crate("aws-smithy-http-server-typescript", UNSTABLE_VERSION_PROP_NAME),
+        )
+
+    val SERVER_SMITHY_RUNTIME = SMITHY_RUNTIME_COMMON + SERVER_SPECIFIC_SMITHY_RUNTIME
 
     val ENTIRE_SMITHY_RUNTIME = (AWS_SDK_SMITHY_RUNTIME + SERVER_SMITHY_RUNTIME).toSortedSet(compareBy { it.name })
 

tools/ci-build/sdk-lockfiles/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "sdk-lockfiles"
-version = "0.1.0"
+version = "0.1.1"
 authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>"]
 description = """
 A CLI tool to audit lockfiles for Smithy runtime crates, AWS runtime crates, `aws-config`, and the workspace containing

tools/ci-build/sdk-lockfiles/README.md

@@ -2,23 +2,41 @@ sdk-lockfiles
 =============
 
 This CLI tool audits the `Cargo.lock` files in the `smithy-rs` repository. These lockfiles are used to ensure
-reproducible builds. The `sdk-lockfiles` tool specifically audits the following lockfiles:
-- The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/rust-runtime/Cargo.lock) for Smithy runtime crates
-- The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/aws/rust-runtime/Cargo.lock) for AWS runtime crates
-- The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/aws/rust-runtime/aws-config/Cargo.lock) for the `aws-config` crate
-- The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/aws/sdk/Cargo.lock) for the workspace containing code-generated AWS SDK crates (*)
+reproducible builds during our release process for both `smithy-rs` and `aws-sdk-rust`. When a crate dependency is not
+pinned to a fixed version, it risks being affected by newer versions of that dependency published to crates.io, which
+could potentially break the build.
 
-Specifically, the tool ensures that the lockfile marked with (*) is a superset containing all dependencies listed in
-the rest of the runtime lockfiles. If it detects a new dependency in the AWS SDK crates introduced by any of the runtime
-lockfiles (unless the dependency is introduced by a server runtime crate), it will output a message similar to the
-following:
+We track the following lockfiles in the `smithy-rs` repository:
+1. The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/rust-runtime/Cargo.lock) for Smithy runtime crates
+2. The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/aws/rust-runtime/Cargo.lock) for AWS runtime crates
+3. The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/aws/rust-runtime/aws-config/Cargo.lock) for the `aws-config` crate
+4. The [lockfile](https://github.com/smithy-lang/smithy-rs/blob/main/aws/sdk/Cargo.lock) for the workspace containing code-generated AWS SDK crates
+
+The first three lockfiles can be easily updated during development with a `cargo` command. However, the fourth lockfile
+, known as the SDK lockfile, is generated by the code generator and is not checked into to the `smithy-rs` repository as
+frequently as the first three runtime lockfiles. As a result, new dependencies added to any of the runtime lockfiles may
+not be reflected in the SDK lockfile.
+
+The `sdk-lockfiles` tool ensures that the SDK lockfile is a superset containing all dependencies listed in the three
+runtime lockfiles. If it detects a new dependency in the AWS SDK crates introduced by any of the runtime lockfiles it
+will output a message similar to the following (unless the dependency is introduced by a server specific runtime crate):
 ```
 $ sdk-lockfiles audit
 2024-09-10T16:48:38.460518Z  INFO sdk_lockfiles::audit: checking whether `rust-runtime/Cargo.lock` is covered by the SDK lockfile...
 2024-09-10T16:48:38.489879Z  INFO sdk_lockfiles::audit: checking whether `aws/rust-runtime/Cargo.lock` is covered by the SDK lockfile...
 2024-09-10T16:48:38.490306Z  INFO sdk_lockfiles::audit: checking whether `aws/rust-runtime/aws-config/Cargo.lock` is covered by the SDK lockfile...
-`minicbor` (0.24.2), used by `rust-runtime/Cargo.lock`, is not contained in SDK lockfile!
+`minicbor` (0.24.2), used by `rust-runtime/Cargo.lock`, is not contained in the SDK lockfile!
 Error: there are lockfile audit failures
 ```
 
 This tool is intended for automated use.
+
+## Limitation
+The `sdk-lockfiles` tool does not verify whether new dependencies introduced in [CargoDependency.kt](https://github.com/smithy-lang/smithy-rs/blob/main/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt)
+are included in the SDK lockfile. This is because dependencies in `CargoDependency.kt` are represented as a Kotlin data
+class. Consequently, dependencies added via the code generator, `inlineable`, or `aws-inlineable` are not considered by
+`sdk-lockfiles`.
+
+This limitation is acceptable for our operational purposes. Our release script always executes
+`./gradlew aws:sdk:syncAwsSdkLockfile`, which ensures that any dependencies added in `CargoDependency.kt` are properly
+reflected in the SDK lockfile.

tools/ci-build/sdk-lockfiles/src/audit.rs

@@ -16,25 +16,23 @@ use std::env;
 use std::iter;
 use std::path::PathBuf;
 
-// A list of AWS runtime crate must be in sync with
+// A list of the names of AWS runtime crates (crate versions do not need to match) must be in sync with
 // https://github.com/smithy-lang/smithy-rs/blob/0f9b9aba386ea3063912a0464ba6a1fd7c596018/buildSrc/src/main/kotlin/CrateSet.kt#L42-L53
-// plus `aws-inlineable`
 const AWS_SDK_RUNTIMES: &[&str] = &[
     "aws-config",
     "aws-credential-types",
     "aws-endpoint",
     "aws-http",
     "aws-hyper",
-    "aws-inlineable",
     "aws-runtime",
     "aws-runtime-api",
     "aws-sig-auth",
     "aws-sigv4",
     "aws-types",
 ];
 
-// A list of server runtime crates must be in sync with
-// https://github.com/smithy-lang/smithy-rs/blob/0f9b9aba386ea3063912a0464ba6a1fd7c596018/buildSrc/src/main/kotlin/CrateSet.kt#L85-L87
+// A list of the names of server specific runtime crates (crate versions do not need to match) must be in sync with
+// https://github.com/smithy-lang/smithy-rs/blob/main/buildSrc/src/main/kotlin/CrateSet.kt#L42
 const SERVER_SPECIFIC_RUNTIMES: &[&str] = &[
     "aws-smithy-http-server",
     "aws-smithy-http-server-python",
@@ -43,34 +41,37 @@ const SERVER_SPECIFIC_RUNTIMES: &[&str] = &[
 
 fn new_dependency_for_aws_sdk(crate_name: &str) -> bool {
     AWS_SDK_RUNTIMES.contains(&crate_name)
-        || crate_name == "inlineable"
         || (crate_name.starts_with("aws-smithy-")
             && !SERVER_SPECIFIC_RUNTIMES.contains(&crate_name))
 }
 
 // Recursively traverses a chain of dependencies originating from a potential new dependency. Returns true as soon as
 // it encounters a crate name that matches a runtime crate used by the AWS SDK.
-fn visit(graph: &Graph, node_index: NodeIndex, visited: &mut BTreeSet<NodeIndex>) -> bool {
+fn is_consumed_by_aws_sdk(
+    graph: &Graph,
+    node_index: NodeIndex,
+    visited: &mut BTreeSet<NodeIndex>,
+) -> bool {
     if !visited.insert(node_index) {
         return false;
     }
 
-    let dependencies = graph
+    let consumers = graph
         .edges_directed(
             node_index,
             cargo_lock::dependency::graph::EdgeDirection::Incoming,
         )
         .map(|edge| edge.source())
         .collect::<Vec<_>>();
 
-    for dependency_node_index in dependencies.iter() {
-        let package = &graph[*dependency_node_index];
+    for consumer_node_index in consumers.iter() {
+        let package = &graph[*consumer_node_index];
         tracing::debug!("visiting `{}`", package.name.as_str());
         if new_dependency_for_aws_sdk(package.name.as_str()) {
             tracing::debug!("it's a new dependency for the AWS SDK!");
             return true;
         }
-        if visit(graph, *dependency_node_index, visited) {
+        if is_consumed_by_aws_sdk(graph, *consumer_node_index, visited) {
             return true;
         }
     }
@@ -88,22 +89,17 @@ fn new_dependency(lockfile: &Lockfile, target: &str) -> bool {
             target
     );
     let tree = lockfile.dependency_tree().unwrap();
-    let indices: Vec<_> = [target.to_owned()]
+    let package = lockfile
+        .packages
         .iter()
-        .map(|dep| {
-            let package = lockfile
-                .packages
-                .iter()
-                .find(|pkg| pkg.name.as_str() == dep)
-                .unwrap();
-            tree.nodes()[&package.into()]
-        })
-        .collect();
+        .find(|pkg| pkg.name.as_str() == target)
+        .expect("{target} must be in dependencies listed in `lockfile`");
+    let indices = vec![tree.nodes()[&package.into()]];
 
     for index in &indices {
         let mut visited: BTreeSet<NodeIndex> = BTreeSet::new();
         tracing::debug!("traversing a dependency chain for `{}`...", target);
-        if visit(tree.graph(), *index, &mut visited) {
+        if is_consumed_by_aws_sdk(tree.graph(), *index, &mut visited) {
             return true;
         }
     }
@@ -307,17 +303,17 @@ dependencies = [
 ]
 
 [[package]]
-name = "inlineable"
-version = "0.1.0"
+name = "aws-smithy-compression"
+version = "0.0.1"
 dependencies = [
- "md-5"
+ "flate2"
 ]
 
 [[package]]
-name = "md-5"
-version = "0.10.6"
+name = "flate2"
+version = "1.0.33"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d89e7ee0cfbedfc4da3340218492196241d89eefb6dab27de5df917a6d2e78cf"
+checksum = "324a1be68054ef05ad64b861cc9eaf1d623d2d8cb25b4bf2cb9cdd902b4bf253"
 
 [[package]]
 name = "minicbor"
@@ -329,7 +325,7 @@ checksum = "5f8e213c36148d828083ae01948eed271d03f95f7e72571fa242d78184029af2"
             .unwrap();
 
             assert_eq!(
-                vec!["md-5", "minicbor"],
+                vec!["flate2", "minicbor"],
                 audit_runtime_lockfile_covered_by_sdk_lockfile(
                     &runtime_lockfile,
                     &sdk_dependency_set(),
@@ -351,19 +347,13 @@ dependencies = [
  "zeroize",
 ]
 
-[[package]]
-name = "aws-inlineable"
-version = "0.1.0"
-dependencies = [
- "ahash",
- "lru"
-]
-
 [[package]]
 name = "aws-sigv4"
 version = "1.2.3"
 dependencies = [
+ "ahash",
  "aws-credential-types",
+ "lru",
  "p256",
 ]