diff --git a/testing/acl-allowlist/.gitignore b/testing/acl-allowlist/.gitignore index 31315f6..b06c7df 100644 --- a/testing/acl-allowlist/.gitignore +++ b/testing/acl-allowlist/.gitignore @@ -1 +1 @@ -generated-configs +generated-configs* diff --git a/testing/acl-allowlist/README.md b/testing/acl-allowlist/README.md index 6c5c231..95d0f2e 100644 --- a/testing/acl-allowlist/README.md +++ b/testing/acl-allowlist/README.md @@ -79,6 +79,12 @@ Docker service/container/hostname identifiers in this harness intentionally use `node-a` through `node-f`. For data-plane checks and operator examples, use the explicit FIPS names such as `node-a.fips` and `node-d.fips`. +The bridge network requests no subnet, so docker assigns one from its own +address pool and two concurrent runs of this harness never contend for a fixed +range. Consequently no node's IPv4 address is known before startup, and the +generated peer stanzas address each other by docker hostname (`host-a` … +`host-f`), resolved through the container's dnsmasq to docker's embedded DNS. + ACL paths are fixed in this branch: - `/etc/fips/peers.allow` @@ -93,7 +99,9 @@ Mounted ACL files in this harness: Generated fixture location: -- `testing/acl-allowlist/generated-configs/` +- `testing/acl-allowlist/generated-configs/`, or + `generated-configs/` when `FIPS_CI_NAME_SUFFIX` is set, which is how + concurrent runs keep their fixtures apart Inspect peer state: diff --git a/testing/acl-allowlist/docker-compose.yml b/testing/acl-allowlist/docker-compose.yml index abef96d..7d731d9 100644 --- a/testing/acl-allowlist/docker-compose.yml +++ b/testing/acl-allowlist/docker-compose.yml @@ -1,11 +1,21 @@ networks: + # No subnet is requested: docker assigns one from the daemon's address pool. + # A fixed request is honoured verbatim, so two runs asking for the same range + # collide on "Pool overlaps" — which is why the generated peer addresses are + # docker hostnames (host-a … host-f) rather than literal IPs. Docker's + # embedded DNS is per-network, so the same hostname in two concurrent runs + # resolves inside each run's own subnet. + # + # That removes one of the two obstacles to running this suite twice at once, + # not both. The compose project name is still fixed, so two runs that do not + # set COMPOSE_PROJECT_NAME share a project, and the second `up` recreates the + # first's containers while either `down` removes both. Nothing scopes it for + # this suite: it is run by hand only, so the caller has to. Do not read the + # floating subnet as making concurrent bare runs safe. acl-net: driver: bridge labels: - "com.corganlabs.fips-ci=1" - ipam: - config: - - subnet: 172.31.0.0/24 x-fips-common: &fips-common build: @@ -32,14 +42,13 @@ services: hostname: host-a volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-a/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-a/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-a/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-a/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-a/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.10 + - acl-net service-b: <<: *fips-common @@ -47,14 +56,13 @@ services: hostname: host-b volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-b/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-b/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-b/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-b/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-b/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.11 + - acl-net service-c: <<: *fips-common @@ -62,14 +70,13 @@ services: hostname: host-c volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-c/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-c/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-c/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-c/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-c/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.12 + - acl-net service-d: <<: *fips-common @@ -77,14 +84,13 @@ services: hostname: host-d volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-d/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-d/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-d/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-d/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-d/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.13 + - acl-net service-e: <<: *fips-common @@ -92,12 +98,11 @@ services: hostname: host-e volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-e/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-e/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-e/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-e/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-e/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-e/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.14 + - acl-net service-f: <<: *fips-common @@ -105,9 +110,8 @@ services: hostname: host-f volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-f/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-f/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-f/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-f/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-f/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-f/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.15 + - acl-net diff --git a/testing/acl-allowlist/generate-configs.sh b/testing/acl-allowlist/generate-configs.sh index 01ced41..302b127 100755 --- a/testing/acl-allowlist/generate-configs.sh +++ b/testing/acl-allowlist/generate-configs.sh @@ -3,7 +3,12 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -GENERATED_DIR="$SCRIPT_DIR/generated-configs" + +# Scoped by the per-run suffix because this directory is wiped and rewritten +# below: two runs sharing one output directory would delete each other's +# fixtures out from under running containers. Unset (a bare hand run, or the +# GitHub-hosted path) it collapses to the historical "generated-configs". +GENERATED_DIR="$SCRIPT_DIR/generated-configs${FIPS_CI_NAME_SUFFIX:-}" write_file() { local path="$1" @@ -23,6 +28,10 @@ node-f npub1ytrut7gjncn2zfnhn56c0zgftf0w6p99gf6fu8j73hzw5603zglqc9av6c EOF } +# Peers are addressed by the docker hostname the compose file assigns +# (host-a … host-f), not by IP. The network requests no subnet so that two +# concurrent runs cannot collide on one address range, which means no node's +# address is knowable before `docker compose up`. echo "Generating ACL allowlist fixtures..." rm -rf "$GENERATED_DIR" @@ -48,31 +57,31 @@ peers: alias: "node-b" addresses: - transport: udp - addr: "172.31.0.11:2121" + addr: "host-b:2121" connect_policy: auto_connect - npub: "npub1cld9yay0u24davpu6c35l4vldrhzvaq66pcqtg9a0j2cnjrn9rtsxx2pe6" alias: "node-c" addresses: - transport: udp - addr: "172.31.0.12:2121" + addr: "host-c:2121" connect_policy: auto_connect - npub: "npub1n9lpnv0592cc2ps6nm0ca3qls642vx7yjsv35rkxqzj2vgds52sqgpverl" alias: "node-d" addresses: - transport: udp - addr: "172.31.0.13:2121" + addr: "host-d:2121" connect_policy: auto_connect - npub: "npub1x5z9rwzzm26q9verutx4aajhf2zw2pyp34c6whhde2zduxqav40qgq36l6" alias: "node-e" addresses: - transport: udp - addr: "172.31.0.14:2121" + addr: "host-e:2121" connect_policy: auto_connect - npub: "npub1ytrut7gjncn2zfnhn56c0zgftf0w6p99gf6fu8j73hzw5603zglqc9av6c" alias: "node-f" addresses: - transport: udp - addr: "172.31.0.15:2121" + addr: "host-f:2121" connect_policy: auto_connect EOF @@ -113,19 +122,19 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect - npub: "npub1cld9yay0u24davpu6c35l4vldrhzvaq66pcqtg9a0j2cnjrn9rtsxx2pe6" alias: "node-c" addresses: - transport: udp - addr: "172.31.0.12:2121" + addr: "host-c:2121" connect_policy: auto_connect - npub: "npub1n9lpnv0592cc2ps6nm0ca3qls642vx7yjsv35rkxqzj2vgds52sqgpverl" alias: "node-d" addresses: - transport: udp - addr: "172.31.0.13:2121" + addr: "host-d:2121" connect_policy: auto_connect EOF @@ -166,7 +175,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF @@ -209,7 +218,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF @@ -252,7 +261,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF @@ -282,7 +291,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 145e1b6..8cef658 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -412,15 +412,20 @@ ci_teardown() { --images "$CI_IMAGE_TEST $CI_IMAGE_APP" \ --veth-suffixes "${_suffixes[*]}" >/dev/null || true - # 3. The static suite's generated configs are per-run (a shared directory - # would let concurrent runs overwrite each other's node configs), so - # they are this run's to remove. Only on a green run: after a failure - # they are the evidence of what the failing nodes were actually + # 3. The static and firewall suites' generated configs are per-run (a + # shared directory would let concurrent runs overwrite each other's node + # configs), so they are this run's to remove. Only on a green run: after + # a failure they are the evidence of what the failing nodes were actually # configured with. Guarded on a non-empty suffix too, since without one # the path is the unscoped working directory a developer uses by hand, # which is not ours to delete. + # + # Every suite whose configs become per-run owes a line here. acl-allowlist + # also generates per-run now but is not in this runner's suite list, so + # this teardown never creates its directory and must not remove one. if [[ $run_status -eq 0 && -n "${CI_RUN_NAME_SUFFIX:-}" ]]; then rm -rf "$SCRIPT_DIR/static/generated-configs${CI_RUN_NAME_SUFFIX}" + rm -rf "$SCRIPT_DIR/firewall/generated-configs${CI_RUN_NAME_SUFFIX}" fi } diff --git a/testing/firewall/.gitignore b/testing/firewall/.gitignore index 31315f6..b06c7df 100644 --- a/testing/firewall/.gitignore +++ b/testing/firewall/.gitignore @@ -1 +1 @@ -generated-configs +generated-configs* diff --git a/testing/firewall/README.md b/testing/firewall/README.md index cecd186..3a6bd99 100644 --- a/testing/firewall/README.md +++ b/testing/firewall/README.md @@ -24,10 +24,18 @@ actively DROP'd by the fips chain (not silently unrouted). Two FIPS nodes peered over UDP on a Docker bridge network: -| Container | Hostname | docker IPv4 | Firewall | -|-------------------------|----------|---------------|----------| -| `fips-fw-container-a` | `host-a` | 172.32.0.10 | none (probe) | -| `fips-fw-container-b` | `host-b` | 172.32.0.11 | `fips.nft` + drop-in | +| Container | Hostname | Firewall | +|-------------------------|----------|----------| +| `fips-fw-container-a` | `host-a` | none (probe) | +| `fips-fw-container-b` | `host-b` | `fips.nft` + drop-in | + +The bridge network requests no subnet, so docker assigns one from its own +address pool and two concurrent runs never contend for a fixed range. No +node's IPv4 address is therefore known before startup, and the generated peer +stanzas address each other by docker hostname, resolved through the +container's dnsmasq to docker's embedded DNS. The firewall assertions +themselves are unaffected: they run over the fips0 overlay, whose addresses +are derived from the node npubs. `node-b` mounts the production `packaging/common/fips.nft` read-only at `/etc/fips/fips.nft`, plus a drop-in at `/etc/fips/fips.d/services.nft` @@ -108,4 +116,6 @@ docker compose -f testing/firewall/docker-compose.yml down ## Generated fixture location -`testing/firewall/generated-configs/` (gitignored). +`testing/firewall/generated-configs/` (gitignored), or +`generated-configs/` when `FIPS_CI_NAME_SUFFIX` is set, which is how +concurrent runs keep their fixtures apart. diff --git a/testing/firewall/docker-compose.yml b/testing/firewall/docker-compose.yml index 60b16ea..88110dd 100644 --- a/testing/firewall/docker-compose.yml +++ b/testing/firewall/docker-compose.yml @@ -1,11 +1,21 @@ networks: + # No subnet is requested: docker assigns one from the daemon's address pool. + # A fixed request is honoured verbatim, so two runs asking for the same range + # collide on "Pool overlaps" — which is why the generated peer addresses are + # docker hostnames (host-a, host-b) rather than literal IPs. Docker's embedded + # DNS is per-network, so the same hostname in two concurrent runs resolves + # inside each run's own subnet. + # + # That removes one of the two obstacles to running this suite twice at once, + # not both. The compose project name is still fixed, so two runs that do not + # set COMPOSE_PROJECT_NAME share a project, and the second `up` recreates the + # first's containers while either `down` removes both. The local CI runner + # scopes it externally (run_firewall in ci-local.sh); a bare hand run does + # not. Do not read the floating subnet as making concurrent bare runs safe. fw-net: driver: bridge labels: - "com.corganlabs.fips-ci=1" - ipam: - config: - - subnet: 172.32.0.0/24 x-fips-common: &fips-common build: @@ -31,12 +41,11 @@ services: hostname: host-a volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-a/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-a/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-a/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.key:/etc/fips/fips.key:ro networks: - fw-net: - ipv4_address: 172.32.0.10 + - fw-net service-b: <<: *fips-common @@ -44,11 +53,10 @@ services: hostname: host-b volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-b/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-b/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-b/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.key:/etc/fips/fips.key:ro - ../../packaging/common/fips.nft:/etc/fips/fips.nft:ro - - ./generated-configs/node-b/fips.d:/etc/fips/fips.d:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.d:/etc/fips/fips.d:ro networks: - fw-net: - ipv4_address: 172.32.0.11 + - fw-net diff --git a/testing/firewall/generate-configs.sh b/testing/firewall/generate-configs.sh index 1fca1de..8ecd328 100755 --- a/testing/firewall/generate-configs.sh +++ b/testing/firewall/generate-configs.sh @@ -9,7 +9,12 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -GENERATED_DIR="$SCRIPT_DIR/generated-configs" + +# Scoped by the per-run suffix because this directory is wiped and rewritten +# below: two runs sharing one output directory would delete each other's +# fixtures out from under running containers. Unset (a bare hand run, or the +# GitHub-hosted path) it collapses to the historical "generated-configs". +GENERATED_DIR="$SCRIPT_DIR/generated-configs${FIPS_CI_NAME_SUFFIX:-}" # Deterministic test identities (mirrors the acl-allowlist style). NPUB_A="npub1sjlh2c3x9w7kjsqg2ay080n2lff2uvt325vpan33ke34rn8l5jcqawh57m" @@ -32,6 +37,10 @@ node-b $NPUB_B EOF } +# Peers are addressed by the docker hostname the compose file assigns (host-a, +# host-b), not by IP. The network requests no subnet so that two concurrent +# runs cannot collide on one address range, which means neither node's address +# is knowable before `docker compose up`. echo "Generating firewall fixtures..." rm -rf "$GENERATED_DIR" @@ -58,7 +67,7 @@ peers: alias: "node-b" addresses: - transport: udp - addr: "172.32.0.11:2121" + addr: "host-b:2121" connect_policy: auto_connect EOF @@ -89,7 +98,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.32.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF diff --git a/testing/firewall/test.sh b/testing/firewall/test.sh index 4d942e7..833e9c8 100755 --- a/testing/firewall/test.sh +++ b/testing/firewall/test.sh @@ -81,21 +81,46 @@ wait_for_fips0() { fail "$container fips0 did not come up within ${timeout}s" } +# Connected-peer count for a container, or the empty string if it did not +# answer. +# +# Empty is deliberately distinct from a real 0. An `|| echo 0` fallback here +# would make "the container is unreachable, or its daemon never came up" and +# "the daemon answered, and the answer was zero" the same value, so any caller +# expecting zero would be satisfied on the first iteration without the +# property it is checking ever being observed. No caller in this file expects +# zero today, which is exactly why the fallback has to go now rather than when +# one is added. Same shape as the acl-allowlist suite's read_connected_peers. +read_connected_peers() { + local container="$1" + docker exec "$container" fipsctl show peers 2>/dev/null \ + | python3 -c 'import json,sys; data=json.load(sys.stdin); print(sum(1 for p in data.get("peers", []) if p.get("connectivity") == "connected"))' 2>/dev/null \ + || true +} + # Wait for the peer count on a container to reach the expected value. wait_for_peers_exact() { local container="$1" local expected_count="$2" local timeout="${3:-30}" + + local count="" answered=false for _ in $(seq 1 "$timeout"); do - local count - count=$(docker exec "$container" fipsctl show peers 2>/dev/null \ - | python3 -c 'import json,sys; data=json.load(sys.stdin); print(sum(1 for p in data.get("peers", []) if p.get("connectivity") == "connected"))' 2>/dev/null || echo 0) - if [ "$count" -eq "$expected_count" ]; then - return 0 + count=$(read_connected_peers "$container") + if [ -n "$count" ]; then + answered=true + if [ "$count" -eq "$expected_count" ]; then + return 0 + fi fi sleep 1 done - fail "$container did not reach $expected_count connected peers in ${timeout}s" + + if [ "$answered" = false ]; then + fail "$container never answered a peer query in ${timeout}s, so a count of $expected_count was never actually observed" + fi + docker exec "$container" fipsctl show peers >&2 || true + fail "$container did not reach $expected_count connected peers in ${timeout}s (last answer: $count)" } # Resolve `.fips` inside a container and print the AAAA answer. @@ -230,14 +255,20 @@ log "Case (b): node-b initiates outbound TCP, expects reply via conntrack" # node-b → node-a:8000 on the fips overlay. node-a has http.server on # [::]:8000 and is NOT firewalled, so this is purely a test of node-b's # outbound + ct state established,related path on the way back. +# +# mktemp rather than a fixed /tmp name: two concurrent runs of this suite +# would otherwise share one host file, and either one's `rm` between the +# other's write and read leaves an empty read that fails the http_code check +# for a reason that has nothing to do with the firewall. +CURL_OUT="$(mktemp)" set +e docker exec "$CONTAINER_B" curl -6 --silent --max-time 5 \ --output /dev/null --write-out '%{http_code}' \ - "http://[${ADDR_A}]:${OUTBOUND_TARGET_PORT}/" >/tmp/fw_b_rc 2>/dev/null + "http://[${ADDR_A}]:${OUTBOUND_TARGET_PORT}/" >"$CURL_OUT" 2>/dev/null RC=$? set -e -HTTP_CODE="$(cat /tmp/fw_b_rc 2>/dev/null || true)" -rm -f /tmp/fw_b_rc +HTTP_CODE="$(cat "$CURL_OUT" 2>/dev/null || true)" +rm -f "$CURL_OUT" if [ "$RC" -ne 0 ]; then fail "(b) outbound from node-b failed (curl rc=$RC, http=$HTTP_CODE) — conntrack reply path broken" fi diff --git a/testing/lib/wait-converge-test.sh b/testing/lib/wait-converge-test.sh index fda25c4..7b7c0ab 100755 --- a/testing/lib/wait-converge-test.sh +++ b/testing/lib/wait-converge-test.sh @@ -180,6 +180,47 @@ check "case4: returns 0 with 4 args" "$c4_rc_ok" "rc=$rc" c4_hold_ok=1; echo "$out" | grep -q "$HOLD_MSG" && c4_hold_ok=0 check "case4: default slack triggered near-converged hold" "$c4_hold_ok" +# --- Case 5: wait_for_peers refuses a floor of zero ------------------- +# +# The reader inside wait_for_peers falls back to 0 when a container does +# not answer. That is safe against a floor of 1 or more, where 0 reads as +# "not converged yet", and unsafe against a floor of 0, where the first +# read from a dead container satisfies the wait immediately. This case is +# the break-what-it-guards check for the rejection: drive the guard with a +# floor of 0 and confirm it refuses, then drive the same dead container +# with a floor of 1 and confirm the refusal is specific to the dangerous +# input rather than a blanket failure. +# +# `docker` is stubbed to fail, which is what an unreachable container looks +# like to this reader, so no container or network is involved and the suite +# stays hermetic. +echo +echo "== Case 5: wait_for_peers refuses a zero floor ==" +docker() { return 1; } + +out=$(wait_for_peers stub-container 0 2 2>&1); rc=$? +echo "$out" +c5_reject_ok=1; [ "$rc" -eq 2 ] && c5_reject_ok=0 +check "case5: floor of 0 is refused" "$c5_reject_ok" "rc=$rc" +c5_msg_ok=1; echo "$out" | grep -q "refusing a minimum" && c5_msg_ok=0 +check "case5: refusal names the reason" "$c5_msg_ok" +# The pre-guard behaviour, asserted so a regression is visible rather than +# quiet: without the rejection this returned 0 on its first iteration +# against a container that never answered. +c5_notpass_ok=1; [ "$rc" -ne 0 ] && c5_notpass_ok=0 +check "case5: floor of 0 does not report success" "$c5_notpass_ok" "rc=$rc" + +start=$SECONDS +out=$(wait_for_peers stub-container 1 2 2>&1); rc=$? +elapsed=$((SECONDS - start)) +echo "$out" +c5_floor1_ok=1; [ "$rc" -eq 1 ] && c5_floor1_ok=0 +check "case5: floor of 1 times out rather than being refused" "$c5_floor1_ok" "rc=$rc" +c5_polled_ok=1; [ "$elapsed" -ge 2 ] && c5_polled_ok=0 +check "case5: floor of 1 polled its full budget" "$c5_polled_ok" "elapsed=${elapsed}s >= 2s" + +unset -f docker + # --- Summary ---------------------------------------------------------- echo echo "==============================================" diff --git a/testing/lib/wait-converge.sh b/testing/lib/wait-converge.sh index 8832b67..17933a5 100644 --- a/testing/lib/wait-converge.sh +++ b/testing/lib/wait-converge.sh @@ -1,44 +1,47 @@ #!/bin/bash # Shared convergence wait helpers for FIPS integration tests. # -# Source this file to get wait_for_links(), wait_for_peers(), and -# wait_until_connected(). +# Source this file to get wait_for_peers() and wait_until_connected(). # # Usage: # source "$(dirname "$0")/../../lib/wait-converge.sh" -# wait_for_links [timeout_secs] # wait_for_peers [timeout_secs] # wait_until_connected [poll_secs] \ # [near_converged_slack] - -# Wait until a container has at least min_links active links. -# Returns 0 on success, 1 on timeout. -wait_for_links() { - local container="$1" - local min_links="$2" - local timeout="${3:-30}" - - for i in $(seq 1 "$timeout"); do - local count - count=$(docker exec "$container" fipsctl show links 2>/dev/null \ - | python3 -c "import sys,json; print(len(json.load(sys.stdin).get('links',[])))" 2>/dev/null || echo 0) - if [ "$count" -ge "$min_links" ]; then - echo " $container: $count link(s) after ${i}s" - return 0 - fi - sleep 1 - done - echo " $container: TIMEOUT waiting for $min_links link(s) after ${timeout}s" - return 1 -} +# +# There was a wait_for_links() here. It was removed rather than kept for +# symmetry: it had no caller anywhere in the tree on any branch, and its +# reader carried the same failure-to-zero fallback wait_for_peers does. An +# uncalled helper cannot be wrong today, so the risk was that the first +# caller to appear would inherit the hazard below without the reasoning +# that goes with it. `git log` has the implementation if one is needed. # Wait until a container has at least min_peers connected peers. # Returns 0 on success, 1 on timeout. +# +# The read below falls back to 0 when the container does not answer, which +# is safe ONLY because this is a floor: a fallback of 0 reads as "not +# converged yet", the loop keeps polling, and a container that never answers +# times out and returns 1. That safety is a property of the comparison, not +# of the reader. +# +# A minimum of 0 inverts it. `[ 0 -ge 0 ]` is true, so the first read from a +# dead container would satisfy the wait immediately and the caller would +# proceed as though convergence had been observed. No caller passes 0, and +# rejecting it here means none can start to — which is cheaper than auditing +# every future caller, and is why this is a hard error rather than a warning. +# A caller that genuinely wants to assert "exactly zero peers" needs a reader +# that distinguishes no-answer from zero, not this floor. wait_for_peers() { local container="$1" local min_peers="$2" local timeout="${3:-30}" + if [ "$min_peers" -lt 1 ]; then + echo " wait_for_peers: refusing a minimum of $min_peers for $container — a floor of 0 is satisfied by a container that never answered" >&2 + return 2 + fi + for i in $(seq 1 "$timeout"); do local count count=$(docker exec "$container" fipsctl show peers 2>/dev/null \