diff --git a/.github/workflows/runtime-proof.yml b/.github/workflows/runtime-proof.yml index bd861a1..d78eae0 100644 --- a/.github/workflows/runtime-proof.yml +++ b/.github/workflows/runtime-proof.yml @@ -222,6 +222,78 @@ jobs: # images are missing, so deleting this step fails at the doorstep with the # command to run, instead of ninety seconds later on an ssh timeout that # blames the network. + # The build container must reach a package repository before anything + # spends a minute finding out that it cannot (#583). + # + # On the night of 2026-08-28 both legs died here, on every image: + # + # apk add --no-cache openssh in the build instance: + # fetching https://dl-cdn.alpinelinux.org/alpine/v3.21/main: Permission denied + # + # `Permission denied` on a connect() is EACCES — what an nftables or + # iptables REJECT answers, not what a missing route (Network unreachable) + # or a CDN refusal (403) answers. Nothing on this side moved: the action is + # pinned by SHA, egress-policy is `audit`, and the Alpine CDN answered 200 + # in 0.121 s from the maintainer's station the same morning. What moved is + # the runner image, from 20260729.566 on the last night that cleared this + # step to 20260819.586 on the night that did not. + # + # Both Docker and Incus manage that filter on a hosted runner: Docker sets + # a FORWARD DROP policy, Incus adds accept rules for its own bridge. An + # image that changes either one's version, or the order they are applied, + # takes egress away from containers in exactly this shape. + # + # So this step measures first and repairs second, and then **proves the + # repair before the build depends on it**: a throwaway container fetches + # one real URL. A prerequisite asserted rather than demonstrated is the + # defect this repository spends its time removing, and an image build that + # discovers a dead network a minute in reports the wrong thing — the night + # of the 28th surfaced `the emulator did not answer /_feint/conformance`, + # which was true and thirty milliseconds too late to be useful. + - name: The build container reaches the network, or this step says why + run: | + set -euo pipefail + + echo "== what the filter looks like before anything" + sudo iptables -S FORWARD 2>&1 | head -20 || true + sudo iptables -t nat -S POSTROUTING 2>&1 | head -10 || true + docker --version 2>&1 || true + sudo incus --version 2>&1 || true + + bridge="$(sudo incus network list --format csv \ + | awk -F, '$2=="bridge" && $1 ~ /^incusbr/ {print $1; exit}')" + echo "== bridge: ${bridge:-none found}" + + if [ -n "${bridge}" ]; then + # Insert rather than append: Docker's DROP policy and its own chains + # are already there, and a rule appended after a DROP never runs. + sudo iptables -I FORWARD -i "${bridge}" -j ACCEPT + sudo iptables -I FORWARD -o "${bridge}" -j ACCEPT + echo "== accept rules inserted for ${bridge}" + fi + + # The positive control. A throwaway container, one real fetch, and a + # refusal that names what it could not reach — never a silent pass. + probe=rp-egress-probe + sudo incus delete --force "${probe}" >/dev/null 2>&1 || true + sudo incus launch images:alpine/3.21/cloud "${probe}" >/dev/null + trap 'sudo incus delete --force "${probe}" >/dev/null 2>&1 || true' EXIT + for _ in $(seq 1 20); do + if sudo incus exec "${probe}" -- ip -4 -o addr show scope global \ + | grep -q .; then break; fi + sleep 1 + done + echo "== the probe carries: $(sudo incus exec "${probe}" -- ip -4 -o addr show scope global | awk '{print $4}' | tr '\n' ' ')" + if ! sudo incus exec "${probe}" -- wget -q -O /dev/null -T 20 \ + https://dl-cdn.alpinelinux.org/alpine/v3.21/main/x86_64/APKINDEX.tar.gz; then + echo "FAIL: a container on ${bridge:-no bridge} cannot reach the Alpine CDN," >&2 + echo " so every image build below would fail on apk with the runner's" >&2 + echo " filter, not with anything this repository controls (#583)." >&2 + sudo iptables -S FORWARD 2>&1 | head -20 >&2 || true + exit 1 + fi + echo "== a container reaches the Alpine CDN" + - name: Build the machine images the suites boot env: MODE: ${{ matrix.mode }} @@ -553,6 +625,78 @@ jobs: # The gate refuses without them rather than booting an upstream image with # no ssh daemon, so this is the half that makes the refusal never fire. + # The build container must reach a package repository before anything + # spends a minute finding out that it cannot (#583). + # + # On the night of 2026-08-28 both legs died here, on every image: + # + # apk add --no-cache openssh in the build instance: + # fetching https://dl-cdn.alpinelinux.org/alpine/v3.21/main: Permission denied + # + # `Permission denied` on a connect() is EACCES — what an nftables or + # iptables REJECT answers, not what a missing route (Network unreachable) + # or a CDN refusal (403) answers. Nothing on this side moved: the action is + # pinned by SHA, egress-policy is `audit`, and the Alpine CDN answered 200 + # in 0.121 s from the maintainer's station the same morning. What moved is + # the runner image, from 20260729.566 on the last night that cleared this + # step to 20260819.586 on the night that did not. + # + # Both Docker and Incus manage that filter on a hosted runner: Docker sets + # a FORWARD DROP policy, Incus adds accept rules for its own bridge. An + # image that changes either one's version, or the order they are applied, + # takes egress away from containers in exactly this shape. + # + # So this step measures first and repairs second, and then **proves the + # repair before the build depends on it**: a throwaway container fetches + # one real URL. A prerequisite asserted rather than demonstrated is the + # defect this repository spends its time removing, and an image build that + # discovers a dead network a minute in reports the wrong thing — the night + # of the 28th surfaced `the emulator did not answer /_feint/conformance`, + # which was true and thirty milliseconds too late to be useful. + - name: The build container reaches the network, or this step says why + run: | + set -euo pipefail + + echo "== what the filter looks like before anything" + sudo iptables -S FORWARD 2>&1 | head -20 || true + sudo iptables -t nat -S POSTROUTING 2>&1 | head -10 || true + docker --version 2>&1 || true + sudo incus --version 2>&1 || true + + bridge="$(sudo incus network list --format csv \ + | awk -F, '$2=="bridge" && $1 ~ /^incusbr/ {print $1; exit}')" + echo "== bridge: ${bridge:-none found}" + + if [ -n "${bridge}" ]; then + # Insert rather than append: Docker's DROP policy and its own chains + # are already there, and a rule appended after a DROP never runs. + sudo iptables -I FORWARD -i "${bridge}" -j ACCEPT + sudo iptables -I FORWARD -o "${bridge}" -j ACCEPT + echo "== accept rules inserted for ${bridge}" + fi + + # The positive control. A throwaway container, one real fetch, and a + # refusal that names what it could not reach — never a silent pass. + probe=rp-egress-probe-stacks + sudo incus delete --force "${probe}" >/dev/null 2>&1 || true + sudo incus launch images:alpine/3.21/cloud "${probe}" >/dev/null + trap 'sudo incus delete --force "${probe}" >/dev/null 2>&1 || true' EXIT + for _ in $(seq 1 20); do + if sudo incus exec "${probe}" -- ip -4 -o addr show scope global \ + | grep -q .; then break; fi + sleep 1 + done + echo "== the probe carries: $(sudo incus exec "${probe}" -- ip -4 -o addr show scope global | awk '{print $4}' | tr '\n' ' ')" + if ! sudo incus exec "${probe}" -- wget -q -O /dev/null -T 20 \ + https://dl-cdn.alpinelinux.org/alpine/v3.21/main/x86_64/APKINDEX.tar.gz; then + echo "FAIL: a container on ${bridge:-no bridge} cannot reach the Alpine CDN," >&2 + echo " so every image build below would fail on apk with the runner's" >&2 + echo " filter, not with anything this repository controls (#583)." >&2 + sudo iptables -S FORWARD 2>&1 | head -20 >&2 || true + exit 1 + fi + echo "== a container reaches the Alpine CDN" + - name: Build the machine images the stacks boot run: sudo ./feint images --vm incus-ovn diff --git a/internal/core/machine/images_test.go b/internal/core/machine/images_test.go index da8da48..584356d 100644 --- a/internal/core/machine/images_test.go +++ b/internal/core/machine/images_test.go @@ -2,8 +2,10 @@ package machine import ( "context" + "errors" "strings" "testing" + "time" ) // The image table is data, and a row that names no source or no package is a @@ -387,3 +389,61 @@ func TestLocalImagesSurviveASecondAlias(t *testing.T) { t.Fatalf("the listing did not ask for JSON, so a second alias will truncate: %v", asked) } } + +// The builder is ready when it carries an address, not when its init answers. +// +// `exec -- true` proves an init is up; the very next thing BuildImage does is +// fetch a package over the network. Treating the two as one killed the whole +// nightly runtime proof on 2026-08-28, on both legs, before a suite ran — apk +// on Alpine, whose cloud image carries no cloud-init and which boots in about a +// second, so the fetch beat DHCP to it. AlmaLinux in the same pass, from the +// same bridge, succeeded every time. +// +// The control that makes this test mean something is the second half: a builder +// that answers WITH an address must not be waited on at all, or the test would +// pass over a wait that always blocks. +func TestTheBuilderIsNotDeclaredReadyWithoutAnAddress(t *testing.T) { + addressAsked := 0 + d := NewIncus() + d.runner = func(_ context.Context, args ...string) ([]byte, error) { + joined := strings.Join(args, " ") + switch { + case strings.Contains(joined, "-- true"): + return nil, nil + case strings.Contains(joined, "cloud-init"): + return nil, errors.New("cloud-init: not found") + case strings.Contains(joined, "ip -4 -o addr show scope global"): + addressAsked++ + return nil, nil // up, and holding no address + } + return nil, nil + } + + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + err := d.waitForBuilder(ctx, "feint-build-x") + if err == nil { + t.Fatal("a builder with no global address was declared ready, so the package " + + "fetch that follows would fail on the network and blame the repository") + } + if addressAsked == 0 { + t.Fatal("the address was never asked for: this test would pass over a wait " + + "that returns early for any other reason") + } + + // The control: the same builder, carrying an address, is ready at once. + d2 := NewIncus() + d2.runner = func(_ context.Context, args ...string) ([]byte, error) { + joined := strings.Join(args, " ") + switch { + case strings.Contains(joined, "cloud-init"): + return nil, errors.New("cloud-init: not found") + case strings.Contains(joined, "ip -4 -o addr show scope global"): + return []byte("2: eth0 inet 10.248.68.10/24 scope global eth0\n"), nil + } + return nil, nil + } + if err := d2.waitForBuilder(context.Background(), "feint-build-x"); err != nil { + t.Fatalf("a builder holding 10.248.68.10 was refused: %v", err) + } +} diff --git a/internal/core/machine/incus_imagebuild_test.go b/internal/core/machine/incus_imagebuild_test.go index facc62f..c854ef2 100644 --- a/internal/core/machine/incus_imagebuild_test.go +++ b/internal/core/machine/incus_imagebuild_test.go @@ -56,6 +56,13 @@ func (r *buildRecorder) run(_ context.Context, args ...string) ([]byte, error) { r.mu.Unlock() switch { + // A real builder carries an address, and BuildImage now waits for one + // before it fetches a package (#583). A recorder that answered nothing here + // would make every build in this file wait out its three-minute deadline — + // which is what it did for one run, and is why this case says what it + // stands for rather than returning a bare string. + case len(args) > 3 && args[0] == "exec" && args[3] == "ip": + return []byte("2: eth0 inet 10.248.68.10/24 scope global eth0\n"), nil case len(args) > 1 && args[0] == "image" && args[1] == "list": out := "[" for i, alias := range published { diff --git a/internal/core/machine/incus_images.go b/internal/core/machine/incus_images.go index 303de7d..8af8d14 100644 --- a/internal/core/machine/incus_images.go +++ b/internal/core/machine/incus_images.go @@ -169,7 +169,7 @@ func (d *Incus) waitForBuilder(ctx context.Context, builder string) error { if _, err := d.run(ctx, "exec", builder, "--", "true"); err == nil { // cloud-init is absent on some images and that is not an error. _, _ = d.run(ctx, "exec", builder, "--", "cloud-init", "status", "--wait") - return nil + return d.waitForBuilderAddress(ctx, builder, deadline) } select { case <-ctx.Done(): @@ -180,6 +180,50 @@ func (d *Incus) waitForBuilder(ctx context.Context, builder string) error { return fmt.Errorf("the build instance never answered") } +// waitForBuilderAddress waits for the thing the next command actually needs. +// +// Answering `exec -- true` proves an init is up. It does not prove the instance +// has an address, and the very next thing this build does is fetch a package +// over the network. The two were treated as one, and on 2026-08-28 the whole +// nightly runtime proof died on the difference, on both legs, before a suite +// ran: +// +// apk add --no-cache openssh in the build instance: +// fetching https://dl-cdn.alpinelinux.org/alpine/v3.21/main: Permission denied +// +// Alpine is the one that shows it because Alpine is the one that boots in about +// a second: `exec -- true` answers almost at once, its cloud image carries no +// cloud-init so the wait above is a no-op, and apk runs before DHCP has handed +// out a lease. AlmaLinux, built in the same pass and from the same bridge, +// succeeded every time — systemd and cloud-init take long enough that the lease +// is there by the time dnf runs. What made a five-year-old race visible was a +// runner image that jumped three weeks and changed the timings; the race was +// always there. +// +// So this waits on the observable condition rather than on a proxy for it, +// which is #459's rule applied one layer down: the machine carries a global +// address, asked of the machine itself. A build instance that never gets one is +// refused by name, because a build that proceeds without an address fails later +// and blames the package repository. +// +// TestTheBuilderIsNotDeclaredReadyWithoutAnAddress fails without it. +func (d *Incus) waitForBuilderAddress(ctx context.Context, builder string, deadline time.Time) error { + for time.Now().Before(deadline) { + out, err := d.run(ctx, "exec", builder, "--", + "ip", "-4", "-o", "addr", "show", "scope", "global") + if err == nil && strings.TrimSpace(string(out)) != "" { + return nil + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(time.Second): + } + } + return fmt.Errorf("the build instance never carried a global address, so the " + + "package fetch below would fail on the network rather than on the package") +} + func (d *Incus) execInBuilder(ctx context.Context, builder string, command []string) error { args := append([]string{"exec", builder, "--"}, command...) if _, err := d.run(ctx, args...); err != nil { diff --git a/tools/falsify/specs/the-builder-waits-for-an-address.json b/tools/falsify/specs/the-builder-waits-for-an-address.json new file mode 100644 index 0000000..ff46c05 --- /dev/null +++ b/tools/falsify/specs/the-builder-waits-for-an-address.json @@ -0,0 +1,19 @@ +{ + "package": "./internal/core/machine/", + "mutations": [ + { + "label": "the builder is declared ready as soon as its init answers, so apk runs before DHCP and the package repository takes the blame (#583)", + "file": "internal/core/machine/incus_images.go", + "find": "\t\t\treturn d.waitForBuilderAddress(ctx, builder, deadline)", + "replace": "\t\t\tif false {\n\t\t\t\treturn d.waitForBuilderAddress(ctx, builder, deadline)\n\t\t\t}\n\t\t\treturn nil", + "test": "TestTheBuilderIsNotDeclaredReadyWithoutAnAddress" + }, + { + "label": "an empty address list counts as an address, so a builder carrying nothing is let through (#583)", + "file": "internal/core/machine/incus_images.go", + "find": "\t\tif err == nil && strings.TrimSpace(string(out)) != \"\" {", + "replace": "\t\tif err == nil && (strings.TrimSpace(string(out)) != \"\" || true) {", + "test": "TestTheBuilderIsNotDeclaredReadyWithoutAnAddress" + } + ] +}