Merge branch 'main' into RFC30/test2

Author: thomas-k-cameron

CHANGELOG.next.toml

@@ -11,6 +11,12 @@
 # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
 # author = "rcoh"
 
+ [[aws-sdk-rust]]
+ message = "Automatically exclude X-Ray trace ID headers and authorization headers from SigV4 canonical request calculations."
+ references = ["smithy-rs#2815"]
+ meta = { "breaking" = false, "tada" = false, "bug" = true }
+ author = "relevantsam"
+
  [[aws-sdk-rust]]
  message = "Add accessors to Builders"
  references = ["smithy-rs#2791"]

aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs

@@ -774,14 +774,46 @@ mod tests {
         assert_eq!(creq.values.signed_headers().as_str(), "host;x-amz-date");
     }
 
-    // It should exclude user-agent and x-amz-user-agent headers from presigning
+    // It should exclude authorization, user-agent, x-amzn-trace-id headers from presigning
+    #[test]
+    fn non_presigning_header_exclusion() {
+        let request = http::Request::builder()
+            .uri("https://some-endpoint.some-region.amazonaws.com")
+            .header("authorization", "test-authorization")
+            .header("content-type", "application/xml")
+            .header("content-length", "0")
+            .header("user-agent", "test-user-agent")
+            .header("x-amzn-trace-id", "test-trace-id")
+            .header("x-amz-user-agent", "test-user-agent")
+            .body("")
+            .unwrap();
+        let request = SignableRequest::from(&request);
+
+        let settings = SigningSettings {
+            signature_location: SignatureLocation::Headers,
+            ..Default::default()
+        };
+
+        let signing_params = signing_params(settings);
+        let canonical = CanonicalRequest::from(&request, &signing_params).unwrap();
+
+        let values = canonical.values.as_headers().unwrap();
+        assert_eq!(
+            "content-length;content-type;host;x-amz-date;x-amz-user-agent",
+            values.signed_headers.as_str()
+        );
+    }
+
+    // It should exclude authorization, user-agent, x-amz-user-agent, x-amzn-trace-id headers from presigning
     #[test]
     fn presigning_header_exclusion() {
         let request = http::Request::builder()
             .uri("https://some-endpoint.some-region.amazonaws.com")
+            .header("authorization", "test-authorization")
             .header("content-type", "application/xml")
             .header("content-length", "0")
             .header("user-agent", "test-user-agent")
+            .header("x-amzn-trace-id", "test-trace-id")
             .header("x-amz-user-agent", "test-user-agent")
             .body("")
             .unwrap();

aws/rust-runtime/aws-sigv4/src/http_request/settings.rs

@@ -3,12 +3,14 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-use http::header::{HeaderName, USER_AGENT};
+use http::header::{HeaderName, AUTHORIZATION, USER_AGENT};
 use std::time::Duration;
 
 /// HTTP signing parameters
 pub type SigningParams<'a> = crate::SigningParams<'a, SigningSettings>;
 
+const HEADER_NAME_X_RAY_TRACE_ID: &str = "x-amzn-trace-id";
+
 /// HTTP-specific signing settings
 #[derive(Debug, PartialEq)]
 #[non_exhaustive]
@@ -97,15 +99,30 @@ pub enum SessionTokenMode {
 
 impl Default for SigningSettings {
     fn default() -> Self {
-        // The user agent header should not be signed because it may be altered by proxies
-        const EXCLUDED_HEADERS: [HeaderName; 1] = [USER_AGENT];
-
+        // Headers that are potentially altered by proxies or as a part of standard service operations.
+        // Reference:
+        // Go SDK: <https://github.com/aws/aws-sdk-go/blob/v1.44.289/aws/signer/v4/v4.go#L92>
+        // Java SDK: <https://github.com/aws/aws-sdk-java-v2/blob/master/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java#L70>
+        // JS SDK: <https://github.com/aws/aws-sdk-js/blob/master/lib/signers/v4.js#L191>
+        // There is no single source of truth for these available, so this uses the minimum common set of the excluded options.
+        // Instantiate this every time, because SigningSettings takes a Vec (which cannot be const);
+        let excluded_headers = Some(
+            [
+                // This header is calculated as part of the signing process, so if it's present, discard it
+                AUTHORIZATION,
+                // Changes when sent by proxy
+                USER_AGENT,
+                // Changes based on the request from the client
+                HeaderName::from_static(HEADER_NAME_X_RAY_TRACE_ID),
+            ]
+            .to_vec(),
+        );
         Self {
             percent_encoding_mode: PercentEncodingMode::Double,
             payload_checksum_kind: PayloadChecksumKind::NoHeader,
             signature_location: SignatureLocation::Headers,
             expires_in: None,
-            excluded_headers: Some(EXCLUDED_HEADERS.to_vec()),
+            excluded_headers,
             uri_path_normalization_mode: UriPathNormalizationMode::Enabled,
             session_token_mode: SessionTokenMode::Include,
         }

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt

@@ -8,6 +8,7 @@ package software.amazon.smithy.rustsdk
 import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
 import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
+import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.Approx
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.AsyncStd
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.AsyncStream
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.BytesUtils
@@ -26,6 +27,8 @@ import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Compani
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TracingAppender
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TracingSubscriber
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TracingTest
+import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.smithyRuntime
+import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.smithyRuntimeApi
 import software.amazon.smithy.rust.codegen.core.rustlang.DependencyScope
 import software.amazon.smithy.rust.codegen.core.rustlang.Writable
 import software.amazon.smithy.rust.codegen.core.rustlang.writable
@@ -73,21 +76,30 @@ class IntegrationTestDependencies(
     private val hasTests: Boolean,
     private val hasBenches: Boolean,
 ) : LibRsCustomization() {
+    private val runtimeConfig = codegenContext.runtimeConfig
     override fun section(section: LibRsSection) = when (section) {
         is LibRsSection.Body -> testDependenciesOnly {
             if (hasTests) {
                 val smithyClient = CargoDependency.smithyClient(codegenContext.runtimeConfig)
                     .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
                 val smithyAsync = CargoDependency.smithyAsync(codegenContext.runtimeConfig)
                     .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
+                val smithyTypes = CargoDependency.smithyTypes(codegenContext.runtimeConfig)
+                    .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
                 addDependency(smithyClient)
                 addDependency(smithyAsync)
+                addDependency(smithyTypes)
                 addDependency(CargoDependency.smithyProtocolTestHelpers(codegenContext.runtimeConfig))
                 addDependency(SerdeJson)
                 addDependency(Tokio)
                 addDependency(FuturesUtil)
                 addDependency(Tracing.toDevDependency())
                 addDependency(TracingSubscriber)
+
+                if (codegenContext.smithyRuntimeMode.generateOrchestrator) {
+                    addDependency(smithyRuntime(runtimeConfig).copy(features = setOf("test-util"), scope = DependencyScope.Dev))
+                    addDependency(smithyRuntimeApi(runtimeConfig).copy(features = setOf("test-util"), scope = DependencyScope.Dev))
+                }
             }
             if (hasBenches) {
                 addDependency(Criterion)
@@ -103,6 +115,7 @@ class IntegrationTestDependencies(
     private fun serviceSpecificCustomizations(): List<LibRsCustomization> = when (moduleName) {
         "transcribestreaming" -> listOf(TranscribeTestDependencies())
         "s3" -> listOf(S3TestDependencies(codegenContext))
+        "dynamodb" -> listOf(DynamoDbTestDependencies())
         else -> emptyList()
     }
 }
@@ -116,6 +129,13 @@ class TranscribeTestDependencies : LibRsCustomization() {
         }
 }
 
+class DynamoDbTestDependencies : LibRsCustomization() {
+    override fun section(section: LibRsSection): Writable =
+        writable {
+            addDependency(Approx)
+        }
+}
+
 class S3TestDependencies(private val codegenContext: ClientCodegenContext) : LibRsCustomization() {
     override fun section(section: LibRsSection): Writable =
         writable {

aws/sdk/integration-tests/dynamodb/Cargo.toml

@@ -11,23 +11,27 @@ publish = false
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+approx = "0.5.1"
+aws-config = { path = "../../build/aws-sdk/sdk/aws-config" }
 aws-credential-types = { path = "../../build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] }
 aws-http = { path = "../../build/aws-sdk/sdk/aws-http" }
 aws-sdk-dynamodb = { path = "../../build/aws-sdk/sdk/dynamodb" }
+aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async" }
 aws-smithy-client = { path = "../../build/aws-sdk/sdk/aws-smithy-client", features = ["test-util", "rustls"] }
 aws-smithy-http = { path = "../../build/aws-sdk/sdk/aws-smithy-http" }
-aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" }
 aws-smithy-protocol-test = { path = "../../build/aws-sdk/sdk/aws-smithy-protocol-test" }
-aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async" }
+aws-smithy-runtime = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime", features = ["test-util"]}
+aws-smithy-runtime-api = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime-api", features = ["test-util"]}
+aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types", features = ["test-util"]}
 aws-types = { path = "../../build/aws-sdk/sdk/aws-types" }
 bytes = "1.0.0"
 criterion = { version = "0.4.0" }
 futures-util = { version = "0.3.16", default-features = false }
 http = "0.2.0"
 serde_json = "1.0.0"
 tokio = { version = "1.23.1", features = ["full", "test-util"] }
-tracing-subscriber = { version = "0.3.15", features = ["env-filter"] }
 tokio-stream = "0.1.5"
+tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
 
 [[bench]]
 name = "deserialization_bench"

aws/sdk/integration-tests/dynamodb/tests/retries-with-client-rate-limiting.rs

@@ -0,0 +1,173 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#[cfg(aws_sdk_orchestrator_mode)]
+mod test {
+    use aws_sdk_dynamodb::config::{Credentials, Region, SharedAsyncSleep};
+    use aws_sdk_dynamodb::{config::retry::RetryConfig, error::ProvideErrorMetadata};
+    use aws_smithy_async::rt::sleep::TokioSleep;
+    use aws_smithy_async::test_util::instant_time_and_sleep;
+    use aws_smithy_async::time::SharedTimeSource;
+    use aws_smithy_async::time::SystemTimeSource;
+    use aws_smithy_client::test_connection::TestConnection;
+    use aws_smithy_http::body::SdkBody;
+    use aws_smithy_runtime::client::retries::RetryPartition;
+    use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse};
+    use aws_smithy_types::timeout::TimeoutConfigBuilder;
+    use std::time::{Duration, Instant, SystemTime};
+
+    fn req() -> HttpRequest {
+        http::Request::builder()
+            .body(SdkBody::from("request body"))
+            .unwrap()
+    }
+
+    fn ok() -> HttpResponse {
+        http::Response::builder()
+            .status(200)
+            .header("server", "Server")
+            .header("content-type", "application/x-amz-json-1.0")
+            .header("content-length", "23")
+            .header("connection", "keep-alive")
+            .header("x-amz-crc32", "2335643545")
+            .body(SdkBody::from("{ \"TableNames\": [ \"Test\" ] }"))
+            .unwrap()
+    }
+
+    fn err() -> HttpResponse {
+        http::Response::builder()
+            .status(500)
+            .body(SdkBody::from("{ \"message\": \"The request has failed because of an unknown error, exception or failure.\", \"code\": \"InternalServerError\" }"))
+            .unwrap()
+    }
+
+    fn throttling_err() -> HttpResponse {
+        http::Response::builder()
+            .status(400)
+            .body(SdkBody::from("{ \"message\": \"The request was denied due to request throttling.\", \"code\": \"ThrottlingException\" }"))
+            .unwrap()
+    }
+
+    #[tokio::test]
+    async fn test_adaptive_retries_with_no_throttling_errors() {
+        let (time_source, sleep_impl) = instant_time_and_sleep(SystemTime::UNIX_EPOCH);
+
+        let events = vec![
+            // First operation
+            (req(), err()),
+            (req(), err()),
+            (req(), ok()),
+            // Second operation
+            (req(), err()),
+            (req(), ok()),
+            // Third operation will fail, only errors
+            (req(), err()),
+            (req(), err()),
+            (req(), err()),
+            (req(), err()),
+        ];
+
+        let conn = TestConnection::new(events);
+        let config = aws_sdk_dynamodb::Config::builder()
+            .credentials_provider(Credentials::for_tests())
+            .region(Region::new("us-east-1"))
+            .retry_config(
+                RetryConfig::adaptive()
+                    .with_max_attempts(4)
+                    .with_use_static_exponential_base(true),
+            )
+            .time_source(SharedTimeSource::new(time_source))
+            .sleep_impl(SharedAsyncSleep::new(sleep_impl.clone()))
+            .retry_partition(RetryPartition::new(
+                "test_adaptive_retries_with_no_throttling_errors",
+            ))
+            .http_connector(conn.clone())
+            .build();
+        let expected_table_names = vec!["Test".to_owned()];
+
+        // We create a new client each time to ensure that the cross-client retry state is working.
+        let client = aws_sdk_dynamodb::Client::from_conf(config.clone());
+        let res = client.list_tables().send().await.unwrap();
+        assert_eq!(sleep_impl.total_duration(), Duration::from_secs(3));
+        assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));
+        // Three requests should have been made, two failing & one success
+        assert_eq!(conn.requests().len(), 3);
+
+        let client = aws_sdk_dynamodb::Client::from_conf(config.clone());
+        let res = client.list_tables().send().await.unwrap();
+        assert_eq!(sleep_impl.total_duration(), Duration::from_secs(3 + 1));
+        assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));
+        // Two requests should have been made, one failing & one success (plus previous requests)
+        assert_eq!(conn.requests().len(), 5);
+
+        let client = aws_sdk_dynamodb::Client::from_conf(config);
+        let err = client.list_tables().send().await.unwrap_err();
+        assert_eq!(sleep_impl.total_duration(), Duration::from_secs(3 + 1 + 7),);
+        assert_eq!(err.code(), Some("InternalServerError"));
+        // four requests should have been made, all failing (plus previous requests)
+        assert_eq!(conn.requests().len(), 9);
+    }
+
+    #[tokio::test]
+    async fn test_adaptive_retries_with_throttling_errors_times_out() {
+        tracing_subscriber::fmt::init();
+        let events = vec![
+            // First operation
+            (req(), err()),
+            (req(), ok()),
+            // Second operation
+            (req(), err()),
+            (req(), throttling_err()),
+            (req(), ok()),
+        ];
+
+        let conn = TestConnection::new(events);
+        let config = aws_sdk_dynamodb::Config::builder()
+            .credentials_provider(Credentials::for_tests())
+            .region(Region::new("us-east-1"))
+            .retry_config(
+                RetryConfig::adaptive()
+                    .with_max_attempts(4)
+                    .with_initial_backoff(Duration::from_millis(50))
+                    .with_use_static_exponential_base(true),
+            )
+            .timeout_config(
+                TimeoutConfigBuilder::new()
+                    .operation_attempt_timeout(Duration::from_millis(100))
+                    .build(),
+            )
+            .time_source(SharedTimeSource::new(SystemTimeSource::new()))
+            .sleep_impl(SharedAsyncSleep::new(TokioSleep::new()))
+            .http_connector(conn.clone())
+            .retry_partition(RetryPartition::new(
+                "test_adaptive_retries_with_throttling_errors_times_out",
+            ))
+            .build();
+
+        let expected_table_names = vec!["Test".to_owned()];
+        let start = Instant::now();
+
+        // We create a new client each time to ensure that the cross-client retry state is working.
+        let client = aws_sdk_dynamodb::Client::from_conf(config.clone());
+        let res = client.list_tables().send().await.unwrap();
+        assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));
+        // Three requests should have been made, two failing & one success
+        assert_eq!(conn.requests().len(), 2);
+
+        let client = aws_sdk_dynamodb::Client::from_conf(config);
+        let err = client.list_tables().send().await.unwrap_err();
+        assert_eq!(err.to_string(), "request has timed out".to_owned());
+        // two requests should have been made, both failing (plus previous requests)
+        assert_eq!(conn.requests().len(), 2 + 2);
+
+        let since = start.elapsed();
+        // At least 300 milliseconds must pass:
+        // - 50ms for the first retry on attempt 1
+        // - 50ms for the second retry on attempt 3
+        // - 100ms for the throttling delay triggered by attempt 4, which required a delay longer than the attempt timeout.
+        // - 100ms for the 5th attempt, which would have succeeded, but required a delay longer than the attempt timeout.
+        assert!(since.as_secs_f64() > 0.3);
+    }
+}