fix: gracefully handle manifests missing from storage (prepare for sparse indexes) (#3503)

GC and scrub should not stop if a manifest or index is missing from storage.
Other similar changes are also included.

WRT metadb, the missing manifests cannot be added, and the results returned from metadb
do not include the descriptors for these manifests.

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
This commit is contained in:
Andrei Aaron
2025-11-13 19:26:18 +02:00
committed by GitHub
parent 2b6fba7059
commit 008527b7bb
20 changed files with 1240 additions and 138 deletions
+36
View File
@@ -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
}
+160
View File
@@ -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)
})
}
+54
View File
@@ -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")
+81 -5
View File
@@ -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)
})
})
}
+7 -4
View File
@@ -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() {
+62 -9
View File
@@ -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 {
+307 -57
View File
@@ -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")
})
})
}
+1 -1
View File
@@ -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)
})
}