From 5a5faa0857b6cf5e8e585b5fb65e7474b8f667e5 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 09:11:39 +0000 Subject: [PATCH 1/7] test(nat): contain strfry relay aborts in the NAT lab The NAT-lab suites (cone, symmetric, lan, STUN faults and nostr publish/consume) share one strfry relay, and it has aborted in several runs. Three things made each abort cost a whole run and a misdirected diagnosis: only the publish/consume suite said what state the relay was in, the relay image moved with upstream's latest tag, and the relay never restarted. State the relay's condition in every NAT-lab suite. relay_verdict moves into testing/lib/relay-verdict.sh, taking the container as an argument, and is called first in every NAT-lab failure dump and on each success path, where a relay event the assertions survived is noted rather than made a failure. Before this, the cone, symmetric and lan dumps named the nodes and the network instead: hundreds of lines of socket tables and router counters, with the relay's crash lines unlabelled in an 80-line log tail. The STUN fault dump now also carries the relay's log, which it did not include. Pin the relay's strfry build. Dockerfile.app takes the strfry image as a build argument, defaulting to the same latest tag, so the user-facing example is unchanged, and the NAT lab's compose file passes the current multi-arch index digest. Which build a run exercised was never recorded before. This stops the drift; it does not select a build that does not abort, since upstream publishes no other tag to choose from. Restart the relay when it aborts. The relay ran with restart "no", so one abort left every suite sharing it without a relay for the rest of the run. It now restarts on failure up to three times, so a relay that keeps aborting still ends up exited and is reported rather than hidden in a crash loop. The restart policy is the containment. The verdict says the relay restarted under that policy and prints the fault lines from its log, which spans restarts of the same container, so a rescued run still names the event. The relay service also gains init: true, only so the restart can be exercised by an injected abort. It changes the relay's PID 1 from strfry (started with exec in the image's entrypoint) to docker's init, with strfry as its child. Without it, a SIGABRT sent with docker kill to strfry as PID 1 logged "caught a signal: SIGABRT" and left the container running with no restart, so that injection could not show the policy working. With the init, strfry signalled from inside the container exits it non-zero, which is what the real aborts did: clients saw the relay vanish. Those real aborts would restart under the policy with or without the init. Injected aborts (strfry signalled from inside the container the moment both cone nodes had connected) restarted the relay once each time; in eight of nine cone runs both nodes reconnected and peered 8 to 16 s after the abort. In the ninth, the initiator's offer went out in the second between its reconnect and the responder's, was lost, and the 30 s answer timeout pushed peering past the 45 s wait, so restart shortens the outage but does not guarantee the run. Without the restart policy the same injection left the relay exited and the cone suite timed out waiting for its peer, with no line in the dump naming the relay. --- examples/sidecar-nostr-relay/Dockerfile.app | 6 +- testing/lib/relay-verdict.sh | 85 +++++++++++++++++++++ testing/nat/README.md | 3 +- testing/nat/docker-compose.yml | 21 ++++- testing/nat/scripts/nat-test.sh | 26 +++++++ testing/nat/scripts/nostr-relay-test.sh | 66 ++-------------- testing/nat/scripts/stun-faults-test.sh | 18 ++++- 7 files changed, 160 insertions(+), 65 deletions(-) create mode 100644 testing/lib/relay-verdict.sh diff --git a/examples/sidecar-nostr-relay/Dockerfile.app b/examples/sidecar-nostr-relay/Dockerfile.app index 5f6348f7..ae59129a 100644 --- a/examples/sidecar-nostr-relay/Dockerfile.app +++ b/examples/sidecar-nostr-relay/Dockerfile.app @@ -5,7 +5,11 @@ # the final stage. Copying the strfry binary into a glibc-based image such as # debian:bookworm-slim causes "not found" at exec time because the musl dynamic # linker (/lib/ld-musl-*.so.1) and its shared libraries are absent there. -FROM ghcr.io/hoytech/strfry:latest AS strfry +# +# STRFRY_IMAGE lets a caller pin the strfry build. The default follows the +# upstream `latest` tag; the NAT test lab passes a digest. +ARG STRFRY_IMAGE=ghcr.io/hoytech/strfry:latest +FROM ${STRFRY_IMAGE} AS strfry FROM alpine:3.18 diff --git a/testing/lib/relay-verdict.sh b/testing/lib/relay-verdict.sh new file mode 100644 index 00000000..90784745 --- /dev/null +++ b/testing/lib/relay-verdict.sh @@ -0,0 +1,85 @@ +#!/bin/bash +# Shared relay verdict for the NAT-lab suites. +# +# Source this file to get relay_verdict(). +# +# Usage: +# source "$ROOT_DIR/testing/lib/relay-verdict.sh" +# relay_verdict +# +# The lab's Nostr relay is a third-party container (strfry), and it has died +# mid-run: once on a SIGSEGV twelve milliseconds after both nodes had +# connected, and several times since on an internal assertion that aborts it +# the instant two clients connect together. Every symptom under that is a +# correct report of a dead relay: subscriptions dropped, no advert consumed, +# empty peer lists, and a run that fails on the peer-count wait. That verdict +# is indistinguishable from a product failure unless the relay's state is +# stated, and the only evidence of the real cause was one container-log line +# far above the summary. relay_verdict says whose failure it was, in a line a +# reader of the output can find. + +# State the relay's own condition, in its own words. +# +# This does not decide the run. It returns 0 when the run had a relay event +# (the relay is gone, not running, restarted, faulted, or its health could not +# be read) and 1 when the relay was running with no fault in its log. Callers +# use it first in a failure dump, and on the success path to note a relay +# event the assertions survived. +# +# A relay whose state or log cannot be read has not been shown to be healthy, +# so that case is reported as unestablished rather than as "not the relay". +relay_verdict() { + local container="$1" + local state="" status="" exit_code="" restarts="" logs="" + local faults="caught a signal|SIGSEGV|SIGABRT|terminate called" + + state="$(docker inspect \ + -f '{{.State.Status}} {{.State.ExitCode}} {{.RestartCount}}' \ + "$container" 2>/dev/null)" || state="" + + if [ -z "$state" ]; then + echo "RELAY FAILURE: $container is gone; whatever this run" \ + "asserted about peering happened without a relay" >&2 + return 0 + fi + + read -r status exit_code restarts <<<"$state" + + if [ "$status" != "running" ]; then + echo "RELAY FAILURE: $container is $status (exit $exit_code);" \ + "the assertions in this run are downstream of that, not of the" \ + "nodes" >&2 + return 0 + fi + + # docker logs spans every restart of the same container, so the fault + # lines of the run that died are still readable here. + if [ "${restarts:-0}" -gt 0 ]; then + echo "RELAY FAILURE: $container restarted $restarts time(s) during" \ + "this run under its restart policy; the nodes lost their" \ + "subscriptions each time and had to reconnect" >&2 + if logs="$(docker logs "$container" 2>&1)"; then + grep -E "$faults" <<<"$logs" >&2 || true + else + echo " (its logs could not be read, so the fault lines are" \ + "not shown)" >&2 + fi + return 0 + fi + + if ! logs="$(docker logs "$container" 2>&1)"; then + echo "RELAY HEALTH NOT ESTABLISHED: could not read" \ + "$container's logs, so a crash cannot be ruled in or out" >&2 + return 0 + fi + + if grep -Eq "$faults" <<<"$logs"; then + echo "RELAY FAILURE: $container faulted during this run:" >&2 + grep -E "$faults" <<<"$logs" >&2 + return 0 + fi + + echo "relay: $container running, no fault in its log — this run's" \ + "verdict is about the nodes" + return 1 +} diff --git a/testing/nat/README.md b/testing/nat/README.md index dca37fa4..1af14b1e 100644 --- a/testing/nat/README.md +++ b/testing/nat/README.md @@ -73,7 +73,8 @@ Run one scenario: - `stun/` - minimal STUN binding responder - `relay/` - - local `strfry` config + - local `strfry` config. The relay image's strfry build is pinned by digest + in `docker-compose.yml` (`STRFRY_IMAGE`); bump it there deliberately. - `scripts/generate-configs.sh` - derives ephemeral identities and writes per-scenario FIPS configs - `scripts/setup-topology.sh` diff --git a/testing/nat/docker-compose.yml b/testing/nat/docker-compose.yml index f3dddb6d..93c2fb39 100644 --- a/testing/nat/docker-compose.yml +++ b/testing/nat/docker-compose.yml @@ -34,8 +34,27 @@ services: build: context: ../.. dockerfile: examples/sidecar-nostr-relay/Dockerfile.app + args: + # Pinned so the relay build does not move underneath the harness. + # Upstream publishes only `latest`; this is its multi-arch index + # digest as of 2026-09-19 (amd64 and arm64 manifests built + # 2026-09-04). Bump it deliberately, reading the new digest with + # `docker buildx imagetools inspect ghcr.io/hoytech/strfry:latest`. + STRFRY_IMAGE: ghcr.io/hoytech/strfry@sha256:36f1886d185a88ca57c66ebe52e6e9e8428dac2486eea0a5d50ff934f18b60c3 container_name: fips-nat-relay${FIPS_CI_NAME_SUFFIX:-} - restart: "no" + # A third-party relay abort should not fail a whole run, so the relay comes + # back on its own; relay_verdict still reports the restart and the fault + # lines. Bounded so a relay that aborts repeatedly ends up exited, and the + # verdict says so, rather than a crash loop hiding it. + # + # `init` is here so the restart can be tested, not for the restart itself. + # It makes docker's init PID 1, with strfry as its child, where strfry was + # PID 1 before. A test SIGABRT sent with `docker kill` to strfry as PID 1 + # ran its handler and left the container running, so it never exercised + # the restart. With an init, strfry signalled from inside the container + # exits it non-zero, as the relay aborts seen in the lab did. + restart: on-failure:3 + init: true volumes: - relay-data:/usr/src/app/strfry-db - ./relay/strfry.conf:/usr/src/app/strfry.conf:ro diff --git a/testing/nat/scripts/nat-test.sh b/testing/nat/scripts/nat-test.sh index cbf7c7dc..aec7aa72 100755 --- a/testing/nat/scripts/nat-test.sh +++ b/testing/nat/scripts/nat-test.sh @@ -9,6 +9,7 @@ BUILD_SCRIPT="$ROOT_DIR/testing/scripts/build.sh" GENERATE_SCRIPT="$SCRIPT_DIR/generate-configs.sh" TOPOLOGY_SCRIPT="$SCRIPT_DIR/setup-topology.sh" WAIT_LIB="$ROOT_DIR/testing/lib/wait-converge.sh" +RELAY_LIB="$ROOT_DIR/testing/lib/relay-verdict.sh" # Must track generate-configs.sh's OUTPUT_DIR and the compose bind-mounts: the # npubs are read back here after the containers are up, so reading a different # directory than the one the generator wrote pings an npub no node owns. @@ -42,6 +43,10 @@ if [ -n "${FIPS_NAT_EXTRA_COMPOSE:-}" ]; then fi source "$WAIT_LIB" +# shellcheck disable=SC1090 +source "$RELAY_LIB" + +RELAY_CONTAINER="fips-nat-relay${FIPS_CI_NAME_SUFFIX:-}" cleanup() { "${COMPOSE[@]}" --profile cone --profile symmetric --profile lan \ @@ -249,6 +254,9 @@ dump_stun_udp_probe() { } dump_cone_diagnostics() { + echo "" + echo "=== relay verdict ===" + relay_verdict "$RELAY_CONTAINER" || true echo "" echo "=== cone diagnostics ===" dump_fips_state fips-nat-cone-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}.30 7777 ${NAT_WAN}.40 3478 @@ -266,6 +274,9 @@ dump_cone_diagnostics() { } dump_symmetric_diagnostics() { + echo "" + echo "=== relay verdict ===" + relay_verdict "$RELAY_CONTAINER" || true echo "" echo "=== symmetric diagnostics ===" dump_fips_state fips-nat-symmetric-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}.30 7777 ${NAT_WAN}.40 3478 @@ -277,6 +288,9 @@ dump_symmetric_diagnostics() { } dump_lan_diagnostics() { + echo "" + echo "=== relay verdict ===" + relay_verdict "$RELAY_CONTAINER" || true echo "" echo "=== lan diagnostics ===" dump_fips_state fips-nat-lan-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_LAN}.30 7777 ${NAT_LAN}.40 3478 @@ -434,6 +448,15 @@ ping_peer() { fi } +# A relay that faulted while a scenario's assertions still passed is a finding +# about the relay, not about the scenario, so it is reported and not made a +# failure: the scenario proved what it set out to prove. +note_relay_event() { + if relay_verdict "$RELAY_CONTAINER"; then + echo "NOTE: the assertions above passed despite that." >&2 + fi +} + run_cone() { echo "=== NAT lab: cone ===" cleanup @@ -474,6 +497,7 @@ run_cone() { dump_cone_diagnostics return 1 } + note_relay_event cleanup } @@ -519,6 +543,7 @@ run_symmetric() { dump_symmetric_diagnostics return 1 } + note_relay_event cleanup } @@ -561,6 +586,7 @@ run_lan() { dump_lan_diagnostics return 1 } + note_relay_event # Skip the final teardown when the mesh-lab harness wraps this # script: it needs to docker-logs the containers before teardown, # and will run its own cleanup after capture. Failure paths above diff --git a/testing/nat/scripts/nostr-relay-test.sh b/testing/nat/scripts/nostr-relay-test.sh index a075d05a..8cffbb2c 100755 --- a/testing/nat/scripts/nostr-relay-test.sh +++ b/testing/nat/scripts/nostr-relay-test.sh @@ -21,6 +21,7 @@ ROOT_DIR="$(cd "$NAT_DIR/../.." && pwd)" BUILD_SCRIPT="$ROOT_DIR/testing/scripts/build.sh" GENERATE_SCRIPT="$SCRIPT_DIR/generate-configs.sh" WAIT_LIB="$ROOT_DIR/testing/lib/wait-converge.sh" +RELAY_LIB="$ROOT_DIR/testing/lib/relay-verdict.sh" # Must track generate-configs.sh's OUTPUT_DIR and the compose bind-mounts. CONFIG_DIR="$NAT_DIR/generated-configs${FIPS_CI_NAME_SUFFIX:-}" @@ -52,6 +53,8 @@ RELAY_CONTAINER="fips-nat-relay${FIPS_CI_NAME_SUFFIX:-}" # shellcheck disable=SC1090 source "$WAIT_LIB" +# shellcheck disable=SC1090 +source "$RELAY_LIB" cleanup() { "${COMPOSE[@]}" --profile "$PROFILE" down -v --remove-orphans \ @@ -85,69 +88,10 @@ require_test_image() { "$BUILD_SCRIPT" } -# State the relay's own condition, in its own words, before anything else. -# -# The relay is a third-party container and it has died mid-run before: strfry -# took a SIGSEGV twelve milliseconds after both nodes had connected, and every -# symptom under that was a correct report of a dead relay — subscriptions -# dropped, no advert consumed, empty peer lists, and a run that failed on the -# peer-count wait. That verdict is indistinguishable from a product failure -# unless the relay's state is stated, and the only evidence of the real cause -# was one container-log line ninety lines above the summary. This does not -# decide the run; it says whose failure it was, in a line a reader of the -# output can find. -# -# A relay whose state or log cannot be read has not been shown to be healthy, -# so that case is reported as unestablished rather than as "not the relay". -relay_verdict() { - local state="" status="" exit_code="" restarts="" logs="" - - state="$(docker inspect \ - -f '{{.State.Status}} {{.State.ExitCode}} {{.RestartCount}}' \ - "$RELAY_CONTAINER" 2>/dev/null)" || state="" - - if [ -z "$state" ]; then - echo "RELAY FAILURE: $RELAY_CONTAINER is gone; whatever this run" \ - "asserted about peering happened without a relay" >&2 - return 0 - fi - - read -r status exit_code restarts <<<"$state" - - if [ "$status" != "running" ]; then - echo "RELAY FAILURE: $RELAY_CONTAINER is $status (exit $exit_code);" \ - "the assertions in this run are downstream of that, not of the" \ - "nodes" >&2 - return 0 - fi - - if [ "${restarts:-0}" -gt 0 ]; then - echo "RELAY FAILURE: $RELAY_CONTAINER has restarted $restarts time(s)" \ - "during this run; the nodes lost their subscriptions with it" >&2 - return 0 - fi - - if ! logs="$(docker logs "$RELAY_CONTAINER" 2>&1)"; then - echo "RELAY HEALTH NOT ESTABLISHED: could not read" \ - "$RELAY_CONTAINER's logs, so a crash cannot be ruled in or out" >&2 - return 0 - fi - - if grep -Eq "caught a signal|SIGSEGV|SIGABRT|terminate called" <<<"$logs"; then - echo "RELAY FAILURE: $RELAY_CONTAINER faulted during this run:" >&2 - grep -E "caught a signal|SIGSEGV|SIGABRT|terminate called" <<<"$logs" >&2 - return 0 - fi - - echo "relay: $RELAY_CONTAINER running, no fault in its log — this run's" \ - "verdict is about the nodes" - return 1 -} - dump_diagnostics() { echo "" echo "=== relay verdict ===" - relay_verdict || true + relay_verdict "$RELAY_CONTAINER" || true echo "" echo "=== nostr publish/consume diagnostics ===" for c in "$NODE_A" "$NODE_B" "$RELAY_CONTAINER"; do @@ -572,7 +516,7 @@ run_test() { # A relay that faulted while the assertions still passed is a finding # about the relay, not about this run, so it is reported and not made a # failure: the suite proved what it set out to prove. - if relay_verdict; then + if relay_verdict "$RELAY_CONTAINER"; then echo "NOTE: the assertions above passed despite that." >&2 fi diff --git a/testing/nat/scripts/stun-faults-test.sh b/testing/nat/scripts/stun-faults-test.sh index 5a5fafea..149d5b33 100755 --- a/testing/nat/scripts/stun-faults-test.sh +++ b/testing/nat/scripts/stun-faults-test.sh @@ -29,6 +29,7 @@ NAT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" ROOT_DIR="$(cd "$NAT_DIR/../.." && pwd)" BUILD_SCRIPT="$ROOT_DIR/testing/scripts/build.sh" GENERATE_SCRIPT="$SCRIPT_DIR/generate-configs.sh" +RELAY_LIB="$ROOT_DIR/testing/lib/relay-verdict.sh" PROFILE="stun-faults" SCENARIO="$PROFILE" @@ -53,6 +54,9 @@ NODE="fips-nat-stun-fault-node${FIPS_CI_NAME_SUFFIX:-}" PEER="fips-nat-stun-fault-peer${FIPS_CI_NAME_SUFFIX:-}" SHIM="fips-nat-stun-fault-shim${FIPS_CI_NAME_SUFFIX:-}" STUN_CONTAINER="fips-nat-stun${FIPS_CI_NAME_SUFFIX:-}" +# The node and peer find each other through this relay's adverts, so a relay +# that died takes the pre-flight down with it; the dump states it first. +RELAY_CONTAINER="fips-nat-relay${FIPS_CI_NAME_SUFFIX:-}" # Claimed per run by ci-local.sh; unset renders the lab's historical address. STUN_HOST="${NAT_LAN_PREFIX:-172.31.10}.40" STUN_PORT=3478 @@ -67,6 +71,9 @@ cleanup() { >/dev/null 2>&1 || true } +# shellcheck disable=SC1090 +source "$RELAY_LIB" + trap 'echo ""; echo "stun-faults-test interrupted"; cleanup; exit 130' INT TERM require_docker_daemon() { @@ -95,9 +102,12 @@ require_test_image() { } dump_diagnostics() { + echo "" + echo "=== relay verdict ===" + relay_verdict "$RELAY_CONTAINER" || true echo "" echo "=== stun-faults diagnostics ===" - for c in "$NODE" "$PEER" "$SHIM" "$STUN_CONTAINER"; do + for c in "$NODE" "$PEER" "$SHIM" "$STUN_CONTAINER" "$RELAY_CONTAINER"; do echo "" echo "--- $c: logs (last 80) ---" docker logs "$c" 2>&1 | tail -80 || true @@ -364,6 +374,12 @@ run_test() { return 1 } + # A relay that faulted while the phases still passed is a finding about + # the relay, not about this run, so it is reported and not made a failure. + if relay_verdict "$RELAY_CONTAINER"; then + echo "NOTE: the assertions above passed despite that." >&2 + fi + cleanup if [ ${#SKIPPED_PHASES[@]} -eq 0 ]; then echo "stun-faults-test passed (3/3 phases ran)" From 2236371537004fc87c51e13362250a27666a81f2 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 01:47:09 +0000 Subject: [PATCH 2/7] fix(nostr): stop signing traversal deletion requests with the routing key After a traversal attempt, both sides published a NIP-09 deletion request signed by the node's routing key and naming the attempt's offer and answer wraps. Each request put the node's public identity next to the ids of its traversal signals on every relay it reached, which the one-time signing keys on the wraps are meant to avoid. What the requests deleted differed by side. A relay honouring NIP-59 deletes a gift wrap when the request is signed by the wrap's p-tagged recipient, so the initiator's request, sent after a successful punch, did delete the answer wrap addressed to it; strfry in the NAT lab logged that deletion. The offer it also named is addressed to the responder, and the responder's requests named only the answer it had sent to the initiator, so those deleted nothing. Drop all three traversal calls (the initiator after a punch, the responder on refusing an offer, and the responder after its punch attempt), and accept that an answer wrap now stays on a relay that stores it until its NIP-40 expiration. The wraps are ephemeral kinds, so relays that do not store ephemeral events never held them. The advertisement retraction keeps its deletion request, since it names an event the routing key signed itself. The signal envelope's event id had no other reader and is removed with its import. The NAT lab now asserts that its relay holds no kind 5 event after the cone, symmetric and lan scenarios, scanning the relay's own store and failing if the scan cannot run or be parsed. Before the fix each scenario failed it: cone and lan held two requests (the initiator's naming offer and answer, the responder's naming the answer) and symmetric held the responder's one. After it, all three held none. The responder's refusal path is not reached by any lab scenario and is covered by reading only. --- src/nostr/runtime.rs | 8 ---- src/nostr/signal.rs | 2 - testing/nat/scripts/nat-test.sh | 51 +++++++++++++++++++++++++ testing/nat/scripts/nostr-relay-test.sh | 6 +-- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/nostr/runtime.rs b/src/nostr/runtime.rs index 7903c94f..557441b0 100644 --- a/src/nostr/runtime.rs +++ b/src/nostr/runtime.rs @@ -850,7 +850,6 @@ impl NostrRendezvous { { let _ = tx.send(SignalEnvelope { payload: answer, - event_id: event.id, sender_npub: sender_npub.clone(), }); } @@ -1301,10 +1300,6 @@ impl NostrRendezvous { "traversal: initiator punch succeeded" ); - let _ = self - .publish_delete(&relays, [offer_event.id, answer.event_id]) - .await; - self.failure_state .record_success(&peer_config.npub, now_ms()); @@ -1471,7 +1466,6 @@ impl NostrRendezvous { "traversal: answer sent" ); if !accepted { - let _ = self.publish_delete(&relays, [answer_event.id]).await; return Ok(()); } @@ -1521,8 +1515,6 @@ impl NostrRendezvous { ); } } - - let _ = self.publish_delete(&relays, [answer_event.id]).await; Ok(()) } diff --git a/src/nostr/signal.rs b/src/nostr/signal.rs index 6b2306e5..3d3eddac 100644 --- a/src/nostr/signal.rs +++ b/src/nostr/signal.rs @@ -1,4 +1,3 @@ -use nostr::EventId; use nostr::nips::{nip44, nip59}; use nostr::prelude::{ Event, EventBuilder, JsonUtil, Kind, NostrSigner, PublicKey, Tag, Timestamp, UnsignedEvent, @@ -19,7 +18,6 @@ pub(crate) const FRESHNESS_SKEW_TOLERANCE_MS: u64 = 60_000; pub(super) struct SignalEnvelope { pub(super) payload: T, - pub(super) event_id: EventId, pub(super) sender_npub: String, } diff --git a/testing/nat/scripts/nat-test.sh b/testing/nat/scripts/nat-test.sh index aec7aa72..5e9c040e 100755 --- a/testing/nat/scripts/nat-test.sh +++ b/testing/nat/scripts/nat-test.sh @@ -448,6 +448,45 @@ ping_peer() { fi } +# Fail if the relay holds any kind 5 deletion request. +# +# A node signs a deletion request with its routing key, so one naming a +# traversal signal's wrap would tie that key to a signal sent under a one-time +# key. Nodes delete only adverts they withdraw, and the lab's nodes advertise +# throughout, so after a scenario the relay should hold none. strfry scan reads +# the relay's own store and prints one event per line. A scan that cannot run +# or print parseable events has observed nothing, so it fails the check too. +assert_no_deletion_requests() { + local relay="$1" events="" report="" + if ! events="$(docker exec "$relay" strfry scan '{"kinds":[5]}' 2>/dev/null)"; then + echo "FAIL: could not scan $relay for deletion requests" >&2 + return 1 + fi + if ! report="$(python3 -c ' +import json, sys +events = [] +for line in sys.stdin: + line = line.strip() + if line: + events.append(json.loads(line)) +for ev in events: + ids = [t[1] for t in ev.get("tags", []) if len(t) > 1 and t[0] == "e"] + print(" kind 5 by %s... naming %s" % (ev["pubkey"][:16], ", ".join(ids))) +print(len(events)) +' <<<"$events")"; then + echo "FAIL: could not parse $relay's scan for deletion requests" >&2 + return 1 + fi + local count="${report##*$'\n'}" + if [ "$count" != "0" ]; then + echo "FAIL: $relay holds $count deletion request(s):" >&2 + sed '$d' <<<"$report" >&2 + return 1 + fi + echo " $relay holds no deletion requests" + return 0 +} + # A relay that faulted while a scenario's assertions still passed is a finding # about the relay, not about the scenario, so it is reported and not made a # failure: the scenario proved what it set out to prove. @@ -497,6 +536,10 @@ run_cone() { dump_cone_diagnostics return 1 } + assert_no_deletion_requests "$RELAY_CONTAINER" || { + dump_cone_diagnostics + return 1 + } note_relay_event cleanup } @@ -543,6 +586,10 @@ run_symmetric() { dump_symmetric_diagnostics return 1 } + assert_no_deletion_requests "$RELAY_CONTAINER" || { + dump_symmetric_diagnostics + return 1 + } note_relay_event cleanup } @@ -586,6 +633,10 @@ run_lan() { dump_lan_diagnostics return 1 } + assert_no_deletion_requests "$RELAY_CONTAINER" || { + dump_lan_diagnostics + return 1 + } note_relay_event # Skip the final teardown when the mesh-lab harness wraps this # script: it needs to docker-logs the containers before teardown, diff --git a/testing/nat/scripts/nostr-relay-test.sh b/testing/nat/scripts/nostr-relay-test.sh index 8cffbb2c..df6bdfff 100755 --- a/testing/nat/scripts/nostr-relay-test.sh +++ b/testing/nat/scripts/nostr-relay-test.sh @@ -115,9 +115,9 @@ dump_diagnostics() { # # Which rejection they take is not what the event's gibberish `content` # suggests. `parse_overlay_advert_event` looks for the `protocol` tag -# first (src/nostr/runtime.rs:1665-1671) and this event carries only `d` +# first (src/nostr/runtime.rs:1657-1663) and this event carries only `d` # and `app`, so it fails with `missing required protocol tag` and never -# reaches the `serde_json::from_str` at :1679. The content is therefore +# reaches the `serde_json::from_str` at :1671. The content is therefore # belt and braces rather than the thing under test. # # Neither branch logs anything: see the coverage-gap note in run_test. @@ -236,7 +236,7 @@ while int.from_bytes(secret, "big") == 0 or int.from_bytes(secret, "big") >= N: pubkey = xonly_pubkey(secret).hex() created_at = int(time.time()) # Both of these must match the consumers' subscription filter, which is -# kind + identifier and no author clause (src/nostr/runtime.rs:1042-1044). +# kind + identifier and no author clause (src/nostr/runtime.rs:1041-1043). # The literals are ADVERT_KIND and ADVERT_IDENTIFIER in src/nostr/types.rs # and are duplicated here rather than derived, so changing either there # silently stops this event reaching the daemons while the relay goes on From 10451813fd86987b390185c6e600d2df833eefda Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 03:26:31 +0000 Subject: [PATCH 3/7] docs(design): describe traversal signals as left to expire, not deleted The discovery and traversal design documents still said both sides publish NIP-09 deletion requests for their offer and answer after an attempt, and listed deletion as the fallback for relays that store ephemeral kinds. Nodes no longer send those requests. Say so, and why: a relay honouring NIP-59 deletes a gift wrap only at its p-tagged recipient's request, which would have to be signed by the node's long-term key and would tie it to the traversal's events. The wraps carry a NIP-40 expiration tag, so a relay that stores them keeps them until they expire. --- docs/design/fips-nostr-discovery.md | 13 ++++++++++--- .../design/port-advertisement-and-nat-traversal.md | 14 +++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/design/fips-nostr-discovery.md b/docs/design/fips-nostr-discovery.md index fff701c9..df27ee81 100644 --- a/docs/design/fips-nostr-discovery.md +++ b/docs/design/fips-nostr-discovery.md @@ -272,9 +272,16 @@ from the far side records the working remote address and completes the attempt. On timeout (`attempt_timeout_secs` as overall bound, -`punch_duration_ms` as probe window), both sides issue NIP-9 deletes -for their offer and answer events and report failure up to the -discovery runtime's `BootstrapEvent::Failed` channel. +`punch_duration_ms` as probe window), the attempt reports failure up +to the discovery runtime's `BootstrapEvent::Failed` channel. + +Neither side publishes a NIP-9 deletion request for the offer or +answer, on success or failure. A relay honouring NIP-59 deletes a gift +wrap only at the request of its p-tagged recipient, so such a request +would have to be signed by the node's routing key and would name the +traversal's events, tying that key to them on every relay it reached. +The wraps are ephemeral kinds carrying a NIP-40 expiration tag, so +relays that store them at all keep them until they expire. ### Phase 5 — Adoption diff --git a/docs/design/port-advertisement-and-nat-traversal.md b/docs/design/port-advertisement-and-nat-traversal.md index ed35428f..c12fcf69 100644 --- a/docs/design/port-advertisement-and-nat-traversal.md +++ b/docs/design/port-advertisement-and-nat-traversal.md @@ -411,10 +411,14 @@ Once the path has acknowledged in both directions: After the attempt completes (success or failure): 1. Close the relay subscription used for signaling. -2. Optionally publish a NIP-09 deletion event referencing any - signaling events the peer published. Because the wraps were - ephemeral kinds with NIP-40 expiration tags, well-behaved relays - will discard them automatically without explicit deletion. +2. Do not publish a NIP-09 deletion request for the signaling + events. Under NIP-59 a relay deletes a gift wrap only at the + request of its p-tagged recipient, so the request would be signed + by the recipient's long-term key and would name the traversal's + events, linking that key to them. The wraps are ephemeral kinds + with NIP-40 expiration tags: well-behaved relays discard them + without being asked, and a relay that stores them keeps them until + they expire. 3. Discard the per-attempt punch socket if the attempt failed; a retry must allocate a new socket and a fresh reflexive address. @@ -532,7 +536,7 @@ own. | Symmetric NAT (one side) | Punch timeout | Retry with port-prediction heuristics; otherwise fall back to an application-level relay | | Symmetric NAT (both sides) | Punch timeout | Application-level relay required | | Relay latency > 60 s | Stale reflexive address | Use low-latency relays; consider self-hosted relay | -| Relay does not support ephemeral kinds | Signaling events persist | Use NIP-40 expiration + NIP-09 deletion as fallback | +| Relay does not support ephemeral kinds | Signaling events persist | NIP-40 expiration bounds how long; no deletion request is sent (see Phase 6) | | Responder offline | No answer received | Initiator times out after configurable period | | Stale advert (responder no longer up) | Offer reaches no listener | Application-level failure suppression (see below) | | STUN server unreachable | No reflexive address | Fall back to alternate STUN server; fail if none reachable | From d4acdfc39f5212fd89209ada5c64ed4734ababef Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 03:35:47 +0000 Subject: [PATCH 4/7] Check the package's declared dependencies against the libraries its binaries need The package's Depends comes from cargo-deb's "$auto", which runs dpkg-shlibdeps over each binary. cargo-deb deliberately removes every libgcc entry from that result, on the grounds that every system has one, so the package never declared the libgcc-s1 all four binaries link. Otherwise it passes the dpkg-shlibdeps output through unchanged, with one more gap: when dpkg-shlibdeps fails on a binary it only warns and builds anyway, leaving that binary's libraries out of the list. testing/check-deb-depends.sh runs dpkg-shlibdeps once over every ELF object in the built package and compares the result with the shipped Depends. Every derived entry must be shipped, and the highest shipped floor for it must equal the derived floor. A missing or lower entry means the package installs where its binaries cannot run; a higher one means a hand-written floor no longer tracks what the binaries need. Both fail the build. build-deb-container.sh runs it inside the build image, so it reads the same symbols files and C library cargo-deb did, and every producer of the package is gated. libgcc-s1 (>= 4.2) is now declared by hand beside "$auto". There is no exception for it in the check, so its floor is held equal to the derived one and cannot fall behind or run ahead unnoticed. --- CHANGELOG.md | 13 ++ Cargo.toml | 7 +- packaging/debian/build-deb-container.sh | 12 +- testing/check-deb-depends.sh | 208 ++++++++++++++++++++++++ 4 files changed, 238 insertions(+), 2 deletions(-) create mode 100755 testing/check-deb-depends.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 05de8b83..15796550 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -233,6 +233,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 scripts, so the `.ipk` and `.apk` packages install the same bodies and the scenarios in `testing/openwrt/` run what ships. +#### Packaging + +- The `.deb` now declares `libgcc-s1 (>= 4.2)`. All four binaries link + `libgcc_s.so.1`, but cargo-deb removes every libgcc entry from the + dependencies it derives, so the package never said so. `libc6` depends on + `libgcc-s1` on Debian 12 and Ubuntu 22.04, 24.04 and 26.04, so installs there + were not affected. A new check, `testing/check-deb-depends.sh`, runs + `dpkg-shlibdeps` over the package's binaries on every build and fails the + build when the declared `Depends` leaves out a library the binaries need, or + states a floor lower or higher than the one they need. A dependency the + packaging tool drops, including one it drops after only a warning when it + cannot resolve a binary, now fails the build instead of shipping. + ### Changed - The lockfile moves `chacha20` from 0.10.1 to 0.10.2, because 0.10.1 is yanked. diff --git a/Cargo.toml b/Cargo.toml index 0efde69b..a1107074 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,8 +88,13 @@ priority = "optional" # then could not start. Deriving it also picks up a libdbus floor the hand # written list lacked, and re-derives per architecture, which matters because # arm64 links libdbus in all four binaries where amd64 links it in one. +# libgcc-s1 is written by hand beside "$auto" because cargo-deb removes every +# libgcc entry from what it derives, on the grounds that every system has one. +# testing/check-deb-depends.sh runs dpkg-shlibdeps on every build and fails if +# this floor differs from the derived one, so it cannot fall behind or run +# ahead of what the binaries need. # systemd stays by hand: nothing links it, so nothing can derive it. -depends = "$auto, systemd" +depends = "$auto, libgcc-s1 (>= 4.2), systemd" recommends = "bluez" extended-description = """\ FIPS is a distributed, decentralized network routing protocol for mesh \ diff --git a/packaging/debian/build-deb-container.sh b/packaging/debian/build-deb-container.sh index ede5a92d..e8b94c4b 100755 --- a/packaging/debian/build-deb-container.sh +++ b/packaging/debian/build-deb-container.sh @@ -152,8 +152,18 @@ DEB="$DEST_ABS/$DEB_NAME" # Check the artifact here rather than in one workflow, so every producer is # gated: the release, the CI job, a local run and packaging/Makefile all reach -# the check through this script. +# both checks through this script. The glibc floor is read from the binaries +# and runs on the host. The Depends check runs in the build image, because it +# compares against dpkg-shlibdeps and has to read the same symbols files and C +# library that cargo-deb's "$auto" read; a host of another distribution could +# produce a difference of its own. "$REPO_ROOT/testing/check-glibc-floor.sh" "$DEB" >&2 +docker run --rm \ + -v "$REPO_ROOT":/src:ro \ + -v "$DEST_ABS":/out:ro \ + -w /src \ + "$IMAGE_TAG" \ + testing/check-deb-depends.sh "/out/$DEB_NAME" >&2 echo "=== Built $DEB ===" >&2 printf '%s\n' "$DEB" diff --git a/testing/check-deb-depends.sh b/testing/check-deb-depends.sh new file mode 100755 index 00000000..63180d84 --- /dev/null +++ b/testing/check-deb-depends.sh @@ -0,0 +1,208 @@ +#!/bin/bash +# Fail when a package's declared Depends does not match the libraries its +# binaries actually need. +# +# The package's Depends comes from cargo-deb's "$auto", which runs +# dpkg-shlibdeps over each binary. cargo-deb 3.6.3 changes that result in two +# ways, both silent. It removes every libgcc entry on the grounds that every +# system has one (src/dependencies.rs in the cargo-deb source), so the package +# never declared the libgcc-s1 its binaries link. And when dpkg-shlibdeps +# fails on a binary it prints a warning and builds anyway, leaving that +# binary's libraries out of the list. Neither shows up anywhere but in the +# field itself. +# +# This runs dpkg-shlibdeps once over every ELF object the package contains and +# compares the result with the Depends the package ships: +# +# - every derived entry must be shipped; +# - for a derived "name (>= v)", the highest ">=" floor shipped for that name +# must EQUAL v. Below it, the package installs where the binaries cannot +# run. Above it, a hand-written floor no longer tracks what the binaries +# need. There is no exception for any name; +# - shipped entries that nothing derives (systemd) are listed, not failed: +# they are the ones written by hand, and Cargo.toml says why each is there. +# +# libgcc-s1 is written by hand in Cargo.toml beside "$auto" because cargo-deb +# drops it. The equality rule is what keeps that hand-written floor honest: if +# the toolchain stops needing it, or starts needing a newer one, this fails. +# +# Run it inside the build image (build-deb-container.sh does), so the symbols +# files and the C library it reads are the ones cargo-deb read. On a host of a +# different distribution a difference could come from the host instead. +# +# Usage: check-deb-depends.sh ... +# Exit: 0 every package matches, 1 a mismatch, 2 could not establish a result. + +set -euo pipefail + +for tool in dpkg dpkg-deb dpkg-query dpkg-shlibdeps readelf; do + command -v "$tool" >/dev/null 2>&1 || { + echo "check-deb-depends: $tool is not installed; cannot check anything." >&2 + echo " Refusing to report a pass I did not establish." >&2 + exit 2 + } +done + +[ $# -gt 0 ] || { + echo "usage: check-deb-depends.sh ..." >&2 + exit 2 +} + +FAILED=0 +UNKNOWN=0 +CHECKED=0 + +# A relation list entry of the simple forms dpkg-shlibdeps prints: a bare name +# or "name (>= version)". Anything else is compared word for word. +SIMPLE_RE='^([a-z0-9][a-z0-9+.-]*)( \(>= ([^)]+)\))?$' + +# Split a Depends value on commas into one trimmed entry per line. +split_deps() { + printf '%s\n' "$1" | tr ',' '\n' | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//' | sed '/^$/d' +} + +check_deb() { + local deb="$1" label tmp arch shipped derived out + label=$(basename "$deb") + tmp=$(mktemp -d) + # shellcheck disable=SC2064 + trap "rm -rf '$tmp'" RETURN + + if ! dpkg-deb -x "$deb" "$tmp/root"; then + echo " ERROR $label could not be unpacked" >&2 + UNKNOWN=$((UNKNOWN + 1)) + return + fi + arch=$(dpkg-deb -f "$deb" Architecture) + shipped=$(dpkg-deb -f "$deb" Depends) + if [ -z "$arch" ]; then + echo " ERROR $label has no Architecture field" >&2 + UNKNOWN=$((UNKNOWN + 1)) + return + fi + + # The same filter as check-glibc-floor.sh: executable files that parse as + # ELF. A shared library shipped later is executable too, so it is covered. + local bins=() f + while IFS= read -r -d '' f; do + readelf -hW "$f" >/dev/null 2>&1 || continue + bins+=("$f") + done < <(find "$tmp/root" -type f -perm -u+x -print0 | sort -z) + if [ "${#bins[@]}" -eq 0 ]; then + echo " ERROR $label contains no ELF objects; the layout moved and nothing was checked" >&2 + UNKNOWN=$((UNKNOWN + 1)) + return + fi + + # dpkg-shlibdeps needs a debian/control in its working directory, and an + # empty one is enough; cargo-deb sets it up the same way. DEB_HOST_ARCH is + # set as cargo-deb sets it, so a foreign-architecture package is resolved + # against that architecture's symbols files, not the host's. + mkdir -p "$tmp/work/debian" + : > "$tmp/work/debian/control" + if ! out=$(cd "$tmp/work" && DEB_HOST_ARCH="$arch" dpkg-shlibdeps -O "${bins[@]}" 2>"$tmp/shlibdeps.err"); then + echo " ERROR $label: dpkg-shlibdeps failed:" >&2 + sed 's/^/ /' "$tmp/shlibdeps.err" >&2 + UNKNOWN=$((UNKNOWN + 1)) + return + fi + derived=$(printf '%s\n' "$out" | sed -n 's/^shlibs:Depends=//p' | head -n 1) + if [ -z "$derived" ]; then + echo " ERROR $label: dpkg-shlibdeps derived no dependencies for ${#bins[@]} ELF object(s)" >&2 + UNKNOWN=$((UNKNOWN + 1)) + return + fi + + echo " $label ($arch, ${#bins[@]} ELF objects)" + echo " derived: $derived" + echo " shipped: $shipped" + + # Index the shipped list: the highest ">=" floor per name, the names + # present at all, and every entry verbatim. + declare -A floor=() present=() verbatim=() derivednames=() + local e name ver + while IFS= read -r e; do + verbatim["$e"]=1 + [[ "$e" =~ $SIMPLE_RE ]] || continue + name="${BASH_REMATCH[1]}" + ver="${BASH_REMATCH[3]}" + present["$name"]=1 + [ -n "$ver" ] || continue + if [ -z "${floor[$name]:-}" ] || dpkg --compare-versions "$ver" gt "${floor[$name]}"; then + floor["$name"]="$ver" + fi + done < <(split_deps "$shipped") + + local bad=0 have + while IFS= read -r e; do + if ! [[ "$e" =~ $SIMPLE_RE ]]; then + if [ -n "${verbatim[$e]:-}" ]; then + echo " ok $e" + else + echo " FAIL $label: derived '$e' not matched by shipped Depends (missing: $shipped)" >&2 + bad=1 + fi + continue + fi + name="${BASH_REMATCH[1]}" + ver="${BASH_REMATCH[3]}" + derivednames["$name"]=1 + if [ -z "${present[$name]:-}" ]; then + echo " FAIL $label: derived '$e' not matched by shipped Depends (missing: $shipped)" >&2 + bad=1 + continue + fi + if [ -z "$ver" ]; then + echo " ok $e" + continue + fi + have="${floor[$name]:-}" + if [ -z "$have" ]; then + echo " FAIL $label: derived '$e' not matched by shipped Depends (below: $name with no version floor)" >&2 + bad=1 + elif dpkg --compare-versions "$have" lt "$ver"; then + echo " FAIL $label: derived '$e' not matched by shipped Depends (below: $name (>= $have))" >&2 + bad=1 + elif dpkg --compare-versions "$have" gt "$ver"; then + echo " FAIL $label: derived '$e' not matched by shipped Depends (above: $name (>= $have))" >&2 + bad=1 + else + echo " ok $e" + fi + done < <(split_deps "$derived") + + while IFS= read -r e; do + if [[ "$e" =~ $SIMPLE_RE ]] && [ -n "${derivednames[${BASH_REMATCH[1]}]:-}" ]; then + continue + fi + echo " hand-declared $e" + done < <(split_deps "$shipped") + + CHECKED=$((CHECKED + 1)) + [ "$bad" -eq 0 ] || FAILED=$((FAILED + 1)) +} + +echo "=== Depends check (shipped Depends against dpkg-shlibdeps) ===" +for arg in "$@"; do + if [ ! -f "$arg" ]; then + echo " ERROR $arg does not exist" >&2 + UNKNOWN=$((UNKNOWN + 1)) + continue + fi + check_deb "$arg" +done + +if [ "$FAILED" -ne 0 ]; then + echo "check-deb-depends: $FAILED of $CHECKED package(s) declare Depends that differ from what their binaries need." >&2 + echo " A missing or low entry installs where the binaries cannot run; a high one" >&2 + echo " is a hand-written floor that no longer tracks them. Fix Cargo.toml's depends." >&2 + exit 1 +fi + +# Anything not established, or nothing examined at all, is not a pass. +if [ "$UNKNOWN" -ne 0 ] || [ "$CHECKED" -eq 0 ]; then + echo "check-deb-depends: could not establish a result ($UNKNOWN error(s), $CHECKED package(s) checked); refusing to report a pass." >&2 + exit 2 +fi + +echo "=== Depends check passed ($CHECKED package(s)) ===" From 01be207274aa785c99ecb3fcc57ffba1c24560fe Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 03:41:06 +0000 Subject: [PATCH 5/7] Embed the source revision in container-built binaries again The build image had no git, so build.rs could not read the revision and every binary built through the container carried none: -V printed only the version. The image now installs git, and trusts the source mounted at /src, which is owned by the host user while the build runs as root; without that entry git refuses the repository and the revision is silently empty just the same. The image tag now includes a hash of Dockerfile.build. Before, the tag named only the floor image and the toolchain, so a host with the image cached kept using it after the Dockerfile changed, and this change would never have reached it. A build from a git worktree still has no revision, because the worktree's git directory is outside the mounted tree. That is documented, with the -V reference noting that the revision is omitted when it could not be read, rather than worked around; release and CI builds use full checkouts. --- CHANGELOG.md | 7 ++++++ docs/reference/cli-fips.md | 2 +- docs/reference/cli-fipsctl.md | 2 +- docs/reference/cli-fipstop.md | 2 +- packaging/debian/Dockerfile.build | 16 +++++++++++++ packaging/debian/build-deb-container.sh | 30 ++++++++++++++++--------- packaging/debian/build-deb.sh | 4 ++-- 7 files changed, 48 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15796550..16effa74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -245,6 +245,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 states a floor lower or higher than the one they need. A dependency the packaging tool drops, including one it drops after only a warning when it cannot resolve a binary, now fails the build instead of shipping. +- `-V` on binaries built into the Linux packages now includes the source + revision, as ` (rev )`. The build image had no git, so + every container-built binary printed the version alone. A package built from + a git worktree still has no revision, because the worktree's git directory is + outside the tree the build sees. The build image's tag now includes a hash of + its Dockerfile, so a host with an older image cached builds a new one instead + of reusing it. ### Changed diff --git a/docs/reference/cli-fips.md b/docs/reference/cli-fips.md index 6571ddbe..aca69f28 100644 --- a/docs/reference/cli-fips.md +++ b/docs/reference/cli-fips.md @@ -28,7 +28,7 @@ controlled through the standard service control manager. | Flag | Argument | Description | | ---- | -------- | ----------- | | `-c`, `--config` | `FILE` | Use `FILE` as the configuration. Skips the default search paths. | -| `-V` | — | Print the short version, ` (rev )`. | +| `-V` | — | Print the short version, ` (rev )`. The `rev` part is omitted when the build could not read a git revision, as in a package built from a git worktree. | | `--version` | — | Print the long version: short version plus build target triple. | | `-h`, `--help` | — | Print usage and exit. | | `--install-service` | — | (Windows only) Install `fips` as a Windows service. Requires Administrator. | diff --git a/docs/reference/cli-fipsctl.md b/docs/reference/cli-fipsctl.md index 2ee7f6f9..df70b67c 100644 --- a/docs/reference/cli-fipsctl.md +++ b/docs/reference/cli-fipsctl.md @@ -29,7 +29,7 @@ that defines the socket location, see | Flag | Argument | Description | | ---- | -------- | ----------- | | `-s`, `--socket` | `PATH` | Override the control-socket path (Linux/macOS) or TCP port (Windows). | -| `-V` | — | Print the short version, ` (rev )`. | +| `-V` | — | Print the short version, ` (rev )`. The `rev` part is omitted when the build could not read a git revision, as in a package built from a git worktree. | | `--version` | — | Print the long version: short version plus build target triple. | | `-h`, `--help` | — | Print usage and exit. Per-subcommand help via `fipsctl --help`. | diff --git a/docs/reference/cli-fipstop.md b/docs/reference/cli-fipstop.md index 5291df8b..452b2cfe 100644 --- a/docs/reference/cli-fipstop.md +++ b/docs/reference/cli-fipstop.md @@ -28,7 +28,7 @@ a confirmation prompt — see [Keybindings](#keybindings)). For | `-s`, `--socket` | `PATH` | (auto) | Daemon control-socket path / port. Same default as `fipsctl`. | | `--gateway-socket` | `PATH` | (auto) | `fips-gateway` control-socket path / port. Default: `/run/fips/gateway.sock` (Unix), TCP port `21211` (Windows). | | `-r`, `--refresh` | `SECONDS` | `2` | Poll interval. | -| `-V` | — | — | Print the short version, ` (rev )`. | +| `-V` | — | — | Print the short version, ` (rev )`. The `rev` part is omitted when the build could not read a git revision, as in a package built from a git worktree. | | `--version` | — | — | Print the long version: short version plus build target triple. | | `-h`, `--help` | — | — | Print usage and exit. | diff --git a/packaging/debian/Dockerfile.build b/packaging/debian/Dockerfile.build index 52182848..6e69e644 100644 --- a/packaging/debian/Dockerfile.build +++ b/packaging/debian/Dockerfile.build @@ -8,6 +8,10 @@ # This image carries the toolchain and the build dependencies only. It never # carries the source: the source is mounted at run time, so editing a file does # not invalidate the image and a warm rebuild costs seconds rather than minutes. +# +# A build from a git worktree carries no source revision: the worktree's .git is +# a file pointing outside the mounted tree, so git cannot read it here. Release +# and CI builds use full checkouts and carry one. ARG BASE=ubuntu:22.04 FROM ${BASE} @@ -24,10 +28,22 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ clang \ binutils \ dpkg-dev \ + git \ curl \ ca-certificates \ && apt-get clean && rm -rf /var/lib/apt/lists/* +# build.rs asks git for the revision it embeds in the binaries. The source is +# mounted at /src owned by the host user, while the build runs as root, and git +# refuses a repository owned by someone else: without this entry the revision is +# silently empty, exactly as it was when the image had no git at all. +# The dirty flag can be stale in a local container build. build.rs reruns only +# when .git/HEAD or .git/refs change, and the target directory is a persistent +# volume, so an uncommitted edit alone does not refresh it; git here also runs +# without the host user's global excludes, so a file only those ignore reads as +# dirty. Release and CI builds start from a committed, fresh tree. +RUN git config --system --add safe.directory /src + # The toolchain version is passed in, read from rust-toolchain.toml by the # calling script, and the image tag carries it -- so the image cannot drift from # the compiler the rest of CI uses, and bumping the pin rebuilds the image. The diff --git a/packaging/debian/build-deb-container.sh b/packaging/debian/build-deb-container.sh index e8b94c4b..361e99ef 100755 --- a/packaging/debian/build-deb-container.sh +++ b/packaging/debian/build-deb-container.sh @@ -12,9 +12,10 @@ # Usage: build-deb-container.sh [--output-dir DIR] [--version V] [--features LIST] # [--rebuild-image] # -# Requires docker. The image is cached between runs and rebuilt only when the -# Dockerfile or the floor changes; the source is mounted rather than copied, so -# editing code does not invalidate it. +# Requires docker. The image is cached between runs under a tag made of the +# floor image, the Rust toolchain and a hash of Dockerfile.build, so a change to +# any of the three builds a new image; the source is mounted rather than copied, +# so editing code does not invalidate it. set -euo pipefail @@ -35,7 +36,7 @@ while [[ $# -gt 0 ]]; do --version) VERSION="${2:?missing value for --version}"; shift 2 ;; --features) FEATURES="${2:?missing value for --features}"; shift 2 ;; --rebuild-image) REBUILD_IMAGE=1; shift ;; - -h|--help) sed -n '2,17p' "$0"; exit 0 ;; + -h|--help) sed -n '2,18p' "$0"; exit 0 ;; *) echo "Unknown option: $1" >&2; exit 2 ;; esac done @@ -47,13 +48,21 @@ command -v docker >/dev/null 2>&1 || { # Read the toolchain from the pin rather than choosing one here, and put it in # the tag so a bump rebuilds the image instead of silently reusing a stale one. +# The Dockerfile's content goes in the tag for the same reason: without it a +# host that has the image cached keeps using it after the Dockerfile changes. RUST_TOOLCHAIN=$(awk -F'"' '/^channel *=/{print $2; exit}' "$REPO_ROOT/rust-toolchain.toml") [ -n "$RUST_TOOLCHAIN" ] || { echo "build-deb-container: could not read channel from rust-toolchain.toml" >&2 exit 2 } -IMAGE_TAG="fips-deb-builder:${FIPS_BUILD_IMAGE//[:\/]/-}-rust${RUST_TOOLCHAIN}" +DOCKERFILE_HASH=$(sha256sum "$SCRIPT_DIR/Dockerfile.build" | cut -c1-12) || DOCKERFILE_HASH="" +[[ "$DOCKERFILE_HASH" =~ ^[0-9a-f]{12}$ ]] || { + echo "build-deb-container: could not hash $SCRIPT_DIR/Dockerfile.build" >&2 + exit 2 +} + +IMAGE_TAG="fips-deb-builder:${FIPS_BUILD_IMAGE//[:\/]/-}-rust${RUST_TOOLCHAIN}-${DOCKERFILE_HASH}" if [ "$REBUILD_IMAGE" -eq 1 ] || ! docker image inspect "$IMAGE_TAG" >/dev/null 2>&1; then echo "=== Building $IMAGE_TAG from $FIPS_BUILD_IMAGE with Rust $RUST_TOOLCHAIN ===" >&2 @@ -67,10 +76,10 @@ else echo "=== Using cached $IMAGE_TAG ===" >&2 fi -# Derive the version and the timestamp on the host, where git works, and pass -# both in. The container then never runs git, which matters for two reasons: a -# worktree's .git is a file pointing outside the mount and would not resolve, -# and a bind-mounted repository trips git's dubious-ownership check. +# Derive the version and the timestamp on the host and pass both in, because a +# worktree's .git is a file pointing outside the mount and does not resolve in +# the container. The image's git is there only for build.rs's revision, which is +# empty for a worktree build for the same reason. if [ -z "$VERSION" ]; then CRATE_VERSION=$(awk -F'"' '/^version = /{print $2; exit}' "$REPO_ROOT/Cargo.toml") if [[ "$CRATE_VERSION" == *-dev ]]; then @@ -100,7 +109,8 @@ if [ -n "$FEATURES" ]; then # it is also what marks the version so a feature package is distinguishable # from the default build of the same commit. It refuses --features with # --no-build for that reason, so the two cases cannot share one command. - # The version still comes from the host, because the image has no git. + # The version still comes from the host, because a worktree's .git does + # not resolve inside the mount. BUILD_CMD="packaging/debian/build-deb.sh --features '$FEATURES' --version '$VERSION' --output-dir /out --name-file /name/deb" else BUILD_CMD="cargo build --release --locked diff --git a/packaging/debian/build-deb.sh b/packaging/debian/build-deb.sh index 5712cbe1..623200f4 100755 --- a/packaging/debian/build-deb.sh +++ b/packaging/debian/build-deb.sh @@ -145,8 +145,8 @@ elif [[ -n "${FEATURES}" ]]; then # An explicit version needs the same marker for the same reason, and it is # the only way a caller that cannot derive the version here can get one. # The container build is that caller: it derives the version on the host - # because the image has no git, and the source is mounted read-only from a - # worktree whose .git is a file pointing outside the mount. + # because the source may be mounted read-only from a worktree whose .git is + # a file pointing outside the mount, which git in the container cannot read. if [[ "${VERSION_OVERRIDE}" == *"+$(printf '%s' "${FEATURES}" | tr -c 'a-zA-Z0-9.' '.')"* ]]; then : # already marked by the caller elif [[ "${VERSION_OVERRIDE}" == *-* ]]; then From 9a1797d3ed5345e71075143f5b6e74f86a900ecc Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 11:03:06 +0000 Subject: [PATCH 6/7] Build CI's release binaries once and reuse the package builder image The dns-resolver suite's end-to-end scenarios compiled fips and fips-gateway themselves, in a Debian 12 image with whatever Rust was current. On GitHub that was a second release build on every run, with no cache, and it was neither the toolchain nor the build that ships. And every GitHub package build assembled its builder image from scratch on a fresh runner: apt, rustup and a source compile of cargo-deb, on both legs of the release workflow and in CI's package job. The suite now takes --deb PATH and unpacks the two binaries from the package with dpkg-deb. The package is built in the pinned floor container, so its binaries start on all five e2e distributions. Without --deb the suite builds the package through build-deb-container.sh, the same fallback the install suite uses, so the inline Debian 12 builder is gone rather than kept as a second path. A missing --deb file is refused before any scenario runs, and a missing dpkg-deb is a named error. In the workflow the dns-resolver leg moves to a job of its own that downloads the package the install legs use, keeping its displayed check name. In local CI a shared helper builds the package once for both the dns-resolver and deb-install suites. build-deb-container.sh gains --print-image-tag, which prints the image tag without needing docker, and --image-archive PATH: when the image is absent and the archive exists it is loaded from there, and when the run builds the image it is saved there, through a temporary file renamed into place. An archive that fails to load, or does not hold the expected tag, is a warning and a rebuild rather than a failed build, since the archive only saves time. Both workflows restore the archive from the Actions cache under a key made from the image tag, so any change that rebuilds the image locally also misses the cache. Only pushes to maint, master and next save an entry, so pull requests and topic branches read the default branch's entry instead of each storing a copy that nothing else can read. A restored image is not refreshed from apt or the base image until one of the tag's inputs changes, as was already the case locally. --- .github/workflows/ci.yml | 182 +++++++++++++++------ .github/workflows/package-linux.yml | 55 +++++++ CHANGELOG.md | 20 +++ packaging/debian/build-deb-container.sh | 88 +++++++++-- testing/README.md | 4 +- testing/check-ci-parity.sh | 2 + testing/ci-local.sh | 72 +++++++-- testing/dns-resolver/test.sh | 201 ++++++++++++++---------- 8 files changed, 456 insertions(+), 168 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a35dbe8..83aa84ad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,11 +27,12 @@ env: # ───────────────────────────────────────────────────────────────────────────── # CI parity invariant # -# This workflow's integration matrices — the `integration:` job and the -# `deb-install:` job — and the local default suite set -# (testing/ci-local.sh) MUST run the same integration suites, EXCEPT for the -# deliberate local-only entries below. Adding a suite to one runner without -# the other means "local green" and "GitHub green" stop being equivalent. +# This workflow's integration matrices — the `integration:` job, the +# `dns-resolver:` job and the `deb-install:` job — and the local default suite +# set (testing/ci-local.sh) MUST run the same integration suites, EXCEPT for +# the deliberate local-only entries below. Adding a suite to one runner +# without the other means "local green" and "GitHub green" stop being +# equivalent. # testing/check-ci-parity.sh enforces this and fails on unexpected drift. # # Deliberate local-only (NOT on the GitHub gate), with reason: @@ -547,15 +548,6 @@ jobs: # recovers from delay, and never panics. - suite: stun-faults type: stun-faults - # ── DNS resolver multi-backend coverage ──────────────────────── - # Exercises every fips-dns-setup backend (resolved, dnsmasq, - # NM+dnsmasq, dns-delegate, no-resolver) across five distros, - # plus end-to-end scenarios that boot a real fips daemon with a - # real TUN and assert `dig @127.0.0.53 AAAA .fips` - # returns AAAA. Pins the production DNS bind path where a - # loopback-delivered query was once misattributed to the mesh - # interface and dropped. Single matrix entry runs all 13 - # scenarios sequentially; ~7-12 min warm, ~12-15 min cold. # Native datagram API: a client process opening a pubkey-to-pubkey # flow over the daemon's Unix socket. One single-node leg covering # the socket, its access mode and the command surface, plus a @@ -564,9 +556,6 @@ jobs: - suite: native-api type: native-api - - suite: dns-resolver - type: dns-resolver - steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 @@ -780,39 +769,13 @@ jobs: docker rm -f "$c" >/dev/null 2>&1 || true done - # ── DNS resolver multi-backend integration ────────────────────────── - # The dns-resolver harness builds its own fips binary from source in a - # Debian 12 builder image (shared cache layout with deb-install). Runs - # all 13 scenarios in a single job: dummy-TUN backend-detection tests - # plus real-fips end-to-end queries through systemd-resolved across - # five distros. ~7-12 min warm, ~12-15 min cold. - - name: Run dns-resolver test - if: matrix.type == 'dns-resolver' - timeout-minutes: 30 - run: bash testing/dns-resolver/test.sh - - - name: Collect logs on failure (dns-resolver) - if: matrix.type == 'dns-resolver' && failure() - run: | - docker ps -a --filter "name=fips-dns-test-" --format '{{.Names}}' | while read -r c; do - echo "--- ${c} fips.service ---" - docker exec "$c" journalctl -u fips.service --no-pager 2>&1 | tail -100 || true - echo "--- ${c} fips-dns.service ---" - docker exec "$c" journalctl -u fips-dns.service --no-pager 2>&1 | tail -100 || true - done - - - name: Stop containers (dns-resolver) - if: matrix.type == 'dns-resolver' && always() - run: | - docker ps -a --filter "name=fips-dns-test-" --format '{{.Names}}' | while read -r c; do - docker rm -f "$c" >/dev/null 2>&1 || true - done - # ───────────────────────────────────────────────────────────────────────────── # Job 4 – The .deb the install suite installs # # Built once, here, by the same script the release workflow and a local run # call, so the package the suite installs is built the way the shipped one is. +# Two jobs consume it: the install legs install it, and the dns-resolver job +# runs its binaries. # That was not true before: each install leg built its own package on a fresh # runner with no cache, so one run performed five complete Rust release builds # and four were waste — and none of them was built the way the release is, so @@ -832,9 +795,63 @@ jobs: steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + # The builder image travels between runners through the Actions cache, + # keyed on the image tag the script computes, so any change that would + # rebuild the image locally (base image, toolchain, Dockerfile.build) + # also misses here and cannot pick up a stale image. Every run restores; + # only a push to maint, master or next saves, because the cache is + # scoped per ref and an entry saved by a pull request or a topic branch + # could be read by nothing else while it pushed the cargo caches toward + # the repository's size limit. Topic branches and pull requests read the + # default branch's entry. What this gives up: an image restored from the + # cache is not rebuilt, so, as on a developer's machine, apt and the + # ubuntu:22.04 base are not refreshed until one of the tag's inputs + # changes. The image carries build tools only, and the glibc floor and + # Depends checks still run on every package. + - name: Resolve the builder image cache key + id: builder + shell: bash + run: | + set -euo pipefail + tag=$(bash packaging/debian/build-deb-container.sh --print-image-tag) + [ -n "$tag" ] + echo "key=deb-builder-${{ runner.arch }}-${tag//:/-}" >> "$GITHUB_OUTPUT" + + - name: Restore the builder image + id: builder-restore + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ runner.temp }}/deb-builder-image.tar + key: ${{ steps.builder.outputs.key }} + - name: Build the .deb in the pinned build container timeout-minutes: 30 - run: bash packaging/debian/build-deb-container.sh --output-dir deploy + run: | + bash packaging/debian/build-deb-container.sh --output-dir deploy \ + --image-archive "$RUNNER_TEMP/deb-builder-image.tar" + + # On a cache miss the archive exists only if the script built the image + # and saved it, so its presence is what says there is something to save. + # A failed build skips this and the save, so no image is cached from a + # job that did not produce a package. + - name: Check for a new builder image archive + id: builder-archive + shell: bash + run: | + if [ -f "$RUNNER_TEMP/deb-builder-image.tar" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + fi + + - name: Save the builder image + if: >- + github.event_name == 'push' + && contains(fromJSON('["refs/heads/maint", "refs/heads/master", "refs/heads/next"]'), github.ref) + && steps.builder-restore.outputs.cache-hit != 'true' + && steps.builder-archive.outputs.present == 'true' + uses: actions/cache/save@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ runner.temp }}/deb-builder-image.tar + key: ${{ steps.builder.outputs.key }} - name: Upload the .deb uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 @@ -844,6 +861,73 @@ jobs: if-no-files-found: error retention-days: 1 +# ───────────────────────────────────────────────────────────────────────────── +# DNS resolver multi-backend coverage +# +# Exercises every fips-dns-setup backend (resolved, dnsmasq, NM+dnsmasq, +# dns-delegate, no-resolver) across five distros, plus end-to-end scenarios +# that boot a real fips daemon with a real TUN and assert +# `dig @127.0.0.53 AAAA .fips` returns AAAA. Pins the production DNS bind +# path where a loopback-delivered query was once misattributed to the mesh +# interface and dropped. One leg runs all 13 scenarios sequentially. +# +# A job of its own rather than a leg of the integration matrix: its e2e +# scenarios run the binaries from the package job 4 built, whose glibc floor is +# low enough for all five distros. The fips-linux artifact from job 1 is built +# on the newest runner and would not start on the older ones, and the suite +# used to compile a second copy itself, cold, on every run. The cost of the +# dependency: when the package build fails, the eight scenarios that need no +# binary are skipped along with the five that do. +# +# The leg keeps `suite:` so testing/check-ci-parity.sh matches it against +# DNS_RESOLVER_SUITES in ci-local.sh, and `name:` keeps the check's displayed +# name `Integration (dns-resolver)`. +# ───────────────────────────────────────────────────────────────────────────── + dns-resolver: + name: Integration (${{ matrix.suite }}) + runs-on: ubuntu-latest + needs: [deb-package] + if: ${{ !inputs.skip_integration }} + + strategy: + fail-fast: false + matrix: + include: + - suite: dns-resolver + + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + + - name: Download the .deb + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + name: fips-deb + path: _deb + + - name: Run dns-resolver test + timeout-minutes: 30 + run: | + deb=$(find _deb -maxdepth 1 -type f -name 'fips_*.deb' | sort | head -1) + [ -n "$deb" ] || { echo "no .deb in the downloaded artifact" >&2; exit 1; } + bash testing/dns-resolver/test.sh --deb "$deb" + + - name: Collect logs on failure + if: failure() + run: | + docker ps -a --filter "name=fips-dns-test-" --format '{{.Names}}' | while read -r c; do + echo "--- ${c} fips.service ---" + docker exec "$c" journalctl -u fips.service --no-pager 2>&1 | tail -100 || true + echo "--- ${c} fips-dns.service ---" + docker exec "$c" journalctl -u fips-dns.service --no-pager 2>&1 | tail -100 || true + done + + - name: Stop containers + if: always() + run: | + docker ps -a --filter "name=fips-dns-test-" --format '{{.Names}}' | while read -r c; do + docker rm -f "$c" >/dev/null 2>&1 || true + done + # ───────────────────────────────────────────────────────────────────────────── # Job 5 – Real-deb install across target distros # @@ -854,9 +938,9 @@ jobs: # ordering, real TUN, and the DNS responder filter on a per-distro resolver # backend. # -# A job of its own rather than legs of the integration matrix: the install legs -# are the only ones that need the package, and as integration legs every other -# integration suite would wait on the package build. +# A job of its own rather than legs of the integration matrix: only these legs +# and the dns-resolver job need the package, and as integration legs every +# other integration suite would wait on the package build. # # The legs keep `type: deb-install` and `scenario:` because # testing/check-ci-parity.sh reads those to match this matrix against the local diff --git a/.github/workflows/package-linux.yml b/.github/workflows/package-linux.yml index d4526c4d..0c4db44e 100644 --- a/.github/workflows/package-linux.yml +++ b/.github/workflows/package-linux.yml @@ -79,6 +79,37 @@ jobs: - name: Install host packaging tools run: sudo apt-get update && sudo apt-get install -y --no-install-recommends llvm + # The builder image travels between runners through the Actions cache, + # shared with ci.yml's package job (same script, same key), rather than + # being assembled from apt, rustup and a cargo-deb compile on every leg. + # It is keyed on the image tag the script computes, so any change that + # would rebuild the image locally (base image, toolchain, + # Dockerfile.build) also misses here and cannot pick up a stale image. Every run restores; + # only a push to maint, master or next saves, because the cache is + # scoped per ref and an entry saved by a pull request or a topic branch + # could be read by nothing else while it pushed the cargo caches toward + # the repository's size limit. Topic branches and pull requests read the + # default branch's entry. What this gives up: an image restored from the + # cache is not rebuilt, so, as on a developer's machine, apt and the + # ubuntu:22.04 base are not refreshed until one of the tag's inputs + # changes. The image carries build tools only, and the glibc floor and + # Depends checks still run on every package. + - name: Resolve the builder image cache key + id: builder + shell: bash + run: | + set -euo pipefail + tag=$(bash packaging/debian/build-deb-container.sh --print-image-tag) + [ -n "$tag" ] + echo "key=deb-builder-${{ runner.arch }}-${tag//:/-}" >> "$GITHUB_OUTPUT" + + - name: Restore the builder image + id: builder-restore + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ runner.temp }}/deb-builder-image.tar + key: ${{ steps.builder.outputs.key }} + # Build in the pinned container rather than on the runner. The runner's # glibc is what put a GLIBC_2.39 requirement into every Linux artifact # from v0.3.0 onward, so the package installed cleanly and then could not @@ -99,6 +130,7 @@ jobs: packaging/debian/build-deb-container.sh \ --version "${{ needs.determine-versioning.outputs.linux_package_version }}" \ --output-dir deploy \ + --image-archive "$RUNNER_TEMP/deb-builder-image.tar" \ | tee /tmp/build-deb-container.log # The script prints the package path as its last line of stdout. @@ -123,6 +155,29 @@ jobs: # release job's dist/*.deb glob does not look in. echo "deb=${DEB_FILE#"$PWD"/}" >> "$GITHUB_OUTPUT" + # On a cache miss the archive exists only if the script built the image + # and saved it, so its presence is what says there is something to save. + # A failed build skips this and the save, so no image is cached from a + # job that did not produce a package. + - name: Check for a new builder image archive + id: builder-archive + shell: bash + run: | + if [ -f "$RUNNER_TEMP/deb-builder-image.tar" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + fi + + - name: Save the builder image + if: >- + github.event_name == 'push' + && contains(fromJSON('["refs/heads/maint", "refs/heads/master", "refs/heads/next"]'), github.ref) + && steps.builder-restore.outputs.cache-hit != 'true' + && steps.builder-archive.outputs.present == 'true' + uses: actions/cache/save@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ runner.temp }}/deb-builder-image.tar + key: ${{ steps.builder.outputs.key }} + # The container writes its target directory to a Docker volume, so the # runner's target/release is empty. Recover the four binaries from the # package instead: they are the container-built ones, so the tarball ships diff --git a/CHANGELOG.md b/CHANGELOG.md index 16effa74..c2ad5efe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -263,6 +263,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 against `chacha20` at either version, and 0.10.1 was withdrawn by its maintainer rather than flagged by an advisory. What it buys is that a fresh checkout can resolve the lockfile without reaching for a yanked version. +- The dns-resolver test suite's end-to-end scenarios now run the `fips` and + `fips-gateway` binaries from the Debian package rather than compiling their + own. The suite used to build both in a Debian 12 image with whatever Rust was + current, a second release build on every CI run with no cache, and not the + toolchain or the build that ships. It now takes `--deb PATH`, and CI hands it + the package the install suite installs, so one package build serves both; run + on its own it builds the package through the same container script the + release uses. The GitHub leg moves to a job of its own that waits for the + package build, with its check name unchanged, and a local CI run builds the + package once for both suites. The suite now needs `dpkg-deb` on the host. +- The Linux release and CI package builds reuse their builder image across + GitHub runners instead of assembling it on every leg from apt, rustup and a + compile of `cargo-deb`. `build-deb-container.sh` gains `--print-image-tag` and + `--image-archive PATH`: the workflows key an Actions cache entry on the image + tag, load the image from it when present, and save it after a build. Only a + push to `maint`, `master` or `next` saves an entry; pull requests and topic + branches read the default branch's. A corrupt or mismatched archive is a + warning and a rebuild, never a failed build. A cached image is not refreshed + from apt or the base image until the base image name, the toolchain or + `Dockerfile.build` changes, as was already true of a developer's machine. ## [0.5.1] - 2026-09-06 diff --git a/packaging/debian/build-deb-container.sh b/packaging/debian/build-deb-container.sh index 361e99ef..29f75f59 100755 --- a/packaging/debian/build-deb-container.sh +++ b/packaging/debian/build-deb-container.sh @@ -10,12 +10,19 @@ # releases it did not. # # Usage: build-deb-container.sh [--output-dir DIR] [--version V] [--features LIST] -# [--rebuild-image] +# [--rebuild-image] [--image-archive PATH] +# build-deb-container.sh --print-image-tag # -# Requires docker. The image is cached between runs under a tag made of the -# floor image, the Rust toolchain and a hash of Dockerfile.build, so a change to -# any of the three builds a new image; the source is mounted rather than copied, -# so editing code does not invalidate it. +# Requires docker, except for --print-image-tag. The image is cached between +# runs under a tag made of the floor image, the Rust toolchain and a hash of +# Dockerfile.build, so a change to any of the three builds a new image; the +# source is mounted rather than copied, so editing code does not invalidate it. +# +# --print-image-tag prints that tag and exits. --image-archive carries the image +# between hosts that do not share a docker daemon, such as fresh CI runners: +# when the image is absent and PATH exists it is loaded from there, and when +# this run builds the image it is saved there. A bad archive is warned about and +# the image rebuilt; it never fails the build. set -euo pipefail @@ -29,6 +36,8 @@ DEST_DIR="$REPO_ROOT/deploy" VERSION="" FEATURES="" REBUILD_IMAGE=0 +PRINT_TAG=0 +IMAGE_ARCHIVE="" while [[ $# -gt 0 ]]; do case "$1" in @@ -36,16 +45,13 @@ while [[ $# -gt 0 ]]; do --version) VERSION="${2:?missing value for --version}"; shift 2 ;; --features) FEATURES="${2:?missing value for --features}"; shift 2 ;; --rebuild-image) REBUILD_IMAGE=1; shift ;; - -h|--help) sed -n '2,18p' "$0"; exit 0 ;; + --print-image-tag) PRINT_TAG=1; shift ;; + --image-archive) IMAGE_ARCHIVE="${2:?missing value for --image-archive}"; shift 2 ;; + -h|--help) sed -n '2,25p' "$0"; exit 0 ;; *) echo "Unknown option: $1" >&2; exit 2 ;; esac done -command -v docker >/dev/null 2>&1 || { - echo "build-deb-container: docker is required and was not found." >&2 - exit 2 -} - # Read the toolchain from the pin rather than choosing one here, and put it in # the tag so a bump rebuilds the image instead of silently reusing a stale one. # The Dockerfile's content goes in the tag for the same reason: without it a @@ -64,7 +70,50 @@ DOCKERFILE_HASH=$(sha256sum "$SCRIPT_DIR/Dockerfile.build" | cut -c1-12) || DOCK IMAGE_TAG="fips-deb-builder:${FIPS_BUILD_IMAGE//[:\/]/-}-rust${RUST_TOOLCHAIN}-${DOCKERFILE_HASH}" -if [ "$REBUILD_IMAGE" -eq 1 ] || ! docker image inspect "$IMAGE_TAG" >/dev/null 2>&1; then +# Before the docker check, so a workflow can key a cache on the tag without +# docker being involved. +if [ "$PRINT_TAG" -eq 1 ]; then + printf '%s\n' "$IMAGE_TAG" + exit 0 +fi + +command -v docker >/dev/null 2>&1 || { + echo "build-deb-container: docker is required and was not found." >&2 + exit 2 +} + +# A problem with the image archive is a warning, not a failure: the archive only +# saves time, and failing a release over a bad cache entry would hold the tag +# until someone removed the entry by hand. The ::warning:: line puts it on the +# GitHub run summary; the plain line is for everywhere else. +archive_warning() { + echo "::warning::build-deb-container: $*" + echo "build-deb-container: warning: $*" >&2 +} + +# Stays unset when the image was found or loaded, so only an image this run +# built is saved: saving a loaded one would only rewrite the archive it came from. +BUILT_IMAGE=0 +if [ "$REBUILD_IMAGE" -eq 1 ]; then + BUILT_IMAGE=1 +elif docker image inspect "$IMAGE_TAG" >/dev/null 2>&1; then + echo "=== Using cached $IMAGE_TAG ===" >&2 +elif [ -n "$IMAGE_ARCHIVE" ] && [ -f "$IMAGE_ARCHIVE" ]; then + echo "=== Loading $IMAGE_TAG from $IMAGE_ARCHIVE ===" >&2 + if ! docker load -i "$IMAGE_ARCHIVE" >&2; then + archive_warning "could not load $IMAGE_ARCHIVE; building $IMAGE_TAG instead" + BUILT_IMAGE=1 + elif ! docker image inspect "$IMAGE_TAG" >/dev/null 2>&1; then + archive_warning "$IMAGE_ARCHIVE does not hold $IMAGE_TAG; building it instead" + BUILT_IMAGE=1 + else + echo "=== Using $IMAGE_TAG loaded from $IMAGE_ARCHIVE ===" >&2 + fi +else + BUILT_IMAGE=1 +fi + +if [ "$BUILT_IMAGE" -eq 1 ]; then echo "=== Building $IMAGE_TAG from $FIPS_BUILD_IMAGE with Rust $RUST_TOOLCHAIN ===" >&2 docker build \ --build-arg "BASE=$FIPS_BUILD_IMAGE" \ @@ -72,8 +121,19 @@ if [ "$REBUILD_IMAGE" -eq 1 ] || ! docker image inspect "$IMAGE_TAG" >/dev/null -t "$IMAGE_TAG" \ -f "$SCRIPT_DIR/Dockerfile.build" \ "$SCRIPT_DIR" -else - echo "=== Using cached $IMAGE_TAG ===" >&2 + + # Written to a temporary name and renamed, so a failed or interrupted save + # never leaves a truncated archive where a cache step would pick it up. + if [ -n "$IMAGE_ARCHIVE" ]; then + ARCHIVE_TMP="$IMAGE_ARCHIVE.tmp.$$" + if docker save "$IMAGE_TAG" -o "$ARCHIVE_TMP" >&2 \ + && mv -f "$ARCHIVE_TMP" "$IMAGE_ARCHIVE"; then + echo "=== Saved $IMAGE_TAG to $IMAGE_ARCHIVE ===" >&2 + else + rm -f "$ARCHIVE_TMP" + archive_warning "could not save $IMAGE_TAG to $IMAGE_ARCHIVE; the next run will build it again" + fi + fi fi # Derive the version and the timestamp on the host and pass both in, because a diff --git a/testing/README.md b/testing/README.md index 4a03c444..aeb6aec2 100644 --- a/testing/README.md +++ b/testing/README.md @@ -102,7 +102,9 @@ and exchanges datagrams on it with no TUN device and no IPv6 emulation. Runs `fips-dns-setup` against each supported Linux resolver backend in systemd containers, verifying backend detection, generated config and teardown, plus an end-to-end scenario that resolves a `.fips` name -through the configured backend. +through the configured backend. The end-to-end scenarios run the +binaries from a Debian package: `--deb PATH` supplies one, and without +it the suite builds one through `packaging/debian/build-deb-container.sh`. ### [deb-install/](deb-install/) -- Debian Package Install diff --git a/testing/check-ci-parity.sh b/testing/check-ci-parity.sh index 9c2fd897..d0b029f5 100755 --- a/testing/check-ci-parity.sh +++ b/testing/check-ci-parity.sh @@ -27,6 +27,8 @@ # dns-resolver is the one leg still compared at leg granularity rather than # per scenario: it is a single leg and a single suite on both sides, and it # runs all of its scenarios internally. Its scenario list is NOT cross-checked. +# On GitHub it is the one leg of a job of its own (it waits for the package +# build), which the sweep across every job below finds like any other leg. # # The local suite set is discovered by sweeping ci-local.sh for *_SUITES arrays # rather than from a hardcoded list of variable names, and every run_suite diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 76d5f4cb..daaa7de3 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -118,7 +118,9 @@ # guard compares through that shape rather than around it: chaos legs are # compared per scenario (and per flag), deb-install legs per distro. The one # leg still compared at leg granularity is dns-resolver — a single suite on -# both sides that runs all of its scenarios internally. +# both sides that runs all of its scenarios internally. On GitHub it is a job +# of its own, because it runs the package's binaries and so waits for the +# package build. # ───────────────────────────────────────────────────────────────────────────── set -uo pipefail @@ -1007,9 +1009,16 @@ run_native_api() { } # Run dns-resolver harness (multi-distro + e2e scenarios) +# +# Its e2e scenarios run the fips binaries from the package build_ci_deb +# produces, the same package deb-install installs, as the GitHub job does. run_dns_resolver() { - info "[dns-resolver] Running multi-distro test (slow — builds per-distro images)" - if bash testing/dns-resolver/test.sh 2>&1; then + if ! build_ci_deb dns-resolver; then + record "dns-resolver" "$CI_DEB_RC" + return + fi + info "[dns-resolver] Running multi-distro test against $CI_DEB_PATH (slow — builds per-distro images)" + if bash testing/dns-resolver/test.sh --deb "$CI_DEB_PATH" 2>&1; then record "dns-resolver" 0 else record "dns-resolver" 1 @@ -1030,14 +1039,29 @@ run_dns_resolver() { # ate into it would shrink theirs. Neither phase is left unbounded, which is # the property this exists for. DEB_INSTALL_TIMEOUT=${DEB_INSTALL_TIMEOUT:-2400} -run_deb_install() { - # Build once through the shared container script, then install that one - # artifact into every distro. The harness would build its own package if - # handed none, and that fallback goes through the same script -- but the - # GitHub job builds explicitly and passes `--deb`, so doing it explicitly - # here too makes the two systems read as the same work rather than leaving - # a reader to discover that a fallback happens to match. - info "[deb-install] Building the package in the pinned container" + +# Build the package once per process through the shared container script, for +# every suite that consumes it: deb-install installs it and dns-resolver runs +# its binaries. The GitHub workflow builds it once in its own job and hands the +# artifact to both, so doing the same here keeps the two systems the same work. +# The first caller builds; later callers get the stored result, a failure +# included, so a failed build is not retried and reported twice as two builds. +# Sets CI_DEB_PATH on success and CI_DEB_RC (the value to record) on failure. +CI_DEB_DONE=0 +CI_DEB_PATH="" +CI_DEB_RC=0 +build_ci_deb() { + local suite="$1" + if [[ $CI_DEB_DONE -eq 1 ]]; then + if [[ $CI_DEB_RC -ne 0 ]]; then + echo " ERROR: the package build this suite shares failed earlier in the run (exit $CI_DEB_RC)." >&2 + return 1 + fi + info "[$suite] Reusing the package built earlier in this run: $CI_DEB_PATH" + return 0 + fi + CI_DEB_DONE=1 + info "[$suite] Building the package in the pinned container" local build_log deb rc=0 build_log=$(mktemp "/tmp/ci-deb-install-build.XXXXXX") # stdout carries the package path on its last line, so it is captured; @@ -1049,23 +1073,37 @@ run_deb_install() { echo " ERROR: the container build exceeded ${DEB_INSTALL_TIMEOUT}s and was killed;" >&2 echo " no package was produced, so this is not an assertion failure." >&2 else - echo " ERROR: the container build failed (exit $rc); no package to install." >&2 + echo " ERROR: the container build failed (exit $rc); no package for $suite." >&2 fi rm -f "$build_log" - record "deb-install" "$rc" - return + CI_DEB_RC=$rc + return 1 fi deb=$(tail -n 1 "$build_log") rm -f "$build_log" if [[ -z "$deb" || ! -f "$deb" ]]; then echo " ERROR: the container build reported success but its last line of stdout" >&2 echo " was not a package path: '${deb}'" >&2 - record "deb-install" 1 + CI_DEB_RC=1 + return 1 + fi + CI_DEB_PATH="$deb" + return 0 +} + +run_deb_install() { + # Install the one package build_ci_deb built into every distro. The harness + # would build its own package if handed none, and that fallback goes through + # the same script -- but the GitHub job builds explicitly and passes + # `--deb`, so doing it explicitly here too makes the two systems read as the + # same work rather than leaving a reader to discover that a fallback happens + # to match. The build is shared with dns-resolver. + if ! build_ci_deb deb-install; then + record "deb-install" "$CI_DEB_RC" return fi - + local deb="$CI_DEB_PATH" rc=0 info "[deb-install] Running multi-distro install test against $deb" - rc=0 timeout "$DEB_INSTALL_TIMEOUT" bash testing/deb-install/test.sh --deb "$deb" 2>&1 || rc=$? if [[ $rc -eq 124 ]]; then # Say so explicitly. A bare red here reads as a failed assertion, and diff --git a/testing/dns-resolver/test.sh b/testing/dns-resolver/test.sh index 38ef5bfd..3dccef8b 100755 --- a/testing/dns-resolver/test.sh +++ b/testing/dns-resolver/test.sh @@ -8,20 +8,23 @@ # script, verifies the detected backend and generated config, runs # teardown, and verifies cleanup. # -# The end-to-end scenario additionally builds fips in a Debian 12 -# builder image (cached between runs) so the binary is glibc-compatible -# across all target distros. It then starts the daemon, configures DNS -# via the script, and confirms `dig @127.0.0.53 AAAA .fips` -# returns a non-empty AAAA answer. +# The end-to-end scenarios take fips and fips-gateway from the Debian +# package, which is built in the pinned floor container and so runs on +# every target distro. Each starts the daemon, configures DNS via the +# script, and confirms `dig @127.0.0.53 AAAA .fips` returns a +# non-empty AAAA answer. # -# Usage: ./test.sh [scenario ...] -# No args = run all scenarios. -# Named args = run only those (e.g., ./test.sh debian12-resolved e2e-debian12) +# Usage: ./test.sh [--deb PATH] [scenario ...] +# --deb PATH = take the binaries from this package. Without it the +# e2e scenarios build the package through +# packaging/debian/build-deb-container.sh. +# No scenarios = run all scenarios. +# Named scenarios = run only those (e.g., ./test.sh debian12-resolved e2e-debian12) # # Requirements: Docker able to grant SYS_ADMIN and NET_ADMIN and an # unconfined AppArmor profile (the containers are not privileged; see -# testing/lib/systemd-container.sh). The e2e scenario also needs -# /dev/net/tun on the host (standard). +# testing/lib/systemd-container.sh). The e2e scenarios also need +# /dev/net/tun on the host (standard) and dpkg-deb to read the package. set -uo pipefail @@ -35,6 +38,11 @@ CACHE_DIR="$SCRIPT_DIR/.cache" FIPS_BIN_CACHE="$CACHE_DIR/fips" FIPS_GATEWAY_BIN_CACHE="$CACHE_DIR/fips-gateway" +# Set by --deb. BINARIES_READY keeps the package unpack to a single +# operation however many e2e scenarios run in one process. +SUPPLIED_DEB="" +BINARIES_READY=0 + # Timeout for systemd boot inside container BOOT_TIMEOUT=30 @@ -312,99 +320,89 @@ verify_resolved_backend() { } # ───────────────────────────────────────────────────────────────────── -# Build the fips binary once in a Debian 12 builder image so it's -# glibc-compatible with every target distro (Debian 12/13, Ubuntu 22/24). -# Cached at testing/dns-resolver/.cache/fips between runs; rebuild if -# any source file is newer than the cached binary. +# Take fips and fips-gateway from the Debian package rather than +# compiling them here. The package is built in the pinned floor +# container (packaging/build-floor.env), whose glibc is the lowest of +# every target distro, so its binaries run in all five e2e images. The +# suite used to compile its own copy in a Debian 12 image with whatever +# Rust was current, which was a second, uncached release build per run +# and was not the toolchain or the build that ships. +# +# With --deb the caller's package is used, which is how CI runs it: +# one package build serves this suite and the install suite. Without +# it, the package is built through the same container script the +# release uses. Done once per process however many e2e scenarios run. # ───────────────────────────────────────────────────────────────────── -build_fips_for_e2e() { - mkdir -p "$CACHE_DIR" - - if [ -f "$FIPS_BIN_CACHE" ] && [ -f "$FIPS_GATEWAY_BIN_CACHE" ]; then - local newest_src - newest_src=$(find "$REPO_ROOT/src" "$REPO_ROOT/Cargo.toml" "$REPO_ROOT/Cargo.lock" \ - -type f -printf '%T@\n' 2>/dev/null | sort -nr | head -1) - local cached_age - cached_age=$(stat -c '%Y' "$FIPS_BIN_CACHE" 2>/dev/null || echo 0) - local cached_gateway_age - cached_gateway_age=$(stat -c '%Y' "$FIPS_GATEWAY_BIN_CACHE" 2>/dev/null || echo 0) - local oldest_cached=$((cached_age < cached_gateway_age ? cached_age : cached_gateway_age)) - if awk "BEGIN { exit !($oldest_cached >= $newest_src) }"; then - log "Using cached fips + fips-gateway binaries at $CACHE_DIR" - return 0 - fi - log "Cached binaries are stale, rebuilding" - else - log "No cached binaries, building" +prepare_binaries() { + if [ "$BINARIES_READY" -eq 1 ]; then + return 0 fi - local builder_tag="fips-dns-test:builder" - log "Building Debian 12 builder image (this may take a few minutes on first run)" - docker build -t "$builder_tag" -f - "$REPO_ROOT" <<'DOCKERFILE' >/dev/null -FROM debian:12 -ENV DEBIAN_FRONTEND=noninteractive -RUN apt-get update && apt-get install -y --no-install-recommends \ - build-essential pkg-config libdbus-1-dev curl ca-certificates \ - libclang-dev clang && \ - apt-get clean && rm -rf /var/lib/apt/lists/* -RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | \ - sh -s -- -y --default-toolchain stable --profile minimal -ENV PATH="/root/.cargo/bin:${PATH}" -WORKDIR /src -COPY Cargo.toml Cargo.lock build.rs ./ -COPY src ./src -RUN cargo build --release --bin fips --bin fips-gateway -DOCKERFILE - - if [ ! "$(docker images -q "$builder_tag" 2>/dev/null)" ]; then - echo " ERROR: builder image build failed" + if ! command -v dpkg-deb >/dev/null 2>&1; then + echo " ERROR: dpkg-deb is required to read the package and was not found" >&2 return 1 fi - log "Extracting fips + fips-gateway binaries from builder image" - # Drop the previous run's binaries before extracting. Without this, a failed - # extraction below leaves them in place, they satisfy the caller's -x check, - # and the e2e scenarios silently exercise the previous commit's code. + mkdir -p "$CACHE_DIR" + local deb + if [ -n "$SUPPLIED_DEB" ]; then + deb="$SUPPLIED_DEB" + log "Using the supplied package $(basename "$deb")" + else + # The cache holds one package at a time, and the package used is + # the one the build names on the last line of its stdout, never one + # found by listing the directory (as in deb-install/test.sh). + log "Building the .deb in the pinned build container (slow on first run)" + mkdir -p "$CACHE_DIR/deb" + rm -f "$CACHE_DIR"/deb/*.deb + local build_out + if ! build_out=$(bash "$REPO_ROOT/packaging/debian/build-deb-container.sh" \ + --output-dir "$CACHE_DIR/deb"); then + echo " ERROR: container build failed" >&2 + return 1 + fi + deb=$(printf '%s\n' "$build_out" | tail -n 1) + if [ -z "$deb" ] || [ ! -f "$deb" ]; then + echo " ERROR: the container build did not report a package path: '$deb'" >&2 + return 1 + fi + fi + + # Drop the previous run's binaries before extracting. Without this, a + # failed extraction below leaves them in place and the e2e scenarios + # silently exercise the previous commit's code. rm -f "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE" - # stderr goes to its own file rather than into $cid: docker prints - # warnings (a platform mismatch, say) on success too, and folding them - # into the id would leave every later reference pointing at nothing. - local cid err errfile - errfile=$(mktemp) - if ! cid=$(docker create "$builder_tag" 2>"$errfile"); then - echo " ERROR: docker create failed: $(cat "$errfile")" - rm -f "$errfile" + local tmp err + tmp=$(mktemp -d) + if ! err=$(dpkg-deb -x "$deb" "$tmp" 2>&1); then + echo " ERROR: dpkg-deb could not unpack $deb: $err" >&2 + rm -rf "$tmp" return 1 fi - rm -f "$errfile" - local rc=0 spec bin dest + local spec bin dest for spec in "fips:$FIPS_BIN_CACHE" "fips-gateway:$FIPS_GATEWAY_BIN_CACHE"; do bin="${spec%%:*}" dest="${spec#*:}" - if ! err=$(docker cp "$cid:/src/target/release/$bin" "$dest" 2>&1); then - echo " ERROR: extracting $bin from the builder image failed: $err" - rc=1 + if [ ! -f "$tmp/usr/bin/$bin" ] || [ ! -s "$tmp/usr/bin/$bin" ]; then + echo " ERROR: $(basename "$deb") has no usable /usr/bin/$bin" >&2 + rm -rf "$tmp" + return 1 fi - done - docker rm "$cid" >/dev/null - [ "$rc" -eq 0 ] || return 1 - - if ! chmod +x "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"; then - echo " ERROR: chmod +x failed on the extracted binaries" - return 1 - fi - - for dest in "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"; do - if [ ! -s "$dest" ] || [ ! -x "$dest" ]; then - echo " ERROR: extracted binary missing, empty or not executable: $dest" + if ! install -m 0755 "$tmp/usr/bin/$bin" "$dest"; then + echo " ERROR: could not install $bin to $dest" >&2 + rm -rf "$tmp" return 1 fi done + rm -rf "$tmp" - log "Cached fips ($(stat -c %s "$FIPS_BIN_CACHE") bytes) + fips-gateway ($(stat -c %s "$FIPS_GATEWAY_BIN_CACHE") bytes)" + for dest in "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"; do + log "$(basename "$dest"): $(stat -c %s "$dest") bytes, sha256 $(sha256sum "$dest" | cut -d' ' -f1)" + done + BINARIES_READY=1 return 0 } @@ -726,9 +724,8 @@ DOCKERFILE # DNS via the script, dig through systemd-resolved. # # Parameterized across Debian 12/13 and Ubuntu 22/24/26. The fips -# and fips-gateway binaries are built once in a Debian 12 builder -# image (lowest glibc target → forward-compatible with all newer -# distros) and copied into each per-distro runtime image. +# and fips-gateway binaries come from the package once per run (see +# prepare_binaries) and are copied into each per-distro runtime image. # ───────────────────────────────────────────────────────────────────── # Args: @@ -748,7 +745,7 @@ _run_e2e_scenario() { local image="fips-dns-test:e2e-${distro_label}" log "End-to-end: ${base_image} + systemd-resolved + real fips + fips-gateway + dig" - build_fips_for_e2e || { fail "fips build failed"; return; } + prepare_binaries || { fail "could not prepare the fips binaries"; return; } if [ ! -x "$FIPS_BIN_CACHE" ] || [ ! -x "$FIPS_GATEWAY_BIN_CACHE" ]; then fail "binaries not available at $CACHE_DIR" @@ -980,6 +977,36 @@ test_e2e_ubuntu26() { _run_e2e_scenario ubuntu26 ubuntu:26.04 "$_pkgs_with_re ALL_SCENARIOS="debian12-resolved debian13-resolved ubuntu22-resolved ubuntu24-resolved ubuntu26-resolved dnsmasq nm-dnsmasq no-resolver e2e-debian12 e2e-debian13 e2e-ubuntu22 e2e-ubuntu24 e2e-ubuntu26" +# A missing package is refused here, before any scenario runs, rather +# than surfacing as a failure of the first e2e scenario. +_args=() +while [ $# -gt 0 ]; do + case "$1" in + --deb) + SUPPLIED_DEB="${2:?--deb requires a path}" + if [ ! -f "$SUPPLIED_DEB" ]; then + echo "--deb $SUPPLIED_DEB does not exist" >&2 + exit 2 + fi + shift 2 + ;; + -h|--help) + echo "usage: test.sh [--deb PATH] [scenario ...]" + echo "scenarios: $ALL_SCENARIOS" + exit 0 + ;; + -*) + echo "Unknown option: $1" >&2 + exit 1 + ;; + *) + _args+=("$1") + shift + ;; + esac +done +set -- ${_args[@]+"${_args[@]}"} + if [ $# -eq 0 ]; then scenarios="$ALL_SCENARIOS" else From db254f6d44812925f0e6d589f828d2c3dda9f2b4 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 11:11:05 +0000 Subject: [PATCH 7/7] Build and install-test the arm64 package in CI The arm64 .deb that ships to single-board hosts was floor-checked and never installed anywhere in the pipeline, so a packaging fault that only appears on arm64 would reach a user first. CI's package job becomes a two-leg matrix, building natively on an ubuntu-24.04-arm runner beside the amd64 leg, and each leg fails unless the package it produced is its own architecture. The install job gains one arm64 leg on ubuntu22, the oldest supported distribution: a fresh install and a daemon start. The upgrade, purge and conffile scenarios stay amd64-only. The displayed names of the existing checks are unchanged. The parity guard now reads each install leg's arch. Only amd64 legs are compared with the local distro list; an arm64 leg must name a known distro and is reported as GitHub-only. Without this, deleting the amd64 ubuntu22 leg would have passed, because the arm64 leg still supplied the distro name. --- .github/workflows/ci.yml | 63 ++++++++++++++++++++++++++++++++------ CHANGELOG.md | 7 +++++ README.md | 6 ++-- testing/README.md | 4 ++- testing/check-ci-parity.sh | 34 ++++++++++++++++++-- testing/ci-local.sh | 5 +++ 6 files changed, 104 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83aa84ad..d53d00ce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,10 @@ env: # unreliable on GitHub-hosted runners. # tor-directory — same; live Tor dependency. # +# Deliberate GitHub-only: the arm64 install leg (ubuntu22). The local host is +# x86_64 and has no arm64 execution; the leg is compared by distribution only +# and does not stand in for the amd64 leg of the same distribution. +# # The two runners express the same work in different matrix shapes, and the # parity guard compares through that shape rather than around it: chaos legs # are compared per scenario (and per flag) via their `scenario:` field, @@ -787,11 +791,24 @@ jobs: # floor violation stops the run. # ───────────────────────────────────────────────────────────────────────────── deb-package: - name: Build .deb - runs-on: ubuntu-latest + name: Build .deb${{ matrix.deb_arch == 'arm64' && ' (arm64)' || '' }} + runs-on: ${{ matrix.os }} needs: [build, test] if: ${{ !inputs.skip_integration }} + # The arm64 leg builds natively on an arm runner so the arm64 package the + # release ships is install-tested too (job 5). Being one job, both legs + # gate job 5 and the dns-resolver job: an arm64 build failure skips the + # amd64 install legs on that run as well. + strategy: + fail-fast: false + matrix: + include: + - os: ubuntu-latest + deb_arch: amd64 + - os: ubuntu-24.04-arm + deb_arch: arm64 + steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 @@ -824,11 +841,23 @@ jobs: path: ${{ runner.temp }}/deb-builder-image.tar key: ${{ steps.builder.outputs.key }} + # The package path is the script's last line of stdout. A leg that + # produced the other architecture's package goes red here rather than + # handing an amd64 package to the arm64 install leg. - name: Build the .deb in the pinned build container timeout-minutes: 30 + shell: bash run: | + set -euo pipefail bash packaging/debian/build-deb-container.sh --output-dir deploy \ - --image-archive "$RUNNER_TEMP/deb-builder-image.tar" + --image-archive "$RUNNER_TEMP/deb-builder-image.tar" \ + | tee "$RUNNER_TEMP/build-deb-container.log" + deb=$(tail -n 1 "$RUNNER_TEMP/build-deb-container.log") + [ -f "$deb" ] || { echo "build-deb-container.sh did not name a package: '$deb'" >&2; exit 1; } + case "$deb" in + *_${{ matrix.deb_arch }}.deb) ;; + *) echo "Package $deb is not ${{ matrix.deb_arch }}" >&2; exit 1 ;; + esac # On a cache miss the archive exists only if the script built the image # and saved it, so its presence is what says there is something to save. @@ -856,8 +885,8 @@ jobs: - name: Upload the .deb uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: fips-deb - path: deploy/fips_*.deb + name: fips-deb-${{ matrix.deb_arch }} + path: deploy/fips_*_${{ matrix.deb_arch }}.deb if-no-files-found: error retention-days: 1 @@ -901,7 +930,7 @@ jobs: - name: Download the .deb uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: - name: fips-deb + name: fips-deb-amd64 path: _deb - name: Run dns-resolver test @@ -944,11 +973,12 @@ jobs: # # The legs keep `type: deb-install` and `scenario:` because # testing/check-ci-parity.sh reads those to match this matrix against the local -# suite's distro list; the steps below use `scenario:` only. +# suite's distro list; it reads `arch:` too, and compares only the amd64 legs +# with the local run. The steps below use `scenario:` and `arch:`. # ───────────────────────────────────────────────────────────────────────────── deb-install: - name: Deb install (${{ matrix.scenario }}) - runs-on: ubuntu-latest + name: Deb install (${{ matrix.scenario }}${{ matrix.arch == 'arm64' && ' arm64' || '' }}) + runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} needs: [deb-package] if: ${{ !inputs.skip_integration }} @@ -958,14 +988,27 @@ jobs: include: - type: deb-install scenario: debian12 + arch: amd64 - type: deb-install scenario: debian13 + arch: amd64 - type: deb-install scenario: ubuntu22 + arch: amd64 - type: deb-install scenario: ubuntu24 + arch: amd64 - type: deb-install scenario: ubuntu26 + arch: amd64 + # The arm64 package on the oldest supported distribution: a fresh + # install and a daemon start. Deliberately GitHub-only (the local host + # is x86_64), and deliberately one leg: the upgrade, purge and + # conffile paths run under debian12 on amd64 only and stay + # unexercised on arm64. + - type: deb-install + scenario: ubuntu22 + arch: arm64 steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 @@ -973,7 +1016,7 @@ jobs: - name: Download the .deb uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: - name: fips-deb + name: fips-deb-${{ matrix.arch }} path: _deb - name: Run deb-install scenario diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ad5efe..24b0e88c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -283,6 +283,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 warning and a rebuild, never a failed build. A cached image is not refreshed from apt or the base image until the base image name, the toolchain or `Dockerfile.build` changes, as was already true of a developer's machine. +- CI now builds the arm64 `.deb` on an arm64 runner and installs it on Ubuntu + 22.04, the oldest supported distribution, starting the daemon, on every push + and pull request. Until now the arm64 package was floor-checked and never + installed anywhere in the pipeline. Its upgrade, purge and conffile paths + remain unexercised; those run on amd64 only. The parity check reads each + install leg's architecture, so the arm64 leg is reported as GitHub-only and + cannot stand in for a missing amd64 leg of the same distribution. ## [0.5.1] - 2026-09-06 diff --git a/README.md b/README.md index 6c26c371..1af760f1 100644 --- a/README.md +++ b/README.md @@ -195,8 +195,10 @@ below. **Only the `.deb` is exercised by an install test**, by the `deb-install` suite across debian12, debian13, ubuntu22, ubuntu24 and ubuntu26; neither the AUR package nor the flake is. That suite runs on every push and pull request, on x86_64, against a `.deb` built by the same -pinned container as the released one. It does not run at a tag, and the -arm64 package is install-tested by nothing: no workflow installs a published +pinned container as the released one. The arm64 package, built the same way +on an arm64 runner, is installed and its daemon started on ubuntu22 on every +push and pull request as well; its upgrade, purge and conffile paths are not +exercised. The suite does not run at a tag: no workflow installs a published artifact, so the released packages are checked by hand. OpenWrt is a musl target rather than glibc, and it takes an `.ipk` on 24.x and earlier or diff --git a/testing/README.md b/testing/README.md index aeb6aec2..2c006fac 100644 --- a/testing/README.md +++ b/testing/README.md @@ -110,7 +110,9 @@ it the suite builds one through `packaging/debian/build-deb-container.sh`. Installs the built `.deb` in systemd containers for each target distro and verifies unit enablement, conffile placement and -end-to-end `.fips` resolution as a user would meet it. +end-to-end `.fips` resolution as a user would meet it. GitHub CI also +installs the arm64 package on ubuntu22 on an arm64 runner, a leg the +local run cannot have and the parity check reports as GitHub-only. ### [boringtun/](boringtun/) -- WireGuard Throughput Baseline diff --git a/testing/check-ci-parity.sh b/testing/check-ci-parity.sh index d0b029f5..d69eeaad 100755 --- a/testing/check-ci-parity.sh +++ b/testing/check-ci-parity.sh @@ -11,6 +11,11 @@ # unreliable on GitHub-hosted runners. # tor-directory — same; live Tor dependency. # +# Deliberate GitHub-only (NOT in the local run), with reason: +# deb-install ubuntu22 on arm64 — the local host is x86_64 and cannot run an +# arm64 package. The GitHub leg installs the arm64 package +# and starts the daemon on an arm runner. +# # What is compared, and at what granularity: # chaos — per scenario, plus its flags. GitHub fans each scenario # into its own matrix leg carrying `scenario:` (and @@ -21,7 +26,11 @@ # deb-install — per distro. GitHub splits into per-distro legs carrying # `scenario:`, in a job of their own; local runs the same # distro set in one suite, enumerated by ALL_SCENARIOS in -# deb-install/test.sh. +# deb-install/test.sh. A leg's `arch:` defaults to amd64, +# and only amd64 legs are compared with the local set, so +# an arm64 leg cannot stand in for a missing amd64 leg of +# the same distro. An arm64 leg must name a distro the +# local suite knows; any other arch is unidentifiable. # everything else — per suite name. # # dns-resolver is the one leg still compared at leg granularity rather than @@ -153,6 +162,10 @@ if not include: f"{ci_yml_path}; cannot verify CI parity", file=sys.stderr) sys.exit(2) github_chaos, github_deb, github = {}, set(), set() +# Deliberate GitHub-only install legs on another architecture, by distro. Kept +# out of github_deb: were they in it, deleting the amd64 leg of a distro that +# also has an arm64 leg would leave the distro in the set and pass. +github_deb_extra = {} malformed = [] for leg in include: if "suite" not in leg and "scenario" not in leg: @@ -166,8 +179,15 @@ for leg in include: continue if kind == "chaos": github_chaos[str(leg["scenario"])] = str(leg.get("chaos_flags", "")) - else: + continue + arch = str(leg.get("arch", "amd64")) + if arch == "amd64": github_deb.add(str(leg["scenario"])) + elif arch == "arm64": + github_deb_extra.setdefault(arch, set()).add(str(leg["scenario"])) + else: + malformed.append(f"deb-install leg {leg['scenario']} has arch " + f"{arch}, which is neither amd64 nor arm64") elif "suite" in leg: github.add(str(leg["suite"])) else: @@ -233,6 +253,12 @@ chaos_flag_drift = sorted( ) deb_local_only = sorted(local_deb - github_deb) deb_github_only = sorted(github_deb - local_deb) +# An extra-arch leg for a distro the local suite does not know is still drift. +deb_github_only += sorted( + f"{d} ({arch})" + for arch, distros in github_deb_extra.items() + for d in distros - local_deb +) problems = (local_only or github_only or chaos_local_only or chaos_github_only or chaos_flag_drift or deb_local_only or deb_github_only @@ -290,5 +316,9 @@ print("CI parity OK: both runners cover the same work " print(f" {len(github)} suites, {len(github_chaos)} chaos scenarios " f"(flags compared), {len(github_deb)} deb-install distros " f"— {total} legs on each side.") +for arch, distros in sorted(github_deb_extra.items()): + legs = "leg" if len(distros) == 1 else "legs" + print(f" plus {len(distros)} GitHub-only {arch} install {legs} " + f"({', '.join(sorted(distros))}).") sys.exit(0) PY diff --git a/testing/ci-local.sh b/testing/ci-local.sh index daaa7de3..ecab9d5e 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -114,6 +114,11 @@ # unreliable on GitHub-hosted runners. # tor-directory — same; live Tor dependency. # +# Deliberate GitHub-only (NOT in this local run), with reason: +# deb-install ubuntu22 on arm64 — this host is x86_64 and cannot run an +# arm64 package. The guard compares it by distro only and +# does not let it stand in for the amd64 ubuntu22 leg. +# # The two runners express the same work in different matrix shapes, and the # guard compares through that shape rather than around it: chaos legs are # compared per scenario (and per flag), deb-install legs per distro. The one