Avoid repeated os.SetEnv calls in container init.

libcontainer/init_linux.go

@@ -79,47 +79,77 @@ func newContainerInit(t initType, pipe *os.File, consoleSocket *os.File, fifoFd
 	if err := json.NewDecoder(pipe).Decode(&config); err != nil {
 		return nil, err
 	}
-	if err := populateProcessEnvironment(config.Env); err != nil {
+	environmentMap, err := populateEnvironmentMap(config.Env)
+	if err != nil {
 		return nil, err
 	}
+
 	switch t {
 	case initSetns:
 		return &linuxSetnsInit{
-			pipe:          pipe,
-			consoleSocket: consoleSocket,
-			config:        config,
+			pipe:           pipe,
+			consoleSocket:  consoleSocket,
+			config:         config,
+			environmentMap: environmentMap,
 		}, nil
 	case initStandard:
 		return &linuxStandardInit{
-			pipe:          pipe,
-			consoleSocket: consoleSocket,
-			parentPid:     unix.Getppid(),
-			config:        config,
-			fifoFd:        fifoFd,
+			pipe:           pipe,
+			consoleSocket:  consoleSocket,
+			parentPid:      unix.Getppid(),
+			config:         config,
+			fifoFd:         fifoFd,
+			environmentMap: environmentMap,
 		}, nil
 	}
 	return nil, fmt.Errorf("unknown init type %q", t)
 }
 
-// populateProcessEnvironment loads the provided environment variables into the
-// current processes's environment.
-func populateProcessEnvironment(env []string) error {
-	for _, pair := range env {
-		p := strings.SplitN(pair, "=", 2)
-		if len(p) < 2 {
-			return fmt.Errorf("invalid environment '%v'", pair)
+// parseEnvironmentVariable takes a environment variable with the format
+// key=value, and returns the key and value seperated.
+func parseEnvironmentVariable(env string) (key, value string, err error) {
+	p := strings.SplitN(env, "=", 2)
+	if len(p) < 2 {
+		err = fmt.Errorf("invalid environment '%+v'", env)
+		return
+	}
+	key = p[0]
+	value = p[1]
+	return
+}
+
+// createEnvironmentMap loads the provided environment variables into a
+// map.
+func populateEnvironmentMap(env []string) (map[string]string, error) {
+	environ := os.Environ()
+	mappedEnv := make(map[string]string, len(environ)+len(env))
+
+	for _, pair := range environ {
+		key, _, err := parseEnvironmentVariable(pair)
+		if err != nil {
+			return nil, err
 		}
-		if err := os.Setenv(p[0], p[1]); err != nil {
-			return err
+		mappedEnv[key] = pair
+	}
+	for _, pair := range env {
+		key, _, err := parseEnvironmentVariable(pair)
+		if err != nil {
+			return nil, err
 		}
+		mappedEnv[key] = pair
 	}
-	return nil
+
+	// Certain functions that are used later, such as exec.LookPath, require that the PATH
+	// environment variable is setup with the container's PATH.
+	os.Setenv("PATH", mappedEnv["PATH"])
+
+	return mappedEnv, nil
 }
 
 // finalizeNamespace drops the caps, sets the correct user
 // and working dir, and closes any leaked file descriptors
 // before executing the command inside the namespace
-func finalizeNamespace(config *initConfig) error {
+func finalizeNamespace(config *initConfig, environmentMap map[string]string) error {
 	// Ensure that all unwanted fds we may have accidentally
 	// inherited are marked close-on-exec so they stay out of the
 	// container
@@ -145,7 +175,7 @@ func finalizeNamespace(config *initConfig) error {
 	if err := system.SetKeepCaps(); err != nil {
 		return errors.Wrap(err, "set keep caps")
 	}
-	if err := setupUser(config); err != nil {
+	if err := setupUser(config, environmentMap); err != nil {
 		return errors.Wrap(err, "setup user")
 	}
 	if err := system.ClearKeepCaps(); err != nil {
@@ -237,7 +267,7 @@ func syncParentHooks(pipe io.ReadWriter) error {
 }
 
 // setupUser changes the groups, gid, and uid for the user inside the container
-func setupUser(config *initConfig) error {
+func setupUser(config *initConfig, environmentMap map[string]string) error {
 	// Set up defaults.
 	defaultExecUser := user.ExecUser{
 		Uid:  0,
@@ -319,10 +349,8 @@ func setupUser(config *initConfig) error {
 	}
 
 	// if we didn't get HOME already, set it based on the user's HOME
-	if envHome := os.Getenv("HOME"); envHome == "" {
-		if err := os.Setenv("HOME", execUser.Home); err != nil {
-			return err
-		}
+	if _, ok := environmentMap["HOME"]; !ok {
+		environmentMap["HOME"] = "HOME" + "=" + execUser.Home
 	}
 	return nil
 }

libcontainer/setns_init_linux.go

@@ -11,6 +11,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/keys"
 	"github.com/opencontainers/runc/libcontainer/seccomp"
 	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
 
@@ -20,9 +21,10 @@ import (
 // linuxSetnsInit performs the container's initialization for running a new process
 // inside an existing container.
 type linuxSetnsInit struct {
-	pipe          *os.File
-	consoleSocket *os.File
-	config        *initConfig
+	pipe           *os.File
+	consoleSocket  *os.File
+	config         *initConfig
+	environmentMap map[string]string
 }
 
 func (l *linuxSetnsInit) getSessionRingName() string {
@@ -70,7 +72,7 @@ func (l *linuxSetnsInit) Init() error {
 			return err
 		}
 	}
-	if err := finalizeNamespace(l.config); err != nil {
+	if err := finalizeNamespace(l.config, l.environmentMap); err != nil {
 		return err
 	}
 	if err := apparmor.ApplyProfile(l.config.AppArmorProfile); err != nil {
@@ -84,5 +86,5 @@ func (l *linuxSetnsInit) Init() error {
 			return newSystemErrorWithCause(err, "init seccomp")
 		}
 	}
-	return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ())
+	return system.Execv(l.config.Args[0], l.config.Args[0:], utils.StringMapToSlice(l.environmentMap))
 }

libcontainer/standard_init_linux.go

@@ -14,18 +14,20 @@ import (
 	"github.com/opencontainers/runc/libcontainer/keys"
 	"github.com/opencontainers/runc/libcontainer/seccomp"
 	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
 
 	"golang.org/x/sys/unix"
 )
 
 type linuxStandardInit struct {
-	pipe          *os.File
-	consoleSocket *os.File
-	parentPid     int
-	fifoFd        int
-	config        *initConfig
+	pipe           *os.File
+	consoleSocket  *os.File
+	parentPid      int
+	fifoFd         int
+	config         *initConfig
+	environmentMap map[string]string
 }
 
 func (l *linuxStandardInit) getSessionRingParams() (string, uint32, uint32) {
@@ -154,7 +156,7 @@ func (l *linuxStandardInit) Init() error {
 			return err
 		}
 	}
-	if err := finalizeNamespace(l.config); err != nil {
+	if err := finalizeNamespace(l.config, l.environmentMap); err != nil {
 		return err
 	}
 	// finalizeNamespace can change user/group which clears the parent death
@@ -203,7 +205,7 @@ func (l *linuxStandardInit) Init() error {
 			return newSystemErrorWithCause(err, "init seccomp")
 		}
 	}
-	if err := syscall.Exec(name, l.config.Args[0:], os.Environ()); err != nil {
+	if err := syscall.Exec(name, l.config.Args[0:], utils.StringMapToSlice(l.environmentMap)); err != nil {
 		return newSystemErrorWithCause(err, "exec user process")
 	}
 	return nil

libcontainer/utils/utils.go

@@ -110,3 +110,15 @@ func Annotations(labels []string) (bundle string, userAnnotations map[string]str
 func GetIntSize() int {
 	return int(unsafe.Sizeof(1))
 }
+
+// StringMapToSlice takes a string map, and converts its values into
+// a slice with an undefined ordering.
+func StringMapToSlice(m map[string]string) []string {
+	slice := make([]string, len(m))
+	i := 0
+	for _, value := range m {
+		slice[i] = value
+		i++
+	}
+	return slice
+}

libcontainer/utils/utils_test.go

@@ -140,3 +140,36 @@ func TestCleanPath(t *testing.T) {
 		t.Errorf("expected to receive '/foo' and received %s", path)
 	}
 }
+
+func contains(slice []string, item string) bool {
+	for _, v := range slice {
+		if v == item {
+			return true
+		}
+	}
+	return false
+}
+
+func TestStringMapToSlice(t *testing.T) {
+	slice := StringMapToSlice(nil)
+	if len(slice) != 0 || slice == nil {
+		t.Errorf("expected 0 length slice, instead got %+v", slice)
+	}
+
+	m := make(map[string]string)
+	slice = StringMapToSlice(m)
+	if len(slice) != 0 || slice == nil {
+		t.Errorf("expected 0 length slice, instead got %+v", slice)
+	}
+
+	m = make(map[string]string)
+	m["key1"] = "value1"
+	m["key2"] = "value2"
+	slice = StringMapToSlice(m)
+	if len(slice) != 2 {
+		t.Errorf("expected slice of length 2, instead got slice of length %d", len(slice))
+	}
+	if !contains(slice, "value1") || !contains(slice, "value2") {
+		t.Errorf("expected slice to contain both value1 and value2, instead got %+v", slice)
+	}
+}