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.

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

Include the OS Information available in the SBOM model in the SPDX reports #3462

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

josegomezr
Copy link

Description

Include the OS Information available in the SBOM model in the SPDX reports

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have added unit tests that cover changed behavior
  • I have tested my code in common scenarios and confirmed there are no regressions
  • I have added comments to my code, particularly in hard-to-understand sections

TODO:

  • Understand the offset logic for {relationship,pkgCont}OffsetPerVersion

@popey
Copy link
Contributor

popey commented Nov 25, 2024

Thanks for the PR @josegomezr !

I gave it a quick test here, and it looks good from a functional point of view. I'll leave others for code review.

$ grep ">" diff.txt
                                                              >     },
                                                              >     {
                                                              >       "name": "debian",
                                                              >       "SPDXID": "SPDXRef-OperatingSystem-debian",
                                                              >       "versionInfo": "12",
                                                              >       "supplier": "NOASSERTION",
                                                              >       "downloadLocation": "NOASSERTION",
                                                              >       "filesAnalyzed": false,
                                                              >       "licenseConcluded": "NOASSERTION",
                                                              >       "licenseDeclared": "NOASSERTION",
                                                              >       "description": "Debian GNU/Linux 12 (bookworm)",
                                                              >       "primaryPackagePurpose": "OPERATING-SYSTEM"
                                                              >       "relationshipType": "CONTAINS"
                                                              >     },
                                                              >     {
                                                              >       "spdxElementId": "SPDXRef-DocumentRoot-Image-nextcloud"
                                                              >       "relatedSpdxElement": "SPDXRef-OperatingSystem-debian",

Add a new package reference describing the linux environment available
via `SBOM.Artifacts.LinuxDistribution`

When it's not found, it behaves as before this PR.

When found it'll interject the `OperatingSystem` reference between the
target and the packages to reflect:

Document DESCRIBES target
target CONTAINS OperatingSystem
OperatingSystem CONTAINS *Packages

Signed-off-by: Jose D. Gomez R <[email protected]>
@spiffcs
Copy link
Contributor

spiffcs commented Dec 3, 2024

Thanks @josegomezr! I'm taking a look at this now.

@spiffcs
Copy link
Contributor

spiffcs commented Dec 3, 2024

I kicked off the tests and CI so we have that green going into our livestream this week - I've added a needs-discussion label to this so we can talk about it on Thursday.

I think the team has varying opinions on adding distro information to the package list of other formats.

Opinion: If we were to take this change it would first be reflected in the core model of the syft-json and then propagated UP from a created syft package. The goal here is so that the other format model code treats the delivered SBOM as immutable and doesn't need to do any out of band package or relationship creation.

This keeps the behavior separated and easier to reason about as a developer on the project so one doesn't have to track every special case for every format model behavior and how it might mutate(additive, subtract, or modify) of core parts of sbom.SBOM.

Very much TY for the PR since this puts the ball in our court as far as how we want to address distro as a package going forward for the core part of syft/other formats!

Dev note: CLI tests are catching a schema validation error for the new package that we would have to track down if we decided to add distro as a package to the core syft model.

@josegomezr
Copy link
Author

Side note, CI fails with:

    spdx_json_schema_test.go:80:   - packages.203.primaryPackagePurpose: packages.203.primaryPackagePurpose must be one of the following: "OTHER", "INSTALL", "ARCHIVE", "FIRMWARE", "APPLICATION", "FRAMEWORK", "LIBRARY", "CONTAINER", "SOURCE", "DEVICE", "OPERATING_SYSTEM", "FILE"

But Section 7.24.1 of the SPDX v3 spec uses dash instead of underscore for OPERATING-SYSTEM:

@spiffcs
Copy link
Contributor

spiffcs commented Dec 3, 2024

Section 7.24.1 of the SPDX v3 spec

Ahh Thanks for the highlight here - Looks like we're still using the 2.3 schema found here for this test.

https://github.com/anchore/syft/blob/main/test/cli/spdx_json_schema_test.go

@josegomezr
Copy link
Author

The goal here is so that the other format model code treats the delivered SBOM as immutable and doesn't need to do any out of band package or relationship creation.

This definitely makes a lot of sense. Thanks for the info @spiffcs I'll be waiting for news on your side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

OS information missing in SPDX format SBOM for a container image
3 participants