fix(CVE): attempt to scan now returns early with an error if trivyDB metadata json is missing (#1548)

Also modify zli to retry in case of such errors,
assuming the trivyDB will eventually be downloaded by the scheduled task.

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

errors/errors.go

@@ -46,6 +46,7 @@ var (
 	ErrEmptyValue                     = errors.New("cache: empty value")
 	ErrEmptyRepoList                  = errors.New("search: no repository found")
 	ErrCVESearchDisabled              = errors.New("search: CVE search is disabled")
+	ErrCVEDBNotFound                  = errors.New("cve: CVE DB is not present")
 	ErrInvalidRepositoryName          = errors.New("repository: not a valid repository name")
 	ErrSyncMissingCatalog             = errors.New("sync: couldn't fetch upstream registry's catalog")
 	ErrMethodNotSupported             = errors.New("storage: method not supported")

pkg/cli/cve_cmd.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"strings"
+	"time"
 
 	"github.com/briandowns/spinner"
 	"github.com/spf13/cobra"
@@ -14,6 +16,10 @@ import (
 	zotErrors "zotregistry.io/zot/errors"
 )
 
+const (
+	cveDBRetryInterval = 3
+)
+
 func NewCveCommand(searchService SearchService) *cobra.Command {
 	searchCveParams := make(map[string]*string)
 
@@ -148,13 +154,41 @@ func searchCve(searchConfig searchConfig) error {
 	}
 
 	for _, searcher := range searchers {
+		// there can be CVE DB readyness issues on the server side
+		// we need a retry mechanism for that specific type of errors
+		maxAttempts := 20
+
+		for i := 0; i < maxAttempts; i++ {
 			found, err := searcher.search(searchConfig)
-		if found {
-			if err != nil {
+			if !found {
+				// searcher does not support this searchConfig
+				// exit the attempts loop and try a different searcher
+				break
+			}
+
+			if err == nil {
+				// searcher matcher search config and results are already printed
+				return nil
+			}
+
+			if i+1 >= maxAttempts {
+				// searcher matches search config but there are errors
+				// this is the last attempt and we cannot retry
 				return err
 			}
 
-			return nil
+			if strings.Contains(err.Error(), zotErrors.ErrCVEDBNotFound.Error()) {
+				// searches matches search config but CVE DB is not ready server side
+				// wait and retry a few more times
+				fmt.Fprintln(searchConfig.resultWriter,
+					"[warning] CVE DB is not ready [", i, "] - retry in ", cveDBRetryInterval, " seconds")
+				time.Sleep(cveDBRetryInterval * time.Second)
+
+				continue
+			}
+
+			// an unrecoverable error occurred
+			return err
 		}
 	}
 

pkg/cli/cve_cmd_test.go

@@ -14,7 +14,9 @@ import (
 	"os"
 	"path"
 	"regexp"
+	"strconv"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -287,6 +289,48 @@ func TestSearchCVECmd(t *testing.T) {
 		So(err, ShouldBeNil)
 	})
 
+	Convey("Test images by CVE ID - positive with retries", t, func() {
+		args := []string{"cvetest", "--cve-id", "aCVEID", "--url", "someURL"}
+		configPath := makeConfigFile(`{"configs":[{"_name":"cvetest","showspinner":false}]}`)
+		defer os.Remove(configPath)
+		mockService := mockServiceForRetry{succeedOn: 2} // CVE info will be provided in 2nd attempt
+		cveCmd := NewCveCommand(&mockService)
+		buff := bytes.NewBufferString("")
+		cveCmd.SetOut(buff)
+		cveCmd.SetErr(buff)
+		cveCmd.SetArgs(args)
+		err := cveCmd.Execute()
+		space := regexp.MustCompile(`\s+`)
+		str := space.ReplaceAllString(buff.String(), " ")
+		t.Logf("Output: %s", str)
+		So(strings.TrimSpace(str), ShouldContainSubstring,
+			"[warning] CVE DB is not ready [ 0 ] - retry in "+strconv.Itoa(cveDBRetryInterval)+" seconds")
+		So(strings.TrimSpace(str), ShouldContainSubstring,
+			"IMAGE NAME TAG OS/ARCH DIGEST SIGNED SIZE anImage tag os/arch 6e2f80bf false 123kB")
+		So(err, ShouldBeNil)
+	})
+
+	Convey("Test images by CVE ID - failed after retries", t, func() {
+		args := []string{"cvetest", "--cve-id", "aCVEID", "--url", "someURL"}
+		configPath := makeConfigFile(`{"configs":[{"_name":"cvetest","showspinner":false}]}`)
+		defer os.Remove(configPath)
+		mockService := mockServiceForRetry{succeedOn: -1} // CVE info will be unavailable on all retries
+		cveCmd := NewCveCommand(&mockService)
+		buff := bytes.NewBufferString("")
+		cveCmd.SetOut(buff)
+		cveCmd.SetErr(buff)
+		cveCmd.SetArgs(args)
+		err := cveCmd.Execute()
+		space := regexp.MustCompile(`\s+`)
+		str := space.ReplaceAllString(buff.String(), " ")
+		t.Logf("Output: %s", str)
+		So(strings.TrimSpace(str), ShouldContainSubstring,
+			"[warning] CVE DB is not ready [ 0 ] - retry in "+strconv.Itoa(cveDBRetryInterval)+" seconds")
+		So(strings.TrimSpace(str), ShouldNotContainSubstring,
+			"IMAGE NAME TAG OS/ARCH DIGEST SIGNED SIZE anImage tag os/arch 6e2f80bf false 123kB")
+		So(err, ShouldNotBeNil)
+	})
+
 	Convey("Test images by CVE ID - invalid CVE ID", t, func() {
 		args := []string{"cvetest", "--cve-id", "invalidCVEID"}
 		configPath := makeConfigFile(`{"configs":[{"_name":"cvetest","showspinner":false}]}`)
@@ -1222,3 +1266,26 @@ func getMockCveInfo(repoDB repodb.RepoDB, log log.Logger) cveinfo.CveInfo {
 		RepoDB:  repoDB,
 	}
 }
+
+type mockServiceForRetry struct {
+	mockService
+	retryCounter int
+	succeedOn    int
+}
+
+func (service *mockServiceForRetry) getImagesByCveID(ctx context.Context, config searchConfig,
+	username, password, cvid string, rch chan stringResult, wtgrp *sync.WaitGroup,
+) {
+	service.retryCounter += 1
+
+	if service.retryCounter < service.succeedOn || service.succeedOn < 0 {
+		rch <- stringResult{"", zotErrors.ErrCVEDBNotFound}
+		close(rch)
+
+		wtgrp.Done()
+
+		return
+	}
+
+	service.getImageByName(ctx, config, username, password, "anImage", rch, wtgrp)
+}

pkg/cli/service.go

@@ -943,7 +943,7 @@ func isContextDone(ctx context.Context) bool {
 	}
 }
 
-// Query using JQL, the query string is passed as a parameter
+// Query using GQL, the query string is passed as a parameter
 // errors are returned in the stringResult channel, the unmarshalled payload is in resultPtr.
 func (service searchService) makeGraphQLQuery(ctx context.Context,
 	config searchConfig, username, password, query string,

pkg/extensions/search/cve/trivy/scanner.go

@@ -4,9 +4,11 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
 	"path"
 	"sync"
 
+	"github.com/aquasecurity/trivy-db/pkg/metadata"
 	dbTypes "github.com/aquasecurity/trivy-db/pkg/types"
 	"github.com/aquasecurity/trivy/pkg/commands/artifact"
 	"github.com/aquasecurity/trivy/pkg/commands/operation"
@@ -155,6 +157,11 @@ func (scanner Scanner) getTrivyOptions(image string) flag.Options {
 func (scanner Scanner) runTrivy(opts flag.Options) (types.Report, error) {
 	ctx := context.Background()
 
+	err := scanner.checkDBPresence()
+	if err != nil {
+		return types.Report{}, err
+	}
+
 	runner, err := artifact.NewRunner(ctx, opts)
 	if err != nil {
 		return types.Report{}, err
@@ -333,7 +340,7 @@ func (scanner Scanner) ScanImage(image string) (map[string]cvemodel.CVE, error)
 	return cveidMap, nil
 }
 
-// UpdateDb download the Trivy DB / Cache under the store root directory.
+// UpdateDB downloads the Trivy DB / Cache under the store root directory.
 func (scanner Scanner) UpdateDB() error {
 	// We need a lock as using multiple substores each with it's own DB
 	// can result in a DATARACE because some varibles in trivy-db are global
@@ -395,6 +402,34 @@ func (scanner Scanner) updateDB(dbDir string) error {
 	return nil
 }
 
+// checkDBPresence errors if the DB metadata files cannot be accessed.
+func (scanner Scanner) checkDBPresence() error {
+	result := true
+
+	if scanner.storeController.DefaultStore != nil {
+		dbDir := path.Join(scanner.storeController.DefaultStore.RootDir(), "_trivy")
+		if _, err := os.Stat(metadata.Path(dbDir)); err != nil {
+			result = false
+		}
+	}
+
+	if scanner.storeController.SubStore != nil {
+		for _, storage := range scanner.storeController.SubStore {
+			dbDir := path.Join(storage.RootDir(), "_trivy")
+
+			if _, err := os.Stat(metadata.Path(dbDir)); err != nil {
+				result = false
+			}
+		}
+	}
+
+	if !result {
+		return zerr.ErrCVEDBNotFound
+	}
+
+	return nil
+}
+
 func (scanner Scanner) CompareSeverities(severity1, severity2 string) int {
 	return dbTypes.CompareSeverityString(severity1, severity2)
 }

pkg/extensions/search/cve/trivy/scanner_internal_test.go

@@ -15,6 +15,7 @@ import (
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	. "github.com/smartystreets/goconvey/convey"
 
+	zerr "zotregistry.io/zot/errors"
 	"zotregistry.io/zot/pkg/common"
 	"zotregistry.io/zot/pkg/extensions/monitoring"
 	"zotregistry.io/zot/pkg/log"
@@ -124,6 +125,11 @@ func TestMultipleStoragePath(t *testing.T) {
 		generateTestImage(storeController, img1)
 		generateTestImage(storeController, img2)
 
+		// Try to scan without the DB being downloaded
+		_, err = scanner.ScanImage(img0)
+		So(err, ShouldNotBeNil)
+		So(err, ShouldWrap, zerr.ErrCVEDBNotFound)
+
 		// Download DB since DB download on scan is disabled
 		err = scanner.UpdateDB()
 		So(err, ShouldBeNil)
@@ -197,12 +203,20 @@ func TestTrivyLibraryErrors(t *testing.T) {
 		err = repodb.ParseStorage(repoDB, storeController, log)
 		So(err, ShouldBeNil)
 
+		img := "zot-test:0.0.1" //nolint:goconst
+
 		// Download DB fails for missing DB url
 		scanner := NewScanner(storeController, repoDB, "", "", log)
 
 		err = scanner.UpdateDB()
 		So(err, ShouldNotBeNil)
 
+		// Try to scan without the DB being downloaded
+		opts := scanner.getTrivyOptions(img)
+		_, err = scanner.runTrivy(opts)
+		So(err, ShouldNotBeNil)
+		So(err, ShouldWrap, zerr.ErrCVEDBNotFound)
+
 		// Download DB fails for invalid Java DB
 		scanner = NewScanner(storeController, repoDB, "ghcr.io/project-zot/trivy-db",
 			"ghcr.io/project-zot/trivy-not-db", log)
@@ -217,10 +231,8 @@ func TestTrivyLibraryErrors(t *testing.T) {
 		err = scanner.UpdateDB()
 		So(err, ShouldBeNil)
 
-		img := "zot-test:0.0.1"
-
 		// Scanning image with correct options
-		opts := scanner.getTrivyOptions(img)
+		opts = scanner.getTrivyOptions(img)
 		_, err = scanner.runTrivy(opts)
 		So(err, ShouldBeNil)
 
@@ -482,7 +494,7 @@ func TestDefaultTrivyDBUrl(t *testing.T) {
 		So(err, ShouldBeNil)
 
 		// Scanning image
-		img := "zot-test:0.0.1"
+		img := "zot-test:0.0.1" //nolint:goconst
 
 		opts := scanner.getTrivyOptions(img)
 		_, err = scanner.runTrivy(opts)