mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-25 12:37:06 +09:00
main
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
3fc45ed183 |
fix(backups, postgres): refuse a restore into another PostgreSQL major (#4669)
<!-- 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 A CNPG restore is physical, so the recovered data files only open on the PostgreSQL major version that wrote them. `reconcileCNPGRestore` purged the target Cluster and its PVCs without comparing the two. Observed on v1.6.3: a PostgreSQL 18 backup restored into a v16 application emptied the target, the `full-recovery` pods then looped on `FATAL: database files are incompatible with server`, and the RestoreJob failed at the 30-minute deadline with the target left empty. An in-place restore into an application whose `spec.version` was raised since the backup ends the same way. The restore now compares the two majors before the purge, ahead of the WAL-archive gate, and when they differ marks the RestoreJob `Failed` with reason `MajorVersionMismatch` and a message naming both versions. Nothing is suspended, patched or deleted on that path. - Target major: the Postgres application's `spec.version` (`v13` to `v18`). That is what the chart renders the recovered Cluster from once the live one is gone, so the live Cluster's image would be the wrong thing to compare. - Backup major: the `cnpg.io/Backup`'s `status.majorVersion`, now copied into the artifact's `driverMetadata` as `cnpg.io/major-version` at backup time, because barman retention deletes the `cnpg.io/Backup` while the artifact stays restorable. An artifact taken before this change falls back to the `cnpg.io/Backup` while it still exists. - When either side is unknown (no record and the `cnpg.io/Backup` gone, a CNPG that predates `majorVersion`, an empty `spec.version`), the restore goes ahead as before and the controller logs it, so existing backups keep restoring. `TestReconcileCNPGRestore_RefusesCrossMajorRestore` drives the real reconcile against a fake client with a live Cluster and PVC on the target and counts every `Delete`. With the check removed, the two refusal cases (18 into a v16 copy, 16 in place into an app moved to v18) and the fallback case fail: the Cluster and the PVC are deleted and the app is patched for recovery. With the check in place they end `Failed` with zero deletes, while the same-major and unknown-version cases still reach the purge. `TestCreateCNPGBackupArtifact_RecordsMajorVersion` covers the `driverMetadata` record. ### 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: Walked against the diff: it touches only `internal/backupcontroller`, with no CRD, no app `values.yaml` or schema and no ApplicationDefinition change. The new `cnpg.io/major-version` entry lives in the free-form `Backup.spec.driverMetadata` map, which no downstream repository reads by key. ### 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, postgres): a Postgres restore whose backup was taken on another PostgreSQL major version than the target's spec.version now fails before touching the target, instead of purging it and then failing to recover. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Cross-major PostgreSQL restores are now refused before the target is changed. The error identifies the backup and target versions. * Restores proceed when either version is unavailable or cannot be parsed. * **New Features** * Backup metadata now records the PostgreSQL major version when it is available. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
f514f8722b |
fix(backups, postgres): refuse a restore into another PostgreSQL major
A CNPG restore is physical: the recovered data files only open on the major version that wrote them. The driver purged the target Cluster and its PVCs without comparing the two, so a PostgreSQL 18 backup restored into a v16 application, or in place into an application upgraded since the backup, emptied the target. The recovery pods then looped on "database files are incompatible with server" until the restore deadline, and the RestoreJob failed with nothing left to fall back on. The restore now fails before the purge, naming both versions, when they differ. The target side is spec.version, since that is what the chart renders the recovered Cluster from once the live one is gone. The backup side is CNPG's status.majorVersion, copied into the artifact's driverMetadata at backup time because retention deletes the cnpg.io/Backup while the artifact stays restorable; artifacts taken before that fall back to the cnpg.io/Backup. When either side is unknown the restore goes ahead as before, so older backups keep restoring. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
f714ada567 |
fix(postgres): refuse a physical restore across image flavors
A recovery replays the source's data directory, so it carries the same risk as switching the image of a live cluster: the postgresql and postgis images are built on different Debian releases, and the glibc change invalidates collatable indexes, while a postgis-to-postgresql move also loses the libraries behind every PostGIS object. The chart only guarded the live cluster, so a fresh release bootstrapped from a backup of the other flavor went through, and the guard's own advice was to restore a backup, which is that very move. The backup-controller now records the source flavor in the snapshot it stores on each Backup and fails a RestoreJob whose target runs another flavor, before the purge deletes the target's Cluster and PVCs. A snapshot without a flavor predates flavors and so comes from a postgresql cluster. For the chart-managed bootstrap, the chart compares against the recovery source's Cluster while it still exists. Both paths, and the live-cluster guard, point a flavor change to a logical dump. 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> |
||
|
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
|
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 KolkovandClaude Opus 4.8
|
5bcdb8cfb8 |
fix(postgres): let in-place restore re-bootstrap instead of wedging
An in-place RestoreJob (targetApplicationRef omitted) re-bootstraps the
Postgres app under its own name. On stock v1.6.2 it never comes back up,
for two independent reasons in the same restore patch - both surface only
in place, because a to-copy restore targets a different app.
1. Archive-vs-recovery serverName collision. The driver set the restored
cluster's WAL-archive serverName equal to the recovery source
(bootstrap.serverName = bootstrap.oldName = sourceServerName) while the
chart pinned the archiver serverName to the release name. In place those
coincide, so the cluster archived onto the source's own WAL prefix and
barman-cloud-check-wal-archive refused to start ("Expected empty
archive"). Fixed by giving the restored cluster a fresh
bootstrap.newServerName, keyed by the RestoreJob UID (unique per
restore, stable across reconciles); the chart archives there while
externalClusters still recovers from bootstrap.serverName. This is what
the manual CNPG strategies hit (serverName defaults to the cluster
name); the cozy-default flow dodged it (serverName is <ns>-<app>).
2. useSystemBucket guard. The restore populates explicit S3 coordinates
(destinationPath / s3CredentialsSecret / endpointCA) but left
backup.useSystemBucket=true, so the chart's "bootstrap.enabled is
incompatible with useSystemBucket" guard failed the HelmRelease upgrade
and the cluster never re-rendered. This is what the cozy-default
(system-bucket) flow hits on an in-place restore. Fixed by clearing
useSystemBucket in the restore patch, since the app is now on the
explicit-creds path. The driver's app view (postgresapp.Backup) gains
the field, without omitempty so a false survives the merge patch.
Both are exercised only by an in-place restore; the postgres backup e2e
drove only to-copy, so neither surfaced in CI. A step 50 in-place restore
now guards reason 1 (manual-strategy flow); reason 2 is covered by a Go
unit assertion (a system-bucket in-place e2e needs the platform bucket in
CI and is left as a follow-up).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Andrey Kolkov
|
ed5ae16e36 |
feat(platform): implement backup strategy for postgres application
Signed-off-by: Andrey Kolkov <androndo@gmail.com> |