mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-29 16:17:20 +09:00
feat/postgres-credentials
455
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Yan Bondarenko
|
fe59a6bf91 |
fix(etcd): leave compaction to etcd and pass the real quota to defrag
The hourly defrag CronJob runs etcd-defrag with its defaults, which compact to the latest revision before defragmenting. Every cluster already compacts itself with a periodic five-minute retention, so the second compaction either fails with "required revision has been compacted" when nothing was written since, or drops the history that retention keeps for watchers to resume from. The same defaults evaluate dbQuotaUsage in the defrag rule against 2 GiB rather than the quotaBackendBytes the cluster runs with, so the 80 percent threshold fires at a different database size than intended. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
d9689bd6e7 |
fix(gateway): stop describing Cilium 1.19.5 as the shipped release
With Cilium at 1.20.2 the tenant gateway comments, README and one status message still explained behaviour as that of "the pin" at 1.19.5. Several of those claims no longer hold: a Gateway with two routed passthrough ports now gets an Envoy listener per port, Cilium judges a route's kinds against the listener the route names, and the shipped operator accepts an HTTPS and a passthrough listener on one port when the passthrough hostname is strictly narrower. Behaviour that is unchanged is re-anchored on the 1.20.2 source, behaviour that is gone is stated as history, and the refusal of one hostname on two passthrough ports no longer blames a Cilium limitation the shipped release does not have. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ad3f15d4f3 |
feat(gateway)!: reserve a passthrough listener's name by the spec alone
The TenantGateway controller withheld the HTTPS-terminate listener for a name a passthrough listener answers only once a TLSRoute was attached there and resolved a backend, mirroring how Cilium builds its filter chains. That kept a served endpoint up on Cilium 1.19.5, but it tied every route's status to one implementation's translator and needed Service and ReferenceGrant watches to land the deferred decision. The shipped operator accepts a wildcard terminate listener beside a narrower passthrough listener on 443 and keeps conflicting every other overlap. A terminate listener for the passthrough name itself is not narrower, so on 443 the rule now protects the dataplane outright; on a native port Gateway API would serve the pair, and the rule holds as a policy that keeps a platform name with its passthrough listener. Reading it off the spec makes the outcome the same on every implementation. Under http01 an HTTPRoute claiming such a name gets no listener and no certificate whether or not a TLSRoute is attached. A route whose every hostname is reserved reads Accepted=False with NoMatchingListenerHostname; one that also claims a free hostname stays Accepted and is served there, as Gateway API accepts a route whose hostnames intersect at least one listener. dns01 and existingSecret are unchanged: their wildcard listener still covers such a name, and the passthrough listener answers it by its exact server name. The chart writes the passthrough services on the publishing tenant alone, so the names reserved by default are the ones the platform publishes there through its own TLSRoutes. The shape an upgrade changes is an HTTPRoute on such a name whose platform TLSRoute forwards nowhere, because it never attached (the package owning the service is not installed) or because none of its backends resolve: that route kept a terminate listener and now gets none. The backend resolution, its watches and the RBAC rules that served them go away. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
52f09d1aed |
fix(gateway)!: render passthrough services only where platform routes attach
The chart wrote the tlsPassthroughServices default onto every non-edge tenant's TenantGateway, so every tenant Gateway carried tls-api, tls-vm-exportproxy and tls-cdi-uploadproxy. The TLSRoutes that serve those names are platform routes: the packages owning the services render one per name in publishing.exposedServices while gateway.enabled is on, parented to the publishing tenant's Gateway alone. Everywhere else no shipped route attached to the listeners, and a passthrough listener holds its name whether or not anything is routed through it. The chart now writes the field only on the publishing tenant, with the same gateway.enabled switch and the same exposedServices filter those routes use, so a listener exists exactly where a platform route can attach. A TLSRoute attached by hand to one of those listeners on another tenant loses its parent. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d624049a3a |
feat(gateway): allow tlsPassthroughListeners under dns01 and existingSecret
The refusal existed because Cilium 1.19.5 served every Gateway port from one Envoy listener matched by SNI alone, so the "*.<apex>" terminate listener of the wildcard modes and a passthrough listener under the apex could reach each other's backend. The shipped Cilium 1.20.2 gives each port its own Envoy listener when SNI overlaps across ports, and Gateway API treats listeners on different ports as compatible, so the pair is served as declared. edge still refuses the field: it renders no TLS listener for an entry to bind to. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Alexey Artamonov
|
e63b9e439c |
fix(monitoring): count a central request as the receiver admitted it
The limiter refuses for good any request it counts above burst, and burst was the receiver's 4 MiB cap. A request grows on its way to the limiter: the sampler adds a tracestate to every span it keeps, and the limiter counts a request as the collector re-encodes it, which writes the empty value of every attribute the wire may omit, so a valid request of spans with empty-valued attributes is counted 1.4x. A request the receiver admitted near its cap was therefore refused on every retry. The limiter now runs right after memory_limiter, so it counts the request before sampling. The receiver caps a request at 3 MiB and burst stays at 5 MiB, which holds 1.4x the cap; a larger burst would also be what each pod start lets through above the rate. The rate is counted before sampling, so the store keeps only the sampled share of it. The texts say that a request above the cap gets a 400, which senders do not retry, and that each collector rollout a tenant causes lets that burst through again on every pod. The replica bound that keeps that allowance small now applies only to a central tenant's collector, whose render fails above ten replicas: a collector writing to the tenant's own store has no rate, and the replica counts it shipped with stay valid. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
65f0caae2b |
fix(monitoring): keep a central tenant's write login out of its Grafana
A tenant's Grafana pod may reach the shared traces store's vmauth, and
the Secret a tenant names in oidc.customConfig.secretRef was mounted
into it whole. The apps API does not prune keys the Monitoring schema
leaves out, so a tenant could also set grafana.image: an image of its
choosing, with the collector's Secret mounted, writes past the rate
limit the collector enforces.
Grafana's image is the platform's, like the collector's, and for a
central tenant only the auth.ini key Grafana reads is mounted from the
named Secret. Other tenants have no collector login to keep out and
keep the whole Secret, which an auth.ini reading a sibling key with
$__file{} relies on.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
cc51cac63d |
fix(monitoring): give a central tenant's Grafana a login that cannot write
The collector holds a central tenant to its write rate, but the one login it writes with was also the login of the tenant's Grafana datasource, and it could both read and write. vmauth cleans the request path before it picks a route, so any user the tenant admits to its Grafana, a Viewer included, could POST through the datasource proxy to ../insert and write into the shared store past the limiter. Each central tenant now has two VMUsers on its one account: the collector's, which may only write, and Grafana's, which may only read, each with its own chart-owned Secret and password. A namespace name holds no dot, so the reader's username never matches another tenant's. On upgrade the datasource moves to the new login in the same release that creates it, so reads may fail until vmauth loads the new user. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
fc0db49187 |
fix(monitoring): bound what a central tenant writes above its rate
Each collector pod starts with a full token bucket, and burst was at least 64 MiB, the largest request vtinsert accepts. A tenant controls when its collector pods start, through any Monitoring change or by deleting them, and how many there are, so it could write tens of mebibytes above its rate per pod start and gigabytes per rollout of many replicas, which the two-day bound does not survive. The limiter only sees requests the receiver admits, so in central mode the receiver now caps them at 4 MiB, decompressed for HTTP, and burst drops to that floor; a pod start lets at most 4 MiB through above the rate. The replica count is capped at 10. The texts now say what the default rate guarantees: one tenant writes at most a default store's disk cap over two days counted uncompressed, rather than that it cannot fill it, and they name the per-pod-start allowance and that a sender pinned to one replica gets that replica's share. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
177b207a27 |
feat(monitoring): hold each shared-central tenant to a write rate
The design has the per-tenant collector rate-limit its tenant, so that a noisy or hostile tenant cannot exhaust a shared backend. The disk cap alone does not do that: VictoriaTraces always keeps the last two days, so a tenant writing faster than the cap allows over two days fills the disk for every tenant on the store. The platform sets the rate with monitoring.tracingCentralTenantBytesPerSecond, and it reaches every tenant in _cluster through the cozystack-values Secret. A change there is a change in the values each monitoring HelmRelease reads, so Flux upgrades them all; a rate read with lookup at render time would never reach a tenant whose release is already in sync. The platform writes the key only when it is set, since every release in every tenant reads that Secret. Without it the collector holds the tenant to the rate of a default 10Gi store. Each central tenant's collector splits the rate across its replicas and refuses data above it before the batch processor, so the sender sees the refusal. An OTLP/HTTP sender gets a 429 and retries; the limiter answers gRPC with RESOURCE_EXHAUSTED and no RetryInfo, which the OTLP specification makes final, so central tenants should export over HTTP. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
32a6c71972 |
fix(monitoring): keep the collector image out of a tenant's reach
The apps API defaults a Monitoring spec from its schema but does not prune keys the schema leaves out, and the Monitoring chart hands its values to the inner chart as they are. A tenant could therefore set tracingCollector.image and run any collector it liked in its namespace; with a per-tenant rate limit in that collector, the same key would let a tenant drop the limit. The image is the platform's, so it is no longer passed through. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
bf03c84711 |
chore(deps): move to the Kubernetes 1.37 libraries without the apimachinery fork (#4473)
## What this PR does This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, and drops the `k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and #4472; those were a stack of the same commits. The four commits are meant to be read one by one, and each builds and passes the tests on its own: 1. **Plain watches stop getting the initial-events-end bookmark.** The apiserver defaults `sendInitialEvents=true` onto any watch with an empty or `0` resourceVersion and leaves bookmarks off, and our aggregated storages took that flag alone as a WatchList request. From `k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api panics on that bookmark, so `kubectl get -w` on any cozystack-api resource would die after the bump. The marker now needs both flags, the same condition the watch cache uses. There are tests per storage and one through the real handler chain. 2. **The apimachinery fork goes.** Every apps kind shares the `Application` Go type, and stock `Scheme.ConvertToVersion` picks the first kind registered for it. The fork patched that; the apps group now encodes through a serializer that keeps the kind the object carries. A handler-chain test covers create, get, list, update, watch, apply and table for two kinds and fails without it. 3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the plugin is on by default and refuses every write until it has synced `mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and which clusters before 1.36 do not serve. Without this, every create, update and delete would fail after the bump. It is also re-applied in `Complete`, since `--disable-admission-plugins` replaces the list. 4. **The bump itself**, with the regenerated clients, openapi and strategy CRDs. Commits 2 and 4 are where the review effort goes; 1 and 3 are small. Known changes in behaviour: - Without the fork, the row objects of a Table requested with `includeObject=Object` carry the first registered app kind. kubectl asks for that only with `--sort-by`, and it sorts on fields, not on the kind. Nothing at this layer can fix it, because the handler converts through `APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks and ValidatingAdmissionPolicy still see the right kind. - The alpha PodSpec field `workloadRef` became `schedulingGroup` upstream. Once the new strategy CRDs are applied, the apiserver prunes `workloadRef` from stored strategies it serves, and their next write erases it. It only ever worked behind an alpha feature gate. The branch is rebased onto the Renovate rollup (#4447), with `go mod tidy` and codegen rerun on top of it. ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note chore(deps): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application API responses now preserve the requested resource kind across operations, including lists and watches. * Backup strategy resources support additional Kubernetes pod settings, including probe protocols, volume ownership options, and scheduling groups. * Backup strategy schemas include updated Kubernetes pod fields and descriptions. * **Bug Fixes** * Initial-event watch bookmarks are sent only when requested; plain watches continue without an ending bookmark. * Mutating Admission Policy is disabled by default, including when admission plugin options are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
64f43f4f77 |
fix(computeplane): do not grant tenants the cluster admin kubeconfig
The computeplane module deploys the kubernetes chart into the tenant namespace, and that chart's dashboard Role lets the tenant's "use" subjects read <release>-admin-kubeconfig. Withholding the Secret from the tenant secrets view does not help when RBAC grants it directly, so any tenant user at "use" level or above could fetch the cluster-admin credential of a cluster the module promises to keep operator-owned. The kubernetes chart now drops that rule when the internal value withholdAdminKubeconfig is set, and the module sets it in the cluster HelmRelease's inline values. The tenant cannot reach those values: the module reads only nodeGroups from them and splices an allowlisted subset into the pool releases, and Flux applies spec.values over valuesFrom. A regular kubernetes app keeps the rule, so its dashboard can still offer the kubeconfig. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Alexey Artamonov
|
68ee97300b |
docs(monitoring): say a central tenant's collector still needs enabling
The tracingStorages description said a tenant with shared-central tracing gets the collector without a local entry, which reads as if tracingCentral alone deploys it. tracingCollector.enabled still decides whether it renders. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
d5fc723208 |
feat(monitoring): bound every traces store by disk usage
The distributed tracing design requires each traces backend to be disk-bounded, not only age-bounded, so a full volume cannot block ingest. retentionDiskUsageBytes was optional, so a store without it filled its volume and stopped taking spans; with a shared-central store that stops ingest for every tenant at once. A store without a byte cap now drops its oldest day once its filesystem is 80% full. VictoriaTraces refuses to start with both limits set, so a byte cap, when given, replaces the percentage one. It always keeps the last two days, which the values text now says. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
27b7376bb9 |
feat(monitoring): add a shared-central tracing topology
A per-tenant traces backend costs every tenant its own VictoriaTraces stack. A tenant can now send new spans to one shared store in tenant-root instead, switched on by _namespace.tracingCentral, which the tenant chart publishes and the apps API does not let an application set. tenant-root decides whether it hosts that store, with tracingCentralHost next to a cluster-mode store named generic. VictoriaTraces takes the tenant account from request headers and does no authorization of its own, so other tenants reach the store only through a vmauth in tenant-root; tenant-root itself, with whoever it admits to its Grafana, stays inside the trust boundary. Each central tenant gets a VMUser that pins an account derived from its namespace name and UID for both writes and reads and caps its requests in flight, which the collector's two export senders stay under; its collector exports and its Grafana reads with credentials the chart generates and keeps, and vmauth overrides any account a client sends. The chart owns those credentials because the operator publishes a VMUser's own Secret only once a VMAuth selects it; the collector rolls when they change, since it reads them at start. The vmauth internal routes, whose /metrics names every tenant's user, are served on a port tenants cannot reach, and the vmauth is sized by a VPA, spread across nodes under a disruption budget, has a WorkloadMonitor like the other monitoring workloads, and may reach the apiserver, where its config-reloader reads its config Secret. Renders refuse the states in which spans would be dropped with every object Ready: a central tenant while tenant-root hosts nothing, tenant-root dropping the hosting while central tenants remain, and the host flag without a store to host. Each refusal reads the other side at render time, so a concurrent opt-in and stop-hosting can pass both; the values text names that race and its recovery. At run time tenant-root's release is not Ready while the vmauth has failed. Only the collector's export moves: local stores the tenant still lists keep rendering with their datasources, because the switch belongs to the parent tenant and the stores to this one, and the one-store limit of the collector holds in both modes, since turning central off makes it export locally again. tenant-root never counts as central. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Yan Bondarenko
|
17aaf3e038 |
feat(monitoring): add optional email alert delivery
Platform alerts need an email delivery path alongside Alerta without exposing arbitrary namespace Secrets through tenant-controlled values. SMTP credentials therefore use a fixed Secret name. Alertmanager parses the addresses and connects to the SMTP host only when it sends a notification, so a malformed value there drops alerts instead of failing the release. The chart validates these fields when it renders. Addresses must be literal dot-atom mailboxes, and a domain literal must hold a dotted-quad IPv4 address, because IPv6 literal syntax differs across the supported Go mail parsers. A bracketed smarthost must be a valid IPv4 or IPv6 address, and its port must be between 1 and 65535 without leading zeros. The repeat interval follows the Alertmanager duration grammar and range, so a value that renders is one Alertmanager loads. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Mattia Eleuteri
|
7c9ab0e28e |
fix(cozy-lib): round the sanitized CPU request up to a whole milli-CPU
cozy-lib.resources.sanitize divides the CPU limit by cpuAllocationRatio and rendered the quotient as a bare float. When the ratio does not divide the value evenly the result is finer than a milli-CPU, and the VPA admission webhook rejects it outright: admission webhook "vpa.k8s.io" denied the request: minAllowed: CPU [62500u] must be a whole number of milli CPUs packages/extra/etcd routes its VPA minAllowed/maxAllowed through the helper, so 250m / 4 = 62.5m left the etcd HelmRelease looping UpgradeFailed -> RollbackFailed forever: the rollback target renders the same invalid value. Since v1.6.0 HelmReleases wait on each other, so that one stuck app also blocks tenant-root, cozystack-basics and kubevirt-cdi-operator, and without CDI the Talos worker rollover cannot import the worker OS image. Only the divisors of 250 worked as ratios, which is an artefact of one chart's hardcoded floor rather than a real constraint on the operator. The request is rounded up, never down, so it stays at or above the intended fraction of the limit. It is rounded only when it has to be: if the milli-CPU value equals its own ceiling, the float keeps its current rendering, so nothing changes on the default ratio of 10 or on any ratio that divides evenly; otherwise it renders as the ceiling in m. The comparison is exact, with no tolerance. sprig's divf and mulf run on shopspring/decimal rather than float64, so mulf (divf 0.25 10.0) 1000.0 renders exactly 25 and there is no binary-float noise on this codepath to absorb. A tolerance would reintroduce the bug: a value whole-milli only to within it, such as 100.0000004m at ratio 1, would read as exact and render "0.1000000004", rejected for the same reason 62.5m is. The same predicate covers sub-milli input, since ceil carries any positive request to at least 1m, and a zero request stays "0". Fixes #2917 Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
02461acdde |
Merge remote-tracking branch 'origin/main' into feat/arm64-bundle
Signed-off-by: Aleksei Sviridkin <f@lex.la> # Conflicts: # hack/image-builders-cross-compile.bats |
||
|
IvanHunters
|
2fa51bf587 |
test(charts): isolate guard coverage for every guarded template
Add per-template guard suites so each guarded hostname site is exercised on its own: harbor harbor.yaml and httproute.yaml, bootbox machines.yaml, seaweedfs cosi-deployment.yaml, bucket deployment.yaml and httproute.yaml, monitoring alerta.yaml, grafana httproute.yaml and the grafanaHost helper via oidc-keycloak.yaml. Each suite renders only its target template, so removing that file's guard reddens it. Also add a postgres positive case that renders the Certificate with an apex set (external=true). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
0f4d39f1ef |
test(charts): add empty-apex guard tests for the remaining guarded charts
Add host_guard_test.yaml for postgres, vpn, bootbox, seaweedfs, monitoring and bucket. Each renders its guarded template with a non-empty injected _namespace whose host is empty and no per-app host override, and asserts the ns-host-required failure, plus a positive render case with the apex set. Completes guard coverage alongside the existing harbor, qdrant and kubernetes tests. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
37f59369d3 |
fix(charts): fail fast on empty tenant apex in host composition
Charts that publish a service compose their hostname as <sub>.<_namespace.host>. When the injected tenant apex is empty and no per-app host override is set, they used to emit a malformed trailing-dot hostname (e.g. "harbor.") into Ingresses, HTTPRoutes and Certificate dnsNames silently. Source the apex through cozy-lib.ns-host-required at each hostname-emitting site so the render fails with a clear message instead, matching the gateway path. Covered: harbor, postgres (external), qdrant (external), kubernetes (control-plane endpoint), vpn, seaweedfs, bootbox, monitoring (grafana, alerta, oidc client), and system/bucket (adds the cozy-lib subchart symlink it lacked). The guard is placed at the point each resource is actually emitted, so standalone renders, no-exposure tenants and the kubernetes no-etcd beacon path are unaffected. The post-delete cleanup hooks are left unguarded so an empty apex cannot block uninstall. Behavior change: a tenant with an empty injected apex and no host override that enables one of these components now fails its render (HelmRelease NotReady) instead of deploying a broken hostname. In practice the apex is always injected (root has root-host, children inherit it), so this only trips a genuinely misconfigured tenant. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
8b20260105 |
feat(talos): build the installer and matchbox for arm64
The package pinned linux/amd64 because its assets were amd64 only. With arm64 profiles it follows PLATFORM like every other image. The installer is published as one index under every tag. skopeo cannot assemble an index from the imager's docker archives, so the archives go into one OCI layout and a small script writes the index there for skopeo to push. buildx could build it from OCI layout sources, but the release job builds on the classic docker driver, which has no such source. matchbox carries the kernel and initramfs of both arches whatever PLATFORM is, rather than those of TARGETARCH: the node matchbox runs on is not the arch of the machines it boots, so a per-arch image would hand an amd64 machine an arm64 kernel whenever the pod lands on an arm64 node. The amd64 assets stay at their old paths, and a group keyed on the arch iPXE reports sends arm64 machines to the arm64 ones. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
e62f510faf |
chore(deps): move the Kubernetes libraries to v0.37.1
The root module's k8s.io/* move from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, its release for Kubernetes 1.37. Two things held them back: the apimachinery fork, which had to be rebased onto every minor, and the watch handler that from k8s.io/apiserver v0.36 panicked on the initial-events-end bookmark the aggregated storages sent to plain watches. Both are gone. The rest of the upgrades are what these releases require: cel-go v0.29.2, the etcd client v3.7.0, kube-openapi, structured-merge-diff and the x/ modules. code-generator moves with the libraries. Regenerating the strategy CRDs that embed core types adds the Kubernetes 1.37 fields and follows one removal: the alpha PodSpec field workloadRef became schedulingGroup upstream. Once the new CRD is applied, the apiserver prunes workloadRef from every stored strategy it serves, and the object's next write erases it from etcd. It only ever took effect behind an alpha feature gate. The gateway README and the tenantgateway CRD tests cite the apiextensions-apiserver list-type ratchet at the pinned version; it is byte-identical in v0.37.1, so only the version they name changes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
scooby87
|
cffe509dcf |
feat(monitoring): per-tenant OTLP collector (tracing phase 2) (#3763)
## What this PR does Phase 2 of distributed tracing (design cozystack/community#38, epic #3761), built on the phase-1 backend (#3762, merged): a per-tenant **OpenTelemetry Collector** — the app-facing OTLP ingest gateway. It lives in the tenant namespace (like a per-tenant vmagent), so app→collector traffic is intra-namespace and never crosses the tenant boundary. No OTel operator exists in the platform, so this is a plain Deployment + ConfigMap + Service (the fluent-bit pattern). ## Changes - **`templates/vtraces/collector.yaml`** — OTLP receivers (gRPC `:4317`, HTTP `:4318`) → `memory_limiter` + head sampling (`probabilistic_sampler`, default 10%) + optional `attributes/redact` (PII/secret control point) + `batch` → `otlphttp` exporter to the `tracingStorages` backend (`vtinsert:10481` cluster / `vtsingle:10428` single, endpoints per upstream docs). PSS-restricted securityContext, digest-pinned image via `cozy-lib.image`, config-checksum roll. - **app-facing Service `otel-traces`** (4317/4318). - **`tracingCollector` values** — `enabled` / `replicas` / `samplingPercentage` (schema-bounded 0-100) / `redactAttributes` / `resources`. A **fail-guard** rejects an enabled collector fronting more than one `tracingStorages` backend (it exports to a single backend by design). - **`otel-traces` WorkloadMonitor** and dashboard resource-map entry, rendered under the collector's own gate. The same resource-map Role also gets the pre-existing `alermanager` typo corrected to `alertmanager`, so the alertmanager WorkloadMonitor is readable from the dashboard. - helm-unittest coverage for the collector and its WorkloadMonitor; regenerated `values.schema.json` / `README.md` / `monitoring-rd` ApplicationDefinition. ## Verification - `helm unittest` — `system/monitoring` 110, `extra/monitoring` 15, all pass. - `helm template` renders cleanly; `make generate` idempotent. - Live check on a test cluster (single-mode backend): the collector starts with `redactAttributes` containing a numeric, a colon-bearing and an empty key; an OTLP/HTTP span sent to `otel-traces:4318` is accepted and is queryable from the backend through the Jaeger API, with the redacted attributes removed and the others kept. - Local `/branch-review` — **LGTM** (one blocker + four recommendations fixed, each with a test: multi-backend guard, memory_limiter cgroup limit, configurable replicas, sampling bounds, trust-boundary wording). ## Scope / deferred (epic #3761) `AccountID`/`ProjectID` header injection for the shared-central topology (phase 4) and engine integrations (phase 3) are follow-up PRs. ### Screenshots N/A, no UI change. ### Downstream repositories The diff touches only `packages/extra/monitoring`, `packages/system/monitoring` and `packages/system/monitoring-rd`. The website's monitoring reference page is regenerated from the package README by the release bot, and the Terraform provider models only the tenant's `monitoring` flag, not these values, so no follow-up is needed. - [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 feat(monitoring): deploy a per-tenant OpenTelemetry (OTLP) collector for traces once a VictoriaTraces backend is configured. Applications send OTLP to the in-namespace `otel-traces` Service (gRPC `:4317`, HTTP `:4318`); the collector applies head sampling (`samplingPercentage`, default 10%) and optional attribute redaction (`redactAttributes`) before exporting to the backend. Off by default (no backend configured). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable trace storage in cluster or single-instance modes, with retention settings and Grafana datasources. * Added an enabled-by-default OpenTelemetry collector with configurable replicas, sampling, attribute redaction, and resource limits. * Added workload monitoring and automatic resource recommendations for configured trace components. * Added readiness health checks for both tracing backend modes. * **Bug Fixes** * Corrected dashboard permissions for alert monitoring and trace collector resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
368d00b5d4 |
test(e2e): remove unused monitoring suites and audit BATS runners (#4019)
## What this PR does I removed the two monitoring OIDC BATS suites that had no runner after the Chainsaw migration, and brought the runner audit and wrapper OIDC test from [#3778](https://github.com/cozystack/cozystack/pull/3778). Also added checks for audience mapper and `OIDC_MODE=System`. The named `KeycloakRealmGroup` test documents coverage the document-count and kind assertions already provided. Runner audit reports unused BATS suites, missing files in runner paths, and unresolved Make variables (including `$(E2E_SUITE)`). Live Monitoring API admission, Grafana operator reconciliation and mode transitions are not tested here. ### Validation - `make -j4 unit-tests SHELL=/usr/bin/dash` passed. - Monitoring Helm tests: 53 system-chart tests and 10 wrapper-chart tests passed. - Runner audit: 12 cases passed under dash and Bats 1.14.0. The full shell syntax and BATS sweep passed under dash too. - Three chart mutations and seven runner mutations failed as expected. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` |
||
|
Alexey Artamonov
|
9d6fea7017 |
fix(monitoring): correct the alertmanager name in the dashboard Role
The Role listed the WorkloadMonitor as 'alermanager', so the real 'alertmanager' monitor was never granted: a tenant dashboard is forbidden from reading its status and the alertmanager health tile breaks. The test pins the correct name and the absence of the typo. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
60b1b05434 |
feat(monitoring): add per-tenant OTLP collector for distributed tracing
Applications need one in-namespace OTLP endpoint that also enforces
sampling and redaction before spans reach the VictoriaTraces backend.
No OpenTelemetry operator ships with the platform, so the collector is
a plain Deployment, ConfigMap and Service ('otel-traces', gRPC 4317,
HTTP 4318) in the tenant namespace; app-to-collector traffic therefore
never leaves the tenant, and isolation comes from the existing tenant
network policies.
The collector renders only when it is enabled and a tracingStorages
backend is configured. Tracing is off by default, and an empty backend
list is a clean off-state rather than a render failure; an app exporter
that finds no Service simply retries. It writes to the first backend
only and rejects a list with several, instead of silently dropping
spans for the others.
redactAttributes is the PII and secret control point, so its keys are
rendered quoted and empty keys are dropped: an unquoted key with YAML
structural characters, or an empty one, makes the collector fail at
startup, and a key YAML re-types (numbers, booleans, null) silently
escapes redaction. replicas has a lower bound of 1 so a bad value fails
schema validation instead of the whole release at apply time; a zeroed
collector is expressed with enabled: false.
The collector gets a WorkloadMonitor and a dashboard-resources Role
entry under the same render gate, so it is part of tenant workload
accounting like every other workload in the chart.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Aleksei Sviridkin
|
2d2186d1f0 |
feat(monitoring): VictoriaTraces backend + Grafana datasource (tracing phase 1) (#3762)
## What this PR does Phase 1 of distributed tracing (design: cozystack/community#38; epic: #3761): a **VictoriaTraces backend and Grafana traces datasource** in the monitoring stack, mirroring the logs (`VLCluster`) implementation. No new operator: `VTCluster`/`VTSingle` already ship in victoria-metrics-operator. - **`tracingStorages`** values in `extra/monitoring` (annotated, schema-generating) and `system/monitoring` (defaults), per-entry `mode: cluster|single`. **Empty by default, so tracing is off out of the box**: nothing produces spans until the collector ships, so no backend is provisioned until an operator adds an entry. - **`templates/vtraces/vtraces.yaml`**: `VTCluster` (default) or `VTSingle` (edge/dev/small); `retentionPeriod` plus optional `retentionMaxDiskSpaceUsageBytes`; a release-scoped **PVC cleanup label**, reused by the existing post-delete hook, so no hook change is needed. - **Render-time validation** (`_helpers.tpl`): the installed VictoriaTraces CRDs are the trimmed ones, so a bad value would otherwise reach the operator and fail after admission. Each `tracingStorages` entry is checked before anything renders: `name` must be an RFC 1123 label of at most 42 characters (the `vtstorage-<name>-<hash>` pod revision label is capped at 63) and unique across the list, `mode` must be `cluster` or `single`, `storage` must be a positive quantity (a zero size makes the operator mount an EmptyDir in single mode, and a negative one fails when the PVC is created), and `retentionDiskUsageBytes` must be a VictoriaMetrics `BytesString` (the operator rejects the Kubernetes-quantity `8Gi` grammar that `storage` uses). The `storage` field is also typed as a quantity in the schema, and the rendered name is quoted so a numeric name stays a string. - **`templates/vtraces/grafana-datasource.yaml`**: a Jaeger-typed traces datasource per entry (built-in Grafana core datasource, no plugin). - **VPA**: a `VerticalPodAutoscaler` for each trace component (`vtinsert`/`vtselect` Deployments, `vtstorage` StatefulSet, `vtsingle` Deployment), matching the metrics/logs sizing, with `updateMode: Initial`. - **Readiness gate**: `VTCluster`/`VTSingle` `healthCheckExprs` on `status.updateStatus`, alongside the existing `VLCluster` gate. - **VictoriaLogs datasource fix** (`templates/vlogs/grafana-datasource.yaml`): the range over `logsStorages` emitted no `---` between documents, so two or more entries collapsed into one document and only the last datasource survived. Each entry is now its own document. - **Tests and schema**: helm-unittest suites for off-by-default, cluster vs single, datasource type and both query URLs, retention, PVC label, VPA targets, every validation guard with its boundaries, and per-entry vlogs datasource documents, plus regenerated `values.schema.json`, `README.md` and the `monitoring-rd` ApplicationDefinition. #### Upgrade impact With default values the only rendered object that changes is the `monitoring-system` HelmRelease: it gains the `VTCluster`/`VTSingle` `healthCheckExprs` and `tracingStorages: []`. Flux evaluates those expressions only against objects in the release inventory, so they are inert while tracing is off. No VictoriaTraces object is created on upgrade. Installs with two or more `logsStorages` entries additionally gain the `GrafanaDatasource` objects that were previously dropped (see the VictoriaLogs fix above); nothing else under VictoriaLogs or VictoriaMetrics changes. #### Verification - `helm unittest`: `system/monitoring` (89 tests) and `extra/monitoring` (11 tests) pass. Each validation guard and each branch of the zero-storage pattern was mutation-checked: reverting it turns its own test red. - `helm template` renders `VTCluster`/`VTSingle`/datasource/VPA cleanly. `tracingStorages: []` (the default) renders nothing. An invalid, duplicate or over-long `name`, a zero or negative `storage`, or a bad `retentionDiskUsageBytes` (`8Gi`) is refused at render with an actionable message; a numeric `name` renders as a string. `make generate` is idempotent. - **Datasource URLs verified against upstream docs** ([VictoriaTraces and Grafana](https://docs.victoriametrics.com/victoriatraces/querying/grafana/)): cluster `vtselect:10471/select/jaeger`, single `vtsingle:10428/select/jaeger`. `jaeger` is the stable, built-in Grafana datasource; the Tempo Query API (`/select/tempo`, `type: tempo`, VictoriaTraces v0.9.4+) has a richer TraceQL UX but is upstream-experimental, so Jaeger is the default. Both URLs are pinned by unit tests. - The `retentionDiskUsageBytes` pattern, the zero-storage check and the name limit were checked against victoria-metrics-operator v0.68.4 (`BytesString.UnmarshalJSON`, `isStorageEmpty`, the `vtstorage-<name>` StatefulSet name) and the Kubernetes `ControllerRevisionName` naming. #### Scope / deferred (tracked in #3761) Per-tenant OTLP Collector, engine integrations, shared-central topology, and docs are follow-up PRs. This PR intentionally does **not** enable the OTLP/gRPC listener: apps can use `vtinsert`'s native OTLP/HTTP, and the gRPC listener lands with the collector. Builds on the now-merged design proposal cozystack/community#38. ### Screenshots Not a UI change. ### Downstream repositories Walked against the diff, no box ticked yet: - cozystack/website: `monitoring` is in the website `Makefile` `SERVICES` list, so its reference page is regenerated from `packages/extra/monitoring/README.md` automatically on the next stable tag. A user guide for tracing belongs to the docs follow-up in #3761; no website issue or PR exists for it yet. - cozystack/terraform-provider-cozystack: the provider has no `Monitoring` resource (only the tenant's `monitoring` flag), so the new schema field does not reach it. - The remaining repositories in the trigger map are not reached by this diff: no platform values, variants, installer values, `hack/` tooling, node prerequisites, ApplicationDefinition semantics or telemetry names change. - [ ] 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 feat(monitoring): add a VictoriaTraces backend and a Grafana traces datasource to the monitoring stack (distributed tracing, phase 1). Tracing is off by default; enable it per tenant by adding a `tracingStorages` entry (`cluster` or `single` mode). Also fix the VictoriaLogs Grafana datasources: with two or more `logsStorages` entries, every entry except the last was silently dropped, so existing installs with several log stores gain the missing datasources on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable trace storage with cluster or single-node deployment modes. * Supports retention periods, persistent storage sizing, storage classes, and optional disk-usage limits. * Automatically configures Grafana tracing data sources for each enabled trace backend. * Monitoring readiness reflects the health of configured trace storage services. * Invalid trace storage settings are rejected during configuration. * **Bug Fixes** * Every VictoriaLogs storage now gets its own Grafana datasource; with several log storages, all but the last were previously dropped. * **Tests** * Added coverage for storage rendering, retention settings, validation guards, Grafana endpoints, health checks, and cleanup behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3800f63513 |
fix(ingress): trust CF-Connecting-IP only from Cloudflare ranges
cloudflareProxy passed the Cloudflare ranges to the controller under set_real_ip_from, which is an nginx directive and not an ingress-nginx ConfigMap key, so the controller dropped it without a warning. With use-forwarded-headers on, the realip module then trusted every source from the default proxy-real-ip-cidr of 0.0.0.0/0, and any client that reached the load balancer directly could set CF-Connecting-IP to an address of its choice. That address is what whitelist-source-range checks, so the tenant whitelist, and any per-Ingress one, could be walked past with one header. proxy-real-ip-cidr is the key the controller reads for the trusted ranges. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
16d23c8a53 |
test(monitoring): preserve OIDC coverage without live BATS suites
Rendering the inner chart alone cannot catch a wrapper dropping tenant OIDC settings. The audience mapper and System-mode reconcile flag also need assertions before the unused live suites are removed. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Alexey Artamonov
|
e8779e479c |
refactor(monitoring): keep the HelmRelease rationale in template comments
The timeout, waitStrategy and healthCheckExprs rationale in the monitoring-system HelmRelease was written as `#` comments, which Helm renders verbatim, so every tenant's HelmRelease carried about thirty lines of prose. Move it into template comments like the traces paragraph beside it: the reasoning stays in the chart source and the rendered object is unchanged apart from losing the comments. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
2682185bff |
docs(monitoring): warn that renaming a tracingStorages entry replaces its backend
A tracingStorages name is the identity of everything the entry renders: the VTCluster/VTSingle, its Services and PVCs, and its Grafana datasource. Renaming an entry therefore deploys an empty backend and removes the old one, exactly like changing mode, but only mode said so. Give name the same treat-as-immutable note, and state the limits the render guard enforces. Reword the neighbouring mode and retentionDiskUsageBytes descriptions without arrows and dashes, which render awkwardly in the dashboard and in the generated README. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
37789e1768 |
fix(monitoring): reject zero storage and over-long names in tracingStorages
Two tracingStorages values render cleanly and only fail once the operator
acts on them.
- A zero storage is silently destructive in single mode: the operator's
isStorageEmpty gate checks IsZero() and mounts an EmptyDir instead of a
PVC, the VTSingle reports operational and the release goes Ready, and every
stored span is lost on the next reschedule. Type the storage field as a
quantity so the schema refuses non-quantities at admission, and refuse
every zero spelling ("", 0, 0Gi, 0.0, .0, 0e3, 0m, +0, -0Mi) in the render
guard, since the quantity pattern accepts those.
- A name over 42 characters wedges cluster mode: the operator names the
StatefulSet vtstorage-<name>, and each pod gets the label
controller-revision-hash: vtstorage-<name>-<hash>, whose value is capped at
63 with up to 10 characters of hash. Past that no pod is created, the
VTCluster goes failed and the release stays not-Ready under retries: -1.
Cap the length in the same guard.
- Quote the rendered name, so a numeric name such as 2024 stays a string
instead of failing the apply on metadata.name.
- Rewrite the helper's rationale so it matches the trimmed CRDs and the
pinned operator, and state in the mode field description that the API
cannot enforce immutability on an array-item field, so the note is the only
guard.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Assisted-by: LLM
|
||
|
Alexey Artamonov
|
05eebbdf4f |
refactor(monitoring): keep tracing rationale in template comments
The `#` prose in the traces datasource and in the HelmRelease health-check
block is not stripped at render the way `{{/* */}}` is, so it shipped into
every tenant's rendered GrafanaDatasource and HelmRelease. Move it into
template comments: the reasoning stays in the chart source but no longer lands
in the emitted objects.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Assisted-by: LLM
|
||
|
Alexey Artamonov
|
f20bd7b334 |
fix(monitoring): validate tracingStorages entries and flag mode/retention hazards
The installed VictoriaTraces CRDs are the trimmed ones (crds.plain: false), so their spec is x-kubernetes-preserve-unknown-fields and a bad value reaches the operator instead of being refused by the API server. Catch the ones a render guard can, and document the one it cannot. - name is used verbatim as the VTCluster/VTSingle and GrafanaDatasource metadata.name, so an invalid name is a late admission error and a duplicate silently overwrites the first entry (last write wins in the range). Guard it against an RFC 1123 label and a uniqueness set. - retentionDiskUsageBytes maps to the operator's BytesString field, whose UnmarshalJSON rejects anything outside ^[0-9]+(kb|mb|gb|tb|KB|MB|GB|TB|KiB|MiB|GiB|TiB)?$, notably not the Gi grammar of the sibling storage field. The operator then marks the CR failed and the release fails well after the value was written. Refuse it at render with the upstream pattern instead. - changing mode re-identifies the object instead of reconfiguring it, so switching single to cluster prunes the VTSingle and the operator garbage-collects its PVC, and cluster to single (or dropping the entry) leaks the vtstorage PVCs until the release is deleted. A stateless template cannot see the prior mode, so record the loss and the one-way migration in the field description. The name, uniqueness, mode and retention checks share one pass in a helper; the inline mode guard in vtraces.yaml moves there. The retention description also notes that the cap is per vtstorage replica in cluster mode. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
796340fc79 |
fix(monitoring): correct stale 'render-rejected when empty' comments
The healthCheckExprs comment (and the mirroring test comment) still claimed tracingStorages is render-rejected when empty and that the 'Ready with no trace store' hole cannot be reached, but defaulting tracing off removed that guard and made empty the default. Rewrite both to state the actual behaviour: empty is the default (tracing off), so the exprs are usually inert (Flux evaluates them only against CRs present in the inventory), and a Ready release with no trace store is intentional and harmless because nothing produces spans without a collector, so no fail-on-empty guard is needed. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
00ff19ec5c |
fix(monitoring): default tracing off and defer fail-on-empty guard
tracingStorages had two defaults (extra + system), so 'tracingStorages: null' silently backfilled the inner default and a VTCluster rendered anyway, while 'tracingStorages: []' aborted the whole monitoring release. There was no clean 'off', and the default shipped a 6-pod VTCluster per tenant for a pipeline that nothing writes spans into yet. - Default tracingStorages to [] in both packages/system/monitoring/values.yaml and packages/extra/monitoring/values.yaml, so absence has nothing to fall through to and the generated schema advertises the shipped (off) state. - Drop the fail-on-empty guard in templates/vtraces/vtraces.yaml. Nothing produces spans without a collector, so an absent backend drops no data. The guard belongs with the collector, where it becomes correct: an enabled producer with no backend really would drop spans, the logsStorages #3181 rationale. - healthCheckExprs stay unconditional (Flux evaluates them only against CRs present in the inventory, so they are inert when nothing renders). - Tests: replace the empty-fails case with off-by-default (renders nothing); set tracingStorages explicitly where cases relied on the old default. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
b24c3e1bc4 |
test(monitoring): assert VTCluster/VTSingle readiness-gate entries
helmrelease_test.yaml pinned only healthCheckExprs[0] (VLCluster), not the VTCluster [1] and VTSingle [2] entries that hold the release not-Ready until a configured trace store reports operational. Assert both entries (apiVersion/kind/current/inProgress/failed) so a dropped/reordered block or a typo'd CEL expression fails a test instead of silently regressing the gate. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
3774e09159 |
fix(monitoring): reject unsupported tracingStorages mode
tracingStorages[].mode accepted any string, so a typo (e.g. 'singel') silently rendered a VTCluster instead of failing. Add a render guard that fails on anything but cluster|single, restrict the schema property with a TracingMode enum, and add a helm-unittest case asserting the rejected mode. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Alexey Artamonov
|
c2fce3d41d |
feat(monitoring): add VictoriaTraces backend + Grafana datasource for tracing
Phase 1 of the distributed-tracing design proposal (cozystack/community#38): a per-tenant traces backend in the monitoring stack, mirroring the logs (VLCluster) implementation. No new operator: VTCluster/VTSingle already ship in victoria-metrics-operator. - tracingStorages values (extra + system), per-entry mode cluster|single - templates/vtraces/vtraces.yaml: VTCluster (default) / VTSingle (edge/dev), #3181-style empty-list render guard, retentionPeriod + optional disk cap (retentionMaxDiskSpaceUsageBytes), release-scoped PVC cleanup label - templates/vtraces/grafana-datasource.yaml: Jaeger-typed traces datasource - extra/monitoring readiness gate: VTCluster/VTSingle healthCheckExprs on status.updateStatus, alongside VLCluster - helm-unittest suites (required/empty-guard, cluster-vs-single, datasource, PVC label) + regenerated values.schema.json / README / ApplicationDefinition The per-tenant OTLP Collector, engine integrations, shared-central topology and docs are out of scope and tracked in the tracing epic. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM |
||
|
Myasnikov Daniil
|
bc8c54d6e6 |
feat(kubernetes): boot tenant worker disks from a shared golden Talos image
Every tenant worker VM imports the same Talos raw image from the Image Factory over HTTP, once per worker: a 20-worker cluster fetches the identical ~4.15 GiB artifact 20 times, and each fetch is a live dependency on the Factory at exactly the moment a node joins. packages/system/kubernetes-worker-image is a new opt-in catalog that holds one golden DataVolume per (schematicID, version) in cozy-public, named talos-worker-<schematicID>-<version>. A worker pool opts in with osImage.builtin on its own KubernetesNodes resource, and its disk-system DataVolume then carries source.pvc pointing at that golden, so CDI performs a storage-layer CSI clone instead of an HTTP import -- one import per flavor, shared by every worker of every tenant. Confirmed on a LINSTOR-backed cluster: the worker disk is a CSI clone plus a resize, with no host-assisted upload pod. Both halves are off unless asked for. The catalog is registered through the platform's optional-package helper and disabled by default, so a cluster that does not enable it creates no golden and keeps importing over HTTP. The disk source is derived entirely from osImage, which is absent by default, so a pool that does not set it renders byte-identical to the pre-feature chart and adopting the feature never rolls existing workers. Editing osImage on an existing pool re-images that pool, as changing any other image reference does. What the render refuses, rather than degrading into a silent fallback: - A pool whose golden has no catalog entry, or whose golden terminally failed to import. A golden that is merely mid-import is not an error; CDI holds the worker disk until the source populates. - A pool whose StorageClass differs from its golden's. CDI cannot CSI-clone across classes and falls back to a host-assisted copy over the pod network, which is the transfer this feature exists to remove. An empty class on either side means the cluster default and is resolved before the comparison, and a pool that has already cloned reads its class from its own bound PVC rather than from today's default -- the golden's side is frozen the same way, and an admin moving storageclass.kubernetes.io/is-default-class must not fail the release of a pool whose disks are all on the right class. That resolved class is then written onto the worker disk, so the disk binds where the guard checked instead of on whatever the default has become by then -- reading one class and binding another is how a pool at an empty storageClass passed the comparison and still took the host-assisted copy. The HTTP path leaves an empty class to the cluster as before, which is what keeps its render, and the KubevirtMachineTemplate hash named after it, byte-identical. - A diskSize below the golden's storage size, which a clone target cannot be. - A schematicID that is not a plain lowercase alphanumeric identifier, or a version that is not a vMAJOR.MINOR.PATCH release. Both are interpolated into a DataVolume name and an image URL, and values.schema.json types them as bare strings, so a malformed one otherwise reaches the render. The catalog applies the same patterns, since both sides build the same name from the same fields. - An images[] entry edited after its golden exists. A DataVolume spec cannot be patched in place, so the message names the entry and the supported move -- a new (schematicID, version) -- instead of leaving the API server to reject the patch on the next upgrade. Goldens carry helm.sh/resource-policy: keep, so dropping an entry stops managing a golden rather than deleting it: a missing golden aborts the whole Kubernetes HelmRelease of every tenant pinned to it, not just the disk. Each cloned worker disk annotates the golden it came from and the pool that owns it, so "does anything still clone this?" is answerable before a manual delete instead of after. A builtin pool needs a StorageClass whose volumes are reachable from any node, in practice a replicated/DRBD one. The golden in cozy-public has no consumer and binds as soon as it imports, wherever CDI's importer landed, while a worker disk binds where its VM is scheduled, and nothing here arranges for those to be the same node. The platform ships cloneStrategyOverride: csi-clone, which rules out the host-assisted strategy that would cross nodes. What a node-pinned class does in that situation is not measured, so values.yaml states the requirement and not a symptom. ComputePlane pools forward only the per-pool shape fields, and osImage is excluded there by name: the worker OS image, its Talos installer and its Image Factory are operator decisions about the platform's own images. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Alexey Artamonov
|
80d32b3ab7 |
fix(keycloak): complete realm-name coverage and guard non-string names
The enable-update-email hook Job in keycloak-configure still pinned REALM=cozy, so on a custom realmName it waited on a realm the chart never provisions and the cold install timed out — the one seventh consumer left behind, and the only value nobody could set without breaking the install. A bare-integer or YAML-reserved realm name reached the printf %s that builds the derived keycloakInternalUrl as a Go int, rendering a "%!s(...)" format error into the URL; coerce the name to a string once at the source. Document that renaming the realm on a running cluster provisions a fresh empty realm and orphans the accounts in the old one, and note in the OIDC tenant and grafana guides that the realm name is now the configurable default. info/Makefile carried NAME=etcd, so cozyhr drove the wrong release from that package; correct it while touching the file. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
44a9a8e735 |
feat(platform): make the Keycloak realm name configurable
The realm name "cozy" was hardcoded in keycloak-configure (which owns the realm, its clients and the cozystack-cluster-admin roles), in the dashboard and linstor-gui gatekeepers, in the info kubeconfig, and in the Grafana and managed-Kubernetes apiserver OIDC issuers. Operators could not rename the realm without patching vendored templates, which `make update` reverts. The apiserver OIDC issuer is already configurable via talm oidcIssuerUrl, so pointing it at a custom realm left every SSO component on "cozy" and broke login with an issuer mismatch. authentication.oidc.realmName (default "cozy") now rides the _cluster channel so every consumer resolves the same realm, and the default keycloakInternalUrl is derived from it so the realm is named in one place. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
a2db2aeb8c |
fix(seaweedfs): raise the number of buckets a volume server can serve
A volume server started with -max 0 takes a slot count derived from disk rather than a fixed one, replication 001 spends a slot on each of the two servers per logical volume, and a collection claims COPY_2 volumes whenever it grows. So the bucket ceiling is set by the product of the size limit and COPY_2, and moving either number alone moves it. The mechanism is upstream in MaybeAdjustVolumeMax (weed/storage/store.go) and findVolumeCount (weed/topology/volume_growth.go). Do not divide these out of the nominal PVC size, because the count follows the space the filesystem makes available and not the volume's advertised size: the shipped 10Gi volume PVC was seen to give nine slots per server at the upstream 1000 and 19 at 500, which puts 400 at 23 to 24 by the same division, or eight collections against two. The span is the observation's own, since the free space behind 19 at 500 fits more than one filesystem geometry and those divide differently at 400; eight either way, because a collection that cannot get a whole batch of COPY_2 keeps the volumes it did get and writes into them. The division itself is measured: on a 1 GB image the master reports max as the filesystem's available space over the limit in every run, 25 slots and nine collections at the 40 and 3 that scale to this pair. Two is what the platform's own cozy-backups bucket and a single tenant bucket already use. Past that every S3 PUT to a new bucket times out while the disk still reads as nearly empty, and no COSI object reports anything wrong, because the claim and the access are both granted. 400 and 3 make the budget 1200 MB per collection. Splitting the reduction across both numbers rather than taking it out of the size limit alone keeps the cost off the curve that grows: open descriptors track stored bytes divided by the limit, so a 10 TiB server keeps roughly 52k of them at this limit against 21k upstream, where moving the size limit alone would have put it at 105k. A .dat and an .idx stay open per volume; the .vif beside them is written and closed. The ceiling reached is the same either way. Both figures describe the shipped replicationFactor of 2. The master picks its growth count from the replication the filer asks for, so at 1 and 3 copies it reads COPY_1 and COPY_3 instead and this COPY_2 does not apply; lowering the size limit still raises the ceiling there. The size limit was out of an operator's reach: it lived in the data-plane chart, which is pulled by ExternalArtifact from a component that declares no install section, so a Package-level values override for it is dropped before any merge. The app chart now declares master.volumeSizeLimitMB and sends it on every release, and 400 is set there. The data-plane chart keeps upstream's 1000 as its fallback on purpose. A platform upgrade can re-render that release from its artifact before the parent has sent anything new, and a lower fallback would then reach the master first and drop the limit ahead of an operator's own setting. COPY_2 is sent by nothing and lives in the data-plane chart, so it does land in that window, where a grow costs 3000 MB against the 6000 MB it cost before. The app chart also writes maxVolumes into dataDirs on every release, and because dataDirs is a list Helm replaces the data-plane chart's copy wholesale, so the zero that makes the slot count auto-derived at all comes from there. Losing it drops the server to the subchart's fixed seven without failing any render. The field is bounded at both ends, because either end takes an instance down. Zero is not a smaller limit but an off switch: the volume server leaves its slot calculation untouched at zero, keeps reporting no capacity, and nothing is ever grown. Past 30000 the master does not start at all. The bound sits in the values schema rather than in a template guard, so Helm refuses the value before any template runs. The apps API does not: it reads that schema only for defaults, so it accepts a value out of range, and the release then fails to render. An explicit null gets past both bounds, because Helm drops the key before it validates, and would render an empty -volumeSizeLimitMB= that the master exits on at start, measured on the shipped image, so the template requires the value. No bound against the disk goes with it, though the field invites one. A limit at or above the volume's own size does not break an instance: MaybeAdjustVolumeMax floors the slot count at one, measured on 4.31 with a 255 MiB disk against a 1000 MB limit, which reported Max 1 and served a write it then read back. Such a server is capped at a single slot rather than dead, so refusing it at render would be a capacity policy that stops a working release on every reconcile, not only on the upgrade. The field's description carries the relation instead, where the value is chosen. The growth count is deliberately not exposed beside the limit. The master picks which of its four growth counts to apply from the replication the filer requests, and the filer takes that from replicationFactor, so a field wired to COPY_2 would do nothing at 1 or 3 copies while still appearing to be a setting. A knob that is silently inert for two of the three supported values is worse than no knob. Two further prices. A collection absorbs the budget before it needs another grow, so growth requests arrive five times as often as at the upstream pair. A collection also starts with half as many writable volumes, and a volume server takes the writes into each volume one at a time (syncWrite holds that volume's dataFileAccessLock), so a heavily parallel writer spreads over fewer volumes until the collection grows again; that holds for instances kept at 1000 too. And lowering the limit on an instance that already holds data carries the other one: its volumes between the new limit and the old go oversized on the next heartbeat and leave the master's writable set, and each affected collection needs one successful grow before its next write lands. Nothing is lost and the volumes stay readable; the room left inside them for further writes is given up and does not return as capacity. Those grows compete for one pool, and the pool is smaller than it looks. An oversized volume is charged no headroom, while one below the limit is charged all of its unfilled room; a grow then claims a whole COPY_2 batch and takes as much of it as is there, and the volumes it creates are empty and charge their headroom straight back. Measured on a local SeaweedFS 4.31 setup: two oversized collections with two free slots between them, the first to write took both, and the second stayed refused with the disk still reporting a quarter of itself free. Collections that need no grow are not free either, since their volumes below the new limit were created six at a time and each can hold back a whole limit. Also measured there, with the limit and batch scaled down: a server with free space for one batch per collection refused the grow its one affected collection needed, because a second, nearly empty collection held the room, and the same free space without it let the grow through. The minFreeSpacePercent reserve only comes into it on a server whose writable volumes have all passed the limit, where nothing is charged headroom: from 8000 MiB of filesystem upward, a 10Gi PVC measuring 9979, five percent is at least 400 MB, and such a server then refuses a grow only once its disk is down to the reserve. Everywhere else the headroom charge decides, and a refusal can arrive with the disk far from full whatever the reserve. Deleting a collection gives the room back a slot per volume, however little it stored, since the headroom charged to those volumes leaves with them. So, on a variant that runs platform migrations, the upgrade does not change the limit on an existing instance unless its operator already set one, which the previous chart accepted and ignored. Platform migration 57 writes master.volumeSizeLimitMB: 1000 into every existing SeaweedFS instance that has not chosen a limit, and 400 applies only to instances created from here on. It leaves alone only an instance whose master is not an object, which no v1 chart renders, and names that one in its log. The default variant runs no platform migrations, so a cluster there that installed SeaweedFS by hand gets no pin, and the document below gives the same patch to apply before upgrading. A chosen limit is a whole number the new chart accepts; anything else counts as not chosen and is replaced with 1000, which is what the previous chart ran in its place, since it ignored the key and the new chart would refuse to render it. A valid value already set is kept, and since the previous chart ignored it, the upgrade is when it starts to apply; the document below lists the ones that change the limit ahead of it. Raised values are listed as well: existing buckets keep writing, but every volume below the new limit is charged up to it, and on a local SeaweedFS 4.31 setup seven nearly empty volumes left no free slot once the limit went up tenfold, so a new bucket was refused. The value goes into the application's HelmRelease, whose spec.values is the application spec, for an instance created through the API and for one the tenant module renders alike. The tenant module never renders spec.values on that release, and a Helm upgrade leaves a key alone that neither manifest carries: measured with Helm 4.3.0 on client-side, server-side and forced server-side apply, and read in createPatch at the v4.1.1 that helm-controller v1.5.0 runs. Where the migration runs, it is a pre-upgrade hook of the platform release, and the cozystack-packages source every application chart comes from is a manifest of that same release, applied only after its hooks; so no instance meets the new chart unstamped, and a failed migration stops the platform upgrade before that source moves. That holds while the platform source is pinned by digest, as it is out of the box; one that follows a tag, a semver range or a branch lets source-controller move cozystack-packages on its own schedule. Until cozystack-packages moves, each patched release keeps its old chart, which ignores the key, since no v1 release reads it or closes the master object in its schema. An instance kept at 1000 still gains, because at a replication factor of 2 a grow now claims three volumes rather than six, so it has room for roughly twice as many new buckets. Lowering an existing instance is left to its operator, and docs/operations/seaweedfs-lowering-the-volume-size-limit.md is the check for it. It lists each server's volume files and adds one limit for every volume below the limit and a batch for every collection with no volume at or below 90 percent of it, since the master grows such a collection on its own schedule without a write; measured the same way, a collection with every volume in that band and no writes got two new volumes per server within one pass. Where each disk type is served by as many volume servers as the replication factor, as the shipped two at a factor of 2 are, each collection's volumes sit on every server of that type, and that bounds the claim from above without assuming how the collections grew. The master places a new volume on any server of the requested disk type with room rather than where its collection already lives, whichever group or pool that server belongs to, and it reads an empty disk type as hdd; under MultiZone it also grows each zone on its own. So the document keeps zoned instances, and any disk type served by more servers than copies, at 1000. BREAKING CHANGE: on the default variant, which runs no platform migrations, set master.volumeSizeLimitMB to 1000 before upgrading on every SeaweedFS instance that has no valid value and whose master is an object, with the step in docs/operations/seaweedfs-lowering-the-volume-size-limit.md. On the variants that run platform migrations, migration 57 does this. If the step is skipped, every such instance drops to 400 on the upgrade: each of its volumes at or above 400 MB leaves the master's writable set, and a bucket whose volumes have all crossed that line takes no writes until it can grow. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
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
|
4b2507b7a1 |
docs(gateway): align gateway prose with the code
The comment on the shared CEL string said what the hostname requirement is but not what it is worth per kind. Two of the three schemas leave spec.hostnames optional at every version the vendored bundle serves, so for HTTPRoute and GRPCRoute this policy is the only rejection a hostname-less route meets; tlsroutes requires the field at v1 and v1alpha3 and not at v1alpha2, so there the policy closes the gap for as long as v1alpha2 is served. State that where the requirement is stated. The comment on the port-80 allowedRoutes filter credited the cert-manager namespace entry for letting ACME complete, which the paragraph above it retracts: the solver route lives in the tenant namespace, and that is what keeps ACME working. The same sentence claimed the filter keeps app routes off port 80, and the Layer 1 paragraph of the gateway README claimed it too. It does not. A tenant that owns its Gateway is its own gateway namespace, so its app routes are admitted by this very filter; what it excludes is cozy-* and inheriting children. Both copies now say that. Nothing said why naming the apex on the redirect costs no coverage, which is the question a reader has once the route stops matching every host. It holds because of the other half of this change: the port-80 listener admits routes only from the tenant namespace and cert-manager's, and the tightened route policy requires every route there to declare hostnames inside the apex. An inheriting child does not widen that set either, and the comment now says so at the point the hostnames are chosen: the gateway hostname policy compares every listener against the host label of the Gateway's own namespace, so a child apex outside it never gets its listener, the Gateway write carrying it being refused at admission whole, in reconcileGateway and before this route is rendered, while a child apex under this one passes that check and the wildcard already covers it. The empty-apex guard in renderHTTPRedirect, and the test pinning it, both read as though the guard is what spares an operator an unreadable schema error. It is, but the condition stated for it named the number of attached routes, which does not enter into it. In HTTP-01 the listener set is built from route hostnames alone, so reconcileGateway never reads the apex ahead of the guard however many routes attach. DNS-01 and existingSecret render an apex listener, and any tlsPassthroughServices entry builds <service>.<apex>, so in those the operator meets the equivalent error against the Gateway first. State that condition once, at the guard, and let the test defer to it rather than carry a second copy that can drift; the test stays green either way because the fake client does not validate against the CRD schema. The e2e suite explained omitting the TLSRoute empty-list probe by the v1 schema rejecting an empty list. Both halves of that are true and the conclusion does not follow: the probe runs at v1alpha2, which marks spec.hostnames neither required nor minItems, so an empty list reaches admission there exactly as an absent field does. What makes the omission safe is that all three policies render one byte-identical CEL string, so the HTTPRoute pair proves that term already. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
900711567a |
fix(cozystack-basics): require route hostnames in tenant namespaces
The route hostname policy short-circuited on an absent field. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute, TLSRoute or GRPCRoute declaring no hostnames satisfied the first operand and was admitted without a single hostname compared against the tenant apex. An explicitly empty list reached the same outcome by a different path, because `all()` over an empty list is vacuously true. A route that names no hostname does not match nothing. Under Gateway API it inherits the hostnames of every listener it attaches to, so it covers whatever that listener covers. Require the field to be present and non-empty before validating it. All three policies rendered by the template share one CEL string, so HTTPRoute, TLSRoute and GRPCRoute pick the change up together. The denial message now names the missing-hostnames case, and the Gateway e2e grep moves to the part of that message common to every denial it issues. The Gateway e2e suite carried the one fixture that relied on the old behaviour: its parentRef probe declared no hostnames and is applied into the suite's tenant namespace. It now reads the apex from the namespace label and names a hostname under it, as the neighbouring cases in that suite already did. Radius is unchanged: the policy still matches only tenant-* namespaces. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5976ddf02f |
fix(gateway): normalise the GatewayClass name and refuse a blank one
The class lists arrive over the values channel comma-joined, so every reader trims each entry as it splits them. The scalar those entries are compared against was not trimmed anywhere. A class name carrying a stray space therefore failed `has` against its own padded self, and the two readers of that scalar disagreed about the same class: the gateway chart put the tenant on an edge-terminated class, while the cluster-wide ACME issuers still pinned their solver to a listener an edge Gateway does not render. That stops every HTTP-01 certificate in the cluster from issuing, with nothing in any status naming the cause. Normalise once at the single writer so both readers see a clean value, and keep a defensive trim at each reader. The tenant's own choice does not travel that channel — it comes from tenant.spec.gatewayClass — so it is trimmed where the gateway chart reads it. Unset and whitespace-only are different events and no longer share an outcome. Unset means the choice was not made, and falling back to the bundled class stays its documented behaviour. Whitespace-only means a choice was made and is garbage, in the field that decides where TLS terminates: resolving it quietly would take a tenant the operator meant to put behind an edge terminator and publish it directly, with a real certificate on a real address. Both the platform default and the tenant's own choice now refuse it by name. The kind check beside that refusal is part of the same fix, not a tidy up. Through Helm's values path an unquoted numeric scalar arrives as a float64, and the whitespace check compares it against a string, so `className: 123` dies at that comparison — one line before the trim — with an error naming neither the key nor the file. Refusing whitespace without checking the kind first would therefore have made the numeric case worse than it was. `kindIs "string"` is the right test because it never has to name the type that actually arrived. The order of the guard's steps is part of the guard. The value is read before anything normalises it, so a default that fires on any falsy value cannot launder 0 or false into the empty string and resolve them to the bundled class ahead of the kind check. Unset is the one case that still falls back, and each of those four inputs is pinned, so a normalisation drifting back ahead of a check fails rather than quietly covering only the loudest bad input. Also correct a cleanup log line: the wildcard Certificate is deleted for every mode that is not DNS-01, so naming HTTP-01 in that message is wrong for existingSecret and for edge alike. And state the listener name invariant plainly: the cap is 253, not the 63 of a DNS label, and the longest name the builders produce is 84. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
7851969448 |
docs(gateway): document GatewayClass selection and edge TLS termination
An operator setting up a class whose provider terminates TLS upstream had no page answering their questions: which platform values bound the choice, what a tenant writes to make it, what a move between classes does to certificates and to the replicated wildcard Secret, and how the mechanism fails when a configured class name matches nothing installed. Add docs/gateway-classes.md covering that surface. Describe the mode and its consequences in the chart README alongside the other three, including the one constraint stated nowhere else: the publishing tenant cannot select a class whose TLS termination differs from the platform default, because the cluster-wide ACME issuers derive their solver pin from that default alone. State the blast radius where it actually applies. The endpoints an edge Gateway stops publishing exist only where those routes are rendered at all, and both pages say what the transition does to the route conditions the controller owns, so a reader is not left inferring it from the code. The security model names the selector each listener really carries. The port-80 listener selects on the built-in kubernetes.io/metadata.name label, which kube-apiserver writes and a namespace cannot forge; the HTTPS and TLS-passthrough listeners select on namespace.cozystack.io/gateway, written by the tenant chart and the controller, so that guarantee does not extend to them. What closes the exposure is that Namespace is cluster-scoped with no tenant verb on it, and that the route-hostname policies bound what a labelled namespace could publish. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |