Skip to content

feat(output): display base image info on HTML output#1499

Closed
hogo6002 wants to merge 22 commits intogoogle:base-image-queriesfrom
hogo6002:base-image
Closed

feat(output): display base image info on HTML output#1499
hogo6002 wants to merge 22 commits intogoogle:base-image-queriesfrom
hogo6002:base-image

Conversation

@hogo6002
Copy link
Contributor

@hogo6002 hogo6002 commented Jan 15, 2025

Resolves #1410

Adding detailed base image and layer data for HTML output: https://hogo6002.github.io/mvp/
TODO:

  • Cleanup code after ImageMetadata code get merged

Follow-up PR:

  • Add base image and layer data for table output

@hogo6002 hogo6002 marked this pull request as ready for review January 20, 2025 00:30

packageFixedVersion := calculatePackageFixedVersion(vulnPkg.Package.Ecosystem, regularVulnList)

binaryNames := []string{vulnPkg.Package.OSPackageName}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably just be placed inline

var sourceResult SourceResult
packages := make([]PackageResult, 0)
packageSet := make(map[string]struct{})
packageMap := make(map[string]PackageResult)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some descriptions/comments to this function to explain what this is doing? It's not clear to me why we need a packageMap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, added. This map is used to deduplicate same packages with the same source name but different OSPackageName


// Build the final result
return buildResult(ecosystemMap, resultCount)
outputResult := populateResult(ecosystemMap, resultCount, vulnResult.ImageMetadata)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can just be one line.

// This filtering should be handled by the container scanning process.
// TODO(gongh@): Revisit this once container scanning can distinguish these cases.
if strings.HasPrefix(packageSource.Source.Path, "usr/lib/") {
if strings.HasPrefix(packageSource.Source.Path, "usr/") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can update this to check the annotation instead now.

// buildResult builds the final Result object from the ecosystem map and total vulnerability count.
func buildResult(ecosystemMap map[string][]SourceResult, resultCount VulnCount) Result {
// populateResult builds the final Result object from the ecosystem map and total vulnerability count.
func populateResult(ecosystemMap map[string][]SourceResult, resultCount VulnCount, imageMetadata *models.ImageMetadata) Result {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why is this renamed to populateResult? It looks like the result is what is being returned, so I think buildResult is still more appropriate.


// populateImageMetadata modifies the result by adding image metadata to it.
// It uses a pointer receiver (*Result) to modify the original result in place.
func populateImageMetadata(result *Result, imageMetadata models.ImageMetadata) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: populateResultWithImageMetadata.


}

func getAllBaseImages(baseImages [][]models.BaseImageDetails) []BaseImageGroupInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can these functions be named something like build?

allLayers := getAllLayers(imageMetadata.LayerMetadata)
allBaseImages := getAllBaseImages(imageMetadata.BaseImages)

layerCount := make(map[int]VulnCount)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two suggestions here:

  1. Use an array instead of a map, as there should be no gaps between the layers.
  2. Set the VulnCount type to *VulnCount.
    This way you don't need to do the whole save to a variable, call .Add, then reassign on lines 255+, you can just call Add directly.

Actually I'm not sure 2 is even necessary, you might be able to do that directly, needs some testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to use array. I can just modify the array element.

}
}

baseImageMap := make(map[int][]LayerInfo)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, use an array instead of a map.

another-rex added a commit that referenced this pull request Jan 21, 2025
This PR fully removes the image package in osv-scanner, and switches to
use osv-scalibr to perform image scanning.

Also includes:
- Implementing base image identification, along with the
baseimagematcher interface and implementation
- Some temporary placeholder values in output_result.go to allow it to
compile. This is resolved in #1499
- Adds image metadata types to show image metadata in the output.
- Minor refactors in osvscanner.go
- determineReturnErr is an extracted function for the very end of DoScan
- Move container scanning support out of DoScan, and into it's own
DoContainerScan function
- Move the helper methods that help pull docker images into it's own
package
- Support checking if docker image exists locally.
- Support local paths with the docker flag.

---------

Co-authored-by: Holly Gong <39108850+hogo6002@users.noreply.github.com>
Co-authored-by: Holly Gong <gongh@google.com>
@another-rex another-rex deleted the branch google:base-image-queries January 21, 2025 00:19
@hogo6002
Copy link
Contributor Author

comments will be resolved in #1513

hogo6002 added a commit to hogo6002/osv-scanner that referenced this pull request Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants