fix(apps): let the cleanup hooks watch the objects they delete - #4135

NOT LGTM — one MAJOR blocker (independent cozy-review pass).

The diagnosis and the RBAC shape are right: kubectl delete defaults to --wait=true and the wait needs watch, so the missing verb made the hook skip the wait and report "complete" while PVCs were still terminating. watch is added to all Roles and the tests are green (clickhouse 3/3, mariadb 4/4).

[MAJOR] Enabling the real wait makes the post-delete hooks an unbounded teardown blocker with no limit added. clickhouse cleanup-pvc.yaml:54, harbor cleanup.yaml:106, mariadb cleanup-pvc.yaml:61, qdrant cleanup-pvc.yaml:107 all run kubectl delete pvc with the default --wait=true and no --timeout, and none of the Jobs set activeDeadlineSeconds. Before this diff the missing verb made kubectl skip the wait, so the hang was unreachable; the diff makes the wait real and the hazard attributable to this change. A stuck pvc-protection finalizer (node NotReady, stuck detach — a real LINSTOR/DRBD situation) blocks the delete forever, the Job hangs to the hook timeout, and the HelmRelease gets stuck uninstalling. Fix: a bounded wait — --timeout=<N> on the delete and/or activeDeadlineSeconds (60-300s) on the Job. Note --wait=false alone would reintroduce the original "success while terminating" bug.

[MINOR] The watch added to the mariadb resourceNames-scoped secrets rule (cleanup-pvc.yaml:135) is a dead grant: those secrets have no finalizer (deleted synchronously, watch phase never reached) and a resourceNames rule does not authorize a collection watch anyway. Remove it.

Verdict

LGTM with non-blocking notes

This is a re-review. My own prior pass (2026-09-17, on 711fcc168) was NOT LGTM with one MAJOR blocker: enabling the real wait (granting watch) turned the PVC deletes into an unbounded teardown blocker with no limit added, so a stuck pvc-protection finalizer would hang the hook to the timeout and leave the HelmRelease stuck uninstalling. The new commit f886f26ad closes it exactly as requested — --wait=true --timeout=120s on every PVC delete plus activeDeadlineSeconds: 270 and terminationGracePeriodSeconds: 10 on every Job, the same bounded-wait shape packages/apps/tenant/templates/cleanup-job.yaml already documents against the helm-controller 300s uninstall budget. My prior MINOR (a dead watch grant on the mariadb resourceNames-scoped Secrets rule) is also gone: that rule is back to ["get","delete"]. The reversal lands on changed code, not on the revision I blocked, so the flip is earned.

Verified hermetically on f886f26ad: helm unittest green (clickhouse 24/24, harbor 21/21, mariadb 111/111, qdrant 104/104) and the behavioural suite hack/app-cleanup-hooks_test.bats 11/11. The new assertions are not inert — mutating clickhouse (dropping --wait=true --timeout=120s and changing 270→260) turns the suite red on both the activeDeadlineSeconds equal and the command matchRegex, and restoring goes green.

The diagnosis and the RBAC shape are correct: kubectl delete defaults to --wait=true, that wait is a watch, and the missing verb made the hook skip the wait and report "complete" while claims were still terminating. watch is added only to the collection-scoped rules that sit behind a delete (PVCs in all four; certificates/challenges/solver ingresses in harbor), and the mariadb named-Secret rule correctly keeps get/delete only — least privilege is preserved and everything stays namespace-scoped, so there is no tenant-isolation concern.

Findings

[MINOR] clickhouse & mariadb cleanup Jobs keep the default backoffLimit (6) — a stuck PVC now surfaces as DeadlineExceeded, not the hook's own error. packages/apps/clickhouse/templates/hooks/cleanup-pvc.yaml:15, packages/apps/mariadb/templates/hooks/cleanup-pvc.yaml:12. harbor and qdrant deliberately set backoffLimit: 1 with a comment explaining that six retries would outlast the hook timeout and make "Helm report that timeout instead of the hook's own error." This PR is what makes that failure mode reachable for the other two charts: with the new 120s wait, a genuinely stuck claim burns ~120s per attempt, and under the default backoffLimit: 6 the Job hits activeDeadlineSeconds: 270 mid-second-attempt, so Helm sees DeadlineExceeded while the hook's named-PVC diagnostic (the whole point of this change) is buried in an earlier pod's log rather than being the proximate error. The 270s backstop still prevents the hang, so this is not a blocker — but adding backoffLimit: 1 to these two would make all four charts fail the same way and preserve the signal the PR is built to surface.

[NIT] harbor's Certificate / Challenge / solver-Ingress deletes now perform a real wait but carry no --timeout. packages/apps/harbor/templates/hooks/cleanup.yaml:126,153,163. With watch granted on these kinds, kubectl delete … --ignore-not-found (no --timeout) now waits with kubectl's default, which the tenant precedent documents as "a week." Only the PVC delete is explicitly bounded; these three rely solely on the Job's activeDeadlineSeconds. cert-manager objects rarely block on their finalizers and the 270s deadline caps the pathological case, so this is defensive completeness only — but bounding them (or a one-line note that the deadline is the intended bound for them) would match the PR's own "bound the wait" thesis.

Found outside this diff (non-blocking)

• clickhouse and qdrant pin the cleanup image as docker.io/clastix/kubectl:v1.32 (mutable tag, not routed through cozy-lib.image), whereas harbor and mariadb use the digest-pinned, registry-routed cozy-lib.image form. Mirrored / air-gapped installs would fail to pull the clickhouse/qdrant cleanup image at uninstall (then hit the 270s deadline). Pre-existing, untouched by this PR.

Static-checker false positives (rejected)

• 4× cleanup-hook-unbounded [MAJOR] (clickhouse/harbor/mariadb/qdrant): the heuristic wants --wait=false on every kubectl delete. The PR deliberately uses a bounded --wait=true --timeout=120s backed by activeDeadlineSeconds: 270 — which is both the only way to fix the actual bug (reporting complete-while-terminating) and the exact pattern packages/apps/tenant/templates/cleanup-job.yaml already establishes. The teardown-hang the invariant warns about is prevented because 270 + 10s grace < 300s helm budget. Rejected.
• 1× hook-succeeded-on-job [MINOR] (harbor): the message claims hook-succeeded GCs a failed Job immediately. It does not — hook-succeeded deletes only on success; failed Jobs are retained (only hook-failed would GC on failure, and harbor does not set it). The harbor script is explicitly designed around keeping failed Jobs for their logs. Also pre-existing (not in the diff). Rejected.

Phase 5b (upgrade + fresh install)

These are post-delete hook templates — they render only at uninstall, so a fresh install and an in-place upgrade of an existing release are both unaffected until the operator uninstalls, at which point the new hook RBAC + bounded Job render fresh. No default flip, no RBAC contraction on running workloads, no cold-start dependency. Nothing to migrate.