mirror of
https://github.com/dtzp555-max/aci-sim.git
synced 2026-07-19 09:46:33 +00:00
feat: APIC-style write validation, query-target=children, contract shadow mirroring (v0.21.0)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
7fdf31aca9
commit
867c05c9c5
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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).
|
||||
|
||||
+1
-1
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user