Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(bindings): feature gate network tests #4907

Merged
merged 4 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci_rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
goatgoose marked this conversation as resolved.
Show resolved Hide resolved

- name: Test external build
# if this test is failing, make sure that api headers are appropriately
Expand Down
8 changes: 5 additions & 3 deletions bindings/rust/integration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion bindings/rust/integration/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

jmayclin marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(all(feature = "network-tests", test))]
#[cfg(all(not(feature = "no-network-tests"), test))]
mod network;

#[cfg(test)]
Expand Down
74 changes: 45 additions & 29 deletions bindings/rust/integration/src/network/https_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof, but that is the behavior...

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
Expand Down Expand Up @@ -83,10 +94,15 @@ async fn http_get_test() -> Result<(), Box<dyn std::error::Error>> {
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());
}
Expand Down
Loading