fix(release): get RHEL base image version in verify-release#19905
fix(release): get RHEL base image version in verify-release#19905
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The minor version extraction assumes a plain
x.y.zSemVer string and may break for pre-release or suffixed tags (e.g.4.11.0-rc.1); consider parsing with shell parameter expansion and stripping non-digit suffixes to make this more robust. - It may be worth adding a small guard to ensure
minor_versionis numeric before using-ge(e.g. defaulting to a safe value or failing fast with a clear error) to avoid unexpected behavior if the version format changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The minor version extraction assumes a plain `x.y.z` SemVer string and may break for pre-release or suffixed tags (e.g. `4.11.0-rc.1`); consider parsing with shell parameter expansion and stripping non-digit suffixes to make this more robust.
- It may be worth adding a small guard to ensure `minor_version` is numeric before using `-ge` (e.g. defaulting to a safe value or failing fast with a clear error) to avoid unexpected behavior if the version format changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/scripts/verify-release.sh:
- Around line 97-100: Validate RELEASE_PATCH format before extracting
minor_version: check RELEASE_PATCH matches a semantic-like pattern (e.g. use a
regex such as ^[0-9]+\.[0-9]+\.[0-9]+$ or at least ensure it contains two dots)
and if it fails, log an error and exit non-zero instead of proceeding; then
safely compute minor_version from RELEASE_PATCH (using cut or shell parameter
expansion) and perform the numeric comparison to set rhel_version ("rhel8" or
"rhel9"). Reference variables: RELEASE_PATCH, minor_version, rhel_version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9a50aca0-78f8-400c-b35e-5f95062b726c
📒 Files selected for processing (1)
.github/workflows/scripts/verify-release.sh
🚀 Build Images ReadyImages are ready for commit 602ba73. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-610-g602ba73593 |
The verify-release script hardcoded rhacs-main-rhel9, which fails for releases 4.10 and below that use rhel8 base images. Instead of hardcoding a version cutoff, read the externalRepo from the release branch's .tekton/create-custom-snapshot.yaml which is the source of truth for the downstream image repository name. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
da6fa38 to
debf59c
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Parsing the GitHub API JSON response with
grep/sedis brittle; consider usingjqto extractexternalReporeliably and avoid breaking if the JSON formatting changes. - It may be worth explicitly checking the exit status of the
gh apicall before piping it, so that a failure to fetch.tekton/create-custom-snapshot.yamlis detected and reported clearly rather than just resulting in an emptymain_repo.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Parsing the GitHub API JSON response with `grep`/`sed` is brittle; consider using `jq` to extract `externalRepo` reliably and avoid breaking if the JSON formatting changes.
- It may be worth explicitly checking the exit status of the `gh api` call before piping it, so that a failure to fetch `.tekton/create-custom-snapshot.yaml` is detected and reported clearly rather than just resulting in an empty `main_repo`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19905 +/- ##
=======================================
Coverage 49.60% 49.61%
=======================================
Files 2766 2765 -1
Lines 208567 208541 -26
=======================================
+ Hits 103454 103459 +5
+ Misses 97436 97405 -31
Partials 7677 7677
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Tom Martensen <tmartens@redhat.com>
problem:
The verify-release script hardcoded
rhacs-main-rhel9, which fails for releases 4.10 and below that use rhel8 base images.fix:
Check if the operator image (no "rhelN" metadata is in the image name) for the release version.
Verified by running against 4.8.10, 4.9.5, and 4.10.1 — all pass the image check (example run with @tommartensen 's suggestion: https://github.com/stackrox/stackrox/actions/runs/24196594919)