diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 7f56e2e71d..1d3717298e 100644 --- a/cmd/executor/cmd/root.go +++ b/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 { diff --git a/cmd/executor/cmd/root_test.go b/cmd/executor/cmd/root_test.go index cec6dff693..79e8ea1228 100644 --- a/cmd/executor/cmd/root_test.go +++ b/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) + }) + } +} diff --git a/integration/dockerfiles/Dockerfile_test_arg_secret b/integration/dockerfiles/Dockerfile_test_arg_secret new file mode 100644 index 0000000000..4429f938cd --- /dev/null +++ b/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 diff --git a/integration/images.go b/integration/images.go index f4df7af494..13bf6d7ecf 100644 --- a/integration/images.go +++ b/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