-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat: add relationships #6563
feat: add relationships #6563
Conversation
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
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.
@knqyf263 i left some comments.
Also some thoughts:
- Just as idea:
Add Relationship for sortingLibraries
:
root -> direct deps -> indirect deps:
or simple setroot
package first (root -> other deps)
e.g. -trivy/pkg/dependency/parser/java/pom/parse_test.go
Lines 679 to 718 in 3ddee97
want: []types.Library{ { ID: "com.example:hard:1.0.0", Name: "com.example:hard", Version: "1.0.0", License: "Apache 2.0", Relationship: types.RelationshipRoot, }, { ID: "org.example:example-api:2.0.0", Name: "org.example:example-api", Version: "2.0.0", License: "The Apache Software License, Version 2.0", Relationship: types.RelationshipIndirect, }, { ID: "org.example:example-dependency:1.2.3", Name: "org.example:example-dependency", Version: "1.2.3", Relationship: types.RelationshipDirect, Locations: types.Locations{ { StartLine: 33, EndLine: 37, }, }, }, { ID: "org.example:example-nested:3.3.4", Name: "org.example:example-nested", Version: "3.3.4", Relationship: types.RelationshipDirect, Locations: types.Locations{ { StartLine: 28, EndLine: 32, }, }, }, }, - Perhaps we want to mark aggregated package (i mean jar, package.json, etc.) as
RelationshipRoot
?
pkg/report/table/vulnerability.go
Outdated
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 think we need to update logic for files without direct dependencies.
But perhaps this is just another PR job.
Also vulnerability_test.go
doesn't use new fields:
trivy/pkg/report/table/vulnerability_test.go
Lines 173 to 178 in 3ddee97
{ | |
ID: "[email protected]", | |
Name: "node-fetch", | |
Version: "1.7.3", | |
Indirect: 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.
Also vulnerability_test.go doesn't use new fields:
Fixed a0667ed
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 think we need to update logic for files without direct dependencies.
Which files?
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 meant files like package-lock.json
v1 (there is info about child deps, but not about direct deps).
But I rechecked our logic. It looks like it fit perfectly without any changes.
But it would be nice to update the comment here:
trivy/pkg/report/table/vulnerability.go
Lines 317 to 329 in 3ddee97
// Direct dependencies cannot be identified in some package managers like "package-lock.json" v1, | |
// then the "Indirect" field can be always true. We try to guess direct dependencies in this case. | |
// A dependency with no parents must be a direct dependency. | |
// | |
// e.g. | |
// -> styled-components | |
// -> fbjs | |
// -> isomorphic-fetch | |
// -> node-fetch | |
// | |
// Even if `styled-components` is not marked as a direct dependency, it must be a direct dependency | |
// as it has no parents. Note that it doesn't mean `fbjs` is an indirect dependency. | |
ancestors[parent.ID] = struct{}{} |
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.
Done 4ea19d1
Signed-off-by: knqyf263 <[email protected]>
Sounds good. Added in 7a1c9a8
I'm not sure about it. The relationship currently represents a relationship in a project, and I feel like the relationships of these standalone packages are unknown. |
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
@DmitriyLewen I think I've reflected your comments. Please take another look. |
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.
LGTM.
Left 1 small comment.
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Signed-off-by: knqyf263 <[email protected]>
Description
This PR adds a new field,
Relationship
toPackage
and deprecatesIndirect
.We currently set
Indirect
to true when the relationship is unknown, but this PR stops doing that and usesRelationshipUnknown
.Also, root packages are treated as direct packages now, but with the introduction of
RelationshipRoot
, the hierarchical structure is now handled correctly; SBOM reflects this, and the relationship would be like Root package -> Direct packages -> Indirect packages.See #6092 for more details.
Related issues
Indirect
Boolean with Enum Field #6092Checklist