From 24ca8141a663496d4b386c6845ba0714135522de Mon Sep 17 00:00:00 2001
From: Jake Howerton <jake.howerton@gmail.com>
Date: Fri, 3 Aug 2018 13:25:52 -0500
Subject: [PATCH] fix edge cases in build arg processing (#870)

---
 pkg/skaffold/docker/parse.go      | 24 ++++++++++++----
 pkg/skaffold/docker/parse_test.go | 48 +++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/pkg/skaffold/docker/parse.go b/pkg/skaffold/docker/parse.go
index 8f8d0d813f0..ac9c1f70fff 100644
--- a/pkg/skaffold/docker/parse.go
+++ b/pkg/skaffold/docker/parse.go
@@ -60,13 +60,26 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s
 	for i, value := range res.AST.Children {
 		switch value.Value {
 		case command.Arg:
-			var val string
-			arg := strings.Split(value.Original, " ")[1]
+			var val, defaultValue, arg string
+			argSetting := strings.Fields(value.Original)[1]
+
+			argValues := strings.Split(argSetting, "=")
+
+			if len(argValues) > 1 {
+				arg, defaultValue = argValues[0], argValues[1]
+			} else {
+				arg = argValues[0]
+			}
+
 			valuePtr := buildArgs[arg]
-			if valuePtr == nil {
-				logrus.Warnf("arg %s referenced in dockerfile but not provided in build args", arg)
+			if valuePtr == nil && defaultValue == "" {
+				logrus.Warnf("arg %s referenced in dockerfile but not provided with default or in build args", arg)
 			} else {
-				val = *valuePtr
+				if valuePtr == nil {
+					val = defaultValue
+				} else {
+					val = *valuePtr
+				}
 				if val == "" {
 					logrus.Warnf("empty build arg provided in skaffold config: %s", arg)
 					break
@@ -79,6 +92,7 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s
 							break
 						}
 						currentNode.Value = strings.Replace(currentNode.Value, "$"+arg, val, -1)
+						currentNode.Value = strings.Replace(currentNode.Value, "${"+arg+"}", val, -1)
 						currentNode = currentNode.Next
 					}
 				}
diff --git a/pkg/skaffold/docker/parse_test.go b/pkg/skaffold/docker/parse_test.go
index 48b20e20382..66bfd6d22ad 100644
--- a/pkg/skaffold/docker/parse_test.go
+++ b/pkg/skaffold/docker/parse_test.go
@@ -121,6 +121,27 @@ COPY $FOO .
 CMD $FOO
 `
 
+const copyServerGoBuildArgCurlyBraces = `
+FROM ubuntu:14.04
+ARG FOO
+COPY ${FOO} .
+CMD ${FOO}
+`
+
+const copyServerGoBuildArgExtraWhitespace = `
+FROM ubuntu:14.04
+ARG  FOO
+COPY $FOO .
+CMD $FOO
+`
+
+const copyServerGoBuildArgDefaultValue = `
+FROM ubuntu:14.04
+ARG FOO=server.go
+COPY $FOO .
+CMD $FOO
+`
+
 var fooArg = "server.go" // used for build args
 
 var ImageConfigs = map[string]*v1.ConfigFile{
@@ -282,6 +303,33 @@ func TestGetDependencies(t *testing.T) {
 			buildArgs:   map[string]*string{"FOO": &fooArg},
 			expected:    []string{"Dockerfile", "server.go"},
 		},
+		{
+			description: "build args with curly braces",
+			dockerfile:  copyServerGoBuildArgCurlyBraces,
+			workspace:   ".",
+			buildArgs:   map[string]*string{"FOO": &fooArg},
+			expected:    []string{"Dockerfile", "server.go"},
+		},
+		{
+			description: "build args with extra whitespace",
+			dockerfile:  copyServerGoBuildArgExtraWhitespace,
+			workspace:   ".",
+			buildArgs:   map[string]*string{"FOO": &fooArg},
+			expected:    []string{"Dockerfile", "server.go"},
+		},
+		{
+			description: "build args with default value and buildArgs unset",
+			dockerfile:  copyServerGoBuildArgDefaultValue,
+			workspace:   ".",
+			expected:    []string{"Dockerfile", "server.go"},
+		},
+		{
+			description: "build args with default value and buildArgs set",
+			dockerfile:  copyServerGoBuildArgDefaultValue,
+			workspace:   ".",
+			buildArgs:   map[string]*string{"FOO": &fooArg},
+			expected:    []string{"Dockerfile", "server.go"},
+		},
 	}
 
 	RetrieveImage = mockRetrieveImage