mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
feat/externalip-api
2158
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
8b2223d1b9 |
feat(vm-instance): let the external endpoint wear a reserved address
The claim annotation lands on the VM's own Service beside the cozy-proxy datapath annotations already there. The two are independent: whole-IP NAT behaves the same on a held address as on an auto-assigned one, so both methods take a reservation. A VM has one external endpoint, so attaching several addresses to one VM stays out of reach of this field — that needs the exposure half of the model, not the allocation half. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
a1f58a5c08 |
feat(clickhouse): add external access and let it wear a reserved address
ClickHouse was the only managed database in the catalog a tenant could not reach from outside the cluster: the installation-wide Service had no way to become a LoadBalancer short of an operator editing the service template by hand. It now carries the same external boolean as the rest of the catalog, and the same externalIPs field. The Service is operator-owned, so this is the in-place type change mariadb and rabbitmq already make on theirs. It keeps its clusterIP, so the transition is one Kubernetes accepts. With external unset the rendered installation is unchanged. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
2e093c7a65 |
feat(mariadb): let the external endpoint wear a reserved address
Which Service actually receives the address depends on the replica count: the operator publishes the cluster Service for a single replica and primaryService once there are more, and the chart already branches on exactly that. The claim annotation follows the same branch, so a tenant writes one entry either way. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
ce00ac8652 |
feat(mongodb): let the sharded external endpoint wear a reserved address
Sharded mode publishes mongos through a chart-owned LoadBalancer Service, which takes the substrate's claim annotation directly. A replica set cannot: it is published as one LoadBalancer per pod, and the operator takes a single serviceAnnotations map for the whole replset, so one claim would be handed to every pod Service at once and the pins would collide. The chart refuses that combination with the reason rather than rendering something that cannot work. It is the discovery-engine case community#45 defers, and closing it needs an address per pod. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
f31f819366 |
feat(rabbitmq): let the external endpoint wear a reserved address
The chart owns no Service here — the operator does — but it already reaches into spec.service to flip the type, and the operator copies annotations from the same block onto the client Service that receives the load-balancer address. So the substrate's claim annotation gets to its consumer without the chart taking ownership of anything new. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
958dde8a22 |
feat(kafka): let the external endpoints wear reserved addresses
Strimzi publishes one bootstrap Service plus one Service per broker, and Kafka advertises every broker's own address to clients. Pinning the bootstrap address alone therefore does not make the cluster reachable at fixed addresses, so externalIPs accepts a broker-N target alongside bootstrap and each entry is rendered into the listener's own configuration block. Broker targets follow the replica count, so a claim for a broker that does not exist is refused as the typo it is rather than kept as a reservation for future growth. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
773d6b9aee |
feat(redis): let the external endpoint wear a reserved address
Threads externalIPs onto the LoadBalancer Service the chart renders for the Sentinel-elected master. Empty is the previous behaviour. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
7c2f73080f |
feat(postgres): let the external endpoint wear a reserved address
Adds externalIPs, a list of {claim, target} entries, and threads the
resulting annotation onto the LoadBalancer Service the chart already renders
for the primary. Empty is the previous behaviour, so nothing changes for an
existing release.
The list shape is deliberate even though the chart publishes a single endpoint
today: the same field lands on every catalog app in this series, and Kafka does
have several external endpoints. A target that the chart does not publish is
refused with the valid ones named, rather than being accepted and ignored.
Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
||
|
|
d000d02b99 |
feat(rabbitmq): add TLS support for AMQPS and management HTTPS (#2683)
## What this PR does Adds TLS for AMQP and the management API, and hands the tenant a key-free trust anchor instead of a private key. - A chart-authored cert-manager chain issues the broker certificate. The chart names its CA Secret in a `TenantProjection` sentinel; the CA-extraction controller reads `ca.crt` from that Secret and writes a key-free projection, stamping `internal.cozystack.io/tenant-ca` on the projection it owns. That label is what the ResourceDefinition selects. Nothing labels the CA Secret itself, which holds the CA private key and is excluded by name. - The leaf `<release>-tls` carries the broker's private key and reaches no tenant. Neither the dashboard Role nor the ResourceDefinition grants it, and the ResourceDefinition excludes it by name as well, so a label mis-stamped onto it later still does not get through. - SAN coverage now lists every resolvable form explicitly, including the bare `*.<release>-nodes` wildcard that the operator uses for `RABBITMQ_NODENAME`. ## TLS is opt-in here, unlike the sibling charts `tls.enabled` defaults to `false` and is not inferred from `external`. That diverges from nats, qdrant and kafka, which infer it, and the divergence is deliberate for a reason specific to this operator. Setting `spec.tls.secretName` makes the cluster operator rewrite the `port` key in the `<release>-default-user` Secret from 5672 to 5671, on the mere fact of TLS and regardless of whether the plaintext listener is still open. That Secret is the Service Binding the chart hands the tenant. A client with a hardcoded `:5672` keeps working. A client that reads the endpoint from the binding, which is the documented way, gets moved to the TLS port and must then speak TLS and be given a CA to verify against, neither of which it was configured for. Inferring TLS from `external` would fire all of that on an ordinary upgrade, with no configuration change, against exactly the clients that followed the documentation. Whether the other engines should follow is an epic-level question rather than this PR's to answer; for nats it is raised in #3348. ## The plaintext ports are still published, and this PR says so There is no operator-supported way to publish the TLS ports alone. `spec.override.service.spec.ports` is a strategic merge keyed on `port` against defaults the operator rebuilds every reconcile, so it adds and mutates entries but never subtracts one, and a TLS-only port list there would read as a guarantee while changing nothing. The chart renders no service override at all and says why at the callsite. `tls.disableNonTLSListeners` is offered as an explicit opt-in, and it refuses to render rather than failing silently in three situations. - Together with `users` or `vhosts`. The flag forces messaging-topology-operator onto TLS regardless of its own configuration, after which it verifies the broker against the system certificate pool, where a per-release CA cannot be. Every topology object bound to the release would stop reconciling, silently: already-created users keep working while password rotation and new users simply never apply. There is no working route left for it, so the chart refuses instead. Managing those objects as hand-written CRs is not a way out, because the operator takes TLS from the `RabbitmqCluster` and not from whoever created the CR, so manual objects break identically and without the render-time refusal. `rabbitmqctl` or a definitions import are. - While topology objects are still live in the namespace. See the next section. - With TLS off, where no listener of either kind would remain. Erlang distribution (25672) and epmd (4369) stay plaintext under every setting; the flag does not touch them, so `disableNonTLSListeners: true` is not "TLS everywhere". A third route, closing AMQP at the broker through `spec.rabbitmq.additionalConfig` while leaving the operator on plaintext management, has been suggested and is plausible, but the operator's sources are not vendored here and it was not verified. It is recorded as an open question rather than promised. ## Closing the listeners takes two upgrades, and the chart enforces it Clearing `users` and `vhosts` and setting `tls.disableNonTLSListeners: true` in one edit is the natural thing to try, and it wedges the objects being removed. The values-level refusal cannot catch it, because by then both maps are empty. Helm applies every object in the target release before pruning the ones the release dropped, so the broker closes plaintext and loses the `rabbitmq.com/operator-connection-uri` annotation while the topology objects still hold `deletion.finalizers.<plural>.rabbitmq.com`. Their operator then has to reach a management API it can only get to over a certificate it verifies against the system trust pool. The delete never completes, and the upgrade reports success over objects left in `Terminating`, with the users still live in the broker and `kubectl delete namespace` on that tenant hanging. The chart now reads the namespace rather than the values. When the flag is being introduced and any topology object is still there, the render refuses and names them, gives the two-upgrade order and the `kubectl get` that confirms nothing bound to this release is left, and says how to unwedge a release that is already stuck. The check keys on the live `RabbitmqCluster` as well, so it applies only to the transition: once the listeners are closed, an object a tenant creates afterwards is theirs to clean up rather than something that stops the release rendering. Eleven unit cases cover the firing and the silent paths. The set is wider than the three kinds this chart creates. All twelve kinds `messaging-topology-operator` drives through its shared `TopologyReconciler` carry that finalizer and share one deletion path, so a `Queue` or a `Shovel` written by hand against the release wedges exactly the same way, and a runbook step that checks only users, vhosts and permissions says the namespace is clear when it is not. The list is derived in one place and read from both the lookup and the `kubectl get` the refusal prints, and a unit test holds it against the operator's own CRDs so a bump cannot introduce a kind the guard has never heard of. `SuperStream` is the one CRD left out: it runs on its own reconciler with no finalizer, and the objects it owns are already on the list. An object that binds through `connectionSecret` instead of naming the cluster stays outside the guard, which the comment says. ## Breaking for metrics scrapers Enabling TLS moves the metrics endpoint from 15692 to 15691. A ServiceMonitor selecting the port by the name `prometheus` silently stops matching and the instance's metrics disappear with no error anywhere. Nothing in this repository scrapes RabbitMQ, so only user-defined monitoring is affected, silently, which is why it is stated here. ## Known limitations 1. Turning TLS back off leaves the raw CA material behind. The anchor itself is collected: the projection carries an owner reference to the `TenantProjection` sentinel, and Helm deletes the sentinel along with the rest of the chain. What survives is `<release>-ca` and `<release>-tls`, because cert-manager runs with `enableCertificateOwnerRef: false` and does not delete the Secrets it issued, so the README says to remove them by hand. #3349 asks the same question of the label-driven leg of the contract, which this chart does not use. 2. No e2e coverage for the TLS path. The one-upgrade wedge above is the case that most deserves it: install with TLS and users, upgrade to no users plus the flag, and assert the `User` is actually gone rather than merely marked for deletion. A template test cannot reach that. On certificate renewal, the leaf carries `duration: 8760h` and `renewBefore: 720h`, so cert-manager rewrites the Secret around day 335. The cluster operator mounts it as a projected volume with no `subPath`, so the kubelet refreshes the files in place, and the operator does not watch a user-supplied TLS Secret, so nothing rolls the pods. The last hop, the broker picking the new file up, rests on Erlang's `ssl_pem_cache` re-reading by mtime and has only been read from sources rather than exercised against a live broker. The merged qdrant chart ships the same duration and `rotationPolicy`, so this is a platform-wide property rather than something this PR introduces. ### Downstream repositories `packages/apps/rabbitmq/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_rabbitmq`; tracked in cozystack/terraform-provider-cozystack#42. ### Release note ```release-note feat(rabbitmq): opt-in TLS for AMQP and the management API, with the CA published to tenants as a key-free trust anchor. Enabling TLS moves the metrics port from 15692 to 15691 (ServiceMonitors selecting the port named `prometheus` must be updated) and moves the `port` value in the `<release>-default-user` binding Secret from 5672 to 5671. Plaintext AMQP and management ports remain published; `tls.disableNonTLSListeners` closes them but also drops in-cluster plaintext clients, and must be enabled in a separate upgrade after the topology objects are gone. ``` |
||
|
|
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 --> |
||
|
|
a115745c69 |
feat(vm-disk): let the operator defer binding to the consumer
Immediate binding for every source fixed a real bug: a standalone disk on a WaitForFirstConsumer class has no consumer to wait for, so it never populated. It also took away the path that is correct for the other case. For a disk a VMInstance consumes, KubeVirt renders a temporary launcher pod carrying the VM's own affinity and nodeSelector, so the volume binds where the VM can actually run; with the annotation always present the CDI worker becomes the first consumer instead and the volume lands where that pod happened to be scheduled. On a node-pinned class the VM is then unschedulable against a node-affinity conflict, and storageClass is immutable, so the only exit is deleting the disk. The two cases want opposite defaults and only the operator knows which one a given disk is. bindImmediately defaults to true, so nothing moves for existing disks or for anyone on `replicated`, which binds Immediate either way. Both branches are pinned in tests, including the existing-DataVolume path, which renders its own annotation block. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
0c90abcedd |
test(rbac): cover the marketplace tap grants in the pinned RBAC suites
The tap flow added a packagesources delete grant and an ocirepositories read grant to the cozystack-api ClusterRole, and taps to the tenant core.cozystack.io grants. Both suites pin those lists on purpose, so move the count bound and assert the new grants by name instead of leaving them uncovered. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
d1e0f41cbd |
refactor(marketplace): drop community-prefix naming in the dashboard
deriveTapName now yields the tap source id (tap-<org>-<repo>) that the server assigns, and the community-prefix constant is gone. The Repositories view labels a tapped source "external" rather than "community"; it already renders a pending or blocked tap's message, which the server now returns for taps with no materialized PackageSource yet. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
b6f14373b5 |
fix(marketplace): scope controller OCIRepository writes and split validate YAML safely
Close the two non-blocking notes from the latest review, mirroring fixes already applied on the other side: - The tap materializer's update/patch on ocirepositories moves from the cluster-wide cozystack-controller ClusterRole to a namespaced Role in cozy-system (get/list/watch stay cluster-wide for the informer), matching the API-side scoping. The materializer only ever writes tap sources in cozy-system. - cozypkg validate now splits multi-document YAML with the apimachinery reader instead of a naive ^---$ regex, so a PackageSource whose block scalar contains a separator-looking line is not fractured — the gate's validator now decodes the same content the materializer applies. Covered by a block-scalar test. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
c9accd6086 |
fix(marketplace): address maintainer review — url collisions, fail-closed digest, centralized prefix
From @kvaps's review of the untrusted-artifact path: - Refuse to silently retarget: two repositories that share an org/repo path on different registry hosts derive the same tap name. The API and CLI now reject a connect when an existing source points at a different spec.url, instead of blindly repointing the first at the second. - Fail closed on integrity: verifyAndExtract now errors on an absent digest (not just a mismatch), and the reconciler waits for status.artifact.digest, so a tap artifact is never materialized without a verified digest. - Centralize the load-bearing community. prefix in internal/marketplace/tapconst (removing two duplicate consts and several literals across the CLI, API, materializer, and dashboard) so the disconnect guard, the community badge, and the anti-shadowing guarantee cannot drift. - Split multi-document YAML with the apimachinery reader so a PackageSource whose values contain a separator-looking line is not fractured. (The short-name version-pin drop was already fixed in an earlier commit.) Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
3ce8279019 |
fix(marketplace): prune stale taps, preserve source state, scope RBAC, pull by digest
Address the remaining review findings: - The materializer now prunes PackageSources it materialized from an earlier revision that the current artifact no longer contains (including a single/multi rename), so a removed package leaves the catalog; installed Packages stay. - A repeat connect updates the fetched OCIRepository's spec, label and name annotation instead of replacing the object, so the operator's finalizer and materialized-revision annotation survive; covered by a test. - OCIRepository write verbs move from the cluster-wide cozystack-api ClusterRole to a namespaced Role in cozy-system (get/list/watch stay cluster-wide for the watch cache), since the API only manages tap sources there. - 'cozypkg validate --require-signature' now pulls the exact manifest digest cosign verified, closing the tag-mutation window between verify and pull. - safeJoin rejects any '..' path segment outright (defense in depth and to satisfy the Zip-Slip scanner) on top of the existing containment check. - Document why the index gate's merge/label steps need a workflow_run split for fork pull requests. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
35e54fb49e |
feat(dashboard): search and trust notice on the Repositories view
Add a search box that filters connected repositories and their packages by name, kind, description, category, or tag, and a trust notice stating that connecting a repository runs its charts in the management cluster and that signatures are verified by the index at publication time, not at connect time. Both close gaps the design-conformance review found against #18/#23. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
1fbceee10e |
fix(marketplace): operator RBAC for tap materialization and dot-safe artifact naming
Second review round found the write-path RBAC fix was only half applied and the extracted naming helper diverged from its producer: - The tap materializer runs as the cozystack-controller ServiceAccount, whose ClusterRole granted source.toolkit.fluxcd.io/ocirepositories only get/list/watch. Its finalizer and materialized-revision writes (r.Update on the OCIRepository) were therefore Forbidden, so a dashboard-connected tap never materialized. Grant the controller update/patch on ocirepositories. - naming.ArtifactName replaced dots only in the PackageSource name, but the reconciler replaces them in the variant and component too, so a tapped repo with a dotted variant or component name dropped its packages from the catalog and tripped a false appdef-dangling warning. Replace dots in all three segments and add a direct parity test with dotted names. - Pin cosign-installer and the cosign release in the index gate so the signature verifier is reproducible. - Add a unit test for the tap rollback objectExists predicate. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
52de2e460e |
fix(cozystack-api): grant Tap write-path RBAC and recover pending taps
The marketplace Tap create/delete run with the cozystack-api ServiceAccount, which was granted only get/watch/list on cozystack.io and nothing on source.toolkit.fluxcd.io, so connecting or disconnecting a tap returned Forbidden on every cluster. Grant the ServiceAccount delete on cozystack.io/packagesources and full management of source.toolkit.fluxcd.io/ocirepositories. Also make a pending or failed connect recoverable: when a tap has an OCIRepository but no materialized PackageSource yet, Delete now falls back to removing the labeled OCIRepository matched by its tap-name annotation, instead of returning NotFound and orphaning a finalized source. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
2cedd76fb9 |
feat(dashboard): add marketplace Repositories view backed by the Tap resource
Add a Repositories view to the console marketplace that lists connected External-Apps repositories from the taps.core.cozystack.io resource, showing each tap's source, community/official and ready status, and the packages it exposes with a privileged badge. Operators can connect a repository by oci:// reference (with an optional pull secret for private repositories) and disconnect community taps, backed by create and delete on the Tap resource; installed packages are left in place on disconnect. The view reuses the existing k8s-client list/watch and mutation hooks, so it updates live, and adds a Repositories entry to the marketplace sidebar. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
5848d85840 |
feat(cozystack-api): add read-only Tap resource for the marketplace view
Add a cluster-scoped, read-only Tap resource to the core.cozystack.io API group that powers the dashboard marketplace view. Each Tap is computed on read from a PackageSource and the ApplicationDefinitions attributable to it (matched by the component's assembled-artifact name), exposing the connected repository, whether it is a community tap, its readiness, and the packages it exposes with their catalog metadata. It is served through the existing aggregation layer, so the dashboard consumes it with the same list/watch client it already uses, and authorization is the delegated RBAC the apiserver already enforces. Taps carry catalog metadata only and never a pull-credential Secret, so browsing cannot leak a private tap's credentials across the tenant boundary. Read access is granted to the tenant catalog-read roles alongside options. Modeling the marketplace as a virtual resource in an existing API group avoids a new API group and needs no etcd or CRD: the object is computed from cluster state via the apiserver privileged dynamic client, mirroring the options resource. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
cc70cef7b9 |
docs(linstor): record the measured topology requirement
The first paragraph asserted what external-provisioner sends without the flag; it is now read off the wire on a three-node dev stand. A WaitForFirstConsumer CreateVolume carries requisite as the single segment the scheduler chose with the flag set, and all three node segments without it, the chosen one merely first in preferred. Same run confirmed piraeus re-derives the csi-controller Deployment once when the flag lands on an existing LinstorCluster, and that the satellite DaemonSets do not move. The `replicated` note claimed less than the facts support. That class binds Immediate, and external-provisioner scopes strict topology to late binding, so the flag never reaches that path -- which is a stronger statement than the PV merely being remotely accessible, and does not depend on reasoning about placement at all. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
be55c68fbc |
docs(linstor): keep the strict-topology rationale out of the applied object
A `#` comment in a rendered template is output, not source: the eight lines explaining --strict-topology were applied to the live LinstorCluster and came back on every `kubectl get`. Moved into a template comment, which is stripped at render. The test comment beside the argument-count assertion claimed the count was "a tripwire that forces a re-check" on a piraeus-operator bump. It cannot be. The assertion reads the chart's own rendered args, and those do not move when upstream adds a flag -- the count stays 12 and the suite stays green. It catches an edit to this chart, which is the case that needs it least. Say what it does catch, and say where the upstream half actually has to be checked: the tree vendors the piraeus chart but not the operator resources carrying those defaults, so there is nothing in-tree to diff against and the re-check on a bump stays a human obligation. The list is recorded as verified against v2.10.2. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
f34a7f5ea1 |
fix(linstor): refuse drbd.enabled=false on a non-Talos substrate
drbd.enabled=false removes the drbd-logger sidecar, and that is all it removes. The satellite DaemonSet also carries two DRBD-only initContainers that piraeus-operator contributes unconditionally -- drbd-module-loader, which cannot load a module the substrate does not have, and drbd-shutdown-guard -- and the only configuration that deletes them is satellites-talos.yaml, gated on talos.enabled. So with talos.enabled=false and drbd.enabled=false the chart rendered a satellite that can never reach Ready: exactly the state the flag exists to prevent, reached by setting the flag. Not hypothetical -- the isp-full-generic bundle sets talos.enabled=false (packages/core/platform/templates/bundles/system.yaml). Fail at render time and name both flags. The alternative, moving the two initContainer deletions and their volumes out from under talos.enabled, is the real fix for generic Linux and is a larger change than this lane needs; it is tracked in cozystack/cozystack#4092, and the message points there. Refusing is what keeps an operator from getting no signal at all in the meantime. values.yaml states the requirement beside the flag. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
0418577f55 |
fix(kubevirt-cdi): make importer resources configurable
podResourceRequirements applies to every worker pod CDI creates -- importer, uploader and host-assisted cloner alike, not only the importer the parameter is named for -- so a value set here reaches the console's disk-upload path as well as the imports. It has to be set on the CDI CR and not on CDIConfig: the operator reconciles CDIConfig.spec from this resource, so a direct patch is silently reverted and status.defaultPodResourceRequirements stays where it was. The default restates CDI's own built-in values exactly -- limits 750m/600M, requests 100m/60M, read off a live cluster -- so an install that sets nothing behaves as it did before the parameter existed. That is deliberate. A 4Gi ceiling was measured as necessary on one hand-built stand, where the decompress+convert of a tenant worker disk was OOM-killed at 99.8-100% and CDI then retried from scratch forever, but the same 20Gi import completes at stock 600M on a QEMU-node cluster and no CI run on either lane has recorded an OOMKilled importer. What has been shown to need the headroom is that substrate, so the override lives with the substrate in hack/e2e-platform-packages.sh rather than here. The failure being answered is memory, and only memory. CDI already ships a CPU limit of its own at 750m, nothing measured has asked for more, and the default here restates that figure rather than raising it. The package had no `test` target, and hack/helm-unit-tests.sh only runs a package whose `make -n test` resolves, so a tests/ directory here was collected by nothing and skipped in silence. Added with the suite. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
499034e960 |
fix(linstor): constrain CSI provisioning to the scheduler's chosen node
Without --strict-topology, external-provisioner passes EVERY topology segment as `requisite` and merely *prefers* the node the scheduler picked, so when that node is out of space linstor-csi may legally provision the volume somewhere else. For a node-pinned volume (allowRemoteVolumeAccess: false, the `local` class that is the cluster default) the result is a PV whose node affinity contradicts the pod's other volumes, and the pod is then unschedulable forever against pv "pvc-..." node affinity doesn't match node "srv1": no matching NodeSelectorTerms with nothing erroring anywhere. It bites hardest on CDI imports, because an importer pod mounts two volumes: the disk, and a ~21 GiB scratch volume CDI creates only AFTER the pod is scheduled. The scheduler therefore sizes the node against the disk alone, can place two importers where both disks plus both scratch volumes do not fit, and the second scratch lands elsewhere. Capacity tracking cannot help -- the scratch volume does not exist when placement is decided, and CSIStorageCapacity agreed with LINSTOR to within 5% when this was measured. Reproduced on 2026-08-25 by cordoning two of three nodes so both importers landed on one constrained to 45 GiB: enough for two 20 GiB disks, not for their scratch. With the flag the scratch PVC stays Pending and the pod reports `1 node(s) did not have enough free storage`, then binds and completes once space frees. A silent permanent deadlock becomes a legible, self-correcting wait. The full argument list is restated because `args` has no patchMergeKey, so a strategic merge replaces rather than appends. That makes the list a drift hazard on every piraeus-operator bump, and the note beside it says so -- the count assertion in the test guards this chart's own list and cannot see a flag the operator adds upstream. Verified for node-pinned volumes. For `replicated` the PV is remotely accessible so the constraint is satisfiable wherever the pod lands, and that case is reasoned rather than measured -- hence the switch. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
06ee08f636 |
feat(linstor): gate the DRBD sidecar behind drbd.enabled
On a substrate where the DRBD kernel module is unavailable the drbd-logger sidecar exits immediately, the satellite pod is therefore never Ready, and piraeus consequently registers zero nodes: LINSTOR reports none, linstor-csi-node hangs in Init polling for a node that does not exist, and every PVC stays Pending behind a cluster that looks healthy. So this flag is the difference between working storage and no storage at all there, not a cosmetic reduction in logging. The concrete case is Talos nodes running as containers. DRBD's resource registry is global per kernel, so three containers sharing one kernel cannot represent three DRBD nodes -- the second new-resource for a given name silently aliases the first instead of failing, which is worse than an error because LINSTOR then records a replica it does not have. It applies to kind equally; it is a property of one kernel, not of Talos. Omitting the sidecar from Cozystack's own pod template is not enough, because piraeus strategically merges satellite configurations and a container another configuration contributes survives the omission. So a LinstorSatelliteConfiguration deletes it from the effective DaemonSet as well. What the flag does NOT do is touch a StorageClass, and the comment says so explicitly. Cozystack does not own them -- an operator writes them by hand from the install guide -- so a class carrying layerList: "drbd storage" still exists after setting this false and volumes on it will never provision. The operator-facing half of that warning goes beside the examples they copy, in cozystack/website#675. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
6935df8c7f |
chore(kubevirt): update KubeVirt to v1.9.0 (#4015)
## What this PR does KubeVirt operator manifest goes from v1.8.4 to v1.9.0. The 1.9 release turns every Beta feature gate on by default and adds `disabledFeatureGates` to the CR for opting out, so the CR side needed a few decisions. `Template` is disabled. With it on, virt-operator deploys virt-template: two Deployments, two APIServices, a webhook and a ValidatingAdmissionPolicy that nothing here uses. One more aggregated API server is one more way to break discovery. This one is not a knob. `- Template` is written into the CR unconditionally and there is no value that takes it back out, because nothing on the platform uses VirtualMachineTemplate; if something starts to, the static entry becomes a value then. Asking for it through `extraFeatureGates` is refused at render time with an error that says so, rather than producing a CR KubeVirt would reject. `ImageVolume` is the host-dependent one: off below Kubernetes 1.35, left on from 1.35, with a platform value for runtimes that cannot serve image volumes even there (#3190 has the k3s case). Nothing the platform ships uses containerDisk. The value is `kubevirt.disabledFeatureGates` on the platform values, which the iaas bundle forwards into the emitted `cozystack.kubevirt` Package as `components.kubevirt.values.disabledFeatureGates`: Closes #3190. ```yaml kubevirt: disabledFeatureGates: - ImageVolume ``` The chart refuses to render a CR that names one gate in both lists. KubeVirt's `kubevirt-update-validator.kubevirt.io` webhook rejects that combination and runs with `failurePolicy: Fail`, so on a cluster that already has the CR the write is refused and the `kubevirt` HelmRelease stops converging with everything else the CR carries. The create admitter runs no validation at all (its own comment says the update validator is where that happens, and it only gets installed after the CR exists), and `IsEnabled` in virt-config checks `FeatureGates` before `DisabledFeatureGates`, so on a fresh install the same CR is admitted and the gate silently stays on. Render-time is the only place left where an operator gets told. A name repeated inside one list is not an error: upstream compares the two lists against each other, and both fields are `x-kubernetes-list-type: atomic` in the CRD, so that CR is accepted and the guard stays quiet. The gates the chart pins off can arrive a second time from a supported values file, since the chart writes them itself and an operator listing the gates their cluster declines has no reason to know that, so the range skips them and the CR states each opt-out once. The guard walks its own copy of the seven gates the chart enables. That copy exists because the rendered list has to stay a literal list for `make update` to find its `- VMExport` anchor, and a gate quietly dropped from the copy would keep rendering under `featureGates` while leaving the guard. The suite carries one case per gate, so removing any single name turns exactly one of the seven red. `HotplugVolumes` stays in the list even though 1.9 deprecates it. A deprecated gate listed in the CR is still enabled, and while it is enabled virt-controller skips the declarative hotplug path. Dropping it would switch on declarative hotplug, but then virt-api rejects the VMI-level `removevolume` subresource on VM-owned VMIs, and kubevirt-csi uses that as a fallback during teardown. The CSI driver has to move off that subresource first. Upstream names no removal release for it: the `Deprecated` state's own doc comment says "discontinued in the following release", but `MultiArchitecture` and `DisableMDEVConfiguration` have been deprecated since 1.8.0 and are still registered at 1.9.0, so that is a direction rather than a date. What makes that opt-out list matter is a semantics change: on 1.8 `isFeatureGateEnabled` ended in `return false`, so a Beta gate nobody listed was off; on 1.9 `IsEnabled` ends in `return fg.State == Beta`. Sixteen gates are Beta at v1.9.0 and none of them is in this chart's `featureGates`, so all sixteen arrive on with the bump. Two are switched back off here, one is switched off below Kubernetes 1.35, and I went through the rest: - `Template`: off. Reason above. - `ExternalNetResourceInjection`: off. With it on, virt-controller stops calling `multus.NetworkToResource`, so a NAD's `k8s.v1.cni.cncf.io/resourceName` no longer becomes a resource request on the launcher pod. That is correct for clusters running a network-resources-injector; this platform ships none, so a VM on an SR-IOV NAD would schedule without its device. - `ImageVolume`: off below Kubernetes 1.35, left alone from 1.35 on. KubeVirt renders containerDisk images as `VolumeSource.Image` volumes when the gate is on, and it does not check the host: `dropImageVolumes` in the apiserver strips every image volume and the mounts referencing it while Kubernetes' own `ImageVolume` gate is off, which is the default until 1.35 (`kube_features.go` has it `Default: false` at 1.31 and 1.33). The pod is admitted without the disk and virt-launcher fails looking for it. The management cluster is supported from 1.33, so on 1.33 and 1.34 the chart keeps the KubeVirt gate off and the containerDisk sidecar path in use; from 1.35 the gate stands and `extraFeatureGates` can name it. The apiserver version is a proxy for a runtime capability, not the capability itself, so a cluster at 1.35 or newer whose container runtime cannot serve image volumes (the k3s case in #3190) switches the gate off through `kubevirt.disabledFeatureGates`; that is what the value is for. - `Snapshot`: on. `VirtualMachineSnapshot` needed the gate listed on 1.8; snapshot and export were exercised on the test cluster. - `DeclarativeHotplugVolumes`: on and inert. `handleDeclarativeVolumeHotplug` returns early while `HotplugVolumes` is enabled, which this chart still does. - `GPUsWithDRA`, `HostDevicesWithDRA`: on; each only lets a VMI request a DRA-provisioned device, and nothing here creates one. - `WorkloadEncryptionSEV`: on; needs SEV hardware and a VM that asks for it. - `PasstBinding`: on; needs a VM using the passt binding. - `RebootPolicy`: on; adds a field on the VMI domain spec, unset means the old behaviour. - `VmiMemoryOverheadReport`: on; fills `status.memory.memoryOverhead`, reporting only. - `OptOutRoleAggregation`: on; it enables the `RoleAggregationStrategy` field, and aggregation stops only for a CR that sets it to `Manual`. This one does not. - `KubevirtSeccompProfile`: on; installs the profile, and a workload uses it only where `seccompConfiguration` says so. - `NodeRestriction`: on; virt-handler node authorization, needs the Kubernetes `ServiceAccountTokenPodNodeInfo` gate, GA since 1.32. - `LibvirtHooksServerAndClient`: on; pre-migration hooks on the target launcher, and no hooks ship here. - `PodSecondaryInterfaceNamingUpgrade`: flips on; it adds `--upgrade-ordinal-ifaces` to virt-launcher and a pre-migration hook that renames the ordinal secondary interfaces of VMs still carrying them. The test cluster's migrations exercised that path. Five of the seven gates the chart enables (`ExpandDisks`, `LiveMigration`, `AutoResourceLimitsGate`, `GPU`, `VMExport`) reached GA by 1.9 and are inert: `IsEnabled` returns true for a GA gate whether or not the CR names it. I left the list as it is; dropping the five is a follow-up, because it changes what the CR says without changing what the cluster does and it belongs in its own commit. QEMU is 10.1.0 on both sides of this jump; the el9 build revision moves from -10 to -20 and libvirt goes 11.9 to 11.10. A moving build revision is the thing to check rather than a major: [kubevirt/kubevirt#16386](https://github.com/kubevirt/kubevirt/issues/16386), where every existing VM lost live migration, was 1.6.2 to 1.7.0 on qemu-kvm 9.1.0-19 to 9.1.0-20, one build apart within the same QEMU version. So I tested it: VMs running on the 1.8.4 launcher live-migrated onto the 1.9.0 one on a three-node cluster. `make update` in `kubevirt-operator` used to `rm -rf templates` before the fetch and took the local PrometheusRule template with it. Now it only overwrites the file it downloads. `make update` in `kubevirt` now greps for every line of the new block before it passes: the range directive, the `disabledFeatureGates:` key, the three static entries, and the guard header that decides which static entries render. Every insert in that target is guarded by one directive it also inserts, so a hand-merge that keeps the directive and drops a sibling line leaves the guard satisfied and the insert skipped. That shape covers the directive and nothing else. What makes it worth a grep per line here is the static entries: they are the only lines those inserts add that are CR content in their own right rather than a wrapper around a value, so they render on every install and their absence cannot surface as a value that stopped taking effect. The CR just goes back to deploying virt-template and to letting KubeVirt skip NAD resource injection. The target cannot rebuild a partial block, so it names the missing line and stops. The payload lines of the other four blocks are exposed the same way; I checked, and both dropping `- {{ . }}` from the `extraFeatureGates` range and dropping the `permittedHostDevices:` key from its `with` render green and lose the operator's values. That is a wider change than this PR and I left it out. Two sharp edges I know about and left. `kubevirt.disabledFeatureGates: [HostDevices]` on a cluster that also has `cozystack.gpu-operator` in `enabledPackages` makes the bundle forward the same gate into both lists, and the platform renders fine while the child kubevirt HelmRelease then stops on the guard: fail-closed, with the gate named, but diagnosed one level below where it was configured. And the version-conditional opt-out is evaluated at render time, so a cluster upgraded from 1.34 to 1.35 keeps `ImageVolume` disabled until something re-renders the chart; the direction is the safe one, since the gate stays off and containerDisk keeps the path it had on 1.8.4. Two things I decided against doing here. The guard's copy of the gate list is pinned against a name being dropped, not against a name being added to the rendered list only; the copy exists because `make update` needs the literal `- VMExport` anchor, so closing the other direction means changing the self-heal contract. And `hack/run-kubernetes-block-io_test.bats` still names a `virt-launcher:v1.8.4` image in a fixture: inert, the test matches on the container name, and the file is outside this change. Tested on a three-node Talos v1.13.6 / Kubernetes v1.36.2 cluster with Flux suspended. The upgrade took about ten minutes. virt-operator had already deployed virt-template before the CR change landed and removed it afterwards, CRDs included. Two VMs started on the 1.8.4 launcher, one DataVolume-backed and one containerDisk-backed, live-migrated to the 1.9.0 launcher. VMDisk plus VMInstance through the apps API boots. `addvolume` on the VM subresource reaches Ready, `removevolume` on the VMI subresource is accepted. VirtualMachineSnapshot and VirtualMachineExport from it reach Ready. Restore from snapshot and PVC clone failed on that cluster inside the LINSTOR API layer, unrelated to this change. CDI v1.66.0 from #3920 on top of it: HTTP imports to Block and Filesystem, blank population and hotplug all work. `helm unittest` is 26/26 for the kubevirt chart and 142/142 for the platform chart, and the update idempotency script passes its eleven cases. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff. No app added or renamed, no variant, release asset, `ApplicationDefinition` or tooling contract touched, and none of the `packages/core/platform/values.yaml` keys the ansible role sets are in the diff. One repository is reached. `packages/core/platform/values.yaml` gains a top-level key, `kubevirt.disabledFeatureGates`, defaulting to the empty list, and `operations/configuration/platform-package.md` on the website is a hand-written table of `spec.components.platform.values.*` that nothing generates. The row is in cozystack/website#678, which lands after this PR merges. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/678 - [ ] [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 chore(kubevirt): update KubeVirt to v1.9.0. Beta feature gates are on by default upstream now. The platform keeps `Template` off, which would deploy virt-template, and `ExternalNetResourceInjection`, which would hand `resourceName` injection for NetworkAttachmentDefinitions to an injector the platform does not run. `ImageVolume` is disabled on management clusters below Kubernetes 1.35: containerDisk-backed VMs are rendered with native image volumes when it is on, and an apiserver whose own ImageVolume gate is off, the default before 1.35, drops those volumes silently, leaving the VM without its disk. On 1.35 and newer the gate stands, and that is the first release where containerDisk-backed VMs are rendered with image volumes instead of sidecars; the bundled Talos ships Kubernetes 1.36, so this is the default there. A `kubevirt.disabledFeatureGates` list in the platform values opts out of any other gate a cluster cannot carry. ``` |
||
|
|
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> |
||
|
|
b9ecb2e39f |
fix(tenantgateway): name the apex on the http-to-https redirect route
The controller-owned redirect route carried no spec.hostnames, relying on the port-80 listener to match every host. Name the tenant apex and its wildcard instead. A route in a tenant namespace has to declare hostnames inside its own apex to pass the route-hostname admission policy, and a route that names none has nothing to compare against that apex. Naming them also states the coverage the route wants rather than inheriting whatever its listener happens to carry. Both hostnames are required: a wildcard is a suffix match over any number of labels, so "*.<apex>" covers every subdomain at every depth but not the bare "<apex>". Coverage narrows in one direction worth stating: a request arriving on port 80 with a Host outside the tenant apex now falls through to a 404 instead of a 301. Skip this route when collecting hostname claims. Before it carried hostnames it contributed nothing to that set, and letting its hostnames through instead provisions listeners and certificates for them, which fails. The check matches on name and controller ownership, not on the labels the renderer stamps, since those are writable by anyone who can create a route. Deriving hostnames from spec.apex makes that field load-bearing for admission, so it gains MinLength=1 and a matching check in the renderer for clusters whose CRD is older than the binary. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
b167fd112a |
feat(gateway): let a tenant pick its Gateway class, and follow that class for TLS termination (#3861)
## What this PR does Lets a tenant publish through a GatewayClass whose provider terminates TLS upstream of the Gateway, and gives the platform the two knobs that bound the choice. The first consumer is the Cloudflare Tunnel controller from #3858, which ends TLS at the Cloudflare edge and reaches the Gateway over the tunnel, but nothing here is tunnel-specific. Until now every cert mode rendered HTTPS listeners with `certificateRefs` and tied the TenantGateway's `Ready` to their issuance. On such a class the platform issued origin certificates nobody serves, and where ACME could not complete, the tenant's `gateway` release never became Ready at all. **Who picks the class.** The platform names a default in `gateway.className` and the set a tenant may choose from in `gateway.tenantSelectableClasses`; a tenant picks its own with `tenant.spec.gatewayClass`. That set is an allowlist rather than free choice because Tenant is a tenant-writable object while GatewayClass is cluster-scoped: without it a tenant could attach its Gateway to a class an admin keeps for internal-only traffic. Naming anything outside the set fails that tenant's own gateway release with a message listing what is allowed, and reaches no other tenant. **Where TLS ends.** That is a property of the class, not of the tenant, so it is declared once in `gateway.edgeTerminatedClasses`, and whichever class a tenant lands on decides its `certMode`. The two therefore cannot be configured out of step: a tenant on a tunnel class cannot keep issuing origin certificates because a separate switch still says otherwise. In `edge` the controller renders the apex, its wildcard and every inheriting child apex as plain HTTP listeners carrying the same label-based `allowedRoutes` as the HTTPS listeners of the other modes, so app HTTPRoutes attach by hostname unchanged. It mints no Issuer, no Certificate and no http-to-https redirect route, and skips the TLS-passthrough listeners an HTTP-only edge cannot serve, which leaves the tenant Kubernetes API, VM export and CDI upload endpoints unpublished through that Gateway. Those TLSRoutes each pinned a listener that no longer exists, so the controller flips the acceptance it wrote for them to `Accepted=False` with reason `NoMatchingParent`. Moving a tenant into the mode removes the ACME objects it owned, and the cluster-wide ACME solver drops its `sectionName: http` pin when the publishing Gateway itself sits on such a class. Pointing `gateway.className` at an edge class is therefore a supported topology (the whole platform behind one terminator), and its cost is that those three endpoints stop being published for the publishing tenant rather than for one tenant among many. Nothing refuses it, because refusing it would forbid the topology; `docs/gateway-classes.md` states the cost where an operator will meet it. Two consequences worth stating plainly. The wildcard-secret controller replicated the operator's TLS Secret, private key included, into every namespace owning a termination point, and a Gateway owner counted as one; a Gateway on an edge class does not, so that tenant's replica is pruned while its neighbours keep theirs. And listing a class as edge-terminated is a claim nothing validates: name the bundled `cilium` and its Gateways publish every app hostname over plain HTTP on a public address. A hard `fail` there would forbid a legitimate topology, a Cilium Gateway behind an external terminator, so it is stated in the platform values, the chart README and the CRD field description instead. **Documentation.** `docs/gateway-classes.md` is the operator-facing page for the whole surface: the three platform values and what each bounds, what a tenant writes to pick a class, how `certMode` is derived and in what precedence, what moving a tenant between classes does to its Issuer, its Certificates, the TLS Secrets those leave behind, the redirect route and the wildcard replica, and what each way of getting the names wrong actually looks like: a class name matching nothing installed renders fine and never programs, a misspelling in `edgeTerminatedClasses` silently leaves the tenant issuing certificates, a default moved out from under a tenant that pinned the old name fails that tenant's release. The chart README keeps the per-mode reference and gains the one constraint it had not stated: the publishing tenant cannot select a class that terminates TLS differently from the platform default, because the cluster-wide ACME issuers derive their solver pin from that default alone. Verified on a v1.6.0-rc.2 dev cluster (Talos, k8s 1.36, cilium 1.19.5) by building the controller from this branch: the CRD enum gained `edge`, the Gateway came up with `edge` / `edge-apex` / `edge-child-*` HTTP listeners and nothing else, no Issuer, no Certificate, no redirect route, and the LoadBalancer Service exposed 80/TCP only. That run also caught a defect no unit test could. The first implementation kept the hostname-less `:80` listener at index 0, and on cilium 1.19.5 `CheckGatewayAllowedForNamespace` is first-listener-wins ([cilium#42159](https://github.com/cilium/cilium/issues/42159), fixed in 1.19.6), so an inheriting tenant's HTTPRoute came back `NotAllowedByListeners` even though a later listener admitted it. Dropping that listener, which in edge mode carries neither ACME nor the redirect, flipped the route to `Accepted=True` and removed a catch-all at the same time. The live check predates the redesign to per-tenant classes, so it exercised the mode rather than the selection path; the selection is covered by unit tests across the four charts and both controllers. Moving a live Gateway from one class to another is covered nowhere. `Gateway.spec.gatewayClassName` has no immutability rule in the shipped CRD, so the apiserver accepts the update; what the outgoing class controller then does with the LoadBalancer Service it provisioned was not exercised by the live run and no unit test reaches it. ### Screenshots Not a UI change. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: `networking/gateway-api.md` documents the platform `gateway.*` rows and the cert modes, so `className`, `tenantSelectableClasses`, `edgeTerminatedClasses` and `tenant.spec.gatewayClass` need a doc PR there; `docs/gateway-classes.md` from this PR is what that port starts from. Not filed yet. - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: the provider hand-writes the Tenant schema, so `spec.gatewayClass` needs an attribute there; not filed yet. - [ ] [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: Two boxes stay unticked on purpose: the website and provider follow-ups are real and not filed yet. I will open them once the shape here settles in review. ### Release note ```release-note feat(gateway): tenant Gateways can now run on a GatewayClass other than the bundled Cilium one. The platform sets the default in `gateway.className` and bounds tenant choice with `gateway.tenantSelectableClasses`; a tenant selects its own with `tenant.spec.gatewayClass`. Classes whose provider terminates TLS upstream are listed in `gateway.edgeTerminatedClasses`, and a tenant Gateway on such a class serves plain HTTP listeners, issues no certificates, and no longer receives a replica of the operator wildcard Secret. Such a Gateway also has no TLS-passthrough listeners, so the `api`, `vm-exportproxy` and `cdi-uploadproxy` entries of `publishing.exposedServices` are not published through it, and their TLSRoutes report `Accepted=False` with reason `NoMatchingParent` ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added edge certificate mode with HTTP-only listeners, no certificate issuance, and no TLS passthrough. * Added configurable GatewayClass selection for tenant-owned Gateways, with platform defaults, allowlists, and validation. * Added platform configuration for edge-terminating and tenant-selectable GatewayClasses. * **Bug Fixes** * Improved wildcard certificate replication for edge-terminated Gateways. * Ensured safe cleanup and restoration when switching certificate modes. * **Documentation** * Updated Gateway, tenant, and certificate configuration guidance for edge termination and GatewayClass selection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
8adf3a0863 |
feat(platform): forward kubevirt.disabledFeatureGates to the kubevirt CR
The kubevirt chart reads disabledFeatureGates from its own values, and the iaas bundle is the only writer of the cozystack.kubevirt Package, so nothing could set it: a Package patched by hand is reverted the next time the platform re-renders it. Platform values gain a documented kubevirt.disabledFeatureGates list, forwarded into the emitted Package as components.kubevirt.values.disabledFeatureGates alongside the entries derived from resources.cpuAllocationRatio and the gpu block. The parenthesized value walk is nil-safe, matching the gpu block, so a cluster that nulls out the whole kubevirt key still renders. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
4c3692b0da |
feat(kubevirt): add disabledFeatureGates and pin the gates the host cannot carry
KubeVirt 1.9 changes what an unlisted Beta gate means. On 1.8 isFeatureGateEnabled ended in `return false`; on 1.9 IsEnabled ends in `return fg.State == Beta`. Sixteen gates are Beta at 1.9.0 and none of them is in this chart's featureGates list, so all sixteen arrive on with the bump. The CR writes three of them back into disabledFeatureGates. Template deploys the virt-template aggregated API server: two Deployments, two APIServices, a webhook and a ValidatingAdmissionPolicy that nothing on the platform uses, and one more aggregated API to break discovery. Upstream also has spec.configuration.virtTemplateDeployment, but it reads TemplateEnabled() first, so it stops the deployment and leaves the gate and its API on; the gate stops both. ExternalNetResourceInjection makes virt-controller skip multus.NetworkToResource, so a NetworkAttachmentDefinition's k8s.v1.cni.cncf.io/resourceName no longer becomes a resource request on the launcher pod. That is right for a cluster running a network-resources-injector, which this platform does not deploy: with the gate on, a VM on an SR-IOV network schedules without its device. ImageVolume is the one that depends on the host, so it is disabled by cluster version rather than always. With it on, virt-controller drops the containerDisk sidecars and mounts the image as a VolumeSource.Image volume. Kubernetes has its own ImageVolume gate, off by default until 1.35, and an apiserver with it off strips every image volume and the mounts referencing it — dropImageVolumes in pkg/api/pod/util.go — so the launcher pod is admitted without the disk it needs and fails looking for it. KubeVirt has no capability check to notice. The management cluster is supported from 1.33, so below 1.35 the chart keeps the gate off and the sidecar path in use, and from 1.35 leaves it alone. The rest of the Beta set stays on, but a cluster may be unable to carry a given gate. A disabledFeatureGates list in the chart values appends to the static opt-outs for that case, and naming a static opt-out again renders one entry, since the operator writing down what their cluster declines has no reason to know the chart already disables it. A gate named in both lists is not a configuration KubeVirt honours, and it fails differently depending on when it lands. The update validator refuses the write and runs with failurePolicy Fail, so on a cluster that already has the CR the HelmRelease stops converging with every other field in it. The create admitter validates nothing and virt-config reads featureGates before disabledFeatureGates, so a fresh install is admitted and the gate stays on while the values file says it is off. The template stops the render and names the gate instead. The guard reads the same version-conditional list as the render, so extraFeatureGates may ask for ImageVolume exactly where the chart is not disabling it. Which list the colliding gate came from decides what the operator can do about it, so the message splits three ways: Template is pinned off and cannot be asked back, a gate the chart enables cannot be switched off from values at all, and anything else arrived through extraFeatureGates and can leave from either side. HotplugVolumes carries a note now. It is deprecated at 1.9 in favour of DeclarativeHotplugVolumes, but virt-api accepts kubevirt-csi's VMI-level removevolume on a VM-owned VMI only while the gate is on, so tenant volume detach depends on the pin, and the pin in turn keeps the newly default-on DeclarativeHotplugVolumes inert. The update target's header stops describing the two lists as curated defaults and says to check them against the pinned release's featuregate registrations at each bump, since a gate that reached GA is enabled whether the CR names it or not. make update grows a marker per line of the new block. Every insert in that target is guarded by one directive it also inserts, so a hand-merge that keeps the directive and drops a sibling line leaves the guard satisfied, the insert skipped and the render green. The three static entries are the only lines these inserts add that are CR content in their own right rather than a wrapper around a value: their absence cannot surface as a value that stopped taking effect, the CR simply goes back to deploying virt-template, to letting KubeVirt skip NAD resource injection, and to image volumes an apiserver below 1.35 discards. The target cannot rebuild a partial block, so it names the missing marker and stops. The block it does rebuild is compared against the committed one byte for byte, because the insert text and the template are two copies of the same lines and only one of them renders. The tail also greps for the guard header the block's skip reads, which no insert restores: losing it makes the template unrenderable, and that is the one hand-merge loss the tail would otherwise wave through. The suite states the cluster version instead of inheriting the test plugin's default, which is unstated and below 1.35, and pins both sides of the boundary. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
3e370e79a0 |
chore(kubevirt): update KubeVirt to v1.9.0
Refetches the vendored operator manifest from the v1.9.0 release. The release turns every Beta feature gate on by default and adds disabledFeatureGates for opting out; the KubeVirt CR side of that lands separately. The virt-launcher QEMU build moves with the bump — el9 qemu-kvm 10.1.0-10 to 10.1.0-20 — and that is the thing to check before moving the release pin, rather than a QEMU major: kubevirt/kubevirt#16386 lost live migration for every running VM on 1.6.2 to 1.7.0, one build apart within the same QEMU version, on virtio feature bits the target launcher would not accept. VMs running on the 1.8.4 launcher were migrated onto the 1.9.0 one on a three-node test cluster before this bump. The update target now carries the check as a note above the pin. make update used to wipe templates/ before fetching, which also removed the local PrometheusRule template; it now only overwrites the file it fetches. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7b0c35b9b3 |
feat(monitoring): add KubeVirt VM dashboard (#2198)
## Summary - Adds `dashboards/vm/virtual-machines.json` — KubeVirt VM resource metrics (CPU, memory, network, storage) with namespace and VM template variables - Adds `dashboards/vm/console-logs.json` — VM serial console log viewer via VictoriaLogs with namespace filtering - Adds `vm/virtual-machines` and `vm/console-logs` entries to `dashboards.list` ## Problem `dashboards.list` references `vm/virtual-machines` and `vm/console-logs` but the corresponding JSON files don't exist. The console-logs dashboard includes `kubernetes_namespace_name` filtering to prevent cross-tenant log exposure. Relates to #2194 ## Test plan - [ ] Verify JSON is valid for both dashboards - [ ] Import dashboards in Grafana and verify panels render with KubeVirt metrics - [ ] Verify console-logs queries include namespace filter ```release-note Add KubeVirt VM dashboards (virtual-machines and console-logs) ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * **VM Console Logs dashboard**: View live console output with namespace and VM filtering, fast refresh, and improved log presentation (details, wrapping, and sorting). * **Virtual Machines dashboard**: Rich VM monitoring covering CPU, memory, network, storage I/O, latency, swap/page faults, and filesystem usage with thresholds and formatted visuals. * **Dashboard catalog updated**: Added multiple new monitoring dashboards for broader coverage and easier discovery, including the new VM dashboards. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
5976ddf02f |
fix(gateway): normalise the GatewayClass name and refuse a blank one
The class lists arrive over the values channel comma-joined, so every reader trims each entry as it splits them. The scalar those entries are compared against was not trimmed anywhere. A class name carrying a stray space therefore failed `has` against its own padded self, and the two readers of that scalar disagreed about the same class: the gateway chart put the tenant on an edge-terminated class, while the cluster-wide ACME issuers still pinned their solver to a listener an edge Gateway does not render. That stops every HTTP-01 certificate in the cluster from issuing, with nothing in any status naming the cause. Normalise once at the single writer so both readers see a clean value, and keep a defensive trim at each reader. The tenant's own choice does not travel that channel — it comes from tenant.spec.gatewayClass — so it is trimmed where the gateway chart reads it. Unset and whitespace-only are different events and no longer share an outcome. Unset means the choice was not made, and falling back to the bundled class stays its documented behaviour. Whitespace-only means a choice was made and is garbage, in the field that decides where TLS terminates: resolving it quietly would take a tenant the operator meant to put behind an edge terminator and publish it directly, with a real certificate on a real address. Both the platform default and the tenant's own choice now refuse it by name. The kind check beside that refusal is part of the same fix, not a tidy up. Through Helm's values path an unquoted numeric scalar arrives as a float64, and the whitespace check compares it against a string, so `className: 123` dies at that comparison — one line before the trim — with an error naming neither the key nor the file. Refusing whitespace without checking the kind first would therefore have made the numeric case worse than it was. `kindIs "string"` is the right test because it never has to name the type that actually arrived. The order of the guard's steps is part of the guard. The value is read before anything normalises it, so a default that fires on any falsy value cannot launder 0 or false into the empty string and resolve them to the bundled class ahead of the kind check. Unset is the one case that still falls back, and each of those four inputs is pinned, so a normalisation drifting back ahead of a check fails rather than quietly covering only the loudest bad input. Also correct a cleanup log line: the wildcard Certificate is deleted for every mode that is not DNS-01, so naming HTTP-01 in that message is wrong for existingSecret and for edge alike. And state the listener name invariant plainly: the cap is 253, not the 63 of a DNS label, and the longest name the builders produce is 84. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
1b2beac3f8 |
[cilium] Set resources requests/limits on cilium-operator (#2618)
## Summary Companion to #2617 (cilium-agent). The upstream cilium chart ships with `operator.resources: {}`, which triggers a `no CPU limit` conformance warning on the `cilium-operator` Deployment. Override `packages/system/cilium/values.yaml` to set the sizing recommended by the upstream `values.yaml` comment: - `limits` : `cpu: 1000m`, `memory: 1Gi` - `requests` : `cpu: 100m`, `memory: 128Mi` ## Test plan - [ ] `helm template . --namespace kube-system` from `packages/system/cilium/` renders a `resources` block on the `cilium-operator` container of the Deployment (verified locally) - [ ] Re-run the conformance/lint tool — the `cilium-operator has no CPU limit` warning should be gone - [ ] CI passes ### Release note \`\`\`release-note Set CPU/memory requests and limits on the cilium-operator Deployment to clear the "no CPU limit" conformance warning. Values follow the upstream chart's recommended sizing (limits 1000m/1Gi, requests 100m/128Mi). \`\`\` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated Cilium system component resource configuration with CPU and memory requests and limits. [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2618) <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
487cafac3e |
feat(platform): let a tenant pick its Gateway class from an allowlist
Tenant Gateways were pinned to the bundled Cilium class. Give the platform a default in gateway.className, the set a tenant may choose from in gateway.tenantSelectableClasses, and the set whose provider terminates TLS upstream of the Gateway in gateway.edgeTerminatedClasses. A tenant picks its own with tenant.spec.gatewayClass, read only when it owns a Gateway; an inheriting tenant runs on its ancestor's. The selectable set is an allowlist rather than free choice because Tenant is a tenant-writable object while a GatewayClass is cluster-scoped: without it a tenant could attach its Gateway to a class an admin keeps for internal-only traffic. Naming anything outside the set fails that tenant's own gateway release and reaches no other tenant. Where TLS ends is a property of the class, so the gateway chart derives certMode from whichever class the tenant landed on instead of taking it as a separate input. The two therefore cannot be configured out of step. The publishing tenant cannot cross that line by itself. The cluster-wide ACME issuers point their HTTP-01 solver at its Gateway and are rendered from the platform default alone, so a mismatch would pin the solver to a listener that Gateway does not have. Those issuers now drop the sectionName pin when the default class is edge-terminated, letting the challenge attach by hostname instead. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
2ef0e8d3a1 |
feat(gateway): add an edge cert mode for upstream-terminated classes
Some GatewayClasses have their provider terminate TLS in front of the Gateway and reach it over plain HTTP. On such a class the platform issued origin certificates nobody serves, and where ACME could not complete, the Gateway's listeners never became ready. Add certMode=edge to TenantGateway. The controller renders the apex, its wildcard and every inheriting child apex as plain HTTP listeners carrying the same label-based allowedRoutes as the HTTPS listeners of the other modes, so application HTTPRoutes attach by hostname unchanged. It mints no Issuer and no Certificate, and removes the http-to-https redirect route, because the edge performs that redirect itself. TLSPassthroughServices are not rendered: an HTTP-only edge cannot carry a TLS listener, which leaves the endpoints relying on one unpublished through such a Gateway. The hostname-less port-80 listener is left out. It carries the ACME challenge and the redirect in the other modes and has neither here, so it would only be a catch-all admitting any hostname. Leaving it out also keeps a hostname-pinned listener at index 0, which is what Cilium's first-listener-wins namespace check reads before the fix in cilium/cilium#45693. Moving into the mode garbage-collects the Issuer and Certificate objects the controller owned. The TLS Secrets they issued stay behind: cert-manager runs with enableCertificateOwnerRef false, so a Certificate does not own the Secret it issued into. Every mode transition already behaves that way. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
a2cb47c961 |
feat(gateway): update the CRD bundle to Gateway API v1.6.1 and gate GRPCRoute hostnames (#3872)
## What this PR does Bumps the vendored Gateway API CRD bundle in `packages/system/gateway-api-crds` from v1.5.1 to v1.6.1. The package Makefile pins the upstream ref, so the change is that pin plus the re-rendered `templates/crds-experimental.yaml`. The bundle version is not only bookkeeping: it is a compatibility signal consumers read at runtime. Every CRD in the bundle carries a `gateway.networking.k8s.io/bundle-version` annotation, and a Gateway API controller can read it off the `gatewayclasses` CRD to decide whether the installed bundle matches the one it was built against. Controllers that do this typically accept any patch release but require an exact major and minor match, and report `SupportedVersion=False` with reason `UnsupportedVersion` on their GatewayClass when the minor differs, a status condition that surfaces the skew rather than a functional break. The Cloudflare Tunnel Gateway API controller proposed for the platform in #3858 is built against Gateway API v1.6.1 and performs exactly that comparison, so against the v1.5.1 bundle shipped today its GatewayClass reports the bundle unsupported, and with this bump it reports supported. Neither change depends on the other landing first: this PR only moves the bundle, and the check lives entirely on the consumer side. One consequence worth stating plainly, since it will outlive this PR: an exact-minor check makes the shipped bundle version a coupled pin for any such controller. A future bump to v1.7.x flips the same condition back to `UnsupportedVersion` until that controller is rebuilt against the newer bundle, so the two want to move together. The bundle still emits the same twelve CustomResourceDefinitions and the same two `safe-upgrades.gateway.networking.k8s.io` admission objects, and every CRD still carries the `helm.sh/resource-policy: keep` stamp the Makefile's post-processing step adds. One of the policy's two CEL rules is rewritten, though, and it changes what a cluster accepts. That has its own section below. Two kinds change their version surface: TCPRoute and UDPRoute gain a served `v1` that takes over as the storage version, and their `v1alpha2` stays served with a deprecation marker. Every other kind keeps exactly the versions it served at v1.5.1. That matters because the Cilium operator resolves GatewayClass, Gateway, HTTPRoute and GRPCRoute in `v1`, ReferenceGrant in `v1beta1` and TLSRoute in `v1alpha2` before it enables its Gateway API controller; all six are still served by the v1.6.1 bundle. TLSRoute in particular keeps `v1alpha2`, which the tenant gateway controller, the platform TLSRoute templates and the tenant hostname admission policy all address. Schema changes are small and land on fields nothing in this repository sets. `sessionPersistence.idleTimeout` is gone from HTTPRoute, GRPCRoute and XBackendTrafficPolicy, removed upstream under GEP-1619 because no implementation ever implemented it and HTTP cookies have no native idle-timeout mechanism, so no working behaviour is lost even though the field disappears from the schema; HTTPRoute gains a lower bound on `retry.attempts` and now rejects duplicate `retry.codes`; ReferenceGrant requires `spec` at the root. In the other direction, Gateway infrastructure annotations and frontend CA certificate references go from a limit of 8 to 16, and TLSRoute hostnames from 16 to 1024. No CRD schema loses or changes a CEL validation rule, and the six added belong exclusively to the new TCPRoute and UDPRoute `v1` schemas. ### The safe-upgrades admission policy accepts a different set of bundles The experimental kustomization ships a `ValidatingAdmissionPolicy` and its binding, both named `safe-upgrades.gateway.networking.k8s.io`, next to the twelve CRDs. The binding is `validationActions: [Deny]` with `failurePolicy: Fail`, matching CREATE and UPDATE on `apiextensions.k8s.io/v1 customresourcedefinitions` cluster-wide, and Cozystack installs it in two places: on the host through `cozystack.gateway-api-crds`, and inside every guest cluster that sets `addons.gatewayAPI.enabled`. Its second rule reads the `gateway.networking.k8s.io/bundle-version` annotation off an incoming CRD. At v1.5.1 that rule was a blacklist: reject anything matching `v1.[0-4].\d+` or containing `v0`, allow the rest. At v1.6.1 it is a whitelist: allow `v0.0.0-dev` outright, otherwise require the annotation to start with `v1.` and not match `^v1\.[0-4](\.|$)`. Evaluating both expressions over a spread of annotation values: | bundle-version | v1.5.1 policy | v1.6.1 policy | | --- | --- | --- | | `v1.6.1`, `v1.5.1`, `v1.5.0-dev`, `v1.10.0` | allow | allow | | `v1.4.0`, `v1.4.10`, `v1.0.0` | reject | reject | | `v0.0.0-dev` | reject | **allow** | | `v2.0.0` | allow | **reject** | | `1.6.1` (no leading `v`) | allow | **reject** | | `v1.4` (no patch) | allow | **reject** | | `main`, empty string | allow | **reject** | Nothing in this repository trips the new rejections: every CRD the bundle ships is stamped `v1.6.1`. What is exposed is an operator applying a Gateway API CRD set from somewhere else on a cluster that carries the policy. A fork, a vendor build or a nightly whose annotation reads `1.6.1` or `v2.0.0` used to go in and is now denied with `reason: Invalid`, naming a policy the operator may not know is installed, and whatever HelmRelease owns those CRDs then sits un-Ready. Uninstalling the policy is the escape hatch, and the denial message says so. The same hunk fixes an upstream inconsistency: at v1.5.1 the policy pair was stamped `v1.5.0-dev` while its twelve CRDs said `v1.5.1`. At v1.6.1 all fourteen documents agree. The Gateway and HTTPRoute objects the tenant gateway controller renders were serialized across all three certificate modes and checked against both the v1.5.1 and the v1.6.1 schemas; the two runs are identical, so the bump does not change what the controller can create. `go.mod` still pins `sigs.k8s.io/gateway-api` v1.4.1 and this PR deliberately leaves it there. The CRD bundle and the Go client types have been on separate versions since before this change, and nothing the controller emits depends on the gap. One thing to know before the next bump: from v1.6 on, the two things upstream calls the experimental bundle disagree. This package follows `config/crd/experimental`, the kustomization, which emits 12 CRDs at v1.6.1; the published `experimental-install.yaml` for the same tag emits 13, the extra one being `xbackends.gateway.networking.x-k8s.io`, new in v1.6 and not listed in the kustomization. At v1.5.1 the two agreed, so there was nothing to choose between. Nothing in this repository references XBackend and no vendored CRD depends on it, so following the kustomization costs nothing today, but whoever bumps next inherits the choice, and it should be made deliberately rather than by whichever command gets typed. The second commit corrects a comment this bump invalidated. It claimed TLSRoute stays at `v1alpha2` through Gateway API v1.5 and that the promotion to `v1` was still upstream-only; TLSRoute has been served at `v1` since v1.5.0, so the claim was already wrong before this bump and the version range it named is now wrong too. The third commit closes a gap that correction exposed. `cozystack-route-hostname-policy-tls` named `v1alpha2` alone while the bundle serves `tlsroutes` at `v1`, `v1alpha2` and `v1alpha3`: three served versions against one named. The gap is not new and this bump neither creates nor widens it: the v1.5.1 bundle and the v1.6.1 one serve the same three versions, with the same two marked deprecated. Coverage of the two unnamed versions rested entirely on request conversion, because `matchPolicy` is unset on both rules in that file and so defaults to `Equivalent`, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by `failurePolicy: Fail`, because nothing failed. The rule now names all three, which is the shape the HTTPRoute rule above it already used, and `hack/route-hostname-policy-version-coverage.bats` reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, so a future bundle move cannot outrun the list silently, in either direction. That guard carries negative fixtures rather than only watching a correct tree pass. Three of them drive `assert_covers` itself, with the two readers redefined in a subshell, against an omitted served version, an empty served set and an empty named set. Each requires the specific complaint text rather than merely a non-zero exit, so a helper renamed out from under it fails for the right reason instead of reading as a successful rejection. A fourth drives the comparison helper directly. The fourth commit pins the vendored bundle version in the CRD package's unit suite. Nothing outside the package Makefile named a Gateway API version, so re-running `make update` against a different ref landed green: twelve CRDs, every one stamped, document count unchanged, suite passing. The new case reads the `gateway.networking.k8s.io/bundle-version` annotation off every rendered CRD and fails when it is not the version the Makefile pins. The fifth commit extends the version-coverage guard to `cozystack-gateway-hostname-policy`, which names the `gateways` versions literally the same way the two route policies do, and whose version set lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Two notes for whoever reads the release: the TCPRoute and UDPRoute storage version moves from `v1alpha2` to `v1`, and while nothing in this repository ships either kind and the CRDs declare no conversion block, an upgraded cluster accumulates both entries in `status.storedVersions` — inert today, load-bearing only if a later bundle stops serving `v1alpha2`. The sixth commit gives GRPCRoute the same hostname rule the other two route kinds have. GRPCRoute carries `spec.hostnames` and it can reach a tenant Gateway: the port-443 listeners pin `allowedRoutes.kinds` to HTTPRoute and TLSRoute, but the port-80 listener leaves `kinds` unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could claim any hostname on the plaintext listener with nothing checking it. Tenants hold no `gateway.networking.k8s.io` RBAC in Cozystack, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The rule is the HTTPRoute rule with `grpcroutes` in place of `httproutes` and the one version the bundle serves for it, and the coverage guard pins that list to the bundle like the others. Several places in the tree enumerate which route kinds the hostname policies cover, and all of them named two: the port-443 kinds invariant in the tenant gateway controller and the test that pins it, `packages/extra/gateway/README.md`, and the threat model together with the self-assessment. They name three now. The port-443 kinds set also carries an accurate reason now. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. The README had also described HTTPS and TLS-passthrough listeners as carrying different kinds, where the controller gives both the same one. One more commit makes the suite's existing assertions bite. A ValidatingAdmissionPolicyBinding whose `policyName` points at another policy renders without complaint and leaves its own policy inert, and the suite pinned `validationActions` on the bindings while never pinning `policyName` at all, so pointing the GRPCRoute binding at the HTTPRoute policy kept every case green with the GRPCRoute control doing nothing. All three bindings now pin the policy they name. In the same file, `uncovered()` assigned to the variable names its caller reads back when printing a mismatch, so a call site with the arguments swapped would have printed a diagnostic naming the wrong side. Two things are named rather than made. The version-coverage guard collects `apiVersions` from every `resourceRules` entry mentioning the resource without also matching on `apiGroups` or `operations`, so a rule in another group, or one scoped to a single verb, would donate its versions to the set coverage is measured against; the chart's own suite pins the group and the resource on the rule it checks, which closes that a layer below. And `packages/system/cozystack-api/templates/api-tlsroute.yaml` renders `gateway.networking.k8s.io/v1alpha2` while the bundle serves `v1` as storage and marks `v1alpha2` deprecated, but the identical pin sits in the kubevirt and kubevirt-cdi TLSRoute templates, and the pinned `sigs.k8s.io/gateway-api` carries no `apis/v1` TLSRoute type for the controller side to follow, so moving one of the three here would leave the tree less consistent than leaving all three. Closes #3789. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file: neither `packages/system/gateway-api-crds/Makefile` nor its rendered CRD template matches any entry. Leaving every box empty rather than ticking the "no downstream repository" line, because the docs site carries a Gateway API version statement that is already inaccurate and needs a decision that is not mine: `content/en/docs/next/networking/gateway-api.md` and its v1.5 and v1.6 copies say Gateway API v1.5 ships TLSRoute at `v1alpha2` and that the graduation to `v1` is still upstream-only, when v1.5.0 already shipped TLSRoute `v1` as the storage version. ### Release note ```release-note chore(gateway-api-crds): update the vendored Gateway API CRD bundle to v1.6.1. TCPRoute and UDPRoute gain a served v1 that becomes their storage version, with v1alpha2 still served and now deprecated; every other kind keeps the versions it served at v1.5.1. Upstream removed `sessionPersistence.idleTimeout` from HTTPRoute, GRPCRoute and XBackendTrafficPolicy; the field is not set by anything Cozystack renders, but a hand-written route that carries it loses it silently on its next write, with no admission error. The bundle's `safe-upgrades.gateway.networking.k8s.io` ValidatingAdmissionPolicy changes which CRDs it accepts. Cozystack installs it cluster-wide on the host and in every guest cluster with `addons.gatewayAPI.enabled`. Its `gateway.networking.k8s.io/bundle-version` rule was a blacklist on two patterns and is now a whitelist on the `v1.` prefix with a `v0.0.0-dev` escape hatch. A bundle annotated `v0.0.0-dev` is now accepted where it was denied. A bundle annotated `v2.0.0`, `1.6.1` without the leading `v`, `main`, `v1.4` without a patch, or carrying an empty annotation is now denied where it was accepted. Nothing Cozystack ships is affected, since every CRD in the bundle is stamped `v1.6.1`, but applying a Gateway API CRD set from a fork, a vendor build or a nightly on such a cluster can now fail admission with `reason: Invalid`. Uninstalling the policy remains the documented escape hatch. fix(cozystack-basics): the tenant TLSRoute hostname admission policy now names every served TLSRoute version (`v1`, `v1alpha2`, `v1alpha3`) instead of `v1alpha2` alone. Coverage of the other two rested on request conversion, which holds only while a named version stays served, and a rule left selecting nothing is not caught by `failurePolicy: Fail`. feat(cozystack-basics): GRPCRoute hostnames in `tenant-*` namespaces are now gated by the same admission policy that gates HTTPRoute and TLSRoute. A tenant Gateway's port-80 listener leaves `allowedRoutes.kinds` unset, which admits GRPCRoute by protocol default, so a GRPCRoute could claim any hostname on the plaintext listener unchecked. Tenants hold no `gateway.networking.k8s.io` RBAC, so this is defence-in-depth against a chart bug rather than a tenant-user control. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added hostname admission validation for `GRPCRoute` resources. - Expanded `TLSRoute` hostname validation to support all served Gateway API versions. - Ensured route hostname policies consistently cover HTTP, TLS, and gRPC routes. - **Security** - Updated listener policies to restrict port-443 routes to supported HTTP and TLS route types. - Clarified fail-closed hostname validation behavior across Gateway, Route, and Ingress resources. - **Documentation** - Updated security and threat-model documentation to include GRPCRoute handling and validation coverage. - **Maintenance** - Updated the bundled Gateway API CRDs to version 1.6.1. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
b7e5100e73 |
fix(monitoring): strip infrastructure metadata from tenant logs (#2196)
## Summary - Adds a `modify` filter in fluent-bit that removes `kubernetes_node_name` and `kubernetes_host` from all container logs - Prevents infrastructure topology (node names, hosts) from leaking to tenant users ## Problem `kubernetes_node_name` is exposed in log metadata, leaking node topology to end-users. Other sensitive fields like `kubernetes_host` are also visible. Relates to #2194 ## Test plan - [ ] `helm template` monitoring-agents and verify `Remove kubernetes_node_name` filter is present - [ ] Deploy and verify `kubernetes_node_name` is absent from logs in tenant Grafana ```release-note Strip infrastructure metadata (node name, host) from tenant logs ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved Kubernetes monitoring log processing by filtering out most `kubernetes_*` fields for `kube.*` events, while preserving the key Kubernetes identity fields (such as pod, namespace, and container-related names) for clearer, more consistent log output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
fcab5dbb07 |
docs(rabbitmq): carry the TLS consequences to where the toggle is rendered
cozyvalues-gen propagates the @field line and nothing under it, so the description reaching values.schema.json, the README table and the dashboard form said only that TLS turns on, while the continuation lines holding the breaking part of the change stayed in values.yaml. Fold the listener, binding-port and metrics-port moves into the @field line, and leave the continuation with the part that is genuinely about the struct. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
af003fd520 |
docs(rabbitmq-rd): describe the trust-anchor include by its real mechanism
Both the cozyrds comment and the bats header said the chart labels its CA Secret for publication. Nothing labels that Secret: the chart names it in a TenantProjection sentinel, and the CA-extraction controller stamps internal.cozystack.io/tenant-ca on the key-free projection it writes. A reader who went looking for the missing label on the CA Certificate would have found nothing, and the obvious repair — a secretTemplate stamping the label onto the CA Secret — would offer a tenant the CA private key. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
411a3d6c02 |
docs(rabbitmq): cut template comments duplicating the README
The TLS comments had grown to narrate rejected alternatives and operator internals at length, roughly matching the volume of configuration they annotate. The README already carries every long-form explanation, so the duplicates are prose that will drift out of sync with it. Each block keeps the mechanism a next editor needs to avoid breaking something and points at the README section holding the rest. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
36ee8a2608 |
feat(rabbitmq): make TLS opt-in instead of following external
tls.enabled inherited the value of external when unset, so enabling external access also enabled TLS. That is not free on an existing instance: setting spec.tls.secretName makes the operator rewrite the port key of the <release>-default-user Secret from 5672 to 5671, and that Secret is the Service Binding handed to the tenant. Every client resolving its endpoint from the binding — the documented way — would move onto TLS on upgrade and have to verify a per-release CA it has no key-free way to obtain. Resolve unset, null and false to false instead, so TLS is only ever on because someone asked for it. enabled becomes a plain bool now that there is no third state to express. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
d668dfbd6a |
feat(rabbitmq): add opt-in switch to close the plaintext listeners
TLS so far only added encrypted endpoints; nothing closed the unencrypted ones, and the chart had no way to express the intent. tls.disableNonTLSListeners gives the operator that switch, defaulting to false so an upgrade never closes a listener a client is already using — the choice to break plaintext has to be made deliberately. Its reach is worth stating plainly, because the natural reading is wrong in both directions. It writes `listeners.tcp = none` into rabbitmq.conf, which is broker level: the socket is not opened on any interface, so in-cluster clients lose plaintext at the same instant external ones do, and there is no configuration that closes it outside while keeping it inside. In the other direction it is not "TLS everywhere" either — Erlang distribution on 25672 and epmd on 4369 ride the headless Service, which the flag never touches, so inter-node traffic stays in the clear regardless. The flag also cannot coexist with chart-managed users. messaging-topology-operator derives its connection as TLSEnabled() && (!connectUsingHTTP || DisableNonTLSListeners()), so the flag forces TLS on that side whatever the operator's own configuration says, and it verifies the broker against x509.SystemCertPool(), which cannot hold a per-release CA. The connection-uri annotation is no escape, since it would point at the port the flag just closed — so that annotation is now omitted when the flag is set, rather than advertising a closed port. Every User, Vhost and Permission would silently stop reconciling. Rendering fails instead, naming the conflict, and the flag stays usable for an instance whose users and vhosts are managed outside the chart. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |