do not mutate defaults in replaceOrAppendEnvValues
Signed-off-by: Andrey Kolomentsev <andrey.kolomentsev@docker.com>
This commit is contained in:
		| @@ -141,8 +141,10 @@ func WithEnv(environmentVariables []string) SpecOpts { | |||||||
| // replaced by env key or appended to the list | // replaced by env key or appended to the list | ||||||
| func replaceOrAppendEnvValues(defaults, overrides []string) []string { | func replaceOrAppendEnvValues(defaults, overrides []string) []string { | ||||||
| 	cache := make(map[string]int, len(defaults)) | 	cache := make(map[string]int, len(defaults)) | ||||||
|  | 	results := make([]string, 0, len(defaults)) | ||||||
| 	for i, e := range defaults { | 	for i, e := range defaults { | ||||||
| 		parts := strings.SplitN(e, "=", 2) | 		parts := strings.SplitN(e, "=", 2) | ||||||
|  | 		results = append(results, e) | ||||||
| 		cache[parts[0]] = i | 		cache[parts[0]] = i | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| @@ -150,7 +152,7 @@ func replaceOrAppendEnvValues(defaults, overrides []string) []string { | |||||||
| 		// Values w/o = means they want this env to be removed/unset. | 		// Values w/o = means they want this env to be removed/unset. | ||||||
| 		if !strings.Contains(value, "=") { | 		if !strings.Contains(value, "=") { | ||||||
| 			if i, exists := cache[value]; exists { | 			if i, exists := cache[value]; exists { | ||||||
| 				defaults[i] = "" // Used to indicate it should be removed | 				results[i] = "" // Used to indicate it should be removed | ||||||
| 			} | 			} | ||||||
| 			continue | 			continue | ||||||
| 		} | 		} | ||||||
| @@ -158,21 +160,21 @@ func replaceOrAppendEnvValues(defaults, overrides []string) []string { | |||||||
| 		// Just do a normal set/update | 		// Just do a normal set/update | ||||||
| 		parts := strings.SplitN(value, "=", 2) | 		parts := strings.SplitN(value, "=", 2) | ||||||
| 		if i, exists := cache[parts[0]]; exists { | 		if i, exists := cache[parts[0]]; exists { | ||||||
| 			defaults[i] = value | 			results[i] = value | ||||||
| 		} else { | 		} else { | ||||||
| 			defaults = append(defaults, value) | 			results = append(results, value) | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	// Now remove all entries that we want to "unset" | 	// Now remove all entries that we want to "unset" | ||||||
| 	for i := 0; i < len(defaults); i++ { | 	for i := 0; i < len(results); i++ { | ||||||
| 		if defaults[i] == "" { | 		if results[i] == "" { | ||||||
| 			defaults = append(defaults[:i], defaults[i+1:]...) | 			results = append(results[:i], results[i+1:]...) | ||||||
| 			i-- | 			i-- | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	return defaults | 	return results | ||||||
| } | } | ||||||
|  |  | ||||||
| // WithProcessArgs replaces the args on the generated spec | // WithProcessArgs replaces the args on the generated spec | ||||||
|   | |||||||
| @@ -31,6 +31,49 @@ import ( | |||||||
| 	specs "github.com/opencontainers/runtime-spec/specs-go" | 	specs "github.com/opencontainers/runtime-spec/specs-go" | ||||||
| ) | ) | ||||||
|  |  | ||||||
|  | func TestReplaceOrAppendEnvValues(t *testing.T) { | ||||||
|  | 	t.Parallel() | ||||||
|  |  | ||||||
|  | 	defaults := []string{ | ||||||
|  | 		"o=ups", "p=$e", "x=foo", "y=boo", "z", "t=", | ||||||
|  | 	} | ||||||
|  | 	overrides := []string{ | ||||||
|  | 		"x=bar", "y", "a=42", "o=", "e", "s=", | ||||||
|  | 	} | ||||||
|  | 	expected := []string{ | ||||||
|  | 		"o=", "p=$e", "x=bar", "z", "t=", "a=42", "s=", | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	defaultsOrig := make([]string, len(defaults)) | ||||||
|  | 	copy(defaultsOrig, defaults) | ||||||
|  | 	overridesOrig := make([]string, len(overrides)) | ||||||
|  | 	copy(overridesOrig, overrides) | ||||||
|  |  | ||||||
|  | 	results := replaceOrAppendEnvValues(defaults, overrides) | ||||||
|  |  | ||||||
|  | 	for i, x := range defaultsOrig { | ||||||
|  | 		if defaults[i] != x { | ||||||
|  | 			t.Fatal("defaults slice was mutated") | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	for i, x := range overridesOrig { | ||||||
|  | 		if overrides[i] != x { | ||||||
|  | 			t.Fatal("overrides slice was mutated") | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	if len(expected) != len(results) { | ||||||
|  | 		t.Fatalf("expected %s, but found %s", expected, results) | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	for i, x := range expected { | ||||||
|  | 		if results[i] != x { | ||||||
|  | 			t.Fatalf("expected %s, but found %s", expected, results) | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  |  | ||||||
| func TestWithEnv(t *testing.T) { | func TestWithEnv(t *testing.T) { | ||||||
| 	t.Parallel() | 	t.Parallel() | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 akolomentsev
					akolomentsev