-
Notifications
You must be signed in to change notification settings - Fork 579
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
add cataloger for rust crates from Cargo.lock files #345
Conversation
Signed-off-by: Weston Steimel <[email protected]>
Version: "0.3.9", | ||
Language: pkg.Rust, | ||
Type: pkg.RustPkg, | ||
Licenses: nil, |
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.
is licenses not a thing with Rust packages? if it is, it would be useful to capture that in these tests
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.
No, there isn't any information about licenses available in the Cargo lock file, at least not that I'm aware of
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.
This is an interesting question! I'm very new to Rust 😃 . From what I can tell, @westonsteimel is right about licenses not being stored in cargo.lock files. It looks like they are stored in cargo.toml files (akin to a package.json
file), but that'd be a different parser than what's tackled here. We could handle that down the road.
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.
Yes, you can also use the cargo metadata command to output json with everything about all crates and dependencies in the workspace, but that would require executing a command
|
||
import "github.com/anchore/syft/syft/pkg" | ||
|
||
type CargoMetadataPackage 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.
This would be great raw information to capture onto Package.Metadata
(and hint the type via the new MetadataType
you added onto Package.MetadataType
).
This also implies that we should migrate the struct to the syft/pkg
package and rename the struct to CargoPackageMetadata
to mirror the naming scheme for other metadata structs in the syft/pkg
package.
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.
Okay, I'll try to look at that a bit over the weekend. I was a bit unsure on this as I think I was mostly looking at the go modules one as an example (and the python poetry file parser for the toml bits) and it didn't have a metadata component defined under pkg.
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.
no problem! Taking a look at the poetry metadata we could have done the same thing (moved it up to pkg/
and expose it via Package.Metadata
) but it seems like we didn't (I admit we're being somewhat inconsistent here).
We can also merge this PR as is without capturing the extra cargo metadata onto Package.Metadata
--we can always do that in a follow up PR if we find we really need it. Let us know which direction you want to go in, either is OK 👍
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.
So I had a try at this in another branch westonsteimel@8b620de, but I'm running into issues getting the tests to pass. First the unit tests passed, but the integration tests did not, and I was able to figure out that I needed to increment and generate a new json schema file (1.0.3). I did that and the integration tests now pass, but the TestJsonDirsPresenter
and TestJsonImgsPresenter
unit tests fail because they expect json schema file version 1.0.2 and now get 1.0.3, and I haven't yet figured out how to resolve that one.
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.
awesome! I cherry picked that commit over and made a couple minor adjustments:
- We have snapshots of expected output for the json presenter tests. We use test flags to update the test snapshots:
go test ./syft/presenters/json -update
and then manually ensure the snapshots are what we expect before committing (I did this already, so no need to update) - I updated the
MetadataType
to reflect where the information was parsed from (a cargo package section)
Signed-off-by: Weston Steimel <[email protected]>
…metadata type Signed-off-by: Alex Goodman <[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.
Solid work, thanks for the contribution! 🚀
congrats @westonsteimel ! Excellent work |
Thanks everyone! |
add cataloger for rust crates from Cargo.lock files
Fixes #338
Signed-off-by: Weston Steimel [email protected]