Add dotnet deps parsing #258
Conversation
Signed-off-by: Benji Visser <[email protected]>
|
|
|
Hello @noqcks I will check your PR and write to you. |
DmitriyLewen
left a comment
DmitriyLewen
left a comment
There was a problem hiding this comment.
Hello @noqcks
Left some comments. Take a look, when you have time, please
pkg/dotnet/core_deps/parse.go
Outdated
| if !strings.EqualFold(lib.Type, "package") { | ||
| var deps []types.Dependency | ||
| for pkgNameVersion, target := range depsFile.Targets[depsFile.RuntimeTarget.Name] { | ||
| library, ok := depsFile.Libraries[pkgNameVersion] |
There was a problem hiding this comment.
Can we discuss about this:
It looks like we can take package name and version from pkgNameVersion.
Do we need to check "Libraries"?
Is this necessary for backwards compatibility?
There was a problem hiding this comment.
I think I did this initially because I wanted to keep track of whether a Library was a project or not and then be able to set the CycloneDX componentType in response, but I don't think go-dep-parser keeps track of this? I don't see any field on the Library struct that would allow us to set something resembling componentType
There was a problem hiding this comment.
So, in this case, we don't need to check Libraries at all
There was a problem hiding this comment.
I don't think go-dep-parser keeps track of this
We set componentType in Trivy.
I wanted to keep track of whether a Library was a project or not
We currently take only packages -
For project we use
application type with filepath as name:
➜ trivy -d fs -f cyclonedx pkg/dotnet/core_deps/testdata/ExampleApp1.deps.json | jq '.components[] | select(.type == "application")'
...
{
"bom-ref": "908ce44b-3644-44ce-b282-8f0d44566e20",
"type": "application",
"name": "ExampleApp1.deps.json",
"properties": [
{
"name": "aquasecurity:trivy:Class",
"value": "lang-pkgs"
},
{
"name": "aquasecurity:trivy:Type",
"value": "dotnet-core"
}
]
}
pkg/dotnet/core_deps/parse_test.go
Outdated
| { | ||
| file: "testdata/MyExample.deps.json", | ||
| wantLibs: []types.Library{ | ||
| {ID: "[email protected]", Name: "MyWebApp", Version: "1.0.0", Locations: []types.Location{{StartLine: 148, EndLine: 152}}}, |
There was a problem hiding this comment.
Is there reason to add project to result?
We didn't include this earlier:
There was a problem hiding this comment.
yes, because otherwise you can't extract a full and complete dependency tree if you need to parse the SBOM
There was a problem hiding this comment.
This makes sense for SBOM, but can be confusing when used by default (like json format).
We save project in dependencies. This can be confusing.
Perhaps we need to add new type or field for Library for project name/version. But I think it is change for another PR.
We use application componentType for SBOM - #258 (comment)
Previously this was enough.
Signed-off-by: Benji Visser <[email protected]>
Signed-off-by: Benji Visser <[email protected]>
|
@DmitriyLewen ready for another review |
DmitriyLewen
left a comment
DmitriyLewen
left a comment
There was a problem hiding this comment.
I left 2 small comment. Take a look, please.
Also i understood that MyExample.deps.json already contains dependencies.
Do we need new test file or will MyExample.deps.json be enough?
| return &Parser{} | ||
| } | ||
|
|
||
| func packageID(name, version string) string { |
There was a problem hiding this comment.
| func packageID(name, version string) string { | |
| // .NET uses `<name>/<version>` ID format: | |
| // https://github.com/dotnet/cli/blob/f0ee1e44114f161ce9268103d18ce32d2a995d0f/Documentation/specs/runtime-configuration-file.md?plain=1#L87 | |
| func packageID(name, version string) string { |
| } | ||
|
|
||
| return libraries, nil, nil | ||
| return libraries, deps, nil |
There was a problem hiding this comment.
Libraries and Dependencies can be sorted.
| return libraries, deps, nil | |
| sort.Sort(types.Libraries(libs)) | |
| sort.Sort(types.Dependencies(deps)) | |
| return libraries, deps, nil |
After these changes you can remove sorting from tests.
3 participants
Fixes: #257
cc: @nikpivkin @DmitriyLewen