diff --git a/.github/workflows/ci_rust.yml b/.github/workflows/ci_rust.yml index b53c1408da5..6c7f7d74fb5 100644 --- a/.github/workflows/ci_rust.yml +++ b/.github/workflows/ci_rust.yml @@ -60,7 +60,10 @@ jobs: - name: Network-enabled integration tests working-directory: ${{env.ROOT_PATH}}/integration - run: RUST_LOG=TRACE cargo test --features network-tests + # no-default-features is used because network tests are hidden behind a + # default "negative" feature. This is because we don't want network tests + # invoked on the `cargo test --all-features` pattern. + run: RUST_LOG=TRACE cargo test --no-default-features --features pq - name: Test external build # if this test is failing, make sure that api headers are appropriately diff --git a/bindings/rust/integration/Cargo.toml b/bindings/rust/integration/Cargo.toml index 8c1f75470a0..1b61246f104 100644 --- a/bindings/rust/integration/Cargo.toml +++ b/bindings/rust/integration/Cargo.toml @@ -6,11 +6,13 @@ edition = "2021" publish = false [features] -default = ["pq"] +default = ["pq", "no-network-tests"] # Network tests are useful but relatively slow and inherently flaky. So they are -# behind this feature flag. -network-tests = [] +# behind this feature flag. This is specified as a "negative" feature because +# many of our CI jobs use `cargo test --all-features`, and we don't want those +# to run these tests +no-network-tests = [] # Not all libcryptos support PQ capabilities. Tests relying on PQ functionality # can be disabled by turning off this feature. diff --git a/bindings/rust/integration/src/lib.rs b/bindings/rust/integration/src/lib.rs index 612f362e2dd..25252295875 100644 --- a/bindings/rust/integration/src/lib.rs +++ b/bindings/rust/integration/src/lib.rs @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -#[cfg(all(feature = "network-tests", test))] +#[cfg(all(not(feature = "no-network-tests"), test))] mod network; #[cfg(test)] diff --git a/bindings/rust/integration/src/network/https_client.rs b/bindings/rust/integration/src/network/https_client.rs index 84c90e5a10a..47573130153 100644 --- a/bindings/rust/integration/src/network/https_client.rs +++ b/bindings/rust/integration/src/network/https_client.rs @@ -16,44 +16,55 @@ use std::str::FromStr; #[derive(Debug)] struct TestCase { pub query_target: &'static str, - pub expected_status_code: u16, + /// We accept multiple possible results because some websites frequently change + /// behavior, possibly as a result of throttling the IP ranges of our CI + /// providers. + pub expected_status_codes: &'static [u16], } impl TestCase { - const fn new(domain: &'static str, expected_status_code: u16) -> Self { + const fn new(domain: &'static str, expected_status_codes: &'static [u16]) -> Self { TestCase { query_target: domain, - expected_status_code, + expected_status_codes, } } } const TEST_CASES: &[TestCase] = &[ // this is a link to the s2n-tls unit test coverage report, hosted on cloudfront - TestCase::new("https://dx1inn44oyl7n.cloudfront.net/main/index.html", 200), + TestCase::new( + "https://dx1inn44oyl7n.cloudfront.net/main/index.html", + &[200], + ), // this is a link to a non-existent S3 item - TestCase::new("https://notmybucket.s3.amazonaws.com/folder/afile.jpg", 403), - TestCase::new("https://www.amazon.com", 200), - TestCase::new("https://www.apple.com", 200), - TestCase::new("https://www.att.com", 200), - TestCase::new("https://www.cloudflare.com", 200), - TestCase::new("https://www.ebay.com", 200), - TestCase::new("https://www.google.com", 200), - TestCase::new("https://www.mozilla.org", 200), - TestCase::new("https://www.netflix.com", 200), - TestCase::new("https://www.openssl.org", 200), - TestCase::new("https://www.t-mobile.com", 200), - TestCase::new("https://www.verizon.com", 200), - TestCase::new("https://www.wikipedia.org", 200), - TestCase::new("https://www.yahoo.com", 200), - TestCase::new("https://www.youtube.com", 200), - TestCase::new("https://www.github.com", 301), - TestCase::new("https://www.samsung.com", 301), - TestCase::new("https://www.twitter.com", 301), - TestCase::new("https://www.facebook.com", 302), - TestCase::new("https://www.microsoft.com", 302), - TestCase::new("https://www.ibm.com", 303), - TestCase::new("https://www.f5.com", 403), + TestCase::new( + "https://notmybucket.s3.amazonaws.com/folder/afile.jpg", + &[403], + ), + TestCase::new("https://www.amazon.com", &[200]), + TestCase::new("https://www.apple.com", &[200]), + TestCase::new("https://www.att.com", &[200]), + TestCase::new("https://www.cloudflare.com", &[200]), + TestCase::new("https://www.ebay.com", &[200]), + TestCase::new("https://www.google.com", &[200]), + TestCase::new("https://www.mozilla.org", &[200]), + TestCase::new("https://www.netflix.com", &[200]), + TestCase::new("https://www.openssl.org", &[200]), + TestCase::new("https://www.t-mobile.com", &[200]), + TestCase::new("https://www.verizon.com", &[200]), + TestCase::new("https://www.wikipedia.org", &[200]), + TestCase::new("https://www.yahoo.com", &[200]), + TestCase::new("https://www.youtube.com", &[200]), + TestCase::new("https://www.github.com", &[301]), + TestCase::new("https://www.samsung.com", &[301]), + TestCase::new("https://www.twitter.com", &[301]), + TestCase::new("https://www.facebook.com", &[302]), + // 2024-11-21: Microsoft had been consistently returning a 302. It then started + // returning 403 codes in CI, but was returning 200 codes when run locally. + TestCase::new("https://www.microsoft.com", &[200, 302, 403]), + TestCase::new("https://www.ibm.com", &[303]), + TestCase::new("https://www.f5.com", &[403]), ]; /// perform an HTTP GET request against `uri` using an s2n-tls config with @@ -83,10 +94,15 @@ async fn http_get_test() -> Result<(), Box> { tracing::info!("executing test case {:#?} with {:?}", test_case, policy); let response = https_get(test_case.query_target, &policy).await?; - let expected_status = StatusCode::from_u16(test_case.expected_status_code).unwrap(); - assert_eq!(response.status(), expected_status); + let status_code = response.status().as_u16(); - if expected_status == StatusCode::OK { + let status_was_expected = test_case.expected_status_codes.contains(&status_code); + if !status_was_expected { + tracing::error!("unexpected status code: {status_code}"); + } + assert!(status_was_expected); + + if status_code == StatusCode::OK.as_u16() { let body = response.into_body().collect().await?.to_bytes(); assert!(!body.is_empty()); }