From 5319679106c07a02d14c6e2e2ab3cb47f8a3f85f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 28 Sep 2020 15:00:27 +0200 Subject: [PATCH] image look up: consult registries.conf When looking up local images, take the unqualified-serach registries of the registries.conf into account (on top of "localhost/"). Also extend the integration tests to prevent future regressions. Fixes: #6381 Signed-off-by: Valentin Rothberg --- libpod/image/image.go | 72 ++++++++++++++++++++++++------ test/e2e/pull_test.go | 100 +++++++++++++++++++++++++++++++++++++++++- test/registries.conf | 1 + 3 files changed, 157 insertions(+), 16 deletions(-) diff --git a/libpod/image/image.go b/libpod/image/image.go index 5dfb33afbe95..f5bf47694271 100644 --- a/libpod/image/image.go +++ b/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 diff --git a/test/e2e/pull_test.go b/test/e2e/pull_test.go index 2280d16cc578..edc17fdbfbe1 100644 --- a/test/e2e/pull_test.go +++ b/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)) + } + }) }) diff --git a/test/registries.conf b/test/registries.conf index bb7072d45d3d..f27a282d67a5 100644 --- a/test/registries.conf +++ b/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']