[mariadb] Add version management system with automated version updates #1680
Conversation
|
Warning Rate limit exceeded@kvaps has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
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. WalkthroughThis PR introduces versioning management for MariaDB deployments by adding configurable version parameters, version mappings, an automated version-fetching script, and Helm template helpers to dynamically select container image versions instead of using hard-coded tags. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 significantly enhances the MariaDB application's maintainability by implementing an automated version management system. It allows for dynamic selection of MariaDB versions, ensuring that deployments can easily stay up-to-date with the latest stable LTS releases without manual intervention in the image tags. This change streamlines the update process and improves the overall robustness of version handling. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces an excellent version management system for the MariaDB chart, including a script to automate updates to the latest LTS versions. This is a significant improvement for maintainability. The changes are well-thought-out, utilizing a helper script, a version map file, and a Helm template helper. My review focuses on enhancing the automation script's robustness, correctness, and completeness. I've identified a couple of high-severity issues where the automation is incomplete or could produce invalid files, and I've also provided several suggestions to improve the script's clarity and implementation.
| #!/usr/bin/env bash | ||
|
|
||
| set -o errexit | ||
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| MYSQL_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" | ||
| VALUES_FILE="${MYSQL_DIR}/values.yaml" | ||
| VERSIONS_FILE="${MYSQL_DIR}/files/versions.yaml" | ||
| MARIADB_API_URL="https://downloads.mariadb.org/rest-api/mariadb/" | ||
|
|
||
| # Check if jq is installed | ||
| if ! command -v jq &> /dev/null; then | ||
| echo "Error: jq is not installed. Please install jq and try again." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Get LTS versions from MariaDB REST API | ||
| echo "Fetching LTS versions from MariaDB REST API..." | ||
| LTS_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}") | ||
|
|
||
| if [ -z "$LTS_VERSIONS_JSON" ]; then | ||
| echo "Error: Could not fetch versions from MariaDB REST API" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Extract LTS stable major versions | ||
| LTS_MAJOR_VERSIONS=$(echo "$LTS_VERSIONS_JSON" | jq -r '.major_releases[] | select(.release_support_type == "Long Term Support") | select(.release_status == "Stable") | .release_id' | sort -V -r) | ||
|
|
||
| if [ -z "$LTS_MAJOR_VERSIONS" ]; then | ||
| echo "Error: Could not find any LTS stable versions" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Found LTS major versions: $(echo "$LTS_MAJOR_VERSIONS" | tr '\n' ' ')" | ||
|
|
||
| # Build versions map: major version -> latest patch version | ||
| declare -A VERSION_MAP | ||
| MAJOR_VERSIONS=() | ||
|
|
||
| for major_version in $LTS_MAJOR_VERSIONS; do | ||
| echo "Fetching patch versions for ${major_version}..." | ||
|
|
||
| # Get patch versions for this major version | ||
| PATCH_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}${major_version}") | ||
|
|
||
| if [ -z "$PATCH_VERSIONS_JSON" ]; then | ||
| echo "Warning: Could not fetch patch versions for ${major_version}, skipping..." >&2 | ||
| continue | ||
| fi | ||
|
|
||
| # Extract all stable patch version IDs (format: MAJOR.MINOR.PATCH) | ||
| # Filter only Stable releases | ||
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r --arg major "$major_version" '.releases | to_entries[] | select(.key | startswith($major + ".")) | select(.value.release_status == "Stable") | .key' | sort -V) | ||
|
|
||
| # If no stable releases found, try to get any releases (for backwards compatibility) | ||
| if [ -z "$PATCH_VERSIONS" ]; then | ||
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r '.releases | keys[]' | grep -E "^${major_version}\." | sort -V) | ||
| fi | ||
|
|
||
| if [ -z "$PATCH_VERSIONS" ]; then | ||
| echo "Warning: Could not find any patch versions for ${major_version}, skipping..." >&2 | ||
| continue | ||
| fi | ||
|
|
||
| # Get the latest patch version | ||
| LATEST_PATCH=$(echo "$PATCH_VERSIONS" | tail -n1) | ||
|
|
||
| # major_version already has format MAJOR.MINOR (e.g., "11.8") | ||
| VERSION_MAP["v${major_version}"]="${LATEST_PATCH}" | ||
| MAJOR_VERSIONS+=("v${major_version}") | ||
| echo "Found version: v${major_version} -> ${LATEST_PATCH}" | ||
| done | ||
|
|
||
| if [ ${#MAJOR_VERSIONS[@]} -eq 0 ]; then | ||
| echo "Error: No matching versions found" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Sort major versions in descending order (newest first) | ||
| IFS=$'\n' MAJOR_VERSIONS=($(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)) | ||
| unset IFS | ||
|
|
||
| echo "Major versions to add: ${MAJOR_VERSIONS[*]}" | ||
|
|
||
| # Create/update versions.yaml file | ||
| echo "Updating $VERSIONS_FILE..." | ||
| { | ||
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | ||
| echo "\"${major_ver}\": \"${VERSION_MAP[$major_ver]}\"" | ||
| done | ||
| } > "$VERSIONS_FILE" | ||
|
|
||
| echo "Successfully updated $VERSIONS_FILE" | ||
|
|
||
| # Update values.yaml - enum with major.minor versions only | ||
| TEMP_FILE=$(mktemp) | ||
| trap "rm -f $TEMP_FILE" EXIT | ||
|
|
||
| # Build new version section | ||
| NEW_VERSION_SECTION="## @enum {string} Version" | ||
| for major_ver in "${MAJOR_VERSIONS[@]}"; do | ||
| NEW_VERSION_SECTION="${NEW_VERSION_SECTION} | ||
| ## @value $major_ver" | ||
| done | ||
| NEW_VERSION_SECTION="${NEW_VERSION_SECTION} | ||
|
|
||
| ## @param {Version} version - MariaDB major.minor version to deploy | ||
| version: ${MAJOR_VERSIONS[0]}" | ||
|
|
||
| # Check if version section already exists | ||
| if grep -q "^## @enum {string} Version" "$VALUES_FILE"; then | ||
| # Version section exists, update it using awk | ||
| echo "Updating existing version section in $VALUES_FILE..." | ||
|
|
||
| # Use awk to replace the section from "## @enum {string} Version" to "version: " (inclusive) | ||
| # Delete the old section and insert the new one | ||
| awk -v new_section="$NEW_VERSION_SECTION" ' | ||
| /^## @enum {string} Version/ { | ||
| in_section = 1 | ||
| print new_section | ||
| next | ||
| } | ||
| in_section && /^version: / { | ||
| in_section = 0 | ||
| next | ||
| } | ||
| in_section { | ||
| next | ||
| } | ||
| { print } | ||
| ' "$VALUES_FILE" > "$TEMP_FILE.tmp" | ||
| mv "$TEMP_FILE.tmp" "$VALUES_FILE" | ||
| else | ||
| # Version section doesn't exist, insert it before Application-specific parameters section | ||
| echo "Inserting new version section in $VALUES_FILE..." | ||
|
|
||
| # Use awk to insert before "## @section Application-specific parameters" | ||
| awk -v new_section="$NEW_VERSION_SECTION" ' | ||
| /^## @section Application-specific parameters/ { | ||
| print new_section | ||
| print "" | ||
| } | ||
| { print } | ||
| ' "$VALUES_FILE" > "$TEMP_FILE.tmp" | ||
| mv "$TEMP_FILE.tmp" "$VALUES_FILE" | ||
| fi | ||
|
|
||
| echo "Successfully updated $VALUES_FILE with major.minor versions: ${MAJOR_VERSIONS[*]}" | ||
|
|
There was a problem hiding this comment.
This script automates updating MariaDB versions, which is great. However, it misses updating values.schema.json. The enum for the version property in values.schema.json is hardcoded in this PR. When this script runs and adds new MariaDB versions, values.schema.json will become outdated. This will cause validation to fail for any new versions selected by users, breaking the chart's deployment.
The script should also update the enum and default values in values.schema.json. You can achieve this using jq.
Here's a suggestion for how you could implement this. You would add this block to the end of the script:
# Update values.schema.json
VALUES_SCHEMA_FILE="${MYSQL_DIR}/values.schema.json"
echo "Updating $VALUES_SCHEMA_FILE..."
# Create a JSON array of versions for jq
VERSIONS_JSON=$(printf '%s\n' "${MAJOR_VERSIONS[@]}" | jq -R . | jq -s .)
# Get the latest version for the default value
DEFAULT_VERSION="${MAJOR_VERSIONS[0]}"
# Use jq to update the schema file
jq \
--argjson versions "$VERSIONS_JSON" \
--arg default_version "$DEFAULT_VERSION" \
'.properties.version.enum = $versions | .properties.version.default = $default_version' \
"$VALUES_SCHEMA_FILE" > "$TEMP_FILE"
mv "$TEMP_FILE" "$VALUES_SCHEMA_FILE"
echo "Successfully updated $VALUES_SCHEMA_FILE"Note: You'll need to add VALUES_SCHEMA_FILE to the variables at the top of the script.
| { | ||
| 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 versions.yaml file generated by this part of the script is not a valid YAML document for a map. It's a stream of what looks like single-entry JSON objects, but without commas or enclosing braces. While some YAML parsers might be lenient enough to merge these into a single map, this is not guaranteed behavior and relies on implementation details of Helm's fromYaml function. This could break in future Helm versions or with other YAML tools.
To ensure correctness, the script should generate a single, valid YAML document representing a 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 | |
| } > "$VERSIONS_FILE" |
|
|
||
| # If no stable releases found, try to get any releases (for backwards compatibility) | ||
| if [ -z "$PATCH_VERSIONS" ]; then | ||
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r '.releases | keys[]' | grep -E "^${major_version}\." | sort -V) |
There was a problem hiding this comment.
For better performance and to keep the logic within a single tool, you can replace the grep command with a jq filter. This avoids spawning an extra process and makes the pipeline cleaner.
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r '.releases | keys[]' | grep -E "^${major_version}\." | sort -V) | |
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r --arg major "$major_version" '.releases | keys[] | select(startswith($major + "."))' | sort -V) |
| IFS=$'\n' MAJOR_VERSIONS=($(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)) | ||
| unset IFS |
There was a problem hiding this comment.
The MAJOR_VERSIONS array is being sorted here, but it should already be in the correct descending version order. It's populated from LTS_MAJOR_VERSIONS, which is sorted with sort -V -r on line 29. This re-sorting is redundant and can be removed.
| ' "$VALUES_FILE" > "$TEMP_FILE.tmp" | ||
| mv "$TEMP_FILE.tmp" "$VALUES_FILE" |
There was a problem hiding this comment.
There's a minor issue with temporary file handling. The script creates a temp file at $TEMP_FILE and sets a trap to clean it up. However, you are writing to "$TEMP_FILE.tmp" and then moving it. If the script is interrupted, "$TEMP_FILE.tmp" might be left behind as it's not covered by the trap. It's safer to write directly to the file created by mktemp.
| ' "$VALUES_FILE" > "$TEMP_FILE.tmp" | |
| mv "$TEMP_FILE.tmp" "$VALUES_FILE" | |
| ' "$VALUES_FILE" > "$TEMP_FILE" | |
| mv "$TEMP_FILE" "$VALUES_FILE" |
| ' "$VALUES_FILE" > "$TEMP_FILE.tmp" | ||
| mv "$TEMP_FILE.tmp" "$VALUES_FILE" |
There was a problem hiding this comment.
Similar to the block above, it's safer to write directly to the temporary file created by mktemp to ensure it's cleaned up by the trap in case of interruption.
| ' "$VALUES_FILE" > "$TEMP_FILE.tmp" | |
| mv "$TEMP_FILE.tmp" "$VALUES_FILE" | |
| ' "$VALUES_FILE" > "$TEMP_FILE" | |
| mv "$TEMP_FILE" "$VALUES_FILE" |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/apps/mysql/templates/_versions.tpl (1)
1-7: Consider consistent context usage and improved error formatting.The template works correctly, but there are minor style improvements:
- Line 3 uses
.Values.versionwhile line 4 uses$.Values.version. While both work when called with root context, consistency improves readability.- The error message could format the allowed versions list more clearly.
Consider this improvement:
{{- define "mysql.versionMap" }} {{- $versionMap := .Files.Get "files/versions.yaml" | fromYaml }} {{- if not (hasKey $versionMap .Values.version) }} - {{- printf `MariaDB version %s is not supported, allowed versions are %s` $.Values.version (keys $versionMap) | fail }} + {{- printf "MariaDB version %s is not supported. Allowed versions: %s" .Values.version (keys $versionMap | sortAlpha | join ", ") | fail }} {{- end }} {{- index $versionMap .Values.version }} {{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/apps/mysql/.helmignore(1 hunks)packages/apps/mysql/Makefile(1 hunks)packages/apps/mysql/README.md(1 hunks)packages/apps/mysql/files/versions.yaml(1 hunks)packages/apps/mysql/hack/update-versions.sh(1 hunks)packages/apps/mysql/templates/_versions.tpl(1 hunks)packages/apps/mysql/templates/mariadb.yaml(1 hunks)packages/apps/mysql/values.schema.json(1 hunks)packages/apps/mysql/values.yaml(1 hunks)packages/system/cozystack-resource-definitions/cozyrds/mysql.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/mysql/hack/update-versions.shpackages/apps/mysql/files/versions.yaml
🧬 Code graph analysis (1)
packages/apps/mysql/hack/update-versions.sh (1)
scripts/installer.sh (1)
run_migrations(8-20)
🪛 GitHub Actions: Pre-Commit Checks
packages/apps/mysql/.helmignore
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/templates/_versions.tpl
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/Makefile
[error] 1-1: make generate failed in packages/apps/mysql: controller-gen: cannot locate metav1 import (undefined types: Version). Exit code 1 during generation step.
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/values.yaml
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/templates/mariadb.yaml
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/values.schema.json
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/README.md
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/hack/update-versions.sh
[error] 1-1: Process completed with exit code 123.
packages/apps/mysql/files/versions.yaml
[error] 1-1: Process completed with exit code 123.
🪛 Shellcheck (0.11.0)
packages/apps/mysql/hack/update-versions.sh
[warning] 82-82: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
[warning] 99-99: 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 (11)
packages/apps/mysql/.helmignore (1)
4-4: LGTM!Excluding the
/hackdirectory from Helm packaging is appropriate to prevent build tooling from being included in the chart artifact.packages/apps/mysql/README.md (1)
82-82: LGTM!The documentation for the new
versionparameter is clear, properly positioned, and consistent with the implementation defaults and schema.packages/system/cozystack-resource-definitions/cozyrds/mysql.yaml (1)
11-11: LGTM!The CRD schema properly integrates the new
versionproperty with consistent defaults and enum values matching the values schema. The keysOrder placement afterexternalis logical and maintains proper field ordering.Also applies to: 30-30
packages/apps/mysql/values.schema.json (1)
189-200: LGTM!The schema addition for the
versionproperty is well-formatted, includes proper description and enum constraints, and is consistent with the values file and CRD definitions. The default matches the implementation.packages/apps/mysql/templates/mariadb.yaml (1)
11-11: No action needed — the referenced template helper is properly implemented.The
mysql.versionMaphelper inpackages/apps/mysql/templates/_versions.tplis correctly defined and fully implements the expected behavior: it loads version mappings fromfiles/versions.yaml, validates that.Values.versionexists in the mapping with a clear error message if not found, and returns the mapped version. The template reference on line 11 is safe and well-supported.packages/apps/mysql/values.yaml (1)
36-44: TheVersionenum is properly defined in bothvalues.yamlandvalues.schema.jsonwith consistent values (v11.8,v11.4,v10.11,v10.6). The@enum {string}syntax is the standard pattern used across the codebase for enum documentation. If a pipeline failure mentions "undefined types: Version", it is likely caused by a different issue in the build tooling or code generation process, not by the enum definition itself.packages/apps/mysql/Makefile (1)
10-12: No issues found with the update target.The Version enum is correctly defined in values.yaml using proper cozyvalues-gen syntax (
@enum {string} Version). The update target correctly callshack/update-versions.shfollowed bymake generate, which invokescozyvalues-gen. controller-gen is not involved in the mysql package build pipeline—it's used in other system packages. No type system errors or pipeline failures related to this code exist.Likely an incorrect or invalid review comment.
packages/apps/mysql/hack/update-versions.sh (4)
1-17: LGTM! Good script setup and error handling.The script correctly sets error handling flags and validates dependencies before proceeding.
87-95: LGTM! Clean version file generation.The version map is correctly written to the YAML file with proper formatting.
101-148: LGTM! Robust values.yaml update logic.The awk-based section replacement and insertion logic correctly handles both cases (existing version section and new insertion).
1-151: The original review comment is based on incorrect assumptions. The pre-commit configuration does not include a shellcheck hook—it only runsmake generate, which does not invokeupdate-versions.sh. While shellcheck warnings do exist in the script (SC2207 on line 82 and SC2064 on line 99), they cannot be causing the reported pre-commit failure with exit code 123 since shellcheck is not part of the pre-commit checks andupdate-versions.shis not called duringmake generate. The actual cause of the pre-commit failure lies elsewhere and should be investigated from the output ofmake generate, not from this script.Likely an incorrect or invalid review comment.
| # Get LTS versions from MariaDB REST API | ||
| echo "Fetching LTS versions from MariaDB REST API..." | ||
| LTS_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}") | ||
|
|
||
| if [ -z "$LTS_VERSIONS_JSON" ]; then | ||
| echo "Error: Could not fetch versions from MariaDB REST API" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Add timeout to curl command.
The curl command lacks a timeout, which could cause the script to hang indefinitely if the API is unresponsive.
Apply this diff:
-LTS_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}")
+LTS_VERSIONS_JSON=$(curl -sSL --max-time 30 "${MARIADB_API_URL}")📝 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.
| # Get LTS versions from MariaDB REST API | |
| echo "Fetching LTS versions from MariaDB REST API..." | |
| LTS_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}") | |
| if [ -z "$LTS_VERSIONS_JSON" ]; then | |
| echo "Error: Could not fetch versions from MariaDB REST API" >&2 | |
| exit 1 | |
| fi | |
| # Get LTS versions from MariaDB REST API | |
| echo "Fetching LTS versions from MariaDB REST API..." | |
| LTS_VERSIONS_JSON=$(curl -sSL --max-time 30 "${MARIADB_API_URL}") | |
| if [ -z "$LTS_VERSIONS_JSON" ]; then | |
| echo "Error: Could not fetch versions from MariaDB REST API" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In packages/apps/mysql/hack/update-versions.sh around lines 19 to 26, the curl
call that fetches LTS versions has no timeout and can hang; update the curl
invocation to include connection and overall timeouts (for example
--connect-timeout 5 and --max-time 15) and enable fail-on-error (e.g. -f) so the
command exits non-zero on HTTP errors; ensure the captured variable still checks
for empty output and preserve the existing stderr error/exit handling.
| for major_version in $LTS_MAJOR_VERSIONS; do | ||
| echo "Fetching patch versions for ${major_version}..." | ||
|
|
||
| # Get patch versions for this major version | ||
| PATCH_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}${major_version}") | ||
|
|
||
| if [ -z "$PATCH_VERSIONS_JSON" ]; then | ||
| echo "Warning: Could not fetch patch versions for ${major_version}, skipping..." >&2 | ||
| continue | ||
| fi | ||
|
|
||
| # Extract all stable patch version IDs (format: MAJOR.MINOR.PATCH) | ||
| # Filter only Stable releases | ||
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r --arg major "$major_version" '.releases | to_entries[] | select(.key | startswith($major + ".")) | select(.value.release_status == "Stable") | .key' | sort -V) | ||
|
|
||
| # If no stable releases found, try to get any releases (for backwards compatibility) | ||
| if [ -z "$PATCH_VERSIONS" ]; then | ||
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r '.releases | keys[]' | grep -E "^${major_version}\." | sort -V) | ||
| fi | ||
|
|
||
| if [ -z "$PATCH_VERSIONS" ]; then | ||
| echo "Warning: Could not find any patch versions for ${major_version}, skipping..." >&2 | ||
| continue | ||
| fi | ||
|
|
||
| # Get the latest patch version | ||
| LATEST_PATCH=$(echo "$PATCH_VERSIONS" | tail -n1) | ||
|
|
||
| # major_version already has format MAJOR.MINOR (e.g., "11.8") | ||
| VERSION_MAP["v${major_version}"]="${LATEST_PATCH}" | ||
| MAJOR_VERSIONS+=("v${major_version}") | ||
| echo "Found version: v${major_version} -> ${LATEST_PATCH}" | ||
| done |
There was a problem hiding this comment.
Add timeout to curl command; logic is sound.
The version mapping logic correctly fetches patch versions and handles missing data gracefully. However, the curl command at line 46 also lacks a timeout.
Apply this diff:
- PATCH_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}${major_version}")
+ PATCH_VERSIONS_JSON=$(curl -sSL --max-time 30 "${MARIADB_API_URL}${major_version}")📝 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.
| for major_version in $LTS_MAJOR_VERSIONS; do | |
| echo "Fetching patch versions for ${major_version}..." | |
| # Get patch versions for this major version | |
| PATCH_VERSIONS_JSON=$(curl -sSL "${MARIADB_API_URL}${major_version}") | |
| if [ -z "$PATCH_VERSIONS_JSON" ]; then | |
| echo "Warning: Could not fetch patch versions for ${major_version}, skipping..." >&2 | |
| continue | |
| fi | |
| # Extract all stable patch version IDs (format: MAJOR.MINOR.PATCH) | |
| # Filter only Stable releases | |
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r --arg major "$major_version" '.releases | to_entries[] | select(.key | startswith($major + ".")) | select(.value.release_status == "Stable") | .key' | sort -V) | |
| # If no stable releases found, try to get any releases (for backwards compatibility) | |
| if [ -z "$PATCH_VERSIONS" ]; then | |
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r '.releases | keys[]' | grep -E "^${major_version}\." | sort -V) | |
| fi | |
| if [ -z "$PATCH_VERSIONS" ]; then | |
| echo "Warning: Could not find any patch versions for ${major_version}, skipping..." >&2 | |
| continue | |
| fi | |
| # Get the latest patch version | |
| LATEST_PATCH=$(echo "$PATCH_VERSIONS" | tail -n1) | |
| # major_version already has format MAJOR.MINOR (e.g., "11.8") | |
| VERSION_MAP["v${major_version}"]="${LATEST_PATCH}" | |
| MAJOR_VERSIONS+=("v${major_version}") | |
| echo "Found version: v${major_version} -> ${LATEST_PATCH}" | |
| done | |
| for major_version in $LTS_MAJOR_VERSIONS; do | |
| echo "Fetching patch versions for ${major_version}..." | |
| # Get patch versions for this major version | |
| PATCH_VERSIONS_JSON=$(curl -sSL --max-time 30 "${MARIADB_API_URL}${major_version}") | |
| if [ -z "$PATCH_VERSIONS_JSON" ]; then | |
| echo "Warning: Could not fetch patch versions for ${major_version}, skipping..." >&2 | |
| continue | |
| fi | |
| # Extract all stable patch version IDs (format: MAJOR.MINOR.PATCH) | |
| # Filter only Stable releases | |
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r --arg major "$major_version" '.releases | to_entries[] | select(.key | startswith($major + ".")) | select(.value.release_status == "Stable") | .key' | sort -V) | |
| # If no stable releases found, try to get any releases (for backwards compatibility) | |
| if [ -z "$PATCH_VERSIONS" ]; then | |
| PATCH_VERSIONS=$(echo "$PATCH_VERSIONS_JSON" | jq -r '.releases | keys[]' | grep -E "^${major_version}\." | sort -V) | |
| fi | |
| if [ -z "$PATCH_VERSIONS" ]; then | |
| echo "Warning: Could not find any patch versions for ${major_version}, skipping..." >&2 | |
| continue | |
| fi | |
| # Get the latest patch version | |
| LATEST_PATCH=$(echo "$PATCH_VERSIONS" | tail -n1) | |
| # major_version already has format MAJOR.MINOR (e.g., "11.8") | |
| VERSION_MAP["v${major_version}"]="${LATEST_PATCH}" | |
| MAJOR_VERSIONS+=("v${major_version}") | |
| echo "Found version: v${major_version} -> ${LATEST_PATCH}" | |
| done |
🤖 Prompt for AI Agents
In packages/apps/mysql/hack/update-versions.sh around lines 42 to 74, the curl
invocation that sets PATCH_VERSIONS_JSON has no timeout; update that command to
include explicit timeout flags (for example add --connect-timeout 5 and
--max-time 10) while keeping -sSL, and consider adding -f to fail on HTTP errors
so the script can handle failures consistently; ensure the rest of the logic
remains unchanged.
| # Sort major versions in descending order (newest first) | ||
| IFS=$'\n' MAJOR_VERSIONS=($(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)) | ||
| unset IFS |
There was a problem hiding this comment.
Fix array assignment to avoid word splitting issues.
The current array assignment using command substitution can cause unexpected behavior with word splitting. Use mapfile for robust array population.
Apply this diff:
-# Sort major versions in descending order (newest first)
-IFS=$'\n' MAJOR_VERSIONS=($(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r))
-unset IFS
+# Sort major versions in descending order (newest first)
+mapfile -t MAJOR_VERSIONS < <(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)📝 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.
| # Sort major versions in descending order (newest first) | |
| IFS=$'\n' MAJOR_VERSIONS=($(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r)) | |
| unset IFS | |
| # Sort major versions in descending order (newest first) | |
| mapfile -t MAJOR_VERSIONS < <(printf '%s\n' "${MAJOR_VERSIONS[@]}" | sort -V -r) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 82-82: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
🤖 Prompt for AI Agents
In packages/apps/mysql/hack/update-versions.sh around lines 81 to 83, the
MAJOR_VERSIONS array is populated via command substitution which can suffer from
word splitting; replace the command-substitution assignment with mapfile to read
sorted lines into the array (e.g. pipe the printf|sort output into mapfile -t
MAJOR_VERSIONS) so elements are preserved exactly and remove the manual
IFS/unset IFS handling.
| # Update values.yaml - enum with major.minor versions only | ||
| TEMP_FILE=$(mktemp) | ||
| trap "rm -f $TEMP_FILE" EXIT | ||
|
|
There was a problem hiding this comment.
Fix trap to use single quotes for deferred expansion.
The trap command with double quotes expands the variable immediately rather than when the signal is received. Use single quotes for proper deferred expansion.
Apply this diff:
-trap "rm -f $TEMP_FILE" EXIT
+trap 'rm -f "$TEMP_FILE"' EXIT📝 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.
| # Update values.yaml - enum with major.minor versions only | |
| TEMP_FILE=$(mktemp) | |
| trap "rm -f $TEMP_FILE" EXIT | |
| # Update values.yaml - enum with major.minor versions only | |
| TEMP_FILE=$(mktemp) | |
| trap 'rm -f "$TEMP_FILE"' EXIT |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 99-99: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
🤖 Prompt for AI Agents
In packages/apps/mysql/hack/update-versions.sh around lines 97 to 100, the trap
currently uses double quotes which expands $TEMP_FILE immediately; change the
trap to use single quotes so removal is deferred (e.g., trap 'rm -f $TEMP_FILE'
EXIT) ensuring the temporary file variable is evaluated at trap execution time
rather than at assignment.
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
5828178 to
3312874
Compare
Signed-off-by: Andrei Kvapil kvapss@gmail.com
What this PR does
Release note
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.