diff --git a/pkg/skaffold/docker/parse.go b/pkg/skaffold/docker/parse.go index cc59e7a6e00..26d49027e66 100644 --- a/pkg/skaffold/docker/parse.go +++ b/pkg/skaffold/docker/parse.go @@ -27,6 +27,7 @@ import ( "sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha3" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/docker/docker/builder/dockerignore" "github.com/docker/docker/pkg/fileutils" "github.com/google/go-containerregistry/pkg/v1" @@ -62,6 +63,33 @@ func ValidateDockerfile(path string) bool { return true } +func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) { + var key, value string + + for _, node := range nodes { + switch node.Value { + case command.Arg: + // build arg's key + keyValue := strings.Split(node.Next.Value, "=") + key = keyValue[0] + + // build arg's value + if buildArgs[key] != nil { + value = *buildArgs[key] + } else if len(keyValue) > 1 { + value = keyValue[1] + } + default: + if key != "" { + // replace $key with value + for curr := node; curr != nil; curr = curr.Next { + curr.Value = util.Expand(curr.Value, key, value) + } + } + } + } +} + func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*string) ([]string, error) { f, err := os.Open(absDockerfilePath) if err != nil { @@ -74,52 +102,7 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s return nil, errors.Wrap(err, "parsing dockerfile") } - var copied [][]string - envs := map[string]string{} - - // First process all build args, and replace if necessary. - for i, value := range res.AST.Children { - switch value.Value { - case command.Arg: - 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 && defaultValue == "" { - logrus.Warnf("arg %s referenced in dockerfile but not provided with default or in build args", arg) - } else { - if valuePtr == nil { - val = defaultValue - } else { - val = *valuePtr - } - if val == "" { - logrus.Warnf("empty build arg provided in skaffold config: %s", arg) - break - } - // we have a non-empty arg: replace it in all subsequent nodes - for j := i; j < len(res.AST.Children); j++ { - currentNode := res.AST.Children[j] - for { - if currentNode == nil { - break - } - currentNode.Value = strings.Replace(currentNode.Value, "$"+arg, val, -1) - currentNode.Value = strings.Replace(currentNode.Value, "${"+arg+"}", val, -1) - currentNode = currentNode.Next - } - } - } - } - } + expandBuildArgs(res.AST.Children, buildArgs) // Then process onbuilds, if present. onbuildsImages := [][]string{} @@ -147,6 +130,9 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s } } + var copied [][]string + envs := map[string]string{} + var dispatchInstructions = func(r *parser.Result) { for _, value := range r.AST.Children { switch value.Value { diff --git a/pkg/skaffold/docker/parse_test.go b/pkg/skaffold/docker/parse_test.go index 6cd110cdb5f..2b4799b3a0c 100644 --- a/pkg/skaffold/docker/parse_test.go +++ b/pkg/skaffold/docker/parse_test.go @@ -70,7 +70,6 @@ RUN go build -o worker . FROM gcr.io/distroless/base WORKDIR /root/ COPY --from=0 /go/src/github.com/r2d4/leeroy . -CMD ["./worker"] ` const envTest = ` @@ -84,7 +83,6 @@ const copyDirectory = ` FROM nginx ADD . /etc/ COPY ./file /etc/file -CMD nginx ` const multiFileCopy = ` FROM ubuntu:14.04 @@ -100,7 +98,6 @@ const contextDockerfile = ` FROM nginx ADD nginx.conf /etc/nginx COPY . /files -CMD nginx ` // This has an ONBUILD instruction of "COPY . /go/src/app" @@ -117,28 +114,38 @@ const copyServerGoBuildArg = ` FROM ubuntu:14.04 ARG FOO COPY $FOO . -CMD $FOO +` + +const copyServerGoBuildArgSamePrefix = ` +FROM ubuntu:14.04 +ARG FOO=server.go +ARG FOO2 +COPY $FOO2 . ` 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 +` + +const copyServerGoBuildArgRedefinedDefaultValue = ` +FROM ubuntu:14.04 +ARG FOO=server.go +ARG FOO=worker.go +COPY $FOO . ` const fromStage = ` @@ -311,6 +318,14 @@ func TestGetDependencies(t *testing.T) { expected: []string{"Dockerfile", "server.go"}, fetched: []string{"ubuntu:14.04"}, }, + { + description: "build args with same prefix", + dockerfile: copyServerGoBuildArgSamePrefix, + workspace: ".", + buildArgs: map[string]*string{"FOO2": util.StringPtr("worker.go")}, + expected: []string{"Dockerfile", "worker.go"}, + fetched: []string{"ubuntu:14.04"}, + }, { description: "build args with curly braces", dockerfile: copyServerGoBuildArgCurlyBraces, @@ -328,17 +343,32 @@ func TestGetDependencies(t *testing.T) { fetched: []string{"ubuntu:14.04"}, }, { - description: "build args with default value and buildArgs unset", + description: "build args with default value", dockerfile: copyServerGoBuildArgDefaultValue, workspace: ".", expected: []string{"Dockerfile", "server.go"}, fetched: []string{"ubuntu:14.04"}, }, { - description: "build args with default value and buildArgs set", + description: "build args with redefined default value", + dockerfile: copyServerGoBuildArgRedefinedDefaultValue, + workspace: ".", + expected: []string{"Dockerfile", "worker.go"}, + fetched: []string{"ubuntu:14.04"}, + }, + { + description: "override default build arg", dockerfile: copyServerGoBuildArgDefaultValue, workspace: ".", - buildArgs: map[string]*string{"FOO": util.StringPtr("server.go")}, + buildArgs: map[string]*string{"FOO": util.StringPtr("worker.go")}, + expected: []string{"Dockerfile", "worker.go"}, + fetched: []string{"ubuntu:14.04"}, + }, + { + description: "ignore build arg and use default arg value", + dockerfile: copyServerGoBuildArgDefaultValue, + workspace: ".", + buildArgs: map[string]*string{"FOO": nil}, expected: []string{"Dockerfile", "server.go"}, fetched: []string{"ubuntu:14.04"}, }, diff --git a/pkg/skaffold/util/util.go b/pkg/skaffold/util/util.go index bb4dc58e6e2..7eade1ac029 100644 --- a/pkg/skaffold/util/util.go +++ b/pkg/skaffold/util/util.go @@ -23,6 +23,7 @@ import ( "net/http" "os" "path/filepath" + "regexp" "sort" "strings" @@ -178,3 +179,26 @@ func RemoveFromSlice(s []string, target string) []string { } return s } + +// Expand replaces placeholders for a given key with a given value. +// It supports the ${key} and the $key syntax. +func Expand(text, key, value string) string { + text = strings.Replace(text, "${"+key+"}", value, -1) + + indices := regexp.MustCompile(`\$`+key).FindAllStringIndex(text, -1) + + for i := len(indices) - 1; i >= 0; i-- { + from := indices[i][0] + to := indices[i][1] + + if to >= len(text) || !isAlphaNum(text[to]) { + text = text[0:from] + value + text[to:] + } + } + + return text +} + +func isAlphaNum(c uint8) bool { + return c == '_' || '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' +} diff --git a/pkg/skaffold/util/util_test.go b/pkg/skaffold/util/util_test.go index 57e52a2a572..ef3b3998077 100644 --- a/pkg/skaffold/util/util_test.go +++ b/pkg/skaffold/util/util_test.go @@ -114,3 +114,64 @@ func TestDefaultConfigFilenameAlternate(t *testing.T) { testutil.CheckErrorAndDeepEqual(t, false, err, []byte("foo"), content) } + +func TestExpand(t *testing.T) { + var tests = []struct { + description string + text string + key string + value string + expected string + }{ + { + description: "${key} syntax", + text: "BEFORE[${key}]AFTER", + key: "key", + value: "VALUE", + expected: "BEFORE[VALUE]AFTER", + }, + { + description: "$key syntax", + text: "BEFORE[$key]AFTER", + key: "key", + value: "VALUE", + expected: "BEFORE[VALUE]AFTER", + }, + { + description: "replace all", + text: "BEFORE[$key][${key}][$key][${key}]AFTER", + key: "key", + value: "VALUE", + expected: "BEFORE[VALUE][VALUE][VALUE][VALUE]AFTER", + }, + { + description: "ignore common prefix", + text: "BEFORE[$key1][${key1}]AFTER", + key: "key", + value: "VALUE", + expected: "BEFORE[$key1][${key1}]AFTER", + }, + { + description: "just the ${key} placeholder", + text: "${key}", + key: "key", + value: "VALUE", + expected: "VALUE", + }, + { + description: "just the $key placeholder", + text: "$key", + key: "key", + value: "VALUE", + expected: "VALUE", + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + actual := Expand(test.text, test.key, test.value) + + testutil.CheckDeepEqual(t, test.expected, actual) + }) + } +}