fix(storage): do not open/download blobs when validating manifests (#1566)

when pushing manifests, zot will validate blobs (layers + config blob) are
present in repo, currently it opens(in case of filesystem storage) or download(
in case of cloud storage) each blob.

fixed that by adding a new method ImageStore.CheckBlobPresence() on storage
to check blobs presence without checking the cache like ImageStore.CheckBlob() method does.

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>

pkg/api/controller_test.go

@@ -3439,7 +3439,12 @@ func TestCrossRepoMount(t *testing.T) {
 		}
 
 		dir := t.TempDir()
-		ctlr := makeController(conf, dir, "../../test/data")
+		err := os.MkdirAll(path.Join(dir, "zot-cve-test"), storageConstants.DefaultDirPerms)
+		So(err, ShouldBeNil)
+
+		ctlr := makeController(conf, path.Join(dir, "zot-cve-test"), "../../test/data/zot-cve-test")
+
+		ctlr.Config.Storage.RootDirectory = dir
 		ctlr.Config.Storage.RemoteCache = false
 		ctlr.Config.Storage.Dedupe = false
 
@@ -3551,7 +3556,7 @@ func TestCrossRepoMount(t *testing.T) {
 		cm.StartAndWait(port)
 
 		// wait for dedupe task to run
-		time.Sleep(15 * time.Second)
+		time.Sleep(10 * time.Second)
 
 		params["mount"] = string(manifestDigest)
 		postResponse, err = client.R().

pkg/storage/common/common.go

@@ -138,8 +138,8 @@ func validateOCIManifest(imgStore storageTypes.ImageStore, repo, reference strin
 			continue
 		}
 
-		_, err := imgStore.GetBlobContent(repo, layer.Digest)
+		ok, _, err := imgStore.StatBlob(repo, layer.Digest)
-		if err != nil {
+		if err != nil || !ok {
 			return layer.Digest, zerr.ErrBlobNotFound
 		}
 	}

pkg/storage/local/local.go

@@ -1087,7 +1087,11 @@ func (is *ImageStoreLocal) BlobPath(repo string, digest godigest.Digest) string
 	return path.Join(is.rootDir, repo, "blobs", digest.Algorithm().String(), digest.Encoded())
 }
 
-// CheckBlob verifies a blob and returns true if the blob is correct.
+/*
+	CheckBlob verifies a blob and returns true if the blob is correct
+
+If the blob is not found but it's found in cache then it will be copied over.
+*/
 func (is *ImageStoreLocal) CheckBlob(repo string, digest godigest.Digest) (bool, int64, error) {
 	var lockLatency time.Time
 
@@ -1095,8 +1099,6 @@ func (is *ImageStoreLocal) CheckBlob(repo string, digest godigest.Digest) (bool,
 		return false, -1, err
 	}
 
-	blobPath := is.BlobPath(repo, digest)
-
 	if is.dedupe && fmt.Sprintf("%v", is.cache) != fmt.Sprintf("%v", nil) {
 		is.Lock(&lockLatency)
 		defer is.Unlock(&lockLatency)
@@ -1105,14 +1107,13 @@ func (is *ImageStoreLocal) CheckBlob(repo string, digest godigest.Digest) (bool,
 		defer is.RUnlock(&lockLatency)
 	}
 
-	binfo, err := os.Stat(blobPath)
+	if ok, size, err := is.StatBlob(repo, digest); err == nil || ok {
-	if err == nil {
+		return true, size, nil
-		is.log.Debug().Str("blob path", blobPath).Msg("blob path found")
-
-		return true, binfo.Size(), nil
 	}
 
-	is.log.Debug().Err(err).Str("blob", blobPath).Msg("failed to find blob, searching it in cache")
+	blobPath := is.BlobPath(repo, digest)
+
+	is.log.Debug().Str("blob", blobPath).Msg("failed to find blob, searching it in cache")
 
 	// Check blobs in cache
 	dstRecord, err := is.checkCacheBlob(digest)
@@ -1135,6 +1136,24 @@ func (is *ImageStoreLocal) CheckBlob(repo string, digest godigest.Digest) (bool,
 	return true, blobSize, nil
 }
 
+// StatBlob verifies if a blob is present inside a repository. The caller function SHOULD lock from outside.
+func (is *ImageStoreLocal) StatBlob(repo string, digest godigest.Digest) (bool, int64, error) {
+	if err := digest.Validate(); err != nil {
+		return false, -1, err
+	}
+
+	blobPath := is.BlobPath(repo, digest)
+
+	binfo, err := os.Stat(blobPath)
+	if err != nil {
+		is.log.Debug().Str("blob path", blobPath).Msg("failed to find blob")
+
+		return false, -1, zerr.ErrBlobNotFound
+	}
+
+	return true, binfo.Size(), nil
+}
+
 func (is *ImageStoreLocal) checkCacheBlob(digest godigest.Digest) (string, error) {
 	if err := digest.Validate(); err != nil {
 		return "", err

pkg/storage/local/local_test.go

@@ -2191,10 +2191,18 @@ func TestGarbageCollect(t *testing.T) {
 			So(err, ShouldNotBeNil)
 			So(hasBlob, ShouldEqual, false)
 
+			hasBlob, _, err = imgStore.StatBlob(repoName, odigest)
+			So(err, ShouldNotBeNil)
+			So(hasBlob, ShouldEqual, false)
+
 			hasBlob, _, err = imgStore.CheckBlob(repoName, bdigest)
 			So(err, ShouldBeNil)
 			So(hasBlob, ShouldEqual, true)
 
+			hasBlob, _, err = imgStore.StatBlob(repoName, bdigest)
+			So(err, ShouldBeNil)
+			So(hasBlob, ShouldEqual, true)
+
 			// sleep so orphan blob can be GC'ed
 			time.Sleep(5 * time.Second)
 

pkg/storage/s3/s3.go

@@ -987,7 +987,11 @@ func (is *ObjectStorage) BlobPath(repo string, digest godigest.Digest) string {
 	return path.Join(is.rootDir, repo, "blobs", digest.Algorithm().String(), digest.Encoded())
 }
 
-// CheckBlob verifies a blob and returns true if the blob is correct.
+/*
+	CheckBlob verifies a blob and returns true if the blob is correct
+
+If the blob is not found but it's found in cache then it will be copied over.
+*/
 func (is *ObjectStorage) CheckBlob(repo string, digest godigest.Digest) (bool, int64, error) {
 	var lockLatency time.Time
 
@@ -1036,6 +1040,47 @@ func (is *ObjectStorage) CheckBlob(repo string, digest godigest.Digest) (bool, i
 	return true, blobSize, nil
 }
 
+// StatBlob verifies if a blob is present inside a repository. The caller function SHOULD lock from outside.
+func (is *ObjectStorage) StatBlob(repo string, digest godigest.Digest) (bool, int64, error) {
+	if err := digest.Validate(); err != nil {
+		return false, -1, err
+	}
+
+	blobPath := is.BlobPath(repo, digest)
+
+	binfo, err := is.store.Stat(context.Background(), blobPath)
+	if err == nil && binfo.Size() > 0 {
+		is.log.Debug().Str("blob path", blobPath).Msg("blob path found")
+
+		return true, binfo.Size(), nil
+	}
+
+	if err != nil {
+		is.log.Error().Err(err).Str("blob", blobPath).Msg("failed to stat blob")
+
+		return false, -1, zerr.ErrBlobNotFound
+	}
+
+	// then it's a 'deduped' blob
+
+	// Check blobs in cache
+	dstRecord, err := is.checkCacheBlob(digest)
+	if err != nil {
+		is.log.Error().Err(err).Str("digest", digest.String()).Msg("cache: not found")
+
+		return false, -1, zerr.ErrBlobNotFound
+	}
+
+	binfo, err = is.store.Stat(context.Background(), dstRecord)
+	if err != nil {
+		is.log.Error().Err(err).Str("blob", blobPath).Msg("failed to stat blob")
+
+		return false, -1, zerr.ErrBlobNotFound
+	}
+
+	return true, binfo.Size(), nil
+}
+
 func (is *ObjectStorage) checkCacheBlob(digest godigest.Digest) (string, error) {
 	if err := digest.Validate(); err != nil {
 		return "", err
@@ -1256,7 +1301,7 @@ func (is *ObjectStorage) GetBlob(repo string, digest godigest.Digest, mediaType
 	return blobReadCloser, binfo.Size(), nil
 }
 
-// GetBlobContent returns blob contents, SHOULD lock from outside.
+// GetBlobContent returns blob contents, the caller function SHOULD lock from outside.
 func (is *ObjectStorage) GetBlobContent(repo string, digest godigest.Digest) ([]byte, error) {
 	if err := digest.Validate(); err != nil {
 		return []byte{}, err
@@ -1321,7 +1366,7 @@ func (is *ObjectStorage) GetOrasReferrers(repo string, gdigest godigest.Digest,
 	return common.GetOrasReferrers(is, repo, gdigest, artifactType, is.log)
 }
 
-// GetIndexContent returns index.json contents, SHOULD lock from outside.
+// GetIndexContent returns index.json contents, the caller function SHOULD lock from outside.
 func (is *ObjectStorage) GetIndexContent(repo string) ([]byte, error) {
 	dir := path.Join(is.rootDir, repo)
 

pkg/storage/s3/s3_test.go

@@ -890,6 +890,9 @@ func TestNegativeCasesObjectsStorage(t *testing.T) {
 
 			_, _, err = imgStore.CheckBlob(testImage, digest)
 			So(err, ShouldNotBeNil)
+
+			_, _, err = imgStore.StatBlob(testImage, digest)
+			So(err, ShouldNotBeNil)
 		})
 
 		Convey("Test ValidateRepo", func(c C) {
@@ -1270,7 +1273,13 @@ func TestS3Dedupe(t *testing.T) {
 		So(err, ShouldBeNil)
 		So(blob, ShouldEqual, buflen)
 
-		_, checkBlobSize1, err := imgStore.CheckBlob("dedupe1", digest)
+		ok, checkBlobSize1, err := imgStore.CheckBlob("dedupe1", digest)
+		So(ok, ShouldBeTrue)
+		So(checkBlobSize1, ShouldBeGreaterThan, 0)
+		So(err, ShouldBeNil)
+
+		ok, checkBlobSize1, err = imgStore.StatBlob("dedupe1", digest)
+		So(ok, ShouldBeTrue)
 		So(checkBlobSize1, ShouldBeGreaterThan, 0)
 		So(err, ShouldBeNil)
 
@@ -3745,6 +3754,9 @@ func TestS3DedupeErr(t *testing.T) {
 		_, err = imgStore.GetBlobContent("repo2", digest)
 		So(err, ShouldNotBeNil)
 
+		_, _, err = imgStore.StatBlob("repo2", digest)
+		So(err, ShouldNotBeNil)
+
 		_, _, _, err = imgStore.GetBlobPartial("repo2", digest, "application/vnd.oci.image.layer.v1.tar+gzip", 0, 1)
 		So(err, ShouldNotBeNil)
 	})
@@ -3829,6 +3841,9 @@ func TestS3DedupeErr(t *testing.T) {
 		_, err = imgStore.GetBlobContent("repo2", digest)
 		So(err, ShouldNotBeNil)
 
+		_, _, err = imgStore.StatBlob("repo2", digest)
+		So(err, ShouldNotBeNil)
+
 		_, _, _, err = imgStore.GetBlobPartial("repo2", digest, "application/vnd.oci.image.layer.v1.tar+gzip", 0, 1)
 		So(err, ShouldNotBeNil)
 	})

pkg/storage/storage_test.go

@@ -259,6 +259,10 @@ func TestStorageAPIs(t *testing.T) {
 						_, _, err = imgStore.CheckBlob("test", digest)
 						So(err, ShouldBeNil)
 
+						ok, _, err := imgStore.StatBlob("test", digest)
+						So(ok, ShouldBeTrue)
+						So(err, ShouldBeNil)
+
 						blob, _, err := imgStore.GetBlob("test", digest, "application/vnd.oci.image.layer.v1.tar+gzip")
 						So(err, ShouldBeNil)
 
@@ -401,6 +405,10 @@ func TestStorageAPIs(t *testing.T) {
 							So(err, ShouldNotBeNil)
 							So(hasBlob, ShouldEqual, false)
 
+							hasBlob, _, err = imgStore.StatBlob("test", digest)
+							So(err, ShouldNotBeNil)
+							So(hasBlob, ShouldEqual, false)
+
 							err = imgStore.DeleteBlob("test", "inexistent")
 							So(err, ShouldNotBeNil)
 
@@ -457,7 +465,12 @@ func TestStorageAPIs(t *testing.T) {
 						err = imgStore.FinishBlobUpload("test", bupload, buf, digest)
 						So(err, ShouldBeNil)
 
-						_, _, err = imgStore.CheckBlob("test", digest)
+						ok, _, err := imgStore.CheckBlob("test", digest)
+						So(ok, ShouldBeTrue)
+						So(err, ShouldBeNil)
+
+						ok, _, err = imgStore.StatBlob("test", digest)
+						So(ok, ShouldBeTrue)
 						So(err, ShouldBeNil)
 
 						_, _, err = imgStore.GetBlob("test", "inexistent", "application/vnd.oci.image.layer.v1.tar+gzip")
@@ -486,6 +499,9 @@ func TestStorageAPIs(t *testing.T) {
 
 							_, _, err = imgStore.CheckBlob("test", "inexistent")
 							So(err, ShouldNotBeNil)
+
+							_, _, err = imgStore.StatBlob("test", "inexistent")
+							So(err, ShouldNotBeNil)
 						})
 
 						Convey("Bad image manifest", func() {

pkg/storage/types/types.go

@@ -38,6 +38,7 @@ type ImageStore interface { //nolint:interfacebloat
 	DeleteBlobUpload(repo, uuid string) error
 	BlobPath(repo string, digest godigest.Digest) string
 	CheckBlob(repo string, digest godigest.Digest) (bool, int64, error)
+	StatBlob(repo string, digest godigest.Digest) (bool, int64, error)
 	GetBlob(repo string, digest godigest.Digest, mediaType string) (io.ReadCloser, int64, error)
 	GetBlobPartial(repo string, digest godigest.Digest, mediaType string, from, to int64,
 	) (io.ReadCloser, int64, int64, error)

pkg/test/mocks/image_store_mock.go

@@ -35,6 +35,7 @@ type MockedImageStore struct {
 	DeleteBlobUploadFn     func(repo string, uuid string) error
 	BlobPathFn             func(repo string, digest godigest.Digest) string
 	CheckBlobFn            func(repo string, digest godigest.Digest) (bool, int64, error)
+	StatBlobFn             func(repo string, digest godigest.Digest) (bool, int64, error)
 	GetBlobPartialFn       func(repo string, digest godigest.Digest, mediaType string, from, to int64,
 	) (io.ReadCloser, int64, int64, error)
 	GetBlobFn          func(repo string, digest godigest.Digest, mediaType string) (io.ReadCloser, int64, error)
@@ -251,6 +252,14 @@ func (is MockedImageStore) CheckBlob(repo string, digest godigest.Digest) (bool,
 	return true, 0, nil
 }
 
+func (is MockedImageStore) StatBlob(repo string, digest godigest.Digest) (bool, int64, error) {
+	if is.StatBlobFn != nil {
+		return is.StatBlobFn(repo, digest)
+	}
+
+	return true, 0, nil
+}
+
 func (is MockedImageStore) GetBlobPartial(repo string, digest godigest.Digest, mediaType string, from, to int64,
 ) (io.ReadCloser, int64, int64, error) {
 	if is.GetBlobPartialFn != nil {