From d61bed2b2dcca27d8baecc2b63255f9df456ba2f Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 27 Jul 2018 01:16:00 +0200 Subject: Split refNamesFromImageReference from Runtime.getPullListFromRef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Again, that makes the core logic independent from Runtime == containers-storage, and easier to test independently. So, this also adds tests. Should not change behavior. Signed-off-by: Miloslav Trmač Closes: #1176 Approved by: rhatdan --- libpod/image/pull.go | 15 ++- libpod/image/pull_test.go | 109 ++++++++++++++++++++++ libpod/image/testdata/docker-name-only.tar.xz | Bin 0 -> 1024 bytes libpod/image/testdata/docker-registry-name.tar.xz | Bin 0 -> 1028 bytes libpod/image/testdata/docker-two-images.tar.xz | Bin 0 -> 1416 bytes libpod/image/testdata/docker-two-names.tar.xz | Bin 0 -> 1040 bytes libpod/image/testdata/docker-unnamed.tar.xz | Bin 0 -> 968 bytes libpod/image/testdata/oci-name-only.tar.gz | Bin 0 -> 975 bytes libpod/image/testdata/oci-non-docker-name.tar.gz | Bin 0 -> 991 bytes libpod/image/testdata/oci-registry-name.tar.gz | Bin 0 -> 979 bytes libpod/image/testdata/oci-unnamed.tar.gz | Bin 0 -> 928 bytes 11 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 libpod/image/testdata/docker-name-only.tar.xz create mode 100644 libpod/image/testdata/docker-registry-name.tar.xz create mode 100644 libpod/image/testdata/docker-two-images.tar.xz create mode 100644 libpod/image/testdata/docker-two-names.tar.xz create mode 100644 libpod/image/testdata/docker-unnamed.tar.xz create mode 100644 libpod/image/testdata/oci-name-only.tar.gz create mode 100644 libpod/image/testdata/oci-non-docker-name.tar.gz create mode 100644 libpod/image/testdata/oci-registry-name.tar.gz create mode 100644 libpod/image/testdata/oci-unnamed.tar.gz (limited to 'libpod') diff --git a/libpod/image/pull.go b/libpod/image/pull.go index f7131f11e..bbabba7b9 100644 --- a/libpod/image/pull.go +++ b/libpod/image/pull.go @@ -84,8 +84,8 @@ func getPullRefName(srcRef types.ImageReference, destName string) (*pullRefName, }, nil } -// returns a list of pullRefPair with the srcRef and DstRef based on the transport being used -func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageReference, imgName string, sc *types.SystemContext) ([]*pullRefPair, error) { +// refNamesFromImageReference returns a list of pullRefName for a single ImageReference, depending on the used transport. +func refNamesFromImageReference(ctx context.Context, srcRef types.ImageReference, imgName string, sc *types.SystemContext) ([]*pullRefName, error) { var pullNames []*pullRefName splitArr := strings.Split(imgName, ":") archFile := splitArr[len(splitArr)-1] @@ -178,8 +178,17 @@ func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageRef } pullNames = append(pullNames, pullInfo) } + return pullNames, nil +} + +// getPullListFromRef returns a list of pullRefPair for a single ImageReference, depending on the used transport. +func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageReference, imgName string, sc *types.SystemContext) ([]*pullRefPair, error) { + refNames, err := refNamesFromImageReference(ctx, srcRef, imgName, sc) + if err != nil { + return nil, err + } - return ir.pullRefPairsFromRefNames(pullNames) + return ir.pullRefPairsFromRefNames(refNames) } // pullImage pulls an image from configured registries diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go index e5e9f6b91..9305527d4 100644 --- a/libpod/image/pull_test.go +++ b/libpod/image/pull_test.go @@ -1,6 +1,7 @@ package image import ( + "context" "fmt" "io/ioutil" "os" @@ -78,6 +79,114 @@ func TestGetPullRefName(t *testing.T) { } } +func TestRefNamesFromImageReference(t *testing.T) { + type expected struct{ image, dstName string } + for _, c := range []struct { + srcName string + expected []expected + }{ + // == docker-archive: + {"docker-archive:/dev/this-does-not-exist", nil}, // Input does not exist. + {"docker-archive:/dev/null", nil}, // Input exists but does not contain a manifest. + // FIXME: The implementation has extra code for len(manifest) == 0?! That will fail in getImageDigest anyway. + { // RepoTags is empty + "docker-archive:testdata/docker-unnamed.tar.xz", + []expected{{"@ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951", "@ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951"}}, + }, + { // RepoTags is a [docker.io/library/]name:latest, normalized to the short format. + "docker-archive:testdata/docker-name-only.tar.xz", + []expected{{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}}, + }, + { // RepoTags is a registry/name:latest + "docker-archive:testdata/docker-registry-name.tar.xz", + []expected{{"example.com/empty:latest", "example.com/empty:latest"}}, + }, + { // RepoTags has multiple items for a single image + "docker-archive:testdata/docker-two-names.tar.xz", + []expected{ + {"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}, + {"example.com/empty:latest", "example.com/empty:latest"}, + }, + }, + { // FIXME: Two images in a single archive - only the "first" one (whichever it is) is returned + // (and docker-archive: then refuses to read anything when the manifest has more than 1 item) + "docker-archive:testdata/docker-two-images.tar.xz", + []expected{{"example.com/empty:latest", "example.com/empty:latest"}}, + // "example.com/empty/but:different" exists but is ignored + }, + + // == oci-archive: + {"oci-archive:/dev/this-does-not-exist", nil}, // Input does not exist. + {"oci-archive:/dev/null", nil}, // Input exists but does not contain a manifest. + // FIXME: The remaining tests are commented out for now, because oci-archive: does not work unprivileged. + // { // No name annotation + // "oci-archive:testdata/oci-unnamed.tar.gz", + // []expected{{"@5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6", "@5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6"}}, + // }, + // { // Name is a name:latest (no normalization is defined). + // "oci-archive:testdata/oci-name-only.tar.gz", + // []expected{{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}}, + // }, + // { // Name is a registry/name:latest + // "oci-archive:testdata/oci-registry-name.tar.gz", + // []expected{{"example.com/empty:latest", "example.com/empty:latest"}}, + // }, + // { // Name exists, but is an invalid Docker reference; such names are passed through, and will fail when intepreting dstName. + // "oci-archive:testdata/oci-non-docker-name.tar.gz", + // []expected{{"UPPERCASE-IS-INVALID", "UPPERCASE-IS-INVALID"}}, + // }, + // Maybe test support of two images in a single archive? It should be transparently handled by adding a reference to srcRef. + + // == dir: + { // Absolute path + "dir:/dev/this-does-not-exist", + []expected{{"localhost/dev/this-does-not-exist", "localhost/dev/this-does-not-exist"}}, + }, + { // Relative path, single element. + // FIXME? Note the :latest difference in .image. (In .dstName as well, but it has the same semantics in there.) + "dir:this-does-not-exist", + []expected{{"localhost/this-does-not-exist:latest", "localhost/this-does-not-exist:latest"}}, + }, + { // Relative path, multiple elements. + // FIXME: This does not add localhost/, and dstName is parsed as docker.io/testdata. + "dir:testdata/this-does-not-exist", + []expected{{"testdata/this-does-not-exist", "testdata/this-does-not-exist"}}, + }, + + // == Others, notably: + // === docker:// (has ImageReference.DockerReference) + { // Fully-specified input + "docker://docker.io/library/busybox:latest", + []expected{{"docker://docker.io/library/busybox:latest", "docker.io/library/busybox:latest"}}, + }, + { // Minimal form of the input + "docker://busybox", + []expected{{"docker://busybox", "docker.io/library/busybox:latest"}}, + }, + + // === tarball: (as an example of what happens when ImageReference.DockerReference is nil). + { // FIXME? The dstName value is invalid, and will fail. + // (This is NOT an API promise that the results will continue to be this way.) + "tarball:/dev/null", + []expected{{"tarball:/dev/null", "tarball:/dev/null"}}, + }, + } { + srcRef, err := alltransports.ParseImageName(c.srcName) + require.NoError(t, err, c.srcName) + + res, err := refNamesFromImageReference(context.Background(), srcRef, c.srcName, nil) + if len(c.expected) == 0 { + assert.Error(t, err, c.srcName) + } else { + require.NoError(t, err, c.srcName) + require.Len(t, res, len(c.expected), c.srcName) + for i, e := range c.expected { + assert.Equal(t, &pullRefName{image: e.image, srcRef: srcRef, dstName: e.dstName}, res[i], fmt.Sprintf("%s #%d", c.srcName, i)) + } + } + } +} + const registriesConfWithSearch = `[registries.search] registries = ['example.com', 'docker.io'] ` diff --git a/libpod/image/testdata/docker-name-only.tar.xz b/libpod/image/testdata/docker-name-only.tar.xz new file mode 100644 index 000000000..0cad9f108 Binary files /dev/null and b/libpod/image/testdata/docker-name-only.tar.xz differ diff --git a/libpod/image/testdata/docker-registry-name.tar.xz b/libpod/image/testdata/docker-registry-name.tar.xz new file mode 100644 index 000000000..181816c2e Binary files /dev/null and b/libpod/image/testdata/docker-registry-name.tar.xz differ diff --git a/libpod/image/testdata/docker-two-images.tar.xz b/libpod/image/testdata/docker-two-images.tar.xz new file mode 100644 index 000000000..148d8a86b Binary files /dev/null and b/libpod/image/testdata/docker-two-images.tar.xz differ diff --git a/libpod/image/testdata/docker-two-names.tar.xz b/libpod/image/testdata/docker-two-names.tar.xz new file mode 100644 index 000000000..07fbc479c Binary files /dev/null and b/libpod/image/testdata/docker-two-names.tar.xz differ diff --git a/libpod/image/testdata/docker-unnamed.tar.xz b/libpod/image/testdata/docker-unnamed.tar.xz new file mode 100644 index 000000000..ba6ea1bae Binary files /dev/null and b/libpod/image/testdata/docker-unnamed.tar.xz differ diff --git a/libpod/image/testdata/oci-name-only.tar.gz b/libpod/image/testdata/oci-name-only.tar.gz new file mode 100644 index 000000000..57bc07564 Binary files /dev/null and b/libpod/image/testdata/oci-name-only.tar.gz differ diff --git a/libpod/image/testdata/oci-non-docker-name.tar.gz b/libpod/image/testdata/oci-non-docker-name.tar.gz new file mode 100644 index 000000000..5ffc0eabd Binary files /dev/null and b/libpod/image/testdata/oci-non-docker-name.tar.gz differ diff --git a/libpod/image/testdata/oci-registry-name.tar.gz b/libpod/image/testdata/oci-registry-name.tar.gz new file mode 100644 index 000000000..e6df87339 Binary files /dev/null and b/libpod/image/testdata/oci-registry-name.tar.gz differ diff --git a/libpod/image/testdata/oci-unnamed.tar.gz b/libpod/image/testdata/oci-unnamed.tar.gz new file mode 100644 index 000000000..de445fdf8 Binary files /dev/null and b/libpod/image/testdata/oci-unnamed.tar.gz differ -- cgit v1.2.3-54-g00ecf