From 31625f5bacce3d687b48f8de4323254b7e0dda8f Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Thu, 16 Nov 2023 13:33:42 -0500 Subject: [PATCH] Assorted cleanups of stable runtime crates (#3205) ## Motivation and Context Stable crates MUST only expose other stable crates. ## Description This: - fixes the remaining issues - adds a lint tool to be sure we don't expose unstable crates by accident in the future ## Testing CI run ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- CHANGELOG.next.toml | 20 +- .../aws-config/external-types.toml | 12 +- .../aws-config/src/imds/client.rs | 18 +- .../aws-config/src/imds/client/error.rs | 9 - .../aws-config/src/imds/credentials.rs | 9 +- aws/rust-runtime/aws-config/src/lib.rs | 1 - .../aws-config/src/sts/assume_role.rs | 9 +- .../aws-credential-types/Cargo.toml | 3 + aws/rust-runtime/aws-runtime/Cargo.toml | 3 + .../aws-runtime/external-types.toml | 8 - .../aws-runtime/src/request_info.rs | 8 +- aws/rust-runtime/aws-sigv4/Cargo.toml | 3 + .../aws-sigv4/external-types.toml | 2 +- .../aws-sigv4/src/http_request.rs | 2 +- .../aws-sigv4/src/http_request/sign.rs | 21 +- aws/sdk/integration-tests/s3/tests/no_auth.rs | 9 +- .../s3/tests/request_information_headers.rs | 3 +- .../timestreamquery/tests/endpoint_disco.rs | 4 +- aws/sdk/sdk-external-types.toml | 12 +- buildSrc/src/main/kotlin/CrateSet.kt | 77 +++++--- buildSrc/src/test/kotlin/CrateSetTest.kt | 20 +- rust-runtime/aws-smithy-runtime/Cargo.toml | 3 + .../aws-smithy-runtime/external-types.toml | 16 +- .../src/client/http/test_util/dvr.rs | 1 - .../src/client/http/test_util/dvr/replay.rs | 6 +- rust-runtime/aws-smithy-runtime/src/lib.rs | 1 + .../ci-build/sdk-lints/src/lint_cargo_toml.rs | 182 +++++++++++++++++- tools/ci-build/sdk-lints/src/main.rs | 7 +- 28 files changed, 333 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fc4c4c2578..a99b8c59a8 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -9,4 +9,22 @@ # message = "Fix typos in module documentation for generated crates" # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} -# author = "rcoh" \ No newline at end of file +# author = "rcoh" + +[[smithy-rs]] +message = "SignableRequest::apply_to_request in aws_sigv4 has been renamed `apply_to_request_http0x`" +references = ["smithy-rs#3205"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } +author = "rcoh" + +[[aws-sdk-rust]] +message = "imds::client::Builder::endpoint has been updated to accept a string instead of a URI. The method now returns a result instead." +references = ["smithy-rs#3205"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" + +[[aws-sdk-rust]] +message = "The `AssumeRoleBuilder::policy_arns` now accepts strings instead of an STS specific type" +references = ["smithy-rs#3205"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" diff --git a/aws/rust-runtime/aws-config/external-types.toml b/aws/rust-runtime/aws-config/external-types.toml index bd0ada2461..19c1f6aab8 100644 --- a/aws/rust-runtime/aws-config/external-types.toml +++ b/aws/rust-runtime/aws-config/external-types.toml @@ -7,13 +7,11 @@ allowed_external_types = [ "aws_credential_types::provider::ProvideCredentials", "aws_credential_types::provider::Result", "aws_credential_types::provider::SharedCredentialsProvider", - "aws_sdk_sts::types::_policy_descriptor_type::PolicyDescriptorType", "aws_smithy_async::rt::sleep::AsyncSleep", "aws_smithy_async::rt::sleep::SharedAsyncSleep", "aws_smithy_async::time::SharedTimeSource", "aws_smithy_async::time::TimeSource", - "aws_smithy_http::endpoint", - "aws_smithy_http::endpoint::error::InvalidEndpointError", + "aws_smithy_runtime_api::box_error::BoxError", "aws_smithy_runtime::client::identity::cache::IdentityCache", "aws_smithy_runtime::client::identity::cache::lazy::LazyCacheBuilder", "aws_smithy_runtime_api::client::dns::ResolveDns", @@ -33,12 +31,4 @@ allowed_external_types = [ "aws_smithy_types::timeout::TimeoutConfig", "aws_smithy_types::timeout::TimeoutConfigBuilder", "aws_types::*", - "http::response::Response", - "http::uri::Uri", - "tower_service::Service", - - # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Decide if the following should be exposed - "hyper::client::connect::Connection", - "tokio::io::async_read::AsyncRead", - "tokio::io::async_write::AsyncWrite", ] diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 1e4d7f02fa..154225b29e 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -15,6 +15,7 @@ use aws_http::user_agent::{ApiMetadata, AwsUserAgent}; use aws_runtime::user_agent::UserAgentInterceptor; use aws_smithy_runtime::client::orchestrator::operation::Operation; use aws_smithy_runtime::client::retries::strategy::StandardRetryStrategy; +use aws_smithy_runtime_api::box_error::BoxError; use aws_smithy_runtime_api::client::auth::AuthSchemeOptionResolverParams; use aws_smithy_runtime_api::client::endpoint::{ EndpointFuture, EndpointResolverParams, ResolveEndpoint, @@ -87,10 +88,9 @@ fn user_agent() -> AwsUserAgent { /// 1. Explicit configuration of `Endpoint` via the [builder](Builder): /// ```no_run /// use aws_config::imds::client::Client; -/// use http::Uri; /// # async fn docs() { /// let client = Client::builder() -/// .endpoint(Uri::from_static("http://customidms:456/")) +/// .endpoint("http://customidms:456/").expect("valid URI") /// .build(); /// # } /// ``` @@ -358,9 +358,10 @@ impl Builder { /// By default, the client will resolve an endpoint from the environment, AWS config, and endpoint mode. /// /// See [`Client`] for more information. - pub fn endpoint(mut self, endpoint: impl Into) -> Self { - self.endpoint = Some(EndpointSource::Explicit(endpoint.into())); - self + pub fn endpoint(mut self, endpoint: impl AsRef) -> Result { + let uri: Uri = endpoint.as_ref().parse()?; + self.endpoint = Some(EndpointSource::Explicit(uri)); + Ok(self) } /// Override the endpoint mode for [`Client`] @@ -992,7 +993,8 @@ pub(crate) mod test { let client = Client::builder() // 240.* can never be resolved - .endpoint(Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .expect("valid uri") .build(); let now = SystemTime::now(); let resp = client @@ -1057,7 +1059,9 @@ pub(crate) mod test { .with_http_client(http_client); let mut imds_client = Client::builder().configure(&provider_config); if let Some(endpoint_override) = test_case.endpoint_override { - imds_client = imds_client.endpoint(endpoint_override.parse::().unwrap()); + imds_client = imds_client + .endpoint(endpoint_override) + .expect("invalid URI"); } if let Some(mode_override) = test_case.mode_override { diff --git a/aws/rust-runtime/aws-config/src/imds/client/error.rs b/aws/rust-runtime/aws-config/src/imds/client/error.rs index 05412d75ea..7e418c28f8 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/error.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/error.rs @@ -5,7 +5,6 @@ //! Error types for [`ImdsClient`](crate::imds::client::Client) -use aws_smithy_http::endpoint::error::InvalidEndpointError; use aws_smithy_runtime_api::client::orchestrator::HttpResponse; use aws_smithy_runtime_api::client::result::SdkError; use std::error::Error; @@ -224,14 +223,6 @@ impl Error for BuildError { } } -impl From for BuildError { - fn from(err: InvalidEndpointError) -> Self { - Self { - kind: BuildErrorKind::InvalidEndpointUri(err.into()), - } - } -} - #[derive(Debug)] pub(super) enum TokenErrorKind { /// The token was invalid diff --git a/aws/rust-runtime/aws-config/src/imds/credentials.rs b/aws/rust-runtime/aws-config/src/imds/credentials.rs index 845a53b877..896e66d6df 100644 --- a/aws/rust-runtime/aws-config/src/imds/credentials.rs +++ b/aws/rust-runtime/aws-config/src/imds/credentials.rs @@ -421,7 +421,8 @@ mod test { async fn read_timeout_during_credentials_refresh_should_yield_last_retrieved_credentials() { let client = crate::imds::Client::builder() // 240.* can never be resolved - .endpoint(http::Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .unwrap() .build(); let expected = aws_credential_types::Credentials::for_tests(); let provider = ImdsCredentialsProvider::builder() @@ -439,7 +440,8 @@ mod test { ) { let client = crate::imds::Client::builder() // 240.* can never be resolved - .endpoint(http::Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .unwrap() .build(); let provider = ImdsCredentialsProvider::builder() .imds_client(client) @@ -458,7 +460,8 @@ mod test { use aws_smithy_async::rt::sleep::AsyncSleep; let client = crate::imds::Client::builder() // 240.* can never be resolved - .endpoint(http::Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .unwrap() .build(); let expected = aws_credential_types::Credentials::for_tests(); let provider = ImdsCredentialsProvider::builder() diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index 76fa855538..73d5106e9d 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -96,7 +96,6 @@ //! # } //! ``` -pub use aws_smithy_http::endpoint; pub use aws_smithy_runtime_api::client::behavior_version::BehaviorVersion; // Re-export types from aws-types pub use aws_types::{ diff --git a/aws/rust-runtime/aws-config/src/sts/assume_role.rs b/aws/rust-runtime/aws-config/src/sts/assume_role.rs index 6b827a8b7e..9605cb0e07 100644 --- a/aws/rust-runtime/aws-config/src/sts/assume_role.rs +++ b/aws/rust-runtime/aws-config/src/sts/assume_role.rs @@ -161,8 +161,13 @@ impl AssumeRoleProviderBuilder { /// This parameter is optional. /// For more information, see /// [policy_arns](aws_sdk_sts::operation::assume_role::builders::AssumeRoleInputBuilder::policy_arns) - pub fn policy_arns(mut self, policy_arns: Vec) -> Self { - self.policy_arns = Some(policy_arns); + pub fn policy_arns(mut self, policy_arns: Vec) -> Self { + self.policy_arns = Some( + policy_arns + .into_iter() + .map(|arn| PolicyDescriptorType::builder().arn(arn).build()) + .collect::>(), + ); self } diff --git a/aws/rust-runtime/aws-credential-types/Cargo.toml b/aws/rust-runtime/aws-credential-types/Cargo.toml index 33c1b2a91e..865542ca0e 100644 --- a/aws/rust-runtime/aws-credential-types/Cargo.toml +++ b/aws/rust-runtime/aws-credential-types/Cargo.toml @@ -26,3 +26,6 @@ all-features = true targets = ["x86_64-unknown-linux-gnu"] rustdoc-args = ["--cfg", "docsrs"] # End of docs.rs metadata + +[package.metadata.smithy-rs-release-tooling] +stable = true diff --git a/aws/rust-runtime/aws-runtime/Cargo.toml b/aws/rust-runtime/aws-runtime/Cargo.toml index b28a063881..b255e8b842 100644 --- a/aws/rust-runtime/aws-runtime/Cargo.toml +++ b/aws/rust-runtime/aws-runtime/Cargo.toml @@ -45,3 +45,6 @@ all-features = true targets = ["x86_64-unknown-linux-gnu"] rustdoc-args = ["--cfg", "docsrs"] # End of docs.rs metadata + +[package.metadata.smithy-rs-release-tooling] +stable = true diff --git a/aws/rust-runtime/aws-runtime/external-types.toml b/aws/rust-runtime/aws-runtime/external-types.toml index 0449ba09bb..941f09767b 100644 --- a/aws/rust-runtime/aws-runtime/external-types.toml +++ b/aws/rust-runtime/aws-runtime/external-types.toml @@ -1,14 +1,6 @@ allowed_external_types = [ - "aws_credential_types::*", "aws_sigv4::*", - "aws_smithy_http::*", "aws_smithy_types::*", "aws_smithy_runtime_api::*", "aws_types::*", - # TODO(audit-external-type-usage) We should newtype these or otherwise avoid exposing them - "http::header::name::HeaderName", - "http::header::value::HeaderValue", - "http::request::Request", - "http::response::Response", - "http::uri::Uri", ] diff --git a/aws/rust-runtime/aws-runtime/src/request_info.rs b/aws/rust-runtime/aws-runtime/src/request_info.rs index 1a2e066b49..b07c3479d7 100644 --- a/aws/rust-runtime/aws-runtime/src/request_info.rs +++ b/aws/rust-runtime/aws-runtime/src/request_info.rs @@ -129,19 +129,19 @@ impl Intercept for RequestInfoInterceptor { /// retry information header that is sent with every request. The information conveyed by this /// header allows services to anticipate whether a client will time out or retry a request. #[derive(Default, Debug)] -pub struct RequestPairs { +struct RequestPairs { inner: Vec<(Cow<'static, str>, Cow<'static, str>)>, } impl RequestPairs { /// Creates a new `RequestPairs` builder. - pub fn new() -> Self { + fn new() -> Self { Default::default() } /// Adds a pair to the `RequestPairs` builder. /// Only strings that can be converted to header values are considered valid. - pub fn with_pair( + fn with_pair( mut self, pair: (impl Into>, impl Into>), ) -> Self { @@ -151,7 +151,7 @@ impl RequestPairs { } /// Converts the `RequestPairs` builder into a `HeaderValue`. - pub fn try_into_header_value(self) -> Result { + fn try_into_header_value(self) -> Result { self.try_into() } } diff --git a/aws/rust-runtime/aws-sigv4/Cargo.toml b/aws/rust-runtime/aws-sigv4/Cargo.toml index ce91ae5209..8a428c2924 100644 --- a/aws/rust-runtime/aws-sigv4/Cargo.toml +++ b/aws/rust-runtime/aws-sigv4/Cargo.toml @@ -69,3 +69,6 @@ all-features = true targets = ["x86_64-unknown-linux-gnu"] rustdoc-args = ["--cfg", "docsrs"] # End of docs.rs metadata + +[package.metadata.smithy-rs-release-tooling] +stable = true diff --git a/aws/rust-runtime/aws-sigv4/external-types.toml b/aws/rust-runtime/aws-sigv4/external-types.toml index 6c80133e45..94701a5285 100644 --- a/aws/rust-runtime/aws-sigv4/external-types.toml +++ b/aws/rust-runtime/aws-sigv4/external-types.toml @@ -1,5 +1,5 @@ allowed_external_types = [ - # TODO(refactorHttp): Remove this and remove the signing helpers + # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `http0-compat` feature "http::request::Request", # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature "aws_smithy_types::event_stream::Message", diff --git a/aws/rust-runtime/aws-sigv4/src/http_request.rs b/aws/rust-runtime/aws-sigv4/src/http_request.rs index 5616509eaf..6bb6b9235c 100644 --- a/aws/rust-runtime/aws-sigv4/src/http_request.rs +++ b/aws/rust-runtime/aws-sigv4/src/http_request.rs @@ -49,7 +49,7 @@ //! let mut my_req = http::Request::new("..."); //! // Sign and then apply the signature to the request //! let (signing_instructions, _signature) = sign(signable_request, &signing_params)?.into_parts(); -//! signing_instructions.apply_to_request(&mut my_req); +//! signing_instructions.apply_to_request_http0x(&mut my_req); //! # Ok(()) //! # } //! ``` diff --git a/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs b/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs index 12d1e6cb52..2cef0dbd3c 100644 --- a/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs +++ b/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs @@ -162,7 +162,7 @@ impl SigningInstructions { #[cfg(any(feature = "http0-compat", test))] /// Applies the instructions to the given `request`. - pub fn apply_to_request(self, request: &mut http::Request) { + pub fn apply_to_request_http0x(self, request: &mut http::Request) { let (new_headers, new_query) = self.into_parts(); for header in new_headers.into_iter() { let mut value = http::HeaderValue::from_str(&header.value).unwrap(); @@ -492,7 +492,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = test::v4::test_signed_request("get-vanilla-query-order-key-case"); assert_req_eq!(expected, signed); @@ -547,7 +547,8 @@ mod tests { let out = sign(signable_req, ¶ms).unwrap(); // Sigv4a signatures are non-deterministic, so we can't compare the signature directly. - out.output.apply_to_request(&mut req.as_http_request()); + out.output + .apply_to_request_http0x(&mut req.as_http_request()); let creds = params.credentials().unwrap(); let signing_key = @@ -777,7 +778,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = test::v4::test_signed_request(test); assert_req_eq!(expected, signed); @@ -809,7 +810,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = test::v4::test_signed_request_query_params("get-vanilla-query-order-key-case"); @@ -843,7 +844,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = http::Request::builder() .uri("https://some-endpoint.some-region.amazonaws.com") @@ -904,7 +905,7 @@ mod tests { let mut signed = original.as_http_request(); out_with_session_token_but_excluded .output - .apply_to_request(&mut signed); + .apply_to_request_http0x(&mut signed); let expected = http::Request::builder() .uri("https://some-endpoint.some-region.amazonaws.com") @@ -961,7 +962,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = http::Request::builder() .uri("https://some-endpoint.some-region.amazonaws.com") @@ -1031,7 +1032,7 @@ mod tests { .body("") .unwrap(); - instructions.apply_to_request(&mut request); + instructions.apply_to_request_http0x(&mut request); let get_header = |n: &str| request.headers().get(n).unwrap().to_str().unwrap(); assert_eq!("foo", get_header("some-header")); @@ -1051,7 +1052,7 @@ mod tests { .body("") .unwrap(); - instructions.apply_to_request(&mut request); + instructions.apply_to_request_http0x(&mut request); assert_eq!( "/some/path?some-param=f%26o%3Fo&some-other-param%3F=bar", diff --git a/aws/sdk/integration-tests/s3/tests/no_auth.rs b/aws/sdk/integration-tests/s3/tests/no_auth.rs index f3029e4a1d..170332d00d 100644 --- a/aws/sdk/integration-tests/s3/tests/no_auth.rs +++ b/aws/sdk/integration-tests/s3/tests/no_auth.rs @@ -4,7 +4,6 @@ */ use aws_sdk_s3::{Client, Config}; -use aws_smithy_protocol_test::MediaType; use aws_smithy_runtime::client::http::test_util::dvr::ReplayingClient; use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs; @@ -34,7 +33,7 @@ async fn list_objects() { dbg!(result).expect("success"); http_client - .validate_body_and_headers(None, MediaType::Xml) + .validate_body_and_headers(None, "application/xml") .await .unwrap(); } @@ -66,7 +65,7 @@ async fn list_objects_v2() { dbg!(result).expect("success"); http_client - .validate_body_and_headers(None, MediaType::Xml) + .validate_body_and_headers(None, "application/xml") .await .unwrap(); } @@ -97,7 +96,7 @@ async fn head_object() { dbg!(result).expect("success"); http_client - .validate_body_and_headers(None, MediaType::Xml) + .validate_body_and_headers(None, "application/xml") .await .unwrap(); } @@ -128,7 +127,7 @@ async fn get_object() { dbg!(result).expect("success"); http_client - .validate_body_and_headers(None, MediaType::Xml) + .validate_body_and_headers(None, "application/xml") .await .unwrap(); } diff --git a/aws/sdk/integration-tests/s3/tests/request_information_headers.rs b/aws/sdk/integration-tests/s3/tests/request_information_headers.rs index c020309377..e12a4317a9 100644 --- a/aws/sdk/integration-tests/s3/tests/request_information_headers.rs +++ b/aws/sdk/integration-tests/s3/tests/request_information_headers.rs @@ -15,7 +15,6 @@ use aws_sdk_s3::Client; use aws_smithy_async::test_util::InstantSleep; use aws_smithy_async::test_util::ManualTimeSource; use aws_smithy_async::time::SharedTimeSource; -use aws_smithy_protocol_test::MediaType; use aws_smithy_runtime::client::http::test_util::dvr::ReplayingClient; use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs; use aws_smithy_runtime_api::box_error::BoxError; @@ -104,7 +103,7 @@ async fn three_retries_and_then_success() { let resp = resp.expect("valid e2e test"); assert_eq!(resp.name(), Some("test-bucket")); http_client - .full_validate(MediaType::Xml) + .full_validate("application/xml") .await .expect("failed") } diff --git a/aws/sdk/integration-tests/timestreamquery/tests/endpoint_disco.rs b/aws/sdk/integration-tests/timestreamquery/tests/endpoint_disco.rs index 42d5461971..c5a22c5d1b 100644 --- a/aws/sdk/integration-tests/timestreamquery/tests/endpoint_disco.rs +++ b/aws/sdk/integration-tests/timestreamquery/tests/endpoint_disco.rs @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_runtime::client::http::test_util::dvr::{MediaType, ReplayingClient}; +use aws_smithy_runtime::client::http::test_util::dvr::ReplayingClient; #[tokio::test] async fn do_endpoint_discovery() { @@ -75,7 +75,7 @@ async fn do_endpoint_discovery() { "content-type", "x-amz-target", ]), - MediaType::Json, + "application/json", ) .await .unwrap(); diff --git a/aws/sdk/sdk-external-types.toml b/aws/sdk/sdk-external-types.toml index a58f912dff..0fa9bbc3a2 100644 --- a/aws/sdk/sdk-external-types.toml +++ b/aws/sdk/sdk-external-types.toml @@ -13,19 +13,11 @@ allowed_external_types = [ # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature "aws_smithy_eventstream::*", - # Consider moving making this crate 1.0 "aws_smithy_runtime::client::identity::cache::IdentityCache", - ### All below this line need to be fixed: - # The following are not OK - "aws_http::request_id::RequestId", # move to aws-types - # `set_invocation_id_generator` — consider make pub(crate) or moving trait to AwsTypes "aws_runtime::invocation_id::SharedInvocationIdGenerator", "aws_runtime::invocation_id::InvocationIdGenerator", - # Pending fix in open PRs: - "aws_smithy_http::result::*", - - # Pending future fix - "aws_smithy_http::event_stream::*", + # Only exposed in transcribestreaming. This crate will be major version bumped if we MV aws_smithy_http + "aws_smithy_http::event_stream::sender::EventStreamSender", ] diff --git a/buildSrc/src/main/kotlin/CrateSet.kt b/buildSrc/src/main/kotlin/CrateSet.kt index d8a95838e6..72eb37d3fb 100644 --- a/buildSrc/src/main/kotlin/CrateSet.kt +++ b/buildSrc/src/main/kotlin/CrateSet.kt @@ -16,36 +16,59 @@ object CrateSet { * stable = true */ - val AWS_SDK_RUNTIME = listOf( - Crate("aws-config", STABLE_VERSION_PROP_NAME), - Crate("aws-credential-types", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-endpoint", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-http", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-hyper", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-runtime", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-runtime-api", STABLE_VERSION_PROP_NAME), - Crate("aws-sig-auth", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-sigv4", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-types", STABLE_VERSION_PROP_NAME), + val StableCrates = setOf( + // AWS crates + "aws-config", + "aws-credential-types", + "aws-runtime", + "aws-runtime-api", + "aws-sigv4", + "aws-types", + + // smithy crates + "aws-smithy-async", + "aws-smithy-runtime-api", + "aws-smithy-runtime", + "aws-smithy-types", ) + val version = { name: String -> + when { + StableCrates.contains(name) -> STABLE_VERSION_PROP_NAME + else -> UNSTABLE_VERSION_PROP_NAME + } + } + + val AWS_SDK_RUNTIME = listOf( + "aws-config", + "aws-credential-types", + "aws-endpoint", + "aws-http", + "aws-hyper", + "aws-runtime", + "aws-runtime-api", + "aws-sig-auth", + "aws-sigv4", + "aws-types", + ).map { Crate(it, version(it)) } + val SMITHY_RUNTIME_COMMON = listOf( - Crate("aws-smithy-async", STABLE_VERSION_PROP_NAME), - Crate("aws-smithy-checksums", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-client", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-eventstream", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-http", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-http-auth", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-http-tower", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-json", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-protocol-test", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-query", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-runtime", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-runtime-api", STABLE_VERSION_PROP_NAME), - Crate("aws-smithy-types", STABLE_VERSION_PROP_NAME), - Crate("aws-smithy-types-convert", UNSTABLE_VERSION_PROP_NAME), - Crate("aws-smithy-xml", UNSTABLE_VERSION_PROP_NAME), - ) + "aws-smithy-async", + "aws-smithy-checksums", + "aws-smithy-client", + "aws-smithy-eventstream", + "aws-smithy-http", + "aws-smithy-http-auth", + "aws-smithy-http-tower", + "aws-smithy-json", + "aws-smithy-protocol-test", + "aws-smithy-query", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-smithy-types-convert", + "aws-smithy-xml", + ).map { Crate(it, version(it)) } val AWS_SDK_SMITHY_RUNTIME = SMITHY_RUNTIME_COMMON diff --git a/buildSrc/src/test/kotlin/CrateSetTest.kt b/buildSrc/src/test/kotlin/CrateSetTest.kt index 59b1c82551..dacc75cc32 100644 --- a/buildSrc/src/test/kotlin/CrateSetTest.kt +++ b/buildSrc/src/test/kotlin/CrateSetTest.kt @@ -22,10 +22,10 @@ class CrateSetTest { * matches what `package.metadata.smithy-rs-release-tooling` says in the `Cargo.toml` * for the corresponding crate. */ - private fun sutStabilityMatchesManifestStability(versionPropertyName: String, stabilityInManifest: Boolean) { + private fun sutStabilityMatchesManifestStability(versionPropertyName: String, stabilityInManifest: Boolean, crate: String) { when (stabilityInManifest) { - true -> assertEquals(STABLE_VERSION_PROP_NAME, versionPropertyName) - false -> assertEquals(UNSTABLE_VERSION_PROP_NAME, versionPropertyName) + true -> assertEquals(STABLE_VERSION_PROP_NAME, versionPropertyName, "Crate: $crate") + false -> assertEquals(UNSTABLE_VERSION_PROP_NAME, versionPropertyName, "Crate: $crate") } } @@ -41,17 +41,13 @@ class CrateSetTest { crateSet.forEach { val path = "$relativePathToRustRuntime/${it.name}/Cargo.toml" val contents = File(path).readText() - val manifest = try { - Toml().read(contents) + val isStable = try { + Toml().read(contents).getTable("package.metadata.smithy-rs-release-tooling")?.getBoolean("stable") ?: false } catch (e: java.lang.IllegalStateException) { - // Currently, `aws-sigv4` cannot be read as a `TOML` because of the following error: - // Invalid table definition on line 54: [target.'cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))'.dev-dependencies]] - logger.info("failed to read ${it.name} as TOML: $e") - Toml() + // sigv4 doesn't parse but it's stable now, hax hax hax + contents.trim().endsWith("[package.metadata.smithy-rs-release-tooling]\nstable = true") } - manifest.getTable("package.metadata.smithy-rs-release-tooling")?.entrySet()?.map { entry -> - sutStabilityMatchesManifestStability(it.versionPropertyName, entry.value as Boolean) - } ?: sutStabilityMatchesManifestStability(it.versionPropertyName, false) + sutStabilityMatchesManifestStability(it.versionPropertyName, isStable, it.name) } } diff --git a/rust-runtime/aws-smithy-runtime/Cargo.toml b/rust-runtime/aws-smithy-runtime/Cargo.toml index e33b334d77..5ac41f45bd 100644 --- a/rust-runtime/aws-smithy-runtime/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime/Cargo.toml @@ -58,3 +58,6 @@ all-features = true targets = ["x86_64-unknown-linux-gnu"] rustdoc-args = ["--cfg", "docsrs"] # End of docs.rs metadata + +[package.metadata.smithy-rs-release-tooling] +stable = true diff --git a/rust-runtime/aws-smithy-runtime/external-types.toml b/rust-runtime/aws-smithy-runtime/external-types.toml index 0c0d03c305..ef336fec78 100644 --- a/rust-runtime/aws-smithy-runtime/external-types.toml +++ b/rust-runtime/aws-smithy-runtime/external-types.toml @@ -1,24 +1,22 @@ allowed_external_types = [ "aws_smithy_runtime_api::*", "aws_smithy_async::*", - "aws_smithy_http::*", "aws_smithy_types::*", - # TODO(audit-external-type-usage) We should newtype these or otherwise avoid exposing them - "http::header::name::HeaderName", - "http::request::Request", - "http::response::Response", - "http::uri::Uri", - - # Used for creating hyper connectors + # Used for creating hyper connectors in the hyper feature and test-util features "tower_service::Service", # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `test-util` feature - "aws_smithy_protocol_test::MediaType", "bytes::bytes::Bytes", "serde::ser::Serialize", "serde::de::Deserialize", "hyper::client::connect::dns::Name", + # used by dvr + "http::request::Request", + "http::response::Response", + "http::uri::Uri", + + # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `connector-hyper-0-14-x` feature "hyper::client::client::Builder", diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr.rs b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr.rs index c23d711ae6..c5f10961db 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr.rs @@ -20,7 +20,6 @@ use std::collections::HashMap; mod record; mod replay; -pub use aws_smithy_protocol_test::MediaType; pub use record::RecordingClient; pub use replay::ReplayingClient; diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr/replay.rs b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr/replay.rs index 7607b53c08..df06eab87b 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr/replay.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr/replay.rs @@ -75,7 +75,7 @@ impl ReplayingClient { } /// Validate all headers and bodies - pub async fn full_validate(self, media_type: MediaType) -> Result<(), Box> { + pub async fn full_validate(self, media_type: &str) -> Result<(), Box> { self.validate_body_and_headers(None, media_type).await } @@ -95,13 +95,13 @@ impl ReplayingClient { pub async fn validate_body_and_headers( self, checked_headers: Option<&[&str]>, - media_type: MediaType, + media_type: &str, ) -> Result<(), Box> { self.validate_base(checked_headers, |b1, b2| { aws_smithy_protocol_test::validate_body( b1, std::str::from_utf8(b2).unwrap(), - media_type.clone(), + MediaType::from(media_type), ) .map_err(|e| Box::new(e) as _) }) diff --git a/rust-runtime/aws-smithy-runtime/src/lib.rs b/rust-runtime/aws-smithy-runtime/src/lib.rs index 2fbb596657..3a7974e9b5 100644 --- a/rust-runtime/aws-smithy-runtime/src/lib.rs +++ b/rust-runtime/aws-smithy-runtime/src/lib.rs @@ -17,6 +17,7 @@ unreachable_pub, rust_2018_idioms )] +#![cfg_attr(docsrs, feature(doc_auto_cfg))] /// Runtime support logic for generated clients. #[cfg(feature = "client")] diff --git a/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs b/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs index 2bb7610c5a..c7dee81ed2 100644 --- a/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs +++ b/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs @@ -3,16 +3,19 @@ * SPDX-License-Identifier: Apache-2.0 */ -use crate::anchor::replace_anchor; -use crate::lint::LintError; -use crate::{all_cargo_tomls, Check, Fix, Lint}; +use std::collections::HashSet; +use std::fs::read_to_string; +use std::ops::Deref; +use std::path::{Path, PathBuf}; + use anyhow::{Context, Result}; use cargo_toml::{Manifest, Package}; use serde::de::DeserializeOwned; use serde::Deserialize; -use std::fs::read_to_string; -use std::ops::Deref; -use std::path::{Path, PathBuf}; + +use crate::anchor::replace_anchor; +use crate::lint::LintError; +use crate::{all_cargo_tomls, repo_root, Check, Fix, Lint}; macro_rules! return_errors { ($errs: expr) => { @@ -224,3 +227,170 @@ fn fix_docs_rs(contents: &str) -> Result { )?; Ok(new) } + +#[derive(Deserialize)] +struct SmithyRsMetadata { + #[serde(rename = "smithy-rs-release-tooling")] + smithy_rs_release_tooling: ToolingMetadata, +} + +#[derive(Deserialize)] +struct ToolingMetadata { + stable: bool, +} + +pub(crate) struct StableCratesExposeStableCrates { + stable_crates: HashSet, +} + +impl StableCratesExposeStableCrates { + pub(crate) fn new() -> Result { + let mut stable_crates = all_cargo_tomls()? + .flat_map(crate_is_stable) + .map(|c| c.replace('-', "_")) + .collect::>(); + for crte in [ + "tokio", + "hyper", + "http", + "bytes", + "http-body", + "aws-smithy-eventstream", + ] { + stable_crates.insert(crte.replace('-', "_")); + } + + Ok(Self { stable_crates }) + } +} +impl Lint for StableCratesExposeStableCrates { + fn name(&self) -> &str { + "StableCratesExposeStableCrates" + } + + fn files_to_check(&self) -> Result> { + Ok(all_cargo_tomls()?.collect()) + } +} + +pub(crate) struct SdkExternalLintsExposesStableCrates { + checker: StableCratesExposeStableCrates, +} + +impl SdkExternalLintsExposesStableCrates { + pub(crate) fn new() -> Result { + Ok(Self { + checker: StableCratesExposeStableCrates::new()?, + }) + } +} + +impl Lint for SdkExternalLintsExposesStableCrates { + fn name(&self) -> &str { + "sdk-external-types exposes stable types" + } + + fn files_to_check(&self) -> Result> { + Ok(vec![repo_root().join("aws/sdk/sdk-external-types.toml")]) + } +} + +impl Check for SdkExternalLintsExposesStableCrates { + fn check(&self, path: impl AsRef) -> Result> { + Ok(self + .checker + .check_external_types(path)? + .iter() + .filter(|it| it != &"aws_smithy_http") // currently exposed for transcribe streaming + .map(|crte| { + LintError::new(format!( + "the list of allowed SDK external types contained unstable crate {crte}" + )) + }) + .collect()) + } +} + +impl Check for StableCratesExposeStableCrates { + fn check(&self, path: impl AsRef) -> Result> { + let parsed = match package(path.as_ref()) { + // if the package doesn't parse, someone else figured that out + Err(_) => return Ok(vec![]), + // if there is something wrong, someone figured that out too + Ok(Err(_errs)) => return Ok(vec![]), + Ok(Ok(pkg)) => pkg, + }; + self.check_stable_crates_only_expose_stable_crates(parsed, path) + } +} + +#[derive(Deserialize)] +struct ExternalTypes { + allowed_external_types: Vec, +} + +impl StableCratesExposeStableCrates { + /// returns a list of unstable types exposed by this list + fn check_external_types(&self, external_types: impl AsRef) -> Result> { + let external_types = + toml::from_str::(&read_to_string(external_types.as_ref())?) + .context("invalid external types format")?; + let mut errs = vec![]; + for tpe in external_types.allowed_external_types { + let referenced_crate = tpe.split_once("::").map(|(pkg, _r)| pkg).unwrap_or(&tpe); + if !self.stable_crates.contains(referenced_crate) { + errs.push(referenced_crate.to_string()) + } + } + Ok(errs) + } + fn check_stable_crates_only_expose_stable_crates( + &self, + package: Package, + path: impl AsRef, + ) -> Result> { + // [package.metadata.smithy-rs-release-tooling] + if package + .metadata + .map(|meta| meta.smithy_rs_release_tooling.stable) + == Some(true) + { + let name = package.name; + Ok(self + .check_external_types(path.as_ref().parent().unwrap().join("external-types.toml"))? + .iter() + // TODO(tooling): When tooling allows, specify this at the crate level. These + // are gated in test-util + .filter(|tpe| { + !(name == "aws-smithy-runtime" + && ["tower_service", "serde"].contains(&tpe.as_str())) + }) + .map(|crte| { + LintError::new(format!( + "stable crate {name} exposed {crte} which is unstable" + )) + }) + .collect::>()) + } else { + Ok(vec![]) + } + } +} + +/// if the crate is stable, returns the name of the crate +fn crate_is_stable(path: impl AsRef) -> Option { + let parsed: Package = match package(path.as_ref()) { + // if the package doesn't parse, someone else figured that out + Err(_) => return None, + // if there is something wrong, someone figured that out too + Ok(Err(_errs)) => return None, + Ok(Ok(pkg)) => pkg, + }; + match parsed + .metadata + .map(|meta| meta.smithy_rs_release_tooling.stable) + { + Some(true) => Some(parsed.name), + _ => None, + } +} diff --git a/tools/ci-build/sdk-lints/src/main.rs b/tools/ci-build/sdk-lints/src/main.rs index 501b485218..d05788387f 100644 --- a/tools/ci-build/sdk-lints/src/main.rs +++ b/tools/ci-build/sdk-lints/src/main.rs @@ -6,7 +6,10 @@ use crate::changelog::ChangelogNext; use crate::copyright::CopyrightHeader; use crate::lint::{Check, Fix, Lint, LintError, Mode}; -use crate::lint_cargo_toml::{CrateAuthor, CrateLicense, DocsRs}; +use crate::lint_cargo_toml::{ + CrateAuthor, CrateLicense, DocsRs, SdkExternalLintsExposesStableCrates, + StableCratesExposeStableCrates, +}; use crate::readmes::{ReadmesExist, ReadmesHaveFooters}; use crate::todos::TodosHaveContext; use anyhow::{bail, Context, Result}; @@ -135,6 +138,8 @@ fn main() -> Result<()> { if todos || all { errs.extend(TodosHaveContext.check_all()?); } + errs.extend(StableCratesExposeStableCrates::new()?.check_all()?); + errs.extend(SdkExternalLintsExposesStableCrates::new()?.check_all()?); ok(errs)? } Args::Fix {