fix(zli): Improve zli CVE diff output (#3994)

* fix(cli): improve zli CVE diff output

Signed-off-by: Akash Kumar <meakash7902@gmail.com>

* test(api): avoid TestRoutes port collision

Signed-off-by: Akash Kumar <meakash7902@gmail.com>

* test(cli): cover CVE diff formatting helpers

Signed-off-by: Akash Kumar <meakash7902@gmail.com>

* test(search): remove redundant test case copy

Signed-off-by: Akash Kumar <meakash7902@gmail.com>

---------

Signed-off-by: Akash Kumar <meakash7902@gmail.com>

pkg/api/routes_test.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"testing"
 
 	"github.com/google/uuid"
@@ -39,10 +40,8 @@ const sessionStr = "session"
 
 func TestRoutes(t *testing.T) {
 	Convey("Make a new controller", t, func() {
-		port := test.GetFreePort()
-		baseURL := test.GetBaseURL(port)
 		conf := config.New()
-		conf.HTTP.Port = port
+		conf.HTTP.Port = "0"
 
 		username, seedUser := test.GenerateRandomString()
 		password, seedPass := test.GenerateRandomString()
@@ -88,9 +87,10 @@ func TestRoutes(t *testing.T) {
 		ctlr.Config.Storage.Commit = true
 
 		cm := test.NewControllerManager(ctlr)
-		cm.StartAndWait(port)
+		cm.StartAndWait(conf.HTTP.Port)
 		defer cm.StopServer()
 
+		baseURL := test.GetBaseURL(strconv.Itoa(ctlr.GetPort()))
 		rthdlr := api.NewRouteHandler(ctlr)
 
 		// NOTE: the url or method itself doesn't matter below since we are calling the handlers directly,

pkg/cli/client/cve_cmd_test.go

@@ -386,6 +386,7 @@ func TestCVEDiffList(t *testing.T) {
 		space := regexp.MustCompile(`\s+`)
 		str := space.ReplaceAllString(buff.String(), " ")
 		str = strings.TrimSpace(str)
+		So(str, ShouldContainSubstring, "CVEs in image repo:image that are not in image repo:base-image")
 		So(str, ShouldContainSubstring, "CVE3")
 		So(str, ShouldNotContainSubstring, "CVE1")
 		So(str, ShouldNotContainSubstring, "CVE2")
@@ -406,6 +407,22 @@ func TestCVEDiffList(t *testing.T) {
 			cveCmd.SetArgs(args)
 			So(cveCmd.Execute(), ShouldNotBeNil)
 		})
+		Convey("Minuend image not found includes image name", func() {
+			args := []string{"diff", "repo:missing-image", "repo:base-image", "--config", "cvetest"}
+			cveCmd.SetArgs(args)
+			err := cveCmd.Execute()
+			So(err, ShouldNotBeNil)
+			So(err.Error(), ShouldContainSubstring, "minuend image repo:missing-image")
+			So(err.Error(), ShouldContainSubstring, "image not found")
+		})
+		Convey("Subtrahend image not found includes image name", func() {
+			args := []string{"diff", "repo:image", "repo:missing-base", "--config", "cvetest"}
+			cveCmd.SetArgs(args)
+			err := cveCmd.Execute()
+			So(err, ShouldNotBeNil)
+			So(err.Error(), ShouldContainSubstring, "subtrahend image repo:missing-base")
+			So(err.Error(), ShouldContainSubstring, "image not found")
+		})
 		Convey("Second input is arch but not enough args", func() {
 			args := []string{"diff", "repo:base-image", "linux/amd64", "--config", "cvetest"}
 			cveCmd.SetArgs(args)

pkg/cli/client/search_functions.go

@@ -297,6 +297,9 @@ func SearchCVEDiffList(config SearchConfig, minuend, subtrahend ImageIdentifier)
 	var builder strings.Builder
 
 	if config.OutputFormat == defaultOutputFormat || config.OutputFormat == "" {
+		fmt.Fprintf(config.ResultWriter, "CVEs in image %s that are not in image %s\n\n",
+			formatImageIdentifier(cveDiffResult.Minuend), formatImageIdentifier(cveDiffResult.Subtrahend))
+
 		imageCVESummary := result.Data.CVEListForImage.Summary
 
 		statsStr := fmt.Sprintf("CRITICAL %d, HIGH %d, MEDIUM %d, LOW %d, UNKNOWN %d, TOTAL %d\n\n",
@@ -319,6 +322,25 @@ func SearchCVEDiffList(config SearchConfig, minuend, subtrahend ImageIdentifier)
 	return nil
 }
 
+func formatImageIdentifier(image ImageIdentifier) string {
+	if image.Repo == "" {
+		return "unknown image"
+	}
+
+	name := image.Repo
+	if image.Tag != "" {
+		name += ":" + image.Tag
+	} else if image.Digest != "" {
+		name += "@" + image.Digest
+	}
+
+	if image.Platform != nil && image.Platform.Os != "" && image.Platform.Arch != "" {
+		name += fmt.Sprintf(" (%s/%s)", image.Platform.Os, image.Platform.Arch)
+	}
+
+	return name
+}
+
 func SearchImagesByCVEIDGQL(config SearchConfig, repo, cveid string) error {
 	username, password := getUsernameAndPassword(config.User)
 	ctx, cancel := context.WithCancel(context.Background())

pkg/cli/client/search_functions_internal_test.go

@@ -561,6 +561,58 @@ func TestSearchCVEForImageGQL(t *testing.T) {
 	})
 }
 
+func TestFormatImageIdentifier(t *testing.T) {
+	Convey("Format image identifier", t, func() {
+		testCases := []struct {
+			name     string
+			image    ImageIdentifier
+			expected string
+		}{
+			{
+				name:     "unknown image",
+				image:    ImageIdentifier{},
+				expected: "unknown image",
+			},
+			{
+				name:     "tag reference",
+				image:    ImageIdentifier{Repo: "repo", Tag: "tag"},
+				expected: "repo:tag",
+			},
+			{
+				name:     "digest reference",
+				image:    ImageIdentifier{Repo: "repo", Digest: "sha256:123"},
+				expected: "repo@sha256:123",
+			},
+			{
+				name: "tag reference with platform",
+				image: ImageIdentifier{
+					Repo:     "repo",
+					Tag:      "tag",
+					Platform: &osArch{Os: "linux", Arch: "amd64"},
+				},
+				expected: "repo:tag (linux/amd64)",
+			},
+			{
+				name: "partial platform is omitted",
+				image: ImageIdentifier{
+					Repo:     "repo",
+					Tag:      "tag",
+					Platform: &osArch{Os: "linux"},
+				},
+				expected: "repo:tag",
+			},
+		}
+
+		for _, testCase := range testCases {
+			testCase := testCase
+
+			Convey(testCase.name, func() {
+				So(formatImageIdentifier(testCase.image), ShouldEqual, testCase.expected)
+			})
+		}
+	})
+}
+
 func TestSearchImagesByCVEIDGQL(t *testing.T) {
 	Convey("SearchImagesByCVEIDGQL", t, func() {
 		buff := bytes.NewBufferString("")

pkg/cli/client/service.go

@@ -168,8 +168,8 @@ func (service *searchService) getCVEDiffListGQL(ctx context.Context, config Sear
 	query := fmt.Sprintf(`
 		{
 			CVEDiffListForImages( minuend: %s, subtrahend: %s ) {
-				Minuend {Repo Tag}
-				Subtrahend {Repo Tag}
+				Minuend {Repo Tag Digest Platform {Os Arch}}
+				Subtrahend {Repo Tag Digest Platform {Os Arch}}
 				CVEList {
 					Id Title Description Severity Reference 
 					PackageList {Name InstalledVersion FixedVersion}

pkg/extensions/search/resolver.go

@@ -284,27 +284,30 @@ func getCVEDiffListForImages(
 	excludedCVE string,
 	log log.Logger, //nolint:unparam // may be used by devs for debugging
 ) (*gql_generated.CVEDiffResult, error) {
-	minuend, err := resolveImageData(ctx, minuend, metaDB)
+	resolvedMinuend, err := resolveImageData(ctx, minuend, metaDB)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("minuend image %s: %w", formatImageInputForError(minuend), err)
 	}
 
+	minuend = resolvedMinuend
 	resultMinuend := getImageIdentifier(minuend)
 	resultSubtrahend := gql_generated.ImageIdentifier{}
 
 	if subtrahend.Repo != "" {
-		subtrahend, err = resolveImageData(ctx, subtrahend, metaDB)
+		resolvedSubtrahend, err := resolveImageData(ctx, subtrahend, metaDB)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("subtrahend image %s: %w", formatImageInputForError(subtrahend), err)
 		}
 
+		subtrahend = resolvedSubtrahend
 		resultSubtrahend = getImageIdentifier(subtrahend)
 	} else {
 		// search for base images
 		// get minuend image meta
 		minuendSummary, err := metaDB.GetImageMeta(godigest.Digest(deref(minuend.Digest, "")))
 		if err != nil {
-			return &gql_generated.CVEDiffResult{}, err
+			return &gql_generated.CVEDiffResult{},
+				fmt.Errorf("minuend image %s: %w", formatImageInputForError(minuend), err)
 		}
 
 		// get the base images for the minuend
@@ -495,6 +498,25 @@ func resolveImageData(ctx context.Context, imageInput gql_generated.ImageInput,
 	return imageInput, nil
 }
 
+func formatImageInputForError(image gql_generated.ImageInput) string {
+	name := image.Repo
+	if name == "" {
+		name = "<empty>"
+	}
+
+	if image.Tag != "" {
+		name += ":" + image.Tag
+	} else if dderef(image.Digest) != "" {
+		name += "@" + dderef(image.Digest)
+	}
+
+	if isPlatformSpecified(image.Platform) {
+		name += fmt.Sprintf(" (%s/%s)", dderef(image.Platform.Os), dderef(image.Platform.Arch))
+	}
+
+	return name
+}
+
 func isPlatformSpecified(platformInput *gql_generated.PlatformInput) bool {
 	if platformInput == nil {
 		return false

pkg/extensions/search/resolver_test.go

@@ -2200,6 +2200,61 @@ func TestCVEResolvers(t *testing.T) { //nolint:gocyclo
 	})
 }
 
+func TestFormatImageInputForError(t *testing.T) {
+	Convey("Format image input for errors", t, func() {
+		testCases := []struct {
+			name     string
+			image    gql_generated.ImageInput
+			expected string
+		}{
+			{
+				name:     "empty image",
+				image:    gql_generated.ImageInput{},
+				expected: "<empty>",
+			},
+			{
+				name:     "tag reference",
+				image:    gql_generated.ImageInput{Repo: "repo", Tag: "tag"},
+				expected: "repo:tag",
+			},
+			{
+				name:     "digest reference",
+				image:    gql_generated.ImageInput{Repo: "repo", Digest: ref("sha256:123")},
+				expected: "repo@sha256:123",
+			},
+			{
+				name: "tag reference with platform",
+				image: gql_generated.ImageInput{
+					Repo: "repo",
+					Tag:  "tag",
+					Platform: &gql_generated.PlatformInput{
+						Os:   ref("linux"),
+						Arch: ref("amd64"),
+					},
+				},
+				expected: "repo:tag (linux/amd64)",
+			},
+			{
+				name: "partial platform is omitted",
+				image: gql_generated.ImageInput{
+					Repo: "repo",
+					Tag:  "tag",
+					Platform: &gql_generated.PlatformInput{
+						Os: ref("linux"),
+					},
+				},
+				expected: "repo:tag",
+			},
+		}
+
+		for _, testCase := range testCases {
+			Convey(testCase.name, func() {
+				So(formatImageInputForError(testCase.image), ShouldEqual, testCase.expected)
+			})
+		}
+	})
+}
+
 func TestMockedDerivedImageList(t *testing.T) {
 	Convey("MetaDB FilterTags error", t, func() {
 		log := log.NewLogger("debug", "/dev/null")

pkg/test/common/utils.go

@@ -106,6 +106,18 @@ func (cm *ControllerManager) WaitServerToBeReady(port string) {
 func (cm *ControllerManager) StartAndWait(port string) {
 	cm.StartServer()
 
+	if port == "0" || port == "" {
+		for {
+			if chosenPort := cm.controller.GetPort(); chosenPort > 0 {
+				port = strconv.Itoa(chosenPort)
+
+				break
+			}
+
+			time.Sleep(SleepTime)
+		}
+	}
+
 	url := GetBaseURL(port)
 	WaitTillServerReady(url)
 }