Implemented new CLI flag: --show-suppressed #966
Conversation
…ed/suppressed matches to be displayed in the table output format when that flag is provided. Signed-off-by: Vimal Patel <vimalpatel0611@gmail.com>
There was a problem hiding this comment.
I've had a cursory look through this change but I have a question: do these suppressed vulnerabilities need to be included in the JSON or just the table view (they are already included in the JSON)? If it's just the latter, maybe a better approach would be to not modify the data structure, but just print out both normal and suppressed vulns together?
|
I can't seem to find the Feature Request, for this. I know I cared ... and a search does not find that Feature Request. |
|
@freedom-isnotanarchy it's linked in the description and on the right in the Development section |
|
@kzantow the suppressed vulnerabilities should already be displayed in the JSON and template output formats. The question I had was do we want to also display the suppressed vulnerabilities in the just the table format, or do we want to attempt to display them in the rest of the other output formats as well (such as cyclonedx, sarif, etc.)? If it is just for the table format, I can proceed as you suggested and not modify Match data structure. Otherwise if we need to include suppressed vulnerabilities for each of the remaining formats that don't currently do that, I think it might be better to keep the modified Match data structure as opposed to passing in an additional parameter (containing the list of suppressed vulnerabilities) to the NewPresenter method for each of the output formats here: https://github.com/anchore/grype/blob/main/grype/presenter/presenter.go. |
|
|
@vimalpatel19 these are already included in JSON output, I'm pretty sure, though in a different spot, so I think we should just add these to the table output when the flag is set. 👍 |
|
Yes, the JSON output format does have both the matching and suppressed vulnerabilities in the output. I will proceed with just the table output format with the approach you had discussed previously above. |
…st. Instead, procceded with making changes for the new flag for just the table output format. Signed-off-by: Vimal Patel <vimalpatel0611@gmail.com>
|
@kzantow can you review again when you get a please chance? Pushed updates based on your suggestions earlier. |
There was a problem hiding this comment.
Sorry for the delay here, I left some more specific feedback, but a couple general comments:
- I feel the parameters dealing with showing suppressed vulnerabilities should be the last parameters in every function call for consistency, especially since it's the most recent featured added
- after pondering this a bit more, I think
--show-suppressedis probably a better name for this parameter, as--include-suppresseddoes not affect failure codes and such, which seems a bit misleading
Happy to hear thoughts on this!
Signed-off-by: Vimal Patel <vimalpatel0611@gmail.com>
|
@kzantow thanks for the feedback. I have made updates based on your suggestions, can you please review again when you get a chance? |
kzantow
changed the title
There was a problem hiding this comment.
Thanks for the contribution @vimalpatel19, this looks great!
|
Hi @vimalpatel19 -- it looks like there's just one tiny lint issue: https://github.com/anchore/grype/actions/runs/3370373110/jobs/5592852609#step:8:20 I'd be fine with just adding a |
Signed-off-by: Vimal Patel <vimalpatel0611@gmail.com>
|
@kzantow addressed the lint issue for function length. |
Implemented new CLI flag (--include-suppressed) that allows for ignored/suppressed matches to be displayed in the table output format when that flag is provided.
Closes: #887