[Feature]: digest support for OCI transport

[mwopfner]

Feature request description

Using digest in the oci transport currently does not work (podman pull oci:/tmp/local/alpine@sha256:3d426b0bfc361d6e8303f51459f17782b219dece42a1c7fe463b6014b189c86d) because the parsing only handles the :tag notation.
From the documentation (https://github.com/containers/image/blob/main/docs/containers-transports.5.md#ocipathreference) I'm not sure if this is desired behavior or a bug.
I looked into the problem and found that it is not much needed to support it (see patch below). I'm not sure if this is the right place to request the feature, because all changes have to be made in vendor libs.

Suggest potential solution

I created a path oci_digest_support.txt and tested it on my system:

diff --git i/vendor/github.com/containers/common/libimage/pull.go w/vendor/github.com/containers/common/libimage/pull.go
index 51712bb3b..70703e1d4 100644
--- i/vendor/github.com/containers/common/libimage/pull.go
+++ w/vendor/github.com/containers/common/libimage/pull.go
@@ -227,7 +227,11 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference,
 		storageName = imageName
 
 	case ociTransport.Transport.Name():
-		split := strings.SplitN(ref.StringWithinTransport(), ":", 2)
+		split := strings.SplitN(ref.StringWithinTransport(), "@", 2)
+
+		if len(split) == 1 {
+			split = strings.SplitN(ref.StringWithinTransport(), ":", 2)
+		}
 		storageName = toLocalImageName(split[0])
 		imageName = storageName
 
diff --git i/vendor/github.com/containers/image/v5/oci/internal/oci_util.go w/vendor/github.com/containers/image/v5/oci/internal/oci_util.go
index 148bc12fa..6849459f5 100644
--- i/vendor/github.com/containers/image/v5/oci/internal/oci_util.go
+++ w/vendor/github.com/containers/image/v5/oci/internal/oci_util.go
@@ -58,7 +58,14 @@ func splitPathAndImageWindows(reference string) (string, string) {
 }
 
 func splitPathAndImageNonWindows(reference string) (string, string) {
-	sep := strings.SplitN(reference, ":", 2)
+
+	sep := strings.SplitN(reference, "@", 2)
+
+	if len(sep) == 1 {
+		sep = strings.SplitN(reference, ":", 2)
+	}
+
+	// sep := strings.SplitN(reference, ":", 2)
 	path := sep[0]
 
 	var image string
diff --git i/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go w/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go
index be22bed6d..527cadbb8 100644
--- i/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go
+++ w/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go
@@ -103,7 +103,15 @@ func (ref ociReference) Transport() types.ImageTransport {
 // e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
 // WARNING: Do not use the return value
[oci_digest_support.txt](https://github.com/containers/podman/files/10555452/oci_digest_support.txt)
 in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref ociReference) StringWithinTransport() string {
-	return fmt.Sprintf("%s:%s", ref.dir, ref.image)
+
+	// if we use a digest for the image
+	_, err := digest.Parse(ref.image)
+	if err == nil {
+		return fmt.Sprintf("%s@%s", ref.dir, ref.image)
+	} else {
+		// if we use a normal tag for the image
+		return fmt.Sprintf("%s:%s", ref.dir, ref.image)
+	}
 }
 
 // DockerReference returns a Docker reference associated with this reference
@@ -194,13 +202,26 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) {
 			if md.MediaType != imgspecv1.MediaTypeImageManifest && md.MediaType != imgspecv1.MediaTypeImageIndex {
 				continue
 			}
-			refName, ok := md.Annotations[imgspecv1.AnnotationRefName]
-			if !ok {
-				continue
-			}
-			if refName == ref.image {
-				d = &md
-				break
+
+			// if we use a digest for the image
+			digest, err := digest.Parse(ref.image)
+			if err == nil {
+				if md.Digest == digest {
+					d = &md
+					break
+				} else {
+					continue
+				}
+			} else {
+				// if we use a tag
+				refName, ok := md.Annotations[imgspecv1.AnnotationRefName]
+				if !ok {
+					continue
+				}
+				if refName == ref.image {
+					d = &md
+					break
+				}
 			}
 		}
 	}

Have you considered any alternatives?

I considered using :tag but i don't have any tags. I only have digests.

Additional context

The version section of podman info

version:
  APIVersion: 4.3.1
  Built: 1668026638
  BuiltTime: Wed Nov  9 20:43:58 2022
  GitCommit: 814b7b003cc630bf6ab188274706c383f9fb9915-dirty
  GoVersion: go1.17.13
  Os: linux
  OsArch: linux/amd64
  Version: 4.3.1

[vrothberg]

Thanks for reaching out!

@mtrmac WDYT?

[mtrmac]

(FWIW, there is the containers/image#1381 / containers/image#1677 which adds path:@index. That might be an alternative to a digest reference in some situations, but certainly not a 100% replacement. We should get that finished eventually…)

Sure, that feature seems desirable to provide.

Some specific concerns to deal with:

• I’m not sure we can just introduce a @ metacharacter; that could break existing users. We probably need to design some syntax that continues to terminate the path with :. The ~obvious choice of :@sha256:… is somewhat EDIT in conflict with the :@index work; we might be able to clearly disambiguate, or we need to change one or the other to use a different syntax. It’s certainly possible to solve, but it will require a bit more precision.
• Implementation-wise, I do think we need to have specific and exact semantics for data; not “if it parses as a digest, it is a digest” (note that https://github.com/opencontainers/image-spec/blob/main/annotations.md clearly allows a digest-lookalike as the reference annotation) — separate fields in the “reference” object, with unambiguous semantics.
• (Due to relevance to image signing, we require the reference code to have very good unit test coverage)

So fully implementing this would be a somewhat larger effort than the diff above, but it’s a good idea and something that should happen.

[mwopfner]

@mtrmac: Thanks for your feedback. I was pretty sure the patch is not a proper solution 😉 (especially the splitting of the reference into its parts). Here I also got a bit lost with the naming.

• In the source code the variable named reference actually contains path:reference as described in https://github.com/containers/image/blob/main/docs/containers-transports.5.md#ocipathreference. So the reference variable is not just the reference but the path plus the reference separated by :
• Is it also correct that reference in oic:path:reference must have the format of org.opencontainers.image.ref.name or is it path:reference which must have the format? (https://github.com/opencontainers/image-spec/blob/main/annotations.md)
• I think adding a @digest notation in generally would work oci:path[:{reference|@source-index}|@digest] when we split only on the first @ or :. I think if a proper regex is used this should be possible (something like https://regex101.com/r/oDjEDX/4 for the last path element). This of course would prevent the use of @ in the last path element. But I don't think that is a large problem because registries like quay.io also do not allow it.

If I can help implementing this feature in any way please let me know.

[mtrmac]

The “reference” naming is an unfortunate consequence of several layers of indirection/abstraction. ociReference is an instance of a “c/image image reference”, and it refers to any single image (text form oci:…). That has up to three parts:

• oci:
• a path
• the org.opencontainers.image.ref.name annotation.

… and the last part is, by the OCI spec, described as “name of the reference”, let’s call that “OCI reference”. Hence the man page names that part reference, meaning OCI reference; OTOH the man page does not use the “reference” word to refer to ”c/image image reference”; it usually says “image name”.

So, the org.opencontainers.image.ref.name syntax applies to the third part of the image reference (the field ociReference.image)

---

We can’t use oci:path@digest because right now the @digest part (or at least the prefix up to the : in @sha256:) is interpreted as _path. E.g. oci:/foo@sha256:abc… currently refers to file /foo@sha256, with the …ref.name annotation value abc….

The path refers to a local filesystem, so quay.io restrictions on repository names are not relevant.

Sure, @ in path names is somewhat unusual, but we are already in enough trouble :) for making it impossible to use : in the path part.

---

WRT the way to implement this, most of the work will need to happen in the https://github.com/containers/image repository ; we can discuss both the detailed syntax and implementation there.

Similarly, the containers/common part would be a PR in that repository; and both will flow into Podman.

I have, at least, filed containers/image#1828 , so that this is tracked in the relevant repo.

[mwopfner]

@mtrmac Thanks for openning containers/image#1828. I will add the proposal details there.

Regarding the use of oci:path@digest

• Yes, it is exactly as you described it and it is correct, that oci is on the filesystem so registry restrictions do not apply.
• It is more like: should we keep the syntax between registries and oci the same or have different ones and does anyone currently use @. (man podman-pull tells us podman pull [options] [transport]name[:tag|@digest] which suggests, on a very high level, that i can use @digest with oci).
• I'm also fine with another syntax like :@.

[github-actions]

A friendly reminder that this issue had no activity for 30 days.

[rhatdan]

@mtrmac I think this is waiting for a response from you?

[mtrmac]

@rhatdan This will happen in c/image, via the tracked RFE, when/if it is implemented there.

[github-actions]

A friendly reminder that this issue had no activity for 30 days.