-
Notifications
You must be signed in to change notification settings - Fork 74
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
Implement parsing of go list -deps -json ./... and maintain the rest #247
Conversation
break | ||
} | ||
|
||
project, err := modToProjectList(mod) | ||
if _, ok := err.(*NoVersionError); ok { | ||
if module, ok := mod["Module"].(map[string]interface{}); 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.
The easiest way I found to kind of tell if something was of go list -m all
style versus go list -deps -json
was this. I'm doing type checks on map keys, basically looking for existence. This lost us some of the nicer json parsing into a struct, but the amount of data we end up using is so minimal this didn't seem too wild. If a Module
key exists, I know I'm in go list -deps
town. If it doesn't, I look for Path
, which is go list -m all
town. If we find none of that, we just parse like it's a regular list (which we should drop in the future, there's not much of a reason to keep it around).
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.
Neato. The explanation above makes total sense now, while it's on my mind. It feels like adding some code comments around the "why" of these check might avoid head scratching down the road.
BTW, this looks very nice to me: 207
vs 49
$ go list -json -m all | ./nancy sleuth
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Summary ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━┫
┃ Audited Dependencies ┃ 207 ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━━┫
┃ Vulnerable Dependencies ┃ 0 ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━┛
(base) MBP-DRollo5:nancy bhamail$ go list -json -deps | ./nancy sleuth
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Summary ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━┫
┃ Audited Dependencies ┃ 49 ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━┫
┃ Vulnerable Dependencies ┃ 0 ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━┛
if _, ok := err.(*NoVersionError); ok { | ||
if module, ok := mod["Module"].(map[string]interface{}); ok { | ||
if version, ok := module["Version"].(string); ok { | ||
deps.Projects = append(deps.Projects, types.Projects{Version: version, Name: module["Path"].(string)}) |
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 was iffy on doing another type assertion check on module["Path"]
because I know if Version
is there then Path
is but if anyone wants me to go through the extra hoop let me know!
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'm less interested in the extra hoop, but some code comments about the "why" might illuminate.
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.
+1
I'm thinking we should change the docs to suggest the use of go list -json -deps
.
Would it be helpful if I pushed some doco changes?
cc @dnwe |
This should resolve #228 , it adds the ability to parse that type of output, and streamlines the code a tiny bit (at the expense of stronger typing, sadly)
For others edification:
go list -json -m all
includes every dependency you are using, on paper, and includes info on replace blocks, etc... so this is the recipego list -json -deps
tells go to let us know about ONLY the dependencies that end up in the final product, so this the cake, what got built with the recipe. This excludes test deps best I can tell, although they are reference in some of the json we end up with (only to the point where we see that a package has test dependencies)This pull request makes the following changes:
parse_test.go
parse.go
around how we parse json, using amap[string]interface{}
as what we originally parse in to, such that we can just check if keys existIt relates to the following issue #s:
go list -m json all
vulnerability checks swathes of dependencies that never end up in a binary #228cc @bhamail / @DarthHater
A nice bow to @SirMaster who got the groundwork laid, too!