-
Notifications
You must be signed in to change notification settings - Fork 8
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
SbomExtractor improvements #95
Conversation
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.
looks good, a few optional comments to consider, and you'll have to update the scripted test expectations to match the new behavior.
900959f
to
b1dad9c
Compare
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.
code is looking good, just needs the default value for 'include bom hashes' in the plugin and updated test expectations in the scripted tests
oh no why are the hashes different in ci... |
ohh they are selected based on the java version in org.cyclonedx.util.BomUtils: try {
digests.add(DigestUtils.getSha3_256Digest());
} catch (Exception | NoSuchMethodError e) { /* Not available in Java 8 and only available in later versions of DigestUtils */ } |
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.
One final tweak, then looks great to me!
includeBomTimestamp := false, | ||
includeBomToolVersion := true, | ||
includeBomHashes := true, | ||
enableBomSha3Hashes := true, |
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 wonder if we should default this to false
, to be more reproducible by default...
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 also thought about this but java 8 is mostly end of life anyway and the sha3 family will be pretty important in the future and should probably be enabled by default sooner than later. The gradle cyclonedx plugin also does not filter out hashes.
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.
Great improvement!
This PR adds the following attributes to sboms:
It also fixes an error caused by modules that are reported multiple times (cross builds, complex dependsOn structure and such)