Merge pull request containers#7823 from vrothberg/fix-6381

image look up: consult registries.conf

libpod/image/image.go

@@ -410,48 +410,92 @@ func (ir *Runtime) getLocalImage(inputName string) (string, *storage.Image, erro
 	if inputName == "" {
 		return "", nil, errors.Errorf("input name is blank")
 	}
+
 	// Check if the input name has a transport and if so strip it
 	dest, err := alltransports.ParseImageName(inputName)
 	if err == nil && dest.DockerReference() != nil {
 		inputName = dest.DockerReference().String()
 	}
 
-	img, err := ir.getImage(stripSha256(inputName))
+	// Early check for fully-qualified images and (short) IDs.
+	img, err := ir.store.Image(stripSha256(inputName))
 	if err == nil {
-		return inputName, img, err
+		return inputName, img, nil
 	}
 
-	// container-storage wasn't able to find it in its current form
-	// check if the input name has a tag, and if not, run it through
-	// again
+	// Note that it's crucial to first decompose the image and check if
+	// it's a fully-qualified one or a "short name".  The latter requires
+	// some normalization with search registries and the
+	// "localhost/prefix".
 	decomposedImage, err := decompose(inputName)
 	if err != nil {
+		// We may have a storage reference.  We can't parse it to a
+		// reference before. Otherwise, we'd normalize "alpine" to
+		// "docker.io/library/alpine:latest" which would break the
+		// order in which we should query local images below.
+		if ref, err := is.Transport.ParseStoreReference(ir.store, inputName); err == nil {
+			img, err = is.Transport.GetStoreImage(ir.store, ref)
+			if err == nil {
+				return inputName, img, nil
+			}
+		}
 		return "", nil, err
 	}
 
-	// The image has a registry name in it and we made sure we looked for it locally
-	// with a tag.  It cannot be local.
+	// The specified image is fully qualified, so it doesn't exist in the
+	// storage.
 	if decomposedImage.hasRegistry {
+		// However ... we may still need to normalize to docker.io:
+		// `docker.io/foo` -> `docker.io/library/foo`
+		if ref, err := is.Transport.ParseStoreReference(ir.store, inputName); err == nil {
+			img, err = is.Transport.GetStoreImage(ir.store, ref)
+			if err == nil {
+				return inputName, img, nil
+			}
+		}
 		return "", nil, errors.Wrapf(ErrNoSuchImage, imageError)
 	}
-	// if the image is saved with the repository localhost, searching with localhost prepended is necessary
-	// We don't need to strip the sha because we have already determined it is not an ID
-	ref, err := decomposedImage.referenceWithRegistry(DefaultLocalRegistry)
+
+	// "Short-name image", so let's try out certain prefixes:
+	// 1) DefaultLocalRegistry (i.e., "localhost/)
+	// 2) Unqualified-search registries from registries.conf
+	unqualifiedSearchRegistries, err := registries.GetRegistries()
 	if err != nil {
 		return "", nil, err
 	}
-	img, err = ir.getImage(ref.String())
+
+	for _, candidate := range append([]string{DefaultLocalRegistry}, unqualifiedSearchRegistries...) {
+		ref, err := decomposedImage.referenceWithRegistry(candidate)
+		if err != nil {
+			return "", nil, err
+		}
+		img, err := ir.store.Image(ref.String())
+		if err == nil {
+			return ref.String(), img, nil
+		}
+	}
+
+	// Backwards compat: normalize to docker.io as some users may very well
+	// rely on that.
+	ref, err := is.Transport.ParseStoreReference(ir.store, inputName)
 	if err == nil {
-		return inputName, img, err
+		img, err = is.Transport.GetStoreImage(ir.store, ref)
+		if err == nil {
+			return inputName, img, nil
+		}
 	}
 
-	// grab all the local images
+	// Last resort: look at the repotags of all images and try to find a
+	// match.
 	images, err := ir.GetImages()
 	if err != nil {
 		return "", nil, err
 	}
 
-	// check the repotags of all images for a match
+	decomposedImage, err = decompose(inputName)
+	if err != nil {
+		return "", nil, err
+	}
 	repoImage, err := findImageInRepotags(decomposedImage, images)
 	if err == nil {
 		return inputName, repoImage, nil

pkg/registries/registries.go

@@ -1,5 +1,10 @@
 package registries
 
+// TODO: this package should not exist anymore.  Users should either use
+// c/image's `sysregistriesv2` package directly OR, even better, we cache a
+// config in libpod's image runtime so we don't need to parse the
+// registries.conf files redundantly.
+
 import (
 	"os"
 	"path/filepath"

test/e2e/pull_test.go

@@ -1,9 +1,8 @@
 package integration
 
 import (
-	"os"
-
 	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -400,4 +399,101 @@ var _ = Describe("Podman pull", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Not(Equal(0)))
 	})
+
+	It("podman pull + inspect from unqualified-search registry", func() {
+		// Regression test for #6381:
+		// Make sure that `pull shortname` and `inspect shortname`
+		// refer to the same image.
+
+		// We already tested pulling, so we can save some energy and
+		// just restore local artifacts and tag them.
+		podmanTest.RestoreArtifact(ALPINE)
+		podmanTest.RestoreArtifact(BB)
+
+		// What we want is at least two images which have the same name
+		// and are prefixed with two different unqualified-search
+		// registries from ../registries.conf.
+		//
+		// A `podman inspect $name` must yield the one from the _first_
+		// matching registry in the registries.conf.
+		getID := func(image string) string {
+			setup := podmanTest.PodmanNoCache([]string{"image", "inspect", image})
+			setup.WaitWithDefaultTimeout()
+			Expect(setup.ExitCode()).To(Equal(0))
+
+			data := setup.InspectImageJSON() // returns []inspect.ImageData
+			Expect(len(data)).To(Equal(1))
+			return data[0].ID
+		}
+
+		untag := func(image string) {
+			setup := podmanTest.PodmanNoCache([]string{"untag", image})
+			setup.WaitWithDefaultTimeout()
+			Expect(setup.ExitCode()).To(Equal(0))
+
+			setup = podmanTest.PodmanNoCache([]string{"image", "inspect", image})
+			setup.WaitWithDefaultTimeout()
+			Expect(setup.ExitCode()).To(Equal(0))
+
+			data := setup.InspectImageJSON() // returns []inspect.ImageData
+			Expect(len(data)).To(Equal(1))
+			Expect(len(data[0].RepoTags)).To(Equal(0))
+		}
+
+		tag := func(image, tag string) {
+			setup := podmanTest.PodmanNoCache([]string{"tag", image, tag})
+			setup.WaitWithDefaultTimeout()
+			Expect(setup.ExitCode()).To(Equal(0))
+			setup = podmanTest.PodmanNoCache([]string{"image", "exists", tag})
+			setup.WaitWithDefaultTimeout()
+			Expect(setup.ExitCode()).To(Equal(0))
+		}
+
+		image1 := getID(ALPINE)
+		image2 := getID(BB)
+
+		// $ head -n2 ../registries.conf
+		// [registries.search]
+		// registries = ['docker.io', 'quay.io', 'registry.fedoraproject.org']
+		registries := []string{"docker.io", "quay.io", "registry.fedoraproject.org"}
+		name := "foo/test:tag"
+		tests := []struct {
+			// tag1 has precedence (see list above) over tag2 when
+			// doing an inspect on "test:tag".
+			tag1, tag2 string
+		}{
+			{
+				fmt.Sprintf("%s/%s", registries[0], name),
+				fmt.Sprintf("%s/%s", registries[1], name),
+			},
+			{
+				fmt.Sprintf("%s/%s", registries[0], name),
+				fmt.Sprintf("%s/%s", registries[2], name),
+			},
+			{
+				fmt.Sprintf("%s/%s", registries[1], name),
+				fmt.Sprintf("%s/%s", registries[2], name),
+			},
+		}
+
+		for _, t := range tests {
+			// 1) untag both images
+			// 2) tag them according to `t`
+			// 3) make sure that an inspect of `name` returns `image1` with `tag1`
+			untag(image1)
+			untag(image2)
+			tag(image1, t.tag1)
+			tag(image2, t.tag2)
+
+			setup := podmanTest.PodmanNoCache([]string{"image", "inspect", name})
+			setup.WaitWithDefaultTimeout()
+			Expect(setup.ExitCode()).To(Equal(0))
+
+			data := setup.InspectImageJSON() // returns []inspect.ImageData
+			Expect(len(data)).To(Equal(1))
+			Expect(len(data[0].RepoTags)).To(Equal(1))
+			Expect(data[0].RepoTags[0]).To(Equal(t.tag1))
+			Expect(data[0].ID).To(Equal(image1))
+		}
+	})
 })

test/registries.conf

@@ -1,3 +1,4 @@
+# Note that changing the order here may break tests.
 [registries.search]
 registries = ['docker.io', 'quay.io', 'registry.fedoraproject.org']