Add attended script for multi-branch UBI bump PRs#3430
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe new ChangesUBI bump pull request automation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Script
participant Git
participant Registry
participant Podman
participant GitHub
Script->>Git: validate state and fetch upstream
Script->>Registry: resolve latest UBI digest
Script->>Git: create branch and run image bump
Script->>Podman: compare RPM inventories
Script->>Git: commit and push changes
Script->>GitHub: create pull request
GitHub-->>Script: return pull request URL
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoAdd attended script to open UBI bump PRs across release branches
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
FYI I used this to generate these PRs:
Works pretty good afaict. |
|
🤖 Finished Review · ✅ Success · Started 3:13 PM UTC · Completed 3:27 PM UTC |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/ubi-bump-prs.sh`:
- Around line 172-176: Update the failed-push handling around the git push in
the script to distinguish non-fast-forward failures, fetch the current remote
state before retrying, and use an explicit --force-with-lease instead of -f. Do
not offer or perform an unconditional force push; preserve the existing prompt
and branch variables while ensuring collaborators’ remote commits cannot be
silently overwritten.
- Around line 140-145: In the declined-PR branch of the interactive loop, reset
the work branch to "$UPSTREAM_REMOTE/$BRANCH" before continuing so the bump’s
tracked changes are discarded and later git checkout -B operations can proceed.
Update the logic surrounding the answer check and preserve the existing skip
messages and continue behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 49eab71c-fba6-403c-b62d-d77c46e186ef
📒 Files selected for processing (1)
hack/ubi-bump-prs.sh
Code Review by Qodo
Context used✅ Compliance rules (platform):
27 rules 1.
|
| read -rp ">>> Create PR for $BRANCH? [y/N] " answer | ||
| echo | ||
| if [[ "${answer,,}" != "y" ]]; then | ||
| echo "Skipping $BRANCH." | ||
| echo | ||
| continue | ||
| fi | ||
|
|
||
| # --- Commit --------------------------------------------------------------- | ||
|
|
||
| COMMIT_MSG="chore(deps): Update ubi-minimal base image | ||
|
|
||
| Old digest: sha256:$OLD_DIGEST | ||
| New digest: sha256:$NEW_DIGEST" | ||
|
|
||
| if [[ -n "$RPM_DIFF" ]]; then | ||
| COMMIT_MSG="$COMMIT_MSG | ||
|
|
||
| RPM changes: | ||
|
|
||
| $RPM_DIFF" | ||
| fi | ||
|
|
||
| EXISTING_FILES=() | ||
| for f in "${DOCKER_FILES[@]}" rpms.lock.yaml; do | ||
| [[ -f "$f" ]] && EXISTING_FILES+=("$f") | ||
| done | ||
| git add "${EXISTING_FILES[@]}" | ||
| git commit -m "$COMMIT_MSG" | ||
|
|
||
| # --- Push ----------------------------------------------------------------- | ||
|
|
||
| if ! git push -u "$PUSH_REMOTE" "$WORK_BRANCH" 2>&1; then | ||
| echo | ||
| read -rp ">>> Push failed (branch may already exist). Force push? [y/N] " force_answer | ||
| if [[ "${force_answer,,}" == "y" ]]; then | ||
| git push -f -u "$PUSH_REMOTE" "$WORK_BRANCH" |
There was a problem hiding this comment.
4. Bash 4-only lowercase 🐞 Bug ☼ Reliability
The script uses ${answer,,} / ${force_answer,,} for case-folding, which requires Bash 4+ and
will fail on older Bash installations. This makes the script unexpectedly non-portable for developer
environments.
Agent Prompt
### Issue description
The script uses Bash 4+ case conversion `${var,,}` when checking prompt responses, which breaks on older Bash versions.
### Issue Context
This script is intended as an attended developer tool, so it should either (a) run on common developer shells or (b) explicitly assert a minimum Bash version.
### Fix Focus Areas
- hack/ubi-bump-prs.sh[140-146]
- hack/ubi-bump-prs.sh[174-177]
### Suggested fix
- Replace `${answer,,}` checks with a Bash-3-compatible construct, e.g.:
- `case "$answer" in [yY]) ... ;; *) ... ;; esac`
- Alternatively, add an early version guard (`BASH_VERSINFO`) with a clear error message if Bash < 4 is detected.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Labels: PR adds developer automation tooling in hack/ for UBI base image management |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Wraps hack/ubi-base-image-bump.sh to automate creating PRs for all active branches, including an RPM version diff in the commit and PR body. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c48f604 to
a3090fb
Compare
|
🤖 Finished Review · ✅ Success · Started 1:27 PM UTC · Completed 1:44 PM UTC |
|
|
||
| # Checkout working branch | ||
| WORK_BRANCH="ubi-bump-${BRANCH}" | ||
| git checkout -B "$WORK_BRANCH" "$UPSTREAM_REMOTE/$BRANCH" --no-track |
There was a problem hiding this comment.
[medium] edge-case
When NEW_DIGEST extraction fails after the bump script has already modified files (line 100: hack/ubi-base-image-bump.sh --no-commit), the continue on line 106 skips to the next branch iteration without cleaning up the dirty working tree. The next iteration's git checkout -B will fail because git refuses to overwrite uncommitted modifications. With set -o errexit, this kills the script, and the cleanup trap's git checkout may also fail, leaving the repository in a dirty state on the wrong branch.
Suggested fix: Add git reset --hard "$UPSTREAM_REMOTE/$BRANCH" before the continue on line 106, matching the reset done on the user-skip path (line 137).
|
|
||
| if [[ $# -gt 0 ]]; then | ||
| BRANCHES=("$@") | ||
| else |
There was a problem hiding this comment.
[medium] indentation
The script uses hard tab indentation throughout, while all other hack/ shell scripts consistently use 2-space indentation. Verified against ubi-base-image-bump.sh, cut-release.sh, go-mod-upgrade-helper.sh, derive-version.sh, add-auto-tag.sh, bump-tekton-bundles.sh, and update-rpm-lock.sh.
Suggested fix: Convert tab indentation to 2-space indentation to match the established convention.
| git fetch "$UPSTREAM_REMOTE" | ||
| echo | ||
|
|
||
| echo "=== Checking latest $UBI_MINIMAL digest ===" |
There was a problem hiding this comment.
[low] edge-case
LATEST_DIGEST is computed once at setup via skopeo inspect --raw | sha256sum, but the inner hack/ubi-base-image-bump.sh runs its own independent skopeo inspect. If the :latest tag moves between calls, the initial printout may differ from the actual digest written. Cosmetic only — commit messages and PR bodies correctly use NEW_DIGEST from the Dockerfile.
| UBI_MINIMAL="${UBI_MINIMAL_REPO}:latest" | ||
|
|
||
| DOCKER_FILES=(Dockerfile Dockerfile.dist acceptance/kubernetes/kind/acceptance.Dockerfile) | ||
|
|
There was a problem hiding this comment.
[low] maintenance
The DOCKER_FILES array duplicates the identical array in hack/ubi-base-image-bump.sh (line 33). If the inner script's list changes, the outer script's git add would miss newly-modified Dockerfiles, resulting in partial commits.
Suggested fix: Consider using git add -u to add all modified tracked files, or sourcing the list from a shared location.
|
|
||
| if [[ $# -gt 0 ]]; then | ||
| BRANCHES=("$@") | ||
| else |
There was a problem hiding this comment.
[low] command-injection
Branch names from command-line arguments flow into git show, git checkout -B, git push, and gh pr create --base commands. Mitigated by proper quoting of $BRANCH in all uses and git's naming constraints; further reduced by the attended, interactive nature of the script.
| hack/ubi-base-image-bump.sh --no-commit | ||
|
|
||
| # Verify the new digest (use whatever the bump script actually wrote) | ||
| NEW_DIGEST=$(sed -nE 's/.*ubi-minimal:latest@sha256:([0-9a-f]{64}).*/\1/p' Dockerfile | head -1) |
There was a problem hiding this comment.
[low] command-injection
Containers run via podman run --rm without explicit security hardening flags. Digest values are regex-constrained to [0-9a-f]{64}, preventing injection. Matches the existing hack/update-rpm-lock.sh pattern.
Summary
hack/ubi-bump-prs.sh, an attended script that creates UBI base image bump PRs across all active release brancheshack/ubi-base-image-bump.shand adds RPM version diffing (viapodman run rpm -qa) in both the commit message and PR bodyUsage
🤖 Generated with Claude Code