From 0d286b7cc81179db721e62e7def9751e3a2506ed Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 1 Sep 2026 21:54:59 +0200 Subject: [PATCH] What review found in the lab, fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A segment named "uplink" is refused. The lab claims that name for the NAT bridge behind `egress: true`, and a scenario wearing it first would have its egress machines silently attached to an isolated bridge — a declared key doing nothing, which is the fault this repo exists to refuse, in the repo that refuses it. settled() parses inside the try. A truncated status from a struggling machine was the one shape of bad answer that still threw out of the wait, and the likeliest moment for one is exactly the machine the poll is watching. Malformed now counts as "could not ask", like the exec that times out. And a sentence on the uplink's UseDNS saying its inertness is load-bearing: it matters only where systemd-resolved runs, and on a machine whose modules own resolv.conf the uplink must not outvote the resolver a scenario is testing. --- src/declaration/validate.ts | 10 ++++++++++ src/lifecycle/address.ts | 3 +++ test/integration/mesh.test.ts | 24 +++++++++++++++--------- test/validate.test.ts | 7 +++++++ 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/declaration/validate.ts b/src/declaration/validate.ts index 8f772e0..ee6f2da 100644 --- a/src/declaration/validate.ts +++ b/src/declaration/validate.ts @@ -199,6 +199,16 @@ export function validate(scenario: Scenario): void { } } + // "uplink" is the one network name the lab itself claims, for the NAT bridge behind + // `egress: true`. A segment wearing it would be created first, as an isolated bridge — and the + // uplink code, finding a network by that name, would attach egress machines to it. No error + // anywhere, no route anywhere: a scenario key silently ignored, which is the fault this file + // exists to refuse. + if (scenario.segments["uplink"]) { + problems.push(`segment 'uplink': the name is reserved for the lab's own NAT bridge — ` + + `an egress machine would be silently attached to this segment instead of the world`); + } + for (const [name, machine] of Object.entries(scenario.machines)) { if (machine.at === "detached") { if (machine.published?.length) { diff --git a/src/lifecycle/address.ts b/src/lifecycle/address.ts index 8af4143..6d6ed48 100644 --- a/src/lifecycle/address.ts +++ b/src/lifecycle/address.ts @@ -50,6 +50,9 @@ function networkUnit(wire: Wire): string { "IPv6AcceptRA=no", "", "[DHCPv4]", + // UseDNS matters only where systemd-resolved runs; on a machine whose mesh modules own + // /etc/resolv.conf it is inert, and that inertness is load-bearing — the uplink must not + // outvote the resolver a scenario is testing. // **Worse than any route the scenario states.** A machine behind a declared gateway must // keep using it: the uplink is a way out of the scenario, not a better way around inside // it. On-link segments win regardless, being connected routes; this only settles which diff --git a/test/integration/mesh.test.ts b/test/integration/mesh.test.ts index 8aff77c..6b91734 100644 --- a/test/integration/mesh.test.ts +++ b/test/integration/mesh.test.ts @@ -131,23 +131,29 @@ async function settled(node: string, withinMs = 240_000): Promise { // And a poll that *threw* — an exec timeout, a lost fifo — is also "could not ask", not a // verdict. The distinction failed once as an IncusError surfacing at minute four of a wait // whose machine was merely slow. - let said = "", ok = false; + let state: { + wrong: { node: string; outcome: string; refused?: string; + failed?: { id: string; error: string }[] }[]; + waiting: { node: string; never: boolean }[]; + } | undefined; + let said = ""; try { - ({ out: said, ok } = await on("anchor", - `docker exec mesh-control /mesh-control status --json`)); + const asked = await on("anchor", + `docker exec mesh-control /mesh-control status --json`); + said = asked.out; + // Parsed inside the try on purpose: a truncated answer from a struggling machine is the + // same fact as no answer, and the likeliest moment for one is exactly the machine this + // poll is watching. + if (asked.ok) state = JSON.parse(said); } catch (err) { said = (err as Error).message; } - if (!ok) { + if (!state) { last = said; await new Promise((r) => setTimeout(r, 5000)); continue; } - const state = JSON.parse(said) as { - wrong: { node: string; outcome: string; refused?: string; - failed?: { id: string; error: string }[] }[]; - waiting: { node: string; never: boolean }[]; - }; + const bad = state.wrong.find((w) => w.node === node); if (bad) { const why = [bad.refused, ...(bad.failed ?? []).map((f) => `${f.id}: ${f.error}`)] diff --git a/test/validate.test.ts b/test/validate.test.ts index b928d07..e07a309 100644 --- a/test/validate.test.ts +++ b/test/validate.test.ts @@ -250,3 +250,10 @@ segments: { net: { kind: public, cidr: [192.0.2.0/24] } } machines: { a: { at: detached, egress: true } }`, /detached but declares egress/); }); + +test("a segment may not be named 'uplink' — the lab claims that name for egress", () => { + refuses(`scenario: x +segments: { uplink: { kind: public, cidr: [192.0.2.0/24] } } +machines: { a: { at: { segment: uplink, address: [192.0.2.1] }, egress: true } }`, + /reserved for the lab's own NAT bridge/); +});