Skip to content

Commit bef4222

Browse files
scooby87scooby87
authored
fix(harbor): clean up orphaned jobservice PVC and ACME solver resources on delete (#3061) (#3077)
## What this PR does After deleting a `Harbor` app CR, two things were left orphaned in the tenant namespace: - the **jobservice / trivy PVCs** (`<release>-jobservice`, and `<release>-trivy` when Trivy is enabled), because `packages/apps/harbor/templates/harbor.yaml` sets `persistence.resourcePolicy: "keep"`, which annotates the upstream Harbor subchart's PVCs with `helm.sh/resource-policy: keep` so Helm/Flux deliberately never deletes them; - cert-manager's **ACME http01-solver** resources (`cm-acme-http-solver-*`), via the same stuck-challenge mechanism as the Bucket issue (#3060) when Harbor issues a non-wildcard cert. This adds a single `post-delete` cleanup hook that: - **deletes the upstream Harbor subchart's kept PVCs** — selecting by `app.kubernetes.io/instance=<release>-system` (the subchart runs as the `<release>-system` HelmRelease, so its PVCs carry that label, **not** `<release>`). A hook is required precisely because `resourcePolicy: keep` blocks normal reclaim. This intentionally covers both the jobservice and trivy PVCs: both hold only regenerable data (jobservice file logs; trivy's vulnerability-DB cache), and registry images live in S3. - **deletes this release's `Certificate`** (deterministic name `<release>-ingress-tls`); cert-manager then cascade-deletes the `Order`, `Challenge` and solver `Pod`/`Service`/`Ingress` objects it owns via owner references. - **host-scoped belt-and-braces sweep** for any ACME http01 solver objects cert-manager may have orphaned (e.g. the Certificate was already gone, so deleting it did not cascade). The sweep is scoped **strictly to this release's Harbor host** — Challenges filtered by `.spec.dnsName`, solver Ingresses by `.spec.rules[0].host` — **not** a namespace-wide `acme.cert-manager.io/http01-solver=true` label sweep, because a Cozystack tenant namespace can host many apps and a blanket label sweep would delete sibling apps' in-flight challenge solvers. The Job is hardened: non-root (uid 65534), read-only root fs, all caps dropped, `seccompProfile: RuntimeDefault`, with CPU/memory requests and limits set. RBAC is scoped to exactly the resources it deletes (`persistentvolumeclaims`, `certificates`, `orders`, `challenges`, solver `ingresses`) and verified against the vendored cert-manager CRDs. Failures are logged but never block teardown. ## Cleanup scope — what is and isn't removed **Removed:** the upstream Harbor subchart's kept PVCs (jobservice + trivy) and this release's cert-manager ACME artifacts (Certificate, and host-scoped Order/Challenge/solver-Ingress). **Not touched (by design):** - **CNPG database PVCs** (`<release>-db-N`) — owned by the CNPG `Cluster` and governed by CNPG's own reclaim policy; they carry CNPG labels, not `app.kubernetes.io/instance=<release>-system`. Re-installing Harbor with a different DB size into the same namespace can therefore leak the old DB PVCs; this is left to CNPG's lifecycle on purpose (the data is the user's). - **Redis PVCs** — managed by the redis-operator with their own labels, likewise untouched. - **Solver `Pod`/`Service`** — not deleted directly; they are removed by cert-manager's owner-ref cascade when the Certificate/Challenge is deleted, so the hook grants no RBAC for them. > [!NOTE] > The PVC sweep is eventually-consistent: while the inner `<release>-system` > HelmRelease is still being uninstalled by Flux, a PVC may briefly stay in > `Terminating` until its consumer Pods (jobservice/trivy) exit. The Job's > deletion request always succeeds; reclaim completes once the pods are gone. > [!WARNING] > **This overrides the intentional `persistence.resourcePolicy: "keep"` setting > and force-deletes the jobservice/trivy PVCs on app deletion.** Those PVCs hold > only jobservice file logs and the trivy vuln-DB cache (registry images live in > S3), so data-loss risk is low — but if retaining them across re-installs is > desired, this is a conscious product decision to revisit. The ACME cleanup > affects only cert-manager's transient artifacts. Fixes #3061. > Follow-up tracked in #3088: digest-pin the cleanup `kubectl` image and route it > through `cozy-lib.images-registry` (pre-existing debt shared with the MariaDB > cleanup hook), out of scope for this PR. ### Release note ```release-note fix(harbor): clean up the orphaned jobservice/trivy PVCs and cert-manager ACME http01-solver resources left behind after a Harbor app is deleted. NOTE: this force-deletes the jobservice/trivy PVCs, overriding persistence.resourcePolicy=keep (jobservice file logs and trivy vuln-DB cache only; registry images are in S3). CNPG database PVCs are not touched. ```
2 parents 3e20a0c + b25f294 commit bef4222

3 files changed

Lines changed: 350 additions & 0 deletions

File tree

‎packages/apps/harbor/Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ NAME=harbor
22

33
include ../../../hack/package.mk
44

5+
.PHONY: test
56
test:
67
helm unittest .
78

‎packages/apps/harbor/templates/hooks/cleanup.yaml‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
{{- $harborHost := .Values.host | default (printf "%s.%s" .Release.Name .Values._namespace.host) }}
2+
---
3+
apiVersion: batch/v1
4+
kind: Job
5+
metadata:
6+
name: {{ .Release.Name }}-cleanup
7+
labels:
8+
app.kubernetes.io/instance: {{ .Release.Name }}
9+
annotations:
10+
"helm.sh/hook": post-delete
11+
"helm.sh/hook-weight": "10"
12+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
13+
spec:
14+
template:
15+
metadata:
16+
labels:
17+
app.kubernetes.io/instance: {{ .Release.Name }}
18+
policy.cozystack.io/allow-to-apiserver: "true"
19+
spec:
20+
serviceAccountName: {{ .Release.Name }}-cleanup
21+
restartPolicy: Never
22+
securityContext:
23+
runAsNonRoot: true
24+
runAsUser: 65534
25+
runAsGroup: 65534
26+
seccompProfile:
27+
type: RuntimeDefault
28+
containers:
29+
- name: cleanup
30+
image: docker.io/clastix/kubectl:v1.32
31+
securityContext:
32+
allowPrivilegeEscalation: false
33+
readOnlyRootFilesystem: true
34+
capabilities:
35+
drop: ["ALL"]
36+
resources:
37+
requests:
38+
cpu: 10m
39+
memory: 64Mi
40+
limits:
41+
cpu: 200m
42+
memory: 128Mi
43+
env:
44+
- name: HOME
45+
value: /tmp
46+
command:
47+
- /bin/sh
48+
- -c
49+
- |
50+
set -u
51+
# Harbor's jobservice and trivy PVCs are annotated with
52+
# helm.sh/resource-policy: keep by the upstream subchart
53+
# (persistence.resourcePolicy: keep), so Helm/Flux deliberately
54+
# leave them behind on uninstall. The subchart runs as the
55+
# "{{ .Release.Name }}-system" HelmRelease, so both PVCs carry the
56+
# label release={{ .Release.Name }}-system: jobservice via the
57+
# chart's "harbor.labels" helper, and trivy's StatefulSet PVC via
58+
# "harbor.legacy.labels". We must select on "release" (not
59+
# app.kubernetes.io/instance) because the trivy PVC does NOT carry
60+
# app.kubernetes.io/instance, so an instance-label sweep would
61+
# silently miss trivy (enabled by default). CNPG (db) and Redis
62+
# PVCs use different label schemes and the registry is on S3, so
63+
# this selector matches only those two kept PVCs.
64+
echo "Deleting orphaned PVCs for {{ .Release.Name }}-system..."
65+
kubectl delete pvc -n {{ .Release.Namespace }} \
66+
-l release={{ .Release.Name }}-system --ignore-not-found \
67+
|| echo "WARNING: PVC cleanup failed for {{ .Release.Name }}; orphaned PVCs may remain" >&2
68+
69+
# Remove the cert-manager Certificate owned by this release's
70+
# Ingress so an in-flight http01 challenge does not leave a
71+
# dangling cm-acme-http-solver-* Pod/Service/Ingress behind.
72+
# Deleting the release's Certificate (deterministic name
73+
# "{{ .Release.Name }}-ingress-tls") triggers cert-manager to
74+
# cascade-delete the Orders, Challenges and solver
75+
# Pod/Service/Ingress objects it owns via owner references.
76+
# We delete only the Certificate by name: Orders/Challenges have
77+
# hash-suffixed names, so a by-name delete of those kinds would
78+
# be a no-op (the host-scoped sweep below covers them instead).
79+
# In Gateway mode no per-app Ingress or "-ingress-tls"
80+
# Certificate is rendered, so this delete is simply a no-op there
81+
# and the dnsName-scoped sweep below still handles http01.
82+
echo "Deleting ACME Certificate for {{ .Release.Name }}-ingress..."
83+
kubectl delete certificate -n {{ .Release.Namespace }} \
84+
{{ .Release.Name }}-ingress-tls --ignore-not-found \
85+
|| echo "WARNING: cert-manager cleanup failed for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
86+
87+
# Belt-and-braces sweep for ACME http01 solver objects that
88+
# cert-manager may have orphaned (e.g. the Certificate was
89+
# already gone, so deleting it above did not cascade). We scope
90+
# STRICTLY to THIS release's TLS host ("$HOST") instead of a
91+
# namespace-wide acme.cert-manager.io/http01-solver=true label
92+
# sweep: a Cozystack tenant namespace can host many apps and
93+
# cert-manager sets that label on every app's solver objects, so
94+
# a blanket sweep would delete sibling apps' in-flight ACME
95+
# challenge solvers. Filtering Challenges by .spec.dnsName and
96+
# solver Ingresses by .spec.rules[0].host keeps us to objects
97+
# issued for this release's single Harbor host only.
98+
HOST="{{ $harborHost }}"
99+
echo "Sweeping orphaned ACME http01 solvers for host $HOST..."
100+
for ch in $(kubectl get challenge -n {{ .Release.Namespace }} \
101+
-o jsonpath="{range .items[?(@.spec.dnsName=='$HOST')]}{.metadata.name} {end}" 2>/dev/null); do
102+
kubectl delete challenge -n {{ .Release.Namespace }} "$ch" --ignore-not-found \
103+
|| echo "WARNING: failed to delete challenge $ch for {{ .Release.Name }}; orphaned solver may remain" >&2
104+
done
105+
for ing in $(kubectl get ingress -n {{ .Release.Namespace }} \
106+
-l acme.cert-manager.io/http01-solver=true \
107+
-o jsonpath="{range .items[?(@.spec.rules[0].host=='$HOST')]}{.metadata.name} {end}" 2>/dev/null); do
108+
kubectl delete ingress -n {{ .Release.Namespace }} "$ing" --ignore-not-found \
109+
|| echo "WARNING: failed to delete solver ingress $ing for {{ .Release.Name }}; orphaned solver may remain" >&2
110+
done
111+
112+
echo "Cleanup complete."
113+
volumeMounts:
114+
- name: tmp
115+
mountPath: /tmp
116+
volumes:
117+
- name: tmp
118+
emptyDir: {}
119+
---
120+
apiVersion: v1
121+
kind: ServiceAccount
122+
metadata:
123+
name: {{ .Release.Name }}-cleanup
124+
labels:
125+
app.kubernetes.io/instance: {{ .Release.Name }}
126+
annotations:
127+
"helm.sh/hook": post-delete
128+
helm.sh/hook-weight: "0"
129+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
130+
---
131+
apiVersion: rbac.authorization.k8s.io/v1
132+
kind: Role
133+
metadata:
134+
name: {{ .Release.Name }}-cleanup
135+
labels:
136+
app.kubernetes.io/instance: {{ .Release.Name }}
137+
annotations:
138+
"helm.sh/hook": post-delete
139+
"helm.sh/hook-weight": "5"
140+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
141+
rules:
142+
- apiGroups: [""]
143+
resources: ["persistentvolumeclaims"]
144+
verbs: ["get", "list", "delete"]
145+
- apiGroups: ["cert-manager.io"]
146+
resources: ["certificates"]
147+
verbs: ["get", "list", "delete"]
148+
# Only Challenges are touched directly (the host-scoped sweep lists and
149+
# deletes them); Orders are cascade-deleted by cert-manager itself when the
150+
# Certificate is removed, so the hook needs no RBAC for them.
151+
- apiGroups: ["acme.cert-manager.io"]
152+
resources: ["challenges"]
153+
verbs: ["get", "list", "delete"]
154+
# We list ACME http01 solver Ingresses (filtered by this release's host)
155+
# and delete the orphaned ones by name.
156+
- apiGroups: ["networking.k8s.io"]
157+
resources: ["ingresses"]
158+
verbs: ["get", "list", "delete"]
159+
---
160+
apiVersion: rbac.authorization.k8s.io/v1
161+
kind: RoleBinding
162+
metadata:
163+
name: {{ .Release.Name }}-cleanup
164+
labels:
165+
app.kubernetes.io/instance: {{ .Release.Name }}
166+
annotations:
167+
"helm.sh/hook": post-delete
168+
helm.sh/hook-weight: "5"
169+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
170+
roleRef:
171+
apiGroup: rbac.authorization.k8s.io
172+
kind: Role
173+
name: {{ .Release.Name }}-cleanup
174+
subjects:
175+
- kind: ServiceAccount
176+
name: {{ .Release.Name }}-cleanup

‎packages/apps/harbor/tests/cleanup_hook_test.yaml‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
suite: harbor post-delete cleanup hook
2+
templates:
3+
- templates/hooks/cleanup.yaml
4+
5+
release:
6+
name: harbor-test
7+
namespace: tenant-root
8+
9+
# The cleanup hook scopes its ACME solver sweep to this release's TLS host,
10+
# which defaults to "<release>.<_namespace.host>".
11+
set:
12+
_namespace:
13+
host: example.org
14+
15+
tests:
16+
- it: renders the cleanup Job as a post-delete hook
17+
documentIndex: 0
18+
asserts:
19+
- equal:
20+
path: kind
21+
value: Job
22+
- equal:
23+
path: metadata.name
24+
value: harbor-test-cleanup
25+
- equal:
26+
path: metadata.annotations["helm.sh/hook"]
27+
value: post-delete
28+
- equal:
29+
path: spec.template.spec.serviceAccountName
30+
value: harbor-test-cleanup
31+
32+
- it: targets both kept PVCs (jobservice + trivy) via the shared release label and ACME objects
33+
documentIndex: 0
34+
asserts:
35+
# The upstream Harbor subchart runs as the "<release>-system"
36+
# HelmRelease. Its kept PVCs must be selected by release=<release>-system:
37+
# jobservice carries it via "harbor.labels", and trivy's StatefulSet PVC
38+
# carries it via "harbor.legacy.labels". The trivy PVC does NOT carry
39+
# app.kubernetes.io/instance, so selecting on that label would silently
40+
# miss trivy (which is enabled by default).
41+
- matchRegex:
42+
path: spec.template.spec.containers[0].command[2]
43+
pattern: kubectl delete pvc[\s\S]*-l release=harbor-test-system
44+
# Guard against regressing to the instance label, which misses trivy.
45+
- notMatchRegex:
46+
path: spec.template.spec.containers[0].command[2]
47+
pattern: app.kubernetes.io/instance=harbor-test-system
48+
- matchRegex:
49+
path: spec.template.spec.containers[0].command[2]
50+
pattern: harbor-test-ingress-tls
51+
52+
- it: scopes the ACME solver sweep to this release's TLS host
53+
documentIndex: 0
54+
asserts:
55+
# The solver sweep must be scoped to this release's host, not a
56+
# namespace-wide label sweep, so sibling apps' in-flight challenges in
57+
# the shared tenant namespace are never touched.
58+
- matchRegex:
59+
path: spec.template.spec.containers[0].command[2]
60+
pattern: HOST="harbor-test\.example\.org"
61+
# Challenges are filtered by .spec.dnsName == $HOST.
62+
- matchRegex:
63+
path: spec.template.spec.containers[0].command[2]
64+
pattern: \@\.spec\.dnsName==.\$HOST
65+
# Solver Ingresses are filtered by .spec.rules[0].host == $HOST.
66+
- matchRegex:
67+
path: spec.template.spec.containers[0].command[2]
68+
pattern: \@\.spec\.rules\[0\]\.host==.\$HOST
69+
# There must be NO namespace-wide blanket delete that targets every
70+
# object carrying the http01-solver label. The label may still appear
71+
# in the scoped `kubectl get ingress -l ...` selector, but it must
72+
# never be used in a `kubectl delete` against the whole namespace.
73+
- notMatchRegex:
74+
path: spec.template.spec.containers[0].command[2]
75+
pattern: kubectl delete[^\n]*-l acme.cert-manager.io/http01-solver=true
76+
77+
- it: sets resource requests and limits on the cleanup container
78+
documentIndex: 0
79+
asserts:
80+
- equal:
81+
path: spec.template.spec.containers[0].resources.requests.cpu
82+
value: 10m
83+
- equal:
84+
path: spec.template.spec.containers[0].resources.requests.memory
85+
value: 64Mi
86+
- equal:
87+
path: spec.template.spec.containers[0].resources.limits.cpu
88+
value: 200m
89+
- equal:
90+
path: spec.template.spec.containers[0].resources.limits.memory
91+
value: 128Mi
92+
93+
- it: creates a ServiceAccount for the cleanup job
94+
documentIndex: 1
95+
asserts:
96+
- equal:
97+
path: kind
98+
value: ServiceAccount
99+
- equal:
100+
path: metadata.name
101+
value: harbor-test-cleanup
102+
103+
- it: grants RBAC for PVCs and cert-manager ACME resources
104+
documentIndex: 2
105+
asserts:
106+
- equal:
107+
path: kind
108+
value: Role
109+
- equal:
110+
path: metadata.name
111+
value: harbor-test-cleanup
112+
- contains:
113+
path: rules
114+
content:
115+
apiGroups: [""]
116+
resources: ["persistentvolumeclaims"]
117+
verbs: ["get", "list", "delete"]
118+
- contains:
119+
path: rules
120+
content:
121+
apiGroups: ["cert-manager.io"]
122+
resources: ["certificates"]
123+
verbs: ["get", "list", "delete"]
124+
# Only "challenges" are granted: the host-scoped sweep lists/deletes
125+
# them, while Orders are cascade-deleted by cert-manager itself when the
126+
# Certificate is removed, so the hook needs no RBAC for orders.
127+
- contains:
128+
path: rules
129+
content:
130+
apiGroups: ["acme.cert-manager.io"]
131+
resources: ["challenges"]
132+
verbs: ["get", "list", "delete"]
133+
- notContains:
134+
path: rules
135+
content:
136+
apiGroups: ["acme.cert-manager.io"]
137+
resources: ["orders", "challenges"]
138+
verbs: ["get", "list", "delete"]
139+
# We list+delete orphaned http01 solver Ingresses by name, so we need
140+
# get/list/delete on networking.k8s.io ingresses.
141+
- contains:
142+
path: rules
143+
content:
144+
apiGroups: ["networking.k8s.io"]
145+
resources: ["ingresses"]
146+
verbs: ["get", "list", "delete"]
147+
# The hook must not grant services/pods: solver Pods/Services cascade
148+
# from the Challenge/Certificate and are never deleted directly.
149+
- notContains:
150+
path: rules
151+
content:
152+
apiGroups: [""]
153+
resources: ["services"]
154+
verbs: ["get", "list", "delete"]
155+
- notContains:
156+
path: rules
157+
content:
158+
apiGroups: [""]
159+
resources: ["pods"]
160+
verbs: ["get", "list", "delete"]
161+
162+
- it: binds the Role to the cleanup ServiceAccount
163+
documentIndex: 3
164+
asserts:
165+
- equal:
166+
path: kind
167+
value: RoleBinding
168+
- equal:
169+
path: roleRef.name
170+
value: harbor-test-cleanup
171+
- equal:
172+
path: subjects[0].name
173+
value: harbor-test-cleanup

0 commit comments

Comments
 (0)