From d651a11472260b2c6f112c25042eb97f510b5737 Mon Sep 17 00:00:00 2001 From: nessi Date: Thu, 9 Jul 2026 21:31:20 +0200 Subject: [PATCH] feat: add automatic policy rule cleanup when disabling or switching to audit mode with provider-level rule deletion Add cluster_provider_targets helper to build target list from all cluster workloads, implement cleanup_policy_provider_rules to delete policy rules across all clusters with per-cluster result tracking, add delete_policy_rules method to ProxmoxProvider that removes rules matching policy ID marker with error collection, extend policy_id_marker and rule_comment_matches_marker helpers for --- backend/app/api/v1/router.py | 67 +++++++++++++++++++++-- backend/app/services/providers/proxmox.py | 38 ++++++++++++- backend/tests/test_proxmox_provider.py | 20 +++++++ 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/backend/app/api/v1/router.py b/backend/app/api/v1/router.py index 7ec6046..3f5f670 100644 --- a/backend/app/api/v1/router.py +++ b/backend/app/api/v1/router.py @@ -853,6 +853,45 @@ def resolve_firewall_preview(db: Session, cluster: Cluster, preview: FirewallPre ) +def cluster_provider_targets(db: Session, cluster: Cluster) -> list[dict]: + targets = [] + workloads = db.scalars(select(Workload).where(Workload.cluster_id == cluster.id).order_by(Workload.name)).all() + for workload in workloads: + resolved = workload_provider_target(db, cluster, f"workload:{workload.id}") + if resolved: + target, _ = resolved + targets.append(target) + return targets + + +async def cleanup_policy_provider_rules(db: Session, policy_id: str) -> dict[str, object]: + results = [] + total_deleted = 0 + for cluster in db.scalars(select(Cluster).order_by(Cluster.name)).all(): + targets = cluster_provider_targets(db, cluster) + if not targets: + results.append({"cluster_id": cluster.id, "cluster_name": cluster.name, "rules_deleted": 0, "reason": "No targets"}) + continue + provider = get_provider(cluster.provider) + delete_rules = getattr(provider, "delete_policy_rules", None) + if not delete_rules: + results.append({"cluster_id": cluster.id, "cluster_name": cluster.name, "rules_deleted": 0, "reason": "Provider cannot delete policy rules"}) + continue + result = await delete_rules( + ProviderConnection( + api_url=cluster.api_url, + token=cluster.token_ref or "", + verify_tls=cluster.verify_tls, + read_only=cluster.mode == "read_only", + ), + targets, + policy_id, + ) + total_deleted += int(result.get("rules_deleted") or 0) + results.append({"cluster_id": cluster.id, "cluster_name": cluster.name, **result}) + return {"policy_id": policy_id, "rules_deleted": total_deleted, "clusters": results} + + @api_router.get("/setup/status", response_model=SetupStatus) def setup_status(db: Session = Depends(get_db)) -> SetupStatus: setting = setup_setting(db) @@ -1820,31 +1859,47 @@ def create_policy(payload: PolicyCreate, user: CurrentUser, db: Session = Depend @api_router.patch("/policies/{policy_id}", response_model=PolicyRead) -def update_policy(policy_id: str, payload: PolicyCreate, user: CurrentUser, db: Session = Depends(get_db)) -> Policy: +async def update_policy(policy_id: str, payload: PolicyCreate, user: CurrentUser, db: Session = Depends(get_db)) -> Policy: policy = db.get(Policy, policy_id) if not policy: raise HTTPException(status_code=404, detail="Policy not found") old_values = {"name": policy.name, "enabled": policy.enabled, "definition": policy.definition, "version": policy.version} + old_mode = policy.enforcement_mode + old_enabled = policy.enabled + new_definition = normalized_policy_definition(payload.definition) + new_mode = str(new_definition.get("enforcement_mode") or "enforced") policy.project_id = payload.project_id policy.name = payload.name policy.enabled = payload.enabled - policy.definition = normalized_policy_definition(payload.definition) + policy.definition = new_definition policy.version += 1 commit_or_400(db) db.refresh(policy) - write_audit(db, action="policy.updated", object_type="policy", object_id=policy.id, user_id=user.id, old_values=old_values, new_values=payload.model_dump()) + cleanup_result = None + if old_enabled and old_mode == "enforced" and (not payload.enabled or new_mode == "audit"): + cleanup_result = await cleanup_policy_provider_rules(db, policy.id) + write_audit( + db, + action="policy.updated", + object_type="policy", + object_id=policy.id, + user_id=user.id, + old_values=old_values, + new_values={**payload.model_dump(), "provider_cleanup": cleanup_result}, + ) return policy @api_router.delete("/policies/{policy_id}") -def delete_policy(policy_id: str, user: CurrentUser, db: Session = Depends(get_db)) -> dict[str, str]: +async def delete_policy(policy_id: str, user: CurrentUser, db: Session = Depends(get_db)) -> dict[str, object]: policy = db.get(Policy, policy_id) if not policy: raise HTTPException(status_code=404, detail="Policy not found") + cleanup_result = await cleanup_policy_provider_rules(db, policy.id) db.delete(policy) commit_or_400(db) - write_audit(db, action="policy.deleted", object_type="policy", object_id=policy_id, user_id=user.id) - return {"status": "deleted", "id": policy_id} + write_audit(db, action="policy.deleted", object_type="policy", object_id=policy_id, user_id=user.id, new_values={"provider_cleanup": cleanup_result}) + return {"status": "deleted", "id": policy_id, "provider_cleanup": cleanup_result} @api_router.post("/policies/{policy_id}/compile", response_model=PolicyRead) diff --git a/backend/app/services/providers/proxmox.py b/backend/app/services/providers/proxmox.py index 87e6b33..fcd62d7 100644 --- a/backend/app/services/providers/proxmox.py +++ b/backend/app/services/providers/proxmox.py @@ -162,6 +162,12 @@ class ProxmoxProvider(Provider): def policy_marker(self, rule: dict[str, Any]) -> str: return f"NexaFabric policy={rule.get('policy_id')}" + def policy_id_marker(self, policy_id: str) -> str: + return f"NexaFabric policy={policy_id}" + + def rule_comment_matches_marker(self, comment: str, marker: str) -> bool: + return comment == marker or comment.startswith(f"{marker} ") + def network_firewall_enabled_value(self, value: str) -> str: parts = [part for part in value.split(",") if part] found = False @@ -190,7 +196,7 @@ class ProxmoxProvider(Provider): for existing_rule in sorted(existing_rules, key=lambda item: int(item.get("pos", 0)), reverse=True): comment = str(existing_rule.get("comment") or "") pos = existing_rule.get("pos") - if marker in comment and pos is not None: + if self.rule_comment_matches_marker(comment, marker) and pos is not None: delete_response = await client.delete(f"{rules_url}/{pos}", headers=headers) delete_response.raise_for_status() deletions.append({"pos": pos, "comment": comment}) @@ -350,3 +356,33 @@ class ProxmoxProvider(Provider): "audit_only": audit_only_rules, "rules": applied_rules, } + + async def delete_policy_rules( + self, + connection: ProviderConnection, + targets: list[dict[str, Any]], + policy_id: str, + ) -> dict[str, Any]: + if connection.read_only: + return {"applied": False, "reason": "Cluster is read-only", "rules_deleted": 0, "targets": targets} + + headers = {"Authorization": self.auth_header(connection.token)} + marker = self.policy_id_marker(policy_id) + deleted_rules = [] + errors = [] + async with httpx.AsyncClient(verify=connection.verify_tls, timeout=20) as client: + for target in targets: + rules_url = self.firewall_rules_url(connection, target) + try: + deleted = await self.delete_existing_policy_rules(client, headers, rules_url, marker) + except Exception as exc: + errors.append({"target": target, "error": str(exc)}) + continue + deleted_rules.extend({"target": target, **item} for item in deleted) + + return { + "applied": not errors, + "rules_deleted": len(deleted_rules), + "deleted_rules": deleted_rules, + "errors": errors, + } diff --git a/backend/tests/test_proxmox_provider.py b/backend/tests/test_proxmox_provider.py index 7ceb21b..dac2736 100644 --- a/backend/tests/test_proxmox_provider.py +++ b/backend/tests/test_proxmox_provider.py @@ -107,3 +107,23 @@ async def test_apply_rules_replaces_only_marked_nexafabric_rules(monkeypatch: py "comment": "NexaFabric policy=policy-1 version=2 rule=1 target=web", } ] + + +@pytest.mark.asyncio +async def test_delete_policy_rules_removes_marked_rules(monkeypatch: pytest.MonkeyPatch) -> None: + FakeAsyncClient.deleted_urls = [] + FakeAsyncClient.posted_payloads = [] + FakeAsyncClient.put_urls = [] + FakeAsyncClient.put_payloads = [] + monkeypatch.setattr(proxmox.httpx, "AsyncClient", FakeAsyncClient) + + result = await ProxmoxProvider().delete_policy_rules( + ProviderConnection(api_url="https://pve.example:8006", token="user@pve!token=secret", read_only=False), + [{"node": "pve1", "kind": "qemu", "vmid": "100"}], + "policy-1", + ) + + assert result["applied"] is True + assert result["rules_deleted"] == 1 + assert FakeAsyncClient.deleted_urls == ["https://pve.example:8006/api2/json/nodes/pve1/qemu/100/firewall/rules/1"] + assert FakeAsyncClient.posted_payloads == []