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

Add a script for sbom enrichment #181

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Conversation

Allda
Copy link
Contributor

@Allda Allda commented Nov 13, 2024

The script is responsible for expanding a image sbom by adding a reference pointing to the image into a SBOM content.

The script recognize an input format and adds necessary data into the list of components or packages. The script also sets a name of the SBOM based on a pullspec.

JIRA: ISV-5411, ISV-5320

@Allda Allda force-pushed the ISV-5411 branch 3 times, most recently from 4d2bbe1 to fc72b1d Compare November 13, 2024 12:30
@Allda
Copy link
Contributor Author

Allda commented Nov 13, 2024

@chmeliik Hey, this is the prerequisite of ISV-5411. Once this is merged I'll continue on the buildah task and remove embedded sbom in image as we agreed.
Please assign any reviewers to this PR based on your review procedure. Thanks!!

Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, some minor concerns

@chmeliik
Copy link
Contributor

/ok-to-test

@midnightercz
Copy link

I don't think approach for spdx is correct. Every spdx document on the input (produced either by cachi2 or syft) will have "virtual" or "wrapper "package (I don't have better name for it) in relationship ROOT DESCRIBES SPDXRef-VirtualPackage and then other packages representing real packages are in relation SPDXRef-VirtualPackage CONTAINS SPDXRef-Package-A. Build dependencies (builder images) are in relation SPDXRef-Builder-Image1 BUILD_TOOL_OF SPDXRef-VirtualPackage.
So this virtual package represent the source which was used to created the sbom (container, directory, file,...). You can see here: https://github.com/konflux-ci/architecture/pull/213/files#diff-71b891db9bf4d748c67ae1b5779e6d57ad3fc384f672fae94b3b2627d8770679R149 how syft names it.
And if I undestand this PR correctly, you basically want to change the SBOM document to express that source of the sbom document is not some VirtualPackage (or in case of syft generated sboms SPDXRef-DocumentRoot-Directory-

-) but it's specific container with name and digest and purl and other attributes.
If that's correct assumption I think you need to replace existing "VirtualPackage" with SPDXRef-Image instead of adding new package to the list of packages.
Btw I tried to run syft convert on cdx sbom with metadata.component to see how syft converts it so we can maybe get some hints there, but it doesn't convert it at all

@chmeliik
Copy link
Contributor

Hypothetically, I can imagine a situation where an SPDX SBOM DESCRIBES two or more packages.

Using cachi2 as an example (this is probably not what the SBOMs will actually look like, just for illustration):

input: [{"type": "pip"}, {"type": "npm"}]


cachi2 SPDX SBOM:

                ROOT
             /        \
            /          \
        DESCRIBES   DESCRIBES
          /              \
         /                \
<pip main package>  <npm main package>
        |                  |
        |                  |
     CONTAINS           CONTAINS
        |                  |
        |                  |
    <pip deps>         <npm deps>


SBOM after enrichment?

                ROOT
                 |
                 |
              DESCRIBES
                 |
                 |
             <container>
             /        \
            /          \
        CONTAINS    CONTAINS
          /              \
         /                \
<pip main package>  <npm main package>
        |                  |
        |                  |
     CONTAINS           CONTAINS
        |                  |
        |                  |
    <pip deps>         <npm deps>

@chmeliik
Copy link
Contributor

The most general handling could look something like this:

  • for all ROOT DESCRIBES some_pkg
    • if some_pkg doesn't look like a "virtual" element (e.g. has a name and/or version),
      • replace ROOT DESCRIBES some_pkg with container CONTAINS some_pkg
    • else
      • remove ROOT DESCRIBES some_pkg
      • for all remaining relations of some_pkg, replace some_pkg with container
  • add ROOT DESCRIBES container

Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

One CI thing, otherwise LGTM

@Allda Allda force-pushed the ISV-5411 branch 2 times, most recently from e42c4a1 to a642238 Compare November 20, 2024 14:21
@chmeliik
Copy link
Contributor

The CI is failing with

/home/runner/work/_temp/3426a099-da9e-4189-9ab2-8368e6cce995.sh: line 5: -: command not found

indentation problem I think

The script is responsible for expanding a image sbom by adding a
reference pointing to the image into a SBOM content.

The script recognize an input format and adds necessary data into the
list of components or packages. The script also sets a name of the SBOM
based on a pullspec.

JIRA: ISV-5411, ISV-5320

Signed-off-by: Ales Raszka <[email protected]>
Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@chmeliik
Copy link
Contributor

Giving the team some time for review as well, will merge soon if no other comments

@chmeliik
Copy link
Contributor

The pending Konflux check is a false positive, this part of the code has no Konflux pipeline. Merging

@chmeliik chmeliik merged commit 377a181 into konflux-ci:main Nov 21, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants