diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index fedd6a917..6abd170fb 100644 --- a/pkg/cluster/k8sres.go +++ b/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...) } @@ -2490,7 +2495,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, @@ -2522,7 +2527,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, diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 137c24081..ca5c52705 100644 --- a/pkg/cluster/k8sres_test.go +++ b/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)) - } } }