Repository navigation
feat(postgres): add postgis image flavor - #2672
Matthieu ROBIN (matthieu-robin) wants to merge 2 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a chart/configuration flag ChangesPostgreSQL Image Flavor Selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.69.3)Trivy execution failed: 2026-06-01T18:58:26Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: kubernetes scan error: fs filter error: fs filter error: walk error range error: stat smartylint.json: no such file or directory: range error: stat smartylint.json: no such file or directory 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/apps/postgres/README.md`:
- Around line 181-185: The README mentions a non-existent `bootstrap.recovery`
key; update the sentence to reference the actual chart values used for recovery
bootstrapping such as `bootstrap.enabled`, and the recovery-specific keys like
`bootstrap.oldName` and `bootstrap.recoveryTime` (or whichever recovery-related
keys are defined in the chart values) so users know to enable bootstrapping and
supply the old release name and recovery timestamp when performing a fresh
release from backup; replace the `bootstrap.recovery` token with these real
value names and ensure the wording instructs setting `bootstrap.enabled: true`
plus the appropriate recovery keys.
🪄 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: 52e9ee4f-b5e4-4ab3-9fcb-1710fbf796fe
📒 Files selected for processing (10)
api/apps/v1alpha1/postgresql/types.gopackages/apps/postgres/README.mdpackages/apps/postgres/files/postgis-versions.yamlpackages/apps/postgres/templates/_versions.tplpackages/apps/postgres/templates/db.yamlpackages/apps/postgres/templates/init-job.yamlpackages/apps/postgres/tests/flavor_test.yamlpackages/apps/postgres/values.schema.jsonpackages/apps/postgres/values.yamlpackages/system/postgres-rd/cozyrds/postgres.yaml
There was a problem hiding this comment.
Code Review
This pull request introduces a flavor parameter for PostgreSQL, enabling support for PostGIS by switching to the cloudnative-pg/postgis image. The changes include updates to the API types, Helm templates, documentation, and validation schemas, along with a new version mapping for PostGIS tags. Feedback was provided to improve the readability of the error message when an unsupported PostgreSQL version is used with the PostGIS flavor by formatting the list of allowed versions as a comma-separated string.
| {{- define "postgres.postgisVersionMap" }} | ||
| {{- $versionMap := .Files.Get "files/postgis-versions.yaml" | fromYaml }} | ||
| {{- if not (hasKey $versionMap .Values.version) }} | ||
| {{- printf `PostgreSQL version %s is not supported by the postgis flavor, allowed versions are %s` $.Values.version (keys $versionMap | sortAlpha) | fail }} |
There was a problem hiding this comment.
The error message uses $.Values.version and (keys $versionMap | sortAlpha). While functional, the bracketed list format produced by printf %s on a slice (e.g., [v13 v14 ...]) is slightly less readable than a comma-separated string. Consider using join ", " for a cleaner presentation, although the current implementation matches the test expectations.
{{- printf `PostgreSQL version %s is not supported by the postgis flavor, allowed versions are %s` .Values.version (keys $versionMap | sortAlpha | join ", ") | fail }}
639717b to
f91b65c
Compare
Introduce a `flavor: postgresql | postgis` enum on the postgres app. `postgis` swaps the CNPG image to `ghcr.io/cloudnative-pg/postgis`, which ships PostGIS and companion extensions (postgis_topology, postgis_raster, pgrouting, address_standardizer) precompiled, so tenants can activate them per database via `databases.<name>.extensions`. The cluster and the post-install init Job share a single `postgres.imageName` helper to stay wire-compatible. Tag mapping for the postgis variant lives in `files/postgis-versions.yaml` (v13-v17 covered; v18 returns a clear error at install). Signed-off-by: Matthieu <[email protected]>
bootstrap.recovery does not exist in values.yaml. Point users at bootstrap.enabled / bootstrap.oldName / bootstrap.recoveryTime instead. Signed-off-by: Matthieu <[email protected]>
f91b65c to
2d98d79
Compare
|
Heads up, this conflicts with main and needs a rebase. Otherwise the postgis flavor still applies — there's no |
|
This PR has had no activity for 60 days and was marked |
|
Closed because no activity followed the |
<!-- 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 -->
What this PR does
Adds a
flavor: postgresql | postgisenum on theapps/postgreschart so tenants can opt in to the CloudNativePG PostGIS image (ghcr.io/cloudnative-pg/postgis) instead of the standardghcr.io/cloudnative-pg/postgresql. The variant ships PostGIS and its companion extensions (postgis_topology,postgis_raster,pgrouting,address_standardizer) precompiled — they activate per database via the existingdatabases.<name>.extensionsmechanism, no other plumbing required.Design
postgres.imageNametemplate helper. Bothtemplates/db.yaml(the cnpg.io Cluster) andtemplates/init-job.yaml(the post-install role/DB management Job) call it, so they stay wire-compatible — the init Job'spsqlalways matches the running server.files/postgis-versions.yamlmirrors the existingfiles/versions.yaml: maps theversionenum (v13-v17) to the corresponding PostGIS image tag (e.g.17.7-3.5). v18 is not yet covered by the upstream PostGIS image; selectingflavor: postgiswithversion: v18fails at install with a clear error.flavoris a bootstrap-time choice. Switching it on an existing release is documented as requiring a fresh release (orbootstrap.enabled: truewithbootstrap.oldNameandbootstrap.recoveryTimeto restore from a backup) — CNPG cannot hot-swap between images with different on-disk capabilities.CREATE EXTENSION IF NOT EXISTSininit-script.yaml); it just works once the binaries are present.api/apps/v1alpha1/postgresql/types.go,values.schema.json, the README parameters table andpackages/system/postgres-rd/cozyrds/postgres.yamlwere refreshed viamake generate.Release note
Test plan
helm unittest .— 18/18 (existing 13 + new 5 intests/flavor_test.yamlcovering default, explicit postgresql, postgis on Cluster, postgis on init Job, and the v18 failure path)helm template t . -n tenant-test --set version=v17 --set flavor=postgis→ bothCluster.spec.imageNameandJob.spec.template.spec.containers[0].imageresolve toghcr.io/cloudnative-pg/postgis:17.7-3.5helm template ... --set version=v18 --set flavor=postgis→ fails withPostgreSQL version v18 is not supported by the postgis flavor, allowed versions are [v13 v14 v15 v16 v17]go build ./...after codegenflavor: postgis+databases.gis.extensions: [postgis], thenSELECT postgis_full_version();Summary by CodeRabbit
New Features
flavoroption to choose standard PostgreSQL or a PostGIS-enabled image; chart now switches all relevant container images (cluster and init jobs) and validates supported PostGIS/Postgres versions.Documentation
Tests