Skip to content

Commit 81198b2

Browse files
lexfreilexfrei
authored
fix(bucket): clean up orphaned cert-manager ACME solver resources on delete (#3060) (#3076)
## What this PR does After deleting a `Bucket` app CR, cert-manager's temporary ACME HTTP-01 solver resources (`cm-acme-http-solver-*` Ingress/Pod/Service) were left orphaned in the tenant namespace. They are created by cert-manager to answer an ACME challenge and are owned by the `Challenge`/`Order` → `Certificate`, not by the app Ingress. If a challenge gets stuck and the app is deleted, the parent Ingress is removed but the orphaned challenge leaves the solver objects with no live owner to cascade-delete them, so Helm/Flux uninstall cannot reap them. This adds a `post-delete` cleanup hook that deletes the release's `Certificate`/`Order`/`Challenge` (cert-manager then cascade-removes the solver objects) and then sweeps any leftover solver objects by the reliable `acme.cert-manager.io/http01-solver=true` label. RBAC is verified against the vendored cert-manager CRDs (`cert-manager.io` → certificates; `acme.cert-manager.io` → orders, challenges) plus core `services`/`pods` and `networking.k8s.io` ingresses. The cleanup Job is hardened: non-root (uid 65534), read-only root fs, all caps dropped, `seccompProfile: RuntimeDefault`. Failures are logged but never block teardown. The preventive wildcard-cert guard the issue mentions already exists in the vendored `bucket-system` Ingress template (it drops the ACME annotations in wildcard mode), so no preventive change is needed — only cleanup. No user data is affected: only cert-manager's transient challenge artifacts are removed. Fixes #3060. ### Release note ```release-note fix(bucket): clean up orphaned cert-manager ACME http01-solver resources (Ingress/Pod/Service) left behind after a Bucket app is deleted. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a bucket app `test` command that runs Helm unit tests. * Introduced a post-delete ACME cleanup hook to remove release-scoped cert-manager TLS and HTTP-01 solver resources, using dedicated least-privilege access controls. * **Bug Fixes** * Improved cleanup safety with bounded execution and hardened container security. * **Tests** * Added a Helm test suite covering rendered hook documents, including host-scoped targeting and negative assertions to prevent overly broad deletions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 544ba5b + 6a43adb commit 81198b2

3 files changed

Lines changed: 346 additions & 0 deletions

File tree

‎packages/apps/bucket/Makefile‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
include ../../../hack/package.mk
22

3+
.PHONY: test
4+
test:
5+
helm unittest .
6+
37
generate:
48
cozyvalues-gen -m 'bucket' -v values.yaml -s values.schema.json -r README.md -g ../../../api/apps/v1alpha1/bucket/types.go
59
../../../hack/update-crd.sh

‎packages/apps/bucket/templates/hooks/cleanup-acme.yaml‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
---
2+
apiVersion: batch/v1
3+
kind: Job
4+
metadata:
5+
name: {{ .Release.Name }}-cleanup-acme
6+
labels:
7+
app.kubernetes.io/instance: {{ .Release.Name }}
8+
annotations:
9+
"helm.sh/hook": post-delete
10+
"helm.sh/hook-weight": "10"
11+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
12+
spec:
13+
# Bound the whole cleanup: never let a stuck finalizer or a hung API call
14+
# leave this post-delete hook running indefinitely and block the uninstall.
15+
activeDeadlineSeconds: 120
16+
backoffLimit: 0
17+
template:
18+
metadata:
19+
labels:
20+
app.kubernetes.io/instance: {{ .Release.Name }}
21+
policy.cozystack.io/allow-to-apiserver: "true"
22+
spec:
23+
serviceAccountName: {{ .Release.Name }}-cleanup-acme
24+
restartPolicy: Never
25+
securityContext:
26+
runAsNonRoot: true
27+
runAsUser: 65534
28+
runAsGroup: 65534
29+
seccompProfile:
30+
type: RuntimeDefault
31+
containers:
32+
- name: cleanup
33+
image: docker.io/clastix/kubectl:v1.32
34+
securityContext:
35+
allowPrivilegeEscalation: false
36+
readOnlyRootFilesystem: true
37+
capabilities:
38+
drop: ["ALL"]
39+
resources:
40+
requests:
41+
cpu: 10m
42+
memory: 64Mi
43+
limits:
44+
cpu: 200m
45+
memory: 128Mi
46+
env:
47+
- name: HOME
48+
value: /tmp
49+
command:
50+
- /bin/sh
51+
- -c
52+
- |
53+
echo "Cleaning up orphaned cert-manager ACME resources for {{ .Release.Name }}..."
54+
# This release's TLS host. The bucket-system chart renders its
55+
# Ingress / Certificate for "<bucketName>.<_namespace.host>", and
56+
# bucketName is set to .Release.Name in the HelmRelease. cert-manager
57+
# stamps this exact value onto its Challenge .spec.dnsName and onto
58+
# the orphaned solver Ingress .spec.rules[].host, so the host is the
59+
# one reliable per-release identifier for ACME solver objects (they
60+
# carry no app/release label of their own).
61+
HOST="{{ .Release.Name }}.{{ .Values._namespace.host }}"
62+
# Delete the per-bucket Certificate/Order/Challenge. cert-manager
63+
# then cascade-removes the cm-acme-http-solver-* solver objects it
64+
# owns. The release's TLS Certificate is named after the secret it
65+
# populates ({{ .Release.Name }}-ui-tls).
66+
kubectl delete certificate {{ .Release.Name }}-ui-tls -n {{ .Release.Namespace }} --ignore-not-found --wait=false --request-timeout=30s \
67+
|| echo "WARNING: cert-manager cleanup failed for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
68+
# Orders are owned by the Certificate and are removed by the cascade
69+
# above; Challenges carry no labels and are reaped by the
70+
# host-scoped .spec.dnsName sweep below. There is therefore no
71+
# reliable label selector for Order/Challenge, so we do not attempt
72+
# a label-based delete here.
73+
#
74+
# Release-scoped solver sweep. We deliberately do NOT delete
75+
# ingress/service/pod objects namespace-wide by the
76+
# acme.cert-manager.io/http01-solver=true label: that label is set
77+
# on EVERY app's solver objects in this shared tenant namespace, so
78+
# a blanket delete would tear down sibling apps' in-flight ACME
79+
# challenges. Instead we scope by HOST, which is unique to this
80+
# release, so we only ever touch our own solver objects.
81+
#
82+
# Delete Challenges for our host (cascades the solver
83+
# Ingress/Service/Pod they own).
84+
challenges="$(kubectl get challenge -n {{ .Release.Namespace }} --request-timeout=30s -o jsonpath="{range .items[?(@.spec.dnsName=='$HOST')]}{.metadata.name} {end}")" \
85+
|| { echo "WARNING: failed to list challenges for {{ .Release.Name }}; orphaned ACME resources may remain" >&2; challenges=""; }
86+
for ch in $challenges; do
87+
kubectl delete challenge -n {{ .Release.Namespace }} "$ch" --ignore-not-found --wait=false --request-timeout=30s \
88+
|| echo "WARNING: failed to delete challenge $ch for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
89+
done
90+
# Delete any orphaned solver Ingress for our host (no live owner
91+
# left to cascade it). Its solver Service/Pod are owned by a
92+
# Challenge and are cascade-deleted with it; truly owner-less
93+
# solver Service/Pod with no matching Challenge are not expected
94+
# and are not swept namespace-wide.
95+
ingresses="$(kubectl get ingress -n {{ .Release.Namespace }} -l acme.cert-manager.io/http01-solver=true --request-timeout=30s -o jsonpath="{range .items[?(@.spec.rules[0].host=='$HOST')]}{.metadata.name} {end}")" \
96+
|| { echo "WARNING: failed to list solver ingresses for {{ .Release.Name }}; orphaned ACME resources may remain" >&2; ingresses=""; }
97+
for ing in $ingresses; do
98+
kubectl delete ingress -n {{ .Release.Namespace }} "$ing" --ignore-not-found --wait=false --request-timeout=30s \
99+
|| echo "WARNING: failed to delete solver ingress $ing for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
100+
done
101+
echo "ACME cleanup complete."
102+
volumeMounts:
103+
- name: tmp
104+
mountPath: /tmp
105+
volumes:
106+
- name: tmp
107+
emptyDir: {}
108+
---
109+
apiVersion: v1
110+
kind: ServiceAccount
111+
metadata:
112+
name: {{ .Release.Name }}-cleanup-acme
113+
labels:
114+
app.kubernetes.io/instance: {{ .Release.Name }}
115+
annotations:
116+
"helm.sh/hook": post-delete
117+
"helm.sh/hook-weight": "0"
118+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
119+
---
120+
apiVersion: rbac.authorization.k8s.io/v1
121+
kind: Role
122+
metadata:
123+
name: {{ .Release.Name }}-cleanup-acme
124+
labels:
125+
app.kubernetes.io/instance: {{ .Release.Name }}
126+
annotations:
127+
"helm.sh/hook": post-delete
128+
"helm.sh/hook-weight": "5"
129+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
130+
rules:
131+
- apiGroups: ["cert-manager.io"]
132+
resources: ["certificates"]
133+
verbs: ["get", "list", "delete"]
134+
- apiGroups: ["acme.cert-manager.io"]
135+
resources: ["orders", "challenges"]
136+
verbs: ["get", "list", "delete"]
137+
# Needed to list+delete this release's orphaned http01 solver Ingress objects
138+
# by host. Services/Pods are intentionally omitted: the solver Service/Pod are
139+
# owned by their Challenge and cascade-delete with it, so we never delete them
140+
# directly (and thus never sweep them namespace-wide).
141+
- apiGroups: ["networking.k8s.io"]
142+
resources: ["ingresses"]
143+
verbs: ["get", "list", "delete"]
144+
---
145+
apiVersion: rbac.authorization.k8s.io/v1
146+
kind: RoleBinding
147+
metadata:
148+
name: {{ .Release.Name }}-cleanup-acme
149+
labels:
150+
app.kubernetes.io/instance: {{ .Release.Name }}
151+
annotations:
152+
"helm.sh/hook": post-delete
153+
"helm.sh/hook-weight": "5"
154+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
155+
roleRef:
156+
apiGroup: rbac.authorization.k8s.io
157+
kind: Role
158+
name: {{ .Release.Name }}-cleanup-acme
159+
subjects:
160+
- kind: ServiceAccount
161+
name: {{ .Release.Name }}-cleanup-acme

‎packages/apps/bucket/tests/cleanup_acme_test.yaml‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
suite: bucket post-delete ACME cleanup hook
2+
templates:
3+
- templates/hooks/cleanup-acme.yaml
4+
5+
release:
6+
name: my-bucket
7+
namespace: tenant-root
8+
9+
set:
10+
_namespace:
11+
host: example.org
12+
13+
tests:
14+
- it: renders only the expected cleanup hook objects (Job, SA, Role, RoleBinding)
15+
asserts:
16+
- hasDocuments:
17+
count: 4
18+
19+
- it: renders a post-delete cleanup Job with the expected name and SA
20+
documentIndex: 0
21+
asserts:
22+
- equal:
23+
path: kind
24+
value: Job
25+
- equal:
26+
path: metadata.name
27+
value: my-bucket-cleanup-acme
28+
- equal:
29+
path: metadata.annotations["helm.sh/hook"]
30+
value: post-delete
31+
- equal:
32+
path: spec.template.spec.serviceAccountName
33+
value: my-bucket-cleanup-acme
34+
- equal:
35+
path: spec.template.spec.restartPolicy
36+
value: Never
37+
- equal:
38+
path: spec.template.metadata.labels["policy.cozystack.io/allow-to-apiserver"]
39+
value: "true"
40+
# Bounded execution so a stuck hook can't block the uninstall.
41+
- equal:
42+
path: spec.activeDeadlineSeconds
43+
value: 120
44+
- equal:
45+
path: spec.backoffLimit
46+
value: 0
47+
# Security hardening contract the PR relies on.
48+
- equal:
49+
path: spec.template.spec.securityContext.runAsNonRoot
50+
value: true
51+
- equal:
52+
path: spec.template.spec.containers[0].securityContext.readOnlyRootFilesystem
53+
value: true
54+
- equal:
55+
path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation
56+
value: false
57+
- equal:
58+
path: spec.template.spec.containers[0].securityContext.capabilities.drop[0]
59+
value: ALL
60+
- equal:
61+
path: spec.template.spec.securityContext.seccompProfile.type
62+
value: RuntimeDefault
63+
64+
- it: deletes release-scoped cert-manager resources with a host-scoped solver sweep
65+
documentIndex: 0
66+
asserts:
67+
- matchRegex:
68+
path: spec.template.spec.containers[0].command[2]
69+
pattern: "kubectl delete certificate my-bucket-ui-tls -n tenant-root"
70+
# Orders are removed by the Certificate cascade and Challenges by the
71+
# host-scoped dnsName sweep. cert-manager puts no usable label on either
72+
# (Challenges are unlabeled; certificate-name is an annotation on Orders),
73+
# so the hook must NOT attempt a label-based order/challenge delete.
74+
- notMatchRegex:
75+
path: spec.template.spec.containers[0].command[2]
76+
pattern: "delete order,challenge"
77+
- notMatchRegex:
78+
path: spec.template.spec.containers[0].command[2]
79+
pattern: "delete .*-l app.kubernetes.io/instance=my-bucket"
80+
# Host derived from <release>.<_namespace.host> == my-bucket.example.org
81+
- matchRegex:
82+
path: spec.template.spec.containers[0].command[2]
83+
pattern: 'HOST="my-bucket\.example\.org"'
84+
# Host-scoped challenge sweep filtering on .spec.dnsName == our host
85+
- matchRegex:
86+
path: spec.template.spec.containers[0].command[2]
87+
pattern: "kubectl get challenge -n tenant-root --request-timeout=30s -o jsonpath=.*@.spec.dnsName=='\\$HOST'"
88+
- matchRegex:
89+
path: spec.template.spec.containers[0].command[2]
90+
pattern: 'kubectl delete challenge -n tenant-root "\$ch" --ignore-not-found'
91+
# Host-scoped orphaned solver Ingress sweep filtering on rules[0].host
92+
- matchRegex:
93+
path: spec.template.spec.containers[0].command[2]
94+
pattern: "kubectl get ingress -n tenant-root -l acme.cert-manager.io/http01-solver=true --request-timeout=30s -o jsonpath=.*@.spec.rules\\[0\\].host=='\\$HOST'"
95+
- matchRegex:
96+
path: spec.template.spec.containers[0].command[2]
97+
pattern: 'kubectl delete ingress -n tenant-root "\$ing" --ignore-not-found'
98+
# NO namespace-wide blanket solver sweep
99+
- notMatchRegex:
100+
path: spec.template.spec.containers[0].command[2]
101+
pattern: "kubectl delete ingress,svc,pod"
102+
- notMatchRegex:
103+
path: spec.template.spec.containers[0].command[2]
104+
pattern: "delete .*-l acme.cert-manager.io/http01-solver=true"
105+
106+
- it: sets resource requests and limits on the cleanup container
107+
documentIndex: 0
108+
asserts:
109+
- equal:
110+
path: spec.template.spec.containers[0].resources.requests.cpu
111+
value: 10m
112+
- equal:
113+
path: spec.template.spec.containers[0].resources.requests.memory
114+
value: 64Mi
115+
- equal:
116+
path: spec.template.spec.containers[0].resources.limits.cpu
117+
value: 200m
118+
- equal:
119+
path: spec.template.spec.containers[0].resources.limits.memory
120+
value: 128Mi
121+
122+
- it: creates a ServiceAccount for the cleanup job
123+
documentIndex: 1
124+
asserts:
125+
- equal:
126+
path: kind
127+
value: ServiceAccount
128+
- equal:
129+
path: metadata.name
130+
value: my-bucket-cleanup-acme
131+
132+
- it: grants RBAC limited to cert-manager and acme resources
133+
documentIndex: 2
134+
asserts:
135+
- equal:
136+
path: kind
137+
value: Role
138+
- contains:
139+
path: rules
140+
content:
141+
apiGroups: ["cert-manager.io"]
142+
resources: ["certificates"]
143+
verbs: ["get", "list", "delete"]
144+
- contains:
145+
path: rules
146+
content:
147+
apiGroups: ["acme.cert-manager.io"]
148+
resources: ["orders", "challenges"]
149+
verbs: ["get", "list", "delete"]
150+
# Ingresses re-added: we list+delete this release's orphaned solver
151+
# Ingress objects by host.
152+
- contains:
153+
path: rules
154+
content:
155+
apiGroups: ["networking.k8s.io"]
156+
resources: ["ingresses"]
157+
verbs: ["get", "list", "delete"]
158+
# Services/Pods stay OUT: solver Service/Pod cascade-delete with their
159+
# owning Challenge, so the hook never deletes them directly.
160+
- notContains:
161+
path: rules
162+
content:
163+
apiGroups: [""]
164+
resources: ["services", "pods"]
165+
verbs: ["get", "list", "delete"]
166+
- lengthEqual:
167+
path: rules
168+
count: 3
169+
170+
- it: binds the Role to the cleanup ServiceAccount
171+
documentIndex: 3
172+
asserts:
173+
- equal:
174+
path: kind
175+
value: RoleBinding
176+
- equal:
177+
path: roleRef.name
178+
value: my-bucket-cleanup-acme
179+
- equal:
180+
path: subjects[0].name
181+
value: my-bucket-cleanup-acme

0 commit comments

Comments
 (0)