Skip to content

Commit

Permalink
fix edge cases in build arg processing (#870)
Browse files Browse the repository at this point in the history
  • Loading branch information
jakehow authored and dlorenc committed Aug 3, 2018
1 parent 85d0729 commit 24ca814
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 5 deletions.
24 changes: 19 additions & 5 deletions pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
}
Expand Down
48 changes: 48 additions & 0 deletions pkg/skaffold/docker/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 24ca814

Please sign in to comment.