From d71f5bf708ffe912f31cc909e9946dfed4f3c0ec Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 15 Nov 2023 21:27:29 -0500 Subject: [PATCH] Add tooling & fix some missed crates --- .../aws-credential-types/Cargo.toml | 3 + aws/rust-runtime/aws-runtime/Cargo.toml | 3 + aws/rust-runtime/aws-sigv4/Cargo.toml | 3 + aws/sdk/integration-tests/s3/tests/no_auth.rs | 8 +- .../s3/tests/request_information_headers.rs | 2 +- .../timestreamquery/tests/endpoint_disco.rs | 2 +- buildSrc/src/main/kotlin/CrateSet.kt | 78 +++++--- rust-runtime/aws-smithy-runtime/Cargo.toml | 3 + .../aws-smithy-runtime/external-types.toml | 1 - .../src/client/http/test_util/dvr.rs | 1 - .../src/client/http/test_util/dvr/replay.rs | 9 +- .../ci-build/sdk-lints/src/lint_cargo_toml.rs | 182 +++++++++++++++++- tools/ci-build/sdk-lints/src/main.rs | 7 +- 13 files changed, 256 insertions(+), 46 deletions(-) diff --git a/aws/rust-runtime/aws-credential-types/Cargo.toml b/aws/rust-runtime/aws-credential-types/Cargo.toml index 33c1b2a91e2..865542ca0e1 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 b28a0638817..b255e8b8421 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-sigv4/Cargo.toml b/aws/rust-runtime/aws-sigv4/Cargo.toml index ce91ae52090..8a428c29241 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/sdk/integration-tests/s3/tests/no_auth.rs b/aws/sdk/integration-tests/s3/tests/no_auth.rs index f3029e4a1db..475fce91393 100644 --- a/aws/sdk/integration-tests/s3/tests/no_auth.rs +++ b/aws/sdk/integration-tests/s3/tests/no_auth.rs @@ -34,7 +34,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 +66,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 +97,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 +128,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 c0203093773..87ae6360127 100644 --- a/aws/sdk/integration-tests/s3/tests/request_information_headers.rs +++ b/aws/sdk/integration-tests/s3/tests/request_information_headers.rs @@ -104,7 +104,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 42d54619711..d6a5afeddcb 100644 --- a/aws/sdk/integration-tests/timestreamquery/tests/endpoint_disco.rs +++ b/aws/sdk/integration-tests/timestreamquery/tests/endpoint_disco.rs @@ -75,7 +75,7 @@ async fn do_endpoint_discovery() { "content-type", "x-amz-target", ]), - MediaType::Json, + "application/json", ) .await .unwrap(); diff --git a/buildSrc/src/main/kotlin/CrateSet.kt b/buildSrc/src/main/kotlin/CrateSet.kt index d8a95838e60..58bf3dee884 100644 --- a/buildSrc/src/main/kotlin/CrateSet.kt +++ b/buildSrc/src/main/kotlin/CrateSet.kt @@ -16,36 +16,60 @@ 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-types", + "aws-smithy-types-convert", + "aws-smithy-xml", ) + 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/rust-runtime/aws-smithy-runtime/Cargo.toml b/rust-runtime/aws-smithy-runtime/Cargo.toml index e33b334d77b..5ac41f45bd9 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 f9dffb758c2..ef336fec784 100644 --- a/rust-runtime/aws-smithy-runtime/external-types.toml +++ b/rust-runtime/aws-smithy-runtime/external-types.toml @@ -7,7 +7,6 @@ allowed_external_types = [ "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", 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 c23d711ae6b..c5f10961db2 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 7607b53c08b..3f37f49e38a 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,8 +75,9 @@ impl ReplayingClient { } /// Validate all headers and bodies - pub async fn full_validate(self, media_type: MediaType) -> Result<(), Box> { - self.validate_body_and_headers(None, media_type).await + pub async fn full_validate(self, media_type: &str) -> Result<(), Box> { + self.validate_body_and_headers(None, MediaType::from(media_type)) + .await } /// Validate actual requests against expected requests @@ -95,13 +96,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/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs b/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs index 2bb7610c5a7..a5fca6daefe 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 501b4852183..d05788387fd 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 {