Repository navigation
feat(postgres): add ClusterImageCatalog for system components - #2342
IvanHunters wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughUpdated CNPG PostgreSQL image selection across multiple Helm templates to use ChangesDatabase Template Updates
Postgres Operator Catalog Infrastructure
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer
participant Helm as "Helm templates"
participant K8s as "Kubernetes API"
participant CNPG as "cloudnative-pg Operator"
participant Registry as "Image Registry"
Developer->>Helm: Deploy charts (templates reference imageCatalogRef)
Helm->>K8s: Apply Cluster resources with imageCatalogRef
K8s->>CNPG: Deliver Cluster CRs
CNPG->>K8s: Read ClusterImageCatalog (postgresql-bookworm)
CNPG->>Registry: Resolve image digest for major 17 (from catalog)
Registry-->>CNPG: Return image@sha256
CNPG->>K8s: Create Pods using resolved image
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
dosubot
Bot
added
size/M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the PostgreSQL database configurations for Harbor, Keycloak, Monitoring (Alerta and Grafana), and SeaweedFS to use the ClusterImageCatalog resource instead of direct image names. New ClusterImageCatalog templates have been added for each service to manage PostgreSQL 17.7 images. I have no feedback to provide.
2a6249c to
b7486ae
Compare
Protect system PostgreSQL databases from automatic major version upgrades by introducing centralized ClusterImageCatalog resources based on official CNPG artifacts repository. Changes: - Add two ClusterImageCatalog resources (bookworm and trixie OS variants) - Based on CNPG official standard images with PostgreSQL 13-17 support - PostgreSQL 18 excluded to prevent auto-upgrades - Replace hardcoded imageName with imageCatalogRef in all system databases - Add Makefile target 'update-image-catalogs' for automated catalog updates System components using catalogs: - keycloak-db → postgresql-bookworm major 17 - harbor-db → postgresql-bookworm major 17 - grafana-db → postgresql-bookworm major 17 - alerta-db → postgresql-bookworm major 17 - seaweedfs-db → postgresql-bookworm major 17 Benefits: - Catalogs deploy with operator, always available before cluster changes - Standard images include additional extensions (pgaudit, etc.) - Support for both Debian bookworm and trixie - Automated updates via 'make update-image-catalogs' - Changes propagate through OCIRepo without manual migrations Signed-off-by: IvanHunters <[email protected]>
b7486ae to
a0ddacd
Compare
…atalogs Replace fragile awk/sed pipeline with yq for reliable YAML manipulation. Exclude PostgreSQL 13 (EOL since Nov 2025) in addition to PG18. Refresh catalog images to upstream 20260427 build. Signed-off-by: IvanHunters <[email protected]>
dosubot
Bot
added
size:S
and removed
size/M
github-actions
Bot
added
the
area/database
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/system/postgres-operator/Makefile`:
- Around line 15-20: The Makefile's update-image-catalogs step uses curl -sSL
piped to yq and redirects straight to templates/image-catalog-bookworm.yaml and
templates/image-catalog-trixie.yaml which can produce truncated/invalid files on
HTTP errors or partial writes; change curl to fail-fast (use -fsSL), write the
output to temporary files (e.g. templates/image-catalog-bookworm.yaml.tmp and
templates/image-catalog-trixie.yaml.tmp), only move the temp to the final path
atomically (mv) after verifying curl+yq exited successfully, and ensure the
target (update-image-catalogs) fails on any non-zero exit so broken catalogs are
never left in place. Include references to the commands that currently invoke
curl | yq and the target name update-image-catalogs so you update the correct
rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bdd83dc-2708-4600-8c46-825a70bb665d
📒 Files selected for processing (3)
packages/system/postgres-operator/Makefilepackages/system/postgres-operator/templates/image-catalog-bookworm.yamlpackages/system/postgres-operator/templates/image-catalog-trixie.yaml
| @curl -sSL https://raw.githubusercontent.com/cloudnative-pg/artifacts/main/image-catalogs/catalog-standard-bookworm.yaml \ | ||
| | yq '.metadata.name = "postgresql-bookworm" | del(.spec.images[] | select(.major == 13 or .major == 18))' \ | ||
| > templates/image-catalog-bookworm.yaml | ||
| @curl -sSL https://raw.githubusercontent.com/cloudnative-pg/artifacts/main/image-catalogs/catalog-standard-trixie.yaml \ | ||
| | yq '.metadata.name = "postgresql-trixie" | del(.spec.images[] | select(.major == 13 or .major == 18))' \ | ||
| > templates/image-catalog-trixie.yaml |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current implementation uses non-fail-fast curl + direct pipeline redirection.
rg -n -A3 -B1 'update-image-catalogs:|curl -sSL|\\| yq|> templates/image-catalog' packages/system/postgres-operator/Makefile
# Verify whether fail-fast/atomic patterns are already present.
rg -n 'curl -f|curl -fsSL|mktemp|mv .*image-catalog' packages/system/postgres-operator/Makefile || trueRepository: cozystack/cozystack
Length of output: 812
Harden catalog generation against silent fetch failures and partial writes.
The update-image-catalogs target streams curl into yq with direct redirection to final files. The -sSL flags do not include -f, so HTTP errors are not fatal; if the fetch or transformation fails partway through, a truncated/invalid catalog file gets written to the destination. Use fail-fast curl (-fsSL), temporary files, and atomic moves to prevent corrupted output.
Proposed fix
update-image-catalogs:
`@echo` "Downloading PostgreSQL image catalogs from CNPG artifacts..."
- `@curl` -sSL https://raw.githubusercontent.com/cloudnative-pg/artifacts/main/image-catalogs/catalog-standard-bookworm.yaml \
- | yq '.metadata.name = "postgresql-bookworm" | del(.spec.images[] | select(.major == 13 or .major == 18))' \
- > templates/image-catalog-bookworm.yaml
- `@curl` -sSL https://raw.githubusercontent.com/cloudnative-pg/artifacts/main/image-catalogs/catalog-standard-trixie.yaml \
- | yq '.metadata.name = "postgresql-trixie" | del(.spec.images[] | select(.major == 13 or .major == 18))' \
- > templates/image-catalog-trixie.yaml
+ `@tmp_in`=$$(mktemp); tmp_out=$$(mktemp); \
+ curl -fsSL https://raw.githubusercontent.com/cloudnative-pg/artifacts/main/image-catalogs/catalog-standard-bookworm.yaml -o "$$tmp_in"; \
+ yq '.metadata.name = "postgresql-bookworm" | del(.spec.images[] | select(.major == 13 or .major == 18))' "$$tmp_in" > "$$tmp_out"; \
+ mv "$$tmp_out" templates/image-catalog-bookworm.yaml; rm -f "$$tmp_in"
+ `@tmp_in`=$$(mktemp); tmp_out=$$(mktemp); \
+ curl -fsSL https://raw.githubusercontent.com/cloudnative-pg/artifacts/main/image-catalogs/catalog-standard-trixie.yaml -o "$$tmp_in"; \
+ yq '.metadata.name = "postgresql-trixie" | del(.spec.images[] | select(.major == 13 or .major == 18))' "$$tmp_in" > "$$tmp_out"; \
+ mv "$$tmp_out" templates/image-catalog-trixie.yaml; rm -f "$$tmp_in"
`@echo` "Updated image catalogs (PostgreSQL 14-17, excluded v13 EOL and v18)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/system/postgres-operator/Makefile` around lines 15 - 20, The
Makefile's update-image-catalogs step uses curl -sSL piped to yq and redirects
straight to templates/image-catalog-bookworm.yaml and
templates/image-catalog-trixie.yaml which can produce truncated/invalid files on
HTTP errors or partial writes; change curl to fail-fast (use -fsSL), write the
output to temporary files (e.g. templates/image-catalog-bookworm.yaml.tmp and
templates/image-catalog-trixie.yaml.tmp), only move the temp to the final path
atomically (mv) after verifying curl+yq exited successfully, and ensure the
target (update-image-catalogs) fails on any non-zero exit so broken catalogs are
never left in place. Include references to the commands that currently invoke
curl | yq and the target name update-image-catalogs so you update the correct
rule.
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the change is ~1138 commits behind main and its premise has already been solved differently on main; it also carries a CRD-ordering risk and an undeclared OS/minor migration. The ClusterImageCatalog approach has merit, but it needs a rebase and re-scoping against current main.
Business context: prevent CNPG auto-major-upgrades and move system Postgres DBs off the deprecated "system" images by pinning them to a centralized ClusterImageCatalog.
Blockers
B1: stale and superseded by what already landed on main
The branch's merge-base is 2026-04-01 (~1138 commits behind main). The diff replaces imageName: ghcr.io/cloudnative-pg/postgresql:17.7, but main no longer has that line: commits 620c8fb3c + a3f50ba2b already moved all five system DBs to ghcr.io/cloudnative-pg/postgresql:17.7-standard-trixie via an imageName dig/regex pin. So main already runs standard (not system) images and already prevents the auto-upgrade this PR targets — the stated premise ("17.7 points to deprecated system images") no longer holds, and a rebase will conflict on all five DB files. The PR should be re-framed as "replace main's imageName dig-pin with the idiomatic ClusterImageCatalog", not "fix deprecated system images".
B2: undeclared OS + minor migration
main standardized on trixie (17.7-standard-trixie); this PR points the same DBs at postgresql-bookworm major 17, which resolves to 17.9-standard-bookworm. Merging (after rebase) switches every system DB from trixie 17.7 to bookworm 17.9 — an OS change plus a minor bump that triggers a rolling pod replacement on each cluster. That should be a deliberate, called-out decision (or aligned to trixie to match main), not a side effect of adopting catalogs.
B3: CRD ordering on a fresh install
The ClusterImageCatalog resources are added to the same Helm release as the CNPG operator, whose CRDs render from charts/cloudnative-pg/templates/crds/crds.yaml — a regular template, not Helm's special crds/ directory. On a fresh cluster the catalog CRs can fail to apply with no matches for kind "ClusterImageCatalog" if the CRD isn't established first. Gate the catalogs behind the CRD's existence (a separate release / dependsOn / post-CRD apply).
Non-blocking
make update-image-catalogspipescurl … | yq > template.yamlwith nopipefail/temp-file guard; an upstream fetch failure or format change would silently overwrite the template with truncated/empty content. Write to a temp file and move it into place only on success.
Note: imageCatalogRef requires CNPG ≥ 1.24 and main is on appVersion 1.27.1, so the mechanism is available — the approach is viable, it just needs reconciliation with what already landed on main.
The Keycloak DB backup (#3174) uses the in-tree native `barmanObjectStore`, whose `barman-cloud-*` binaries ship ONLY in the CNPG `system` image variant. Since #2342 the DB is pinned to the `standard` variant, which omits them — so with backups enabled a base backup fails `barman-cloud-backup: executable file not found` and WAL archiving cannot run (also risks WAL accumulation). Pin the barman-capable `system` variant only when backups are enabled (backup.enabled + destinationPath); otherwise keep `standard`. Verified on a freedom-portal cluster: with `system-trixie` the CNPG Backup completes and WAL archiving works to S3; with `standard-trixie` it fails as above. TEMPORARY bridge: `system` is deprecated in CNPG 1.27 (removed in 1.29). Remove this once Postgres backups migrate to the Barman Cloud Plugin — see #3300. Refs #3300, #2342, #3174. Signed-off-by: Andrey Kolkov <[email protected]>
2 participants
Summary
Protects system PostgreSQL databases from automatic major version upgrades by introducing centralized ClusterImageCatalog resources based on official CNPG artifacts.
Background
CloudNativePG operator v1.26+ automatically triggers major version upgrades when
imageNamechanges in the Cluster spec. Legacy tag17.7is deprecated and pointed tosystemimages which are also deprecated.Solution
Deploy two shared
ClusterImageCatalogresources (bookworm and trixie) with postgres-operator using standard image type from CNPG official artifacts. Standard images provide more extensions than minimal (includes pgaudit, etc.) while system images are deprecated.Image Types Comparison
Based on CNPG postgres-containers:
Changes
ClusterImageCatalog resources:
postgresql-bookworm- Standard images on Debian bookworm (PostgreSQL 13-17)postgresql-trixie- Standard images on Debian trixie (PostgreSQL 13-17)System databases updated:
postgresql-bookwormmajor 17postgresql-bookwormmajor 17postgresql-bookwormmajor 17postgresql-bookwormmajor 17postgresql-bookwormmajor 17Makefile automation:
make update-image-catalogstargetBenefits
make update-image-catalogssyncs with upstreamUsage
Update catalogs when CNPG releases new PostgreSQL versions:
cd packages/system/postgres-operator make update-image-catalogsTest Plan
make update-image-catalogscorrectly downloads and filters catalogsSummary by CodeRabbit