mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
b9ecb2e39f705eac2cf1bf35e9cfa54ae5ed0548
974
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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 --> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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 --> |
||
|
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 --> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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 --> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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 --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
ba32df69c0 |
feat(backups): add RabbitMQ definitions backup via a dedicated strategy driver
RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no
consistent online export of message payloads, so back up broker
DEFINITIONS (vhosts, users, permissions, queues, exchanges, bindings,
policies, parameters) over the management HTTP API and store them in the
system bucket. Message payloads and PITR are out of scope - a full-data
backup is a Velero volume snapshot, a separate strategy.
Add 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, push/pull to S3 with
--aws-sigv4) as a batch/v1.Job, for both in-place and to-copy restore.
- A Go-read precondition gates each run on the RabbitmqCluster reporting
AllReplicasReady (via the rabbitmqtypes partial type), surfacing a
precise Ready=False wait instead of a Job that starts and fails.
- The produced Backup records the object location on status.artifact.uri
(rendered from spec.artifactURITemplate; the Backup CRD has no status
subresource, so it persists on Create, as in the Etcd driver).
Wire dispatch in both controllers + supportedBackupStrategyKinds, register
rabbitmqtypes in the scheme, and grant read on rabbitmqclusters. Ship
cozy-default-rabbitmq (a $bucketName-gated Rabbitmq strategy) and route
apps.cozystack.io/RabbitMQ to it in the cozy-default BackupClass. The
application chart is untouched.
Add unit tests for the driver's pure helpers and the precondition gate, a
runnable example, and an honest-roundtrip Chainsaw e2e that seeds a
sentinel vhost, backs it up, drops it, and asserts it round-trips through
object storage on both restore flows.
The Job pod reuses the platform-migrations image (curl + ca-certificates)
under a new rabbitmqBackupClientImage key, stamped in lockstep with
chBackupClientImage so the repository stays pinned at one digest; a
dedicated slim image is a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Aleksei Sviridkin
|
c94a1da5bb |
feat(mongodb): add TLS support via PSMDB operator-managed certificates (#2692)
## What this PR does Enables operator-managed TLS for MongoDB and stops handing tenants any Secret that carries private key material. - Percona Server for MongoDB serves TLS unconditionally (`mode: preferTLS`), so this PR drops the misleading `tls.enabled` toggle rather than pretending the chart controls it. - The tenant no longer receives the operator's CA Secret (`<release>-ca-cert`, which holds `ca.key`) or the leaf (`<release>-ssl`). Both were previously reachable, and either one is a private key in tenant hands, the CA one is a full trust-chain compromise. - The chart declares a `TenantProjection` sentinel naming its CA source Secret (`<release>-ssl`, the operator-managed leaf, which carries `ca.crt`), and the ResourceDefinition selects the resulting key-free `<release>.tenant-ca` projection by the engine-agnostic `internal.cozystack.io/tenant-ca` label. The CA-extraction controller reads the sentinel and publishes the projection holding only `ca.crt`. The tenant gets what it needs to verify the server and nothing else. ### Why the include also carries an exclude list The trust-anchor selector matches by label with no name constraint, so any Secret in the namespace that acquired that label would be promoted with its data copied verbatim. The enumerable dangerous set is therefore excluded explicitly: the CA, both leaves, the rotation snapshots (`-ssl-old`, `-ssl-internal-old`, same server keys as the leaves, and unlike them they carry no owner reference, so an interrupted rotation can leave them behind after the release is gone), the system-user passwords, the at-rest encryption key, the keyfile, the object-storage credentials, and both forms of the operator's internal Secret. That backstop is bounded, and the comment says so: matching is by exact name with no globs, so a Secret a future operator version introduces, or a transient object an issuer creates mid-renewal, stays in scope of the label-only include. The per-user Secrets cannot be enumerated at all, since their names carry the username. The primary defence remains that the controller stamps only its own projection; this closes the enumerable set, not the whole surface. ### Release note ```release-note feat(mongodb): stop exposing TLS private key material to tenants and publish the CA as a key-free trust anchor ``` |
||
|
Aleksei Sviridkin
|
edd767ea0f |
feat(nats): publish ca.crt-only trust anchor to tenants (#3340)
## What this PR does Converges nats onto the key-free CA trust-anchor contract. A TLS-enabled nats release renders a `TenantProjection` sentinel naming the chart's cert-manager CA Secret as its source, and the nats ResourceDefinition selects the resulting `<release>.tenant-ca` object by label. The projection is built by the CA extraction controller in `internal/controller/cacert`, which parses the source certificate and re-encodes it into a single `ca.crt` key, so a tenant that enables TLS gets something to verify the server against without ever getting a private key. No key-bearing Secret is granted to a tenant, by name or by label. `packages/apps/nats/templates/certmanager.yaml` is not touched. An earlier draft of this change stamped publish labels onto the CA `Certificate`'s `secretTemplate`; the controller that shipped consumes a sentinel object instead, so the chart now declares its source rather than marking it. The functional change is one new template and one `matchLabels` entry in the ResourceDefinition. The sentinel is gated on the chart's effective TLS state, which is tri-state: left unset it follows `external`. That gate is the single divergence from the postgres implementation of the same contract, and it exists because the nats CA is only issued while TLS resolves true. An ungated sentinel would sit `Ready=False/SourceNotFound` forever on every plaintext release. The CA extraction controller is already on main, so this is live on merge rather than dormant until a follow-up lands. The `do-not-merge/hold` label predates that and no longer guards anything. ## Verified against a running cluster The premise this rests on is that cert-manager populates `ca.crt` on the Secret of a `selfSigned`, `isCA: true` Certificate. Nothing in this repository witnesses it: `hack/e2e-chainsaw/` has no nats suite, and nats appears in no e2e workflow, so every unit test here encodes the same assumption the chart encodes and the two cannot catch each other being wrong. I closed it against a live cluster instead, running cert-manager v1.20.2, the version this repo ships, on two independent nats releases: - `nats-<name>-ca`, the chart's CA Secret, holds `ca.crt`, `tls.crt` and `tls.key`. The declared source key exists. - `nats-<name>.tenant-ca`, the projection, holds exactly one key, `ca.crt`. - The projection's SHA-256 certificate fingerprint equals the source's, so it carries that CA and not some other certificate. - The projection carries `internal.cozystack.io/tenant-ca: "true"` and `internal.cozystack.io/tenantresource: "true"`. The second is stamped by the lineage webhook, computed from the ResourceDefinition selector this PR adds, so its presence means the whole chain held. - `kubectl get tenantsecret nats-<name>-ca` returns NotFound. The key-bearing Secret is not served through the tenant API. What is still missing is an automated regression witness for that premise. Adding one means a new chainsaw suite for nats, which has no e2e coverage of any kind today, and that is a larger change than this one. ## What the tests pin 61 helm-unittest cases and 3 bats cases. The sentinel's absence of `ownerReferences` (an owner would short-circuit the lineage walk before it reaches the Flux label the attribution depends on), the length of `spec.projections` as well as its single entry (the controller refuses a multi-entry sentinel, so a second entry is a silent loss of the anchor rather than a leak), both implied legs of the tri-state TLS gate, and `notExists` on either visibility label for both key-bearing Certificates. The bats guard covers the two grant surfaces nothing else asserts on. The ResourceDefinition is rendered, by the `nats-rd` chart inlining `cozyrds/*`, but that chart has neither a tests directory nor the `test` target `hack/helm-unit-tests.sh` looks for, and no `packages/system/*-rd` package does. The dashboard Role is rendered by a chart that has both, and no suite under `packages/apps/nats/tests/` asserts on it. Both are compared whole rather than filtered down to the entries that mention a Secret, because every such filter admits the spelling it did not anticipate. A name whitelist passes an added label selector, and `controller.cert-manager.io/fao` is stamped on both key-bearing Secrets, so a selector on it is a one-line grant of the CA private key. On the Role side, a rule with no `resourceNames` names no Secret, a rule with `resources: ["*"]` names no resource, and a ClusterRole is not a Role, while all three grant read access to the CA key. Comparing the rendered output whole needs no filter and leaves no spelling to find. ## Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No trigger fires, and no follow-up PR is owed. `packages/apps/nats/templates/tenant-projection.yaml` is a new template and `packages/apps/nats/tests/` holds chart tests. No package is added, renamed or removed; `values.yaml` and `values.schema.json` are untouched, so neither the hand-written Terraform schema, its version enums, nor its restated defaults have anything to track. `kind`, `plural` and `release.prefix` are unchanged, and no output Secret or Service is renamed, so the provider's concatenated name lookups still resolve. `packages/system/nats-rd/cozyrds/nats.yaml` is edited in place rather than renamed, so the ccp external-app skill's dependency-contract anchor still resolves. The provider offers no typed access to the new projection, which is the position it already holds for every Secret it does not model, and not a break. `packages/apps/nats/README.md` is covered by the one automated path that does exist: `nats` is present in the website `Makefile` `APPS` list, so the docs bot regenerates the managed-apps reference page from this `README.md` on the next stable tag. This is the silent-skip failure mode the trigger map warns about, so I checked the list rather than assuming; an app missing from it gets no page at all, which is what happened to `opensearch`. `hack/check-nats-rd-secrets.bats` is a new file, not a move or a rename, and it changes no make target's contract: `Makefile` globs `hack/*.bats` into `BATS_UNIT_FILES` automatically. The ccp skills anchor on `hack/package.mk` and `hack/common-envs.mk`, neither of which is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: Part of #2814. ### Release note ```release-note feat(nats): expose the CA certificate to tenants as a key-free `<release>.tenant-ca` Secret ``` |
||
|
Aleksei Sviridkin
|
65e700486b |
feat(qdrant): publish ca.crt-only trust anchor to tenants (#3341)
## What this PR does Converges qdrant onto the key-free CA trust-anchor contract (#2814), the same way #3340 does it for nats. The chart declares a `TenantProjection` sentinel naming its cert-manager CA Secret. The CA-extraction controller reads that declaration and publishes a key-free copy, `ca.crt` and nothing else, as `<release>.tenant-ca`. The qdrant ResourceDefinition selects that projection by label so it shows up in the tenant's secret list. No key-bearing Secret gains a tenant-visible label. The sentinel is gated on the chart's TLS condition. Engines whose operator mints a CA unconditionally do not need that, but qdrant only has a CA while TLS is on, and an ungated sentinel would sit `Ready=False`/`SourceNotFound` on every plaintext release. This was held while the CA-extraction controller was still open. That controller landed in #3407, so the sentinel is honoured as soon as this merges and the hold is gone. Tests pin the invariants that otherwise fail silently. The sentinel carries no `ownerReferences`, so the lineage walk falls through to the Flux release label. Neither key-bearing Certificate stamps a tenant-visible label on its Secret. The two objects that decide which Secrets a tenant may actually read, the dashboard Role with its binding and the ResourceDefinition, are pinned whole rather than checked for known-bad names: a rule granting secrets with no `resourceNames` is unrestricted rather than narrow, and a label selector can reach a key-bearing Secret while every name in the file stays correct, so neither adds a name for a probe to notice. Part of #2814. ### Follow-ups What this PR does not do, and where each belongs instead. - nats converges the same contract in #3340 and has no `dashboard-resourcemap_test.yaml`, so the dashboard Role pin belongs there too. Tracked in #3701 along with postgres, which already carries the label selector with no guard behind it. - The qdrant e2e fixture runs plaintext (`hack/e2e-chainsaw/qdrant/qdrant.yaml` sets `external: false`), so nothing in CI walks the path from the sentinel to `tenantsecrets`. postgres has a `verify-tenant-ca-projection` step to copy. - `hack/check-qdrant-rd-secrets.bats` is engine-agnostic apart from two literals, the chart path and the expected Secret set. The third copy of it is the point to parameterise over `packages/system/*-rd/cozyrds/` instead of copying again. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. Nothing needs a manual follow-up, but not for the reason a values-only walk would give: the website reference page for an app is regenerated from that package's `README.md`, and this PR does change it. `qdrant` is in the app list hardcoded in the website `Makefile`, so the docs bot picks the new section up on the next stable tag and there is nothing to open by hand. The Terraform provider's schema is hand-written against `values.schema.json`, which is untouched, and the provider offers no typed access to the new projection, the same position it holds for every other Secret it does not model. No package is added, renamed or removed, and the `cozyrds` file is edited in place rather than renamed, so the ccp dependency-contract anchor still resolves. User-facing documentation for consuming the trust anchor ships with the chart README, matching #3340. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(qdrant): expose the CA certificate to tenants as a key-free `<release>.tenant-ca` Secret ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Tenant environments can securely access the Qdrant TLS CA certificate without receiving the private key. * TLS CA access is automatically enabled when TLS or external access is configured. * **Documentation** * Expanded TLS guidance explains certificate validation and provides commands for retrieving the tenant CA certificate. * **Tests** * Added coverage for TLS projections, secret visibility, dashboard resources, and secure fail-closed secret exposure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8b6867dc47 |
fix(mongodb): correct the trust-anchor prose and two e2e assertions
The name-clash note claimed the shared TLS helper documents <release>-ca-cert as the conventional key-free anchor. It documents the opposite: the canonical name is <release>.tenant-ca, and it names this engine as the reason, because MongoDB already claims <release>-ca-cert with a key-bearing Secret. There is no clash to warn about and the chart follows the convention rather than deviating from it. The paragraph above already says <release>-ca-cert holds the CA private key, so drop the note instead of inverting it. Guard the ownerReference assertions against an absent list. length() of null is a type error rather than a failed comparison, and the harness returns it as an error, which aborts the poll instead of retrying — so the timeout never applies while the operator is still patching the reference on. Restore the two-variable form of the fingerprint comparison. Inlining both command substitutions into the test swallows a non-zero exit: with no openssl on PATH both sides are empty, they compare equal, and the check passes having compared nothing. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
01cfb5fda0 |
fix(mongodb): correct what a missing extraction controller does
The README said the sentinel would sit inert with nothing else affected. The CRD is a template of the controller's own chart, so a cluster without the controller has no TenantProjection kind and the release fails to render at all. The caveat also guarded a state that cannot occur, since the controller is a mandatory platform component — drop it rather than restate it, matching the postgres chart, and keep the paragraph explaining the choice of source Secret. Enumerate all three key-bearing Secrets in the registry negative. Two of the three were listed, which leaves the third free to acquire the label and leak a key, the exact reasoning the step above it already spells out. Collect TenantProjection status on failure. The controller reports SourceNotFound and its siblings there, and without it a failed projection surfaces only as an absent Secret with no reason given. Rename the source-shape step, which no longer looks at the CA Secret, and cross-reference the two halves of the leaf Secret's name. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c0b2ddcdf7 |
test(mongodb): assert the published trust anchor end-to-end
The existing steps only assert that key-bearing Secrets are NOT promoted to the tenant, and every one of those is satisfied by an absent label — they hold just as well if nothing was ever published. Add the positive: the projection exists, carries exactly ca.crt, matches the source certificate by fingerprint, and is reachable through the tenantsecrets registry rather than only as a raw Secret. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
29d2955241 |
fix(mongodb): declare the trust anchor with a TenantProjection
A tenant verifying a MongoDB endpoint needs the CA certificate, and every Secret that holds one also holds a private key: the operator's <release>-ca-cert carries the CA key, the leaf <release>-ssl the server key. Granting either would hand over the key along with the anchor. Render a TenantProjection naming the source, so the CA-extraction controller can publish the key-free <release>.tenant-ca, and select that projection by label from the managed-service definition. The sentinel carries no ownerReferences: it is resolved back to its release through the label Flux stamps, and an owner reference short-circuits that walk and silently costs the tenant its anchor. The sentinel is ungated. The operator issues the certificate chain whatever the chart asks for, so the source exists for every release and there is no plaintext leg to skip. The source is <release>-ssl rather than <release>-ca-cert. It is the name this chart itself sets in spec.secrets.ssl, so the reference cannot drift from the CR; the operator writes it on both its cert-manager path and the self-signed fallback it uses when cert-manager is absent, where no <release>-ca-cert exists at all; and it holds the merged old and new CA through a rotation, where <release>-ca-cert holds only the new one and would stop verifying pods still serving the previous leaf. Withhold the key-bearing Secrets from the tenant explicitly as well. The label selector matches on no name, so a single mis-stamp would promote a private key; the exclude list is evaluated first and closes the enumerable set. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ea7bc00016 |
docs(mongodb): correct the TLS secret ownership and external SAN claims
The uninstall section promised that no manual cleanup of the TLS Secrets is needed, reversing the instruction it replaced. That promise rests on an ownerReference the operator patches on at reconcile time rather than sets at creation, and two paths skip the patch: a dry-run reconcile returns before it, and a Secret already present under the same name with a mismatched cert-manager.io/certificate-name annotation is counted toward success and returned on unpatched. The deletion finalizer does not cover the gap — it enumerates only the users, internal-<release>, internal-<release>-users and encryption-key Secrets. So a Secret predating the release at one of these names can stay unowned and survive uninstall, and for <release>-ca-cert that orphans the CA private key. State the reconcile-time mechanism and tell operators to confirm the CA Secret is gone. Pin the outcome with a chainsaw step asserting the CR owns all three TLS Secrets on a normal install, so the claim goes red if the operator stops patching instead of rotting as prose. The external-hostname note described a permanently empty _namespace.host as a bootstrap window that self-heals. A tenant with no host of its own and no ancestor supplying one resolves to an empty host for good, leaving external TLS hostname verification silently unavailable. Name that case alongside the transient one, and say why the render is not failed closed: it would break existing hostless tenants already running external access over IP. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
27f71bfbe9 |
fix(mongodb): deliver the TLS trust anchor without private key material
The tenant-facing paths handed over both Secrets the PSMDB operator creates for TLS. <release>-ca-cert holds the CA private key, so read access is a full trust-chain compromise; <release>-ssl holds the server private key, so read access allows impersonation. A client verifying the endpoint needs ca.crt and nothing else, so neither object belongs on a tenant-facing path. Declare the trust anchor by name instead. The operator creates its CA Secret itself and offers no way to label it, so the ApplicationDefinition names it and the CA-extraction controller lifts ca.crt alone into the key-free projection. The include selector matches that projection by label, and the dashboard Role drops both key-bearing names without gaining the projection by name: a name grant conveys whatever occupies the name, while the label path conveys only what the platform vouched for. Witness both delivery paths against a running cluster, since the two that matter are separate. The Role is asserted as an exact grant with bounded rule counts, because a rule with no resourceNames — or one granting every resource — would otherwise pass an absence check while conveying every Secret in the namespace. The label path is asserted as a negative on all three key-bearing Secrets. A bats check pins the declaration's names and keys, which nothing else covers: the field is pruned at admission until the controller ships, so drift there would surface as an empty trust anchor with nothing red. Drop the mongodb-rd unit test. That package has no test target, so the file never ran, and what it asserted was the leak. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrei Kvapil
|
df1345f56d |
test(valkey): port e2e to chainsaw suite
The app e2e harness on main is chainsaw (hack/e2e-chainsaw/<app>/), not the older bats layout. Add hack/e2e-chainsaw/valkey/ mirroring the redis suite so select-e2e.sh maps the valkey-application source to the valkey suite, and drop the misplaced hack/e2e-apps/valkey.bats. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
440c3b58f3 |
test(valkey): add e2e test and wire build, labeler and docs
Add the Valkey e2e bats test (mirrors redis), build the compat image in the top-level build target, map the valkey scope to area/database in the PR labeler, and list valkey among charts with an immutable storageClass. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrey Kolkov
|
b9a95a0c4a |
fix(postgres): let in-place restore re-bootstrap instead of wedging (#3959)
## What this PR does An in-place `RestoreJob` (one whose `targetApplicationRef` is omitted, so the app is restored back into itself) re-bootstraps the Postgres app under its own name. On stock v1.6.2 it never comes back up — the cluster is deleted and then hangs in `Setting up primary` with `*-full-recovery-*` pods crash-looping, or the HelmRelease upgrade fails outright. There are two independent causes, both in the same restore patch, and both surface only in place because a to-copy restore targets a different app. **1. Archive-vs-recovery `serverName` collision.** The driver set the restored cluster's WAL-archive `serverName` equal to the recovery source (`bootstrap.serverName = bootstrap.oldName = sourceServerName`), while the chart pinned the archiver `serverName` to the release name. In place those coincide, so the cluster archived onto the source's own WAL prefix and `barman-cloud-check-wal-archive` refused to start with `Expected empty archive`. Fixed by giving the restored cluster a fresh `bootstrap.newServerName`, keyed by the `RestoreJob` UID (unique per restore, stable across reconciles); the chart archives there while `externalClusters` still recovers from `bootstrap.serverName`. This is what manual `CNPG` strategies hit (their `serverName` defaults to the cluster name, including the shipped `examples/backups/postgres/10-cnpg-strategy.yaml`); the `cozy-default` flow dodged it because its `serverName` is `<namespace>-<app>`. **2. `useSystemBucket` guard.** The restore populates explicit S3 coordinates (`destinationPath` / `s3CredentialsSecret` / `endpointCA`) but left `backup.useSystemBucket=true`, so the chart's `bootstrap.enabled is incompatible with useSystemBucket` guard failed the HelmRelease upgrade and the cluster never re-rendered. This is what the default `cozy-default` (system-bucket) flow hits on an in-place restore. Fixed by clearing `useSystemBucket` in the restore patch, since the app is now on the explicit-creds path. Verified live on a v1.6.2 cluster: with `useSystemBucket` cleared, the same in-place restore re-bootstraps to a healthy cluster and the sentinel row survives. Both are exercised only by an in-place restore; the postgres backup e2e drove only to-copy, so neither surfaced in CI. A step 50 in-place restore now guards cause 1 (manual-strategy flow); cause 2 is covered by a Go unit assertion (a system-bucket in-place e2e needs the platform bucket wired in CI and is left as a follow-up). This is a bug in released v1.6.2 (chart and driver are byte-identical on `main`), so it is a backport candidate. ### Screenshots Not applicable (no UI changes). ### Downstream repositories The only externally visible schema surface is one new field, `spec.bootstrap.newServerName`, in the postgres `values.schema.json` / CRD; it is set by the CNPG backup driver on restore and never authored by tenants, exactly like the existing `bootstrap.serverName` / `bootstrap.oldName`. (The `useSystemBucket` clearing is internal to the Go driver — `backup.useSystemBucket` already exists in the schema.) The chart `README.md` is regenerated from `values.yaml` automatically by the website docs bot on release, and `terraform-provider-cozystack` models user-authored spec (its guard tracks top-level fields, not driver-set nested ones), so neither needs a change in practice. Leaving every box unticked per the trigger-map guidance so a maintainer confirms rather than acting on a speculative follow-up. - [ ] 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(postgres): in-place restore no longer wedges — the restored cluster archives WAL under a fresh serverName distinct from the recovery source, and a system-bucket app is switched to explicit credentials so the bootstrap/useSystemBucket guard no longer fails its HelmRelease. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for specifying a distinct WAL archive destination when restoring PostgreSQL clusters. - In-place restores now automatically use a fresh archive location while continuing to recover from the original source. - Added configuration and schema support for the optional `bootstrap.newServerName` setting. - **Bug Fixes** - Prevented archive conflicts that could cause in-place restores to fail. - **Documentation** - Documented the new restore archive configuration and behavior. - **Tests** - Added coverage for unique restore destinations and in-place restore validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
c88da18139 |
test(tests): keep trace switch suite ASCII-only
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
e949f39800 |
fix(e2e): preserve previous-log cap slots
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
76d397523b |
docs(tests): record why an INT/TERM trap cannot cover a killed test
The comment added with the trace switch pointed at a signal trap as the fix for the one case quiet mode does not cover: a test killed rather than failed never reaches the fail handler, so its trace is gone rather than already printed. That advice is wrong, and wrong in a way that costs whoever follows it. POSIX sh defers a trap until the current foreground command returns, so installing an INT/TERM handler stops the runner dying on the signal at all. Measured: a suite hung on `sleep 30` exits in 30s with the trap installed against 2s under the default disposition. GitHub's cancellation escalates INT, TERM, then KILL, so the deferred handler gets SIGKILLed before it runs -- no dump, and the grace period spent waiting for a hang. Built it, measured it, reverted it. Making it work means running the test pipeline in the background and waiting on it, because `wait` is the interruptible one. That is surgery on the path every suite in this repo takes, unit and e2e, to recover a partial trace in a rare case that already has a 48-second escape hatch. Not taken; recorded so the next reader does not spend the afternoon finding out the same way. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a5c90d984e |
fix(tests): guard the finishedAt leaf and make three guards actually bite
Findings from two independent adversarial reviews of the preceding two
commits. Each was reproduced locally before being fixed.
The ordering inverted itself on a shape I had not probed. `finishedAt`
is a metav1.Time behind `json:",omitempty"`, which cannot omit a
struct, and its 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). Two guards deep left the leaf exposed, and kubectl
renders it as the literal `<no value>` -- which sorts ABOVE every digit,
so that row took the front of the queue from the most recent real
restart, and the header printed `previous instance ended <no value>`, a
claim about a time nothing observed. Verified with a text/template probe
over seven lastState shapes: `null` and a missing key both produced it.
Fixed at both layers, because they fail independently. The template now
guards the leaf, so the string cannot be produced. And the sort
normalises any field-6 value not shaped like a timestamp to empty, so
the ordering stays correct and the header stays quiet even if some other
producer emits one. Earlier verification against a live cluster showed
no `<no value>` and I read that as the guard being right; it only meant
no container on that cluster was in that state.
Three guards did not bite:
- both stability fixtures were pre-sorted ascending, which is exactly
what sort's last-resort whole-line compare reproduces with no -s at
all, so the tests written for -s passed without it. Reversed; they
now fail when -s is dropped.
- no test fed a six-field row through the whole script, so the
timestamp reached the dump header uncovered -- deleting the read
variable left the suite green at 39/39. One integration test now
asserts the header names the time.
- the e2e guard matched any mention of COZYTEST_TRACE, so documenting
the design over there would have turned it red. It now matches an
assignment, and tolerating a comment is verified too.
Also: a failed sort no longer discards a whole namespace group. The
caller compares row counts to report the cap, so losing a group silently
became "0 of N captured, N more not captured" and sent the reader after
a cap that never fired. It falls back to discovery order and says so.
LC_ALL=C on the sort, since the key is fixed-format ASCII.
And the claim itself was too strong. "Nothing diagnostic is lost" holds
only for a test that RETURNS: the fail handler runs after the pipeline,
and there is no INT/TERM trap, so a test killed rather than failed -- a
hang reaped by the job timeout -- never dumps its log, and under quiet
mode its trace is gone rather than already printed. The comments in
hack/cozytest.sh and the Makefile now say so and name the escape hatch.
A trap that flushed the tail of the log would close it; left out
deliberately rather than bundled in here.
Every fix is mutation-proven: reverting the leaf guard, removing the
key normalisation, dropping the read variable, dropping -s, and
silencing an e2e recipe are each caught. Unit lane 1513 passed, 1 failed
(hack/ghcr-mirror_test.bats, which reads a live cluster and fails the
same way under main's runner).
Assisted-By: Claude <noreply@anthropic.com>
Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
599ace69fd |
test(tests): guard the COZYTEST_TRACE switch against silent regression
The quiet unit lane rests on one invariant: run_one tees the raw stream
to $log and the fail handler prints all of it for whichever test fails,
so suppressing the live stream drops the output of PASSING tests only.
That invariant fails silently. It is exercised only when a test fails,
so a green CI run proves nothing about it - a refactor of run_one's
reader loop that also broke the dump would ship, and would be found by
whoever next needed a red unit run diagnosed, with no trace and the run
already lost. Nothing in the tree covered it.
Seven tests, fixtures that fail on purpose, following the structure of
hack/cozytest-capture-gate.bats:
- quiet mode streams no trace for a passing suite
- the unset default stays verbose, which is what the e2e call sites in
packages/core/testing/Makefile rely on
- a failing test still dumps its whole trace under quiet mode, xtrace
included, which is the load-bearing one
- quiet mode still fails the run
- the per-test start/end markers survive, so a hang still names the
test that never returned
- the unit recipe asks for the quiet runner
- the e2e recipes do not silence theirs
Every one is mutation-proven: removing the fail-handler dump, flipping
the default, making the switch a no-op, dropping the variable from the
unit recipe, adding it to an e2e recipe, and swallowing the exit code
are each caught by exactly one test. The last of those is worth a note -
it disables the mechanism the outer runner uses to report failure, so
the run exits 0 while the failure is in its output. A probe keyed on the
exit code alone reads that as uncaught.
Two traps are recorded in the file's header rather than left to be
rediscovered. Fixtures are built with printf because the awk transform
harvests a heredoc's own @test line into the suite that contains it. And
the default is asserted under `env -u COZYTEST_TRACE`, because the
Makefile passes the variable as a command prefix, which exports it - a
nested runner started from a suite that `make unit-tests` is running
would otherwise inherit 0 and the default would never be under test.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
7741843178 |
test(e2e): order previous-instance log capture by restart recency
The container cap (COZY_PREVLOG_MAX=12) is spent in namespace order, and "unrelated" has meant only "in another namespace" - time was never a key. So a container that restarted once during cluster 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, restarted once at install time by `Unable to create certwatcher` hours before any app suite ran; on a development cluster 178 containers carried a restart, their timestamps spanning 24 days, so the cap binds hard enough for the order to decide everything. Ordering, not filtering, and deliberately: an install-time restart IS the explanation when the install is what broke, and this capture has no reference point for "when the test began" that would make a cutoff honest. Namespace stays the primary key - a stale restart in the namespace under test beats a fresh one three namespaces away. So this fixes cap displacement, not the raw volume of any single run. lastState.terminated.finishedAt joins the row, guarded so a missing lastState cannot error the list call and fail the capture open (verified against a live cluster: no <no value>, empty where the kubelet has already dropped it). Reading it also fixes a latent bug the sixth field would otherwise have caused - `read -r ... restarts` folds a trailing field into the last variable, so the dump header would have printed `restarts=1|2026-08-22T09:06:00Z`. It now names the time instead, which is the first thing that decides whether the dump is worth reading. Sorting is lexicographic over fixed-width RFC3339 UTC, so no date parsing: reverse puts newest first and leaves undated rows last, and -s keeps a node reboot's simultaneous restarts in input order so the selection is comparable between runs. Rows with no timestamp pass through untouched, which is what makes this a no-op for the existing five-field fixtures. Five tests added; both mutations are caught (a passthrough sort_recent, and dropping -s - the latter by the pre-existing namespace test). capture-previous-logs 39/39, cozyreport 184/184, bats-no-exit-trap 22/22. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
cb60f743d3 |
test(ci): extend the backport label contract to both spellings
The concurrency-group key and prepare's `if:` gate now name four labels rather than two, and release-freeze-contract.bats pinned the pair on both sides — as a literal and as an exact count. Every namespaced name appended to the workflow therefore reported as a widening, which is the right default: the count exists because the literal is a substring check and an appended exclusion satisfies it unchanged. Here the appended names are not a widening. `kind/backport` and `kind/backport-previous` are genuine backport requests, so they belong on the same side of the concurrency split as the legacy pair, and the gate has to admit them or the rename stops the bot firing. Both counts go to four and both literals name all four operands, so a half-finished rename still reports. Marked TRANSITIONAL at each site, matching the comment in backport.yaml: when dosubot's PR labelling is switched off and the legacy arms are dropped, these go back to the pair in the same pass. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
fdb2e89fe7 |
test(unit): make the bats trace stream opt-in, quiet on the unit lane
hack/cozytest.sh runs every @test body under `set -x` and streams the trace live. Across the ~40 hack/*.bats unit suites that is 233,444 of 238,794 lines - 98% of a 31 MB job log - and the single failing assertion sits at line 238,573 of it. The web log viewer is unusable at that size, and anyone (or anything) reading the job to find out what broke pays for the whole file first. Nothing in that stream is load-bearing on the unit lane. run_one already tees the raw output to $log and the fail handler dumps all of it for whichever test fails, so suppressing the live stream drops the output of PASSING tests only. Add COZYTEST_TRACE, default 1 so the e2e call sites in packages/core/testing/Makefile keep the live stream - there it 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 can run. The bats-unit-tests target sets 0; running the runner by hand stays verbose, which is the case where a human is actually watching. Measured on 25 suites locally, quiet vs verbose: 29,528 -> 951 lines, 3.9 MB -> 94 KB, and 71s -> 48s wall, the last because the per-line formatter forks date twice for every line it prints. Quiet output is byte-for-byte the non-trace subset of verbose, and a hanging test still shows as a ╭ with no matching ╰. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
scooby87
|
9dd2841c74 |
feat(clickhouse): add configurable version parameter (#3476)
## What this PR does The ClickHouse server and Keeper images were hardcoded to `24.9.2.42` in the chart templates, so the chart exposed no top-level `version` values key. The Cozystack API returns a managed database's engine version by passing the HelmRelease's `spec.values.version` straight through into the Application object, so ClickHouse instances carried no `spec.version` in the API — unlike postgres, mariadb, mongodb, opensearch, rabbitmq and redis, which all ship a `version` parameter. This closes the ClickHouse half of the gap tracked in #1246. This adds the same version-selection scaffolding the other managed-database charts already use: - `files/versions.yaml` maps a `major.minor` key to the full image tag, shared by the ClickHouse server and Keeper images (both are pinned to one version). - `templates/_versions.tpl` (`clickhouse.versionMap`) resolves the tag and fails the render on an unsupported version. - `values.yaml` gains a `version` enum (`v25.8`, `v25.3`, `v24.9`), consumed by `templates/clickhouse.yaml` and `templates/chkeeper.yaml`. - `hack/update-versions.sh` + `make update` refresh the map from Docker Hub, keeping only tags published for *both* the server and Keeper images and preserving the current default so a regeneration never bumps the major of existing deployments. The default stays `v24.9` → `24.9.2.42`, so existing installations render byte-for-byte identically and are never upgraded to a new major implicitly. The generated `values.schema.json`, `README.md`, the Go API type and the `clickhouse-rd` `openAPISchema` are regenerated with `cozyvalues-gen` v1.6.0. A new `tests/version_test.yaml` helm-unittest suite covers the server/Keeper mapping for each version and the schema-level rejection of an unsupported value. ### Downstream repositories The diff changes `packages/apps/clickhouse/values.schema.json` by adding a new `version` field with an `enum`. Per the trigger map, this reaches `terraform-provider-cozystack`, whose ClickHouse resource is hand-written and would need a matching `version` attribute with a `stringvalidator.OneOf` list and its expand/flatten pair. There is no existing PR or issue there for it yet. This is left for a maintainer decision rather than a speculative cross-repo PR, so the box below is intentionally left unticked and flagged here. No other downstream repository is reached (the website reference page is regenerated from `README.md` by the release docs bot; ClickHouse is already in its app list). - [ ] 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: needs a `version` attribute on the ClickHouse resource (maintainer decision — see note above) - [ ] [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(clickhouse): add a configurable `version` parameter (v25.8, v25.3, v24.9; default v24.9) so the deployed ClickHouse version is selectable and returned through the API. Existing installations are unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a configurable ClickHouse version setting for both server and Keeper images, defaulting to `v24.9`. * Supported versions are `v25.8`, `v25.3`, and `v24.9`, with compatible image patches selected automatically. * **Documentation** * Documented the version setting, default, and supported values. * **Validation** * Unsupported versions are rejected with an error listing allowed options. * **Tests** * Added coverage for version mapping, defaults, invalid values, and update behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
f5b957ee27 |
feat(kubernetes)!: manage worker pools as KubernetesNodes, adopt on upgrade (Phase 2, breaking) (#3315)
## What this PR does Phase 2 of the Kubernetes-app split ([cozystack/community#8](https://github.com/cozystack/community/pull/8)) — the **breaking** half. Stacked on #3314 (review that first). Removes worker node pools from the `kubernetes` chart (they moved to `kubernetes-nodes` in #3314) and adopts existing pools on upgrade: - `kubernetes` chart is now **control-plane only**: drops the pool `range` block, the `talos-reconcile` Job + RBAC (deleted; moved to `kubernetes-nodes`), the preserve-old-KMT block, the `kubernetes.nodeGroups` helper, and the per-group dashboard RBAC. - **`spec.nodeGroups` is removed from the `Kubernetes` CR** (BREAKING). Worker pools are now separate `KubernetesNodes` resources. - **Migration 54** (bumps `targetVersion` to 55): pre-upgrade hook that, for every `kubernetes-<cluster>` release, creates one `kubernetes-nodes-<cluster>-<pool>` HelmRelease and re-annotates the existing worker objects onto it with `helm.sh/resource-policy=keep`, so the now control-plane-only parent upgrade does not prune running workers. The default `md0` (implicit when `nodeGroups` was empty) is materialised explicitly. Idempotent. Genuine infrastructure/read failures fail closed (`exit 1`, retried on the next upgrade); hostile stored shapes (a pool name over Helm's 53-char release-name cap, an RFC-1123-invalid or whitespace/slash map key, a non-object `nodeGroups`) are warned, pinned, and skipped rather than deadlocking the entire fleet's upgrade. - The `ingressNginx` addon no longer auto-provisions a node (the implicit `md0` default is gone). If enabled without a pool carrying `roles: [ingress-nginx]`, the controller DaemonSet simply schedules nothing (degraded, not failed) — the requirement is documented in the chart NOTES.txt rather than enforced at render time. Because the child chart renders the pool objects **byte-identically** to the old parent render (golden parity in #3314; the KubevirtMachineTemplate content-hash name is preserved), adoption does not churn live worker VMs. ### Migration number `54` is contiguous with main (latest is `53`) and `targetVersion` is `55`. The `52`/#3201 collision noted in earlier revisions has been resolved: this branch was rebased and merged past it, so the numbering is contiguous and `run-migrations.sh` has no gap to hard-fail on. ### Not yet done (blocking draft → ready) - [ ] **dev e2e upgrade validation** (`cozystack-pr-test`): upgrade a custom-`nodeGroups` cluster and a default-`md0` cluster; assert zero worker-VM churn (KMT hash, Machine uid/creationTimestamp stable) and that the adopted child HRs reconcile. - [ ] downstream trigger-map walk (website docs for the `nodeGroups` → `KubernetesNodes` change; terraform-provider for the removed API field). ### Downstream repositories Deliberately unticked (draft) — the breaking API change affects docs and the terraform provider; follow-ups to be filed before ready. - [ ] 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: ```release-note Worker node pools of a managed Kubernetes cluster are now managed as separate `KubernetesNodes` resources. `spec.nodeGroups` on the `Kubernetes` CR is removed; existing pools are adopted automatically on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Worker pools are now separate `KubernetesNodes` resources with their own lifecycle. * Added a render-time compatibility guard to prevent worker pools from running ahead of the parent control-plane minor version. * Improved upgrade/uninstall behavior to better preserve already-adopted worker objects. * **Breaking Changes** * Removed per-pool `nodeGroups` and worker node health settings from the `Kubernetes` chart/CR; pool configuration now lives in `KubernetesNodes`. * Ingress requires at least one `KubernetesNodes` pool with `roles: [ingress-nginx]`. * `cluster` and `storageClass` are immutable after creation; pool `version` may lag but must not be ahead of the parent minor. * **Documentation** * Updated READMEs, notes, and schemas to reflect the `KubernetesNodes` split and the immutability/version rules. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
e707475f96 |
fix(clickhouse): fetch tags via skopeo and fail closed on a missing pin
Address review of the version generator (hack/update-versions.sh): - Fetch tags with `skopeo list-tags` (registry v2 /tags/list) instead of the Docker Hub v2 API, whose anonymous pagination caps at 1000 entries and then returns an error page with no `.results`, silently truncating the list so the frozen 24.9.2.42 fell outside the window and the run failed. skopeo is already in build-deps and is how the sibling generators (postgres, redis, mongodb, opensearch, kubernetes) fetch. - Make a frozen default tag missing from the registry a hard error instead of silently falling back to a newer patch, which would move the default image for existing installs. - Restore 0644 on the regenerated files (mktemp's 0600 survived the mv). - Exclude /hack from the chart tarball via .helmignore. - Refresh files/versions.yaml: v25.8 -> 25.8.32.4, the true latest 25.8 patch now visible with the complete tag list. - bats: add a missing-frozen-tag hard-error case and make the atomicity test reach the resolution phase after a valid default rather than failing at the default-in-set check. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Andrey KolkovandClaude Opus 4.8
|
5bcdb8cfb8 |
fix(postgres): let in-place restore re-bootstrap instead of wedging
An in-place RestoreJob (targetApplicationRef omitted) re-bootstraps the
Postgres app under its own name. On stock v1.6.2 it never comes back up,
for two independent reasons in the same restore patch - both surface only
in place, because a to-copy restore targets a different app.
1. Archive-vs-recovery serverName collision. The driver set the restored
cluster's WAL-archive serverName equal to the recovery source
(bootstrap.serverName = bootstrap.oldName = sourceServerName) while the
chart pinned the archiver serverName to the release name. In place those
coincide, so the cluster archived onto the source's own WAL prefix and
barman-cloud-check-wal-archive refused to start ("Expected empty
archive"). Fixed by giving the restored cluster a fresh
bootstrap.newServerName, keyed by the RestoreJob UID (unique per
restore, stable across reconciles); the chart archives there while
externalClusters still recovers from bootstrap.serverName. This is what
the manual CNPG strategies hit (serverName defaults to the cluster
name); the cozy-default flow dodged it (serverName is <ns>-<app>).
2. useSystemBucket guard. The restore populates explicit S3 coordinates
(destinationPath / s3CredentialsSecret / endpointCA) but left
backup.useSystemBucket=true, so the chart's "bootstrap.enabled is
incompatible with useSystemBucket" guard failed the HelmRelease upgrade
and the cluster never re-rendered. This is what the cozy-default
(system-bucket) flow hits on an in-place restore. Fixed by clearing
useSystemBucket in the restore patch, since the app is now on the
explicit-creds path. The driver's app view (postgresapp.Backup) gains
the field, without omitempty so a false survives the merge patch.
Both are exercised only by an in-place restore; the postgres backup e2e
drove only to-copy, so neither surfaced in CI. A step 50 in-place restore
now guards reason 1 (manual-strategy flow); reason 2 is covered by a Go
unit assertion (a system-bucket in-place e2e needs the platform bucket in
CI and is left as a follow-up).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
IvanHunters
|
c0fcdd767c |
fix(migrations): discover pre-chartRef parents via inline spec.chart in the restamp
The restamp discovered unlabelled parent kubernetes releases by spec.chartRef only, but that misses the population it exists for. The apps.cozystack.io/ application.kind label was introduced 2025-12-19 ( |
||
|
IvanHunters
|
c6b6755eec |
fix(migrations): record foreign-owner skips, harden record_skip, scope restamp to kubernetes- names
Three hardening fixes on migration 56's new code: - The foreign-owner branch in adopt_one pins the object prune-proof but does not adopt it (the child release can then stay Failed on the ownership conflict), yet it was the one warn+pin+skip path that did not record itself in the durable unadopted ConfigMap -- so an operator relying on that record to find stuck pools would miss it. Record it like every sibling branch. - record_skip ran a bare append under set -e inside skip branches that are designed never to abort; a failed write (e.g. /tmp full) could deadlock the fleet upgrade for one tenant. Make it fail-open, matching the branches' intent. - The restamp selector matched any release by chartRef; scope it to names starting with kubernetes- so it cannot mislabel (and then have the sweep skip) a non-standard release, keeping restamp and the sweep's contract in sync. Also fix the previously-vacuous negative restamp test (its fixture lacked the chartRef, so it never reached the label guard it claimed to check). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3d1a83f419 |
fix(migrations): make the unadopted-pools record publish best-effort
The durable-record ConfigMap write sat under set -euo pipefail right before the version stamp, so a permanent failure (e.g. the record exceeding etcd's object size limit at a very large skip count) would abort the migration and deadlock the fleet upgrade with no path out -- even though every skipped pool was already pinned prune-proof earlier in the sweep, so the workers are safe regardless of whether the record lands. The record is a convenience artifact, not a safety mechanism, so swallow its failure with a warning instead of failing closed. Covered by a bats case that fails the publish and asserts the run still stamps. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
952901776a |
fix(migrations): adopt unlabelled parent kubernetes releases and record unadopted pools in migration 56
The worker-adoption sweep selected parents by the apps.cozystack.io/ application.kind label, which postdates v1.2. A parent kubernetes HelmRelease older than that, or one restored by hand, lacks the label: the sweep skipped it, stamped, and the parent's control-plane-only re-render then pruned its un-pinned workers with no pin and no warning. Discover such releases by their chartRef instead and restamp the three application labels before the sweep (additive, cannot corrupt a release, and repairs it for cozystack-api too). Every warn+pin+skip branch leaves a pool pinned prune-proof but unadopted, recorded only in the Job log, which the next upgrade reaps; the migration does not re-run, so an operator cannot find those pools afterward. Accumulate the skips and publish them as configmap/cozystack-migration-56-unadopted before the stamp. Extend the fake kubectl and add bats coverage for the restamp (positive and negative) and the unadopted-ConfigMap paths. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
3c78c0be56 |
feat(clickhouse): add configurable version parameter
Add a `version` value (v25.8, v25.3, v24.9; default v24.9) to the ClickHouse chart, backed by files/versions.yaml and resolved by templates/_versions.tpl for both the ClickHouse server and Keeper images, so the Cozystack API returns the engine version (spec.version) like the other managed-DB charts. Default v24.9 -> 24.9.2.42 keeps existing installations byte-for-byte identical. The maintainer-run hack/update-versions.sh generator intersects the server/keeper Docker Hub tags byte-collated (LC_ALL=C) so comm keeps tags across the X.9 -> X.10 boundary; freezes the default major to the tag it already ships (read from versions.yaml, so the freeze follows the default); errors on a dropped default or an unresolvable major; writes atomically; and runs on stock macOS bash 3.2 / BSD. Covered by hack/clickhouse-update-versions_test.bats (10 cases) and the tests/version_test.yaml helm-unittest suite. Refs: #1246 Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Myasnikov Daniil
|
a94abe0971 |
test(e2e): record the runner's SVM feature flags in the identity capture
Adds one line to the runner identity record, taken from the lscpu read it
already does:
svm features: svm npt nrip_save vgif
vgif decides whether a guest that is itself running a hypervisor pays a VM exit
on every CLGI/STGI. Here that is the sandbox Talos nodes hosting the tenant
KubeVirt VMs, so it only bites at double nesting, and the sandbox nodes' own
boot is unaffected.
Across the archived reports no run missing the flag has passed a
tenant-Kubernetes suite: 0 of 62, against 11 of 12 with it. Exposure varies
between hosts of the same shape, so it is not a property of the lane and cannot
be recovered once the runner is recycled. Recording it on every run, red and
green, is what makes the correlation countable rather than reconstructed by hand.
No new bounded read, no compiler on the runner, no sysfs. Gated on svm so an
Intel host's vnmi is not reported as an SVM feature.
Five tests for the reader. Identity suite 62 assertions, hack/ sweep 732.
No behaviour change and no gating on the flag.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Aleksei Sviridkin
|
ad62a8d62b |
test(e2e): stop blocking merges on the tenant node-join deadline
The tenant node-join wait no longer discriminates. Nested virtualisation on the shared runners degrades far enough that a guest which registers in minutes on real hardware can miss any deadline this test can afford, so a red on that one wait says as much about the machine the run landed on as about the product, and it teaches a reader to re-run the job. The suite still fails on it. That is what keeps Chainsaw's catch running: the previous-instance container logs, the host snapshot, the data-plane capture and the event dump are all keyed on the test failing, and the report stays honest. What changes is one layer up. The wait writes a marker beside its diagnostics, and a new lane-side gate reads that marker to decide whether the job blocks. It opens only when every failed test in the run carries one, and blocks on anything it cannot attribute: a report it could not read, a failed test the report does not name, a failure with no marker. Only the lanes that block a merge ask it. The two pull-request lanes do; nightly and the tag lane run the same suites and stay hard. A soft pass on the tag lane would ship a release candidate whose tenant clusters brought up no worker nodes, and nightly is the only place an e2e failure reaches anyone who did not open a pull request, since no lane in this tree notifies anywhere. A softened red finishes the job green, so the job's result stops answering whether the suite passed, and both kinds of consumer are handed the decision instead of inferring it. A step gated on the job failing would silently stop running: the SSH breakpoint is that case, and it still opens for a pull request marked for debugging. A job that reports the result would silently start reporting a pass: the required commit status now says the deadline was missed and that the merge is not blocked on it, rather than that the suite passed. The deadline goes to 29m. That is the test's own budget and nothing more: its clock starts after the machinedeployment wait, the LoadBalancer assignment and the healthz probe, while the chart's autoscaler and health-check timers have been running since the Machine was created, so no ordering against them is claimed. The chart's 20m and 30m give a scale to pick against, and a budget over the first leaves room for a remediation cycle without promising that one fits. The operation ceiling both suites give the script rises with the deadline, and the diagnostics phase budget rises inside it, because the collectors gated ahead of the serial console cost more between them than the old budget covered. Both budget guards read their terms from the sources that set them. The e2e job cap rises with them, and its guard holds what a guard can hold: the pair's operation and catch ceilings, and one figure across every lane that runs the suite. It also records that the suites' full summed ceilings have never fit any cap in that file, so the margin above the bound is taken from a measured run instead. The prose that restates the cap elsewhere moves with it, and the sweep that holds the deadline's restatements now reads the figure spelled in words as well as in minutes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e6be333c06 |
test(e2e): capture CertificateSigningRequests in cozyreport
hack/cozyreport.sh never captured CertificateSigningRequests, leaving the hypothesis that a node join hangs on kubelet certificate issuance (bootstrap token, CSR, approval) unverifiable from the collected artifact. Add a bounded CSR list and a per object yaml capture for the host cluster, next to the existing node walk, and the same capture for every tenant Kubernetes cluster whose kubeconfig a failed kubernetes-* Chainsaw test left on disk. Both walks reuse the existing bounded read helpers, so a cut off or refused read produces the same truncation and failure markers every other capture in the file already writes, and an empty CSR list is not treated as an error. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |