-
Notifications
You must be signed in to change notification settings - Fork 26
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
Implement Syft2, 3 and CycloneDX 1.3 SBOM formats #312
Conversation
There are some TODOs in here for tweaking parts of the implementation. Would love feedback on approaching those! I've been staring at it all too long. Need some fresh eyes to help me tighten things up. |
Merging this is blocked on the release of the corresponding lifecycle feature -- since we'll need to use the feature to update the |
return sbomFormatByID(selected) | ||
} | ||
|
||
func sbomFormatByID(id sbom.FormatID) (sbomFormat, error) { |
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.
I've been looking at this function for a bit now, but I really can't think of a better way to do it than what you've laid out
@fg-j as discussed, it seems like the path of this PR is subject to change based off of our conversation with the Syft team at their working group meeting today. Do you want to pause on reviews until we have a clearer picture of what will happen in Syft first? |
I think we'll still want to merge this PR or something like it in the short-term so that we can unblock other issues, but we have a much higher degree of confidence that this debt we will be incurring can be paid down at some point in the medium-term future. That being said, I think the goals for what we should try to support here are to lock down the formats we currently support as outputs so that we can bump the Syft library forward to the latest versions without breaking our API contract. Specifically, we need support for:
We get Syft 3 and SPDX 2.2 for now with what is on the latest versions of Syft, but there is the concern that it might be broken at some point in the future. Should we duplicate support in our own package to make sure the API stays stable, or should we just keep up with checking that the schema versions don't change for those formats? |
@ryanmoran Given that syft has shown some interest in exposing APIs that'll make it easier to create SBOM format implementations (without copying in their internal packages), I'm inclined to do the minimum (i.e. add Syft 3.0.1) in the short term and wait to see if we can unload some tech debt before we add our own CycloneDX 1.4 and SPDX 2.2 implementations. I will update this PR to make CycloneDX 1.3 the default SBOM version generated when no |
@fg-j you mentioned that when new Syft versions come out we'll occasionally have to change our implementation like in 7335806. Once this is merged in, what kind of Syft changes do we need to be cognizant of in releases? I'd imagine it will differ from what we're on the lookout for now (backwards incompatible schema changes). |
@sophiewigmore – For CycloneDX 1.4 and SPDX 2.2, since this PR still relies on Syft's implementations of some schema versions, we'll still want to assert that the SBOMs generated with the syft Formats have the expected versions. We test this here for CycloneDX 1.4 and here for SPDX 2.2 When syft changes the underlying structs it uses to represent packages, we may need to change the code that converts from the syft representation to the JSON output representations. The entrypoint for this logic is in the If syft changes its internal representation of packages significantly, we may need to write bespoke converters, like |
Removing the blocked label because this implementation defaults to producing CycloneDX 1.3 and Syft 3.0.1 SBOMs, which are the same formats we currently generate. Merging this will not change the schema of buildpack generated SBOMs. No longer blocked on the release of the lifecycle change allowing schema versions. |
@fg-j I've gone through the code a few times and it looks good to me! It makes sense and I think covers all of our bases. @ryanmoran tagging you for a review as well before we merge it in to make sure it aligns with what you expected in #302 / to provide more technical expertise on the code structure. I've also tried it out with the defaults as you mentioned, and the output looks as expected. |
so that they can be used in syft.Encode() to generate pinned SBOM schema versions. Upgrades to syft 0.42.2 NOTE: upgrading syft required me to upgrade to go.1.18 on my machine We'll need to update our pipelines accordingly
I'm trying to get |
@ryanmoran The latest commit here should fix those test failures -- the |
Summary
This PR pulls in a lot of borrowed code from anchore/syft and CycloneDX/cyclonedx-go, because both of these libraries do not currently support multiple versions of their SBOM schema. I've had to yank sections of old versions of these codebases and embed them here.
As much as possible, I've attempted to document the sources of the code – and where things diverge from their original content.
It's my intent not to change the API of packit's SBOM package at all with this PR, given how experimental this featureset is. The hope is that if this is added to packit and this change to the lifecycle is released, Paketo buildpacks can pin to CycloneDX 1.3 SBOMs.
Supercedes #325 as well.
Partially addresses #302 , starting with requested / backward-incompatible SBOM formats. Ideally, I'd like to push these implementations of the Syft sbom.Format interface into syft itself. Our use of them in packit can serve as a proof of concept of the viability of supporting multiple SBOM formats.
Checklist