[Backport release-1.6] fix(kafka): delete release topics before the topic operator is removed - #4456

The post-delete absence assertions could pass vacuously if the two
release topics were never created. Asserting both topics first makes
the E2E prove that the pre-delete cleanup actually removed them.

Signed-off-by: Yan Bondarenko <[email protected]>
(cherry picked from commit 9d2fddd)
Signed-off-by: Myasnikov Daniil <[email protected]>

Signed-off-by: Yan Bondarenko <[email protected]>
(cherry picked from commit 210b7fe)
Signed-off-by: Myasnikov Daniil <[email protected]>

The hook's exit code is the whole contract: a non-zero one returns from
Uninstall.Run before any release resource goes, keeping the topic operator
alive; a zero one lets Helm remove it. delete_test.yaml matches manifest text,
so it stayed green when the timeout branch was put back to exit 1, when the
CRD-absent branch was, and when the selector was widened past this release.

Add a bats suite in the shape hack/app-cleanup-hooks_test.bats already uses for
qdrant and harbor: render the script, run it against a fake kubectl, pin the
exit code of each branch. Each of the three mutations above now turns it red.

Drop hook-succeeded from the delete policy and reclaim the Job with
ttlSecondsAfterFinished instead. The script exits 0 on the degraded path too,
where topics still hold the finalizer, so deleting on success removed the only
record of that seconds after it was written. Assert both in delete_test.yaml.

Pin the hook image by shape rather than by literal digest: Renovate rewrites
digests under packages/*/templates/hooks/ and nothing rewrites tests/, so the
literal turns the next bump red for no defect.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 3326e0f)
Signed-off-by: Myasnikov Daniil <[email protected]>

The suite used bats' run helper. make bats-unit-tests does not invoke the bats
binary: it runs hack/cozytest.sh over every hack/*.bats, and that runner
implements @test and little else, so the file died at 'run: command not found'
and took make unit-tests with it. Under bats it was 5/5, which is where the 5/5
in the previous commit came from. Save $? by hand instead, the shape
app-cleanup-hooks_test.bats already uses; verified under both runners.

The fake kubectl no longer defaults DELETE_RC to 0. An unset value made every
failure-path assertion vacuous; now it exits 64 and says so. Cover both
RESTMapper wordings for an absent CRD rather than just the first.

Widen the image pattern to v[0-9]+\.[0-9]+(\.[0-9]+)? like harbor and mariadb:
pinning v1\.32 reintroduced the breakage the digest was made pattern-based to
avoid, one Renovate bump later. Assert the _cluster.images-registry override
and the no-leading-slash default, neither of which was covered: the hook is the
one image in this chart whose pull failure surfaces at uninstall.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 415aa82)
Signed-off-by: Myasnikov Daniil <[email protected]>

Both suites matched "strimzi.io/cluster=kafka-test" as a bare substring,
and that is a prefix of the sibling release kafka-test2 and of
kafka-test-topics. Rendering the hook with either selector left both
suites green, so neither pinned the scoping they exist to pin. Anchor on
the leading "-l " and on the trailing delimiter; both variants now red in
both suites, and the six earlier hook mutations still red exactly where
they did.

Drop the notMatchRegex on a leading slash. It cannot fail on its own: the
anchored image pattern above rejects a leading slash first, and an empty
_cluster.images-registry renders no slash to begin with. Its rationale
moves into that pattern's comment.

Record why activeDeadlineSeconds and the 180s wait have to move together.
A deadline that fires mid-wait kills the Job, and with backoffLimit: 0 the
hook then returns non-zero, which is the wedged uninstall the deliberate
exit 0 on the timeout branch exists to avoid.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 637098f)
Signed-off-by: Myasnikov Daniil <[email protected]>

The Job carries no hook-succeeded, so it outlives the hook run and
ttlSecondsAfterFinished reclaims it, which is what leaves a readable
record when the finalizer outlives the wait. Its ServiceAccount, Role and
RoleBinding still carried hook-succeeded and hook-failed, so Helm reaped
them at the end of the run either way and left the Job behind them.

A Job whose ServiceAccount is gone still starts a pod. kubelet mounts the
projected volume without a token, client-go finds no in-cluster config,
kubectl falls back to localhost:8080, and the script takes its catch-all
branch and exits 1. A non-zero pre-delete hook returns from Uninstall.Run
before any resource goes, so the release never finishes deleting and
helm-controller retries it: the e2e run on the merge commit shows six
such pods in five minutes, in two namespaces, every one of them dead the
same way. The hook meant to prevent one stuck Kafka release was instead
stopping all of them.

Give all four objects the same before-hook-creation policy, so the
credentials live exactly as long as the consumer and the next run
replaces them. Both other in-tree hooks that reach the apiserver,
apps/kubernetes and apps/harbor, are symmetric this way already.

The suite now pins the policy on each of the three, because nothing
compared them: the asymmetry rendered cleanly, passed 37 helm-unittest
cases and 5 bats cases, and only a cluster showed it.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 64d5689)
Signed-off-by: Myasnikov Daniil <[email protected]>

…hook

A non-zero --request-timeout makes kubectl skip its in-cluster credentials
altogether. client-go consults them only while the assembled config still
equals the default, and the flag lands in that config, so the branch that
would load the ServiceAccount token never runs. kubectl falls back to its
built-in default cluster, localhost:8080, is refused, and the script reads
the refusal as a failed delete and exits 1. A non-zero pre-delete hook
returns from Uninstall.Run before any resource is removed, so no Kafka
release could finish deleting and helm-controller retried forever.

Measured on the pinned image rather than argued from the docs: with the
token and ca.crt mounted and KUBERNETES_SERVICE_HOST set, the same delete
reaches the cluster without the flag, goes to localhost:8080 with
--request-timeout=20s, goes to localhost:8080 with 1m, and reaches the
cluster again with 0.

The three in-tree hooks that work do not pass it. apps/kubernetes names it
in a comment only, apps/harbor and apps/mariadb never mention it, and
apps/tenant records that its DELETE calls "carry kubectl's default
--request-timeout of 0". The watch stays bounded by --timeout, and the Job
by activeDeadlineSeconds, which is that same shape.

The suite now rejects the flag. It needs the "=" because the explanation
in the script names the flag too, and matching the bare name would fail on
the comment rather than on a use.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit e63b859)
Signed-off-by: Myasnikov Daniil <[email protected]>

(cherry picked from commit a295e29)
Signed-off-by: Myasnikov Daniil <[email protected]>

…fferently

The policies are right and nothing here changes them. What is missing is
the reason, and this file has now been changed in both directions inside
a week by people reasoning from the annotations alone, so the reason is
the part worth keeping.

The Job carries no hook-succeeded because the script exits 0 on the
degraded path as well as the clean one, and its log is the only record
that a finalizer outlived the wait; ttlSecondsAfterFinished reaps it on a
timescale a human can act within. The ServiceAccount, Role and
RoleBinding have no such clock, and before-hook-creation fires on a next
activation that after the last uninstall never comes, so without a
terminal policy they sit in the tenant namespace for the life of the
cluster still granting delete over every KafkaTopic in it.

Reaping them cannot strand a running pod, which is the objection the
symmetrical shape was written against: helm applies hook-succeeded only
once the hook is terminal, and activeDeadlineSeconds ends the Job inside
the uninstall hook timeout. hook-failed would decide nothing, because a
failing hook already reaps the earlier successful ones under
hook-succeeded.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 3a43bcf)
Signed-off-by: Myasnikov Daniil <[email protected]>