From a5ed99178e3e35dc94d88f161aee9ecdc254fa1f Mon Sep 17 00:00:00 2001 From: Alex Stan Date: Tue, 2 Aug 2022 18:58:30 +0300 Subject: [PATCH] replace dependency of tagsInfo and repoInfo with just a list of manifests - replace dependency of tagsInfo and repoInfo with a list of manifests, since it provides all the needed data - Mock tests added Signed-off-by: Alex Stan --- pkg/extensions/search/common/oci_layout.go | 5 +- pkg/extensions/search/resolver.go | 70 +++++++------- pkg/extensions/search/resolver_test.go | 104 ++++++++++++++++----- pkg/test/mocks/oci_mock.go | 11 ++- 4 files changed, 124 insertions(+), 66 deletions(-) diff --git a/pkg/extensions/search/common/oci_layout.go b/pkg/extensions/search/common/oci_layout.go index 2e11d135..6702bf27 100644 --- a/pkg/extensions/search/common/oci_layout.go +++ b/pkg/extensions/search/common/oci_layout.go @@ -33,6 +33,7 @@ type OciLayoutUtils interface { GetRepoLastUpdated(repo string) (TagInfo, error) GetExpandedRepoInfo(name string) (RepoInfo, error) GetImageConfigInfo(repo string, manifestDigest godigest.Digest) (ispec.Image, error) + CheckManifestSignature(name string, digest godigest.Digest) bool } // OciLayoutInfo ... @@ -270,7 +271,7 @@ func (olu BaseOciLayoutUtils) checkCosignSignature(name string, digest godigest. // checks if manifest is signed or not // checks for notary or cosign signature // if cosign signature found it does not looks for notary signature. -func (olu BaseOciLayoutUtils) checkManifestSignature(name string, digest godigest.Digest) bool { +func (olu BaseOciLayoutUtils) CheckManifestSignature(name string, digest godigest.Digest) bool { if !olu.checkCosignSignature(name, digest) { return olu.checkNotarySignature(name, digest) } @@ -395,7 +396,7 @@ func (olu BaseOciLayoutUtils) GetExpandedRepoInfo(name string) (RepoInfo, error) return RepoInfo{}, err } - manifestInfo.IsSigned = olu.checkManifestSignature(name, man.Digest) + manifestInfo.IsSigned = olu.CheckManifestSignature(name, man.Digest) manifestSize := olu.GetImageManifestSize(name, man.Digest) olu.Log.Debug().Msg(fmt.Sprintf("%v", man.Digest)) diff --git a/pkg/extensions/search/resolver.go b/pkg/extensions/search/resolver.go index 38874ce8..a5b37a44 100644 --- a/pkg/extensions/search/resolver.go +++ b/pkg/extensions/search/resolver.go @@ -12,6 +12,7 @@ import ( godigest "github.com/opencontainers/go-digest" "zotregistry.io/zot/pkg/log" // nolint: gci + ispec "github.com/opencontainers/image-spec/specs-go/v1" "zotregistry.io/zot/pkg/extensions/search/common" cveinfo "zotregistry.io/zot/pkg/extensions/search/cve" digestinfo "zotregistry.io/zot/pkg/extensions/search/digest" @@ -216,63 +217,57 @@ func globalSearch(repoList []string, name, tag string, olu common.OciLayoutUtils log.Error().Err(err).Msgf("can't find latest updated tag for repo: %s", repo) } - tagsInfo, err := olu.GetImageTagsWithTimestamp(repo) + manifests, err := olu.GetImageManifests(repo) if err != nil { - log.Error().Err(err).Msgf("can't get tags info for repo: %s", repo) - - continue - } - - repoInfo, err := olu.GetExpandedRepoInfo(repo) - if err != nil { - log.Error().Err(err).Msgf("can't get repo info for repo: %s", repo) + log.Error().Err(err).Msgf("can't get manifests for repo: %s", repo) continue } var lastUpdatedImageSummary gql_generated.ImageSummary - repoPlatforms := make([]*gql_generated.OsArch, 0, len(tagsInfo)) - repoVendors := make([]*string, 0, len(repoInfo.Manifests)) + repoPlatforms := make([]*gql_generated.OsArch, 0, len(manifests)) + repoVendors := make([]*string, 0, len(manifests)) - for i, manifest := range repoInfo.Manifests { + for i, manifest := range manifests { imageLayersSize := int64(0) - imageBlobManifest, err := olu.GetImageBlobManifest(repo, godigest.Digest(tagsInfo[i].Digest)) - if err != nil { - log.Error().Err(err).Msgf("can't read manifest for repo %s %s", repo, manifest.Tag) + manifestTag, ok := manifest.Annotations[ispec.AnnotationRefName] + if !ok { + log.Error().Msg("reference not found for this manifest") continue } - manifestSize := olu.GetImageManifestSize(repo, godigest.Digest(tagsInfo[i].Digest)) + imageBlobManifest, err := olu.GetImageBlobManifest(repo, manifests[i].Digest) + if err != nil { + log.Error().Err(err).Msgf("can't read manifest for repo %s %s", repo, manifestTag) + + continue + } + + manifestSize := olu.GetImageManifestSize(repo, manifests[i].Digest) configSize := imageBlobManifest.Config.Size - repoBlob2Size[tagsInfo[i].Digest] = manifestSize + repoBlob2Size[manifests[i].Digest.String()] = manifestSize repoBlob2Size[imageBlobManifest.Config.Digest.Hex] = configSize - for _, layer := range manifest.Layers { + for _, layer := range imageBlobManifest.Layers { layer := layer - - layerSize, err := strconv.ParseInt(layer.Size, 10, 64) - if err != nil { - log.Error().Err(err).Msg("invalid layer size") - - continue - } - - repoBlob2Size[layer.Digest] = layerSize - imageLayersSize += layerSize + layerDigest := layer.Digest.String() + layerSizeStr := strconv.Itoa(int(layer.Size)) + repoBlob2Size[layer.Digest.String()] = layer.Size + imageLayersSize += layer.Size // if we have a tag we won't match a layer if tag != "" { continue } - if index := strings.Index(layer.Digest, name); index != -1 { + if index := strings.Index(layerDigest, name); index != -1 { layers = append(layers, &gql_generated.LayerSummary{ - Digest: &layer.Digest, - Size: &layer.Size, + Digest: &layerDigest, + Size: &layerSizeStr, Score: &index, }) } @@ -281,19 +276,18 @@ func globalSearch(repoList []string, name, tag string, olu common.OciLayoutUtils imageSize := imageLayersSize + manifestSize + configSize index := strings.Index(repo, name) - matchesTag := strings.HasPrefix(manifest.Tag, tag) + matchesTag := strings.HasPrefix(manifestTag, tag) if index != -1 { - imageConfigInfo, err := olu.GetImageConfigInfo(repo, godigest.Digest(tagsInfo[i].Digest)) + imageConfigInfo, err := olu.GetImageConfigInfo(repo, manifests[i].Digest) if err != nil { - log.Error().Err(err).Msgf("can't retrieve config info for the image %s %s", repo, manifest.Tag) + log.Error().Err(err).Msgf("can't retrieve config info for the image %s %s", repo, manifestTag) continue } - tag := manifest.Tag size := strconv.Itoa(int(imageSize)) - isSigned := manifest.IsSigned + isSigned := olu.CheckManifestSignature(repo, manifests[i].Digest) // update matching score score := calculateImageMatchingScore(repo, index, matchesTag) @@ -311,7 +305,7 @@ func globalSearch(repoList []string, name, tag string, olu common.OciLayoutUtils imageSummary := gql_generated.ImageSummary{ RepoName: &repo, - Tag: &tag, + Tag: &manifestTag, LastUpdated: &lastUpdated, IsSigned: &isSigned, Size: &size, @@ -320,7 +314,7 @@ func globalSearch(repoList []string, name, tag string, olu common.OciLayoutUtils Score: &score, } - if tagsInfo[i].Digest == lastUpdatedTag.Digest { + if manifests[i].Digest.String() == lastUpdatedTag.Digest { lastUpdatedImageSummary = imageSummary } diff --git a/pkg/extensions/search/resolver_test.go b/pkg/extensions/search/resolver_test.go index b094e918..b3422861 100644 --- a/pkg/extensions/search/resolver_test.go +++ b/pkg/extensions/search/resolver_test.go @@ -5,7 +5,9 @@ import ( "strings" "testing" + v1 "github.com/google/go-containerregistry/pkg/v1" godigest "github.com/opencontainers/go-digest" + ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" "zotregistry.io/zot/pkg/extensions/search/common" "zotregistry.io/zot/pkg/log" @@ -36,49 +38,101 @@ func TestGlobalSearch(t *testing.T) { globalSearch([]string{"repo1"}, "name", "tag", mockOlum, log.NewLogger("debug", "")) }) - Convey("GetExpandedRepoInfo fail", func() { + Convey("GetImageManifests fail", func() { mockOlum := mocks.OciLayoutUtilsMock{ - GetExpandedRepoInfoFn: func(name string) (common.RepoInfo, error) { - return common.RepoInfo{}, ErrTestError + GetImageManifestsFn: func(name string) ([]ispec.Descriptor, error) { + return []ispec.Descriptor{}, ErrTestError }, } globalSearch([]string{"repo1"}, "name", "tag", mockOlum, log.NewLogger("debug", "")) }) - Convey("Bad layer digest in manifest", func() { + Convey("Manifests given, bad image blob manifest", func() { mockOlum := mocks.OciLayoutUtilsMock{ - GetExpandedRepoInfoFn: func(name string) (common.RepoInfo, error) { - return common.RepoInfo{ - Manifests: []common.Manifest{ - { - Tag: "latest", - Layers: []common.Layer{ - { - Size: "this is a bad size format", - Digest: "digest", - }, - }, + GetImageManifestsFn: func(name string) ([]ispec.Descriptor, error) { + return []ispec.Descriptor{ + { + Digest: "digest", + Size: -1, + Annotations: map[string]string{ + ispec.AnnotationRefName: "this is a bad format", }, }, }, nil }, - GetImageManifestSizeFn: func(repo string, manifestDigest godigest.Digest) int64 { - return 100 + GetImageBlobManifestFn: func(imageDir string, digest godigest.Digest) (v1.Manifest, error) { + return v1.Manifest{}, ErrTestError }, - GetImageConfigSizeFn: func(repo string, manifestDigest godigest.Digest) int64 { - return 100 - }, - GetImageTagsWithTimestampFn: func(repo string) ([]common.TagInfo, error) { - return []common.TagInfo{ + } + globalSearch([]string{"repo1"}, "name", "tag", mockOlum, log.NewLogger("debug", "")) + }) + + Convey("Manifests given, no manifest tag", func() { + mockOlum := mocks.OciLayoutUtilsMock{ + GetImageManifestsFn: func(name string) ([]ispec.Descriptor, error) { + return []ispec.Descriptor{ { - Name: "test", - Digest: "test", + Digest: "digest", + Size: -1, }, }, nil }, } - globalSearch([]string{"repo1"}, "name", "tag", mockOlum, log.NewLogger("debug", "")) + + globalSearch([]string{"repo1"}, "test", "tag", mockOlum, log.NewLogger("debug", "")) + }) + + Convey("Global search success, no tag", func() { + mockOlum := mocks.OciLayoutUtilsMock{ + GetRepoLastUpdatedFn: func(repo string) (common.TagInfo, error) { + return common.TagInfo{ + Digest: "sha256:855b1556a45637abf05c63407437f6f305b4627c4361fb965a78e5731999c0c7", + }, nil + }, + GetImageManifestsFn: func(name string) ([]ispec.Descriptor, error) { + return []ispec.Descriptor{ + { + Digest: "sha256:855b1556a45637abf05c63407437f6f305b4627c4361fb965a78e5731999c0c7", + Size: -1, + Annotations: map[string]string{ + ispec.AnnotationRefName: "this is a bad format", + }, + }, + }, nil + }, + GetImageBlobManifestFn: func(imageDir string, digest godigest.Digest) (v1.Manifest, error) { + return v1.Manifest{ + Layers: []v1.Descriptor{ + { + Size: 0, + Digest: v1.Hash{}, + }, + }, + }, nil + }, + } + globalSearch([]string{"repo1/name"}, "name", "tag", mockOlum, log.NewLogger("debug", "")) + }) + + Convey("Manifests given, bad image config info", func() { + mockOlum := mocks.OciLayoutUtilsMock{ + GetImageManifestsFn: func(name string) ([]ispec.Descriptor, error) { + return []ispec.Descriptor{ + { + Digest: "digest", + Size: -1, + Annotations: map[string]string{ + ispec.AnnotationRefName: "this is a bad format", + }, + }, + }, nil + }, + GetImageConfigInfoFn: func(repo string, manifestDigest godigest.Digest) (ispec.Image, error) { + return ispec.Image{}, ErrTestError + }, + } + globalSearch([]string{"repo1/name"}, "name", "tag", mockOlum, log.NewLogger("debug", "")) }) Convey("Tag given, no layer match", func() { diff --git a/pkg/test/mocks/oci_mock.go b/pkg/test/mocks/oci_mock.go index 98e6c515..44d6bcf4 100644 --- a/pkg/test/mocks/oci_mock.go +++ b/pkg/test/mocks/oci_mock.go @@ -23,10 +23,11 @@ type OciLayoutUtilsMock struct { GetRepoLastUpdatedFn func(repo string) (common.TagInfo, error) GetExpandedRepoInfoFn func(name string) (common.RepoInfo, error) GetImageConfigInfoFn func(repo string, manifestDigest godigest.Digest) (ispec.Image, error) + CheckManifestSignatureFn func(name string, digest godigest.Digest) bool } func (olum OciLayoutUtilsMock) GetImageManifests(image string) ([]ispec.Descriptor, error) { - if olum.GetImageBlobManifestFn != nil { + if olum.GetImageManifestsFn != nil { return olum.GetImageManifestsFn(image) } @@ -128,3 +129,11 @@ func (olum OciLayoutUtilsMock) GetImageConfigInfo(repo string, manifestDigest go return ispec.Image{}, nil } + +func (olum OciLayoutUtilsMock) CheckManifestSignature(name string, digest godigest.Digest) bool { + if olum.CheckManifestSignatureFn != nil { + return olum.CheckManifestSignatureFn(name, digest) + } + + return false +}