Merge pull request #4412 from crosbymichael/fix-env-clobber

Don't always just append env vars, replace defaults with ones from confi...

container.go

@@ -507,10 +507,10 @@ func (container *Container) Start() (err error) {
 		}
 	}
 
-	for _, elem := range container.Config.Env {
-		env = append(env, elem)
-	}
-
+	// because the env on the container can override certain default values
+	// we need to replace the 'env' keys where they match and append anything
+	// else.
+	env = utils.ReplaceOrAppendEnvValues(env, container.Config.Env)
 	if err := container.generateEnvConfig(env); err != nil {
 		return err
 	}

utils/utils.go

@@ -978,3 +978,22 @@ func NewReadCloserWrapper(r io.Reader, closer func() error) io.ReadCloser {
 		closer: closer,
 	}
 }
+
+// ReplaceOrAppendValues returns the defaults with the overrides either
+// replaced by env key or appended to the list
+func ReplaceOrAppendEnvValues(defaults, overrides []string) []string {
+	cache := make(map[string]int, len(defaults))
+	for i, e := range defaults {
+		parts := strings.SplitN(e, "=", 2)
+		cache[parts[0]] = i
+	}
+	for _, value := range overrides {
+		parts := strings.SplitN(value, "=", 2)
+		if i, exists := cache[parts[0]]; exists {
+			defaults[i] = value
+		} else {
+			defaults = append(defaults, value)
+		}
+	}
+	return defaults
+}

utils/utils_test.go

@@ -480,3 +480,21 @@ func StrSlicesEqual(a, b []string) bool {
 
 	return true
 }
+
+func TestReplaceAndAppendEnvVars(t *testing.T) {
+	var (
+		d = []string{"HOME=/"}
+		o = []string{"HOME=/root", "TERM=xterm"}
+	)
+
+	env := ReplaceOrAppendEnvValues(d, o)
+	if len(env) != 2 {
+		t.Fatalf("expected len of 2 got %d", len(env))
+	}
+	if env[0] != "HOME=/root" {
+		t.Fatalf("expected HOME=/root got '%s'", env[0])
+	}
+	if env[1] != "TERM=xterm" {
+		t.Fatalf("expected TERM=xterm got '%s'", env[1])
+	}
+}