diff --git a/mender-auto-accept/node-id-test-vectors.json b/mender-auto-accept/node-id-test-vectors.json new file mode 100644 index 0000000..fda2613 --- /dev/null +++ b/mender-auto-accept/node-id-test-vectors.json @@ -0,0 +1,91 @@ +{ + "_comment": [ + "Canonical test vectors for the node_id derivation. Vendored into every", + "repo that validates or derives a node_id, so the four implementations", + "are checked against one artefact rather than against each other.", + "", + "Generated by owl-os, which owns configuration/mender/identity/", + "mender-device-identity, the single implementation of the rule.", + "", + "Serials here are synthetic. Real board serials are hardware identifiers", + "and are not committed to any repo." + ], + "family_tag": "rpi-cpuinfo-serial", + "construction": "retg + first 15 hex chars of SHA-256(\":\")", + "pattern": "^ret(?:[0-9a-f]{8}|g[0-9a-f]{15})$", + "derive": [ + { + "serial": "0000000000000001", + "node_id": "retg3d02b56e8dc8eed", + "why": "minimum plausible serial" + }, + { + "serial": "00633d6e00000000", + "node_id": "retg99f4028bee349d8", + "why": "trailing zeros, so the old rule would have produced ret00000000" + }, + { + "serial": "123456789abcdef0", + "node_id": "retg0e0998b3b40e91c", + "why": "every hex digit" + }, + { + "serial": "ffffffffffffffff", + "node_id": "retg15804102b4e1a4c", + "why": "all ones" + }, + { + "serial": "a1b2c3d4e5f60718", + "node_id": "retgdfe16b5d4d22957", + "why": "ordinary" + } + ], + "valid": [ + { + "node_id": "ret7dd2cb0d", + "why": "legacy format, still carried by unmigrated nodes" + }, + { + "node_id": "retgec420d03ea4b064", + "why": "current format" + } + ], + "invalid": [ + { + "node_id": "Unknown", + "why": "retina-gui's get_node_id() returns this on failure" + }, + { + "node_id": "ret000000000", + "why": "the placeholder in retina-node/config/default.yml" + }, + { + "node_id": "retg", + "why": "prefix alone" + }, + { + "node_id": "retgec420d03ea4b06", + "why": "one hex char short of the current format" + }, + { + "node_id": "retgec420d03ea4b0644", + "why": "one hex char too long" + }, + { + "node_id": "retGEC420D03EA4B064", + "why": "uppercase" + }, + { + "node_id": "retg ec420d03ea4b064", + "why": "embedded space" + }, + { + "node_id": "ret1a2b3c4d5", + "why": "between the two formats, matching neither" + }, + { + "node_id": "mac=b8:27:eb:00:11:22", + "why": "the non-Pi fallback is not a node_id" + } + ] +} diff --git a/mender-auto-accept/test_tunnel_sync.py b/mender-auto-accept/test_tunnel_sync.py index c9ed095..d70431d 100644 --- a/mender-auto-accept/test_tunnel_sync.py +++ b/mender-auto-accept/test_tunnel_sync.py @@ -99,3 +99,85 @@ def test_the_final_rule_is_a_catch_all(): def test_another_node_is_not_served_by_this_tunnel(): for rule in tunnel_sync.build_ingress(NODE): assert rule.get("hostname") in (HOST, None) + + +# ── which names this script may touch, and which it owns ───────────── +# +# Two questions, one shape, opposite failure modes. They used to share a +# constant, so widening the node id format for one purpose silently widened it +# for the other — and the other decides what --prune deletes. + +#: Every tunnel on retnode.com as of 2026-09-21 that this script did not create. +#: Several serve live customer nodes. If any of these ever matches, --prune +#: deletes somebody's working tunnel. +HAND_BUILT = [ + "fairforest", + "jonathan-node-1", + "jonathan-node-2", + "joshOffice", + "mississippi", + "nightcrawler", + "sacremento", + "wilderness", +] + +LEGACY_ID = "ret4c844c20" +CURRENT_ID = "retgec420d03ea4b064" + + +@pytest.mark.parametrize("node_id", [LEGACY_ID, CURRENT_ID]) +def test_both_node_id_formats_may_be_acted_on(node_id): + """The fleet is migrated one node at a time, so both are live at once.""" + tunnel_sync._guard(node_id) + + +@pytest.mark.parametrize( + "name", + HAND_BUILT + ["", None, "retg", "ret000000000", "retgec420d03ea4b06", "Unknown"], +) +def test_guard_refuses_anything_that_is_not_a_node_id(name): + with pytest.raises(RuntimeError, match="refusing to act"): + tunnel_sync._guard(name) + + +@pytest.mark.parametrize("name", HAND_BUILT) +def test_hand_built_tunnels_are_not_ours_to_sweep(name): + """The property the orphan sweep rests on. Too narrow costs a lingering + orphan; too wide deletes a production tunnel.""" + assert not tunnel_sync.OWNED_BY_US.match(name) + + +@pytest.mark.parametrize("node_id", [LEGACY_ID, CURRENT_ID]) +def test_node_tunnels_in_either_format_are_ours(node_id): + assert tunnel_sync.OWNED_BY_US.match(node_id) + + +def test_reconcile_never_offers_a_hand_built_tunnel_as_an_orphan(): + """End to end, because the constant being right is not the same as it being + used in all three sweeps.""" + tunnels = [{"name": n, "id": f"id-{n}", "connections": []} for n in HAND_BUILT] + tunnels.append({"name": CURRENT_ID, "id": "id-node", "connections": []}) + + _, orphans, _ = tunnel_sync.reconcile( + wanted=set(), state={}, tunnels=tunnels, dns_records=[], access_apps=[] + ) + + assert [name for _, name, _ in orphans] == [CURRENT_ID] + + +def test_the_dns_and_access_sweeps_agree_with_the_tunnel_sweep(): + """All three read OWNED_BY_US. A hand-built hostname in the zone must not + be swept from any of them.""" + domain = tunnel_sync.REMOTE_ACCESS_DOMAIN + dns = [ + {"name": f"{n}.{domain}", "type": "CNAME", "id": f"dns-{n}"} + for n in HAND_BUILT + [CURRENT_ID] + ] + apps = [{"domain": f"{n}.{domain}", "id": f"app-{n}"} for n in HAND_BUILT + [CURRENT_ID]] + + _, orphans, _ = tunnel_sync.reconcile( + wanted=set(), state={}, tunnels=[], dns_records=dns, access_apps=apps + ) + + swept = {name for _, name, _ in orphans} + assert swept == {f"{CURRENT_ID}.{domain}"} diff --git a/mender-auto-accept/tunnel_sync.py b/mender-auto-accept/tunnel_sync.py index d737cfb..a3586a5 100644 --- a/mender-auto-accept/tunnel_sync.py +++ b/mender-auto-accept/tunnel_sync.py @@ -138,24 +138,47 @@ ABSENT = "absent" +#: Both node_id formats the fleet carries: ret<8 hex> is the legacy format, +#: retg<15 hex> the current one. Nodes are migrated one at a time, so both are +#: live at once and this script sees every node in the fleet. +_NODE_ID = r"ret(?:[0-9a-f]{8}|g[0-9a-f]{15})" + #: Names this script is allowed to create, reconfigure or delete. #: #: retnode.com already carries hand-built tunnels for live customer nodes, some #: serving several hostnames each, and ensure_tunnel() PUTs a tunnel's *entire* #: ingress config. A miscomputed name would therefore not fail, it would quietly #: replace a working tunnel's routing or delete a production DNS record. Nothing -#: existing is named ret<8 hex>, so pinning the shape makes that unreachable +#: existing is named like a node id, so pinning the shape makes that unreachable #: rather than merely unlikely. -NODE_ID_RE = re.compile(r"^ret[0-9a-f]{8}$") +#: +#: Too narrow here is safe: the worst case is refusing to act. +MAY_ACT_ON = re.compile(rf"^{_NODE_ID}$") + +#: Names this script considers its own, for deciding what is an orphan. +#: +#: Identical to MAY_ACT_ON today, and deliberately a separate constant, because +#: it answers the opposite question and fails the opposite way. "May I touch +#: this?" is safe to get wrong by being too narrow. "Is this mine, and therefore +#: rubbish to sweep up?" is *dangerous* to get wrong by being too wide: an +#: over-broad pattern claims something somebody built by hand, and --prune then +#: deletes it. +#: +#: They were one constant, so widening the format for one purpose silently +#: widened it for the other. Before adding a format here, check the zone: as of +#: 2026-09-21 the eight hand-built tunnels (fairforest, jonathan-node-1, +#: jonathan-node-2, joshOffice, mississippi, nightcrawler, sacremento, +#: wilderness) match neither format, and nothing in it begins retg. +OWNED_BY_US = re.compile(rf"^{_NODE_ID}$") def _guard(node_id): """Raise unless this is a name we are allowed to touch.""" - if not node_id or not NODE_ID_RE.match(node_id): + if not node_id or not MAY_ACT_ON.match(node_id): raise RuntimeError( - f"refusing to act on {node_id!r}: not a ret<8 hex> node id. " - f"retnode.com carries hand-built tunnels that this script must " - f"never reconfigure or delete." + f"refusing to act on {node_id!r}: not a ret<8 hex> or retg<15 hex> " + f"node id. retnode.com carries hand-built tunnels that this script " + f"must never reconfigure or delete." ) @@ -526,7 +549,9 @@ def reconcile(wanted, state, tunnels, dns_records, access_apps): Returns (repairs, orphans, notes): repairs things we believe exist but do not, or point somewhere wrong. Fixed by re-running ensure_tunnel, which is idempotent. - orphans ret<8 hex> tunnels and records nothing wants any more. Reported + orphans node-id-shaped tunnels and records nothing wants any more, by + OWNED_BY_US rather than MAY_ACT_ON: this decides what --prune + may delete, so a name we do not own must never match. Reported rather than deleted unless --prune, because a Mender outage that returned a short device list would otherwise look exactly like a fleet that had all opted out. @@ -592,14 +617,14 @@ def reconcile(wanted, state, tunnels, dns_records, access_apps): for tunnel in tunnels: name = tunnel["name"] - if NODE_ID_RE.match(name) and name not in wanted and name not in state: + if OWNED_BY_US.match(name) and name not in wanted and name not in state: orphans.append(("tunnel", name, tunnel["id"])) for app in access_apps: domain = app.get("domain") or "" node_id = domain.split(".")[0] if (domain.endswith("." + REMOTE_ACCESS_DOMAIN) - and NODE_ID_RE.match(node_id) + and OWNED_BY_US.match(node_id) and node_id not in wanted and node_id not in state): orphans.append(("access-app", domain, app["id"])) @@ -608,7 +633,7 @@ def reconcile(wanted, state, tunnels, dns_records, access_apps): node_id = name.split(".")[0] if (record.get("type") == "CNAME" and name.endswith("." + REMOTE_ACCESS_DOMAIN) - and NODE_ID_RE.match(node_id) + and OWNED_BY_US.match(node_id) and node_id not in wanted and node_id not in state): orphans.append(("dns", name, record["id"]))