allow granularity when requesting PR images#255
Conversation
Allow to set labels like build-pr-images/jumpstarter to build only the jumpstarter issues to reduce chances for throttle and speed up the process Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> Assisted-by: claude-opus-4.6
📝 WalkthroughWalkthroughThe PR reworks the image-selection logic in the build workflow, replacing array-based label checks with JSON-based checks and introducing a pre-check step that conditionally gates subsequent build steps based on PR labels. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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.
🧹 Nitpick comments (2)
.github/workflows/build-images.yaml (2)
204-220: HardcodedallImageslist must stay in sync with the matrix.The
allImagesarray (lines 206-214) duplicates the image names from the matrix (lines 33-61). If a new image is added to the matrix or one is removed, this list must be updated in lockstep—easy to forget.GitHub Actions doesn't provide a straightforward way to pass the matrix definition into a downstream job, so this is a known limitation. Consider adding a comment near both locations cross-referencing each other to reduce drift risk, e.g.:
+ // NOTE: keep in sync with the matrix in build-and-push-image const allImages = [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-images.yaml around lines 204 - 220, The duplicated hardcoded image list in the workflow (the const allImages array) can drift from the matrix; add a clear cross-reference comment adjacent to the allImages declaration and the matrix definition that states they must be kept in sync (e.g., "Keep in sync with matrix 'images' above" and link to the other section), and optionally note the label format (`build-pr-images/<shortName>`) used by the images filtering logic (variables: allImages, images, labels.includes('build-pr-images')). This ensures future maintainers see the relationship and update both places together.
22-22: Substring match oncontains(toJSON(...), ...)could produce false positives.
contains(toJSON(...), 'build-pr-images')matches any label whose name includes that substring (e.g., a hypotheticalno-build-pr-images). The inner pre-check step (lines 77-79) uses exactjqmatching, so actual builds are safe, but the job itself would still spin up matrix runners unnecessarily in that edge case.A stricter alternative would be to use
fromJSON+ ajq-like check or simply matchstartsWith:if: >- github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'build-pr-images') || join(toJSON(github.event.pull_request.labels.*.name), ',') ...That said, with the inner pre-check gating, the practical risk is near-zero—just runner allocation cost on a false match.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-images.yaml at line 22, The current conditional uses contains(toJSON(github.event.pull_request.labels.*.name), 'build-pr-images') which performs a substring search and can yield false positives; update the if condition to perform an exact label-element match instead by replacing contains(toJSON(...), 'build-pr-images') with contains(github.event.pull_request.labels.*.name, 'build-pr-images') (or alternatively use a join/toJSON approach or startsWith on each label name) so the check matches whole label names rather than substrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-images.yaml:
- Around line 204-220: The duplicated hardcoded image list in the workflow (the
const allImages array) can drift from the matrix; add a clear cross-reference
comment adjacent to the allImages declaration and the matrix definition that
states they must be kept in sync (e.g., "Keep in sync with matrix 'images'
above" and link to the other section), and optionally note the label format
(`build-pr-images/<shortName>`) used by the images filtering logic (variables:
allImages, images, labels.includes('build-pr-images')). This ensures future
maintainers see the relationship and update both places together.
- Line 22: The current conditional uses
contains(toJSON(github.event.pull_request.labels.*.name), 'build-pr-images')
which performs a substring search and can yield false positives; update the if
condition to perform an exact label-element match instead by replacing
contains(toJSON(...), 'build-pr-images') with
contains(github.event.pull_request.labels.*.name, 'build-pr-images') (or
alternatively use a join/toJSON approach or startsWith on each label name) so
the check matches whole label names rather than substrings.
Container ImagesThe following container images have been built for this PR:
Images expire after 7 days. |
Allow to set labels like build-pr-images/jumpstarter to build only the jumpstarter issues to reduce chances for throttle and speed up the process
fixes #252
Summary by CodeRabbit
Release Notes