Changed version and publish chart workflow #29
Conversation
…if the version has been bumped
📝 Walkthrough## Walkthrough
The workflow for publishing a Helm chart was updated to allow manual triggering, restrict event monitoring to specific files, and introduce a new job to detect version changes in the chart. Additionally, the script responsible for building and pushing the chart now verifies prerequisites and extracts the version from the chart's YAML file using `yq` instead of reading from a separate version file.
## Changes
| File(s) | Change Summary |
|----------------------------------|-----------------------------------------------------------------------------------------------------------|
| .github/workflows/publish-chart.yml | Added `workflow_dispatch` manual trigger; restricted event paths to `chart/**` and workflow file; introduced `check-version-change` job to detect chart version changes; made `publish-chart` job conditional on version change or manual trigger; updated checkout step in both jobs to handle PR and push refs. |
| do.sh | Enhanced `build_push_chart` to check for `yq` and `chart/Chart.yaml` existence before proceeding; changed version extraction to use `yq` on `chart/Chart.yaml`; reformatted Helm push command for clarity. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant GitHub Actions
participant check-version-change
participant publish-chart
User->>GitHub Actions: Push/PR to chart/** or manual trigger
GitHub Actions->>check-version-change: Run job
alt Manual trigger
check-version-change->>publish-chart: Set changed=true
else Version change detected
check-version-change->>publish-chart: Set changed=true if version changed
else No version change
check-version-change->>publish-chart: Set changed=false
end
publish-chart-->>GitHub Actions: Run only if changed=true |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic found 2 issues across 2 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
.github/workflows/publish-chart.yml
Outdated
| if: github.event_name != 'workflow_dispatch' # Skip if manually triggered | ||
| run: | | ||
| if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then | ||
| PREVIOUS_VERSION=$(git show HEAD^:chart/Chart.yaml | grep 'version:' | awk '{print $2}') |
There was a problem hiding this comment.
If Chart.yaml did not exist in the previous commit (e.g. the file was just added), git show HEAD^:chart/Chart.yaml returns a non-zero status which, together with set -eo pipefail used by GitHub runners, terminates the whole step and aborts the workflow. Guard the command or swallow the error so the version-check logic can proceed safely.
| @@ -15,12 +15,11 @@ build_push_image() { | |||
| } | |||
|
|
|||
| build_push_chart() { | |||
| version=$(cat VERSION) | |||
| version=$(yq -r '.version' chart/Chart.yaml) | |||
There was a problem hiding this comment.
The yq command is invoked with the -r flag but without the required sub-command (e|eval) that newer versions of mikefarah/yq expect; this exits with "unknown shorthand flag" and stops the build script (set -e).
![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
🔭 Outside diff range comments (1)
.github/workflows/publish-chart.yml (1)
57-61: 🛠️ Refactor suggestionAlign checkout ref with version-check job
Thecheck-version-changejob uses:ref: ${{ github.head_ref || github.ref_name }}but the
publish-chartjob only usesgithub.head_ref. To cover both PRs and pushes, update it:- name: Checkout Code uses: actions/checkout@v3 with: - ref: ${{ github.head_ref }} + ref: ${{ github.head_ref || github.ref_name }} fetch-depth: 0🧰 Tools
🪛 actionlint (1.7.7)
58-58: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🧹 Nitpick comments (3)
.github/workflows/publish-chart.yml (3)
29-47: Useyqfor robust YAML parsing in version check
Relying ongrepandawkcan break ifChart.yamlformatting changes. Since you already depend onyq, consider:- PREVIOUS_VERSION=$(git show HEAD^:chart/Chart.yaml | grep 'version:' | awk '{print $2}') + PREVIOUS_VERSION=$(git show HEAD^:chart/Chart.yaml | yq -r '.version') - CURRENT_VERSION=$(grep 'version:' chart/Chart.yaml | awk '{print $2}') + CURRENT_VERSION=$(yq -r '.version' chart/Chart.yaml)This ensures consistent parsing.
🧰 Tools
🪛 actionlint (1.7.7)
31-31: shellcheck reported issue in this script: SC2086:info:7:28: Double quote to prevent globbing and word splitting
(shellcheck)
31-31: shellcheck reported issue in this script: SC2086:info:10:29: Double quote to prevent globbing and word splitting
(shellcheck)
31-31: shellcheck reported issue in this script: SC2086:info:14:27: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 YAMLlint (1.37.1)
[error] 35-35: trailing spaces
(trailing-spaces)
36-36: Remove trailing whitespace
This blank line contains trailing spaces and is flagged by YAML lint. Please delete the extra whitespace to satisfy the linter.
53-55: Simplify publish-chart job condition
Since the manual step already setsshould_publish=true, the|| github.event_name == 'workflow_dispatch'is redundant. You can streamline to:if: needs.check-version-change.outputs.should_publish == 'true'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish-chart.yml(1 hunks)do.sh(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/publish-chart.yml
23-23: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
31-31: shellcheck reported issue in this script: SC2086:info:7:28: Double quote to prevent globbing and word splitting
(shellcheck)
31-31: shellcheck reported issue in this script: SC2086:info:10:29: Double quote to prevent globbing and word splitting
(shellcheck)
31-31: shellcheck reported issue in this script: SC2086:info:14:27: Double quote to prevent globbing and word splitting
(shellcheck)
50-50: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/publish-chart.yml
[error] 35-35: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
.github/workflows/publish-chart.yml (3)
3-12: Restrict workflow triggers and enable manual dispatch
Theon:section now scopes PR and push events to justchart/Chart.yamland the workflow file, and addsworkflow_dispatchfor manual runs. This aligns perfectly with the goal of reducing unnecessary runs.
17-21: Expose version-change flag as a job output
Mappingsteps.check-version.outputs.changedtooutputs.should_publishmakes downstream logic more readable.
49-51: Override version check for manual dispatch
The step correctly setschanged=truewhenworkflow_dispatchis used, ensuring the chart is published on manual trigger.🧰 Tools
🪛 actionlint (1.7.7)
50-50: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/publish-chart.yml (2)
37-37: Trim trailing whitespace
Line 37 contains trailing spaces which can break YAML linting.Apply this diff to remove them:
-run: | - +run: | +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 37-37: trailing spaces
(trailing-spaces)
61-68: DRY up duplicated checkout configuration
Theactions/checkout@v3step is repeated in both jobs. Consider extracting it via a YAML anchor or a composite action to simplify maintenance and avoid drift.🧰 Tools
🪛 actionlint (1.7.7)
65-65: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-chart.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/publish-chart.yml
23-23: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
65-65: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/publish-chart.yml
[error] 37-37: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/publish-chart.yml (4)
4-4: Enable manual triggering with workflow_dispatch
Addingworkflow_dispatchallows on-demand chart publishes, which aligns well with the goal of manual runs.
7-8: Restrict triggers to relevant files only
Limitingpull_requestandpushpaths tochart/Chart.yamland the workflow file prevents unnecessary runs on unrelated changes.
17-27: Introduce version-change detection job
The newcheck-version-changejob correctly checks out full history (fetch-depth: 0) and setsshould_publishbased on version diffs or manual dispatch. This cleanly isolates the publish condition.🧰 Tools
🪛 actionlint (1.7.7)
23-23: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
54-58: Manual runs always trigger publication
Themanual-flagstep correctly setschanged=trueforworkflow_dispatchevents, ensuring thepublish-chartjob executes as intended.
| run: | | ||
| if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then | ||
| # Check if Chart.yaml existed in previous commit |
There was a problem hiding this comment.
Avoid missing version changes on multi-commit pushes
Using git diff HEAD^ HEAD inspects only the last commit. If chart/Chart.yaml was modified earlier in a multi-commit push, this will be missed.
Switch to comparing the full push range provided by GitHub Actions:
- if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then
+ if git diff --name-only ${{ github.event.before }} ${{ github.sha }} | grep -q 'chart/Chart.yaml'; thenThis ensures all changes in the push are evaluated.
📝 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.
| run: | | |
| if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then | |
| # Check if Chart.yaml existed in previous commit | |
| run: | | |
| if git diff --name-only ${{ github.event.before }} ${{ github.sha }} | grep -q 'chart/Chart.yaml'; then | |
| # Check if Chart.yaml existed in previous commit |
🤖 Prompt for AI Agents
In .github/workflows/publish-chart.yml around lines 31 to 33, the script uses
'git diff HEAD^ HEAD' which only checks the last commit for changes to
'chart/Chart.yaml', missing modifications in earlier commits of a multi-commit
push. Update the git diff command to use the full push range provided by GitHub
Actions, such as 'git diff ${{ github.event.before }} ${{ github.event.after
}}', to ensure all changes in the entire push are evaluated.
…hing is changed in the charts directory
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/publish-chart.yml (1)
31-33: Avoid missing version changes on multi-commit pushesUsing
git diff HEAD^ HEADonly inspects the last commit. In a multi-commit push, earlier changes tochart/Chart.yamlwould be skipped.Update the diff command to cover the full push range:
- if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then + if git diff --name-only ${{ github.event.before }} ${{ github.event.after }} | grep -q 'chart/Chart.yaml'; then
🧹 Nitpick comments (1)
.github/workflows/publish-chart.yml (1)
37-37: Remove trailing whitespaceThere's a line with stray spaces that could cause YAML lint errors.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 37-37: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-chart.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/publish-chart.yml
23-23: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
65-65: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/publish-chart.yml
[error] 37-37: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/publish-chart.yml (5)
4-8: Manual and path-trigger configuration looks good.The addition of
workflow_dispatchfor manual runs and precise path filters forpull_requestandpushevents correctly scopes the workflow to chart-related changes. This aligns with the PR objective to reduce unnecessary runs.Also applies to: 11-12
17-27: check-version-change job configuration is sound.The job initializes outputs, fetches the full commit history (
fetch-depth: 0), and correctly determines the ref to checkout using${{ github.head_ref || github.ref_name }}. This sets up the version-check logic robustly.🧰 Tools
🪛 actionlint (1.7.7)
23-23: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
54-57: Manual-run flag step is correct.The
manual-flagstep appropriately setschanged=truewhenworkflow_dispatchis used. This ensures the publish job will run on manual triggers.
60-62: Conditional publish job gating is correct.Requiring
needs.check-version-change.outputs.should_publish == 'true'ensures thepublish-chartjob only executes when a version bump or manual trigger occurs.
67-68: Checkout ref expression is consistent.Using
${{ github.head_ref || github.ref_name }}for both jobs ensures branches and PR contexts are handled uniformly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/publish-chart.yml (1)
29-30: Update Git diff to cover entire push range
Usinggit diff HEAD^ HEADonly examines the last commit and can miss changes in multi-commit pushes. Switch to:- if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then + if git diff --name-only ${{ github.event.before }} ${{ github.sha }} | grep -q 'chart/Chart.yaml'; thenThis aligns with GitHub’s full push range.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-chart.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/publish-chart.yml
21-21: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
59-59: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (8)
.github/workflows/publish-chart.yml (8)
3-3: Enable manual triggering of the workflow
Addingworkflow_dispatchis a great way to allow on-demand publishes without code changes.
6-7: Restrict PR triggers to chart and workflow file changes
Limiting pull_request events tochart/**and this workflow ensures you don’t run the chart-publish logic on unrelated changes.
10-11: Restrict push triggers to chart and workflow file changes on main
This mirrors the PR filter and keeps runs focused whenchart/**or this workflow are updated.
15-18: Introducecheck-version-changejob with consolidated output
Good use of a dedicated job and a combinedshould_publishoutput to gate the publish step.
20-24: Use full clone and correct ref for both PRs and pushes
Fetching full history and using${{ github.head_ref || github.ref_name }}covers PR branches as well as tag or branch builds.🧰 Tools
🪛 actionlint (1.7.7)
21-21: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
26-29: Skip version diff logic on manual runs
Theif: github.event_name != 'workflow_dispatch'guard cleanly separates manual versus automated checks.
50-53: Set publish flag on manual dispatch
Echoingchanged=trueforworkflow_dispatchruns is clear and straightforward.
56-57: Conditional publish job based on version check
Gatingpublish-chartwithneeds.check-version-change.outputs.should_publish == 'true'cleanly prevents unnecessary publishes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/publish-chart.yml (1)
33-56:⚠️ Potential issueUse full push range for detecting version changes
The check
git diff --name-only HEAD^ HEADonly inspects the last commit and can miss modifications in earlier commits of a multi-commit push. Switch to the full range provided by GitHub Actions:- if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then + if git diff --name-only ${{ github.event.before }} ${{ github.event.after }} | grep -q 'chart/Chart.yaml'; thenThis guarantees that all commits in the push are evaluated for version bumps.
🧹 Nitpick comments (1)
.github/workflows/publish-chart.yml (1)
26-31: Pinyqversion to ensure reproducibilityInstalling
yqvia thelatestdownload link can introduce breaking changes unexpectedly. Consider pinning to a specific release, for example:- sudo wget https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 -O /usr/local/bin/yq + sudo wget https://github.com/mikefarah/yq/releases/download/v4.35.1/yq_linux_amd64 -O /usr/local/bin/yq
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-chart.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/publish-chart.yml
21-21: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
69-69: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
.github/workflows/publish-chart.yml (4)
3-11: Approve workflow trigger updatesSupporting manual dispatch and restricting
pull_requestandpushevents to changes inchart/**and the workflow file reduces unnecessary runs and centralizes version bump checks.
15-19: Approveshould_publishoutput expressionCombining the version-change and manual-flag outputs via
${{ steps.check-version.outputs.changed || steps.manual-flag.outputs.changed }}
correctly encapsulates the publish decision in one output.
23-24: Approve dynamic checkout ref usageUsing
ref: ${{ github.head_ref || github.ref_name }}withfetch-depth: 0ensures the workflow checks out the correct branch or PR head and retains full git history for diff operations.Also applies to: 71-71
58-62: Approve manual publish flagThe
manual-flagstep correctly overrides the version check forworkflow_dispatchevents by settingchanged=true, enabling manual publishes.
| needs: check-version-change | ||
| if: needs.check-version-change.outputs.should_publish == 'true' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent publishing on pull_request events
As written, a version bump in a PR will trigger the publish-chart job and push artifacts for unmerged code. Restrict the publish step to pushes (e.g., on main) by extending the condition:
- if: needs.check-version-change.outputs.should_publish == 'true'
+ if: needs.check-version-change.outputs.should_publish == 'true' && github.event_name == 'push'📝 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.
| needs: check-version-change | |
| if: needs.check-version-change.outputs.should_publish == 'true' | |
| needs: check-version-change | |
| if: needs.check-version-change.outputs.should_publish == 'true' && github.event_name == 'push' |
🤖 Prompt for AI Agents
In .github/workflows/publish-chart.yml at lines 64 to 65, the current condition
allows the publish-chart job to run on pull_request events, which can cause
unmerged code to be published. Modify the if condition to also check that the
event is a push event, for example by adding a condition to ensure
github.event_name == 'push' along with the existing should_publish check, so
publishing only occurs on pushes like to the main branch.
There was a problem hiding this comment.
cubic found 3 issues across 2 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| id: check-version | ||
| if: github.event_name != 'workflow_dispatch' # Skip if manually triggered | ||
| run: | | ||
| if git diff --name-only HEAD^ HEAD | grep -q 'chart/Chart.yaml'; then |
There was a problem hiding this comment.
Using HEAD^ assumes the previous commit exists; on the first commit of a branch or repository this command fails, causing the entire job to error out.
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y jq | ||
| sudo wget https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 -O /usr/local/bin/yq |
There was a problem hiding this comment.
Binary is downloaded and executed without any checksum or signature verification, exposing the build environment to potential supply-chain attacks if the URL is ever compromised.
| echo "Error: chart/Chart.yaml not found." >&2 | ||
| exit 1 | ||
| fi | ||
| version=$(yq -r '.version' chart/Chart.yaml) |
There was a problem hiding this comment.
Using yq without the "eval" (or "e") sub-command fails with modern versions of mikefarah/yq (v4+): running yq -r returns an unknown flag error, causing the script to exit. Use yq e -r (or yq eval -r) so the command works across common yq versions.
| version=$(yq -r '.version' chart/Chart.yaml) | |
| version=$(yq e -r '.version' chart/Chart.yaml) |
The version in the helm chart created from the build_push_chart in do.sh now depends on the version specified in chart/CHart.yaml. This means that the version will only be specified in one place instead of both in the Chart.yaml and a proprietary VERSION file.
Also, the publish-chart.yml workflow has been updated, so it only runs when triggered manually or if the version has been bumped.
Summary by cubic
The chart version is now read directly from chart/Chart.yaml, and the publish-chart workflow only runs on manual trigger or when the chart version changes.
Summary by CodeRabbit