From 3c8030b2c7eaddbdf20eec27ec2d55fd96abce4c Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Tue, 3 Feb 2026 21:10:35 +0200 Subject: [PATCH] fix(meta): fixes for LastUpdated and TaggedTimestamp (#3754) 1. Parse repos without metadata in ParseStorage The timestamp check in ParseStorage was skipping repos that exist in storage but don't have metadata. When GetRepoLastUpdated returns zero time (no metadata), we should always parse the repo to create its metadata. Check if metaLastUpdated is zero before comparing timestamps. If zero, always parse regardless of storageLastUpdated. 2. Change the logic of how LastUpdated is computed in RepoSummary It is not the latest tagged timestamp from the available images or the last updated image created timestamp, based on whichever is the latest. Signed-off-by: Andrei Aaron --- pkg/cli/client/image_cmd_internal_test.go | 11 +- pkg/cli/client/image_cmd_test.go | 24 ++- pkg/cli/client/search_cmd_test.go | 3 +- pkg/cli/server/verify_retention_test.go | 10 +- pkg/common/model.go | 43 ++--- pkg/extensions/search/convert/convert_test.go | 154 +++++++++++++++++ pkg/extensions/search/convert/metadb.go | 42 +++-- pkg/extensions/search/cve/update_test.go | 2 +- pkg/extensions/search/search_test.go | 52 +++++- pkg/meta/boltdb/boltdb.go | 7 +- pkg/meta/boltdb/boltdb_test.go | 37 +++++ pkg/meta/common/common.go | 1 + pkg/meta/dynamodb/dynamodb.go | 21 +++ pkg/meta/dynamodb/dynamodb_test.go | 156 ++++++++++++++++++ pkg/meta/parse.go | 4 +- pkg/meta/parse_test.go | 49 ++++++ pkg/meta/redis/redis.go | 7 +- pkg/meta/redis/redis_test.go | 34 ++++ 18 files changed, 602 insertions(+), 55 deletions(-) diff --git a/pkg/cli/client/image_cmd_internal_test.go b/pkg/cli/client/image_cmd_internal_test.go index 4b79119d..d74a0f12 100644 --- a/pkg/cli/client/image_cmd_internal_test.go +++ b/pkg/cli/client/image_cmd_internal_test.go @@ -407,7 +407,8 @@ func TestOutputFormat(t *testing.T) { `"score":0}],"history":null,"vulnerabilities":{"maxSeverity":"","unknownCount":0,"lowCount":0,`+ `"mediumCount":0,"highCount":0,"criticalCount":0,"count":0},`+ `"referrers":null,"artifactType":"","signatureInfo":null}],"size":"123445",`+ - `"downloadCount":0,"lastUpdated":"0001-01-01T00:00:00Z","description":"","isSigned":false,"licenses":"",`+ + `"downloadCount":0,"lastUpdated":"0001-01-01T00:00:00Z","lastPullTimestamp":"0001-01-01T00:00:00Z",`+ + `"pushTimestamp":"0001-01-01T00:00:00Z","taggedTimestamp":"0001-01-01T00:00:00Z","description":"","isSigned":false,"licenses":"",`+ `"labels":"","title":"","source":"","documentation":"","authors":"","vendor":"",`+ `"vulnerabilities":{"maxSeverity":"","unknownCount":0,"lowCount":0,"mediumCount":0,"highCount":0,`+ `"criticalCount":0,"count":0},"referrers":null,"signatureInfo":null}`+"\n") @@ -442,7 +443,9 @@ func TestOutputFormat(t *testing.T) { `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 `+ `referrers: [] artifacttype: "" `+ `signatureinfo: [] size: "123445" downloadcount: 0 `+ - `lastupdated: 0001-01-01T00:00:00Z description: "" issigned: false licenses: "" labels: "" `+ + `lastupdated: 0001-01-01T00:00:00Z lastpulltimestamp: 0001-01-01T00:00:00Z `+ + `pushtimestamp: 0001-01-01T00:00:00Z taggedtimestamp: 0001-01-01T00:00:00Z `+ + `description: "" issigned: false licenses: "" labels: "" `+ `title: "" source: "" documentation: "" authors: "" vendor: "" vulnerabilities: maxseverity: "" `+ `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 `+ `count: 0 referrers: [] signatureinfo: []`, @@ -479,7 +482,9 @@ func TestOutputFormat(t *testing.T) { `history: [] vulnerabilities: maxseverity: "" unknowncount: 0 lowcount: 0 mediumcount: 0 `+ `highcount: 0 criticalcount: 0 count: 0 referrers: [] artifacttype: "" `+ `signatureinfo: [] size: "123445" downloadcount: 0 `+ - `lastupdated: 0001-01-01T00:00:00Z description: "" issigned: false licenses: "" labels: "" `+ + `lastupdated: 0001-01-01T00:00:00Z lastpulltimestamp: 0001-01-01T00:00:00Z `+ + `pushtimestamp: 0001-01-01T00:00:00Z taggedtimestamp: 0001-01-01T00:00:00Z `+ + `description: "" issigned: false licenses: "" labels: "" `+ `title: "" source: "" documentation: "" authors: "" vendor: "" vulnerabilities: maxseverity: "" `+ `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 `+ `count: 0 referrers: [] signatureinfo: []`, diff --git a/pkg/cli/client/image_cmd_test.go b/pkg/cli/client/image_cmd_test.go index 802be27b..a1adc87c 100644 --- a/pkg/cli/client/image_cmd_test.go +++ b/pkg/cli/client/image_cmd_test.go @@ -389,7 +389,9 @@ func TestOutputFormatGQL(t *testing.T) { `"history":null,"vulnerabilities":{"maxSeverity":"","unknownCount":0,"lowCount":0,"mediumCount":0,` + `"highCount":0,"criticalCount":0,"count":0},` + `"referrers":null,"artifactType":"","signatureInfo":null}],` + - `"size":"528","downloadCount":0,"lastUpdated":"2023-01-01T12:00:00Z","description":"","isSigned":false,` + + `"size":"528","downloadCount":0,"lastUpdated":"2023-01-01T12:00:00Z","lastPullTimestamp":"0001-01-01T00:00:00Z",` + + `"pushTimestamp":"0001-01-01T00:00:00Z","taggedTimestamp":"0001-01-01T00:00:00Z",` + + `"description":"","isSigned":false,` + `"licenses":"","labels":"","title":"","source":"","documentation":"","authors":"","vendor":"",` + `"vulnerabilities":{"maxSeverity":"","unknownCount":0,"lowCount":0,"mediumCount":0,` + `"highCount":0,"criticalCount":0,"count":0},"referrers":null,"signatureInfo":null}` + "\n" + @@ -404,7 +406,9 @@ func TestOutputFormatGQL(t *testing.T) { `"history":null,"vulnerabilities":{"maxSeverity":"","unknownCount":0,"lowCount":0,"mediumCount":0,` + `"highCount":0,"criticalCount":0,"count":0},` + `"referrers":null,"artifactType":"","signatureInfo":null}],` + - `"size":"528","downloadCount":0,"lastUpdated":"2023-01-01T12:00:00Z","description":"","isSigned":false,` + + `"size":"528","downloadCount":0,"lastUpdated":"2023-01-01T12:00:00Z","lastPullTimestamp":"0001-01-01T00:00:00Z",` + + `"pushTimestamp":"0001-01-01T00:00:00Z","taggedTimestamp":"0001-01-01T00:00:00Z",` + + `"description":"","isSigned":false,` + `"licenses":"","labels":"","title":"","source":"","documentation":"","authors":"","vendor":"",` + `"vulnerabilities":{"maxSeverity":"","unknownCount":0,"lowCount":0,"mediumCount":0,` + `"highCount":0,"criticalCount":0,"count":0},"referrers":null,"signatureInfo":null}` + "\n" @@ -438,7 +442,9 @@ func TestOutputFormatGQL(t *testing.T) { `history: [] vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + `referrers: [] artifacttype: "" signatureinfo: [] ` + - `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z description: "" ` + + `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z ` + + `lastpulltimestamp: 0001-01-01T00:00:00Z pushtimestamp: 0001-01-01T00:00:00Z ` + + `taggedtimestamp: 0001-01-01T00:00:00Z description: "" ` + `issigned: false licenses: "" labels: "" title: "" source: "" documentation: "" ` + `authors: "" vendor: "" vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + @@ -454,7 +460,9 @@ func TestOutputFormatGQL(t *testing.T) { `history: [] vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + `referrers: [] artifacttype: "" signatureinfo: [] ` + - `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z description: "" ` + + `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z ` + + `lastpulltimestamp: 0001-01-01T00:00:00Z pushtimestamp: 0001-01-01T00:00:00Z ` + + `taggedtimestamp: 0001-01-01T00:00:00Z description: "" ` + `issigned: false licenses: "" labels: "" title: "" source: "" documentation: "" ` + `authors: "" vendor: "" vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + @@ -488,7 +496,9 @@ func TestOutputFormatGQL(t *testing.T) { `history: [] vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + `referrers: [] artifacttype: "" signatureinfo: [] ` + - `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z description: "" ` + + `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z ` + + `lastpulltimestamp: 0001-01-01T00:00:00Z pushtimestamp: 0001-01-01T00:00:00Z ` + + `taggedtimestamp: 0001-01-01T00:00:00Z description: "" ` + `issigned: false licenses: "" labels: "" title: "" source: "" documentation: "" ` + `authors: "" vendor: "" vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + @@ -504,7 +514,9 @@ func TestOutputFormatGQL(t *testing.T) { `history: [] vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + `referrers: [] artifacttype: "" signatureinfo: [] ` + - `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z description: "" ` + + `size: "528" downloadcount: 0 lastupdated: 2023-01-01T12:00:00Z ` + + `lastpulltimestamp: 0001-01-01T00:00:00Z pushtimestamp: 0001-01-01T00:00:00Z ` + + `taggedtimestamp: 0001-01-01T00:00:00Z description: "" ` + `issigned: false licenses: "" labels: "" title: "" source: "" documentation: "" ` + `authors: "" vendor: "" vulnerabilities: maxseverity: "" ` + `unknowncount: 0 lowcount: 0 mediumcount: 0 highcount: 0 criticalcount: 0 count: 0 ` + diff --git a/pkg/cli/client/search_cmd_test.go b/pkg/cli/client/search_cmd_test.go index 201d0cfa..c81031f1 100644 --- a/pkg/cli/client/search_cmd_test.go +++ b/pkg/cli/client/search_cmd_test.go @@ -485,7 +485,8 @@ func TestSearchCLI(t *testing.T) { space := regexp.MustCompile(`\s+`) str := strings.TrimSpace(space.ReplaceAllString(buff.String(), " ")) So(str, ShouldContainSubstring, "NAME SIZE LAST UPDATED DOWNLOADS STARS PLATFORMS") - So(str, ShouldContainSubstring, "repo/test/alpine 1.1kB 2010-01-01 01:01:01 +0000 UTC 0 0") + So(str, ShouldContainSubstring, "repo/test/alpine 1.1kB") + So(str, ShouldContainSubstring, "+0000 UTC 0 0") So(str, ShouldContainSubstring, "Os/Arch") So(str, ShouldContainSubstring, "linux/amd64") diff --git a/pkg/cli/server/verify_retention_test.go b/pkg/cli/server/verify_retention_test.go index fc761b09..54e1c5d3 100644 --- a/pkg/cli/server/verify_retention_test.go +++ b/pkg/cli/server/verify_retention_test.go @@ -476,7 +476,7 @@ func TestRetentionCheckWithRetentionEnabledAndRedisDriver(t *testing.T) { defer ctrlManager.StopServer() - os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "1s", configFile} + os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "2s", configFile} err = cli.NewServerRootCmd().Execute() So(err, ShouldBeNil) @@ -720,7 +720,7 @@ func TestRetentionCheckWithRetentionEnabled(t *testing.T) { gcDelay, _ := time.ParseDuration(testGCDelay) time.Sleep(gcDelay + 50*time.Millisecond) // wait for GC delay to pass - os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "1s", configFile} + os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "2s", configFile} err = cli.NewServerRootCmd().Execute() So(err, ShouldBeNil) @@ -935,7 +935,7 @@ func TestRetentionCheckWithDeleteReferrers(t *testing.T) { gcDelay, _ := time.ParseDuration(testGCDelay) time.Sleep(gcDelay + 50*time.Millisecond) // wait for GC delay to pass - os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "1s", configFile} + os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "2s", configFile} err = cli.NewServerRootCmd().Execute() So(err, ShouldBeNil) @@ -1080,7 +1080,7 @@ func TestRetentionCheckWithRetentionDisabled(t *testing.T) { gcDelay, _ := time.ParseDuration(testGCDelay) time.Sleep(gcDelay + 50*time.Millisecond) // wait for GC delay to pass - os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "1s", configFile} + os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "2s", configFile} err = cli.NewServerRootCmd().Execute() So(err, ShouldBeNil) @@ -1354,7 +1354,7 @@ func TestRetentionCheckWithSubpaths(t *testing.T) { gcDelay, _ := time.ParseDuration(testGCDelay) time.Sleep(gcDelay + 50*time.Millisecond) // wait for GC delay to pass - os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "1s", configFile} + os.Args = []string{"cli_test", "verify-feature", "retention", "-l", logFile, "-t", "2s", configFile} err = cli.NewServerRootCmd().Execute() So(err, ShouldBeNil) diff --git a/pkg/common/model.go b/pkg/common/model.go index ab3ad6a9..62f0b8e1 100644 --- a/pkg/common/model.go +++ b/pkg/common/model.go @@ -33,26 +33,29 @@ type PaginatedImagesResult struct { } type ImageSummary struct { - RepoName string `json:"repoName"` - Tag string `json:"tag"` - Digest string `json:"digest"` - MediaType string `json:"mediaType"` - Manifests []ManifestSummary `json:"manifests"` - Size string `json:"size"` - DownloadCount int `json:"downloadCount"` - LastUpdated time.Time `json:"lastUpdated"` - Description string `json:"description"` - IsSigned bool `json:"isSigned"` - Licenses string `json:"licenses"` - Labels string `json:"labels"` - Title string `json:"title"` - Source string `json:"source"` - Documentation string `json:"documentation"` - Authors string `json:"authors"` - Vendor string `json:"vendor"` - Vulnerabilities ImageVulnerabilitySummary `json:"vulnerabilities"` - Referrers []Referrer `json:"referrers"` - SignatureInfo []SignatureSummary `json:"signatureInfo"` + RepoName string `json:"repoName"` + Tag string `json:"tag"` + Digest string `json:"digest"` + MediaType string `json:"mediaType"` + Manifests []ManifestSummary `json:"manifests"` + Size string `json:"size"` + DownloadCount int `json:"downloadCount"` + LastUpdated time.Time `json:"lastUpdated"` + LastPullTimestamp time.Time `json:"lastPullTimestamp"` + PushTimestamp time.Time `json:"pushTimestamp"` + TaggedTimestamp time.Time `json:"taggedTimestamp"` + Description string `json:"description"` + IsSigned bool `json:"isSigned"` + Licenses string `json:"licenses"` + Labels string `json:"labels"` + Title string `json:"title"` + Source string `json:"source"` + Documentation string `json:"documentation"` + Authors string `json:"authors"` + Vendor string `json:"vendor"` + Vulnerabilities ImageVulnerabilitySummary `json:"vulnerabilities"` + Referrers []Referrer `json:"referrers"` + SignatureInfo []SignatureSummary `json:"signatureInfo"` } type ManifestSummary struct { diff --git a/pkg/extensions/search/convert/convert_test.go b/pkg/extensions/search/convert/convert_test.go index 8a86d19c..d217b129 100644 --- a/pkg/extensions/search/convert/convert_test.go +++ b/pkg/extensions/search/convert/convert_test.go @@ -1020,6 +1020,160 @@ func TestIndexAnnotations(t *testing.T) { }) } +func TestRepoMeta2RepoSummary(t *testing.T) { + ctx := context.Background() + + Convey("Test RepoMeta2RepoSummary LastUpdated with TaggedTimestamp", t, func() { + now := time.Now() + olderTime := now.Add(-2 * time.Hour) + newerTime := now.Add(-1 * time.Hour) + newestTime := now + futureTime := now.Add(1 * time.Hour) + + // Create a repo with multiple tags having different TaggedTimestamp values + repoMeta := mTypes.RepoMeta{ + Name: "test-repo", + Tags: map[mTypes.Tag]mTypes.Descriptor{ + "tag1": { + Digest: "sha256:digest1", + MediaType: "application/vnd.oci.image.manifest.v1+json", + TaggedTimestamp: olderTime, + }, + "tag2": { + Digest: "sha256:digest2", + MediaType: "application/vnd.oci.image.manifest.v1+json", + TaggedTimestamp: newerTime, + }, + "tag3": { + Digest: "sha256:digest3", + MediaType: "application/vnd.oci.image.manifest.v1+json", + TaggedTimestamp: futureTime, // This is the newest TaggedTimestamp + }, + }, + LastUpdatedImage: &mTypes.LastUpdatedImage{ + Descriptor: mTypes.Descriptor{ + Digest: "sha256:digest2", + MediaType: "application/vnd.oci.image.manifest.v1+json", + }, + Tag: "tag2", + LastUpdated: &newestTime, // This is newer than olderTime and newerTime, but older than futureTime + }, + } + + imageMetaMap := map[string]mTypes.ImageMeta{ + "sha256:digest2": { + Digest: godigest.FromString("sha256:digest2"), + MediaType: "application/vnd.oci.image.manifest.v1+json", + Manifests: []mTypes.ManifestMeta{ + { + Digest: godigest.FromString("sha256:digest2"), + Config: ispec.Image{ + Created: &newestTime, + }, + }, + }, + }, + } + + repoSummary := convert.RepoMeta2RepoSummary(ctx, repoMeta, imageMetaMap) + + // LastUpdated should be futureTime (the maximum of all TaggedTimestamp values and LastUpdatedImage.LastUpdated) + So(repoSummary, ShouldNotBeNil) + So(repoSummary.LastUpdated, ShouldNotBeNil) + So(*repoSummary.LastUpdated, ShouldEqual, futureTime) + }) + + Convey("Test RepoMeta2RepoSummary LastUpdated when TaggedTimestamp is older than LastUpdated", t, func() { + now := time.Now() + olderTime := now.Add(-2 * time.Hour) + newestTime := now + + repoMeta := mTypes.RepoMeta{ + Name: "test-repo", + Tags: map[mTypes.Tag]mTypes.Descriptor{ + "tag1": { + Digest: "sha256:digest1", + MediaType: "application/vnd.oci.image.manifest.v1+json", + TaggedTimestamp: olderTime, // Older than LastUpdated + }, + }, + LastUpdatedImage: &mTypes.LastUpdatedImage{ + Descriptor: mTypes.Descriptor{ + Digest: "sha256:digest1", + MediaType: "application/vnd.oci.image.manifest.v1+json", + }, + Tag: "tag1", + LastUpdated: &newestTime, + }, + } + + imageMetaMap := map[string]mTypes.ImageMeta{ + "sha256:digest1": { + Digest: godigest.FromString("sha256:digest1"), + MediaType: "application/vnd.oci.image.manifest.v1+json", + Manifests: []mTypes.ManifestMeta{ + { + Digest: godigest.FromString("sha256:digest1"), + Config: ispec.Image{ + Created: &newestTime, + }, + }, + }, + }, + } + + repoSummary := convert.RepoMeta2RepoSummary(ctx, repoMeta, imageMetaMap) + + // LastUpdated should be newestTime (the LastUpdatedImage.LastUpdated, which is newer than TaggedTimestamp) + So(repoSummary, ShouldNotBeNil) + So(repoSummary.LastUpdated, ShouldNotBeNil) + So(*repoSummary.LastUpdated, ShouldEqual, newestTime) + }) + + Convey("Test RepoMeta2RepoSummary LastUpdated with zero timestamps", t, func() { + zeroTime := time.Time{} + + repoMeta := mTypes.RepoMeta{ + Name: "test-repo", + Tags: map[mTypes.Tag]mTypes.Descriptor{ + "tag1": { + Digest: "sha256:digest1", + MediaType: "application/vnd.oci.image.manifest.v1+json", + TaggedTimestamp: zeroTime, + }, + }, + LastUpdatedImage: &mTypes.LastUpdatedImage{ + Descriptor: mTypes.Descriptor{ + Digest: "sha256:digest1", + MediaType: "application/vnd.oci.image.manifest.v1+json", + }, + Tag: "tag1", + LastUpdated: nil, + }, + } + + imageMetaMap := map[string]mTypes.ImageMeta{ + "sha256:digest1": { + Digest: godigest.FromString("sha256:digest1"), + MediaType: "application/vnd.oci.image.manifest.v1+json", + Manifests: []mTypes.ManifestMeta{ + { + Digest: godigest.FromString("sha256:digest1"), + Config: ispec.Image{}, + }, + }, + }, + } + + repoSummary := convert.RepoMeta2RepoSummary(ctx, repoMeta, imageMetaMap) + + // LastUpdated should be zero time when all timestamps are zero + So(repoSummary, ShouldNotBeNil) + So(repoSummary.LastUpdated, ShouldNotBeNil) + So(*repoSummary.LastUpdated, ShouldEqual, zeroTime) + }) +} + func TestConvertErrors(t *testing.T) { ctx := context.Background() log := log.NewTestLogger() diff --git a/pkg/extensions/search/convert/metadb.go b/pkg/extensions/search/convert/metadb.go index 08633ee8..5b29b354 100644 --- a/pkg/extensions/search/convert/metadb.go +++ b/pkg/extensions/search/convert/metadb.go @@ -324,24 +324,38 @@ func RepoMeta2RepoSummary(ctx context.Context, repoMeta mTypes.RepoMeta, imageMetaMap map[string]mTypes.ImageMeta, ) *gql_generated.RepoSummary { var ( - repoName = repoMeta.Name - lastUpdatedImage = deref(repoMeta.LastUpdatedImage, mTypes.LastUpdatedImage{}) - lastUpdatedImageMeta = imageMetaMap[lastUpdatedImage.Digest] - lastUpdatedTag = lastUpdatedImage.Tag - repoLastUpdatedTimestamp = lastUpdatedImage.LastUpdated - repoPlatforms = repoMeta.Platforms - repoVendors = repoMeta.Vendors - repoDownloadCount = repoMeta.DownloadCount - repoStarCount = repoMeta.StarCount - repoIsUserStarred = repoMeta.IsStarred // value specific to the current user - repoIsUserBookMarked = repoMeta.IsBookmarked // value specific to the current user - repoSize = repoMeta.Size + repoName = repoMeta.Name + lastUpdatedImage = deref(repoMeta.LastUpdatedImage, mTypes.LastUpdatedImage{}) + lastUpdatedImageMeta = imageMetaMap[lastUpdatedImage.Digest] + lastUpdatedTag = lastUpdatedImage.Tag + repoPlatforms = repoMeta.Platforms + repoVendors = repoMeta.Vendors + repoDownloadCount = repoMeta.DownloadCount + repoStarCount = repoMeta.StarCount + repoIsUserStarred = repoMeta.IsStarred // value specific to the current user + repoIsUserBookMarked = repoMeta.IsBookmarked // value specific to the current user + repoSize = repoMeta.Size ) - if repoLastUpdatedTimestamp == nil { - repoLastUpdatedTimestamp = &time.Time{} + // Compute LastUpdated as the latest of: + // 1. The LastUpdated timestamp of the last updated image + // 2. All TaggedTimestamp values of all tags in the repository + var maxTimestamp time.Time + + // Start with the LastUpdated from the last updated image + if lastUpdatedImage.LastUpdated != nil { + maxTimestamp = *lastUpdatedImage.LastUpdated } + // Check all TaggedTimestamp values from all tags + for _, descriptor := range repoMeta.Tags { + if !descriptor.TaggedTimestamp.IsZero() && descriptor.TaggedTimestamp.After(maxTimestamp) { + maxTimestamp = descriptor.TaggedTimestamp + } + } + + repoLastUpdatedTimestamp := &maxTimestamp + imageSummary, _, err := FullImageMeta2ImageSummary(ctx, GetFullImageMeta(lastUpdatedTag, repoMeta, lastUpdatedImageMeta)) _ = err diff --git a/pkg/extensions/search/cve/update_test.go b/pkg/extensions/search/cve/update_test.go index 634aca56..f6c649b6 100644 --- a/pkg/extensions/search/cve/update_test.go +++ b/pkg/extensions/search/cve/update_test.go @@ -66,7 +66,7 @@ func TestCVEDBGenerator(t *testing.T) { // Wait for trivy db to download found, err := test.ReadLogFileAndCountStringOccurence(logPath, - "cve-db update completed, next update scheduled after interval", 140*time.Second, 2) + "cve-db update completed, next update scheduled after interval", 240*time.Second, 2) So(err, ShouldBeNil) So(found, ShouldBeTrue) }) diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index 979d554d..2d4f62da 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -3403,6 +3403,7 @@ func TestGlobalSearch(t *testing.T) { //nolint: gocyclo allExpectedRepoInfoMap := make(map[string]zcommon.RepoInfo) allExpectedImageSummaryMap := make(map[string]zcommon.ImageSummary) + expectedLastUpdatedMap := make(map[string]time.Time) for _, repo := range repos { repoInfo, err := olu.GetExpandedRepoInfo(repo) @@ -3413,6 +3414,23 @@ func TestGlobalSearch(t *testing.T) { //nolint: gocyclo imageName := fmt.Sprintf("%s:%s", repo, image.Tag) allExpectedImageSummaryMap[imageName] = image } + + // Compute expected LastUpdated as the maximum of: + // 1. NewestImage.LastUpdated (from the last updated image) + // 2. All TaggedTimestamp values from all tags in the repository + repoMeta, err := ctlr.MetaDB.GetRepoMeta(context.Background(), repo) + So(err, ShouldBeNil) + + expectedLastUpdated := repoInfo.Summary.NewestImage.LastUpdated + + // Check all TaggedTimestamp values from all tags + for _, descriptor := range repoMeta.Tags { + if !descriptor.TaggedTimestamp.IsZero() && descriptor.TaggedTimestamp.After(expectedLastUpdated) { + expectedLastUpdated = descriptor.TaggedTimestamp + } + } + + expectedLastUpdatedMap[repo] = expectedLastUpdated } query := ` @@ -3493,10 +3511,16 @@ func TestGlobalSearch(t *testing.T) { //nolint: gocyclo // Check if data in NewestImage is consistent with the data in RepoSummary So(repoName, ShouldEqual, repoSummary.NewestImage.RepoName) So(repoSummary.Name, ShouldEqual, repoSummary.NewestImage.RepoName) - So(repoSummary.LastUpdated, ShouldEqual, repoSummary.NewestImage.LastUpdated) + + // Verify the actual LastUpdated matches the computed expected value + expectedLastUpdated, exists := expectedLastUpdatedMap[repoName] + So(exists, ShouldBeTrue) + So(repoSummary.LastUpdated, ShouldEqual, expectedLastUpdated) // The data in the RepoSummary returned from the request matches the data returned from the disk repoInfo := allExpectedRepoInfoMap[repoName] + // Update the expected LastUpdated to account for TaggedTimestamp (which is not available from disk) + repoInfo.Summary.LastUpdated = expectedLastUpdated t.Logf("Validate repo summary returned by global search with vulnerability scanning disabled") verifyRepoSummaryFields(t, &repoSummary, &repoInfo.Summary) @@ -3747,6 +3771,7 @@ func TestGlobalSearch(t *testing.T) { //nolint: gocyclo allExpectedRepoInfoMap := make(map[string]zcommon.RepoInfo) allExpectedImageSummaryMap := make(map[string]zcommon.ImageSummary) + expectedLastUpdatedMap := make(map[string]time.Time) for _, repo := range repos { repoInfo, err := olu.GetExpandedRepoInfo(repo) @@ -3757,6 +3782,23 @@ func TestGlobalSearch(t *testing.T) { //nolint: gocyclo imageName := fmt.Sprintf("%s:%s", repo, image.Tag) allExpectedImageSummaryMap[imageName] = image } + + // Compute expected LastUpdated as the maximum of: + // 1. NewestImage.LastUpdated (from the last updated image) + // 2. All TaggedTimestamp values from all tags in the repository + repoMeta, err := ctlr.MetaDB.GetRepoMeta(context.Background(), repo) + So(err, ShouldBeNil) + + expectedLastUpdated := repoInfo.Summary.NewestImage.LastUpdated + + // Check all TaggedTimestamp values from all tags + for _, descriptor := range repoMeta.Tags { + if !descriptor.TaggedTimestamp.IsZero() && descriptor.TaggedTimestamp.After(expectedLastUpdated) { + expectedLastUpdated = descriptor.TaggedTimestamp + } + } + + expectedLastUpdatedMap[repo] = expectedLastUpdated } query := ` @@ -3833,10 +3875,16 @@ func TestGlobalSearch(t *testing.T) { //nolint: gocyclo // Check if data in NewestImage is consistent with the data in RepoSummary So(repoName, ShouldEqual, repoSummary.NewestImage.RepoName) So(repoSummary.Name, ShouldEqual, repoSummary.NewestImage.RepoName) - So(repoSummary.LastUpdated, ShouldEqual, repoSummary.NewestImage.LastUpdated) + + // Verify the actual LastUpdated matches the computed expected value + expectedLastUpdated, exists := expectedLastUpdatedMap[repoName] + So(exists, ShouldBeTrue) + So(repoSummary.LastUpdated, ShouldEqual, expectedLastUpdated) // The data in the RepoSummary returned from the request matches the data returned from the disk repoInfo := allExpectedRepoInfoMap[repoName] + // Update the expected LastUpdated to account for TaggedTimestamp (which is not available from disk) + repoInfo.Summary.LastUpdated = expectedLastUpdated t.Logf("Validate repo summary returned by global search with vulnerability scanning enabled") verifyRepoSummaryFields(t, &repoSummary, &repoInfo.Summary) diff --git a/pkg/meta/boltdb/boltdb.go b/pkg/meta/boltdb/boltdb.go index d21ba42a..7f21a00a 100644 --- a/pkg/meta/boltdb/boltdb.go +++ b/pkg/meta/boltdb/boltdb.go @@ -1182,7 +1182,12 @@ func (bdw *BoltDB) ResetRepoReferences(repo string, tagsToKeep map[string]bool) repoMetaBlob := buck.Get([]byte(repo)) protoRepoMeta, err := unmarshalProtoRepoMeta(repo, repoMetaBlob) - if err != nil && !errors.Is(err, zerr.ErrRepoMetaNotFound) { + if err != nil { + if errors.Is(err, zerr.ErrRepoMetaNotFound) { + // Repo doesn't exist, nothing to reset + return nil + } + return err } diff --git a/pkg/meta/boltdb/boltdb_test.go b/pkg/meta/boltdb/boltdb_test.go index 8790b6f1..e7ab6342 100644 --- a/pkg/meta/boltdb/boltdb_test.go +++ b/pkg/meta/boltdb/boltdb_test.go @@ -308,6 +308,43 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ResetRepoReferences", func() { + Convey("repo doesn't exist - returns early without error", func() { + ctx := context.Background() + + // Verify repo doesn't exist + _, err := boltdbWrapper.GetRepoMeta(ctx, "nonexistent-repo") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // ResetRepoReferences should return early without error + err = boltdbWrapper.ResetRepoReferences("nonexistent-repo", nil) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist + _, err = boltdbWrapper.GetRepoMeta(ctx, "nonexistent-repo") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + }) + + Convey("repo doesn't exist with tagsToKeep - returns early without error", func() { + ctx := context.Background() + + // Verify repo doesn't exist + _, err := boltdbWrapper.GetRepoMeta(ctx, "nonexistent-repo2") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // ResetRepoReferences should return early without error even with tagsToKeep + tagsToKeep := map[string]bool{"tag1": true} + err = boltdbWrapper.ResetRepoReferences("nonexistent-repo2", tagsToKeep) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist + _, err = boltdbWrapper.GetRepoMeta(ctx, "nonexistent-repo2") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + }) + Convey("unmarshalProtoRepoMeta error", func() { err := setRepoMeta("repo", badProtoBlob, boltdbWrapper.DB) So(err, ShouldBeNil) diff --git a/pkg/meta/common/common.go b/pkg/meta/common/common.go index a9e0d0b0..7c75f8b6 100644 --- a/pkg/meta/common/common.go +++ b/pkg/meta/common/common.go @@ -258,6 +258,7 @@ func AddImageMetaToRepoMeta(repoMeta *proto_go.RepoMeta, repoBlobs *proto_go.Rep repoMeta.Size = size imageBlobInfo := repoBlobs.Blobs[imageMeta.Digest.String()] + repoMeta.LastUpdatedImage = mConvert.GetProtoEarlierUpdatedImage(repoMeta.LastUpdatedImage, &proto_go.RepoLastUpdatedImage{ LastUpdated: imageBlobInfo.LastUpdated, diff --git a/pkg/meta/dynamodb/dynamodb.go b/pkg/meta/dynamodb/dynamodb.go index 562f3132..06c25106 100644 --- a/pkg/meta/dynamodb/dynamodb.go +++ b/pkg/meta/dynamodb/dynamodb.go @@ -134,6 +134,11 @@ func (dwr *DynamoDB) GetRepoLastUpdated(repo string) time.Time { repoLastUpdatedBlob := []byte{} if resp.Item != nil { + // Check if RepoLastUpdated attribute exists in the item + if _, exists := resp.Item["RepoLastUpdated"]; !exists { + return time.Time{} + } + err = attributevalue.Unmarshal(resp.Item["RepoLastUpdated"], &repoLastUpdatedBlob) if err != nil { return time.Time{} @@ -144,7 +149,18 @@ func (dwr *DynamoDB) GetRepoLastUpdated(repo string) time.Time { if err != nil { return time.Time{} } + } else { + // Empty blob means no timestamp was set + return time.Time{} } + } else { + // Item doesn't exist, return zero time + return time.Time{} + } + + // Check if the timestamp is zero before converting + if protoRepoLastUpdated.Seconds == 0 && protoRepoLastUpdated.Nanos == 0 { + return time.Time{} } lastUpdated := *mConvert.GetTime(protoRepoLastUpdated) @@ -881,6 +897,11 @@ func getProtoImageMetaFromAttribute(imageMetaAttribute types.AttributeValue) (*p func (dwr *DynamoDB) ResetRepoReferences(repo string, tagsToKeep map[string]bool) error { protoRepoMeta, err := dwr.getProtoRepoMeta(context.Background(), repo) if err != nil { + if errors.Is(err, zerr.ErrRepoMetaNotFound) { + // Repo doesn't exist, nothing to reset + return nil + } + return err } diff --git a/pkg/meta/dynamodb/dynamodb_test.go b/pkg/meta/dynamodb/dynamodb_test.go index c32b9a5d..182891cb 100644 --- a/pkg/meta/dynamodb/dynamodb_test.go +++ b/pkg/meta/dynamodb/dynamodb_test.go @@ -2,6 +2,7 @@ package dynamodb_test import ( "context" + "errors" "os" "testing" "time" @@ -15,10 +16,14 @@ import ( godigest "github.com/opencontainers/go-digest" ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" + zerr "zotregistry.dev/zot/v2/errors" "zotregistry.dev/zot/v2/pkg/extensions/imagetrust" "zotregistry.dev/zot/v2/pkg/log" mdynamodb "zotregistry.dev/zot/v2/pkg/meta/dynamodb" + proto_go "zotregistry.dev/zot/v2/pkg/meta/proto/gen" mTypes "zotregistry.dev/zot/v2/pkg/meta/types" reqCtx "zotregistry.dev/zot/v2/pkg/requestcontext" . "zotregistry.dev/zot/v2/pkg/test/image-utils" @@ -426,6 +431,39 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ResetRepoReferences", func() { + Convey("repo doesn't exist - returns early without error", func() { + // Verify repo doesn't exist + _, err := dynamoWrapper.GetRepoMeta(ctx, "nonexistent-repo") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // ResetRepoReferences should return early without error + err = dynamoWrapper.ResetRepoReferences("nonexistent-repo", nil) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist + _, err = dynamoWrapper.GetRepoMeta(ctx, "nonexistent-repo") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + }) + + Convey("repo doesn't exist with tagsToKeep - returns early without error", func() { + // Verify repo doesn't exist + _, err := dynamoWrapper.GetRepoMeta(ctx, "nonexistent-repo2") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // ResetRepoReferences should return early without error even with tagsToKeep + tagsToKeep := map[string]bool{"tag1": true} + err = dynamoWrapper.ResetRepoReferences("nonexistent-repo2", tagsToKeep) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist + _, err = dynamoWrapper.GetRepoMeta(ctx, "nonexistent-repo2") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + }) + Convey("unmarshalProtoRepoMeta error", func() { err := setRepoMeta("repo", badProtoBlob, dynamoWrapper) So(err, ShouldBeNil) @@ -813,6 +851,58 @@ func TestWrapperErrors(t *testing.T) { err := dynamoWrapper.SetRepoReference(ctx, "repo", "tag", image.AsImageMeta()) So(err, ShouldNotBeNil) }) + Convey("setRepoBlobsInfo fails", func() { + // First set up image meta and repo meta successfully + err := dynamoWrapper.SetImageMeta(imageMeta.Digest, imageMeta) //nolint: contextcheck + So(err, ShouldBeNil) + + // Set up repo meta manually so getProtoRepoMeta succeeds + err = dynamoWrapper.SetRepoMeta("repo", mTypes.RepoMeta{ //nolint: contextcheck + Name: "repo", + }) + So(err, ShouldBeNil) + + // Set up repo blobs manually so getProtoRepoBlobs succeeds + repoBlobs := &proto_go.RepoBlobs{ + Name: "repo", + } + repoBlobsBytes, err := proto.Marshal(repoBlobs) + So(err, ShouldBeNil) + err = setRepoBlobInfo("repo", repoBlobsBytes, dynamoWrapper) //nolint: contextcheck + So(err, ShouldBeNil) + + // Now set bad table name to cause setRepoBlobsInfo to fail + dynamoWrapper.RepoBlobsTablename = badTablename + + err = dynamoWrapper.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldNotBeNil) + }) + Convey("setProtoRepoMeta fails", func() { + // First set up image meta and repo blobs successfully + err := dynamoWrapper.SetImageMeta(imageMeta.Digest, imageMeta) //nolint: contextcheck + So(err, ShouldBeNil) + + // Set up repo meta manually so getProtoRepoMeta succeeds + err = dynamoWrapper.SetRepoMeta("repo", mTypes.RepoMeta{ //nolint: contextcheck + Name: "repo", + }) + So(err, ShouldBeNil) + + // Set up repo blobs manually so getProtoRepoBlobs succeeds + repoBlobs := &proto_go.RepoBlobs{ + Name: "repo", + } + repoBlobsBytes, err := proto.Marshal(repoBlobs) + So(err, ShouldBeNil) + err = setRepoBlobInfo("repo", repoBlobsBytes, dynamoWrapper) //nolint: contextcheck + So(err, ShouldBeNil) + + // Now set bad table name to cause setProtoRepoMeta to fail + dynamoWrapper.RepoMetaTablename = badTablename + + err = dynamoWrapper.SetRepoReference(ctx, "repo", "tag", imageMeta) + So(err, ShouldNotBeNil) + }) }) Convey("GetProtoImageMeta", func() { @@ -1088,6 +1178,72 @@ func TestWrapperErrors(t *testing.T) { lastUpdated := dynamoWrapper.GetRepoLastUpdated("repo") So(lastUpdated, ShouldEqual, time.Time{}) }) + + Convey("item doesn't exist", func() { + // Delete the repo to ensure item doesn't exist + err := dynamoWrapper.DeleteRepoMeta("nonexistent-repo") + So(err, ShouldBeNil) + + lastUpdated := dynamoWrapper.GetRepoLastUpdated("nonexistent-repo") + So(lastUpdated, ShouldEqual, time.Time{}) + }) + + Convey("item exists but RepoLastUpdated attribute missing", func() { + // Create an item in RepoBlobsTablename without RepoLastUpdated attribute + // by setting RepoBlobsInfo only + repoBlobs := &proto_go.RepoBlobs{ + Name: "repo-no-timestamp", + } + repoBlobsBytes, err := proto.Marshal(repoBlobs) + So(err, ShouldBeNil) + + err = setRepoBlobInfo("repo-no-timestamp", repoBlobsBytes, dynamoWrapper) + So(err, ShouldBeNil) + + lastUpdated := dynamoWrapper.GetRepoLastUpdated("repo-no-timestamp") + So(lastUpdated, ShouldEqual, time.Time{}) + }) + + Convey("empty blob", func() { + // Set an empty blob for RepoLastUpdated + err := setRepoLastUpdated("repo-empty-blob", []byte{}, dynamoWrapper) + So(err, ShouldBeNil) + + lastUpdated := dynamoWrapper.GetRepoLastUpdated("repo-empty-blob") + So(lastUpdated, ShouldEqual, time.Time{}) + }) + + Convey("zero timestamp", func() { + // Set a zero timestamp + zeroTime := ×tamppb.Timestamp{ + Seconds: 0, + Nanos: 0, + } + zeroTimeBlob, err := proto.Marshal(zeroTime) + So(err, ShouldBeNil) + + err = setRepoLastUpdated("repo-zero-timestamp", zeroTimeBlob, dynamoWrapper) + So(err, ShouldBeNil) + + lastUpdated := dynamoWrapper.GetRepoLastUpdated("repo-zero-timestamp") + So(lastUpdated, ShouldEqual, time.Time{}) + }) + + Convey("valid timestamp", func() { + // Set a valid timestamp + validTime := timestamppb.New(time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)) + validTimeBlob, err := proto.Marshal(validTime) + So(err, ShouldBeNil) + + err = setRepoLastUpdated("repo-valid-timestamp", validTimeBlob, dynamoWrapper) + So(err, ShouldBeNil) + + lastUpdated := dynamoWrapper.GetRepoLastUpdated("repo-valid-timestamp") + So(lastUpdated, ShouldNotEqual, time.Time{}) + So(lastUpdated.Year(), ShouldEqual, 2024) + So(lastUpdated.Month(), ShouldEqual, time.January) + So(lastUpdated.Day(), ShouldEqual, 1) + }) }) Convey("DeleteUserAPIKey returns nil", func() { diff --git a/pkg/meta/parse.go b/pkg/meta/parse.go index 9dd7e769..77e9783c 100644 --- a/pkg/meta/parse.go +++ b/pkg/meta/parse.go @@ -69,7 +69,9 @@ func ParseStorage(metaDB mTypes.MetaDB, storeController stypes.StoreController, metaLastUpdated := metaDB.GetRepoLastUpdated(repo) - if storageLastUpdated.Before(metaLastUpdated) { + // If repo metadata doesn't exist (zero time), always parse it + // Otherwise, only parse if storage is newer than metadata + if !metaLastUpdated.IsZero() && storageLastUpdated.Before(metaLastUpdated) { continue } diff --git a/pkg/meta/parse_test.go b/pkg/meta/parse_test.go index 624f6760..9e3bd45b 100644 --- a/pkg/meta/parse_test.go +++ b/pkg/meta/parse_test.go @@ -3,6 +3,7 @@ package meta_test import ( "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -832,6 +833,54 @@ func RunParseStorageTests(rootDir string, metaDB mTypes.MetaDB, log log.Logger) // - defaultRepo (from default store, no prefix) So(len(repoMetaList), ShouldEqual, 3) }) + + Convey("ParseStorage should parse repos without metadata", func() { + imageStore := local.NewImageStore(rootDir, false, false, + log, monitoring.NewMetricsServer(false, log), nil, nil, nil, nil) + + storeController := storage.StoreController{DefaultStore: imageStore} + + // Create a repo in storage + testRepo := "repo-without-metadata" + + // Ensure repo doesn't exist in metadata (clean up from previous test runs if needed) + err := metaDB.DeleteRepoMeta(testRepo) + So(err, ShouldBeNil) + + // Verify GetRepoLastUpdated returns zero (repo doesn't exist in metadata) + metaLastUpdated := metaDB.GetRepoLastUpdated(testRepo) + So(metaLastUpdated.IsZero(), ShouldBeTrue) + + image := CreateRandomImage() + + err = WriteImageToFileSystem(image, testRepo, "tag1", storeController) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist in metadata (GetRepoMeta should return ErrRepoMetaNotFound) + _, err = metaDB.GetRepoMeta(ctx, testRepo) + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // Verify GetRepoLastUpdated still returns zero + metaLastUpdated = metaDB.GetRepoLastUpdated(testRepo) + So(metaLastUpdated.IsZero(), ShouldBeTrue) + + // Parse storage - repos without metadata (zero time) are always parsed + // Note: This behavior is the same with or without the !metaLastUpdated.IsZero() guard + // because storageLastUpdated.Before(time.Time{}) is always false for valid timestamps + err = meta.ParseStorage(metaDB, storeController, log) //nolint: contextcheck + So(err, ShouldBeNil) + + // Verify repo metadata was created + repoMeta, err := metaDB.GetRepoMeta(ctx, testRepo) + So(err, ShouldBeNil) + So(repoMeta.Name, ShouldEqual, testRepo) + So(repoMeta.Tags, ShouldContainKey, "tag1") + + // Verify GetRepoLastUpdated now returns a non-zero time + metaLastUpdated = metaDB.GetRepoLastUpdated(testRepo) + So(metaLastUpdated.IsZero(), ShouldBeFalse) + }) } func TestGetSignatureLayersInfo(t *testing.T) { diff --git a/pkg/meta/redis/redis.go b/pkg/meta/redis/redis.go index 938dc49d..5bd780f2 100644 --- a/pkg/meta/redis/redis.go +++ b/pkg/meta/redis/redis.go @@ -2009,7 +2009,12 @@ func (rc *RedisDB) ResetRepoReferences(repo string, tagsToKeep map[string]bool) err := rc.withRSLocks(ctx, []string{rc.getRepoLockKey(repo)}, func() error { protoRepoMeta, err := rc.getProtoRepoMeta(ctx, repo) - if err != nil && !errors.Is(err, zerr.ErrRepoMetaNotFound) { + if err != nil { + if errors.Is(err, zerr.ErrRepoMetaNotFound) { + // Repo doesn't exist, nothing to reset + return nil + } + return err } diff --git a/pkg/meta/redis/redis_test.go b/pkg/meta/redis/redis_test.go index 44d9c7a6..13decdb0 100644 --- a/pkg/meta/redis/redis_test.go +++ b/pkg/meta/redis/redis_test.go @@ -16,6 +16,7 @@ import ( . "github.com/smartystreets/goconvey/convey" "google.golang.org/protobuf/proto" + zerr "zotregistry.dev/zot/v2/errors" "zotregistry.dev/zot/v2/pkg/log" proto_go "zotregistry.dev/zot/v2/pkg/meta/proto/gen" "zotregistry.dev/zot/v2/pkg/meta/redis" @@ -811,6 +812,39 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ResetRepoReferences", func() { + Convey("repo doesn't exist - returns early without error", func() { + // Verify repo doesn't exist + _, err := metaDB.GetRepoMeta(ctx, "nonexistent-repo") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // ResetRepoReferences should return early without error + err = metaDB.ResetRepoReferences("nonexistent-repo", nil) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist + _, err = metaDB.GetRepoMeta(ctx, "nonexistent-repo") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + }) + + Convey("repo doesn't exist with tagsToKeep - returns early without error", func() { + // Verify repo doesn't exist + _, err := metaDB.GetRepoMeta(ctx, "nonexistent-repo2") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + + // ResetRepoReferences should return early without error even with tagsToKeep + tagsToKeep := map[string]bool{"tag1": true} + err = metaDB.ResetRepoReferences("nonexistent-repo2", tagsToKeep) + So(err, ShouldBeNil) + + // Verify repo still doesn't exist + _, err = metaDB.GetRepoMeta(ctx, "nonexistent-repo2") + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoMetaNotFound), ShouldBeTrue) + }) + Convey("unmarshalProtoRepoMeta error", func() { err := setRepoMeta("repo", badProtoBlob, client) So(err, ShouldBeNil)