diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 27abec50c7..fb9213af12 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -11,6 +11,18 @@ # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} # author = "rcoh" +[[aws-sdk-rust]] +message = "Loading native TLS trusted certs for the default HTTP client now only occurs if the default HTTP client is not overridden in config." +references = ["smithy-rs#3262"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "jdisanti" + +[[smithy-rs]] +message = "Loading native TLS trusted certs for the default HTTP client now only occurs if the default HTTP client is not overridden in config." +references = ["smithy-rs#3262"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" } +author = "jdisanti" + [[aws-sdk-rust]] message = """Client creation now takes microseconds instead of milliseconds. Previously, it would take 2-3 milliseconds for each client instantiation due to time spent compiling regexes. diff --git a/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs b/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs index 9e4808118c..40a4fb0578 100644 --- a/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs +++ b/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs @@ -6,6 +6,8 @@ use aws_credential_types::provider::SharedCredentialsProvider; use aws_credential_types::Credentials; use aws_smithy_async::rt::sleep::{SharedAsyncSleep, TokioSleep}; +use aws_smithy_runtime::client::http::test_util::NeverClient; +use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs; use aws_smithy_runtime_api::client::result::SdkError; use aws_smithy_types::timeout::TimeoutConfig; use aws_types::region::Region; @@ -13,9 +15,10 @@ use aws_types::SdkConfig; use std::time::Duration; use tokio::time::Instant; -/// Use a 5 second operation timeout on SdkConfig and a 0ms connect timeout on the service config +/// Use a 5 second operation timeout on SdkConfig and a 0ms operation timeout on the service config #[tokio::test] async fn timeouts_can_be_set_by_service() { + let (_guard, _) = capture_test_logs(); let sdk_config = SdkConfig::builder() .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests())) .region(Region::from_static("us-east-1")) @@ -25,6 +28,7 @@ async fn timeouts_can_be_set_by_service() { .operation_timeout(Duration::from_secs(5)) .build(), ) + .http_client(NeverClient::new()) // ip that .endpoint_url( // Emulate a connect timeout error by hitting an unroutable IP @@ -34,7 +38,7 @@ async fn timeouts_can_be_set_by_service() { let config = aws_sdk_s3::config::Builder::from(&sdk_config) .timeout_config( TimeoutConfig::builder() - .connect_timeout(Duration::from_secs(0)) + .operation_timeout(Duration::from_secs(0)) .build(), ) .build(); @@ -48,8 +52,8 @@ async fn timeouts_can_be_set_by_service() { .await .expect_err("unroutable IP should timeout"); match err { - SdkError::DispatchFailure(err) => assert!(err.is_timeout()), - // if the connect timeout is not respected, this times out after 1 second because of the operation timeout with `SdkError::Timeout` + SdkError::TimeoutError(_err) => { /* ok */ } + // if the connect timeout is not respected, this times out after 5 seconds because of the operation timeout with `SdkError::Timeout` _other => panic!("unexpected error: {:?}", _other), } // there should be a 0ms timeout, we gotta set some stuff up. Just want to make sure diff --git a/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md new file mode 100644 index 0000000000..4ac61d51b0 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md @@ -0,0 +1,9 @@ +Validate the base client configuration. + +This gets called upon client construction. The full config may not be available at +this time (hence why it has [`RuntimeComponentsBuilder`] as an argument rather +than [`RuntimeComponents`]). Any error returned here will become a panic +in the client constructor. + +[`RuntimeComponentsBuilder`]: crate::client::runtime_components::RuntimeComponentsBuilder +[`RuntimeComponents`]: crate::client::runtime_components::RuntimeComponents diff --git a/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md new file mode 100644 index 0000000000..768bda5dd5 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md @@ -0,0 +1,7 @@ +Validate the final client configuration. + +This gets called immediately after the [`Intercept::read_before_execution`] trait hook +when the final configuration has been resolved. Any error returned here will +cause the operation to return that error. + +[`Intercept::read_before_execution`]: crate::client::interceptors::Intercept::read_before_execution diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/http.rs b/rust-runtime/aws-smithy-runtime-api/src/client/http.rs index 1e90d40fb1..aa66f79699 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/http.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/http.rs @@ -50,11 +50,13 @@ //! [`tower`]: https://crates.io/crates/tower //! [`aws-smithy-runtime`]: https://crates.io/crates/aws-smithy-runtime +use crate::box_error::BoxError; use crate::client::orchestrator::{HttpRequest, HttpResponse}; use crate::client::result::ConnectorError; use crate::client::runtime_components::sealed::ValidateConfig; -use crate::client::runtime_components::RuntimeComponents; +use crate::client::runtime_components::{RuntimeComponents, RuntimeComponentsBuilder}; use crate::impl_shared_conversions; +use aws_smithy_types::config_bag::ConfigBag; use std::fmt; use std::sync::Arc; use std::time::Duration; @@ -143,6 +145,26 @@ pub trait HttpClient: Send + Sync + fmt::Debug { settings: &HttpConnectorSettings, components: &RuntimeComponents, ) -> SharedHttpConnector; + + #[doc = include_str!("../../rustdoc/validate_base_client_config.md")] + fn validate_base_client_config( + &self, + runtime_components: &RuntimeComponentsBuilder, + cfg: &ConfigBag, + ) -> Result<(), BoxError> { + let _ = (runtime_components, cfg); + Ok(()) + } + + #[doc = include_str!("../../rustdoc/validate_final_config.md")] + fn validate_final_config( + &self, + runtime_components: &RuntimeComponents, + cfg: &ConfigBag, + ) -> Result<(), BoxError> { + let _ = (runtime_components, cfg); + Ok(()) + } } /// Shared HTTP client for use across multiple clients and requests. @@ -170,7 +192,24 @@ impl HttpClient for SharedHttpClient { } } -impl ValidateConfig for SharedHttpClient {} +impl ValidateConfig for SharedHttpClient { + fn validate_base_client_config( + &self, + runtime_components: &super::runtime_components::RuntimeComponentsBuilder, + cfg: &aws_smithy_types::config_bag::ConfigBag, + ) -> Result<(), crate::box_error::BoxError> { + self.selector + .validate_base_client_config(runtime_components, cfg) + } + + fn validate_final_config( + &self, + runtime_components: &RuntimeComponents, + cfg: &aws_smithy_types::config_bag::ConfigBag, + ) -> Result<(), crate::box_error::BoxError> { + self.selector.validate_final_config(runtime_components, cfg) + } +} impl_shared_conversions!(convert SharedHttpClient from HttpClient using SharedHttpClient::new); diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs index 669c4490ab..3632029e7b 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs @@ -64,12 +64,7 @@ pub trait ResolveCachedIdentity: fmt::Debug + Send + Sync { config_bag: &'a ConfigBag, ) -> IdentityFuture<'a>; - /// Validate the base client configuration for this implementation. - /// - /// This gets called upon client construction. The full config may not be available at - /// this time (hence why it has [`RuntimeComponentsBuilder`] as an argument rather - /// than [`RuntimeComponents`]). Any error returned here will become a panic - /// in the client constructor. + #[doc = include_str!("../../rustdoc/validate_base_client_config.md")] fn validate_base_client_config( &self, runtime_components: &RuntimeComponentsBuilder, @@ -79,13 +74,7 @@ pub trait ResolveCachedIdentity: fmt::Debug + Send + Sync { Ok(()) } - /// Validate the final client configuration for this implementation. - /// - /// This gets called immediately after the [`Intercept::read_before_execution`] trait hook - /// when the final configuration has been resolved. Any error returned here will - /// cause the operation to return that error. - /// - /// [`Intercept::read_before_execution`]: crate::client::interceptors::Intercept::read_before_execution + #[doc = include_str!("../../rustdoc/validate_final_config.md")] fn validate_final_config( &self, runtime_components: &RuntimeComponents, diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs index 0b0c58c321..19471ddf6b 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs @@ -44,12 +44,7 @@ pub(crate) mod sealed { /// This trait can be used to validate that certain required components or config values /// are available, and provide an error with helpful instructions if they are not. pub trait ValidateConfig: fmt::Debug + Send + Sync { - /// Validate the base client configuration. - /// - /// This gets called upon client construction. The full config may not be available at - /// this time (hence why it has [`RuntimeComponentsBuilder`] as an argument rather - /// than [`RuntimeComponents`]). Any error returned here will become a panic - /// in the client constructor. + #[doc = include_str!("../../rustdoc/validate_base_client_config.md")] fn validate_base_client_config( &self, runtime_components: &RuntimeComponentsBuilder, @@ -59,11 +54,7 @@ pub(crate) mod sealed { Ok(()) } - /// Validate the final client configuration. - /// - /// This gets called immediately after the [`Intercept::read_before_execution`] trait hook - /// when the final configuration has been resolved. Any error returned here will - /// cause the operation to return that error. + #[doc = include_str!("../../rustdoc/validate_final_config.md")] fn validate_final_config( &self, runtime_components: &RuntimeComponents, diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs index 344c60714a..1359beb4a3 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs @@ -15,9 +15,12 @@ use aws_smithy_runtime_api::client::http::{ }; use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; use aws_smithy_runtime_api::client::result::ConnectorError; -use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; +use aws_smithy_runtime_api::client::runtime_components::{ + RuntimeComponents, RuntimeComponentsBuilder, +}; use aws_smithy_runtime_api::shared::IntoShared; use aws_smithy_types::body::SdkBody; +use aws_smithy_types::config_bag::ConfigBag; use aws_smithy_types::error::display::DisplayErrorContext; use aws_smithy_types::retry::ErrorKind; use h2::Reason; @@ -36,38 +39,42 @@ use tokio::io::{AsyncRead, AsyncWrite}; mod default_connector { use aws_smithy_async::rt::sleep::SharedAsyncSleep; use aws_smithy_runtime_api::client::http::HttpConnectorSettings; + use hyper_0_14::client::HttpConnector; + use hyper_rustls::HttpsConnector; // Creating a `with_native_roots` HTTP client takes 300ms on OS X. Cache this so that we // don't need to repeatedly incur that cost. - static HTTPS_NATIVE_ROOTS: once_cell::sync::Lazy< + pub(crate) static HTTPS_NATIVE_ROOTS: once_cell::sync::Lazy< hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { + > = once_cell::sync::Lazy::new(default_tls); + + fn default_tls() -> HttpsConnector { use hyper_rustls::ConfigBuilderExt; hyper_rustls::HttpsConnectorBuilder::new() - .with_tls_config( - rustls::ClientConfig::builder() - .with_cipher_suites(&[ - // TLS1.3 suites - rustls::cipher_suite::TLS13_AES_256_GCM_SHA384, - rustls::cipher_suite::TLS13_AES_128_GCM_SHA256, - // TLS1.2 suites - rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - rustls::cipher_suite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - ]) - .with_safe_default_kx_groups() - .with_safe_default_protocol_versions() - .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") - .with_native_roots() - .with_no_client_auth() - ) - .https_or_http() - .enable_http1() - .enable_http2() - .build() - }); + .with_tls_config( + rustls::ClientConfig::builder() + .with_cipher_suites(&[ + // TLS1.3 suites + rustls::cipher_suite::TLS13_AES_256_GCM_SHA384, + rustls::cipher_suite::TLS13_AES_128_GCM_SHA256, + // TLS1.2 suites + rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + rustls::cipher_suite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + ]) + .with_safe_default_kx_groups() + .with_safe_default_protocol_versions() + .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") + .with_native_roots() + .with_no_client_auth() + ) + .https_or_http() + .enable_http1() + .enable_http2() + .build() + } pub(super) fn base( settings: &HttpConnectorSettings, @@ -474,6 +481,20 @@ where C::Future: Unpin + Send + 'static, C::Error: Into, { + fn validate_base_client_config( + &self, + _: &RuntimeComponentsBuilder, + _: &ConfigBag, + ) -> Result<(), BoxError> { + // Initialize the TCP connector at this point so that native certs load + // at client initialization time instead of upon first request. We do it + // here rather than at construction so that it won't run if this is not + // the selected HTTP client for the base config (for example, if this was + // the default HTTP client, and it was overridden by a later plugin). + let _ = (self.tcp_connector_fn)(); + Ok(()) + } + fn http_connector( &self, settings: &HttpConnectorSettings, @@ -490,7 +511,14 @@ where .connector_settings(settings.clone()); builder.set_sleep_impl(components.sleep_impl()); + let start = components.time_source().map(|ts| ts.now()); let tcp_connector = (self.tcp_connector_fn)(); + let end = components.time_source().map(|ts| ts.now()); + if let (Some(start), Some(end)) = (start, end) { + if let Ok(elapsed) = end.duration_since(start) { + tracing::debug!("new TCP connector created in {:?}", elapsed); + } + } let connector = SharedHttpConnector::new(builder.build(tcp_connector)); cache.insert(key.clone(), connector); } @@ -535,10 +563,13 @@ impl HyperClientBuilder { self } - /// Create a [`HyperConnector`] with the default rustls HTTPS implementation. + /// Create a hyper client with the default rustls HTTPS implementation. + /// + /// The trusted certificates will be loaded later when this becomes the selected + /// HTTP client for a Smithy client. #[cfg(feature = "tls-rustls")] pub fn build_https(self) -> SharedHttpClient { - self.build(default_connector::https()) + self.build_with_fn(default_connector::https) } /// Create a [`SharedHttpClient`] from this builder and a given connector. @@ -555,14 +586,9 @@ impl HyperClientBuilder { C::Future: Unpin + Send + 'static, C::Error: Into, { - SharedHttpClient::new(HyperClient { - connector_cache: RwLock::new(HashMap::new()), - client_builder: self.client_builder.unwrap_or_default(), - tcp_connector_fn: move || tcp_connector.clone(), - }) + self.build_with_fn(move || tcp_connector.clone()) } - #[cfg(all(test, feature = "test-util"))] fn build_with_fn(self, tcp_connector_fn: F) -> SharedHttpClient where F: Fn() -> C + Send + Sync + 'static, @@ -952,6 +978,7 @@ mod timeout_middleware { mod test { use super::*; use crate::client::http::test_util::NeverTcpConnector; + use aws_smithy_async::time::SystemTimeSource; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use http::Uri; use hyper_0_14::client::connect::{Connected, Connection}; @@ -993,7 +1020,10 @@ mod test { ]; // Kick off thousands of parallel tasks that will try to create a connector - let components = RuntimeComponentsBuilder::for_tests().build().unwrap(); + let components = RuntimeComponentsBuilder::for_tests() + .with_time_source(Some(SystemTimeSource::new())) + .build() + .unwrap(); let mut handles = Vec::new(); for setting in &settings { for _ in 0..1000 { diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs index d61608cbeb..128579e384 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs @@ -146,6 +146,7 @@ async fn never_tcp_connector_plugs_into_hyper_014() { use super::*; use crate::client::http::hyper_014::HyperClientBuilder; use aws_smithy_async::rt::sleep::TokioSleep; + use aws_smithy_async::time::SystemTimeSource; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use std::time::Duration; @@ -153,6 +154,7 @@ async fn never_tcp_connector_plugs_into_hyper_014() { let client = HyperClientBuilder::new().build(NeverTcpConnector::new()); let components = RuntimeComponentsBuilder::for_tests() .with_sleep_impl(Some(TokioSleep::new())) + .with_time_source(Some(SystemTimeSource::new())) .build() .unwrap(); let http_connector = client.http_connector(