Tie chart releases to app releases#43
Conversation
- Reset Chart.yaml version to 0.0.1 with explanatory comment - Update helm-chart-oci-publisher to @17e4144d7ba68f7c3e8c16eece5aed15fd7c2dc8 - Change CHART_VERSION to use github.ref_name instead of reading from Chart.yaml 🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
WalkthroughThese changes modify how the Helm chart version is handled during tagged releases. The workflow now extracts the version from the git tag name instead of reading from Chart.yaml, and updates the publishing action reference. The Chart.yaml version is reset with comments noting dynamic replacement at build time. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ko-build-tag.yaml:
- Around line 36-40: The workflow currently strips all "v" characters when
computing APP_VERSION and CHART_VERSION (using sed 's/v//g'), which corrupts
tags like "v1.2.3+rev"; change the logic so APP_VERSION is computed by removing
only a single leading "v" from github.ref_name (e.g., strip '^v' or use shell
parameter expansion) and then set CHART_VERSION to reuse APP_VERSION (remove the
duplicate sed call), updating the references to APP_VERSION and CHART_VERSION in
the block accordingly.
| set -euo pipefail | ||
| APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | ||
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | ||
| CHART_VERSION="$(yq '.version' charts/*/Chart.yaml)" | ||
| CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | ||
| { |
There was a problem hiding this comment.
Strip only a leading v to avoid corrupting versions.
sed 's/v//g' removes every v, so tags like v1.2.3+rev become 1.2.3+re. This will publish the wrong app/chart version. Strip only a leading v once and reuse APP_VERSION for CHART_VERSION.
✅ Suggested fix
- APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g')
+ APP_VERSION=$(echo "${{ github.ref_name }}" | sed 's/^v//')
CHART_NAME="$(yq '.name' charts/*/Chart.yaml)"
- CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g')
+ CHART_VERSION="$APP_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.
| set -euo pipefail | |
| APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | |
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | |
| CHART_VERSION="$(yq '.version' charts/*/Chart.yaml)" | |
| CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | |
| { | |
| set -euo pipefail | |
| APP_VERSION=$(echo "${{ github.ref_name }}" | sed 's/^v//') | |
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | |
| CHART_VERSION="$APP_VERSION" | |
| { |
🤖 Prompt for AI Agents
In @.github/workflows/ko-build-tag.yaml around lines 36 - 40, The workflow
currently strips all "v" characters when computing APP_VERSION and CHART_VERSION
(using sed 's/v//g'), which corrupts tags like "v1.2.3+rev"; change the logic so
APP_VERSION is computed by removing only a single leading "v" from
github.ref_name (e.g., strip '^v' or use shell parameter expansion) and then set
CHART_VERSION to reuse APP_VERSION (remove the duplicate sed call), updating the
references to APP_VERSION and CHART_VERSION in the block accordingly.
There was a problem hiding this comment.
Pull request overview
This PR implements a change to tie Helm chart releases to application releases by making chart versions dynamic and based on git tags instead of manually maintained in Chart.yaml.
Changes:
- Chart version reset from 0.5.6 to 0.0.1 with explanatory comments about dynamic replacement
- Workflow updated to derive CHART_VERSION from git tags (github.ref_name) instead of reading from Chart.yaml
- Updated helm-chart-oci-publisher action to newer commit that supports dynamic version replacement
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/lfx-v2-project-service/Chart.yaml | Reset version to 0.0.1 and added comment explaining dynamic version replacement during chart build process |
| .github/workflows/ko-build-tag.yaml | Changed CHART_VERSION logic to use github.ref_name (matching APP_VERSION) and updated helm-chart-oci-publisher action commit hash |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| version: 0.5.6 | ||
| # This version should not be incremented, as it is dynamically replaced with the release version during the chart build | ||
| # job. | ||
| version: 0.0.1 |
There was a problem hiding this comment.
Resetting the chart version from 0.5.6 to 0.0.1 could cause semantic versioning issues. When the next release is tagged (e.g., v0.2.0), the chart version will be 0.2.0, which is lower than the previous 0.5.6. This could cause problems with Helm deployments that expect monotonically increasing versions. Consider whether the next release tag should be at least v0.5.7 or v0.6.0 to maintain semantic versioning compatibility, or document this as a breaking change that may require users to use the --force flag or uninstall/reinstall the chart.
| # This version should not be incremented, as it is dynamically replaced with the release version during the chart build | ||
| # job. | ||
| version: 0.0.1 |
There was a problem hiding this comment.
The PR description states that this change "eliminates need to manually update Chart.yaml versions", but the README.md file (lines 260-261, 272-278, and 297) still contains outdated documentation instructing users to manually update the chart version in Chart.yaml. This creates a significant discrepancy between the documented process and the actual implementation. The documentation should be updated in the same PR to avoid confusion for contributors and maintainers.
bramwelt
left a comment
bramwelt
left a comment
There was a problem hiding this comment.
Can you update the README to remove the details around bumping the chart version?
2 participants
This PR implements the changes needed for LFXV2-872 to tie chart releases to app releases.
Changes Made
Chart.yaml Updates
0.5.6to0.0.1Workflow Updates (.github/workflows/ko-build-tag.yaml)
@c465d6571fa0b8be9d551d902955164ea04a00afto@17e4144d7ba68f7c3e8c16eece5aed15fd7c2dc8github.ref_name(matching app version)Impact
Jira: LFXV2-872