Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add initial support for generating SPDX SBOM documents (COMPOSER-2274) #1818

Merged
merged 10 commits into from
Sep 18, 2024
Merged

Add initial support for generating SPDX SBOM documents (COMPOSER-2274) #1818

merged 10 commits into from
Sep 18, 2024

Conversation

thozza
Copy link
Member

@thozza thozza commented Jul 11, 2024 •
edited
Loading

This PR adds:

  • An implementation of standard-agnostic model for representing entities that are usually covered by SBOM documents. For now, these are software packages.
  • A simple SPDX v2.3 model implementation with serialization to JSON (dict) format.
  • Helper functions for converting DNF4 package set -> standard agnostic model -> SPDX model.
  • osbuild-depsolve-dnf extension to return SPDX JSON document for depsolved package set, if requested.
  • A new org.osbuild.dnf4.sbom.spdx stage, which can generate an SPDX JSON document for the package set installed in a filesystem tree (of the current pipeline or one passed as an input).

I considered using spdx_tools Python package, instead of implementing the model on my own. However, there were good reasons to not use it:

  1. The package is not in RHEL-9 and adding it would require updating numerous other Python packages in RHEL-9.
  2. The required (latest) version of it is not available on Fedora 39 and updating it would require also updating numerous other packages.
  3. The added benefit of SPDX document validation is not really usable, because license strings of some packages in Fedora can't be parsed and transformed to SPDX compliant form. I decided that using license strings from RPMs as they are specified is better than not specifying the package license at all in the SPDX SBOM document.

Open questions:

  1. Remove the sbom stage from the PR, since it is not needed for adding the support for SBOMs into osbuild/images and osbuild/osbuild-composer?
  2. Not sure if the depsolve API version should be bumped in the SPEC file, because all the changes are backward compatible (no SBOM is returned, unless explicitly requested)?

@thozza thozza marked this pull request as draft July 11, 2024 15:34
@thozza thozza changed the title Add initial support for generating SPDX SBOM documents Add initial support for generating SPDX SBOM documents (COMPOSER-2274) Jul 11, 2024
@thozza thozza force-pushed the sbom-poc branch 11 times, most recently from 5c7a37e to c76616a Compare July 12, 2024 15:03
@achilleas-k achilleas-k self-requested a review July 16, 2024 15:14
@thozza thozza force-pushed the sbom-poc branch 3 times, most recently from 097e707 to e24a5b4 Compare July 17, 2024 11:36
mvo5
mvo5 reviewed Jul 29, 2024
View reviewed changes
osbuild/util/bom/model.py Outdated Show resolved Hide resolved
achilleas-k
achilleas-k previously approved these changes Aug 5, 2024
View reviewed changes
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this must have been A LOT of work.
It all looks great to me.

I had a tiny debate with myself about whether reading the root tree was the best approach vs:

  • Having a stage that takes a giant list of RPM metadata objects to generate the SBOM without reading the tree.
  • Getting the SBOM straight from the depsolver and just feeding it into a file write stage.

But at the end of the day I think the approach in this PR is the more correct one. It guarantees that the generated document is created from the rpm db of the tree and not an indirect representation of it that might diverge due to bugs.

@mvo5 mvo5 self-requested a review August 27, 2024 07:40
@thozza thozza force-pushed the sbom-poc branch 5 times, most recently from 9a80483 to eea7bfa Compare August 29, 2024 15:36
@achilleas-k achilleas-k self-requested a review September 3, 2024 09:35
@thozza
Copy link
Member Author

thozza commented Sep 16, 2024

There's another issue with running the enhanced osbuild-depsolve-dnf on RHEL-8.10 😕

Sep 16 14:52:33 ip-10-31-19-53.us-east-1.aws.redhat.com osbuild-worker[23386]: AttributeError: '_hawkey.Reldep' object has no attribute 'name'
Sep 16 14:52:33 ip-10-31-19-53.us-east-1.aws.redhat.com osbuild-worker[23368]: time="2024-09-16T14:52:33Z" level=error msg="Unhandled dnf-json error in depsolve job: running osbuild-depsolve-dnf failed:\nDNF error occurred: AttributeError: '_hawkey.Reldep' object has no attribute 'name'" channel=local jobId=49322bc4-2470-4ed7-b902-1cca02d5cddd

achilleas-k
achilleas-k previously approved these changes Sep 16, 2024
View reviewed changes
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried very hard but couldn't come up with any good nitpicks.

supakeen
supakeen reviewed Sep 16, 2024
View reviewed changes
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial review.

.github/workflows/test.yml Outdated Show resolved Hide resolved
osbuild/solver/dnf.py Outdated Show resolved Hide resolved
osbuild/testutil/dnf4.py Show resolved Hide resolved
supakeen
supakeen previously approved these changes Sep 16, 2024
View reviewed changes
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a herculean effort I have added a few minor nits but feel free to ignore them.

kingsleyzissou
kingsleyzissou previously approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@kingsleyzissou kingsleyzissou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And my axe!

@thozza thozza mentioned this pull request Sep 17, 2024
Merged
achilleas-k
achilleas-k previously approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but linter is sad

@thozza
testutil: add helper function for depsolving pkgset with DNF4
e7c06e1 
This will be useful for testing SBOM implementations.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Tests: allow site packages in tox to test code depending on 'dnf'
94aab66 
'dnf' Python package can't be installed using pip in the tox
environment. In order to test the code which uses it, we need to use the
system version. Our testing environment uses Fedora as the system,
therefore we can reasonably use the system version of 'dnf' only with
Python version which is on Fedora.

Enable site packages in tox for Python 3.12 when testing osbuild
internals.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Add initial SBOM library implementation
10a85f7 
Add implementation of standard-agnostic model for SBOM, and simple SPDX
v2.3 model. Also add convenience functions for converting DNF4 package
set to the standard-agnostic model and for converting it to SPDX model.

Cover the functionality with unit tests.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Test SPDX model implementation against spec JSON schema
309c489 
Verify the documents generated by the internal implementation of SPDX
v2.3 model against the upstream spec JSON schema.

The schema has been downloaded from:
https://github.com/spdx/spdx-spec/blob/development/v2.3.1/schemas/spdx-schema.json

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
osbuild/solver: add InvalidRequestError exception
c2732d8 
This will allow validating request arguments in the solver method in a
different way for dnf4 and dnf5 and raising an exception if needed.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Support SBOM for depsolving in osbuild-depsolve-dnf
de0a0da 
Extend osbuild-depsolve-dnf, to return JSON with SPDX SBOM that
corresponds to the depsolved package set, if it has been requested.
For now, only DNF4 is supported.

Cover the new functionality with unit test.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Add org.osbuild.dnf4.sbom.spdx stage
ef81459 
Add a new stage, which allows analyzing the installed packages in a
given filesystem tree using DNF4 API and generating an SPDX v2.3 SBOM
document for it.

One can provide the filesystem tree to be analyzed as a stage input. If
no input is provided, the stage will analyze the filesystem tree of the
current pipeline.

Add tests cases for both usage variants of the stage, as well as the
unit test for stage schema validation.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
CI: determine the libdnf python version from /osb/libdnf-python-version
37aae7e 
Instead of hard-coding the Python version that the installed
python3-dnf has been built against on the latest Fedora, read the
value from the osbuild-ci container. The container now has the version
written in /osb/libdnf-python-version.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Solver/dnf.py: simplify the code a bit
fbbb0c7 
Since the `with_sbom` variable was used only in a single place, we can
simplify the code (and remove one extra line of it) to just directly use
the if condition.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza
Util/SBOM: add compatibility layer for old lib Hawkey
8409dcf 
'_hawkey.Reldep' object has no attribute 'name' in the version shipped
on RHEL-8. Add code to handle this situation in case it happens.
Default to using named attributes if these are available.

Signed-off-by: Tomáš Hozza <[email protected]>
@supakeen supakeen enabled auto-merge (rebase) September 18, 2024 07:46
@supakeen supakeen merged commit 3df75de into osbuild:main Sep 18, 2024
48 checks passed
@thozza thozza deleted the sbom-poc branch September 18, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@achilleas-k achilleas-k achilleas-k approved these changes

@kingsleyzissou kingsleyzissou kingsleyzissou approved these changes

@supakeen supakeen supakeen approved these changes

@mvo5 mvo5 Awaiting requested review from mvo5

@ondrejbudai ondrejbudai Awaiting requested review from ondrejbudai

@schuellerf schuellerf Awaiting requested review from schuellerf

@croissanne croissanne Awaiting requested review from croissanne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thozza @mvo5 @achilleas-k @kingsleyzissou @supakeen