mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-04 02:57:27 +09:00
feat/externalip-api
164
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
d685fb30ec |
fix(gateway)!: require tenant routes to declare their hostnames (#3475)
## What this PR does The tenant route hostname policy skipped the shape it most needed to check. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute or TLSRoute declaring no hostnames satisfied the first operand and got admitted with nothing compared against the tenant apex. An empty list passed too: `has()` is true on `[]`, and `all()` over an empty list is vacuously true. Such a route is not inert. Under Gateway API it inherits the hostnames of every listener it attaches to, and `parentRefs` can name a Gateway in another namespace, so it covers whatever that listener covers. Reported in #3467. The predicate now requires the field to be present and non-empty before validating it. Both policies rendered from that template share one CEL string, so HTTPRoute and TLSRoute get the change together, and both match `CREATE` and `UPDATE`, so an existing route that drops its hostnames is denied exactly as a new one is. The issue is titled after TLSRoute, but the reachable shape is mostly HTTPRoute. TLSRoute allows a hostname-less route only on the deprecated `v1alpha2`; at `v1` and `v1alpha3` the bundle this repo vendors marks `spec.hostnames` required and rejects an empty list on `minItems: 1`, while `v1alpha2` carries neither constraint. HTTPRoute `v1` carries neither either, so it accepts both shapes. All four are checkable in `packages/system/gateway-api-crds/templates/crds-experimental.yaml`. Checking that turned up a stale claim in the template header, which said TLSRoute had no `v1` in Gateway API 1.4 to 1.5 and used it to justify the rule naming only `v1alpha2`. Gateway API 1.5.1, which this repo vendors, serves TLSRoute at `v1` as the storage version. The rule still covers every served version because `matchConstraints.matchPolicy` defaults to `Equivalent`, and that default is now written down, because setting `matchPolicy: Exact` reads like a tightening and would silently stop the policy seeing `v1` and `v1alpha3` TLSRoutes. I corrected the text and left the coverage alone: the wrong sentence predates this branch, but my change is what makes the file contradict itself, so the text is mine to fix while widening version coverage is a separate job. A `ValidatingAdmissionPolicy` has no cross-object lookup, so it cannot resolve the parent Gateway to see which hostnames a route would inherit. Requiring the route to name its own is what keeps every admitted hostname comparable against the apex. That has a cost: a route relying on listener inheritance now has to spell its hostnames out, and a tenant that hand-writes one gets a denial where it used to get an admission. ### The controller change belongs in the same PR `renderHTTPRedirect` built the http-to-https redirect HTTPRoute with no hostnames on purpose, to catch every host on port 80. It lands in a `tenant-*` namespace, so the stricter policy denies it, `reconcileHTTPToHTTPSRedirect` returns that error, and since it runs before the status update the TenantGateway never reaches Ready. That redirect is what keeps app routes attaching by hostname from serving plaintext on port 80. Neither half of this change is safe alone. The redirect now names the tenant apex and its wildcard. Both entries are needed: a wildcard hostname is a suffix match over any number of labels, so `*.<apex>` covers subdomains at every depth but not the bare `<apex>`. `renderWildcardCertificate` already builds the same pair. Coverage narrows in one direction. A request on port 80 with a Host outside the tenant apex now falls through to a 404 instead of a 301. That narrowing costs no covered host: the port-80 listener admits routes only from the Gateway's own namespace and cert-manager's, and the tightened policy requires every route in the tenant namespace to declare its hostnames inside the apex, so any host published from this namespace already falls under the apex or its wildcard. An inheriting child tenant does not widen that set either. Its hostnames would reach port 443 through a listener on this same Gateway, and `cozystack-gateway-hostname-policy` compares every listener against the host label of the Gateway's own namespace, so a child apex outside that label never gets its listener: the Gateway write carrying it is refused at admission whole, in `reconcileGateway` and before the redirect is rendered. A child apex under this one passes that check and the wildcard already covers it. That holds at merge base as well, so it is not something this branch changes. Deriving hostnames from `spec.apex` makes that field load-bearing for admission, so it gains `MinLength=1`. An empty apex used to reconcile to Ready, since the redirect declared no hostnames at all; it would now render `""` and `"*."` and be rejected on the HTTPRoute hostname schema, surfacing as a validation error naming the route rather than the field. The renderer checks it too, because the CRD and the controller binary roll out separately and an older CRD in the cluster will not carry the bound. That schema bound is a breaking change to an existing field and the API review gate flags it as one, so here is what it actually invalidates. The chart that renders these CRs already fails on an empty `_namespace.host`, and in DNS-01 or existingSecret mode an empty apex already broke earlier, in `renderGateway`, on a listener with `hostname: "*."`. Nothing that works today becomes invalid. What the bound rejects is a TenantGateway written by hand, past the chart, with an empty apex, and I would rather that be refused at admission naming the field than at reconcile time in controller logs, which is why the bound sits in the CRD as well as the renderer. Naming those hostnames also meant excluding the redirect from `collectHostnameClaims`. A route that attaches to the Gateway and declares hostnames otherwise reads as an app publishing them, and each claimed hostname becomes an HTTPS listener named after its first label. The first label of `*.<apex>` is `*`, which the Gateway CRD's listener-name pattern rejects, so the Gateway write fails and the reconcile stops before anything downstream. The exclusion matches on namespace, name and controller ownership rather than on the labels the renderer stamps: those labels are writable by anyone who can create a route, and the claim set also drives conflict resolution, so a label-keyed exception would let any route opt out of being marked `Accepted=False` when it loses a hostname race. Name and ownership are each required and each is pinned by its own test, since ownership alone would exclude any other controller-owned route and name alone would extend the exception to a route planted under that name by someone else. ### Radius Only namespaces starting with `tenant-` are matched. The three first-party TLSRoutes the platform ships, `kubernetes-api` in `default` plus `cdi-uploadproxy` and `vm-exportproxy`, set hostnames explicitly and none sits in a `tenant-*` namespace, so nothing changes for them. I went through every producer of routes in the tree, in three places rather than one. No Helm template emits an absent or empty hostname list in any configuration a tenant can reach: the vendored openbao, harbor, opencost and flux-operator route templates can render one, but each is behind a flag that defaults off with no values path to turn it on, and openbao would fail CRD validation first because its default route apiVersion requires hostnames. Exactly one Go site builds a route, the redirect fixed here. And one e2e fixture did rely on the old behaviour: the Gateway suite's parentRef probe declared no hostnames and is applied into `tenant-test`, which the policy matches, so it now reads the apex from the namespace label and names a hostname under it, the same way the neighbouring cases in that suite already did. cert-manager also writes routes into tenant namespaces, since it creates the Challenge in the Certificate's own namespace and the HTTP-01 solver route beside it. That route declares the hostname it is validating, so the policy admits it. ### Related issues Two open issues have a text half that this PR happens to settle, and neither of them closes here. #3788 reports that the port-80 allow-list contains the namespace its own comment says it excludes; the comment and the README paragraph now describe what that filter actually does, while the behavioural exit it names stays open. #3789 reports that the TLSRoute rule names only the deprecated API version; that premise is now written down in the template header, including what has to change before a bundle drops `v1alpha2`, but the rule still names `v1alpha2` alone. #3787 covers the wildcard-hostname listener name that the apiserver rejects, which is what makes the claim-set exclusion here load-bearing rather than tidy. ### Screenshots N/A (no UI changes). ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No package is added, renamed or removed; no `values.yaml` key, `values.schema.json` field, version enum or default moves; no `kind` or `plural` changes; no namespace or release-asset name changes; nothing under `hack/` moves, is renamed, or changes what a make target does. One row deserves a sentence rather than a blanket denial, because the diff does change a CRD schema: `spec.apex` on `gateway.cozystack.io/v1alpha1 TenantGateway` gains a `minLength` bound, which lands in the generated `gateway.cozystack.io_tenantgateways.yaml`. The provider hand-types `Package` under `cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`; `gateway.cozystack.io` is not among them, and a tightening on an existing field adds no new field for it to offer typed access to. The `packages/extra/gateway/README.md` edit sits in the `## Security model` section, below the `## Parameters` block that `cozyvalues-gen` regenerates, so it survives `make generate`. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(gateway)!: the tenant route hostname policy now requires an HTTPRoute or TLSRoute in a `tenant-*` namespace to declare a non-empty `spec.hostnames`. A route that omitted the field, or set it to an empty list, was previously admitted without any hostname being checked against the tenant apex, even though such a route inherits the hostnames of the listener it attaches to rather than matching nothing. A route in a `tenant-*` namespace relying on that inheritance is now rejected and has to name its hostnames. The controller-owned http-to-https redirect route names the tenant apex and its wildcard accordingly, so a request reaching port 80 with a Host outside the tenant apex now gets a 404 instead of a 301. `TenantGateway.spec.apex` additionally gains a `minLength: 1` bound, because the redirect derives its hostnames from that field; a TenantGateway written past the chart with an empty apex is now rejected at admission instead of failing later in the controller. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added edge-terminated gateway support with plain-HTTP listeners and TLS passthrough handling. - HTTP-to-HTTPS redirects now explicitly cover the tenant apex and subdomains. - Gateway configurations require a non-empty apex hostname. - HTTPRoute, TLSRoute, and GRPCRoute resources must declare hostnames. - **Bug Fixes** - Invalid hostname-less routes are now rejected consistently. - Controller-managed redirects no longer trigger unnecessary hostname claims, listeners, or certificates. - Switching certificate modes now cleans up obsolete ACME resources and route status. - **Documentation** - Updated security and gateway guidance for hostname validation, edge termination, route compatibility, and certificate handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
94a202bc5a |
test(e2e): run the merge-gating lanes on Talos containers (#4020)
Replaces QEMU substrate with Talos containers on both lanes that gate a merge. srv1-srv3 run as containers, so tenant worker sits at L2 instead of L3. Measured in CI on commits where both substrates ran the same tree: | | qemu | container | |---|---|---| | `kubernetes-latest` / `-previous` | 1143-2040s, red on every vgif-absent runner | 466-809s, green in both strata | | whole job | | 24-49 min faster | Almost all of it is in Chainsaw. Install takes about half an hour either way. Node-join soft-red gate goes with it. It was a prosthetic for nested virt: a worker that registers in minutes elsewhere could miss any deadline this test can afford, and #3513 established the discriminator is host `kvm_amd` `vgif`, not anything about the product. Remove the nesting level and there is nothing left to tolerate, so `hack/e2e-node-join-soft-red.sh`, the marker and the `soft_red` outputs are gone from all four workflows. ### What moves off the per-PR path DRBD, and with it `replicated` StorageClass, its `Immediate` binding mode, tenant StorageClass propagation (applies only to remotely-accessible classes), and cozystack talos node image with its extensions. None of it is lost - `nightly.yaml` and `e2e-tag.yaml` still run the suite on QEMU, so all four keep nightly and release-candidate coverage. Live migration is not on that list because nothing tests it today anyway. One more belongs on the list and was missing from it: the nocloud disk build. `build-talos` no longer runs `make -C packages/core/talos talos-nocloud`, so `images/talos/profiles/nocloud.yaml` is compiled by `nightly.yaml` and by the tag build's `make assets` rather than by anything a PR runs, and `promote-rc.yaml` still hard-fails a promotion whose rc release has no `nocloud-amd64.raw.xz`. Nothing in that chain breaks, because the tag build supplies the asset as before. What a PR no longer catches is narrow: it still compiles `installer.yaml` through the same imager, and the two profiles are generated by the same script and differ only in `platform`, `kind`, `imageOptions` and `outFormat`. ### Product fixes Four, all found by measurement while building the lane, each on its own commit: The linstor `drbd.enabled` gate. Without it drbd-logger sidecar exits, satellite is never Ready, piraeus registers zero nodes and every PVC hangs Pending behind a cluster that looks healthy. linstor `--strict-topology`. Without it external-provisioner passes every topology segment as `requisite`, so a node-pinned volume can be provisioned away from its pod and the pod is then unschedulable forever with nothing erroring anywhere. kubevirt-cdi importer resources are configurable now. Default restates CDI's own values exactly so production behaviour does not move, and the lane raises the ceiling through its own Package. vm-disk binds standalone disks immediately. A VMDisk on `local` silently never populated, while the same disk on `replicated` did. ### Where the run stands The lane has gone green twice, most recently on `cc70cef7b` where `E2E (in-tree)` took 2h3m against its 215m cap with 45 Chainsaw tests passing. The measurements quoted elsewhere in this description come from the first of those runs, on a commit that has since been rebased away: the two tenant-Kubernetes suites came in at 569.72s (`kubernetes-latest`) and 470.19s (`kubernetes-previous`) against a 67m operation. Both runs answer the four earlier reds, all of which were the lua-protobuf segfault from #4012 once `vminstance`/`vmdisk` were fixed on the branch. ghcr.io mirror removal is carried here too and is the same change as #4007, whichever lands first makes the other a no-op. ### What review changed `drbd.enabled=false` now refuses to render together with `talos.enabled=false`. It removed the logger sidecar only, while the two DRBD initContainers piraeus contributes unconditionally are deleted by the Talos configuration alone, so that combination produced a satellite that can never reach Ready. `isp-full-generic` sets `talos.enabled=false`, so it was reachable and not theoretical. vm-disk keeps unconditional immediate binding, a standalone disk has no consumer to wait for. What it costs on a node-pinned class is in the chart README now, and the annotation is pinned per source type and on the existing-DataVolume path where `helm upgrade` retrofits it. The rest is the harness holding its own promises: HelmRelease gate reads are bounded and its minimum-count half is pinned by a test that fails without it, two Chainsaw ops sit above the deadlines they contain, the fallback-StorageClass render restores `replicated` when it fails, container lane compose project and pool files are per-checkout, and three comments that said the opposite of the code now say what is true (QEMU is still wired into nightly and e2e-tag, the lane replaces two Packages and not one, the argument-count assertion cannot see upstream drift). ### Release note ```release-note fix(linstor): CSI provisioning is constrained to the node the scheduler chose (`--strict-topology`), and the DRBD sidecar can be switched off with `drbd.enabled` on substrates whose kernel cannot run DRBD. On an existing cluster the new default re-templates `spec.csiController.podTemplate`, so piraeus rolls `linstor-csi-controller` once during the upgrade. `drbd.enabled=false` requires `talos.enabled=true` and the chart now refuses the other combination at render time, because only the Talos satellite configuration removes the DRBD initContainers. fix(kubevirt-cdi): CDI worker pod resources are configurable through `importerResources`, which reaches the importer, the uploader and the host-assisted cloner. The default restates CDI's own built-in values exactly, so an install that sets nothing behaves as before. fix(vm-disk): every disk requests immediate binding, not only `upload` sources, so a standalone disk on a WaitForFirstConsumer StorageClass is populated instead of waiting for a consumer that may never arrive. On a node-pinned class such as `local` the volume then binds where CDI's worker pod was scheduled rather than where the VM will run; see the chart README before choosing such a class for VM disks. The default `replicated` is unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added container-based end-to-end testing with local storage support. * Added optional backup-access preflight checks for database backup examples. * Added configurable CDI worker resources and LINSTOR storage settings. * Added configurable immediate storage binding for VM disks. * Added stronger readiness checks, diagnostics, and parallel test execution. * **Bug Fixes** * Improved cleanup failure reporting, capacity validation, and missing-image detection. * Improved FoundationDB health verification and storage placement behavior. * **Removed** * Removed GHCR mirror support and soft-red node-join tolerance. * Removed standalone OIDC render test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
b30c7efaf6 |
fix(e2e): make the OIDC teardown assertions fail closed
The three checks that System-mode artifacts are gone after the mode switch were `if kubectl get ... >/dev/null 2>&1`, which reads every non-zero exit as "the object is gone". An RBAC denial, a timeout or a missing CRD produces the same exit, and stderr went to /dev/null, so the suite reported a clean System teardown for never having observed one. --ignore-not-found is what separates absent from unaskable: absent is exit 0 with empty output, everything else stays non-zero. Preferred over matching NotFound in the message, which depends on wording and, under a runner with tracing on, captures the trace instead of the error. Each probe is pinned individually rather than as a group: the new case breaks one query per iteration while the other two answer absent, so reverting any single probe to the bare form reddens the suite. Verified by doing exactly that three times. A test that broke all three at once would have passed on any one surviving probe, which is how the first attempt at this test fooled me. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
c748c5b707 |
docs(e2e): drop the references the removed suites and carve-out left behind
Convention 7 still sent readers to the node-join carve-out, while section 1 of the same file -- rewritten on this branch -- says the script, the marker and the soft_red output no longer exist. Restated as what the deadline now is. hack/e2e-chainsaw/README.md still listed kubernetes-oidc-system and kubernetes-oidc-customconfig among the suites, and select-install.sh's suite_to_source() still mapped both. The mapping is harmless today because the round-trip pin walks existing directories, but select-e2e.sh says the two functions have to stay in step, so a stale half is a trap for whoever reads that promise. sandbox-runner-headroom.bats keeps reading the QEMU sandbox file for every lane, container lanes included: `prepare-env` also matches `prepare-env-container`, and the compose file gives its nodes the same 8 CPU / 24576 MiB by design. The figures are therefore right for both substrates, but only because the two agree -- and that they agree is pinned in container-lane-capacity_test.bats rather than here. Said so in the test, so the next reader knows what would have to change if the container lane is ever sized differently. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
b131bd56a4 |
fix(e2e): restore the default StorageClass when the fallback render fails
verify_storageclass_fallback_default deletes the `replicated` class, renders with --dry-run=server, and restores it inline "before any assertion exit" -- the comment's words. Under the caller's `set -eu` the render is a plain assignment, so a failed render exits the function there: before rc is read, and before the restore runs. The management cluster is then left with no default StorageClass for every suite after it, which is the opposite of what the comment promises. Captured with `|| rc=$?`. Pre-existing shape, but the restore block is edited on this branch anyway, and the QEMU lanes are where it bites. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
b0e0f38fbe |
test(e2e): keep the chainsaw ops above the deadlines they contain
.chainsaw.yaml states the rule and says why: an op that expires before its inner bounds do is a SIGKILL on the process group, which takes out the diagnostics the failure branch exists to print, while an inner timeout leaves a partial capture and lets the remaining steps run. refresh-and-assert-full-health, added on this branch, breaks it. The poll deadline is 270s, the last in-loop read can start just under it and run 25s, and the failure branch then reads again for another 25s: 320s of inner bounds under a 5m op. Raised to 6m. verify-processes-and-security-context breaks it harder and did so before this branch: 240s for the process counts, then 120s for connectionString and 120s for generations.reconciled, sequentially, is 480s under the same 5m. Fixed here rather than filed, because it is the same defect one step away, found by applying the rule to the neighbour. Raised to 9m. The kubernetes-latest ceiling gets the measurement instead of a change. The OIDC lifecycle folded onto that cluster adds up to 1m + 600s + 1m of passing-path waits that the 67m op does not price, and raising the op is not available: a guard holds COZY_OP_CEILING equal to both suites' timeout, and two suites at 79m plus their teardowns exceed the job's 215-minute cap. On the green run of this branch the whole Test took 569.72s against the 4020s op, with kubernetes-previous at 470.19s. The ceilings only bind once something is already hung, which is what the existing residual note (#3666) says; now it says it with a number. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
186547d835 |
test(e2e): run the merge-gating lanes on Talos containers
The srv1-srv3 nodes of both lanes that gate a merge -- the same-repo one in pull-requests.yaml and the fork one in e2e-fork.yaml -- run as Talos containers instead of QEMU guests, so a tenant worker sits at L2 rather than L3. Measured in CI on commits where both substrates ran the same tree: whole-job wall clock down 24-49 minutes, essentially all of it in Chainsaw, while the two tenant-Kubernetes suites go from 1143-2040s to 466-809s and stop depending on the runner's vgif flag. The node-join soft-red gate goes with the substrate, which is what it was for. It existed because nested virtualisation on the shared runners degraded far enough that a worker registering in minutes elsewhere could miss any deadline this test could afford; #3513 established that the discriminator was the host's kvm_amd vgif flag, and removing a nesting level removes what it acted on. So hack/e2e-node-join-soft-red.sh, the SOFT-RED-node-join.txt marker and the soft_red job outputs are gone from all four workflows, and a missed deadline is an ordinary red again. The ::warning annotation survives, because the reason a run went red should be legible without opening the log, and so does the 124-versus-everything -else test, because "the workers were slow" and "the wait never ran" send a reader to different places. Done by swapping the substrate inside the existing e2e jobs rather than by promoting a separate container job. Those jobs carry more than a substrate -- the GitHub App token, the report-overrun guard, the images list, the SSH breakpoint -- and promoting a job that never had them would have dropped four features without saying so. Three properties of container mode drive the design, and each fails quietly rather than loudly. machine.kernel.modules is a silent no-op -- kernel_module_spec.go returns early on ModeContainer with no error and no event -- so the host loads openvswitch and zfs before the nodes start and the workflow step asserts both rather than trusting them; a missing module surfaces an hour later looking like a CNI or a storage regression. Docker gives a privileged container its own tmpfs /dev seeded once at creation, so /dev/kvm works but ZFS zvols created later never appear and linstor-csi fails ControllerPublishVolume; the compose file binds the host devtmpfs, which is also why talosctl cluster create docker can never carry LINSTOR. And there is no ARP VIP, so 192.168.123.11 stands in for the QEMU lane's .10. The QEMU machinery stays in the tree and keeps running: nightly.yaml and e2e-tag.yaml still call `make prepare-env`, so DRBD, the replicated StorageClass, its Immediate binding mode, the tenant StorageClass propagation that only applies to remotely-accessible classes, and the Cozystack Talos node image with its extensions all keep nightly and release-candidate coverage. None of them is exercised per pull request any more. That is the trade. Nothing on the PR path builds or downloads a nocloud disk now, so build-talos drops the talos-nocloud step and the talos-image artifact. build-talos itself stays: finalize consumes its digest fragment. One property this does not fix, recorded beside the runner class because it will be read off a red run eventually: a container node reports the HOST's CPU and memory as capacity, and hack/e2e-container-up.sh counters that with kubelet systemReserved. That corrects the scheduler and not what a pod reads from /proc, so a workload sizing itself from /proc/cpuinfo or /proc/meminfo is configured for the whole runner. Also carried here because the same files carry them: the ghcr.io pull-through mirror removal (see #4007 -- whichever lands first makes the other a no-op), the authoritative HelmRelease readiness gate, the backup-credential preflight, the prepull guard against an image-less container fragment, and folding the two standalone OIDC suites into the latest-version tenant cluster. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
3a369fed99 |
feat(redis): add opt-in TLS for Redis and Sentinel (#2729)
## What this PR does `tls.enabled` puts Redis and Sentinel on a TLS-only listener with a per-release cert-manager chain, and hands the tenant the CA certificate as `redis-<name>.tenant-ca`. Off unless set. redis-operator v3.3.5 has no TLS support. The work is ported upstream in freshworks-oss/redis-operator#85, which is open and not in a release, so this carries it as three patches over the v3.3.5 tarball. `images/redis-operator/patches/tls.diff` is the source half, cut from that PR's commit `2ad2ed8d1`, which the patch header pins; it is applied by the glob the image Dockerfile already runs, after the Dockerfile strips the tarball's `manifests`, `charts`, `example` and `.github`, so the image builds from source alone and the upstream tests that read the CRD YAML skip instead of failing against the stale v3.3.5 copy. `patches/crd-tls.diff` adds `spec.tls` to the vendored CRD, and with it the CEL rule that fixes `tls.enabled` at creation; without the schema block the apiserver prunes the field on admission and the patched binary waits for something it never sees, and the rule cannot exist without the block it references. `patches/rbac-tls.diff` grants Create, Get and Update on Secrets, so the operator can publish the CA-only Secret it addresses by name. That is the whole delta, and it is narrower than the ported operator's own chart asks for: those three are the only Secret calls in the patched source, and cert-manager Certificates get no grant at all, because the operator only touches one under `spec.tls.certManager` and this chart pins `certificateSecret` mode instead. One change in `tls.diff` is not TLS and takes effect with TLS off, named in the patch header: `MakeSlaveOfWithPort` connects to the replica on its own port rather than on the master's. It changes a dial, not a pod template, so shipping the rebuilt image rolls nothing; the plaintext pod templates stay byte-identical to v3.3.5. The image build registration this branch used to carry (`make -C packages/system/redis-operator image` in the root `build:` target) landed on main with the Valkey service (#3380), so the rebuilt operator image is produced by CI from main as well; the digest in `values.yaml` is still refreshed by release preparation, not here. The chart patches re-apply at the end of `update:`, the same way reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji carry theirs. Otherwise `rm -rf charts` takes TLS back out on the next `make update` and nothing fails. `make update` reproduces the committed tree byte for byte. TLS is chosen when the instance is created. The config gets `port 0` and the listener moves to `tls-port`, so TLS replaces plaintext rather than running beside it, and a running instance cannot change mode: a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The RedisFailover schema in the carried CRD patch rejects a change of `tls.enabled`, which on an existing instance surfaces as a failed HelmRelease upgrade while the instance keeps serving what it did; the README points at creating a new instance and migrating. That is also why TLS is not inferred from `external`: an externally reachable instance is the one most likely to have clients an upgrade must not drop. Existing releases keep their plaintext listener and are not restarted by the rebuilt operator image. The tenant gets `ca.crt` and nothing else, through `tenantsecrets`. The chart's own Secrets take a dot between the release and the suffix (`<release>.ca-tls`, `<release>.tls`, `<release>.ca-cert`): the suffixes are dot-free, so the split is unique and release `foo`'s CA Secret can no longer be release `foo-ca`'s leaf, the collision the hyphenated form had. The chart renders a `TenantProjection` pointing at `<release>.ca-tls`, and the CA-extraction controller re-encodes the one named key from the parsed certificate, so nothing that is not a certificate gets through. Same path as postgres, mongodb, nats and qdrant. No Secret of this release is granted to the tenant by name beyond the generated password, and that includes `<release>.ca-cert`, the CA-only copy the operator publishes for itself. It holds no key today, but the operator has Create and Update on Secrets cluster-wide, so what sits at that name is the operator's call rather than the platform's, and RBAC cannot filter by key. `hack/check-redis-rd-secrets.bats` pins the whole rendered Secret surface instead of a list of known-bad names, because an include with no `resourceNames` and no `matchLabels` restricts nothing. The chainsaw suite for redis gains two cases. A TLS fixture asserts the cert-manager chain is issued, the TLS-probed Redis replicas and Sentinel come up, the operator publishes its CA-only Secret under the widened ClusterRole, the tenant receives exactly `ca.crt` through `tenantsecrets` while the key-bearing Secrets stay off that path, and a `redis-cli --tls` PING through the service name completes with the same CA. A second case applies `tls.enabled` to a running plaintext release and asserts the refusal: the HelmRelease upgrade fails with the schema's message, the StatefulSet keeps its revision with both replicas Ready, plaintext still answers and no TLS listener appears. Certificate renewal rolls the pods from inside the operator. Redis and Sentinel read the certificate once at startup, so the patched operator stamps a hash of the TLS Secret's `tls.crt` and `ca.crt` into the pod template of both the Redis StatefulSet and the Sentinel Deployment on every reconcile, as `redis-failover.freshworks.com/tls-secret-hash`. A renewal changes the hash; the operator's own roller replaces the Redis pods one at a time and the Deployment controller rolls Sentinel. README names the fallback: if the roll cannot run, the deadline is the old certificate's expiry, past which the operator's own TLS connections fail along with the clients. `tls.authClients` maps to `tls-auth-clients` and defaults to `no`. The platform mints no client certificate for the tenant and the CA key is out of reach, so `yes` is only usable when the client certificate comes from whoever also controls the CA. Changing the value on a running release restarts the Redis and Sentinel pods: Redis reads the directive from its config file only at startup, so the operator folds the effective value into the certificate hash that drives the rollout, and the change lands when it is made rather than at the next unrelated restart. Follow-ups in the carried operator patch go upstream first, to freshworks-oss/redis-operator#85 or a successor there, and are picked up here afterwards; the carried patch does not grow Go that upstream has not seen. `tlsConfigFor` fetches the Secret on every call, tens of GETs per reconcile. The renewal roll is static analysis so far: the live check, `cmctl renew` on a running release while watching the `rfr-*` and `rfs-*` pods roll, is still owed and belongs with the e2e follow-up. Two facts about the shipped image rather than the patch. `packages/system/redis-operator/values.yaml` keeps the digest of the image built before the patch until release preparation rebuilds it, so on `main` between this merge and the next release-prep, `tls.enabled` renders the chain and `spec.tls` while the running operator ignores them and serves plaintext. And the pinned v3.3.5 tarball carries GO-2026-5026 in `golang.org/x/net` v0.53.0, reached through `PodService.UpdatePodLabels`; the port neither introduces nor fixes it, upstream `master` is already on v0.55.0, so it lands with the next tag bump rather than here. Outside the patch: `cozyvalues-gen` renders `{}` in the README Value column for `tls.authClients`: a child property without a default inherits the parent object's `{}`, and `make generate` reproduces it byte for byte, so it is a generator bug rather than something to hand-edit here. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#37 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: I walked the trigger map against the diff file by file. terraform-provider-cozystack is reached, tracked as cozystack/terraform-provider-cozystack#37 rather than a speculative PR: `values.schema.json` gains a `tls` object, and the provider hand-writes a schema, model and expand/flatten pair per Kind. It already models `tls` for postgres, kafka, nats and qdrant, not for redis. Redis needs different wording from those four, whose descriptions say to omit the block to follow `external`, which is the one thing redis does not do. Nothing else is reached. The website regenerates this package's reference page from `README.md` on a stable tag, so the README changes need no PR there. No package added, renamed or removed. `packages/core/platform/values.yaml`, variant and bundle composition and release asset names are untouched, and so are `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh`, which is what ccp and external-apps-example read. The two new files under `hack/` are bats unit tests. No node prerequisite, installer value, telemetry metric or cozy-proxy label changes. ### Release note ```release-note feat(redis): add opt-in TLS for Redis and Sentinel. Set `tls.enabled: true` to serve both on a TLS-only listener backed by a per-release cert-manager chain; the tenant reads the CA certificate as the `<release>.tenant-ca` tenant secret. TLS is off unless set and is not inferred from `external`, so existing releases keep serving plaintext and are not restarted by the rebuilt operator image. TLS is chosen at creation: a running instance cannot switch modes, the change is rejected and surfaces as a failed HelmRelease upgrade, so add TLS by creating a new instance and migrating to it. Requires cert-manager, which the platform's `system` bundle installs on every variant. A renewed certificate rolls the Redis and Sentinel pods through the operator. TLS support is carried as a downstream patch over redis-operator v3.3.5 until freshworks-oss/redis-operator#85 lands in a release. ``` |
||
|
|
d800526696 |
feat(mariadb): add TLS support via cert-manager (#2680)
## What this PR does Gives MariaDB a chart-managed certificate chain, makes TLS enforcement explicit, and hands the tenant a key-free trust anchor instead of a private key. - `tls.issuer` selects who issues the server certificate: `operator` (the CA mariadb-operator manages itself, the default and today's behaviour) or `cert-manager` (the instance gets its own CA, and this is what covers the external hostname). The field is named for the issuer rather than for on/off because TLS is served under both — unlike the similarly spelled field in some other charts, which really is a switch. - `tls.required` refuses plaintext connections. It is opt-in and defaults to false: enforcing TLS before the tenant can obtain the CA would be backwards, and a platform upgrade must not sever connections that work today. - Neither issuer is inferred from `external`. Existing instances keep the CA they have and change nothing on upgrade. - The tenant no longer receives `<release>-ca-tls` or `<release>-tls` — the CA private key and the server private key. It receives the operator's `ca.crt`-only bundle, which works today, and the ResourceDefinition additionally declares that bundle as the source for the CA-extraction controller. ### Fixes along the way - The TLS backup path could never have worked: it passed `--ssl-mode=VERIFY_IDENTITY`, a MySQL-8 flag the MariaDB client rejects during argument parsing. Replaced with `--ssl-ca` + `--ssl-verify-server-cert`, and the test now asserts the dialect rather than the presence of a substring. - The leaf certificate carried `clientAuth`. Because `REQUIRE X509` accepts any certificate signed by a trusted CA, the server's own key was usable as a client credential. Removed. - The CA and server private keys survived deletion of the instance: cert-manager runs with `enableCertificateOwnerRef: false`, so removing the `Certificate` left the Secrets behind. Both are now cleaned up by the existing post-delete hook. - The Secrets are labelled for the operator's watch predicate, without which a renewed leaf would not reach the pods until a full resync. ### Why the include carries an exclude list The trust-anchor selector matches by label with no name constraint, so the enumerable dangerous set is excluded explicitly — both issuers' private keys, the internal credentials, the backup keys, and the metrics configuration, which the operator writes unconditionally with credentials in plain text for a user holding `SELECT` on everything. The backstop is bounded and the comment says so: matching is by exact name with no globs, so a Secret introduced by a future operator version, or a transient object created mid-renewal, stays in scope of the label-only include. The primary defence remains that the controller stamps only its own projection. ## Known limitations 1. **Merge is blocked on #3299 — and the blocking is mechanical, not just procedural.** The CRD on `main` does not carry `spec.caCert`, and under Flux's Server-Side Apply the API server rejects a field the schema does not declare (`field not declared in schema`); under the older client-side apply the same field was silently dropped, so installing this branch without #3299 fails outright. An earlier revision of this description claimed the field would be silently pruned; that was wrong. The tenant still gets a usable trust anchor once merged in order, through the operator's bundle. 2. **The deprecated backup CronJob cannot start at all**, for reasons that predate this branch: its `secretKeyRef` names a Secret and key the chart never renders. That is filed separately as #3347 and deliberately not fixed here, since repairing it would start scheduled dumps on production instances from a TLS change. The tests that pin the backup TLS flags therefore pin a path that does not currently execute — the suite header says so. 3. **The operator's `clusterName` is hardcoded**, so on a cluster with a non-default domain the SANs and the name the operator dials diverge. Pre-existing, raised separately. 4. **No e2e coverage for the managed path.** Unit coverage pins the rendered chain, the SAN set, the exclusion set and the guards; the end-to-end handshake is not asserted in CI. ### On review coverage Two independent review passes were run repeatedly against this branch — seven rounds, converging from a security finding, through comment accuracy, to guard strength, and finally to nothing new. A third, model-based pass reviewed the branch through `6cb1bd236` and its findings were fixed; it could not be run against the final revision because of an external account quota lasting until 25 July. The delta since that revision is substantial and includes runtime code, not only tests and docs. Recording this so the coverage is not overstated: if the quota returns before this is ready to merge, that pass is worth running again. ### Downstream repositories `packages/apps/mariadb/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_mariadb`; tracked in cozystack/terraform-provider-cozystack#40. ### Release note ```release-note feat(mariadb): chart-managed TLS via cert-manager, opt-in enforcement, and the CA published to tenants as a key-free trust anchor ``` |
||
|
|
4b2507b7a1 |
docs(gateway): align gateway prose with the code
The comment on the shared CEL string said what the hostname requirement is but not what it is worth per kind. Two of the three schemas leave spec.hostnames optional at every version the vendored bundle serves, so for HTTPRoute and GRPCRoute this policy is the only rejection a hostname-less route meets; tlsroutes requires the field at v1 and v1alpha3 and not at v1alpha2, so there the policy closes the gap for as long as v1alpha2 is served. State that where the requirement is stated. The comment on the port-80 allowedRoutes filter credited the cert-manager namespace entry for letting ACME complete, which the paragraph above it retracts: the solver route lives in the tenant namespace, and that is what keeps ACME working. The same sentence claimed the filter keeps app routes off port 80, and the Layer 1 paragraph of the gateway README claimed it too. It does not. A tenant that owns its Gateway is its own gateway namespace, so its app routes are admitted by this very filter; what it excludes is cozy-* and inheriting children. Both copies now say that. Nothing said why naming the apex on the redirect costs no coverage, which is the question a reader has once the route stops matching every host. It holds because of the other half of this change: the port-80 listener admits routes only from the tenant namespace and cert-manager's, and the tightened route policy requires every route there to declare hostnames inside the apex. An inheriting child does not widen that set either, and the comment now says so at the point the hostnames are chosen: the gateway hostname policy compares every listener against the host label of the Gateway's own namespace, so a child apex outside it never gets its listener, the Gateway write carrying it being refused at admission whole, in reconcileGateway and before this route is rendered, while a child apex under this one passes that check and the wildcard already covers it. The empty-apex guard in renderHTTPRedirect, and the test pinning it, both read as though the guard is what spares an operator an unreadable schema error. It is, but the condition stated for it named the number of attached routes, which does not enter into it. In HTTP-01 the listener set is built from route hostnames alone, so reconcileGateway never reads the apex ahead of the guard however many routes attach. DNS-01 and existingSecret render an apex listener, and any tlsPassthroughServices entry builds <service>.<apex>, so in those the operator meets the equivalent error against the Gateway first. State that condition once, at the guard, and let the test defer to it rather than carry a second copy that can drift; the test stays green either way because the fake client does not validate against the CRD schema. The e2e suite explained omitting the TLSRoute empty-list probe by the v1 schema rejecting an empty list. Both halves of that are true and the conclusion does not follow: the probe runs at v1alpha2, which marks spec.hostnames neither required nor minItems, so an empty list reaches admission there exactly as an absent field does. What makes the omission safe is that all three policies render one byte-identical CEL string, so the HTTPRoute pair proves that term already. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
900711567a |
fix(cozystack-basics): require route hostnames in tenant namespaces
The route hostname policy short-circuited on an absent field. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute, TLSRoute or GRPCRoute declaring no hostnames satisfied the first operand and was admitted without a single hostname compared against the tenant apex. An explicitly empty list reached the same outcome by a different path, because `all()` over an empty list is vacuously true. A route that names no hostname does not match nothing. Under Gateway API it inherits the hostnames of every listener it attaches to, so it covers whatever that listener covers. Require the field to be present and non-empty before validating it. All three policies rendered by the template share one CEL string, so HTTPRoute, TLSRoute and GRPCRoute pick the change up together. The denial message now names the missing-hostnames case, and the Gateway e2e grep moves to the part of that message common to every denial it issues. The Gateway e2e suite carried the one fixture that relied on the old behaviour: its parentRef probe declared no hostnames and is applied into the suite's tenant namespace. It now reads the apex from the namespace label and names a hostname under it, as the neighbouring cases in that suite already did. Radius is unchanged: the policy still matches only tenant-* namespaces. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
4758718d66 |
fix(mariadb): separate the chart-chosen Secret suffixes with a dot
The chart asks cert-manager for two Secrets and names them after the release: "<release>-ca-tls" for the CA and "<release>-tls" for the leaf. The release prefix is "mariadb-", so an application named foo produces mariadb-foo-ca-tls, and an application named foo-ca produces mariadb-foo-ca-tls as well. One release's CA private key and another release's server private key claim the same object. Two sibling applications in one tenant namespace are enough to hit it: whichever cert-manager writes last owns the Secret, and each instance then serves or trusts material issued for the other. The suffix set is what makes it ambiguous. It holds both "-tls" and "-ca-tls", and one is a tail of the other, so a hyphen leaves nothing to say where the release name ends. A dot does: an application name is validated as a DNS-1035 label and the release prefix as ^[a-z0-9-]*$, so neither can contain one, and "<release>.ca-tls" and "<release>.tls" split uniquely. Secret names are DNS-1123 subdomains, which admit the dot. Only the two suffixes this chart chooses move. The Certificate and Issuer object names keep the hyphen, because their suffixes are not a pair where one is a tail of the other, and the operator-managed "<release>-ca-bundle" that the trust-anchor projection reads is not this chart's to rename. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
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 --> |
||
|
|
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 --> |
||
|
|
ce2cabb2d6 |
test(mariadb): verify the tenant-ca projection end to end
The unit suite pins what the chart renders — the sentinel, its source name, its key — from the same assumptions the chart itself encodes, so the two agree by construction and neither can catch the other being wrong. Only a live mariadb-operator says whether <release>-ca-bundle is the right name, and only a live apiserver says whether the lineage webhook promotes the projection to something a tenant can read. Both converged engines already carry this step; mariadb had none. The fixture sets no tls.issuer, which is the case worth covering rather than a gap: TLS is always served and the projection template is ungated, so the operator reconciles the source here too. A negative satisfied by an absent label is also satisfied by a webhook that stamped nothing all run, so the credentials Secret leads as a positive control. For the same reason the key-bearing names are checked for private key material before their absence from the registry is required, the count of names that reached that check is asserted non-zero, and the read must answer NotFound specifically: an RBAC denial or an unavailable aggregated apiserver also makes it fail, and reading that as "safely absent" is how the check would stop discriminating without ever going red. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
3cf9431f1e |
test(redis): fail the negative dial probes on an unread state
The two negative dials in the chainsaw suite, the plaintext dial at a TLS instance and the TLS dial at the instance whose flip was rejected, decided by whether PONG appeared on stdout with stderr discarded. A missing pod, a 403 on exec, a wrong container name or the outer cap firing print no PONG either, so each of them passed as the refusal being asserted, and the flip case is the check that a rejected flip left TLS off. Keep what the dial printed and pass only on the refusal each listener actually gives, verified in the shipped images: a TLS listener drops a plaintext client at AUTH and redis-cli reports it as an I/O error; a plaintext listener holds a ClientHello in silence, because the inline parser's newline scan stops at a NUL in the handshake header, so the inner cap expires and kubectl's exit-code trailer carries the 124. That trailer is what separates the cap firing inside the pod from the wrapper's cap firing outside it. A listener that answers PONG fails the check, and anything else fails as a state the check did not read, with the dial's output in the message. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
25a0618184 |
test(redis): exercise TLS end to end in the chainsaw suite
Everything the TLS unit tests can only render now runs on a live cluster. A second fixture with tls.enabled asserts that cert-manager issues the chain, that the patched operator brings two Redis replicas and the Sentinel Deployment to Ready through its --tls probes, that the operator publishes its CA-only Secret under the widened ClusterRole, and that the CA-extraction controller hands the tenant exactly ca.crt through core.cozystack.io/tenantsecrets while the key-bearing Secrets and the operator's copy stay off that path. The last check completes a handshake through the rfr- service name with the same CA the tenant receives. A third case applies tls.enabled to a running plaintext release and asserts the refusal end to end: the HelmRelease upgrade fails with the schema's own message, the StatefulSet keeps its revision and both replicas Ready, plaintext still answers and no TLS listener appears. The TLS case also asserts the topology the operator built rather than stopping at readyReplicas: exactly one master, a replica whose link is up, and sentinels that moved off the bootstrap address. Ready alone witnesses the bootstrap, since a pod still replicating from the initial 127.0.0.1 never gets there, but the readiness script exits 0 for anything reporting role:master, so it stops witnessing once the bootstrap is over. Both PING calls note where the pinger credentials come from: the ACL user the operator writes into every redis.conf it generates, and the one its own liveness probe uses. Nothing in this tree defines it, so a reader grepping for it here finds nothing and reads the calls as broken. Every redis-cli call runs under the container's own timeout, and both script operations carry an explicit one. The inner cap is the part that matters. The flip case dials TLS at a plaintext listener that will never send a ServerHello, and redis-cli has no handshake deadline of its own. Chainsaw does kill the shell when its exec budget runs out, but it captures the script's stdout and stderr through pipes, and Go's exec then waits for every holder of the write end to close it; the orphaned kubectl is one, so the suite would go silent until the job ceiling instead of failing. Expiry reads as the refusal for the negative check and as a named failure for the PINGs. Both engine images provide /usr/bin/timeout: busybox on the sentinel, coreutils on the data pods. The negative half of the tenant-secret pair requires the error to say NotFound. A 403, a timeout or an unserved CRD would otherwise be read as proof that the Secret is not served, which is the direction that fails silently. The dashboard Role is asserted on the deployed object, not only in the chart unit tests: it must reach the auth Secret and nothing else, which is the name-granting half of the pair the label-driven projection completes. Every kubectl in both scripts runs under a wall-clock bound, because the per-command cap inside a container cannot fire when the exec stream itself never establishes, and a kubectl left running holds the script's stdout open. The plaintext StatefulSet in the flip case waits 5m rather than 2m: it measured 98s of that budget on a real run, being the third failover the group brings up in one namespace. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
f035da8a0f |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Resolve conflicts introduced by main: - Drop the FluxCD tenant addon (API type, deepcopy, values schema, RD) in favour of the GPU type main added; main removed the fluxcd addon wholesale. - Renumber the Phase 2 worker-pool adoption migration 54 -> 56, behind main's redis-operator (54) and fluxcd-removal (55) migrations; targetVersion 55 -> 57. - Accept the branch's removal of the in-chart talos-reconcile Job; it moved to the kubernetes-nodes chart, main only touched its comment. - Regenerate kubernetes values.schema.json / README and the kubernetes-rd ApplicationDefinition schema. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
7cca3d75e7 |
feat(kubernetes)!: remove the FluxCD tenant addon
The optional `addons.fluxcd` toggle of the Kubernetes app deployed a Flux Operator + FluxInstance into a tenant cluster. It was served by two vendored charts, packages/system/fluxcd-operator and packages/system/ fluxcd, which were the last AGPL-3.0 content in this Apache-2.0 repository and a governance blocker for CNCF Incubation. The management cluster does not use these charts (it runs classic Flux via internal/fluxinstall; migration 21 already removed the operator from it), and the addon defaulted to disabled. Rather than maintain an AGPL payload for an unused feature, drop it entirely: - delete both system packages and their platform source entries - remove the addons.fluxcd HelmReleases, values and generated schema from the Kubernetes app - drop the addons.fluxcd block still submitted by the e2e harness - drop the removed fluxcd package entries from CODEOWNERS BREAKING CHANGE: the addons.fluxcd option of the Kubernetes app is removed. Tenant clusters that had it enabled keep their running Flux release; see the companion migration for the upgrade path. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
4385990869 |
feat(redis-operator): migrate from archived spotahome to freshworks-oss fork (#3406)
## What this PR does `spotahome/redis-operator` was archived on 2026-06-11 (read-only; last release v1.2.4, 2022). This migrates the operator to the actively-maintained **freshworks-oss/redis-operator** fork (v3.3.5, Apache-2.0). ### Why freshworks-oss It keeps the same `RedisFailover` CRD and API group (`databases.spotahome.com`) — so existing managed Redis is unaffected — and adds a native `spec.engine: Redis|Valkey` switch, letting managed Valkey drop its Redis-compatibility shim image and run the official `valkey/valkey` image directly. ### Changes - Re-vendor the chart from `oci://ghcr.io/freshworks-oss/charts/redis-operator` (v3.3.5), which carries the `engine`-aware CRD. - Build the operator image from freshworks-oss v3.3.5 source. - Re-target the label/annotation-preservation patch onto the freshworks Go module (`github.com/freshworks/redis-operator`). ### Verification - freshworks-oss source + our `labels.diff` patch compiles natively with Go 1.26.3 (`service` and `cmd` packages). - `git apply --check` of the re-targeted patch against v3.3.5 succeeds. - The operator chart renders with the cozystack-registry image override. - The image is built (freshworks-oss v3.3.5 source + our patch, multi-arch amd64/arm64), pushed to `ghcr.io/cozystack/cozystack/redis-operator`, and pinned by digest in `values.yaml`. - `hack/promote-retag_test.bats` and `hack/nightly-mirror_test.bats` pass with the pinned digest. ### Note for maintainers `packages/system/redis-operator` is not part of the automated `make build`; its image is built out-of-band and the digest is pinned in `values.yaml`. That image is already built and pinned in this PR; re-run `make -C packages/system/redis-operator image` only if you bump the operator version. ### Follow-up The cozystack/redis-operator fork carries cert-manager TLS work (cozystack/cozystack#2729) that freshworks-oss does not have yet; the plan is to upstream it to freshworks-oss so it lands in the shared maintained fork rather than a cozystack-only one. ### Release note ```release-note feat(redis-operator): migrate from the archived spotahome/redis-operator to the actively-maintained freshworks-oss/redis-operator fork (same RedisFailover CRD, adds native spec.engine: Redis|Valkey) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for injecting additional environment variables into the Redis Operator (including a default operator group id). * Added default configurable labels for the Redis Operator. * **Bug Fixes** * Preserves and merges existing Service labels and annotations during Redis Operator updates. * **Chores** * Updated the Redis Operator and Helm chart to the Freshworks-maintained 3.3.5 release, aligned chart/image sources, and simplified Helm chart retrieval. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
7ec5dff93a |
test(e2e): add a runner-layer fixed-work canary around the node-join wait
The counters the node-join failure path collects are read in whatever unit their source publishes: time in the CPU rows, events in the KVM exits, instantaneous values and running maxima in the files beside those. Not one of them carries a unit of work of its own, so a machine that spent every tick and got a fraction of the work done reads healthy in all of them. Add a collector that carries its own unit of work. It runs two fixed amounts of work at the runner layer and times each: a loop over a handful of integer-valued scalars, which pressure on the shared cache or the memory controller moves little, and a store stream through blocks larger than the last-level cache one core can allocate into, which is dominated by it. Fixed work gives each reading a scale of its own, so a red run can be read with no green one beside it, and the capture states the expected range and where the estimate comes from. Sampled where the three counter pairs already are, before the wait on every run and after it on both paths, but outside their brackets: it occupies a core rather than reading a file, and a burn inside one of those intervals would land in the interval each pair divides by: runner-kernel CPU time charged to the join window, and for the two counting the guests, a core taken away from what they count. Two arms, each under a twenty-second ceiling with the five-second kill grace behind it, so fifty seconds at worst against the 420s diagnostics budget. The budget guard derives that from the ceiling the source declares, and the phase-warning guard names the canary among the collectors that keep running when timeout is missing. An exit shaped like the ceiling is attributed to it only when the whole bound was on the clock: an outside kill produces the same status, and read by status alone it would publish exactly the finding this collector exists to detect. A reading below the range the legend states, and an arm the ceiling stopped before it could produce one, reach the job log from all three reporting sites, each naming where it was taken, the way a missing reading does from the two outside the diagnostics block. On the passing path the report that holds the figure is the artifact nobody downloads. Each floor is printed in the legend it comes from and sits where a bounded arm crosses it before the ceiling stops the arm, because a floor the ceiling reaches first never fires: the arm is cut off before its rate can fall that far, and every alert then arrives as a bound rather than as the slowdown the floor was meant to name. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
cd72d81bb4 |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Reconcile the phase-2b split with main's configurable worker-health feature (MHC condition timeouts, maxNodeProvisionTime, podCpuLimit/ podCpuRequest), which landed on both charts after this branch forked: - kubernetes (parent): pool rendering stays removed; main's parent worker-pool additions do not apply post-split. - kubernetes-nodes (pool): absorb the feature; keep the directional parent-version skew guard and the immutable cluster/storageClass rules. KubevirtMachineTemplate content hashes are unchanged (render snapshot). - migration 54: carry podCpuLimit/podCpuRequest (KMT-hash inputs) and the cluster-wide readyUnknownTimeout/readyFalseTimeout/maxNodeProvisionTime into adopted pools, so adoption neither rolls nor resets a tuned pool. Covered by a new adopt-values bats case. - drop the parent chart's obsolete MHC-timeout suites; the kubernetes-nodes twins cover the feature. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
caf9511365 |
fix(platform): CreateReplace the redis-operator CRDs on upgrade
helm-controller skips CRD upgrades by default, so an upgraded cluster would keep the old spotahome schema and prune the fork's spec.engine field on admission. The schema diff is additive, so CreateReplace is safe. Also refresh the harbor e2e wait comment for the freshworks operator. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
b1b8cbecc2 |
test(e2e): price the layers above the sandbox on the node-join window
Every CPU reading on this failure path has a subject inside the sandbox: a cgroup, a tenant worker's threads, a sandbox node's own /proc/stat. Each of them can say a guest was given its ticks while getting no work done, and none can say what the layers above spent. That is where this suite's own tax is paid: the sandbox nodes' guest time is charged to the runner kernel as user time, and whatever the runner VM loses to the machine hosting it is charged to nobody the sandbox can see. Two readings close that. /proc/stat is not namespaced, so reading it from the sandbox container reads the runner VM's kernel -- the only place steal on that layer appears, which is what separates a slow suite from a machine sharing a core with somebody else. The sandbox VMs are QEMU processes in this container's own PID namespace, so their threads are a local read too, and they say which node spent the kernel's user time and whether the three spent it evenly. Both are taken at the same two points as the KVM counters and for the same reason: the values are cumulative, so a rate needs a pair, and the interval worth measuring is the window the guest is failing to boot in. The thread reading runs the probe the worker capture already uses rather than a copy of it, since a second copy would drift exactly at the failure paths. Each carries the layer it describes in its own legend, because three kernels in this report publish a /proc/stat and two captures read QEMU threads, one layer apart in each direction -- and a reader who takes one for the other compares a hypervisor against its own guest. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
0a1d2673d0 |
test(e2e): capture worker block IO and volume state on node-join failure
A worker that burns its whole vCPU while making no progress is either computing without result or waiting on blocks, and every counter this path already collects is blind to the second: the CPU families say what the group was allowed and what it got, the byte counters say what crossed the network, and neither counts a read. Read the Pod's block IO counters from the same kubelet endpoint the CPU ones come from, once rather than twice, because they accumulate from the container starting and a worker container starts with the guest it carries. Read the state of the replicated volumes behind the worker disks as well. A replica that is SyncTarget, Inconsistent or Diskless serves the guest's system disk from a peer over the network rather than locally, and the report collects the same tables only after the suite, by which time cleanup has deleted the volumes a red run would need them for. The guest's own /proc/diskstats would answer this from inside and cannot be reached: the credential minted for the guest carries the os:reader role, which is not allowed to read file contents, and Talos publishes no resource carrying disk statistics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
827f4e7e10 |
test(e2e): measure the sandbox kernel KVM exit counters across the node-join wait
Every CPU reading on this failure path describes a subject inside the sandbox -- a cgroup, a QEMU thread, a node's own /proc/stat -- and all of them can report a guest that was handed its ticks while the guest gets no work done. None of them can say what a tick cost. That price is paid one layer down: a tenant worker is a guest of a sandbox node, which is itself a guest of the runner, and the runner's kernel absorbs the exits, nested page faults and TLB flushes the arrangement provokes. Read those counters from the debugfs that kernel publishes them in. The suite runs in a container sharing it, so this is a local read rather than a walk over nodes, which is what makes it affordable at all: the phase budget has no room left for a further per-node read. debugfs is mounted in a mount namespace belonging to the call, so nothing is left behind for the twenty minutes of suite that follow it. The pair is taken across the node-join wait rather than inside the diagnostics block: once before the wait on every run, once after it on the failing path and on the passing one. The counters are running totals -- per-VM since that VM was created, kernel-wide summed over the VMs alive at the read -- so the interval a difference divides by becomes the window the guest was failing to boot in, and a passing run leaves a baseline to read the failing ones against. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
5297df5cf3 |
Merge remote-tracking branch 'origin/main' into pr-3315-lgtm
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> # Conflicts: # api/apps/v1alpha1/kubernetes/types.go # api/apps/v1alpha1/kubernetes/zz_generated.deepcopy.go # hack/e2e-chainsaw/_lib/run-kubernetes.sh # packages/apps/kubernetes-nodes/tests/render-parity.sh # packages/apps/kubernetes/README.md # packages/apps/kubernetes/templates/cluster.yaml # packages/apps/kubernetes/values.schema.json # packages/apps/kubernetes/values.yaml # packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml # packages/system/kubernetes-rd/cozyrds/kubernetes.yaml |
||
|
|
73b8afc256 |
test(e2e): run tenant workers on one vCPU with CPU headroom above it
Under a two-vCPU worker both vCPU threads burn 1.4 to 1.8 cores between them while the guest kernel never gets past SMP bringup, and raising the CFS ceiling from two cores to three changed neither the burn nor the progress, so the spin is between the two vCPUs, not against the quota. A single-vCPU guest has no sibling to spin on. The ceiling of two keeps the QEMU emulator and IO threads from eating into the one core the guest computes with, and the request keeps scheduling where the workers always were. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |