Use imageParts.referenceWithRegistry in pullGoalFromPossiblyUnqualifiedName

CHANGES BEHAVIOR.

This bypasses .assemble, and preserves the original
lack of tag / original digest instead of adding :latest/:none
(still subject to ParseStoreReference normalization).

Using the original digest seems clearly correct; dropping the :latest
suffix from .image strings, and adding /library to docker.io/shortname,
 only affects user-visible input; later uses of the return value of
pullImageFrom... use ParseStoreReference, which calls reference.ParseNormalizedNamed
and reference.TagNameOnly, so the image name should be processed
the same way whether it contains a tag, or libray/, or not.

This also allows us to drop the problematic hasShaInInputName heuristic/condition/helper.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

2 files changed, 12 insertions, 20 deletions

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 70f89ea7d..af6b84c34 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"
@@ -272,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) {
@@ -308,17 +301,17 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 	}
 	var refPairs []pullRefPair
 	for _, registry := range searchRegistries {
-		decomposedImage.registry = registry
-		imageName := decomposedImage.assemble()
-		if hasShaInInputName(inputName) {
-			imageName = fmt.Sprintf("%s/%s", registry, inputName)
+		ref, err := decomposedImage.referenceWithRegistry(registry)
+		if err != nil {
+			return nil, err
 		}
+		imageName := ref.String()
 		srcRef, err := docker.ParseReference("//" + imageName)
 		if err != nil {
-			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
+			return nil, errors.Wrapf(err, "unable to parse '%s'", imageName)
 		}
 		ps := pullRefPair{
-			image:  decomposedImage.assemble(),
+			image:  imageName,
 			srcRef: srcRef,
 		}
 		ps.dstRef, err = is.Transport.ParseStoreReference(ir.store, ps.image)
diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go
index 2176678d3..3890c5e6c 100644
--- a/libpod/image/pull_test.go
+++ b/libpod/image/pull_test.go
@@ -328,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,
 		},
@@ -346,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,
 		},