From 0f7d54b0260c1be992ee3b9cee359ef3a9e8bd21 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 22 Apr 2021 08:01:12 +0200 Subject: migrate Podman to containers/common/libimage Migrate the Podman code base over to `common/libimage` which replaces `libpod/image` and a lot of glue code entirely. Note that I tried to leave bread crumbs for changed tests. Miscellaneous changes: * Some errors yield different messages which required to alter some tests. * I fixed some pre-existing issues in the code. Others were marked as `//TODO`s to prevent the PR from exploding. * The `NamesHistory` of an image is returned as is from the storage. Previously, we did some filtering which I think is undesirable. Instead we should return the data as stored in the storage. * Touched handlers use the ABI interfaces where possible. * Local image resolution: previously Podman would match "foo" on "myfoo". This behaviour has been changed and Podman will now only match on repository boundaries such that "foo" would match "my/foo" but not "myfoo". I consider the old behaviour to be a bug, at the very least an exotic corner case. * Futhermore, "foo:none" does *not* resolve to a local image "foo" without tag anymore. It's a hill I am (almost) willing to die on. * `image prune` prints the IDs of pruned images. Previously, in some cases, the names were printed instead. The API clearly states ID, so we should stick to it. * Compat endpoint image removal with _force_ deletes the entire not only the specified tag. Signed-off-by: Valentin Rothberg --- libpod/image/pull_test.go | 394 ---------------------------------------------- 1 file changed, 394 deletions(-) delete mode 100644 libpod/image/pull_test.go (limited to 'libpod/image/pull_test.go') diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go deleted file mode 100644 index d2930451c..000000000 --- a/libpod/image/pull_test.go +++ /dev/null @@ -1,394 +0,0 @@ -package image - -import ( - "context" - "fmt" - "io/ioutil" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/containers/image/v5/transports" - "github.com/containers/image/v5/transports/alltransports" - "github.com/containers/image/v5/types" - "github.com/containers/storage" - "github.com/containers/storage/pkg/idtools" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// newTestRuntime returns a *Runtime implementation and a cleanup function which the caller is expected to call. -func newTestRuntime(t *testing.T) (*Runtime, func()) { - wd, err := ioutil.TempDir("", "testStorageRuntime") - require.NoError(t, err) - err = os.MkdirAll(wd, 0700) - require.NoError(t, err) - - store, err := storage.GetStore(storage.StoreOptions{ - RunRoot: filepath.Join(wd, "run"), - GraphRoot: filepath.Join(wd, "root"), - GraphDriverName: "vfs", - GraphDriverOptions: []string{}, - UIDMap: []idtools.IDMap{{ - ContainerID: 0, - HostID: os.Getuid(), - Size: 1, - }}, - GIDMap: []idtools.IDMap{{ - ContainerID: 0, - HostID: os.Getgid(), - Size: 1, - }}, - }) - require.NoError(t, err) - - ir := NewImageRuntimeFromStore(store) - cleanup := func() { _ = os.RemoveAll(wd) } - return ir, cleanup -} - -// storageReferenceWithoutLocation returns ref.StringWithinTransport(), -// stripping the [store-specification] prefix from containers/image/storage reference format. -func storageReferenceWithoutLocation(ref types.ImageReference) string { - res := ref.StringWithinTransport() - if res[0] == '[' { - closeIndex := strings.IndexRune(res, ']') - if closeIndex > 0 { - res = res[closeIndex+1:] - } - } - return res -} - -func TestGetPullRefPair(t *testing.T) { - const imageID = "@0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - - ir, cleanup := newTestRuntime(t) - defer cleanup() - - for _, c := range []struct{ srcName, destName, expectedImage, expectedDstName string }{ - // == Source does not have a Docker reference (as is the case for docker-archive:, oci-archive, dir:); destination formats: - { // registry/name, no tag: - "dir:/dev/this-does-not-exist", "example.com/from-directory", - "example.com/from-directory", "example.com/from-directory:latest", - }, - { // name, no registry, no tag: - "dir:/dev/this-does-not-exist", "from-directory", - "localhost/from-directory", "localhost/from-directory:latest", - }, - { // registry/name:tag : - "dir:/dev/this-does-not-exist", "example.com/from-directory:notlatest", - "example.com/from-directory:notlatest", "example.com/from-directory:notlatest", - }, - { // name:tag, no registry: - "dir:/dev/this-does-not-exist", "from-directory:notlatest", - "localhost/from-directory:notlatest", "localhost/from-directory:notlatest", - }, - { // name@digest, no registry: - "dir:/dev/this-does-not-exist", "from-directory" + digestSuffix, - "localhost/from-directory" + digestSuffix, "localhost/from-directory" + digestSuffix, - }, - { // registry/name@digest: - "dir:/dev/this-does-not-exist", "example.com/from-directory" + digestSuffix, - "example.com/from-directory" + digestSuffix, "example.com/from-directory" + digestSuffix, - }, - { // ns/name:tag, no registry: - "dir:/dev/this-does-not-exist", "ns/from-directory:notlatest", - "localhost/ns/from-directory:notlatest", "localhost/ns/from-directory:notlatest", - }, - { // containers-storage image ID - "dir:/dev/this-does-not-exist", imageID, - imageID, imageID, - }, - // == Source does have a Docker reference. - // In that case getPullListFromRef uses the full transport:name input as a destName, - // which would be invalid in the returned dstName - but dstName is derived from the source, so it does not really matter _so_ much. - // Note that unlike real-world use we use different :source and :destination to verify the data flow in more detail. - { // registry/name:tag - "docker://example.com/busybox:source", "docker://example.com/busybox:destination", - "docker://example.com/busybox:destination", "example.com/busybox:source", - }, - { // Implied docker.io/library and :latest - "docker://busybox", "docker://busybox:destination", - "docker://busybox:destination", "docker.io/library/busybox:latest", - }, - // == Invalid destination format. - {"tarball:/dev/null", "tarball:/dev/null", "", ""}, - } { - testDescription := fmt.Sprintf("%#v %#v", c.srcName, c.destName) - srcRef, err := alltransports.ParseImageName(c.srcName) - require.NoError(t, err, testDescription) - - res, err := ir.getPullRefPair(srcRef, c.destName) - if c.expectedDstName == "" { - assert.Error(t, err, testDescription) - } else { - require.NoError(t, err, testDescription) - assert.Equal(t, c.expectedImage, res.image, testDescription) - assert.Equal(t, srcRef, res.srcRef, testDescription) - assert.Equal(t, c.expectedDstName, storageReferenceWithoutLocation(res.dstRef), testDescription) - } - } -} - -func TestPullGoalFromImageReference(t *testing.T) { - ir, cleanup := newTestRuntime(t) - defer cleanup() - - type expected struct{ image, dstName string } - for _, c := range []struct { - srcName string - expected []expected - expectedPullAllPairs bool - }{ - // == docker-archive: - {"docker-archive:/dev/this-does-not-exist", nil, false}, // Input does not exist. - {"docker-archive:/dev/null", nil, false}, // 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"}}, - true, - }, - { // 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"}}, - true, - }, - { // RepoTags is a registry/name:latest - "docker-archive:testdata/docker-registry-name.tar.xz", - []expected{{"example.com/empty:latest", "example.com/empty:latest"}}, - true, - }, - { // 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"}, - }, - true, - }, - { // Reference image by name in multi-image archive - "docker-archive:testdata/docker-two-images.tar.xz:example.com/empty:latest", - []expected{ - {"example.com/empty:latest", "example.com/empty:latest"}, - }, - true, - }, - { // Reference image by name in multi-image archive - "docker-archive:testdata/docker-two-images.tar.xz:example.com/empty/but:different", - []expected{ - {"example.com/empty/but:different", "example.com/empty/but:different"}, - }, - true, - }, - { // Reference image by index in multi-image archive - "docker-archive:testdata/docker-two-images.tar.xz:@0", - []expected{ - {"example.com/empty:latest", "example.com/empty:latest"}, - }, - true, - }, - { // Reference image by index in multi-image archive - "docker-archive:testdata/docker-two-images.tar.xz:@1", - []expected{ - {"example.com/empty/but:different", "example.com/empty/but:different"}, - }, - true, - }, - { // Reference entire multi-image archive must fail (more than one manifest) - "docker-archive:testdata/docker-two-images.tar.xz", - []expected{}, - true, - }, - - // == oci-archive: - {"oci-archive:/dev/this-does-not-exist", nil, false}, // Input does not exist. - {"oci-archive:/dev/null", nil, false}, // 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"}}, - // false, - // }, - // { // 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"}}, - // false, - // }, - // { // Name is a registry/name:latest - // "oci-archive:testdata/oci-registry-name.tar.gz", - // []expected{{"example.com/empty:latest", "example.com/empty:latest"}}, - // false, - // }, - // // Name exists, but is an invalid Docker reference; such names will fail when creating dstReference. - // {"oci-archive:testdata/oci-non-docker-name.tar.gz", nil, false}, - // 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:latest"}}, - false, - }, - { // Relative path, single element. - "dir:this-does-not-exist", - []expected{{"localhost/this-does-not-exist", "localhost/this-does-not-exist:latest"}}, - false, - }, - { // Relative path, multiple elements. - "dir:testdata/this-does-not-exist", - []expected{{"localhost/testdata/this-does-not-exist", "localhost/testdata/this-does-not-exist:latest"}}, - false, - }, - - // == 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"}}, - false, - }, - { // Minimal form of the input - "docker://busybox", - []expected{{"docker://busybox", "docker.io/library/busybox:latest"}}, - false, - }, - - // === tarball: (as an example of what happens when ImageReference.DockerReference is nil). - // FIXME? This tries to parse "tarball:/dev/null" as a storageReference, and fails. - // (This is NOT an API promise that the results will continue to be this way.) - {"tarball:/dev/null", nil, false}, - } { - srcRef, err := alltransports.ParseImageName(c.srcName) - require.NoError(t, err, c.srcName) - - res, err := ir.pullGoalFromImageReference(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.refPairs, len(c.expected), c.srcName) - for i, e := range c.expected { - testDescription := fmt.Sprintf("%s #%d", c.srcName, i) - assert.Equal(t, e.image, res.refPairs[i].image, testDescription) - assert.Equal(t, transports.ImageName(srcRef), transports.ImageName(res.refPairs[i].srcRef), testDescription) - assert.Equal(t, e.dstName, storageReferenceWithoutLocation(res.refPairs[i].dstRef), testDescription) - } - assert.Equal(t, c.expectedPullAllPairs, res.pullAllPairs, c.srcName) - } - } -} - -const registriesConfWithSearch = `unqualified-search-registries = ['example.com', 'docker.io']` - -func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { - const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - type pullRefStrings struct{ image, srcRef, dstName string } // pullRefPair with string data only - - registriesConf, err := ioutil.TempFile("", "TestPullGoalFromPossiblyUnqualifiedName") - require.NoError(t, err) - defer registriesConf.Close() - defer os.Remove(registriesConf.Name()) - - err = ioutil.WriteFile(registriesConf.Name(), []byte(registriesConfWithSearch), 0600) - require.NoError(t, err) - - ir, cleanup := newTestRuntime(t) - defer cleanup() - - sc := GetSystemContext("", "", false) - - aliasesConf, err := ioutil.TempFile("", "short-name-aliases.conf") - require.NoError(t, err) - defer aliasesConf.Close() - defer os.Remove(aliasesConf.Name()) - sc.UserShortNameAliasConfPath = aliasesConf.Name() - sc.SystemRegistriesConfPath = registriesConf.Name() - - // Make sure to not sure the system's registries.conf.d - dir, err := ioutil.TempDir("", "example") - require.NoError(t, err) - sc.SystemRegistriesConfDirPath = dir - defer os.RemoveAll(dir) // clean up - - for _, c := range []struct { - input string - expected []pullRefStrings - }{ - {"#", nil}, // Clearly invalid. - { // Fully-explicit docker.io, name-only. - "docker.io/library/busybox", - // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - []pullRefStrings{{"docker.io/library/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}}, - }, - { // docker.io with implied /library/, name-only. - "docker.io/busybox", - // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - []pullRefStrings{{"docker.io/library/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}}, - }, - { // Qualified example.com, name-only. - "example.com/ns/busybox", - []pullRefStrings{{"example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}}, - }, - { // Qualified example.com, name:tag. - "example.com/ns/busybox:notlatest", - []pullRefStrings{{"example.com/ns/busybox:notlatest", "docker://example.com/ns/busybox:notlatest", "example.com/ns/busybox:notlatest"}}, - }, - { // Qualified example.com, name@digest. - "example.com/ns/busybox" + digestSuffix, - []pullRefStrings{{"example.com/ns/busybox" + digestSuffix, "docker://example.com/ns/busybox" + digestSuffix, - "example.com/ns/busybox" + digestSuffix}}, - }, - // Qualified example.com, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input. - {"example.com/ns/busybox:notlatest" + digestSuffix, nil}, - { // Unqualified, single-name, name-only - "busybox", - []pullRefStrings{ - {"example.com/busybox:latest", "docker://example.com/busybox:latest", "example.com/busybox:latest"}, - // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - {"docker.io/library/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}, - }, - }, - { // Unqualified, namespaced, name-only - "ns/busybox", - []pullRefStrings{ - {"example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}, - }, - }, - { // Unqualified, name:tag - "busybox:notlatest", - []pullRefStrings{ - {"example.com/busybox:notlatest", "docker://example.com/busybox:notlatest", "example.com/busybox:notlatest"}, - // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - {"docker.io/library/busybox:notlatest", "docker://busybox:notlatest", "docker.io/library/busybox:notlatest"}, - }, - }, - { // Unqualified, name@digest - "busybox" + digestSuffix, - []pullRefStrings{ - {"example.com/busybox" + digestSuffix, "docker://example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix}, - // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - {"docker.io/library/busybox" + digestSuffix, "docker://busybox" + digestSuffix, "docker.io/library/busybox" + digestSuffix}, - }, - }, - // Unqualified, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input. - {"busybox:notlatest" + digestSuffix, nil}, - } { - res, err := ir.pullGoalFromPossiblyUnqualifiedName(sc, nil, c.input) - if len(c.expected) == 0 { - assert.Error(t, err, c.input) - } else { - assert.NoError(t, err, c.input) - for i, e := range c.expected { - testDescription := fmt.Sprintf("%s #%d (%v)", c.input, i, res.refPairs) - assert.Equal(t, e.image, res.refPairs[i].image, testDescription) - assert.Equal(t, e.srcRef, transports.ImageName(res.refPairs[i].srcRef), testDescription) - assert.Equal(t, e.dstName, storageReferenceWithoutLocation(res.refPairs[i].dstRef), testDescription) - } - assert.False(t, res.pullAllPairs, c.input) - } - } -} -- cgit v1.2.3-54-g00ecf