Skip to content

Allow environment variables to be used in docker build argument #1912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/content/en/schemas/v1beta8.json
Original file line number Diff line number Diff line change
Expand Up @@ -1191,11 +1191,11 @@
"type": "string"
},
"type": "object",
"description": "arguments passed to the docker build.",
"x-intellij-html-description": "arguments passed to the docker build.",
"description": "arguments passed to the docker build. It also accepts environment variables via the go template syntax.",
"x-intellij-html-description": "arguments passed to the docker build. It also accepts environment variables via the go template syntax.",
"default": "{}",
"examples": [
"{\"key1\": \"value1\", \"key2\": \"value2\"}"
"{\"key1\": \"value1\", \"key2\": \"value2\", \"key3\": \"{{.ENV_VARIABLE}}\"}"
]
},
"buildContext": {
Expand Down
4 changes: 3 additions & 1 deletion integration/examples/nodejs/backend/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
FROM node:8.12.0-alpine

ARG SCRIPT
ENV SCRIPT ${SCRIPT}
WORKDIR /opt/backend
EXPOSE 3000
CMD ["npm", "run", "dev"]
CMD ["npm", "run", ${SCRIPT}]

COPY . .
RUN npm install
3 changes: 3 additions & 0 deletions integration/examples/nodejs/skaffold.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ build:
artifacts:
- image: gcr.io/k8s-skaffold/node-example
context: backend
docker:
buildArgs:
SCRIPT: "{{.SCRIPT_ENV}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am guessing you meant "{{.SCRIPT}} here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing it out.

sync:
'*.js': .
deploy:
Expand Down
1 change: 1 addition & 0 deletions integration/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func TestRun(t *testing.T) {
}, {
description: "nodejs",
dir: "examples/nodejs",
env: []string{"SCRIPT_ENV=dev"},
pods: []string{"node"},
}, {
description: "structure-tests",
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/docker/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,11 @@ func GetBuildArgs(a *latest.DockerArtifact) []string {
if v == nil {
args = append(args, k)
} else {
args = append(args, fmt.Sprintf("%s=%s", k, *v))
value, err := evaluateBuildArgsValue(*v)
if err != nil {
logrus.Errorf("unable to get value for build arg: %s", k)
}
args = append(args, fmt.Sprintf("%s=%s", k, value))
}
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/skaffold/docker/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func TestGetBuildArgs(t *testing.T) {
tests := []struct {
description string
artifact *latest.DockerArtifact
env []string
want []string
}{
{
Expand All @@ -180,9 +181,11 @@ func TestGetBuildArgs(t *testing.T) {
BuildArgs: map[string]*string{
"key1": util.StringPtr("value1"),
"key2": nil,
"key3": util.StringPtr("{{.FOO}}"),
},
},
want: []string{"--build-arg", "key1=value1", "--build-arg", "key2"},
env: []string{"FOO=bar"},
want: []string{"--build-arg", "key1=value1", "--build-arg", "key2", "--build-arg", "key3=bar"},
},
{
description: "cache from",
Expand Down Expand Up @@ -212,6 +215,9 @@ func TestGetBuildArgs(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
util.OSEnviron = func() []string {
return tt.env
}
result := GetBuildArgs(tt.artifact)
if diff := cmp.Diff(result, tt.want); diff != "" {
t.Errorf("%T differ (-got, +want): %s", tt.want, diff)
Expand Down
23 changes: 20 additions & 3 deletions pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,16 @@ type from struct {
// RetrieveImage is overridden for unit testing
var RetrieveImage = retrieveImage

func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) {
func evaluateBuildArgsValue(nameTemplate string) (string, error) {
tmpl, err := util.ParseEnvTemplate(nameTemplate)
if err != nil {
return "", errors.Wrap(err, "parsing template")
}

return util.ExecuteEnvTemplate(tmpl, nil)
}

func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) error {
for i, node := range nodes {
if node.Value != command.Arg {
continue
Expand All @@ -56,8 +65,12 @@ func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) {

// build arg's value
var value string
var err error
if buildArgs[key] != nil {
value = *buildArgs[key]
value, err = evaluateBuildArgsValue(*buildArgs[key])
if err != nil {
return errors.Wrapf(err, "unable to get value for build arg: %s", key)
}
} else if len(keyValue) > 1 {
value = keyValue[1]
}
Expand All @@ -74,6 +87,7 @@ func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) {
}
}
}
return nil
}

func fromInstructions(nodes []*parser.Node) []from {
Expand Down Expand Up @@ -181,7 +195,10 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s

dockerfileLines := res.AST.Children

expandBuildArgs(dockerfileLines, buildArgs)
err = expandBuildArgs(dockerfileLines, buildArgs)
if err != nil {
return nil, errors.Wrap(err, "putting build arguments")
}

instructions, err := onbuildInstructions(dockerfileLines)
if err != nil {
Expand Down
13 changes: 13 additions & 0 deletions pkg/skaffold/docker/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ func TestGetDependencies(t *testing.T) {
workspace string
ignore string
buildArgs map[string]*string
env []string

expected []string
fetched []string
Expand Down Expand Up @@ -486,10 +487,22 @@ func TestGetDependencies(t *testing.T) {
expected: []string{"Dockerfile", "file"},
fetched: []string{"jboss/wildfly:14.0.1.Final"},
},
{
description: "build args with an environment variable",
dockerfile: copyServerGoBuildArg,
workspace: ".",
buildArgs: map[string]*string{"FOO": util.StringPtr("{{.FILE_NAME}}")},
env: []string{"FILE_NAME=server.go"},
expected: []string{"Dockerfile", "server.go"},
fetched: []string{"ubuntu:14.04"},
},
}

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
util.OSEnviron = func() []string {
return test.env
}
tmpDir, cleanup := testutil.NewTempDir(t)
defer cleanup()

Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,8 @@ type KanikoArtifact struct {
Target string `yaml:"target,omitempty"`

// BuildArgs are arguments passed to the docker build.
// For example: `{"key1": "value1", "key2": "value2"}`.
// It also accepts environment variables via the go template syntax.
// For example: `{"key1": "value1", "key2": "value2", "key3": "{{.ENV_VARIABLE}}"}`.
BuildArgs map[string]*string `yaml:"buildArgs,omitempty"`

// BuildContext is where the build context for this artifact resides.
Expand Down