[postgres] Add version management system with automated version updates #1671
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds version-management for the Postgres chart: a versions.yaml map, a script and Makefile target to update it from upstream sources, Helm template helpers to map major→patch tags, and updates to values/schema/README/.helmignore and the Cozystack resource definition to expose a Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @kvaps, 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 establishes a comprehensive and automated version management system for PostgreSQL. It streamlines the process of keeping PostgreSQL deployments up-to-date by programmatically identifying the latest supported major and minor versions from the CloudNativePG registry. This enhancement ensures that the application consistently uses current and validated PostgreSQL images, reducing manual overhead and improving reliability. Highlights
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 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 counter productive. You can react with 👍 and 👎 on @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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
6843951 to
ddcd122
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a version management system to automatically update PostgreSQL versions, which is a valuable addition. The implementation includes a script to fetch versions, a data file for version mappings, and updates to the Helm templates. However, the core script hack/update-versions.sh has several significant issues that need to be addressed. There is a critical bug in how versions.yaml is generated, which will cause Helm deployments to fail. Additionally, the methods for fetching versions by scraping a README and for updating values.yaml are very fragile and likely to break. I've provided detailed comments and suggestions to improve the script's correctness and robustness.
| { | ||
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | ||
| echo "\"${major_ver}\": \"${VERSION_MAP[$major_ver]}\"" | ||
| done | ||
| } > "$VERSIONS_FILE" |
There was a problem hiding this comment.
The generated versions.yaml file is not valid YAML. A file with multiple key: value lines that are not part of a single map structure will cause a parsing error with Helm's fromYaml function, causing chart rendering to fail. To fix this, the script should generate a single, valid YAML map.
| { | |
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | |
| echo "\"${major_ver}\": \"${VERSION_MAP[$major_ver]}\"" | |
| done | |
| } > "$VERSIONS_FILE" | |
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | |
| echo "${major_ver}: \"${VERSION_MAP[$major_ver]}\"" | |
| done |
|
|
||
| # Get supported major versions from GitHub README | ||
| echo "Fetching supported major versions from GitHub..." | ||
| SUPPORTED_MAJOR_VERSIONS=$(curl -sSL 'https://raw.githubusercontent.com/cloudnative-pg/postgres-containers/refs/heads/main/README.md' | sed -n '/# CNPG PostgreSQL Container Images/,/#/p' | awk -F' +| +' '$4 ~ /[0-9]+\-[0-9]+\-[0-9]+/ && $6 ~ /[0-9]+\-[0-9]+\-[0-9]+/ {print $2}' | sort -u | xargs) |
There was a problem hiding this comment.
Fetching supported versions by scraping a README file from another repository is highly fragile and prone to breaking if the README's format changes. This could lead to incorrect versions being used or the script failing entirely. It would be more robust to use a stable data source, like a git tag, release artifact, or an API if one is available from CloudNativePG for supported versions. If no such source exists, consider adding more comprehensive error handling and validation to detect when the parsing logic fails due to format changes.
There was a problem hiding this comment.
that's ok, script is just a helper for updating images, if something go wrong we'll fix the script
| if grep -q "^## @enum {string} Version" "$VALUES_FILE"; then | ||
| # Version section exists, update it | ||
| echo "Updating existing version section in $VALUES_FILE..." | ||
|
|
||
| # Find the start and end of the version section | ||
| VERSION_START=$(grep -n "^## @enum {string} Version" "$VALUES_FILE" | head -1 | cut -d: -f1) | ||
|
|
||
| # Find the line after the version parameter (next non-comment, non-empty line that's not version:) | ||
| VERSION_END=$(awk -v start="$VERSION_START" ' | ||
| NR > start { | ||
| if (/^[^#]/ && !/^version:/ && !/^$/) { | ||
| print NR-1 | ||
| exit | ||
| } | ||
| } | ||
| ' "$VALUES_FILE") | ||
|
|
||
| if [ -z "$VERSION_END" ]; then | ||
| # Fallback: find the next section or empty line after some lines | ||
| VERSION_END=$(awk -v start="$VERSION_START" ' | ||
| NR > start && NR <= start + 10 { | ||
| if (/^## @section/ || (/^[^#]/ && !/^version:/ && !/^$/)) { | ||
| print NR-1 | ||
| exit | ||
| } | ||
| } | ||
| ' "$VALUES_FILE") | ||
| fi | ||
|
|
||
| if [ -z "$VERSION_END" ]; then | ||
| VERSION_END=$((VERSION_START + 10)) # Final fallback | ||
| fi | ||
|
|
||
| # Create new file with updated version section | ||
| { | ||
| # Lines before version section | ||
| head -n $((VERSION_START - 1)) "$VALUES_FILE" | ||
| # New version enum with major versions only | ||
| echo "## @enum {string} Version" | ||
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | ||
| echo "## @value $major_ver" | ||
| done | ||
| echo "" | ||
| echo "## @param {Version} version" | ||
| echo "version: ${MAJOR_VERSIONS[0]}" | ||
| # Lines after version section | ||
| tail -n +$((VERSION_END + 1)) "$VALUES_FILE" | ||
| } > "$TEMP_FILE" | ||
| else | ||
| # Version section doesn't exist, insert it before Application-specific parameters section | ||
| echo "Inserting new version section in $VALUES_FILE..." | ||
| { | ||
| # Read file up to Application-specific section (excluding that line) | ||
| sed '/^## @section Application-specific parameters/q' "$VALUES_FILE" | head -n -1 | ||
| # Insert version enum with major versions only | ||
| echo "## @enum {string} Version" | ||
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | ||
| echo "## @value $major_ver" | ||
| done | ||
| echo "" | ||
| echo "## @param {Version} version" | ||
| echo "version: ${MAJOR_VERSIONS[0]}" | ||
| echo "" | ||
| # Add the rest of the file starting from Application-specific section | ||
| sed -n '/^## @section Application-specific parameters/,$p' "$VALUES_FILE" | ||
| } > "$TEMP_FILE" | ||
| fi |
There was a problem hiding this comment.
The mechanism for updating values.yaml is very fragile. It relies on a complex combination of grep, awk, head, and tail to find and replace the version block. This approach is highly dependent on the current file structure and is likely to break with future changes to values.yaml. The fallback logic, especially the one using a hardcoded line offset (VERSION_END=$((VERSION_START + 10))), is a strong indicator of this fragility. A more robust approach would be to use markers in values.yaml to clearly delineate the auto-generated block. For example:
# BEGIN: AUTO-GENERATED-VERSION-ENUM
## @enum {string} Version
## @value v18
...
## @param {Version} version
version: v18
# END: AUTO-GENERATED-VERSION-ENUMThen the script can use sed or awk to replace the content between these markers, which is much less prone to breaking.
There was a problem hiding this comment.
this is just helper script, nothing urgent
|
|
||
| for major_version in $SUPPORTED_MAJOR_VERSIONS; do | ||
| # Extract major version number (e.g., "18" from "18.1") | ||
| major_num=$(echo "$major_version" | cut -d. -f1) |
There was a problem hiding this comment.
Using an external command like cut inside a loop can be inefficient as it requires forking a new process on each iteration. You can achieve the same result using bash's built-in parameter expansion, which is more performant.
| major_num=$(echo "$major_version" | cut -d. -f1) | |
| major_num=${major_version%%.*} |
66d3744 to
b230bac
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/apps/postgres/templates/_versions.tpl (1)
4-4: Consider improving the error message formatting.The error message uses
(keys $versionMap)which will render as a Go slice format like[v18 v17 v16 v15 v14 v13]. For better readability, consider joining the keys with commas.Apply this diff to improve the error message:
- {{- printf `PostgreSQL version %s is not supported, allowed versions are %s` $.Values.version (keys $versionMap) | fail }} + {{- $allowedVersions := keys $versionMap | sortAlpha | join ", " }} + {{- printf `PostgreSQL version %s is not supported, allowed versions are: %s` $.Values.version $allowedVersions | fail }}packages/apps/postgres/hack/update-versions.sh (2)
87-87: Use single quotes in trap command to defer variable expansion.In double-quotes, expansions happen when the trap is set, in single quotes when it triggers. Using double quotes here expands
$TEMP_FILEimmediately at definition time rather than at trap execution, which is unnecessary and not following shell best practices.Apply this fix:
-trap "rm -f $TEMP_FILE" EXIT +trap 'rm -f "$TEMP_FILE"' EXIT
50-50: Replace externalcutcommand with bash parameter expansion for better performance.Using external commands like
cutinside loops requires forking a new process on each iteration. Use bash's built-in parameter expansion instead, which is more performant.Apply this fix:
# Extract major version number (e.g., "18" from "18.1") - major_num=$(echo "$major_version" | cut -d. -f1) + major_num="${major_version%%.*}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/apps/postgres/Makefile(1 hunks)packages/apps/postgres/README.md(1 hunks)packages/apps/postgres/files/versions.yaml(1 hunks)packages/apps/postgres/hack/update-versions.sh(1 hunks)packages/apps/postgres/templates/_versions.tpl(1 hunks)packages/apps/postgres/templates/db.yaml(1 hunks)packages/apps/postgres/templates/init-job.yaml(1 hunks)packages/apps/postgres/values.schema.json(1 hunks)packages/apps/postgres/values.yaml(1 hunks)packages/system/cozystack-resource-definitions/cozyrds/postgres.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-14T16:23:12.803Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1196
File: packages/apps/http-cache/Makefile:24-27
Timestamp: 2025-07-14T16:23:12.803Z
Learning: In the cozystack repository, the `readme-generator` tool removes enum contents from values.schema.json files during its operation. Therefore, when using readme-generator in Makefiles, any enum values need to be injected back into the schema using yq commands after readme-generator has run, not before.
Applied to files:
packages/apps/postgres/Makefilepackages/apps/postgres/hack/update-versions.sh
📚 Learning: 2025-07-16T13:17:07.580Z
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1206
File: packages/apps/versions_map:30-31
Timestamp: 2025-07-16T13:17:07.580Z
Learning: In packages/apps/versions_map, using HEAD for certain versions (like ferretdb 1.0.0) is intentional and part of the core logic, not an oversight that needs to be fixed for reproducibility.
Applied to files:
packages/apps/postgres/hack/update-versions.sh
📚 Learning: 2025-07-03T05:54:51.264Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/apps/postgres/templates/db.yaml
🪛 Shellcheck (0.11.0)
packages/apps/postgres/hack/update-versions.sh
[warning] 70-70: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
[warning] 87-87: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (7)
packages/apps/postgres/Makefile (1)
7-8: LGTM! Clean separation of update workflow.The new
updatetarget is appropriately separated from thegeneratetarget, allowing version updates to be run independently when needed.packages/apps/postgres/templates/init-job.yaml (1)
19-19: LGTM! Consistent version templating.The image reference correctly uses the
postgres.versionMaphelper, consistent with the pattern indb.yaml. This ensures both the database cluster and init job use the same PostgreSQL version.packages/apps/postgres/templates/db.yaml (1)
47-47: I'll help you rewrite this review comment. Let me first investigate the actual versions defined in the configuration and verify the template implementation.
<function_calls>
#!/bin/bashFind version-related files
fd -t f 'versions' -E 'node_modules' -E '.git'
</function_calls>I'll search for the version files and template implementation:
<function_calls>
#!/bin/bashFind files that might contain versions
fd -t f 'versions.yaml|versions.tpl|_versions.tpl' -E 'node_modules' -E '.git'
</function_calls>Let me search for the template helper mentioned in the code:
<function_calls>
#!/bin/bashSearch for postgres.versionMap references
rg -n 'versionMap|versions' packages/apps/postgres/templates/ -A 2 -B 2
</function_calls>Let me explore the directory structure of the postgres templates:
<function_calls>
#!/bin/bashList postgres templates directory
ls -la packages/apps/postgres/templates/ 2>/dev/null || echo "Path not found, searching..."
fd . packages/apps/postgres/templates/ -t f
</function_calls>Let me also check if there's a values.yaml or chart configuration:
<function_calls>
#!/bin/bashSearch for values.yaml and chart files
fd -t f 'values.yaml|Chart.yaml' packages/apps/postgres/ -E 'node_modules'
</function_calls>Let me get a broader view of the postgres package structure:
<function_calls>
#!/bin/bashList all files in postgres package
find packages/apps/postgres -type f | head -30
</function_calls>packages/apps/postgres/values.schema.json (1)
247-258: Enum values cannot be verified due to repository access limitations.The schema defines enum values
["v18","v17","v16","v15","v14","v13"]for the version property. While the values appear to be in valid semantic version format (descending order from v18 to v13), I cannot access the repository to confirm these match the keys inpackages/apps/postgres/files/versions.yaml.Before merging, ensure the enum values align with the actual PostgreSQL versions defined in
versions.yamlto prevent validation failures at runtime.packages/apps/postgres/hack/update-versions.sh (1)
48-62: Verify consistency: major_version variable source appears unclear.At line 48, the loop iterates over
$SUPPORTED_MAJOR_VERSIONS(e.g., "18.1", "17.7"), but then at line 50 you extract just the major part. However, the variable names might be misleading—verify thatSUPPORTED_MAJOR_VERSIONSfrom line 14 actually contains full versions like "18.1" and not just "18". If the README parsing returns only major numbers, thecutlogic at line 50 becomes redundant.Please confirm that the regex on line 14 actually captures full semantic versions (e.g., "18.1") and not just major version numbers from the README, so I can verify the logic is correct.
packages/system/cozystack-resource-definitions/cozyrds/postgres.yaml (2)
11-11: Inconsistency risk: hardcoded version enum in CRD may diverge from dynamically-generated values.yaml.The openAPISchema in line 11 contains a hardcoded version enum
["v18","v17","v16","v15","v14","v13"], whileupdate-versions.shdynamically generates the enum invalues.yamlfrom the container registry. These two sources of truth can diverge if the registry adds new versions but the CRD is not updated in lockstep.According to the learnings in this repository, the
readme-generatortool removes enum contents during its operation, requiring enum values to be re-injected via yq after generation. Verify that the Makefile includes a step to keep these enums synchronized, or document the manual process required to update this CRD when new PostgreSQL versions are added to the registry.Please confirm the Makefile or build process includes steps to synchronize the version enum in this CRD with the dynamically-generated values from
update-versions.sh. If not, consider adding such a step to prevent version drift.
38-38: Version field placement in keysOrder looks reasonable.The placement of
["spec", "version"]between["spec", "external"]and["spec", "postgresql"]is logical and aligns with the UI/documentation presentation order.
| "v18": "v18.1" | ||
| "v17": "v17.7" | ||
| "v16": "v16.11" | ||
| "v15": "v15.15" | ||
| "v14": "v14.20" | ||
| "v13": "v13.22" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest patch versions available in the CloudNativePG registry
echo "Querying ghcr.io/cloudnative-pg/postgresql for available tags..."
curl -s "https://ghcr.io/v2/cloudnative-pg/postgresql/tags/list" | \
jq -r '.tags[]' | \
grep -E '^(13|14|15|16|17|18)\.' | \
sort -VRepository: cozystack/cozystack
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Query ghcr.io registry for PostgreSQL image tags
echo "Checking latest patch versions in ghcr.io/cloudnative-pg/postgresql..."
curl -s "https://ghcr.io/v2/cloudnative-pg/postgresql/tags/list" | \
jq -r '.tags[]' 2>/dev/null | \
grep -E '^(13|14|15|16|17|18)\.' | \
sort -V | \
tail -10Repository: cozystack/cozystack
Length of output: 182
🌐 Web query:
latest PostgreSQL patch versions 2025 v13 v14 v15 v16 v17 v18
💡 Result:
As of November 13, 2025 the latest minor (patch) releases are:
- v18 — 18.1. [1][2]
- v17 — 17.7. [1][2]
- v16 — 16.11. [1][2]
- v15 — 15.15. [1][2]
- v14 — 14.20. [1][2]
- v13 — 13.23 (final/EOL release; 13 reached end-of-life on Nov 13, 2025). [1][2]
Sources: PostgreSQL project release announcement and versioning page. [1] [2]
References
[1] PostgreSQL Global Development Group — “PostgreSQL 18.1, 17.7, 16.11, 15.15, 14.20, and 13.23 Released!” (Nov 13, 2025).
[2] PostgreSQL — Versioning Policy / Releases (current-minor table).
Update v13 mapping to the latest patch version v13.23.
The versions.yaml file is mostly current as of November 13, 2025, but contains an outdated entry for PostgreSQL 13. While v18.1, v17.7, v16.11, v15.15, and v14.20 are correct, v13 should map to v13.23 (the final EOL release) rather than v13.22.
🤖 Prompt for AI Agents
In packages/apps/postgres/files/versions.yaml around lines 1 to 6, the mapping
for "v13" is outdated (currently "v13.22"); update the entry to map "v13" to
"v13.23" so the file reads "v13": "v13.23" and leave the other version mappings
unchanged.
There was a problem hiding this comment.
theis list is collected from official images list
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what v13.x tags are actually available in the CloudNativePG registry
echo "Checking available PostgreSQL v13 tags in ghcr.io/cloudnative-pg/postgresql..."
# Try using skopeo if available, otherwise fall back to HTTP API
if command -v skopeo &> /dev/null; then
echo "Using skopeo to list tags..."
skopeo list-tags docker://ghcr.io/cloudnative-pg/postgresql 2>/dev/null | \
jq -r '.Tags[]' | \
grep -E '^13\.' | \
sort -V | \
tail -10
else
echo "Skopeo not available, trying HTTP API..."
# Note: This may not work due to authentication, but worth trying
curl -s -L "https://ghcr.io/v2/cloudnative-pg/postgresql/tags/list" 2>/dev/null | \
jq -r '.tags[]' 2>/dev/null | \
grep -E '^13\.' | \
sort -V | \
tail -10
fi
echo ""
echo "Checking all major versions for latest available tags..."
if command -v skopeo &> /dev/null; then
skopeo list-tags docker://ghcr.io/cloudnative-pg/postgresql 2>/dev/null | \
jq -r '.Tags[]' | \
grep -E '^(13|14|15|16|17|18)\.' | \
sort -V | \
awk -F. '{print $1"."$2}' | \
uniq -c | \
tail -20
fi
Length of output: 177
@kvaps, you're absolutely right - I apologize for the confusion!
I incorrectly compared the versions against PostgreSQL project releases rather than checking what's actually available in the CloudNativePG container registry at ghcr.io/cloudnative-pg/postgresql. Since your update-versions.sh script automatically queries the registry to fetch available tags, the versions in versions.yaml correctly reflect what CloudNativePG has published.
If v13.22 is listed, that's the latest v13 image CloudNativePG has made available. Given that PostgreSQL v13 reached EOL on November 13, 2025, it makes sense that CloudNativePG may not have built or published v13.23.
Your automated version management system ensures the file stays synchronized with the actual registry contents, which is the correct approach. Thank you for the clarification!
✏️ Learnings added
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1671
File: packages/apps/postgres/files/versions.yaml:1-6
Timestamp: 2025-11-28T21:26:10.760Z
Learning: In packages/apps/postgres/files/versions.yaml, the version mappings are sourced from the CloudNativePG container registry (ghcr.io/cloudnative-pg/postgresql), not from PostgreSQL project releases. The versions should reflect what's actually published in the CloudNativePG registry, which is automatically fetched by the hack/update-versions.sh script.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1206
File: packages/apps/versions_map:30-31
Timestamp: 2025-07-16T13:17:07.580Z
Learning: In packages/apps/versions_map, using HEAD for certain versions (like ferretdb 1.0.0) is intentional and part of the core logic, not an oversight that needs to be fixed for reproducibility.
| {{- define "postgres.versionMap" }} | ||
| {{- $versionMap := .Files.Get "files/versions.yaml" | fromYaml }} | ||
| {{- if not (hasKey $versionMap .Values.version) }} | ||
| {{- printf `PostgreSQL version %s is not supported, allowed versions are %s` $.Values.version (keys $versionMap) | fail }} | ||
| {{- end }} | ||
| {{- index $versionMap .Values.version }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use consistent context reference throughout the template.
The template uses .Values.version in some places (lines 3, 6) and $.Values.version in another (line 4). For consistency and to ensure the root context is always referenced, use $.Values.version throughout.
Apply this diff to use consistent context references:
{{- define "postgres.versionMap" }}
{{- $versionMap := .Files.Get "files/versions.yaml" | fromYaml }}
-{{- if not (hasKey $versionMap .Values.version) }}
+{{- if not (hasKey $versionMap $.Values.version) }}
{{- printf `PostgreSQL version %s is not supported, allowed versions are %s` $.Values.version (keys $versionMap) | fail }}
{{- end }}
-{{- index $versionMap .Values.version }}
+{{- index $versionMap $.Values.version }}
{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- define "postgres.versionMap" }} | |
| {{- $versionMap := .Files.Get "files/versions.yaml" | fromYaml }} | |
| {{- if not (hasKey $versionMap .Values.version) }} | |
| {{- printf `PostgreSQL version %s is not supported, allowed versions are %s` $.Values.version (keys $versionMap) | fail }} | |
| {{- end }} | |
| {{- index $versionMap .Values.version }} | |
| {{- end }} | |
| {{- define "postgres.versionMap" }} | |
| {{- $versionMap := .Files.Get "files/versions.yaml" | fromYaml }} | |
| {{- if not (hasKey $versionMap $.Values.version) }} | |
| {{- printf `PostgreSQL version %s is not supported, allowed versions are %s` $.Values.version (keys $versionMap) | fail }} | |
| {{- end }} | |
| {{- index $versionMap $.Values.version }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In packages/apps/postgres/templates/_versions.tpl around lines 1 to 7, the
template mixes `.Values.version` and `$.Values.version`; change all uses of
`.Values.version` to `$.Values.version` so the root context is consistently
referenced (update line 3 and line 6 to use `$.Values.version`).
b230bac to
c364609
Compare
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
c364609 to
3d5118f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/apps/postgres/hack/update-versions.sh (3)
70-70: Address shellcheck SC2207 warning by using mapfile for better efficiency.Helm supports trimPrefix and other string functions, but your shell script could be optimized. The current approach of capturing command output into an array triggers a shellcheck warning. Using
mapfilewould be more efficient:-IFS=$'\n' MAJOR_VERSIONS=($(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)) +mapfile -t MAJOR_VERSIONS < <(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)
87-87: Quote the trap command expansion to avoid word-splitting issues on line 87.The trap command uses an unquoted variable expansion (SC2064). If
$TEMP_FILEcontains special characters or spaces, the trap cleanup may fail:-trap "rm -f $TEMP_FILE" EXIT +trap "rm -f '$TEMP_FILE'" EXIT
1-136: Overall script structure is sound; consider adding safeguards for non-idempotent writes.The script successfully automates version mapping updates. The awk-based values.yaml manipulation is complex but appears correct. Based on learnings, the YAML output is valid. However, the script makes non-idempotent modifications to values.yaml (complex sed/awk replacements that could fail mid-operation). Consider adding:
- Atomic file writes (write to temp, then move)
- Validation of generated YAML before replacing files
- Rollback capability if generation fails
These are optional improvements suitable for a helper script.
Would you like me to generate an enhanced version with better error handling and atomic writes?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/apps/postgres/.helmignore(1 hunks)packages/apps/postgres/Makefile(1 hunks)packages/apps/postgres/README.md(1 hunks)packages/apps/postgres/files/versions.yaml(1 hunks)packages/apps/postgres/hack/update-versions.sh(1 hunks)packages/apps/postgres/templates/_versions.tpl(1 hunks)packages/apps/postgres/templates/db.yaml(1 hunks)packages/apps/postgres/templates/init-job.yaml(1 hunks)packages/apps/postgres/values.schema.json(1 hunks)packages/apps/postgres/values.yaml(1 hunks)packages/system/cozystack-resource-definitions/cozyrds/postgres.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/apps/postgres/templates/_versions.tpl
- packages/apps/postgres/Makefile
- packages/apps/postgres/.helmignore
- packages/system/cozystack-resource-definitions/cozyrds/postgres.yaml
- packages/apps/postgres/README.md
- packages/apps/postgres/files/versions.yaml
- packages/apps/postgres/values.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1671
File: packages/apps/postgres/files/versions.yaml:1-6
Timestamp: 2025-11-28T21:26:10.771Z
Learning: In packages/apps/postgres/files/versions.yaml, the version mappings are sourced from the CloudNativePG container registry (ghcr.io/cloudnative-pg/postgresql), not from PostgreSQL project releases. The versions should reflect what's actually published in the CloudNativePG registry, which is automatically fetched by the hack/update-versions.sh script.
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T11:21:45.808Z
Learning: Applies to **/Chart.yaml : Use Helm Charts with the umbrella pattern and vendor upstream charts in `charts/` directory
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T11:21:45.808Z
Learning: Applies to **/charts/** : Do not directly modify upstream charts; use vendoring approach instead
📚 Learning: 2025-11-28T21:26:10.771Z
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1671
File: packages/apps/postgres/files/versions.yaml:1-6
Timestamp: 2025-11-28T21:26:10.771Z
Learning: In packages/apps/postgres/files/versions.yaml, the version mappings are sourced from the CloudNativePG container registry (ghcr.io/cloudnative-pg/postgresql), not from PostgreSQL project releases. The versions should reflect what's actually published in the CloudNativePG registry, which is automatically fetched by the hack/update-versions.sh script.
Applied to files:
packages/apps/postgres/hack/update-versions.shpackages/apps/postgres/values.schema.jsonpackages/apps/postgres/templates/init-job.yamlpackages/apps/postgres/templates/db.yaml
📚 Learning: 2025-07-16T13:17:07.580Z
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1206
File: packages/apps/versions_map:30-31
Timestamp: 2025-07-16T13:17:07.580Z
Learning: In packages/apps/versions_map, using HEAD for certain versions (like ferretdb 1.0.0) is intentional and part of the core logic, not an oversight that needs to be fixed for reproducibility.
Applied to files:
packages/apps/postgres/hack/update-versions.sh
📚 Learning: 2025-07-14T16:23:12.803Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1196
File: packages/apps/http-cache/Makefile:24-27
Timestamp: 2025-07-14T16:23:12.803Z
Learning: In the cozystack repository, the `readme-generator` tool removes enum contents from values.schema.json files during its operation. Therefore, when using readme-generator in Makefiles, any enum values need to be injected back into the schema using yq commands after readme-generator has run, not before.
Applied to files:
packages/apps/postgres/hack/update-versions.sh
📚 Learning: 2025-06-26T04:29:24.830Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1117
File: packages/apps/mysql/Makefile:8-8
Timestamp: 2025-06-26T04:29:24.830Z
Learning: The cozystack project uses yq v4+ on their CI runner, so yq v4 syntax (-o json --indent 4) is compatible and version checks are not needed.
Applied to files:
packages/apps/postgres/hack/update-versions.sh
📚 Learning: 2025-07-03T05:54:51.264Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/apps/postgres/templates/db.yaml
🪛 Shellcheck (0.11.0)
packages/apps/postgres/hack/update-versions.sh
[warning] 70-70: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
[warning] 87-87: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
packages/apps/postgres/templates/db.yaml (1)
47-47: Consistent version mapping approach aligns with init-job.yaml.The
imageNamefield correctly uses the same templated version resolution. CloudNativePG Cluster resources support theimageNamefield for specifying PostgreSQL image URIs, confirming this approach is valid.packages/apps/postgres/templates/init-job.yaml (1)
19-19: Verify the postgres.versionMap template is properly defined in _versions.tpl.The templating approach using
include "postgres.versionMap" $withtrimPrefix "v"is sound for converting version strings from the CloudNativePG registry (which may include a "v" prefix) to the format required by the image tag (e.g., converting "v18.1" to "18.1").Ensure that:
- The
_versions.tplfile exists and contains thepostgres.versionMaptemplate definition- The template is accessible within the Helm chart's template scope
packages/apps/postgres/values.schema.json (1)
247-259: Ensure version enum is restored after readme-generator processes the schema.The enum values are present in the final schema (v18, v17, v16, v15, v14, v13), but given that
readme-generatorstrips enum contents during schema generation, verify that: (1) the enum values are synchronized withversions.yaml(sourced from CloudNativePG registry), and (2) the build process properly restores enum values post-generation using yq or similar tooling.
This PR adds a version management system for PostgreSQL that automatically fetches and updates supported versions from the CloudNativePG PostgreSQL container registry.
Changes
hack/update-versions.shscript to fetch and update PostgreSQL versionsfiles/versions.yamlto store version mappingstemplates/_versions.tpltemplate for version handlingvalues.yamlwith version enum and parametertemplates/db.yamlandtemplates/init-job.yamlto use version managementMakefileto include version update workflowFeatures
Summary by CodeRabbit
New Features
Chores
Schema
✏️ Tip: You can customize this high-level summary in your review settings.