Skip to content

Commit da835c0

Browse files
EliShteinmanclaude
andcommitted
fix(ci): tighten publish_chart phase against silent half-success
Three issues from a second pass over the new opt-in chart publish: 1. Each gated step had `if: steps.bump.outputs.changed == 'true'` without `success()`. In GitHub Actions an explicit `if:` REPLACES the default `success()` check, so a failed `git push` (e.g. branch moved during the run) would still let the subsequent helm push run — leaving the OCI chart pushed but the source-of-truth commit not in git. Prepend `success() &&` to every gated step. 2. Version parsing `IFS=. read -r MAJ MIN PATCH` blindly trusted `X.Y.Z` shape. A version like `1.5.6-rc1` parsed PATCH as `6-rc1` and `$((PATCH+1))` would crash mid-bump after sed had already modified files. Validate `OLD_VERSION` matches `^[0-9]+\.[0-9]+\.[0-9]+$` before mutating anything, and exit with a clear message pointing to BUILD.md step 5 for manual bumps. 3. appVersion parsing assumes double-quoted value (current Chart.yaml convention). If a future edit drops the quotes, the sed fallback silently captures the entire line and the global `sed -i` would mangle every file. Validate `OLD_HASH` matches `^[a-f0-9]{7,40}$` before proceeding. Also drop `inputs.publish_chart == true` for the cleaner `inputs.publish_chart` (the input is `type: boolean`, so the comparison was redundant). Edge cases verified: happy path, same-hash skip, pre-release suffix rejected, build-meta suffix rejected, non-hex appVersion rejected, 7-char hash accepted, uppercase hash rejected, double- digit patch (1.5.99 -> 1.5.100). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent de2dc35 commit da835c0

1 file changed

Lines changed: 18 additions & 6 deletions

File tree

.github/workflows/airgap-build.yml

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ jobs:
813813
name: 5. Publish Helm chart (opt-in)
814814
needs: [setup, docker]
815815
if: >-
816-
inputs.publish_chart == true &&
816+
inputs.publish_chart &&
817817
inputs.variant == 'both' &&
818818
inputs.platforms == 'both' &&
819819
inputs.tag_override == ''
@@ -838,13 +838,22 @@ jobs:
838838
OLD_VERSION=$(grep '^version:' "$CHART" | awk '{print $2}')
839839
OLD_HASH=$(grep '^appVersion:' "$CHART" | sed -E 's/.*"([^"]+)".*/\1/')
840840
841+
# Defensive: ensure parsing produced sensible values before we sed
842+
if [[ ! "$OLD_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
843+
echo "Cannot auto-bump: version '$OLD_VERSION' is not plain X.Y.Z (pre-release / build suffix not supported by auto-bump — run BUILD.md step 5 manually)." >&2
844+
exit 1
845+
fi
846+
if [[ ! "$OLD_HASH" =~ ^[a-f0-9]{7,40}$ ]]; then
847+
echo "Cannot auto-bump: appVersion '$OLD_HASH' is not a hex commit hash (parse fallback hit — check Chart.yaml format)." >&2
848+
exit 1
849+
fi
850+
841851
if [ "$OLD_HASH" = "$NEW_HASH" ]; then
842852
echo "appVersion already $NEW_HASH — nothing to bump" >&2
843853
echo "changed=false" >> "$GITHUB_OUTPUT"
844854
exit 0
845855
fi
846856
847-
# Auto-bump patch (X.Y.Z -> X.Y.(Z+1))
848857
IFS=. read -r MAJ MIN PATCH <<<"$OLD_VERSION"
849858
NEW_VERSION="$MAJ.$MIN.$((PATCH + 1))"
850859
@@ -866,8 +875,11 @@ jobs:
866875
echo "changed=true"
867876
} >> "$GITHUB_OUTPUT"
868877
878+
# success() must be repeated in every gated step because an explicit `if`
879+
# replaces the default success() check — without it, a failed `git push`
880+
# would still let the helm push run, leaving git and OCI inconsistent.
869881
- name: Commit & push
870-
if: steps.bump.outputs.changed == 'true'
882+
if: success() && steps.bump.outputs.changed == 'true'
871883
env:
872884
NEW_VERSION: ${{ steps.bump.outputs.new_version }}
873885
NEW_HASH: ${{ steps.bump.outputs.new_hash }}
@@ -880,11 +892,11 @@ jobs:
880892
git push
881893
882894
- name: Install Helm
883-
if: steps.bump.outputs.changed == 'true'
895+
if: success() && steps.bump.outputs.changed == 'true'
884896
uses: azure/setup-helm@v4
885897

886898
- name: Helm registry login
887-
if: steps.bump.outputs.changed == 'true'
899+
if: success() && steps.bump.outputs.changed == 'true'
888900
env:
889901
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
890902
DOCKERHUB_USERNAME: ${{ vars.DOCKERHUB_USERNAME }}
@@ -893,7 +905,7 @@ jobs:
893905
-u "$DOCKERHUB_USERNAME" --password-stdin registry-1.docker.io
894906
895907
- name: Package & push chart
896-
if: steps.bump.outputs.changed == 'true'
908+
if: success() && steps.bump.outputs.changed == 'true'
897909
id: chart_push
898910
env:
899911
DOCKERHUB_USERNAME: ${{ vars.DOCKERHUB_USERNAME }}

0 commit comments

Comments
 (0)