mirror of
https://github.com/dtzp555-max/aci-sim.git
synced 2026-07-21 21:15:12 +00:00
Initial public release — aci-sim v0.16.0
aci-sim is a stateful REST simulator of a 2-site Cisco ACI fabric (per-site APIC + ND/NDO management planes) — for testing ACI automation (Ansible cisco.aci / cisco.mso, aci-py, custom REST clients) and CI gates without real hardware. Highlights: APIC + NDO REST surface served from one in-memory MIT built from a declarative topology.yaml; port mode + sandbox (real per-device IPs on :443) run modes; LLDP/CDP neighbor visibility; NDO->APIC deploy mirror (multi-site templates materialize onto the target sites' APIC stores); real-APIC fvBD default attributes; file-backed state save/restore; an `aci-sim` CLI (validate/show/graph/run/new/init/lldp); and an 888-test suite. History squashed for the public release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Claude Fable 5
@@ -0,0 +1,321 @@
|
||||
"""PR-16 — NDO site-local BD duplication (bdRef normalization).
|
||||
|
||||
CONFIRMED PRODUCTION SYMPTOM: after deploying the Acme App1-Net tenant through
|
||||
`create_tenant.yml` + `create_bd.yml` against a real NDO schema
|
||||
(`App1-Net-LAB0`), `sites[0].bds` carried TWO entries for the same template
|
||||
BD `App1-Net_VLAN10` — one empty mirror
|
||||
(`bdRef=".../templates/LAB1/bds/App1-Net_VLAN10"`, `subnets=[]`)
|
||||
created by `_mirror_template_bd_to_sites` (PR-15) when the template BD was
|
||||
added, and a SECOND entry carrying the real subnet
|
||||
(`10.10.10.1/24`) added later by `mso_schema_site_bd`'s own
|
||||
`add /sites/{siteId}-{tmpl}/bds/-` (append) payload — confirmed by the
|
||||
pre-existing `test_patch_add_site_bd_by_composite_key` (test_pr11_ndo_write.py)
|
||||
to use the DICT bdRef form (`{bdName: ...}`, no `schemaId`/`templateName`),
|
||||
a form `_find_by_name`'s ref-lookup never matched because the append (`-`)
|
||||
branch of `apply_json_patch` didn't consult `_find_by_name`/ref-resolution
|
||||
AT ALL before appending — it always appended unconditionally.
|
||||
|
||||
ROOT CAUSE: `apply_json_patch`'s `op == "add"` branch has three
|
||||
sub-branches keyed by the *last path token* — `"-"` (append),
|
||||
a numeric index (`insert`), or a name (resolved via `_find_by_name`, which
|
||||
DOES already dedup by matching a `*Ref` field's bare trailing name). Only
|
||||
the named-segment sub-branch deduped. The append (`"-"`) sub-branch — the
|
||||
exact form `mso_schema_site_bd.py`/`mso_schema_site_anp.py`/
|
||||
`mso_schema_template_contract_filter.py`'s "does not exist yet" branches
|
||||
all use when addressing a site-local `bds`/`anps`/`contracts` collection —
|
||||
never checked for an existing entry, so a template-add mirror followed by
|
||||
a site-module add for the SAME underlying object always produced a
|
||||
duplicate.
|
||||
|
||||
FIX: `_find_shadow_by_identity()` resolves a candidate append *value*'s own
|
||||
IDENTITY ref for the target collection (`bds`→`bdRef`, `anps`→`anpRef`,
|
||||
`epgs`→`epgRef`, `contracts`→`contractRef`), in either string or dict form,
|
||||
to a bare name and looks for an existing NAMELESS shadow entry with that
|
||||
same identity ref. The `add`+`-` branch calls it before appending: a match
|
||||
REPLACES that entry in place (real NDO's "one site-BD row per template BD"
|
||||
invariant and cisco.mso's GET-then-write idiom); only a genuinely new
|
||||
shadow appends.
|
||||
|
||||
CRITICAL SCOPING (the regression the first PR-16 cut had): identity
|
||||
matching is restricted to NAMELESS shadows and to the ONE identity ref per
|
||||
collection. A NAMED object (a template-level BD add, which carries its own
|
||||
`name`) always appends — it is never deduped by a `*Ref`. And a *property*
|
||||
ref like `vrfRef` (which every template BD under one L3-attached VRF shares,
|
||||
e.g. `vrf-L3_LAB0`) is NEVER used to establish identity. Matching on shared
|
||||
property refs collapsed every template's BDs into a single entry in the
|
||||
first cut; `test_distinct_template_bds_sharing_vrfref_do_not_collapse`
|
||||
guards against any recurrence.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from aci_sim.ndo.patch import apply_json_patch
|
||||
|
||||
|
||||
def _mirror_bd(doc: dict) -> None:
|
||||
"""Template BD add — fires _mirror_template_bd_to_sites (PR-15)."""
|
||||
ops = [
|
||||
{
|
||||
"op": "add",
|
||||
"path": "/templates/LAB1/bds/-",
|
||||
"value": {"name": "App1-Net_VLAN10", "displayName": "App1-Net_VLAN10"},
|
||||
}
|
||||
]
|
||||
apply_json_patch(doc, ops)
|
||||
|
||||
|
||||
def _base_doc() -> dict:
|
||||
return {
|
||||
"id": "schema-app1",
|
||||
"templates": [{"name": "LAB1", "bds": []}],
|
||||
"sites": [{"siteId": "101", "templateName": "LAB1"}],
|
||||
}
|
||||
|
||||
|
||||
def test_site_module_add_by_dict_bdref_updates_mirrored_entry_not_duplicates():
|
||||
"""The exact production sequence: template BD add (mirror fires, string
|
||||
bdRef, subnets=[]) THEN a site-module-style `add /sites/{sid}-{tmpl}/
|
||||
bds/-` addressed by the OTHER bdRef form (a dict, no schemaId/
|
||||
templateName — `mso_schema_site_bd.py`'s literal "BD does not exist"
|
||||
payload per test_patch_add_site_bd_by_composite_key). Must converge on
|
||||
ONE site-bd entry, not two."""
|
||||
doc = _base_doc()
|
||||
_mirror_bd(doc)
|
||||
site = doc["sites"][0]
|
||||
assert len(site["bds"]) == 1, "mirror must create exactly one shadow"
|
||||
assert site["bds"][0]["bdRef"] == "/schemas/schema-app1/templates/LAB1/bds/App1-Net_VLAN10"
|
||||
assert site["bds"][0]["subnets"] == []
|
||||
|
||||
site_add_ops = [
|
||||
{
|
||||
"op": "add",
|
||||
"path": "/sites/101-LAB1/bds/-",
|
||||
"value": {
|
||||
"bdRef": {"bdName": "App1-Net_VLAN10"},
|
||||
"hostBasedRouting": False,
|
||||
},
|
||||
}
|
||||
]
|
||||
apply_json_patch(doc, site_add_ops)
|
||||
|
||||
# No duplicate entry: the site-module add must have resolved to the
|
||||
# SAME list slot the mirror created (updating it in place), not
|
||||
# appended a second row. The incoming bare {"bdName": ...} form (no
|
||||
# schemaId/templateName) isn't stringified — matching pre-existing,
|
||||
# unchanged `_stringify_refs` behavior — so the stored bdRef becomes
|
||||
# whatever the caller sent; what matters is there is exactly ONE row.
|
||||
assert len(site["bds"]) == 1, (
|
||||
f"expected exactly one site-bd entry after site-module add, got {len(site['bds'])}: {site['bds']}"
|
||||
)
|
||||
assert site["bds"][0]["bdRef"] == {"bdName": "App1-Net_VLAN10"}
|
||||
assert site["bds"][0]["hostBasedRouting"] is False
|
||||
|
||||
|
||||
def test_site_module_add_by_full_schema_dict_bdref_then_subnet_converges_on_one_entry():
|
||||
"""Same as above but the dict form carries schemaId+templateName (the
|
||||
OTHER real cisco.mso payload shape, see test_pr11_ndo_write.py's
|
||||
`test_patch_add_site_bd_subnet_by_ref_name_resolution` family), and a
|
||||
subsequent subnet add lands on the SAME entry the mirror created — this
|
||||
is the exact Acme App1-Net symptom (empty mirror + subnet-carrying entry
|
||||
must be ONE row, not two)."""
|
||||
doc = _base_doc()
|
||||
_mirror_bd(doc)
|
||||
|
||||
site_add_ops = [
|
||||
{
|
||||
"op": "add",
|
||||
"path": "/sites/101-LAB1/bds/-",
|
||||
"value": {
|
||||
"bdRef": {
|
||||
"schemaId": "schema-app1",
|
||||
"templateName": "LAB1",
|
||||
"bdName": "App1-Net_VLAN10",
|
||||
},
|
||||
"hostBasedRouting": False,
|
||||
},
|
||||
}
|
||||
]
|
||||
apply_json_patch(doc, site_add_ops)
|
||||
|
||||
site = doc["sites"][0]
|
||||
assert len(site["bds"]) == 1, f"expected one entry after site add, got {site['bds']}"
|
||||
|
||||
subnet_ops = [
|
||||
{
|
||||
"op": "add",
|
||||
"path": "/sites/101-LAB1/bds/App1-Net_VLAN10/subnets/-",
|
||||
"value": {"ip": "10.10.10.1/24", "scope": "private"},
|
||||
}
|
||||
]
|
||||
apply_json_patch(doc, subnet_ops)
|
||||
|
||||
assert len(site["bds"]) == 1, (
|
||||
f"subnet add must not create a second entry, got {len(site['bds'])}: {site['bds']}"
|
||||
)
|
||||
only_bd = site["bds"][0]
|
||||
assert only_bd["bdRef"] == "/schemas/schema-app1/templates/LAB1/bds/App1-Net_VLAN10"
|
||||
assert only_bd["subnets"] == [{"ip": "10.10.10.1/24", "scope": "private"}]
|
||||
|
||||
|
||||
def test_site_module_add_before_mirror_still_dedups_on_later_mirror_call():
|
||||
"""Order-independence: if the site-module add somehow lands BEFORE a
|
||||
(re-applied/idempotent) template mirror call, the mirror's own
|
||||
_find_by_name dedup (unchanged by this fix) still recognizes the
|
||||
site-module's entry and does not append a second one."""
|
||||
doc = _base_doc()
|
||||
site_add_ops = [
|
||||
{
|
||||
"op": "add",
|
||||
"path": "/sites/101-LAB1/bds/-",
|
||||
"value": {"bdRef": {"bdName": "App1-Net_VLAN10"}, "hostBasedRouting": False},
|
||||
}
|
||||
]
|
||||
apply_json_patch(doc, site_add_ops)
|
||||
assert len(doc["sites"][0]["bds"]) == 1
|
||||
|
||||
_mirror_bd(doc)
|
||||
assert len(doc["sites"][0]["bds"]) == 1, "re-mirroring must not duplicate an existing site-bd"
|
||||
|
||||
|
||||
def test_unrelated_appends_without_ref_field_still_append_normally():
|
||||
"""Values with no identity `*Ref` (e.g. a template-level BD add, which
|
||||
carries its own name, or a subnet add's own value) must keep appending
|
||||
unconditionally — the dedup-on-append behavior is scoped to nameless
|
||||
site-local shadow objects only, via _find_shadow_by_identity returning
|
||||
None for named / non-shadow values."""
|
||||
doc = {"templates": [{"name": "LAB1", "bds": []}], "sites": []}
|
||||
ops = [
|
||||
{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-A"}},
|
||||
{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-B"}},
|
||||
]
|
||||
apply_json_patch(doc, ops)
|
||||
assert [b["name"] for b in doc["templates"][0]["bds"]] == ["bd-A", "bd-B"]
|
||||
|
||||
|
||||
def test_multiple_distinct_bds_in_same_site_all_kept_separate():
|
||||
"""Dedup must match by bare name, not accidentally collapse different
|
||||
BDs mirrored into the same site."""
|
||||
doc = {
|
||||
"id": "schema-app1",
|
||||
"templates": [{"name": "LAB1", "bds": []}],
|
||||
"sites": [{"siteId": "101", "templateName": "LAB1"}],
|
||||
}
|
||||
apply_json_patch(doc, [{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-A"}}])
|
||||
apply_json_patch(doc, [{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-B"}}])
|
||||
site = doc["sites"][0]
|
||||
assert len(site["bds"]) == 2
|
||||
|
||||
apply_json_patch(
|
||||
doc,
|
||||
[
|
||||
{
|
||||
"op": "add",
|
||||
"path": "/sites/101-LAB1/bds/-",
|
||||
"value": {"bdRef": {"bdName": "bd-B"}, "hostBasedRouting": True},
|
||||
}
|
||||
],
|
||||
)
|
||||
assert len(site["bds"]) == 2, f"expected bd-A untouched, bd-B updated in place, got {site['bds']}"
|
||||
|
||||
def _name(bd: dict) -> str | None:
|
||||
ref = bd["bdRef"]
|
||||
if isinstance(ref, str):
|
||||
return ref.rsplit("/", 1)[-1]
|
||||
return ref.get("bdName")
|
||||
|
||||
names = {_name(b) for b in site["bds"]}
|
||||
assert names == {"bd-A", "bd-B"}
|
||||
bd_b = next(b for b in site["bds"] if _name(b) == "bd-B")
|
||||
assert bd_b["hostBasedRouting"] is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# REGRESSION GUARD — the first PR-16 cut collapsed every template's BDs to
|
||||
# one entry because it deduped named objects via a SHARED non-identity ref.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_distinct_template_bds_sharing_vrfref_do_not_collapse():
|
||||
"""CATASTROPHIC REGRESSION GUARD. In the real Acme create_bd run, every
|
||||
template BD carries the SAME `vrfRef` (`vrf-L3_LAB0`). The first PR-16
|
||||
cut resolved an appended value's identity by iterating ALL
|
||||
`_REF_LOOKUP_KEYS` (including `vrfRef`), so two DISTINCT template BDs
|
||||
(`bd-App2-...` and `bd-App3-...`) both matched via their shared
|
||||
`vrfRef` and collapsed — each `add /templates/{T}/bds/-` overwrote the
|
||||
previous BD, leaving exactly ONE BD per template. A NAMED object must
|
||||
match ONLY by name, and identity refs must exclude property refs like
|
||||
`vrfRef`."""
|
||||
doc = {"id": "s1", "templates": [{"name": "T", "bds": []}], "sites": []}
|
||||
shared_vrf = {"schemaId": "s1", "templateName": "T", "vrfName": "vrf-L3_LAB0"}
|
||||
for name in ("bd-App2-Net", "bd-App3-Hst", "bd-App4-Bkp"):
|
||||
apply_json_patch(
|
||||
doc,
|
||||
[{"op": "add", "path": "/templates/T/bds/-", "value": {"name": name, "vrfRef": dict(shared_vrf)}}],
|
||||
)
|
||||
names = [b["name"] for b in doc["templates"][0]["bds"]]
|
||||
assert names == ["bd-App2-Net", "bd-App3-Hst", "bd-App4-Bkp"], (
|
||||
f"distinct template BDs sharing a vrfRef must NOT collapse — got {names}"
|
||||
)
|
||||
|
||||
|
||||
def test_template_bd_add_with_shared_vrfref_mirrors_all_into_site():
|
||||
"""The site-mirror side of the same regression: three distinct template
|
||||
BDs sharing one `vrfRef`, added to a template with a site attached, must
|
||||
each produce their OWN site-local shadow (3 shadows, not 1)."""
|
||||
doc = {
|
||||
"id": "s1",
|
||||
"templates": [{"name": "LAB1", "bds": []}],
|
||||
"sites": [{"siteId": "101", "templateName": "LAB1"}],
|
||||
}
|
||||
shared_vrf = {"schemaId": "s1", "templateName": "LAB1", "vrfName": "vrf-L3_LAB0"}
|
||||
for name in ("bd-One", "bd-Two", "bd-Three"):
|
||||
apply_json_patch(
|
||||
doc,
|
||||
[{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": name, "vrfRef": dict(shared_vrf)}}],
|
||||
)
|
||||
assert len(doc["templates"][0]["bds"]) == 3
|
||||
site = doc["sites"][0]
|
||||
mirrored = {b["bdRef"].rsplit("/", 1)[-1] for b in site["bds"]}
|
||||
assert mirrored == {"bd-One", "bd-Two", "bd-Three"}, f"each BD needs its own shadow, got {site['bds']}"
|
||||
|
||||
|
||||
def test_site_shadow_with_vrfref_property_still_dedups_only_on_bdref():
|
||||
"""A site-BD shadow may legitimately carry a `vrfRef` property in
|
||||
addition to its identity `bdRef`. Two site-module adds for DIFFERENT
|
||||
BDs that happen to share a `vrfRef` must still stay separate (dedup is
|
||||
on `bdRef` identity, not the shared `vrfRef` property)."""
|
||||
doc = {
|
||||
"id": "s1",
|
||||
"templates": [{"name": "LAB1", "bds": []}],
|
||||
"sites": [{"siteId": "101", "templateName": "LAB1"}],
|
||||
}
|
||||
apply_json_patch(doc, [{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-P"}}])
|
||||
apply_json_patch(doc, [{"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-Q"}}])
|
||||
site = doc["sites"][0]
|
||||
assert len(site["bds"]) == 2
|
||||
vrf = "/schemas/s1/templates/LAB1/vrfs/vrf-Shared"
|
||||
apply_json_patch(
|
||||
doc,
|
||||
[{"op": "add", "path": "/sites/101-LAB1/bds/-", "value": {"bdRef": {"bdName": "bd-P"}, "vrfRef": vrf}}],
|
||||
)
|
||||
apply_json_patch(
|
||||
doc,
|
||||
[{"op": "add", "path": "/sites/101-LAB1/bds/-", "value": {"bdRef": {"bdName": "bd-Q"}, "vrfRef": vrf}}],
|
||||
)
|
||||
assert len(site["bds"]) == 2, f"distinct shadows sharing a vrfRef must not merge, got {site['bds']}"
|
||||
|
||||
|
||||
def test_replace_named_object_still_resolves_by_name_only():
|
||||
"""A `replace` addressing a named template BD by name must still work,
|
||||
and must not accidentally resolve to a sibling via a shared ref."""
|
||||
doc = {"id": "s1", "templates": [{"name": "T", "bds": []}], "sites": []}
|
||||
shared_vrf = {"schemaId": "s1", "templateName": "T", "vrfName": "vrf-Shared"}
|
||||
apply_json_patch(doc, [{"op": "add", "path": "/templates/T/bds/-", "value": {"name": "bd-A", "vrfRef": dict(shared_vrf)}}])
|
||||
apply_json_patch(doc, [{"op": "add", "path": "/templates/T/bds/-", "value": {"name": "bd-B", "vrfRef": dict(shared_vrf)}}])
|
||||
apply_json_patch(
|
||||
doc,
|
||||
[{"op": "replace", "path": "/templates/T/bds/bd-B", "value": {"name": "bd-B", "displayName": "B-updated"}}],
|
||||
)
|
||||
bds = {b["name"]: b for b in doc["templates"][0]["bds"]}
|
||||
assert set(bds) == {"bd-A", "bd-B"}
|
||||
assert bds["bd-B"].get("displayName") == "B-updated"
|
||||
assert "displayName" not in bds["bd-A"], "replace must not have touched the sibling BD"
|
||||
Reference in New Issue
Block a user