Merge pull request #2098 from joelanford/fix/subscription-env-merge

fix environment variable merging from subscription.config

Author: openshift-merge-robot

pkg/controller/operators/olm/overrides/inject/inject.go

@@ -25,37 +25,40 @@ func InjectEnvIntoDeployment(podSpec *corev1.PodSpec, envVars []corev1.EnvVar) e
 	return nil
 }
 
-func mergeEnvVars(containerEnvVars []corev1.EnvVar, newEnvVars []corev1.EnvVar) (merged []corev1.EnvVar) {
-	merged = containerEnvVars
-
-	for _, newEnvVar := range newEnvVars {
-		existing, found := findEnvVar(containerEnvVars, newEnvVar.Name)
-		if found {
-			existing.Value = newEnvVar.Value
+func mergeEnvVars(containerEnvVars []corev1.EnvVar, newEnvVars []corev1.EnvVar) []corev1.EnvVar {
+	// Build a map of environment variables.
+	// newEnvVars always overrides containerEnvVars.
+	mergedMap := map[string]corev1.EnvVar{}
+	for _, envVar := range containerEnvVars {
+		mergedMap[envVar.Name] = envVar
+	}
+	for _, envVar := range newEnvVars {
+		mergedMap[envVar.Name] = envVar
+	}
+
+	// To keep things in the expected order, always put the
+	// original environment variable names into the merged
+	// output in place first.
+	merged := make([]corev1.EnvVar, 0, len(mergedMap))
+	for _, e := range containerEnvVars {
+		envVar := mergedMap[e.Name]
+		merged = append(merged, envVar)
+		delete(mergedMap, e.Name)
+	}
+
+	// Then for any remaining newEnvVars (i.e. env vars
+	// that weren't present in the containerEnvVars), add
+	// them at the end in the order they were provided in
+	// the subscription.
+	for _, e := range newEnvVars {
+		envVar, ok := mergedMap[e.Name]
+		if !ok {
 			continue
 		}
-
-		merged = append(merged, corev1.EnvVar{
-			Name:  newEnvVar.Name,
-			Value: newEnvVar.Value,
-		})
-	}
-
-	return
-}
-
-func findEnvVar(proxyEnvVar []corev1.EnvVar, name string) (foundEnvVar *corev1.EnvVar, found bool) {
-	for i := range proxyEnvVar {
-		if name == proxyEnvVar[i].Name {
-			// Environment variable names are case sensitive.
-			found = true
-			foundEnvVar = &proxyEnvVar[i]
-
-			break
-		}
+		merged = append(merged, envVar)
 	}
 
-	return
+	return merged
 }
 
 // InjectVolumesIntoDeployment injects the provided Volumes

pkg/controller/operators/olm/overrides/inject/inject_test.go

@@ -3,10 +3,11 @@ package inject_test
 import (
 	"testing"
 
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/overrides/inject"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/overrides/inject"
 )
 
 var (
@@ -386,6 +387,10 @@ func TestInjectEnvIntoDeployment(t *testing.T) {
 				},
 			},
 			envVar: []corev1.EnvVar{
+				corev1.EnvVar{
+					Name:  "extra",
+					Value: "extra_value",
+				},
 				corev1.EnvVar{
 					Name:  "foo",
 					Value: "new_foo_value",
@@ -407,6 +412,10 @@ func TestInjectEnvIntoDeployment(t *testing.T) {
 								Name:  "bar",
 								Value: "new_bar_value",
 							},
+							corev1.EnvVar{
+								Name:  "extra",
+								Value: "extra_value",
+							},
 						},
 					},
 				},