From cbbdf2c13c9c9e0250c4431d772f13d408ce9136 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 26 Jul 2026 15:37:50 +0000 Subject: [PATCH] Stop the firewall and ACL suites rebuilding an image their caller supplied Both suites ran `docker compose up -d --build` unconditionally, and their compose files carry both a build context and an image name, so the flag rebuilt and retagged the shared test image from the shared build context on every run. Their --skip-build flag did not cover it: it guarded only the Rust build above. Under a harness that has already built the image and handed it over, that rebuild can only replace the binaries under test with whatever the shared context currently holds. The nat scripts had the mirror-image problem. Their image guard built a replacement whenever the image was missing, which is right by hand and wrong under a harness: when the caller names an image it built, a miss means something upstream is broken, and manufacturing a substitute hides that. They now fail loudly in that case and keep building only on the hand path. The mesh-lab guard reads the same variable and its build hint moves to the script that still produces the shared tag. --- testing/acl-allowlist/test.sh | 10 +++++++++- testing/firewall/test.sh | 10 +++++++++- testing/mesh-lab/run-loop.sh | 7 ++++--- testing/nat/scripts/nat-test.sh | 17 ++++++++++++++--- testing/nat/scripts/nostr-relay-test.sh | 17 ++++++++++++++--- testing/nat/scripts/stun-faults-test.sh | 17 ++++++++++++++--- 6 files changed, 64 insertions(+), 14 deletions(-) diff --git a/testing/acl-allowlist/test.sh b/testing/acl-allowlist/test.sh index 1bf9905..79f3fc3 100755 --- a/testing/acl-allowlist/test.sh +++ b/testing/acl-allowlist/test.sh @@ -185,7 +185,15 @@ log "Generating ACL allowlist fixtures" log "Starting ACL allowlist harness" docker compose -f "$COMPOSE_FILE" down >/dev/null 2>&1 || true -docker compose -f "$COMPOSE_FILE" up -d --build +# --build only on the hand path. Under a harness, --skip-build means the caller +# has already built the image this compose file names, and rebuilding it here +# would overwrite that image from whatever the shared build context happens to +# hold — which is how a suite ends up certifying binaries it was never given. +if [ "$SKIP_BUILD" = false ]; then + docker compose -f "$COMPOSE_FILE" up -d --build +else + docker compose -f "$COMPOSE_FILE" up -d +fi log "Waiting for expected peer convergence" wait_for_peers_exact fips-acl-container-a${FIPS_CI_NAME_SUFFIX:-} 3 40 diff --git a/testing/firewall/test.sh b/testing/firewall/test.sh index 833e9c8..418af19 100755 --- a/testing/firewall/test.sh +++ b/testing/firewall/test.sh @@ -191,7 +191,15 @@ log "Generating firewall fixtures" log "Starting firewall harness" docker compose -f "$COMPOSE_FILE" down >/dev/null 2>&1 || true -docker compose -f "$COMPOSE_FILE" up -d --build +# --build only on the hand path. Under a harness, --skip-build means the caller +# has already built the image this compose file names, and rebuilding it here +# would overwrite that image from whatever the shared build context happens to +# hold — which is how a suite ends up certifying binaries it was never given. +if [ "$SKIP_BUILD" = false ]; then + docker compose -f "$COMPOSE_FILE" up -d --build +else + docker compose -f "$COMPOSE_FILE" up -d +fi log "Waiting for fips0 on both nodes" wait_for_fips0 "$CONTAINER_A" 40 diff --git a/testing/mesh-lab/run-loop.sh b/testing/mesh-lab/run-loop.sh index 9fa9487..a640baa 100755 --- a/testing/mesh-lab/run-loop.sh +++ b/testing/mesh-lab/run-loop.sh @@ -135,9 +135,10 @@ require_docker() { } require_test_image() { - if ! docker image inspect fips-test:latest >/dev/null 2>&1; then - echo "ERROR: fips-test:latest not present" >&2 - echo "Build it once with: bash testing/ci-local.sh --build-only" >&2 + local img="${FIPS_TEST_IMAGE:-fips-test:latest}" + if ! docker image inspect "$img" >/dev/null 2>&1; then + echo "ERROR: $img not present" >&2 + echo "Build it once with: bash testing/scripts/build.sh" >&2 exit 2 fi } diff --git a/testing/nat/scripts/nat-test.sh b/testing/nat/scripts/nat-test.sh index 3628e87..c1eea64 100755 --- a/testing/nat/scripts/nat-test.sh +++ b/testing/nat/scripts/nat-test.sh @@ -262,10 +262,21 @@ dump_lan_diagnostics() { trap 'echo ""; echo "NAT test interrupted"; cleanup; exit 130' INT TERM require_test_image() { - if ! docker image inspect fips-test:latest >/dev/null 2>&1; then - echo "fips-test:latest not found; building test image" - "$BUILD_SCRIPT" + local img="${FIPS_TEST_IMAGE:-fips-test:latest}" + if docker image inspect "$img" >/dev/null 2>&1; then + return 0 fi + # Building here is right for a hand run and wrong under a harness. When + # FIPS_TEST_IMAGE is set the caller has already built the image it named, so + # a miss means something upstream is broken; building a substitute would + # hide that and run binaries nobody asked for. + if [ -n "${FIPS_TEST_IMAGE:-}" ]; then + echo "ERROR: $img not present, and FIPS_TEST_IMAGE names the caller's own image" >&2 + echo "The harness that set it is expected to have built it." >&2 + exit 1 + fi + echo "$img not found; building test image" + "$BUILD_SCRIPT" } require_docker_daemon() { diff --git a/testing/nat/scripts/nostr-relay-test.sh b/testing/nat/scripts/nostr-relay-test.sh index 812d879..718120d 100755 --- a/testing/nat/scripts/nostr-relay-test.sh +++ b/testing/nat/scripts/nostr-relay-test.sh @@ -49,10 +49,21 @@ require_docker_daemon() { } require_test_image() { - if ! docker image inspect fips-test:latest >/dev/null 2>&1; then - echo "fips-test:latest not found; building test image" - "$BUILD_SCRIPT" + local img="${FIPS_TEST_IMAGE:-fips-test:latest}" + if docker image inspect "$img" >/dev/null 2>&1; then + return 0 fi + # Building here is right for a hand run and wrong under a harness. When + # FIPS_TEST_IMAGE is set the caller has already built the image it named, so + # a miss means something upstream is broken; building a substitute would + # hide that and run binaries nobody asked for. + if [ -n "${FIPS_TEST_IMAGE:-}" ]; then + echo "ERROR: $img not present, and FIPS_TEST_IMAGE names the caller's own image" >&2 + echo "The harness that set it is expected to have built it." >&2 + exit 1 + fi + echo "$img not found; building test image" + "$BUILD_SCRIPT" } dump_diagnostics() { diff --git a/testing/nat/scripts/stun-faults-test.sh b/testing/nat/scripts/stun-faults-test.sh index c9dbbb5..bc508fe 100755 --- a/testing/nat/scripts/stun-faults-test.sh +++ b/testing/nat/scripts/stun-faults-test.sh @@ -60,10 +60,21 @@ require_docker_daemon() { } require_test_image() { - if ! docker image inspect fips-test:latest >/dev/null 2>&1; then - echo "fips-test:latest not found; building test image" - "$BUILD_SCRIPT" + local img="${FIPS_TEST_IMAGE:-fips-test:latest}" + if docker image inspect "$img" >/dev/null 2>&1; then + return 0 fi + # Building here is right for a hand run and wrong under a harness. When + # FIPS_TEST_IMAGE is set the caller has already built the image it named, so + # a miss means something upstream is broken; building a substitute would + # hide that and run binaries nobody asked for. + if [ -n "${FIPS_TEST_IMAGE:-}" ]; then + echo "ERROR: $img not present, and FIPS_TEST_IMAGE names the caller's own image" >&2 + echo "The harness that set it is expected to have built it." >&2 + exit 1 + fi + echo "$img not found; building test image" + "$BUILD_SCRIPT" } dump_diagnostics() {