diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index 631f0077..4879abe3 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -33,6 +33,7 @@ import ( "zotregistry.dev/zot/v2/pkg/api/config" "zotregistry.dev/zot/v2/pkg/api/constants" zcommon "zotregistry.dev/zot/v2/pkg/common" + "zotregistry.dev/zot/v2/pkg/compat" extconf "zotregistry.dev/zot/v2/pkg/extensions/config" "zotregistry.dev/zot/v2/pkg/extensions/monitoring" cveinfo "zotregistry.dev/zot/v2/pkg/extensions/search/cve" @@ -5333,6 +5334,7 @@ func TestMetaDBIndexOperations(t *testing.T) { baseURL := GetBaseURL(port) conf := config.New() conf.HTTP.Port = port + conf.HTTP.Compat = []compat.MediaCompatibility{compat.DockerManifestV2SchemaV2} conf.Storage.RootDirectory = dir conf.Storage.GC = false defaultVal := true @@ -5431,7 +5433,219 @@ func RunMetaDBIndexTests(baseURL, port string) { responseImage = responseImages[0] So(responseImage.IsSigned, ShouldBeFalse) + // Download count is 1 because SignImageUsingCosign fetches the manifest to sign it + So(responseImage.DownloadCount, ShouldEqual, 1) + + // Get initial repository download count - query repository separately + repoQuery := ` + { + GlobalSearch(query:"repo"){ + Repos { + Name DownloadCount + } + } + }` + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(repoQuery)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoResponseStruct := &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), repoResponseStruct) + So(err, ShouldBeNil) + repos := repoResponseStruct.GlobalSearchResult.GlobalSearch.Repos + So(repos, ShouldNotBeEmpty) + initialRepoDownloadCount := repos[0].DownloadCount + + // Test download count - download the index manifest 3 times + resp, err = resty.R().Get(baseURL + "/v2/" + repo + "/manifests/" + "tag1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = resty.R().Get(baseURL + "/v2/" + repo + "/manifests/" + "tag1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = resty.R().Get(baseURL + "/v2/" + repo + "/manifests/" + "tag1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // Verify download count increased at both image and repository level + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + responseStruct = &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + responseImages = responseStruct.GlobalSearchResult.GlobalSearch.Images + So(responseImages, ShouldNotBeEmpty) + responseImage = responseImages[0] + // Started with 1 (from cosign signing), added 3 more downloads = 4 total + So(responseImage.DownloadCount, ShouldEqual, 4) + + // Verify repository-level download count also increased - query repository separately + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(repoQuery)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoResponseStruct = &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), repoResponseStruct) + So(err, ShouldBeNil) + repos = repoResponseStruct.GlobalSearchResult.GlobalSearch.Repos + So(repos, ShouldNotBeEmpty) + So(repos[0].DownloadCount, ShouldEqual, initialRepoDownloadCount+3) }) + + Convey("Push test index with Docker media types", func() { + const repo = "repo-docker" + + multiarchImage := CreateRandomMultiarch().AsDockerImage() + + err := UploadMultiarchImage(multiarchImage, baseURL, repo, "tag1") + So(err, ShouldBeNil) + + query := ` + { + GlobalSearch(query:"repo-docker:tag1"){ + Images { + RepoName Tag DownloadCount + IsSigned + Manifests { + Digest + ConfigDigest + Platform {Os Arch} + Layers {Size Digest} + LastUpdated + Size + } + } + } + }` + + resp, err := resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + responseStruct := &zcommon.GlobalSearchResultResp{} + + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + + responseImages := responseStruct.GlobalSearchResult.GlobalSearch.Images + So(responseImages, ShouldNotBeEmpty) + responseImage := responseImages[0] + So(len(responseImage.Manifests), ShouldEqual, 3) + + err = signature.SignImageUsingCosign("repo-docker@"+multiarchImage.DigestStr(), port, false) + So(err, ShouldBeNil) + + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + responseStruct = &zcommon.GlobalSearchResultResp{} + + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + + responseImages = responseStruct.GlobalSearchResult.GlobalSearch.Images + So(responseImages, ShouldNotBeEmpty) + responseImage = responseImages[0] + + So(responseImage.IsSigned, ShouldBeTrue) + + // remove signature + cosignTag := "sha256-" + multiarchImage.Digest().Encoded() + ".sig" + _, err = resty.R().Delete(baseURL + "/v2/" + repo + "/manifests/" + cosignTag) + So(err, ShouldBeNil) + + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + responseStruct = &zcommon.GlobalSearchResultResp{} + + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + + responseImages = responseStruct.GlobalSearchResult.GlobalSearch.Images + So(responseImages, ShouldNotBeEmpty) + responseImage = responseImages[0] + + So(responseImage.IsSigned, ShouldBeFalse) + // Download count is 1 because SignImageUsingCosign fetches the manifest to sign it + initialDownloadCount := responseImage.DownloadCount + So(initialDownloadCount, ShouldEqual, 1) + + // Get initial repository download count - query repository separately + repoQuery := ` + { + GlobalSearch(query:"repo-docker"){ + Repos { + Name DownloadCount + } + } + }` + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(repoQuery)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoResponseStruct := &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), repoResponseStruct) + So(err, ShouldBeNil) + repos := repoResponseStruct.GlobalSearchResult.GlobalSearch.Repos + So(repos, ShouldNotBeEmpty) + initialRepoDownloadCount := repos[0].DownloadCount + + // Test download count - download the index manifest 3 times + resp, err = resty.R().Get(baseURL + "/v2/" + repo + "/manifests/" + "tag1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = resty.R().Get(baseURL + "/v2/" + repo + "/manifests/" + "tag1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = resty.R().Get(baseURL + "/v2/" + repo + "/manifests/" + "tag1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // Verify download count increased at both image and repository level + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + responseStruct = &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + responseImages = responseStruct.GlobalSearchResult.GlobalSearch.Images + So(responseImages, ShouldNotBeEmpty) + responseImage = responseImages[0] + // Started with initialDownloadCount of 1 (from SignImageUsingCosign), added 3 more downloads + So(responseImage.DownloadCount, ShouldEqual, initialDownloadCount+3) + + // Verify repository-level download count also increased - query repository separately + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(repoQuery)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoResponseStruct = &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), repoResponseStruct) + So(err, ShouldBeNil) + repos = repoResponseStruct.GlobalSearchResult.GlobalSearch.Repos + So(repos, ShouldNotBeEmpty) + So(repos[0].DownloadCount, ShouldEqual, initialRepoDownloadCount+3) + }) + Convey("Index base images", func() { // ---------------- BASE IMAGE ------------------- imageAMD64 := CreateImageWith().LayerBlobs([][]byte{ @@ -5916,6 +6130,26 @@ func TestMetaDBWhenReadingImages(t *testing.T) { So(err, ShouldBeNil) So(responseStruct.Images, ShouldNotBeEmpty) So(responseStruct.Images[0].DownloadCount, ShouldEqual, 3) + + // Verify repository-level download count also increased - query repository separately + repoQuery := ` + { + GlobalSearch(query:"repo1"){ + Repos { + Name DownloadCount + } + } + }` + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(repoQuery)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoResponseStruct := &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), repoResponseStruct) + So(err, ShouldBeNil) + So(repoResponseStruct.Repos, ShouldNotBeEmpty) + So(repoResponseStruct.Repos[0].DownloadCount, ShouldEqual, 3) }) Convey("Error when incrementing", func() { @@ -5930,6 +6164,87 @@ func TestMetaDBWhenReadingImages(t *testing.T) { So(resp.StatusCode(), ShouldEqual, http.StatusInternalServerError) }) }) + + Convey("Push test image with Docker media types", t, func() { + dir := t.TempDir() + + port := GetFreePort() + baseURL := GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + conf.HTTP.Compat = []compat.MediaCompatibility{compat.DockerManifestV2SchemaV2} + conf.Storage.RootDirectory = dir + defaultVal := true + conf.Extensions = &extconf.ExtensionConfig{ + Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}}, + } + + ctlr := api.NewController(conf) + + ctlrManager := NewControllerManager(ctlr) + ctlrManager.StartAndWait(port) + defer ctlrManager.StopServer() + + image := CreateImageWith().RandomLayers(1, 100).DefaultConfig().Build().AsDockerImage() + + err := UploadImage(image, baseURL, "repo2", "2.0.1") + So(err, ShouldBeNil) + + Convey("Download 3 times", func() { + resp, err := resty.R().Get(baseURL + "/v2/" + "repo2" + "/manifests/" + "2.0.1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = resty.R().Get(baseURL + "/v2/" + "repo2" + "/manifests/" + "2.0.1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = resty.R().Get(baseURL + "/v2/" + "repo2" + "/manifests/" + "2.0.1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + query := ` + { + GlobalSearch(query:"repo2:2.0"){ + Images { + RepoName Tag DownloadCount + } + } + }` + + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + responseStruct := &zcommon.GlobalSearchResultResp{} + + err = json.Unmarshal(resp.Body(), responseStruct) + So(err, ShouldBeNil) + So(responseStruct.Images, ShouldNotBeEmpty) + So(responseStruct.Images[0].DownloadCount, ShouldEqual, 3) + + // Verify repository-level download count also increased - query repository separately + repoQuery := ` + { + GlobalSearch(query:"repo2"){ + Repos { + Name DownloadCount + } + } + }` + resp, err = resty.R().Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(repoQuery)) + So(resp, ShouldNotBeNil) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoResponseStruct := &zcommon.GlobalSearchResultResp{} + err = json.Unmarshal(resp.Body(), repoResponseStruct) + So(err, ShouldBeNil) + So(repoResponseStruct.Repos, ShouldNotBeEmpty) + So(repoResponseStruct.Repos[0].DownloadCount, ShouldEqual, 3) + }) + }) } func TestMetaDBWhenDeletingImages(t *testing.T) { diff --git a/pkg/meta/boltdb/boltdb.go b/pkg/meta/boltdb/boltdb.go index 2b712487..936a1e57 100644 --- a/pkg/meta/boltdb/boltdb.go +++ b/pkg/meta/boltdb/boltdb.go @@ -247,15 +247,25 @@ func (bdw *BoltDB) SetRepoReference(ctx context.Context, repo string, reference } } - if _, ok := protoRepoMeta.Statistics[imageMeta.Digest.String()]; !ok { - protoRepoMeta.Statistics[imageMeta.Digest.String()] = &proto_go.DescriptorStatistics{ + digestStr := imageMeta.Digest.String() + stats, ok := protoRepoMeta.Statistics[digestStr] + + if !ok { + stats = &proto_go.DescriptorStatistics{ DownloadCount: 0, LastPullTimestamp: ×tamppb.Timestamp{}, PushTimestamp: timestamppb.Now(), PushedBy: userid, } - } else if protoRepoMeta.Statistics[imageMeta.Digest.String()].PushTimestamp.AsTime().IsZero() { - protoRepoMeta.Statistics[imageMeta.Digest.String()].PushTimestamp = timestamppb.Now() + protoRepoMeta.Statistics[digestStr] = stats + } else { + if stats.PushTimestamp.AsTime().IsZero() { + stats.PushTimestamp = timestamppb.Now() + } + + if userid != "" && stats.PushedBy == "" { + stats.PushedBy = userid + } } if _, ok := protoRepoMeta.Signatures[imageMeta.Digest.String()]; !ok { @@ -1250,7 +1260,30 @@ func (bdw *BoltDB) UpdateStatsOnDownload(repo string, reference string) error { manifestStatistics, ok := protoRepoMeta.Statistics[manifestDigest] if !ok { - return zerr.ErrImageMetaNotFound + // Statistics entry doesn't exist - validate digest exists in this repository before creating it + // Check if digest is referenced in any tag for this repository + digestExists := false + + for _, tagDescriptor := range protoRepoMeta.Tags { + if tagDescriptor.Digest == manifestDigest { + digestExists = true + + break + } + } + + if !digestExists { + return zerr.ErrImageMetaNotFound + } + + // Statistics entry doesn't exist - create it + // This can happen if SetRepoReference failed or wasn't called + manifestStatistics = &proto_go.DescriptorStatistics{ + DownloadCount: 0, + LastPullTimestamp: ×tamppb.Timestamp{}, + PushTimestamp: ×tamppb.Timestamp{}, // Unknown push time + PushedBy: "", // Unknown pusher + } } manifestStatistics.DownloadCount++ diff --git a/pkg/meta/boltdb/boltdb_test.go b/pkg/meta/boltdb/boltdb_test.go index 7d4f53d2..c261e630 100644 --- a/pkg/meta/boltdb/boltdb_test.go +++ b/pkg/meta/boltdb/boltdb_test.go @@ -190,6 +190,111 @@ func TestWrapperErrors(t *testing.T) { err = boltdbWrapper.UpdateStatsOnDownload("repo", godigest.FromString("not-found").String()) So(err, ShouldNotBeNil) }) + + Convey("statistics entry missing but digest exists in tags - should create and increment", func() { + // Set repo reference to create tag + err := boltdbWrapper.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldBeNil) + + // Manually remove Statistics entry to simulate missing Statistics + err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { + repoMetaBuck := tx.Bucket([]byte(boltdb.RepoMetaBuck)) + repoMetaBlob := repoMetaBuck.Get([]byte("repo")) + + if len(repoMetaBlob) == 0 { + return zerr.ErrRepoMetaNotFound + } + + var protoRepoMeta proto_go.RepoMeta + + err := proto.Unmarshal(repoMetaBlob, &protoRepoMeta) + if err != nil { + return err + } + + // Remove Statistics entry for the digest + delete(protoRepoMeta.Statistics, imageMeta.Digest.String()) + + repoMetaBlob, err = proto.Marshal(&protoRepoMeta) + if err != nil { + return err + } + + return repoMetaBuck.Put([]byte("repo"), repoMetaBlob) + }) + So(err, ShouldBeNil) + + // Verify Statistics entry doesn't exist + repoMeta, err := boltdbWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + _, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeFalse) + + // Update stats - should create Statistics entry and increment + err = boltdbWrapper.UpdateStatsOnDownload("repo", "tag") + So(err, ShouldBeNil) + + // Verify Statistics entry was created and incremented + repoMeta, err = boltdbWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 1) + + // Update stats again - should increment existing entry + err = boltdbWrapper.UpdateStatsOnDownload("repo", "tag") + So(err, ShouldBeNil) + + repoMeta, err = boltdbWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists = repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 2) + }) + + Convey("statistics entry missing but digest exists in tags - using digest reference", func() { + // Set repo reference to create tag + err := boltdbWrapper.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldBeNil) + + // Manually remove Statistics entry + err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { + repoMetaBuck := tx.Bucket([]byte(boltdb.RepoMetaBuck)) + repoMetaBlob := repoMetaBuck.Get([]byte("repo")) + + if len(repoMetaBlob) == 0 { + return zerr.ErrRepoMetaNotFound + } + + var protoRepoMeta proto_go.RepoMeta + + err := proto.Unmarshal(repoMetaBlob, &protoRepoMeta) + if err != nil { + return err + } + + delete(protoRepoMeta.Statistics, imageMeta.Digest.String()) + + repoMetaBlob, err = proto.Marshal(&protoRepoMeta) + if err != nil { + return err + } + + return repoMetaBuck.Put([]byte("repo"), repoMetaBlob) + }) + So(err, ShouldBeNil) + + // Update stats using digest directly + err = boltdbWrapper.UpdateStatsOnDownload("repo", imageMeta.Digest.String()) + So(err, ShouldBeNil) + + // Verify Statistics entry was created + repoMeta, err := boltdbWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 1) + }) }) Convey("GetReferrersInfo", func() { diff --git a/pkg/meta/convert/convert.go b/pkg/meta/convert/convert.go index fb75b171..28d0879a 100644 --- a/pkg/meta/convert/convert.go +++ b/pkg/meta/convert/convert.go @@ -35,15 +35,17 @@ func GetHistory(history []*proto_go.History) []ispec.History { } func GetImageArtifactType(imageMeta *proto_go.ImageMeta) string { - switch imageMeta.GetMediaType() { - case ispec.MediaTypeImageManifest: + mediaType := imageMeta.GetMediaType() + + switch { + case mediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(mediaType): manifestArtifactType := imageMeta.GetManifests()[0].GetManifest().GetArtifactType() if manifestArtifactType != "" { return manifestArtifactType } return imageMeta.GetManifests()[0].GetManifest().GetConfig().GetMediaType() - case ispec.MediaTypeImageIndex: + case mediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(mediaType): return imageMeta.GetIndex().GetIndex().GetArtifactType() default: return "" @@ -51,10 +53,12 @@ func GetImageArtifactType(imageMeta *proto_go.ImageMeta) string { } func GetImageManifestSize(imageMeta *proto_go.ImageMeta) int64 { - switch imageMeta.GetMediaType() { - case ispec.MediaTypeImageManifest: + mediaType := imageMeta.GetMediaType() + + switch { + case mediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(mediaType): return imageMeta.GetManifests()[0].GetSize() - case ispec.MediaTypeImageIndex: + case mediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(mediaType): return imageMeta.GetIndex().GetSize() default: return 0 @@ -62,10 +66,12 @@ func GetImageManifestSize(imageMeta *proto_go.ImageMeta) int64 { } func GetImageDigest(imageMeta *proto_go.ImageMeta) godigest.Digest { - switch imageMeta.GetMediaType() { - case ispec.MediaTypeImageManifest: + mediaType := imageMeta.GetMediaType() + + switch { + case mediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(mediaType): return godigest.Digest(imageMeta.GetManifests()[0].GetDigest()) - case ispec.MediaTypeImageIndex: + case mediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(mediaType): return godigest.Digest(imageMeta.GetIndex().GetDigest()) default: return "" @@ -73,10 +79,12 @@ func GetImageDigest(imageMeta *proto_go.ImageMeta) godigest.Digest { } func GetImageDigestStr(imageMeta *proto_go.ImageMeta) string { - switch imageMeta.GetMediaType() { - case ispec.MediaTypeImageManifest: + mediaType := imageMeta.GetMediaType() + + switch { + case mediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(mediaType): return imageMeta.GetManifests()[0].GetDigest() - case ispec.MediaTypeImageIndex: + case mediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(mediaType): return imageMeta.GetIndex().GetDigest() default: return "" @@ -84,10 +92,12 @@ func GetImageDigestStr(imageMeta *proto_go.ImageMeta) string { } func GetImageAnnotations(imageMeta *proto_go.ImageMeta) map[string]string { - switch imageMeta.GetMediaType() { - case ispec.MediaTypeImageManifest: + mediaType := imageMeta.GetMediaType() + + switch { + case mediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(mediaType): return imageMeta.GetManifests()[0].GetManifest().GetAnnotations() - case ispec.MediaTypeImageIndex: + case mediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(mediaType): return imageMeta.GetIndex().GetIndex().GetAnnotations() default: return map[string]string{} @@ -95,14 +105,16 @@ func GetImageAnnotations(imageMeta *proto_go.ImageMeta) map[string]string { } func GetImageSubject(imageMeta *proto_go.ImageMeta) *ispec.Descriptor { - switch imageMeta.GetMediaType() { - case ispec.MediaTypeImageManifest: + mediaType := imageMeta.GetMediaType() + + switch { + case mediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(mediaType): if imageMeta.GetManifests()[0].GetManifest().GetSubject() == nil { return nil } return GetDescriptorRef(imageMeta.GetManifests()[0].GetManifest().GetSubject()) - case ispec.MediaTypeImageIndex: + case mediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(mediaType): return GetDescriptorRef(imageMeta.GetIndex().GetIndex().GetSubject()) default: return nil @@ -530,7 +542,8 @@ func GetImageMeta(dbImageMeta *proto_go.ImageMeta) mTypes.ImageMeta { Digest: GetImageDigest(dbImageMeta), } - if dbImageMeta.GetMediaType() == ispec.MediaTypeImageIndex { + if dbImageMeta.GetMediaType() == ispec.MediaTypeImageIndex || + compat.IsCompatibleManifestListMediaType(dbImageMeta.GetMediaType()) { manifests := make([]ispec.Descriptor, 0, len(dbImageMeta.GetManifests())) for _, manifest := range dbImageMeta.GetIndex().GetIndex().GetManifests() { diff --git a/pkg/meta/dynamodb/dynamodb.go b/pkg/meta/dynamodb/dynamodb.go index fbc73736..818f4427 100644 --- a/pkg/meta/dynamodb/dynamodb.go +++ b/pkg/meta/dynamodb/dynamodb.go @@ -425,15 +425,25 @@ func (dwr *DynamoDB) SetRepoReference(ctx context.Context, repo string, referenc } } - if _, ok := repoMeta.Statistics[imageMeta.Digest.String()]; !ok { - repoMeta.Statistics[imageMeta.Digest.String()] = &proto_go.DescriptorStatistics{ + digestStr := imageMeta.Digest.String() + stats, ok := repoMeta.Statistics[digestStr] + + if !ok { + stats = &proto_go.DescriptorStatistics{ DownloadCount: 0, LastPullTimestamp: ×tamppb.Timestamp{}, PushTimestamp: timestamppb.Now(), PushedBy: userid, } - } else if repoMeta.Statistics[imageMeta.Digest.String()].PushTimestamp.AsTime().IsZero() { - repoMeta.Statistics[imageMeta.Digest.String()].PushTimestamp = timestamppb.Now() + repoMeta.Statistics[digestStr] = stats + } else { + if stats.PushTimestamp.AsTime().IsZero() { + stats.PushTimestamp = timestamppb.Now() + } + + if userid != "" && stats.PushedBy == "" { + stats.PushedBy = userid + } } if _, ok := repoMeta.Signatures[imageMeta.Digest.String()]; !ok { @@ -1157,7 +1167,30 @@ func (dwr *DynamoDB) UpdateStatsOnDownload(repo string, reference string) error manifestStatistics, ok := repoMeta.Statistics[descriptorDigest] if !ok { - return zerr.ErrImageMetaNotFound + // Statistics entry doesn't exist - validate digest exists in this repository before creating it + // Check if digest is referenced in any tag for this repository + digestExists := false + + for _, tagDescriptor := range repoMeta.Tags { + if tagDescriptor.Digest == descriptorDigest { + digestExists = true + + break + } + } + + if !digestExists { + return zerr.ErrImageMetaNotFound + } + + // Statistics entry doesn't exist - create it + // This can happen if SetRepoReference failed or wasn't called + manifestStatistics = &proto_go.DescriptorStatistics{ + DownloadCount: 0, + LastPullTimestamp: ×tamppb.Timestamp{}, + PushTimestamp: ×tamppb.Timestamp{}, // Unknown push time + PushedBy: "", // Unknown pusher + } } manifestStatistics.DownloadCount++ diff --git a/pkg/meta/dynamodb/dynamodb_test.go b/pkg/meta/dynamodb/dynamodb_test.go index f8b5cf9a..91e91acd 100644 --- a/pkg/meta/dynamodb/dynamodb_test.go +++ b/pkg/meta/dynamodb/dynamodb_test.go @@ -334,6 +334,74 @@ func TestWrapperErrors(t *testing.T) { err = dynamoWrapper.UpdateStatsOnDownload("repo", godigest.FromString("not-found").String()) //nolint: contextcheck So(err, ShouldNotBeNil) }) + + Convey("statistics entry missing but digest exists in tags - should create and increment", func() { + // Set repo reference to create tag + err := dynamoWrapper.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldBeNil) + + // Manually remove Statistics entry to simulate missing Statistics + repoMeta, err := dynamoWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + delete(repoMeta.Statistics, imageMeta.Digest.String()) + + // Set repo meta back without Statistics + err = dynamoWrapper.SetRepoMeta("repo", repoMeta) + So(err, ShouldBeNil) + + // Verify Statistics entry doesn't exist + repoMeta, err = dynamoWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + _, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeFalse) + + // Update stats - should create Statistics entry and increment + err = dynamoWrapper.UpdateStatsOnDownload("repo", "tag") //nolint: contextcheck + So(err, ShouldBeNil) + + // Verify Statistics entry was created and incremented + repoMeta, err = dynamoWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 1) + + // Update stats again - should increment existing entry + err = dynamoWrapper.UpdateStatsOnDownload("repo", "tag") //nolint: contextcheck + So(err, ShouldBeNil) + + repoMeta, err = dynamoWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists = repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 2) + }) + + Convey("statistics entry missing but digest exists in tags - using digest reference", func() { + // Set repo reference to create tag + err := dynamoWrapper.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldBeNil) + + // Manually remove Statistics entry + repoMeta, err := dynamoWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + delete(repoMeta.Statistics, imageMeta.Digest.String()) + + // Set repo meta back without Statistics + err = dynamoWrapper.SetRepoMeta("repo", repoMeta) + So(err, ShouldBeNil) + + // Update stats using digest directly + err = dynamoWrapper.UpdateStatsOnDownload("repo", imageMeta.Digest.String()) //nolint: contextcheck + So(err, ShouldBeNil) + + // Verify Statistics entry was created + repoMeta, err = dynamoWrapper.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 1) + }) }) Convey("GetReferrersInfo", func() { Convey("unmarshalProtoRepoMeta error", func() { diff --git a/pkg/meta/meta_test.go b/pkg/meta/meta_test.go index 92e03b94..480f4611 100644 --- a/pkg/meta/meta_test.go +++ b/pkg/meta/meta_test.go @@ -847,6 +847,103 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func So(err, ShouldBeNil) So(*repoMeta.LastUpdatedImage.LastUpdated, ShouldEqual, time.Date(2011, 3, 1, 12, 0, 0, 0, time.UTC)) }) + + Convey("PushedBy field behavior", func() { + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test-user") + ctxWithUser := userAc.DeriveContext(context.Background()) + + emptyCtx := context.Background() + + Convey("PushedBy is set when userid provided and PushedBy is empty", func() { + // Set repo reference with userid + err := metaDB.SetRepoReference(ctxWithUser, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is set + repoMeta, err := metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "test-user") + }) + + Convey("PushedBy is NOT overwritten when it already has a value", func() { + // Set repo reference with first user + err := metaDB.SetRepoReference(ctxWithUser, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is set + repoMeta, err := metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "test-user") + + // Try to set repo reference again with different userid + userAc2 := reqCtx.NewUserAccessControl() + userAc2.SetUsername("different-user") + ctx2 := userAc2.DeriveContext(context.Background()) + + err = metaDB.SetRepoReference(ctx2, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is NOT overwritten + repoMeta, err = metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists = repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "test-user") // Still the original value + }) + + Convey("PushedBy is set when Statistics entry exists but PushedBy is empty", func() { + // Create repo reference without userid (empty context) + err := metaDB.SetRepoReference(emptyCtx, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is empty + repoMeta, err := metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "") + + // Set repo reference again with userid + err = metaDB.SetRepoReference(ctxWithUser, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is now set + repoMeta, err = metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists = repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "test-user") + }) + + Convey("PushedBy is NOT set when userid is empty", func() { + // Set repo reference without userid (empty context) + err := metaDB.SetRepoReference(emptyCtx, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is empty + repoMeta, err := metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "") + + // Set repo reference again without userid + err = metaDB.SetRepoReference(emptyCtx, repo1, tag1, imgData1) + So(err, ShouldBeNil) + + // Verify PushedBy is still empty + repoMeta, err = metaDB.GetRepoMeta(ctxWithUser, repo1) + So(err, ShouldBeNil) + stats, exists = repoMeta.Statistics[imgData1.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.PushedBy, ShouldEqual, "") + }) + }) }) Convey("Test RemoveRepoReference", func() { diff --git a/pkg/meta/redis/redis.go b/pkg/meta/redis/redis.go index b64639b6..ece03419 100644 --- a/pkg/meta/redis/redis.go +++ b/pkg/meta/redis/redis.go @@ -811,15 +811,25 @@ func (rc *RedisDB) SetRepoReference(ctx context.Context, repo string, } } - if _, ok := protoRepoMeta.Statistics[imageMeta.Digest.String()]; !ok { - protoRepoMeta.Statistics[imageMeta.Digest.String()] = &proto_go.DescriptorStatistics{ + digestStr := imageMeta.Digest.String() + stats, ok := protoRepoMeta.Statistics[digestStr] + + if !ok { + stats = &proto_go.DescriptorStatistics{ DownloadCount: 0, LastPullTimestamp: ×tamppb.Timestamp{}, PushTimestamp: timestamppb.Now(), PushedBy: userid, } - } else if protoRepoMeta.Statistics[imageMeta.Digest.String()].PushTimestamp.AsTime().IsZero() { - protoRepoMeta.Statistics[imageMeta.Digest.String()].PushTimestamp = timestamppb.Now() + protoRepoMeta.Statistics[digestStr] = stats + } else { + if stats.PushTimestamp.AsTime().IsZero() { + stats.PushTimestamp = timestamppb.Now() + } + + if userid != "" && stats.PushedBy == "" { + stats.PushedBy = userid + } } if _, ok := protoRepoMeta.Signatures[imageMeta.Digest.String()]; !ok { @@ -1753,7 +1763,30 @@ func (rc *RedisDB) UpdateStatsOnDownload(repo string, reference string) error { manifestStatistics, ok := protoRepoMeta.Statistics[manifestDigest] if !ok { - return zerr.ErrImageMetaNotFound + // Statistics entry doesn't exist - validate digest exists in this repository before creating it + // Check if digest is referenced in any tag for this repository + digestExists := false + + for _, tagDescriptor := range protoRepoMeta.Tags { + if tagDescriptor.Digest == manifestDigest { + digestExists = true + + break + } + } + + if !digestExists { + return zerr.ErrImageMetaNotFound + } + + // Statistics entry doesn't exist - create it + // This can happen if SetRepoReference failed or wasn't called + manifestStatistics = &proto_go.DescriptorStatistics{ + DownloadCount: 0, + LastPullTimestamp: ×tamppb.Timestamp{}, + PushTimestamp: ×tamppb.Timestamp{}, // Unknown push time + PushedBy: "", // Unknown pusher + } } manifestStatistics.DownloadCount++ diff --git a/pkg/meta/redis/redis_test.go b/pkg/meta/redis/redis_test.go index bf1272be..6fcf7a6f 100644 --- a/pkg/meta/redis/redis_test.go +++ b/pkg/meta/redis/redis_test.go @@ -717,6 +717,87 @@ func TestWrapperErrors(t *testing.T) { err = metaDB.UpdateStatsOnDownload("repo", godigest.FromString("not-found").String()) So(err, ShouldNotBeNil) }) + + Convey("statistics entry missing but digest exists in tags - should create and increment", func() { + // Set repo reference to create tag + err := metaDB.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldBeNil) + + // Manually remove Statistics entry to simulate missing Statistics + // Get proto blob directly from Redis + key := keyPrefix + ":" + redis.RepoMetaBucket + repoMetaBlob, err := client.HGet(ctx, key, "repo").Bytes() + So(err, ShouldBeNil) + + var protoRepoMeta proto_go.RepoMeta + err = proto.Unmarshal(repoMetaBlob, &protoRepoMeta) + So(err, ShouldBeNil) + delete(protoRepoMeta.Statistics, imageMeta.Digest.String()) + + repoMetaBlob, err = proto.Marshal(&protoRepoMeta) + So(err, ShouldBeNil) + err = setRepoMeta("repo", repoMetaBlob, client) + So(err, ShouldBeNil) + + // Verify Statistics entry doesn't exist + repoMeta, err := metaDB.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + _, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeFalse) + + // Update stats - should create Statistics entry and increment + err = metaDB.UpdateStatsOnDownload("repo", "tag") + So(err, ShouldBeNil) + + // Verify Statistics entry was created and incremented + repoMeta, err = metaDB.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 1) + + // Update stats again - should increment existing entry + err = metaDB.UpdateStatsOnDownload("repo", "tag") + So(err, ShouldBeNil) + + repoMeta, err = metaDB.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists = repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 2) + }) + + Convey("statistics entry missing but digest exists in tags - using digest reference", func() { + // Set repo reference to create tag + err := metaDB.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldBeNil) + + // Manually remove Statistics entry + key := keyPrefix + ":" + redis.RepoMetaBucket + repoMetaBlob, err := client.HGet(ctx, key, "repo").Bytes() + So(err, ShouldBeNil) + + var protoRepoMeta proto_go.RepoMeta + err = proto.Unmarshal(repoMetaBlob, &protoRepoMeta) + So(err, ShouldBeNil) + delete(protoRepoMeta.Statistics, imageMeta.Digest.String()) + + repoMetaBlob, err = proto.Marshal(&protoRepoMeta) + So(err, ShouldBeNil) + err = setRepoMeta("repo", repoMetaBlob, client) + So(err, ShouldBeNil) + + // Update stats using digest directly + err = metaDB.UpdateStatsOnDownload("repo", imageMeta.Digest.String()) + So(err, ShouldBeNil) + + // Verify Statistics entry was created + repoMeta, err := metaDB.GetRepoMeta(ctx, "repo") + So(err, ShouldBeNil) + stats, exists := repoMeta.Statistics[imageMeta.Digest.String()] + So(exists, ShouldBeTrue) + So(stats.DownloadCount, ShouldEqual, 1) + }) }) Convey("GetReferrersInfo", func() { diff --git a/pkg/test/image-utils/images.go b/pkg/test/image-utils/images.go index 419df31c..da0386eb 100644 --- a/pkg/test/image-utils/images.go +++ b/pkg/test/image-utils/images.go @@ -193,7 +193,7 @@ func (img Image) AsDockerImage() Image { } img.ManifestDescriptor = ispec.Descriptor{ - MediaType: docker.MediaTypeImageConfig, + MediaType: docker.MediaTypeManifest, Digest: img.digestAlgorithm.FromBytes(manifestBlob), Size: int64(len(manifestBlob)), Data: manifestBlob, diff --git a/pkg/test/image-utils/upload.go b/pkg/test/image-utils/upload.go index 43bd520c..b5f4bf8d 100644 --- a/pkg/test/image-utils/upload.go +++ b/pkg/test/image-utils/upload.go @@ -118,8 +118,19 @@ func UploadImage(img Image, baseURL, repo, ref string) error { } } + // Use the media type from ManifestDescriptor, or fall back to Manifest.MediaType, or default to OCI + mediaType := img.ManifestDescriptor.MediaType + + if mediaType == "" { + mediaType = img.Manifest.MediaType + } + + if mediaType == "" { + mediaType = ispec.MediaTypeImageManifest + } + resp, err = resty.R(). - SetHeader("Content-type", ispec.MediaTypeImageManifest). + SetHeader("Content-type", mediaType). SetBody(manifestBlob). Put(baseURL + "/v2/" + repo + "/manifests/" + ref) @@ -216,9 +227,20 @@ func UploadImageWithBasicAuth(img Image, baseURL, repo, ref, user, password stri return err } + // Use the media type from ManifestDescriptor, or fall back to Manifest.MediaType, or default to OCI + mediaType := img.ManifestDescriptor.MediaType + + if mediaType == "" { + mediaType = img.Manifest.MediaType + } + + if mediaType == "" { + mediaType = ispec.MediaTypeImageManifest + } + _, err = resty.R(). SetBasicAuth(user, password). - SetHeader("Content-type", "application/vnd.oci.image.manifest.v1+json"). + SetHeader("Content-type", mediaType). SetBody(manifestBlob). Put(baseURL + "/v2/" + repo + "/manifests/" + ref) @@ -245,8 +267,19 @@ func UploadMultiarchImage(multiImage MultiarchImage, baseURL string, repo, ref s } } + // Use the media type from IndexDescriptor, or fall back to Index.MediaType, or default to OCI + mediaType := multiImage.IndexDescriptor.MediaType + + if mediaType == "" { + mediaType = multiImage.Index.MediaType + } + + if mediaType == "" { + mediaType = ispec.MediaTypeImageIndex + } + resp, err := resty.R(). - SetHeader("Content-type", ispec.MediaTypeImageIndex). + SetHeader("Content-type", mediaType). SetBody(indexBlob). Put(baseURL + "/v2/" + repo + "/manifests/" + ref)