ROX-31023: arm64 support for operator images#16915
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
General comments:
- Consider extracting the QEMU and Buildx setup into a shared step or composite action so you don’t need to repeat it across arches and jobs.
- The new push-operator-manifests job has a lot of overlap with your existing multi-arch manifest push logic—consider refactoring common parts into a reusable workflow or action.
- Since you now require TARGET_ARCH for local builds via GOARCH, add a default (e.g. defaulting to amd64) so 'make docker-build' still works outside CI without extra env vars.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the QEMU and Buildx setup into a shared step or composite action so you don’t need to repeat it across arches and jobs.
- The new push-operator-manifests job has a lot of overlap with your existing multi-arch manifest push logic—consider refactoring common parts into a reusable workflow or action.
- Since you now require TARGET_ARCH for local builds via GOARCH, add a default (e.g. defaulting to amd64) so 'make docker-build' still works outside CI without extra env vars.
## Individual Comments
### Comment 1
<location> `operator/Makefile:337` </location>
<code_context>
docker-build: build/Dockerfile.gen test smuggled-status-sh ## Build docker image with the operator.
DOCKER_BUILDKIT=1 BUILDKIT_PROGRESS=plain docker build \
-t ${IMG} \
+ --build-arg TARGET_ARCH=$(GOARCH) \
-f $< \
..
</code_context>
<issue_to_address>
**suggestion (bug_risk):** TARGET_ARCH build argument is passed from GOARCH.
Make sure GOARCH is always set or provide a default to prevent build failures.
</issue_to_address>
### Comment 2
<location> `.github/workflows/build.yaml:603` </location>
<code_context>
uses: docker/setup-qemu-action@v3
</code_context>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>
### Comment 3
<location> `.github/workflows/build.yaml:606` </location>
<code_context>
uses: docker/setup-buildx-action@v3
</code_context>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Images are ready for the commit at bff2128. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #16915 +/- ##
=======================================
Coverage 48.80% 48.80%
=======================================
Files 2707 2707
Lines 202201 202201
=======================================
+ Hits 98679 98683 +4
+ Misses 95752 95747 -5
- Partials 7770 7771 +1
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:
|
cec8939 to
2fb3433
Compare
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Blocking issues:
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/workflows/build.yaml:654` </location>
<code_context>
+ if [[ "${{ github.event_name }}" == "push" && "${{ github.ref_name }}" == "master" ]]; then
+ push_context="merge-to-master"
+ fi
+ push_operator_image_set "$push_context" "${{ env.ROX_PRODUCT_BRANDING }}" "${{ matrix.arch }}"
+
+ - name: Push bundle image
</code_context>
<issue_to_address>
**suggestion:** Custom push logic replaces previous make-based push for operator images.
Please verify that error handling and logging in the custom push function are sufficient, particularly for multi-arch pushes, and that any failures are clearly reported in CI.
Suggested implementation:
```
echo "Starting operator image push for context: $push_context, branding: ${{ env.ROX_PRODUCT_BRANDING }}, arch: ${{ matrix.arch }}"
set -o pipefail
if ! push_operator_image_set "$push_context" "${{ env.ROX_PRODUCT_BRANDING }}" "${{ matrix.arch }}"; then
echo "ERROR: push_operator_image_set failed for context: $push_context, branding: ${{ env.ROX_PRODUCT_BRANDING }}, arch: ${{ matrix.arch }}" >&2
exit 1
fi
echo "Operator image push completed successfully for context: $push_context, branding: ${{ env.ROX_PRODUCT_BRANDING }}, arch: ${{ matrix.arch }}"
```
- Ensure that the `push_operator_image_set` function in `./scripts/ci/lib.sh` itself logs errors and outputs relevant information for multi-arch pushes.
- If the function does not already print detailed error messages, update it to do so.
- If the function does not return a non-zero exit code on failure, update it to ensure CI can detect failures.
</issue_to_address>
### Comment 2
<location> `scripts/ci/lib.sh:324` </location>
<code_context>
fi
}
+push_operator_image_set() {
+ info "Pushing stackrox-operator image"
+
</code_context>
<issue_to_address>
**suggestion:** Function expects exactly three arguments; consider validating input types.
Currently, only the number of arguments is checked. Please add validation for argument values to ensure they are not empty or invalid.
</issue_to_address>
### Comment 3
<location> `.github/workflows/build.yaml:603` </location>
<code_context>
uses: docker/setup-qemu-action@v3
</code_context>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>
### Comment 4
<location> `.github/workflows/build.yaml:606` </location>
<code_context>
uses: docker/setup-buildx-action@v3
</code_context>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
porridge
left a comment
There was a problem hiding this comment.
Please replace the placeholders from PR description.
dcbf173 to
61dbc77
Compare
|
@porridge I don't understand yet where this zombie jobs is coming from
|
|
The stackrox/stackrox repo branch protection configuration specifies that a GH check of that name must pass for the change to be mergeable. I guess this needs to be relaxed for a moment while the name changes. |
Will contact automation team once this is PR is code-wise ready. |
a89ebe7 to
e5aaeb0
Compare
|
/ai-review |
For consistency reasons with the existing code this PR continues using tags in the image refs.
e5aaeb0 to
5951005
Compare
e0cc0be to
b4f11b2
Compare
porridge
left a comment
There was a problem hiding this comment.
Just a few nitpicks inline.
|
@mclasmeier: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |

Description
The GitHub Actions built operator images only work on amd64 currently.
Adding arm64 support would enable us to quickly test CI-built operator images locally also on arm64 machines.
User-facing documentation
No user-facing changes, no changes to production code, no tests.
How I validated my change
Set
TAG.Alternatively: can be deployed with roxie.