Repository: ratify-project/ratify
Pull request #1740 (2024-08-23 14:00:28)

fix: remove critical cache failure in oras `GetBlobContent`
# Description

## What this PR does / why we need it:
In a previous [PR](https://github.com/ratify-project/ratify/pull/1726), we have removed critical cache failure in oras `getReferenceManifest` method.
This PR will address the same issue in the `GetBlobContent` method.
And added more unit tests to fully cover both methods.

## Which issue(s) this PR fixes *(optional, using `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when the PR gets merged)*:
Fixes https://github.com/ratify-project/ratify/issues/1741

## Type of change

Please delete options that are not relevant.

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Helm Chart Change (any edit/addition/update that is necessary for changes merged to the `main` branch)
- [ ] This change requires a documentation update

# How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

- [ ] Test A
- [ ] Test B

# Checklist:

- [ ] Does the affected code have corresponding tests?
- [ ] Are the changes documented, not just with inline documentation, but also with conceptual documentation such as an overview of a new feature, or task-based documentation like a tutorial? Consider if this change should be announced on your project blog.
- [ ] Does this introduce breaking changes that would require an announcement or bumping the major version?
- [ ] Do all new files have appropriate license header?

# Post Merge Requirements
- [ ] MAINTAINERS: manually trigger the "Publish Package" workflow after merging any PR that indicates `Helm Chart Change`


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/ratify-project/ratify/pull/1740?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ratify-project) Report
All modified and coverable lines are covered by tests :white_check_mark:

| [Files](https://app.codecov.io/gh/ratify-project/ratify/pull/1740?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ratify-project) | Coverage Δ | |
|---|---|---|
| [pkg/referrerstore/oras/mocks/oras\_storage.go](https://app.codecov.io/gh/ratify-project/ratify/pull/1740?src=pr&el=tree&filepath=pkg%2Freferrerstore%2Foras%2Fmocks%2Foras_storage.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ratify-project#diff-cGtnL3JlZmVycmVyc3RvcmUvb3Jhcy9tb2Nrcy9vcmFzX3N0b3JhZ2UuZ28=) | `94.11% <100.00%> (+3.20%)` | :arrow_up: |
| [pkg/referrerstore/oras/oras.go](https://app.codecov.io/gh/ratify-project/ratify/pull/1740?src=pr&el=tree&filepath=pkg%2Freferrerstore%2Foras%2Foras.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ratify-project#diff-cGtnL3JlZmVycmVyc3RvcmUvb3Jhcy9vcmFzLmdv) | `74.49% <100.00%> (+6.71%)` | :arrow_up: |

... and [35 files with indirect coverage changes](https://app.codecov.io/gh/ratify-project/ratify/pull/1740/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ratify-project)


Patch:
diff --git a/pkg/referrerstore/oras/mocks/oras_storage.go b/pkg/referrerstore/oras/mocks/oras_storage.go
index b2bffa4..5567a31 100644
--- a/pkg/referrerstore/oras/mocks/oras_storage.go
+++ b/pkg/referrerstore/oras/mocks/oras_storage.go
@@ -30,6 +30,7 @@ type TestStorage struct {
 	ExistsMap map[digest.Digest]io.Reader
 	ExistsErr error
 	FetchErr  error
+	PushErr   error
 }
 
 func (s TestStorage) Exists(_ context.Context, target oci.Descriptor) (bool, error) {
@@ -43,6 +44,9 @@ func (s TestStorage) Exists(_ context.Context, target oci.Descriptor) (bool, err
 }
 
 func (s TestStorage) Push(_ context.Context, expected oci.Descriptor, content io.Reader) error {
+	if s.PushErr != nil {
+		return s.PushErr
+	}
 	s.ExistsMap[expected.Digest] = content
 	return nil
 }
diff --git a/pkg/referrerstore/oras/oras.go b/pkg/referrerstore/oras/oras.go
index ef11164..0a8f146 100644
--- a/pkg/referrerstore/oras/oras.go
+++ b/pkg/referrerstore/oras/oras.go
@@ -256,6 +256,8 @@ func (store *orasStore) ListReferrers(ctx context.Context, subjectReference comm
 
 func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference common.Reference, digest digest.Digest) ([]byte, error) {
 	var err error
+	var blobContent []byte
+
 	repository, err := store.createRepository(ctx, store, subjectReference)
 	if err != nil {
 		return nil, err
@@ -270,10 +272,18 @@ func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference com
 	// check if blob exists in local ORAS cache
 	isCached, err := store.localCache.Exists(ctx, blobDescriptor)
 	if err != nil {
-		return nil, err
+		logger.GetLogger(ctx, logOpt).Warnf("failed to check if blob [%s] exists in cache: %v", blobDescriptor.Digest.String(), err)
 	}
 	metrics.ReportBlobCacheCount(ctx, isCached)
 
+	if isCached {
+		blobContent, err = store.getRawContentFromCache(ctx, blobDescriptor)
+		if err != nil {
+			isCached = false
+			logger.GetLogger(ctx, logOpt).Warnf("failed to get blob [%s] from cache: %v", blobDescriptor.Digest.String(), err)
+		}
+	}
+
 	if !isCached {
 		// generate the reference path with digest
 		ref := fmt.Sprintf("%s@%s", subjectReference.Path, digest)
@@ -284,16 +294,20 @@ func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference com
 			evictOnError(ctx, err, subjectReference.Original)
 			return nil, err
 		}
+		if blobContent, err = io.ReadAll(rc); err != nil {
+			return nil, re.ErrorCodeGetBlobContentFailure.WithError(err)
+		}
 
 		// push fetched content to local ORAS cache
+		// If multiple goroutines try to push the same blob to the cache, oras-go
+		// may return `ErrAlreadyExists` error. This is expected and can be ignored.
 		orasExistsExpectedError := fmt.Errorf("%s: %s: %w", blobDesc.Digest, blobDesc.MediaType, errdef.ErrAlreadyExists)
-		err = store.localCache.Push(ctx, blobDesc, rc)
-		if err != nil && err.Error() != orasExistsExpectedError.Error() {
-			return nil, err
+		if err = store.localCache.Push(ctx, blobDesc, bytes.NewReader(blobContent)); err != nil && err.Error() != orasExistsExpectedError.Error() {
+			logger.GetLogger(ctx, logOpt).Warnf("failed to save blob [%s] in cache: %v", blobDesc.Digest, err)
 		}
 	}
 
-	return store.getRawContentFromCache(ctx, blobDescriptor)
+	return blobContent, nil
 }
 
 func (store *orasStore) GetReferenceManifest(ctx context.Context, subjectReference common.Reference, referenceDesc ocispecs.ReferenceDescriptor) (ocispecs.ReferenceManifest, error) {
@@ -331,6 +345,8 @@ func (store *orasStore) GetReferenceManifest(ctx context.Context, subjectReferen
 		}
 
 		// push fetched manifest to local ORAS cache
+		// If multiple goroutines try to push the same manifest to the cache, oras-go
+		// may return `ErrAlreadyExists` error. This is expected and can be ignored.
 		orasExistsExpectedError := fmt.Errorf("%s: %s: %w", referenceDesc.Descriptor.Digest, referenceDesc.Descriptor.MediaType, errdef.ErrAlreadyExists)
 		err = store.localCache.Push(ctx, referenceDesc.Descriptor, bytes.NewReader(manifestBytes))
 		if err != nil && err.Error() != orasExistsExpectedError.Error() {


Test patch:
diff --git a/pkg/referrerstore/oras/oras_test.go b/pkg/referrerstore/oras/oras_test.go
index 818d9c7..33eface 100644
--- a/pkg/referrerstore/oras/oras_test.go
+++ b/pkg/referrerstore/oras/oras_test.go
@@ -36,6 +36,7 @@ import (
 	"github.com/ratify-project/ratify/pkg/ocispecs"
 	"github.com/ratify-project/ratify/pkg/referrerstore/config"
 	"github.com/ratify-project/ratify/pkg/referrerstore/oras/mocks"
+	"oras.land/oras-go/v2/content"
 	"oras.land/oras-go/v2/registry"
 	"oras.land/oras-go/v2/registry/remote/errcode"
 )
@@ -46,6 +47,50 @@ const (
 	validReferenceMediatype = "application/vnd.oci.image.manifest.right.v1+json"
 )
 
+var (
+	artifactDigestNotCached          = digest.FromString("testArtifactDigestNotCached")
+	artifactDigest                   = digest.FromString("testArtifactDigest")
+	invalidManifestBytes             = []byte("invalid manifest")
+	blobDigest                       = digest.FromString("testBlobDigest")
+	firstDigest                      = digest.FromString("testDigest")
+	manifestNotCachedBytes           []byte
+	manifestCachedBytesWithWrongType []byte
+	manifestCachedBytes              []byte
+)
+
+func init() {
+	manifestNotCached := oci.Manifest{
+		MediaType: validReferenceMediatype,
+		Config:    oci.Descriptor{},
+		Layers:    []oci.Descriptor{},
+	}
+	manifestNotCachedBytes, _ = json.Marshal(manifestNotCached)
+
+	manifestCachedWithWrongType := oci.Manifest{
+		MediaType: wrongReferenceMediatype,
+		Config:    oci.Descriptor{},
+		Layers:    []oci.Descriptor{},
+	}
+	manifestCachedBytesWithWrongType, _ = json.Marshal(manifestCachedWithWrongType)
+
+	manifestCached := oci.Manifest{
+		MediaType: validReferenceMediatype,
+		Config:    oci.Descriptor{},
+		Layers:    []oci.Descriptor{},
+	}
+	manifestCachedBytes, _ = json.Marshal(manifestCached)
+}
+
+type errorReader struct{}
+
+func (r *errorReader) Read(_ []byte) (int, error) {
+	return 0, errors.New("error reading")
+}
+
+func (r *errorReader) Close() error {
+	return nil
+}
+
 // TestORASName tests the Name method of the oras store.
 func TestORASName(t *testing.T) {
 	conf := config.StorePluginConfig{
@@ -193,366 +238,522 @@ func TestORASListReferrers_NoSubjectDesc(t *testing.T) {
 
 // TODO: add cosign test for List Referrers
 
-// TestORASGetReferenceManifest_CachedDesc tests that the reference manifest is returned from the cache if it exists
-func TestORASGetReferenceManifest_CachedDesc(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	artifactDigest := digest.FromString("testArtifactDigest")
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	manifestCached := oci.Manifest{
-		MediaType: validReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestCachedBytes, err := json.Marshal(manifestCached)
-	if err != nil {
-		t.Fatalf("failed to marshal cached manifest: %v", err)
-	}
-	manifestNotCached := oci.Manifest{
-		MediaType: wrongReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestNotCachedBytes, err := json.Marshal(manifestNotCached)
-	if err != nil {
-		t.Fatalf("failed to marshal not cached manifest: %v", err)
-	}
-	testRepo := mocks.TestRepository{
-		FetchMap: map[digest.Digest]io.ReadCloser{
-			artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+func TestORASGetReferenceManifest(t *testing.T) {
+	tests := []struct {
+		name              string
+		inputRef          common.Reference
+		referenceDesc     ocispecs.ReferenceDescriptor
+		repo              registry.Repository
+		repoCreateErr     error
+		localCache        content.Storage
+		expectedErr       bool
+		expectedMediaType string
+	}{
+		{
+			name: "cache exists failure",
+			inputRef: common.Reference{
+				Original: "inputOriginalPath",
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{},
+			},
+			localCache: mocks.TestStorage{
+				ExistsErr: errors.New("cache exists error"),
+			},
+			expectedErr: true,
 		},
-	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsMap: map[digest.Digest]io.Reader{
-			artifactDigest: bytes.NewReader(manifestCachedBytes),
+		{
+			name: "cache fetch manifest failure",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigest: bytes.NewReader(manifestCachedBytes),
+				},
+				FetchErr: errors.New("cache fetch error"),
+			},
+			expectedErr:       false,
+			expectedMediaType: validReferenceMediatype,
 		},
-	}
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	manifest, err := store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{
-		Descriptor: oci.Descriptor{
-			MediaType: ocispecs.MediaTypeArtifactManifest,
-			Digest:    artifactDigest,
+		{
+			name: "not cached desc and fetch failed",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigestNotCached: bytes.NewReader(manifestCachedBytesWithWrongType),
+				},
+			},
+			expectedErr: true,
 		},
-	})
-	if err != nil {
-		t.Fatalf("failed to get reference manifest: %v", err)
-	}
-	if manifest.MediaType != validReferenceMediatype {
-		t.Fatalf("expected media type %s, got %s", validReferenceMediatype, manifest.MediaType)
-	}
-}
-
-// TestORASGetReferenceManifest_NotCachedDesc tests that the reference manifest is fetched from the registry if it is not cached
-func TestORASGetReferenceManifest_NotCachedDesc(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	artifactDigest := digest.FromString("testArtifactDigest")
-	artifactDigestNotCached := digest.FromString("testArtifactDigestNotCached")
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	manifestCached := oci.Manifest{
-		MediaType: wrongReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestCachedBytes, err := json.Marshal(manifestCached)
-	if err != nil {
-		t.Fatalf("failed to marshal cached manifest: %v", err)
-	}
-	manifestNotCached := oci.Manifest{
-		MediaType: validReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestNotCachedBytes, err := json.Marshal(manifestNotCached)
-	if err != nil {
-		t.Fatalf("failed to marshal not cached manifest: %v", err)
-	}
-	testRepo := mocks.TestRepository{
-		FetchMap: map[digest.Digest]io.ReadCloser{
-			artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+		{
+			name:          "create repository failed",
+			inputRef:      common.Reference{},
+			referenceDesc: ocispecs.ReferenceDescriptor{},
+			repo:          mocks.TestRepository{},
+			repoCreateErr: errors.New("create repository error"),
+			expectedErr:   true,
 		},
-	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsMap: map[digest.Digest]io.Reader{
-			artifactDigestNotCached: bytes.NewReader(manifestCachedBytes),
+		{
+			name: "reference manifest is returned from the cache if it exists",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigest: bytes.NewReader(manifestCachedBytes),
+				},
+			},
+			expectedErr:       false,
+			expectedMediaType: validReferenceMediatype,
 		},
-	}
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	manifest, err := store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{
-		Descriptor: oci.Descriptor{
-			MediaType: ocispecs.MediaTypeArtifactManifest,
-			Digest:    artifactDigest,
+		{
+			name: "reference manifest is fetched from the registry if it is not cached",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigestNotCached: bytes.NewReader(manifestCachedBytesWithWrongType),
+				},
+			},
+			expectedErr:       false,
+			expectedMediaType: validReferenceMediatype,
 		},
-	})
-	if err != nil {
-		t.Fatalf("failed to get reference manifest: %v", err)
-	}
-	if manifest.MediaType != validReferenceMediatype {
-		t.Fatalf("expected media type %s, got %s", validReferenceMediatype, manifest.MediaType)
-	}
-}
-
-func TestORASGetReferenceManifest_CacheFetchManifestFailure(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	artifactDigest := digest.FromString("testArtifactDigest")
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	manifestCached := oci.Manifest{
-		MediaType: wrongReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestCachedBytes, err := json.Marshal(manifestCached)
-	if err != nil {
-		t.Fatalf("failed to marshal cached manifest: %v", err)
-	}
-	manifestNotCached := oci.Manifest{
-		MediaType: validReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestNotCachedBytes, err := json.Marshal(manifestNotCached)
-	if err != nil {
-		t.Fatalf("failed to marshal not cached manifest: %v", err)
-	}
-	testRepo := mocks.TestRepository{
-		FetchMap: map[digest.Digest]io.ReadCloser{
-			artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+		{
+			name: "descriptor not cached and fail during io.ReadAll from manifest",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: &errorReader{},
+				},
+			},
+			localCache: mocks.TestStorage{
+				FetchErr: errors.New("cache fetch error"),
+			},
+			expectedErr: true,
 		},
-	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsMap: map[digest.Digest]io.Reader{
-			artifactDigest: bytes.NewReader(manifestCachedBytes),
+		{
+			name: "failed to unmarshal to oci manifest",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: oci.MediaTypeImageManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(invalidManifestBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigestNotCached: bytes.NewReader(manifestCachedBytesWithWrongType),
+				},
+			},
+			expectedErr: true,
 		},
-		FetchErr: errors.New("cache fetch error"),
-	}
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	manifest, err := store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{
-		Descriptor: oci.Descriptor{
-			MediaType: ocispecs.MediaTypeArtifactManifest,
-			Digest:    artifactDigest,
+		{
+			name: "failed to unmarshal to artifact manifest",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(invalidManifestBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigestNotCached: bytes.NewReader(manifestCachedBytesWithWrongType),
+				},
+			},
+			expectedErr: true,
 		},
-	})
-	if err != nil {
-		t.Fatalf("failed to get reference manifest: %v", err)
-	}
-	if manifest.MediaType != validReferenceMediatype {
-		t.Fatalf("expected media type %s, got %s", validReferenceMediatype, manifest.MediaType)
-	}
-}
-
-func TestORASGetReferenceManifest_CacheExistsFailure(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	artifactDigest := digest.FromString("testArtifactDigest")
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	if err != nil {
-		t.Fatalf("failed to marshal cached manifest: %v", err)
-	}
-
-	testRepo := mocks.TestRepository{
-		FetchMap: map[digest.Digest]io.ReadCloser{},
-	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsErr: errors.New("cache exists error"),
-	}
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	_, err = store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{
-		Descriptor: oci.Descriptor{
-			MediaType: ocispecs.MediaTypeArtifactManifest,
-			Digest:    artifactDigest,
+		{
+			name: "unsupported manifest media type",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: "unsupported media type",
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(invalidManifestBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigestNotCached: bytes.NewReader(manifestCachedBytesWithWrongType),
+				},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "failed to push manifest to cache",
+			inputRef: common.Reference{
+				Original: inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			referenceDesc: ocispecs.ReferenceDescriptor{
+				Descriptor: oci.Descriptor{
+					MediaType: ocispecs.MediaTypeArtifactManifest,
+					Digest:    artifactDigest,
+				},
+			},
+			repo: mocks.TestRepository{
+				FetchMap: map[digest.Digest]io.ReadCloser{
+					artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)),
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					artifactDigestNotCached: bytes.NewReader(manifestCachedBytesWithWrongType),
+				},
+				PushErr: errors.New("push content error"),
+			},
+			expectedErr:       false,
+			expectedMediaType: validReferenceMediatype,
 		},
-	})
-	if err == nil {
-		t.Fatalf("expected error fetching reference manifest")
 	}
-}
 
-func TestORASGetReferenceManifest_NotCachedDescAndFetchFailed(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	artifactDigest := digest.FromString("testArtifactDigest")
-	artifactDigestNotCached := digest.FromString("testArtifactDigestNotCached")
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	manifestCached := oci.Manifest{
-		MediaType: wrongReferenceMediatype,
-		Config:    oci.Descriptor{},
-		Layers:    []oci.Descriptor{},
-	}
-	manifestCachedBytes, err := json.Marshal(manifestCached)
-	if err != nil {
-		t.Fatalf("failed to marshal cached manifest: %v", err)
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			conf := config.StorePluginConfig{
+				"name": "oras",
+			}
+			store, err := createBaseStore("1.0.0", conf)
+			if err != nil {
+				t.Fatalf("failed to create oras store: %v", err)
+			}
+			store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
+				return tc.repo, tc.repoCreateErr
+			}
+			store.localCache = tc.localCache
+
+			manifest, err := store.GetReferenceManifest(context.Background(), tc.inputRef, tc.referenceDesc)
+			if tc.expectedErr {
+				if err == nil {
+					t.Fatalf("expected error fetching reference manifest")
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("failed to get reference manifest: %v", err)
+				}
+				if manifest.MediaType != tc.expectedMediaType {
+					t.Fatalf("expected media type %s, got %s", tc.expectedMediaType, manifest.MediaType)
+				}
+			}
+		})
 	}
+}
 
-	testRepo := mocks.TestRepository{
-		FetchMap: map[digest.Digest]io.ReadCloser{},
-	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsMap: map[digest.Digest]io.Reader{
-			artifactDigestNotCached: bytes.NewReader(manifestCachedBytes),
+func TestORASGetBlobContent(t *testing.T) {
+	tests := []struct {
+		name             string
+		repo             registry.Repository
+		localCache       content.Storage
+		repoCreateErr    error
+		subjectReference common.Reference
+		digest           digest.Digest
+		expectedContent  []byte
+		expectedErr      bool
+	}{
+		{
+			name:          "fail to create repository",
+			repo:          mocks.TestRepository{},
+			repoCreateErr: errors.New("create repository error"),
+			expectedErr:   true,
 		},
-	}
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	_, err = store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{
-		Descriptor: oci.Descriptor{
-			MediaType: ocispecs.MediaTypeArtifactManifest,
-			Digest:    artifactDigest,
+		{
+			name: "fail to check blob existence",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{
+						fmt.Sprintf("%s@%s", inputOriginalPath, blobDigest.String()): {
+							Descriptor: oci.Descriptor{
+								Digest: blobDigest,
+							},
+							Reader: io.NopCloser(bytes.NewReader([]byte("test content"))),
+						},
+					},
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{},
+				ExistsErr: errors.New("check blob existence error"),
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:          blobDigest,
+			expectedContent: []byte("test content"),
+			expectedErr:     false,
 		},
-	})
-	if err == nil {
-		t.Fatalf("expected error fetching reference manifest")
-	}
-}
-
-// TestORASGetBlobContent_CachedDesc tests that the blob content is fetched from the cache if it is cached
-func TestORASGetBlobContent_CachedDesc(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	blobDigest := digest.FromString("testBlobDigest")
-	expectedContent := []byte("test content")
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Path:     inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	testRepo := mocks.TestRepository{
-		BlobStoreTest: mocks.TestBlobStore{
-			BlobMap: map[string]mocks.BlobPair{
-				fmt.Sprintf("%s@%s", inputRef.Path, blobDigest.String()): {
-					Descriptor: oci.Descriptor{
-						Digest: blobDigest,
+		{
+			name: "fail to get raw content from cache",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{
+						fmt.Sprintf("%s@%s", inputOriginalPath, blobDigest.String()): {
+							Descriptor: oci.Descriptor{
+								Digest: blobDigest,
+							},
+							Reader: io.NopCloser(bytes.NewReader([]byte("test content"))),
+						},
 					},
-					Reader: io.NopCloser(bytes.NewReader(expectedContent)),
 				},
 			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					blobDigest: bytes.NewReader([]byte("test content")),
+				},
+				FetchErr: errors.New("fetch blob error"),
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:          blobDigest,
+			expectedContent: []byte("test content"),
+			expectedErr:     false,
 		},
-	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsMap: map[digest.Digest]io.Reader{
-			blobDigest: bytes.NewReader(expectedContent),
+		{
+			name: "fail to fetch blob from repository",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{},
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{},
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:      blobDigest,
+			expectedErr: true,
 		},
-	}
-	content, err := store.GetBlobContent(ctx, inputRef, blobDigest)
-	if err != nil {
-		t.Fatalf("failed to get blob content: %v", err)
-	}
-	if !bytes.Equal(content, expectedContent) {
-		t.Fatalf("expected content %s, got %s", expectedContent, content)
-	}
-}
-
-// TestORASGetBlobContent_NotCachedDesc tests that the blob content is fetched from the registry if it is not cached
-func TestORASGetBlobContent_NotCachedDesc(t *testing.T) {
-	conf := config.StorePluginConfig{
-		"name": "oras",
-	}
-	ctx := context.Background()
-	firstDigest := digest.FromString("testDigest")
-	blobDigest := digest.FromString("testBlobDigest")
-	expectedContent := []byte("test content")
-	inputRef := common.Reference{
-		Original: inputOriginalPath,
-		Path:     inputOriginalPath,
-		Digest:   firstDigest,
-	}
-	store, err := createBaseStore("1.0.0", conf)
-	if err != nil {
-		t.Fatalf("failed to create oras store: %v", err)
-	}
-	testRepo := mocks.TestRepository{
-		BlobStoreTest: mocks.TestBlobStore{
-			BlobMap: map[string]mocks.BlobPair{
-				fmt.Sprintf("%s@%s", inputRef.Path, blobDigest.String()): {
-					Descriptor: oci.Descriptor{
-						Digest: blobDigest,
+		{
+			name: "fail to read fetched blob",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{
+						fmt.Sprintf("%s@%s", inputOriginalPath, blobDigest.String()): {
+							Descriptor: oci.Descriptor{
+								Digest: blobDigest,
+							},
+							Reader: &errorReader{},
+						},
 					},
-					Reader: io.NopCloser(bytes.NewReader(expectedContent)),
 				},
 			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{},
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:      blobDigest,
+			expectedErr: true,
+		},
+		{
+			name: "fail to push content to local cache",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{
+						fmt.Sprintf("%s@%s", inputOriginalPath, blobDigest.String()): {
+							Descriptor: oci.Descriptor{
+								Digest: blobDigest,
+							},
+							Reader: io.NopCloser(bytes.NewReader([]byte("test content"))),
+						},
+					},
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{},
+				PushErr:   errors.New("push content error"),
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:          blobDigest,
+			expectedContent: []byte("test content"),
+			expectedErr:     false,
+		},
+		{
+			name: "blob content is fetched from the cache if it is cached",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{
+						fmt.Sprintf("%s@%s", inputOriginalPath, blobDigest.String()): {
+							Descriptor: oci.Descriptor{
+								Digest: blobDigest,
+							},
+							Reader: io.NopCloser(bytes.NewReader([]byte("test content"))),
+						},
+					},
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{
+					blobDigest: bytes.NewReader([]byte("test content")),
+				},
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:          blobDigest,
+			expectedContent: []byte("test content"),
+			expectedErr:     false,
+		},
+		{
+			name: "blob content is fetched from the registry if it is not cached",
+			repo: mocks.TestRepository{
+				BlobStoreTest: mocks.TestBlobStore{
+					BlobMap: map[string]mocks.BlobPair{
+						fmt.Sprintf("%s@%s", inputOriginalPath, blobDigest.String()): {
+							Descriptor: oci.Descriptor{
+								Digest: blobDigest,
+							},
+							Reader: io.NopCloser(bytes.NewReader([]byte("test content"))),
+						},
+					},
+				},
+			},
+			localCache: mocks.TestStorage{
+				ExistsMap: map[digest.Digest]io.Reader{},
+			},
+			subjectReference: common.Reference{
+				Original: inputOriginalPath,
+				Path:     inputOriginalPath,
+				Digest:   firstDigest,
+			},
+			digest:          blobDigest,
+			expectedContent: []byte("test content"),
+			expectedErr:     false,
 		},
 	}
-	store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
-		return testRepo, nil
-	}
-	store.localCache = mocks.TestStorage{
-		ExistsMap: map[digest.Digest]io.Reader{},
-	}
-	content, err := store.GetBlobContent(ctx, inputRef, blobDigest)
-	if err != nil {
-		t.Fatalf("failed to get blob content: %v", err)
-	}
-	if !bytes.Equal(content, expectedContent) {
-		t.Fatalf("expected content %s, got %s", expectedContent, content)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			conf := config.StorePluginConfig{
+				"name": "oras",
+			}
+			store, err := createBaseStore("1.0.0", conf)
+			if err != nil {
+				t.Fatalf("failed to create oras store: %v", err)
+			}
+			store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) {
+				return tt.repo, tt.repoCreateErr
+			}
+			store.localCache = tt.localCache
+			content, err := store.GetBlobContent(context.Background(), tt.subjectReference, tt.digest)
+			if tt.expectedErr {
+				if err == nil {
+					t.Fatalf("expected error, got nil")
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("expected no error, got %v", err)
+				}
+				if !bytes.Equal(content, tt.expectedContent) {
+					t.Fatalf("expected content %s, got %s", tt.expectedContent, content)
+				}
+			}
+		})
 	}
 }