mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
2f294c65cd50e83bc97844f66896d25408017d48
6068
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Alexey Artamonov
|
2f294c65cd |
test(cozy-lib): cover every fail-closed guard of the keda.scaledObject helper
The helper validates ~10 required inputs but the suite only exercised three rejection paths (non-dict arg, missing scaleTargetRef.name, inverted bounds). Add a parameterised fixture that builds a fully-valid argument dict and drops exactly one required field per case, and assert the fail message for name, scaleTargetRef.kind, scaleTargetRef.apiVersion, query, threshold and serverAddress. The helper's rejection contract is now executable-documented in full rather than sampled. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
42a533fc50 |
fix(platform): inject networking.clusterDomain into the KEDA package
KEDA's clusterDomain is a vendored-subchart value baked into the metrics-apiserver gRPC target (keda-operator.<ns>.svc.<clusterDomain>:9666) and the operator's serving-cert SANs at render time. A chart template cannot derive a subchart value from _cluster, so the keda package hardcoded cozy.local — which silently breaks (NXDOMAIN, every DB HPA FailedGetExternalMetric) on any cluster whose domain is not the default. Inject the authoritative networking.clusterDomain into the shared cozystack-values Secret under the keda subchart key so the value tracks the platform config; the keda package keeps cozy.local as a standalone fallback. The key is ignored by packages without a keda subchart (no app/extra schema sets root additionalProperties:false). Add platform unittests asserting the Secret carries keda.clusterDomain and that it tracks a non-default networking.clusterDomain. Addresses the hardcoded-clusterDomain portability gap found in branch review. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
a7b5077da2 |
fix(postgres): floor the lag-brake frozen term so a KSM gap can't empty the query
The read-load query's base term is floored with `or vector(0)`, but the replication-lag brake's frozen additive term (count(kube_pod_labels) * target) was not. PromQL `+` is a set intersection, so if count(kube_pod_labels) is momentarily empty — a kube-state-metrics scrape gap or KSM rollout — the frozen term is empty and the whole `base + frozen*braking` sum collapses to empty, defeating the base term's own graceful degradation. KEDA's prometheus scaler (ignoreNullValues: true by default) then reads that as 0 and pins desired to minReplicaCount, ignoring real read load exactly when metrics are flaky. This hits the default config (maxReplicationLagSeconds: 30, brake active). Floor the frozen summand with `or vector(0)` so base + 0 = base, and assert the floor in the lag-brake unittest. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
573213ef8b |
test(postgres): assert the standby-only read-load selector in the query
Cover the load-bearing replica selector that both read-load queries depend on: the join must restrict to standby pods via label_cnpg_io_instance_role="replica", the exact key kube-state-metrics exposes for CNPG's cnpg.io/instance_role. A wrong key (e.g. a camelCase label_cnpg_io_instanceRole) would match no series, floor the read term to 0, and silently pin desired to the minimum - the suite previously never asserted the selector at all. Verified live on dev9 (CNPG 1.30.0) that the snake_case key is the one present in kube_pod_labels and the camelCase form does not exist. Adds the assertion for both the ReadConnections and ReadCPUUtilization paths. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
842a59a957 |
fix(postgres): correct the migration-dip docs and pin the KEDA metricType
Two review findings: - The enablement note claimed the two-phase `transition` flow avoids the count dip when enabling autoscaling on an existing cluster. It does not: helm-controller upgrades with Helm's three-way merge (not SSA), which keys off the old-vs-new manifest diff, not KEDA's live /scale value. At phase 2 the chart stops rendering the staged static count, so the field is deleted (no quorum -> CNPG defaults to 1, a brief collapse) or changed (quorum -> rebased to effectiveMin), then the warm KEDA restores it. Rewrite the docs to state the flow reduces (KEDA is pre-warmed, no cold-start) but does not eliminate the dip, and spell out the suspend + scale-subresource pin that actually makes the handoff dip-free. This is the SSA-vs-three-way-merge handoff the design proposal left for the PoC to settle. - Pin metricType: AverageValue on the Prometheus trigger in cozy-lib.keda.scaledObject. The desired = ceil(query / threshold) identity holds only for an AverageValue external metric; it was relying on KEDA's default. Make it explicit and assert it so a re-vendor or default change cannot silently break the arithmetic. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
87b26d6595 |
fix(postgres): only seed spec.instances when synchronous quorum needs it
Narrow the previous unconditional seed to a conditional one, keeping design
proposal §3 (chart omits the field, HPA is the sole writer) intact for the
default path. Without synchronous quorum (maxSyncReplicas=0) CNPG's defaulted
instances=1 is admissible, so the field is omitted exactly as §3 specifies and
KEDA owns it via /scale with nothing contested. Only when maxSyncReplicas>=1 -
where an omitted/defaulted instances=1 fails the admission webhook
("maxSyncReplicas must be lower than the number of instances") - does the chart
seed the quorum floor (effectiveMin) so a fresh Cluster is admitted; the constant
re-render keeps three-way merge from reverting KEDA's higher live value.
This confines the deviation to the case §3 did not consider (sync quorum), so no
design-proposal amendment is needed for the documented path. Verified live on a
dev cluster: the quorum render (instances=2, maxSyncReplicas=1) and the
non-quorum render (instances omitted) are both admitted by the CNPG webhook. Adds
a render test for the non-quorum omit path alongside the quorum seed tests.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
610bcb5df9 |
fix(postgres): seed spec.instances at the quorum floor under autoscaling
Under active autoscaling the chart previously omitted .spec.instances entirely,
letting the KEDA HPA own it via /scale. But CNPG defaults an absent
.spec.instances to 1, and its admission webhook rejects a Cluster where
maxSyncReplicas >= instances ("maxSyncReplicas must be lower than the number of
instances"). So a fresh database created with autoscaling.enabled: true AND
synchronous quorum (maxSyncReplicas >= 1) was rejected at admission before KEDA
could ever scale it to effectiveMin (= maxSyncReplicas+1) — the whole tenant
HelmRelease failed. The default maxSyncReplicas: 0 path was unaffected, which is
why it went unnoticed. Verified live: a rendered Cluster with maxSyncReplicas: 1
and an omitted instances is rejected by the dev-cluster webhook; with the seeded
floor it is admitted.
Render instances: effectiveMin under active autoscaling instead of omitting it,
so a fresh Cluster starts at a valid, quorum-satisfying count. Because the chart
emits the same constant floor on every reconcile, Helm's three-way merge is a
no-op on the field and never reverts KEDA's higher live value, so the HPA still
owns the runtime count. Update the enablement docs (a new DB can enable directly
even with synchronous replication; existing DBs above the floor rebase to it
unless the two-phase transition flow is used) and add render tests for the fresh
create + maxSyncReplicas>=1 case.
Addresses the fresh-create-with-quorum admission gap found in branch review.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
284f5e5543 |
fix(postgres): fail closed when autoscaling is enabled without KEDA, guard target
autoscaling.enabled is a tenant-facing knob, but KEDA is an optional platform package that is not installed by default. A tenant enabling autoscaling on a cluster without KEDA would render a ScaledObject against an unknown kind and the whole database HelmRelease would fail with an opaque `no matches for kind "ScaledObject"`. Guard the ScaledObject template on `.Capabilities.APIVersions.Has "keda.sh/v1alpha1"` and stop with an actionable message instead. Skipping the render is deliberately NOT an option: db.yaml already drops the static .spec.instances under active autoscaling, so a missing ScaledObject would let CNPG collapse the cluster to 1 instance. Document the KEDA prerequisite on the `enabled` field (README/schema). Also reject a non-positive autoscaling.target at render time: it is a plain integer now, but the desired-replica formula divides the read load by it, so 0 is undefined and negatives invert scaling. cozyvalues-gen has no minimum directive, so the bound is a template guard. Add unittests for both guards (a dedicated suite renders without the KEDA CRD so the fail-closed path is exercised; the main suite now advertises keda.sh/v1alpha1 via capabilities). Addresses IvanHunters (KEDA-absent) and CodeRabbit (non-positive target) review feedback on #3954. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
ec123649bf |
fix(keda): use the KEDA 2.20 metric name in the scaler-errors alert
The DatabaseAutoscalerScalerErrors rule queried keda_scaler_errors_total, which KEDA 2.20 no longer exposes (verified against a live 2.20.2 operator: it emits keda_scaled_object_errors_total for per-ScaledObject errors and keda_scaler_detail_errors_total for per-scaler errors). The alert would have silently never fired. Switch to keda_scaled_object_errors_total and add a unittest assertion pinning the metric name so a rename regresses loudly. Addresses CodeRabbit review feedback on #3954. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
f8fc2d99bf |
fix(postgres): make autoscaling.target an integer and drop the dead root metric
Two review findings on the generated API: - autoscaling.target was a resource.Quantity, which accepts suffixed values like `150m`. The chart splices target straight into the PromQL query and the KEDA threshold, where `150m` means 150 minutes to PromQL and is not a usable KEDA float threshold. Retype it as a dimensionless integer so the schema rejects suffixed input - helm unittest's schema validation now enforces it, and the existing autoscaling suite exercises the integer contract. - ConfigSpec carried a root `metric` field (and spec.metric in the postgres-rd schema) that nothing reads - the chart consumes only autoscaling.metric - so setting it silently did nothing. Remove the dead root param, keeping the ReadMetric enum that autoscaling.metric references. Regenerate types.go, zz_generated.deepcopy.go, values.schema.json, README.md and the postgres-rd schema; update the autoscaling tests to the integer target. Addresses CodeRabbit review feedback on #3954. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
ec06dafd25 |
feat(postgres): fold the replication-lag brake into the autoscaling query
Wire autoscaling.maxReplicationLagSeconds into the read-load query: while the max standby lag has exceeded the threshold within the cooldown window and the primary is actively writing WAL, the query returns currentInstances*target, pinning desired to the current count and freezing scaling in both directions (safer than pinning maxReplicas, which would still allow scale-down under lag). The lag term uses max_over_time as a hysteresis band; the write gate (WAL-record rate > 0) keeps an idle primary from tripping it. Metrics verified live on CNPG 1.30.0: cnpg_pg_replication_lag and cnpg_collector_wal_records exist and carry namespace/pod (the proposal's cnpg_pg_stat_replication_sent_diff_bytes does not). Also floor the read load at zero via `or vector(0)` so an idle cluster returns target rather than an empty result. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
ac82ce186c |
fix(keda): pin cozy.local domain and raise operator API throttle
The vendored KEDA chart defaults clusterDomain to cluster.local, but cozystack runs on cozy.local. KEDA's metrics-apiserver dials the operator metrics gRPC service at keda-operator.<ns>.svc.<clusterDomain>:9666 and the operator mints that service's serving-cert SANs from the same domain, so the default resolves to NXDOMAIN and every HPA hangs on FailedGetExternalMetric. Pin keda.clusterDomain=cozy.local. Also raise the operator's client-go throttle to 50 QPS / 100 burst: at the upstream 20/30 a single-replica operator loses its leader lease under scale-subresource polling and crash-loops (the client rate limiter starves the lease renewal). Both were observed live on a dev cluster; guard them in the package unittest. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
d4c76fadb4 |
feat(postgres): two-phase autoscaling migration and read-CPU metric
Make enabling autoscaling on an existing database count-preserving, and add a second read-load signal: - autoscaling.transition (migration phase 1): keep .spec.instances rendered as a floor (from replicas, which the operator stages to the live count) AND pause the ScaledObject, so KEDA stands up without contending with Flux for the field. Flip transition off (phase 2) to drop the static field and hand the count to KEDA — suspend the HelmRelease across that apply, since helm-controller uses Helm's three-way merge (not SSA) and would otherwise race CNPG's default-to-1. The procedure is documented on the values field. - autoscaling.metric (ReadConnections | ReadCPUUtilization): the query helper emits either the active-connections sum or the postgres-container CPU sum, both + target. active = enabled && !dryRun && !transition; the ScaledObject is paused whenever dryRun or transition is set. Covered by helm unittest (phase-1 holds the staged count + pauses; CPU metric query) and regenerated schema/README/Go types/postgres-rd. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
237053191e |
feat(keda): ship database-autoscaling alerts and Grafana dashboard
Add cozystack-owned observability for managed-database read-replica autoscaling, rendered by the keda umbrella (not the vendored subchart): - PrometheusRule (templates/alerts.yaml), scoped to the KEDA-generated HPAs (keda-hpa-*): DatabaseAutoscalerScaleStuck (desired > current for 15m — stuck scale-up with no automatic rollback), DatabaseAutoscalerAtMax (ScalingLimited=true for 30m — at max or quorum-raised ceiling), and DatabaseAutoscalerScalerErrors (KEDA cannot evaluate the trigger — never scale blind). - A Grafana dashboard ConfigMap (grafana_dashboard sidecar label) with desired-vs-current replicas, active read connections, replication lag, and scaling-limited panels. Both gated by databaseAutoscaling.alerts.enabled / .dashboard.enabled (default true). Metric names are calibrated on a live cluster per the proposal's PoC; the rule and panel shapes are what ships. Covered by helm unittest (PrometheusRule with the three alerts + disable; dashboard ConfigMap label + disable). Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
7f8f2c1d86 |
feat(keda): add KEDA as an optional platform component
Vendor the upstream KEDA chart (kedacore/keda 2.20.2) as a new optional system package and wire it into the platform, so database read-replica autoscaling (which renders KEDA ScaledObjects) has a backend. - packages/system/keda: umbrella cozy-keda + vendored chart (Makefile pins KEDA_CHART_VERSION with a lockstep assert), shim values enabling the operator / metrics-server / webhook ServiceMonitors under the platform Prometheus. - core/platform: PackageSource cozystack.keda (namespace cozy-keda, dependsOn networking + prometheus-operator-crds; cert-manager is not needed, KEDA self-generates its webhook TLS) plus an optional bundle entry, opt-in via bundles.enabledPackages. It is not a dependsOn of any default package, so the platform HelmRelease never blocks on it. - KEDA claims the cluster-wide external.metrics.k8s.io APIService (distinct from metrics-server's metrics.k8s.io — no conflict). Covered by helm unittest: platform bundle enable/disable/opt-out, the PackageSource dependsOn, and the vendored chart's external-metrics APIService + shim-enabled ServiceMonitor. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
0335276b61 |
feat(postgres): render a KEDA ScaledObject for read-replica autoscaling
Implement the KEDA-based read-replica autoscaler for postgres per the merged design proposal (cozystack/community#44): - cozy-lib.keda.scaledObject: engine-agnostic helper rendering a KEDA ScaledObject (scale target, effective bounds, prometheus trigger, scale-down behavior, optional pause), fail-closed on bad input. - postgres: an `autoscaling` values block (enabled / minReplicas / maxReplicas / target / maxReplicationLagSeconds / dryRun). Under active autoscaling the chart drops the static .spec.instances so Flux and the KEDA-managed HPA no longer contend, and renders a ScaledObject whose minReplicaCount folds in the synchronous-quorum floor (max(minReplicas, maxSyncReplicas+1, 2)); when the floor exceeds maxReplicas, quorum wins. dry-run keeps the static count and pauses the ScaledObject. The WorkloadMonitor tracks the quorum floor instead of a stale static value. Covered by helm unittest (cozy-lib helper: valid render, pause, three fail-closed guards; postgres: instances dropped, quorum-floored bounds, quorum-over-max, dry-run pause). Regenerated values.schema.json, README, Go types and postgres-rd. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
3a369fed99 |
feat(redis): add opt-in TLS for Redis and Sentinel (#2729)
## What this PR does `tls.enabled` puts Redis and Sentinel on a TLS-only listener with a per-release cert-manager chain, and hands the tenant the CA certificate as `redis-<name>.tenant-ca`. Off unless set. redis-operator v3.3.5 has no TLS support. The work is ported upstream in freshworks-oss/redis-operator#85, which is open and not in a release, so this carries it as three patches over the v3.3.5 tarball. `images/redis-operator/patches/tls.diff` is the source half, cut from that PR's commit `2ad2ed8d1`, which the patch header pins; it is applied by the glob the image Dockerfile already runs, after the Dockerfile strips the tarball's `manifests`, `charts`, `example` and `.github`, so the image builds from source alone and the upstream tests that read the CRD YAML skip instead of failing against the stale v3.3.5 copy. `patches/crd-tls.diff` adds `spec.tls` to the vendored CRD, and with it the CEL rule that fixes `tls.enabled` at creation; without the schema block the apiserver prunes the field on admission and the patched binary waits for something it never sees, and the rule cannot exist without the block it references. `patches/rbac-tls.diff` grants Create, Get and Update on Secrets, so the operator can publish the CA-only Secret it addresses by name. That is the whole delta, and it is narrower than the ported operator's own chart asks for: those three are the only Secret calls in the patched source, and cert-manager Certificates get no grant at all, because the operator only touches one under `spec.tls.certManager` and this chart pins `certificateSecret` mode instead. One change in `tls.diff` is not TLS and takes effect with TLS off, named in the patch header: `MakeSlaveOfWithPort` connects to the replica on its own port rather than on the master's. It changes a dial, not a pod template, so shipping the rebuilt image rolls nothing; the plaintext pod templates stay byte-identical to v3.3.5. The image build registration this branch used to carry (`make -C packages/system/redis-operator image` in the root `build:` target) landed on main with the Valkey service (#3380), so the rebuilt operator image is produced by CI from main as well; the digest in `values.yaml` is still refreshed by release preparation, not here. The chart patches re-apply at the end of `update:`, the same way reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji carry theirs. Otherwise `rm -rf charts` takes TLS back out on the next `make update` and nothing fails. `make update` reproduces the committed tree byte for byte. TLS is chosen when the instance is created. The config gets `port 0` and the listener moves to `tls-port`, so TLS replaces plaintext rather than running beside it, and a running instance cannot change mode: a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The RedisFailover schema in the carried CRD patch rejects a change of `tls.enabled`, which on an existing instance surfaces as a failed HelmRelease upgrade while the instance keeps serving what it did; the README points at creating a new instance and migrating. That is also why TLS is not inferred from `external`: an externally reachable instance is the one most likely to have clients an upgrade must not drop. Existing releases keep their plaintext listener and are not restarted by the rebuilt operator image. The tenant gets `ca.crt` and nothing else, through `tenantsecrets`. The chart's own Secrets take a dot between the release and the suffix (`<release>.ca-tls`, `<release>.tls`, `<release>.ca-cert`): the suffixes are dot-free, so the split is unique and release `foo`'s CA Secret can no longer be release `foo-ca`'s leaf, the collision the hyphenated form had. The chart renders a `TenantProjection` pointing at `<release>.ca-tls`, and the CA-extraction controller re-encodes the one named key from the parsed certificate, so nothing that is not a certificate gets through. Same path as postgres, mongodb, nats and qdrant. No Secret of this release is granted to the tenant by name beyond the generated password, and that includes `<release>.ca-cert`, the CA-only copy the operator publishes for itself. It holds no key today, but the operator has Create and Update on Secrets cluster-wide, so what sits at that name is the operator's call rather than the platform's, and RBAC cannot filter by key. `hack/check-redis-rd-secrets.bats` pins the whole rendered Secret surface instead of a list of known-bad names, because an include with no `resourceNames` and no `matchLabels` restricts nothing. The chainsaw suite for redis gains two cases. A TLS fixture asserts the cert-manager chain is issued, the TLS-probed Redis replicas and Sentinel come up, the operator publishes its CA-only Secret under the widened ClusterRole, the tenant receives exactly `ca.crt` through `tenantsecrets` while the key-bearing Secrets stay off that path, and a `redis-cli --tls` PING through the service name completes with the same CA. A second case applies `tls.enabled` to a running plaintext release and asserts the refusal: the HelmRelease upgrade fails with the schema's message, the StatefulSet keeps its revision with both replicas Ready, plaintext still answers and no TLS listener appears. Certificate renewal rolls the pods from inside the operator. Redis and Sentinel read the certificate once at startup, so the patched operator stamps a hash of the TLS Secret's `tls.crt` and `ca.crt` into the pod template of both the Redis StatefulSet and the Sentinel Deployment on every reconcile, as `redis-failover.freshworks.com/tls-secret-hash`. A renewal changes the hash; the operator's own roller replaces the Redis pods one at a time and the Deployment controller rolls Sentinel. README names the fallback: if the roll cannot run, the deadline is the old certificate's expiry, past which the operator's own TLS connections fail along with the clients. `tls.authClients` maps to `tls-auth-clients` and defaults to `no`. The platform mints no client certificate for the tenant and the CA key is out of reach, so `yes` is only usable when the client certificate comes from whoever also controls the CA. Changing the value on a running release restarts the Redis and Sentinel pods: Redis reads the directive from its config file only at startup, so the operator folds the effective value into the certificate hash that drives the rollout, and the change lands when it is made rather than at the next unrelated restart. Follow-ups in the carried operator patch go upstream first, to freshworks-oss/redis-operator#85 or a successor there, and are picked up here afterwards; the carried patch does not grow Go that upstream has not seen. `tlsConfigFor` fetches the Secret on every call, tens of GETs per reconcile. The renewal roll is static analysis so far: the live check, `cmctl renew` on a running release while watching the `rfr-*` and `rfs-*` pods roll, is still owed and belongs with the e2e follow-up. Two facts about the shipped image rather than the patch. `packages/system/redis-operator/values.yaml` keeps the digest of the image built before the patch until release preparation rebuilds it, so on `main` between this merge and the next release-prep, `tls.enabled` renders the chain and `spec.tls` while the running operator ignores them and serves plaintext. And the pinned v3.3.5 tarball carries GO-2026-5026 in `golang.org/x/net` v0.53.0, reached through `PodService.UpdatePodLabels`; the port neither introduces nor fixes it, upstream `master` is already on v0.55.0, so it lands with the next tag bump rather than here. Outside the patch: `cozyvalues-gen` renders `{}` in the README Value column for `tls.authClients`: a child property without a default inherits the parent object's `{}`, and `make generate` reproduces it byte for byte, so it is a generator bug rather than something to hand-edit here. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#37 - [ ] [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: I walked the trigger map against the diff file by file. terraform-provider-cozystack is reached, tracked as cozystack/terraform-provider-cozystack#37 rather than a speculative PR: `values.schema.json` gains a `tls` object, and the provider hand-writes a schema, model and expand/flatten pair per Kind. It already models `tls` for postgres, kafka, nats and qdrant, not for redis. Redis needs different wording from those four, whose descriptions say to omit the block to follow `external`, which is the one thing redis does not do. Nothing else is reached. The website regenerates this package's reference page from `README.md` on a stable tag, so the README changes need no PR there. No package added, renamed or removed. `packages/core/platform/values.yaml`, variant and bundle composition and release asset names are untouched, and so are `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh`, which is what ccp and external-apps-example read. The two new files under `hack/` are bats unit tests. No node prerequisite, installer value, telemetry metric or cozy-proxy label changes. ### Release note ```release-note feat(redis): add opt-in TLS for Redis and Sentinel. Set `tls.enabled: true` to serve both on a TLS-only listener backed by a per-release cert-manager chain; the tenant reads the CA certificate as the `<release>.tenant-ca` tenant secret. TLS is off unless set and is not inferred from `external`, so existing releases keep serving plaintext and are not restarted by the rebuilt operator image. TLS is chosen at creation: a running instance cannot switch modes, the change is rejected and surfaces as a failed HelmRelease upgrade, so add TLS by creating a new instance and migrating to it. Requires cert-manager, which the platform's `system` bundle installs on every variant. A renewed certificate rolls the Redis and Sentinel pods through the operator. TLS support is carried as a downstream patch over redis-operator v3.3.5 until freshworks-oss/redis-operator#85 lands in a release. ``` |
||
|
Aleksei Sviridkin
|
d800526696 |
feat(mariadb): add TLS support via cert-manager (#2680)
## What this PR does Gives MariaDB a chart-managed certificate chain, makes TLS enforcement explicit, and hands the tenant a key-free trust anchor instead of a private key. - `tls.issuer` selects who issues the server certificate: `operator` (the CA mariadb-operator manages itself, the default and today's behaviour) or `cert-manager` (the instance gets its own CA, and this is what covers the external hostname). The field is named for the issuer rather than for on/off because TLS is served under both — unlike the similarly spelled field in some other charts, which really is a switch. - `tls.required` refuses plaintext connections. It is opt-in and defaults to false: enforcing TLS before the tenant can obtain the CA would be backwards, and a platform upgrade must not sever connections that work today. - Neither issuer is inferred from `external`. Existing instances keep the CA they have and change nothing on upgrade. - The tenant no longer receives `<release>-ca-tls` or `<release>-tls` — the CA private key and the server private key. It receives the operator's `ca.crt`-only bundle, which works today, and the ResourceDefinition additionally declares that bundle as the source for the CA-extraction controller. ### Fixes along the way - The TLS backup path could never have worked: it passed `--ssl-mode=VERIFY_IDENTITY`, a MySQL-8 flag the MariaDB client rejects during argument parsing. Replaced with `--ssl-ca` + `--ssl-verify-server-cert`, and the test now asserts the dialect rather than the presence of a substring. - The leaf certificate carried `clientAuth`. Because `REQUIRE X509` accepts any certificate signed by a trusted CA, the server's own key was usable as a client credential. Removed. - The CA and server private keys survived deletion of the instance: cert-manager runs with `enableCertificateOwnerRef: false`, so removing the `Certificate` left the Secrets behind. Both are now cleaned up by the existing post-delete hook. - The Secrets are labelled for the operator's watch predicate, without which a renewed leaf would not reach the pods until a full resync. ### Why the include carries an exclude list The trust-anchor selector matches by label with no name constraint, so the enumerable dangerous set is excluded explicitly — both issuers' private keys, the internal credentials, the backup keys, and the metrics configuration, which the operator writes unconditionally with credentials in plain text for a user holding `SELECT` on everything. The backstop is bounded and the comment says so: matching is by exact name with no globs, so a Secret introduced by a future operator version, or a transient object created mid-renewal, stays in scope of the label-only include. The primary defence remains that the controller stamps only its own projection. ## Known limitations 1. **Merge is blocked on #3299 — and the blocking is mechanical, not just procedural.** The CRD on `main` does not carry `spec.caCert`, and under Flux's Server-Side Apply the API server rejects a field the schema does not declare (`field not declared in schema`); under the older client-side apply the same field was silently dropped, so installing this branch without #3299 fails outright. An earlier revision of this description claimed the field would be silently pruned; that was wrong. The tenant still gets a usable trust anchor once merged in order, through the operator's bundle. 2. **The deprecated backup CronJob cannot start at all**, for reasons that predate this branch: its `secretKeyRef` names a Secret and key the chart never renders. That is filed separately as #3347 and deliberately not fixed here, since repairing it would start scheduled dumps on production instances from a TLS change. The tests that pin the backup TLS flags therefore pin a path that does not currently execute — the suite header says so. 3. **The operator's `clusterName` is hardcoded**, so on a cluster with a non-default domain the SANs and the name the operator dials diverge. Pre-existing, raised separately. 4. **No e2e coverage for the managed path.** Unit coverage pins the rendered chain, the SAN set, the exclusion set and the guards; the end-to-end handshake is not asserted in CI. ### On review coverage Two independent review passes were run repeatedly against this branch — seven rounds, converging from a security finding, through comment accuracy, to guard strength, and finally to nothing new. A third, model-based pass reviewed the branch through `6cb1bd236` and its findings were fixed; it could not be run against the final revision because of an external account quota lasting until 25 July. The delta since that revision is substantial and includes runtime code, not only tests and docs. Recording this so the coverage is not overstated: if the quota returns before this is ready to merge, that pass is worth running again. ### Downstream repositories `packages/apps/mariadb/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_mariadb`; tracked in cozystack/terraform-provider-cozystack#40. ### Release note ```release-note feat(mariadb): chart-managed TLS via cert-manager, opt-in enforcement, and the CA published to tenants as a key-free trust anchor ``` |
||
|
Aleksei Sviridkin
|
18d02d813a |
fix(cacert): make the comments match the reconciler (#4064)
## What this PR does Comments in the CA-extraction package claim guards the code does not have. No behaviour change; one test is renamed to match the property it pins. - `ManagedLabel` said the reconciler only ever writes over a Secret carrying that label. It does not. `isOurProjection` decides that from the Secret type and the owner reference, and never looks at the label, which any writer in the namespace can strip. - `managedByCozystackLabel` said the reconciler never writes it. It deletes it whenever the selectors digest changes, which is what gets the projection re-admitted by the lineage webhook. - `projectionSuffix` said the dotted canonical name cannot collide with an operator-owned object. A release the apps API created cannot carry a dot, since application names are DNS-1035 labels and `release.prefix` is restricted to `^[a-z0-9-]*$`. The guarantee stops there: the controller reads the release from the `helm.toolkit.fluxcd.io/name` label, where a dot is legal, so a hand-written HelmRelease sits outside it. On the validated path the dot can only come from the suffix an operator or chart appends, which nothing validates at all, and since a canonical name ends in `.tenant-ca` and carries that one dot, a suffix ending in `.tenant-ca` is the only one that reaches a canonical name. The comment says all of that now. - The test pinning that name claimed the property for any engine suffix, in its doc and in its own name, while its loop samples only dot-free ones. It is now `TestProjectionNameCannotCollideWithADotFreeEngineSuffix`, and its doc says what the loop covers. ### Screenshots Not a UI change. ### Downstream repositories Comments and one test name in two Go files. No package, values key, schema field, CRD, metric, release asset or `hack/` target changes, so nothing in the trigger map fires. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified certificate reconciliation behavior, including label handling, projection naming, and source certificate scenarios. * Documented naming guarantees and their API boundaries more precisely. * **Tests** * Updated test descriptions and failure messaging to clarify coverage for dot-free engine suffixes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b167fd112a |
feat(gateway): let a tenant pick its Gateway class, and follow that class for TLS termination (#3861)
## What this PR does Lets a tenant publish through a GatewayClass whose provider terminates TLS upstream of the Gateway, and gives the platform the two knobs that bound the choice. The first consumer is the Cloudflare Tunnel controller from #3858, which ends TLS at the Cloudflare edge and reaches the Gateway over the tunnel, but nothing here is tunnel-specific. Until now every cert mode rendered HTTPS listeners with `certificateRefs` and tied the TenantGateway's `Ready` to their issuance. On such a class the platform issued origin certificates nobody serves, and where ACME could not complete, the tenant's `gateway` release never became Ready at all. **Who picks the class.** The platform names a default in `gateway.className` and the set a tenant may choose from in `gateway.tenantSelectableClasses`; a tenant picks its own with `tenant.spec.gatewayClass`. That set is an allowlist rather than free choice because Tenant is a tenant-writable object while 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 with a message listing what is allowed, and reaches no other tenant. **Where TLS ends.** That is a property of the class, not of the tenant, so it is declared once in `gateway.edgeTerminatedClasses`, and whichever class a tenant lands on decides its `certMode`. The two therefore cannot be configured out of step: a tenant on a tunnel class cannot keep issuing origin certificates because a separate switch still says otherwise. In `edge` the controller renders the apex, its wildcard and every inheriting child apex as plain HTTP listeners carrying the same label-based `allowedRoutes` as the HTTPS listeners of the other modes, so app HTTPRoutes attach by hostname unchanged. It mints no Issuer, no Certificate and no http-to-https redirect route, and skips the TLS-passthrough listeners an HTTP-only edge cannot serve, which leaves the tenant Kubernetes API, VM export and CDI upload endpoints unpublished through that Gateway. Those TLSRoutes each pinned a listener that no longer exists, so the controller flips the acceptance it wrote for them to `Accepted=False` with reason `NoMatchingParent`. Moving a tenant into the mode removes the ACME objects it owned, and the cluster-wide ACME solver drops its `sectionName: http` pin when the publishing Gateway itself sits on such a class. Pointing `gateway.className` at an edge class is therefore a supported topology (the whole platform behind one terminator), and its cost is that those three endpoints stop being published for the publishing tenant rather than for one tenant among many. Nothing refuses it, because refusing it would forbid the topology; `docs/gateway-classes.md` states the cost where an operator will meet it. Two consequences worth stating plainly. The wildcard-secret controller replicated the operator's TLS Secret, private key included, into every namespace owning a termination point, and a Gateway owner counted as one; a Gateway on an edge class does not, so that tenant's replica is pruned while its neighbours keep theirs. And listing a class as edge-terminated is a claim nothing validates: name the bundled `cilium` and its Gateways publish every app hostname over plain HTTP on a public address. A hard `fail` there would forbid a legitimate topology, a Cilium Gateway behind an external terminator, so it is stated in the platform values, the chart README and the CRD field description instead. **Documentation.** `docs/gateway-classes.md` is the operator-facing page for the whole surface: the three platform values and what each bounds, what a tenant writes to pick a class, how `certMode` is derived and in what precedence, what moving a tenant between classes does to its Issuer, its Certificates, the TLS Secrets those leave behind, the redirect route and the wildcard replica, and what each way of getting the names wrong actually looks like: a class name matching nothing installed renders fine and never programs, a misspelling in `edgeTerminatedClasses` silently leaves the tenant issuing certificates, a default moved out from under a tenant that pinned the old name fails that tenant's release. The chart README keeps the per-mode reference and gains the one constraint it had not stated: the publishing tenant cannot select a class that terminates TLS differently from the platform default, because the cluster-wide ACME issuers derive their solver pin from that default alone. Verified on a v1.6.0-rc.2 dev cluster (Talos, k8s 1.36, cilium 1.19.5) by building the controller from this branch: the CRD enum gained `edge`, the Gateway came up with `edge` / `edge-apex` / `edge-child-*` HTTP listeners and nothing else, no Issuer, no Certificate, no redirect route, and the LoadBalancer Service exposed 80/TCP only. That run also caught a defect no unit test could. The first implementation kept the hostname-less `:80` listener at index 0, and on cilium 1.19.5 `CheckGatewayAllowedForNamespace` is first-listener-wins ([cilium#42159](https://github.com/cilium/cilium/issues/42159), fixed in 1.19.6), so an inheriting tenant's HTTPRoute came back `NotAllowedByListeners` even though a later listener admitted it. Dropping that listener, which in edge mode carries neither ACME nor the redirect, flipped the route to `Accepted=True` and removed a catch-all at the same time. The live check predates the redesign to per-tenant classes, so it exercised the mode rather than the selection path; the selection is covered by unit tests across the four charts and both controllers. Moving a live Gateway from one class to another is covered nowhere. `Gateway.spec.gatewayClassName` has no immutability rule in the shipped CRD, so the apiserver accepts the update; what the outgoing class controller then does with the LoadBalancer Service it provisioned was not exercised by the live run and no unit test reaches it. ### Screenshots Not a UI change. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: `networking/gateway-api.md` documents the platform `gateway.*` rows and the cert modes, so `className`, `tenantSelectableClasses`, `edgeTerminatedClasses` and `tenant.spec.gatewayClass` need a doc PR there; `docs/gateway-classes.md` from this PR is what that port starts from. Not filed yet. - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: the provider hand-writes the Tenant schema, so `spec.gatewayClass` needs an attribute there; not filed yet. - [ ] [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: Two boxes stay unticked on purpose: the website and provider follow-ups are real and not filed yet. I will open them once the shape here settles in review. ### Release note ```release-note feat(gateway): tenant Gateways can now run on a GatewayClass other than the bundled Cilium one. The platform sets the default in `gateway.className` and bounds tenant choice with `gateway.tenantSelectableClasses`; a tenant selects its own with `tenant.spec.gatewayClass`. Classes whose provider terminates TLS upstream are listed in `gateway.edgeTerminatedClasses`, and a tenant Gateway on such a class serves plain HTTP listeners, issues no certificates, and no longer receives a replica of the operator wildcard Secret. Such a Gateway also has no TLS-passthrough listeners, so the `api`, `vm-exportproxy` and `cdi-uploadproxy` entries of `publishing.exposedServices` are not published through it, and their TLSRoutes report `Accepted=False` with reason `NoMatchingParent` ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added edge certificate mode with HTTP-only listeners, no certificate issuance, and no TLS passthrough. * Added configurable GatewayClass selection for tenant-owned Gateways, with platform defaults, allowlists, and validation. * Added platform configuration for edge-terminating and tenant-selectable GatewayClasses. * **Bug Fixes** * Improved wildcard certificate replication for edge-terminated Gateways. * Ensured safe cleanup and restoration when switching certificate modes. * **Documentation** * Updated Gateway, tenant, and certificate configuration guidance for edge termination and GatewayClass selection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
7b0c35b9b3 |
feat(monitoring): add KubeVirt VM dashboard (#2198)
## Summary - Adds `dashboards/vm/virtual-machines.json` — KubeVirt VM resource metrics (CPU, memory, network, storage) with namespace and VM template variables - Adds `dashboards/vm/console-logs.json` — VM serial console log viewer via VictoriaLogs with namespace filtering - Adds `vm/virtual-machines` and `vm/console-logs` entries to `dashboards.list` ## Problem `dashboards.list` references `vm/virtual-machines` and `vm/console-logs` but the corresponding JSON files don't exist. The console-logs dashboard includes `kubernetes_namespace_name` filtering to prevent cross-tenant log exposure. Relates to #2194 ## Test plan - [ ] Verify JSON is valid for both dashboards - [ ] Import dashboards in Grafana and verify panels render with KubeVirt metrics - [ ] Verify console-logs queries include namespace filter ```release-note Add KubeVirt VM dashboards (virtual-machines and console-logs) ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * **VM Console Logs dashboard**: View live console output with namespace and VM filtering, fast refresh, and improved log presentation (details, wrapping, and sorting). * **Virtual Machines dashboard**: Rich VM monitoring covering CPU, memory, network, storage I/O, latency, swap/page faults, and filesystem usage with thresholds and formatted visuals. * **Dashboard catalog updated**: Added multiple new monitoring dashboards for broader coverage and easier discovery, including the new VM dashboards. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
773d8dabce |
fix(gateway): withdraw TLSRoute acceptance when the mode drops passthrough
Edge terminates TLS at the class provider, so its Gateway carries plain HTTP listeners only and a route pinned to a passthrough section has nothing left to attach to. The api, vm-exportproxy and cdi-uploadproxy endpoints of publishing.exposedServices are the ones that pin one. Hostname claims are empty for every whole-apex mode, so the status pass ran over an empty ref set and left the Accepted=True written while the tenant ran HTTP-01 standing on a route the switch had just unpublished. Whoever went looking for the dead endpoint read that condition as proof the route was fine. The rule this settles on is that the controller owns a condition only where it judges. Edge judges every attached route by the absence of the listener it needs and writes Accepted=False with Gateway API's NoMatchingParent, the reason defined for a parentRef whose port or sectionName matches no listener. The other whole-apex modes render those listeners and serve them but collect no claims, so they hold no opinion: they drop the entry rather than leave a withdrawal standing on a listener that is serving, naming a class provider that no longer terminates anything. HTTP-01 collects claims and has already judged every route this pass could speak about. A route declaring no hostnames is judged by none of them, because claims are gathered inside a loop over spec.hostnames and TLSRoute v1alpha2 sets no minItems on that field. Withdrawing one would write a condition with no path back, since HTTP-01 renders the listener again and judges only the routes that produced claims, so both directions skip it. The tests read the controller name from the constant the controller stamps rather than restating it, so a helper cannot go looking for a name that is no longer written and report absence it never checked. The switch already deletes the redirect route, the Issuer and the wildcard Certificate it invalidates. The statuses it owns were the one thing left behind. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5976ddf02f |
fix(gateway): normalise the GatewayClass name and refuse a blank one
The class lists arrive over the values channel comma-joined, so every reader trims each entry as it splits them. The scalar those entries are compared against was not trimmed anywhere. A class name carrying a stray space therefore failed `has` against its own padded self, and the two readers of that scalar disagreed about the same class: the gateway chart put the tenant on an edge-terminated class, while the cluster-wide ACME issuers still pinned their solver to a listener an edge Gateway does not render. That stops every HTTP-01 certificate in the cluster from issuing, with nothing in any status naming the cause. Normalise once at the single writer so both readers see a clean value, and keep a defensive trim at each reader. The tenant's own choice does not travel that channel — it comes from tenant.spec.gatewayClass — so it is trimmed where the gateway chart reads it. Unset and whitespace-only are different events and no longer share an outcome. Unset means the choice was not made, and falling back to the bundled class stays its documented behaviour. Whitespace-only means a choice was made and is garbage, in the field that decides where TLS terminates: resolving it quietly would take a tenant the operator meant to put behind an edge terminator and publish it directly, with a real certificate on a real address. Both the platform default and the tenant's own choice now refuse it by name. The kind check beside that refusal is part of the same fix, not a tidy up. Through Helm's values path an unquoted numeric scalar arrives as a float64, and the whitespace check compares it against a string, so `className: 123` dies at that comparison — one line before the trim — with an error naming neither the key nor the file. Refusing whitespace without checking the kind first would therefore have made the numeric case worse than it was. `kindIs "string"` is the right test because it never has to name the type that actually arrived. The order of the guard's steps is part of the guard. The value is read before anything normalises it, so a default that fires on any falsy value cannot launder 0 or false into the empty string and resolve them to the bundled class ahead of the kind check. Unset is the one case that still falls back, and each of those four inputs is pinned, so a normalisation drifting back ahead of a check fails rather than quietly covering only the loudest bad input. Also correct a cleanup log line: the wildcard Certificate is deleted for every mode that is not DNS-01, so naming HTTP-01 in that message is wrong for existingSecret and for edge alike. And state the listener name invariant plainly: the cap is 253, not the 63 of a DNS label, and the longest name the builders produce is 84. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
7851969448 |
docs(gateway): document GatewayClass selection and edge TLS termination
An operator setting up a class whose provider terminates TLS upstream had no page answering their questions: which platform values bound the choice, what a tenant writes to make it, what a move between classes does to certificates and to the replicated wildcard Secret, and how the mechanism fails when a configured class name matches nothing installed. Add docs/gateway-classes.md covering that surface. Describe the mode and its consequences in the chart README alongside the other three, including the one constraint stated nowhere else: the publishing tenant cannot select a class whose TLS termination differs from the platform default, because the cluster-wide ACME issuers derive their solver pin from that default alone. State the blast radius where it actually applies. The endpoints an edge Gateway stops publishing exist only where those routes are rendered at all, and both pages say what the transition does to the route conditions the controller owns, so a reader is not left inferring it from the code. The security model names the selector each listener really carries. The port-80 listener selects on the built-in kubernetes.io/metadata.name label, which kube-apiserver writes and a namespace cannot forge; the HTTPS and TLS-passthrough listeners select on namespace.cozystack.io/gateway, written by the tenant chart and the controller, so that guarantee does not extend to them. What closes the exposure is that Namespace is cluster-scoped with no tenant verb on it, and that the route-hostname policies bound what a labelled namespace could publish. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
myasnikovdaniil
|
1b2beac3f8 |
[cilium] Set resources requests/limits on cilium-operator (#2618)
## Summary Companion to #2617 (cilium-agent). The upstream cilium chart ships with `operator.resources: {}`, which triggers a `no CPU limit` conformance warning on the `cilium-operator` Deployment. Override `packages/system/cilium/values.yaml` to set the sizing recommended by the upstream `values.yaml` comment: - `limits` : `cpu: 1000m`, `memory: 1Gi` - `requests` : `cpu: 100m`, `memory: 128Mi` ## Test plan - [ ] `helm template . --namespace kube-system` from `packages/system/cilium/` renders a `resources` block on the `cilium-operator` container of the Deployment (verified locally) - [ ] Re-run the conformance/lint tool — the `cilium-operator has no CPU limit` warning should be gone - [ ] CI passes ### Release note \`\`\`release-note Set CPU/memory requests and limits on the cilium-operator Deployment to clear the "no CPU limit" conformance warning. Values follow the upstream chart's recommended sizing (limits 1000m/1Gi, requests 100m/128Mi). \`\`\` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated Cilium system component resource configuration with CPU and memory requests and limits. [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2618) <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
57c51f58ee |
feat(gateway): stop replicating the wildcard key into edge namespaces
The wildcard-secret controller replicates the operator's TLS Secret, private key included, into every namespace that owns a termination point, and a namespace owning a Gateway counted as one without further question. A Gateway whose class ends TLS at its provider's edge terminates nothing locally, so the replica has no consumer there. Read certMode off the tenant's own TenantGateway rather than a cluster-wide value, because the class is a per-tenant choice. Keep the key whenever the answer is not a confident no: a namespace that also owns an ingress controller still terminates locally, a namespace holding more than one TenantGateway keeps it while any of them is not at the edge, and an unreadable list or a namespace whose TenantGateway has not appeared yet keeps it too, so a transient API error never withdraws a key a Gateway may still be serving. Watch TenantGateway, so the withdrawal follows the class change rather than waiting for the periodic resync. The watched set is a value the tests assert on, because the controller-runtime builder exposes nothing about what it watched: a dropped watch would otherwise leave both suites green while the withdrawal quietly degraded to that resync, leaving the private key in a namespace that stopped terminating TLS. Pruning the replica is not the same as leaving the namespace key-free. The Certificate objects go, but the Secrets they issued stay, as they do on any other mode transition. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
2ef0e8d3a1 |
feat(gateway): add an edge cert mode for upstream-terminated classes
Some GatewayClasses have their provider terminate TLS in front of the Gateway and reach it over plain HTTP. On such a class the platform issued origin certificates nobody serves, and where ACME could not complete, the Gateway's listeners never became ready. Add certMode=edge to TenantGateway. The controller renders the apex, its wildcard and every inheriting child apex as plain HTTP listeners carrying the same label-based allowedRoutes as the HTTPS listeners of the other modes, so application HTTPRoutes attach by hostname unchanged. It mints no Issuer and no Certificate, and removes the http-to-https redirect route, because the edge performs that redirect itself. TLSPassthroughServices are not rendered: an HTTP-only edge cannot carry a TLS listener, which leaves the endpoints relying on one unpublished through such a Gateway. The hostname-less port-80 listener is left out. It carries the ACME challenge and the redirect in the other modes and has neither here, so it would only be a catch-all admitting any hostname. Leaving it out also keeps a hostname-pinned listener at index 0, which is what Cilium's first-listener-wins namespace check reads before the fix in cilium/cilium#45693. Moving into the mode garbage-collects the Issuer and Certificate objects the controller owned. The TLS Secrets they issued stay behind: cert-manager runs with enableCertificateOwnerRef false, so a Certificate does not own the Secret it issued into. Every mode transition already behaves that way. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
a2cb47c961 |
feat(gateway): update the CRD bundle to Gateway API v1.6.1 and gate GRPCRoute hostnames (#3872)
## What this PR does Bumps the vendored Gateway API CRD bundle in `packages/system/gateway-api-crds` from v1.5.1 to v1.6.1. The package Makefile pins the upstream ref, so the change is that pin plus the re-rendered `templates/crds-experimental.yaml`. The bundle version is not only bookkeeping: it is a compatibility signal consumers read at runtime. Every CRD in the bundle carries a `gateway.networking.k8s.io/bundle-version` annotation, and a Gateway API controller can read it off the `gatewayclasses` CRD to decide whether the installed bundle matches the one it was built against. Controllers that do this typically accept any patch release but require an exact major and minor match, and report `SupportedVersion=False` with reason `UnsupportedVersion` on their GatewayClass when the minor differs, a status condition that surfaces the skew rather than a functional break. The Cloudflare Tunnel Gateway API controller proposed for the platform in #3858 is built against Gateway API v1.6.1 and performs exactly that comparison, so against the v1.5.1 bundle shipped today its GatewayClass reports the bundle unsupported, and with this bump it reports supported. Neither change depends on the other landing first: this PR only moves the bundle, and the check lives entirely on the consumer side. One consequence worth stating plainly, since it will outlive this PR: an exact-minor check makes the shipped bundle version a coupled pin for any such controller. A future bump to v1.7.x flips the same condition back to `UnsupportedVersion` until that controller is rebuilt against the newer bundle, so the two want to move together. The bundle still emits the same twelve CustomResourceDefinitions and the same two `safe-upgrades.gateway.networking.k8s.io` admission objects, and every CRD still carries the `helm.sh/resource-policy: keep` stamp the Makefile's post-processing step adds. One of the policy's two CEL rules is rewritten, though, and it changes what a cluster accepts. That has its own section below. Two kinds change their version surface: TCPRoute and UDPRoute gain a served `v1` that takes over as the storage version, and their `v1alpha2` stays served with a deprecation marker. Every other kind keeps exactly the versions it served at v1.5.1. That matters because the Cilium operator resolves GatewayClass, Gateway, HTTPRoute and GRPCRoute in `v1`, ReferenceGrant in `v1beta1` and TLSRoute in `v1alpha2` before it enables its Gateway API controller; all six are still served by the v1.6.1 bundle. TLSRoute in particular keeps `v1alpha2`, which the tenant gateway controller, the platform TLSRoute templates and the tenant hostname admission policy all address. Schema changes are small and land on fields nothing in this repository sets. `sessionPersistence.idleTimeout` is gone from HTTPRoute, GRPCRoute and XBackendTrafficPolicy, removed upstream under GEP-1619 because no implementation ever implemented it and HTTP cookies have no native idle-timeout mechanism, so no working behaviour is lost even though the field disappears from the schema; HTTPRoute gains a lower bound on `retry.attempts` and now rejects duplicate `retry.codes`; ReferenceGrant requires `spec` at the root. In the other direction, Gateway infrastructure annotations and frontend CA certificate references go from a limit of 8 to 16, and TLSRoute hostnames from 16 to 1024. No CRD schema loses or changes a CEL validation rule, and the six added belong exclusively to the new TCPRoute and UDPRoute `v1` schemas. ### The safe-upgrades admission policy accepts a different set of bundles The experimental kustomization ships a `ValidatingAdmissionPolicy` and its binding, both named `safe-upgrades.gateway.networking.k8s.io`, next to the twelve CRDs. The binding is `validationActions: [Deny]` with `failurePolicy: Fail`, matching CREATE and UPDATE on `apiextensions.k8s.io/v1 customresourcedefinitions` cluster-wide, and Cozystack installs it in two places: on the host through `cozystack.gateway-api-crds`, and inside every guest cluster that sets `addons.gatewayAPI.enabled`. Its second rule reads the `gateway.networking.k8s.io/bundle-version` annotation off an incoming CRD. At v1.5.1 that rule was a blacklist: reject anything matching `v1.[0-4].\d+` or containing `v0`, allow the rest. At v1.6.1 it is a whitelist: allow `v0.0.0-dev` outright, otherwise require the annotation to start with `v1.` and not match `^v1\.[0-4](\.|$)`. Evaluating both expressions over a spread of annotation values: | bundle-version | v1.5.1 policy | v1.6.1 policy | | --- | --- | --- | | `v1.6.1`, `v1.5.1`, `v1.5.0-dev`, `v1.10.0` | allow | allow | | `v1.4.0`, `v1.4.10`, `v1.0.0` | reject | reject | | `v0.0.0-dev` | reject | **allow** | | `v2.0.0` | allow | **reject** | | `1.6.1` (no leading `v`) | allow | **reject** | | `v1.4` (no patch) | allow | **reject** | | `main`, empty string | allow | **reject** | Nothing in this repository trips the new rejections: every CRD the bundle ships is stamped `v1.6.1`. What is exposed is an operator applying a Gateway API CRD set from somewhere else on a cluster that carries the policy. A fork, a vendor build or a nightly whose annotation reads `1.6.1` or `v2.0.0` used to go in and is now denied with `reason: Invalid`, naming a policy the operator may not know is installed, and whatever HelmRelease owns those CRDs then sits un-Ready. Uninstalling the policy is the escape hatch, and the denial message says so. The same hunk fixes an upstream inconsistency: at v1.5.1 the policy pair was stamped `v1.5.0-dev` while its twelve CRDs said `v1.5.1`. At v1.6.1 all fourteen documents agree. The Gateway and HTTPRoute objects the tenant gateway controller renders were serialized across all three certificate modes and checked against both the v1.5.1 and the v1.6.1 schemas; the two runs are identical, so the bump does not change what the controller can create. `go.mod` still pins `sigs.k8s.io/gateway-api` v1.4.1 and this PR deliberately leaves it there. The CRD bundle and the Go client types have been on separate versions since before this change, and nothing the controller emits depends on the gap. One thing to know before the next bump: from v1.6 on, the two things upstream calls the experimental bundle disagree. This package follows `config/crd/experimental`, the kustomization, which emits 12 CRDs at v1.6.1; the published `experimental-install.yaml` for the same tag emits 13, the extra one being `xbackends.gateway.networking.x-k8s.io`, new in v1.6 and not listed in the kustomization. At v1.5.1 the two agreed, so there was nothing to choose between. Nothing in this repository references XBackend and no vendored CRD depends on it, so following the kustomization costs nothing today, but whoever bumps next inherits the choice, and it should be made deliberately rather than by whichever command gets typed. The second commit corrects a comment this bump invalidated. It claimed TLSRoute stays at `v1alpha2` through Gateway API v1.5 and that the promotion to `v1` was still upstream-only; TLSRoute has been served at `v1` since v1.5.0, so the claim was already wrong before this bump and the version range it named is now wrong too. The third commit closes a gap that correction exposed. `cozystack-route-hostname-policy-tls` named `v1alpha2` alone while the bundle serves `tlsroutes` at `v1`, `v1alpha2` and `v1alpha3`: three served versions against one named. The gap is not new and this bump neither creates nor widens it: the v1.5.1 bundle and the v1.6.1 one serve the same three versions, with the same two marked deprecated. Coverage of the two unnamed versions rested entirely on request conversion, because `matchPolicy` is unset on both rules in that 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. The rule now names all three, which is the shape the HTTPRoute rule above it already used, and `hack/route-hostname-policy-version-coverage.bats` reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, so a future bundle move cannot outrun the list silently, in either direction. That guard carries negative fixtures rather than only watching a correct tree pass. Three of them drive `assert_covers` itself, with the two readers redefined in a subshell, against an omitted served version, an empty served set and an empty named set. Each requires the specific complaint text rather than merely a non-zero exit, so a helper renamed out from under it fails for the right reason instead of reading as a successful rejection. A fourth drives the comparison helper directly. The fourth commit pins the vendored bundle version in the CRD package's unit suite. Nothing outside the package Makefile named a Gateway API version, so re-running `make update` against a different ref landed green: twelve CRDs, every one stamped, document count unchanged, suite passing. The new case reads the `gateway.networking.k8s.io/bundle-version` annotation off every rendered CRD and fails when it is not the version the Makefile pins. The fifth commit extends the version-coverage guard to `cozystack-gateway-hostname-policy`, which names the `gateways` versions literally the same way the two route policies do, and whose version set lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Two notes for whoever reads the release: the TCPRoute and UDPRoute storage version moves from `v1alpha2` to `v1`, and while nothing in this repository ships either kind and the CRDs declare no conversion block, an upgraded cluster accumulates both entries in `status.storedVersions` — inert today, load-bearing only if a later bundle stops serving `v1alpha2`. The sixth commit gives GRPCRoute the same hostname rule the other two route kinds have. GRPCRoute carries `spec.hostnames` 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 claim any hostname on the plaintext listener with nothing checking it. Tenants hold no `gateway.networking.k8s.io` RBAC in Cozystack, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The rule is the HTTPRoute rule with `grpcroutes` in place of `httproutes` and the one version the bundle serves for it, and the coverage guard pins that list to the bundle like the others. Several places in the tree enumerate which route kinds the hostname policies cover, and all of them named two: the port-443 kinds invariant in the tenant gateway controller and the test that pins it, `packages/extra/gateway/README.md`, and the threat model together with the self-assessment. They name three now. The port-443 kinds set also carries an accurate reason now. 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. The README had also described HTTPS and TLS-passthrough listeners as carrying different kinds, where the controller gives both the same one. One more commit makes the suite's existing assertions bite. A ValidatingAdmissionPolicyBinding whose `policyName` points at another policy renders without complaint and leaves its own policy inert, and the suite pinned `validationActions` on the bindings while never pinning `policyName` at all, so pointing the GRPCRoute binding at the HTTPRoute policy kept every case green with the GRPCRoute control doing nothing. All three bindings now pin the policy they name. In the same file, `uncovered()` assigned to the variable names its caller reads back when printing a mismatch, so a call site with the arguments swapped would have printed a diagnostic naming the wrong side. Two things are named rather than made. The version-coverage guard collects `apiVersions` from every `resourceRules` entry mentioning the resource without also matching on `apiGroups` or `operations`, so a rule in another group, or one scoped to a single verb, would donate its versions to the set coverage is measured against; the chart's own suite pins the group and the resource on the rule it checks, which closes that a layer below. And `packages/system/cozystack-api/templates/api-tlsroute.yaml` renders `gateway.networking.k8s.io/v1alpha2` while the bundle serves `v1` as storage and marks `v1alpha2` deprecated, but the identical pin sits in the kubevirt and kubevirt-cdi TLSRoute templates, and the pinned `sigs.k8s.io/gateway-api` carries no `apis/v1` TLSRoute type for the controller side to follow, so moving one of the three here would leave the tree less consistent than leaving all three. Closes #3789. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file: neither `packages/system/gateway-api-crds/Makefile` nor its rendered CRD template matches any entry. Leaving every box empty rather than ticking the "no downstream repository" line, because the docs site carries a Gateway API version statement that is already inaccurate and needs a decision that is not mine: `content/en/docs/next/networking/gateway-api.md` and its v1.5 and v1.6 copies say Gateway API v1.5 ships TLSRoute at `v1alpha2` and that the graduation to `v1` is still upstream-only, when v1.5.0 already shipped TLSRoute `v1` as the storage version. ### Release note ```release-note chore(gateway-api-crds): update the vendored Gateway API CRD bundle to v1.6.1. TCPRoute and UDPRoute gain a served v1 that becomes their storage version, with v1alpha2 still served and now deprecated; every other kind keeps the versions it served at v1.5.1. Upstream removed `sessionPersistence.idleTimeout` from HTTPRoute, GRPCRoute and XBackendTrafficPolicy; the field is not set by anything Cozystack renders, but a hand-written route that carries it loses it silently on its next write, with no admission error. The bundle's `safe-upgrades.gateway.networking.k8s.io` ValidatingAdmissionPolicy changes which CRDs it accepts. Cozystack installs it cluster-wide on the host and in every guest cluster with `addons.gatewayAPI.enabled`. Its `gateway.networking.k8s.io/bundle-version` rule was a blacklist on two patterns and is now a whitelist on the `v1.` prefix with a `v0.0.0-dev` escape hatch. A bundle annotated `v0.0.0-dev` is now accepted where it was denied. A bundle annotated `v2.0.0`, `1.6.1` without the leading `v`, `main`, `v1.4` without a patch, or carrying an empty annotation is now denied where it was accepted. Nothing Cozystack ships is affected, since every CRD in the bundle is stamped `v1.6.1`, but applying a Gateway API CRD set from a fork, a vendor build or a nightly on such a cluster can now fail admission with `reason: Invalid`. Uninstalling the policy remains the documented escape hatch. fix(cozystack-basics): the tenant TLSRoute hostname admission policy now names every served TLSRoute version (`v1`, `v1alpha2`, `v1alpha3`) instead of `v1alpha2` alone. Coverage of the other two rested on request conversion, which holds only while a named version stays served, and a rule left selecting nothing is not caught by `failurePolicy: Fail`. feat(cozystack-basics): GRPCRoute hostnames in `tenant-*` namespaces are now gated by the same admission policy that gates HTTPRoute and TLSRoute. A tenant Gateway's port-80 listener leaves `allowedRoutes.kinds` unset, which admits GRPCRoute by protocol default, so a GRPCRoute could claim any hostname on the plaintext listener unchecked. Tenants hold no `gateway.networking.k8s.io` RBAC, so this is defence-in-depth against a chart bug rather than a tenant-user control. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added hostname admission validation for `GRPCRoute` resources. - Expanded `TLSRoute` hostname validation to support all served Gateway API versions. - Ensured route hostname policies consistently cover HTTP, TLS, and gRPC routes. - **Security** - Updated listener policies to restrict port-443 routes to supported HTTP and TLS route types. - Clarified fail-closed hostname validation behavior across Gateway, Route, and Ingress resources. - **Documentation** - Updated security and threat-model documentation to include GRPCRoute handling and validation coverage. - **Maintenance** - Updated the bundled Gateway API CRDs to version 1.6.1. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
b7e5100e73 |
fix(monitoring): strip infrastructure metadata from tenant logs (#2196)
## Summary - Adds a `modify` filter in fluent-bit that removes `kubernetes_node_name` and `kubernetes_host` from all container logs - Prevents infrastructure topology (node names, hosts) from leaking to tenant users ## Problem `kubernetes_node_name` is exposed in log metadata, leaking node topology to end-users. Other sensitive fields like `kubernetes_host` are also visible. Relates to #2194 ## Test plan - [ ] `helm template` monitoring-agents and verify `Remove kubernetes_node_name` filter is present - [ ] Deploy and verify `kubernetes_node_name` is absent from logs in tenant Grafana ```release-note Strip infrastructure metadata (node name, host) from tenant logs ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved Kubernetes monitoring log processing by filtering out most `kubernetes_*` fields for `kube.*` events, while preserving the key Kubernetes identity fields (such as pod, namespace, and container-related names) for clearer, more consistent log output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
7786f72b9f |
feat(etcd-operator): bump to v0.5.5 and vendor EtcdDefragPolicy CRDs (#3968)
## What this PR does Bumps the vendored `etcd-operator` from **v0.5.4** to **[v0.5.5](https://github.com/cozystack/etcd-operator/releases/tag/v0.5.5)** and adopts the new defrag API. v0.5.5 ships the `EtcdDefrag` and `EtcdDefragPolicy` types — a controller that runs a one-shot defrag against a target etcd cluster, plus a cron-style policy that stamps scheduled `EtcdDefrag` runs (`schedule`/`suspend`/`concurrencyPolicy`/`historyLimit`). - **`etcd-operator-crds`**: vendor the two new CRDs (`etcddefragpolicies`, `etcddefrags`) alongside the existing three; bump `ETCD_OPERATOR_REF`. Re-vendoring also refreshed `etcdmembers.yaml` (a doc-string change from the version-matched-`etcdutl` restore work in v0.5.5). All five carry `helm.sh/resource-policy: keep`; the existing resource-policy suite covers the new CRDs via its wildcard template scope. - **`etcd-operator`**: bump `appVersion`/image tag to v0.5.5 and re-vendor the manager `ClusterRole` from the operator's `charts/etcd-operator/files/manager-role-rules.yaml`. New grants match what the defrag controllers need: full CRUD on `etcddefrags`, read/patch/watch on `etcddefragpolicies`, the two new `/status` subresources, and cluster-scoped `events` create/patch. Fixed the stale `config/rbac/role.yaml` path in the Makefile/test comments (upstream moved the role rules). ### Screenshots n/a — no UI changes. ### Downstream repositories Walked the trigger map against the diff. The change is confined to `packages/system/etcd-operator{,-crds}`: etcd-operator is a system component (not a `packages/apps`/`packages/extra` package the website and TF provider track), the new CRDs are in group `etcd-operator.cozystack.io` (not `cozystack.io`/`backups.cozystack.io`, the only CRD groups the TF provider types), and nothing under `hack/`, the installer values, or the platform values changed. - [x] No downstream repository is affected by this change ### Release note ```release-note feat(etcd-operator): bump to v0.5.5, adding the EtcdDefrag and EtcdDefragPolicy APIs for on-demand and scheduled etcd defragmentation ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added APIs for configuring recurring and one-time etcd defragmentation tasks. * Added status reporting for defragmentation progress, results, and conditions. * Added validation and lifecycle controls for defragmentation policies and runs. * **Enhancements** * Updated the etcd operator to version 0.5.5. * Expanded permissions to manage defragmentation resources and report events. * Improved restore documentation for etcd members. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
fb740db0b0 |
docs(security): separate the fail-open Gateway-listener policy from the fail-closed route policies
The self-assessment summarised the whole hostname policy set as fail-closed, while its own table row states that the Gateway-listener policy passes a Gateway whose namespace carries no host label. Say which policies are which. The port-443 kinds comment in the reconciler test also tied the GRPCRoute admission rule to the port-80 listener; the rule matches the kind wherever it attaches, and the port-443 exclusion is a least-privilege choice, not the reason the rule exists. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
19eaff6a30 |
test(cozystack-basics): pin which policy each route binding names
A ValidatingAdmissionPolicyBinding whose policyName points at another policy renders without complaint and leaves its own policy inert. The suite pinned validationActions on the bindings but never policyName, so pointing the GRPCRoute binding at the HTTPRoute policy kept all 56 cases green while the GRPCRoute control did nothing. uncovered() assigned to the same variable names its caller reads back when printing the mismatch, so a future call site with the arguments swapped would print a diagnostic naming the wrong side. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
af19aa933c |
docs(security): record GRPCRoute in the route hostname policy set
The port-443 kinds set still excludes GRPCRoute, TCPRoute and UDPRoute. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. TCPRoute and UDPRoute additionally carry no hostname and no admission rule gates them. The gateway README described HTTPS and TLS-passthrough listeners as carrying different kinds. They carry the same one, which is what cilium#45559 requires. The threat model and the self-assessment enumerate which route kinds the hostname policies cover, and both listed two. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
6ddae65b45 |
test(cozystack-basics): cover the Gateway listener policy in the guard
cozystack-gateway-hostname-policy names the gateways versions literally, the same way the two route policies do, and the set it must cover lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
72437d127f |
test(gateway-api-crds): pin the vendored bundle version in the suite
Nothing outside the package Makefile named the Gateway API version, so re-running `make update` against a different ref landed with the whole suite still green: twelve CRDs, every one stamped, document count unchanged. The new case reads the bundle-version annotation off every rendered CustomResourceDefinition and fails when it is not the version the Makefile pins. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9311a48f6a |
test(cozystack-basics): make the version-coverage guard prove it still compares
The guard added alongside the TLSRoute version fix only ever ran against a tree that already satisfied it. A passing check that is checking nothing looks exactly like a passing check that is checking something, so a later refactor could empty out the comparison and the suite would stay green forever. That is the same shape as the negated assertions that cannot fail elsewhere in this repository: the assertion survives, the guarantee does not. Drive the checker with synthetic input instead of only with the live tree, the way hack/bats-no-exit-trap.bats and hack/md-no-hardwrap.bats feed their checkers a broken sample and require a complaint. Two layers need it separately. The comparison itself moves into uncovered() so a fixture can call it with a served set the named set does not cover. Above that, assert_covers runs in a subshell with both readers replaced, so the wiring, the empty-set guards and the exit status are exercised too -- covering only the inner function leaves the outer one free to become a no-op while every other case in the file stays green, because the live-tree cases are its only callers and the tree they read is correct. Each fixture requires the complaint it expects rather than any non-zero exit. A renamed helper exits 127 and an unbound variable under set -u exits 1, and either would read as a successful rejection if only the status were checked; a rejection for the wrong reason is the same false comfort as no fixture, and harder to notice because the red looks earned. Each way of breaking the guard now fails at its own case. Emptying uncovered(), reducing assert_covers to a no-op, and deleting the empty-set guards each turn a different fixture red while the live-tree cases stay green, and changing only the wording of assert_covers' complaint -- which still rejects, just for an unstated reason -- turns the omission fixture red on the message rather than passing on the exit status. Select a resourceRules entry by comparing each resource name for equality, and match versions as fixed strings rather than patterns. yq's contains() compares array elements by substring, so a rule scoped to a subresource would have satisfied a search for the parent: a rule naming tlsroutes/status answers a query for tlsroutes and donates its apiVersions to the set the guard checks coverage against. That direction is the wrong one for a fail-closed check -- it makes coverage succeed where the real rule falls short. No rule in this policy is scoped that way today. Also pin apiGroups on the TLSRoute document in the chart suite. The HTTPRoute document has always had that assertion; the asymmetry meant the case named after both kinds only checked the group for one of them. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Aleksei Sviridkin
|
9ea4a961ec |
fix(cacert): make the comments match the reconciler
Comments in the CA-extraction package described guards the code does not have. ManagedLabel: the anti-clobber decision is made by isOurProjection, from the Secret type and an owner reference back to the sentinel, not from the marker, which no decision keys on beyond the drift check. managedByCozystackLabel: the reconciler does mutate it. It deletes the label on a selectors-digest change, which is what hands the projection back to the lineage webhook for a fresh verdict. projectionSuffix: the projection name is kept clear of engine CA Secrets by a dot, and a release name cannot carry one, but the suffix an operator appends can and nothing validates it. A suffix ending in ".tenant-ca" is the only one that reaches a canonical name. TestProjectionNameCannotCollideWithADotFreeEngineSuffix: both its doc and its name claimed the property for any engine suffix while the loop samples only dot-free ones, which is the same overstatement one file over. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
eb175aa5a9 |
chore(gateway-api-crds): vendor the Gateway API v1.6.1 CRD bundle
Re-renders the experimental kustomization at v1.6.1. The bundle still carries the same twelve CRDs plus the safe-upgrades admission policy pair, and every CRD keeps its helm.sh/resource-policy: keep stamp. TCPRoute and UDPRoute gain a served v1 that takes over as their storage version; their v1alpha2 stays served, now marked deprecated. No other kind changes its served versions, so the group/version pairs the Cilium operator and the tenant gateway controller resolve -- GatewayClass, Gateway, HTTPRoute and GRPCRoute in v1, ReferenceGrant in v1beta1, TLSRoute in v1alpha2 -- are all still served. Schema changes land on fields this repository does not set: sessionPersistence.idleTimeout is dropped from HTTPRoute, GRPCRoute and XBackendTrafficPolicy, HTTPRoute gains a lower bound on retry.attempts and rejects duplicate retry.codes, and ReferenceGrant now requires spec. Gateway infrastructure annotations, frontend CA certificate refs and TLSRoute hostnames all get larger upper bounds. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
b24ed36f00 |
test(mariadb): bind the uninstall Secret names to the delete that removes them
Both cleanup suites asserted the delete as "(?s)kubectl delete secret -n \S+.*<name>". Under (?s) the dot matches a newline, so the gap runs past the end of the command: any earlier "kubectl delete secret" satisfies the opening and the names may then sit on any later command in the script. The pattern never established that they are arguments of the delete at all. Moving the two cert-manager Secrets off the delete onto a "kubectl get", leaving the password delete above them untouched, keeps both suites green while the CA private key and the server private key outlive the release in a namespace the tenant shares with its other applications — the one outcome this hook exists to prevent. The comments beside the assertions already claimed the verb was pinned, so a reader trusting them had no reason to re-check; they stand unchanged because the patterns now do what they say. Pin the whole invocation with its argument list instead, spelling out the shell's line continuations rather than stepping over them. Whitespace and backslashes are the only separators the pattern admits, and no command can be written in those, so a match cannot leave the command it began in. That mutation now reddens both suites, as does dropping one name from the list, as does swapping the verb for a read. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3fd032e28b |
docs(mariadb): say the CA Secret is excluded by name, not unnamed
The clause claimed the ApplicationDefinition names neither Secret. It names this one: "mariadb-<name>.ca-tls" sits in its secrets.exclude list, and the bats guard pins it there. What is true is narrower — neither is named as the source the anchor is published from — so say that and say where the name does appear, or the sentence invites a reader to conclude the ApplicationDefinition has no reference to the Secret at all. Rewrap the SAN-list paragraph's one 124-column line back into the block it sits in, which is otherwise at 78. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
55a4046541 |
test(mariadb): pin the schema rejection of a misspelled tls.issuer
The issuer resolver maps anything that is not exactly "cert-manager" to operator issuance, so a typo fails open: the instance keeps the operator's CA and nothing at render time reports that the value was not understood. The enum is the only thing that refuses it, and one kubebuilder marker generates that enum into both the chart's values.schema.json and the ResourceDefinition's copy, so a single lost marker opens both guards at once. The suite already pins the rejection for the sibling field on this object and left this one, the direction that fails quietly, uncovered. Removing the enum from the schema fails this case and no other. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2e759c7008 |
docs(mariadb): correct what names the trust anchor and what lineage resolves
Two claims the chart makes about itself stopped being true when the anchor moved to a projection. certmanager.yaml said the ApplicationDefinition names the operator's key-free "<release>-ca-bundle" as the publication source. It does not: the TenantProjection this chart renders names it, and the ApplicationDefinition carries no name for it at all — it selects the published anchor by label. The ApplicationDefinition's own include comment then listed the two cert-manager Secrets among those the lineage webhook resolves to this instance. The webhook resolves an object through ownerReferences, or failing that the Flux helm label, and returns nothing when it has neither. A cert-manager Secret has neither while enableCertificateOwnerRef stays false, which is the upstream default this platform does not override, and is the same fact the uninstall hook relies on when it deletes those Secrets itself. What does resolve is the operator's own "-ca", "-server-cert", "-client-cert" and "-ca-bundle". The two names stay in exclude, because one setting flip is all it takes to make the sentence true. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4758718d66 |
fix(mariadb): separate the chart-chosen Secret suffixes with a dot
The chart asks cert-manager for two Secrets and names them after the release: "<release>-ca-tls" for the CA and "<release>-tls" for the leaf. The release prefix is "mariadb-", so an application named foo produces mariadb-foo-ca-tls, and an application named foo-ca produces mariadb-foo-ca-tls as well. One release's CA private key and another release's server private key claim the same object. Two sibling applications in one tenant namespace are enough to hit it: whichever cert-manager writes last owns the Secret, and each instance then serves or trusts material issued for the other. The suffix set is what makes it ambiguous. It holds both "-tls" and "-ca-tls", and one is a tail of the other, so a hyphen leaves nothing to say where the release name ends. A dot does: an application name is validated as a DNS-1035 label and the release prefix as ^[a-z0-9-]*$, so neither can contain one, and "<release>.ca-tls" and "<release>.tls" split uniquely. Secret names are DNS-1123 subdomains, which admit the dot. Only the two suffixes this chart chooses move. The Certificate and Issuer object names keep the hyphen, because their suffixes are not a pair where one is a tail of the other, and the operator-managed "<release>-ca-bundle" that the trust-anchor projection reads is not this chart's to rename. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrey Kolkov
|
e94fcf8219 |
feat(backups): RabbitMQ definitions backup via a dedicated strategy driver (#3990)
## What this PR does Adds backup/restore support for the managed `RabbitMQ` application. RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no consistent online export of message payloads, so this backs up broker **definitions** (vhosts, users, permissions, queues, exchanges, bindings, policies, parameters) over the management HTTP API (`GET`/`POST /api/definitions`) and stores them in the system bucket. Message payloads and point-in-time recovery are intentionally out of scope; a full-data backup would be a Velero volume snapshot, which is a separate strategy. It is delivered as a **first-class `strategy.backups.cozystack.io/Rabbitmq` driver**, following the Altinity pattern (a dedicated driver that renders a one-shot Job for an HTTP-API engine): - `reconcileRabbitmq` / `reconcileRabbitmqRestore` render the strategy's `PodTemplateSpec` — `curl` `GET`/`POST /api/definitions`, pushing/pulling the JSON to S3 with `curl --aws-sigv4` — as a `batch/v1.Job`, covering both restore flows: in-place (into the source broker) and to-copy (into a separate, differently-named RabbitMQ in the same namespace, authenticating with the target's own default-user credentials and reading the source's object key). - A **Go-read precondition** gates each run on the `RabbitmqCluster` reporting `AllReplicasReady` (read through a small `rabbitmqtypes` partial type), so a not-yet-ready broker surfaces a precise `Ready=False` wait on the BackupJob/RestoreJob rather than a Job that starts and fails against an unreachable management API. - The produced `Backup` records the stored object's location on `status.artifact.uri`, rendered from the strategy's `spec.artifactURITemplate`. The `Backup` CRD has no status subresource, so this persists on Create, mirroring the Etcd driver. Dispatch is wired in both controllers (plus `supportedBackupStrategyKinds`), `rabbitmqtypes` is registered in the scheme, and RBAC grants read on `rabbitmqclusters.rabbitmq.com` (the Job itself is already covered by the existing `batch/jobs` grant). It ships `cozy-default-rabbitmq` (a `$bucketName`-gated `Rabbitmq` strategy) and routes `apps.cozystack.io/RabbitMQ` to it in the `cozy-default` BackupClass, which also enrols it in the `DefaultObjectsGate`. The RabbitMQ application chart is left untouched — it had no prior backup wiring, and the strategy carries the S3 coordinates itself. Unit tests cover the driver's pure helpers (applicationRef validation, cluster-name derivation, the readiness-message builder, artifact-URI rendering, parameter round-trip) and the reconcile precondition gate (defers with `Ready=False` and no Job until the cluster is ready, then proceeds). A runnable example under `examples/backups/rabbitmq/` and an honest-roundtrip Chainsaw e2e under `hack/e2e-chainsaw/rabbitmq/` seed a sentinel vhost, back it up, drop it, and assert it round-trips through object storage on both restore flows. The Job pod reuses the existing `platform-migrations` image (curl + ca-certificates) under a new `backupStrategyController.rabbitmqBackupClientImage` key, stamped in lockstep with `chBackupClientImage` in `packages/core/platform/Makefile` so the repository stays pinned at a single digest (`hack/image-pin-consistency.bats`). A dedicated slim image built by this package is a follow-up. **Validation:** `go build`/`go vet`/`gofmt`/`go test ./internal/backupcontroller/... ./internal/template/...`, deepcopy + Rabbitmq CRD generation, `helm template` proving the CRD-ordering gate both on and off, `helm lint`, and `helm unittest` (18/18). The backup + in-place + to-copy roundtrip was verified end-to-end on a dev cluster (a real management-API export and a real S3 `--aws-sigv4` upload/download against SeaweedFS), so the driver's core contract is exercised on a real cluster; the in-CI Chainsaw run is the remaining automated evidence. The object-cleanup, terminating-namespace teardown and readiness-deadline paths added during review are unit-tested (including `Reconcile`-level finalizer retention and the terminating-namespace release) but not yet cluster-exercised. (Note found while testing: RabbitMQ 4.2 is OOMKilled on the `nano` preset, so the example/e2e use `c1.small`.) ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff. This is an additive backup strategy plus a new strategy CRD; no downstream repository is mechanically forced by the change. A documentation page for RabbitMQ backup on [cozystack/website](https://github.com/cozystack/website) would be a reasonable follow-up, but it is not required by this diff — leaving the boxes unticked for a human to decide rather than ticking "no downstream" to clear the checklist. ### Release note ```release-note feat(backups): add RabbitMQ definitions backup/restore via a dedicated strategy.backups.cozystack.io/Rabbitmq driver; it gates each run on the RabbitmqCluster being ready and records the stored object on Backup.status.artifact.uri. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added RabbitMQ backup and restore support for definitions, including vhosts, users, queues, exchanges, bindings, policies, and related settings. * Added in-place and restore-to-copy workflows with readiness checks. * Added scheduled and on-demand backup examples. * Integrated RabbitMQ backups with the default backup configuration. * Backups store definitions only; message payloads and point-in-time recovery are not included. * **Documentation** * Added a complete RabbitMQ backup and restore demonstration guide. * **Tests** * Added end-to-end coverage for readiness and backup/restore round trips. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
6afe69b35b |
chore(etcd-operator): retrigger CI
The prior CI run stalled in the self-hosted runner queue for days and never scheduled; an empty commit forces a fresh run on the same tree. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
5896484c4a |
docs(etcd-operator): fix stale role-rules vendoring path in rbac.yaml comment
The manager ClusterRole header still pointed at the upstream config/rbac/role.yaml path, which 404s at v0.5.5; the rules now live in charts/etcd-operator/files/manager-role-rules.yaml (already corrected in the Makefile and rbac_test.yaml). Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
85cbb3d7cb |
feat(etcd-operator): bump to v0.5.5 and vendor EtcdDefragPolicy CRDs
v0.5.5 ships the EtcdDefrag and EtcdDefragPolicy API types (a controller plus a cron that stamps scheduled defrag runs). Vendor both CRDs into the etcd-operator-crds package and grant the manager ClusterRole the verbs the new controllers need: defragpolicies read/patch, defrags full CRUD, the two new /status subresources, and cluster-scoped event emission. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
