Newer
Older
openstack-caracal-dc-dc / docs / changelog-20260714-netbox-write-path-hardening.md

2026-07-14 -- NetBox write-path hardening (the bugs that would have corrupted the apex at C2)

Found by an adversarial review sweep during D-119. Independent of the naming work -- these are the defects that would have bitten during C2 (feed the validated carve UPSTREAM), which is the production IPAM write Stage 2 still needs.

1. roles-aggregates-import.py still died HALF-WAY through a production write

The preflight added on 2026-07-13 -- after this exact script created 4 roles, 400'd on the 5th, and left the apex half-populated -- only covered role-NAME collisions. Two die() calls remained BELOW the first write:

  • the ARIN RIR lookup (an apex without ARIN dies after 5 roles are committed), and
  • validate_ula_48() (a typo'd ORG_ULA_48 dies after roles AND RIRs are committed).

So the tool could still leave the production IPAM half-written with no rollback -- the exact failure the preflight was added to prevent, still armed one section lower. The RIR half of the preflight loop was also a literal no-op (if ...: continue followed by a comment).

Fixed: both checks moved INTO the preflight; the RIR name-collision check is now real. A preflight that does not cover every die() downstream of the first write is not a preflight.

2. sandbox-fidelity-check.py could return a FALSE GREEN

This is the one that stings: it is the script we have been citing as proof the sandbox is a faithful replica.

It field-compares only the SHARED objects. The entire planned delta was checked merely as a subset of EXPECTED_NEW:

unexpected = extra - expected     # proves the delta is a SUBSET of the plan
                                  # NEVER proves the plan was CARRIED OUT

So with zero of the 36 DC prefixes created, extra = {}, unexpected = {}, and it printed "Nothing lost, nothing stray" and exited 0. EXPECTED_NEW was an upper bound masquerading as an assertion. A delta prefix created with the wrong scope, role or status also passed clean -- it was never field-compared against anything.

Fixed: EXPECTED_NEW is now BOTH bounds (EXPECTED-BUT-ABSENT is a hard failure), and delta prefixes are checked for scope and role. That second check is the 17-prefix scope-drop bug relocated into the delta, where the old check was structurally blind to it.

3. dc-dc-prefixes-import.py died mid-loop on a missing role

find_role() was called lazily INSIDE the write loop. Against the apex as it stands today (no six-plane roles exist upstream), a --commit would create the site and the first 4 prefixes, then die on the 5th: a half-written datacenter in the production IPAM. Now a whole-plan preflight resolves every role BEFORE the first write.

4. The test fake was KINDER THAN REALITY at exactly the failure point

fake_pynetbox.get() returned matches[0] on a multi-match. Real pynetbox RAISES. NetBox permits duplicate prefixes (this importer's own docstring anticipates them against vr0-dc0), so a global .get(prefix=...) would blow up mid-loop in production while the harness sailed through. A fake that is gentler than the real thing hides the bug it exists to catch. It now raises.

Tests

  • tests/sandbox-fidelity-check/ -- NEW, 10/10. The checker shipped with no harness at all, which is precisely why the false green survived. T3 reproduces it: an unapplied plan used to exit 0. T1 derives its expectation FROM THE CHECKER'S OWN EXPECTED_NEW rather than re-typing it -- a harness holding its own copy of the expectation cannot detect the two drifting apart.
  • tests/roles-aggregates-import/ 16 -> 19: asserts by POSITION that the ARIN check and the ULA validation both precede the first create. The old harness only asserted the word "PREFLIGHT" appeared before the first write -- so it went green while the landmine was armed.
  • tests/dc-dc-prefixes-import/ 82 (unchanged count; the fake now raises).

GAUNTLET ALL GREEN (58 harnesses); repo-lint 0 fail.

Still OPEN (logged, not fixed)

  • Same-dumper blind spot: the fidelity check compares two dumps produced by the SAME field list, so any field the dumper omits is invisible on BOTH sides by construction (prefix vrf/tenant/ vlan/tags/custom_fields; site tenant/group/facility). Structurally the same shape as the region-scope bug. Benign only while upstream uses none of them -- nothing enforces that.
  • Duplicate-CIDR collapse: prefix-keyed dicts in the seeder and the checker collapse duplicate prefixes, so a whole prefix object could vanish with a green check. Latent (no duplicates upstream today) -- but the importer's .get(prefix=...) is a GLOBAL lookup, so --update could rebind another site's prefix. Scoping it by scope_id is the real fix.

Revert

git revert <this commit>

No NetBox was written during this change -- all work was offline against synthetic fixtures.