mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-13 08:05:19 +09:00
4198914daec25a7533f2d0ddadef74cc771870fc
6312
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
4198914dae |
fix(opensearch): give the securityconfig update job its own resources
The operator hands spec.security.config.updateJob.resources straight to the Job container and the chart left it unset, so a tenant namespace applied its LimitRange default of 128Mi instead. securityadmin.sh runs java with no -Xmx and is OOMKilled at that size, and the operator builds the Job with backoffLimit 0, so that one kill is final: the security index is never written and every node stays unready. Constants rather than a preset value, because the tool configures the cluster once and exits. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Timur Tukaev
|
37e22dc356 |
docs(readme): stop the logo filling the page and thin the badge wall (#4222)
## What it looks like now The README opens with the logo rendered at **734×246** on a desktop viewport — a wall of logo before a single word of text — followed by eight badges of ragged widths. The logo is an SVG with no intrinsic width, so GitHub stretches it to the full width of the README column. It also uses the `#gh-light-mode-only` / `#gh-dark-mode-only` fragment hack, which GitHub has since superseded with `<picture>` and `prefers-color-scheme`. ## Changes **Logo** → a `<picture>` element at a fixed `width="340"`. Bounds the logo (now 340×114, measured on the branch) and drops the hack in the same move. Light and dark variants still switch. **Badges** → eight down to six. Two carried no information: - `Open Source` links to opensource.org and says what the Apache-2.0 badge directly beside it already says. - `Status: Active` is a hand-set claim rather than a measurement — the commit-activity badge next to it is the real thing, and this one was also served over plain `http://`. The remaining six are ordered identity → health → support: release, licence, commit activity, OpenSSF Best Practices, OpenSSF Scorecard, commercial support. The licence badge now points at `LICENSE` in this repository rather than at opensource.org. Also removed trailing whitespace on lines where it did nothing. The deliberate two-space line breaks in the use-case list are untouched. ## Verification Rendered on the branch and measured in the browser: logo 340×114, six badges, light and dark variants both resolving. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Updated the README logo for light and dark modes. - Reorganized project badges, updated LICENSE and commit activity links, and added a CLOMonitor badge. - Removed the Open Source and Active badges. - Revised introductory, contribution, and community information. - Updated the Semantic Versioning link and removed the Roadmap link. - Added a project documents table and CNCF/Kubernetes community channels. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
tym83andClaude
|
79ead9cf8d |
docs(readme): give the Slack links an invite path
Both Slack channel links assume the reader is already in the workspace; anonymously they just 403. Added the invite link beside each, so someone arriving from the README can actually get in: cncf-slack.netlify.app for CNCF Slack and slack.kubernetes.io for Kubernetes Slack. Both verified. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
5cdb10d8d2 |
docs(readme): tidy the table header, a duplicate link and a sentence
Final review nits: the project-documents table had an empty header row that GitHub renders as a blank band, ROADMAP ended up linked twice once the table landed, and one sentence in Contributions still read wrong. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
7a4ac1df02 |
docs(readme): fix review findings
Reverted the <picture> element. #gh-light-mode-only keys off GitHub's account theme; prefers-color-scheme keys off the OS. They diverge for anyone who pins a GitHub theme rather than syncing with the system, and the combination 'OS dark, GitHub light' renders the white logo on white. The fragment form is correct in all four combinations. Kept the width attribute, which was the actual fix -- the logo stretched to 734px because the SVG has no intrinsic width, not because of the fragment. Also from review: - /docs/roadmap/ on the website is a 404; linked ROADMAP.md instead. - Added a project-documents table. The README linked one of the eight policy files in the repository root; governance, code of conduct, security policy and adopters are what a newcomer and a CNCF reviewer both look for first. - Added both Slack channels. The community section listed Telegram only, while the CNCF-hosted #cozystack channel went unmentioned. - Added the CLOMonitor badge, the standard CNCF project-health signal. It currently reports A. - semver.org was the one remaining plain-http link, which sat oddly next to dropping a badge for the same reason. - The commit-activity badge linked to the contributors graph. - 'free platform' reads as price now that the Open Source badge is gone. - 'CNCF Sandbox Level Project' is legacy phrasing. - Fixed an ungrammatical sentence in Contributions. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
34c7a20a21 |
docs(readme): stop the logo filling the page and thin the badge wall
The logo is an SVG with no intrinsic width, so GitHub rendered it at the full width of the README column -- 734x246 on a desktop viewport, a wall of logo before a single word of text. It also used the #gh-light-mode-only / #gh-dark-mode-only fragment hack, which GitHub has since superseded with <picture> and prefers-color-scheme. Replaced with a <picture> element at a fixed 340px, which both bounds the logo and drops the hack. The badge block had eight entries of ragged widths. Two carried no information: 'Open Source' says what the Apache-2.0 badge beside it already says, and a hand-set 'Status: Active' is a claim rather than a measurement -- the commit-activity badge next to it is the real thing, and the static one was also served over plain http. Both removed. The remaining six are ordered identity, then health, then support, and the licence badge now points at the LICENSE file rather than at opensource.org. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@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> |
||
|
Andrey Kolkov
|
1289f522fa |
fix(backups): unbreak install, kill destructive cleanup, reach the resource knob
Store the Redis strategy Template schemaless, as the Rabbitmq strategy already does: without the markers controller-gen emitted the full PodTemplateSpec schema and the CRD reached ~602 KB, pushing the chart's Helm-release Secret past the 1 MiB cap so backupstrategy-controller failed to install at all. Give the example strategy a cleanup branch, and both scripts an explicit `elif restore` / `else exit 1`, so a deleted Backup rendered with Mode=cleanup deletes its object instead of falling into the restore path and FLUSHALLing the live master. Forward a backupStrategyController block through the platform bundle alongside backupStorage, so redisBackupResources — the restore-Pod sizing knob the loader-not-ready message points at — is reachable via spec.components.platform.values.backupStrategyController, not merely settable on a Helm-managed Strategy CR that reverts on the next upgrade. Correct the endpointCASecretName doc row to match the now-optional mount, and `int` the activeDeadlineSeconds interpolation so a large values-file override renders as an integer, not exponent form. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
651a6e3023 |
fix(backups): harden the redis restore AOF wait and image build
Wait for the throwaway loader's AOF rewrite to finish - no rewrite in flight, last one ok, base AOF on disk - instead of a bare aof_rewrite_in_progress:0 check, which right after `appendonly yes` can fall through before the scheduled rewrite starts and replay an absent or half-written base AOF. Same fix in the example strategy. Declare the package image targets PHONY and serialize image-redis-backup after image-backupstrategy-controller so their `yq -i values.yaml` digest stamps cannot race under `make -j`. Run the demo cleanup under `set -e` (every delete is already --ignore-not-found) so a real API failure aborts instead of printing "Cleanup complete" over a half-torn-down demo. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
b4d621f4f7 |
fix(backups): delete Redis backup objects on cleanup and size the restore Pod
Delete the backup object when its Backup is removed. The Redis driver
owns its S3 object (one per BackupJob, no engine retention prunes it), so
it now spawns a cleanup-mode Job that deletes the object and holds the
Backup's finalizer until it completes, mirroring the Rabbitmq driver.
Without this a retention-pruned Plan grew the bucket unboundedly and the
key was lost with the CR.
Size the strategy Pod from backupStrategyController.redisBackupResources
so an admin can raise the restore ceiling (the throwaway loader holds the
whole dataset in memory) via chart values, which a direct edit of the
cluster-scoped, Helm-managed Strategy CR could not survive. The
loader-not-ready diagnostic now names the memory limit, the likeliest
cause, instead of only the image version.
Make the endpoint CA volume optional: nothing projects that Secret into
the app namespace, so a required mount wedged every backup Pod on
FailedMount; a missing ca.crt now falls through to the system CA store
and fails the handshake fast, which is not the same as skipping verify.
Promote the Pod-level activeDeadlineSeconds to the Job so a wedged run is
bounded once, not (backoffLimit+1) times. Use find, not ls, for the base
AOF so its no-match diagnostic is not dead code under pipefail.
Pin the object key reaching the rendered Job: both fixtures now render
--key={{ .ObjectKey }} and assert it, so zeroing the key (which left the
package green) is caught. Document .ObjectKey and the cleanup mode in the
CRD render-context, list cozy-default-redis among the Helm-managed CRs,
and mirror the CA/TLS and find fixes into the example strategy.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey Kolkov
|
56e10b1580 |
fix(backupstrategy-controller): gate the default MongoDB Strategy on its CRD (#3980)
## What breaks today Upgrading any existing cluster to a build that carries the new MongoDB backup driver wedges the `backupstrategy-controller` HelmRelease permanently, with `resource mapping not found for name: "cozy-default-mongodb" ... no matches for kind "MongoDB" in version "strategy.backups.cozystack.io/v1alpha1"; ensure CRDs are installed first`. This chart ships its `strategy.backups.cozystack.io` CRDs as ordinary templates, and Helm resolves every document in a release manifest through the cluster's RESTMapper in `Build()` **before** it applies any of them. A Strategy CR whose CRD is not served yet therefore fails `Build()`, so the release applies **zero** objects — including the CRD that would have made the mapping resolve — and every retry renders the same manifest. Nothing recovers on its own. ## Why the gate that exists does not cover it The chart already knows this trap: it is why every default Strategy gates on the bucket name. That gate covers the **first install** of a fresh cluster, where nothing is resolvable yet and no Strategy renders at all. It cannot cover shipping a **new kind** to a chart that is already installed — there the bucket name resolved releases ago, so the first upgraded revision renders the new CRD and the new Strategy CR together and wedges. ## The fix Gate the CR on its CRD actually being served, through a `crdEstablished` helper. The helper asks about the **CRD object** rather than listing the custom kind, because `lookup` on a kind the apiserver does not serve is a render *error*, not an empty result, which would trade this wedge for a different permanent failure; `apiextensions.k8s.io/v1` is always served, so that lookup can only answer yes or no. It requires `Established=True` rather than mere existence, because that is when the kind enters the RESTMapper. Convergence afterwards is the mechanism the bucket gate already relies on, and is **not** the HelmRelease interval: once the CRD is served, the controller's `DefaultObjectsGate` sees the kind mapped and the CR absent and forces a real Helm upgrade whose render includes it. Kinds that do not map at all are skipped there (`meta.IsNoMatchError`), which is what stops the gate forcing upgrades against a render that cannot yet produce the object. `bucketNameOverride` short-circuits the CRD lookup for the same reason it short-circuits the bucket lookup: it marks an offline render — `helm template`, a CI diff, a unit test — where there is no apiserver to ask. Live deploys go through Flux and never set it. ## How it was found The release upgrade E2E lane in #3276, on the `v1.6.2 → main` path. `mongodbs.strategy.backups.cozystack.io` is the only definition added to this chart since the tag, so this is reachable only by upgrading an existing install — no install-only suite can see it. ## Verification Reproduced and fixed against a live cluster that happens to be in exactly the pre-upgrade state (the seven older strategy CRDs present, `mongodbs` absent). `helm template --dry-run=server` of main's chart fails with the error above and produces zero documents; the same render with this change succeeds with 21 documents and the Strategy correctly withheld. The helper's two branches were probed the same way against that cluster: a served CRD answers `true`, an absent one answers empty. The new unit case pins the gate offline through the external-S3 path, where the bucket name resolves without the offline override so that only the CRD gate can hold the CR back. Removing the gate turns exactly that case red; the suite is 19/19 green with it. ```release-note Fixed a permanent failure of the backupstrategy-controller release when upgrading a cluster to a version that adds a new backup-strategy kind: the default MongoDB Strategy is now withheld until its CRD is served, instead of failing the whole release with "no matches for kind". ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved backup strategy rendering during upgrades by waiting until the required MongoDB resource definition is available. * Preserved offline rendering when an explicit bucket name is provided. * Prevented invalid default strategies from being generated when the required resource definition is unavailable. * **Tests** * Added coverage for offline rendering and unavailable resource-definition scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9e577a5771 |
docs(rabbitmq): say that deleting the application leaves the TLS Secrets behind (#4143)
## What this PR does `packages/apps/rabbitmq/README.md` documented the key material that survives turning TLS off, but not the same outcome on deleting the application. The chart renders `Certificate` and `Issuer` objects and never the Secrets, and cert-manager runs with `enableCertificateOwnerRef: false`, so a Helm uninstall leaves `rabbitmq-<name>-ca` (the CA private key) and `rabbitmq-<name>-tls` in the tenant namespace with nothing referencing them. One paragraph names the two Secrets and the cleanup command. Documentation only. ### Screenshots Not a UI change. ### Downstream repositories A README paragraph in one chart; the generated reference page is built from `values.yaml` and does not move. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added uninstall guidance for TLS-enabled RabbitMQ applications. - Clarified that generated certificate Secrets must be manually removed from the tenant namespace after application deletion. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
2055341ba9 |
fix(opensearch-operator): raise vm.max_map_count on every node from a DaemonSet (#4152)
## What this PR does OpenSearch will not start below `vm.max_map_count=262144` when the store may use mmap, and nothing in the platform sets it. The Talos profiles declare no sysctls and no documented node-preparation path sets one, so nodes sit at the kernel default of 65530. The app chart asks the operator to cover that with `spec.general.setVMMaxMapCount`, which appends a privileged init container to every OpenSearch pod. Tenant namespaces carry no PodSecurity label of their own, so they take the cluster default, which Talos ships as baseline through the apiserver's `PodSecurityConfiguration`. That default refuses the container and the StatefulSet never produces a pod. On vanilla kubeadm, kind or k3s the default is `privileged` and the same release comes up, which is why this only bites on a Talos install. The value cannot come from the pod at all. The kernel does not namespace `vm.*`, so kubelet rejects it in `securityContext.sysctls`, and refuses to start if it appears in `--allowed-unsafe-sysctls`. That leaves node configuration or a privileged pod, and a privileged pod belongs in a system namespace rather than in every tenant. This adds a DaemonSet to the `opensearch-operator` package that sets the value once per node from a privileged init container, and marks the component privileged so the platform labels `cozy-opensearch-operator` with `enforce=privileged`. The init container writes to `/proc/sys` and nothing else, with no hostPath. The pod restarts with the node and reapplies the value, so there is nothing to persist. It raises the limit to a floor rather than assigning it, which matters more than it looks. `vm.max_map_count` is node-global, and a stock Ubuntu 24.04 node already reads 1048576 from a `sysctl.d` drop-in. An unconditional write would drop that to 262144 for every workload on the node, on every boot, and undo whatever the operator set it for. So the init container reads the current value first and writes only when it is below the threshold. The idle main container runs its sleep under a shell that traps SIGTERM, so a pod exits on its own when the DaemonSet rolls or a node drains, instead of ignoring the signal and being killed after the full grace period. This does not fix OpenSearch on its own. The app chart still requests the operator's init container through `setVMMaxMapCount: true`, so a release stays blocked until the flip to `false` lands with #2682, after this merges. The DaemonSet has to be in place first: flipping it without one would trade the admission failure for a failed bootstrap check. Carrying the sysctl in the node contract instead, across the Talos profiles and the downstream node-prep paths, is tracked in #4144; that is what would let this DaemonSet be retired. Two things change on upgrade, and neither is visible from the diff. One small DaemonSet pod appears per node in `cozy-opensearch-operator`, and that namespace gains `pod-security.kubernetes.io/enforce=privileged`. That label has no finer granularity than the namespace, so the operator Deployment sitting there leaves the cluster default too, not just the DaemonSet. Keeping the manager at baseline would mean a namespace of its own and another component entry, which did not look worth it, but it is a real cost rather than an accident. The operator is in the `paas` bundle's always-on list, so this lands on every node of every paas installation and raises a node-global kernel tunable there, whether or not anyone ever creates an OpenSearchCluster. That is the price of a node-level sysctl having no cheaper carrier, and it is worth an explicit decision rather than being noticed later. The second is `cozypkg add`, which gates privileged components behind a confirmation or `--allow-privileged`. The gate runs over the resolved dependency closure, and `opensearch-application` dependsOn the operator, so installing the app that way now asks one more question. This is not a new break for scripts: `add` already prompts for a variant on every package it resolves and fails on EOF, so it was never usable non-interactively in the first place. Platform-bundle installs go through the reconciler and are unaffected. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It adds a template, a test and a values key under `packages/system/opensearch-operator/`, plus one field in `packages/core/platform/sources/opensearch-operator.yaml`. No new app package, no app schema or version enum, no `ApplicationDefinition`, no installer value, no namespace rename, no variant or bundle, no `hack/` path or make target, and no node prerequisite in `hack/e2e-prepare-cluster.bats`. Nothing in the map matches. - [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): set `vm.max_map_count` on every node from a DaemonSet in the operator's namespace, so OpenSearch no longer depends on a privileged init container that PodSecurity baseline refuses in a tenant namespace. Upgrading adds one small DaemonSet pod per node and labels `cozy-opensearch-operator` with `pod-security.kubernetes.io/enforce=privileged`; installing the package with `cozypkg add` now requires confirmation or `--allow-privileged`. Existing OpenSearch releases are unchanged until a follow-up stops the app chart requesting the operator's own init container. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - OpenSearch Operator installation now runs with the required privileged configuration. - OpenSearch deployments automatically ensure nodes meet the required `vm.max_map_count` setting, improving startup reliability. - Existing higher kernel values are preserved. - Added support for configuring the sysctl image, including use with air-gapped registries. - **Tests** - Added coverage to verify privileged installation, node configuration, security settings, scheduling behavior, and image customization. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
84fe2ecf6f |
docs(backups): mirror the redis TLS fix into the example strategy
The demo strategy CR states its in-Pod script is identical to the shipped cozy-default-redis strategy, but it kept the plaintext Sentinel and master dial after the shipped template gained server-side TLS. Add the same optional redis-<app>.ca-cert mount and RTLS auto-detection so a tenant who copies this example to back up a tls.enabled Redis does not reproduce the plaintext-against-a-TLS-only-listener failure. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Alexey Artamonov
|
9511f56307 |
fix(postgres): require service.namespace.svc shape in the autoscaling SSRF host guard
The serverAddress host regex accepted any dotted tail ending in .svc, so a single-label host like foo.svc passed the in-cluster-DNS check. It then reached the namespace-pin, whose extraction regex left the unresolvable host unchanged and rejected it as a namespace outside the tenant subtree - the right outcome by the wrong path, with a misleading message. Anchor the host to two DNS labels before .svc so it is refused at the host check with the actionable message, and drop the stray dot the old label class allowed. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |