mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 00:27:04 +09:00
docs/agents-dev-principles
195
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
d9689bd6e7 |
fix(gateway): stop describing Cilium 1.19.5 as the shipped release
With Cilium at 1.20.2 the tenant gateway comments, README and one status message still explained behaviour as that of "the pin" at 1.19.5. Several of those claims no longer hold: a Gateway with two routed passthrough ports now gets an Envoy listener per port, Cilium judges a route's kinds against the listener the route names, and the shipped operator accepts an HTTPS and a passthrough listener on one port when the passthrough hostname is strictly narrower. Behaviour that is unchanged is re-anchored on the 1.20.2 source, behaviour that is gone is stated as history, and the refusal of one hostname on two passthrough ports no longer blames a Cilium limitation the shipped release does not have. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ad3f15d4f3 |
feat(gateway)!: reserve a passthrough listener's name by the spec alone
The TenantGateway controller withheld the HTTPS-terminate listener for a name a passthrough listener answers only once a TLSRoute was attached there and resolved a backend, mirroring how Cilium builds its filter chains. That kept a served endpoint up on Cilium 1.19.5, but it tied every route's status to one implementation's translator and needed Service and ReferenceGrant watches to land the deferred decision. The shipped operator accepts a wildcard terminate listener beside a narrower passthrough listener on 443 and keeps conflicting every other overlap. A terminate listener for the passthrough name itself is not narrower, so on 443 the rule now protects the dataplane outright; on a native port Gateway API would serve the pair, and the rule holds as a policy that keeps a platform name with its passthrough listener. Reading it off the spec makes the outcome the same on every implementation. Under http01 an HTTPRoute claiming such a name gets no listener and no certificate whether or not a TLSRoute is attached. A route whose every hostname is reserved reads Accepted=False with NoMatchingListenerHostname; one that also claims a free hostname stays Accepted and is served there, as Gateway API accepts a route whose hostnames intersect at least one listener. dns01 and existingSecret are unchanged: their wildcard listener still covers such a name, and the passthrough listener answers it by its exact server name. The chart writes the passthrough services on the publishing tenant alone, so the names reserved by default are the ones the platform publishes there through its own TLSRoutes. The shape an upgrade changes is an HTTPRoute on such a name whose platform TLSRoute forwards nowhere, because it never attached (the package owning the service is not installed) or because none of its backends resolve: that route kept a terminate listener and now gets none. The backend resolution, its watches and the RBAC rules that served them go away. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bdca165f4c |
feat(gateway): report a GatewayClass that does not serve TLSRoute
A GatewayClass lists the features it serves in status.supportedFeatures, and TLS passthrough is TLSRoute. On a class that leaves it out the passthrough listeners are accepted and serve nothing, so the TenantGateway now reads Ready=False/GatewayClassUnsupported naming the class instead of Ready. A class that lists no features at all is not judged: implementations predating the field leave it empty. The class writes that list after it is accepted and may change it when its controller is upgraded, so a GatewayClass event requeues the TenantGateways on it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d624049a3a |
feat(gateway): allow tlsPassthroughListeners under dns01 and existingSecret
The refusal existed because Cilium 1.19.5 served every Gateway port from one Envoy listener matched by SNI alone, so the "*.<apex>" terminate listener of the wildcard modes and a passthrough listener under the apex could reach each other's backend. The shipped Cilium 1.20.2 gives each port its own Envoy listener when SNI overlaps across ports, and Gateway API treats listeners on different ports as compatible, so the pair is served as declared. edge still refuses the field: it renders no TLS listener for an entry to bind to. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2eeaa59055 |
test(controller): seed the DataVolume informer for controller-runtime 0.25
controller-runtime 0.25 made FakeInformer's sync state a private channel, so the DataVolume watch tests no longer compile: they toggled the exported Synced field and its lock. FakeInformers also builds every informer it creates already synced, so an unsynced informer has to be seeded into InformersByGVK and marked synced with Synced() later. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bf03c84711 |
chore(deps): move to the Kubernetes 1.37 libraries without the apimachinery fork (#4473)
## What this PR does This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, and drops the `k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and #4472; those were a stack of the same commits. The four commits are meant to be read one by one, and each builds and passes the tests on its own: 1. **Plain watches stop getting the initial-events-end bookmark.** The apiserver defaults `sendInitialEvents=true` onto any watch with an empty or `0` resourceVersion and leaves bookmarks off, and our aggregated storages took that flag alone as a WatchList request. From `k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api panics on that bookmark, so `kubectl get -w` on any cozystack-api resource would die after the bump. The marker now needs both flags, the same condition the watch cache uses. There are tests per storage and one through the real handler chain. 2. **The apimachinery fork goes.** Every apps kind shares the `Application` Go type, and stock `Scheme.ConvertToVersion` picks the first kind registered for it. The fork patched that; the apps group now encodes through a serializer that keeps the kind the object carries. A handler-chain test covers create, get, list, update, watch, apply and table for two kinds and fails without it. 3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the plugin is on by default and refuses every write until it has synced `mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and which clusters before 1.36 do not serve. Without this, every create, update and delete would fail after the bump. It is also re-applied in `Complete`, since `--disable-admission-plugins` replaces the list. 4. **The bump itself**, with the regenerated clients, openapi and strategy CRDs. Commits 2 and 4 are where the review effort goes; 1 and 3 are small. Known changes in behaviour: - Without the fork, the row objects of a Table requested with `includeObject=Object` carry the first registered app kind. kubectl asks for that only with `--sort-by`, and it sorts on fields, not on the kind. Nothing at this layer can fix it, because the handler converts through `APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks and ValidatingAdmissionPolicy still see the right kind. - The alpha PodSpec field `workloadRef` became `schedulingGroup` upstream. Once the new strategy CRDs are applied, the apiserver prunes `workloadRef` from stored strategies it serves, and their next write erases it. It only ever worked behind an alpha feature gate. The branch is rebased onto the Renovate rollup (#4447), with `go mod tidy` and codegen rerun on top of it. ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note chore(deps): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application API responses now preserve the requested resource kind across operations, including lists and watches. * Backup strategy resources support additional Kubernetes pod settings, including probe protocols, volume ownership options, and scheduling groups. * Backup strategy schemas include updated Kubernetes pod fields and descriptions. * **Bug Fixes** * Initial-event watch bookmarks are sent only when requested; plain watches continue without an ending bookmark. * Mutating Admission Policy is disabled by default, including when admission plugin options are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ace69cb075 |
docs(controller): say what a nil DataVolume reader means
The field comment still said DataVolumes do not count while the reader is nil, and that the reader appears once the kind is served. Neither holds: the reader is installed only after the informer has synced, and until then a monitor keeps its stored verdict, which still decides Operational. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a6a32d79cd |
fix(controller): recompute stored DataVolume verdicts once the reader is in
A reconcile that runs before the DataVolume watch starts finds no reader, keeps the monitor's stored verdict and is not requeued. The watch's initial replay then delivers only the DataVolumes that still exist, so a monitor whose DataVolume was deleted while the controller was down kept reporting it not ready until the periodic resync, hours later. Installing the reader now queues every monitor that holds a DataVolume verdict, through the controller's own queue, so each is recomputed against the synced cache. Monitors without a verdict are left alone: the replay already reaches every DataVolume they could select. If the monitors cannot be listed, the next discovery poll retries. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
694f4f2ff9 |
fix(controller): report a disk's PVC Workload by its DataVolume
CDI copies the DataVolume labels onto the PVC it creates, so the vm-disk monitor also publishes a Workload for that PVC, and its readiness was bind state alone. Mid-import the PVC is already Bound: the Workload read operational while the monitor, and the Application's WorkloadsReady, said the disk was not populated. A PVC whose controller is a DataVolume the monitor finds not ready now reads not operational. While the DataVolumes cannot be read, its Workload keeps the verdict it has, as the monitor does. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
366b5e80f4 |
perf(controller): skip the monitor status write when nothing changed
While the DataVolume watch syncs, every WorkloadMonitor of every kind is requeued every few seconds, and an informer that cannot sync keeps that window open. Each pass wrote status unconditionally, so a cluster with many monitors sent the API server a steady stream of writes that changed nothing. Comparing against the stored status first leaves the requeue as cache reads only. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
dd72583dfd |
fix(controller): record an event when DataVolumes cannot be read
A failed DataVolume List keeps the monitor's last verdict and retries, and the only trace of it was a controller log line. A monitor with no verdict yet then reads operational while its disks are unreadable, with nothing a tenant can see saying why. A Warning event on the monitor makes the failure visible where the bucket metrics failure already is. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
beeb7994cc |
fix(controller): start the DataVolume watch source only once
A controller-runtime Kind source adds an event handler to the informer when it starts and never removes it; cancelling it on a sync timeout only ends its context. Building a new source on every failed sync attempt therefore left one more handler on the DataVolume informer per retry, each enqueueing the same events, until the controller restarted. The source is now started once and later attempts only wait for the informer to sync. Because the started source keeps delivering events between attempts, the syncing flag stays set until the reader is installed, so a reconcile queued in that gap is requeued as well. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a9b276f99b |
fix(controller): requeue monitors reconciled while the DataVolume watch syncs
Starting the DataVolume source replays every DataVolume to its handler at once, and the handler queues the matching monitors, while the reader is installed only after the source has synced. A monitor reconciled in that window saw no reader, kept its last verdict and returned with nothing left to queue it again, so a disk that failed or finished while the controller was down could keep a wrong WorkloadsReady until the next unrelated event. Such a reconcile still publishes the replica counts and now asks to be requeued after a few seconds, until the reader is in place or the sync attempt gives up. It is a requeue rather than an error because the window is expected: an error would count as a reconcile failure and log a line for every monitor in the cluster while it lasts. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
99f3d3b2b7 |
feat(controller): tag a DataVolume cause with a condition reason
WorkloadsReady=False covers different situations: a stopped VM has no pod and its monitor reports it not operational on purpose, while a disk whose DataVolume never populates is broken. A consumer that wants to react to one and not the other had only the message text to go by. The WorkloadMonitor status gains a Reason next to Message, set to DataVolumeNotReady by the DataVolume check, and the Application API carries it into the WorkloadsReady condition in place of the generic reason whenever a monitor that is not operational names one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
81ffcda86f |
fix(controller): keep publishing monitor status when DataVolumes are unread
The DataVolume List sits on the reconcile path of every WorkloadMonitor, and returning its error before the status write froze the replica counts and Operational of a monitor that selects no DataVolume at all whenever CDI's API hiccupped. The status is now written either way, with the last DataVolume verdict kept whenever the DataVolumes could not be read, and the error still returned so the request is retried. The same applies while no reader is installed yet, so a stuck disk does not read as ready for the window between a controller restart and the DataVolume informer syncing. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
48ece79ffc |
feat(controller): name the DataVolume that keeps a disk not ready
For a vm-disk, WorkloadsReady is the one condition that reports whether CDI populated the disk, and its generic "One or more workloads are not operational" gives an operator nothing to act on: a disk has no workloads, and the stuck DataVolume and its phase are exactly what they need to know. The WorkloadMonitor status gains a message that the controller fills with each selected DataVolume that is not ready and its phase, and the Application API uses the messages of the monitors that are not operational in WorkloadsReady instead of the generic text whenever one of them names a cause. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bf7e8f5bbb |
fix(controller): count DataVolume readiness in WorkloadMonitor
A vm-disk application reported its workloads operational whatever state its DataVolume was in, so an import or clone that never finished left the disk looking healthy while it was unusable. Its monitor has minReplicas 0 and only pods fed Operational. The monitor now also lists the DataVolumes its selector matches and reports not operational while any of them is in flight, Failed, Unknown or without a phase. UploadReady counts as in flight: CDI holds it from the moment the upload server is ready until the transfer succeeds, so it covers an empty disk and a half-uploaded one alike. Waiting for the first consumer counts as ready, since that is where such a disk rests, and so does a phase this controller does not know, so a CDI release that renames one cannot turn working disks red. This is deliberately not a Helm wait: a readiness gate holds a helm-controller worker for its whole window, and a disk that never populates would keep taking it back. The signal lands in the application's WorkloadsReady condition instead, and Ready keeps following the HelmRelease. CDI ships with the IaaS bundle only and can be installed while this controller is running. A watch on a kind whose CRD is absent keeps the controller from starting, so the DataVolume watch is added from a runnable that polls discovery until the kind is served. It starts on the first reconcile, the earliest point at which a new watch is started at once, rather than on the next tick, so a disk created just after a restart is not read as populated for a poll interval. The reader is installed only once that watch has synced within a bound: a reconcile context has no deadline, and a List on an informer that never syncs would park every WorkloadMonitor reconcile for good. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2df29639df |
fix(clickhouse, workloadmonitor): add the cluster domain to keeper hosts and stop a PVC nil dereference (#4608)
## What this PR does Two small bug fixes, one per commit. Both were first proposed by @officialasishkumar in #2334 and #2336. Those PRs went stale, and part of each was fixed on main in the meantime, so this carries only what is still broken. ClickHouse Keeper hosts rendered without a cluster domain, as `chk-<name>-keeper-cluster1-0-0.<namespace>.svc.` with nothing after `svc.`. The zookeeper block in `clickhouse.yaml` redeclared `$clusterDomain` from `.Values.clusterDomain`, which the chart never defines, and that shadowed the value read from `_cluster.cluster-domain` at the top of the template. Keeper is enabled by default, so every ClickHouse release got these hosts. The keeper name test set `clusterDomain` itself and hid the bug. It now sets only `_cluster.cluster-domain`, like the platform does, and fails without the fix. The WorkloadMonitor controller panicked on a PVC with no `storageClassName`, which is any PVC that takes the cluster default class. The check used `||` where it needed `&&`. controller-runtime recovers the panic, but the reconcile of that monitor fails on every retry and never reaches its status update. A PVC with an empty `storageClassName` also got a resource key with an empty class name instead of `default`. A new table test covers unset, empty and explicit classes. The duplicate `template` key in the ClickHouse backup CronJob from #2334 and the `Operational` status fix from #2336 are already on main, so they are not in this PR. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(clickhouse, workloadmonitor): ClickHouse Keeper node hostnames now include the cluster domain, and the WorkloadMonitor controller no longer panics on a PVC without a storage class. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Workload resource labels now use the default storage class when a PVC’s storage class is unset or empty, while continuing to reflect explicitly configured storage classes. * ClickHouse ZooKeeper hostnames now consistently use the configured cluster domain, or `cozy.local` when no domain is set. This helps hostnames align with the cluster’s domain configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
harshitrwt
|
ffb6564f8b |
fix(controller): drop stale bucket-class label from BucketClaim labels before applying validated value
maps.Copy(workload.Labels, bc.Labels) can bring in a workloads.cozystack.io/bucket-class label directly set on the BucketClaim, which would survive unmodified when spec.bucketClassName is empty or invalid, leaving the Workload reporting a class that does not match the source BucketClaim. Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
harshitrwt
|
f41a500b07 |
test(controller): cover invalid bucket class label in reconcileBucketClaimForMonitor
A BucketClassName longer than 63 chars (valid as a Kubernetes object name, up to 253 chars) is not a valid label value, so the bucket-class label is skipped with an Info log rather than failing the Workload write. This can happen with SeaweedFS pool class names (<namespace>-<pool>-lock) when the tenant and pool names are both long -- a missing label in that case does not mean no class was set. Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
harshitrwt
|
882929ff86 |
fix(controller): skip bucket-class label when BucketClassName is not a valid label value
Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
harshitrwt
|
0c37fff5f4 |
feat(controller): expose bucket class on BucketClaim Workloads
Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
Aleksei Sviridkin
|
8b8d4115fc |
fix(workloadmonitor): stop panicking on a PVC without a storage class
The storage class check used || where it needed &&, so a PVC with no storageClassName (one that takes the cluster default) was dereferenced as a nil pointer. controller-runtime recovers the panic, but the reconcile of that WorkloadMonitor fails on every retry and never reaches its status update. A PVC with an empty storageClassName produced a resource key with an empty class name instead of falling back to "default". Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
e510d49b3d |
feat(monitoring): re-render node agents when a monitoring label changes
The node fluent-bit builds its tenant routes from a lookup of namespace labels, and helm-controller upgrades a release only when its chart or values digest changes. A tenant turning monitoring on therefore got no route, and a removed one kept its route, until an unrelated change upgraded monitoring-agents. The reconciler digests the label across all namespaces and requests a forced upgrade of the release when the digest moves. It digests every labelled namespace rather than repeating the chart's selection, so it cannot drift from the chart; the cost is an occasional upgrade that renders the same config. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
e62f510faf |
chore(deps): move the Kubernetes libraries to v0.37.1
The root module's k8s.io/* move from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, its release for Kubernetes 1.37. Two things held them back: the apimachinery fork, which had to be rebased onto every minor, and the watch handler that from k8s.io/apiserver v0.36 panicked on the initial-events-end bookmark the aggregated storages sent to plain watches. Both are gone. The rest of the upgrades are what these releases require: cel-go v0.29.2, the etcd client v3.7.0, kube-openapi, structured-merge-diff and the x/ modules. code-generator moves with the libraries. Regenerating the strategy CRDs that embed core types adds the Kubernetes 1.37 fields and follows one removal: the alpha PodSpec field workloadRef became schedulingGroup upstream. Once the new CRD is applied, the apiserver prunes workloadRef from every stored strategy it serves, and the object's next write erases it from etcd. It only ever took effect behind an alpha feature gate. The gateway README and the tenantgateway CRD tests cite the apiextensions-apiserver list-type ratchet at the pinned version; it is byte-identical in v0.37.1, so only the version they name changes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## What this PR does This replaces the open Renovate PRs with one rollup. Go modules move to their newest releases that keep the Kubernetes libraries and controller-runtime where they are, the toolchain and builder images move to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers. Tests for the code the bumps touch come first, in their own commits, and pass on both the old and the new dependencies. Before the bumps: - The e2e sandbox image builds again: dl.min.io answers 410 for mc since MinIO archived it, so mc now comes from its last GitHub release with a pinned checksum. - Renovate stops offering tags of the apimachinery fork. They mirror upstream, so #2999 is a downgrade that drops our patch. - `make unit-tests` now runs the tests of the nested Go modules (kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1). They never ran in CI before. - New tests: JWT verification in the dashboard token proxy, the kube-ovn plunger metrics, cert-manager object idempotency in the tenant gateway, values-file layering in the operator, and the kubevirt CSI driver wrapper. - One real fix. The lineage webhook decoded admitted objects with encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5 compares numbers exactly and would emit a replace that rewrites such a field in the pod. The webhook now decodes with apimachinery's json. Held back on purpose, with the reasons in the commit messages: - Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork rebased onto release-1.37 first. - cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is built and tested with. - grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch release carries the fix yet. - kubevirt.io/api and the CDI API stay on what upstream csi-driver builds against. From CDI v1.60 it no longer compiles. - api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types are not forced onto Go 1.27. - go fix runs without embedlit (Go 1.27-only syntax, breaks backports to release branches on Go 1.26) and omitzero (would make metadata, spec and status required in the published OpenAPI schemas), and one errorsastype rewrite that did not compile is left out. govulncheck reports nothing reachable in any of the five modules, standard library included. Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805, #3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open. Not checked locally: the images that build upstream projects (kube-ovn, kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The image build jobs of this PR cover them. Closes #4487. ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note chore(deps): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a restore resource policy option for Velero strategies. * Added a test target to run Go tests across tracked modules. * **Bug Fixes** * Updated the end-to-end test environment to download and verify the MinIO client from its current release location. * Added coverage for token validation, webhook patching, controller behavior, and reconciliation. * **Maintenance** * Updated the Go toolchain, dependencies, and container image versions. * Refreshed pinned workflow actions and base image digests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
0b7dd23b99 |
fix(appdef): let one ApplicationDefinition own each application kind
Nothing enforced that spec.application.kind is unique, and each consumer that maps a kind back to a definition resolved a duplicate its own way: the chartRef reconciler let every definition rewrite every release of its kind, the lineage webhook and the CA projection reconciler took whichever came first in an unordered list, and cozystack-api registered one resource per definition. A second definition declaring a kind already in use could therefore repoint that kind's tenant releases at its own artifact. Add one ownership rule, the definition created first owns the kind with ties broken by name, and route every consumer through it. A definition that arrives later with a kind already in use no longer manages that kind's releases whatever it is named: the chartRef reconciler skips it with a Warning event and re-reconciles the next owner when the current one goes, cozystack-api registers the owner only and logs the rest, its restart hash moves when ownership moves, and the lineage and CA projection lookups resolve the owner. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
IvanHunters
|
1f9bad0643 |
fix(workloadmonitor): populate VM instance-profile label from the KubeVirt preference annotation
getWorkloadMetadata filled the workloads.cozystack.io/kubevirt-vmi-instance-profile label from the annotation kubevirt.io/cluster-instanceprofile-name, a key KubeVirt never stamps. KubeVirt records the cluster preference (which carries the guest OS profile) under kubevirt.io/cluster-preference-name, so the profile label was never emitted and a VM's instance profile never reached the billing metric. The sibling instance-type label worked only because its key is the real one. Read the correct annotation. The emitted label name is unchanged, so the downstream metric pipeline is untouched. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
626e1fe19d |
test(gateway): pin that a second reconcile leaves cert-manager objects alone
Idempotency was pinned only for the Gateway and the HTTP01 Issuer. A cert-manager type change that makes the rendered spec differ from the stored one would turn every reconcile into an update. Cover the DNS01 Issuer and wildcard Certificate and the per-listener HTTP01 ones. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5c857c9d9f |
test(kube-ovn): pin the plunger's OVN cluster metrics
The metrics had no tests. Pin the series written for a cluster and its members, and that dropping one cluster removes only its series. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
22f2d76ead |
test(controller): pin the invalid-annotation and kind-selector paths of the wait carry-over
The reconciler skips a disable-wait annotation it cannot parse, and it selects releases by application kind. Removing either left the suite green; each now has a test that fails without it. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
5f379577d5 |
fix(controller): carry the readiness-wait setting to existing releases
A HelmRelease created before its ApplicationDefinition carried release.cozystack.io/helm-install-disable-wait keeps waiting on readiness, because cozystack-api only reads that annotation while building a release and an existing release is not rebuilt until its Application is written through the apps API again. Where the wait is what broke, that is every existing instance of the kind at once, each looping upgrade and timeout, and rewriting them by hand is the operator's only way out. The definition already drives chartRef, valuesFrom and labels on releases that exist, so drive the wait from there too, in both directions: the API rebuilds the whole spec on every write and emits DisableWait=false once the annotation is gone, so a release left disabled would drift back anyway. An absent install or upgrade block behaves as DisableWait=false already and is only materialised when the definition asks to disable the wait, which keeps a release nothing asks to change from being rewritten. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1ed32aec58 |
fix(controller): roll cozystack-api when a release annotation changes
The ApplicationDefinition config hash was built from name and spec alone, so a definition whose whole change is a release.cozystack.io/* annotation left the hash untouched and cozystack-api kept running with the configuration it read at its last start-up. Those annotations are only ever read there, once, in buildResourceFromCRD, which makes the config-hash rollout the single mechanism that delivers a change in one: without it the annotation is inert until the api pod restarts for an unrelated reason, and an inert annotation is indistinguishable from one that was never delivered. Hash the release.cozystack.io/* annotations alongside the spec. Only that prefix, because kubectl and Helm rewrite annotations of their own on every apply and hashing those would roll the api Deployment on every reconcile. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
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
|
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 --> |
||
|
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
|
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> |
||
|
Aleksei Sviridkin
|
af19aa933c |
docs(security): record GRPCRoute in the route hostname policy set
The port-443 kinds set still excludes GRPCRoute, TCPRoute and UDPRoute. 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. TCPRoute and UDPRoute additionally carry no hostname and no admission rule gates them. The gateway README described HTTPS and TLS-passthrough listeners as carrying different kinds. They carry the same one, which is what cilium#45559 requires. The threat model and the self-assessment enumerate which route kinds the hostname policies cover, and both listed two. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9ea4a961ec |
fix(cacert): make the comments match the reconciler
Comments in the CA-extraction package described guards the code does not have. ManagedLabel: the anti-clobber decision is made by isOurProjection, from the Secret type and an owner reference back to the sentinel, not from the marker, which no decision keys on beyond the drift check. managedByCozystackLabel: the reconciler does mutate it. It deletes the label on a selectors-digest change, which is what hands the projection back to the lineage webhook for a fresh verdict. projectionSuffix: the projection name is kept clear of engine CA Secrets by a dot, and a release name cannot carry one, but the suffix an operator appends can and nothing validates it. A suffix ending in ".tenant-ca" is the only one that reaches a canonical name. TestProjectionNameCannotCollideWithADotFreeEngineSuffix: both its doc and its name claimed the property for any engine suffix while the loop samples only dot-free ones, which is the same overstatement one file over. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3d076099a2 |
docs(postgres): name the trust anchor as the tenant actually sees it
The README told a tenant to fetch <release>.tenant-ca, but the release name carries the postgres- prefix the application definition applies, so the object is postgres-<name>.tenant-ca. Following the old text returned NotFound. Also drop two comments promising a companion ValidatingAdmissionPolicy that pins sentinel writes to helm-controller. No such policy ships: the proposal for it was rejected because authorization by username duplicates RBAC in a more brittle form. The RBAC boundary is the whole control. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Timofei Larkin
|
95e4faf68e |
feat(workloadmonitor): bucket metrics endpoint override
The controller discovers where to query SeaweedFS bucket size metrics via the namespace.cozystack.io/monitoring label, which assumes the monitoring stack that scrapes SeaweedFS runs in the same cluster. Some deployments host SeaweedFS and its metrics in a separate cluster, so add a --seaweedfs-metrics-endpoint flag to cozystack-controller (and a matching cozystackController.seaweedfsMetricsEndpoint chart value): an explicit base URL of a Prometheus-compatible query API, with scheme, host, optional port and path prefix; /api/v1/query is appended. Basic auth may be embedded as URL userinfo. The flag is validated and normalized at startup, so a malformed URL fails the rollout rather than the queries. Empty keeps label-based discovery unchanged. Bucket sizes feed billing, so metrics failures no longer collapse to "no size": previously any query failure yielded an empty result map and the reconciler overwrote Workload resources with it, silently wiping the recorded sizes. The reconciler now retains the last known s3-storage-bytes and s3-physical-storage-bytes values, emits a BucketMetricsUnavailable warning event on the WorkloadMonitor, and returns the error from Reconcile so it lands in the reconcile error metrics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |