Check external types for runtime and SDK crates in CI (#1625)

* Rename `cargo-api-linter` to `cargo-check-external-types`
* Run `cargo-check-external-types` on Smithy client/shared runtime crates
* Run `cargo-check-external-types` on SDK runtime crates
* Fix bug in determining crate name
* Run `cargo-check-external-types` on generated SDK crates
This commit is contained in:
John DiSanti 2022-08-10 18:33:43 -07:00 committed by GitHub
parent 6e1d1f6c18
commit cbbbec4a48
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
49 changed files with 301 additions and 32 deletions

View File

@ -17,10 +17,14 @@ allowed_external_types = [
"aws_smithy_types::timeout::error::ConfigError",
"aws_types::*",
"http::response::Response",
"http::uri::InvalidUri",
"http::uri::Uri",
"tower_service::Service",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if `InvalidUri` should be exposed
"http::uri::InvalidUri",
# TODO(https://github.com/awslabs/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",
"tower_service::Service",
]

View File

@ -0,0 +1,10 @@
allowed_external_types = [
"aws_smithy_http::*",
"aws_smithy_types::*",
"aws_types::*",
"bytes::bytes::Bytes",
"http_body::Body",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if the following should be exposed
"http::header::value::InvalidHeaderValue",
]

View File

@ -0,0 +1,2 @@
allowed_external_types = [
]

View File

@ -1,5 +1,5 @@
[package]
name = "inlineable-aws"
name = "aws-inlineable"
version = "0.0.0-smithy-rs-head"
authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>", "Russell Cohen <rcoh@amazon.com>"]
description = """

View File

@ -0,0 +1,15 @@
allowed_external_types = [
"aws_smithy_client::*",
"aws_smithy_http::*",
"aws_smithy_types::*",
"aws_types::*",
"http::header::map::HeaderMap",
"http::header::value::HeaderValue",
"http::request::Request",
"http::error::Error",
"http::uri::Uri",
"http::method::Method",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if we want to continue exposing tower_layer
"tower_layer::Layer",
]

View File

@ -18,8 +18,8 @@ use aws_endpoint::partition::endpoint::{Protocol, SignatureVersion};
use aws_endpoint::set_endpoint_resolver;
use aws_http::retry::AwsErrorRetryPolicy;
use aws_http::user_agent::AwsUserAgent;
use aws_inlineable::middleware::DefaultMiddleware;
use aws_sig_auth::signer::OperationSigningConfig;
use inlineable_aws::middleware::DefaultMiddleware;
use aws_smithy_client::test_connection::TestConnection;
use aws_smithy_http::body::SdkBody;

View File

@ -0,0 +1,9 @@
allowed_external_types = [
"aws_sigv4::http_request::sign::SignableBody",
"aws_smithy_http::*",
"aws_types::*",
"http::request::Request",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature
"aws_smithy_eventstream::frame::SignMessage",
]

View File

@ -0,0 +1,14 @@
allowed_external_types = [
"http::header::map::HeaderMap",
"http::header::name::HeaderName",
"http::header::value::HeaderValue",
"http::method::Method",
"http::request::Request",
"http::uri::Uri",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose on `ring`
"ring::hmac::Tag",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature
"aws_smithy_eventstream::frame::Message",
]

View File

@ -0,0 +1,10 @@
allowed_external_types = [
"aws_smithy_async::rt::sleep::AsyncSleep",
"aws_smithy_client::http_connector",
"aws_smithy_client::http_connector::HttpConnector",
"aws_smithy_http::endpoint::Endpoint",
"aws_smithy_http::endpoint::EndpointPrefix",
"aws_smithy_types::retry::RetryConfig",
"aws_smithy_types::timeout::config::Config",
"http::uri::Uri",
]

View File

@ -0,0 +1,24 @@
# These are the allowed external types in the `aws-sdk-*` generated crates, checked by CI.
allowed_external_types = [
"aws_http::*",
"aws_smithy_async::*",
"aws_smithy_client::*",
"aws_smithy_http::*",
"aws_smithy_types::*",
"aws_types::*",
"http::header::map::HeaderMap",
"http::header::value::HeaderValue",
"http::request::Request",
"http::error::Error",
"http::uri::Uri",
"http::method::Method",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Switch to AsyncIterator once standardized
"futures_core::stream::Stream",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature
"aws_smithy_eventstream::*",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if we want to continue exposing tower_layer
"tower_layer::Layer",
]

View File

@ -0,0 +1,7 @@
allowed_external_types = [
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Switch to AsyncIterator once standardized
"futures_core::stream::Stream",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose `SendError`
"tokio::sync::mpsc::error::SendError",
]

View File

@ -0,0 +1,8 @@
allowed_external_types = [
"aws_smithy_http::*",
"bytes::bytes::Bytes",
"http::header::map::HeaderMap",
"http::header::name::HeaderName",
"http::header::value::HeaderValue",
"http_body::Body",
]

View File

@ -0,0 +1,39 @@
allowed_external_types = [
"aws_smithy_async::*",
"aws_smithy_http::*",
"aws_smithy_http_tower::*",
"aws_smithy_types::*",
"http::header::name::HeaderName",
"http::request::Request",
"http::response::Response",
"http::uri::Uri",
"tower::retry::policy::Policy",
"tower_service::Service",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Move `rustls`/`native-tls` features into separate crates
"hyper::client::connect::http::HttpConnector",
"hyper_rustls::connector::HttpsConnector",
"hyper_tls::client::HttpsConnector",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `client-hyper` feature
"hyper::client::client::Builder",
"hyper::client::connect::Connection",
"tokio::io::async_read::AsyncRead",
"tokio::io::async_write::AsyncWrite",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `test-utils` feature
"bytes::bytes::Bytes",
"serde::ser::Serialize",
"serde::de::Deserialize",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if we want to continue exposing tower_layer
"tower_layer::Layer",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Feature gate middleware_fn and service_fn, or remove them if they're unused
"tower::util::map_request::MapRequestLayer",
"tower::util::service_fn::ServiceFn",
"tower_util::MapRequestLayer",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose on `tower::BoxError`
"tower::BoxError",
]

View File

@ -0,0 +1,9 @@
allowed_external_types = [
"aws_smithy_types::*",
"bytes::buf::buf_impl::Buf",
"bytes::buf::buf_mut::BufMut",
"bytes::bytes::Bytes",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `derive-arbitrary` feature
"arbitrary::Arbitrary",
]

View File

@ -0,0 +1,12 @@
allowed_external_types = [
"aws_smithy_http::*",
"http::request::Request",
"http::response::Response",
"tower_service::Service",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose on `tower::BoxError`
"tower::BoxError",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if we want to continue exposing tower_layer
"tower_layer::Layer",
]

View File

@ -0,0 +1,40 @@
allowed_external_types = [
"aws_smithy_types::*",
"bytes::buf::buf_impl::Buf",
"bytes::bytes::Bytes",
"http::error::Error",
"http::header::map::HeaderMap",
"http::header::map::ValueIter",
"http::header::name::HeaderName",
"http::header::value::HeaderValue",
"http::request::Builder",
"http::request::Request",
"http::response::Builder",
"http::response::Response",
"http::uri::Uri",
"http::version::Version",
"http_body::Body",
"http_body::combinators::box_body::BoxBody",
"hyper::body::body::Body",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Feature gate Tokio `AsyncRead`
"tokio::io::async_read::AsyncRead",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Switch to AsyncIterator once standardized
"futures_core::stream::Stream",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Feature gate references to Tokio `File`
"tokio::fs::file::File",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if `InvalidUri` should be exposed
"http::uri::InvalidUri",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose `once_cell` in public API
"once_cell::sync::Lazy",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature
"aws_smithy_eventstream::*",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide whether to expose this type or not
"bytes_utils::segmented::SegmentedBuf",
]

View File

@ -0,0 +1,3 @@
allowed_external_types = [
"aws_smithy_types::*",
]

View File

@ -0,0 +1,3 @@
allowed_external_types = [
"aws_smithy_types::*",
]

View File

@ -0,0 +1,7 @@
allowed_external_types = [
"aws_smithy_types::*",
"chrono::datetime::DateTime",
"chrono::offset::fixed::FixedOffset",
"chrono::offset::utc::Utc",
"time::offset_date_time::OffsetDateTime",
]

View File

@ -10,6 +10,3 @@ set -e
echo "### Checking for duplicate dependency versions in the normal dependency graph with all features enabled"
cargo tree -d --edges normal --all-features
echo "### Running api-linter"
cargo "+${RUST_NIGHTLY_VERSION}" api-linter --all-features

View File

@ -0,0 +1,2 @@
allowed_external_types = [
]

View File

@ -0,0 +1,5 @@
allowed_external_types = [
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose `xmlparser` at all
"xmlparser::Token",
"xmlparser::error::Error",
]

View File

@ -0,0 +1,2 @@
allowed_external_types = [
]

View File

@ -102,7 +102,7 @@ RUN set -eux; \
git checkout ${smithy_rs_commit_hash}; \
fi; \
cargo install --locked --path tools/publisher; \
cargo +${rust_nightly_version} install --locked --path tools/api-linter; \
cargo install --locked --path tools/cargo-check-external-types; \
cargo install --locked --path tools/changelogger; \
cargo install --locked --path tools/crate-hasher; \
cargo install --locked --path tools/sdk-lints; \

View File

@ -88,7 +88,7 @@ dependencies = [
]
[[package]]
name = "cargo-api-linter"
name = "cargo-check-external-types"
version = "0.1.0"
dependencies = [
"anyhow",

View File

@ -1,11 +1,11 @@
[package]
name = "cargo-api-linter"
name = "cargo-check-external-types"
version = "0.1.0"
authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>", "John DiSanti <jdisanti@amazon.com>"]
description = "Static analysis tool to detect external types exposed in a library's public API."
edition = "2021"
license = "Apache-2.0"
repository = "https://github.com/awslabs/smithy-rs"
publish = false
[dependencies]
anyhow = "1"

View File

@ -1,5 +1,5 @@
cargo-api-linter
================
cargo-check-external-types
==========================
Static analysis tool that detects external types used in a Rust library's public API.
Configuration can be provided to allow certain external types so that this tool can
@ -33,5 +33,5 @@ cargo install --path .
Then, in your library crate path, run:
```
cargo api-linter
cargo check-external-types
```

View File

@ -19,6 +19,8 @@ struct CrateFormatVersion {
/// Runs the `cargo rustdoc` command required to produce Rustdoc's JSON output with a nightly compiler.
pub struct CargoRustDocJson {
/// Name of the crate (as specified in the Cargo.toml file)
crate_name: String,
/// Path of the crate to examine
crate_path: PathBuf,
/// Expected `target/` directory where the output will be
@ -29,11 +31,13 @@ pub struct CargoRustDocJson {
impl CargoRustDocJson {
pub fn new(
crate_name: impl Into<String>,
crate_path: impl Into<PathBuf>,
target_path: impl Into<PathBuf>,
features: Vec<String>,
) -> Self {
CargoRustDocJson {
crate_name: crate_name.into(),
crate_path: crate_path.into(),
target_path: target_path.into(),
features,
@ -48,7 +52,6 @@ impl ShellOperation for CargoRustDocJson {
let cargo = std::env::var("CARGO")
.ok()
.unwrap_or_else(|| "cargo".to_string());
let crate_path = self.crate_path.canonicalize().context(here!())?;
let mut command = Command::new(&cargo);
command.current_dir(&self.crate_path).arg("rustdoc");
@ -68,12 +71,11 @@ impl ShellOperation for CargoRustDocJson {
.context(here!("failed to run nightly rustdoc"))?;
handle_failure("rustdoc", &output)?;
let crate_name = crate_path.file_name().expect("file name").to_string_lossy();
let output_file_name = self
.target_path
.canonicalize()
.context(here!())?
.join(format!("doc/{}.json", crate_name.replace('-', "_")));
.join(format!("doc/{}.json", self.crate_name.replace('-', "_")));
let json = fs::read_to_string(output_file_name).context(here!())?;
let format_version: CrateFormatVersion = serde_json::from_str(&json)

View File

@ -12,6 +12,7 @@ use clap::Parser;
use owo_colors::{OwoColorize, Stream};
use smithy_rs_tool_common::macros::here;
use smithy_rs_tool_common::shell::ShellOperation;
use std::borrow::Cow;
use std::fmt;
use std::fs;
use std::path::PathBuf;
@ -57,7 +58,7 @@ impl FromStr for OutputFormat {
}
#[derive(clap::Args, Debug)]
struct ApiLinterArgs {
struct CheckExternalTypesArgs {
/// Enables all crate features
#[clap(long)]
all_features: bool,
@ -84,7 +85,7 @@ struct ApiLinterArgs {
#[derive(Parser, Debug)]
#[clap(author, version, about, bin_name = "cargo")]
enum Args {
ApiLinter(ApiLinterArgs),
CheckExternalTypes(CheckExternalTypesArgs),
}
enum Error {
@ -110,7 +111,7 @@ fn main() {
}
fn run_main() -> Result<(), Error> {
let Args::ApiLinter(args) = Args::parse();
let Args::CheckExternalTypes(args) = Args::parse();
if args.verbose {
let filter_layer = EnvFilter::try_from_default_env()
.or_else(|_| EnvFilter::try_new("debug"))
@ -153,13 +154,21 @@ fn run_main() -> Result<(), Error> {
.expect("parent path")
.to_path_buf()
} else {
std::env::current_dir().context(here!())?
std::env::current_dir()
.context(here!())?
.canonicalize()
.context(here!())?
};
let cargo_metadata = cargo_metadata_cmd.exec().context(here!())?;
let cargo_features = resolve_features(&cargo_metadata)?;
eprintln!("Running rustdoc to produce json doc output...");
let package = cargo::CargoRustDocJson::new(
&*cargo_metadata
.root_package()
.as_ref()
.map(|package| Cow::Borrowed(package.name.as_str()))
.unwrap_or_else(|| crate_path.file_name().expect("file name").to_string_lossy()),
&crate_path,
&cargo_metadata.target_directory,
cargo_features,

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
//! This crate exports a bunch of types for testing the api-linter against `test-crate`
//! This crate exports a bunch of types for testing cargo-check-external-types against `test-crate`
pub struct SomeStruct;
pub struct SomeOtherStruct;

View File

@ -6,8 +6,8 @@
#![feature(generic_associated_types)]
#![allow(dead_code)]
//! This crate is used to test the api-linter by exercising the all possible exposure
//! of external types in a public API.
//! This crate is used to test the cargo-check-external-types by exercising the all possible
//! exposure of external types in a public API.
use external_lib::{
AssociatedGenericTrait,
@ -55,7 +55,7 @@ pub fn external_in_fn_output_generic() -> Option<SomeStruct> {
unimplemented!()
}
// Try to trick api-linter here by putting something in a private module and re-exporting it
// Try to trick cargo-check-external-types here by putting something in a private module and re-exporting it
mod private_module {
use external_lib::SomeStruct;

View File

@ -10,16 +10,18 @@ use std::path::Path;
use test_bin::get_test_bin;
fn run_with_args(in_path: impl AsRef<Path>, args: &[&str]) -> String {
let mut cmd = get_test_bin("cargo-api-linter");
let mut cmd = get_test_bin("cargo-check-external-types");
cmd.current_dir(in_path.as_ref());
cmd.arg("api-linter");
cmd.arg("check-external-types");
for &arg in args {
cmd.arg(arg);
}
let output = cmd.output().expect("failed to start cargo-api-linter");
let output = cmd
.output()
.expect("failed to start cargo-check-external-types");
match output.status.code() {
Some(1) => { /* expected */ }
_ => handle_failure("cargo-api-linter", &output).unwrap(),
_ => handle_failure("cargo-check-external-types", &output).unwrap(),
}
let (stdout, _) = output_text(&output);
stdout

View File

@ -30,7 +30,7 @@ echo "${C_YELLOW}## Running 'cargo minimal-versions check'${C_RESET}"
cargo +"${RUST_NIGHTLY_VERSION}" minimal-versions check --all-features
echo "${C_YELLOW}## Checking for external types in public API${C_RESET}"
cargo "+${RUST_NIGHTLY_VERSION:-nightly}" api-linter --all-features --config api-linter.toml
cargo "+${RUST_NIGHTLY_VERSION:-nightly}" check-external-types --all-features --config external-types.toml
echo "${C_YELLOW}## Checking for duplicate dependency versions in the normal dependency graph with all features enabled${C_RESET}"
cargo tree -d --edges normal --all-features

View File

@ -23,3 +23,15 @@ mv aws-sdk-smoketest smithy-rs/aws/sdk/build/aws-sdk
pushd smithy-rs/aws/sdk/integration-tests
cargo check
popd &>/dev/null
pushd smithy-rs/aws/sdk/build/aws-sdk/sdk &>/dev/null
for crate_path in $(ls | grep -v "aws-"); do
if [[ -d "${crate_path}" ]]; then
pushd "${crate_path}" &>/dev/null
# Override "fail on warning" for smoke test docs since DynamoDB's modeled docs cause rustdoc warnings,
# and `cargo-check-external-types` relies on rustdoc JSON output.
RUSTDOCFLAGS="" cargo +"${RUST_NIGHTLY_VERSION}" check-external-types --all-features --config ../../../../sdk-external-types.toml
popd &>/dev/null
fi
done
popd &>/dev/null

View File

@ -14,12 +14,34 @@ for runtime_path in \
"rust-runtime" \
"aws/rust-runtime"
do
echo -e "${C_YELLOW}Testing ${runtime_path}...${C_RESET}"
echo -e "# ${C_YELLOW}Testing ${runtime_path}...${C_RESET}"
pushd "${runtime_path}" &>/dev/null
echo -e "## ${C_YELLOW}Running 'cargo clippy' on ${runtime_path}...${C_RESET}"
cargo clippy --all-features
echo -e "## ${C_YELLOW}Running 'cargo test' on ${runtime_path}...${C_RESET}"
cargo test --all-features
echo -e "## ${C_YELLOW}Running 'cargo doc' on ${runtime_path}...${C_RESET}"
cargo doc --no-deps --document-private-items --all-features
echo -e "## ${C_YELLOW}Running 'cargo minimal-versions check' on ${runtime_path}...${C_RESET}"
cargo +"${RUST_NIGHTLY_VERSION}" minimal-versions check --all-features
for crate_path in *; do
if [[ -f "${crate_path}/external-types.toml" ]]; then
# Skip `aws-config` since it has its own checks in `check-aws-config`
if [[ "${crate_path}" != "aws-config" ]]; then
echo -e "## ${C_YELLOW}Running 'cargo check-external-types' on ${crate_path}...${C_RESET}"
pushd "${crate_path}" &>/dev/null
# Override "fail on warning" for docs since `cargo-check-external-types` relies on rustdoc JSON output.
RUSTDOCFLAGS="" cargo +"${RUST_NIGHTLY_VERSION}" check-external-types --all-features --config external-types.toml
popd &>/dev/null
fi
fi
done
popd &>/dev/null
echo -e "${C_YELLOW}Running additional per-crate checks for ${runtime_path}...${C_RESET}"

View File

@ -22,7 +22,7 @@ function test_tool {
popd &>/dev/null
}
test_tool "tools/api-linter" "${RUST_NIGHTLY_VERSION}"
test_tool "tools/cargo-check-external-types" "${RUST_NIGHTLY_VERSION}"
test_tool "tools/changelogger" "${RUST_STABLE_VERSION}"
test_tool "tools/ci-cdk/canary-runner" "${RUST_STABLE_VERSION}"
test_tool "tools/crate-hasher" "${RUST_STABLE_VERSION}"