Add a validated field for Postgres parameters

The validation rules of Kubernetes 1.29 (Beta in 1.25) allow for this
kind of field.

Issue: PGO-313

config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

@@ -4370,6 +4370,7 @@ spec:
               config:
                 properties:
                   files:
+                    description: Files to mount under "/etc/postgres".
                     items:
                       description: |-
                         Projection that may be projected along with other supported volume types.
@@ -4688,6 +4689,54 @@ spec:
                           type: object
                       type: object
                     type: array
+                  parameters:
+                    additionalProperties:
+                      anyOf:
+                      - type: integer
+                      - type: string
+                      x-kubernetes-int-or-string: true
+                    description: |-
+                      Configuration parameters for the PostgreSQL server. Some values will
+                      be reloaded without validation and some cause PostgreSQL to restart.
+                      Some values cannot be changed at all.
+                      More info: https://www.postgresql.org/docs/current/runtime-config.html
+                    maxProperties: 50
+                    type: object
+                    x-kubernetes-map-type: granular
+                    x-kubernetes-validations:
+                    - message: 'cannot change PGDATA path: config_file, data_directory'
+                      rule: '!has(self.config_file) && !has(self.data_directory)'
+                    - message: cannot change external_pid_file
+                      rule: '!has(self.external_pid_file)'
+                    - message: 'cannot change authentication path: hba_file, ident_file'
+                      rule: '!has(self.hba_file) && !has(self.ident_file)'
+                    - message: 'network connectivity is always enabled: listen_addresses'
+                      rule: '!has(self.listen_addresses)'
+                    - message: change port using .spec.port instead
+                      rule: '!has(self.port)'
+                    - message: TLS is always enabled
+                      rule: '!has(self.ssl) && !self.exists(k, k.startsWith("ssl_"))'
+                    - message: domain socket paths cannot be changed
+                      rule: '!self.exists(k, k.startsWith("unix_socket_"))'
+                    - message: wal_level must be "replica" or higher
+                      rule: '!has(self.wal_level) || self.wal_level in ["logical"]'
+                    - message: wal_log_hints are always enabled
+                      rule: '!has(self.wal_log_hints)'
+                    - rule: '!has(self.archive_mode) && !has(self.archive_command)
+                        && !has(self.restore_command)'
+                    - rule: '!has(self.recovery_target) && !self.exists(k, k.startsWith("recovery_target_"))'
+                    - message: hot_standby is always enabled
+                      rule: '!has(self.hot_standby)'
+                    - rule: '!has(self.synchronous_standby_names)'
+                    - rule: '!has(self.primary_conninfo) && !has(self.primary_slot_name)'
+                    - message: delayed replication is not supported at this time
+                      rule: '!has(self.recovery_min_apply_delay)'
+                    - message: cluster_name is derived from the PostgresCluster name
+                      rule: '!has(self.cluster_name)'
+                    - message: disabling logging_collector is unsafe
+                      rule: '!has(self.logging_collector)'
+                    - message: log_file_mode cannot be changed
+                      rule: '!has(self.log_file_mode)'
                 type: object
               customReplicationTLSSecret:
                 description: |-

internal/config/config.go

@@ -22,20 +22,24 @@ func defaultFromEnv(value, key string) string {
 // FetchKeyCommand returns the fetch_key_cmd value stored in the encryption_key_command
 // variable used to enable TDE.
 func FetchKeyCommand(spec *v1beta1.PostgresClusterSpec) string {
+	if parameters := spec.Config.Parameters; parameters != nil {
+		if v, ok := parameters["encryption_key_command"]; ok {
+			return v.String()
+		}
+	}
+
 	if spec.Patroni != nil {
-		if spec.Patroni.DynamicConfiguration != nil {
-			configuration := spec.Patroni.DynamicConfiguration
-			if configuration != nil {
-				if postgresql, ok := configuration["postgresql"].(map[string]any); ok {
-					if parameters, ok := postgresql["parameters"].(map[string]any); ok {
-						if parameters["encryption_key_command"] != nil {
-							return fmt.Sprintf("%s", parameters["encryption_key_command"])
-						}
+		if configuration := spec.Patroni.DynamicConfiguration; configuration != nil {
+			if postgresql, ok := configuration["postgresql"].(map[string]any); ok {
+				if parameters, ok := postgresql["parameters"].(map[string]any); ok {
+					if parameters["encryption_key_command"] != nil {
+						return fmt.Sprintf("%s", parameters["encryption_key_command"])
 					}
 				}
 			}
 		}
 	}
+
 	return ""
 }
 

internal/config/config_test.go

@@ -15,68 +15,115 @@ import (
 )
 
 func TestFetchKeyCommand(t *testing.T) {
-
-	spec1 := v1beta1.PostgresClusterSpec{}
-	assert.Assert(t, FetchKeyCommand(&spec1) == "")
-
-	spec2 := v1beta1.PostgresClusterSpec{
-		Patroni: &v1beta1.PatroniSpec{},
-	}
-	assert.Assert(t, FetchKeyCommand(&spec2) == "")
-
-	spec3 := v1beta1.PostgresClusterSpec{
-		Patroni: &v1beta1.PatroniSpec{
-			DynamicConfiguration: map[string]any{},
-		},
-	}
-	assert.Assert(t, FetchKeyCommand(&spec3) == "")
-
-	spec4 := v1beta1.PostgresClusterSpec{
-		Patroni: &v1beta1.PatroniSpec{
-			DynamicConfiguration: map[string]any{
-				"postgresql": map[string]any{},
+	t.Run("missing", func(t *testing.T) {
+		spec1 := v1beta1.PostgresClusterSpec{}
+		assert.Assert(t, FetchKeyCommand(&spec1) == "")
+
+		spec2 := v1beta1.PostgresClusterSpec{
+			Patroni: &v1beta1.PatroniSpec{},
+		}
+		assert.Assert(t, FetchKeyCommand(&spec2) == "")
+
+		spec3 := v1beta1.PostgresClusterSpec{
+			Patroni: &v1beta1.PatroniSpec{
+				DynamicConfiguration: map[string]any{},
 			},
-		},
-	}
-	assert.Assert(t, FetchKeyCommand(&spec4) == "")
+		}
+		assert.Assert(t, FetchKeyCommand(&spec3) == "")
 
-	spec5 := v1beta1.PostgresClusterSpec{
-		Patroni: &v1beta1.PatroniSpec{
-			DynamicConfiguration: map[string]any{
-				"postgresql": map[string]any{
-					"parameters": map[string]any{},
+		spec4 := v1beta1.PostgresClusterSpec{
+			Patroni: &v1beta1.PatroniSpec{
+				DynamicConfiguration: map[string]any{
+					"postgresql": map[string]any{},
 				},
 			},
-		},
-	}
-	assert.Assert(t, FetchKeyCommand(&spec5) == "")
-
-	spec6 := v1beta1.PostgresClusterSpec{
-		Patroni: &v1beta1.PatroniSpec{
-			DynamicConfiguration: map[string]any{
-				"postgresql": map[string]any{
-					"parameters": map[string]any{
-						"encryption_key_command": "",
+		}
+		assert.Assert(t, FetchKeyCommand(&spec4) == "")
+
+		spec5 := v1beta1.PostgresClusterSpec{
+			Patroni: &v1beta1.PatroniSpec{
+				DynamicConfiguration: map[string]any{
+					"postgresql": map[string]any{
+						"parameters": map[string]any{},
 					},
 				},
 			},
-		},
-	}
-	assert.Assert(t, FetchKeyCommand(&spec6) == "")
-
-	spec7 := v1beta1.PostgresClusterSpec{
-		Patroni: &v1beta1.PatroniSpec{
-			DynamicConfiguration: map[string]any{
-				"postgresql": map[string]any{
-					"parameters": map[string]any{
-						"encryption_key_command": "echo mykey",
+		}
+		assert.Assert(t, FetchKeyCommand(&spec5) == "")
+	})
+
+	t.Run("blank", func(t *testing.T) {
+		var spec1 v1beta1.PostgresClusterSpec
+		assert.NilError(t, yaml.Unmarshal([]byte(`{
+			patroni: {
+				dynamicConfiguration: {
+					postgresql: {
+						parameters: {
+							encryption_key_command: "",
+						},
 					},
 				},
 			},
-		},
-	}
-	assert.Assert(t, FetchKeyCommand(&spec7) == "echo mykey")
+		}`), &spec1))
+		assert.Equal(t, "", FetchKeyCommand(&spec1))
+
+		var spec2 v1beta1.PostgresClusterSpec
+		assert.NilError(t, yaml.Unmarshal([]byte(`{
+			config: {
+				parameters: {
+					encryption_key_command: "",
+				},
+			},
+		}`), &spec2))
+		assert.Equal(t, "", FetchKeyCommand(&spec2))
+	})
+
+	t.Run("exists", func(t *testing.T) {
+		var spec1 v1beta1.PostgresClusterSpec
+		assert.NilError(t, yaml.Unmarshal([]byte(`{
+			patroni: {
+				dynamicConfiguration: {
+					postgresql: {
+						parameters: {
+							encryption_key_command: "echo mykey",
+						},
+					},
+				},
+			},
+		}`), &spec1))
+		assert.Equal(t, "echo mykey", FetchKeyCommand(&spec1))
+
+		var spec2 v1beta1.PostgresClusterSpec
+		assert.NilError(t, yaml.Unmarshal([]byte(`{
+			config: {
+				parameters: {
+					encryption_key_command: "cat somefile",
+				},
+			},
+		}`), &spec2))
+		assert.Equal(t, "cat somefile", FetchKeyCommand(&spec2))
+	})
 
+	t.Run("config.parameters takes precedence", func(t *testing.T) {
+		var spec v1beta1.PostgresClusterSpec
+		assert.NilError(t, yaml.Unmarshal([]byte(`{
+			config: {
+				parameters: {
+					encryption_key_command: "cat somefile",
+				},
+			},
+			patroni: {
+				dynamicConfiguration: {
+					postgresql: {
+						parameters: {
+							encryption_key_command: "echo mykey",
+						},
+					},
+				},
+			},
+		}`), &spec))
+		assert.Equal(t, "cat somefile", FetchKeyCommand(&spec))
+	})
 }
 
 func TestPGAdminContainerImage(t *testing.T) {

internal/patroni/config.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/yaml"
 
 	"github.com/crunchydata/postgres-operator/internal/config"
@@ -244,7 +245,11 @@ func DynamicConfiguration(
 			parameters[k] = v
 		}
 	}
-	// Override the above with mandatory parameters.
+	// Copy spec.config.parameters over spec.patroni...parameters.
+	for k, v := range spec.Config.Parameters {
+		parameters[k] = v
+	}
+	// Override all of the above with mandatory parameters.
 	if pgParameters.Mandatory != nil {
 		for k, v := range pgParameters.Mandatory.AsMap() {
 
@@ -254,8 +259,15 @@ func DynamicConfiguration(
 			// that out as well.
 			if k == "shared_preload_libraries" {
 				// Load mandatory libraries ahead of user-defined libraries.
-				if s, ok := parameters[k].(string); ok && len(s) > 0 {
-					v = v + "," + s
+				switch s := parameters[k].(type) {
+				case string:
+					if len(s) > 0 {
+						v = v + "," + s
+					}
+				case intstr.IntOrString:
+					if len(s.StrVal) > 0 {
+						v = v + "," + s.StrVal
+					}
 				}
 				// Load "citus" ahead of any other libraries.
 				// - https://github.com/citusdata/citus/blob/v12.0.0/src/backend/distributed/shared_library_init.c#L417-L419