mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 08:37:31 +09:00
feat/vm-disk-http-auth
6366
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Timofei Larkin
|
79b2f56058 |
feat(vm-disk): let an http source carry header Secrets and a CA ConfigMap
An image behind an authenticated endpoint could not be imported: the http source took a bare URL, so the only way to hand CDI credentials was a pre-signed URL that anyone holding it could use until it expired. The source gains secretExtraHeaders, the names of Secrets in the release namespace each holding one extra request header, and certConfigMap, a ConfigMap with the CA bundle to trust, both passed to the DataVolume as CDI defines them. The immutability guard treats a stored http source with these keys as chart-shaped, so a later change to them is refused like a URL change; a stored key the chart never renders still marks a foreign source it leaves alone. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
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 --> |
||
|
IvanHunters
|
a19fd0274f |
fix(kafka): delete release topics before the topic operator is removed (#3938)
## What this PR does Fixes Kafka application deletion when a release-owned KafkaTopic still has Strimzi's `strimzi.io/topic-operator` finalizer. Without ordering, Helm removes the topic operator with the release, the KafkaTopic cannot finish deleting, and reinstalling the same release name times out on the leftover object (#3793). A namespace-scoped pre-delete hook deletes only KafkaTopics carrying the current release label while the operator is still running. It never removes finalizers directly. The hook uses a digest-pinned kubectl image, restricted RBAC, a 270-second job deadline, a 180-second deletion timeout, and a 20-second request timeout. The E2E absence checks allow five minutes, matching the hook's Helm budget. A wait that times out with the finalizer still held exits 0 on purpose, so the uninstall can finish rather than wedging the release under helm-controller's retry loop. That is a deliberate trade and the hook says so on stderr; the Job survives an hour under `ttlSecondsAfterFinished` so the record outlives the run. Validation: - `helm unittest packages/apps/kafka`: 37/37 passed, covering the hook resources, RBAC wiring, the digest pin and the `_cluster.images-registry` override, the release-scoped selector, and the Job deadline and delete-policy settings. - `hack/cozytest.sh hack/kafka-pre-delete-hook.bats`: 5/5 passed. The suite renders the hook and runs the script against a fake kubectl, because every property worth pinning there is an exit code and matching manifest text cannot establish one. Run under the runner `make bats-unit-tests` uses rather than the `bats` binary, which do not accept the same test file. - Both suites were mutation-checked: putting the timeout branch back to `exit 1`, flipping the catch-all to `exit 0`, dropping `--ignore-not-found`, inverting the guard, widening the selector to a sibling release, and setting `--wait=false` each turn one of them red. - The raw template parses with `yq`; all leading Helm-expression scalars are quoted. - Chainsaw asserts both release topics are absent after deletion, but the full E2E suite was not run locally because it requires a disposable cluster. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the chart hook and E2E test. No values/schema/API/default or downstream hardcoded contract changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(kafka): delete release-owned KafkaTopics before removing the Strimzi topic operator ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Kafka deletion now automatically removes associated Kafka topics before the Kafka resource is removed. - Cleanup waits for topics to disappear before checking ZooKeeper storage reclamation. - Deletion remains resilient when topic resources or their definitions are absent, or when finalizers delay removal. - Cleanup failures are reported appropriately without blocking deletion when a timeout occurs. - **Tests** - Added coverage for cleanup behavior, execution settings, permissions, and failure scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
Yan Bondarenko
|
210b7fe30c |
fix(kafka): stop the pre-delete hook from wedging the uninstall
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
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> |
||
|
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> |
||
|
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 --> |
||
|
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 --> |
||
|
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> |
||
|
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 -->
|
||
|
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> |
||
|
IvanHunters
|
79602e9ca0 |
fix(opensearch-operator): keep a single-replica cluster electable after bootstrap (#4231)
## What this PR does Builds the OpenSearch operator in-tree, the way kamaji and redis-operator already are, and carries one source patch. The operator deletes its bootstrap pod the moment `status.initialized` flips, without looking at the voting configuration. With `replicas: 1` there are exactly two cluster-manager-eligible nodes, OpenSearch keeps the voting configuration at an odd size of one, and the voter it keeps is the bootstrap node that formed the cluster. Deleting that pod takes away the only vote there was, and the survivor can never hold an election again, because changing the voting configuration itself needs a quorum. Full write-up in #4230. The patch excludes the bootstrap node from the voting configuration before the pod goes, and confirms the exclusion committed before deleting anything: 1. Pod already gone: nothing to do. 2. Build a cluster client. Cannot: requeue, keep the pod. 3. `POST /_cluster/voting_config_exclusions?node_names=<release>-bootstrap-0`. Fails: requeue, keep the pod. 4. Read the voting configuration back. Still a voter: requeue, keep the pod. 5. Delete the pod. 6. `DELETE /_cluster/voting_config_exclusions`. Fails: log and carry on, the pod is already gone and the list is only capped, not load-bearing. Step 4 is the one place this differs from the upstream attempt, which relies on the POST being blocking. I did not verify that property, and polling the committed configuration is correct whether or not it holds. Every failure to reach the cluster keeps the bootstrap pod and requeues. That is a deliberate trade: a bootstrap pod outliving its purpose costs one pod, a cluster that cannot elect a cluster-manager needs a human. ### How the values were arrived at Not from the documentation. Reproduced outside Kubernetes first, two containers with the env the operator sets, `cluster.initial_master_nodes` pointing at the first: ```text last_committed_config: ["D6Fijf-SRN-FwIXVGNehPw"] # the bootstrapper, alone POST .../voting_config_exclusions?node_names=osA -> 200 last_committed_config: ["oy8DPOV-SKKildaIZpgoPg"] # moved to the survivor ``` Without the exclusion, removing the first container leaves the second printing `cluster-manager not discovered or elected yet, an election requires a node with id [...]` forever, which is the same sentence the e2e run produced. ### Why carry it here rather than wait Upstream knows: [opensearch-k8s-operator#1448](https://github.com/opensearch-project/opensearch-k8s-operator/issues/1448) is open with `release-blocker`, and its third point is this exact case. Three PRs address it and none looks close: [#1483](https://github.com/opensearch-project/opensearch-k8s-operator/pull/1483) has changes-requested, a merge conflict and a red lint; [#1150](https://github.com/opensearch-project/opensearch-k8s-operator/pull/1150) and [#1430](https://github.com/opensearch-project/opensearch-k8s-operator/pull/1430) have not moved since July. When a release carries the fix, this patch and this image build come back out. The alternative considered and rejected was refusing `replicas: 1` in `values.schema.json`. It is a breaking change for existing releases and removes a configuration rather than fixing it. ### Before this can merge `values.yaml` deliberately carries no `manager.image` yet. The image does not exist, and committing a first-party reference without a digest is exactly what `docs/agents/image-refs.md` forbids. Once the first build here publishes it, I will add `repository` plus `tag@sha256:...` in a second commit and take the PR out of draft. ### Note on CI cost Adding the build line to the root `Makefile` puts it in `full_rebuild_pattern`, so every push to this branch rebuilds all 28 units rather than one. Verified with `hack/build-matrix.sh`. Unavoidable: the line is what puts the image in the matrix at all. ### What was checked locally - The image builds and the patch applies cleanly; `/manager --help` runs from the built image. - `go build ./...`, `go vet` and the operator's own `pkg/reconcilers` tests pass with the patch applied. - `gofmt` clean, `helm unittest` for the package green, `pre-commit` green on the changed files. - `hack/build-matrix.sh` selects the package for a package-only change. Not checked locally, and the reason this needs the e2e lane: that the fix actually keeps a real cluster electable. The `opensearch` chainsaw suite is the test for it. ### 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: ### Release note ```release-note fix(opensearch-operator): exclude the bootstrap node from the voting configuration before removing it, so a single-replica OpenSearch cluster stays able to elect a cluster-manager ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for building and publishing the OpenSearch operator image as part of the standard build workflow. - Operator image versions now stay aligned automatically with the packaged source version. - Added a packaged operator image configuration for consistent deployment. - Improved cluster initialization by safely excluding the bootstrap node from voting before removal and clearing the exclusion afterward. - Added retry handling when cluster state is unavailable or voting changes are not yet committed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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> |
||
|
IvanHunters
|
b222cb6580 |
fix(opensearch-operator): keep a single-replica cluster electable after bootstrap
The operator deletes the bootstrap pod as soon as initialization completes and never looks at the voting configuration first. With one cluster_manager replica there are exactly two eligible nodes, OpenSearch keeps the configuration at an odd size of one, and the voter it keeps is the bootstrap node that formed the cluster. Deleting that pod takes away the only vote there was, and the survivor can never hold an election again, because changing the configuration itself needs a quorum. The cluster is unusable about a minute after it installs, and a node with no cluster-manager still answers the readiness probe, so nothing says so until something restarts the pod. Build the operator in-tree, the way kamaji and redis-operator already are, and carry a patch that excludes the bootstrap node from the voting configuration and waits for that to commit before deleting the pod. Reaching the cluster is a precondition rather than a nicety: every failure keeps the pod and requeues, because a bootstrap pod that outlives its purpose costs a pod, while a cluster that cannot elect does not recover without a human. Deleting a pod is a request rather than a wait, so clearing the exclusion afterwards can find the node still in the cluster and fail. That path requeues and is retried once the pod is really gone, which keeps a stale entry out of the capped exclusion list; clearing stays at the API default wait_for_removal=true so it cannot re-admit a node some other operation is still draining. Upstream carries the same defect, tracked as opensearch-project/opensearch-k8s-operator#1448. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
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 --> |
||
|
Andrey Kolkov
|
01967b56d3 |
docs(backups): correct the Kafka .Mode doc and the embedded-script templating note
The KafkaSpec.Template doc listed .Mode as "backup" or "restore", but the driver also renders it as "cleanup" when reaping a removed Backup's object; the comment is copied verbatim into the CRD, so kubectl explain and the dashboard showed two values while a custom strategy's restore branch would run on the cleanup pass. List all three, matching the Redis sibling, and regenerate the CRD. The strategy template also claimed the embedded script "reads only env vars, never Helm/controller values, so no tpl is applied". The controller renders every string leaf of the PodTemplateSpec through internal/template, args[0] included, so the script is templated on every run. State that instead, without literal template braces that Helm would itself expand. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
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 |
||
|
Andrey Kolkov
|
02bf3e9970 |
fix(backups): probe live Kafka topics without a pipe and isolate the scratch file
The restore existence check piped the live topic list into `grep -qxF`. grep exits at the first match, so once the matched topic is far enough from the end of a list larger than the 64 KiB pipe buffer, the producing printf takes SIGPIPE, pipefail turns the pipeline into a failure, the `if` reads false, and a topic that exists is sent to --create, which fails with TopicExistsException. Read the list from a here-string instead: no pipe, no SIGPIPE, decided by content rather than list size. Also make the local metadata scratch file overridable (METADATA_FILE), like BIN already is, so the bats suite can point each test at its own path instead of sharing /tmp/kafka-metadata.txt across runs. The Job never sets it and keeps the default. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
ca2a6d6429 |
fix(backups): warn when a Kafka backupTimeout is rejected
kafkaRunDeadline silently substituted the 30m default for a backupTimeout that time.ParseDuration rejects or that falls below the one-minute floor, with no Event, condition or log line. An operator who set an unparseable or sub-minute value, then raised it, saw every run keep dying at DeadlineExceeded with nothing naming the discarded parameter. Return the applied deadline together with a message when the configured value is rejected, and record it as a BackupTimeoutInvalid Warning Event on the BackupJob and RestoreJob (the reconcilers already hold a Recorder). An honoured or unset value stays silent. TestKafkaRunDeadline now asserts the warning fires for the rejected cases and not otherwise. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
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> |
||
|
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> |
||
|
Aleksei Sviridkin
|
5b91c6b7c6 |
fix(registry): run validating admission on TenantSecret writes (#4226)
## What this PR does The apiserver hands validating admission to a storage as the `ValidateObjectFunc` it passes into `Create`, `Update` and `Delete`. Mutating admission runs in the handler, but validating webhooks and `ValidatingAdmissionPolicy` execute only when the storage calls that callback, as `rest.AdmissionToValidateObjectFunc` in `k8s.io/apiserver` builds it for exactly that purpose. TenantSecret bound all three callbacks to `_`, so every write answered as if no policy existed. The practical effect is that the cluster-wide `cozystack-no-delete-guardrail` policy, and any webhook an operator writes for `tenantsecrets`, are inert on this resource. `application` and `securitygroup` call all three callbacks; TenantSecret was the only write-capable storage in the tree that did not. The callbacks are now called on all four write paths, including the create-on-update branch, which is a create and so carries `createValidation` rather than `updateValidation`. What goes to admission is the aggregated TenantSecret the caller submitted, or the stored one it replaces, not the backing Secret, so a policy sees the resource it was written against. This is a behaviour change for anyone who has a policy matching `tenantsecrets` today: writes that silently passed will start being evaluated, and may now be denied. That is the point of the fix, but it is worth saying out loud before it ships. ### Tests Each write path is covered by a denying admission callback, asserting both that the callback runs and that the backing Secret is left alone when it denies. A nil callback keeps every path working, which is what the apiserver passes when no plugin handles the operation. Removing any one of the four calls, one at a time, turns the suite red. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It touches one aggregated storage's write paths: no package added, renamed or removed, no `values.schema.json`, no version enum, no changed default, no `kind` or `plural`, no `release.prefix`, no renamed output Secret or Service, no hand-typed CRD, and nothing under `hack/`. - [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(registry): run validating admission on TenantSecret writes, so ValidatingAdmissionPolicies and webhooks matching tenantsecrets are no longer bypassed ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - TenantSecret create, update, force-create, and delete operations now apply validation checks before completing changes. - Denied create or update requests are stopped before data is saved. - Delete validation runs after the current secret is retrieved and before removal. - TenantSecret write operations remain available when no validation callback is provided. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
682f24f0aa |
fix(opensearch): give the securityconfig update job its own resources (#4228)
## What this PR does The chart rendered `spec.security.config` without `updateJob`, and the operator passes that field straight through to the Job container it builds for `securityadmin`. With nothing set, a tenant namespace applies its own `tenant-range-limits` LimitRange default of 128Mi instead. `securityadmin.sh` runs `java` with no `-Xmx` of its own, so the JVM sizes its heap from the container limit and does not fit in 128Mi. The operator builds the Job with `backoffLimit: 0`, so the single OOM kill is final. Nothing writes the `.opendistro_security` index after that, every node sits behind `Not yet initialized (you may need to run securityadmin)`, and the release never reaches ready. This is what has been failing the `opensearch` chainsaw suite in every run that executes it since 2026-09-11. The suite waits 12 minutes for `status.readyReplicas` on `opensearch-test-nodes`, which is the whole 748s runtime of the red test. Full write-up in #4227. ### Where the numbers come from Measured against `opensearchproject/opensearch:2.11.1` on `linux/amd64`, running the operator's own command against a fresh single-node cluster: | Memory limit | Result | Peak | | --- | --- | --- | | 128Mi | OOMKilled mid-apply | 128Mi | | 192Mi | OOMKilled mid-apply | 192Mi | | 256Mi | success | 208Mi | | 512Mi | success | 253Mi | Having no `-Xmx`, the tool grows into whatever it is given, so the peak tracks the limit and the headroom is what matters rather than the peak itself. The values are constants and not a `resourcesPreset` lookup: the tool configures the cluster once and exits, so what it needs does not scale with the nodes it configures, and the smallest preset would hand it back the 128Mi it dies at. ### Upgrading an already-stuck release This does not unstick one on its own. The operator skips the Job entirely while the existing one carries a matching securityconfig checksum, and bumping the chart does not change that checksum, so the failed Job stays where it is. Delete it once and the operator rebuilds it with the resources above: ```bash kubectl -n <namespace> delete job <release>-securityconfig-update ``` A fresh install needs nothing. ### Not fixed here `backoffLimit: 0` is the operator's, and it is what turns one transient kill into a permanent dead cluster. Worth raising upstream separately. ### 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: ### Release note ```release-note fix(opensearch): stop the securityconfig update job being OOMKilled at the tenant namespace memory default, which left new clusters stuck with security uninitialized and every node unready ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * OpenSearch security configuration updates now use consistent CPU and memory allocations, regardless of the selected node sizing preset. * Improved reliability for update jobs by ensuring they receive defined minimum and maximum resource levels. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
bed172bea2 |
fix(registry): preserve dry-run in tap storage (#4225)
## What this PR does Tap `Create` and `Delete` bound the options parameter to `_` and built their own `metav1.CreateOptions` and `DeleteOptions` for the dynamic client, so the caller's dry-run directive never reached the backing write. `kubectl create --dry-run=server` on a tap created the Flux `OCIRepository` for real, and `kubectl delete tap <name> --dry-run=server` removed the `PackageSource` and its source for real. That is the opposite of what a server-side dry-run promises, and it is reachable from the dashboard and from any client that previews a change before applying it. The directive now reaches all five backing writes: the source create, the update taken on a repeat connect, the `PackageSource` delete, the delete of an unreferenced `OCIRepository`, and the orphan-source fallback, which had no access to the options at all and needed them threaded in. Field manager stays as the storage sets it. Tap synthesizes the backing objects rather than storing what the caller sent, so who owns their fields is a separate question from whether the write happens, and only the second one is a correctness bug. `taps` was the last write-capable storage in the aggregated apiserver still discarding its write options. #3937 covers application, tenant-secret and security-group, and excludes this one on the grounds that it uses a different client; the client is not what stood in the way, since `dynamic.Interface` takes the options by value, dry-run included. ### Tests The fake dynamic client's tracker ignores `DryRun` and persists regardless, so asserting that an object is absent after a dry-run would pass with or without the fix. The tests assert the options recorded on the backing action instead. Reverting any one of the five forwards, one at a time, turns the suite red. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It touches only the write plumbing of the `taps` aggregated storage: no package added, renamed or removed, no `values.schema.json`, no version enum, no changed default, no `kind` or `plural`, no `release.prefix`, no renamed output Secret or Service, no hand-typed CRD, and nothing under `hack/`. - [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(registry): honor server-side dry-run when connecting or disconnecting a tap, which previously performed the write for real ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Server-side dry-run requests for TAP connect and disconnect operations now prevent changes from being written to backing resources. * Dry-run settings are consistently applied when creating, updating, or deleting associated sources, including orphaned sources. * **Tests** * Added coverage confirming dry-run behavior across create and delete workflows, including requests without write options. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
4198914dae |
fix(opensearch): give the securityconfig update job its own resources
The operator hands spec.security.config.updateJob.resources straight to the Job container and the chart left it unset, so a tenant namespace applied its LimitRange default of 128Mi instead. securityadmin.sh runs java with no -Xmx and is OOMKilled at that size, and the operator builds the Job with backoffLimit 0, so that one kill is final: the security index is never written and every node stays unready. Constants rather than a preset value, because the tool configures the cluster once and exits. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
64eff1f3c1 |
fix(registry): run validating admission on TenantSecret writes
The apiserver hands validating admission to a storage as the ValidateObjectFunc it passes into Create, Update and Delete: mutating admission runs in the handler, but validating webhooks and ValidatingAdmissionPolicies only execute when the storage calls that callback. TenantSecret bound all three to _, so every write answered as if no policy existed. The cluster-wide no-delete guardrail and any webhook written for tenantsecrets were inert on this resource, while application and securitygroup honour them. Call the callbacks on all four write paths, including the create-on-update branch, which is a create and so carries createValidation rather than updateValidation. The object handed to admission is the aggregated TenantSecret the caller submitted or the stored one it replaces, not the backing Secret, so a policy sees the resource it was written against. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
6d42223208 |
fix(registry): preserve dry-run in tap storage
Tap Create and Delete bound the options parameter to _ and built their own metav1.CreateOptions and DeleteOptions for the dynamic client, so the caller's dry-run directive never reached the backing write. A --dry-run=server connect created the Flux OCIRepository and a dry-run disconnect deleted the PackageSource and its source for real, which is the opposite of what a server-side dry-run promises. Forward the directive to all five backing writes, including the orphan-source fallback that had no access to the options at all. Field manager stays as the storage sets it: the objects are synthesized here rather than supplied by the caller, so who owns them is a separate question from whether the write happens. The fake dynamic client's tracker ignores DryRun and persists regardless, so the tests assert the options recorded on the backing action instead of the absence of an object; reverting any one of the five turns the suite red. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Timur Tukaev
|
37e22dc356 |
docs(readme): stop the logo filling the page and thin the badge wall (#4222)
## What it looks like now The README opens with the logo rendered at **734×246** on a desktop viewport — a wall of logo before a single word of text — followed by eight badges of ragged widths. The logo is an SVG with no intrinsic width, so GitHub stretches it to the full width of the README column. It also uses the `#gh-light-mode-only` / `#gh-dark-mode-only` fragment hack, which GitHub has since superseded with `<picture>` and `prefers-color-scheme`. ## Changes **Logo** → a `<picture>` element at a fixed `width="340"`. Bounds the logo (now 340×114, measured on the branch) and drops the hack in the same move. Light and dark variants still switch. **Badges** → eight down to six. Two carried no information: - `Open Source` links to opensource.org and says what the Apache-2.0 badge directly beside it already says. - `Status: Active` is a hand-set claim rather than a measurement — the commit-activity badge next to it is the real thing, and this one was also served over plain `http://`. The remaining six are ordered identity → health → support: release, licence, commit activity, OpenSSF Best Practices, OpenSSF Scorecard, commercial support. The licence badge now points at `LICENSE` in this repository rather than at opensource.org. Also removed trailing whitespace on lines where it did nothing. The deliberate two-space line breaks in the use-case list are untouched. ## Verification Rendered on the branch and measured in the browser: logo 340×114, six badges, light and dark variants both resolving. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Updated the README logo for light and dark modes. - Reorganized project badges, updated LICENSE and commit activity links, and added a CLOMonitor badge. - Removed the Open Source and Active badges. - Revised introductory, contribution, and community information. - Updated the Semantic Versioning link and removed the Roadmap link. - Added a project documents table and CNCF/Kubernetes community channels. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
tym83andClaude
|
79ead9cf8d |
docs(readme): give the Slack links an invite path
Both Slack channel links assume the reader is already in the workspace; anonymously they just 403. Added the invite link beside each, so someone arriving from the README can actually get in: cncf-slack.netlify.app for CNCF Slack and slack.kubernetes.io for Kubernetes Slack. Both verified. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
5cdb10d8d2 |
docs(readme): tidy the table header, a duplicate link and a sentence
Final review nits: the project-documents table had an empty header row that GitHub renders as a blank band, ROADMAP ended up linked twice once the table landed, and one sentence in Contributions still read wrong. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
7a4ac1df02 |
docs(readme): fix review findings
Reverted the <picture> element. #gh-light-mode-only keys off GitHub's account theme; prefers-color-scheme keys off the OS. They diverge for anyone who pins a GitHub theme rather than syncing with the system, and the combination 'OS dark, GitHub light' renders the white logo on white. The fragment form is correct in all four combinations. Kept the width attribute, which was the actual fix -- the logo stretched to 734px because the SVG has no intrinsic width, not because of the fragment. Also from review: - /docs/roadmap/ on the website is a 404; linked ROADMAP.md instead. - Added a project-documents table. The README linked one of the eight policy files in the repository root; governance, code of conduct, security policy and adopters are what a newcomer and a CNCF reviewer both look for first. - Added both Slack channels. The community section listed Telegram only, while the CNCF-hosted #cozystack channel went unmentioned. - Added the CLOMonitor badge, the standard CNCF project-health signal. It currently reports A. - semver.org was the one remaining plain-http link, which sat oddly next to dropping a badge for the same reason. - The commit-activity badge linked to the contributors graph. - 'free platform' reads as price now that the Open Source badge is gone. - 'CNCF Sandbox Level Project' is legacy phrasing. - Fixed an ungrammatical sentence in Contributions. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
34c7a20a21 |
docs(readme): stop the logo filling the page and thin the badge wall
The logo is an SVG with no intrinsic width, so GitHub rendered it at the full width of the README column -- 734x246 on a desktop viewport, a wall of logo before a single word of text. It also used the #gh-light-mode-only / #gh-dark-mode-only fragment hack, which GitHub has since superseded with <picture> and prefers-color-scheme. Replaced with a <picture> element at a fixed 340px, which both bounds the logo and drops the hack. The badge block had eight entries of ragged widths. Two carried no information: 'Open Source' says what the Apache-2.0 badge beside it already says, and a hand-set 'Status: Active' is a claim rather than a measurement -- the commit-activity badge next to it is the real thing, and the static one was also served over plain http. Both removed. The remaining six are ordered identity, then health, then support, and the licence badge now points at the LICENSE file rather than at opensource.org. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
Andrey Kolkov
|
caae13a60a |
docs(backups): drop the inaccurate NAMESPACE-override note from the demo runner
run-all.sh said NAMESPACE could be overridden through the environment, but the numbered manifests pin namespace: tenant-root while only the waits read $NAMESPACE, so an override would create resources in one namespace and wait in another. The demo is tenant-root-specific anyway: the system bucket credentials and the derived strategy live there. Say that instead. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
bd7d4abe84 |
fix(backups): size the Kafka metadata demo brokers at the chart default
The demo's source and to-copy Kafka fixtures requested resourcesPreset "small", the deprecated legacy alias worth 1 CPU / 512Mi — half the Kafka chart's own c1.small default (1 CPU / 1Gi). 512Mi is too little for the broker: it crashlooped before the HelmRelease went Ready, so the demo never reached the backup. Use c1.small on the kafka and zookeeper blocks of both fixtures, matching the chart default and the in-tree kafka e2e fixture that comes up cleanly. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
e3ab40eb83 |
fix(virtual-machine): guard Kube-OVN IP lookup (#4013)
## What this PR does Guards the Kube-OVN `IP` lookup behind API discovery. The `vm-instance` chart now renders on clusters using another CNI instead of trying to query a CRD that is not installed. The Helm regression suite mocks the IP object through `kubernetesProvider` and pins both sides: with `kubeovn.io/v1/IP` advertised the annotations carry the mocked MAC and IP, without it they are absent even though the object exists, which is the branch the guard adds. helm-unittest's fake lookup never errors on an unserved kind, so only the second case distinguishes the guard; it fails on main and passes here. Open PRs #3991, #3978, #3931, and #3900 touch the same template. Their current hunks do not alter this lookup; #3900 adds secondary-interface annotations immediately after it, so it is the only nearby merge hunk to watch. ### Screenshots Not applicable; there is no 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: ### Release note ```release-note fix(virtual-machine): render VM instances on clusters where the Kube-OVN IP API is not installed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM instances now render correctly in environments without the Kube-OVN IP capability. * OVN IP lookups and MAC/IP annotations are only generated when the required Kube-OVN API is available. * MAC/IP annotations are omitted when the capability is unavailable, even if an IP object exists. * **Tests** * Added coverage for both supported and unsupported Kube-OVN IP discovery scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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 --> |
||
|
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 ``` |
||
|
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 --> |
||
|
Aleksei Sviridkin
|
76c9ece4ab |
fix(opensearch): stop asking for a drain the cluster cannot perform
The chart asked the operator to drain data nodes before restarting them, whatever the replica count. Draining moves a node's shards to another data node and waits until none are left, and below three data nodes there is nowhere for them to go: with one the operator requeues forever and never deletes the pod, and with two it gives up on a full drain by itself, warning that some shards may not drain and then waiting for the system index primaries alone, which the surviving node cannot take while it holds their replicas. Such a release could not complete any rolling restart the operator drives: a version bump, a configuration change, or the move of the HTTP listener onto a cert-manager certificate. Drain only where a spare data node exists. Without draining, the operator pauses replica allocation for the restart and re-enables it once every pod runs the new revision, which is the path a small cluster can take. The predicate also reads the data role, since the operator counts data nodes over pools that carry it and a release with the role off has none. Every boundary is pinned in the unit tests. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
884f719b75 |
fix(opensearch): stop asking the operator for a privileged sysctl container
spec.general.setVMMaxMapCount makes the operator append a privileged init container to every OpenSearch pod, so that vm.max_map_count reaches the 262144 the store needs. Tenant namespaces run at the cluster's default PodSecurity level, and on Talos that is baseline, which refuses the container before a pod exists: the StatefulSet never produces one and the release never comes up. The opensearch-operator package now raises the sysctl on every node from a DaemonSet in its own namespace, which the platform labels privileged, so the pod no longer has to ask. The flag is set to an explicit false rather than dropped, and a unit test pins it, so the next reader sees a decision and not an omission. Carrying the value in the node contract would let the DaemonSet go; until then the DaemonSet is the carrier. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
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> |
||
|
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> |
||
|
Aleksei Sviridkin
|
84daf750d9 |
fix(opensearch): render the credentials Secret without platform values
Reading the cluster values through index aborts the whole release when they are absent: index on an untyped nil is a hard render error, not a miss returning nil, so the default written after it never runs. The platform always injects them, so this surfaces only where the chart is rendered on its own, which is where the unit suites and a bare helm template both live. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ae935db0dc |
feat(opensearch): project the CA into the tenant as a client trust anchor
A client inside the tenant needs the anchor that signs the HTTP chain, and the only copy lives in the cert-manager CA Secret, which stores the CA private key alongside it. Granting that Secret by name in the resource map would convey the key with it, and would keep conveying whatever later occupies the name after the platform has withdrawn the verdict, so the one revocation channel the anchor has would be bypassed. Project it instead: the controller copies ca.crt alone into a key-free Secret the tenant roles already reach through tenantsecrets, gated on the label the lineage webhook stamps. The private key never leaves the release namespace, and withdrawing the label withdraws the anchor. The projection is gated on the same condition as the certificates it follows, since its source Secret exists only under the cert-manager issuer; ungated it would sit SourceNotFound on every release the operator signs for. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |