From 2d65be548a52a21033e2a68c1d45f911de074a5e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 12 Mar 2018 15:15:54 -0700 Subject: [PATCH] Added env unit test, updated env integration test, fixed env --- .../dockerfiles/Dockerfile_test_env | 12 +++- integration_tests/dockerfiles/test_env.yaml | 24 +++++++- integration_tests/integration_test_yaml.go | 26 +++++++-- pkg/commands/env.go | 49 ++++++++++++----- pkg/commands/env_test.go | 55 +++++++++++++++++++ 5 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 pkg/commands/env_test.go diff --git a/integration_tests/dockerfiles/Dockerfile_test_env b/integration_tests/dockerfiles/Dockerfile_test_env index f88a560fe..50de509cb 100644 --- a/integration_tests/dockerfiles/Dockerfile_test_env +++ b/integration_tests/dockerfiles/Dockerfile_test_env @@ -1,5 +1,15 @@ -FROM gcr.io/distroless/base +FROM gcr.io/google-appengine/debian9 ENV hey hey ENV PATH /usr/local ENV hey hello ENV first=foo second=foo2 +ENV third $second:/third +ENV myName="John Doe" myDog=Rex\ The\ Dog \ + myCat=fluffy +ENV PATH something +ENV test=value\ value2 +ENV test1="a'b'c" +ENV test2='a"b"c' +ENV test3=a\ b name2=b\ c +ENV test4="a\"b" +ENV test5='a\"b' diff --git a/integration_tests/dockerfiles/test_env.yaml b/integration_tests/dockerfiles/test_env.yaml index 7c4847f52..435bc3ad3 100644 --- a/integration_tests/dockerfiles/test_env.yaml +++ b/integration_tests/dockerfiles/test_env.yaml @@ -4,8 +4,30 @@ metadataTest: - key: hey value: hello - key: PATH - value: /usr/local + value: something - key: first value: foo - key: second value: foo2 + - key: third + value: foo2:/third + - key: myName + value: John Doe + - key: myDog + value: Rex The Dog + - key: myCat + value: fluffy + - key: test + value: value value2 + - key: test1 + value: a'b'c + - key: test2 + value: a"b"c + - key: test3 + value: a b + - key: name2 + value: b c + - key: test4 + value: a"b + - key: test5 + value: a\"b diff --git a/integration_tests/integration_test_yaml.go b/integration_tests/integration_test_yaml.go index 001461eb4..5471e207e 100644 --- a/integration_tests/integration_test_yaml.go +++ b/integration_tests/integration_test_yaml.go @@ -55,12 +55,14 @@ var structureTests = []struct { description string dockerfilePath string structureTestYamlPath string + dockerBuildContext string repo string }{ { - description: "test env", - dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_env", - repo: "test-env", + 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", }, } @@ -156,6 +158,14 @@ func main() { } for _, test := range structureTests { + + // First, build the image with docker + dockerImageTag := testRepo + dockerPrefix + test.repo + dockerBuild := step{ + Name: dockerImage, + Args: []string{"build", "-t", dockerImageTag, "-f", test.dockerfilePath, test.dockerBuildContext}, + } + // Build the image with kbuild kbuildImage := testRepo + kbuildPrefix + test.repo kbuild := step{ @@ -167,15 +177,21 @@ func main() { Name: dockerImage, Args: []string{"pull", kbuildImage}, } - // Run structure tests on the image + // Run structure tests on the kbuild and docker image args := "container-structure-test -image " + kbuildImage + " " + test.structureTestYamlPath structureTest := step{ Name: ubuntuImage, Args: []string{"sh", "-c", args}, Env: []string{"PATH=/workspace:/bin"}, } + args = "container-structure-test -image " + dockerImageTag + " " + test.structureTestYamlPath + dockerStructureTest := step{ + Name: ubuntuImage, + Args: []string{"sh", "-c", args}, + Env: []string{"PATH=/workspace:/bin"}, + } - y.Steps = append(y.Steps, kbuild, pullKbuildImage, structureTest) + y.Steps = append(y.Steps, dockerBuild, kbuild, pullKbuildImage, structureTest, dockerStructureTest) } d, _ := yaml.Marshal(&y) diff --git a/pkg/commands/env.go b/pkg/commands/env.go index b8f1881f3..2f92339de 100644 --- a/pkg/commands/env.go +++ b/pkg/commands/env.go @@ -19,6 +19,7 @@ package commands import ( "github.com/containers/image/manifest" "github.com/docker/docker/builder/dockerfile/instructions" + "github.com/google/shlex" "github.com/sirupsen/logrus" "os" "strings" @@ -31,34 +32,52 @@ type EnvCommand struct { func (e *EnvCommand) ExecuteCommand(config *manifest.Schema2Config) error { logrus.Info("cmd: ENV") newEnvs := e.cmd.Env - for _, pair := range newEnvs { - logrus.Infof("Setting environment variable %s:%s", pair.Key, pair.Value) + for index, pair := range newEnvs { + newVal := os.Expand(pair.Value, os.Getenv) + tokens, _ := shlex.Split(newVal) + newEnvs[index] = instructions.KeyValuePair{ + Key: pair.Key, + Value: strings.Join(tokens, " "), + } + logrus.Infof("Setting environment variable %s=%s", pair.Key, pair.Value) if err := os.Setenv(pair.Key, pair.Value); err != nil { return err } } - return updateConfig(newEnvs, config) + return updateConfigEnv(newEnvs, config) } -func updateConfig(newEnvs []instructions.KeyValuePair, config *manifest.Schema2Config) error { - // First, create map of current environment variables to prevent duplicates - envMap := make(map[string]string) +func updateConfigEnv(newEnvs []instructions.KeyValuePair, config *manifest.Schema2Config) error { + // First, convert config.Env array to []instruction.KeyValuePair + var kvps []instructions.KeyValuePair for _, env := range config.Env { entry := strings.Split(env, "=") - envMap[entry[0]] = entry[1] + kvps = append(kvps, instructions.KeyValuePair{ + Key: entry[0], + Value: entry[1], + }) } - // Now, add new envs to the map - for _, pair := range newEnvs { - envMap[pair.Key] = pair.Value + // Iterate through new environment variables, and replace existing keys + // We can't use a map because we need to preserve the order of the environment variables +Loop: + for _, newEnv := range newEnvs { + for index, kvp := range kvps { + // If key exists, replace the KeyValuePair... + if kvp.Key == newEnv.Key { + logrus.Debugf("Replacing environment variable %v with %v in config", kvp, newEnv) + kvps[index] = newEnv + continue Loop + } + } + // ... Else, append it as a new env variable + kvps = append(kvps, newEnv) } - - // Now, convert back to array and modify config + // Convert back to array and set in config envArray := []string{} - for key, value := range envMap { - entry := key + "=" + value + for _, kvp := range kvps { + entry := kvp.Key + "=" + kvp.Value envArray = append(envArray, entry) } - logrus.Debugf("Setting Env in config to %v", envArray) config.Env = envArray return nil } diff --git a/pkg/commands/env_test.go b/pkg/commands/env_test.go new file mode 100644 index 000000000..2399ff199 --- /dev/null +++ b/pkg/commands/env_test.go @@ -0,0 +1,55 @@ +/* +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" +) + +func TestUpdateEnvConfig(t *testing.T) { + cfg := &manifest.Schema2Config{ + Env: []string{ + "PATH=/path/to/dir", + "hey=hey", + }, + } + + newEnvs := []instructions.KeyValuePair{ + { + Key: "foo", + Value: "foo2", + }, + { + Key: "PATH", + Value: "/new/path/", + }, + { + Key: "foo", + Value: "newfoo", + }, + } + + expectedEnvArray := []string{ + "PATH=/new/path/", + "hey=hey", + "foo=newfoo", + } + updateConfigEnv(newEnvs, cfg) + testutil.CheckErrorAndDeepEqual(t, false, nil, expectedEnvArray, cfg.Env) +}