Split normalizeTag from Image.TagImage

... so that it can be tested without side effects, and add the tests.

Should not change behavior.

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

Closes: #1112
Approved by: rhatdan

2 files changed, 35 insertions, 4 deletions

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 3863338b5..7ea851f5d 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -457,12 +457,11 @@ func getImageDigest(ctx context.Context, src types.ImageReference, sc *types.Sys
 	return "@" + digest.Hex(), nil
 }
 
-// TagImage adds a tag to the given image
-func (i *Image) TagImage(tag string) error {
-	i.reloadImage()
+// normalizeTag returns the canonical version of tag for use in Image.Names()
+func normalizeTag(tag string) (string, error) {
 	decomposedTag, err := decompose(tag)
 	if err != nil {
-		return err
+		return "", err
 	}
 	// If the input does not have a tag, we need to add one (latest)
 	if !decomposedTag.isTagged {
@@ -472,6 +471,16 @@ func (i *Image) TagImage(tag string) error {
 	if !decomposedTag.hasRegistry {
 		tag = fmt.Sprintf("%s/%s", DefaultLocalRepo, tag)
 	}
+	return tag, nil
+}
+
+// TagImage adds a tag to the given image
+func (i *Image) TagImage(tag string) error {
+	i.reloadImage()
+	tag, err := normalizeTag(tag)
+	if err != nil {
+		return err
+	}
 	tags := i.Names()
 	if util.StringInSlice(tag, tags) {
 		return nil
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index 04877dbe5..2236b2010 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -198,3 +198,25 @@ func Test_stripSha256(t *testing.T) {
 	assert.Equal(t, stripSha256("sha256:"), "sha256:")
 	assert.Equal(t, stripSha256("sha256:a"), "a")
 }
+
+func TestNormalizeTag(t *testing.T) {
+	const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+
+	for _, c := range []struct{ input, expected string }{
+		{"#", ""}, // Clearly invalid
+		{"example.com/busybox", "example.com/busybox:latest"},                                            // Qualified name-only
+		{"example.com/busybox:notlatest", "example.com/busybox:notlatest"},                               // Qualified name:tag
+		{"example.com/busybox" + digestSuffix, "example.com/busybox" + digestSuffix + ":none"},           // Qualified name@digest; FIXME: The result is not even syntactically valid!
+		{"example.com/busybox: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
+	} {
+		res, err := normalizeTag(c.input)
+		if c.expected == "" {
+			assert.Error(t, err, c.input)
+		} else {
+			assert.NoError(t, err, c.input)
+			assert.Equal(t, c.expected, res)
+		}
+	}
+}