-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add image reference to SBOM #1629
Conversation
37483c9
to
26ec0e3
Compare
The build task with new changes has been tested as part of this pipelinerun https://github.com/Allda/devfile-sample-python-basic/pull/16/checks?check_run_id=33323099120 |
@brunoapimentel @rcerven Can you guys please review this PR. Thanks |
/ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you split this into two commits?
- Stop injecting SBOM into image
- Add image reference to SBOM
The first change has potential consequences, it shouldn't be "hidden" in a commit that does something else
I split the change into 2 commits as suggested. |
/ok-to-test |
/retest |
Looks like this will need a corresponding change in https://github.com/konflux-ci/e2e-tests |
I am not familiar with the e2e code base and I did just a quick check. Is this the place that needs to be remove? |
Me neither 😅 but yes, I think that's the place |
I opened a PR there with the change. Once the CI passes please review konflux-ci/e2e-tests#1465 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file not found in path /tmp/eimage3422397966/root/buildinfo/content_manifests/sbom-purl.json
This made me realize we can also delete the code that creates sbom-purl.json
. I believe that the file isn't used for anything anymore
/retest |
1 similar comment
/retest |
@chmeliik The e2e test is green now. Can you do a last round of review and merge if all looks good? Thanks |
6c56baf
to
39d172c
Compare
@chmeliik I also removed the purl script from the tekton task and here is another PR that removes the script itself konflux-ci/build-tasks-dockerfiles#193 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We can keep the version bump after all, it doesn't really hurt anything. Thanks for adding the "changelog" to the README
So do you want to bump a patch version to EDIT: Never mind.. I didn't realize I never reverted it back to |
/retest |
2 similar comments
/retest |
/retest |
The SBOM was stored into a container image as part of a build step. This feature is however obsolete and not needed anymore. From now on a containers won't have an SBOM stored inside of /root/buildinfo/content_manifests/ directory. JIRA: ISV-5411 Signed-off-by: Ales Raszka <[email protected]>
The SBOM generated by the buildah task now contains the reference to the image itself. The new script supports both spdx and cyclonedx format. JIRA: ISV-5411 Signed-off-by: Ales Raszka <[email protected]>
The script is no longer needed since we don't use its output anymore. Signed-off-by: Ales Raszka <[email protected]>
The SBOM generated by the buildah task now contains the reference to the image itself. The new script supports both spdx and cyclonedx format.
In order to inject the image reference to the SBOM steps were rearanged to push first and then generate SBOM. The code that stored the sbom into image itself was removed as not used anymore.
JIRA: ISV-5411
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.