Merge pull request #2117 from mtrmac/no-imageParts

RFC: Mostly replace imageParts

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
 }