mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 14:02:56 +09:00
fix/select-e2e-find-not
76
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
fb7e780517 |
fix(postgres): reassign dropped roles' objects to an unprivileged role (#4574)
## What this PR does Objects owned by a role the Postgres init-job drops no longer end up owned by the `postgres` superuser. This covers a user removed from `users` and an orphaned managed `<db>_admin` or `<db>_readonly` role. Before dropping such a role the job reassigned everything it owned to `postgres`, and objects a tenant role created should not become superuser-owned. They now go to `cozystack:orphaned`, a NOLOGIN role that is granted nothing. The init-job creates it and resets its attributes on every run. The `:` in the name is outside the charset allowed for user and database names, so values cannot declare it. There is a trade-off. Grants and memberships of the dropped role do not carry over, so a view or function it owned that ran with its owner's privileges can lose access it had through them. To keep such objects working, keep the user in values or move the objects to another owner before removing it. `docs/operations/backup-classes.md` is updated accordingly. Role names read from the catalog now reach SQL as psql variables instead of being spliced in by the shell. The queries are echoed after substitution, so the job log shows which role is dropped. Tested with helm-unittest and by running the rendered init script against PostgreSQL 13 and 18. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(postgres): objects owned by a user removed from `users`, or by an orphaned managed database role, are now reassigned to the unprivileged `cozystack:orphaned` role instead of the `postgres` superuser. A view or function of the removed role that ran with its owner's privileges can lose access it had through that role's grants or memberships. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that after restore, the first initialization releases undeclared managed databases without deleting their data and removes users not declared in the chart. * Removed users’ objects are reassigned to a restricted, non-login role. Grants and memberships are not preserved; functions use invoker rights, and PostgreSQL 15+ views are set to use invoker security. * Added guidance for finding eligible objects still owned by the PostgreSQL superuser for manual reassignment. * Declare a user in `users` to keep it; the chart will generate a new password. * **Bug Fixes** * Removed users’ objects are reassigned across connectable, non-template databases before their users are dropped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
54159736bc |
feat(talos)!: bump Talos to v1.14.2 (#4598)
## What this PR does Bumps the Talos image Cozystack builds from v1.13.6 to v1.14.2 to get a newer DRBD. v1.13.6 ships DRBD 9.3.2, v1.13.7 and later and v1.14.0/v1.14.1 ship 9.3.3, and v1.14.2 is the first release with 9.3.4. ZFS moves from 2.4.3 to 2.4.4. On 9.3.2 I've seen a peer reboot leave the DRBD sender thread spinning on a failed send (EPIPE). What followed was RCU stalls, processes stuck in D-state, a hanging `drbdsetup disconnect` and two-phase-commit timeouts. The [9.3.4 ChangeLog](https://github.com/LINBIT/drbd/blob/drbd-9.3.4/ChangeLog) has fixes for a sender thread pinning a CPU while its connection is down, for two-phase-commit issues and a hanging `drbdadm disconnect`, and for two nodes ending UpToDate with different data after a reconnect. None of them are in 9.3.3. I matched the trace to those entries by their descriptions only, not against the commits. The base installer image had to change. Talos v1.14 no longer publishes `ghcr.io/siderolabs/installer`, so `gen-profiles.sh` now uses `ghcr.io/siderolabs/installer-base`, same as upstream's own v1.14 imager profiles. I built the amd64 installer from the new profile with `imager:v1.14.2` locally and it builds without errors. PR CI will not boot the new image. It builds the installer and matchbox images, but the e2e runs on the upstream `talos:v1.13.5` container from `hack/e2e-compose.yaml`. The first run that boots the new kernel, DRBD and ZFS is the nightly after merge, which builds the nocloud disk from these profiles and runs it in QEMU. On Talos v1.14.2 the `drbd_transport_tcp` module is no longer loaded on demand. After upgrading a node from v1.13.6, `drbd` was loaded and the transport module was not, `drbdsetup new-peer` failed with "Failed to create transport (drbd_transport_xxx module missing?)", and every DRBD resource on the node stayed in Connecting. Listing the module in `machine.kernel.modules` fixed it without a reboot. The cause is [siderolabs/pkgs#1565](https://github.com/siderolabs/pkgs/pull/1565), in Talos since v1.14.0: the kernel is built with an empty modprobe path, so it no longer loads modules on `request_module()`, which is how DRBD asks for its transport. Any module that used to load that way now has to be listed explicitly (reported upstream as siderolabs/talos#14501). Outside DRBD this includes `dm-thin-pool` and `dm-multipath`, which the v1.14.2 kernel builds as modules, so LVM-thin or multipath users on Talos need them listed too. The e2e node config now loads `drbd_transport_tcp` explicitly, and the talm preset (cozystack/talm#252) and the install docs (cozystack/website#720) get the same line. The module ships in the drbd extension on v1.13 too, so the extra line is safe before the upgrade. The order matters for host nodes. Either update the cozystack preset in the talm project to a version with cozystack/talm#252 and re-render and apply the node config, or add `drbd_transport_tcp` to `machine.kernel.modules` by hand. Only then upgrade the node to v1.14.2. A node upgraded first loses DRBD replication until the module is added. The host network policy now also denies port 2383 to `world`. Talos v1.14 moved etcd's `/metrics`, `/health` and gRPC-gateway JSON API from 2379 to a separate listener on 2383, and the policy only blocked 2379 and 2380. Without this, the etcd JSON API on v1.14 control-plane nodes would be reachable from outside, protected only by client mTLS. Two smaller changes come with the bump. The multus `talos-cni-plugins-checked-against` marker moves to v1.14.2, because the pkgs release behind Talos v1.14.2 still pins CNI plugins v1.9.1, same as multus. The system memory limits doc now describes the v1.14 OOM trigger, since v1.14.0 dropped its global memory PSI clause ([siderolabs/talos#13895](https://github.com/siderolabs/talos/pull/13895)). Before upgrading hosts, operators should know a few things from the [v1.14.0 release notes](https://github.com/siderolabs/talos/releases/tag/v1.14.0) and the Talos compatibility code: - Talos v1.14 accepts a host upgrade from v1.12.0 or later. It supports Kubernetes 1.32 through 1.37, while v1.13 still accepted 1.31, so a cluster on Kubernetes 1.31 has to upgrade Kubernetes first. - etcd's HTTP endpoints (`/metrics`, `/health`) moved from port 2379 to 2383. The Cozystack etcd scrape proxy reads a separate metrics listener on `127.0.0.1:2381`, which this change does not move. Anything else that scraped or health-checked etcd on 2379 has to switch to 2383, and a firewall outside the cluster that blocked 2379 should block 2383 too. - etcd and kube-apiserver now require TLS 1.3, so a client that can only speak TLS 1.2 stops connecting. - Workload isolation (`SecurityProfileConfig`) stays off on upgraded clusters, but `talosctl gen config` turns it on for new ones. I haven't tested Cozystack with it enabled. - `talosctl apply-config --mode=reboot` is gone. ### Screenshots Not a UI change. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#720 - [ ] [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: - [x] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: cozystack/talm#252 - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The website PR refreshes the `next` version pins for v1.14.2 and adds the module to the Talos install pages. The talm PR adds it to the cozystack preset. ### Release note ```release-note feat(talos): Bump Talos to v1.14.2, which ships DRBD 9.3.4 and ZFS 2.4.4. DRBD 9.3.4 fixes a sender thread spinning on a dead connection, two-phase-commit hangs and a data divergence after a reconnect. Before upgrading a node, add `drbd_transport_tcp` to its Talos `machine.kernel.modules` next to `drbd`: Talos v1.14.2 no longer loads it on demand, and without it DRBD cannot connect to any peer. We recommend skipping the v1.13 line and upgrading nodes straight to v1.14.2. Talos accepts a direct upgrade from v1.12.0 or later. Talos v1.14 moves etcd's HTTP metrics and health endpoints from port 2379 to 2383, and the Cozystack host network policy now blocks 2383 from outside the cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Updates** * Upgraded host node support to Talos v1.14.2 across machine images, with refreshed firmware and storage extensions. * Updated memory-pressure guidance for Talos v1.14, including version-specific behavior and configuration recommendations. * Added protection for Talos v1.14’s etcd HTTP endpoint on port 2383. * Updated cluster preparation to load the TCP transport module required for DRBD. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
031ded71ee |
feat(seaweedfs): alert before an instance stops taking writes (#4573)
## What this PR does A SeaweedFS instance that runs out of room stops taking writes without anything paging. A volume server below its `minFreeSpacePercent` reserve turns every volume it holds read-only; before that, it can run out of slots, and a bucket whose volumes are all full then cannot get a new one, even with half the disk free. Backups archiving into the instance fail in turn, and a Postgres cluster doing continuous WAL archiving keeps the segments on its own volume until that fills too. The generic `KubePersistentVolumeFillingUp` alert does not catch this: it pages below 3% free, while a volume server stalls at its 5% reserve or, when slots run out, with the disk still half empty; and its early warning needs the disk to keep filling, which stops once writes fail. This PR adds per-instance rules to the SeaweedFS system chart, on signals the servers already publish and the platform already scrapes, rendered under the same `monitoring.enabled` gate as the chart's ServiceMonitors: - `SeaweedFSVolumeServerDiskLow` (warning): a volume server has less than 10% of its disk left above the reserve. - `SeaweedFSVolumeServerFull` (warning): a volume server cannot host another volume, because it is at the reserve or has no slot left. - `SeaweedFSWritesFailing` (critical): the master has failed to place a write in every 5-minute window for 15 minutes. Writes may still land in between; the usual cause is a bucket with no writable volume left that the master cannot grow. A new bucket's first writes fail a few picks while its volumes grow, which the 15-minute hold does not reach unless new buckets keep arriving. Every expression is pinned to the release namespace, because the root vmalert evaluates each instance's rules against every namespace's metrics. It also adds `docs/operations/seaweedfs-capacity-for-backups.md`: how to size an instance for continuous WAL archiving, what the rules mean, and how to grow an instance. The expressions were validated live rather than by unit tests, since the repository has no rule-evaluation tooling: on a test cluster, a standalone SeaweedFS 4.45 on a 1 GiB volume was scraped by the platform's vmagent and the rules rendered from this chart were evaluated by the root vmalert. All three loaded healthy and each reached `firing`: `SeaweedFSVolumeServerDiskLow` with 5.7% of the disk left above the reserve, `SeaweedFSVolumeServerFull` both at the reserve and on slot exhaustion, which stopped one bucket at 50% disk use, and `SeaweedFSWritesFailing` under continuous failing writes, resolving once they stopped. A new bucket's first successful writes on a free disk did fail a few picks, which is why that rule holds for 15 minutes. On the healthy platform instance all three stay quiet. helm-unittest pins what a render can break: the namespace selector of each rule, the shared label set of the two arms of the full-server rule, the gate, the critical rule's hold, and that alert templating reaches vmalert unrendered; each assertion was checked to fail when its line is mutated. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches a system chart template, its test and a document under `docs/operations/`; no app values, platform values, ApplicationDefinition, tooling or `hack/` path from the trigger map. ### Release note ```release-note feat(seaweedfs): alert when a SeaweedFS volume server nears its free-space reserve, cannot host another volume, or keeps failing to place writes, and document how to size an instance for continuous WAL archiving. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added namespace-scoped SeaweedFS monitoring alerts for low or exhausted disk capacity, unavailable volume slots, and write-placement failures. Capacity alerts use a 10% disk threshold, and alerts trigger after 15 minutes; write failures are marked critical. - **Documentation** - Added guidance on estimating SeaweedFS storage needs for Postgres backups, interpreting capacity alerts, and expanding storage when supported by the StorageClass. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
f6a615f693 |
fix(backups): scrub legacy plaintext passwords from CNPG Backup snapshots (#4558)
## What this PR does A CNPG `Backup` taken while the Postgres app still accepted `users[].password` copied that password into `status.underlyingResources`, where anyone with `get backups` in the namespace can read it. #4078 stopped new snapshots from carrying the field but left every existing `Backup` as it was (#4179). This adds `LegacyPasswordScrubber`, a leader-elected runnable in the backup strategy controller that, on start, lists `Backup` objects and removes `users.*.password` from every snapshot whose `kind`/`apiVersion` is the CNPG snapshot's. - **Why on every leader start and not a platform migration.** The migration hook runs pre-upgrade, while the old controller is still leader, so a `Backup` the old replica writes during the rollout would keep its password and the migration never re-runs. With `--leader-elect` on, the old replicas stop writing before the new leader's scrub runs, and a `Backup` brought back from an object-level export is cleaned on the next start too. On a clean cluster the whole pass is one List from the manager cache, which `BackupReconciler` already populates. - **Write shape.** The patch is a merge patch computed from the diff, so the request body only nulls the `password` keys and cannot overwrite status another writer changed since the List. The snapshot is handled as generic JSON (numbers kept as `json.Number`), so fields the current snapshot type does not know survive. - **Failure handling.** A `Backup` that cannot be decoded or patched is logged and skipped, and a failed List is logged too; `Start` always returns nil, because a returned error would stop the manager and take every backup down over one object. - **Restore compatibility.** Restore already ignored the key; a test decodes a scrubbed snapshot through `unmarshalCNPGBackupSnapshot` to pin that. - RBAC: `list` and `patch` on `backups.cozystack.io/backups` are already granted to the controller, so the chart is unchanged. - `docs/operations/backup-classes.md` no longer tells operators to delete pre-upgrade `Backup` objects, and says what the scrub does not reach (audit logs, etcd backups, external exports of the objects, Helm history of the init-script Secret), so a password once set through `users[].password` is still treated as exposed until rotated. The runnable carries a `TODO(#4179)` to remove it once upgrading from a release that accepted `users[].password` is no longer supported. Verification: `go test ./internal/backupcontroller/...` and `go build ./cmd/backupstrategy-controller/` are green. The new tests cover scrubbing with everything else kept (including an unknown field with a large integer), leaving non-CNPG, other-apiVersion, clean and snapshot-less Backups unpatched, and continuing past a Backup that fails to decode or patch. Disabling the delete, or returning on the first patch error, makes the tests fail. Not exercised on a live cluster yet. Fixes #4179 ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map against the diff: it touches only `internal/backupcontroller`, `cmd/backupstrategy-controller` and the in-repo `docs/operations/backup-classes.md`, with no package, values, schema, CRD, variant or asset change. The website carries no "delete pre-upgrade Backups" workaround this would make obsolete; its stale `users[name].password` rows are the #4078 follow-up already noted there. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(backups): the backup strategy controller now removes plaintext `users[].password` values left in `status.underlyingResources` of CNPG `Backup` objects taken before managed Postgres stopped accepting user passwords. It runs on every controller start and needs no action. It does not reach copies outside the cluster (audit logs, etcd backups, exported `Backup` objects), so a password once set through `users[].password` should still be treated as exposed until it is rotated. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * At controller startup, plaintext user passwords are removed from matching CNPG backup snapshots. Other snapshot data is preserved. * This cleanup does not affect external copies or remove older init-script manifests from Helm history. Treat passwords previously set through `users[].password` as exposed until rotated. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
4d3c8dd7bc |
docs(talos): describe the Talos v1.14 OOM trigger
Talos v1.14.0 dropped the global memory PSI clause from the default OOM trigger, so the page no longer matches the version Cozystack ships. The OOMConfig example now serves host nodes still on v1.13.x, which can shed the clause without upgrading. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
810d293421 |
fix(postgres): reassign dropped roles' objects to an unprivileged role
When a user leaves values, or a managed database role is orphaned, the init-job reassigned everything the role owned to the postgres superuser before dropping it. Objects a tenant role created should not become superuser-owned. They now go to "cozystack:orphaned", a NOLOGIN role whose attributes and memberships the init-job resets on every run; its name is outside the charset allowed for users and databases, so values cannot declare it. Because that one role receives the objects of every dropped role, functions it owns are switched to SECURITY INVOKER and, on PostgreSQL 15 and later, its views to security_invoker. The cost is that grants and memberships of the dropped role do not carry over, so a view or function it owned can lose access it had through them; keep the user in values, or move such objects to another owner first, to preserve it. The change is not retroactive: objects earlier releases reassigned to postgres stay superuser-owned, and the backup docs describe how to find and hand them over. The orphaned-roles pass now reassigns and drops in every connectable database, as the user pass already did, so a role owning an object outside the default database no longer fails DROP ROLE on every run. Role names read from the catalog now reach SQL as psql variables instead of being spliced in by the shell, and the queries are echoed after substitution so the log names the role being dropped. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Alexey Artamonov
|
35e3593c85 |
fix(seaweedfs): describe the writes-failing alert by what it measures
The rule fires when the master fails a volume pick in every 5-minute window for 15 minutes. That proves recurring refusals, not that no upload gets through: writes into other buckets can succeed in between, and a steady stream of new buckets, each failing a few picks while its first volumes grow, produces the same pattern. The master publishes no count of successful picks to tell the two apart, so the rule stays and its description and the operations page now say what it has seen. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
9b56e70ca6 |
feat(mongodb): support the system-bucket backup flow (useSystemBucket) (#3967)
## What this PR does MongoDB was the only `cozy-default` app with **no `useSystemBucket` flow**: its backup storage was always static on the `PerconaServerMongoDB` CR (bucket/endpoint/credentials from `backup.*` values plus a chart-emitted `<release>-s3-creds` Secret), so a tenant could not back up to the platform system bucket out of the box the way Postgres/ClickHouse/MariaDB/etcd do. This closes that gap so the default MongoDB backup-restore works OOTB. PBM takes `bucket`/`endpointUrl`/`region`/`prefix` as **static CR fields** (not secret refs), and the platform bucket name/endpoint are only known at BackupJob time — so, unlike ClickHouse's runtime `secretKeyRef` env, the chart cannot render them. The implementation mirrors the CNPG driver's pattern: - **Chart** (`packages/apps/mongodb`): a new `backup.useSystemBucket` flag keeps `spec.backup.enabled` (so the pbm agents run) but omits the static `storages`/`tasks`/`pitr` and the `<release>-s3-creds` Secret. Helm never owns `spec.backup.storages`, so a Flux re-render can't fight the driver. A `bootstrap.enabled + useSystemBucket` render guard mirrors Postgres, and `useSystemBucket: true` with `backup.enabled: false` is refused both at render time and by an `x-kubernetes-validations` rule on `backup` in the schema (the same mechanism as the `storageClass` immutability rule). Note on the schema rule: the aggregated apiserver does not evaluate CEL on application-backed resources yet (#2657), so today it is a UI/schema-level marker and the render-time refusal is what stops a Flux release; it also trips the API review gate (“validation rule added”), so this PR needs an API-owner approval on top of the code review. - **Strategy** (`cozy-default-mongodb`): now carries the S3 coordinates (`endpointUrl` already scheme'd by the `backupstrategy-controller.endpoint` helper, `credentialsSecret: cozy-backups-creds`, `prefix: <namespace>/<application>`). - **Driver** (`psmdbstrategy_controller.go`): SSA-injects `spec.backup.storages[s3-storage]` onto the live `PerconaServerMongoDB` at BackupJob time under a dedicated field manager (`cozystack-psmdb-backup-driver`) with `ForceOwnership`. Injection is **gated on the app's `backup.useSystemBucket` flag** (a legacy app leaves it false and is never touched; a useSystemBucket app is re-injected once per mint (once per BackupJob, plus once per storage-race re-mint), before its operator CR is created, so a later coordinate change catches up on the next job while a streaming dump is never patched under). The backup CR is minted only once the operator reports the storage pushed into pbm (`status.observedGeneration` caught up and `PBMReady` True): pbm writes to what its own config says, not to the spec, so a CR minted before that push would land at the previous coordinates while its Backup recorded the new ones; the job waits at `PerconaServerMongoDBPBMConfigPending` meanwhile, bounded by the backup deadline. Once a job has its CR, none of the mint-time reads or holds can fail it: the strategy and app reads the artifact still needs are bounded by the 24h running ceiling instead, and the cluster is not read at all **and on the release having rendered that flag**: the app CR is projected from the HelmRelease's desired values, so the flag reads true before helm-controller renders the revision, while the tenant's own storage, scheduled tasks and PITR stream from the previous render still sit on the cluster. As long as the cluster carries `spec.backup.tasks` or `spec.backup.pitr`, the driver does not inject and holds the BackupJob at `Ready=False PerconaServerMongoDBLegacyRender` (failing it at the deadline), so a wedged legacy revision never has its nightly dump and oplog redirected into the shared bucket. The backup snapshot falls back to the strategy coordinates when the operator status echo is empty; that fallback is gated on the CR's own `percona.com/delete-backup` finalizer (the flow the CR was minted on), not on the app flag re-read at completion, so a legacy backup never records the platform bucket and a flag flipped mid-dump does not change the artifact. The restore path rebuilds `backupSource` from it (the `cozy-backups-creds` reference is projected into the restore namespace before dispatch). - **Retention/cleanup**: on the `useSystemBucket` flow the driver owns the shared-bucket archive (the chart's psmdb `tasks[].keep` retention is gated out, and an on-demand backup is not covered by task retention). It stamps the operator's `percona.com/delete-backup` finalizer on each backup it mints **against a storage that carries the platform coordinates** — the strategy's bucket and `credentialsSecret`, read back from the live cluster after the injection — never on the flag alone: a strategy without an `s3` block, or a storage that points anywhere else, holds the job (`MongoDBStrategyHasNoS3` / `PerconaServerMongoDBStorageForeign`) instead of minting a finalized dump against a bucket the platform does not own. On Cozystack `Backup` deletion (by hand today; by platform retention once it lands, since a `Plan` only creates BackupJobs and never deletes a `Backup`) the driver deletes the operator `PerconaServerMongoDBBackup` CR so that finalizer frees the object — the finalizer's presence is the ownership marker cleanup reads back. That reading is unambiguous because cleanup resolves the CR by the name recorded in `driverMetadata` (the one the driver minted), not by scanning: the psmdb operator does stamp `percona.com/delete-backup` on its own `cron-*` task-minted backups, but those are never in scope. A legacy backup keeps the pre-existing hands-off contract, decided from the snapshot before any read of the operator CR. A terminating namespace or the `backups.cozystack.io/skip-artifact-cleanup: "true"` annotation strips the driver's finalizer and releases; a `Get` of the operator CR that fails with anything other than NotFound/NoMatchError still requeues, and the operator's own `release-lock` finalizer can hold the CR while the operator is down, so those two are what can keep a namespace in `Terminating`. `schedule` and `retentionPolicy` are inert on this flow (there is no engine-side retention and a `Plan` has no retention field yet); platform-level retention is tracked in [#4192](https://github.com/cozystack/cozystack/pull/4192) and teardown GC of orphaned backups in [#3966](https://github.com/cozystack/cozystack/issues/3966), so until those land bound `cozy-backups` with an S3 lifecycle policy. - **Bounds on the waits**: the injection race with the operator's cluster cache (an `unable to get storage` error on a freshly injected storage) is retried by deleting the errored CR and minting a fresh one, with the retry count kept on the BackupJob (`backups.cozystack.io/mongodb-storage-race-retries`) because the deleted CR is reaped before the next poll; after three retries the operator's error is terminal. A system-bucket dump that has started streaming is not cancelled on the 30-minute deadline, but fails after 24 hours in `running` (`BackupAbandoned` warning naming the operator CR, which is left in place as the handle to whatever pbm still writes). The deadline cancel of a not-yet-started CR pins its delete to the resourceVersion the live read answered for. - **RBAC**: `patch` on `perconaservermongodbs` for the injection (server-side apply), and `create`/`delete`/`patch` on `perconaservermongodbbackups` — `delete` to prune the shared-bucket archive on `Backup` deletion, `patch` to strip the driver's finalizer when releasing a wedged cleanup. Coverage: Go unit tests over `internal/backupcontroller` — the injection gate keyed off the app flag (legacy cluster never patched) and off the applied render (a cluster still carrying tasks or pitr is held, untouched), the SSA-injection precondition with the whole entry (per-application prefix included) asserted on the cluster, strategy templating per application, the finalizer refused against a storage without platform coordinates (strategy without `s3` over a legacy entry, foreign coordinates on the merged entry), the retry budget driven with the operator simulated reaping the deleted CR between polls, the restore snapshot fallback and credential swap across all four legacy/system flow combinations, the finalizer-owned cleanup with its escape hatches (skip annotation before any read, `NoMatchError` treated as gone, terminating-namespace release using the pre-read CR, a failed strip surfaced as an Event, a legacy CR never deleted, a legacy snapshot released without a read, a system-bucket snapshot reaching the prune), the deadline cancelling a not-yet-running operator backup only on a positive live read (including no live client at all, and a CR that moved since the read), and the running ceiling. Plus helm-unittest on both charts (chart skips the creds Secret and omits `spec.backup.storages`/`tasks`/`pitr` on `useSystemBucket` and keeps them on the legacy flow; the strategy renders the S3 coordinates and the `insecureSkipTLSVerify` knob). A live `useSystemBucket` backup+restore e2e against the platform bucket is a follow-up (needs the system bucket wired in CI, same status as the Postgres/ClickHouse useSystemBucket e2e). One item to confirm in e2e: the psmdb operator accepting `spec.backup.enabled=true` with an empty `storages` between install and the first BackupJob. ### Screenshots Not applicable (no UI changes). ### Downstream repositories The tenant-facing schema surface is one new field, `spec.backup.useSystemBucket`, on the MongoDB `values.schema.json` / CRD (regenerated), plus a validation rule on `spec.backup`. `terraform-provider-cozystack` hand-models MongoDB and may want to expose it, but it is a new optional boolean that defaults false (no drift for existing users), and the chart `README.md` regenerates on release. Leaving every box unticked per the trigger-map guidance so a maintainer decides on the provider 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 feat(mongodb): MongoDB backups now support the platform system bucket via backup.useSystemBucket, so the default cozy-default BackupClass works without per-app S3 credentials — the backup driver injects the system-bucket storage onto the cluster at backup time, once the release has rendered the flag. On this flow nothing prunes the shared bucket automatically yet (a Plan carries no retention field; tracked in #4192 and #3966), so an S3 lifecycle policy on cozy-backups is a prerequisite for enabling it, not a suggestion. useSystemBucket requires backup.enabled=true; the chart refuses the combination with backups disabled at render time, and the schema carries the same rule as a marker until the apiserver enforces CEL (#2657). A system-bucket dump still running after 24 hours fails its BackupJob with the operator backup left in place. A MongoDB restore now adopts the target cluster's S3 credentials only for a storage on the same bucket as the archive (it used to adopt them unconditionally); a legacy restore into a target on a different bucket that relied on one key pair covering both must keep or re-create the source release's -s3-creds Secret by name, and is held at Ready=False RestoreCredentialsMissing until it exists. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added opt-in system bucket mode for MongoDB backups. * Automatically injects platform bucket storage without application-managed S3 credentials. * Supports endpoint, region, path prefix, addressing, and TLS settings. * Manages shared-bucket backup artifacts when system bucket backups are deleted. * **Bug Fixes** * Improved restore credential selection to avoid cross-bucket mismatches. * Preserved backup metadata when storage details are unavailable. * **Documentation** * Documented configuration, scheduling, retention, PITR limitations, and restore guidance. * **Validation** * Prevented incompatible bootstrap and system bucket restore configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
1ead6ed90d |
docs(seaweedfs): size an instance for continuous WAL archiving
The default instance holds a little under 10 GiB, while a Postgres cluster archiving WAL into it keeps every segment for the whole retention window, 30 days by default. Nothing in the tree says how to work out what a bucket needs or what happens when it does not fit, so the first sign is usually a database that has stopped. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Mattia Eleuteri
|
1cefadcef6 |
docs(backups): say when a base backup gone from the bucket is still pinned
A restore leaves the base backup to the plugin only once the cnpg.io/Backup is gone. A base backup removed from the bucket by a lifecycle rule or by hand keeps its cnpg.io/Backup until the plugin's next maintenance pass on the source cluster, and indefinitely once that cluster no longer runs under the same UID. A restore in that window fails after the purge and on every retry, with nothing on the RestoreJob to unpin it; deleting the cnpg.io/Backup is the way out and does not touch the object store. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
64e837727c |
docs(backups): say what recovery does when the latest timeline is a sibling
recovery_target_timeline latest targets the highest consecutively numbered history file, without checking that it descends from the base backup. On an archive holding sibling branches that can be another branch, and recovery then stops with "requested timeline N is not a child of this server's history" rather than following the backup's own line of descent. The page also names the second case where a restore does not pin its base backup: a cnpg.io/Backup removed by retention. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
6999609585 |
docs(backups): bound Postgres PITR by the last archived transaction
The recoverable window was described as ending at the newest archived WAL segment, and the safe fallback as "at/before the most recent completed backup". A time-targeted recovery ends just before the first transaction after the target, and a segment switch or a backup is not one, so on a database at rest every instant after its last write fails, including the end of a later backup. Aiming at a backup's stoppedAt also misses it: stoppedAt is truncated to the second and the plugin compares barman's end time to the microsecond. Describe the bound the recovery actually has, how to read it from the archive, the one-second margin on stoppedAt, what pinning the base backup changes, and why archives with several timelines need it. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Andrey Kolkov
|
19764c4bba |
fix(backups): scrub legacy plaintext passwords from CNPG Backup snapshots
A CNPG Backup taken while the Postgres app still accepted users[].password copied that password into status.underlyingResources, where anyone with `get backups` in the namespace can read it. Dropping the field stopped new snapshots from carrying it but left every existing Backup exposed. The backup strategy controller now removes users.*.password from CNPG snapshots when its leader starts. It runs on every start rather than once, so a Backup written by a pre-upgrade replica during the rollout, or brought back from an object-level export, is still cleaned; on a clean cluster it is a single cached List. The patch is a diff-computed merge patch that only nulls the password keys, and a Backup that cannot be decoded or patched is logged and skipped instead of stopping the manager. Fixes #4179 Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
6555ae16b8 |
fix(mongodb): start a backup only once the operator has pushed the storage into pbm
The wait after a storage injection watched the cluster's spec, which is not what pbm writes to. At the pinned operator version the backup reconciler hands pbm only a storage name and records status.s3 from the spec entry it read, while the storage pbm uses is whatever the cluster reconciler last pushed into pbm's own config; that push flips PBMReady False and back to True and the reconcile stamps observedGeneration when it ends. A backup started between the apply and that push dumped to the previous coordinates while its Backup recorded the new ones, so both its restore and its prune pointed at the wrong bucket. The wait was also no wait at all: the status write that named it re-queued the job at once, and the live read always showed the apply. The mint now waits for observedGeneration to reach the cluster's generation and for PBMReady to be True, and fails at the backup deadline if the operator never gets there. The strategy, app and cluster reads still ran ahead of the operator CR lookup and were bounded by the backup deadline, so a strategy absent for one reconcile 40 minutes into a streaming dump failed the job and left the finalized CR unreachable. The CR is resolved first now; with one in hand the cluster is not read at all, and the reads the artifact needs are bounded by the running ceiling, past which the job fails naming the CR left in place. The settle check treated a field the entry lacked as agreement, so a strategy that newly set forcePathStyle minted in the same pass as the apply that changed addressing. A missing field now counts as its zero value, which keeps an entry the operator rewrote without an empty region settled while catching a value the apply adds; the storage type is compared outright. The runbook still said the finalizer keeps a Plan from accumulating objects, the RBAC comment still said the timeout cancel runs on either flow, and four wait messages read a duration as a delay. All corrected; the runbook also describes the pbm wait. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
841933dcec |
docs(mongodb): stop saying a Plan prunes the shared bucket
A Plan creates BackupJobs and never deletes a Backup, and PlanSpec has no retention field, so nothing on the platform bounds a MongoDB archive on the shared bucket today except an S3 lifecycle policy or a manual Backup delete. The runbook's tuning section said a Plan does, two hundred lines after the same page said a Plan carries no retention, and three driver comments named "Plan retention" as the path that reclaims the bucket. An operator reading the first sentence would skip the lifecycle policy the release note calls a prerequisite. All of them now say what deletes a Backup today, and that the finalizer frees the object whenever one is deleted, whatever does the deleting. The settle check after an apply compared only the fields the desired entry carries, so an entry written with forcePathStyle while the strategy still set it counted as settled once the strategy dropped the field, and the apply that removed it minted in the same pass. The check now treats a field the apply is about to remove as a change too. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
707e38d17d |
fix(mongodb): keep every mint-time hold off a job that already has its CR
The injection and the ownership switch were gated on the job having no operator CR yet; the precondition between them was not, and neither was the pair of holds ahead of them. Each of those fails the job at the 30-minute deadline, so a tenant disabling backups, or the platform dropping the strategy's s3 block, while a system-bucket dump streamed failed the BackupJob at 30 minutes with the CR left finalized and no Backup object to reach it, bypassing the 24-hour ceiling that exists so a shared-bucket archive is never stranded. Everything up to the mint now runs only while there is no CR; a job with one goes straight to the state switch. The settle wait after the apply was keyed on the storage key being present, so a change to the strategy coordinates rewrote an entry the operator had cached and minted in the same pass, the case the wait exists for. It is now keyed on the entry's content: only an apply that changes what the entry says costs a poll. The mint-time reads (the render's tasks and pitr, the storage entry) came from the manager's cache, which can lag a re-render back to the legacy flow by one reconcile and show the flag true with the tasks not yet visible. With a dynamic client they now come from the apiserver, the way the deadline cancel already reads the operator CR. Tests age the existing-CR case past the deadline and vary the three holds, pin the coordinate-change wait, the live read and the term that keeps the storage-race retry from swallowing an unrelated operator error. The runbook's tuning section no longer summarises the defaults as 30-day retention: only the CNPG and Velero strategies carry one. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
46a8bd7363 |
fix(mongodb): apply the storage once per job, and pin the terms that decide ownership
The system-bucket storage was applied on every reconcile of a job, not once per job: with the operator CR already minted nothing returned before the apply, so a streaming dump saw an SSA patch every poll for up to a day, and the only thing keeping that patch off a cluster that had re-acquired the chart's tasks and pitr since the mint was a predicate term no test held. The storage a CR names is fixed for its dump, so the apply now runs only before the CR exists; the legacy-render hold covers that window, and a test pins that an existing CR is never injected over. The other predicate that decides whether a Backup owns an archive compared the live entry's bucket and credentialsSecret against the strategy, and only the bucket half had a fixture. The credential is what tells a tenant's entry from the platform's when a bucket name collides, so a case varying only that term joins the foreign-bucket one. The cleanup shortcut's snapshot-kind term and the 24h running ceiling are pinned the same way, the latter as an absolute boundary pair. The generated field description said the schema rejects useSystemBucket without backup.enabled. The aggregated apiserver does not evaluate CEL on application schemas yet, so the render-time refusal is the one that holds; the description and the runbook now say so. The runbook's restore paragraph still described the unconditional credential swap this branch narrowed, exempted MariaDB from a retention warning its shipped strategy does not earn, and pointed at a Plan retention field that does not exist; the example README and a dispatcher test's comment still carried the pre-branch claim that the driver never injects and never deletes. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
a6bb0556fc |
fix(mongodb): own archives by where the storage points, and bound the open loops
The prune finalizer on a driver-minted PerconaServerMongoDBBackup means the platform owns the archive: cleanup deletes the operator CR on that marker and the operator removes the object from storage. It was stamped from the app's useSystemBucket flag, which says nothing about where the storage the CR names actually points. With a strategy carrying no s3 block nothing was injected, the presence-only precondition passed on a legacy tenant-bucket entry, and the CR went out finalized against the tenant's own bucket; deleting the Cozystack Backup then deleted the tenant's archive. The finalizer now follows the storage's coordinates as read back from the live cluster after the injection, and a strategy without s3 coordinates, or a storage pointing anywhere else, holds the job instead of minting. The injection keyed on the same desired value. The app CR is projected from the HelmRelease's spec.values, so the flag reads true before helm-controller renders the revision, while the tenant's own storage, scheduled tasks and PITR stream stay on the cluster. Injecting over that entry redirected the tenant's nightly dump and oplog into the shared bucket under platform credentials. A cluster still carrying tasks or pitr is now left alone and the job held until the release renders the flag, and the schema carries a rule marking useSystemBucket without backup.enabled invalid, the pair the chart already refuses at render. The storage-race retry counted surviving operator CRs against its budget, but an errored CR is reaped the moment it is deleted (the operator drops delete-backup without pruning on a non-ready CR), so the count never left one and the job minted a CR per poll for the whole deadline. The count now lives on the BackupJob and survives the reap. A system-bucket dump in running had no bound at all: a wedged pbm agent left the job Running forever and a nightly Plan stacked another beside it. It now fails after 24 hours with the operator CR left in place as the handle to whatever pbm still writes. The deadline cancel pins its delete to the resourceVersion the live read answered for, and the artifact-retry failure names the window that was actually in force. Tests pin the guards that were exercised only in the harmless direction: a system-bucket snapshot reaching the prune, the resolved prefix reaching the cluster, a missing live client deferring the cancel, and the finalizer refused against foreign coordinates. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Alexey Artamonov
|
ce74fdeea9 |
docs(backups): describe restored credentials and pre-upgrade residue
Say that a restored copy converges its application passwords shortly after recovery, that MariaDB backups leave out the grant table, and that dropping the password field does not rewrite copies made before the upgrade: old Backup snapshots and Helm revisions still hold the password that stays live. The credentials projector comment counts only the engines that still take a password from values. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Andrey Kolkov
|
4233e90792 |
fix(mongodb): decide a backup's flow from its own CR, not the app flag
The opt-out refusal ran before the lookup of the job's existing operator CR, so it was a per-poll gate rather than a mint-time one: flipping backup.useSystemBucket to false while a dump streamed routed every later reconcile into the refusal, failed the job at the deadline and left the CR minted under the old flag streaming into the platform bucket with a finalizer no Backup would ever resolve — the stranded archive the running exemption exists to prevent. The refusal now applies only while the job has no CR, and everything after the mint reads the flow off the CR's delete-backup finalizer, which is fixed at mint time, instead of the mutable app flag. A late dump on the legacy flow is likewise handled by its CR's flow: the deadline fails the job and leaves the operator CR, as before this driver existed. The refusal also identified the injected entry by bucket name and told the operator to wait for the chart to re-render it, which the docs say never happens. The entry is now recognised by its credentialsSecret, the one field on it the driver chose (bucket names collide with an admin's external bucket and drift when the platform bucket is re-provisioned), and the message names the manual removal the docs prescribe. At the pinned operator `requested` is set right after the command is sent to pbm, so deleting a CR in that state cannot recall it and a late dump would land with nothing to own it. Only an unobserved or queued CR is cancelled; a requested one is left as the handle to whatever pbm writes, and the job fails. The credential pre-check failed at once where every sibling precondition waits for the restore deadline, and it re-ran against a restore already replaying, so a Secret pruned mid-replay marked an in-progress restore Failed. It now waits like its neighbours and is skipped once the operator restore exists. The status-write suppression on the waiting helper is pinned by counting status updates across repeated reconciles. The legacy deadline's justification claimed psmdb task retention covers the archive; the operator prunes only its own scheduled backups, so the object is the tenant's to reclaim, and the comments say that instead. The bootstrap refusal names the value that actually clears it (bootstrap.enabled is not reset after a bootstrap restore). Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
42a29e3597 |
fix(mongodb): keep the legacy deadline on a running dump and name the wait
Exempting state=running from the wall-clock deadline was justified by the shared bucket: on the useSystemBucket flow a failed BackupJob leaves pbm streaming an archive no Backup object represents. The exemption was applied to both flows, though, so a legacy dump whose operator or node died mid-run went from failing on the deadline to polling forever with no condition and no Event. Gate the exemption on the flow: a legacy archive lands in the tenant's own bucket under psmdb task retention and the pre-existing contract failed the job while leaving the operator CR alone, so that is restored. On the system-bucket flow the wait is now named on the BackupJob (Ready=False with the start time) instead of being silent; the waiting helper writes status only when the condition changes so a dump that runs for hours does not cost a status write per poll. The point-in-time refusal read the snapshot flag with the decode error discarded, so an undecodable snapshot fell open and a recoveryTime restore of a system-bucket archive reached the operator to die on a pbm-level message. Treat a decode failure as "flow unknown" and refuse. Adopting a target credential only for a storage on the same bucket left the source's own reference in place when nothing matched, and opting an app into the system bucket prunes exactly that Secret while older Backups still name it. Check the reference resolves before the operator sees it and fail with a named reason instead of a missing-Secret error deep in the restore. Opting back out stops the injection but leaves the driver's storage on the live cluster until the chart re-renders; a BackupJob in that window passed the presence-only precondition, minted a CR without the delete-backup finalizer and wrote an object nothing owns into the platform bucket. Refuse to mint onto a storage whose bucket is the platform's while the flag reads false, bounded by the deadline like the other waits. Also realign the snapshot literal for gofmt, and fix two doc slips: the pbm agents start on backup.enabled alone (a declared storage is needed only to service a backup), and the opt-in cross-reference targeted the parent heading instead of its subsection. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
a89321fa8c |
fix(mongodb): keep retentionPolicy a genuine no-op, and cancel a timed-out backup so it cannot orphan an archive
Every prose surface (the useSystemBucket field, the retentionPolicy field, and backup-classes.md) tells a tenant that schedule and retentionPolicy are ignored on this flow, and cozyvalues-gen copies that into the schema, README and the dashboard. A render-time reject of a non-default retentionPolicy contradicted all of them: a tenant who followed the documented opt-in with a retentionPolicy still set got a failed render and a HelmRelease that stopped converging, pointed at the very doc that says the field does nothing. Drop the reject so retentionPolicy is inert exactly as documented, handled the same way as schedule (gated out, no error). The absence of retention on this flow is real but belongs to a follow-up, so the docs now point at the BackupRetentionPolicy design (#4192) and the teardown-GC issue (#3966), and recommend an S3 lifecycle policy until they land. Cancel a timed-out operator backup instead of only failing the BackupJob. The deadline fires for a backup the operator has not begun streaming; but `waiting` is the state pbm reports for a backup queued behind another, and the operator starts it once the slot frees — after the BackupJob is Failed and never reconciled again, writing an archive into the shared bucket that no Backup object represents. The deadline branch now deletes the operator CR before failing, which prevents that late run entirely: at v1.22.0 the operator returns for a CR with a deletion timestamp before it dispatches to pbm, so nothing is written. The delete is best-effort so it never blocks the failure. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
d58322e253 |
fix(mongodb): let the system-bucket cleanup escape hatch survive an unreadable operator CR
The two give-up paths sat behind the operator-CR Get, so a Get that hard-fails the way a removed CRD (NoKindMatchError) or a revoked verb (Forbidden) does returned the error and never reached the release — leaving a Backup that keeps its cleanup finalizer and requeues forever, exactly in the situation someone sets the escape-hatch annotation to get out of. Read the skip annotation before any apiserver call (as the Redis cleanup does) so it always releases, and classify NoMatchError alongside NotFound (as the Job/Redis cleanups do) so a missing CRD reads as "gone". releaseMongoDBCleanup now takes the CR name and treats every apiserver step as best-effort, so the release is never blocked by the same unreachable CR. Also correct the docs, which promised an automatic release when "the operator is down": only a terminating namespace or the annotation release. A down operator in a live namespace keeps the Backup in Terminating as a visible signal, which is what the code does. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
542f42ef67 |
fix(mongodb): make the system-bucket cleanup teardown-safe and single-source its ownership
Four issues in the system-bucket cleanup path. Teardown wedge. The cleanup requeued forever waiting for the operator to prune, and the escape-hatch annotation released the Cozystack Backup without touching the delete-backup finalizer the driver stamps on the operator CR. So a namespace teardown, where the operator's delete-backup step cannot run (it execs against a pbm agent that is going away), left the operator CR — and the namespace — stuck in Terminating with no way out but hand-patching finalizers. Cleanup now detects a terminating namespace (as Redis/Rabbitmq do) and, on that or the annotation, strips its own finalizer and releases, leaving the object for a bucket lifecycle policy. Needs patch on perconaservermongodbbackups for the finalizer strip. Ownership was read from two places written at different times: the finalizer (stamped when the operator CR is created) and a driverMetadata flag (written when the artifact is created, a later reconcile). If the app's useSystemBucket flag flipped between them they disagreed. Read ownership from the finalizer on the one CR named in driverMetadata (the one the driver minted): resolving it by name, never by scan, keeps the operator's own cron-* task-minted backups (which do carry percona.com/delete-backup at v1.22.0) out of scope, so the finalizer on that named CR is an unambiguous single marker. Drops the driverMetadata flag. Injection error was terminal. A transient server-side apply — an apiserver hiccup, or a conflict while the operator writes the same cluster — failed the BackupJob outright. Requeue with backoff instead, bounded by the existing deadline, matching the cluster/app read errors alongside it. Injection ran before the enabled check, mutating a cluster that can never back up. Gate it on backup.enabled so a disabled cluster is left untouched. Also: reframe the restore S3 backfill as the defensive guard it is (the pinned operator always echoes status.s3, so the live path normally carries coordinates), and correct the psmdbtypes doc that still claimed the driver never patches the operator CRs. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
1d05dafb2e |
fix(mongodb): own and prune the system-bucket archive, and heal the restore read path
Two correctness gaps on the useSystemBucket flow. Retention. Gating spec.backup.tasks out of the chart also dropped the keep that psmdb task retention used, and an on-demand PerconaServerMongoDBBackup is not covered by task retention, so nothing pruned the shared cozy-backups bucket — a retention-swept Plan grew it unboundedly for every tenant at once. Own the archive the way the Redis/Rabbitmq drivers own theirs: stamp the operator's percona.com/delete-backup finalizer onto the backups minted on this flow, and on Cozystack Backup deletion delete that operator CR and wait for the finalizer to free the object before releasing. A legacy backup (tenant's own bucket) keeps the pre-existing hands-off no-op. Adds the delete verb on perconaservermongodbbackups and a skip-artifact-cleanup escape hatch for a wedged operator, matching the sibling drivers. Also drops the unused update verb from the psmdb cluster RBAC (server-side apply is a patch, not an update). Restore. resolveMongoDBBackupSource returned early off the live operator Backup CR whenever it reported a destination, copying status.s3 as-is. On this flow the operator can echo a destination with status.s3 empty (the storage was injected, not declared), and the target has no declared storage to resolve from either, so the restore stranded with no coordinates. Backfill a missing live S3 block from the snapshot the BackupJob recorded. Also correct the precondition godoc: the pbm agents start on spec.backup.enabled alone; the declared storage is required by the backup CR's resolution, not by the agents. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
47fe7c9630 |
test(mongodb): pin the restore credential swap and document the opt-out
The restore-time decision to re-point a backup's credentialsSecret at the
target cluster lived inline in reconcileMongoDBRestore with no test, so the
same-bucket guard that keeps a cross-flow restore from authenticating
against the wrong bucket could be dropped unnoticed. Extract it into
mongodbRestoreCredentialsSecret and cover all four
{legacy,system}x{legacy,system} combinations; neutralising the bucket
match now reddens the suite. The projected-secret skip is kept as well: a
target storage sharing the platform bucket but naming a different secret is
better restored with the platform-projected cozy-backups-creds than with
that secret.
Document the reverse toggle: flipping useSystemBucket true->false leaves
the driver-injected s3-storage on the live PerconaServerMongoDB under the
driver's field manager, which the chart does not own and never prunes. It
is inert once backups are disabled; the operator must delete it by hand to
return the release to the legacy flow.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
|
||
|
Andrey Kolkov
|
9545bee233 |
fix(mongodb): correct system-bucket prose, pin strategy rendering, make restore credential swap flow-aware
Several comments and the useSystemBucket field description had drifted from
what the code does.
The strategy header still said the $bucketName gate was "not because it
needs the value" and that the sibling strategies needed the bucket name
while this one only asked for ordering, but the strategy now renders
bucket: {{ $bucketName }} for the injected coordinates, so it needs the
value; the precondition comment claimed the s3-storage entry exists on
backup.enabled=true, which is only the legacy flow. The rendering-test
comment carried the same "needs no bucket coordinates" claim. The
useSystemBucket field description said injection happens "at first BackupJob
time" (it now runs on every BackupJob) and that backups scope to
<namespace>/<release> (the prefix templates the application name, not the
release), and it ran ~700 chars of implementation detail into the CRD that
backup-classes.md already carries; trimmed and corrected, then regenerated
schema/README/types/cozyrds. Same <release> slip fixed in the driver
comment and the docs.
Nothing asserted the new strategy rendering. Added the MongoDB row to the
endpoint-form suite (s3.endpointUrl, scheme preserved both ways) and field
assertions to the rendering suite — bucket, endpointUrl, region,
credentialsSecret, forcePathStyle, and prefix, which must reach the CR as a
literal go-template string so the driver can template it per-application;
a stray brace would empty it with every doc-count test still green.
The restore credential swap broke across flows. It replaced the source
credentialsSecret with whatever the target declared, comparing no buckets,
so a system-bucket backup restored into a legacy target kept the platform
bucket against the tenant's own -s3-creds, and a legacy backup restored
into a system-bucket target got cozy-backups-creds against the tenant
bucket - both AccessDenied. Skip the swap when the source already names the
platform-projected cozy-backups-creds (it outlives the source app, nothing
to repair), and only adopt a target credential whose storage is on the
same bucket as the archive. psmdbStorageS3 now reads .s3.bucket alongside
.s3.credentialsSecret; TestPsmdbTargetCredentialsSecret covers the match,
the cross-bucket refusal, and the empty-bucket no-op.
Also drop the post-apply re-Get: the typed SSA Patch decodes the server's
merged object back into the passed struct, so the precondition reads the
injected storage from it instead of a follow-up Get that hit the informer
cache and briefly reported Ready=False on the happy path.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
|
||
|
Andrey Kolkov
|
4ea4888685 |
fix(mongodb): key system-bucket injection off the app flag, not live storage
The MongoDB backup driver decided what to inject and what to record from the live cluster's storage set rather than from the app's opt-in flag. That had three consequences. The snapshot fallback fired for legacy apps too. Its gate was `snap.S3 == nil && rendered.S3 != nil`, but the coordinates moved onto the shared cozy-default strategy, so rendered.S3 is now non-nil for every MongoDB app. A legacy backup written to the tenant's own bucket got its snapshot rewritten to the platform bucket, sending restore to the wrong place. Gate the fallback on the app's backup.useSystemBucket flag instead, threaded through createMongoDBBackupArtifact, and pin the legacy case. The injected storage never caught up with the strategy. Injection was gated on the named storage being absent, so it ran once per cluster and froze the coordinates at the first backup; a later endpoint/region change or bucket re-provision was a silent no-op, and Helm could not repair it because the useSystemBucket chart renders no storages. Key injection off the app flag and re-apply on every BackupJob (the prefix is deterministic, so this never splits the archive the way CNPG's serverName would). Add UseSystemBucket to mongodbapp.MongoDBSpec so a legacy app is protected by the flag, not by whatever happens to be on the cluster. The docs said the opposite of what the driver does. backup-classes.md claimed "the MongoDB driver cannot inject its S3 target per-backup"; correct it and add an opt-in-to-the-system-bucket section that names which values stop being read and that scheduled backups + PITR must move to a Plan. Fix the endpoint table row, the tuning reference, the RBAC comment that said the driver never patches, the strategy header/inline comments, and the example comment. Also assert notExists on spec.backup.pitr in the helm test, and lift the duplicated cozy-backups-creds default into a const. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Aleksei Sviridkin
|
a2db2aeb8c |
fix(seaweedfs): raise the number of buckets a volume server can serve
A volume server started with -max 0 takes a slot count derived from disk rather than a fixed one, replication 001 spends a slot on each of the two servers per logical volume, and a collection claims COPY_2 volumes whenever it grows. So the bucket ceiling is set by the product of the size limit and COPY_2, and moving either number alone moves it. The mechanism is upstream in MaybeAdjustVolumeMax (weed/storage/store.go) and findVolumeCount (weed/topology/volume_growth.go). Do not divide these out of the nominal PVC size, because the count follows the space the filesystem makes available and not the volume's advertised size: the shipped 10Gi volume PVC was seen to give nine slots per server at the upstream 1000 and 19 at 500, which puts 400 at 23 to 24 by the same division, or eight collections against two. The span is the observation's own, since the free space behind 19 at 500 fits more than one filesystem geometry and those divide differently at 400; eight either way, because a collection that cannot get a whole batch of COPY_2 keeps the volumes it did get and writes into them. The division itself is measured: on a 1 GB image the master reports max as the filesystem's available space over the limit in every run, 25 slots and nine collections at the 40 and 3 that scale to this pair. Two is what the platform's own cozy-backups bucket and a single tenant bucket already use. Past that every S3 PUT to a new bucket times out while the disk still reads as nearly empty, and no COSI object reports anything wrong, because the claim and the access are both granted. 400 and 3 make the budget 1200 MB per collection. Splitting the reduction across both numbers rather than taking it out of the size limit alone keeps the cost off the curve that grows: open descriptors track stored bytes divided by the limit, so a 10 TiB server keeps roughly 52k of them at this limit against 21k upstream, where moving the size limit alone would have put it at 105k. A .dat and an .idx stay open per volume; the .vif beside them is written and closed. The ceiling reached is the same either way. Both figures describe the shipped replicationFactor of 2. The master picks its growth count from the replication the filer asks for, so at 1 and 3 copies it reads COPY_1 and COPY_3 instead and this COPY_2 does not apply; lowering the size limit still raises the ceiling there. The size limit was out of an operator's reach: it lived in the data-plane chart, which is pulled by ExternalArtifact from a component that declares no install section, so a Package-level values override for it is dropped before any merge. The app chart now declares master.volumeSizeLimitMB and sends it on every release, and 400 is set there. The data-plane chart keeps upstream's 1000 as its fallback on purpose. A platform upgrade can re-render that release from its artifact before the parent has sent anything new, and a lower fallback would then reach the master first and drop the limit ahead of an operator's own setting. COPY_2 is sent by nothing and lives in the data-plane chart, so it does land in that window, where a grow costs 3000 MB against the 6000 MB it cost before. The app chart also writes maxVolumes into dataDirs on every release, and because dataDirs is a list Helm replaces the data-plane chart's copy wholesale, so the zero that makes the slot count auto-derived at all comes from there. Losing it drops the server to the subchart's fixed seven without failing any render. The field is bounded at both ends, because either end takes an instance down. Zero is not a smaller limit but an off switch: the volume server leaves its slot calculation untouched at zero, keeps reporting no capacity, and nothing is ever grown. Past 30000 the master does not start at all. The bound sits in the values schema rather than in a template guard, so Helm refuses the value before any template runs. The apps API does not: it reads that schema only for defaults, so it accepts a value out of range, and the release then fails to render. An explicit null gets past both bounds, because Helm drops the key before it validates, and would render an empty -volumeSizeLimitMB= that the master exits on at start, measured on the shipped image, so the template requires the value. No bound against the disk goes with it, though the field invites one. A limit at or above the volume's own size does not break an instance: MaybeAdjustVolumeMax floors the slot count at one, measured on 4.31 with a 255 MiB disk against a 1000 MB limit, which reported Max 1 and served a write it then read back. Such a server is capped at a single slot rather than dead, so refusing it at render would be a capacity policy that stops a working release on every reconcile, not only on the upgrade. The field's description carries the relation instead, where the value is chosen. The growth count is deliberately not exposed beside the limit. The master picks which of its four growth counts to apply from the replication the filer requests, and the filer takes that from replicationFactor, so a field wired to COPY_2 would do nothing at 1 or 3 copies while still appearing to be a setting. A knob that is silently inert for two of the three supported values is worse than no knob. Two further prices. A collection absorbs the budget before it needs another grow, so growth requests arrive five times as often as at the upstream pair. A collection also starts with half as many writable volumes, and a volume server takes the writes into each volume one at a time (syncWrite holds that volume's dataFileAccessLock), so a heavily parallel writer spreads over fewer volumes until the collection grows again; that holds for instances kept at 1000 too. And lowering the limit on an instance that already holds data carries the other one: its volumes between the new limit and the old go oversized on the next heartbeat and leave the master's writable set, and each affected collection needs one successful grow before its next write lands. Nothing is lost and the volumes stay readable; the room left inside them for further writes is given up and does not return as capacity. Those grows compete for one pool, and the pool is smaller than it looks. An oversized volume is charged no headroom, while one below the limit is charged all of its unfilled room; a grow then claims a whole COPY_2 batch and takes as much of it as is there, and the volumes it creates are empty and charge their headroom straight back. Measured on a local SeaweedFS 4.31 setup: two oversized collections with two free slots between them, the first to write took both, and the second stayed refused with the disk still reporting a quarter of itself free. Collections that need no grow are not free either, since their volumes below the new limit were created six at a time and each can hold back a whole limit. Also measured there, with the limit and batch scaled down: a server with free space for one batch per collection refused the grow its one affected collection needed, because a second, nearly empty collection held the room, and the same free space without it let the grow through. The minFreeSpacePercent reserve only comes into it on a server whose writable volumes have all passed the limit, where nothing is charged headroom: from 8000 MiB of filesystem upward, a 10Gi PVC measuring 9979, five percent is at least 400 MB, and such a server then refuses a grow only once its disk is down to the reserve. Everywhere else the headroom charge decides, and a refusal can arrive with the disk far from full whatever the reserve. Deleting a collection gives the room back a slot per volume, however little it stored, since the headroom charged to those volumes leaves with them. So, on a variant that runs platform migrations, the upgrade does not change the limit on an existing instance unless its operator already set one, which the previous chart accepted and ignored. Platform migration 57 writes master.volumeSizeLimitMB: 1000 into every existing SeaweedFS instance that has not chosen a limit, and 400 applies only to instances created from here on. It leaves alone only an instance whose master is not an object, which no v1 chart renders, and names that one in its log. The default variant runs no platform migrations, so a cluster there that installed SeaweedFS by hand gets no pin, and the document below gives the same patch to apply before upgrading. A chosen limit is a whole number the new chart accepts; anything else counts as not chosen and is replaced with 1000, which is what the previous chart ran in its place, since it ignored the key and the new chart would refuse to render it. A valid value already set is kept, and since the previous chart ignored it, the upgrade is when it starts to apply; the document below lists the ones that change the limit ahead of it. Raised values are listed as well: existing buckets keep writing, but every volume below the new limit is charged up to it, and on a local SeaweedFS 4.31 setup seven nearly empty volumes left no free slot once the limit went up tenfold, so a new bucket was refused. The value goes into the application's HelmRelease, whose spec.values is the application spec, for an instance created through the API and for one the tenant module renders alike. The tenant module never renders spec.values on that release, and a Helm upgrade leaves a key alone that neither manifest carries: measured with Helm 4.3.0 on client-side, server-side and forced server-side apply, and read in createPatch at the v4.1.1 that helm-controller v1.5.0 runs. Where the migration runs, it is a pre-upgrade hook of the platform release, and the cozystack-packages source every application chart comes from is a manifest of that same release, applied only after its hooks; so no instance meets the new chart unstamped, and a failed migration stops the platform upgrade before that source moves. That holds while the platform source is pinned by digest, as it is out of the box; one that follows a tag, a semver range or a branch lets source-controller move cozystack-packages on its own schedule. Until cozystack-packages moves, each patched release keeps its old chart, which ignores the key, since no v1 release reads it or closes the master object in its schema. An instance kept at 1000 still gains, because at a replication factor of 2 a grow now claims three volumes rather than six, so it has room for roughly twice as many new buckets. Lowering an existing instance is left to its operator, and docs/operations/seaweedfs-lowering-the-volume-size-limit.md is the check for it. It lists each server's volume files and adds one limit for every volume below the limit and a batch for every collection with no volume at or below 90 percent of it, since the master grows such a collection on its own schedule without a write; measured the same way, a collection with every volume in that band and no writes got two new volumes per server within one pass. Where each disk type is served by as many volume servers as the replication factor, as the shipped two at a factor of 2 are, each collection's volumes sit on every server of that type, and that bounds the claim from above without assuming how the collections grew. The master places a new volume on any server of the requested disk type with room rather than where its collection already lives, whichever group or pool that server belongs to, and it reads an empty disk type as hdd; under MultiZone it also grows each zone on its own. So the document keeps zoned instances, and any disk type served by more servers than copies, at 1000. BREAKING CHANGE: on the default variant, which runs no platform migrations, set master.volumeSizeLimitMB to 1000 before upgrading on every SeaweedFS instance that has no valid value and whose master is an object, with the step in docs/operations/seaweedfs-lowering-the-volume-size-limit.md. On the variants that run platform migrations, migration 57 does this. If the step is skipped, every such instance drops to 400 on the upgrade: each of its volumes at or above 400 MB leaves the master's writable set, and a bucket whose volumes have all crossed that line takes no writes until it can grow. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrey Kolkov
|
38d6c4cf58 |
fix(backups): bound Kafka metadata run at the Job layer, not a controller wall clock
A controller-side running-Job deadline was enforced in reconcileKafka / reconcileKafkaRestore: at kafkaDefaultBackupDeadline from StartedAt the BackupJob/RestoreJob was marked Failed. But the batch Job carried no activeDeadlineSeconds and markBackupJobFailed only writes status, so the Job kept running, finished, and its final `s3 PUT` uploaded the object — while the terminal phase meant no Backup was ever created and cleanup (keyed on Backup deletion) never ran. Every timed-out run thus orphaned a uniquely-keyed object, and a cluster whose export legitimately exceeds the deadline could never produce a usable backup. The shipped docs also claimed the run "keeps the BackupJob Running ... rather than failing on a wall clock", which the deadline made false. Move the bound onto the Job as activeDeadlineSeconds (setKafkaJobDeadline): Kubernetes fails an unschedulable or wedged Job as DeadlineExceeded and kills its pod before the upload, so a timed-out run lands in the JobFailed branch cleanly and leaves no object behind. Make it operator-tunable with the `backupTimeout` BackupClass parameter (a Go duration), which round-trips through the Backup so a restore is bounded by the same value; the readiness-precondition wait honours it too. Correct the docs. Also requeue on a transient broker-image List error instead of failing the whole run: resolveKafkaBrokerImage now wraps a sentinel (errNoKafkaBroker) so only a Ready cluster with no listable broker is terminal. Tests: pin the deadline on the built Job, the backupTimeout override, a JobFailed run marking the owner Failed, and the transient-List requeue. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
14c6af122e |
fix(backups): restore Kafka S3 port workaround and hold the script with a test
- Regression fix: an earlier change dropped the curl --connect-to port workaround along with -k, but curl 7.76.1 (the image's) 403s on ANY ported endpoint, which is the shape backupStorage.endpoint ships (:8333) and what provisionBucket:false and s3.ingress.enabled:false produce. Restore --connect-to (sign a portless URL, redirect to the real port), keeping -k off so TLS is still verified. IPv6-safe host/port split, path prefix preserved. The port-less ingress passed; the ported case is the one that regressed. - Correct the S3 envelope claim in values.yaml and backup-classes.md: the 403 is decided by the port, not a body-hash the backend recomputes, and --connect-to handles it on SeaweedFS and MinIO alike. - Hold the script fixes at the layer the contract lives at. Externalise the driver to files/kafka-backup.sh (embedded verbatim via .Files.Get, render byte-identical) and add hack/kafka-backup-script.bats, which EXECUTES it against stubbed kafka CLIs + curl and asserts behaviour: \Q literal match on the colliding pair, --connect-to with no -k on a ported endpoint, fail-closed --list, comma-value bracket grouping, RF-divergence abort, and the cleanup delete's HTTP handling. Not a template grep. - Pin the Backup-delete dispatch and the readiness deadline in Go: TestCleanupOnDelete_Kafka_RoutesToKafkaCleanup and TestReconcileKafka_FailsAfterReadinessDeadline. - Docs: config restore is additive (no --delete-config); export time scales with topic count and the Job has no wall clock (as its siblings). Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
0270650237 |
fix(backups): harden Kafka metadata S3 handling and own the artifact
- Drop `-k`: mirror the RabbitMQ S3 layer (ARTIFACT_URI as the single source of truth, forcePathStyle honoured, whole base URL kept) so curl --aws-sigv4 verifies TLS against the ACME endpoint the helper returns. The derived endpoint is default-port, so the curl 7.76.1 --connect-to port workaround is gone too; this also fixes the IPv6 and path-prefix mangling in the old host-splitting. - Own the artifact: a cleanup mode in the script plus cleanupKafkaBackup wired into BackupReconciler.cleanupOnDelete delete a deleted Backup's object (best-effort, waited on) instead of orphaning it, matching the RabbitMQ precedent. Kafka has the identical own-your-object property. - Restore probes existence with --list (fail closed), so an unreachable broker aborts instead of being misread as "topic absent" and created. - Hold the fixes with tests at the right level: a colliding topic pair (audit.events / audit-events, distinct partition counts) in the e2e so a revert of the \Q...\E literal-match breaks the round-trip, and Go tests pinning the scope stamp and the cleanup Job. - Correct the docs/comments: endpoint-shape row, the "cached on nodes" claim, and the narrower curl 7.76.1 S3 envelope (empty-body payload hash) that makes a small modern-curl client image the right follow-up. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
405eb25095 |
fix(backups): harden Kafka metadata backup
- Fail closed on topic listing: set -o pipefail, capture kafka-topics --list --exclude-internal exit separately, and abort on failure so an unreachable broker can no longer produce an empty backup reported as Succeeded. An empty list after a successful call stays legitimate. - Check each kafka-topics --describe exit and validate the parsed partition count / replication factor before writing a record, so a timed-out describe cannot write a corrupt orders\t\t\t line. - Store topic configs one per line with key and value in separate tab-delimited fields, sourced from kafka-configs --describe (one config per line), so a value containing a comma (cleanup.policy=compact,delete) round-trips; on restore each config is applied with the value bracketed only when it contains a comma. - Detect topic divergence on restore instead of a silent --if-not-exists no-op: grow partitions with --alter when the backup asks for more, and fail loudly (naming the topic and both values) when it asks for fewer partitions or a different replication factor. A restore that cannot reach the recorded state no longer reports Succeeded. - Add KafkaStrategyKind to the RestoreJob cleanup-dispatch no-op group so deleting a Kafka RestoreJob does not run a pointless Velero cleanup; add a regression test alongside the siblings, plus a reconcile Kind-gate test and a rendering test that actually renders the strategy template. - Wire the kafka-metadata Chainsaw suite into both selectors (select-install suite_to_source, select-e2e src_to_suites) so editing the kafka app or the suite selects it and the bats unit tests pass. - Pin the client image to the operator's default broker version (kafka 3.9.1) with a digest, and align the example default; fix the values comment. Point the RabbitMQ comment references (which shipped in the CRD description) at the Job strategy actually mirrored, and trim the in-pod comments that rendered into the manifest. - Document the Strimzi Topic Operator caveat: for a topic owned by a KafkaTopic CR the operator reconciles its config from the CR, so an in-place config restore is durable only for out-of-band topics. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
a410a3f48b |
feat(backups): Kafka topic-metadata backup via a dedicated strategy driver
Add a strategy.backups.cozystack.io/Kafka driver that backs up the topic metadata of a Cozystack-managed Kafka application — partition counts, replication factors and non-default topic configs — through the Kafka Admin API, and restores it with kafka-topics --create + kafka-configs --alter. Modelled on the Job/Rabbitmq strategies: the driver renders a one-shot batch Job, gates on the Strimzi Kafka cluster reporting Ready, and keys each backup by .BackupName so runs do not overwrite each other. Scope is deliberately narrow: message payloads, consumer-group offsets, ACLs, quotas and KafkaUsers are out of scope, and there is no PITR. Restored topics are created directly (unmanaged on the target) — the accepted tradeoff of an Admin-API-only driver. Both restore flows are implemented: in-place recreates dropped topics into the source, to-copy applies the source's topics onto a freshly-bootstrapped empty cluster. Wire it into cozy-default (strategy-kafka-default.yaml gated on the bucket name, a BackupClass binding, read RBAC on kafka.strimzi.io/kafkas for the readiness gate, and the Strimzi Kafka client image), document it in backup-classes.md, and add an honest round-trip example plus Chainsaw e2e (examples/backups/kafka-metadata, kafka-3-metadata-roundtrip) that proves a per-run retention.ms sentinel survives both restores. Unit tests cover the driver's pure helpers (application-ref validation, readiness verdict, parameter round-trip, per-run artifact-URI keying). Not yet verified on a live cluster. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
9eb7eeaca5 |
test(backups): behaviourally pin the shipped Redis strategy script
helm-unittest renders this template but asserts only its structural decisions, so blanking the object key, breaking the rfs-redis- sentinel prefix, or disabling the cleanup guard each left the suite green. Add a bats test (auto-discovered by make bats-unit-tests) that extracts the rendered script and EXECUTES it with stubbed redis-cli / curl: cleanup must DELETE its object and never reach the restore FLUSHALL against the live master, backup must resolve rfs-redis-<app> and upload under the recorded object key, and an unrecognised mode must fail closed. Each of those three mutations now turns the suite red. Also correct the endpoint doc row: the Redis Job always connects over https (the projector strips the scheme and the script prepends it), so its TLS is not scheme-driven the way MariaDB/FoundationDB is. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
1289f522fa |
fix(backups): unbreak install, kill destructive cleanup, reach the resource knob
Store the Redis strategy Template schemaless, as the Rabbitmq strategy already does: without the markers controller-gen emitted the full PodTemplateSpec schema and the CRD reached ~602 KB, pushing the chart's Helm-release Secret past the 1 MiB cap so backupstrategy-controller failed to install at all. Give the example strategy a cleanup branch, and both scripts an explicit `elif restore` / `else exit 1`, so a deleted Backup rendered with Mode=cleanup deletes its object instead of falling into the restore path and FLUSHALLing the live master. Forward a backupStrategyController block through the platform bundle alongside backupStorage, so redisBackupResources — the restore-Pod sizing knob the loader-not-ready message points at — is reachable via spec.components.platform.values.backupStrategyController, not merely settable on a Helm-managed Strategy CR that reverts on the next upgrade. Correct the endpointCASecretName doc row to match the now-optional mount, and `int` the activeDeadlineSeconds interpolation so a large values-file override renders as an integer, not exponent form. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
b4d621f4f7 |
fix(backups): delete Redis backup objects on cleanup and size the restore Pod
Delete the backup object when its Backup is removed. The Redis driver
owns its S3 object (one per BackupJob, no engine retention prunes it), so
it now spawns a cleanup-mode Job that deletes the object and holds the
Backup's finalizer until it completes, mirroring the Rabbitmq driver.
Without this a retention-pruned Plan grew the bucket unboundedly and the
key was lost with the CR.
Size the strategy Pod from backupStrategyController.redisBackupResources
so an admin can raise the restore ceiling (the throwaway loader holds the
whole dataset in memory) via chart values, which a direct edit of the
cluster-scoped, Helm-managed Strategy CR could not survive. The
loader-not-ready diagnostic now names the memory limit, the likeliest
cause, instead of only the image version.
Make the endpoint CA volume optional: nothing projects that Secret into
the app namespace, so a required mount wedged every backup Pod on
FailedMount; a missing ca.crt now falls through to the system CA store
and fails the handshake fast, which is not the same as skipping verify.
Promote the Pod-level activeDeadlineSeconds to the Job so a wedged run is
bounded once, not (backoffLimit+1) times. Use find, not ls, for the base
AOF so its no-match diagnostic is not dead code under pipefail.
Pin the object key reaching the rendered Job: both fixtures now render
--key={{ .ObjectKey }} and assert it, so zeroing the key (which left the
package green) is caught. Document .ObjectKey and the cleanup mode in the
CRD render-context, list cozy-default-redis among the Helm-managed CRs,
and mirror the CA/TLS and find fixes into the example strategy.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey Kolkov
|
45a2f54c62 |
feat(backups): back up TLS-enabled Redis apps via the default strategy
The shipped cozy-default-redis strategy connected to Sentinel and the master in plaintext, so a BackupJob against a Redis app with tls.enabled (the operator moves both to a TLS-only listener) failed at master resolution. Mount the app CA the operator publishes at redis-<app>.ca-cert as an optional Secret, and dial redis-cli with --tls --cacert exactly when its ca.crt is present, so a non-TLS app keeps the plaintext path and the throwaway restore loader stays local-only. Covers the default authClients: no; mutual TLS is out of scope, since the platform issues no client certificate. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
5c603586e1 |
docs(backups): document Redis in backup-classes
The Redis route was added to the cozy-default BackupClass but backup-classes.md still described the pre-Redis bound set. Add the Redis row to the out-of-the-box table, the endpoint-format table (S3_ENDPOINT bare host, https:// prepended when unscheme'd), and the projected-secret key table (AWS_* plus endpoint/bucketName/region). Document the new endpointCASecretName knob and its https/TLS interaction. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
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>
|
||
|
Myasnikov Daniil
|
ca7d2b0182 |
docs(platform): explain system component memory limits
Add docs/operations/system-memory-limits.md, recording why system components carry memory limits. That reasoning is not recoverable from the values themselves, and is exactly the kind of thing a well-meaning later change removes as redundant. The Talos userspace OOM handler (v1.12+, enabled by default) selects its victim by ranking cgroups that lack memory.max. Three consequences are each easy to get wrong on their own: only a limit grants immunity, since a request merely moves BestEffort to Burstable and under strictCgroupClassOrdering that means "killed second"; every container in the pod needs one, init containers included, because kubelet sets pod-level memory.max only when all of them have it; and PriorityClass, system-node-critical included, has no bearing on it whatsoever, because the handler walks cgroupfs and holds no Kubernetes client. That last one is the trap, since setting a priority looks like a fix. The page also covers the two installer knobs and the constraint that makes them dangerous to lower (a defaulted limit below a container's own request fails admission), how to confirm a pod cgroup really carries memory.max rather than trusting the QoS class, what is deliberately left uncovered, and how to relax the global-PSI backstop clause of the trigger on a cluster that trips it persistently. Every Talos claim is checked against the siderolabs/talos v1.13.6 tag, which is the version packages/core/talos currently ships, and every Cozystack claim against the tree. Ordering: this documents behaviour introduced by the system-namespace LimitRange and the node-DaemonSet resource changes, so it should not merge ahead of them. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
b496fb1ae8 |
docs(backups): note to-copy MongoDB restore needs target creds that can read the source bucket
A to-copy restore reads the dump from the source backup's bucket/endpoint but authenticates with the target cluster's own S3 credentials (the source release's Secret dies with it in a DR scenario). Document that the target's credentials must therefore be able to read the source bucket - the common same-bucket cozy-backups case - and that a cross-account/bucket target fails loudly with AccessDenied rather than silently restoring nothing. Addresses the MINOR credential-scoping note from PR review. Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
0d4a3e3134 |
refactor(backups): make MongoDB PITR option uniform with CNPG + warn on unknown restore options
PITR: replace the bespoke spec.options.pitr{type,date} blob with a single
spec.options.recoveryTime RFC3339 string, matching the CNPG strategy's option
exactly (CNPGRestoreOptions.RecoveryTime). The driver converts recoveryTime to
the psmdb Restore's pitr{type:date,date} in the operator's "YYYY-MM-DD HH:MM:SS"
UTC format. An empty recoveryTime restores the snapshot as-taken (documented
difference from CNPG, whose empty value means latest WAL — a psmdb logical
backup is already consistent). A non-RFC3339 recoveryTime fails terminally.
Unknown options: parseMongoDBRestoreOptions now also returns the set of
spec.options keys the driver does not recognise; reconcileMongoDBRestore emits a
UnknownRestoreOption Warning event per unknown key (and logs it) instead of
silently ignoring it — so a "recoverytime" typo no longer silently degrades a
PITR restore to a snapshot restore. (BackupJob carries no spec.options blob, so
there is no backup-path equivalent; the only backup-time input is the free-form
BackupClass parameters used as template vars.)
Docs + examples updated to recoveryTime; tests cover the RFC3339→UTC mapping,
empty/invalid recoveryTime, and unknown-key reporting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@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> |
||
|
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 --> |
||
|
Mattia Eleuteri
|
f859b710a4 |
fix(backupstrategy-controller): repair the bucket credentials Secret from the gate, not a chart fail
Addresses the four findings on #3524. **The `fail` in the bucket chart (MAJOR).** The <bucket>-<user>-credentials Secret was made to `fail` the render while its COSI lookup was empty, so the release would retry and converge instead of skipping the Secret permanently. Helm cannot render a partial set, so that fail aborts the WHOLE <bucket>-system release: the other users' Secrets, and the bucket UI Deployment/Service/Ingress/HTTPRoute with them. One declared user whose BucketAccess never provisions takes down every other user of that bucket, and on an already-installed release parks it in Failed, blocking every later upgrade. **No per-bucket escape (MAJOR).** Rather than add one, remove the need for it. The gate already forces the release that renders the lookup-gated Strategy CRs; the credentials Secret is the same trap one release earlier, and the gate reads that Secret to resolve the bucket name in the first place. So while it is absent, or carries no bucket name, force the bucket's <bucket>-system release instead of doing nothing. That cannot deadlock its own precondition: the BucketClaim and the BucketAccess whose COSI Secret the lookup reads are rendered unconditionally by the parent release, not by the one being forced. The two releases are throttled independently — they are forced in sequence on a bootstrap, so a shared timestamp would delay the second by a full MinForceInterval for no reason. Coordinates come from the chart under provisionBucket, and are omitted on external S3 where the Secret is admin-managed and no release renders it. No RBAC delta: patch on helmreleases is already cluster-scoped. The chart goes back to a partial render, with the comment now explaining why the skip is not self-healing and why failing is the wrong lever, and a test that renders the UI with every user unresolvable so a reintroduced `fail` fails the suite. Tenant buckets keep their pre-existing behaviour; generalising the repair to every bucket needs a controller that owns them and is tracked separately. **Stale gauge (MINOR).** cozystack_backup_default_objects_missing was only written on the happy path, so the state the gate exists to catch reported 0 and the documented alert never fired. It is now written on the unresolved path too — the credentials Secret counts as one of the objects the default backups depend on, which is what it always was. On an API error the gauge is deliberately left alone rather than flapping, so a new cozystack_backup_default_objects_check_errors_total marks it stale, and the doc pairs the two instead of overstating the gauge. **Suspended releases (MINOR).** forceHelmRelease now does a point Get and skips a release with spec.suspend: true. helm-controller ignores forceAt and requestedAt while suspended (`cozyhr suspend` sets exactly that), so the gate was re-stamping every MinForceInterval for the whole suspension and climbing the force counter — which the runbook attributes to a render that is not producing the objects, the wrong diagnosis. The skip is logged and not counted, so a climbing counter keeps its documented meaning. Reported-by: IvanHunters <xorokhotnikov@gmail.com> Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Mattia Eleuteri
|
51ad226706 |
fix(backupstrategy-controller): repair lookup-gated backup objects
The default Strategy CRs and the Velero BackupStorageLocation are Helm-templated behind a `lookup` of the BucketClaim the same chart creates, and the <bucket>-<user>-credentials Secret behind a `lookup` of the COSI Secret. When those lookups are empty at install time the objects were silently skipped, and helm-controller does not re-render a release whose chart and values did not change (drift detection is off on operator-generated HelmReleases), so the skip was permanent: clusters ran for months with only BackupClass cozy-default and no Strategy CRs at all. That also fail-closes the pre-adoption snapshot in the v1.6.0 etcd migration, which reads the projected credentials Secret. Add a DefaultObjectsGate runnable to backupstrategy-controller. Once the bucket name is resolvable from the projector's source Secret, it checks that every object cozy-default routes to exists and forces one real Helm upgrade (reconcile.fluxcd.io/forceAt + requestedAt) when any is missing. Helm remains the objects' only author; the gate only makes sure the render that produces them actually happens. It cannot be a render-time `fail` in this chart: the chart is the producer of the Bucket its own lookup reads, so a failed render would deadlock the condition. The bucket chart's user-credentials template CAN fail safely — the BucketAccess that produces the COSI Secret belongs to the parent release and that release retries forever — so make it fail loudly instead of skipping, with requireUserCredentials=false as the offline-render escape. Also correct the chart and doc comments that claimed Flux re-renders on its interval, and document the two-release manual recovery for clusters already affected. Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Andrey KolkovandClaude Opus 4.8
|
2687e095f4 |
feat(clickhouse): let the backup sidecar trust a private S3 CA
The clickhouse-backup sidecar had no TLS/CA surface at all: bucket coordinates
and credentials arrive as S3_* env, and nothing let it verify an endpoint whose
certificate is 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, while
MariaDB (storage.s3.tls.caSecretKeyRef) and Postgres (barman endpointCA) have had
a CA field all along.
Add backup.endpointCA {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 untouched.
SSL_CERT_DIR rather than AWS_CA_BUNDLE, measured against
altinity/clickhouse-backup:2.7.4 with a self-signed endpoint and public AWS S3 in
turn: Go reads SSL_CERT_DIR in addition to the system bundle, so both verify;
AWS_CA_BUNDLE replaces the SDK trust pool, so the private endpoint verifies and
public S3 starts failing `x509: certificate signed by unknown authority`. That
would silently break a release that also talks to a public endpoint.
S3_DISABLE_CERT_VERIFICATION was rejected outright — it drops verification
instead of trusting one more CA.
Covered by helm-unittest: the unset default, the mount/env/volume wiring, a
custom key still projected as ca.crt, composition with useSystemBucket, and that
backup.enabled=false renders nothing.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
myasnikovdaniil
|
2c453778f1 |
fix(seaweedfs): make naming audit fail closed on kubectl and payload errors (#3436)
## What this PR does Fixes #3431. `hack/seaweedfs-naming-audit.sh` was fail-open: any kubectl failure produced an empty table indistinguishable from an honestly clean fleet — a failed namespace LIST meant zero namespaces walked, failed secret/PVC/STS LISTs meant zero findings, all silenced by `2>/dev/null` with no error handling anywhere in the script. The runbook uses this output as the gate before PVC deletion, as post-deletion verification, and as an upgrade precondition, so a transient API error could green-light destroying data. Every kubectl call now routes through a `run_kubectl` helper: a non-zero exit prints a FATAL line naming the failed query and the code propagates up the whole chain (enumerations restructured to capture-then-check, since an `exit` inside `$(...)` dies with the subshell). By-name GETs separate existence from extraction: a `--ignore-not-found -o name` probe keeps legitimate absence (Helm-pruned revision, unbound PVC) a clean skip, while everything else fails loudly — including a release Secret that exists but has no decodable payload or decodes without a chart name, which previously dropped the whole tenant from the audit. Incomplete PV-age evidence now marks the generation incomplete and falls to the safe "direction cannot be established" branch, so a failed GET can no longer flip an OVERLAP verdict into a wrong Step-3 deletion candidate. A successful audit prints the same bytes as before. Tests: 14 new cozytest cases — failure injection for every LIST and by-name GET site, corrupt-payload variants (missing `.data.release`, undecodable base64/gzip, chartless `{}`), and full-output golden diffs proving the success paths stay byte-identical to the pre-change script. 25/25 green via `hack/cozytest.sh`. ### Release note ```release-note fix(seaweedfs): the naming-audit script fails loudly when a kubectl query or a Helm release payload read fails, instead of printing an empty table indistinguishable from a clean fleet ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Audits now fail clearly (non-zero exit + fatal messaging) when Kubernetes queries or Helm release data is unavailable or corrupt, instead of producing incomplete results. * Missing release revisions are skipped safely; missing/unreadable PV age evidence no longer leads to incorrect deletion candidates, and MIXED now reports conservative “direction cannot be established” when evidence is incomplete. * Non-SeaweedFS releases remain silently ignored. * **Tests** * Added end-to-end acceptance coverage for fail-closed behavior and byte-for-byte stable output across key naming scenarios. * **Documentation** * Updated the runbook to require relying on the audit script exit code before proceeding, with a dedicated MIXED re-check step. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
e02adad38f |
fix(seaweedfs): read the chart name first-match, fail the fake on unmodelled calls
Review findings from #3436, plus what an adversarial re-check of the first attempt at them turned up. The chart-name extraction was a greedy sed, i.e. LAST match. Helm marshals "config" (the release`s values) after "chart", so a values subtree that spells chart.metadata.name shadowed the real chart name: the release then read as non-SeaweedFS, the tenant silently vanished from the report, and the script exited 0 -- the precise false clean this PR exists to prevent, reachable without any corruption at all. It now takes the FIRST match. The path stays adjacent on purpose: a looser "any name after metadata" matches chart.templates[].name, which Helm emits immediately after metadata on every healthy release, so the review`s suggested relaxation would have returned a template path for every tenant. Over-strictness fails loudly and recoverably; over-looseness deletes data quietly. Newlines are now folded before matching, so a pretty-printed payload parses at all rather than aborting, and whitespace around the punctuation is tolerated. first_deployed got the same treatment -- it had the identical greedy, single- line-only shape, and a spaced payload silently dropped the PV-vintage row from the report. The FATAL message names the path it read and the shape it expected, so a future Helm format change is diagnosable instead of looking like real corruption. Tests: three new payload shapes (spaced, pretty multi-line, and a values decoy) each byte-compared against the same golden as the compact payload, so the shape must make no difference to the report. Mutation-checked -- restoring last-match extraction fails the decoy test on its own, restoring the single-line matcher fails the pretty test. The test fake answered any unmodelled kubectl invocation with exit 0 and empty stdout, the fail-open shape this script was rewritten to reject, and enough to let a newly added query pass the goldens unnoticed. Unmodelled calls now exit 97 naming the invocation. Verified inert first by instrumenting the fake: 88 invocations across the suite, none unmodelled. Also added the missing test for the by-name PVC GET fatal path, whose FAIL mode existed with nothing behind it. The runbook now tells the operator to read the exit code: a non-zero exit means the table is incomplete and no step may be taken on it. That contract lived only in the script header, while the runbook is what the operator follows. 29/29 tests green; sh -n clean. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |