OCI: prefer digest over tag (#953)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

If in a OCI ref both, the tag and the digest is given, the tag is
preferred, so far. BUt the digest should always have precedence.

#### Which issue(s) this PR fixes
Fixes #911

api/oci/ref.go

@@ -263,12 +263,12 @@ type ArtSpec struct {
 }
 
 func (r *ArtSpec) Version() string {
-	if r.Tag != nil {
-		return *r.Tag
-	}
 	if r.Digest != nil {
 		return "@" + string(*r.Digest)
 	}
+	if r.Tag != nil {
+		return *r.Tag
+	}
 	return "latest"
 }
 
@@ -284,14 +284,11 @@ func (r *ArtSpec) IsTagged() bool {
 	return r.Tag != nil
 }
 
-func (r *ArtSpec) Reference() string {
+func (r *ArtSpec) GetTag() string {
 	if r.Tag != nil {
 		return *r.Tag
 	}
-	if r.Digest != nil {
-		return "@" + string(*r.Digest)
-	}
-	return "latest"
+	return ""
 }
 
 func (r *ArtSpec) String() string {

api/oci/utils.go

@@ -2,6 +2,7 @@ package oci
 
 import (
 	"fmt"
+	"strings"
 
 	"ocm.software/ocm/api/credentials/cpi"
 	"ocm.software/ocm/api/oci/artdesc"
@@ -19,8 +20,15 @@ func AsTags(tag string) []string {
 
 func StandardOCIRef(host, repository, version string) string {
 	sep := grammar.TagSeparator
+	i := strings.Index(version, grammar.DigestSeparator)
+	if i > 1 {
+		return fmt.Sprintf("%s%s%s%s%s", host, grammar.RepositorySeparator, repository, sep, version)
+	}
 	if ok, _ := artdesc.IsDigest(version); ok {
 		sep = grammar.DigestSeparator
+		if strings.HasPrefix(version, sep) {
+			sep = ""
+		}
 	}
 	return fmt.Sprintf("%s%s%s%s%s", host, grammar.RepositorySeparator, repository, sep, version)
 }

api/ocm/cpi/repocpi/bridge_cv.go

@@ -328,7 +328,8 @@ func (b *componentVersionAccessBridge) AddBlob(blob cpi.BlobAccess, artType, ref
 	}
 	if prov != nil {
 		storagectx := b.GetStorageContext()
-		h := prov.LookupHandler(storagectx, artType, blob.MimeType())
+		mime := blob.MimeType()
+		h := prov.LookupHandler(storagectx, artType, mime)
 		if h != nil {
 			acc, err := h.StoreBlob(blob, artType, refName, nil, storagectx)
 			if err != nil {

api/ocm/extensions/accessmethods/maven/README.md

@@ -3,7 +3,7 @@
 ## Synopsis
 
 ```yaml
-type: mvn/v1
+type: maven/v1
 ```
 
 Provided blobs use the following media type: `application/x-tgz`
@@ -21,7 +21,7 @@ Supported specification version is `v1`
 
 The type specific specification fields are:
 
-- **`repository`** *string*
+- **`repoUrl`** *string*
 
   Base URL of the Maven (mvn) repository
 

api/ocm/extensions/accessmethods/ociartifact/method.go

@@ -155,6 +155,9 @@ var (
 )
 
 func NewMethod(ctx accspeccpi.ContextProvider, a accspeccpi.AccessSpec, ref string, repo ...oci.Repository) (accspeccpi.AccessMethod, error) {
+	if ref == "" {
+		return nil, errors.ErrInvalid(ocmcpi.KIND_OCM_REFERENCE, "<empty>")
+	}
 	m := &accessMethod{
 		spec:      a,
 		reference: ref,

api/ocm/extensions/accessmethods/ociartifact/method_test.go

@@ -31,20 +31,82 @@ var _ = Describe("Method", func() {
 		env.Cleanup()
 	})
 
-	It("accesses artifact", func() {
-		env.OCICommonTransport(OCIPATH, accessio.FormatDirectory, func() {
-			OCIManifest1(env)
+	Context("tag only", func() {
+		It("accesses artifact", func() {
+			env.OCICommonTransport(OCIPATH, accessio.FormatDirectory, func() {
+				OCIManifest1(env)
+			})
+
+			FakeOCIRepo(env, OCIPATH, OCIHOST)
+
+			spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, OCIVERSION))
+
+			m, err := spec.AccessMethod(&cpi.DummyComponentVersionAccess{env.OCMContext()})
+			Expect(err).To(Succeed())
+
+			// no credentials required for CTF as fake OCI registry.
+			Expect(credentials.GetProvidedConsumerId(m)).To(BeNil())
+			Expect(accspeccpi.GetAccessMethodImplementation(m).(blobaccess.DigestSource).Digest().String()).To(Equal("sha256:" + D_OCIMANIFEST1))
+		})
+
+		It("provides artifact hint", func() {
+			spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, OCIVERSION))
+
+			hint := spec.GetReferenceHint(&cpi.DummyComponentVersionAccess{env.OCMContext()})
+			Expect(hint).To(Equal("ocm/value:v2.0"))
+		})
+	})
+
+	Context("tag + digest", func() {
+		It("accesses artifact", func() {
+			env.OCICommonTransport(OCIPATH, accessio.FormatDirectory, func() {
+				OCIManifest1(env)
+			})
+
+			FakeOCIRepo(env, OCIPATH, OCIHOST)
+
+			spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, OCIVERSION+"@sha256:"+D_OCIMANIFEST1))
+
+			m, err := spec.AccessMethod(&cpi.DummyComponentVersionAccess{env.OCMContext()})
+			Expect(err).To(Succeed())
+
+			// no credentials required for CTF as fake OCI registry.
+			Expect(credentials.GetProvidedConsumerId(m)).To(BeNil())
+			Expect(accspeccpi.GetAccessMethodImplementation(m).(blobaccess.DigestSource).Digest().String()).To(Equal("sha256:" + D_OCIMANIFEST1))
+		})
+
+		It("provides artifact hint", func() {
+			spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, OCIVERSION+"@sha256:"+D_OCIMANIFEST1))
+
+			hint := spec.GetReferenceHint(&cpi.DummyComponentVersionAccess{env.OCMContext()})
+			Expect(hint).To(Equal("ocm/value:v2.0"))
 		})
+	})
 
-		FakeOCIRepo(env, OCIPATH, OCIHOST)
+	Context("digest", func() {
+		It("accesses artifact", func() {
+			env.OCICommonTransport(OCIPATH, accessio.FormatDirectory, func() {
+				OCIManifest1(env)
+			})
 
-		spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, OCIVERSION))
+			FakeOCIRepo(env, OCIPATH, OCIHOST)
 
-		m, err := spec.AccessMethod(&cpi.DummyComponentVersionAccess{env.OCMContext()})
-		Expect(err).To(Succeed())
+			spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, "@sha256:"+D_OCIMANIFEST1))
 
-		// no credentials required for CTF as fake OCI registry.
-		Expect(credentials.GetProvidedConsumerId(m)).To(BeNil())
-		Expect(accspeccpi.GetAccessMethodImplementation(m).(blobaccess.DigestSource).Digest().String()).To(Equal("sha256:0c4abdb72cf59cb4b77f4aacb4775f9f546ebc3face189b2224a966c8826ca9f"))
+			m, err := spec.AccessMethod(&cpi.DummyComponentVersionAccess{env.OCMContext()})
+			Expect(err).To(Succeed())
+
+			// no credentials required for CTF as fake OCI registry.
+			Expect(credentials.GetProvidedConsumerId(m)).To(BeNil())
+			Expect(accspeccpi.GetAccessMethodImplementation(m).(blobaccess.DigestSource).Digest().String()).To(Equal("sha256:" + D_OCIMANIFEST1))
+		})
+
+		It("provides artifact hint", func() {
+			spec := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE, "@sha256:"+D_OCIMANIFEST1))
+
+			hint := spec.GetReferenceHint(&cpi.DummyComponentVersionAccess{env.OCMContext()})
+			Expect(hint).To(Equal("ocm/value"))
+		})
 	})
+
 })

api/ocm/extensions/accessmethods/relativeociref/transfer_test.go

@@ -144,6 +144,6 @@ var _ = Describe("Transfer handler", func() {
 
 		fmt.Printf("%s\n", string(data))
 		hash := HashManifest1(artifactset.DefaultArtifactSetDescriptorFileName)
-		Expect(string(data)).To(StringEqualWithContext(fmt.Sprintf(`{"globalAccess":{"imageReference":"baseurl.io/ocm/value:v2.0","type":"ociArtifact"},"localReference":"%s","mediaType":"application/vnd.oci.image.manifest.v1+tar+gzip","referenceName":"ocm/value:v2.0","type":"localBlob"}`, hash)))
+		Expect(string(data)).To(StringEqualWithContext(fmt.Sprintf(`{"globalAccess":{"imageReference":"baseurl.io/ocm/value:v2.0@sha256:`+D_OCIMANIFEST1+`","type":"ociArtifact"},"localReference":"%s","mediaType":"application/vnd.oci.image.manifest.v1+tar+gzip","referenceName":"ocm/value:v2.0","type":"localBlob"}`, hash)))
 	})
 })

api/ocm/extensions/blobhandler/handlers/generic/ocirepo/blobhandler.go

@@ -2,6 +2,7 @@ package ocirepo
 
 import (
 	"encoding/json"
+	"fmt"
 	"path"
 	"strings"
 
@@ -86,15 +87,21 @@ func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, g
 	if hint == "" {
 		name = path.Join(prefix, ctx.TargetComponentName())
 	} else {
-		i := strings.LastIndex(hint, ":")
+		i := strings.LastIndex(hint, "@")
 		if i > 0 {
 			version = hint[i:]
-			name = path.Join(prefix, hint[:i])
-			tag = version[1:] // remove colon
+			name = hint[:i]
 		} else {
 			name = hint
 		}
+		i = strings.LastIndex(name, ":")
+		if i > 0 {
+			tag = name[i+1:]
+			name = name[:i]
+		}
+		name = path.Join(prefix, name)
 	}
+
 	namespace, err = repo.LookupNamespace(name)
 	if err != nil {
 		return nil, errors.Wrapf(err, "lookup namespace %s in target repository %s", name, attr.Ref)
@@ -109,6 +116,10 @@ func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, g
 	digest := set.GetMain()
 	if version == "" {
 		version = "@" + digest.String()
+	} else {
+		if version != "@"+digest.String() {
+			return nil, fmt.Errorf("corrupted digest: hint requests %q, but found %q", version[1:], digest.String())
+		}
 	}
 	art, err := set.GetArtifact(digest.String())
 	if err != nil {
@@ -120,7 +131,9 @@ func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, g
 	if err != nil {
 		return nil, err
 	}
-
-	ref := base.ComposeRef(namespace.GetNamespace() + version)
+	if tag != "" {
+		tag = ":" + tag
+	}
+	ref := base.ComposeRef(namespace.GetNamespace() + tag + version)
 	return ociartifact.New(ref), nil
 }

api/ocm/extensions/blobhandler/handlers/generic/ocirepo/upload_test.go

@@ -146,7 +146,7 @@ var _ = Describe("upload", func() {
 		Expect(acc.GetKind()).To(Equal(ociartifact.Type))
 		val := Must(ctx.AccessSpecForSpec(acc))
 		// TODO: the result is invalid for ctf: better handling for ctf refs
-		Expect(val.(*ociartifact.AccessSpec).ImageReference).To(Equal("/tmp/target//copy/ocm/value:v2.0"))
+		Expect(val.(*ociartifact.AccessSpec).ImageReference).To(Equal("/tmp/target//copy/ocm/value:v2.0@sha256:" + D_OCIMANIFEST1))
 
 		attr.Close()
 		target, err := ctfoci.Open(ctx.OCIContext(), accessobj.ACC_READONLY, TARGET, 0, env)
@@ -188,7 +188,7 @@ var _ = Describe("upload", func() {
 		Expect(acc.GetKind()).To(Equal(ociartifact.Type))
 		val := Must(ctx.AccessSpecForSpec(acc))
 		// TODO: the result is invalid for ctf: better handling for ctf refs
-		Expect(val.(*ociartifact.AccessSpec).ImageReference).To(Equal("/tmp/target//copy/ocm/value:v2.0"))
+		Expect(val.(*ociartifact.AccessSpec).ImageReference).To(Equal("/tmp/target//copy/ocm/value:v2.0@sha256:" + D_OCIMANIFEST1))
 
 		// attr.Close()
 		env.OCMContext().Finalize()
@@ -232,7 +232,7 @@ var _ = Describe("upload", func() {
 		Expect(acc.GetKind()).To(Equal(ociartifact.Type))
 		val := Must(ctx.AccessSpecForSpec(acc))
 		// TODO: the result is invalid for ctf: better handling for ctf refs
-		Expect(val.(*ociartifact.AccessSpec).ImageReference).To(Equal("/tmp/target//copy/ocm/value:v2.0"))
+		Expect(val.(*ociartifact.AccessSpec).ImageReference).To(Equal("/tmp/target//copy/ocm/value:v2.0@sha256:" + D_OCIMANIFEST1))
 
 		// attr.Close()
 		env.OCMContext().Finalize()

api/ocm/extensions/blobhandler/handlers/oci/ocirepo/blobhandler.go

@@ -148,7 +148,6 @@ func (b *artifactHandler) CheckBlob(blob cpi.BlobAccess, artType, hint string, g
 	defer finalizer.Finalize()
 
 	var namespace oci.NamespaceAccess
-	var version string
 	var name string
 	var tag string
 
@@ -162,16 +161,15 @@ func (b *artifactHandler) CheckBlob(blob cpi.BlobAccess, artType, hint string, g
 		prefix := cpi.RepositoryPrefix(ctx.TargetComponentRepository().GetSpecification())
 		i := strings.LastIndex(hint, "@")
 		if i >= 0 {
-			hint = hint[:i] // remove digest
-		}
-		i = strings.LastIndex(hint, ":")
-		if i > 0 {
-			version = hint[i:]
-			tag = version[1:] // remove colon
-			name = hint[:i]
+			name = hint[:i] // remove digest
 		} else {
 			name = hint
 		}
+		i = strings.LastIndex(name, ":")
+		if i > 0 {
+			tag = name[i+1:]
+			name = name[:i]
+		}
 
 		hash := mapocirepoattr.Get(ctx.GetContext())
 		if hash.Prefix != nil {
@@ -263,16 +261,16 @@ func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, g
 		prefix := cpi.RepositoryPrefix(ctx.TargetComponentRepository().GetSpecification())
 		i := strings.LastIndex(hint, "@")
 		if i >= 0 {
-			hint = hint[:i] // remove digest
-		}
-		i = strings.LastIndex(hint, ":")
-		if i > 0 {
 			version = hint[i:]
-			tag = version[1:] // remove colon
-			name = hint[:i]
+			name = hint[:i] // remove digest
 		} else {
 			name = hint
 		}
+		i = strings.LastIndex(name, ":")
+		if i > 0 {
+			tag = name[i+1:]
+			name = name[:i]
+		}
 
 		hash := mapocirepoattr.Get(ctx.GetContext())
 		if hash.Prefix != nil {
@@ -320,6 +318,10 @@ func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, g
 
 	if version == "" {
 		version = "@" + digest.String()
+	} else {
+		if version != "@"+digest.String() {
+			return nil, fmt.Errorf("corrupted digest: hint requests %q, but found %q", version[1:], digest.String())
+		}
 	}
 
 	err = transfer.TransferArtifact(art, namespace, oci.AsTags(tag)...)
@@ -335,7 +337,10 @@ func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, g
 	if scheme != "" {
 		scheme += "://"
 	}
-	ref := scheme + path.Join(base, namespace.GetNamespace()) + version
+	if tag != "" {
+		tag = ":" + tag
+	}
+	ref := scheme + path.Join(base, namespace.GetNamespace()) + tag + version
 	return ociartifact.New(ref), nil
 }