fix(ci): decide E2E test coverage per changed path - #3441

…gregated-API auth (#3444)

## What this PR does

Two unrelated defects made the sandbox install job's failure handling
unreliable. This fixes both.

The first hit every red run. When a test fails, the diagnostic snapshot
runs `crust-gather collect`, and its host-log leg spawns a privileged
debug pod per node. The sandbox runs baseline PodSecurity, which rejects
each of those pods with a 403, so crust-gather keeps retrying until it
burns its whole collection budget and exits 1. The snapshot verdict then
printed `FAILED` on every failed run even though the namespaced object
state was already captured, so the line that says whether the uploaded
snapshot is trustworthy was always wrong. The host-log leg collects
nothing usable here anyway, so this passes `--disable-additional-logs`
at all three collect sites: the host snapshot from the BATS runner, the
host snapshot from the Chainsaw error handler, and the nested tenant
snapshot. Object collection now finishes and the verdict again separates
a real collection failure from a complete snapshot.

The second is a race in the install test. The "Configure Tenant" step
issues its first `kubectl patch` of the root tenant as soon as an
earlier test sees the aggregated APIService go `Available`. That
condition only tracks endpoint availability, not the serving pod's auth
state. A freshly rolled cozystack-api pod can start answering before it
has loaded the requestheader client-CA, and until it does it drops the
front-proxy identity and answers as `system:anonymous`, so the patch is
rejected with a 403 on the test's very first command. This adds a short
authenticated GET against the same resource before the patch, so the
step proceeds only once the aggregated API both serves and authenticates
the request. If the aggregation layer is actually wedged past two
minutes the gate fails loud instead of hiding a real regression.

Both were traced from a recent sandbox failure (run 30004030331, during
PR #3441's CI).

Out of scope: the broader convergence flakiness of the install step is
not touched here. That covers the long reconcile windows for the storage
and platform releases and the app-level races in the Chainsaw suite. The
durable product-side fix for the second defect, where cozystack-api
should not report ready before the requestheader client-CA is loaded, is
a separate follow-up.

This partially addresses #3368, its crust-gather diagnostics item, and
does not close it.

### Screenshots

<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->

No UI changes.

### Downstream repositories

<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.

Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.

IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:

1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->

The diff touches only the e2e test harness (`hack/cozytest.sh`,
`hack/e2e-chainsaw/.chainsaw.yaml`,
`hack/e2e-chainsaw/_lib/run-kubernetes.sh`,
`hack/e2e-install-cozystack.bats`). The trigger map's `hack/` couplings
are `hack/e2e-prepare-cluster.bats` (node prerequisites for
ansible-cozystack and talm) and `hack/package.mk`,
`hack/common-envs.mk`, `hack/update-crd.sh` plus the package layout
(ccp, external-apps-example, cozyhr). None of those are moved, renamed,
or edited here, and nothing else in the map matches.

- [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

<!--  Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->

```release-note
fix(e2e): keep failure snapshots complete when PodSecurity blocks crust-gather's host-log leg, and wait for the aggregated API to authenticate before patching the root tenant so the install step no longer flakes on a system:anonymous 403
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Improved failure diagnostics during automated Kubernetes and tenant
tests by avoiding unsupported privileged log collection steps.
- Ensured failure snapshots complete more reliably instead of timing out
or returning incomplete results.
- Added a readiness check before configuring the root tenant, preventing
intermittent authorization failures during installation tests.

- **Tests**
- Increased reliability of installation and end-to-end test workflows
under baseline security policies.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

…#3622)

<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->

## What this PR does

The ban on EXIT-trap cleanup in `hack/*.bats` was enforced by two lists
inside `hack/cozyreport.bats`: one naming the files known to be clean,
one freezing an exact trap count for each file that was not. Both had to
be edited from whatever change moved them, and that is where the guard
kept failing.

It is worth being exact about how badly, because the history is sharper
than "it went stale a few times". **The inventory was never correct on
main for a single commit.** It landed in #3567 at 15:25:20. #3195 had
landed `multus-install-cni-plugins.bats` carrying twelve traps at
15:24:37, forty-three seconds earlier. Replaying the old guard's own
logic against the tree at the very commit that introduced it already
gives `found != frozen`. Main then stayed red for roughly twenty-two and
three quarter hours. The first repair, #3584, landed *already red* the
next morning at 10:42:43, because #3548 had brought in
`run-kubernetes-talos-diagnostics_test.bats` with eight traps at
10:40:03, under three minutes ahead of it. That fix was correct and
bought zero green time. Green arrived only with the second repair,
#3602.

Neither pair of PRs shared a line, and each was green against its own
base. That is the whole mechanism, and it is why a third one-line repair
is not the answer.

The mechanism is structural rather than careless. A change that adds a
trap to its own file had to edit a string in a suite it otherwise never
touches, so two changes sharing no line still invalidated each other:
each stayed green against its own base, git merged both cleanly, and the
guard went red only once the second one landed. A file *arriving* with
traps was worse still, because nothing in its author's diff pointed at
that string at all. That is exactly how
`run-kubernetes-talos-diagnostics_test.bats` got in, twice.

That contention is not in the past tense. Two open PRs are editing that
one line right now, and they disagree about what it should say: #3575
adds `run-kubernetes-talos-diagnostics_test.bats=8` to it, repeating a
repair that has already landed, and #3441 removes
`select-e2e_test.bats=15` from it, because it converts that file.
Neither PR is about EXIT traps. Both have to touch that string anyway,
and whichever lands second is wrong until someone edits it again.

So this PR is not fixing a red main. It removes the thing that keeps
making main red, which is why it is worth more than the one-line fix
that is now the established habit.

One honest caveat about its own landing. The textual-conflict property
is steady-state: a branch forked *before* the declaration exists has no
line to disagree with, so it converts traps in its own file, merges
clean, and the count only goes wrong once both sides are on main. I
checked this against #3441, which takes `select-e2e_test.bats` from
fifteen traps to zero. Merged after this, that file would declare
fifteen and hold none. What the move buys even in that case is that the
red names a file the branch already edited, the repair is one line
inside it, and rebasing before merge catches it on the branch's own CI.
None of those three held against the central inventory. Whoever merges
this should expect one such adjustment on the conversion branches still
in flight.

So this replaces both lists with a declaration each file makes about
itself: one `# EXIT-TRAP DEBT: N` comment in its leading comment block.
A file carrying no declaration must install no EXIT trap. Growing or
shedding a trap now fails in the file the change already edits, so two
changes that disagree about a count get a real textual conflict instead
of silently invalidating each other, and a change that leaves the traps
alone edits nothing.

**The include list is redundant, not lost.** It named the files proven
clean, so that a trap reappearing in one of them would fail. Under the
new rule those files carry no declaration, and a file with no
declaration must hold zero traps, so a trap reappearing in any of them
fails on its own, with no list to be on. Coverage widens rather than
narrows: the two lists named twenty files between them, and the rule
covers all fifty bats files under `hack/`, subdirectories included, plus
the ones added tomorrow.

Rebasing this branch onto current main is the property working. Main has
since gained `hack/kubernetes-pre-delete-hook.bats` and
`hack/tenant-pre-delete-hook.bats`, and `hack/cozyreport.bats` grew by
some eight hundred lines. Neither new file installs an EXIT trap, so
neither needed a declaration and neither needed an edit here; the rebase
took no conflict at all. Under the inventory, each arriving file was a
coin toss on whether somebody had remembered the string.

To be precise about what the inventory could and could not do, since it
is easy to overstate: it did *notice* a new file carrying traps. The
string it compared was built by scanning the directory, so an arriving
file appended a token and failed the comparison, which is exactly how
main went red. What it could not do is let that file arrive without an
edit in a foreign suite. Being seen and being absorbable are different
properties, and only the second one decides whether two changes can land
independently.

The declaration is pinned in both directions. Declaring N while holding
N+1 fails, obviously; declaring N while holding N−1 fails too. Without
that second half the number becomes a ceiling and rots upward: somebody
converts half a file, the declaration stays, and the guard quietly
licenses traps that were removed long ago.

It is read only from the leading comment block, under the shebang and
above the first line of code. A `.bats` file is shell that writes shell,
so the same line turns up inside a heredoc, a fixture writer or an
expected-output string, where it is data belonging to one test;
honouring it there would let an unrelated fixture excuse a real trap,
silently, with nothing in that test's own diff looking like a
declaration. A comment block is the region with no interior; stopping
instead at the first `@test` would still read a line out of a helper's
heredoc.

Not every counted handler is debt. A trap inside an explicit subshell
does not replace the one the `bats` binary installs, so a test failing
inside `( … )` still prints its `not ok`, checked against a test-level
trap in the same file, where the TAP line vanishes.
`hack/e2e-test-openapi.bats` kills a backgrounded `kubectl proxy`
exactly that way, and "convert it like the others" would leak a process
holding a fixed port and wedge the next run. Its declaration now records
the carve-out instead of scheduling a conversion, and because the
ratchet is exact in both directions, *removing* that trap fails too, so
the count protects the construct rather than marking it for deletion.
`docs/agents/e2e-testing.md` previously scoped this exception to
Chainsaw `script` steps only; it now names the BATS subshell case as
well.

The counting bounds the keyword and the signal the same way, at any
character that cannot be part of an identifier, and matches the signal
in either case. Whitespace on the right missed `trap … EXIT; cd "$tmp"`;
whitespace on the left missed `tmp=$(mktemp -d);trap … EXIT` and `(trap
… EXIT; true)`; upper case missed `trap … exit`, which bash and dash
both install. All of those are real handlers that scored zero. The left
boundary is the one worth dwelling on, because the inventory being
replaced had none at all and *did* catch the semicolon form. Getting it
wrong here would have narrowed coverage while the commit claimed to
widen it. A plain word boundary is not enough either: `bootstrap ` ends
in `trap `, and it has to keep scoring nothing, or the documented answer
to a red guard (add a debt line) would buy a file a permanent licence
for one real trap to silence a line that has none.

Two handlers sharing one line are reported rather than counted, because
the count is a count of lines and the second would otherwise arrive
free. Splitting such a line properly needs a shell parser, since a
semicolon inside a handler's own quoted action is not a separator, and
guessing wrong undercounts, the one direction a ratchet cannot afford.

**What this does not fix.** The declaration is still a loophole: a new
file can write `# EXIT-TRAP DEBT: 8` instead of cleaning up, and nothing
here makes that impossible. What changes is that the admission is local
and visible. It sits at the top of the file it excuses, in front of
whoever reviews that file, instead of being a number in a neighbouring
suite nobody in that review is reading. Today's loophole is the same
size and invisible.

Three more limits, all stated in the guard's own header rather than left
to be discovered. The scan is lexical, so a signal computed at runtime
and a quoted action spanning physical lines without a backslash are both
invisible. An exact count catches addition and removal but never
substitution: swap the openapi file's subshell trap for a test-level one
and the total stays 1. And the scan reads `.bats` only, so a handler
arriving through a sourced `.sh` is outside it.
`hack/e2e-chainsaw/_lib/run-kubernetes.sh` installs two right now, and
each is benign for its own reason rather than by design: the one in
`cozy_capture_tenant_talos` because that function is declared with `(`
and so runs in a subshell, the one in `run_kubernetes_test` because no
`@test` calls it despite being declared with `{`. Three `hack/*.bats`
source that library, and two tests in the converted file call
`cozy_capture_tenant_talos`, so flipping a single `(` to `{` reinstates
a test-level handler in both of them with the guard green. Widening the
scan to `.sh` would mean counting handlers that are correct in a script
and wrong only in a test body, so the honest answer is that this is
where the instrument stops.

Three further boundaries, recorded here so they land as known edges
rather than as surprises. The old include list also failed when a file
named on it disappeared from the tree; the new rule can only judge a
file that is present, so a deleted converted file goes unnoticed. That
is a genuinely smaller check, though its absence shows up in the diff
that deletes the file. The guard's own failure messages are code lines,
so they are scanned by the pattern they belong to: they pass today only
because no bare `EXIT` or `0` happens to follow the keyword in any of
them, and a rewording that introduced one would make the file demand a
debt of itself. It fails loudly rather than quietly, and the fixture
writers and test titles already split the keyword from the signal for
this reason, but the messages do not. Finally,
`docs/agents/e2e-testing.md` bans test-level `EXIT` *and* `RETURN`
traps, while every mechanical guard this repo has had, the one being
deleted included, matches only `EXIT` and `0`. `hack/` holds no RETURN
trap today, so nothing regresses here, but half of that documented rule
has never had an executor.

The guard moves out of `hack/cozyreport.bats` into
`hack/bats-no-exit-trap.bats`, because its subject is every unit suite
under `hack/` and not the report collector it grew up in. Living inside
`cozyreport.bats` is precisely why unrelated PRs kept converging on one
line. `hack/md-no-hardwrap.bats` is the neighbouring precedent.

Separately, the eight EXIT traps in
`hack/run-kubernetes-talos-diagnostics_test.bats` are converted to
end-of-body cleanup rather than given a declaration. That file was
written six weeks after the ban was documented, so "it predates the
rule" is not available to it, and handing it a debt is literally the
move that failed the last two times. The remaining files keep their
declarations, since a mass conversion would collide with the branches
that own them.

One note on where the prose sits. The guard's own header carries only
what a reader has to keep true: why the ban exists, the two carve-outs,
where the declaration lives and why there, that the count is exact in
both directions, why the marker is anchored at column zero, and where
the lexical scan stops. Everything above about *how* the inventory
failed (the timestamps, the PR numbers, the arithmetic) is deliberately
not in the file. It is true today and will rot on its own; a comment
nobody is obliged to maintain is how a file grows a paragraph that is
quietly false.

### Screenshots

<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->

### Downstream repositories

<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.

Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.

IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:

1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->

- [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

<!--  Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->

```release-note
test(tests): each hack/*.bats file now declares its own remaining EXIT-trap debt in a `# EXIT-TRAP DEBT: N` header comment, checked by hack/bats-no-exit-trap.bats, replacing the central inventory in hack/cozyreport.bats
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Testing**
- Added automated auditing for `EXIT` traps across Bats end-to-end
tests, including validation of tracking declarations and edge cases.
- Improved diagnostics tests by replacing trap-based temporary-directory
cleanup with explicit cleanup steps.
  - Added tracking annotations for remaining trap-related cleanup work.

- **Documentation**
- Clarified when traps are permitted inside self-contained subshells and
how remaining cleanup debt is reported.
  - Updated review guidance for consistent end-to-end test maintenance.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->