mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-02 10:06:53 +09:00
11bac54f931682dd94de4081bb9e8a7eef60bef1
291
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c290dee945 |
fix(vm-instance)!: refuse resources that cannot size the VM
resources.cpu and resources.sockets are typed as quantities, but they land in domain.cpu.cores and domain.cpu.sockets, which KubeVirt declares as uint32. The chart cast them with int64, which turns 500m, 1.5 or 1e1 into 0 without an error, and KubeVirt boots a zero core count as one core. The user asked for half a core and got a one-core VM with nothing reporting it. The raw spelling is now decoded as YAML, the way an integer field reads it, and the render stops unless the result is a positive whole number that fits in uint32. A leading zero is refused, because YAML reads 010 as octal 8. The check keys on presence rather than truthiness, so an unquoted 0 is refused instead of being read as unset. Memory gets the same presence check. A quoted "0" was truthy and rendered requests.memory: "0", while an unquoted 0 was falsy and dropped, so one quantity took two paths. A zero or negative amount now stops the render in both spellings. BREAKING CHANGE: values that used to render now either fail or render differently. A cpu or sockets that does not decode to a plain whole number within uint32 (a millicore, suffixed such as 1k, fractional, zero, negative or leading-zero value), and a zero or negative memory, fail the next upgrade of the release until the value is fixed. This includes cpu or sockets set on its own beside an instanceType, which used to be ignored whatever it held. A spelling that the int64 cast truncated to 0 but that YAML decodes to a whole number, such as the exponent 1e1 or the trailing dot 2., used to boot one core and now sizes the VM with the count it spells, so such a VM changes size on upgrade. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
65a038caa4 |
docs(api): separate the healthCheckExprs failure modes
The field doc filed every mistake under one outcome, a HelmRelease that hangs until its timeout. helm-controller treats them differently. An expression that does not parse is rejected before any Helm action and stalls the release at once, with a named reason. One that parses but errors on the object keeps the wait polling with the CEL error attached, and a current expression that evaluates cleanly but never turns true runs the wait out with no error at all. A read of status errors until the CR's controller writes one, even inside has(), but the wait survives it. A wrong group or kind silently falls back to the built-in readiness rules, and a CR whose CRD is missing fails the Helm action before the wait starts. The text ships in the CRDs and is what kubectl explain shows, so a reader who cannot check it against the controller source was sent to the wrong place for the parse error and the missing CRD. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
4bf95815a8 |
fix(kubernetes): render the aggregation-layer UID header for tenant OIDC (#3539)
## What this PR does Tenant clusters with `spec.oidc.mode: System` or `CustomConfig` authenticate users fine, but a UID does not survive the hop to an aggregated API server. The tenant kube-apiserver publishes `requestheader-uid-headers` into the `extension-apiserver-authentication` ConfigMap only when the flag is set, and the chart never set it. The aggregation layer still sends `X-Remote-Uid`, but the extension server trusts only the header names published in that ConfigMap, so it ignores the one it gets and the call arrives without a UID. The field documentation even told operators to add the flag by hand via `controlPlane.apiServer.extraArgs`. It forwards a UID rather than creating one. ServiceAccount tokens carry one, and for those the aggregation hop starts working. Users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only; giving them one means mapping a claim, which changes the identity of users who have none today, and is tracked separately. The chart now renders `--requestheader-uid-headers=X-Remote-Uid` on v1.32 and newer, and adds `--feature-gates=RemoteRequestHeaderUID=true` on v1.32, where the gate is still alpha and the apiserver rejects the flag without it. From v1.33 the gate is beta and on by default, so only the header flag is rendered. v1.31 gets nothing: the flag does not exist there, and writing it by hand stops the apiserver from starting. `--emulated-version` in the same passthrough moves that decision, because the gate resolves through the versioned spec at the emulated version. Rendering the gate on every version from v1.32 rather than on v1.32 alone was the alternative, and it would remove the migration step entirely, because an opt-out would then drop two entries instead of one and move the entry count by itself. It loses on what it leaves behind: a redundant gate term on every cluster, which stops the apiserver from starting on the day the gate is retired from the known list. The design also prefers rendering over failing for the same reason the fail branches are narrow, since a cluster stranded on a HelmRelease that no longer converges is a worse outcome than one that never gets the UID fix. The chart reads an operator entry the way the apiserver reads it: header list elements matched whole and untrimmed and folded as `EqualFold` folds them, gate names compared exactly, gate values resolved as `strconv.ParseBool` resolves them, `AllAlpha` and `AllBeta` blankets honoured, an empty value treated as no entry. On v1.32 an entry that says nothing about `RemoteRequestHeaderUID` gets a second one carrying the operator's own terms plus `RemoteRequestHeaderUID=true`. Carrying those terms into the merged copy is what makes it land correctly whether the tenant control plane collapses duplicate flags into the last one or hands both to the apiserver, which merges them in the order it reads them. Tests cover the render matrix over mode, version and operator input: every fail path with its exact message, the ownership and merge paths, duplicate entries in both orders, blanket gates, empty values, and case variants on the header name and on the gate value. The latest-version tenant lane creates its tenant with `spec.oidc.mode: System` and asserts the flag on the rendered KamajiControlPlane, so the tenant apiserver starts with the flag in CI. Nothing asserts the UID arriving at an aggregated apiserver; that needs an extension server inside the tenant and is not in this change. The layer where the count-keyed reset actually bites is the tenant Deployment's arguments, and asserting there needs a live tenant control plane, so it is not in this change either. The rule this follows is that a configuration which renders and boots today keeps rendering. Both flags are new, so an `extraArgs` shape reaching this code was rendering before it existed, and stranding a running tenant on a HelmRelease that no longer converges is a worse outcome than one that never gets the fix. A few shapes do stop rendering, and reaching any of them takes writing `--requestheader-uid-headers` into the passthrough by hand, attempting exactly what the chart now does and getting it wrong; in each the kube-apiserver fails option validation and never starts, so nothing that was working is lost. `docs/oidc-tenant.md` covers these shapes. One known limitation is in there and worth naming here: on v1.33 and newer, adding the documented opt-out is count-neutral in `extraArgs`, and so is removing it again, and on v1.33 and v1.34 adding or removing `--emulated-version=1.31`. The pinned Kamaji build rewrites a running apiserver's arguments only when that count changes and otherwise keeps every flag that has left the list, so either the header flag stays beside a now-disabled gate or the disabled gate stays beside the header flag, and the apiserver refuses to start. Changing the entry count in the same edit is the mitigation, and the fix belongs to the pin rather than to this chart, tracked in #3541. The paths that need no operator edit are clear: turning the feature on for a default tenant moves the count, and a tenant already carrying the opt-out renders byte-identically before and after. Out of scope: `mode: None` still publishes no UID header even though ServiceAccount tokens carry UIDs. That is pre-existing, and widening the condition changes the render for every default tenant. Tracked separately, along with the claim mapping above. Closes #3377 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change Checked the trigger map against the diff: no package added or removed, no schema field, enum or default changed (description text only), no ApplicationDefinition surface touched, no hack/ layout moved. The managed-apps reference page picks up the new field description through release-time README regeneration. ### Release note ```release-note fix(kubernetes): tenant clusters with `spec.oidc.mode` set to `System` or `CustomConfig` now render `--requestheader-uid-headers=X-Remote-Uid` on the tenant kube-apiserver, plus the `RemoteRequestHeaderUID` feature gate on v1.32, so that an aggregated API server can read the caller's UID out of the request headers. Without that flag the tenant apiserver never publishes `requestheader-uid-headers`, and an aggregated API server ignores the UID header even for callers that have one, ServiceAccount tokens among them. It forwards a UID rather than creating one: users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only, and that is tracked separately. Adding the flag by hand through `controlPlane.apiServer.extraArgs` is no longer necessary, and an entry of your own is respected: the chart steps aside when yours already carries `X-Remote-Uid` or already enables the gate, merges `RemoteRequestHeaderUID=true` into a copy of a v1.32 `--feature-gates` entry that says nothing about the gate, and renders neither flag when you switch the gate off. The render fails where a `--requestheader-uid-headers` list of your own would stop the kube-apiserver from starting anyway: one that omits `X-Remote-Uid`, one with an empty element, and a list of your own while the gate is off. On v1.33 and newer there is a migration step, and it applies in both directions: the pinned Kamaji build rewrites a running apiserver's arguments only when the number of `extraArgs` entries changes, and otherwise keeps every flag that has left the list. Adding the documented opt-out keeps that number the same, so the header flag already on the Deployment stays beside the now-disabled gate. Removing the opt-out later keeps it the same too, so the disabled gate stays on the Deployment beside the header flag the chart renders again, although your spec no longer shows it. Adding or removing `--emulated-version=1.31` has the same shape both ways on v1.33 and v1.34. The apiserver refuses to start in each case, so change the entry count in the same edit. ``` |
||
|
|
12e64a99cd |
fix(rabbitmq): right-size the default resources preset to s1.nano (#3936)
## What this PR does
Raises the RabbitMQ default resource preset from `t1.nano` (128Mi
memory) to `s1.nano` (250m CPU / 512Mi memory).
A default-sized RabbitMQ 4.2 broker on Cozystack 1.6.1 was OOMKilled
with exit 137, reached 10 restarts, and never became Ready. A canary on
a larger preset reached 3/3 Ready with zero restarts; its working set
peaked at 125.8Mi. Against the 128Mi `t1.nano` ceiling that is 98% of
the limit in steady state, and cgroup accounting (working set plus page
cache) crosses it under load. The canary and its storage were removed
after validation.
### Sizing
`s1.nano` keeps the CPU of `t1.nano` (250m) and quadruples memory to
512Mi — a 4x margin over the observed 125.8Mi peak.
An earlier revision of this PR proposed `u1.nano` (1Gi). Review
correctly objected that 1Gi reserves 8x the observed working set with no
measured peak to justify it, and asked to either report that peak or
right-size the default. The peak is reported above, and the default is
right-sized to the smallest preset that clears the ceiling with a real
margin rather than to the first one that is comfortably large.
### Upgrade impact
cozystack-api applies OpenAPI schema defaults on the read path and
stores what the client sent: `Get` runs `applySpecDefaults` on its way
out (`pkg/registry/apps/application/rest.go:1232`) and the write puts
`app.Spec` into `HelmRelease.spec.values` (`rest.go:1652`). That splits
existing brokers in two.
**Brokers with no stored `resourcesPreset`** pick up the new chart
default on the next Helm reconcile. For them this upgrade is:
- a rolling restart of every replica, and
- a rise in guaranteed memory reservation from 128Mi to 512Mi per
replica, because the sanitize helper sets memory request equal to limit
— roughly 1.1Gi extra reserved per 3-replica instance.
**Brokers whose values went through a read-modify-write keep `t1.nano`
and are not resized.** `Update` opens with `r.Get` (`rest.go:489`), so a
GET-then-PUT — `kubectl edit` is the unambiguous case, and any client
that reads before writing behaves the same — freezes the schema default
of the moment into the stored values, and a stored value beats the chart
default. Those brokers are not restarted, not resized, and keep hitting
the ceiling this PR raises.
Nothing downstream can separate a deliberate `t1.nano` from one a round
trip baked in, so this is documented rather than migrated. After the
upgrade the affected brokers are the ones that still report the old
preset:
```
kubectl get rabbitmq -A -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,PRESET:.spec.resourcesPreset
```
The same command before the upgrade tells you nothing: the cozyrd
default is still `t1.nano` then, so the read path fills it in for both
groups. Afterwards, anything reading `t1.nano` carries a stored value;
setting `resourcesPreset: s1.nano` explicitly on the ones that should
move is the fix.
PVCs are untouched, so there is no data loss, but node headroom should
be checked before upgrading.
Also worth knowing when reading the parameter table: the preset does not
only apply to a fully omitted `resources`.
`cozy-lib.resources.defaultingSanitize` merges it key by key, so a
broker pinned to `resources: {cpu: 700m}` keeps its CPU and moves from
128Mi to 512Mi with everyone else. The field descriptions now say so,
and a test pins it.
The values, generated schema, ApplicationDefinition, typed API, and
README all carry the same default. Tests cover the new default, an
explicit legacy override, explicit resources, and a partially set
`resources` map inheriting the preset for what it leaves out.
Validation:
- `helm template` with the chart's `cozy-lib` dependency renders `limits
250m / 512Mi` and `requests 0.025 / 536870912` for the default; an
explicit `t1.nano` renders `128Mi / 134217728`; explicit `700m / 2Gi`
renders `0.07 / 2147483648`. The unit test asserts exactly these values.
- Provider counterpart: cozystack/terraform-provider-cozystack#34.
### Screenshots
Not a UI change.
### Downstream repositories
The default is restated by the Terraform provider, so the linked
provider PR updates it and documents the state-plan implication. No
other trigger-map entry matches this diff.
- [ ] 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:
https://github.com/cozystack/terraform-provider-cozystack/pull/34
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
fix(rabbitmq): raise the default resource preset from t1.nano to s1.nano so default-sized brokers are no longer OOMKilled. Brokers with no stored `resourcesPreset` are restarted on upgrade and their guaranteed memory reservation grows from 128Mi to 512Mi per replica; check node headroom before upgrading. Brokers whose values already pin `resourcesPreset`, including ones where a read-modify-write through the API froze the old default into them, keep their old size: list them after the upgrade with `kubectl get rabbitmq -A -o custom-columns=NAME:.metadata.name,PRESET:.spec.resourcesPreset` and set `resourcesPreset: s1.nano` explicitly on the ones that should move. The preset also fills any resource a partially set `resources` leaves out, so a broker pinned to cpu alone moves too. A tenant that opted into `resourceQuotas` and sits near its memory ceiling has the extra reservation rejected by quota admission, which shows up as a StatefulSet that never scales rather than as pods left Pending.
```
|
||
|
|
7d07bbe3ce |
[kafka] Migrate from ZooKeeper to KRaft mode (#2352)
## Summary - Replace ZooKeeper-based Kafka deployments with KRaft (Kafka Raft) metadata mode - Existing ZooKeeper instances migrate automatically and in place via a Helm `pre-install,pre-upgrade` hook Job that drives Strimzi's ZK-to-KRaft migration state machine - New instances deploy directly in KRaft mode with broker and controller `KafkaNodePool`s - The node pools are owned by the migration hook, not by regular chart manifests (see "Why the pools are hook-managed") - Kafka version is user-configurable (v3.9 default, v3.8 available) - Strimzi operator is unchanged in this PR (stays at 0.45.1-rc1); 0.45.x is the last line that supports ZooKeeper→KRaft migration - ZooKeeper removed from the dashboard UI (ApplicationDefinition) ## Migration strategy Both fresh installs and ZooKeeper→KRaft migrations are driven by a single Helm hook Job (`templates/migration-hook.yaml`, `pre-install,pre-upgrade`) that owns the `KafkaNodePool`s. On every install and upgrade the Job: 1. Ensures both node pools exist (idempotent): a broker pool named exactly `kafka` for a migrating cluster — so Strimzi adopts the existing `<cluster>-kafka-N` brokers and their PVCs in place — or `b-<hash>` for a fresh cluster, plus a controller pool `c-<hash>`. The controller pool pins `strimzi.io/next-node-ids` above the broker range so the broker pool keeps node IDs `[0..N-1]` and Strimzi cannot hand the existing broker IDs to the fresh controllers. The shared `kafka` name is claimed atomically (via `kubectl create`) so only one migration per namespace can win it. 2. Inspects the Kafka CR: no CR → fresh install (pools ensured; the main apply then creates the KRaft Kafka CR, nothing to migrate); already `strimzi.io/kraft=enabled` or `status.kafkaMetadataState: KRaft` → done; any other read error → fail closed. Otherwise (a ZooKeeper cluster) it migrates. 3. Annotates `strimzi.io/node-pools=enabled` then `strimzi.io/kraft=migration`, polls `status.kafkaMetadataState` through `ZooKeeper → KRaftMigration → KRaftDualWriting → KRaftPostMigration → PreKRaft → KRaft`, and only once a safe state is reached flips `strimzi.io/kraft=enabled`. If it times out in an intermediate state it aborts without finalising, so Helm fails the upgrade and the next reconcile retries from where it left off. Data is migrated in place: brokers keep their node IDs and reuse their existing PVCs, nothing is copied. Migration is one-way — Strimzi has no supported KRaft→ZooKeeper rollback once complete, so lowering the `version` field or rolling the app back to a pre-KRaft chart is unsupported (take a backup first). ### Why the pools are hook-managed The hook must create the pools and walk the migration state machine *before* the main release apply. A resource a hook creates cannot also be a regular chart manifest across the ZK→KRaft boundary: Helm's main apply finds the pool live in the cluster but absent from the previous (ZooKeeper) release revision and aborts with `original object KafkaNodePool ... not found`, leaving the release `failed` even though the cluster migrated fine (reproduced under server-side apply, the helm-controller default). Managing the pools only from the hook avoids that, so `templates/kafkanodepools.yaml` and the deployed-version gate ConfigMap (`templates/version.yaml`) are removed and the pools are applied idempotently by the Job on every install and upgrade. ## Bug fixes included - ZooKeeper `metricsConfig` referenced wrong ConfigMap key (`kafka-metrics-config.yml` instead of `zookeeper-metrics-config.yml`) - ZooKeeper `storageClass` used `kafka.storageClass` instead of `zookeeper.storageClass` - WorkloadMonitor used `.Values.replicas` instead of `.Values.kafka.replicas` - `validateNameLength` now caps a Kafka app name at the KRaft controller pod-hostname budget (`<release>-c-<hash>-<id>` must fit 63 chars), instead of admitting names that later fail the chart render forever ## What's next A follow-up PR will upgrade Strimzi to 0.51.0 and Kafka to 4.2 once all instances are confirmed migrated to KRaft. That PR is already prepared on branch `KRaft-upgrade-strimzi-051`. ## Test plan - [x] `helm template` renders correctly for new KRaft instances (no ZooKeeper refs) - [x] `helm template` with custom values (1 replica, external, topics, v3.8) - [x] Version validation rejects unsupported versions via JSON schema - [x] helm-unittest: 86/86 across 9 suites - [x] `hack/kafka-migration-hook.bats`: 12/12 — the real hook script executed against a stub kubectl (resolution branches, fail-closed reads, atomic collision guard, finalize guard) - [x] Operator template renders with the unchanged Strimzi 0.45.1-rc1 - [x] Live migration on a disposable cluster under server-side apply: fresh install → healthy KRaft, release `deployed`; in-place migration of a 20000-message ZooKeeper cluster → release `deployed`, broker PVCs adopted in place (same UIDs), and the KRaft data byte-for-byte identical to the pre-migration ZooKeeper data (matching sha256 over every `partition:offset:key:value`) ### Release note ```release-note [kafka] Enable KRaft metadata mode with automatic, in-place ZooKeeper-to-KRaft migration. Kafka version is now configurable (v3.9, v3.8). The Strimzi operator is unchanged (0.45.1-rc1). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added chart-level Kafka version selector (default v3.9) and KRaft-aware node pool rendering with controller storage sizing. * **Updates** * Default Kafka appVersion updated to 3.9.0; operator and image tag defaults aligned to 0.45.0. * Templates now omit ZooKeeper resources and metrics when KRaft is in use. * **Tests** * New Helm unit tests for Kafka CR, node pools, metrics, topics, dashboard mappings, and workload monitors. * **Documentation** * Values, schema, and README updated to document new version and controller storage options. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
f4e923aa82 |
docs(kubernetes): restate the apiServer extraArgs contract for OIDC
The extraArgs field documentation told operators to add --requestheader-uid-headers themselves, which the chart now renders whenever OIDC is on. Describe what it renders per version, when it defers to an operator entry, when it merges into one, and that it still refuses to render shapes the apiserver would not start on. The version field says it too: choosing v1.32 is what makes the chart render the feature gate next to extraArgs, and a reader of that field alone would otherwise not learn that the two interact. The package's hack/update-versions.sh writes that description itself on every make update, so it carries the same text, or the next version bump would drop the sentence again. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
6d5666f7e6 |
fix(rabbitmq): describe the preset as filling what resources leaves unset
cozy-lib merges the preset under resources key by key, so a partially set resources map still takes the preset for everything it omits: with resources.cpu=700m alone the broker renders 700m of CPU against the preset's 512Mi of memory, and this upgrade moves that broker from 128Mi to 512Mi as well. The description said the preset applies when resources is omitted, and cozyvalues-gen copies it verbatim into values.schema.json, the README parameter table and the cozyrd openAPISchema the dashboard renders, so one sentence made the claim on three surfaces. The suite covered a fully omitted resources map and one that sets both cpu and memory, which is the one shape that does not exercise the merge. The new case pins it: reverting defaultingSanitize to replace the preset instead of merging over it turns that case red and leaves the other three green. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
|
cf749988a4 |
fix(rabbitmq): right-size the default resources preset to s1.nano
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
|
ff637ed9a2 |
fix(platform): validate the realm-name charset and regenerate kubernetes artifacts
The realm-name guard checked only the type and echoed the parsed value back, so its hint told an operator who wrote `on` to set realmName: "true" — the token YAML had already discarded. Drop the echo and add a slug-charset check: the name is spliced verbatim into URL path segments and a shell token, so a "/" or a space must be rejected at the source rather than split a URL or the hook script downstream. The description edit in the previous commit was hand-applied to the generated kubernetes README and never reached the Go types, leaving pre-commit red; regenerate both with cozyvalues-gen. Finish the oidc-tenant System-mode prose that still read cozy as a fixed name. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
8ef230f0bf |
Merge remote-tracking branch 'origin/main' into kafka-multi-instance
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> # Conflicts: # docs/storage-immutability.md # packages/apps/kafka/tests/topics_test.yaml # packages/system/kafka-rd/cozyrds/kafka.yaml |
||
|
|
8ec76a3614 |
fix(kafka): make topics[].config optional
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
|
c398e0e116 |
Merge branch 'main' into chore/drop-http-cache
main took migration 57 and moved migrations.targetVersion to 58 while this branch was open, so that line is the one conflict: 57 at the merge base, 58 on main, 59 here. Resolved as 59, which is what this branch's migration 58 requires, since run-migrations.sh loops up to targetVersion - 1. Merged rather than rebased because the branch carries commits that are not mine to rewrite. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
|
710cdd18bb |
fix(kafka): address KRaft-migration review — broker pool adoption, hook safety, tests
Addresses the branch review on the ZK->KRaft migration path. CRITICAL — broker node pool adoption. Strimzi derives broker/PVC names as <cluster>-<pool>-<id>; a pre-node-pool ZooKeeper cluster has brokers <cluster>-kafka-N, so the adopting broker pool must be named exactly "kafka" or the operator provisions fresh empty brokers and orphans the tenant's data. A new kafka.brokerPoolName helper resolves the broker pool to "kafka" for an existing (migrating) cluster and keeps the release-scoped "<release>-broker" for fresh installs, so multiple Kafka instances still coexist in one namespace. The migration hook creates the pool as "kafka" and fails closed if a "kafka" pool already belongs to another cluster (node pool names are namespace-unique, so in-place migration of co-namespaced ZK clusters is unsupported). Wired through kafkanodepools.yaml, workloadmonitor.yaml and the hook. MAJOR — migration robustness. - Raise the HelmRelease timeout: the migration Job can run tens of minutes but the release timeout defaults to 10m, so helm-controller aborted and thrashed the pre-upgrade hook. Add release.cozystack.io/helm-install-timeout=45m on the kafka ApplicationDefinition, above the Job's activeDeadlineSeconds (2400s). - Idempotent migration: do not re-issue kraft=migration once the cluster is past the ZooKeeper phase (an invalid backward transition Strimzi rejects on retry). - Fail closed on the decision gates: distinguish a genuine NotFound (skip) from any other kubectl error (abort), and never treat an empty kafkaMetadataState on an existing CR as "migrated". MINOR — version downgrade documented as unsupported; integer (not lexical) compare in the version gate; deleteClaim/PVC-retention and the one-per-namespace migration limitation documented in the README; docs/storage-immutability.md updated for KRaft. Tests — helm-unittest coverage for the broker pool name, the hook guards and the idempotent/fail-closed logic; a bats check for the helm-install-timeout budget ordering; and a Chainsaw ZK->KRaft migration suite that seeds a ZooKeeper cluster, drives the migration, and asserts the original broker PVC survives (not recreated) and the cluster reaches KRaft. The migration sequence and the Chainsaw assertions were validated against a live cluster. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
42db2ca141 |
feat(kafka): migrate from ZooKeeper to KRaft mode with automatic migration
Replace ZooKeeper-based Kafka with KRaft (Kafka Raft) metadata mode. Strimzi 0.45 is the last line that still supports ZooKeeper, so this prepares the transition before ZooKeeper support is removed upstream. Chart changes: - The Kafka CR always renders strimzi.io/kraft=enabled with node pools and no spec.zookeeper block. Storage, replicas and resources move to separate broker + controller KafkaNodePools (a homogeneous separate-pool layout works for both fresh installs and migrations; Strimzi 0.45 does not support migration into a combined pool). - Kafka version is now user-configurable via the standard files/versions.yaml pattern (v3.9 default, v3.8 available); appVersion bumped to 3.9.0. - WorkloadMonitors track the broker and controller pools (no zookeeper monitor); the dashboard RBAC grants the controller monitor and drops the zookeeper one. Migration of existing ZooKeeper clusters is handled by a Helm pre-upgrade Job (templates/migration-hook.yaml) gated by a <release>-kafka-deployed-version ConfigMap, following the established seaweedfs/etcd hook pattern. The Job creates the broker + controller pools with Helm ownership labels, drives the Strimzi ZK->KRaft state machine (node-pools=enabled, kraft=migration -> kraft=enabled once KRaftPostMigration is reached) and is skipped once the ConfigMap records version >= "1". Testing: - Helm unit tests cover the Kafka CR, node pools, migration hook, metrics, topics, version map, dashboard RBAC and workload monitors. - The e2e suite is ported to the current Chainsaw framework (hack/e2e-chainsaw/kafka): it asserts the KRaft topology (both node pools, kafkaMetadataState=KRaft), the absence of ZooKeeper, and bootstrap endpoints. Rebased onto current main: TLS listener support, entity-operator resource guards, x-cozystack-options schema annotations and the WorkloadMonitor replica-path fix are all preserved and reconciled with KRaft. Signed-off-by: Matthieu <matthieu@hidora.com> Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
f9ea0da3de |
feat(backups): Kafka topic-metadata backup via a dedicated strategy driver (#4000)
## What this PR does Adds a dedicated `strategy.backups.cozystack.io/Kafka` Go driver that backs up the **topic metadata** of a Cozystack-managed `Kafka` application and wires it into the platform `cozy-default` flow — the counterpart to the app-specific drivers (CNPG, MariaDB, Altinity, MongoDB). It is a typed variant of the generic Job strategy: the controller renders a one-shot batch Job that talks to the Kafka Admin API, and adds on top an applicationRef Kind gate, a `Ready` precondition on the Strimzi `Kafka` cluster, and a per-run object key via `.BackupName` so distinct runs never overwrite each other. What it captures: every non-internal topic's partition count, replication factor and non-default configs (via `kafka-topics --describe`), stored as one small object per run in the system bucket; restore recreates them with `kafka-topics --create` + `kafka-configs --alter`. Deliberately **out of scope**: message payloads (a volume-snapshot strategy's job), consumer-group offsets (a log position, only meaningful with a byte-identical data restore), ACLs, quotas, and `KafkaUser`s. No PITR. Restored topics are created directly through the Admin API, so on the target they are unmanaged (no `KafkaTopic` CR) — the accepted tradeoff of an Admin-API-only driver. Both restore flows are implemented: **in-place** recreates dropped topics into the source, and **to-copy** applies the source's topics onto a freshly-bootstrapped empty `Kafka` (the "bootstrap a cluster, then apply the metadata state" flow). Contents: - Driver: `api/backups/strategy/v1alpha1/kafka_types.go`, `internal/backupcontroller/kafkastrategy_controller.go` (+ unit tests for its pure helpers), `internal/backupcontroller/kafkatypes/` (Strimzi `Kafka` readiness gate), dispatch cases in both controllers + `supportedBackupStrategyKinds()`, scheme registration, generated CRD + deepcopy. - Chart: `strategy-kafka-default.yaml` (gated on the bucket name), a `cozy-default` binding, read RBAC on `kafka.strimzi.io/kafkas`, and the Strimzi Kafka client image value. - Docs: a Kafka section in `docs/operations/backup-classes.md`. - Example + e2e: `examples/backups/kafka-metadata/` and `hack/e2e-chainsaw/kafka-metadata/` (`kafka-3-metadata-roundtrip`), an honest round-trip that proves a per-run `retention.ms` sentinel survives both restores. The original `packages/apps/kafka` chart is left untouched (it has no legacy backup mechanism). Validated: `go build ./...`, `go vet`, `go test ./internal/backupcontroller/...` (pure helpers plus a reconcile Kind-gate test and the RestoreJob cleanup regression test), `helm unittest` on the package (which now renders `strategy-kafka-default.yaml` and asserts its kind/name/`artifactURITemplate`), the `select-install`/`select-e2e` bats suites, `helm template` of the strategy CR, `bash -n` of the rendered script, and `make generate` (clean). The end-to-end round-trip (`kafka-3-metadata-roundtrip`) is enabled and mapped like the sibling backup suites, so CI builds this branch's controller and runs it live against a fresh sandbox; it has not been merged-and-run before, so that first live run must go green before merge. ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. This PR adds a new `strategy.backups.cozystack.io/Kafka` CRD, a Go driver, chart wiring, docs and examples. It does not add or rename a `packages/apps`/`packages/extra` package, does not change any `ApplicationDefinition`, `values.schema.json`, version enum, `release.prefix`, output Secret/Service, the `Package`/`Plan`/`RestoreJob` CRDs the provider types by hand, any `hack/*.mk` anchor or make-target behaviour, node prerequisites, labels/annotations, or telemetry metrics. A brand-new `strategy.backups.cozystack.io` CRD travels to the Terraform provider as opaque JSON (it types only `Plan`/`RestoreJob`/`Package`), so it keeps working untouched. None of the downstream triggers match. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(backups): add a Kafka topic-metadata backup strategy bound by cozy-default — backs up topic definitions and configs (partitions, replication factor, non-default configs) via the Kafka Admin API, with in-place and to-copy restore; message data, consumer offsets, ACLs, quotas and KafkaUsers are out of scope ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added Kafka topic-metadata backup and restore support, excluding message data. - Supports ad-hoc and scheduled backups, in-place restoration, and restoration to a separate Kafka cluster. - Recreates missing topics and restores compatible configurations while validating partition and replication-factor constraints. - Waits for Kafka readiness and cleans up backup artifacts after completion. - Backup and restore jobs now enforce configurable deadlines to prevent stalled operations. - **Documentation** - Added guidance covering Kafka metadata backups, restoration limitations, and S3 configuration. - **Tests** - Added end-to-end coverage for backup, restoration, validation, and cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
01967b56d3 |
docs(backups): correct the Kafka .Mode doc and the embedded-script templating note
The KafkaSpec.Template doc listed .Mode as "backup" or "restore", but the driver also renders it as "cleanup" when reaping a removed Backup's object; the comment is copied verbatim into the CRD, so kubectl explain and the dashboard showed two values while a custom strategy's restore branch would run on the cleanup pass. List all three, matching the Redis sibling, and regenerate the CRD. The strategy template also claimed the embedded script "reads only env vars, never Helm/controller values, so no tpl is applied". The controller renders every string leaf of the PodTemplateSpec through internal/template, args[0] included, so the script is templated on every run. State that instead, without literal template braces that Helm would itself expand. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
|
d7c0332d5e |
chore(dashboard,api): drop the HTTPCache catalog entry and values type
The dashboard no longer registers a brand icon for the kind, and the ServicesTab fixture moves to TCPBalancer, which the catalog still ships. An instance that survives the removal still renders, on the generic sidebar icon. api/apps/v1alpha1/httpcache goes too. types.go is generated by values-gen out of packages/apps/http-cache, which is gone, so the file could never be regenerated, and no Go file in this repository imports it. Surviving instances do not depend on it: the aggregated API serves the schema recorded in the in-cluster ApplicationDefinition. The terraform provider's cozystack_httpcache resource is hand-written and does not import it either; only its per-kind schema-guard test does, and that goes away with the resource. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
b3b280ea73 |
feat(gateway)!: add TLS-passthrough listeners to TenantGateway (#3342)
## What this PR does Phase 1 of external database exposure via Gateway API TLS-passthrough (#2815): adds a `tlsPassthroughListeners` field to `TenantGateway` and renders one layer-4 `mode: Passthrough` listener per entry, on a native database port with a per-engine subdomain hostname, alongside the Gateway's terminate listeners. No engine is wired: no TLSRoute, no certificate or CA changes. The existing layer-7 `tlsPassthroughServices` field gains a 62-entry count bound, an item length and a DNS-1123 item pattern, and a repeated entry now fails the reconcile with `Ready=False` instead of rendering the same listener twice. `spec.apex` gains a 253-character bound and nothing else: the CEL rules concatenate it, so the install-time cost estimator refuses the CRD without a declared bound. It gains no format pattern, and that is not a claim that a mixed-case host works. Such a host reaches the field verbatim from a namespace label, the renderer then composes listener hostnames Gateway API refuses, and that tenant has no Gateway either way; what a pattern would change is only that the write is refused, which fails this HelmRelease where the same fault costs the one Gateway. An apex no listener hostname can sit within is reported by the controller instead, and only when `tlsPassthroughListeners` is in use. The constraints and what they do to objects that already exist are described further down. The bound on an entry is what the schema can state on its own. An entry also becomes a listener name and a hostname built from the apex, so the controller checks both composed values against the 253 characters Gateway API allows and names whichever one overflows. Validation rejects duplicate listener names or ports, ports reserved by the platform Gateway (80 and 443), out-of-range ports, invalid DNS-1123 names, and hostnames outside the tenant apex (mirroring the Gateway hostname admission policy so an invalid entry cannot make the whole Gateway rejected). The SNI-overlap rule refuses two passthrough listeners that answer the same name on different ports. That is what [cilium#42898](https://github.com/cilium/cilium/issues/42898) reports: only one of the two routes ever works. It does not extend to a terminate listener and a passthrough listener sharing a name across ports. No failure is reported for that shape, and Cilium counts a terminate/passthrough pair as conflicting only when they share a port. The per-engine subdomain hostname scheme follows the phase-1 gate. The flat-hostname alternative (pending the Cilium listener-isolation fix targeting 1.20) is a later scheduling decision that does not change this API field. One shape differs from #2815 on purpose. The ticket has `hostname` optional with a `*.<name>.<apex>` default; here it is required. Deriving the SNI from the listener name hides the one value an operator has to get right, and the two drift apart the moment the naming scheme changes. Found while checking that pairing, unrelated to this field: a stock `dns01` gateway already renders `tls-api` (passthrough, 443) beside `https` (terminate, 443, `*.<apex>`), one port with overlapping SNI, which is the shape Cilium 1.19.6 marks `Conflicted`. That belongs to the Cilium bump and is recorded on #3343. Also named rather than fixed: the controller-side rules on this field stop the whole reconcile when they refuse a spec. `renderGateway` returns an error, so the Gateway is not updated at all, and the tenant sees `Ready=False` with `ReconcileError` while new app listeners and every certificate the controller would issue or amend wait behind it. Certificates already issued go on renewing, since cert-manager drives that from the objects already in the cluster and an aborted reconcile writes nothing to them. Dropping only the conflicting listener and reporting it on status would be the better shape, but that changes the error model of the whole renderer rather than one rule, so it is out of scope here. The current behaviour is at least observable: `Ready=False` is a signal, not silent drift. On HTTP-01, a hostname a passthrough listener answers earns no HTTPS-terminate listener once a TLSRoute claiming that hostname can be served on the listener, matched by SNI overlap so a wildcard entry reaches a name beneath it as readily as the name written in the spec, and the field itself is accepted under certMode http01 alone. That refusal is stated as the set of modes that can carry a passthrough listener rather than the set that cannot, because a mode added to the enum would otherwise be admitted by default: dns01 and existingSecret serve the whole apex from one wildcard terminate listener the pinned Cilium cannot keep apart from a passthrough listener beneath it, and edge ends TLS at the class provider and renders no TLS listener for one to sit beside. The route half of that condition is load-bearing rather than incidental. On the pinned v1.19.5, tlsPassthroughFilterChains walks a listener's routes and skips one with no backends, so a declared passthrough listener nothing attaches to contributes no filter chain and matches no ClientHello, while the terminate listener's chain is built from its own Secret and carries the name alone. Keying the withdrawal on the declaration would therefore take a name that is being served and hand it to a listener that forwards nowhere, and that is the ordinary shape rather than a corner: the chart puts api, vm-exportproxy and cdi-uploadproxy on every tenant Gateway, while there is one platform TLSRoute per name cluster-wide and each names one tenant's Gateway as its parent, so every other tenant renders all three entries with a route on none. The backend half is that same rule reached by a different road: the chain is built per route out of the backends that survived resolution, so a route that attached and forwards nowhere puts no chain on the name either, a backendRef naming a Service that does not exist yet because manifests applied out of order, one deleted since, a name with a typo, or one in another namespace with no ReferenceGrant, and without that half an ordinary backend mistake takes a live endpoint offline while Cilium builds nothing to replace it. Resolution asks what v1.19.5 asks and no more: the reference allowed, the Service present, and neither its ports nor its endpoints read, because the translator reads neither. Services and ReferenceGrants are watched alongside routes, so both halves are deferrals rather than exemptions and the withdrawal lands on the pass that sees the missing piece arrive. The withdrawal reaches HTTP-01 alone, because `collectHostnameClaims` returns nothing under the other two modes and they render no per-hostname listener to withdraw. A TLSRoute that declares no `spec.hostnames` claims the hostname of each rendered passthrough listener its parentRef selects, because that is the name Cilium serves it on: `ComputeHosts` at v1.19.5 substitutes the listener hostname for a route that names none, so such a route takes the name over exactly as one that spells it out does. The tenant-namespace hostname policy requires a route there to name its hostnames, so this shape reaches a Gateway from the system namespaces where the platform routes live, or from an administrator, not from a tenant chart. A claim broader than the listener's counts the same way from the other side: a TLSRoute claiming `*.<apex>` is served on `api.<apex>` rather than on the wildcard, because `ComputeHosts` returns the listener's own hostname for a route hostname covering it, so a claim covering a reserved name is judged as a claim on that name while the route keeps one entry under the key its owner wrote. Both restrictions are tied to the Cilium pin, currently v1.19.5. That version translates the Gateway into a single Envoy listener whose filter chains match on SNI without the port, so a terminate chain and a passthrough chain carrying one name collide regardless of which Gateway port declared them; upstream states the consequence above NeedsCrossProtocolSplit in operator/pkg/model/model.go as of v1.19.6, the release that starts splitting Envoy listeners per port when NeedsPerPortListeners holds. That is a disjunction rather than a blanket change, and the passthrough side of every one of its three clauses is counted from attached routes, so a listener with no TLSRoute behind it contributes to none of them: the split arrives with the route, not with the bump. The same single-listener translation has a consequence that is not a restriction: the declared port is where the listener is published, not a bound on reach, because the backend answers its hostname's SNI on every port the Gateway exposes. Firewalling a tenant address down to 80 and 443 does not put a database out of reach; the hostname is the access surface, and the field's own doc and the chart README now say so. Both restrictions are validation-only, and they lift together on the pin plus a configuration the split actually covers rather than on the bump alone. A route whose hostname is withdrawn this way is told so under this controller's own name: Accepted=False with `NoMatchingListenerHostname`, and a message naming the passthrough hostname that answers it, which for a wildcard entry the route cannot derive on its own. It matters most there. A wildcard entry can withdraw the listener from several published names beneath it at once, each one a TLSRoute claims and can be served on, and the certificates are garbage-collected with them, and a rollback is cheaper than that sounds: cert-manager here runs with `enableCertificateOwnerRef: false`, so deleting a Certificate leaves its Secret behind and a re-created Certificate adopts a still-valid one rather than placing an order. Before this the only signal was Cilium's own RouteParentStatus entry under a different controllerName. A TLSRoute whose hostname no passthrough listener answers is told so as well, on the same condition and for the same reason: before this change that hostname got a terminate listener Cilium refused with ResolvedRefs=False, so withdrawing the listener would otherwise have removed the only object saying the route went nowhere. Two things stay named rather than fixed. A `tlsPassthroughListeners` hostname of `*.<apex>` is refused only indirectly: the overlap rule rejects it while `tlsPassthroughServices` carries its defaults, because the wildcard covers api.<apex>, so emptying that list is enough to let it through, and the terminate listener then goes from every published name a TLSRoute claims under the wildcard. The README states this; refusing the apex and its wildcard on their own terms is a one-line rule worth adding when the field is wired up to chart values. A wildcard hostname on an attached HTTPRoute used to render a listener name the shipped Gateway CRD refuses, `https-*-<hash>`, which failed the whole Gateway write while the README said the shape worked. Listener and Certificate names now spell a leading `*` as `wildcard`, with the hash still covering the whole hostname, and a test reads both the listener-name and the hostname patterns off the shipped CRD bundle and renders wildcards through them; every name that was already admissible is pinned byte for byte, so nothing is renamed on upgrade. Under http01 an HTTPRoute wildcard claim is refused before the race and the overlap are read, with `NoMatchingListenerHostname`, because the HTTP-01 challenge issues no wildcard certificate and a terminate listener could never have carried the name. A route status write that fails no longer leaves the route carrying its previous condition: `updateRouteStatuses` and the whole-apex pass collect every failed write and return them after the loop, so the reconcile fails, the TenantGateway reports `Ready=False` with `ReconcileError` and controller-runtime requeues, while the loop still runs to the end so one route's conflict does not hold the others back. Conditions this controller wrote under http01 are retracted when the tenant moves to dns01, existingSecret or edge, for HTTPRoutes and hostname-less TLSRoutes alike, so a refusal does not outlive the mode that produced it. And a TLSRoute claiming a wildcard passthrough entry itself leaves the names beneath the wildcard alone, since Envoy ranks an exact server name above the wildcard and nothing collides. One thing this does not cover yet: Accepted is computed from the hostname race and the withdrawals above, so a route is still told it is fine when an HTTPRoute holds the name and the TLSRoute claiming it cannot attach to the terminate listener that answers it. That one predates this change. Two neighbours of it no longer fall in the gap. A `parentRef` port that names no listener answering the hostname is compared alongside the `sectionName`, since Gateway API requires every key a route pins itself with to match the listener it selects. An HTTPRoute's `parentRef.port` is judged the same way: the terminate listener sits on 443, so any other pinned port earns no listener and no certificate through that parentRef and reads the port-mismatch cause; port 80, or `sectionName: http`, from a namespace the plain-HTTP listener admits (the publishing tenant and cozy-cert-manager) is served plain with no terminate listener and no refusal, from any other namespace it is refused with `NotAllowedByListeners`, and a `sectionName` and a port that select different listeners select none and read `NoMatchingParent`. A plain-served route stays in the hostname race on the same footing as a terminating one, in every recount as well as the first count. And a `sectionName` naming a `tls-<name>` listener this Gateway does not render now reports `Accepted=False` with `NoMatchingParent`, Gateway API's reason for a parentRef matching no listener: the controller renders every listener in that namespace, so an absent name there is a fact rather than a guess, while a `sectionName` naming the plain-HTTP listener is judged like port 80 below, and any other name outside the prefix is written Accepted like an attached route, with the class controller's own status entry answering for the listener it named. The namespace case is off that list. Native-port listeners admit the publishing tenant alone, and a TLSRoute elsewhere on one of their hostnames is refused with `NotAllowedByListeners`. An HTTPRoute that pins itself by `sectionName` to a rendered `tls-<name>` passthrough listener is refused with the same reason: that listener forwards the stream it matched by SNI and terminates nothing, so it takes TLSRoute alone by protocol and cannot serve the route whatever answers its hostname. Its claim also leaves the hostname race before ownership is ranked, so the TLSRoute that can be served is not told it lost to a route that could never attach. Cilium does not report this refusal at the pin. The port-443 passthrough listeners name HTTPRoute in `allowedRoutes.kinds` to keep the port-443 set uniform (cilium#45559), the Gateway reports that entry back as `ResolvedRefs=False/InvalidRouteKinds` on the listener, and Cilium's route-side kind check reads a listener's kinds against every route on the Gateway rather than against the routes that named it, so it reports the route Accepted and the condition this controller writes is the only refusal the route gets. A hostname whose every claimant is refused this way earns no HTTPS-terminate listener and no certificate, where before it earned both. The refusal is decided per route rather than per hostname, which matters once a claim overlaps more than one reserved name: a wildcard covering both a port-443 service hostname and a native-port one is attachable through the first and refused through the second, so the answer comes from the listener the `parentRef` names, its own hostname and its own attach set. The row that pinned a `sectionName` naming no rendered listener as a known gap now pins the verdict instead. On the port-443 listeners the two namespace sets agree by construction, since `collectHostnameClaims` and the listeners' allowedRoutes selector run off the same list. And the justification for the pin is written out at every site that depends on it rather than once, so the day the pin moves they all have to move together and the one that does not is a false statement no diff can catch. That is not hypothetical here: three claims of exactly that shape were wrong in this branch and were caught by reading the source they described rather than by any diff, so the volume of restated reasoning is itself what produces them. One canonical block with references to it would carry the same knowledge with one place to update, and rewriting the narrative under review would be a documentation change rather than a fix, so it is tracked on #3842 instead. The gateway chainsaw suite now covers the withdrawal, which it did not before: a tenant owning its own Gateway renders the shipped passthrough entries with nothing routed to them, an HTTPRoute on api.<apex> keeps its terminate listener, its certificate and Accepted=True there, and attaching a TLSRoute to the same listener, pointed at a Service in the tenant's own namespace, since a route that forwards nowhere is the case the first phase already measures, withdraws all three and moves the route to NoMatchingListenerHostname. A third phase patches in a native-port entry and reads the per-listener readiness flag, which is the claim no test against a fake client can settle. The first two phases are one measurement: withdrawing on demand is what dates the earlier assertion to settled state rather than to a withdrawal that had not arrived yet. The `Require API owner review for sizeable API changes` check is red on purpose and does not clear with a code change. A new field plus tightened validation on already-shipped fields counts as a sizeable API change, so `.github/workflows/api-review-gate.yaml` holds the PR until an API owner approves it. The gate counts each owner's latest review on the current head commit, so the approval has to land on the head as pushed and any later push invalidates it; the job re-runs on review submission, so no push is needed to flip it. Reading that red as "not ready" is the mistake this paragraph exists to prevent. Part of #2815. ### Release note ```release-note feat(gateway): TenantGateway supports layer-4 TLS-passthrough listeners on native database ports for SNI-based external exposure. On HTTP-01 gateways a hostname a TLS-passthrough listener answers, including any name beneath a wildcard entry, no longer earns an HTTPS-terminate listener or a Gateway-issued certificate once a TLSRoute claiming that hostname can be served on the listener and forwards to a backend that resolves; a hostname claimed only by a TLSRoute stops earning one too. A TLSRoute with no spec.hostnames counts as claiming the hostname of each passthrough listener its parentRef selects, and one whose hostname covers a listener's counts as claiming that listener's name, both being the name Cilium serves it on. A declared passthrough listener with no route on it changes nothing, because it emits no filter chain and the terminate listener is what answers the name, and neither does a route whose backendRefs resolve to nothing, a Service that does not exist, or one in another namespace with no ReferenceGrant, since such a route emits no filter chain either. The controller watches Services and ReferenceGrants, so the withdrawal it defers lands once the backend appears. So the tenant whose Gateway the platform TLSRoutes name, tenant-root unless the cluster's expose-ingress says otherwise, sheds the terminate listeners and certificates for api, vm-exportproxy and cdi-uploadproxy on upgrade (those TLSRoutes ship with the cozystack-api, kubevirt and kubevirt-cdi packages, each rendered only where the Gateway integration is enabled and the service is among the platform's exposed services), and a tenant that renders the same three entries with no route on them keeps what it has. The shed certificates leave their TLS Secrets behind: cert-manager here ships enableCertificateOwnerRef: false, so each Secret stays in the namespace with no owner, nothing renewing it and nothing deleting it. That cuts both ways, as stale Secrets to remove by hand and as a cheap way back, since a re-created Certificate adopts a still-valid Secret instead of placing an order. A route the controller renders no listener it can attach to now carries Accepted=False from this controller instead of Accepted=True. Five shapes report NoMatchingListenerHostname and the message tells them apart: the HTTPRoute whose hostname a passthrough listener answers, the TLSRoute with no passthrough listener behind it, the route whose parentRef names a listener that answers a different hostname, the route whose parentRef names a port no listener answering its hostname is published on, and the HTTPRoute claiming a wildcard hostname under http01, which the HTTP-01 challenge cannot certify. A route pinned by sectionName to a tls-<name> listener the Gateway does not render carries NoMatchingParent instead, which is what Gateway API calls a parentRef matching no listener, and so does an HTTPRoute whose sectionName and parentRef port select different listeners. A TLSRoute outside the publishing tenant's namespace claiming a native-port hostname carries NotAllowedByListeners instead, since those listeners admit the tenant alone. So does an HTTPRoute pinned by sectionName to a rendered tls-<name> passthrough listener, which cannot serve its kind, and an HTTPRoute selecting the plain-HTTP listener from a namespace it does not admit; a refused route also stops counting in the hostname race, so a hostname whose every claimant is refused earns no terminate listener and no certificate. Listener and Certificate names derived from a wildcard hostname are admissible now, names that were already admissible do not change, a failed route-status write fails the reconcile instead of being logged away, and conditions written under http01 are retracted when certMode leaves it. The dns01 and existingSecret cert modes shed nothing, because neither renders per-hostname listeners at all, and tlsPassthroughListeners is accepted under http01 alone: those two serve the whole apex from one wildcard terminate listener, and edge terminates TLS upstream and renders no TLS listener a passthrough entry could sit beside. The withdrawal does not take a working path away: on the pinned Cilium both port-443 filter chains already carry these hostnames and nothing on the objects says which one answers a connection, so a race that was previously undetermined and unobservable now resolves the same way every time. Treat this as a change of serving path rather than the removal of something idle: today each of those three hostnames has a terminate listener and a passthrough listener on port 443, both filter chains carry the SNI, and nothing on the objects says which one answers a connection. Gateway API calls for both listeners to report Conflicted; the pinned v1.19.5 has no Conflicted condition at all, so there is nothing to read, and you cannot tell from the cluster whether api.<apex> is terminated by the Gateway or passed through. Afterwards it is passed through, always. Read the Gateway's listener conditions before upgrading to see what those listeners were serving. Before upgrading, confirm the management apiserver serving certificate carries api.<apex> in its SANs, because once the ACME certificate is withdrawn that is the only certificate left on the name; a cluster that came from the ingress-nginx path already meets that, a cluster served by the Gateway's ACME certificate since it moved does not necessarily and loses the name until the SAN is added. If something depends on the terminate path for one of those names, the package README lists three ways forward and what each costs. Moving the consumer to another hostname under the apex is the only one that gives up nothing. ``` ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against this diff: `api/gateway/v1alpha1/{tenantgateway_types.go,zz_generated.deepcopy.go}`, `internal/controller/tenantgateway/{reconciler,renderers,conflict,mapper,status,backends,reconciler_test,crdcel_test,mapper_test,backends_test}.go`, `internal/controller/tenantgateway/rbac_test.go`, `cmd/cozystack-controller/main.go`, `hack/e2e-chainsaw/gateway/chainsaw-test.yaml`, `packages/system/cozystack-controller/definitions/gateway.cozystack.io_tenantgateways.yaml`, `packages/system/cozystack-controller/templates/rbac.yaml`, `packages/core/platform/values.yaml`, and `packages/extra/gateway/README.md`. No trigger fires, and the two repositories worth ruling out explicitly are below rather than waved off. `cozystack/website`: no package under `packages/apps/` or `packages/extra/` is added, renamed or removed. `packages/extra/gateway/README.md` is edited in place. The docs generator regenerates a reference page from a package README only for the apps hardcoded in the website `Makefile`, and `gateway` is in none of `APPS`, `K8S`, `VMS`, `NETWORKING` or `SERVICES`, so there is no generated page for this package to drift out of step with. That gap is the same shape the trigger map already records for `packages/apps/opensearch` and `packages/apps/bucket`; it predates this change and is neither created nor widened here. None of the other website triggers apply: `packages/core/platform/values.yaml` changes only in a comment, which now says that api, vm-exportproxy and cdi-uploadproxy render a TLSRoute rather than an HTTPRoute, with no key added, moved or given a new default, no platform variant or component moves, no release asset is renamed, `ApplicationDefinition` semantics are unchanged, there is no Talos bump and no developer-tooling change. `cozystack/terraform-provider-cozystack`: this adds an API field, so the field-level triggers are the ones to check, and they are keyed to app charts rather than to this CRD. No app is added under `packages/apps/`; no `values.schema.json` gains, loses or renames a field; no version enum, `values.yaml` default, `kind`, `plural`, `release.prefix` or output Secret/Service name changes. `TenantGateway` is not among the CRDs the provider types by hand. Those are `Package` under `cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`, and the provider carries no reference to `gateway.cozystack.io` at all. It therefore falls under the map's last bullet: the spec travels as opaque JSON, so the provider keeps working and simply offers no typed access to `tlsPassthroughListeners`, which is the position it already holds for every other field it does not model, and not a break. The provider's new-top-level-field test guard is scoped to `api/apps/v1alpha1`, which this diff does not touch. `cozystack/ansible-cozystack`: nothing in reach. `packages/core/installer/values.yaml` and the `cozy-system`/PSA contract are untouched, no platform variant is added or renamed, none of the `packages/core/platform/values.yaml` keys the role sets are involved (the file changes in a comment alone), no object the role waits on is renamed, and `hack/e2e-prepare-cluster.bats` node prerequisites are unchanged. The schema constraints this diff adds to already-shipped fields all reach objects that already exist, and every one of them is a kind the apiserver ratchets. `spec.apex` gains a 253-character limit (no pattern: a mixed-case apex is admitted and refused by the controller only when passthrough listeners are declared), and `spec.tlsPassthroughServices` gains a DNS-1123 item pattern and a length bound. A stored value that violates one still passes any write that leaves it unchanged, so the rule bites only once the value itself changes. A repeated `tlsPassthroughServices` entry is reported by the controller as `Ready=False` rather than refused at admission, and the reason is not the one this section used to give. Ratcheting does reach a list-type error: `customresource.ValidateUpdate` validates an incoming object's list-type semantics only when the stored object passes them, so a stored duplicate is ratcheted through rather than blocking every write. What makes the marker the wrong tool is the grain of that ratchet, which is the whole object rather than the field, so one stored duplicate would switch list-type checking off for every list on the `TenantGateway` at once and say nothing about it. Documented in `packages/extra/gateway/README.md`, and pinned by `TestTightenedConstraintsOnExistingObjects` together with `TestListTypeSetWouldNotRefuseAStoredDuplicate`, which compiles a copy of the shipped schema carrying the marker to show what the apiserver actually does with one. Shipped defaults conform and the chart already fails on an empty host, so this is a no-op in practice. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added layer-4 TLS passthrough listeners (`tlsPassthroughListeners`) with native port + SNI hostname, rendered as passthrough listeners and limited to TLSRoute attachment. * **Bug Fixes** * Added stricter admission-time and controller validation (reserved ports 80/443, distinct listener ports, tenant-apex hostname rules, and collision prevention with passthrough services). * Improved listener rendering safety: prevents exceeding the Gateway listener cap and ensures AllowedRoutes are isolated per listener. * **Documentation** * Updated gateway docs with listener-cap considerations and layer-4 passthrough usage details. * **Tests** * Added CRD/CEL and reconciliation coverage for valid/invalid configurations and rendering behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
cecb4b11d3 |
feat(opensearch)!: issue HTTP and Dashboards certificates from cert-manager (#2682)
## What this PR does The OpenSearch HTTP API (9200) and Dashboards (5601) already served TLS, from the operator's own private CA and with a SAN built from `cluster.local` while the credentials Secret advertised the release on the platform domain, so nothing a client was handed could verify. Both certificates now come from a per-release cert-manager CA in the tenant namespace, and the CA reaches tenants as a key-free trust anchor. - `tls.issuer` selects who issues the HTTP server certificate, `cert-manager` or `operator`, and is named for the issuer because TLS is served under both. Unset resolves from `external` (cert-manager for externally published services, the operator for cluster-internal); an explicit value always wins. - Renders a self-contained cert-manager chain in the tenant namespace: a self-signed Issuer, a CA Certificate, a CA Issuer, and two leaf Certificates, one for the HTTP API and one for Dashboards, each with the SANs of its own Service and the external hostname when `external: true`. Leaf keys are written as PKCS8, since the JVM loader rejects the PKCS1/SEC1 encoding cert-manager emits by default, and the CA subject is pinned so the operator's signature check accepts the chain. - The CA Secret is `<release>.http-ca`. The dot separates the chart-chosen suffix from release names, so no other release can produce that name; the two leaf Secrets stay hyphenated because none of the chart's suffixes is a tail of another. The operator's own transport CA remains at `<release>-ca` and is not touched. - Transport mTLS (9300) stays operator-managed (`transport.generate: true`, `perNode: true`). The security plugin requires it and the chart cannot turn it off. - `hack/e2e-chainsaw/opensearch/` runs the move on a cluster: it installs the app on the operator's HTTP listener, removes `tls.issuer` so it resolves from `external`, and asserts the cert-manager chain, the CR dropping `http.generate`, the admin certificate reissued under the chart CA, the StatefulSet rolling onto the three subPath mounts, the tenant anchor, and the certificate the endpoint actually presents to a client holding that anchor. ### Sysctl carrier and single-node rolls The E2E suite for this PR was red because the chart set `spec.general.setVMMaxMapCount: true`, which makes the operator prepend a privileged `init-sysctl` container to every OpenSearch pod. Tenant namespaces run at the cluster's default PodSecurity level, baseline on Talos, so admission refused the container and the StatefulSet never produced a pod (#4131). The carrier for the sysctl is now on main: #4152 added a DaemonSet to the `opensearch-operator` package that raises `vm.max_map_count` to a floor of 262144 on every node, from a namespace the platform labels privileged. This branch is rebased onto that main and sets the flag to an explicit `false`, with a template comment saying why and a unit test pinning it, so a release no longer asks the operator for a container admission refuses. The value stays with the DaemonSet until the node contract carries it (#4144); the DaemonSet's own template comment, which said the chart still asks, now says it does not. Three more things came out of the same pass. The chart asked the operator to drain data nodes before every rolling restart, and with a single data node the drain never finishes: at v2.8.0 `PreparePodForDelete` excludes the node from allocation and then waits for it to hold no shards, which the only data node can never satisfy, so a one-replica release could not roll for a version bump or for the certificate move this PR relies on. The chart now drains only when a second data node exists, pinned both ways in the unit tests. The chart maps an empty `storageClass` to `replicated` at one replica, and the PR lane runs LINSTOR without DRBD and creates only `local`, so the E2E fixture takes its class from the `storageClass` value the harness passes with `--set-string` from `COZY_E2E_STORAGE_CLASS`, the way the vminstance fixtures do; on the nightly QEMU lane that still resolves to `replicated`. And the external Dashboards Service selected labels no pod carries. `pkg/builders/dashboards.go` at v2.8.0 stamps `opensearch.cluster.dashboards: <cluster name>` on the Deployment, its selector and the pod template, and nothing else; the chart asked for `opster.io/opensearch-cluster` together with `app.kubernetes.io/component: dashboards`, so the LoadBalancer came up with no endpoints and no error to show for it. It selects the label the operator sets now. ### securityadmin cross-trust, still blocked on the DNS base Chart-managed HTTP TLS previously broke the operator's securityadmin reconciliation: the admin certificate was signed by the transport CA while the HTTP listener trusted the cert-manager CA, so the two never verified against each other and users and roles silently never applied. This chart puts one CA under both sides, handing the HTTP CA to the operator through `spec.security.tls.http.caSecret` so that it signs the admin certificate with the CA the listener trusts. That closes the CA mismatch. The other half, the job resolving a name the operator built from `cluster.local` on a cluster that does not use it, was closed separately and is on main, so the security configuration applies once this lands. No operator bump is required; the API exists in the pinned version. That path is version-gated upstream at OpenSearch 2.0.0: below it the operator falls back to the transport CA and the mismatch returns with no error anywhere. The chart therefore refuses chart-managed HTTP TLS on `version: v1` with an explicit render-time failure that names the mechanism and offers the alternatives, rather than letting it fail silently at runtime. ### Trust anchor for tenants The HTTP server's private key is not exposed to tenants. The chart renders a `TenantProjection` sentinel that names `<release>.http-ca` as its source; the CA-extraction controller publishes a `ca.crt`-only copy as `<release>.tenant-ca`, and the ResourceDefinition selects it with the engine-agnostic `internal.cozystack.io/tenant-ca` label. The sentinel is gated on the same condition as the chain, so a plaintext release renders no sentinel instead of one stuck at `SourceNotFound`. Tenants get what they need to verify the server and nothing more. ## Known limitations These are stated rather than left to be discovered: 1. **securityadmin needed both trust and reachability, and both are closed now.** This branch put one CA under the admin certificate and the HTTP listener. The other half was the operator resolving `<svc>.<ns>.svc.<DNS_BASE>` with `DNS_BASE` left at the vendored `cluster.local`, so the job waited on a name that never resolved whichever CA had signed; the platform now renders that value from `networking.clusterDomain` and it is on main, which is what unblocks this branch. 2. **The e2e suite reaches the move, not the chart bump that triggers it.** The suite above arrives at the chart-managed chain by removing `tls.issuer` from a running release, which leaves exactly the values an upgraded release arrives with. The one step it cannot run is the one where the previous chart version wrote the CR, because there is no second chart version to install from in this tree. Everything downstream of that write is exercised on a cluster. 3. **A reissue does not reach a running pod.** Neither service reloads its certificate, and cert-manager reissues on any change to `external`, the tenant host or the cluster domain, so an ordinary edit leaves the pods serving material that no longer matches their own names with no error, event or condition to show for it. Completing such a change is manual. So is removing the CA private key cert-manager leaves behind when TLS is turned off again, since the platform runs with `enableCertificateOwnerRef: false` and deleting a `Certificate` does not delete its `Secret`. `packages/apps/opensearch/README.md` carries both, with the commands. ### Downstream repositories `packages/apps/opensearch/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_opensearch`; tracked in cozystack/terraform-provider-cozystack#41. ### Upgrade impact An existing `external: true` release with `tls.issuer` unset serves operator-issued HTTP TLS today and moves to chart-managed TLS on its first reconcile after this lands: the cluster rolls once and is re-anchored on a new per-release CA, so any client pinned to the operator's CA has to be repointed at the key-free `<release>.tenant-ca`. Setting `tls.issuer: operator` before the upgrade keeps that half as it is. Releases that are not external are untouched. Every existing release rolls once regardless of `tls.issuer` and `external`, because the pod template loses the operator's `init-sysctl` container; on a single-replica release that roll is the first one the operator can complete, since draining the only data node used to block every rolling restart. The published external-dns name changes on the same upgrade whatever `tls.issuer` says, because the Services are gated on `external` alone: they move from `<release>.<namespace>.<cluster-domain>`, an in-cluster name that never resolved outside the cluster, to `<release>.<tenant-host>` and `<release>-dashboards.<tenant-host>`. external-dns is configured upsert-only here, so the old record is not withdrawn and keeps pointing at the LoadBalancer until it is removed from the zone by hand. That cleanup is per zone rather than per release and nothing enumerates the records, so the README shows how to derive the list from the cluster. ### Release note ```release-note feat(opensearch)!: the OpenSearch HTTP API and Dashboards already served TLS, from the operator's own private CA and with a SAN built from cluster.local while the credentials Secret advertised the release on the platform domain; both certificates now come from a per-release cert-manager CA whose anchor the platform publishes into the tenant as <release>.tenant-ca. Any client that worked against the old endpoint was skipping verification, because no hostname it was handed could have matched that SAN. Existing external releases move from the operator's CA to the chart's on their first reconcile, and their published external-dns name moves from the in-cluster domain to <release>.<tenant-host> with the old record left in place; set tls.issuer: operator beforehand to opt out of the CA move. With dashboards.enabled the application name is limited to 41 characters, and to 32 when external is on as well; past either bound the chart fails the render and withholds every object in the release. A release already above a bound is not broken by the guard, it is told why it has not been upgrading: the Dashboards Service the operator builds from that name exceeds the 63-character limit for a DNS label, the operator returns that failure on every pass, and the reconcilers queued behind Dashboards, which are upgrade and restart and snapshot repository, have not run since the release was created. Set dashboards.enabled to false to drop the bound. The external Dashboards Service also selected labels the operator does not put on Dashboards pods, so it stood up with no endpoints; it selects opensearch.cluster.dashboards now. The drain of data nodes is now requested only with the data role on and more than two replicas, since at two the operator gives up on a full drain itself. The chart stops asking the operator for its privileged init-sysctl container, since the opensearch-operator package now sets vm.max_map_count from a DaemonSet, so every existing release rolls once on upgrade; single-replica releases no longer ask the operator to drain their only data node, which used to block every rolling restart ``` |
||
|
|
85fa60a33f |
feat(opensearch)!: issue the HTTP API certificate from cert-manager
The HTTP listener and Dashboards ran on certificates the operator minted itself, so a client outside the cluster had no anchor it could obtain through the platform and no way to verify the endpoint it was handed. Issue the HTTP chain from cert-manager instead, and give the operator the same CA through spec.security.tls.http.caSecret so it signs the securityadmin admin certificate with it. Without that the admin certificate would keep the operator's own CA while the listener trusted cert-manager's, and securityadmin could no longer authenticate to apply users, roles and audit policy. tls.issuer names the authority rather than a switch, because the listener serves TLS under either value and only the signer changes. It is tri-state: an explicit value is obeyed, an absent one follows external, because a release nobody publishes has no third party to prove anything to. The cert-manager issuer needs OpenSearch 2.0.0 or newer, since the operator honours the HTTP CA only from that version and silently signs with its transport CA below it. An explicit choice under that floor fails the render and one resolved from external falls back to the operator, so asking for a verifiable certificate never quietly yields less than was asked, and turning on external access never breaks an upgrade. BREAKING CHANGE: an externally published release on OpenSearch 2.0.0 or newer serves the HTTP API from a cert-manager CA rather than the operator's own. A client pinned to the operator CA must move to the trust anchor the platform projects into the tenant. BREAKING CHANGE: the external-dns name of an externally published release moves from <release>.<namespace>.<cluster-domain> to <release>.<tenant-host>, and the Dashboards name with it. This half is gated on external alone and does not depend on the issuer. external-dns runs upsert-only here, so the old record stays in the zone until it is removed by hand. BREAKING CHANGE: with dashboards enabled the application name is limited to 41 characters, and to 32 when external access is also on. A release above either limit now fails to render rather than converging. Such a release was already stalled: the Dashboards Service built from its name exceeds the 63-character DNS label limit, and the operator returns that failure ahead of its upgrade, restart and snapshot-repository reconcilers, so it has not upgraded or rolled since it was created. Set dashboards.enabled to false to converge again, or recreate the application under a shorter name. Assisted-by: LLM Signed-off-by: Arsolitt <arsolitt@gmail.com> [lexfrei: reworded subject and body for the issuer field rename] Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
ed3cb34cbf |
fix(backups): adopt-guard Kafka Backup artifact and correct URI-template doc
- createKafkaBackupArtifact now rejects adopting a same-named Backup that describes a different application, so a retained Backup reused for a different app of the same name is not reported as this run's success. Mirrors the RabbitMQ driver's guard; pinned by a Go test. - ArtifactURITemplate godoc said empty leaves status.artifact unset, but the driver hard-fails an empty template. Reword to state it is required, and regenerate the CRD description to match. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
|
405eb25095 |
fix(backups): harden Kafka metadata backup
- Fail closed on topic listing: set -o pipefail, capture kafka-topics --list --exclude-internal exit separately, and abort on failure so an unreachable broker can no longer produce an empty backup reported as Succeeded. An empty list after a successful call stays legitimate. - Check each kafka-topics --describe exit and validate the parsed partition count / replication factor before writing a record, so a timed-out describe cannot write a corrupt orders\t\t\t line. - Store topic configs one per line with key and value in separate tab-delimited fields, sourced from kafka-configs --describe (one config per line), so a value containing a comma (cleanup.policy=compact,delete) round-trips; on restore each config is applied with the value bracketed only when it contains a comma. - Detect topic divergence on restore instead of a silent --if-not-exists no-op: grow partitions with --alter when the backup asks for more, and fail loudly (naming the topic and both values) when it asks for fewer partitions or a different replication factor. A restore that cannot reach the recorded state no longer reports Succeeded. - Add KafkaStrategyKind to the RestoreJob cleanup-dispatch no-op group so deleting a Kafka RestoreJob does not run a pointless Velero cleanup; add a regression test alongside the siblings, plus a reconcile Kind-gate test and a rendering test that actually renders the strategy template. - Wire the kafka-metadata Chainsaw suite into both selectors (select-install suite_to_source, select-e2e src_to_suites) so editing the kafka app or the suite selects it and the bats unit tests pass. - Pin the client image to the operator's default broker version (kafka 3.9.1) with a digest, and align the example default; fix the values comment. Point the RabbitMQ comment references (which shipped in the CRD description) at the Job strategy actually mirrored, and trim the in-pod comments that rendered into the manifest. - Document the Strimzi Topic Operator caveat: for a topic owned by a KafkaTopic CR the operator reconciles its config from the CR, so an in-place config restore is durable only for out-of-band topics. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
|
a410a3f48b |
feat(backups): Kafka topic-metadata backup via a dedicated strategy driver
Add a strategy.backups.cozystack.io/Kafka driver that backs up the topic metadata of a Cozystack-managed Kafka application — partition counts, replication factors and non-default topic configs — through the Kafka Admin API, and restores it with kafka-topics --create + kafka-configs --alter. Modelled on the Job/Rabbitmq strategies: the driver renders a one-shot batch Job, gates on the Strimzi Kafka cluster reporting Ready, and keys each backup by .BackupName so runs do not overwrite each other. Scope is deliberately narrow: message payloads, consumer-group offsets, ACLs, quotas and KafkaUsers are out of scope, and there is no PITR. Restored topics are created directly (unmanaged on the target) — the accepted tradeoff of an Admin-API-only driver. Both restore flows are implemented: in-place recreates dropped topics into the source, to-copy applies the source's topics onto a freshly-bootstrapped empty cluster. Wire it into cozy-default (strategy-kafka-default.yaml gated on the bucket name, a BackupClass binding, read RBAC on kafka.strimzi.io/kafkas for the readiness gate, and the Strimzi Kafka client image), document it in backup-classes.md, and add an honest round-trip example plus Chainsaw e2e (examples/backups/kafka-metadata, kafka-3-metadata-roundtrip) that proves a per-run retention.ms sentinel survives both restores. Unit tests cover the driver's pure helpers (application-ref validation, readiness verdict, parameter round-trip, per-run artifact-URI keying). Not yet verified on a live cluster. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
|
4b6b1dc2da |
fix(vm-instance): let complete resources replace the instance type (#4109)
## What this PR does Closes #2909. The matcher keyed off `instanceType`, the domain sizing keyed off `resources`, and neither side looked at the other, so the chart could render both. KubeVirt refuses that, and the two halves of its rule are different shapes: `validateCPU` conflicts on `domain.cpu.cores` and `domain.cpu.sockets` only when they are non-zero, while `validateMemory` conflicts on `domain.resources.requests.memory` whenever the key is present. `values.yaml` ships `instanceType: "u1.medium"`, so clearing the field in the dashboard just falls back to that default. Fill in cpu, sockets and memory after clearing it and you get a VM the webhook rejects, naming fields the form gives you no way to clear. Resources that size the VM completely now suppress the matcher and size it directly. `packages/apps/kubernetes-nodes` already does this for its worker VMs. Beside an instance type, sizing that is only partial stops the render, naming what sized the VM and what did not; with no instance type in play there is nothing to conflict with, so that half renders as it does today. Letting resources win unconditionally would silently drop the instance type for a VM that sets memory alone, which is a different bug in the same place. Completeness is decided from what `domainResources` emitted, never from which keys the user set, and all three fields go through that one reading. The two readings come apart in more than one place. `resources.cpu` on its own emits nothing, so it conflicts with nothing and goes on rendering a plain instancetype-sized VM. cpu and sockets pass through `int64`, which yields 0 for a quantity the schema accepts, so `cpu: 500m` beside a whole sockets and a memory would read as complete from the values, drop the matcher, and hand KubeVirt a `cores: 0` the user never asked for. And a `resources` block nulled outright leaves `.Values.resources` nil rather than the chart's `{}`, so a raw field read is a render-time nil pointer where the emitted object is merely empty. The existence check and the pre-upgrade update hook read the same effective value. Keying the `lookup` on the raw one would fail a resource-sized render over an instance type it never references, which is the same confusing error this fixes. Reading `.Values.instanceType` in the hook would see the leftover value, skip the removal patch, and leave `spec.instancetype.revisionName` on a VM Helm had just given explicit domain resources. ### What changes in the rendered output | `instanceType` | `resources` | Before | After | | --- | --- | --- | --- | | set | unset | matcher only | unchanged | | unset | cpu, sockets and memory | domain cpu and resources | unchanged | | set | cpu, sockets and memory | matcher plus domain sizing, refused by KubeVirt | domain sizing, no matcher | | set | any subset with memory, or cpu with sockets | matcher plus a conflicting domain field, refused by KubeVirt | render stops, message names both sides | | set | cpu alone, or sockets alone | matcher only | unchanged | | set | cpu and sockets that both truncate to zero | matcher plus `cores: 0` | unchanged | | unset | anything short of all three | already refused by the existing check | unchanged | The rows that stay unchanged are deliberate. `virtual-machine.domainResources` writes `domain.cpu` only when cpu and sockets arrive together, so cpu alone emits no override at all. A `cores: 0` beside a matcher is not a conflict either, because `validateCPU` guards on non-zero, so KubeVirt admits it and the instance type sizes the VM. Both shapes render today and go on rendering. `cpuModel` is left alone for a related reason: it writes `domain.cpu.model`, which KubeVirt flags only when the instance type declares a model of its own, and no instance type shipped in `packages/system/kubevirt-instancetypes` declares one. ### Why no working release breaks Every shape that now fails at render was already being rejected by the KubeVirt validating webhook at admission, so it could not have produced a running VM in the first place. The failure moves from an opaque admission error to a render error that names the fields. Nothing that worked stops working. The unchanged rows above are measured. Each non-failing shape in the table, plus firmware on and off, an explicitly nulled `resources` block and the three update-hook branches, was rendered against a mocked cluster on both sides and compared document by document. Across 14 value shapes and 78 rendered documents, 77 come out byte-identical. The one that differs is the row the table marks as changed, which is also what the harness was checked against first, so it is known to be able to fail. ### Live-VM impact This changes a rendered VM spec, so an upgrade can resize or restart a running VM. Adding a complete `resources` block to a VM currently sized by an instance type drops its matcher, and the pre-upgrade hook patches `spec.instancetype` off the live object and writes the domain resources in its place. Cozystack runs KubeVirt with `vmRolloutStrategy: LiveUpdate` (`packages/system/kubevirt/templates/kubevirt-cr.yaml:125`), so KubeVirt applies what it can to the running VMI and marks the rest `RestartRequired`; I have not measured which half a given resize lands in, so plan the upgrade as if it restarts. Dropping the matcher drops the whole instance type, sizing and everything else it carries. Of the 69 shipped cluster instancetypes, 36 declare `dedicatedCPUPlacement`, 36 declare `hugepages` and 26 declare both. 20 declare NUMA passthrough, 8 declare `realtime`. A VM moved from `rt1.medium` to explicit resources loses those characteristics along with the type. Before this change that combination was rejected outright, so the VM never existed; now it is admitted, and the field help says so. ### Tests `tests/instancetype_resources_test.yaml` covers matcher only, resources only, both complete, each failing partial shape, the shapes that emit nothing, both cpu and sockets truncating to zero, `cpuModel` beside a matcher and beside complete resources, a stale instance type with and without complete resources, an explicit `instanceType: null`, an explicitly nulled `resources` block, and every branch of the update hook's instancetype selector, in both directions plus the two no-op cases. 24 cases in this package, 2127 across `make helm-unit-tests`, all green. Every case pinning new behaviour was checked against a reverted template to confirm it goes red; the rest pin behaviour this change deliberately leaves alone. A ten-mutation ladder over the helper, `vm.yaml` and the update hook leaves no survivor. The memory branch took the most work. Swapping its emitted read for a raw `.Values.resources.memory` looks like an equivalent mutant, since `domainResources` emits `resources.requests.memory` under the same truthiness condition. It is not: a `resources` block nulled outright leaves `.Values.resources` nil, and the raw read dereferences it. The case pinning that shape kills the mutant. The update-hook cases assert the `kubectl patch` command the Job will run, which is as far as a render-level test reaches. They do not show that KubeVirt accepts the resulting object, that helm-controller converges on a live VM whose `spec.instancetype` also carries a `revisionName` written by virt-controller, or which half of a resize lands as a live update rather than `RestartRequired`. That needs a cluster. The `dig` calls survive the shapes that reach them. `values.schema.json` rejects a non-object `resources` before render; Helm coalescing turns a nulled one into a nil `.Values.resources`, which digs to an empty object; and for any other non-map intermediate, current Helm turns the underlying sprig type assertion into a render error rather than a panic. ### Interaction with the firmware block `firmware` (UEFI/BIOS selection, Secure Boot, persistent EFI NVRAM) landed in this chart in the meantime, and it does not touch the sizing question. `domainResources` does not read it, so it takes no part in the completeness decision. KubeVirt's instancetype spec carries neither firmware nor `domain.features`, so nothing an instance type declares can conflict with what the firmware block renders. The boot mode a VM inherits when the block is empty comes from the preference, and the preference is not what gets dropped when complete resources replace the matcher. Both firmware states are covered on each side by the byte-identity comparison above. `README.md`, `values.schema.json`, `api/apps/v1alpha1/vminstance/types.go` and the ApplicationDefinition all come out of `make generate` in the package, not out of a hand merge. Re-running it on the result produces no further diff, and the root `make generate` reports no drift on top of it. ### On the ticket's second proposal #2909 also asks whether the non-empty `u1.medium` default belongs in the chart at all. That requirement is met, and not by removing the default. The default was a problem because a schema-generated form gave no way to say "no instance type": clearing the field falls back to `u1.medium`. Filling cpu, sockets and memory now says it directly, so the obstacle the ticket names is gone while the default stays. Dropping the default on top of that would change the rendered spec of every release that never set the field, with nothing left to gain. ### Named improvements, not done here The failure message names the fields that did not size the VM, computed from what was emitted, so for `cpu: 4` with no `sockets` it names `resources.cpu` even though the user set it. The message states the pair rule that explains this, but naming the actually-missing field first would read better. The two tests that pin `domain.cpu.cores: 0` document the pre-existing `int64` truncation in prose rather than through a `FIXME` reference, so a later reader has to read the comment to tell a contract from a bug we chose not to fix. Both are wording; neither changes behaviour. `resources.cpu`, `sockets` and `memory` are checked for shape and not for value, so `memory: "0"` and `-1Gi` pass the schema and fail further down. Both forms fail before and after this change and only the place of the failure moves, so that belongs with [#4104](https://github.com/cozystack/cozystack/issues/4104) rather than here. The new template comments run to about 46 lines against about 28 lines of logic. All of it is why rather than what and all of it is accurate, but the helper's third paragraph restates the commit body, and two sentences would carry that weight. The seven-line block in `vm.yaml` is the one worth keeping: swapping that gate to the effective value survives the entire suite, so what the block records is an equivalence no test can pin, and trimming it would delete the only record of it. The `instanceType` description runs to about 370 characters and reaches the dashboard as tooltip help, through `values.schema.json` and the ApplicationDefinition. That is long for a tooltip. It is accurate as it stands, and rewriting accurate text for length is a fresh unmeasured claim in the surface with the most readers, so it is not done here: the place for the detail about what a dropped instance type takes with it is the chart README, leaving the field help carrying the precedence rule alone. Choosing a `cx1.*` type and then giving it complete resources drops what the type guaranteed along with the type, and nothing says so at runtime. That is a consequence of the precedence rule rather than a defect in it, and the field help states it. A runtime signal would need the chart to compare what the type declares against what `resources` sets, which is its own task. Defaulting `sockets` to 1 would let a user size a VM from cpu and memory alone and would remove the commonest way to land in the partial case. It is more than the ticket asked for and it changes the rendered spec of stored releases that set cpu and memory without sockets, so it needs its own call on compatibility. ### Not changed Two pre-existing quirks stay as they are, because both reach past the conflict this fixes. `resources` fields are read for truthiness, so an explicit `0` reads as unset, in `domainResources` and in the existing "Either instanceType or resources" check alike. And with no instance type in play, a fractional `resources.cpu` still truncates to `cores: 0` and renders a VM sized by nothing, with no error. The new guard covers only the half where a matcher is present. The tests pin both as they stand, which is what keeps working releases rendering. ### Screenshots No dashboard code is touched. The only user-visible surface is two field descriptions the dashboard renders as help text, regenerated into `values.schema.json` and the ApplicationDefinition. `instanceType` now reads "Ignored when `resources` sizes the VM, which needs whole-number cpu and sockets plus memory; the whole type is dropped then, so any dedicated CPU placement, hugepages, NUMA or realtime settings it carries go with it. While this field is set, a `resources` block that sizes only part of the VM is rejected outright instead of ignored, so complete it or clear it." `resources` states the same rule from its own side. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: I walked the trigger map against the diff and no listed trigger fires: no package added, renamed or removed; no `values.schema.json` field added, renamed or removed; no default changed; nothing under `hack/` moved; `templates/service.yaml` and its `wholeIP` and `allowICMP` annotations untouched. Two repositories do carry prose this makes incomplete. I left every box empty rather than push follow-ups nobody agreed to, so a maintainer decides. `cozystack/website`: `content/en/docs/next/virtualization/resources.md` opens with "Each virtual machine has these two configuration settings" and lists only `instanceType` and `instanceProfile`. The page is hand-written, so the release-time bot that regenerates app reference pages from `README.md` will not reach it. `cozystack/terraform-provider-cozystack`: `internal/provider/vminstance_schema.go` describes `instance_type` as "Virtual machine instance type." Its `kubernetes_schema.go` and `kubernetes_nodes_schema.go` already spell out the equivalent precedence rule for their own charts. No schema field or default changed here, so its new-field test guard has nothing to catch. ### Release note ```release-note fix(vm-instance): a complete `resources` block (whole-number cpu and sockets, plus memory) now sizes the VM directly and suppresses the `instanceType` matcher, instead of rendering both and having KubeVirt refuse the VM. Beside an `instanceType`, a `resources` block that sizes the VM only in part now stops the render with a message naming both sides, where it previously produced a VM the admission webhook rejected. Note that suppressing the matcher drops the whole instance type, including any dedicated CPU placement, hugepages, NUMA or realtime settings it carries. A release whose values already carry both converts on the next platform upgrade rather than at a moment the operator picks: the pre-upgrade hook strips `spec.instancetype` off the live VM and writes the domain sizing in its place, so plan that upgrade as if the VM restarts. ``` |
||
|
|
1f04e31ed3 |
feat(backups): add Redis backup/restore strategy driver (#3992)
## What this PR does Adds backup/restore support for the `Redis` app (spotahome `RedisFailover`) via a dedicated `strategy.backups.cozystack.io/Redis` strategy controller. The spotahome operator ships no backup CRD, so the driver follows the generic-script-Job pattern (a specialisation of the Job/Altinity drivers): it runs a one-shot Job that dumps an RDB snapshot from the current master to the system bucket and replays it on restore. - **Backup**: discover the current master via the operator's sentinel Service (`rfs-<name>`, master `mymaster`) → `redis-cli --rdb` → `curl --aws-sigv4` PUT to `s3://<bucket>/<namespace>/<app>/dump.rdb`. - **Restore**: GET the object → convert RDB → RESP (`hdt3213/rdb`) → `FLUSHALL` + `redis-cli --pipe` into the target's master. Both **in-place** and **to-copy** are implemented; a to-copy restore reads the source's prefix via `.Backup.ApplicationRef.Name` while writing into a differently-named target. - **PITR**: not supported — RDB is a discrete snapshot with no continuous log to replay to an arbitrary time (the etcd-like case). The driver is a specialisation of the Job/Altinity script-Job flow with a `Redis` applicationRef gate and an app-readiness precondition (surfaces a precise `Ready=False` instead of a silent hang; blocks only on an explicit `Ready=False` so a missing condition never wedges backups). Strategy parameters round-trip through the Backup's `DriverMetadata`; there is no `underlyingResources` snapshot (the Job pattern re-renders on restore). ### System bucket Coordinates and credentials reach the Job as env `secretKeyRef` from the projected `cozy-backups-creds` (the ClickHouse/Altinity delivery model), so **the app chart is untouched** — no `backup.useSystemBucket`, no new RBAC (`batch/jobs` and `apps.cozystack.io/*` are already granted). The bare-host `endpoint` gets `https://` prepended when unscheme'd; against a self-signed endpoint the strategy verifies with the CA from `backupStorage.endpointCASecretName` when set, otherwise against the image's system CA store; skip-verify is a separate explicit `backupStorage.insecureSkipTLSVerify` opt-in, never a fallback. The `cozy-default-redis` strategy is gated on `$bucketName` (CRD-ordering) and bound in the `cozy-default` BackupClass. ### Changes - `api/backups/strategy/v1alpha1/redis_types.go` + generated CRD and deepcopy. - `internal/backupcontroller/redisstrategy_controller.go` (+ unit tests) wired into both dispatch switches, `supportedBackupStrategyKinds`, and the no-op restore-cleanup case. - `cozy-default-redis` strategy template + `cozy-default` BackupClass route + helm rendering-gate coverage. - `images/redis-backup` (redis-cli + curl + RDB→RESP converter) + Makefile target + `backupStrategyController.redisBackupClientImage`. - `examples/backups/redis/` — an honest marker round-trip for both restore flows. ### Validation `go build`/`vet`, `go test ./internal/backupcontroller/... ./api/backups/...`, `make generate` (clean), and `helm unittest` (10/10) all pass. Not yet cluster-verified. ## Downstream None. ```release-note Add backup and restore support for the Redis application (spotahome RedisFailover) via a new Redis backup strategy, wired into the cozy-default BackupClass. Supports in-place and to-copy restore; PITR is not supported. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Redis backup and restore support using RDB snapshots in S3-compatible storage. * Supports restoring data in place or to a separate Redis instance. * Added configuration for Redis backup images, custom HTTPS certificate authorities, and TLS verification. * Added a complete Redis backup and restore demonstration with automated setup, verification, and cleanup. * **Documentation** * Documented Redis storage, credentials, endpoints, TLS, and restore behavior. * Added administrator and tenant scenario guides. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
50e1e2b2ed |
feat(gateway): add tlsPassthroughListeners to the TenantGateway API
A tlsPassthroughServices entry publishes a hostname on port 443, which suits a service whose clients speak HTTP and mis-serves one whose clients expect a database on its own port. The new field takes a name, a native port and the SNI hostname to answer on it, so an engine can be reached where its drivers already look. The cross-field rules are enforced at admission as well as in the controller. renderGateway is the first thing that renders, so a spec the apiserver accepts and the renderer refuses aborts everything behind it, and one mistyped hostname would freeze the Gateway and every route status where they stand. The controller keeps its own copy because it and the CRD roll out separately. Rules with no admission form, and why each one has none, are named on the type. tlsPassthroughServices and apex gain bounds at the same time, for the CEL cost estimator rather than for their own sake: the new rules concatenate apex and scan the services list, and an unbounded string or list makes the install-time estimate exceed the per-CRD budget and the apiserver refuse the whole definition. Every bound is a kind the apiserver ratchets, so a stored value that violates one keeps accepting writes that leave it alone. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
afc003c464 |
feat(postgres): database read-replica autoscaling via KEDA (#3954)
## What this PR does Adds automatic horizontal autoscaling of a PostgreSQL database's **read replicas** in response to load, implementing the merged design proposal [Database Horizontal Autoscaler](https://github.com/cozystack/community/tree/main/design-proposals/database-horizontal-autoscaling) (community#44). The mechanism is entirely stock — no bespoke operator, no new CRD: - **KEDA as an optional platform component** (`packages/system/keda`, wired into `core/platform` as a source + system-bundle entry). It claims the `external.metrics.k8s.io` APIService that nothing served before. - **`cozy-lib.keda.scaledObject` helper** (`packages/library/cozy-lib/templates/_keda.tpl`) renders a KEDA `ScaledObject`, fail-closed on missing/invalid arguments. - **`packages/apps/postgres` autoscaling** — an `autoscaling` values block (`enabled`, `minReplicas`, `maxReplicas`, `metric`, `target`, `maxReplicationLagSeconds`, `dryRun`, `transition`) validated by `values.schema.json`; under active autoscaling the chart renders a `ScaledObject` and seeds `spec.instances` to a constant floor (`max(replicas, effectiveMin)`), while the postgres release is forced to client-side (two-way) Helm apply so the seed is a merge no-op on upgrade and the KEDA-managed HPA — the runtime writer via the CNPG `Cluster` scale subresource — is not reverted. - **Single-value metric (§1)** — one PromQL query returns `Σ(read load over standbys) + target`, so a stock HPA computes `desired = ceil((Σ+target)/target) = 1 + ceil(Σ/target) = primary + desiredRead`, with no per-pod emission. - **The four static brakes (§5)** — quorum floor `max(minReplicas, maxSyncReplicas+1, 2)` (quorum wins over `maxReplicas`), scale-down pacing (`Pods/1/600s`), a replication-lag clamp folded into the query (freeze both directions while lag exceeds the threshold and the primary is writing, with `max_over_time` hysteresis), and dry-run (render paused). - **Two-phase enable/disable migration** via a `transition` sub-flag: phase 1 stages `spec.instances` to the live count and brings KEDA up with its HPA paused in both directions; flipping to phase 2 hands the runtime count to KEDA, and because the seed re-renders the same count the flip is a merge no-op with no dip. - **Observability** — a database-autoscaling Grafana dashboard and PrometheusRule alerts. **Live validation (dev cluster, CNPG 1.30.0).** The full path works end-to-end: the CNPG `Cluster` scale subresource serves `status.selector`, KEDA renders the HPA, the metric reaches it (`ScalingActive=ValidMetricFound`, `TARGETS 151/150`, `desiredReplicas=2 = 1 primary + 1 replica` — the §1 arithmetic), and the lag-clamp query returns the base value on live VictoriaMetrics. Two packaging fixes were needed and are included: KEDA must run on the cozystack `cozy.local` cluster domain (the upstream `cluster.local` default makes the metrics gRPC target NXDOMAIN and every HPA hangs on `FailedGetExternalMetric`), and the operator's client-go throttle is raised so a single-replica operator does not lose its leader lease under scale-subresource polling. Implements #3953. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two repositories are affected; both are follow-ups to open once this lands (and after its dependency #3951 merges and the values surface is final), left unticked here for a maintainer to confirm rather than claimed with a speculative link: - **cozystack/terraform-provider-cozystack** — this adds the nested `autoscaling` block to `packages/apps/postgres/values.schema.json`. The provider hand-models each app's fields (schema + model + expand/flatten), so it needs the new block to expose it in Terraform. - **cozystack/website** — KEDA is a new platform component (`guides/platform-stack`), and the postgres reference page gains the `autoscaling` params. The managed-app reference is regenerated from `README.md` by the release bot; the platform-stack component list is hand-maintained. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: to open (new platform component + postgres autoscaling params) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: to open (new `autoscaling` block in postgres values.schema.json) ### Depends on - #3951 — bump CloudNativePG to 1.30.0. The HPA can only drive the CNPG `Cluster` scale subresource on CNPG ≥ 1.28.4, where it serves `status.selector`; without it the HPA fails with `InvalidSelector`. This PR should not merge before #3951. ### Release note ```release-note feat(postgres): add optional horizontal autoscaling of read replicas via KEDA, configured through the new `autoscaling` values block; KEDA ships as a new optional platform component. Requires CloudNativePG >= 1.28.4 for the Cluster scale subresource selector. Note: every Postgres release now applies client-side (`release.cozystack.io/helm-server-side-apply: "false"`) so the autoscaler's live `/scale` count is not reverted by Helm; a kind-wide side effect is that chart-rendered fields on Postgres releases are no longer re-asserted by server-side-apply drift correction (driftDetection is unset), a documented trade-off with a per-instance apply strategy as follow-up. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable PostgreSQL read-replica autoscaling with KEDA. * Added CPU and read-connection metrics, replica bounds, replication-lag safeguards, dry-run mode, and transition handling. * Added optional KEDA platform support, Grafana autoscaling dashboard, and Prometheus alerts. * **Documentation** * Clarified integer thresholds, prerequisites, configuration, and transition guidance. * **Bug Fixes** * Improved validation and actionable errors for unavailable KEDA or invalid settings. * **Tests** * Expanded coverage for autoscaling, observability, package selection, and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
1289f522fa |
fix(backups): unbreak install, kill destructive cleanup, reach the resource knob
Store the Redis strategy Template schemaless, as the Rabbitmq strategy already does: without the markers controller-gen emitted the full PodTemplateSpec schema and the CRD reached ~602 KB, pushing the chart's Helm-release Secret past the 1 MiB cap so backupstrategy-controller failed to install at all. Give the example strategy a cleanup branch, and both scripts an explicit `elif restore` / `else exit 1`, so a deleted Backup rendered with Mode=cleanup deletes its object instead of falling into the restore path and FLUSHALLing the live master. Forward a backupStrategyController block through the platform bundle alongside backupStorage, so redisBackupResources — the restore-Pod sizing knob the loader-not-ready message points at — is reachable via spec.components.platform.values.backupStrategyController, not merely settable on a Helm-managed Strategy CR that reverts on the next upgrade. Correct the endpointCASecretName doc row to match the now-optional mount, and `int` the activeDeadlineSeconds interpolation so a large values-file override renders as an integer, not exponent form. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
b4d621f4f7 |
fix(backups): delete Redis backup objects on cleanup and size the restore Pod
Delete the backup object when its Backup is removed. The Redis driver
owns its S3 object (one per BackupJob, no engine retention prunes it), so
it now spawns a cleanup-mode Job that deletes the object and holds the
Backup's finalizer until it completes, mirroring the Rabbitmq driver.
Without this a retention-pruned Plan grew the bucket unboundedly and the
key was lost with the CR.
Size the strategy Pod from backupStrategyController.redisBackupResources
so an admin can raise the restore ceiling (the throwaway loader holds the
whole dataset in memory) via chart values, which a direct edit of the
cluster-scoped, Helm-managed Strategy CR could not survive. The
loader-not-ready diagnostic now names the memory limit, the likeliest
cause, instead of only the image version.
Make the endpoint CA volume optional: nothing projects that Secret into
the app namespace, so a required mount wedged every backup Pod on
FailedMount; a missing ca.crt now falls through to the system CA store
and fails the handshake fast, which is not the same as skipping verify.
Promote the Pod-level activeDeadlineSeconds to the Job so a wedged run is
bounded once, not (backoffLimit+1) times. Use find, not ls, for the base
AOF so its no-match diagnostic is not dead code under pipefail.
Pin the object key reaching the rendered Job: both fixtures now render
--key={{ .ObjectKey }} and assert it, so zeroing the key (which left the
package green) is caught. Document .ObjectKey and the cleanup mode in the
CRD render-context, list cozy-default-redis among the Helm-managed CRs,
and mirror the CA/TLS and find fixes into the example strategy.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
|
b0eb104276 |
feat(backups): add Redis backup/restore strategy driver
The spotahome RedisFailover operator ships no backup CRD, so add a dedicated strategy.backups.cozystack.io/Redis driver that runs a one-shot Job: it dumps an RDB snapshot from the current master (discovered via the operator's sentinel Service) to the system bucket over S3, and on restore replays it (RDB -> RESP via redis-cli --pipe) into the target's master. Structurally a specialisation of the Job/Altinity script-Job pattern with a Redis applicationRef gate and an app-readiness precondition. Backup parameters round-trip through the Backup's DriverMetadata (no underlyingResources snapshot); restore drives from .Backup.ApplicationRef so a to-copy restore reads the source's object-storage prefix while writing into a differently-named target. Both in-place and to-copy are supported; PITR is not (RDB is a discrete snapshot). - api: Redis/RedisList strategy type + generated CRD and deepcopy - driver: reconcileRedis / reconcileRedisRestore + pure helpers, wired into both dispatch switches, supportedBackupStrategyKinds and the no-op restore cleanup case - default: gated cozy-default-redis strategy (system-bucket coords via cozy-backups-creds env secretKeyRef, https:// scheme + optional CA) and the Redis route in the cozy-default BackupClass - image: images/redis-backup bundling redis-cli + curl + an RDB->RESP converter, wired via backupStrategyController.redisBackupClientImage - tests: driver unit tests (gate, precondition, render, param round-trip, both reconcile paths) and the helm rendering-gate coverage - example: examples/backups/redis honest marker round-trip for both restore flows Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
|
59731429a8 |
fix(vm-instance): let complete resources replace the instance type
KubeVirt refuses a VirtualMachine that carries an instancetype matcher and explicit domain sizing at once. validateCPU in pkg/instancetype/apply conflicts on domain.cpu.cores and domain.cpu.sockets when they are non-zero, and validateMemory conflicts on domain.resources.requests.memory whenever the key is present. The chart rendered both, because the matcher keyed off instanceType while the domain sizing keyed off resources, and neither side looked at the other. values.yaml ships instanceType "u1.medium", so clearing that field in the dashboard only falls back to the default. A user who then fills in cpu, sockets and memory gets a VM the webhook rejects, with an error naming fields the form gives them no way to clear. Resources that size the VM completely now replace the matcher, the same way the kubernetes-nodes chart sizes its worker VMs. Beside an instance type, sizing that is only partial stops the render, because there is then no safe reading: dropping the matcher would size the VM from fields the user never finished, and keeping it either loses to the webhook or silently discards what was typed. With no instance type in play there is nothing to conflict with, so that half is left as it renders today. Completeness is decided from what domainResources emitted, not from which keys the user set, and the two readings come apart. resources.cpu on its own emits nothing, so it conflicts with nothing and goes on rendering a plain instancetype-sized VM. cpu and sockets pass through int64, which yields 0 for a quantity the schema accepts, so a cpu of 500m beside a whole sockets and a memory would read as complete from the values, drop the matcher, and hand KubeVirt a cores of 0 the user never asked for. And a resources block nulled outright leaves .Values.resources nil, where a raw field read is a render-time nil pointer and the emitted object is merely empty. The existence check and the update hook read the same effective value. Keying the lookup on the raw one would fail a resource-sized render over an instance type it never references, which is the same confusing error this fixes. Reading .Values in the hook would see the leftover instanceType, skip the removal patch, and leave spec.instancetype.revisionName on a VM Helm had just given explicit domain resources. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
678d22f209 |
docs(postgres): move the serverAddress namespace-pin note out of a template comment
Closes IvanHunters' MINOR on scaledobject.yaml:97. The API-owner-gate marker was a Go template comment, stripped at render, so the ownership question and the pin's residual surface were recorded where nobody reads them again. State the constraint on the serverAddress field doc (propagated into values.schema.json, README and the cozyrds openAPISchema) including the accepted residual: tenant-root is always in the allow list because the derived default points there, so a tenant can still aim the operator at a root Service as a blind in-cluster probe (no response body reaches the tenant), which the egress NetworkPolicy does not narrow. The API-owner sign-off is tracked on the PR, not in a stripped comment. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
07f1daa22d |
docs(postgres): correct the replicas "restored on disable" claim
Closes IvanHunters' MINOR on values.yaml:58. The replicas description (propagated into values.schema.json, README and the cozyrds openAPISchema) said the count is "restored on disable". Under client-side apply it is not: the two-way merge writes instances only when the rendered value changes, so a cluster KEDA grew past replicas stays there after autoscaling.enabled: false with no autoscaler left to reduce it — the sibling dryRun doc already states the true rule, so the two contradicted each other. Reword both the @param and the @typedef to say the live count is not auto-restored and that the operator lowers replicas to reclaim the grown capacity; regenerated the derived artifacts. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
d000d02b99 |
feat(rabbitmq): add TLS support for AMQPS and management HTTPS (#2683)
## What this PR does Adds TLS for AMQP and the management API, and hands the tenant a key-free trust anchor instead of a private key. - A chart-authored cert-manager chain issues the broker certificate. The chart names its CA Secret in a `TenantProjection` sentinel; the CA-extraction controller reads `ca.crt` from that Secret and writes a key-free projection, stamping `internal.cozystack.io/tenant-ca` on the projection it owns. That label is what the ResourceDefinition selects. Nothing labels the CA Secret itself, which holds the CA private key and is excluded by name. - The leaf `<release>-tls` carries the broker's private key and reaches no tenant. Neither the dashboard Role nor the ResourceDefinition grants it, and the ResourceDefinition excludes it by name as well, so a label mis-stamped onto it later still does not get through. - SAN coverage now lists every resolvable form explicitly, including the bare `*.<release>-nodes` wildcard that the operator uses for `RABBITMQ_NODENAME`. ## TLS is opt-in here, unlike the sibling charts `tls.enabled` defaults to `false` and is not inferred from `external`. That diverges from nats, qdrant and kafka, which infer it, and the divergence is deliberate for a reason specific to this operator. Setting `spec.tls.secretName` makes the cluster operator rewrite the `port` key in the `<release>-default-user` Secret from 5672 to 5671, on the mere fact of TLS and regardless of whether the plaintext listener is still open. That Secret is the Service Binding the chart hands the tenant. A client with a hardcoded `:5672` keeps working. A client that reads the endpoint from the binding, which is the documented way, gets moved to the TLS port and must then speak TLS and be given a CA to verify against, neither of which it was configured for. Inferring TLS from `external` would fire all of that on an ordinary upgrade, with no configuration change, against exactly the clients that followed the documentation. Whether the other engines should follow is an epic-level question rather than this PR's to answer; for nats it is raised in #3348. ## The plaintext ports are still published, and this PR says so There is no operator-supported way to publish the TLS ports alone. `spec.override.service.spec.ports` is a strategic merge keyed on `port` against defaults the operator rebuilds every reconcile, so it adds and mutates entries but never subtracts one, and a TLS-only port list there would read as a guarantee while changing nothing. The chart renders no service override at all and says why at the callsite. `tls.disableNonTLSListeners` is offered as an explicit opt-in, and it refuses to render rather than failing silently in three situations. - Together with `users` or `vhosts`. The flag forces messaging-topology-operator onto TLS regardless of its own configuration, after which it verifies the broker against the system certificate pool, where a per-release CA cannot be. Every topology object bound to the release would stop reconciling, silently: already-created users keep working while password rotation and new users simply never apply. There is no working route left for it, so the chart refuses instead. Managing those objects as hand-written CRs is not a way out, because the operator takes TLS from the `RabbitmqCluster` and not from whoever created the CR, so manual objects break identically and without the render-time refusal. `rabbitmqctl` or a definitions import are. - While topology objects are still live in the namespace. See the next section. - With TLS off, where no listener of either kind would remain. Erlang distribution (25672) and epmd (4369) stay plaintext under every setting; the flag does not touch them, so `disableNonTLSListeners: true` is not "TLS everywhere". A third route, closing AMQP at the broker through `spec.rabbitmq.additionalConfig` while leaving the operator on plaintext management, has been suggested and is plausible, but the operator's sources are not vendored here and it was not verified. It is recorded as an open question rather than promised. ## Closing the listeners takes two upgrades, and the chart enforces it Clearing `users` and `vhosts` and setting `tls.disableNonTLSListeners: true` in one edit is the natural thing to try, and it wedges the objects being removed. The values-level refusal cannot catch it, because by then both maps are empty. Helm applies every object in the target release before pruning the ones the release dropped, so the broker closes plaintext and loses the `rabbitmq.com/operator-connection-uri` annotation while the topology objects still hold `deletion.finalizers.<plural>.rabbitmq.com`. Their operator then has to reach a management API it can only get to over a certificate it verifies against the system trust pool. The delete never completes, and the upgrade reports success over objects left in `Terminating`, with the users still live in the broker and `kubectl delete namespace` on that tenant hanging. The chart now reads the namespace rather than the values. When the flag is being introduced and any topology object is still there, the render refuses and names them, gives the two-upgrade order and the `kubectl get` that confirms nothing bound to this release is left, and says how to unwedge a release that is already stuck. The check keys on the live `RabbitmqCluster` as well, so it applies only to the transition: once the listeners are closed, an object a tenant creates afterwards is theirs to clean up rather than something that stops the release rendering. Eleven unit cases cover the firing and the silent paths. The set is wider than the three kinds this chart creates. All twelve kinds `messaging-topology-operator` drives through its shared `TopologyReconciler` carry that finalizer and share one deletion path, so a `Queue` or a `Shovel` written by hand against the release wedges exactly the same way, and a runbook step that checks only users, vhosts and permissions says the namespace is clear when it is not. The list is derived in one place and read from both the lookup and the `kubectl get` the refusal prints, and a unit test holds it against the operator's own CRDs so a bump cannot introduce a kind the guard has never heard of. `SuperStream` is the one CRD left out: it runs on its own reconciler with no finalizer, and the objects it owns are already on the list. An object that binds through `connectionSecret` instead of naming the cluster stays outside the guard, which the comment says. ## Breaking for metrics scrapers Enabling TLS moves the metrics endpoint from 15692 to 15691. A ServiceMonitor selecting the port by the name `prometheus` silently stops matching and the instance's metrics disappear with no error anywhere. Nothing in this repository scrapes RabbitMQ, so only user-defined monitoring is affected, silently, which is why it is stated here. ## Known limitations 1. Turning TLS back off leaves the raw CA material behind. The anchor itself is collected: the projection carries an owner reference to the `TenantProjection` sentinel, and Helm deletes the sentinel along with the rest of the chain. What survives is `<release>-ca` and `<release>-tls`, because cert-manager runs with `enableCertificateOwnerRef: false` and does not delete the Secrets it issued, so the README says to remove them by hand. #3349 asks the same question of the label-driven leg of the contract, which this chart does not use. 2. No e2e coverage for the TLS path. The one-upgrade wedge above is the case that most deserves it: install with TLS and users, upgrade to no users plus the flag, and assert the `User` is actually gone rather than merely marked for deletion. A template test cannot reach that. On certificate renewal, the leaf carries `duration: 8760h` and `renewBefore: 720h`, so cert-manager rewrites the Secret around day 335. The cluster operator mounts it as a projected volume with no `subPath`, so the kubelet refreshes the files in place, and the operator does not watch a user-supplied TLS Secret, so nothing rolls the pods. The last hop, the broker picking the new file up, rests on Erlang's `ssl_pem_cache` re-reading by mtime and has only been read from sources rather than exercised against a live broker. The merged qdrant chart ships the same duration and `rotationPolicy`, so this is a platform-wide property rather than something this PR introduces. ### Downstream repositories `packages/apps/rabbitmq/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_rabbitmq`; tracked in cozystack/terraform-provider-cozystack#42. ### Release note ```release-note feat(rabbitmq): opt-in TLS for AMQP and the management API, with the CA published to tenants as a key-free trust anchor. Enabling TLS moves the metrics port from 15692 to 15691 (ServiceMonitors selecting the port named `prometheus` must be updated) and moves the `port` value in the `<release>-default-user` binding Secret from 5672 to 5671. Plaintext AMQP and management ports remain published; `tls.disableNonTLSListeners` closes them but also drops in-cluster plaintext clients, and must be enabled in a separate upgrade after the topology objects are gone. ``` |
||
|
|
d685fb30ec |
fix(gateway)!: require tenant routes to declare their hostnames (#3475)
## What this PR does The tenant route hostname policy skipped the shape it most needed to check. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute or TLSRoute declaring no hostnames satisfied the first operand and got admitted with nothing compared against the tenant apex. An empty list passed too: `has()` is true on `[]`, and `all()` over an empty list is vacuously true. Such a route is not inert. Under Gateway API it inherits the hostnames of every listener it attaches to, and `parentRefs` can name a Gateway in another namespace, so it covers whatever that listener covers. Reported in #3467. The predicate now requires the field to be present and non-empty before validating it. Both policies rendered from that template share one CEL string, so HTTPRoute and TLSRoute get the change together, and both match `CREATE` and `UPDATE`, so an existing route that drops its hostnames is denied exactly as a new one is. The issue is titled after TLSRoute, but the reachable shape is mostly HTTPRoute. TLSRoute allows a hostname-less route only on the deprecated `v1alpha2`; at `v1` and `v1alpha3` the bundle this repo vendors marks `spec.hostnames` required and rejects an empty list on `minItems: 1`, while `v1alpha2` carries neither constraint. HTTPRoute `v1` carries neither either, so it accepts both shapes. All four are checkable in `packages/system/gateway-api-crds/templates/crds-experimental.yaml`. Checking that turned up a stale claim in the template header, which said TLSRoute had no `v1` in Gateway API 1.4 to 1.5 and used it to justify the rule naming only `v1alpha2`. Gateway API 1.5.1, which this repo vendors, serves TLSRoute at `v1` as the storage version. The rule still covers every served version because `matchConstraints.matchPolicy` defaults to `Equivalent`, and that default is now written down, because setting `matchPolicy: Exact` reads like a tightening and would silently stop the policy seeing `v1` and `v1alpha3` TLSRoutes. I corrected the text and left the coverage alone: the wrong sentence predates this branch, but my change is what makes the file contradict itself, so the text is mine to fix while widening version coverage is a separate job. A `ValidatingAdmissionPolicy` has no cross-object lookup, so it cannot resolve the parent Gateway to see which hostnames a route would inherit. Requiring the route to name its own is what keeps every admitted hostname comparable against the apex. That has a cost: a route relying on listener inheritance now has to spell its hostnames out, and a tenant that hand-writes one gets a denial where it used to get an admission. ### The controller change belongs in the same PR `renderHTTPRedirect` built the http-to-https redirect HTTPRoute with no hostnames on purpose, to catch every host on port 80. It lands in a `tenant-*` namespace, so the stricter policy denies it, `reconcileHTTPToHTTPSRedirect` returns that error, and since it runs before the status update the TenantGateway never reaches Ready. That redirect is what keeps app routes attaching by hostname from serving plaintext on port 80. Neither half of this change is safe alone. The redirect now names the tenant apex and its wildcard. Both entries are needed: a wildcard hostname is a suffix match over any number of labels, so `*.<apex>` covers subdomains at every depth but not the bare `<apex>`. `renderWildcardCertificate` already builds the same pair. Coverage narrows in one direction. A request on port 80 with a Host outside the tenant apex now falls through to a 404 instead of a 301. That narrowing costs no covered host: the port-80 listener admits routes only from the Gateway's own namespace and cert-manager's, and the tightened policy requires every route in the tenant namespace to declare its hostnames inside the apex, so any host published from this namespace already falls under the apex or its wildcard. An inheriting child tenant does not widen that set either. Its hostnames would reach port 443 through a listener on this same Gateway, and `cozystack-gateway-hostname-policy` compares every listener against the host label of the Gateway's own namespace, so a child apex outside that label never gets its listener: the Gateway write carrying it is refused at admission whole, in `reconcileGateway` and before the redirect is rendered. A child apex under this one passes that check and the wildcard already covers it. That holds at merge base as well, so it is not something this branch changes. Deriving hostnames from `spec.apex` makes that field load-bearing for admission, so it gains `MinLength=1`. An empty apex used to reconcile to Ready, since the redirect declared no hostnames at all; it would now render `""` and `"*."` and be rejected on the HTTPRoute hostname schema, surfacing as a validation error naming the route rather than the field. The renderer checks it too, because the CRD and the controller binary roll out separately and an older CRD in the cluster will not carry the bound. That schema bound is a breaking change to an existing field and the API review gate flags it as one, so here is what it actually invalidates. The chart that renders these CRs already fails on an empty `_namespace.host`, and in DNS-01 or existingSecret mode an empty apex already broke earlier, in `renderGateway`, on a listener with `hostname: "*."`. Nothing that works today becomes invalid. What the bound rejects is a TenantGateway written by hand, past the chart, with an empty apex, and I would rather that be refused at admission naming the field than at reconcile time in controller logs, which is why the bound sits in the CRD as well as the renderer. Naming those hostnames also meant excluding the redirect from `collectHostnameClaims`. A route that attaches to the Gateway and declares hostnames otherwise reads as an app publishing them, and each claimed hostname becomes an HTTPS listener named after its first label. The first label of `*.<apex>` is `*`, which the Gateway CRD's listener-name pattern rejects, so the Gateway write fails and the reconcile stops before anything downstream. The exclusion matches on namespace, name and controller ownership rather than on the labels the renderer stamps: those labels are writable by anyone who can create a route, and the claim set also drives conflict resolution, so a label-keyed exception would let any route opt out of being marked `Accepted=False` when it loses a hostname race. Name and ownership are each required and each is pinned by its own test, since ownership alone would exclude any other controller-owned route and name alone would extend the exception to a route planted under that name by someone else. ### Radius Only namespaces starting with `tenant-` are matched. The three first-party TLSRoutes the platform ships, `kubernetes-api` in `default` plus `cdi-uploadproxy` and `vm-exportproxy`, set hostnames explicitly and none sits in a `tenant-*` namespace, so nothing changes for them. I went through every producer of routes in the tree, in three places rather than one. No Helm template emits an absent or empty hostname list in any configuration a tenant can reach: the vendored openbao, harbor, opencost and flux-operator route templates can render one, but each is behind a flag that defaults off with no values path to turn it on, and openbao would fail CRD validation first because its default route apiVersion requires hostnames. Exactly one Go site builds a route, the redirect fixed here. And one e2e fixture did rely on the old behaviour: the Gateway suite's parentRef probe declared no hostnames and is applied into `tenant-test`, which the policy matches, so it now reads the apex from the namespace label and names a hostname under it, the same way the neighbouring cases in that suite already did. cert-manager also writes routes into tenant namespaces, since it creates the Challenge in the Certificate's own namespace and the HTTP-01 solver route beside it. That route declares the hostname it is validating, so the policy admits it. ### Related issues Two open issues have a text half that this PR happens to settle, and neither of them closes here. #3788 reports that the port-80 allow-list contains the namespace its own comment says it excludes; the comment and the README paragraph now describe what that filter actually does, while the behavioural exit it names stays open. #3789 reports that the TLSRoute rule names only the deprecated API version; that premise is now written down in the template header, including what has to change before a bundle drops `v1alpha2`, but the rule still names `v1alpha2` alone. #3787 covers the wildcard-hostname listener name that the apiserver rejects, which is what makes the claim-set exclusion here load-bearing rather than tidy. ### Screenshots N/A (no UI changes). ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No package is added, renamed or removed; no `values.yaml` key, `values.schema.json` field, version enum or default moves; no `kind` or `plural` changes; no namespace or release-asset name changes; nothing under `hack/` moves, is renamed, or changes what a make target does. One row deserves a sentence rather than a blanket denial, because the diff does change a CRD schema: `spec.apex` on `gateway.cozystack.io/v1alpha1 TenantGateway` gains a `minLength` bound, which lands in the generated `gateway.cozystack.io_tenantgateways.yaml`. The provider hand-types `Package` under `cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`; `gateway.cozystack.io` is not among them, and a tightening on an existing field adds no new field for it to offer typed access to. The `packages/extra/gateway/README.md` edit sits in the `## Security model` section, below the `## Parameters` block that `cozyvalues-gen` regenerates, so it survives `make generate`. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(gateway)!: the tenant route hostname policy now requires an HTTPRoute or TLSRoute in a `tenant-*` namespace to declare a non-empty `spec.hostnames`. A route that omitted the field, or set it to an empty list, was previously admitted without any hostname being checked against the tenant apex, even though such a route inherits the hostnames of the listener it attaches to rather than matching nothing. A route in a `tenant-*` namespace relying on that inheritance is now rejected and has to name its hostnames. The controller-owned http-to-https redirect route names the tenant apex and its wildcard accordingly, so a request reaching port 80 with a Host outside the tenant apex now gets a 404 instead of a 301. `TenantGateway.spec.apex` additionally gains a `minLength: 1` bound, because the redirect derives its hostnames from that field; a TenantGateway written past the chart with an empty apex is now rejected at admission instead of failing later in the controller. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added edge-terminated gateway support with plain-HTTP listeners and TLS passthrough handling. - HTTP-to-HTTPS redirects now explicitly cover the tenant apex and subdomains. - Gateway configurations require a non-empty apex hostname. - HTTPRoute, TLSRoute, and GRPCRoute resources must declare hostnames. - **Bug Fixes** - Invalid hostname-less routes are now rejected consistently. - Controller-managed redirects no longer trigger unnecessary hostname claims, listeners, or certificates. - Switching certificate modes now cleans up obsolete ACME resources and route status. - **Documentation** - Updated security and gateway guidance for hostname validation, edge termination, route compatibility, and certificate handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
94a202bc5a |
test(e2e): run the merge-gating lanes on Talos containers (#4020)
Replaces QEMU substrate with Talos containers on both lanes that gate a merge. srv1-srv3 run as containers, so tenant worker sits at L2 instead of L3. Measured in CI on commits where both substrates ran the same tree: | | qemu | container | |---|---|---| | `kubernetes-latest` / `-previous` | 1143-2040s, red on every vgif-absent runner | 466-809s, green in both strata | | whole job | | 24-49 min faster | Almost all of it is in Chainsaw. Install takes about half an hour either way. Node-join soft-red gate goes with it. It was a prosthetic for nested virt: a worker that registers in minutes elsewhere could miss any deadline this test can afford, and #3513 established the discriminator is host `kvm_amd` `vgif`, not anything about the product. Remove the nesting level and there is nothing left to tolerate, so `hack/e2e-node-join-soft-red.sh`, the marker and the `soft_red` outputs are gone from all four workflows. ### What moves off the per-PR path DRBD, and with it `replicated` StorageClass, its `Immediate` binding mode, tenant StorageClass propagation (applies only to remotely-accessible classes), and cozystack talos node image with its extensions. None of it is lost - `nightly.yaml` and `e2e-tag.yaml` still run the suite on QEMU, so all four keep nightly and release-candidate coverage. Live migration is not on that list because nothing tests it today anyway. One more belongs on the list and was missing from it: the nocloud disk build. `build-talos` no longer runs `make -C packages/core/talos talos-nocloud`, so `images/talos/profiles/nocloud.yaml` is compiled by `nightly.yaml` and by the tag build's `make assets` rather than by anything a PR runs, and `promote-rc.yaml` still hard-fails a promotion whose rc release has no `nocloud-amd64.raw.xz`. Nothing in that chain breaks, because the tag build supplies the asset as before. What a PR no longer catches is narrow: it still compiles `installer.yaml` through the same imager, and the two profiles are generated by the same script and differ only in `platform`, `kind`, `imageOptions` and `outFormat`. ### Product fixes Four, all found by measurement while building the lane, each on its own commit: The linstor `drbd.enabled` gate. Without it drbd-logger sidecar exits, satellite is never Ready, piraeus registers zero nodes and every PVC hangs Pending behind a cluster that looks healthy. linstor `--strict-topology`. Without it external-provisioner passes every topology segment as `requisite`, so a node-pinned volume can be provisioned away from its pod and the pod is then unschedulable forever with nothing erroring anywhere. kubevirt-cdi importer resources are configurable now. Default restates CDI's own values exactly so production behaviour does not move, and the lane raises the ceiling through its own Package. vm-disk binds standalone disks immediately. A VMDisk on `local` silently never populated, while the same disk on `replicated` did. ### Where the run stands The lane has gone green twice, most recently on `cc70cef7b` where `E2E (in-tree)` took 2h3m against its 215m cap with 45 Chainsaw tests passing. The measurements quoted elsewhere in this description come from the first of those runs, on a commit that has since been rebased away: the two tenant-Kubernetes suites came in at 569.72s (`kubernetes-latest`) and 470.19s (`kubernetes-previous`) against a 67m operation. Both runs answer the four earlier reds, all of which were the lua-protobuf segfault from #4012 once `vminstance`/`vmdisk` were fixed on the branch. ghcr.io mirror removal is carried here too and is the same change as #4007, whichever lands first makes the other a no-op. ### What review changed `drbd.enabled=false` now refuses to render together with `talos.enabled=false`. It removed the logger sidecar only, while the two DRBD initContainers piraeus contributes unconditionally are deleted by the Talos configuration alone, so that combination produced a satellite that can never reach Ready. `isp-full-generic` sets `talos.enabled=false`, so it was reachable and not theoretical. vm-disk keeps unconditional immediate binding, a standalone disk has no consumer to wait for. What it costs on a node-pinned class is in the chart README now, and the annotation is pinned per source type and on the existing-DataVolume path where `helm upgrade` retrofits it. The rest is the harness holding its own promises: HelmRelease gate reads are bounded and its minimum-count half is pinned by a test that fails without it, two Chainsaw ops sit above the deadlines they contain, the fallback-StorageClass render restores `replicated` when it fails, container lane compose project and pool files are per-checkout, and three comments that said the opposite of the code now say what is true (QEMU is still wired into nightly and e2e-tag, the lane replaces two Packages and not one, the argument-count assertion cannot see upstream drift). ### Release note ```release-note fix(linstor): CSI provisioning is constrained to the node the scheduler chose (`--strict-topology`), and the DRBD sidecar can be switched off with `drbd.enabled` on substrates whose kernel cannot run DRBD. On an existing cluster the new default re-templates `spec.csiController.podTemplate`, so piraeus rolls `linstor-csi-controller` once during the upgrade. `drbd.enabled=false` requires `talos.enabled=true` and the chart now refuses the other combination at render time, because only the Talos satellite configuration removes the DRBD initContainers. fix(kubevirt-cdi): CDI worker pod resources are configurable through `importerResources`, which reaches the importer, the uploader and the host-assisted cloner. The default restates CDI's own built-in values exactly, so an install that sets nothing behaves as before. fix(vm-disk): every disk requests immediate binding, not only `upload` sources, so a standalone disk on a WaitForFirstConsumer StorageClass is populated instead of waiting for a consumer that may never arrive. On a node-pinned class such as `local` the volume then binds where CDI's worker pod was scheduled rather than where the VM will run; see the chart README before choosing such a class for VM disks. The default `replicated` is unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added container-based end-to-end testing with local storage support. * Added optional backup-access preflight checks for database backup examples. * Added configurable CDI worker resources and LINSTOR storage settings. * Added configurable immediate storage binding for VM disks. * Added stronger readiness checks, diagnostics, and parallel test execution. * **Bug Fixes** * Improved cleanup failure reporting, capacity validation, and missing-image detection. * Improved FoundationDB health verification and storage placement behavior. * **Removed** * Removed GHCR mirror support and soft-red node-join tolerance. * Removed standalone OIDC render test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
a115745c69 |
feat(vm-disk): let the operator defer binding to the consumer
Immediate binding for every source fixed a real bug: a standalone disk on a WaitForFirstConsumer class has no consumer to wait for, so it never populated. It also took away the path that is correct for the other case. For a disk a VMInstance consumes, KubeVirt renders a temporary launcher pod carrying the VM's own affinity and nodeSelector, so the volume binds where the VM can actually run; with the annotation always present the CDI worker becomes the first consumer instead and the volume lands where that pod happened to be scheduled. On a node-pinned class the VM is then unschedulable against a node-affinity conflict, and storageClass is immutable, so the only exit is deleting the disk. The two cases want opposite defaults and only the operator knows which one a given disk is. bindImmediately defaults to true, so nothing moves for existing disks or for anyone on `replicated`, which binds Immediate either way. Both branches are pinned in tests, including the existing-DataVolume path, which renders its own annotation block. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
|
5848d85840 |
feat(cozystack-api): add read-only Tap resource for the marketplace view
Add a cluster-scoped, read-only Tap resource to the core.cozystack.io API group that powers the dashboard marketplace view. Each Tap is computed on read from a PackageSource and the ApplicationDefinitions attributable to it (matched by the component's assembled-artifact name), exposing the connected repository, whether it is a community tap, its readiness, and the packages it exposes with their catalog metadata. It is served through the existing aggregation layer, so the dashboard consumes it with the same list/watch client it already uses, and authorization is the delegated RBAC the apiserver already enforces. Taps carry catalog metadata only and never a pull-credential Secret, so browsing cannot leak a private tap's credentials across the tenant boundary. Read access is granted to the tenant catalog-read roles alongside options. Modeling the marketplace as a virtual resource in an existing API group avoids a new API group and needs no etcd or CRD: the object is computed from cluster state via the apiserver privileged dynamic client, mirroring the options resource. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
c693ed36fb |
fix(postgres): force client-side apply so the autoscaler seed survives
Under active read-replica autoscaling the chart seeds spec.instances at a constant max(replicas, effectiveMin) rather than omitting it (an omitted field defaults to 1, which the CNPG quorum webhook rejects, and — under server-side apply — the phase-2 omit prunes the field whenever KEDA has not yet written /scale, collapsing an idle-at-floor cluster to 1 and shedding its replica PVCs). Keeping the constant only works if Flux does not force-own and revert it. The postgres ApplicationDefinition therefore carries release.cozystack.io/helm-server-side-apply: "false", so the tenant HelmRelease applies client-side: helm-controller patches the CNPG Cluster from the previous-vs-new rendered manifest without consulting live state, the unchanged constant yields no patch, and KEDA's higher live /scale count is preserved. The constant also equals the disabled/dryRun/transition render (instances: replicas) whenever replicas sits at/above the floor, so those flips are patch-free too. Correct the merge-semantics comments across db.yaml, _autoscaling.tpl and values.yaml to describe the two-way previous-vs-new patch precisely (not a "three-way no-op"), and document the residual floor-change hazard: editing replicas / minReplicas / quorum.maxSyncReplicas moves the seed, so that one reconcile does patch spec.instances to the new seed. Add autoscaling.serverAddress so a monitoring:true tenant can point the ScaledObject at a vmselect that carries both the CNPG metrics and kube_pod_labels. Regenerated README/schema/types/cozyrds. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
826f08972f |
fix(apps/postgres): make autoscaler fail-safe, dip-free, and observable
Address review findings on the KEDA read-replica autoscaler where the
implementation diverged from the merged design proposal.
Fail-safe read load. Gate the `or vector(0)` floor on the existence of ANY CNPG
instance pod (`count(kube_pod_labels{...instance_role=~".+"}) > 0`) — primary or
replica — so the query is empty ONLY when no CNPG pod labels exist at all (a
broken kube-state-metrics pipeline), not during initial provisioning when the
sole replica-side pod is CNPG's join Job (jobRole, not instanceRole). Also pin
`ignoreNullValues: "false"` on the trigger: KEDA's prometheus scaler defaults it
true, turning an EMPTY result into metric 0 (not an error) — the HPA would then
desire 0 and clamp to the floor, i.e. silently scale a broken-pipeline cluster
DOWN. With it false an empty result is a scaler error, so the HPA HOLDS and
DatabaseAutoscalerScalerErrors fires; gating on any instance role keeps a healthy
cluster (including a long basebackup) from tripping that alert.
Constant seed = max(replicas, effectiveMin), so enable/dryRun/transition flips
are a merge no-op with no dip and never undercut the quorum floor.
Fail closed when the quorum floor exceeds maxReplicas (was: effectiveMax silently
raised both bounds, unbounded for a large maxSyncReplicas).
Dry-run / transition keep the HPA (unidirectional pause pair, not the HPA-deleting
full pause) but both SelectPolicy=Disabled, so desired normalizes to current — the
recommendation is read from the read-load metric, not a diverging desired count.
Observability. DatabaseAutoscalerAtMax keys on desired >= maxReplicas, NOT the
ScalingLimited condition (which is also true at the floor, so it fired on every
idle cluster with wrong "raise maxReplicas" advice). Dashboard read-load panels
are per-cluster (label_cnpg_io_cluster) with a CPU panel for ReadCPUUtilization
and the exact clamp in descriptions; ScaleStuck's Pending intersection is scoped
to CNPG pods; alerts.yaml narrative comments moved to Go-template comments so they
do not ship into the PrometheusRule.
Guard the seeding invariant: TestBuildHelmReleaseSpecNoForceOrDriftDetection.
Regenerated README/schema/types.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
|
6e3da63586 |
fix(postgres): guard the disable-wedge, role-scope the freeze count, and harden edges
Address the remaining IvanHunters review points on #3954: - [MAJOR] Disabling autoscaling wedged the HelmRelease under synchronous quorum. An autoscaled + maxSyncReplicas>=1 cluster runs with instances seeded from effectiveMin while replicas stays at its default, so flipping autoscaling off rendered instances: <replicas> next to the quorum and CNPG's webhook rejected it ("maxSyncReplicas must be lower than the number of instances"). Add a symmetric render-time guard on the static-count path (covers off/dryRun/ transition): fail with an actionable message when replicas <= maxSyncReplicas. - [MINOR] Role-scope the freeze brake's currentInstances count (label_cnpg_io_instance_role=~".+") so lingering CNPG join/bootstrap Job pods (cnpg.io/cluster but jobRole, not instanceRole) don't inflate it and nudge desired up while the brake is meant to hold it. - [MINOR] Default maxReplicationLagSeconds to 0 (brake opt-in): only the non-braking path is validated live, so ship the validated default and let the freeze be enabled explicitly until its branch is exercised under load. - [MINOR] Document the floor-change and non-quorum-disable dips (same three-way- merge hazard) and the pin-replicas-first mitigation. - [NIT] Coerce the cozy-lib helper's `paused` (eq "true") so a caller passing the string "false" does not pause the ScaledObject. - [NIT] Trim the keda.clusterDomain injection comment so it no longer ships ~8 verbatim lines into the cozystack-values Secret on every cluster. Regenerate types/schema/README/postgres-rd. New tests cover the disable-wedge guard, the role-scoped freeze count, and the string-"false" paused coercion. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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. ``` |
||
|
|
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 ``` |