diff options
-rw-r--r-- | libpod/image/pull.go | 39 | ||||
-rw-r--r-- | libpod/image/pull_test.go | 68 |
2 files changed, 86 insertions, 21 deletions
diff --git a/libpod/image/pull.go b/libpod/image/pull.go index 7ea19d0e6..f7131f11e 100644 --- a/libpod/image/pull.go +++ b/libpod/image/pull.go @@ -64,7 +64,7 @@ type pullRefName struct { dstName string } -func (ir *Runtime) getPullRefPair(srcRef types.ImageReference, destName string) (*pullRefPair, error) { +func getPullRefName(srcRef types.ImageReference, destName string) (*pullRefName, error) { imgPart, err := decompose(destName) if err == nil && !imgPart.hasRegistry { // If the image doesn't have a registry, set it as the default repo @@ -77,20 +77,16 @@ func (ir *Runtime) getPullRefPair(srcRef types.ImageReference, destName string) if srcRef.DockerReference() != nil { reference = srcRef.DockerReference().String() } - destRef, err := is.Transport.ParseStoreReference(ir.store, reference) - if err != nil { - return nil, errors.Wrapf(err, "error parsing dest reference name") - } - return &pullRefPair{ - image: destName, - srcRef: srcRef, - dstRef: destRef, + return &pullRefName{ + image: destName, + srcRef: srcRef, + dstName: reference, }, 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) { - var pullRefPairs []*pullRefPair + var pullNames []*pullRefName splitArr := strings.Split(imgName, ":") archFile := splitArr[len(splitArr)-1] @@ -112,11 +108,11 @@ func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageRef if err != nil { return nil, err } - pullInfo, err := ir.getPullRefPair(srcRef, reference) + pullInfo, err := getPullRefName(srcRef, reference) if err != nil { return nil, err } - pullRefPairs = append(pullRefPairs, pullInfo) + pullNames = append(pullNames, pullInfo) } else { var dest []string if len(manifest[0].RepoTags) > 0 { @@ -131,11 +127,11 @@ func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageRef } // Need to load in all the repo tags from the manifest for _, dst := range dest { - pullInfo, err := ir.getPullRefPair(srcRef, dst) + pullInfo, err := getPullRefName(srcRef, dst) if err != nil { return nil, err } - pullRefPairs = append(pullRefPairs, pullInfo) + pullNames = append(pullNames, pullInfo) } } } else if srcRef.Transport().Name() == OCIArchive { @@ -156,11 +152,11 @@ func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageRef } else { dest = manifest.Annotations["org.opencontainers.image.ref.name"] } - pullInfo, err := ir.getPullRefPair(srcRef, dest) + pullInfo, err := getPullRefName(srcRef, dest) if err != nil { return nil, err } - pullRefPairs = append(pullRefPairs, pullInfo) + pullNames = append(pullNames, pullInfo) } else if srcRef.Transport().Name() == DirTransport { // supports pull from a directory image := splitArr[1] @@ -170,19 +166,20 @@ func (ir *Runtime) getPullListFromRef(ctx context.Context, srcRef types.ImageRef // so docker.io isn't prepended, and the path becomes the repository image = DefaultLocalRepo + image } - pullInfo, err := ir.getPullRefPair(srcRef, image) + pullInfo, err := getPullRefName(srcRef, image) if err != nil { return nil, err } - pullRefPairs = append(pullRefPairs, pullInfo) + pullNames = append(pullNames, pullInfo) } else { - pullInfo, err := ir.getPullRefPair(srcRef, imgName) + pullInfo, err := getPullRefName(srcRef, imgName) if err != nil { return nil, err } - pullRefPairs = append(pullRefPairs, pullInfo) + pullNames = append(pullNames, pullInfo) } - return pullRefPairs, nil + + return ir.pullRefPairsFromRefNames(pullNames) } // pullImage pulls an image from configured registries diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go index 535dd4e9d..e5e9f6b91 100644 --- a/libpod/image/pull_test.go +++ b/libpod/image/pull_test.go @@ -1,15 +1,83 @@ package image import ( + "fmt" "io/ioutil" "os" "testing" "github.com/containers/image/transports" + "github.com/containers/image/transports/alltransports" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestGetPullRefName(t *testing.T) { + const imageID = "@0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + 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", + // The destName value will be interpreted as "example.com/from-directory:latest" by storageTransport. + "example.com/from-directory", "example.com/from-directory", + }, + { // name, no registry, no tag: + "dir:/dev/this-does-not-exist", "from-directory", + // FIXME(?) Adding a registry also adds a :latest tag. OTOH that actually matches the used destination. + // Either way it is surprising that the localhost/ addition changes this. (mitr hoping to remove the "image" member). + "localhost/from-directory:latest", "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, + // FIXME?! Why is this dropping the digest, and adding :none?! + "localhost/from-directory:none", "localhost/from-directory:none", + }, + { // 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: + // FIXME: This is interpreted as "registry == ns" + "dir:/dev/this-does-not-exist", "ns/from-directory:notlatest", + "ns/from-directory:notlatest", "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", + }, + } { + srcRef, err := alltransports.ParseImageName(c.srcName) + require.NoError(t, err, c.srcName) + + testName := fmt.Sprintf("%#v %#v", c.srcName, c.destName) + res, err := getPullRefName(srcRef, c.destName) + require.NoError(t, err, testName) + assert.Equal(t, &pullRefName{image: c.expectedImage, srcRef: srcRef, dstName: c.expectedDstName}, res, testName) + } +} + const registriesConfWithSearch = `[registries.search] registries = ['example.com', 'docker.io'] ` |