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

Ensure aggregate BOMs contain exact dependency hierarchies from each Maven module #306

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

knrc
Copy link
Contributor

@knrc knrc commented Mar 9, 2023

see #310 for more details

prior to this PR, the dependency tree would only include one of the dependency trees. After this PR is applied, the SBOM will contain multiple components for dependency_B, with the same purl but differing bom-refs, and each component will be included in a separate dependency tree.

@sonatype-lift
Copy link

sonatype-lift bot commented Mar 9, 2023

🛠 Lift Auto-fix

Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.1

# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/CycloneDX/cyclonedx-maven-plugin/306.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff

Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:

curl https://lift.sonatype.com/api/patch/github.com/CycloneDX/cyclonedx-maven-plugin/306.diff | git apply

Once you're satisfied, commit and push your changes in your project.

Footnotes

  1. You can preview the patch by opening the patch URL in the browser.

@hboutemy
Copy link
Contributor

we need a description of what this PR changed in terms of results: how is the new alternate dependency tree different (I suppose better :) ) from the previous one?

@knrc knrc changed the title Add support for alternate dependency trees for components Ensure aggregate BOMs contain all dependency hierarchies Mar 15, 2023
@knrc
Copy link
Contributor Author

knrc commented Mar 16, 2023

@hboutemy I created #310 to describe the issue, is there anything else needing to be done for this PR?

@hboutemy hboutemy changed the title Ensure aggregate BOMs contain all dependency hierarchies Ensure aggregate BOMs contain dependency hierarchies from each module Mar 16, 2023
knrc and others added 2 commits March 16, 2023 09:10
…y hierarchies

Signed-off-by: Kevin Conner <kev.conner@gmail.com>
Signed-off-by: Hervé Boutemy <hboutemy@apache.org>
@hboutemy hboutemy changed the title Ensure aggregate BOMs contain dependency hierarchies from each module Ensure aggregate BOMs contain dependency hierarchies from each Maven module Mar 18, 2023
@hboutemy hboutemy changed the title Ensure aggregate BOMs contain dependency hierarchies from each Maven module Ensure aggregate BOMs contain exact dependency hierarchies from each Maven module Mar 22, 2023
@stevespringett
Copy link
Member

If we were to stack rank the importance of data in an SBOM, it would be:

Rank
Inventory
Component identity
Dependency relationships
...

My issue with this approach is that data of lesser importance is taking priority over more important data in the BOM. We are sacrificing accurate inventory for the ability to have accurate dependency relationships. I don't know if it's worth the risk given that most systems will likely deduplicate the components anyway which will leave one of the branches in the dependency tree orphaned on import. Dependency-Track and likely many other systems will do this.

@knrc
Copy link
Contributor Author

knrc commented Mar 23, 2023

We are sacrificing accurate inventory for the ability to have accurate dependency relationships.

@stevespringett Can you explain why you think this is sacrificing accurate inventory? The set of components included in the SBOM remain the same, the only change is that there will be duplicates of some because of the need for a different bom-ref.

I don't know if it's worth the risk given that most systems will likely deduplicate the components anyway which will leave one of the branches in the dependency tree orphaned on import. Dependency-Track and likely many other systems will do this.

They would likely face the same issue when importing the individual SBOMs, so either way there would be issues needing to be addressed.

@stevespringett
Copy link
Member

the only change is that there will be duplicates of some because of the need for a different bom-ref.

One reading the BOM will conclude that there are multiple of the same component included for the same metadata component, which is not true.

By separating out the components into assemblies, the reader of the BOM will interpret this to mean the components are included in other components (maven modules) which in term are included in the same metadata component, which would be true statement.

They would likely face the same issue when importing the individual SBOMs, so either way there would be issues needing to be addressed.

Not sure about other systems, but users would not have any issues with Dependency-Track with this approach.

Now, if we used component assemblies, I know Dependency-Track can handle parent/child components, but I don't know if it will deduplicate if the same component is used across multiple parent components.

@knrc
Copy link
Contributor Author

knrc commented Mar 24, 2023

One reading the BOM will conclude that there are multiple of the same component included for the same metadata component, which is not true.

I'm not sure I would agree with that statement, if each of these projects were being packaged up you would end up with multiple instances of components with different dependencies so it's likely build dependent and not something we have sufficient information to reason about in the plugin.

By separating out the components into assemblies, the reader of the BOM will interpret this to mean the components are included in other components (maven modules) which in term are included in the same metadata component, which would be true statement.

The only way I can think of representing the above statement would be the exploded view through the assemblies, in which case de-duplication, as it is currently handled in the aggregate, would be wrong for both components and the dependency hierarchies (even if they were currently accurate). We would end up with an aggregated SBOM which was essentially nothing more than a concatenation of the individual SBOMs for each project. This is one of the approaches we discussed on the call resulting in the full, exploded dependency graph being represented.

They would likely face the same issue when importing the individual SBOMs, so either way there would be issues needing to be addressed.

Not sure about other systems, but users would not have any issues with Dependency-Track with this approach.

Now, if we used component assemblies, I know Dependency-Track can handle parent/child components, but I don't know if it will deduplicate if the same component is used across multiple parent components.

I know guac would handle the de-duplication of the component if processing individual SBOMs, but I'm not sure how they would handle the same component being referenced multiple times in the same SBOM. I believe they would have the same issue whether using the solution from this PR or assemblies, since they generate a map of bom-ref to component irrespective of whether the component is nested or top level. They would still hit the dependency graph issue since they currently assume the dependencies of a component in the graph will never change.

@knrc knrc mentioned this pull request Apr 4, 2023
@hboutemy hboutemy assigned knrc and unassigned knrc Apr 4, 2023
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.

None yet

3 participants