From 4b3de8f164acc1ada407e6438e3ae6775d800c6c Mon Sep 17 00:00:00 2001 From: LKSNDRTMLKV Date: Thu, 24 Sep 2026 15:40:37 +0200 Subject: [PATCH 1/2] fix(ci): run the S3 suites against RustFS --- .github/workflows/ci.yml | 46 ++--- CHANGELOG.md | 26 +++ crates/dpp-node/tests/s3_archive.rs | 109 +++++++----- crates/dpp-node/tests/snapshot_static_tier.rs | 166 ++++++++++++++---- 4 files changed, 242 insertions(+), 105 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54d45993..6f7d6ff7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -204,7 +204,7 @@ jobs: tool: nextest - name: Create results dir run: mkdir -p dpp-engine/results - # One MinIO for the whole job, rather than one per S3 test — the same + # One S3 server for the whole job, rather than one per S3 test — the same # arrangement as the Postgres service above, for the same reason. The four # `s3_archive` tests booted their own and spent about ten seconds each # doing it, which put them on the wrong side of the slow-test budget @@ -212,35 +212,37 @@ jobs: # unset, they still start their own container, so a bare `cargo test` # keeps working. # - # A step rather than a `services:` entry because MinIO needs `server /data` - # as its command, and a service container can set `image`, `env`, `ports`, - # `volumes` and `options` but not a command. The alternative — an image - # that runs the server by default — would have CI and a local run testing - # two different MinIO builds, which is a worse trade than a longer step. - # The image and tag are the ones `s3_archive.rs` pins for its own container; - # the two must move together. + # A step rather than a `services:` entry so the readiness wait and the + # server's own log on failure sit in one place. The image and tag are the + # ones `s3_archive.rs` and `snapshot_static_tier.rs` pin for their own + # containers; the three must move together. # - # `quay.io`, not Docker Hub: `docker.io/minio/minio` was removed. The Hub API - # answers 404 for the repository and every tag is denied, including this one, - # which this step pulled successfully until it vanished. quay.io is MinIO's own - # registry and carries this exact release — same publisher, same tag. - - name: Start MinIO + # RustFS, not MinIO: `docker.io/minio/minio` was removed, and on 2026-09-24 + # `quay.io/minio/minio` stopped granting anonymous pulls — the registry's + # token for it carries no actions. RustFS takes the same shape (port 9000, + # an access key pair, `/data`), and enforces bucket policies, which the + # snapshot suite depends on. It fetches `version.rustfs.com` at every + # start and `RUSTFS_CHECK_UPDATES=false` does not stop that in 1.0.0, so + # the host resolves to loopback instead: a test double has no business + # reaching the network. + - name: Start S3 test server run: | - docker run -d --name minio \ + docker run -d --name s3 \ -p 9000:9000 \ - -e MINIO_ROOT_USER=minioadmin \ - -e MINIO_ROOT_PASSWORD=minioadmin \ - quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z \ - server /data --console-address :9001 + --add-host version.rustfs.com:127.0.0.1 \ + -e RUSTFS_ACCESS_KEY=minioadmin \ + -e RUSTFS_SECRET_KEY=minioadmin \ + rustfs/rustfs:1.0.0 for _ in $(seq 1 30); do - if curl -fsS http://127.0.0.1:9000/minio/health/live >/dev/null 2>&1; then - echo "minio ready" + if curl -fsS http://127.0.0.1:9000/health/live >/dev/null 2>&1; then + echo "s3 server ready" exit 0 fi sleep 1 done - echo "minio did not become ready in 30s" - docker logs minio + echo "s3 server did not become ready in 30s" + docker logs s3 + docker exec s3 sh -c 'tail -n 50 /logs/*' || true exit 1 # Docker is pre-installed on ubuntu-latest; testcontainers uses it. # The plugin-host suite includes the fuel-exhaustion sandbox test, which diff --git a/CHANGELOG.md b/CHANGELOG.md index 29812372..9544b37d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,32 @@ under the pre-1.0 conventions in [VERSIONING.md](docs/governance/VERSIONING.md): ## [Unreleased] +### Fixed + +- **CI could not pull its S3 test server, so nothing could go green — again.** + `docker.io/minio/minio` was removed on 2026-09-13 and the pin moved to + `quay.io/minio/minio`; on 2026-09-24 that repository stopped granting + anonymous pulls. Its anonymous token carries an empty `actions` list, while a + control repository on the same registry grants `pull`, so this is the + repository closing rather than the registry failing. Every run failed at the + server's start, before a test ran. + + The S3 suites now run against **RustFS 1.0.0** (`rustfs/rustfs`, Apache-2.0) + in CI and locally, pinned at the same three sites. Same shape — port 9000, an + access-key pair, `/data`. RustFS fetches `version.rustfs.com` at every start, + and `RUSTFS_CHECK_UPDATES=false` does not stop it in 1.0.0, so every container + resolves that host to loopback: the test double makes no outbound call. The + tests' own containers wait on `/health/live` rather than a log line, because + RustFS logs to a file inside the container. + + **A new test pins what made the swap safe.** + `an_anonymous_read_is_refused_until_the_bucket_policy_allows_it` checks that an + unauthenticated read fails before the public-read policy is applied and + succeeds after. Every other snapshot test reads anonymously only after that + policy, so a server that let anyone read anything would have passed them all + while proving nothing about the policy the production bucket depends on — the + exact risk in replacing the server under them. + ## [0.14.0] - 2026-09-24 ### Breaking diff --git a/crates/dpp-node/tests/s3_archive.rs b/crates/dpp-node/tests/s3_archive.rs index 8869e17f..e98a1ee0 100644 --- a/crates/dpp-node/tests/s3_archive.rs +++ b/crates/dpp-node/tests/s3_archive.rs @@ -1,4 +1,4 @@ -//! Integration test: `S3ArchiveAdapter` against a real MinIO instance. +//! Integration test: `S3ArchiveAdapter` against a real S3-compatible server. //! //! Run: `cargo test -p dpp-node --features integration-tests` @@ -6,7 +6,7 @@ use testcontainers::{ GenericImage, ImageExt, - core::{WaitFor, ports::ContainerPort}, + core::{Host, ports::ContainerPort}, runners::AsyncRunner, }; @@ -19,23 +19,24 @@ use dpp_domain::{ }; use dpp_node::infra::s3_archive::{S3ArchiveAdapter, S3ArchiveConfig}; -/// The MinIO build every path here tests against. +/// The S3 server build every path here tests against. /// /// One constant so the shared server CI starts and the container this file /// starts locally cannot drift onto different releases. A suite that runs -/// against one MinIO in CI and a different one on a developer machine proves -/// less than it looks like it does. -/// `quay.io`, not Docker Hub. +/// against one server in CI and a different one on a developer machine proves +/// less than it looks like it does. The CI workflow and `snapshot_static_tier.rs` +/// pin the same pair; the three must move together. /// -/// `docker.io/minio/minio` was removed: the Hub API answers **404** for the -/// repository and a pull is denied for every tag, including this one, which CI -/// had been pulling successfully until it vanished. quay.io is MinIO's own -/// registry and carries this exact release, so the pin is otherwise unchanged — -/// same publisher, same tag. The CI workflow pins the same pair and the two must -/// move together. -const MINIO_IMAGE: (&str, &str) = ("quay.io/minio/minio", "RELEASE.2025-09-07T16-13-09Z"); - -/// Point this at a running MinIO and the suite uses it instead of starting a +/// **RustFS, because MinIO can no longer be pulled.** `docker.io/minio/minio` was +/// removed, and on 2026-09-24 `quay.io/minio/minio` stopped granting anonymous +/// pulls: the registry hands out a token whose `access` carries no actions, +/// while a control repository on the same registry grants `pull`. RustFS is an +/// Apache-2.0 S3 server configured the way MinIO was, and it enforces bucket +/// policies — which `snapshot_static_tier.rs` depends on, and a mock that +/// ignored them would pass while proving nothing. +const S3_IMAGE: (&str, &str) = ("rustfs/rustfs", "1.0.0"); + +/// Point this at a running S3 server and the suite uses it instead of starting a /// container per test. /// /// # Why this exists @@ -66,8 +67,8 @@ const MINIO_IMAGE: (&str, &str) = ("quay.io/minio/minio", "RELEASE.2025-09-07T16 /// `cargo test` needs no orchestration. const SHARED_ENDPOINT_ENV: &str = "ODAL_TEST_S3_ENDPOINT"; -/// A MinIO to talk to, and the bucket this test owns on it. -struct Minio { +/// An S3 server to talk to, and the bucket this test owns on it. +struct S3Server { /// Held only on the container path — dropping it stops the container. /// `None` when the endpoint came from the environment, where the server /// outlives every test process and is not this test's to stop. @@ -76,7 +77,7 @@ struct Minio { bucket: String, } -async fn start_minio() -> Minio { +async fn start_s3() -> S3Server { // A bucket per test, so one shared server gives the same isolation a // container per test gave. `ensure_bucket` creates it, buckets are cheap, // and simple lowercase hex keeps the name inside S3's naming rules. @@ -86,42 +87,62 @@ async fn start_minio() -> Minio { .ok() .filter(|s| !s.is_empty()) { - return Minio { + return S3Server { _container: None, endpoint, bucket, }; } - // `with_wait_for` is a `GenericImage` method; the `ImageExt` builders - // (`with_env_var`/`with_cmd`) convert to `ContainerRequest`, which has no - // `with_wait_for`. So set the wait condition before those calls. - // Pinned (not `latest`) for reproducibility — `latest` drifts its startup - // log (this release emits the `API:` banner on stderr). - let image = GenericImage::new(MINIO_IMAGE.0, MINIO_IMAGE.1) + // The image's default command runs the server on `/data`. RustFS fetches + // `version.rustfs.com` at every start, and `RUSTFS_CHECK_UPDATES=false` does + // not stop it in this release — measured, not assumed. Resolving the host to + // loopback does: a test double has no business reaching the network. + let image = GenericImage::new(S3_IMAGE.0, S3_IMAGE.1) .with_exposed_port(ContainerPort::Tcp(9000)) - .with_wait_for(WaitFor::message_on_stderr("API:")) - .with_env_var("MINIO_ROOT_USER", "minioadmin") - .with_env_var("MINIO_ROOT_PASSWORD", "minioadmin") - .with_cmd(vec!["server", "/data", "--console-address", ":9001"]); + .with_env_var("RUSTFS_ACCESS_KEY", "minioadmin") + .with_env_var("RUSTFS_SECRET_KEY", "minioadmin") + .with_host( + "version.rustfs.com", + Host::Addr(std::net::Ipv4Addr::LOCALHOST.into()), + ); - let container = image.start().await.expect("start minio container"); + let container = image.start().await.expect("start the S3 server container"); let port = container .get_host_port_ipv4(9000) .await - .expect("minio mapped port"); + .expect("S3 server mapped port"); + let endpoint = format!("http://127.0.0.1:{port}"); + wait_until_live(&endpoint).await; - Minio { + S3Server { _container: Some(container), - endpoint: format!("http://127.0.0.1:{port}"), + endpoint, bucket, } } -fn build_adapter(minio: &Minio) -> S3ArchiveAdapter { +/// Poll the server's liveness route until it answers. +/// +/// Not a log-line wait: RustFS writes its log to a file inside the container, so +/// nothing on stdout or stderr marks the moment it starts serving. +async fn wait_until_live(endpoint: &str) { + let url = format!("{endpoint}/health/live"); + for _ in 0..60 { + if let Ok(r) = reqwest::get(&url).await + && r.status().is_success() + { + return; + } + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + } + panic!("the S3 server at {endpoint} did not become live in 30s"); +} + +fn build_adapter(s3: &S3Server) -> S3ArchiveAdapter { S3ArchiveAdapter::new(S3ArchiveConfig { - endpoint: Some(minio.endpoint.clone()), - bucket: minio.bucket.clone(), + endpoint: Some(s3.endpoint.clone()), + bucket: s3.bucket.clone(), access_key_id: "minioadmin".into(), secret_access_key: "minioadmin".into(), region: "us-east-1".into(), @@ -179,8 +200,8 @@ fn make_passport() -> Passport { #[tokio::test] async fn archive_then_verify_integrity() { - let minio = start_minio().await; - let adapter = build_adapter(&minio); + let s3 = start_s3().await; + let adapter = build_adapter(&s3); adapter.ensure_bucket().await.expect("create bucket"); let passport = make_passport(); @@ -200,8 +221,8 @@ async fn archive_then_verify_integrity() { #[tokio::test] async fn verify_wrong_hash_returns_not_ok() { - let minio = start_minio().await; - let adapter = build_adapter(&minio); + let s3 = start_s3().await; + let adapter = build_adapter(&s3); adapter.ensure_bucket().await.expect("create bucket"); let passport = make_passport(); @@ -216,8 +237,8 @@ async fn verify_wrong_hash_returns_not_ok() { #[tokio::test] async fn retrieve_returns_original_passport() { - let minio = start_minio().await; - let adapter = build_adapter(&minio); + let s3 = start_s3().await; + let adapter = build_adapter(&s3); adapter.ensure_bucket().await.expect("create bucket"); let passport = make_passport(); @@ -235,8 +256,8 @@ async fn retrieve_returns_original_passport() { #[tokio::test] async fn retrieve_unknown_passport_returns_none() { - let minio = start_minio().await; - let adapter = build_adapter(&minio); + let s3 = start_s3().await; + let adapter = build_adapter(&s3); adapter.ensure_bucket().await.expect("create bucket"); let result = adapter.retrieve(PassportId::new()).await.expect("retrieve"); diff --git a/crates/dpp-node/tests/snapshot_static_tier.rs b/crates/dpp-node/tests/snapshot_static_tier.rs index e274aea5..53dad890 100644 --- a/crates/dpp-node/tests/snapshot_static_tier.rs +++ b/crates/dpp-node/tests/snapshot_static_tier.rs @@ -1,4 +1,4 @@ -//! Integration test: `S3SnapshotStore` against a real MinIO instance. +//! Integration test: `S3SnapshotStore` against a real S3-compatible server. //! //! Run: `cargo test -p dpp-node --features integration-tests` //! @@ -28,7 +28,7 @@ use testcontainers::{ GenericImage, ImageExt, - core::{WaitFor, ports::ContainerPort}, + core::{Host, ports::ContainerPort}, runners::AsyncRunner, }; @@ -36,63 +36,85 @@ use chrono::{Duration, SubsecRound as _, Utc}; use dpp_node::infra::s3_snapshot::{S3SnapshotConfig, S3SnapshotStore}; use dpp_types::snapshot::{SnapshotMeta, SnapshotStore, snapshot_json_key}; -/// The MinIO build this file tests against. +/// The S3 server build this file tests against. /// -/// 🚨 `quay.io`, not Docker Hub — `docker.io/minio/minio` was removed and every -/// pull is denied. Pinned to the same pair `s3_archive.rs` and the CI workflow +/// 🚨 RustFS, not MinIO — neither `docker.io/minio/minio` nor +/// `quay.io/minio/minio` can be pulled any more; `s3_archive.rs` says how that +/// was established. Pinned to the same pair `s3_archive.rs` and the CI workflow /// use; the three must move together. -const MINIO_IMAGE: (&str, &str) = ("quay.io/minio/minio", "RELEASE.2025-09-07T16-13-09Z"); +const S3_IMAGE: (&str, &str) = ("rustfs/rustfs", "1.0.0"); -/// Point this at a running MinIO and the suite uses it instead of starting a +/// Point this at a running S3 server and the suite uses it instead of starting a /// container. Same arrangement `s3_archive.rs` uses, for the same reason: /// nextest gives each test its own process, so an in-process shared container /// is one container per test again. const SHARED_ENDPOINT_ENV: &str = "ODAL_TEST_S3_ENDPOINT"; -struct Minio { +struct S3Server { _container: Option>, endpoint: String, bucket: String, } -async fn start_minio() -> Minio { +async fn start_s3() -> S3Server { let bucket = format!("test-snapshot-{}", uuid::Uuid::new_v4().simple()); if let Some(endpoint) = std::env::var(SHARED_ENDPOINT_ENV) .ok() .filter(|s| !s.is_empty()) { - return Minio { + return S3Server { _container: None, endpoint, bucket, }; } - let image = GenericImage::new(MINIO_IMAGE.0, MINIO_IMAGE.1) + let image = GenericImage::new(S3_IMAGE.0, S3_IMAGE.1) .with_exposed_port(ContainerPort::Tcp(9000)) - .with_wait_for(WaitFor::message_on_stderr("API:")) - .with_env_var("MINIO_ROOT_USER", "minioadmin") - .with_env_var("MINIO_ROOT_PASSWORD", "minioadmin") - .with_cmd(vec!["server", "/data", "--console-address", ":9001"]); - - let container = image.start().await.expect("start minio container"); + .with_env_var("RUSTFS_ACCESS_KEY", "minioadmin") + .with_env_var("RUSTFS_SECRET_KEY", "minioadmin") + // No update check reaches the network; `s3_archive.rs` says why this + // is a host pin and not an environment variable. + .with_host( + "version.rustfs.com", + Host::Addr(std::net::Ipv4Addr::LOCALHOST.into()), + ); + + let container = image.start().await.expect("start the S3 server container"); let port = container .get_host_port_ipv4(9000) .await - .expect("minio mapped port"); + .expect("S3 server mapped port"); + let endpoint = format!("http://127.0.0.1:{port}"); + wait_until_live(&endpoint).await; - Minio { + S3Server { _container: Some(container), - endpoint: format!("http://127.0.0.1:{port}"), + endpoint, bucket, } } -fn config(minio: &Minio) -> S3SnapshotConfig { +/// Poll the server's liveness route until it answers. Not a log-line wait: +/// RustFS logs to a file inside the container, not to stdout or stderr. +async fn wait_until_live(endpoint: &str) { + let url = format!("{endpoint}/health/live"); + for _ in 0..60 { + if let Ok(r) = reqwest::get(&url).await + && r.status().is_success() + { + return; + } + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + } + panic!("the S3 server at {endpoint} did not become live in 30s"); +} + +fn config(s3: &S3Server) -> S3SnapshotConfig { S3SnapshotConfig { - endpoint: Some(minio.endpoint.clone()), - bucket: minio.bucket.clone(), + endpoint: Some(s3.endpoint.clone()), + bucket: s3.bucket.clone(), access_key_id: "minioadmin".into(), secret_access_key: "minioadmin".into(), region: "us-east-1".into(), @@ -105,34 +127,46 @@ fn config(minio: &Minio) -> S3SnapshotConfig { /// by infrastructure, public by policy, because the tier's readers are /// anonymous. The test has to reproduce both halves or it would be checking a /// private bucket and proving nothing about the reader that matters. -async fn ensure_public_bucket(minio: &Minio) { +async fn ensure_public_bucket(s3: &S3Server) { + let client = admin_client(s3); + create_bucket(&client, s3).await; + allow_anonymous_reads(&client, s3).await; +} + +/// An SDK client holding the server's credentials. +fn admin_client(s3: &S3Server) -> aws_sdk_s3::Client { let credentials = aws_sdk_s3::config::Credentials::new("minioadmin", "minioadmin", None, None, "static"); let conf = aws_sdk_s3::config::Builder::new() .credentials_provider(credentials) .region(aws_sdk_s3::config::Region::new("us-east-1")) .behavior_version(aws_sdk_s3::config::BehaviorVersion::latest()) - .endpoint_url(minio.endpoint.clone()) + .endpoint_url(s3.endpoint.clone()) .force_path_style(true) .build(); - let client = aws_sdk_s3::Client::from_conf(conf); + aws_sdk_s3::Client::from_conf(conf) +} +async fn create_bucket(client: &aws_sdk_s3::Client, s3: &S3Server) { client .create_bucket() - .bucket(&minio.bucket) + .bucket(&s3.bucket) .send() .await .expect("create bucket"); +} +/// The production bucket's policy: anyone may `GetObject`, nothing else. +async fn allow_anonymous_reads(client: &aws_sdk_s3::Client, s3: &S3Server) { let policy = format!( r#"{{"Version":"2012-10-17","Statement":[{{"Effect":"Allow", "Principal":{{"AWS":["*"]}},"Action":["s3:GetObject"], "Resource":["arn:aws:s3:::{}/*"]}}]}}"#, - minio.bucket + s3.bucket ); client .put_bucket_policy() - .bucket(&minio.bucket) + .bucket(&s3.bucket) .policy(policy) .send() .await @@ -172,9 +206,9 @@ fn signed_snapshot( /// verifies. #[tokio::test] async fn a_stored_snapshot_verifies_when_fetched_anonymously() { - let minio = start_minio().await; - ensure_public_bucket(&minio).await; - let store = S3SnapshotStore::new(config(&minio)); + let s3 = start_s3().await; + ensure_public_bucket(&s3).await; + let store = S3SnapshotStore::new(config(&s3)); let as_of = Utc::now().trunc_subsecs(0); let valid_until = as_of + Duration::days(7); @@ -197,8 +231,8 @@ async fn a_stored_snapshot_verifies_when_fetched_anonymously() { // No credential, no SDK — exactly what a holder with no node has. let url = format!( "{}/{}/{}", - minio.endpoint, - minio.bucket, + s3.endpoint, + s3.bucket, snapshot_json_key(dpp_id) ); let response = reqwest::get(&url).await.expect("fetch the stored snapshot"); @@ -232,6 +266,60 @@ async fn a_stored_snapshot_verifies_when_fetched_anonymously() { ); } +/// An anonymous read is refused until the bucket policy allows it. +/// +/// Every other test here reads anonymously *after* `ensure_public_bucket`, so a +/// server that let anyone read any bucket would pass them all while proving +/// nothing about the policy the production bucket depends on. This pins the +/// difference on whichever server the suite runs against — a guard against the +/// test double, not the adapter, and it matters because that double has already +/// had to be replaced once. +#[tokio::test] +async fn an_anonymous_read_is_refused_until_the_bucket_policy_allows_it() { + let s3 = start_s3().await; + let client = admin_client(&s3); + create_bucket(&client, &s3).await; + let store = S3SnapshotStore::new(config(&s3)); + + let as_of = Utc::now().trunc_subsecs(0); + let valid_until = as_of + Duration::days(7); + let (_dir, document, _key) = signed_snapshot(as_of, valid_until); + let dpp_id = "0198f000-0000-7000-8000-000000000002"; + store + .put_public_json( + dpp_id, + &serde_json::to_vec(&document).expect("serialise"), + SnapshotMeta { + as_of, + valid_until, + max_age: std::time::Duration::from_secs(86_400), + }, + ) + .await + .expect("store the snapshot"); + + let url = format!( + "{}/{}/{}", + s3.endpoint, + s3.bucket, + snapshot_json_key(dpp_id) + ); + let before = reqwest::get(&url).await.expect("fetch").status(); + assert_eq!( + before, + reqwest::StatusCode::FORBIDDEN, + "with no policy the object must not be anonymously readable, or every \ + anonymous read in this suite passes for the server's reasons, not the policy's" + ); + + allow_anonymous_reads(&client, &s3).await; + let after = reqwest::get(&url).await.expect("fetch").status(); + assert!( + after.is_success(), + "the policy must be what grants the read: HTTP {after}" + ); +} + /// A copy served off the bucket with its proof removed is `Absent`, and /// `Absent` off the static tier means the bound was stripped. /// @@ -240,9 +328,9 @@ async fn a_stored_snapshot_verifies_when_fetched_anonymously() { /// copy as fresh. #[tokio::test] async fn a_stored_snapshot_stripped_of_its_proof_is_absent() { - let minio = start_minio().await; - ensure_public_bucket(&minio).await; - let store = S3SnapshotStore::new(config(&minio)); + let s3 = start_s3().await; + ensure_public_bucket(&s3).await; + let store = S3SnapshotStore::new(config(&s3)); let as_of = Utc::now().trunc_subsecs(0); let valid_until = as_of + Duration::days(7); @@ -268,8 +356,8 @@ async fn a_stored_snapshot_stripped_of_its_proof_is_absent() { let url = format!( "{}/{}/{}", - minio.endpoint, - minio.bucket, + s3.endpoint, + s3.bucket, snapshot_json_key(dpp_id) ); let fetched: serde_json::Value = reqwest::get(&url) From 491bfdb0d5744c4a1c76b743576cf723b0ad0eca Mon Sep 17 00:00:00 2001 From: LKSNDRTMLKV Date: Thu, 24 Sep 2026 16:01:12 +0200 Subject: [PATCH 2/2] fix(ci): generate the S3 test keys and gate on readiness --- .github/workflows/ci.yml | 18 +++++- CHANGELOG.md | 7 ++- crates/dpp-node/tests/s3_archive.rs | 63 ++++++++++++++----- crates/dpp-node/tests/snapshot_static_tier.rs | 60 +++++++++++++----- 4 files changed, 115 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f7d6ff7..e5948809 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -225,16 +225,28 @@ jobs: # start and `RUSTFS_CHECK_UPDATES=false` does not stop that in 1.0.0, so # the host resolves to loopback instead: a test double has no business # reaching the network. + # + # The key pair is generated per run and handed to the tests through + # `ODAL_TEST_S3_ACCESS_KEY`/`_SECRET_KEY`, so no credential is written into + # the repository. The readiness probe (not liveness: the first thing every + # test does is create a bucket) carries its own `--max-time`, so a server + # that accepts and never answers cannot hold one probe past the deadline. - name: Start S3 test server run: | + access_key=$(openssl rand -hex 16) + secret_key=$(openssl rand -hex 16) + echo "::add-mask::$access_key" + echo "::add-mask::$secret_key" + echo "ODAL_TEST_S3_ACCESS_KEY=$access_key" >> "$GITHUB_ENV" + echo "ODAL_TEST_S3_SECRET_KEY=$secret_key" >> "$GITHUB_ENV" docker run -d --name s3 \ -p 9000:9000 \ --add-host version.rustfs.com:127.0.0.1 \ - -e RUSTFS_ACCESS_KEY=minioadmin \ - -e RUSTFS_SECRET_KEY=minioadmin \ + -e RUSTFS_ACCESS_KEY="$access_key" \ + -e RUSTFS_SECRET_KEY="$secret_key" \ rustfs/rustfs:1.0.0 for _ in $(seq 1 30); do - if curl -fsS http://127.0.0.1:9000/health/live >/dev/null 2>&1; then + if curl -fsS --max-time 2 http://127.0.0.1:9000/health/ready >/dev/null 2>&1; then echo "s3 server ready" exit 0 fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 9544b37d..35a790dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,8 +25,11 @@ under the pre-1.0 conventions in [VERSIONING.md](docs/governance/VERSIONING.md): access-key pair, `/data`. RustFS fetches `version.rustfs.com` at every start, and `RUSTFS_CHECK_UPDATES=false` does not stop it in 1.0.0, so every container resolves that host to loopback: the test double makes no outbound call. The - tests' own containers wait on `/health/live` rather than a log line, because - RustFS logs to a file inside the container. + tests' own containers wait on `/health/ready` rather than a log line, because + RustFS logs to a file inside the container, and every probe carries its own + timeout. The `minioadmin` key pair the suites carried is gone: a container a + test starts gets a random pair, and the shared CI server's pair is generated + per run and passed as `ODAL_TEST_S3_ACCESS_KEY`/`_SECRET_KEY`. **A new test pins what made the swap safe.** `an_anonymous_read_is_refused_until_the_bucket_policy_allows_it` checks that an diff --git a/crates/dpp-node/tests/s3_archive.rs b/crates/dpp-node/tests/s3_archive.rs index e98a1ee0..97d34854 100644 --- a/crates/dpp-node/tests/s3_archive.rs +++ b/crates/dpp-node/tests/s3_archive.rs @@ -67,7 +67,13 @@ const S3_IMAGE: (&str, &str) = ("rustfs/rustfs", "1.0.0"); /// `cargo test` needs no orchestration. const SHARED_ENDPOINT_ENV: &str = "ODAL_TEST_S3_ENDPOINT"; -/// An S3 server to talk to, and the bucket this test owns on it. +/// The shared server's key pair, read beside [`SHARED_ENDPOINT_ENV`]. Whoever +/// starts that server generates the pair; none is written into this file. +const SHARED_ACCESS_KEY_ENV: &str = "ODAL_TEST_S3_ACCESS_KEY"; +const SHARED_SECRET_KEY_ENV: &str = "ODAL_TEST_S3_SECRET_KEY"; + +/// An S3 server to talk to, the bucket this test owns on it, and the key pair +/// that authenticates to it. struct S3Server { /// Held only on the container path — dropping it stops the container. /// `None` when the endpoint came from the environment, where the server @@ -75,6 +81,17 @@ struct S3Server { _container: Option>, endpoint: String, bucket: String, + access_key: String, + secret_key: String, +} + +/// A required variable of the shared-server arrangement, or a panic that says +/// which one is missing. +fn shared_key(name: &str) -> String { + std::env::var(name) + .ok() + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| panic!("{SHARED_ENDPOINT_ENV} is set, so {name} must be too")) } async fn start_s3() -> S3Server { @@ -91,17 +108,24 @@ async fn start_s3() -> S3Server { _container: None, endpoint, bucket, + access_key: shared_key(SHARED_ACCESS_KEY_ENV), + secret_key: shared_key(SHARED_SECRET_KEY_ENV), }; } + // A fresh pair for a container that lives as long as this test, so no + // reusable credential is written down anywhere. + let access_key = uuid::Uuid::new_v4().simple().to_string(); + let secret_key = uuid::Uuid::new_v4().simple().to_string(); + // The image's default command runs the server on `/data`. RustFS fetches // `version.rustfs.com` at every start, and `RUSTFS_CHECK_UPDATES=false` does // not stop it in this release — measured, not assumed. Resolving the host to // loopback does: a test double has no business reaching the network. let image = GenericImage::new(S3_IMAGE.0, S3_IMAGE.1) .with_exposed_port(ContainerPort::Tcp(9000)) - .with_env_var("RUSTFS_ACCESS_KEY", "minioadmin") - .with_env_var("RUSTFS_SECRET_KEY", "minioadmin") + .with_env_var("RUSTFS_ACCESS_KEY", access_key.clone()) + .with_env_var("RUSTFS_SECRET_KEY", secret_key.clone()) .with_host( "version.rustfs.com", Host::Addr(std::net::Ipv4Addr::LOCALHOST.into()), @@ -113,38 +137,49 @@ async fn start_s3() -> S3Server { .await .expect("S3 server mapped port"); let endpoint = format!("http://127.0.0.1:{port}"); - wait_until_live(&endpoint).await; + wait_until_ready(&endpoint).await; S3Server { _container: Some(container), endpoint, bucket, + access_key, + secret_key, } } -/// Poll the server's liveness route until it answers. +/// Poll the server's readiness route until it answers, for at most 30 seconds. /// -/// Not a log-line wait: RustFS writes its log to a file inside the container, so -/// nothing on stdout or stderr marks the moment it starts serving. -async fn wait_until_live(endpoint: &str) { - let url = format!("{endpoint}/health/live"); - for _ in 0..60 { - if let Ok(r) = reqwest::get(&url).await +/// Readiness, not liveness: a server can be up before its storage is, and the +/// first thing every test does is create a bucket. Each probe carries its own +/// timeout, because a server that accepts a connection and never answers would +/// otherwise hold a single probe past any deadline. Not a log-line wait: RustFS +/// writes its log to a file inside the container, so nothing on stdout or +/// stderr marks the moment it starts serving. +async fn wait_until_ready(endpoint: &str) { + let url = format!("{endpoint}/health/ready"); + let probe = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(2)) + .build() + .expect("probe client"); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + while tokio::time::Instant::now() < deadline { + if let Ok(r) = probe.get(&url).send().await && r.status().is_success() { return; } tokio::time::sleep(std::time::Duration::from_millis(500)).await; } - panic!("the S3 server at {endpoint} did not become live in 30s"); + panic!("the S3 server at {endpoint} was not ready within 30s"); } fn build_adapter(s3: &S3Server) -> S3ArchiveAdapter { S3ArchiveAdapter::new(S3ArchiveConfig { endpoint: Some(s3.endpoint.clone()), bucket: s3.bucket.clone(), - access_key_id: "minioadmin".into(), - secret_access_key: "minioadmin".into(), + access_key_id: s3.access_key.clone(), + secret_access_key: s3.secret_key.clone(), region: "us-east-1".into(), }) } diff --git a/crates/dpp-node/tests/snapshot_static_tier.rs b/crates/dpp-node/tests/snapshot_static_tier.rs index 53dad890..8bc41398 100644 --- a/crates/dpp-node/tests/snapshot_static_tier.rs +++ b/crates/dpp-node/tests/snapshot_static_tier.rs @@ -50,10 +50,23 @@ const S3_IMAGE: (&str, &str) = ("rustfs/rustfs", "1.0.0"); /// is one container per test again. const SHARED_ENDPOINT_ENV: &str = "ODAL_TEST_S3_ENDPOINT"; +/// The shared server's key pair — same variables `s3_archive.rs` reads. +const SHARED_ACCESS_KEY_ENV: &str = "ODAL_TEST_S3_ACCESS_KEY"; +const SHARED_SECRET_KEY_ENV: &str = "ODAL_TEST_S3_SECRET_KEY"; + struct S3Server { _container: Option>, endpoint: String, bucket: String, + access_key: String, + secret_key: String, +} + +fn shared_key(name: &str) -> String { + std::env::var(name) + .ok() + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| panic!("{SHARED_ENDPOINT_ENV} is set, so {name} must be too")) } async fn start_s3() -> S3Server { @@ -67,13 +80,19 @@ async fn start_s3() -> S3Server { _container: None, endpoint, bucket, + access_key: shared_key(SHARED_ACCESS_KEY_ENV), + secret_key: shared_key(SHARED_SECRET_KEY_ENV), }; } + // A fresh pair per container, so no reusable credential is written down. + let access_key = uuid::Uuid::new_v4().simple().to_string(); + let secret_key = uuid::Uuid::new_v4().simple().to_string(); + let image = GenericImage::new(S3_IMAGE.0, S3_IMAGE.1) .with_exposed_port(ContainerPort::Tcp(9000)) - .with_env_var("RUSTFS_ACCESS_KEY", "minioadmin") - .with_env_var("RUSTFS_SECRET_KEY", "minioadmin") + .with_env_var("RUSTFS_ACCESS_KEY", access_key.clone()) + .with_env_var("RUSTFS_SECRET_KEY", secret_key.clone()) // No update check reaches the network; `s3_archive.rs` says why this // is a host pin and not an environment variable. .with_host( @@ -87,36 +106,44 @@ async fn start_s3() -> S3Server { .await .expect("S3 server mapped port"); let endpoint = format!("http://127.0.0.1:{port}"); - wait_until_live(&endpoint).await; + wait_until_ready(&endpoint).await; S3Server { _container: Some(container), endpoint, bucket, + access_key, + secret_key, } } -/// Poll the server's liveness route until it answers. Not a log-line wait: -/// RustFS logs to a file inside the container, not to stdout or stderr. -async fn wait_until_live(endpoint: &str) { - let url = format!("{endpoint}/health/live"); - for _ in 0..60 { - if let Ok(r) = reqwest::get(&url).await +/// Poll the server's readiness route for at most 30 seconds, each probe under +/// its own timeout. `s3_archive.rs` says why readiness and why the per-probe +/// bound. +async fn wait_until_ready(endpoint: &str) { + let url = format!("{endpoint}/health/ready"); + let probe = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(2)) + .build() + .expect("probe client"); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + while tokio::time::Instant::now() < deadline { + if let Ok(r) = probe.get(&url).send().await && r.status().is_success() { return; } tokio::time::sleep(std::time::Duration::from_millis(500)).await; } - panic!("the S3 server at {endpoint} did not become live in 30s"); + panic!("the S3 server at {endpoint} was not ready within 30s"); } fn config(s3: &S3Server) -> S3SnapshotConfig { S3SnapshotConfig { endpoint: Some(s3.endpoint.clone()), bucket: s3.bucket.clone(), - access_key_id: "minioadmin".into(), - secret_access_key: "minioadmin".into(), + access_key_id: s3.access_key.clone(), + secret_access_key: s3.secret_key.clone(), region: "us-east-1".into(), } } @@ -135,8 +162,13 @@ async fn ensure_public_bucket(s3: &S3Server) { /// An SDK client holding the server's credentials. fn admin_client(s3: &S3Server) -> aws_sdk_s3::Client { - let credentials = - aws_sdk_s3::config::Credentials::new("minioadmin", "minioadmin", None, None, "static"); + let credentials = aws_sdk_s3::config::Credentials::new( + s3.access_key.clone(), + s3.secret_key.clone(), + None, + None, + "static", + ); let conf = aws_sdk_s3::config::Builder::new() .credentials_provider(credentials) .region(aws_sdk_s3::config::Region::new("us-east-1"))