Merge pull request #993 from antechrestos/feature/expand_build_args_with_environment_variable

Expand build argument from environment when no value specified

Author: tejal29

cmd/executor/cmd/root.go

@@ -55,6 +55,7 @@ var RootCmd = &cobra.Command{
 	Use: "executor",
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 		if cmd.Use == "executor" {
+			resolveEnvironmentBuildArgs(opts.BuildArgs, os.Getenv)
 			if err := util.ConfigureLogging(logLevel); err != nil {
 				return err
 			}
@@ -200,6 +201,17 @@ func resolveDockerfilePath() error {
 	return errors.New("please provide a valid path to a Dockerfile within the build context with --dockerfile")
 }
 
+// resolveEnvironmentBuildArgs replace build args without value by the same named environment variable
+func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) string) {
+	for index, argument := range arguments {
+		i := strings.Index(argument, "=")
+		if i < 0 {
+			value := resolver(argument)
+			arguments[index] = fmt.Sprintf("%s=%s", argument, value)
+		}
+	}
+}
+
 // copy Dockerfile to /kaniko/Dockerfile so that if it's specified in the .dockerignore
 // it won't be copied into the image
 func copyDockerfile() error {

cmd/executor/cmd/root_test.go

@@ -97,3 +97,55 @@ func TestIsUrl(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveEnvironmentBuildArgs(t *testing.T) {
+	tests := []struct {
+		description               string
+		input                     []string
+		expected                  []string
+		mockedEnvironmentResolver func(string) string
+	}{
+		{
+			description: "replace when environment variable is present and value is not specified",
+			input:       []string{"variable1"},
+			expected:    []string{"variable1=value1"},
+			mockedEnvironmentResolver: func(variable string) string {
+				if variable == "variable1" {
+					return "value1"
+				}
+				return ""
+			},
+		},
+		{
+			description: "do not replace when environment variable is present and value is specified",
+			input:       []string{"variable1=value1", "variable2=value2"},
+			expected:    []string{"variable1=value1", "variable2=value2"},
+			mockedEnvironmentResolver: func(variable string) string {
+				return "unexpected"
+			},
+		},
+		{
+			description: "do not replace when environment variable is present and empty value is specified",
+			input:       []string{"variable1="},
+			expected:    []string{"variable1="},
+			mockedEnvironmentResolver: func(variable string) string {
+				return "unexpected"
+			},
+		},
+		{
+			description: "replace with empty value when environment variable is not present or empty and value is not specified",
+			input:       []string{"variable1", "variable2=value2"},
+			expected:    []string{"variable1=", "variable2=value2"},
+			mockedEnvironmentResolver: func(variable string) string {
+				return ""
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			resolveEnvironmentBuildArgs(tt.input, tt.mockedEnvironmentResolver)
+			testutil.CheckDeepEqual(t, tt.expected, tt.input)
+		})
+	}
+}

integration/dockerfiles/Dockerfile_test_arg_secret

@@ -0,0 +1,10 @@
+FROM debian:9.11
+
+ARG SSH_PRIVATE_KEY
+ARG SSH_PUBLIC_KEY
+
+RUN mkdir .ssh                                 && \
+    chmod 700 .ssh                             && \
+    echo "$SSH_PRIVATE_KEY" > .ssh/id_rsa      && \
+    echo "$SSH_PUBLIC_KEY"  > .ssh/id_rsa.pub  && \
+    chmod 600 .ssh/id_rsa .ssh/id_rsa.pub

integration/images.go

@@ -17,6 +17,7 @@ limitations under the License.
 package integration
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -46,10 +47,11 @@ const (
 
 // Arguments to build Dockerfiles with, used for both docker and kaniko builds
 var argsMap = map[string][]string{
-	"Dockerfile_test_run":     {"file=/file"},
-	"Dockerfile_test_workdir": {"workdir=/arg/workdir"},
-	"Dockerfile_test_add":     {"file=context/foo"},
-	"Dockerfile_test_onbuild": {"file=/tmp/onbuild"},
+	"Dockerfile_test_run":        {"file=/file"},
+	"Dockerfile_test_workdir":    {"workdir=/arg/workdir"},
+	"Dockerfile_test_add":        {"file=context/foo"},
+	"Dockerfile_test_arg_secret": {"SSH_PRIVATE_KEY", "SSH_PUBLIC_KEY=Pµbl1cK€Y"},
+	"Dockerfile_test_onbuild":    {"file=/tmp/onbuild"},
 	"Dockerfile_test_scratch": {
 		"image=scratch",
 		"hello=hello-value",
@@ -59,6 +61,11 @@ var argsMap = map[string][]string{
 	"Dockerfile_test_multistage": {"file=/foo2"},
 }
 
+// Environment to build Dockerfiles with, used for both docker and kaniko builds
+var envsMap = map[string][]string{
+	"Dockerfile_test_arg_secret": {"SSH_PRIVATE_KEY=ThEPriv4t3Key"},
+}
+
 // Arguments to build Dockerfiles with when building with docker
 var additionalDockerFlagsMap = map[string][]string{
 	"Dockerfile_test_target": {"--target=second"},
@@ -71,6 +78,36 @@ var additionalKanikoFlagsMap = map[string][]string{
 	"Dockerfile_test_target":  {"--target=second"},
 }
 
+// output check to do when building with kaniko
+var outputChecks = map[string]func(string, []byte) error{
+	"Dockerfile_test_arg_secret": checkArgsNotPrinted,
+}
+
+// Checks if argument are not printed in output.
+// Argument may be passed through --build-arg key=value manner or --build-arg key with key in environment
+func checkArgsNotPrinted(dockerfile string, out []byte) error {
+	for _, arg := range argsMap[dockerfile] {
+		argSplitted := strings.Split(arg, "=")
+		if len(argSplitted) == 2 {
+			if idx := bytes.Index(out, []byte(argSplitted[1])); idx >= 0 {
+				return fmt.Errorf("Argument value %s for argument %s displayed in output", argSplitted[1], argSplitted[0])
+			}
+		} else if len(argSplitted) == 1 {
+			if envs, ok := envsMap[dockerfile]; ok {
+				for _, env := range envs {
+					envSplitted := strings.Split(env, "=")
+					if len(envSplitted) == 2 {
+						if idx := bytes.Index(out, []byte(envSplitted[1])); idx >= 0 {
+							return fmt.Errorf("Argument value %s for argument %s displayed in output", envSplitted[1], argSplitted[0])
+						}
+					}
+				}
+			}
+		}
+	}
+	return nil
+}
+
 var bucketContextTests = []string{"Dockerfile_test_copy_bucket"}
 var reproducibleTests = []string{"Dockerfile_test_reproducible"}
 
@@ -164,8 +201,7 @@ func (d *DockerFileBuilder) BuildImage(config *gcpConfig, dockerfilesPath, docke
 	var buildArgs []string
 	buildArgFlag := "--build-arg"
 	for _, arg := range argsMap[dockerfile] {
-		buildArgs = append(buildArgs, buildArgFlag)
-		buildArgs = append(buildArgs, arg)
+		buildArgs = append(buildArgs, buildArgFlag, arg)
 	}
 	// build docker image
 	additionalFlags := append(buildArgs, additionalDockerFlagsMap[dockerfile]...)
@@ -177,6 +213,9 @@ func (d *DockerFileBuilder) BuildImage(config *gcpConfig, dockerfilesPath, docke
 			"."},
 			additionalFlags...)...,
 	)
+	if env, ok := envsMap[dockerfile]; ok {
+		dockerCmd.Env = append(dockerCmd.Env, env...)
+	}
 
 	timer := timing.Start(dockerfile + "_docker")
 	out, err := RunCommandWithoutTest(dockerCmd)
@@ -225,7 +264,11 @@ func (d *DockerFileBuilder) BuildImage(config *gcpConfig, dockerfilesPath, docke
 		"-v", cwd + ":/workspace",
 		"-v", benchmarkDir + ":/kaniko/benchmarks",
 	}
-
+	if env, ok := envsMap[dockerfile]; ok {
+		for _, envVariable := range env {
+			dockerRunFlags = append(dockerRunFlags, "-e", envVariable)
+		}
+	}
 	dockerRunFlags = addServiceAccountFlags(dockerRunFlags, serviceAccount)
 
 	dockerRunFlags = append(dockerRunFlags, ExecutorImage,
@@ -242,6 +285,11 @@ func (d *DockerFileBuilder) BuildImage(config *gcpConfig, dockerfilesPath, docke
 	if err != nil {
 		return fmt.Errorf("Failed to build image %s with kaniko command \"%s\": %s %s", dockerImage, kanikoCmd.Args, err, string(out))
 	}
+	if outputCheck := outputChecks[dockerfile]; outputCheck != nil {
+		if err := outputCheck(dockerfile, out); err != nil {
+			return fmt.Errorf("Output check failed for image %s with kaniko command \"%s\": %s %s", dockerImage, kanikoCmd.Args, err, string(out))
+		}
+	}
 
 	d.FilesBuilt[dockerfile] = true
 	return nil