-
Notifications
You must be signed in to change notification settings - Fork 16
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
🐛 Fix progress for unscored assets #424
Conversation
Signed-off-by: Christian Zunker <[email protected]>
@@ -595,7 +595,11 @@ func (s *localAssetScanner) run() (*AssetReport, error) { | |||
return ar, err | |||
} | |||
s.ProgressReporter.Score(report.Score.Rating().Letter()) | |||
s.ProgressReporter.Completed() | |||
if report.Score.Rating().Letter() == "U" { |
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.
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 think, this is different. It is not an error, the asset matched a policy, but didn't get scored.
I used this policy to provoke this:
- asset_filter:
query: platform.name == "k8s-deployment"
scoring_queries:
data_queries:
@preslavgerchev In which situation did you see the problem with the progress bar?
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 tested this with the Kubernetes Best Practices by Mondoo
policy, however it had most of its controls disabled or ignored, leading to the situation you described above (matching filter, no queries).
i can provide the exact policy (with ignored queries) if needed
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 am not 100% sure I understand what we are solving here then. In case policies applied it was scored. Feels like the issue is with the policy itself.
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.
Preslav showed me the issue. I agree that we need to fix the overall progress, but reading the score itself feels wrong. The progress bar should not care about the rating at all. We can move forward with this approach as mitigation but we need a better solution for that handling going forward.
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.
Follow-up is #439
cli/reporter/print_compact.go
Outdated
if !ok { | ||
continue | ||
} | ||
if len(report.Scores) >= 0 { |
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.
was this meant to be > 0
instead of >= 0
? else the check does not make a lot of sense
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.
Oh, good catch. Thanks.
Signed-off-by: Christian Zunker <[email protected]>
e213aa3
to
d3c0cbb
Compare
Before, the progress bars looked like this, when unscored assets where included:
Now, it looks like this: