do not mutate defaults in replaceOrAppendEnvValues

andrey-ko · estesp · commit c8d75ca5eda8 · 2019-08-16T11:06:18.000-04:00
Signed-off-by: Andrey Kolomentsev &lt;andrey.kolomentsev@docker.com&gt;

Signed-off-by: akolomentsev &lt;andrey.kolomentsev@gmail.com&gt;
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -141,38 +141,40 @@ func WithEnv(environmentVariables []string) SpecOpts {
 // replaced by env key or appended to the list
 func replaceOrAppendEnvValues(defaults, overrides []string) []string {
 	cache := make(map[string]int, len(defaults))
+	results := make([]string, 0, len(defaults))
 	for i, e := range defaults {
 		parts := strings.SplitN(e, "=", 2)
+		results = append(results, e)
 		cache[parts[0]] = i
 	}
 
 	for _, value := range overrides {
 		// Values w/o = means they want this env to be removed/unset.
 		if !strings.Contains(value, "=") {
 			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
 		}
 
 		// Just do a normal set/update
 		parts := strings.SplitN(value, "=", 2)
 		if i, exists := cache[parts[0]]; exists {
-			defaults[i] = value
+			results[i] = value
 		} else {
-			defaults = append(defaults, value)
+			results = append(results, value)
 		}
 	}
 
 	// Now remove all entries that we want to "unset"
-	for i := 0; i < len(defaults); i++ {
-		if defaults[i] == "" {
-			defaults = append(defaults[:i], defaults[i+1:]...)
+	for i := 0; i < len(results); i++ {
+		if results[i] == "" {
+			results = append(results[:i], results[i+1:]...)
 			i--
 		}
 	}
 
-	return defaults
+	return results
 }
 
 // WithProcessArgs replaces the args on the generated spec

Original file line number	Diff line number	Diff line change
`@@ -141,38 +141,40 @@ func WithEnv(environmentVariables []string) SpecOpts {`
`141`	`141`	`// replaced by env key or appended to the list`
`142`	`142`	`func replaceOrAppendEnvValues(defaults, overrides []string) []string {`
`143`	`143`	`cache := make(map[string]int, len(defaults))`
	`144`	`+ results := make([]string, 0, len(defaults))`
`144`	`145`	`for i, e := range defaults {`
`145`	`146`	`parts := strings.SplitN(e, "=", 2)`
	`147`	`+ results = append(results, e)`
`146`	`148`	`cache[parts[0]] = i`
`147`	`149`	`}`
`148`	`150`
`149`	`151`	`for _, value := range overrides {`
`150`	`152`	`// Values w/o = means they want this env to be removed/unset.`
`151`	`153`	`if !strings.Contains(value, "=") {`
`152`	`154`	`if i, exists := cache[value]; exists {`
`153`		`- defaults[i] = "" // Used to indicate it should be removed`
	`155`	`+ results[i] = "" // Used to indicate it should be removed`
`154`	`156`	`}`
`155`	`157`	`continue`
`156`	`158`	`}`
`157`	`159`
`158`	`160`	`// Just do a normal set/update`
`159`	`161`	`parts := strings.SplitN(value, "=", 2)`
`160`	`162`	`if i, exists := cache[parts[0]]; exists {`
`161`		`- defaults[i] = value`
	`163`	`+ results[i] = value`
`162`	`164`	`} else {`
`163`		`- defaults = append(defaults, value)`
	`165`	`+ results = append(results, value)`
`164`	`166`	`}`
`165`	`167`	`}`
`166`	`168`
`167`	`169`	`// Now remove all entries that we want to "unset"`
`168`		`- for i := 0; i < len(defaults); i++ {`
`169`		`- if defaults[i] == "" {`
`170`		`- defaults = append(defaults[:i], defaults[i+1:]...)`
	`170`	`+ for i := 0; i < len(results); i++ {`
	`171`	`+ if results[i] == "" {`
	`172`	`+ results = append(results[:i], results[i+1:]...)`
`171`	`173`	`i--`
`172`	`174`	`}`
`173`	`175`	`}`
`174`	`176`
`175`		`- return defaults`
	`177`	`+ return results`
`176`	`178`	`}`
`177`	`179`
`178`	`180`	`// WithProcessArgs replaces the args on the generated spec`