Merge pull request #10486 from vrothberg/tag-digest-support

support tag@digest notation

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/containernetworking/cni v0.8.1
 	github.com/containernetworking/plugins v0.9.1
 	github.com/containers/buildah v1.21.0
-	github.com/containers/common v0.39.0
+	github.com/containers/common v0.39.1-0.20210527140106-e5800a20386a
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/image/v5 v5.12.0
 	github.com/containers/ocicrypt v1.1.1

go.sum

@@ -219,8 +219,8 @@ github.com/containernetworking/plugins v0.9.1/go.mod h1:xP/idU2ldlzN6m4p5LmGiwRD
 github.com/containers/buildah v1.21.0 h1:LuwuqRPjan3X3AIdGwfkEkqMgmrDMNpQznFqNdHgCz8=
 github.com/containers/buildah v1.21.0/go.mod h1:yPdlpVd93T+i91yGxrJbW1YOWrqN64j5ZhHOZmHUejs=
 github.com/containers/common v0.38.4/go.mod h1:egfpX/Y3+19Dz4Wa1eRZDdgzoEOeneieF9CQppKzLBg=
-github.com/containers/common v0.39.0 h1:MrvpFa/bM4UmUILACv2IhOif4oLmWAiD4C+CpOc/MUo=
-github.com/containers/common v0.39.0/go.mod h1:vPUHCg/dHoiyqIyLN+EdbjUaGrVEhs/hAvsqsxuYepk=
+github.com/containers/common v0.39.1-0.20210527140106-e5800a20386a h1:XzYOUf7qjgVJ59YGqAzehlbT63EgjUJhMnfhsPSSJV0=
+github.com/containers/common v0.39.1-0.20210527140106-e5800a20386a/go.mod h1:CxHAf4iQOZZ8nASIjMdYHHRyA8dMR4tINSS7WQWlv90=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/image/v5 v5.12.0 h1:1hNS2QkzFQ4lH3GYQLyAXB0acRMhS1Ubm6oV++8vw4w=

pkg/api/handlers/compat/containers_create.go

@@ -71,13 +71,12 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 	imgNameOrID := newImage.ID()
 	// if the img had multi names with the same sha256 ID, should use the InputName, not the ID
 	if len(newImage.Names()) > 1 {
-		imageRef, err := utils.ParseDockerReference(resolvedName)
-		if err != nil {
+		if err := utils.IsRegistryReference(resolvedName); err != nil {
 			utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err)
 			return
 		}
 		// maybe the InputName has no tag, so use full name to display
-		imgNameOrID = imageRef.DockerReference().String()
+		imgNameOrID = resolvedName
 	}
 
 	sg := specgen.NewSpecGenerator(imgNameOrID, cliOpts.RootFS)

pkg/api/handlers/libpod/images.go

@@ -482,7 +482,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
 		destination = source
 	}
 
-	if _, err := utils.ParseDockerReference(destination); err != nil {
+	if err := utils.IsRegistryReference(destination); err != nil {
 		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err)
 		return
 	}

pkg/api/handlers/libpod/images_pull.go

@@ -48,7 +48,7 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Make sure that the reference has no transport or the docker one.
-	if _, err := utils.ParseDockerReference(query.Reference); err != nil {
+	if err := utils.IsRegistryReference(query.Reference); err != nil {
 		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err)
 		return
 	}

pkg/api/handlers/libpod/manifests.go

@@ -169,7 +169,7 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) {
 			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
 		return
 	}
-	if _, err := utils.ParseDockerReference(query.Destination); err != nil {
+	if err := utils.IsRegistryReference(query.Destination); err != nil {
 		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err)
 		return
 	}

pkg/api/handlers/utils/images.go

@@ -15,22 +15,19 @@ import (
 	"github.com/pkg/errors"
 )
 
-// ParseDockerReference parses the specified image name to a
-// `types.ImageReference` and enforces it to refer to a docker-transport
-// reference.
-func ParseDockerReference(name string) (types.ImageReference, error) {
-	dockerPrefix := fmt.Sprintf("%s://", docker.Transport.Name())
+// IsRegistryReference checks if the specified name points to the "docker://"
+// transport.  If it points to no supported transport, we'll assume a
+// non-transport reference pointing to an image (e.g., "fedora:latest").
+func IsRegistryReference(name string) error {
 	imageRef, err := alltransports.ParseImageName(name)
-	if err == nil && imageRef.Transport().Name() != docker.Transport.Name() {
-		return nil, errors.Errorf("reference %q must be a docker reference", name)
-	} else if err != nil {
-		origErr := err
-		imageRef, err = alltransports.ParseImageName(fmt.Sprintf("%s%s", dockerPrefix, name))
-		if err != nil {
-			return nil, errors.Wrapf(origErr, "reference %q must be a docker reference", name)
-		}
+	if err != nil {
+		// No supported transport -> assume a docker-stype reference.
+		return nil
 	}
-	return imageRef, nil
+	if imageRef.Transport().Name() == docker.Transport.Name() {
+		return nil
+	}
+	return errors.Errorf("unsupport transport %s in %q: only docker transport is supported", imageRef.Transport().Name(), name)
 }
 
 // ParseStorageReference parses the specified image name to a

test/e2e/play_kube_test.go

@@ -2119,7 +2119,7 @@ MemoryReservation: {{ .HostConfig.MemoryReservation }}`})
 		kube := podmanTest.Podman([]string{"play", "kube", kubeYaml})
 		kube.WaitWithDefaultTimeout()
 		Expect(kube.ExitCode()).To(Equal(125))
-		Expect(kube.ErrorToString()).To(ContainSubstring(invalidImageName))
+		Expect(kube.ErrorToString()).To(ContainSubstring("invalid reference format"))
 	})
 
 	It("podman play kube applies log driver to containers", func() {

test/system/001-basic.bats

@@ -49,6 +49,14 @@ function setup() {
 
 @test "podman can pull an image" {
     run_podman pull $IMAGE
+
+    # Also make sure that the tag@digest syntax is supported.
+    run_podman inspect --format "{{ .Digest }}" $IMAGE
+    digest=$output
+    run_podman pull $IMAGE@$digest
+
+    # Now untag the digest reference again.
+    run_podman untag $IMAGE $IMAGE@$digest
 }
 
 # PR #7212: allow --remote anywhere before subcommand, not just as 1st flag

vendor/github.com/containers/common/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
+}

vendor/github.com/containers/common/libimage/pull.go

@@ -52,6 +52,7 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
 		options = &PullOptions{}
 	}
 
+	var possiblyUnqualifiedName string // used for short-name resolution
 	ref, err := alltransports.ParseImageName(name)
 	if err != nil {
 		// If the image clearly refers to a local one, we can look it up directly.
@@ -67,6 +68,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)
@@ -74,6 +84,17 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
 			return nil, err
 		}
 		ref = dockerRef
+		possiblyUnqualifiedName = name
+	} else if ref.Transport().Name() == registryTransport.Transport.Name() {
+		// Normalize the input if we're referring to the docker
+		// transport directly. That makes sure that a `docker://fedora`
+		// will resolve directly to `docker.io/library/fedora:latest`
+		// and not be subject to short-name resolution.
+		named := ref.DockerReference()
+		if named == nil {
+			return nil, errors.New("internal error: unexpected nil reference")
+		}
+		possiblyUnqualifiedName = named.String()
 	}
 
 	if options.AllTags && ref.Transport().Name() != registryTransport.Transport.Name() {
@@ -94,7 +115,7 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
 
 	// DOCKER REGISTRY
 	case registryTransport.Transport.Name():
-		pulledImages, pullError = r.copyFromRegistry(ctx, ref, strings.TrimPrefix(name, "docker://"), pullPolicy, options)
+		pulledImages, pullError = r.copyFromRegistry(ctx, ref, possiblyUnqualifiedName, pullPolicy, options)
 
 	// DOCKER ARCHIVE
 	case dockerArchiveTransport.Transport.Name():

vendor/github.com/containers/common/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

vendor/github.com/containers/common/pkg/config/default.go

@@ -195,7 +195,7 @@ func DefaultConfig() (*Config, error) {
 			Init:               false,
 			InitPath:           "",
 			IPCNS:              "private",
-			LogDriver:          DefaultLogDriver,
+			LogDriver:          defaultLogDriver(),
 			LogSizeMax:         DefaultLogSizeMax,
 			NetNS:              netns,
 			NoHosts:            false,

vendor/github.com/containers/common/pkg/config/nosystemd.go

@@ -3,9 +3,17 @@
 package config
 
 func defaultCgroupManager() string {
-	return "cgroupfs"
+	return CgroupfsCgroupsManager
 }
 
 func defaultEventsLogger() string {
 	return "file"
 }
+
+func defaultLogDriver() string {
+	return DefaultLogDriver
+}
+
+func useSystemd() bool {
+	return false
+}

vendor/github.com/containers/common/pkg/config/systemd.go

@@ -3,11 +3,23 @@
 package config
 
 import (
+	"io/ioutil"
+	"strings"
+	"sync"
+
 	"github.com/containers/common/pkg/cgroupv2"
 	"github.com/containers/storage/pkg/unshare"
 )
 
+var (
+	systemdOnce sync.Once
+	usesSystemd bool
+)
+
 func defaultCgroupManager() string {
+	if !useSystemd() {
+		return CgroupfsCgroupsManager
+	}
 	enabled, err := cgroupv2.Enabled()
 	if err == nil && !enabled && unshare.IsRootless() {
 		return CgroupfsCgroupsManager
@@ -15,6 +27,32 @@ func defaultCgroupManager() string {
 
 	return SystemdCgroupsManager
 }
+
 func defaultEventsLogger() string {
-	return "journald"
+	if useSystemd() {
+		return "journald"
+	}
+	return "file"
+}
+
+func defaultLogDriver() string {
+	// If we decide to change the default for logdriver, it should be done here.
+	if useSystemd() {
+		return DefaultLogDriver
+	}
+
+	return DefaultLogDriver
+
+}
+
+func useSystemd() bool {
+	systemdOnce.Do(func() {
+		dat, err := ioutil.ReadFile("/proc/1/comm")
+		if err == nil {
+			val := strings.TrimSuffix(string(dat), "\n")
+			usesSystemd = (val == "systemd")
+		}
+		return
+	})
+	return usesSystemd
 }

vendor/github.com/containers/common/version/version.go

@@ -1,4 +1,4 @@
 package version
 
 // Version is the version of the build.
-const Version = "0.39.0"
+const Version = "0.39.1-dev"

vendor/modules.txt

@@ -91,7 +91,7 @@ github.com/containers/buildah/pkg/overlay
 github.com/containers/buildah/pkg/parse
 github.com/containers/buildah/pkg/rusage
 github.com/containers/buildah/util
-# github.com/containers/common v0.39.0
+# github.com/containers/common v0.39.1-0.20210527140106-e5800a20386a
 github.com/containers/common/libimage
 github.com/containers/common/libimage/manifests
 github.com/containers/common/pkg/apparmor