From bcdd9988f5e451e588d41a3a15dbabb027abc467 Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Mon, 18 Sep 2023 01:12:20 +0300 Subject: [PATCH] fix(cve): cummulative fixes and improvements for CVE scanning logic (#1810) 1. Only scan CVEs for images returned by graphql calls Since pagination was refactored to account for image indexes, we had started to run the CVE scanner before pagination was applied, resulting in decreased ZOT performance if CVE information was requested 2. Increase in medory-cache of cve results to 1m, from 10k digests. 3. Update CVE model to use CVSS severity values in our code. Previously we relied upon the strings returned by trivy directly, and the sorting they implemented. Since CVE severities are standardized, we don't need to pass around an adapter object just for pagination and sorting purposes anymore. This also improves our testing since we don't mock the sorting functions anymore. 4. Fix a flaky CLI test not waiting for the zot service to start. 5. Add the search build label on search/cve tests which were missing it. 6. The boltdb update method was used in a few places where view was supposed to be called. 7. Add logs for start and finish of parsing MetaDB. 8. Avoid unmarshalling twice to obtain annotations for multiarch images. Signed-off-by: Andrei Aaron --- pkg/cli/client/cve_cmd_internal_test.go | 26 +- pkg/extensions/extension_search_test.go | 2 +- pkg/extensions/search/convert/annotations.go | 7 +- .../search/convert/convert_internal_test.go | 302 ++++++++++++++++++ pkg/extensions/search/convert/convert_test.go | 140 +------- pkg/extensions/search/convert/cve.go | 109 +++++++ pkg/extensions/search/convert/metadb.go | 268 +++++----------- pkg/extensions/search/cve/cve.go | 28 +- .../search/cve/cve_internal_test.go | 2 + pkg/extensions/search/cve/cve_test.go | 13 +- pkg/extensions/search/cve/model/models.go | 51 ++- pkg/extensions/search/cve/pagination.go | 20 +- pkg/extensions/search/cve/pagination_test.go | 11 +- pkg/extensions/search/cve/trivy/scanner.go | 20 +- .../search/cve/trivy/scanner_test.go | 2 + .../search/pagination/pagination_test.go | 2 + pkg/extensions/search/resolver.go | 21 +- pkg/extensions/search/resolver_test.go | 67 ++-- pkg/extensions/search/schema.resolvers.go | 7 +- pkg/extensions/search/search_test.go | 11 - pkg/meta/boltdb/boltdb.go | 4 +- pkg/meta/parse.go | 4 + pkg/test/mocks/cve_mock.go | 20 +- 23 files changed, 675 insertions(+), 462 deletions(-) create mode 100644 pkg/extensions/search/convert/convert_internal_test.go create mode 100644 pkg/extensions/search/convert/cve.go diff --git a/pkg/cli/client/cve_cmd_internal_test.go b/pkg/cli/client/cve_cmd_internal_test.go index c6807c252..94e15ca85 100644 --- a/pkg/cli/client/cve_cmd_internal_test.go +++ b/pkg/cli/client/cve_cmd_internal_test.go @@ -59,7 +59,7 @@ func TestSearchCVECmd(t *testing.T) { ctlr := api.NewController(conf) cm := test.NewControllerManager(ctlr) - cm.StartServer() + cm.StartAndWait(port) defer cm.StopServer() Convey("Test CVE help", t, func() { @@ -947,21 +947,10 @@ func TestCVESort(t *testing.T) { panic(err) } - severities := map[string]int{ - "UNKNOWN": 0, - "LOW": 1, - "MEDIUM": 2, - "HIGH": 3, - "CRITICAL": 4, - } - ctlr.CveInfo = cveinfo.BaseCveInfo{ Log: ctlr.Log, MetaDB: mocks.MetaDBMock{}, Scanner: mocks.CveScannerMock{ - CompareSeveritiesFn: func(severity1, severity2 string) int { - return severities[severity2] - severities[severity1] - }, ScanImageFn: func(image string) (map[string]cvemodel.CVE, error) { return map[string]cvemodel.CVE{ "CVE-2023-1255": { @@ -1385,15 +1374,7 @@ func TestCVECommandErrors(t *testing.T) { } func getMockCveInfo(metaDB mTypes.MetaDB, log log.Logger) cveinfo.CveInfo { - // MetaDB loaded with initial data, mock the scanner - severities := map[string]int{ - "UNKNOWN": 0, - "LOW": 1, - "MEDIUM": 2, - "HIGH": 3, - "CRITICAL": 4, - } - + // MetaDB loaded with initial data now mock the scanner // Setup test CVE data in mock scanner scanner := mocks.CveScannerMock{ ScanImageFn: func(image string) (map[string]cvemodel.CVE, error) { @@ -1436,9 +1417,6 @@ func getMockCveInfo(metaDB mTypes.MetaDB, log log.Logger) cveinfo.CveInfo { // 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(repo string, reference string) (bool, error) { // Almost same logic compared to actual Trivy specific implementation imageDir := repo diff --git a/pkg/extensions/extension_search_test.go b/pkg/extensions/extension_search_test.go index 14d4a3706..d1f94cd07 100644 --- a/pkg/extensions/extension_search_test.go +++ b/pkg/extensions/extension_search_test.go @@ -70,7 +70,7 @@ func TestTrivyDBGenerator(t *testing.T) { // Wait for trivy db to download found, err := ReadLogFileAndCountStringOccurence(logPath, - "DB update completed, next update scheduled", 120*time.Second, 2) + "DB update completed, next update scheduled", 140*time.Second, 2) So(err, ShouldBeNil) So(found, ShouldBeTrue) }) diff --git a/pkg/extensions/search/convert/annotations.go b/pkg/extensions/search/convert/annotations.go index 2b15aebff..1ebb2cff4 100644 --- a/pkg/extensions/search/convert/annotations.go +++ b/pkg/extensions/search/convert/annotations.go @@ -134,9 +134,10 @@ func GetAnnotations(annotations, labels map[string]string) ImageAnnotations { } } -func GetIndexAnnotations(indexAnnotations, manifestAnnotations, manifestLabels map[string]string) ImageAnnotations { - annotationsFromManifest := GetAnnotations(manifestAnnotations, manifestLabels) - +func GetIndexAnnotations( + indexAnnotations map[string]string, + annotationsFromManifest *ImageAnnotations, +) ImageAnnotations { description := GetDescription(indexAnnotations) if description == "" { description = annotationsFromManifest.Description diff --git a/pkg/extensions/search/convert/convert_internal_test.go b/pkg/extensions/search/convert/convert_internal_test.go new file mode 100644 index 000000000..fe99ae68c --- /dev/null +++ b/pkg/extensions/search/convert/convert_internal_test.go @@ -0,0 +1,302 @@ +//go:build search + +package convert + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "github.com/99designs/gqlgen/graphql" + godigest "github.com/opencontainers/go-digest" + ispec "github.com/opencontainers/image-spec/specs-go/v1" + . "github.com/smartystreets/goconvey/convey" + + cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" + "zotregistry.io/zot/pkg/extensions/search/gql_generated" + "zotregistry.io/zot/pkg/log" + "zotregistry.io/zot/pkg/meta/boltdb" + mTypes "zotregistry.io/zot/pkg/meta/types" + "zotregistry.io/zot/pkg/test/mocks" +) + +var ErrTestError = errors.New("TestError") + +func TestCVEConvert(t *testing.T) { + Convey("Test adding CVE information to Summary objects", t, func() { + params := boltdb.DBParameters{ + RootDir: t.TempDir(), + } + boltDB, err := boltdb.GetBoltDriver(params) + So(err, ShouldBeNil) + + metaDB, err := boltdb.New(boltDB, log.NewLogger("debug", "")) + So(err, ShouldBeNil) + + configBlob, err := json.Marshal(ispec.Image{}) + So(err, ShouldBeNil) + + manifestBlob, err := json.Marshal(ispec.Manifest{ + Layers: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageLayerGzip, + Size: 0, + Digest: godigest.NewDigestFromEncoded(godigest.SHA256, "digest"), + }, + }, + }) + So(err, ShouldBeNil) + + repoMeta11 := mTypes.ManifestMetadata{ + ManifestBlob: manifestBlob, + ConfigBlob: configBlob, + } + + digest11 := godigest.FromString("abc1") + err = metaDB.SetManifestMeta("repo1", digest11, repoMeta11) + So(err, ShouldBeNil) + err = metaDB.SetRepoReference("repo1", "0.1.0", digest11, ispec.MediaTypeImageManifest) + So(err, ShouldBeNil) + + reposMeta, manifestMetaMap, _, err := metaDB.SearchRepos(context.Background(), "") + So(err, ShouldBeNil) + + ctx := graphql.WithResponseContext(context.Background(), + graphql.DefaultErrorPresenter, graphql.DefaultRecover) + + Convey("Add CVE Summary to ImageSummary", func() { + var imageSummary *gql_generated.ImageSummary + + So(imageSummary, ShouldBeNil) + + updateImageSummaryVulnerabilities(ctx, + imageSummary, + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{}, ErrTestError + }, + }, + ) + + So(imageSummary, ShouldBeNil) + So(graphql.GetErrors(ctx), ShouldBeNil) + + imageSummary, _, err = ImageManifest2ImageSummary(ctx, "repo1", "0.1.0", digest11, reposMeta[0], + manifestMetaMap[digest11.String()]) + So(err, ShouldBeNil) + + So(imageSummary, ShouldNotBeNil) + So(imageSummary.Vulnerabilities, ShouldBeNil) + + updateImageSummaryVulnerabilities(ctx, + imageSummary, + SkipQGLField{ + Vulnerabilities: true, + }, + mocks.CveInfoMock{}, + ) + + So(imageSummary.Vulnerabilities, ShouldNotBeNil) + So(*imageSummary.Vulnerabilities.Count, ShouldEqual, 0) + So(*imageSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "") + So(graphql.GetErrors(ctx), ShouldBeNil) + + imageSummary.Vulnerabilities = nil + + updateImageSummaryVulnerabilities(ctx, + imageSummary, + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{ + Count: 1, + MaxSeverity: "HIGH", + }, nil + }, + }, + ) + + So(imageSummary.Vulnerabilities, ShouldNotBeNil) + So(*imageSummary.Vulnerabilities.Count, ShouldEqual, 1) + So(*imageSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "HIGH") + So(graphql.GetErrors(ctx), ShouldBeNil) + So(len(imageSummary.Manifests), ShouldEqual, 1) + So(imageSummary.Manifests[0].Vulnerabilities, ShouldNotBeNil) + So(*imageSummary.Manifests[0].Vulnerabilities.Count, ShouldEqual, 1) + So(*imageSummary.Manifests[0].Vulnerabilities.MaxSeverity, ShouldEqual, "HIGH") + + imageSummary.Vulnerabilities = nil + + updateImageSummaryVulnerabilities(ctx, + imageSummary, + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{}, ErrTestError + }, + }, + ) + + So(imageSummary.Vulnerabilities, ShouldNotBeNil) + So(*imageSummary.Vulnerabilities.Count, ShouldEqual, 0) + So(*imageSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "") + So(graphql.GetErrors(ctx).Error(), ShouldContainSubstring, "unable to run vulnerability scan on tag") + }) + + Convey("Add CVE Summary to RepoSummary", func() { + var repoSummary *gql_generated.RepoSummary + So(repoSummary, ShouldBeNil) + + updateRepoSummaryVulnerabilities(ctx, + repoSummary, + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{ + Count: 1, + MaxSeverity: "HIGH", + }, nil + }, + }, + ) + + So(repoSummary, ShouldBeNil) + So(graphql.GetErrors(ctx), ShouldBeNil) + + imageSummary, _, err := ImageManifest2ImageSummary(ctx, "repo1", "0.1.0", digest11, reposMeta[0], + manifestMetaMap[digest11.String()]) + So(err, ShouldBeNil) + + So(imageSummary, ShouldNotBeNil) + + repoSummary = &gql_generated.RepoSummary{} + repoSummary.NewestImage = imageSummary + + So(repoSummary.NewestImage.Vulnerabilities, ShouldBeNil) + + updateImageSummaryVulnerabilities(ctx, + imageSummary, + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{ + Count: 1, + MaxSeverity: "HIGH", + }, nil + }, + }, + ) + + So(repoSummary.NewestImage.Vulnerabilities, ShouldNotBeNil) + So(*repoSummary.NewestImage.Vulnerabilities.Count, ShouldEqual, 1) + So(*repoSummary.NewestImage.Vulnerabilities.MaxSeverity, ShouldEqual, "HIGH") + So(graphql.GetErrors(ctx), ShouldBeNil) + }) + + Convey("Add CVE Summary to ManifestSummary", func() { + var manifestSummary *gql_generated.ManifestSummary + + So(manifestSummary, ShouldBeNil) + + updateManifestSummaryVulnerabilities(ctx, + manifestSummary, + "repo1", + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{ + Count: 1, + MaxSeverity: "HIGH", + }, nil + }, + }, + ) + + So(manifestSummary, ShouldBeNil) + So(graphql.GetErrors(ctx), ShouldBeNil) + + imageSummary, _, err := ImageManifest2ImageSummary(ctx, "repo1", "0.1.0", digest11, reposMeta[0], + manifestMetaMap[digest11.String()]) + So(err, ShouldBeNil) + manifestSummary = imageSummary.Manifests[0] + + updateManifestSummaryVulnerabilities(ctx, + manifestSummary, + "repo1", + SkipQGLField{ + Vulnerabilities: true, + }, + mocks.CveInfoMock{}, + ) + + So(manifestSummary, ShouldNotBeNil) + So(manifestSummary.Vulnerabilities, ShouldNotBeNil) + So(*manifestSummary.Vulnerabilities.Count, ShouldEqual, 0) + So(*manifestSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "") + + manifestSummary.Vulnerabilities = nil + + updateManifestSummaryVulnerabilities(ctx, + manifestSummary, + "repo1", + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{ + Count: 1, + MaxSeverity: "HIGH", + }, nil + }, + }, + ) + + So(manifestSummary.Vulnerabilities, ShouldNotBeNil) + So(*manifestSummary.Vulnerabilities.Count, ShouldEqual, 1) + So(*manifestSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "HIGH") + + manifestSummary.Vulnerabilities = nil + + updateManifestSummaryVulnerabilities(ctx, + manifestSummary, + "repo1", + SkipQGLField{ + Vulnerabilities: false, + }, + mocks.CveInfoMock{ + GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, + ) (cvemodel.ImageCVESummary, error) { + return cvemodel.ImageCVESummary{}, ErrTestError + }, + }, + ) + + So(manifestSummary.Vulnerabilities, ShouldNotBeNil) + So(*manifestSummary.Vulnerabilities.Count, ShouldEqual, 0) + So(*manifestSummary.Vulnerabilities.MaxSeverity, ShouldEqual, "") + So(graphql.GetErrors(ctx).Error(), ShouldContainSubstring, "unable to run vulnerability scan in repo") + }) + }) +} diff --git a/pkg/extensions/search/convert/convert_test.go b/pkg/extensions/search/convert/convert_test.go index 1ceea2e0a..62f7a0d41 100644 --- a/pkg/extensions/search/convert/convert_test.go +++ b/pkg/extensions/search/convert/convert_test.go @@ -1,3 +1,5 @@ +//go:build search + package convert_test import ( @@ -17,7 +19,6 @@ import ( "zotregistry.io/zot/pkg/extensions/search/gql_generated" "zotregistry.io/zot/pkg/extensions/search/pagination" "zotregistry.io/zot/pkg/log" - "zotregistry.io/zot/pkg/meta/boltdb" mTypes "zotregistry.io/zot/pkg/meta/types" "zotregistry.io/zot/pkg/test" . "zotregistry.io/zot/pkg/test/image-utils" @@ -27,64 +28,6 @@ import ( var ErrTestError = errors.New("TestError") func TestConvertErrors(t *testing.T) { - Convey("Convert Errors", t, func() { - params := boltdb.DBParameters{ - RootDir: t.TempDir(), - } - boltDB, err := boltdb.GetBoltDriver(params) - So(err, ShouldBeNil) - - metaDB, err := boltdb.New(boltDB, log.NewLogger("debug", "")) - So(err, ShouldBeNil) - - configBlob, err := json.Marshal(ispec.Image{}) - So(err, ShouldBeNil) - - manifestBlob, err := json.Marshal(ispec.Manifest{ - Layers: []ispec.Descriptor{ - { - MediaType: ispec.MediaTypeImageLayerGzip, - Size: 0, - Digest: godigest.NewDigestFromEncoded(godigest.SHA256, "digest"), - }, - }, - }) - So(err, ShouldBeNil) - - repoMeta11 := mTypes.ManifestMetadata{ - ManifestBlob: manifestBlob, - ConfigBlob: configBlob, - } - - digest11 := godigest.FromString("abc1") - err = metaDB.SetManifestMeta("repo1", digest11, repoMeta11) - So(err, ShouldBeNil) - err = metaDB.SetRepoReference("repo1", "0.1.0", digest11, ispec.MediaTypeImageManifest) - So(err, ShouldBeNil) - - reposMeta, manifestMetaMap, _, err := metaDB.SearchRepos(context.Background(), "") - So(err, ShouldBeNil) - - ctx := graphql.WithResponseContext(context.Background(), - graphql.DefaultErrorPresenter, graphql.DefaultRecover) - - _ = convert.RepoMeta2RepoSummary( - ctx, - reposMeta[0], - manifestMetaMap, - map[string]mTypes.IndexData{}, - convert.SkipQGLField{}, - mocks.CveInfoMock{ - GetCVESummaryForImageMediaFn: func(repo string, digest, mediaType string, - ) (cvemodel.ImageCVESummary, error) { - return cvemodel.ImageCVESummary{}, ErrTestError - }, - }, - ) - - So(graphql.GetErrors(ctx).Error(), ShouldContainSubstring, "unable to run vulnerability scan on tag") - }) - Convey("ImageIndex2ImageSummary errors", t, func() { ctx := graphql.WithResponseContext(context.Background(), graphql.DefaultErrorPresenter, graphql.DefaultRecover) @@ -94,13 +37,11 @@ func TestConvertErrors(t *testing.T) { "repo", "tag", godigest.FromString("indexDigest"), - true, mTypes.RepoMetadata{}, mTypes.IndexData{ IndexBlob: []byte("bad json"), }, map[string]mTypes.ManifestMetadata{}, - mocks.CveInfoMock{}, ) So(err, ShouldNotBeNil) }) @@ -114,17 +55,11 @@ func TestConvertErrors(t *testing.T) { "repo", "tag", godigest.FromString("indexDigest"), - false, mTypes.RepoMetadata{}, mTypes.IndexData{ IndexBlob: []byte("{}"), }, map[string]mTypes.ManifestMetadata{}, - mocks.CveInfoMock{ - GetCVESummaryForImageMediaFn: func(repo, digest, mediaType string) (cvemodel.ImageCVESummary, error) { - return cvemodel.ImageCVESummary{}, ErrTestError - }, - }, ) So(err, ShouldBeNil) }) @@ -146,17 +81,11 @@ func TestConvertErrors(t *testing.T) { "repo", "tag", godigest.FromString("manifestDigest"), - false, mTypes.RepoMetadata{}, mTypes.ManifestMetadata{ ManifestBlob: []byte("{}"), ConfigBlob: configBlob, }, - mocks.CveInfoMock{ - GetCVESummaryForImageMediaFn: func(repo, digest, mediaType string) (cvemodel.ImageCVESummary, error) { - return cvemodel.ImageCVESummary{}, ErrTestError - }, - }, ) So(err, ShouldBeNil) }) @@ -166,7 +95,7 @@ func TestConvertErrors(t *testing.T) { graphql.DefaultErrorPresenter, graphql.DefaultRecover) // with bad config json, shouldn't error when unmarshaling - _, _, err := convert.ImageManifest2ManifestSummary( + _, _, _, err := convert.ImageManifest2ManifestSummary( ctx, "repo", "tag", @@ -174,7 +103,6 @@ func TestConvertErrors(t *testing.T) { Digest: "dig", MediaType: ispec.MediaTypeImageManifest, }, - false, mTypes.RepoMetadata{ Tags: map[string]mTypes.Descriptor{}, Statistics: map[string]mTypes.DescriptorStatistics{}, @@ -186,7 +114,6 @@ func TestConvertErrors(t *testing.T) { ConfigBlob: []byte("bad json"), }, nil, - mocks.CveInfoMock{}, ) So(err, ShouldBeNil) @@ -200,7 +127,7 @@ func TestConvertErrors(t *testing.T) { }) So(err, ShouldBeNil) - _, _, err = convert.ImageManifest2ManifestSummary( + _, _, _, err = convert.ImageManifest2ManifestSummary( ctx, "repo", "tag", @@ -208,7 +135,6 @@ func TestConvertErrors(t *testing.T) { Digest: "dig", MediaType: ispec.MediaTypeImageManifest, }, - false, mTypes.RepoMetadata{ Tags: map[string]mTypes.Descriptor{}, Statistics: map[string]mTypes.DescriptorStatistics{}, @@ -220,11 +146,6 @@ func TestConvertErrors(t *testing.T) { ConfigBlob: configBlob, }, nil, - mocks.CveInfoMock{ - GetCVESummaryForImageMediaFn: func(repo, digest, mediaType string) (cvemodel.ImageCVESummary, error) { - return cvemodel.ImageCVESummary{}, ErrTestError - }, - }, ) So(err, ShouldBeNil) }) @@ -772,37 +693,7 @@ func TestPaginatedConvert(t *testing.T) { }) } -func TestGetOneManifestAnnotations(t *testing.T) { - Convey("GetOneManifestAnnotations errors", t, func() { - manifestAnnotations, configLabels := convert.GetOneManifestAnnotations( - ispec.Index{Manifests: []ispec.Descriptor{ - {Digest: "bad-manifest"}, {Digest: "dig2"}, - }}, - map[string]mTypes.ManifestMetadata{ - "bad-manifest": { - ManifestBlob: []byte(`bad`), - ConfigBlob: []byte("{}"), - }, - }, - ) - So(manifestAnnotations, ShouldBeEmpty) - So(configLabels, ShouldBeEmpty) - - manifestAnnotations, configLabels = convert.GetOneManifestAnnotations( - ispec.Index{Manifests: []ispec.Descriptor{ - {Digest: "bad-config"}, - }}, - map[string]mTypes.ManifestMetadata{ - "bad-config": { - ManifestBlob: []byte("{}"), - ConfigBlob: []byte("bad"), - }, - }, - ) - So(manifestAnnotations, ShouldBeEmpty) - So(configLabels, ShouldBeEmpty) - }) - +func TestIndexAnnotations(t *testing.T) { Convey("Test ImageIndex2ImageSummary annotations logic", t, func() { ctx := context.Background() @@ -864,8 +755,8 @@ func TestGetOneManifestAnnotations(t *testing.T) { digest := indexWithAnnotations.Digest() - imageSummary, _, err := convert.ImageIndex2ImageSummary(ctx, "repo", "tag", digest, true, repoMeta[0], - indexData[digest.String()], manifestMetadata, nil) + imageSummary, _, err := convert.ImageIndex2ImageSummary(ctx, "repo", "tag", digest, repoMeta[0], + indexData[digest.String()], manifestMetadata) So(err, ShouldBeNil) So(*imageSummary.Description, ShouldResemble, "IndexDescription") So(*imageSummary.Licenses, ShouldResemble, "IndexLicenses") @@ -887,7 +778,7 @@ func TestGetOneManifestAnnotations(t *testing.T) { digest = indexWithManifestAndConfigAnnotations.Digest() imageSummary, _, err = convert.ImageIndex2ImageSummary(ctx, "repo", "tag", digest, - true, repoMeta[0], indexData[digest.String()], manifestMetadata, nil) + repoMeta[0], indexData[digest.String()], manifestMetadata) So(err, ShouldBeNil) So(*imageSummary.Description, ShouldResemble, "ManifestDescription") So(*imageSummary.Licenses, ShouldResemble, "ManifestLicenses") @@ -908,7 +799,7 @@ func TestGetOneManifestAnnotations(t *testing.T) { digest = indexWithConfigAnnotations.Digest() imageSummary, _, err = convert.ImageIndex2ImageSummary(ctx, "repo", "tag", digest, - true, repoMeta[0], indexData[digest.String()], manifestMetadata, nil) + repoMeta[0], indexData[digest.String()], manifestMetadata) So(err, ShouldBeNil) So(*imageSummary.Description, ShouldResemble, "ConfigDescription") So(*imageSummary.Licenses, ShouldResemble, "ConfigLicenses") @@ -950,7 +841,7 @@ func TestGetOneManifestAnnotations(t *testing.T) { digest = indexWithMixAnnotations.Digest() imageSummary, _, err = convert.ImageIndex2ImageSummary(ctx, "repo", "tag", digest, - true, repoMeta[0], indexData[digest.String()], manifestMetadata, nil) + repoMeta[0], indexData[digest.String()], manifestMetadata) So(err, ShouldBeNil) So(*imageSummary.Description, ShouldResemble, "ConfigDescription") So(*imageSummary.Licenses, ShouldResemble, "ConfigLicenses") @@ -970,7 +861,7 @@ func TestGetOneManifestAnnotations(t *testing.T) { digest = indexWithNoAnnotations.Digest() imageSummary, _, err = convert.ImageIndex2ImageSummary(ctx, "repo", "tag", digest, - true, repoMeta[0], indexData[digest.String()], manifestMetadata, nil) + repoMeta[0], indexData[digest.String()], manifestMetadata) So(err, ShouldBeNil) So(*imageSummary.Description, ShouldBeBlank) So(*imageSummary.Licenses, ShouldBeBlank) @@ -999,8 +890,7 @@ func TestDownloadCount(t *testing.T) { }, ) - repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap, - convert.SkipQGLField{}, nil) + repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap) So(*repoSummary.DownloadCount, ShouldEqual, 10) So(*repoSummary.NewestImage.DownloadCount, ShouldEqual, 10) }) @@ -1027,8 +917,7 @@ func TestDownloadCount(t *testing.T) { }, ) - repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap, - convert.SkipQGLField{}, nil) + repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap) So(*repoSummary.DownloadCount, ShouldEqual, 100) So(*repoSummary.NewestImage.DownloadCount, ShouldEqual, 100) }) @@ -1067,8 +956,7 @@ func TestDownloadCount(t *testing.T) { }, ) - repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap, - convert.SkipQGLField{}, nil) + repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap) So(*repoSummary.DownloadCount, ShouldEqual, 105) So(*repoSummary.NewestImage.DownloadCount, ShouldEqual, 100) }) diff --git a/pkg/extensions/search/convert/cve.go b/pkg/extensions/search/convert/cve.go new file mode 100644 index 000000000..b266f3f1e --- /dev/null +++ b/pkg/extensions/search/convert/cve.go @@ -0,0 +1,109 @@ +package convert + +import ( + "context" + + "github.com/99designs/gqlgen/graphql" + ispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/vektah/gqlparser/v2/gqlerror" + + cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" + cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" + "zotregistry.io/zot/pkg/extensions/search/gql_generated" +) + +func updateRepoSummaryVulnerabilities( + ctx context.Context, + repoSummary *gql_generated.RepoSummary, + skip SkipQGLField, + cveInfo cveinfo.CveInfo, +) { + if repoSummary == nil { + return + } + + updateImageSummaryVulnerabilities(ctx, repoSummary.NewestImage, skip, cveInfo) +} + +func updateImageSummaryVulnerabilities( + ctx context.Context, + imageSummary *gql_generated.ImageSummary, + skip SkipQGLField, + cveInfo cveinfo.CveInfo, +) { + if imageSummary == nil { + return + } + + imageCveSummary := cvemodel.ImageCVESummary{} + + imageSummary.Vulnerabilities = &gql_generated.ImageVulnerabilitySummary{ + MaxSeverity: &imageCveSummary.MaxSeverity, + Count: &imageCveSummary.Count, + } + + // Check if vulnerability scanning is disabled + if cveInfo == nil || skip.Vulnerabilities { + return + } + + imageCveSummary, err := cveInfo.GetCVESummaryForImageMedia(*imageSummary.RepoName, *imageSummary.Digest, + *imageSummary.MediaType) + if err != nil { + // Log the error, but we should still include the image in results + graphql.AddError( + ctx, + gqlerror.Errorf( + "unable to run vulnerability scan on tag %s in repo %s: error: %s", + *imageSummary.Tag, *imageSummary.RepoName, err.Error(), + ), + ) + } + + imageSummary.Vulnerabilities.MaxSeverity = &imageCveSummary.MaxSeverity + imageSummary.Vulnerabilities.Count = &imageCveSummary.Count + + for _, manifestSummary := range imageSummary.Manifests { + updateManifestSummaryVulnerabilities(ctx, manifestSummary, *imageSummary.RepoName, skip, cveInfo) + } +} + +func updateManifestSummaryVulnerabilities( + ctx context.Context, + manifestSummary *gql_generated.ManifestSummary, + repoName string, + skip SkipQGLField, + cveInfo cveinfo.CveInfo, +) { + if manifestSummary == nil { + return + } + + imageCveSummary := cvemodel.ImageCVESummary{} + + manifestSummary.Vulnerabilities = &gql_generated.ImageVulnerabilitySummary{ + MaxSeverity: &imageCveSummary.MaxSeverity, + Count: &imageCveSummary.Count, + } + + // Check if vulnerability scanning is disabled + if cveInfo == nil || skip.Vulnerabilities { + return + } + + imageCveSummary, err := cveInfo.GetCVESummaryForImageMedia(repoName, *manifestSummary.Digest, + ispec.MediaTypeImageManifest) + if err != nil { + // Log the error, but we should still include the manifest in results + graphql.AddError( + ctx, + gqlerror.Errorf( + "unable to run vulnerability scan in repo %s: manifest digest: %s, error: %s", + repoName, *manifestSummary.Digest, err.Error(), + ), + ) + } + + manifestSummary.Vulnerabilities.MaxSeverity = &imageCveSummary.MaxSeverity + manifestSummary.Vulnerabilities.Count = &imageCveSummary.Count +} diff --git a/pkg/extensions/search/convert/metadb.go b/pkg/extensions/search/convert/metadb.go index 7a2f7061f..5ff8f0329 100644 --- a/pkg/extensions/search/convert/metadb.go +++ b/pkg/extensions/search/convert/metadb.go @@ -17,7 +17,6 @@ import ( zerr "zotregistry.io/zot/errors" zcommon "zotregistry.io/zot/pkg/common" cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" - cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" "zotregistry.io/zot/pkg/extensions/search/gql_generated" "zotregistry.io/zot/pkg/extensions/search/pagination" "zotregistry.io/zot/pkg/log" @@ -31,7 +30,6 @@ type SkipQGLField struct { func RepoMeta2RepoSummary(ctx context.Context, repoMeta mTypes.RepoMetadata, manifestMetaMap map[string]mTypes.ManifestMetadata, indexDataMap map[string]mTypes.IndexData, - skip SkipQGLField, cveInfo cveinfo.CveInfo, ) *gql_generated.RepoSummary { var ( repoName = repoMeta.Name @@ -53,8 +51,9 @@ func RepoMeta2RepoSummary(ctx context.Context, repoMeta mTypes.RepoMetadata, ) for tag, descriptor := range repoMeta.Tags { - imageSummary, imageBlobsMap, err := Descriptor2ImageSummary(ctx, descriptor, repoMeta.Name, tag, true, repoMeta, - manifestMetaMap, indexDataMap, cveInfo) + imageSummary, imageBlobsMap, err := Descriptor2ImageSummary(ctx, descriptor, repoMeta.Name, + tag, repoMeta, manifestMetaMap, indexDataMap, + ) if err != nil { continue } @@ -101,28 +100,6 @@ func RepoMeta2RepoSummary(ctx context.Context, repoMeta mTypes.RepoMetadata, repoVendors = append(repoVendors, &vendor) } - // We only scan the latest image on the repo for performance reasons - // Check if vulnerability scanning is disabled - if cveInfo != nil && lastUpdatedImageSummary != nil && !skip.Vulnerabilities { - imageCveSummary, err := cveInfo.GetCVESummaryForImageMedia(repoMeta.Name, *lastUpdatedImageSummary.Digest, - *lastUpdatedImageSummary.MediaType) - if err != nil { - // Log the error, but we should still include the image in results - graphql.AddError( - ctx, - gqlerror.Errorf( - "unable to run vulnerability scan on tag %s in repo %s: error: %s", - *lastUpdatedImageSummary.Tag, repoMeta.Name, err.Error(), - ), - ) - } - - lastUpdatedImageSummary.Vulnerabilities = &gql_generated.ImageVulnerabilitySummary{ - MaxSeverity: &imageCveSummary.MaxSeverity, - Count: &imageCveSummary.Count, - } - } - return &gql_generated.RepoSummary{ Name: &repoName, LastUpdated: &repoLastUpdatedTimestamp, @@ -148,7 +125,7 @@ func PaginatedRepoMeta2RepoSummaries(ctx context.Context, reposMeta []mTypes.Rep } for _, repoMeta := range reposMeta { - repoSummary := RepoMeta2RepoSummary(ctx, repoMeta, manifestMetaMap, indexDataMap, skip, cveInfo) + repoSummary := RepoMeta2RepoSummary(ctx, repoMeta, manifestMetaMap, indexDataMap) if RepoSumAcceptedByFilter(repoSummary, filter) { reposPageFinder.Add(repoSummary) @@ -157,6 +134,11 @@ func PaginatedRepoMeta2RepoSummaries(ctx context.Context, reposMeta []mTypes.Rep page, pageInfo := reposPageFinder.Page() + // CVE scanning is expensive, only scan for the current page + for _, repoSummary := range page { + updateRepoSummaryVulnerabilities(ctx, repoSummary, skip, cveInfo) + } + return page, pageInfo, nil } @@ -177,25 +159,24 @@ func UpdateLastUpdatedTimestamp(repoLastUpdatedTimestamp *time.Time, return newLastUpdatedImageSummary } -func Descriptor2ImageSummary(ctx context.Context, descriptor mTypes.Descriptor, repo, tag string, skipCVE bool, +func Descriptor2ImageSummary(ctx context.Context, descriptor mTypes.Descriptor, repo, tag string, repoMeta mTypes.RepoMetadata, manifestMetaMap map[string]mTypes.ManifestMetadata, - indexDataMap map[string]mTypes.IndexData, cveInfo cveinfo.CveInfo, + indexDataMap map[string]mTypes.IndexData, ) (*gql_generated.ImageSummary, map[string]int64, error) { switch descriptor.MediaType { case ispec.MediaTypeImageManifest: - return ImageManifest2ImageSummary(ctx, repo, tag, godigest.Digest(descriptor.Digest), skipCVE, - repoMeta, manifestMetaMap[descriptor.Digest], cveInfo) + return ImageManifest2ImageSummary(ctx, repo, tag, godigest.Digest(descriptor.Digest), + repoMeta, manifestMetaMap[descriptor.Digest]) case ispec.MediaTypeImageIndex: - return ImageIndex2ImageSummary(ctx, repo, tag, godigest.Digest(descriptor.Digest), skipCVE, - repoMeta, indexDataMap[descriptor.Digest], manifestMetaMap, cveInfo) + return ImageIndex2ImageSummary(ctx, repo, tag, godigest.Digest(descriptor.Digest), + repoMeta, indexDataMap[descriptor.Digest], manifestMetaMap) default: return &gql_generated.ImageSummary{}, map[string]int64{}, zerr.ErrMediaTypeNotSupported } } -func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest godigest.Digest, skipCVE bool, +func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest godigest.Digest, repoMeta mTypes.RepoMetadata, indexData mTypes.IndexData, manifestMetaMap map[string]mTypes.ManifestMetadata, - cveInfo cveinfo.CveInfo, ) (*gql_generated.ImageSummary, map[string]int64, error) { var indexContent ispec.Index @@ -205,22 +186,22 @@ func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest } var ( - indexLastUpdated time.Time - isSigned bool - totalIndexSize int64 - indexSize string - totalDownloadCount int - maxSeverity string - manifestSummaries = make([]*gql_generated.ManifestSummary, 0, len(indexContent.Manifests)) - indexBlobs = make(map[string]int64, 0) + indexLastUpdated time.Time + isSigned bool + totalIndexSize int64 + indexSize string + totalDownloadCount int + manifestAnnotations *ImageAnnotations + manifestSummaries = make([]*gql_generated.ManifestSummary, 0, len(indexContent.Manifests)) + indexBlobs = make(map[string]int64, 0) indexDigestStr = indexDigest.String() indexMediaType = ispec.MediaTypeImageIndex ) for _, descriptor := range indexContent.Manifests { - manifestSummary, manifestBlobs, err := ImageManifest2ManifestSummary(ctx, repo, tag, descriptor, false, - repoMeta, manifestMetaMap[descriptor.Digest.String()], repoMeta.Referrers[descriptor.Digest.String()], cveInfo) + manifestSummary, manifestBlobs, annotations, err := ImageManifest2ManifestSummary(ctx, repo, tag, descriptor, + repoMeta, manifestMetaMap[descriptor.Digest.String()], repoMeta.Referrers[descriptor.Digest.String()]) if err != nil { return &gql_generated.ImageSummary{}, map[string]int64{}, err } @@ -236,13 +217,12 @@ func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest indexLastUpdated = *manifestSummary.LastUpdated } - totalIndexSize += manifestSize - - if cvemodel.SeverityValue(*manifestSummary.Vulnerabilities.MaxSeverity) > - cvemodel.SeverityValue(maxSeverity) { - maxSeverity = *manifestSummary.Vulnerabilities.MaxSeverity + if manifestAnnotations == nil { + manifestAnnotations = annotations } + totalIndexSize += manifestSize + manifestSummaries = append(manifestSummaries, manifestSummary) } @@ -254,24 +234,16 @@ func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest } } - imageCveSummary := cvemodel.ImageCVESummary{} - - if cveInfo != nil && !skipCVE { - imageCveSummary, err = cveInfo.GetCVESummaryForImageMedia(repo, indexDigestStr, ispec.MediaTypeImageIndex) - - if err != nil { - // Log the error, but we should still include the manifest in results - graphql.AddError(ctx, gqlerror.Errorf("unable to run vulnerability scan on tag %s in repo %s: "+ - "manifest digest: %s, error: %s", tag, repo, indexDigest, err.Error())) - } - } - indexSize = strconv.FormatInt(totalIndexSize, 10) signaturesInfo := GetSignaturesInfo(isSigned, repoMeta, indexDigest) - manifestAnnotations, configLabels := GetOneManifestAnnotations(indexContent, manifestMetaMap) - annotations := GetIndexAnnotations(indexContent.Annotations, manifestAnnotations, configLabels) + if manifestAnnotations == nil { + // The index doesn't have manifests + manifestAnnotations = &ImageAnnotations{} + } + + annotations := GetIndexAnnotations(indexContent.Annotations, manifestAnnotations) indexSummary := gql_generated.ImageSummary{ RepoName: &repo, @@ -292,42 +264,14 @@ func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest Source: &annotations.Source, Vendor: &annotations.Vendor, Authors: &annotations.Authors, - Vulnerabilities: &gql_generated.ImageVulnerabilitySummary{ - MaxSeverity: &imageCveSummary.MaxSeverity, - Count: &imageCveSummary.Count, - }, - Referrers: getReferrers(repoMeta.Referrers[indexDigest.String()]), + Referrers: getReferrers(repoMeta.Referrers[indexDigest.String()]), } return &indexSummary, indexBlobs, nil } -func GetOneManifestAnnotations(indexContent ispec.Index, manifestMetaMap map[string]mTypes.ManifestMetadata, -) (map[string]string, map[string]string) { - if len(manifestMetaMap) == 0 || len(indexContent.Manifests) == 0 { - return map[string]string{}, map[string]string{} - } - - manifestMeta := manifestMetaMap[indexContent.Manifests[0].Digest.String()] - - manifestContent := ispec.Manifest{} - configContent := ispec.Image{} - - err := json.Unmarshal(manifestMeta.ManifestBlob, &manifestContent) - if err != nil { - return map[string]string{}, map[string]string{} - } - - err = json.Unmarshal(manifestMeta.ConfigBlob, &configContent) - if err != nil { - return map[string]string{}, map[string]string{} - } - - return manifestContent.Annotations, configContent.Config.Labels -} - -func ImageManifest2ImageSummary(ctx context.Context, repo, tag string, digest godigest.Digest, skipCVE bool, - repoMeta mTypes.RepoMetadata, manifestMeta mTypes.ManifestMetadata, cveInfo cveinfo.CveInfo, +func ImageManifest2ImageSummary(ctx context.Context, repo, tag string, digest godigest.Digest, + repoMeta mTypes.RepoMetadata, manifestMeta mTypes.ManifestMetadata, ) (*gql_generated.ImageSummary, map[string]int64, error) { var ( manifestContent ispec.Manifest @@ -390,45 +334,29 @@ func ImageManifest2ImageSummary(ctx context.Context, repo, tag string, digest go "manifest digest: %s, error: %s", tag, repo, manifestDigest, err.Error())) } - imageCveSummary := cvemodel.ImageCVESummary{} - - if cveInfo != nil && !skipCVE { - imageCveSummary, err = cveInfo.GetCVESummaryForImageMedia(repo, manifestDigest, ispec.MediaTypeImageManifest) + signaturesInfo := GetSignaturesInfo(isSigned, repoMeta, digest) - if err != nil { - // Log the error, but we should still include the manifest in results - graphql.AddError(ctx, gqlerror.Errorf("unable to run vulnerability scan on tag %s in repo %s: "+ - "manifest digest: %s, error: %s", tag, repo, manifestDigest, err.Error())) - } + manifestSummary := gql_generated.ManifestSummary{ + Digest: &manifestDigest, + ConfigDigest: &configDigest, + LastUpdated: &imageLastUpdated, + Size: &imageSize, + IsSigned: &isSigned, + SignatureInfo: signaturesInfo, + Platform: &platform, + DownloadCount: &downloadCount, + Layers: getLayersSummaries(manifestContent), + History: historyEntries, + Referrers: getReferrers(repoMeta.Referrers[manifestDigest]), + ArtifactType: &artifactType, } - signaturesInfo := GetSignaturesInfo(isSigned, repoMeta, digest) - imageSummary := gql_generated.ImageSummary{ - RepoName: &repoName, - Tag: &tag, - Digest: &manifestDigest, - MediaType: &mediaType, - Manifests: []*gql_generated.ManifestSummary{ - { - Digest: &manifestDigest, - ConfigDigest: &configDigest, - LastUpdated: &imageLastUpdated, - Size: &imageSize, - IsSigned: &isSigned, - SignatureInfo: signaturesInfo, - Platform: &platform, - DownloadCount: &downloadCount, - Layers: getLayersSummaries(manifestContent), - History: historyEntries, - Vulnerabilities: &gql_generated.ImageVulnerabilitySummary{ - MaxSeverity: &imageCveSummary.MaxSeverity, - Count: &imageCveSummary.Count, - }, - Referrers: getReferrers(repoMeta.Referrers[manifestDigest]), - ArtifactType: &artifactType, - }, - }, + RepoName: &repoName, + Tag: &tag, + Digest: &manifestDigest, + MediaType: &mediaType, + Manifests: []*gql_generated.ManifestSummary{&manifestSummary}, LastUpdated: &imageLastUpdated, IsSigned: &isSigned, SignatureInfo: signaturesInfo, @@ -442,11 +370,7 @@ func ImageManifest2ImageSummary(ctx context.Context, repo, tag string, digest go Source: &annotations.Source, Vendor: &annotations.Vendor, Authors: &authors, - Vulnerabilities: &gql_generated.ImageVulnerabilitySummary{ - MaxSeverity: &imageCveSummary.MaxSeverity, - Count: &imageCveSummary.Count, - }, - Referrers: getReferrers(repoMeta.Referrers[manifestDigest]), + Referrers: getReferrers(repoMeta.Referrers[manifestDigest]), } return &imageSummary, imageBlobsMap, nil @@ -487,9 +411,9 @@ func getAnnotationsFromMap(annotationsMap map[string]string) []*gql_generated.An } func ImageManifest2ManifestSummary(ctx context.Context, repo, tag string, descriptor ispec.Descriptor, - skipCVE bool, repoMeta mTypes.RepoMetadata, manifestMeta mTypes.ManifestMetadata, - referrersInfo []mTypes.ReferrerInfo, cveInfo cveinfo.CveInfo, -) (*gql_generated.ManifestSummary, map[string]int64, error) { + repoMeta mTypes.RepoMetadata, manifestMeta mTypes.ManifestMetadata, + referrersInfo []mTypes.ReferrerInfo, +) (*gql_generated.ManifestSummary, map[string]int64, *ImageAnnotations, error) { var ( manifestContent ispec.Manifest digest = descriptor.Digest @@ -500,10 +424,11 @@ func ImageManifest2ManifestSummary(ctx context.Context, repo, tag string, descri graphql.AddError(ctx, gqlerror.Errorf("can't unmarshal manifest blob for image: %s:%s, manifest digest: %s, "+ "error: %s", repo, tag, digest, err.Error())) - return &gql_generated.ManifestSummary{}, map[string]int64{}, err + return &gql_generated.ManifestSummary{}, map[string]int64{}, &ImageAnnotations{}, err } configContent := mcommon.InitializeImageConfig(manifestMeta.ConfigBlob) + annotations := GetAnnotations(manifestContent.Annotations, configContent.Config.Labels) var ( manifestDigestStr = digest.String() @@ -537,18 +462,6 @@ func ImageManifest2ManifestSummary(ctx context.Context, repo, tag string, descri "manifest digest: %s, error: %s", tag, repo, manifestDigestStr, err.Error())) } - imageCveSummary := cvemodel.ImageCVESummary{} - - if cveInfo != nil && !skipCVE { - imageCveSummary, err = cveInfo.GetCVESummaryForImageMedia(repo, manifestDigestStr, ispec.MediaTypeImageManifest) - - if err != nil { - // Log the error, but we should still include the manifest in results - graphql.AddError(ctx, gqlerror.Errorf("unable to run vulnerability scan on tag %s in repo %s: "+ - "manifest digest: %s, error: %s", tag, repo, manifestDigestStr, err.Error())) - } - } - for _, signatures := range repoMeta.Signatures[manifestDigestStr] { if len(signatures) > 0 { isSigned = true @@ -568,15 +481,11 @@ func ImageManifest2ManifestSummary(ctx context.Context, repo, tag string, descri History: historyEntries, IsSigned: &isSigned, SignatureInfo: signaturesInfo, - Vulnerabilities: &gql_generated.ImageVulnerabilitySummary{ - MaxSeverity: &imageCveSummary.MaxSeverity, - Count: &imageCveSummary.Count, - }, - Referrers: getReferrers(referrersInfo), - ArtifactType: &artifactType, + Referrers: getReferrers(referrersInfo), + ArtifactType: &artifactType, } - return &manifestSummary, imageBlobsMap, nil + return &manifestSummary, imageBlobsMap, &annotations, nil } func getImageBlobsInfo(manifestDigest string, manifestSize int64, configDigest string, configSize int64, @@ -622,12 +531,15 @@ func RepoMeta2ImageSummaries(ctx context.Context, repoMeta mTypes.RepoMetadata, for _, tag := range tags { descriptor := repoMeta.Tags[tag] - imageSummary, _, err := Descriptor2ImageSummary(ctx, descriptor, repoMeta.Name, tag, skip.Vulnerabilities, - repoMeta, manifestMetaMap, indexDataMap, cveInfo) + imageSummary, _, err := Descriptor2ImageSummary(ctx, descriptor, repoMeta.Name, tag, + repoMeta, manifestMetaMap, indexDataMap) if err != nil { continue } + // CVE scanning is expensive, only scan for final slice of results + updateImageSummaryVulnerabilities(ctx, imageSummary, skip, cveInfo) + imageSummaries = append(imageSummaries, imageSummary) } @@ -647,8 +559,8 @@ func PaginatedRepoMeta2ImageSummaries(ctx context.Context, reposMeta []mTypes.Re for tag := range repoMeta.Tags { descriptor := repoMeta.Tags[tag] - imageSummary, _, err := Descriptor2ImageSummary(ctx, descriptor, repoMeta.Name, tag, skip.Vulnerabilities, - repoMeta, manifestMetaMap, indexDataMap, cveInfo) + imageSummary, _, err := Descriptor2ImageSummary(ctx, descriptor, repoMeta.Name, tag, + repoMeta, manifestMetaMap, indexDataMap) if err != nil { continue } @@ -661,6 +573,11 @@ func PaginatedRepoMeta2ImageSummaries(ctx context.Context, reposMeta []mTypes.Re page, pageInfo := imagePageFinder.Page() + for _, imageSummary := range page { + // CVE scanning is expensive, only scan for this page + updateImageSummaryVulnerabilities(ctx, imageSummary, skip, cveInfo) + } + return page, pageInfo, nil } @@ -691,7 +608,7 @@ func RepoMeta2ExpandedRepoInfo(ctx context.Context, repoMeta mTypes.RepoMetadata for tag, descriptor := range repoMeta.Tags { imageSummary, imageBlobs, err := Descriptor2ImageSummary(ctx, descriptor, repoName, tag, - skip.Vulnerabilities, repoMeta, manifestMetaMap, indexDataMap, cveInfo) + repoMeta, manifestMetaMap, indexDataMap) if err != nil { log.Error().Str("repository", repoName).Str("reference", tag). Msg("metadb: error while converting descriptor for image") @@ -713,6 +630,8 @@ func RepoMeta2ExpandedRepoInfo(ctx context.Context, repoMeta mTypes.RepoMetadata repoVendorsSet[*imageSummary.Vendor] = true } + updateImageSummaryVulnerabilities(ctx, imageSummary, skip, cveInfo) + lastUpdatedImageSummary = UpdateLastUpdatedTimestamp(&repoLastUpdatedTimestamp, lastUpdatedImageSummary, imageSummary) repoDownloadCount += *imageSummary.DownloadCount @@ -739,27 +658,6 @@ func RepoMeta2ExpandedRepoInfo(ctx context.Context, repoMeta mTypes.RepoMetadata vendor := vendor repoVendors = append(repoVendors, &vendor) } - // We only scan the latest image on the repo for performance reasons - // Check if vulnerability scanning is disabled - if cveInfo != nil && lastUpdatedImageSummary != nil && !skip.Vulnerabilities { - imageCveSummary, err := cveInfo.GetCVESummaryForImageMedia(repoMeta.Name, *lastUpdatedImageSummary.Digest, - *lastUpdatedImageSummary.MediaType) - if err != nil { - // Log the error, but we should still include the image in results - graphql.AddError( - ctx, - gqlerror.Errorf( - "unable to run vulnerability scan on tag %s in repo %s: error: %s", - *lastUpdatedImageSummary.Tag, repoMeta.Name, err.Error(), - ), - ) - } - - lastUpdatedImageSummary.Vulnerabilities = &gql_generated.ImageVulnerabilitySummary{ - MaxSeverity: &imageCveSummary.MaxSeverity, - Count: &imageCveSummary.Count, - } - } summary := &gql_generated.RepoSummary{ Name: &repoName, @@ -774,6 +672,8 @@ func RepoMeta2ExpandedRepoInfo(ctx context.Context, repoMeta mTypes.RepoMetadata IsStarred: &isStarred, } + updateRepoSummaryVulnerabilities(ctx, summary, skip, cveInfo) + return summary, imageSummaries } diff --git a/pkg/extensions/search/cve/cve.go b/pkg/extensions/search/cve/cve.go index 7a4607808..ae9df83d5 100644 --- a/pkg/extensions/search/cve/cve.go +++ b/pkg/extensions/search/cve/cve.go @@ -24,7 +24,6 @@ type CveInfo interface { ) ([]cvemodel.CVE, zcommon.PageInfo, error) GetCVESummaryForImage(repo, ref string) (cvemodel.ImageCVESummary, error) GetCVESummaryForImageMedia(repo, digest, mediaType string) (cvemodel.ImageCVESummary, error) - CompareSeverities(severity1, severity2 string) int UpdateDB() error } @@ -32,7 +31,6 @@ type Scanner interface { ScanImage(image string) (map[string]cvemodel.CVE, error) IsImageFormatScannable(repo, ref string) (bool, error) IsImageMediaScannable(repo, digestStr, mediaType string) (bool, error) - CompareSeverities(severity1, severity2 string) int UpdateDB() error } @@ -347,7 +345,7 @@ func (cveinfo BaseCveInfo) GetCVEListForImage(repo, ref string, searchedCVE stri return []cvemodel.CVE{}, zcommon.PageInfo{}, err } - pageFinder, err := NewCvePageFinder(pageInput.Limit, pageInput.Offset, pageInput.SortBy, cveinfo) + pageFinder, err := NewCvePageFinder(pageInput.Limit, pageInput.Offset, pageInput.SortBy) if err != nil { return []cvemodel.CVE{}, zcommon.PageInfo{}, err } @@ -366,7 +364,7 @@ func (cveinfo BaseCveInfo) GetCVESummaryForImage(repo, ref string) (cvemodel.Ima // scannable issues found - max severity from Scanner - cve count >0 - no Errors imageCVESummary := cvemodel.ImageCVESummary{ Count: 0, - MaxSeverity: "", + MaxSeverity: cvemodel.SeverityNotScanned, } isValidImage, err := cveinfo.Scanner.IsImageFormatScannable(repo, ref) @@ -384,14 +382,14 @@ func (cveinfo BaseCveInfo) GetCVESummaryForImage(repo, ref string) (cvemodel.Ima imageCVESummary.Count = len(cveMap) if imageCVESummary.Count == 0 { - imageCVESummary.MaxSeverity = "NONE" + imageCVESummary.MaxSeverity = cvemodel.SeverityNone return imageCVESummary, nil } - imageCVESummary.MaxSeverity = "UNKNOWN" + imageCVESummary.MaxSeverity = cvemodel.SeverityUnknown for _, cve := range cveMap { - if cveinfo.Scanner.CompareSeverities(imageCVESummary.MaxSeverity, cve.Severity) > 0 { + if cvemodel.CompareSeverities(imageCVESummary.MaxSeverity, cve.Severity) > 0 { imageCVESummary.MaxSeverity = cve.Severity } } @@ -401,9 +399,13 @@ func (cveinfo BaseCveInfo) GetCVESummaryForImage(repo, ref string) (cvemodel.Ima func (cveinfo BaseCveInfo) GetCVESummaryForImageMedia(repo, digest, mediaType string, ) (cvemodel.ImageCVESummary, error) { + // There are several cases, expected returned values below: + // not scannable / error during scan - max severity "" - cve count 0 - Errors + // scannable no issues found - max severity "NONE" - cve count 0 - no Errors + // scannable issues found - max severity from Scanner - cve count >0 - no Errors imageCVESummary := cvemodel.ImageCVESummary{ Count: 0, - MaxSeverity: "", + MaxSeverity: cvemodel.SeverityNotScanned, } isValidImage, err := cveinfo.Scanner.IsImageMediaScannable(repo, digest, mediaType) @@ -421,14 +423,14 @@ func (cveinfo BaseCveInfo) GetCVESummaryForImageMedia(repo, digest, mediaType st imageCVESummary.Count = len(cveMap) if imageCVESummary.Count == 0 { - imageCVESummary.MaxSeverity = "NONE" + imageCVESummary.MaxSeverity = cvemodel.SeverityNone return imageCVESummary, nil } - imageCVESummary.MaxSeverity = "UNKNOWN" + imageCVESummary.MaxSeverity = cvemodel.SeverityUnknown for _, cve := range cveMap { - if cveinfo.Scanner.CompareSeverities(imageCVESummary.MaxSeverity, cve.Severity) > 0 { + if cvemodel.CompareSeverities(imageCVESummary.MaxSeverity, cve.Severity) > 0 { imageCVESummary.MaxSeverity = cve.Severity } } @@ -440,10 +442,6 @@ func (cveinfo BaseCveInfo) UpdateDB() error { return cveinfo.Scanner.UpdateDB() } -func (cveinfo BaseCveInfo) CompareSeverities(severity1, severity2 string) int { - return cveinfo.Scanner.CompareSeverities(severity1, severity2) -} - func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo { sort.Slice(allTags, func(i, j int) bool { return allTags[i].Timestamp.Before(allTags[j].Timestamp) diff --git a/pkg/extensions/search/cve/cve_internal_test.go b/pkg/extensions/search/cve/cve_internal_test.go index a890b9672..c4cbc1d2f 100644 --- a/pkg/extensions/search/cve/cve_internal_test.go +++ b/pkg/extensions/search/cve/cve_internal_test.go @@ -1,3 +1,5 @@ +//go:build search + package cveinfo import ( diff --git a/pkg/extensions/search/cve/cve_test.go b/pkg/extensions/search/cve/cve_test.go index 38a019757..1637df5d9 100644 --- a/pkg/extensions/search/cve/cve_test.go +++ b/pkg/extensions/search/cve/cve_test.go @@ -1029,15 +1029,7 @@ func TestCVEStruct(t *testing.T) { err = metaDB.SetRepoReference("repoIndex", "tagIndex", indexDigest, ispec.MediaTypeImageIndex) So(err, ShouldBeNil) - // MetaDB loaded with initial data, mock the scanner - severities := map[string]int{ - "UNKNOWN": 0, - "LOW": 1, - "MEDIUM": 2, - "HIGH": 3, - "CRITICAL": 4, - } - + // MetaDB loaded with initial data, now mock the scanner // Setup test CVE data in mock scanner scanner := mocks.CveScannerMock{ ScanImageFn: func(image string) (map[string]cvemodel.CVE, error) { @@ -1124,9 +1116,6 @@ func TestCVEStruct(t *testing.T) { // 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(repo string, reference string) (bool, error) { if repo == "repoIndex" { return true, nil diff --git a/pkg/extensions/search/cve/model/models.go b/pkg/extensions/search/cve/model/models.go index 857fe8b1a..28aee3266 100644 --- a/pkg/extensions/search/cve/model/models.go +++ b/pkg/extensions/search/cve/model/models.go @@ -28,23 +28,50 @@ type Package struct { } const ( - None = iota - Low - Medium - High - Critical + unScanned = iota + none + unknown + low + medium + high + critical ) -func SeverityValue(severity string) int { +// Values from https://www.first.org/cvss/v3.0/specification-document +const ( + SeverityNotScanned = "" // scanning was not done or was not complete + SeverityNone = "NONE" // no vulnerabilities were detected at all + SeverityUnknown = "UNKNOWN" // coresponds to CVSS 3 score NONE + SeverityLow = "LOW" // coresponds to CVSS 3 score LOW + SeverityMedium = "MEDIUM" // coresponds to CVSS 3 score MEDIUM + SeverityHigh = "HIGH" // coresponds to CVSS 3 score HIGH + SeverityCritical = "CRITICAL" // coresponds to CVSS 3 score CRITICAL +) + +func severityInt(severity string) int { sevMap := map[string]int{ - "NONE": None, - "LOW": Low, - "MEDIUM": Medium, - "HIGH": High, - "CRITICAL": Critical, + SeverityNotScanned: unScanned, + SeverityNone: none, + SeverityUnknown: unknown, + SeverityLow: low, + SeverityMedium: medium, + SeverityHigh: high, + SeverityCritical: critical, + } + + severityInt, ok := sevMap[severity] + + if !ok { + // In the unlikely case the key is not in the map we + // return the unknown severity level + return unknown } - return sevMap[severity] + return severityInt +} + +func CompareSeverities(sev1, sev2 string) int { + return severityInt(sev2) - severityInt(sev1) } type Descriptor struct { diff --git a/pkg/extensions/search/cve/pagination.go b/pkg/extensions/search/cve/pagination.go index 046942f97..e6fe04ca4 100644 --- a/pkg/extensions/search/cve/pagination.go +++ b/pkg/extensions/search/cve/pagination.go @@ -15,33 +15,33 @@ const ( SeverityDsc = cvemodel.SortCriteria("SEVERITY") ) -func SortFunctions() map[cvemodel.SortCriteria]func(pageBuffer []cvemodel.CVE, cveInfo CveInfo) func(i, j int) bool { - return map[cvemodel.SortCriteria]func(pageBuffer []cvemodel.CVE, cveInfo CveInfo) func(i, j int) bool{ +func SortFunctions() map[cvemodel.SortCriteria]func(pageBuffer []cvemodel.CVE) func(i, j int) bool { + return map[cvemodel.SortCriteria]func(pageBuffer []cvemodel.CVE) func(i, j int) bool{ AlphabeticAsc: SortByAlphabeticAsc, AlphabeticDsc: SortByAlphabeticDsc, SeverityDsc: SortBySeverity, } } -func SortByAlphabeticAsc(pageBuffer []cvemodel.CVE, cveInfo CveInfo) func(i, j int) bool { +func SortByAlphabeticAsc(pageBuffer []cvemodel.CVE) func(i, j int) bool { return func(i, j int) bool { return pageBuffer[i].ID < pageBuffer[j].ID } } -func SortByAlphabeticDsc(pageBuffer []cvemodel.CVE, cveInfo CveInfo) func(i, j int) bool { +func SortByAlphabeticDsc(pageBuffer []cvemodel.CVE) func(i, j int) bool { return func(i, j int) bool { return pageBuffer[i].ID > pageBuffer[j].ID } } -func SortBySeverity(pageBuffer []cvemodel.CVE, cveInfo CveInfo) func(i, j int) bool { +func SortBySeverity(pageBuffer []cvemodel.CVE) func(i, j int) bool { return func(i, j int) bool { - if cveInfo.CompareSeverities(pageBuffer[i].Severity, pageBuffer[j].Severity) == 0 { + if cvemodel.CompareSeverities(pageBuffer[i].Severity, pageBuffer[j].Severity) == 0 { return pageBuffer[i].ID < pageBuffer[j].ID } - return cveInfo.CompareSeverities(pageBuffer[i].Severity, pageBuffer[j].Severity) < 0 + return cvemodel.CompareSeverities(pageBuffer[i].Severity, pageBuffer[j].Severity) < 0 } } @@ -60,10 +60,9 @@ type CvePageFinder struct { offset int sortBy cvemodel.SortCriteria pageBuffer []cvemodel.CVE - cveInfo CveInfo } -func NewCvePageFinder(limit, offset int, sortBy cvemodel.SortCriteria, cveInfo CveInfo) (*CvePageFinder, error) { +func NewCvePageFinder(limit, offset int, sortBy cvemodel.SortCriteria) (*CvePageFinder, error) { if sortBy == "" { sortBy = SeverityDsc } @@ -85,7 +84,6 @@ func NewCvePageFinder(limit, offset int, sortBy cvemodel.SortCriteria, cveInfo C offset: offset, sortBy: sortBy, pageBuffer: make([]cvemodel.CVE, 0, limit), - cveInfo: cveInfo, }, nil } @@ -104,7 +102,7 @@ func (bpt *CvePageFinder) Page() ([]cvemodel.CVE, common.PageInfo) { pageInfo := &common.PageInfo{} - sort.Slice(bpt.pageBuffer, SortFunctions()[bpt.sortBy](bpt.pageBuffer, bpt.cveInfo)) + sort.Slice(bpt.pageBuffer, SortFunctions()[bpt.sortBy](bpt.pageBuffer)) // the offset and limit are calculated in terms of CVEs counted start := bpt.offset diff --git a/pkg/extensions/search/cve/pagination_test.go b/pkg/extensions/search/cve/pagination_test.go index 7cadd7230..ab518c2cd 100644 --- a/pkg/extensions/search/cve/pagination_test.go +++ b/pkg/extensions/search/cve/pagination_test.go @@ -146,30 +146,27 @@ func TestCVEPagination(t *testing.T) { // By default the image has no vulnerabilities return cveMap, nil }, - CompareSeveritiesFn: func(severity1, severity2 string) int { - return severityToInt[severity2] - severityToInt[severity1] - }, } log := log.NewLogger("debug", "") cveInfo := cveinfo.BaseCveInfo{Log: log, Scanner: scanner, MetaDB: metaDB} Convey("create new paginator errors", func() { - paginator, err := cveinfo.NewCvePageFinder(-1, 10, cveinfo.AlphabeticAsc, cveInfo) + paginator, err := cveinfo.NewCvePageFinder(-1, 10, cveinfo.AlphabeticAsc) So(paginator, ShouldBeNil) So(err, ShouldNotBeNil) - paginator, err = cveinfo.NewCvePageFinder(2, -1, cveinfo.AlphabeticAsc, cveInfo) + paginator, err = cveinfo.NewCvePageFinder(2, -1, cveinfo.AlphabeticAsc) So(paginator, ShouldBeNil) So(err, ShouldNotBeNil) - paginator, err = cveinfo.NewCvePageFinder(2, 1, "wrong sorting criteria", cveInfo) + paginator, err = cveinfo.NewCvePageFinder(2, 1, "wrong sorting criteria") So(paginator, ShouldBeNil) So(err, ShouldNotBeNil) }) Convey("Reset", func() { - paginator, err := cveinfo.NewCvePageFinder(1, 0, cveinfo.AlphabeticAsc, cveInfo) + paginator, err := cveinfo.NewCvePageFinder(1, 0, cveinfo.AlphabeticAsc) So(err, ShouldBeNil) So(paginator, ShouldNotBeNil) diff --git a/pkg/extensions/search/cve/trivy/scanner.go b/pkg/extensions/search/cve/trivy/scanner.go index 0326cf612..e952d6316 100644 --- a/pkg/extensions/search/cve/trivy/scanner.go +++ b/pkg/extensions/search/cve/trivy/scanner.go @@ -30,6 +30,8 @@ import ( "zotregistry.io/zot/pkg/storage" ) +const cacheSize = 1000000 + // getNewScanOptions sets trivy configuration values for our scans and returns them as // a trivy Options structure. func getNewScanOptions(dir, dbRepository, javaDBRepository string) *flag.Options { @@ -118,7 +120,7 @@ func NewScanner(storeController storage.StoreController, cveController: cveController, storeController: storeController, dbLock: &sync.Mutex{}, - cache: NewCveCache(10000, log), //nolint:gomnd + cache: NewCveCache(cacheSize, log), dbRepository: dbRepository, javaDBRepository: javaDBRepository, } @@ -415,7 +417,7 @@ func (scanner Scanner) scanManifest(repo, digest string) (map[string]cvemodel.CV ID: vulnerability.VulnerabilityID, Title: vulnerability.Title, Description: vulnerability.Description, - Severity: vulnerability.Severity, + Severity: convertSeverity(vulnerability.Severity), PackageList: newPkgList, } } @@ -552,6 +554,16 @@ func (scanner Scanner) checkDBPresence() error { return nil } -func (scanner Scanner) CompareSeverities(severity1, severity2 string) int { - return dbTypes.CompareSeverityString(severity1, severity2) +func convertSeverity(detectedSeverity string) string { + trivySeverity, _ := dbTypes.NewSeverity(detectedSeverity) + + sevMap := map[dbTypes.Severity]string{ + dbTypes.SeverityUnknown: cvemodel.SeverityUnknown, + dbTypes.SeverityLow: cvemodel.SeverityLow, + dbTypes.SeverityMedium: cvemodel.SeverityMedium, + dbTypes.SeverityHigh: cvemodel.SeverityHigh, + dbTypes.SeverityCritical: cvemodel.SeverityCritical, + } + + return sevMap[trivySeverity] } diff --git a/pkg/extensions/search/cve/trivy/scanner_test.go b/pkg/extensions/search/cve/trivy/scanner_test.go index 28043fc50..bb336c41d 100644 --- a/pkg/extensions/search/cve/trivy/scanner_test.go +++ b/pkg/extensions/search/cve/trivy/scanner_test.go @@ -1,3 +1,5 @@ +//go:build search + package trivy_test import ( diff --git a/pkg/extensions/search/pagination/pagination_test.go b/pkg/extensions/search/pagination/pagination_test.go index 312ef3d8e..2d2f74a7d 100644 --- a/pkg/extensions/search/pagination/pagination_test.go +++ b/pkg/extensions/search/pagination/pagination_test.go @@ -1,3 +1,5 @@ +//go:build search + package pagination_test import ( diff --git a/pkg/extensions/search/resolver.go b/pkg/extensions/search/resolver.go index 94850de9b..5bbd0daab 100644 --- a/pkg/extensions/search/resolver.go +++ b/pkg/extensions/search/resolver.go @@ -152,8 +152,8 @@ func getImageListForDigest(ctx context.Context, digest string, metaDB mTypes.Met }, nil } -func getImageSummary(ctx context.Context, repo, tag string, digest *string, metaDB mTypes.MetaDB, - cveInfo cveinfo.CveInfo, log log.Logger, //nolint:unparam +func getImageSummary(ctx context.Context, repo, tag string, digest *string, skipCVE convert.SkipQGLField, + metaDB mTypes.MetaDB, cveInfo cveinfo.CveInfo, log log.Logger, //nolint:unparam ) ( *gql_generated.ImageSummary, error, ) { @@ -268,10 +268,7 @@ func getImageSummary(ctx context.Context, repo, tag string, digest *string, meta log.Error().Str("mediaType", manifestDescriptor.MediaType).Msg("resolver: media type not supported") } - skip := convert.SkipQGLField{ - Vulnerabilities: canSkipField(convert.GetPreloads(ctx), "Vulnerabilities"), - } - imageSummaries := convert.RepoMeta2ImageSummaries(ctx, repoMeta, manifestMetaMap, indexDataMap, skip, cveInfo) + imageSummaries := convert.RepoMeta2ImageSummaries(ctx, repoMeta, manifestMetaMap, indexDataMap, skipCVE, cveInfo) if len(imageSummaries) == 0 { return &gql_generated.ImageSummary{}, nil @@ -809,7 +806,11 @@ func derivedImageList(ctx context.Context, image string, digest *string, metaDB return &gql_generated.PaginatedImagesResult{}, gqlerror.Errorf("no reference provided") } - searchedImage, err := getImageSummary(ctx, imageRepo, imageTag, digest, metaDB, cveInfo, log) + skipReferenceImage := convert.SkipQGLField{ + Vulnerabilities: true, + } + + searchedImage, err := getImageSummary(ctx, imageRepo, imageTag, digest, skipReferenceImage, metaDB, cveInfo, log) if err != nil { if errors.Is(err, zerr.ErrRepoMetaNotFound) { return &gql_generated.PaginatedImagesResult{}, gqlerror.Errorf("repository: not found") @@ -911,7 +912,11 @@ func baseImageList(ctx context.Context, image string, digest *string, metaDB mTy return &gql_generated.PaginatedImagesResult{}, gqlerror.Errorf("no reference provided") } - searchedImage, err := getImageSummary(ctx, imageRepo, imageTag, digest, metaDB, cveInfo, log) + skipReferenceImage := convert.SkipQGLField{ + Vulnerabilities: true, + } + + searchedImage, err := getImageSummary(ctx, imageRepo, imageTag, digest, skipReferenceImage, metaDB, cveInfo, log) if err != nil { if errors.Is(err, zerr.ErrRepoMetaNotFound) { return &gql_generated.PaginatedImagesResult{}, gqlerror.Errorf("repository: not found") diff --git a/pkg/extensions/search/resolver_test.go b/pkg/extensions/search/resolver_test.go index b052c387d..29a108c82 100644 --- a/pkg/extensions/search/resolver_test.go +++ b/pkg/extensions/search/resolver_test.go @@ -1,3 +1,5 @@ +//go:build search + package search //nolint import ( @@ -16,6 +18,7 @@ import ( . "github.com/smartystreets/goconvey/convey" "zotregistry.io/zot/pkg/common" + "zotregistry.io/zot/pkg/extensions/search/convert" cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" "zotregistry.io/zot/pkg/extensions/search/gql_generated" @@ -1182,9 +1185,13 @@ func TestGetImageSummary(t *testing.T) { } log = log.NewLogger("debug", "") + + skip = convert.SkipQGLField{ + Vulnerabilities: true, + } ) - _, err := getImageSummary(responseContext, "repo", "tag", nil, metaDB, mocks.CveInfoMock{}, log) + _, err := getImageSummary(responseContext, "repo", "tag", nil, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldNotBeNil) }) @@ -1201,9 +1208,13 @@ func TestGetImageSummary(t *testing.T) { } log = log.NewLogger("debug", "") + + skip = convert.SkipQGLField{ + Vulnerabilities: true, + } ) - _, err := getImageSummary(responseContext, "repo", "tag", nil, metaDB, mocks.CveInfoMock{}, log) + _, err := getImageSummary(responseContext, "repo", "tag", nil, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldBeNil) }) @@ -1225,9 +1236,13 @@ func TestGetImageSummary(t *testing.T) { log = log.NewLogger("debug", "") digest = "wrongDigest" + + skip = convert.SkipQGLField{ + Vulnerabilities: true, + } ) - _, err := getImageSummary(responseContext, "repo", "tag", &digest, metaDB, mocks.CveInfoMock{}, log) + _, err := getImageSummary(responseContext, "repo", "tag", &digest, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldNotBeNil) }) }) @@ -1249,9 +1264,13 @@ func TestGetImageSummary(t *testing.T) { } log = log.NewLogger("debug", "") + + skip = convert.SkipQGLField{ + Vulnerabilities: true, + } ) - _, err := getImageSummary(responseContext, "repo", "tag", nil, metaDB, mocks.CveInfoMock{}, log) + _, err := getImageSummary(responseContext, "repo", "tag", nil, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldNotBeNil) }) @@ -1273,9 +1292,13 @@ func TestGetImageSummary(t *testing.T) { } log = log.NewLogger("debug", "") + + skip = convert.SkipQGLField{ + Vulnerabilities: true, + } ) - _, err := getImageSummary(responseContext, "repo", "tag", nil, metaDB, mocks.CveInfoMock{}, log) + _, err := getImageSummary(responseContext, "repo", "tag", nil, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldNotBeNil) }) @@ -1313,7 +1336,12 @@ func TestGetImageSummary(t *testing.T) { Convey("digest not found", func() { wrongDigest := "wrongDigest" - _, err = getImageSummary(responseContext, "repo", "tag", &wrongDigest, metaDB, mocks.CveInfoMock{}, log) + + skip := convert.SkipQGLField{ + Vulnerabilities: true, + } + + _, err = getImageSummary(responseContext, "repo", "tag", &wrongDigest, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldNotBeNil) }) @@ -1322,7 +1350,11 @@ func TestGetImageSummary(t *testing.T) { return mTypes.ManifestData{}, ErrTestError } - _, err = getImageSummary(responseContext, "repo", "tag", &goodDigest, metaDB, mocks.CveInfoMock{}, log) + skip := convert.SkipQGLField{ + Vulnerabilities: true, + } + + _, err = getImageSummary(responseContext, "repo", "tag", &goodDigest, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldNotBeNil) }) }) @@ -1341,9 +1373,13 @@ func TestGetImageSummary(t *testing.T) { } log = log.NewLogger("debug", "") + + skip = convert.SkipQGLField{ + Vulnerabilities: true, + } ) - _, err := getImageSummary(responseContext, "repo", "tag", nil, metaDB, mocks.CveInfoMock{}, log) + _, err := getImageSummary(responseContext, "repo", "tag", nil, skip, metaDB, mocks.CveInfoMock{}, log) So(err, ShouldBeNil) }) }) @@ -2042,17 +2078,7 @@ func TestCVEResolvers(t *testing.T) { //nolint:gocyclo } } - // Create the repo metadata using previously defined manifests - - // MetaDB loaded with initial data, mock the scanner - severities := map[string]int{ - "UNKNOWN": 0, - "LOW": 1, - "MEDIUM": 2, - "HIGH": 3, - "CRITICAL": 4, - } - + // MetaDB loaded with initial data, now mock the scanner // Setup test CVE data in mock scanner scanner := mocks.CveScannerMock{ ScanImageFn: func(image string) (map[string]cvemodel.CVE, error) { @@ -2126,9 +2152,6 @@ func TestCVEResolvers(t *testing.T) { //nolint:gocyclo // By default the image has no vulnerabilities return map[string]cvemodel.CVE{}, nil }, - CompareSeveritiesFn: func(severity1, severity2 string) int { - return severities[severity2] - severities[severity1] - }, } cveInfo := &cveinfo.BaseCveInfo{ diff --git a/pkg/extensions/search/schema.resolvers.go b/pkg/extensions/search/schema.resolvers.go index 8c09d53b0..cd70f2b1f 100644 --- a/pkg/extensions/search/schema.resolvers.go +++ b/pkg/extensions/search/schema.resolvers.go @@ -10,6 +10,7 @@ import ( "github.com/vektah/gqlparser/v2/gqlerror" zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/common" + "zotregistry.io/zot/pkg/extensions/search/convert" "zotregistry.io/zot/pkg/extensions/search/gql_generated" ) @@ -133,7 +134,11 @@ func (r *queryResolver) Image(ctx context.Context, image string) (*gql_generated return &gql_generated.ImageSummary{}, gqlerror.Errorf("no reference provided") } - return getImageSummary(ctx, repo, tag, nil, r.metaDB, r.cveInfo, r.log) + skip := convert.SkipQGLField{ + Vulnerabilities: canSkipField(convert.GetPreloads(ctx), "Vulnerabilities"), + } + + return getImageSummary(ctx, repo, tag, nil, skip, r.metaDB, r.cveInfo, r.log) } // Referrers is the resolver for the Referrers field. diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index aa92ee5ad..33a8d8580 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -222,14 +222,6 @@ func uploadNewRepoTag(tag string, repoName string, baseURL string, layers [][]by func getMockCveInfo(metaDB mTypes.MetaDB, log log.Logger) cveinfo.CveInfo { // MetaDB 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) { @@ -310,9 +302,6 @@ func getMockCveInfo(metaDB mTypes.MetaDB, log log.Logger) cveinfo.CveInfo { // 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(repo string, reference string) (bool, error) { // Almost same logic compared to actual Trivy specific implementation imageDir := repo diff --git a/pkg/meta/boltdb/boltdb.go b/pkg/meta/boltdb/boltdb.go index 1b23c0aae..fc4e850f2 100644 --- a/pkg/meta/boltdb/boltdb.go +++ b/pkg/meta/boltdb/boltdb.go @@ -465,7 +465,7 @@ func (bdw *BoltDB) SetRepoReference(repo string, reference string, manifestDiges func (bdw *BoltDB) GetRepoMeta(repo string) (mTypes.RepoMetadata, error) { var repoMeta mTypes.RepoMetadata - err := bdw.DB.Update(func(tx *bbolt.Tx) error { + err := bdw.DB.View(func(tx *bbolt.Tx) error { buck := tx.Bucket([]byte(RepoMetadataBucket)) repoMetaBlob := buck.Get([]byte(repo)) @@ -490,7 +490,7 @@ func (bdw *BoltDB) GetRepoMeta(repo string) (mTypes.RepoMetadata, error) { func (bdw *BoltDB) GetUserRepoMeta(ctx context.Context, repo string) (mTypes.RepoMetadata, error) { var repoMeta mTypes.RepoMetadata - err := bdw.DB.Update(func(tx *bbolt.Tx) error { + err := bdw.DB.View(func(tx *bbolt.Tx) error { buck := tx.Bucket([]byte(RepoMetadataBucket)) userBookmarks := getUserBookmarks(ctx, tx) userStars := getUserStars(ctx, tx) diff --git a/pkg/meta/parse.go b/pkg/meta/parse.go index b9c9652c2..d9317f65c 100644 --- a/pkg/meta/parse.go +++ b/pkg/meta/parse.go @@ -20,6 +20,8 @@ import ( // ParseStorage will sync all repos found in the rootdirectory of the oci layout that zot was deployed on with the // ParseStorage database. func ParseStorage(metaDB mTypes.MetaDB, storeController storage.StoreController, log log.Logger) error { + log.Info().Msg("Started parsing storage and updating MetaDB") + allRepos, err := getAllRepos(storeController) if err != nil { rootDir := storeController.DefaultStore.RootDir() @@ -38,6 +40,8 @@ func ParseStorage(metaDB mTypes.MetaDB, storeController storage.StoreController, } } + log.Info().Msg("Done parsing storage and updating MetaDB") + return nil } diff --git a/pkg/test/mocks/cve_mock.go b/pkg/test/mocks/cve_mock.go index 83e01cfa9..1cd460f81 100644 --- a/pkg/test/mocks/cve_mock.go +++ b/pkg/test/mocks/cve_mock.go @@ -14,8 +14,7 @@ type CveInfoMock struct { ) (cvemodel.ImageCVESummary, error) GetCVESummaryForImageMediaFn func(repo string, digest, mediaType string, ) (cvemodel.ImageCVESummary, error) - CompareSeveritiesFn func(severity1, severity2 string) int - UpdateDBFn func() error + UpdateDBFn func() error } func (cveInfo CveInfoMock) GetImageListForCVE(repo, cveID string) ([]cvemodel.TagInfo, error) { @@ -66,14 +65,6 @@ func (cveInfo CveInfoMock) GetCVESummaryForImageMedia(repo, digest, mediaType st return cvemodel.ImageCVESummary{}, nil } -func (cveInfo CveInfoMock) CompareSeverities(severity1, severity2 string) int { - if cveInfo.CompareSeveritiesFn != nil { - return cveInfo.CompareSeveritiesFn(severity1, severity2) - } - - return 0 -} - func (cveInfo CveInfoMock) UpdateDB() error { if cveInfo.UpdateDBFn != nil { return cveInfo.UpdateDBFn() @@ -86,7 +77,6 @@ type CveScannerMock struct { IsImageFormatScannableFn func(repo string, reference string) (bool, error) IsImageMediaScannableFn func(repo string, digest, mediaType string) (bool, error) ScanImageFn func(image string) (map[string]cvemodel.CVE, error) - CompareSeveritiesFn func(severity1, severity2 string) int UpdateDBFn func() error } @@ -114,14 +104,6 @@ func (scanner CveScannerMock) ScanImage(image string) (map[string]cvemodel.CVE, return map[string]cvemodel.CVE{}, nil } -func (scanner CveScannerMock) CompareSeverities(severity1, severity2 string) int { - if scanner.CompareSeveritiesFn != nil { - return scanner.CompareSeveritiesFn(severity1, severity2) - } - - return 0 -} - func (scanner CveScannerMock) UpdateDB() error { if scanner.UpdateDBFn != nil { return scanner.UpdateDBFn()