mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
35e54fb49e986b41cfbd813b658f7fceb07dc6eb
238
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
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> |
||
|
Aleksei Sviridkin
|
3a369fed99 |
feat(redis): add opt-in TLS for Redis and Sentinel (#2729)
## What this PR does `tls.enabled` puts Redis and Sentinel on a TLS-only listener with a per-release cert-manager chain, and hands the tenant the CA certificate as `redis-<name>.tenant-ca`. Off unless set. redis-operator v3.3.5 has no TLS support. The work is ported upstream in freshworks-oss/redis-operator#85, which is open and not in a release, so this carries it as three patches over the v3.3.5 tarball. `images/redis-operator/patches/tls.diff` is the source half, cut from that PR's commit `2ad2ed8d1`, which the patch header pins; it is applied by the glob the image Dockerfile already runs, after the Dockerfile strips the tarball's `manifests`, `charts`, `example` and `.github`, so the image builds from source alone and the upstream tests that read the CRD YAML skip instead of failing against the stale v3.3.5 copy. `patches/crd-tls.diff` adds `spec.tls` to the vendored CRD, and with it the CEL rule that fixes `tls.enabled` at creation; without the schema block the apiserver prunes the field on admission and the patched binary waits for something it never sees, and the rule cannot exist without the block it references. `patches/rbac-tls.diff` grants Create, Get and Update on Secrets, so the operator can publish the CA-only Secret it addresses by name. That is the whole delta, and it is narrower than the ported operator's own chart asks for: those three are the only Secret calls in the patched source, and cert-manager Certificates get no grant at all, because the operator only touches one under `spec.tls.certManager` and this chart pins `certificateSecret` mode instead. One change in `tls.diff` is not TLS and takes effect with TLS off, named in the patch header: `MakeSlaveOfWithPort` connects to the replica on its own port rather than on the master's. It changes a dial, not a pod template, so shipping the rebuilt image rolls nothing; the plaintext pod templates stay byte-identical to v3.3.5. The image build registration this branch used to carry (`make -C packages/system/redis-operator image` in the root `build:` target) landed on main with the Valkey service (#3380), so the rebuilt operator image is produced by CI from main as well; the digest in `values.yaml` is still refreshed by release preparation, not here. The chart patches re-apply at the end of `update:`, the same way reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji carry theirs. Otherwise `rm -rf charts` takes TLS back out on the next `make update` and nothing fails. `make update` reproduces the committed tree byte for byte. TLS is chosen when the instance is created. The config gets `port 0` and the listener moves to `tls-port`, so TLS replaces plaintext rather than running beside it, and a running instance cannot change mode: a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The RedisFailover schema in the carried CRD patch rejects a change of `tls.enabled`, which on an existing instance surfaces as a failed HelmRelease upgrade while the instance keeps serving what it did; the README points at creating a new instance and migrating. That is also why TLS is not inferred from `external`: an externally reachable instance is the one most likely to have clients an upgrade must not drop. Existing releases keep their plaintext listener and are not restarted by the rebuilt operator image. The tenant gets `ca.crt` and nothing else, through `tenantsecrets`. The chart's own Secrets take a dot between the release and the suffix (`<release>.ca-tls`, `<release>.tls`, `<release>.ca-cert`): the suffixes are dot-free, so the split is unique and release `foo`'s CA Secret can no longer be release `foo-ca`'s leaf, the collision the hyphenated form had. The chart renders a `TenantProjection` pointing at `<release>.ca-tls`, and the CA-extraction controller re-encodes the one named key from the parsed certificate, so nothing that is not a certificate gets through. Same path as postgres, mongodb, nats and qdrant. No Secret of this release is granted to the tenant by name beyond the generated password, and that includes `<release>.ca-cert`, the CA-only copy the operator publishes for itself. It holds no key today, but the operator has Create and Update on Secrets cluster-wide, so what sits at that name is the operator's call rather than the platform's, and RBAC cannot filter by key. `hack/check-redis-rd-secrets.bats` pins the whole rendered Secret surface instead of a list of known-bad names, because an include with no `resourceNames` and no `matchLabels` restricts nothing. The chainsaw suite for redis gains two cases. A TLS fixture asserts the cert-manager chain is issued, the TLS-probed Redis replicas and Sentinel come up, the operator publishes its CA-only Secret under the widened ClusterRole, the tenant receives exactly `ca.crt` through `tenantsecrets` while the key-bearing Secrets stay off that path, and a `redis-cli --tls` PING through the service name completes with the same CA. A second case applies `tls.enabled` to a running plaintext release and asserts the refusal: the HelmRelease upgrade fails with the schema's message, the StatefulSet keeps its revision with both replicas Ready, plaintext still answers and no TLS listener appears. Certificate renewal rolls the pods from inside the operator. Redis and Sentinel read the certificate once at startup, so the patched operator stamps a hash of the TLS Secret's `tls.crt` and `ca.crt` into the pod template of both the Redis StatefulSet and the Sentinel Deployment on every reconcile, as `redis-failover.freshworks.com/tls-secret-hash`. A renewal changes the hash; the operator's own roller replaces the Redis pods one at a time and the Deployment controller rolls Sentinel. README names the fallback: if the roll cannot run, the deadline is the old certificate's expiry, past which the operator's own TLS connections fail along with the clients. `tls.authClients` maps to `tls-auth-clients` and defaults to `no`. The platform mints no client certificate for the tenant and the CA key is out of reach, so `yes` is only usable when the client certificate comes from whoever also controls the CA. Changing the value on a running release restarts the Redis and Sentinel pods: Redis reads the directive from its config file only at startup, so the operator folds the effective value into the certificate hash that drives the rollout, and the change lands when it is made rather than at the next unrelated restart. Follow-ups in the carried operator patch go upstream first, to freshworks-oss/redis-operator#85 or a successor there, and are picked up here afterwards; the carried patch does not grow Go that upstream has not seen. `tlsConfigFor` fetches the Secret on every call, tens of GETs per reconcile. The renewal roll is static analysis so far: the live check, `cmctl renew` on a running release while watching the `rfr-*` and `rfs-*` pods roll, is still owed and belongs with the e2e follow-up. Two facts about the shipped image rather than the patch. `packages/system/redis-operator/values.yaml` keeps the digest of the image built before the patch until release preparation rebuilds it, so on `main` between this merge and the next release-prep, `tls.enabled` renders the chain and `spec.tls` while the running operator ignores them and serves plaintext. And the pinned v3.3.5 tarball carries GO-2026-5026 in `golang.org/x/net` v0.53.0, reached through `PodService.UpdatePodLabels`; the port neither introduces nor fixes it, upstream `master` is already on v0.55.0, so it lands with the next tag bump rather than here. Outside the patch: `cozyvalues-gen` renders `{}` in the README Value column for `tls.authClients`: a child property without a default inherits the parent object's `{}`, and `make generate` reproduces it byte for byte, so it is a generator bug rather than something to hand-edit here. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#37 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: I walked the trigger map against the diff file by file. terraform-provider-cozystack is reached, tracked as cozystack/terraform-provider-cozystack#37 rather than a speculative PR: `values.schema.json` gains a `tls` object, and the provider hand-writes a schema, model and expand/flatten pair per Kind. It already models `tls` for postgres, kafka, nats and qdrant, not for redis. Redis needs different wording from those four, whose descriptions say to omit the block to follow `external`, which is the one thing redis does not do. Nothing else is reached. The website regenerates this package's reference page from `README.md` on a stable tag, so the README changes need no PR there. No package added, renamed or removed. `packages/core/platform/values.yaml`, variant and bundle composition and release asset names are untouched, and so are `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh`, which is what ccp and external-apps-example read. The two new files under `hack/` are bats unit tests. No node prerequisite, installer value, telemetry metric or cozy-proxy label changes. ### Release note ```release-note feat(redis): add opt-in TLS for Redis and Sentinel. Set `tls.enabled: true` to serve both on a TLS-only listener backed by a per-release cert-manager chain; the tenant reads the CA certificate as the `<release>.tenant-ca` tenant secret. TLS is off unless set and is not inferred from `external`, so existing releases keep serving plaintext and are not restarted by the rebuilt operator image. TLS is chosen at creation: a running instance cannot switch modes, the change is rejected and surfaces as a failed HelmRelease upgrade, so add TLS by creating a new instance and migrating to it. Requires cert-manager, which the platform's `system` bundle installs on every variant. A renewed certificate rolls the Redis and Sentinel pods through the operator. TLS support is carried as a downstream patch over redis-operator v3.3.5 until freshworks-oss/redis-operator#85 lands in a release. ``` |
||
|
Aleksei Sviridkin
|
d800526696 |
feat(mariadb): add TLS support via cert-manager (#2680)
## What this PR does Gives MariaDB a chart-managed certificate chain, makes TLS enforcement explicit, and hands the tenant a key-free trust anchor instead of a private key. - `tls.issuer` selects who issues the server certificate: `operator` (the CA mariadb-operator manages itself, the default and today's behaviour) or `cert-manager` (the instance gets its own CA, and this is what covers the external hostname). The field is named for the issuer rather than for on/off because TLS is served under both — unlike the similarly spelled field in some other charts, which really is a switch. - `tls.required` refuses plaintext connections. It is opt-in and defaults to false: enforcing TLS before the tenant can obtain the CA would be backwards, and a platform upgrade must not sever connections that work today. - Neither issuer is inferred from `external`. Existing instances keep the CA they have and change nothing on upgrade. - The tenant no longer receives `<release>-ca-tls` or `<release>-tls` — the CA private key and the server private key. It receives the operator's `ca.crt`-only bundle, which works today, and the ResourceDefinition additionally declares that bundle as the source for the CA-extraction controller. ### Fixes along the way - The TLS backup path could never have worked: it passed `--ssl-mode=VERIFY_IDENTITY`, a MySQL-8 flag the MariaDB client rejects during argument parsing. Replaced with `--ssl-ca` + `--ssl-verify-server-cert`, and the test now asserts the dialect rather than the presence of a substring. - The leaf certificate carried `clientAuth`. Because `REQUIRE X509` accepts any certificate signed by a trusted CA, the server's own key was usable as a client credential. Removed. - The CA and server private keys survived deletion of the instance: cert-manager runs with `enableCertificateOwnerRef: false`, so removing the `Certificate` left the Secrets behind. Both are now cleaned up by the existing post-delete hook. - The Secrets are labelled for the operator's watch predicate, without which a renewed leaf would not reach the pods until a full resync. ### Why the include carries an exclude list The trust-anchor selector matches by label with no name constraint, so the enumerable dangerous set is excluded explicitly — both issuers' private keys, the internal credentials, the backup keys, and the metrics configuration, which the operator writes unconditionally with credentials in plain text for a user holding `SELECT` on everything. The backstop is bounded and the comment says so: matching is by exact name with no globs, so a Secret introduced by a future operator version, or a transient object created mid-renewal, stays in scope of the label-only include. The primary defence remains that the controller stamps only its own projection. ## Known limitations 1. **Merge is blocked on #3299 — and the blocking is mechanical, not just procedural.** The CRD on `main` does not carry `spec.caCert`, and under Flux's Server-Side Apply the API server rejects a field the schema does not declare (`field not declared in schema`); under the older client-side apply the same field was silently dropped, so installing this branch without #3299 fails outright. An earlier revision of this description claimed the field would be silently pruned; that was wrong. The tenant still gets a usable trust anchor once merged in order, through the operator's bundle. 2. **The deprecated backup CronJob cannot start at all**, for reasons that predate this branch: its `secretKeyRef` names a Secret and key the chart never renders. That is filed separately as #3347 and deliberately not fixed here, since repairing it would start scheduled dumps on production instances from a TLS change. The tests that pin the backup TLS flags therefore pin a path that does not currently execute — the suite header says so. 3. **The operator's `clusterName` is hardcoded**, so on a cluster with a non-default domain the SANs and the name the operator dials diverge. Pre-existing, raised separately. 4. **No e2e coverage for the managed path.** Unit coverage pins the rendered chain, the SAN set, the exclusion set and the guards; the end-to-end handshake is not asserted in CI. ### On review coverage Two independent review passes were run repeatedly against this branch — seven rounds, converging from a security finding, through comment accuracy, to guard strength, and finally to nothing new. A third, model-based pass reviewed the branch through `6cb1bd236` and its findings were fixed; it could not be run against the final revision because of an external account quota lasting until 25 July. The delta since that revision is substantial and includes runtime code, not only tests and docs. Recording this so the coverage is not overstated: if the quota returns before this is ready to merge, that pass is worth running again. ### Downstream repositories `packages/apps/mariadb/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_mariadb`; tracked in cozystack/terraform-provider-cozystack#40. ### Release note ```release-note feat(mariadb): chart-managed TLS via cert-manager, opt-in enforcement, and the CA published to tenants as a key-free trust anchor ``` |
||
|
Aleksei Sviridkin
|
487cafac3e |
feat(platform): let a tenant pick its Gateway class from an allowlist
Tenant Gateways were pinned to the bundled Cilium class. Give the platform a default in gateway.className, the set a tenant may choose from in gateway.tenantSelectableClasses, and the set whose provider terminates TLS upstream of the Gateway in gateway.edgeTerminatedClasses. A tenant picks its own with tenant.spec.gatewayClass, read only when it owns a Gateway; an inheriting tenant runs on its ancestor's. The selectable set is an allowlist rather than free choice because Tenant is a tenant-writable object while a GatewayClass is cluster-scoped: without it a tenant could attach its Gateway to a class an admin keeps for internal-only traffic. Naming anything outside the set fails that tenant's own gateway release and reaches no other tenant. Where TLS ends is a property of the class, so the gateway chart derives certMode from whichever class the tenant landed on instead of taking it as a separate input. The two therefore cannot be configured out of step. The publishing tenant cannot cross that line by itself. The cluster-wide ACME issuers point their HTTP-01 solver at its Gateway and are rendered from the platform default alone, so a mismatch would pin the solver to a listener that Gateway does not have. Those issuers now drop the sectionName pin when the default class is edge-terminated, letting the challenge attach by hostname instead. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
2ef0e8d3a1 |
feat(gateway): add an edge cert mode for upstream-terminated classes
Some GatewayClasses have their provider terminate TLS in front of the Gateway and reach it over plain HTTP. On such a class the platform issued origin certificates nobody serves, and where ACME could not complete, the Gateway's listeners never became ready. Add certMode=edge to TenantGateway. The controller renders the apex, its wildcard and every inheriting child apex as plain HTTP listeners carrying the same label-based allowedRoutes as the HTTPS listeners of the other modes, so application HTTPRoutes attach by hostname unchanged. It mints no Issuer and no Certificate, and removes the http-to-https redirect route, because the edge performs that redirect itself. TLSPassthroughServices are not rendered: an HTTP-only edge cannot carry a TLS listener, which leaves the endpoints relying on one unpublished through such a Gateway. The hostname-less port-80 listener is left out. It carries the ACME challenge and the redirect in the other modes and has neither here, so it would only be a catch-all admitting any hostname. Leaving it out also keeps a hostname-pinned listener at index 0, which is what Cilium's first-listener-wins namespace check reads before the fix in cilium/cilium#45693. Moving into the mode garbage-collects the Issuer and Certificate objects the controller owned. The TLS Secrets they issued stay behind: cert-manager runs with enableCertificateOwnerRef false, so a Certificate does not own the Secret it issued into. Every mode transition already behaves that way. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Andrey Kolkov
|
e94fcf8219 |
feat(backups): RabbitMQ definitions backup via a dedicated strategy driver (#3990)
## What this PR does Adds backup/restore support for the managed `RabbitMQ` application. RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no consistent online export of message payloads, so this backs up broker **definitions** (vhosts, users, permissions, queues, exchanges, bindings, policies, parameters) over the management HTTP API (`GET`/`POST /api/definitions`) and stores them in the system bucket. Message payloads and point-in-time recovery are intentionally out of scope; a full-data backup would be a Velero volume snapshot, which is a separate strategy. It is delivered as a **first-class `strategy.backups.cozystack.io/Rabbitmq` driver**, following the Altinity pattern (a dedicated driver that renders a one-shot Job for an HTTP-API engine): - `reconcileRabbitmq` / `reconcileRabbitmqRestore` render the strategy's `PodTemplateSpec` — `curl` `GET`/`POST /api/definitions`, pushing/pulling the JSON to S3 with `curl --aws-sigv4` — as a `batch/v1.Job`, covering both restore flows: in-place (into the source broker) and to-copy (into a separate, differently-named RabbitMQ in the same namespace, authenticating with the target's own default-user credentials and reading the source's object key). - A **Go-read precondition** gates each run on the `RabbitmqCluster` reporting `AllReplicasReady` (read through a small `rabbitmqtypes` partial type), so a not-yet-ready broker surfaces a precise `Ready=False` wait on the BackupJob/RestoreJob rather than a Job that starts and fails against an unreachable management API. - The produced `Backup` records the stored object's location on `status.artifact.uri`, rendered from the strategy's `spec.artifactURITemplate`. The `Backup` CRD has no status subresource, so this persists on Create, mirroring the Etcd driver. Dispatch is wired in both controllers (plus `supportedBackupStrategyKinds`), `rabbitmqtypes` is registered in the scheme, and RBAC grants read on `rabbitmqclusters.rabbitmq.com` (the Job itself is already covered by the existing `batch/jobs` grant). It ships `cozy-default-rabbitmq` (a `$bucketName`-gated `Rabbitmq` strategy) and routes `apps.cozystack.io/RabbitMQ` to it in the `cozy-default` BackupClass, which also enrols it in the `DefaultObjectsGate`. The RabbitMQ application chart is left untouched — it had no prior backup wiring, and the strategy carries the S3 coordinates itself. Unit tests cover the driver's pure helpers (applicationRef validation, cluster-name derivation, the readiness-message builder, artifact-URI rendering, parameter round-trip) and the reconcile precondition gate (defers with `Ready=False` and no Job until the cluster is ready, then proceeds). A runnable example under `examples/backups/rabbitmq/` and an honest-roundtrip Chainsaw e2e under `hack/e2e-chainsaw/rabbitmq/` seed a sentinel vhost, back it up, drop it, and assert it round-trips through object storage on both restore flows. The Job pod reuses the existing `platform-migrations` image (curl + ca-certificates) under a new `backupStrategyController.rabbitmqBackupClientImage` key, stamped in lockstep with `chBackupClientImage` in `packages/core/platform/Makefile` so the repository stays pinned at a single digest (`hack/image-pin-consistency.bats`). A dedicated slim image built by this package is a follow-up. **Validation:** `go build`/`go vet`/`gofmt`/`go test ./internal/backupcontroller/... ./internal/template/...`, deepcopy + Rabbitmq CRD generation, `helm template` proving the CRD-ordering gate both on and off, `helm lint`, and `helm unittest` (18/18). The backup + in-place + to-copy roundtrip was verified end-to-end on a dev cluster (a real management-API export and a real S3 `--aws-sigv4` upload/download against SeaweedFS), so the driver's core contract is exercised on a real cluster; the in-CI Chainsaw run is the remaining automated evidence. The object-cleanup, terminating-namespace teardown and readiness-deadline paths added during review are unit-tested (including `Reconcile`-level finalizer retention and the terminating-namespace release) but not yet cluster-exercised. (Note found while testing: RabbitMQ 4.2 is OOMKilled on the `nano` preset, so the example/e2e use `c1.small`.) ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff. This is an additive backup strategy plus a new strategy CRD; no downstream repository is mechanically forced by the change. A documentation page for RabbitMQ backup on [cozystack/website](https://github.com/cozystack/website) would be a reasonable follow-up, but it is not required by this diff — leaving the boxes unticked for a human to decide rather than ticking "no downstream" to clear the checklist. ### Release note ```release-note feat(backups): add RabbitMQ definitions backup/restore via a dedicated strategy.backups.cozystack.io/Rabbitmq driver; it gates each run on the RabbitmqCluster being ready and records the stored object on Backup.status.artifact.uri. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added RabbitMQ backup and restore support for definitions, including vhosts, users, queues, exchanges, bindings, policies, and related settings. * Added in-place and restore-to-copy workflows with readiness checks. * Added scheduled and on-demand backup examples. * Integrated RabbitMQ backups with the default backup configuration. * Backups store definitions only; message payloads and point-in-time recovery are not included. * **Documentation** * Added a complete RabbitMQ backup and restore demonstration guide. * **Tests** * Added end-to-end coverage for readiness and backup/restore round trips. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
5a9c3dd79f |
feat(mariadb): name the TLS field for the issuer instead of for on/off
tls.enabled read as a switch and was not one — TLS is served under every value, and what the field actually chose was which authority issues the server certificate. The same key means on/off in other charts here, so a tenant carrying that habit across would have read this exactly backwards. Replace it with tls.issuer: operator | cert-manager, defaulting to operator. The value now states the behaviour instead of needing prose to undo the name: operator is what runs today, so the fact that nothing changes on upgrade is visible in the default rather than argued in a comment. Being an enum, a typo is rejected by the schema, which a bool could not do. The explanation the old name required is gone with it — the helper docblock and the CR comment lose the paragraphs that existed only to say this was not a switch, and the README states the split in a table. Not marked breaking: the field is introduced by this series and does not exist on the target branch, so there is no released spelling to migrate from. Breaking is measured against what is merged, not against the intermediate history of the branch. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2e4ed43b88 |
docs(mariadb): shorten the TLS field descriptions to fit where they render
These strings flow into the ApplicationDefinition schema and become the help text beside the fields in the console. They had grown into paragraphs of rationale — 704 and 613 characters against 48 for the neighbouring external — which is the wrong length for a form label and duplicated what the README already explains at the right length. Say what the field does and that it is opt-in; the reasoning stays in the README, which is where a reader goes for it. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d2d85cb634 |
fix(mariadb): let a missing CA bundle fail the backup instead of stopping it
The bundle volume was a required mount. The operator reconciles it only after certificates settle, so on first install with managed TLS there is a window where it does not exist, and a required mount leaves the pod in ContainerCreating rather than failing. With concurrencyPolicy: Forbid that one stuck pod blocks every subsequent backup for as long as it hangs. Marking it optional turns that into a run that fails with a message naming the missing file, retries, and lets the schedule proceed. Correct the reason given for enforcement defaulting to false while here. It said the tenant had nothing to verify the server against, but this same change grants the tenant read access to the bundle, so the anchor is reachable. The default is still right — enforcement refuses clients that have not moved yet, which is a decision to make rather than something an upgrade does — but that is the reason, and it was stated in five places, all fed from the values annotations. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e1cc5e5284 |
fix(mariadb): make managed TLS opt-in instead of deriving it from external
tls.enabled defaulted to the value of external, so every existing external instance would have moved from the operator's CA to a chart-issued one on the next platform upgrade, without its owner doing anything. Clients pinning the operator's ca.crt fail chain validation the moment the new certificate is served, and external instances are exactly the population where a copied ca.crt is most likely. The usual argument for applying something on upgrade does not hold here: taking over the CA does not raise the security floor. These instances already serve TLS under the operator's CA, and what changes is which authority issues it — an architectural move toward the platform trust contract, not protection against a threat. There is no security gain to weigh the breakage against, so it becomes a choice. Rendered for an existing external instance, the result is now identical to what the operator already applies on its own: TLS on, enforcement off, no chart-issued certificates. Nothing happens on upgrade. The default flips as an announced change together with the published trust anchor, the same way tls.required does. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
18215895fd |
fix(mariadb): apply TLS enforcement regardless of who owns the CA
tls.required was only emitted when the chart managed the certificates, so setting it on an instance using the operator's own CA was accepted by the schema and then dropped on the way to the CR — no enforcement, no error, no warning. For a setting whose entire purpose is refusing unprotected connections, silently ignoring it is the worst available outcome. The coupling was artificial: in the operator API required sits beside enabled with no dependency on either certificate reference, and enforcement works against operator-issued certificates just as well. Emit it in every configuration and let it mean what it says. Also loosen the two negative guards in the ApplicationDefinition check, which required a literal closing quote and exact brace spacing and so would have missed an unquoted key-bearing source. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d9c4d614c5 |
fix(mariadb): make TLS enforcement opt-in
Enforcement has to follow the trust anchor, not lead it. The platform does not yet publish a key-free CA to the tenant, so requiring TLS today tells clients they must encrypt while giving them nothing to verify the server against — protection on paper, unverified connections in practice. It also cannot be opted out of in time. Enforcement would arrive with a platform upgrade rather than a user action, and the escape hatch lives in a field this change is the first to introduce, so an operator has no way to set it beforehand. The only available order was upgrade, break, then repair. Default required to false and let enforcement be chosen. Managed TLS still issues the per-instance CA and certificate, so the path to verified connections is in place; what is deferred is refusing the connections that predate it. The default flips to true as an announced change once the published anchor is universal, and the field descriptions say so rather than leaving it implied. This does not change any running instance: enforcement was never applied to one. The chart previously sent no enforcement at all unless it managed the certificates, and managing them is itself opt-in, so an existing instance keeps the operator's unenforced default either way. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4339eb4631 |
test(mariadb): pin the ApplicationDefinition trust-anchor invariants
The ApplicationDefinition is where a mistake decides whether a private key can reach a tenant, and it is the one file in this change the chart test suite cannot reach: helm-unittest only covers rendered templates, and the *-rd packages expose no test target. Add a shell check in the style of the existing kafka one, which the Makefile discovers automatically. It pins that the CA source is the operator's key-free bundle rather than either key-bearing Secret, that the tenant selector is the specific label rather than an empty selector that would match every Secret in the namespace, and that no key-bearing Secret is listed by name. Type required as a pointer while here. As a plain bool it rendered a default of false in the generated documentation while the description promised true, and the zero value could not represent an explicit false for consumers going through the generated struct — which is the value the documented migration path depends on. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c6678ea053 |
fix(mariadb): stop advertising enforcement and trust the tenant cannot reach
Three corrections to what the rendered objects claim. The schema defaulted tls.required to true unconditionally, so every CR carried required: true even where the chart renders no enforcement at all. The operator reads an absent required as not-enforced, so anyone reading the CR was told plaintext is refused on an instance that accepts it. Drop the schema default and let the template supply the managed default, which keeps enforcement on for managed instances and silent for the rest. The tenant grant for the CA bundle was gated on TLS being managed, but the operator reconciles that bundle whenever TLS is on, which for this chart is always. Ungate it: the default instance serves TLS, so leaving the tenant unable to fetch the CA meant it could not verify the server it was talking to. Reword the claim that TLS cannot be switched off. The operator does honour an explicit disable; what is true is narrower, that omitting the field is not how you select it and this chart always asks for TLS. The text is what a reader relies on when deciding whether a connection is protected, so it has to describe the chart rather than invent an operator invariant. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6c1bcb4ff8 |
feat(mariadb): declare the trust anchor and stop claiming TLS can be off
Two corrections to what the chart tells the platform and the user. The tls values never controlled whether TLS runs. The operator substitutes an enabled TLS spec whenever the field is absent, so every instance has always served TLS, and a chart that omits the block only hides that. Render it explicitly and let the values select what they actually select: who owns the CA, and whether TLS is enforced. Rendering the resolved value instead would be the one change that genuinely turns TLS off on instances that have it today, which is not something to ship silently. The field descriptions said the opposite and are corrected with it. Publication of the trust anchor moves to a declaration on the ApplicationDefinition, naming the operator's <release>-ca-bundle. That Secret holds ca.crt alone and carries the previous CA across a rollover, so the publication path never runs next to a private key at all — better than labelling the chart's own CA Secret, which does hold one. The labels on that Secret are dropped rather than left as a second, redundant path. The tenant keeps direct read access to the bundle so the feature is usable before the extraction controller exists; that grant is key-free and comes out once the published anchor is universal. Also set a commonName on the leaf, which cert-manager otherwise leaves empty, breaking clients that verify the hostname against the subject rather than the SAN list. The CA for external instances changes hands here, from the operator's to one issued for the instance. A client verifying against <release>-ca-bundle follows that automatically, because the bundle carries both CAs; a client that pinned a copied ca.crt has to re-read it. Enforcement is a separate, opt-in setting and is not changed by this commit. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Arsolitt
|
924e856878 |
fix(mariadb): move version to its own section and expose tls.required default
Add a @section Version parameters marker in values.yaml so that the version parameter is grouped separately from TLS parameters in the generated README and schema. Explicitly set tls.required: true in values.yaml so UI clients and schema consumers see the correct default (true, matching the template behaviour documented in the description field). Also add [\"spec\",\"tls\",\"enabled\"] and [\"spec\",\"tls\",\"required\"] to keysOrder in mariadb-rd so TLS child fields render in a stable order in the dashboard. Regenerate codegen artifacts accordingly. Assisted-by: LLM Signed-off-by: Arsolitt <arsolitt@gmail.com> |
||
|
Arsolitt
|
c1b40d39cf |
chore(mariadb): regenerate codegen artifacts
Run root and per-package make generate to bring deepcopy, types, README, and CRD descriptor in sync with values.yaml annotations after the TLS work landed. Signed-off-by: Arsolitt <arsolitt@gmail.com> |
||
|
Arsolitt
|
9ae332cf29 |
feat(mariadb): add TLS schema and generated artifacts
- Add tls.enabled (*bool) and tls.required fields to API types and values - Update JSON schema with nullable tls.enabled tri-state and required flag - Extend Makefile with test target wiring helm-unittest into CI - Update cozyrds descriptor to expose new TLS fields - Update README to document tls.required semantics and nullable type Signed-off-by: Arsolitt <arsolitt@gmail.com> |
||
|
scooby87
|
3a70f91423 |
fix(vm-disk): fail on storageClass/source edit of existing disk (#3969)
## What this PR does Fixes #2985. Editing a `VMDisk`'s `storageClass` or `source` after its DataVolume already exists was silently ignored: `packages/apps/vm-disk/templates/dv.yaml` looks up the existing DataVolume and, when found, reused its `spec` verbatim (because `DataVolume.spec` is immutable in CDI). The edit therefore rendered as a no-op — the typed CR accepted it and the HelmRelease reported success, while the DataVolume/PVC kept the original spec and the source of truth silently diverged from the cluster. The template now builds the desired `source` into a shared `$desiredSource` variable (used both for rendering and comparison) and, when a DataVolume already exists, calls `fail()` with an explicit message if the requested `storageClass` or `source` differs from the immutable existing spec — mirroring the explicit size handling already done in `pvc-resize-hook.yaml`. Storage growth is unaffected and still flows through the resize hook. `source` is also marked `@immutable` in `values.yaml`, so the generated schema now carries the same `x-kubernetes-validations` transition rule (`self == oldSelf`) that `storageClass` already had; `types.go`, `values.schema.json` and the generated `vm-disk-rd` CRD are regenerated to match. ### Verification - `helm unittest` covers both the immutability guard (via a mocked existing DataVolume through `kubernetesProvider`, since `lookup` needs a cluster) and initial-creation source rendering for every source type — 12/12 pass. - Verified live on a dev cluster with `helm install --dry-run=server` against a real DataVolume: an unchanged edit renders, a `storageClass` change fails, a `source` change fails, and a storage grow still renders the resize hook without tripping the guard. ### Scope note This PR only makes the drift visible and marks `source` immutable. Two deeper platform findings sit outside its scope: (1) #2657 (pre-existing) — the `apps.cozystack.io` aggregated apiserver does not evaluate the `x-kubernetes-validations` CEL rules on update, so `@immutable` is a no-op at admission for every Application kind, which is exactly why the chart-level `fail()` — not the CEL rule — is the substantive guard here; (2) #3971 — that same apiserver does not honour `--dry-run=server` and persists the change (surfaced while testing this fix). ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff file by file. The diff touches only `packages/apps/vm-disk/` (template, `values.yaml`, regenerated `values.schema.json`, tests), the regenerated `packages/system/vm-disk-rd/cozyrds/vm-disk.yaml`, and `api/apps/v1alpha1/vmdisk/types.go`. No package was added, renamed or removed; no `values.schema.json` field was added, renamed or removed (only a validation rule on the existing `source` field); no default changed; `README.md` is unchanged; the `cozyrds` file was regenerated, not renamed. The terraform provider models fields, defaults and enums but not CEL immutability, so it is unaffected. No downstream repository is reached by this change. - [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(vm-disk): editing storageClass or source on an existing VMDisk now fails with a clear message instead of being silently ignored; delete and recreate the disk to change these immutable fields. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for blank, HTTP, image, disk, and upload VM disk sources. * VM disk sources and storage classes are now protected from unintended changes, while storage expansion remains supported. * Existing disks preserve their configured source and storage settings. * **Bug Fixes** * Invalid or conflicting source configurations are rejected. * Prevented silent no-op updates and incorrect immutability errors for existing disks. * **Documentation** * Documented source and storage class immutability, including the need to recreate disks when changing either setting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9ef1f28855 |
build(redis-operator): carry TLS support as a downstream patch
freshworks-oss/redis-operator v3.3.5 has no TLS support. The operator work that packages/apps/redis renders against was ported upstream as freshworks-oss/redis-operator#85 and is not in a release yet, so carry it here over the v3.3.5 tarball until one lands. Three pieces, and TLS is silently off unless all three are present: - images/redis-operator/patches/tls.diff is the source half. The Dockerfile already applies patches/*.diff, and the shell expands that to labels.diff then tls.diff; they touch disjoint files, so the order does not matter, but it is alphabetically stable either way. - patches/crd-tls.diff teaches the vendored CRD about spec.tls. Without it the apiserver prunes spec.tls from every RedisFailover the redis chart renders and the patched binary waits for a field it never sees. - patches/rbac-tls.diff grants Create, Get and Update on Secrets, so the operator can publish the CA-only Secret named by spec.tls.caCertSecretName. Those are the only Secret calls in the patched source: upstream v3.3.5 only ever reads one, and the patch adds Create and Update on a Secret it addresses by name, so nothing lists, watches, patches or deletes. tls.diff is cut from that PR's commit 2ad2ed8d1 rebased onto v3.3.5, and its header pins that SHA so the next refresh can be diffed against it. Beyond the TLS listener it carries the renewal roll: on every reconcile the operator stamps a hash of the TLS Secret's tls.crt and ca.crt into the pod template of the Redis StatefulSet and the Sentinel Deployment, so a renewed certificate rolls Redis through the operator's own roller and Sentinel through its Deployment. The effective spec.tls.authClients value is folded into the same hash, so changing it rolls the pods the same way instead of landing at the next unrelated restart, and the operator presents its client certificate whenever the Secret carries one, independent of that field, so neither direction of the change locks it out of the instance it reconciles. It also mounts the TLS volume 0440 when the pod declares an fsGroup, rejects a caCertSecretName that names the TLS Secret itself, and waits for the TLS material before it writes the ConfigMaps and the workloads, so a first TLS install starts its pods once. tls.enabled is fixed at creation. Redis replication and Sentinel cannot straddle plaintext and TLS: a replica restarted onto the other protocol cannot sync from the master, and Sentinels restarted onto it fail over to such a replica while the master still takes writes. The rule that refuses the change is a CEL transition rule on the RedisFailover spec; it lives in the CRD, so it ships here through patches/crd-tls.diff together with the schema block it references. The cut carries source only, and the Dockerfile strips the tarball's manifests/, charts/, example/ and .github/ before applying the patches, so the image builds from source alone and the two upstream tests that read the CRD YAML skip instead of failing against v3.3.5's stale copy; the CRD half is patches/crd-tls.diff. cert-manager Certificates get no grant. The patched operator manages a Certificate only under spec.tls.certManager, and packages/apps/redis pins the other mode: it always supplies spec.tls.certificateSecret, so that code path is unreachable as the platform ships it. A comment at the rule site records the decision and names the verbs the rule would need, so a re-vendor review reads the absence as deliberate. The chart patches go through the mechanism the packages that customise a vendored chart already use — reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji all re-apply a patches/*.diff at the end of update: — because `rm -rf charts` discards a hand-edit and the next `make update` would take TLS back out without failing. tls.diff also carries one change that is not TLS and takes effect with TLS off: MakeSlaveOfWithPort connects to the replica on its own port rather than on the master's. It changes a dial, not a pod template, so nothing rolls when this image ships; the probes dial localhost only under TLS, and the plaintext pod templates stay byte-identical to v3.3.5. The patch header names it, so a reader of patches/ does not take the file for TLS-only. The grant is narrower than the ported operator's own chart asks for: that one also grants patch on Secrets, and create, delete, get, list, patch, update and watch on Certificates. None of that is reachable from the source as shipped here. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
eb4abc8d93 |
feat(redis): make TLS opt-in instead of following external access
TLS was enabled automatically whenever external was true. In this operator TLS is not additive — the rendered config sets port 0 and moves the listener to tls-port — so that inference turned every already-running externally-reachable Redis into a TLS-only server on its first reconcile after the upgrade and cut every plaintext client with it. A platform upgrade must not sever working connections. Resolve tls.enabled to false when unset, whatever external says. Enabling TLS becomes an explicit act at creation time, and existing releases keep the behaviour they have today. Creation time, not any time: 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, so the operator's RedisFailover schema rejects a change of tls.enabled and the HelmRelease upgrade fails while the instance keeps serving what it served before. The README says so and points at creating a new instance and migrating. Pin the decision directly: external: true on its own now has to leave the cert-manager chain unrendered, the operator without spec.tls, and the tenant without TLS secrets. Every case that previously reached TLS through external says enabled: true instead, and the null and empty-map forms resolve to off alongside the unset one. With nothing inferred there is no upgrade-time behaviour change left, so the breaking-change marker and footer are gone from this branch. Say how to get out of it as well. The cozystack Redis API accepts the change, so the rejection lands on the HelmRelease, and every later edit to that instance queues behind the failure until tls.enabled goes back to what it was. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f5206a6e28 |
docs(redis): declare the TLS-on-upgrade break and fix the parameter sections
TLS replaces plaintext in this operator rather than running beside it — with TLS on, Redis and Sentinel are configured with port 0 and serve only on the TLS port. Since tls.enabled falls back to external, an already deployed instance with external: true switches to TLS-only on its first reconcile after the upgrade and every plaintext client breaks. That was documented as a feature and nowhere as a change of behaviour for existing instances. State it as a breaking change and name tls.enabled: false as the opt-out. Also spell out which hostname the certificate actually covers. The SAN list carries the four service names and the external tenant hostname, and its only IP entries are the loopback addresses used by the in-pod probes and the metrics sidecar, so connecting to the external LoadBalancer by IP fails for clients that verify the hostname. The TLS section marker sat above the version parameter, which put version in the TLS table in the generated README instead of Common parameters. Move the marker below it. Regenerating also reorders version ahead of tls in keysOrder, matching how postgres orders the same two fields. Pin the PackageSource CRD replacement policy the way the mongodb-operator one is pinned. The RedisFailover CRD gained spec.tls with the fork, and Helm never upgrades CRDs shipped in crds/, so losing CreateReplace would leave the live CRD behind and the apiserver would strip spec.tls from every RedisFailover the chart renders — TLS silently off, no error. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Arsolitt
|
9095afea63 |
feat(redis): constrain tls.authClients to enum at the values source
Lift the documented `no` / `optional` / `yes` values for tls.authClients
from prose into an `## @enum {string} AuthClients` typedef in
packages/apps/redis/values.yaml. cozyvalues-gen then propagates the
constraint into:
- values.schema.json — adds `"enum": ["no","optional","yes"]` so
invalid values fail chart-value validation up front.
- api/apps/v1alpha1/redis/types.go — `AuthClients` becomes its own
Go type with a `+kubebuilder:validation:Enum=...` annotation,
aligning the cozystack Redis CR with the operator-side enum on
the RedisFailover CRD.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Arsolitt <arsolitt@gmail.com>
|
||
|
Arsolitt
|
56b9b4fcaa |
fix(redis): regenerate deepcopy for TLS field
Run the root `make generate` (hack/update-codegen.sh) to refresh api/apps/v1alpha1/redis/zz_generated.deepcopy.go with the new TLS struct introduced for operator-managed TLS support. The previous commit only ran the per-package `make generate`, which updates the chart schema and CRD ApplicationDefinition but not the deepcopy methods generated by deepcopy-gen. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com> |
||
|
Arsolitt
|
2ea4aad5de |
feat(redis): add operator-managed TLS via cert-manager PKI chain
Expose tls.enabled in the Redis chart values, mirroring the pattern already in use by mariadb, nats, opensearch, qdrant and rabbitmq. The flag is tri-state: when unset, TLS follows the value of `external`; an explicit boolean overrides the default. When TLS is enabled the chart renders the standard cozystack 4-resource cert-manager chain in the release namespace: - Issuer (self-signed bootstrap) - Certificate (CA, ECDSA, isCA, rotationPolicy: Never, 5y) - Issuer (CA-backed) - Certificate (leaf, secret <release>-tls, ECDSA, 1y, rotationPolicy: Always) The leaf certificate covers the SAN list the redis-operator computes for its own Certificate (operator/redisfailover/service/generator.go: redisCertificateDNSNames): short, namespace, .svc and .svc.<clusterDomain> forms for rfr- / rfrm- / rfrs- / rfs- services plus a wildcard for per-pod headless DNS names of the redis statefulset. Localhost is included for in-pod readiness probes; when `external` is true the tenant LB hostname is added too. The chart hands the resulting Secret to the operator via spec.tls.certificateSecret.secretName so the operator never creates a duplicate Certificate; it just mounts the Secret into both Redis and Sentinel pods and wires the redis tls-* directives. tls.authClients maps to the Redis tls-auth-clients directive and defaults to "no". The dashboard-resources Role is extended with the new TLS secret names only when TLS is on, so the cozystack dashboard can read them to display connection details. Helm unit tests in tests/tls_test.yaml cover the tri-state resolution (four enable/external combinations), SAN list coverage for all four service names and the wildcard, custom cluster-domain handling, the external SAN injection path, authClients propagation, and dashboard RBAC expansion. The comment above the CA usages named the wrong bits: cert-manager maps signing to the digital signature bit, and crl sign and ocsp signing are separate usages this chart does not request. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
ba32df69c0 |
feat(backups): add RabbitMQ definitions backup via a dedicated strategy driver
RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no
consistent online export of message payloads, so back up broker
DEFINITIONS (vhosts, users, permissions, queues, exchanges, bindings,
policies, parameters) over the management HTTP API and store them in the
system bucket. Message payloads and PITR are out of scope - a full-data
backup is a Velero volume snapshot, a separate strategy.
Add a first-class strategy.backups.cozystack.io/Rabbitmq driver, following
the Altinity pattern (a dedicated driver that renders a one-shot Job for an
HTTP-API engine):
- reconcileRabbitmq / reconcileRabbitmqRestore render the strategy's
PodTemplateSpec (curl GET/POST /api/definitions, push/pull to S3 with
--aws-sigv4) as a batch/v1.Job, for both in-place and to-copy restore.
- A Go-read precondition gates each run on the RabbitmqCluster reporting
AllReplicasReady (via the rabbitmqtypes partial type), surfacing a
precise Ready=False wait instead of a Job that starts and fails.
- The produced Backup records the object location on status.artifact.uri
(rendered from spec.artifactURITemplate; the Backup CRD has no status
subresource, so it persists on Create, as in the Etcd driver).
Wire dispatch in both controllers + supportedBackupStrategyKinds, register
rabbitmqtypes in the scheme, and grant read on rabbitmqclusters. Ship
cozy-default-rabbitmq (a $bucketName-gated Rabbitmq strategy) and route
apps.cozystack.io/RabbitMQ to it in the cozy-default BackupClass. The
application chart is untouched.
Add unit tests for the driver's pure helpers and the precondition gate, a
runnable example, and an honest-roundtrip Chainsaw e2e that seeds a
sentinel vhost, backs it up, drops it, and asserts it round-trips through
object storage on both restore flows.
The Job pod reuses the platform-migrations image (curl + ca-certificates)
under a new rabbitmqBackupClientImage key, stamped in lockstep with
chBackupClientImage so the repository stays pinned at one digest; a
dedicated slim image is a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrei Kvapil
|
6660a44376 |
feat(valkey): add managed Valkey application package
Add a managed Valkey service mirroring the managed Redis app. Valkey is the BSD-3-Clause, Linux Foundation-governed fork of Redis 7.2 and a drop-in replacement for it, offered as a governance-clean alternative to the now source-available (AGPLv3/SSPL/RSALv2) Redis 8.x. Valkey runs on the same Spotahome redis-operator: the operator hardcodes the redis-server command and redis-cli in its (non-overridable) probe scripts, so the chart deploys a thin compat image (FROM valkey/valkey:<ver>) that adds the redis-* symlinks over the official upstream binaries — no Valkey code rebuilt. Versions offered: v8 (8.1.8, default) and v7 (7.2.13, the BSD Redis 7.2 line). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Alexey Artamonov
|
06624bacd0 |
fix(vm-disk): fail on storageClass/source edit of existing disk
Editing a VMDisk's storageClass or source after its DataVolume already
exists was silently ignored: dv.yaml reused the existing DataVolume.spec
verbatim (DataVolume.spec is immutable in CDI), so the edit rendered as a
no-op while the HelmRelease reported success and the CR diverged from the
cluster.
When a DataVolume already exists, fail with a clear message if the
requested storageClass or source differs from what the disk was created
with. Both compare the operator's input recorded in an annotation at
creation (vm-disk.cozystack.io/storage-class and .../source), written only
at creation and preserved verbatim afterwards — never overwritten with an
unapplied edit. Recording the input rather than reading the live object
matters on both axes: the image-clone PVC name carries a chart-version
prefix (vm-image- was renamed to vm-default-images-, commit
|
||
|
Andrey Kolkov
|
b9a95a0c4a |
fix(postgres): let in-place restore re-bootstrap instead of wedging (#3959)
## What this PR does An in-place `RestoreJob` (one whose `targetApplicationRef` is omitted, so the app is restored back into itself) re-bootstraps the Postgres app under its own name. On stock v1.6.2 it never comes back up — the cluster is deleted and then hangs in `Setting up primary` with `*-full-recovery-*` pods crash-looping, or the HelmRelease upgrade fails outright. There are two independent causes, both in the same restore patch, and both surface only in place because a to-copy restore targets a different app. **1. Archive-vs-recovery `serverName` collision.** The driver set the restored cluster's WAL-archive `serverName` equal to the recovery source (`bootstrap.serverName = bootstrap.oldName = sourceServerName`), while the chart pinned the archiver `serverName` to the release name. In place those coincide, so the cluster archived onto the source's own WAL prefix and `barman-cloud-check-wal-archive` refused to start with `Expected empty archive`. Fixed by giving the restored cluster a fresh `bootstrap.newServerName`, keyed by the `RestoreJob` UID (unique per restore, stable across reconciles); the chart archives there while `externalClusters` still recovers from `bootstrap.serverName`. This is what manual `CNPG` strategies hit (their `serverName` defaults to the cluster name, including the shipped `examples/backups/postgres/10-cnpg-strategy.yaml`); the `cozy-default` flow dodged it because its `serverName` is `<namespace>-<app>`. **2. `useSystemBucket` guard.** The restore populates explicit S3 coordinates (`destinationPath` / `s3CredentialsSecret` / `endpointCA`) but left `backup.useSystemBucket=true`, so the chart's `bootstrap.enabled is incompatible with useSystemBucket` guard failed the HelmRelease upgrade and the cluster never re-rendered. This is what the default `cozy-default` (system-bucket) flow hits on an in-place restore. Fixed by clearing `useSystemBucket` in the restore patch, since the app is now on the explicit-creds path. Verified live on a v1.6.2 cluster: with `useSystemBucket` cleared, the same in-place restore re-bootstraps to a healthy cluster and the sentinel row survives. Both are exercised only by an in-place restore; the postgres backup e2e drove only to-copy, so neither surfaced in CI. A step 50 in-place restore now guards cause 1 (manual-strategy flow); cause 2 is covered by a Go unit assertion (a system-bucket in-place e2e needs the platform bucket wired in CI and is left as a follow-up). This is a bug in released v1.6.2 (chart and driver are byte-identical on `main`), so it is a backport candidate. ### Screenshots Not applicable (no UI changes). ### Downstream repositories The only externally visible schema surface is one new field, `spec.bootstrap.newServerName`, in the postgres `values.schema.json` / CRD; it is set by the CNPG backup driver on restore and never authored by tenants, exactly like the existing `bootstrap.serverName` / `bootstrap.oldName`. (The `useSystemBucket` clearing is internal to the Go driver — `backup.useSystemBucket` already exists in the schema.) The chart `README.md` is regenerated from `values.yaml` automatically by the website docs bot on release, and `terraform-provider-cozystack` models user-authored spec (its guard tracks top-level fields, not driver-set nested ones), so neither needs a change in practice. Leaving every box unticked per the trigger-map guidance so a maintainer confirms rather than acting on a speculative follow-up. - [ ] 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(postgres): in-place restore no longer wedges — the restored cluster archives WAL under a fresh serverName distinct from the recovery source, and a system-bucket app is switched to explicit credentials so the bootstrap/useSystemBucket guard no longer fails its HelmRelease. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for specifying a distinct WAL archive destination when restoring PostgreSQL clusters. - In-place restores now automatically use a fresh archive location while continuing to recover from the original source. - Added configuration and schema support for the optional `bootstrap.newServerName` setting. - **Bug Fixes** - Prevented archive conflicts that could cause in-place restores to fail. - **Documentation** - Documented the new restore archive configuration and behavior. - **Tests** - Added coverage for unique restore destinations and in-place restore validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
9dd2841c74 |
feat(clickhouse): add configurable version parameter (#3476)
## What this PR does The ClickHouse server and Keeper images were hardcoded to `24.9.2.42` in the chart templates, so the chart exposed no top-level `version` values key. The Cozystack API returns a managed database's engine version by passing the HelmRelease's `spec.values.version` straight through into the Application object, so ClickHouse instances carried no `spec.version` in the API — unlike postgres, mariadb, mongodb, opensearch, rabbitmq and redis, which all ship a `version` parameter. This closes the ClickHouse half of the gap tracked in #1246. This adds the same version-selection scaffolding the other managed-database charts already use: - `files/versions.yaml` maps a `major.minor` key to the full image tag, shared by the ClickHouse server and Keeper images (both are pinned to one version). - `templates/_versions.tpl` (`clickhouse.versionMap`) resolves the tag and fails the render on an unsupported version. - `values.yaml` gains a `version` enum (`v25.8`, `v25.3`, `v24.9`), consumed by `templates/clickhouse.yaml` and `templates/chkeeper.yaml`. - `hack/update-versions.sh` + `make update` refresh the map from Docker Hub, keeping only tags published for *both* the server and Keeper images and preserving the current default so a regeneration never bumps the major of existing deployments. The default stays `v24.9` → `24.9.2.42`, so existing installations render byte-for-byte identically and are never upgraded to a new major implicitly. The generated `values.schema.json`, `README.md`, the Go API type and the `clickhouse-rd` `openAPISchema` are regenerated with `cozyvalues-gen` v1.6.0. A new `tests/version_test.yaml` helm-unittest suite covers the server/Keeper mapping for each version and the schema-level rejection of an unsupported value. ### Downstream repositories The diff changes `packages/apps/clickhouse/values.schema.json` by adding a new `version` field with an `enum`. Per the trigger map, this reaches `terraform-provider-cozystack`, whose ClickHouse resource is hand-written and would need a matching `version` attribute with a `stringvalidator.OneOf` list and its expand/flatten pair. There is no existing PR or issue there for it yet. This is left for a maintainer decision rather than a speculative cross-repo PR, so the box below is intentionally left unticked and flagged here. No other downstream repository is reached (the website reference page is regenerated from `README.md` by the release docs bot; ClickHouse is already in its app list). - [ ] 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: needs a `version` attribute on the ClickHouse resource (maintainer decision — see note above) - [ ] [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(clickhouse): add a configurable `version` parameter (v25.8, v25.3, v24.9; default v24.9) so the deployed ClickHouse version is selectable and returned through the API. Existing installations are unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a configurable ClickHouse version setting for both server and Keeper images, defaulting to `v24.9`. * Supported versions are `v25.8`, `v25.3`, and `v24.9`, with compatible image patches selected automatically. * **Documentation** * Documented the version setting, default, and supported values. * **Validation** * Unsupported versions are rejected with an error listing allowed options. * **Tests** * Added coverage for version mapping, defaults, invalid values, and update behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
5bcdb8cfb8 |
fix(postgres): let in-place restore re-bootstrap instead of wedging
An in-place RestoreJob (targetApplicationRef omitted) re-bootstraps the
Postgres app under its own name. On stock v1.6.2 it never comes back up,
for two independent reasons in the same restore patch - both surface only
in place, because a to-copy restore targets a different app.
1. Archive-vs-recovery serverName collision. The driver set the restored
cluster's WAL-archive serverName equal to the recovery source
(bootstrap.serverName = bootstrap.oldName = sourceServerName) while the
chart pinned the archiver serverName to the release name. In place those
coincide, so the cluster archived onto the source's own WAL prefix and
barman-cloud-check-wal-archive refused to start ("Expected empty
archive"). Fixed by giving the restored cluster a fresh
bootstrap.newServerName, keyed by the RestoreJob UID (unique per
restore, stable across reconciles); the chart archives there while
externalClusters still recovers from bootstrap.serverName. This is what
the manual CNPG strategies hit (serverName defaults to the cluster
name); the cozy-default flow dodged it (serverName is <ns>-<app>).
2. useSystemBucket guard. The restore populates explicit S3 coordinates
(destinationPath / s3CredentialsSecret / endpointCA) but left
backup.useSystemBucket=true, so the chart's "bootstrap.enabled is
incompatible with useSystemBucket" guard failed the HelmRelease upgrade
and the cluster never re-rendered. This is what the cozy-default
(system-bucket) flow hits on an in-place restore. Fixed by clearing
useSystemBucket in the restore patch, since the app is now on the
explicit-creds path. The driver's app view (postgresapp.Backup) gains
the field, without omitempty so a false survives the merge patch.
Both are exercised only by an in-place restore; the postgres backup e2e
drove only to-copy, so neither surfaced in CI. A step 50 in-place restore
now guards reason 1 (manual-strategy flow); reason 2 is covered by a Go
unit assertion (a system-bucket in-place e2e needs the platform bucket in
CI and is left as a follow-up).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Alexey Artamonov
|
3c78c0be56 |
feat(clickhouse): add configurable version parameter
Add a `version` value (v25.8, v25.3, v24.9; default v24.9) to the ClickHouse chart, backed by files/versions.yaml and resolved by templates/_versions.tpl for both the ClickHouse server and Keeper images, so the Cozystack API returns the engine version (spec.version) like the other managed-DB charts. Default v24.9 -> 24.9.2.42 keeps existing installations byte-for-byte identical. The maintainer-run hack/update-versions.sh generator intersects the server/keeper Docker Hub tags byte-collated (LC_ALL=C) so comm keeps tags across the X.9 -> X.10 boundary; freezes the default major to the tag it already ships (read from versions.yaml, so the freeze follows the default); errors on a dropped default or an unresolvable major; writes atomically; and runs on stock macOS bash 3.2 / BSD. Covered by hack/clickhouse-update-versions_test.bats (10 cases) and the tests/version_test.yaml helm-unittest suite. Refs: #1246 Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
IvanHunters
|
50e06de113 |
fix(kubernetes): drop stale GPU deepcopy left by the main merge
The main merge kept main's GPU DeepCopy funcs, but the Phase 2 chart split removed the worker nodeGroups (and their gpus field) from the kubernetes chart values, so cozyvalues-gen no longer emits `type GPU` in types.go. The orphaned DeepCopy referenced an undefined type and broke the Go build and the generated-code drift check. Regenerate: no GPU deepcopy. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
f035da8a0f |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Resolve conflicts introduced by main: - Drop the FluxCD tenant addon (API type, deepcopy, values schema, RD) in favour of the GPU type main added; main removed the fluxcd addon wholesale. - Renumber the Phase 2 worker-pool adoption migration 54 -> 56, behind main's redis-operator (54) and fluxcd-removal (55) migrations; targetVersion 55 -> 57. - Accept the branch's removal of the in-chart talos-reconcile Job; it moved to the kubernetes-nodes chart, main only touched its comment. - Regenerate kubernetes values.schema.json / README and the kubernetes-rd ApplicationDefinition schema. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Andrei Kvapil
|
7cca3d75e7 |
feat(kubernetes)!: remove the FluxCD tenant addon
The optional `addons.fluxcd` toggle of the Kubernetes app deployed a Flux Operator + FluxInstance into a tenant cluster. It was served by two vendored charts, packages/system/fluxcd-operator and packages/system/ fluxcd, which were the last AGPL-3.0 content in this Apache-2.0 repository and a governance blocker for CNCF Incubation. The management cluster does not use these charts (it runs classic Flux via internal/fluxinstall; migration 21 already removed the operator from it), and the addon defaulted to disabled. Rather than maintain an AGPL payload for an unused feature, drop it entirely: - delete both system packages and their platform source entries - remove the addons.fluxcd HelmReleases, values and generated schema from the Kubernetes app - drop the addons.fluxcd block still submitted by the e2e harness - drop the removed fluxcd package entries from CODEOWNERS BREAKING CHANGE: the addons.fluxcd option of the Kubernetes app is removed. Tenant clusters that had it enabled keep their running Flux release; see the companion migration for the upgrade path. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
IvanHunters
|
cd72d81bb4 |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Reconcile the phase-2b split with main's configurable worker-health feature (MHC condition timeouts, maxNodeProvisionTime, podCpuLimit/ podCpuRequest), which landed on both charts after this branch forked: - kubernetes (parent): pool rendering stays removed; main's parent worker-pool additions do not apply post-split. - kubernetes-nodes (pool): absorb the feature; keep the directional parent-version skew guard and the immutable cluster/storageClass rules. KubevirtMachineTemplate content hashes are unchanged (render snapshot). - migration 54: carry podCpuLimit/podCpuRequest (KMT-hash inputs) and the cluster-wide readyUnknownTimeout/readyFalseTimeout/maxNodeProvisionTime into adopted pools, so adoption neither rolls nor resets a tuned pool. Covered by a new adopt-values bats case. - drop the parent chart's obsolete MHC-timeout suites; the kubernetes-nodes twins cover the feature. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
3710541278 |
fix(kubernetes): validate MachineHealthCheck condition timeouts at render time
The CAPI webhook validates nodeStartupTimeout with a 30s floor but does
not validate unhealthyConditions timeouts at all, so a zero or negative
readyUnknownTimeout or readyFalseTimeout applied and remediated a
Machine the moment its Ready condition flipped. Route both fields
through the same render-time duration guard as maxNodeProvisionTime,
extracted into a kubernetes.positiveDuration helper in each chart.
The guard now also accepts the canonical form a duration serializes
back to ("20m0s", "1h0m0s"): metav1.Duration prints that shape, so a
value copied from kubectl output previously failed the render. The
first segment must still be positive and later segments may be zero,
which keeps every accepted string a positive duration.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
|
||
|
Aleksei Sviridkin
|
ddb15ebc6a |
fix(kubernetes): widen the worker node startup budget to 20m
nodeStartupTimeout is the window a Machine has to produce a Node at all, measured from the later of the Machine's creation and its InfrastructureReady transition. What has to fit inside it is the guest boot and the kubelet registering with the tenant apiserver, and on a host under load neither is quick. At 10m a worker that was still booting was deleted before it ever joined, and its replacement started the same race from the beginning. Also record what a percentage maxUnhealthy does at small group sizes, because the interaction is not what the field name suggests: the value is scaled against the machines the check covers and rounded down, and remediation proceeds only while the unhealthy count is at or below the result. At the default "50%" a group of one never remediates, and a group of two remediates one unhealthy machine but not two. The golden-parity gate pinned the child chart's MachineHealthCheck values to the parent's defaults as inputs, which made a default moved on both sides fail there for the wrong reason. Drop the pins so the comparison is between the two charts' defaults, which is the contract it exists to hold, and pin the defaults themselves in each chart's own unit tests instead. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
33701800d6 |
fix(kubernetes): make the worker MachineHealthCheck timeouts configurable
The two unhealthyConditions timeouts were fixed in the template at 30s for `Ready=Unknown` and 300s for `Ready=False`, and both are too tight for a worker joining on a loaded host. `Ready=Unknown` is what the node lifecycle controller writes when a kubelet stops reporting, on a grace period of its own, so 30s on top of that deleted the VM of a guest that had merely stopped being scheduled for a stretch, before it had a chance to answer again. And `Ready=False` is the condition a registered Node holds until its CNI is up, so 300s was the entire budget a first boot got for the CNI rollout to reach it -- a worker deleted mid-rollout takes the rollout with it and its replacement starts over. Move both to values, defaulting to 5m and 10m. The remediation of a worker that is genuinely dead is correspondingly later, which costs less than it appears to: Pods leave an unreachable Node on the node lifecycle controller's taint-based eviction and the toleration that carries, not on either of these timers. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
83f0eef645 |
fix(kubernetes): give the autoscaler the real worker join time
The cluster-autoscaler treats a machine as long-unregistered once its own max-node-provision-time elapses, and a worker that has not registered by then stops counting towards its node group's upcoming capacity. On a host slow enough that the Talos image import, the guest boot and the CNI rollout do not fit inside that window, the timer fires on machines that were still joining: the group reads as short of capacity, so the autoscaler asks for replacements, and the extra provisioning load makes the join it was waiting for slower still. Render the per-node-group override the autoscaler reads off the MachineDeployment, sourced from a new maxNodeProvisionTime value that defaults to 30m. The duration is checked at render time because the autoscaler discards one it cannot parse and falls back to its own default without failing, and because the values schema types the field as a string and so accepts every malformed duration. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
IvanHunters
|
5297df5cf3 |
Merge remote-tracking branch 'origin/main' into pr-3315-lgtm
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> # Conflicts: # api/apps/v1alpha1/kubernetes/types.go # api/apps/v1alpha1/kubernetes/zz_generated.deepcopy.go # hack/e2e-chainsaw/_lib/run-kubernetes.sh # packages/apps/kubernetes-nodes/tests/render-parity.sh # packages/apps/kubernetes/README.md # packages/apps/kubernetes/templates/cluster.yaml # packages/apps/kubernetes/values.schema.json # packages/apps/kubernetes/values.yaml # packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml # packages/system/kubernetes-rd/cozyrds/kubernetes.yaml |
||
|
Aleksei Sviridkin
|
3cb131785c |
fix(kubernetes): settle what a pod CPU quantity of zero or less means
Both podCpuLimit and podCpuRequest were rendered under `with`, which skips a zero as readily as an absent field, and their guards were keyed the same way. The generated schema behind them admits a leading minus and a zero as readily as a sane value, so rather than patch whichever edge turned up, the whole value domain is settled in one place. Presence, not truth, is the predicate everywhere either field is read. A negative value is refused: it never reaches a running Pod. A zero request on its own is allowed and reserves nothing. A zero request beside a non-zero limit is refused: KubeVirt's defaulter tests the request with IsZero, which an explicit zero satisfies exactly as an absent key does, so it overwrites the zero with the limit and the worker asks the scheduler for its whole ceiling. That is the outcome podCpuRequest exists to prevent, reached silently and reported nowhere, which is why it is refused rather than written down. A zero limit removes the ceiling rather than being the tightest one -- the kubelet turns a zero CPU limit into no CFS quota -- but it reaches a Pod only paired with a zero request. KubeVirt keeps deriving a request from the vCPU count when the VM declares none, the defaulter that would have copied the limit across does not fire because the limit is itself zero, and API server validation refuses a request above a limit that exists. Zero is a limit that exists. So a lone zero limit is refused too. Both fields document the same rules and each class has a test on both charts. A group that sets neither field renders the bytes it rendered before, so the content-hash name of the worker VM template does not move. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
1b8340298e |
feat(kubernetes): let a node group set the CPU request of its worker VMs
Adds nodeGroups.<name>.podCpuRequest, rendering into domain.resources.requests.cpu on the worker VM, and the same field on the kubernetes-nodes chart, which is the other half of a byte-identity contract rather than a separate feature surface. podCpuLimit on its own cannot raise the ceiling of a worker without raising what that worker asks the scheduler for, because KubeVirt copies a declared CPU limit into the request when the VM declares none. The request is the half that has to hold still, so the pair can now be moved apart. Both fields conflict with instancetype sizing for the same reason and in the same upstream check, so the request gets the guard the limit already had. A request above its own limit fails the render too, rather than reaching API server validation on a Pod two objects away from the CR that named the values; the quantities are compared as numbers, so 500m under a ceiling of 1 is accepted where a lexical guard would refuse it. A node group that sets neither field renders the bytes it rendered before, which the content-hash name of the KubevirtMachineTemplate depends on, and the render-parity gate gains a case that compares the pair across the two charts. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
669c3e60ab |
docs(kubernetes): correct what podCpuLimit does to the CPU request
The field was documented as moving the ceiling alone and leaving the CPU request at the value KubeVirt derives from the guest vCPU count and the cluster CPU allocation ratio. KubeVirt does the opposite: its VMI defaulter copies a declared CPU limit into the CPU request whenever the VM declares no request of its own, so a node group that sets only the limit asks the scheduler for its whole ceiling, and a ceiling the nodes cannot spare leaves the workers Pending on Insufficient cpu. The same paragraph promised a floor that does not exist either. A limit below the derived request was said to leave the container with a request above its limit; the copy makes the two equal instead, whatever the limit is. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
78b9bc3e6e |
feat(kubernetes-nodes): mirror the worker pod CPU limit into the pool chart
The split-out pool chart renders the same worker VM as the monolithic one, and tests/render-parity.sh exists to prove it renders it byte-identically, because the KubevirtMachineTemplate is content-hash named and any divergence rolls every live worker VM. A field that only one of the two charts knows breaks that quietly rather than loudly: a pool adopted into a kubernetes-nodes release would render without the CPU limit it was given, the hash would change, and the ceiling would fall back to the vCPU count with nothing reporting it. So the field, its render-time guard against combining it with an instancetype, and its tests arrive here in the shape the parent chart has them, and the parity gate gains a case that compares the two renders with the limit set. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
a666b9b3bd |
feat(kubernetes): let a node group set the CPU limit of its worker VMs
The chart had no way to give a worker VM a CPU ceiling different from its vCPU count. `resources.cpu` sizes the guest, and the pod side is filled in by KubeVirt: with a ResourceQuota limiting CPU in the namespace, which every tenant carrying resourceQuotas has, it derives limits.cpu from the guest vCPU count. The QEMU emulator thread and the IO threads then come out of the same allowance as the vCPU threads, which is tight for a nested guest that pays emulation overhead. Declaring the limit on the VM is what stops KubeVirt deriving one, and it is the only way out that does not make things worse: dropping cpu from the tenant resourceQuotas would let the LimitRange apply its 250m default instead, four times tighter than what it replaced. The limit is usable only on a group sized by resources, because KubeVirt's instancetype applier reports domain.resources.limits.cpu as a conflict rather than resolving it, and a VM carrying both is refused by the admission webhook with nothing on the CR to say why. Sizing by resources is what drops the instancetype from the VM, so the chart fails the render for a group that kept its instancetype, beside the existing guard for the same conflict class one field over. The field renders no CPU request, so the request stays derived from the vCPU count and the cluster CPU allocation ratio, and the scheduler weight of a worker does not move with its ceiling. It renders nothing at all when unset, byte for byte: the enclosing KubevirtMachineTemplate is named by a hash of its content, so anything else there would roll every live worker VM on upgrade. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
dff56aa998 |
feat(backups): add a native MongoDB (psmdb) backup-strategy driver
Adds a MongoDB (Percona Server for MongoDB / psmdb) backup-strategy driver so `apps.cozystack.io/MongoDB` reaches feature parity with the CNPG, MariaDB and ClickHouse native drivers — logical backups, restore-to-differently-named instances, and point-in-time recovery through the unified BackupJob/RestoreJob/BackupClass/Plan interface (closes #3545). The driver orchestrates the operator's PerconaServerMongoDBBackup / PerconaServerMongoDBRestore CRDs: - BackupJob: creates a one-shot PerconaServerMongoDBBackup, polls status.state, and persists the S3 destination + storage descriptor (by-reference, no credentials) onto the Cozystack Backup artifact. - RestoreJob: drives PerconaServerMongoDBRestore via backupSource (read back from the source backup status, snapshot fallback) so the same artifact restores in-place or into a differently-named instance; PITR via spec.options.pitr (date/latest). Because psmdb resolves the backup storage by name against the cluster's spec.backup.storages, the driver names the chart's storage (s3-storage) via the cozy-default-mongodb Strategy rather than injecting it, and surfaces a clear Ready=False precondition when a target has backups disabled. Wiring: - api/backups/strategy/v1alpha1: MongoDB strategy type + generated CRD/deepcopy. - internal/backupcontroller: psmdbstrategy_controller + psmdbtypes/mongodbapp partial-schema mirrors; dispatch, supported-kinds and scheme registration. - backupstrategy-controller: cozy-default MongoDB route, cozy-default-mongodb Strategy CR, and psmdb.percona.com RBAC. - apps/mongodb: backup.insecureSkipTLSVerify knob (psmdb s3 storage has no CA-bundle field) so backups can target Cozystack's self-signed seaweedfs. Tests + e2e: - unit tests for the driver (validation, ensure-idempotency, precondition, backupSource resolution, snapshot round-trip, PITR option parsing). - examples/backups/mongodb + a mongodb-2-backup-roundtrip Chainsaw suite that drives run-all.sh end-to-end (sentinel document round-trips through S3 into a restored copy while the source stays untouched), per docs/agents/e2e-testing.md. - docs/operations/backup-classes.md updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
IvanHunters
|
b4a2f6f791 |
Merge remote-tracking branch 'origin/main' into pr-3315-lgtm
# Conflicts: # api/apps/v1alpha1/kubernetes/types.go # api/apps/v1alpha1/kubernetes/zz_generated.deepcopy.go # hack/e2e-chainsaw/_lib/run-kubernetes.sh # packages/apps/kubernetes-nodes/tests/render-parity.sh # packages/apps/kubernetes/README.md # packages/apps/kubernetes/templates/cluster.yaml # packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml # packages/apps/kubernetes/tests/talos_templates_test.yaml # packages/apps/kubernetes/values.schema.json # packages/apps/kubernetes/values.yaml # packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml # packages/system/kubernetes-rd/cozyrds/kubernetes.yaml |
||
|
Andrey Kolkov
|
d0990299ed |
feat(clickhouse): let the backup sidecar trust a private S3 CA, unblocking the backup e2e round-trip (#3385)
## What this PR does Gives the ClickHouse `clickhouse-backup` sidecar a way to trust an S3 endpoint whose certificate is signed by a private CA, and uses that to make the ClickHouse backup/restore e2e round-trip actually run in CI. This actualizes #2600, whose feature code and example scripts have since landed on `main` independently, and whose remaining contribution — the e2e test — was still in the retired BATS format. #2600 can be closed once this merges. ### 1. `backup.endpointCA` on the ClickHouse chart The sidecar had **no TLS/CA surface at all**: bucket coordinates and credentials arrive as `S3_*` env, and nothing let it verify an endpoint signed by a private CA. Cozystack's own in-cluster SeaweedFS is exactly that case — it serves `:8333` behind the self-signed "SeaweedFS CA" — so a ClickHouse release could only ever back up to a publicly-trusted endpoint. MariaDB (`storage.s3.tls.caSecretKeyRef`) and Postgres (barman `endpointCA`) have had a CA field all along; this closes the gap for ClickHouse. `backup.endpointCA` takes `{name, key}`. When set, the chart mounts that Secret into the sidecar and points `SSL_CERT_DIR` at it. Empty — the default — renders no env, no mount and no volume, so existing releases are byte-for-byte unchanged. **Why `SSL_CERT_DIR` and not `AWS_CA_BUNDLE`.** Both are honoured by `altinity/clickhouse-backup:2.7.4`; I measured them against a self-signed endpoint and public AWS S3 in turn. Go reads `SSL_CERT_DIR` *in addition to* the system bundle, so both endpoints verify. `AWS_CA_BUNDLE` *replaces* the SDK's trust pool: the private endpoint verifies, and public S3 then fails `x509: certificate signed by unknown authority` — it would silently break a release that also talks to a public endpoint. `S3_DISABLE_CERT_VERIFICATION` was rejected outright, since it drops verification rather than trusting one more CA. ### 2. The e2e suite, and what it was really testing Two Tests added to the existing `hack/e2e-chainsaw/clickhouse/` suite (in name order, mirroring the `etcd`/`mariadb`/`postgres` shape — see the layout rationale below for why they live here rather than in a new directory): - **`clickhouse-1-backup-contracts`** (CI, `tenant-test`, ~98s): a backup-enabled ClickHouse asserted against the *running* cluster — the `clickhouse-backup` sidecar on the `ch-backup-api` `:7171` port, and the `<release>-backup-api-auth` Secret whose keys the sidecar's `API_USERNAME`/`API_PASSWORD` actually resolve `valueFrom`. Each is a silent break if a chart bump renames it. Needs no object store. - **`clickhouse-2-backup-roundtrip`** (CI, `tenant-root`): `Bucket` → `Altinity` strategy + `BackupClass` → source ClickHouse with a sentinel row → `BackupJob` → in-place `RestoreJob` → to-copy `RestoreJob` into a second instance, driving `examples/backups/clickhouse/run-all.sh` so test and docs cannot drift. The round-trip was originally gated behind `CLICKHOUSE_E2E_S3_ROUNDTRIP` and **skipped in CI** — its `PASS (1.01s)` meant the gate fired, not that a backup happened. `backup.endpointCA` removes the reason for the gate, so the suite now adopts the same three preconditions mariadb/postgres rely on: run in `tenant-root` (an isolated tenant's Cilium egress allowlist blocks its Pods from reaching `tenant-root`'s SeaweedFS), target the in-cluster endpoint instead of the `Bucket`'s external ingress URL (an unroutable placeholder in CI), and trust its certificate via the copied CA. It also pre-cleans with `cleanup.sh`, because `tenant-root` is shared and a leftover `Succeeded` `BackupJob` would satisfy the harness's wait without a backup ever running. Both backup Tests live in the existing `hack/e2e-chainsaw/clickhouse/` suite rather than a new suite directory, so no selector mapping is added: `hack/select-e2e.sh` already derives `clickhouse` both from `packages/apps/clickhouse/**` (the `*-application` rule) and from `examples/backups/clickhouse/**` (the harness path), and both paths therefore select the suite that actually runs the harness. That alignment is the reason for the layout — a separate `clickhouse-backup/` directory would have made an edit to the harness select a suite that cannot exercise it. `hack/select-e2e.sh` and `hack/select-install.sh` are otherwise untouched (the only change is a comment recording that invariant), and the existing `backup example harness edit selects its app suite` case in `hack/select-e2e_test.bats` covers the mapping. ### Verification - `helm unittest` — 17 pass, including 5 new `endpointCA` cases: the unset default renders nothing, the env/mount/volume wiring, a custom key still projected as `ca.crt`, composition with `useSystemBucket`, and `backup.enabled=false` renders nothing. - The CA-trust behaviour was measured against the real image, not assumed: without a CA knob the sidecar reports `x509: certificate signed by unknown authority`; with `SSL_CERT_DIR` the handshake succeeds. - `hack/select-e2e_test.bats` (16) and `hack/select-install_test.bats` (17) pass; `select-install.sh --validate` reports graph OK. - `make generate` is idempotent, so the pre-commit job's `git diff --exit-code` stays clean. - The contracts Test passed in CI on earlier runs of this branch. The round-trip only became CI-live with this work, so it still needs a green E2E run before merge — the branch has since been rebased onto current `main` to get one. ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff, file by file. The load-bearing change is the new `backup.endpointCA` field in `packages/apps/clickhouse/values.yaml` / `values.schema.json`; everything else is `hack/e2e-chainsaw/`, `examples/backups/`, one docs paragraph, and regenerated artifacts, none of which appear in the map. - [ ] No downstream repository is affected by this change - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/18 — "add, remove or rename a field in an app's `values.schema.json`" reaches the provider's hand-maintained schema, model and expand/flatten pair, so the new nested object is not surfaced until it is added there. Opened as an issue rather than a PR because the natural moment is alongside that repo's next `api/apps/v1alpha1` pin bump, which is a maintainer call. Not ticked, with reasons rather than silence: **cozystack/website** — the trigger is *adding, renaming or removing a package*, and this adds a field to an existing one. `clickhouse` is already in the website `Makefile`'s app list and its reference page is generated from this package's `README.md`, so the regenerated `backup.endpointCA` rows are carried by the existing release-tag automation with no manual PR. **cozystack/external-apps-example** — vendors `hack/package.mk` and `update-crd.sh`, neither of which this PR touches. **cozystack/ansible-cozystack**, **ccp**, **talm**, **cozyhr**, **cozy-proxy**, **cozystack-telemetry-server**, **examples** — no installer values, `hack/` layout, namespace, variant, node-prerequisite, annotation, label or metric in their trigger rows is touched here. ### Release note ```release-note feat(clickhouse): add `backup.endpointCA` so the clickhouse-backup sidecar can verify an S3 endpoint whose certificate is signed by a private CA, such as Cozystack's in-cluster SeaweedFS. Leave it unset for publicly-trusted endpoints; existing releases are unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional private S3 endpoint CA support for ClickHouse backup/restore sidecars via `backup.endpointCA`, including configurable Secret name and CA bundle key (default `ca.crt`). * Sidecars mount the CA and extend trust while leaving public endpoints unchanged. * **Documentation** * Updated ClickHouse chart docs, operational backup guidance, and backup/restore examples to cover CA setup, overrides, and cleanup. * **Tests** * Added helm-unittest and end-to-end coverage validating CA rendering/mounting behavior and ClickHouse backup/restore workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3ae1e10c92 |
docs(kubernetes): correct the registryMirrors field and README claims
State in the README that in-guest registry pulls are routable again via talos.registryMirrors, which knob covers which artifact (the mirror routes registry pulls; the worker OS disk image arrives as a raw HTTP artifact under talos.imageFactoryURL and no registry mirror touches it), and the limits: skipFallback is needed for air-gap, machine.registries.config is not passed through, and existing nodes keep their boot-time config until replaced. Trim the field doc comment. The regenerated schema, README tables and cozyrds openAPISchema follow from those sources. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |