From 5da78632a18ebd16bca2c7c37bf1e32b518bca0c Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 20 Mar 2018 10:40:45 -0700 Subject: [PATCH 01/23] CMD command and unit tests --- pkg/commands/cmd.go | 63 ++++++++++++++++++++++++++++++++++++++++ pkg/commands/cmd_test.go | 61 ++++++++++++++++++++++++++++++++++++++ pkg/commands/commands.go | 2 ++ 3 files changed, 126 insertions(+) create mode 100644 pkg/commands/cmd.go create mode 100644 pkg/commands/cmd_test.go diff --git a/pkg/commands/cmd.go b/pkg/commands/cmd.go new file mode 100644 index 000000000..54e18e241 --- /dev/null +++ b/pkg/commands/cmd.go @@ -0,0 +1,63 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" + "strings" +) + +type CmdCommand struct { + cmd *instructions.CmdCommand +} + +// ExecuteCommand executes the CMD command +// Argument handling is the same as RUN. +func (c *CmdCommand) ExecuteCommand(config *manifest.Schema2Config) error { + logrus.Info("cmd: CMD") + var newCommand []string + if c.cmd.PrependShell { + // This is the default shell on Linux + // TODO: Support shell command here + shell := []string{"/bin/sh", "-c"} + newCommand = append(shell, strings.Join(c.cmd.CmdLine, " ")) + } else { + newCommand = c.cmd.CmdLine + } + + logrus.Infof("Replacing CMD in config with %v", newCommand) + config.Cmd = newCommand + return nil +} + +// FilesToSnapshot returns an empty array since this is a metadata command +func (c *CmdCommand) FilesToSnapshot() []string { + return []string{} +} + +// CreatedBy returns some information about the command for the image config history +func (c *CmdCommand) CreatedBy() string { + cmdLine := strings.Join(c.cmd.CmdLine, " ") + if c.cmd.PrependShell { + // TODO: Support shell command here + shell := []string{"/bin/sh", "-c"} + return strings.Join(append(shell, cmdLine), " ") + } + return cmdLine +} diff --git a/pkg/commands/cmd_test.go b/pkg/commands/cmd_test.go new file mode 100644 index 000000000..3a966da11 --- /dev/null +++ b/pkg/commands/cmd_test.go @@ -0,0 +1,61 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package commands + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" + "github.com/containers/image/manifest" + "github.com/containers/image/pkg/strslice" + "github.com/docker/docker/builder/dockerfile/instructions" + "testing" +) + +var cmdTests = []struct { + prependShell bool + cmdLine []string + expectedCmd strslice.StrSlice +}{ + { + prependShell: true, + cmdLine: []string{"echo", "cmd1"}, + expectedCmd: strslice.StrSlice{"/bin/sh", "-c", "echo cmd1"}, + }, + { + prependShell: false, + cmdLine: []string{"echo", "cmd2"}, + expectedCmd: strslice.StrSlice{"echo", "cmd2"}, + }, +} + +func TestExecuteCmd(t *testing.T) { + + cfg := &manifest.Schema2Config{ + Cmd: nil, + } + + for _, test := range cmdTests { + cmd := CmdCommand{ + &instructions.CmdCommand{ + ShellDependantCmdLine: instructions.ShellDependantCmdLine{ + PrependShell: test.prependShell, + CmdLine: test.cmdLine, + }, + }, + } + err := cmd.ExecuteCommand(cfg) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedCmd, cfg.Cmd) + } +} diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 6fc2a8c1a..3ce6960aa 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -40,6 +40,8 @@ func GetCommand(cmd instructions.Command) (DockerCommand, error) { return &RunCommand{cmd: c}, nil case *instructions.EnvCommand: return &EnvCommand{cmd: c}, nil + case *instructions.CmdCommand: + return &CmdCommand{cmd: c}, nil } return nil, errors.Errorf("%s is not a supported command", cmd.Name()) } From 773d2b06fdc4b65b145db4757f6c7b2c2648a220 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 20 Mar 2018 10:50:04 -0700 Subject: [PATCH 02/23] CMD integration test --- integration_tests/dockerfiles/Dockerfile_test_cmd | 4 ++++ integration_tests/dockerfiles/test_cmd.yaml | 3 +++ integration_tests/integration_test_yaml.go | 7 +++++++ 3 files changed, 14 insertions(+) create mode 100644 integration_tests/dockerfiles/Dockerfile_test_cmd create mode 100644 integration_tests/dockerfiles/test_cmd.yaml diff --git a/integration_tests/dockerfiles/Dockerfile_test_cmd b/integration_tests/dockerfiles/Dockerfile_test_cmd new file mode 100644 index 000000000..7b8b04999 --- /dev/null +++ b/integration_tests/dockerfiles/Dockerfile_test_cmd @@ -0,0 +1,4 @@ +FROM gcr.io/distroless/base:latest +CMD ["command", "one"] +CMD ["command", "two"] +CMD echo "hello" diff --git a/integration_tests/dockerfiles/test_cmd.yaml b/integration_tests/dockerfiles/test_cmd.yaml new file mode 100644 index 000000000..c05b14824 --- /dev/null +++ b/integration_tests/dockerfiles/test_cmd.yaml @@ -0,0 +1,3 @@ +schemaVersion: '2.0.0' +metadataTest: + cmd: ["/bin/sh", "-c", "echo \"hello\""] diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index 5471e207e..d610e5146 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -65,6 +65,13 @@ var structureTests = []struct { dockerBuildContext: "/workspace/integration_tests/dockerfiles/", structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_env.yaml", }, + { + description: "test cmd", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_cmd", + repo: "test-cmd", + dockerBuildContext: "/workspace/integration_tests/dockerfiles/", + structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_cmd.yaml", + }, } type step struct { From d49c7c5ed1fb430bb4dd0986239e923448f09524 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 20 Mar 2018 12:58:38 -0700 Subject: [PATCH 03/23] Entrypoint command and unit tests --- pkg/commands/commands.go | 2 ++ pkg/commands/entrypoint.go | 64 +++++++++++++++++++++++++++++++++ pkg/commands/entrypoint_test.go | 61 +++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 pkg/commands/entrypoint.go create mode 100644 pkg/commands/entrypoint_test.go diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 3ce6960aa..04622035b 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -42,6 +42,8 @@ func GetCommand(cmd instructions.Command) (DockerCommand, error) { return &EnvCommand{cmd: c}, nil case *instructions.CmdCommand: return &CmdCommand{cmd: c}, nil + case *instructions.EntrypointCommand: + return &EntrypointCommand{cmd: c}, nil } return nil, errors.Errorf("%s is not a supported command", cmd.Name()) } diff --git a/pkg/commands/entrypoint.go b/pkg/commands/entrypoint.go new file mode 100644 index 000000000..a7fd5f3ed --- /dev/null +++ b/pkg/commands/entrypoint.go @@ -0,0 +1,64 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" + "strings" +) + +type EntrypointCommand struct { + cmd *instructions.EntrypointCommand +} + +// ExecuteCommand handles command processing similar to CMD and RUN, +func (e *EntrypointCommand) ExecuteCommand(config *manifest.Schema2Config) error { + logrus.Info("cmd: ENTRYPOINT") + var newCommand []string + if e.cmd.PrependShell { + // This is the default shell on Linux + // TODO: Support shell command here + shell := []string{"/bin/sh", "-c"} + newCommand = append(shell, strings.Join(e.cmd.CmdLine, " ")) + } else { + newCommand = e.cmd.CmdLine + } + + logrus.Infof("Replacing Entrypoint in config with %v", newCommand) + config.Entrypoint = newCommand + return nil +} + +// FilesToSnapshot returns an empty array since this is a metadata command +func (e *EntrypointCommand) FilesToSnapshot() []string { + return []string{} +} + +// CreatedBy returns some information about the command for the image config history +func (e *EntrypointCommand) CreatedBy() string { + entrypoint := []string{"ENTRYPOINT"} + cmdLine := strings.Join(e.cmd.CmdLine, " ") + if e.cmd.PrependShell { + // TODO: Support shell command here + shell := []string{"/bin/sh", "-c"} + appendedShell := append(entrypoint, shell...) + return strings.Join(append(appendedShell, cmdLine), " ") + } + return strings.Join(append(entrypoint, cmdLine), " ") +} diff --git a/pkg/commands/entrypoint_test.go b/pkg/commands/entrypoint_test.go new file mode 100644 index 000000000..0835de07e --- /dev/null +++ b/pkg/commands/entrypoint_test.go @@ -0,0 +1,61 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package commands + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" + "github.com/containers/image/manifest" + "github.com/containers/image/pkg/strslice" + "github.com/docker/docker/builder/dockerfile/instructions" + "testing" +) + +var entrypointTests = []struct { + prependShell bool + cmdLine []string + expectedCmd strslice.StrSlice +}{ + { + prependShell: true, + cmdLine: []string{"echo", "cmd1"}, + expectedCmd: strslice.StrSlice{"/bin/sh", "-c", "echo cmd1"}, + }, + { + prependShell: false, + cmdLine: []string{"echo", "cmd2"}, + expectedCmd: strslice.StrSlice{"echo", "cmd2"}, + }, +} + +func TestEntrypointExecuteCmd(t *testing.T) { + + cfg := &manifest.Schema2Config{ + Cmd: nil, + } + + for _, test := range entrypointTests { + cmd := EntrypointCommand{ + &instructions.EntrypointCommand{ + ShellDependantCmdLine: instructions.ShellDependantCmdLine{ + PrependShell: test.prependShell, + CmdLine: test.cmdLine, + }, + }, + } + err := cmd.ExecuteCommand(cfg) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedCmd, cfg.Entrypoint) + } +} From bf47ea928b1b8288dac5b29b46a4b23636e62ede Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 20 Mar 2018 13:00:36 -0700 Subject: [PATCH 04/23] Integration tests for entrypoint --- .../{Dockerfile_test_cmd => Dockerfile_test_metadata} | 3 +++ .../dockerfiles/{test_cmd.yaml => test_metadata.yaml} | 1 + integration_tests/integration_test_yaml.go | 8 ++++---- pkg/commands/cmd.go | 6 ++++-- 4 files changed, 12 insertions(+), 6 deletions(-) rename integration_tests/dockerfiles/{Dockerfile_test_cmd => Dockerfile_test_metadata} (56%) rename integration_tests/dockerfiles/{test_cmd.yaml => test_metadata.yaml} (66%) diff --git a/integration_tests/dockerfiles/Dockerfile_test_cmd b/integration_tests/dockerfiles/Dockerfile_test_metadata similarity index 56% rename from integration_tests/dockerfiles/Dockerfile_test_cmd rename to integration_tests/dockerfiles/Dockerfile_test_metadata index 7b8b04999..261db4d5b 100644 --- a/integration_tests/dockerfiles/Dockerfile_test_cmd +++ b/integration_tests/dockerfiles/Dockerfile_test_metadata @@ -2,3 +2,6 @@ FROM gcr.io/distroless/base:latest CMD ["command", "one"] CMD ["command", "two"] CMD echo "hello" + +ENTRYPOINT ["execute", "something"] +ENTRYPOINT ["execute", "entrypoint"] diff --git a/integration_tests/dockerfiles/test_cmd.yaml b/integration_tests/dockerfiles/test_metadata.yaml similarity index 66% rename from integration_tests/dockerfiles/test_cmd.yaml rename to integration_tests/dockerfiles/test_metadata.yaml index c05b14824..cb0024311 100644 --- a/integration_tests/dockerfiles/test_cmd.yaml +++ b/integration_tests/dockerfiles/test_metadata.yaml @@ -1,3 +1,4 @@ schemaVersion: '2.0.0' metadataTest: cmd: ["/bin/sh", "-c", "echo \"hello\""] + entrypoint: ["execute", "entrypoint"] diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index d610e5146..19c5a8926 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -66,11 +66,11 @@ var structureTests = []struct { structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_env.yaml", }, { - description: "test cmd", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_cmd", - repo: "test-cmd", + description: "test metadata", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_metadata", + repo: "test-metadata", dockerBuildContext: "/workspace/integration_tests/dockerfiles/", - structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_cmd.yaml", + structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_metadata.yaml", }, } diff --git a/pkg/commands/cmd.go b/pkg/commands/cmd.go index 54e18e241..4d49234b2 100644 --- a/pkg/commands/cmd.go +++ b/pkg/commands/cmd.go @@ -53,11 +53,13 @@ func (c *CmdCommand) FilesToSnapshot() []string { // CreatedBy returns some information about the command for the image config history func (c *CmdCommand) CreatedBy() string { + cmd := []string{"CMD"} cmdLine := strings.Join(c.cmd.CmdLine, " ") if c.cmd.PrependShell { // TODO: Support shell command here shell := []string{"/bin/sh", "-c"} - return strings.Join(append(shell, cmdLine), " ") + appendedShell := append(cmd, shell...) + return strings.Join(append(appendedShell, cmdLine), " ") } - return cmdLine + return strings.Join(append(cmd, cmdLine), " ") } From 58e70d8b77ba499f3d77de6de726a25d935c8144 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 20 Mar 2018 15:56:31 -0700 Subject: [PATCH 05/23] Environment replacement --- pkg/util/command_util.go | 23 +++++++++++++++++++++++ pkg/util/command_util_test.go | 17 +++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 pkg/util/command_util.go create mode 100644 pkg/util/command_util_test.go diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go new file mode 100644 index 000000000..b904a4987 --- /dev/null +++ b/pkg/util/command_util.go @@ -0,0 +1,23 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +// ResolvEnvironmentReplacement resolves replacing env variables in fp from envs +// Ex: fp = $foo/newdir, envs = [foo=/foodir], then this should return /foodir/newdir +func ResolveEnvironmentReplacement(fp string, envs []string) string { + +} diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go new file mode 100644 index 000000000..3a65ebf87 --- /dev/null +++ b/pkg/util/command_util_test.go @@ -0,0 +1,17 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util From cf2423112123da94b5f89559978b267efbd7c710 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 20 Mar 2018 17:15:09 -0700 Subject: [PATCH 06/23] WIP --- pkg/commands/env.go | 24 +++++--------- pkg/commands/env_test.go | 37 +++++++++++++++++++++ pkg/commands/expose.go | 39 +++++++++++++--------- pkg/commands/expose_test.go | 27 +++++++++++++-- pkg/util/command_util.go | 29 ++++++++++++++-- pkg/util/command_util_test.go | 62 +++++++++++++++++++++++++++++++++++ 6 files changed, 181 insertions(+), 37 deletions(-) diff --git a/pkg/commands/env.go b/pkg/commands/env.go index f7f004940..23fbfd658 100644 --- a/pkg/commands/env.go +++ b/pkg/commands/env.go @@ -17,11 +17,9 @@ limitations under the License. package commands import ( - "bytes" + "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util" "github.com/containers/image/manifest" "github.com/docker/docker/builder/dockerfile/instructions" - "github.com/docker/docker/builder/dockerfile/parser" - "github.com/docker/docker/builder/dockerfile/shell" "github.com/sirupsen/logrus" "os" "strings" @@ -33,26 +31,20 @@ type EnvCommand struct { func (e *EnvCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Info("cmd: ENV") - // The dockerfile/shell package handles processing env values - // It handles escape characters and supports expansion from the config.Env array - // Shlex handles some of the following use cases (these and more are tested in integration tests) - // ""a'b'c"" -> "a'b'c" - // "Rex\ The\ Dog \" -> "Rex The Dog" - // "a\"b" -> "a"b" + envString := envToString(e.cmd) - p, err := parser.Parse(bytes.NewReader([]byte(envString))) - if err != nil { - return err - } - shlex := shell.NewLex(p.EscapeToken) newEnvs := e.cmd.Env for index, pair := range newEnvs { - expandedValue, err := shlex.ProcessWord(pair.Value, config.Env) + expandedKey, err := util.ResolveEnvironmentReplacement(envString, pair.Key, config.Env) + if err != nil { + return err + } + expandedValue, err := util.ResolveEnvironmentReplacement(envString, pair.Value, config.Env) if err != nil { return err } newEnvs[index] = instructions.KeyValuePair{ - Key: pair.Key, + Key: expandedKey, Value: expandedValue, } logrus.Infof("Setting environment variable %s=%s", pair.Key, expandedValue) diff --git a/pkg/commands/env_test.go b/pkg/commands/env_test.go index f0cccdaa4..c7c767059 100644 --- a/pkg/commands/env_test.go +++ b/pkg/commands/env_test.go @@ -71,3 +71,40 @@ func TestEnvToString(t *testing.T) { actualString := envToString(envCmd) testutil.CheckErrorAndDeepEqual(t, false, nil, expectedString, actualString) } + +func Test_EnvExecute(t *testing.T) { + cfg := &manifest.Schema2Config{ + Env: []string{ + "path=/usr/", + "home=/root", + }, + } + + envCmd := &EnvCommand{ + &instructions.EnvCommand{ + Env: []instructions.KeyValuePair{ + { + Key: "path", + Value: "/some/path", + }, + { + Key: "HOME", + Value: "$home", + }, + { + Key: "$path", + Value: "$home", + }, + }, + }, + } + + expectedEnvs := []string{ + "path=/some/path", + "home=/root", + "HOME=/root", + "/usr/=/root", + } + err := envCmd.ExecuteCommand(cfg) + testutil.CheckErrorAndDeepEqual(t, false, err, expectedEnvs, cfg.Env) +} diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index daf4dc781..883a4cd35 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -18,6 +18,7 @@ package commands import ( "fmt" + "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util" "github.com/containers/image/manifest" "github.com/docker/docker/builder/dockerfile/instructions" "github.com/sirupsen/logrus" @@ -29,25 +30,16 @@ type ExposeCommand struct { } func (r *ExposeCommand) ExecuteCommand(config *manifest.Schema2Config) error { - return updateExposedPorts(r.cmd.Ports, config) -} - -func validProtocol(protocol string) bool { - validProtocols := [2]string{"tcp", "udp"} - for _, p := range validProtocols { - if protocol == p { - return true - } - } - return false -} - -func updateExposedPorts(ports []string, config *manifest.Schema2Config) error { // Grab the currently exposed ports existingPorts := config.ExposedPorts - + exposeString := r.exposeToString() // Add any new ones in - for _, p := range ports { + for _, p := range r.cmd.Ports { + // Resolve any environment variables + p, err := util.ResolveEnvironmentReplacement(exposeString, p, config.Env) + if err != nil { + return err + } // Add the default protocol if one isn't specified if !strings.Contains(p, "/") { p = p + "/tcp" @@ -64,6 +56,21 @@ func updateExposedPorts(ports []string, config *manifest.Schema2Config) error { return nil } +func validProtocol(protocol string) bool { + validProtocols := [2]string{"tcp", "udp"} + for _, p := range validProtocols { + if protocol == p { + return true + } + } + return false +} + +func (r *ExposeCommand) exposeToString() string { + expose := []string{"EXPOSE"} + return strings.Join(append(expose, r.cmd.Ports...), " ") +} + func (r *ExposeCommand) FilesToSnapshot() []string { return []string{} } diff --git a/pkg/commands/expose_test.go b/pkg/commands/expose_test.go index fc7fb2c0e..ad23cca25 100644 --- a/pkg/commands/expose_test.go +++ b/pkg/commands/expose_test.go @@ -19,6 +19,7 @@ package commands import ( "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" "testing" ) @@ -27,6 +28,10 @@ func TestUpdateExposedPorts(t *testing.T) { ExposedPorts: manifest.Schema2PortSet{ "8080/tcp": {}, }, + Env: []string{ + "port=udp", + "num=8085", + }, } ports := []string{ @@ -34,6 +39,15 @@ func TestUpdateExposedPorts(t *testing.T) { "8081/tcp", "8082", "8083/udp", + "8084/$port", + "$num", + "$num/$port", + } + + exposeCmd := &ExposeCommand{ + &instructions.ExposeCommand{ + Ports: ports, + }, } expectedPorts := manifest.Schema2PortSet{ @@ -41,9 +55,12 @@ func TestUpdateExposedPorts(t *testing.T) { "8081/tcp": {}, "8082/tcp": {}, "8083/udp": {}, + "8084/udp": {}, + "8085/tcp": {}, + "8085/udp": {}, } - err := updateExposedPorts(ports, cfg) + err := exposeCmd.ExecuteCommand(cfg) testutil.CheckErrorAndDeepEqual(t, false, err, expectedPorts, cfg.ExposedPorts) } @@ -56,6 +73,12 @@ func TestInvalidProtocol(t *testing.T) { "80/garbage", } - err := updateExposedPorts(ports, cfg) + exposeCmd := &ExposeCommand{ + &instructions.ExposeCommand{ + Ports: ports, + }, + } + + err := exposeCmd.ExecuteCommand(cfg) testutil.CheckErrorAndDeepEqual(t, true, err, nil, nil) } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index b904a4987..de10fed42 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -16,8 +16,31 @@ limitations under the License. package util -// ResolvEnvironmentReplacement resolves replacing env variables in fp from envs -// Ex: fp = $foo/newdir, envs = [foo=/foodir], then this should return /foodir/newdir -func ResolveEnvironmentReplacement(fp string, envs []string) string { +import ( + "bytes" + "github.com/docker/docker/builder/dockerfile/parser" + "github.com/docker/docker/builder/dockerfile/shell" + "path/filepath" +) +// ResolveEnvironmentReplacement resolves replacing env variables in some text from envs +// It takes in a string representation of the command, the value to be resolved, and a list of envs (config.Env) +// Ex: fp = $foo/newdir, envs = [foo=/foodir], then this should return /foodir/newdir +// The dockerfile/shell package handles processing env values +// It handles escape characters and supports expansion from the config.Env array +// Shlex handles some of the following use cases (these and more are tested in integration tests) +// ""a'b'c"" -> "a'b'c" +// "Rex\ The\ Dog \" -> "Rex The Dog" +// "a\"b" -> "a"b" +func ResolveEnvironmentReplacement(command, value string, envs []string) (string, error) { + p, err := parser.Parse(bytes.NewReader([]byte(command))) + if err != nil { + return "", err + } + shlex := shell.NewLex(p.EscapeToken) + processedWord, err := shlex.ProcessWord(value, envs) + if err != nil { + return "", err + } + return filepath.Clean(processedWord), nil } diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 3a65ebf87..59cdc2454 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -15,3 +15,65 @@ limitations under the License. */ package util + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" + "testing" +) + +var testEnvReplacement = []struct { + path string + command string + envs []string + expectedPath string +}{ + { + path: "/simple/path", + command: "WORKDIR /simple/path", + envs: []string{ + "simple=/path/", + }, + expectedPath: "/simple/path", + }, + { + path: "${a}/b", + command: "WORKDIR ${a}/b", + envs: []string{ + "a=/path/", + "b=/path2/", + }, + expectedPath: "/path/b", + }, + { + path: "/$a/b", + command: "COPY ${a}/b /c/", + envs: []string{ + "a=/path/", + "b=/path2/", + }, + expectedPath: "/path/b", + }, + { + path: "\\$foo", + command: "COPY \\$foo /quux", + envs: []string{ + "foo=/path/", + }, + expectedPath: "$foo", + }, + { + path: "8080/$protocol", + command: "EXPOSE 8080/$protocol", + envs: []string{ + "protocol=udp", + }, + expectedPath: "8080/udp", + }, +} + +func Test_EnvReplacement(t *testing.T) { + for _, test := range testEnvReplacement { + actualPath, err := ResolveEnvironmentReplacement(test.command, test.path, test.envs) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedPath, actualPath) + } +} From ba77ba0822d337bed000235525bfe7c97754fb57 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Wed, 21 Mar 2018 11:34:37 -0700 Subject: [PATCH 07/23] adding LABEL command --- pkg/commands/commands.go | 2 ++ pkg/commands/expose.go | 2 +- pkg/commands/label.go | 57 ++++++++++++++++++++++++++++++++++++++ pkg/commands/label_test.go | 34 +++++++++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 pkg/commands/label.go create mode 100644 pkg/commands/label_test.go diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 742fd5ef3..3889a9ffd 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -42,6 +42,8 @@ func GetCommand(cmd instructions.Command) (DockerCommand, error) { return &ExposeCommand{cmd: c}, nil case *instructions.EnvCommand: return &EnvCommand{cmd: c}, nil + case *instructions.LabelCommand: + return &LabelCommand{cmd: c}, nil } return nil, errors.Errorf("%s is not a supported command", cmd.Name()) } diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index daf4dc781..716b03294 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -69,6 +69,6 @@ func (r *ExposeCommand) FilesToSnapshot() []string { } func (r *ExposeCommand) CreatedBy() string { - s := []string{"/bin/sh", "-c"} + s := []string{r.cmd.Name()} return strings.Join(append(s, r.cmd.Ports...), " ") } diff --git a/pkg/commands/label.go b/pkg/commands/label.go new file mode 100644 index 000000000..d83811eec --- /dev/null +++ b/pkg/commands/label.go @@ -0,0 +1,57 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" + "strings" +) + +type LabelCommand struct { + cmd *instructions.LabelCommand +} + +func (r *LabelCommand) ExecuteCommand(config *manifest.Schema2Config) error { + return updateLabels(r.cmd.Labels, config) +} + +func updateLabels(labels []instructions.KeyValuePair, config *manifest.Schema2Config) error { + existingLabels := config.Labels + + for _, kvp := range labels { + logrus.Infof("Applying label %s=%s", kvp.Key, kvp.Value) + existingLabels[kvp.Key] = kvp.Value + } + + config.Labels = existingLabels + return nil + +} + +func (r *LabelCommand) FilesToSnapshot() []string { + return []string{} +} + +func (r *LabelCommand) CreatedBy() string { + l := []string{r.cmd.Name()} + for _, kvp := range r.cmd.Labels { + l = append(l, kvp.String()) + } + return strings.Join(l, " ") +} diff --git a/pkg/commands/label_test.go b/pkg/commands/label_test.go new file mode 100644 index 000000000..9567f6150 --- /dev/null +++ b/pkg/commands/label_test.go @@ -0,0 +1,34 @@ +package commands + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "testing" +) + +func TestUpdateLabels(t *testing.T) { + cfg := &manifest.Schema2Config{ + Labels: map[string]string { + "foo": "bar", + }, + } + + labels := []instructions.KeyValuePair{ + { + Key: "foo", + Value: "override", + }, + { + Key: "bar", + Value: "baz", + }, + } + + expectedLabels := map[string]string { + "foo": "override", + "bar": "baz", + } + updateLabels(labels, cfg) + testutil.CheckErrorAndDeepEqual(t, false, nil, expectedLabels, cfg.Labels) +} From f352583bc1533c8792d58840c5928509c4f984f1 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Wed, 21 Mar 2018 11:45:41 -0700 Subject: [PATCH 08/23] boilerplate --- pkg/commands/label_test.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/pkg/commands/label_test.go b/pkg/commands/label_test.go index 9567f6150..23606c3e6 100644 --- a/pkg/commands/label_test.go +++ b/pkg/commands/label_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package commands import ( @@ -9,23 +25,23 @@ import ( func TestUpdateLabels(t *testing.T) { cfg := &manifest.Schema2Config{ - Labels: map[string]string { + Labels: map[string]string{ "foo": "bar", }, } labels := []instructions.KeyValuePair{ { - Key: "foo", + Key: "foo", Value: "override", }, { - Key: "bar", + Key: "bar", Value: "baz", }, } - expectedLabels := map[string]string { + expectedLabels := map[string]string{ "foo": "override", "bar": "baz", } From f56b3e9542d52d38c164a91a080b4f4cfdb27db4 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 21 Mar 2018 13:37:00 -0700 Subject: [PATCH 09/23] Environment replacement --- pkg/util/command_util.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index de10fed42..dde948b84 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -38,9 +38,24 @@ func ResolveEnvironmentReplacement(command, value string, envs []string) (string return "", err } shlex := shell.NewLex(p.EscapeToken) - processedWord, err := shlex.ProcessWord(value, envs) + return shlex.ProcessWord(value, envs) +} + +// ResolveFilepathEnvironmentReplacement replaces env variables in filepaths +// and returns a cleaned version of the path +func ResolveFilepathEnvironmentReplacement(command, value string, envs []string) (string, error) { + p, err := parser.Parse(bytes.NewReader([]byte(command))) if err != nil { return "", err } - return filepath.Clean(processedWord), nil + shlex := shell.NewLex(p.EscapeToken) + fp, err := shlex.ProcessWord(value, envs) + if err != nil { + return "", err + } + fp = filepath.Clean(fp) + if filepath.IsAbs(value) { + fp = filepath.Join(fp, "/") + } + return fp, nil } From 0787a93372f5c44880182ac68f9e8f81ba339473 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 26 Mar 2018 11:11:31 -0700 Subject: [PATCH 10/23] Workdir command and unit tests --- .../dockerfiles/Dockerfile_test_workdir | 10 +++ .../dockerfiles/config_test_workdir.json | 12 +++ integration_tests/integration_test_yaml.go | 71 +++++++++-------- pkg/commands/commands.go | 2 + pkg/commands/run.go | 1 + pkg/commands/workdir.go | 64 +++++++++++++++ pkg/commands/workdir_test.go | 79 +++++++++++++++++++ 7 files changed, 207 insertions(+), 32 deletions(-) create mode 100644 integration_tests/dockerfiles/Dockerfile_test_workdir create mode 100644 integration_tests/dockerfiles/config_test_workdir.json create mode 100644 pkg/commands/workdir.go create mode 100644 pkg/commands/workdir_test.go diff --git a/integration_tests/dockerfiles/Dockerfile_test_workdir b/integration_tests/dockerfiles/Dockerfile_test_workdir new file mode 100644 index 000000000..4fa6b6143 --- /dev/null +++ b/integration_tests/dockerfiles/Dockerfile_test_workdir @@ -0,0 +1,10 @@ +FROM gcr.io/google-appengine/debian9:latest +COPY context/foo foo +WORKDIR /test +WORKDIR workdir +COPY context/foo . +RUN cp foo newfoo +WORKDIR /new/dir +ENV dir /another/new/dir +WORKDIR $dir/newdir +WORKDIR / diff --git a/integration_tests/dockerfiles/config_test_workdir.json b/integration_tests/dockerfiles/config_test_workdir.json new file mode 100644 index 000000000..c87503403 --- /dev/null +++ b/integration_tests/dockerfiles/config_test_workdir.json @@ -0,0 +1,12 @@ +[ + { + "Image1": "gcr.io/kbuild-test/docker-test-workdir:latest", + "Image2": "gcr.io/kbuild-test/kbuild-test-workdir:latest", + "DiffType": "File", + "Diff": { + "Adds": null, + "Dels": null, + "Mods": null + } + } +] \ No newline at end of file diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index e81d4269d..1a91eb54d 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -29,33 +29,40 @@ var fileTests = []struct { context string repo string }{ + // { + // description: "test extract filesystem", + // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_extract_fs", + // configPath: "/workspace/integration_tests/dockerfiles/config_test_extract_fs.json", + // context: "integration_tests/dockerfiles/", + // repo: "extract-filesystem", + // }, + // { + // description: "test run", + // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run", + // configPath: "/workspace/integration_tests/dockerfiles/config_test_run.json", + // context: "integration_tests/dockerfiles/", + // repo: "test-run", + // }, + // { + // description: "test run no files changed", + // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run_2", + // configPath: "/workspace/integration_tests/dockerfiles/config_test_run_2.json", + // context: "integration_tests/dockerfiles/", + // repo: "test-run-2", + // }, + // { + // description: "test copy", + // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_copy", + // configPath: "/workspace/integration_tests/dockerfiles/config_test_copy.json", + // context: "/workspace/integration_tests/", + // repo: "test-copy", + // }, { - description: "test extract filesystem", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_extract_fs", - configPath: "/workspace/integration_tests/dockerfiles/config_test_extract_fs.json", - context: "integration_tests/dockerfiles/", - repo: "extract-filesystem", - }, - { - description: "test run", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run", - configPath: "/workspace/integration_tests/dockerfiles/config_test_run.json", - context: "integration_tests/dockerfiles/", - repo: "test-run", - }, - { - description: "test run no files changed", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run_2", - configPath: "/workspace/integration_tests/dockerfiles/config_test_run_2.json", - context: "integration_tests/dockerfiles/", - repo: "test-run-2", - }, - { - description: "test copy", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_copy", - configPath: "/workspace/integration_tests/dockerfiles/config_test_copy.json", + description: "test workdir", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_workdir", + configPath: "/workspace/integration_tests/dockerfiles/config_test_workdir.json", context: "/workspace/integration_tests/", - repo: "test-copy", + repo: "test-workdir", }, } @@ -66,13 +73,13 @@ var structureTests = []struct { dockerBuildContext string repo string }{ - { - description: "test env", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_env", - repo: "test-env", - dockerBuildContext: "/workspace/integration_tests/dockerfiles/", - structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_env.yaml", - }, + // { + // description: "test env", + // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_env", + // repo: "test-env", + // dockerBuildContext: "/workspace/integration_tests/dockerfiles/", + // structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_env.yaml", + // }, } type step struct { diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 8f6b6a8cc..7e10f64be 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -44,6 +44,8 @@ func GetCommand(cmd instructions.Command, buildcontext string) (DockerCommand, e return &ExposeCommand{cmd: c}, nil case *instructions.EnvCommand: return &EnvCommand{cmd: c}, nil + case *instructions.WorkdirCommand: + return &WorkdirCommand{cmd: c}, nil } return nil, errors.Errorf("%s is not a supported command", cmd.Name()) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index b08cf8800..b6aa2b19c 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -44,6 +44,7 @@ func (r *RunCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Infof("args: %s", newCommand[1:]) cmd := exec.Command(newCommand[0], newCommand[1:]...) + cmd.Dir = config.WorkingDir cmd.Stdout = os.Stdout return cmd.Run() } diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go new file mode 100644 index 000000000..276abec22 --- /dev/null +++ b/pkg/commands/workdir.go @@ -0,0 +1,64 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util" + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" + "os" + "path/filepath" + "strings" +) + +type WorkdirCommand struct { + cmd *instructions.WorkdirCommand + snapshotFiles []string +} + +func (w *WorkdirCommand) ExecuteCommand(config *manifest.Schema2Config) error { + logrus.Info("cmd: workdir") + workdirPath := w.cmd.Path + resolvedWorkingDir, err := util.ResolveEnvironmentReplacement(w.workdirToString(), workdirPath, config.Env, true) + if err != nil { + return err + } + if filepath.IsAbs(resolvedWorkingDir) { + config.WorkingDir = resolvedWorkingDir + } else { + config.WorkingDir = filepath.Join(config.WorkingDir, resolvedWorkingDir) + } + logrus.Infof("Changed working directory to %s", config.WorkingDir) + w.snapshotFiles = []string{config.WorkingDir} + return os.MkdirAll(config.WorkingDir, 0755) +} + +func (w *WorkdirCommand) workdirToString() string { + workdir := []string{"WORKDIR"} + return strings.Join(append(workdir, w.cmd.Path), " ") +} + +// FilesToSnapshot returns the workingdir, which should have been created if it didn't already exist +func (w *WorkdirCommand) FilesToSnapshot() []string { + return w.snapshotFiles +} + +// CreatedBy returns some information about the command for the image config history +func (w *WorkdirCommand) CreatedBy() string { + return w.cmd.Name() + " " + w.cmd.Path +} diff --git a/pkg/commands/workdir_test.go b/pkg/commands/workdir_test.go new file mode 100644 index 000000000..3225980c6 --- /dev/null +++ b/pkg/commands/workdir_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package commands + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "testing" +) + +var workdirTests = []struct { + path string + expectedPath string +}{ + { + path: "/a", + expectedPath: "/a", + }, + { + path: "b", + expectedPath: "/a/b", + }, + { + path: "c", + expectedPath: "/a/b/c", + }, + { + path: "/d", + expectedPath: "/d", + }, + { + path: "$path", + expectedPath: "/d/usr", + }, + { + path: "$home", + expectedPath: "/root", + }, + { + path: "$path/$home", + expectedPath: "/root/usr/root", + }, +} + +func TestWorkdirCommand(t *testing.T) { + + cfg := &manifest.Schema2Config{ + WorkingDir: "/", + Env: []string{ + "path=usr/", + "home=/root", + }, + } + + for _, test := range workdirTests { + cmd := WorkdirCommand{ + cmd: &instructions.WorkdirCommand{ + Path: test.path, + }, + snapshotFiles: []string{}, + } + cmd.ExecuteCommand(cfg) + testutil.CheckErrorAndDeepEqual(t, false, nil, test.expectedPath, cfg.WorkingDir) + } +} From 986074eb45a44c7785154b52534cadbfceefee89 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Mon, 26 Mar 2018 11:38:43 -0700 Subject: [PATCH 11/23] unescaping values when appropriate --- pkg/commands/label.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/commands/label.go b/pkg/commands/label.go index d83811eec..ec80e494f 100644 --- a/pkg/commands/label.go +++ b/pkg/commands/label.go @@ -19,6 +19,7 @@ package commands import ( "github.com/containers/image/manifest" "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/docker/docker/builder/dockerfile/shell" "github.com/sirupsen/logrus" "strings" ) @@ -34,6 +35,18 @@ func (r *LabelCommand) ExecuteCommand(config *manifest.Schema2Config) error { func updateLabels(labels []instructions.KeyValuePair, config *manifest.Schema2Config) error { existingLabels := config.Labels + // Let's unescape values before setting the label + shlex := shell.NewLex('\\') + for index, kvp := range labels { + unescaped, err := shlex.ProcessWord(kvp.Value, []string{}) + if err != nil { + return err + } + labels[index] = instructions.KeyValuePair{ + Key: kvp.Key, + Value: unescaped, + } + } for _, kvp := range labels { logrus.Infof("Applying label %s=%s", kvp.Key, kvp.Value) existingLabels[kvp.Key] = kvp.Value @@ -44,10 +57,12 @@ func updateLabels(labels []instructions.KeyValuePair, config *manifest.Schema2Co } +// No files have changed, this command only touches metadata. func (r *LabelCommand) FilesToSnapshot() []string { return []string{} } +// CreatedBy returns some information about the command for the image config history func (r *LabelCommand) CreatedBy() string { l := []string{r.cmd.Name()} for _, kvp := range r.cmd.Labels { From 4551bd0dc07cbf50ffac46a1cdd93c8a1cf803bc Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Mon, 26 Mar 2018 11:45:59 -0700 Subject: [PATCH 12/23] adding test for escaped words --- pkg/commands/label_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/commands/label_test.go b/pkg/commands/label_test.go index 23606c3e6..02185889e 100644 --- a/pkg/commands/label_test.go +++ b/pkg/commands/label_test.go @@ -39,11 +39,16 @@ func TestUpdateLabels(t *testing.T) { Key: "bar", Value: "baz", }, + { + Key: "multiword", + Value: "lots\\ of\\ words", + }, } expectedLabels := map[string]string{ - "foo": "override", - "bar": "baz", + "foo": "override", + "bar": "baz", + "multiword": "lots of words", } updateLabels(labels, cfg) testutil.CheckErrorAndDeepEqual(t, false, nil, expectedLabels, cfg.Labels) From a5a84ed552f460d80ef62a9679fdabe8b9eb0d76 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 26 Mar 2018 11:38:02 -0700 Subject: [PATCH 13/23] workdir integration test --- .../dockerfiles/Dockerfile_test_workdir | 9 ++- integration_tests/integration_test_yaml.go | 70 +++++++++---------- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/integration_tests/dockerfiles/Dockerfile_test_workdir b/integration_tests/dockerfiles/Dockerfile_test_workdir index 4fa6b6143..6c89e4c11 100644 --- a/integration_tests/dockerfiles/Dockerfile_test_workdir +++ b/integration_tests/dockerfiles/Dockerfile_test_workdir @@ -1,10 +1,13 @@ FROM gcr.io/google-appengine/debian9:latest COPY context/foo foo WORKDIR /test -WORKDIR workdir -COPY context/foo . -RUN cp foo newfoo +# Test that this will be appended on to the previous command, to create /test/workdir +WORKDIR workdir +COPY context/foo ./currentfoo +# Test that the RUN command will happen in the correct directory +RUN cp currentfoo newfoo WORKDIR /new/dir ENV dir /another/new/dir WORKDIR $dir/newdir +WORKDIR $dir/$doesntexist WORKDIR / diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index 1a91eb54d..cda55ac61 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -29,34 +29,34 @@ var fileTests = []struct { context string repo string }{ - // { - // description: "test extract filesystem", - // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_extract_fs", - // configPath: "/workspace/integration_tests/dockerfiles/config_test_extract_fs.json", - // context: "integration_tests/dockerfiles/", - // repo: "extract-filesystem", - // }, - // { - // description: "test run", - // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run", - // configPath: "/workspace/integration_tests/dockerfiles/config_test_run.json", - // context: "integration_tests/dockerfiles/", - // repo: "test-run", - // }, - // { - // description: "test run no files changed", - // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run_2", - // configPath: "/workspace/integration_tests/dockerfiles/config_test_run_2.json", - // context: "integration_tests/dockerfiles/", - // repo: "test-run-2", - // }, - // { - // description: "test copy", - // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_copy", - // configPath: "/workspace/integration_tests/dockerfiles/config_test_copy.json", - // context: "/workspace/integration_tests/", - // repo: "test-copy", - // }, + { + description: "test extract filesystem", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_extract_fs", + configPath: "/workspace/integration_tests/dockerfiles/config_test_extract_fs.json", + context: "integration_tests/dockerfiles/", + repo: "extract-filesystem", + }, + { + description: "test run", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run", + configPath: "/workspace/integration_tests/dockerfiles/config_test_run.json", + context: "integration_tests/dockerfiles/", + repo: "test-run", + }, + { + description: "test run no files changed", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_run_2", + configPath: "/workspace/integration_tests/dockerfiles/config_test_run_2.json", + context: "integration_tests/dockerfiles/", + repo: "test-run-2", + }, + { + description: "test copy", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_copy", + configPath: "/workspace/integration_tests/dockerfiles/config_test_copy.json", + context: "/workspace/integration_tests/", + repo: "test-copy", + }, { description: "test workdir", dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_workdir", @@ -73,13 +73,13 @@ var structureTests = []struct { dockerBuildContext string repo string }{ - // { - // description: "test env", - // dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_env", - // repo: "test-env", - // dockerBuildContext: "/workspace/integration_tests/dockerfiles/", - // structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_env.yaml", - // }, + { + description: "test env", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_env", + repo: "test-env", + dockerBuildContext: "/workspace/integration_tests/dockerfiles/", + structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_env.yaml", + }, } type step struct { From 96b0b12bc73903779c852b8736cf3fa849e38095 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Mon, 26 Mar 2018 14:13:20 -0700 Subject: [PATCH 14/23] adding extra test case --- pkg/commands/label_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/commands/label_test.go b/pkg/commands/label_test.go index 02185889e..f997611c3 100644 --- a/pkg/commands/label_test.go +++ b/pkg/commands/label_test.go @@ -43,12 +43,17 @@ func TestUpdateLabels(t *testing.T) { Key: "multiword", Value: "lots\\ of\\ words", }, + { + Key: "backslashes", + Value: "lots\\\\ of\\\\ words", + }, } expectedLabels := map[string]string{ - "foo": "override", - "bar": "baz", - "multiword": "lots of words", + "foo": "override", + "bar": "baz", + "multiword": "lots of words", + "backslashes": "lots\\ of\\ words", } updateLabels(labels, cfg) testutil.CheckErrorAndDeepEqual(t, false, nil, expectedLabels, cfg.Labels) From 54a53489b21bf3a1068203d347e45c61894e284f Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 26 Mar 2018 16:56:28 -0700 Subject: [PATCH 15/23] Added comment to unit test --- pkg/commands/workdir_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/commands/workdir_test.go b/pkg/commands/workdir_test.go index 3225980c6..439d77fd5 100644 --- a/pkg/commands/workdir_test.go +++ b/pkg/commands/workdir_test.go @@ -22,6 +22,10 @@ import ( "testing" ) +// Each test here changes the same WorkingDir field in the config +// So, some of the tests build off of each other +// This is needed to make sure WorkingDir handles paths correctly +// For example, if WORKDIR specifies a non-absolute path, it should be appended to the current WORKDIR var workdirTests = []struct { path string expectedPath string From b64f23b07800f5119a6dfc4665f03a42da86bf1d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 27 Mar 2018 09:44:51 -0700 Subject: [PATCH 16/23] Use default token for parsing --- pkg/commands/copy.go | 7 +------ pkg/commands/env.go | 13 ++----------- pkg/commands/expose.go | 8 +------- pkg/util/command_util.go | 15 +++++---------- pkg/util/command_util_test.go | 2 +- 5 files changed, 10 insertions(+), 35 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 96db34911..ff3ad9f9b 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -40,7 +40,7 @@ func (c *CopyCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Infof("dest: %s", dest) // First, resolve any environment replacement - resolvedEnvs, err := util.ResolveEnvironmentReplacementList(c.copyToString(), c.cmd.SourcesAndDest, config.Env, true) + resolvedEnvs, err := util.ResolveEnvironmentReplacementList(c.cmd.SourcesAndDest, config.Env, true) if err != nil { return err } @@ -86,11 +86,6 @@ func (c *CopyCommand) ExecuteCommand(config *manifest.Schema2Config) error { return nil } -func (c *CopyCommand) copyToString() string { - copy := []string{"COPY"} - return strings.Join(append(copy, c.cmd.SourcesAndDest...), " ") -} - // FilesToSnapshot should return an empty array if still nil; no files were changed func (c *CopyCommand) FilesToSnapshot() []string { return c.snapshotFiles diff --git a/pkg/commands/env.go b/pkg/commands/env.go index 87e668efd..94e1edabe 100644 --- a/pkg/commands/env.go +++ b/pkg/commands/env.go @@ -31,14 +31,13 @@ type EnvCommand struct { func (e *EnvCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Info("cmd: ENV") - envString := envToString(e.cmd) newEnvs := e.cmd.Env for index, pair := range newEnvs { - expandedKey, err := util.ResolveEnvironmentReplacement(envString, pair.Key, config.Env, false) + expandedKey, err := util.ResolveEnvironmentReplacement(pair.Key, config.Env, false) if err != nil { return err } - expandedValue, err := util.ResolveEnvironmentReplacement(envString, pair.Value, config.Env, false) + expandedValue, err := util.ResolveEnvironmentReplacement(pair.Value, config.Env, false) if err != nil { return err } @@ -89,14 +88,6 @@ Loop: return nil } -func envToString(cmd *instructions.EnvCommand) string { - env := []string{"ENV"} - for _, kvp := range cmd.Env { - env = append(env, kvp.Key+"="+kvp.Value) - } - return strings.Join(env, " ") -} - // We know that no files have changed, so return an empty array func (e *EnvCommand) FilesToSnapshot() []string { return []string{} diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index 7421a8697..c736a2eb3 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -32,11 +32,10 @@ type ExposeCommand struct { func (r *ExposeCommand) ExecuteCommand(config *manifest.Schema2Config) error { // Grab the currently exposed ports existingPorts := config.ExposedPorts - exposeString := r.exposeToString() // Add any new ones in for _, p := range r.cmd.Ports { // Resolve any environment variables - p, err := util.ResolveEnvironmentReplacement(exposeString, p, config.Env, false) + p, err := util.ResolveEnvironmentReplacement(p, config.Env, false) if err != nil { return err } @@ -66,11 +65,6 @@ func validProtocol(protocol string) bool { return false } -func (r *ExposeCommand) exposeToString() string { - expose := []string{"EXPOSE"} - return strings.Join(append(expose, r.cmd.Ports...), " ") -} - func (r *ExposeCommand) FilesToSnapshot() []string { return []string{} } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index ab56dd923..0f6472f79 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -17,7 +17,6 @@ limitations under the License. package util import ( - "bytes" "github.com/docker/docker/builder/dockerfile/instructions" "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/builder/dockerfile/shell" @@ -29,10 +28,10 @@ import ( ) // ResolveEnvironmentReplacement resolves a list of values by calling resolveEnvironmentReplacement -func ResolveEnvironmentReplacementList(command string, values, envs []string, isFilepath bool) ([]string, error) { +func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ([]string, error) { var resolvedValues []string for _, value := range values { - resolved, err := ResolveEnvironmentReplacement(command, value, envs, isFilepath) + resolved, err := ResolveEnvironmentReplacement(value, envs, isFilepath) logrus.Debugf("Resolved %s to %s", value, resolved) if err != nil { return nil, err @@ -42,7 +41,7 @@ func ResolveEnvironmentReplacementList(command string, values, envs []string, is return resolvedValues, nil } -// resolveEnvironmentReplacement resolves replacing env variables in some text from envs +// ResolveEnvironmentReplacement resolves replacing env variables in some text from envs // It takes in a string representation of the command, the value to be resolved, and a list of envs (config.Env) // Ex: fp = $foo/newdir, envs = [foo=/foodir], then this should return /foodir/newdir // The dockerfile/shell package handles processing env values @@ -51,12 +50,8 @@ func ResolveEnvironmentReplacementList(command string, values, envs []string, is // ""a'b'c"" -> "a'b'c" // "Rex\ The\ Dog \" -> "Rex The Dog" // "a\"b" -> "a"b" -func ResolveEnvironmentReplacement(command, value string, envs []string, isFilepath bool) (string, error) { - p, err := parser.Parse(bytes.NewReader([]byte(command))) - if err != nil { - return "", err - } - shlex := shell.NewLex(p.EscapeToken) +func ResolveEnvironmentReplacement(value string, envs []string, isFilepath bool) (string, error) { + shlex := shell.NewLex(parser.DefaultEscapeToken) fp, err := shlex.ProcessWord(value, envs) if !isFilepath { return fp, err diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index f5ec7ae7e..611ad7b81 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -98,7 +98,7 @@ var testEnvReplacement = []struct { func Test_EnvReplacement(t *testing.T) { for _, test := range testEnvReplacement { - actualPath, err := ResolveEnvironmentReplacement(test.command, test.path, test.envs, test.isFilepath) + actualPath, err := ResolveEnvironmentReplacement(test.path, test.envs, test.isFilepath) testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedPath, actualPath) } From e885d6a5e652fc8ac50bae6ea2737f65fa6fc330 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 27 Mar 2018 09:48:37 -0700 Subject: [PATCH 17/23] Use default token for env replacement parsing --- pkg/commands/copy.go | 2 +- pkg/commands/env_test.go | 19 ------------------- pkg/commands/label.go | 5 ++--- 3 files changed, 3 insertions(+), 23 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index ff3ad9f9b..e9f8cab59 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -44,7 +44,7 @@ func (c *CopyCommand) ExecuteCommand(config *manifest.Schema2Config) error { if err != nil { return err } - dest = resolvedEnvs[len(c.cmd.SourcesAndDest)-1] + dest = resolvedEnvs[len(resolvedEnvs)-1] // Get a map of [src]:[files rooted at src] srcMap, err := util.ResolveSources(resolvedEnvs, c.buildcontext) if err != nil { diff --git a/pkg/commands/env_test.go b/pkg/commands/env_test.go index a6c8af246..5aedcff02 100644 --- a/pkg/commands/env_test.go +++ b/pkg/commands/env_test.go @@ -53,25 +53,6 @@ func TestUpdateEnvConfig(t *testing.T) { updateConfigEnv(newEnvs, cfg) testutil.CheckErrorAndDeepEqual(t, false, nil, expectedEnvArray, cfg.Env) } - -func TestEnvToString(t *testing.T) { - envCmd := &instructions.EnvCommand{ - Env: []instructions.KeyValuePair{ - { - Key: "PATH", - Value: "/some/path", - }, - { - Key: "HOME", - Value: "/root", - }, - }, - } - expectedString := "ENV PATH=/some/path HOME=/root" - actualString := envToString(envCmd) - testutil.CheckErrorAndDeepEqual(t, false, nil, expectedString, actualString) -} - func Test_EnvExecute(t *testing.T) { cfg := &manifest.Schema2Config{ Env: []string{ diff --git a/pkg/commands/label.go b/pkg/commands/label.go index ec80e494f..3cf8896db 100644 --- a/pkg/commands/label.go +++ b/pkg/commands/label.go @@ -17,9 +17,9 @@ limitations under the License. package commands import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util" "github.com/containers/image/manifest" "github.com/docker/docker/builder/dockerfile/instructions" - "github.com/docker/docker/builder/dockerfile/shell" "github.com/sirupsen/logrus" "strings" ) @@ -36,9 +36,8 @@ func updateLabels(labels []instructions.KeyValuePair, config *manifest.Schema2Co existingLabels := config.Labels // Let's unescape values before setting the label - shlex := shell.NewLex('\\') for index, kvp := range labels { - unescaped, err := shlex.ProcessWord(kvp.Value, []string{}) + unescaped, err := util.ResolveEnvironmentReplacement(kvp.Value, []string{}, false) if err != nil { return err } From ad17811c39cee21676f7291c973a4d0017f18b1b Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 27 Mar 2018 15:15:55 -0700 Subject: [PATCH 18/23] Update workdir to use default escape token --- pkg/commands/workdir.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go index 276abec22..f249608ac 100644 --- a/pkg/commands/workdir.go +++ b/pkg/commands/workdir.go @@ -23,7 +23,6 @@ import ( "github.com/sirupsen/logrus" "os" "path/filepath" - "strings" ) type WorkdirCommand struct { @@ -34,7 +33,7 @@ type WorkdirCommand struct { func (w *WorkdirCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Info("cmd: workdir") workdirPath := w.cmd.Path - resolvedWorkingDir, err := util.ResolveEnvironmentReplacement(w.workdirToString(), workdirPath, config.Env, true) + resolvedWorkingDir, err := util.ResolveEnvironmentReplacement(workdirPath, config.Env, true) if err != nil { return err } @@ -48,11 +47,6 @@ func (w *WorkdirCommand) ExecuteCommand(config *manifest.Schema2Config) error { return os.MkdirAll(config.WorkingDir, 0755) } -func (w *WorkdirCommand) workdirToString() string { - workdir := []string{"WORKDIR"} - return strings.Join(append(workdir, w.cmd.Path), " ") -} - // FilesToSnapshot returns the workingdir, which should have been created if it didn't already exist func (w *WorkdirCommand) FilesToSnapshot() []string { return w.snapshotFiles From bc78e2b83887eef53d55a8726b9a30bf78bf5014 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Thu, 29 Mar 2018 11:53:31 -0700 Subject: [PATCH 19/23] adding USER command --- .../dockerfiles/Dockerfile_test_user_run | 19 +++++ integration_tests/dockerfiles/test_user.yaml | 15 ++++ integration_tests/integration_test_yaml.go | 7 ++ pkg/commands/expose.go | 1 + pkg/commands/label.go | 1 + pkg/commands/run.go | 22 +++++ pkg/commands/user.go | 79 ++++++++++++++++++ pkg/commands/user_test.go | 83 +++++++++++++++++++ 8 files changed, 227 insertions(+) create mode 100644 integration_tests/dockerfiles/Dockerfile_test_user_run create mode 100644 integration_tests/dockerfiles/test_user.yaml create mode 100644 pkg/commands/user.go create mode 100644 pkg/commands/user_test.go diff --git a/integration_tests/dockerfiles/Dockerfile_test_user_run b/integration_tests/dockerfiles/Dockerfile_test_user_run new file mode 100644 index 000000000..2c58a6b7a --- /dev/null +++ b/integration_tests/dockerfiles/Dockerfile_test_user_run @@ -0,0 +1,19 @@ +# Copyright 2018 Google, Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +FROM gcr.io/google-appengine/debian9 +RUN useradd testuser +RUN groupadd testgroup +USER testuser:testgroup +RUN echo "hey" > /etc/foo diff --git a/integration_tests/dockerfiles/test_user.yaml b/integration_tests/dockerfiles/test_user.yaml new file mode 100644 index 000000000..9a4bed1dc --- /dev/null +++ b/integration_tests/dockerfiles/test_user.yaml @@ -0,0 +1,15 @@ +schemaVersion: '2.0.0' +commandTests: +- name: 'whoami' + command: 'whoami' + expectedOutput: ['testuser'] + excludedOutput: ['root'] +- name: 'file owner' + command: 'ls' + args: ['-l', '/tmp/foo'] + expectedOutput: ['.*testuser.*', '.*testgroup.*'] + excludedOutput: ['.*root.*'] +fileContentTests: +- name: "/tmp/foo" + path: "/tmp/foo" + expectedContent: ["hey"] diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index 3129456ff..36c53d84d 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -80,6 +80,13 @@ var structureTests = []struct { dockerBuildContext: "/workspace/integration_tests/dockerfiles/", structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_metadata.yaml", }, + { + description: "test user command", + dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_user_run", + repo: "test-user", + dockerBuildContext: "/workspace/integration_tests/dockerfiles/", + structureTestYamlPath: "/workspace/integration_tests/dockerfiles/test_user.yaml", + }, } type step struct { diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index fa12ec110..fc9d6fe75 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -30,6 +30,7 @@ type ExposeCommand struct { } func (r *ExposeCommand) ExecuteCommand(config *manifest.Schema2Config) error { + logrus.Info("cmd: EXPOSE") // Grab the currently exposed ports existingPorts := config.ExposedPorts // Add any new ones in diff --git a/pkg/commands/label.go b/pkg/commands/label.go index 3cf8896db..81b9bab56 100644 --- a/pkg/commands/label.go +++ b/pkg/commands/label.go @@ -29,6 +29,7 @@ type LabelCommand struct { } func (r *LabelCommand) ExecuteCommand(config *manifest.Schema2Config) error { + logrus.Info("cmd: LABEL") return updateLabels(r.cmd.Labels, config) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index b08cf8800..b7e59bae8 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -22,7 +22,9 @@ import ( "github.com/sirupsen/logrus" "os" "os/exec" + "strconv" "strings" + "syscall" ) type RunCommand struct { @@ -45,6 +47,26 @@ func (r *RunCommand) ExecuteCommand(config *manifest.Schema2Config) error { cmd := exec.Command(newCommand[0], newCommand[1:]...) cmd.Stdout = os.Stdout + // If specified, run the command as a specific user + if config.User != "" { + userAndGroup := strings.Split(config.User, ":") + // uid and gid need to be uint32 + uid64, err := strconv.ParseUint(userAndGroup[0], 10, 32) + if err != nil { + return err + } + uid := uint32(uid64) + var gid uint32 + if len(userAndGroup) > 1 { + gid64, err := strconv.ParseUint(userAndGroup[1], 10, 32) + if err != nil { + return err + } + gid = uint32(gid64) + } + cmd.SysProcAttr = &syscall.SysProcAttr{} + cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid} + } return cmd.Run() } diff --git a/pkg/commands/user.go b/pkg/commands/user.go new file mode 100644 index 000000000..b207eaf38 --- /dev/null +++ b/pkg/commands/user.go @@ -0,0 +1,79 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/sirupsen/logrus" + "os/user" + "strings" +) + +type UserCommand struct { + cmd *instructions.UserCommand +} + +func (r *UserCommand) ExecuteCommand(config *manifest.Schema2Config) error { + logrus.Info("cmd: USER") + u := r.cmd.User + userAndGroup := strings.Split(u, ":") + userStr := userAndGroup[0] + var groupStr string + if len(userAndGroup) > 1 { + groupStr = userAndGroup[1] + } + + // Lookup by username + userObj, err := user.Lookup(userStr) + if err != nil { + if _, ok := err.(user.UnknownUserError); ok { + // Lookup by id + userObj, err = user.LookupId(userStr) + if err != nil { + return err + } + } else { + return err + } + } + + // Same dance with groups + var group *user.Group + if groupStr != "" { + group, err = user.LookupGroup(groupStr) + if err != nil { + if _, ok := err.(user.UnknownGroupError); ok { + group, err = user.LookupGroupId(groupStr) + if err != nil { + return err + } + } else { + return err + } + } + } + + uid := userObj.Uid + if group != nil { + uid = uid + ":" + group.Gid + } + + logrus.Infof("Setting user to %s", uid) + config.User = uid + return nil +} diff --git a/pkg/commands/user_test.go b/pkg/commands/user_test.go new file mode 100644 index 000000000..fb5641939 --- /dev/null +++ b/pkg/commands/user_test.go @@ -0,0 +1,83 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package commands + +import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/testutil" + "github.com/containers/image/manifest" + "github.com/docker/docker/builder/dockerfile/instructions" + "testing" +) + +var userTests = []struct { + user string + expectedUid string + shouldError bool +}{ + { + user: "root", + expectedUid: "0", + shouldError: false, + }, + { + user: "0", + expectedUid: "0", + shouldError: false, + }, + { + user: "fakeUser", + expectedUid: "", + shouldError: true, + }, + { + user: "root:root", + expectedUid: "0:0", + shouldError: false, + }, + { + user: "0:root", + expectedUid: "0:0", + shouldError: false, + }, + { + user: "root:0", + expectedUid: "0:0", + shouldError: false, + }, + { + user: "0:0", + expectedUid: "0:0", + shouldError: false, + }, + { + user: "root:fakeGroup", + expectedUid: "", + shouldError: true, + }, +} + +func TestUpdateUser(t *testing.T) { + for _, test := range userTests { + cfg := &manifest.Schema2Config{} + cmd := UserCommand{ + &instructions.UserCommand{ + User: test.user, + }, + } + err := cmd.ExecuteCommand(cfg) + testutil.CheckErrorAndDeepEqual(t, test.shouldError, err, test.expectedUid, cfg.User) + } +} From b315cf104966cccdbab857b74e5c1f0bd7662fa6 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Thu, 29 Mar 2018 11:54:51 -0700 Subject: [PATCH 20/23] adding user command to switch --- pkg/commands/commands.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 08fa3ceb1..0c4eef17d 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -50,6 +50,8 @@ func GetCommand(cmd instructions.Command, buildcontext string) (DockerCommand, e return &EntrypointCommand{cmd: c}, nil case *instructions.LabelCommand: return &LabelCommand{cmd: c}, nil + case *instructions.UserCommand: + return &UserCommand{cmd: c}, nil } return nil, errors.Errorf("%s is not a supported command", cmd.Name()) } From abc85905c0eadeae8dc5c4990ecd29fb51d1f4ac Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Thu, 29 Mar 2018 12:54:00 -0700 Subject: [PATCH 21/23] adding necessary functions --- pkg/commands/user.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/commands/user.go b/pkg/commands/user.go index b207eaf38..ae5a9e4e9 100644 --- a/pkg/commands/user.go +++ b/pkg/commands/user.go @@ -77,3 +77,12 @@ func (r *UserCommand) ExecuteCommand(config *manifest.Schema2Config) error { config.User = uid return nil } + +func (r *UserCommand) FilesToSnapshot() []string { + return []string{} +} + +func (r *UserCommand) CreatedBy() string { + s := []string{r.cmd.Name(), r.cmd.User} + return strings.Join(s, " ") +} From 7ae8f35eb95a897e3990df9726cf32c785743291 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Thu, 29 Mar 2018 13:35:37 -0700 Subject: [PATCH 22/23] write to /tmp --- integration_tests/dockerfiles/Dockerfile_test_user_run | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_tests/dockerfiles/Dockerfile_test_user_run b/integration_tests/dockerfiles/Dockerfile_test_user_run index 2c58a6b7a..a71fb535e 100644 --- a/integration_tests/dockerfiles/Dockerfile_test_user_run +++ b/integration_tests/dockerfiles/Dockerfile_test_user_run @@ -16,4 +16,4 @@ FROM gcr.io/google-appengine/debian9 RUN useradd testuser RUN groupadd testgroup USER testuser:testgroup -RUN echo "hey" > /etc/foo +RUN echo "hey" > /tmp/foo From da0231a4d1cd6d18181328d4fb1754692614d5c5 Mon Sep 17 00:00:00 2001 From: sharifelgamal Date: Fri, 30 Mar 2018 10:13:35 -0700 Subject: [PATCH 23/23] adding support of env variable replacement --- pkg/commands/user.go | 11 +++++++++-- pkg/commands/user_test.go | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/commands/user.go b/pkg/commands/user.go index ae5a9e4e9..d2e4cff61 100644 --- a/pkg/commands/user.go +++ b/pkg/commands/user.go @@ -17,6 +17,7 @@ limitations under the License. package commands import ( + "github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util" "github.com/containers/image/manifest" "github.com/docker/docker/builder/dockerfile/instructions" "github.com/sirupsen/logrus" @@ -32,10 +33,16 @@ func (r *UserCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Info("cmd: USER") u := r.cmd.User userAndGroup := strings.Split(u, ":") - userStr := userAndGroup[0] + userStr, err := util.ResolveEnvironmentReplacement(userAndGroup[0], config.Env, false) + if err != nil { + return err + } var groupStr string if len(userAndGroup) > 1 { - groupStr = userAndGroup[1] + groupStr, err = util.ResolveEnvironmentReplacement(userAndGroup[1], config.Env, false) + if err != nil { + return err + } } // Lookup by username diff --git a/pkg/commands/user_test.go b/pkg/commands/user_test.go index fb5641939..c1ebe0ab2 100644 --- a/pkg/commands/user_test.go +++ b/pkg/commands/user_test.go @@ -67,11 +67,26 @@ var userTests = []struct { expectedUid: "", shouldError: true, }, + { + user: "$envuser", + expectedUid: "0", + shouldError: false, + }, + { + user: "root:$envgroup", + expectedUid: "0:0", + shouldError: false, + }, } func TestUpdateUser(t *testing.T) { for _, test := range userTests { - cfg := &manifest.Schema2Config{} + cfg := &manifest.Schema2Config{ + Env: []string{ + "envuser=root", + "envgroup=root", + }, + } cmd := UserCommand{ &instructions.UserCommand{ User: test.user,