mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-29 08:07:33 +09:00
feat/vm-disk-http-auth
382
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
f9ea0da3de |
feat(backups): Kafka topic-metadata backup via a dedicated strategy driver (#4000)
## What this PR does Adds a dedicated `strategy.backups.cozystack.io/Kafka` Go driver that backs up the **topic metadata** of a Cozystack-managed `Kafka` application and wires it into the platform `cozy-default` flow — the counterpart to the app-specific drivers (CNPG, MariaDB, Altinity, MongoDB). It is a typed variant of the generic Job strategy: the controller renders a one-shot batch Job that talks to the Kafka Admin API, and adds on top an applicationRef Kind gate, a `Ready` precondition on the Strimzi `Kafka` cluster, and a per-run object key via `.BackupName` so distinct runs never overwrite each other. What it captures: every non-internal topic's partition count, replication factor and non-default configs (via `kafka-topics --describe`), stored as one small object per run in the system bucket; restore recreates them with `kafka-topics --create` + `kafka-configs --alter`. Deliberately **out of scope**: message payloads (a volume-snapshot strategy's job), consumer-group offsets (a log position, only meaningful with a byte-identical data restore), ACLs, quotas, and `KafkaUser`s. No PITR. Restored topics are created directly through the Admin API, so on the target they are unmanaged (no `KafkaTopic` CR) — the accepted tradeoff of an Admin-API-only driver. Both restore flows are implemented: **in-place** recreates dropped topics into the source, and **to-copy** applies the source's topics onto a freshly-bootstrapped empty `Kafka` (the "bootstrap a cluster, then apply the metadata state" flow). Contents: - Driver: `api/backups/strategy/v1alpha1/kafka_types.go`, `internal/backupcontroller/kafkastrategy_controller.go` (+ unit tests for its pure helpers), `internal/backupcontroller/kafkatypes/` (Strimzi `Kafka` readiness gate), dispatch cases in both controllers + `supportedBackupStrategyKinds()`, scheme registration, generated CRD + deepcopy. - Chart: `strategy-kafka-default.yaml` (gated on the bucket name), a `cozy-default` binding, read RBAC on `kafka.strimzi.io/kafkas`, and the Strimzi Kafka client image value. - Docs: a Kafka section in `docs/operations/backup-classes.md`. - Example + e2e: `examples/backups/kafka-metadata/` and `hack/e2e-chainsaw/kafka-metadata/` (`kafka-3-metadata-roundtrip`), an honest round-trip that proves a per-run `retention.ms` sentinel survives both restores. The original `packages/apps/kafka` chart is left untouched (it has no legacy backup mechanism). Validated: `go build ./...`, `go vet`, `go test ./internal/backupcontroller/...` (pure helpers plus a reconcile Kind-gate test and the RestoreJob cleanup regression test), `helm unittest` on the package (which now renders `strategy-kafka-default.yaml` and asserts its kind/name/`artifactURITemplate`), the `select-install`/`select-e2e` bats suites, `helm template` of the strategy CR, `bash -n` of the rendered script, and `make generate` (clean). The end-to-end round-trip (`kafka-3-metadata-roundtrip`) is enabled and mapped like the sibling backup suites, so CI builds this branch's controller and runs it live against a fresh sandbox; it has not been merged-and-run before, so that first live run must go green before merge. ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. This PR adds a new `strategy.backups.cozystack.io/Kafka` CRD, a Go driver, chart wiring, docs and examples. It does not add or rename a `packages/apps`/`packages/extra` package, does not change any `ApplicationDefinition`, `values.schema.json`, version enum, `release.prefix`, output Secret/Service, the `Package`/`Plan`/`RestoreJob` CRDs the provider types by hand, any `hack/*.mk` anchor or make-target behaviour, node prerequisites, labels/annotations, or telemetry metrics. A brand-new `strategy.backups.cozystack.io` CRD travels to the Terraform provider as opaque JSON (it types only `Plan`/`RestoreJob`/`Package`), so it keeps working untouched. None of the downstream triggers match. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(backups): add a Kafka topic-metadata backup strategy bound by cozy-default — backs up topic definitions and configs (partitions, replication factor, non-default configs) via the Kafka Admin API, with in-place and to-copy restore; message data, consumer offsets, ACLs, quotas and KafkaUsers are out of scope ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added Kafka topic-metadata backup and restore support, excluding message data. - Supports ad-hoc and scheduled backups, in-place restoration, and restoration to a separate Kafka cluster. - Recreates missing topics and restores compatible configurations while validating partition and replication-factor constraints. - Waits for Kafka readiness and cleans up backup artifacts after completion. - Backup and restore jobs now enforce configurable deadlines to prevent stalled operations. - **Documentation** - Added guidance covering Kafka metadata backups, restoration limitations, and S3 configuration. - **Tests** - Added end-to-end coverage for backup, restoration, validation, and cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
ca2a6d6429 |
fix(backups): warn when a Kafka backupTimeout is rejected
kafkaRunDeadline silently substituted the 30m default for a backupTimeout that time.ParseDuration rejects or that falls below the one-minute floor, with no Event, condition or log line. An operator who set an unparseable or sub-minute value, then raised it, saw every run keep dying at DeadlineExceeded with nothing naming the discarded parameter. Return the applied deadline together with a message when the configured value is rejected, and record it as a BackupTimeoutInvalid Warning Event on the BackupJob and RestoreJob (the reconcilers already hold a Recorder). An honoured or unset value stays silent. TestKafkaRunDeadline now asserts the warning fires for the rejected cases and not otherwise. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
b3b280ea73 |
feat(gateway)!: add TLS-passthrough listeners to TenantGateway (#3342)
## What this PR does Phase 1 of external database exposure via Gateway API TLS-passthrough (#2815): adds a `tlsPassthroughListeners` field to `TenantGateway` and renders one layer-4 `mode: Passthrough` listener per entry, on a native database port with a per-engine subdomain hostname, alongside the Gateway's terminate listeners. No engine is wired: no TLSRoute, no certificate or CA changes. The existing layer-7 `tlsPassthroughServices` field gains a 62-entry count bound, an item length and a DNS-1123 item pattern, and a repeated entry now fails the reconcile with `Ready=False` instead of rendering the same listener twice. `spec.apex` gains a 253-character bound and nothing else: the CEL rules concatenate it, so the install-time cost estimator refuses the CRD without a declared bound. It gains no format pattern, and that is not a claim that a mixed-case host works. Such a host reaches the field verbatim from a namespace label, the renderer then composes listener hostnames Gateway API refuses, and that tenant has no Gateway either way; what a pattern would change is only that the write is refused, which fails this HelmRelease where the same fault costs the one Gateway. An apex no listener hostname can sit within is reported by the controller instead, and only when `tlsPassthroughListeners` is in use. The constraints and what they do to objects that already exist are described further down. The bound on an entry is what the schema can state on its own. An entry also becomes a listener name and a hostname built from the apex, so the controller checks both composed values against the 253 characters Gateway API allows and names whichever one overflows. Validation rejects duplicate listener names or ports, ports reserved by the platform Gateway (80 and 443), out-of-range ports, invalid DNS-1123 names, and hostnames outside the tenant apex (mirroring the Gateway hostname admission policy so an invalid entry cannot make the whole Gateway rejected). The SNI-overlap rule refuses two passthrough listeners that answer the same name on different ports. That is what [cilium#42898](https://github.com/cilium/cilium/issues/42898) reports: only one of the two routes ever works. It does not extend to a terminate listener and a passthrough listener sharing a name across ports. No failure is reported for that shape, and Cilium counts a terminate/passthrough pair as conflicting only when they share a port. The per-engine subdomain hostname scheme follows the phase-1 gate. The flat-hostname alternative (pending the Cilium listener-isolation fix targeting 1.20) is a later scheduling decision that does not change this API field. One shape differs from #2815 on purpose. The ticket has `hostname` optional with a `*.<name>.<apex>` default; here it is required. Deriving the SNI from the listener name hides the one value an operator has to get right, and the two drift apart the moment the naming scheme changes. Found while checking that pairing, unrelated to this field: a stock `dns01` gateway already renders `tls-api` (passthrough, 443) beside `https` (terminate, 443, `*.<apex>`), one port with overlapping SNI, which is the shape Cilium 1.19.6 marks `Conflicted`. That belongs to the Cilium bump and is recorded on #3343. Also named rather than fixed: the controller-side rules on this field stop the whole reconcile when they refuse a spec. `renderGateway` returns an error, so the Gateway is not updated at all, and the tenant sees `Ready=False` with `ReconcileError` while new app listeners and every certificate the controller would issue or amend wait behind it. Certificates already issued go on renewing, since cert-manager drives that from the objects already in the cluster and an aborted reconcile writes nothing to them. Dropping only the conflicting listener and reporting it on status would be the better shape, but that changes the error model of the whole renderer rather than one rule, so it is out of scope here. The current behaviour is at least observable: `Ready=False` is a signal, not silent drift. On HTTP-01, a hostname a passthrough listener answers earns no HTTPS-terminate listener once a TLSRoute claiming that hostname can be served on the listener, matched by SNI overlap so a wildcard entry reaches a name beneath it as readily as the name written in the spec, and the field itself is accepted under certMode http01 alone. That refusal is stated as the set of modes that can carry a passthrough listener rather than the set that cannot, because a mode added to the enum would otherwise be admitted by default: dns01 and existingSecret serve the whole apex from one wildcard terminate listener the pinned Cilium cannot keep apart from a passthrough listener beneath it, and edge ends TLS at the class provider and renders no TLS listener for one to sit beside. The route half of that condition is load-bearing rather than incidental. On the pinned v1.19.5, tlsPassthroughFilterChains walks a listener's routes and skips one with no backends, so a declared passthrough listener nothing attaches to contributes no filter chain and matches no ClientHello, while the terminate listener's chain is built from its own Secret and carries the name alone. Keying the withdrawal on the declaration would therefore take a name that is being served and hand it to a listener that forwards nowhere, and that is the ordinary shape rather than a corner: the chart puts api, vm-exportproxy and cdi-uploadproxy on every tenant Gateway, while there is one platform TLSRoute per name cluster-wide and each names one tenant's Gateway as its parent, so every other tenant renders all three entries with a route on none. The backend half is that same rule reached by a different road: the chain is built per route out of the backends that survived resolution, so a route that attached and forwards nowhere puts no chain on the name either, a backendRef naming a Service that does not exist yet because manifests applied out of order, one deleted since, a name with a typo, or one in another namespace with no ReferenceGrant, and without that half an ordinary backend mistake takes a live endpoint offline while Cilium builds nothing to replace it. Resolution asks what v1.19.5 asks and no more: the reference allowed, the Service present, and neither its ports nor its endpoints read, because the translator reads neither. Services and ReferenceGrants are watched alongside routes, so both halves are deferrals rather than exemptions and the withdrawal lands on the pass that sees the missing piece arrive. The withdrawal reaches HTTP-01 alone, because `collectHostnameClaims` returns nothing under the other two modes and they render no per-hostname listener to withdraw. A TLSRoute that declares no `spec.hostnames` claims the hostname of each rendered passthrough listener its parentRef selects, because that is the name Cilium serves it on: `ComputeHosts` at v1.19.5 substitutes the listener hostname for a route that names none, so such a route takes the name over exactly as one that spells it out does. The tenant-namespace hostname policy requires a route there to name its hostnames, so this shape reaches a Gateway from the system namespaces where the platform routes live, or from an administrator, not from a tenant chart. A claim broader than the listener's counts the same way from the other side: a TLSRoute claiming `*.<apex>` is served on `api.<apex>` rather than on the wildcard, because `ComputeHosts` returns the listener's own hostname for a route hostname covering it, so a claim covering a reserved name is judged as a claim on that name while the route keeps one entry under the key its owner wrote. Both restrictions are tied to the Cilium pin, currently v1.19.5. That version translates the Gateway into a single Envoy listener whose filter chains match on SNI without the port, so a terminate chain and a passthrough chain carrying one name collide regardless of which Gateway port declared them; upstream states the consequence above NeedsCrossProtocolSplit in operator/pkg/model/model.go as of v1.19.6, the release that starts splitting Envoy listeners per port when NeedsPerPortListeners holds. That is a disjunction rather than a blanket change, and the passthrough side of every one of its three clauses is counted from attached routes, so a listener with no TLSRoute behind it contributes to none of them: the split arrives with the route, not with the bump. The same single-listener translation has a consequence that is not a restriction: the declared port is where the listener is published, not a bound on reach, because the backend answers its hostname's SNI on every port the Gateway exposes. Firewalling a tenant address down to 80 and 443 does not put a database out of reach; the hostname is the access surface, and the field's own doc and the chart README now say so. Both restrictions are validation-only, and they lift together on the pin plus a configuration the split actually covers rather than on the bump alone. A route whose hostname is withdrawn this way is told so under this controller's own name: Accepted=False with `NoMatchingListenerHostname`, and a message naming the passthrough hostname that answers it, which for a wildcard entry the route cannot derive on its own. It matters most there. A wildcard entry can withdraw the listener from several published names beneath it at once, each one a TLSRoute claims and can be served on, and the certificates are garbage-collected with them, and a rollback is cheaper than that sounds: cert-manager here runs with `enableCertificateOwnerRef: false`, so deleting a Certificate leaves its Secret behind and a re-created Certificate adopts a still-valid one rather than placing an order. Before this the only signal was Cilium's own RouteParentStatus entry under a different controllerName. A TLSRoute whose hostname no passthrough listener answers is told so as well, on the same condition and for the same reason: before this change that hostname got a terminate listener Cilium refused with ResolvedRefs=False, so withdrawing the listener would otherwise have removed the only object saying the route went nowhere. Two things stay named rather than fixed. A `tlsPassthroughListeners` hostname of `*.<apex>` is refused only indirectly: the overlap rule rejects it while `tlsPassthroughServices` carries its defaults, because the wildcard covers api.<apex>, so emptying that list is enough to let it through, and the terminate listener then goes from every published name a TLSRoute claims under the wildcard. The README states this; refusing the apex and its wildcard on their own terms is a one-line rule worth adding when the field is wired up to chart values. A wildcard hostname on an attached HTTPRoute used to render a listener name the shipped Gateway CRD refuses, `https-*-<hash>`, which failed the whole Gateway write while the README said the shape worked. Listener and Certificate names now spell a leading `*` as `wildcard`, with the hash still covering the whole hostname, and a test reads both the listener-name and the hostname patterns off the shipped CRD bundle and renders wildcards through them; every name that was already admissible is pinned byte for byte, so nothing is renamed on upgrade. Under http01 an HTTPRoute wildcard claim is refused before the race and the overlap are read, with `NoMatchingListenerHostname`, because the HTTP-01 challenge issues no wildcard certificate and a terminate listener could never have carried the name. A route status write that fails no longer leaves the route carrying its previous condition: `updateRouteStatuses` and the whole-apex pass collect every failed write and return them after the loop, so the reconcile fails, the TenantGateway reports `Ready=False` with `ReconcileError` and controller-runtime requeues, while the loop still runs to the end so one route's conflict does not hold the others back. Conditions this controller wrote under http01 are retracted when the tenant moves to dns01, existingSecret or edge, for HTTPRoutes and hostname-less TLSRoutes alike, so a refusal does not outlive the mode that produced it. And a TLSRoute claiming a wildcard passthrough entry itself leaves the names beneath the wildcard alone, since Envoy ranks an exact server name above the wildcard and nothing collides. One thing this does not cover yet: Accepted is computed from the hostname race and the withdrawals above, so a route is still told it is fine when an HTTPRoute holds the name and the TLSRoute claiming it cannot attach to the terminate listener that answers it. That one predates this change. Two neighbours of it no longer fall in the gap. A `parentRef` port that names no listener answering the hostname is compared alongside the `sectionName`, since Gateway API requires every key a route pins itself with to match the listener it selects. An HTTPRoute's `parentRef.port` is judged the same way: the terminate listener sits on 443, so any other pinned port earns no listener and no certificate through that parentRef and reads the port-mismatch cause; port 80, or `sectionName: http`, from a namespace the plain-HTTP listener admits (the publishing tenant and cozy-cert-manager) is served plain with no terminate listener and no refusal, from any other namespace it is refused with `NotAllowedByListeners`, and a `sectionName` and a port that select different listeners select none and read `NoMatchingParent`. A plain-served route stays in the hostname race on the same footing as a terminating one, in every recount as well as the first count. And a `sectionName` naming a `tls-<name>` listener this Gateway does not render now reports `Accepted=False` with `NoMatchingParent`, Gateway API's reason for a parentRef matching no listener: the controller renders every listener in that namespace, so an absent name there is a fact rather than a guess, while a `sectionName` naming the plain-HTTP listener is judged like port 80 below, and any other name outside the prefix is written Accepted like an attached route, with the class controller's own status entry answering for the listener it named. The namespace case is off that list. Native-port listeners admit the publishing tenant alone, and a TLSRoute elsewhere on one of their hostnames is refused with `NotAllowedByListeners`. An HTTPRoute that pins itself by `sectionName` to a rendered `tls-<name>` passthrough listener is refused with the same reason: that listener forwards the stream it matched by SNI and terminates nothing, so it takes TLSRoute alone by protocol and cannot serve the route whatever answers its hostname. Its claim also leaves the hostname race before ownership is ranked, so the TLSRoute that can be served is not told it lost to a route that could never attach. Cilium does not report this refusal at the pin. The port-443 passthrough listeners name HTTPRoute in `allowedRoutes.kinds` to keep the port-443 set uniform (cilium#45559), the Gateway reports that entry back as `ResolvedRefs=False/InvalidRouteKinds` on the listener, and Cilium's route-side kind check reads a listener's kinds against every route on the Gateway rather than against the routes that named it, so it reports the route Accepted and the condition this controller writes is the only refusal the route gets. A hostname whose every claimant is refused this way earns no HTTPS-terminate listener and no certificate, where before it earned both. The refusal is decided per route rather than per hostname, which matters once a claim overlaps more than one reserved name: a wildcard covering both a port-443 service hostname and a native-port one is attachable through the first and refused through the second, so the answer comes from the listener the `parentRef` names, its own hostname and its own attach set. The row that pinned a `sectionName` naming no rendered listener as a known gap now pins the verdict instead. On the port-443 listeners the two namespace sets agree by construction, since `collectHostnameClaims` and the listeners' allowedRoutes selector run off the same list. And the justification for the pin is written out at every site that depends on it rather than once, so the day the pin moves they all have to move together and the one that does not is a false statement no diff can catch. That is not hypothetical here: three claims of exactly that shape were wrong in this branch and were caught by reading the source they described rather than by any diff, so the volume of restated reasoning is itself what produces them. One canonical block with references to it would carry the same knowledge with one place to update, and rewriting the narrative under review would be a documentation change rather than a fix, so it is tracked on #3842 instead. The gateway chainsaw suite now covers the withdrawal, which it did not before: a tenant owning its own Gateway renders the shipped passthrough entries with nothing routed to them, an HTTPRoute on api.<apex> keeps its terminate listener, its certificate and Accepted=True there, and attaching a TLSRoute to the same listener, pointed at a Service in the tenant's own namespace, since a route that forwards nowhere is the case the first phase already measures, withdraws all three and moves the route to NoMatchingListenerHostname. A third phase patches in a native-port entry and reads the per-listener readiness flag, which is the claim no test against a fake client can settle. The first two phases are one measurement: withdrawing on demand is what dates the earlier assertion to settled state rather than to a withdrawal that had not arrived yet. The `Require API owner review for sizeable API changes` check is red on purpose and does not clear with a code change. A new field plus tightened validation on already-shipped fields counts as a sizeable API change, so `.github/workflows/api-review-gate.yaml` holds the PR until an API owner approves it. The gate counts each owner's latest review on the current head commit, so the approval has to land on the head as pushed and any later push invalidates it; the job re-runs on review submission, so no push is needed to flip it. Reading that red as "not ready" is the mistake this paragraph exists to prevent. Part of #2815. ### Release note ```release-note feat(gateway): TenantGateway supports layer-4 TLS-passthrough listeners on native database ports for SNI-based external exposure. On HTTP-01 gateways a hostname a TLS-passthrough listener answers, including any name beneath a wildcard entry, no longer earns an HTTPS-terminate listener or a Gateway-issued certificate once a TLSRoute claiming that hostname can be served on the listener and forwards to a backend that resolves; a hostname claimed only by a TLSRoute stops earning one too. A TLSRoute with no spec.hostnames counts as claiming the hostname of each passthrough listener its parentRef selects, and one whose hostname covers a listener's counts as claiming that listener's name, both being the name Cilium serves it on. A declared passthrough listener with no route on it changes nothing, because it emits no filter chain and the terminate listener is what answers the name, and neither does a route whose backendRefs resolve to nothing, a Service that does not exist, or one in another namespace with no ReferenceGrant, since such a route emits no filter chain either. The controller watches Services and ReferenceGrants, so the withdrawal it defers lands once the backend appears. So the tenant whose Gateway the platform TLSRoutes name, tenant-root unless the cluster's expose-ingress says otherwise, sheds the terminate listeners and certificates for api, vm-exportproxy and cdi-uploadproxy on upgrade (those TLSRoutes ship with the cozystack-api, kubevirt and kubevirt-cdi packages, each rendered only where the Gateway integration is enabled and the service is among the platform's exposed services), and a tenant that renders the same three entries with no route on them keeps what it has. The shed certificates leave their TLS Secrets behind: cert-manager here ships enableCertificateOwnerRef: false, so each Secret stays in the namespace with no owner, nothing renewing it and nothing deleting it. That cuts both ways, as stale Secrets to remove by hand and as a cheap way back, since a re-created Certificate adopts a still-valid Secret instead of placing an order. A route the controller renders no listener it can attach to now carries Accepted=False from this controller instead of Accepted=True. Five shapes report NoMatchingListenerHostname and the message tells them apart: the HTTPRoute whose hostname a passthrough listener answers, the TLSRoute with no passthrough listener behind it, the route whose parentRef names a listener that answers a different hostname, the route whose parentRef names a port no listener answering its hostname is published on, and the HTTPRoute claiming a wildcard hostname under http01, which the HTTP-01 challenge cannot certify. A route pinned by sectionName to a tls-<name> listener the Gateway does not render carries NoMatchingParent instead, which is what Gateway API calls a parentRef matching no listener, and so does an HTTPRoute whose sectionName and parentRef port select different listeners. A TLSRoute outside the publishing tenant's namespace claiming a native-port hostname carries NotAllowedByListeners instead, since those listeners admit the tenant alone. So does an HTTPRoute pinned by sectionName to a rendered tls-<name> passthrough listener, which cannot serve its kind, and an HTTPRoute selecting the plain-HTTP listener from a namespace it does not admit; a refused route also stops counting in the hostname race, so a hostname whose every claimant is refused earns no terminate listener and no certificate. Listener and Certificate names derived from a wildcard hostname are admissible now, names that were already admissible do not change, a failed route-status write fails the reconcile instead of being logged away, and conditions written under http01 are retracted when certMode leaves it. The dns01 and existingSecret cert modes shed nothing, because neither renders per-hostname listeners at all, and tlsPassthroughListeners is accepted under http01 alone: those two serve the whole apex from one wildcard terminate listener, and edge terminates TLS upstream and renders no TLS listener a passthrough entry could sit beside. The withdrawal does not take a working path away: on the pinned Cilium both port-443 filter chains already carry these hostnames and nothing on the objects says which one answers a connection, so a race that was previously undetermined and unobservable now resolves the same way every time. Treat this as a change of serving path rather than the removal of something idle: today each of those three hostnames has a terminate listener and a passthrough listener on port 443, both filter chains carry the SNI, and nothing on the objects says which one answers a connection. Gateway API calls for both listeners to report Conflicted; the pinned v1.19.5 has no Conflicted condition at all, so there is nothing to read, and you cannot tell from the cluster whether api.<apex> is terminated by the Gateway or passed through. Afterwards it is passed through, always. Read the Gateway's listener conditions before upgrading to see what those listeners were serving. Before upgrading, confirm the management apiserver serving certificate carries api.<apex> in its SANs, because once the ACME certificate is withdrawn that is the only certificate left on the name; a cluster that came from the ingress-nginx path already meets that, a cluster served by the Gateway's ACME certificate since it moved does not necessarily and loses the name until the SAN is added. If something depends on the terminate path for one of those names, the package README lists three ways forward and what each costs. Moving the consumer to another hostname under the apex is the only one that gives up nothing. ``` ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against this diff: `api/gateway/v1alpha1/{tenantgateway_types.go,zz_generated.deepcopy.go}`, `internal/controller/tenantgateway/{reconciler,renderers,conflict,mapper,status,backends,reconciler_test,crdcel_test,mapper_test,backends_test}.go`, `internal/controller/tenantgateway/rbac_test.go`, `cmd/cozystack-controller/main.go`, `hack/e2e-chainsaw/gateway/chainsaw-test.yaml`, `packages/system/cozystack-controller/definitions/gateway.cozystack.io_tenantgateways.yaml`, `packages/system/cozystack-controller/templates/rbac.yaml`, `packages/core/platform/values.yaml`, and `packages/extra/gateway/README.md`. No trigger fires, and the two repositories worth ruling out explicitly are below rather than waved off. `cozystack/website`: no package under `packages/apps/` or `packages/extra/` is added, renamed or removed. `packages/extra/gateway/README.md` is edited in place. The docs generator regenerates a reference page from a package README only for the apps hardcoded in the website `Makefile`, and `gateway` is in none of `APPS`, `K8S`, `VMS`, `NETWORKING` or `SERVICES`, so there is no generated page for this package to drift out of step with. That gap is the same shape the trigger map already records for `packages/apps/opensearch` and `packages/apps/bucket`; it predates this change and is neither created nor widened here. None of the other website triggers apply: `packages/core/platform/values.yaml` changes only in a comment, which now says that api, vm-exportproxy and cdi-uploadproxy render a TLSRoute rather than an HTTPRoute, with no key added, moved or given a new default, no platform variant or component moves, no release asset is renamed, `ApplicationDefinition` semantics are unchanged, there is no Talos bump and no developer-tooling change. `cozystack/terraform-provider-cozystack`: this adds an API field, so the field-level triggers are the ones to check, and they are keyed to app charts rather than to this CRD. No app is added under `packages/apps/`; no `values.schema.json` gains, loses or renames a field; no version enum, `values.yaml` default, `kind`, `plural`, `release.prefix` or output Secret/Service name changes. `TenantGateway` is not among the CRDs the provider types by hand. Those are `Package` under `cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`, and the provider carries no reference to `gateway.cozystack.io` at all. It therefore falls under the map's last bullet: the spec travels as opaque JSON, so the provider keeps working and simply offers no typed access to `tlsPassthroughListeners`, which is the position it already holds for every other field it does not model, and not a break. The provider's new-top-level-field test guard is scoped to `api/apps/v1alpha1`, which this diff does not touch. `cozystack/ansible-cozystack`: nothing in reach. `packages/core/installer/values.yaml` and the `cozy-system`/PSA contract are untouched, no platform variant is added or renamed, none of the `packages/core/platform/values.yaml` keys the role sets are involved (the file changes in a comment alone), no object the role waits on is renamed, and `hack/e2e-prepare-cluster.bats` node prerequisites are unchanged. The schema constraints this diff adds to already-shipped fields all reach objects that already exist, and every one of them is a kind the apiserver ratchets. `spec.apex` gains a 253-character limit (no pattern: a mixed-case apex is admitted and refused by the controller only when passthrough listeners are declared), and `spec.tlsPassthroughServices` gains a DNS-1123 item pattern and a length bound. A stored value that violates one still passes any write that leaves it unchanged, so the rule bites only once the value itself changes. A repeated `tlsPassthroughServices` entry is reported by the controller as `Ready=False` rather than refused at admission, and the reason is not the one this section used to give. Ratcheting does reach a list-type error: `customresource.ValidateUpdate` validates an incoming object's list-type semantics only when the stored object passes them, so a stored duplicate is ratcheted through rather than blocking every write. What makes the marker the wrong tool is the grain of that ratchet, which is the whole object rather than the field, so one stored duplicate would switch list-type checking off for every list on the `TenantGateway` at once and say nothing about it. Documented in `packages/extra/gateway/README.md`, and pinned by `TestTightenedConstraintsOnExistingObjects` together with `TestListTypeSetWouldNotRefuseAStoredDuplicate`, which compiles a copy of the shipped schema carrying the marker to show what the apiserver actually does with one. Shipped defaults conform and the chart already fails on an empty host, so this is a no-op in practice. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added layer-4 TLS passthrough listeners (`tlsPassthroughListeners`) with native port + SNI hostname, rendered as passthrough listeners and limited to TLSRoute attachment. * **Bug Fixes** * Added stricter admission-time and controller validation (reserved ports 80/443, distinct listener ports, tenant-apex hostname rules, and collision prevention with passthrough services). * Improved listener rendering safety: prevents exceeding the Gateway listener cap and ensures AllowedRoutes are isolated per listener. * **Documentation** * Updated gateway docs with listener-cap considerations and layer-4 passthrough usage details. * **Tests** * Added CRD/CEL and reconciliation coverage for valid/invalid configurations and rendering behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
cca3b08733 |
test(backups): pin the Kafka restore path and fail fast on a missing artifact
The restore half of the driver - the half that mutates a live Kafka cluster - had no test reaching its happy path, so three reversions of it left the suite green. - Add restore reconcile tests mirroring the Rabbitmq driver this was copied from: ToCopy (pins the targetApplicationRef.Name merge - a to-copy restore must resolve the TARGET, not write the source's topics back into the source), NoRecordedArtifactURI, TargetClusterNotReady and TargetNotFound. Verified non-vacuous: neutralising the name merge reddens ToCopy, and reverting the fail-fast ordering below reddens NoRecordedArtifactURI. - Fail fast on a Backup with no status.artifact.uri: move the check ahead of the readiness gate so it terminates immediately instead of waiting out the whole deadline and then reporting a misleading "timed out waiting for Kafka cluster to become Ready" (reconcileRabbitmqRestore checks it first; same order here). - Add bats cases for the fail-closed guards that no test reddened: MODE validation, the s3:// URI prefix, cleanup treating 404 as success, an unparsable backup/live describe, a kafka-configs describe error, and an unrecognised record kind (plus a CFG_FAIL knob in the configs stub). The describe-parse guards abort via `set -euo pipefail` at the `X=$(describe_field ...)` assignment, so these pin the fail-closed behaviour; the explicit guards remain as belt-and-braces. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
38d6c4cf58 |
fix(backups): bound Kafka metadata run at the Job layer, not a controller wall clock
A controller-side running-Job deadline was enforced in reconcileKafka / reconcileKafkaRestore: at kafkaDefaultBackupDeadline from StartedAt the BackupJob/RestoreJob was marked Failed. But the batch Job carried no activeDeadlineSeconds and markBackupJobFailed only writes status, so the Job kept running, finished, and its final `s3 PUT` uploaded the object — while the terminal phase meant no Backup was ever created and cleanup (keyed on Backup deletion) never ran. Every timed-out run thus orphaned a uniquely-keyed object, and a cluster whose export legitimately exceeds the deadline could never produce a usable backup. The shipped docs also claimed the run "keeps the BackupJob Running ... rather than failing on a wall clock", which the deadline made false. Move the bound onto the Job as activeDeadlineSeconds (setKafkaJobDeadline): Kubernetes fails an unschedulable or wedged Job as DeadlineExceeded and kills its pod before the upload, so a timed-out run lands in the JobFailed branch cleanly and leaves no object behind. Make it operator-tunable with the `backupTimeout` BackupClass parameter (a Go duration), which round-trips through the Backup so a restore is bounded by the same value; the readiness-precondition wait honours it too. Correct the docs. Also requeue on a transient broker-image List error instead of failing the whole run: resolveKafkaBrokerImage now wraps a sentinel (errNoKafkaBroker) so only a Ready cluster with no listable broker is terminal. Tests: pin the deadline on the built Job, the backupTimeout override, a JobFailed run marking the owner Failed, and the transient-List requeue. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
4925a84c26 |
fix(backups): pin the restore-side guards and bound a running Kafka Job
- delete_topic waited with `grep -qx` (BRE), so deleting `audit.events` matched the live `audit-events` and spun to its timeout — the e2e failed at step 25 on every run that reached it. Use `grep -qxF`. - The two -k/--connect-to guards were written as `! grep`, which cannot fail under hack/cozytest.sh (it wraps each @test in `set -e` + a trailing `return 0`, and bash exempts a `!`-negated command from errexit). Rewrite them as positive `if grep ...; then return 1; fi`; verified the -k reinstatement now reddens the suite under cozytest. - Cover the restore-side literal-topic guard that was unpinned at every level: a bats case runs the existing-topic branch with a colliding sibling and asserts the `--alter` argv carries `\Q…\E`, and run-all.sh now leaves `audit.events` live so the e2e exercises that branch instead of only `--create`. Add bats cases for the partition-shrink refusal, the `-f` upload guard, and `--exclude-internal`. - Bound a running Job: reconcileKafka / reconcileKafkaRestore now fail the BackupJob/RestoreJob once kafkaDefaultBackupDeadline elapses in the Job-running branch (buildJobStrategyBatchJob sets no activeDeadlineSeconds), so an unschedulable pod no longer pins it in Running forever. Pinned by TestReconcileKafka_FailsWhenJobNeverCompletes. - Fail closed on an unknown record kind and an unknown MODE, and validate the live partition/RF parsed on restore (an empty value silently skipped reconciliation). Correct the stale "image pinned in the strategy" comment in the example helpers and the chainsaw test (the controller now resolves the target broker's image as .ClientImage). Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
0d918e1f91 |
fix(backups): resolve Kafka backup client image from the target broker
The Kafka metadata strategy pinned kafkaBackupClientImage to an upstream Strimzi tag with no producer: nothing stamped it and Renovate does not manage helm-values, so it drifted from the broker version the operator provisions. A dedicated small client image (the sibling curl-only drivers' follow-up) is not viable here because the driver runs the Kafka Admin-API CLI, which only the broker image ships. Resolve the client image from a live broker pod of the target Strimzi Kafka at reconcile time instead, so the CLI always matches the broker (any version the operator picks, not only the chart default), and record it on the Backup so cleanup can reuse it once the source cluster is gone. The chart no longer pins the image; the strategy template renders it as the .ClientImage placeholder the controller fills. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
14c6af122e |
fix(backups): restore Kafka S3 port workaround and hold the script with a test
- Regression fix: an earlier change dropped the curl --connect-to port workaround along with -k, but curl 7.76.1 (the image's) 403s on ANY ported endpoint, which is the shape backupStorage.endpoint ships (:8333) and what provisionBucket:false and s3.ingress.enabled:false produce. Restore --connect-to (sign a portless URL, redirect to the real port), keeping -k off so TLS is still verified. IPv6-safe host/port split, path prefix preserved. The port-less ingress passed; the ported case is the one that regressed. - Correct the S3 envelope claim in values.yaml and backup-classes.md: the 403 is decided by the port, not a body-hash the backend recomputes, and --connect-to handles it on SeaweedFS and MinIO alike. - Hold the script fixes at the layer the contract lives at. Externalise the driver to files/kafka-backup.sh (embedded verbatim via .Files.Get, render byte-identical) and add hack/kafka-backup-script.bats, which EXECUTES it against stubbed kafka CLIs + curl and asserts behaviour: \Q literal match on the colliding pair, --connect-to with no -k on a ported endpoint, fail-closed --list, comma-value bracket grouping, RF-divergence abort, and the cleanup delete's HTTP handling. Not a template grep. - Pin the Backup-delete dispatch and the readiness deadline in Go: TestCleanupOnDelete_Kafka_RoutesToKafkaCleanup and TestReconcileKafka_FailsAfterReadinessDeadline. - Docs: config restore is additive (no --delete-config); export time scales with topic count and the Job has no wall clock (as its siblings). Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
ed3cb34cbf |
fix(backups): adopt-guard Kafka Backup artifact and correct URI-template doc
- createKafkaBackupArtifact now rejects adopting a same-named Backup that describes a different application, so a retained Backup reused for a different app of the same name is not reported as this run's success. Mirrors the RabbitMQ driver's guard; pinned by a Go test. - ArtifactURITemplate godoc said empty leaves status.artifact unset, but the driver hard-fails an empty template. Reword to state it is required, and regenerate the CRD description to match. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
0270650237 |
fix(backups): harden Kafka metadata S3 handling and own the artifact
- Drop `-k`: mirror the RabbitMQ S3 layer (ARTIFACT_URI as the single source of truth, forcePathStyle honoured, whole base URL kept) so curl --aws-sigv4 verifies TLS against the ACME endpoint the helper returns. The derived endpoint is default-port, so the curl 7.76.1 --connect-to port workaround is gone too; this also fixes the IPv6 and path-prefix mangling in the old host-splitting. - Own the artifact: a cleanup mode in the script plus cleanupKafkaBackup wired into BackupReconciler.cleanupOnDelete delete a deleted Backup's object (best-effort, waited on) instead of orphaning it, matching the RabbitMQ precedent. Kafka has the identical own-your-object property. - Restore probes existence with --list (fail closed), so an unreachable broker aborts instead of being misread as "topic absent" and created. - Hold the fixes with tests at the right level: a colliding topic pair (audit.events / audit-events, distinct partition counts) in the e2e so a revert of the \Q...\E literal-match breaks the round-trip, and Go tests pinning the scope stamp and the cleanup Job. - Correct the docs/comments: endpoint-shape row, the "cached on nodes" claim, and the narrower curl 7.76.1 S3 envelope (empty-body payload hash) that makes a small modern-curl client image the right follow-up. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
537aadf389 |
fix(backups): fix Kafka backup S3 signing and literal topic names
Reproduced end-to-end against a real Kafka 3.9.1 broker: - [CRITICAL] The Strimzi image ships curl 7.76.1, whose --aws-sigv4 omits a non-default port from the SigV4 canonical Host while sending it in the Host header, so every S3 PUT/GET 403s and the driver never uploads or downloads. Sign/send the bare host and redirect the connection to the real port with --connect-to (the same fix the topic-data example already carries), and accept the in-cluster self-signed cert with -k. - [CRITICAL] kafka-topics --topic is a Java regex, not a literal: a backup entry for "audit.events" also matched a live "audit-events", so restore repartitioned an unrelated topic irreversibly and backup recorded the wrong partition/replication values. Wrap the name in \Q...\E on every filter call (--describe on both paths, --alter on restore) so it matches exactly one topic; --create and kafka-configs --entity-name are already literal. Applied in the example helpers too. - [MAJOR] cozy-default's Kafka route looked like a data backup at runtime. Stamp scope=topic-metadata on the produced Backup's DriverMetadata and say "topic definitions and configs only; no message data" in the Ready condition, so kubectl describe backup is honest. - [MAJOR] Add reconcile tests for the two Go behaviours a mutation could silently drop: the Strimzi-Ready precondition on backup (BackupJob stays out of Succeeded and creates no Job) and the applicationRef Kind gate on the restore target. - [MINOR] Fail closed on the config-capture grep (exit != 1 aborts) and clarify the image pin has no automated producer. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
405eb25095 |
fix(backups): harden Kafka metadata backup
- Fail closed on topic listing: set -o pipefail, capture kafka-topics --list --exclude-internal exit separately, and abort on failure so an unreachable broker can no longer produce an empty backup reported as Succeeded. An empty list after a successful call stays legitimate. - Check each kafka-topics --describe exit and validate the parsed partition count / replication factor before writing a record, so a timed-out describe cannot write a corrupt orders\t\t\t line. - Store topic configs one per line with key and value in separate tab-delimited fields, sourced from kafka-configs --describe (one config per line), so a value containing a comma (cleanup.policy=compact,delete) round-trips; on restore each config is applied with the value bracketed only when it contains a comma. - Detect topic divergence on restore instead of a silent --if-not-exists no-op: grow partitions with --alter when the backup asks for more, and fail loudly (naming the topic and both values) when it asks for fewer partitions or a different replication factor. A restore that cannot reach the recorded state no longer reports Succeeded. - Add KafkaStrategyKind to the RestoreJob cleanup-dispatch no-op group so deleting a Kafka RestoreJob does not run a pointless Velero cleanup; add a regression test alongside the siblings, plus a reconcile Kind-gate test and a rendering test that actually renders the strategy template. - Wire the kafka-metadata Chainsaw suite into both selectors (select-install suite_to_source, select-e2e src_to_suites) so editing the kafka app or the suite selects it and the bats unit tests pass. - Pin the client image to the operator's default broker version (kafka 3.9.1) with a digest, and align the example default; fix the values comment. Point the RabbitMQ comment references (which shipped in the CRD description) at the Job strategy actually mirrored, and trim the in-pod comments that rendered into the manifest. - Document the Strimzi Topic Operator caveat: for a topic owned by a KafkaTopic CR the operator reconciles its config from the CR, so an in-place config restore is durable only for out-of-band topics. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
a410a3f48b |
feat(backups): Kafka topic-metadata backup via a dedicated strategy driver
Add a strategy.backups.cozystack.io/Kafka driver that backs up the topic metadata of a Cozystack-managed Kafka application — partition counts, replication factors and non-default topic configs — through the Kafka Admin API, and restores it with kafka-topics --create + kafka-configs --alter. Modelled on the Job/Rabbitmq strategies: the driver renders a one-shot batch Job, gates on the Strimzi Kafka cluster reporting Ready, and keys each backup by .BackupName so runs do not overwrite each other. Scope is deliberately narrow: message payloads, consumer-group offsets, ACLs, quotas and KafkaUsers are out of scope, and there is no PITR. Restored topics are created directly (unmanaged on the target) — the accepted tradeoff of an Admin-API-only driver. Both restore flows are implemented: in-place recreates dropped topics into the source, to-copy applies the source's topics onto a freshly-bootstrapped empty cluster. Wire it into cozy-default (strategy-kafka-default.yaml gated on the bucket name, a BackupClass binding, read RBAC on kafka.strimzi.io/kafkas for the readiness gate, and the Strimzi Kafka client image), document it in backup-classes.md, and add an honest round-trip example plus Chainsaw e2e (examples/backups/kafka-metadata, kafka-3-metadata-roundtrip) that proves a per-run retention.ms sentinel survives both restores. Unit tests cover the driver's pure helpers (application-ref validation, readiness verdict, parameter round-trip, per-run artifact-URI keying). Not yet verified on a live cluster. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
9121456ba8 |
chore(cozystack-controller): name the Service and ReferenceGrant reads
Deciding whether a TLSRoute forwards anywhere reads the backend Service and the ReferenceGrant admitting a cross-namespace reference to it, and both are watched rather than fetched on demand. The read-everything rule at the foot of the ClusterRole covers them today, so nothing is broken. Naming them keeps the feature working the day that rule is narrowed, where an informer starting without a grant fails with Forbidden and takes the manager with it. The kubebuilder markers in the package generate nothing: controller-gen runs over ./api/... alone and writes no rbac artifact, so the shipped role is hand-written and only a test connects the two. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2bf9ae4a27 |
feat(gateway)!: serve passthrough hostnames and stop terminating them
Each tlsPassthroughListeners entry renders a mode=Passthrough listener on its own port, answering the entry's hostname by SNI and admitting TLSRoutes from the publishing tenant's namespace alone. It declares no allowedRoutes.kinds: at the pinned Cilium v1.19.5 a listener that spells its kinds out has them applied to every route on the Gateway rather than to itself, so naming TLSRoute here would reject every HTTPRoute on the Gateway. A hostname a passthrough listener answers no longer earns an HTTPS-terminate listener or a Gateway-issued certificate. That is the break, and it is keyed on service rather than on declaration: the same Cilium builds a passthrough filter chain per route out of the backends that survived resolution, so a listener no TLSRoute attaches to, and a route whose backendRefs resolve to nothing, both match no ClientHello and leave the terminate listener holding the name. Eligibility asks what v1.19.5 asks and no more, the reference allowed and the Service present, and fails closed on a read it cannot complete. Services and ReferenceGrants are watched, so a withdrawal deferred on a missing backend lands on the pass that sees it arrive, and the Service watch takes only the creates and deletes, existence being all that is read. Which hostname a route claims is answered the same way. spec.hostnames is optional on a TLSRoute, and ComputeHosts substitutes the hostname of each TLS listener the route's parentRefs select for a route that declares none, so such a route carries the SNI and takes the name over exactly as one that spells it out. A declared hostname broader than the listener's is that substitution from the other side: the route is served on the listener's name and not on the wildcard it wrote, so a claim covering a reserved name counts as a claim on it. The substitution reaches only the listeners that would admit the route. A native-port listener takes the publishing tenant's namespace alone, so Cilium never puts a route from elsewhere on it and never serves that route on its hostname; borrowing the name anyway refuses the route for a namespace, over a hostname its own object does not carry, in the same pass that sheds a terminate listener because that same route is serving a name it borrowed legitimately. A route that names the listener in its sectionName keeps the borrow, since there the refusal is the answer to what it asked for. A route the Gateway renders no listener for hears it from this controller instead of from Cilium alone: the hostname a passthrough listener took, the hostname nothing answers, the sectionName or port naming a listener that answers something else, the sectionName naming no rendered passthrough listener, and the native-port listener that admits the tenant alone, each with its own Gateway API reason and a message naming the hostnames that lost. An HTTPRoute pinning a rendered passthrough listener by sectionName is refused there too, since such a listener forwards a stream it never terminates and takes TLSRoute alone by protocol. Nothing else tells that route: the port-443 listeners name HTTPRoute in their kinds to keep every port-443 set uniform, which the Gateway reports back as InvalidRouteKinds rather than as a grant, and the pin reads a listener's kinds against every route on the Gateway, so it reports the route accepted. The field is confined to certMode http01, stated as the modes that can carry it so one added later is refused until someone rules on it. The wildcard modes cover the whole apex with one terminate listener the pinned Cilium cannot keep apart from a passthrough listener beneath it, and edge terminates TLS at the class provider and renders no TLS listener at all. An HTTPRoute claiming a wildcard is refused under http01 before the race and the overlap are read. The HTTP-01 challenge issues no wildcard certificate, so the terminate listener rendered for the claim could never carry the name, and the listener name built from the hostname's first label carried the asterisk, which the Gateway CRD admits in a hostname and refuses in a name, so the apiserver refused the whole Gateway write and every other listener on it froze where it stood. The renderer now writes that label as "wildcard", so every hostname the CRD admits yields a name it admits, and no name that was already admissible moves. That refusal is a statement about the terminate listener, so it is judged where that listener is decided rather than ahead of everything else. The port-80 listener carries no hostname and serves a wildcard claim as readily as any other, so a route that selects it, by pinning the port or by naming the listener, never needed the listener HTTP-01 cannot supply and is served rather than refused. Judged first, the refusal reached that route too and reported it unserved while Cilium was serving it in plaintext. A parentRef port on an HTTPRoute is read the way servableOn reads it for a TLSRoute. The terminate listener sits on 443, so a route pinning another port attaches to no listener the render would give it, and it used to earn a listener and an ACME certificate for a route that attached to nothing. Port 80 is the plain-HTTP listener, which admits the publishing tenant alone: a route from there pinning it is served plain and earns neither a listener nor a refusal. A route status write that fails is told apart by kind. A conflict or a route that vanished between the list and the write is a race between the two status writers this Gateway has: the reconcile requeues and retries the write without pinning Ready=False on the tenant, and the status pass runs after the http-to-https redirect, so such a race cannot withhold the one object that guard exists to provide. A write the apiserver will refuse on every pass still fails the reconcile, after every other route has been written, because the condition is the only object saying why a hostname is not served, and a pass that reported success over a lost write left the route reading Accepted=True until the resync. The retryable half is logged on the way out. Declining to pin Ready=False leaves the requeue as the only thing acting on it, and the requeue says nothing: the tenant goes on reading Ready, no event is recorded, and returning no error opts out of the backoff that would widen the interval, so a write failing on every pass retries at a fixed period indefinitely with nothing anywhere to show for it. A TLSRoute claiming a hostname broader than the listener's is pulled into the eligibility question for each name it covers, because the pinned Cilium serves it on the listener's own name. That is right only where the listener answering a name is named exactly that name. Under a wildcard entry the substitution hands such a route the wildcard, so its chain carries "*.db.<apex>" while the terminate chain carries the exact name beneath, and Envoy matches an exact server name ahead of a wildcard, so nothing collides; pulling the route in there withdrew a served endpoint and handed its name to the database behind the passthrough listener. The pull-in is now keyed on an exact listener match. Every condition this controller writes on an HTTPRoute, and the one it writes on a TLSRoute declaring no hostnames, is written under http01, and the whole-apex modes collect no claims, so a switch of certMode left those verdicts standing over listeners the new mode does not render. The whole-apex pass now drops this controller's entry from HTTPRoutes and from hostname-less TLSRoutes; the edge refusal stays confined to routes that declare hostnames, since a refusal written on a lent hostname would have no path back. Within http01 the same verdicts outlive their listeners for a second reason, and no mode change is needed to reach it. The status pass walks the routes that attached rather than the ones that claimed a hostname, and retracts its entry from the difference, because a claim set is not a route's own property once a hostname can be borrowed: a TLSRoute declaring none takes the hostname of every passthrough listener its parentRef selects, so removing that listener empties the claim set while the route stands unchanged, and an HTTPRoute reaches the same state by dropping its hostnames. Judged on claims alone such a route keeps whatever the last pass that did judge it left behind, describing a listener the Gateway no longer has. An HTTPRoute pinned by sectionName to a passthrough listener this Gateway does not render is refused with NoMatchingParent, the reading a TLSRoute in the same shape already got, instead of earning a terminate listener it could never attach to. A route served on the port-80 listener, by parentRef port or by naming it, earns no terminate listener and is refused nothing, but it publishes the name from its namespace all the same, so it stays in the hostname race. Every recount now ranks it with the field it recounts, because a recount that left it out crowned the namespace that had lost to it and dropped a loss the first count recorded: a third route refused in a third namespace, or a passthrough entry declared for the name, flipped the loser to Accepted=True. Selecting the port-80 listener by name is judged as selecting it by port: from a namespace the listener admits the route is served plain, from anywhere else the listener turns it away and the route reads NotAllowedByListeners, and a sectionName and a parentRef port that select different listeners select none and read NoMatchingParent, which is what Gateway API requires when both are given. Before this a route naming the listener from a refused namespace earned a terminate listener and a certificate it could never attach to. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
1f04e31ed3 |
feat(backups): add Redis backup/restore strategy driver (#3992)
## What this PR does Adds backup/restore support for the `Redis` app (spotahome `RedisFailover`) via a dedicated `strategy.backups.cozystack.io/Redis` strategy controller. The spotahome operator ships no backup CRD, so the driver follows the generic-script-Job pattern (a specialisation of the Job/Altinity drivers): it runs a one-shot Job that dumps an RDB snapshot from the current master to the system bucket and replays it on restore. - **Backup**: discover the current master via the operator's sentinel Service (`rfs-<name>`, master `mymaster`) → `redis-cli --rdb` → `curl --aws-sigv4` PUT to `s3://<bucket>/<namespace>/<app>/dump.rdb`. - **Restore**: GET the object → convert RDB → RESP (`hdt3213/rdb`) → `FLUSHALL` + `redis-cli --pipe` into the target's master. Both **in-place** and **to-copy** are implemented; a to-copy restore reads the source's prefix via `.Backup.ApplicationRef.Name` while writing into a differently-named target. - **PITR**: not supported — RDB is a discrete snapshot with no continuous log to replay to an arbitrary time (the etcd-like case). The driver is a specialisation of the Job/Altinity script-Job flow with a `Redis` applicationRef gate and an app-readiness precondition (surfaces a precise `Ready=False` instead of a silent hang; blocks only on an explicit `Ready=False` so a missing condition never wedges backups). Strategy parameters round-trip through the Backup's `DriverMetadata`; there is no `underlyingResources` snapshot (the Job pattern re-renders on restore). ### System bucket Coordinates and credentials reach the Job as env `secretKeyRef` from the projected `cozy-backups-creds` (the ClickHouse/Altinity delivery model), so **the app chart is untouched** — no `backup.useSystemBucket`, no new RBAC (`batch/jobs` and `apps.cozystack.io/*` are already granted). The bare-host `endpoint` gets `https://` prepended when unscheme'd; against a self-signed endpoint the strategy verifies with the CA from `backupStorage.endpointCASecretName` when set, otherwise against the image's system CA store; skip-verify is a separate explicit `backupStorage.insecureSkipTLSVerify` opt-in, never a fallback. The `cozy-default-redis` strategy is gated on `$bucketName` (CRD-ordering) and bound in the `cozy-default` BackupClass. ### Changes - `api/backups/strategy/v1alpha1/redis_types.go` + generated CRD and deepcopy. - `internal/backupcontroller/redisstrategy_controller.go` (+ unit tests) wired into both dispatch switches, `supportedBackupStrategyKinds`, and the no-op restore-cleanup case. - `cozy-default-redis` strategy template + `cozy-default` BackupClass route + helm rendering-gate coverage. - `images/redis-backup` (redis-cli + curl + RDB→RESP converter) + Makefile target + `backupStrategyController.redisBackupClientImage`. - `examples/backups/redis/` — an honest marker round-trip for both restore flows. ### Validation `go build`/`vet`, `go test ./internal/backupcontroller/... ./api/backups/...`, `make generate` (clean), and `helm unittest` (10/10) all pass. Not yet cluster-verified. ## Downstream None. ```release-note Add backup and restore support for the Redis application (spotahome RedisFailover) via a new Redis backup strategy, wired into the cozy-default BackupClass. Supports in-place and to-copy restore; PITR is not supported. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Redis backup and restore support using RDB snapshots in S3-compatible storage. * Supports restoring data in place or to a separate Redis instance. * Added configuration for Redis backup images, custom HTTPS certificate authorities, and TLS verification. * Added a complete Redis backup and restore demonstration with automated setup, verification, and cleanup. * **Documentation** * Documented Redis storage, credentials, endpoints, TLS, and restore behavior. * Added administrator and tenant scenario guides. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
afc003c464 |
feat(postgres): database read-replica autoscaling via KEDA (#3954)
## What this PR does Adds automatic horizontal autoscaling of a PostgreSQL database's **read replicas** in response to load, implementing the merged design proposal [Database Horizontal Autoscaler](https://github.com/cozystack/community/tree/main/design-proposals/database-horizontal-autoscaling) (community#44). The mechanism is entirely stock — no bespoke operator, no new CRD: - **KEDA as an optional platform component** (`packages/system/keda`, wired into `core/platform` as a source + system-bundle entry). It claims the `external.metrics.k8s.io` APIService that nothing served before. - **`cozy-lib.keda.scaledObject` helper** (`packages/library/cozy-lib/templates/_keda.tpl`) renders a KEDA `ScaledObject`, fail-closed on missing/invalid arguments. - **`packages/apps/postgres` autoscaling** — an `autoscaling` values block (`enabled`, `minReplicas`, `maxReplicas`, `metric`, `target`, `maxReplicationLagSeconds`, `dryRun`, `transition`) validated by `values.schema.json`; under active autoscaling the chart renders a `ScaledObject` and seeds `spec.instances` to a constant floor (`max(replicas, effectiveMin)`), while the postgres release is forced to client-side (two-way) Helm apply so the seed is a merge no-op on upgrade and the KEDA-managed HPA — the runtime writer via the CNPG `Cluster` scale subresource — is not reverted. - **Single-value metric (§1)** — one PromQL query returns `Σ(read load over standbys) + target`, so a stock HPA computes `desired = ceil((Σ+target)/target) = 1 + ceil(Σ/target) = primary + desiredRead`, with no per-pod emission. - **The four static brakes (§5)** — quorum floor `max(minReplicas, maxSyncReplicas+1, 2)` (quorum wins over `maxReplicas`), scale-down pacing (`Pods/1/600s`), a replication-lag clamp folded into the query (freeze both directions while lag exceeds the threshold and the primary is writing, with `max_over_time` hysteresis), and dry-run (render paused). - **Two-phase enable/disable migration** via a `transition` sub-flag: phase 1 stages `spec.instances` to the live count and brings KEDA up with its HPA paused in both directions; flipping to phase 2 hands the runtime count to KEDA, and because the seed re-renders the same count the flip is a merge no-op with no dip. - **Observability** — a database-autoscaling Grafana dashboard and PrometheusRule alerts. **Live validation (dev cluster, CNPG 1.30.0).** The full path works end-to-end: the CNPG `Cluster` scale subresource serves `status.selector`, KEDA renders the HPA, the metric reaches it (`ScalingActive=ValidMetricFound`, `TARGETS 151/150`, `desiredReplicas=2 = 1 primary + 1 replica` — the §1 arithmetic), and the lag-clamp query returns the base value on live VictoriaMetrics. Two packaging fixes were needed and are included: KEDA must run on the cozystack `cozy.local` cluster domain (the upstream `cluster.local` default makes the metrics gRPC target NXDOMAIN and every HPA hangs on `FailedGetExternalMetric`), and the operator's client-go throttle is raised so a single-replica operator does not lose its leader lease under scale-subresource polling. Implements #3953. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two repositories are affected; both are follow-ups to open once this lands (and after its dependency #3951 merges and the values surface is final), left unticked here for a maintainer to confirm rather than claimed with a speculative link: - **cozystack/terraform-provider-cozystack** — this adds the nested `autoscaling` block to `packages/apps/postgres/values.schema.json`. The provider hand-models each app's fields (schema + model + expand/flatten), so it needs the new block to expose it in Terraform. - **cozystack/website** — KEDA is a new platform component (`guides/platform-stack`), and the postgres reference page gains the `autoscaling` params. The managed-app reference is regenerated from `README.md` by the release bot; the platform-stack component list is hand-maintained. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: to open (new platform component + postgres autoscaling params) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: to open (new `autoscaling` block in postgres values.schema.json) ### Depends on - #3951 — bump CloudNativePG to 1.30.0. The HPA can only drive the CNPG `Cluster` scale subresource on CNPG ≥ 1.28.4, where it serves `status.selector`; without it the HPA fails with `InvalidSelector`. This PR should not merge before #3951. ### Release note ```release-note feat(postgres): add optional horizontal autoscaling of read replicas via KEDA, configured through the new `autoscaling` values block; KEDA ships as a new optional platform component. Requires CloudNativePG >= 1.28.4 for the Cluster scale subresource selector. Note: every Postgres release now applies client-side (`release.cozystack.io/helm-server-side-apply: "false"`) so the autoscaler's live `/scale` count is not reverted by Helm; a kind-wide side effect is that chart-rendered fields on Postgres releases are no longer re-asserted by server-side-apply drift correction (driftDetection is unset), a documented trade-off with a per-instance apply strategy as follow-up. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable PostgreSQL read-replica autoscaling with KEDA. * Added CPU and read-connection metrics, replica bounds, replication-lag safeguards, dry-run mode, and transition handling. * Added optional KEDA platform support, Grafana autoscaling dashboard, and Prometheus alerts. * **Documentation** * Clarified integer thresholds, prerequisites, configuration, and transition guidance. * **Bug Fixes** * Improved validation and actionable errors for unavailable KEDA or invalid settings. * **Tests** * Expanded coverage for autoscaling, observability, package selection, and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
b4d621f4f7 |
fix(backups): delete Redis backup objects on cleanup and size the restore Pod
Delete the backup object when its Backup is removed. The Redis driver
owns its S3 object (one per BackupJob, no engine retention prunes it), so
it now spawns a cleanup-mode Job that deletes the object and holds the
Backup's finalizer until it completes, mirroring the Rabbitmq driver.
Without this a retention-pruned Plan grew the bucket unboundedly and the
key was lost with the CR.
Size the strategy Pod from backupStrategyController.redisBackupResources
so an admin can raise the restore ceiling (the throwaway loader holds the
whole dataset in memory) via chart values, which a direct edit of the
cluster-scoped, Helm-managed Strategy CR could not survive. The
loader-not-ready diagnostic now names the memory limit, the likeliest
cause, instead of only the image version.
Make the endpoint CA volume optional: nothing projects that Secret into
the app namespace, so a required mount wedged every backup Pod on
FailedMount; a missing ca.crt now falls through to the system CA store
and fails the handshake fast, which is not the same as skipping verify.
Promote the Pod-level activeDeadlineSeconds to the Job so a wedged run is
bounded once, not (backoffLimit+1) times. Use find, not ls, for the base
AOF so its no-match diagnostic is not dead code under pipefail.
Pin the object key reaching the rendered Job: both fixtures now render
--key={{ .ObjectKey }} and assert it, so zeroing the key (which left the
package green) is caught. Document .ObjectKey and the cleanup mode in the
CRD render-context, list cozy-default-redis among the Helm-managed CRs,
and mirror the CA/TLS and find fixes into the example strategy.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey Kolkov
|
919efa044f |
fix(backups): branch the Redis Job-existence check on IsNotFound
The readiness re-gating guard read `r.Get(...) == nil` to mean "the Job does not exist yet", folding any non-NotFound error (an apiserver hiccup, an RBAC denial) into that branch. On such a transient error the driver would re-enter the readiness precondition even though the Job may already exist and have completed, reintroducing the spurious terminal failure this guard removes. Switch on the Get error: nil means the Job exists (observe it), NotFound means run the pre-launch readiness gate, and any other error requeues without deciding. Same on the backup and restore paths. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
ff061a8eea |
fix(backups): stop re-gating Redis readiness after the backup Job exists
The Redis backup/restore reconcilers checked the application's Ready condition on every pass, ahead of the Job-state switch. Once the Job has been created that re-check is wrong: if the app's Ready flips False after launch (an unrelated HelmRelease blip) and stays so past the deadline, the driver marks a run Failed whose Job may already have completed, a spurious terminal failure of a successful backup or restore. Gate the readiness precondition on the Job not yet existing; once it does, observe the Job directly. The deadline still bounds the pre-launch wait. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
632f7b31a0 |
test(backups): cover Redis backup-path unmappable kind
The restore path already asserts a terminal failure for an applicationRef that passes the Redis gate but has no REST mapping; add the symmetric backup-path case so both dispatch sides are covered. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Andrey Kolkov
|
b0eb104276 |
feat(backups): add Redis backup/restore strategy driver
The spotahome RedisFailover operator ships no backup CRD, so add a dedicated strategy.backups.cozystack.io/Redis driver that runs a one-shot Job: it dumps an RDB snapshot from the current master (discovered via the operator's sentinel Service) to the system bucket over S3, and on restore replays it (RDB -> RESP via redis-cli --pipe) into the target's master. Structurally a specialisation of the Job/Altinity script-Job pattern with a Redis applicationRef gate and an app-readiness precondition. Backup parameters round-trip through the Backup's DriverMetadata (no underlyingResources snapshot); restore drives from .Backup.ApplicationRef so a to-copy restore reads the source's object-storage prefix while writing into a differently-named target. Both in-place and to-copy are supported; PITR is not (RDB is a discrete snapshot). - api: Redis/RedisList strategy type + generated CRD and deepcopy - driver: reconcileRedis / reconcileRedisRestore + pure helpers, wired into both dispatch switches, supportedBackupStrategyKinds and the no-op restore cleanup case - default: gated cozy-default-redis strategy (system-bucket coords via cozy-backups-creds env secretKeyRef, https:// scheme + optional CA) and the Redis route in the cozy-default BackupClass - image: images/redis-backup bundling redis-cli + curl + an RDB->RESP converter, wired via backupStrategyController.redisBackupClientImage - tests: driver unit tests (gate, precondition, render, param round-trip, both reconcile paths) and the helm rendering-gate coverage - example: examples/backups/redis honest marker round-trip for both restore flows Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Andrei Kvapil
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Aleksei Sviridkin
|
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 --> |
||
|
IvanHunters
|
8738063287 |
fix(operator): reject tar entries containing ".." before any path operation
safeJoin already rejected traversal and re-checked the joined path against destDir, but the guard sits behind a call boundary that CodeQL's go/zipslip barrier does not follow, so the extraction loop reported three high-severity arbitrary-write alerts. Reject the entry name up front instead, which is both the fail-closed spelling and the shape the analysis recognises. Extend the traversal test to cover a ".." buried mid-path, not just a leading one. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
15be9329a1 |
refactor(marketplace): source-centric Tap resource, label-based identity
The aggregated Tap resource no longer derives a community.<org>.<repo> name from the connect URL. A connected repository is identified by its Flux source (tap-<org>-<repo>), and the PackageSource(s) it materializes keep their own declared names. Community is computed from the marketplace-tap label, and disconnect is guarded by that label instead of a name prefix. List also surfaces tap sources with no materialized PackageSource yet (a connect in progress, or one blocked by a collision), carrying the materializer error message, so a blocked tap is visible in the dashboard rather than silently never appearing. The community. prefix constant is removed. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1653da967b |
refactor(marketplace): drop artifact-prefix injection and reserved names
Without the community. rename, a tapped repository's cozyrds ApplicationDefinition references its component's artifact by the concrete <packagesource>-<variant>-<component> name again, so the operator no longer injects a cozystackArtifactPrefix value and the -rd chart renders its cozyrds asset with a plain Files.Get. cozypkg init scaffolds the concrete chartRef. Reserved name prefixes are removed: repositories choose their own PackageSource names, and a clash with a core component is caught on-cluster at tap time (the collision check) rather than by refusing prefixes offline. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
95badea1c8 |
refactor(marketplace): identify taps by marker label, drop community rename
Tapped repositories keep their own declared PackageSource names instead of being renamed under the community. prefix. Tap-ness is now marked by the marketplace-tap label (and source annotation) on the OCIRepository and every materialized PackageSource, so untap, the Flux-source cleanup, and idempotent re-tap key on the marker rather than a name prefix. A declared name that clashes with an existing core component (or another tap) is rejected: the CLI tap fails fast before applying anything, and the operator materializer refuses to overwrite the foreign object, records the reason on the source (annotation and Event), and does not stamp the revision so a corrected artifact retries. The shared check lives in internal/marketplace/collision. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
0c0609ad63 |
fix(marketplace): resolve community chartRef via injected artifact prefix
Fixes the rename-vs-chartRef break found on dev3: a community-tapped
PackageSource is renamed to community.<org>.<repo>, so the assembled artifact
name changes, but a repo's cozyrds ApplicationDefinition baked chartRef.name
under the author's original name and the ExternalArtifact it referenced no
longer existed, leaving every community-installed app permanently ArtifactFailed.
Implements @kvaps's option 2 (derive at the -rd component, keep platform charts
literal):
- naming.ArtifactPrefix exposes the per-(source,variant) artifact-name prefix.
- The Package reconciler injects it as the cozystackArtifactPrefix Helm value,
but ONLY for community.-prefixed sources, so platform HelmRelease values are
byte-for-byte unchanged (no churn, no platform chart edits).
- cozypkg init now scaffolds a community-style -rd chart: cozyrd.yaml renders the
cozyrds asset through tpl, and chartRef.name templates
'{{ .Values.cozystackArtifactPrefix }}-<app>', resolving to the real artifact
whatever the tap-time rename produces.
- validate skips the dangling-chartRef check for a templated name (it cannot be
cross-checked at authoring time; the operator resolves it at install).
Verified end-to-end: init -> validate is clean, and helm template with the
prefix value renders chartRef.name to the exact artifact the reconciler emits.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
950268cccc |
feat(marketplace): reject reserved PackageSource name prefixes
From @kvaps's review: a repository author could name a PackageSource 'cozystack.<x>' or 'community.<x>' and claim the platform or marketplace namespace. Until now validate only rejected an empty metadata.name and the forced rename in the materializer was the sole guard. - validate now flags 'cozystack.'- and 'community.'-prefixed PackageSource names as an error. The escape hatch --allow-reserved-names lives on the caller side only: the index gate (validate-entry.sh) never passes it, so a submitter cannot hand it to themselves. - cozypkg init refuses the same prefixes early with a neutral-name hint. - Document why the ClusterIP fail-open is safe: it falls back only on address resolution, and integrity stays enforced by the fail-closed digest check, so a wrong host yields a mismatch error rather than a content substitution. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b96a97e5c4 |
fix(marketplace): resolve tap artifact host to ClusterIP for the hostNetwork operator
The tap materializer fetches the Flux artifact from status.artifact.url, which is a cluster-DNS name (flux.<ns>.svc/...). cozystack-operator runs with hostNetwork: true and dnsPolicy: ClusterFirst, so it resolves against the host's DNS rather than CoreDNS and that name is NXDOMAIN — every dashboard-driven connect (API Create, which relies entirely on the materializer) then hangs in 'connecting' forever, while the CLI tap works only because it pulls via 'flux pull artifact' from the public registry. Resolve the artifact host to the backing Service's ClusterIP (which a hostNetwork pod reaches directly, the same way the in-cluster apiserver is reached via KUBERNETES_SERVICE_HOST) before fetching. This fails open: a non-cluster host, a missing Service, or a headless/empty ClusterIP falls back to the original URL, so clusters where DNS already works are unaffected. Services are readable through the operator's existing cluster-wide get/list/watch grant. Found on dev3 via cozystack-pr-test; the connect->materialize path never worked on a stock operator deployment before this. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
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> |
||
|
IvanHunters
|
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> |
||
|
IvanHunters
|
7b703f2859 |
fix(cozypkg): satisfy golangci-lint and derive a valid kind for hyphenated apps
Address the linters flagged in review: check the previously-ignored writer, Close, and RemoveAll return values (errcheck), lowercase an error string (ST1005), drop the embedded-field selector (QF1008), remove an unused const, and switch the materializer's post-finalizer requeue from the deprecated Result.Requeue to RequeueAfter. Also fix 'cozypkg init': capitalize now builds an upper CamelCase kind from a hyphenated --app (foo-bar -> FooBar) so the scaffolded ApplicationDefinition is not rejected by the apiserver at install time; covered by a test. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3643fceceb |
refactor(marketplace): fail-safe re-tap rollback and single-source artifact naming
Close the two non-blocking notes from the third review round: - objectExists now fails safe: it returns 'not existing' (eligible for rollback) only on a definite NotFound; any other Get error (RBAC, throttling, timeout) is treated as possibly-pre-existing, so an idempotent re-tap that hits a transient probe error cannot delete a pre-existing resource on rollback. Mirrors the fail-direction of sourceStillReferenced; covered by a new interceptor-based test. - The PackageSource reconciler now calls internal/marketplace/naming.ArtifactName instead of inlining the formula, so the artifact-name convention has a single definition shared by the producer, the marketplace backend, and the validator, and the parity test guards producer-vs-helper. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1fbceee10e |
fix(marketplace): operator RBAC for tap materialization and dot-safe artifact naming
Second review round found the write-path RBAC fix was only half applied and the extracted naming helper diverged from its producer: - The tap materializer runs as the cozystack-controller ServiceAccount, whose ClusterRole granted source.toolkit.fluxcd.io/ocirepositories only get/list/watch. Its finalizer and materialized-revision writes (r.Update on the OCIRepository) were therefore Forbidden, so a dashboard-connected tap never materialized. Grant the controller update/patch on ocirepositories. - naming.ArtifactName replaced dots only in the PackageSource name, but the reconciler replaces them in the variant and component too, so a tapped repo with a dotted variant or component name dropped its packages from the catalog and tripped a false appdef-dangling warning. Replace dots in all three segments and add a direct parity test with dotted names. - Pin cosign-installer and the cosign release in the index gate so the signature verifier is reproducible. - Add a unit test for the tap rollback objectExists predicate. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
be03900262 |
refactor(marketplace): address review — shared naming, tap rollback, pinned gate, trust docs
- Extract the per-component artifact-name convention into internal/marketplace/naming and have the validator and the Tap backend delegate to it, so the component-to-ApplicationDefinition linkage cannot drift across copies. - cozypkg tap only rolls back resources it actually created, so an idempotent re-tap no longer deletes a pre-existing OCIRepository/PackageSource when a later apply fails. - Pin cozypkg and yq in the index publication gate so the auto-merge decision runs on a reproducible toolchain. - Document the trust model explicitly: tap validates structure but does not verify signatures; verification is enforced by the index CI gate (pinned to the recorded cosign identity) and optionally by Flux OCIRepository verification at pull time. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
e61d2c7b53 |
feat(marketplace): connect a community tap from the API with async materialization
Add server-side connect to complete the marketplace write path. Creating a Tap records the intent only: the API validates the oci:// reference and creates a labeled Flux OCIRepository (with an optional pull-credential secretRef), never blocking the request on a registry pull, and returns a Tap whose status shows materialization pending. The connect inputs (url, tag, secretRef) are write-only and are never echoed back or returned on read. A new tap materializer reconciler in the operator watches community-tap OCIRepositories, and once source-controller has pulled the artifact, fetches the tarball over the Flux artifact URL (digest-verified, size-bounded, path-traversal guarded), parses the PackageSource(s) it carries, and materializes them under the community. prefix with their sourceRef repointed at the tap's source. A finalizer cleans up the materialized PackageSources when the source is removed, leaving installed Packages in place. The shared label/annotation keys live in internal/marketplace/tapconst so the API and the operator cannot drift on the connect-to-materialize handoff. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
27d7dbb140 |
feat(api): add per-app helm apply-strategy override annotation
Add release.cozystack.io/helm-server-side-apply on the ApplicationDefinition, threaded through ReleaseConfig into every emitted HelmRelease's Install/Upgrade.ServerSideApply. Empty leaves the helm-controller default (server-side apply on v1.5.0); "false"/"disabled" forces client-side apply. The postgres read-replica autoscaler needs this: KEDA's HPA writes the CNPG Cluster's /scale subresource (spec.instances) while the chart renders a constant seed. Under helm-controller's default server-side apply the rendered field is force-owned and reverts KEDA's live value on every reconcile. Under client-side apply helm-controller patches the CRD from the previous-vs-new rendered manifest without consulting live state, so the unchanged constant yields no patch and the /scale value survives (verified against Helm v4 createPatch: unstructured/CRD objects take the two-way jsonpatch.CreateMergePatch(original, modified) path unless threeWayMergeForUnstructured is set, which helm-controller v1.5.0 leaves at its default). Guarded by TestConvertApplicationToHelmRelease_ServerSideApplyOverride on the tenant-release builder (convertApplicationToHelmRelease), and by TestParseHelmServerSideApplyAnnotation. Removes a stale package_reconciler_test guard that asserted a false invariant on the wrong (platform Package) builder. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
826f08972f |
fix(apps/postgres): make autoscaler fail-safe, dip-free, and observable
Address review findings on the KEDA read-replica autoscaler where the
implementation diverged from the merged design proposal.
Fail-safe read load. Gate the `or vector(0)` floor on the existence of ANY CNPG
instance pod (`count(kube_pod_labels{...instance_role=~".+"}) > 0`) — primary or
replica — so the query is empty ONLY when no CNPG pod labels exist at all (a
broken kube-state-metrics pipeline), not during initial provisioning when the
sole replica-side pod is CNPG's join Job (jobRole, not instanceRole). Also pin
`ignoreNullValues: "false"` on the trigger: KEDA's prometheus scaler defaults it
true, turning an EMPTY result into metric 0 (not an error) — the HPA would then
desire 0 and clamp to the floor, i.e. silently scale a broken-pipeline cluster
DOWN. With it false an empty result is a scaler error, so the HPA HOLDS and
DatabaseAutoscalerScalerErrors fires; gating on any instance role keeps a healthy
cluster (including a long basebackup) from tripping that alert.
Constant seed = max(replicas, effectiveMin), so enable/dryRun/transition flips
are a merge no-op with no dip and never undercut the quorum floor.
Fail closed when the quorum floor exceeds maxReplicas (was: effectiveMax silently
raised both bounds, unbounded for a large maxSyncReplicas).
Dry-run / transition keep the HPA (unidirectional pause pair, not the HPA-deleting
full pause) but both SelectPolicy=Disabled, so desired normalizes to current — the
recommendation is read from the read-load metric, not a diverging desired count.
Observability. DatabaseAutoscalerAtMax keys on desired >= maxReplicas, NOT the
ScalingLimited condition (which is also true at the floor, so it fired on every
idle cluster with wrong "raise maxReplicas" advice). Dashboard read-load panels
are per-cluster (label_cnpg_io_cluster) with a CPU panel for ReadCPUUtilization
and the exact clamp in descriptions; ScaleStuck's Pending intersection is scoped
to CNPG pods; alerts.yaml narrative comments moved to Go-template comments so they
do not ship into the PrometheusRule.
Guard the seeding invariant: TestBuildHelmReleaseSpecNoForceOrDriftDetection.
Regenerated README/schema/types.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Aleksei Sviridkin
|
18d02d813a |
fix(cacert): make the comments match the reconciler (#4064)
## What this PR does Comments in the CA-extraction package claim guards the code does not have. No behaviour change; one test is renamed to match the property it pins. - `ManagedLabel` said the reconciler only ever writes over a Secret carrying that label. It does not. `isOurProjection` decides that from the Secret type and the owner reference, and never looks at the label, which any writer in the namespace can strip. - `managedByCozystackLabel` said the reconciler never writes it. It deletes it whenever the selectors digest changes, which is what gets the projection re-admitted by the lineage webhook. - `projectionSuffix` said the dotted canonical name cannot collide with an operator-owned object. A release the apps API created cannot carry a dot, since application names are DNS-1035 labels and `release.prefix` is restricted to `^[a-z0-9-]*$`. The guarantee stops there: the controller reads the release from the `helm.toolkit.fluxcd.io/name` label, where a dot is legal, so a hand-written HelmRelease sits outside it. On the validated path the dot can only come from the suffix an operator or chart appends, which nothing validates at all, and since a canonical name ends in `.tenant-ca` and carries that one dot, a suffix ending in `.tenant-ca` is the only one that reaches a canonical name. The comment says all of that now. - The test pinning that name claimed the property for any engine suffix, in its doc and in its own name, while its loop samples only dot-free ones. It is now `TestProjectionNameCannotCollideWithADotFreeEngineSuffix`, and its doc says what the loop covers. ### Screenshots Not a UI change. ### Downstream repositories Comments and one test name in two Go files. No package, values key, schema field, CRD, metric, release asset or `hack/` target changes, so nothing in the trigger map fires. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified certificate reconciliation behavior, including label handling, projection naming, and source certificate scenarios. * Documented naming guarantees and their API boundaries more precisely. * **Tests** * Updated test descriptions and failure messaging to clarify coverage for dot-free engine suffixes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
773d8dabce |
fix(gateway): withdraw TLSRoute acceptance when the mode drops passthrough
Edge terminates TLS at the class provider, so its Gateway carries plain HTTP listeners only and a route pinned to a passthrough section has nothing left to attach to. The api, vm-exportproxy and cdi-uploadproxy endpoints of publishing.exposedServices are the ones that pin one. Hostname claims are empty for every whole-apex mode, so the status pass ran over an empty ref set and left the Accepted=True written while the tenant ran HTTP-01 standing on a route the switch had just unpublished. Whoever went looking for the dead endpoint read that condition as proof the route was fine. The rule this settles on is that the controller owns a condition only where it judges. Edge judges every attached route by the absence of the listener it needs and writes Accepted=False with Gateway API's NoMatchingParent, the reason defined for a parentRef whose port or sectionName matches no listener. The other whole-apex modes render those listeners and serve them but collect no claims, so they hold no opinion: they drop the entry rather than leave a withdrawal standing on a listener that is serving, naming a class provider that no longer terminates anything. HTTP-01 collects claims and has already judged every route this pass could speak about. A route declaring no hostnames is judged by none of them, because claims are gathered inside a loop over spec.hostnames and TLSRoute v1alpha2 sets no minItems on that field. Withdrawing one would write a condition with no path back, since HTTP-01 renders the listener again and judges only the routes that produced claims, so both directions skip it. The tests read the controller name from the constant the controller stamps rather than restating it, so a helper cannot go looking for a name that is no longer written and report absence it never checked. The switch already deletes the redirect route, the Issuer and the wildcard Certificate it invalidates. The statuses it owns were the one thing left behind. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
7851969448 |
docs(gateway): document GatewayClass selection and edge TLS termination
An operator setting up a class whose provider terminates TLS upstream had no page answering their questions: which platform values bound the choice, what a tenant writes to make it, what a move between classes does to certificates and to the replicated wildcard Secret, and how the mechanism fails when a configured class name matches nothing installed. Add docs/gateway-classes.md covering that surface. Describe the mode and its consequences in the chart README alongside the other three, including the one constraint stated nowhere else: the publishing tenant cannot select a class whose TLS termination differs from the platform default, because the cluster-wide ACME issuers derive their solver pin from that default alone. State the blast radius where it actually applies. The endpoints an edge Gateway stops publishing exist only where those routes are rendered at all, and both pages say what the transition does to the route conditions the controller owns, so a reader is not left inferring it from the code. The security model names the selector each listener really carries. The port-80 listener selects on the built-in kubernetes.io/metadata.name label, which kube-apiserver writes and a namespace cannot forge; the HTTPS and TLS-passthrough listeners select on namespace.cozystack.io/gateway, written by the tenant chart and the controller, so that guarantee does not extend to them. What closes the exposure is that Namespace is cluster-scoped with no tenant verb on it, and that the route-hostname policies bound what a labelled namespace could publish. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
57c51f58ee |
feat(gateway): stop replicating the wildcard key into edge namespaces
The wildcard-secret controller replicates the operator's TLS Secret, private key included, into every namespace that owns a termination point, and a namespace owning a Gateway counted as one without further question. A Gateway whose class ends TLS at its provider's edge terminates nothing locally, so the replica has no consumer there. Read certMode off the tenant's own TenantGateway rather than a cluster-wide value, because the class is a per-tenant choice. Keep the key whenever the answer is not a confident no: a namespace that also owns an ingress controller still terminates locally, a namespace holding more than one TenantGateway keeps it while any of them is not at the edge, and an unreadable list or a namespace whose TenantGateway has not appeared yet keeps it too, so a transient API error never withdraws a key a Gateway may still be serving. Watch TenantGateway, so the withdrawal follows the class change rather than waiting for the periodic resync. The watched set is a value the tests assert on, because the controller-runtime builder exposes nothing about what it watched: a dropped watch would otherwise leave both suites green while the withdrawal quietly degraded to that resync, leaving the private key in a namespace that stopped terminating TLS. Pruning the replica is not the same as leaving the namespace key-free. The Certificate objects go, but the Secrets they issued stay, as they do on any other mode transition. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
a2cb47c961 |
feat(gateway): update the CRD bundle to Gateway API v1.6.1 and gate GRPCRoute hostnames (#3872)
## What this PR does Bumps the vendored Gateway API CRD bundle in `packages/system/gateway-api-crds` from v1.5.1 to v1.6.1. The package Makefile pins the upstream ref, so the change is that pin plus the re-rendered `templates/crds-experimental.yaml`. The bundle version is not only bookkeeping: it is a compatibility signal consumers read at runtime. Every CRD in the bundle carries a `gateway.networking.k8s.io/bundle-version` annotation, and a Gateway API controller can read it off the `gatewayclasses` CRD to decide whether the installed bundle matches the one it was built against. Controllers that do this typically accept any patch release but require an exact major and minor match, and report `SupportedVersion=False` with reason `UnsupportedVersion` on their GatewayClass when the minor differs, a status condition that surfaces the skew rather than a functional break. The Cloudflare Tunnel Gateway API controller proposed for the platform in #3858 is built against Gateway API v1.6.1 and performs exactly that comparison, so against the v1.5.1 bundle shipped today its GatewayClass reports the bundle unsupported, and with this bump it reports supported. Neither change depends on the other landing first: this PR only moves the bundle, and the check lives entirely on the consumer side. One consequence worth stating plainly, since it will outlive this PR: an exact-minor check makes the shipped bundle version a coupled pin for any such controller. A future bump to v1.7.x flips the same condition back to `UnsupportedVersion` until that controller is rebuilt against the newer bundle, so the two want to move together. The bundle still emits the same twelve CustomResourceDefinitions and the same two `safe-upgrades.gateway.networking.k8s.io` admission objects, and every CRD still carries the `helm.sh/resource-policy: keep` stamp the Makefile's post-processing step adds. One of the policy's two CEL rules is rewritten, though, and it changes what a cluster accepts. That has its own section below. Two kinds change their version surface: TCPRoute and UDPRoute gain a served `v1` that takes over as the storage version, and their `v1alpha2` stays served with a deprecation marker. Every other kind keeps exactly the versions it served at v1.5.1. That matters because the Cilium operator resolves GatewayClass, Gateway, HTTPRoute and GRPCRoute in `v1`, ReferenceGrant in `v1beta1` and TLSRoute in `v1alpha2` before it enables its Gateway API controller; all six are still served by the v1.6.1 bundle. TLSRoute in particular keeps `v1alpha2`, which the tenant gateway controller, the platform TLSRoute templates and the tenant hostname admission policy all address. Schema changes are small and land on fields nothing in this repository sets. `sessionPersistence.idleTimeout` is gone from HTTPRoute, GRPCRoute and XBackendTrafficPolicy, removed upstream under GEP-1619 because no implementation ever implemented it and HTTP cookies have no native idle-timeout mechanism, so no working behaviour is lost even though the field disappears from the schema; HTTPRoute gains a lower bound on `retry.attempts` and now rejects duplicate `retry.codes`; ReferenceGrant requires `spec` at the root. In the other direction, Gateway infrastructure annotations and frontend CA certificate references go from a limit of 8 to 16, and TLSRoute hostnames from 16 to 1024. No CRD schema loses or changes a CEL validation rule, and the six added belong exclusively to the new TCPRoute and UDPRoute `v1` schemas. ### The safe-upgrades admission policy accepts a different set of bundles The experimental kustomization ships a `ValidatingAdmissionPolicy` and its binding, both named `safe-upgrades.gateway.networking.k8s.io`, next to the twelve CRDs. The binding is `validationActions: [Deny]` with `failurePolicy: Fail`, matching CREATE and UPDATE on `apiextensions.k8s.io/v1 customresourcedefinitions` cluster-wide, and Cozystack installs it in two places: on the host through `cozystack.gateway-api-crds`, and inside every guest cluster that sets `addons.gatewayAPI.enabled`. Its second rule reads the `gateway.networking.k8s.io/bundle-version` annotation off an incoming CRD. At v1.5.1 that rule was a blacklist: reject anything matching `v1.[0-4].\d+` or containing `v0`, allow the rest. At v1.6.1 it is a whitelist: allow `v0.0.0-dev` outright, otherwise require the annotation to start with `v1.` and not match `^v1\.[0-4](\.|$)`. Evaluating both expressions over a spread of annotation values: | bundle-version | v1.5.1 policy | v1.6.1 policy | | --- | --- | --- | | `v1.6.1`, `v1.5.1`, `v1.5.0-dev`, `v1.10.0` | allow | allow | | `v1.4.0`, `v1.4.10`, `v1.0.0` | reject | reject | | `v0.0.0-dev` | reject | **allow** | | `v2.0.0` | allow | **reject** | | `1.6.1` (no leading `v`) | allow | **reject** | | `v1.4` (no patch) | allow | **reject** | | `main`, empty string | allow | **reject** | Nothing in this repository trips the new rejections: every CRD the bundle ships is stamped `v1.6.1`. What is exposed is an operator applying a Gateway API CRD set from somewhere else on a cluster that carries the policy. A fork, a vendor build or a nightly whose annotation reads `1.6.1` or `v2.0.0` used to go in and is now denied with `reason: Invalid`, naming a policy the operator may not know is installed, and whatever HelmRelease owns those CRDs then sits un-Ready. Uninstalling the policy is the escape hatch, and the denial message says so. The same hunk fixes an upstream inconsistency: at v1.5.1 the policy pair was stamped `v1.5.0-dev` while its twelve CRDs said `v1.5.1`. At v1.6.1 all fourteen documents agree. The Gateway and HTTPRoute objects the tenant gateway controller renders were serialized across all three certificate modes and checked against both the v1.5.1 and the v1.6.1 schemas; the two runs are identical, so the bump does not change what the controller can create. `go.mod` still pins `sigs.k8s.io/gateway-api` v1.4.1 and this PR deliberately leaves it there. The CRD bundle and the Go client types have been on separate versions since before this change, and nothing the controller emits depends on the gap. One thing to know before the next bump: from v1.6 on, the two things upstream calls the experimental bundle disagree. This package follows `config/crd/experimental`, the kustomization, which emits 12 CRDs at v1.6.1; the published `experimental-install.yaml` for the same tag emits 13, the extra one being `xbackends.gateway.networking.x-k8s.io`, new in v1.6 and not listed in the kustomization. At v1.5.1 the two agreed, so there was nothing to choose between. Nothing in this repository references XBackend and no vendored CRD depends on it, so following the kustomization costs nothing today, but whoever bumps next inherits the choice, and it should be made deliberately rather than by whichever command gets typed. The second commit corrects a comment this bump invalidated. It claimed TLSRoute stays at `v1alpha2` through Gateway API v1.5 and that the promotion to `v1` was still upstream-only; TLSRoute has been served at `v1` since v1.5.0, so the claim was already wrong before this bump and the version range it named is now wrong too. The third commit closes a gap that correction exposed. `cozystack-route-hostname-policy-tls` named `v1alpha2` alone while the bundle serves `tlsroutes` at `v1`, `v1alpha2` and `v1alpha3`: three served versions against one named. The gap is not new and this bump neither creates nor widens it: the v1.5.1 bundle and the v1.6.1 one serve the same three versions, with the same two marked deprecated. Coverage of the two unnamed versions rested entirely on request conversion, because `matchPolicy` is unset on both rules in that file and so defaults to `Equivalent`, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by `failurePolicy: Fail`, because nothing failed. The rule now names all three, which is the shape the HTTPRoute rule above it already used, and `hack/route-hostname-policy-version-coverage.bats` reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, so a future bundle move cannot outrun the list silently, in either direction. That guard carries negative fixtures rather than only watching a correct tree pass. Three of them drive `assert_covers` itself, with the two readers redefined in a subshell, against an omitted served version, an empty served set and an empty named set. Each requires the specific complaint text rather than merely a non-zero exit, so a helper renamed out from under it fails for the right reason instead of reading as a successful rejection. A fourth drives the comparison helper directly. The fourth commit pins the vendored bundle version in the CRD package's unit suite. Nothing outside the package Makefile named a Gateway API version, so re-running `make update` against a different ref landed green: twelve CRDs, every one stamped, document count unchanged, suite passing. The new case reads the `gateway.networking.k8s.io/bundle-version` annotation off every rendered CRD and fails when it is not the version the Makefile pins. The fifth commit extends the version-coverage guard to `cozystack-gateway-hostname-policy`, which names the `gateways` versions literally the same way the two route policies do, and whose version set lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Two notes for whoever reads the release: the TCPRoute and UDPRoute storage version moves from `v1alpha2` to `v1`, and while nothing in this repository ships either kind and the CRDs declare no conversion block, an upgraded cluster accumulates both entries in `status.storedVersions` — inert today, load-bearing only if a later bundle stops serving `v1alpha2`. The sixth commit gives GRPCRoute the same hostname rule the other two route kinds have. GRPCRoute carries `spec.hostnames` and it can reach a tenant Gateway: the port-443 listeners pin `allowedRoutes.kinds` to HTTPRoute and TLSRoute, but the port-80 listener leaves `kinds` unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could claim any hostname on the plaintext listener with nothing checking it. Tenants hold no `gateway.networking.k8s.io` RBAC in Cozystack, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The rule is the HTTPRoute rule with `grpcroutes` in place of `httproutes` and the one version the bundle serves for it, and the coverage guard pins that list to the bundle like the others. Several places in the tree enumerate which route kinds the hostname policies cover, and all of them named two: the port-443 kinds invariant in the tenant gateway controller and the test that pins it, `packages/extra/gateway/README.md`, and the threat model together with the self-assessment. They name three now. The port-443 kinds set also carries an accurate reason now. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. The README had also described HTTPS and TLS-passthrough listeners as carrying different kinds, where the controller gives both the same one. One more commit makes the suite's existing assertions bite. A ValidatingAdmissionPolicyBinding whose `policyName` points at another policy renders without complaint and leaves its own policy inert, and the suite pinned `validationActions` on the bindings while never pinning `policyName` at all, so pointing the GRPCRoute binding at the HTTPRoute policy kept every case green with the GRPCRoute control doing nothing. All three bindings now pin the policy they name. In the same file, `uncovered()` assigned to the variable names its caller reads back when printing a mismatch, so a call site with the arguments swapped would have printed a diagnostic naming the wrong side. Two things are named rather than made. The version-coverage guard collects `apiVersions` from every `resourceRules` entry mentioning the resource without also matching on `apiGroups` or `operations`, so a rule in another group, or one scoped to a single verb, would donate its versions to the set coverage is measured against; the chart's own suite pins the group and the resource on the rule it checks, which closes that a layer below. And `packages/system/cozystack-api/templates/api-tlsroute.yaml` renders `gateway.networking.k8s.io/v1alpha2` while the bundle serves `v1` as storage and marks `v1alpha2` deprecated, but the identical pin sits in the kubevirt and kubevirt-cdi TLSRoute templates, and the pinned `sigs.k8s.io/gateway-api` carries no `apis/v1` TLSRoute type for the controller side to follow, so moving one of the three here would leave the tree less consistent than leaving all three. Closes #3789. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file: neither `packages/system/gateway-api-crds/Makefile` nor its rendered CRD template matches any entry. Leaving every box empty rather than ticking the "no downstream repository" line, because the docs site carries a Gateway API version statement that is already inaccurate and needs a decision that is not mine: `content/en/docs/next/networking/gateway-api.md` and its v1.5 and v1.6 copies say Gateway API v1.5 ships TLSRoute at `v1alpha2` and that the graduation to `v1` is still upstream-only, when v1.5.0 already shipped TLSRoute `v1` as the storage version. ### Release note ```release-note chore(gateway-api-crds): update the vendored Gateway API CRD bundle to v1.6.1. TCPRoute and UDPRoute gain a served v1 that becomes their storage version, with v1alpha2 still served and now deprecated; every other kind keeps the versions it served at v1.5.1. Upstream removed `sessionPersistence.idleTimeout` from HTTPRoute, GRPCRoute and XBackendTrafficPolicy; the field is not set by anything Cozystack renders, but a hand-written route that carries it loses it silently on its next write, with no admission error. The bundle's `safe-upgrades.gateway.networking.k8s.io` ValidatingAdmissionPolicy changes which CRDs it accepts. Cozystack installs it cluster-wide on the host and in every guest cluster with `addons.gatewayAPI.enabled`. Its `gateway.networking.k8s.io/bundle-version` rule was a blacklist on two patterns and is now a whitelist on the `v1.` prefix with a `v0.0.0-dev` escape hatch. A bundle annotated `v0.0.0-dev` is now accepted where it was denied. A bundle annotated `v2.0.0`, `1.6.1` without the leading `v`, `main`, `v1.4` without a patch, or carrying an empty annotation is now denied where it was accepted. Nothing Cozystack ships is affected, since every CRD in the bundle is stamped `v1.6.1`, but applying a Gateway API CRD set from a fork, a vendor build or a nightly on such a cluster can now fail admission with `reason: Invalid`. Uninstalling the policy remains the documented escape hatch. fix(cozystack-basics): the tenant TLSRoute hostname admission policy now names every served TLSRoute version (`v1`, `v1alpha2`, `v1alpha3`) instead of `v1alpha2` alone. Coverage of the other two rested on request conversion, which holds only while a named version stays served, and a rule left selecting nothing is not caught by `failurePolicy: Fail`. feat(cozystack-basics): GRPCRoute hostnames in `tenant-*` namespaces are now gated by the same admission policy that gates HTTPRoute and TLSRoute. A tenant Gateway's port-80 listener leaves `allowedRoutes.kinds` unset, which admits GRPCRoute by protocol default, so a GRPCRoute could claim any hostname on the plaintext listener unchecked. Tenants hold no `gateway.networking.k8s.io` RBAC, so this is defence-in-depth against a chart bug rather than a tenant-user control. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added hostname admission validation for `GRPCRoute` resources. - Expanded `TLSRoute` hostname validation to support all served Gateway API versions. - Ensured route hostname policies consistently cover HTTP, TLS, and gRPC routes. - **Security** - Updated listener policies to restrict port-443 routes to supported HTTP and TLS route types. - Clarified fail-closed hostname validation behavior across Gateway, Route, and Ingress resources. - **Documentation** - Updated security and threat-model documentation to include GRPCRoute handling and validation coverage. - **Maintenance** - Updated the bundled Gateway API CRDs to version 1.6.1. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
fb740db0b0 |
docs(security): separate the fail-open Gateway-listener policy from the fail-closed route policies
The self-assessment summarised the whole hostname policy set as fail-closed, while its own table row states that the Gateway-listener policy passes a Gateway whose namespace carries no host label. Say which policies are which. The port-443 kinds comment in the reconciler test also tied the GRPCRoute admission rule to the port-80 listener; the rule matches the kind wherever it attaches, and the port-443 exclusion is a least-privilege choice, not the reason the rule exists. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |