mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-06 03:57:52 +09:00
fix/select-e2e-find-not
127
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|
7d44372a16 |
fix(postgres): read the live Cluster when deciding the WAL-archive prefix (#4299)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Fixes #4298. `applyClusterPluginBackup` preserves the `serverName` already on the Cluster so that a backup does not flip the WAL-archive prefix mid-stream, which would split the archive across two prefixes. That rule is right; the read it rests on is not reliable in one window. The value was read through the manager's cached client, and `BackupJobReconciler.SetupWithManager` registers `For(&BackupJob{})` and nothing else, so a Cluster only reaches the cache through a lazily started informer with no resync tied to the Cluster's own changes. An in-place `RestoreJob` deletes the Cluster and the chart re-renders it with `bootstrap.newServerName` seconds later, so a read served during that window describes the object that was purged — and the source's `serverName` is written back over the freshly rendered one. `barman-cloud-check-wal-archive` then refuses the destination with `Expected empty archive` and the `RestoreJob` ends on its 30-minute deadline, which is the failure `newServerName` was added in #3988 to prevent. The same stale read carries a stale `UID`, and that `UID` owns the `ObjectStore` this function applies, so the store is garbage-collected as soon as it is written. Reading through the dynamic client this reconciler already carries keeps the preservation rule and makes the value it rests on truthful. There is precedent in the same file: the HelmRelease suspend/resume path reads that way for the same reason. Observed on a v1.6.3 cluster, twice on separate runs with the same four-second gap between the chart's apply and the driver's. The regression test makes the two clients disagree the way they disagree in that window and asserts the driver believes the live one; reverting the fix fails it on the serverName assertion. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [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 <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(postgres): an in-place restore no longer loses its fresh WAL-archive prefix to a stale Cluster read, which left the restored cluster archiving onto the source's prefix and the RestoreJob failing on its deadline ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backup processing now uses the latest CNPG Cluster state rather than potentially outdated cached information. This ensures the cluster’s current server name and identity are used when associating backup ObjectStore resources, including when the cached and live cluster details differ. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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 --> |
||
|
Mattia Eleuteri
|
67bf9ca317 |
fix(postgres): read the live Cluster when deciding the WAL-archive prefix
applyClusterPluginBackup preserves the serverName already on the Cluster, so
that a backup does not flip the archive prefix mid-stream. It read that value
through the manager's cached client, and this controller registers
For(&BackupJob{}) and nothing else, so a Cluster only reaches the cache
through a lazily started informer with no resync tied to its own changes.
An in-place RestoreJob deletes the Cluster and the chart re-renders it with
bootstrap.newServerName seconds later. A read served during that window
describes the object that was purged, so the source's serverName is written
back over the freshly rendered one and the restored cluster archives onto the
source's WAL prefix. barman-cloud-check-wal-archive then refuses to start with
"Expected empty archive" and the RestoreJob ends on its deadline -- the
failure newServerName was added to prevent.
The stale read carries a stale UID too, and that UID owns the ObjectStore this
function applies, so the store is garbage-collected as soon as it is written.
Reading through the dynamic client keeps the preservation rule and makes the
value it rests on truthful.
Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
|
||
|
Aleksei Sviridkin
|
717d091970 |
fix(backups, postgres): start a Postgres restore from the backup it names (#4443)
## What this PR does A Postgres `RestoreJob` now starts from the base backup its `Backup` refers to, and the PITR documentation describes the window the recovery actually has. **Starting from the named backup.** Without `recoveryTarget.backupID` the barman-cloud plugin picks the base backup itself (`internal/cnpgi/restore/restore.go` in plugin-barman-cloud, then `FindBackupInfo` in barman-cloud `pkg/catalog`): the newest backup of the catalog when no `recoveryTime` is set, otherwise the newest one whose end is not after `recoveryTime`, on any timeline since no `targetTLI` is passed. A `RestoreJob` naming an older `Backup` therefore started from another one, and on an archive that holds sibling timelines — left by in-place restores that archived back under the source `serverName`, before restored clusters got a fresh `newServerName` — it could start from another branch and bring back that branch's data. `FindBackupInfo` gives `backupID` precedence over time-based selection, and CNPG's webhook does not count it among the mutually exclusive targets, so it combines with `targetTime`. - on restore the CNPG driver reads barman's `backupId` and the backup's `stoppedAt` from the `Backup`'s `cnpg.io/Backup` and sets the new chart value `bootstrap.backupID`, rendered as `bootstrap.recovery.recoveryTarget.backupID`; - it does not pin for a `recoveryTime` earlier than one second past `stoppedAt`: the named backup cannot serve an instant before it is consistent, and `stoppedAt` is truncated to the second while the catalog compares barman's end time to the microsecond. Those requests keep the plugin's time-based choice, as today; - it does not pin either when the `cnpg.io/Backup` is gone. plugin-barman-cloud's retention deletes it together with the base backup it describes (`deleteBackupsNotInCatalog`), and the driver deletes one only with its artifact, so a missing one means the catalog has lost that base backup: pinning it would purge the target and then fail the recovery. The plugin's choice recovers to the same end of the archive, and the `RestoreJob` gets a `BaseBackupGone` warning event. A base backup deleted from the object store by other means is still pinned for as long as its `cnpg.io/Backup` stays, and that restore fails after the purge; plugin-barman-cloud deletes such a `cnpg.io/Backup` on its next maintenance pass (`deleteBackupsNotInCatalog`, every `retentionPolicyIntervalSeconds`, retention policy or not), but only from the primary of the cluster that took it, under the same UID. Telling it apart earlier would need reading the barman catalog, which the driver does not do; the documentation says so, and that deleting the `cnpg.io/Backup` by hand is the way out; - a re-restore into the same target clears a `backupID` left by a previous one. **Documentation** (`docs/operations/backup-classes.md`, PostgreSQL PITR): - the upper bound of the window is the last transaction (commit or abort) in the archived WAL, not the newest segment: recovery ends just before the first transaction after the target, and a segment switch or a base backup is not one. On a database at rest every instant after its last write fails with `recovery ended before configured recovery target was reached`, including "at/before the most recent completed backup", which the page recommended as the safe choice; the state as of the last write is obtained by omitting `recoveryTime`; - aiming at a backup's `stoppedAt` misses that backup (the plugin keeps only a backup whose end is not after the target), falling back to an older one or to `no target backup found`; the page now says to aim one second past it; - when a base backup removed from the bucket outside retention is still pinned, and how to unpin it; - how to read the upper bound from the archive, what pinning the base backup changes, and a section on archives with several timelines: PostgreSQL follows `recovery_target_timeline: latest` from the backup's timeline through the consecutively numbered history files it can find, so a missing `.history` confines the replay to the backup's own timeline and the bound is the last transaction on that timeline, while a targeted timeline that does not descend from the backup stops recovery with `requested timeline N is not a child of this server's history`. **Tests.** `TestCNPGRestoreBackupID` (the pinning rule), `TestReconcileCNPGRestore_PinsTheRequestedBackup` (drives a restore through the purge and reads back the patched `Postgres` app, including a `cnpg.io/Backup` removed by retention, with and without `recoveryTime`), `TestBuildPostgresAppRestorePatch_ReplacesBackupID`, a deep-copy test for the new `stoppedAt` field, and three `helm unittest` cases for `recoveryTarget`. With the rule disabled, 4 of the rule cases and 3 of the reconcile cases fail; with the template change removed, 2 chart cases fail; the retention case fails against the first version of this PR, which pinned an id recorded on the artifact; the deep-copy test fails once `StoppedAt` is dropped from the generated `DeepCopyInto`. `go test ./internal/backupcontroller/...` and `helm unittest packages/apps/postgres` (139/139, after rebasing on main) pass. Not run: the e2e suite (`hack/e2e-chainsaw`), and a restore against a live cluster with this driver. ### Screenshots No UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/716. `content/en/docs/next/operations/services/backup-classes.md` (and `v1.6/`) carries a copy of the corrected "The recoverable window" and "Discovering the earliest / latest restorable time" sections; the window and `stoppedAt` corrections apply to released versions too, the `backupID` part only once this is released. - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: none expected. The diff adds `bootstrap.backupID` to the postgres `values.schema.json`, but the provider does not manage the `bootstrap` block (`internal/provider/postgresql_schema.go`: "bootstrap blocks use server defaults"), and the `RestoreJob` CRD is unchanged. Left unticked for a maintainer to confirm. - [ ] [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 <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(backups): a Postgres RestoreJob now starts from the base backup its Backup refers to (recoveryTarget.backupID) instead of whichever backup the barman-cloud plugin picks, which on an archive with several timelines could restore another branch's data; a recoveryTime before that backup's end keeps the previous time-based choice. The PITR documentation now bounds the recoverable window by the last archived transaction rather than the newest WAL segment. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Restores can target a specific base backup and optionally replay to a requested recovery time. If no backup is specified, the newest eligible backup is selected automatically. * **Bug Fixes** * Restore handling warns when the referenced base backup is unavailable and allows the plugin to select a backup from the catalog. * **Documentation** * Clarified recovery-window boundaries, timestamp precision, idle databases, timeline branches, and behavior when a requested target cannot be reached or predates the earliest base backup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
28e42fbe4d |
fix(backups): match a Velero restore in its Backup's API group
The Velero driver resolves a restore's target from the Backup's applicationRef, overriding only the kind and name, and restores the Backup's resources. A targetApplicationRef.apiGroup never reaches it. The restore gate honoured that group all the same, so a Velero RestoreJob naming another group let a BackupJob of the application actually being restored start against it, and held one of an application the restore never touches. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
497dd6fc4f |
fix(backups): start a Postgres restore from the backup it names
Without recoveryTarget.backupID the barman-cloud plugin chooses the base backup itself: the newest one in the catalog when no recoveryTime is set, and the newest one ending by recoveryTime otherwise, on any timeline since no targetTLI is passed. A RestoreJob naming an older Backup therefore started from another one, and on an archive holding sibling timelines - left by in-place restores that archived back under the source serverName - from another branch, bringing back that branch's data. The restore reads barman's backupId and the backup's stoppedAt from the live cnpg.io/Backup behind the Backup, and passes the id through the chart as bootstrap.recovery.recoveryTarget.backupID. The plugin gives backupID precedence over time-based selection, and CNPG does not count it among the mutually exclusive targets, so it combines with targetTime. Two cases keep the plugin's choice. A recoveryTime earlier than one second past stoppedAt: the named backup cannot serve an instant before it is consistent, and stoppedAt is truncated to the second while barman compares its end time to the microsecond. And a cnpg.io/Backup that is gone: plugin-barman-cloud's retention deletes it together with the base backup it describes, and the driver deletes one only with its artifact, so pinning its id would purge the target and then fail the recovery on every retry. That restore records a BaseBackupGone warning event instead. A re-restore into the same target clears a backupID left by a previous one. 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> |
||
|
Mattia Eleuteri
|
af8cf743cd |
fix(backups): hold a BackupJob while its application is being restored
Nothing on the backup path looks at restores. A Plan creates its BackupJobs on schedule, and the BackupJob reconciler dispatches them to their driver whatever else is happening to the application. For CNPG that is not merely a backup of a half-restored state: every BackupJob reconcile re-applies the Cluster's barman plugin, and on a cluster under an in-place restore it can land after the chart has re-rendered the Cluster with its new serverName and put it back on the source's. The restored cluster then archives onto the source's WAL prefix and its recovery fails with "WAL archive check failed ... Expected empty archive". This was reproduced with a backup triggered a few seconds after an in-place restore, and an hourly plan overlaps a restore of up to thirty minutes about half the time. A BackupJob that has not started yet now waits while a non-terminal RestoreJob in its namespace writes into the same application, resolved as the drivers resolve it: the Backup's applicationRef, with the target's kind and name when a targetApplicationRef is set, and the target's API group only when it names one. A RestoreJob whose Backup is gone holds nothing, since the restore controller fails it on its next pass. options.targetNamespace is honoured only for a Backup taken by the Velero strategy, the one driver that reads it; the CNPG and Job-based drivers ignore the option and restore in place, so their restores hold whatever it says. The held job stays Pending with a Ready=False RestoreInProgress condition naming the restore and is requeued, rather than failed, so a scheduled run still happens once the restore ends. A failed write of that status is returned, so the manager retries it. When the restore ends the condition is removed before the job starts, since it would otherwise read "waiting" until the driver next writes Ready, which for a healthy run is when the backup completes. A run that has already started is left alone: its driver owns state the gate would strand. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## What this PR does This replaces the open Renovate PRs with one rollup. Go modules move to their newest releases that keep the Kubernetes libraries and controller-runtime where they are, the toolchain and builder images move to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers. Tests for the code the bumps touch come first, in their own commits, and pass on both the old and the new dependencies. Before the bumps: - The e2e sandbox image builds again: dl.min.io answers 410 for mc since MinIO archived it, so mc now comes from its last GitHub release with a pinned checksum. - Renovate stops offering tags of the apimachinery fork. They mirror upstream, so #2999 is a downgrade that drops our patch. - `make unit-tests` now runs the tests of the nested Go modules (kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1). They never ran in CI before. - New tests: JWT verification in the dashboard token proxy, the kube-ovn plunger metrics, cert-manager object idempotency in the tenant gateway, values-file layering in the operator, and the kubevirt CSI driver wrapper. - One real fix. The lineage webhook decoded admitted objects with encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5 compares numbers exactly and would emit a replace that rewrites such a field in the pod. The webhook now decodes with apimachinery's json. Held back on purpose, with the reasons in the commit messages: - Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork rebased onto release-1.37 first. - cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is built and tested with. - grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch release carries the fix yet. - kubevirt.io/api and the CDI API stay on what upstream csi-driver builds against. From CDI v1.60 it no longer compiles. - api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types are not forced onto Go 1.27. - go fix runs without embedlit (Go 1.27-only syntax, breaks backports to release branches on Go 1.26) and omitzero (would make metadata, spec and status required in the published OpenAPI schemas), and one errorsastype rewrite that did not compile is left out. govulncheck reports nothing reachable in any of the five modules, standard library included. Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805, #3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open. Not checked locally: the images that build upstream projects (kube-ovn, kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The image build jobs of this PR cover them. Closes #4487. ### Screenshots No UI changes. ### 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 chore(deps): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a restore resource policy option for Velero strategies. * Added a test target to run Go tests across tracked modules. * **Bug Fixes** * Updated the end-to-end test environment to download and verify the MinIO client from its current release location. * Added coverage for token validation, webhook patching, controller behavior, and reconciliation. * **Maintenance** * Updated the Go toolchain, dependencies, and container image versions. * Refreshed pinned workflow actions and base image digests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
9fd590a5a7 |
fix(backups): keep a MariaDB restore target's accounts
The operator ships mysql.global_priv with every logical dump, so a restore into a copy replaced the target's grant table with the source's password hashes: application logins failed, and root, which the operator sets only at datadir bootstrap, stayed wrong. Set ignoreGlobalPriv on every Backup the driver creates. An account created out of band on the source is therefore not carried into the copy. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
70dcd9daed |
fix(backups): let a restored Postgres app converge its credentials
A restore sets bootstrap.enabled on the target app, which skips the init-job, so the recovered roles keep the source's password hashes while the target's credentials Secret holds generated ones and every application login fails. Once recovery converges, the driver records RecoveryConverged and then clears bootstrap.enabled, so the next HelmRelease reconcile runs the init-job and applies the passwords. Ready carries RestoreCompletedCredentialsPending and an Event says the convergence is still pending. A clear that keeps failing is retried under BootstrapDisablePending within a fixed grace window after convergence and then fails the restore. bootstrap.enabled is serialized without omitempty so the patch sends false instead of dropping the key. Running the init-job on a restored cluster exposes its delete pass to a catalog the spec does not describe: the driver sets spec.databases to the Backup's snapshot, but recovery replays WAL past it, so a database created after the Backup comes back still marked as managed and would be dropped with its data. The first run after a restore, told apart by the per-RestoreJob bootstrap.newServerName that the cluster has not yet recorded, releases such a database and its roles from management instead, and the run records the value once every pass has succeeded. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Andrey Kolkov
|
d50385f310 |
fix(backups): tailor the Velero rename refusal to VMDisk and in-flight jobs
The refusal pointed every kind at "the application's engine-native strategy", but VMDisk, the only non-VMInstance kind the default BackupClass routes to Velero, has none. Offer that remedy only to kinds that can have one. The check also runs on every reconcile, so a rename RestoreJob that was already Running when the controller picked up the check is failed after its Velero Restore has completed. That job now says the application may already sit in the target namespace under its source name (a retry that drops the rename would otherwise trip failIfTargetExists on that copy without explanation), and its resource-modifier ConfigMap is released right away instead of at the job's deletion. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
11bac54f93 |
test(backups): pin both sides of the Velero rename guard
The guard's regression cases only exercised renames, so dropping the IsRenamed conjunct (failing every non-VMInstance restore) or keying the check on targetApplicationRef.kind instead of the backup's kind left the package green. Add the inputs the guard must let through, a VMDisk copy that keeps its name and a VMDisk in-place restore without targetApplicationRef, and a VMDisk backup restored with a VMInstance target kind, which must still be rejected by the source kind. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
88073bb209 |
fix(backups): fail Velero RestoreJob rename for non-VMInstance kinds
A RestoreJob that set targetApplicationRef.name to a new name and options.targetNamespace to another namespace reported Succeeded for any Velero-backed application, but only a VMInstance was actually renamed. postRestoreRename looks up the HelmRelease under the vm-instance- prefix, gets NotFound for every other kind and returns nil, so a Postgres, MariaDB, ClickHouse, MongoDB or standalone VMDisk copy came up under the source name with no error surfaced. Renaming those kinds through the Velero strategy cannot work: DataUpload restores volumes under their original names, and only a VMInstance keeps using them after its HelmRelease is renamed, because its disks keep their names and CDI adopts the restored PVCs. A renamed database Cluster or VMDisk renders fresh volumes and comes up empty. Reject the rename up front for every kind other than VMInstance, before the RestoreJob enters Running, with a message that points at omitting targetApplicationRef.name or using the engine-native strategy. The existing same-namespace guard keeps precedence, so its message is unchanged. Closes #3537 Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
8849aa13de |
fix(mongodb): keep a completed archive reachable and a legacy Backup unblocked
Once the operator CR is ready the archive exists, and the Backup object is the only thing that can ever reach it again. A transient failure creating that object (etcd timeout, quota, webhook) failed the job terminally with no artifact, and the terminal-phase guard stopped any retry, so the dump sat in the shared bucket with nothing to reclaim it. The failure is now retried with backoff for a window after the dump completed, and when it does give up the job names the operator CR that still holds the archive. Cleanup read the operator CR before deciding whether the Backup owned anything, so a legacy Backup — an unconditional no-op before this driver — could be held Terminating by a throttle or a revoked verb on a CR the driver never owned. The snapshot records the flow, so a legacy Backup now releases before any apiserver read; a missing or undecodable snapshot still falls through to the read. The opt-out refusal recognised the lingering injected entry only by the strategy's current credentialsSecret and only while the strategy still carried s3, so a platform change to the strategy after a tenant opted out let a legacy CR be minted onto the platform bucket. The platform default Secret name is now recognised as well, independent of the strategy's current block. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
f004b35fb4 |
fix(mongodb): record the CR's flow on the artifact and bound the two open loops
Reading a backup's flow off its CR's finalizer had reached the state switch but not the artifact: the snapshot was still written from the app flag, re-read on the completing reconcile. A flag flipped to false while a system-bucket dump streamed produced a Backup whose snapshot called a cozy-backups archive legacy, so the point-in-time refusal and the S3 backfill both switched off for it. The artifact now takes the same flow the switch does, pinned by a test that flips the flag between mint and completion. Two waits had no upper bound. A live-state read that kept failing at the deadline deferred every poll behind a debug line with no condition, so an unreadable CR pinned the job Running with nothing to act on; it is now a named wait and fails after a grace, with the CR left in place rather than deleted blind. The storage-race retry minted a fresh operator CR per poll while the previous ones terminated, for the whole deadline; it now stops after a small budget and surfaces the operator's error. On useSystemBucket the chart already guarantees backup.enabled, so a strategy without s3 coordinates failed with a remedy the tenant had in place; the wait names the missing coordinates instead. The missing-credentials message no longer blames useSystemBucket on a release that never enabled it. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
5e3b8c217b |
fix(mongodb): refuse useSystemBucket without backup.enabled and pin the retry's flow
The pbm agents run only when backup.enabled is true, so useSystemBucket with backups disabled renders a cluster that can never service a BackupJob, and the tenant learned that thirty minutes into the first job when the driver's precondition gave up. The chart already fails closed on the bootstrap clash; do the same here so the combination is a render-time error with the remedy in it. The storage-race retry keyed on the app flag while everything else in the state switch had moved to the flow recorded on the CR's own finalizer, and that flag term was the only thing keeping a legacy tenant's operator CR from being deleted and retried on the same operator error — with no test holding it. Key it on the CR's flow like its neighbours and pin the legacy side: the error is terminal and the CR survives. The retry matches the operator's own error wording, which is correct at the pinned crVersion and nothing else ties together: a bump that rewords it turns the retry off with the suite still green. Name the coupling at the pin and at the matcher so a bump re-checks it. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
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
|
69f80dbdc2 |
fix(mongodb): drop the false-positive stall detector and harden the cancel
The pinned psmdb operator persists a backup's status only when the state string or error changes, so status.lastTransition freezes at the instant a dump enters running and never advances while it streams. A stall detector keyed on that field fires on every dump that outruns its window and raises the same signal a genuine wedge would, so it cannot be acted on either way. Drop it: a running dump is polled quietly and never cancelled, since this CR carries no field a driver-side timer can use to tell a slow dump from a wedged one. The deadline cancel re-read the live state but could not tell "could not read" from "not started": the helper returned "" for a nil client, a throttle, a 403 and a real absence alike, and a live ready or error state also fell through, so an unanswerable read or a just-completed CR was deleted, taking a finished archive with it via the delete-backup finalizer. The helper now returns an error, and the cancel deletes only on a positive not-started answer, deferring on anything else. The restore's point-in-time refusal keyed on the resolved credentialsSecret name, which a BackupClass can set to any Secret; record the useSystemBucket flag on the backup snapshot and key the refusal on that instead. The storage-race retry deleted the errored CR, but the lookup kept returning it while it lingered Terminating, so a fresh CR was never minted; skip a CR carrying a deletion timestamp in the lookup. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
2293c0c073 |
fix(mongodb): bound and surface the useSystemBucket backup race-recovery waits
The useSystemBucket driver injects storage onto the live cluster at BackupJob time and works around the psmdb operator resolving that storage from a cache that has not yet observed the apply. Several of those waits were unbounded or silent, and a PITR restore of a system-bucket backup reached the operator despite no oplog ever being captured on that flow: - the first-injection requeue now fails terminally once the backup deadline passes, instead of polling a cluster whose cache never reflects the storage - a state=error the operator raises because it has not yet seen the injected storage is retried (delete the CR, requeue) within the deadline rather than failing the BackupJob on the driver's own race - a backup wedged in state=running past the stall window surfaces Ready=False and a Warning once its status stops transitioning, so it no longer parks later BackupJobs in silence, and is never cancelled (that would strand the partial archive pbm keeps writing to the shared bucket) - the timeout-cancel path re-reads the CR live before deleting, so a waiting CR whose live state just turned running at the boundary is not deleted mid-dump - a failed delete-backup finalizer strip now emits a Warning naming the stuck object instead of the success-case event - a recoveryTime restore over a system-bucket backup fails with a named error rather than handing the operator a point-in-time target it cannot serve Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
d091c9951c |
fix(mongodb): requeue after a fresh storage injection so the operator's cache can observe it
The first BackupJob of a useSystemBucket app SSA-injected the storage and minted the operator PerconaServerMongoDBBackup in one reconcile. The psmdb operator resolves spec.backup.storages from a cached cluster read when it services that CR, and a cache that has not yet observed the apply takes the not-found branch and latches the CR at State=error — which the operator never re-drives (it returns for a terminal-state CR and watches only the Backup CR and Pods) and the driver reads as a terminal failure. Only the first job per app is exposed; later ones re-apply an already-declared storage. When the pre-apply cluster did not already declare the storage, requeue after the apply instead of minting in the same pass. The next reconcile finds the storage in this driver's own cache (a good proxy that the operator's has caught up too) and only then mints the CR — one 5s poll per app, well inside the deadline. A fake client has no informer lag, so this is pinned by asserting the first pass requeues and mints nothing rather than by reproducing the timing. Also correct the RBAC delete-grant comment: the 30m timeout branch cancels a not-yet-started operator CR on either flow, not just the useSystemBucket cleanup, so a Failed BackupJob never leaves a CR the operator would run later. And pin the wantBucket=="" restore-credential short-circuit with a no-bucket source case. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
e653a4fa5d |
fix(mongodb): correct the finalizer-prune claims, pin the pre-read release CR, and fix the retention field text
The delete-backup finalizer prunes storage only for a `ready` backup at v1.22.0; a timed-out backup never reaches `ready`, so the deadline-cancel comment no longer claims the finalizer "takes anything partially written" — nothing is written until the backup runs, which the cancel prevents. The psmdbBackupTimedOut godoc no longer says the operator "has not started" the backup, which is wrong for `requested` (the operator sets it as it dispatches); it now describes the not-yet-running states it actually gates. The pre-read CR handed to releaseMongoDBCleanup on the terminating path had no test that failed when the argument was dropped, because the existing cases either call the function directly or let the second Get succeed. Added a cleanupMongoDBBackup case whose operator-CR Get fails after the first: the release must strip from the passed object, not re-read, or the teardown wedges. The useSystemBucket field said "retention move[s] to the Plan", but a Plan has no retention field; corrected to "scheduling moves to a Plan; this flow has no retention yet" and regenerated the schema, README and mongodb-rd. Softened the "never pins a namespace in Terminating" wording in the cleanup godoc and RBAC comment: the driver only bounds its own finalizer, the operator's release-lock finalizer can still hold the CR while the operator is down. 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
|
f8554d289d |
fix(mongodb): stop the system-bucket flow stranding archives, and reject an inert retentionPolicy
Two ways the shared cozy-backups bucket could accumulate objects nothing prunes. A dump that outran the 30-minute deadline was declared failed while pbm kept writing. The BackupJob's terminal-phase guard then never runs the branch that mints the Cozystack Backup, so the archive lands with no Backup object to represent it and no retention can reach it (the operator CR carries no ownerRef either). Leave a running dump to finish — a real dataset routinely outruns 30m, and a broken one still terminates through the operator's error/rejected state. The deadline keeps bounding the case it was written for: the operator never starts the backup (state stays "", requested or waiting), where nothing has been written and failing strands nothing. retentionPolicy is inert on this flow (the chart's tasks[].keep is gated out and no Plan-level retention exists yet), so a value a tenant set expecting it to work would grow the bucket silently. Reject a retentionPolicy changed from the chart default when useSystemBucket is on, rather than ignore it; the default still renders so an untouched install is unaffected. Also on the cleanup path: hand the terminating-namespace release the operator CR the caller already read and confirmed owned, so the finalizer strip is not gated on a second Get whose transient failure would silently drop it and leave the CR finalized in a namespace being torn down. Correct the ownership comments — the delete-backup finalizer is not one the operator never sets (it finalizes task-minted backups on the legacy flow); what keeps it unambiguous is that cleanup only ever resolves the one CR named in driverMetadata. Add the missing isolation test that a legacy app's own s3-storage is never overwritten by the injection call site, and move the strategy's storage comment into a Go-template comment so it no longer ships into every rendered Strategy CR. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
cc51e040ae |
fix(mongodb): keep the cleanup escape hatch off legacy operator CRs, and surface a stuck finalizer
The skip-artifact-cleanup annotation reaches releaseMongoDBCleanup before the ownership check, and the operator-CR delete sat outside the finalizer gate, so a legacy backup (no delete-backup finalizer) lost its PerconaServerMongoDBBackup record when a tenant set the annotation to release a stuck Backup or applied it across a namespace during teardown. That record is what a psmdb restore-by-name resolves against, and both the RBAC comment and the docs promise it is never touched. Gate every write in the release on the finalizer being present: an unowned (legacy) CR or one that cannot be read at all is left intact, and the Cozystack Backup still releases. Surface a failed finalizer strip. The release must proceed even against an unreachable CR, but the swallowed patch error left the operator CR holding percona.com/delete-backup — pinning its namespace in Terminating, the outcome the branch exists to prevent — while the Event reported the success case. Emit a FinalizerNotStripped Warning naming the object so an operator can find it. Mark the sibling backup fields (schedule, retentionPolicy, destinationPath, endpointURL, insecureSkipTLSVerify, s3AccessKey, s3SecretKey) as ignored under useSystemBucket, so a tenant editing one instead of the flag itself is not misled by a description that describes only the legacy flow; regenerate the schema, README and mongodb-rd. Pair the insecureSkipTLSVerify rendering assertion with a case that flips the platform knob on, so the two together discriminate the rendered value from a hardcoded default. 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 |
||
|
Andrey Kolkov
|
c036efaa5a |
test(mongodb): cover system-bucket storage injection gate and builder
Extract the two untested contracts from applyMongoDBSystemStorage into
pure functions and unit-test both, mirroring the CNPG driver's
buildBarmanObjectStore split:
- shouldInjectMongoDBSystemStorage decides whether the driver injects the
system-bucket storage; the tests pin both branches, including the one
that leaves a legacy app's manual bucket untouched.
- buildMongoDBSystemStorageEntry builds the {type:s3,s3:{...}} entry,
defaulting credentialsSecret and omitting forcePathStyle when nil.
Also add a helm-unittest for the restore.yaml guard rejecting
bootstrap.enabled together with backup.useSystemBucket.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
|
||
|
Andrey Kolkov
|
730592f323 |
feat(mongodb): support the system-bucket backup flow (useSystemBucket)
MongoDB was the only cozy-default app with no useSystemBucket flow: its backup storage was always static on the app CR (bucket/endpoint/creds from backup.* values + 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/etc. do. 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 env secretKeyRef - the chart cannot render them. The fix mirrors the CNPG driver: the chart adds a useSystemBucket flag that keeps spec.backup.enabled (so the pbm agents run) but omits the static storages/tasks/pitr and the creds Secret; the cozy-default MongoDB strategy carries the coordinates (endpointUrl already scheme'd by the endpoint helper, credentialsSecret cozy-backups-creds); and the psmdb driver SSA-injects spec.backup.storages[s3-storage] onto the live PerconaServerMongoDB at BackupJob time under its own field manager with ForceOwnership, so a Flux re-render never reverts it. Injection is gated on the storage being absent, so a legacy app that ships its own static storage is left untouched. The backup snapshot falls back to the strategy coordinates when the operator status echo is empty, so the restore path rebuilds backupSource (credentialsSecret cozy-backups-creds is projected into the restore namespace before dispatch). RBAC grows patch/update on perconaservermongodbs for the injection. A bootstrap.enabled + useSystemBucket render guard mirrors postgres. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Yan Bondarenko
|
4dc6065ec9 |
fix(backups): give the sidecar sizing its reason per caller
The Keycloak comment and the helper doc claimed the tenant LimitRange as the reason for the resources, but cozy-keycloak has no LimitRange and neither does a tenant that leaves resourceQuotas empty: there the sidecar had no requests and no limit at all, and the change gives it a 1Gi ceiling where there was none. The comments and both doc blocks now say what applies to each caller. No rendered object changes. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
9214430bd8 |
fix(backups): give the barman-cloud sidecar its own resources
The barman-cloud plugin injects its sidecar without resource requests or limits. Tenant namespaces ship a LimitRange that defaults containers to 128Mi, so the sidecar inherits that default and is OOMKilled mid-backup: the ObjectStore stays healthy, the Cluster stays Ready, and only the backup fails. A cgroup high-water mark of 254 MiB on a 38 MB database shows the default cannot hold it. The shared cozy-lib helper and the Go path that builds the platform's own ObjectStore now request 100m/256Mi and cap memory at 1Gi. No CPU limit is set, matching the entityOperator precedent in the kafka chart: a throttled sidecar stalls WAL archiving instead of failing it. The helper is renamed from checksumSidecarConfiguration to sidecarConfiguration, since it no longer carries only the checksum pin. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Andrey Kolkov
|
ca2a6d6429 |
fix(backups): warn when a Kafka backupTimeout is rejected
kafkaRunDeadline silently substituted the 30m default for a backupTimeout that time.ParseDuration rejects or that falls below the one-minute floor, with no Event, condition or log line. An operator who set an unparseable or sub-minute value, then raised it, saw every run keep dying at DeadlineExceeded with nothing naming the discarded parameter. Return the applied deadline together with a message when the configured value is rejected, and record it as a BackupTimeoutInvalid Warning Event on the BackupJob and RestoreJob (the reconcilers already hold a Recorder). An honoured or unset value stays silent. TestKafkaRunDeadline now asserts the warning fires for the rejected cases and not otherwise. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
cca3b08733 |
test(backups): pin the Kafka restore path and fail fast on a missing artifact
The restore half of the driver - the half that mutates a live Kafka cluster - had no test reaching its happy path, so three reversions of it left the suite green. - Add restore reconcile tests mirroring the Rabbitmq driver this was copied from: ToCopy (pins the targetApplicationRef.Name merge - a to-copy restore must resolve the TARGET, not write the source's topics back into the source), NoRecordedArtifactURI, TargetClusterNotReady and TargetNotFound. Verified non-vacuous: neutralising the name merge reddens ToCopy, and reverting the fail-fast ordering below reddens NoRecordedArtifactURI. - Fail fast on a Backup with no status.artifact.uri: move the check ahead of the readiness gate so it terminates immediately instead of waiting out the whole deadline and then reporting a misleading "timed out waiting for Kafka cluster to become Ready" (reconcileRabbitmqRestore checks it first; same order here). - Add bats cases for the fail-closed guards that no test reddened: MODE validation, the s3:// URI prefix, cleanup treating 404 as success, an unparsable backup/live describe, a kafka-configs describe error, and an unrecognised record kind (plus a CFG_FAIL knob in the configs stub). The describe-parse guards abort via `set -euo pipefail` at the `X=$(describe_field ...)` assignment, so these pin the fail-closed behaviour; the explicit guards remain as belt-and-braces. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
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
|
4925a84c26 |
fix(backups): pin the restore-side guards and bound a running Kafka Job
- delete_topic waited with `grep -qx` (BRE), so deleting `audit.events` matched the live `audit-events` and spun to its timeout — the e2e failed at step 25 on every run that reached it. Use `grep -qxF`. - The two -k/--connect-to guards were written as `! grep`, which cannot fail under hack/cozytest.sh (it wraps each @test in `set -e` + a trailing `return 0`, and bash exempts a `!`-negated command from errexit). Rewrite them as positive `if grep ...; then return 1; fi`; verified the -k reinstatement now reddens the suite under cozytest. - Cover the restore-side literal-topic guard that was unpinned at every level: a bats case runs the existing-topic branch with a colliding sibling and asserts the `--alter` argv carries `\Q…\E`, and run-all.sh now leaves `audit.events` live so the e2e exercises that branch instead of only `--create`. Add bats cases for the partition-shrink refusal, the `-f` upload guard, and `--exclude-internal`. - Bound a running Job: reconcileKafka / reconcileKafkaRestore now fail the BackupJob/RestoreJob once kafkaDefaultBackupDeadline elapses in the Job-running branch (buildJobStrategyBatchJob sets no activeDeadlineSeconds), so an unschedulable pod no longer pins it in Running forever. Pinned by TestReconcileKafka_FailsWhenJobNeverCompletes. - Fail closed on an unknown record kind and an unknown MODE, and validate the live partition/RF parsed on restore (an empty value silently skipped reconciliation). Correct the stale "image pinned in the strategy" comment in the example helpers and the chainsaw test (the controller now resolves the target broker's image as .ClientImage). Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
0d918e1f91 |
fix(backups): resolve Kafka backup client image from the target broker
The Kafka metadata strategy pinned kafkaBackupClientImage to an upstream Strimzi tag with no producer: nothing stamped it and Renovate does not manage helm-values, so it drifted from the broker version the operator provisions. A dedicated small client image (the sibling curl-only drivers' follow-up) is not viable here because the driver runs the Kafka Admin-API CLI, which only the broker image ships. Resolve the client image from a live broker pod of the target Strimzi Kafka at reconcile time instead, so the CLI always matches the broker (any version the operator picks, not only the chart default), and record it on the Backup so cleanup can reuse it once the source cluster is gone. The chart no longer pins the image; the strategy template renders it as the .ClientImage placeholder the controller fills. 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
|
ed3cb34cbf |
fix(backups): adopt-guard Kafka Backup artifact and correct URI-template doc
- createKafkaBackupArtifact now rejects adopting a same-named Backup that describes a different application, so a retained Backup reused for a different app of the same name is not reported as this run's success. Mirrors the RabbitMQ driver's guard; pinned by a Go test. - ArtifactURITemplate godoc said empty leaves status.artifact unset, but the driver hard-fails an empty template. Reword to state it is required, and regenerate the CRD description to match. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |