-
Notifications
You must be signed in to change notification settings - Fork 40
chore: merge SBOM packages and remove old SBOM's #583
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
base: develop
Are you sure you want to change the base?
Conversation
e964ba4 to
d289cc8
Compare
d289cc8 to
84d0cca
Compare
|
nit: this seems less like a chore and more like a feature |
| # Install SBOM subpackages for each installed RPM (only for EROFS images) | ||
| if [[ "${EROFS_ROOT_PARTITION}" == "yes" ]]; then |
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.
This restriction doesn't make much sense as build system policy - "we will only produce SBOM artifacts if you're using EROFS".
Options:
- Drop it and do it unconditionally.
- Add a different image feature flag that's related to SBOM.
- Compress the SBOM files on disk, if that gives enough space?
- Emit the build artifacts always, and only add to the image if we're using EROFS?
Not sure which is best, assuming you can't just always include SBOM artifacts.
| if rpm -q --root "${ROOT_MOUNT}" "${sbom_rpm}" >/dev/null 2>&1; then | ||
| SBOM_RPMS+=("${sbom_rpm}") | ||
| else |
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.
This is to handle the case where someone adds foo-sbom to their variant's package list, so we've installed it? That seems pretty weird.
| if [[ -n "${sbom_file}" ]]; then | ||
| if rpm -iv --ignorearch --root "${ROOT_MOUNT}" "${sbom_file}" >/dev/null 2>&1; then | ||
| SBOM_RPMS+=("${sbom_rpm}") | ||
| fi |
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.
Need to fix your editor settings to not insert a mix of tabs/spaces. Please make sure this follows existing whitespace conventions.
| sbom_file=$(find "${EXTERNAL_KITS_PATH}" -name "${sbom_rpm}*.rpm" -print -quit) | ||
| fi | ||
| if [[ -n "${sbom_file}" ]]; then | ||
| if rpm -iv --ignorearch --root "${ROOT_MOUNT}" "${sbom_file}" >/dev/null 2>&1; then |
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.
We shouldn't install packages only to turn around and erase them. We can rpm -iv them into a separate directory, not the final root.
| # Check if SBOM package file exists in local packages or external kits | ||
| sbom_file=$(find "${PACKAGE_DIR}" -maxdepth 1 -name "${sbom_rpm}*.rpm" -print -quit) | ||
| if [[ -z "${sbom_file}" && -n "${EXTERNAL_KITS_PATH}" ]]; then | ||
| sbom_file=$(find "${EXTERNAL_KITS_PATH}" -name "${sbom_rpm}*.rpm" -print -quit) |
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.
This seems pretty hacky and I think we can do better:
- Add an "sbom" subpackage to
metadata.spec. - Have it
Provides: %{_cross_os}image-metadata(sbom)(andRequires: %{name}). - Copy rpmdb from
${ROOT_MOUNT}into new temp dir${SBOM_MOUNT}. - Install
metadata-sbominto${SBOM_MOUNT}viarpm -iv. - For each of the RPM SBOM packages, add
Supplements: (%{_cross_os}image-metadata(sbom) if %{name}).
That should give you a new temporary directory with only the SBOM files installed, which you can then concatenate together.
Issue number: #584
Closes #584
Description of changes:
Use
sbomtoolto merge SBOM packages into a single file, for eachspdxandcyclonedxformats. Remove all of the prior SBOM packages.Testing done:
/usr/share/sbomsdirectory but had ausr/share/bottlerocket/sbom/directory withimage-spdx.jsonandimage-cyclonedx.jsonfiles.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.