feat: multi-arch operator, bundle, and catalog image builds#3055
Open
polasudo wants to merge 10 commits into
Open
feat: multi-arch operator, bundle, and catalog image builds#3055polasudo wants to merge 10 commits into
polasudo wants to merge 10 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Member
|
/agentic_review |
Code Review by Qodo
1. Branch detection uses HEAD
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
Comment on lines
+185
to
+242
| run: | | ||
| SHORT_SHA=$(git rev-parse --short HEAD) | ||
| echo "SHORT_SHA=$SHORT_SHA" >> $GITHUB_ENV | ||
| BASE_VERSION=$(grep -E "^VERSION \?=" Makefile | sed -r -e "s/.+= //") | ||
| echo "BASE_VERSION=$BASE_VERSION" >> $GITHUB_ENV | ||
|
|
||
| latestNext="next" | ||
| if [[ $(git rev-parse --abbrev-ref HEAD) != "main" ]]; then | ||
| latestNext="latest" | ||
| fi | ||
| echo "LATEST_NEXT=$latestNext" >> $GITHUB_ENV | ||
|
|
||
| # Check which architectures actually built | ||
| echo "Build markers found:" | ||
| ls -la /tmp/build-markers/ || echo "No markers found" | ||
|
|
||
| - name: Login to registry (${{ env.REGISTRY }}) | ||
| uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4 | ||
| with: | ||
| registry: ${{ env.REGISTRY }} | ||
| username: ${{ vars.QUAY_USERNAME }} | ||
| password: ${{ secrets.QUAY_TOKEN }} | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 | ||
|
|
||
| - name: Create manifest lists and push | ||
| run: | | ||
| export REGISTRY_WITH_ORG=${{ env.REGISTRY }}/${{ vars.REGISTRY_ORG }} | ||
| export OPERATOR_IMAGE_NAME=${{ vars.OPERATOR_IMAGE_NAME }} | ||
| OPERATOR_IMAGE_NAME=${OPERATOR_IMAGE_NAME:-operator} | ||
|
|
||
| set -ex | ||
|
|
||
| for image in ${OPERATOR_IMAGE_NAME} ${OPERATOR_IMAGE_NAME}-bundle ${OPERATOR_IMAGE_NAME}-catalog; do | ||
| echo "=== Creating manifest list for ${image} ===" | ||
|
|
||
| # Create manifest list for version tag | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-arm64 | ||
|
|
||
| # Create manifest list for version-sha tag | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-arm64 | ||
|
|
||
| # Create manifest list for latestNext tag (next or latest) | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT}-arm64 | ||
|
|
||
| echo "=== Inspecting manifest for ${image}:${LATEST_NEXT} ===" | ||
| docker buildx imagetools inspect ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT} | ||
| done |
There was a problem hiding this comment.
1. Missing set -u strictness 📘 Rule violation ☼ Reliability
New GitHub Actions run scripts use set -ex (or no strict mode) but omit set -u and do not
validate required environment variables, which can mask unset-variable failures. The scripts also
use unquoted variable expansions (e.g., >> $GITHUB_ENV, ${REGISTRY_WITH_ORG}/${image}...),
increasing brittleness and ShellCheck-style compliance risk.
Agent Prompt
## Issue description
The workflow `run` scripts are not hardened: they omit `set -euo pipefail` (notably `-u`), do not validate required env vars, and contain unquoted variable expansions.
## Issue Context
PR Compliance ID 5 requires strict modes, quoting, and env validation for shell scripts embedded in workflows.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[71-99]
- .github/workflows/next-container-build.yaml[114-140]
- .github/workflows/next-container-build.yaml[184-242]
- .github/workflows/next-container-build.yaml[247-264]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+228
to
+232
| # Create manifest list for version-sha tag | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-arm64 |
There was a problem hiding this comment.
2. Wrong sha manifest sources 🐞 Bug ≡ Correctness
The merge job creates the ${BASE_VERSION}-${SHORT_SHA} manifest list from
${BASE_VERSION}-amd64/arm64 instead of the SHA-scoped per-arch tags, so the commit-specific tag
points at the wrong image content. This also makes the pushed per-arch SHA tags unused (and cleanup
deletes tags that were never referenced).
Agent Prompt
## Issue description
The merge job tags `${BASE_VERSION}-${SHORT_SHA}` but incorrectly uses `${BASE_VERSION}-{amd64,arm64}` as sources. This makes the SHA tag not represent the commit build.
## Issue Context
The build job already pushes per-arch SHA tags `${BASE_VERSION}-${SHORT_SHA}-${arch}`.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[228-233]
## Implementation notes
Update the `docker buildx imagetools create` invocation for the `${BASE_VERSION}-${SHORT_SHA}` tag to reference:
- `${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA}-amd64`
- `${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA}-arm64`
(instead of `${BASE_VERSION}-amd64/arm64`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+161
to
+183
| merge: | ||
| name: Create multi-arch manifests | ||
| runs-on: ubuntu-latest | ||
| needs: | ||
| - build | ||
| permissions: | ||
| contents: read | ||
| packages: write | ||
| env: | ||
| HAS_QUAY_OAUTH: ${{ secrets.QUAY_OAUTH_TOKEN != '' }} | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Download build markers | ||
| uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 | ||
| with: | ||
| path: /tmp/build-markers | ||
| pattern: build-marker-* | ||
| merge-multiple: true | ||
|
|
There was a problem hiding this comment.
3. Merge runs without builds 🐞 Bug ☼ Reliability
All build/push and marker upload steps are gated by any_changed == 'true', but the merge job runs unconditionally and will attempt to download markers and create manifests even when nothing was built. This can fail the workflow (no artifacts/tags) on pushes that only change ignored files (e.g., docs/tests).
Agent Prompt
## Issue description
`merge` always runs after the matrix job, but the matrix build/push steps are skipped when `changed-files` reports `any_changed != 'true'`. In that case, `merge` still tries to download build markers and create manifests from tags that were never pushed.
## Issue Context
The build job already computes `steps.changed-files.outputs.any_changed`, but that value is not available to `merge` and the build marker artifacts are only uploaded when changes exist.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[39-62]
- .github/workflows/next-container-build.yaml[146-160]
- .github/workflows/next-container-build.yaml[161-183]
## Implementation notes
Recommended approach:
1. Add a new `changes` job that runs `tj-actions/changed-files` once and exposes `outputs.any_changed`.
2. Add job-level `if: needs.changes.outputs.any_changed == 'true'` to both `build` and `merge`.
3. Update `merge.needs` to include `changes`.
Alternative (less clean): re-run `changed-files` in `merge` and gate the manifest/cleanup steps on its result.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+93
to
+98
| latestNext="next" | ||
| # for main branch, use next tags; for 1.y branches, use :latest tags | ||
| if [[ $(git rev-parse --abbrev-ref HEAD) != "main" ]]; then | ||
| latestNext="latest" | ||
| fi | ||
| echo "LATEST_NEXT=$latestNext" >> $GITHUB_ENV |
There was a problem hiding this comment.
4. Branch detection uses head 🐞 Bug ≡ Correctness
LATEST_NEXT is computed using git rev-parse --abbrev-ref HEAD, but the checkout step does not set ref: and commonly results in a detached HEAD; this can resolve to HEAD and incorrectly pick latest even on main. That mis-tags published images/manifests (next vs latest) for the main branch.
Agent Prompt
## Issue description
Branch/tag selection uses `git rev-parse --abbrev-ref HEAD`, which is unreliable under `actions/checkout` because the repo may be in detached HEAD state. This can miscompute `LATEST_NEXT`.
## Issue Context
Other workflows in this repo already use `${{ github.ref_name }}` to determine the branch.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[34-38]
- .github/workflows/next-container-build.yaml[93-98]
- .github/workflows/next-container-build.yaml[191-195]
## Implementation notes
Replace the `git rev-parse --abbrev-ref HEAD` checks with `${GITHUB_REF_NAME}` (or `${{ github.ref_name }}`) and compare against `main`. Apply the same fix in both the build and merge "Prepare" steps.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
polasudo
force-pushed
the
feat/multi-arch-operator-build
branch
from
June 29, 2026 09:52
367d356 to
38ac280
Compare
polasudo
marked this pull request as ready for review
July 1, 2026 08:22
polasudo
force-pushed
the
feat/multi-arch-operator-build
branch
from
July 1, 2026 09:17
09fe0c7 to
256a876
Compare
Member
|
/review |
PR Reviewer Guide 🔍Warning
Here are some key observations to aid the review process:
|
Replace the single-runner amd64-only build with a matrix strategy that builds on both ubuntu-24.04 (amd64) and ubuntu-24.04-arm (arm64), then merges per-arch images into multi-arch manifest lists. This follows the same proven pattern used by the hub repo's next-build-image.yaml workflow. The operator Dockerfile already supports TARGETOS/TARGETARCH and bundle images use FROM scratch, so no other changes are needed. All three images (operator, operator-bundle, operator-catalog) are now built and published as multi-arch manifest lists. Assisted-by: Claude Code
GitHub Actions doesn't allow direct secrets references in step `if` conditions. Use an env var intermediary instead. Assisted-by: Claude Code
The --annotation flag is not supported by docker buildx imagetools create on the ubuntu-latest runner. Expiry labels are already set during the per-arch image build via the Makefile LABEL variable. Assisted-by: Claude Code
- Use ${{ github.ref_name }} instead of detached-HEAD-unsafe git command
for branch detection; avoids mistagging images (next vs latest)
- Add strict shell mode (set -euo pipefail) and env var validation
across all run: blocks; quote all variable expansions
- Fix SHA-scoped manifest list to use ${BASE_VERSION}-${SHORT_SHA}-{amd64,arm64}
instead of ${BASE_VERSION}-{amd64,arm64} (was using wrong image content)
- Add separate `changes` job to gate both `build` and `merge` on
any_changed output; prevents merge from running when no files changed
- Remove unquoted redirections and improve shell robustness
All 4 Qodo findings resolved.
Assisted-by: Claude Code
Add if: needs.changes.outputs.any_changed == 'true' to individual steps in build job (Prepare, Setup Go, Login, Build, Upload marker) to explicitly skip when no relevant changes detected. This makes the gating more granular and avoids unnecessary compute. Add clarifying comment above Prepare step. Assisted-by: Claude Code
GitHub Actions doesn't support bash-style default syntax (:-). Use || 'operator' instead for fallback values. Assisted-by: Claude Code
- Move QUAY_OAUTH_TOKEN from inline ${{ secrets }} expansion in the
curl command to the step's env: block, resolving SonarCloud warning
about secrets expanded directly in run steps
- Add always() && needs.build.result == 'success' to merge job's if
condition so it won't attempt manifest creation when build failed
Assisted-by: Claude Code
Fortune-Ndlovu
left a comment
Fortune-Ndlovu
left a comment
Member
There was a problem hiding this comment.
thanks for pushing this PR the design is sound and well-documented, I've added a few nitpicks. PTAL thanks
| run: | | ||
| # install skopeo, podman | ||
| set -euo pipefail | ||
| sudo apt-get -y update; sudo apt-get -y install skopeo podman |
Member
There was a problem hiding this comment.
skopeo is installed but never used. The old workflow used skopeo --insecure-policy copy --all for tag copies, but this PR replaced that with podman tag + podman push. Remove skopeo to save ~30s per matrix leg.
|
|
||
| # Skip remaining steps if no relevant changes detected | ||
| - name: Prepare | ||
| if: needs.changes.outputs.any_changed == 'true' |
Member
There was a problem hiding this comment.
This per-step if condition is redundant the job-level if at line 135 already prevents all steps from running when any_changed is false. Same applies to lines 189, 195, 203, 246, and 253. Remove these for clarity.
| # to avoid throttling on RHD org, use GH token | ||
| GH_TOKEN: ${{ secrets.RHDH_BOT_TOKEN }} | ||
|
|
||
| - name: Upload build marker |
Member
There was a problem hiding this comment.
The build markers are uploaded here and downloaded in the merge job (line 279), but they're never actually read or used the docker buildx imagetools create commands at lines 332-347 hardcode amd64 and arm64. Either remove the markers entirely (the needs.build.result == 'success' check already ensures
both matrix legs succeeded), or use them to dynamically build the source list for imagetools create so the workflow can handle partial arch builds gracefully.
| export CONTAINER_TOOL=podman | ||
| export VERSION="${{ env.BASE_VERSION }}" | ||
| export REGISTRY_WITH_ORG="${{ env.REGISTRY }}/${{ vars.REGISTRY_ORG }}" | ||
| export OPERATOR_IMAGE_NAME="${{ vars.OPERATOR_IMAGE_NAME || 'operator' }}" |
Member
There was a problem hiding this comment.
SonarCloud flagged this pattern. ${{ vars.OPERATOR_IMAGE_NAME || 'operator' }} is interpolated directly into the run script by GitHub Actions before bash sees it. If the variable value ever contains shell metacharacters, this enables injection. The safer pattern is already used in the env:
block at lines 240-243 apply the same consistently: add OPERATOR_IMAGE_NAME: ${{ vars.OPERATOR_IMAGE_NAME || 'operator' }} to the step's env: block and reference
${OPERATOR_IMAGE_NAME} in the script. Same applies to lines 211, 319, and 360.
Comment on lines
+371
to
+374
| curl -s -o /dev/null -w "%{http_code}" -X DELETE \ | ||
| -H "Authorization: Bearer ${QUAY_OAUTH_TOKEN}" \ | ||
| "https://quay.io/api/v1/repository/${NAMESPACE}/${image}/tag/${tag}" \ | ||
| || true |
Member
There was a problem hiding this comment.
The HTTP status code from -w '%{http_code}' is printed to stdout but never captured or checked cleanup failures are completely silent. Consider:
status=$(curl -s -o /dev/null -w '%{http_code}' -X DELETE
-H "Authorization: Bearer ${QUAY_OAUTH_TOKEN}"
"https://quay.io/api/v1/repository/${NAMESPACE}/${image}/tag/${tag}\")
echo " -> HTTP ${status} for ${image}:${tag}"
This keeps the || true best-effort behavior while making failures visible in the logs.
Comment on lines
+79
to
+91
| set -euo pipefail | ||
| SHORT_SHA=$(git rev-parse --short HEAD) | ||
| echo "SHORT_SHA=${SHORT_SHA}" >> "$GITHUB_ENV" | ||
| BASE_VERSION=$(grep -E "^VERSION \?=" Makefile | sed -r -e "s/.+= //") | ||
| echo "BASE_VERSION=${BASE_VERSION}" >> "$GITHUB_ENV" | ||
|
|
||
| BRANCH_REF="${{ github.ref_name }}" | ||
| if [[ "${BRANCH_REF}" == "main" ]]; then | ||
| LATEST_NEXT="next" | ||
| else | ||
| LATEST_NEXT="latest" | ||
| fi | ||
| echo "LATEST_NEXT=${LATEST_NEXT}" >> "$GITHUB_ENV" |
Member
There was a problem hiding this comment.
This SHORT_SHA, BASE_VERSION, and LATEST_NEXT computation is copy-pasted identically in the build job (line 157) and merge job (line 287). Consider computing these once in the changes job and exporting them as additional outputs to eliminate three-way drift.
operator-bundle is FROM scratch (pure YAML manifests/metadata, no compiled binaries), so its content is identical regardless of build architecture. The build matrix was rebuilding and pushing it on both the amd64 and arm64 legs to the same shared tag, which catalog-build's opm index add then pulled back non-deterministically (opm always pulls the bundle from the registry, never the local image). Build and push the bundle exactly once in a new, non-matrixed job. Both matrix legs' catalog-build now read that already-published, stable tag instead of racing to write it -- via `make -o bundle-push`, which skips catalog-build's phony bundle-push prerequisite. Verified locally end-to-end: real bundle build (FROM scratch, no RUN steps), real opm index add against a local registry, real arm64 catalog build. Assisted-by: Claude Code
polasudo
force-pushed
the
feat/multi-arch-operator-build
branch
from
July 17, 2026 14:48
b111709 to
2bb081f
Compare
Fortune-Ndlovu
left a comment
Fortune-Ndlovu
left a comment
Member
There was a problem hiding this comment.
The new workflow_dispatch trigger (line 11) will be silently skipped when the latest commit didn't change relevant files, because tj-actions/changed-files still gates everything. Manual triggers typically mean 'build regardless.' Add || github.event_name == 'workflow_dispatch' to the if: on this line, line135, and line 267 (with parentheses on 267 to preserve the && precedence).
| bundle: | ||
| name: Build and push operator-bundle | ||
| needs: changes | ||
| if: needs.changes.outputs.any_changed == 'true' |
Member
There was a problem hiding this comment.
Suggested change
| if: needs.changes.outputs.any_changed == 'true' | |
| if: needs.changes.outputs.any_changed == 'true' || github.event_name == 'workflow_dispatch' |
| needs: | ||
| - changes | ||
| - bundle | ||
| if: needs.changes.outputs.any_changed == 'true' |
Member
There was a problem hiding this comment.
Suggested change
| if: needs.changes.outputs.any_changed == 'true' | |
| if: needs.changes.outputs.any_changed == 'true' || github.event_name == 'workflow_dispatch' |
| needs: | ||
| - changes | ||
| - build | ||
| if: always() && needs.changes.outputs.any_changed == 'true' && needs.build.result == 'success' |
Member
There was a problem hiding this comment.
Suggested change
| if: always() && needs.changes.outputs.any_changed == 'true' && needs.build.result == 'success' | |
| if: always() && (needs.changes.outputs.any_changed == 'true' || github.event_name == 'workflow_dispatch') && needs.build.result == 'success' |
Centralize image metadata in the changes job, drop unused skopeo and build markers, remove redundant per-step if guards, pass image name via env blocks, and log Quay cleanup HTTP status codes. Co-authored-by: Cursor <cursoragent@cursor.com>
Manual runs should always build even when changed-files reports no matching paths, matching Fortune's review suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ubuntu-24.04(amd64) andubuntu-24.04-arm(arm64)mergejob that creates multi-arch manifest lists usingdocker buildx imagetools createWhat changes
File:
.github/workflows/next-container-build.yamlThe single
next-buildjob is replaced with two jobs:build— matrix over[ubuntu-24.04, ubuntu-24.04-arm], each runner builds all 3 images for its native architecture using existing Makefile targets (image-build,bundle-build,catalog-buildwithPLATFORMoverride), then pushes with per-arch tag suffixes (e.g.,operator:next-amd64)merge— downloads build markers, creates multi-arch manifest lists viadocker buildx imagetools create, tags final images (e.g.,operator:next), and optionally cleans up per-arch tags via Quay APIWhy
The community hub image (
quay.io/rhdh-community/rhdh) already ships as a multi-arch manifest list (amd64 + arm64). The operator stack is the remaining gap blocking ARM64 RHDH deployments via the operator.No Makefile or Dockerfile changes are needed — the operator Dockerfile already uses
TARGETOS/TARGETARCHbuild args and bundle images useFROM scratch.Pattern
Follows the same proven multi-arch build pattern from the hub repo's
next-build-image.yamlworkflow (matrix runners → per-arch push → manifest merge).Test plan
docker buildx imagetools inspect quay.io/rhdh-community/operator:nextshows both architecturespodman pull --platform linux/arm64 quay.io/rhdh-community/operator:nextsucceeds🤖 Generated with Claude Code