Improve image reference parsing

GoogleContainerTools · May 19, 2018 · 27c326f · 27c326f
1 parent a7a584c
commit 27c326f
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 46 deletions.
diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go
@@ -23,14 +23,14 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"text/template"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+	"github.com/docker/distribution/reference"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
@@ -326,30 +326,58 @@ func recursiveReplaceImage(i interface{}, replacements map[string]*replacement)
 		}
 	case map[interface{}]interface{}:
 		for k, v := range t {
-			if k.(string) == "image" {
-				name, tag := splitTag(v.(string))
-				if img, present := replacements[name]; present {
-					if tag == "" || tag == "latest" {
-						t[k] = img.tag
-						img.found = true
-					} else {
-						// TODO(1.0.0): Remove this warning.
-						logrus.Infof("Not replacing fully qualified image: %s (see #565)", v)
-					}
-				}
-			} else {
+			if k.(string) != "image" {
 				recursiveReplaceImage(v, replacements)
+				continue
+			}
+
+			image := v.(string)
+			parsed, err := parseReference(image)
+			if err != nil {
+				logrus.Warnf("Couldn't parse image: %s", v)
+				continue
+			}
+
+			if parsed.fullyQualified {
+				// TODO(1.0.0): Remove this warning.
+				logrus.Infof("Not replacing fully qualified image: %s (see #565)", v)
+				continue
+			}
+
+			if img, present := replacements[parsed.baseName]; present {
+				t[k] = img.tag
+				img.found = true
 			}
 		}
 	}
 }
 
-func splitTag(image string) (string, string) {
-	re := regexp.MustCompile(`(.*):([^/]+)$`)
-	matches := re.FindStringSubmatch(image)
+type imageReference struct {
+	baseName       string
+	fullyQualified bool
+}
+
+func parseReference(image string) (*imageReference, error) {
+	r, err := reference.Parse(image)
+	if err != nil {
+		return nil, err
+	}
+
+	baseName := image
+	if n, ok := r.(reference.Named); ok {
+		baseName = n.Name()
+	}
 
-	if len(matches) == 3 {
-		return matches[1], matches[2]
+	fullyQualified := false
+	switch n := r.(type) {
+	case reference.Tagged:
+		fullyQualified = n.Tag() != "latest"
+	case reference.Digested:
+		fullyQualified = true
 	}
-	return image, ""
+
+	return &imageReference{
+		baseName:       baseName,
+		fullyQualified: fullyQualified,
+	}, nil
 }
diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go
@@ -344,47 +344,49 @@ spec:
 
 func TestSplitTag(t *testing.T) {
 	var tests = []struct {
-		description  string
-		image        string
-		expectedName string
-		expectedTag  string
+		description            string
+		image                  string
+		expectedName           string
+		expectedFullyQualified bool
 	}{
 		{
-			description:  "port and tag",
-			image:        "host:1234/user/container:tag",
-			expectedName: "host:1234/user/container",
-			expectedTag:  "tag",
+			description:            "port and tag",
+			image:                  "host:1234/user/container:tag",
+			expectedName:           "host:1234/user/container",
+			expectedFullyQualified: true,
 		},
 		{
-			description:  "port",
-			image:        "host:1234/user/container",
-			expectedName: "host:1234/user/container",
-			expectedTag:  "",
+			description:            "port",
+			image:                  "host:1234/user/container",
+			expectedName:           "host:1234/user/container",
+			expectedFullyQualified: false,
 		},
 		{
-			description:  "tag",
-			image:        "host/user/container:tag",
-			expectedName: "host/user/container",
-			expectedTag:  "tag",
+			description:            "tag",
+			image:                  "host/user/container:tag",
+			expectedName:           "host/user/container",
+			expectedFullyQualified: true,
 		},
 		{
-			description:  "latest",
-			image:        "host/user/container:latest",
-			expectedName: "host/user/container",
-			expectedTag:  "latest",
+			description:            "latest",
+			image:                  "host/user/container:latest",
+			expectedName:           "host/user/container",
+			expectedFullyQualified: false,
+		},
+		{
+			description:            "digest",
+			image:                  "gcr.io/k8s-skaffold/example@sha256:81daf011d63b68cfa514ddab7741a1adddd59d3264118dfb0fd9266328bb8883",
+			expectedName:           "gcr.io/k8s-skaffold/example",
+			expectedFullyQualified: true,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			name, tag := splitTag(test.image)
+			parsed, err := parseReference(test.image)
 
-			if name != test.expectedName {
-				t.Errorf("Expected name: [%s]. Got: [%s]", test.expectedName, name)
-			}
-			if tag != test.expectedTag {
-				t.Errorf("Expected tag: [%s]. Got: [%s]", test.expectedTag, tag)
-			}
+			testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedName, parsed.baseName)
+			testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedFullyQualified, parsed.fullyQualified)
 		})
 	}
 }