From fe3d13221a1cd926ed6fdcb0616489ade73fef37 Mon Sep 17 00:00:00 2001 From: dtzp555-max Date: Thu, 9 Jul 2026 09:59:34 +1000 Subject: [PATCH] =?UTF-8?q?feat(validation):=20server-side=20MO=20value=20?= =?UTF-8?q?validation=20=E2=80=94=20real-APIC=20400=20on=20bad=20values=20?= =?UTF-8?q?(v0.23.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes F10: the sim silently stored out-of-range/malformed scalar values (e.g. vlan-9999, IP 999.888.777.x) that real APIC/NDO reject server-side. New aci_sim/rest_aci/validators.py: table-driven RULES {(class,prop) -> validator} (range/enum/IP·MAC-format/vlan-encap). Enforced in writes.py::_validate_planned (APIC) and ndo/patch.py::apply_json_patch (NDO) before any store mutation. Fail-safe default-allow: only registered (class,prop) validated; unknown pass. Bad values now -> APIC Error 103 Malformed MO body. ~45 rules / ~18 classes. Constraints sourced from vendored cisco.aci arg-specs + standard ACI ranges, cross-checked against the real-machine var corpus for zero false-rejects (regression: full create_tenant MS-TN1 + SF-TN1 chains stay green). +176 tests (1098 passed). Referential integrity split out as F11 (needs real-APIC study). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017zdUTqU9fvCCvF3uVGZsu1 --- CHANGELOG.md | 26 +++ aci_sim/ndo/patch.py | 79 +++++++ aci_sim/rest_aci/validators.py | 387 ++++++++++++++++++++++++++++++ aci_sim/rest_aci/writes.py | 53 ++--- pyproject.toml | 2 +- tests/test_f10_apic_writeface.py | 168 +++++++++++++ tests/test_f10_ndo_validation.py | 279 ++++++++++++++++++++++ tests/test_f10_validators.py | 388 +++++++++++++++++++++++++++++++ 8 files changed, 1353 insertions(+), 29 deletions(-) create mode 100644 aci_sim/rest_aci/validators.py create mode 100644 tests/test_f10_apic_writeface.py create mode 100644 tests/test_f10_ndo_validation.py create mode 100644 tests/test_f10_validators.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac968d..f7087b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,32 @@ 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.23.0] - 2026-07-09 + +### Added +- **Server-side value validation (F10)**: the sim now rejects out-of-range / + malformed scalar MO property values that real APIC/NDO reject server-side, + instead of silently storing them. New `aci_sim/rest_aci/validators.py` holds a + table-driven `RULES: {(class, prop) -> validator}` registry (range / enum / + IP·MAC-format / vlan-encap checks); `writes.py::_validate_planned` (APIC) and + `ndo/patch.py::apply_json_patch` (NDO) enforce it before any store mutation. + **Fail-safe default-allow**: only registered `(class, prop)` pairs are checked, + so unknown properties pass untouched (no false-rejects). Bad values now return + `APIC Error 103: Malformed MO body: Invalid value ...` (real-APIC-style) — + e.g. VLAN encap outside [1,4094], VXLAN outside [1,16777215], invalid + IPv4/IPv6 address, ASN out of range, malformed MAC, out-of-range MTU/port. + Constraints sourced from the vendored cisco.aci module arg-specs and standard + ACI ranges, cross-checked against the real-machine var corpus for zero + false-rejects. ~45 rules across ~18 MO classes; +176 tests. Closes the F10 + fidelity gap (previously a var with `vlan-9999` / `999.888.777.x` pushed + rc=0 and was stored; now 400 + not stored). + +### Notes +- Referential integrity (service-graph device / BD-VRF must-exist) is + deliberately NOT included here — tracked separately (F11), as it needs a + real-APIC fidelity study to avoid over-rejecting the dangling refs that real + APIC tolerates. + ## [0.22.0] - 2026-07-08 ### Added diff --git a/aci_sim/ndo/patch.py b/aci_sim/ndo/patch.py index 3564e84..c6b7ff6 100644 --- a/aci_sim/ndo/patch.py +++ b/aci_sim/ndo/patch.py @@ -28,6 +28,8 @@ import hashlib import re from typing import Any +from aci_sim.rest_aci.validators import Validator, fmt, rng + #: Every collection key a schema template can carry. Real NDO always serves #: these back as (at minimum) empty lists even when a caller's create/add #: payload omitted them — e.g. `mso_schema_template.py`'s "schema does not @@ -81,6 +83,77 @@ class PatchError(Exception): """Raised when a JSON-Patch op cannot be applied to the stored document.""" +# --------------------------------------------------------------------------- +# F10 batch 3 — NDO value validation (design doc §3.3). +# +# NDO stores schema JSON, not APIC MOs, so values arrive as JSON-Patch +# add/replace op values rather than (class, prop) MO attrs — the RULES +# registry keying in aci_sim/rest_aci/validators.py doesn't apply verbatim. +# Instead NDO_RULES keys on (collection, field): *collection* is the name of +# the array a value is being written into/under (e.g. "subnets", +# "staticPorts" — the dict key one level up from the array itself), *field* +# is the attribute name within that array's object shape. +# +# The NDO value surface is much narrower than APIC's (design §3.3: "subnet +# ip, static-port vlan, a few vrf/bd enums") — this batch implements exactly +# the two the design doc calls out by name: subnet ip and static-port VLAN. +# The real `cisco.mso` field for the static-port VLAN is `portEncapVlan` +# (confirmed against this repo's own PR-12 test fixtures, +# tests/test_pr12_site_local.py — the design doc's pseudocode used the +# placeholder name "vlan"; `portEncapVlan` is the actual wire field). +# --------------------------------------------------------------------------- +NDO_RULES: dict[tuple[str, str], Validator] = { + ("subnets", "ip"): fmt("ipv4_iface"), + ("staticPorts", "portEncapVlan"): rng(1, 4094), +} + + +def _validate_ndo_value(collection: str, field: str, value: Any) -> None: + """Look up *collection*/*field* in NDO_RULES and raise PatchError if + *value* fails it. Fail-safe default-allow, same policy as the APIC + registry: an unregistered (collection, field) or a None value is not + validated.""" + validator = NDO_RULES.get((collection, field)) + if validator is None or value is None: + return + try: + validator(collection, field, value, None) + except ValueError as reason: + raise PatchError( + f"Invalid value {value!r} for property {field!r} of {collection!r}: {reason}" + ) from None + + +def _validate_ndo_op(tokens: list[str], container: Any, last: str, value: Any) -> None: + """Validate a single JSON-Patch add/replace op's *value* against + NDO_RULES, given the already-resolved (container, last) from + ``_resolve_container`` and the op's full *tokens* path. + + Two value shapes reach here (design §3.3 "applied to each op's value"): + + - Whole-object add/insert (``container`` is the collection list + itself, e.g. ``.../subnets/-`` or ``.../staticPorts/-``): *value* is + a dict of fields for the new item. The collection name is the token + that named this list, i.e. ``tokens[-2]`` (the list was reached by + indexing into ``doc[...][tokens[-2]]``). Validate every field of + *value* that NDO_RULES knows about. + - Single-field replace/add on an EXISTING item (``container`` is that + item's own dict, e.g. ``.../subnets/0/ip``): *last* IS the field + name and *value* is the scalar. The collection name is one level up + again — ``tokens[-3]`` (the list containing the item ``tokens[-2]`` + indexes into). + """ + if isinstance(container, list): + collection = tokens[-2] if len(tokens) >= 2 else None + if collection is not None and isinstance(value, dict): + for field, sub_value in value.items(): + _validate_ndo_value(collection, field, sub_value) + elif isinstance(container, dict): + collection = tokens[-3] if len(tokens) >= 3 else None + if collection is not None: + _validate_ndo_value(collection, last, value) + + def _normalize_object(obj: dict, array_key: str, schema_id: str, template_name: str, anp_name: str = "") -> None: """Backfill *obj*'s missing collection-default keys for its container array (e.g. array_key="bds" -> obj gets subnets/dhcpLabels defaults), @@ -695,6 +768,12 @@ def apply_json_patch(doc: dict, ops: list[dict]) -> dict: container, last = _resolve_container(doc, tokens) + if op in ("add", "replace"): + # F10 batch 3: validate before mutating (design §3.3) — a bad + # subnet ip / static-port vlan 400s instead of landing in the + # stored schema doc. + _validate_ndo_op(tokens, container, last, value) + if op == "add": if isinstance(container, list): if last == "-": diff --git a/aci_sim/rest_aci/validators.py b/aci_sim/rest_aci/validators.py new file mode 100644 index 0000000..d3503fd --- /dev/null +++ b/aci_sim/rest_aci/validators.py @@ -0,0 +1,387 @@ +"""Server-side value validators for the aci-sim APIC write face (F10). + +aci-sim is a faithful ACI management-plane MODEL: it stores/associates MOs +correctly but historically performed no APIC-style server-side attribute +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. + +Architecture (design §3.1): + - Small composable validator *factories* (``rng``, ``one_of``, ``fmt``, + ``all_of``) each return a ``Validator`` callable with signature + ``(cls: str, prop: str, value, ctx) -> None`` that raises ``ValueError`` + with a human-readable reason on an invalid value. + - ``RULES`` maps ``(class, prop) -> Validator``. A ``(class, prop)`` pair + with NO entry is intentionally unvalidated — see "Fail-safe" below. + +Fail-safe / false-reject policy (design §3.6, load-bearing): **default-allow**. +Only pairs present in RULES are checked; every other (class, prop) passes +through untouched. The bias is hard toward "never reject a legal value" — +every rule here is proven against the canonical corpus (design §6) before +being added, and any rule that would reject a canonical value must be +loosened to admit it (never loosened to admit a known-bad value instead). +""" + +from __future__ import annotations + +import ipaddress +import re +from typing import Callable + +#: A validator inspects one (class, prop, value) triple and either returns +#: None (value is legal) or raises ValueError(reason) (value is illegal). +#: ``ctx`` is reserved for cross-field checks (Priority-3 in the design doc, +#: e.g. fvnsEncapBlk from<=to) — NOT implemented in this batch (P1+P2 only); +#: it is threaded through now so a later batch can add cross-field rules +#: without changing the RULES calling convention. +Validator = Callable[[str, str, object, object], None] + + +# --------------------------------------------------------------------------- +# Factories +# --------------------------------------------------------------------------- + + +def rng(lo: int, hi: int | None = None, *, transform: Callable[[object], object] | None = None) -> Validator: + """Integer range check: ``lo <= int(value) <= hi``. + + *hi=None* means "no upper bound" (used for the few design-doc rules that + only specify a floor, e.g. infraPortBlk ``fromPort/toPort/fromCard/ + toCard`` >= 1). *transform* is applied to the raw value before the int() + conversion (e.g. stripping a "vlan-" prefix) — not currently used by any + RULES entry below (that parsing lives in the ``vlan_encap`` fmt kind + instead) but kept per the design §3.1 factory signature for forward use. + """ + + def _validator(cls: str, prop: str, value: object, ctx: object = None) -> None: + raw = value if transform is None else transform(value) + try: + n = int(raw) # type: ignore[arg-type] + except (TypeError, ValueError): + raise ValueError(f"not an integer, got {value!r}") from None + if n < lo or (hi is not None and n > hi): + bound = f"[{lo},{hi}]" if hi is not None else f">= {lo}" + raise ValueError(f"must be {bound}, got {n}") + + return _validator + + +def one_of(allowed: set[str] | frozenset[str], *, list_sep: str | None = None) -> Validator: + """Enum check. Plain membership by default; with *list_sep* the MO's + string value is a token-list (real APIC stores multi-valued enum props + like ``scope``/``ctrl``/``aggregate``/``peerCtrl`` as a single + comma-joined string — confirmed against ``cisco.aci`` module source, + e.g. ``aci_bd_subnet.py``/``aci_l3out_extsubnet.py`` doing + ``",".join(sorted(scope))``) — split on *list_sep*, and every non-empty + token must be a member of *allowed*. An empty value (no tokens at all) + is treated as vacuously valid — nothing to check, and rejecting it would + risk a false-reject on an unset/omitted enum (design §3.6 bias). + """ + allowed_set = set(allowed) + + def _validator(cls: str, prop: str, value: object, ctx: object = None) -> None: + if not isinstance(value, str): + raise ValueError(f"expected a string, got {type(value).__name__}") + if list_sep is None: + if value not in allowed_set: + raise ValueError(f"must be one of {sorted(allowed_set)}, got {value!r}") + return + tokens = [t.strip() for t in value.split(list_sep) if t.strip()] + for token in tokens: + if token not in allowed_set: + raise ValueError( + f"token {token!r} not in allowed set {sorted(allowed_set)} (value={value!r})" + ) + + return _validator + + +_VLAN_ENCAP_RE = re.compile(r"^(vlan|vxlan)-(\d+)$") +_VLAN_MIN, _VLAN_MAX = 1, 4094 # NOT 4096 — see design doc §7 point 2: +# cisco.aci docs say "1 and 4096" but real APIC reserves 4095/4096; 4094 is +# the real server ceiling and comfortably admits every canonical value +# (max observed: 2611). +_VXLAN_MIN, _VXLAN_MAX = 1, 16777215 # F10 fix (independent review): the +# vlan-N 4094 ceiling does NOT apply to vxlan-N — VXLAN VNID is a 24-bit +# field (RFC 7348), max 2**24-1 = 16777215. The original single shared +# ceiling 400-rejected every legal vxlan-N value above 4094, a 100% +# false-reject on this prefix. Fail-safe bias (module docstring / design +# §3.6): take the widest legal range rather than guess a tighter one. + +_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". +_NAMED_PORTS = {"https", "smtp", "http", "dns", "pop3", "rtsp", "ftpData", "unspecified"} + + +def _check_vlan_encap(value: object, *, allow_unknown: bool) -> None: + if not isinstance(value, str): + raise ValueError(f"expected a string, got {type(value).__name__}") + if allow_unknown and value == "unknown": + return + m = _VLAN_ENCAP_RE.match(value) + if not m: + suffix = " or literal 'unknown'" if allow_unknown else "" + raise ValueError(f"expected 'vlan-N' or 'vxlan-N'{suffix}, got {value!r}") + prefix, n_str = m.group(1), m.group(2) + n = int(n_str) + lo, hi = (_VLAN_MIN, _VLAN_MAX) if prefix == "vlan" else (_VXLAN_MIN, _VXLAN_MAX) + if not (lo <= n <= hi): + raise ValueError(f"{prefix} id must be in [{lo},{hi}], got {n}") + + +def _check_ipv4_iface(value: object) -> None: + """``a.b.c.d[/prefix]`` OR the IPv6 equivalent — real APIC uses the same + attribute (``ip``/``addr``) for either family on these props. Uses the + stdlib ``ipaddress`` module (never a regex) per design §3.1/§7 point 3. + Reason text is byte-identical to the pre-F10 hardcoded fvSubnet/ + l3extSubnet ip check this replaces (design §3.2: "no behavior change, + one code path") — do not append the raw ``ipaddress`` exception text + here, existing regression tests + callers depend on the exact string.""" + if not isinstance(value, str): + raise ValueError(f"expected a string, got {type(value).__name__}") + try: + ipaddress.ip_interface(value) + except ValueError: + raise ValueError("not a valid address[/prefix]") from None + + +def _check_ip_addr(value: object) -> None: + """A bare IPv4 or IPv6 address, no ``/prefix`` (e.g. bgpPeerP.addr, + vnsRedirectDest.ip). Reason text is byte-identical to the pre-F10 + hardcoded vnsRedirectDest ip check this replaces — see + ``_check_ipv4_iface`` docstring.""" + if not isinstance(value, str): + raise ValueError(f"expected a string, got {type(value).__name__}") + try: + ipaddress.ip_address(value) + except ValueError: + raise ValueError("not a valid IP address") from None + + +def _check_ipv4_addr(value: object) -> None: + """IPv4-only dotted-quad (design table: l3extRsNodeL3OutAtt.rtrId is + explicitly "valid IPv4 dotted-quad (octet <=255)", not a dual-family + field like the iface/addr checks above).""" + if not isinstance(value, str): + raise ValueError(f"expected a string, got {type(value).__name__}") + try: + ipaddress.IPv4Address(value) + except ValueError as exc: + raise ValueError(f"not a valid IPv4 address: {exc}") from None + + +def _check_mac(value: object) -> None: + """48-bit MAC, colon form (``HH:HH:HH:HH:HH:HH``) or Cisco dot form + (``HHHH.HHHH.HHHH``) — design §7 point 4.""" + if not isinstance(value, str): + raise ValueError(f"expected a string, got {type(value).__name__}") + if _MAC_COLON_RE.match(value) or _MAC_DOT_RE.match(value): + return + raise ValueError(f"not a valid MAC address (colon or dot form), got {value!r}") + + +def _check_port(value: object) -> None: + """int in [0,65535], or one of the named ports APIC accepts.""" + if isinstance(value, str) and value in _NAMED_PORTS: + return + try: + n = int(value) # type: ignore[arg-type] + except (TypeError, ValueError): + raise ValueError( + f"not an integer or a named port {sorted(_NAMED_PORTS)}, got {value!r}" + ) from None + if not (0 <= n <= 65535): + raise ValueError(f"port must be in [0,65535], got {n}") + + +_FMT_KINDS: dict[str, Callable[..., None]] = { + "ipv4_iface": _check_ipv4_iface, + "ip_addr": _check_ip_addr, + "ipv4_addr": _check_ipv4_addr, + "mac": _check_mac, + "port": _check_port, +} + + +def fmt(kind: str, **opts: object) -> Validator: + """Format-check factory. *kind* selects the checker: + + - ``vlan_encap``: ``vlan-N`` (N in [1,4094]) or ``vxlan-N`` (N in + [1,16777215], 24-bit VNID — NOT the vlan-N ceiling, see + ``_VXLAN_MIN``/``_VXLAN_MAX``); pass ``allow_unknown=True`` for the + props whose design-table rule text explicitly says "or literal + unknown" (``fvRsPathAtt.primaryEncap``, + ``fvRsDomAtt.encap``) — every other encap prop does NOT accept + "unknown" per its own row text (design §7 point 1: "needs a + per-rule unknown-ok flag"). + - ``ipv4_iface``: address[/prefix], v4 or v6, via ``ipaddress.ip_interface``. + - ``ip_addr``: bare address, v4 or v6, via ``ipaddress.ip_address``. + - ``ipv4_addr``: v4-only dotted-quad, via ``ipaddress.IPv4Address``. + - ``mac``: 48-bit MAC, colon or dot form. + - ``port``: int in [0,65535] or a named L4 port. + """ + if kind == "vlan_encap": + allow_unknown = bool(opts.get("allow_unknown", False)) + + def _validator(cls: str, prop: str, value: object, ctx: object = None) -> None: + _check_vlan_encap(value, allow_unknown=allow_unknown) + + return _validator + + checker = _FMT_KINDS.get(kind) + if checker is None: + raise ValueError(f"unknown fmt kind {kind!r}") + + def _validator(cls: str, prop: str, value: object, ctx: object = None) -> None: + checker(value) + + return _validator + + +def mtu() -> Validator: + """Literal ``inherit`` or int in [576,9216] — l3extRsPathL3OutAtt.mtu / + fvBD.mtu (design table row 2A "mtu").""" + _range = rng(576, 9216) + + def _validator(cls: str, prop: str, value: object, ctx: object = None) -> None: + if isinstance(value, str) and value == "inherit": + return + _range(cls, prop, value, ctx) + + return _validator + + +def all_of(*validators: Validator) -> Validator: + """Compose several validators on one prop; all must pass.""" + + def _validator(cls: str, prop: str, value: object, ctx: object = None) -> None: + for v in validators: + v(cls, prop, value, ctx) + + return _validator + + +# --------------------------------------------------------------------------- +# RULES — (class, prop) -> Validator +# +# Every entry below is sourced from a row in _F10_VALIDATION_DESIGN.md §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. +# --------------------------------------------------------------------------- + +RULES: dict[tuple[str, str], Validator] = { + # -- §2A Priority 1: server-only format/range -------------------------- + # encap (vlan-N N in [1,4094] / vxlan-N N in [1,16777215] — see + # _VXLAN_MIN/_VXLAN_MAX) — "unknown" only where the row text says so. + ("l3extRsPathL3OutAtt", "encap"): fmt("vlan_encap"), + ("fvRsPathAtt", "encap"): fmt("vlan_encap"), + ("fvRsPathAtt", "primaryEncap"): fmt("vlan_encap", allow_unknown=True), + ("fvRsDomAtt", "encap"): fmt("vlan_encap", allow_unknown=True), + # fvnsEncapBlk from/to: same vlan-N format+range; from<=to cross-check is + # §2C (out of scope this batch). + ("fvnsEncapBlk", "from"): fmt("vlan_encap"), + ("fvnsEncapBlk", "to"): fmt("vlan_encap"), + # rtrId: IPv4-only dotted-quad. + ("l3extRsNodeL3OutAtt", "rtrId"): fmt("ipv4_addr"), + # l3extRsPathL3OutAtt.addr: iface form (a.b.c.d[/p]), v4 or v6. + ("l3extRsPathL3OutAtt", "addr"): fmt("ipv4_iface"), + # bgpPeerP.addr: iface form (a.b.c.d[/p]), v4 or v6 — F10 fix + # (independent review): real ACI subnet-based BGP peers legally carry a + # peer addr with an optional /prefix (e.g. "10.100.211.0/30"); the + # original bare-address-only ip_addr check 400-rejected those. Widened + # to ipv4_iface (ip_interface, same as l3extRsPathL3OutAtt.addr) which + # still accepts a bare address (defaults to /32 or /128) so the + # existing no-prefix canonical values keep passing too. + ("bgpPeerP", "addr"): fmt("ipv4_iface"), + ("bgpPeerP", "ttl"): rng(1, 255), + ("bgpAsP", "asn"): rng(1, 4294967295), + ("l3extRsPathL3OutAtt", "mtu"): mtu(), + ("fvBD", "mtu"): mtu(), + ("fvBD", "mac"): fmt("mac"), + ("vnsRedirectDest", "mac"): fmt("mac"), + # infraPortBlk: int >= 1 (no stated upper bound; le-cross-check is §2C). + ("infraPortBlk", "fromPort"): rng(1, None), + ("infraPortBlk", "toPort"): rng(1, None), + ("infraPortBlk", "fromCard"): rng(1, None), + ("infraPortBlk", "toCard"): rng(1, None), + # vzEntry ports: int in [0,65535] or a named port. + ("vzEntry", "dFromPort"): fmt("port"), + ("vzEntry", "dToPort"): fmt("port"), + ("vzEntry", "sFromPort"): fmt("port"), + ("vzEntry", "sToPort"): fmt("port"), + + # -- §2A Priority 1 (continued): ip already-validated (migrated, byte- + # identical semantics to the pre-F10 hardcoded branches in writes.py) -- + ("fvSubnet", "ip"): fmt("ipv4_iface"), + ("l3extSubnet", "ip"): fmt("ipv4_iface"), + ("vnsRedirectDest", "ip"): fmt("ip_addr"), + + # -- §2B Priority 2: server-only enums ---------------------------------- + ("fvSubnet", "scope"): one_of({"private", "public", "shared"}, list_sep=","), + ("fvSubnet", "ctrl"): one_of( + {"nd", "no-default-gateway", "querier", "unspecified"}, list_sep="," + ), + ("l3extSubnet", "scope"): one_of( + { + "import-security", + "export-rtctrl", + "import-rtctrl", + "shared-rtctrl", + "shared-security", + }, + list_sep=",", + ), + ("l3extSubnet", "aggregate"): one_of( + {"export-rtctrl", "import-rtctrl", "shared-rtctrl"}, list_sep="," + ), + ("fvCtx", "pcEnfPref"): one_of({"enforced", "unenforced"}), + ("fvCtx", "pcEnfDir"): one_of({"ingress", "egress"}), + ("fvBD", "unkMacUcastAct"): one_of({"proxy", "flood"}), + ("fvBD", "multiDstPktAct"): one_of({"bd-flood", "drop", "encap-flood"}), + ("fvBD", "unkMcastAct"): one_of({"flood", "opt-flood"}), + ("fvBD", "type"): one_of({"regular", "fc"}), + ("fvBD", "epMoveDetectMode"): one_of({"", "garp"}), + ("vzEntry", "etherT"): one_of( + { + "arp", + "fcoe", + "ip", + "ipv4", + "ipv6", + "mac_security", + "mpls_ucast", + "trill", + "unspecified", + } + ), + ("vzBrCP", "scope"): one_of({"context", "global", "tenant", "application-profile"}), + ("fvRsDomAtt", "classPref"): one_of({"encap", "useg"}), + ("fvRsDomAtt", "instrImedcy"): one_of({"immediate", "lazy"}), + # resImedcy (Resolution Immediacy) — F10 fix (independent review): real + # ACI's legal set is {immediate, lazy, pre-provision}; the original + # rule dropped "pre-provision", which is the value 100% of the + # canonical corpus's EPG-domain bindings actually use (every + # bind_epg_to_*_domain / migrate_existing_vlan* render), so the bug + # 400-rejected every one of them. Do NOT add "pre-provision" to + # instrImedcy (Deployment Immediacy) above — that enum really is just + # {immediate, lazy} per its own design-table row and 100% of the + # corpus uses "immediate". + ("fvRsDomAtt", "resImedcy"): one_of({"immediate", "lazy", "pre-provision"}), + ("bgpPeerP", "peerCtrl"): one_of( + {"bfd", "dis-conn-check", "nh-self", "allow-self-as", "send-com", "send-ext-com"}, + list_sep=",", + ), + ("fvnsVlanInstP", "allocMode"): one_of({"dynamic", "static", "inherit"}), + ("fvnsEncapBlk", "allocMode"): one_of({"dynamic", "static", "inherit"}), +} diff --git a/aci_sim/rest_aci/writes.py b/aci_sim/rest_aci/writes.py index 1263850..48fd2e4 100644 --- a/aci_sim/rest_aci/writes.py +++ b/aci_sim/rest_aci/writes.py @@ -2,7 +2,6 @@ from __future__ import annotations -import ipaddress import re from aci_sim.build.fabric import loopback_ip, oob_ip @@ -11,6 +10,7 @@ from aci_sim.build.neighbors import add_switch_adjacency from aci_sim.mit.dn import pod_from_dn from aci_sim.mit.mo import MO from aci_sim.mit.store import MITStore +from aci_sim.rest_aci.validators import RULES from aci_sim.topology.schema import Node # Class -> RN template for children POSTed without an explicit ``dn``. @@ -215,36 +215,33 @@ def _validate_node(cls: str, body: dict, path: str) -> None: 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).""" + """Reject malformed/out-of-range property values a real APIC would 400 on + (error-107/801 style) — e.g. ``fvSubnet ip=".1/24"``, + ``l3extRsPathL3OutAtt encap="vlan-9999"``, ``bgpAsP asn="0"``. 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). + + 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).""" 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 + for prop, value in mo_attrs.items(): + validator = RULES.get((mo_cls, prop)) + if validator is None or value is None: + continue # fail-safe: unregistered (class, prop) or unset value ⇒ allow + try: + validator(mo_cls, prop, value, mo_attrs) + except ValueError as reason: + raise WriteValidationError( + f"Invalid value {value!r} for property {prop!r} of {mo_cls} " + f"{mo_attrs.get('dn', '')!r}: {reason}" + ) from None def _plan_recursive(cls: str, attrs: dict, children: list, planned: list[tuple[str, dict]]) -> None: diff --git a/pyproject.toml b/pyproject.toml index 45a2271..6e5e2d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "aci-sim" -version = "0.22.0" +version = "0.23.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_f10_apic_writeface.py b/tests/test_f10_apic_writeface.py new file mode 100644 index 0000000..b6a3d65 --- /dev/null +++ b/tests/test_f10_apic_writeface.py @@ -0,0 +1,168 @@ +"""F10 — server-side value validation, end-to-end through the APIC write +face (POST /api/mo/{dn}.json -> aci_sim/rest_aci/writes.py::_validate_planned +-> aci_sim/rest_aci/validators.py::RULES). + +Complements tests/test_f10_validators.py (which exercises RULES entries +directly) by proving the FULL plumbing: a bad value 400s with the real APIC +error envelope and never lands in the store; the pre-existing 3 +fvSubnet/l3extSubnet/vnsRedirectDest ip checks keep working byte-identically +now that they're migrated into the RULES registry +(tests/test_write_validation_and_children.py already covers that regression +directly — this file adds the NEW P1/P2 rules' end-to-end wiring).""" + +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 = "F10TEST-T1" + + +@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}}}) + return c + + +def _post(client, dn: str, body: dict): + return client.post(f"/api/mo/{dn}.json", json=body) + + +def _assert_not_in_store(client, cls: str, dn: str): + q = client.get(f"/api/class/{cls}.json").json() + assert not any( + i[cls]["attributes"]["dn"] == dn for i in q["imdata"] + ), f"invalid {cls} must not land in the MIT" + + +# --------------------------------------------------------------------------- +# The exact F10 probe finding: encap=vlan-9999 / rtrId=999.888.777.x now 400. +# --------------------------------------------------------------------------- + + +def test_f10_probe_bad_encap_rejected(client): + dn = f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/lifp-IP1/rspathL3OutAtt-[topology/pod-1/paths-101/pathep-[eth1/1]]" + r = _post( + client, dn, + {"l3extRsPathL3OutAtt": {"attributes": {"encap": "vlan-9999", "dn": dn}}}, + ) + assert r.status_code == 400 + err = r.json()["imdata"][0]["error"]["attributes"] + assert "vlan-9999" in err["text"] + # app.py's post_mo handler catches WriteValidationError with code="103" + # (the design doc's "code 107" citation is for a different catch site — + # the GET-query FilterParseError handlers at get_class/get_mo). + assert err["code"] == "103" + _assert_not_in_store(client, "l3extRsPathL3OutAtt", dn) + + +def test_f10_probe_bad_rtrid_rejected(client): + dn = f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/rsnodeL3OutAtt-[topology/pod-1/node-101]" + r = _post( + client, dn, + {"l3extRsNodeL3OutAtt": {"attributes": {"rtrId": "999.888.777.241", "dn": dn}}}, + ) + assert r.status_code == 400 + err = r.json()["imdata"][0]["error"]["attributes"] + assert "999.888.777.241" in err["text"] + _assert_not_in_store(client, "l3extRsNodeL3OutAtt", dn) + + +def test_f10_valid_encap_and_rtrid_accepted(client): + dn1 = f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/lifp-IP1/rspathL3OutAtt-[topology/pod-1/paths-101/pathep-[eth1/2]]" + r = _post( + client, dn1, + {"l3extRsPathL3OutAtt": {"attributes": {"encap": "vlan-51", "mtu": "9000", "dn": dn1}}}, + ) + assert r.status_code == 200 + + dn2 = f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/rsnodeL3OutAtt-[topology/pod-1/node-102]" + r = _post( + client, dn2, + {"l3extRsNodeL3OutAtt": {"attributes": {"rtrId": "10.100.201.241", "dn": dn2}}}, + ) + assert r.status_code == 200 + + +# --------------------------------------------------------------------------- +# All-or-nothing: a bad value nested deep in a subtree must reject the WHOLE +# POST, leaving zero side effects (design §3.2 "preserves the existing +# all-or-nothing guarantee"). +# --------------------------------------------------------------------------- + + +def test_bad_nested_asn_rejects_whole_subtree(client): + peer_dn = ( + f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/lifp-IP1/" + f"paths-101/pathep-[eth1/3]]/peerP-[10.100.0.5]" + ) + body = { + "bgpPeerP": { + "attributes": {"addr": "10.100.0.5", "dn": peer_dn}, + "children": [ + {"bgpAsP": {"attributes": {"asn": "0"}}}, # illegal: asn must be >= 1 + ], + } + } + r = _post(client, peer_dn, body) + assert r.status_code == 400 + # The parent bgpPeerP must NOT have landed either — all-or-nothing. + _assert_not_in_store(client, "bgpPeerP", peer_dn) + + +def test_good_nested_asn_accepted(client): + peer_dn = ( + f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/lifp-IP1/" + f"paths-101/pathep-[eth1/4]]/peerP-[10.100.0.6]" + ) + body = { + "bgpPeerP": { + "attributes": {"addr": "10.100.0.6", "ttl": "2", "dn": peer_dn}, + "children": [ + {"bgpAsP": {"attributes": {"asn": "65100"}}}, + ], + } + } + r = _post(client, peer_dn, body) + assert r.status_code == 200 + + +# --------------------------------------------------------------------------- +# Fail-safe: an unregistered class or property is never validated, even with +# a wild value. +# --------------------------------------------------------------------------- + + +def test_unregistered_class_prop_passes_through_unvalidated(client): + dn = f"uni/tn-{TENANT}/ap-AP1" + r = _post(client, dn, {"fvAp": {"attributes": {"name": "AP1", "someUnknownProp": "!!not-validated!!", "dn": dn}}}) + assert r.status_code == 200 + + +def test_delete_skips_validation_even_with_bad_leftover_shape(client): + dn = f"uni/tn-{TENANT}/out-L3OUT1/lnodep-NP1/lifp-IP1/rspathL3OutAtt-[topology/pod-1/paths-101/pathep-[eth1/9]]" + r = _post(client, dn, {"l3extRsPathL3OutAtt": {"attributes": {"encap": "vlan-51", "dn": dn}}}) + assert r.status_code == 200 + # A delete carries only dn/status — no encap attr to (mis)validate. + r = _post(client, dn, {"l3extRsPathL3OutAtt": {"attributes": {"dn": dn, "status": "deleted"}}}) + assert r.status_code == 200 diff --git a/tests/test_f10_ndo_validation.py b/tests/test_f10_ndo_validation.py new file mode 100644 index 0000000..42cc1ab --- /dev/null +++ b/tests/test_f10_ndo_validation.py @@ -0,0 +1,279 @@ +"""F10 batch 3 — NDO value validation (aci_sim/ndo/patch.py::NDO_RULES / +_validate_ndo_op), per design doc §3.3. + +NDO stores schema JSON (not APIC MOs); values arrive as JSON-Patch op values +rather than (class, prop) MO attrs, so this is validated separately from the +APIC RULES registry (tests/test_f10_validators.py, +tests/test_f10_apic_writeface.py) even though it reuses the same ``fmt``/ +``rng`` factories. Design §3.3 scope is deliberately narrow: subnet ``ip`` +and static-port VLAN (the real cisco.mso wire field is ``portEncapVlan`` — +see NDO_RULES's own comment in patch.py for why the design doc's "vlan" +placeholder was resolved to that name). + +Both the whole-object-add shape (``.../subnets/-`` with a dict value) and +the single-field-replace shape (``.../subnets/0/ip`` with a scalar value) +are covered, since both are legal JSON-Patch forms even though only the +whole-object shape is what the real ansible-mso client sends today (per this +repo's existing PR-11/PR-12 test fixtures).""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from aci_sim.ndo.app import make_ndo_app +from aci_sim.ndo.model import build_ndo_model +from aci_sim.ndo.patch import NDO_RULES, PatchError, apply_json_patch +from aci_sim.topology.loader import load_topology + +TOPO_PATH = "topology.yaml" + + +# --------------------------------------------------------------------------- +# Unit level: apply_json_patch directly against a bare doc dict. +# --------------------------------------------------------------------------- + + +def _bd_doc_with_subnets() -> dict: + return { + "sites": [ + { + "siteId": "1", + "templateName": "LAB1", + "bds": [ + { + "bdRef": "/schemas/schema-abc/templates/LAB1/bds/bd-App1_LAB1", + "subnets": [], + } + ], + } + ] + } + + +def test_ndo_subnet_ip_valid_accepted_whole_object_add(): + doc = _bd_doc_with_subnets() + ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/-", + "value": {"ip": "192.168.41.1/24", "scope": "public"}, + } + ] + apply_json_patch(doc, ops) # must not raise + assert doc["sites"][0]["bds"][0]["subnets"] == [{"ip": "192.168.41.1/24", "scope": "public"}] + + +def test_ndo_subnet_ip_zero_route_accepted(): + """0.0.0.0/0 (external-EPG default route) is a real canonical value — + see design §6 corpus table.""" + doc = _bd_doc_with_subnets() + ops = [ + {"op": "add", "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/-", "value": {"ip": "0.0.0.0/0"}} + ] + apply_json_patch(doc, ops) + + +def test_ndo_subnet_bad_ip_rejected_whole_object_add(): + doc = _bd_doc_with_subnets() + ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/-", + "value": {"ip": "999.888.777.17/30", "scope": "public"}, + } + ] + with pytest.raises(PatchError, match="999.888.777.17/30"): + apply_json_patch(doc, ops) + # Reject must not have mutated the subnets list. + assert doc["sites"][0]["bds"][0]["subnets"] == [] + + +def test_ndo_subnet_ip_single_field_replace_shape(): + """Direct scalar replace on an EXISTING item's field + (.../subnets/0/ip) — the other JSON-Patch shape _validate_ndo_op + supports (container is a dict, last=='ip').""" + doc = _bd_doc_with_subnets() + doc["sites"][0]["bds"][0]["subnets"] = [{"ip": "192.168.41.1/24", "scope": "public"}] + ops = [{"op": "replace", "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/0/ip", "value": "192.168.41.2/24"}] + apply_json_patch(doc, ops) + assert doc["sites"][0]["bds"][0]["subnets"][0]["ip"] == "192.168.41.2/24" + + ops_bad = [{"op": "replace", "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/0/ip", "value": "999.888.777.2/24"}] + with pytest.raises(PatchError): + apply_json_patch(doc, ops_bad) + # Reject must not have overwritten the previously-good value. + assert doc["sites"][0]["bds"][0]["subnets"][0]["ip"] == "192.168.41.2/24" + + +def _epg_doc_with_static_ports() -> dict: + return { + "sites": [ + { + "siteId": "1", + "templateName": "LAB1", + "anps": [ + { + "anpRef": "/schemas/s/templates/LAB1/anps/app-Web_LAB1", + "epgs": [ + { + "epgRef": "/schemas/s/templates/LAB1/anps/app-Web_LAB1/epgs/epg-web", + "staticPorts": [], + } + ], + } + ], + } + ] + } + + +def test_ndo_static_port_vlan_valid_accepted(): + doc = _epg_doc_with_static_ports() + ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/anps/app-Web_LAB1/epgs/epg-web/staticPorts/-", + "value": { + "path": "topology/pod-1/protpaths-101-102/pathep-[ipg-vpc-LegacySW01]", + "portEncapVlan": 100, + "mode": "regular", + "deploymentImmediacy": "immediate", + "type": "vpc", + }, + } + ] + apply_json_patch(doc, ops) + static_ports = doc["sites"][0]["anps"][0]["epgs"][0]["staticPorts"] + assert len(static_ports) == 1 + assert static_ports[0]["portEncapVlan"] == 100 + + +def test_ndo_static_port_vlan_out_of_range_rejected(): + """F10-probe-shaped bad value: vlan-9999 equivalent on the NDO side.""" + doc = _epg_doc_with_static_ports() + ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/anps/app-Web_LAB1/epgs/epg-web/staticPorts/-", + "value": { + "path": "topology/pod-1/protpaths-101-102/pathep-[ipg-vpc-LegacySW01]", + "portEncapVlan": 9999, + "mode": "regular", + }, + } + ] + with pytest.raises(PatchError, match="9999"): + apply_json_patch(doc, ops) + assert doc["sites"][0]["anps"][0]["epgs"][0]["staticPorts"] == [] + + +def test_ndo_static_port_vlan_string_form_also_validated(): + """cisco.mso's own JSON payload uses a plain int (see the accept test + above) but the sim must not assume that — a numeric STRING is just as + real a wire value elsewhere in this codebase (ACI side is all-string).""" + doc = _epg_doc_with_static_ports() + ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/anps/app-Web_LAB1/epgs/epg-web/staticPorts/-", + "value": {"path": "topology/pod-1/pathep-[eth1/1]", "portEncapVlan": "0"}, + } + ] + with pytest.raises(PatchError): + apply_json_patch(doc, ops) + + +# --------------------------------------------------------------------------- +# Fail-safe: an unregistered collection/field is never validated. +# --------------------------------------------------------------------------- + + +def test_ndo_unregistered_collection_field_passes_through(): + doc = {"templates": [{"name": "LAB1", "vrfs": []}]} + ops = [ + { + "op": "add", + "path": "/templates/LAB1/vrfs/-", + "value": {"name": "vrf1", "someWildField": "!!not validated!!"}, + } + ] + apply_json_patch(doc, ops) # must not raise — ("vrfs", *) is not in NDO_RULES + assert doc["templates"][0]["vrfs"] == [{"name": "vrf1", "someWildField": "!!not validated!!"}] + + +def test_ndo_subnet_scope_field_not_validated_only_ip_is(): + """Design §3.3 scope is deliberately narrow (subnet ip + static-port + vlan only) — an arbitrary 'scope' value on a subnet must NOT be + rejected by this batch even though it's semantically wrong, since + ("subnets", "scope") has no NDO_RULES entry.""" + doc = _bd_doc_with_subnets() + ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/-", + "value": {"ip": "192.168.41.1/24", "scope": "totally-not-a-real-scope"}, + } + ] + apply_json_patch(doc, ops) # must not raise + + +def test_ndo_rules_table_scope(): + """Locks the batch-3 scope: exactly the two design-doc-named rules.""" + assert set(NDO_RULES.keys()) == {("subnets", "ip"), ("staticPorts", "portEncapVlan")} + + +# --------------------------------------------------------------------------- +# Integration level: full FastAPI PATCH /mso/api/v1/schemas/{id} round trip +# (mirrors tests/test_pr12_site_local.py's own end-to-end pattern). +# --------------------------------------------------------------------------- + + +@pytest.fixture() +def topo(): + return load_topology(TOPO_PATH) + + +@pytest.fixture() +def client(topo): + state = build_ndo_model(topo) + app = make_ndo_app(state) + with TestClient(app) as c: + yield c + + +def test_schema_patch_bad_subnet_ip_400s_end_to_end(client): + payload = { + "displayName": "F10-TEST", + "templates": [{"name": "LAB1", "displayName": "LAB1", "tenantId": "t1", "bds": []}], + "sites": [{"siteId": "1", "templateName": "LAB1"}], + } + schema_id = client.post("/mso/api/v1/schemas", json=payload).json()["id"] + + bd_ops = [ + {"op": "add", "path": "/templates/LAB1/bds/-", "value": {"name": "bd-App1_LAB1", "displayName": "bd-App1_LAB1", "subnets": []}} + ] + resp = client.patch(f"/mso/api/v1/schemas/{schema_id}", json=bd_ops) + assert resp.status_code == 200 + + bad_subnet_ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/-", + "value": {"ip": "999.888.777.17/30", "scope": "public"}, + } + ] + resp2 = client.patch(f"/mso/api/v1/schemas/{schema_id}", json=bad_subnet_ops) + assert resp2.status_code == 400 + assert "999.888.777.17/30" in resp2.json()["detail"] + + good_subnet_ops = [ + { + "op": "add", + "path": "/sites/1-LAB1/bds/bd-App1_LAB1/subnets/-", + "value": {"ip": "192.168.41.1/24", "scope": "public"}, + } + ] + resp3 = client.patch(f"/mso/api/v1/schemas/{schema_id}", json=good_subnet_ops) + assert resp3.status_code == 200 + assert resp3.json()["sites"][0]["bds"][0]["subnets"] == [{"ip": "192.168.41.1/24", "scope": "public"}] diff --git a/tests/test_f10_validators.py b/tests/test_f10_validators.py new file mode 100644 index 0000000..ee173dc --- /dev/null +++ b/tests/test_f10_validators.py @@ -0,0 +1,388 @@ +"""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 +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 +errors per rule MUST 400 (here: raise ValueError, since these are unit-level +RULES lookups — the FastAPI-level 400 plumbing is exercised separately in +tests/test_f10_apic_writeface.py and tests/test_f10_ndo_validation.py). + +This file also locks the fail-safe default-allow contract (design §3.6): an +unregistered (class, prop) pair must never be checked. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from aci_sim.rest_aci.validators import RULES, fmt, mtu, one_of, rng + +# --------------------------------------------------------------------------- +# Golden set: canonical values that must ACCEPT. (cls, prop, value) — sourced +# from the design doc's own corpus sample (§6 table) and, where noted, the +# live canonical corpus at ~/e2e-20260708/vars/{ms,sf,common}/ on PI230. +# --------------------------------------------------------------------------- +ACCEPT_CASES: list[tuple[str, str, str]] = [ + # encap (vlan-N) — real l3out_vlan/bind_epg_to_static_port/migrate_ + # existing_vlan values from ms/sf vars. + ("l3extRsPathL3OutAtt", "encap", "vlan-51"), + ("l3extRsPathL3OutAtt", "encap", "vlan-100"), + ("l3extRsPathL3OutAtt", "encap", "vlan-2501"), + ("l3extRsPathL3OutAtt", "encap", "vlan-2611"), + ("l3extRsPathL3OutAtt", "encap", "vlan-1"), + ("l3extRsPathL3OutAtt", "encap", "vlan-4094"), + ("fvRsPathAtt", "encap", "vlan-2502"), + ("fvRsPathAtt", "encap", "vlan-2503"), + ("fvRsPathAtt", "encap", "vlan-2602"), + ("fvRsPathAtt", "encap", "vlan-2603"), + ("fvRsPathAtt", "primaryEncap", "vlan-100"), + ("fvRsPathAtt", "primaryEncap", "unknown"), # edge: literal unknown allowed here + ("fvRsDomAtt", "encap", "vlan-51"), + ("fvRsDomAtt", "encap", "unknown"), # edge: literal unknown allowed here + ("fvnsEncapBlk", "from", "vlan-1"), + ("fvnsEncapBlk", "to", "vlan-4094"), + ("fvnsEncapBlk", "from", "vlan-2501"), + # vxlan-N — F10 F2 fix: NOT capped at the vlan-N 4094 ceiling (24-bit + # VNID space instead). 5000 is a legal VNID that the pre-fix shared + # ceiling would have 400-rejected. + ("l3extRsPathL3OutAtt", "encap", "vxlan-5000"), + # rtrId — IPv4-only dotted quad (l3out node profile router-id). + ("l3extRsNodeL3OutAtt", "rtrId", "10.100.201.241"), + ("l3extRsNodeL3OutAtt", "rtrId", "10.100.202.242"), + ("l3extRsNodeL3OutAtt", "rtrId", "10.100.211.243"), + ("l3extRsNodeL3OutAtt", "rtrId", "10.100.212.241"), + # l3extRsPathL3OutAtt.addr — interface form with /prefix, v4 and v6. + ("l3extRsPathL3OutAtt", "addr", "10.100.201.17/30"), + ("l3extRsPathL3OutAtt", "addr", "10.100.211.49/30"), + ("l3extRsPathL3OutAtt", "addr", "2001:db8::1/64"), # edge: IPv6 + # bgpPeerP.addr — bare address, v4 and v6. + ("bgpPeerP", "addr", "10.100.0.1"), + ("bgpPeerP", "addr", "2001:db8::1"), # edge: IPv6 + # bgpPeerP.addr with /prefix — F10 F3 fix: subnet-based BGP peers + # legally carry a /prefix; the old ip_addr (bare-only) check rejected it. + ("bgpPeerP", "addr", "10.100.211.0/30"), + ("bgpPeerP", "ttl", "1"), + ("bgpPeerP", "ttl", "255"), + ("bgpPeerP", "ttl", "2"), + ("bgpAsP", "asn", "65100"), + ("bgpAsP", "asn", "1"), + ("bgpAsP", "asn", "4294967295"), + # mtu — literal inherit or [576,9216]. + ("l3extRsPathL3OutAtt", "mtu", "9000"), + ("l3extRsPathL3OutAtt", "mtu", "9216"), + ("l3extRsPathL3OutAtt", "mtu", "1492"), + ("l3extRsPathL3OutAtt", "mtu", "576"), + ("l3extRsPathL3OutAtt", "mtu", "inherit"), # edge: literal inherit + ("fvBD", "mtu", "9000"), + ("fvBD", "mtu", "inherit"), # edge: literal inherit + # mac — colon and Cisco dot form. + ("fvBD", "mac", "00:22:BD:F8:19:FF"), # the sim's own BD create-default + ("fvBD", "mac", "00:11:22:33:44:55"), + ("fvBD", "mac", "0011.2233.4455"), # edge: dot form + ("vnsRedirectDest", "mac", "00:00:0c:9f:f7:01"), + ("vnsRedirectDest", "mac", "0000.0c9f.f701"), # edge: dot form + # infraPortBlk — int >= 1. + ("infraPortBlk", "fromPort", "1"), + ("infraPortBlk", "toPort", "48"), + ("infraPortBlk", "fromCard", "1"), + ("infraPortBlk", "toCard", "1"), + # vzEntry ports — int in [0,65535] or a named port. + ("vzEntry", "dFromPort", "443"), + ("vzEntry", "dToPort", "https"), # edge: named + ("vzEntry", "sFromPort", "0"), + ("vzEntry", "sToPort", "unspecified"), # edge: named + ("vzEntry", "dFromPort", "20"), + # ip (migrated, byte-identical semantics) — including the 0.0.0.0/0 and + # 192.168.x/24 forms from create_bd/create_tenant vars. + ("fvSubnet", "ip", "192.168.41.1/24"), + ("fvSubnet", "ip", "192.168.31.1/24"), + ("fvSubnet", "ip", "192.168.250.1/24"), + ("l3extSubnet", "ip", "0.0.0.0/0"), + ("l3extSubnet", "ip", "10.100.201.17/30"), + ("vnsRedirectDest", "ip", "10.100.201.241"), + ("vnsRedirectDest", "ip", "2001:db8::1"), # edge: IPv6 + # scope / ctrl / aggregate — single token and multi-token (comma) forms. + ("fvSubnet", "scope", "public"), + ("fvSubnet", "scope", "private,shared"), # edge: token-list + ("fvSubnet", "ctrl", "unspecified"), + ("fvSubnet", "ctrl", "nd,querier"), # edge: token-list + ("l3extSubnet", "scope", "export-rtctrl"), + ("l3extSubnet", "scope", "import-security,shared-rtctrl"), # edge: token-list + ("l3extSubnet", "aggregate", "export-rtctrl"), + ("fvCtx", "pcEnfPref", "enforced"), + ("fvCtx", "pcEnfPref", "unenforced"), + ("fvCtx", "pcEnfDir", "ingress"), + ("fvCtx", "pcEnfDir", "egress"), + ("fvBD", "unkMacUcastAct", "proxy"), + ("fvBD", "unkMacUcastAct", "flood"), + ("fvBD", "multiDstPktAct", "bd-flood"), + ("fvBD", "multiDstPktAct", "drop"), + ("fvBD", "unkMcastAct", "flood"), + ("fvBD", "unkMcastAct", "opt-flood"), + ("fvBD", "type", "regular"), + ("fvBD", "epMoveDetectMode", ""), # edge: empty-string literal member + ("fvBD", "epMoveDetectMode", "garp"), + ("vzEntry", "etherT", "ip"), + ("vzEntry", "etherT", "unspecified"), + ("vzBrCP", "scope", "context"), + ("vzBrCP", "scope", "application-profile"), + ("fvRsDomAtt", "classPref", "encap"), + ("fvRsDomAtt", "instrImedcy", "immediate"), + ("fvRsDomAtt", "resImedcy", "lazy"), + # resImedcy "pre-provision" — F10 F1 fix (BLOCKER): the canonical + # corpus's ACTUAL resolution_immediacy value (34/34 in the campaign + # that motivated this batch); the pre-fix enum {immediate, lazy} 400- + # rejected every single EPG-domain binding. + ("fvRsDomAtt", "resImedcy", "pre-provision"), + ("bgpPeerP", "peerCtrl", "bfd"), + ("bgpPeerP", "peerCtrl", "bfd,nh-self"), # edge: token-list + ("fvnsVlanInstP", "allocMode", "dynamic"), + ("fvnsEncapBlk", "allocMode", "static"), +] + + +@pytest.mark.parametrize("cls,prop,value", ACCEPT_CASES, ids=[f"{c}.{p}={v!r}" for c, p, v in ACCEPT_CASES]) +def test_canonical_value_accepted(cls, prop, value): + validator = RULES[(cls, prop)] + validator(cls, prop, value, {}) # must NOT raise + + +def test_canonical_corpus_sample_size(): + """Design doc §6 samples ~40 distinct canonical values; this suite + covers at least that many (cls, prop, value) accept-cases.""" + assert len(ACCEPT_CASES) >= 40 + + +# --------------------------------------------------------------------------- +# Bad values that must REJECT — the F10 probe values plus 1-2 hand-built +# out-of-range/format errors per rule family. +# --------------------------------------------------------------------------- +REJECT_CASES: list[tuple[str, str, str]] = [ + # F10 probe values (the actual empirically-confirmed gap). + ("l3extRsPathL3OutAtt", "encap", "vlan-9999"), + ("l3extRsNodeL3OutAtt", "rtrId", "999.888.777.241"), + ("l3extRsPathL3OutAtt", "addr", "999.888.777.17/30"), + ("l3extRsPathL3OutAtt", "addr", "999.888.777.25/30"), + # encap: out of range / malformed. + ("l3extRsPathL3OutAtt", "encap", "vlan-0"), + ("l3extRsPathL3OutAtt", "encap", "vlan-4095"), + ("l3extRsPathL3OutAtt", "encap", "garbage"), + ("fvRsPathAtt", "encap", "unknown"), # NOT allowed for this specific prop + ("fvRsDomAtt", "encap", "vlan-9999"), + ("fvnsEncapBlk", "from", "vlan-4095"), + ("fvnsEncapBlk", "to", "vlan-0"), + # vxlan-N: still must reject beyond the 24-bit VNID ceiling (F10 F2 + # regression guard — widening vlan-N's cap must not become "no cap"). + ("l3extRsPathL3OutAtt", "encap", "vxlan-99999999"), + ("l3extRsPathL3OutAtt", "encap", "vxlan-0"), + # rtrId: IPv6 not allowed (row says IPv4-only dotted-quad). + ("l3extRsNodeL3OutAtt", "rtrId", "2001:db8::1"), + ("l3extRsNodeL3OutAtt", "rtrId", "not-an-ip"), + # bgpPeerP. + ("bgpPeerP", "addr", "999.888.777.1"), + ("bgpPeerP", "addr", "999.888.777.5"), # F10 F3 regression guard: /prefix now + # tolerated but a garbage octet must still 400 (ip_interface catches it too). + ("bgpPeerP", "addr", "999.888.777.5/30"), # same, with a /prefix + ("bgpPeerP", "ttl", "0"), + ("bgpPeerP", "ttl", "256"), + ("bgpPeerP", "peerCtrl", "not-a-flag"), + # bgpAsP. + ("bgpAsP", "asn", "0"), + ("bgpAsP", "asn", "4294967296"), + # mtu. + ("l3extRsPathL3OutAtt", "mtu", "99999"), + ("l3extRsPathL3OutAtt", "mtu", "500"), + ("fvBD", "mtu", "not-a-number"), + # mac. + ("fvBD", "mac", "zz:11:22:33:44:55"), + ("fvBD", "mac", "00:11:22:33:44"), + ("vnsRedirectDest", "mac", "not-a-mac"), + # infraPortBlk. + ("infraPortBlk", "fromPort", "0"), + ("infraPortBlk", "toCard", "-1"), + # vzEntry ports. + ("vzEntry", "dFromPort", "70000"), + ("vzEntry", "sToPort", "httpz"), + # enums. + ("fvSubnet", "scope", "publicc"), + ("fvSubnet", "scope", "private,publicc"), + ("fvSubnet", "ctrl", "bogus"), + ("l3extSubnet", "scope", "bogus-scope"), + ("l3extSubnet", "aggregate", "bogus-agg"), + ("fvCtx", "pcEnfPref", "bogus"), + ("fvCtx", "pcEnfDir", "bogus"), + ("fvBD", "unkMacUcastAct", "bogus"), + ("fvBD", "multiDstPktAct", "bogus"), + ("fvBD", "type", "weird"), + ("fvBD", "epMoveDetectMode", "bogus"), + ("vzEntry", "etherT", "ipv7"), + ("vzBrCP", "scope", "bogus"), + ("fvRsDomAtt", "classPref", "bogus"), + ("fvRsDomAtt", "instrImedcy", "bogus"), + ("fvnsVlanInstP", "allocMode", "weird"), + # ip (migrated) regression — the exact garbage from the 2026-07-07 J2 + # incident that motivated the pre-F10 hardcoded checks. + ("fvSubnet", "ip", ".1/24"), + ("l3extSubnet", "ip", ".1/24"), + ("vnsRedirectDest", "ip", "."), +] + + +@pytest.mark.parametrize("cls,prop,value", REJECT_CASES, ids=[f"{c}.{p}={v!r}" for c, p, v in REJECT_CASES]) +def test_bad_value_rejected(cls, prop, value): + validator = RULES[(cls, prop)] + with pytest.raises(ValueError): + validator(cls, prop, value, {}) + + +# --------------------------------------------------------------------------- +# Corpus enumeration lock (independent-review test-blind-spot fix, F10 F1 +# post-mortem): ACCEPT_CASES above hand-picks ONE value per enum prop (e.g. +# resImedcy="lazy"), which is exactly how F1 slipped past review — "lazy" is +# legal but is NOT what the real campaign ever sends (100% "pre-provision"). +# A hand-picked accept-case proves the validator accepts *a* legal value, not +# that it accepts the corpus's *actual* values. This test closes that gap +# mechanically: it scans every resolution_immediacy/deployment_immediacy +# value that the real campaign's vars (~/e2e-20260708/vars/{ms,sf,common}/) +# actually rendered into (the rendered artifacts live at +# ~/e2e-20260708/logs/ansible/*.yml — same corpus this module's docstring +# already cites) and asserts EVERY distinct value found is ACCEPTed, with no +# human hand-picking a subset. Skipped (not failed) off PI230 where this +# machine-local e2e corpus doesn't exist. +# --------------------------------------------------------------------------- + +_E2E_ANSIBLE_LOGS = Path.home() / "e2e-20260708" / "logs" / "ansible" +_IMMEDIACY_RE = re.compile(r'(resolution_immediacy|deployment_immediacy):\s*"([^"]+)"') + + +def _enumerate_corpus_immediacy_values() -> dict[str, set[str]]: + """Mechanically extract every distinct resolution_immediacy / + deployment_immediacy string literal from the rendered aci-model-*.yml / + mso-model-*.yml artifacts under ~/e2e-20260708/logs/ansible/ — these are + exactly what results from rendering ~/e2e-20260708/vars/{ms,sf,common}/ + through the campaign's pipeline. Values are read off disk, not typed in + by hand, so a future enum with a real-corpus value this suite doesn't + already know about will surface here instead of staying invisible.""" + found: dict[str, set[str]] = {"resolution_immediacy": set(), "deployment_immediacy": set()} + for path in sorted(_E2E_ANSIBLE_LOGS.glob("*.yml")): + text = path.read_text() + for key, value in _IMMEDIACY_RE.findall(text): + found[key].add(value) + return found + + +@pytest.mark.skipif( + not _E2E_ANSIBLE_LOGS.is_dir(), + reason="~/e2e-20260708/logs/ansible corpus not present on this machine", +) +def test_corpus_resolution_and_deployment_immediacy_values_all_accepted(): + found = _enumerate_corpus_immediacy_values() + assert found["resolution_immediacy"], ( + "corpus scan found zero resolution_immediacy values — the glob/regex " + "is broken, this is not a real pass" + ) + res_imedcy = RULES[("fvRsDomAtt", "resImedcy")] + for value in sorted(found["resolution_immediacy"]): + res_imedcy("fvRsDomAtt", "resImedcy", value, {}) # must NOT raise + + assert found["deployment_immediacy"], ( + "corpus scan found zero deployment_immediacy values — the glob/regex " + "is broken, this is not a real pass" + ) + instr_imedcy = RULES[("fvRsDomAtt", "instrImedcy")] + for value in sorted(found["deployment_immediacy"]): + instr_imedcy("fvRsDomAtt", "instrImedcy", value, {}) # must NOT raise + + +def test_corpus_resolution_immediacy_is_actually_pre_provision(): + """Pins the specific fact that motivated the F1 fix (not just "some + value accepts") — if this ever flips, F1's own justification is stale + and needs re-checking, not silently trusted.""" + found = _enumerate_corpus_immediacy_values() + if not found["resolution_immediacy"]: + pytest.skip("~/e2e-20260708/logs/ansible corpus not present on this machine") + assert found["resolution_immediacy"] == {"pre-provision"} + + +# --------------------------------------------------------------------------- +# Fail-safe / default-allow contract (design §3.6). +# --------------------------------------------------------------------------- + + +def test_unregistered_class_prop_pair_has_no_validator(): + assert ("fvTenant", "name") not in RULES + assert ("fvAp", "name") not in RULES + assert ("bogusClass", "bogusProp") not in RULES + + +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).""" + assert 35 <= len(RULES) <= 55 + + +# --------------------------------------------------------------------------- +# Direct factory-level tests (not routed through RULES) — pin the exact +# behavior of the composable helpers themselves. +# --------------------------------------------------------------------------- + + +def test_rng_no_upper_bound(): + v = rng(1, None) + v("infraPortBlk", "fromPort", "1", {}) + v("infraPortBlk", "fromPort", "999999", {}) # no ceiling specified + with pytest.raises(ValueError): + v("infraPortBlk", "fromPort", "0", {}) + + +def test_rng_rejects_non_integer(): + v = rng(1, 10) + with pytest.raises(ValueError): + v("x", "y", "not-a-number", {}) + + +def test_one_of_plain_membership(): + v = one_of({"a", "b"}) + v("x", "y", "a", {}) + with pytest.raises(ValueError): + v("x", "y", "c", {}) + + +def test_one_of_token_list_empty_value_is_vacuously_allowed(): + """An empty string with list_sep set has no tokens to check — the + fail-safe bias (design §3.6) treats 'nothing to validate' as pass rather + than risk a false-reject on an omitted/unset multi-valued enum.""" + v = one_of({"a", "b"}, list_sep=",") + v("x", "y", "", {}) # must not raise + + +def test_fmt_unknown_kind_raises_at_construction(): + with pytest.raises(ValueError): + fmt("not-a-real-kind") + + +def test_mtu_helper_directly(): + m = mtu() + m("fvBD", "mtu", "inherit", {}) + m("fvBD", "mtu", "9216", {}) + with pytest.raises(ValueError): + m("fvBD", "mtu", "9217", {}) + with pytest.raises(ValueError): + m("fvBD", "mtu", "not-inherit-and-not-a-number", {}) + + +def test_vlan_encap_unknown_only_where_flagged(): + strict = fmt("vlan_encap") + lenient = fmt("vlan_encap", allow_unknown=True) + with pytest.raises(ValueError): + strict("x", "y", "unknown", {}) + lenient("x", "y", "unknown", {}) # must not raise