From 715f3671470fd648fa8d5440636d6ef24f2b813d Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 25 Aug 2026 00:21:09 +0200 Subject: [PATCH] Step 1: an invariant that holds of any raised scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The address collision was found by eye. This is the mechanical form of it: no two machines hold one address on one segment. Pure over already-collected facts, so the logic is tested without a hypervisor — including the cases that would make it useless if got wrong: the same address on DIFFERENT segments is normal and must not be reported, and one machine holding an address twice is not two machines. Asserted against whatever the integration suite has standing, read from the hypervisor rather than from the declaration. The declaration is what was accepted, and it was accepted. --- src/lifecycle/invariants.ts | 67 +++++++++++++++++++++++++++++++ test/integration/underlay.test.ts | 20 +++++++++ test/invariants.test.ts | 60 +++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 src/lifecycle/invariants.ts create mode 100644 test/invariants.test.ts diff --git a/src/lifecycle/invariants.ts b/src/lifecycle/invariants.ts new file mode 100644 index 0000000..059e79b --- /dev/null +++ b/src/lifecycle/invariants.ts @@ -0,0 +1,67 @@ +/** + * Properties that must hold of ANY raised scenario, whatever it declares. + * + * Distinct from validation, which reads a file and can only catch what the file says. These + * read what actually came up. The first one exists because two routers were raised holding + * one address on one segment: the declaration was accepted, the raise reported success, and + * the address resolved to whichever container answered ARP last — so a published port + * worked or did not, run to run, with nothing reporting a fault. + * + * Pure over already-collected facts, so the logic is testable without a hypervisor and the + * reading of the hypervisor stays in one place. + */ + +/** One address, held by one machine, on one segment. */ +export interface Held { + machine: string; + segment: string; + address: string; +} + +export interface Conflict { + segment: string; + address: string; + machines: string[]; +} + +/** Strip a prefix length: what is held is an address, the mask is a property of the link. */ +function bare(address: string): string { + const slash = address.lastIndexOf("/"); + return slash === -1 ? address : address.slice(0, slash); +} + +/** + * Two machines holding one address on one segment. + * + * The same address on DIFFERENT segments is not a conflict — `192.168.1.1` on one private + * network and on another are two different machines' idea of "the gateway", which is the + * normal case and must not be reported. + */ +export function duplicateAddresses(held: Held[]): Conflict[] { + const byPlace = new Map>(); + for (const entry of held) { + const key = `${entry.segment} ${bare(entry.address)}`; + const machines = byPlace.get(key) ?? new Set(); + machines.add(entry.machine); + byPlace.set(key, machines); + } + + const conflicts: Conflict[] = []; + for (const [key, machines] of byPlace) { + if (machines.size < 2) continue; + const [segment, address] = key.split(" "); + conflicts.push({ + segment: segment as string, + address: address as string, + machines: [...machines].sort(), + }); + } + return conflicts.sort((a, b) => a.address.localeCompare(b.address)); +} + +/** Render conflicts as something a failing test can print without further work. */ +export function describeConflicts(conflicts: Conflict[]): string { + return conflicts + .map((c) => `${c.address} is held by ${c.machines.join(" and ")} on '${c.segment}'`) + .join("; "); +} diff --git a/test/integration/underlay.test.ts b/test/integration/underlay.test.ts index 05f19e9..d9960e1 100644 --- a/test/integration/underlay.test.ts +++ b/test/integration/underlay.test.ts @@ -13,6 +13,7 @@ import { labIsUsable, destroyAll } from "./harness.ts"; import { diagramFromLive } from "../../src/diagram/from-live.ts"; import { diagramFromDeclaration } from "../../src/diagram/from-declaration.ts"; import { toDrawio } from "../../src/diagram/drawio.ts"; +import { duplicateAddresses, describeConflicts } from "../../src/lifecycle/invariants.ts"; const capability = await labIsUsable(); const skip = capability.usable ? false : `lab not usable: ${capability.why}`; @@ -121,6 +122,25 @@ test("ADR 0032 — the workstation has no route into the scenario", { skip, time assert.equal(stdout.trim(), "inside", "exec is the only way in, and it works"); }); +test("no two machines hold one address on one segment", { skip }, async () => { + // True of ANY raised scenario, so it is asserted against whatever is standing rather than + // against something this test declares. Two gateways with the same public address became + // two containers both holding it, and the address resolved to whichever answered ARP last. + // + // Read from the hypervisor, never from the declaration — the declaration is what was + // accepted, and it was accepted. + const drawn = await diagramFromLive(instanceId); + const held = drawn.machines.flatMap((machine) => + machine.attachments.flatMap((attachment) => + attachment.addresses.map((address) => ({ machine: machine.name, segment: attachment.segment, address })), + ), + ); + assert.ok(held.length > 0, "no addresses were read back at all"); + + const conflicts = duplicateAddresses(held); + assert.deepEqual(conflicts, [], `address conflict: ${describeConflicts(conflicts)}`); +}); + // The diagram tests read the instance the file raised, so they run before the one that // tears it down. Ordering is load-bearing here: appended after the destroy test they read // an instance that no longer existed, and reported it as the diagram failing. diff --git a/test/invariants.test.ts b/test/invariants.test.ts new file mode 100644 index 0000000..ce274ba --- /dev/null +++ b/test/invariants.test.ts @@ -0,0 +1,60 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { duplicateAddresses, describeConflicts } from "../src/lifecycle/invariants.ts"; + +/** + * The fault this defends against, in the shape it actually occurred: two gateways declared + * with the same public address became two router containers, both holding it on one segment. + */ + +test("two machines holding one address on one segment is a conflict", () => { + const conflicts = duplicateAddresses([ + { machine: "gw-home", segment: "isp-home", address: "198.51.100.7/24" }, + { machine: "gw-devices", segment: "isp-home", address: "198.51.100.7/24" }, + { machine: "transit", segment: "isp-home", address: "198.51.100.254/24" }, + ]); + assert.equal(conflicts.length, 1); + assert.equal(conflicts[0]?.address, "198.51.100.7"); + assert.deepEqual(conflicts[0]?.machines, ["gw-devices", "gw-home"]); + assert.match(describeConflicts(conflicts), /198\.51\.100\.7 is held by gw-devices and gw-home/); +}); + +test("the same address on different segments is NOT a conflict", () => { + // Every private network has its own `.1`. Reporting that would make the check useless. + assert.deepEqual( + duplicateAddresses([ + { machine: "gw-a", segment: "home", address: "192.168.1.1/24" }, + { machine: "gw-b", segment: "cafe", address: "192.168.1.1/24" }, + ]), + [], + ); +}); + +test("one machine holding an address twice is not two machines", () => { + // A machine multi-homed onto the same segment, or an address read back from two places. + assert.deepEqual( + duplicateAddresses([ + { machine: "gw", segment: "isp", address: "198.51.100.7/24" }, + { machine: "gw", segment: "isp", address: "198.51.100.7" }, + ]), + [], + ); +}); + +test("the prefix length is not part of the address", () => { + // The same address declared /24 in one place and /16 in another is still one address. + const conflicts = duplicateAddresses([ + { machine: "a", segment: "isp", address: "198.51.100.7/24" }, + { machine: "b", segment: "isp", address: "198.51.100.7/16" }, + ]); + assert.equal(conflicts.length, 1); +}); + +test("both families are checked", () => { + const conflicts = duplicateAddresses([ + { machine: "a", segment: "isp", address: "2001:db8:b::7/48" }, + { machine: "b", segment: "isp", address: "2001:db8:b::7/48" }, + ]); + assert.equal(conflicts.length, 1); + assert.equal(conflicts[0]?.address, "2001:db8:b::7"); +});