1009 Commits

Include renames

Author	SHA1	Message	Date
IvanHunters	559c050de0	fix(cozypkg): pin tap short-name to the index version and harden the gate ... Conformance review against the design proposals surfaced that a short-name tap ignored the index entry's recorded version: resolveTapTarget returned the bare ociRef, so 'cozypkg tap <name>' pulled :latest while the CI gate had validated and cosign-verified <ociRef>:<version>. The gate signed one tag and the cluster pulled another, breaking both the versioning model (#18) and the signature chain (#23). Resolve the short name to <ociRef>:<version> when a version is recorded. Also harden the publication gate: run 'cozypkg validate --helm-lint' (with helm installed in the workflow) so charts are linted as the design specifies, and add a CODEOWNERS so every index entry requires maintainer review. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>	2026-09-07 10:58:37 +03:00
IvanHunters	1fbceee10e	fix(marketplace): operator RBAC for tap materialization and dot-safe artifact naming ... Second review round found the write-path RBAC fix was only half applied and the extracted naming helper diverged from its producer: - The tap materializer runs as the cozystack-controller ServiceAccount, whose ClusterRole granted source.toolkit.fluxcd.io/ocirepositories only get/list/watch. Its finalizer and materialized-revision writes (r.Update on the OCIRepository) were therefore Forbidden, so a dashboard-connected tap never materialized. Grant the controller update/patch on ocirepositories. - naming.ArtifactName replaced dots only in the PackageSource name, but the reconciler replaces them in the variant and component too, so a tapped repo with a dotted variant or component name dropped its packages from the catalog and tripped a false appdef-dangling warning. Replace dots in all three segments and add a direct parity test with dotted names. - Pin cosign-installer and the cosign release in the index gate so the signature verifier is reproducible. - Add a unit test for the tap rollback objectExists predicate. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>	2026-09-07 10:58:36 +03:00
IvanHunters	be03900262	refactor(marketplace): address review — shared naming, tap rollback, pinned gate, trust docs ... - Extract the per-component artifact-name convention into internal/marketplace/naming and have the validator and the Tap backend delegate to it, so the component-to-ApplicationDefinition linkage cannot drift across copies. - cozypkg tap only rolls back resources it actually created, so an idempotent re-tap no longer deletes a pre-existing OCIRepository/PackageSource when a later apply fails. - Pin cozypkg and yq in the index publication gate so the auto-merge decision runs on a reproducible toolchain. - Document the trust model explicitly: tap validates structure but does not verify signatures; verification is enforced by the index CI gate (pinned to the recorded cosign identity) and optionally by Flux OCIRepository verification at pull time. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>	2026-09-07 10:58:36 +03:00
IvanHunters	11e2f07fae	feat(packages-index): add community index gate template with two-lane merge ... Add a template for the standalone cozystack/packages-index repository: the metadata-only entry format (with a version and a recorded cosign signing identity), an example entry, and the publication gate. The gate script validates every changed entry with 'cozypkg validate --require-signature' against the entry's recorded identity and classifies the change into the auto-merge lane (a pure version bump of an already-listed entry) or the maintainer-review lane (a new entry or a change to ociRef, maintainer, or signing). It fails closed: a validation error, a parse error, a signature failure, or a security-relevant edit never auto-merges. A companion GitHub Actions workflow runs the gate and enables auto-merge or requests review accordingly. Add a version field to the index entry so a bump is detectable. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>	2026-09-07 10:58:36 +03:00
myasnikovdaniil	41f92ec27a	fix(release): compare host-less image refs on digest alone (#3908) ... Ports #3905 to `main`. Same fix, same tests, two cherry-picks, no conflicts. `main`'s copy of `hack/verify-promoted-packages.sh` was byte-identical to the buggy one, so the next promotion cut from here fails the way v1.6.2 just did: `normalized_refs()` gives a host-less ref a repository equal to its own tag, because `repo="${image%:*}"` is a no-op on a bare `tag@sha256:...` scalar with neither `/` nor `:` in it. `hack/lib/image-refs.sh` emits one of those per shape-3 split map, promotion rewrites exactly those tags, and the check then reports changed repositories at identical digests. It cost several rounds on v1.6.2 before the cause was clear, so it is worth not paying twice. Checked against `main`'s own tree rather than assumed from the 1.6 port. 13 host-less refs, same count as `release-1.6`: 12 pair one-to-one with a host-bearing ref at an identical digest, and the 13th is `packages/system/kuberture/values.yaml` (`tag:` only, no `repository:`, no `registry:`), which is genuinely digest-only and is the case the comment says must not be dropped. The narrowing the 1.6 review left open still does not exist here. Of 14 repository-bearing digest maps, exactly one has a slash-free `repository:` — kube-ovn's `global.images.kubeovn` — and `global.registry.address` gives it a shape-4 join that keeps its repository in the compared set. No map anywhere has an empty or non-string `registry:` sibling. `hack/lib/image-refs.sh` differs from `release-1.6`'s in comments only: 47 code lines either side and an empty diff once comments are stripped, same globs and same selectors, so the refs reaching the verifier are unchanged. Not modified here. Red phase checked both directions rather than inherited. Reverting the one line reddens the new split-map case on the digest-set assertion. Replacing `repo=""` with an outright `continue` for host-less refs flips which case fails: the split-map one goes green and `rejects a changed digest carried only by a repository-less image map` reddens at its exit-status assertion, because the verifier then exits 0 on a genuinely changed digest. That second direction is the one worth having a test for, since dropping host-less refs is the obvious-looking fix. Gate: the four promote bats suites individually green, `shellcheck -x --shell=sh` clean, `pre-commit` green. No Go, charts or generated files, so build, vet, test and `make generate` do not apply. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved promoted package validation for image references without explicit registries or repositories. * Promotion-only tag changes are now accepted when the image digest remains unchanged. * Digest changes continue to be detected and rejected correctly. * **Tests** * Added coverage for split image reference formats and repository-less image mappings. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-05 14:53:51 +05:00
Aleksei Sviridkin	3a369fed99	feat(redis): add opt-in TLS for Redis and Sentinel (#2729) ... ## What this PR does `tls.enabled` puts Redis and Sentinel on a TLS-only listener with a per-release cert-manager chain, and hands the tenant the CA certificate as `redis-<name>.tenant-ca`. Off unless set. redis-operator v3.3.5 has no TLS support. The work is ported upstream in freshworks-oss/redis-operator#85, which is open and not in a release, so this carries it as three patches over the v3.3.5 tarball. `images/redis-operator/patches/tls.diff` is the source half, cut from that PR's commit `2ad2ed8d1`, which the patch header pins; it is applied by the glob the image Dockerfile already runs, after the Dockerfile strips the tarball's `manifests`, `charts`, `example` and `.github`, so the image builds from source alone and the upstream tests that read the CRD YAML skip instead of failing against the stale v3.3.5 copy. `patches/crd-tls.diff` adds `spec.tls` to the vendored CRD, and with it the CEL rule that fixes `tls.enabled` at creation; without the schema block the apiserver prunes the field on admission and the patched binary waits for something it never sees, and the rule cannot exist without the block it references. `patches/rbac-tls.diff` grants Create, Get and Update on Secrets, so the operator can publish the CA-only Secret it addresses by name. That is the whole delta, and it is narrower than the ported operator's own chart asks for: those three are the only Secret calls in the patched source, and cert-manager Certificates get no grant at all, because the operator only touches one under `spec.tls.certManager` and this chart pins `certificateSecret` mode instead. One change in `tls.diff` is not TLS and takes effect with TLS off, named in the patch header: `MakeSlaveOfWithPort` connects to the replica on its own port rather than on the master's. It changes a dial, not a pod template, so shipping the rebuilt image rolls nothing; the plaintext pod templates stay byte-identical to v3.3.5. The image build registration this branch used to carry (`make -C packages/system/redis-operator image` in the root `build:` target) landed on main with the Valkey service (#3380), so the rebuilt operator image is produced by CI from main as well; the digest in `values.yaml` is still refreshed by release preparation, not here. The chart patches re-apply at the end of `update:`, the same way reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji carry theirs. Otherwise `rm -rf charts` takes TLS back out on the next `make update` and nothing fails. `make update` reproduces the committed tree byte for byte. TLS is chosen when the instance is created. The config gets `port 0` and the listener moves to `tls-port`, so TLS replaces plaintext rather than running beside it, and a running instance cannot change mode: a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The RedisFailover schema in the carried CRD patch rejects a change of `tls.enabled`, which on an existing instance surfaces as a failed HelmRelease upgrade while the instance keeps serving what it did; the README points at creating a new instance and migrating. That is also why TLS is not inferred from `external`: an externally reachable instance is the one most likely to have clients an upgrade must not drop. Existing releases keep their plaintext listener and are not restarted by the rebuilt operator image. The tenant gets `ca.crt` and nothing else, through `tenantsecrets`. The chart's own Secrets take a dot between the release and the suffix (`<release>.ca-tls`, `<release>.tls`, `<release>.ca-cert`): the suffixes are dot-free, so the split is unique and release `foo`'s CA Secret can no longer be release `foo-ca`'s leaf, the collision the hyphenated form had. The chart renders a `TenantProjection` pointing at `<release>.ca-tls`, and the CA-extraction controller re-encodes the one named key from the parsed certificate, so nothing that is not a certificate gets through. Same path as postgres, mongodb, nats and qdrant. No Secret of this release is granted to the tenant by name beyond the generated password, and that includes `<release>.ca-cert`, the CA-only copy the operator publishes for itself. It holds no key today, but the operator has Create and Update on Secrets cluster-wide, so what sits at that name is the operator's call rather than the platform's, and RBAC cannot filter by key. `hack/check-redis-rd-secrets.bats` pins the whole rendered Secret surface instead of a list of known-bad names, because an include with no `resourceNames` and no `matchLabels` restricts nothing. The chainsaw suite for redis gains two cases. A TLS fixture asserts the cert-manager chain is issued, the TLS-probed Redis replicas and Sentinel come up, the operator publishes its CA-only Secret under the widened ClusterRole, the tenant receives exactly `ca.crt` through `tenantsecrets` while the key-bearing Secrets stay off that path, and a `redis-cli --tls` PING through the service name completes with the same CA. A second case applies `tls.enabled` to a running plaintext release and asserts the refusal: the HelmRelease upgrade fails with the schema's message, the StatefulSet keeps its revision with both replicas Ready, plaintext still answers and no TLS listener appears. Certificate renewal rolls the pods from inside the operator. Redis and Sentinel read the certificate once at startup, so the patched operator stamps a hash of the TLS Secret's `tls.crt` and `ca.crt` into the pod template of both the Redis StatefulSet and the Sentinel Deployment on every reconcile, as `redis-failover.freshworks.com/tls-secret-hash`. A renewal changes the hash; the operator's own roller replaces the Redis pods one at a time and the Deployment controller rolls Sentinel. README names the fallback: if the roll cannot run, the deadline is the old certificate's expiry, past which the operator's own TLS connections fail along with the clients. `tls.authClients` maps to `tls-auth-clients` and defaults to `no`. The platform mints no client certificate for the tenant and the CA key is out of reach, so `yes` is only usable when the client certificate comes from whoever also controls the CA. Changing the value on a running release restarts the Redis and Sentinel pods: Redis reads the directive from its config file only at startup, so the operator folds the effective value into the certificate hash that drives the rollout, and the change lands when it is made rather than at the next unrelated restart. Follow-ups in the carried operator patch go upstream first, to freshworks-oss/redis-operator#85 or a successor there, and are picked up here afterwards; the carried patch does not grow Go that upstream has not seen. `tlsConfigFor` fetches the Secret on every call, tens of GETs per reconcile. The renewal roll is static analysis so far: the live check, `cmctl renew` on a running release while watching the `rfr-*` and `rfs-*` pods roll, is still owed and belongs with the e2e follow-up. Two facts about the shipped image rather than the patch. `packages/system/redis-operator/values.yaml` keeps the digest of the image built before the patch until release preparation rebuilds it, so on `main` between this merge and the next release-prep, `tls.enabled` renders the chain and `spec.tls` while the running operator ignores them and serves plaintext. And the pinned v3.3.5 tarball carries GO-2026-5026 in `golang.org/x/net` v0.53.0, reached through `PodService.UpdatePodLabels`; the port neither introduces nor fixes it, upstream `master` is already on v0.55.0, so it lands with the next tag bump rather than here. Outside the patch: `cozyvalues-gen` renders `{}` in the README Value column for `tls.authClients`: a child property without a default inherits the parent object's `{}`, and `make generate` reproduces it byte for byte, so it is a generator bug rather than something to hand-edit here. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#37 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: I walked the trigger map against the diff file by file. terraform-provider-cozystack is reached, tracked as cozystack/terraform-provider-cozystack#37 rather than a speculative PR: `values.schema.json` gains a `tls` object, and the provider hand-writes a schema, model and expand/flatten pair per Kind. It already models `tls` for postgres, kafka, nats and qdrant, not for redis. Redis needs different wording from those four, whose descriptions say to omit the block to follow `external`, which is the one thing redis does not do. Nothing else is reached. The website regenerates this package's reference page from `README.md` on a stable tag, so the README changes need no PR there. No package added, renamed or removed. `packages/core/platform/values.yaml`, variant and bundle composition and release asset names are untouched, and so are `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh`, which is what ccp and external-apps-example read. The two new files under `hack/` are bats unit tests. No node prerequisite, installer value, telemetry metric or cozy-proxy label changes. ### Release note ```release-note feat(redis): add opt-in TLS for Redis and Sentinel. Set `tls.enabled: true` to serve both on a TLS-only listener backed by a per-release cert-manager chain; the tenant reads the CA certificate as the `<release>.tenant-ca` tenant secret. TLS is off unless set and is not inferred from `external`, so existing releases keep serving plaintext and are not restarted by the rebuilt operator image. TLS is chosen at creation: a running instance cannot switch modes, the change is rejected and surfaces as a failed HelmRelease upgrade, so add TLS by creating a new instance and migrating to it. Requires cert-manager, which the platform's `system` bundle installs on every variant. A renewed certificate rolls the Redis and Sentinel pods through the operator. TLS support is carried as a downstream patch over redis-operator v3.3.5 until freshworks-oss/redis-operator#85 lands in a release. ```	2026-09-04 19:32:32 +04:00
Aleksei Sviridkin	d800526696	feat(mariadb): add TLS support via cert-manager (#2680) ... ## What this PR does Gives MariaDB a chart-managed certificate chain, makes TLS enforcement explicit, and hands the tenant a key-free trust anchor instead of a private key. - `tls.issuer` selects who issues the server certificate: `operator` (the CA mariadb-operator manages itself, the default and today's behaviour) or `cert-manager` (the instance gets its own CA, and this is what covers the external hostname). The field is named for the issuer rather than for on/off because TLS is served under both — unlike the similarly spelled field in some other charts, which really is a switch. - `tls.required` refuses plaintext connections. It is opt-in and defaults to false: enforcing TLS before the tenant can obtain the CA would be backwards, and a platform upgrade must not sever connections that work today. - Neither issuer is inferred from `external`. Existing instances keep the CA they have and change nothing on upgrade. - The tenant no longer receives `<release>-ca-tls` or `<release>-tls` — the CA private key and the server private key. It receives the operator's `ca.crt`-only bundle, which works today, and the ResourceDefinition additionally declares that bundle as the source for the CA-extraction controller. ### Fixes along the way - The TLS backup path could never have worked: it passed `--ssl-mode=VERIFY_IDENTITY`, a MySQL-8 flag the MariaDB client rejects during argument parsing. Replaced with `--ssl-ca` + `--ssl-verify-server-cert`, and the test now asserts the dialect rather than the presence of a substring. - The leaf certificate carried `clientAuth`. Because `REQUIRE X509` accepts any certificate signed by a trusted CA, the server's own key was usable as a client credential. Removed. - The CA and server private keys survived deletion of the instance: cert-manager runs with `enableCertificateOwnerRef: false`, so removing the `Certificate` left the Secrets behind. Both are now cleaned up by the existing post-delete hook. - The Secrets are labelled for the operator's watch predicate, without which a renewed leaf would not reach the pods until a full resync. ### Why the include carries an exclude list The trust-anchor selector matches by label with no name constraint, so the enumerable dangerous set is excluded explicitly — both issuers' private keys, the internal credentials, the backup keys, and the metrics configuration, which the operator writes unconditionally with credentials in plain text for a user holding `SELECT` on everything. The backstop is bounded and the comment says so: matching is by exact name with no globs, so a Secret introduced by a future operator version, or a transient object created mid-renewal, stays in scope of the label-only include. The primary defence remains that the controller stamps only its own projection. ## Known limitations 1. **Merge is blocked on #3299 — and the blocking is mechanical, not just procedural.** The CRD on `main` does not carry `spec.caCert`, and under Flux's Server-Side Apply the API server rejects a field the schema does not declare (`field not declared in schema`); under the older client-side apply the same field was silently dropped, so installing this branch without #3299 fails outright. An earlier revision of this description claimed the field would be silently pruned; that was wrong. The tenant still gets a usable trust anchor once merged in order, through the operator's bundle. 2. **The deprecated backup CronJob cannot start at all**, for reasons that predate this branch: its `secretKeyRef` names a Secret and key the chart never renders. That is filed separately as #3347 and deliberately not fixed here, since repairing it would start scheduled dumps on production instances from a TLS change. The tests that pin the backup TLS flags therefore pin a path that does not currently execute — the suite header says so. 3. **The operator's `clusterName` is hardcoded**, so on a cluster with a non-default domain the SANs and the name the operator dials diverge. Pre-existing, raised separately. 4. **No e2e coverage for the managed path.** Unit coverage pins the rendered chain, the SAN set, the exclusion set and the guards; the end-to-end handshake is not asserted in CI. ### On review coverage Two independent review passes were run repeatedly against this branch — seven rounds, converging from a security finding, through comment accuracy, to guard strength, and finally to nothing new. A third, model-based pass reviewed the branch through `6cb1bd236` and its findings were fixed; it could not be run against the final revision because of an external account quota lasting until 25 July. The delta since that revision is substantial and includes runtime code, not only tests and docs. Recording this so the coverage is not overstated: if the quota returns before this is ready to merge, that pass is worth running again. ### Downstream repositories `packages/apps/mariadb/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_mariadb`; tracked in cozystack/terraform-provider-cozystack#40. ### Release note ```release-note feat(mariadb): chart-managed TLS via cert-manager, opt-in enforcement, and the CA published to tenants as a key-free trust anchor ```	2026-09-04 19:32:16 +04:00
Aleksei Sviridkin	a2cb47c961	feat(gateway): update the CRD bundle to Gateway API v1.6.1 and gate GRPCRoute hostnames (#3872) ... ## What this PR does Bumps the vendored Gateway API CRD bundle in `packages/system/gateway-api-crds` from v1.5.1 to v1.6.1. The package Makefile pins the upstream ref, so the change is that pin plus the re-rendered `templates/crds-experimental.yaml`. The bundle version is not only bookkeeping: it is a compatibility signal consumers read at runtime. Every CRD in the bundle carries a `gateway.networking.k8s.io/bundle-version` annotation, and a Gateway API controller can read it off the `gatewayclasses` CRD to decide whether the installed bundle matches the one it was built against. Controllers that do this typically accept any patch release but require an exact major and minor match, and report `SupportedVersion=False` with reason `UnsupportedVersion` on their GatewayClass when the minor differs, a status condition that surfaces the skew rather than a functional break. The Cloudflare Tunnel Gateway API controller proposed for the platform in #3858 is built against Gateway API v1.6.1 and performs exactly that comparison, so against the v1.5.1 bundle shipped today its GatewayClass reports the bundle unsupported, and with this bump it reports supported. Neither change depends on the other landing first: this PR only moves the bundle, and the check lives entirely on the consumer side. One consequence worth stating plainly, since it will outlive this PR: an exact-minor check makes the shipped bundle version a coupled pin for any such controller. A future bump to v1.7.x flips the same condition back to `UnsupportedVersion` until that controller is rebuilt against the newer bundle, so the two want to move together. The bundle still emits the same twelve CustomResourceDefinitions and the same two `safe-upgrades.gateway.networking.k8s.io` admission objects, and every CRD still carries the `helm.sh/resource-policy: keep` stamp the Makefile's post-processing step adds. One of the policy's two CEL rules is rewritten, though, and it changes what a cluster accepts. That has its own section below. Two kinds change their version surface: TCPRoute and UDPRoute gain a served `v1` that takes over as the storage version, and their `v1alpha2` stays served with a deprecation marker. Every other kind keeps exactly the versions it served at v1.5.1. That matters because the Cilium operator resolves GatewayClass, Gateway, HTTPRoute and GRPCRoute in `v1`, ReferenceGrant in `v1beta1` and TLSRoute in `v1alpha2` before it enables its Gateway API controller; all six are still served by the v1.6.1 bundle. TLSRoute in particular keeps `v1alpha2`, which the tenant gateway controller, the platform TLSRoute templates and the tenant hostname admission policy all address. Schema changes are small and land on fields nothing in this repository sets. `sessionPersistence.idleTimeout` is gone from HTTPRoute, GRPCRoute and XBackendTrafficPolicy, removed upstream under GEP-1619 because no implementation ever implemented it and HTTP cookies have no native idle-timeout mechanism, so no working behaviour is lost even though the field disappears from the schema; HTTPRoute gains a lower bound on `retry.attempts` and now rejects duplicate `retry.codes`; ReferenceGrant requires `spec` at the root. In the other direction, Gateway infrastructure annotations and frontend CA certificate references go from a limit of 8 to 16, and TLSRoute hostnames from 16 to 1024. No CRD schema loses or changes a CEL validation rule, and the six added belong exclusively to the new TCPRoute and UDPRoute `v1` schemas. ### The safe-upgrades admission policy accepts a different set of bundles The experimental kustomization ships a `ValidatingAdmissionPolicy` and its binding, both named `safe-upgrades.gateway.networking.k8s.io`, next to the twelve CRDs. The binding is `validationActions: [Deny]` with `failurePolicy: Fail`, matching CREATE and UPDATE on `apiextensions.k8s.io/v1 customresourcedefinitions` cluster-wide, and Cozystack installs it in two places: on the host through `cozystack.gateway-api-crds`, and inside every guest cluster that sets `addons.gatewayAPI.enabled`. Its second rule reads the `gateway.networking.k8s.io/bundle-version` annotation off an incoming CRD. At v1.5.1 that rule was a blacklist: reject anything matching `v1.[0-4].\d+` or containing `v0`, allow the rest. At v1.6.1 it is a whitelist: allow `v0.0.0-dev` outright, otherwise require the annotation to start with `v1.` and not match `^v1\.[0-4](\.|$)`. Evaluating both expressions over a spread of annotation values: | bundle-version | v1.5.1 policy | v1.6.1 policy | | --- | --- | --- | | `v1.6.1`, `v1.5.1`, `v1.5.0-dev`, `v1.10.0` | allow | allow | | `v1.4.0`, `v1.4.10`, `v1.0.0` | reject | reject | | `v0.0.0-dev` | reject | **allow** | | `v2.0.0` | allow | **reject** | | `1.6.1` (no leading `v`) | allow | **reject** | | `v1.4` (no patch) | allow | **reject** | | `main`, empty string | allow | **reject** | Nothing in this repository trips the new rejections: every CRD the bundle ships is stamped `v1.6.1`. What is exposed is an operator applying a Gateway API CRD set from somewhere else on a cluster that carries the policy. A fork, a vendor build or a nightly whose annotation reads `1.6.1` or `v2.0.0` used to go in and is now denied with `reason: Invalid`, naming a policy the operator may not know is installed, and whatever HelmRelease owns those CRDs then sits un-Ready. Uninstalling the policy is the escape hatch, and the denial message says so. The same hunk fixes an upstream inconsistency: at v1.5.1 the policy pair was stamped `v1.5.0-dev` while its twelve CRDs said `v1.5.1`. At v1.6.1 all fourteen documents agree. The Gateway and HTTPRoute objects the tenant gateway controller renders were serialized across all three certificate modes and checked against both the v1.5.1 and the v1.6.1 schemas; the two runs are identical, so the bump does not change what the controller can create. `go.mod` still pins `sigs.k8s.io/gateway-api` v1.4.1 and this PR deliberately leaves it there. The CRD bundle and the Go client types have been on separate versions since before this change, and nothing the controller emits depends on the gap. One thing to know before the next bump: from v1.6 on, the two things upstream calls the experimental bundle disagree. This package follows `config/crd/experimental`, the kustomization, which emits 12 CRDs at v1.6.1; the published `experimental-install.yaml` for the same tag emits 13, the extra one being `xbackends.gateway.networking.x-k8s.io`, new in v1.6 and not listed in the kustomization. At v1.5.1 the two agreed, so there was nothing to choose between. Nothing in this repository references XBackend and no vendored CRD depends on it, so following the kustomization costs nothing today, but whoever bumps next inherits the choice, and it should be made deliberately rather than by whichever command gets typed. The second commit corrects a comment this bump invalidated. It claimed TLSRoute stays at `v1alpha2` through Gateway API v1.5 and that the promotion to `v1` was still upstream-only; TLSRoute has been served at `v1` since v1.5.0, so the claim was already wrong before this bump and the version range it named is now wrong too. The third commit closes a gap that correction exposed. `cozystack-route-hostname-policy-tls` named `v1alpha2` alone while the bundle serves `tlsroutes` at `v1`, `v1alpha2` and `v1alpha3`: three served versions against one named. The gap is not new and this bump neither creates nor widens it: the v1.5.1 bundle and the v1.6.1 one serve the same three versions, with the same two marked deprecated. Coverage of the two unnamed versions rested entirely on request conversion, because `matchPolicy` is unset on both rules in that file and so defaults to `Equivalent`, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by `failurePolicy: Fail`, because nothing failed. The rule now names all three, which is the shape the HTTPRoute rule above it already used, and `hack/route-hostname-policy-version-coverage.bats` reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, so a future bundle move cannot outrun the list silently, in either direction. That guard carries negative fixtures rather than only watching a correct tree pass. Three of them drive `assert_covers` itself, with the two readers redefined in a subshell, against an omitted served version, an empty served set and an empty named set. Each requires the specific complaint text rather than merely a non-zero exit, so a helper renamed out from under it fails for the right reason instead of reading as a successful rejection. A fourth drives the comparison helper directly. The fourth commit pins the vendored bundle version in the CRD package's unit suite. Nothing outside the package Makefile named a Gateway API version, so re-running `make update` against a different ref landed green: twelve CRDs, every one stamped, document count unchanged, suite passing. The new case reads the `gateway.networking.k8s.io/bundle-version` annotation off every rendered CRD and fails when it is not the version the Makefile pins. The fifth commit extends the version-coverage guard to `cozystack-gateway-hostname-policy`, which names the `gateways` versions literally the same way the two route policies do, and whose version set lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Two notes for whoever reads the release: the TCPRoute and UDPRoute storage version moves from `v1alpha2` to `v1`, and while nothing in this repository ships either kind and the CRDs declare no conversion block, an upgraded cluster accumulates both entries in `status.storedVersions` — inert today, load-bearing only if a later bundle stops serving `v1alpha2`. The sixth commit gives GRPCRoute the same hostname rule the other two route kinds have. GRPCRoute carries `spec.hostnames` and it can reach a tenant Gateway: the port-443 listeners pin `allowedRoutes.kinds` to HTTPRoute and TLSRoute, but the port-80 listener leaves `kinds` unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could claim any hostname on the plaintext listener with nothing checking it. Tenants hold no `gateway.networking.k8s.io` RBAC in Cozystack, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The rule is the HTTPRoute rule with `grpcroutes` in place of `httproutes` and the one version the bundle serves for it, and the coverage guard pins that list to the bundle like the others. Several places in the tree enumerate which route kinds the hostname policies cover, and all of them named two: the port-443 kinds invariant in the tenant gateway controller and the test that pins it, `packages/extra/gateway/README.md`, and the threat model together with the self-assessment. They name three now. The port-443 kinds set also carries an accurate reason now. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. The README had also described HTTPS and TLS-passthrough listeners as carrying different kinds, where the controller gives both the same one. One more commit makes the suite's existing assertions bite. A ValidatingAdmissionPolicyBinding whose `policyName` points at another policy renders without complaint and leaves its own policy inert, and the suite pinned `validationActions` on the bindings while never pinning `policyName` at all, so pointing the GRPCRoute binding at the HTTPRoute policy kept every case green with the GRPCRoute control doing nothing. All three bindings now pin the policy they name. In the same file, `uncovered()` assigned to the variable names its caller reads back when printing a mismatch, so a call site with the arguments swapped would have printed a diagnostic naming the wrong side. Two things are named rather than made. The version-coverage guard collects `apiVersions` from every `resourceRules` entry mentioning the resource without also matching on `apiGroups` or `operations`, so a rule in another group, or one scoped to a single verb, would donate its versions to the set coverage is measured against; the chart's own suite pins the group and the resource on the rule it checks, which closes that a layer below. And `packages/system/cozystack-api/templates/api-tlsroute.yaml` renders `gateway.networking.k8s.io/v1alpha2` while the bundle serves `v1` as storage and marks `v1alpha2` deprecated, but the identical pin sits in the kubevirt and kubevirt-cdi TLSRoute templates, and the pinned `sigs.k8s.io/gateway-api` carries no `apis/v1` TLSRoute type for the controller side to follow, so moving one of the three here would leave the tree less consistent than leaving all three. Closes #3789. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file: neither `packages/system/gateway-api-crds/Makefile` nor its rendered CRD template matches any entry. Leaving every box empty rather than ticking the "no downstream repository" line, because the docs site carries a Gateway API version statement that is already inaccurate and needs a decision that is not mine: `content/en/docs/next/networking/gateway-api.md` and its v1.5 and v1.6 copies say Gateway API v1.5 ships TLSRoute at `v1alpha2` and that the graduation to `v1` is still upstream-only, when v1.5.0 already shipped TLSRoute `v1` as the storage version. ### Release note ```release-note chore(gateway-api-crds): update the vendored Gateway API CRD bundle to v1.6.1. TCPRoute and UDPRoute gain a served v1 that becomes their storage version, with v1alpha2 still served and now deprecated; every other kind keeps the versions it served at v1.5.1. Upstream removed `sessionPersistence.idleTimeout` from HTTPRoute, GRPCRoute and XBackendTrafficPolicy; the field is not set by anything Cozystack renders, but a hand-written route that carries it loses it silently on its next write, with no admission error. The bundle's `safe-upgrades.gateway.networking.k8s.io` ValidatingAdmissionPolicy changes which CRDs it accepts. Cozystack installs it cluster-wide on the host and in every guest cluster with `addons.gatewayAPI.enabled`. Its `gateway.networking.k8s.io/bundle-version` rule was a blacklist on two patterns and is now a whitelist on the `v1.` prefix with a `v0.0.0-dev` escape hatch. A bundle annotated `v0.0.0-dev` is now accepted where it was denied. A bundle annotated `v2.0.0`, `1.6.1` without the leading `v`, `main`, `v1.4` without a patch, or carrying an empty annotation is now denied where it was accepted. Nothing Cozystack ships is affected, since every CRD in the bundle is stamped `v1.6.1`, but applying a Gateway API CRD set from a fork, a vendor build or a nightly on such a cluster can now fail admission with `reason: Invalid`. Uninstalling the policy remains the documented escape hatch. fix(cozystack-basics): the tenant TLSRoute hostname admission policy now names every served TLSRoute version (`v1`, `v1alpha2`, `v1alpha3`) instead of `v1alpha2` alone. Coverage of the other two rested on request conversion, which holds only while a named version stays served, and a rule left selecting nothing is not caught by `failurePolicy: Fail`. feat(cozystack-basics): GRPCRoute hostnames in `tenant-*` namespaces are now gated by the same admission policy that gates HTTPRoute and TLSRoute. A tenant Gateway's port-80 listener leaves `allowedRoutes.kinds` unset, which admits GRPCRoute by protocol default, so a GRPCRoute could claim any hostname on the plaintext listener unchecked. Tenants hold no `gateway.networking.k8s.io` RBAC, so this is defence-in-depth against a chart bug rather than a tenant-user control. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added hostname admission validation for `GRPCRoute` resources. - Expanded `TLSRoute` hostname validation to support all served Gateway API versions. - Ensured route hostname policies consistently cover HTTP, TLS, and gRPC routes. - **Security** - Updated listener policies to restrict port-443 routes to supported HTTP and TLS route types. - Clarified fail-closed hostname validation behavior across Gateway, Route, and Ingress resources. - **Documentation** - Updated security and threat-model documentation to include GRPCRoute handling and validation coverage. - **Maintenance** - Updated the bundled Gateway API CRDs to version 1.6.1. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-04 15:23:36 +04:00
Aleksei Sviridkin	19eaff6a30	test(cozystack-basics): pin which policy each route binding names ... A ValidatingAdmissionPolicyBinding whose policyName points at another policy renders without complaint and leaves its own policy inert. The suite pinned validationActions on the bindings but never policyName, so pointing the GRPCRoute binding at the HTTPRoute policy kept all 56 cases green while the GRPCRoute control did nothing. uncovered() assigned to the same variable names its caller reads back when printing the mismatch, so a future call site with the arguments swapped would print a diagnostic naming the wrong side. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 22:17:29 +03:00
Aleksei Sviridkin	70a675fd6b	feat(cozystack-basics): gate GRPCRoute hostnames in tenant namespaces ... The route hostname policy covered HTTPRoute and TLSRoute. GRPCRoute carries spec.hostnames too, and it can reach a tenant Gateway: the port-443 listeners pin allowedRoutes.kinds to HTTPRoute and TLSRoute, but the port-80 listener leaves kinds unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could therefore claim any hostname on the plaintext listener with nothing checking it. Tenants hold no gateway.networking.k8s.io RBAC, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The new rule is the HTTPRoute rule with grpcroutes in place of httproutes and the single served version the bundle provides; the coverage guard pins that list to the bundle like the others. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 22:17:28 +03:00
Aleksei Sviridkin	6ddae65b45	test(cozystack-basics): cover the Gateway listener policy in the guard ... cozystack-gateway-hostname-policy names the gateways versions literally, the same way the two route policies do, and the set it must cover lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 22:17:04 +03:00
Aleksei Sviridkin	9311a48f6a	test(cozystack-basics): make the version-coverage guard prove it still compares ... The guard added alongside the TLSRoute version fix only ever ran against a tree that already satisfied it. A passing check that is checking nothing looks exactly like a passing check that is checking something, so a later refactor could empty out the comparison and the suite would stay green forever. That is the same shape as the negated assertions that cannot fail elsewhere in this repository: the assertion survives, the guarantee does not. Drive the checker with synthetic input instead of only with the live tree, the way hack/bats-no-exit-trap.bats and hack/md-no-hardwrap.bats feed their checkers a broken sample and require a complaint. Two layers need it separately. The comparison itself moves into uncovered() so a fixture can call it with a served set the named set does not cover. Above that, assert_covers runs in a subshell with both readers replaced, so the wiring, the empty-set guards and the exit status are exercised too -- covering only the inner function leaves the outer one free to become a no-op while every other case in the file stays green, because the live-tree cases are its only callers and the tree they read is correct. Each fixture requires the complaint it expects rather than any non-zero exit. A renamed helper exits 127 and an unbound variable under set -u exits 1, and either would read as a successful rejection if only the status were checked; a rejection for the wrong reason is the same false comfort as no fixture, and harder to notice because the red looks earned. Each way of breaking the guard now fails at its own case. Emptying uncovered(), reducing assert_covers to a no-op, and deleting the empty-set guards each turn a different fixture red while the live-tree cases stay green, and changing only the wording of assert_covers' complaint -- which still rejects, just for an unstated reason -- turns the omission fixture red on the message rather than passing on the exit status. Select a resourceRules entry by comparing each resource name for equality, and match versions as fixed strings rather than patterns. yq's contains() compares array elements by substring, so a rule scoped to a subresource would have satisfied a search for the parent: a rule naming tlsroutes/status answers a query for tlsroutes and donates its apiVersions to the set the guard checks coverage against. That direction is the wrong one for a fail-closed check -- it makes coverage succeed where the real rule falls short. No rule in this policy is scoped that way today. Also pin apiGroups on the TLSRoute document in the chart suite. The HTTPRoute document has always had that assertion; the asymmetry meant the case named after both kinds only checked the group for one of them. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>	2026-09-03 22:17:04 +03:00
Aleksei Sviridkin	7ba22369d4	fix(cozystack-basics): match every served TLSRoute version in the hostname policy ... The tenant route hostname policy names v1alpha2 alone while the vendored Gateway API bundle serves tlsroutes at v1, v1alpha2 and v1alpha3: three served versions against one named. The gap is not new. The v1.5.1 bundle this package shipped before and the v1.6.1 one it ships now serve the same three versions, with the same two marked deprecated, so nothing in this change created or widened it -- a bundle bump is simply when someone last opened the file. Coverage of the two unnamed versions rested entirely on request conversion. matchConstraints.matchPolicy is unset on both rules in this file and so defaults to Equivalent, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by failurePolicy: Fail, because nothing failed -- there was simply no match. Name all three served versions, which is the shape the HTTPRoute rule directly above already uses. The defect is a version list drifting from the bundle it must track, so a longer list on its own would not catch the next drift: add a check that reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, in either direction. Both rules get an exact-value assertion in the chart's own suite, so the coverage check and the pinned value fail for different reasons and neither stands alone. The header comment being replaced claimed TLSRoute stays at v1alpha2 through Gateway API v1.4-v1.5 and that its promotion to v1 was still upstream-only. TLSRoute has been served at v1 since v1.5.0, so that was already false, and false in the direction that hid this gap: a reader who believed it had no reason to look at the version list at all. Widening the rule then falsified every remaining place that restated which versions it matches -- the template's own header, the security-model layer list in packages/extra/gateway/README.md, and the name of the test case that asserts the value. Rather than resync three copies of a list that will move again, they now say which kinds are gated and point at the rule and its guard for the versions. A comment in the same test file now states only the behaviour it guards: an earlier form of the CEL used `... ? true : (...)`, which allowed routes in any tenant-* namespace whose host label was missing or scrubbed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>	2026-09-03 22:17:01 +03:00
Aleksei Sviridkin	4758718d66	fix(mariadb): separate the chart-chosen Secret suffixes with a dot ... The chart asks cert-manager for two Secrets and names them after the release: "<release>-ca-tls" for the CA and "<release>-tls" for the leaf. The release prefix is "mariadb-", so an application named foo produces mariadb-foo-ca-tls, and an application named foo-ca produces mariadb-foo-ca-tls as well. One release's CA private key and another release's server private key claim the same object. Two sibling applications in one tenant namespace are enough to hit it: whichever cert-manager writes last owns the Secret, and each instance then serves or trusts material issued for the other. The suffix set is what makes it ambiguous. It holds both "-tls" and "-ca-tls", and one is a tail of the other, so a hyphen leaves nothing to say where the release name ends. A dot does: an application name is validated as a DNS-1035 label and the release prefix as ^[a-z0-9-]*$, so neither can contain one, and "<release>.ca-tls" and "<release>.tls" split uniquely. Secret names are DNS-1123 subdomains, which admit the dot. Only the two suffixes this chart chooses move. The Certificate and Issuer object names keep the hyphen, because their suffixes are not a pair where one is a tail of the other, and the operator-managed "<release>-ca-bundle" that the trust-anchor projection reads is not this chart's to rename. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 20:13:17 +03:00
Andrey Kolkov	e94fcf8219	feat(backups): RabbitMQ definitions backup via a dedicated strategy driver (#3990) ... ## What this PR does Adds backup/restore support for the managed `RabbitMQ` application. RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no consistent online export of message payloads, so this backs up broker **definitions** (vhosts, users, permissions, queues, exchanges, bindings, policies, parameters) over the management HTTP API (`GET`/`POST /api/definitions`) and stores them in the system bucket. Message payloads and point-in-time recovery are intentionally out of scope; a full-data backup would be a Velero volume snapshot, which is a separate strategy. It is delivered as a **first-class `strategy.backups.cozystack.io/Rabbitmq` driver**, following the Altinity pattern (a dedicated driver that renders a one-shot Job for an HTTP-API engine): - `reconcileRabbitmq` / `reconcileRabbitmqRestore` render the strategy's `PodTemplateSpec` — `curl` `GET`/`POST /api/definitions`, pushing/pulling the JSON to S3 with `curl --aws-sigv4` — as a `batch/v1.Job`, covering both restore flows: in-place (into the source broker) and to-copy (into a separate, differently-named RabbitMQ in the same namespace, authenticating with the target's own default-user credentials and reading the source's object key). - A **Go-read precondition** gates each run on the `RabbitmqCluster` reporting `AllReplicasReady` (read through a small `rabbitmqtypes` partial type), so a not-yet-ready broker surfaces a precise `Ready=False` wait on the BackupJob/RestoreJob rather than a Job that starts and fails against an unreachable management API. - The produced `Backup` records the stored object's location on `status.artifact.uri`, rendered from the strategy's `spec.artifactURITemplate`. The `Backup` CRD has no status subresource, so this persists on Create, mirroring the Etcd driver. Dispatch is wired in both controllers (plus `supportedBackupStrategyKinds`), `rabbitmqtypes` is registered in the scheme, and RBAC grants read on `rabbitmqclusters.rabbitmq.com` (the Job itself is already covered by the existing `batch/jobs` grant). It ships `cozy-default-rabbitmq` (a `$bucketName`-gated `Rabbitmq` strategy) and routes `apps.cozystack.io/RabbitMQ` to it in the `cozy-default` BackupClass, which also enrols it in the `DefaultObjectsGate`. The RabbitMQ application chart is left untouched — it had no prior backup wiring, and the strategy carries the S3 coordinates itself. Unit tests cover the driver's pure helpers (applicationRef validation, cluster-name derivation, the readiness-message builder, artifact-URI rendering, parameter round-trip) and the reconcile precondition gate (defers with `Ready=False` and no Job until the cluster is ready, then proceeds). A runnable example under `examples/backups/rabbitmq/` and an honest-roundtrip Chainsaw e2e under `hack/e2e-chainsaw/rabbitmq/` seed a sentinel vhost, back it up, drop it, and assert it round-trips through object storage on both restore flows. The Job pod reuses the existing `platform-migrations` image (curl + ca-certificates) under a new `backupStrategyController.rabbitmqBackupClientImage` key, stamped in lockstep with `chBackupClientImage` in `packages/core/platform/Makefile` so the repository stays pinned at a single digest (`hack/image-pin-consistency.bats`). A dedicated slim image built by this package is a follow-up. **Validation:** `go build`/`go vet`/`gofmt`/`go test ./internal/backupcontroller/... ./internal/template/...`, deepcopy + Rabbitmq CRD generation, `helm template` proving the CRD-ordering gate both on and off, `helm lint`, and `helm unittest` (18/18). The backup + in-place + to-copy roundtrip was verified end-to-end on a dev cluster (a real management-API export and a real S3 `--aws-sigv4` upload/download against SeaweedFS), so the driver's core contract is exercised on a real cluster; the in-CI Chainsaw run is the remaining automated evidence. The object-cleanup, terminating-namespace teardown and readiness-deadline paths added during review are unit-tested (including `Reconcile`-level finalizer retention and the terminating-namespace release) but not yet cluster-exercised. (Note found while testing: RabbitMQ 4.2 is OOMKilled on the `nano` preset, so the example/e2e use `c1.small`.) ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff. This is an additive backup strategy plus a new strategy CRD; no downstream repository is mechanically forced by the change. A documentation page for RabbitMQ backup on [cozystack/website](https://github.com/cozystack/website) would be a reasonable follow-up, but it is not required by this diff — leaving the boxes unticked for a human to decide rather than ticking "no downstream" to clear the checklist. ### Release note ```release-note feat(backups): add RabbitMQ definitions backup/restore via a dedicated strategy.backups.cozystack.io/Rabbitmq driver; it gates each run on the RabbitmqCluster being ready and records the stored object on Backup.status.artifact.uri. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added RabbitMQ backup and restore support for definitions, including vhosts, users, queues, exchanges, bindings, policies, and related settings. * Added in-place and restore-to-copy workflows with readiness checks. * Added scheduled and on-demand backup examples. * Integrated RabbitMQ backups with the default backup configuration. * Backups store definitions only; message payloads and point-in-time recovery are not included. * **Documentation** * Added a complete RabbitMQ backup and restore demonstration guide. * **Tests** * Added end-to-end coverage for readiness and backup/restore round trips. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-03 16:03:46 +04:00
myasnikovdaniil	2b99853e6d	fix(networking): prevent webhook admission outages during namespace churn (#3997) ... ## What this PR does Moves ingress-nginx and kubeovn-webhook pod deny rules to source-side cilium policies, and keeps a target-side `ingressDeny` on the webhook pods beside them, so enforcement does not depend on which component performs the service DNAT. Pod access to both webhooks stays denied, ingress-nginx keeps exception for konnectivity agent (now keyed on its ServiceAccount instead of `k8s-app` label that any pod can set on itself), and external `world` deny stays as static target rule. Source-side rules deny only backend port. That covers ClusterIP traffic too, because cilium translates service to its backends before it enforces egress policy, and that ordering exists because `packages/system/cilium/values.yaml` sets `kubeProxyReplacement: true` against upstream default of `"false"`. The value is operator-reachable, so the target-side rule is what keeps the deny working if it is ever flipped, and two render-time beacons make a flip visible instead of silent. Naming the frontend with `toServices` is not possible, and templates now say why, because the construct looks correct and fails silently: cilium resolves `toServices` only in allow rules (`pkg/policy/k8s/service.go` walks `spec.Egress`, never `spec.EgressDeny`), so such `egressDeny` gets non-nil empty L3 selector from `pkg/policy/utils/parserules.go`, and `pkg/policy/types/policyentry.go` treats that as wildcard when rule carries `toPorts`. This does not remove the namespace-churn map writes. The target-side rule carries a cluster-spanning peer, so those writes come back. What changes is that they stop being load-bearing, because whichever end is degraded the other still denies. Cost is policy-map cardinality on the two webhook endpoints, and residual risk is an endpoint near `MaxEntries` getting the inverted add-before-delete order (`pkg/endpoint/bpf.go:1168-1177`). Both policies also change kind and name: namespaced `CiliumNetworkPolicy/ingress-nginx-admission-webhook` and `CiliumNetworkPolicy/kube-ovn-webhook` become cluster-scoped `CiliumClusterwideNetworkPolicy/ingress-nginx-admission-webhook-c5d94713a379` and `kube-ovn-webhook-c2bdbc146112`. Anything keyed on the old namespaced objects goes quiet with no error. ## Tests Package suites pin the deny shape per package, one rule per spec with a destination selector cilium can actually resolve, and pin the peer shape of the target-side rule against the two substitutions that would deny the API server (`fromEntities: [all]`, and dropping the peer). Checked by mutation, all of those go red. `matchRegexRaw` and `notMatchRegexRaw` are not used, helm-unittest 1.0.3 ignores both. `packages/system/kubeovn-webhook` had no `test` target in Makefile, and `hack/helm-unit-tests.sh` finds suites by probing `make -n test` per package, so all three of its suites never ran in CI, including port agreement guard that exists to catch a drift failing open. Added the target, plus `.PHONY: test` to it and to cilium. E2e churn case proves webhooks stay available. Second case proves they are still unreachable from pods, and that deny is confined to webhook port. Both now gate every verdict on the admission endpoints having ready addresses before, during and after, on controller pod identity and restart count being equal across the window, and on a positive control where the same pods' own 443 must stay reachable. So a dead or flapping backend fails as setup failure instead of reporting a deny that was never enforced. First run of these cases reported a false verdict for exactly that reason. ## Not verified Original outage in #3983 was not reproduced in isolation, and its symptom argues against the mechanism this PR was written around. `connect: no route to host` is `EHOSTUNREACH`, and a policy denial cannot produce it, since a deny drops after `connect()` has already returned and the caller sees a timeout. On a ClusterIP that errno is what cilium's socket LB returns with no usable backend, and socket LB is active in host netns because `pkg/kpr/kpr.go:47-49` forces it whenever kube-proxy replacement is set. Namespace deletion churns EndpointSlices and service backend maps as well as identities, which is directly on that path. So this change is justified as removing a structural weakness and adding a topology-independent deny, not as a confirmed root cause fix. What would settle it: `cilium-dbg monitor --type drop` on the controller's node during the window, agent log checked for the map-overflow warning, and the admission Service's ready backend count across those seconds. `world` deny that issue named as its first suspect is kept, on `cilium monitor` evidence recorded when kubeovn-webhook policy was written - api server arrives with identity `host` on same node and `kube-apiserver` from another, never as `world`. That is one topology, a kube-ovn-chained management cluster, and it is not re-derivable on cilium-primary variants or on a Kamaji guest cluster without a run. Whether cilium without kube-proxy replacement is a supported configuration is a maintainer decision and is open. If it is not supported, a `fail()` in the cilium chart is the right shape and should follow this PR. Refs #3983. ### Screenshots Not applicable. ### 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(networking): enforce the Ingress and Pod admission webhook deny on both the caller and the webhook endpoint, so it no longer depends on Cilium performing service load-balancing. Both policies become cluster-scoped: CiliumNetworkPolicy/ingress-nginx-admission-webhook and CiliumNetworkPolicy/kube-ovn-webhook are replaced by CiliumClusterwideNetworkPolicy/ingress-nginx-admission-webhook-c5d94713a379 and kube-ovn-webhook-c2bdbc146112. ``` ## Summary by CodeRabbit * **Bug Fixes** * Improved admission webhook reliability during namespace deletion and churn. * Preserved webhook protection while allowing unrelated secure traffic to continue. * Corrected webhook access controls for both ingress-nginx and kube-ovn. * **New Features** * Added cluster-wide policy support with safer backend-port enforcement and automatic capability detection. * Added safeguards for policy naming and configuration overrides. * **Tests** * Expanded automated coverage for webhook availability, access restrictions, policy rendering, and end-to-end suite selection. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-03 14:10:45 +05:00
Aleksei Sviridkin	02b7651b90	test(mariadb-rd): pin the rendered grant surface whole ... The exclude checks read .spec.secrets.exclude[].resourceNames[]?, which sees only name-shaped entries. An exclude entry selecting by label was therefore invisible to every one of them, and adding one revokes the tenant's connection string and its trust anchor at once: matchName returns true for every name when resourceNames is nil, and exclude is evaluated before include and wins. The test that existed to stop exactly that edit let it through. The empty-selector scan keyed on the literal token matchLabels, so an empty matchExpressions and a bare {} passed it anywhere in the file, including under services, which the comment claimed the same pass covered. Both are legal: the selector type inlines metav1.LabelSelector. One more comment stated the mechanism backwards — an exclude entry that lost resourceNames makes the backstop total rather than voiding it — and the check under it required only that some entry carry the key, so an added entry without one went unnoticed. Comparing spec.secrets and spec.services whole closes all of it. The comparison is against the rendered chart rather than the source file, because templates/cozyrd.yaml globs cozyrds/* and ships every match: reading one file leaves a second one, dotfiles included, granting whatever it likes. A document count fails closed on that. The two per-name tests are kept ahead of the comparison, subsumed but more legible, because the runner stops at the first failure. The block-extraction helper is gone: the cozytest runner injects return 0 before a column-zero brace, so its status was discarded there. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:57 +03:00
Aleksei Sviridkin	ce2cabb2d6	test(mariadb): verify the tenant-ca projection end to end ... The unit suite pins what the chart renders — the sentinel, its source name, its key — from the same assumptions the chart itself encodes, so the two agree by construction and neither can catch the other being wrong. Only a live mariadb-operator says whether <release>-ca-bundle is the right name, and only a live apiserver says whether the lineage webhook promotes the projection to something a tenant can read. Both converged engines already carry this step; mariadb had none. The fixture sets no tls.issuer, which is the case worth covering rather than a gap: TLS is always served and the projection template is ungated, so the operator reconciles the source here too. A negative satisfied by an absent label is also satisfied by a webhook that stamped nothing all run, so the credentials Secret leads as a positive control. For the same reason the key-bearing names are checked for private key material before their absence from the registry is required, the count of names that reached that check is asserted non-zero, and the read must answer NotFound specifically: an RBAC denial or an unavailable aggregated apiserver also makes it fail, and reading that as "safely absent" is how the check would stop discriminating without ever going red. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:55 +03:00
Aleksei Sviridkin	14af86aac0	test(mariadb): pin the tenant-ca sentinel instead of the declared source ... The file asserted that the ApplicationDefinition declares the CA source itself -- sourceSecretName, sourceKey -- and that the operator's ca-bundle is granted by name in secrets.include. The chart renders none of that any more. The trust anchor now reaches the tenant as the key-free "<release>.tenant-ca" projection: a TenantProjection sentinel in the chart names the extraction source, and the ApplicationDefinition selects the result on internal.cozystack.io/tenant-ca. Three assertions therefore pinned fields the file no longer contains, and a fourth pinned a by-name grant this design deliberately withholds. Pin the grant surface instead, the way nats-rd and mongodb-rd do: compare the whole value of secrets.include against the credentials Secret plus the tenant-ca selector, so neither an added name nor an added label slips past a per-dimension check, and assert that neither the credentials Secret nor the projection is named in exclude, which is evaluated first and wins. The source declaration stays pinned where it is rendered, by packages/apps/mariadb/tests/tenant_projection_test.yaml, which compares sourceSecretName against an exact key-free name -- a stronger guard than a grep here could make, since this file never sees the chart. Every assertion guarding a key-bearing Secret is kept. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:53 +03:00
Aleksei Sviridkin	69d5ce0644	test(mariadb): assert the CA error names its Secret, and correct the ledger ... The backup CA case is called "checks the CA file exists and names it" but only guarded the first half. Deleting the Secret name from the error left the suite green, so the reason MYSQL_CA_SECRET is passed to the CronJob at all — telling the operator which Secret to go look at, since the client reports a missing CA without naming a file or its source — was unpinned. The cozyrds suite header also counted its own coverage wrong. It claimed two live tests and three placeholders; there are seven and four. The four exclude guards are live and went unmentioned, and the case reading the source key is inert like its neighbours but carried no placeholder marker. That header is the file's honest-coverage statement, so an understated ledger is the one error it cannot afford. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:52 +03:00
Aleksei Sviridkin	6cb1914ec0	test(mariadb): assert the guards abort, not just that they are spelled ... Three assertions matched text that survived the mutation they exist to catch. The backup CA guard was pinned by three separate substrings: the two test expressions and the message. Flipping its exit status to 0, or short circuiting the condition with `false &&`, leaves all three intact and the suite green — while the backup runs on with --ssl-ca pointing at a file that is missing or empty, and in the exit-0 case reports success having dumped nothing. Match the whole construct through its exit instead. The exclude entries were checked by name without checking they still sit under resourceNames. A nil resourceNames matches every name, so dropping that key turns the backstop into a match-everything selector while each name is still present in the block. Assert the key. The leaf certificate pinned renewBefore but not duration. cert-manager rejects a duration that does not exceed renewBefore, so pinning one of the pair lets the other drift into a certificate that renders but cannot be applied. Each is verified by the mutation it was blind to: exit 1 to exit 0, a short-circuited condition, a renamed resourceNames key, and an 8760h duration shrunk to 1h all now fail the suite. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:51 +03:00
Aleksei Sviridkin	66d2c9b399	test(mariadb-rd): scope the exclude guards to the exclude block ... The backstop guards grepped the whole file for each name. Both lists sit at the same indentation, so an entry in `include` satisfied a guard that exists to prove the name is in `exclude` — and moving a name between them is the single most dangerous edit this file allows, since exclude is what keeps a Secret away from the tenant. Moving `-ca`, the operator's CA private key, from exclude to include left all ten guards green. So did moving `-root`. Extract the exclude block and match inside it; both mutations now fail on the name that moved, and the suite is unchanged otherwise. Also correct two descriptions the TLS work outdated. The cleanup hook's Role comment still said the rule covered two operator-generated Secrets; it now covers four, and two of them are issued by cert-manager, which the block above already explains. The runtime message called them all operator-generated for the same reason. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:51 +03:00
Aleksei Sviridkin	18a8bf296d	test(mariadb): scope the CA-bundle guard and name the CR field it checks ... The guard asserting that the trust anchor is surfaced to the tenant grepped the whole file. The bundle is also named in a comment and in the caCert declaration, so either of those could satisfy it while the secrets.include entry it exists to protect was gone. Scope it to the include block, the way the sibling key-bearing-Secret guard already does. Verified by deleting the include entry with a matching string left elsewhere in the file: the guard now fails, where before it passed. The suite header described a comment as stating that spec.tls.issuer is true. There is no such field on the MariaDB CR — issuer is a chart value, and what the chart renders is spec.tls.enabled, which is what the rest of that sentence already describes. Name the field the assertions actually check. Also drop the claim that an 11.4 client verifies by default. The default that changed in 11.4 is the --ssl-verify-server-cert flag, not whether verification happens: without --ssl-ca that client has no trust anchor, and falls back to an anti-tampering check derived from the password hash which establishes neither the chain nor the hostname, and which cannot be derived at all for a passwordless user. Passing both flags is what gives chain and hostname verification, on either client version. The documented command was already correct; the sentence around it invited dropping the flag that carries it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:51 +03:00
Aleksei Sviridkin	da2f5c33eb	fix(mariadb-rd): restore the release variable in the CA source name ... The CA source name was rewritten to a literal release prefix on the premise that no release variable exists in the template context. That premise came from the lineage webhook, which renders a different field: resourceNames in the secrets selectors, where the context really is only kind, name and namespace. caCert.sourceSecretName is rendered by the CA-extraction controller instead, and that one supplies a release variable explicitly. It also parses with missingkey=error, so an undefined key would fail loudly rather than resolve to an empty string — the silent collapse the rewrite was meant to prevent could not have happened. Restore `{{ .release }}-ca-bundle`. It is also the better spelling on its own: the literal hardcodes an assumption about the release prefix and would point at a Secret that does not exist if that prefix ever changed, while the variable is the release name by construction. The reference implementation for this field spells it the same way. Drop the guard that forbade the variable. The controller's own tests pin that spelling as correct, so the guard would have started failing the canonical form the moment the field became live. Keep the test pinning the source name, and say in it which renderer this field belongs to, so the two contexts are not conflated again. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:50 +03:00
Aleksei Sviridkin	5a27ee4e88	fix(mariadb-rd): exclude the exporter config Secret from tenant promotion ... The exclude backstop enumerated the internal credential Secrets but omitted `-metrics-config`. The operator writes it for every instance — `MetricsConfigSecretKeyRef` at the pinned 25.10.2 fixes the name as `<name>-metrics-config` — and its `exporter.cnf` carries [client] user=... password=... in plaintext. The exporter user is granted SELECT, PROCESS, REPLICATION CLIENT, REPLICA MONITOR and SLAVE MONITOR on `*.*`, so that password reads every table in every database. It is not a hypothetical Secret: the chart sets `metrics.enabled: true` unconditionally, so it exists on every ready instance. The include side selects by label with no name constraint, and a nil `resourceNames` matches everything, so the exclude list is what keeps it away from the tenant. Exclude beats include, so naming it there is sufficient. This is inside the enumerable part of the gap, not the part the comment already disclaims: that carve-out covers Secrets a future operator adds or creates mid-rotation, and this one exists today under a stable name. Also complete a sentence in the suite that lost the words naming which issuer it describes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:49 +03:00
Aleksei Sviridkin	f2bce8f57d	fix(mariadb-rd): name the CA source Secret with a key the context defines ... The CA source was spelled `{{ .release }}-ca-bundle`. The webhook builds the only context these templates are rendered against, and it carries kind, name and namespace — no release. The template resolves to nothing, so the name would come out as `-ca-bundle` and match no Secret. The exclude entry a few lines down already names the same Secret the way that works, `mariadb-{{ .name }}-ca-bundle`: the release prefix is a literal and the instance name is the templated part. Spell the source the same. One Secret described two ways in one file meant one of them was wrong, and it was this one. The declaration is inert until the extraction controller lands, so this changes nothing that runs today. That is also why it could sit here unnoticed: the test pinned the broken spelling and passed on it, which would have made the name wrong exactly when it first started being read. Pin the working spelling instead, and add a guard that no undefined key is templated into a Secret name. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:49 +03:00
Aleksei Sviridkin	81c44574d0	fix(mariadb): scope the Secret guards to the secrets section ... The awk that was supposed to read secrets.include reset on any two-space key and then re-entered on the next four-space include:, so it swept services.include as well. It passes today only because no Service happens to end in -tls — the exact case the comment promised it handled would have misfired on a Service name. The exclude reader had the same shape. Gate both on the secrets: section header instead of on indentation alone. Verified in both directions: a Service named -tls no longer trips the guard, and a Secret named -tls under secrets.include still does. Also correct which CA the backup client verifies against: the bundle carries the chart's CA too under the cert-manager issuer, which is what makes the unconditional --ssl-ca right in both configurations. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:48 +03:00
Aleksei Sviridkin	a55dc5ff19	feat(mariadb): name the dangerous Secrets in the ApplicationDefinition exclude ... The include selector matches by label and carries no name constraint, so any Secret of this instance that acquires the tenant-ca label is promoted to the tenant with its data copied verbatim. The label on the source is policy-guarded; the one on the destination is not. Exclude is evaluated first and short-circuits, so naming the dangerous Secrets closes the part of that gap we can enumerate. Listed by group with the reason each is dangerous: the private keys from both issuers, the internal database credentials, the backup S3 keys and restic password, and the registry pull secret. The guarantee is only as wide as the list. Matching is by exact name with no globs, so a Secret added by a future operator version, or one created transiently during a rotation, is not covered — the protection that holds in general is still that only the extraction controller writes the label. mariadb-operator has no rotation-snapshot Secret of the kind that makes this worse elsewhere; its Secret names are fixed. The existing guard against a key-bearing name is rescoped to secrets.include while here, since such a name under exclude is the backstop working rather than a leak. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:47 +03:00
Aleksei Sviridkin	e164050cc1	docs(mariadb): state what the guards and the docs actually cover ... Four claims of mine were broader than the thing they described. The bats header said it pins the invariants deciding whether a private key can reach a tenant. Three of its tests touch the declaration that the API does not accept yet, so they pin an intended spelling and nothing more; if the field lands under another name they stay green while the feature does nothing. Say which tests are live and which are placeholders. The certificate comment said the operator skips reconciliation entirely. It skips issuance, which is why SANs go unvalidated, but it still reads the Secret for the pod annotation hash and status. The README mapped the client's verification default onto the version enum, which selects the server image. The default belongs to the client library, so an 11.4 client verifies against a 10.6 server. The backup guard is described as catching a missing or unusable bundle. On a fresh managed-TLS install the bundle can be present and non-empty while holding only the operator's client CA, and the failure lands at verification instead. Note the window, and note in the README that a new instance stays down until cert-manager issues. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:46 +03:00
Aleksei Sviridkin	f0b77de1e2	docs(mariadb): correct the empty-selector blast radius and order the teardown ... Two claims of mine were wrong about mechanism. The empty-selector warning said such a selector would project every Secret in the namespace. The lineage webhook computes those labels only for objects whose ownership resolves to the application, so the reach is the instance's own Secrets. Still the thing to prevent — that set contains -ca-tls and -tls — but the stated reason should match the code, since it is what a reader checks the guard against. The teardown step told the user to delete both Secrets with no ordering. The server keypair stays mounted in the running pods until the operator reissues from its own CA and rolls the StatefulSet, so following that literally leaves the next pod unable to start. Wait for the rollout first. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:45 +03:00
Aleksei Sviridkin	2be135a821	fix(mariadb): pin the verification default to the version it changed in ... The note said the hostname check is off on 10.11 and on across 11.x. The default actually changed in 11.4. Measured all four versions this chart offers: 10.6 and 10.11 leave it off, 11.4 and 11.8 enable it. Name the versions rather than the series, since the reader is choosing between exactly those. Scope the key-bearing-Secret guard to the secrets block as well. It scanned the whole file, so a Service whose name ends in -tls would have tripped it while a Secret entry with a trailing comment slipped past — the wrong answer in both directions for the check that decides whether a private key can reach a tenant. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:45 +03:00
Aleksei Sviridkin	3de6181da0	feat(mariadb): surface the operator CA bundle as a tenant resource ... The dashboard Role already let the tenant read <release>-ca-bundle, but the ApplicationDefinition did not list it, so it was never marked as a tenant resource and never appeared through the API. The documentation tells clients to fetch exactly that Secret, which worked with kubectl and not in the interface the platform actually gives tenants. Name it in secrets.include, the same way kafka names its CA certificates. This works against the current API, unlike the label selector beside it, which stays for the projection an unmerged controller will publish — so the trust anchor is reachable now rather than only after that lands. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:42 +03:00
Aleksei Sviridkin	38671c3ff0	test(mariadb): catch a bare empty label selector, not just the inline one ... The guard against an empty selector matched only "matchLabels: {}". A bare "matchLabels:" with nothing nested under it is the same empty selector — it compiles to match-everything and would project every Secret in the namespace to the tenant — and it slipped through. Check that a bare selector is followed by an indented label as well. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:41 +03:00
Aleksei Sviridkin	85c3ff1419	test(mariadb): require the tenant CA label inside the selector ... The check accepted the label anywhere in the ApplicationDefinition, so a stray occurrence would satisfy it while the selector that actually exposes the projection stayed empty or wrong. Scope it to the matchLabels block it has to appear in. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:39 +03:00
Aleksei Sviridkin	38e2c4fea4	test(mariadb): close a quoting hole in the ApplicationDefinition guard ... The check rejecting key-bearing Secrets from the tenant include list matched only unquoted list entries, while its sibling check was already quote-agnostic. A quoted entry would have passed both the guard and the comment claiming it could not. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:38 +03:00
Aleksei Sviridkin	18215895fd	fix(mariadb): apply TLS enforcement regardless of who owns the CA ... tls.required was only emitted when the chart managed the certificates, so setting it on an instance using the operator's own CA was accepted by the schema and then dropped on the way to the CR — no enforcement, no error, no warning. For a setting whose entire purpose is refusing unprotected connections, silently ignoring it is the worst available outcome. The coupling was artificial: in the operator API required sits beside enabled with no dependency on either certificate reference, and enforcement works against operator-issued certificates just as well. Emit it in every configuration and let it mean what it says. Also loosen the two negative guards in the ApplicationDefinition check, which required a literal closing quote and exact brace spacing and so would have missed an unquoted key-bearing source. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:36 +03:00
Aleksei Sviridkin	4339eb4631	test(mariadb): pin the ApplicationDefinition trust-anchor invariants ... The ApplicationDefinition is where a mistake decides whether a private key can reach a tenant, and it is the one file in this change the chart test suite cannot reach: helm-unittest only covers rendered templates, and the *-rd packages expose no test target. Add a shell check in the style of the existing kafka one, which the Makefile discovers automatically. It pins that the CA source is the operator's key-free bundle rather than either key-bearing Secret, that the tenant selector is the specific label rather than an empty selector that would match every Secret in the namespace, and that no key-bearing Secret is listed by name. Type required as a pointer while here. As a plain bool it rendered a default of false in the generated documentation while the description promised true, and the zero value could not represent an explicit false for consumers going through the generated struct — which is the value the documented migration path depends on. Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-03 01:38:35 +03:00
Aleksei Sviridkin	3cf9431f1e	test(redis): fail the negative dial probes on an unread state ... The two negative dials in the chainsaw suite, the plaintext dial at a TLS instance and the TLS dial at the instance whose flip was rejected, decided by whether PONG appeared on stdout with stderr discarded. A missing pod, a 403 on exec, a wrong container name or the outer cap firing print no PONG either, so each of them passed as the refusal being asserted, and the flip case is the check that a rejected flip left TLS off. Keep what the dial printed and pass only on the refusal each listener actually gives, verified in the shipped images: a TLS listener drops a plaintext client at AUTH and redis-cli reports it as an I/O error; a plaintext listener holds a ClientHello in silence, because the inline parser's newline scan stops at a NUL in the handshake header, so the inner cap expires and kubectl's exit-code trailer carries the 124. That trailer is what separates the cap firing inside the pod from the wrapper's cap firing outside it. A listener that answers PONG fails the check, and anything else fails as a state the check did not read, with the dial's output in the message. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-02 20:48:52 +03:00
Andrey Kolkov	6eba4d9fa7	test(backups): use .svc FQDN for rabbitmq backup e2e S3 endpoint ... The reworked rabbitmq backup roundtrip reached the upload step and failed TLS verification: `curl (60) no alternative certificate subject name matches target hostname 'seaweedfs-s3.tenant-root'`. The demo Bucket, in-cluster endpoint resolution and the copied SeaweedFS CA all worked; the remaining gap is the hostname. The seaweedfs filer serving cert's SANs are `*.tenant-root.svc` and `*.tenant-root.svc.cluster.local`, which the 2-label `seaweedfs-s3.tenant-root` matches neither; the 3-label `.svc` FQDN does (and still resolves via search-domain expansion). Point S3_ENDPOINT at `https://seaweedfs-s3.tenant-root.svc:8333`. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com>	2026-09-02 21:25:21 +04:00
Myasnikov Daniil	686a81a491	docs(networking): stop claiming the churn coupling is removed ... Restoring the target-side pod deny put a cluster-spanning peer selector back on the webhook endpoint, which is the exact thing the source-side rules were added to take off it. The comments still said the coupling was "structural and removed", and the release note still said deleting a namespace no longer rewrites that endpoint's policy map. Both were true of the previous revision and are not true of this one. What ships now is two denies that fail in different directions: the source-side one keeps enforcing while the webhook endpoint's policy is being recomputed or has overflowed its map, the target-side one keeps enforcing where Cilium is not the service load balancer, and neither subsumes the other. So the coupling is not eliminated, it is made non-load-bearing -- whichever end is degraded the other still denies, and the map writes are writes rather than an outage. Eliminating it outright would mean dropping the target-side rule and accepting a silent fail-open, which is the worse trade. The chainsaw case's header claimed the deletion "briefly made fail-closed admission webhooks unreachable". That is the unconfirmed hypothesis, and the errno in the report contradicts it: EHOSTUNREACH cannot come from a policy denial, which drops after connect() has returned. It now says what the case actually asserts -- both webhooks still admit across a namespace deletion -- rather than claiming to reproduce a cause. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>	2026-09-02 20:52:06 +05:00
Myasnikov Daniil	0335421769	test(e2e): check every webhook registration, and name the controller Service ... Three narrowing fixes to the two webhook cases. The availability precondition resolved only the FIRST registration of each webhook name (awk ... exit) while every matching registration fires on the applies below, so an unrelated flaky ingress-nginx release would have aborted an apply and been reported as a churn-induced outage. It now waits on all of them and says which one it is waiting for. Split into require_service_up plus a loop, indexed with sed rather than piped into `while read`, because a pipeline's body runs in a subshell under dash and a `return` from it would not reach the caller. That split immediately produced its own bug, caught by the scenario harness: POSIX functions share one scope, so the callee's `label="$1"` overwrote the caller's copy and the second registration reported itself as "(1/2) (2/2)". The callee now uses distinct names. The positive control's Service was picked with `grep -v -- '-admission$' | head -1`, which relies on ingress-nginx-controller sorting before ingress-nginx-controller-metrics. Deterministic but undocumented, and the control has to be the Service that actually exposes the https port, so -metrics is now excluded explicitly. Also drops a stale sentence: the churn pod's comment claimed the old target-side policies coupled the webhook endpoint to identity churn, which overstates a map write as an outage and is moot now that the target-side rule is back. The pod is there because an empty namespace allocates no identity, so without it the case would exercise nothing it names. Eleven scenarios through both scripts under dash with a stubbed kubectl, all as specified, the new multi-registration case included. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>	2026-09-02 20:45:22 +05:00
Aleksei Sviridkin	25a0618184	test(redis): exercise TLS end to end in the chainsaw suite ... Everything the TLS unit tests can only render now runs on a live cluster. A second fixture with tls.enabled asserts that cert-manager issues the chain, that the patched operator brings two Redis replicas and the Sentinel Deployment to Ready through its --tls probes, that the operator publishes its CA-only Secret under the widened ClusterRole, and that the CA-extraction controller hands the tenant exactly ca.crt through core.cozystack.io/tenantsecrets while the key-bearing Secrets and the operator's copy stay off that path. The last check completes a handshake through the rfr- service name with the same CA the tenant receives. A third case applies tls.enabled to a running plaintext release and asserts the refusal end to end: the HelmRelease upgrade fails with the schema's own message, the StatefulSet keeps its revision and both replicas Ready, plaintext still answers and no TLS listener appears. The TLS case also asserts the topology the operator built rather than stopping at readyReplicas: exactly one master, a replica whose link is up, and sentinels that moved off the bootstrap address. Ready alone witnesses the bootstrap, since a pod still replicating from the initial 127.0.0.1 never gets there, but the readiness script exits 0 for anything reporting role:master, so it stops witnessing once the bootstrap is over. Both PING calls note where the pinger credentials come from: the ACL user the operator writes into every redis.conf it generates, and the one its own liveness probe uses. Nothing in this tree defines it, so a reader grepping for it here finds nothing and reads the calls as broken. Every redis-cli call runs under the container's own timeout, and both script operations carry an explicit one. The inner cap is the part that matters. The flip case dials TLS at a plaintext listener that will never send a ServerHello, and redis-cli has no handshake deadline of its own. Chainsaw does kill the shell when its exec budget runs out, but it captures the script's stdout and stderr through pipes, and Go's exec then waits for every holder of the write end to close it; the orphaned kubectl is one, so the suite would go silent until the job ceiling instead of failing. Expiry reads as the refusal for the negative check and as a named failure for the PINGs. Both engine images provide /usr/bin/timeout: busybox on the sentinel, coreutils on the data pods. The negative half of the tenant-secret pair requires the error to say NotFound. A 403, a timeout or an unserved CRD would otherwise be read as proof that the Secret is not served, which is the direction that fails silently. The dashboard Role is asserted on the deployed object, not only in the chart unit tests: it must reach the auth Secret and nothing else, which is the name-granting half of the pair the label-driven projection completes. Every kubectl in both scripts runs under a wall-clock bound, because the per-command cap inside a container cannot fire when the exec stream itself never establishes, and a kubectl left running holds the script's stdout open. The plaintext StatefulSet in the flip case waits 5m rather than 2m: it measured 98s of that budget on a real run, being the third failover the group brings up in one namespace. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-02 17:58:06 +03:00
Aleksei Sviridkin	d0cab491f3	feat(redis): move the tenant trust anchor onto the platform projection ... The tenant read path was two grants on Secret names: the operator's CA-only <release>-ca-cert in the dashboard Role, and the same name in the ApplicationDefinition's include list. Replace both with the projection the previous commit declares. A name grant conveys whatever occupies the name. The operator holds create and update on Secrets cluster-wide, so what sits at <release>-ca-cert is the operator's choice rather than the platform's, and RBAC has no key-level filter to catch it if that ever stops being just ca.crt. <release>.tenant-ca is written only by the CA-extraction controller, which copies one named key and refuses PEM private key material outright. That is the difference between a name that happens to hold a certificate today and provenance the platform vouches for, and it is the reason postgres, mongodb, nats and qdrant all reach the tenant this way — mongodb's operator publishes a CA Secret of its own too, and that one is excluded rather than granted. Also drop spec.caCert from the ApplicationDefinition. The ApplicationDefinition CRD does not define that field and Flux applies these server-side, so the apiserver rejects the whole object with "field not declared in schema" and the redis-rd release never goes ready. It was carried deliberately as a merge-order gate for a controller that reads it; that controller landed as the TenantProjection reconciler, which reads a sentinel in the app chart instead, so the gate has nothing left to wait for. hack/check-redis-rd-secrets.bats pins the whole rendered Secret surface rather than probing for known-bad names, alongside the existing guards for nats, qdrant and mongodb: an include with neither resourceNames nor matchLabels restricts nothing, and a second matchLabels entry selecting a label cert-manager also stamps on the key-bearing Secrets would grant the CA private key while every name in the file stayed correct. The connection example passes the generated password as well. Password generation is on by default, so the command as printed connected and then answered NOAUTH on the first real command. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-02 17:58:06 +03:00
Aleksei Sviridkin	cf3cb0f133	test(values): reject a boolean default on a string-typed schema property ... cozyvalues-gen quotes a default only when the declared type is the literal `string` or `quantity`; a named alias, including one declared `@enum {string}`, falls through to a YAML re-parse of the bare value where the 1.1 boolean words resolve. A chart spelling out a `"no"` default under such a type emits `"default": false` into a string-typed, enum-constrained property and `+kubebuilder:default:=false` onto the matching Go field. Templates read values.yaml rather than the schema, so the chart suites stay green while the shipped schema and CRD carry a default the field rejects. Vendored schemas under packages/*/*/charts/ are left out of the scan: cozyvalues-gen never writes them, they change only through `make update`, and a violation there would have no fix inside this tree. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-02 17:58:05 +03:00
Myasnikov Daniil	cbd0e78cad	test(e2e): stop the webhook cases passing for the wrong reason ... Both new cases could report a verdict they had not observed, and the first CI run of them did exactly that against a crash-looping ingress-nginx controller. admission-webhooks-survive-namespace-churn died in setup: the admission Service had no ready backend, so the first apply -- which passes through that same fail-closed webhook -- failed with "no route to host" before any churn ran, and the failure read as the outage under test. It now asserts both webhooks are up BEFORE the churn, resolving their coordinates from the webhook registrations themselves rather than from a guessed release name, and says so in its own words when they are not. Proven against the pre-change script with a stubbed kubectl: with a dead admission Service the old script proceeded into the churn (delete namespace, two applies, delete namespace, two dry-runs); the new one stops and names the cause without touching the churn namespace. admission-webhook-deny-still-enforced measured a corpse. Its `kubectl wait` did observe both pods Ready, but the controller was flapping on a ~100s cycle and the probe ran against a dead backend 19 seconds later. "Denied by policy" and "no backend to route to" are the same observation from inside the probe, so the two deny assertions would have reported success against a cluster with no policy installed at all, and the port-scoping assertion reported a policy bug that did not exist. The deny verdicts are now gated on the admission Service having ready backends before the probe, on every poll while it runs, and after it finishes, and on the controller pods neither restarting nor being replaced across that window -- EndpointSlice readiness rather than pod readiness, because a `kubectl wait` is what a flapping pod satisfies transiently. The probe reports reachable/unreachable per destination and always exits 0; this script holds the endpoint evidence and is the only place that can attribute a refused connect, so anything undecidable now fails as a setup failure instead of as a verdict. Ten scenarios driven through both scripts under dash with a stubbed kubectl, all behaving as specified: a dead backend, a backend dying mid-probe, a controller restarting mid-probe, a probe that never completed, an unregistered webhook, the deny lost, the deny widened to every 443 destination, and the healthy path. The restart-mid-probe case is the one that used to produce a confident false pass. Two more things the same rewrite fixes. The step budget was 6m against inner budgets summing to 420s, so a slow-but-healthy cluster was killed with no verdict and no describe output; both steps now carry a timeout above their own sum, per the rule in hack/e2e-chainsaw/.chainsaw.yaml, and the `kubectl wait` whose 180s dominated that sum is gone. The churn namespace now carries a running pod, because a Cilium identity is a set of endpoint labels including k8s:io.kubernetes.pod.namespace: an empty namespace allocates no identity and its deletion drives none of the UpdateIdentities path the old target-side policies coupled the webhook endpoint to. And both server-dry-run Pods carry a restricted-compliant securityContext -- never scheduled, but a Pod that PodSecurity would reject aborts the check under set -eu and reads as the webhook being down. Refs #3983. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>	2026-09-02 19:34:51 +05:00
Andrey Kolkov	86fe8e5a7e	feat(valkey): add managed Valkey service (#3380) ... ## What this PR does Adds a **managed Valkey** service to the catalog, alongside managed Redis. > **Stacked on #3406** (redis-operator migration to freshworks-oss). This PR targets that branch and depends on it; review/merge #3406 first. ### Why Redis relicensed at 8.x to a source-available family (AGPLv3 / SSPL / RSALv2), and the managed Redis app currently ships `v8: 8.4.0` with no BSD-licensed option ≤ 7.2. That is a licensing risk for CNCF Incubation. [Valkey](https://valkey.io) is the BSD-3-Clause fork of Redis 7.2.4, governed by the Linux Foundation, and a drop-in replacement for Redis. This is the same move Argo CD made — [`cncf/foundation#750`](https://github.com/cncf/foundation/issues/750) was closed with "no need for exception" after they migrated to Valkey. ### How it works Valkey runs on the same operator as managed Redis. #3406 migrates that operator from the archived `spotahome/redis-operator` to the actively-maintained **freshworks-oss/redis-operator** fork, which adds a native `spec.engine: Valkey` switch. With it, the operator runs `valkey-server` / `valkey-cli` natively (in the launch command and in its liveness / readiness / shutdown probes), so the chart uses the official `valkey/valkey` image directly — **no Redis-compatibility shim image**. The app mirrors managed Redis: same shape/values (`replicas`, `resources`/`resourcesPreset`, `size`, `storageClass`, `external`, `version`, `authEnabled`), Sentinel-based HA, metrics exporter, `WorkloadMonitor`s, dashboard resource map, and `ApplicationDefinition` (`kind: Valkey`). Versions offered: `v8 → 8.1.8` (default) and `v7 → 7.2.13` (the BSD-3-Clause Redis 7.2 line, most conservative drop-in). ### How it was verified - `helm template` renders cleanly for `v8`, `v7`, and `external: true`; the `RedisFailover` carries `spec.engine: Valkey` and `image: valkey/valkey:<version>`. `helm lint` shows only the pre-existing icon-URL / `cozy-lib` warnings that managed Redis emits too. - `helm template` of `valkey-rd` renders a valid `ApplicationDefinition` (`kind: Valkey`, `plural: valkeys`, `category: PaaS`). - Generated artifacts (`values.schema.json`, `README.md`, `types.go`, `zz_generated.deepcopy.go`, the ApplicationDefinition `openAPISchema`) reproduce with the pinned `cozyvalues-gen` v1.6.0 and `controller-gen` v0.16.4 — root `make generate` and per-app generate both produce no drift; `go build ./valkey/...` passes. - A live-cluster e2e was not run from this environment; the Chainsaw suite `hack/e2e-chainsaw/valkey/` (mirroring the redis suite) is included and `hack/select-e2e.sh` maps the `valkey-application` source to it, so it runs in CI once the freshworks operator image is built. ### Notes - The Valkey logo (`packages/apps/valkey/logos/valkey.svg`) is a placeholder in Valkey's brand colors, pending the official brand asset. - A Grafana `db/valkey` dashboard was intentionally left out of this initial PoC; it can follow. ### Screenshots N/A — no UI changes. ### Downstream repositories This adds a new managed-app kind, which likely warrants follow-ups in `cozystack/website` (user-facing docs for the new service) and possibly `cozystack/terraform-provider-cozystack`. No follow-up PRs are opened yet — leaving these for a maintainer to decide, per the template guidance. - [ ] 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(valkey): add managed Valkey service — a BSD-3-Clause, Linux Foundation-governed alternative to managed Redis, running on the freshworks-oss redis-operator via native spec.engine: Valkey (versions 8.1.8 and 7.2.13) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added managed Valkey service support with high availability and configurable replicas. * Supports Valkey versions 7 and 8, persistent storage, resource presets, authentication, and optional external access. * Added monitoring metrics and dashboard integration. * Added platform package integration and tenant administrator access. * **Documentation** * Added Valkey configuration guidance and storage-class immutability documentation. * **Tests** * Added end-to-end coverage for deployment readiness, storage, and replicas. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-02 18:18:42 +04:00
Myasnikov Daniil	f73b54fee9	test(e2e): assert the webhook deny is enforced and scoped to its port ... The churn case added alongside it proves the webhooks stay AVAILABLE. Nothing proved they are still UNREACHABLE from pods, which is the property the policies exist for, and no test anywhere checked that the deny is confined to the webhook port -- which is why a rule that denied all TCP/443 egress cluster-wide passed every existing assertion. Probe from tenant-root, where the host cluster's ingress-nginx lives and where allow-internal-communication permits pods to reach it, so the deny is the only thing that can block the webhook port; from the isolated tenant-test the probe would pass for the wrong reason. Three assertions against the same pods and the same tenant egress allowances, differing only in port: the controller's own HTTPS port must stay reachable, the admission Service ClusterIP must not be, and the backend port must not be. The first is the one that separates a scoped deny from a wildcard. Uses busybox:musl, already staged on every node by hack/e2e-prepull-images.sh, with a securityContext that satisfies the restricted PodSecurity profile. Service names, ports and the backend port are discovered at runtime rather than hardcoded, since the release name differs between installs. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>	2026-09-02 19:15:45 +05:00
Myasnikov Daniil	ad961f6de1	test(e2e): cover webhook admission after namespace churn ... Exercise ingress and pod admission immediately after deleting a tenant namespace, and route ingress-nginx changes to the gateway suite. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>	2026-09-02 19:15:44 +05:00
Andrey Kolkov	329ef9cf3f	test(backups): make rabbitmq backup e2e self-contained via demo bucket ... The rabbitmq backup roundtrip was the only backup e2e routing through the platform cozy-default system bucket, whose advertised endpoint in the CI sandbox is the unroutable s3.example.org placeholder, so the backup Job failed with "Could not resolve host: s3.example.org". Mirror the clickhouse/mariadb backup examples: provision a demo Bucket, project its access keys into rabbitmq-backup-creds and copy the seaweedfs CA into rabbitmq-backup-ca, and render a self-contained Rabbitmq strategy + rabbitmq-default BackupClass whose curl uploads to the in-cluster seaweedfs endpoint (S3_ENDPOINT override, CURL_CA_BUNDLE for the self-signed cert). The in-Pod script is kept identical to the shipped cozy-default-rabbitmq strategy, which is left unchanged; the demo only reuses its client image. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com>	2026-09-02 15:37:30 +04:00
myasnikovdaniil	67b4e23ca2	test(tests): cut the noise in unit and e2e job logs (#3973) ... ## What this PR does Unit test job log is 238,794 lines and 31MB, and 233,444 of those are `set -x` trace from `hack/cozytest.sh`. The one failing assertion sits at line 238,573. Nothing in that stream is needed there, `run_one` already tees raw output to `$log` and the fail handler dumps all of it for whichever test fails, so dropping the live stream loses output of passing tests only. `COZYTEST_TRACE` defaults to 1, so e2e call sites in `packages/core/testing/Makefile` keep the live stream. For a long e2e suite that stream is the only progress signal, and the only record at all when a step timeout or a dead runner kills the job before the fail handler runs. `bats-unit-tests` sets 0. Running the runner by hand stays verbose, that is the case where a human is actually watching. Second thing, previous-instance log capture now orders by restart recency. Container cap (`COZY_PREVLOG_MAX=12`) is spent in namespace order and "unrelated" meant only "in another namespace", time was never a key. So a container that restarted once during bring-up competed on equal footing with one that crash-looped during the test, and won whenever the pod list reached it first. Two measured e2e jobs spent 1034 and 1545 lines on `virt-api` and `cdi-apiserver`, both restarted once at install time by `Unable to create certwatcher`, hours before any app suite ran. On a live stand 178 containers carry a restart and their timestamps span 24 days, so the cap binds hard enough for order to decide everything. This orders, it does not filter. Install-time restart is the explanation when install is what broke, and the capture has no reference point for "when the test began" that would make a cutoff honest. Namespace stays primary key, because a stale restart in the namespace under test beats a fresh one three namespaces away. So it fixes cap displacement, not raw volume of a single run. `lastState.terminated.finishedAt` joins the row. The guard on it is three deep and the innermost one is load bearing, which I got wrong first time round and review caught: `finishedAt` is a metav1.Time behind `json:",omitempty"`, omitempty cannot omit a struct, and the zero value marshals to `null`. The kubelet does synthesize a terminated state with no time, ContainerStatusUnknown on a pod deleted mid-run, which also bumps restartCount and so clears the restart filter. kubectl renders that leaf as the literal `<no value>`, and `<` sorts above every digit, so under the reverse sort that row landed FIRST and displaced the most recent real restart. Exactly the inversion the ordering exists to prevent, plus a header reading `previous instance ended <no value>`. Fixed at both layers because they fail independently: the template guards the leaf so the string cannot be produced, and the sort normalises any field that is not shaped like a timestamp to empty so the ordering holds even if some other producer emits one. Worth naming how that slipped through. I probed the template against a live cluster, saw no `<no value>`, and read it as the guard being correct. It only meant no container on that cluster was in that state. Reading the field also fixes a latent bug it would otherwise have caused: `read -r ... restarts` folds a trailing field into the last variable, so the dump header would print `restarts=1|2026-08-22T09:06:00Z`. Now it names the time, which is the first thing telling a reader whether the dump predates the test. ### What quiet mode keeps and what it drops Two things are dropped, and the second one came out of review rather than out of me. First, xtrace and stdout of tests that pass. You cannot diff a passing test's trace against the failing one inside the same job log, and a warning printed by a test that passed is gone. Second, a test that is KILLED rather than failed loses its trace as well. The fail handler runs only after the pipeline returns, so a hang reaped by the job timeout never dumps anything, and quiet mode has by then dropped the live stream. Verbose mode never noticed, because the trace was already printed. Measured: verbose leaves the line the test blocked on, quiet leaves the `╭` and nothing else. So this one is a real regression on the unit lane, not a pre-existing gap. An INT/TERM trap is the obvious repair and it does not work. POSIX sh defers a trap until the current foreground command returns, so installing one stops the runner dying on the signal at all: a suite hung on `sleep 30` took 30s to exit with the trap against 2s under the default disposition. GitHub escalates INT, then TERM, then KILL, so the deferred handler gets SIGKILLed before it ever runs, and the grace period goes on waiting for the hang. Built it, measured it, reverted it, and the measurement is in the comment now so the next reader does not repeat it. Making it work needs the test pipeline moved into the background with `wait`, which is the interruptible one, and that is surgery on the path every suite in this repo takes for a partial trace in a rare case. What is left for that case is the escape hatch below, which for this lane costs 48s and no cluster. The same argument is why e2e keeps the stream on: re-running there is an hour and a live cluster, so the record has to be written as it goes. Everything else stays. The `╭`/`╰` pair per test with elapsed time, so which tests ran, in what order and how long each took is all still there. Full trace, stdout and stderr of the failing test, same bytes as before, because `run_one` tees to `$log` and the fail handler dumps it, and that path is untouched. Anything printed outside `run_one` as well, file level code, `.bats` parse errors, and the `--- running <file> ---` markers. A hang still shows up as `╭` with no matching `╰`. And `make unit-tests` stops at the first failing test anyway, so there was never more than one dump to read. Measured on a suite that is actually red (`hack/ghcr-mirror_test.bats` fails on my machine because it reads a live cluster): verbose 1472 lines, quiet 63 lines, and the post-failure dump is 23 lines and byte-identical in both modes. Getting the trace back, both paths checked: ``` COZYTEST_TRACE=1 make unit-tests # env prefix, 29200 trace lines make unit-tests COZYTEST_TRACE=1 # make cmdline, same hack/cozytest.sh hack/foo.bats # by hand, verbose by default ``` This lane needs no cluster and runs locally in 48s, so anyone who wants the trace just reruns it. That is exactly why the same argument does not extend to e2e, and why the default stays 1 there. Nothing else consumed that stream. Unit lane stdout goes only to the job log, `cozyreport.sh` folds in crust-gather snapshots and the trap writes those to disk independently, and the unit lane produces none of them. ### Verification Measured on this PR's own run: unit job went from 238,794 lines and 31MB with 233,444 trace lines, to 5,720 lines and 580KB with 0 trace lines and 1548 tests OK. Those two totals are not strictly like for like, the old run was red and this one is green, so the honest comparator is 233,444 trace lines down to 0. Quiet output is byte-for-byte the non-trace subset of verbose, and the fail handler dump is unaffected (checked with a deliberately failing suite). Verbose output unchanged against main, byte-identical sorted on `hack/bats-no-exit-trap.bats`. Other suites are not self-stable run to run (temp names, epochs, stdout/stderr interleaving) so that is the ceiling on that check, not a finding. Eight tests on the capture and seven on the switch. Every fix is mutation proven: reverting the template leaf guard, removing the sort key normalisation, dropping the read variable, dropping `-s`, silencing an e2e recipe, removing the fail handler dump, flipping the default, and making the switch a no-op are each caught. Three of those guards did not bite when first written, which review also found: both stability fixtures were pre-sorted ascending, which is what sort's last-resort whole-line compare reproduces with no `-s` at all, and no test fed a six-field row through the whole script, so deleting the read variable left the suite green at 39/39. Whole unit lane, 83 files, 1503 passed and 1 failed. The failure is `hack/ghcr-mirror_test.bats`, it reads a live cluster and fails identically under main's own runner. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map file by file against the diff. Nothing under `hack/` is moved or renamed, no make target changes what it does (`bats-unit-tests` runs the same files to the same verdict, only quieter), and `hack/package.mk`, `hack/common-envs.mk`, `hack/update-crd.sh` and `hack/e2e-prepare-cluster.bats` are untouched, which is what ccp, external-apps-example, talm and ansible-cozystack couple to. No package `values.yaml`, `values.schema.json`, CRD, namespace, label, annotation or metric name is involved. - [x] No downstream repository is affected by this change ### Release note ```release-note test(tests): bats unit test trace is now opt-in via COZYTEST_TRACE and quiet by default in `make unit-tests`, and the e2e previous-instance log capture now picks the most recently restarted containers first ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Test runs can suppress live trace output for passing tests while preserving full diagnostics for failures. - Trace visibility can be customized through the test configuration. - Previous-instance logs are prioritized by the most recent container termination within the preferred namespace. - Captured log headers now show when the previous instance ended. - Undated logs are retained and placed after dated entries. - **Tests** - Added coverage for trace controls, failure handling, and recent-log ordering. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-02 10:44:23 +05:00