From 06b9597589d03d3e0341751c2f90acbce053f9ed Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Thu, 4 May 2023 17:51:22 +0200 Subject: [PATCH 01/17] remove native-tls Signed-off-by: Daniele Ahmed --- aws/rust-runtime/aws-config/Cargo.toml | 1 - aws/rust-runtime/aws-config/src/connector.rs | 16 ++---- .../src/default_provider/credentials.rs | 6 +-- .../aws-config/src/imds/client.rs | 2 +- .../aws-config/src/imds/credentials.rs | 6 +-- .../aws-config/src/meta/credentials/chain.rs | 4 +- .../aws-config/src/profile/credentials.rs | 4 +- .../aws-config/src/provider_config.rs | 2 +- .../tests/middleware_e2e_test.rs | 2 +- .../rustsdk/AwsFluentClientDecorator.kt | 7 ++- .../Cargo.toml | 20 ------- .../tests/no-rustls-in-dependency.rs | 52 ------------------- .../client/FluentClientDecorator.kt | 1 - design/src/transport/connector.md | 42 +++++++++++++++ rust-runtime/aws-smithy-client/Cargo.toml | 1 - .../aws-smithy-client/external-types.toml | 2 +- rust-runtime/aws-smithy-client/src/builder.rs | 38 +++----------- rust-runtime/aws-smithy-client/src/conns.rs | 44 +++++++--------- .../aws-smithy-client/src/hyper_ext.rs | 8 +-- rust-runtime/aws-smithy-client/src/lib.rs | 5 +- 20 files changed, 93 insertions(+), 170 deletions(-) delete mode 100644 aws/sdk/integration-tests/using-native-tls-instead-of-rustls/Cargo.toml delete mode 100644 aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs create mode 100644 design/src/transport/connector.md diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index 2a43647379..228415c8da 100644 --- a/aws/rust-runtime/aws-config/Cargo.toml +++ b/aws/rust-runtime/aws-config/Cargo.toml @@ -11,7 +11,6 @@ repository = "https://github.com/awslabs/smithy-rs" [features] client-hyper = ["aws-smithy-client/client-hyper"] rustls = ["aws-smithy-client/rustls"] -native-tls = ["aws-smithy-client/native-tls"] rt-tokio = ["aws-smithy-async/rt-tokio", "tokio/rt"] default = ["client-hyper", "rustls", "rt-tokio"] diff --git a/aws/rust-runtime/aws-config/src/connector.rs b/aws/rust-runtime/aws-config/src/connector.rs index e023695363..0c0a4ee1c4 100644 --- a/aws/rust-runtime/aws-config/src/connector.rs +++ b/aws/rust-runtime/aws-config/src/connector.rs @@ -13,10 +13,10 @@ use std::sync::Arc; // unused when all crate features are disabled /// Unwrap an [`Option`](aws_smithy_client::erase::DynConnector), and panic with a helpful error message if it's `None` pub(crate) fn expect_connector(connector: Option) -> DynConnector { - connector.expect("No HTTP connector was available. Enable the `rustls` or `native-tls` crate feature or set a connector to fix this.") + connector.expect("No HTTP connector was available. Enable the `rustls` crate feature or set a connector to fix this.") } -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] fn base( settings: &ConnectorSettings, sleep: Option>, @@ -40,17 +40,7 @@ pub fn default_connector( } /// Given `ConnectorSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. -#[cfg(all(not(feature = "rustls"), feature = "native-tls"))] -pub fn default_connector( - settings: &ConnectorSettings, - sleep: Option>, -) -> Option { - let hyper = base(settings, sleep).build(aws_smithy_client::conns::native_tls()); - Some(DynConnector::new(hyper)) -} - -/// Given `ConnectorSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. -#[cfg(not(any(feature = "rustls", feature = "native-tls")))] +#[cfg(not(feature = "rustls"))] pub fn default_connector( _settings: &ConnectorSettings, _sleep: Option>, diff --git a/aws/rust-runtime/aws-config/src/default_provider/credentials.rs b/aws/rust-runtime/aws-config/src/default_provider/credentials.rs index 70b5725d0b..622e4cbd8e 100644 --- a/aws/rust-runtime/aws-config/src/default_provider/credentials.rs +++ b/aws/rust-runtime/aws-config/src/default_provider/credentials.rs @@ -14,7 +14,7 @@ use crate::meta::credentials::CredentialsProviderChain; use crate::meta::region::ProvideRegion; use crate::provider_config::ProviderConfig; -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] /// Default Credentials Provider chain /// /// The region from the default region provider will be used @@ -170,8 +170,8 @@ impl Builder { /// Creates a `DefaultCredentialsChain` /// /// ## Panics - /// This function will panic if no connector has been set and neither `rustls` and `native-tls` - /// features have both been disabled. + /// This function will panic if no connector has been set or the `rustls` + /// feature has been disabled. pub async fn build(self) -> DefaultCredentialsChain { let region = match self.region_override { Some(provider) => provider.region().await, diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index e791ec929a..5e7ca23b0a 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -929,7 +929,7 @@ pub(crate) mod test { /// Verify that the end-to-end real client has a 1-second connect timeout #[tokio::test] - #[cfg(any(feature = "rustls", feature = "native-tls"))] + #[cfg(feature = "rustls")] async fn one_second_connect_timeout() { use crate::imds::client::ImdsError; use aws_smithy_types::error::display::DisplayErrorContext; diff --git a/aws/rust-runtime/aws-config/src/imds/credentials.rs b/aws/rust-runtime/aws-config/src/imds/credentials.rs index 8cd3df7d0e..8ef30364f5 100644 --- a/aws/rust-runtime/aws-config/src/imds/credentials.rs +++ b/aws/rust-runtime/aws-config/src/imds/credentials.rs @@ -390,7 +390,7 @@ mod test { } #[tokio::test] - #[cfg(any(feature = "rustls", feature = "native-tls"))] + #[cfg(feature = "rustls")] async fn read_timeout_during_credentials_refresh_should_yield_last_retrieved_credentials() { let client = crate::imds::Client::builder() // 240.* can never be resolved @@ -409,7 +409,7 @@ mod test { } #[tokio::test] - #[cfg(any(feature = "rustls", feature = "native-tls"))] + #[cfg(feature = "rustls")] async fn read_timeout_during_credentials_refresh_should_error_without_last_retrieved_credentials( ) { let client = crate::imds::Client::builder() @@ -430,7 +430,7 @@ mod test { } #[tokio::test] - #[cfg(any(feature = "rustls", feature = "native-tls"))] + #[cfg(feature = "rustls")] async fn external_timeout_during_credentials_refresh_should_yield_last_retrieved_credentials() { use aws_sdk_sso::config::AsyncSleep; let client = crate::imds::Client::builder() diff --git a/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs b/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs index 02ae424c25..7ce91eaab7 100644 --- a/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs +++ b/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs @@ -60,7 +60,7 @@ impl CredentialsProviderChain { } /// Add a fallback to the default provider chain - #[cfg(any(feature = "rustls", feature = "native-tls"))] + #[cfg(feature = "rustls")] pub async fn or_default_provider(self) -> Self { self.or_else( "DefaultProviderChain", @@ -69,7 +69,7 @@ impl CredentialsProviderChain { } /// Creates a credential provider chain that starts with the default provider - #[cfg(any(feature = "rustls", feature = "native-tls"))] + #[cfg(feature = "rustls")] pub async fn default_provider() -> Self { Self::first_try( "DefaultProviderChain", diff --git a/aws/rust-runtime/aws-config/src/profile/credentials.rs b/aws/rust-runtime/aws-config/src/profile/credentials.rs index 9ce085503b..efc563722c 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials.rs @@ -97,7 +97,7 @@ impl ProvideCredentials for ProfileFileCredentialsProvider { /// future::ProvideCredentials::new(self.load_credentials()) /// } /// } -/// # if cfg!(any(feature = "rustls", feature = "native-tls")) { +/// # if cfg!(feature = "rustls") { /// let provider = ProfileFileCredentialsProvider::builder() /// .with_custom_provider("Custom", MyCustomProvider) /// .build(); @@ -362,7 +362,7 @@ impl Builder { /// } /// } /// - /// # if cfg!(any(feature = "rustls", feature = "native-tls")) { + /// # if cfg!(feature = "rustls") { /// let provider = ProfileFileCredentialsProvider::builder() /// .with_custom_provider("Custom", MyCustomProvider) /// .build(); diff --git a/aws/rust-runtime/aws-config/src/provider_config.rs b/aws/rust-runtime/aws-config/src/provider_config.rs index 9ca283d25f..4d7fd27402 100644 --- a/aws/rust-runtime/aws-config/src/provider_config.rs +++ b/aws/rust-runtime/aws-config/src/provider_config.rs @@ -121,7 +121,7 @@ impl ProviderConfig { /// /// # Examples /// ```no_run - /// # #[cfg(any(feature = "rustls", feature = "native-tls"))] + /// # #[cfg(feature = "rustls")] /// # fn example() { /// use aws_config::provider_config::ProviderConfig; /// use aws_sdk_sts::Region; diff --git a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs index 29db9af0fa..3aefdf5093 100644 --- a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs +++ b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs @@ -104,7 +104,7 @@ fn test_operation() -> Operation builder.connector(c), None =>{ - ##[cfg(any(feature = "rustls", feature = "native-tls"))] + ##[cfg(feature = "rustls")] { // Use default connector based on enabled features builder.dyn_https_connector(#{ConnectorSettings}::from_timeout_config(&timeout_config)) } - ##[cfg(not(any(feature = "rustls", feature = "native-tls")))] + ##[cfg(not(feature = "rustls"))] { - panic!("No HTTP connector was available. Enable the `rustls` or `native-tls` crate feature or set a connector to fix this."); + panic!("No HTTP connector was available. Enable the `rustls` crate feature or set a connector to fix this."); } } }; diff --git a/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/Cargo.toml b/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/Cargo.toml deleted file mode 100644 index 3642d7ba24..0000000000 --- a/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/Cargo.toml +++ /dev/null @@ -1,20 +0,0 @@ -[package] -name = "using-native-tls-instead-of-rustls" -version = "0.1.0" -authors = ["AWS Rust SDK Team "] -edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dev-dependencies] -# aws-config pulls in rustls and several other things by default. We have to disable defaults in order to use native-tls -# and then manually bring the other defaults back -aws-config = { path = "../../build/aws-sdk/sdk/aws-config", default-features = false, features = [ - "native-tls", - "rt-tokio", -] } -# aws-sdk-s3 brings in rustls by default so we disable that in order to use native-tls only -aws-sdk-s3 = { path = "../../build/aws-sdk/sdk/s3", default-features = false, features = [ - "native-tls", -] } -tokio = { version = "1.20.1", features = ["rt", "macros"] } diff --git a/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs b/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs deleted file mode 100644 index dddeebc479..0000000000 --- a/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -/// The SDK defaults to using RusTLS by default but you can also use [`native_tls`](https://github.com/sfackler/rust-native-tls) -/// which will choose a TLS implementation appropriate for your platform. This test looks much like -/// any other. Activating and deactivating `features` in your app's `Cargo.toml` is all that's needed. - -async fn list_buckets() -> Result<(), aws_sdk_s3::Error> { - let sdk_config = aws_config::load_from_env().await; - let client = aws_sdk_s3::Client::new(&sdk_config); - - let _resp = client.list_buckets().send().await?; - - Ok(()) -} - -/// You can run this test to ensure that it is only using `native-tls` and -/// that nothing is pulling in `rustls` as a dependency -#[test] -#[should_panic = "error: package ID specification `rustls` did not match any packages"] -fn test_rustls_is_not_in_dependency_tree() { - let cargo_location = std::env::var("CARGO").unwrap(); - let cargo_command = std::process::Command::new(&cargo_location) - .arg("tree") - .arg("--invert") - .arg("rustls") - .output() - .expect("failed to run 'cargo tree'"); - - let stderr = String::from_utf8_lossy(&cargo_command.stderr); - - // We expect the call to `cargo tree` to error out. If it did, we panic with the resulting - // message here. In the case that no error message is set, that's bad. - if !stderr.is_empty() { - panic!("{}", stderr); - } - - // Uh oh. We expected an error message but got none, likely because `cargo tree` found - // `rustls` in our dependencies. We'll print out the message we got to see what went wrong. - let stdout = String::from_utf8_lossy(&cargo_command.stdout); - - println!("{}", stdout) -} - -// NOTE: not currently run in CI, separate PR will set up a with-creds CI runner -#[tokio::test] -#[ignore] -async fn needs_creds_native_tls_works() { - list_buckets().await.expect("should succeed") -} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt index 475c9c490a..ba4125d9f0 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt @@ -40,7 +40,6 @@ class FluentClientDecorator : ClientCodegenDecorator { customizations = listOf(GenericFluentClient(codegenContext)), ).render(rustCrate) rustCrate.mergeFeature(Feature("rustls", default = true, listOf("aws-smithy-client/rustls"))) - rustCrate.mergeFeature(Feature("native-tls", default = false, listOf("aws-smithy-client/native-tls"))) } override fun libRsCustomizations( diff --git a/design/src/transport/connector.md b/design/src/transport/connector.md new file mode 100644 index 0000000000..9988e55b9e --- /dev/null +++ b/design/src/transport/connector.md @@ -0,0 +1,42 @@ +The Smithy client provides a default TLS connector, but a custom one can be plugged in. +`rustls` is enabled with the feature flag `rustls`. + +The client had previously supported `native-tls`. You can use your custom connector like this. + +Create your connector: + +```rust +/// A `hyper` connector that uses the `native-tls` crate for TLS. To use this in a smithy client, +/// wrap it in a [hyper_ext::Adapter](crate::hyper_ext::Adapter). +pub type NativeTls = hyper_tls::HttpsConnector; + +pub fn native_tls() -> NativeTls { + use hyper_rustls::HttpsConnector; + use tokio_rustls::TlsConnector; + // `TlsConnector` actually comes for here: https://docs.rs/native-tls/latest/native_tls/ + let mut tls = hyper_tls::native_tls::TlsConnector::builder(); + let tls = tls + .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12)) + .build() + .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e)); + let mut http = hyper::client::HttpConnector::new(); + http.enforce_http(false); + hyper_tls::HttpsConnector::from((http, tls.into())) +} +``` + +Plug the connector in the client: +```rust +let mut builder = hyper::client::Builder::default(); +builder.pool_max_idle_per_host(70); +let connector = aws_smithy_client::erase::DynConnector::new( + aws_smithy_client::hyper_ext::Adapter::builder() + .hyper_builder(builder) + .connector_settings(std::default::Default::default()) + .build(native_tls()), +); +let raw_client = aws_smithy_client::builder::Builder::new() + .connector(connector) + .middleware_fn(...) + .build_dyn(); +``` diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml index d8f8041bbd..3e4868735b 100644 --- a/rust-runtime/aws-smithy-client/Cargo.toml +++ b/rust-runtime/aws-smithy-client/Cargo.toml @@ -10,7 +10,6 @@ repository = "https://github.com/awslabs/smithy-rs" [features] rt-tokio = ["aws-smithy-async/rt-tokio"] test-util = ["aws-smithy-protocol-test", "serde/derive", "rustls"] -native-tls = ["client-hyper", "hyper-tls", "rt-tokio"] rustls = ["client-hyper", "hyper-rustls", "rt-tokio", "lazy_static"] client-hyper = ["hyper"] hyper-webpki-doctest-only = ["hyper-rustls/webpki-roots"] diff --git a/rust-runtime/aws-smithy-client/external-types.toml b/rust-runtime/aws-smithy-client/external-types.toml index fd2d76368f..306eebe119 100644 --- a/rust-runtime/aws-smithy-client/external-types.toml +++ b/rust-runtime/aws-smithy-client/external-types.toml @@ -10,7 +10,7 @@ allowed_external_types = [ "tower::retry::policy::Policy", "tower_service::Service", - # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Move `rustls`/`native-tls` features into separate crates + # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Move `rustls` feature into separate crates "hyper::client::connect::http::HttpConnector", "hyper_rustls::connector::HttpsConnector", "hyper_tls::client::HttpsConnector", diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index 1fe4ba12eb..e99d948144 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -80,21 +80,21 @@ where } } -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] use crate::erase::DynConnector; -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] use crate::http_connector::ConnectorSettings; -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] use crate::hyper_ext::Adapter as HyperAdapter; /// Max idle connections is not standardized across SDKs. Java V1 and V2 use 50, and Go V2 uses 100. /// The number below was chosen arbitrarily between those two reference points, and should allow /// for 14 separate SDK clients in a Lambda where the max file handles is 1024. -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] const DEFAULT_MAX_IDLE_CONNECTIONS: usize = 70; /// Returns default HTTP client settings for hyper. -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] fn default_hyper_builder() -> hyper::client::Builder { let mut builder = hyper::client::Builder::default(); builder.pool_max_idle_per_host(DEFAULT_MAX_IDLE_CONNECTIONS); @@ -117,24 +117,7 @@ impl Builder<(), M, R> { } } -#[cfg(feature = "native-tls")] -impl Builder<(), M, R> { - /// Connect to the service over HTTPS using the native TLS library on your - /// platform using dynamic dispatch. - pub fn native_tls_connector( - self, - connector_settings: ConnectorSettings, - ) -> Builder { - self.connector(DynConnector::new( - HyperAdapter::builder() - .hyper_builder(default_hyper_builder()) - .connector_settings(connector_settings) - .build(crate::conns::native_tls()), - )) - } -} - -#[cfg(any(feature = "rustls", feature = "native-tls"))] +#[cfg(feature = "rustls")] impl Builder<(), M, R> { /// Create a Smithy client builder with an HTTPS connector and the [standard retry /// policy](crate::retry::Standard) over the default middleware implementation. @@ -143,16 +126,12 @@ impl Builder<(), M, R> { /// dynamic dispatch. This comes at a slight runtime performance cost. See /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use /// [`Builder::rustls_connector`] or [`Builder::native_tls_connector`] instead. + #[cfg(feature = "rustls")] pub fn dyn_https_connector( self, connector_settings: ConnectorSettings, ) -> Builder { - #[cfg(feature = "rustls")] let with_https = |b: Builder<_, M, R>| b.rustls_connector(connector_settings); - // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled - #[cfg(not(feature = "rustls"))] - let with_https = |b: Builder<_, M, R>| b.native_tls_connector(connector_settings); - with_https(self) } } @@ -537,8 +516,5 @@ mod tests { #[cfg(feature = "rustls")] let _builder: Builder = Builder::new().rustls_connector(Default::default()); - #[cfg(feature = "native-tls")] - let _builder: Builder = - Builder::new().native_tls_connector(Default::default()); } } diff --git a/rust-runtime/aws-smithy-client/src/conns.rs b/rust-runtime/aws-smithy-client/src/conns.rs index 1aac78cf46..6ec2dba51d 100644 --- a/rust-runtime/aws-smithy-client/src/conns.rs +++ b/rust-runtime/aws-smithy-client/src/conns.rs @@ -10,11 +10,6 @@ /// wrap it in a [hyper_ext::Adapter](crate::hyper_ext::Adapter). pub type Https = hyper_rustls::HttpsConnector; -#[cfg(feature = "native-tls")] -/// A `hyper` connector that uses the `native-tls` crate for TLS. To use this in a smithy client, -/// wrap it in a [hyper_ext::Adapter](crate::hyper_ext::Adapter). -pub type NativeTls = hyper_tls::HttpsConnector; - #[cfg(feature = "rustls")] /// A smithy connector that uses the `rustls` crate for TLS. pub type Rustls = crate::hyper_ext::Adapter; @@ -42,25 +37,7 @@ pub fn https() -> Https { HTTPS_NATIVE_ROOTS.clone() } -#[cfg(feature = "native-tls")] -/// Return a default HTTPS connector backed by the `hyper_tls` crate. -/// -/// It requires a minimum TLS version of 1.2. -/// It allows you to connect to both `http` and `https` URLs. -pub fn native_tls() -> NativeTls { - // `TlsConnector` actually comes for here: https://docs.rs/native-tls/latest/native_tls/ - // hyper_tls just re-exports the crate for convenience. - let mut tls = hyper_tls::native_tls::TlsConnector::builder(); - let tls = tls - .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12)) - .build() - .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e)); - let mut http = hyper::client::HttpConnector::new(); - http.enforce_http(false); - hyper_tls::HttpsConnector::from((http, tls.into())) -} - -#[cfg(all(test, any(feature = "native-tls", feature = "rustls")))] +#[cfg(all(test, feature = "rustls"))] mod tests { use crate::erase::DynConnector; use crate::hyper_ext::Adapter; @@ -79,11 +56,26 @@ mod tests { assert!(res.status().is_success()); } - #[cfg(feature = "native-tls")] - mod native_tls_tests { + #[cfg(not(feature = "rustls"))] + mod custom_tls_tests { use super::super::native_tls; use super::*; + type NativeTls = hyper_tls::HttpsConnector; + + fn native_tls() -> NativeTls { + use hyper_rustls::HttpsConnector; + use tokio_rustls::TlsConnector; + let mut tls = hyper_tls::native_tls::TlsConnector::builder(); + let tls = tls + .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12)) + .build() + .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e)); + let mut http = hyper::client::HttpConnector::new(); + http.enforce_http(false); + hyper_tls::HttpsConnector::from((http, tls.into())) + } + #[tokio::test] async fn test_native_tls_connector_can_make_http_requests() { let conn = Adapter::builder().build(native_tls()); diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index 11a27c1d53..40860ca632 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -19,14 +19,14 @@ //! #![cfg_attr( not(all( - any(feature = "rustls", feature = "native-tls"), + feature = "rustls", feature = "client-hyper" )), doc = "```no_run,ignore" )] #![cfg_attr( all( - any(feature = "rustls", feature = "native-tls"), + feature = "rustls", feature = "client-hyper" ), doc = "```no_run" @@ -204,8 +204,8 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option /// Builder for [`hyper_ext::Adapter`](Adapter) /// /// Unlike a Smithy client, the [`tower::Service`] inside a [`hyper_ext::Adapter`](Adapter) is actually a service that -/// accepts a `Uri` and returns a TCP stream. Two default implementations of this are provided, one -/// that encrypts the stream with `rustls`, the other that encrypts the stream with `native-tls`. +/// accepts a `Uri` and returns a TCP stream. One default implementation of this is provided, +/// that encrypts the stream with `rustls`. /// /// # Examples /// Construct a HyperAdapter with the default HTTP implementation (rustls). This can be useful when you want to share a Hyper connector diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index 6e5e5ba9ee..05404db345 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -10,7 +10,6 @@ //! | `event-stream` | Provides Sender/Receiver implementations for Event Stream codegen. | //! | `rt-tokio` | Run async code with the `tokio` runtime | //! | `test-util` | Include various testing utils | -//! | `native-tls` | Use `native-tls` as the HTTP client's TLS implementation | //! | `rustls` | Use `rustls` as the HTTP client's TLS implementation | //! | `client-hyper` | Use `hyper` to handle HTTP requests | @@ -81,8 +80,8 @@ use tracing::{debug_span, field, field::display, Instrument}; /// to the inner service, and then ultimately returning the inner service's response. /// /// With the `hyper` feature enabled, you can construct a `Client` directly from a -/// [`hyper::Client`] using [`hyper_ext::Adapter::builder`]. You can also enable the `rustls` or `native-tls` -/// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls_connector`] and +/// [`hyper::Client`] using [`hyper_ext::Adapter::builder`]. You can also enable the `rustls` +/// feature to construct a Client against a standard HTTPS endpoint using [`Builder::rustls_connector`] and /// `Builder::native_tls_connector` respectively. #[derive(Debug)] pub struct Client< From 4b929fa9d8f9834190e255fbcaf206016566d0b4 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 5 May 2023 10:28:50 +0200 Subject: [PATCH 02/17] Remove conflicts Signed-off-by: Daniele Ahmed --- rust-runtime/aws-smithy-client/Cargo.toml | 22 ++- rust-runtime/aws-smithy-client/src/conns.rs | 2 +- .../aws-smithy-client/src/hyper_ext.rs | 168 +++++++++++------- rust-runtime/aws-smithy-client/src/lib.rs | 79 ++++---- 4 files changed, 162 insertions(+), 109 deletions(-) diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml index 3e4868735b..bb6c4575ca 100644 --- a/rust-runtime/aws-smithy-client/Cargo.toml +++ b/rust-runtime/aws-smithy-client/Cargo.toml @@ -9,10 +9,11 @@ repository = "https://github.com/awslabs/smithy-rs" [features] rt-tokio = ["aws-smithy-async/rt-tokio"] -test-util = ["aws-smithy-protocol-test", "serde/derive", "rustls"] -rustls = ["client-hyper", "hyper-rustls", "rt-tokio", "lazy_static"] -client-hyper = ["hyper"] -hyper-webpki-doctest-only = ["hyper-rustls/webpki-roots"] +test-util = ["dep:aws-smithy-protocol-test", "dep:hyper", "hyper?/server", "hyper?/h2", "dep:serde", "dep:serde_json", "serde?/derive", "rustls", "tokio/full"] +rustls = ["dep:hyper-rustls", "dep:lazy_static", "dep:rustls", "client-hyper", "rt-tokio"] +client-hyper = ["dep:hyper"] +hyper-webpki-doctest-only = ["dep:hyper-rustls", "hyper-rustls?/webpki-roots"] + [dependencies] aws-smithy-async = { path = "../aws-smithy-async" } @@ -24,16 +25,18 @@ bytes = "1" fastrand = "1.4.0" http = "0.2.3" http-body = "0.4.4" -hyper = { version = "0.14.12", features = ["client", "http2", "http1", "tcp"], optional = true } +hyper = { version = "0.14.25", features = ["client", "http2", "http1", "tcp"], optional = true } # cargo does not support optional test dependencies, so to completely disable rustls when -# the native-tls feature is enabled, we need to add the webpki-roots feature here. +# feature is enabled, we need to add the webpki-roots feature here. # https://github.com/rust-lang/cargo/issues/1596 hyper-rustls = { version = "0.23.0", optional = true, features = ["rustls-native-certs", "http2"] } hyper-tls = { version = "0.5.0", optional = true } +rustls = { version = "0.20", optional = true } lazy_static = { version = "1", optional = true } pin-project-lite = "0.2.7" serde = { version = "1", features = ["derive"], optional = true } -tokio = { version = "1.8.4" } +serde_json = { version = "1", optional = true } +tokio = { version = "1.13.1" } tower = { version = "0.4.6", features = ["util", "retry"] } tracing = "0.1" @@ -41,8 +44,11 @@ tracing = "0.1" aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio"] } serde = { version = "1", features = ["derive"] } serde_json = "1" -tokio = { version = "1.8.4", features = ["full", "test-util"] } +tokio = { version = "1.23.1", features = ["full", "test-util"] } tower-test = "0.4.0" +tracing-subscriber = "0.3.16" +tracing-test = "0.2.4" + [package.metadata.docs.rs] all-features = true diff --git a/rust-runtime/aws-smithy-client/src/conns.rs b/rust-runtime/aws-smithy-client/src/conns.rs index 6ec2dba51d..1e64bcde12 100644 --- a/rust-runtime/aws-smithy-client/src/conns.rs +++ b/rust-runtime/aws-smithy-client/src/conns.rs @@ -58,7 +58,7 @@ mod tests { #[cfg(not(feature = "rustls"))] mod custom_tls_tests { - use super::super::native_tls; + use super::native_tls; use super::*; type NativeTls = hyper_tls::HttpsConnector; diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index 40860ca632..5e4214ab33 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -18,18 +18,18 @@ //! [`Client`](crate::Client), directly, use the `dyn_https_https()` method to match that default behavior: //! #![cfg_attr( - not(all( - feature = "rustls", - feature = "client-hyper" - )), - doc = "```no_run,ignore" +not(all( +feature = "rustls", +feature = "client-hyper" +)), +doc = "```no_run,ignore" )] #![cfg_attr( - all( - feature = "rustls", - feature = "client-hyper" - ), - doc = "```no_run" +all( +feature = "rustls", +feature = "client-hyper" +), +doc = "```no_run" )] //! use aws_smithy_client::Client; //! @@ -48,16 +48,16 @@ //! that aren't otherwise exposed by the `Client` builder interface. //! #![cfg_attr( - not(all(feature = "rustls", feature = "client-hyper")), - doc = "```no_run,ignore" +not(all(feature = "rustls", feature = "client-hyper")), +doc = "```no_run,ignore" )] #![cfg_attr( - all( - feature = "rustls", - feature = "client-hyper", - feature = "hyper-webpki-doctest-only" - ), - doc = "```no_run" +all( +feature = "rustls", +feature = "client-hyper", +feature = "hyper-webpki-doctest-only" +), +doc = "```no_run" )] //! use std::time::Duration; //! use aws_smithy_client::{Client, conns, hyper_ext}; @@ -92,13 +92,22 @@ use crate::never::stream::EmptyStream; use aws_smithy_async::future::timeout::TimedOutError; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; + use aws_smithy_http::result::ConnectorError; use aws_smithy_types::error::display::DisplayErrorContext; use aws_smithy_types::retry::ErrorKind; -use http::Uri; -use hyper::client::connect::{Connected, Connection}; +use http::{Extensions, Uri}; +use hyper::client::connect::{ + capture_connection, CaptureConnection, Connected, Connection, HttpInfo, +}; + use std::error::Error; +use std::fmt::Debug; + use std::sync::Arc; + +use crate::erase::boxclone::BoxFuture; +use aws_smithy_http::connection::{CaptureSmithyConnection, ConnectionMetadata}; use tokio::io::{AsyncRead, AsyncWrite}; use tower::{BoxError, Service}; @@ -107,34 +116,58 @@ use tower::{BoxError, Service}; /// This adapter also enables TCP `CONNECT` and HTTP `READ` timeouts via [`Adapter::builder`]. For examples /// see [the module documentation](crate::hyper_ext). #[derive(Clone, Debug)] -#[non_exhaustive] -pub struct Adapter(HttpReadTimeout, SdkBody>>); +pub struct Adapter { + client: HttpReadTimeout, SdkBody>>, +} + +/// Extract a smithy connection from a hyper CaptureConnection +fn extract_smithy_connection(capture_conn: &CaptureConnection) -> Option { + let capture_conn = capture_conn.clone(); + if let Some(conn) = capture_conn.clone().connection_metadata().as_ref() { + let mut extensions = Extensions::new(); + conn.get_extras(&mut extensions); + let http_info = extensions.get::(); + let smithy_connection = ConnectionMetadata::new( + conn.is_proxied(), + http_info.map(|info| info.remote_addr()), + move || match capture_conn.connection_metadata().as_ref() { + Some(conn) => conn.poison(), + None => tracing::trace!("no connection existed to poison"), + }, + ); + Some(smithy_connection) + } else { + None + } +} impl Service> for Adapter -where - C: Clone + Send + Sync + 'static, - C: tower::Service, - C::Response: Connection + AsyncRead + AsyncWrite + Send + Unpin + 'static, - C::Future: Unpin + Send + 'static, - C::Error: Into, + where + C: Clone + Send + Sync + 'static, + C: Service, + C::Response: Connection + AsyncRead + AsyncWrite + Send + Unpin + 'static, + C::Future: Unpin + Send + 'static, + C::Error: Into, { type Response = http::Response; type Error = ConnectorError; - #[allow(clippy::type_complexity)] - type Future = std::pin::Pin< - Box> + Send + 'static>, - >; + type Future = BoxFuture; fn poll_ready( &mut self, cx: &mut std::task::Context<'_>, ) -> std::task::Poll> { - self.0.poll_ready(cx).map_err(downcast_error) + self.client.poll_ready(cx).map_err(downcast_error) } - fn call(&mut self, req: http::Request) -> Self::Future { - let fut = self.0.call(req); + fn call(&mut self, mut req: http::Request) -> Self::Future { + let capture_connection = capture_connection(&mut req); + if let Some(capture_smithy_connection) = req.extensions().get::() { + capture_smithy_connection + .set_connection_retriever(move || extract_smithy_connection(&capture_connection)); + } + let fut = self.client.call(req); Box::pin(async move { Ok(fut.await.map_err(downcast_error)?.map(SdkBody::from)) }) } } @@ -203,17 +236,17 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option /// Builder for [`hyper_ext::Adapter`](Adapter) /// -/// Unlike a Smithy client, the [`tower::Service`] inside a [`hyper_ext::Adapter`](Adapter) is actually a service that +/// Unlike a Smithy client, the [`Service`] inside a [`hyper_ext::Adapter`](Adapter) is actually a service that /// accepts a `Uri` and returns a TCP stream. One default implementation of this is provided, -/// that encrypts the stream with `rustls`. +/// that encrypts the stream with `rustls` /// /// # Examples /// Construct a HyperAdapter with the default HTTP implementation (rustls). This can be useful when you want to share a Hyper connector /// between multiple Smithy clients. /// #[cfg_attr( - not(all(feature = "rustls", feature = "client-hyper")), - doc = "```no_run,ignore" +not(all(feature = "rustls", feature = "client-hyper")), +doc = "```no_run,ignore" )] #[cfg_attr(all(feature = "rustls", feature = "client-hyper"), doc = "```no_run")] /// use tower::layer::util::Identity; @@ -235,12 +268,12 @@ pub struct Builder { impl Builder { /// Create a HyperAdapter from this builder and a given connector pub fn build(self, connector: C) -> Adapter - where - C: Clone + Send + Sync + 'static, - C: tower::Service, - C::Response: Connection + AsyncRead + AsyncWrite + Send + Unpin + 'static, - C::Future: Unpin + Send + 'static, - C::Error: Into, + where + C: Clone + Send + Sync + 'static, + C: Service, + C::Response: Connection + AsyncRead + AsyncWrite + Send + Unpin + 'static, + C::Future: Unpin + Send + 'static, + C::Error: Into, { let client_builder = self.client_builder.unwrap_or_default(); let sleep_impl = self.sleep_impl.or_else(default_async_sleep); @@ -271,13 +304,15 @@ impl Builder { ), None => HttpReadTimeout::no_timeout(base), }; - Adapter(read_timeout) + Adapter { + client: read_timeout, + } } /// Set the async sleep implementation used for timeouts /// /// Calling this is only necessary for testing or to use something other than - /// [`aws_smithy_async::rt::sleep::default_async_sleep`]. + /// [`default_async_sleep`]. pub fn sleep_impl(mut self, sleep_impl: Arc) -> Self { self.sleep_impl = Some(sleep_impl); self @@ -286,7 +321,7 @@ impl Builder { /// Set the async sleep implementation used for timeouts /// /// Calling this is only necessary for testing or to use something other than - /// [`aws_smithy_async::rt::sleep::default_async_sleep`]. + /// [`default_async_sleep`]. pub fn set_sleep_impl( &mut self, sleep_impl: Option>, @@ -343,7 +378,6 @@ mod timeout_middleware { use pin_project_lite::pin_project; use tower::BoxError; - use aws_smithy_async::future; use aws_smithy_async::future::timeout::{TimedOutError, Timeout}; use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_async::rt::sleep::Sleep; @@ -387,14 +421,14 @@ mod timeout_middleware { /// Create a new `ConnectTimeout` around `inner`. /// /// Typically, `I` will implement [`hyper::client::connect::Connect`]. - pub fn new(inner: I, sleep: Arc, timeout: Duration) -> Self { + pub(crate) fn new(inner: I, sleep: Arc, timeout: Duration) -> Self { Self { inner, timeout: Some((sleep, timeout)), } } - pub fn no_timeout(inner: I) -> Self { + pub(crate) fn no_timeout(inner: I) -> Self { Self { inner, timeout: None, @@ -403,7 +437,7 @@ mod timeout_middleware { } #[derive(Clone, Debug)] - pub struct HttpReadTimeout { + pub(crate) struct HttpReadTimeout { inner: I, timeout: Option<(Arc, Duration)>, } @@ -412,14 +446,14 @@ mod timeout_middleware { /// Create a new `HttpReadTimeout` around `inner`. /// /// Typically, `I` will implement [`tower::Service>`]. - pub fn new(inner: I, sleep: Arc, timeout: Duration) -> Self { + pub(crate) fn new(inner: I, sleep: Arc, timeout: Duration) -> Self { Self { inner, timeout: Some((sleep, timeout)), } } - pub fn no_timeout(inner: I) -> Self { + pub(crate) fn no_timeout(inner: I) -> Self { Self { inner, timeout: None, @@ -448,9 +482,9 @@ mod timeout_middleware { } impl Future for MaybeTimeoutFuture - where - F: Future>, - E: Into, + where + F: Future>, + E: Into, { type Output = Result; @@ -476,9 +510,9 @@ mod timeout_middleware { } impl tower::Service for ConnectTimeout - where - I: tower::Service, - I::Error: Into, + where + I: tower::Service, + I::Error: Into, { type Response = I::Response; type Error = BoxError; @@ -493,7 +527,7 @@ mod timeout_middleware { Some((sleep, duration)) => { let sleep = sleep.sleep(*duration); MaybeTimeoutFuture::Timeout { - timeout: future::timeout::Timeout::new(self.inner.call(req), sleep), + timeout: Timeout::new(self.inner.call(req), sleep), error_type: "HTTP connect", duration: *duration, } @@ -506,8 +540,8 @@ mod timeout_middleware { } impl tower::Service> for HttpReadTimeout - where - I: tower::Service, Error = hyper::Error>, + where + I: tower::Service, Error = hyper::Error>, { type Response = I::Response; type Error = BoxError; @@ -522,7 +556,7 @@ mod timeout_middleware { Some((sleep, duration)) => { let sleep = sleep.sleep(*duration); MaybeTimeoutFuture::Timeout { - timeout: future::timeout::Timeout::new(self.inner.call(req), sleep), + timeout: Timeout::new(self.inner.call(req), sleep), error_type: "HTTP read", duration: *duration, } @@ -688,7 +722,7 @@ mod test { _cx: &mut Context<'_>, _buf: &mut ReadBuf<'_>, ) -> Poll> { - Poll::Ready(Err(std::io::Error::new( + Poll::Ready(Err(Error::new( ErrorKind::ConnectionReset, "connection reset", ))) @@ -719,8 +753,8 @@ mod test { } impl tower::Service for TestConnection - where - T: Clone + hyper::client::connect::Connection, + where + T: Clone + Connection, { type Response = T; type Error = BoxError; diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index 05404db345..da7ad5e064 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -15,16 +15,17 @@ #![allow(clippy::derive_partial_eq_without_eq)] #![warn( - missing_debug_implementations, - missing_docs, - rustdoc::all, - rust_2018_idioms +missing_docs, +rustdoc::missing_crate_level_docs, +unreachable_pub, +rust_2018_idioms )] pub mod bounds; pub mod erase; pub mod http_connector; pub mod never; +mod poison; pub mod retry; pub mod timeout; @@ -49,20 +50,23 @@ pub mod hyper_ext; #[doc(hidden)] pub mod static_tests; +use crate::poison::PoisonLayer; use aws_smithy_async::rt::sleep::AsyncSleep; + use aws_smithy_http::operation::Operation; use aws_smithy_http::response::ParseHttpResponse; pub use aws_smithy_http::result::{SdkError, SdkSuccess}; +use aws_smithy_http::retry::ClassifyRetry; use aws_smithy_http_tower::dispatch::DispatchLayer; use aws_smithy_http_tower::parse_response::ParseResponseLayer; use aws_smithy_types::error::display::DisplayErrorContext; -use aws_smithy_types::retry::ProvideErrorKind; +use aws_smithy_types::retry::{ProvideErrorKind, ReconnectMode}; use aws_smithy_types::timeout::OperationTimeoutConfig; use std::sync::Arc; use timeout::ClientTimeoutParams; pub use timeout::TimeoutLayer; use tower::{Service, ServiceBuilder, ServiceExt}; -use tracing::{debug_span, field, field::display, Instrument}; +use tracing::{debug_span, field, Instrument}; /// Smithy service client. /// @@ -80,9 +84,8 @@ use tracing::{debug_span, field, field::display, Instrument}; /// to the inner service, and then ultimately returning the inner service's response. /// /// With the `hyper` feature enabled, you can construct a `Client` directly from a -/// [`hyper::Client`] using [`hyper_ext::Adapter::builder`]. You can also enable the `rustls` -/// feature to construct a Client against a standard HTTPS endpoint using [`Builder::rustls_connector`] and -/// `Builder::native_tls_connector` respectively. +/// `hyper::Client` using `hyper_ext::Adapter::builder`. You can also enable the `rustls` +/// feature to construct a Client against a standard HTTPS endpoint using `Builder::rustls_connector`. #[derive(Debug)] pub struct Client< Connector = erase::DynConnector, @@ -92,6 +95,7 @@ pub struct Client< connector: Connector, middleware: Middleware, retry_policy: RetryPolicy, + reconnect_mode: ReconnectMode, operation_timeout_config: OperationTimeoutConfig, sleep_impl: Option>, } @@ -105,8 +109,8 @@ impl Client<(), (), ()> { // Quick-create for people who just want "the default". impl Client -where - M: Default, + where + M: Default, { /// Create a Smithy client from the given `connector`, a middleware default, the /// [standard retry policy](retry::Standard), and the @@ -124,22 +128,23 @@ fn check_send_sync(t: T) -> T { } impl Client -where - C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware, - R: retry::NewRequestPolicy, + where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware, + R: retry::NewRequestPolicy, { /// Dispatch this request to the network /// /// For ergonomics, this does not include the raw response for successful responses. To /// access the raw response use `call_raw`. pub async fn call(&self, op: Operation) -> Result> - where - O: Send + Sync, - E: std::error::Error + Send + Sync + 'static, - Retry: Send + Sync, - R::Policy: bounds::SmithyRetryPolicy, - bounds::Parsed<>::Service, O, Retry>: + where + O: Send + Sync, + E: std::error::Error + Send + Sync + 'static, + Retry: Send + Sync, + R::Policy: bounds::SmithyRetryPolicy, + Retry: ClassifyRetry, SdkError>, + bounds::Parsed<>::Service, O, Retry>: Service, Response = SdkSuccess, Error = SdkError> + Clone, { self.call_raw(op).await.map(|res| res.parsed) @@ -153,18 +158,19 @@ where &self, op: Operation, ) -> Result, SdkError> - where - O: Send + Sync, - E: std::error::Error + Send + Sync + 'static, - Retry: Send + Sync, - R::Policy: bounds::SmithyRetryPolicy, + where + O: Send + Sync, + E: std::error::Error + Send + Sync + 'static, + Retry: Send + Sync, + R::Policy: bounds::SmithyRetryPolicy, + Retry: ClassifyRetry, SdkError>, // This bound is not _technically_ inferred by all the previous bounds, but in practice it // is because _we_ know that there is only implementation of Service for Parsed // (ParsedResponseService), and it will apply as long as the bounds on C, M, and R hold, // and will produce (as expected) Response = SdkSuccess, Error = SdkError. But Rust // doesn't know that -- there _could_ theoretically be other implementations of Service for // Parsed that don't return those same types. So, we must give the bound. - bounds::Parsed<>::Service, O, Retry>: + bounds::Parsed<>::Service, O, Retry>: Service, Response = SdkSuccess, Error = SdkError> + Clone, { let connector = self.connector.clone(); @@ -178,6 +184,7 @@ where self.retry_policy .new_request_policy(self.sleep_impl.clone()), ) + .layer(PoisonLayer::new(self.reconnect_mode)) .layer(TimeoutLayer::new(timeout_params.operation_attempt_timeout)) .layer(ParseResponseLayer::::new()) // These layers can be considered as occurring in order. That is, first invoke the @@ -197,8 +204,13 @@ where ); let (mut req, parts) = op.into_request_response(); if let Some(metadata) = &parts.metadata { - span.record("operation", &metadata.name()); - span.record("service", &metadata.service()); + // Clippy has a bug related to needless borrows so we need to allow them here + // https://github.com/rust-lang/rust-clippy/issues/9782 + #[allow(clippy::needless_borrow)] + { + span.record("operation", &metadata.name()); + span.record("service", &metadata.service()); + } // This will clone two `Cow::<&'static str>::Borrow`s in the vast majority of cases req.properties_mut().insert(metadata.clone()); } @@ -207,6 +219,7 @@ where let result = async move { check_send_sync(svc).ready().await?.call(op).await } .instrument(span.clone()) .await; + #[allow(clippy::needless_borrow)] match &result { Ok(_) => { span.record("status", &"ok"); @@ -223,7 +236,7 @@ where _ => "error", }, ) - .record("message", &display(DisplayErrorContext(err))); + .record("message", &field::display(DisplayErrorContext(err))); } } result @@ -236,15 +249,15 @@ where /// ensure (statically) that all the various constructors actually produce "useful" types. #[doc(hidden)] pub fn check(&self) - where - R::Policy: tower::retry::Policy< + where + R::Policy: tower::retry::Policy< static_tests::ValidTestOperation, SdkSuccess<()>, SdkError, > + Clone, { let _ = |o: static_tests::ValidTestOperation| { - let _ = self.call_raw(o); + drop(self.call_raw(o)); }; } } From 4447ba7d1e639367b69494c9f8771435872afdbf Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 5 May 2023 10:30:18 +0200 Subject: [PATCH 03/17] Remove native_tls Signed-off-by: Daniele Ahmed --- rust-runtime/aws-smithy-client/src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index e99d948144..5b331e6af7 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -125,7 +125,7 @@ impl Builder<(), M, R> { /// For convenience, this constructor type-erases the concrete TLS connector backend used using /// dynamic dispatch. This comes at a slight runtime performance cost. See /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use - /// [`Builder::rustls_connector`] or [`Builder::native_tls_connector`] instead. + /// [`Builder::rustls_connector`] instead. #[cfg(feature = "rustls")] pub fn dyn_https_connector( self, From 9b3976692c715d8d538adc9c1041f58bc898e7fb Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 5 May 2023 11:12:20 +0200 Subject: [PATCH 04/17] update tests Signed-off-by: Daniele Ahmed --- design/src/transport/connector.md | 3 - rust-runtime/aws-smithy-client/Cargo.toml | 6 +- rust-runtime/aws-smithy-client/src/conns.rs | 90 ++++++++++++--------- 3 files changed, 54 insertions(+), 45 deletions(-) diff --git a/design/src/transport/connector.md b/design/src/transport/connector.md index 9988e55b9e..cadbcb6dd5 100644 --- a/design/src/transport/connector.md +++ b/design/src/transport/connector.md @@ -11,9 +11,6 @@ Create your connector: pub type NativeTls = hyper_tls::HttpsConnector; pub fn native_tls() -> NativeTls { - use hyper_rustls::HttpsConnector; - use tokio_rustls::TlsConnector; - // `TlsConnector` actually comes for here: https://docs.rs/native-tls/latest/native_tls/ let mut tls = hyper_tls::native_tls::TlsConnector::builder(); let tls = tls .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12)) diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml index bb6c4575ca..4146aa151f 100644 --- a/rust-runtime/aws-smithy-client/Cargo.toml +++ b/rust-runtime/aws-smithy-client/Cargo.toml @@ -14,7 +14,6 @@ rustls = ["dep:hyper-rustls", "dep:lazy_static", "dep:rustls", "client-hyper", " client-hyper = ["dep:hyper"] hyper-webpki-doctest-only = ["dep:hyper-rustls", "hyper-rustls?/webpki-roots"] - [dependencies] aws-smithy-async = { path = "../aws-smithy-async" } aws-smithy-http = { path = "../aws-smithy-http" } @@ -26,8 +25,8 @@ fastrand = "1.4.0" http = "0.2.3" http-body = "0.4.4" hyper = { version = "0.14.25", features = ["client", "http2", "http1", "tcp"], optional = true } -# cargo does not support optional test dependencies, so to completely disable rustls when -# feature is enabled, we need to add the webpki-roots feature here. +# cargo does not support optional test dependencies, so to completely disable rustls +# we need to add the webpki-roots feature here. # https://github.com/rust-lang/cargo/issues/1596 hyper-rustls = { version = "0.23.0", optional = true, features = ["rustls-native-certs", "http2"] } hyper-tls = { version = "0.5.0", optional = true } @@ -42,6 +41,7 @@ tracing = "0.1" [dev-dependencies] aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio"] } +hyper-tls = { version = "0.5.0" } serde = { version = "1", features = ["derive"] } serde_json = "1" tokio = { version = "1.23.1", features = ["full", "test-util"] } diff --git a/rust-runtime/aws-smithy-client/src/conns.rs b/rust-runtime/aws-smithy-client/src/conns.rs index 2e68c30d11..0b0f43ccfa 100644 --- a/rust-runtime/aws-smithy-client/src/conns.rs +++ b/rust-runtime/aws-smithy-client/src/conns.rs @@ -77,45 +77,6 @@ mod tests { assert!(res.status().is_success()); } - #[cfg(not(feature = "rustls"))] - mod custom_tls_tests { - use super::native_tls; - use super::*; - - type NativeTls = hyper_tls::HttpsConnector; - - fn native_tls() -> NativeTls { - use hyper_rustls::HttpsConnector; - use tokio_rustls::TlsConnector; - let mut tls = hyper_tls::native_tls::TlsConnector::builder(); - let tls = tls - .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12)) - .build() - .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e)); - let mut http = hyper::client::HttpConnector::new(); - http.enforce_http(false); - hyper_tls::HttpsConnector::from((http, tls.into())) - } - - #[tokio::test] - async fn test_native_tls_connector_can_make_http_requests() { - let conn = Adapter::builder().build(native_tls()); - let conn = DynConnector::new(conn); - let http_uri: Uri = "http://example.com/".parse().unwrap(); - - send_request_and_assert_success(conn, &http_uri).await; - } - - #[tokio::test] - async fn test_native_tls_connector_can_make_https_requests() { - let conn = Adapter::builder().build(native_tls()); - let conn = DynConnector::new(conn); - let https_uri: Uri = "https://example.com/".parse().unwrap(); - - send_request_and_assert_success(conn, &https_uri).await; - } - } - #[cfg(feature = "rustls")] mod rustls_tests { use super::super::https; @@ -140,3 +101,54 @@ mod tests { } } } + +#[cfg(test)] +mod custom_tls_tests { + use crate::hyper_ext::Adapter; + use crate::erase::DynConnector; + use aws_smithy_http::body::SdkBody; + use http::{Method, Request, Uri}; + use tower::{Service, ServiceBuilder}; + + type NativeTls = hyper_tls::HttpsConnector; + + fn native_tls() -> NativeTls { + let mut tls = hyper_tls::native_tls::TlsConnector::builder(); + let tls = tls + .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12)) + .build() + .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e)); + let mut http = hyper::client::HttpConnector::new(); + http.enforce_http(false); + hyper_tls::HttpsConnector::from((http, tls.into())) + } + + #[tokio::test] + async fn test_native_tls_connector_can_make_http_requests() { + let conn = Adapter::builder().build(native_tls()); + let conn = DynConnector::new(conn); + let http_uri: Uri = "http://example.com/".parse().unwrap(); + + send_request_and_assert_success(conn, &http_uri).await; + } + + #[tokio::test] + async fn test_native_tls_connector_can_make_https_requests() { + let conn = Adapter::builder().build(native_tls()); + let conn = DynConnector::new(conn); + let https_uri: Uri = "https://example.com/".parse().unwrap(); + + send_request_and_assert_success(conn, &https_uri).await; + } + + async fn send_request_and_assert_success(conn: DynConnector, uri: &Uri) { + let mut svc = ServiceBuilder::new().service(conn); + let req = Request::builder() + .uri(uri) + .method(Method::GET) + .body(SdkBody::empty()) + .unwrap(); + let res = svc.call(req).await.unwrap(); + assert!(res.status().is_success()); + } +} From 5ed99443d7e547023ff31e83717d575b515dd2cf Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 5 May 2023 11:26:50 +0200 Subject: [PATCH 05/17] cargo fmt Signed-off-by: Daniele Ahmed --- rust-runtime/aws-smithy-client/src/conns.rs | 2 +- .../aws-smithy-client/src/hyper_ext.rs | 51 ++++++++----------- rust-runtime/aws-smithy-client/src/lib.rs | 18 +++---- 3 files changed, 31 insertions(+), 40 deletions(-) diff --git a/rust-runtime/aws-smithy-client/src/conns.rs b/rust-runtime/aws-smithy-client/src/conns.rs index 0b0f43ccfa..041ba85089 100644 --- a/rust-runtime/aws-smithy-client/src/conns.rs +++ b/rust-runtime/aws-smithy-client/src/conns.rs @@ -104,8 +104,8 @@ mod tests { #[cfg(test)] mod custom_tls_tests { - use crate::hyper_ext::Adapter; use crate::erase::DynConnector; + use crate::hyper_ext::Adapter; use aws_smithy_http::body::SdkBody; use http::{Method, Request, Uri}; use tower::{Service, ServiceBuilder}; diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index 3b7356dc31..daac7ecb92 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -18,19 +18,10 @@ //! [`Client`](crate::Client), directly, use the `dyn_https_https()` method to match that default behavior: //! #![cfg_attr( -not(all( -feature = "rustls", -feature = "client-hyper" -)), -doc = "```no_run,ignore" -)] -#![cfg_attr( -all( -feature = "rustls", -feature = "client-hyper" -), -doc = "```no_run" + not(all(feature = "rustls", feature = "client-hyper")), + doc = "```no_run,ignore" )] +#![cfg_attr(all(feature = "rustls", feature = "client-hyper"), doc = "```no_run")] //! use aws_smithy_client::Client; //! //! let client = Client::builder() @@ -48,16 +39,16 @@ doc = "```no_run" //! that aren't otherwise exposed by the `Client` builder interface. //! #![cfg_attr( -not(all(feature = "rustls", feature = "client-hyper")), -doc = "```no_run,ignore" + not(all(feature = "rustls", feature = "client-hyper")), + doc = "```no_run,ignore" )] #![cfg_attr( -all( -feature = "rustls", -feature = "client-hyper", -feature = "hyper-webpki-doctest-only" -), -doc = "```no_run" + all( + feature = "rustls", + feature = "client-hyper", + feature = "hyper-webpki-doctest-only" + ), + doc = "```no_run" )] //! use std::time::Duration; //! use aws_smithy_client::{Client, conns, hyper_ext}; @@ -245,8 +236,8 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option /// between multiple Smithy clients. /// #[cfg_attr( -not(all(feature = "rustls", feature = "client-hyper")), -doc = "```no_run,ignore" + not(all(feature = "rustls", feature = "client-hyper")), + doc = "```no_run,ignore" )] #[cfg_attr(all(feature = "rustls", feature = "client-hyper"), doc = "```no_run")] /// use tower::layer::util::Identity; @@ -482,9 +473,9 @@ mod timeout_middleware { } impl Future for MaybeTimeoutFuture - where - F: Future>, - E: Into, + where + F: Future>, + E: Into, { type Output = Result; @@ -510,9 +501,9 @@ mod timeout_middleware { } impl tower::Service for ConnectTimeout - where - I: tower::Service, - I::Error: Into, + where + I: tower::Service, + I::Error: Into, { type Response = I::Response; type Error = BoxError; @@ -540,8 +531,8 @@ mod timeout_middleware { } impl tower::Service> for HttpReadTimeout - where - I: tower::Service, Error = hyper::Error>, + where + I: tower::Service, Error = hyper::Error>, { type Response = I::Response; type Error = BoxError; diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index 18df7cfb33..7d3a6f256e 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -109,8 +109,8 @@ impl Client<(), (), ()> { // Quick-create for people who just want "the default". impl Client - where - M: Default, +where + M: Default, { /// Create a Smithy client from the given `connector`, a middleware default, the /// [standard retry policy](retry::Standard), and the @@ -128,10 +128,10 @@ fn check_send_sync(t: T) -> T { } impl Client - where - C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware, - R: retry::NewRequestPolicy, +where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware, + R: retry::NewRequestPolicy, { /// Dispatch this request to the network /// @@ -170,7 +170,7 @@ impl Client // and will produce (as expected) Response = SdkSuccess, Error = SdkError. But Rust // doesn't know that -- there _could_ theoretically be other implementations of Service for // Parsed that don't return those same types. So, we must give the bound. - bounds::Parsed<>::Service, O, Retry>: + bounds::Parsed<>::Service, O, Retry>: Service, Response = SdkSuccess, Error = SdkError> + Clone, { let connector = self.connector.clone(); @@ -249,8 +249,8 @@ impl Client /// ensure (statically) that all the various constructors actually produce "useful" types. #[doc(hidden)] pub fn check(&self) - where - R::Policy: tower::retry::Policy< + where + R::Policy: tower::retry::Policy< static_tests::ValidTestOperation, SdkSuccess<()>, SdkError, From 1922e24ec20f8023358029a0570cb82d1043917c Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 5 May 2023 13:39:38 +0200 Subject: [PATCH 06/17] remove using-native-tls-instead test Signed-off-by: Daniele Ahmed --- aws/sdk/integration-tests/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/aws/sdk/integration-tests/Cargo.toml b/aws/sdk/integration-tests/Cargo.toml index 7410b5f824..74a6f261dc 100644 --- a/aws/sdk/integration-tests/Cargo.toml +++ b/aws/sdk/integration-tests/Cargo.toml @@ -15,6 +15,5 @@ members = [ "s3control", "sts", "transcribestreaming", - "using-native-tls-instead-of-rustls", "webassembly", ] From d040b6a11fa1dbe416f05afa146c7f09fe66909d Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 5 May 2023 13:58:34 +0200 Subject: [PATCH 07/17] remove test folder Signed-off-by: Daniele Ahmed --- .../tests/no-rustls-in-dependency.rs | 52 ------------------- 1 file changed, 52 deletions(-) delete mode 100644 aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs diff --git a/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs b/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs deleted file mode 100644 index 1df97865db..0000000000 --- a/aws/sdk/integration-tests/using-native-tls-instead-of-rustls/tests/no-rustls-in-dependency.rs +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -/// The SDK defaults to using RusTLS by default but you can also use [`native_tls`](https://github.com/sfackler/rust-native-tls) -/// which will choose a TLS implementation appropriate for your platform. This test looks much like -/// any other. Activating and deactivating `features` in your app's `Cargo.toml` is all that's needed. - -async fn list_buckets() -> Result<(), aws_sdk_s3::Error> { - let sdk_config = aws_config::load_from_env().await; - let client = aws_sdk_s3::Client::new(&sdk_config); - - let _resp = client.list_buckets().send().await?; - - Ok(()) -} - -/// You can run this test to ensure that it is only using `native-tls` and -/// that nothing is pulling in `rustls` as a dependency -#[test] -#[should_panic = "error: package ID specification `rustls` did not match any packages"] -fn test_rustls_is_not_in_dependency_tree() { - let cargo_location = std::env::var("CARGO").unwrap(); - let cargo_command = std::process::Command::new(cargo_location) - .arg("tree") - .arg("--invert") - .arg("rustls") - .output() - .expect("failed to run 'cargo tree'"); - - let stderr = String::from_utf8_lossy(&cargo_command.stderr); - - // We expect the call to `cargo tree` to error out. If it did, we panic with the resulting - // message here. In the case that no error message is set, that's bad. - if !stderr.is_empty() { - panic!("{}", stderr); - } - - // Uh oh. We expected an error message but got none, likely because `cargo tree` found - // `rustls` in our dependencies. We'll print out the message we got to see what went wrong. - let stdout = String::from_utf8_lossy(&cargo_command.stdout); - - println!("{}", stdout) -} - -// NOTE: not currently run in CI, separate PR will set up a with-creds CI runner -#[tokio::test] -#[ignore] -async fn needs_creds_native_tls_works() { - list_buckets().await.expect("should succeed") -} From 0b6cd7426633b51e00d6e0bfe3b78aa9e90f8a3d Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Mon, 22 May 2023 11:51:49 +0200 Subject: [PATCH 08/17] address comments Signed-off-by: Daniele Ahmed --- aws/rust-runtime/aws-types/Cargo.toml | 4 ++-- rust-runtime/aws-smithy-client/src/hyper_ext.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/aws/rust-runtime/aws-types/Cargo.toml b/aws/rust-runtime/aws-types/Cargo.toml index 31d72bd1ec..7a4757b996 100644 --- a/aws/rust-runtime/aws-types/Cargo.toml +++ b/aws/rust-runtime/aws-types/Cargo.toml @@ -19,8 +19,8 @@ aws-smithy-client = { path = "../../../rust-runtime/aws-smithy-client" } aws-smithy-http = { path = "../../../rust-runtime/aws-smithy-http" } tracing = "0.1" http = "0.2.6" -# cargo does not support optional test dependencies, so to completely disable rustls when -# the native-tls feature is enabled, we need to add the webpki-roots feature here. +# cargo does not support optional test dependencies, so to completely disable rustls +# we need to add the webpki-roots feature here. # https://github.com/rust-lang/cargo/issues/1596 hyper-rustls = { version = "0.23.0", optional = true, features = ["rustls-native-certs", "http2", "webpki-roots"] } diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index daac7ecb92..8f0ca40609 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -229,7 +229,7 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option /// /// Unlike a Smithy client, the [`Service`] inside a [`hyper_ext::Adapter`](Adapter) is actually a service that /// accepts a `Uri` and returns a TCP stream. One default implementation of this is provided, -/// that encrypts the stream with `rustls` +/// that encrypts the stream with `rustls`. /// /// # Examples /// Construct a HyperAdapter with the default HTTP implementation (rustls). This can be useful when you want to share a Hyper connector From 9e0923bd00bfcf42cb6b943cc2cbbd8f14198439 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 23 May 2023 11:39:00 +0200 Subject: [PATCH 09/17] compiler error for native-tls Signed-off-by: Daniele Ahmed --- aws/rust-runtime/aws-config/Cargo.toml | 1 + rust-runtime/aws-smithy-client/Cargo.toml | 1 + rust-runtime/aws-smithy-client/src/builder.rs | 3 +++ 3 files changed, 5 insertions(+) diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index 7306872ea3..9fb7ff39d4 100644 --- a/aws/rust-runtime/aws-config/Cargo.toml +++ b/aws/rust-runtime/aws-config/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/awslabs/smithy-rs" [features] client-hyper = ["aws-smithy-client/client-hyper"] rustls = ["aws-smithy-client/rustls"] +native-tls = ["aws-smithy-client/native-tls"] rt-tokio = ["aws-smithy-async/rt-tokio", "tokio/rt"] credentials-sso = ["dep:aws-sdk-sso", "dep:ring", "dep:hex", "dep:zeroize"] diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml index 4146aa151f..ef17fc6c17 100644 --- a/rust-runtime/aws-smithy-client/Cargo.toml +++ b/rust-runtime/aws-smithy-client/Cargo.toml @@ -10,6 +10,7 @@ repository = "https://github.com/awslabs/smithy-rs" [features] rt-tokio = ["aws-smithy-async/rt-tokio"] test-util = ["dep:aws-smithy-protocol-test", "dep:hyper", "hyper?/server", "hyper?/h2", "dep:serde", "dep:serde_json", "serde?/derive", "rustls", "tokio/full"] +native-tls = [] rustls = ["dep:hyper-rustls", "dep:lazy_static", "dep:rustls", "client-hyper", "rt-tokio"] client-hyper = ["dep:hyper"] hyper-webpki-doctest-only = ["dep:hyper-rustls", "hyper-rustls?/webpki-roots"] diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index edb64008d6..ddd3e1402f 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -95,6 +95,9 @@ use crate::http_connector::ConnectorSettings; #[cfg(feature = "rustls")] use crate::hyper_ext::Adapter as HyperAdapter; +#[cfg(feature = "native-tls")] +compile_error!("Feature native-tls has been removed. For upgrade instructions, see: https://awslabs.github.io/smithy-rs/design/transport/connector.html"); + /// Max idle connections is not standardized across SDKs. Java V1 and V2 use 50, and Go V2 uses 100. /// The number below was chosen arbitrarily between those two reference points, and should allow /// for 14 separate SDK clients in a Lambda where the max file handles is 1024. From 12429540ce23cc73ff3484f9a831c707f49668a4 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 23 May 2023 11:39:57 +0200 Subject: [PATCH 10/17] add changelog entry Signed-off-by: Daniele Ahmed --- CHANGELOG.next.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 9f69d7e109..db62cd0667 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -23,3 +23,9 @@ message = "Implement `Ord` and `PartialOrd` for `DateTime`." author = "henriiik" references = ["smithy-rs#2653"] meta = { "breaking" = false, "tada" = false, "bug" = false } + +[[smithy-rs]] +message = "Remove native-tls and add a migration guide." +author = "82marbag" +references = ["smithy-rs#2675"] +meta = { "breaking" = true, "tada" = false, "bug" = false } From 562a32707b6dd5f52a09c64dc4e9009dbcd6435d Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 23 May 2023 11:41:43 +0200 Subject: [PATCH 11/17] add changelog entry Signed-off-by: Daniele Ahmed --- CHANGELOG.next.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 1424dd55bc..dd0ca199a3 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -35,6 +35,12 @@ references = ["smithy-rs#2696"] meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"} author = "jdisanti" +[[aws-sdk-rust]] +message = "Remove native-tls and add a migration guide." +author = "82marbag" +references = ["smithy-rs#2675"] +meta = { "breaking" = true, "tada" = false, "bug" = false } + [[smithy-rs]] message = "Remove native-tls and add a migration guide." author = "82marbag" From bd19d336187c68a83b3664eba84f648d5019fcb8 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 23 May 2023 16:05:23 +0200 Subject: [PATCH 12/17] update aws runtime feature Signed-off-by: Daniele Ahmed --- aws/rust-runtime/aws-config/Cargo.toml | 2 +- aws/rust-runtime/aws-config/src/connector.rs | 3 +++ tools/ci-scripts/check-aws-config | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index 9fb7ff39d4..473fe42173 100644 --- a/aws/rust-runtime/aws-config/Cargo.toml +++ b/aws/rust-runtime/aws-config/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/awslabs/smithy-rs" [features] client-hyper = ["aws-smithy-client/client-hyper"] rustls = ["aws-smithy-client/rustls"] -native-tls = ["aws-smithy-client/native-tls"] +native-tls = [] rt-tokio = ["aws-smithy-async/rt-tokio", "tokio/rt"] credentials-sso = ["dep:aws-sdk-sso", "dep:ring", "dep:hex", "dep:zeroize"] diff --git a/aws/rust-runtime/aws-config/src/connector.rs b/aws/rust-runtime/aws-config/src/connector.rs index edfcd1308b..fa5caf2f70 100644 --- a/aws/rust-runtime/aws-config/src/connector.rs +++ b/aws/rust-runtime/aws-config/src/connector.rs @@ -16,6 +16,9 @@ pub(crate) fn expect_connector(connector: Option) -> DynConnector connector.expect("No HTTP connector was available. Enable the `rustls` crate feature or set a connector to fix this.") } +#[cfg(feature = "native-tls")] +compile_error!("Feature native-tls has been removed. For upgrade instructions, see: https://awslabs.github.io/smithy-rs/design/transport/connector.html"); + #[cfg(feature = "rustls")] fn base( settings: &ConnectorSettings, diff --git a/tools/ci-scripts/check-aws-config b/tools/ci-scripts/check-aws-config index 60af5d28d2..5c6d910fe2 100755 --- a/tools/ci-scripts/check-aws-config +++ b/tools/ci-scripts/check-aws-config @@ -36,7 +36,7 @@ echo "${C_YELLOW}## Checking for duplicate dependency versions in the normal dep cargo tree -d --edges normal --all-features echo "${C_YELLOW}## Testing every combination of features${C_RESET}" -cargo hack test --feature-powerset --exclude-all-features +cargo hack test --feature-powerset --exclude-all-features --exclude-features native-tls echo "${C_YELLOW}## Checking the wasm32-unknown-unknown and wasm32-wasi targets${C_RESET}" cargo check --target wasm32-unknown-unknown --no-default-features From 5a482d65a69e1703acf191dc53dcc35c4df9aaf3 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 23 May 2023 16:42:23 +0200 Subject: [PATCH 13/17] update aws runtime feature Signed-off-by: Daniele Ahmed --- aws/rust-runtime/aws-config/Cargo.toml | 1 + aws/rust-runtime/aws-config/src/connector.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index 473fe42173..2a350f3ef2 100644 --- a/aws/rust-runtime/aws-config/Cargo.toml +++ b/aws/rust-runtime/aws-config/Cargo.toml @@ -12,6 +12,7 @@ repository = "https://github.com/awslabs/smithy-rs" client-hyper = ["aws-smithy-client/client-hyper"] rustls = ["aws-smithy-client/rustls"] native-tls = [] +allow-compilation = [] # our tests use `cargo test --all-features` and native-tls breaks CI rt-tokio = ["aws-smithy-async/rt-tokio", "tokio/rt"] credentials-sso = ["dep:aws-sdk-sso", "dep:ring", "dep:hex", "dep:zeroize"] diff --git a/aws/rust-runtime/aws-config/src/connector.rs b/aws/rust-runtime/aws-config/src/connector.rs index fa5caf2f70..82bda6e02e 100644 --- a/aws/rust-runtime/aws-config/src/connector.rs +++ b/aws/rust-runtime/aws-config/src/connector.rs @@ -16,7 +16,7 @@ pub(crate) fn expect_connector(connector: Option) -> DynConnector connector.expect("No HTTP connector was available. Enable the `rustls` crate feature or set a connector to fix this.") } -#[cfg(feature = "native-tls")] +#[cfg(all(feature = "native-tls", not(feature = "allow-compilation")))] compile_error!("Feature native-tls has been removed. For upgrade instructions, see: https://awslabs.github.io/smithy-rs/design/transport/connector.html"); #[cfg(feature = "rustls")] From cc112f9fb4f20b54a2c505e393d4691c02a645ef Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 23 May 2023 17:03:20 +0200 Subject: [PATCH 14/17] update runtime feature Signed-off-by: Daniele Ahmed --- rust-runtime/aws-smithy-client/Cargo.toml | 1 + rust-runtime/aws-smithy-client/src/builder.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml index ef17fc6c17..95d8e0c005 100644 --- a/rust-runtime/aws-smithy-client/Cargo.toml +++ b/rust-runtime/aws-smithy-client/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/awslabs/smithy-rs" rt-tokio = ["aws-smithy-async/rt-tokio"] test-util = ["dep:aws-smithy-protocol-test", "dep:hyper", "hyper?/server", "hyper?/h2", "dep:serde", "dep:serde_json", "serde?/derive", "rustls", "tokio/full"] native-tls = [] +allow-compilation = [] # our tests use `cargo test --all-features` and native-tls breaks CI rustls = ["dep:hyper-rustls", "dep:lazy_static", "dep:rustls", "client-hyper", "rt-tokio"] client-hyper = ["dep:hyper"] hyper-webpki-doctest-only = ["dep:hyper-rustls", "hyper-rustls?/webpki-roots"] diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index ddd3e1402f..063c9a6a47 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -95,7 +95,7 @@ use crate::http_connector::ConnectorSettings; #[cfg(feature = "rustls")] use crate::hyper_ext::Adapter as HyperAdapter; -#[cfg(feature = "native-tls")] +#[cfg(all(feature = "native-tls", not(feature = "allow-compilation")))] compile_error!("Feature native-tls has been removed. For upgrade instructions, see: https://awslabs.github.io/smithy-rs/design/transport/connector.html"); /// Max idle connections is not standardized across SDKs. Java V1 and V2 use 50, and Go V2 uses 100. From 35afd88989ada18b86f0b7909ac89da4e34c9481 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Thu, 25 May 2023 15:57:31 +0200 Subject: [PATCH 15/17] remove native-tls from workflow Signed-off-by: Daniele Ahmed --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb0d9e5207..f9dc88baf3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -221,17 +221,17 @@ jobs: test_aws_exclude: '' test_smithy_rs_exclude: --exclude aws-smithy-http-server-python --exclude aws-smithy-http-server-typescript - target: powerpc-unknown-linux-gnu - build_smithy_rs_features: --features native-tls + build_smithy_rs_features: '' build_aws_exclude: --exclude aws-inlineable build_smithy_rs_exclude: --exclude aws-smithy-http-server-python --exclude aws-smithy-http-server-typescript - test_smithy_rs_features: --features native-tls + test_smithy_rs_features: '' test_aws_exclude: --exclude aws-inlineable test_smithy_rs_exclude: --exclude aws-smithy-http-server-python --exclude aws-smithy-http-server-typescript - target: powerpc64-unknown-linux-gnu - build_smithy_rs_features: --features native-tls + build_smithy_rs_features: '' build_aws_exclude: --exclude aws-inlineable build_smithy_rs_exclude: --exclude aws-smithy-http-server-python --exclude aws-smithy-http-server-typescript - test_smithy_rs_features: --features native-tls + test_smithy_rs_features: '' test_aws_exclude: --exclude aws-inlineable test_smithy_rs_exclude: --exclude aws-smithy-http-server-python --exclude aws-smithy-http-server-typescript env: From 0c4b92f71edb845df7fd0dae184efe1ecb4ad139 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Thu, 25 May 2023 16:12:12 +0200 Subject: [PATCH 16/17] remove native-tls in ci Signed-off-by: Daniele Ahmed --- rust-runtime/aws-smithy-client/additional-ci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-client/additional-ci b/rust-runtime/aws-smithy-client/additional-ci index fde542e9fc..bb21b8b01f 100755 --- a/rust-runtime/aws-smithy-client/additional-ci +++ b/rust-runtime/aws-smithy-client/additional-ci @@ -12,4 +12,4 @@ echo "### Checking for duplicate dependency versions in the normal dependency gr cargo tree -d --edges normal --all-features echo "### Testing every combination of features (excluding --all-features)" -cargo hack test --feature-powerset --exclude-all-features +cargo hack test --feature-powerset --exclude-all-features --exclude-features native-tls From 810731027b6d419f6f01536cfa72d9a6131ec536 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 26 May 2023 11:23:23 +0200 Subject: [PATCH 17/17] remove last native-tls references Signed-off-by: Daniele Ahmed --- .github/workflows/ci.yml | 2 -- .../no-default-features/tests/client-construction.rs | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f9dc88baf3..a3ce1bd51c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,8 +211,6 @@ jobs: matrix: include: # We always exclude aws-smithy-http-server-python since the Python framework is experimental. - # We only build the `native-tls` feature here because `rustls` depends on `ring` which in turn - # does not support powerpc as a target platform (see https://github.com/briansmith/ring/issues/389) - target: i686-unknown-linux-gnu build_smithy_rs_features: --all-features build_aws_exclude: '' diff --git a/aws/sdk/integration-tests/no-default-features/tests/client-construction.rs b/aws/sdk/integration-tests/no-default-features/tests/client-construction.rs index 8f250f9322..64f0137b24 100644 --- a/aws/sdk/integration-tests/no-default-features/tests/client-construction.rs +++ b/aws/sdk/integration-tests/no-default-features/tests/client-construction.rs @@ -6,7 +6,7 @@ // This will fail due to lack of a connector when constructing the SDK Config #[tokio::test] #[should_panic( - expected = "No HTTP connector was available. Enable the `rustls` or `native-tls` crate feature or set a connector to fix this." + expected = "No HTTP connector was available. Enable the `rustls` crate feature or set a connector to fix this." )] async fn test_clients_from_sdk_config() { aws_config::load_from_env().await; @@ -15,7 +15,7 @@ async fn test_clients_from_sdk_config() { // This will fail due to lack of a connector when constructing the service client #[tokio::test] #[should_panic( - expected = "No HTTP connector was available. Enable the `rustls` or `native-tls` crate feature or set a connector to fix this." + expected = "No HTTP connector was available. Enable the `rustls` crate feature or set a connector to fix this." )] async fn test_clients_from_service_config() { #[derive(Clone, Debug)]