Skip to content

Commit 9a348ee

Browse files
authored
fix(kubernetes): delete orphaned resources in new post-delete job (#3856)
## What this PR does When tenant deletes the kubernetes cluster, 3 resources are orphaned and leftover in the tenant namespace. - jobs/<name>-oidc-bootstrap - secrets/<name>-talos-ca - secrets/<name>-talos-tls-cert This PR adds a new "step 7" in existing `packages/apps/kubernetes/templates/delete.yaml` in order to delete these orphaned resources manually. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(kubernetes): Fixed the `<name>-talos-ca` and `<name>-talos-tls-cert` Secrets and the `<name>-oidc-bootstrap` Job being left in the tenant namespace after a Kubernetes cluster is deleted. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Kubernetes release cleanup now removes the Talos CA and TLS certificates before their associated Secrets, and also removes the OIDC bootstrap Job. * The final cleanup report now includes best-effort failures from Step 7, alongside failures from Steps 3, 5, and 6. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 84e5e94 + 904bc38 commit 9a348ee

3 files changed

Lines changed: 119 additions & 5 deletions

File tree

‎hack/kubernetes-pre-delete-hook.bats‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ run_hook() {
382382
# still end the run under `set -e`. Nothing but `set -e` enforces that, so a
383383
# `|| true` appended to one of them later would reinstate the exact defect
384384
# this file pins: a cleanup that did not happen, reported as success. Step 3
385-
# stands in for Steps 3, 5, 6 and the patch inside Step 1. Step 1's
385+
# stands in for Steps 3, 5, 6, 7 and the patch inside Step 1. Step 1's
386386
# enumeration is NOT among them: it carries `|| true` on purpose, because
387387
# Step 2 deletes by label rather than from that list, so a failure there
388388
# leaves nothing behind to report.

‎packages/apps/kubernetes/templates/delete.yaml‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ spec:
9090
# final message as their only report. Record what those steps
9191
# leave behind so that message can name it instead of claiming
9292
# success in exactly the case the warnings were written for. The
93-
# steps that are NOT backstopped — Steps 3, 5 and 6, and the patch
93+
# steps that are NOT backstopped — Steps 3, 5, 6 and 7, and the patch
9494
# inside Step 1 — end the run under set -e and never reach that
9595
# message at all.
9696
#
@@ -404,6 +404,35 @@ spec:
404404
--field-selector spec.type=LoadBalancer \
405405
--ignore-not-found=true --wait=false
406406

407+
echo "Step 7: Deleting the Talos PKI and the OIDC bootstrap Job, which outlive the release"
408+
# cert-manager runs with enableCertificateOwnerRef: false, so the
409+
# two Talos Secrets carry no ownerReference to their Certificates
410+
# and survive the uninstall, holding the CA private key of a
411+
# cluster that no longer exists. A new cluster of the same name
412+
# would then adopt that stale CA instead of issuing its own.
413+
#
414+
# The Certificates go first: deleting a Secret whose Certificate
415+
# still exists is how cert-manager is told to re-issue it, so the
416+
# other order brings the Secret back with a fresh key, orphaned
417+
# all the same. Neither carries a finalizer, so --wait=false
418+
# still leaves each one gone before the next call.
419+
#
420+
# After Step 4, because the control plane's CSR signer mounts the
421+
# CA and the TenantControlPlane is what runs it.
422+
#
423+
# The OIDC bootstrap Job is a hook with before-hook-creation
424+
# only, and hooks are not in the release manifest, so nothing
425+
# else ever removes it.
426+
kubectl -n {{ .Release.Namespace }} delete certificates.cert-manager.io \
427+
{{ .Release.Name }}-talos-ca {{ .Release.Name }}-talos-tls-cert \
428+
--ignore-not-found=true --wait=false
429+
kubectl -n {{ .Release.Namespace }} delete secrets \
430+
{{ .Release.Name }}-talos-ca {{ .Release.Name }}-talos-tls-cert \
431+
--ignore-not-found=true --wait=false
432+
kubectl -n {{ .Release.Namespace }} delete jobs.batch \
433+
{{ .Release.Name }}-oidc-bootstrap \
434+
--ignore-not-found=true --wait=false
435+
407436
if [ -n "$residue_from" ]; then
408437
echo "Cleanup incomplete; residue left by: $residue_from" >&2
409438
echo "Objects named in the warnings above may need removing by hand; exiting 0 so the release can still be uninstalled" >&2
@@ -446,7 +475,7 @@ rules:
446475
- patch
447476
- delete
448477
# watch is granted only where a delete still waits: Step 2 on helmreleases and
449-
# Step 4 on tenantcontrolplanes. The other three deletes pass --wait=false, so
478+
# Step 4 on tenantcontrolplanes. The other deletes pass --wait=false, so
450479
# kubectl never opens a watch for them and the verb would be unexercised.
451480
- apiGroups:
452481
- "controlplane.cluster.x-k8s.io"
@@ -499,6 +528,37 @@ rules:
499528
verbs:
500529
- get
501530
- patch
531+
# Step 7. Named rather than granted on the kind: the tenant namespace holds
532+
# other Certificates, Secrets and Jobs that this hook must not reach.
533+
- apiGroups:
534+
- "cert-manager.io"
535+
resources:
536+
- certificates
537+
resourceNames:
538+
- {{ .Release.Name }}-talos-ca
539+
- {{ .Release.Name }}-talos-tls-cert
540+
verbs:
541+
- get
542+
- delete
543+
- apiGroups:
544+
- ""
545+
resources:
546+
- secrets
547+
resourceNames:
548+
- {{ .Release.Name }}-talos-ca
549+
- {{ .Release.Name }}-talos-tls-cert
550+
verbs:
551+
- get
552+
- delete
553+
- apiGroups:
554+
- "batch"
555+
resources:
556+
- jobs
557+
resourceNames:
558+
- {{ .Release.Name }}-oidc-bootstrap
559+
verbs:
560+
- get
561+
- delete
502562
---
503563
apiVersion: rbac.authorization.k8s.io/v1
504564
kind: RoleBinding

‎packages/apps/kubernetes/tests/delete_hook_test.yaml‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ tests:
134134
# failure it is written to prevent would take the assertion with it.
135135
# Matching the entire rule pins the verb set in both directions and names
136136
# the resource when it fails; lengthEqual is what closes the remaining
137-
# hole, a sixth rule appended past everything asserted here.
137+
# hole, a tenth rule appended past everything asserted here.
138138
- lengthEqual:
139139
path: rules
140-
count: 6
140+
count: 9
141141
- contains:
142142
path: rules
143143
content:
@@ -169,6 +169,60 @@ tests:
169169
resources: ["services"]
170170
verbs: ["get", "list", "delete"]
171171

172+
# Step 7 removes what outlives the release: the two Talos PKI Secrets, which
173+
# cert-manager issues without an ownerReference to their Certificates, and the
174+
# OIDC bootstrap Job, a hook whose only delete policy is before-hook-creation.
175+
- it: deletes the Talos Certificates before their Secrets, so cert-manager cannot re-issue them
176+
documentSelector:
177+
path: kind
178+
value: Job
179+
asserts:
180+
- matchRegex:
181+
path: spec.template.spec.containers[0].command[2]
182+
pattern: 'delete certificates\.cert-manager\.io \\\n\s+test-k8s-talos-ca test-k8s-talos-tls-cert(?:[^\n]*\\\n)*[^\n]*--wait=false'
183+
- matchRegex:
184+
path: spec.template.spec.containers[0].command[2]
185+
pattern: 'delete secrets \\\n\s+test-k8s-talos-ca test-k8s-talos-tls-cert(?:[^\n]*\\\n)*[^\n]*--wait=false'
186+
- notMatchRegex:
187+
path: spec.template.spec.containers[0].command[2]
188+
pattern: 'delete secrets \\\n\s+test-k8s-talos-ca[\s\S]*delete certificates\.cert-manager\.io'
189+
190+
- it: deletes the orphaned OIDC bootstrap Job
191+
documentSelector:
192+
path: kind
193+
value: Job
194+
asserts:
195+
- matchRegex:
196+
path: spec.template.spec.containers[0].command[2]
197+
pattern: 'delete jobs\.batch \\\n\s+test-k8s-oidc-bootstrap(?:[^\n]*\\\n)*[^\n]*--wait=false'
198+
199+
- it: cleanup Role can delete only the Step 7 objects, by name
200+
documentSelector:
201+
path: kind
202+
value: Role
203+
asserts:
204+
- contains:
205+
path: rules
206+
content:
207+
apiGroups: ["cert-manager.io"]
208+
resources: ["certificates"]
209+
resourceNames: ["test-k8s-talos-ca", "test-k8s-talos-tls-cert"]
210+
verbs: ["get", "delete"]
211+
- contains:
212+
path: rules
213+
content:
214+
apiGroups: [""]
215+
resources: ["secrets"]
216+
resourceNames: ["test-k8s-talos-ca", "test-k8s-talos-tls-cert"]
217+
verbs: ["get", "delete"]
218+
- contains:
219+
path: rules
220+
content:
221+
apiGroups: ["batch"]
222+
resources: ["jobs"]
223+
resourceNames: ["test-k8s-oidc-bootstrap"]
224+
verbs: ["get", "delete"]
225+
172226
- it: pre-delete Job makes a single attempt
173227
documentSelector:
174228
path: kind

0 commit comments

Comments
 (0)