mirror of
https://github.com/dtzp555-max/aci-sim.git
synced 2026-07-19 09:46:33 +00:00
fix(ndo): tenant-delete guard also scans tenant policy templates (F5b) (v0.26.0)
The in-use guard added in 0.22.0 (F5) only scanned SCHEMA templates'
tenantId, so a tenant referenced only by a tenant policy template
(tenant_policy_templates[*].tenantPolicyTemplate.template.tenantId — the
same field path _get_template_objects/_backfill_policy_uuids already
traverse) could be deleted, leaving a dangling reference. Real NDO refuses
to delete an in-use tenant regardless of which template type holds the
reference.
DELETE /mso/api/v1/tenants/{id} (and the bare /api/v1 twin) now scans
both. A policy-template-only reference returns 400 "Tenant '<id>' is
referenced by tenant policy template(s): <names> — delete those templates
first"; the wording is a format-mirrored approximation of the
hardware-grounded schema-template message (no real-NDO capture of this
specific rejection text exists yet — the code comment says so). The
schema-template message is byte-for-byte unchanged. Deleting the
referencing template still releases the guard. Malformed policy-template
entries default-allow (fail-safe, consistent with the sim's philosophy).
Verification: full pytest 1147 passed (1143 baseline + 4 new TDD tests,
red-phase confirmed pre-fix; all 8 pre-existing F5 tests intact);
independent opus review APPROVE (over-rejection trace: different-tenant
policy template still deletes clean; release-test proven non-vacuous);
live golden gate on restarted sim — create tenant -> create referencing
policy template -> DELETE 400 golden -> delete template -> DELETE 200.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zdUTqU9fvCCvF3uVGZsu1
This commit is contained in:
dtzp555-max
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
c2959ab2cb
commit
4ca201085b
@@ -6,6 +6,23 @@ 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.26.0] - 2026-07-11
|
||||
|
||||
### Fixed
|
||||
- **NDO tenant-delete guard now covers tenant policy templates (F5b)** — the
|
||||
in-use guard added in 0.22.0 only scanned SCHEMA templates' `tenantId`, so a
|
||||
tenant referenced only by a tenant policy template
|
||||
(`tenant_policy_templates[*].tenantPolicyTemplate.template.tenantId`) could
|
||||
be deleted, leaving a dangling reference (fails-open). `DELETE
|
||||
/mso/api/v1/tenants/{id}` now scans both: a tenant referenced by a policy
|
||||
template returns `400 "Tenant '<id>' is referenced by tenant policy
|
||||
template(s): <names> — delete those templates first"` (wording is a
|
||||
format-mirrored approximation of the hardware-grounded schema-template
|
||||
message — no real-NDO capture of this specific rejection text exists yet,
|
||||
and the code comment says so). The schema-template message is byte-for-byte
|
||||
unchanged; deleting the referencing template still releases the guard;
|
||||
malformed policy-template entries default-allow (fail-safe). +4 tests.
|
||||
|
||||
## [0.25.0] - 2026-07-10
|
||||
|
||||
### Changed
|
||||
|
||||
+49
-8
@@ -180,6 +180,21 @@ def make_ndo_app(state: NdoState, apic_states: dict[str, Any] | None = None) ->
|
||||
# GET /mso/api/v1/tenants ↔ GET /api/v1/tenants dual-shape pair above
|
||||
# (`delegate_to: localhost` cisco.mso tasks skip the /mso prefix —
|
||||
# see the PR-13 templates-store comment below).
|
||||
#
|
||||
# F5b: the F5 guard above only ever scanned SCHEMA templates — a
|
||||
# tenant referenced ONLY by a tenant policy template
|
||||
# (`tenant_policy_templates[*].tenantPolicyTemplate.template.
|
||||
# tenantId` — same field path `_get_template_objects`/
|
||||
# `_backfill_policy_uuids` already traverse above) deleted cleanly,
|
||||
# fails-open vs. real NDO which refuses regardless of which template
|
||||
# type holds the reference. Extend the scan to cover both. The
|
||||
# policy-template branch's exact wording is an approximation mirrored
|
||||
# from the F5 schema-template message's format/shape — no real-NDO
|
||||
# hardware capture of THIS specific rejection text exists yet, unlike
|
||||
# the schema-template wording above (see F5's commit evidence). A
|
||||
# malformed policy-template entry (non-dict `tenantPolicyTemplate`/
|
||||
# `template`) default-allows rather than crashing the guard, matching
|
||||
# this sim's fail-safe philosophy elsewhere.
|
||||
@app.delete("/mso/api/v1/tenants/{tenant_id}")
|
||||
@app.delete("/api/v1/tenants/{tenant_id}")
|
||||
async def delete_tenant(tenant_id: str):
|
||||
@@ -188,7 +203,7 @@ def make_ndo_app(state: NdoState, apic_states: dict[str, Any] | None = None) ->
|
||||
raise HTTPException(
|
||||
status_code=404, detail=f"Tenant '{tenant_id}' not found"
|
||||
)
|
||||
referencing = sorted(
|
||||
referencing_schemas = sorted(
|
||||
{
|
||||
detail.get("displayName") or detail.get("name") or detail["id"]
|
||||
for detail in state.schema_details.values()
|
||||
@@ -196,14 +211,40 @@ def make_ndo_app(state: NdoState, apic_states: dict[str, Any] | None = None) ->
|
||||
if isinstance(tmpl, dict) and tmpl.get("tenantId") == tenant_id
|
||||
}
|
||||
)
|
||||
if referencing:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
f"Tenant '{tenant_id}' is referenced by schema(s): "
|
||||
f"{', '.join(referencing)} — delete those schemas first"
|
||||
),
|
||||
referencing_policy_templates = sorted(
|
||||
{
|
||||
doc.get("displayName") or key
|
||||
for key, doc in state.tenant_policy_templates.items()
|
||||
if isinstance(doc, dict)
|
||||
and isinstance(doc.get("tenantPolicyTemplate"), dict)
|
||||
and isinstance(
|
||||
doc["tenantPolicyTemplate"].get("template"), dict
|
||||
)
|
||||
and doc["tenantPolicyTemplate"]["template"].get("tenantId")
|
||||
== tenant_id
|
||||
}
|
||||
)
|
||||
if referencing_schemas and referencing_policy_templates:
|
||||
detail_msg = (
|
||||
f"Tenant '{tenant_id}' is referenced by schema(s): "
|
||||
f"{', '.join(referencing_schemas)} — delete those schemas "
|
||||
f"first; and tenant policy template(s): "
|
||||
f"{', '.join(referencing_policy_templates)} — delete those "
|
||||
f"templates first"
|
||||
)
|
||||
elif referencing_schemas:
|
||||
detail_msg = (
|
||||
f"Tenant '{tenant_id}' is referenced by schema(s): "
|
||||
f"{', '.join(referencing_schemas)} — delete those schemas first"
|
||||
)
|
||||
elif referencing_policy_templates:
|
||||
detail_msg = (
|
||||
f"Tenant '{tenant_id}' is referenced by tenant policy "
|
||||
f"template(s): {', '.join(referencing_policy_templates)} — "
|
||||
f"delete those templates first"
|
||||
)
|
||||
if referencing_schemas or referencing_policy_templates:
|
||||
raise HTTPException(status_code=400, detail=detail_msg)
|
||||
state.tenants.remove(tenant)
|
||||
return {}
|
||||
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "aci-sim"
|
||||
version = "0.25.0"
|
||||
version = "0.26.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"
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
"""F5b — NDO tenant-delete in-use guard: tenant policy templates.
|
||||
|
||||
F5 (`ffaf996`, v0.22.0) added the tenant-delete in-use guard, but it only
|
||||
scanned SCHEMA templates' `tenantId` for references to the tenant being
|
||||
deleted (`state.schema_details[*].templates[*].tenantId`). It never
|
||||
scanned tenant policy templates
|
||||
(`state.tenant_policy_templates[*].tenantPolicyTemplate.template.
|
||||
tenantId` — confirmed as the real field path against the sim's own
|
||||
`_get_template_objects`/`_backfill_policy_uuids` traversal of the same
|
||||
structure in aci_sim/ndo/app.py). So a tenant referenced ONLY by a tenant
|
||||
policy template deleted cleanly — fails-open. Real NDO refuses to delete
|
||||
an in-use tenant regardless of which template type holds the reference;
|
||||
this fixes the gap so the same 400 in-use rejection applies.
|
||||
|
||||
Semantics under test:
|
||||
- Tenant referenced ONLY by a tenant policy template -> 400 (the fix).
|
||||
- Tenant referenced by nothing -> still 200 (guard doesn't over-reject).
|
||||
- Tenant referenced by a schema template -> still 400 (F5 guard intact).
|
||||
- Referencing policy template deleted, then tenant delete -> 200 (guard
|
||||
releases once dereferenced).
|
||||
"""
|
||||
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 NdoState
|
||||
|
||||
POLICY_TENANT_ID = "tenant-f5b-policy-1"
|
||||
POLICY_TENANT_NAME = "ANS-POLICY-TN1"
|
||||
POLICY_TEMPLATE_ID = "template-f5b-policy-1"
|
||||
POLICY_TEMPLATE_NAME = "POLICY-TN1-template"
|
||||
|
||||
SCHEMA_TENANT_ID = "tenant-f5b-schema-1"
|
||||
SCHEMA_TENANT_NAME = "ANS-SCHEMA-TN1"
|
||||
SCHEMA_ID = "schema-f5b-1"
|
||||
SCHEMA_NAME = "F5B-SCHEMA-TN1-LAB1"
|
||||
SCHEMA_TEMPLATE_NAME = "F5B-SCHEMA-TN1-template"
|
||||
|
||||
SCRATCH_TENANT_ID = "tenant-f5b-scratch-1"
|
||||
SCRATCH_TENANT_NAME = "ANS-SCRATCH-TN1"
|
||||
|
||||
|
||||
def _build_state() -> NdoState:
|
||||
"""Three tenants, exercising the three reference sources independently:
|
||||
- POLICY_TENANT_ID: referenced ONLY by a tenant policy template.
|
||||
- SCHEMA_TENANT_ID: referenced ONLY by a schema template (F5 baseline).
|
||||
- SCRATCH_TENANT_ID: referenced by nothing.
|
||||
"""
|
||||
policy_doc = {
|
||||
"templateId": POLICY_TEMPLATE_ID,
|
||||
"displayName": POLICY_TEMPLATE_NAME,
|
||||
"templateType": "tenantPolicy",
|
||||
"tenantPolicyTemplate": {
|
||||
"template": {
|
||||
"tenantId": POLICY_TENANT_ID,
|
||||
"dhcpRelayPolicies": [],
|
||||
"dhcpOptionPolicies": [],
|
||||
},
|
||||
"sites": [],
|
||||
},
|
||||
}
|
||||
schema_template = {
|
||||
"name": SCHEMA_TEMPLATE_NAME,
|
||||
"tenantId": SCHEMA_TENANT_ID,
|
||||
"vrfs": [],
|
||||
"bds": [],
|
||||
"anps": [],
|
||||
"contracts": [],
|
||||
}
|
||||
schema_detail = {
|
||||
"id": SCHEMA_ID,
|
||||
"displayName": SCHEMA_NAME,
|
||||
"name": SCHEMA_NAME,
|
||||
"templates": [schema_template],
|
||||
"sites": [],
|
||||
}
|
||||
schema_summary = {
|
||||
"id": SCHEMA_ID,
|
||||
"displayName": SCHEMA_NAME,
|
||||
"name": SCHEMA_NAME,
|
||||
"templates": [{"name": SCHEMA_TEMPLATE_NAME}],
|
||||
}
|
||||
return NdoState(
|
||||
sites=[],
|
||||
tenants=[
|
||||
{
|
||||
"id": POLICY_TENANT_ID,
|
||||
"name": POLICY_TENANT_NAME,
|
||||
"displayName": POLICY_TENANT_NAME,
|
||||
},
|
||||
{
|
||||
"id": SCHEMA_TENANT_ID,
|
||||
"name": SCHEMA_TENANT_NAME,
|
||||
"displayName": SCHEMA_TENANT_NAME,
|
||||
},
|
||||
{
|
||||
"id": SCRATCH_TENANT_ID,
|
||||
"name": SCRATCH_TENANT_NAME,
|
||||
"displayName": SCRATCH_TENANT_NAME,
|
||||
},
|
||||
],
|
||||
schemas=[schema_summary],
|
||||
schema_details={SCHEMA_ID: schema_detail},
|
||||
template_summaries=[],
|
||||
tenant_policy_templates={POLICY_TEMPLATE_ID: policy_doc},
|
||||
fabric_connectivity={},
|
||||
policy_states={},
|
||||
audit_records=[],
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def state():
|
||||
return _build_state()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client(state):
|
||||
app = make_ndo_app(state)
|
||||
with TestClient(app) as c:
|
||||
yield c
|
||||
|
||||
|
||||
def test_delete_tenant_referenced_only_by_policy_template_400(client):
|
||||
"""The fix: a tenant referenced ONLY by a tenant policy template's
|
||||
`tenantPolicyTemplate.template.tenantId` must be refused, matching the
|
||||
F5 schema-template guard's semantics (previously fails-open)."""
|
||||
resp = client.delete(f"/mso/api/v1/tenants/{POLICY_TENANT_ID}")
|
||||
assert resp.status_code == 400
|
||||
detail = resp.json()["detail"]
|
||||
assert "tenant policy template(s)" in detail
|
||||
assert POLICY_TEMPLATE_NAME in detail
|
||||
|
||||
tenants = client.get("/mso/api/v1/tenants").json()["tenants"]
|
||||
assert any(t["id"] == POLICY_TENANT_ID for t in tenants)
|
||||
|
||||
|
||||
def test_delete_tenant_no_references_200(client):
|
||||
"""Guard must not over-reject: a tenant referenced by neither a schema
|
||||
template nor a tenant policy template deletes cleanly."""
|
||||
resp = client.delete(f"/mso/api/v1/tenants/{SCRATCH_TENANT_ID}")
|
||||
assert resp.status_code == 200
|
||||
|
||||
tenants = client.get("/mso/api/v1/tenants").json()["tenants"]
|
||||
assert all(t["id"] != SCRATCH_TENANT_ID for t in tenants)
|
||||
|
||||
|
||||
def test_delete_tenant_referenced_by_schema_still_400(client):
|
||||
"""F5 guard intact: schema-template reference still refuses deletion."""
|
||||
resp = client.delete(f"/mso/api/v1/tenants/{SCHEMA_TENANT_ID}")
|
||||
assert resp.status_code == 400
|
||||
detail = resp.json()["detail"]
|
||||
assert "schema(s)" in detail
|
||||
assert SCHEMA_NAME in detail
|
||||
|
||||
tenants = client.get("/mso/api/v1/tenants").json()["tenants"]
|
||||
assert any(t["id"] == SCHEMA_TENANT_ID for t in tenants)
|
||||
|
||||
|
||||
def test_delete_tenant_after_policy_template_removed_200(client):
|
||||
"""Guard releases once dereferenced: delete the referencing tenant
|
||||
policy template first, then the tenant delete succeeds."""
|
||||
assert (
|
||||
client.delete(f"/mso/api/v1/templates/{POLICY_TEMPLATE_ID}").status_code
|
||||
== 200
|
||||
)
|
||||
|
||||
resp = client.delete(f"/mso/api/v1/tenants/{POLICY_TENANT_ID}")
|
||||
assert resp.status_code == 200
|
||||
|
||||
tenants = client.get("/mso/api/v1/tenants").json()["tenants"]
|
||||
assert all(t["id"] != POLICY_TENANT_ID for t in tenants)
|
||||
Reference in New Issue
Block a user