chore(bindings): feature gate network testsa and relax http status as…

…sertions (#4907)

.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

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.

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)]

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<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());
             }