[europrinter (yankawai)]

What this PR does

Fixes #5002. A DELETE of a tenant namespace is refused unless it comes from the identities that remove it on the normal path, and the denial says to delete the Tenant instead.

The new ValidatingAdmissionPolicy in cozystack-basics matches DELETE of core namespaces, and its binding selects namespaces labelled tenant.cozystack.io/tenant-root. The tenant chart puts that label on every tenant namespace below tenant-root (tenant.ancestorTenantLabels, and migration 49 backfilled older ones). tenant-root itself has no such label and stays with cozystack-no-delete-guardrail. The tenant-* probe namespaces of the e2e suites carry only namespace.cozystack.io/host and are not selected either.

Two groups pass. system:serviceaccounts:cozy-fluxcd, because helm-controller and its shards run as flux there, and a Tenant uninstall is helm-controller deleting the namespace after the pre-delete hook. system:serviceaccounts:kube-system, for the garbage collector following the ownerReference of a child namespace once its parent is gone. The namespace controller never sends a DELETE for a namespace, it updates finalize, so it is not affected (pkg/controller/namespace/deletion and pkg/registry/core/namespace/storage at v1.33.12 and v1.34.1). system:masters is not exempt, the same as with the no-delete guardrail.

This is the denial kube-apiserver 1.35 returns.

namespaces "tenant-foo" is forbidden: ValidatingAdmissionPolicy 'cozystack-tenant-namespace-delete-policy' with binding 'cozystack-tenant-namespace-delete-policy' denied request: Namespace tenant-foo belongs to the Tenant foo in tenant-root. Delete the Tenant instead with kubectl delete tenants.apps.cozystack.io foo --namespace tenant-root. Deleting the namespace directly can remove the tenant etcd before the Kubernetes clusters that use it and leave the namespace stuck in Terminating. If the Tenant is already gone, remove the label first with kubectl label namespace tenant-foo tenant.cozystack.io/tenant-root-

There are a few limits.

• The bypass is removing the label, like with the guardrail. That also takes the namespace out of the tenant-root egress policy and the dashboard tree, which only matters for a namespace that is about to go.
• Migration 49 labelled every tenant-* namespace that existed when it ran, so a non-tenant namespace with such a name is refused too and needs the same label removal.
• Because the garbage collector is exempt, a parent namespace whose label was removed and that is deleted with foreground propagation still cascades into its children directly.
• A DELETE of a namespace that is already Terminating is answered before admission, so the policy does not see it. It changes nothing either.
• A cluster that made helm-controller impersonate another account (--default-service-account, or spec.serviceAccountName on Tenant HelmReleases) would need that account allowed. Cozystack sets neither.
• It does not change how Kamaji cleans up once the order is broken, which is the second idea in Tenant namespace deleted directly is stuck Terminating on Kamaji finalizers #5002.

The helper rename in external_tls_policy_test.go (loadPolicy(t, path) and compilePolicy) is byte for byte the one in #2622, so the two merge in either order.

Checked locally.

• go test ./pkg/registry/apps/application/ -run 'TestTenantNamespaceDeletePolicy|TestDatabaseExternalTLSPolicy' -count=1 -v passes 4 tests, the new TestTenantNamespaceDeletePolicy_Decisions with 11 cases. go test ./pkg/registry/... ./pkg/config/... ./pkg/cmd/server/... passes in all 11 packages that have tests.
• helm unittest in packages/system/cozystack-basics with helm-unittest 1.1.2 in docker passes 12 suites and 70 tests, against 11 and 66 on main. The new tests on main without the template fail, both Go tests and 4 helm tests in 2 suites.
• 12 mutations of the template each fail a test. They exempt system:masters, drop either exempt group, select on the no-delete label, add UPDATE, always name tenant-root as the parent, drop the capability gate, label tenant-root, switch to Warn, switch failurePolicy to Ignore, drop the bypass from the message and drop the objectSelector.
• The rendered policy, the guardrail and namespaces rendered from the tenant chart were applied to kube-apiserver v1.33.0, v1.35.0 and v1.36.2 from setup-envtest, 16 checks on each. Real and dry-run deletes by a cluster admin, by an OIDC user bound to cluster-admin and by a cozy-system and a tenant ServiceAccount are refused with 403. The flux and garbage collector identities get their delete accepted. tenant-root gets the guardrail's denial, and with the guardrail binding removed a dry-run delete of it goes through, so this policy does not select it. A tenant-* namespace without the label and a tenant namespace after its label is removed are accepted for deletion. With this binding removed, a dry-run delete by the cluster admin goes through. That harness is not part of the PR.
• hack/bats-no-negated-assert.bats, hack/bats-no-exit-trap.bats, hack/bats-runner-coverage.bats, hack/helmrelease-readiness.bats and hack/route-hostname-policy-version-coverage.bats pass under hack/cozytest.sh.

The new e2e case in hack/e2e-install-cozystack.bats creates an empty Tenant, checks that a server dry-run delete of its namespace is refused with the message, deletes the Tenant and waits for the namespace to go. It was not run here, there is no Cozystack cluster at hand. Its kubectl steps were run against the envtest apiserver 1.35, the selector and field selector read, the refused dry-run with the exact strings it greps for, and kubectl wait --for=delete on a namespace that is gone. The Chainsaw suites that delete Tenants, the gateway inheritance ones among them, go through the uninstall path as well.

The release-1.6 backport is #5012.

Screenshots

Not a UI change.

Downstream repositories

• No downstream repository is affected by this change
• cozystack/website - follow-up:
• cozystack/terraform-provider-cozystack - follow-up:
• cozystack/ansible-cozystack - follow-up:
• cozystack/ccp - follow-up:
• cozystack/talm - follow-up:
• cozystack/cozyhr - follow-up:
• cozystack/cozy-proxy - follow-up:
• cozystack/cozystack-telemetry-server - follow-up:
• cozystack/external-apps-example - follow-up:
• cozystack/examples - follow-up:
• cozystack/community - follow-up:

The trigger map was walked against the diff. It adds a policy and tests to cozystack-basics and an e2e case, and no entry covers that. The website docs do not tell users to delete a tenant namespace directly, and terraform-provider-cozystack deletes Tenants through the apps API.

Release note

fix(cozystack-basics): refuse a direct DELETE of a tenant namespace unless it comes from helm-controller or the garbage collector, and point to deleting the Tenant instead, so a tenant namespace deleted by hand no longer gets stuck Terminating on Kamaji finalizers

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3729b470-8f60-4a10-9c71-32500b83e6b7

📥 Commits

Reviewing files that changed from the base of the PR and between 1f92bcd and 6bb0fda.

📒 Files selected for processing: 7

• hack/e2e-install-cozystack.bats
• packages/system/cozystack-basics/templates/tenant-namespace-delete-policy.yaml
• packages/system/cozystack-basics/tests/hostname-policies-capability-gate_test.yaml
• packages/system/cozystack-basics/tests/tenant-namespace-delete-policy_test.yaml
• packages/system/cozystack-basics/tests/tenant-root_test.yaml
• pkg/registry/apps/application/external_tls_policy_test.go
• pkg/registry/apps/application/tenant_namespace_delete_policy_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The chart adds a validating admission policy that denies direct deletion of tenant namespaces, except for specified platform service-account groups. Helm, Go, and end-to-end tests cover policy rendering, admission decisions, and removal of the namespace through Tenant deletion.

Changes: Tenant namespace deletion guard

Layer / File(s)	Summary
Admission policy and chart coverage packages/system/cozystack-basics/templates/tenant-namespace-delete-policy.yaml, packages/system/cozystack-basics/tests/tenant-namespace-delete-policy_test.yaml, packages/system/cozystack-basics/tests/hostname-policies-capability-gate_test.yaml, packages/system/cozystack-basics/tests/tenant-root_test.yaml	The chart adds a validating admission policy and binding for tenant namespace DELETE requests. Helm tests check the rendered resources, API capability gate, and tenant-root label.
Policy decision tests pkg/registry/apps/application/external_tls_policy_test.go, pkg/registry/apps/application/tenant_namespace_delete_policy_test.go	Go tests check policy binding and decisions for allowed and denied callers. The external TLS test helpers now accept an explicit template path and use generic names.
Tenant uninstall end-to-end test hack/e2e-install-cozystack.bats	An end-to-end test checks that direct namespace deletion is denied and that deleting the Tenant removes its namespace.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant E2ETest
  participant KubernetesAPIServer
  participant TenantNamespaceDeletePolicy
  E2ETest->>KubernetesAPIServer: Request direct namespace deletion
  KubernetesAPIServer->>TenantNamespaceDeletePolicy: Evaluate bound policy
  TenantNamespaceDeletePolicy-->>KubernetesAPIServer: Deny and return remediation guidance
  KubernetesAPIServer-->>E2ETest: Namespace deletion rejected
  E2ETest->>KubernetesAPIServer: Delete Tenant
  KubernetesAPIServer-->>E2ETest: Associated namespace is removed

Loading

Suggested reviewers: lexfrei

Merge Risk: ⚪ Minimal · up to 6bb0f

The change adds a guard against direct deletion of tenant namespaces and keeps normal Tenant uninstall working for the platform service accounts. No concrete merge-blocking risk was found. The new end-to-end case has not been run against a real cluster.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (4 skipped: 4…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preventing direct deletion of tenant namespaces in cozystack-basics.
Linked Issues check		Issue [#5002] requests a guard for direct deletion of child tenant namespaces. The PR adds a gated ValidatingAdmissionPolicy for core namespace DELETE requests and binds it to `tenant.cozystack.io…
Out of Scope Changes check		The changes stay within [#5002]. The policy template implements the namespace guard. The tenant-root and capability-gate tests verify related chart behavior. The Go helper rename supports reuse by the…

Full details: Linked Issues check

Explanation

Issue [#5002] requests a guard for direct deletion of child tenant namespaces. The PR adds a gated ValidatingAdmissionPolicy for core namespace DELETE requests and binds it to tenant.cozystack.io/tenant-root. The policy denies ordinary users, including system:masters, and directs them to delete the Tenant or remove the label for an orphaned namespace. It permits the documented Flux and garbage-collector service-account groups for normal uninstall and cascading cleanup. The Go and Helm tests cover matching, selection, decisions, exemptions, and denial messages. The separate Kamaji cleanup idea is not a requirement for this guard.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#5002]. The policy template implements the namespace guard. The tenant-root and capability-gate tests verify related chart behavior. The Go helper rename supports reuse by the new policy tests. The e2e test verifies the guarded delete and normal Tenant uninstall path. No unrelated product behavior or Kamaji cleanup implementation is added.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (4 skipped: 4 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[IvanHunters]

Verified the invariant rather than only the guard: enumerated every namespace deleter (no Go code deletes a tenant namespace; helm-controller in cozy-fluxcd and the GC in kube-system are exempt, so legitimate teardown via kubectl delete tenant still works). The direct human DELETE is refused, confirmed with the in-process CEL decision test (11/11) and a mutation that drops the kube-system exemption (the GC case then fails). No blocking issues.

Non-blocking:

• tenant-namespace-delete-policy.yaml:69-73: the system:serviceaccounts:kube-system exemption is broad (any SA in kube-system, not just the GC). Acceptable as a footgun guardrail since RBAC still gates who can delete, but worth awareness.
• tenant-namespace-delete-policy.yaml:74-84: the denial message parses tenant/parent by splitting on the last -, which is correct only while tenant names stay single-word. Deny fires regardless, so this is message accuracy only.
• Migration 49 labels every ^tenant- namespace, so a non-tenant namespace named tenant-* becomes undeletable until the label is removed (the message gives the command). Protection also depends on the tenant-root label being present, with a brief gap for a not-yet-reconciled deep tenant namespace.