mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
597d966d7d688b7fbaec27632b4e87f61ba6ca1f
431
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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 --> |
||
|
|
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 ``` |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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 --> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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 |
||
|
|
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 |
||
|
|
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
|
||
|
|
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
|
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
487cafac3e |
feat(platform): let a tenant pick its Gateway class from an allowlist
Tenant Gateways were pinned to the bundled Cilium class. Give the platform a default in gateway.className, the set a tenant may choose from in gateway.tenantSelectableClasses, and the set whose provider terminates TLS upstream of the Gateway in gateway.edgeTerminatedClasses. A tenant picks its own with tenant.spec.gatewayClass, read only when it owns a Gateway; an inheriting tenant runs on its ancestor's. The selectable set is an allowlist rather than free choice because Tenant is a tenant-writable object while a GatewayClass is cluster-scoped: without it a tenant could attach its Gateway to a class an admin keeps for internal-only traffic. Naming anything outside the set fails that tenant's own gateway release and reaches no other tenant. Where TLS ends is a property of the class, so the gateway chart derives certMode from whichever class the tenant landed on instead of taking it as a separate input. The two therefore cannot be configured out of step. The publishing tenant cannot cross that line by itself. The cluster-wide ACME issuers point their HTTP-01 solver at its Gateway and are rendered from the platform default alone, so a mismatch would pin the solver to a listener that Gateway does not have. Those issuers now drop the sectionName pin when the default class is edge-terminated, letting the challenge attach by hostname instead. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
af19aa933c |
docs(security): record GRPCRoute in the route hostname policy set
The port-443 kinds set still excludes GRPCRoute, TCPRoute and UDPRoute. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. TCPRoute and UDPRoute additionally carry no hostname and no admission rule gates them. The gateway README described HTTPS and TLS-passthrough listeners as carrying different kinds. They carry the same one, which is what cilium#45559 requires. The threat model and the self-assessment enumerate which route kinds the hostname policies cover, and both listed two. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
70a675fd6b |
feat(cozystack-basics): gate GRPCRoute hostnames in tenant namespaces
The route hostname policy covered HTTPRoute and TLSRoute. GRPCRoute carries spec.hostnames too, and it can reach a tenant Gateway: the port-443 listeners pin allowedRoutes.kinds to HTTPRoute and TLSRoute, but the port-80 listener leaves kinds unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could therefore claim any hostname on the plaintext listener with nothing checking it. Tenants hold no gateway.networking.k8s.io RBAC, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The new rule is the HTTPRoute rule with grpcroutes in place of httproutes and the single served version the bundle provides; the coverage guard pins that list to the bundle like the others. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7ba22369d4 |
fix(cozystack-basics): match every served TLSRoute version in the hostname policy
The tenant route hostname policy names v1alpha2 alone while the vendored Gateway API bundle serves tlsroutes at v1, v1alpha2 and v1alpha3: three served versions against one named. The gap is not new. The v1.5.1 bundle this package shipped before and the v1.6.1 one it ships now serve the same three versions, with the same two marked deprecated, so nothing in this change created or widened it -- a bundle bump is simply when someone last opened the file. Coverage of the two unnamed versions rested entirely on request conversion. matchConstraints.matchPolicy is unset on both rules in this file and so defaults to Equivalent, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by failurePolicy: Fail, because nothing failed -- there was simply no match. Name all three served versions, which is the shape the HTTPRoute rule directly above already uses. The defect is a version list drifting from the bundle it must track, so a longer list on its own would not catch the next drift: add a check that reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, in either direction. Both rules get an exact-value assertion in the chart's own suite, so the coverage check and the pinned value fail for different reasons and neither stands alone. The header comment being replaced claimed TLSRoute stays at v1alpha2 through Gateway API v1.4-v1.5 and that its promotion to v1 was still upstream-only. TLSRoute has been served at v1 since v1.5.0, so that was already false, and false in the direction that hid this gap: a reader who believed it had no reason to look at the version list at all. Widening the rule then falsified every remaining place that restated which versions it matches -- the template's own header, the security-model layer list in packages/extra/gateway/README.md, and the name of the test case that asserts the value. Rather than resync three copies of a list that will move again, they now say which kinds are gated and point at the rule and its guard for the versions. A comment in the same test file now states only the behaviour it guards: an earlier form of the CEL used `... ? true : (...)`, which allowed routes in any tenant-* namespace whose host label was missing or scrubbed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
6fdb5e0840 |
docs(etcd): note that affinity is not applied to existing members
The operator applies spec.affinity only when it creates or replaces a member Pod; it does not reschedule running members. Document that on the `affinity` value so operators do not expect the default (or a newly set rule) to redistribute already co-located members until each is replaced. Addresses the non-blocking review notes on #3962. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
94614c021a |
fix(etcd): spread members across nodes with default soft pod anti-affinity (#3962)
## What this PR does The `etcd` chart hardcoded only a **soft** `topologySpreadConstraints` (`whenUnsatisfiable: ScheduleAnyway`) and never rendered `spec.affinity`, even though the `EtcdCluster` CRD supports it. On small clusters the scheduler therefore co-locates multiple members on one node; losing that node takes out quorum (or the whole cluster). We hit this in production: a single node reboot left several 3-member etcd clusters with 2–3 members on the downed node, and one tenant's control-plane etcd lost quorum until the node came back. This renders `EtcdCluster.spec.affinity` from a new `affinity` value: - **Default (value left empty):** the chart applies a **soft** (preferred) `podAntiAffinity` on `kubernetes.io/hostname`, so members spread across nodes on a best-effort basis. Soft on purpose — a hard rule would leave a member `Pending` when replicas exceed schedulable nodes, or when node-local storage pins a member to a node, so it is safe to default on. - **Override:** set `affinity` to any rule — e.g. a hard `requiredDuringSchedulingIgnoredDuringExecution` podAntiAffinity — to forbid co-location outright on clusters with enough nodes and non-node-pinned storage. A supplied value replaces the default cleanly. The existing `topologySpreadConstraints` block (and its `_cluster.scheduling.globalAppTopologySpreadConstraints` override) is unchanged. Generated artifacts (`values.schema.json`, `README.md`, and the embedded `openAPISchema` in `etcd-rd`) were regenerated with `make generate`; `make test` passes (18/18). ### Screenshots n/a — no UI changes. ### Downstream repositories Walked the trigger map against the diff: this only adds one values key to an app chart. No package is added/renamed/removed, `kind`/`plural` are unchanged, no installer values or release assets or variants change, and `README.md` is regenerated here (the website bot regenerates reference pages from it on release). No manual downstream follow-up is required. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(etcd): etcd clusters now spread their members across nodes by default via a soft podAntiAffinity, and expose an `affinity` value so operators can set a hard rule; previously a single node could host a quorum of members and take out the cluster when it failed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable pod affinity rules for etcd deployments. * By default, etcd members use soft anti-affinity to spread across hosts without blocking scheduling. * Custom affinity settings can override the default behavior. * Exposed affinity configuration in the etcd application schema and dashboard. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9858fb63ac |
test(etcd): cover default and overridden affinity rendering
The affinity value added in this branch introduced a two-branch conditional in templates/etcd-cluster.yaml (default soft podAntiAffinity vs. a supplied value) with no coverage. Add a helm unittest suite pinning both branches: the default preferred per-host rule, and that a supplied affinity replaces the default outright (no preferred rule leaks through). Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> |
||
|
|
c83a22e077 |
fix(etcd): spread members across nodes with default soft pod anti-affinity
The etcd chart only set a soft topologySpreadConstraints and never rendered spec.affinity, so the scheduler could place several members of a 3-node EtcdCluster on the same node; losing that node then took out quorum (observed in production after a single node reboot). Render EtcdCluster.spec.affinity from a new `affinity` value. When it is left empty the chart applies a soft (preferred) podAntiAffinity on kubernetes.io/hostname, so members are spread on a best-effort basis without ever leaving a member Pending on small or node-pinned-storage clusters. Operators can override the value — e.g. a hard requiredDuringScheduling rule — where node count and storage allow. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
bf38655ab5 |
docs(computeplane): nodeGroups allowlist drops unlisted fields, not pass-through
The nodeGroups docs still described the old denylist behaviour: "unlisted NodeGroup fields pass through to the wrapped chart unvalidated". After the switch to an allowlist (pick) both halves are wrong: fields go to the kubernetes-nodes pool chart (not the wrapped kubernetes chart), and anything the pick does not name is dropped, not passed through. A tenant that sets e.g. talos.version on a nodeGroup gets no error and no effect (accepted-but-inert). Correct the source @param and the hand-written README intro, regenerate schema/README/cozyrd, and refresh the stale "omit" wording in the splice test comment to say allowlist. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
3630842172 |
fix(computeplane): reject non-RFC-1123 pool keys at render time
The pool key becomes the pool HelmRelease name and its CAPI object names, which must be RFC-1123 labels, but only the length was guarded. An uppercase/underscore key rendered an invalid metadata.name the apiserver rejects, and a key containing ':' rendered invalid YAML that failed the whole module render -- a non-uniform, late failure next to the guarded length case. Add a render-time RFC-1123 guard on the key alongside the length guard (mirrors migration 56's child-name guard), and pin the length guard's derived remedy budget (15) in the overflow test so a wrong computation is caught, not just the presence of the error. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
c2ee58756e |
fix(computeplane): allowlist pool values instead of denylisting, derive length-guard prefix
The pool-value splice used omit (a denylist), which leaks every key it does not name. Since this module runs untrusted tenant code and the NodeGroup schema has no additionalProperties:false, a tenant could pass the platform-injected _cluster object (cluster-domain/oidc/dns01/proxy, delivered via cozystack-values and merged UNDER spec.values, so a tenant key wins) or nameOverride straight through to the pool chart -- and any future kubernetes-nodes top-level value would silently re-open the same hole. Switch to pick: pass through only the enumerated per-pool shape fields, so everything cluster-level or module-owned is dropped by construction (fail-closed for new fields). Also derive the length-guard's release-name prefix from $clusterRelease so it cannot drift from the name actually rendered. Regression-tested: _cluster and nameOverride no longer reach the child values; maxUnhealthy and the module wiring still do. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
34e801cb9e |
fix(computeplane): stop tenant pool values leaking cluster-level keys and overflowing release names
The per-pool child HelmRelease spliced the tenant nodeGroup dict verbatim (toYaml) after setting cluster/clusterReleaseName, and the NodeGroup schema has no additionalProperties:false. A tenant could therefore (a) set a duplicate cluster/clusterReleaseName key that wins the YAML merge and re-parents the pool onto a tenant-controlled cluster, or (b) pass talos/ version/images through a shape-only knob and pick the worker OS image, installer and kubelet version. omit those five keys at the splice so only the pool's own shape flows through (tolerated extras like maxUnhealthy still do). Separately, each pool renders a 38-char fixed release-name prefix against Helm's 53-char cap, so a nodeGroup key over 15 chars created a HelmRelease the apiserver accepts but Flux never reconciles, with no other signal. Guard the length at render time, next to the ingress-role guard, mirroring the migration overflow branch and the aggregated API's validateNameLength. Also refresh the generated values doc: the module now materialises its own default md0 and pool storageClass falls back to the kubernetes-nodes chart default, not the wrapped kubernetes app. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
d93e136e23 |
fix(computeplane): provision worker pools via kubernetes-nodes after Phase 2 split
Phase 2 moved worker nodeGroups out of apps/kubernetes into apps/kubernetes-nodes, but the ComputePlane module still passed nodeGroups to the wrapped kubernetes chart, which now ignores it: a fresh ComputePlane cluster came up with zero workers (and the pinned ingress-nginx addon stuck Pending), while existing ones were unreachable by migration 56 (it only adopts kubernetes-* releases, not computeplane-cluster). The module now emits one kubernetes-nodes child HelmRelease per nodeGroup, shaped like a natively created pool, and materialises the pre-split default md0 (roles: [ingress-nginx]) when nodeGroups is empty. The pool chart's clusterName gains an internal clusterReleaseName override so pools attach to the module's fixed computeplane-cluster release, which is off the aggregated API's kubernetes-<cluster> convention; the default keeps every API-created pool byte-identical. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
9d1d6808db |
test(seaweedfs,api): bound both Roles against a respelled rule
Each suite forbade a rule by naming it in full, and helm-unittest compares every field it is handed by value equality, so one respelling walks past. For seaweedfs that is not theoretical. Ungating the cnpg block and adding a verb to the rule (get,list,delete rather than get,delete) is the same grant and more, and the guard stayed green while a Client instance held delete on another app's Postgres cluster. Bound the Role by shape: a Client renders exactly two rules, both in the core group, and the cnpg grant is gated between them, so ungating it adds a third whatever its spelling. The rules themselves stay unpinned because the secret list is long and changes on its own schedule. For cozystack-api, add a count bound beside the existing spelling assert rather than in place of it. The two catch different things and neither covers the other: templates/rbac.yaml carries no template directives, so the list is pinned at exactly sixteen rules, while a grant swapped in for an existing one leaves the count where it was and only the named-rule assert sees it. The count is pinned from both sides — a dropped grant is the failure this suite is about, since the apiserver then returns forbidden and List silently drops the dropdown. Measured on all three: append, delete, and swap. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
93338cc434 |
refactor(computeplane): drop the in-cluster Flux pin
The fluxcd addon pin was a holdover from the #3150-era model where app charts rendered nested HelmReleases into the target cluster and needed an in-cluster helm-controller. Consumers now keep their releases on the management cluster and remote-apply via spec.kubeConfig, so the management Flux resolves charts and applies manifests — nothing inside the ComputePlane reconciles HelmReleases. This also decouples the module from the FluxCD addon removal (#3379). certManager and ingressNginx stay pinned: Certificate and Ingress objects land inside the cluster and need their controllers there. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
8fc8d23dfd |
fix(computeplane): address review round three
Gate the Proxied ingress Service on _namespace.etcd like the addon HelmRelease it fronts, so an etcd-less tenant with the addon pinned on does not get a selector-less Service (+ test). Correct the secrets framing: the tenant-visible surface is an allowlist (include: [] withholds everything), so drop the never-load-bearing exclude entry, the bats cross-check and the rename-coupling comments, and state the real mechanism. Fix the etcd prerequisite docs (an ancestor's etcd satisfies the wrapped chart; the awaiting-etcd beacon is the diagnostic), reword the tenant toggle to stop advertising the not-yet-shipped placement routing, require maxReplicas >= 1 on the ingress-role node group, and fix comment drift (Service name after the release rename, placement-hook label, package-guard lookup semantics). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
5a3d25b8f5 |
docs(computeplane): document the etcd prerequisite and unlisted-field pass-through
The tenant Kubernetes machinery — including the cluster-autoscaler that brings up the default scale-from-zero md0 group — is backed by the tenant's etcd module, so a ComputePlane without it provisions no workers and gives no diagnostic; state the prerequisite in the README. Also state plainly that unlisted NodeGroup fields pass through to the wrapped chart unvalidated (the schema generator cannot close the object), with a test pinning that behaviour. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
6dcf5373e7 |
fix(computeplane): let Flux derive both release names
The aggregated API cannot represent spec.releaseName: the conversion rebuilds HelmRelease specs without it, so the first tenant edit through the API dropped the module's explicit computeplane-module release name, let it default to the object name computeplane, and collided with the Helm storage record the cluster release owned — with the tenant chart writing the old name back every interval. Drop releaseName from both HelmReleases and derive names from object names, like every other tenant module. Kamaji now writes the admin kubeconfig to computeplane-cluster-admin-kubeconfig; the computeplane-rd secrets.exclude entry follows in the same change, and a bats test pins the exclusion to the name the cluster release actually produces so a future rename cannot silently re-expose the credential. Also add the check-release-name guard the other extra modules carry, and pin the fluxcd addon's distribution artifact to the flux-operator version vendored in system/fluxcd-operator instead of :latest. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
96842d4a94 |
fix(computeplane): validate nodeGroups and require an ingress-nginx role
nodeGroups was a bare object: typos surfaced as a stuck release deep in the wrapped chart instead of an admission error, and groups without the ingress-nginx role silently left the pinned ingress-nginx controller (and with it the ComputePlane ingress) Pending forever, since supplied groups are not merged with the wrapped chart's default md0. Reuse the kubernetes app's nodeGroups typedefs for the schema and refuse rendering when no group carries the role. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
4c3262a4ee |
feat(computeplane): deliver ComputePlane as a tenant module wrapping the kubernetes app
Add the operator-owned computeplane tenant module per the revised design (cozystack/community design-proposals/compute-plane): an extra/computeplane chart that provisions a single-tenant, Cozystack-managed cluster for untrusted-code apps by deploying the ordinary apps/kubernetes chart with operator-fixed values (fluxcd, cert-manager, ingress-nginx Proxied). The package re-declares apps/kubernetes as its own source-only component, so the module is self-contained; the cluster release is named computeplane so Kamaji writes the admin kubeconfig to computeplane-admin-kubeconfig — the contract consumed by placement: ComputePlane applications. The tenant holds no admin kubeconfig and can set cluster shape (nodeGroups) only. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
9c63df7545 |
build(monitoring): stop building and publishing the Grafana rebuild
Remove the custom Grafana image build now that the deployment runs on the upstream grafana/grafana image with catalog plugins. Drops the Dockerfile, the pinned image tag, the image targets in both monitoring Makefiles, and the build step from the root Makefile, so ghcr.io/cozystack/cozystack/grafana is no longer produced or published. Assisted-By: Claude Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
235d38dae1 |
Prepare release v1.6.0 (promoted from v1.6.0-rc.4)
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
|
d95685c427 |
Prepare release v1.6.0-rc.4
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
|
8355f3e36f |
docs(seaweedfs): correct the stale "D-wedged is adopted" comments
The naming guard now refuses every two-generation state, but two comments in extra/seaweedfs still described a D-wedged duplicate (zero ready replicas) as safe to adopt and rendering through. readyReplicas is a snapshot, not proof the duplicate never served, so it cannot be told apart from D-split and the guard refuses it too. Align the state summary in the seaweedfs template comment and the test header with the presence-only refusal behaviour. No classifier change -- comments only. Address review feedback from coderabbitai on packages/extra/seaweedfs/tests/fullname_override_test.yaml. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |