Merge pull request #47 from priyawadhwa/env-replacement

Support environment replacement in expose/copy/env

integration_tests/dockerfiles/Dockerfile_test_copy

@@ -12,3 +12,9 @@ COPY ["context/foo", "/tmp/foo" ]
 COPY context/b* /baz/
 COPY context/foo context/bar/ba? /test/
 COPY context/arr[[]0].txt /mydir/
+COPY context/bar/bat .
+
+ENV contextenv ./context
+COPY ${contextenv}/foo /tmp/foo2
+COPY $contextenv/foo /tmp/foo3
+COPY $contextenv/* /tmp/${contextenv}/

pkg/commands/copy.go

@@ -39,8 +39,14 @@ func (c *CopyCommand) ExecuteCommand(config *manifest.Schema2Config) error {
 	logrus.Infof("cmd: copy %s", srcs)
 	logrus.Infof("dest: %s", dest)
 
+	// First, resolve any environment replacement
+	resolvedEnvs, err := util.ResolveEnvironmentReplacementList(c.cmd.SourcesAndDest, config.Env, true)
+	if err != nil {
+		return err
+	}
+	dest = resolvedEnvs[len(resolvedEnvs)-1]
 	// Get a map of [src]:[files rooted at src]
-	srcMap, err := util.ResolveSources(c.cmd.SourcesAndDest, c.buildcontext)
+	srcMap, err := util.ResolveSources(resolvedEnvs, c.buildcontext)
 	if err != nil {
 		return err
 	}

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,18 @@ 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(pair.Key, config.Env, false)
+		if err != nil {
+			return err
+		}
+		expandedValue, err := util.ResolveEnvironmentReplacement(pair.Value, config.Env, false)
 		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)
@@ -98,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{}

pkg/commands/env_test.go

@@ -53,21 +53,39 @@ func TestUpdateEnvConfig(t *testing.T) {
 	updateConfigEnv(newEnvs, cfg)
 	testutil.CheckErrorAndDeepEqual(t, false, nil, expectedEnvArray, cfg.Env)
 }
+func Test_EnvExecute(t *testing.T) {
+	cfg := &manifest.Schema2Config{
+		Env: []string{
+			"path=/usr/",
+			"home=/root",
+		},
+	}
 
-func TestEnvToString(t *testing.T) {
-	envCmd := &instructions.EnvCommand{
-		Env: []instructions.KeyValuePair{
-			{
-				Key:   "PATH",
-				Value: "/some/path",
-			},
-			{
-				Key:   "HOME",
-				Value: "/root",
+	envCmd := &EnvCommand{
+		&instructions.EnvCommand{
+			Env: []instructions.KeyValuePair{
+				{
+					Key:   "path",
+					Value: "/some/path",
+				},
+				{
+					Key:   "HOME",
+					Value: "$home",
+				},
+				{
+					Key:   "$path",
+					Value: "$home/",
+				},
 			},
 		},
 	}
-	expectedString := "ENV PATH=/some/path HOME=/root"
-	actualString := envToString(envCmd)
-	testutil.CheckErrorAndDeepEqual(t, false, nil, expectedString, actualString)
+
+	expectedEnvs := []string{
+		"path=/some/path",
+		"home=/root",
+		"HOME=/root",
+		"/usr/=/root/",
+	}
+	err := envCmd.ExecuteCommand(cfg)
+	testutil.CheckErrorAndDeepEqual(t, false, err, expectedEnvs, cfg.Env)
 }

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,15 @@ 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
-
 	// Add any new ones in
-	for _, p := range ports {
+	for _, p := range r.cmd.Ports {
+		// Resolve any environment variables
+		p, err := util.ResolveEnvironmentReplacement(p, config.Env, false)
+		if err != nil {
+			return err
+		}
 		// Add the default protocol if one isn't specified
 		if !strings.Contains(p, "/") {
 			p = p + "/tcp"
@@ -64,6 +55,16 @@ 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) FilesToSnapshot() []string {
 	return []string{}
 }

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,23 +28,39 @@ func TestUpdateExposedPorts(t *testing.T) {
 		ExposedPorts: manifest.Schema2PortSet{
 			"8080/tcp": {},
 		},
+		Env: []string{
+			"port=udp",
+			"num=8085",
+		},
 	}
 
 	ports := []string{
 		"8080",
 		"8081/tcp",
 		"8082",
 		"8083/udp",
+		"8084/$port",
+		"$num",
+		"$num/$port",
+	}
+
+	exposeCmd := &ExposeCommand{
+		&instructions.ExposeCommand{
+			Ports: ports,
+		},
 	}
 
 	expectedPorts := manifest.Schema2PortSet{
 		"8080/tcp": {},
 		"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)
 }

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
 		}

pkg/util/command_util.go

@@ -18,13 +18,54 @@ package util
 
 import (
 	"github.com/docker/docker/builder/dockerfile/instructions"
+	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/builder/dockerfile/shell"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"os"
 	"path/filepath"
 	"strings"
 )
 
+// ResolveEnvironmentReplacement resolves a list of values by calling resolveEnvironmentReplacement
+func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ([]string, error) {
+	var resolvedValues []string
+	for _, value := range values {
+		resolved, err := ResolveEnvironmentReplacement(value, envs, isFilepath)
+		logrus.Debugf("Resolved %s to %s", value, resolved)
+		if err != nil {
+			return nil, err
+		}
+		resolvedValues = append(resolvedValues, resolved)
+	}
+	return resolvedValues, nil
+}
+
+// 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(value string, envs []string, isFilepath bool) (string, error) {
+	shlex := shell.NewLex(parser.DefaultEscapeToken)
+	fp, err := shlex.ProcessWord(value, envs)
+	if !isFilepath {
+		return fp, err
+	}
+	if err != nil {
+		return "", err
+	}
+	fp = filepath.Clean(fp)
+	if IsDestDir(value) {
+		fp = fp + "/"
+	}
+	return fp, nil
+}
+
 // ContainsWildcards returns true if any entry in paths contains wildcards
 func ContainsWildcards(paths []string) bool {
 	for _, path := range paths {