fix(backups): support cnpg-operator with barman plugin - #3313

The keycloak-db backup added in #3174 uses the in-tree native
spec.backup.barmanObjectStore, whose barman-cloud-* binaries ship only in the
CNPG system image variant; keycloak-db is pinned to the standard variant, so
base backups fail with 'barman-cloud-backup: executable file not found'. The
proposed workaround (#3306) switches keycloak-db to the deprecated system image
variant when backups are enabled.

Migrate keycloak-db to the barman-cloud plugin instead, matching the rest of
this PR: render spec.plugins referencing a barmancloud.cnpg.io ObjectStore (the
S3/barman config moves there) and a method=plugin ScheduledBackup, keeping the
Cluster on the standard image (the plugin runs the barman tooling in a sidecar).
This removes the need for the system-variant workaround entirely, so #3306 can
be closed. #3300.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…recovery rendering

Review follow-ups for the CNPG barman-cloud plugin migration (#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) <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…eferences

Review follow-ups (#3300):

- Lifecycle/leak: applyClusterPluginBackup now sets an ownerReference from the
  driver-created barmancloud.cnpg.io/ObjectStore to its cnpg.io/Cluster, so
  Kubernetes garbage-collects it when the Cluster is deleted (tenant app
  teardown or the RestoreJob purge). In the platform useSystemBucket flow the
  chart does not render this ObjectStore, so previously it was orphaned in the
  tenant namespace. Drop the now-accurate-only 'delete' verb on objectstores
  from the driver RBAC (GC handles removal; the driver never deletes it) and
  correct the RBAC comment that falsely claimed the purge deletes it.
- Diagnostics/comments: the RestoreJob WAL-archive-deadline failure message and
  several doc comments still told operators to check spec.backup.barmanObjectStore
  / externalClusters[].barmanObjectStore, fields no longer emitted after the
  migration. Point them at the barman-cloud plugin (spec.plugins + ObjectStore,
  externalClusters[].plugin) instead so they don't mislead during an incident.
- Test: TestApplyClusterPluginBackup_PatchesExistingCluster now asserts the
  ObjectStore carries an ownerReference to the Cluster (kind/name/uid).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

Review follow-up (#3300): ensureCNPGBackup stamps spec.method=plugin +
spec.pluginConfiguration on the postgresql.cnpg.io/Backup it creates — the field
that routes a driver-initiated run through the barman-cloud plugin. If it
regressed to the default (barmanObjectStore) method, CNPG would run the legacy
method against a Cluster that no longer has one and every platform backup would
fail. Add TestEnsureCNPGBackup_UsesPluginMethod asserting method, plugin name,
and cluster ref. (The chart ScheduledBackup path was already covered; the driver
path was not.)

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…in flow

Review follow-up (#3300): the apps/postgres user-facing docs still
described the removed native spec.backup.barmanObjectStore / archive_command
model. Update the values.yaml @field/@typedef/@PARAM annotations (useSystemBucket,
schedule, S3CredentialsSecret, EndpointCA, endpointCA, the @PARAM backup note,
bootstrap.serverName) and the hand-written README Operations prose to the
barman-cloud plugin + ObjectStore model, then regenerate values.schema.json, the
README parameters table, the api types, and the postgres-rd ApplicationDefinition
schema (which feed the dashboard form help + cozyrd schema). Field names,
defaults, and the storageClass immutability marker are unchanged; only backup/
bootstrap description text changed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

Review follow-up (#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) <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

Port examples/backups/postgres into an automated e2e, following the etcd
backup precedent: the numbered example manifests become a runnable harness
(00-helpers.sh, run-all.sh, cleanup.sh) that a human reads and the test
drives, so the documented flow and the test can never drift.

run-all.sh provisions a Bucket, deploys the source Postgres (writing a
sentinel row), derives the two Secrets the barman-cloud ObjectStore
references from the Bucket's BucketInfo and seaweedfs' CA, submits an
ad-hoc BackupJob and waits for Succeeded, then restores to a copy and
asserts the sentinel round-tripped through S3 — the in-cluster witness
that a plugin backup is both taken and restorable end to end.

A second Test document (postgres-2-backup-roundtrip) in the existing
hack/e2e-chainsaw/postgres suite drives run-all.sh, gated out of CI behind
POSTGRES_E2E_S3_ROUNDTRIP=1 (kind has no reachable, CA-trusted S3 endpoint,
and a tenant Pod cannot cross the Cilium egress policy to tenant-root's
seaweedfs). Placing it in the postgres suite (rather than a new dir) keeps
it triggered by packages/apps/postgres changes under Test Impact Analysis.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…the e2e

Running the gated postgres backup e2e against a cluster whose apps still
render the pre-upgrade chart surfaced the CNPG webhook rejection
("Cannot enable a WAL archiver plugin when barmanObjectStore is
configured"): the driver cannot attach spec.plugins while the native
config is still present. Document the window and its resolution in the
upgrade notes.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

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 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…ple harness

The CNPG strategy template renders <app>-cnpg-backup-creds / -ca against
whichever application it drives, so a to-copy restore looks the pair up
under the TARGET's name. The harness only created the source's pair; on a
cluster with stale target Secrets from an earlier run the restore failed
TLS verification against the wrong CA. Materialise the pair for the
target too (kubectl apply, so stale content is corrected) and delete both
pairs in cleanup.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

Review follow-ups: the vendored cloudnative-pg chart ships the CNPG CRDs
as ordinary release manifests (crds.create), so helm upgrade updates them
with the operator — the out-of-band kubectl apply claim was wrong (the
historical skew came from the image.tag pin, which this PR removes). And
the missing barman-cloud-* binaries only affect the standard image
variant keycloak-db pins; apps/postgres pins bare system-flavor tags that
still ship them — its breakage was the operator/CRD skew.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…erName

The pre-commit run-make-generate hook regenerates the Go types from the
values annotations; the serverName default note landed in values.yaml,
README, schema and postgres-rd but missed types.go.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

Ungate postgres-2-backup-roundtrip: instead of an opt-in env flag it now
runs whenever TIA selects the postgres suite (postgres changes, and every
release cut), like the other chainsaw tests. The blocker was network
placement, not capability: the S3 client is the barman-cloud sidecar
inside the Postgres Pod, and the isolated e2e tenant's Cilium egress
allowlist blocks it from tenant-root's seaweedfs — so the Test runs in
tenant-root (same-tenant egress) against the in-cluster S3 endpoint,
trusting its self-signed cert via the copied seaweedfs-system-ca-cert.
This is exactly the recipe validated end to end on a real cluster.

Also map examples/backups/<app>/ edits to that app's suite in
hack/select-e2e.sh (the round-trip executes those scripts as its
harness; previously such edits were silently ignored), with bats
coverage for both the mapped and the no-suite case.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…cription

make -C packages/apps/postgres generate re-pads the Bootstrap parameters
table for the new bootstrap.serverName row; the committed README still
carried the old padding and kept the pre-commit generate check red.

Assisted-By: Claude <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>

…r webhook note

The db.yaml plugins-block comment still carried the over-generalized
claim that the pinned postgresql image variants no longer ship the
barman-cloud-* binaries; mirror the scoped wording from
docs/operations/backup-classes.md (only the standard variant keycloak-db
pins lacks them; this chart's bare system-flavor tags still ship them
but the flavor is deprecated upstream).

After the rebase onto main this PR no longer touches linstor-scheduler:
main re-vendored it to upstream chart 0.3.1 / extender v0.3.6, which
keeps the admission webhook and stops stripping unknown pod fields
(including initContainers[].restartPolicy the barman sidecar needs). The
upgrade note claiming the webhook is removed and locality scheduling
becomes advisory is rewritten as a hold-back warning.

Also: afterwards -> afterward (American English, review nit).

Assisted-By: Claude <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Andrey Kolkov <[email protected]>