-
Notifications
You must be signed in to change notification settings - Fork 405
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
Convert our SPDX SBOMs to spdx+json. #740
Conversation
Codecov Report
@@ Coverage Diff @@
## main #740 +/- ##
=======================================
Coverage 51.38% 51.38%
=======================================
Files 44 44
Lines 3295 3295
=======================================
Hits 1693 1693
Misses 1390 1390
Partials 212 212
Continue to review full report at Codecov.
|
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.
Nice Matt, just a small nit 👇
.github/workflows/sbom.yaml
Outdated
|
||
java -jar ./spdx-tools-2.2.7-jar-with-dependencies.jar Verify sbom.txt | ||
java -jar ./spdx-tools-2.2.7-jar-with-dependencies.jar Verify sbom.json |
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 jar has been deprecated for the new version of the tools. The new URL to download the new jar is:
https://github.com/spdx/tools-java/releases/download/v1.0.4/tools-java-1.0.4.zip
(the version is lower but the jar is newer)
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.
thanks!
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 was a big help, I think it caught a bunch of things we'd missed with the old tool 🙏
Name: bi.Main.Path, | ||
ID: mainPackageID, | ||
// TODO: PackageSupplier: "Organization: " + bs.Main.Path | ||
DownloadLocation: "https://" + bi.Main.Path, |
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 isn't very likely to work. Can we make this a proxy.golang.org URL? 🤔
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 used this before 🤔
I can TAL later, my tether time is up :)
Version = "SPDX-2.2" | ||
) | ||
|
||
type Document struct { |
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.
Nit since it's internal anyway but these types can be unexported
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 was trying to avoid touching this code too much. I was about to change, but remembered this is in internal/
so I'm sort of inclined to leave it. WDYT?
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.
Thanks for doing this! I agree this isn't any worse than what we had before, and the JSON seems a lot easier to consume. Anything else remaining can be cleaned up in a future PR.
I mapped all of the fields from what we had to the new struct.
cc @imjasonh @jonjohnsonjr @puerco