refactor(artifact): remove oci artifact support (#1359)

* refactor(artifact): remove oci artifact support
- add header to referrers call to indicated applied artifact type filters

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

* feat(gc): simplify gc logic to increase coverage

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

---------

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

pkg/storage/common.go

@@ -16,10 +16,16 @@ import (
 	"github.com/sigstore/cosign/v2/pkg/oci/remote"
 
 	zerr "zotregistry.io/zot/errors"
+	zcommon "zotregistry.io/zot/pkg/common"
 	"zotregistry.io/zot/pkg/scheduler"
 	storageConstants "zotregistry.io/zot/pkg/storage/constants"
 )
 
+const (
+	CosignType   = "cosign"
+	NotationType = "notation"
+)
+
 func SignatureMediaTypes() map[string]bool {
 	return map[string]bool{
 		notreg.ArtifactTypeNotation: true,
@@ -106,22 +112,6 @@ func ValidateManifest(imgStore ImageStore, repo, reference, mediaType string, bo
 
 			return "", zerr.ErrBadManifest
 		}
-	case ispec.MediaTypeArtifactManifest:
-		var artifact ispec.Artifact
-		if err := json.Unmarshal(body, &artifact); err != nil {
-			log.Error().Err(err).Msg("unable to unmarshal JSON")
-
-			return "", zerr.ErrBadManifest
-		}
-
-		if artifact.Subject != nil {
-			var m ispec.Descriptor
-			if err := json.Unmarshal(body, &m); err != nil {
-				log.Error().Err(err).Msg("unable to unmarshal JSON")
-
-				return "", zerr.ErrBadManifest
-			}
-		}
 	}
 
 	return "", nil
@@ -303,6 +293,27 @@ func GetImageIndex(imgStore ImageStore, repo string, digest godigest.Digest, log
 	return imageIndex, nil
 }
 
+func GetImageManifest(imgStore ImageStore, repo string, digest godigest.Digest, log zerolog.Logger,
+) (ispec.Manifest, error) {
+	var manifestContent ispec.Manifest
+
+	manifestBlob, err := imgStore.GetBlobContent(repo, digest)
+	if err != nil {
+		return manifestContent, err
+	}
+
+	manifestPath := path.Join(imgStore.RootDir(), repo, "blobs",
+		digest.Algorithm().String(), digest.Encoded())
+
+	if err := json.Unmarshal(manifestBlob, &manifestContent); err != nil {
+		log.Error().Err(err).Str("path", manifestPath).Msg("invalid JSON")
+
+		return manifestContent, err
+	}
+
+	return manifestContent, nil
+}
+
 func RemoveManifestDescByReference(index *ispec.Index, reference string, detectCollisions bool,
 ) (ispec.Descriptor, error) {
 	var removedManifest ispec.Descriptor
@@ -456,31 +467,50 @@ func PruneImageManifestsFromIndex(imgStore ImageStore, repo string, digest godig
 	return prunedManifests, nil
 }
 
-func ApplyLinter(imgStore ImageStore, linter Lint, repo string, manifestDesc ispec.Descriptor) (bool, error) {
+func ApplyLinter(imgStore ImageStore, linter Lint, repo string, descriptor ispec.Descriptor) (bool, error) {
 	pass := true
 
-	if linter != nil {
-		tag := manifestDesc.Annotations[ispec.AnnotationRefName]
-		// apply linter only on images, not signatures
-		if manifestDesc.MediaType == ispec.MediaTypeImageManifest &&
-			// check that image manifest is not cosign signature
-			!strings.HasPrefix(tag, "sha256-") &&
-			!strings.HasSuffix(tag, remote.SignatureTagSuffix) {
-			// lint new index with new manifest before writing to disk
-			pass, err := linter.Lint(repo, manifestDesc.Digest, imgStore)
-			if err != nil {
-				return false, err
-			}
+	// we'll skip anything that's not a image manifest
+	if descriptor.MediaType != ispec.MediaTypeImageManifest {
+		return pass, nil
+	}
 
-			if !pass {
-				return false, zerr.ErrImageLintAnnotations
-			}
+	if linter != nil && !IsSignature(descriptor) {
+		// lint new index with new manifest before writing to disk
+		pass, err := linter.Lint(repo, descriptor.Digest, imgStore)
+		if err != nil {
+			return false, err
+		}
+
+		if !pass {
+			return false, zerr.ErrImageLintAnnotations
 		}
 	}
 
 	return pass, nil
 }
 
+func IsSignature(descriptor ispec.Descriptor) bool {
+	tag := descriptor.Annotations[ispec.AnnotationRefName]
+
+	switch descriptor.MediaType {
+	case ispec.MediaTypeImageManifest:
+		// is cosgin signature
+		if strings.HasPrefix(tag, "sha256-") && strings.HasSuffix(tag, remote.SignatureTagSuffix) {
+			return true
+		}
+
+		// is notation signature
+		if descriptor.ArtifactType == notreg.ArtifactTypeNotation {
+			return true
+		}
+	default:
+		return false
+	}
+
+	return false
+}
+
 func GetOrasReferrers(imgStore ImageStore, repo string, gdigest godigest.Digest, artifactType string,
 	log zerolog.Logger,
 ) ([]oras.Descriptor, error) {
@@ -609,67 +639,18 @@ func GetReferrers(imgStore ImageStore, repo string, gdigest godigest.Digest, art
 			}
 
 			// filter by artifact type
-			if len(artifactTypes) > 0 {
-				found := false
+			manifestArtifactType := zcommon.GetManifestArtifactType(mfst)
 
-				for _, artifactType := range artifactTypes {
-					if artifactType != "" && mfst.Config.MediaType != artifactType {
-						continue
-					}
-
-					found = true
-
-					break
-				}
-
-				if !found {
-					continue
-				}
-			}
-
-			result = append(result, ispec.Descriptor{
-				MediaType:    manifest.MediaType,
-				ArtifactType: mfst.Config.MediaType,
-				Size:         manifest.Size,
-				Digest:       manifest.Digest,
-				Annotations:  mfst.Annotations,
-			})
-		} else if manifest.MediaType == ispec.MediaTypeArtifactManifest {
-			var art ispec.Artifact
-			if err := json.Unmarshal(buf, &art); err != nil {
-				log.Error().Err(err).Str("manifest digest", manifest.Digest.String()).Msg("invalid JSON")
-
-				return nilIndex, err
-			}
-
-			if art.Subject == nil || art.Subject.Digest != gdigest {
+			if len(artifactTypes) > 0 && !zcommon.Contains(artifactTypes, manifestArtifactType) {
 				continue
 			}
 
-			// filter by artifact type
-			if len(artifactTypes) > 0 {
-				found := false
-				for _, artifactType := range artifactTypes {
-					if artifactType != "" && art.ArtifactType != artifactType {
-						continue
-					}
-
-					found = true
-
-					break
-				}
-
-				if !found {
-					continue
-				}
-			}
-
 			result = append(result, ispec.Descriptor{
 				MediaType:    manifest.MediaType,
-				ArtifactType: art.ArtifactType,
+				ArtifactType: manifestArtifactType,
 				Size:         manifest.Size,
 				Digest:       manifest.Digest,
-				Annotations:  art.Annotations,
+				Annotations:  mfst.Annotations,
 			})
 		}
 	}
@@ -719,14 +700,13 @@ func GetOrasManifestByDigest(imgStore ImageStore, repo string, digest godigest.D
 func IsSupportedMediaType(mediaType string) bool {
 	return mediaType == ispec.MediaTypeImageIndex ||
 		mediaType == ispec.MediaTypeImageManifest ||
-		mediaType == ispec.MediaTypeArtifactManifest ||
 		mediaType == oras.MediaTypeArtifactManifest
 }
 
 func IsNonDistributable(mediaType string) bool {
-	return mediaType == ispec.MediaTypeImageLayerNonDistributable ||
-		mediaType == ispec.MediaTypeImageLayerNonDistributableGzip ||
-		mediaType == ispec.MediaTypeImageLayerNonDistributableZstd
+	return mediaType == ispec.MediaTypeImageLayerNonDistributable || //nolint:staticcheck
+		mediaType == ispec.MediaTypeImageLayerNonDistributableGzip || //nolint:staticcheck
+		mediaType == ispec.MediaTypeImageLayerNonDistributableZstd //nolint:staticcheck
 }
 
 // CheckIsImageSignature checks if the given image (repo:tag) represents a signature. The function
@@ -742,30 +722,30 @@ func IsNonDistributable(mediaType string) bool {
 func CheckIsImageSignature(repoName string, manifestBlob []byte, reference string,
 	storeController StoreController,
 ) (bool, string, godigest.Digest, error) {
-	const cosign = "cosign"
-
-	var manifestContent ispec.Artifact
+	var manifestContent ispec.Manifest
 
 	err := json.Unmarshal(manifestBlob, &manifestContent)
 	if err != nil {
 		return false, "", "", err
 	}
 
+	manifestArtifactType := zcommon.GetManifestArtifactType(manifestContent)
+
 	// check notation signature
-	if _, ok := SignatureMediaTypes()[manifestContent.ArtifactType]; ok && manifestContent.Subject != nil {
+	if _, ok := SignatureMediaTypes()[manifestArtifactType]; ok && manifestContent.Subject != nil {
 		imgStore := storeController.GetImageStore(repoName)
 
 		_, signedImageManifestDigest, _, err := imgStore.GetImageManifest(repoName,
 			manifestContent.Subject.Digest.String())
 		if err != nil {
 			if errors.Is(err, zerr.ErrManifestNotFound) {
-				return true, "notation", signedImageManifestDigest, zerr.ErrOrphanSignature
+				return true, NotationType, signedImageManifestDigest, zerr.ErrOrphanSignature
 			}
 
 			return false, "", "", err
 		}
 
-		return true, "notation", signedImageManifestDigest, nil
+		return true, NotationType, signedImageManifestDigest, nil
 	}
 
 	// check cosign
@@ -785,17 +765,17 @@ func CheckIsImageSignature(repoName string, manifestBlob []byte, reference strin
 			signedImageManifestDigest.String())
 		if err != nil {
 			if errors.Is(err, zerr.ErrManifestNotFound) {
-				return true, cosign, signedImageManifestDigest, zerr.ErrOrphanSignature
+				return true, CosignType, signedImageManifestDigest, zerr.ErrOrphanSignature
 			}
 
 			return false, "", "", err
 		}
 
 		if signedImageManifestDigest.String() == "" {
-			return true, cosign, signedImageManifestDigest, zerr.ErrOrphanSignature
+			return true, CosignType, signedImageManifestDigest, zerr.ErrOrphanSignature
 		}
 
-		return true, cosign, signedImageManifestDigest, nil
+		return true, CosignType, signedImageManifestDigest, nil
 	}
 
 	return false, "", "", nil

pkg/storage/common_test.go

@@ -118,7 +118,7 @@ func TestValidateManifest(t *testing.T) {
 				},
 				Layers: []ispec.Descriptor{
 					{
-						MediaType: ispec.MediaTypeImageLayerNonDistributable,
+						MediaType: ispec.MediaTypeImageLayerNonDistributable, //nolint:staticcheck
 						Digest:    digest,
 						Size:      int64(len(content)),
 					},
@@ -299,38 +299,11 @@ func TestGetReferrersErrors(t *testing.T) {
 			So(err, ShouldNotBeNil)
 		})
 
-		Convey("Trigger unmarshal error on artifact mediaType", func(c C) {
-			index = ispec.Index{
-				Manifests: []ispec.Descriptor{
-					{
-						MediaType: ispec.MediaTypeArtifactManifest,
-						Digest:    digest,
-					},
-				},
-			}
-
-			indexBuf, err = json.Marshal(index)
-			So(err, ShouldBeNil)
-
-			imgStore = &mocks.MockedImageStore{
-				GetIndexContentFn: func(repo string) ([]byte, error) {
-					return indexBuf, nil
-				},
-				GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) {
-					return []byte{}, nil
-				},
-			}
-
-			_, err = storage.GetReferrers(imgStore, "zot-test", validDigest,
-				[]string{artifactType}, log.With().Caller().Logger())
-			So(err, ShouldNotBeNil)
-		})
-
 		Convey("Trigger nil subject", func(c C) {
 			index = ispec.Index{
 				Manifests: []ispec.Descriptor{
 					{
-						MediaType: ispec.MediaTypeArtifactManifest,
+						MediaType: ispec.MediaTypeImageManifest,
 						Digest:    digest,
 					},
 				},
@@ -398,3 +371,12 @@ func TestGetImageIndexErrors(t *testing.T) {
 		So(err, ShouldNotBeNil)
 	})
 }
+
+func TestIsSignature(t *testing.T) {
+	Convey("Unknown media type", t, func(c C) {
+		isSingature := storage.IsSignature(ispec.Descriptor{
+			MediaType: "unknown media type",
+		})
+		So(isSingature, ShouldBeFalse)
+	})
+}

pkg/storage/local/local.go

@@ -29,7 +29,7 @@ import (
 	"github.com/sigstore/cosign/v2/pkg/oci/remote"
 
 	zerr "zotregistry.io/zot/errors"
-	"zotregistry.io/zot/pkg/common"
+	zcommon "zotregistry.io/zot/pkg/common"
 	"zotregistry.io/zot/pkg/extensions/monitoring"
 	zlog "zotregistry.io/zot/pkg/log"
 	zreg "zotregistry.io/zot/pkg/regexp"
@@ -66,7 +66,7 @@ func (is *ImageStoreLocal) RootDir() string {
 }
 
 func (is *ImageStoreLocal) DirExists(d string) bool {
-	return common.DirExists(d)
+	return zcommon.DirExists(d)
 }
 
 // NewImageStore returns a new image store backed by a file storage.
@@ -535,7 +535,18 @@ func (is *ImageStoreLocal) PutImageManifest(repo, reference, mediaType string, /
 		return "", err
 	}
 
-	// apply linter only on images, not signatures
+	if mediaType == ispec.MediaTypeImageManifest {
+		var manifest ispec.Manifest
+
+		err := json.Unmarshal(body, &manifest)
+		if err != nil {
+			return "", err
+		}
+
+		desc.ArtifactType = zcommon.GetManifestArtifactType(manifest)
+	}
+
+	// apply linter only on images, not signatures or indexes
 	pass, err := storage.ApplyLinter(is, is.linter, repo, desc)
 	if !pass {
 		is.log.Error().Err(err).Str("repository", repo).Str("reference", reference).Msg("linter didn't pass")
@@ -1440,6 +1451,12 @@ func ensureDir(dir string, log zerolog.Logger) error {
 	return nil
 }
 
+type extendedManifest struct {
+	ispec.Manifest
+
+	Digest godigest.Digest
+}
+
 func (is *ImageStoreLocal) garbageCollect(dir string, repo string) error {
 	oci, err := umoci.OpenLayout(dir)
 	if err := test.Error(err); err != nil {
@@ -1455,7 +1472,7 @@ func (is *ImageStoreLocal) garbageCollect(dir string, repo string) error {
 
 	referencedByImageIndex := []string{}
 	cosignDescriptors := []ispec.Descriptor{}
-	notationDescriptors := []ispec.Descriptor{}
+	notationManifests := []extendedManifest{}
 
 	/* gather manifests references by multiarch images (to skip gc)
 	gather cosign and notation signatures descriptors */
@@ -1479,10 +1496,27 @@ func (is *ImageStoreLocal) garbageCollect(dir string, repo string) error {
 				// gather cosign signatures
 				if strings.HasPrefix(tag, "sha256-") && strings.HasSuffix(tag, remote.SignatureTagSuffix) {
 					cosignDescriptors = append(cosignDescriptors, desc)
+
+					continue
 				}
 			}
-		case ispec.MediaTypeArtifactManifest:
-			notationDescriptors = append(notationDescriptors, desc)
+
+			manifestContent, err := storage.GetImageManifest(is, repo, desc.Digest, is.log)
+			if err != nil {
+				is.log.Error().Err(err).Str("repo", repo).Str("digest", desc.Digest.String()).
+					Msg("gc: failed to read manifest image")
+
+				return err
+			}
+
+			if zcommon.GetManifestArtifactType(manifestContent) == notreg.ArtifactTypeNotation {
+				notationManifests = append(notationManifests, extendedManifest{
+					Digest:   desc.Digest,
+					Manifest: manifestContent,
+				})
+
+				continue
+			}
 		}
 	}
 
@@ -1500,7 +1534,7 @@ func (is *ImageStoreLocal) garbageCollect(dir string, repo string) error {
 
 	is.log.Info().Msg("gc: notation signatures")
 
-	if err := gcNotationSignatures(is, oci, &index, repo, notationDescriptors); err != nil {
+	if err := gcNotationSignatures(is, oci, &index, repo, notationManifests); err != nil {
 		return err
 	}
 
@@ -1519,7 +1553,7 @@ func gcUntaggedManifests(imgStore *ImageStoreLocal, oci casext.Engine, index *is
 ) error {
 	for _, desc := range index.Manifests {
 		// skip manifests referenced in image indexex
-		if common.Contains(referencedByImageIndex, desc.Digest.String()) {
+		if zcommon.Contains(referencedByImageIndex, desc.Digest.String()) {
 			continue
 		}
 
@@ -1616,46 +1650,24 @@ func gcCosignSignatures(imgStore *ImageStoreLocal, oci casext.Engine, index *isp
 }
 
 func gcNotationSignatures(imgStore *ImageStoreLocal, oci casext.Engine, index *ispec.Index, repo string,
-	notationDescriptors []ispec.Descriptor,
+	notationManifests []extendedManifest,
 ) error {
-	for _, notationDesc := range notationDescriptors {
+	for _, notationManifest := range notationManifests {
 		foundSubject := false
-		// check if we can find the manifest which the signature points to
-		var artManifest ispec.Artifact
-
-		buf, err := imgStore.GetBlobContent(repo, notationDesc.Digest)
-		if err != nil {
-			imgStore.log.Error().Err(err).Str("repository", repo).Str("digest", notationDesc.Digest.String()).
-				Msg("gc: failed to get oras artifact manifest")
-
-			return err
-		}
-
-		if err := json.Unmarshal(buf, &artManifest); err != nil {
-			imgStore.log.Error().Err(err).Str("repository", repo).Str("digest", notationDesc.Digest.String()).
-				Msg("gc: failed to get oras artifact manifest")
-
-			return err
-		}
-
-		// skip oci artifacts which are not signatures
-		if artManifest.ArtifactType != notreg.ArtifactTypeNotation {
-			continue
-		}
 
 		for _, desc := range index.Manifests {
-			if desc.Digest == artManifest.Subject.Digest {
+			if desc.Digest == notationManifest.Subject.Digest {
 				foundSubject = true
 			}
 		}
 
 		if !foundSubject {
 			// remove manifest
-			imgStore.log.Info().Str("repository", repo).Str("digest", notationDesc.Digest.String()).
+			imgStore.log.Info().Str("repository", repo).Str("digest", notationManifest.Digest.String()).
 				Msg("gc: removing notation signature without subject")
 
 			// no need to check for manifest conflict, if one doesn't have a subject, then none with same digest will have
-			_, _ = storage.RemoveManifestDescByReference(index, notationDesc.Digest.String(), false)
+			_, _ = storage.RemoveManifestDescByReference(index, notationManifest.Digest.String(), false)
 
 			err := oci.PutIndex(context.Background(), *index)
 			if err != nil {
@@ -1808,7 +1820,7 @@ func (is *ImageStoreLocal) GetNextDigestWithBlobPaths(lastDigests []godigest.Dig
 			return nil //nolint:nilerr // ignore files which are not blobs
 		}
 
-		if digest == "" && !common.DContains(lastDigests, blobDigest) {
+		if digest == "" && !zcommon.DContains(lastDigests, blobDigest) {
 			digest = blobDigest
 		}
 

pkg/storage/s3/s3.go

@@ -24,7 +24,7 @@ import (
 	"github.com/rs/zerolog"
 
 	zerr "zotregistry.io/zot/errors"
-	"zotregistry.io/zot/pkg/common"
+	zcommon "zotregistry.io/zot/pkg/common"
 	"zotregistry.io/zot/pkg/extensions/monitoring"
 	zlog "zotregistry.io/zot/pkg/log"
 	zreg "zotregistry.io/zot/pkg/regexp"
@@ -438,6 +438,17 @@ func (is *ObjectStorage) PutImageManifest(repo, reference, mediaType string, //n
 		return "", err
 	}
 
+	if mediaType == ispec.MediaTypeImageManifest {
+		var manifest ispec.Manifest
+
+		err := json.Unmarshal(body, &manifest)
+		if err != nil {
+			return "", err
+		}
+
+		desc.ArtifactType = zcommon.GetManifestArtifactType(manifest)
+	}
+
 	// apply linter only on images, not signatures
 	pass, err := storage.ApplyLinter(is, is.linter, repo, desc)
 	if !pass {
@@ -1419,7 +1430,7 @@ func (is *ObjectStorage) GetNextDigestWithBlobPaths(lastDigests []godigest.Diges
 			return nil //nolint:nilerr // ignore files which are not blobs
 		}
 
-		if digest == "" && !common.DContains(lastDigests, blobDigest) {
+		if digest == "" && !zcommon.DContains(lastDigests, blobDigest) {
 			digest = blobDigest
 		}
 

pkg/storage/s3/s3_test.go

@@ -584,44 +584,6 @@ func TestGetOrasAndOCIReferrers(t *testing.T) {
 			So(index.Manifests[0].Digest, ShouldEqual, manDigest)
 		})
 
-		Convey("Get oci referrers - application/vnd.oci.artifact.manifest.v1+json", func(c C) {
-			artifactType := "application/vnd.example.icecream.v1"
-
-			artifactManifest := ispec.Artifact{
-				MediaType:    ispec.MediaTypeArtifactManifest,
-				ArtifactType: artifactType,
-				Blobs: []ispec.Descriptor{
-					{
-						MediaType: "application/octet-stream",
-						Size:      int64(buflen),
-						Digest:    digest,
-					},
-				},
-				Subject: &ispec.Descriptor{
-					MediaType: ispec.MediaTypeImageManifest,
-					Size:      int64(mbuflen),
-					Digest:    mdigest,
-				},
-			}
-
-			manBuf, err := json.Marshal(artifactManifest)
-			So(err, ShouldBeNil)
-
-			manBufLen := len(manBuf)
-			manDigest := godigest.FromBytes(manBuf)
-
-			_, err = imgStore.PutImageManifest(repo, manDigest.Encoded(), ispec.MediaTypeArtifactManifest, manBuf)
-			So(err, ShouldBeNil)
-
-			index, err := imgStore.GetReferrers(repo, mdigest, []string{artifactType})
-			So(err, ShouldBeNil)
-			So(index, ShouldNotBeEmpty)
-			So(index.Manifests[1].ArtifactType, ShouldEqual, artifactType)
-			So(index.Manifests[1].MediaType, ShouldEqual, ispec.MediaTypeArtifactManifest)
-			So(index.Manifests[1].Size, ShouldEqual, manBufLen)
-			So(index.Manifests[1].Digest, ShouldEqual, manDigest)
-		})
-
 		Convey("Get oras referrers", func(c C) {
 			artifactManifest := artifactspec.Manifest{}
 			artifactManifest.ArtifactType = "signature-example"