fix(sbom): keep the dependency graph intact when merged SBOMs share a package - #324
Closed
reyreavman wants to merge 3 commits into
Closed
reyreavman wants to merge 3 commits into
reyreavman wants to merge 3 commits into
Conversation
Deduplication dropped a component but left every dependsOn entry pointing at its bom-ref, so a package shared by two images produced a graph with dangling targets. Removed refs are now redirected to the surviving component, collapsed entries merged. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Reuse the package's existing ref rewriter so vulnerabilities, compositions, annotations and declarations follow the surviving component too, instead of a dependencies-only remap. Merging entries that collapse onto one subject stays as a small step after the rewrite. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Collaborator
Author
Verification
Review focus
Follow-up
|
Collaborator
Author
|
Superseded by #329 — closing. The approach changed. Here dangling #329 additionally covers a case this PR did not: when the only Kaiten card 70002611 is closed and the work continues in a new one, 70686804. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When two images being merged into one SBOM contain the same package, deduplication kept one component and dropped the other, but every reference to the dropped
bom-ref—dependsOn,provides,vulnerabilities[].affects— was left in place. The resulting document referenced components that were not in it. Reproduces whenever a shared package has an incoming edge — a library both images' binaries link against — and affectswerf build(base + imports + os-pm merge) and both--ispras-formatoutputs ofwerf sbom merge.What
bom-refreference to a deduplicated component — independencies,vulnerabilities,compositions,annotations,declarations— now names the component that survived deduplication.bom-ref; previously the entry and all its outgoing edges were dropped.dependsOnandprovidesunioned, first occurrence ordered first; an edge that becomes self-referential is dropped.bom-refis removed without redirection; references to the removed one stay as they are, since there is no ref to point them at.--ispras-format container): on today'smainits dependency graph comes from the merged BOM, so it receives this fix; once fix(sbom): keep the dependency graph intact in container-format product SBOM #321 lands the graph is copied from each image before the merge and this code no longer touches it.componentsare not deduplicated, here or before this change; only top-level components are.Why
dedupComponentsByPURLrecorded removed components as a set of refs, which carries no way back to the component that replaced them.dropDependenciesByRefscould therefore only delete entries whose subject was removed, and had nothing to rewrite targets with — every incoming edge was left dangling, and the outgoing edges of the removed subject were lost rather than transferred. Recording removed → survivor makes both directions expressible, and the package already hasrewriteAllRefsfor exactly that map shape — the same rewriterensureUniqueBOMRefsuses — so the fix reuses it rather than remappingdependenciesalone.The ISPRAS checker does not validate that refs resolve, so this passes validation and surfaces only when a consumer walks the graph — for example to decide which components are reachable from a root.