mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
eafc6423f3d90c97379110fc172dcebbd03cd6fc
6320
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrei Kvapil
|
eafc6423f3 |
fix(postgres): tolerate databases declared without roles
`databases.<name>.roles` and its `admin`/`readonly` lists are optional in
the schema, but the init script dereferenced `$d.roles.admin` for every
database. A database declared with only `extensions`, or with an empty
body, failed the whole render with a nil pointer error instead of simply
granting nothing.
Treat a missing `roles`, `admin` or `readonly` as an empty list, so such a
database reconciles the same way as `roles: {}`: no grants, every managed
role revoked.
Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
||
|
IvanHunters
|
ed9ddf9c3a |
fix(vm-instance-rd): disable helm readiness wait on VMInstance releases
The generated HelmRelease is health-assessed by helm-controller's kstatus poller, which returns InProgress for any object whose status.observedGeneration trails metadata.generation: a built-in pre-check in fluxcd/pkg/runtime/cel that runs before, and cannot be overridden by, healthCheckExprs. KubeVirt legitimately keeps a VirtualMachine's observedGeneration behind its generation while a template change is pending a restart, the VM is stopped, or a live-updatable change never converges (kubevirt/kubevirt#18700), so a healthy Running VM is read as permanently InProgress and the release never reaches Ready. Releases still carrying the pre-1.5.0 remediation.retries:-1 + rollback spec then revert the previous manifest, dropping just-applied changes such as opened ports. Disable the readiness wait so the release settles once the VirtualMachine is applied; VM health is observed on the VirtualMachine itself. This restores the pre-1.5.0 behaviour under which the VM was never a helm readiness gate. A helm-unittest suite guards that the annotation stays in place. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Andrei Kvapil
|
4c5f258b15 |
chore(api): regenerate deepcopy for the new ExternalIP value type
Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
d0bdf99781 |
test(cozy-lib): cover the externalIP claim helpers
Eight catalog charts thread the same value through these two helpers, so the contract belongs in one suite rather than eight: which annotation key is written, that a target with no entry gets no annotation at all rather than an empty map, and which malformed values are refused. The malformed-value cases are only reachable here. In an app chart the generated values schema rejects them first, so the helper's own refusals would never be exercised. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
2c15cae200 |
feat(vm-instance): let the external endpoint wear a reserved address
The claim annotation lands on the VM's own Service beside the cozy-proxy datapath annotations already there. The two are independent: whole-IP NAT behaves the same on a held address as on an auto-assigned one, so both methods take a reservation. A VM has one external endpoint, so attaching several addresses to one VM stays out of reach of this field — that needs the exposure half of the model, not the allocation half. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
047426631e |
feat(clickhouse): add external access and let it wear a reserved address
ClickHouse was the only managed database in the catalog a tenant could not reach from outside the cluster: the installation-wide Service had no way to become a LoadBalancer short of an operator editing the service template by hand. It now carries the same external boolean as the rest of the catalog, and the same externalIPs field. The Service is operator-owned, so this is the in-place type change mariadb and rabbitmq already make on theirs. It keeps its clusterIP, so the transition is one Kubernetes accepts. With external unset the rendered installation is unchanged. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
dca7a5fd0d |
feat(mariadb): let the external endpoint wear a reserved address
Which Service actually receives the address depends on the replica count: the operator publishes the cluster Service for a single replica and primaryService once there are more, and the chart already branches on exactly that. The claim annotation follows the same branch, so a tenant writes one entry either way. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
1cef6f27b4 |
feat(mongodb): let the sharded external endpoint wear a reserved address
Sharded mode publishes mongos through a chart-owned LoadBalancer Service, which takes the substrate's claim annotation directly. A replica set cannot: it is published as one LoadBalancer per pod, and the operator takes a single serviceAnnotations map for the whole replset, so one claim would be handed to every pod Service at once and the pins would collide. The chart refuses that combination with the reason rather than rendering something that cannot work. It is the discovery-engine case community#45 defers, and closing it needs an address per pod. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
8c0436dd05 |
feat(rabbitmq): let the external endpoint wear a reserved address
The chart owns no Service here — the operator does — but it already reaches into spec.service to flip the type, and the operator copies annotations from the same block onto the client Service that receives the load-balancer address. So the substrate's claim annotation gets to its consumer without the chart taking ownership of anything new. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
d316a1c0c1 |
feat(kafka): let the external endpoints wear reserved addresses
Strimzi publishes one bootstrap Service plus one Service per broker, and Kafka advertises every broker's own address to clients. Pinning the bootstrap address alone therefore does not make the cluster reachable at fixed addresses, so externalIPs accepts a broker-N target alongside bootstrap and each entry is rendered into the listener's own configuration block. Broker targets follow the replica count, so a claim for a broker that does not exist is refused as the typo it is rather than kept as a reservation for future growth. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
b21e8ccbcb |
feat(redis): let the external endpoint wear a reserved address
Threads externalIPs onto the LoadBalancer Service the chart renders for the Sentinel-elected master. Empty is the previous behaviour. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
acd715501f |
feat(postgres): let the external endpoint wear a reserved address
Adds externalIPs, a list of {claim, target} entries, and threads the
resulting annotation onto the LoadBalancer Service the chart already renders
for the primary. Empty is the previous behaviour, so nothing changes for an
existing release.
The list shape is deliberate even though the chart publishes a single endpoint
today: the same field lands on every catalog app in this series, and Kafka does
have several external endpoints. A target that the chart does not publish is
refused with the valid ones named, rather than being accepted and ignored.
Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
||
|
Andrei Kvapil
|
9d8fcf1db5 |
feat(cozy-lib): add helpers for pinning endpoints to reserved external IPs
The address substrate from cozystack/community#35 lets a tenant hold an IP across a workload rebuild, and a workload consumes that reservation through a single annotation on the LoadBalancer Service that should wear the address. Every catalog chart needs to emit the same annotation, but each engine takes Service annotations in a different place, so without a shared helper the key would be spelled out once per chart. The naming of the kinds and the API group is still open on #35, so the key is expected to move. Writing it down once keeps that a one-line change instead of a sweep across the catalog. Rendering fails on a malformed externalIPs value rather than dropping it: a typo in a claim name is the difference between the address a customer allow-listed and some other address, and an install-time error is far more visible than a Service that quietly came up on the wrong IP. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
b390271f76 |
feat(monitoring): scrape KubeVirt VMI metrics from virt-handler
KubeVirt exports the resource usage of every VirtualMachineInstance (kubevirt_vmi_*) from virt-handler, not from the VMI's own pod, so no per-application scrape object collects it. virt-operator would create a ServiceMonitor for it, but only when the monitorAccount ServiceAccount exists in the monitorNamespace, and Cozystack never creates that account, so no Monitoring application received VM metrics at all. Every Monitoring application now scrapes virt-handler through a VMPodScrape. A tenant Monitoring keeps only the series of its own namespace and of the sub-tenants below it, which is what the namespace.cozystack.io/monitoring label already promises for a sub-tenant without a Monitoring of its own; the root Monitoring keeps everything. The tenant chart already opens the network path for this (the per-tenant egress-virt-handler policy). Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Aleksei Sviridkin
|
e3ab40eb83 |
fix(virtual-machine): guard Kube-OVN IP lookup (#4013)
## What this PR does Guards the Kube-OVN `IP` lookup behind API discovery. The `vm-instance` chart now renders on clusters using another CNI instead of trying to query a CRD that is not installed. The Helm regression suite mocks the IP object through `kubernetesProvider` and pins both sides: with `kubeovn.io/v1/IP` advertised the annotations carry the mocked MAC and IP, without it they are absent even though the object exists, which is the branch the guard adds. helm-unittest's fake lookup never errors on an unserved kind, so only the second case distinguishes the guard; it fails on main and passes here. Open PRs #3991, #3978, #3931, and #3900 touch the same template. Their current hunks do not alter this lookup; #3900 adds secondary-interface annotations immediately after it, so it is the only nearby merge hunk to watch. ### Screenshots Not applicable; there is no 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: ### Release note ```release-note fix(virtual-machine): render VM instances on clusters where the Kube-OVN IP API is not installed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM instances now render correctly in environments without the Kube-OVN IP capability. * OVN IP lookups and MAC/IP annotations are only generated when the required Kube-OVN API is available. * MAC/IP annotations are omitted when the capability is unavailable, even if an IP object exists. * **Tests** * Added coverage for both supported and unsupported Kube-OVN IP discovery scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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 --> |
||
|
Aleksei Sviridkin
|
cecb4b11d3 |
feat(opensearch)!: issue HTTP and Dashboards certificates from cert-manager (#2682)
## What this PR does The OpenSearch HTTP API (9200) and Dashboards (5601) already served TLS, from the operator's own private CA and with a SAN built from `cluster.local` while the credentials Secret advertised the release on the platform domain, so nothing a client was handed could verify. Both certificates now come from a per-release cert-manager CA in the tenant namespace, and the CA reaches tenants as a key-free trust anchor. - `tls.issuer` selects who issues the HTTP server certificate, `cert-manager` or `operator`, and is named for the issuer because TLS is served under both. Unset resolves from `external` (cert-manager for externally published services, the operator for cluster-internal); an explicit value always wins. - Renders a self-contained cert-manager chain in the tenant namespace: a self-signed Issuer, a CA Certificate, a CA Issuer, and two leaf Certificates, one for the HTTP API and one for Dashboards, each with the SANs of its own Service and the external hostname when `external: true`. Leaf keys are written as PKCS8, since the JVM loader rejects the PKCS1/SEC1 encoding cert-manager emits by default, and the CA subject is pinned so the operator's signature check accepts the chain. - The CA Secret is `<release>.http-ca`. The dot separates the chart-chosen suffix from release names, so no other release can produce that name; the two leaf Secrets stay hyphenated because none of the chart's suffixes is a tail of another. The operator's own transport CA remains at `<release>-ca` and is not touched. - Transport mTLS (9300) stays operator-managed (`transport.generate: true`, `perNode: true`). The security plugin requires it and the chart cannot turn it off. - `hack/e2e-chainsaw/opensearch/` runs the move on a cluster: it installs the app on the operator's HTTP listener, removes `tls.issuer` so it resolves from `external`, and asserts the cert-manager chain, the CR dropping `http.generate`, the admin certificate reissued under the chart CA, the StatefulSet rolling onto the three subPath mounts, the tenant anchor, and the certificate the endpoint actually presents to a client holding that anchor. ### Sysctl carrier and single-node rolls The E2E suite for this PR was red because the chart set `spec.general.setVMMaxMapCount: true`, which makes the operator prepend a privileged `init-sysctl` container to every OpenSearch pod. Tenant namespaces run at the cluster's default PodSecurity level, baseline on Talos, so admission refused the container and the StatefulSet never produced a pod (#4131). The carrier for the sysctl is now on main: #4152 added a DaemonSet to the `opensearch-operator` package that raises `vm.max_map_count` to a floor of 262144 on every node, from a namespace the platform labels privileged. This branch is rebased onto that main and sets the flag to an explicit `false`, with a template comment saying why and a unit test pinning it, so a release no longer asks the operator for a container admission refuses. The value stays with the DaemonSet until the node contract carries it (#4144); the DaemonSet's own template comment, which said the chart still asks, now says it does not. Three more things came out of the same pass. The chart asked the operator to drain data nodes before every rolling restart, and with a single data node the drain never finishes: at v2.8.0 `PreparePodForDelete` excludes the node from allocation and then waits for it to hold no shards, which the only data node can never satisfy, so a one-replica release could not roll for a version bump or for the certificate move this PR relies on. The chart now drains only when a second data node exists, pinned both ways in the unit tests. The chart maps an empty `storageClass` to `replicated` at one replica, and the PR lane runs LINSTOR without DRBD and creates only `local`, so the E2E fixture takes its class from the `storageClass` value the harness passes with `--set-string` from `COZY_E2E_STORAGE_CLASS`, the way the vminstance fixtures do; on the nightly QEMU lane that still resolves to `replicated`. And the external Dashboards Service selected labels no pod carries. `pkg/builders/dashboards.go` at v2.8.0 stamps `opensearch.cluster.dashboards: <cluster name>` on the Deployment, its selector and the pod template, and nothing else; the chart asked for `opster.io/opensearch-cluster` together with `app.kubernetes.io/component: dashboards`, so the LoadBalancer came up with no endpoints and no error to show for it. It selects the label the operator sets now. ### securityadmin cross-trust, still blocked on the DNS base Chart-managed HTTP TLS previously broke the operator's securityadmin reconciliation: the admin certificate was signed by the transport CA while the HTTP listener trusted the cert-manager CA, so the two never verified against each other and users and roles silently never applied. This chart puts one CA under both sides, handing the HTTP CA to the operator through `spec.security.tls.http.caSecret` so that it signs the admin certificate with the CA the listener trusts. That closes the CA mismatch. The other half, the job resolving a name the operator built from `cluster.local` on a cluster that does not use it, was closed separately and is on main, so the security configuration applies once this lands. No operator bump is required; the API exists in the pinned version. That path is version-gated upstream at OpenSearch 2.0.0: below it the operator falls back to the transport CA and the mismatch returns with no error anywhere. The chart therefore refuses chart-managed HTTP TLS on `version: v1` with an explicit render-time failure that names the mechanism and offers the alternatives, rather than letting it fail silently at runtime. ### Trust anchor for tenants The HTTP server's private key is not exposed to tenants. The chart renders a `TenantProjection` sentinel that names `<release>.http-ca` as its source; the CA-extraction controller publishes a `ca.crt`-only copy as `<release>.tenant-ca`, and the ResourceDefinition selects it with the engine-agnostic `internal.cozystack.io/tenant-ca` label. The sentinel is gated on the same condition as the chain, so a plaintext release renders no sentinel instead of one stuck at `SourceNotFound`. Tenants get what they need to verify the server and nothing more. ## Known limitations These are stated rather than left to be discovered: 1. **securityadmin needed both trust and reachability, and both are closed now.** This branch put one CA under the admin certificate and the HTTP listener. The other half was the operator resolving `<svc>.<ns>.svc.<DNS_BASE>` with `DNS_BASE` left at the vendored `cluster.local`, so the job waited on a name that never resolved whichever CA had signed; the platform now renders that value from `networking.clusterDomain` and it is on main, which is what unblocks this branch. 2. **The e2e suite reaches the move, not the chart bump that triggers it.** The suite above arrives at the chart-managed chain by removing `tls.issuer` from a running release, which leaves exactly the values an upgraded release arrives with. The one step it cannot run is the one where the previous chart version wrote the CR, because there is no second chart version to install from in this tree. Everything downstream of that write is exercised on a cluster. 3. **A reissue does not reach a running pod.** Neither service reloads its certificate, and cert-manager reissues on any change to `external`, the tenant host or the cluster domain, so an ordinary edit leaves the pods serving material that no longer matches their own names with no error, event or condition to show for it. Completing such a change is manual. So is removing the CA private key cert-manager leaves behind when TLS is turned off again, since the platform runs with `enableCertificateOwnerRef: false` and deleting a `Certificate` does not delete its `Secret`. `packages/apps/opensearch/README.md` carries both, with the commands. ### Downstream repositories `packages/apps/opensearch/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_opensearch`; tracked in cozystack/terraform-provider-cozystack#41. ### Upgrade impact An existing `external: true` release with `tls.issuer` unset serves operator-issued HTTP TLS today and moves to chart-managed TLS on its first reconcile after this lands: the cluster rolls once and is re-anchored on a new per-release CA, so any client pinned to the operator's CA has to be repointed at the key-free `<release>.tenant-ca`. Setting `tls.issuer: operator` before the upgrade keeps that half as it is. Releases that are not external are untouched. Every existing release rolls once regardless of `tls.issuer` and `external`, because the pod template loses the operator's `init-sysctl` container; on a single-replica release that roll is the first one the operator can complete, since draining the only data node used to block every rolling restart. The published external-dns name changes on the same upgrade whatever `tls.issuer` says, because the Services are gated on `external` alone: they move from `<release>.<namespace>.<cluster-domain>`, an in-cluster name that never resolved outside the cluster, to `<release>.<tenant-host>` and `<release>-dashboards.<tenant-host>`. external-dns is configured upsert-only here, so the old record is not withdrawn and keeps pointing at the LoadBalancer until it is removed from the zone by hand. That cleanup is per zone rather than per release and nothing enumerates the records, so the README shows how to derive the list from the cluster. ### Release note ```release-note feat(opensearch)!: the OpenSearch HTTP API and Dashboards already served TLS, from the operator's own private CA and with a SAN built from cluster.local while the credentials Secret advertised the release on the platform domain; both certificates now come from a per-release cert-manager CA whose anchor the platform publishes into the tenant as <release>.tenant-ca. Any client that worked against the old endpoint was skipping verification, because no hostname it was handed could have matched that SAN. Existing external releases move from the operator's CA to the chart's on their first reconcile, and their published external-dns name moves from the in-cluster domain to <release>.<tenant-host> with the old record left in place; set tls.issuer: operator beforehand to opt out of the CA move. With dashboards.enabled the application name is limited to 41 characters, and to 32 when external is on as well; past either bound the chart fails the render and withholds every object in the release. A release already above a bound is not broken by the guard, it is told why it has not been upgrading: the Dashboards Service the operator builds from that name exceeds the 63-character limit for a DNS label, the operator returns that failure on every pass, and the reconcilers queued behind Dashboards, which are upgrade and restart and snapshot repository, have not run since the release was created. Set dashboards.enabled to false to drop the bound. The external Dashboards Service also selected labels the operator does not put on Dashboards pods, so it stood up with no endpoints; it selects opensearch.cluster.dashboards now. The drain of data nodes is now requested only with the data role on and more than two replicas, since at two the operator gives up on a full drain itself. The chart stops asking the operator for its privileged init-sysctl container, since the opensearch-operator package now sets vm.max_map_count from a DaemonSet, so every existing release rolls once on upgrade; single-replica releases no longer ask the operator to drain their only data node, which used to block every rolling restart ``` |
||
|
scooby87
|
080c649da0 |
fix(hack): avoid a SIGPIPE false-negative in the resourcesPreset check (#4204)
## Summary `hack/check-rd-presets.sh` intermittently fails the "Unit & controller tests" CI job with a false `resourcesPreset enum missing: <preset>` for a preset that is actually present — most visibly on `http-cache-rd`, and on PRs that have nothing to do with that package. ## Root cause The check tests each expected preset with `printf '%s\n' "$enums" | grep -Fqx -- "$want"`. When grep finds the preset it exits immediately and closes the pipe, so `printf` is killed by SIGPIPE (exit 141) while it still has data to write. The script runs under `set -o pipefail`, so that 141 becomes the pipeline's status and `if ! <pipeline>` records a present preset as missing. It is a race with the pipe buffer, so it fires intermittently and mostly on the larger enums (`http-cache-rd`'s schema carries two 47-entry enums). The CI log shows the tell: `check-rd-presets.sh: line 39: printf: write error: Broken pipe` immediately followed by `FAIL: ... resourcesPreset enum missing: s1.micro`. ## Fix Feed grep from a here-string instead of a pipe — no second process, no SIGPIPE, no pipefail interaction. Deterministic reproduction (target at the front of a 100k-line list, under `set -euo pipefail`): the `printf | grep -Fqx` form reports the target as missing; the here-string form finds it. After the change `bash hack/check-rd-presets.sh` passes green. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved preset validation to prevent false errors when configured presets are present. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
76c9ece4ab |
fix(opensearch): stop asking for a drain the cluster cannot perform
The chart asked the operator to drain data nodes before restarting them, whatever the replica count. Draining moves a node's shards to another data node and waits until none are left, and below three data nodes there is nowhere for them to go: with one the operator requeues forever and never deletes the pod, and with two it gives up on a full drain by itself, warning that some shards may not drain and then waiting for the system index primaries alone, which the surviving node cannot take while it holds their replicas. Such a release could not complete any rolling restart the operator drives: a version bump, a configuration change, or the move of the HTTP listener onto a cert-manager certificate. Drain only where a spare data node exists. Without draining, the operator pauses replica allocation for the restart and re-enables it once every pod runs the new revision, which is the path a small cluster can take. The predicate also reads the data role, since the operator counts data nodes over pools that carry it and a release with the role off has none. Every boundary is pinned in the unit tests. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
884f719b75 |
fix(opensearch): stop asking the operator for a privileged sysctl container
spec.general.setVMMaxMapCount makes the operator append a privileged init container to every OpenSearch pod, so that vm.max_map_count reaches the 262144 the store needs. Tenant namespaces run at the cluster's default PodSecurity level, and on Talos that is baseline, which refuses the container before a pod exists: the StatefulSet never produces one and the release never comes up. The opensearch-operator package now raises the sysctl on every node from a DaemonSet in its own namespace, which the platform labels privileged, so the pod no longer has to ask. The flag is set to an explicit false rather than dropped, and a unit test pins it, so the next reader sees a decision and not an omission. Carrying the value in the node contract would let the DaemonSet go; until then the DaemonSet is the carrier. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
19adffb515 |
test(opensearch): exercise the TLS rollout on a cluster
No suite created an OpenSearchCluster anywhere in the tree, so the whole class of failure where the chart renders correctly and the cluster then refuses the workload had no coverage at all. Rendering proves the manifests; only a cluster proves that securityadmin authenticates against the CA the chart handed the operator and that the security configuration actually applies. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f3c28e91c4 |
test(opensearch): pin the trust anchor, the name guard and the upgrade
Most of what this chart decides about TLS is invisible in a rendered manifest until a cluster disagrees with it. The upgrade case is the sharpest: a release that already runs operator-managed HTTP TLS must keep working when the chart takes the CA over, and nothing about the rendered output says whether it will. The negative assertions carry as much as the positive ones. The trust anchor must stay out of the resource map, since a name grant there would outlive the platform's verdict on it, and a null tls value must leave the chart rendering rather than aborting the whole release. Both are properties that fail silently and only a test states them. The bats guard covers what helm-unittest cannot see: which Secrets the opensearch-rd definition exposes, so a key-bearing one cannot be added to that surface without the guard going red. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
84daf750d9 |
fix(opensearch): render the credentials Secret without platform values
Reading the cluster values through index aborts the whole release when they are absent: index on an untyped nil is a hard render error, not a miss returning nil, so the default written after it never runs. The platform always injects them, so this surfaces only where the chart is rendered on its own, which is where the unit suites and a bare helm template both live. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ae935db0dc |
feat(opensearch): project the CA into the tenant as a client trust anchor
A client inside the tenant needs the anchor that signs the HTTP chain, and the only copy lives in the cert-manager CA Secret, which stores the CA private key alongside it. Granting that Secret by name in the resource map would convey the key with it, and would keep conveying whatever later occupies the name after the platform has withdrawn the verdict, so the one revocation channel the anchor has would be bypassed. Project it instead: the controller copies ca.crt alone into a key-free Secret the tenant roles already reach through tenantsecrets, gated on the label the lineage webhook stamps. The private key never leaves the release namespace, and withdrawing the label withdraws the anchor. The projection is gated on the same condition as the certificates it follows, since its source Secret exists only under the cert-manager issuer; ungated it would sit SourceNotFound on every release the operator signs for. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0ad5072d77 |
fix(opensearch): select dashboards pods by the label the operator sets
The external Dashboards Service selected opster.io/opensearch-cluster together with app.kubernetes.io/component. The operator labels Dashboards pods with opensearch.cluster.dashboards and nothing else, so the selector matched no pod and the LoadBalancer stood up with no endpoints and no error to show for it. That was survivable while nothing pointed at it. It is not now: the certificate carries a SAN for the Dashboards hostname and the parameter documentation describes it as a reachable external endpoint, so the chart promises something the Service cannot deliver. Select the label that exists, and pin it against the two that do not. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Arsolitt
|
85fa60a33f |
feat(opensearch)!: issue the HTTP API certificate from cert-manager
The HTTP listener and Dashboards ran on certificates the operator minted itself, so a client outside the cluster had no anchor it could obtain through the platform and no way to verify the endpoint it was handed. Issue the HTTP chain from cert-manager instead, and give the operator the same CA through spec.security.tls.http.caSecret so it signs the securityadmin admin certificate with it. Without that the admin certificate would keep the operator's own CA while the listener trusted cert-manager's, and securityadmin could no longer authenticate to apply users, roles and audit policy. tls.issuer names the authority rather than a switch, because the listener serves TLS under either value and only the signer changes. It is tri-state: an explicit value is obeyed, an absent one follows external, because a release nobody publishes has no third party to prove anything to. The cert-manager issuer needs OpenSearch 2.0.0 or newer, since the operator honours the HTTP CA only from that version and silently signs with its transport CA below it. An explicit choice under that floor fails the render and one resolved from external falls back to the operator, so asking for a verifiable certificate never quietly yields less than was asked, and turning on external access never breaks an upgrade. BREAKING CHANGE: an externally published release on OpenSearch 2.0.0 or newer serves the HTTP API from a cert-manager CA rather than the operator's own. A client pinned to the operator CA must move to the trust anchor the platform projects into the tenant. BREAKING CHANGE: the external-dns name of an externally published release moves from <release>.<namespace>.<cluster-domain> to <release>.<tenant-host>, and the Dashboards name with it. This half is gated on external alone and does not depend on the issuer. external-dns runs upsert-only here, so the old record stays in the zone until it is removed by hand. BREAKING CHANGE: with dashboards enabled the application name is limited to 41 characters, and to 32 when external access is also on. A release above either limit now fails to render rather than converging. Such a release was already stalled: the Dashboards Service built from its name exceeds the 63-character DNS label limit, and the operator returns that failure ahead of its upgrade, restart and snapshot-repository reconcilers, so it has not upgraded or rolled since it was created. Set dashboards.enabled to false to converge again, or recreate the application under a shorter name. Assisted-by: LLM Signed-off-by: Arsolitt <arsolitt@gmail.com> [lexfrei: reworded subject and body for the issuer field rename] Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
scooby87
|
f7cb9e7854 |
fix(postgres): follow-up review remarks from #3954 (KEDA autoscaling) (#4188)
Follow-up to #3954 (postgres read-replica autoscaling via KEDA), closing the non-blocking review remarks tracked in #4187. Each change is small, isolated, and covered by a red→green test where it is a behaviour change. ## Changes - **fix(postgres): pin `job="kube-state-metrics"` on the autoscaler read-load query.** The trigger query joined on `kube_pod_labels` without the job pin the sibling `DatabaseAutoscaler*` alerts already carry; a duplicate `kube_pod_labels` series from another exporter would multiply the join. Pinned on every selector; unittest asserts it in both the base and lag-brake query shapes. - **fix(cozy-lib): reject a negative `minReplicaCount` in `cozy-lib.keda.scaledObject`.** Only `max >= min` was checked, so a mixed-sign pair (`min -5, max -3`) passed and rendered a nonsensical floor. Now failed closed before the ordering check, with a unit case. Also added coverage for the previously-untested `annotations` parameter. - **docs(postgres): correct the KEDA-missing guard rationale and note the gradual client-side rollout.** The guard comment still described the replaced drop-instances design; corrected to the seed design (a skipped ScaledObject leaves a fixed-size cluster with no HPA, not a collapse to 1). The cozyrds apply-strategy annotation now notes that the client-side switch rolls out per-app as each Application is next written through the aggregated API, not fleet-wide at upgrade. ## Testing `helm unittest` green: postgres 66/66, cozy-lib-tests 45/45, keda 12/12. Both behaviour changes are mutation-verified (reverting the fix reddens the suite). No generated artifacts affected (no `values.yaml`/`values.schema.json`/`Chart.yaml`/`README.md` changes). ## Release note ```release-note NONE ``` Closes #4187 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved PostgreSQL autoscaling metrics by ensuring pod data is sourced from the correct metrics exporter, preventing duplicate data from distorting scaling decisions. - Added validation to reject negative minimum replica counts. - Preserved caller-provided annotations on generated autoscaling resources. - **Documentation** - Clarified PostgreSQL autoscaling fallback behavior when the autoscaling controller is unavailable. - Documented the gradual transition of existing PostgreSQL resources to the updated apply behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ae5b626c8f |
docs(gateway): describe the passthrough listeners and what they shed
The chart README gains the native-port field, the three limitations it carries and where each comes from, the one that is a phase-1 narrowing rather than a consequence of the Cilium pin, and the three ways forward for anything that depended on a terminate listener the change takes away. The withdrawal is stated as what it is, a change of serving path rather than the removal of something idle, because on the pinned version both filter chains carry the SNI today and nothing on the objects says which one answers. The layers section is brought back in step with the listeners: which label each listener class selects on, that the attach set is the whole inheriting tenant subtree rather than the tenant plus the admin list, and the route conditions an operator reads, including the one that says the sectionName itself names nothing. The platform values comment on gateway.enabled said every exposed service renders an HTTPRoute, which the chart value the withdrawal reads contradicts. It now separates them: the services whose TLS the Gateway terminates get an HTTPRoute, while api, vm-exportproxy and cdi-uploadproxy get a TLSRoute pinned to their passthrough listener, one each from the packages that own those services. The overlap paragraph no longer describes an HTTPRoute wildcard as keeping its terminate listener. Such a claim is refused under HTTP-01, which issues no wildcard certificate, and the paragraph says so and what the render used to do with it. The route-condition list gains that shape and the port shapes an HTTPRoute now reaches, and the per-app naming bullets carry the hash suffix the rendered names have. Two sentences that were false against the code are corrected: a TLSRoute on a wildcard entry itself leaves the names beneath it their terminate listeners, and an unusable apex is refused by the containment rule at admission before the controller's own check is reached. The edge section says what happens to the entries the HTTP-01 pass wrote once the mode moves. The NoMatchingParent sentence now separates the two kinds: an HTTPRoute pinned to an absent passthrough section is refused whatever answers its hostname, while a TLSRoute is judged where a passthrough listener answers it. A sectionName outside the passthrough prefix is written Accepted like any attached route rather than left without a verdict, and the text says so; the port-80 listener's refusal joins the NotAllowedByListeners shapes and the name-and-port disagreement the NoMatchingParent ones. The attachedNamespaces comment described the list as reaching every listener, which the native-port ones do not: they select on kubernetes.io/metadata.name and name the publishing tenant alone, so a namespace listed there cannot attach to a database port. It also credited the wrong label with keeping the list honest; what a listed entry gets is the namespace.cozystack.io/gateway label, and what keeps a tenant from stamping it on itself is that no tenant role carries patch on Namespace. The class reference is brought along. Its certificate-mode table promised one Certificate per published hostname under http01, which a hostname served by a passthrough listener no longer earns, and its edge table said every attached TLSRoute is refused, where only a route declaring hostnames is and the rest have this controller's entry removed instead. The refusal-shape list is corrected against the code, without a count of its own so the next correction does not have to move one. A route the plain-HTTP listener turns away on its namespace selector was listed under NoMatchingListenerHostname as well as under NotAllowedByListeners; the controller reports the second. The port shape standing beside it swept port 80 back in by naming every port but 443, where a route selecting the plain-HTTP listener is answered by the namespace shapes and never reaches the port refusal at all. And a TLSRoute pinned to an absent section is said to carry NoMatchingListenerHostname where no passthrough listener answers its hostname, which holds only while no HTTPRoute claims that hostname: with one there a terminate listener stands on the name, and the route is judged like any other attached route. The wildcard shape gains the same qualifier. A wildcard claim is refused because no terminate listener can carry a certificate HTTP-01 will not issue, so the refusal reaches a route that wanted a terminate listener and not one that asked for the plain-HTTP listener, which carries no hostname and serves the claim there. The Helm merge paragraph named the two-way path as what happens to a custom resource Helm has no Go type for. That path is one of two in createPatch, the version deciding which runs, and the version comes from helm-controller rather than from this tree, so the claim was not checkable where it was written. It now names both and says the outcome it rests on holds either way. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
04d6659bb6 |
test(e2e): cover the passthrough withdrawal in the gateway suite
Three phases on a tenant that owns its Gateway. The shipped passthrough entries render with nothing routed to them and an HTTPRoute on api.<apex> keeps its terminate listener, its certificate and Accepted=True, which is the state a withdrawal keyed on the declaration alone would take away. Attaching a TLSRoute to that listener, pointed at a Service in the tenant's own namespace so it resolves, 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 rather than two: withdrawing on demand is what dates the earlier assertion to settled state instead of to a withdrawal that had not arrived yet. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
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
|
7c739a14fa |
fix(cdi): omit the CDI clone strategy override when unset (#4138)
## What this PR does
`cdi-cr.yaml` rendered `cloneStrategyOverride` unconditionally, so there
was no way to let CDI pick a clone strategy per StorageProfile. The CRD
enum is `copy`/`snapshot`/`csi-clone` with no empty member, so neither
obvious workaround got there: an empty value rendered
`cloneStrategyOverride: ""`, which that enum does not accept, and a null
one rendered a bare key. Both checked against `main` with `helm
template` rather than assumed.
Now the key is guarded on a non-empty value, same as
`uploadProxyURLOverride` just below it in the same file. With the field
absent, CDI reads the target StorageClass's StorageProfile and uses its
clone strategy.
Default stays `csi-clone`. Rendered at the default and diffed against
`main`, the output is byte-identical, so nothing already deployed moves.
### Tests
`tests/clone_strategy_test.yaml` pins both directions: absent when the
value is empty, absent when it is null, `csi-clone` on the chart
default, `snapshot` through unchanged. It sits alongside the existing
`importer-resources` and `standalone_render_test` suites, with its own
file and suite name and no shared test names; `make test` runs all
three.
I checked the suite can fail rather than assuming it works. Reverting
the guard reddens the two absent-field cases, deleting the field
outright reddens the default and passthrough cases, hardcoding the value
reddens the passthrough case, and rendering a bare key reddens all four.
The two sibling suites stay green through every one of those, so the
reds are attributable to this suite.
The null case is pinned as an input rather than argued from `with`
treating null and empty alike. It cannot pass vacuously: flipping its
assertion to expect the default returns `unknown path`, not a
`csi-clone` mismatch, which is what shows the null actually reaches the
template instead of the assertion passing on a value that never arrived.
### Known gaps
`copy` is accepted by the enum but no test drives it, so the suite does
not pin that it reaches the CR. Concretely: wrapping the guard in `{{-
if ne . "copy" }}`, which silently drops that one strategy, still passes
all nine tests. Named here rather than fixed so the boundary is visible
to the next reader instead of being rediscovered.
### Scope
Context: https://github.com/cozystack/cozystack/issues/2908. This is the
first of the three directions that issue lists, so it does not close it
and carries no `Closes`. The default is deliberately left at
`csi-clone`, since whether that is the right default is the second
direction and a separate decision. The third, `vm-disk` warning on a
cross-StorageClass request, is untouched. An operator who has not set
`cloneStrategyOverride: ""` sees the reported symptom exactly as before.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff. It is one guarded key in a
`packages/system` chart, a values comment, and a test suite: no package
added or renamed, no `values.schema.json` (this chart has none), no
`ApplicationDefinition`, no namespace, nothing under `hack/`, no
platform values key. The `ccp` entry covers `hack/package.mk` and
make-target behaviour, and this touches neither. Nothing in the map
reaches it.
- [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(cdi): `cloneStrategyOverride` in the `kubevirt-cdi` package can now be left empty, which omits the field and lets CDI pick a clone strategy per StorageProfile. The default is unchanged at `csi-clone`.
```
|
||
|
Aleksei Sviridkin
|
4b6b1dc2da |
fix(vm-instance): let complete resources replace the instance type (#4109)
## What this PR does Closes #2909. The matcher keyed off `instanceType`, the domain sizing keyed off `resources`, and neither side looked at the other, so the chart could render both. KubeVirt refuses that, and the two halves of its rule are different shapes: `validateCPU` conflicts on `domain.cpu.cores` and `domain.cpu.sockets` only when they are non-zero, while `validateMemory` conflicts on `domain.resources.requests.memory` whenever the key is present. `values.yaml` ships `instanceType: "u1.medium"`, so clearing the field in the dashboard just falls back to that default. Fill in cpu, sockets and memory after clearing it and you get a VM the webhook rejects, naming fields the form gives you no way to clear. Resources that size the VM completely now suppress the matcher and size it directly. `packages/apps/kubernetes-nodes` already does this for its worker VMs. Beside an instance type, sizing that is only partial stops the render, naming what sized the VM and what did not; with no instance type in play there is nothing to conflict with, so that half renders as it does today. Letting resources win unconditionally would silently drop the instance type for a VM that sets memory alone, which is a different bug in the same place. Completeness is decided from what `domainResources` emitted, never from which keys the user set, and all three fields go through that one reading. The two readings come apart in more than one place. `resources.cpu` on its own emits nothing, so it conflicts with nothing and goes on rendering a plain instancetype-sized VM. cpu and sockets pass through `int64`, which yields 0 for a quantity the schema accepts, so `cpu: 500m` beside a whole sockets and a memory would read as complete from the values, drop the matcher, and hand KubeVirt a `cores: 0` the user never asked for. And a `resources` block nulled outright leaves `.Values.resources` nil rather than the chart's `{}`, so a raw field read is a render-time nil pointer where the emitted object is merely empty. The existence check and the pre-upgrade update hook read the same effective value. Keying the `lookup` on the raw one would fail a resource-sized render over an instance type it never references, which is the same confusing error this fixes. Reading `.Values.instanceType` in the hook would see the leftover value, skip the removal patch, and leave `spec.instancetype.revisionName` on a VM Helm had just given explicit domain resources. ### What changes in the rendered output | `instanceType` | `resources` | Before | After | | --- | --- | --- | --- | | set | unset | matcher only | unchanged | | unset | cpu, sockets and memory | domain cpu and resources | unchanged | | set | cpu, sockets and memory | matcher plus domain sizing, refused by KubeVirt | domain sizing, no matcher | | set | any subset with memory, or cpu with sockets | matcher plus a conflicting domain field, refused by KubeVirt | render stops, message names both sides | | set | cpu alone, or sockets alone | matcher only | unchanged | | set | cpu and sockets that both truncate to zero | matcher plus `cores: 0` | unchanged | | unset | anything short of all three | already refused by the existing check | unchanged | The rows that stay unchanged are deliberate. `virtual-machine.domainResources` writes `domain.cpu` only when cpu and sockets arrive together, so cpu alone emits no override at all. A `cores: 0` beside a matcher is not a conflict either, because `validateCPU` guards on non-zero, so KubeVirt admits it and the instance type sizes the VM. Both shapes render today and go on rendering. `cpuModel` is left alone for a related reason: it writes `domain.cpu.model`, which KubeVirt flags only when the instance type declares a model of its own, and no instance type shipped in `packages/system/kubevirt-instancetypes` declares one. ### Why no working release breaks Every shape that now fails at render was already being rejected by the KubeVirt validating webhook at admission, so it could not have produced a running VM in the first place. The failure moves from an opaque admission error to a render error that names the fields. Nothing that worked stops working. The unchanged rows above are measured. Each non-failing shape in the table, plus firmware on and off, an explicitly nulled `resources` block and the three update-hook branches, was rendered against a mocked cluster on both sides and compared document by document. Across 14 value shapes and 78 rendered documents, 77 come out byte-identical. The one that differs is the row the table marks as changed, which is also what the harness was checked against first, so it is known to be able to fail. ### Live-VM impact This changes a rendered VM spec, so an upgrade can resize or restart a running VM. Adding a complete `resources` block to a VM currently sized by an instance type drops its matcher, and the pre-upgrade hook patches `spec.instancetype` off the live object and writes the domain resources in its place. Cozystack runs KubeVirt with `vmRolloutStrategy: LiveUpdate` (`packages/system/kubevirt/templates/kubevirt-cr.yaml:125`), so KubeVirt applies what it can to the running VMI and marks the rest `RestartRequired`; I have not measured which half a given resize lands in, so plan the upgrade as if it restarts. Dropping the matcher drops the whole instance type, sizing and everything else it carries. Of the 69 shipped cluster instancetypes, 36 declare `dedicatedCPUPlacement`, 36 declare `hugepages` and 26 declare both. 20 declare NUMA passthrough, 8 declare `realtime`. A VM moved from `rt1.medium` to explicit resources loses those characteristics along with the type. Before this change that combination was rejected outright, so the VM never existed; now it is admitted, and the field help says so. ### Tests `tests/instancetype_resources_test.yaml` covers matcher only, resources only, both complete, each failing partial shape, the shapes that emit nothing, both cpu and sockets truncating to zero, `cpuModel` beside a matcher and beside complete resources, a stale instance type with and without complete resources, an explicit `instanceType: null`, an explicitly nulled `resources` block, and every branch of the update hook's instancetype selector, in both directions plus the two no-op cases. 24 cases in this package, 2127 across `make helm-unit-tests`, all green. Every case pinning new behaviour was checked against a reverted template to confirm it goes red; the rest pin behaviour this change deliberately leaves alone. A ten-mutation ladder over the helper, `vm.yaml` and the update hook leaves no survivor. The memory branch took the most work. Swapping its emitted read for a raw `.Values.resources.memory` looks like an equivalent mutant, since `domainResources` emits `resources.requests.memory` under the same truthiness condition. It is not: a `resources` block nulled outright leaves `.Values.resources` nil, and the raw read dereferences it. The case pinning that shape kills the mutant. The update-hook cases assert the `kubectl patch` command the Job will run, which is as far as a render-level test reaches. They do not show that KubeVirt accepts the resulting object, that helm-controller converges on a live VM whose `spec.instancetype` also carries a `revisionName` written by virt-controller, or which half of a resize lands as a live update rather than `RestartRequired`. That needs a cluster. The `dig` calls survive the shapes that reach them. `values.schema.json` rejects a non-object `resources` before render; Helm coalescing turns a nulled one into a nil `.Values.resources`, which digs to an empty object; and for any other non-map intermediate, current Helm turns the underlying sprig type assertion into a render error rather than a panic. ### Interaction with the firmware block `firmware` (UEFI/BIOS selection, Secure Boot, persistent EFI NVRAM) landed in this chart in the meantime, and it does not touch the sizing question. `domainResources` does not read it, so it takes no part in the completeness decision. KubeVirt's instancetype spec carries neither firmware nor `domain.features`, so nothing an instance type declares can conflict with what the firmware block renders. The boot mode a VM inherits when the block is empty comes from the preference, and the preference is not what gets dropped when complete resources replace the matcher. Both firmware states are covered on each side by the byte-identity comparison above. `README.md`, `values.schema.json`, `api/apps/v1alpha1/vminstance/types.go` and the ApplicationDefinition all come out of `make generate` in the package, not out of a hand merge. Re-running it on the result produces no further diff, and the root `make generate` reports no drift on top of it. ### On the ticket's second proposal #2909 also asks whether the non-empty `u1.medium` default belongs in the chart at all. That requirement is met, and not by removing the default. The default was a problem because a schema-generated form gave no way to say "no instance type": clearing the field falls back to `u1.medium`. Filling cpu, sockets and memory now says it directly, so the obstacle the ticket names is gone while the default stays. Dropping the default on top of that would change the rendered spec of every release that never set the field, with nothing left to gain. ### Named improvements, not done here The failure message names the fields that did not size the VM, computed from what was emitted, so for `cpu: 4` with no `sockets` it names `resources.cpu` even though the user set it. The message states the pair rule that explains this, but naming the actually-missing field first would read better. The two tests that pin `domain.cpu.cores: 0` document the pre-existing `int64` truncation in prose rather than through a `FIXME` reference, so a later reader has to read the comment to tell a contract from a bug we chose not to fix. Both are wording; neither changes behaviour. `resources.cpu`, `sockets` and `memory` are checked for shape and not for value, so `memory: "0"` and `-1Gi` pass the schema and fail further down. Both forms fail before and after this change and only the place of the failure moves, so that belongs with [#4104](https://github.com/cozystack/cozystack/issues/4104) rather than here. The new template comments run to about 46 lines against about 28 lines of logic. All of it is why rather than what and all of it is accurate, but the helper's third paragraph restates the commit body, and two sentences would carry that weight. The seven-line block in `vm.yaml` is the one worth keeping: swapping that gate to the effective value survives the entire suite, so what the block records is an equivalence no test can pin, and trimming it would delete the only record of it. The `instanceType` description runs to about 370 characters and reaches the dashboard as tooltip help, through `values.schema.json` and the ApplicationDefinition. That is long for a tooltip. It is accurate as it stands, and rewriting accurate text for length is a fresh unmeasured claim in the surface with the most readers, so it is not done here: the place for the detail about what a dropped instance type takes with it is the chart README, leaving the field help carrying the precedence rule alone. Choosing a `cx1.*` type and then giving it complete resources drops what the type guaranteed along with the type, and nothing says so at runtime. That is a consequence of the precedence rule rather than a defect in it, and the field help states it. A runtime signal would need the chart to compare what the type declares against what `resources` sets, which is its own task. Defaulting `sockets` to 1 would let a user size a VM from cpu and memory alone and would remove the commonest way to land in the partial case. It is more than the ticket asked for and it changes the rendered spec of stored releases that set cpu and memory without sockets, so it needs its own call on compatibility. ### Not changed Two pre-existing quirks stay as they are, because both reach past the conflict this fixes. `resources` fields are read for truthiness, so an explicit `0` reads as unset, in `domainResources` and in the existing "Either instanceType or resources" check alike. And with no instance type in play, a fractional `resources.cpu` still truncates to `cores: 0` and renders a VM sized by nothing, with no error. The new guard covers only the half where a matcher is present. The tests pin both as they stand, which is what keeps working releases rendering. ### Screenshots No dashboard code is touched. The only user-visible surface is two field descriptions the dashboard renders as help text, regenerated into `values.schema.json` and the ApplicationDefinition. `instanceType` now reads "Ignored when `resources` sizes the VM, which needs whole-number cpu and sockets plus memory; the whole type is dropped then, so any dedicated CPU placement, hugepages, NUMA or realtime settings it carries go with it. While this field is set, a `resources` block that sizes only part of the VM is rejected outright instead of ignored, so complete it or clear it." `resources` states the same rule from its own side. ### Downstream repositories - [ ] 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: I walked the trigger map against the diff and no listed trigger fires: no package added, renamed or removed; no `values.schema.json` field added, renamed or removed; no default changed; nothing under `hack/` moved; `templates/service.yaml` and its `wholeIP` and `allowICMP` annotations untouched. Two repositories do carry prose this makes incomplete. I left every box empty rather than push follow-ups nobody agreed to, so a maintainer decides. `cozystack/website`: `content/en/docs/next/virtualization/resources.md` opens with "Each virtual machine has these two configuration settings" and lists only `instanceType` and `instanceProfile`. The page is hand-written, so the release-time bot that regenerates app reference pages from `README.md` will not reach it. `cozystack/terraform-provider-cozystack`: `internal/provider/vminstance_schema.go` describes `instance_type` as "Virtual machine instance type." Its `kubernetes_schema.go` and `kubernetes_nodes_schema.go` already spell out the equivalent precedence rule for their own charts. No schema field or default changed here, so its new-field test guard has nothing to catch. ### Release note ```release-note fix(vm-instance): a complete `resources` block (whole-number cpu and sockets, plus memory) now sizes the VM directly and suppresses the `instanceType` matcher, instead of rendering both and having KubeVirt refuse the VM. Beside an `instanceType`, a `resources` block that sizes the VM only in part now stops the render with a message naming both sides, where it previously produced a VM the admission webhook rejected. Note that suppressing the matcher drops the whole instance type, including any dedicated CPU placement, hugepages, NUMA or realtime settings it carries. A release whose values already carry both converts on the next platform upgrade rather than at a moment the operator picks: the pre-upgrade hook strips `spec.instancetype` off the live VM and writes the domain sizing in its place, so plan that upgrade as if the VM restarts. ``` |
||
|
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 --> |
||
|
Alexey Artamonov
|
c2e7c86ff0 |
fix(hack): avoid a SIGPIPE false-negative in the resourcesPreset check
check-rd-presets.sh tests each expected preset with `printf '%s\n' "$enums" | grep -Fqx -- "$want"`. When grep matches it exits immediately and closes the pipe, so printf is killed by SIGPIPE (exit 141) while it still has data to write. Under `set -o pipefail` that 141 propagates as the pipeline's status, so `if ! <pipeline>` reads a preset that IS present as missing. It is a race with the pipe buffer, so it fires intermittently and mostly on the larger enums -- seen in CI on http-cache-rd, whose schema carries two 47-entry enums -- failing the "Unit & controller tests" job for unrelated PRs. Feed grep from a here-string instead of a pipe: no second process, no SIGPIPE, no pipefail interaction. Deterministic repro of the old behaviour (target at the front of a 100k-line list): the pipe form reports "missing", the here-string form finds it. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
9033d41bc0 |
test(postgres): assert the job pin on each autoscaler selector, not once for the query
The pin check matched job="kube-state-metrics" anywhere in the trigger query, so one pinned selector satisfied it no matter what the other three did: unpinning $joinReplica, $idleFloor or $joinCluster individually left the suite green while the join went back to matching any exporter's series. Anchor one assertion per selector on the label that identifies its site so a single dropped pin fails. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
0d8b249b15 |
fix(cozy-lib): guard fractional-negative minReplicaCount and cover the pause-merge collision
The negative-floor guard read $min, already int .minReplicaCount, and int truncates toward zero, so a fractional negative such as -0.5 became 0 and slipped past into exactly the scale-to-zero-capable ScaledObject the type check above exists to prevent. Test the untruncated value instead. The annotations test only set an extra key with no pause, so it exercised the pass-through branch and never the merge where the pause pair is the destination and must win a key collision. Pass a colliding paused-scale-in="false" alongside a distinct key and assert the pause value overrides it on the rendered object. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
e89617d754 |
fix(opensearch-operator): align DNS_BASE with the platform cluster domain (#4185)
## What this PR does The opensearch-operator resolves every in-cluster name it dials as `<svc>.<ns>.svc.<DNS_BASE>`, and nothing set `DNS_BASE`, so it stayed on the vendored chart default `cluster.local` while the platform runs on a configurable domain. On a cluster that is not on `cluster.local` the securityconfig bootstrap job waits forever on a name that never resolves, the security configuration is never applied, and OpenSearch nodes stay unready. That is issue #4073. The platform now carries the domain to the operator in the `cozystack-values` Secret, under the subchart key the vendored chart already reads: ```yaml opensearch-operator: manager: dnsBase: "cozy.local" ``` `charts/opensearch-operator` reads that as `.Values.manager.dnsBase` and hands it to the manager as `DNS_BASE`, so nothing under `charts/` changes and the package needs no patch. The value renders from `networking.clusterDomain` rather than a literal, so a cluster that keeps `cluster.local` keeps working, and a package rendered without the Secret keeps the upstream `cluster.local` default. This is the route the KEDA cluster domain already travels through the same Secret. The whole change is one key in `packages/core/platform/templates/apps.yaml` plus two test suites: one pinning that `networking.clusterDomain` reaches the Secret under that key, one pinning that the key reaches the Deployment env in the package. ### Why this is not the `global` route Earlier revisions of this branch, and #3357 before it, emitted the domain as `global.clusterDomain` and patched the vendored template to read it. `global` is the one key Helm copies into every subchart, which is why it looked right. That is also the problem. The `cozystack-values` Secret is `valuesFrom` on every HelmRelease the operator builds unless the PackageSource opts out with `operator.cozystack.io/skip-cozystack-values` (`internal/operator/package_reconciler.go:265-273`), so a new key under `global` lands in the values of nearly every release on the cluster. One of them broke on it. The vendored smee chart in `packages/system/bootbox` gates on the truthiness of the whole `global` map and then indexes `global.rbac`, so a single unrelated key under `global` ends its render. This still reproduces on `main`: ```console $ helm template bootbox packages/system/bootbox # exit 0 $ helm template bootbox packages/system/bootbox --set global.clusterDomain=cozy.local Error: cozy-bootbox/charts/smee/templates/role.yaml:4:32 executing "cozy-bootbox/charts/smee/templates/role.yaml" at <.Values.global.rbac.type>: nil pointer evaluating interface {}.type ``` The subchart key reaches the operator with no patch, no change to any other release, and nothing to harden in bootbox. I measured that rather than reasoning about it: rendering all 170 charts under `packages/*/*` plain and with `--set opensearch-operator.manager.dnsBase=cozy.local`, exactly one chart's output changes, opensearch-operator, by the one `DNS_BASE` line this PR is about. Nine other charts differ between the two runs by exactly as much as two identical runs of the same command differ, which is the random material Helm regenerates per render, and thirty-five fail in both modes for reasons unrelated to the flag, with byte-identical errors either way. The smee hardening is still worth having, because the next key anyone puts under `global` hits the same gate, and that gate is still on the default branch of tinkerbell/charts. It is not carried here. It is available as a separate change against its own package. ### On having more than one way to say the cluster domain Review feedback on the earlier revision asked for the opposite of this: fold the `keda:` block into `global.clusterDomain` so the platform has one answer rather than two added a fortnight apart for the same reason. The instinct is right and I am not doing it here, because `global` is the channel that carries the cost. A key under it lands in the values of nearly every release, one vendored chart already ends its render on it, and the next chart with the same gate breaks on the day someone adds a second key. The subchart key has no reach: the chart that declares it reads it and everything else ignores it, which is how the KEDA domain already travels. That does leave the repo with several ways to tell a chart its domain, and that complaint stands. What it avoids is a shared channel whose blast radius grows with every new user of it. Whether the platform should grow one general mechanism here, and which one, is worth settling on its own rather than inside a bug fix. ### Why the stuck Job is a manual step and not a migration The release note asks operators to delete a wedged `<release>-securityconfig-update` Job once. On an existing OpenSearch installation that Job is already stuck, so this is a step for everyone the fix is for, and `packages/core/platform/images/migrations/` is the mechanism built for exactly that. It is not in this PR because the migration deletes Jobs in tenant namespaces, which is a wider grant and a wider blast radius than the one key this change adds, and the fix works without it. If it should ride the same release, it is a separate commit and I will add it. ### Upgrade behaviour on a running cluster Read from the operator source at v2.8.0. `DNS_BASE` is read on every call through `helpers.ClusterDnsBase` (`pkg/helpers/constants.go:35-41`), so the manager restart is the whole switch. On a cluster that already runs OpenSearch clusters: - Certificates are not re-issued. Transport and http certs are generated only when their Secret is missing (`pkg/reconcilers/tls.go:272-295`, `490-518`), per-node transport certs only for a pod with no entry in the Secret (`tls.go:373-377`), the admin cert only when missing, expiring within five days or not signed by the CA (`tls.go:176-211`). Existing SANs keep the old suffix. Nothing in-cluster verifies them: the securityadmin job runs `curl -k` and `securityadmin.sh -nhnv` (`pkg/reconcilers/securityconfig.go:34,40,46`), the operator client sets `InsecureSkipVerify` (`opensearch-gateway/services/os_client.go:77`), Dashboards run with `opensearch.ssl.verificationMode: none` (`pkg/builders/dashboards.go:180`). - OpenSearch node pods do not roll, because no StatefulSet field derives from the DNS base. Dashboards roll, since `OPENSEARCH_HOSTS` changes (`pkg/builders/dashboards.go:46-47`). - A securityconfig Job already stuck in its wait loop is not recreated. The reconciler keeps an existing `<release>-securityconfig-update` Job while its checksum annotation matches the securityconfig Secret (`securityconfig.go:140-146`), and the target hostname is not part of that checksum. Delete that Job once by hand. The next reconcile recreates it against the new name, and users and roles start applying from there. ### Known and not addressed here The upstream `manager.dnsBase` knob stays usable. The platform writes it through `spec.valuesFrom`, and a per-package override in `Package.spec.components[].values` lands in `spec.values`, which Flux merges on top of the references, so a package that needs a different domain can still say so and win. The fallback differs from its neighbours and that is deliberate. `packages/apps/opensearch` falls back to `cozy.local`, `packages/system/keda` declares `cozy.local` as its package default, and this package stays on the upstream `cluster.local`. Inside the bundle all three receive the same value, so nothing diverges in practice, and `cluster.local` is the right standalone default for a Cozystack installed over an existing kubeadm or k3s cluster. A cluster domain that YAML reads as a number does not survive the round trip. The platform quotes the value into the Secret, so `networking.clusterDomain: 123` arrives as the string `123`, but the upstream template renders it as a bare `value: 123` in the container env, and an `EnvVar` value has to be a string. Quoting it means patching the vendored template, which is the thing this approach removes, and an all-numeric cluster domain is not a usable DNS name to begin with. Before this change the key was never set from the platform, so the path is new here even though nothing reaches it. The vendored `opensearch-operator` Deployment template does not reproduce from `make update` and has not for a while: a fresh pull plus the patch differs in three places, and one of them is a live defect. The committed `nindent 10` on `manager.extraEnv` renders invalid YAML, so that knob cannot be used at all, while upstream's `nindent 8` works. Nothing in the tree sets `extraEnv`, so it is latent, and the drift predates this branch. This PR does not touch that patch at all and does not carry a fix for it. Nothing in CI renders a package chart with the values the platform injects, which is why the smee breakage on the earlier approach had to be found by hand. That is a gap in the harness. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff, which is one template key in `packages/core/platform/templates/apps.yaml` and two test files. `packages/core/platform/values.yaml` is untouched and gains no key, so the hand-written platform-package table on the website is not affected. No package, app schema, version enum, CRD, release prefix or naming convention moves, so the Terraform provider is not reached. The `networking.clusterDomain` key already exists and its default does not change, so ansible-cozystack, which passes `--cluster-domain=cozy.local` to k3s and sets only the networking CIDRs in its platform-package template, needs nothing: the operator now follows the domain that repo already configures. talm sets the Talos `cluster.network.dnsDomain` from its own value and does not write the platform value either. Nothing under `hack/`, no namespace and no package-tree layout moves, so ccp is not reached. - [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(opensearch-operator): the operator's DNS_BASE now follows the platform cluster domain (`networking.clusterDomain`) instead of the upstream `cluster.local` default, so the securityconfig job and OpenSearch Dashboards reach the cluster on installations that use `cozy.local`. On upgrade, an existing `<release>-securityconfig-update` Job that is already stuck waiting on the old hostname is not recreated by the operator: delete it once by hand and the next reconcile recreates it against the new hostname. Existing certificates keep their SANs and keep working, nothing in-cluster verifies them. ``` |
||
|
Aleksei Sviridkin
|
50e1e2b2ed |
feat(gateway): add tlsPassthroughListeners to the TenantGateway API
A tlsPassthroughServices entry publishes a hostname on port 443, which suits a service whose clients speak HTTP and mis-serves one whose clients expect a database on its own port. The new field takes a name, a native port and the SNI hostname to answer on it, so an engine can be reached where its drivers already look. The cross-field rules are enforced at admission as well as in the controller. renderGateway is the first thing that renders, so a spec the apiserver accepts and the renderer refuses aborts everything behind it, and one mistyped hostname would freeze the Gateway and every route status where they stand. The controller keeps its own copy because it and the CRD roll out separately. Rules with no admission form, and why each one has none, are named on the type. tlsPassthroughServices and apex gain bounds at the same time, for the CEL cost estimator rather than for their own sake: the new rules concatenate apex and scan the services list, and an unbounded string or list makes the install-time estimate exceed the per-CRD budget and the apiserver refuse the whole definition. Every bound is a kind the apiserver ratchets, so a stored value that violates one keeps accepting writes that leave it alone. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrei Kvapil
|
774f788016 |
ci(agents): check commit trailers on every pull request (#4103)
## What this PR does `docs/agents/contributing.md` makes a few properties of a commit message blocking at review: the attribution trailer takes the value `LLM` and nothing else and appears once, no message carries a session trailer or a link to a transcript, no line is a generation byline naming the tool. Nothing checked any of it, so the only reader was a human scrolling `git log`. That reader is expensive and misses things. One open PR carried eight commits naming a model in the trailer, another four, all found by hand after the work was done, each one costing a review round and a rebase. `hack/check-commit-trailers.sh` decides all of it from the text, and the `Commit trailers` job in `.github/workflows/pre-commit.yml` runs it over the commits between the merge base and the head. Over all 6062 commits on `main` it rejects 1300, every one a true positive and no prose false-fires: 1289 attribution trailers naming a model, 24 `Generated with <tool>` bylines, 15 commits carrying two trailers, two `claude.ai/code/session_...` links. ### Where it lives Next to the other per-PR text lint in `pre-commit.yml`. The unit-test job in `pull-requests.yaml` was the other candidate and is wrong for this: it is gated on the diff touching code, so a docs-only PR skips it, and a trailer is wrong or right whatever files the commit changed. The range is `merge-base..head`, recomputed every run, so a rebase or a force-push is read as it now is. It never reaches the history on `main`, roughly a quarter of which carries the old form. ### Decisions worth arguing with It fails rather than warns. A warning in a job nobody watches guards nothing. It is not a required check, and that part is deliberate. Branch protection on `main` requires `pre-commit` and `E2E Tests`, and `Commit trailers` is a new context, so a PR can still merge with it red. 77 of the 200 open PRs carry at least one commit this rejects, so requiring it today turns those red at their next push. Flipping it is a branch-protection change once the open set has drained, and somebody else's call. Fork PRs are checked like any other. The job runs on `pull_request` with a read-only token and reads commit messages as data. A merge commit with no trailer of its own passes. Nothing is required of a commit, only what is written is judged, so a merge commit that does carry a bad trailer fails like the rest. Backport branches are exempt. The backport action cherry-picks merged commits verbatim, so those messages are history and not the author's to rewrite. The exemption matches the generated name `backport-<pr>-to-<branch>`, which is the only signal a `pull_request` event carries, since the label sits on the original PR. So it is a convention, not something enforced. The checker is read from the base branch instead of the checked-out PR. A `pull_request` event takes the workflow definition from the merge of base and head, so the job shows up on every open PR the moment it lands, including the ones whose head predates the script, where reading it from the head is exit 127 and a red check saying nothing about trailers. It also keeps the checker out of reach of the PR being measured, though the job around it is not. Two shapes are left to the reviewer. A model named in `Co-authored-by:` needs a list of model names to tell it from a person, and such a list either misses next month's model or fires on somebody's colleague; most of the 771 such lines on `main` name a model, so it would be deciding a lot of PRs. A `Generated-by:` trailer has never appeared here at all. The byline rule matches a capital `G` for a related reason: case-insensitively, one line in 25 on `main` is an ordinary sentence that wrapped onto "generated with cozyvalues-gen". ### Known costs `hack/check-commit-trailers.sh` matches `hack/[^/]+\.sh$` in `select-e2e.sh`'s `full_suite_pattern`, so this PR and any later one touching the script escalates to the full e2e suite. That pattern's own comment scopes it to the e2e harness, which this script is not, and `inert_config_pattern` already carries `hack/[^/]+\.bats$`. Escalation is fail-safe so it is cost rather than breakage, and reclassifying it wants its own review. The step drops the explicit `git fetch origin "$BASE_REF"` that `api-review-gate.yaml` keeps before its own `merge-base`. `fetch-depth: 0` already fetches `+refs/heads/*:refs/remotes/origin/*`, and if that were ever wrong the failure is a loud `merge-base` error under `set -euo pipefail`, not a silent pass. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff. The two entries that could fire are `cozystack/ccp` and `cozystack/community`, both keyed on a change to the commit convention itself. The convention text is untouched here, only its enforcement is new, so both copies stay accurate. `ccp`'s `package-bump` skill already writes `Assisted-by: LLM`, which this check accepts. - [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 ci(agents): a pull request now fails when one of its commits carries an attribution trailer whose value is not `LLM`, a second attribution trailer, a session trailer or transcript link, or a `Generated with <tool>` byline. The check reads only the commits the pull request adds; backport branches are exempt. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Pull requests now include automated checks for commit attribution trailers, session links, and generation bylines. * Backport branches are excluded from this validation. * **Documentation** * Added guidance describing the commit-message checks, accepted attribution formats, exceptions, and how to run the validation locally. * **Tests** * Added coverage for valid and invalid commit-message formats, range handling, merge commits, and exempted content. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
d473454529 |
chore(labels): add triage/needs-proposal for requests that go through cozystack/community (#4042)
## What this PR does Adds the `triage/needs-proposal` label to `.github/labels.yml` for feature requests that need a design proposal in [cozystack/community](https://github.com/cozystack/community) before any work starts here. Until now that routing rule lived only in chat, so such requests kept landing in this repository and were re-filed by hand with nothing on the issue saying why. AGENTS.md now spells out the handling next to the rest of the triage guidance: the request is re-filed in cozystack/community, closed here with the label and a link to the proposal, and work in this repository starts from a new issue once the proposal is accepted. ### Screenshots Not applicable, no 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: The diff touches `.github/labels.yml` and `AGENTS.md` only; neither file appears in the downstream trigger map in `docs/agents/contributing.md`. ### Release note ```release-note chore(labels): add triage/needs-proposal for feature requests that need a design proposal in cozystack/community before work starts here ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Added a new triage label for issues that require a design proposal before implementation. - Documented the process for submitting, reviewing, and accepting feature proposals before work begins. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
3cb9f2179c |
fix(ci): map the recurring app scopes to their area labels (#4121)
## What this PR does The auto-labeler maps a Conventional Commits scope to an `area/*` label. Scopes that PR titles use for apps under `packages/apps` were missing from that map, so a PR whose title carries only such a scope gets `area/uncategorized` and the area has to be set by hand afterwards. Each scope added here clears the bar the map's own comment sets for a new entry, three or more occurrences over the whole PR history in both title forms the labeler parses: `vm-instance` 14, `nats` 10, `rabbitmq` 8, `vpc` 8, `foundationdb` 5, `qdrant` 4, `kubernetes-nodes` 3, `vm-disk` 3. Each also has an unambiguous home among the labels that already exist. `area/virtualization` names `cdi`, and `vm-disk` renders a `DataVolume` while `vm-instance` renders a `VirtualMachine`. `area/kubernetes` is the tenant Kubernetes app and `kubernetes-nodes` is the worker half split out of it. `area/database` is described as managed databases and already lists `kafka`, so `rabbitmq`, `nats`, `qdrant` and `foundationdb` sit there without stretching it. `vpc` renders `Vpc` and `Subnet`, and `area/networking` already names `kube-ovn`. Replayed the map before and after this change over the whole PR title history: 53 titles come out with a different label set. Each of them gains one `area/*` and loses nothing. 48 stop falling back to `area/uncategorized`; the other 5 already had an area from a second scope in the title and now carry both. A scope that clears the bar but has no unambiguous home is left out rather than pushed into the nearest label. AGENTS.md asks for a new `area/*` in that case, which is a separate decision. Those, and the scopes still below the bar, are collected in #4122 along with the measurement and two bits of drift found on the way. `opensearch` is not that case: its home was never in question, it was below the bar when this PR opened and crossed it while the PR waited. It stays in #4122 rather than being folded in here. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The diff touches `.github/workflows/pr-labeler.yaml` only. The nearest triggers are the ccp and community entries on the commit convention in `docs/agents/contributing.md`, and that file is not touched here: the scope table there is a summary that already lags the workflow, and the workflow is named as the full mapping. - [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(ci): more of the app scopes used in PR titles now map to an `area/*` label instead of falling back to `area/uncategorized` ``` |
||
|
Aleksei Sviridkin
|
b083d8e9e6 |
test(platform): pin the opensearch-operator DNS base wiring
Two ends of the same path. The platform suite pins that the shared values Secret carries networking.clusterDomain to the operator under the subchart key, and the package suite pins that a domain set on that key reaches the manager container as DNS_BASE, with the upstream cluster.local default intact when nothing sets it. Every pattern in the platform suite anchors dnsBase to its opensearch-operator and manager parents rather than matching the key anywhere in the payload. An unanchored pattern is satisfied by any sibling block that happens to carry the same domain, which leaves the assertion green after the block it is meant to pin has been deleted. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
857ad4a128 |
fix(opensearch-operator): resolve in-cluster names on the platform domain
The operator reads DNS_BASE and builds every in-cluster name it needs from it: the securityconfig job target, its own OpenSearch API client URL, the Dashboards host, and the SANs it writes into node certificates. The vendored chart defaults that to cluster.local, so on a cluster whose domain is cozy.local the securityconfig job never resolves its target, the .opendistro_security index is never created, and the OpenSearch nodes never go Ready. The domain the platform already holds could not reach the operator before: _cluster stops at each package's own templates and never reaches a vendored subchart, and the wrapper's values.yaml is not templated. Carry it in the shared values Secret under the subchart key instead, the same way the KEDA cluster domain travels, so the vendored chart picks it up with no patch and a package rendered without the Secret keeps the upstream cluster.local fallback. A global key would have worked for the operator but reaches every release on the cluster, and the vendored smee chart in bootbox cannot render once global is non-empty. Scoping the value to the subchart that consumes it avoids putting every other package on that path. The diagnosis and the first working fix came from fuad00 in an earlier pull request. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Alexey Artamonov
|
2d1e425339 |
docs(postgres): correct the KEDA-guard rationale and note the gradual client-side rollout
The KEDA-missing fail-closed guard's comment still described the replaced
drop-instances design ("db.yaml drops spec.instances ... CNPG collapses to 1");
under the shipped seed design db.yaml seeds instances at max(replicas,
effectiveMin), so a skipped ScaledObject leaves a fixed-size cluster with no HPA
- autoscaling silently absent, not a collapse. Correct the rationale. Also note
on the cozyrds apply-strategy annotation that the client-side switch rolls out
per-app as each Application is next written through the aggregated API, not
fleet-wide at upgrade (the ApplicationDefinition reconciler syncs only
chartRef/valuesFrom/labels onto a live HelmRelease). Follow-up from #3954 (#4187).
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
154548f57c |
fix(cozy-lib): reject a negative minReplicaCount and cover the annotations parameter
cozy-lib.keda.scaledObject only checked max >= min, so an all-negative or mixed-sign pair (min -5, max -3) passed the ordering guard and rendered a ScaledObject with a nonsensical floor. Reject minReplicaCount < 0 before the ordering check. Also add a cozy-lib-tests case for the documented annotations parameter, which had no caller and no coverage. Follow-up from #3954 (#4187). Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
1f2ff31470 |
fix(postgres): pin job=kube-state-metrics on the autoscaler read-load query
The read-load trigger query joined on kube_pod_labels without pinning job="kube-state-metrics", while the sibling DatabaseAutoscaler* alert rules already pin it. A second exporter emitting a kube_pod_labels series (or a duplicate scrape) would multiply the join and distort the desired-count arithmetic. Pin the job on every kube_pod_labels selector in the query for parity with the alerts. Follow-up from #3954 (#4187). Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
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
|
92edfb810b |
fix(backups): run the strategy bats under bash; move template essays to {{/* */}}
The behavioural bats test ran the rendered `set -euo pipefail` script via
`sh`; the CI unit-tests runner's /bin/sh is dash, which rejects `-o pipefail`
at line 1 and reddened the whole suite (production is busybox ash, which
supports it, so the script itself is fine). Run it via `bash`, matching ash
semantics and the hack/migration-50-etcd-adopt.bats precedent.
Convert the multi-paragraph `#` rationale blocks in the shipped strategy
template to `{{/* */}}` so Helm strips them at render instead of copying the
prose verbatim into every rendered Strategy CR, where it drifts as the code
changes. Short one- to three-line guard comments stay as `#`.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey Kolkov
|
9eb7eeaca5 |
test(backups): behaviourally pin the shipped Redis strategy script
helm-unittest renders this template but asserts only its structural decisions, so blanking the object key, breaking the rfs-redis- sentinel prefix, or disabling the cleanup guard each left the suite green. Add a bats test (auto-discovered by make bats-unit-tests) that extracts the rendered script and EXECUTES it with stubbed redis-cli / curl: cleanup must DELETE its object and never reach the restore FLUSHALL against the live master, backup must resolve rfs-redis-<app> and upload under the recorded object key, and an unrecognised mode must fail closed. Each of those three mutations now turns the suite red. Also correct the endpoint doc row: the Redis Job always connects over https (the projector strips the scheme and the script prepends it), so its TLS is not scheme-driven the way MariaDB/FoundationDB is. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |