From 150ee889451a10a8b0c8b3b2a7c7fbd10ba6f456 Mon Sep 17 00:00:00 2001 From: LaurentiuNiculae Date: Wed, 15 Mar 2023 19:34:48 +0200 Subject: [PATCH] fix(repodb): GQL request for ExpandedRepoInfo errors when artifacts with tags are present (#1265) If we push an artifact and give it a tag, repodb would crash because of the null pointer dereferencing Now when iterating over the tags of a repo and stumbling upon a unsupported media type, it's being ignored Signed-off-by: Laurentiu Niculae --- errors/errors.go | 1 + pkg/extensions/search/common/common_test.go | 68 ++++++++++++++++++- pkg/extensions/search/convert/repodb.go | 3 +- .../repodb/boltdb-wrapper/boltdb_wrapper.go | 6 ++ .../repodb/dynamodb-wrapper/dynamo_wrapper.go | 6 ++ pkg/test/common.go | 9 ++- pkg/test/common_test.go | 2 +- 7 files changed, 89 insertions(+), 6 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 9e2228b8..d8ffcaa3 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -78,6 +78,7 @@ var ( ErrLimitIsNegative = errors.New("pageturner: limit has negative value") ErrOffsetIsNegative = errors.New("pageturner: offset has negative value") ErrSortCriteriaNotSupported = errors.New("pageturner: the sort criteria is not supported") + ErrMediaTypeNotSupported = errors.New("repodb: media type is not supported") ErrTimeout = errors.New("operation timeout") ErrNotImplemented = errors.New("not implemented") ) diff --git a/pkg/extensions/search/common/common_test.go b/pkg/extensions/search/common/common_test.go index 358c4a17..a91b2dbe 100644 --- a/pkg/extensions/search/common/common_test.go +++ b/pkg/extensions/search/common/common_test.go @@ -1018,7 +1018,7 @@ func TestGetReferrersGQL(t *testing.T) { So(err, ShouldBeNil) artifactManifestDigest := godigest.FromBytes(artifactManifestBlob) - err = UploadArtifactManifest(artifact, baseURL, repo) + err = UploadArtifactManifest(artifact, nil, baseURL, repo) So(err, ShouldBeNil) gqlQuery := ` @@ -1401,6 +1401,72 @@ func TestExpandedRepoInfo(t *testing.T) { So(err, ShouldBeNil) }) + Convey("Test expanded repo info with tagged referrers", t, func() { + rootDir := t.TempDir() + port := GetFreePort() + baseURL := GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + conf.Storage.RootDirectory = rootDir + conf.Storage.GC = false + defaultVal := true + conf.Extensions = &extconf.ExtensionConfig{ + Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}}, + } + + conf.Extensions.Search.CVE = nil + + ctlr := api.NewController(conf) + ctlrManager := NewControllerManager(ctlr) + ctlrManager.StartAndWait(port) + defer ctlrManager.StopServer() + + image, err := GetRandomImage("test") + So(err, ShouldBeNil) + manifestDigest, err := image.Digest() + So(err, ShouldBeNil) + + err = UploadImage(image, baseURL, "repo") + So(err, ShouldBeNil) + + referrer, err := GetRandomArtifact(&ispec.Descriptor{ + Digest: manifestDigest, + MediaType: ispec.MediaTypeImageManifest, + }) + So(err, ShouldBeNil) + + tag := "test-ref-tag" + err = UploadArtifactManifest(&referrer.Manifest, &tag, baseURL, "repo") + So(err, ShouldBeNil) + + // ------- Make the call to GQL and see that it doesn't crash and that the referrer isn't in the list of tags + responseStruct := &ExpandedRepoInfoResp{} + query := ` + { + ExpandedRepoInfo(repo:"repo"){ + Images { + RepoName + Tag + Manifests { + Digest + Layers {Size Digest} + } + } + } + }` + resp, err := resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 200) + + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + So(len(responseStruct.ExpandedRepoInfo.RepoInfo.ImageSummaries), ShouldEqual, 1) + + repoInfo := responseStruct.ExpandedRepoInfo.RepoInfo + So(repoInfo.ImageSummaries[0].Tag, ShouldEqual, "test") + }) + Convey("Test image tags order", t, func() { port := GetFreePort() baseURL := GetBaseURL(port) diff --git a/pkg/extensions/search/convert/repodb.go b/pkg/extensions/search/convert/repodb.go index 019eb35e..dea957fa 100644 --- a/pkg/extensions/search/convert/repodb.go +++ b/pkg/extensions/search/convert/repodb.go @@ -13,6 +13,7 @@ import ( ispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/vektah/gqlparser/v2/gqlerror" + zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/extensions/search/common" cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" cvemodel "zotregistry.io/zot/pkg/extensions/search/cve/model" @@ -164,7 +165,7 @@ func Descriptor2ImageSummary(ctx context.Context, descriptor repodb.Descriptor, return ImageIndex2ImageSummary(ctx, repo, tag, godigest.Digest(descriptor.Digest), skipCVE, repoMeta, indexDataMap[descriptor.Digest], manifestMetaMap, cveInfo) default: - return &gql_generated.ImageSummary{}, map[string]int64{}, nil + return &gql_generated.ImageSummary{}, map[string]int64{}, zerr.ErrMediaTypeNotSupported } } diff --git a/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go b/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go index 782c0ce2..9ce057cf 100644 --- a/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go +++ b/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go @@ -1091,6 +1091,8 @@ func (bdw *DBWrapper) SearchRepos(ctx context.Context, searchText string, filter indexDataMap[indexDigest] = indexData default: bdw.Log.Error().Msgf("Unsupported type: %s", descriptor.MediaType) + + continue } } @@ -1416,6 +1418,8 @@ func (bdw *DBWrapper) FilterTags(ctx context.Context, filter repodb.FilterFunc, indexDataMap[indexDigest] = indexData default: bdw.Log.Error().Msgf("Unsupported type: %s", descriptor.MediaType) + + continue } } @@ -1603,6 +1607,8 @@ func (bdw *DBWrapper) SearchTags(ctx context.Context, searchText string, filter indexDataMap[indexDigest] = indexData default: bdw.Log.Error().Msgf("Unsupported type: %s", descriptor.MediaType) + + continue } } diff --git a/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go b/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go index 913e0fe7..f6f55eaa 100644 --- a/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go +++ b/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go @@ -972,6 +972,8 @@ func (dwr *DBWrapper) SearchRepos(ctx context.Context, searchText string, filter indexDataMap[indexDigest] = indexData default: dwr.Log.Error().Msgf("Unsupported type: %s", descriptor.MediaType) + + continue } } @@ -1245,6 +1247,8 @@ func (dwr *DBWrapper) FilterTags(ctx context.Context, filter repodb.FilterFunc, indexDataMap[indexDigest] = indexData default: dwr.Log.Error().Msgf("Unsupported type: %s", descriptor.MediaType) + + continue } } @@ -1409,6 +1413,8 @@ func (dwr *DBWrapper) SearchTags(ctx context.Context, searchText string, filter indexDataMap[indexDigest] = indexData default: dwr.Log.Error().Msgf("Unsupported type: %s", descriptor.MediaType) + + continue } } diff --git a/pkg/test/common.go b/pkg/test/common.go index 26004561..461ff84f 100644 --- a/pkg/test/common.go +++ b/pkg/test/common.go @@ -958,19 +958,22 @@ func DeleteImage(repo, reference, baseURL string) (int, error) { } // UploadArtifactManifest is used in tests where we don't need to upload the blobs of the artifact. -func UploadArtifactManifest(artifactManifest *ispec.Artifact, baseURL, repo string) error { +func UploadArtifactManifest(artifactManifest *ispec.Artifact, ref *string, baseURL, repo string) error { // put manifest artifactManifestBlob, err := json.Marshal(artifactManifest) if err != nil { return err } + reference := godigest.FromBytes(artifactManifestBlob).String() - artifactManifestDigest := godigest.FromBytes(artifactManifestBlob) + if ref != nil { + reference = *ref + } _, err = resty.R(). SetHeader("Content-type", ispec.MediaTypeArtifactManifest). SetBody(artifactManifestBlob). - Put(baseURL + "/v2/" + repo + "/manifests/" + artifactManifestDigest.String()) + Put(baseURL + "/v2/" + repo + "/manifests/" + reference) return err } diff --git a/pkg/test/common_test.go b/pkg/test/common_test.go index 6a577a70..88a0cbb8 100644 --- a/pkg/test/common_test.go +++ b/pkg/test/common_test.go @@ -241,7 +241,7 @@ func TestUploadArtifact(t *testing.T) { artifact := ispec.Artifact{} - err := test.UploadArtifactManifest(&artifact, baseURL, "test") + err := test.UploadArtifactManifest(&artifact, nil, baseURL, "test") So(err, ShouldNotBeNil) }) }