From d12836e69cfe280f7ac9db67f028e4fca34fd650 Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Fri, 10 Feb 2023 07:04:52 +0200 Subject: [PATCH] refactor(cve): improve CVE test time by mocking trivy (#1184) - refactor(cve): remove the global of type cveinfo.CveInfo from the extensions package Replace it with an attribute on controller level - refactor(controller): extract initialization logic from controller.Run() - test(cve): mock cve scanner in cli tests Signed-off-by: Andrei Aaron --- pkg/api/controller.go | 66 +++--- pkg/api/controller_test.go | 13 +- pkg/api/routes.go | 2 +- pkg/cli/cve_cmd_test.go | 166 +++++++++++++- pkg/cli/image_cmd_test.go | 2 +- pkg/cli/root.go | 4 + pkg/cli/root_test.go | 41 +++- pkg/compliance/v1_0_0/check_test.go | 4 + pkg/exporter/api/controller_test.go | 12 +- pkg/extensions/extension_search.go | 56 ++--- pkg/extensions/extension_search_disabled.go | 12 +- pkg/extensions/search/common/common_test.go | 240 +++++++++++++++++--- pkg/extensions/sync/sync_test.go | 4 + pkg/test/common.go | 26 ++- pkg/test/common_test.go | 35 +++ 15 files changed, 552 insertions(+), 131 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 2a975ddc..8952c3ab 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -50,6 +50,7 @@ type Controller struct { Audit *log.Logger Server *http.Server Metrics monitoring.MetricServer + CveInfo ext.CveInfo wgShutDown *goSync.WaitGroup // use it to gracefully shutdown goroutines // runtime params chosenPort int // kernel-chosen port @@ -120,11 +121,7 @@ func (c *Controller) GetPort() int { } func (c *Controller) Run(reloadCtx context.Context) error { - // print the current configuration, but strip secrets - c.Log.Info().Interface("params", c.Config.Sanitize()).Msg("configuration settings") - - // print the current runtime environment - DumpRuntimeParams(c.Log) + c.StartBackgroundTasks(reloadCtx) // setup HTTP API router engine := mux.NewRouter() @@ -153,26 +150,6 @@ func (c *Controller) Run(reloadCtx context.Context) error { c.Router = engine c.Router.UseEncodedPath() - var enabled bool - if c.Config != nil && - c.Config.Extensions != nil && - c.Config.Extensions.Metrics != nil && - *c.Config.Extensions.Metrics.Enable { - enabled = true - } - - c.Metrics = monitoring.NewMetricsServer(enabled, c.Log) - - if err := c.InitImageStore(reloadCtx); err != nil { - return err - } - - if err := c.InitRepoDB(reloadCtx); err != nil { - return err - } - - c.StartBackgroundTasks(reloadCtx) - monitoring.SetServerInfo(c.Metrics, c.Config.Commit, c.Config.BinaryType, c.Config.GoVersion, c.Config.DistSpecVersion) @@ -259,6 +236,43 @@ func (c *Controller) Run(reloadCtx context.Context) error { return server.Serve(listener) } +func (c *Controller) Init(reloadCtx context.Context) error { + // print the current configuration, but strip secrets + c.Log.Info().Interface("params", c.Config.Sanitize()).Msg("configuration settings") + + // print the current runtime environment + DumpRuntimeParams(c.Log) + + var enabled bool + if c.Config != nil && + c.Config.Extensions != nil && + c.Config.Extensions.Metrics != nil && + *c.Config.Extensions.Metrics.Enable { + enabled = true + } + + c.Metrics = monitoring.NewMetricsServer(enabled, c.Log) + + if err := c.InitImageStore(reloadCtx); err != nil { + return err + } + + if err := c.InitRepoDB(reloadCtx); err != nil { + return err + } + + c.InitCVEInfo() + + return nil +} + +func (c *Controller) InitCVEInfo() { + // Enable CVE extension if extension config is provided + if c.Config != nil && c.Config.Extensions != nil { + c.CveInfo = ext.GetCVEInfo(c.Config, c.StoreController, c.RepoDB, c.Log) + } +} + func (c *Controller) InitImageStore(ctx context.Context) error { c.StoreController = storage.StoreController{} @@ -616,7 +630,7 @@ func (c *Controller) StartBackgroundTasks(reloadCtx context.Context) { // Enable extensions if extension config is provided for DefaultStore if c.Config != nil && c.Config.Extensions != nil { ext.EnableMetricsExtension(c.Config, c.Log, c.Config.Storage.RootDirectory) - ext.EnableSearchExtension(c.Config, c.StoreController, c.RepoDB, c.Log) + ext.EnableSearchExtension(c.Config, c.StoreController, c.RepoDB, c.CveInfo, c.Log) } if c.Config.Storage.SubPaths != nil { diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 9d78f2bf..83a1cbee 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -260,7 +260,10 @@ func TestRunAlreadyRunningServer(t *testing.T) { cm.StartAndWait(port) defer cm.StopServer() - err := ctlr.Run(context.Background()) + err := ctlr.Init(context.Background()) + So(err, ShouldBeNil) + + err = ctlr.Run(context.Background()) So(err, ShouldNotBeNil) }) } @@ -328,7 +331,7 @@ func TestObjectStorageController(t *testing.T) { ctlr := makeController(conf, "zot", "") So(ctlr, ShouldNotBeNil) - err := ctlr.Run(context.Background()) + err := ctlr.Init(context.Background()) So(err, ShouldNotBeNil) }) @@ -928,7 +931,7 @@ func TestMultipleInstance(t *testing.T) { }, } ctlr := api.NewController(conf) - err := ctlr.Run(context.Background()) + err := ctlr.Init(context.Background()) So(err, ShouldEqual, errors.ErrImgStoreNotFound) globalDir := t.TempDir() @@ -1016,7 +1019,7 @@ func TestMultipleInstance(t *testing.T) { ctlr.Config.Storage.SubPaths = subPathMap - err := ctlr.Run(context.Background()) + err := ctlr.Init(context.Background()) So(err, ShouldNotBeNil) // subpath root directory does not exist. @@ -1025,7 +1028,7 @@ func TestMultipleInstance(t *testing.T) { ctlr.Config.Storage.SubPaths = subPathMap - err = ctlr.Run(context.Background()) + err = ctlr.Init(context.Background()) So(err, ShouldNotBeNil) subPathMap["/a"] = config.StorageConfig{RootDirectory: subDir, Dedupe: true, GC: true} diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 81e763e1..5cc83337 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -126,7 +126,7 @@ func (rh *RouteHandler) SetupRoutes() { } else { // extended build ext.SetupMetricsRoutes(rh.c.Config, rh.c.Router, rh.c.StoreController, rh.c.Log) - ext.SetupSearchRoutes(rh.c.Config, rh.c.Router, rh.c.StoreController, rh.c.RepoDB, rh.c.Log) + ext.SetupSearchRoutes(rh.c.Config, rh.c.Router, rh.c.StoreController, rh.c.RepoDB, rh.c.CveInfo, rh.c.Log) gqlPlayground.SetupGQLPlaygroundRoutes(rh.c.Config, rh.c.Router, rh.c.StoreController, rh.c.Log) } } diff --git a/pkg/cli/cve_cmd_test.go b/pkg/cli/cve_cmd_test.go index f72ffcdc..bae1fd50 100644 --- a/pkg/cli/cve_cmd_test.go +++ b/pkg/cli/cve_cmd_test.go @@ -5,8 +5,12 @@ package cli //nolint:testpackage import ( "bytes" + "context" + "encoding/json" + "errors" "fmt" "io" + "net/http" "os" "path" "regexp" @@ -14,6 +18,9 @@ import ( "testing" "time" + regTypes "github.com/google/go-containerregistry/pkg/v1/types" + godigest "github.com/opencontainers/go-digest" + ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" "github.com/spf13/cobra" @@ -21,7 +28,12 @@ import ( "zotregistry.io/zot/pkg/api" "zotregistry.io/zot/pkg/api/config" extconf "zotregistry.io/zot/pkg/extensions/config" + cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" + cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" + "zotregistry.io/zot/pkg/log" + "zotregistry.io/zot/pkg/meta/repodb" "zotregistry.io/zot/pkg/test" + "zotregistry.io/zot/pkg/test/mocks" ) func TestSearchCVECmd(t *testing.T) { @@ -441,9 +453,23 @@ func TestNegativeServerResponse(t *testing.T) { ctlr := api.NewController(conf) ctlr.Log.Logger = ctlr.Log.Output(writers) - cm := test.NewControllerManager(ctlr) - cm.StartAndWait(conf.HTTP.Port) - defer cm.StopServer() + ctx := context.Background() + + if err := ctlr.Init(ctx); err != nil { + panic(err) + } + + ctlr.CveInfo = getMockCveInfo(ctlr.RepoDB, ctlr.Log) + + go func() { + if err := ctlr.Run(ctx); !errors.Is(err, http.ErrServerClosed) { + panic(err) + } + }() + + defer ctlr.Shutdown() + + test.WaitTillServerReady(url) _, err = test.ReadLogFileAndSearchString(logPath, "DB update completed, next update scheduled", 90*time.Second) if err != nil { @@ -504,10 +530,23 @@ func TestServerCVEResponse(t *testing.T) { ctlr := api.NewController(conf) ctlr.Log.Logger = ctlr.Log.Output(writers) - cm := test.NewControllerManager(ctlr) + ctx := context.Background() - cm.StartAndWait(conf.HTTP.Port) - defer cm.StopServer() + if err := ctlr.Init(ctx); err != nil { + panic(err) + } + + ctlr.CveInfo = getMockCveInfo(ctlr.RepoDB, ctlr.Log) + + go func() { + if err := ctlr.Run(ctx); !errors.Is(err, http.ErrServerClosed) { + panic(err) + } + }() + + defer ctlr.Shutdown() + + test.WaitTillServerReady(url) _, err = test.ReadLogFileAndSearchString(logPath, "DB update completed, next update scheduled", 90*time.Second) if err != nil { @@ -988,3 +1027,118 @@ func MockSearchCve(searchConfig searchConfig) error { return zotErrors.ErrInvalidFlagsCombination } + +func getMockCveInfo(repoDB repodb.RepoDB, log log.Logger) cveinfo.CveInfo { + // RepoDB loaded with initial data, mock the scanner + severities := map[string]int{ + "UNKNOWN": 0, + "LOW": 1, + "MEDIUM": 2, + "HIGH": 3, + "CRITICAL": 4, + } + + // Setup test CVE data in mock scanner + scanner := mocks.CveScannerMock{ + ScanImageFn: func(image string) (map[string]cvemodel.CVE, error) { + if image == "zot-cve-test:0.0.1" { + return map[string]cvemodel.CVE{ + "CVE-1": { + ID: "CVE-1", + Severity: "CRITICAL", + Title: "Title for CVE-C1", + Description: "Description of CVE-1", + }, + "CVE-2019-9923": { + ID: "CVE-2019-9923", + Severity: "HIGH", + Title: "Title for CVE-2", + Description: "Description of CVE-2", + }, + "CVE-3": { + ID: "CVE-3", + Severity: "MEDIUM", + Title: "Title for CVE-3", + Description: "Description of CVE-3", + }, + "CVE-4": { + ID: "CVE-4", + Severity: "LOW", + Title: "Title for CVE-4", + Description: "Description of CVE-4", + }, + "CVE-5": { + ID: "CVE-5", + Severity: "UNKNOWN", + Title: "Title for CVE-5", + Description: "Description of CVE-5", + }, + }, nil + } + + // By default the image has no vulnerabilities + return map[string]cvemodel.CVE{}, nil + }, + CompareSeveritiesFn: func(severity1, severity2 string) int { + return severities[severity2] - severities[severity1] + }, + IsImageFormatScannableFn: func(image string) (bool, error) { + // Almost same logic compared to actual Trivy specific implementation + var imageDir string + + var inputTag string + + if strings.Contains(image, ":") { + imageDir, inputTag, _ = strings.Cut(image, ":") + } else { + imageDir = image + } + + repoMeta, err := repoDB.GetRepoMeta(imageDir) + if err != nil { + return false, err + } + + manifestDigestStr, ok := repoMeta.Tags[inputTag] + if !ok { + return false, zotErrors.ErrTagMetaNotFound + } + + manifestDigest, err := godigest.Parse(manifestDigestStr.Digest) + if err != nil { + return false, err + } + + manifestData, err := repoDB.GetManifestData(manifestDigest) + if err != nil { + return false, err + } + + var manifestContent ispec.Manifest + + err = json.Unmarshal(manifestData.ManifestBlob, &manifestContent) + if err != nil { + return false, zotErrors.ErrScanNotSupported + } + + for _, imageLayer := range manifestContent.Layers { + switch imageLayer.MediaType { + case ispec.MediaTypeImageLayerGzip, ispec.MediaTypeImageLayer, string(regTypes.DockerLayer): + + return true, nil + default: + + return false, zotErrors.ErrScanNotSupported + } + } + + return false, nil + }, + } + + return &cveinfo.BaseCveInfo{ + Log: log, + Scanner: scanner, + RepoDB: repoDB, + } +} diff --git a/pkg/cli/image_cmd_test.go b/pkg/cli/image_cmd_test.go index b1eb541a..c303102f 100644 --- a/pkg/cli/image_cmd_test.go +++ b/pkg/cli/image_cmd_test.go @@ -1292,7 +1292,7 @@ func TestServerResponseGQLWithoutPermissions(t *testing.T) { } ctlr := api.NewController(conf) - if err := ctlr.Run(context.Background()); err != nil { + if err := ctlr.Init(context.Background()); err != nil { So(err, ShouldNotBeNil) } }) diff --git a/pkg/cli/root.go b/pkg/cli/root.go index 9730f5a7..1da69784 100644 --- a/pkg/cli/root.go +++ b/pkg/cli/root.go @@ -63,6 +63,10 @@ func newServeCmd(conf *config.Config) *cobra.Command { we can change their config on the fly (restart routines with different config) */ reloaderCtx := hotReloader.Start() + if err := ctlr.Init(reloaderCtx); err != nil { + panic(err) + } + if err := ctlr.Run(reloaderCtx); err != nil { panic(err) } diff --git a/pkg/cli/root_test.go b/pkg/cli/root_test.go index 12eb84d9..f871f20b 100644 --- a/pkg/cli/root_test.go +++ b/pkg/cli/root_test.go @@ -78,17 +78,42 @@ func TestServe(t *testing.T) { }) Convey("bad config", func(c C) { - tmpfile, err := os.CreateTemp("", "zot-test*.json") + rootDir := t.TempDir() + + tmpFile := path.Join(rootDir, "zot-test.json") + err := os.WriteFile(tmpFile, []byte(`{"log":{}}`), 0o0600) So(err, ShouldBeNil) - defer os.Remove(tmpfile.Name()) // clean up - content := []byte(`{"log":{}}`) - _, err = tmpfile.Write(content) - So(err, ShouldBeNil) - err = tmpfile.Close() - So(err, ShouldBeNil) - os.Args = []string{"cli_test", "serve", tmpfile.Name()} + + os.Args = []string{"cli_test", "serve", tmpFile} + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) }) + + Convey("config with missing rootDir", func(c C) { + rootDir := t.TempDir() + + // missing storag config should result in an error in Controller.Init() + content := []byte(`{ + "distSpecVersion": "1.1.0-dev", + "http": { + "address":"127.0.0.1", + "port":"8080" + } + }`) + + tmpFile := path.Join(rootDir, "zot-test.json") + err := os.WriteFile(tmpFile, content, 0o0600) + So(err, ShouldBeNil) + + os.Args = []string{"cli_test", "serve", tmpFile} + + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) + + // wait for the config reloader goroutine to start watching the config file + // if we end the test too fast it will delete the config file + // which will cause a panic and mark the test run as a failure + time.Sleep(1 * time.Second) + }) }) } diff --git a/pkg/compliance/v1_0_0/check_test.go b/pkg/compliance/v1_0_0/check_test.go index b02c51fa..38ee70c9 100644 --- a/pkg/compliance/v1_0_0/check_test.go +++ b/pkg/compliance/v1_0_0/check_test.go @@ -81,6 +81,10 @@ func startServer(t *testing.T) (*api.Controller, string) { ctrl.Config.Storage.SubPaths = subPaths go func() { + if err := ctrl.Init(context.Background()); err != nil { + return + } + // this blocks if err := ctrl.Run(context.Background()); err != nil { return diff --git a/pkg/exporter/api/controller_test.go b/pkg/exporter/api/controller_test.go index 8b9fcfa2..c6509a14 100644 --- a/pkg/exporter/api/controller_test.go +++ b/pkg/exporter/api/controller_test.go @@ -126,14 +126,18 @@ func TestNewExporter(t *testing.T) { dir := t.TempDir() serverController.Config.Storage.RootDirectory = dir - go func(c *zotapi.Controller) { + go func(ctrl *zotapi.Controller) { + if err := ctrl.Init(context.Background()); err != nil { + panic(err) + } + // this blocks - if err := c.Run(context.Background()); !errors.Is(err, http.ErrServerClosed) { + if err := ctrl.Run(context.Background()); !errors.Is(err, http.ErrServerClosed) { panic(err) } }(serverController) - defer func(c *zotapi.Controller) { - _ = c.Server.Shutdown(context.TODO()) + defer func(ctrl *zotapi.Controller) { + _ = ctrl.Server.Shutdown(context.TODO()) }(serverController) // wait till ready for { diff --git a/pkg/extensions/extension_search.go b/pkg/extensions/extension_search.go index f005b716..402ac5d1 100644 --- a/pkg/extensions/extension_search.go +++ b/pkg/extensions/extension_search.go @@ -20,13 +20,26 @@ import ( "zotregistry.io/zot/pkg/storage" ) -// We need this object to be a singleton as read/writes in the CVE DB may -// occur at any time via DB downloads as well as during scanning. -// The library doesn't seem to handle concurrency very well internally. -var cveInfo cveinfo.CveInfo //nolint:gochecknoglobals +type CveInfo cveinfo.CveInfo + +func GetCVEInfo(config *config.Config, storeController storage.StoreController, + repoDB repodb.RepoDB, log log.Logger, +) CveInfo { + if config.Extensions.Search == nil || !*config.Extensions.Search.Enable || config.Extensions.Search.CVE == nil { + return nil + } + + dbRepository := "" + + if config.Extensions.Search.CVE.Trivy != nil { + dbRepository = config.Extensions.Search.CVE.Trivy.DBRepository + } + + return cveinfo.NewCVEInfo(storeController, repoDB, dbRepository, log) +} func EnableSearchExtension(config *config.Config, storeController storage.StoreController, - repoDB repodb.RepoDB, log log.Logger, + repoDB repodb.RepoDB, cveInfo CveInfo, log log.Logger, ) { if config.Extensions.Search != nil && *config.Extensions.Search.Enable && config.Extensions.Search.CVE != nil { defaultUpdateInterval, _ := time.ParseDuration("2h") @@ -37,15 +50,8 @@ func EnableSearchExtension(config *config.Config, storeController storage.StoreC log.Warn().Msg("CVE update interval set to too-short interval < 2h, changing update duration to 2 hours and continuing.") //nolint:lll // gofumpt conflicts with lll } - dbRepository := "" - if config.Extensions.Search.CVE.Trivy != nil { - dbRepository = config.Extensions.Search.CVE.Trivy.DBRepository - } - - cveInfo = cveinfo.NewCVEInfo(storeController, repoDB, dbRepository, log) - go func() { - err := downloadTrivyDB(log, config.Extensions.Search.CVE.UpdateInterval) + err := downloadTrivyDB(cveInfo, log, config.Extensions.Search.CVE.UpdateInterval) if err != nil { log.Error().Err(err).Msg("error while downloading TrivyDB") } @@ -55,7 +61,7 @@ func EnableSearchExtension(config *config.Config, storeController storage.StoreC } } -func downloadTrivyDB(log log.Logger, updateInterval time.Duration) error { +func downloadTrivyDB(cveInfo CveInfo, log log.Logger, updateInterval time.Duration) error { for { log.Info().Msg("updating the CVE database") @@ -71,30 +77,12 @@ func downloadTrivyDB(log log.Logger, updateInterval time.Duration) error { } func SetupSearchRoutes(config *config.Config, router *mux.Router, storeController storage.StoreController, - repoDB repodb.RepoDB, log log.Logger, + repoDB repodb.RepoDB, cveInfo CveInfo, log log.Logger, ) { log.Info().Msg("setting up search routes") if config.Extensions.Search != nil && *config.Extensions.Search.Enable { - var resConfig gql_generated.Config - - if config.Extensions.Search.CVE != nil { - // cveinfo should already be initialized by this time - // as EnableSearchExtension is supposed to be called earlier, but let's be sure - if cveInfo == nil { - dbRepository := "" - - if config.Extensions.Search.CVE.Trivy != nil { - dbRepository = config.Extensions.Search.CVE.Trivy.DBRepository - } - - cveInfo = cveinfo.NewCVEInfo(storeController, repoDB, dbRepository, log) - } - - resConfig = search.GetResolverConfig(log, storeController, repoDB, cveInfo) - } else { - resConfig = search.GetResolverConfig(log, storeController, repoDB, nil) - } + resConfig := search.GetResolverConfig(log, storeController, repoDB, cveInfo) graphqlPrefix := router.PathPrefix(constants.FullSearchPrefix).Methods("OPTIONS", "GET", "POST") graphqlPrefix.Handler(gqlHandler.NewDefaultServer(gql_generated.NewExecutableSchema(resConfig))) diff --git a/pkg/extensions/extension_search_disabled.go b/pkg/extensions/extension_search_disabled.go index 5edf43cb..f515a614 100644 --- a/pkg/extensions/extension_search_disabled.go +++ b/pkg/extensions/extension_search_disabled.go @@ -13,9 +13,17 @@ import ( "zotregistry.io/zot/pkg/storage" ) +type CveInfo interface{} + +func GetCVEInfo(config *config.Config, storeController storage.StoreController, + repoDB repodb.RepoDB, log log.Logger, +) CveInfo { + return nil +} + // EnableSearchExtension ... func EnableSearchExtension(config *config.Config, storeController storage.StoreController, - repoDB repodb.RepoDB, log log.Logger, + repoDB repodb.RepoDB, cveInfo CveInfo, log log.Logger, ) { log.Warn().Msg("skipping enabling search extension because given zot binary doesn't include this feature," + "please build a binary that does so") @@ -23,7 +31,7 @@ func EnableSearchExtension(config *config.Config, storeController storage.StoreC // SetupSearchRoutes ... func SetupSearchRoutes(config *config.Config, router *mux.Router, storeController storage.StoreController, - repoDB repodb.RepoDB, log log.Logger, + repoDB repodb.RepoDB, cveInfo CveInfo, log log.Logger, ) { log.Warn().Msg("skipping setting up search routes because given zot binary doesn't include this feature," + "please build a binary that does so") diff --git a/pkg/extensions/search/common/common_test.go b/pkg/extensions/search/common/common_test.go index ea69b7d8..49955a0e 100644 --- a/pkg/extensions/search/common/common_test.go +++ b/pkg/extensions/search/common/common_test.go @@ -20,6 +20,7 @@ import ( dbTypes "github.com/aquasecurity/trivy-db/pkg/types" "github.com/gobwas/glob" + regTypes "github.com/google/go-containerregistry/pkg/v1/types" godigest "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -34,6 +35,8 @@ import ( extconf "zotregistry.io/zot/pkg/extensions/config" "zotregistry.io/zot/pkg/extensions/monitoring" "zotregistry.io/zot/pkg/extensions/search/common" + cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" + cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/repodb" "zotregistry.io/zot/pkg/storage" @@ -354,6 +357,155 @@ func uploadNewRepoTag(tag string, repoName string, baseURL string, layers [][]by return err } +func getMockCveInfo(repoDB repodb.RepoDB, log log.Logger) cveinfo.CveInfo { + // RepoDB loaded with initial data, mock the scanner + severities := map[string]int{ + "UNKNOWN": 0, + "LOW": 1, + "MEDIUM": 2, + "HIGH": 3, + "CRITICAL": 4, + } + + // Setup test CVE data in mock scanner + scanner := mocks.CveScannerMock{ + ScanImageFn: func(image string) (map[string]cvemodel.CVE, error) { + if image == "zot-cve-test:0.0.1" || image == "a/zot-cve-test:0.0.1" { + return map[string]cvemodel.CVE{ + "CVE1": { + ID: "CVE1", + Severity: "MEDIUM", + Title: "Title CVE1", + Description: "Description CVE1", + }, + "CVE2": { + ID: "CVE2", + Severity: "HIGH", + Title: "Title CVE2", + Description: "Description CVE2", + }, + "CVE3": { + ID: "CVE3", + Severity: "LOW", + Title: "Title CVE3", + Description: "Description CVE3", + }, + }, nil + } + + if image == "zot-test:0.0.1" || image == "a/zot-test:0.0.1" { + return map[string]cvemodel.CVE{ + "CVE3": { + ID: "CVE3", + Severity: "LOW", + Title: "Title CVE3", + Description: "Description CVE3", + }, + "CVE4": { + ID: "CVE4", + Severity: "CRITICAL", + Title: "Title CVE4", + Description: "Description CVE4", + }, + }, nil + } + + if image == "test-repo:latest" { + return map[string]cvemodel.CVE{ + "CVE1": { + ID: "CVE1", + Severity: "MEDIUM", + Title: "Title CVE1", + Description: "Description CVE1", + }, + "CVE2": { + ID: "CVE2", + Severity: "HIGH", + Title: "Title CVE2", + Description: "Description CVE2", + }, + "CVE3": { + ID: "CVE3", + Severity: "LOW", + Title: "Title CVE3", + Description: "Description CVE3", + }, + "CVE4": { + ID: "CVE4", + Severity: "CRITICAL", + Title: "Title CVE4", + Description: "Description CVE4", + }, + }, nil + } + + // By default the image has no vulnerabilities + return map[string]cvemodel.CVE{}, nil + }, + CompareSeveritiesFn: func(severity1, severity2 string) int { + return severities[severity2] - severities[severity1] + }, + IsImageFormatScannableFn: func(image string) (bool, error) { + // Almost same logic compared to actual Trivy specific implementation + var imageDir string + + var inputTag string + + if strings.Contains(image, ":") { + imageDir, inputTag, _ = strings.Cut(image, ":") + } else { + imageDir = image + } + + repoMeta, err := repoDB.GetRepoMeta(imageDir) + if err != nil { + return false, err + } + + manifestDigestStr, ok := repoMeta.Tags[inputTag] + if !ok { + return false, zerr.ErrTagMetaNotFound + } + + manifestDigest, err := godigest.Parse(manifestDigestStr.Digest) + if err != nil { + return false, err + } + + manifestData, err := repoDB.GetManifestData(manifestDigest) + if err != nil { + return false, err + } + + var manifestContent ispec.Manifest + + err = json.Unmarshal(manifestData.ManifestBlob, &manifestContent) + if err != nil { + return false, zerr.ErrScanNotSupported + } + + for _, imageLayer := range manifestContent.Layers { + switch imageLayer.MediaType { + case ispec.MediaTypeImageLayerGzip, ispec.MediaTypeImageLayer, string(regTypes.DockerLayer): + + return true, nil + default: + + return false, zerr.ErrScanNotSupported + } + } + + return false, nil + }, + } + + return &cveinfo.BaseCveInfo{ + Log: log, + Scanner: scanner, + RepoDB: repoDB, + } +} + func TestRepoListWithNewestImage(t *testing.T) { Convey("Test repoListWithNewestImage by tag with HTTP", t, func() { subpath := "/a" @@ -671,9 +823,21 @@ func TestRepoListWithNewestImage(t *testing.T) { ctlr := api.NewController(conf) ctlr.Log.Logger = ctlr.Log.Output(writers) - ctlrManager := NewControllerManager(ctlr) - ctlrManager.StartAndWait(port) - defer ctlrManager.StopServer() + ctx := context.Background() + + if err := ctlr.Init(ctx); err != nil { + panic(err) + } + + ctlr.CveInfo = getMockCveInfo(ctlr.RepoDB, ctlr.Log) + + go func() { + if err := ctlr.Run(ctx); !errors.Is(err, http.ErrServerClosed) { + panic(err) + } + }() + + defer ctlr.Shutdown() substring := "{\"Search\":{\"Enable\":true,\"CVE\":{\"UpdateInterval\":3600000000000,\"Trivy\":{\"DBRepository\":\"ghcr.io/project-zot/trivy-db\"}}}" //nolint: lll found, err := readFileAndSearchString(logPath, substring, 2*time.Minute) @@ -688,12 +852,9 @@ func TestRepoListWithNewestImage(t *testing.T) { So(found, ShouldBeTrue) So(err, ShouldBeNil) - resp, err := resty.R().Get(baseURL + "/v2/") - So(resp, ShouldNotBeNil) - So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, 200) + WaitTillServerReady(baseURL) - resp, err = resty.R().Get(baseURL + graphqlQueryPrefix) + resp, err := resty.R().Get(baseURL + graphqlQueryPrefix) So(resp, ShouldNotBeNil) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, 422) @@ -1322,20 +1483,13 @@ func TestUtilsMethod(t *testing.T) { } func TestDerivedImageList(t *testing.T) { - subpath := "/a" - - err := testSetup(t, subpath) - if err != nil { - panic(err) - } + rootDir = t.TempDir() port := GetFreePort() baseURL := GetBaseURL(port) conf := config.New() conf.HTTP.Port = port conf.Storage.RootDirectory = rootDir - conf.Storage.SubPaths = make(map[string]config.StorageConfig) - conf.Storage.SubPaths[subpath] = config.StorageConfig{RootDirectory: subRootDir} defaultVal := true conf.Extensions = &extconf.ExtensionConfig{ Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}}, @@ -1806,20 +1960,13 @@ func TestGetImageManifest(t *testing.T) { } func TestBaseImageList(t *testing.T) { - subpath := "/a" - - err := testSetup(t, subpath) - if err != nil { - panic(err) - } + rootDir = t.TempDir() port := GetFreePort() baseURL := GetBaseURL(port) conf := config.New() conf.HTTP.Port = port conf.Storage.RootDirectory = rootDir - conf.Storage.SubPaths = make(map[string]config.StorageConfig) - conf.Storage.SubPaths[subpath] = config.StorageConfig{RootDirectory: subRootDir} defaultVal := true conf.Extensions = &extconf.ExtensionConfig{ Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}}, @@ -2866,9 +3013,21 @@ func TestGlobalSearch(t *testing.T) { ctlr := api.NewController(conf) ctlr.Log.Logger = ctlr.Log.Output(writers) - ctlrManager := NewControllerManager(ctlr) - ctlrManager.StartAndWait(port) - defer ctlrManager.StopServer() + ctx := context.Background() + + if err := ctlr.Init(ctx); err != nil { + panic(err) + } + + ctlr.CveInfo = getMockCveInfo(ctlr.RepoDB, ctlr.Log) + + go func() { + if err := ctlr.Run(ctx); !errors.Is(err, http.ErrServerClosed) { + panic(err) + } + }() + + defer ctlr.Shutdown() // Wait for trivy db to download substring := "{\"Search\":{\"Enable\":true,\"CVE\":{\"UpdateInterval\":3600000000000,\"Trivy\":{\"DBRepository\":\"ghcr.io/project-zot/trivy-db\"}}}" //nolint: lll @@ -2884,6 +3043,8 @@ func TestGlobalSearch(t *testing.T) { So(found, ShouldBeTrue) So(err, ShouldBeNil) + WaitTillServerReady(baseURL) + // push test images to repo 1 image 1 config1, layers1, manifest1, err := GetImageComponents(100) So(err, ShouldBeNil) @@ -5114,9 +5275,24 @@ func TestImageSummary(t *testing.T) { configBlob, errConfig := json.Marshal(config) configDigest := godigest.FromBytes(configBlob) So(errConfig, ShouldBeNil) // marshall success, config is valid JSON - ctlrManager := NewControllerManager(ctlr) - ctlrManager.StartAndWait(port) - defer ctlrManager.StopServer() + + ctx := context.Background() + + if err := ctlr.Init(ctx); err != nil { + panic(err) + } + + ctlr.CveInfo = getMockCveInfo(ctlr.RepoDB, ctlr.Log) + + go func() { + if err := ctlr.Run(ctx); !errors.Is(err, http.ErrServerClosed) { + panic(err) + } + }() + + defer ctlr.Shutdown() + + WaitTillServerReady(baseURL) manifestBlob, errMarsal := json.Marshal(manifest) So(errMarsal, ShouldBeNil) @@ -5174,8 +5350,8 @@ func TestImageSummary(t *testing.T) { So(imgSummary.Platform.Arch, ShouldEqual, "amd64") So(len(imgSummary.History), ShouldEqual, 1) So(imgSummary.History[0].HistoryDescription.Created, ShouldEqual, createdTime) - So(imgSummary.Vulnerabilities.Count, ShouldEqual, 0) + So(imgSummary.Vulnerabilities.Count, ShouldEqual, 4) // There are 0 vulnerabilities this data used in tests - So(imgSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "NONE") + So(imgSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "CRITICAL") }) } diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index 7225b793..450f68d2 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -784,6 +784,10 @@ func TestConfigReloader(t *testing.T) { go func() { // this blocks + if err := dctlr.Init(reloadCtx); err != nil { + return + } + if err := dctlr.Run(reloadCtx); err != nil { return } diff --git a/pkg/test/common.go b/pkg/test/common.go index 0134e186..c9192229 100644 --- a/pkg/test/common.go +++ b/pkg/test/common.go @@ -187,6 +187,7 @@ func CopyTestFiles(sourceDir, destDir string) { } type Controller interface { + Init(ctx context.Context) error Run(ctx context.Context) error Shutdown() GetPort() int @@ -196,14 +197,22 @@ type ControllerManager struct { controller Controller } +func (cm *ControllerManager) RunServer(ctx context.Context) { + // Useful to be able to call in the same goroutine for testing purposes + if err := cm.controller.Run(ctx); !errors.Is(err, http.ErrServerClosed) { + panic(err) + } +} + func (cm *ControllerManager) StartServer() { - // this blocks ctx := context.Background() + if err := cm.controller.Init(ctx); err != nil { + panic(err) + } + go func() { - if err := cm.controller.Run(ctx); err != nil { - return - } + cm.RunServer(ctx) }() } @@ -217,14 +226,7 @@ func (cm *ControllerManager) WaitServerToBeReady(port string) { } func (cm *ControllerManager) StartAndWait(port string) { - // this blocks - ctx := context.Background() - - go func() { - if err := cm.controller.Run(ctx); err != nil { - return - } - }() + cm.StartServer() url := GetBaseURL(port) WaitTillServerReady(url) diff --git a/pkg/test/common_test.go b/pkg/test/common_test.go index ee1e0551..07c99807 100644 --- a/pkg/test/common_test.go +++ b/pkg/test/common_test.go @@ -4,6 +4,7 @@ package test_test import ( + "context" "encoding/json" "fmt" "os" @@ -192,6 +193,40 @@ func TestWaitTillTrivyDBDownloadStarted(t *testing.T) { }) } +func TestControllerManager(t *testing.T) { + Convey("Test StartServer Init() panic", t, func() { + port := test.GetFreePort() + + conf := config.New() + conf.HTTP.Port = port + + ctlr := api.NewController(conf) + ctlrManager := test.NewControllerManager(ctlr) + + // No storage configured + So(func() { ctlrManager.StartServer() }, ShouldPanic) + }) + + Convey("Test RunServer panic", t, func() { + tempDir := t.TempDir() + + // Invalid port + conf := config.New() + conf.HTTP.Port = "999999" + conf.Storage.RootDirectory = tempDir + + ctlr := api.NewController(conf) + ctlrManager := test.NewControllerManager(ctlr) + + ctx := context.Background() + + err := ctlr.Init(ctx) + So(err, ShouldBeNil) + + So(func() { ctlrManager.RunServer(ctx) }, ShouldPanic) + }) +} + func TestUploadArtifact(t *testing.T) { Convey("Put request results in an error", t, func() { port := test.GetFreePort()