Skip to content

Commit 6bb0fda

Browse files
committed
fix(cozystack-basics): refuse a direct delete of a tenant namespace
A tenant namespace is meant to go away with its Tenant. The tenant chart's pre-delete hook deletes the applications first and the tenant modules such as etcd after them, and helm-controller deletes the namespace at the end. A direct delete hands the namespace to the namespace controller instead. It deletes the pods first and then everything else together, so the tenant etcd and its etcd-ca-tls Secret can be gone before the TenantControlPlanes that use them. Kamaji then cannot reach the datastore to clean up, the TenantControlPlane finalizer never clears and the namespace stays Terminating. The policy refuses DELETE of a namespace labelled tenant.cozystack.io/tenant-root, which the tenant chart puts on every tenant namespace below tenant-root. tenant-root carries no such label and stays with the no-delete guardrail. Two callers are let through. helm-controller and its shards run as the flux ServiceAccount in cozy-fluxcd and delete the namespace during a Tenant uninstall. The garbage collector runs in kube-system and removes a child namespace through its ownerReference once the parent namespace is gone. The namespace controller only finalizes a namespace and never sends a DELETE for it, so it is not affected. Everyone else is refused, system:masters included, as with the no-delete guardrail. The message names the Tenant to delete and the label to remove when the Tenant is already gone. The policy test helpers take the template path now, so the new test compiles the shipped template with the apiserver CEL compiler like the external TLS policy test does. Assisted-by: LLM Signed-off-by: Yan Bondarenko <[email protected]>
1 parent 1f92bcd commit 6bb0fda

7 files changed

Lines changed: 331 additions & 11 deletions

File tree

‎hack/e2e-install-cozystack.bats‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,3 +561,40 @@ data:
561561
version: "${version}"
562562
EOF
563563
}
564+
565+
@test "Tenant namespace delete policy refuses a direct delete and keeps the Tenant uninstall" {
566+
if ! kubectl api-resources --api-group=admissionregistration.k8s.io \
567+
2>/dev/null | grep -qw validatingadmissionpolicies; then
568+
skip "ValidatingAdmissionPolicy API not available on this cluster"
569+
fi
570+
kubectl get validatingadmissionpolicy cozystack-tenant-namespace-delete-policy
571+
kubectl get validatingadmissionpolicybinding cozystack-tenant-namespace-delete-policy
572+
573+
kubectl apply -f - <<EOF
574+
apiVersion: apps.cozystack.io/v1alpha1
575+
kind: Tenant
576+
metadata:
577+
name: deleteprobe
578+
namespace: tenant-root
579+
spec: {}
580+
EOF
581+
timeout 60 sh -ec 'until kubectl get hr/tenant-deleteprobe -n tenant-root >/dev/null 2>&1; do sleep 2; done'
582+
kubectl wait hr/tenant-deleteprobe -n tenant-root --timeout=1m --for=condition=ready
583+
584+
local selected output rc still_there
585+
selected=$(kubectl get namespaces --selector tenant.cozystack.io/tenant-root --field-selector metadata.name=tenant-deleteprobe -o name)
586+
[ "$selected" = "namespace/tenant-deleteprobe" ]
587+
588+
output=$(kubectl delete namespace tenant-deleteprobe --dry-run=server 2>&1) && rc=0 || rc=$?
589+
echo "kubectl delete exit=$rc, output=$output"
590+
[ "$rc" -ne 0 ]
591+
echo "$output" | grep -qF "ValidatingAdmissionPolicy 'cozystack-tenant-namespace-delete-policy'"
592+
echo "$output" | grep -qF 'kubectl delete tenants.apps.cozystack.io deleteprobe --namespace tenant-root.'
593+
594+
kubectl delete tenants.apps.cozystack.io deleteprobe -n tenant-root --timeout=5m
595+
kubectl wait namespace/tenant-deleteprobe --for=delete --timeout=5m
596+
if ! still_there=$(kubectl get namespace tenant-deleteprobe --ignore-not-found -o name); then
597+
echo "FAIL: reading the tenant-deleteprobe namespace did not answer"; false
598+
fi
599+
if [ -n "$still_there" ]; then echo "FAIL: deleting the Tenant must remove its namespace"; false; fi
600+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
{{- if .Capabilities.APIVersions.Has "admissionregistration.k8s.io/v1/ValidatingAdmissionPolicy" }}
2+
---
3+
apiVersion: admissionregistration.k8s.io/v1
4+
kind: ValidatingAdmissionPolicy
5+
metadata:
6+
name: cozystack-tenant-namespace-delete-policy
7+
labels:
8+
internal.cozystack.io/managed-by-cozystack: ""
9+
spec:
10+
failurePolicy: Fail
11+
matchConstraints:
12+
resourceRules:
13+
- apiGroups: [""]
14+
apiVersions: ["v1"]
15+
operations: ["DELETE"]
16+
resources: ["namespaces"]
17+
variables:
18+
- name: platformCaller
19+
expression: >-
20+
has(request.userInfo.groups) && request.userInfo.groups.exists(g,
21+
g == "system:serviceaccounts:cozy-fluxcd" ||
22+
g == "system:serviceaccounts:kube-system")
23+
- name: name
24+
expression: oldObject.metadata.name
25+
- name: cut
26+
expression: variables.name.lastIndexOf("-")
27+
- name: tenant
28+
expression: variables.name.substring(variables.cut + 1)
29+
- name: parentNamespace
30+
expression: >-
31+
variables.cut > 0 && variables.name.substring(0, variables.cut) != "tenant"
32+
? variables.name.substring(0, variables.cut)
33+
: "tenant-root"
34+
validations:
35+
- expression: variables.platformCaller
36+
messageExpression: >-
37+
"Namespace " + variables.name + " belongs to the Tenant " + variables.tenant +
38+
" in " + variables.parentNamespace + ". Delete the Tenant instead with kubectl delete tenants.apps.cozystack.io " +
39+
variables.tenant + " --namespace " + variables.parentNamespace +
40+
". Deleting the namespace directly can remove the tenant etcd before the Kubernetes clusters that use it and leave the namespace stuck in Terminating." +
41+
" If the Tenant is already gone, remove the label first with kubectl label namespace " + variables.name +
42+
" tenant.cozystack.io/tenant-root-"
43+
reason: Forbidden
44+
---
45+
apiVersion: admissionregistration.k8s.io/v1
46+
kind: ValidatingAdmissionPolicyBinding
47+
metadata:
48+
name: cozystack-tenant-namespace-delete-policy
49+
labels:
50+
internal.cozystack.io/managed-by-cozystack: ""
51+
spec:
52+
policyName: cozystack-tenant-namespace-delete-policy
53+
validationActions: [Deny]
54+
matchResources:
55+
objectSelector:
56+
matchExpressions:
57+
- key: tenant.cozystack.io/tenant-root
58+
operator: Exists
59+
{{- end }}

‎packages/system/cozystack-basics/tests/hostname-policies-capability-gate_test.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
suite: cozystack-basics VAP policies are gated on the ValidatingAdmissionPolicy API
22
templates:
3+
- templates/tenant-namespace-delete-policy.yaml
34
- templates/route-hostname-policy.yaml
45
- templates/gateway-hostname-policy.yaml
56
- templates/ingress-hostname-policy.yaml
@@ -20,6 +21,9 @@ capabilities:
2021
tests:
2122
- it: none of the VAP policies render when the API is unavailable
2223
asserts:
24+
- hasDocuments:
25+
count: 0
26+
template: templates/tenant-namespace-delete-policy.yaml
2327
- hasDocuments:
2428
count: 0
2529
template: templates/route-hostname-policy.yaml
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
suite: tenant namespace delete policy renders with its binding
2+
templates:
3+
- templates/tenant-namespace-delete-policy.yaml
4+
release:
5+
name: cozystack-basics
6+
namespace: cozy-system
7+
capabilities:
8+
apiVersions:
9+
- admissionregistration.k8s.io/v1/ValidatingAdmissionPolicy
10+
tests:
11+
- it: renders the policy and a Deny binding to it
12+
asserts:
13+
- hasDocuments:
14+
count: 2
15+
- documentIndex: 0
16+
equal:
17+
path: kind
18+
value: ValidatingAdmissionPolicy
19+
- documentIndex: 0
20+
equal:
21+
path: metadata.name
22+
value: cozystack-tenant-namespace-delete-policy
23+
- documentIndex: 1
24+
equal:
25+
path: spec.policyName
26+
value: cozystack-tenant-namespace-delete-policy
27+
- documentIndex: 1
28+
equal:
29+
path: spec.validationActions
30+
value: [Deny]
31+
32+
- it: matches only DELETE of core namespaces
33+
asserts:
34+
- documentIndex: 0
35+
equal:
36+
path: spec.matchConstraints.resourceRules
37+
value:
38+
- apiGroups: [""]
39+
apiVersions: ["v1"]
40+
operations: ["DELETE"]
41+
resources: ["namespaces"]
42+
43+
- it: binds to the namespaces the tenant chart labels as descendants of tenant-root
44+
asserts:
45+
- documentIndex: 1
46+
equal:
47+
path: spec.matchResources.objectSelector
48+
value:
49+
matchExpressions:
50+
- key: tenant.cozystack.io/tenant-root
51+
operator: Exists

‎packages/system/cozystack-basics/tests/tenant-root_test.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,9 @@ tests:
7979
equal:
8080
path: spec.chartRef.name
8181
value: cozystack-tenant-application-default-tenant
82+
83+
- it: tenant-root namespace carries no tenant.cozystack.io/tenant-root label, which the tenant namespace delete policy selects on
84+
asserts:
85+
- documentIndex: 0
86+
notExists:
87+
path: metadata.labels["tenant.cozystack.io/tenant-root"]

‎pkg/registry/apps/application/external_tls_policy_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ import (
4141
// expressions a cluster actually receives rather than a copy of them.
4242
const externalTLSPolicyPath = "../../../../packages/system/cozystack-basics/templates/database-external-tls-policy.yaml"
4343

44-
// loadExternalTLSPolicy returns the policy and its binding from the chart
45-
// template. The template's only directives are the capability guard lines
46-
// around the manifests; any other directive would mean the test is no longer
47-
// looking at what renders, so it fails instead of guessing.
48-
func loadExternalTLSPolicy(t *testing.T) (*admissionregistrationv1.ValidatingAdmissionPolicy, *admissionregistrationv1.ValidatingAdmissionPolicyBinding) {
44+
// loadPolicy returns the policy and its binding from a chart template. The
45+
// template's only directives are the capability guard lines around the
46+
// manifests; any other directive would mean the test is no longer looking at
47+
// what renders, so it fails instead of guessing.
48+
func loadPolicy(t *testing.T, path string) (*admissionregistrationv1.ValidatingAdmissionPolicy, *admissionregistrationv1.ValidatingAdmissionPolicyBinding) {
4949
t.Helper()
5050

51-
raw, err := os.ReadFile(externalTLSPolicyPath)
51+
raw, err := os.ReadFile(path)
5252
if err != nil {
5353
t.Fatalf("read policy template: %v", err)
5454
}
@@ -92,11 +92,11 @@ func loadExternalTLSPolicy(t *testing.T) (*admissionregistrationv1.ValidatingAdm
9292
return policy, binding
9393
}
9494

95-
// compileExternalTLSPolicy mirrors compilePolicy in
95+
// compilePolicy mirrors compilePolicy in
9696
// k8s.io/apiserver/pkg/admission/plugin/policy/validating, which is
9797
// unexported: the same composition environment, the same variable, match
9898
// condition, validation and message compilers.
99-
func compileExternalTLSPolicy(t *testing.T, policy *admissionregistrationv1.ValidatingAdmissionPolicy) validating.Validator {
99+
func compilePolicy(t *testing.T, policy *admissionregistrationv1.ValidatingAdmissionPolicy) validating.Validator {
100100
t.Helper()
101101

102102
compiler, err := plugincel.NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
@@ -139,7 +139,7 @@ func compileExternalTLSPolicy(t *testing.T, policy *admissionregistrationv1.Vali
139139
}
140140

141141
func TestDatabaseExternalTLSPolicy_Binding(t *testing.T) {
142-
policy, binding := loadExternalTLSPolicy(t)
142+
policy, binding := loadPolicy(t, externalTLSPolicyPath)
143143

144144
if binding.Spec.PolicyName != policy.Name {
145145
t.Errorf("binding names policy %q, want %q", binding.Spec.PolicyName, policy.Name)
@@ -171,8 +171,8 @@ func TestDatabaseExternalTLSPolicy_Binding(t *testing.T) {
171171
}
172172

173173
func TestDatabaseExternalTLSPolicy_Decisions(t *testing.T) {
174-
policy, _ := loadExternalTLSPolicy(t)
175-
validator := compileExternalTLSPolicy(t, policy)
174+
policy, _ := loadPolicy(t, externalTLSPolicyPath)
175+
validator := compilePolicy(t, policy)
176176

177177
tlsOn := map[string]any{"enabled": true}
178178
tlsOff := map[string]any{"enabled": false}
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/*
2+
Copyright 2026 The Cozystack Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package application
18+
19+
import (
20+
"context"
21+
"slices"
22+
"strings"
23+
"testing"
24+
25+
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
28+
"k8s.io/apimachinery/pkg/labels"
29+
"k8s.io/apimachinery/pkg/runtime"
30+
"k8s.io/apimachinery/pkg/runtime/schema"
31+
"k8s.io/apiserver/pkg/admission"
32+
"k8s.io/apiserver/pkg/admission/plugin/policy/validating"
33+
celconfig "k8s.io/apiserver/pkg/apis/cel"
34+
"k8s.io/apiserver/pkg/authentication/user"
35+
)
36+
37+
const tenantNamespaceDeletePolicyPath = "../../../../packages/system/cozystack-basics/templates/tenant-namespace-delete-policy.yaml"
38+
39+
func TestTenantNamespaceDeletePolicy_Binding(t *testing.T) {
40+
policy, binding := loadPolicy(t, tenantNamespaceDeletePolicyPath)
41+
42+
if binding.Spec.PolicyName != policy.Name {
43+
t.Errorf("binding names policy %q, want %q", binding.Spec.PolicyName, policy.Name)
44+
}
45+
if !slices.Equal(binding.Spec.ValidationActions, []admissionregistrationv1.ValidationAction{admissionregistrationv1.Deny}) {
46+
t.Errorf("binding validationActions = %v, want [Deny]", binding.Spec.ValidationActions)
47+
}
48+
if policy.Spec.FailurePolicy == nil || *policy.Spec.FailurePolicy != admissionregistrationv1.Fail {
49+
t.Errorf("failurePolicy must be Fail")
50+
}
51+
52+
rules := policy.Spec.MatchConstraints.ResourceRules
53+
if len(rules) != 1 {
54+
t.Fatalf("want one resource rule, got %d", len(rules))
55+
}
56+
rule := rules[0]
57+
if !slices.Equal(rule.APIGroups, []string{""}) || !slices.Equal(rule.APIVersions, []string{"v1"}) {
58+
t.Errorf("rule must match core v1 only, got apiGroups %v apiVersions %v", rule.APIGroups, rule.APIVersions)
59+
}
60+
if !slices.Equal(rule.Operations, []admissionregistrationv1.OperationType{admissionregistrationv1.Delete}) {
61+
t.Errorf("operations = %v, want [DELETE]", rule.Operations)
62+
}
63+
if !slices.Equal(rule.Resources, []string{"namespaces"}) {
64+
t.Errorf("resources = %v, want [namespaces]", rule.Resources)
65+
}
66+
67+
if binding.Spec.MatchResources == nil || binding.Spec.MatchResources.ObjectSelector == nil {
68+
t.Fatal("binding must select namespaces by label")
69+
}
70+
selector, err := metav1.LabelSelectorAsSelector(binding.Spec.MatchResources.ObjectSelector)
71+
if err != nil {
72+
t.Fatalf("binding objectSelector: %v", err)
73+
}
74+
selected := []struct {
75+
name string
76+
labels labels.Set
77+
want bool
78+
}{
79+
{name: "tenant namespace under tenant-root", labels: labels.Set{"tenant.cozystack.io/tenant-root": "", "tenant.cozystack.io/tenant-foo": "", "namespace.cozystack.io/host": "foo.example.org"}, want: true},
80+
{name: "nested tenant namespace", labels: labels.Set{"tenant.cozystack.io/tenant-root": "", "tenant.cozystack.io/tenant-foo": "", "tenant.cozystack.io/tenant-foo-bar": ""}, want: true},
81+
{name: "tenant-root namespace", labels: labels.Set{"platform.cozystack.io/no-delete": "true", "namespace.cozystack.io/host": "example.org", "namespace.cozystack.io/gateway": "tenant-root"}},
82+
{name: "namespace named tenant-* without tenant labels", labels: labels.Set{"namespace.cozystack.io/host": "case-probe.example.org"}},
83+
{name: "unlabelled namespace", labels: labels.Set{}},
84+
}
85+
for _, tc := range selected {
86+
if got := selector.Matches(tc.labels); got != tc.want {
87+
t.Errorf("%s: selected = %v, want %v", tc.name, got, tc.want)
88+
}
89+
}
90+
}
91+
92+
func TestTenantNamespaceDeletePolicy_Decisions(t *testing.T) {
93+
policy, _ := loadPolicy(t, tenantNamespaceDeletePolicyPath)
94+
validator := compilePolicy(t, policy)
95+
96+
fluxGroups := []string{"system:serviceaccounts", "system:serviceaccounts:cozy-fluxcd", "system:authenticated"}
97+
kubeSystemGroups := []string{"system:serviceaccounts", "system:serviceaccounts:kube-system", "system:authenticated"}
98+
adminGroups := []string{"system:masters", "system:authenticated"}
99+
100+
cases := []struct {
101+
name string
102+
namespace string
103+
user string
104+
groups []string
105+
denied bool
106+
tenant string
107+
parent string
108+
}{
109+
{name: "helm-controller uninstalling a Tenant", namespace: "tenant-foo", user: "system:serviceaccount:cozy-fluxcd:flux", groups: fluxGroups},
110+
{name: "helm-controller uninstalling a nested Tenant", namespace: "tenant-foo-bar", user: "system:serviceaccount:cozy-fluxcd:flux", groups: fluxGroups},
111+
{name: "garbage collector following a deleted parent namespace", namespace: "tenant-foo-bar", user: "system:serviceaccount:kube-system:generic-garbage-collector", groups: kubeSystemGroups},
112+
{name: "cluster admin deleting a tenant namespace", namespace: "tenant-foo", user: "admin", groups: adminGroups, denied: true, tenant: "foo", parent: "tenant-root"},
113+
{name: "cluster admin deleting a nested tenant namespace", namespace: "tenant-foo-bar", user: "admin", groups: adminGroups, denied: true, tenant: "bar", parent: "tenant-foo"},
114+
{name: "cluster admin deleting a third-level tenant namespace", namespace: "tenant-foo-bar-baz", user: "admin", groups: adminGroups, denied: true, tenant: "baz", parent: "tenant-foo-bar"},
115+
{name: "cluster admin deleting a labelled namespace without a dash", namespace: "standalone", user: "admin", groups: adminGroups, denied: true, tenant: "standalone", parent: "tenant-root"},
116+
{name: "OIDC user bound to cluster-admin", namespace: "tenant-foo", user: "cluster-admin-user", groups: []string{"cozystack-cluster-admin", "system:authenticated"}, denied: true, tenant: "foo", parent: "tenant-root"},
117+
{name: "cozy-system service account", namespace: "tenant-foo", user: "system:serviceaccount:cozy-system:cozystack-api", groups: []string{"system:serviceaccounts", "system:serviceaccounts:cozy-system", "system:authenticated"}, denied: true, tenant: "foo", parent: "tenant-root"},
118+
{name: "tenant service account", namespace: "tenant-foo-bar", user: "system:serviceaccount:tenant-foo:default", groups: []string{"system:serviceaccounts", "system:serviceaccounts:tenant-foo", "system:authenticated"}, denied: true, tenant: "bar", parent: "tenant-foo"},
119+
{name: "user without groups", namespace: "tenant-foo", user: "no-groups", denied: true, tenant: "foo", parent: "tenant-root"},
120+
}
121+
122+
gvk := schema.GroupVersionKind{Version: "v1", Kind: "Namespace"}
123+
gvr := schema.GroupVersionResource{Version: "v1", Resource: "namespaces"}
124+
for _, tc := range cases {
125+
t.Run(tc.name, func(t *testing.T) {
126+
oldObj := &unstructured.Unstructured{Object: map[string]any{
127+
"apiVersion": "v1",
128+
"kind": "Namespace",
129+
"metadata": map[string]any{
130+
"name": tc.namespace,
131+
"labels": map[string]any{"tenant.cozystack.io/tenant-root": ""},
132+
},
133+
}}
134+
var newObj runtime.Object
135+
attrs := admission.NewAttributesRecord(newObj, oldObj, gvk, tc.namespace, tc.namespace, gvr, "", admission.Delete, &metav1.DeleteOptions{}, false, &user.DefaultInfo{Name: tc.user, Groups: tc.groups})
136+
versioned := &admission.VersionedAttributes{Attributes: attrs, VersionedObject: admission.NewLazyObject(newObj), VersionedOldObject: admission.NewLazyObject(oldObj), VersionedKind: gvk}
137+
138+
result := validator.Validate(context.Background(), gvr, versioned, nil, nil, celconfig.RuntimeCELCostBudget, nil)
139+
var denials []validating.PolicyDecision
140+
for _, d := range result.Decisions {
141+
if d.Evaluation == validating.EvalError {
142+
t.Fatalf("evaluation error: %s", d.Message)
143+
}
144+
if d.Action == validating.ActionDeny {
145+
denials = append(denials, d)
146+
}
147+
}
148+
if got := len(denials) > 0; got != tc.denied {
149+
t.Fatalf("denied = %v, want %v (decisions %+v)", got, tc.denied, result.Decisions)
150+
}
151+
for _, d := range denials {
152+
for _, want := range []string{
153+
"kubectl delete tenants.apps.cozystack.io " + tc.tenant + " --namespace " + tc.parent + ".",
154+
"kubectl label namespace " + tc.namespace + " tenant.cozystack.io/tenant-root-",
155+
} {
156+
if !strings.Contains(d.Message, want) {
157+
t.Errorf("denial message must carry %q, got %q", want, d.Message)
158+
}
159+
}
160+
}
161+
})
162+
}
163+
}

0 commit comments

Comments
 (0)