mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-26 21:17:32 +09:00
main
61
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
1d813c9445 |
fix(backups): refuse concurrent restores and skip the CNPG purge only for this restore's Cluster (#4754)
## What this PR does Fixes #3322. An in-place Postgres (CNPG) restore decided whether to skip deleting the live `Cluster` by comparing its `creationTimestamp` with the RestoreJob's `StartedAt`. The timestamp says the Cluster is new, not which restore rendered it, so two cases let a restore skip its purge and report `Succeeded` on data from another restore: 1. Two RestoreJobs into the same application at once. The second sees the Cluster the first produced, created after its own `StartedAt`, and skips its purge. 2. The live Cluster is deleted while the restore waits for the WAL archive. The still-active HelmRelease re-renders it from the previous restore's values, again created after `StartedAt`. ### Refuse a restore into an application another restore holds The RestoreJob controller now refuses, for every driver, a RestoreJob that has not started yet when another unfinished RestoreJob in the namespace writes into the same application and goes first. The refused job fails with reason `ConcurrentRestore`, and its message names the job that goes first. It fails instead of waiting, so the application is never restored a second time later, after the first restore, by a job its author may no longer expect to run. A started restore goes before one that has not started. Between two that have not, the older goes first, by creation time and then by name; creation times have one-second precision, so two jobs created in the same second are ordered by name. A running restore is never failed by a newer one. The target application is resolved by `restoreJobTarget`, the same helper the BackupJob hold uses, so a Velero restore into another namespace does not collide with an in-place one. The gate lists RestoreJobs through the manager's API reader, not the informer cache. A job reconciled before the cache holds its peer's `StartedAt` would otherwise see that peer as not started and, on a same-second name tie, proceed alongside it. The controller runs a single worker, so a peer's `StartedAt` written by an earlier reconcile is already on the API server when the next job reads it; `SetupWithManager` now sets `MaxConcurrentReconciles: 1` explicitly, since the guarantee depends on it. ### Skip the CNPG purge only for this restore's Cluster The restore patch already sets `bootstrap.newServerName` to `restoredServerName(cluster, RestoreJob UID)`, a value unique to each RestoreJob. The postgres chart now annotates the `Cluster` with it as `postgres.cozystack.io/restored-server-name`. The annotation is rendered even when the barman plugin is not; the plugin's `serverName` also carries the value, but the plugin renders only with chart-level S3 coordinates. The driver skips the purge only when the annotation names this RestoreJob. A Cluster produced by a concurrent restore, or re-rendered from an earlier restore's values, carries another value and is purged. The mark lives on the rendered Cluster rather than in RestoreJob status. That keeps it valid in the case the skip exists for: the purge succeeded, but the `TargetPurged` status write failed. A UID recorded in status was rejected in the code for exactly that reason. A Cluster rendered by a chart that predates the annotation has none and keeps the creation-time check, so a restore in flight across an upgrade behaves as before. ### Tests - `TestRestoreAhead` covers order, a started job going first, the name tie-break, finished and terminating jobs, other applications, retargeted restores and a Velero restore into another namespace. - `TestReconcileRestore_ConcurrentRestoreIsRefused` checks that the newer job fails with `ConcurrentRestore` and the running one is left alone. - `TestReconcileRestore_ConcurrentRestoreReadsPastTheCache` gives the cache a stale, not-started peer and the API reader the started one, and requires the job to fail with `ConcurrentRestore`. - `TestCNPGClusterFreshlyRecovered` adds both cases from the issue: a Cluster marked by another restore and created after `StartedAt` must be purged. The timestamp-only cases are kept for the unmarked fallback. - `TestReconcileCNPGRestore_RepeatInPlacePurgesStaleRecoveryCluster` gains two reconcile-level cases. One purges a Cluster that another restore marked, even though it was created after `StartedAt`. The other runs a full purge, takes the mark from the `bootstrap.newServerName` the restore patch wrote, drops `TargetPurged` as a lost status write would, and requires a Cluster carrying that mark to survive, although it predates `StartedAt`. That pins the purge decision to the value the chart actually renders. - `packages/apps/postgres/tests/restore_mark_test.yaml` checks that the annotation is rendered with and without the barman plugin, and is absent on a Cluster that was never restored. Each new test was checked against the code it guards: disabling the gate call, listing through the cache, flipping the started-job precedence, ignoring the annotation, or deriving the expected mark from the app name instead of the cluster name each turns one red. `go test ./internal/backupcontroller/...` and `helm unittest packages/apps/postgres` (152 tests) pass. The restore flow was not run against a live cluster. `docs/operations/backup-classes.md` documents the one-restore-at-a-time rule and the annotation. ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff: Go changes in `internal/backupcontroller` (no CRD or API type change), one annotation added to the postgres chart's Cluster (no `values.yaml` or `values.schema.json` change), and a docs paragraph. No package is added or renamed, no platform values, ApplicationDefinition, RestoreJob CRD, metric, label or namespace changes, so no trigger is reached. - [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): a RestoreJob now fails with reason ConcurrentRestore when another unfinished RestoreJob is already restoring the same application, instead of running alongside it; create it again once the first restore has finished. An in-place Postgres restore now skips deleting the live Cluster only when that Cluster was rendered for this RestoreJob, identified by the new postgres.cozystack.io/restored-server-name annotation, so it no longer reports Succeeded on data from a concurrent or earlier restore. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Restores targeting the same application are now handled one at a time. A competing restore is refused rather than queued, with started restores taking priority. * PostgreSQL now calculates a default `shared_buffers` value when the memory limit is below 512 MiB, while respecting explicitly configured values. * **Bug Fixes** * Recovery clusters are more reliably distinguished from manual recoveries and clusters created by other restores, reducing the risk of purging the wrong cluster. * **Documentation** * Clarified restore ordering and how recovery-cluster cleanup is determined. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
e4e991e837 |
fix(backups): skip the CNPG restore purge only for this restore's Cluster
The in-place CNPG restore skips deleting the live Cluster when it carries bootstrap.recovery and was created after the RestoreJob started, taking that as "this restore's own purge and re-render produced it". The timestamp does not say which restore rendered the Cluster. A Cluster deleted while the restore waits for the WAL archive is re-rendered by the still-active HelmRelease from the previous restore's values and is created after StartedAt too; the restore then skipped its purge and reported Succeeded on the previous restore's data. The restore patch already sets bootstrap.newServerName to restoredServerName(cluster, RestoreJob UID). The chart now annotates the Cluster with it as postgres.cozystack.io/restored-server-name, whether or not it renders the barman plugin that also carries the value, and the purge is skipped only when the annotation names this RestoreJob. The mark lives on the rendered Cluster rather than in RestoreJob status, so it still covers the failed status write the skip exists for. A Cluster rendered by a chart that predates the annotation keeps the creation-time check. Fixes #3322 Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Yasar Rahman
|
f6f879cf38 |
fix(postgres)!: refuse plaintext sessions on external instances
CloudNativePG offers TLS but its built-in last pg_hba rule, host all all all, also accepts plaintext, so a client outside the cluster that connects with sslmode=disable gets an unencrypted session. tls.enabled only decides whether the external hostname goes into the certificate and cannot prevent this. When external is true the chart now renders spec.postgresql.pg_hba with hostssl all all all scram-sha-256 followed by host all all all reject. CloudNativePG places user rules after its fixed rules and before its default rule, and the first match wins, so only TLS sessions are accepted. The pod network cannot be told apart from an external client, so in-cluster clients are held to the same rule; libpq's default sslmode=prefer already negotiates TLS. The new tri-state tls.required follows external when omitted. Setting it to false keeps accepting plaintext on an external instance and true requires TLS on an in-cluster one. The hostssl rule uses the same password method as CloudNativePG's own default rule (md5 before PostgreSQL 14, scram-sha-256 from 14), so existing md5 verifiers keep working on PostgreSQL 13. Rendering fails when postgresql.parameters.ssl is off while TLS is required, since hostssl would never match and every remote session would be refused. BREAKING CHANGE: external clients that connect without TLS are refused after the upgrade. Set tls.required to false to keep accepting them. Assisted-by: LLM Signed-off-by: Yasar Rahman <yasar.rehman@gmail.com> |
||
|
Yan Bondarenko
|
9c0085c358 |
fix(postgres): cap shared_buffers at the memory request
CloudNativePG rejects clusters whose shared_buffers exceeds the memory request. Memory overcommit can put that request below both a quarter of the limit and the 16MB floor. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
6dbd76ae50 |
fix(postgres): cap the default shared_buffers at a quarter of the memory limit
The chart renders no shared_buffers, so PostgreSQL starts with its 128MB default whatever the container gets. A full pool is the whole memory limit on t1.nano and half of it on t1.micro, the default preset. The pool is shared memory that the kernel cannot reclaim without swap, so when a dump, index scans over a large table or a replica catching up after a failover fill it, the instance is OOMKilled. Instances below a 512Mi memory limit now default to a quarter of the limit, at least 16MB. From 512Mi up a quarter is at least 128MB, so those instances render exactly as before. shared_buffers needs a restart, so on upgrade CloudNativePG restarts every instance below 512Mi that does not set it explicitly. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
48e3d1d611 |
feat(redis,postgres): certify the per-engine external hostname
The tenant Gateway's TLS-passthrough listeners are designed to route databases by SNI under a per-engine subdomain (*.redis.<apex>, *.postgres.<apex>), and a passthrough listener forwards the handshake untouched, so the engine's own server certificate must carry <release>.<engine>.<apex> or a verifying client fails the hostname check. The name is issued next to the existing flat <release>.<apex>, under the same TLS-and-external condition, so that a later move between the two hostname schemes reissues no certificate and leaves clients on the same trust anchor. Nothing renders a route yet; the certificate is prepared ahead of it. Adding the name changes the SAN list, so existing external TLS releases get their server certificate reissued once on upgrade, from the same CA. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
21fa1fcdb1 |
feat(postgres): add per-user createRole and bypassRls attributes
Without these, a tenant that needs to manage its own roles has no option but the postgres superuser, which also hands over filesystem access and command execution on the instance. Role attributes are not inherited through role membership, so the generated <db>_admin group role cannot carry them: they have to be set on the login user itself. users.<name>.createRole and users.<name>.bypassRls set CREATEROLE and BYPASSRLS on a managed login user. Both are reconciled on every init-script run, so setting one back to false emits the NO... form and revokes the attribute. createRole is refused on PostgreSQL 15 and older. Before PostgreSQL 16, CREATEROLE allowed granting membership in any non-superuser role, so a user holding it could grant itself pg_execute_server_program and reach command execution on the instance, which is exactly what not exposing the superuser prevents. PostgreSQL 16 restricted CREATEROLE to roles the grantee administers. bypassRls carries no such gate: BYPASSRLS only waives row-level security inside the tenant's own databases, whose owner already reads every row, so it adds no reach. The two attributes are needed together, not separately. PostgreSQL only lets a role create other roles carrying BYPASSRLS if it holds the attribute itself, so provisioning a privileged service role fails with "Only roles with the BYPASSRLS attribute may create roles with the BYPASSRLS attribute" when only createRole is granted. createRole also sets createrole_self_grant as a role-level default. PostgreSQL 16 grants a CREATEROLE holder only ADMIN OPTION on the roles it creates, governed by that parameter, whose default is empty. ADMIN OPTION allows granting the role onward but not SET ROLE to it, and creating an object owned by another role requires SET. The attribute therefore looks functional -- CREATE ROLE succeeds -- while the next statement a tenant writes, CREATE SCHEMA ... AUTHORIZATION <new role>, fails with "must be able to SET ROLE". The default is reset when the attribute is withdrawn. The init job runs as the superuser, and before this change it trusted two things a CREATEROLE holder controls for the roles it creates: role comments and role names. It picked the users and roles to drop by their comment, and a PostgreSQL 16 CREATEROLE holder may comment every role it created, then hand it to the chart for deletion. It adopted any role that already existed under a declared name, so a role created under a name an administrator declares later stayed under its creator's control. Neither can be caught from the catalog: creating a role through an intermediate role and then dropping that role leaves the squatted role with no member at all, while the objects, settings and default privileges planted in it stay. The chart therefore records the roles it manages as members of "cozystack:managed", a role only the superuser can grant, and the delete passes act only on those. Its first run marks every role the chart had commented before, when no role could yet hold CREATEROLE through it. Once any user has held createRole, recorded as a setting of the postgres database, the chart refuses to adopt a role it did not mark and fails with the role's name, rather than hand a squatted role REPLICATION or the ownership of a database. Verified against live PostgreSQL instances. On 17 a migration checklist replayed as the login user, with no superuser: role creation with attributes, CREATE SCHEMA ... AUTHORIZATION with the requested owner, extensions in a named schema, per-role search_path and timeouts, RLS including FORCE ROW LEVEL SECURITY, and SECURITY DEFINER functions all succeed. On 17 the forged-comment and squatting paths are refused or ignored, the upgrade from the previous chart marks the roles it already managed, and a fresh install still adopts the role CNPG bootstraps for users.app. On 13, 15 and 16 the init script runs and re-runs cleanly. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
9734a16692 |
feat(postgres): add a postgis image flavor (#4587)
<!-- 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 Adds a `flavor: postgresql | postgis` enum to the `postgres` app. `postgis` runs the cluster on the CloudNativePG PostGIS operand image, so tenants can enable `postgis`, `postgis_raster`, `postgis_topology`, `postgis_sfcgal`, `postgis_tiger_geocoder` and `address_standardizer` through the existing `databases.<name>.extensions`. The default stays `postgresql` and renders exactly the image it rendered before this change. This picks up #2672 by @matthieu-robin, which the stale bot closed after a rebase request, with these changes: - The postgis tags in #2672 (`17.7-3.5`, `16.11-3.5`, ...) do not exist in `ghcr.io/cloudnative-pg/postgis`; the registry answers 404, so the flavor would have ended in `ImagePullBackOff`. The map now uses tags that exist, `<minor>-<postgis>-standard-trixie`, on the same PostgreSQL minor as `files/versions.yaml`. - v18 is supported: PostGIS images for 18 now exist upstream. - `standard` rather than `system`: the chart archives through the barman-cloud plugin since the backup rework, so the operand no longer needs the barman binaries of the deprecated `system` variant. - `hack/update-versions.sh` derives the postgis map from the minors it picks, and `hack/check-postgres-postgis-versions.bats` fails if the two maps drift to different minors or away from `standard-trixie`. - The chart refuses a flavor change on a live `Cluster`. The default images are built on Debian bullseye (glibc 2.31) and the postgis ones on trixie (glibc 2.41): CNPG would roll the swap out as a plain image update, under an initialised data directory, which silently invalidates every index on collatable text, and the postgis-to-postgresql direction also drops the libraries behind every PostGIS object. Only the image name is compared, so minor bumps and registry mirrors keep passing. - `pgrouting` is not claimed: it is absent from the `18.1-3.6.1` build the chart pins and only appears in later postgis builds. A general mechanism (CNPG ImageVolume extensions) was considered and left out: it needs PostgreSQL 18 and Kubernetes 1.35 or the `ImageVolume` gate, so it cannot serve v13-v17 nor most management clusters today, and PostGIS is the only extension CloudNativePG ships as a full operand image. No other extension request exists in this repository. Testing: - `helm unittest packages/apps/postgres`: 145/145, including 6 new cases in `tests/flavor_test.yaml` (default image, postgis on the Cluster and on the init Job, the guard in both directions with a mocked live Cluster, and a mirrored digest-pinned image taking a new minor). Removing the guard makes exactly the two guard cases fail. - `hack/cozytest.sh hack/check-postgres-postgis-versions.bats` passes, and fails on a shifted minor or a `system` tag. - `make -C packages/apps/postgres generate` leaves no diff. - The rendered `ghcr.io/cloudnative-pg/postgis:18.1-3.6.1-standard-trixie` was started locally: `CREATE EXTENSION` for citext, postgis, postgis_raster and postgis_topology succeeds and `ST_Buffer` answers. A three-instance cluster on the `system` sibling of that image has been running PostGIS in production since 2026-09-29. ### 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. --> - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#63 (issue: the provider models every field by hand, and the default handling needs a decision there) - [ ] [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 feat(postgres): add a `flavor` field accepting `postgresql` (default) or `postgis`. `postgis` bootstraps the cluster from the CloudNativePG PostGIS image, so PostGIS extensions can be enabled per database through `databases.<name>.extensions`. The flavor is fixed at creation: changing it on an existing cluster is refused, and so is restoring a backup into a cluster of the other flavor, whether through a RestoreJob or `bootstrap.enabled`; move data across flavors with a logical dump. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a `flavor` option for PostgreSQL clusters. The default `postgresql` flavor remains available, while `postgis` selects an image with PostGIS extensions. - PostGIS images are available for PostgreSQL versions 13–18. - Cluster flavor is fixed at creation. To switch flavors, create a new cluster and migrate data with a logical dump. - **Bug Fixes** - Restores are refused when the backup and target cluster use different flavors. - **Documentation** - Updated PostgreSQL configuration guidance to describe flavor options and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
fb7e780517 |
fix(postgres): reassign dropped roles' objects to an unprivileged role (#4574)
## What this PR does Objects owned by a role the Postgres init-job drops no longer end up owned by the `postgres` superuser. This covers a user removed from `users` and an orphaned managed `<db>_admin` or `<db>_readonly` role. Before dropping such a role the job reassigned everything it owned to `postgres`, and objects a tenant role created should not become superuser-owned. They now go to `cozystack:orphaned`, a NOLOGIN role that is granted nothing. The init-job creates it and resets its attributes on every run. The `:` in the name is outside the charset allowed for user and database names, so values cannot declare it. There is a trade-off. Grants and memberships of the dropped role do not carry over, so a view or function it owned that ran with its owner's privileges can lose access it had through them. To keep such objects working, keep the user in values or move the objects to another owner before removing it. `docs/operations/backup-classes.md` is updated accordingly. Role names read from the catalog now reach SQL as psql variables instead of being spliced in by the shell. The queries are echoed after substitution, so the job log shows which role is dropped. Tested with helm-unittest and by running the rendered init script against PostgreSQL 13 and 18. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(postgres): objects owned by a user removed from `users`, or by an orphaned managed database role, are now reassigned to the unprivileged `cozystack:orphaned` role instead of the `postgres` superuser. A view or function of the removed role that ran with its owner's privileges can lose access it had through that role's grants or memberships. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that after restore, the first initialization releases undeclared managed databases without deleting their data and removes users not declared in the chart. * Removed users’ objects are reassigned to a restricted, non-login role. Grants and memberships are not preserved; functions use invoker rights, and PostgreSQL 15+ views are set to use invoker security. * Added guidance for finding eligible objects still owned by the PostgreSQL superuser for manual reassignment. * Declare a user in `users` to keep it; the chart will generate a new password. * **Bug Fixes** * Removed users’ objects are reassigned across connectable, non-template databases before their users are dropped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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 EleuteriandMatthieu
|
dc2971264d |
feat(postgres): add a postgis image flavor
Tenants migrating spatial data need PostGIS, but the chart always runs the CloudNativePG PostgreSQL image, which does not ship it, and exposes no way to pick another one. CloudNativePG publishes a PostGIS operand image; a closed `flavor` enum selects it without handing tenants a free image field. The flavor pins the same PostgreSQL minor as the default one, on the standard-trixie variant: backups go through the barman-cloud plugin, so the deprecated system variant is not needed. update-versions.sh now derives the postgis map from the minors it picks, and a bats check fails if the two maps drift apart. The default images are built on bullseye and the postgis ones on trixie. Switching a live cluster between them would swap glibc under an initialised data directory and silently break every collatable index, and CNPG would roll it out as a plain image update. The chart therefore refuses a flavor change on an existing Cluster. Co-authored-by: Matthieu <matthieu@hidora.com> Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
810d293421 |
fix(postgres): reassign dropped roles' objects to an unprivileged role
When a user leaves values, or a managed database role is orphaned, the init-job reassigned everything the role owned to the postgres superuser before dropping it. Objects a tenant role created should not become superuser-owned. They now go to "cozystack:orphaned", a NOLOGIN role whose attributes and memberships the init-job resets on every run; its name is outside the charset allowed for users and databases, so values cannot declare it. Because that one role receives the objects of every dropped role, functions it owns are switched to SECURITY INVOKER and, on PostgreSQL 15 and later, its views to security_invoker. The cost is that grants and memberships of the dropped role do not carry over, so a view or function it owned can lose access it had through them; keep the user in values, or move such objects to another owner first, to preserve it. The change is not retroactive: objects earlier releases reassigned to postgres stay superuser-owned, and the backup docs describe how to find and hand them over. The orphaned-roles pass now reassigns and drops in every connectable database, as the user pass already did, so a role owning an object outside the default database no longer fails DROP ROLE on every run. Role names read from the catalog now reach SQL as psql variables instead of being spliced in by the shell, and the queries are echoed after substitution so the log names the role being dropped. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
25cdd51881 |
feat(postgres): allow preloading an allowlist of shipped modules
shared_preload_libraries is blocked in postgresql.parameters, and rightly so: the parameter is free text and accepts a path, so it can point at a writable location such as the PGDATA volume. What the block leaves is an indirect route. CloudNativePG preloads pg_stat_statements, auto_explain and pgaudit by itself as soon as a parameter under their prefix is set, such as pg_stat_statements.track, so enabling one of them today means setting one of its tuning parameters, even to its default value. pg_prewarm has no route at all, and its autoprewarm worker only runs when the module is preloaded. postgresql.sharedPreloadLibraries is the explicit switch. It is a closed enum of four module names that already ship in the CloudNativePG PostgreSQL image, rendered onto the CNPG Cluster field of the same meaning, which the operator merges with the libraries it preloads on its own. A bare name can only resolve inside $libdir, which sits on the read-only image filesystem, and dynamic_library_path stays blocked, so no path can be introduced. The blocklist on postgresql.parameters is left untouched. The template repeats the enum as a backstop for renders that skip schema validation. helm-unittest always validates the schema and never reaches it, so a bats suite renders with --skip-schema-validation to keep the backstop, and its agreement with the enum, under test. Verified on a live CloudNativePG 1.27.3 cluster: with the field set, the operator writes shared_preload_libraries into postgresql.conf, SHOW reports the modules, and pg_stat_statements returns rows. Changing the list restarts PostgreSQL on every instance, the primary included, through the instance manager and without restarting the pod. 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> |
||
|
IvanHunters
|
2fa51bf587 |
test(charts): isolate guard coverage for every guarded template
Add per-template guard suites so each guarded hostname site is exercised on its own: harbor harbor.yaml and httproute.yaml, bootbox machines.yaml, seaweedfs cosi-deployment.yaml, bucket deployment.yaml and httproute.yaml, monitoring alerta.yaml, grafana httproute.yaml and the grafanaHost helper via oidc-keycloak.yaml. Each suite renders only its target template, so removing that file's guard reddens it. Also add a postgres positive case that renders the Certificate with an apex set (external=true). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
0f4d39f1ef |
test(charts): add empty-apex guard tests for the remaining guarded charts
Add host_guard_test.yaml for postgres, vpn, bootbox, seaweedfs, monitoring and bucket. Each renders its guarded template with a non-empty injected _namespace whose host is empty and no per-app host override, and asserts the ns-host-required failure, plus a positive render case with the apex set. Completes guard coverage alongside the existing harbor, qdrant and kubernetes tests. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
237f2e2bbd |
fix(postgres): say how to get past a refused extension clause
Before the charset guard, an extension value was spliced into CREATE EXTENSION whole, so a release can carry a clause after the name, such as earthdistance CASCADE, and that is the usual way to pull in a dependency. The guard refuses such a value on upgrade too, and keeping it that way is right, but the tenant needs to know what to write instead. The reject message now says: drop the clause, which changes nothing for an extension that is already installed because the script never drops one and CREATE EXTENSION IF NOT EXISTS leaves it alone, and list the dependencies first, since extensions are created in list order. 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> |
||
|
Alexey Artamonov
|
2b5877eff9 |
feat(postgres)!: generate user passwords and read them at runtime
users[].password is gone from the chart, its schema and the API type. Every application-user password is generated into <release>-credentials and kept across reconciles by lookup, so an upgraded release keeps the password it already has and a password left in values is ignored. The init-job mounts that Secret and applies each password with ALTER ROLE and a psql variable, so no password is rendered into the init-script Secret that Helm keeps in release history. While bootstrap is enabled the Secret is marked credentials-pending, and NOTES say the generated passwords apply once the init-job runs. Names reach superuser SQL and the init-script shell, so render-time guards reject a username, database or extension name outside [-._a-zA-Z0-9] or past the length PostgreSQL silently truncates: 63 bytes for a role or extension, 54 for a database, whose <db>_readonly role would otherwise truncate and fail the next run. A username over 63 bytes that the credentials Secret already carries still upgrades. users.postgres, the CNPG superuser, is rejected in every bootstrap state. Changing a database's key in values drops the database, so the reject message and the README give the rename that keeps the data: a platform administrator renames it and its roles as the CNPG superuser before the key changes. The init-job cannot hang its Helm hook: the readiness wait ends on elapsed time below activeDeadlineSeconds and the Job never retries, so a purged restore target fails with a diagnostic. The delete-users pass reads managed users from pg_shdescription instead of parsing the version-dependent \du+ layout, which on PostgreSQL 16 and later matched nothing and left a removed user able to log in; it compares names as CREATE ROLE stores them and reassigns a removed user's objects in every database before dropping it. A database without roles renders, and extension names are quoted for hyphenated names and folded to lowercase so they resolve as they did unquoted. A value that carries its own surrounding quotes, the only way a hyphenated extension rendered before, has that pair removed and keeps its case, so such a release still upgrades. The README reads a password with jq, since jsonpath splits a dotted key. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
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> |
||
|
Alexey Artamonov
|
9033d41bc0 |
test(postgres): assert the job pin on each autoscaler selector, not once for the query
The pin check matched job="kube-state-metrics" anywhere in the trigger query, so one pinned selector satisfied it no matter what the other three did: unpinning $joinReplica, $idleFloor or $joinCluster individually left the suite green while the join went back to matching any exporter's series. Anchor one assertion per selector on the label that identifies its site so a single dropped pin fails. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
1f2ff31470 |
fix(postgres): pin job=kube-state-metrics on the autoscaler read-load query
The read-load trigger query joined on kube_pod_labels without pinning job="kube-state-metrics", while the sibling DatabaseAutoscaler* alert rules already pin it. A second exporter emitting a kube_pod_labels series (or a duplicate scrape) would multiply the join and distort the desired-count arithmetic. Pin the job on every kube_pod_labels selector in the query for parity with the alerts. Follow-up from #3954 (#4187). Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
9511f56307 |
fix(postgres): require service.namespace.svc shape in the autoscaling SSRF host guard
The serverAddress host regex accepted any dotted tail ending in .svc, so a single-label host like foo.svc passed the in-cluster-DNS check. It then reached the namespace-pin, whose extraction regex left the unresolvable host unchanged and rejected it as a namespace outside the tenant subtree - the right outcome by the wrong path, with a misleading message. Anchor the host to two DNS labels before .svc so it is refused at the host check with the actionable message, and drop the stray dot the old label class allowed. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
3a09c3e9dc |
fix(postgres): guard dry-run/transition against the paused HPA min-clamp
Closes IvanHunters' MAJOR on scaledobject.yaml:39. The render guard rejected replicas > maxReplicas but not the symmetric low side. Under dry-run/transition the chart renders the static instances=replicas while the ScaledObject carries minReplicaCount=effectiveMin with both scale directions paused. The pause disables the HPA's scaling policies, not its min-replica clamp, so when replicas < effectiveMin the HPA raises the live count to the floor on its first reconcile — a scale-up in a mode documented as "actuates neither way" — and because the rendered instances value never changes, the client-side two-way patch never pulls it back, leaving permanent extra standbys and PVCs. Fail closed and tell the operator to stage replicas to the floor first. Under dry-run/transition this floor guard is strictly broader than the db.yaml quorum wedge (any replicas <= maxSyncReplicas is also < effectiveMin), so it supersedes it there; the wedge stays exercised on the disabled path. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
187f18a167 |
fix(postgres): harden autoscaler serverAddress guard and cap maxReplicas
The serverAddress SSRF guard had two holes beyond the external-domain suffix already closed. It extracted the host by truncating at the first `:` or `/`, which a `http://<svc>.<ns>.svc:port@evil.example.com/` userinfo payload defeats: the truncated text passes the `.svc` check while net/url dials the host after the `@`. And it validated only the host shape, never the namespace, so a tenant could point the cluster-privileged KEDA operator at a sibling tenant's or a system namespace's Service (the egress NetworkPolicy's `cluster` entity does not stop in-cluster egress). Parse the URL authority properly (scheme, path/query/fragment, userinfo to the last `@`, then port) and pin the host's namespace to the tenant's own subtree via cozy-lib.rbac.allParentTenantsAndThis. The namespace pin narrows what a tenant may set and needs API-owner sign-off (noted at the guard). Separately, nothing bounded maxReplicas: the schema carries no maximum and the aggregated apiserver does not range-validate the CR, so a tenant could set maxReplicas: 100000 and drive KEDA to scale CNPG toward that count. Add a render guard rejecting a maxReplicas above a sane read-replica ceiling. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
2bf37f8260 |
fix(postgres): anchor autoscaler SSRF guard to the cluster domain
The serverAddress in-cluster check accepted an unanchored trailing domain (`\.svc(\.[a-zA-Z0-9.-]+)?$`), so a crafted host like x.svc.attacker.com — a `.svc` label followed by any external domain — passed the guard. A tenant could then point the cluster-privileged KEDA operator off-cluster (the egress NetworkPolicy is the only remaining barrier, and it is inert where the CNI does not enforce CiliumNetworkPolicy). Anchor the optional suffix to this cluster's domain (from _cluster.cluster-domain, injected by the platform from networking.clusterDomain): a legitimate vmselect is <service>.<namespace>.svc or ...svc.<clusterDomain>, nothing else. The prior tests only exercised non-.svc hosts and gave false confidence; add a case for the .svc.<external> bypass. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
6083a9fa1d |
feat(postgres): guard replicas>maxReplicas and pin serverAddress to in-cluster
Close two render-guard follow-ups the design proposal names for this PR: - Fail closed when replicas exceeds autoscaling.maxReplicas. The stock HPA's currentReplicas > maxReplicaCount branch writes /scale = maxReplicaCount unconditionally on the first reconcile — even under the both-direction pause of dryRun/transition — so a resting/seed count above the ceiling would shed the surplus standbys and their PVCs in one step, past the scale-down pacing. Mirrors the existing effectiveMin > maxReplicas guard. - Refuse an autoscaling.serverAddress whose host is not an in-cluster Service DNS name (…svc[.<domain>]). The cluster-privileged KEDA operator fetches it every poll, so an external/IP address is an SSRF surface (cloud metadata endpoint, another tenant); the query body is already namespace-scoped, this restricts the destination. Defense-in-depth pairs with the operator egress policy (keda package). Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
c693ed36fb |
fix(postgres): force client-side apply so the autoscaler seed survives
Under active read-replica autoscaling the chart seeds spec.instances at a constant max(replicas, effectiveMin) rather than omitting it (an omitted field defaults to 1, which the CNPG quorum webhook rejects, and — under server-side apply — the phase-2 omit prunes the field whenever KEDA has not yet written /scale, collapsing an idle-at-floor cluster to 1 and shedding its replica PVCs). Keeping the constant only works if Flux does not force-own and revert it. The postgres ApplicationDefinition therefore carries release.cozystack.io/helm-server-side-apply: "false", so the tenant HelmRelease applies client-side: helm-controller patches the CNPG Cluster from the previous-vs-new rendered manifest without consulting live state, the unchanged constant yields no patch, and KEDA's higher live /scale count is preserved. The constant also equals the disabled/dryRun/transition render (instances: replicas) whenever replicas sits at/above the floor, so those flips are patch-free too. Correct the merge-semantics comments across db.yaml, _autoscaling.tpl and values.yaml to describe the two-way previous-vs-new patch precisely (not a "three-way no-op"), and document the residual floor-change hazard: editing replicas / minReplicas / quorum.maxSyncReplicas moves the seed, so that one reconcile does patch spec.instances to the new seed. Add autoscaling.serverAddress so a monitoring:true tenant can point the ScaledObject at a vmselect that carries both the CNPG metrics and kube_pod_labels. Regenerated README/schema/types/cozyrds. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
826f08972f |
fix(apps/postgres): make autoscaler fail-safe, dip-free, and observable
Address review findings on the KEDA read-replica autoscaler where the
implementation diverged from the merged design proposal.
Fail-safe read load. Gate the `or vector(0)` floor on the existence of ANY CNPG
instance pod (`count(kube_pod_labels{...instance_role=~".+"}) > 0`) — primary or
replica — so the query is empty ONLY when no CNPG pod labels exist at all (a
broken kube-state-metrics pipeline), not during initial provisioning when the
sole replica-side pod is CNPG's join Job (jobRole, not instanceRole). Also pin
`ignoreNullValues: "false"` on the trigger: KEDA's prometheus scaler defaults it
true, turning an EMPTY result into metric 0 (not an error) — the HPA would then
desire 0 and clamp to the floor, i.e. silently scale a broken-pipeline cluster
DOWN. With it false an empty result is a scaler error, so the HPA HOLDS and
DatabaseAutoscalerScalerErrors fires; gating on any instance role keeps a healthy
cluster (including a long basebackup) from tripping that alert.
Constant seed = max(replicas, effectiveMin), so enable/dryRun/transition flips
are a merge no-op with no dip and never undercut the quorum floor.
Fail closed when the quorum floor exceeds maxReplicas (was: effectiveMax silently
raised both bounds, unbounded for a large maxSyncReplicas).
Dry-run / transition keep the HPA (unidirectional pause pair, not the HPA-deleting
full pause) but both SelectPolicy=Disabled, so desired normalizes to current — the
recommendation is read from the read-load metric, not a diverging desired count.
Observability. DatabaseAutoscalerAtMax keys on desired >= maxReplicas, NOT the
ScalingLimited condition (which is also true at the floor, so it fired on every
idle cluster with wrong "raise maxReplicas" advice). Dashboard read-load panels
are per-cluster (label_cnpg_io_cluster) with a CPU panel for ReadCPUUtilization
and the exact clamp in descriptions; ScaleStuck's Pending intersection is scoped
to CNPG pods; alerts.yaml narrative comments moved to Go-template comments so they
do not ship into the PrometheusRule.
Guard the seeding invariant: TestBuildHelmReleaseSpecNoForceOrDriftDetection.
Regenerated README/schema/types.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
b40cf82642 |
fix(postgres): seed instances on both paths, add input guards, and harden alerts
Address IvanHunters' deep review round on #3954: - [MAJOR] Enabling autoscaling directly on an existing non-quorum database no longer collapses it to 1 instance and deletes every replica PVC. The chart now SEEDS spec.instances at the quorum floor (effectiveMin) on BOTH the quorum and non-quorum active paths instead of omitting the field in the non-quorum case; the constant seed is a three-way-merge no-op so KEDA still owns the runtime count via /scale, and the enable dip is capped at the floor (>=2) rather than collapsing to 1. Documents the client-side-3-way-merge (not SSA) assumption that the no-op relies on. - [MINOR] Fail closed on autoscaling.minReplicas > autoscaling.maxReplicas instead of silently resolving the ceiling upward (which defeated maxReplicas as a cost knob). The stray maxReplicas: 0 case now rejects rather than clamps. - [MINOR] The disable-wedge guard, its values note and its test no longer emit the invalid `kubectl scale --subresource=scale` (scale has no --subresource); use `kubectl scale cluster.postgresql.cnpg.io/<name> --replicas=N`. - [MINOR] Correct the enablement note: raising minReplicas/maxSyncReplicas on a live cluster is NOT harmless when KEDA's live count exceeds the new floor (it decommissions surplus replicas + PVCs); prescribe pin-first for raises too. - [MINOR] Add DatabaseAutoscalerKedaOperatorDown (absent(keda_build_info)): when the operator is unhealthy a ScaledObject never becomes an HPA and no other alert can fire. Intersect DatabaseAutoscalerScaleStuck with a Pending pod so a healthy long basebackup (Running pod, desired>current >15m) no longer false-fires. - [MINOR] Keep the KEDA CRDs on package uninstall (crds.additionalAnnotations resource-policy: keep via the shim) so removing the keda package doesn't cascade-delete every tenant ScaledObject. - [MINOR] cozy-lib helper: reject a non-numeric bound (int "two" -> 0 -> a scale-to-zero ScaledObject) and render an explicit pollingInterval/cooldownPeriod of 0 (`if not kindIs invalid`, not `with`). - [MINOR] Note the read-load fail-open: a broken metrics join reads as idle and silently pins autoscaling at the floor under real load. New/updated tests cover the non-quorum seed, the min>max guard, the corrected kubectl message, the operator-down + Pending-intersected alerts, the CRD keep policy, and the non-numeric-bound helper guard. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
5a5047ac23 |
fix(dashboards): bind the autoscaling dashboard to a datasource variable
The database-autoscaling dashboard hardcoded "datasource": "VictoriaMetrics" on
its namespace template variable, but cozystack provisions VictoriaMetrics
datasources as vm-shortterm / vm-longterm — there is no datasource named
"VictoriaMetrics", so on import the variable query failed ("datasource not
found"), the namespace variable never populated, and every panel (which filters
on namespace=~"$namespace") showed no data. Switch to the repo-standard pattern
used by db/cloudnativepg and the other dashboards: a `datasource`-typed template
variable (ds_prometheus) referenced as { type: prometheus, uid: ${ds_prometheus} }
from the namespace variable and every panel/target (the panels previously carried
no datasource at all and only worked by falling back to the default).
Also reframe the disable-wedge guard message to lead with the invariant (replicas
must exceed quorum.maxSyncReplicas) so a pure-quorum user who never enabled
autoscaling isn't told to "disable autoscaling"; the disable path is kept as a
parenthetical cause.
Addresses the adversarial branch-review findings on #3954.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
6e3da63586 |
fix(postgres): guard the disable-wedge, role-scope the freeze count, and harden edges
Address the remaining IvanHunters review points on #3954: - [MAJOR] Disabling autoscaling wedged the HelmRelease under synchronous quorum. An autoscaled + maxSyncReplicas>=1 cluster runs with instances seeded from effectiveMin while replicas stays at its default, so flipping autoscaling off rendered instances: <replicas> next to the quorum and CNPG's webhook rejected it ("maxSyncReplicas must be lower than the number of instances"). Add a symmetric render-time guard on the static-count path (covers off/dryRun/ transition): fail with an actionable message when replicas <= maxSyncReplicas. - [MINOR] Role-scope the freeze brake's currentInstances count (label_cnpg_io_instance_role=~".+") so lingering CNPG join/bootstrap Job pods (cnpg.io/cluster but jobRole, not instanceRole) don't inflate it and nudge desired up while the brake is meant to hold it. - [MINOR] Default maxReplicationLagSeconds to 0 (brake opt-in): only the non-braking path is validated live, so ship the validated default and let the freeze be enabled explicitly until its branch is exercised under load. - [MINOR] Document the floor-change and non-quorum-disable dips (same three-way- merge hazard) and the pin-replicas-first mitigation. - [NIT] Coerce the cozy-lib helper's `paused` (eq "true") so a caller passing the string "false" does not pause the ScaledObject. - [NIT] Trim the keda.clusterDomain injection comment so it no longer ships ~8 verbatim lines into the cozystack-values Secret on every cluster. Regenerate types/schema/README/postgres-rd. New tests cover the disable-wedge guard, the role-scoped freeze count, and the string-"false" paused coercion. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
70e4087432 |
fix(postgres): guard empty monitoring namespace; clarify alert scope and helper doc
Address the branch-review nits: - Fail closed when the monitoring namespace is unset. serverAddress builds vmselect-shortterm.<ns-monitoring>.svc, and an empty ns-monitoring rendered the malformed vmselect-shortterm..svc:8481, surfacing late as a KEDA scaler error. The platform always injects it, but a hand-rendered/misconfigured tenant now gets an actionable render-time failure mirroring the KEDA-CRD and target guards. Tested. - Document that the database-autoscaling PrometheusRule matches every KEDA HPA / ScaledObject error (the underlying series carry no owning-app label), which is correct only while database autoscaling is KEDA's sole consumer in cozystack; note to scope the rules if a non-database ScaledObject is ever added. - Correct the cozy-lib.keda.scaledObject usage example: `paused` should reflect dry-run OR migration transition, not dry-run alone, so a second engine copying the example does not drop the transition pause. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
9cf4a171f0 |
fix(postgres): stop coercing numeric-zero autoscaling bounds and floor each lag gate
Two review notes from IvanHunters: - effectiveMin/effectiveMax wrapped minReplicas/maxReplicas/maxSyncReplicas/ maxReplicationLagSeconds in Sprig `default`, which treats a numeric 0 as empty and silently substitutes the fallback (e.g. maxReplicas: 0 became 6, discarding the tenant's value). values.yaml/schema already default these when omitted, so read them directly; an explicit out-of-range value now flows into max() and is clamped to the quorum floor rather than swapped for a different number. Add a test that maxReplicas: 0 clamps to effectiveMin instead of becoming 6. - The replication-lag brake only floored the product $braking with `or vector(0)`, so if a CNPG build stops emitting cnpg_pg_replication_lag or cnpg_collector_wal_records the individual gate resolved to an empty vector. Floor each gate ($lagHigh, $writing) individually so the fail-open decision is explicit and per-gate: the brake engages only on positively-observed lag AND writing, and a missing signal degrades to "scale normally" (safer than freezing on absent data) rather than relying on the emptiness of the product. Assert both gates carry the floor. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
a7b5077da2 |
fix(postgres): floor the lag-brake frozen term so a KSM gap can't empty the query
The read-load query's base term is floored with `or vector(0)`, but the replication-lag brake's frozen additive term (count(kube_pod_labels) * target) was not. PromQL `+` is a set intersection, so if count(kube_pod_labels) is momentarily empty — a kube-state-metrics scrape gap or KSM rollout — the frozen term is empty and the whole `base + frozen*braking` sum collapses to empty, defeating the base term's own graceful degradation. KEDA's prometheus scaler (ignoreNullValues: true by default) then reads that as 0 and pins desired to minReplicaCount, ignoring real read load exactly when metrics are flaky. This hits the default config (maxReplicationLagSeconds: 30, brake active). Floor the frozen summand with `or vector(0)` so base + 0 = base, and assert the floor in the lag-brake unittest. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
573213ef8b |
test(postgres): assert the standby-only read-load selector in the query
Cover the load-bearing replica selector that both read-load queries depend on: the join must restrict to standby pods via label_cnpg_io_instance_role="replica", the exact key kube-state-metrics exposes for CNPG's cnpg.io/instance_role. A wrong key (e.g. a camelCase label_cnpg_io_instanceRole) would match no series, floor the read term to 0, and silently pin desired to the minimum - the suite previously never asserted the selector at all. Verified live on dev9 (CNPG 1.30.0) that the snake_case key is the one present in kube_pod_labels and the camelCase form does not exist. Adds the assertion for both the ReadConnections and ReadCPUUtilization paths. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
87b26d6595 |
fix(postgres): only seed spec.instances when synchronous quorum needs it
Narrow the previous unconditional seed to a conditional one, keeping design
proposal §3 (chart omits the field, HPA is the sole writer) intact for the
default path. Without synchronous quorum (maxSyncReplicas=0) CNPG's defaulted
instances=1 is admissible, so the field is omitted exactly as §3 specifies and
KEDA owns it via /scale with nothing contested. Only when maxSyncReplicas>=1 -
where an omitted/defaulted instances=1 fails the admission webhook
("maxSyncReplicas must be lower than the number of instances") - does the chart
seed the quorum floor (effectiveMin) so a fresh Cluster is admitted; the constant
re-render keeps three-way merge from reverting KEDA's higher live value.
This confines the deviation to the case §3 did not consider (sync quorum), so no
design-proposal amendment is needed for the documented path. Verified live on a
dev cluster: the quorum render (instances=2, maxSyncReplicas=1) and the
non-quorum render (instances omitted) are both admitted by the CNPG webhook. Adds
a render test for the non-quorum omit path alongside the quorum seed tests.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
610bcb5df9 |
fix(postgres): seed spec.instances at the quorum floor under autoscaling
Under active autoscaling the chart previously omitted .spec.instances entirely,
letting the KEDA HPA own it via /scale. But CNPG defaults an absent
.spec.instances to 1, and its admission webhook rejects a Cluster where
maxSyncReplicas >= instances ("maxSyncReplicas must be lower than the number of
instances"). So a fresh database created with autoscaling.enabled: true AND
synchronous quorum (maxSyncReplicas >= 1) was rejected at admission before KEDA
could ever scale it to effectiveMin (= maxSyncReplicas+1) — the whole tenant
HelmRelease failed. The default maxSyncReplicas: 0 path was unaffected, which is
why it went unnoticed. Verified live: a rendered Cluster with maxSyncReplicas: 1
and an omitted instances is rejected by the dev-cluster webhook; with the seeded
floor it is admitted.
Render instances: effectiveMin under active autoscaling instead of omitting it,
so a fresh Cluster starts at a valid, quorum-satisfying count. Because the chart
emits the same constant floor on every reconcile, Helm's three-way merge is a
no-op on the field and never reverts KEDA's higher live value, so the HPA still
owns the runtime count. Update the enablement docs (a new DB can enable directly
even with synchronous replication; existing DBs above the floor rebase to it
unless the two-phase transition flow is used) and add render tests for the fresh
create + maxSyncReplicas>=1 case.
Addresses the fresh-create-with-quorum admission gap found in branch review.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
284f5e5543 |
fix(postgres): fail closed when autoscaling is enabled without KEDA, guard target
autoscaling.enabled is a tenant-facing knob, but KEDA is an optional platform package that is not installed by default. A tenant enabling autoscaling on a cluster without KEDA would render a ScaledObject against an unknown kind and the whole database HelmRelease would fail with an opaque `no matches for kind "ScaledObject"`. Guard the ScaledObject template on `.Capabilities.APIVersions.Has "keda.sh/v1alpha1"` and stop with an actionable message instead. Skipping the render is deliberately NOT an option: db.yaml already drops the static .spec.instances under active autoscaling, so a missing ScaledObject would let CNPG collapse the cluster to 1 instance. Document the KEDA prerequisite on the `enabled` field (README/schema). Also reject a non-positive autoscaling.target at render time: it is a plain integer now, but the desired-replica formula divides the read load by it, so 0 is undefined and negatives invert scaling. cozyvalues-gen has no minimum directive, so the bound is a template guard. Add unittests for both guards (a dedicated suite renders without the KEDA CRD so the fail-closed path is exercised; the main suite now advertises keda.sh/v1alpha1 via capabilities). Addresses IvanHunters (KEDA-absent) and CodeRabbit (non-positive target) review feedback on #3954. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
f8fc2d99bf |
fix(postgres): make autoscaling.target an integer and drop the dead root metric
Two review findings on the generated API: - autoscaling.target was a resource.Quantity, which accepts suffixed values like `150m`. The chart splices target straight into the PromQL query and the KEDA threshold, where `150m` means 150 minutes to PromQL and is not a usable KEDA float threshold. Retype it as a dimensionless integer so the schema rejects suffixed input - helm unittest's schema validation now enforces it, and the existing autoscaling suite exercises the integer contract. - ConfigSpec carried a root `metric` field (and spec.metric in the postgres-rd schema) that nothing reads - the chart consumes only autoscaling.metric - so setting it silently did nothing. Remove the dead root param, keeping the ReadMetric enum that autoscaling.metric references. Regenerate types.go, zz_generated.deepcopy.go, values.schema.json, README.md and the postgres-rd schema; update the autoscaling tests to the integer target. Addresses CodeRabbit review feedback on #3954. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
ec06dafd25 |
feat(postgres): fold the replication-lag brake into the autoscaling query
Wire autoscaling.maxReplicationLagSeconds into the read-load query: while the max standby lag has exceeded the threshold within the cooldown window and the primary is actively writing WAL, the query returns currentInstances*target, pinning desired to the current count and freezing scaling in both directions (safer than pinning maxReplicas, which would still allow scale-down under lag). The lag term uses max_over_time as a hysteresis band; the write gate (WAL-record rate > 0) keeps an idle primary from tripping it. Metrics verified live on CNPG 1.30.0: cnpg_pg_replication_lag and cnpg_collector_wal_records exist and carry namespace/pod (the proposal's cnpg_pg_stat_replication_sent_diff_bytes does not). Also floor the read load at zero via `or vector(0)` so an idle cluster returns target rather than an empty result. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
d4c76fadb4 |
feat(postgres): two-phase autoscaling migration and read-CPU metric
Make enabling autoscaling on an existing database count-preserving, and add a second read-load signal: - autoscaling.transition (migration phase 1): keep .spec.instances rendered as a floor (from replicas, which the operator stages to the live count) AND pause the ScaledObject, so KEDA stands up without contending with Flux for the field. Flip transition off (phase 2) to drop the static field and hand the count to KEDA — suspend the HelmRelease across that apply, since helm-controller uses Helm's three-way merge (not SSA) and would otherwise race CNPG's default-to-1. The procedure is documented on the values field. - autoscaling.metric (ReadConnections | ReadCPUUtilization): the query helper emits either the active-connections sum or the postgres-container CPU sum, both + target. active = enabled && !dryRun && !transition; the ScaledObject is paused whenever dryRun or transition is set. Covered by helm unittest (phase-1 holds the staged count + pauses; CPU metric query) and regenerated schema/README/Go types/postgres-rd. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
0335276b61 |
feat(postgres): render a KEDA ScaledObject for read-replica autoscaling
Implement the KEDA-based read-replica autoscaler for postgres per the merged design proposal (cozystack/community#44): - cozy-lib.keda.scaledObject: engine-agnostic helper rendering a KEDA ScaledObject (scale target, effective bounds, prometheus trigger, scale-down behavior, optional pause), fail-closed on bad input. - postgres: an `autoscaling` values block (enabled / minReplicas / maxReplicas / target / maxReplicationLagSeconds / dryRun). Under active autoscaling the chart drops the static .spec.instances so Flux and the KEDA-managed HPA no longer contend, and renders a ScaledObject whose minReplicaCount folds in the synchronous-quorum floor (max(minReplicas, maxSyncReplicas+1, 2)); when the floor exceeds maxReplicas, quorum wins. dry-run keeps the static count and pauses the ScaledObject. The WorkloadMonitor tracks the quorum floor instead of a stale static value. Covered by helm unittest (cozy-lib helper: valid render, pause, three fail-closed guards; postgres: instances dropped, quorum-floored bounds, quorum-over-max, dry-run pause). Regenerated values.schema.json, README, Go types and postgres-rd. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
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>
|
||
|
Aleksei Sviridkin
|
6ecb36ffc7 |
feat(cozystack-controller): extract application CA into a key-free tenant Secret (#3407)
## What this PR does Adds an engine-agnostic controller that publishes a key-free `<release>.tenant-ca` Secret (only `ca.crt`) for a managed application, so a tenant can verify the application's TLS without ever seeing a private key. The source is declared, not guessed: a chart renders a namespaced `TenantProjection` sentinel (group `internal.cozystack.io`) naming the CA Secret to lift; the controller watches the sentinel, extracts `ca.crt`, and writes the projection owner-referenced to the sentinel, so garbage collection is native. The projection carries the `internal.cozystack.io/tenant-ca` label the lineage webhook turns into tenant visibility. Postgres is the first consumer. The trust boundary is RBAC: no tenant role grants any verb on `internal.cozystack.io`, so a tenant cannot forge a sentinel. A companion `ValidatingAdmissionPolicy` that pins the writer to helm-controller ships separately as defence in depth. This is the controller-and-CRD slice, split out from #3299 for focused review. It is the only piece required for the feature to work; the render-time helper, the writer policy, the capability-gate, and the release-prefix validation land as their own PRs. ### Downstream repositories Walked the trigger map against the diff. This adds an internal CRD and controller and a sentinel to the Postgres chart; no app is added or renamed and no `values.schema.json` changes, so the typed provider and Ansible are unaffected. The one to confirm is the Postgres README edit — if the website mirrors it, it needs a refresh. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: ### Release note ```release-note feat(cozystack-controller): publish a key-free per-application CA trust anchor (`<release>.tenant-ca`) that tenants read to verify managed-application TLS, declared through a new `TenantProjection` CRD. Postgres is the first consumer. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added tenant-facing CA certificate projections for PostgreSQL deployments. * Tenant secrets now provide only the CA certificate, excluding private keys. * Added support for configuring and monitoring tenant CA projections. * **Documentation** * Updated PostgreSQL TLS instructions to retrieve certificates through tenant secrets. * **Bug Fixes** * Improved protection against invalid certificates, secret collisions, and accidental exposure of private key material. * Added validation to ensure projected certificates remain synchronized with their sources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
80a03b1e09 |
test(backups): assert the barman-cloud sidecar checksum env on rendered ObjectStores
helm-unittest coverage for the S3 request-checksum fix (was mutation-negative before): assert spec.instanceSidecarConfiguration.env pins AWS_REQUEST_CHECKSUM_CALCULATION=when_required on all rendered barman-cloud ObjectStores — postgres backup + bootstrap-recovery (packages/apps/postgres/tests/backup_storage_test.yaml) and the Keycloak DB (new packages/system/keycloak/tests/db_backup_test.yaml). Verified by mutation: reverting the template change now fails these assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> |
||
|
Aleksei Sviridkin
|
858c1c04fe |
feat(postgres): declare the tenant CA trust anchor
CloudNativePG stores its self-signed CA in <release>-ca alongside the private key and offers no way to label it, so a tenant cannot be handed the trust anchor without also being handed the key. Render a TenantProjection that lifts ca.crt alone into the key-free <release>.tenant-ca Secret, and select that label from the postgres tenant-resource definition so the projection is served through the tenantsecrets API. Document the new retrieval path in the chart README. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrey KolkovandClaude Fable 5
|
67db8f7da7 |
fix(backups): make the barman-cloud serverName stable across every flow
Two defects of the same class — the WAL-archive S3 prefix (serverName) changing or defaulting differently between components: Recovery default regression (review blocker): native CNPG defaulted an absent externalClusters serverName to the entry name (= bootstrap.oldName), but the barman-cloud plugin defaults it to the NEW cluster's name, so a legacy-flow restore with bootstrap.serverName unset would read the wrong S3 prefix (hard failure, or silently restoring another cluster's data). The chart now always emits serverName, defaulting to bootstrap.oldName. Live-cluster serverName flip (found by the gated postgres backup e2e on a real cluster): the chart renders spec.plugins without serverName (effective prefix = cluster name) while a strategy template may name a different one; the driver's ForceOwnership patch then flipped the prefix mid-stream, splitting the WAL archive across two prefixes — the WAL holding the backup's start was lost between them and the restore failed with "WAL not found". The driver now preserves the live Cluster's effective serverName (explicit parameter, or the cluster name when the plugin is attached without one) and only applies the strategy's value to a Cluster with no plugin attached; both charts pin serverName explicitly, and the example strategy no longer sets one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
a1c135c2a1 |
fix(backups): omit empty retentionPolicy from the ObjectStore
Review follow-up (cozystack#3300, non-blocking): the barman-cloud ObjectStore CRD
validates spec.retentionPolicy against ^[1-9][0-9]*[dwm]$, so an operator who
explicitly sets backup.retentionPolicy="" would render retentionPolicy: "" and
the API server would reject the ObjectStore, breaking backup provisioning. Guard
the field in the apps/postgres and keycloak chart ObjectStores with
{{- if .Values.backup.retentionPolicy }} (the Go driver path is already safe via
omitempty). Add a helm-unittest asserting an empty retentionPolicy is omitted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey KolkovandClaude Opus 4.8
|
302a0193f1 |
docs+test(backups): update upgrade notes for the plugin flow + cover recovery rendering
Review follow-ups for the CNPG barman-cloud plugin migration (cozystack#3300): - docs/operations/backup-classes.md: the 'Upgrade notes from chart-managed backups' section still described the removed native spec.backup.barmanObjectStore behavior. Rewrite it for the plugin flow (chart renders spec.plugins + a barmancloud.cnpg.io/ObjectStore; the driver SSA-applies an ObjectStore and patches spec.plugins), and document the real upgrade hazard: the plugin-barman-cloud operator + cert-manager must be present in cozy-postgres-operator or the Cluster references an unregistered plugin and backups/WAL archiving stall. - apps/postgres backup_storage_test.yaml: add a positive bootstrap-recovery rendering test. Asserts externalClusters[].plugin references barman-cloud.cloudnative-pg.io with barmanObjectName=<release>-recovery and serverName on the plugin parameter, and that the -recovery ObjectStore carries spec.configuration but NOT serverName (the plugin forbids it there). Previously the recovery path had only fail-fast guard tests, no positive coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |