diff --git a/pkg/extensions/search/cve/scan_test.go b/pkg/extensions/search/cve/scan_test.go index e030b6b3..dd6db8d3 100644 --- a/pkg/extensions/search/cve/scan_test.go +++ b/pkg/extensions/search/cve/scan_test.go @@ -384,6 +384,11 @@ func TestScanGeneratorWithMockedData(t *testing.T) { //nolint: gocyclo return false, err } + // If all manifests are missing (e.g., from an index), Manifests will be empty + if len(manifestData.Manifests) == 0 { + return false, nil + } + for _, imageLayer := range manifestData.Manifests[0].Manifest.Layers { switch imageLayer.MediaType { case ispec.MediaTypeImageLayerGzip, ispec.MediaTypeImageLayer, string(regTypes.DockerLayer): diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index 4c76d527..631f0077 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -7406,6 +7406,98 @@ type repoRef struct { Tag string } +func TestSearchWithMissingManifest(t *testing.T) { + Convey("Search with missing manifest", t, func() { + dir := t.TempDir() + + // 1. Write the image to the disk + log := log.NewTestLogger() + storeCtlr := ociutils.GetDefaultStoreController(dir, log) + + // Create a multiarch image with exactly 2 manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + + // Write the multiarch image to filesystem + err := WriteMultiArchImageToFileSystem(multiarchImage, "testrepo", "latest", storeCtlr) + So(err, ShouldBeNil) + + // Get the image store to access index and manifests + imageStore := storeCtlr.GetDefaultImageStore() + + // Get the index content to find all manifest digests + indexBlob, err := imageStore.GetIndexContent("testrepo") + So(err, ShouldBeNil) + + var indexContent ispec.Index + err = json.Unmarshal(indexBlob, &indexContent) + So(err, ShouldBeNil) + + So(len(indexContent.Manifests), ShouldBeGreaterThanOrEqualTo, 2) + + // Get the first manifest digest to delete + firstManifestDigest := indexContent.Manifests[0].Digest + + // Get the second manifest digest (should remain valid) + secondManifestDigest := indexContent.Manifests[1].Digest + + // 2. Delete the manifest from the disk + manifestBlobPath := path.Join(dir, "testrepo", "blobs", "sha256", firstManifestDigest.Encoded()) + err = os.Remove(manifestBlobPath) + So(err, ShouldBeNil) + + // 3. Start the controller (MetaDB parsing would be done in the background) + port := GetFreePort() + baseURL := GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + conf.Storage.RootDirectory = dir + defaultVal := true + conf.Extensions = &extconf.ExtensionConfig{ + Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}}, + } + + conf.Extensions.Search.CVE = nil + + ctlr := api.NewController(conf) + + ctlrManager := NewControllerManager(ctlr) + ctlrManager.StartAndWait(port) + defer ctlrManager.StopServer() + + // Search for the repository + query := ` + { + GlobalSearch(query:"testrepo:latest"){ + Images { + RepoName Tag + Manifests { + Digest + } + } + } + }` + + 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) + + // Verify we found the image + So(responseStruct.GlobalSearchResult.GlobalSearch.Images, ShouldNotBeEmpty) + foundImage := responseStruct.GlobalSearchResult.GlobalSearch.Images[0] + So(foundImage.RepoName, ShouldEqual, "testrepo") + So(foundImage.Tag, ShouldEqual, "latest") + + // Verify only the valid manifest is found in search results (missing one was skipped by ParseStorage) + So(len(foundImage.Manifests), ShouldEqual, 1) + So(foundImage.Manifests[0].Digest, ShouldEqual, secondManifestDigest.String()) + }) +} + func deleteUsedImages(repoTags []repoRef, baseURL string) { for _, image := range repoTags { status, err := DeleteImage(image.Repo, image.Tag, baseURL) diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index 784fb352..be13ed83 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -12,6 +12,7 @@ import ( "path" "strings" + "github.com/distribution/distribution/v3/registry/storage/driver" godigest "github.com/opencontainers/go-digest" ispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/regclient/regclient/types/mediatype" @@ -227,11 +228,27 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri return err } + var firstMissingErr error + for _, manifest := range indexManifest.Manifests { reference := GetDescriptorReference(manifest) manifestBuf, err := tempImageStore.GetBlobContent(repo, manifest.Digest) if err != nil { + // Handle missing manifest blobs gracefully - log warning and continue with other manifests + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + if firstMissingErr == nil { + firstMissingErr = err + } + + registry.log.Warn().Err(err).Str("dir", path.Join(tempImageStore.RootDir(), repo)). + Str("digest", manifest.Digest.String()). + Msg("skipping missing manifest blob in image index, continuing sync with other manifests") + + continue + } + registry.log.Error().Str("errorType", common.TypeOf(err)). Err(err).Str("dir", path.Join(tempImageStore.RootDir(), repo)).Str("digest", manifest.Digest.String()). Msg("failed find manifest which is part of an image index") @@ -254,6 +271,11 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri } } + // Return error if we encountered any missing manifests + if firstMissingErr != nil { + return firstMissingErr + } + _, _, err := imageStore.PutImageManifest(repo, reference, desc.MediaType, manifestContent) if err != nil { registry.log.Error().Str("errorType", common.TypeOf(err)).Str("repo", repo).Str("reference", reference). diff --git a/pkg/meta/boltdb/boltdb.go b/pkg/meta/boltdb/boltdb.go index 87e74f17..2b712487 100644 --- a/pkg/meta/boltdb/boltdb.go +++ b/pkg/meta/boltdb/boltdb.go @@ -500,6 +500,11 @@ func getAllContainedMeta(imageBuck *bbolt.Bucket, imageIndexData *proto_go.Image imageManifestData, err := getProtoImageMeta(imageBuck, manifest.Digest) if err != nil { + // Skip manifests that don't have MetaDB entries (missing from storage) + if errors.Is(err, zerr.ErrImageMetaNotFound) { + continue + } + return imageMetaList, manifestDataList, err } @@ -511,6 +516,8 @@ func getAllContainedMeta(imageBuck *bbolt.Bucket, imageIndexData *proto_go.Image compat.IsCompatibleManifestListMediaType(imageManifestData.MediaType) { partialImageDataList, partialManifestDataList, err := getAllContainedMeta(imageBuck, imageManifestData) if err != nil { + // getAllContainedMeta skips missing items internally, so any error returned + // is a real error that should be propagated return imageMetaList, manifestDataList, err } diff --git a/pkg/meta/boltdb/boltdb_test.go b/pkg/meta/boltdb/boltdb_test.go index cc35fff3..7d4f53d2 100644 --- a/pkg/meta/boltdb/boltdb_test.go +++ b/pkg/meta/boltdb/boltdb_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/base64" + "errors" "math" "testing" "time" @@ -32,6 +33,8 @@ func (its imgTrustStore) VerifySignature( return "", time.Time{}, false, nil } +var errImageMetaBucketNotFound = errors.New("ImageMeta bucket not found") + func TestWrapperErrors(t *testing.T) { image := CreateDefaultImage() imageMeta := image.AsImageMeta() @@ -302,23 +305,63 @@ func TestWrapperErrors(t *testing.T) { So(err, ShouldNotBeNil) }) - Convey("image is index, fail to get manifests", func() { + Convey("image is index, missing manifests are skipped gracefully", func() { + err := boltdbWrapper.SetRepoReference(ctx, "repo", "tag", multiarchImageMeta) + So(err, ShouldBeNil) + + // Missing manifests are skipped gracefully, so GetFullImageMeta succeeds + // but returns an index with no manifests + fullImageMeta, err := boltdbWrapper.GetFullImageMeta(ctx, "repo", "tag") + So(err, ShouldBeNil) + So(len(fullImageMeta.Manifests), ShouldEqual, 0) + }) + + Convey("image is index, corrupted manifest data returns error", func() { + // Create a multiarch image with multiple manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + multiarchImageMeta := multiarchImage.AsImageMeta() err := boltdbWrapper.SetImageMeta(multiarchImageMeta.Digest, multiarchImageMeta) So(err, ShouldBeNil) - err = boltdbWrapper.SetRepoMeta("repo", mTypes.RepoMeta{ - Name: "repo", - Tags: map[mTypes.Tag]mTypes.Descriptor{ - "tag": { - MediaType: ispec.MediaTypeImageIndex, - Digest: multiarchImageMeta.Digest.String(), - }, - }, + // Store the first manifest normally + firstManifest := multiarchImage.Images[0] + firstManifestMeta := firstManifest.AsImageMeta() + err = boltdbWrapper.SetImageMeta(firstManifestMeta.Digest, firstManifestMeta) + So(err, ShouldBeNil) + + // Store the second manifest normally first, then corrupt it + secondManifest := multiarchImage.Images[1] + secondManifestMeta := secondManifest.AsImageMeta() + err = boltdbWrapper.SetImageMeta(secondManifestMeta.Digest, secondManifestMeta) + So(err, ShouldBeNil) + + secondManifestDigest := secondManifest.ManifestDescriptor.Digest + + // Corrupt the data for the second manifest by storing invalid protobuf data + // This will cause getProtoImageMeta to return an unmarshaling error + // which is not ErrImageMetaNotFound, so it will propagate through getAllContainedMeta + corruptedData := []byte("invalid protobuf data") + + // Access BoltDB directly to corrupt the data + err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { + imageBuck := tx.Bucket([]byte(boltdb.ImageMetaBuck)) + if imageBuck == nil { + return errImageMetaBucketNotFound + } + // Store corrupted protobuf data + return imageBuck.Put([]byte(secondManifestDigest.String()), corruptedData) }) So(err, ShouldBeNil) - _, err = boltdbWrapper.GetFullImageMeta(ctx, "repo", "tag") + err = boltdbWrapper.SetRepoReference(ctx, "repo", "tag", multiarchImageMeta) + So(err, ShouldBeNil) + + // GetFullImageMeta should return an error due to corrupted manifest data + // The error from getAllContainedMeta should propagate + fullImageMeta, err := boltdbWrapper.GetFullImageMeta(ctx, "repo", "tag") So(err, ShouldNotBeNil) + // Should still return a FullImageMeta object (even with error) + So(fullImageMeta, ShouldNotBeNil) }) }) @@ -443,6 +486,54 @@ func TestWrapperErrors(t *testing.T) { _, err = boltdbWrapper.FilterTags(ctx, mTypes.AcceptAllRepoTag, mTypes.AcceptAllImageMeta) So(err, ShouldBeNil) }) + + Convey("getAllContainedMeta error for index is joined and processing continues", func() { + // Create a multiarch image with multiple manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + multiarchImageMeta := multiarchImage.AsImageMeta() + err := boltdbWrapper.SetImageMeta(multiarchImageMeta.Digest, multiarchImageMeta) + So(err, ShouldBeNil) + + // Store the first manifest normally + firstManifest := multiarchImage.Images[0] + firstManifestMeta := firstManifest.AsImageMeta() + err = boltdbWrapper.SetImageMeta(firstManifestMeta.Digest, firstManifestMeta) + So(err, ShouldBeNil) + + // Store the second manifest normally first, then corrupt it + secondManifest := multiarchImage.Images[1] + secondManifestMeta := secondManifest.AsImageMeta() + err = boltdbWrapper.SetImageMeta(secondManifestMeta.Digest, secondManifestMeta) + So(err, ShouldBeNil) + + secondManifestDigest := secondManifest.ManifestDescriptor.Digest + + // Corrupt the data for the second manifest by storing invalid protobuf data + // This will cause getProtoImageMeta to return an unmarshaling error + // which is not ErrImageMetaNotFound, so it will propagate through getAllContainedMeta + corruptedData := []byte("invalid protobuf data") + + // Access BoltDB directly to corrupt the data + err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { + imageBuck := tx.Bucket([]byte(boltdb.ImageMetaBuck)) + if imageBuck == nil { + return errImageMetaBucketNotFound + } + // Store corrupted protobuf data + return imageBuck.Put([]byte(secondManifestDigest.String()), corruptedData) + }) + So(err, ShouldBeNil) + + err = boltdbWrapper.SetRepoReference(ctx, "repo", "tag", multiarchImageMeta) + So(err, ShouldBeNil) + + // FilterTags should return an error due to corrupted manifest data + // The error from getAllContainedMeta should be joined with viewError + images, err := boltdbWrapper.FilterTags(ctx, mTypes.AcceptAllRepoTag, mTypes.AcceptAllImageMeta) + So(err, ShouldNotBeNil) + // Should still return some images (the first valid manifest might be processed) + So(images, ShouldNotBeNil) + }) }) Convey("SearchRepos", func() { @@ -470,6 +561,53 @@ func TestWrapperErrors(t *testing.T) { }) }) + Convey("GetImageMeta", func() { + Convey("image is index, getAllContainedMeta error returns error", func() { + // Create a multiarch image with multiple manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + multiarchImageMeta := multiarchImage.AsImageMeta() + err := boltdbWrapper.SetImageMeta(multiarchImageMeta.Digest, multiarchImageMeta) + So(err, ShouldBeNil) + + // Store the first manifest normally + firstManifest := multiarchImage.Images[0] + firstManifestMeta := firstManifest.AsImageMeta() + err = boltdbWrapper.SetImageMeta(firstManifestMeta.Digest, firstManifestMeta) + So(err, ShouldBeNil) + + // Store the second manifest normally first, then corrupt it + secondManifest := multiarchImage.Images[1] + secondManifestMeta := secondManifest.AsImageMeta() + err = boltdbWrapper.SetImageMeta(secondManifestMeta.Digest, secondManifestMeta) + So(err, ShouldBeNil) + + secondManifestDigest := secondManifest.ManifestDescriptor.Digest + + // Corrupt the data for the second manifest by storing invalid protobuf data + // This will cause getProtoImageMeta to return an unmarshaling error + // which is not ErrImageMetaNotFound, so it will propagate through getAllContainedMeta + corruptedData := []byte("invalid protobuf data") + + // Access BoltDB directly to corrupt the data + err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { + imageBuck := tx.Bucket([]byte(boltdb.ImageMetaBuck)) + if imageBuck == nil { + return errImageMetaBucketNotFound + } + // Store corrupted protobuf data + return imageBuck.Put([]byte(secondManifestDigest.String()), corruptedData) + }) + So(err, ShouldBeNil) + + // GetImageMeta should return an error due to corrupted manifest data + // The error from getAllContainedMeta should propagate + imageMeta, err := boltdbWrapper.GetImageMeta(multiarchImageMeta.Digest) + So(err, ShouldNotBeNil) + // Should still return an ImageMeta object (even with error) + So(imageMeta, ShouldNotBeNil) + }) + }) + Convey("SetRepoReference", func() { Convey("getProtoRepoMeta errors", func() { err := setRepoMeta("repo", badProtoBlob, boltdbWrapper.DB) diff --git a/pkg/meta/dynamodb/dynamodb.go b/pkg/meta/dynamodb/dynamodb.go index 428c67dd..fbc73736 100644 --- a/pkg/meta/dynamodb/dynamodb.go +++ b/pkg/meta/dynamodb/dynamodb.go @@ -1358,6 +1358,27 @@ func (dwr *DynamoDB) FilterImageMeta(ctx context.Context, digests []string, return nil, err } + // Check if all requested digests were found + // FilterImageMeta should return an error if any digest is missing (unlike getAllContainedMeta) + if len(imageMetaAttributes) != len(digests) { + // Build a map of found digests to identify which ones are missing + foundDigests := make(map[string]bool) + + for _, attributes := range imageMetaAttributes { + var digest string + if err := attributevalue.Unmarshal(attributes["TableKey"], &digest); err == nil { + foundDigests[digest] = true + } + } + + // Find the first missing digest + for _, digest := range digests { + if !foundDigests[digest] { + return nil, fmt.Errorf("%w for digest %s", zerr.ErrImageMetaNotFound, digest) + } + } + } + results := map[string]mTypes.ImageMeta{} for _, attributes := range imageMetaAttributes { @@ -2021,10 +2042,7 @@ func (dwr *DynamoDB) fetchImageMetaAttributesByDigest(ctx context.Context, diges return nil, err } - if len(resp.Responses[dwr.ImageMetaTablename]) != size { - return nil, zerr.ErrImageMetaNotFound - } - + // Missing manifests are allowed - append whatever was found batchedResp = append(batchedResp, resp.Responses[dwr.ImageMetaTablename]...) start = end } @@ -2045,13 +2063,12 @@ func (dwr *DynamoDB) fetchImageMetaAttributesByDigest(ctx context.Context, diges respMap[digest] = item } + // Only include digests that were actually found (missing ones are skipped gracefully) for _, digest := range digests { imageMeta, ok := respMap[digest] - if !ok { - return nil, fmt.Errorf("%w for digest %s", zerr.ErrImageMetaNotFound, digest) + if ok { + orderedResp = append(orderedResp, imageMeta) } - - orderedResp = append(orderedResp, imageMeta) } return orderedResp, nil @@ -2228,30 +2245,28 @@ func (dwr *DynamoDB) createVersionTable() error { return err } - if err == nil { - mdAttributeValue, err := attributevalue.Marshal(version.CurrentVersion) - if err != nil { - return err - } + mdAttributeValue, err := attributevalue.Marshal(version.CurrentVersion) + if err != nil { + return err + } - _, err = dwr.Client.UpdateItem(context.TODO(), &dynamodb.UpdateItemInput{ - ExpressionAttributeNames: map[string]string{ - "#V": "Version", + _, err = dwr.Client.UpdateItem(context.TODO(), &dynamodb.UpdateItemInput{ + ExpressionAttributeNames: map[string]string{ + "#V": "Version", + }, + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":Version": mdAttributeValue, + }, + Key: map[string]types.AttributeValue{ + "TableKey": &types.AttributeValueMemberS{ + Value: version.DBVersionKey, }, - ExpressionAttributeValues: map[string]types.AttributeValue{ - ":Version": mdAttributeValue, - }, - Key: map[string]types.AttributeValue{ - "TableKey": &types.AttributeValueMemberS{ - Value: version.DBVersionKey, - }, - }, - TableName: aws.String(dwr.VersionTablename), - UpdateExpression: aws.String("SET #V = :Version"), - }) - if err != nil { - return err - } + }, + TableName: aws.String(dwr.VersionTablename), + UpdateExpression: aws.String("SET #V = :Version"), + }) + if err != nil { + return err } return nil diff --git a/pkg/meta/dynamodb/dynamodb_test.go b/pkg/meta/dynamodb/dynamodb_test.go index 41bcf040..f8b5cf9a 100644 --- a/pkg/meta/dynamodb/dynamodb_test.go +++ b/pkg/meta/dynamodb/dynamodb_test.go @@ -383,12 +383,15 @@ func TestWrapperErrors(t *testing.T) { _, err := dynamoWrapper.GetImageMeta(testDigest) So(err, ShouldNotBeNil) }) - Convey("image index, get manifest meta fails", func() { + Convey("image index, missing manifests are skipped gracefully", func() { err := dynamoWrapper.SetRepoReference(ctx, "repo", "tag", multiarchImageMeta) So(err, ShouldBeNil) - _, err = dynamoWrapper.GetImageMeta(multiarchImageMeta.Digest) //nolint: contextcheck - So(err, ShouldNotBeNil) + // Missing manifests are skipped gracefully, so GetImageMeta succeeds + // but returns an index with no manifests + imageMeta, err := dynamoWrapper.GetImageMeta(multiarchImageMeta.Digest) //nolint: contextcheck + So(err, ShouldBeNil) + So(len(imageMeta.Manifests), ShouldEqual, 0) }) }) Convey("GetFullImageMeta", func() { @@ -429,7 +432,7 @@ func TestWrapperErrors(t *testing.T) { So(err, ShouldNotBeNil) }) - Convey("image is index, fail to get manifests", func() { + Convey("image is index, missing manifests are skipped gracefully", func() { err := dynamoWrapper.SetImageMeta(multiarchImageMeta.Digest, multiarchImageMeta) //nolint: contextcheck So(err, ShouldBeNil) @@ -444,8 +447,11 @@ func TestWrapperErrors(t *testing.T) { }) So(err, ShouldBeNil) - _, err = dynamoWrapper.GetFullImageMeta(ctx, "repo", "tag") - So(err, ShouldNotBeNil) + // Missing manifests are skipped gracefully, so GetFullImageMeta succeeds + // but returns an index with no manifests + fullImageMeta, err := dynamoWrapper.GetFullImageMeta(ctx, "repo", "tag") + So(err, ShouldBeNil) + So(len(fullImageMeta.Manifests), ShouldEqual, 0) }) }) diff --git a/pkg/meta/meta_test.go b/pkg/meta/meta_test.go index f1b49c62..92e03b94 100644 --- a/pkg/meta/meta_test.go +++ b/pkg/meta/meta_test.go @@ -572,6 +572,75 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func So(fullImageMeta.Digest.String(), ShouldResemble, multi.DigestStr()) }) + Convey("GetFullImageMeta with nested index missing manifests", func() { + // Create a nested index structure: + // - Top-level index contains a nested index and a manifest + // - The nested index references manifests that are missing from MetaDB + // - The manifest in the top-level index exists + // Create a valid manifest + validImage := CreateDefaultImage() + validImageMeta := validImage.AsImageMeta() + err := metaDB.SetImageMeta(validImageMeta.Digest, validImageMeta) + So(err, ShouldBeNil) + + // Create a nested index that references missing manifests + nestedIndex := CreateMultiarchWith().Images([]Image{CreateRandomImage()}).Build() + nestedIndexMeta := nestedIndex.AsImageMeta() + // Store the nested index metadata (but not its manifests - they're missing) + err = metaDB.SetImageMeta(nestedIndexMeta.Digest, nestedIndexMeta) + So(err, ShouldBeNil) + + // Create a top-level index that contains: + // 1. The nested index (with missing manifests) + // 2. The valid manifest + topLevelIndexContent := ispec.Index{ + Versioned: specs.Versioned{SchemaVersion: 2}, + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: validImageMeta.Digest, + Size: validImageMeta.Size, + }, + { + MediaType: ispec.MediaTypeImageIndex, + Digest: nestedIndexMeta.Digest, + Size: nestedIndexMeta.Size, + }, + }, + } + + // Create and store the top-level index metadata + topLevelIndexBlob, err := json.Marshal(topLevelIndexContent) + So(err, ShouldBeNil) + + // Calculate digest from the index content + topLevelIndexDigest := godigest.FromBytes(topLevelIndexBlob) + + topLevelIndexMeta := mTypes.ImageMeta{ + MediaType: ispec.MediaTypeImageIndex, + Digest: topLevelIndexDigest, + Size: int64(len(topLevelIndexBlob)), + Index: &topLevelIndexContent, + Manifests: []mTypes.ManifestMeta{}, + } + + err = metaDB.SetImageMeta(topLevelIndexMeta.Digest, topLevelIndexMeta) + So(err, ShouldBeNil) + + err = metaDB.SetRepoReference(ctx, "repo", "tag", topLevelIndexMeta) + So(err, ShouldBeNil) + + // GetFullImageMeta should succeed, skipping the nested index with missing manifests + // but including the valid manifest from the top-level index + fullImageMeta, err := metaDB.GetFullImageMeta(ctx, "repo", "tag") + So(err, ShouldBeNil) + // Should have the valid manifest from the top-level index + // The nested index with missing manifests should be skipped + So(len(fullImageMeta.Manifests), ShouldEqual, 1) + So(fullImageMeta.Manifests[0].Digest, ShouldEqual, validImageMeta.Digest) + }) + Convey("Set/Get RepoMeta", func() { err := metaDB.SetRepoMeta("repo", mTypes.RepoMeta{ Name: "repo", diff --git a/pkg/meta/parse.go b/pkg/meta/parse.go index 2f469e0d..50bc0184 100644 --- a/pkg/meta/parse.go +++ b/pkg/meta/parse.go @@ -6,6 +6,7 @@ import ( "errors" "time" + "github.com/distribution/distribution/v3/registry/storage/driver" godigest "github.com/opencontainers/go-digest" ispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -143,8 +144,17 @@ func ParseRepo(repo string, metaDB mTypes.MetaDB, storeController stypes.StoreCo continue } - manifestBlob, _, _, err := imageStore.GetImageManifest(repo, manifest.Digest.String()) + manifestBlob, err := imageStore.GetBlobContent(repo, manifest.Digest) if err != nil { + // Handle missing blobs gracefully - log warning and continue with other manifests + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + log.Warn().Err(err).Str("repository", repo).Str("digest", manifest.Digest.String()). + Msg("skipping missing manifest blob, continuing repo parse") + + continue + } + log.Error().Err(err).Str("repository", repo).Str("digest", manifest.Digest.String()). Msg("failed to get blob for image") diff --git a/pkg/meta/parse_test.go b/pkg/meta/parse_test.go index ba5c792a..8a47baa2 100644 --- a/pkg/meta/parse_test.go +++ b/pkg/meta/parse_test.go @@ -16,6 +16,7 @@ import ( ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" + zerr "zotregistry.dev/zot/v2/errors" rediscfg "zotregistry.dev/zot/v2/pkg/api/config/redis" zcommon "zotregistry.dev/zot/v2/pkg/common" "zotregistry.dev/zot/v2/pkg/extensions/monitoring" @@ -165,13 +166,14 @@ func TestParseStorageErrors(t *testing.T) { So(err, ShouldBeNil) }) - Convey("imageStore.GetImageManifest errors", func() { + Convey("imageStore.GetBlobContent non-missing error", func() { + manifestDigest := godigest.FromString("digest") imageStore.GetIndexContentFn = func(repo string) ([]byte, error) { return getIndexBlob(ispec.Index{ Manifests: []ispec.Descriptor{ { MediaType: ispec.MediaTypeImageManifest, - Digest: godigest.FromString("digest"), + Digest: manifestDigest, Annotations: map[string]string{ ispec.AnnotationRefName: "tag", }, @@ -179,11 +181,75 @@ func TestParseStorageErrors(t *testing.T) { }, }), nil } - imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { - return nil, "", "", ErrTestError + imageStore.GetBlobContentFn = func(repo string, digest godigest.Digest) ([]byte, error) { + // Return a non-missing error (not ErrBlobNotFound or PathNotFoundError) + return nil, ErrTestError } err := meta.ParseRepo("repo", metaDB, storeController, log) So(err, ShouldNotBeNil) + So(err, ShouldEqual, ErrTestError) + }) + + Convey("imageStore.GetImageManifest missing blob - graceful handling", func() { + digest1 := godigest.FromString("digest1") + digest2 := godigest.FromString("digest2") + imageStore.GetIndexContentFn = func(repo string) ([]byte, error) { + return getIndexBlob(ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: digest1, + Annotations: map[string]string{ + ispec.AnnotationRefName: "tag1", + }, + }, + { + MediaType: ispec.MediaTypeImageManifest, + Digest: digest2, + Annotations: map[string]string{ + ispec.AnnotationRefName: "tag2", + }, + }, + }, + }), nil + } + callCount := 0 + setRepoRefCount := 0 + // Create a valid image for the second manifest + validImage := CreateRandomImage() + manifestBlob, _ := json.Marshal(validImage.Manifest) + configBlob, _ := json.Marshal(validImage.Config) + imageStore.GetBlobContentFn = func(repo string, digest godigest.Digest) ([]byte, error) { + callCount++ + // First manifest is missing, second one succeeds + if digest == digest1 { + return nil, zerr.ErrBlobNotFound + } + + if digest == digest2 { + // Return valid manifest for second one + return manifestBlob, nil + } + // Return config blob when requested + if digest == validImage.ConfigDescriptor.Digest { + return configBlob, nil + } + + return nil, zerr.ErrBlobNotFound + } + metaDB.SetRepoReferenceFn = func(ctx context.Context, repo, reference string, imageMeta mTypes.ImageMeta) error { + setRepoRefCount++ + // Verify it's only called for tag2 (the second manifest) + So(reference, ShouldEqual, "tag2") + + return nil + } + err := meta.ParseRepo("repo", metaDB, storeController, log) + So(err, ShouldBeNil) + // Should have called GetBlobContent for both manifests (and config) + So(callCount, ShouldEqual, 3) + // Should have called SetRepoReference only once for the second manifest (first was skipped) + So(setRepoRefCount, ShouldEqual, 1) }) Convey("manifestMetaIsPresent true", func() { diff --git a/pkg/meta/redis/redis.go b/pkg/meta/redis/redis.go index a3223b3a..b64639b6 100644 --- a/pkg/meta/redis/redis.go +++ b/pkg/meta/redis/redis.go @@ -2197,6 +2197,11 @@ func (rc *RedisDB) getAllContainedMeta(ctx context.Context, imageIndexData *prot imageManifestData, err := rc.getProtoImageMeta(ctx, manifest.Digest) if err != nil { + // Skip manifests that don't have MetaDB entries (missing from storage) + if errors.Is(err, zerr.ErrImageMetaNotFound) { + continue + } + return imageMetaList, manifestDataList, err } @@ -2208,6 +2213,8 @@ func (rc *RedisDB) getAllContainedMeta(ctx context.Context, imageIndexData *prot compat.IsCompatibleManifestListMediaType(imageManifestData.MediaType) { partialImageDataList, partialManifestDataList, err := rc.getAllContainedMeta(ctx, imageManifestData) if err != nil { + // getAllContainedMeta skips missing items internally, so any error returned + // is a real error that should be propagated return imageMetaList, manifestDataList, err } diff --git a/pkg/meta/redis/redis_test.go b/pkg/meta/redis/redis_test.go index c2996c7e..bf1272be 100644 --- a/pkg/meta/redis/redis_test.go +++ b/pkg/meta/redis/redis_test.go @@ -857,23 +857,56 @@ func TestWrapperErrors(t *testing.T) { So(err, ShouldNotBeNil) }) - Convey("image is index, fail to get manifests", func() { + Convey("image is index, missing manifests are skipped gracefully", func() { + err := metaDB.SetRepoReference(ctx, "repo", "tag", multiarchImageMeta) + So(err, ShouldBeNil) + + // Missing manifests are skipped gracefully, so GetFullImageMeta succeeds + // but returns an index with no manifests + fullImageMeta, err := metaDB.GetFullImageMeta(ctx, "repo", "tag") + So(err, ShouldBeNil) + So(len(fullImageMeta.Manifests), ShouldEqual, 0) + }) + + Convey("image is index, corrupted manifest data returns error", func() { + // Create a multiarch image with multiple manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + multiarchImageMeta := multiarchImage.AsImageMeta() err := metaDB.SetImageMeta(multiarchImageMeta.Digest, multiarchImageMeta) So(err, ShouldBeNil) - err = metaDB.SetRepoMeta("repo", mTypes.RepoMeta{ - Name: "repo", - Tags: map[mTypes.Tag]mTypes.Descriptor{ - "tag": { - MediaType: ispec.MediaTypeImageIndex, - Digest: multiarchImageMeta.Digest.String(), - }, - }, - }) + // Store the first manifest normally + firstManifest := multiarchImage.Images[0] + firstManifestMeta := firstManifest.AsImageMeta() + err = metaDB.SetImageMeta(firstManifestMeta.Digest, firstManifestMeta) So(err, ShouldBeNil) - _, err = metaDB.GetFullImageMeta(ctx, "repo", "tag") + // Store the second manifest normally first, then corrupt it + secondManifest := multiarchImage.Images[1] + secondManifestMeta := secondManifest.AsImageMeta() + err = metaDB.SetImageMeta(secondManifestMeta.Digest, secondManifestMeta) + So(err, ShouldBeNil) + + secondManifestDigest := secondManifest.ManifestDescriptor.Digest + + // Corrupt the data for the second manifest by storing invalid protobuf data + // This will cause getProtoImageMeta to return an unmarshaling error + // which is not ErrImageMetaNotFound, so it will propagate through getAllContainedMeta + corruptedData := []byte("invalid protobuf data") + + // Access Redis directly to corrupt the data using the helper function pattern + err = setImageMeta(secondManifestDigest, corruptedData, client) + So(err, ShouldBeNil) + + err = metaDB.SetRepoReference(ctx, "repo", "tag", multiarchImageMeta) + So(err, ShouldBeNil) + + // GetFullImageMeta should return an error due to corrupted manifest data + // The error from getAllContainedMeta should propagate + fullImageMeta, err := metaDB.GetFullImageMeta(ctx, "repo", "tag") So(err, ShouldNotBeNil) + // Should still return a FullImageMeta object (even with error) + So(fullImageMeta, ShouldNotBeNil) }) }) diff --git a/pkg/storage/common/common.go b/pkg/storage/common/common.go index 3487e6dc..571da481 100644 --- a/pkg/storage/common/common.go +++ b/pkg/storage/common/common.go @@ -464,6 +464,15 @@ func PruneImageManifestsFromIndex(imgStore storageTypes.ImageStore, repo string, for _, otherIndex := range otherImgIndexes { oindex, err := GetImageIndex(imgStore, repo, otherIndex.Digest, log) if err != nil { + // Handle missing blobs gracefully - log warning and continue with other indexes + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + log.Warn().Err(err).Str("repository", repo).Str("digest", otherIndex.Digest.String()). + Msg("skipping missing image index blob, continuing with other indexes") + + continue + } + return nil, err } @@ -520,6 +529,15 @@ func isBlobReferencedInImageManifest(imgStore storageTypes.ImageStore, repo stri manifestContent, err := GetImageManifest(imgStore, repo, mdigest, log) if err != nil { + // Handle missing blobs gracefully - treat as not referenced and continue + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + log.Warn().Err(err).Str("repo", repo).Str("digest", mdigest.String()).Str("component", "gc"). + Msg("skipping missing manifest blob, treating as not referenced") + + return false, nil + } + log.Error().Err(err).Str("repo", repo).Str("digest", mdigest.String()).Str("component", "gc"). Msg("failed to read manifest image") @@ -554,6 +572,15 @@ func IsBlobReferencedInImageIndex(imgStore storageTypes.ImageStore, repo string, indexImage, err := GetImageIndex(imgStore, repo, desc.Digest, log) if err != nil { + // Handle missing blobs gracefully - treat as not referenced and continue + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + log.Warn().Err(err).Str("repository", repo).Str("digest", desc.Digest.String()). + Msg("skipping missing image index blob, treating as not referenced") + + continue + } + log.Error().Err(err).Str("repository", repo).Str("digest", desc.Digest.String()). Msg("failed to read multiarch(index) image") @@ -787,6 +814,15 @@ func GetBlobDescriptorFromIndex(imgStore storageTypes.ImageStore, index ispec.In case ispec.MediaTypeImageIndex: indexImage, err := GetImageIndex(imgStore, repo, desc.Digest, log) if err != nil { + // Handle missing blobs gracefully - skip this index and continue searching + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + log.Warn().Err(err).Str("repository", repo).Str("digest", desc.Digest.String()). + Msg("skipping missing image index blob, continuing search for blob descriptor") + + continue + } + return ispec.Descriptor{}, err } diff --git a/pkg/storage/common/common_test.go b/pkg/storage/common/common_test.go index 17e8c1da..a3c09764 100644 --- a/pkg/storage/common/common_test.go +++ b/pkg/storage/common/common_test.go @@ -610,3 +610,163 @@ func TestDedupeGeneratorErrors(t *testing.T) { So(task, ShouldBeNil) }) } + +func TestPruneImageManifestsFromIndexMissingIndex(t *testing.T) { + log := log.NewTestLogger() + + Convey("Missing nested index blob is skipped gracefully", t, func(c C) { + // Create a main index + mainIndexDigest := godigest.FromString("main-index") + manifest1Digest := godigest.FromString("manifest1") + mainIndex := ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: manifest1Digest, + }, + }, + } + mainIndexBlob, err := json.Marshal(mainIndex) + So(err, ShouldBeNil) + + // Create other indexes list with one missing index + // The missing index would have referenced manifest1, but since it's missing, + // manifest1 should still be pruned (removed) if it has no tag + otherImgIndexes := []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageIndex, + Digest: godigest.FromString("missing-index"), + Size: 100, + }, + } + + imgStore := &mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + if digest == mainIndexDigest { + return mainIndexBlob, nil + } + // Return ErrBlobNotFound for the missing nested index + return nil, zerr.ErrBlobNotFound + }, + } + + // PruneImageManifestsFromIndex should skip the missing nested index and continue + // outIndex contains a manifest without a tag, so it should be pruned + outIndex := ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: manifest1Digest, + // No tag annotation, so it will be pruned + }, + }, + } + + prunedManifests, err := common.PruneImageManifestsFromIndex( + imgStore, "repo", mainIndexDigest, outIndex, otherImgIndexes, log) + So(err, ShouldBeNil) + // The manifest should be pruned (removed) since it has no tag and the missing index + // couldn't be checked to see if it references this manifest + // The important thing is that the function succeeded (didn't error) despite the missing index + So(len(prunedManifests), ShouldEqual, 0) + }) +} + +func TestIsBlobReferencedInImageManifestMissingManifest(t *testing.T) { + log := log.NewTestLogger() + + Convey("Missing manifest blob is treated as not referenced", t, func(c C) { + blobDigest := godigest.FromString("blob-digest") + missingManifestDigest := godigest.FromString("missing-manifest") + + imgStore := &mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + // Return ErrBlobNotFound for the missing manifest + return nil, zerr.ErrBlobNotFound + }, + } + + // Create an index with a manifest descriptor pointing to a missing manifest + // IsBlobReferencedInImageIndex will call isBlobReferencedInImageManifest internally + index := ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: missingManifestDigest, + Size: 100, + }, + }, + } + + // isBlobReferencedInImageManifest should treat missing manifest as not referenced + referenced, err := common.IsBlobReferencedInImageIndex(imgStore, "repo", blobDigest, index, log) + So(err, ShouldBeNil) + So(referenced, ShouldBeFalse) + }) +} + +func TestIsBlobReferencedInImageIndexNonMissingError(t *testing.T) { + log := log.NewTestLogger() + + Convey("Non-missing error when reading nested index returns error", t, func(c C) { + blobDigest := godigest.FromString("blob-digest") + nestedIndexDigest := godigest.FromString("nested-index") + + // Create an index with a nested index descriptor + index := ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageIndex, + Digest: nestedIndexDigest, + Size: 100, + }, + }, + } + + imgStore := &mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + // Return a non-missing error (not ErrBlobNotFound or PathNotFoundError) + return nil, ErrTestError + }, + } + + // IsBlobReferencedInImageIndex should return the error (not skip it) + referenced, err := common.IsBlobReferencedInImageIndex(imgStore, "repo", blobDigest, index, log) + So(err, ShouldNotBeNil) + So(err, ShouldEqual, ErrTestError) + So(referenced, ShouldBeFalse) + }) +} + +func TestGetBlobDescriptorFromIndexMissingNestedIndex(t *testing.T) { + log := log.NewTestLogger() + + Convey("Missing nested index blob is skipped gracefully", t, func(c C) { + blobDigest := godigest.FromString("blob-digest") + missingIndexDigest := godigest.FromString("missing-nested-index") + + // Create an index that contains a nested index (which will be missing) + topLevelIndex := ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageIndex, + Digest: missingIndexDigest, + Size: 100, + }, + }, + } + + imgStore := &mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + // Return ErrBlobNotFound for the missing nested index + return nil, zerr.ErrBlobNotFound + }, + } + + // GetBlobDescriptorFromIndex should skip the missing nested index and continue + // Since the blob is not found, it should return ErrBlobNotFound + _, err := common.GetBlobDescriptorFromIndex(imgStore, topLevelIndex, "repo", blobDigest, log) + So(err, ShouldNotBeNil) + So(err, ShouldEqual, zerr.ErrBlobNotFound) + }) +} diff --git a/pkg/storage/gc/gc.go b/pkg/storage/gc/gc.go index c658365e..ddcc1068 100644 --- a/pkg/storage/gc/gc.go +++ b/pkg/storage/gc/gc.go @@ -236,6 +236,15 @@ func (gc GarbageCollect) removeIndexReferrers(repo string, rootIndex *ispec.Inde if (desc.MediaType == ispec.MediaTypeImageIndex) || compat.IsCompatibleManifestListMediaType(desc.MediaType) { indexImage, err := common.GetImageIndex(gc.imgStore, repo, desc.Digest, gc.log) if err != nil { + // Handle missing blobs (not found) gracefully + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + gc.log.Warn().Err(err).Str("module", "gc").Str("repository", repo).Str("digest", desc.Digest.String()). + Msg("skipping missing image index blob, continuing GC") + + continue + } + gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo).Str("digest", desc.Digest.String()). Msg("failed to read multiarch(index) image") @@ -265,6 +274,15 @@ func (gc GarbageCollect) removeIndexReferrers(repo string, rootIndex *ispec.Inde } else if (desc.MediaType == ispec.MediaTypeImageManifest) || compat.IsCompatibleManifestMediaType(desc.MediaType) { image, err := common.GetImageManifest(gc.imgStore, repo, desc.Digest, gc.log) if err != nil { + // Handle missing blobs (not found) gracefully + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + gc.log.Warn().Err(err).Str("module", "gc").Str("repo", repo).Str("digest", desc.Digest.String()). + Msg("skipping missing image manifest blob, continuing GC") + + continue + } + gc.log.Error().Err(err).Str("module", "gc").Str("repo", repo).Str("digest", desc.Digest.String()). Msg("failed to read manifest image") @@ -536,6 +554,15 @@ func (gc GarbageCollect) identifyManifestsReferencedInIndex(index ispec.Index, r if (desc.MediaType == ispec.MediaTypeImageIndex) || compat.IsCompatibleManifestListMediaType(desc.MediaType) { indexImage, err := common.GetImageIndex(gc.imgStore, repo, desc.Digest, gc.log) if err != nil { + // Handle missing blobs (not found) gracefully + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + gc.log.Warn().Err(err).Str("module", "gc").Str("repository", repo). + Str("digest", desc.Digest.String()).Msg("skipping missing image index blob, continuing GC") + + continue + } + gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo). Str("digest", desc.Digest.String()).Msg("failed to read multiarch(index) image") @@ -556,6 +583,15 @@ func (gc GarbageCollect) identifyManifestsReferencedInIndex(index ispec.Index, r } else if (desc.MediaType == ispec.MediaTypeImageManifest) || compat.IsCompatibleManifestMediaType(desc.MediaType) { image, err := common.GetImageManifest(gc.imgStore, repo, desc.Digest, gc.log) if err != nil { + // Handle missing blobs (not found) gracefully + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + gc.log.Warn().Err(err).Str("module", "gc").Str("repo", repo). + Str("digest", desc.Digest.String()).Msg("skipping missing image manifest blob, continuing GC") + + continue + } + gc.log.Error().Err(err).Str("module", "gc").Str("repo", repo). Str("digest", desc.Digest.String()).Msg("failed to read manifest image") @@ -718,6 +754,15 @@ func (gc GarbageCollect) addImageIndexBlobsToReferences(repo string, mdigest god ) error { index, err := common.GetImageIndex(gc.imgStore, repo, mdigest, gc.log) if err != nil { + // Handle missing blobs (not found) gracefully + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + gc.log.Warn().Err(err).Str("module", "gc").Str("repository", repo).Str("digest", mdigest.String()). + Msg("skipping missing image index blob, continuing GC") + + return nil + } + gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo).Str("digest", mdigest.String()). Msg("failed to read manifest image") @@ -743,6 +788,15 @@ func (gc GarbageCollect) addImageManifestBlobsToReferences(repo string, mdigest ) error { manifestContent, err := common.GetImageManifest(gc.imgStore, repo, mdigest, gc.log) if err != nil { + // Handle missing blobs (not found) gracefully + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + gc.log.Warn().Err(err).Str("module", "gc").Str("repository", repo). + Str("digest", mdigest.String()).Msg("skipping missing image manifest blob, continuing GC") + + return nil + } + gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo). Str("digest", mdigest.String()).Msg("failed to read manifest image") diff --git a/pkg/storage/gc/gc_internal_test.go b/pkg/storage/gc/gc_internal_test.go index 64f0952e..30111d4b 100644 --- a/pkg/storage/gc/gc_internal_test.go +++ b/pkg/storage/gc/gc_internal_test.go @@ -14,6 +14,7 @@ import ( ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" + zerr "zotregistry.dev/zot/v2/errors" "zotregistry.dev/zot/v2/pkg/api/config" zcommon "zotregistry.dev/zot/v2/pkg/common" "zotregistry.dev/zot/v2/pkg/extensions/monitoring" @@ -63,7 +64,8 @@ func TestGarbageCollectManifestErrors(t *testing.T) { }, }, audit, log) - Convey("trigger repo not found in addImageIndexBlobsToReferences()", func() { + Convey("trigger missing blob in addImageIndexBlobsToReferences()", func() { + // GC should continue when blobs are missing (not found), not return an error err := gc.addIndexBlobsToReferences(repoName, ispec.Index{ Manifests: []ispec.Descriptor{ { @@ -72,10 +74,11 @@ func TestGarbageCollectManifestErrors(t *testing.T) { }, }, }, map[string]bool{}) - So(err, ShouldNotBeNil) + So(err, ShouldBeNil) }) - Convey("trigger repo not found in addImageManifestBlobsToReferences()", func() { + Convey("trigger missing blob in addImageManifestBlobsToReferences()", func() { + // GC should continue when blobs are missing (not found), not return an error err := gc.addIndexBlobsToReferences(repoName, ispec.Index{ Manifests: []ispec.Descriptor{ { @@ -84,7 +87,7 @@ func TestGarbageCollectManifestErrors(t *testing.T) { }, }, }, map[string]bool{}) - So(err, ShouldNotBeNil) + So(err, ShouldBeNil) }) content := []byte("this is a blob") @@ -137,8 +140,12 @@ func TestGarbageCollectManifestErrors(t *testing.T) { So(err, ShouldBeNil) }() + // Note: Permission denied from Stat() is converted to ErrBlobNotFound in originalBlobInfo, + // so we can't distinguish it from missing blobs. GC treats missing blobs gracefully, + // so permission denied from Stat() will also be treated as missing (return nil). + // Permission denied from ReadFile() will still return an error. err = gc.addIndexBlobsToReferences(repoName, index, map[string]bool{}) - So(err, ShouldNotBeNil) + So(err, ShouldBeNil) }) Convey("trigger GetImageManifest error in AddIndexBlobsToReferences", func() { @@ -706,5 +713,74 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }) So(err, ShouldNotBeNil) }) + + Convey("Missing nested index blob in removeIndexReferrers is skipped gracefully", func() { + // Create a top-level index that contains a nested index + // The nested index blob will be missing + topLevelIndex := ispec.Index{ + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageIndex, + Digest: godigest.FromString("missing-nested-index"), + Size: 100, + }, + }, + } + + imgStore := mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + // Return ErrBlobNotFound for the missing nested index + return nil, zerr.ErrBlobNotFound + }, + } + + gcOptions.ImageRetention = config.ImageRetention{ + Policies: []config.RetentionPolicy{ + { + Repositories: []string{"**"}, + DeleteReferrers: true, + }, + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + + // removeIndexReferrers should skip the missing nested index and continue + gced, err := gc.removeIndexReferrers(repoName, &topLevelIndex, topLevelIndex) + So(err, ShouldBeNil) + So(gced, ShouldBeFalse) + }) + + Convey("Missing nested index blob in identifyManifestsReferencedInIndex is skipped gracefully", func() { + // Create a top-level index that contains a nested index + // The nested index blob will be missing + topLevelIndex := ispec.Index{ + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageIndex, + Digest: godigest.FromString("missing-nested-index"), + Size: 100, + }, + }, + } + + imgStore := mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + // Return ErrBlobNotFound for the missing nested index + return nil, zerr.ErrBlobNotFound + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + + // identifyManifestsReferencedInIndex should skip the missing nested index and continue + referenced := make(map[godigest.Digest]bool) + err := gc.identifyManifestsReferencedInIndex(topLevelIndex, repoName, referenced) + So(err, ShouldBeNil) + // No manifests should be marked as referenced since the nested index is missing + So(len(referenced), ShouldEqual, 0) + }) }) } diff --git a/pkg/storage/local/local_test.go b/pkg/storage/local/local_test.go index bd31a426..4147f8a4 100644 --- a/pkg/storage/local/local_test.go +++ b/pkg/storage/local/local_test.go @@ -1969,7 +1969,7 @@ func TestGarbageCollectForImageStore(t *testing.T) { ctx := context.Background() - Convey("Garbage collect error for repo with config removed", func() { + Convey("Garbage collect continues when manifest blob is missing", func() { logFile, _ := os.CreateTemp("", "zot-log*.txt") defer os.Remove(logFile.Name()) // clean up @@ -2006,15 +2006,18 @@ func TestGarbageCollectForImageStore(t *testing.T) { panic(err) } + // GC should succeed (return nil) when manifest blobs are missing + // This is the expected behavior - GC continues gracefully when blobs are not found err = gc.CleanRepo(ctx, repoName) - So(err, ShouldNotBeNil) + So(err, ShouldBeNil) time.Sleep(500 * time.Millisecond) data, err := os.ReadFile(logFile.Name()) So(err, ShouldBeNil) - So(string(data), ShouldContainSubstring, - "failed to run GC for "+path.Join(imgStore.RootDir(), repoName)) + // Verify that GC logged a warning about skipping the missing manifest + So(string(data), ShouldContainSubstring, "skipping missing") + So(string(data), ShouldContainSubstring, manifestDigest.String()) }) Convey("Garbage collect error - not enough permissions to access index.json", func() { diff --git a/pkg/storage/scrub.go b/pkg/storage/scrub.go index 6b0a80de..fb4a6921 100644 --- a/pkg/storage/scrub.go +++ b/pkg/storage/scrub.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/distribution/distribution/v3/registry/storage/driver" "github.com/olekukonko/tablewriter" "github.com/olekukonko/tablewriter/tw" godigest "github.com/opencontainers/go-digest" @@ -31,6 +32,9 @@ const ( statusWidth = 8 affectedBlobWidth = 64 errorWidth = 8 + + statusAffected = "affected" + statusOK = "ok" ) type ScrubImageResult struct { @@ -116,8 +120,14 @@ func CheckRepo(ctx context.Context, imageName string, imgStore storageTypes.Imag layers, err := checkImage(manifest, imgStore, imageName, tag, scrubbedManifests) if err == nil && len(layers) > 0 { // CheckLayers doesn't use locks + // Only update result if current status is not "affected" to preserve subject/blob issues + currentRes, exists := scrubbedManifests[manifest.Digest] + imgRes := CheckLayers(imageName, tag, layers, imgStore) - scrubbedManifests[manifest.Digest] = imgRes + + if !exists || currentRes.Status != statusAffected { + scrubbedManifests[manifest.Digest] = imgRes + } } // ignore the manifest if it isn't found @@ -197,9 +207,24 @@ func scrubManifest( } // check all manifests + indexAffected := false + for _, man := range idx.Manifests { buf, err := imgStore.GetBlobContent(imageName, man.Digest) if err != nil { + // Handle missing blobs gracefully - mark as affected but continue processing + var pathNotFoundErr driver.PathNotFoundError + if errors.Is(err, zerr.ErrBlobNotFound) || errors.As(err, &pathNotFoundErr) { + imgRes := getResult(imageName, tag, man.Digest, err) + scrubbedManifests[man.Digest] = imgRes + scrubbedManifests[manifest.Digest] = imgRes + indexAffected = true + + // Continue checking other manifests instead of returning + continue + } + + // For other errors, mark as affected and return imgRes := getResult(imageName, tag, man.Digest, zerr.ErrBadBlobDigest) scrubbedManifests[man.Digest] = imgRes scrubbedManifests[manifest.Digest] = imgRes @@ -223,14 +248,28 @@ func scrubManifest( } } - // at this point, before starting to check the subject we can consider the index is ok - scrubbedManifests[manifest.Digest] = getResult(imageName, tag, "", nil) + // at this point, before starting to check the subject, mark index as ok only if not affected + // Also check if result was already set to avoid overwriting "affected" status + currentIndexRes, indexResExists := scrubbedManifests[manifest.Digest] + if !indexAffected && (!indexResExists || currentIndexRes.Status != statusAffected) { + scrubbedManifests[manifest.Digest] = getResult(imageName, tag, "", nil) + } // check subject if exists if idx.Subject != nil { buf, err := imgStore.GetBlobContent(imageName, idx.Subject.Digest) if err != nil { - imgRes := getResult(imageName, tag, idx.Subject.Digest, zerr.ErrBadBlobDigest) + // Handle missing blobs gracefully - mark as affected but continue processing + var pathNotFoundErr driver.PathNotFoundError + + resultErr := err + + if !errors.Is(err, zerr.ErrBlobNotFound) && !errors.As(err, &pathNotFoundErr) { + // For other errors, use generic error + resultErr = zerr.ErrBadBlobDigest + } + + imgRes := getResult(imageName, tag, idx.Subject.Digest, resultErr) scrubbedManifests[idx.Subject.Digest] = imgRes scrubbedManifests[manifest.Digest] = imgRes @@ -244,8 +283,12 @@ func scrubManifest( subjectRes := scrubbedManifests[idx.Subject.Digest] - scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, subjectRes.Status, - subjectRes.AffectedBlob, subjectRes.Error) + // Only update index status if it's not already "affected" to preserve missing manifest/blob issues + currentIndexRes, indexResExists := scrubbedManifests[manifest.Digest] + if !indexResExists || currentIndexRes.Status != statusAffected { + scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, subjectRes.Status, + subjectRes.AffectedBlob, subjectRes.Error) + } return layers, err } @@ -263,7 +306,17 @@ func scrubManifest( if err == nil && man.Subject != nil { buf, err := imgStore.GetBlobContent(imageName, man.Subject.Digest) if err != nil { - imgRes := getResult(imageName, tag, man.Subject.Digest, zerr.ErrBadBlobDigest) + // Handle missing blobs gracefully - mark as affected but continue processing + var pathNotFoundErr driver.PathNotFoundError + + resultErr := err + + if !errors.Is(err, zerr.ErrBlobNotFound) && !errors.As(err, &pathNotFoundErr) { + // For other errors, use generic error + resultErr = zerr.ErrBadBlobDigest + } + + imgRes := getResult(imageName, tag, man.Subject.Digest, resultErr) scrubbedManifests[man.Subject.Digest] = imgRes scrubbedManifests[manifest.Digest] = imgRes @@ -340,10 +393,10 @@ func CheckLayers( func getResult(imageName, tag string, affectedBlobDigest godigest.Digest, err error) ScrubImageResult { if err != nil { - return newScrubImageResult(imageName, tag, "affected", affectedBlobDigest.Encoded(), err.Error()) + return newScrubImageResult(imageName, tag, statusAffected, affectedBlobDigest.Encoded(), err.Error()) } - return newScrubImageResult(imageName, tag, "ok", "", "") + return newScrubImageResult(imageName, tag, statusOK, "", "") } func newScrubImageResult(imageName, tag, status, affectedBlob, err string) ScrubImageResult { diff --git a/pkg/storage/scrub_test.go b/pkg/storage/scrub_test.go index 78de961b..617e85cd 100644 --- a/pkg/storage/scrub_test.go +++ b/pkg/storage/scrub_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "os" "path" "regexp" "strings" @@ -347,6 +348,122 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(actual, ShouldContainSubstring, fmt.Sprintf("test 1.0 affected %s blob not found", layerDig)) }) + Convey("Scrub index with missing manifest blob - graceful handling", func() { + // Create a multiarch image with multiple manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + err = WriteMultiArchImageToFileSystem(multiarchImage, repoName, "2.0", storeCtlr) + So(err, ShouldBeNil) + + // Get the index to find the index manifest digest + idx, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + // Find the index manifest + var indexManifestDesc ispec.Descriptor + + for _, desc := range idx.Manifests { + if desc.MediaType == ispec.MediaTypeImageIndex { + indexManifestDesc = desc + + break + } + } + + // Get the index content to find the manifest digests within it + indexBlob, err := imgStore.GetBlobContent(repoName, indexManifestDesc.Digest) + So(err, ShouldBeNil) + + var indexContent ispec.Index + err = json.Unmarshal(indexBlob, &indexContent) + So(err, ShouldBeNil) + + // Delete one of the manifest blobs within the index (but not all) + missingManifestDig := indexContent.Manifests[0].Digest.Encoded() + missingManifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", missingManifestDig) + err = driver.Delete(missingManifestFile) + So(err, ShouldBeNil) + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + + // Should mark the index as affected due to missing manifest + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 2.0 affected") + // Should continue processing and report the missing manifest + So(actual, ShouldContainSubstring, missingManifestDig) + }) + + Convey("Scrub index with non-missing error on manifest blob via file permissions", func() { + // Skip for non-local storage (S3/DynamoDB) since we can't chmod remote files + if driver.Name() != storageConstants.LocalStorageDriverName { + return + } + + // Create a multiarch image with multiple manifests + multiarchImage := CreateMultiarchWith().RandomImages(2).Build() + err = WriteMultiArchImageToFileSystem(multiarchImage, repoName, "2.1", storeCtlr) + So(err, ShouldBeNil) + + // Get the index to find the index manifest digest + idx, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + // Find the index manifest + var indexManifestDesc ispec.Descriptor + + for _, desc := range idx.Manifests { + if desc.MediaType == ispec.MediaTypeImageIndex { + indexManifestDesc = desc + + break + } + } + + // Get the index content to find the manifest digests within it + indexBlob, err := imgStore.GetBlobContent(repoName, indexManifestDesc.Digest) + So(err, ShouldBeNil) + + var indexContent ispec.Index + err = json.Unmarshal(indexBlob, &indexContent) + So(err, ShouldBeNil) + + // Remove read permissions on one of the manifest blobs to cause a permission denied error (non-missing error) + manifestDig := indexContent.Manifests[0].Digest.Encoded() + manifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", manifestDig) + err = os.Chmod(manifestFile, 0o000) + So(err, ShouldBeNil) + + // Restore permissions after test + defer func() { + _ = os.Chmod(manifestFile, 0o644) + }() + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + + // Should mark the index as affected due to non-missing error on manifest + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 2.1 affected") + // Should report the manifest digest as affected blob + So(actual, ShouldContainSubstring, manifestDig) + // Should have "bad blob digest" error + So(actual, ShouldContainSubstring, "bad blob digest") + }) + Convey("Scrub index", func() { newImage := CreateRandomImage() newManifestDigest := newImage.ManifestDescriptor.Digest @@ -544,6 +661,41 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(actual, ShouldContainSubstring, "test 0.0.1 ok") }) + Convey("preserve affected status when CheckLayers would overwrite it", func() { + // Create an image with a subject + index, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + manifestDescriptor, ok := common.GetManifestDescByReference(index, image.ManifestDescriptor.Digest.String()) + So(ok, ShouldBeTrue) + + subjectImage := CreateDefaultImageWith().Subject(&manifestDescriptor).Build() + err = WriteImageToFileSystem(subjectImage, repoName, "0.0.3", storeCtlr) + So(err, ShouldBeNil) + + // Delete the subject manifest to mark it as affected + subjectManifestDig := manifestDescriptor.Digest.Encoded() + subjectManifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", subjectManifestDig) + err = driver.Delete(subjectManifestFile) + So(err, ShouldBeNil) + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + + // The manifest with the missing subject should be marked as affected + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 0.0.3 affected") + // Even if CheckLayers would pass, the affected status from the missing subject should be preserved + So(actual, ShouldContainSubstring, subjectManifestDig) + }) + Convey("the subject of the current manifest doesn't exist", func() { index, err := common.GetIndex(imgStore, repoName, log) So(err, ShouldBeNil) @@ -623,75 +775,173 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") So(actual, ShouldContainSubstring, "test 0.0.2 affected") }) - }) - Convey("test errors", func() { - mockedImgStore := mocks.MockedImageStore{ - GetRepositoriesFn: func() ([]string, error) { - return []string{repoName}, nil - }, - ValidateRepoFn: func(name string) (bool, error) { - return false, nil - }, - } + Convey("test errors", func() { + mockedImgStore := mocks.MockedImageStore{ + GetRepositoriesFn: func() ([]string, error) { + return []string{repoName}, nil + }, + ValidateRepoFn: func(name string) (bool, error) { + return false, nil + }, + } - storeController := storage.StoreController{} - storeController.DefaultStore = mockedImgStore + storeController := storage.StoreController{} + storeController.DefaultStore = mockedImgStore - _, err := storeController.CheckAllBlobsIntegrity(context.Background()) - So(err, ShouldNotBeNil) - So(err, ShouldEqual, zerr.ErrRepoBadLayout) + _, err := storeController.CheckAllBlobsIntegrity(context.Background()) + So(err, ShouldNotBeNil) + So(err, ShouldEqual, zerr.ErrRepoBadLayout) - mockedImgStore = mocks.MockedImageStore{ - GetRepositoriesFn: func() ([]string, error) { - return []string{repoName}, nil - }, - GetIndexContentFn: func(repo string) ([]byte, error) { - return []byte{}, errUnexpectedError - }, - } + mockedImgStore = mocks.MockedImageStore{ + GetRepositoriesFn: func() ([]string, error) { + return []string{repoName}, nil + }, + GetIndexContentFn: func(repo string) ([]byte, error) { + return []byte{}, errUnexpectedError + }, + } - storeController.DefaultStore = mockedImgStore + storeController.DefaultStore = mockedImgStore - _, err = storeController.CheckAllBlobsIntegrity(context.Background()) - So(err, ShouldNotBeNil) - So(err, ShouldEqual, errUnexpectedError) + _, err = storeController.CheckAllBlobsIntegrity(context.Background()) + So(err, ShouldNotBeNil) + So(err, ShouldEqual, errUnexpectedError) - manifestDigest := godigest.FromString("abcd") + manifestDigest := godigest.FromString("abcd") - mockedImgStore = mocks.MockedImageStore{ - GetRepositoriesFn: func() ([]string, error) { - return []string{repoName}, nil - }, - GetIndexContentFn: func(repo string) ([]byte, error) { - var index ispec.Index - index.SchemaVersion = 2 - index.Manifests = []ispec.Descriptor{ - { - MediaType: "InvalidMediaType", - Digest: manifestDigest, - Size: int64(100), - Annotations: map[string]string{ispec.AnnotationRefName: "1.0"}, - }, - } + mockedImgStore = mocks.MockedImageStore{ + GetRepositoriesFn: func() ([]string, error) { + return []string{repoName}, nil + }, + GetIndexContentFn: func(repo string) ([]byte, error) { + var index ispec.Index + index.SchemaVersion = 2 + index.Manifests = []ispec.Descriptor{ + { + MediaType: "InvalidMediaType", + Digest: manifestDigest, + Size: int64(100), + Annotations: map[string]string{ispec.AnnotationRefName: "1.0"}, + }, + } - return json.Marshal(index) - }, - } + return json.Marshal(index) + }, + } - storeController.DefaultStore = mockedImgStore + storeController.DefaultStore = mockedImgStore - res, err := storeController.CheckAllBlobsIntegrity(context.Background()) - So(err, ShouldBeNil) + res, err := storeController.CheckAllBlobsIntegrity(context.Background()) + So(err, ShouldBeNil) - buff := bytes.NewBufferString("") - res.PrintScrubResults(buff) + buff := bytes.NewBufferString("") + res.PrintScrubResults(buff) - space := regexp.MustCompile(`\s+`) - str := space.ReplaceAllString(buff.String(), " ") - actual := strings.TrimSpace(str) - So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") - So(actual, ShouldContainSubstring, fmt.Sprintf("%s 1.0 affected %s invalid manifest content", - repoName, manifestDigest.Encoded())) + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, fmt.Sprintf("%s 1.0 affected %s invalid manifest content", + repoName, manifestDigest.Encoded())) + }) + + Convey("scrub with non-missing error on manifest subject blob via file permissions", func() { + // Skip for non-local storage (S3/DynamoDB) since we can't chmod remote files + if driver.Name() != storageConstants.LocalStorageDriverName { + return + } + + index, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + manifestDescriptor, ok := common.GetManifestDescByReference(index, image.ManifestDescriptor.Digest.String()) + So(ok, ShouldBeTrue) + + // Create an image with a subject + subjectImage := CreateDefaultImageWith().Subject(&manifestDescriptor).Build() + err = WriteImageToFileSystem(subjectImage, repoName, "0.0.6", storeCtlr) + So(err, ShouldBeNil) + + // Get the subject manifest digest + subjectManifestDig := manifestDescriptor.Digest.Encoded() + subjectManifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", subjectManifestDig) + + // Remove read permissions to cause a permission denied error (non-missing error) + err = os.Chmod(subjectManifestFile, 0o000) + So(err, ShouldBeNil) + + // Restore permissions after test + defer func() { + _ = os.Chmod(subjectManifestFile, 0o644) + }() + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + + // Should mark the manifest as affected due to non-missing error on subject + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 0.0.6 affected") + // Should report the subject digest as affected blob + So(actual, ShouldContainSubstring, subjectManifestDig) + // Should have "bad blob digest" error + So(actual, ShouldContainSubstring, "bad blob digest") + }) + + Convey("scrub with non-missing error on index subject blob via file permissions", func() { + // Skip for non-local storage (S3/DynamoDB) since we can't chmod remote files + if driver.Name() != storageConstants.LocalStorageDriverName { + return + } + + index, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + manifestDescriptor, ok := common.GetManifestDescByReference(index, image.ManifestDescriptor.Digest.String()) + So(ok, ShouldBeTrue) + + // Create a multiarch image with a subject + err = WriteMultiArchImageToFileSystem(CreateMultiarchWith().RandomImages(1).Subject(&manifestDescriptor).Build(), + repoName, "0.0.7", storeCtlr) + So(err, ShouldBeNil) + + // Get the subject manifest digest + subjectManifestDig := manifestDescriptor.Digest.Encoded() + subjectManifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", subjectManifestDig) + + // Remove read permissions to cause a permission denied error (non-missing error) + err = os.Chmod(subjectManifestFile, 0o000) + So(err, ShouldBeNil) + + // Restore permissions after test + defer func() { + _ = os.Chmod(subjectManifestFile, 0o644) + }() + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + + // Should mark the index as affected due to non-missing error on subject + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 0.0.7 affected") + // Should report the subject digest as affected blob + So(actual, ShouldContainSubstring, subjectManifestDig) + // Should have "bad blob digest" error + So(actual, ShouldContainSubstring, "bad blob digest") + }) }) } diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index 076635de..2c1eccef 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -1504,7 +1504,7 @@ func TestDeleteBlobsInUse(t *testing.T) { So(err, ShouldBeNil) ok, err := storageCommon.IsBlobReferenced(imgStore, repoName, unusedDigest, log) - So(err, ShouldNotBeNil) + So(err, ShouldBeNil) So(ok, ShouldBeFalse) }) }