Support RHEL content sets in a call to GetNodeVulnerabilities - #1059
Conversation
|
Images are ready for the commit at becf56c. To use the images, use the tag |
jvdm
left a comment
There was a problem hiding this comment.
I have one question that might require a change, so still pending. I also have a Nit that is not a blocker.
| if err != nil { | ||
| return nil, status.Error(codes.Internal, err.Error()) | ||
| } | ||
| log.Debugf("Scanned %d components from NodeInventory and returning %d features", len(components.GetRhelComponents()), len(layer.Features)) |
There was a problem hiding this comment.
Nit: I would adjust the terminology. We usually consider "Vulnerability Scanning" as a two parts process: Analysis (detection and inventory creation) and Vulnerability Matching. What you are doing here is the latter.
Maybe "Matched vulnerabilities on %d RHEL components in node inventory." instead?
There was a problem hiding this comment.
Also, making this an INFO rather than a DEBUG would cause too much impact in the logs? Having this sort of info in the bundles would be valuable, but I am not sure of the impact. Just push me back if that is not feasible.
| cpes := s.repoToCPE.Get(components.GetRhelContentSets()) | ||
| log.Debugf("Converted content sets '%v' to CPEs '%v'", components.GetRhelContentSets(), cpes) | ||
| for _, comp := range components.GetRhelComponents() { | ||
| comp.Cpes = append(comp.Cpes, cpes...) |
There was a problem hiding this comment.
Maybe ensure comp.Cpes is empty since it came from the API call. Or were you purposefully appending not to overwrite?
We will revisit this piece of code as part of ROX-14414. By then, we will have to decide how to support input that has CPEs set and inputs that don't. But until then, overwriting to sanitize input seems the right course of action.
There was a problem hiding this comment.
Okay, I was appending not to overwrite - having more CPEs seems not bad to me. But if we have a followup to tackle this, then I will follow your suggestion too overwrite and leave a comment about the follow-up ticket.
As a result of #1053 we decided that
Analyzewill not provide CPEs but instead would include RHEL content sets. This PR adapts theGetNodeVulnerabilitiesto that change, so that content sets are translated to CPEs in Scanner.How tested
GetNodeVulnerabilities