From 867c05c9c587c679fd465212861d38034d643df2 Mon Sep 17 00:00:00 2001 From: dtzp555-max Date: Tue, 7 Jul 2026 18:11:59 +1000 Subject: [PATCH] feat: APIC-style write validation, query-target=children, contract shadow mirroring (v0.21.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fidelity gaps surfaced by one broken TN2 var file (missing firewall block), all fixed: - writes: reject malformed fvSubnet/l3extSubnet/vnsRedirectDest ip values with an APIC-style 400 before any store mutation (deletes exempt) - query engine: support query-target=children (direct children, flat imdata, root excluded) — was silently returning empty - deploy mirror: materialize vzFilter/vzEntry, vzBrCP/vzSubj (+ filter and service-graph subject bindings) and vnsAbsGraph shadows per target site; undeploy removes them E2E-verified through the real pipeline (aci-py hidden push): a TN2 var missing the fw block now fails with 'Invalid value "." for property ip of vnsRedirectDest ... -> 400' and nothing lands; a complete var set builds both MS tenants with contract shadows + sgt-FW graph binding on both sites. Suite: 914 passed. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 22 +++++ aci_sim/ndo/deploy_mirror.py | 92 +++++++++++++++++++ aci_sim/query/engine.py | 17 ++++ aci_sim/rest_aci/writes.py | 35 ++++++++ pyproject.toml | 2 +- tests/test_deploy_mirror.py | 59 +++++++++++++ tests/test_write_validation_and_children.py | 97 +++++++++++++++++++++ 7 files changed, 323 insertions(+), 1 deletion(-) create mode 100644 tests/test_write_validation_and_children.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d108722..ce1d4e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,28 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) (pre-1.0: minor bumps may include breaking changes to the sim's behavior). +## [0.21.0] - 2026-07-07 + +### Added +- **Write validation — fail like real gear**: `POST /api/mo` now rejects + malformed property values with an APIC-style 400 instead of absorbing + them into the MIT: `fvSubnet`/`l3extSubnet` `ip` must be a valid + address[/prefix], `vnsRedirectDest` `ip` a valid IP. (Surfaced by a TN2 + push whose var file was missing the firewall block: the unguarded J2 + template rendered `ip=".1/24"` and `ip="."` and the sim accepted both.) + Deletes are exempt — cleanup only needs the DN. +- **`query-target=children`** on MO/class queries (real-APIC semantics: + the DIRECT children of each matched root as flat imdata, root excluded, + `target-subtree-class` honored). Previously unsupported and silently + answered with empty imdata — a verification false-negative. +- **Deploy mirror: contract/filter/service-graph shadows** — an NDO deploy + now also materializes `vzFilter`/`vzEntry`, `vzBrCP`/`vzSubj` (+ + `vzRsSubjFiltAtt`, and `vzRsSubjGraphAtt` when the NDO contract carries a + serviceGraphRelationship) and `vnsAbsGraph` on every target site's APIC. + Previously the mirrored EPGs' `fvRsProv`/`fvRsCons` were dangling + references (`GET /api/class/vzBrCP` returned 0 fabric-wide) — same family + as the F1 fvTenant-root gap. Undeploy tears the shadows down. + ## [0.20.5] - 2026-07-07 ### Docs diff --git a/aci_sim/ndo/deploy_mirror.py b/aci_sim/ndo/deploy_mirror.py index 04eef0f..c5d07fd 100644 --- a/aci_sim/ndo/deploy_mirror.py +++ b/aci_sim/ndo/deploy_mirror.py @@ -314,9 +314,97 @@ def _template_object_dns(tenant: str, template: dict) -> list[tuple[str, str]]: epg_name = epg.get("name") if isinstance(epg, dict) else None if epg_name: dns.append((f"uni/tn-{tenant}/ap-{anp_name}/epg-{epg_name}", "fvAEPg")) + for flt in template.get("filters", []) or []: + name = flt.get("name") if isinstance(flt, dict) else None + if name: + dns.append((f"uni/tn-{tenant}/flt-{name}", "vzFilter")) + for con in template.get("contracts", []) or []: + name = con.get("name") if isinstance(con, dict) else None + if name: + dns.append((f"uni/tn-{tenant}/brc-{name}", "vzBrCP")) + for graph in template.get("serviceGraphs", []) or []: + name = graph.get("name") if isinstance(graph, dict) else None + if name: + dns.append((f"uni/tn-{tenant}/AbsGraph-{name}", "vnsAbsGraph")) return dns +def _mirror_contracts(store: MITStore, tenant: str, template: dict) -> int: + """Mirror the template's filters/contracts/service-graphs into a site + APIC store as their shadow MOs (vzFilter/vzEntry, vzBrCP/vzSubj + the + filter and service-graph subject bindings, vnsAbsGraph). + + A real NDO deploy creates these shadow objects on every target site's + APIC; without them the mirrored EPGs' fvRsProv/fvRsCons are DANGLING + references — same family as F1 (children/relations exist, the referenced + object MO doesn't), surfaced 2026-07-07 when `GET /api/class/vzBrCP` + returned 0 fabric-wide despite deployed MS tenants providing/consuming + contracts. Shadow-subject naming: one vzSubj per contract, named after + the contract ("con-X" -> "sub-X"), carrying the filter attachments and + the service-graph binding (tnVnsAbsGraphName) when the NDO contract has + a serviceGraphRelationship.""" + written = 0 + for flt in template.get("filters", []) or []: + if not isinstance(flt, dict) or not flt.get("name"): + continue + flt_name = flt["name"] + flt_dn = f"uni/tn-{tenant}/flt-{flt_name}" + store.upsert(MO("vzFilter", dn=flt_dn, name=flt_name)) + written += 1 + for entry in flt.get("entries", []) or []: + if not isinstance(entry, dict) or not entry.get("name"): + continue + store.upsert(MO( + "vzEntry", + dn=f"{flt_dn}/e-{entry['name']}", + name=entry["name"], + etherT=entry.get("etherType", "unspecified"), + prot=entry.get("ipProtocol", "unspecified"), + )) + written += 1 + for con in template.get("contracts", []) or []: + if not isinstance(con, dict) or not con.get("name"): + continue + con_name = con["name"] + con_dn = f"uni/tn-{tenant}/brc-{con_name}" + store.upsert(MO("vzBrCP", dn=con_dn, name=con_name, scope=con.get("scope", "context"))) + written += 1 + subj_name = f"sub-{con_name[4:]}" if con_name.startswith("con-") else f"sub-{con_name}" + subj_dn = f"{con_dn}/subj-{subj_name}" + store.upsert(MO("vzSubj", dn=subj_dn, name=subj_name)) + written += 1 + for rel in con.get("filterRelationships", []) or []: + ref = rel.get("filterRef", "") if isinstance(rel, dict) else "" + rel_flt = _basename(ref) + if not rel_flt: + continue + store.upsert(MO( + "vzRsSubjFiltAtt", + dn=f"{subj_dn}/rssubjFiltAtt-{rel_flt}", + tnVzFilterName=rel_flt, + )) + written += 1 + graph_rel = con.get("serviceGraphRelationship") or {} + graph_name = _basename(graph_rel.get("serviceGraphRef", "")) if isinstance(graph_rel, dict) else "" + if graph_name: + store.upsert(MO( + "vzRsSubjGraphAtt", + dn=f"{subj_dn}/rsSubjGraphAtt", + tnVnsAbsGraphName=graph_name, + )) + written += 1 + for graph in template.get("serviceGraphs", []) or []: + if not isinstance(graph, dict) or not graph.get("name"): + continue + store.upsert(MO( + "vnsAbsGraph", + dn=f"uni/tn-{tenant}/AbsGraph-{graph['name']}", + name=graph["name"], + )) + written += 1 + return written + + def mirror_template_to_sites( state: NdoState, template_name: str, @@ -432,4 +520,8 @@ def mirror_template_to_sites( _mirror_epg(store, tenant, anp_name, epg, site_epg) written += 1 + # Contract/filter/service-graph shadows for this site — see + # _mirror_contracts (dangling fvRsProv/fvRsCons fix). + written += _mirror_contracts(store, tenant, template) + return written diff --git a/aci_sim/query/engine.py b/aci_sim/query/engine.py index edba3fe..c0e1b49 100644 --- a/aci_sim/query/engine.py +++ b/aci_sim/query/engine.py @@ -197,6 +197,23 @@ def _build_result( page_items = _paginate(flat, params.page, params.page_size) return [mo.to_imdata() for mo in page_items], total + # query-target=children (real APIC): scope = the DIRECT children of each + # matched root — root itself excluded — returned as FLAT top-level imdata + # entries; target-subtree-class and query-target-filter apply per child. + # (Was previously unsupported and silently returned empty imdata, which + # reads as "object has no children" — a verification false-negative.) + if params.query_target == "children" and params.rsp_subtree is None: + cls_filter = _subtree_classes(params) + flat_children: list[MO] = [] + for root in top_level: + for mo in store.children(root.dn): + if cls_filter is None or mo.class_name in cls_filter: + flat_children.append(mo) + flat_children = [mo for mo in flat_children if pred(mo)] + total = len(flat_children) + page_items = _paginate(flat_children, params.page, params.page_size) + return [mo.to_imdata() for mo in page_items], total + # Modern rsp-subtree=children|full: NESTED children under each matched object. filtered = [mo for mo in top_level if pred(mo)] total = len(filtered) diff --git a/aci_sim/rest_aci/writes.py b/aci_sim/rest_aci/writes.py index a007d7f..1263850 100644 --- a/aci_sim/rest_aci/writes.py +++ b/aci_sim/rest_aci/writes.py @@ -2,6 +2,7 @@ from __future__ import annotations +import ipaddress import re from aci_sim.build.fabric import loopback_ip, oob_ip @@ -213,6 +214,39 @@ def _validate_node(cls: str, body: dict, path: str) -> None: _validate_node(child_cls, child_body, child_path) +def _validate_planned(planned: list[tuple[str, dict]]) -> None: + """Reject malformed property values a real APIC would 400 on (error-801 + style) — e.g. ``fvSubnet ip=".1/24"`` or ``vnsRedirectDest ip="."``, the + exact garbage an unguarded J2 template renders when its input vars are + missing. The sim's contract is to FAIL the way real gear fails, not to + absorb it into the MIT. Only values actually PRESENT on a non-delete + write are validated (a delete needs nothing but the DN — that stays the + cleanup path for anything malformed that predates this check).""" + for mo_cls, mo_attrs in planned: + if mo_attrs.get("status") == "deleted": + continue + if mo_cls in ("fvSubnet", "l3extSubnet"): + ip = mo_attrs.get("ip") + if ip is not None: + try: + ipaddress.ip_interface(ip) + except ValueError: + raise WriteValidationError( + f"Invalid value {ip!r} for property 'ip' of {mo_cls} " + f"{mo_attrs.get('dn', '')!r}: not a valid address[/prefix]" + ) from None + elif mo_cls == "vnsRedirectDest": + ip = mo_attrs.get("ip") + if ip is not None: + try: + ipaddress.ip_address(ip) + except ValueError: + raise WriteValidationError( + f"Invalid value {ip!r} for property 'ip' of vnsRedirectDest " + f"{mo_attrs.get('dn', '')!r}: not a valid IP address" + ) from None + + def _plan_recursive(cls: str, attrs: dict, children: list, planned: list[tuple[str, dict]]) -> None: """Build the ordered list of (class, attrs) MOs to write, without touching the store. @@ -253,6 +287,7 @@ def _upsert_recursive(store: MITStore, cls: str, attrs: dict, children: list) -> planned: list[tuple[str, dict]] = [] _plan_recursive(cls, attrs, children, planned) + _validate_planned(planned) # reject malformed values BEFORE any store mutation # Validation passed for the entire subtree — now, and only now, mutate # the store (400 on validation failure => zero side effects). diff --git a/pyproject.toml b/pyproject.toml index 42bfb11..83db37e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "aci-sim" -version = "0.20.5" +version = "0.21.0" description = "Faithful REST simulator of a 2-site Cisco ACI fabric (per-site APIC + ND/NDO)" readme = "README.md" license = "PolyForm-Noncommercial-1.0.0" diff --git a/tests/test_deploy_mirror.py b/tests/test_deploy_mirror.py index fe4dcb2..f05c24d 100644 --- a/tests/test_deploy_mirror.py +++ b/tests/test_deploy_mirror.py @@ -494,3 +494,62 @@ def test_app_level_undeploy_via_task_body(): assert resp.status_code == 200 assert apic_states["1"].store.get(f"uni/tn-{TENANT_NAME}/ap-AP1/epg-Web") is None + + +# --------------------------------------------------------------------------- +# Contract/filter/service-graph shadow mirroring (2026-07-07 gap: vzBrCP=0 +# fabric-wide while mirrored EPGs carried fvRsProv/fvRsCons to those names) +# --------------------------------------------------------------------------- + + +def _firewall_template(state) -> None: + template = state.schema_details[SCHEMA_ID]["templates"][0] + template["filters"] = [{ + "name": "flt-permit_ip_LAB0", + "entries": [{"name": "permit_ip", "etherType": "ip", "ipProtocol": "unspecified"}], + }] + template["serviceGraphs"] = [{"name": "sgt-FW_LAB0"}] + template["contracts"] = [{ + "name": "con-Firewall_LAB0", + "scope": "context", + "filterRelationships": [ + {"filterRef": f"/schemas/{SCHEMA_ID}/templates/{TEMPLATE_NAME}/filters/flt-permit_ip_LAB0"} + ], + "serviceGraphRelationship": { + "serviceGraphRef": f"/schemas/{SCHEMA_ID}/templates/{TEMPLATE_NAME}/serviceGraphs/sgt-FW_LAB0" + }, + }] + + +def test_mirror_materializes_contract_shadows() -> None: + state = _build_state() + _firewall_template(state) + apic = _FakeApicState() + mirror_template_to_sites(state, TEMPLATE_NAME, {"1": apic}, schema_id=SCHEMA_ID) + store = apic.store + tn = f"uni/tn-{TENANT_NAME}" + con_dn = f"{tn}/brc-con-Firewall_LAB0" + subj_dn = f"{con_dn}/subj-sub-Firewall_LAB0" + assert store.get(con_dn) is not None + assert store.get(con_dn).attrs["scope"] == "context" + assert store.get(subj_dn) is not None + filt_att = store.get(f"{subj_dn}/rssubjFiltAtt-flt-permit_ip_LAB0") + assert filt_att is not None and filt_att.attrs["tnVzFilterName"] == "flt-permit_ip_LAB0" + graph_att = store.get(f"{subj_dn}/rsSubjGraphAtt") + assert graph_att is not None and graph_att.attrs["tnVnsAbsGraphName"] == "sgt-FW_LAB0" + assert store.get(f"{tn}/flt-flt-permit_ip_LAB0") is not None + assert store.get(f"{tn}/flt-flt-permit_ip_LAB0/e-permit_ip") is not None + assert store.get(f"{tn}/AbsGraph-sgt-FW_LAB0") is not None + + +def test_undeploy_removes_contract_shadows() -> None: + state = _build_state() + _firewall_template(state) + apic = _FakeApicState() + mirror_template_to_sites(state, TEMPLATE_NAME, {"1": apic}, schema_id=SCHEMA_ID) + tn = f"uni/tn-{TENANT_NAME}" + assert apic.store.get(f"{tn}/brc-con-Firewall_LAB0") is not None + mirror_template_to_sites(state, TEMPLATE_NAME, {"1": apic}, schema_id=SCHEMA_ID, undeploy=True) + assert apic.store.get(f"{tn}/brc-con-Firewall_LAB0") is None + assert apic.store.get(f"{tn}/flt-flt-permit_ip_LAB0") is None + assert apic.store.get(f"{tn}/AbsGraph-sgt-FW_LAB0") is None diff --git a/tests/test_write_validation_and_children.py b/tests/test_write_validation_and_children.py new file mode 100644 index 0000000..21280fb --- /dev/null +++ b/tests/test_write_validation_and_children.py @@ -0,0 +1,97 @@ +"""Malformed-property write validation (APIC-style 400) + query-target=children. + +Both surfaced 2026-07-07 by a TN2 push whose var file was missing the +firewall block: the unguarded J2 template rendered ``fvSubnet ip=".1/24"`` +and ``vnsRedirectDest ip="."`` and the sim ACCEPTED both (a real APIC 400s), +and the verification that should have caught it used ``query-target=children`` +— which the sim silently treated as unsupported and answered with empty +imdata (a false negative).""" + +from __future__ import annotations + +import copy + +import pytest +from fastapi.testclient import TestClient + +from aci_sim.build.orchestrator import build_site +from aci_sim.rest_aci.app import ApicSiteState, make_apic_app +from aci_sim.topology.loader import load_topology + +TOPO_PATH = "topology.yaml" +TENANT = "VALTEST-T1" +BD_DN = f"uni/tn-{TENANT}/BD-bd-Val1" + + +@pytest.fixture(scope="module") +def client(): + topo = load_topology(TOPO_PATH) + site = topo.sites[0] + store = build_site(topo, site) + state = ApicSiteState( + name=site.name, site=site, topo=topo, + store=store, baseline=copy.deepcopy(store), + ) + c = TestClient(make_apic_app(state)) + resp = c.post( + "/api/aaaLogin.json", + json={"aaaUser": {"attributes": {"name": "admin", "pwd": "cisco"}}}, + ) + assert resp.status_code == 200 + _post(c, f"uni/tn-{TENANT}", {"fvTenant": {"attributes": {"name": TENANT}}}) + _post(c, BD_DN, {"fvBD": {"attributes": {"name": "bd-Val1", "dn": BD_DN}}}) + return c + + +def _post(client, dn: str, body: dict): + return client.post(f"/api/mo/{dn}.json", json=body) + + +def test_malformed_fvsubnet_ip_rejected_like_real_apic(client): + dn = f"{BD_DN}/subnet-[.1/24]" + r = _post(client, dn, {"fvSubnet": {"attributes": {"ip": ".1/24", "dn": dn}}}) + assert r.status_code == 400 + err = r.json()["imdata"][0]["error"]["attributes"] + assert ".1/24" in err["text"] + q = client.get("/api/class/fvSubnet.json").json() + assert not any( + ".1/24" in i["fvSubnet"]["attributes"]["dn"] for i in q["imdata"] + ), "malformed subnet must not land in the MIT" + + +def test_malformed_vnsredirectdest_rejected(client): + pol_dn = f"uni/tn-{TENANT}/svcCont/svcRedirectPol-pbr-Val" + r = _post(client, pol_dn, {"vnsSvcRedirectPol": {"attributes": {"name": "pbr-Val", "dn": pol_dn}}}) + assert r.status_code == 200 + dest_dn = f"{pol_dn}/RedirectDest_ip-[.]" + r = _post(client, dest_dn, {"vnsRedirectDest": {"attributes": {"ip": ".", "dn": dest_dn}}}) + assert r.status_code == 400 + + +def test_valid_subnet_accepted_and_delete_skips_validation(client): + sub_dn = f"{BD_DN}/subnet-[10.99.1.1/24]" + r = _post(client, sub_dn, {"fvSubnet": {"attributes": {"ip": "10.99.1.1/24", "dn": sub_dn}}}) + assert r.status_code == 200 + # A delete carries no ip attribute — validation must not block cleanup. + r = _post(client, sub_dn, {"fvSubnet": {"attributes": {"dn": sub_dn, "status": "deleted"}}}) + assert r.status_code == 200 + + +def test_query_target_children_returns_direct_children_flat(client): + # Re-create the subnet so the BD has a child to find. + sub_dn = f"{BD_DN}/subnet-[10.99.1.1/24]" + _post(client, sub_dn, {"fvSubnet": {"attributes": {"ip": "10.99.1.1/24", "dn": sub_dn}}}) + r = client.get(f"/api/node/mo/uni/tn-{TENANT}.json?query-target=children") + data = r.json() + classes = [next(iter(i)) for i in data["imdata"]] + assert "fvBD" in classes, f"direct child missing: {classes}" + assert "fvTenant" not in classes, "root must be excluded" + r2 = client.get( + f"/api/node/mo/uni/tn-{TENANT}.json?query-target=children&target-subtree-class=fvBD" + ) + classes2 = [next(iter(i)) for i in r2.json()["imdata"]] + assert classes2 and set(classes2) == {"fvBD"} + # Grandchildren (the subnet) must NOT appear — children is one level only. + r3 = client.get(f"/api/node/mo/{BD_DN}.json?query-target=children") + classes3 = [next(iter(i)) for i in r3.json()["imdata"]] + assert "fvSubnet" in classes3