Refactor BOM upload processing for better efficiency, correctness, and consistency #3357
Conversation
nscuro
force-pushed
the
refactor-bom-upload-processing
branch
from
46f1afe to
982ff89
Compare
Backport of DependencyTrack/hyades-apiserver#218 Signed-off-by: nscuro <[email protected]>
`InternalComponentIdentificationUtil#isInternalComponent` previously executed two database queries, and up to two `Pattern#compile` calls, *for each invocation*. The method is invoked for each component in a BOM (during BOM upload processing), or for *all* components in the portfolio (for `InternalComponentIdentificationTask`). The work of querying and compiling is now performed only once, such that all following calls become much more efficient. Signed-off-by: nscuro <[email protected]>
For BOMs with large dependency graphs, repeatedly iterating over all nodes when searching for a BOM ref becomes very expensive. Converting the graph into a `Map` structure allows for more efficient lookups. It also helps with decoupling the processing a bit more from CycloneDX's object model. Signed-off-by: nscuro <[email protected]>
The BOM processing logic relies on BOM refs being there, but not all BOM generators populate that field. Signed-off-by: nscuro <[email protected]>
…leanup Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
…omponent` Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Now surrounds MDC key-value pairs with brackets if any are present, and omits brackets when MDC is empty. Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Signed-off-by: nscuro <[email protected]>
Received insufficient user feedback so far, so for 4.11 users will need to manually enable this. Signed-off-by: nscuro <[email protected]>
nscuro
force-pushed
the
refactor-bom-upload-processing
branch
from
6dd9e54 to
7e21023
Compare
Signed-off-by: nscuro <[email protected]>
**note the logs are showing a special character before the date/timestamp Step 2
Step 3
Step 4
Step 5
|
|
Thanks for the feedback @melba-lopez! The Jetty error you've been seeing, I fixed some aspects in the frontend that could cause it:
Basically the frontend issued redundant requests, where some of them would be abandoned before the API server could send the full response. There may be more of those issues lurking in the frontend codebase that I wasn't able to spot yet. In Step 3, the BOM processing failed because at least one component did not have a name:
This is in line with the schema, where As we touched on in Slack, the solution to that would be covered by #3218. Invalid BOMs should be rejected immediately. For step 4, looks like the 6.9MB BOM was processed in just above 2sec, nice!
|
- Test 1 : Uploaded a 4.6 MB SBOM (v1.4)Bom uploaded successfully - Test 2: upload a malformed 9.8 MB SBOM (v1.4)Note that the toast notification did give a "bom upload successful" even though no components show up and the "name" is not there. ---> per your comment above would be covered by #3218
Test 3: Upload an 8.8MB SBOM (v1.4)
Another interesting issue came up where a value for a CVE was not able to be stored into the DB because of the 255 character limit.
Also seeing some interesting NPM/Java errors
Test 4 - Uploaded a 2.1MB SBOM (v1.5)BOM was uploaded successfully, but still seeing some of those errors from step 3 into step 4
|
As reported in DependencyTrack#3357 (comment) Signed-off-by: nscuro <[email protected]>
|
@melba-lopez I resolved the NPM issue in #3456. I have received some more feedback via Slack, and it doesn't look like people are running into any issues due to this change. However, I still want to play it safe, so I'll keep it behind a feature flag for now. We will encourage folks to enable it when upgrading, but they can also still fall back in case something about the change breaks their workflows. |
As reported in DependencyTrack#3357 (comment) Signed-off-by: nscuro <[email protected]> Signed-off-by: Mikael Carneholm <[email protected]>
Description
Note
This PR is largely a backport of DependencyTrack/hyades-apiserver#218 and later enhancements in hyades-apiserver. Huge thanks to @malice00, @mehab, @sahibamittal, and @VithikaS, who also contributed to this work.
Notable Changes
UPDATEs,DELETEs) in batches where possibleINSERTs not possible due to ORM limitationsAddressed Issue
Fixes #1724
Fixes #1905
Fixes #2131
Fixes #2315
Fixes #2519
Fixes #2955 (Fixed by @malice00 via DependencyTrack/hyades-apiserver#278)
Fixes #3201
Fixes #3354
Fixes #3371
Relates to #3234
Additional Details
How to test
A container image including these changes has been published to:
Here's an example
docker-compose.ymlfor local testing:Checklist
This PR introduces changes to the database model, and I have added corresponding update logicThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly