ROX-26026: Introduce registry to Scanner build matrix #13695
Merged
ROX-26026: Introduce registry to Scanner build matrix #13695
registry to Scanner build matrix #13695Conversation
|
Skipping CI for Draft Pull Request. |
registry to Scanner build matrix
Contributor
|
Images are ready for the commit at 6adbae7. To use with deploy scripts, first |
738a083 to
17abb05
Compare
Contributor
Author
|
/test ? |
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Author
|
/retest |
For easier testing changes in PRs.
I gave lengthy explanation in one of commits in #13694 Referring to that if you need more info.
because I want to extend the build job's matrix with an extra attribute that' won't exist in the pre-build job.
for consistency with others.
This would double execution of job steps. Can we afford that?
Since the idea of this overall change is to disable GHA builds into quay.io/rhacs-eng/ for releases, we can simply switch to scan GHA-built images in quay.io/stackrox-io/ which should be there (at least we don't have plans to migrate them to Konflux at this point). Some nerdy extra context: In #13694, the similar change for `.github/workflows/build.yaml`, I could not do the same thing because of a situation with the `stackrox-operator` image: the one is only built into quay.io/rhacs-eng and not built into quay.io/stackrox-io. Therefore I had to introduce a matrix there for the `scan-images-with-roxctl` job. I was musing whether I should introduce the matrix here, in `.github/workflows/scanner-build.yaml`, as well for this `scan-images-with-roxctl` job. The benefit is consistency of workflows. The downside is more complexity to this workflow.
Here's a commit that makes `scan-images-with-roxctl` matrixized. If we like it I keep it. Otherwise, I'll just drop it and we'll stay with only checking quay.io/stackrox-io/ images.
17abb05 to
c45ce21
Compare
Contributor
Author
|
/retest |
RTann
reviewed
Jan 6, 2025
RTann
approved these changes
Jan 6, 2025
Contributor
RTann
left a comment
There was a problem hiding this comment.
thanks for doing this LGTM
tommartensen
approved these changes
Jan 7, 2025
Contributor
tommartensen
left a comment
There was a problem hiding this comment.
LGTM pending one typo fix
|
@msugakov: The following test 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. |
ajheflin
pushed a commit
that referenced
this pull request
Jun 24, 2025
msugakov
added a commit
that referenced
this pull request
Jun 25, 2025
msugakov
added a commit
that referenced
this pull request
Jun 25, 2025
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
4 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.
Description
Similar to #13694 the goal here is to make it possible to disable pushes to
quay.io/rhacs-eng/for release builds (which will be driven by Konflux).Unlike #13694 where the
.github/workflows/build.yamlworkflow is defined through branding, the.github/workflows/scanner-build.yamldoesn't have a notion of branding and so I felt introducing one would be too artificial given that the Scanner V4 doesn't have any conditional on the branding. Therefore, I introduce the "registry"quay.io/stackrox-io/quay.io/rhacs-engas the matrix parameter.This change can be reviewed by commits or in its eventual state.
Request for reviewers: an extra dimension for
build-and-push-scannerandpush-scanner-manifestswill lead to a double execution of some steps which means more GitHub minutes. Given that these will run in parallel, that we don't pay for these minutes, and that these jobs seem to be quick enough (~5 minutes), I hope the impact is ok to take. Please take a critical look let know if you won't be ok with that.Also note that this change will require adjusting required jobs for PR since the one set up earlier for Scanner isn't valid anymore. The effect is that everyone will have to rebase their PRs. That's ok we did such things before.
User-facing documentation
Testing and quality
Automated testing
No contributions to the automated testing.
How I validated my change