Ability to set pod environment variables on cluster resource (#1794)

* Ability to set pod environment variables on cluster resource

Co-authored-by: Felix Kunde <felix-kunde@gmx.de>

charts/postgres-operator/crds/postgresqls.yaml

@@ -196,6 +196,12 @@ spec:
                 type: boolean
               enableShmVolume:
                 type: boolean
+              env:
+                type: array
+                nullable: true
+                items:
+                  type: object
+                  x-kubernetes-preserve-unknown-fields: true
               init_containers:
                 type: array
                 description: deprecated

docs/administrator.md

@@ -706,6 +706,29 @@ data:
 The key-value pairs of the Secret are all accessible as environment variables
 to the Postgres StatefulSet/pods.
 
+### For individual cluster
+
+It is possible to define environment variables directly in the Postgres cluster
+manifest to configure it individually. The variables must be listed under the
+`env` section in the same way you would do for [containers](https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/).
+Global parameters served from a custom config map or secret will be overridden.
+
+```yaml
+apiVersion: "acid.zalan.do/v1"
+kind: postgresql
+metadata:
+  name: acid-test-cluster
+spec:
+  env:
+  - name: wal_s3_bucket
+    value: my-custom-bucket
+  - name: minio_secret_key
+      valueFrom:
+        secretKeyRef:
+          name: my-custom-secret
+          key: minio_secret_key
+```
+
 ## Limiting the number of min and max instances in clusters
 
 As a preventive measure, one can restrict the minimum and the maximum number of

manifests/complete-postgres-manifest.yaml

@@ -49,6 +49,10 @@ spec:
       shared_buffers: "32MB"
       max_connections: "10"
       log_statement: "all"
+#  env:
+#  - name: wal_s3_bucket
+#    value: my-custom-bucket
+
   volume:
     size: 1Gi
 #    storageClass: my-sc
@@ -120,7 +124,7 @@ spec:
 #        database: foo
 #        plugin: pgoutput
     ttl: 30
-    loop_wait: &loop_wait 10
+    loop_wait: 10
     retry_timeout: 10
     synchronous_mode: false
     synchronous_mode_strict: false

manifests/postgresql.crd.yaml

@@ -194,6 +194,12 @@ spec:
                 type: boolean
               enableShmVolume:
                 type: boolean
+              env:
+                type: array
+                nullable: true
+                items:
+                  type: object
+                  x-kubernetes-preserve-unknown-fields: true
               init_containers:
                 type: array
                 description: deprecated

pkg/apis/acid.zalan.do/v1/crds.go

@@ -311,6 +311,16 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 					"enableShmVolume": {
 						Type: "boolean",
 					},
+					"env": {
+						Type:     "array",
+						Nullable: true,
+						Items: &apiextv1.JSONSchemaPropsOrArray{
+							Schema: &apiextv1.JSONSchemaProps{
+								Type:                   "object",
+								XPreserveUnknownFields: util.True(),
+							},
+						},
+					},
 					"init_containers": {
 						Type:        "array",
 						Description: "deprecated",

pkg/apis/acid.zalan.do/v1/postgresql_type.go

@@ -80,6 +80,7 @@ type PostgresSpec struct {
 	TLS                   *TLSDescription             `json:"tls,omitempty"`
 	AdditionalVolumes     []AdditionalVolume          `json:"additionalVolumes,omitempty"`
 	Streams               []Stream                    `json:"streams,omitempty"`
+	Env                   []v1.EnvVar                 `json:"env,omitempty"`
 
 	// deprecated json tags
 	InitContainersOld       []v1.Container `json:"init_containers,omitempty"`

pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go

@@ -779,6 +779,13 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
+	if in.Env != nil {
+		in, out := &in.Env, &out.Env
+		*out = make([]corev1.EnvVar, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 	if in.InitContainersOld != nil {
 		in, out := &in.InitContainersOld, &out.InitContainersOld
 		*out = make([]corev1.Container, len(*in))

pkg/cluster/k8sres.go

@@ -649,8 +649,7 @@ func patchSidecarContainers(in []v1.Container, volumeMounts []v1.VolumeMount, su
 				},
 			},
 		}
-		mergedEnv := append(env, container.Env...)
+		container.Env = appendEnvVars(env, container.Env...)
-		container.Env = deduplicateEnvVars(mergedEnv, container.Name, logger)
 		result = append(result, container)
 	}
 
@@ -769,6 +768,7 @@ func (c *Cluster) generateSpiloPodEnvVars(
 	cloneDescription *acidv1.CloneDescription,
 	standbyDescription *acidv1.StandbyDescription,
 	customPodEnvVarsList []v1.EnvVar) []v1.EnvVar {
+
 	envVars := []v1.EnvVar{
 		{
 			Name:  "SCOPE",
@@ -843,6 +843,11 @@ func (c *Cluster) generateSpiloPodEnvVars(
 			Value: c.OpConfig.PamRoleName,
 		},
 	}
+
+	if c.OpConfig.EnableSpiloWalPathCompat {
+		envVars = append(envVars, v1.EnvVar{Name: "ENABLE_WAL_PATH_COMPAT", Value: "true"})
+	}
+
 	if c.OpConfig.EnablePgVersionEnvVar {
 		envVars = append(envVars, v1.EnvVar{Name: "PGVERSION", Value: c.GetDesiredMajorVersion()})
 	}
@@ -874,73 +879,67 @@ func (c *Cluster) generateSpiloPodEnvVars(
 		envVars = append(envVars, c.generateStandbyEnvironment(standbyDescription)...)
 	}
 
+	if len(c.Spec.Env) > 0 {
+		envVars = appendEnvVars(envVars, c.Spec.Env...)
+	}
+
 	// add vars taken from pod_environment_configmap and pod_environment_secret first
 	// (to allow them to override the globals set in the operator config)
 	if len(customPodEnvVarsList) > 0 {
-		envVars = append(envVars, customPodEnvVarsList...)
+		envVars = appendEnvVars(envVars, customPodEnvVarsList...)
 	}
 
 	if c.OpConfig.WALES3Bucket != "" {
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_S3_BUCKET", Value: c.OpConfig.WALES3Bucket})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_S3_BUCKET", Value: c.OpConfig.WALES3Bucket})
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_PREFIX", Value: ""})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_PREFIX", Value: ""})
 	}
 
 	if c.OpConfig.WALGSBucket != "" {
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_GS_BUCKET", Value: c.OpConfig.WALGSBucket})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_GS_BUCKET", Value: c.OpConfig.WALGSBucket})
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_PREFIX", Value: ""})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_PREFIX", Value: ""})
 	}
 
 	if c.OpConfig.WALAZStorageAccount != "" {
-		envVars = append(envVars, v1.EnvVar{Name: "AZURE_STORAGE_ACCOUNT", Value: c.OpConfig.WALAZStorageAccount})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "AZURE_STORAGE_ACCOUNT", Value: c.OpConfig.WALAZStorageAccount})
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
-		envVars = append(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_PREFIX", Value: ""})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "WAL_BUCKET_SCOPE_PREFIX", Value: ""})
 	}
 
 	if c.OpConfig.GCPCredentials != "" {
-		envVars = append(envVars, v1.EnvVar{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: c.OpConfig.GCPCredentials})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: c.OpConfig.GCPCredentials})
 	}
 
 	if c.OpConfig.LogS3Bucket != "" {
-		envVars = append(envVars, v1.EnvVar{Name: "LOG_S3_BUCKET", Value: c.OpConfig.LogS3Bucket})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "LOG_S3_BUCKET", Value: c.OpConfig.LogS3Bucket})
-		envVars = append(envVars, v1.EnvVar{Name: "LOG_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "LOG_BUCKET_SCOPE_SUFFIX", Value: getBucketScopeSuffix(string(uid))})
-		envVars = append(envVars, v1.EnvVar{Name: "LOG_BUCKET_SCOPE_PREFIX", Value: ""})
+		envVars = appendEnvVars(envVars, v1.EnvVar{Name: "LOG_BUCKET_SCOPE_PREFIX", Value: ""})
 	}
 
 	return envVars
 }
 
-// deduplicateEnvVars makes sure there are no duplicate in the target envVar array. While Kubernetes already
+func appendEnvVars(envs []v1.EnvVar, appEnv ...v1.EnvVar) []v1.EnvVar {
-// deduplicates variables defined in a container, it leaves the last definition in the list and this behavior is not
+	jenvs := envs
-// well-documented, which means that the behavior can be reversed at some point (it may also start producing an error).
+	for _, env := range appEnv {
-// Therefore, the merge is done by the operator, the entries that are ahead in the passed list take priority over those
+		if !isEnvVarPresent(jenvs, env.Name) {
-// that are behind, and only the name is considered in order to eliminate duplicates.
+			jenvs = append(jenvs, env)
-func deduplicateEnvVars(input []v1.EnvVar, containerName string, logger *logrus.Entry) []v1.EnvVar {
-	result := make([]v1.EnvVar, 0)
-	names := make(map[string]int)
-
-	for i, va := range input {
-		if names[va.Name] == 0 {
-			names[va.Name]++
-			result = append(result, input[i])
-		} else if names[va.Name] == 1 {
-			names[va.Name]++
-
-			// Some variables (those to configure the WAL_ and LOG_ shipping) may be overwritten, only log as info
-			if strings.HasPrefix(va.Name, "WAL_") || strings.HasPrefix(va.Name, "LOG_") {
-				logger.Infof("global variable %q has been overwritten by configmap/secret for container %q",
-					va.Name, containerName)
-			} else {
-				logger.Warningf("variable %q is defined in %q more than once, the subsequent definitions are ignored",
-					va.Name, containerName)
 		}
 	}
-	}
+	return jenvs
-	return result
 }
 
-// Return list of variables the pod recieved from the configured ConfigMap
+func isEnvVarPresent(envs []v1.EnvVar, key string) bool {
+	for _, env := range envs {
+		if env.Name == key {
+			return true
+		}
+	}
+	return false
+}
+
+// Return list of variables the pod received from the configured ConfigMap
 func (c *Cluster) getPodEnvironmentConfigMapVariables() ([]v1.EnvVar, error) {
 	configMapPodEnvVarsList := make([]v1.EnvVar, 0)
 
@@ -1105,16 +1104,6 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 		initContainers = spec.InitContainers
 	}
 
-	spiloCompathWalPathList := make([]v1.EnvVar, 0)
-	if c.OpConfig.EnableSpiloWalPathCompat {
-		spiloCompathWalPathList = append(spiloCompathWalPathList,
-			v1.EnvVar{
-				Name:  "ENABLE_WAL_PATH_COMPAT",
-				Value: "true",
-			},
-		)
-	}
-
 	// fetch env vars from custom ConfigMap
 	configMapEnvVarsList, err := c.getPodEnvironmentConfigMapVariables()
 	if err != nil {
@@ -1128,8 +1117,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 	}
 
 	// concat all custom pod env vars and sort them
-	customPodEnvVarsList := append(spiloCompathWalPathList, configMapEnvVarsList...)
+	customPodEnvVarsList := append(configMapEnvVarsList, secretEnvVarsList...)
-	customPodEnvVarsList = append(customPodEnvVarsList, secretEnvVarsList...)
 	sort.Slice(customPodEnvVarsList,
 		func(i, j int) bool { return customPodEnvVarsList[i].Name < customPodEnvVarsList[j].Name })
 
@@ -1210,7 +1198,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 		// use the same filenames as Secret resources by default
 		certFile := ensurePath(spec.TLS.CertificateFile, mountPath, "tls.crt")
 		privateKeyFile := ensurePath(spec.TLS.PrivateKeyFile, mountPath, "tls.key")
-		spiloEnvVars = append(
+		spiloEnvVars = appendEnvVars(
 			spiloEnvVars,
 			v1.EnvVar{Name: "SSL_CERTIFICATE_FILE", Value: certFile},
 			v1.EnvVar{Name: "SSL_PRIVATE_KEY_FILE", Value: privateKeyFile},
@@ -1224,7 +1212,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 			}
 
 			caFile := ensurePath(spec.TLS.CAFile, mountPathCA, "")
-			spiloEnvVars = append(
+			spiloEnvVars = appendEnvVars(
 				spiloEnvVars,
 				v1.EnvVar{Name: "SSL_CA_FILE", Value: caFile},
 			)
@@ -1249,7 +1237,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 	spiloContainer := generateContainer(constants.PostgresContainerName,
 		&effectiveDockerImage,
 		resourceRequirements,
-		deduplicateEnvVars(spiloEnvVars, constants.PostgresContainerName, c.logger),
+		spiloEnvVars,
 		volumeMounts,
 		c.OpConfig.Resources.SpiloPrivileged,
 		c.OpConfig.Resources.SpiloAllowPrivilegeEscalation,

pkg/cluster/k8sres_test.go

@@ -131,17 +131,17 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 		}, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder)
 
 	expectedValuesGSBucket := []ExpectedValue{
-		ExpectedValue{
+		{
 			envIndex:       15,
 			envVarConstant: "WAL_GS_BUCKET",
 			envVarValue:    "wale-gs-bucket",
 		},
-		ExpectedValue{
+		{
 			envIndex:       16,
 			envVarConstant: "WAL_BUCKET_SCOPE_SUFFIX",
 			envVarValue:    "/SomeUUID",
 		},
-		ExpectedValue{
+		{
 			envIndex:       17,
 			envVarConstant: "WAL_BUCKET_SCOPE_PREFIX",
 			envVarValue:    "",
@@ -149,27 +149,48 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 	}
 
 	expectedValuesGCPCreds := []ExpectedValue{
-		ExpectedValue{
+		{
 			envIndex:       15,
 			envVarConstant: "WAL_GS_BUCKET",
 			envVarValue:    "wale-gs-bucket",
 		},
-		ExpectedValue{
+		{
 			envIndex:       16,
 			envVarConstant: "WAL_BUCKET_SCOPE_SUFFIX",
 			envVarValue:    "/SomeUUID",
 		},
-		ExpectedValue{
+		{
 			envIndex:       17,
 			envVarConstant: "WAL_BUCKET_SCOPE_PREFIX",
 			envVarValue:    "",
 		},
-		ExpectedValue{
+		{
 			envIndex:       18,
 			envVarConstant: "GOOGLE_APPLICATION_CREDENTIALS",
 			envVarValue:    "some_path_to_credentials",
 		},
 	}
+	expectedClusterNameLabel := []ExpectedValue{
+		{
+			envIndex:       5,
+			envVarConstant: "KUBERNETES_SCOPE_LABEL",
+			envVarValue:    "cluster-name",
+		},
+	}
+	expectedCustomS3Bucket := []ExpectedValue{
+		{
+			envIndex:       15,
+			envVarConstant: "WAL_S3_BUCKET",
+			envVarValue:    "custom-s3-bucket",
+		},
+	}
+	expectedCustomVariable := []ExpectedValue{
+		{
+			envIndex:       15,
+			envVarConstant: "CUSTOM_VARIABLE",
+			envVarValue:    "cluster-variable",
+		},
+	}
 
 	testName := "TestGenerateSpiloPodEnvVars"
 	tests := []struct {
@@ -181,6 +202,7 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 		standbyDescription *acidv1.StandbyDescription
 		customEnvList      []v1.EnvVar
 		expectedValues     []ExpectedValue
+		pgsql              acidv1.Postgresql
 	}{
 		{
 			subTest: "Will set WAL_GS_BUCKET env",
@@ -207,10 +229,81 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) {
 			customEnvList:      []v1.EnvVar{},
 			expectedValues:     expectedValuesGCPCreds,
 		},
+		{
+			subTest: "Will not overwrite global KUBERNETES_SCOPE_LABEL parameter from the cluster Env option",
+			opConfig: config.Config{
+				Resources: config.Resources{
+					ClusterNameLabel: "cluster-name",
+				},
+			},
+			uid:                "SomeUUID",
+			spiloConfig:        "someConfig",
+			cloneDescription:   &acidv1.CloneDescription{},
+			standbyDescription: &acidv1.StandbyDescription{},
+			customEnvList:      []v1.EnvVar{},
+			expectedValues:     expectedClusterNameLabel,
+			pgsql: acidv1.Postgresql{
+				Spec: acidv1.PostgresSpec{
+					Env: []v1.EnvVar{
+						{
+							Name:  "KUBERNETES_SCOPE_LABEL",
+							Value: "my-scope-label",
+						},
+					},
+				},
+			},
+		},
+		{
+			subTest: "Will overwrite global WAL_S3_BUCKET parameter from the cluster Env option",
+			opConfig: config.Config{
+				WALGSBucket: "global-s3-bucket",
+			},
+			uid:                "SomeUUID",
+			spiloConfig:        "someConfig",
+			cloneDescription:   &acidv1.CloneDescription{},
+			standbyDescription: &acidv1.StandbyDescription{},
+			customEnvList:      []v1.EnvVar{},
+			expectedValues:     expectedCustomS3Bucket,
+			pgsql: acidv1.Postgresql{
+				Spec: acidv1.PostgresSpec{
+					Env: []v1.EnvVar{
+						{
+							Name:  "WAL_S3_BUCKET",
+							Value: "custom-s3-bucket",
+						},
+					},
+				},
+			},
+		},
+		{
+			subTest:            "Will overwrite custom variable parameter from the cluster Env option",
+			uid:                "SomeUUID",
+			spiloConfig:        "someConfig",
+			cloneDescription:   &acidv1.CloneDescription{},
+			standbyDescription: &acidv1.StandbyDescription{},
+			customEnvList: []v1.EnvVar{
+				{
+					Name:  "CUSTOM_VARIABLE",
+					Value: "custom-variable",
+				},
+			},
+			expectedValues: expectedCustomVariable,
+			pgsql: acidv1.Postgresql{
+				Spec: acidv1.PostgresSpec{
+					Env: []v1.EnvVar{
+						{
+							Name:  "CUSTOM_VARIABLE",
+							Value: "cluster-variable",
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		cluster.OpConfig = tt.opConfig
+		cluster.Postgresql = tt.pgsql
 
 		actualEnvs := cluster.generateSpiloPodEnvVars(tt.uid, tt.spiloConfig, tt.cloneDescription, tt.standbyDescription, tt.customEnvList)
 
@@ -853,7 +946,7 @@ func TestPodEnvironmentConfigMapVariables(t *testing.T) {
 			err: fmt.Errorf("could not read PodEnvironmentConfigMap: NotFound"),
 		},
 		{
-			subTest: "simple PodEnvironmentConfigMap",
+			subTest: "Pod environment vars configured by PodEnvironmentConfigMap",
 			opConfig: config.Config{
 				Resources: config.Resources{
 					PodEnvironmentConfigMap: spec.NamespacedName{