carlos / platform Public

Merge pull request #276 from carlosframework/amadan-ci-minio

amadan CI: port the minio-contract job, without a container runtime
Paul Campbell pushed by paul@keymail.dev cd47f38fdfda8bcdf39c639a32d7db39500e03de parent ae6a182parent 062ca63
3 files changed, +360 −29
  • .amadan/README.md +44 −29
  • .amadan/ci.d/30-minio-contract +311 −0
  • .gitignore +5 −0

CI passed — run details

diff --git a/.amadan/README.md b/.amadan/README.md
index 4d96e3089..2f411b85f 100644
--- a/.amadan/README.md
+++ b/.amadan/README.md
@@ -10,32 +10,47 @@ here.
|---|---|---|
| `gate` | `10-gate` | build · vet · `go test ./... -count=1` |
| `cross` | `20-cross@5` | one target per shard, all five |
-| `minio-contract` | **not ported** | see below |
-| `gofmt` | **not ported** | see below |
-
-## What is deliberately missing, and why
-
-Two jobs are absent. Writing that down is the point of this file: a
-`ci.d/` directory that quietly covers less than the workflow next to it
-is worth less than no `ci.d/` at all, because it invites people to read
-a green run as meaning more than it does.
-
-**`minio-contract`** starts a MinIO server in Docker and runs
-`TestDeliveredContractMinio` against it. amadan CI jobs run under
-bubblewrap with the host filesystem read-only and no container runtime;
-`-isolate oci` is designed but unbuilt, precisely because "what
-container access does a job get, and does that leave any isolation at
-all" is unresolved. So this job stays on GitHub, where a service
-container is free. It is the only place that S3 contract runs live.
-
-**`gofmt`** checks only the files a PR changed, deliberately — three
-pre-existing unformatted files elsewhere are left alone, and a
-repo-wide check would fail on arrival and teach everyone to ignore the
-job. That needs the PR's **base SHA**, which is a GitHub concept. amadan
-CI hands a job its commit and its branch and has no notion of what it
-is a pull request against, so the job cannot be reproduced faithfully
-here. Reproducing it *unfaithfully* — repo-wide, or diffed against a
-guessed base — is worse than not running it.
-
-Both are gaps in this runner, not in the gate. The gate is still whole
-on GitHub.
+| `minio-contract` | `30-minio-contract` | MinIO on loopback, no container |
+| `gofmt` | `10-gate` | repo-wide, via `make test` — see below |
+
+## `gofmt`: ported by widening it, not by reproducing it
+
+Nothing is missing now. Writing that down is still the point of this
+file: a `ci.d/` directory that quietly covers less than the workflow
+next to it is worth less than no `ci.d/` at all, because it invites
+people to read a green run as meaning more than it does. This file is
+the check on that, so it has to stay true.
+
+**`gofmt`** was the last gap, and it closed by widening rather than by
+imitation. The workflow checks only the files a PR changed, because it
+needs a **base SHA** — a GitHub concept. amadan CI hands a job its
+commit and its branch and has no notion of what it is a pull request
+against, so that job cannot be reproduced faithfully here.
+
+It does not need to be. The scoping existed to protect three
+pre-existing unformatted files, and those are gone: `gofmt -l .` over
+the whole tree is clean today. So `10-gate` runs `make test`, whose
+recipe already contains a **repo-wide** gofmt check — which is stricter
+than the job it replaces, and one definition shared with what a
+developer runs locally rather than a second list that can drift.
+
+The rule that survives from the old scoping is the one worth keeping:
+if a repo-wide check ever starts failing on arrival, fix the tree
+rather than narrowing the check, because a gate people learn to ignore
+is not a gate.
+
+## `minio-contract`, and why it turned out to be portable
+
+It was listed here as impossible: it starts MinIO in Docker, and amadan
+jobs get no container runtime. That conflated the job with its
+packaging. MinIO and `mc` are single static Go binaries, and
+`TestDeliveredContractMinio` only wants an endpoint and credentials over
+HTTP — it has no opinion about what is serving. So `30-minio-contract`
+downloads both binaries at pinned versions, verifies them against pinned
+sha256 digests, and runs the server on loopback inside the job's
+existing bubblewrap sandbox. Nothing persists on the box.
+
+The test **skips itself** unless `CARLOS_MINIO_TEST=1`, and a skipped
+test exits 0. The step therefore requires the parent's `--- PASS:` line
+to appear in the output and fails without it, so a dropped env var
+cannot turn this into a green step that exercised nothing.
diff --git a/.amadan/ci.d/30-minio-contract b/.amadan/ci.d/30-minio-contract
new file mode 100755
index 000000000..48ed815ee
--- /dev/null
+++ b/.amadan/ci.d/30-minio-contract
@@ -0,0 +1,311 @@
+#!/bin/sh
+# The S3 delivered-contract gate: internal/objstore's
+# TestDeliveredContractMinio, run against a real MinIO, with no container
+# runtime anywhere in sight.
+#
+# A SECOND home for the workflow's `minio-contract` job, not a
+# replacement. .github/workflows/ci.yml stays the required check and
+# stays the definition of green; where the two disagree, the workflow is
+# the one that gates a merge and the difference is a bug in this file.
+#
+# WHY THIS CAN EXIST HERE AT ALL
+#
+# .amadan/README.md used to list this job as "not ported", on the grounds
+# that it starts MinIO in Docker and amadan jobs get no container
+# runtime. That reasoning conflated the job with its packaging. MinIO and
+# mc are single static Go binaries, published per-arch at pinned archive
+# URLs on dl.min.io, and contract_test.go only ever wanted MINIO_ENDPOINT
+# plus credentials over HTTP — it has no opinion about what is serving.
+# So the container was never load-bearing; it was just how GitHub happens
+# to hand you a MinIO.
+#
+# Running the server as a plain process inside the job's existing
+# bubblewrap sandbox is, if anything, tighter than the workflow: it binds
+# loopback only, its data directory is the job's private /tmp tmpfs, and
+# it dies with the job's PID namespace. Nothing persists on the box.
+set -eu
+
+# Fail on a missing tool by name rather than by a confusing error forty
+# lines down. Both are present on the runner and in the workflow's image;
+# this is here so that a stripped-down box says which one it is missing.
+for tool in curl sha256sum; do
+ command -v "$tool" >/dev/null 2>&1 || {
+ echo "$tool is required by this step but was not found on PATH" >&2
+ exit 1
+ }
+done
+
+# ---------------------------------------------------------------------
+# The pins.
+#
+# The workflow pins its MinIO image and its mc download, and says why:
+# a floating minio tag has already broken this exact job class once, when
+# mc's admin-policy syntax churned across versions. This is a port of
+# that pin, not an upgrade — RELEASE.2025-09-07T16-13-09Z is the same
+# server release the workflow's image digest resolves to, and the mc
+# release is the same one the workflow downloads.
+#
+# But a version string is a weaker pin than the workflow's, because the
+# workflow gets a container digest for free and a plain URL does not:
+# dl.min.io could serve different bytes at the same archive path and
+# nothing would notice. So each binary carries its sha256, verified after
+# download and again on every cache hit, and a mismatch refuses to run
+# rather than testing whatever arrived. The four digests below were taken
+# from upstream's own published .sha256sum files alongside each archive
+# (and independently confirmed against the downloaded bytes) on
+# 2026-08-23.
+#
+# Bump the two releases together, and re-derive all four digests when you
+# do.
+MINIO_RELEASE=RELEASE.2025-09-07T16-13-09Z
+MC_RELEASE=RELEASE.2025-08-13T08-35-41Z
+
+# Arch-aware on purpose. The amadan runner is arm64 and the workflow's
+# ubuntu-latest is amd64, so a hardwired path works in exactly one of the
+# two places. The workflow in fact hardwires linux-amd64 for its mc
+# download — harmless there, wrong here, and a bug not to carry across.
+case "$(uname -m)" in
+x86_64 | amd64)
+ arch=amd64
+ minio_sha=7c5bd8512c6e966455b1d198209358b2d191c77a83ab377c4073281065fb855f
+ mc_sha=01f866e9c5f9b87c2b09116fa5d7c06695b106242d829a8bb32990c00312e891
+ ;;
+aarch64 | arm64)
+ arch=arm64
+ minio_sha=5c83cd2cf151717ba0243f73e1c7802ff36e272b67144bdd7f1f7d684fd6f03d
+ mc_sha=14c8c9616cfce4636add161304353244e8de383b2e2752c0e9dad01d4c27c12c
+ ;;
+*)
+ echo "no pinned MinIO build for $(uname -m) — add one above, do not guess" >&2
+ exit 1
+ ;;
+esac
+
+echo "minio contract: $arch, minio $MINIO_RELEASE, mc $MC_RELEASE"
+
+# ---------------------------------------------------------------------
+# The cache.
+#
+# amadan reuses one workspace per repo across jobs and does not clean
+# untracked files between them by default, so an untracked directory in
+# the checkout IS the cache — no separate cache action, no key, nothing
+# to expire. ~140MB of binaries then downloads once per worker rather
+# than once per run.
+#
+# The dot prefix is deliberate: the Go toolchain skips dot-directories
+# when walking ./..., so this cannot leak into `go build ./...` or
+# `go vet ./...` in 10-gate no matter what ends up inside it. The digest
+# check below is what makes reusing the cache safe — a truncated or
+# half-written binary from an interrupted job fails verification and is
+# re-fetched, so a cache hit is never taken on trust.
+cache="$PWD/.amadan-cache/minio-$arch"
+mkdir -p "$cache"
+
+fetch_pinned() {
+ fp_name=$1
+ fp_url=$2
+ fp_want=$3
+ fp_dest=$4
+
+ if [ -f "$fp_dest" ] && [ "$(sha256sum <"$fp_dest" | cut -d' ' -f1)" = "$fp_want" ]; then
+ echo " $fp_name: cached, digest verified"
+ return 0
+ fi
+
+ echo " $fp_name: downloading $fp_url"
+ curl -sfL --retry 3 -o "$fp_dest.part" "$fp_url"
+
+ fp_got=$(sha256sum <"$fp_dest.part" | cut -d' ' -f1)
+ if [ "$fp_got" != "$fp_want" ]; then
+ rm -f "$fp_dest.part"
+ echo "$fp_name digest mismatch — refusing to run" >&2
+ echo " want $fp_want" >&2
+ echo " got $fp_got" >&2
+ echo " from $fp_url" >&2
+ exit 1
+ fi
+
+ chmod +x "$fp_dest.part"
+ mv "$fp_dest.part" "$fp_dest"
+ echo " $fp_name: downloaded, digest verified"
+}
+
+fetch_pinned minio \
+ "https://dl.min.io/server/minio/release/linux-$arch/archive/minio.$MINIO_RELEASE" \
+ "$minio_sha" "$cache/minio"
+fetch_pinned mc \
+ "https://dl.min.io/client/mc/release/linux-$arch/archive/mc.$MC_RELEASE" \
+ "$mc_sha" "$cache/mc"
+
+minio=$cache/minio
+mc=$cache/mc
+
+# ---------------------------------------------------------------------
+# Scratch state, and getting rid of it.
+#
+# Everything mutable this step creates lives under one directory: MinIO's
+# data, mc's config (MC_CONFIG_DIR, so mc never touches $HOME), the
+# policy document, and the server log. /tmp inside an amadan job is a
+# private tmpfs that the sandbox discards when the job ends, so this is
+# scratch in the strongest sense available.
+rundir=$(mktemp -d "${TMPDIR:-/tmp}/carlos-minio-contract-XXXXXX")
+MC_CONFIG_DIR=$rundir/mc
+export MC_CONFIG_DIR
+
+minio_pid=
+
+# The trap runs on the success path and on every failure path, because
+# `set -eu` exits through it — which is the point. A step that leaves a
+# MinIO listening on a shared box after it fails would poison every later
+# run on that worker, and the failure that caused it is exactly when
+# nobody is watching.
+cleanup() {
+ if [ -n "$minio_pid" ]; then
+ kill "$minio_pid" 2>/dev/null || true
+ wait "$minio_pid" 2>/dev/null || true
+ fi
+ rm -rf "$rundir"
+}
+trap cleanup EXIT INT TERM
+
+# Root credentials are minted per run rather than hardcoded. They are not
+# a secret — the server is loopback-only and lives for seconds — but
+# random ones make the identity check below meaningful: creds that only
+# this process knows cannot authenticate against somebody else's MinIO.
+root_user=root$$
+root_pass=$(head -c 18 /dev/urandom | od -An -tx1 | tr -d ' \n')
+
+# ---------------------------------------------------------------------
+# Start the server.
+#
+# The workflow uses a fixed port 9000 because its MinIO is in a container
+# with its own network. amadan jobs deliberately do NOT get --unshare-net
+# (they need the Go module proxy), so a job shares the host's network
+# namespace with the worker and with anything else on it. 9000 is a
+# popular port; assuming it is free is assuming something this step does
+# not know.
+#
+# So: pick a high port, and if MinIO cannot take it, pick another. The
+# liveness probe alone would not be enough to tell the two cases apart —
+# a foreign MinIO already on the port answers /health/live perfectly
+# happily, and we would then test somebody else's server. Requiring OUR
+# process to still be alive is what distinguishes "it started" from "the
+# port was taken"; `mc alias set` with the random root credentials above
+# is the second, independent check that the thing answering is ours.
+port=
+attempt=1
+while [ "$attempt" -le 5 ]; do
+ candidate=$((20000 + ($$ + attempt * 997) % 20000))
+ rm -rf "$rundir/data"
+ mkdir -p "$rundir/data"
+
+ MINIO_ROOT_USER=$root_user MINIO_ROOT_PASSWORD=$root_pass \
+ "$minio" server "$rundir/data" \
+ --address "127.0.0.1:$candidate" \
+ --console-address "127.0.0.1:0" \
+ >"$rundir/minio.log" 2>&1 &
+ minio_pid=$!
+
+ waited=0
+ while [ "$waited" -lt 30 ]; do
+ if ! kill -0 "$minio_pid" 2>/dev/null; then
+ break # died — almost certainly could not bind
+ fi
+ if curl -sf "http://127.0.0.1:$candidate/minio/health/live" >/dev/null; then
+ port=$candidate
+ break
+ fi
+ sleep 1
+ waited=$((waited + 1))
+ done
+
+ [ -n "$port" ] && break
+
+ kill "$minio_pid" 2>/dev/null || true
+ wait "$minio_pid" 2>/dev/null || true
+ minio_pid=
+ echo " minio did not come up on 127.0.0.1:$candidate, trying another port"
+ attempt=$((attempt + 1))
+done
+
+if [ -z "$port" ]; then
+ echo "minio never became healthy after 5 attempts; last log:" >&2
+ cat "$rundir/minio.log" >&2
+ exit 1
+fi
+
+echo " minio healthy on 127.0.0.1:$port"
+
+# ---------------------------------------------------------------------
+# The fixture: two buckets and a scoped user granted exactly one of them.
+#
+# Kept identical to the workflow's `mint scoped user` step — same bucket
+# names, same policy, same user — because the whole value of the test is
+# that cross-bucket denial is enforced by real IAM rather than by the
+# client politely not asking. Drift between this policy and the
+# workflow's is drift in what the two gates mean.
+"$mc" --quiet alias set local "http://127.0.0.1:$port" "$root_user" "$root_pass"
+"$mc" --quiet mb local/contract-bucket local/denied-bucket
+
+cat >"$rundir/policy.json" <<'EOF'
+{"Version": "2012-10-17", "Statement": [
+ {"Effect": "Allow", "Action": ["s3:ListBucket"], "Resource": ["arn:aws:s3:::contract-bucket"]},
+ {"Effect": "Allow", "Action": ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"], "Resource": ["arn:aws:s3:::contract-bucket/*"]}
+]}
+EOF
+
+"$mc" --quiet admin policy create local contract-only "$rundir/policy.json"
+"$mc" --quiet admin user add local scopeduser scopedsecret1
+"$mc" --quiet admin policy attach local contract-only --user scopeduser
+
+echo " buckets and scoped user ready"
+
+# ---------------------------------------------------------------------
+# Run it.
+#
+# Same env var names the test reads (contract_test.go's requireEnv calls),
+# and -count=1 for the same reason 10-gate uses it: never satisfy a gate
+# from a cached pass. Output goes to a file rather than through a pipe
+# because POSIX sh has no pipefail, and this step needs the test's own
+# exit status, not tee's.
+out=$rundir/test.log
+set +e
+CARLOS_MINIO_TEST=1 \
+ MINIO_ENDPOINT="http://127.0.0.1:$port" \
+ MINIO_SCOPED_KEY=scopeduser \
+ MINIO_SCOPED_SECRET=scopedsecret1 \
+ MINIO_BUCKET=contract-bucket \
+ MINIO_DENIED_BUCKET=denied-bucket \
+ go test ./internal/objstore/ -run TestDeliveredContractMinio -count=1 -v >"$out" 2>&1
+test_status=$?
+set -e
+
+cat "$out"
+
+if [ "$test_status" -ne 0 ]; then
+ exit "$test_status"
+fi
+
+# THE ASSERTION. Do not delete this because the test above already
+# "passed".
+#
+# TestDeliveredContractMinio skips itself unless CARLOS_MINIO_TEST=1, and
+# a skipped Go test exits 0. So every interesting way this step can break
+# — a typo'd env var, a MinIO that started but was never reachable, a
+# rename of the test, a future edit that drops the env block — produces a
+# green step that tested nothing at all. That is strictly worse than no
+# step, because it reports coverage this repo does not have.
+#
+# Requiring the parent test's PASS line to actually appear closes it. The
+# trailing " (" anchors on the parent rather than its subtests, whose
+# PASS lines are indented anyway.
+if ! grep -q '^--- PASS: TestDeliveredContractMinio (' "$out"; then
+ echo >&2
+ echo "the contract test did not PASS — it was skipped, filtered out, or renamed." >&2
+ echo "go test exited 0, which for this test means nothing was exercised: it" >&2
+ echo "skips unless CARLOS_MINIO_TEST=1. Check the env block above and that" >&2
+ echo "TestDeliveredContractMinio still exists in internal/objstore." >&2
+ exit 1
+fi
+
+echo "minio contract: PASS"
diff --git a/.gitignore b/.gitignore
index 0df49b9a3..6b209b585 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,3 +3,8 @@ platform
*.tfstate*
.terraform/
.claude/worktrees/
+
+# Pinned MinIO/mc binaries cached by .amadan/ci.d/30-minio-contract.
+# amadan reuses one workspace per repo across jobs, so an untracked
+# directory in the checkout is that step's cache — see the step for why.
+.amadan-cache/