Merge 718690fc42 into 1af4c50ed0

pkg/cluster/k8sres.go

@@ -784,7 +784,7 @@ func patchSidecarContainers(in []v1.Container, volumeMounts []v1.VolumeMount, su
 				},
 			},
 		}
-		container.Env = appendEnvVars(env, container.Env...)
+		container.Env = appendIfNotPresent(env, container.Env...)
 		result = append(result, container)
 	}
 
@@ -1023,7 +1023,7 @@ func (c *Cluster) generateSpiloPodEnvVars(
 
 	// fetch cluster-specific variables that will override all subsequent global variables
 	if len(spec.Env) > 0 {
-		envVars = appendEnvVars(envVars, spec.Env...)
+		envVars = appendIfNotPresent(envVars, spec.Env...)
 	}
 
 	if spec.Clone != nil && spec.Clone.ClusterName != "" {
@@ -1035,22 +1035,19 @@ func (c *Cluster) generateSpiloPodEnvVars(
 	}
 
 	// fetch variables from custom environment Secret
-	// that will override all subsequent global variables
 	secretEnvVarsList, err := c.getPodEnvironmentSecretVariables()
 	if err != nil {
 		return nil, err
 	}
-	envVars = appendEnvVars(envVars, secretEnvVarsList...)
 
 	// fetch variables from custom environment ConfigMap
-	// that will override all subsequent global variables
 	configMapEnvVarsList, err := c.getPodEnvironmentConfigMapVariables()
 	if err != nil {
 		return nil, err
 	}
-	envVars = appendEnvVars(envVars, configMapEnvVarsList...)
 
 	// global variables derived from operator configuration
+	// that do not override custom environment variables
 	opConfigEnvVars := make([]v1.EnvVar, 0)
 	if c.OpConfig.WALES3Bucket != "" {
 		opConfigEnvVars = append(opConfigEnvVars, v1.EnvVar{Name: "WAL_S3_BUCKET", Value: c.OpConfig.WALES3Bucket})
@@ -1080,12 +1077,20 @@ func (c *Cluster) generateSpiloPodEnvVars(
 		opConfigEnvVars = append(opConfigEnvVars, v1.EnvVar{Name: "LOG_BUCKET_SCOPE_PREFIX", Value: ""})
 	}
 
-	envVars = appendEnvVars(envVars, opConfigEnvVars...)
+	for _, env := range opConfigEnvVars {
+		if !isEnvVarPresent(secretEnvVarsList, env.Name) && !isEnvVarPresent(configMapEnvVarsList, env.Name) {
+			envVars = appendIfNotPresent(envVars, env)
+		}
+	}
+
+	// append custom environment variables last to allow chained referencing
+	envVars = appendIfNotPresent(envVars, secretEnvVarsList...)
+	envVars = appendIfNotPresent(envVars, configMapEnvVarsList...)
 
 	return envVars, nil
 }
 
-func appendEnvVars(envs []v1.EnvVar, appEnv ...v1.EnvVar) []v1.EnvVar {
+func appendIfNotPresent(envs []v1.EnvVar, appEnv ...v1.EnvVar) []v1.EnvVar {
 	collectedEnvs := envs
 	for _, env := range appEnv {
 		if !isEnvVarPresent(collectedEnvs, env.Name) {
@@ -1378,7 +1383,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 		}
 		tlsEnv, tlsVolumes := generateTlsMounts(spec, getSpiloTLSEnv)
 		for _, env := range tlsEnv {
-			spiloEnvVars = appendEnvVars(spiloEnvVars, env)
+			spiloEnvVars = appendIfNotPresent(spiloEnvVars, env)
 		}
 		additionalVolumes = append(additionalVolumes, tlsVolumes...)
 	}
@@ -2491,7 +2496,7 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar {
 
 	switch backupProvider {
 	case "s3":
-		envVars = appendEnvVars(envVars, []v1.EnvVar{
+		envVars = appendIfNotPresent(envVars, []v1.EnvVar{
 			{
 				Name:  "LOGICAL_BACKUP_S3_REGION",
 				Value: c.OpConfig.LogicalBackup.LogicalBackupS3Region,
@@ -2523,7 +2528,7 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar {
 		}
 
 	case "az":
-		envVars = appendEnvVars(envVars, []v1.EnvVar{
+		envVars = appendIfNotPresent(envVars, []v1.EnvVar{
 			{
 				Name:  "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_NAME",
 				Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageAccountName,

pkg/cluster/k8sres_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
@@ -622,7 +623,7 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 	}
 	expectedS3BucketConfigMap := []ExpectedValue{
 		{
-			envIndex:       17,
+			envIndex:       19,
 			envVarConstant: "wal_s3_bucket",
 			envVarValue:    "global-s3-bucket-configmap",
 		},
@@ -708,14 +709,14 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 			envVarValue:    fmt.Sprintf("/%s", dummyUUID),
 		},
 		{
-			envIndex:       21,
+			envIndex:       23,
 			envVarConstant: "clone_aws_endpoint",
 			envVarValue:    "s3.eu-west-1.amazonaws.com",
 		},
 	}
 	expectedCloneEnvSecret := []ExpectedValue{
 		{
-			envIndex:       21,
+			envIndex:       24,
 			envVarConstant: "clone_aws_access_key_id",
 			envVarValueRef: &v1.EnvVarSource{
 				SecretKeyRef: &v1.SecretKeySelector{
@@ -734,7 +735,7 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 			envVarValue:    "gs://some/path/",
 		},
 		{
-			envIndex:       20,
+			envIndex:       23,
 			envVarConstant: "standby_google_application_credentials",
 			envVarValueRef: &v1.EnvVarSource{
 				SecretKeyRef: &v1.SecretKeySelector{
@@ -1168,6 +1169,7 @@ func TestCloneEnv(t *testing.T) {
 		cloneOpts *acidv1.CloneDescription
 		env       v1.EnvVar
 		envPos    int
+		envLen    int
 	}{
 		{
 			subTest: "custom s3 path",
@@ -1181,6 +1183,7 @@ func TestCloneEnv(t *testing.T) {
 				Value: "s3://some/path/",
 			},
 			envPos: 1,
+			envLen: 5,
 		},
 		{
 			subTest: "generated s3 path, bucket",
@@ -1194,6 +1197,7 @@ func TestCloneEnv(t *testing.T) {
 				Value: "wale-bucket",
 			},
 			envPos: 1,
+			envLen: 6,
 		},
 		{
 			subTest: "generated s3 path, target time",
@@ -1207,6 +1211,7 @@ func TestCloneEnv(t *testing.T) {
 				Value: "somewhen",
 			},
 			envPos: 4,
+			envLen: 6,
 		},
 	}
 
@@ -1225,6 +1230,19 @@ func TestCloneEnv(t *testing.T) {
 	for _, tt := range tests {
 		envs := cluster.generateCloneEnvironment(tt.cloneOpts)
 
+		if len(envs) != tt.envLen {
+			t.Errorf("%s %s: Expected number of env variables %d, have %d instead",
+				t.Name(), tt.subTest, tt.envLen, len(envs))
+		}
+
+		// verify all env var names start with CLONE_
+		for _, env := range envs {
+			if !strings.HasPrefix(env.Name, "CLONE_") {
+				t.Errorf("%s %s: env var name %q does not start with CLONE_",
+					t.Name(), tt.subTest, env.Name)
+			}
+		}
+
 		env := envs[tt.envPos]
 
 		if env.Name != tt.env.Name {
@@ -1239,7 +1257,7 @@ func TestCloneEnv(t *testing.T) {
 	}
 }
 
-func TestAppendEnvVar(t *testing.T) {
+func TestAppendIfNotPresent(t *testing.T) {
 	tests := []struct {
 		subTest      string
 		envs         []v1.EnvVar
@@ -1291,7 +1309,7 @@ func TestAppendEnvVar(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		finalEnvs := appendEnvVars(tt.envs, tt.envsToAppend...)
+		finalEnvs := appendIfNotPresent(tt.envs, tt.envsToAppend...)
 
 		if len(finalEnvs) != tt.expectedSize {
 			t.Errorf("%s %s: expected %d env variables, got %d",
@@ -1390,6 +1408,19 @@ func TestStandbyEnv(t *testing.T) {
 	for _, tt := range tests {
 		envs := cluster.generateStandbyEnvironment(tt.standbyOpts)
 
+		if len(envs) != tt.envLen {
+			t.Errorf("%s %s: Expected number of env variables %d, have %d instead",
+				t.Name(), tt.subTest, tt.envLen, len(envs))
+		}
+
+		// verify all env var names start with STANDBY_
+		for _, env := range envs {
+			if !strings.HasPrefix(env.Name, "STANDBY_") {
+				t.Errorf("%s %s: env var name %q does not start with STANDBY_",
+					t.Name(), tt.subTest, env.Name)
+			}
+		}
+
 		env := envs[tt.envPos]
 
 		if env.Name != tt.env.Name {
@@ -1401,11 +1432,6 @@ func TestStandbyEnv(t *testing.T) {
 			t.Errorf("%s %s: Expected env value %s, have %s instead",
 				t.Name(), tt.subTest, tt.env.Value, env.Value)
 		}
-
-		if len(envs) != tt.envLen {
-			t.Errorf("%s %s: Expected number of env variables %d, have %d instead",
-				t.Name(), tt.subTest, tt.envLen, len(envs))
-		}
 	}
 }