Commit Graph
1084 Commits
Author SHA1 Message Date
IvanHunters b99db147a7 test(ci): isolate the commit-trailer fixture from global tag signing (#4271)
## What this PR does

`make_repo` in `hack/check-commit-trailers.bats` silences
`commit.gpgsign` in the fixture repository but leaves `tag.gpgsign`
alone. Anyone who signs tags has that setting globally, it reaches the
fixture, `git tag start` becomes an annotated tag, and git refuses it
for want of a message. The helper opens all 27 tests, so the whole suite
dies before a single test reaches its body, and `make bats-unit-tests`
takes `make unit-tests` down with it. Details and the reproduction are
in #4270.

The first commit is that one line, next to the one that is already
there. With it the suite went 27/27 on a machine where the global
setting is set, and was unchanged on one where it is not. I ran it both
ways.

CI never saw this. The `Commit trailers` job runs the checker against
explicit SHAs and does not touch the bats file, and a fresh runner has
no global git configuration to leak in. The failure only lands on the
local `make unit-tests` that `docs/agents/contributing.md` asks for
before pushing, which is the worst place for it: an unexplained red in a
file the contributor did not touch.

That is also how the guard went missing in the first place, and it would
go missing again the same way, so the second commit adds `make_repo
holds against a hostile global configuration`. None of the three `git
config` lines in `make_repo` is exercised on a runner: delete any of
them and CI stays green. The test supplies the hostile configuration
itself rather than waiting for a contributor to have one, so each guard
is held by the suite. Pulling them one at a time gives three distinct
reds: `tag.gpgsign` gives `fatal: no tag message?`, `commit.gpgsign`
gives `gpg failed to sign the data`, and `core.hooksPath` lets a global
`commit-msg` hook rewrite the message and fails the assertion on it. The
suite is 28/28 both with the global settings and without, so a runner
sees no change.

`hack/release-changelog-behaviour.bats` has gaps of the same kind in its
own fixture, which sets `commit.gpgsign` and nothing else. The missing
`tag.gpgsign` is inert there because it creates no tag. The missing
`core.hooksPath` is not: a global `pre-commit` hook that exits non-zero
kills every `git commit` it makes. I left both alone rather than widen
this diff, and noted them in the issue.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map against the diff. The only `hack/`-related
trigger is `cozystack/ccp`, and it fires on moving or renaming something
under `hack/` or changing what a make target does. This adds a line
inside an existing file; the filename and the make target behave as
before, except that the target stops failing.

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

- **Bug Fixes**
- Repository fixtures now remain reliable despite conflicting global Git
settings, including commit and tag signing, failing signing tools, and
commit-message hooks.
- Fixture setup preserves lightweight tags and the original seed commit
message, ensuring consistent repository initialization across
environments.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-15 15:09:10 +03:00
IvanHunters 379c763fc3 test(ci): drop an inert key from the fixture's hostile configuration
tag.forceSignAnnotated governs annotated tags and `git tag <name>` creates
none, so with tag.gpgsign unset the tag still resolves to a commit object. The
key cannot make the new test fail and guards nothing the fixture could lose;
carrying it implied a fourth guard that does not exist.

Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-09-15 14:52:49 +03:00
IvanHunters 80ab355d66 test(ci): hold the fixture's config guards from CI
make_repo silences commit.gpgsign, tag.gpgsign and core.hooksPath because a
contributor's global configuration reaches a fresh repository. A runner carries
no such configuration, so none of the three is exercised there: the suite stays
green with any of them deleted, and the failure lands only on the machine of
whoever signs their commits or tags. That is how tag.gpgsign came to be missing
without anything going red.

The new test supplies the hostile configuration itself, so removing a guard now
fails in CI rather than on a contributor's first local run.

Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-09-15 14:37:29 +03:00
IvanHunters 5a777e9cfc test(ci): isolate the commit-trailer fixture from global tag signing
make_repo silences commit.gpgsign in the fixture repository but not
tag.gpgsign, so a contributor who signs tags has that setting reach the
fixture. git tag start then becomes an annotated tag, git refuses it for
want of a message, and since the helper opens all 27 tests the suite
fails before a single one reaches its body, taking make unit-tests with
it.

CI never saw it: the Commit trailers job runs the checker against
explicit SHAs and does not touch this file, and a fresh runner has no
global configuration to leak. The failure only ever lands on the local
run that contributing.md asks for before pushing.

Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-09-15 13:59:45 +03:00
IvanHunters 637098f222 test(kafka): anchor the release selector the hook suites assert on
Both suites matched "strimzi.io/cluster=kafka-test" as a bare substring,
and that is a prefix of the sibling release kafka-test2 and of
kafka-test-topics. Rendering the hook with either selector left both
suites green, so neither pinned the scoping they exist to pin. Anchor on
the leading "-l " and on the trailing delimiter; both variants now red in
both suites, and the six earlier hook mutations still red exactly where
they did.

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

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

Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-09-15 13:48:08 +03:00
IvanHunters 415aa82409 test(kafka): drive the hook suite through the runner CI actually uses
The suite used bats' run helper. make bats-unit-tests does not invoke the bats
binary: it runs hack/cozytest.sh over every hack/*.bats, and that runner
implements @test and little else, so the file died at 'run: command not found'
and took make unit-tests with it. Under bats it was 5/5, which is where the 5/5
in the previous commit came from. Save $? by hand instead, the shape
app-cleanup-hooks_test.bats already uses; verified under both runners.

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

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

Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-09-15 13:47:04 +03:00
IvanHunters 3326e0fb9e test(kafka): run the pre-delete hook against a fake kubectl
The hook's exit code is the whole contract: a non-zero one returns from
Uninstall.Run before any release resource goes, keeping the topic operator
alive; a zero one lets Helm remove it. delete_test.yaml matches manifest text,
so it stayed green when the timeout branch was put back to exit 1, when the
CRD-absent branch was, and when the selector was widened past this release.

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

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

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

Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-09-15 13:47:04 +03:00
Yan Bondarenko 9d2fddd4e6 test(kafka): assert release topics exist before the deletion check
The post-delete absence assertions could pass vacuously if the two
release topics were never created. Asserting both topics first makes
the E2E prove that the pre-delete cleanup actually removed them.

Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
2026-09-15 13:47:04 +03:00
Yan Bondarenko 44ba13b527 fix(kafka): delete release topics before the topic operator is removed
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
2026-09-15 13:47:04 +03:00
Aleksei Sviridkin 070dcb1e30 fix(e2e): wait for the redis replica link instead of sampling it once (#4252)
## What this PR does

The `verify-tenant-ca-projection` step in the redis suite read `INFO
replication` from both pods once and failed the run when a replica
reported `role:slave` without `master_link_status:up`. That value is
still in motion at that point, so one sampling decided the result.

Redis reports the link up only once a replica reaches
`REPL_STATE_CONNECTED`. The operator's readiness script admits a replica
on two other predicates: `master_host` being off `127.0.0.1`, and
`master_sync_in_progress` not being `1`. `master_host` is set the moment
`REPLICAOF` is issued, ahead of the link coming up, and
`master_sync_in_progress` is `1` only during the transfer phase, so
every handshake state between the two is ready and unlinked. A dropped
link re-enters the same window. `readyReplicas: 2` therefore establishes
nothing about the link, and the comment on the step calling the check
deterministic was wrong.

The step now polls the topology to a bounded deadline and fails only if
it does not settle, reporting each pod's observed state and the elapsed
time. Nothing models the link declaratively: the RedisFailover CRD
defines no status at all. So the poll carries the
`TODO(e2e-replace-fixed-timeouts):` marker the e2e conventions require
for a wait with no object behind it.

What the step does now is wait for convergence where it used to sample
once. No claim about flake rate follows from that. A green run says
nothing about a failure that shows up in a minority of runs.

Shell embedded in a chainsaw step has no unit-test surface in this
repository, so this change is exercised by the suite and nothing else.

Two pre-existing diagnostics defects in the same file turned up while
reading it, tracked in #4246 and #4250. Neither is touched here. The
first one matters a little more after this change: the poll runs its
read path about a dozen times per step instead of twice.

Closes #4238

### 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:

I walked the trigger map against the diff rather than the title. The
change edits one file under `hack/e2e-chainsaw/`, moves and renames
nothing, and alters no make target, so none of the listed triggers fire.
The node-prerequisite trigger that reaches `ansible-cozystack` and
`talm` is `hack/e2e-prepare-cluster.bats`, which this does not touch.

### Release note

```release-note
fix(e2e): the redis TLS suite waits for the replica link to come up instead of reading it once, so a link that is still establishing no longer fails the run
```


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

## Summary by CodeRabbit

- **Tests**
- Improved Redis TLS topology validation by allowing replication status
time to stabilize before verification.
- Extended the test operation timeout to reduce failures caused by slow
environment startup or synchronization.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-15 12:09:21 +04:00
Aleksei Sviridkin 999a8f5456 fix(e2e): keep backup preflight failure evidence across attempts (#4242)
## What this PR does

The S3 access preflight that runs before a backup roundtrip prints its
port-forward capture only when the port-forward never comes up. On the
failure that actually happens, a healthy port-forward and an S3
roundtrip that never completes, that capture is written and then thrown
away. The mc log is truncated at the top of every attempt on top of
that, so what reaches the job log is the tail of the last attempt with
nothing in it to say which stage died. One real run left a single line
of evidence, an upload progress line, which reads as an upload that
finished and a download that hung. The upload was what failed.

The timeout path now prints both logs, the mc log accumulates across
attempts, and every attempt records which of PUT, GET, compare or DELETE
it reached. That is enough to tell a broken channel to S3 from S3
refusing the operation, and to see how many attempts ran and how each
one ended.

The port-forward log goes in whole rather than filtered. `kubectl
port-forward` writes one `Handling connection for <port>` line per TCP
connection, and whether those lines are there at all is the answer to
whether mc ever reached the forwarder, so dropping them to cut noise
would delete the signal this change exists to produce.

Diagnosability only. Nothing changes about what the preflight passes or
fails.

Closes #4240

### Screenshots

No UI change.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. Nothing under `hack/` is moved or renamed, no make target changes
behaviour, and the node prerequisites the Ansible collection and talm
mirror are untouched.

- [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(e2e): a backup access preflight that times out now prints the port-forward log and the outcome of every attempt, not just the tail of the last one
```


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

- **Bug Fixes**
- Backup access preflight checks now validate the complete upload,
download, comparison, and cleanup sequence.
- Retry results identify the specific operation that failed and preserve
failure details across attempts, improving troubleshooting.

- **Tests**
  - Added coverage for reporting the first failed upload.
- Added timeout-path coverage for readiness checks, repeated operation
failures, and reporting each affected stage.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-15 12:09:15 +04:00
Aleksei Sviridkin 4878250a1d fix(e2e): wait for the redis replica link instead of sampling it once
The redis-tls topology check read INFO replication from both pods once
and failed the suite when a replica reported role:slave without
master_link_status:up. That value is legitimately in motion there.
Redis reports the link up only in REPL_STATE_CONNECTED, while the
operator's readiness script admits a replica on two different
predicates -- master_host off 127.0.0.1, and sync not in the transfer
phase -- so every handshake state between the two is ready and
unlinked, and a dropped link re-enters that window. Ready replicas
carry no evidence about the link at all, which is more than the step's
own comment allowed for and makes its claim of determinism wrong.

Poll the topology to a 60s deadline instead, and on expiry report each
pod's observed state along with how long it waited. Nothing models the
link declaratively -- the RedisFailover CRD defines no status, so there
is no condition to assert on and the poll carries the marker the e2e
conventions require.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-14 23:37:54 +03:00
Timofei Larkin ae7a7115f5 fix(harbor-rd): carry the release prefix in the resource selectors (#4170)
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->

## What this PR does

`harbor` declares `release.prefix: "harbor-"` and then selects its
Secret, Service and Ingress without it, so all three `resourceNames`
selectors match nothing. This corrects the three names and adds a guard
so the next omission fails a test instead of passing silently.

### Why they never matched

Release names are `<prefix><app name>`, but the lineage webhook
evaluates selectors against `{kind, name, namespace}`, where `name` is
the custom resource's name and carries no prefix. Every definition
therefore spells the prefix out itself, and the others do — `postgres-{{
.name }}-credentials`, `bucket-{{ .name }}-credentials`, and so on.
`harbor` is the only one that does not.

A selector that matches nothing raises no error: the webhook labels the
object `internal.cozystack.io/tenantresource=false` and continues, which
is indistinguishable from a selector that legitimately has nothing to
match. That is why this survived.

### What actually changes at runtime

Close to nothing, and that is the point — the manifest was wrong, not
the behaviour:

- the Secret already reaches tenants, because #2541 worked around this
exact mismatch by stamping `apps.cozystack.io/user-secret: "true"` on it
and matching by label. That workaround is deliberately left in place;
- the Service and Ingress labels flip from `false` to `true`, which
nothing consumes: `spec.services` and `spec.ingresses` are read only by
the webhook that computes the label, and the label's only consumer is
the TenantSecret registry, which handles Secrets. The dashboard reaches
both through the per-release Role, which names them correctly.

So this closes a latent defect before it bites, rather than fixing an
outage.

### The guard

`hack/check-rd-release-prefix.bats` asserts that every `resourceNames`
entry in `packages/system/*-rd/cozyrds/*.yaml` contains `<prefix>{{
.name }}`. `hack/*.bats` is auto-discovered by `BATS_UNIT_FILES`, so it
runs under `make unit-tests` with no registration.

The assertion is "contains", not "starts with", because operator-created
objects legitimately carry leading words of their own: mongodb excludes
`internal-mongodb-{{ .name }}-users`, which is correct and stays green.
Definitions with an empty prefix are skipped, since `hack/update-crd.sh`
generates the definitions for `packages/extra` charts with `PREFIX=""`.

A selector that is correct and still fails the check can opt out on its
own with a trailing `# cozy-lint:ignore rd-release-prefix -- <reason>`
comment. A directive without a reason does not count.

Verified by mutation: the guard fails on the three harbor selectors
before the fix and passes after. Across the rest of the tree `harbor`
was the only violation.

### What this PR does not do

It does not settle whether the prefix belongs in the template context
instead. That would remove the duplication, but it requires migrating
the definitions that spell the prefix out today, or they would double
it. The guard is the cheaper half and closes the class either way.


### Screenshots

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

### Downstream repositories

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

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

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

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

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

Trigger map walked against the diff, which touches
`packages/system/harbor-rd/cozyrds/harbor.yaml` and adds
`hack/check-rd-release-prefix.bats`. The two nearest entries were
checked and neither fires:

- **terraform-provider-cozystack** lists "change `release.prefix`, or
rename an output Secret or Service". Neither happens here:
`release.prefix` is untouched and no object is renamed. The provider
concatenates the prefix itself to derive names, so it was already
computing the correct ones — it is this definition's selectors that were
wrong.
- **ccp** lists "rename `packages/system/<name>-rd/cozyrds/<name>.yaml`"
and "move or rename anything under `hack/`". The file is edited, not
renamed, and the new bats file is an addition; no make target changes
behaviour.

**website** would fire on a change to `ApplicationDefinition` semantics.
Semantics are unchanged — one definition's data is corrected — and
`content/en/docs/next/cozystack-api/application-definitions.md`
documents neither `resourceNames` nor the prefix rule, so there is
nothing there to bring in step.


### Release note

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

```release-note
fix(harbor-rd): the ApplicationDefinition selectors for Harbor's Secret, Service and Ingress now carry the release prefix, so they match the objects the chart creates. No user-visible change — the Secret already reached tenants through a label-based workaround, and nothing consumes the Service and Ingress labels.
```



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

## Summary by CodeRabbit

- **Bug Fixes**
- Updated Harbor resource naming patterns to consistently include the
`harbor-` prefix for credentials, services, and ingress resources.
- Improved alignment between Harbor resource names and their configured
release prefixes.

- **Tests**
- Added validation to detect missing release prefixes in secret,
service, and ingress resource selectors across system application
definitions.
- Validation reports all affected selectors and supports documented
exceptions where necessary.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-15 00:36:45 +04:00
Aleksei Sviridkin 65c183e311 fix(e2e): keep backup preflight failure evidence across attempts
The backup access preflight printed its port-forward capture only when
the port-forward never came up, and truncated its mc log at the top of
every attempt. On the path that fails in practice, a healthy
port-forward and an S3 round trip that never completes, the output was
the tail of the last attempt with nothing to say which stage it died
at, which reads as a successful upload followed by a hung download.

Print the port-forward capture alongside the mc log on the timeout
path, append the mc log across attempts, and record which of PUT, GET,
compare or DELETE each attempt reached.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-14 22:59:37 +03:00
Andrei Kvapil 91112027a5 test(hack): let one selector opt out of the release-prefix guard
A repo-wide name check will eventually meet a selector that is correct
and still fails it, such as an object named on the operator's terms.
The only escape so far was an empty prefix, which exempts a whole
definition, and a guard with no narrower way out is one people learn to
route around. A trailing "cozy-lint:ignore rd-release-prefix -- <reason>"
comment now exempts that one entry, and a directive without a reason
does not count, so a bare suppression is never the easy path.

Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
2026-09-14 18:08:55 +02:00
Aleksei Sviridkin f9ea0da3de feat(backups): Kafka topic-metadata backup via a dedicated strategy driver (#4000)
## What this PR does

Adds a dedicated `strategy.backups.cozystack.io/Kafka` Go driver that
backs up the **topic metadata** of a Cozystack-managed `Kafka`
application and wires it into the platform `cozy-default` flow — the
counterpart to the app-specific drivers (CNPG, MariaDB, Altinity,
MongoDB). It is a typed variant of the generic Job strategy: the
controller renders a one-shot batch Job that talks to the Kafka Admin
API, and adds on top an applicationRef Kind gate, a `Ready` precondition
on the Strimzi `Kafka` cluster, and a per-run object key via
`.BackupName` so distinct runs never overwrite each other.

What it captures: every non-internal topic's partition count,
replication factor and non-default configs (via `kafka-topics
--describe`), stored as one small object per run in the system bucket;
restore recreates them with `kafka-topics --create` + `kafka-configs
--alter`. Deliberately **out of scope**: message payloads (a
volume-snapshot strategy's job), consumer-group offsets (a log position,
only meaningful with a byte-identical data restore), ACLs, quotas, and
`KafkaUser`s. No PITR. Restored topics are created directly through the
Admin API, so on the target they are unmanaged (no `KafkaTopic` CR) —
the accepted tradeoff of an Admin-API-only driver.

Both restore flows are implemented: **in-place** recreates dropped
topics into the source, and **to-copy** applies the source's topics onto
a freshly-bootstrapped empty `Kafka` (the "bootstrap a cluster, then
apply the metadata state" flow).

Contents:
- Driver: `api/backups/strategy/v1alpha1/kafka_types.go`,
`internal/backupcontroller/kafkastrategy_controller.go` (+ unit tests
for its pure helpers), `internal/backupcontroller/kafkatypes/` (Strimzi
`Kafka` readiness gate), dispatch cases in both controllers +
`supportedBackupStrategyKinds()`, scheme registration, generated CRD +
deepcopy.
- Chart: `strategy-kafka-default.yaml` (gated on the bucket name), a
`cozy-default` binding, read RBAC on `kafka.strimzi.io/kafkas`, and the
Strimzi Kafka client image value.
- Docs: a Kafka section in `docs/operations/backup-classes.md`.
- Example + e2e: `examples/backups/kafka-metadata/` and
`hack/e2e-chainsaw/kafka-metadata/` (`kafka-3-metadata-roundtrip`), an
honest round-trip that proves a per-run `retention.ms` sentinel survives
both restores.

The original `packages/apps/kafka` chart is left untouched (it has no
legacy backup mechanism).

Validated: `go build ./...`, `go vet`, `go test
./internal/backupcontroller/...` (pure helpers plus a reconcile
Kind-gate test and the RestoreJob cleanup regression test), `helm
unittest` on the package (which now renders
`strategy-kafka-default.yaml` and asserts its
kind/name/`artifactURITemplate`), the `select-install`/`select-e2e` bats
suites, `helm template` of the strategy CR, `bash -n` of the rendered
script, and `make generate` (clean). The end-to-end round-trip
(`kafka-3-metadata-roundtrip`) is enabled and mapped like the sibling
backup suites, so CI builds this branch's controller and runs it live
against a fresh sandbox; it has not been merged-and-run before, so that
first live run must go green before merge.

### Screenshots

N/A — no UI changes.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. This PR adds a new `strategy.backups.cozystack.io/Kafka` CRD, a Go
driver, chart wiring, docs and examples. It does not add or rename a
`packages/apps`/`packages/extra` package, does not change any
`ApplicationDefinition`, `values.schema.json`, version enum,
`release.prefix`, output Secret/Service, the
`Package`/`Plan`/`RestoreJob` CRDs the provider types by hand, any
`hack/*.mk` anchor or make-target behaviour, node prerequisites,
labels/annotations, or telemetry metrics. A brand-new
`strategy.backups.cozystack.io` CRD travels to the Terraform provider as
opaque JSON (it types only `Plan`/`RestoreJob`/`Package`), so it keeps
working untouched. None of the downstream triggers match.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

### Release note

```release-note
feat(backups): add a Kafka topic-metadata backup strategy bound by cozy-default — backs up topic definitions and configs (partitions, replication factor, non-default configs) via the Kafka Admin API, with in-place and to-copy restore; message data, consumer offsets, ACLs, quotas and KafkaUsers are out of scope
```



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

- **New Features**
- Added Kafka topic-metadata backup and restore support, excluding
message data.
- Supports ad-hoc and scheduled backups, in-place restoration, and
restoration to a separate Kafka cluster.
- Recreates missing topics and restores compatible configurations while
validating partition and replication-factor constraints.
- Waits for Kafka readiness and cleans up backup artifacts after
completion.
- Backup and restore jobs now enforce configurable deadlines to prevent
stalled operations.

- **Documentation**
- Added guidance covering Kafka metadata backups, restoration
limitations, and S3 configuration.

- **Tests**
- Added end-to-end coverage for backup, restoration, validation, and
cleanup scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-14 19:21:43 +04:00
Andrey Kolkov deb4916f94 test(backups): pin the restore-side Kafka --describe literal-topic guard
The restore path wraps the topic in \Q...\E on both the --describe and the
--alter calls so a name with a regex-special character cannot match a
colliding sibling. Only the --alter guard was pinned: the colliding pair in
the fixture gave both topics the same partition count and replication
factor, so stripping \Q on the --describe read the sibling's identical
numbers and nothing downstream diverged.

Give the sibling a distinct, larger partition count. A stripped --describe
\Q now matches it first (glob order places desc.pay-events before
desc.pay.events), reads 9 > the backup's 7, and aborts with "cannot
decrease partitions" so no alter is recorded and the assertion reddens.
Also point the suite's scratch file at a per-test path via METADATA_FILE so
runs cannot read back a prior test's records.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-14 17:55:48 +04:00
Andrei Kvapil 13549b5cfa test(hack): keep only what the code cannot say in the prefix guard header
The header retold both commit messages at several times their length.
What stays is what nothing else records: why the check is "contains"
rather than a prefix match, so tightening it does not break the nine
selectors with their own leading words, and why an empty prefix is
skipped. The latter now names the actual source of those definitions,
since they live under packages/system and are generated from charts in
packages/extra.

Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
2026-09-14 15:29:44 +02:00
Andrei Kvapil 50f4834edc test(hack): run the release-prefix guard under cozytest.sh with yq
make unit-tests executes hack/*.bats through hack/cozytest.sh, which
has no `run` helper, so the guard aborted before inspecting a single
selector. It also depended on python3 and PyYAML, neither of which the
build dependencies or the unit-test job provide. Walk the definitions
with yq instead, as the neighbouring rd checks do, and fail when zero
selectors were inspected so a broken glob or query cannot pass.

Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
2026-09-14 15:29:25 +02:00
Aleksei Sviridkin b3b280ea73 feat(gateway)!: add TLS-passthrough listeners to TenantGateway (#3342)
## What this PR does

Phase 1 of external database exposure via Gateway API TLS-passthrough
(#2815): adds a `tlsPassthroughListeners` field to `TenantGateway` and
renders one layer-4 `mode: Passthrough` listener per entry, on a native
database port with a per-engine subdomain hostname, alongside the
Gateway's terminate listeners. No engine is wired: no TLSRoute, no
certificate or CA changes. The existing layer-7 `tlsPassthroughServices`
field gains a 62-entry count bound, an item length and a DNS-1123 item
pattern, and a repeated entry now fails the reconcile with `Ready=False`
instead of rendering the same listener twice. `spec.apex` gains a
253-character bound and nothing else: the CEL rules concatenate it, so
the install-time cost estimator refuses the CRD without a declared
bound. It gains no format pattern, and that is not a claim that a
mixed-case host works. Such a host reaches the field verbatim from a
namespace label, the renderer then composes listener hostnames Gateway
API refuses, and that tenant has no Gateway either way; what a pattern
would change is only that the write is refused, which fails this
HelmRelease where the same fault costs the one Gateway. An apex no
listener hostname can sit within is reported by the controller instead,
and only when `tlsPassthroughListeners` is in use. The constraints and
what they do to objects that already exist are described further down.
The bound on an entry is what the schema can state on its own. An entry
also becomes a listener name and a hostname built from the apex, so the
controller checks both composed values against the 253 characters
Gateway API allows and names whichever one overflows.

Validation rejects duplicate listener names or ports, ports reserved by
the platform Gateway (80 and 443), out-of-range ports, invalid DNS-1123
names, and hostnames outside the tenant apex (mirroring the Gateway
hostname admission policy so an invalid entry cannot make the whole
Gateway rejected).

The SNI-overlap rule refuses two passthrough listeners that answer the
same name on different ports. That is what
[cilium#42898](https://github.com/cilium/cilium/issues/42898) reports:
only one of the two routes ever works. It does not extend to a terminate
listener and a passthrough listener sharing a name across ports. No
failure is reported for that shape, and Cilium counts a
terminate/passthrough pair as conflicting only when they share a port.

The per-engine subdomain hostname scheme follows the phase-1 gate. The
flat-hostname alternative (pending the Cilium listener-isolation fix
targeting 1.20) is a later scheduling decision that does not change this
API field.

One shape differs from #2815 on purpose. The ticket has `hostname`
optional with a `*.<name>.<apex>` default; here it is required. Deriving
the SNI from the listener name hides the one value an operator has to
get right, and the two drift apart the moment the naming scheme changes.

Found while checking that pairing, unrelated to this field: a stock
`dns01` gateway already renders `tls-api` (passthrough, 443) beside
`https` (terminate, 443, `*.<apex>`), one port with overlapping SNI,
which is the shape Cilium 1.19.6 marks `Conflicted`. That belongs to the
Cilium bump and is recorded on #3343.

Also named rather than fixed: the controller-side rules on this field
stop the whole reconcile when they refuse a spec. `renderGateway`
returns an error, so the Gateway is not updated at all, and the tenant
sees `Ready=False` with `ReconcileError` while new app listeners and
every certificate the controller would issue or amend wait behind it.
Certificates already issued go on renewing, since cert-manager drives
that from the objects already in the cluster and an aborted reconcile
writes nothing to them. Dropping only the conflicting listener and
reporting it on status would be the better shape, but that changes the
error model of the whole renderer rather than one rule, so it is out of
scope here. The current behaviour is at least observable: `Ready=False`
is a signal, not silent drift.

On HTTP-01, a hostname a passthrough listener answers earns no
HTTPS-terminate listener once a TLSRoute claiming that hostname can be
served on the listener, matched by SNI overlap so a wildcard entry
reaches a name beneath it as readily as the name written in the spec,
and the field itself is accepted under certMode http01 alone. That
refusal is stated as the set of modes that can carry a passthrough
listener rather than the set that cannot, because a mode added to the
enum would otherwise be admitted by default: dns01 and existingSecret
serve the whole apex from one wildcard terminate listener the pinned
Cilium cannot keep apart from a passthrough listener beneath it, and
edge ends TLS at the class provider and renders no TLS listener for one
to sit beside. The route half of that condition is load-bearing rather
than incidental. On the pinned v1.19.5, tlsPassthroughFilterChains walks
a listener's routes and skips one with no backends, so a declared
passthrough listener nothing attaches to contributes no filter chain and
matches no ClientHello, while the terminate listener's chain is built
from its own Secret and carries the name alone. Keying the withdrawal on
the declaration would therefore take a name that is being served and
hand it to a listener that forwards nowhere, and that is the ordinary
shape rather than a corner: the chart puts api, vm-exportproxy and
cdi-uploadproxy on every tenant Gateway, while there is one platform
TLSRoute per name cluster-wide and each names one tenant's Gateway as
its parent, so every other tenant renders all three entries with a route
on none. The backend half is that same rule reached by a different road:
the chain is built per route out of the backends that survived
resolution, so a route that attached and forwards nowhere puts no chain
on the name either, a backendRef naming a Service that does not exist
yet because manifests applied out of order, one deleted since, a name
with a typo, or one in another namespace with no ReferenceGrant, and
without that half an ordinary backend mistake takes a live endpoint
offline while Cilium builds nothing to replace it. Resolution asks what
v1.19.5 asks and no more: the reference allowed, the Service present,
and neither its ports nor its endpoints read, because the translator
reads neither. Services and ReferenceGrants are watched alongside
routes, so both halves are deferrals rather than exemptions and the
withdrawal lands on the pass that sees the missing piece arrive. The
withdrawal reaches HTTP-01 alone, because `collectHostnameClaims`
returns nothing under the other two modes and they render no
per-hostname listener to withdraw. A TLSRoute that declares no
`spec.hostnames` claims the hostname of each rendered passthrough
listener its parentRef selects, because that is the name Cilium serves
it on: `ComputeHosts` at v1.19.5 substitutes the listener hostname for a
route that names none, so such a route takes the name over exactly as
one that spells it out does. The tenant-namespace hostname policy
requires a route there to name its hostnames, so this shape reaches a
Gateway from the system namespaces where the platform routes live, or
from an administrator, not from a tenant chart. A claim broader than the
listener's counts the same way from the other side: a TLSRoute claiming
`*.<apex>` is served on `api.<apex>` rather than on the wildcard,
because `ComputeHosts` returns the listener's own hostname for a route
hostname covering it, so a claim covering a reserved name is judged as a
claim on that name while the route keeps one entry under the key its
owner wrote. Both restrictions are tied to the Cilium pin, currently
v1.19.5. That version translates the Gateway into a single Envoy
listener whose filter chains match on SNI without the port, so a
terminate chain and a passthrough chain carrying one name collide
regardless of which Gateway port declared them; upstream states the
consequence above NeedsCrossProtocolSplit in operator/pkg/model/model.go
as of v1.19.6, the release that starts splitting Envoy listeners per
port when NeedsPerPortListeners holds. That is a disjunction rather than
a blanket change, and the passthrough side of every one of its three
clauses is counted from attached routes, so a listener with no TLSRoute
behind it contributes to none of them: the split arrives with the route,
not with the bump.

The same single-listener translation has a consequence that is not a
restriction: the declared port is where the listener is published, not a
bound on reach, because the backend answers its hostname's SNI on every
port the Gateway exposes. Firewalling a tenant address down to 80 and
443 does not put a database out of reach; the hostname is the access
surface, and the field's own doc and the chart README now say so.

Both restrictions are validation-only, and they lift together on the pin
plus a configuration the split actually covers rather than on the bump
alone.

A route whose hostname is withdrawn this way is told so under this
controller's own name: Accepted=False with `NoMatchingListenerHostname`,
and a message naming the passthrough hostname that answers it, which for
a wildcard entry the route cannot derive on its own. It matters most
there. A wildcard entry can withdraw the listener from several published
names beneath it at once, each one a TLSRoute claims and can be served
on, and the certificates are garbage-collected with them, and a rollback
is cheaper than that sounds: cert-manager here runs with
`enableCertificateOwnerRef: false`, so deleting a Certificate leaves its
Secret behind and a re-created Certificate adopts a still-valid one
rather than placing an order. Before this the only signal was Cilium's
own RouteParentStatus entry under a different controllerName. A TLSRoute
whose hostname no passthrough listener answers is told so as well, on
the same condition and for the same reason: before this change that
hostname got a terminate listener Cilium refused with
ResolvedRefs=False, so withdrawing the listener would otherwise have
removed the only object saying the route went nowhere.

Two things stay named rather than fixed. A `tlsPassthroughListeners`
hostname of `*.<apex>` is refused only indirectly: the overlap rule
rejects it while `tlsPassthroughServices` carries its defaults, because
the wildcard covers api.<apex>, so emptying that list is enough to let
it through, and the terminate listener then goes from every published
name a TLSRoute claims under the wildcard. The README states this;
refusing the apex and its wildcard on their own terms is a one-line rule
worth adding when the field is wired up to chart values.

A wildcard hostname on an attached HTTPRoute used to render a listener
name the shipped Gateway CRD refuses, `https-*-<hash>`, which failed the
whole Gateway write while the README said the shape worked. Listener and
Certificate names now spell a leading `*` as `wildcard`, with the hash
still covering the whole hostname, and a test reads both the
listener-name and the hostname patterns off the shipped CRD bundle and
renders wildcards through them; every name that was already admissible
is pinned byte for byte, so nothing is renamed on upgrade. Under http01
an HTTPRoute wildcard claim is refused before the race and the overlap
are read, with `NoMatchingListenerHostname`, because the HTTP-01
challenge issues no wildcard certificate and a terminate listener could
never have carried the name.

A route status write that fails no longer leaves the route carrying its
previous condition: `updateRouteStatuses` and the whole-apex pass
collect every failed write and return them after the loop, so the
reconcile fails, the TenantGateway reports `Ready=False` with
`ReconcileError` and controller-runtime requeues, while the loop still
runs to the end so one route's conflict does not hold the others back.
Conditions this controller wrote under http01 are retracted when the
tenant moves to dns01, existingSecret or edge, for HTTPRoutes and
hostname-less TLSRoutes alike, so a refusal does not outlive the mode
that produced it. And a TLSRoute claiming a wildcard passthrough entry
itself leaves the names beneath the wildcard alone, since Envoy ranks an
exact server name above the wildcard and nothing collides.

One thing this does not cover yet: Accepted is computed from the
hostname race and the withdrawals above, so a route is still told it is
fine when an HTTPRoute holds the name and the TLSRoute claiming it
cannot attach to the terminate listener that answers it. That one
predates this change. Two neighbours of it no longer fall in the gap. A
`parentRef` port that names no listener answering the hostname is
compared alongside the `sectionName`, since Gateway API requires every
key a route pins itself with to match the listener it selects. An
HTTPRoute's `parentRef.port` is judged the same way: the terminate
listener sits on 443, so any other pinned port earns no listener and no
certificate through that parentRef and reads the port-mismatch cause;
port 80, or `sectionName: http`, from a namespace the plain-HTTP
listener admits (the publishing tenant and cozy-cert-manager) is served
plain with no terminate listener and no refusal, from any other
namespace it is refused with `NotAllowedByListeners`, and a
`sectionName` and a port that select different listeners select none and
read `NoMatchingParent`. A plain-served route stays in the hostname race
on the same footing as a terminating one, in every recount as well as
the first count. And a `sectionName` naming a `tls-<name>` listener this
Gateway does not render now reports `Accepted=False` with
`NoMatchingParent`, Gateway API's reason for a parentRef matching no
listener: the controller renders every listener in that namespace, so an
absent name there is a fact rather than a guess, while a `sectionName`
naming the plain-HTTP listener is judged like port 80 below, and any
other name outside the prefix is written Accepted like an attached
route, with the class controller's own status entry answering for the
listener it named.

The namespace case is off that list. Native-port listeners admit the
publishing tenant alone, and a TLSRoute elsewhere on one of their
hostnames is refused with `NotAllowedByListeners`. An HTTPRoute that
pins itself by `sectionName` to a rendered `tls-<name>` passthrough
listener is refused with the same reason: that listener forwards the
stream it matched by SNI and terminates nothing, so it takes TLSRoute
alone by protocol and cannot serve the route whatever answers its
hostname. Its claim also leaves the hostname race before ownership is
ranked, so the TLSRoute that can be served is not told it lost to a
route that could never attach. Cilium does not report this refusal at
the pin. The port-443 passthrough listeners name HTTPRoute in
`allowedRoutes.kinds` to keep the port-443 set uniform (cilium#45559),
the Gateway reports that entry back as
`ResolvedRefs=False/InvalidRouteKinds` on the listener, and Cilium's
route-side kind check reads a listener's kinds against every route on
the Gateway rather than against the routes that named it, so it reports
the route Accepted and the condition this controller writes is the only
refusal the route gets. A hostname whose every claimant is refused this
way earns no HTTPS-terminate listener and no certificate, where before
it earned both. The refusal is decided per route rather than per
hostname, which matters once a claim overlaps more than one reserved
name: a wildcard covering both a port-443 service hostname and a
native-port one is attachable through the first and refused through the
second, so the answer comes from the listener the `parentRef` names, its
own hostname and its own attach set. The row that pinned a `sectionName`
naming no rendered listener as a known gap now pins the verdict instead.
On the port-443 listeners the two namespace sets agree by construction,
since `collectHostnameClaims` and the listeners' allowedRoutes selector
run off the same list.

And the justification for the pin is written out at every site that
depends on it rather than once, so the day the pin moves they all have
to move together and the one that does not is a false statement no diff
can catch. That is not hypothetical here: three claims of exactly that
shape were wrong in this branch and were caught by reading the source
they described rather than by any diff, so the volume of restated
reasoning is itself what produces them. One canonical block with
references to it would carry the same knowledge with one place to
update, and rewriting the narrative under review would be a
documentation change rather than a fix, so it is tracked on #3842
instead.

The gateway chainsaw suite now covers the withdrawal, which it did not
before: a tenant owning its own Gateway renders the shipped passthrough
entries with nothing routed to them, an HTTPRoute on api.<apex> keeps
its terminate listener, its certificate and Accepted=True there, and
attaching a TLSRoute to the same listener, pointed at a Service in the
tenant's own namespace, since a route that forwards nowhere is the case
the first phase already measures, withdraws all three and moves the
route to NoMatchingListenerHostname. A third phase patches in a
native-port entry and reads the per-listener readiness flag, which is
the claim no test against a fake client can settle. The first two phases
are one measurement: withdrawing on demand is what dates the earlier
assertion to settled state rather than to a withdrawal that had not
arrived yet.

The `Require API owner review for sizeable API changes` check is red on
purpose and does not clear with a code change. A new field plus
tightened validation on already-shipped fields counts as a sizeable API
change, so `.github/workflows/api-review-gate.yaml` holds the PR until
an API owner approves it. The gate counts each owner's latest review on
the current head commit, so the approval has to land on the head as
pushed and any later push invalidates it; the job re-runs on review
submission, so no push is needed to flip it. Reading that red as "not
ready" is the mistake this paragraph exists to prevent.

Part of #2815.

### Release note

```release-note
feat(gateway): TenantGateway supports layer-4 TLS-passthrough listeners on native database ports for SNI-based external exposure. On HTTP-01 gateways a hostname a TLS-passthrough listener answers, including any name beneath a wildcard entry, no longer earns an HTTPS-terminate listener or a Gateway-issued certificate once a TLSRoute claiming that hostname can be served on the listener and forwards to a backend that resolves; a hostname claimed only by a TLSRoute stops earning one too. A TLSRoute with no spec.hostnames counts as claiming the hostname of each passthrough listener its parentRef selects, and one whose hostname covers a listener's counts as claiming that listener's name, both being the name Cilium serves it on. A declared passthrough listener with no route on it changes nothing, because it emits no filter chain and the terminate listener is what answers the name, and neither does a route whose backendRefs resolve to nothing, a Service that does not exist, or one in another namespace with no ReferenceGrant, since such a route emits no filter chain either. The controller watches Services and ReferenceGrants, so the withdrawal it defers lands once the backend appears. So the tenant whose Gateway the platform TLSRoutes name, tenant-root unless the cluster's expose-ingress says otherwise, sheds the terminate listeners and certificates for api, vm-exportproxy and cdi-uploadproxy on upgrade (those TLSRoutes ship with the cozystack-api, kubevirt and kubevirt-cdi packages, each rendered only where the Gateway integration is enabled and the service is among the platform's exposed services), and a tenant that renders the same three entries with no route on them keeps what it has. The shed certificates leave their TLS Secrets behind: cert-manager here ships enableCertificateOwnerRef: false, so each Secret stays in the namespace with no owner, nothing renewing it and nothing deleting it. That cuts both ways, as stale Secrets to remove by hand and as a cheap way back, since a re-created Certificate adopts a still-valid Secret instead of placing an order. A route the controller renders no listener it can attach to now carries Accepted=False from this controller instead of Accepted=True. Five shapes report NoMatchingListenerHostname and the message tells them apart: the HTTPRoute whose hostname a passthrough listener answers, the TLSRoute with no passthrough listener behind it, the route whose parentRef names a listener that answers a different hostname, the route whose parentRef names a port no listener answering its hostname is published on, and the HTTPRoute claiming a wildcard hostname under http01, which the HTTP-01 challenge cannot certify. A route pinned by sectionName to a tls-<name> listener the Gateway does not render carries NoMatchingParent instead, which is what Gateway API calls a parentRef matching no listener, and so does an HTTPRoute whose sectionName and parentRef port select different listeners. A TLSRoute outside the publishing tenant's namespace claiming a native-port hostname carries NotAllowedByListeners instead, since those listeners admit the tenant alone. So does an HTTPRoute pinned by sectionName to a rendered tls-<name> passthrough listener, which cannot serve its kind, and an HTTPRoute selecting the plain-HTTP listener from a namespace it does not admit; a refused route also stops counting in the hostname race, so a hostname whose every claimant is refused earns no terminate listener and no certificate. Listener and Certificate names derived from a wildcard hostname are admissible now, names that were already admissible do not change, a failed route-status write fails the reconcile instead of being logged away, and conditions written under http01 are retracted when certMode leaves it. The dns01 and existingSecret cert modes shed nothing, because neither renders per-hostname listeners at all, and tlsPassthroughListeners is accepted under http01 alone: those two serve the whole apex from one wildcard terminate listener, and edge terminates TLS upstream and renders no TLS listener a passthrough entry could sit beside. The withdrawal does not take a working path away: on the pinned Cilium both port-443 filter chains already carry these hostnames and nothing on the objects says which one answers a connection, so a race that was previously undetermined and unobservable now resolves the same way every time. Treat this as a change of serving path rather than the removal of something idle: today each of those three hostnames has a terminate listener and a passthrough listener on port 443, both filter chains carry the SNI, and nothing on the objects says which one answers a connection. Gateway API calls for both listeners to report Conflicted; the pinned v1.19.5 has no Conflicted condition at all, so there is nothing to read, and you cannot tell from the cluster whether api.<apex> is terminated by the Gateway or passed through. Afterwards it is passed through, always. Read the Gateway's listener conditions before upgrading to see what those listeners were serving. Before upgrading, confirm the management apiserver serving certificate carries api.<apex> in its SANs, because once the ACME certificate is withdrawn that is the only certificate left on the name; a cluster that came from the ingress-nginx path already meets that, a cluster served by the Gateway's ACME certificate since it moved does not necessarily and loses the name until the SAN is added. If something depends on the terminate path for one of those names, the package README lists three ways forward and what each costs. Moving the consumer to another hostname under the apex is the only one that gives up nothing.
```


### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against this
diff:
`api/gateway/v1alpha1/{tenantgateway_types.go,zz_generated.deepcopy.go}`,
`internal/controller/tenantgateway/{reconciler,renderers,conflict,mapper,status,backends,reconciler_test,crdcel_test,mapper_test,backends_test}.go`,
`internal/controller/tenantgateway/rbac_test.go`,
`cmd/cozystack-controller/main.go`,
`hack/e2e-chainsaw/gateway/chainsaw-test.yaml`,
`packages/system/cozystack-controller/definitions/gateway.cozystack.io_tenantgateways.yaml`,
`packages/system/cozystack-controller/templates/rbac.yaml`,
`packages/core/platform/values.yaml`, and
`packages/extra/gateway/README.md`. No trigger fires, and the two
repositories worth ruling out explicitly are below rather than waved
off.

`cozystack/website`: no package under `packages/apps/` or
`packages/extra/` is added, renamed or removed.
`packages/extra/gateway/README.md` is edited in place. The docs
generator regenerates a reference page from a package README only for
the apps hardcoded in the website `Makefile`, and `gateway` is in none
of `APPS`, `K8S`, `VMS`, `NETWORKING` or `SERVICES`, so there is no
generated page for this package to drift out of step with. That gap is
the same shape the trigger map already records for
`packages/apps/opensearch` and `packages/apps/bucket`; it predates this
change and is neither created nor widened here. None of the other
website triggers apply: `packages/core/platform/values.yaml` changes
only in a comment, which now says that api, vm-exportproxy and
cdi-uploadproxy render a TLSRoute rather than an HTTPRoute, with no key
added, moved or given a new default, no platform variant or component
moves, no release asset is renamed, `ApplicationDefinition` semantics
are unchanged, there is no Talos bump and no developer-tooling change.

`cozystack/terraform-provider-cozystack`: this adds an API field, so the
field-level triggers are the ones to check, and they are keyed to app
charts rather than to this CRD. No app is added under `packages/apps/`;
no `values.schema.json` gains, loses or renames a field; no version
enum, `values.yaml` default, `kind`, `plural`, `release.prefix` or
output Secret/Service name changes. `TenantGateway` is not among the
CRDs the provider types by hand. Those are `Package` under
`cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`, and
the provider carries no reference to `gateway.cozystack.io` at all. It
therefore falls under the map's last bullet: the spec travels as opaque
JSON, so the provider keeps working and simply offers no typed access to
`tlsPassthroughListeners`, which is the position it already holds for
every other field it does not model, and not a break. The provider's
new-top-level-field test guard is scoped to `api/apps/v1alpha1`, which
this diff does not touch.

`cozystack/ansible-cozystack`: nothing in reach.
`packages/core/installer/values.yaml` and the `cozy-system`/PSA contract
are untouched, no platform variant is added or renamed, none of the
`packages/core/platform/values.yaml` keys the role sets are involved
(the file changes in a comment alone), no object the role waits on is
renamed, and `hack/e2e-prepare-cluster.bats` node prerequisites are
unchanged.

The schema constraints this diff adds to already-shipped fields all
reach objects that already exist, and every one of them is a kind the
apiserver ratchets. `spec.apex` gains a 253-character limit (no pattern:
a mixed-case apex is admitted and refused by the controller only when
passthrough listeners are declared), and `spec.tlsPassthroughServices`
gains a DNS-1123 item pattern and a length bound. A stored value that
violates one still passes any write that leaves it unchanged, so the
rule bites only once the value itself changes. A repeated
`tlsPassthroughServices` entry is reported by the controller as
`Ready=False` rather than refused at admission, and the reason is not
the one this section used to give. Ratcheting does reach a list-type
error: `customresource.ValidateUpdate` validates an incoming object's
list-type semantics only when the stored object passes them, so a stored
duplicate is ratcheted through rather than blocking every write. What
makes the marker the wrong tool is the grain of that ratchet, which is
the whole object rather than the field, so one stored duplicate would
switch list-type checking off for every list on the `TenantGateway` at
once and say nothing about it. Documented in
`packages/extra/gateway/README.md`, and pinned by
`TestTightenedConstraintsOnExistingObjects` together with
`TestListTypeSetWouldNotRefuseAStoredDuplicate`, which compiles a copy
of the shipped schema carrying the marker to show what the apiserver
actually does with one. Shipped defaults conform and the chart already
fails on an empty host, so this is a no-op in practice.

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

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

* **New Features**
* Added layer-4 TLS passthrough listeners (`tlsPassthroughListeners`)
with native port + SNI hostname, rendered as passthrough listeners and
limited to TLSRoute attachment.
* **Bug Fixes**
* Added stricter admission-time and controller validation (reserved
ports 80/443, distinct listener ports, tenant-apex hostname rules, and
collision prevention with passthrough services).
* Improved listener rendering safety: prevents exceeding the Gateway
listener cap and ensures AllowedRoutes are isolated per listener.
* **Documentation**
* Updated gateway docs with listener-cap considerations and layer-4
passthrough usage details.
* **Tests**
* Added CRD/CEL and reconciliation coverage for valid/invalid
configurations and rendering behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-11 13:02:57 +04:00
Aleksei Sviridkin cecb4b11d3 feat(opensearch)!: issue HTTP and Dashboards certificates from cert-manager (#2682)
## What this PR does

The OpenSearch HTTP API (9200) and Dashboards (5601) already served TLS,
from the operator's own private CA and with a SAN built from
`cluster.local` while the credentials Secret advertised the release on
the platform domain, so nothing a client was handed could verify. Both
certificates now come from a per-release cert-manager CA in the tenant
namespace, and the CA reaches tenants as a key-free trust anchor.

- `tls.issuer` selects who issues the HTTP server certificate,
`cert-manager` or `operator`, and is named for the issuer because TLS is
served under both. Unset resolves from `external` (cert-manager for
externally published services, the operator for cluster-internal); an
explicit value always wins.
- Renders a self-contained cert-manager chain in the tenant namespace: a
self-signed Issuer, a CA Certificate, a CA Issuer, and two leaf
Certificates, one for the HTTP API and one for Dashboards, each with the
SANs of its own Service and the external hostname when `external: true`.
Leaf keys are written as PKCS8, since the JVM loader rejects the
PKCS1/SEC1 encoding cert-manager emits by default, and the CA subject is
pinned so the operator's signature check accepts the chain.
- The CA Secret is `<release>.http-ca`. The dot separates the
chart-chosen suffix from release names, so no other release can produce
that name; the two leaf Secrets stay hyphenated because none of the
chart's suffixes is a tail of another. The operator's own transport CA
remains at `<release>-ca` and is not touched.
- Transport mTLS (9300) stays operator-managed (`transport.generate:
true`, `perNode: true`). The security plugin requires it and the chart
cannot turn it off.
- `hack/e2e-chainsaw/opensearch/` runs the move on a cluster: it
installs the app on the operator's HTTP listener, removes `tls.issuer`
so it resolves from `external`, and asserts the cert-manager chain, the
CR dropping `http.generate`, the admin certificate reissued under the
chart CA, the StatefulSet rolling onto the three subPath mounts, the
tenant anchor, and the certificate the endpoint actually presents to a
client holding that anchor.

### Sysctl carrier and single-node rolls

The E2E suite for this PR was red because the chart set
`spec.general.setVMMaxMapCount: true`, which makes the operator prepend
a privileged `init-sysctl` container to every OpenSearch pod. Tenant
namespaces run at the cluster's default PodSecurity level, baseline on
Talos, so admission refused the container and the StatefulSet never
produced a pod (#4131). The carrier for the sysctl is now on main: #4152
added a DaemonSet to the `opensearch-operator` package that raises
`vm.max_map_count` to a floor of 262144 on every node, from a namespace
the platform labels privileged. This branch is rebased onto that main
and sets the flag to an explicit `false`, with a template comment saying
why and a unit test pinning it, so a release no longer asks the operator
for a container admission refuses. The value stays with the DaemonSet
until the node contract carries it (#4144); the DaemonSet's own template
comment, which said the chart still asks, now says it does not.

Three more things came out of the same pass. The chart asked the
operator to drain data nodes before every rolling restart, and with a
single data node the drain never finishes: at v2.8.0
`PreparePodForDelete` excludes the node from allocation and then waits
for it to hold no shards, which the only data node can never satisfy, so
a one-replica release could not roll for a version bump or for the
certificate move this PR relies on. The chart now drains only when a
second data node exists, pinned both ways in the unit tests. The chart
maps an empty `storageClass` to `replicated` at one replica, and the PR
lane runs LINSTOR without DRBD and creates only `local`, so the E2E
fixture takes its class from the `storageClass` value the harness passes
with `--set-string` from `COZY_E2E_STORAGE_CLASS`, the way the
vminstance fixtures do; on the nightly QEMU lane that still resolves to
`replicated`. And the external Dashboards Service selected labels no pod
carries. `pkg/builders/dashboards.go` at v2.8.0 stamps
`opensearch.cluster.dashboards: <cluster name>` on the Deployment, its
selector and the pod template, and nothing else; the chart asked for
`opster.io/opensearch-cluster` together with
`app.kubernetes.io/component: dashboards`, so the LoadBalancer came up
with no endpoints and no error to show for it. It selects the label the
operator sets now.

### securityadmin cross-trust, still blocked on the DNS base

Chart-managed HTTP TLS previously broke the operator's securityadmin
reconciliation: the admin certificate was signed by the transport CA
while the HTTP listener trusted the cert-manager CA, so the two never
verified against each other and users and roles silently never applied.
This chart puts one CA under both sides, handing the HTTP CA to the
operator through `spec.security.tls.http.caSecret` so that it signs the
admin certificate with the CA the listener trusts. That closes the CA
mismatch. The other half, the job resolving a name the operator built
from `cluster.local` on a cluster that does not use it, was closed
separately and is on main, so the security configuration applies once
this lands. No operator bump is required; the API exists in the pinned
version.

That path is version-gated upstream at OpenSearch 2.0.0: below it the
operator falls back to the transport CA and the mismatch returns with no
error anywhere. The chart therefore refuses chart-managed HTTP TLS on
`version: v1` with an explicit render-time failure that names the
mechanism and offers the alternatives, rather than letting it fail
silently at runtime.

### Trust anchor for tenants

The HTTP server's private key is not exposed to tenants. The chart
renders a `TenantProjection` sentinel that names `<release>.http-ca` as
its source; the CA-extraction controller publishes a `ca.crt`-only copy
as `<release>.tenant-ca`, and the ResourceDefinition selects it with the
engine-agnostic `internal.cozystack.io/tenant-ca` label. The sentinel is
gated on the same condition as the chain, so a plaintext release renders
no sentinel instead of one stuck at `SourceNotFound`. Tenants get what
they need to verify the server and nothing more.

## Known limitations

These are stated rather than left to be discovered:

1. **securityadmin needed both trust and reachability, and both are
closed now.** This branch put one CA under the admin certificate and the
HTTP listener. The other half was the operator resolving
`<svc>.<ns>.svc.<DNS_BASE>` with `DNS_BASE` left at the vendored
`cluster.local`, so the job waited on a name that never resolved
whichever CA had signed; the platform now renders that value from
`networking.clusterDomain` and it is on main, which is what unblocks
this branch.
2. **The e2e suite reaches the move, not the chart bump that triggers
it.** The suite above arrives at the chart-managed chain by removing
`tls.issuer` from a running release, which leaves exactly the values an
upgraded release arrives with. The one step it cannot run is the one
where the previous chart version wrote the CR, because there is no
second chart version to install from in this tree. Everything downstream
of that write is exercised on a cluster.
3. **A reissue does not reach a running pod.** Neither service reloads
its certificate, and cert-manager reissues on any change to `external`,
the tenant host or the cluster domain, so an ordinary edit leaves the
pods serving material that no longer matches their own names with no
error, event or condition to show for it. Completing such a change is
manual. So is removing the CA private key cert-manager leaves behind
when TLS is turned off again, since the platform runs with
`enableCertificateOwnerRef: false` and deleting a `Certificate` does not
delete its `Secret`. `packages/apps/opensearch/README.md` carries both,
with the commands.

### Downstream repositories

`packages/apps/opensearch/values.schema.json` gains the `tls` object, so
`cozystack/terraform-provider-cozystack` needs the field modelled for
`cozystack_opensearch`; tracked in
cozystack/terraform-provider-cozystack#41.

### Upgrade impact

An existing `external: true` release with `tls.issuer` unset serves
operator-issued HTTP TLS today and moves to chart-managed TLS on its
first reconcile after this lands: the cluster rolls once and is
re-anchored on a new per-release CA, so any client pinned to the
operator's CA has to be repointed at the key-free `<release>.tenant-ca`.
Setting `tls.issuer: operator` before the upgrade keeps that half as it
is. Releases that are not external are untouched. Every existing release
rolls once regardless of `tls.issuer` and `external`, because the pod
template loses the operator's `init-sysctl` container; on a
single-replica release that roll is the first one the operator can
complete, since draining the only data node used to block every rolling
restart.

The published external-dns name changes on the same upgrade whatever
`tls.issuer` says, because the Services are gated on `external` alone:
they move from `<release>.<namespace>.<cluster-domain>`, an in-cluster
name that never resolved outside the cluster, to
`<release>.<tenant-host>` and `<release>-dashboards.<tenant-host>`.
external-dns is configured upsert-only here, so the old record is not
withdrawn and keeps pointing at the LoadBalancer until it is removed
from the zone by hand. That cleanup is per zone rather than per release
and nothing enumerates the records, so the README shows how to derive
the list from the cluster.

### Release note

```release-note
feat(opensearch)!: the OpenSearch HTTP API and Dashboards already served TLS, from the operator's own private CA and with a SAN built from cluster.local while the credentials Secret advertised the release on the platform domain; both certificates now come from a per-release cert-manager CA whose anchor the platform publishes into the tenant as <release>.tenant-ca. Any client that worked against the old endpoint was skipping verification, because no hostname it was handed could have matched that SAN. Existing external releases move from the operator's CA to the chart's on their first reconcile, and their published external-dns name moves from the in-cluster domain to <release>.<tenant-host> with the old record left in place; set tls.issuer: operator beforehand to opt out of the CA move. With dashboards.enabled the application name is limited to 41 characters, and to 32 when external is on as well; past either bound the chart fails the render and withholds every object in the release. A release already above a bound is not broken by the guard, it is told why it has not been upgrading: the Dashboards Service the operator builds from that name exceeds the 63-character limit for a DNS label, the operator returns that failure on every pass, and the reconcilers queued behind Dashboards, which are upgrade and restart and snapshot repository, have not run since the release was created. Set dashboards.enabled to false to drop the bound. The external Dashboards Service also selected labels the operator does not put on Dashboards pods, so it stood up with no endpoints; it selects opensearch.cluster.dashboards now. The drain of data nodes is now requested only with the data role on and more than two replicas, since at two the operator gives up on a full drain itself. The chart stops asking the operator for its privileged init-sysctl container, since the opensearch-operator package now sets vm.max_map_count from a DaemonSet, so every existing release rolls once on upgrade; single-replica releases no longer ask the operator to drain their only data node, which used to block every rolling restart
```
2026-09-11 13:02:47 +04:00
scooby87 080c649da0 fix(hack): avoid a SIGPIPE false-negative in the resourcesPreset check (#4204)
## Summary

`hack/check-rd-presets.sh` intermittently fails the "Unit & controller
tests" CI job with a false `resourcesPreset enum missing: <preset>` for
a preset that is actually present — most visibly on `http-cache-rd`, and
on PRs that have nothing to do with that package.

## Root cause

The check tests each expected preset with `printf '%s\n' "$enums" | grep
-Fqx -- "$want"`. When grep finds the preset it exits immediately and
closes the pipe, so `printf` is killed by SIGPIPE (exit 141) while it
still has data to write. The script runs under `set -o pipefail`, so
that 141 becomes the pipeline's status and `if ! <pipeline>` records a
present preset as missing. It is a race with the pipe buffer, so it
fires intermittently and mostly on the larger enums (`http-cache-rd`'s
schema carries two 47-entry enums). The CI log shows the tell:
`check-rd-presets.sh: line 39: printf: write error: Broken pipe`
immediately followed by `FAIL: ... resourcesPreset enum missing:
s1.micro`.

## Fix

Feed grep from a here-string instead of a pipe — no second process, no
SIGPIPE, no pipefail interaction. Deterministic reproduction (target at
the front of a 100k-line list, under `set -euo pipefail`): the `printf |
grep -Fqx` form reports the target as missing; the here-string form
finds it. After the change `bash hack/check-rd-presets.sh` passes green.

### Release note
```release-note
NONE
```


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

## Summary by CodeRabbit

* **Bug Fixes**
* Improved preset validation to prevent false errors when configured
presets are present.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-11 11:00:44 +03:00
Aleksei Sviridkin 19adffb515 test(opensearch): exercise the TLS rollout on a cluster
No suite created an OpenSearchCluster anywhere in the tree, so the whole
class of failure where the chart renders correctly and the cluster then
refuses the workload had no coverage at all. Rendering proves the
manifests; only a cluster proves that securityadmin authenticates
against the CA the chart handed the operator and that the security
configuration actually applies.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-11 01:23:22 +03:00
Aleksei Sviridkin f3c28e91c4 test(opensearch): pin the trust anchor, the name guard and the upgrade
Most of what this chart decides about TLS is invisible in a rendered
manifest until a cluster disagrees with it. The upgrade case is the
sharpest: a release that already runs operator-managed HTTP TLS must
keep working when the chart takes the CA over, and nothing about the
rendered output says whether it will.

The negative assertions carry as much as the positive ones. The trust
anchor must stay out of the resource map, since a name grant there
would outlive the platform's verdict on it, and a null tls value must
leave the chart rendering rather than aborting the whole release. Both
are properties that fail silently and only a test states them.

The bats guard covers what helm-unittest cannot see: which Secrets the
opensearch-rd definition exposes, so a key-bearing one cannot be added
to that surface without the guard going red.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-11 01:23:22 +03:00
Arsolitt 85fa60a33f feat(opensearch)!: issue the HTTP API certificate from cert-manager
The HTTP listener and Dashboards ran on certificates the operator minted
itself, so a client outside the cluster had no anchor it could obtain
through the platform and no way to verify the endpoint it was handed.

Issue the HTTP chain from cert-manager instead, and give the operator
the same CA through spec.security.tls.http.caSecret so it signs the
securityadmin admin certificate with it. Without that the admin
certificate would keep the operator's own CA while the listener trusted
cert-manager's, and securityadmin could no longer authenticate to apply
users, roles and audit policy.

tls.issuer names the authority rather than a switch, because the
listener serves TLS under either value and only the signer changes. It
is tri-state: an explicit value is obeyed, an absent one follows
external, because a release nobody publishes has no third party to
prove anything to. The cert-manager issuer needs OpenSearch 2.0.0 or
newer, since the operator honours the HTTP CA only from that version
and silently signs with its transport CA below it. An explicit choice
under that floor fails the render and one resolved from external falls
back to the operator, so asking for a verifiable certificate never
quietly yields less than was asked, and turning on external access
never breaks an upgrade.

BREAKING CHANGE: an externally published release on OpenSearch 2.0.0 or
newer serves the HTTP API from a cert-manager CA rather than the
operator's own. A client pinned to the operator CA must move to the
trust anchor the platform projects into the tenant.

BREAKING CHANGE: the external-dns name of an externally published
release moves from <release>.<namespace>.<cluster-domain> to
<release>.<tenant-host>, and the Dashboards name with it. This half is
gated on external alone and does not depend on the issuer. external-dns
runs upsert-only here, so the old record stays in the zone until it is
removed by hand.

BREAKING CHANGE: with dashboards enabled the application name is limited
to 41 characters, and to 32 when external access is also on. A release
above either limit now fails to render rather than converging. Such a
release was already stalled: the Dashboards Service built from its name
exceeds the 63-character DNS label limit, and the operator returns that
failure ahead of its upgrade, restart and snapshot-repository
reconcilers, so it has not upgraded or rolled since it was created. Set
dashboards.enabled to false to converge again, or recreate the
application under a shorter name.

Assisted-by: LLM
Signed-off-by: Arsolitt <arsolitt@gmail.com>
[lexfrei: reworded subject and body for the issuer field rename]
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-11 01:23:13 +03:00
Andrey Kolkov c3d1015df0 test(backups): pin the non-numeric recorded replication factor guard
The Kafka restore path rejects a recorded replication factor that is not
numeric, but only the partition-count guard had its own test. Dropping
both guards reddened that test, so the pair looked covered, while dropping
the replication-factor guard alone left the suite green. Add a case that
drives the create path (topic absent) with a valid recorded partition
count and a non-numeric replication factor, so the guard is pinned on its
own and a future edit that removes it fails loudly.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:28:22 +04:00
Andrey Kolkov c5a8a39bcd docs(backups): name the demo BackupClass in the Kafka flow headers
The run-all.sh flow header and the chainsaw harness header both said the
ad-hoc BackupJob runs against the platform cozy-default BackupClass. It
routes to this demo's kafka-metadata BackupClass instead, which maps Kafka
to the kafka-strategy-default strategy that run-all.sh derives from the
shipped cozy-default-kafka at run time. The numbered manifests already
described this; only the two summary headers overstated the one piece of
wiring the demo exists to exercise. Reword them to match.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:23:53 +04:00
Andrey Kolkov 2cb2b459fa fix(backups): reject a non-numeric recorded partition count on Kafka restore
The restore path validated only the shape parsed from the live cluster,
not the partition count and replication factor read from the stored
object. A non-numeric recorded partition count made `[ "${lp}" -lt
"${parts}" ]` and its `elif` exit 2; an `if` condition is exempt from
errexit, so both read as false, the topic was skipped, and the loop still
printed its per-topic success line - so the RestoreJob reached Succeeded
without ever touching the topic. A well-formed but wrong-content object,
which curl -f cannot catch, must fail loudly like the unrecognised-record
guard.

Validate the recorded parts and rf are numeric before the comparisons and
--create, with a bats case that reddens when the guard is dropped.

Also correct a stale to-copy example comment: the strategy reads the exact
object the source backup recorded in status.artifact.uri (injected as
ARTIFACT_URI), not a key derived from .Backup.ApplicationRef.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:23:53 +04:00
Andrey Kolkov cca3b08733 test(backups): pin the Kafka restore path and fail fast on a missing artifact
The restore half of the driver - the half
that mutates a live Kafka cluster - had no test reaching its happy path,
so three reversions of it left the suite green.

- Add restore reconcile tests mirroring the Rabbitmq driver this was
  copied from: ToCopy (pins the targetApplicationRef.Name merge - a
  to-copy restore must resolve the TARGET, not write the source's topics
  back into the source), NoRecordedArtifactURI, TargetClusterNotReady and
  TargetNotFound. Verified non-vacuous: neutralising the name merge reddens
  ToCopy, and reverting the fail-fast ordering below reddens
  NoRecordedArtifactURI.

- Fail fast on a Backup with no status.artifact.uri: move the check ahead
  of the readiness gate so it terminates immediately instead of waiting out
  the whole deadline and then reporting a misleading "timed out waiting for
  Kafka cluster to become Ready" (reconcileRabbitmqRestore checks it first;
  same order here).

- Add bats cases for the fail-closed guards that no test reddened: MODE
  validation, the s3:// URI prefix, cleanup treating 404 as success, an
  unparsable backup/live describe, a kafka-configs describe error, and an
  unrecognised record kind (plus a CFG_FAIL knob in the configs stub). The
  describe-parse guards abort via `set -euo pipefail` at the
  `X=$(describe_field ...)` assignment, so these pin the fail-closed
  behaviour; the explicit guards remain as belt-and-braces.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:23:53 +04:00
Andrey Kolkov 4925a84c26 fix(backups): pin the restore-side guards and bound a running Kafka Job
- delete_topic waited with `grep -qx` (BRE), so deleting `audit.events`
  matched the live `audit-events` and spun to its timeout — the e2e
  failed at step 25 on every run that reached it. Use `grep -qxF`.
- The two -k/--connect-to guards were written as `! grep`, which cannot
  fail under hack/cozytest.sh (it wraps each @test in `set -e` + a
  trailing `return 0`, and bash exempts a `!`-negated command from
  errexit). Rewrite them as positive `if grep ...; then return 1; fi`;
  verified the -k reinstatement now reddens the suite under cozytest.
- Cover the restore-side literal-topic guard that was unpinned at every
  level: a bats case runs the existing-topic branch with a colliding
  sibling and asserts the `--alter` argv carries `\Q…\E`, and run-all.sh
  now leaves `audit.events` live so the e2e exercises that branch instead
  of only `--create`. Add bats cases for the partition-shrink refusal, the
  `-f` upload guard, and `--exclude-internal`.
- Bound a running Job: reconcileKafka / reconcileKafkaRestore now fail the
  BackupJob/RestoreJob once kafkaDefaultBackupDeadline elapses in the
  Job-running branch (buildJobStrategyBatchJob sets no
  activeDeadlineSeconds), so an unschedulable pod no longer pins it in
  Running forever. Pinned by TestReconcileKafka_FailsWhenJobNeverCompletes.
- Fail closed on an unknown record kind and an unknown MODE, and validate
  the live partition/RF parsed on restore (an empty value silently skipped
  reconciliation). Correct the stale "image pinned in the strategy" comment
  in the example helpers and the chainsaw test (the controller now resolves
  the target broker's image as .ClientImage).

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:23:52 +04:00
Andrey Kolkov ec26f1171d fix(backups): make Kafka metadata e2e reach an in-cluster S3 endpoint
The e2e drove run-all.sh against the shipped cozy-default-kafka strategy,
whose endpoint is the platform's EXTERNAL S3 ingress. In CI that host
(s3.example.org) is an unroutable placeholder with no issued cert, so the
backup Pod died on `curl: (6) Could not resolve host` and the BackupJob
went Failed — the chainsaw round-trip failed there, not in the driver.

Mirror the rabbitmq e2e model, but without duplicating the 150-line
driver script: run-all.sh now derives a demo strategy from the shipped
cozy-default-kafka at run time (jq-patch: override S3_ENDPOINT, and when
an endpoint override is given, mount the copied self-signed seaweedfs CA
and point CURL_CA_BUNDLE at it), applies a demo BackupClass, and routes
the BackupJob/Plan to it. The chainsaw run sets
S3_ENDPOINT=https://seaweedfs-s3.tenant-root.svc:8333 (the .svc FQDN the
seaweedfs cert SAN covers). No -k; TLS is verified against the CA. With
no override (a real cluster whose external endpoint resolves and uses a
public CA) the shipped endpoint is reused and no CA is mounted, since
CURL_CA_BUNDLE replaces the system bundle rather than extending it.

The shipped cozy-default-kafka strategy is untouched. jq derivation
verified on a sample in both CA modes; example scripts pass bash -n.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:22:34 +04:00
Andrey Kolkov 06e16d5f47 fix(backups): make the Kafka bats run under cozytest.sh (CI runner)
The suite was written for upstream bats and passed there, but CI runs
hack/cozytest.sh, which knows only @test + bash: it never calls setup()
and provides no $BATS_TEST_TMPDIR / run / $status / $output. So setup()
never ran and the first test aborted with "STATE: parameter not set"
under set -u. Move the stub setup into an init_stubs helper each test
calls, drop the bats builtins for plain exit-code and $(...) assertions,
and mktemp per test instead of $BATS_TEST_TMPDIR. Verified with
hack/cozytest.sh (11/11), the runner make bats-unit-tests uses.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:22:34 +04:00
Andrey Kolkov aaadc70a08 docs(backups): drop a decaying line-count from the Kafka bats header
The comment cited "the 160-line driver script"; a count about the file
decays on the next edit and nothing pins it. State the invariant instead.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:22:33 +04:00
Andrey Kolkov ebb3118694 fix(backups): key Kafka --connect-to off the request host in both S3 modes
The port workaround built --connect-to's HOST1 from ${host}, but the
virtual-hosted branch (forcePathStyle:false) sends to ${bucket}.${host}.
curl matches HOST1 against the request host, so on a virtual-hosted AND
non-default-port endpoint the redirect never fired and the request hit the
wrong port. Derive HOST1 from the host the URL actually uses; the real
connection target stays ${host}:${port}. Path-style is unaffected. Add a
bats case for the virtual-hosted ported mode, which the suite did not cover.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:22:32 +04:00
Andrey Kolkov 14c6af122e fix(backups): restore Kafka S3 port workaround and hold the script with a test
- Regression fix: an earlier change dropped the curl --connect-to port workaround along
  with -k, but curl 7.76.1 (the image's) 403s on ANY ported endpoint,
  which is the shape backupStorage.endpoint ships (:8333) and what
  provisionBucket:false and s3.ingress.enabled:false produce. Restore
  --connect-to (sign a portless URL, redirect to the real port), keeping
  -k off so TLS is still verified. IPv6-safe host/port split, path prefix
  preserved. The port-less ingress passed; the ported case is the one that regressed.
- Correct the S3 envelope claim in values.yaml and backup-classes.md: the
  403 is decided by the port, not a body-hash the backend recomputes, and
  --connect-to handles it on SeaweedFS and MinIO alike.
- Hold the script fixes at the layer the contract lives at. Externalise
  the driver to files/kafka-backup.sh (embedded verbatim via .Files.Get,
  render byte-identical) and add hack/kafka-backup-script.bats, which
  EXECUTES it against stubbed kafka CLIs + curl and asserts behaviour: \Q
  literal match on the colliding pair, --connect-to with no -k on a ported
  endpoint, fail-closed --list, comma-value bracket grouping, RF-divergence
  abort, and the cleanup delete's HTTP handling. Not a template grep.
- Pin the Backup-delete dispatch and the readiness deadline in Go:
  TestCleanupOnDelete_Kafka_RoutesToKafkaCleanup and
  TestReconcileKafka_FailsAfterReadinessDeadline.
- Docs: config restore is additive (no --delete-config); export time
  scales with topic count and the Job has no wall clock (as its siblings).

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:22:32 +04:00
Andrey Kolkov 0270650237 fix(backups): harden Kafka metadata S3 handling and own the artifact
- Drop `-k`: mirror the RabbitMQ S3 layer (ARTIFACT_URI as the single
  source of truth, forcePathStyle honoured, whole base URL kept) so
  curl --aws-sigv4 verifies TLS against the ACME endpoint the helper
  returns. The derived endpoint is default-port, so the curl 7.76.1
  --connect-to port workaround is gone too; this also fixes the IPv6
  and path-prefix mangling in the old host-splitting.
- Own the artifact: a cleanup mode in the script plus cleanupKafkaBackup
  wired into BackupReconciler.cleanupOnDelete delete a deleted Backup's
  object (best-effort, waited on) instead of orphaning it, matching the
  RabbitMQ precedent. Kafka has the identical own-your-object property.
- Restore probes existence with --list (fail closed), so an unreachable
  broker aborts instead of being misread as "topic absent" and created.
- Hold the fixes with tests at the right level: a colliding topic pair
  (audit.events / audit-events, distinct partition counts) in the e2e so
  a revert of the \Q...\E literal-match breaks the round-trip, and Go
  tests pinning the scope stamp and the cleanup Job.
- Correct the docs/comments: endpoint-shape row, the "cached on nodes"
  claim, and the narrower curl 7.76.1 S3 envelope (empty-body payload
  hash) that makes a small modern-curl client image the right follow-up.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:22:06 +04:00
Andrey Kolkov 405eb25095 fix(backups): harden Kafka metadata backup
- Fail closed on topic listing: set -o pipefail, capture kafka-topics
  --list --exclude-internal exit separately, and abort on failure so an
  unreachable broker can no longer produce an empty backup reported as
  Succeeded. An empty list after a successful call stays legitimate.
- Check each kafka-topics --describe exit and validate the parsed
  partition count / replication factor before writing a record, so a
  timed-out describe cannot write a corrupt orders\t\t\t line.
- Store topic configs one per line with key and value in separate
  tab-delimited fields, sourced from kafka-configs --describe (one config
  per line), so a value containing a comma (cleanup.policy=compact,delete)
  round-trips; on restore each config is applied with the value bracketed
  only when it contains a comma.
- Detect topic divergence on restore instead of a silent --if-not-exists
  no-op: grow partitions with --alter when the backup asks for more, and
  fail loudly (naming the topic and both values) when it asks for fewer
  partitions or a different replication factor. A restore that cannot
  reach the recorded state no longer reports Succeeded.
- Add KafkaStrategyKind to the RestoreJob cleanup-dispatch no-op group so
  deleting a Kafka RestoreJob does not run a pointless Velero cleanup; add
  a regression test alongside the siblings, plus a reconcile Kind-gate
  test and a rendering test that actually renders the strategy template.
- Wire the kafka-metadata Chainsaw suite into both selectors
  (select-install suite_to_source, select-e2e src_to_suites) so editing
  the kafka app or the suite selects it and the bats unit tests pass.
- Pin the client image to the operator's default broker version
  (kafka 3.9.1) with a digest, and align the example default; fix the
  values comment. Point the RabbitMQ comment references (which shipped in
  the CRD description) at the Job strategy actually mirrored, and trim the
  in-pod comments that rendered into the manifest.
- Document the Strimzi Topic Operator caveat: for a topic owned by a
  KafkaTopic CR the operator reconciles its config from the CR, so an
  in-place config restore is durable only for out-of-band topics.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:21:24 +04:00
Andrey Kolkov a410a3f48b feat(backups): Kafka topic-metadata backup via a dedicated strategy driver
Add a strategy.backups.cozystack.io/Kafka driver that backs up the topic
metadata of a Cozystack-managed Kafka application — partition counts,
replication factors and non-default topic configs — through the Kafka
Admin API, and restores it with kafka-topics --create + kafka-configs
--alter. Modelled on the Job/Rabbitmq strategies: the driver renders a
one-shot batch Job, gates on the Strimzi Kafka cluster reporting Ready,
and keys each backup by .BackupName so runs do not overwrite each other.

Scope is deliberately narrow: message payloads, consumer-group offsets,
ACLs, quotas and KafkaUsers are out of scope, and there is no PITR.
Restored topics are created directly (unmanaged on the target) — the
accepted tradeoff of an Admin-API-only driver. Both restore flows are
implemented: in-place recreates dropped topics into the source, to-copy
applies the source's topics onto a freshly-bootstrapped empty cluster.

Wire it into cozy-default (strategy-kafka-default.yaml gated on the
bucket name, a BackupClass binding, read RBAC on kafka.strimzi.io/kafkas
for the readiness gate, and the Strimzi Kafka client image), document it
in backup-classes.md, and add an honest round-trip example plus Chainsaw
e2e (examples/backups/kafka-metadata, kafka-3-metadata-roundtrip) that
proves a per-run retention.ms sentinel survives both restores.

Unit tests cover the driver's pure helpers (application-ref validation,
readiness verdict, parameter round-trip, per-run artifact-URI keying).

Not yet verified on a live cluster.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
2026-09-10 22:20:27 +04:00
Aleksei Sviridkin 04d6659bb6 test(e2e): cover the passthrough withdrawal in the gateway suite
Three phases on a tenant that owns its Gateway. The shipped passthrough
entries render with nothing routed to them and an HTTPRoute on
api.<apex> keeps its terminate listener, its certificate and
Accepted=True, which is the state a withdrawal keyed on the declaration
alone would take away. Attaching a TLSRoute to that listener, pointed
at a Service in the tenant's own namespace so it resolves, withdraws
all three and moves the route to NoMatchingListenerHostname. A third
phase patches in a native-port entry and reads the per-listener
readiness flag, which is the claim no test against a fake client can
settle.

The first two phases are one measurement rather than two: withdrawing
on demand is what dates the earlier assertion to settled state instead
of to a withdrawal that had not arrived yet.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-10 18:47:30 +03:00
Aleksei Sviridkin 1f04e31ed3 feat(backups): add Redis backup/restore strategy driver (#3992)
## What this PR does

Adds backup/restore support for the `Redis` app (spotahome
`RedisFailover`) via a dedicated `strategy.backups.cozystack.io/Redis`
strategy controller. The spotahome operator ships no backup CRD, so the
driver follows the generic-script-Job pattern (a specialisation of the
Job/Altinity drivers): it runs a one-shot Job that dumps an RDB snapshot
from the current master to the system bucket and replays it on restore.

- **Backup**: discover the current master via the operator's sentinel
Service (`rfs-<name>`, master `mymaster`) → `redis-cli --rdb` → `curl
--aws-sigv4` PUT to `s3://<bucket>/<namespace>/<app>/dump.rdb`.
- **Restore**: GET the object → convert RDB → RESP (`hdt3213/rdb`) →
`FLUSHALL` + `redis-cli --pipe` into the target's master. Both
**in-place** and **to-copy** are implemented; a to-copy restore reads
the source's prefix via `.Backup.ApplicationRef.Name` while writing into
a differently-named target.
- **PITR**: not supported — RDB is a discrete snapshot with no
continuous log to replay to an arbitrary time (the etcd-like case).

The driver is a specialisation of the Job/Altinity script-Job flow with
a `Redis` applicationRef gate and an app-readiness precondition
(surfaces a precise `Ready=False` instead of a silent hang; blocks only
on an explicit `Ready=False` so a missing condition never wedges
backups). Strategy parameters round-trip through the Backup's
`DriverMetadata`; there is no `underlyingResources` snapshot (the Job
pattern re-renders on restore).

### System bucket

Coordinates and credentials reach the Job as env `secretKeyRef` from the
projected `cozy-backups-creds` (the ClickHouse/Altinity delivery model),
so **the app chart is untouched** — no `backup.useSystemBucket`, no new
RBAC (`batch/jobs` and `apps.cozystack.io/*` are already granted). The
bare-host `endpoint` gets `https://` prepended when unscheme'd; against
a self-signed endpoint the strategy verifies with the CA from
`backupStorage.endpointCASecretName` when set, otherwise against the
image's system CA store; skip-verify is a separate explicit
`backupStorage.insecureSkipTLSVerify` opt-in, never a fallback. The
`cozy-default-redis` strategy is gated on `$bucketName` (CRD-ordering)
and bound in the `cozy-default` BackupClass.

### Changes

- `api/backups/strategy/v1alpha1/redis_types.go` + generated CRD and
deepcopy.
- `internal/backupcontroller/redisstrategy_controller.go` (+ unit tests)
wired into both dispatch switches, `supportedBackupStrategyKinds`, and
the no-op restore-cleanup case.
- `cozy-default-redis` strategy template + `cozy-default` BackupClass
route + helm rendering-gate coverage.
- `images/redis-backup` (redis-cli + curl + RDB→RESP converter) +
Makefile target + `backupStrategyController.redisBackupClientImage`.
- `examples/backups/redis/` — an honest marker round-trip for both
restore flows.

### Validation

`go build`/`vet`, `go test ./internal/backupcontroller/...
./api/backups/...`, `make generate` (clean), and `helm unittest` (10/10)
all pass. Not yet cluster-verified.

## Downstream

None.

```release-note
Add backup and restore support for the Redis application (spotahome RedisFailover) via a new Redis backup strategy, wired into the cozy-default BackupClass. Supports in-place and to-copy restore; PITR is not supported.
```


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

* **New Features**
* Added Redis backup and restore support using RDB snapshots in
S3-compatible storage.
  * Supports restoring data in place or to a separate Redis instance.
* Added configuration for Redis backup images, custom HTTPS certificate
authorities, and TLS verification.
* Added a complete Redis backup and restore demonstration with automated
setup, verification, and cleanup.

* **Documentation**
* Documented Redis storage, credentials, endpoints, TLS, and restore
behavior.
  * Added administrator and tenant scenario guides.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-10 19:17:26 +04:00
Alexey Artamonov c2e7c86ff0 fix(hack): avoid a SIGPIPE false-negative in the resourcesPreset check
check-rd-presets.sh tests each expected preset with
`printf '%s\n' "$enums" | grep -Fqx -- "$want"`. When grep matches it
exits immediately and closes the pipe, so printf is killed by SIGPIPE
(exit 141) while it still has data to write. Under `set -o pipefail`
that 141 propagates as the pipeline's status, so `if ! <pipeline>`
reads a preset that IS present as missing. It is a race with the pipe
buffer, so it fires intermittently and mostly on the larger enums --
seen in CI on http-cache-rd, whose schema carries two 47-entry enums --
failing the "Unit & controller tests" job for unrelated PRs.

Feed grep from a here-string instead of a pipe: no second process, no
SIGPIPE, no pipefail interaction. Deterministic repro of the old
behaviour (target at the front of a 100k-line list): the pipe form
reports "missing", the here-string form finds it.

Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
2026-09-10 17:22:28 +03: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
Andrey Kolkov 92edfb810b fix(backups): run the strategy bats under bash; move template essays to {{/* */}}
The behavioural bats test ran the rendered `set -euo pipefail` script via
`sh`; the CI unit-tests runner's /bin/sh is dash, which rejects `-o pipefail`
at line 1 and reddened the whole suite (production is busybox ash, which
supports it, so the script itself is fine). Run it via `bash`, matching ash
semantics and the hack/migration-50-etcd-adopt.bats precedent.

Convert the multi-paragraph `#` rationale blocks in the shipped strategy
template to `{{/* */}}` so Helm strips them at render instead of copying the
prose verbatim into every rendered Strategy CR, where it drifts as the code
changes. Short one- to three-line guard comments stay as `#`.

Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
2026-09-09 19:23:37 +04:00
Andrey Kolkov 9eb7eeaca5 test(backups): behaviourally pin the shipped Redis strategy script
helm-unittest renders this template but asserts only its structural
decisions, so blanking the object key, breaking the rfs-redis- sentinel
prefix, or disabling the cleanup guard each left the suite green. Add a
bats test (auto-discovered by make bats-unit-tests) that extracts the
rendered script and EXECUTES it with stubbed redis-cli / curl: cleanup
must DELETE its object and never reach the restore FLUSHALL against the
live master, backup must resolve rfs-redis-<app> and upload under the
recorded object key, and an unrecognised mode must fail closed. Each of
those three mutations now turns the suite red.

Also correct the endpoint doc row: the Redis Job always connects over
https (the projector strips the scheme and the script prepends it), so
its TLS is not scheme-driven the way MariaDB/FoundationDB is.

Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
2026-09-09 18:34:37 +04:00
Andrei Kvapil cf6543bce0 test(hack): guard resourceNames selectors against a missing release prefix
A selector that omits the prefix matches nothing and reports nothing,
which is indistinguishable from a selector that legitimately has nothing
to match. Assert instead that every entry contains the declared prefix,
so the next omission fails a test rather than passing silently.

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
2026-09-08 22:45:34 +02:00
Andrey Kolkov ac79c3757b test(backups): self-contained Redis backup round-trip e2e
The Redis driver shipped no round-trip backup/restore e2e, unlike its
ClickHouse, MariaDB and RabbitMQ siblings, so the backup to S3 and restore
data path had no automated coverage. The existing demo backed up through the
platform cozy-default BackupClass, whose endpoint is the shared system
bucket; in the e2e environment that bucket's advertised external address is
not routable, so a cozy-default round-trip cannot run there.

Rework examples/backups/redis to provision its OWN Bucket + Redis strategy +
BackupClass (mirroring the MariaDB and RabbitMQ demos), projecting the
bucket's keys and the seaweedfs CA into the Secrets the strategy references
and honouring an S3_ENDPOINT override so the backup Pod targets the
in-cluster seaweedfs. Add a redis-2-backup-roundtrip Chainsaw test that
drives run-all.sh in tenant-root and asserts the sentinel round-trips
through object storage in place and into a copy.

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

## What this PR does

`hack/helm-unit-tests.sh` decides whether to enter a package by asking
`make -C <pkg> -n test`. `packages/system/gpu-operator` declared only
`update:`, so the sweep skipped it silently and the three test cases in
`tests/kernel_module_config_test.yaml` had never run. They pass, so
nothing was hiding behind them. Still worth fixing: the directory reads
as coverage to whoever opens it, and a suite nothing runs is where a
stale assertion sits.

The target is phony. `hack/package.mk`'s `.PHONY` line lists only the
targets that file defines, so a path named `test` next to the Makefile
would make the target up to date and skip the recipe.

Rather than fix the one package I enumerated the rest of the tree. 84
packages hold a `tests/` directory, 83 ship at least one
`tests/*_test.yaml`, and gpu-operator was the only one of those without
the target. The 84th is `packages/system/nfs-driver`, whose `tests/`
holds a PersistentVolumeClaim and a StorageClass to apply by hand. No
suite there, so a `test:` target would only make the sweep fail it for
reporting zero suites. Left alone.

`hack/package-test-target-coverage.bats` pins the class. Every other
guard in the sweep runs inside a directory it already reached, so a
package with no target escapes all of them at once and the run still
reports success. The new case probes with `make -n` instead of grepping
for `test:`, which is the question the sweep asks, and it mirrors the
sweep's other precondition too: the sweep requires `$dir/Makefile` and
returns before probing, where make would also accept `GNUmakefile` or no
makefile at all. The verdict comes from the sweep's command unchanged,
and a failing package is probed a second time with `LC_ALL=C
--no-print-directory` purely to read the diagnostic, since both of those
are visible to a Makefile through `LC_ALL` and `MAKEFLAGS`.

Mutations, because a guard that has never been red proves nothing:

```text
target removed from gpu-operator  -> red, names packages/system/gpu-operator, quotes make's error
Makefile renamed to GNUmakefile   -> red; make still answers 0 here, so this leg is load-bearing
glob pointed at a missing dir     -> red via the blindness leg, instead of passing on an empty set
unmodified tree                   -> green, 83 packages scanned
```

The full `hack/helm-unit-tests.sh` sweep now enters gpu-operator and
finishes green.

One detail that is invisible on a Mac: the probe runs under
`--no-print-directory`. On modern GNU make `-C` turns on `-w`, and the
fatal path prints `Leaving directory` after the error, so `tail -n 1`
picks up the directory trace and drops the diagnostic. Measured on 4.4.1
where that happens, and on 3.81 (still `/usr/bin/make` on macOS) where
`-C` prints no trace at all.

### Screenshots

No UI changes.

### Downstream repositories

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

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

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

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

Walked the trigger map against both changed files. Two entries come
close and both miss. `ccp` gates on `hack/package.mk`,
`hack/common-envs.mk` and the `packages/{apps,system,extra,core}`
layout; nothing here is moved or renamed and no existing target changes
what it does, so a new `test:` target sitting beside 80 identical ones
is not something its skills read. The `website` entry for developer
tooling points at `content/en/docs/next/development.md`, whose Testing
section covers the e2e sandbox under `packages/core/testing` and says
nothing about the per-package helm-unittest sweep.

- [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
test(tests): run the gpu-operator helm-unittest suite, which the sweep had been skipping because the package declared no `test:` target
```


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

## Summary by CodeRabbit

- **Tests**
- Added coverage checks to verify that all Helm test suites are included
in the package test sweep.
  - Added diagnostics for packages missing a runnable test target.

- **Bug Fixes**
- Added a test target to the GPU operator package, enabling its Helm
tests to run through the standard package test workflow.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-08 02:49:10 +04:00
Andrei Kvapil 54621bc046 feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to
import their own machines without a platform administrator. The chart
approach could not express that: a tenant can neither create the Secret
holding vCenter credentials nor name the VDDK image, so both had to come
from somewhere they cannot reach.

Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A
VMImportSource is a long-lived connection -- type, endpoint, credentials,
readiness -- and a VMImportTask is the one-shot operation that names
machines and produces VMDisks and VMInstances. The outputs carry no owner
reference back to the task, so deleting a finished import leaves the
machines alone. Konveyor Forklift does the transfer underneath; the
controller renders its objects, mirrors its verdicts, and hands each
finished volume into a VMDisk without copying it.

What a live vCenter migration forced out, and no amount of source reading
would have:

- A guest copied verbatim keeps the drivers it had under VMware and no
  virtio, so on a virtio disk it does not fail the import -- it succeeds
  and then does not boot. Imported disks go on SATA.
- VDDK connects straight to the ESXi host at the address vCenter
  advertises, which is routinely unreachable, and on the first cluster
  collided with the Service CIDR. `spec.hosts` redirects per host.
- Firmware must be carried across or a UEFI guest imports as BIOS and
  never boots (stacked on #3002).
- Deleting a task mid-transfer orphaned its DataVolume; a finalizer now
  removes the engine's volumes and leaves the outputs.

The console gains a Migration section: connections are editable, and the
VM field is a picker built from a machine list the controller publishes,
since the aggregated API cannot reach the inventory and should not hold a
source's credentials to try. Two general mechanisms fall out of that --
an option source may take an argument, and one named in a CRD annotation
may reference a sibling field.

Design: cozystack/community#62. Docs: cozystack/website#673.

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
2026-09-07 17:03:44 +02:00
Aleksei Sviridkin d000d02b99 feat(rabbitmq): add TLS support for AMQPS and management HTTPS (#2683)
## What this PR does

Adds TLS for AMQP and the management API, and hands the tenant a
key-free trust anchor instead of a private key.

- A chart-authored cert-manager chain issues the broker certificate. The
chart names its CA Secret in a `TenantProjection` sentinel; the
CA-extraction controller reads `ca.crt` from that Secret and writes a
key-free projection, stamping `internal.cozystack.io/tenant-ca` on the
projection it owns. That label is what the ResourceDefinition selects.
Nothing labels the CA Secret itself, which holds the CA private key and
is excluded by name.
- The leaf `<release>-tls` carries the broker's private key and reaches
no tenant. Neither the dashboard Role nor the ResourceDefinition grants
it, and the ResourceDefinition excludes it by name as well, so a label
mis-stamped onto it later still does not get through.
- SAN coverage now lists every resolvable form explicitly, including the
bare `*.<release>-nodes` wildcard that the operator uses for
`RABBITMQ_NODENAME`.

## TLS is opt-in here, unlike the sibling charts

`tls.enabled` defaults to `false` and is not inferred from `external`.
That diverges from nats, qdrant and kafka, which infer it, and the
divergence is deliberate for a reason specific to this operator.

Setting `spec.tls.secretName` makes the cluster operator rewrite the
`port` key in the `<release>-default-user` Secret from 5672 to 5671, on
the mere fact of TLS and regardless of whether the plaintext listener is
still open. That Secret is the Service Binding the chart hands the
tenant. A client with a hardcoded `:5672` keeps working. A client that
reads the endpoint from the binding, which is the documented way, gets
moved to the TLS port and must then speak TLS and be given a CA to
verify against, neither of which it was configured for. Inferring TLS
from `external` would fire all of that on an ordinary upgrade, with no
configuration change, against exactly the clients that followed the
documentation.

Whether the other engines should follow is an epic-level question rather
than this PR's to answer; for nats it is raised in #3348.

## The plaintext ports are still published, and this PR says so

There is no operator-supported way to publish the TLS ports alone.
`spec.override.service.spec.ports` is a strategic merge keyed on `port`
against defaults the operator rebuilds every reconcile, so it adds and
mutates entries but never subtracts one, and a TLS-only port list there
would read as a guarantee while changing nothing. The chart renders no
service override at all and says why at the callsite.

`tls.disableNonTLSListeners` is offered as an explicit opt-in, and it
refuses to render rather than failing silently in three situations.

- Together with `users` or `vhosts`. The flag forces
messaging-topology-operator onto TLS regardless of its own
configuration, after which it verifies the broker against the system
certificate pool, where a per-release CA cannot be. Every topology
object bound to the release would stop reconciling, silently:
already-created users keep working while password rotation and new users
simply never apply. There is no working route left for it, so the chart
refuses instead. Managing those objects as hand-written CRs is not a way
out, because the operator takes TLS from the `RabbitmqCluster` and not
from whoever created the CR, so manual objects break identically and
without the render-time refusal. `rabbitmqctl` or a definitions import
are.
- While topology objects are still live in the namespace. See the next
section.
- With TLS off, where no listener of either kind would remain.

Erlang distribution (25672) and epmd (4369) stay plaintext under every
setting; the flag does not touch them, so `disableNonTLSListeners: true`
is not "TLS everywhere".

A third route, closing AMQP at the broker through
`spec.rabbitmq.additionalConfig` while leaving the operator on plaintext
management, has been suggested and is plausible, but the operator's
sources are not vendored here and it was not verified. It is recorded as
an open question rather than promised.

## Closing the listeners takes two upgrades, and the chart enforces it

Clearing `users` and `vhosts` and setting `tls.disableNonTLSListeners:
true` in one edit is the natural thing to try, and it wedges the objects
being removed. The values-level refusal cannot catch it, because by then
both maps are empty. Helm applies every object in the target release
before pruning the ones the release dropped, so the broker closes
plaintext and loses the `rabbitmq.com/operator-connection-uri`
annotation while the topology objects still hold
`deletion.finalizers.<plural>.rabbitmq.com`. Their operator then has to
reach a management API it can only get to over a certificate it verifies
against the system trust pool. The delete never completes, and the
upgrade reports success over objects left in `Terminating`, with the
users still live in the broker and `kubectl delete namespace` on that
tenant hanging.

The chart now reads the namespace rather than the values. When the flag
is being introduced and any topology object is still there, the render
refuses and names them, gives the two-upgrade order and the `kubectl
get` that confirms nothing bound to this release is left, and says how
to unwedge a release that is already stuck. The check keys on the live
`RabbitmqCluster` as well, so it applies only to the transition: once
the listeners are closed, an object a tenant creates afterwards is
theirs to clean up rather than something that stops the release
rendering. Eleven unit cases cover the firing and the silent paths.

The set is wider than the three kinds this chart creates. All twelve
kinds `messaging-topology-operator` drives through its shared
`TopologyReconciler` carry that finalizer and share one deletion path,
so a `Queue` or a `Shovel` written by hand against the release wedges
exactly the same way, and a runbook step that checks only users, vhosts
and permissions says the namespace is clear when it is not. The list is
derived in one place and read from both the lookup and the `kubectl get`
the refusal prints, and a unit test holds it against the operator's own
CRDs so a bump cannot introduce a kind the guard has never heard of.
`SuperStream` is the one CRD left out: it runs on its own reconciler
with no finalizer, and the objects it owns are already on the list. An
object that binds through `connectionSecret` instead of naming the
cluster stays outside the guard, which the comment says.

## Breaking for metrics scrapers

Enabling TLS moves the metrics endpoint from 15692 to 15691. A
ServiceMonitor selecting the port by the name `prometheus` silently
stops matching and the instance's metrics disappear with no error
anywhere. Nothing in this repository scrapes RabbitMQ, so only
user-defined monitoring is affected, silently, which is why it is stated
here.

## Known limitations

1. Turning TLS back off leaves the raw CA material behind. The anchor
itself is collected: the projection carries an owner reference to the
`TenantProjection` sentinel, and Helm deletes the sentinel along with
the rest of the chain. What survives is `<release>-ca` and
`<release>-tls`, because cert-manager runs with
`enableCertificateOwnerRef: false` and does not delete the Secrets it
issued, so the README says to remove them by hand. #3349 asks the same
question of the label-driven leg of the contract, which this chart does
not use.
2. No e2e coverage for the TLS path. The one-upgrade wedge above is the
case that most deserves it: install with TLS and users, upgrade to no
users plus the flag, and assert the `User` is actually gone rather than
merely marked for deletion. A template test cannot reach that.

On certificate renewal, the leaf carries `duration: 8760h` and
`renewBefore: 720h`, so cert-manager rewrites the Secret around day 335.
The cluster operator mounts it as a projected volume with no `subPath`,
so the kubelet refreshes the files in place, and the operator does not
watch a user-supplied TLS Secret, so nothing rolls the pods. The last
hop, the broker picking the new file up, rests on Erlang's
`ssl_pem_cache` re-reading by mtime and has only been read from sources
rather than exercised against a live broker. The merged qdrant chart
ships the same duration and `rotationPolicy`, so this is a platform-wide
property rather than something this PR introduces.

### Downstream repositories

`packages/apps/rabbitmq/values.schema.json` gains the `tls` object, so
`cozystack/terraform-provider-cozystack` needs the field modelled for
`cozystack_rabbitmq`; tracked in
cozystack/terraform-provider-cozystack#42.

### Release note

```release-note
feat(rabbitmq): opt-in TLS for AMQP and the management API, with the CA published to tenants as a key-free trust anchor. Enabling TLS moves the metrics port from 15692 to 15691 (ServiceMonitors selecting the port named `prometheus` must be updated) and moves the `port` value in the `<release>-default-user` binding Secret from 5672 to 5671. Plaintext AMQP and management ports remain published; `tls.disableNonTLSListeners` closes them but also drops in-cluster plaintext clients, and must be enabled in a separate upgrade after the topology objects are gone.
```
2026-09-07 18:51:34 +04:00
Aleksei Sviridkin d685fb30ec fix(gateway)!: require tenant routes to declare their hostnames (#3475)
## What this PR does

The tenant route hostname policy skipped the shape it most needed to
check. Its CEL opened with `!has(object.spec.hostnames) ||`, so an
HTTPRoute or TLSRoute declaring no hostnames satisfied the first operand
and got admitted with nothing compared against the tenant apex. An empty
list passed too: `has()` is true on `[]`, and `all()` over an empty list
is vacuously true.

Such a route is not inert. Under Gateway API it inherits the hostnames
of every listener it attaches to, and `parentRefs` can name a Gateway in
another namespace, so it covers whatever that listener covers. Reported
in #3467.

The predicate now requires the field to be present and non-empty before
validating it. Both policies rendered from that template share one CEL
string, so HTTPRoute and TLSRoute get the change together, and both
match `CREATE` and `UPDATE`, so an existing route that drops its
hostnames is denied exactly as a new one is.

The issue is titled after TLSRoute, but the reachable shape is mostly
HTTPRoute. TLSRoute allows a hostname-less route only on the deprecated
`v1alpha2`; at `v1` and `v1alpha3` the bundle this repo vendors marks
`spec.hostnames` required and rejects an empty list on `minItems: 1`,
while `v1alpha2` carries neither constraint. HTTPRoute `v1` carries
neither either, so it accepts both shapes. All four are checkable in
`packages/system/gateway-api-crds/templates/crds-experimental.yaml`.

Checking that turned up a stale claim in the template header, which said
TLSRoute had no `v1` in Gateway API 1.4 to 1.5 and used it to justify
the rule naming only `v1alpha2`. Gateway API 1.5.1, which this repo
vendors, serves TLSRoute at `v1` as the storage version. The rule still
covers every served version because `matchConstraints.matchPolicy`
defaults to `Equivalent`, and that default is now written down, because
setting `matchPolicy: Exact` reads like a tightening and would silently
stop the policy seeing `v1` and `v1alpha3` TLSRoutes. I corrected the
text and left the coverage alone: the wrong sentence predates this
branch, but my change is what makes the file contradict itself, so the
text is mine to fix while widening version coverage is a separate job.

A `ValidatingAdmissionPolicy` has no cross-object lookup, so it cannot
resolve the parent Gateway to see which hostnames a route would inherit.
Requiring the route to name its own is what keeps every admitted
hostname comparable against the apex. That has a cost: a route relying
on listener inheritance now has to spell its hostnames out, and a tenant
that hand-writes one gets a denial where it used to get an admission.

### The controller change belongs in the same PR

`renderHTTPRedirect` built the http-to-https redirect HTTPRoute with no
hostnames on purpose, to catch every host on port 80. It lands in a
`tenant-*` namespace, so the stricter policy denies it,
`reconcileHTTPToHTTPSRedirect` returns that error, and since it runs
before the status update the TenantGateway never reaches Ready. That
redirect is what keeps app routes attaching by hostname from serving
plaintext on port 80. Neither half of this change is safe alone.

The redirect now names the tenant apex and its wildcard. Both entries
are needed: a wildcard hostname is a suffix match over any number of
labels, so `*.<apex>` covers subdomains at every depth but not the bare
`<apex>`. `renderWildcardCertificate` already builds the same pair.

Coverage narrows in one direction. A request on port 80 with a Host
outside the tenant apex now falls through to a 404 instead of a 301.
That narrowing costs no covered host: the port-80 listener admits routes
only from the Gateway's own namespace and cert-manager's, and the
tightened policy requires every route in the tenant namespace to declare
its hostnames inside the apex, so any host published from this namespace
already falls under the apex or its wildcard.

An inheriting child tenant does not widen that set either. Its hostnames
would reach port 443 through a listener on this same Gateway, and
`cozystack-gateway-hostname-policy` compares every listener against the
host label of the Gateway's own namespace, so a child apex outside that
label never gets its listener: the Gateway write carrying it is refused
at admission whole, in `reconcileGateway` and before the redirect is
rendered. A child apex under this one passes that check and the wildcard
already covers it. That holds at merge base as well, so it is not
something this branch changes.

Deriving hostnames from `spec.apex` makes that field load-bearing for
admission, so it gains `MinLength=1`. An empty apex used to reconcile to
Ready, since the redirect declared no hostnames at all; it would now
render `""` and `"*."` and be rejected on the HTTPRoute hostname schema,
surfacing as a validation error naming the route rather than the field.
The renderer checks it too, because the CRD and the controller binary
roll out separately and an older CRD in the cluster will not carry the
bound.

That schema bound is a breaking change to an existing field and the API
review gate flags it as one, so here is what it actually invalidates.
The chart that renders these CRs already fails on an empty
`_namespace.host`, and in DNS-01 or existingSecret mode an empty apex
already broke earlier, in `renderGateway`, on a listener with `hostname:
"*."`. Nothing that works today becomes invalid. What the bound rejects
is a TenantGateway written by hand, past the chart, with an empty apex,
and I would rather that be refused at admission naming the field than at
reconcile time in controller logs, which is why the bound sits in the
CRD as well as the renderer.

Naming those hostnames also meant excluding the redirect from
`collectHostnameClaims`. A route that attaches to the Gateway and
declares hostnames otherwise reads as an app publishing them, and each
claimed hostname becomes an HTTPS listener named after its first label.
The first label of `*.<apex>` is `*`, which the Gateway CRD's
listener-name pattern rejects, so the Gateway write fails and the
reconcile stops before anything downstream. The exclusion matches on
namespace, name and controller ownership rather than on the labels the
renderer stamps: those labels are writable by anyone who can create a
route, and the claim set also drives conflict resolution, so a
label-keyed exception would let any route opt out of being marked
`Accepted=False` when it loses a hostname race. Name and ownership are
each required and each is pinned by its own test, since ownership alone
would exclude any other controller-owned route and name alone would
extend the exception to a route planted under that name by someone else.

### Radius

Only namespaces starting with `tenant-` are matched. The three
first-party TLSRoutes the platform ships, `kubernetes-api` in `default`
plus `cdi-uploadproxy` and `vm-exportproxy`, set hostnames explicitly
and none sits in a `tenant-*` namespace, so nothing changes for them.

I went through every producer of routes in the tree, in three places
rather than one. No Helm template emits an absent or empty hostname list
in any configuration a tenant can reach: the vendored openbao, harbor,
opencost and flux-operator route templates can render one, but each is
behind a flag that defaults off with no values path to turn it on, and
openbao would fail CRD validation first because its default route
apiVersion requires hostnames. Exactly one Go site builds a route, the
redirect fixed here. And one e2e fixture did rely on the old behaviour:
the Gateway suite's parentRef probe declared no hostnames and is applied
into `tenant-test`, which the policy matches, so it now reads the apex
from the namespace label and names a hostname under it, the same way the
neighbouring cases in that suite already did.

cert-manager also writes routes into tenant namespaces, since it creates
the Challenge in the Certificate's own namespace and the HTTP-01 solver
route beside it. That route declares the hostname it is validating, so
the policy admits it.

### Related issues

Two open issues have a text half that this PR happens to settle, and
neither of them closes here. #3788 reports that the port-80 allow-list
contains the namespace its own comment says it excludes; the comment and
the README paragraph now describe what that filter actually does, while
the behavioural exit it names stays open. #3789 reports that the
TLSRoute rule names only the deprecated API version; that premise is now
written down in the template header, including what has to change before
a bundle drops `v1alpha2`, but the rule still names `v1alpha2` alone.
#3787 covers the wildcard-hostname listener name that the apiserver
rejects, which is what makes the claim-set exclusion here load-bearing
rather than tidy.

### Screenshots

N/A (no UI changes).

### Downstream repositories

I walked the trigger map in `docs/agents/contributing.md` against the
diff, file by file. No package is added, renamed or removed; no
`values.yaml` key, `values.schema.json` field, version enum or default
moves; no `kind` or `plural` changes; no namespace or release-asset name
changes; nothing under `hack/` moves, is renamed, or changes what a make
target does.

One row deserves a sentence rather than a blanket denial, because the
diff does change a CRD schema: `spec.apex` on
`gateway.cozystack.io/v1alpha1 TenantGateway` gains a `minLength` bound,
which lands in the generated `gateway.cozystack.io_tenantgateways.yaml`.
The provider hand-types `Package` under `cozystack.io` and
`Plan`/`RestoreJob` under `backups.cozystack.io`; `gateway.cozystack.io`
is not among them, and a tightening on an existing field adds no new
field for it to offer typed access to. The
`packages/extra/gateway/README.md` edit sits in the `## Security model`
section, below the `## Parameters` block that `cozyvalues-gen`
regenerates, so it survives `make generate`.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

### Release note

```release-note
fix(gateway)!: the tenant route hostname policy now requires an HTTPRoute or TLSRoute in a `tenant-*` namespace to declare a non-empty `spec.hostnames`. A route that omitted the field, or set it to an empty list, was previously admitted without any hostname being checked against the tenant apex, even though such a route inherits the hostnames of the listener it attaches to rather than matching nothing. A route in a `tenant-*` namespace relying on that inheritance is now rejected and has to name its hostnames. The controller-owned http-to-https redirect route names the tenant apex and its wildcard accordingly, so a request reaching port 80 with a Host outside the tenant apex now gets a 404 instead of a 301. `TenantGateway.spec.apex` additionally gains a `minLength: 1` bound, because the redirect derives its hostnames from that field; a TenantGateway written past the chart with an empty apex is now rejected at admission instead of failing later in the controller.
```


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

- **New Features**
- Added edge-terminated gateway support with plain-HTTP listeners and
TLS passthrough handling.
- HTTP-to-HTTPS redirects now explicitly cover the tenant apex and
subdomains.
  - Gateway configurations require a non-empty apex hostname.
  - HTTPRoute, TLSRoute, and GRPCRoute resources must declare hostnames.

- **Bug Fixes**
  - Invalid hostname-less routes are now rejected consistently.
- Controller-managed redirects no longer trigger unnecessary hostname
claims, listeners, or certificates.
- Switching certificate modes now cleans up obsolete ACME resources and
route status.

- **Documentation**
- Updated security and gateway guidance for hostname validation, edge
termination, route compatibility, and certificate handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-07 18:49:04 +04:00