Generated a consolidated SBOM by DaveTryon · Pull Request #1126 · microsoft/sbom-tool (original) (raw)

This PR creates the full end-to-end experience with consolidation, using real SBOM as the basis for the local testing. It needs more unit tests and maybe some E2E tests, but this connects the pieces. Here are the key issues that it addresses:

1. The first issue was that we needed to get the correct set of packages, instead of the ones from the component-detection code. To do this, we set the IConfiguration.PackagesList property. This causes SbomPackagesProvider to replace CGScannedPackagesProvider as the source of the packages section of the generated SBOM.
2. The second issue was that the validation of packages with the _manifest folder contained in the drop were failing during consolidation. This was because the validation code uses the ManifestDirPath property to automatically ignore the contents of this folder, meaning that it needs to be set for each SBOM that we validate. We now set, then restore the property around each validation workflow. The code that sets the property is duplicated, but there was no obvious to avoid this, short of creating a new class, which seemed like overkill for the amount of code involved. It's easy to refactor if needed.
3. The third issue was that the consolidate SBOM was being written to the wrong location. Instead of going to the location specified by the ManifestDirPath, it was overwriting one of the input SBOMs. This took a while to figure out, but ultimately it boils down to the fact that the SbomConfigProvider was written with the assumption that ManifestDirPath was constant throughout the app's lifetime. The value set during validation was saved, then reused during generation. There are multiple ways we could address this, but I elected to add a ISbomConfigProvider that clears the cache between validation and generation. I also considered changing the `ISbomConfigProvider from a Singleton to a Transient, but I wanted to minimize the potential side effects. The new method only gets called by the consolidation workflow, so the side effects are constrained.

The rest of the changes are mainly about unit tests (including some refactoring of setting up the mocks) and adding an extension class for managing the MergeableContent. I also added code to clean up our temp directory after exiting. We can tweak that as needed.

This PR changes files in the API project. Does it change any of the API interfaces in any way? Please note that this includes the following types of changes:

• Changing the signature of an existing interface method
• Adding a new method to an existing interface
• Adding a required data member to a class that an existing interface method consumes

Because any of these changes can potentially break a downstream consumer with customized interface implementations, these changes need to be treated as breaking changes. Please do one of the following:

Option 1 - Publish this as a breaking change

1. Update the documentation to show the new functionality
2. Bump the major version in the next release
3. Be sure to highlight the breaking changes in the release notes

Option 2 - Refactor the changes to be non-breaking

1. Review this commit, which adds a new interface in a backward-compatible way
2. Refactor the change to follow this pattern so that existing interfaces are left completely intact
3. Bump the minor version in the next release

Choose a reason for hiding this comment

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

One comment, otherwise LGTM!

This PR changes files in the API project. Does it change any of the API interfaces in any way? Please note that this includes the following types of changes:

• Changing the signature of an existing interface method
• Adding a new method to an existing interface
• Adding a required data member to a class that an existing interface method consumes

Because any of these changes can potentially break a downstream consumer with customized interface implementations, these changes need to be treated as breaking changes. Please do one of the following:

Option 1 - Publish this as a breaking change

1. Update the documentation to show the new functionality
2. Bump the major version in the next release
3. Be sure to highlight the breaking changes in the release notes

Option 2 - Refactor the changes to be non-breaking

1. Review this commit, which adds a new interface in a backward-compatible way
2. Refactor the change to follow this pattern so that existing interfaces are left completely intact
3. Bump the minor version in the next release

DaveTryon deleted the DaveTryon/add-consolidation-generation branch

July 3, 2025 16:57

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})