[ISV-5221][ISV-5222] Add a new step into the “build-index-image” to build and push index sbom. #1513
Conversation
haripate
changed the title
|
/ok-to-test |
haripate
force-pushed
the
ISV-5221
branch
2 times, most recently
from
aab379f to
6e77420
Compare
|
/ok-to-test |
There was a problem hiding this comment.
Looks good now, tested in https://github.com/chmeliik/multi-arch-konflux-sample/pull/4/files and does what it should 👍
Could you address the failing checks?
|
/retest |
|
/ok-to-test |
|
/ok-to-test |
|
/ok-to-test |
|
/retest |
|
/ok-to-test |
… task. Signed-off-by: haripate <[email protected]>
|
/ok-to-test |
1 similar comment
|
/ok-to-test |
|
|
||
| SBOM_RESULT_FILE="/index-build-data/sbom-results.json" | ||
| if [ ! -f "$SBOM_RESULT_FILE" ]; then | ||
| echo "The sbom_results.json file does not exists. Skipping the SBOM upload..." |
There was a problem hiding this comment.
Do we need something like touch "$(results.SBOM_BLOB_URL.path)" here?
There was a problem hiding this comment.
Ah good catch. But I don't think it's actually possible to reach this line, @haripate maybe we can remove the if [ ! -f "$MANIFEST_DATA_FILE" ] and if [ ! -f "$SBOM_RESULT_FILE" ] conditions?
There was a problem hiding this comment.
@chmeliik, could it reach here if ALWAYS_BUILD_INDEX is true and only one Image Manifest was provided?
There was a problem hiding this comment.
Yup, good point. I forgot this task is allowed not to create an image index
There was a problem hiding this comment.
I think that without the touch "$(results.SBOM_BLOB_URL.path)", the SBOM_BLOB_URL result will be missing entirely, as opposed to empty if we add the touch. Considering no other tasks reference the SBOM_BLOB_URL, that seems fine at the moment.
There was a problem hiding this comment.
Yeah, seem reasonable.
We may need to revisit this in the future. If this Task does not produce an Image Index, then the SBOM_BLOB_URL should probably point to the SBOM of the Image Manifest.
TBH, I don't really understand why this Task has the ALWAYS_BUILD_INDEX parameter. If you don't want to build an Image Index, then maybe just don't include the Task? We're probably making things more difficult to use and manage with such parameters.
There was a problem hiding this comment.
I think the idea was that we would have just one build pipeline able to handle single-arch and multi-arch builds without any customization on the user's part. I don't fully remember, @arewm was that it?
There was a problem hiding this comment.
There were at least two reasons why I added this:
- Single arch pipelines should still be able to easily generate an Image Index if it becomes a requirement. This option can provide consistency with the internal build pipeline where every image generated is behind a manifest list. It also better supports operator-based installations where images generally shouldn't be referenced by image manifest.
- Adding the image index generation to the template improved the kustomization for the multi-arch builds (i.e. the image index and image manifest generating tasks can be next to each other).
Now that we support a matrix of size 1, we could potentially change the pipeline default to the multi-arch pipeline as these will build single-arch by default. We can then change the single-arch pipeline to not have the image index and use that only for builds requiring an Image Manifest.
|
FYI, the reversion of this PR is being proposed in #1565. |
This PR is related with two JIRA Issues:
https://issues.redhat.com/browse/ISV-5221
https://issues.redhat.com/browse/ISV-5222
Before you complete this pull request ...
Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.