diff options
-rw-r--r-- | libpod/adapter/runtime_remote.go | 18 | ||||
-rw-r--r-- | libpod/image/image.go | 61 | ||||
-rw-r--r-- | libpod/image/image_test.go | 9 | ||||
-rw-r--r-- | libpod/image/parts.go | 125 | ||||
-rw-r--r-- | libpod/image/parts_test.go | 118 | ||||
-rw-r--r-- | libpod/image/pull.go | 59 | ||||
-rw-r--r-- | libpod/image/pull_test.go | 28 | ||||
-rw-r--r-- | libpod/image/utils.go | 12 |
8 files changed, 227 insertions, 203 deletions
diff --git a/libpod/adapter/runtime_remote.go b/libpod/adapter/runtime_remote.go index 0fe5c449a..27301d90b 100644 --- a/libpod/adapter/runtime_remote.go +++ b/libpod/adapter/runtime_remote.go @@ -5,13 +5,13 @@ package adapter import ( "context" "fmt" - "github.com/containers/libpod/cmd/podman/varlink" - "github.com/containers/libpod/libpod/image" - "github.com/opencontainers/go-digest" - "github.com/urfave/cli" - "github.com/varlink/go/varlink" "strings" "time" + + iopodman "github.com/containers/libpod/cmd/podman/varlink" + digest "github.com/opencontainers/go-digest" + "github.com/urfave/cli" + "github.com/varlink/go/varlink" ) // ImageRuntime is wrapper for image runtime @@ -59,8 +59,6 @@ type remoteImage struct { RepoDigests []string Parent string Size int64 - Tag string - Repository string Created time.Time InputName string Names []string @@ -91,10 +89,6 @@ func (r *LocalRuntime) GetImages() ([]*ContainerImage, error) { } func imageInListToContainerImage(i iopodman.ImageInList, name string, runtime *LocalRuntime) (*ContainerImage, error) { - imageParts, err := image.DecomposeString(name) - if err != nil { - return nil, err - } created, err := splitStringDate(i.Created) if err != nil { return nil, err @@ -108,8 +102,6 @@ func imageInListToContainerImage(i iopodman.ImageInList, name string, runtime *L Parent: i.ParentId, Size: i.Size, Created: created, - Tag: imageParts.Tag, - Repository: imageParts.Registry, Names: i.RepoTags, isParent: i.IsParent, Runtime: runtime, diff --git a/libpod/image/image.go b/libpod/image/image.go index dda753385..ea326d820 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -25,7 +25,7 @@ import ( "github.com/containers/libpod/pkg/util" "github.com/containers/storage" "github.com/containers/storage/pkg/reexec" - "github.com/opencontainers/go-digest" + digest "github.com/opencontainers/go-digest" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -226,7 +226,6 @@ func (i *Image) getLocalImage() (*storage.Image, error) { i.InputName = dest.DockerReference().String() } - var taggedName string img, err := i.imageruntime.getImage(stripSha256(i.InputName)) if err == nil { return img.image, err @@ -240,25 +239,18 @@ func (i *Image) getLocalImage() (*storage.Image, error) { return nil, err } - // the inputname isn't tagged, so we assume latest and try again - if !decomposedImage.isTagged { - taggedName = fmt.Sprintf("%s:latest", i.InputName) - img, err = i.imageruntime.getImage(taggedName) - if err == nil { - return img.image, nil - } - } - // The image has a registry name in it and we made sure we looked for it locally // with a tag. It cannot be local. if decomposedImage.hasRegistry { 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 - img, err = i.imageruntime.getImage(fmt.Sprintf("%s/%s", DefaultLocalRegistry, i.InputName)) + ref, err := decomposedImage.referenceWithRegistry(DefaultLocalRegistry) + if err != nil { + return nil, err + } + img, err = i.imageruntime.getImage(ref.String()) if err == nil { return img.image, err } @@ -452,35 +444,42 @@ func getImageDigest(ctx context.Context, src types.ImageReference, sc *types.Sys return "@" + digest.Hex(), nil } -// normalizeTag returns the canonical version of tag for use in Image.Names() -func normalizeTag(tag string) (string, error) { +// normalizedTag returns the canonical version of tag for use in Image.Names() +func normalizedTag(tag string) (reference.Named, error) { decomposedTag, err := decompose(tag) if err != nil { - return "", err - } - // If the input does not have a tag, we need to add one (latest) - if !decomposedTag.isTagged { - tag = fmt.Sprintf("%s:%s", tag, decomposedTag.Tag) + return nil, err } // If the input doesn't specify a registry, set the registry to localhost + var ref reference.Named if !decomposedTag.hasRegistry { - tag = fmt.Sprintf("%s/%s", DefaultLocalRegistry, tag) + ref, err = decomposedTag.referenceWithRegistry(DefaultLocalRegistry) + if err != nil { + return nil, err + } + } else { + ref, err = decomposedTag.normalizedReference() + if err != nil { + return nil, err + } } - return tag, nil + // If the input does not have a tag, we need to add one (latest) + ref = reference.TagNameOnly(ref) + return ref, nil } // TagImage adds a tag to the given image func (i *Image) TagImage(tag string) error { i.reloadImage() - tag, err := normalizeTag(tag) + ref, err := normalizedTag(tag) if err != nil { return err } tags := i.Names() - if util.StringInSlice(tag, tags) { + if util.StringInSlice(ref.String(), tags) { return nil } - tags = append(tags, tag) + tags = append(tags, ref.String()) if err := i.imageruntime.store.SetNames(i.ID(), tags); err != nil { return err } @@ -931,21 +930,23 @@ func (i *Image) MatchRepoTag(input string) (string, error) { if err != nil { return "", err } + imageRegistry, imageName, imageSuspiciousTagValueForSearch := dcImage.suspiciousRefNameTagValuesForSearch() for _, repoName := range i.Names() { count := 0 dcRepoName, err := decompose(repoName) if err != nil { return "", err } - if dcRepoName.Registry == dcImage.Registry && dcImage.Registry != "" { + repoNameRegistry, repoNameName, repoNameSuspiciousTagValueForSearch := dcRepoName.suspiciousRefNameTagValuesForSearch() + if repoNameRegistry == imageRegistry && imageRegistry != "" { count++ } - if dcRepoName.name == dcImage.name && dcImage.name != "" { + if repoNameName == imageName && imageName != "" { count++ - } else if splitString(dcRepoName.name) == splitString(dcImage.name) { + } else if splitString(repoNameName) == splitString(imageName) { count++ } - if dcRepoName.Tag == dcImage.Tag { + if repoNameSuspiciousTagValueForSearch == imageSuspiciousTagValueForSearch { count++ } results[count] = append(results[count], repoName) diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go index 2a68fd273..077ae460e 100644 --- a/libpod/image/image_test.go +++ b/libpod/image/image_test.go @@ -257,24 +257,25 @@ func Test_stripSha256(t *testing.T) { assert.Equal(t, stripSha256("sha256:a"), "a") } -func TestNormalizeTag(t *testing.T) { +func TestNormalizedTag(t *testing.T) { const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" for _, c := range []struct{ input, expected string }{ {"#", ""}, // Clearly invalid {"example.com/busybox", "example.com/busybox:latest"}, // Qualified name-only {"example.com/busybox:notlatest", "example.com/busybox:notlatest"}, // Qualified name:tag - {"example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix + ":none"}, // Qualified name@digest; FIXME: The result is not even syntactically valid! + {"example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix}, // Qualified name@digest; FIXME? Should we allow tagging with a digest at all? {"example.com/busybox:notlatest" + digestSuffix, "example.com/busybox:notlatest" + digestSuffix}, // Qualified name:tag@digest {"busybox:latest", "localhost/busybox:latest"}, // Unqualified name-only {"ns/busybox:latest", "localhost/ns/busybox:latest"}, // Unqualified with a dot-less namespace + {"docker.io/busybox:latest", "docker.io/library/busybox:latest"}, // docker.io without /library/ } { - res, err := normalizeTag(c.input) + res, err := normalizedTag(c.input) if c.expected == "" { assert.Error(t, err, c.input) } else { assert.NoError(t, err, c.input) - assert.Equal(t, c.expected, res) + assert.Equal(t, c.expected, res.String()) } } } diff --git a/libpod/image/parts.go b/libpod/image/parts.go index b2a69f26c..dfdf0b08a 100644 --- a/libpod/image/parts.go +++ b/libpod/image/parts.go @@ -1,23 +1,20 @@ package image import ( - "fmt" "strings" "github.com/containers/image/docker/reference" + "github.com/pkg/errors" ) -// Parts describes the parts of an image's name -type Parts struct { - transport string - Registry string - name string - Tag string - isTagged bool - hasRegistry bool +// imageParts describes the parts of an image's name +type imageParts struct { + unnormalizedRef reference.Named // WARNING: Did not go through docker.io[/library] normalization + hasRegistry bool } -// Registries must contain a ":" or a "." or be localhost +// Registries must contain a ":" or a "." or be localhost; this helper exists for users of reference.Parse. +// For inputs that should use the docker.io[/library] normalization, use reference.ParseNormalizedNamed instead. func isRegistry(name string) bool { return strings.ContainsAny(name, ".:") || name == "localhost" } @@ -30,68 +27,78 @@ func GetImageBaseName(input string) (string, error) { if err != nil { return "", err } - splitImageName := strings.Split(decomposedImage.name, "/") + splitImageName := strings.Split(decomposedImage.unnormalizedRef.Name(), "/") return splitImageName[len(splitImageName)-1], nil } -// DecomposeString decomposes a string name into imageParts description. This -// is a wrapper for decompose -func DecomposeString(input string) (Parts, error) { - return decompose(input) -} - // decompose breaks an input name into an imageParts description -func decompose(input string) (Parts, error) { - var ( - parts Parts - hasRegistry bool - tag string - ) +func decompose(input string) (imageParts, error) { imgRef, err := reference.Parse(input) if err != nil { - return parts, err + return imageParts{}, err } - ntag, isTagged := imgRef.(reference.NamedTagged) - if !isTagged { - tag = "latest" - if _, hasDigest := imgRef.(reference.Digested); hasDigest { - tag = "none" - } - } else { - tag = ntag.Tag() + unnormalizedNamed := imgRef.(reference.Named) + // ip.unnormalizedRef, because it uses reference.Parse and not reference.ParseNormalizedNamed, + // does not use the standard heuristics for domains vs. namespaces/repos, so we need to check + // explicitly. + hasRegistry := isRegistry(reference.Domain(unnormalizedNamed)) + return imageParts{ + unnormalizedRef: unnormalizedNamed, + hasRegistry: hasRegistry, + }, nil +} + +// suspiciousRefNameTagValuesForSearch returns a "tag" value used in a previous implementation. +// This exists only to preserve existing behavior in heuristic code; it’s dubious that that behavior is correct, +// gespecially for the tag value. +func (ip *imageParts) suspiciousRefNameTagValuesForSearch() (string, string, string) { + registry := reference.Domain(ip.unnormalizedRef) + imageName := reference.Path(ip.unnormalizedRef) + // ip.unnormalizedRef, because it uses reference.Parse and not reference.ParseNormalizedNamed, + // does not use the standard heuristics for domains vs. namespaces/repos. + if registry != "" && !isRegistry(registry) { + imageName = registry + "/" + imageName + registry = "" } - registry := reference.Domain(imgRef.(reference.Named)) - imageName := reference.Path(imgRef.(reference.Named)) - // Is this a Registry or a repo? - if isRegistry(registry) { - hasRegistry = true + + var tag string + if tagged, isTagged := ip.unnormalizedRef.(reference.NamedTagged); isTagged { + tag = tagged.Tag() + } else if _, hasDigest := ip.unnormalizedRef.(reference.Digested); hasDigest { + tag = "none" } else { - if registry != "" { - imageName = registry + "/" + imageName - registry = "" - } + tag = "latest" } - return Parts{ - Registry: registry, - hasRegistry: hasRegistry, - name: imageName, - Tag: tag, - isTagged: isTagged, - transport: DefaultTransport, - }, nil + return registry, imageName, tag } -// assemble concatenates an image's parts into a string -func (ip *Parts) assemble() string { - spec := fmt.Sprintf("%s:%s", ip.name, ip.Tag) - - if ip.Registry != "" { - spec = fmt.Sprintf("%s/%s", ip.Registry, spec) +// referenceWithRegistry returns a (normalized) reference.Named composed of ip (with !ip.hasRegistry) +// qualified with registry. +func (ip *imageParts) referenceWithRegistry(registry string) (reference.Named, error) { + if ip.hasRegistry { + return nil, errors.Errorf("internal error: referenceWithRegistry called on imageParts with a registry (%#v)", *ip) } - return spec + // We could build a reference.WithName+WithTag/WithDigest here, but we need to round-trip via a string + // and a ParseNormalizedNamed anyway to get the right normalization of docker.io/library, so + // just use a string directly. + qualified := registry + "/" + ip.unnormalizedRef.String() + ref, err := reference.ParseNormalizedNamed(qualified) + if err != nil { + return nil, errors.Wrapf(err, "error normalizing registry+unqualified reference %#v", qualified) + } + return ref, nil } -// assemble concatenates an image's parts with transport into a string -func (ip *Parts) assembleWithTransport() string { - return fmt.Sprintf("%s%s", ip.transport, ip.assemble()) +// normalizedReference returns a (normalized) reference for ip (with ip.hasRegistry) +func (ip *imageParts) normalizedReference() (reference.Named, error) { + if !ip.hasRegistry { + return nil, errors.Errorf("internal error: normalizedReference called on imageParts without a registry (%#v)", *ip) + } + // We need to round-trip via a string to get the right normalization of docker.io/library + s := ip.unnormalizedRef.String() + ref, err := reference.ParseNormalizedNamed(s) + if err != nil { // Should never happen + return nil, errors.Wrapf(err, "error normalizing qualified reference %#v", s) + } + return ref, nil } diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go index 1e01c85d3..726e55e86 100644 --- a/libpod/image/parts_test.go +++ b/libpod/image/parts_test.go @@ -4,64 +4,120 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDecompose(t *testing.T) { const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" for _, c := range []struct { - input string - transport, registry, name, tag string - isTagged, hasRegistry bool - assembled string - assembledWithTransport string + input string + registry, name, suspiciousTagValueForSearch string + hasRegistry bool }{ - {"#", "", "", "", "", false, false, "", ""}, // Entirely invalid input + {"#", "", "", "", false}, // Entirely invalid input { // Fully qualified docker.io, name-only input - "docker.io/library/busybox", "docker://", "docker.io", "library/busybox", "latest", false, true, - "docker.io/library/busybox:latest", "docker://docker.io/library/busybox:latest", + "docker.io/library/busybox", "docker.io", "library/busybox", "latest", true, }, { // Fully qualified example.com, name-only input - "example.com/ns/busybox", "docker://", "example.com", "ns/busybox", "latest", false, true, - "example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", + "example.com/ns/busybox", "example.com", "ns/busybox", "latest", true, }, { // Unqualified single-name input - "busybox", "docker://", "", "busybox", "latest", false, false, - "busybox:latest", "docker://busybox:latest", + "busybox", "", "busybox", "latest", false, }, { // Unqualified namespaced input - "ns/busybox", "docker://", "", "ns/busybox", "latest", false, false, - "ns/busybox:latest", "docker://ns/busybox:latest", + "ns/busybox", "", "ns/busybox", "latest", false, }, { // name:tag - "example.com/ns/busybox:notlatest", "docker://", "example.com", "ns/busybox", "notlatest", true, true, - "example.com/ns/busybox:notlatest", "docker://example.com/ns/busybox:notlatest", + "example.com/ns/busybox:notlatest", "example.com", "ns/busybox", "notlatest", true, }, { // name@digest - // FIXME? .tag == "none" - "example.com/ns/busybox" + digestSuffix, "docker://", "example.com", "ns/busybox", "none", false, true, - // FIXME: this drops the digest and replaces it with an incorrect tag. - "example.com/ns/busybox:none", "docker://example.com/ns/busybox:none", + // FIXME? .suspiciousTagValueForSearch == "none" + "example.com/ns/busybox" + digestSuffix, "example.com", "ns/busybox", "none", true, }, { // name:tag@digest - "example.com/ns/busybox:notlatest" + digestSuffix, "docker://", "example.com", "ns/busybox", "notlatest", true, true, - // FIXME: This drops the digest - "example.com/ns/busybox:notlatest", "docker://example.com/ns/busybox:notlatest", + "example.com/ns/busybox:notlatest" + digestSuffix, "example.com", "ns/busybox", "notlatest", true, }, } { parts, err := decompose(c.input) - if c.transport == "" { + if c.name == "" { assert.Error(t, err, c.input) } else { assert.NoError(t, err, c.input) - assert.Equal(t, c.transport, parts.transport, c.input) - assert.Equal(t, c.registry, parts.Registry, c.input) - assert.Equal(t, c.name, parts.name, c.input) - assert.Equal(t, c.tag, parts.Tag, c.input) - assert.Equal(t, c.isTagged, parts.isTagged, c.input) + registry, name, suspiciousTagValueForSearch := parts.suspiciousRefNameTagValuesForSearch() + assert.Equal(t, c.registry, registry, c.input) + assert.Equal(t, c.name, name, c.input) + assert.Equal(t, c.suspiciousTagValueForSearch, suspiciousTagValueForSearch, c.input) assert.Equal(t, c.hasRegistry, parts.hasRegistry, c.input) - assert.Equal(t, c.assembled, parts.assemble(), c.input) - assert.Equal(t, c.assembledWithTransport, parts.assembleWithTransport(), c.input) + } + } +} + +func TestImagePartsReferenceWithRegistry(t *testing.T) { + const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + for _, c := range []struct { + input string + withDocker, withNonDocker string + }{ + {"example.com/ns/busybox", "", ""}, // Fully-qualified input is invalid. + {"busybox", "docker.io/library/busybox", "example.com/busybox"}, // Single-name input + {"ns/busybox", "docker.io/ns/busybox", "example.com/ns/busybox"}, // Namespaced input + {"ns/busybox:notlatest", "docker.io/ns/busybox:notlatest", "example.com/ns/busybox:notlatest"}, // name:tag + {"ns/busybox" + digestSuffix, "docker.io/ns/busybox" + digestSuffix, "example.com/ns/busybox" + digestSuffix}, // name@digest + { // name:tag@digest + "ns/busybox:notlatest" + digestSuffix, + "docker.io/ns/busybox:notlatest" + digestSuffix, "example.com/ns/busybox:notlatest" + digestSuffix, + }, + } { + parts, err := decompose(c.input) + require.NoError(t, err) + if c.withDocker == "" { + _, err := parts.referenceWithRegistry("docker.io") + assert.Error(t, err, c.input) + _, err = parts.referenceWithRegistry("example.com") + assert.Error(t, err, c.input) + } else { + ref, err := parts.referenceWithRegistry("docker.io") + require.NoError(t, err, c.input) + assert.Equal(t, c.withDocker, ref.String()) + ref, err = parts.referenceWithRegistry("example.com") + require.NoError(t, err, c.input) + assert.Equal(t, c.withNonDocker, ref.String()) + } + } + + // Invalid registry value + parts, err := decompose("busybox") + require.NoError(t, err) + _, err = parts.referenceWithRegistry("invalid@domain") + assert.Error(t, err) +} + +func TestImagePartsNormalizedReference(t *testing.T) { + const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + for _, c := range []struct{ input, expected string }{ + {"busybox", ""}, // Unqualified input is invalid + {"docker.io/busybox", "docker.io/library/busybox"}, // docker.io single-name + {"example.com/busybox", "example.com/busybox"}, // example.com single-name + {"docker.io/ns/busybox", "docker.io/ns/busybox"}, // docker.io namespaced + {"example.com/ns/busybox", "example.com/ns/busybox"}, // example.com namespaced + {"example.com/ns/busybox:notlatest", "example.com/ns/busybox:notlatest"}, // name:tag + {"example.com/ns/busybox" + digestSuffix, "example.com/ns/busybox" + digestSuffix}, // name@digest + { // name:tag@digest + "example.com/ns/busybox:notlatest" + digestSuffix, "example.com/ns/busybox:notlatest" + digestSuffix, + }, + } { + parts, err := decompose(c.input) + require.NoError(t, err) + if c.expected == "" { + _, err := parts.normalizedReference() + assert.Error(t, err, c.input) + } else { + ref, err := parts.normalizedReference() + require.NoError(t, err, c.input) + assert.Equal(t, c.expected, ref.String()) } } } diff --git a/libpod/image/pull.go b/libpod/image/pull.go index 203e94310..434b83520 100644 --- a/libpod/image/pull.go +++ b/libpod/image/pull.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io" - "strings" cp "github.com/containers/image/copy" "github.com/containers/image/directory" @@ -76,9 +75,11 @@ func (ir *Runtime) getPullRefPair(srcRef types.ImageReference, destName string) decomposedDest, err := decompose(destName) if err == nil && !decomposedDest.hasRegistry { // If the image doesn't have a registry, set it as the default repo - decomposedDest.Registry = DefaultLocalRegistry - decomposedDest.hasRegistry = true - destName = decomposedDest.assemble() + ref, err := decomposedDest.referenceWithRegistry(DefaultLocalRegistry) + if err != nil { + return pullRefPair{}, err + } + destName = ref.String() } reference := destName @@ -270,12 +271,6 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa return images, nil } -// hasShaInInputName returns a bool as to whether the user provided an image name that includes -// a reference to a specific sha -func hasShaInInputName(inputName string) bool { - return strings.Contains(inputName, "@sha256:") -} - // pullGoalFromPossiblyUnqualifiedName looks at inputName and determines the possible // image references to try pulling in combination with the registries.conf file as well func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullGoal, error) { @@ -284,31 +279,11 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG return nil, err } if decomposedImage.hasRegistry { - var imageName, destName string - if hasShaInInputName(inputName) { - imageName = fmt.Sprintf("%s%s", decomposedImage.transport, inputName) - } else { - imageName = decomposedImage.assembleWithTransport() - } - srcRef, err := alltransports.ParseImageName(imageName) + srcRef, err := docker.ParseReference("//" + inputName) if err != nil { return nil, errors.Wrapf(err, "unable to parse '%s'", inputName) } - if hasShaInInputName(inputName) { - destName = decomposedImage.assemble() - } else { - destName = inputName - } - destRef, err := is.Transport.ParseStoreReference(ir.store, destName) - if err != nil { - return nil, errors.Wrapf(err, "error parsing dest reference name %#v", destName) - } - ps := pullRefPair{ - image: inputName, - srcRef: srcRef, - dstRef: destRef, - } - return singlePullRefPairGoal(ps), nil + return ir.getSinglePullRefPairGoal(srcRef, inputName) } searchRegistries, err := registries.GetRegistries() @@ -317,22 +292,18 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG } var refPairs []pullRefPair for _, registry := range searchRegistries { - decomposedImage.Registry = registry - imageName := decomposedImage.assembleWithTransport() - if hasShaInInputName(inputName) { - imageName = fmt.Sprintf("%s%s/%s", decomposedImage.transport, registry, inputName) - } - srcRef, err := alltransports.ParseImageName(imageName) + ref, err := decomposedImage.referenceWithRegistry(registry) if err != nil { - return nil, errors.Wrapf(err, "unable to parse '%s'", inputName) + return nil, err } - ps := pullRefPair{ - image: decomposedImage.assemble(), - srcRef: srcRef, + imageName := ref.String() + srcRef, err := docker.ParseReference("//" + imageName) + if err != nil { + return nil, errors.Wrapf(err, "unable to parse '%s'", imageName) } - ps.dstRef, err = is.Transport.ParseStoreReference(ir.store, ps.image) + ps, err := ir.getPullRefPair(srcRef, imageName) if err != nil { - return nil, errors.Wrapf(err, "error parsing dest reference name %#v", ps.image) + return nil, err } refPairs = append(refPairs, ps) } diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go index 37b45dc83..3890c5e6c 100644 --- a/libpod/image/pull_test.go +++ b/libpod/image/pull_test.go @@ -76,9 +76,7 @@ func TestGetPullRefPair(t *testing.T) { }, { // 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", + "localhost/from-directory", "localhost/from-directory:latest", }, { // registry/name:tag : "dir:/dev/this-does-not-exist", "example.com/from-directory:notlatest", @@ -90,8 +88,7 @@ func TestGetPullRefPair(t *testing.T) { }, { // 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", + "localhost/from-directory" + digestSuffix, "localhost/from-directory" + digestSuffix, }, { // registry/name@digest: "dir:/dev/this-does-not-exist", "example.com/from-directory" + digestSuffix, @@ -211,14 +208,13 @@ func TestPullGoalFromImageReference(t *testing.T) { false, }, { // Relative path, single element. - // FIXME? Note the :latest difference in .image. "dir:this-does-not-exist", - []expected{{"localhost/this-does-not-exist:latest", "localhost/this-does-not-exist:latest"}}, + []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:latest", "localhost/testdata/this-does-not-exist:latest"}}, + []expected{{"localhost/testdata/this-does-not-exist", "localhost/testdata/this-does-not-exist:latest"}}, false, }, @@ -324,8 +320,7 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { { // Qualified example.com, name@digest. "example.com/ns/busybox" + digestSuffix, []pullRefStrings{{"example.com/ns/busybox" + digestSuffix, "docker://example.com/ns/busybox" + digestSuffix, - // FIXME?! Why is .dstName dropping the digest, and adding :none?! - "example.com/ns/busybox:none"}}, + "example.com/ns/busybox" + digestSuffix}}, false, }, // Qualified example.com, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input. @@ -333,16 +328,16 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { { // Unqualified, single-name, name-only "busybox", []pullRefStrings{ - {"example.com/busybox:latest", "docker://example.com/busybox:latest", "example.com/busybox:latest"}, + {"example.com/busybox", "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/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}, + {"docker.io/library/busybox", "docker://busybox:latest", "docker.io/library/busybox:latest"}, }, true, }, { // Unqualified, namespaced, name-only "ns/busybox", []pullRefStrings{ - {"example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}, + {"example.com/ns/busybox", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}, }, true, }, @@ -351,17 +346,16 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { []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/busybox:notlatest", "docker://busybox:notlatest", "docker.io/library/busybox:notlatest"}, + {"docker.io/library/busybox:notlatest", "docker://busybox:notlatest", "docker.io/library/busybox:notlatest"}, }, true, }, { // Unqualified, name@digest "busybox" + digestSuffix, []pullRefStrings{ - // FIXME?! Why is .input and .dstName dropping the digest, and adding :none?! - {"example.com/busybox:none", "docker://example.com/busybox" + digestSuffix, "example.com/busybox:none"}, + {"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/busybox:none", "docker://busybox" + digestSuffix, "docker.io/library/busybox:none"}, + {"docker.io/library/busybox" + digestSuffix, "docker://busybox" + digestSuffix, "docker.io/library/busybox" + digestSuffix}, }, true, }, diff --git a/libpod/image/utils.go b/libpod/image/utils.go index 135b47008..ad027f32a 100644 --- a/libpod/image/utils.go +++ b/libpod/image/utils.go @@ -16,7 +16,8 @@ import ( // findImageInRepotags takes an imageParts struct and searches images' repotags for // a match on name:tag -func findImageInRepotags(search Parts, images []*Image) (*storage.Image, error) { +func findImageInRepotags(search imageParts, images []*Image) (*storage.Image, error) { + _, searchName, searchSuspiciousTagValueForSearch := search.suspiciousRefNameTagValuesForSearch() var results []*storage.Image for _, image := range images { for _, name := range image.Names() { @@ -25,21 +26,22 @@ func findImageInRepotags(search Parts, images []*Image) (*storage.Image, error) if err != nil { continue } - if d.name == search.name && d.Tag == search.Tag { + _, dName, dSuspiciousTagValueForSearch := d.suspiciousRefNameTagValuesForSearch() + if dName == searchName && dSuspiciousTagValueForSearch == searchSuspiciousTagValueForSearch { results = append(results, image.image) continue } // account for registry:/somedir/image - if strings.HasSuffix(d.name, search.name) && d.Tag == search.Tag { + if strings.HasSuffix(dName, searchName) && dSuspiciousTagValueForSearch == searchSuspiciousTagValueForSearch { results = append(results, image.image) continue } } } if len(results) == 0 { - return &storage.Image{}, errors.Errorf("unable to find a name and tag match for %s in repotags", search.name) + return &storage.Image{}, errors.Errorf("unable to find a name and tag match for %s in repotags", searchName) } else if len(results) > 1 { - return &storage.Image{}, errors.Errorf("found multiple name and tag matches for %s in repotags", search.name) + return &storage.Image{}, errors.Errorf("found multiple name and tag matches for %s in repotags", searchName) } return results[0], nil } |