diff --git a/aci_sim/ndo/app.py b/aci_sim/ndo/app.py index 7aa3d87..aaad231 100644 --- a/aci_sim/ndo/app.py +++ b/aci_sim/ndo/app.py @@ -755,7 +755,7 @@ def make_ndo_app(state: NdoState, apic_states: dict[str, Any] | None = None) -> # PATCH client-side and never sends it, but stay tolerant. return detail - # F4 + F12 (design doc `_F4_F12_VALIDATION_DESIGN.md`): only PATCHes + # F4 + F12 (see aci_sim/ndo/service_graph_validation.py docstring): only PATCHes # that actually touch service-graph surface (a site-local # `serviceGraphs` add/replace, or a site-local contract's # `serviceGraphRelationship`) take the gated copy-validate-commit diff --git a/aci_sim/ndo/service_graph_validation.py b/aci_sim/ndo/service_graph_validation.py index 5dc546d..b9ed9bf 100644 --- a/aci_sim/ndo/service_graph_validation.py +++ b/aci_sim/ndo/service_graph_validation.py @@ -1,6 +1,6 @@ """F4 + F12 — NDO server-side service-graph fidelity enforcements. -See `_F4_F12_VALIDATION_DESIGN.md` (repo root) for the full design. Summary: +Summary of the two enforcements: - **F4 — device-existence gate.** When a site-local service-graph binding op (`/sites/{siteId}-{templateName}/serviceGraphs/...`) references an L4-L7 @@ -341,6 +341,19 @@ def _validate_uniform_site_redirect(working: dict, ops: list[dict]) -> None: `_touched_redirect_contracts`'s docstring for the cross-template false-reject this scoping fixes). A template with only one associated fabric is trivially uniform (design §3.4 / §4). + + Coverage-group bias (intentional fail-safe): the group below only + includes fabrics that already carry a `contracts[]` shadow entry for + *contract* — a fabric with NO shadow entry at all is excluded outright, + not counted as "not yet configured". From working state alone we can't + distinguish "this fabric hasn't received the contract shadow yet" from + "this fabric doesn't participate in this contract", so an all-but-one- + fabric-shadowless group collapses to `total <= 1` and passes rather than + risk a false 400. Live-gate testing (real NDO + cisco.mso, once + cisco.mso has built the shadow entries on every participating fabric) + confirms the real chain still correctly 400s a genuine partial redirect + write — this bias only ever widens the ambiguous states this check + deliberately stays silent on. """ sites = working.get("sites") if not isinstance(sites, list): diff --git a/aci_sim/rest_aci/validators.py b/aci_sim/rest_aci/validators.py index d3503fd..1506fc4 100644 --- a/aci_sim/rest_aci/validators.py +++ b/aci_sim/rest_aci/validators.py @@ -6,10 +6,10 @@ validation. A real APIC 400-rejects out-of-range/malformed values (e.g. ``l3extRsPathL3OutAtt encap=vlan-9999``, ``l3extRsNodeL3OutAtt rtrId=999.888.777.241``) even when the posting client (``cisco.aci.aci_rest`` raw MO bodies, in this campaign) does zero client-side validation of its own. -This module is the table-driven rule catalog that closes that gap — see -``_F10_VALIDATION_DESIGN.md`` (design doc, authoritative) for the full rule -table, sourcing, and false-reject risk analysis. Do not add rules here that -aren't in that table without updating the design doc first. +This module is the table-driven rule catalog that closes that gap — see the +RULES table below and its section comments for the full rule set, sourcing, +and false-reject-risk analysis. Do not add rules here without a comment +documenting that row's sourcing. Architecture (design §3.1): - Small composable validator *factories* (``rng``, ``one_of``, ``fmt``, @@ -117,8 +117,9 @@ _MAC_COLON_RE = re.compile(r"^[0-9A-Fa-f]{2}(:[0-9A-Fa-f]{2}){5}$") _MAC_DOT_RE = re.compile(r"^[0-9A-Fa-f]{4}\.[0-9A-Fa-f]{4}\.[0-9A-Fa-f]{4}$") #: Named L4 ports real APIC/`cisco.aci` accept in place of a numeric port, -#: sourced verbatim from `aci-ansible-dev`'s `FILTER_PORT_MAPPING` -#: (module_utils/constants.py) values, plus the literal "unspecified". +#: sourced verbatim from the vendored cisco.aci collection's +#: `FILTER_PORT_MAPPING` (module_utils/constants.py) values, plus the +#: literal "unspecified". _NAMED_PORTS = {"https", "smtp", "http", "dns", "pop3", "rtsp", "ftpData", "unspecified"} @@ -273,11 +274,12 @@ def all_of(*validators: Validator) -> Validator: # --------------------------------------------------------------------------- # RULES — (class, prop) -> Validator # -# Every entry below is sourced from a row in _F10_VALIDATION_DESIGN.md §2A +# Every entry below falls into one of two source categories: §2A # (Priority 1 — server-only format/range) or §2B (Priority 2 — server-only -# enums). Comments cite the design-doc row. Cross-field / cardinality rules -# (§2C, e.g. infraPortBlk fromPort<=toPort, fvnsEncapBlk from<=to) are OUT OF -# SCOPE for this batch (P1+P2 only) — deliberately not implemented here. +# enums), per the rule-source notes in this module's docstring. Cross-field / +# cardinality rules (§2C, e.g. infraPortBlk fromPort<=toPort, fvnsEncapBlk +# from<=to) are OUT OF SCOPE for this batch (P1+P2 only) — deliberately not +# implemented here. # --------------------------------------------------------------------------- RULES: dict[tuple[str, str], Validator] = { diff --git a/aci_sim/rest_aci/writes.py b/aci_sim/rest_aci/writes.py index 48fd2e4..8c2aff9 100644 --- a/aci_sim/rest_aci/writes.py +++ b/aci_sim/rest_aci/writes.py @@ -224,10 +224,10 @@ def _validate_planned(planned: list[tuple[str, dict]]) -> None: anything malformed that predates this check). Table-driven (F10): every ``(class, prop)`` pair checked here comes from - ``aci_sim.rest_aci.validators.RULES`` — see that module + design doc - ``_F10_VALIDATION_DESIGN.md`` §3.2/§3.6 for the registry and its - fail-safe default-allow policy. A ``(class, prop)`` pair with NO entry in - RULES is intentionally NOT validated (unknown ⇒ allow, never reject).""" + ``aci_sim.rest_aci.validators.RULES`` — see that module's docstring and + section comments for the registry and its fail-safe default-allow + policy. A ``(class, prop)`` pair with NO entry in RULES is intentionally + NOT validated (unknown ⇒ allow, never reject).""" for mo_cls, mo_attrs in planned: if mo_attrs.get("status") == "deleted": continue diff --git a/tests/test_f10_validators.py b/tests/test_f10_validators.py index ee173dc..cc5265d 100644 --- a/tests/test_f10_validators.py +++ b/tests/test_f10_validators.py @@ -1,7 +1,7 @@ """F10 — server-side value validation, unit level (aci_sim/rest_aci/validators.py). -Table-driven per the design doc's own zero-false-reject proof methodology -(``_F10_VALIDATION_DESIGN.md`` §6): every canonical value that appears in the +Table-driven per this suite's own zero-false-reject proof methodology: every +canonical value that appears in the real-hardware-passing corpus (``~/e2e-20260708/vars/{ms,sf,common}/`` + ``logs/ansible/*create_tenant*.yml``) MUST pass; every F10-probe bad value (``logs/probe/*probe_L2b.yml``) and one or two hand-built out-of-range/format @@ -322,11 +322,10 @@ def test_unregistered_class_prop_pair_has_no_validator(): def test_rules_table_size_matches_design_scope(): """Sanity lock on the registry shape: P1+P2 (+3 migrated ip rules) should - land in the low-to-mid 40s of (class, prop) entries — see - _F10_VALIDATION_DESIGN.md §2A/§2B row counts (16+17 table ROWS; several - rows cover 2+ props, e.g. infraPortBlk's 4 port/card fields, so the - (class, prop) key count is naturally higher than the 31 "core rules" - row-count figure quoted in the design doc's summary).""" + land in the low-to-mid 40s of (class, prop) entries (16+17 source-table + rows; several rows cover 2+ props, e.g. infraPortBlk's 4 port/card + fields, so the (class, prop) key count is naturally higher than the 31 + "core rules" row-count figure).""" assert 35 <= len(RULES) <= 55 diff --git a/tests/test_f12_uniform_redirect.py b/tests/test_f12_uniform_redirect.py index 4bd5a7e..64502f6 100644 --- a/tests/test_f12_uniform_redirect.py +++ b/tests/test_f12_uniform_redirect.py @@ -1,11 +1,11 @@ """F12 — NDO uniform per-fabric service-graph redirect gate. -Design: `_F4_F12_VALIDATION_DESIGN.md` §3 (repo root). Confirmed sim gap: a -single-fabric (partial) `serviceGraphRelationship` redirect write used to -succeed in the sim while real NDO 400s with `must have uniform redirect -policy configured on all fabrics` — NDO validates the FINAL POST-REQUEST -state, so a legal atomic multi-op PATCH covering every fabric of a -template in ONE request must still pass. +See aci_sim/ndo/service_graph_validation.py's docstring for the full F12 +rule. Confirmed sim gap: a single-fabric (partial) `serviceGraphRelationship` +redirect write used to succeed in the sim while real NDO 400s with `must +have uniform redirect policy configured on all fabrics` — NDO validates the +FINAL POST-REQUEST state, so a legal atomic multi-op PATCH covering every +fabric of a template in ONE request must still pass. Replicates the mso-model role's "Atomic PATCH — bind service-graph redirect on ALL fabrics in one request" task (design §1.4) — the exact shape diff --git a/tests/test_f4_service_graph_device.py b/tests/test_f4_service_graph_device.py index 4725892..2d9bbd7 100644 --- a/tests/test_f4_service_graph_device.py +++ b/tests/test_f4_service_graph_device.py @@ -1,8 +1,9 @@ """F4 — NDO service-graph device-existence gate. -Design: `_F4_F12_VALIDATION_DESIGN.md` §2 (repo root). Confirmed sim gap: a -phase2-only service-graph bind (skip phase1's APIC-side `vnsLDevVip` -create) used to succeed in the sim while real NDO 400s with +See aci_sim/ndo/service_graph_validation.py's docstring for the full F4 +rule. Confirmed sim gap: a phase2-only service-graph bind (skip phase1's +APIC-side `vnsLDevVip` create) used to succeed in the sim while real NDO +400s with `Service graph device does not exist in tenant in Fabric `.