We are mistakenly seeing repos as registries.

Currently `podman pull rhel7/rhel-tools` is failing because it
sees rhel7 as a registry.  This change will verify that the returned
registry from the parser is actually a registry and not a repo,
if a repo it will return the correct content, and we will pull the image.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Closes: #1387
Approved by: mtrmac

libpod/image/image_test.go

@@ -221,7 +221,7 @@ func TestNormalizeTag(t *testing.T) {
 		{"example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix + ":none"},           // Qualified name@digest; FIXME: The result is not even syntactically valid!
 		{"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", "ns/busybox:latest"},                                                       // Unqualified with a dot-less namespace FIXME: "ns" is treated as a registry
+		{"ns/busybox:latest", "localhost/ns/busybox:latest"},                                             // Unqualified with a dot-less namespace
 	} {
 		res, err := normalizeTag(c.input)
 		if c.expected == "" {

libpod/image/parts.go

@@ -2,6 +2,7 @@ package image
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/containers/image/docker/reference"
 )
@@ -16,6 +17,11 @@ type imageParts struct {
 	hasRegistry bool
 }
 
+// Registries must contain a ":" or a "." or be localhost
+func isRegistry(name string) bool {
+	return strings.ContainsAny(name, ".:") || name == "localhost"
+}
+
 // decompose breaks an input name into an imageParts description
 func decompose(input string) (imageParts, error) {
 	var (
@@ -37,10 +43,16 @@ func decompose(input string) (imageParts, error) {
 		tag = ntag.Tag()
 	}
 	registry := reference.Domain(imgRef.(reference.Named))
-	if registry != "" {
-		hasRegistry = true
-	}
 	imageName := reference.Path(imgRef.(reference.Named))
+	// Is this a registry or a repo?
+	if isRegistry(registry) {
+		hasRegistry = true
+	} else {
+		if registry != "" {
+			imageName = registry + "/" + imageName
+			registry = ""
+		}
+	}
 	return imageParts{
 		registry:    registry,
 		hasRegistry: hasRegistry,
@@ -53,10 +65,15 @@ func decompose(input string) (imageParts, error) {
 
 // assemble concatenates an image's parts into a string
 func (ip *imageParts) assemble() string {
-	return fmt.Sprintf("%s/%s:%s", ip.registry, ip.name, ip.tag)
+	spec := fmt.Sprintf("%s:%s", ip.name, ip.tag)
+
+	if ip.registry != "" {
+		spec = fmt.Sprintf("%s/%s", ip.registry, spec)
+	}
+	return spec
 }
 
 // assemble concatenates an image's parts with transport into a string
 func (ip *imageParts) assembleWithTransport() string {
-	return fmt.Sprintf("%s%s/%s:%s", ip.transport, ip.registry, ip.name, ip.tag)
+	return fmt.Sprintf("%s%s", ip.transport, ip.assemble())
 }

libpod/image/parts_test.go

@@ -27,12 +27,10 @@ func TestDecompose(t *testing.T) {
 		},
 		{ // Unqualified single-name input
 			"busybox", "docker://", "", "busybox", "latest", false, false,
-			// FIXME? The [empty]/busybox syntax is surprising.
+			"busybox:latest", "docker://busybox:latest",
-			"/busybox:latest", "docker:///busybox:latest",
 		},
 		{ // Unqualified namespaced input
-			// FIXME: .registry == "ns" !!
+			"ns/busybox", "docker://", "", "ns/busybox", "latest", false, false,
-			"ns/busybox", "docker://", "ns", "busybox", "latest", false, true,
 			"ns/busybox:latest", "docker://ns/busybox:latest",
 		},
 		{ // name:tag

libpod/image/pull_test.go

@@ -98,9 +98,8 @@ func TestGetPullRefPair(t *testing.T) {
 			"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", "docker.io/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,
@@ -218,9 +217,8 @@ func TestPullGoalFromImageReference(t *testing.T) {
 			false,
 		},
 		{ // Relative path, multiple elements.
-			// FIXME: This does not add localhost/, so dstName is normalized to docker.io/testdata.
 			"dir:testdata/this-does-not-exist",
-			[]expected{{"testdata/this-does-not-exist", "docker.io/testdata/this-does-not-exist:latest"}},
+			[]expected{{"localhost/testdata/this-does-not-exist:latest", "localhost/testdata/this-does-not-exist:latest"}},
 			false,
 		},
 
@@ -343,12 +341,10 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) {
 		},
 		{ // Unqualified, namespaced, name-only
 			"ns/busybox",
-			// FIXME: This is interpreted as "registry == ns", and actual pull happens from docker.io/ns/busybox:latest;
-			// example.com should be first in the list but isn't used at all.
 			[]pullRefStrings{
-				{"ns/busybox", "docker://ns/busybox:latest", "docker.io/ns/busybox:latest"},
+				{"example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"},
 			},
-			false,
+			true,
 		},
 		{ // Unqualified, name:tag
 			"busybox:notlatest",