support tag@digest notation

For the sake of Docker compatibility, support the tag@digest notation. In that case, the tag is stripped off the reference and the digest is the sole source of truth. Add a number of tests to make sure we're behaving as expected. Context: containers/podman/issues/6721 Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
containers · May 26, 2021 · 599da0c · 599da0c
1 parent ea4f702
commit 599da0c
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 8 deletions.
diff --git a/libimage/normalize.go b/libimage/normalize.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // NormalizeName normalizes the provided name according to the conventions by
@@ -40,6 +41,11 @@ func NormalizeName(name string) (reference.Named, error) {
 	}
 
 	if _, hasTag := named.(reference.NamedTagged); hasTag {
+		// Strip off the tag of a tagged and digested reference.
+		named, err = normalizeTaggedDigestedNamed(named)
+		if err != nil {
+			return nil, err
+		}
 		return named, nil
 	}
 	if _, hasDigest := named.(reference.Digested); hasDigest {
@@ -90,3 +96,48 @@ func ToNameTagPairs(repoTags []reference.Named) ([]NameTagPair, error) {
 	}
 	return pairs, nil
 }
+
+// normalizeTaggedDigestedString strips the tag off the specified string iff it
+// is tagged and digested. Note that the tag is entirely ignored to match
+// Docker behavior.
+func normalizeTaggedDigestedString(s string) (string, error) {
+	// Note that the input string is not expected to be parseable, so we
+	// return it verbatim in error cases.
+	ref, err := reference.Parse(s)
+	if err != nil {
+		return "", err
+	}
+	named, ok := ref.(reference.Named)
+	if !ok {
+		return s, nil
+	}
+	named, err = normalizeTaggedDigestedNamed(named)
+	if err != nil {
+		return "", err
+	}
+	return named.String(), nil
+}
+
+// normalizeTaggedDigestedNamed strips the tag off the specified named
+// reference iff it is tagged and digested. Note that the tag is entirely
+// ignored to match Docker behavior.
+func normalizeTaggedDigestedNamed(named reference.Named) (reference.Named, error) {
+	_, isTagged := named.(reference.NamedTagged)
+	if !isTagged {
+		return named, nil
+	}
+	digested, isDigested := named.(reference.Digested)
+	if !isDigested {
+		return named, nil
+	}
+
+	// Now strip off the tag.
+	newNamed := reference.TrimNamed(named)
+	// And re-add the digest.
+	newNamed, err := reference.WithDigest(newNamed, digested.Digest())
+	if err != nil {
+		return named, err
+	}
+	logrus.Debugf("Stripped off tag from tagged and digested reference %q", named.String())
+	return newNamed, nil
+}
diff --git a/libimage/normalize_test.go b/libimage/normalize_test.go
@@ -12,14 +12,15 @@ func TestNormalizeName(t *testing.T) {
 
 	for _, c := range []struct{ input, expected string }{
 		{"#", ""}, // Clearly invalid
-		{"example.com/busybox", "example.com/busybox:latest"},                                            // Qualified name-only
-		{"example.com/busybox:notlatest", "example.com/busybox:notlatest"},                               // Qualified name:tag
-		{"example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix},                     // Qualified name@digest; FIXME? Should we allow tagging with a digest at all?
-		{"example.com/busybox:notlatest" + digestSuffix, "example.com/busybox:notlatest" + digestSuffix}, // Qualified name:tag@digest
-		{"busybox:latest", "localhost/busybox:latest"},                                                   // Unqualified name-only
-		{"localhost/busybox", "localhost/busybox:latest"},                                                // Qualified with localhost
-		{"ns/busybox:latest", "localhost/ns/busybox:latest"},                                             // Unqualified with a dot-less namespace
-		{"docker.io/busybox:latest", "docker.io/library/busybox:latest"},                                 // docker.io without /library/
+		{"example.com/busybox", "example.com/busybox:latest"},                                  // Qualified name-only
+		{"example.com/busybox:notlatest", "example.com/busybox:notlatest"},                     // Qualified name:tag
+		{"example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix},           // Qualified name@digest
+		{"example.com/busybox:notlatest" + digestSuffix, "example.com/busybox" + digestSuffix}, // Qualified name:tag@digest
+		{"busybox:latest", "localhost/busybox:latest"},                                         // Unqualified name-only
+		{"busybox:latest" + digestSuffix, "localhost/busybox" + digestSuffix},                  // Unqualified name:tag@digest
+		{"localhost/busybox", "localhost/busybox:latest"},                                      // Qualified with localhost
+		{"ns/busybox:latest", "localhost/ns/busybox:latest"},                                   // Unqualified with a dot-less namespace
+		{"docker.io/busybox:latest", "docker.io/library/busybox:latest"},                       // docker.io without /library/
 	} {
 		res, err := NormalizeName(c.input)
 		if c.expected == "" {
@@ -30,3 +31,30 @@ func TestNormalizeName(t *testing.T) {
 		}
 	}
 }
+
+func TestNormalizeTaggedDigestedString(t *testing.T) {
+	const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+
+	for _, test := range []struct{ input, expected string }{
+		{"$$garbage", ""},
+		{"fedora", "fedora"},
+		{"fedora:tag", "fedora:tag"},
+		{digestSuffix, ""},
+		{"docker://fedora:latest", ""},
+		{"docker://fedora:latest" + digestSuffix, ""},
+		{"fedora" + digestSuffix, "fedora" + digestSuffix},
+		{"fedora:latest" + digestSuffix, "fedora" + digestSuffix},
+		{"repo/fedora:123456" + digestSuffix, "repo/fedora" + digestSuffix},
+		{"quay.io/repo/fedora:tag" + digestSuffix, "quay.io/repo/fedora" + digestSuffix},
+		{"localhost/fedora:anothertag" + digestSuffix, "localhost/fedora" + digestSuffix},
+		{"localhost:5000/fedora:v1.2.3.4.5" + digestSuffix, "localhost:5000/fedora" + digestSuffix},
+	} {
+		res, err := normalizeTaggedDigestedString(test.input)
+		if test.expected == "" {
+			assert.Error(t, err, "%v", test)
+		} else {
+			assert.NoError(t, err, "%v", test)
+			assert.Equal(t, test.expected, res, "%v", test)
+		}
+	}
+}
diff --git a/libimage/pull.go b/libimage/pull.go
@@ -67,6 +67,15 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
 			return []*Image{local}, err
 		}
 
+		// Docker compat: strip off the tag iff name is tagged and digested
+		// (e.g., fedora:latest@sha256...).  In that case, the tag is stripped
+		// off and entirely ignored.  The digest is the sole source of truth.
+		normalizedName, normalizeError := normalizeTaggedDigestedString(name)
+		if normalizeError != nil {
+			return nil, normalizeError
+		}
+		name = normalizedName
+
 		// If the input does not include a transport assume it refers
 		// to a registry.
 		dockerRef, dockerErr := alltransports.ParseImageName("docker://" + name)

diff --git a/libimage/pull_test.go b/libimage/pull_test.go
@@ -42,6 +42,8 @@ func TestPull(t *testing.T) {
 		{"docker://alpine", false, 1, []string{"docker.io/library/alpine:latest"}},
 		{"docker.io/library/alpine", false, 1, []string{"docker.io/library/alpine:latest"}},
 		{"docker://docker.io/library/alpine", false, 1, []string{"docker.io/library/alpine:latest"}},
+		{"quay.io/libpod/alpine@sha256:634a8f35b5f16dcf4aaa0822adc0b1964bb786fca12f6831de8ddc45e5986a00", false, 1, []string{"quay.io/libpod/alpine@sha256:634a8f35b5f16dcf4aaa0822adc0b1964bb786fca12f6831de8ddc45e5986a00"}},
+		{"quay.io/libpod/alpine:pleaseignorethistag@sha256:634a8f35b5f16dcf4aaa0822adc0b1964bb786fca12f6831de8ddc45e5986a00", false, 1, []string{"quay.io/libpod/alpine@sha256:634a8f35b5f16dcf4aaa0822adc0b1964bb786fca12f6831de8ddc45e5986a00"}},
 	} {
 		pulledImages, err := runtime.Pull(ctx, test.input, config.PullPolicyAlways, pullOptions)
 		if test.expectError {

diff --git a/libimage/runtime.go b/libimage/runtime.go
@@ -180,6 +180,15 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image,
 		}
 		logrus.Debugf("Found image %q in local containers storage (%s)", name, storageRef.StringWithinTransport())
 		return r.storageToImage(img, storageRef), "", nil
+	} else {
+		// Docker compat: strip off the tag iff name is tagged and digested
+		// (e.g., fedora:latest@sha256...).  In that case, the tag is stripped
+		// off and entirely ignored.  The digest is the sole source of truth.
+		normalizedName, err := normalizeTaggedDigestedString(name)
+		if err != nil {
+			return nil, "", err
+		}
+		name = normalizedName
 	}
 
 	originalName := name