Commit Graph
166 Commits
Author SHA1 Message Date
Aleksei Sviridkin c4e6d169a0 fix(e2e): give the cozytest caller a dataplane budget that fits its backstop
The collector's default budget is sized under the 300s Chainsaw
backstop. The cozytest.sh caller allows 600s, and on the default it
would give up more than half of that window, so it passes a budget
matching its own backstop. The budget test reads each caller's
override and fails when a budget leaves more of its backstop unused
than the backstop's kill grace.

The Chainsaw call-site comment still described the fan-out caps as the
bound, and the e2e testing guide described an EndpointSlice exception
to the notes contract that the code does not have; both now describe
what the collector does.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-10-02 03:29:13 +03:00
Aleksei Sviridkin 94b328c6eb test(e2e): hold every raised backup wait to a start margin
The wait-budget guard accepted a wait one second over the install
timeout, although the wait's clock starts before the install's by the
Application reconcile, the chart fetch and the helm-controller queue. A
wait lowered from 660 to 601 passed every test and brought back the bare
assert timeout the budgets exist to avoid. A wait now has to clear its
floor by a named 60s margin. Nothing in the tree bounds those three
delays, so the value is policy: the headroom every current wait already
carries. The sizing guide in docs/agents/e2e-testing.md asked only for
"strictly above", which the guard now rejects; it points at the margin
instead of restating a number.

The mongodb, rabbitmq and clickhouse walkthrough waits were raised for
the same reason as mariadb's, and their Chainsaw ops grown with them,
but nothing held either. Each suite is now a row in one table and gets
its own floor and op tests, so a regression reds only its own suite.
Floors come from the api-server install-timeout default and, for
mariadb, the chart's startup probe; each suite's RD is checked for an
override instead of being assumed not to carry one. Each op keeps the
remainder it had before the raise over every HelmRelease wait it
reaches, the bucket one included, so raising a wait and its op together
keeps the remainder instead of widening the slack. The mongodb target
behind SKIP_RESTORE=1 is not reached and is left out until the op stops
passing it.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-10-01 17:40:20 +03:00
Aleksei Sviridkin 312409ecd2 test(e2e): fail the suite whose pods outlive its cleanup (#4492)
## What this PR does

A Chainsaw suite could pass while the workloads it created kept running
after its own cleanup, and nothing noticed until a later suite failed.
On one E2E run the brokers of four Kafka releases from suites that had
passed and deleted their applications (#4488), plus a TenantControlPlane
sitting on its finalizer with two apiserver pods (#4489), pushed pod
memory requests to the node limit. Harbor then failed on `Insufficient
memory` two suites later, under the wrong name.

This makes the suite that leaked fail, and name what it left behind.

- Chainsaw v0.2.15 has no hook that runs after a test's cleanup: its
`Configuration` carries `error.catch`, which runs only on failure. So
`make test-chainsaw` now runs `hack/e2e-chainsaw-run.sh`, which invokes
`chainsaw test` once per suite directory and checks the cluster between
invocations with `hack/e2e-chainsaw/_lib/leak-guard.sh`.
- Attribution: a pod is charged to a suite when all three hold. It is
not terminal. Its UID was not in the pod list taken before the suite.
The root of its ownerReferences chain was created at or after the suite
started. A platform pod replaced mid-suite by a rollout, an eviction or
a crash traces back to an object the install created, so it is not
charged. A pre-existing pod adopted by a controller recreated mid-suite
is excluded by the UID baseline. A leak is charged once, to its own
suite.
- The suite start is read off an apiserver, from the `creationTimestamp`
of a server-side dry-run create, rather than off the sandbox. With
several control-plane nodes the reading and a root's timestamp can come
from different apiservers, so the rule assumes their clocks agree more
closely than the gap between the reading and the suite's first object;
no skew margin is added. Source for that behaviour: the generic registry
`Store.create` stamps it through `rest.FillObjectMetaSystemFields`, then
`DryRunnableStorage.Create` copies it back without persisting
(`staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go`
and `dryrun.go` at Kubernetes v1.33.0).
- Budget: charged pods get 300s to disappear, the default
`timeouts.cleanup` in `.chainsaw.yaml`; a unit test keeps the two equal.
A suite that overrides its own cleanup timeout (kafka sets 8m) is
covered the same way, because the check runs only after `chainsaw test`
has returned, which is after every cleanup in the suite has finished
under its own timeout; the 300s start from there. The leaks this targets
sat about 110 minutes (Kafka brokers) and 30+ minutes (the
TenantControlPlane) past their suites. Every green suite logs
`leak-guard: suite=<dir> clear after Ns`, so the number can be checked
against later runs.
- Past the budget the run prints `LEAKED-PODS suite=<dir> budget=300s
count=N`, then one line per pod with its phase, deletion state, root and
`app.kubernetes.io/instance`. It raises a `leaked-pods` error annotation
and adds a failed `leak-guard` test case to `chainsaw-report.xml`. The
run continues through the remaining suites and exits non-zero at the
end, as the single invocation did on a red test.
- A suite that already failed on its own is checked once without the
wait. Its leftovers are reported as a `::warning` with `LEAKED-PODS
suite=<dir> checked-once=chainsaw-failed`, since some may still be
tearing down.
- Each pod gets one `kubectl wait` inside the remaining budget, and
nothing is retried. A timeout or a spent budget gives the verdict from
the last good pod list, also when the re-list after it fails, so a leak
fails the suite. A wait that fails only after running for its whole
`--timeout` counts as a timeout whatever it printed: kubectl maps an
expired context to "timed out waiting for the condition" only inside the
watch, so a deadline that passes during its initial lookup surfaces as
"context deadline exceeded" (`wait.go` and `delete.go` in kubectl
v1.33.0). A spent budget is caught before calling kubectl and never
passed as `--timeout=0`, which means "check once".
- Any other error on a wait or a re-list, before the budget is spent and
before any timeout, stops the waiting and is reported as `LEAKED-PODS
suite=<dir> count=N inconclusive="<error>"`. That is a warning, not a
leak failure, so an apiserver blip cannot become a leak red.
- A failed first reading after a suite, including a pod list that cannot
be parsed, leaves the suite unchecked. It fails with a `leak-guard`
annotation and a report case saying the cluster could not be read, not
that it leaked. Errors that are not leaks carry the annotation title
`leak-guard`, not `leaked-pods`.
- Known leaks are listed in `hack/e2e-chainsaw/_lib/leak-allowlist.txt`
as `<suite-dir> <issue-ref>`: `kubernetes-latest` and
`kubernetes-previous` against #4489 (both suites create a
TenantControlPlane through `_lib/run-kubernetes.sh`). `kafka` and
`kafka-metadata` were listed against #4488 until #4490 fixed it; those
entries are gone. An allowlisted suite prints its leak as a `::warning`
and does not fail. An entry that names a directory that is not a suite
stops the run before the first suite, so a rename cannot keep an
exemption alive. The PR fixing each issue deletes its lines.

Suite selection is unchanged. The runner runs the named directories when
there are any, and otherwise every directory directly under
`hack/e2e-chainsaw/`, each handed to Chainsaw's own discovery; it
refuses a tree with a test at its top level, where that split would stop
being exact. I verified this with Chainsaw v0.2.15's own
`discovery.DiscoverTests`, built from source at that tag. For the full
tree, for `kafka redis harbor` and for `kubernetes-latest`, the union of
the per-suite discoveries equals what a single `chainsaw test [suites]`
discovers: 57, 7 and 2 tests, identical lists. 57 matches the test count
in an earlier full run's report. A unit test pins that the runner passes
chainsaw the suites and flags it was given, in order. The order changes
too: suites used to run in Chainsaw's own order and now run one
directory at a time in alphabetical order, which moves the kubernetes
suites after the kafka ones. Before #4490 fixed #4488 this starved them
(see the E2E criterion below). With the fix merged the order has no
effect, and a future leak that lands before a memory-heavy suite is
named at the suite that leaked. One behaviour does change: a selected
name that is not a directory used to fail the single invocation before
any test ran; now only that suite fails and the others run.

`chainsaw-report.xml` keeps its path and shape: one `<testsuites>` root
with the summed counts, one `<testsuite>` per suite directory. Its
consumers are the `Collect chainsaw report` / `Upload chainsaw report`
steps in `pull-requests.yaml`, `e2e-fork.yaml`, `nightly.yaml` and
`e2e-tag.yaml`, which `docker cp` the file and upload it as an artifact.
Nothing in the repository parses it. The merge is unit-tested on two
per-suite reports in Chainsaw's real shape, one of them failed, and the
result is checked to parse as XML.

Tests: `hack/chainsaw-leak-guard_test.bats` (31 tests, in `make
unit-tests`; named without the `e2e-` prefix, which the unit lane
excludes) covers the attribution rule, the budget, the allowlist, the
merge and the runner, all against stub `kubectl` and `chainsaw`. Every
rule was mutation-checked: removing the guard call, having the Makefile
bypass the runner, inverting the allowlist match, flipping or tightening
the root-timestamp comparison, removing the UID baseline, removing the
terminal-phase filter, skipping the budget wait, changing the budget,
lowering or raising the depth cap, accepting a stale allowlist entry,
reading an unreadable pod list as empty, dropping the leak case from the
report, skipping directories in the selection, treating any failed owner
read as a missing owner, dropping the separate report for a suite that
already failed, dropping the not-run reason from the report, reading any
wait error as a timeout, removing the inconclusive branch, letting a
failed re-list after a timeout turn a leak inconclusive, ignoring a
failed re-list, reporting an unreadable cluster as a leak, reading an
unparsable pod list as clean, reporting a kubectl warning instead of the
error, passing the report name without its extension, wording a failed
re-list as a failed wait, dropping an allowlist's last line when it has
no newline, dropping the check that `--set-string` carries a value, and
reading a wait that ran its whole timeout but printed a context error as
inconclusive. Each one turns its named test red, and restoring turns it
green.

E2E criterion, registered before the run: in the `Run E2E tests` log of
this PR, every suite that ran prints `leak-guard: suite=<dir> clear
after`, a `LEAKED-PODS suite=<dir>` line carrying `allowlisted=`, or one
carrying `checked-once=` (a suite that failed on its own) or
`inconclusive=` (waiting stopped on a failed read), and no line matches
`LEAKED-PODS suite=.* budget=`. With #4490 merged, `kafka` and
`kafka-metadata` are no longer allowlisted and must print `leak-guard:
suite=kafka clear after` and `leak-guard: suite=kafka-metadata clear
after`, which is the live check that #4490 fixed #4488. The first run of
this PR (before #4490, run 36223896793) is the positive control: it
printed `LEAKED-PODS suite=kafka count=15` and `suite=kafka-metadata
count=8`, allowlisted, and every other suite cleared in 0-37s. That run
also went red on both kubernetes suites, because those 23 leaked brokers
left no memory for the second tenant worker VM (`0/3 nodes are
available: 3 Insufficient memory`), which the order change described
above made possible. The guard has not yet run against a real cluster,
so the first run of this PR should carry the `full-e2e` label; its
`clear after` lines are the first measured teardown times, and any
unexpected `LEAKED-PODS` line is either a real leak or a false positive
to understand before merge.

Follow-ups, known and out of scope here:

- The allowlist exempts a whole suite, so a new leak in an allowlisted
suite stays hidden until the issue's fix removes the line.
- Attribution is per suite directory, not per Chainsaw `Test`; the leak
lines carry the release instance, which usually names the test.
- Only pods are checked. A leaked PVC, LoadBalancer IP or bucket holds
no node memory and is not seen.
- The 300s budget rests on Chainsaw's cleanup timeout, not on a measured
pod-gone latency; the `clear after` lines are where that measurement
will come from.
- A platform object recreated mid-suite (for example a HelmRelease
uninstall/reinstall remediation, or a force-replace on an immutable
field) has a root newer than the suite start, and its pods would be
charged to whichever suite was running as a leak; the first CI runs are
where to look for this.
- The per-pod wait recomputes the remaining budget before each pod, and
no unit test pins that.
- `kubernetes-previous` is allowlisted for #4489 although the stuck
finalizer was seen only in `kubernetes-latest`; both suites create a
TenantControlPlane the same way, and the exemption hides any new leak
there until #4489 is fixed.
- Each clean suite now waits for its own pods to finish terminating
before the next starts, where the tail used to overlap the next suite.
The first full run's `clear after` lines say what that costs against the
E2E job's 215-minute cap.
- After a timeout followed by a failed re-list, the leak lines come from
the list taken before the wait, so a pod whose own wait had already
succeeded is still named and the count can be too high; the verdict is
unaffected.
- Likewise, when every wait succeeded and the re-list then fails, the
inconclusive report still lists the pods whose deletion was just
confirmed. In the boundary case where the budget runs out during the
last successful wait and the re-list then fails, the result is
inconclusive rather than a verdict from the last good list, because the
guard only treats the budget as spent when it finds it exhausted at a
check.
- A mistyped `COZY_LEAK_ALLOWLIST` path passes allowlist validation; it
fails closed, since no suite is then exempt.
- An allowlisted suite is not waited on, so `kubernetes-latest` and
`kubernetes-previous` will print `LEAKED-PODS ... allowlisted=` on most
runs even for pods still in normal teardown; waiting out the budget
first and applying the allowlist only to what remains would stop that
noise.
- A slow teardown in each of several suites can add up to 300s apiece,
against the E2E job's 215-minute cap. On the first full run of this PR
(run 36223896793) the `clear after` values of the suites that were
waited on summed to 210s; the last full run before this change took 158
of the 215 minutes.
- A pod whose owner's resource type is no longer served gets "the server
doesn't have a resource type" rather than NotFound, so its suite is
reported unchecked instead of leaked; it still fails.
- `kubectl wait --for=delete` waits for deletion, while the guard skips
pods in `Succeeded` or `Failed`; a pod that finishes but is never
deleted (a Job under a leaked root) spends the rest of the budget before
the suite shows `clear`. The verdict is right; only job time is lost.
- The unit tests do not pin the one-line empty report Chainsaw writes
for a directory with no tests (`_lib`), nor the branch that catches a
spent budget before calling `kubectl wait`; both are verified against
the Chainsaw and kubectl sources rather than by a test.
- The runner's per-suite report directory from `mktemp -d` is not
removed; the sandbox is discarded after the job.
- Each non-suite directory under `hack/e2e-chainsaw/` (today `_lib`)
costs one empty Chainsaw invocation, because the runner leaves the
decision of what is a test to Chainsaw.

### Screenshots

Not a UI change.

### 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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

The diff adds files under `hack/`, changes the `test-chainsaw` target,
and rewords one comment and the failure line in the four E2E workflows.
The trigger map's `hack/` entry is for cozystack/ccp, whose skills
anchor on `hack/package.mk`, `hack/common-envs.mk` and `make generate`,
none of which change here. A GitHub code search for `chainsaw` across
the cozystack organisation finds hits only in this repository.

### Release note

```release-note
test(e2e): the E2E app suite now fails the suite whose pods outlive its own cleanup by more than 300s, naming each leftover pod and its owner; suites with a filed leak are listed in hack/e2e-chainsaw/_lib/leak-allowlist.txt and only warn.
```
2026-10-01 14:03:13 +03:00
Aleksei Sviridkin 08011cef09 fix(ci): escalate an edit to the shared backup walkthrough helpers
examples/backups/<app>/ selects that app's suite, and a directory whose
name is no suite selects nothing. A helpers directory shared by every
walkthrough, examples/backups/_lib/, falls into the second case: an
edit to it alone produces an empty selection, which both lanes read as
"skip Chainsaw", so a change reaching every backup round-trip would run
none.

It now escalates to the full suite like hack/e2e-chainsaw/_lib/. The
set of suites that run a walkthrough is spread across the suites' own
scripts, and a derived subset that misses one would fail open.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-10-01 01:06:25 +03:00
Aleksei Sviridkin a6ec30d56f fix(ci): discover Chainsaw suites written as chainsaw-test.yml
Chainsaw loads chainsaw-test.yml when a directory has no
chainsaw-test.yaml, but select-e2e and select-install enumerated the
.yaml name only. A suite spelled that way ran whenever the whole
directory was handed to chainsaw, yet no escalation listed it and no
edit to it could select it, so it silently dropped out of every
narrowed run.

select-install also sent find's errors to /dev/null inside a pipeline
that reported sort's status, so a failed listing validated no suite at
all and passed. It is now captured and fails validation.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-10-01 01:06:24 +03:00
Aleksei Sviridkin e070bc7d1d fix(ci): escalate a per-suite Chainsaw path that names no suite
A path under hack/e2e-chainsaw/<dir>/ put <dir> into the selection
without checking that it is a suite, leaving the final intersection to
drop it. Alone the path escalated through the empty-selection backstop,
but beside any path that contributed a suite the escalation vanished
and the run narrowed to that suite with nothing on stderr. Shared
material next to _lib/ or a suite nested deeper than discovery looks
then went untested in exactly the pull requests that also touch an app.

The directory name is now membership-tested where the path is
classified, as the hack/e2e-apps/ arm already does, so the decision is
a property of the path and names it.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-10-01 01:06:13 +03:00
Aleksei Sviridkin 9ac67d76a2 test(tests): preserve Bats failure reports during fixture cleanup (#3848)
## What this PR does

Test-level `EXIT` cleanup overrides the Bats TAP handler, so a failed
assertion can disappear from test results. This moves cleanup in eight
unit files after their assertions. Failed tests keep scratch files for
inspection.

Prerequisite for [the Bats runner
change](https://github.com/cozystack/cozystack/pull/3849). Existing
subshell traps in the OpenAPI and CAPK tests remain supported.

Closes #3623

### Validation

`make unit-tests test-controllers -j4 --output-sync=target` passed. The
converted files passed under Bats 1.14.0 and `dash`. A deliberately
failed assertion produces `not ok` after this change, and the trap audit
rejects an added test-level handler.

### Downstream repositories

Test cleanup and its documentation only. No downstream consumer changes.

- [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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
NONE
```
2026-10-01 00:45:09 +03:00
Marian Koreniuk 7500a9fbae test(kubernetes): add the Proxmox e2e suite, parked disabled
Upstream CI has no hypervisor, so this suite cannot run there; it ships
`.disabled` and is run by a lane that owns a Proxmox stand.

Named for what the chart actually creates. The resource definition prefixes the
release, so a Kubernetes CR called k8s-e2e produces
ProxmoxCluster/kubernetes-k8s-e2e in the tenant's own namespace, and the admin
kubeconfig Secret is kubernetes-k8s-e2e-admin-kubeconfig.

Stand-specific values are variables rather than hardcoded. Every one of them is
a bare ${COZY_PVE_*} that run-proxmox.sh expands with envsubst, supplying the
defaults itself — GNU envsubst leaves a ${VAR:-default} in the text exactly as
it is, set or not, and that reaches the apiserver as a string where the schema
wants an integer — and refusing a fixture with a variable still in it. The
StorageClass and the PVC probe read the same COZY_PROXMOX_STORAGE, so the volume
lands where the probe looks. The runner reads the tenant kubeconfig's .conf key,
the address a runner outside the management cluster can reach.
The stand's resource pool comes in through COZY_PVE_POOL, empty by default.
hack/proxmox-e2e-fixture_test.bats pins the shape of the fixture and the render.

The PVC probe reads the storage again after the claim is deleted: a deleted
claim whose disk is still there is the leak the pre-delete hook's Step 1b
exists to prevent. keep_tenant is honoured by the suite's own teardown, not
only by the workflow's. The workflow carries no fallback for the stand's
variables: an unset one fails the job before a key is sent anywhere. The
schedule runs only where vars.PROXMOX_NIGHTLY is true: the job targets a
self-hosted runner with the `proxmox` label, and a scheduled run on a repository
without one would sit in the queue until GitHub cancels it.

The suite also asserts the half of the deployment that lives in the management
cluster — the CSI controller and the cloud-controller-manager — because that is
the property the pair is designed around and a tenant-only assertion would pass
with the token in the wrong place.

Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
2026-09-29 21:04:06 +02:00
Aleksei Sviridkin 51a2d859ac docs(release): describe what the release job builds and what blocks an rc
The amd64 release job builds the Talos installer and matchbox for both
arches, so the pages that said CI never builds both in one job, and
that build-amd64 builds amd64 only, were wrong. Every rc stitch also
skips matchbox and the e2e sandbox, and the nightly report lists
matchbox and the refs an in-tree rebuild stamps over; the page said
every such skip and line fails the next rc. Name the ones that do not.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-29 12:36:56 +03:00
Aleksei Sviridkin 71144d4fd6 ci(release): pin the matchbox rebuild in the release job
tags.yaml only runs on tags, so no pull request exercises it. The
matchbox rebuild is the only thing that makes its ref an index, and
losing the step, moving it past the tree capture or letting it switch
the default builder would surface as a failed gate on the next rc. The
comments that called the talos images amd64 only now say they are
built for both arches in the amd64 job.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-29 11:52:08 +03:00
Aleksei Sviridkin c740031453 Merge remote-tracking branch 'origin/main' into ci/build-arm64-leg
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-29 11:20:27 +03:00
Myasnikov Daniil 10727c0391 test(tests): preserve Bats failure reports during fixture cleanup
Test-level EXIT handlers replace the handler Bats uses for TAP output,
so a failed assertion can disappear from the reported test results.
Cleanup must preserve those failures before the unit runner changes.

Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
2026-09-29 12:38:02 +05:00
Aleksei Sviridkin cb035f8eac docs(agents): stop calling the image-ref known gaps unfixed
Two of the gaps listed there are handled in the tree now, and each entry
already says whether it is. The heading sentence still claimed all of
them were open.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-28 22:17:07 +03:00
Aleksei Sviridkin 79854b16fb Merge remote-tracking branch 'origin/fix/build-list-orphan-images' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-28 19:49:54 +03:00
Aleksei Sviridkin 5924b3fd34 docs: describe how a pre-release is built and gated for arm64
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-28 01:32:37 +03:00
Aleksei Sviridkin 144d080342 feat(build): gate a release on every pinned image being multi-arch
A release pins one ref per image for clusters of either architecture.
A ref that resolves to a single-platform manifest, or to an index
missing amd64 or arm64, installs on one and fails to pull on the
other, and nothing before an arm64 install would notice.

hack/verify-multiarch.sh resolves every ref hack/lib/image-refs.sh
enumerates, third-party ones included since an arm64 node pulls them
too, and fails naming each ref that is not an index carrying both
linux/amd64 and linux/arm64. A ref that cannot be resolved fails as
well, rather than passing unchecked. A rate limit or a 5xx is the one
answer retried, a bounded number of times, and a ref still refused
fails as rate-limited rather than as single-arch. --report prints the
same list without failing, for a tree that is not being released.

Images that cannot be built for both architectures go in
hack/multiarch-allowlist by repository, each with a reason, so a bump
needs no edit; they are still listed, and an entry that matches no
single-arch ref is named as stale.

--refs-file checks a list of refs instead of a tree, a tag-only ref
resolved by its tag, with the same allowlist and output. It is for
images the tree does not pin by digest, such as the ones an e2e run's
nodes actually pulled.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-28 01:32:36 +03:00
Aleksei Sviridkin 7753394bfe build: rebuild every package that has an image target
flux-plunger, keycloak-operator, kilo and migration-controller have
image targets but were missing from the root build list, which the
build matrix, the main build and release preparation all iterate. No
CI path rebuilt them, so their pins stayed on images pushed by hand
while Dockerfile, patch and base-image changes never shipped, and the
multi-arch pre-release build would find single-arch pins it cannot
replace. migration-controller shipped a bare :latest with no digest.

Add the four packages, and a test that derives the set from the
package Makefiles so a new image target cannot be left out again.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-28 00:57:24 +03:00
Aleksei Sviridkin f02963a2c7 fix(keycloak-operator): keep the image host inside repository
The image target stamped only `tag`, leaving the committed
`registry: ghcr.io` in place, so a build against the CI registry would
render the public host with a digest that exists only in the build
registry. Stamping the build host into `registry` instead would split
the source registry across two keys, where the nightly mirror's
substring host rewrite cannot reach it and the published tree would
keep pointing at the private registry.

Set `registry: ""` and keep the whole host in `repository`, stamped
by the image target. The upstream chart joins the two keys, so the
rendered image string is unchanged, and the ref now has the same
shape as every other stamped reference the mirror and promotion
already handle.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-28 00:57:24 +03:00
Aleksei Sviridkin 769d3b3897 fix(build): push the kamaji provider image under the build's IMAGE_TAG
The capi-providers-cpprovider Makefile assigned IMAGE_TAG itself, which
beats the value CI puts in the environment. Every PR, main and release
line build therefore pushed the same component-version tag and
overwrote each other's, and its second tag was built from an undefined
TAG variable and ended in a bare "-".

The image now goes through image-tags like every other package: the
build-unique IMAGE_TAG always, the component version only under
PUBLISH_VERSIONED=1. The stamped refs keep the component version as
their tag, so the promote, retag and mirror tooling see the same refs
as before.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-27 07:57:31 +03:00
Aleksei Sviridkin 9a7094427e feat(build): stitch amd64 and arm64 builds into multi-arch indexes
The arm64 images are built on native arm64 runners, apart from the
amd64 build, so each build pushes a single-platform image. The stitch
joins every first-party image with its -arm64 twin into one index,
moves every tag this build pushed for it to that index, and repins
the tree on the index digest, so one ref serves both architectures.

Most refs in the tree name only the build's IMAGE_TAG, while under
PUBLISH_VERSIONED=1 the build also pushes the component version, so
the refs alone cannot say which tags to move. The image-tags macro
therefore appends every tag it pushes to PUSHED_TAGS_LOG when that is
set, and the stitch moves exactly those, plus IMAGE_TAG and the tag a
ref carries. It reads no other tag: another build's tag that sits on
the same amd64 bytes is not this build's to move, and scanning every
tag of every repository would cost thousands of registry reads per
release.

It reads refs through hack/lib/image-refs.sh like every other
consumer, rewrites only the files that enumeration names, and then
searches the whole tree outside charts/, gzip payloads included, for
any old digest it re-pointed. A ref whose digest is also pinned where
the rewrite cannot reach is skipped instead of half rewritten.

The kamaji control-plane provider's chart ships its ref only inside
files/components.gz, built from a plain copy with gzip -n, which no
text search sees. The stitch rewrites both copies, recompressing with
gzip -n so the result is what the package's image target would have
written, rather than leaving that image amd64 only.

An image with no arm64 twin (talos, the e2e sandbox), or a pair that
is not one amd64 and one arm64 image, is skipped and named. A
registry read answered with a rate limit or a 5xx is retried a
bounded number of times; any other registry error fails the run at
once rather than reading as a missing arm64 image.

On a rerun after a stitch that failed partway, the build tag can
already sit on an index of the pinned image and an arm64 image, not
necessarily this run's if the arm64 leg rebuilt it. The stitch repins
on that index instead of skipping the image, and copies it to any of
the build's other tags still left on the amd64 image. A tag moved to
anything else fails the run, since the tree then no longer matches
the registry.

MATRIX_ARCH=arm64 makes hack/build-matrix.sh leave out the e2e
sandbox, which pins linux/amd64: an arm64 leg building it would push
a second amd64 image under its -arm64 tag.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-27 07:57:30 +03:00
Aleksei Sviridkin 6907c22302 docs(agents): describe the talos package as multi-arch
The build notes said the talos package pins linux/amd64, which no
longer holds and would steer an agent into re-adding the pin.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-26 17:44:19 +03:00
Aleksei Sviridkin b6a30bd504 docs(e2e): describe the per-suite runner and the pod-leak guard
The agent guide, the suite README and a workflow comment said the E2E
step is a single chainsaw invocation; it is now one per suite, and a
suite whose pods outlive its cleanup fails. Say how a pod is charged
to a suite, what the budget is, and how the allowlist is kept honest.
The workflows' failure line now points at LEAKED-PODS as well, since a
run that failed only on a leak has no assertion diff to point at.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-26 15:19:49 +03:00
Aleksei Sviridkin 084ac47237 feat(build): build published images for amd64 and arm64 by default
PLATFORM defaulted to empty, so buildx built for the host alone. On an
arm64 workstation `make image` published an arm64-only digest, which
amd64 nodes then fail to pull; the cilium pin shipped exactly that
until it was rebuilt by hand. A build now targets linux/amd64 and
linux/arm64 unless PLATFORM is set, and the digest a package stamps is
the index digest.

LOAD=1 keeps building for the host only. The classic docker image store
cannot load a multi-platform index, and loading is the documented local
workflow.

This changes local pushes: a `make image` on the default docker driver
without the containerd image store now fails with "Multi-platform build
is not supported for the docker driver", where it used to build for
the host. Such a build needs a docker-container builder (BUILDER), or
LOAD=1 or an explicit PLATFORM for a single architecture.

talos and testing pin linux/amd64 with `override`, placed before the
include because common-envs.mk expands BUILDX_ARGS immediately. The
Talos assets are amd64: matchbox bakes the kernel and initramfs into
its image, and the e2e sandbox boots the disk under qemu-system-x86_64.
An arm64 variant of either would still serve or boot amd64 files.

CI pins linux/amd64 on every image build. Nothing there sets up QEMU,
fork builds export a single-platform archive, and tags.yaml builds on
the default docker driver, so the default would break or emulate those
jobs. Native arm64 builds belong in a leg of their own.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-26 15:14:12 +03:00
Aleksei Sviridkin 368d00b5d4 test(e2e): remove unused monitoring suites and audit BATS runners (#4019)
## What this PR does

I removed the two monitoring OIDC BATS suites that had no runner after
the Chainsaw migration, and brought the runner audit and wrapper OIDC
test from [#3778](https://github.com/cozystack/cozystack/pull/3778).

Also added checks for audience mapper and `OIDC_MODE=System`. The named
`KeycloakRealmGroup` test documents coverage the document-count and kind
assertions already provided.

Runner audit reports unused BATS suites, missing files in runner paths,
and unresolved Make variables (including `$(E2E_SUITE)`).

Live Monitoring API admission, Grafana operator reconciliation and mode
transitions are not tested here.

### Validation

- `make -j4 unit-tests SHELL=/usr/bin/dash` passed.
- Monitoring Helm tests: 53 system-chart tests and 10 wrapper-chart
tests passed.
- Runner audit: 12 cases passed under dash and Bats 1.14.0. The full
shell syntax and BATS sweep passed under dash too.
- Three chart mutations and seven runner mutations failed as expected.

### Downstream repositories

- [x] No downstream repository is affected by this change

### Release note

```release-note
NONE
```
2026-09-25 16:55:05 +04:00
Myasnikov Daniil c55ab268e2 test(monitoring): remove orphaned OIDC BATS suites
The two pre-Chainsaw monitoring OIDC suites have had no invocation site since the app suite migrated to Chainsaw. Remove them instead of carrying dead tests that force the selector to run every live suite.

Keep the existing selector and capture guards for the historical path shape, and update their comments now that the directory is empty. Retain a named chart assertion for the no-KeycloakRealmGroup ownership invariant; the existing count and kind assertions already enforce the same behavior.

Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
2026-09-24 16:28:00 +05:00
Timofei Larkin 180030be72 fix(api): refuse only what a name declaration actually breaks
Three cases refused creates of a kind, or were documented as refused,
without the declaration being at fault.

A schema that is not a JSON object was reported as an invalid
x-cozystack-name, although such a schema has no declaration to blame,
and the spec schema's own loader already reports it. It now declares
nothing, so creates keep working and a kind with a legacy cap keeps it.

Any unknown key inside a declaration took the kind offline, so a
generator adding a title or an example would do the same. Keywords that
constrain nothing, the JSON Schema annotations and x- extensions, are
now ignored. A constraint keyword the server does not implement is still
refused, because accepting it would drop it silently.

The docs said a declaration no name satisfies is refused. That holds
for lengths only: a pattern is checked to compile, not to match
anything. The docs now say so.

The parser moves to pkg/apis/apps/validation so the API review gate can
reuse it rather than keep a copy of its rules.

Assisted-by: LLM
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
2026-09-24 09:35:10 +03:00
Timofei Larkin 146e06e75e feat(api): take Application name limits from the application's own schema (#4367)
## What this PR does

The aggregated API server capped two kinds' names by knowing what their
charts render. A `Kubernetes` cluster name was held to 32 characters
because every worker pool is a `KubernetesNodes` release named
`kubernetes-nodes-<name>-<pool>`, and the default `md0` pool has to fit
the 53-character Helm release name limit. A `Kafka` name was held to 44
because the KRaft controller pod `kafka-<name>-c-<hash>-<id>` has to fit
the 63-character DNS-1123 label limit. The arithmetic is right, but it
is the charts' knowledge living in the platform: the next application
with a sub-resource budget needs another `if r.kindName == ...` branch,
and an application served from outside this repository — the whole point
of `ApplicationDefinition` — cannot get one at all.

An application now declares the limits on its own name in its own
schema, and the server applies whatever it finds there for whatever kind
the definition describes. The chart's `values.yaml` carries a `## @name`
annotation, `cozyvalues-gen` v1.7.0 publishes it at the root of
`values.schema.json` as `x-cozystack-name`, and `hack/update-crd.sh`
carries it into the `ApplicationDefinition`:

```yaml
## @name {string} - Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.
## @maxLength 32
```

```json
"x-cozystack-name": {
  "type": "string",
  "description": "Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.",
  "maxLength": 32
}
```

`maxLength`, `minLength` and `pattern` are honoured; `pattern` is
unanchored, as JSON Schema specifies. A declaration only tightens:
`prefix + name` must still fit the 53-character Helm limit. A rejected
name is answered with the chart's own description, or with "limit
declared by the <Kind> application" when the chart gives none.

The schema is untrusted input — definitions from other repositories are
written by hand — so a declaration that cannot be enforced as written
(not an object, a type error, an unknown keyword such as a misspelled
`maxLenght`, `maxLength` below 1, a negative `minLength`, `minLength`
above `maxLength` or above the Helm budget, a pattern that is not RE2)
refuses every create of that kind with `configuration error: the <Kind>
ApplicationDefinition declares an invalid x-cozystack-name: …`, and logs
the same at start-up. Dropping the declaration would admit names the
chart cannot render; applying it would reject every name and blame the
user. Other kinds are unaffected, and existing objects of the affected
kind can still be read, updated and deleted.

An upgrade brings `cozystack-api` up before `kubernetes-rd` and
`kafka-rd` re-render (`kubernetes-rd` depends on `cozystack-engine`), so
the new server can briefly read a definition that predates its
declaration. For exactly that case — a definition that declares nothing
— the two previous caps stay in `legacyNameCaps` as a floor, to be
removed once no supported upgrade starts from such a definition. That is
the only per-kind knowledge left in the name checks.

Also in this PR:

- The `pre-commit` job pins `cozyvalues-gen` v1.7.0, the first release
that understands `@name`
([cozystack/cozyvalues-gen#33](https://github.com/cozystack/cozyvalues-gen/pull/33)).
- The API review gate now diffs the root `x-cozystack-name` of every
definition as `metadata.name`, so a chart that lowers its cap or adds a
pattern is reported like any other tightened field. On this PR it
reports the Kubernetes and Kafka caps as added; their effective limits
are unchanged.
- The migration 56 comment that pointed at the removed
`maxKubernetesClusterName` now points at the chart's declaration.

Not in scope, because none of it is a static limit a chart can declare:
the `Tenant` name pattern in `pkg/apis/apps/validation`, the computed
tenant-namespace length check (it depends on the parent namespace at
request time), the tenant quota hierarchy, and the Phase 2 removed-field
warning for `Kubernetes`.

### How it is tested

`TestNameCapComesFromTheShippedSchema` reads the shipped `kubernetes`
and `kafka` ApplicationDefinitions off disk, builds storage through
`NewREST` as the server does, and checks the cap and the chart's
explanation; it then rewrites `maxLength` inside the shipped schema and
checks the limit moves with it, which a cap held in code would not.
`TestInvalidNameSchemaFailsClosed` covers each malformed declaration
through the same path, `TestLegacyNameCapsCoverUndeclaredDefinitions`
pins the upgrade floor and that a declaration replaces it, and
`TestClassifyNameDeclaration` covers the gate.

### Downstream repositories

Two repositories are affected and neither follow-up is filed yet, so no
box is ticked.

- `cozystack/website` — `content/en/docs/next/development.md` documents
the `values.yaml` annotations and needs `@name`;
`content/en/docs/next/cozystack-api/application-definitions.md`
describes `openAPISchema` and needs `x-cozystack-name`.
- `cozystack/external-apps-example` — it pins no `cozyvalues-gen`
version and regenerates against whatever is current; nothing in it
declares a name constraint, so it is unaffected until it chooses to.

- [ ] 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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
feat(api): Application name limits now come from the application's own schema instead of being hardcoded per kind in the aggregated API server. A chart declares them with a `## @name` annotation in `values.yaml`, which cozyvalues-gen v1.7.0 publishes as `x-cozystack-name`; the API server enforces `maxLength`, `minLength` and `pattern` on `metadata.name` for any kind, answers a rejected name with the chart's own explanation, and refuses creates of a kind whose declaration it cannot enforce with a configuration error. The Kubernetes (32) and Kafka (44) name caps are unchanged in effect and are now declared by their charts.
```


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

* **New Features**
* Applications can declare minimum and maximum name lengths and
formatting patterns.
* Kubernetes cluster names are limited to 32 characters, and Kafka
cluster names to 44 characters, to meet generated resource naming
limits.
* Names that violate declared constraints are rejected with an
explanatory message. Tighter name constraints are also reported as
breaking changes.
* Declared maximum lengths cannot exceed the platform’s existing Helm
release-name budget.

* **Documentation**
* Added guidance on declaring and enforcing chart-specific resource-name
limits.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-24 10:29:27 +04:00
Aleksei Sviridkin 1f30ac4598 fix(tests): let the mariadb waits cover a first boot (#3769)
## What this PR does

Closes #3741

Several waits for a database HelmRelease to become Ready gave up before
the release could: the mariadb, mongodb, rabbitmq and clickhouse backup
walkthroughs waited 300 seconds, and both mariadb Chainsaw suites gave
their HelmRelease-ready assert 6 minutes. A generated release carries a
10-minute install timeout (`pkg/cmd/server/start.go:91`), so a failed
install was reported as a bare timeout, because the release had not yet
recorded why it failed, and for mariadb a first boot the startup probe
was still willing to wait for failed the harness instead.

A wait on such a release has two floors and has to beat the larger. The
first is the app's own first boot: cozystack-api leaves
`spec.waitStrategy` unset and helm-controller defaults it to `poller`,
so the install waits through kstatus, and kstatus maps the operator CR's
`Ready=False` onto `InProgress`. For mariadb that boot may take the
chart's 310-second startup budget (`startupProbe.failureThreshold: 30`
in `packages/apps/mariadb/templates/mariadb.yaml` with the operator's
probe defaults of 20 and 10); mongodb, rabbitmq and clickhouse set no
startup probe. The second is the install timeout. At or below it the
waiter is the one that expires, because its clock starts when the step
runs and the install's only once the Application has been reconciled and
the chart artifact fetched. The install timeout binds in every case
here, so the walkthrough waits go to 660 seconds and the mariadb asserts
to 11 minutes.

Raising a wait inside a walkthrough spends the Chainsaw op that runs the
script, and an op that runs out SIGKILLs the process group and loses the
script's error. Each op therefore grows by exactly what the waits it
reaches gained: mariadb's round-trip from 30 to 42 minutes, rabbitmq's
from 30 to 42, mongodb's from 30 to 36 (its round-trip runs with
`SKIP_RESTORE`, so only the source wait counts), and clickhouse's from
60 to 72, which still sits above the sum of every inner ceiling. That
bounds a failure at a single wait, not a run that is slow at every stage
and then fails late; bounding that needs a phase budget, not a larger
literal. postgres keeps its 360-second waits and every walkthrough keeps
its 300-second bucket wait: whether a suite pays the extra minutes on
every red run is a per-suite call.

The mariadb endpoint asserts keep their job. A freshly applied CR
carries no conditions and kstatus reads it as current, so the install
wait can end before any pod is up; an endpoint address closes that
window. That is also why the `wait_for_field` on the CR stays in the
walkthrough.

`wait_hr_ready` now dumps every condition and the release history on
timeout and on `Stalled=True`, instead of the single Ready message a
scheduled retry can overwrite before anyone reads it. It also stops
printing one `(none recorded)` line for different answers: a release
that never appeared, a release deleted during the wait, a lookup kubectl
could not answer, a failed history read and an empty history each say
so, with kubectl's own stderr kept, and a release seen earlier still
gets its dump when only the last lookup failed. The change goes to every
byte-identical copy of the helper, in
`examples/backups/{clickhouse,mariadb,mongodb,postgres,rabbitmq}/00-helpers.sh`,
and they stay identical.

`hack/mariadb-first-boot-waits.bats` pins the mariadb budgets against
the tree. It derives both floors from their sources instead of pinning
literals, so raising `failureThreshold` past the install timeout, or
changing the install-timeout default, reds it; each source has a test
that refuses to proceed when it cannot be read, including the
ApplicationDefinition, whose two override annotations would each
invalidate the derivation. It holds the round-trip op at a fixed
remainder over the ceilings up to and including the source wait, bucket
stage and S3 preflight included, and at a fixed slack once both mariadb
waits are paid, so raising a wait without the op reds. The extractors
are also tested against fixtures, because the tree cannot tell a correct
extractor from a broken one. Beyond mariadb, the file runs every exit
branch of `wait_hr_ready` in each copy against a stubbed kubectl, and
pins the copies byte-identical. The population is every definition in
the tree found by grep, minus two named exceptions held to still differ,
so a new copy joins without editing the test and an exception whose copy
converges goes red:

- `examples/backups/redis/00-helpers.sh` is written in the compact
one-line style of the rest of that file, so converging it rewrites its
shape rather than applying the shared body.
- `examples/backups/kafka-metadata/00-helpers.sh` is a helper of its own
with the same diagnostic gap on timeout (it prints no reason at all),
and closing that is a change to kafka-metadata.

`docs/agents/e2e-testing.md` replaces the reviewer checklist's 5-6m
standard and its list of exceptions, which the tree had outgrown, with
the floor a wait has to beat and the rule that a longer wait carries its
reason in-line. The derivation gets a section of its own, opening with
the rule in one line. For a release an app chart renders itself the
floor is `spec.install.timeout`, then `spec.timeout`, then
helm-controller's 5m default, and the nested `<name>-system` releases
never take cozystack-api's 10m. The harbor comments name the floor each
of their asserts sits against, and the opensearch comment stops citing
the removed standard.

Adding `hack/mariadb-first-boot-waits.bats` does not escalate the E2E
selection: `hack/[^/]+\.bats$` sits in `inert_config_pattern` in
`hack/select-e2e.sh`, and `full_suite_pattern` escalates only
`hack/e2e-*.bats`. On this changeset `hack/select-e2e.sh` selects
`clickhouse harbor mariadb mongodb opensearch postgres rabbitmq`.

What this costs is paid only on a failing run; the raised numbers are
ceilings. Summing the timeouts stated on each suite's own steps, mariadb
goes from 92m to 114m, clickhouse from 116m to 128m, rabbitmq from 67m
to 79m and mongodb from 95m to 101m, before the untimed steps that fall
back to the defaults in `.chainsaw.yaml`. clickhouse is the most
expensive suite by that sum. All of them compete for the 215m
`timeout-minutes` of the whole `e2e` job, which also covers image
download, sandbox preparation and the Cozystack install, and a job
cancelled at the cap loses both its log and `cozyreport.tgz`.

This does not fix the failure that led here. In the run that prompted
the investigation the mariadb replica never opened port 3306 at all,
spending its whole 310-second budget inside `mariadb-install-db` before
being killed, and no wait budget would have saved it. That defect is
#3744. What this fixes is a second, independent defect found while
reading the same code.

One thing here is argued rather than measured: the mechanism is checked
against helm-controller and kstatus, and every number is derived from
the tree, but no run ID pins that it is what any particular red run hit.
Both floors justify 660s whatever the cause was, so being wrong about
the cause leaves slack rather than breakage.

### Not in this PR

- This PR retires the mariadb, mongodb, rabbitmq and clickhouse
walkthrough waits and the two mariadb asserts. The rest is tracked in
#4404: the other Chainsaw HR-Ready asserts under their floor, the harbor
asserts, the 300-second bucket waits, the postgres waits, and the redis
and kafka-metadata diagnostic gap.
- The clickhouse round-trip comment says every wait fails fast,
`wait_hr_ready` on `Stalled=True`, "so a genuinely broken backup reports
in seconds". A release whose install is still running is neither Ready
nor Stalled, so that holds for a stalled release and not for an install
that has not finished. The sentence predates this change.
- `hack/mariadb-first-boot-waits.bats` is large for what it guards (two
floors, two ratchets on the op, and the helper identity pin), with many
fixture tests on its own extractors. Trimming it to that core is a
separate decision.
- Two comments in that file state facts about the mariadb suite that are
true today and that nothing checks: three script ops carry a minute
timeout, and the only one ahead of the round-trip is the 8m pre-clean.
- The mariadb op comment says a bound on the whole flow "needs a phase
budget (docs/agents/e2e-testing.md)"; the doc describes it as bounding
the collection phase as a phase (`COZY_DIAG_PHASE_BUDGET`), so the
pointer would resolve faster if it named that bullet.
- Cosmetic: the behavioural test's loop body is not indented under its
`for`, one line of the bats header runs past the width of the rest, and
the checklist's sentence under the example assert ("what this block
demonstrates least") reads awkwardly.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff, file by file, for all eighteen files. The five
`examples/backups/*/00-helpers.sh` files change the diagnostics of
`wait_hr_ready`, and the mariadb, mongodb, rabbitmq and clickhouse
walkthrough scripts change wait budgets; the manifests the walkthroughs
apply and the flows they document are unchanged, so no website page
restates anything that moved. Four Chainsaw suites (mariadb, clickhouse,
mongodb, rabbitmq) change timeouts and comments, and the harbor and
opensearch suites change comments only. `docs/agents/e2e-testing.md`
changes the reviewer checklist entry and adds the sizing section.
`hack/mariadb-first-boot-waits.bats` is a new file picked up by the
existing `BATS_UNIT_FILES` wildcard. `hack/package.mk`,
`hack/common-envs.mk` and `hack/update-crd.sh` are untouched, and no
`values.schema.json`, `values.yaml` default, version enum,
`ApplicationDefinition`, CRD, namespace or release asset is touched.

- [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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
fix(tests): the MariaDB, MongoDB, RabbitMQ and ClickHouse backup examples and the MariaDB Chainsaw suites now wait for a HelmRelease past the release's own install timeout, so a failed install reports its own reason instead of a bare timeout, and the examples' HelmRelease wait now tells a release that never appeared, one deleted mid-wait and a kubectl that could not answer apart.
```
2026-09-23 18:23:04 +04:00
Aleksei Sviridkin b1519acfe7 fix(e2e): cap the bucket S3 reads so the verify step honors its timeout (#4281)
## What this PR does

Refs #4268 — a plain reference, not `Fixes`: the issue asks for a
`script` op to be cut off at its declared ceiling, and that is Chainsaw
behaviour this repo cannot change. Chainsaw runs the op's shell under
Go's `exec.CommandContext`, whose default cancel SIGKILLs the shell PID
alone and then blocks on the inherited stdout/stderr pipe until an
orphaned `mc` closes it, so the op returns only when `mc` exits on its
own rather than at the ceiling. This PR mitigates that where it bit —
per-command caps so no descendant outlives its own budget — but the
op-timeout gap itself lives in Chainsaw, so the link stays a reference
rather than a `Fixes`. The sibling step that produced the 606s,
`verify-every-bucket-takes-a-write` in the `bucket-1-capacity` Test, is
on `main` and already carries its own per-call caps; this PR applies the
same shape to `verify-s3-access`.

On run 34948279363 the bucket suite's
`verify-every-bucket-takes-a-write` step declared `timeout: 5m` but ran
606s and ended on `mc`'s own non-zero status rather than being cut at
the ceiling. `verify-s3-access` in
`hack/e2e-chainsaw/bucket/chainsaw-test.yaml` is the same-shaped step in
this suite. This PR wraps every S3 (`mc`) and apiserver (`kubectl`) read
in `verify-s3-access` in its own `timeout`, so a hung read dies at a
known budget instead of holding the step open, and corrects the two
places (`hack/e2e-chainsaw/.chainsaw.yaml` and
`docs/agents/e2e-testing.md`) that explained the op ceiling with a
mechanism that does not hold. It caps `verify-s3-access` for sharing the
shape, not for producing the 606s number.

### Which of the issue's two candidate mechanisms holds

The issue named two candidates and stated the log could not separate
them. The Chainsaw v0.2.15 source separates them, and it is the second
one, with the exact refinement the issue proposed.

**Candidate 1 — "an operation `timeout` does not bound a `script` op" —
is refuted.** The op timeout is applied and does reach the process:
`pkg/runner/operations/script.go:24` copies the step's `timeout` into
`Timeouts.Exec`, and `:38` wraps the op context with `ctx, cancel :=
context.WithTimeout(ctx, tc.Timeouts().Exec)` before calling
`op.Exec(ctx, …)`. So a 5m step produces a real 300s deadline on the
context the command runs under.

**Candidate 2 — "it bounds it and kills the shell, but Chainsaw waits on
a pipe a grandchild (`mc`) still holds open" — is confirmed.** In
`pkg/engine/operations/script/operation.go` the command is built with
`exec.CommandContext(ctx, shell, …)` (`:105`), and in `execute` its
stdout/stderr are set to `*bytes.Buffer` (`:128`–`:129`, the buffers are
`internal.CommandOutput` in
`pkg/engine/operations/internal/command_output.go:12`–`13`) and run via
`cmd.Run()` (`:130`). Two Go `os/exec` properties then produce the
overrun: `exec.CommandContext`'s default cancel is `Process.Kill()` on
the shell PID alone — not a process-group kill, and the operation sets
no `SysProcAttr.Setpgid` — and because stdout/stderr are not `*os.File`,
`os/exec` allocates OS pipes and copier goroutines, so `cmd.Wait()`
blocks until those pipes reach EOF. EOF needs every inherited write-end
closed. When the shell forks `mc`, `mc` inherits those fds; at 300s the
deadline SIGKILLs the shell but `mc` is orphaned, keeps retrying the S3
PUT, and holds the pipe, so the op does not return until `mc` exits on
its own at 606s. `cmd.Run()` then returns `signal: killed`, which is
stored in the `$error` binding and never printed — only the `($error ==
null): true` check failure is logged — which is why grepping the run for
`signal: killed` found nothing.

**Empirical confirmation.** A standalone Go program reproduces it
exactly. `exec.CommandContext` with a 1s timeout, `*bytes.Buffer`
stdout/stderr, running `sh -c 'sleep 2 & sleep 4'` (a descendant that
inherits the pipe): `cmd.Run()` returns after ~4s, not 1s, with
`err=signal: killed` and `ctx.Err()=context deadline exceeded` — the
ceiling fired at 1s, control came back only when the descendant closed
the pipe. Redirecting the descendants' stdout/stderr away from the pipe
(`… >/dev/null 2>&1`) returns at 1.00s. That is the whole mechanism: the
ceiling cuts the shell but not a descendant that still holds the pipe.

### The fix

Per-command caps, the remedy the issue itself points to ("every read
carries its own cap") and the shape already used elsewhere in the suite.
The bucket step now defines a `k()` wrapper (`timeout 30 kubectl`) and
an `mc()` wrapper (`timeout 15 mc`) for the reads, and calls `timeout 45
mc cp` directly for the two PUTs; a hung read is killed at its own
budget, releasing the inherited pipe promptly, so even when the op
ceiling fires the residual is one inner cap rather than `mc`'s full
retry exhaustion. The terminal precedents for these budgets are the
redis suite's terminal `k()` (30s, `redis/chainsaw-test.yaml:298`) and
the sibling `verify-every-bucket-takes-a-write` step's terminal `mc cp`
(45s, `:390`, sized for a volume grow); the 15s cap on `mc` in
`hack/e2e-chainsaw/_lib/backup-access-preflight.sh` is not a precedent,
because it sits inside a `while :;` retry loop where a 124 costs one
attempt, whereas here under `set -e` a 124 ends the step. The inner
budgets — 2×30 for the two `k` reads + 2×45 for the two `mc cp` PUTs +
at most 7×15 for the other `mc` calls (eight other call sites, but the
two cleanup `mc rm` are mutually exclusive) + the 30s port-forward
readiness wait = at most 285s — sit under the 5m op ceiling, which stays
as a backstop, per the "op above the sum of inner budgets" rule in
`docs/agents/e2e-testing.md`. The long-lived `kubectl port-forward`
keeps no cap by design (it must persist for the whole step, and its
stdout/stderr are already redirected to `/dev/null`, so it cannot hold
the step's pipe). The `mc()` wrapper is non-recursive because `timeout`
execs its argument through a PATH lookup that bypasses shell functions —
verified in `dash`, `bash` and `sh`. The `catch` block's diagnostic read
gets the same treatment — `timeout 30 kubectl` under a 1m op `timeout` —
for the same pipe reason.

### The readonly deny probe fails on a timeout

The readonly-upload probe used to branch on `mc` directly (`if mc cp …;
then`), which suspends `set -e`, so a `timeout` there (exit 124) was
indistinguishable from the access-denied non-zero the probe expects: a
45s hang on the deny path would exit 124, take the false branch, reach
cleanup and exit 0 — a long hang turned green, the exact window this PR
exists to bound. This is the first `ro-user` write in the step, so no
earlier call proved that path healthy. The probe now captures the
status: exit 0 is the readonly regression, 124 is a timed-out probe,
both fail; any other non-zero is the expected denial.

### No unit surface; why the evidence is the proof

This is an e2e-config and doc-correctness change. The embedded Chainsaw
script is not extracted into a unit harness anywhere in the tree, so
there is no red-to-green unit test to add without inventing a bespoke
script-linter (which would be a new checker standing in for the fix).
The regression is instead pinned by the mechanism proof above: the
source citations show the ceiling cannot cut a descendant that holds the
pipe, and the caps remove the only way a descendant outlives its budget.
The deny-probe status handling and the pipe-vs-file secret reads were
exercised against stub exit codes (0 → regression-fail, 124 →
timeout-fail, other non-zero → expected denial) and the whole script
passes `sh -n` and `bash -n`.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. The change touches only `hack/e2e-chainsaw/` suite content and
`docs/agents/e2e-testing.md`; it moves or renames nothing under `hack/`,
and touches no node-prerequisite or `package.mk` path. No downstream
repository is reached.

- [x] No downstream repository is affected by this change

### Release note

```release-note
NONE
```


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

- **Bug Fixes**
- Improved end-to-end S3 access verification so stalled Kubernetes API
and S3 operations fail promptly.
- Correctly distinguishes timed-out access checks from expected
permission denials.
- Preserves partial command output and allows subsequent test steps to
continue after individual timeouts.
- Applies separate time limits to bucket reads, uploads, and access
checks for more predictable test behavior.

- **Documentation**
- Clarified operation timeout behavior and recommended per-command
limits within the overall operation limit.
- Documented timeout handling for orphaned processes and updated
worst-case timing guidance.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-23 17:14:56 +04:00
Aleksei Sviridkin b96c822bde docs(agents): give the HR-Ready budget derivation its own section
The checklist entry had grown into a derivation and buried the rule it
states. The rule stays in the checklist and the derivation moves to a
section of its own.

The derivation under-budgeted in two ways. It stated the 10m install
timeout as universal, while releases an app chart renders itself take
their template's spec.install.timeout, then its spec.timeout, or
helm-controller's 5m where neither is set. And it restated which kinds
kstatus gates on, which is easy to get wrong, so it now points at the
upstream table instead.

The harbor comments now name the floor each of their asserts actually
sits against, and the opensearch comment stops citing a 5-6m standard
the checklist no longer states.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
2026-09-23 11:56:12 +03:00
Timofei Larkin a3911811c7 feat(api): take Application name limits from the application's schema
The API server capped Kubernetes and Kafka names by knowing what those
two charts render: worker pool releases under a 17-character prefix,
controller pods with a 13-character suffix. That is the charts'
knowledge sitting in the platform, and it does not generalise. The next
application with a sub-resource budget needs another branch on Kind,
and an application served from outside this repository cannot get one
at all.

An application now declares the limits on its own name in its schema,
under x-cozystack-name, and the server applies whatever it finds there
for whatever kind the ApplicationDefinition describes. A declaration
only tightens: prefix + name must still fit the 53-character Helm
release name limit. A rejected name is answered with the chart's own
explanation of the limit.

The schema is untrusted input, and definitions from other repositories
are written by hand. A declaration that cannot be enforced as written,
whether an unknown keyword, a wrong type, a limit no name satisfies or
a pattern Go cannot compile, refuses creates of that kind with a
configuration error. Dropping it would admit names the chart cannot
render; applying it would reject every name and blame the user.

An upgrade brings cozystack-api up before the definitions it serves are
re-rendered, so for a while the new server can read a Kubernetes or
Kafka definition that predates its declaration. The two old caps stay
as a floor for exactly that case, a definition that declares nothing,
until no supported upgrade starts from one.

Assisted-by: LLM
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
2026-09-23 10:59:51 +03:00
Aleksei Sviridkin 97cb2781db test(e2e): pin the release-history read against the upstream type
The test that claimed to pin the HelmRelease v2 history shape wrote a
HelmRelease document containing the field names it expected, then read
that same document back with the same jsonpath. It established that yq
can read a fixture the test itself composed.

Its own comment described the failure it was meant to catch: a future
Flux release renames the field, the jsonpath matches nothing, the guard
reports no remediation cycle, and real cycles pass the e2e assertion. A
rename moves neither the fixture nor the expression, so the test stays
green through exactly that event. The comment also named where the truth
lives, github.com/fluxcd/helm-controller/api/v2, and pinned a copy of
the author's expectations instead.

Replace it with a Go test that supplies neither half. The expression is
read out of the shell library that ships it, and the object is built
from the upstream type at the version go.mod holds, then serialized and
matched the way kubectl does. A renamed or retyped field is a compile
error or a failed match; a guard that stops reading history fails the
lookup with a message saying so. Both directions were confirmed by
mutation.

Locating the expression by searching the file is not enough on its own.
Inline in a long script, locating it means deciding which occurrences
are code and which are prose, and every way of getting that wrong fails
green: an example quoted near a broken read satisfies the pin while the
guard reads nothing.

So the expression moves into a named assignment in remediation-guard.sh,
which the guard already sources, and the test sources that library and
takes the value the shell is left holding. Deciding which text is the
assignment is then not the test's problem: an `export`, an indent, a
second assignment later in the file all resolve the way the guard will
resolve them, because the thing resolving them is the shell.

Sourcing borrows the environment along with the shell, so the snippet
unsets the name before it sources. `go test` hands the child its own
environment, and a shell that had exported this name would answer in
place of a library that stopped assigning it: the value comes back
non-empty, the empty check stays quiet, and the pin passes over a guard
that reads nothing. That is this file's own subject reaching it through
the environment rather than through the source.

Moving the expression out of the call site puts a step between what is
pinned and what runs, so the call site is checked too, and the check
starts from the call rather than from the text: the local the guard is
handed must be assigned exactly once, by an anchored assignment whose
jsonpath is the expansion of the shared name. That a well-formed read
exists somewhere in the file is a weaker claim than it reads as, since
a second assignment to the same local, or a read filling another one,
satisfies it while the guard runs a literal nothing here pins. The
expansion and not the name: kubectl echoes a braceless template back
verbatim, so a read that dropped the `$` would return its own text,
clear the script's empty-history backstop, and carry no status the cycle
check matches. The library is checked by sourcing rather than by
reading, so an assignment the shell would run is resolved by the shell
instead of by a guess about which text the shell would pick. Each rule
is pinned by its own test, so a regression names which one it opened
rather than only that extraction broke.

How kubectl's printer treats an absent path is its own configuration
rather than part of this contract, so it is left unasserted: guessing at
it would put the expectation and the fixture back in the same hands.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-22 23:41:35 +03:00
Aleksei Sviridkin 9efe78ce44 test(e2e): hold the tenant-cluster install wait to an ordering rule (#3802)
A test that creates a tenant `Kubernetes` cluster must not end while
that release is still installing. The kind drops the install wait on its
main resources, but the chart's blocking post-install hook still holds
the install open, so a test that asserts and ends hands the rest of it
to teardown. Teardown then reds on a step that tests nothing. Reading
the cluster mid-install is fine. Ending mid-install is not.

`hack/e2e-chainsaw/_lib/run-kubernetes.sh` already waits for the parent
release before its tenant-cluster assertions, and that wait is older
than this change. Nothing held it there. This adds the guard that does.

The guard reads the script, not Chainsaw fixtures: the cluster comes
from a heredoc, so a guard walking `apply.file` would find nothing to
check and pass. It requires the CR, then a readiness wait on the parent
release, then the assertions the test ends on. The wait has to block,
and it has to be unconditional in the body that holds it. One more case
holds this script to being the only creator of that kind, since a second
one would be governed by no rule here.

The ordering is read within one function body, so splitting
`run_kubernetes_test` reds the unit lane until the rule is pointed at
the new shape.

Timeout length stays with review. The floor depends on where the wait
sits, and the script text does not carry that.

Which spellings each matcher takes, which rewrites it refuses instead of
judging, and the blind spots it names are written in the guard's header
next to the code, each with its mechanism and whether it fails loudly or
quietly. Fixtures mark their own lines, so a probe resolves by name
instead of by a line number that drifts.

No runtime code here: the guard, its fixtures, and the conventions that
state the rule.

```release-note
NONE
```
2026-09-22 19:00:52 +04:00
myasnikovdaniil 41ef4a6437 fix(ci): teach the overlay to repair an unstamped images/*.tag ref (#4362)
## What

`hack/overlay-main-images.sh` walks `images/*.tag`, but every branch of
its line classifier needs either a YAML key or an `@sha256:`. A `.tag`
file holds one bare reference and nothing else, so until a build stamps
it the line has neither, the drift guard reads it as a non-ref change,
and the committed reference survives.

For a package onboarded with the `:v0.0.0` placeholder this repo already
uses for a new image, that surviving reference is one no registry
serves. The overlay is the only thing that would replace it for a PR
that does not rebuild the package, so once such a package is on main
every full-suite run that skips its build keeps the placeholder. The
same placeholder inside a `values.yaml` repairs itself, because there it
sits under an `image:` key the classifier recognises. The difference is
the file shape, not the placeholder.

## Reproduction

A fixture holding a committed bare tag against an artifact copy carrying
a stamped one:

```
$ cd fixture/fakerepo && sh ../../hack/overlay-main-images.sh ../mainpkgs '[]' ''
overlay: packages/system/x/values.yaml -> current-main
drift (non-ref change) in packages/apps/x/images/x.tag -> keeping committed ref
```

Both files differ only in their reference. The `values.yaml` one is
repaired; the `.tag` one is not. Adding any digest to the same
placeholder makes it pass, which is what narrows this to the classifier
rather than the file.

## Why it has not bitten yet

17 of the 19 `.tag` files in the tree carry `@sha256:` and pass on that
branch alone. The only other digest-less one is
`packages/extra/seaweedfs/images/seaweedfs-cosi-driver.tag`, and it
survives only because it is third-party: `build-main` never rebuilds it,
the artifact copy is byte-identical, and the classifier is never
reached. The first digest-less first-party `.tag` is what turns the gap
live, and one is coming in #3426.

## The fix

`hack/promote-rewrite-tags.sh` already carries a second match branch for
exactly this shape, with a comment saying why ("the second branch is
what catches a bare `.tag` file, whose content has no YAML key at all").
This gives the overlay the same one, scoped to `*.tag` so an unkeyed
line in a `values.yaml` still counts as drift, because only in a `.tag`
file is a whole line known to be a reference and nothing else.

Two tests in `hack/overlay-main-images_test.bats`: the unstamped
placeholder is overlaid, and a `.tag` whose non-reference line differs
still counts as drift, so the rule cannot quietly widen into "every line
in a `.tag` file is a reference". Removing the fix fails the first one.

```release-note
NONE
```


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

- **Bug Fixes**
- Fixed image overlay handling for `.tag` files containing bare image
references, allowing current references to be applied correctly.
- Preserved drift detection when `.tag` files also contain unrelated,
non-reference changes.

- **Documentation**
- Clarified how image-reference tooling handles bare references and
package-tree processing.

- **Tests**
- Added coverage for bare-reference overlays and mixed reference/comment
changes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-22 18:53:28 +05:00
Alexey Artamonov ac92b6b95c fix(e2e): cap the bucket verify S3 reads so the step honors its timeout
A Chainsaw `script` op runs its shell under exec.CommandContext with
stdout and stderr wired to in-process buffers. When the op timeout
fires it SIGKILLs the shell process alone, not its descendants, and
cmd.Wait() does not return until every process that inherited the pipe
fds closes them. An mc that keeps retrying an S3 call after the shell
is gone holds the pipe open, so the op runs to mc's own exhaustion
instead of stopping at its ceiling. This is why the bucket suite's
verify-every-bucket-takes-a-write step ran 606s under a 300s ceiling.

verify-s3-access shares that shape, so wrap each S3 and apiserver read
in its own `timeout`: a hung read dies at a known budget and the op
timeout stays a backstop. The 30s kubectl cap matches the terminal `k`
wrapper the redis suite carries. The two `mc cp` PUTs take 45s, the
budget verify-every-bucket-takes-a-write already gives a write, because
a working PUT can still wait while the master grows a volume for a
freshly created bucket; 15s left too little headroom on that grow and
could turn a slow-but-healthy write red on an unrelated PR. The 15s cap
on the mc reads has no terminal precedent: the backup-access preflight
caps `mc cp` at 15s too, but inside a retry loop where a 124 costs one
attempt rather than the step, so it is not one. Land each secret's
encoded value in a file before base64 -d so a timed-out kubectl read
(exit 124) stops the step under set -e rather than masking as an
empty-credential mc alias set further down.

The readonly-upload deny probe branched on mc directly (if mc cp ...),
which suspends set -e, so a timeout there (exit 124) was
indistinguishable from the access-denied non-zero the probe expects: a
hung deny path would exit 124, take the false branch, and reach a green
cleanup, the exact window this change bounds. Capture the status
instead: exit 0 is the readonly regression, 124 is a timed-out probe,
both fail; any other non-zero is the expected denial.

The suite's own catch read (the `kubectl get pods` in namespace
cozy-objectstorage-controller) carried neither an op timeout nor an
inner cap, so it inherits timeouts.exec (5m) and can hold the pipe the
same way; give it a 30s inner cap and a 1m op timeout so the rule the
rest of the file follows holds for it too.

The .chainsaw.yaml notes still read the op ceiling as a reliable cut,
the old SIGKILL as a de-facto cap and a smaller timeout as guaranteeing
a kill; correct them, since the shell-only kill plus the pipe wait are
why an inner cap on every read is required rather than optional.

Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
2026-09-22 15:16:25 +03:00
Timur Tukaev e32353e839 docs(agents): correct the labeler's trigger list (#4090)
## What this PR does

The PR Title Auto-Labeling section still lists `edited` among the events
`.github/workflows/pr-labeler.yaml` runs on. That trigger was dropped in
#3211 — the workflow carries the note at the top of its `on:` block, and
the v1.6.0 changelog records the change — so the document and the
workflow have disagreed since.

I hit it the way the document's intended reader would. Three PRs opened
with GitHub's branch-derived titles, corrected afterwards to
Conventional Commits, and the labels did not follow:
`area/uncategorized` on all three and no `kind/feature`, because nothing
re-ran. The second paragraph this PR adds is that consequence, which is
the part a contributor actually needs: a corrected title does not
re-label until the next push or a reopen, and since the labeler only
ever adds, the `area/uncategorized` left by the first title stays until
a human removes it.

### Screenshots

Not applicable — no UI change.

### Downstream repositories

I walked the trigger map in `docs/agents/contributing.md` against this
diff. The only file it touches is that document, and the two triggers
pointing at it are narrower than this change: `cozystack/community`
restates the **commit convention** from it (types, scopes, sign-off, the
`Assisted-by: LLM` trailer) and the **PR template and downstream
checklist**, and `cozystack/ccp` gates its `package-bump` skill on the
same commit convention. Neither restates the labeler's event list, which
is what changes here. Nothing else in the map reaches a docs-only
change.

- [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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
NONE
```


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

## Summary by CodeRabbit

- **Documentation**
  - Clarified when pull request title auto-labeling runs.
  - Documented that title edits do not trigger relabeling.
- Explained that existing labels remain until a push or reopen, while
additional uncategorized labels require manual removal.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-21 17:25:03 +03:00
Myasnikov Daniil 1118b340a7 fix(ci): teach the overlay to repair an unstamped images/*.tag ref
hack/overlay-main-images.sh walks images/*.tag, but every branch of its
line classifier needs either a YAML key or an @sha256:. A .tag file holds
one bare reference and nothing else, so until a build stamps it the line
has neither, the drift guard reads it as a non-ref change, and the
committed reference survives.

For a package onboarded with the :v0.0.0 placeholder this repo already
uses for a new image, that surviving reference is one no registry
serves. The overlay is the only thing that would replace it for a PR
that does not rebuild the package, so once such a package is on main
every full-suite run that skips its build keeps the placeholder. The
same placeholder inside a values.yaml repairs itself, because there it
sits under an `image:` key the classifier recognises: the difference is
the file shape, not the placeholder.

The gap is latent rather than new. 17 of the 19 .tag files in the tree
carry @sha256: and pass on that branch alone; the only other digest-less
one, extra/seaweedfs/images/seaweedfs-cosi-driver.tag, is third-party,
so build-main never rebuilds it, the artifact copy is byte-identical and
the classifier is never reached. The first digest-less first-party .tag
is what turns it live.

hack/promote-rewrite-tags.sh already carries a second match branch for
this shape and states why. This is that branch, scoped to *.tag so an
unkeyed line in a values.yaml still counts as drift.

Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
2026-09-21 14:42:41 +05:00
Aleksei Sviridkin 5fe595d4c0 test(e2e): hold the tenant-cluster install wait to an ordering rule
A test that creates a tenant Kubernetes cluster cannot END while that
release is still installing. A Helm release cannot be uninstalled
mid-install, and for this kind the install can outlive the fast
assertions: the kind carries helm-install-disable-wait, which drops
the wait on the main resources but not on the chart's blocking
post-install hook. A test that asserts and ends hands the remainder of
the install to teardown, where it is reported red on a step that tests
nothing.

The rule is about ending, not about touching the cluster at all. The
hook polls for the admin kubeconfig in the modes that have one, and
that Secret is what the test needs to reach the tenant API, so
bringing up a worker or waiting on a child release happens while the
parent is still installing. Reading mid-install is fine.

run-kubernetes.sh already does the right thing: it waits for the
parent release to reach Ready before the tenant-cluster assertions.
Nothing held it there. This adds the guard that does, and states the
rule where the e2e conventions live.

The guard reads the script rather than Chainsaw fixtures, because no
fixture applies this kind and a guard walking apply.file would pass by
having nothing to check. A case asserts that this stays the only
script creating it, and the sweep behind it keys on the apiVersion and
kind together so that an indented apply.resource block counts as a
creator too. It reads scripts and manifests only, and that rule is
pre-emptive rather than a response to what the root holds today: a
manifest quoted in prose or parked in a `.disabled` file creates no
cluster, and this repository writes the first shape in docs that lie
outside the root.

The rules it holds the script to -- what counts as a wait with teeth,
where in the body it has to stand, what may not switch errexit off
above it, and which call shapes count as an end-assertion -- are in
the file's header beside the code that enforces them, each with the
spellings it accepts and what it cannot see. How to size the budget is
derived once in the e2e conventions.

Nothing is added at the wait itself: hack/e2e-chainsaw/_lib is shared
by every Chainsaw suite, so a comment there escalates this change from
no e2e suites to the full suite, and the guard already reds on the
move it would have warned about.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-11 09:06:19 +03:00
Myasnikov Daniil 975ea6acb7 ci(workflows): gate Talos builds on their inputs
Classify the rename-blind changed-path list against direct Talos
sources and shared build inputs. This lets same-repository pull
requests skip the heavy build while removals, renames, and build
configuration changes still select it. Fork builds remain
unconditional.

Preserve the matchbox digest produced by a successful Talos build when
finalize overlays base-branch image references. When the job is
skipped, bootbox continues to receive the current base reference.

Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
2026-09-10 17:48:22 +05:00
Andrei Kvapil 774f788016 ci(agents): check commit trailers on every pull request (#4103)
## What this PR does

`docs/agents/contributing.md` makes a few properties of a commit message
blocking at review: the attribution trailer takes the value `LLM` and
nothing else and appears once, no message carries a session trailer or a
link to a transcript, no line is a generation byline naming the tool.
Nothing checked any of it, so the only reader was a human scrolling `git
log`. That reader is expensive and misses things. One open PR carried
eight commits naming a model in the trailer, another four, all found by
hand after the work was done, each one costing a review round and a
rebase.

`hack/check-commit-trailers.sh` decides all of it from the text, and the
`Commit trailers` job in `.github/workflows/pre-commit.yml` runs it over
the commits between the merge base and the head.

Over all 6062 commits on `main` it rejects 1300, every one a true
positive and no prose false-fires: 1289 attribution trailers naming a
model, 24 `Generated with <tool>` bylines, 15 commits carrying two
trailers, two `claude.ai/code/session_...` links.

### Where it lives

Next to the other per-PR text lint in `pre-commit.yml`. The unit-test
job in `pull-requests.yaml` was the other candidate and is wrong for
this: it is gated on the diff touching code, so a docs-only PR skips it,
and a trailer is wrong or right whatever files the commit changed.

The range is `merge-base..head`, recomputed every run, so a rebase or a
force-push is read as it now is. It never reaches the history on `main`,
roughly a quarter of which carries the old form.

### Decisions worth arguing with

It fails rather than warns. A warning in a job nobody watches guards
nothing.

It is not a required check, and that part is deliberate. Branch
protection on `main` requires `pre-commit` and `E2E Tests`, and `Commit
trailers` is a new context, so a PR can still merge with it red. 77 of
the 200 open PRs carry at least one commit this rejects, so requiring it
today turns those red at their next push. Flipping it is a
branch-protection change once the open set has drained, and somebody
else's call.

Fork PRs are checked like any other. The job runs on `pull_request` with
a read-only token and reads commit messages as data.

A merge commit with no trailer of its own passes. Nothing is required of
a commit, only what is written is judged, so a merge commit that does
carry a bad trailer fails like the rest.

Backport branches are exempt. The backport action cherry-picks merged
commits verbatim, so those messages are history and not the author's to
rewrite. The exemption matches the generated name
`backport-<pr>-to-<branch>`, which is the only signal a `pull_request`
event carries, since the label sits on the original PR. So it is a
convention, not something enforced.

The checker is read from the base branch instead of the checked-out PR.
A `pull_request` event takes the workflow definition from the merge of
base and head, so the job shows up on every open PR the moment it lands,
including the ones whose head predates the script, where reading it from
the head is exit 127 and a red check saying nothing about trailers. It
also keeps the checker out of reach of the PR being measured, though the
job around it is not.

Two shapes are left to the reviewer. A model named in `Co-authored-by:`
needs a list of model names to tell it from a person, and such a list
either misses next month's model or fires on somebody's colleague; most
of the 771 such lines on `main` name a model, so it would be deciding a
lot of PRs. A `Generated-by:` trailer has never appeared here at all.
The byline rule matches a capital `G` for a related reason:
case-insensitively, one line in 25 on `main` is an ordinary sentence
that wrapped onto "generated with cozyvalues-gen".

### Known costs

`hack/check-commit-trailers.sh` matches `hack/[^/]+\.sh$` in
`select-e2e.sh`'s `full_suite_pattern`, so this PR and any later one
touching the script escalates to the full e2e suite. That pattern's own
comment scopes it to the e2e harness, which this script is not, and
`inert_config_pattern` already carries `hack/[^/]+\.bats$`. Escalation
is fail-safe so it is cost rather than breakage, and reclassifying it
wants its own review.

The step drops the explicit `git fetch origin "$BASE_REF"` that
`api-review-gate.yaml` keeps before its own `merge-base`. `fetch-depth:
0` already fetches `+refs/heads/*:refs/remotes/origin/*`, and if that
were ever wrong the failure is a loud `merge-base` error under `set -euo
pipefail`, not a silent pass.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map against the diff. The two entries that could fire
are `cozystack/ccp` and `cozystack/community`, both keyed on a change to
the commit convention itself. The convention text is untouched here,
only its enforcement is new, so both copies stay accurate. `ccp`'s
`package-bump` skill already writes `Assisted-by: LLM`, which this check
accepts.

- [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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
ci(agents): a pull request now fails when one of its commits carries an attribution trailer whose value is not `LLM`, a second attribution trailer, a session trailer or transcript link, or a `Generated with <tool>` byline. The check reads only the commits the pull request adds; backport branches are exempt.
```


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

## Summary by CodeRabbit

* **New Features**
* Pull requests now include automated checks for commit attribution
trailers, session links, and generation bylines.
  * Backport branches are excluded from this validation.

* **Documentation**
* Added guidance describing the commit-message checks, accepted
attribution formats, exceptions, and how to run the validation locally.

* **Tests**
* Added coverage for valid and invalid commit-message formats, range
handling, merge commits, and exempted content.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-09 23:02:10 +02:00
myasnikovdaniil 94a202bc5a test(e2e): run the merge-gating lanes on Talos containers (#4020)
Replaces QEMU substrate with Talos containers on both lanes that gate a
merge. srv1-srv3 run as containers, so tenant worker sits at L2 instead
of L3.

Measured in CI on commits where both substrates ran the same tree:

| | qemu | container |
|---|---|---|
| `kubernetes-latest` / `-previous` | 1143-2040s, red on every
vgif-absent runner | 466-809s, green in both strata |
| whole job | | 24-49 min faster |

Almost all of it is in Chainsaw. Install takes about half an hour either
way.

Node-join soft-red gate goes with it. It was a prosthetic for nested
virt: a worker that registers in minutes elsewhere could miss any
deadline this test can afford, and #3513 established the discriminator
is host `kvm_amd` `vgif`, not anything about the product. Remove the
nesting level and there is nothing left to tolerate, so
`hack/e2e-node-join-soft-red.sh`, the marker and the `soft_red` outputs
are gone from all four workflows.

### What moves off the per-PR path

DRBD, and with it `replicated` StorageClass, its `Immediate` binding
mode, tenant StorageClass propagation (applies only to
remotely-accessible classes), and cozystack talos node image with its
extensions. None of it is lost - `nightly.yaml` and `e2e-tag.yaml` still
run the suite on QEMU, so all four keep nightly and release-candidate
coverage. Live migration is not on that list because nothing tests it
today anyway.

One more belongs on the list and was missing from it: the nocloud disk
build. `build-talos` no longer runs `make -C packages/core/talos
talos-nocloud`, so `images/talos/profiles/nocloud.yaml` is compiled by
`nightly.yaml` and by the tag build's `make assets` rather than by
anything a PR runs, and `promote-rc.yaml` still hard-fails a promotion
whose rc release has no `nocloud-amd64.raw.xz`. Nothing in that chain
breaks, because the tag build supplies the asset as before. What a PR no
longer catches is narrow: it still compiles `installer.yaml` through the
same imager, and the two profiles are generated by the same script and
differ only in `platform`, `kind`, `imageOptions` and `outFormat`.

### Product fixes

Four, all found by measurement while building the lane, each on its own
commit:

The linstor `drbd.enabled` gate. Without it drbd-logger sidecar exits,
satellite is never Ready, piraeus registers zero nodes and every PVC
hangs Pending behind a cluster that looks healthy.

linstor `--strict-topology`. Without it external-provisioner passes
every topology segment as `requisite`, so a node-pinned volume can be
provisioned away from its pod and the pod is then unschedulable forever
with nothing erroring anywhere.

kubevirt-cdi importer resources are configurable now. Default restates
CDI's own values exactly so production behaviour does not move, and the
lane raises the ceiling through its own Package.

vm-disk binds standalone disks immediately. A VMDisk on `local` silently
never populated, while the same disk on `replicated` did.

### Where the run stands

The lane has gone green twice, most recently on `cc70cef7b` where `E2E
(in-tree)` took 2h3m against its 215m cap with 45 Chainsaw tests
passing. The measurements quoted elsewhere in this description come from
the first of those runs, on a commit that has since been rebased away:
the two tenant-Kubernetes suites came in at 569.72s
(`kubernetes-latest`) and 470.19s (`kubernetes-previous`) against a 67m
operation. Both runs answer the four earlier reds, all of which were the
lua-protobuf segfault from #4012 once `vminstance`/`vmdisk` were fixed
on the branch.

ghcr.io mirror removal is carried here too and is the same change as
#4007, whichever lands first makes the other a no-op.

### What review changed

`drbd.enabled=false` now refuses to render together with
`talos.enabled=false`. It removed the logger sidecar only, while the two
DRBD initContainers piraeus contributes unconditionally are deleted by
the Talos configuration alone, so that combination produced a satellite
that can never reach Ready. `isp-full-generic` sets
`talos.enabled=false`, so it was reachable and not theoretical.

vm-disk keeps unconditional immediate binding, a standalone disk has no
consumer to wait for. What it costs on a node-pinned class is in the
chart README now, and the annotation is pinned per source type and on
the existing-DataVolume path where `helm upgrade` retrofits it.

The rest is the harness holding its own promises: HelmRelease gate reads
are bounded and its minimum-count half is pinned by a test that fails
without it, two Chainsaw ops sit above the deadlines they contain, the
fallback-StorageClass render restores `replicated` when it fails,
container lane compose project and pool files are per-checkout, and
three comments that said the opposite of the code now say what is true
(QEMU is still wired into nightly and e2e-tag, the lane replaces two
Packages and not one, the argument-count assertion cannot see upstream
drift).

### Release note

```release-note
fix(linstor): CSI provisioning is constrained to the node the scheduler chose (`--strict-topology`), and the DRBD sidecar can be switched off with `drbd.enabled` on substrates whose kernel cannot run DRBD. On an existing cluster the new default re-templates `spec.csiController.podTemplate`, so piraeus rolls `linstor-csi-controller` once during the upgrade. `drbd.enabled=false` requires `talos.enabled=true` and the chart now refuses the other combination at render time, because only the Talos satellite configuration removes the DRBD initContainers.
fix(kubevirt-cdi): CDI worker pod resources are configurable through `importerResources`, which reaches the importer, the uploader and the host-assisted cloner. The default restates CDI's own built-in values exactly, so an install that sets nothing behaves as before.
fix(vm-disk): every disk requests immediate binding, not only `upload` sources, so a standalone disk on a WaitForFirstConsumer StorageClass is populated instead of waiting for a consumer that may never arrive. On a node-pinned class such as `local` the volume then binds where CDI's worker pod was scheduled rather than where the VM will run; see the chart README before choosing such a class for VM disks. The default `replicated` is unaffected.
```

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

* **New Features**
  * Added container-based end-to-end testing with local storage support.
* Added optional backup-access preflight checks for database backup
examples.
* Added configurable CDI worker resources and LINSTOR storage settings.
  * Added configurable immediate storage binding for VM disks.
* Added stronger readiness checks, diagnostics, and parallel test
execution.

* **Bug Fixes**
* Improved cleanup failure reporting, capacity validation, and
missing-image detection.
* Improved FoundationDB health verification and storage placement
behavior.

* **Removed**
  * Removed GHCR mirror support and soft-red node-join tolerance.
  * Removed standalone OIDC render test suites.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-07 19:33:20 +05:00
Aleksei Sviridkin d45663f1af ci(agents): check commit trailers on every pull request
The attribution and session-link rules in docs/agents/contributing.md
are decided by the text of a commit message alone, and until now the
only reader was a human scrolling git log. That reader is expensive and
misses things: one open pull request carried eight commits naming a
model in the attribution trailer, another four, all found by hand after
the work was otherwise done. Every one of them cost a review round to
report and a rebase to fix.

The check reads what is written and requires nothing, so a commit with
no trailer passes and a merge commit that carries none passes with it.
A trailer that exists and is wrong fails, naming the commit and the
accepted form. It runs over the commits between the merge base and the
head, so it never reaches the history on main, roughly a quarter of
which predates the rule. A backport branch is exempt: its commits are
cherry-picked verbatim and their messages are not the author's to
rewrite.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-07 02:38:02 +03:00
Myasnikov Daniil c748c5b707 docs(e2e): drop the references the removed suites and carve-out left behind
Convention 7 still sent readers to the node-join carve-out, while
section 1 of the same file -- rewritten on this branch -- says the
script, the marker and the soft_red output no longer exist. Restated as
what the deadline now is.

hack/e2e-chainsaw/README.md still listed kubernetes-oidc-system and
kubernetes-oidc-customconfig among the suites, and select-install.sh's
suite_to_source() still mapped both. The mapping is harmless today
because the round-trip pin walks existing directories, but select-e2e.sh
says the two functions have to stay in step, so a stale half is a trap
for whoever reads that promise.

sandbox-runner-headroom.bats keeps reading the QEMU sandbox file for
every lane, container lanes included: `prepare-env` also matches
`prepare-env-container`, and the compose file gives its nodes the same
8 CPU / 24576 MiB by design. The figures are therefore right for both
substrates, but only because the two agree -- and that they agree is
pinned in container-lane-capacity_test.bats rather than here. Said so in
the test, so the next reader knows what would have to change if the
container lane is ever sized differently.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
2026-09-05 14:51:06 +05:00
Myasnikov Daniil 12157a2f93 docs(agents): describe the container lane as the merged standard
The container lane moves out of "In-flight direction (not yet the merged
standard)" and into the body, because it is now what both merge-gating
lanes run. The node-join section is rewritten the other way round: it
described a deadline that failed the suite without blocking the lane,
and there is no longer any such carve-out to describe.

What the section gains is the part that was missing while the lane was a
spike -- what moves off the per-PR path with the substrate, and where it
still gets covered. DRBD and the replicated StorageClass were already
named; Immediate binding, tenant StorageClass propagation and the
Cozystack Talos node image were not, and all three now say nightly and
release-candidate rather than reading as lost. The host CPU and memory
visibility gap is stated too, since it is what a reader meets first on a
red run and it looks like a product bug.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
2026-09-05 13:19:18 +05:00
Marian Koreniuk f8abaa6249 docs(agents): correct the labeler's trigger list
The auto-labeling section still lists `edited` among the events the labeler
runs on. That trigger was removed in #3211, and the workflow carries a note
explaining why: it fires on every body edit, so long-lived bot PRs re-ran the
job dozens of times on an unchanged commit.

The gap is not cosmetic for the reader this document is written for. Acting on
it means correcting a wrong PR title and expecting the labels to follow, which
they do not until the next push — and since labels are only added, never
removed, the `area/uncategorized` the first title produced stays behind either
way.

Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
2026-09-05 09:52:49 +02:00
Aleksei Sviridkin 3a369fed99 feat(redis): add opt-in TLS for Redis and Sentinel (#2729)
## What this PR does

`tls.enabled` puts Redis and Sentinel on a TLS-only listener with a
per-release cert-manager chain, and hands the tenant the CA certificate
as `redis-<name>.tenant-ca`. Off unless set.

redis-operator v3.3.5 has no TLS support. The work is ported upstream in
freshworks-oss/redis-operator#85, which is open and not in a release, so
this carries it as three patches over the v3.3.5 tarball.
`images/redis-operator/patches/tls.diff` is the source half, cut from
that PR's commit `2ad2ed8d1`, which the patch header pins; it is applied
by the glob the image Dockerfile already runs, after the Dockerfile
strips the tarball's `manifests`, `charts`, `example` and `.github`, so
the image builds from source alone and the upstream tests that read the
CRD YAML skip instead of failing against the stale v3.3.5 copy.
`patches/crd-tls.diff` adds `spec.tls` to the vendored CRD, and with it
the CEL rule that fixes `tls.enabled` at creation; without the schema
block the apiserver prunes the field on admission and the patched binary
waits for something it never sees, and the rule cannot exist without the
block it references. `patches/rbac-tls.diff` grants Create, Get and
Update on Secrets, so the operator can publish the CA-only Secret it
addresses by name. That is the whole delta, and it is narrower than the
ported operator's own chart asks for: those three are the only Secret
calls in the patched source, and cert-manager Certificates get no grant
at all, because the operator only touches one under
`spec.tls.certManager` and this chart pins `certificateSecret` mode
instead. One change in `tls.diff` is not TLS and takes effect with TLS
off, named in the patch header: `MakeSlaveOfWithPort` connects to the
replica on its own port rather than on the master's. It changes a dial,
not a pod template, so shipping the rebuilt image rolls nothing; the
plaintext pod templates stay byte-identical to v3.3.5.

The image build registration this branch used to carry (`make -C
packages/system/redis-operator image` in the root `build:` target)
landed on main with the Valkey service (#3380), so the rebuilt operator
image is produced by CI from main as well; the digest in `values.yaml`
is still refreshed by release preparation, not here.

The chart patches re-apply at the end of `update:`, the same way
reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji carry
theirs. Otherwise `rm -rf charts` takes TLS back out on the next `make
update` and nothing fails. `make update` reproduces the committed tree
byte for byte.

TLS is chosen when the instance is created. The config gets `port 0` and
the listener moves to `tls-port`, so TLS replaces plaintext rather than
running beside it, and a running instance cannot change mode: a replica
restarted onto the TLS listener cannot replicate from a master still on
plaintext, and Sentinels already on TLS would fail over to a stale
replica. The RedisFailover schema in the carried CRD patch rejects a
change of `tls.enabled`, which on an existing instance surfaces as a
failed HelmRelease upgrade while the instance keeps serving what it did;
the README points at creating a new instance and migrating. That is also
why TLS is not inferred from `external`: an externally reachable
instance is the one most likely to have clients an upgrade must not
drop. Existing releases keep their plaintext listener and are not
restarted by the rebuilt operator image.

The tenant gets `ca.crt` and nothing else, through `tenantsecrets`. The
chart's own Secrets take a dot between the release and the suffix
(`<release>.ca-tls`, `<release>.tls`, `<release>.ca-cert`): the suffixes
are dot-free, so the split is unique and release `foo`'s CA Secret can
no longer be release `foo-ca`'s leaf, the collision the hyphenated form
had. The chart renders a `TenantProjection` pointing at
`<release>.ca-tls`, and the CA-extraction controller re-encodes the one
named key from the parsed certificate, so nothing that is not a
certificate gets through. Same path as postgres, mongodb, nats and
qdrant.

No Secret of this release is granted to the tenant by name beyond the
generated password, and that includes `<release>.ca-cert`, the CA-only
copy the operator publishes for itself. It holds no key today, but the
operator has Create and Update on Secrets cluster-wide, so what sits at
that name is the operator's call rather than the platform's, and RBAC
cannot filter by key. `hack/check-redis-rd-secrets.bats` pins the whole
rendered Secret surface instead of a list of known-bad names, because an
include with no `resourceNames` and no `matchLabels` restricts nothing.

The chainsaw suite for redis gains two cases. A TLS fixture asserts the
cert-manager chain is issued, the TLS-probed Redis replicas and Sentinel
come up, the operator publishes its CA-only Secret under the widened
ClusterRole, the tenant receives exactly `ca.crt` through
`tenantsecrets` while the key-bearing Secrets stay off that path, and a
`redis-cli --tls` PING through the service name completes with the same
CA. A second case applies `tls.enabled` to a running plaintext release
and asserts the refusal: the HelmRelease upgrade fails with the schema's
message, the StatefulSet keeps its revision with both replicas Ready,
plaintext still answers and no TLS listener appears.

Certificate renewal rolls the pods from inside the operator. Redis and
Sentinel read the certificate once at startup, so the patched operator
stamps a hash of the TLS Secret's `tls.crt` and `ca.crt` into the pod
template of both the Redis StatefulSet and the Sentinel Deployment on
every reconcile, as `redis-failover.freshworks.com/tls-secret-hash`. A
renewal changes the hash; the operator's own roller replaces the Redis
pods one at a time and the Deployment controller rolls Sentinel. README
names the fallback: if the roll cannot run, the deadline is the old
certificate's expiry, past which the operator's own TLS connections fail
along with the clients.

`tls.authClients` maps to `tls-auth-clients` and defaults to `no`. The
platform mints no client certificate for the tenant and the CA key is
out of reach, so `yes` is only usable when the client certificate comes
from whoever also controls the CA. Changing the value on a running
release restarts the Redis and Sentinel pods: Redis reads the directive
from its config file only at startup, so the operator folds the
effective value into the certificate hash that drives the rollout, and
the change lands when it is made rather than at the next unrelated
restart.

Follow-ups in the carried operator patch go upstream first, to
freshworks-oss/redis-operator#85 or a successor there, and are picked up
here afterwards; the carried patch does not grow Go that upstream has
not seen. `tlsConfigFor` fetches the Secret on every call, tens of GETs
per reconcile. The renewal roll is static analysis so far: the live
check, `cmctl renew` on a running release while watching the `rfr-*` and
`rfs-*` pods roll, is still owed and belongs with the e2e follow-up.

Two facts about the shipped image rather than the patch.
`packages/system/redis-operator/values.yaml` keeps the digest of the
image built before the patch until release preparation rebuilds it, so
on `main` between this merge and the next release-prep, `tls.enabled`
renders the chain and `spec.tls` while the running operator ignores them
and serves plaintext. And the pinned v3.3.5 tarball carries GO-2026-5026
in `golang.org/x/net` v0.53.0, reached through
`PodService.UpdatePodLabels`; the port neither introduces nor fixes it,
upstream `master` is already on v0.55.0, so it lands with the next tag
bump rather than here.

Outside the patch: `cozyvalues-gen` renders `{}` in the README Value
column for `tls.authClients`: a child property without a default
inherits the parent object's `{}`, and `make generate` reproduces it
byte for byte, so it is a generator bug rather than something to
hand-edit here.

### Screenshots

No UI changes.

### Downstream repositories

- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up: cozystack/terraform-provider-cozystack#37
- [ ]
[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:

I walked the trigger map against the diff file by file.
terraform-provider-cozystack is reached, tracked as
cozystack/terraform-provider-cozystack#37 rather than a speculative PR:
`values.schema.json` gains a `tls` object, and the provider hand-writes
a schema, model and expand/flatten pair per Kind. It already models
`tls` for postgres, kafka, nats and qdrant, not for redis. Redis needs
different wording from those four, whose descriptions say to omit the
block to follow `external`, which is the one thing redis does not do.

Nothing else is reached. The website regenerates this package's
reference page from `README.md` on a stable tag, so the README changes
need no PR there. No package added, renamed or removed.
`packages/core/platform/values.yaml`, variant and bundle composition and
release asset names are untouched, and so are `hack/package.mk`,
`hack/common-envs.mk` and `hack/update-crd.sh`, which is what ccp and
external-apps-example read. The two new files under `hack/` are bats
unit tests. No node prerequisite, installer value, telemetry metric or
cozy-proxy label changes.

### Release note

```release-note
feat(redis): add opt-in TLS for Redis and Sentinel. Set `tls.enabled: true` to serve both on a TLS-only listener backed by a per-release cert-manager chain; the tenant reads the CA certificate as the `<release>.tenant-ca` tenant secret. TLS is off unless set and is not inferred from `external`, so existing releases keep serving plaintext and are not restarted by the rebuilt operator image. TLS is chosen at creation: a running instance cannot switch modes, the change is rejected and surfaces as a failed HelmRelease upgrade, so add TLS by creating a new instance and migrating to it. Requires cert-manager, which the platform's `system` bundle installs on every variant. A renewed certificate rolls the Redis and Sentinel pods through the operator. TLS support is carried as a downstream patch over redis-operator v3.3.5 until freshworks-oss/redis-operator#85 lands in a release.
```
2026-09-04 19:32:32 +04:00
Aleksei Sviridkin 61026b732c docs(agents): require the neutral Assisted-by: LLM trailer
The attribution trailer exists for disclosure, not to advertise a
model. A model name in the trailer tells a reviewer nothing they can
act on, and it goes stale with every tool change, so the accepted form
is a single `Assisted-by: LLM` line however many tools took part.

A link to an assistant session is worse still: it rots, and it carries
nothing about the change itself, so no session trailer and no
transcript URL belong in a commit, a PR description, or a comment.

The commit body explains why a change is made; the diff already shows
what changed.

The hygiene rules for messages, trailers and in-code comments are
written down as review blockers for the same reason: a NOT LGTM on one
of them is then a stated rule, not a reviewer's taste.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-03 01:14:58 +03:00
Aleksei Sviridkin d2fcf41fb8 docs(agents): drop redis-operator from the build-target violations
The root Makefile build: list now carries the redis-operator image, so
the package stamps a reference from inside the list every other producer
iterates, and the image-refs rule no longer names it as a live violation.
flux-plunger, keycloak-operator and kilo still are.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-02 17:56:06 +03:00
IvanHunters 0fa6cf72a5 chore(ci): complete the backport label rename across all consumers
The initial rename missed several literal consumers of the old label
names beyond labels.yml and backport.yaml:

- stale.yaml exempt-pr-labels still listed backport,backport-previous.
  actions/stale matches label names exactly and does not resolve
  label-sync aliases, so after the rename a kind/backport PR would lose
  its stale exemption and get auto-closed after inactivity. This is the
  one behavioural regression; the rest are documentation drift.
- PULL_REQUEST_TEMPLATE.md told contributors to apply the backport label.
- release.md label-target table and the gh pr list --search commands
  referenced the old names (the searches would silently return nothing).
- contributing.md claimed a backport label is auto-applied on the
  [Backport] prefix; pr-labeler only adds area/release, so the line was
  already stale and is corrected while renaming.

Historical narrative in release.md and the backport scope-to-area doc
rows (a Conventional-Commit scope, not the label) are left as-is.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-27 13:08:15 +05:00
Aleksei Sviridkin ad62a8d62b test(e2e): stop blocking merges on the tenant node-join deadline
The tenant node-join wait no longer discriminates. Nested
virtualisation on the shared runners degrades far enough that a guest
which registers in minutes on real hardware can miss any deadline this
test can afford, so a red on that one wait says as much about the
machine the run landed on as about the product, and it teaches a
reader to re-run the job.

The suite still fails on it. That is what keeps Chainsaw's catch
running: the previous-instance container logs, the host snapshot, the
data-plane capture and the event dump are all keyed on the test
failing, and the report stays honest. What changes is one layer up.
The wait writes a marker beside its diagnostics, and a new lane-side
gate reads that marker to decide whether the job blocks. It opens only
when every failed test in the run carries one, and blocks on anything
it cannot attribute: a report it could not read, a failed test the
report does not name, a failure with no marker.

Only the lanes that block a merge ask it. The two pull-request lanes
do; nightly and the tag lane run the same suites and stay hard. A soft
pass on the tag lane would ship a release candidate whose tenant
clusters brought up no worker nodes, and nightly is the only place an
e2e failure reaches anyone who did not open a pull request, since no
lane in this tree notifies anywhere.

A softened red finishes the job green, so the job's result stops
answering whether the suite passed, and both kinds of consumer are
handed the decision instead of inferring it. A step gated on the job
failing would silently stop running: the SSH breakpoint is that case,
and it still opens for a pull request marked for debugging. A job that
reports the result would silently start reporting a pass: the required
commit status now says the deadline was missed and that the merge is not
blocked on it, rather than that the suite passed.

The deadline goes to 29m. That is the test's own budget and nothing
more: its clock starts after the machinedeployment wait, the
LoadBalancer assignment and the healthz probe, while the chart's
autoscaler and health-check timers have been running since the Machine
was created, so no ordering against them is claimed. The chart's 20m
and 30m give a scale to pick against, and a budget over the first
leaves room for a remediation cycle without promising that one fits.

The operation ceiling both suites give the script rises with the
deadline, and the diagnostics phase budget rises inside it, because
the collectors gated ahead of the serial console cost more between
them than the old budget covered. Both budget guards read their terms
from the sources that set them. The e2e job cap rises with them, and
its guard holds what a guard can hold: the pair's operation and catch
ceilings, and one figure across every lane that runs the suite. It
also records that the suites' full summed ceilings have never fit any
cap in that file, so the margin above the bound is taken from a
measured run instead. The prose that restates the cap elsewhere moves
with it, and the sweep that holds the deadline's restatements now
reads the figure spelled in words as well as in minutes.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-21 12:52:42 +03:00