-
Notifications
You must be signed in to change notification settings - Fork 371
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
feat(output): display base image info on HTML and table outputs #1513
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1513 +/- ##
==========================================
- Coverage 69.00% 69.00% -0.01%
==========================================
Files 197 197
Lines 18796 18863 +67
==========================================
+ Hits 12971 13017 +46
- Misses 5123 5143 +20
- Partials 702 703 +1 ☔ View full report in Codecov by Sentry. |
if strings.HasPrefix(packageSource.Source.Path, "usr/lib/") { | ||
skipSource := false | ||
for _, annotation := range packageSource.ExperimentalAnnotations { | ||
if annotation == extractor.InsideOSPackage { |
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.
nit: You can use labelled for loops here to break the continue the outer loop:
https://go.dev/ref/spec#Continue_statements
@@ -496,6 +614,14 @@ func getFilteredVulnReasons(vulns []VulnResult) string { | |||
return strings.Join(reasons, ", ") | |||
} | |||
|
|||
func getBaseImageNames(baseImageInfo BaseImageGroupInfo) 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.
nit: Should this be getBaseImageName since we are only getting the first name?
@@ -247,6 +367,12 @@ func processSource(packageSource models.PackageSource) SourceResult { | |||
sourceResult.PackageTypeCount.Hidden += 1 | |||
} | |||
} | |||
|
|||
packages := make([]PackageResult, 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.
you can preallocate the array with the size of the packageMap
packages := make([]PackageResult, 0) | |
packages := make([]PackageResult, 0, len(packageMap)) |
<div class="layer-command-container"> | ||
<p><span class="package-detail-title">Layer introduced in: </span></p> | ||
<div class="tooltip"> | ||
<p># {{ $index }} layer</p> |
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.
nit: the " layer" text seems a bit redundant. Also, I still think it's not much value to users to dedicate a whole row to a "# layer" text that gives a diff ID when hovered over.
How about something like this instead?
Also changed the "In base image: True: image") to just "In base image: image" to make it a bit more concise.
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.
LGTM with some small suggestions from a UI perspective.
I'll let @another-rex handle the code review side of things :)
{{ $commandDetail := index $commandSet 1 }} | ||
{{ $diffID := .LayerMetadata.DiffID }} | ||
{{ $longCommand := false }} | ||
{{ if gt (len $command) 109 }} |
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.
why 109? can you leave a comment?
{{ if gt .Count.AnalysisCount.Regular 0 }} | ||
{{ $hasVuln = true }} | ||
{{ end }} | ||
<div title="{{ $diffID }}" class="layer-entry {{ if $hasVuln }}clickable clickable-layer{{ end }}" {{ if $hasVuln }}onclick="quickFilterByLayer('{{ $diffID }}', {{ $command }})"{{ end }}> |
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 it possible to have a better visual representation that you can click on a layer to filter by it?
e.g. something that pops up (a tooltip?) that says "Click to filter vulnerabilities to ones introduced by this layer"
Resolves #1410
Adds detailed base image and layer data for HTML output: https://hogo6002.github.io/mvp/
Adds layer tracing for table output: