From a6e668fac54b8dcc34b9810738b22fa7c8561325 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Mon, 14 Jan 2019 03:49:08 +0100
Subject: Don't call image.DecomposeString in imageInListToContainerImage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- It is used to fill Repository(misnamed)/Tag members which have no users;
  so it's completely unclear why this is useful.
- Given the mishandling of tags by imageParts.tag, at the very least
  all new code should primarily use reference.Named (even if
  after a decompose() to internally deal with unqualified names first),
  introducing new uses of original decompose() just reintroduces known
  trouble - so without any provided rationale, reverting seems
  a reasonable default action.
- This drags in all of libpod/image into the "remote client" build,
  which seems undesirable.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/adapter/runtime_remote.go | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

(limited to 'libpod')

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,
-- 
cgit v1.2.3-54-g00ecf


From 6486e2c41be0aa50bd5b70d859a23f36070d0ae1 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Mon, 14 Jan 2019 03:58:35 +0100
Subject: Drop image.DecomposeString, make image.Parts private imageParts again
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Now that DecomposeString has no users, make the type private again.
Any new users of it should come with a rationale - and new users
of the "none"/"latest" handling of untagged/digested names that is
currently implemented should have an exceptionaly unusual rationale.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go      |  8 ++++----
 libpod/image/parts.go      | 36 +++++++++++++++---------------------
 libpod/image/parts_test.go |  4 ++--
 libpod/image/pull.go       |  4 ++--
 libpod/image/utils.go      |  6 +++---
 5 files changed, 26 insertions(+), 32 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index dda753385..b51aced49 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"
@@ -460,7 +460,7 @@ func normalizeTag(tag string) (string, error) {
 	}
 	// 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)
+		tag = fmt.Sprintf("%s:%s", tag, decomposedTag.tag)
 	}
 	// If the input doesn't specify a registry, set the registry to localhost
 	if !decomposedTag.hasRegistry {
@@ -937,7 +937,7 @@ func (i *Image) MatchRepoTag(input string) (string, error) {
 		if err != nil {
 			return "", err
 		}
-		if dcRepoName.Registry == dcImage.Registry && dcImage.Registry != "" {
+		if dcRepoName.registry == dcImage.registry && dcImage.registry != "" {
 			count++
 		}
 		if dcRepoName.name == dcImage.name && dcImage.name != "" {
@@ -945,7 +945,7 @@ func (i *Image) MatchRepoTag(input string) (string, error) {
 		} else if splitString(dcRepoName.name) == splitString(dcImage.name) {
 			count++
 		}
-		if dcRepoName.Tag == dcImage.Tag {
+		if dcRepoName.tag == dcImage.tag {
 			count++
 		}
 		results[count] = append(results[count], repoName)
diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index b2a69f26c..9adf26fb9 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -7,12 +7,12 @@ import (
 	"github.com/containers/image/docker/reference"
 )
 
-// Parts describes the parts of an image's name
-type Parts struct {
+// imageParts describes the parts of an image's name
+type imageParts struct {
 	transport   string
-	Registry    string
+	registry    string
 	name        string
-	Tag         string
+	tag         string
 	isTagged    bool
 	hasRegistry bool
 }
@@ -34,16 +34,10 @@ func GetImageBaseName(input string) (string, error) {
 	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) {
+func decompose(input string) (imageParts, error) {
 	var (
-		parts       Parts
+		parts       imageParts
 		hasRegistry bool
 		tag         string
 	)
@@ -62,7 +56,7 @@ func decompose(input string) (Parts, error) {
 	}
 	registry := reference.Domain(imgRef.(reference.Named))
 	imageName := reference.Path(imgRef.(reference.Named))
-	// Is this a Registry or a repo?
+	// Is this a registry or a repo?
 	if isRegistry(registry) {
 		hasRegistry = true
 	} else {
@@ -71,27 +65,27 @@ func decompose(input string) (Parts, error) {
 			registry = ""
 		}
 	}
-	return Parts{
-		Registry:    registry,
+	return imageParts{
+		registry:    registry,
 		hasRegistry: hasRegistry,
 		name:        imageName,
-		Tag:         tag,
+		tag:         tag,
 		isTagged:    isTagged,
 		transport:   DefaultTransport,
 	}, nil
 }
 
 // assemble concatenates an image's parts into a string
-func (ip *Parts) assemble() string {
-	spec := fmt.Sprintf("%s:%s", ip.name, ip.Tag)
+func (ip *imageParts) assemble() string {
+	spec := fmt.Sprintf("%s:%s", ip.name, ip.tag)
 
-	if ip.Registry != "" {
-		spec = fmt.Sprintf("%s/%s", ip.Registry, spec)
+	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 *Parts) assembleWithTransport() string {
+func (ip *imageParts) assembleWithTransport() string {
 	return fmt.Sprintf("%s%s", ip.transport, ip.assemble())
 }
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index 1e01c85d3..518538f0b 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -55,9 +55,9 @@ func TestDecompose(t *testing.T) {
 		} 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.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.tag, parts.tag, c.input)
 			assert.Equal(t, c.isTagged, parts.isTagged, c.input)
 			assert.Equal(t, c.hasRegistry, parts.hasRegistry, c.input)
 			assert.Equal(t, c.assembled, parts.assemble(), c.input)
diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 203e94310..09935fe7c 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -76,7 +76,7 @@ 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.registry = DefaultLocalRegistry
 		decomposedDest.hasRegistry = true
 		destName = decomposedDest.assemble()
 	}
@@ -317,7 +317,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 	}
 	var refPairs []pullRefPair
 	for _, registry := range searchRegistries {
-		decomposedImage.Registry = registry
+		decomposedImage.registry = registry
 		imageName := decomposedImage.assembleWithTransport()
 		if hasShaInInputName(inputName) {
 			imageName = fmt.Sprintf("%s%s/%s", decomposedImage.transport, registry, inputName)
diff --git a/libpod/image/utils.go b/libpod/image/utils.go
index 135b47008..b944de1bb 100644
--- a/libpod/image/utils.go
+++ b/libpod/image/utils.go
@@ -16,7 +16,7 @@ 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) {
 	var results []*storage.Image
 	for _, image := range images {
 		for _, name := range image.Names() {
@@ -25,12 +25,12 @@ func findImageInRepotags(search Parts, images []*Image) (*storage.Image, error)
 			if err != nil {
 				continue
 			}
-			if d.name == search.name && d.Tag == search.Tag {
+			if d.name == search.name && d.tag == search.tag {
 				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(d.name, search.name) && d.tag == search.tag {
 				results = append(results, image.image)
 				continue
 			}
-- 
cgit v1.2.3-54-g00ecf


From c19294c0113e4ddea7005bab5d8a31f7ecd6c120 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 18:28:28 +0100
Subject: Record the original reference.Named in imageParts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We will eventually want to eliminate most members of imageParts
in favor of using the c/image/docker/reference API directly.

For now, just record the reference.Named value, and we will
replace uses of the other members before removing them.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index 9adf26fb9..289a02e1f 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -9,12 +9,13 @@ import (
 
 // imageParts describes the parts of an image's name
 type imageParts struct {
-	transport   string
-	registry    string
-	name        string
-	tag         string
-	isTagged    bool
-	hasRegistry bool
+	unnormalizedRef reference.Named // WARNING: Did not go through docker.io[/library] normalization
+	transport       string
+	registry        string
+	name            string
+	tag             string
+	isTagged        bool
+	hasRegistry     bool
 }
 
 // Registries must contain a ":" or a "." or be localhost
@@ -45,6 +46,7 @@ func decompose(input string) (imageParts, error) {
 	if err != nil {
 		return parts, err
 	}
+	unnormalizedNamed := imgRef.(reference.Named)
 	ntag, isTagged := imgRef.(reference.NamedTagged)
 	if !isTagged {
 		tag = "latest"
@@ -54,8 +56,8 @@ func decompose(input string) (imageParts, error) {
 	} else {
 		tag = ntag.Tag()
 	}
-	registry := reference.Domain(imgRef.(reference.Named))
-	imageName := reference.Path(imgRef.(reference.Named))
+	registry := reference.Domain(unnormalizedNamed)
+	imageName := reference.Path(unnormalizedNamed)
 	// Is this a registry or a repo?
 	if isRegistry(registry) {
 		hasRegistry = true
@@ -66,12 +68,13 @@ func decompose(input string) (imageParts, error) {
 		}
 	}
 	return imageParts{
-		registry:    registry,
-		hasRegistry: hasRegistry,
-		name:        imageName,
-		tag:         tag,
-		isTagged:    isTagged,
-		transport:   DefaultTransport,
+		unnormalizedRef: unnormalizedNamed,
+		registry:        registry,
+		hasRegistry:     hasRegistry,
+		name:            imageName,
+		tag:             tag,
+		isTagged:        isTagged,
+		transport:       DefaultTransport,
 	}, nil
 }
 
-- 
cgit v1.2.3-54-g00ecf


From 3d98c42a3f7822bf608834505adcffec7f75bf28 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 18:47:09 +0100
Subject: Inline imageParts.assembleWithTransport into callers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

imageParts.transport is a constant, and the design of imageParts
is not transport-independent in any sense; we will want to eliminate
the transport member entirely.

As a first step, drop assembleWithTransport and inline an exact
equivalent into all callers.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go      |  5 -----
 libpod/image/parts_test.go | 18 ++++++++----------
 libpod/image/pull.go       |  4 ++--
 3 files changed, 10 insertions(+), 17 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index 289a02e1f..cf774030b 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -87,8 +87,3 @@ func (ip *imageParts) assemble() string {
 	}
 	return spec
 }
-
-// assemble concatenates an image's parts with transport into a string
-func (ip *imageParts) assembleWithTransport() string {
-	return fmt.Sprintf("%s%s", ip.transport, ip.assemble())
-}
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index 518538f0b..3ff175f0e 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -14,39 +14,38 @@ func TestDecompose(t *testing.T) {
 		transport, registry, name, tag string
 		isTagged, hasRegistry          bool
 		assembled                      string
-		assembledWithTransport         string
 	}{
-		{"#", "", "", "", "", false, false, "", ""}, // Entirely invalid input
+		{"#", "", "", "", "", false, 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:latest",
 		},
 		{ // 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:latest",
 		},
 		{ // Unqualified single-name input
 			"busybox", "docker://", "", "busybox", "latest", false, false,
-			"busybox:latest", "docker://busybox:latest",
+			"busybox:latest",
 		},
 		{ // Unqualified namespaced input
 			"ns/busybox", "docker://", "", "ns/busybox", "latest", false, false,
-			"ns/busybox:latest", "docker://ns/busybox:latest",
+			"ns/busybox:latest",
 		},
 		{ // 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",
 		},
 		{ // 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",
+			"example.com/ns/busybox:none",
 		},
 		{ // 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",
 		},
 	} {
 		parts, err := decompose(c.input)
@@ -61,7 +60,6 @@ func TestDecompose(t *testing.T) {
 			assert.Equal(t, c.isTagged, parts.isTagged, 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)
 		}
 	}
 }
diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 09935fe7c..82fe0fdd6 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -288,7 +288,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		if hasShaInInputName(inputName) {
 			imageName = fmt.Sprintf("%s%s", decomposedImage.transport, inputName)
 		} else {
-			imageName = decomposedImage.assembleWithTransport()
+			imageName = fmt.Sprintf("%s%s", decomposedImage.transport, decomposedImage.assemble())
 		}
 		srcRef, err := alltransports.ParseImageName(imageName)
 		if err != nil {
@@ -318,7 +318,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 	var refPairs []pullRefPair
 	for _, registry := range searchRegistries {
 		decomposedImage.registry = registry
-		imageName := decomposedImage.assembleWithTransport()
+		imageName := fmt.Sprintf("%s%s", decomposedImage.transport, decomposedImage.assemble())
 		if hasShaInInputName(inputName) {
 			imageName = fmt.Sprintf("%s%s/%s", decomposedImage.transport, registry, inputName)
 		}
-- 
cgit v1.2.3-54-g00ecf


From 99d2259f8a71dbe5a0d9e78757b51551ce0eb8b1 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 18:54:17 +0100
Subject: Simplify pullGoalFromPossiblyUnqualifiedName
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After inlining assembleWithTransport, we have two branches with
the same prepending of decomposedImage.transport; move that out of
the branches.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/pull.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 82fe0fdd6..d09ddd599 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -286,11 +286,11 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 	if decomposedImage.hasRegistry {
 		var imageName, destName string
 		if hasShaInInputName(inputName) {
-			imageName = fmt.Sprintf("%s%s", decomposedImage.transport, inputName)
+			imageName = inputName
 		} else {
-			imageName = fmt.Sprintf("%s%s", decomposedImage.transport, decomposedImage.assemble())
+			imageName = decomposedImage.assemble()
 		}
-		srcRef, err := alltransports.ParseImageName(imageName)
+		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", decomposedImage.transport, imageName))
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
@@ -318,11 +318,11 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 	var refPairs []pullRefPair
 	for _, registry := range searchRegistries {
 		decomposedImage.registry = registry
-		imageName := fmt.Sprintf("%s%s", decomposedImage.transport, decomposedImage.assemble())
+		imageName := decomposedImage.assemble()
 		if hasShaInInputName(inputName) {
-			imageName = fmt.Sprintf("%s%s/%s", decomposedImage.transport, registry, inputName)
+			imageName = fmt.Sprintf("%s/%s", registry, inputName)
 		}
-		srcRef, err := alltransports.ParseImageName(imageName)
+		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", decomposedImage.transport, imageName))
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
-- 
cgit v1.2.3-54-g00ecf


From e9721b757ae5da4dad4f68c5eb721f348cfe935a Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 19:00:59 +0100
Subject: Remove imageParts.transport
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It is only ever set to DefaulTransport, and all of the code
is docker/reference-specific anyway, so there's no point in
making this a variable.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go      |  2 --
 libpod/image/parts_test.go | 27 +++++++++++++--------------
 libpod/image/pull.go       |  4 ++--
 3 files changed, 15 insertions(+), 18 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index cf774030b..b7c5d1252 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -10,7 +10,6 @@ import (
 // imageParts describes the parts of an image's name
 type imageParts struct {
 	unnormalizedRef reference.Named // WARNING: Did not go through docker.io[/library] normalization
-	transport       string
 	registry        string
 	name            string
 	tag             string
@@ -74,7 +73,6 @@ func decompose(input string) (imageParts, error) {
 		name:            imageName,
 		tag:             tag,
 		isTagged:        isTagged,
-		transport:       DefaultTransport,
 	}, nil
 }
 
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index 3ff175f0e..733e8e855 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -10,50 +10,49 @@ 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
+		input                 string
+		registry, name, tag   string
+		isTagged, hasRegistry bool
+		assembled             string
 	}{
-		{"#", "", "", "", "", false, false, ""}, // Entirely invalid input
+		{"#", "", "", "", false, 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", "docker.io", "library/busybox", "latest", false, true,
 			"docker.io/library/busybox:latest",
 		},
 		{ // Fully qualified example.com, name-only input
-			"example.com/ns/busybox", "docker://", "example.com", "ns/busybox", "latest", false, true,
+			"example.com/ns/busybox", "example.com", "ns/busybox", "latest", false, true,
 			"example.com/ns/busybox:latest",
 		},
 		{ // Unqualified single-name input
-			"busybox", "docker://", "", "busybox", "latest", false, false,
+			"busybox", "", "busybox", "latest", false, false,
 			"busybox:latest",
 		},
 		{ // Unqualified namespaced input
-			"ns/busybox", "docker://", "", "ns/busybox", "latest", false, false,
+			"ns/busybox", "", "ns/busybox", "latest", false, false,
 			"ns/busybox:latest",
 		},
 		{ // name:tag
-			"example.com/ns/busybox:notlatest", "docker://", "example.com", "ns/busybox", "notlatest", true, true,
+			"example.com/ns/busybox:notlatest", "example.com", "ns/busybox", "notlatest", true, true,
 			"example.com/ns/busybox:notlatest",
 		},
 		{ // name@digest
 			// FIXME? .tag == "none"
-			"example.com/ns/busybox" + digestSuffix, "docker://", "example.com", "ns/busybox", "none", false, true,
+			"example.com/ns/busybox" + digestSuffix, "example.com", "ns/busybox", "none", false, true,
 			// FIXME: this drops the digest and replaces it with an incorrect tag.
 			"example.com/ns/busybox:none",
 		},
 		{ // name:tag@digest
-			"example.com/ns/busybox:notlatest" + digestSuffix, "docker://", "example.com", "ns/busybox", "notlatest", true, true,
+			"example.com/ns/busybox:notlatest" + digestSuffix, "example.com", "ns/busybox", "notlatest", true, true,
 			// FIXME: This drops the digest
 			"example.com/ns/busybox:notlatest",
 		},
 	} {
 		parts, err := decompose(c.input)
-		if c.transport == "" {
+		if c.assembled == "" {
 			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)
diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index d09ddd599..342ee6f17 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -290,7 +290,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		} else {
 			imageName = decomposedImage.assemble()
 		}
-		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", decomposedImage.transport, imageName))
+		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", DefaultTransport, imageName))
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
@@ -322,7 +322,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		if hasShaInInputName(inputName) {
 			imageName = fmt.Sprintf("%s/%s", registry, inputName)
 		}
-		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", decomposedImage.transport, imageName))
+		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", DefaultTransport, imageName))
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
-- 
cgit v1.2.3-54-g00ecf


From 788bc360219e83028b3b0996ab4b6afbd0530e9c Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 19:12:17 +0100
Subject: Simplify pullGoalFromPossiblyUnqualifiedName
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both imageParts and this function implicitly assume docker.Transport
troughout, so instead of pretending to be flexible about DefaultTransport,
just hard-code docker.ParseReference directly.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/pull.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 342ee6f17..d4be53ef8 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -290,7 +290,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		} else {
 			imageName = decomposedImage.assemble()
 		}
-		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", DefaultTransport, imageName))
+		srcRef, err := docker.ParseReference("//" + imageName)
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
@@ -322,7 +322,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		if hasShaInInputName(inputName) {
 			imageName = fmt.Sprintf("%s/%s", registry, inputName)
 		}
-		srcRef, err := alltransports.ParseImageName(fmt.Sprintf("%s%s", DefaultTransport, imageName))
+		srcRef, err := docker.ParseReference("//" + imageName)
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
-- 
cgit v1.2.3-54-g00ecf


From 035c732dedf37637b9ac5e37748ec1b0bea3c966 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 19:49:53 +0100
Subject: Reorganize normalizeTag
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Move the registry defaulting before tag defaulting.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index b51aced49..249beceac 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -458,14 +458,14 @@ func normalizeTag(tag string) (string, error) {
 	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)
-	}
 	// If the input doesn't specify a registry, set the registry to localhost
 	if !decomposedTag.hasRegistry {
 		tag = fmt.Sprintf("%s/%s", DefaultLocalRegistry, tag)
 	}
+	// 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 tag, nil
 }
 
-- 
cgit v1.2.3-54-g00ecf


From ae2a95196e9e5a3519b396105d01a3661de330ba Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 20:02:54 +0100
Subject: Don't use imageParts.assemble when pulling from a qualified name
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CHANGES BEHAVIOR.

If the name is qualified, instead of decomposing it into components and
re-assembling, just use the input name unmodified:
- For name:tag values, .assemble() just recreates the input.
- For untagged values, .assemble() adds ":latest"; we keep
  the input as is, but both docker.ParseReference and storage.Transport.ParseStoreReference
  use reference.TagNameOnly() already.
- For digested references, .assemble() adds ":none", but
  the code was already bypassing .assemble() on that path
  already - for the source reference.  For the destination,
  this replaces a :none destination with a the @digest reference,
  as expected.

Note that while decompose() has already parsed the input,
it (intentionally) bypassed the docker.io/library normalization;
therefore we parse the input again (via docker.ParseReference) to ensure
that the reference is normalized.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/pull.go      | 17 +++--------------
 libpod/image/pull_test.go |  3 +--
 2 files changed, 4 insertions(+), 16 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index d4be53ef8..ee5e333e3 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -284,24 +284,13 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		return nil, err
 	}
 	if decomposedImage.hasRegistry {
-		var imageName, destName string
-		if hasShaInInputName(inputName) {
-			imageName = inputName
-		} else {
-			imageName = decomposedImage.assemble()
-		}
-		srcRef, err := docker.ParseReference("//" + 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)
+		destRef, err := is.Transport.ParseStoreReference(ir.store, inputName)
 		if err != nil {
-			return nil, errors.Wrapf(err, "error parsing dest reference name %#v", destName)
+			return nil, errors.Wrapf(err, "error parsing dest reference name %#v", inputName)
 		}
 		ps := pullRefPair{
 			image:  inputName,
diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go
index 37b45dc83..4a0119070 100644
--- a/libpod/image/pull_test.go
+++ b/libpod/image/pull_test.go
@@ -324,8 +324,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.
-- 
cgit v1.2.3-54-g00ecf


From 72777b7fee22e04edee08034927d5864ffc4bc3e Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 20:32:30 +0100
Subject: Add imageParts.referenceWithRegistry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is the primary goal of decompose()+assemble(), to support
qualifying an image name.

Does not have any users yet, so does not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go      | 18 ++++++++++++++++++
 libpod/image/parts_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index b7c5d1252..8d059e35b 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/containers/image/docker/reference"
+	"github.com/pkg/errors"
 )
 
 // imageParts describes the parts of an image's name
@@ -76,6 +77,23 @@ func decompose(input string) (imageParts, error) {
 	}, nil
 }
 
+// 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)
+	}
+	// 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 into a string
 func (ip *imageParts) assemble() string {
 	spec := fmt.Sprintf("%s:%s", ip.name, ip.tag)
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index 733e8e855..b2cc0f8c0 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDecompose(t *testing.T) {
@@ -62,3 +63,44 @@ func TestDecompose(t *testing.T) {
 		}
 	}
 }
+
+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)
+}
-- 
cgit v1.2.3-54-g00ecf


From 2171a393904051ab724fa08d01a964adbf7c1880 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 20:43:13 +0100
Subject: Use imageParts.referenceWithRegistry in getPullRefPair
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 only affects user-visible input; later
uses of the return value of pullImageFrom... use ParseStoreReference,
which calls reference.TagNameOnly, so the image name should be processed
the same way whether it contains a tag or not.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/pull.go      |  8 +++++---
 libpod/image/pull_test.go | 12 ++++--------
 2 files changed, 9 insertions(+), 11 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index ee5e333e3..70f89ea7d 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -76,9 +76,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
diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go
index 4a0119070..2176678d3 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,
 		},
 
-- 
cgit v1.2.3-54-g00ecf


From 81204487dbf9903f4a8784be9a580f8a46d1d381 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 20:55:53 +0100
Subject: Use imageParts.referenceWithRegistry in
 pullGoalFromPossiblyUnqualifiedName
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 libpod/image/pull.go      | 19 ++++++-------------
 libpod/image/pull_test.go | 13 ++++++-------
 2 files changed, 12 insertions(+), 20 deletions(-)

(limited to 'libpod')

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,
 		},
-- 
cgit v1.2.3-54-g00ecf


From 633501b1b7450676c47d891092b53680206b8398 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:01:56 +0100
Subject: Use getPullRefPair / getSinglePullRefPairGoal in
 pullGoalFromPossiblyUnqualifiedName
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This shortens the code a bit, but most importantly ensures that all pulls from
docker.Transport are processed exactly the same way, and there is only a single
store.ParseStoreReference in the pull code.

It's a bit wasteful to call decompose() in getPullRefPair just after
pullGoalFromPossiblyUnqualifiedName has qualified the name, but on balance
only having exactly one code path seems worth it.  Alternatively we could
split getPullRefPairToQualifiedDestination from getPullRefPair.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/pull.go | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index af6b84c34..434b83520 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -283,16 +283,7 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", inputName)
 		}
-		destRef, err := is.Transport.ParseStoreReference(ir.store, inputName)
-		if err != nil {
-			return nil, errors.Wrapf(err, "error parsing dest reference name %#v", inputName)
-		}
-		ps := pullRefPair{
-			image:  inputName,
-			srcRef: srcRef,
-			dstRef: destRef,
-		}
-		return singlePullRefPairGoal(ps), nil
+		return ir.getSinglePullRefPairGoal(srcRef, inputName)
 	}
 
 	searchRegistries, err := registries.GetRegistries()
@@ -310,13 +301,9 @@ func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullG
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to parse '%s'", imageName)
 		}
-		ps := pullRefPair{
-			image:  imageName,
-			srcRef: srcRef,
-		}
-		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)
 	}
-- 
cgit v1.2.3-54-g00ecf


From e5c764ec3cc11982cdb8e127554e4ac61ce9a854 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:11:32 +0100
Subject: Remove no longer used imageParts.assemble()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go      | 11 -----------
 libpod/image/parts_test.go | 15 ++-------------
 2 files changed, 2 insertions(+), 24 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index 8d059e35b..566a3842c 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -1,7 +1,6 @@
 package image
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/containers/image/docker/reference"
@@ -93,13 +92,3 @@ func (ip *imageParts) referenceWithRegistry(registry string) (reference.Named, e
 	}
 	return ref, nil
 }
-
-// assemble concatenates an image's parts into a string
-func (ip *imageParts) assemble() string {
-	spec := fmt.Sprintf("%s:%s", ip.name, ip.tag)
-
-	if ip.registry != "" {
-		spec = fmt.Sprintf("%s/%s", ip.registry, spec)
-	}
-	return spec
-}
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index b2cc0f8c0..3ca39b1dc 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -14,43 +14,33 @@ func TestDecompose(t *testing.T) {
 		input                 string
 		registry, name, tag   string
 		isTagged, hasRegistry bool
-		assembled             string
 	}{
-		{"#", "", "", "", false, false, ""}, // Entirely invalid input
+		{"#", "", "", "", false, false}, // Entirely invalid input
 		{ // Fully qualified docker.io, name-only input
 			"docker.io/library/busybox", "docker.io", "library/busybox", "latest", false, true,
-			"docker.io/library/busybox:latest",
 		},
 		{ // Fully qualified example.com, name-only input
 			"example.com/ns/busybox", "example.com", "ns/busybox", "latest", false, true,
-			"example.com/ns/busybox:latest",
 		},
 		{ // Unqualified single-name input
 			"busybox", "", "busybox", "latest", false, false,
-			"busybox:latest",
 		},
 		{ // Unqualified namespaced input
 			"ns/busybox", "", "ns/busybox", "latest", false, false,
-			"ns/busybox:latest",
 		},
 		{ // name:tag
 			"example.com/ns/busybox:notlatest", "example.com", "ns/busybox", "notlatest", true, true,
-			"example.com/ns/busybox:notlatest",
 		},
 		{ // name@digest
 			// FIXME? .tag == "none"
 			"example.com/ns/busybox" + digestSuffix, "example.com", "ns/busybox", "none", false, true,
-			// FIXME: this drops the digest and replaces it with an incorrect tag.
-			"example.com/ns/busybox:none",
 		},
 		{ // name:tag@digest
 			"example.com/ns/busybox:notlatest" + digestSuffix, "example.com", "ns/busybox", "notlatest", true, true,
-			// FIXME: This drops the digest
-			"example.com/ns/busybox:notlatest",
 		},
 	} {
 		parts, err := decompose(c.input)
-		if c.assembled == "" {
+		if c.name == "" {
 			assert.Error(t, err, c.input)
 		} else {
 			assert.NoError(t, err, c.input)
@@ -59,7 +49,6 @@ func TestDecompose(t *testing.T) {
 			assert.Equal(t, c.tag, parts.tag, c.input)
 			assert.Equal(t, c.isTagged, parts.isTagged, c.input)
 			assert.Equal(t, c.hasRegistry, parts.hasRegistry, c.input)
-			assert.Equal(t, c.assembled, parts.assemble(), c.input)
 		}
 	}
 }
-- 
cgit v1.2.3-54-g00ecf


From e58aa74766c14844700330e11e8f0b48843884be Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:18:53 +0100
Subject: Use imageparts.referenceWithRegistry in normalizeTag
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... instead of open-coding something similar.  Eventually
we will use the reference type further in here.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 249beceac..3d8cf2651 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -460,7 +460,11 @@ func normalizeTag(tag string) (string, error) {
 	}
 	// If the input doesn't specify a registry, set the registry to localhost
 	if !decomposedTag.hasRegistry {
-		tag = fmt.Sprintf("%s/%s", DefaultLocalRegistry, tag)
+		ref, err := decomposedTag.referenceWithRegistry(DefaultLocalRegistry)
+		if err != nil {
+			return "", err
+		}
+		tag = ref.String()
 	}
 	// If the input does not have a tag, we need to add one (latest)
 	if !decomposedTag.isTagged {
-- 
cgit v1.2.3-54-g00ecf


From 1c19d19c6ef9627413e34b30a643bef9b2970acb Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:33:01 +0100
Subject: Add imageParts.normalizedReference()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This will be used in normalizeTag to work with references instead of strings.

Not used anywhere yet, should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go      | 14 ++++++++++++++
 libpod/image/parts_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index 566a3842c..a4eb57519 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -92,3 +92,17 @@ func (ip *imageParts) referenceWithRegistry(registry string) (reference.Named, e
 	}
 	return ref, nil
 }
+
+// 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 3ca39b1dc..9955a20ea 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -93,3 +93,31 @@ func TestImagePartsReferenceWithRegistry(t *testing.T) {
 	_, 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())
+		}
+	}
+}
-- 
cgit v1.2.3-54-g00ecf


From e060a19c8768c9dd65d1b8dcbd6da7ec9faa1163 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:35:04 +0100
Subject: Use imageParts.normalizedReference in normalizeTag
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is another step to using reference values instead of strings here.

CHANGES BEHAVIOR: docker.io/busybox is now normalized to docker.io/library/busybox.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go      | 10 ++++++++--
 libpod/image/image_test.go |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 3d8cf2651..2457a1c22 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -459,13 +459,19 @@ func normalizeTag(tag string) (string, error) {
 		return "", err
 	}
 	// If the input doesn't specify a registry, set the registry to localhost
+	var ref reference.Named
 	if !decomposedTag.hasRegistry {
-		ref, err := decomposedTag.referenceWithRegistry(DefaultLocalRegistry)
+		ref, err = decomposedTag.referenceWithRegistry(DefaultLocalRegistry)
+		if err != nil {
+			return "", err
+		}
+	} else {
+		ref, err = decomposedTag.normalizedReference()
 		if err != nil {
 			return "", err
 		}
-		tag = ref.String()
 	}
+	tag = ref.String()
 	// 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)
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index 2a68fd273..a85cd5386 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -268,6 +268,7 @@ func TestNormalizeTag(t *testing.T) {
 		{"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)
 		if c.expected == "" {
-- 
cgit v1.2.3-54-g00ecf


From b9c0f2c987688a8e7ee2c8f4e4fbf6b1a8fee024 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:38:49 +0100
Subject: Use reference.TagNameOnly instead of manually adding imageParts.tag
 in normalizeTag
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Again, rely on the official API, instead of the suprising "suspiciousTagValueForSearch"
value (set to :latest on untagged images, and :none on digested ones!)

CHANGES BEHAVIOR, but the previous output of normalization of digested values was
not even syntatically valid, so this can't really be worse.

Still, maybe we should refuse to tag with digested references in the first place.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go      | 7 ++-----
 libpod/image/image_test.go | 2 +-
 2 files changed, 3 insertions(+), 6 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 2457a1c22..0b743e144 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -471,12 +471,9 @@ func normalizeTag(tag string) (string, error) {
 			return "", err
 		}
 	}
-	tag = ref.String()
 	// 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 tag, nil
+	ref = reference.TagNameOnly(ref)
+	return ref.String(), nil
 }
 
 // TagImage adds a tag to the given image
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index a85cd5386..96148c2cd 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -264,7 +264,7 @@ func TestNormalizeTag(t *testing.T) {
 		{"#", ""}, // 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
-- 
cgit v1.2.3-54-g00ecf


From f92c3ce35033a558a9c86c824a829368a472f873 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:42:37 +0100
Subject: Return a reference.Named from normalizedTag
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Instead of returning a string, return a native value and convert it
into the string in the caller, to make it that small bit more
common to use reference types.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go      | 18 +++++++++---------
 libpod/image/image_test.go |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 0b743e144..7442f1945 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -452,42 +452,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
+		return nil, err
 	}
 	// If the input doesn't specify a registry, set the registry to localhost
 	var ref reference.Named
 	if !decomposedTag.hasRegistry {
 		ref, err = decomposedTag.referenceWithRegistry(DefaultLocalRegistry)
 		if err != nil {
-			return "", err
+			return nil, err
 		}
 	} else {
 		ref, err = decomposedTag.normalizedReference()
 		if err != nil {
-			return "", err
+			return nil, err
 		}
 	}
 	// If the input does not have a tag, we need to add one (latest)
 	ref = reference.TagNameOnly(ref)
-	return ref.String(), nil
+	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
 	}
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index 96148c2cd..077ae460e 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -257,7 +257,7 @@ 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 }{
@@ -270,12 +270,12 @@ func TestNormalizeTag(t *testing.T) {
 		{"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())
 		}
 	}
 }
-- 
cgit v1.2.3-54-g00ecf


From d559365d7aea4f9957e3efb869480aaeb33b9ec6 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:44:05 +0100
Subject: Don't try to look up local images with an explicit :latest suffix
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

imageruntime.getImage, through ParseStoreReference, already uses
reference.TagNameOnly on the input, so this extra lookup is completely
redundant to the lookup that has already happened.

Should not change behavior, apart from speeding up the code a bit.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go | 10 ----------
 1 file changed, 10 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 7442f1945..899b8792f 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -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,15 +239,6 @@ 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 {
-- 
cgit v1.2.3-54-g00ecf


From cf40b7161476414e7abecd19d9d9eefacd8a4ef7 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:50:40 +0100
Subject: Use imageParts.referenceWithRegistry in Image.getLocalImage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... to make sure everything uses the same code path.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 899b8792f..381c28fac 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -243,12 +243,14 @@ func (i *Image) getLocalImage() (*storage.Image, error) {
 	// 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
 	}
-- 
cgit v1.2.3-54-g00ecf


From fa42f97507f5b5c661df2339603383b00293689f Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 19:41:50 +0100
Subject: FIXME? Introduce imageParts.suspiciousRefNameTagValuesForSearch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Image.MatchRepoTag and findImageInRepoTags do some kind of
heuristic search; the motivation and design of both, and how they
should deal with digests, is not obvious to me.

Instead of figuring that out now, just factor it out into a
scary-named method and leave the "tag" value (with its "latest"/"none"
value) alone.

Similarly, the .registry and .name fields should typically not be used;
users should use either hasRegistry or normalized reference types;
so, isolate the difficult-to-understand search code, and computation
of these values, into this new search-specific helper.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/image.go      | 10 ++++++----
 libpod/image/parts.go      | 24 ++++++++++++++++++++++++
 libpod/image/parts_test.go | 15 ++++++++-------
 libpod/image/utils.go      | 10 ++++++----
 4 files changed, 44 insertions(+), 15 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 381c28fac..ea326d820 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -930,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/parts.go b/libpod/image/parts.go
index a4eb57519..ab6a86767 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -76,6 +76,30 @@ func decompose(input string) (imageParts, error) {
 	}, 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 = ""
+	}
+
+	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 {
+		tag = "latest"
+	}
+	return registry, imageName, tag
+}
+
 // referenceWithRegistry returns a (normalized) reference.Named composed of ip (with !ip.hasRegistry)
 // qualified with registry.
 func (ip *imageParts) referenceWithRegistry(registry string) (reference.Named, error) {
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index 9955a20ea..dd4a7b27f 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -11,9 +11,9 @@ func TestDecompose(t *testing.T) {
 	const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
 
 	for _, c := range []struct {
-		input                 string
-		registry, name, tag   string
-		isTagged, hasRegistry bool
+		input                                       string
+		registry, name, suspiciousTagValueForSearch string
+		isTagged, hasRegistry                       bool
 	}{
 		{"#", "", "", "", false, false}, // Entirely invalid input
 		{ // Fully qualified docker.io, name-only input
@@ -32,7 +32,7 @@ func TestDecompose(t *testing.T) {
 			"example.com/ns/busybox:notlatest", "example.com", "ns/busybox", "notlatest", true, true,
 		},
 		{ // name@digest
-			// FIXME? .tag == "none"
+			// FIXME? .suspiciousTagValueForSearch == "none"
 			"example.com/ns/busybox" + digestSuffix, "example.com", "ns/busybox", "none", false, true,
 		},
 		{ // name:tag@digest
@@ -44,9 +44,10 @@ func TestDecompose(t *testing.T) {
 			assert.Error(t, err, c.input)
 		} else {
 			assert.NoError(t, err, 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)
+			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.isTagged, parts.isTagged, c.input)
 			assert.Equal(t, c.hasRegistry, parts.hasRegistry, c.input)
 		}
diff --git a/libpod/image/utils.go b/libpod/image/utils.go
index b944de1bb..ad027f32a 100644
--- a/libpod/image/utils.go
+++ b/libpod/image/utils.go
@@ -17,6 +17,7 @@ import (
 // findImageInRepotags takes an imageParts struct and searches images' repotags for
 // a match on name:tag
 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 imageParts, images []*Image) (*storage.Image, er
 			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
 }
-- 
cgit v1.2.3-54-g00ecf


From ad90c44f8d648ba17f07db7f05f28a67c16796d0 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 22:10:53 +0100
Subject: Use imageParts.unnormalizedRef in GetImageBaseName
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... to remove the last user of imageParts.name.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index ab6a86767..b5b05c32d 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -30,7 +30,7 @@ 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
 }
 
-- 
cgit v1.2.3-54-g00ecf


From 797d194050d4b7b78d255d19f4099463dc048425 Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 22:18:25 +0100
Subject: Clarify comments about isRegistry a bit.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index b5b05c32d..62cca2757 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -17,7 +17,8 @@ type imageParts struct {
 	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"
 }
@@ -57,7 +58,9 @@ func decompose(input string) (imageParts, error) {
 	}
 	registry := reference.Domain(unnormalizedNamed)
 	imageName := reference.Path(unnormalizedNamed)
-	// Is this a registry or a repo?
+	// 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.
 	if isRegistry(registry) {
 		hasRegistry = true
 	} else {
-- 
cgit v1.2.3-54-g00ecf


From 449116af19305a567933effaced5a0a21a8c71be Mon Sep 17 00:00:00 2001
From: Miloslav Trmač <mitr@redhat.com>
Date: Wed, 9 Jan 2019 21:47:50 +0100
Subject: Remove imageParts.{isTagged,registry,name,tag}
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Finally, these members no longer have any users.

Future users should usually call referenceWithRegistry / normalizedReference,
and work with the returned value, instead of reintroducing these variables.
Similarly, direct uses of unnormalizedRef should be rare (only for cases
where the registry and/or path truly does not matter).

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 libpod/image/parts.go      | 35 ++---------------------------------
 libpod/image/parts_test.go | 19 +++++++++----------
 2 files changed, 11 insertions(+), 43 deletions(-)

(limited to 'libpod')

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index 62cca2757..dfdf0b08a 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -10,10 +10,6 @@ import (
 // imageParts describes the parts of an image's name
 type imageParts struct {
 	unnormalizedRef reference.Named // WARNING: Did not go through docker.io[/library] normalization
-	registry        string
-	name            string
-	tag             string
-	isTagged        bool
 	hasRegistry     bool
 }
 
@@ -37,45 +33,18 @@ func GetImageBaseName(input string) (string, error) {
 
 // decompose breaks an input name into an imageParts description
 func decompose(input string) (imageParts, error) {
-	var (
-		parts       imageParts
-		hasRegistry bool
-		tag         string
-	)
 	imgRef, err := reference.Parse(input)
 	if err != nil {
-		return parts, err
+		return imageParts{}, err
 	}
 	unnormalizedNamed := imgRef.(reference.Named)
-	ntag, isTagged := imgRef.(reference.NamedTagged)
-	if !isTagged {
-		tag = "latest"
-		if _, hasDigest := imgRef.(reference.Digested); hasDigest {
-			tag = "none"
-		}
-	} else {
-		tag = ntag.Tag()
-	}
-	registry := reference.Domain(unnormalizedNamed)
-	imageName := reference.Path(unnormalizedNamed)
 	// 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.
-	if isRegistry(registry) {
-		hasRegistry = true
-	} else {
-		if registry != "" {
-			imageName = registry + "/" + imageName
-			registry = ""
-		}
-	}
+	hasRegistry := isRegistry(reference.Domain(unnormalizedNamed))
 	return imageParts{
 		unnormalizedRef: unnormalizedNamed,
-		registry:        registry,
 		hasRegistry:     hasRegistry,
-		name:            imageName,
-		tag:             tag,
-		isTagged:        isTagged,
 	}, nil
 }
 
diff --git a/libpod/image/parts_test.go b/libpod/image/parts_test.go
index dd4a7b27f..726e55e86 100644
--- a/libpod/image/parts_test.go
+++ b/libpod/image/parts_test.go
@@ -13,30 +13,30 @@ func TestDecompose(t *testing.T) {
 	for _, c := range []struct {
 		input                                       string
 		registry, name, suspiciousTagValueForSearch string
-		isTagged, hasRegistry                       bool
+		hasRegistry                                 bool
 	}{
-		{"#", "", "", "", false, false}, // Entirely invalid input
+		{"#", "", "", "", false}, // Entirely invalid input
 		{ // Fully qualified docker.io, name-only input
-			"docker.io/library/busybox", "docker.io", "library/busybox", "latest", false, true,
+			"docker.io/library/busybox", "docker.io", "library/busybox", "latest", true,
 		},
 		{ // Fully qualified example.com, name-only input
-			"example.com/ns/busybox", "example.com", "ns/busybox", "latest", false, true,
+			"example.com/ns/busybox", "example.com", "ns/busybox", "latest", true,
 		},
 		{ // Unqualified single-name input
-			"busybox", "", "busybox", "latest", false, false,
+			"busybox", "", "busybox", "latest", false,
 		},
 		{ // Unqualified namespaced input
-			"ns/busybox", "", "ns/busybox", "latest", false, false,
+			"ns/busybox", "", "ns/busybox", "latest", false,
 		},
 		{ // name:tag
-			"example.com/ns/busybox:notlatest", "example.com", "ns/busybox", "notlatest", true, true,
+			"example.com/ns/busybox:notlatest", "example.com", "ns/busybox", "notlatest", true,
 		},
 		{ // name@digest
 			// FIXME? .suspiciousTagValueForSearch == "none"
-			"example.com/ns/busybox" + digestSuffix, "example.com", "ns/busybox", "none", false, true,
+			"example.com/ns/busybox" + digestSuffix, "example.com", "ns/busybox", "none", true,
 		},
 		{ // name:tag@digest
-			"example.com/ns/busybox:notlatest" + digestSuffix, "example.com", "ns/busybox", "notlatest", true, true,
+			"example.com/ns/busybox:notlatest" + digestSuffix, "example.com", "ns/busybox", "notlatest", true,
 		},
 	} {
 		parts, err := decompose(c.input)
@@ -48,7 +48,6 @@ func TestDecompose(t *testing.T) {
 			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.isTagged, parts.isTagged, c.input)
 			assert.Equal(t, c.hasRegistry, parts.hasRegistry, c.input)
 		}
 	}
-- 
cgit v1.2.3-54-g00ecf