diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b74943b..391ba90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -65,6 +65,8 @@ jobs: run: python3 testing/check-log-strings.py - name: Check no tested function's exit status is a log call's run: python3 testing/check-trailing-log.py + - name: Check nothing resolves the shared mutable test image + run: bash testing/check-image-scoping.sh # Hermetic: synthetic ping functions, no containers, ~45s. Lives beside # the other two so both runners gate on it identically — putting it in # only one would create exactly the drift check-ci-parity.sh exists to diff --git a/testing/README.md b/testing/README.md index 0cab275..f7d35e3 100644 --- a/testing/README.md +++ b/testing/README.md @@ -93,8 +93,21 @@ flight) never collide: - **Compose projects** are named `fipsci__`, so container, network, and volume names are all prefixed per run. - **Build images** are tagged `fips-test:` and - `fips-test-app:` (exported as `FIPS_TEST_IMAGE` / - `FIPS_TEST_APP_IMAGE` for the compose consumers). + `fips-test-app:`, exported as `FIPS_TEST_IMAGE` / + `FIPS_TEST_APP_IMAGE`, and **every** compose file and suite script reads + those. The run does not write `fips-test:latest` at all: a bridge back to + that shared mutable name would let a consumer that had been missed keep + working while resolving whichever concurrent run wrote the tag last. + `:latest` stays the hand-build name, produced by + `testing/scripts/build.sh`, and remains the default every consumer falls + back to when the variables are unset. +- **The build context** is a per-run copy at `testing/docker-/`, + exported as `FIPS_BUILD_CONTEXT`. It is absolute because compose resolves + a relative build context against the compose file's own directory rather + than the working directory. `testing/docker/` is the hand-run context and + a CI run does not write to it. Without this, two runs race on the contents + of one directory and either can build a correctly-per-run-tagged image + from the other's binaries. - Each parallel chaos child gets a unique, non-overlapping `/24` in `10.30.x` (via the sim `--subnet` override). `10.30.x` sits outside Docker's default address pool and the fixed-subnet suites' `172.x` diff --git a/testing/check-image-scoping.sh b/testing/check-image-scoping.sh new file mode 100755 index 0000000..1190184 --- /dev/null +++ b/testing/check-image-scoping.sh @@ -0,0 +1,102 @@ +#!/bin/bash +# ── Test-image scoping guard ──────────────────────────────────────────────── +# A local CI run builds fips-test: and hands it to every suite through +# FIPS_TEST_IMAGE / FIPS_TEST_APP_IMAGE. It does NOT write fips-test:latest, +# deliberately: while a bridge back to that shared mutable name existed, a +# consumer that named it directly kept working while resolving whichever +# concurrent run wrote the tag last, and the verdict was then recorded against +# a commit whose binaries had not run. Nothing in the harness compares a +# running container's binary against the commit under test, so that failure is +# silent and leaves no artifact. +# +# The bridge is gone, so a consumer that names the shared tag now fails loudly +# at run time. This guard is the static half: it stops one being reintroduced, +# because the reintroduction is invisible on any host where a hand build has +# left an fips-test:latest lying around. +# +# What counts as a violation: a reference to the shared tag that is neither a +# comment, nor a documented default of the ${FIPS_TEST_IMAGE:-...} form, nor in +# a file on the allowlist below. +# +# Exit 0 = clean. Exit 1 = an unexpected reference. Exit 2 = the guard could +# not run; never treated as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" + +# Files permitted to name the shared tag outright, each for a stated reason. +# Every one of these is a hand-run path: none is reachable from ci-local.sh +# with FIPS_TEST_IMAGE set. +# +# scripts/build.sh the developer build; :latest IS its product +# sidecar/scripts/test-sidecar.sh hand build, behind its --skip-build guard, +# which ci-local always passes +# static/scripts/iperf-compare-refs.sh hand-run A/B against two refs +# ci-cleanup.sh last-resort name for the image ip(8) runs in, +# after the caller's and the run's have failed +# check-image-scoping.sh this guard, which has to name what it looks +# for; it resolves no image +ALLOWED=( + "scripts/build.sh" + "sidecar/scripts/test-sidecar.sh" + "static/scripts/iperf-compare-refs.sh" + "ci-cleanup.sh" + "check-image-scoping.sh" +) + +if ! command -v git >/dev/null 2>&1; then + echo "check-image-scoping: git not available, cannot sweep" >&2 + exit 2 +fi +if [[ ! -d "$SCRIPT_DIR" ]]; then + echo "check-image-scoping: $SCRIPT_DIR missing" >&2 + exit 2 +fi + +# Tracked files only, and no documentation: prose naming the tag is describing +# it, not resolving it. +mapfile -t files < <(git -C "$SCRIPT_DIR/.." ls-files -- testing/ | grep -vE '\.md$') +if [[ ${#files[@]} -eq 0 ]]; then + echo "check-image-scoping: no tracked files under testing/, refusing to pass" >&2 + exit 2 +fi + +violations=0 +for f in "${files[@]}"; do + rel="${f#testing/}" + skip=0 + for a in "${ALLOWED[@]}"; do + [[ "$rel" == "$a" ]] && skip=1 && break + done + [[ $skip -eq 1 ]] && continue + [[ -f "$SCRIPT_DIR/../$f" ]] || continue + + while IFS= read -r hit; do + n="${hit%%:*}" + text="${hit#*:}" + # A comment line is describing the tag, not resolving it. Shell, python + # and yaml all use #; nothing under testing/ uses // for comments. + [[ "$text" =~ ^[[:space:]]*# ]] && continue + # The documented indirection: the shared tag as a FALLBACK, which is + # what a bare hand run is supposed to get. + [[ "$text" == *'${FIPS_TEST_IMAGE:-fips-test:latest}'* ]] && continue + [[ "$text" == *'${FIPS_TEST_APP_IMAGE:-fips-test-app:latest}'* ]] && continue + [[ "$text" == *'os.environ.get("FIPS_TEST_IMAGE", "fips-test:latest")'* ]] && continue + echo "FAIL $f:$n names the shared test image directly:" + echo " $text" + violations=$((violations + 1)) + done < <(grep -n 'fips-test:latest\|fips-test-app:latest' "$SCRIPT_DIR/../$f" 2>/dev/null) +done + +if [[ $violations -gt 0 ]]; then + echo "" + echo "check-image-scoping: $violations reference(s) to the shared mutable test image." + echo "Read FIPS_TEST_IMAGE (default \${FIPS_TEST_IMAGE:-fips-test:latest}) instead." + echo "A run that resolves the shared tag can execute a concurrent run's binaries" + echo "and record the verdict against this commit." + exit 1 +fi + +echo "check-image-scoping: no unscoped references to the shared test image" +exit 0 diff --git a/testing/ci-cleanup.sh b/testing/ci-cleanup.sh index 0f25243..fa32a86 100755 --- a/testing/ci-cleanup.sh +++ b/testing/ci-cleanup.sh @@ -66,9 +66,15 @@ RUN_ID="" # broad default: every CI run IMAGES="" VETH_SUFFIXES="" # empty AND no --run-id: every simulation veth name # ip(8) runs inside this image, the same way the simulation creates the -# interfaces, so the reap works wherever the simulation does. The chaos -# simulation builds it, and it carries iproute2. -VETH_IMAGE="fips-test:latest" +# interfaces, so the reap works wherever the simulation does. Any fips test +# image will do; it is wanted only for its iproute2. +# +# Empty here and resolved after the argument loop, because the resolution has +# to consider --veth-image. The old default of fips-test:latest is no longer +# safe on its own: ci-local.sh does not write that tag, so on a host that has +# only ever run the harness it need not exist at all, and the reap this script +# advertises as the remedy for orphaned interfaces would be a permanent no-op. +VETH_IMAGE="" while [[ $# -gt 0 ]]; do case "$1" in @@ -77,11 +83,26 @@ while [[ $# -gt 0 ]]; do --run-id) RUN_ID="$2"; shift 2 ;; --images) IMAGES="$2"; shift 2 ;; --veth-suffixes) VETH_SUFFIXES="$2"; shift 2 ;; + --veth-image) VETH_IMAGE="$2"; shift 2 ;; -h|--help) sed -n '2,/^set /{ /^set /d; s/^# \?//; p }' "$0"; exit 0 ;; *) echo "Unknown option: $1" >&2; exit 2 ;; esac done +# Resolve the image to run ip(8) in: the caller's choice, then the run's own +# image, then any surviving fips test image. That last fallback is what keeps +# an unscoped `ci-local.sh --reap` working — it execs this script from inside +# its own argument loop, before the run identity is exported, so it can pass +# neither. The empty case is handled at the point of use, which already warns +# and skips rather than failing the sweep. +if [[ -z "$VETH_IMAGE" ]]; then + VETH_IMAGE="${FIPS_TEST_IMAGE:-}" +fi +if [[ -z "$VETH_IMAGE" ]] && command -v docker >/dev/null 2>&1; then + VETH_IMAGE="$(timeout 10 docker image ls --format '{{.Repository}}:{{.Tag}}' fips-test 2>/dev/null | head -n1)" +fi +[[ -z "$VETH_IMAGE" ]] && VETH_IMAGE="fips-test:latest" + if ! command -v docker >/dev/null 2>&1; then # No docker, nothing to reap. exit 0 @@ -217,8 +238,8 @@ reap_veths() { [[ -z "$pattern" ]] && return 0 # Without the image there is no way to run ip(8). Orphans can outlive it — # `docker image prune -a`, a build host that prunes between runs, or a run - # aborted before ci-local.sh retags :latest all remove it while interfaces - # are still up — so this is a real skip, not "nothing was ever run here". + # whose per-run image was already reaped all remove it while interfaces are + # still up — so this is a real skip, not "nothing was ever run here". if ! docker image inspect "$VETH_IMAGE" >/dev/null 2>&1; then veth_warn "image $VETH_IMAGE not present, cannot run ip(8)" return 0 diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 670795e..3c01dd8 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -280,8 +280,9 @@ record() { # # This script may be preempted (a CI worker sends SIGTERM, waits ~30s, then # SIGKILL) so it can restart on a newer tip. To make that safe: -# * every docker resource is namespaced to THIS run (compose project prefix -# + per-run image tags) so a restart never collides with a dying run; +# * every docker resource is namespaced to THIS run (compose project prefix, +# per-run image tags, per-run build context) so a restart never collides +# with a dying run, and neither does a concurrent one; # * a trap tears down everything this run created on signal/exit, bounded by # `timeout` so a stuck `down` cannot wedge the trap (SIGKILL is the backstop). @@ -916,13 +917,11 @@ run_integration() { docker build -t "$CI_IMAGE_APP" --label "$CI_LABEL" --label "$CI_LABEL_RUN" \ -f "$CI_BUILD_CONTEXT/Dockerfile.app" "$CI_BUILD_CONTEXT" --quiet \ || { record "docker-build-app" 1; return; } - # The remaining bridge back to the shared mutable tag, for any consumer not - # yet reading FIPS_TEST_IMAGE. Removed in the following commit, which is the - # step that makes a missed consumer fail loudly instead of silently - # resolving another run's binaries. Both builds have succeeded by here, so - # :latest never points at a half-built image. - docker tag "$CI_IMAGE_TEST" fips-test:latest - docker tag "$CI_IMAGE_APP" fips-test-app:latest + # Deliberately NOT retagged to fips-test:latest. Every consumer reads + # FIPS_TEST_IMAGE, and a bridge back to the shared mutable name would let a + # consumer that does not keep working silently — resolving whichever + # concurrent run wrote the tag last, and recording that run's binaries under + # this run's commit. Without the bridge a missed consumer fails loudly. # Single suite mode if [[ -n "$ONLY_SUITE" ]]; then @@ -1141,6 +1140,17 @@ run_ci_parity() { record "ci-parity" $rc } +# Nothing may resolve the shared mutable test image. This run does not write +# fips-test:latest, so a consumer naming it fails loudly here and now — but only +# on a host with no hand-built copy lying around, which is not a property to +# rely on. This is the static half of that. +run_image_scoping() { + local rc=0 + info "[image-scoping] Checking that nothing names the shared test image" + "$SCRIPT_DIR/check-image-scoping.sh" || rc=$? + record "image-scoping" $rc +} + # Every daemon log string a test matches on must still be emitted by src/. # A stale one does not fail — it stops observing, and an expect-zero assertion # built on it then passes for the wrong reason. @@ -1194,6 +1204,7 @@ main() { run_ci_parity run_log_strings run_trailing_log + run_image_scoping run_wait_converge if [[ "$TEST_ONLY" == true ]]; then