mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
fix(bucket): harden ACME cleanup hook and fix cert-manager label selector
Address review feedback on the post-delete cleanup hook: - Target Orders/Challenges by cert-manager's own cert-manager.io/certificate-name=<release>-ui-tls label instead of the Helm app.kubernetes.io/instance label, which cert-manager never sets on those resources (the old selector was a silent no-op). - Bound the Job with activeDeadlineSeconds=120 and backoffLimit=0 so a stuck finalizer or hung API call cannot block the uninstall. - Add --wait=false --request-timeout=30s to every kubectl delete/get so the hook never hangs on slow or finalizer-blocked deletes. - Surface list failures in the job log instead of redirecting to /dev/null. - Tests: assert the bounded execution and security-hardening contract, pin the rendered document count to 4, and update selector assertions. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
This commit is contained in:
1 parent
4425d3afa7
commit
58b2a3d779
2 files changed
+55
-9
No files matched your search
@@ -10,6 +10,10 @@ metadata:
|
||||
"helm.sh/hook-weight": "10"
|
||||
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
|
||||
spec:
|
||||
# Bound the whole cleanup: never let a stuck finalizer or a hung API call
|
||||
# leave this post-delete hook running indefinitely and block the uninstall.
|
||||
activeDeadlineSeconds: 120
|
||||
backoffLimit: 0
|
||||
template:
|
||||
metadata:
|
||||
labels:
|
||||
@@ -59,9 +63,13 @@ spec:
|
||||
# then cascade-removes the cm-acme-http-solver-* solver objects it
|
||||
# owns. The release's TLS Certificate is named after the secret it
|
||||
# populates ({{ .Release.Name }}-ui-tls).
|
||||
kubectl delete certificate {{ .Release.Name }}-ui-tls -n {{ .Release.Namespace }} --ignore-not-found \
|
||||
kubectl delete certificate {{ .Release.Name }}-ui-tls -n {{ .Release.Namespace }} --ignore-not-found --wait=false --request-timeout=30s \
|
||||
|| echo "WARNING: cert-manager cleanup failed for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
|
||||
kubectl delete certificate,order,challenge -n {{ .Release.Namespace }} -l app.kubernetes.io/instance={{ .Release.Name }} --ignore-not-found \
|
||||
# Orders/Challenges do NOT carry the Helm app.kubernetes.io/instance
|
||||
# label; cert-manager stamps them with its own
|
||||
# cert-manager.io/certificate-name label, so target them by the
|
||||
# Certificate name to reliably reap any that outlived the cascade.
|
||||
kubectl delete order,challenge -n {{ .Release.Namespace }} -l cert-manager.io/certificate-name={{ .Release.Name }}-ui-tls --ignore-not-found --wait=false --request-timeout=30s \
|
||||
|| echo "WARNING: cert-manager cleanup failed for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
|
||||
# Release-scoped solver sweep. We deliberately do NOT delete
|
||||
# ingress/service/pod objects namespace-wide by the
|
||||
@@ -73,8 +81,10 @@ spec:
|
||||
#
|
||||
# Delete Challenges for our host (cascades the solver
|
||||
# Ingress/Service/Pod they own).
|
||||
for ch in $(kubectl get challenge -n {{ .Release.Namespace }} -o jsonpath="{range .items[?(@.spec.dnsName=='$HOST')]}{.metadata.name} {end}" 2>/dev/null); do
|
||||
kubectl delete challenge -n {{ .Release.Namespace }} "$ch" --ignore-not-found \
|
||||
challenges="$(kubectl get challenge -n {{ .Release.Namespace }} --request-timeout=30s -o jsonpath="{range .items[?(@.spec.dnsName=='$HOST')]}{.metadata.name} {end}")" \
|
||||
|| { echo "WARNING: failed to list challenges for {{ .Release.Name }}; orphaned ACME resources may remain" >&2; challenges=""; }
|
||||
for ch in $challenges; do
|
||||
kubectl delete challenge -n {{ .Release.Namespace }} "$ch" --ignore-not-found --wait=false --request-timeout=30s \
|
||||
|| echo "WARNING: failed to delete challenge $ch for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
|
||||
done
|
||||
# Delete any orphaned solver Ingress for our host (no live owner
|
||||
@@ -82,8 +92,10 @@ spec:
|
||||
# Challenge and are cascade-deleted with it; truly owner-less
|
||||
# solver Service/Pod with no matching Challenge are not expected
|
||||
# and are not swept namespace-wide.
|
||||
for ing in $(kubectl get ingress -n {{ .Release.Namespace }} -l acme.cert-manager.io/http01-solver=true -o jsonpath="{range .items[?(@.spec.rules[0].host=='$HOST')]}{.metadata.name} {end}" 2>/dev/null); do
|
||||
kubectl delete ingress -n {{ .Release.Namespace }} "$ing" --ignore-not-found \
|
||||
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}")" \
|
||||
|| { echo "WARNING: failed to list solver ingresses for {{ .Release.Name }}; orphaned ACME resources may remain" >&2; ingresses=""; }
|
||||
for ing in $ingresses; do
|
||||
kubectl delete ingress -n {{ .Release.Namespace }} "$ing" --ignore-not-found --wait=false --request-timeout=30s \
|
||||
|| echo "WARNING: failed to delete solver ingress $ing for {{ .Release.Name }}; orphaned ACME resources may remain" >&2
|
||||
done
|
||||
echo "ACME cleanup complete."
|
||||
|
||||
@@ -11,6 +11,11 @@ set:
|
||||
host: example.org
|
||||
|
||||
tests:
|
||||
- it: renders only the expected cleanup hook objects (Job, SA, Role, RoleBinding)
|
||||
asserts:
|
||||
- hasDocuments:
|
||||
count: 4
|
||||
|
||||
- it: renders a post-delete cleanup Job with the expected name and SA
|
||||
documentIndex: 0
|
||||
asserts:
|
||||
@@ -32,6 +37,29 @@ tests:
|
||||
- equal:
|
||||
path: spec.template.metadata.labels["policy.cozystack.io/allow-to-apiserver"]
|
||||
value: "true"
|
||||
# Bounded execution so a stuck hook can't block the uninstall.
|
||||
- equal:
|
||||
path: spec.activeDeadlineSeconds
|
||||
value: 120
|
||||
- equal:
|
||||
path: spec.backoffLimit
|
||||
value: 0
|
||||
# Security hardening contract the PR relies on.
|
||||
- equal:
|
||||
path: spec.template.spec.securityContext.runAsNonRoot
|
||||
value: true
|
||||
- equal:
|
||||
path: spec.template.spec.containers[0].securityContext.readOnlyRootFilesystem
|
||||
value: true
|
||||
- equal:
|
||||
path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation
|
||||
value: false
|
||||
- equal:
|
||||
path: spec.template.spec.containers[0].securityContext.capabilities.drop[0]
|
||||
value: ALL
|
||||
- equal:
|
||||
path: spec.template.spec.securityContext.seccompProfile.type
|
||||
value: RuntimeDefault
|
||||
|
||||
- it: deletes release-scoped cert-manager resources with a host-scoped solver sweep
|
||||
documentIndex: 0
|
||||
@@ -39,9 +67,15 @@ tests:
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: "kubectl delete certificate my-bucket-ui-tls -n tenant-root"
|
||||
# Orders/Challenges are targeted by cert-manager's own
|
||||
# cert-manager.io/certificate-name label, not the Helm release label.
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: "kubectl delete certificate,order,challenge -n tenant-root -l app.kubernetes.io/instance=my-bucket"
|
||||
pattern: "kubectl delete order,challenge -n tenant-root -l cert-manager.io/certificate-name=my-bucket-ui-tls"
|
||||
# The release label selector must NOT be used for cert-manager resources.
|
||||
- notMatchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: "delete .*-l app.kubernetes.io/instance=my-bucket"
|
||||
# Host derived from <release>.<_namespace.host> == my-bucket.example.org
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
@@ -49,14 +83,14 @@ tests:
|
||||
# Host-scoped challenge sweep filtering on .spec.dnsName == our host
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: "kubectl get challenge -n tenant-root -o jsonpath=.*@.spec.dnsName=='\\$HOST'"
|
||||
pattern: "kubectl get challenge -n tenant-root --request-timeout=30s -o jsonpath=.*@.spec.dnsName=='\\$HOST'"
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: 'kubectl delete challenge -n tenant-root "\$ch" --ignore-not-found'
|
||||
# Host-scoped orphaned solver Ingress sweep filtering on rules[0].host
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: "kubectl get ingress -n tenant-root -l acme.cert-manager.io/http01-solver=true -o jsonpath=.*@.spec.rules\\[0\\].host=='\\$HOST'"
|
||||
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'"
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].command[2]
|
||||
pattern: 'kubectl delete ingress -n tenant-root "\$ing" --ignore-not-found'
|
||||
|
||||
Reference in new issue
Block a user