Repository navigation
ci(helm): add Helm chart release workflow to GHCR - #864
Conversation
Add automated Helm chart packaging and publishing to GitHub Container Registry (OCI), triggered when bindings/python/pyproject.toml changes on main (same trigger as PyPI and Docker releases). What changed: - .github/workflows/release-helm.yml: new workflow that reads version from pyproject.toml, syncs Chart.yaml version+appVersion at build time, lints, packages, and pushes to oci://ghcr.io/lightseekorg/charts - deploy/helm/smg/Chart.yaml: bump chart version from 0.1.0 to 1.3.3 to match the current SMG release version - scripts/check_release_versions.sh: add helm chart type with getter/setter functions, track Chart.yaml in SMG_VERSION_SYNC so future releases auto-detect version drift Why: The Helm chart had no release pipeline — it was committed and versioned manually. Chart version (0.1.0) was out of sync with the app version (1.3.3). This aligns the chart release with the existing PyPI/Docker release pattern: version sourced from pyproject.toml, published on push to main. Install after publish: helm install smg oci://ghcr.io/lightseekorg/charts/smg --version 1.3.3 Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
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 addresses the lack of an automated release pipeline for the Helm chart, which previously led to outdated chart versions and manual installation processes. By implementing a new GitHub Actions workflow, the Helm chart's release process is now fully automated, ensuring that it is consistently packaged and published to GHCR with synchronized versioning. This enhancement streamlines the deployment experience for users by providing an official registry for chart installation and maintaining version consistency across components. Highlights
Ignored Files
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. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow to publish an OCI-packaged Helm chart (syncing version from Python pyproject), updates Chart.yaml version, and extends the release-version check script to read/update Helm chart version/appVersion. Changes
Sequence DiagramsequenceDiagram
participant GitHub as GitHub Push
participant Actions as GitHub Actions (release-helm.yml)
participant Repo as Repository (pyproject.toml)
participant Helm as Chart.yaml
participant GHCR as GHCR OCI Registry
GitHub->>Actions: Trigger (push to main or manual)
Actions->>Repo: Read bindings/python/pyproject.toml
Repo-->>Actions: Return semver (e.g., 1.3.3)
Actions->>Helm: Update Chart.yaml version & appVersion
Helm-->>Actions: Confirm Chart.yaml updated
Actions->>Actions: Lint and package chart (.helm-pkg/)
Actions->>GHCR: Login with GITHUB_TOKEN
Actions->>GHCR: Push packaged chart to OCI
GHCR-->>Actions: Acknowledge publish
Actions->>Actions: Append release summary to step summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a GitHub Actions workflow to automate the release of the Helm chart to GHCR. The changes include the new workflow file, updating the Helm chart version, and modifying the check_release_versions.sh script to support version synchronization for the Helm chart. The changes look good overall. I've added a couple of suggestions to improve the robustness of the shell script logic for parsing and updating the Chart.yaml file by making it more tolerant to whitespace variations, aligning with the rule to ensure accurate and robust handling of external resource versions.
| get_helm_chart_version() { | ||
| local file="$1" | ||
| grep -m1 '^version:' "$file" | sed 's/version: *//' | ||
| } |
There was a problem hiding this comment.
The current implementation using grep and sed can be brittle. It may not correctly parse the version if there is extra whitespace after the colon, or if the version value is quoted. Using awk can make this parsing more robust.
This awk command correctly parses the version regardless of the amount of whitespace after the colon and strips optional quotes from the value. This is more robust and consistent with how other version-extraction functions in this script handle quotes.
| get_helm_chart_version() { | |
| local file="$1" | |
| grep -m1 '^version:' "$file" | sed 's/version: *//' | |
| } | |
| get_helm_chart_version() { | |
| local file="$1" | |
| awk -F':[[:space:]]*' '/^version:/ {gsub(/"/, "", $2); print $2; exit}' "$file" | |
| } |
References
- Ensuring robust parsing and manipulation of version numbers in scripts is critical for maintaining accuracy and preventing build failures related to incorrect version identification or updates.
| set_helm_chart_version() { | ||
| local file="$1" | ||
| local old_version="$2" | ||
| local new_version="$3" | ||
| local escaped_old | ||
| escaped_old=$(escape_version "$old_version") | ||
| sed_inplace "s/^version: ${escaped_old}/version: ${new_version}/" "$file" | ||
| sed_inplace "s/^appVersion: \"${escaped_old}\"/appVersion: \"${new_version}\"/" "$file" | ||
| if ! grep -q "^version: ${new_version}" "$file"; then | ||
| echo -e " ${RED}FAILED to update $file${NC}" >&2 | ||
| return 1 | ||
| fi | ||
| } |
There was a problem hiding this comment.
The sed commands and the final grep check in this function assume a specific amount of whitespace around the colon in version: and appVersion: lines. This is brittle and may fail if the file formatting changes (e.g., more spaces are added). The logic should handle any amount of whitespace to make the script more robust.
This change uses [[:space:]]* to match any amount of whitespace and uses a capture group with a back-reference to preserve the original spacing, making the update more robust. The final check is also updated to be whitespace-agnostic.
| set_helm_chart_version() { | |
| local file="$1" | |
| local old_version="$2" | |
| local new_version="$3" | |
| local escaped_old | |
| escaped_old=$(escape_version "$old_version") | |
| sed_inplace "s/^version: ${escaped_old}/version: ${new_version}/" "$file" | |
| sed_inplace "s/^appVersion: \"${escaped_old}\"/appVersion: \"${new_version}\"/" "$file" | |
| if ! grep -q "^version: ${new_version}" "$file"; then | |
| echo -e " ${RED}FAILED to update $file${NC}" >&2 | |
| return 1 | |
| fi | |
| } | |
| set_helm_chart_version() { | |
| local file="$1" | |
| local old_version="$2" | |
| local new_version="$3" | |
| local escaped_old | |
| escaped_old=$(escape_version "$old_version") | |
| sed_inplace "s/^\(version:[[:space:]]*\)${escaped_old}/\1${new_version}/" "$file" | |
| sed_inplace "s/^\(appVersion:[[:space:]]*\)\"${escaped_old}\"$/\1\"${new_version}\"/" "$file" | |
| if ! grep -q "^version:[[:space:]]*${new_version}" "$file"; then | |
| echo -e " ${RED}FAILED to update $file${NC}" >&2 | |
| return 1 | |
| fi | |
| } |
References
- Ensuring robust parsing and manipulation of version numbers in scripts is critical for maintaining accuracy and preventing build failures related to incorrect version identification or updates.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-helm.yml:
- Around line 15-23: Add a workflow-level concurrency block to the release job
(jobs.release / name: Package and push Helm chart) to prevent parallel/retried
runs from pushing the same chart; add a concurrency key (e.g. concurrency:
group: ${{ github.repository }}-helm-${{ github.ref }} cancel-in-progress: true)
so only one release for a given ref/branch runs at a time and in-progress runs
are canceled.
- Around line 26-31: The "Determine version" step extracts VERSION from
bindings/python/pyproject.toml but doesn't fail if parsing yields empty/invalid
output; change the step so after computing VERSION (the shell variable VERSION
in the Determine version step) you validate it (non-empty and optionally matches
a semver regex), and if invalid print an error to stderr and exit non-zero
(e.g., echo "Failed to parse version from pyproject.toml" >&2; exit 1), only
write "version=${VERSION}" to $GITHUB_OUTPUT and echo the Using version message
when the check passes.
In `@scripts/check_release_versions.sh`:
- Around line 379-383: The current sed_inplace call only replaces appVersion
when it exactly matches the old escaped value and the subsequent check only
verifies "version:", allowing appVersion to remain stale; change the update to
unconditionally replace the appVersion line (match appVersion: ".*") so it gets
set to "${new_version}", and adjust the verification to assert both
"^appVersion: \"${new_version}\"" and "^version: ${new_version}" are present
(i.e., add a grep check for appVersion after sed_inplace and fail if it doesn't
match) to ensure both fields are synced; refer to the existing sed_inplace
invocation and the grep check for "version:" in this block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 462be764-5f15-4dc0-b523-85d32b323e60
📒 Files selected for processing (3)
.github/workflows/release-helm.ymldeploy/helm/smg/Chart.yamlscripts/check_release_versions.sh
- Workflow: add concurrency group to prevent duplicate chart publish races from parallel/retried runs - Workflow: validate parsed version against semver regex, fail fast if pyproject.toml parsing returns empty or invalid output - Script: use awk for robust Chart.yaml version parsing, handles varying whitespace and optional quotes - Script: set_helm_chart_version now unconditionally replaces appVersion (handles drift) and verifies both version and appVersion after update Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Description
Problem
The Helm chart had no automated release pipeline. The chart version was stuck at
0.1.0while the app version was1.3.3. Users had no way to install the chart from a registry — they had to clone the repo.Solution
Add a GitHub Actions workflow that packages and publishes the Helm chart to GitHub Container Registry (OCI) on every release, using the same version source as PyPI and Docker (
bindings/python/pyproject.toml).Changes
.github/workflows/release-helm.yml— New workflow:pyproject.tomlchanges tomain(same as PyPI/Docker)pyproject.toml, syncsChart.yamlat build timeoci://ghcr.io/lightseekorg/charts/smgdeploy/helm/smg/Chart.yaml— Chart version0.1.0→1.3.3scripts/check_release_versions.sh— Addedhelmtype with getter/setter, Chart.yaml tracked inSMG_VERSION_SYNCarray so version drift is caught during pre-release checksTest Plan
check_release_versions.shdetects helm chart version correctly:azure/setup-helm@v4and standardhelm package/helm pushOCI flowAfter merge, users can install via:
Checklist
Summary by CodeRabbit
New Features
Chores