diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 6e055a5911..36f73ce36c 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/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,52 @@ 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-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)' + - 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 ["replica","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: |- diff --git a/internal/config/config.go b/internal/config/config.go index e3f9ced215..62d8e67bec 100644 --- a/internal/config/config.go +++ b/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 "" } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7b8ca2f863..19634667fc 100644 --- a/internal/config/config_test.go +++ b/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) { diff --git a/internal/patroni/config.go b/internal/patroni/config.go index 64645ec2dd..dcddff4077 100644 --- a/internal/patroni/config.go +++ b/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" @@ -241,7 +242,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() { @@ -251,8 +256,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 diff --git a/internal/patroni/config_test.go b/internal/patroni/config_test.go index eb8b12918f..de04f1d2dd 100644 --- a/internal/patroni/config_test.go +++ b/internal/patroni/config_test.go @@ -15,6 +15,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/yaml" "github.com/crunchydata/postgres-operator/internal/postgres" @@ -384,8 +385,13 @@ func TestDynamicConfiguration(t *testing.T) { }, }, { - name: "postgresql.parameters: input overrides default", + name: "config.parameters takes precedence", spec: `{ + config: { + parameters: { + something: this, + }, + }, patroni: { dynamicConfiguration: { postgresql: { @@ -397,6 +403,30 @@ func TestDynamicConfiguration(t *testing.T) { }, }, }`, + expected: map[string]any{ + "loop_wait": int32(10), + "ttl": int32(30), + "postgresql": map[string]any{ + "parameters": map[string]any{ + "something": intstr.FromString("this"), + "another": float64(5), + }, + "pg_hba": []string{}, + "use_pg_rewind": true, + "use_slots": false, + }, + }, + }, + { + name: "config.parameters: input overrides default", + spec: `{ + config: { + parameters: { + something: str, + another: 5, + }, + }, + }`, params: postgres.Parameters{ Default: parameters(map[string]string{ "something": "overridden", @@ -408,8 +438,8 @@ func TestDynamicConfiguration(t *testing.T) { "ttl": int32(30), "postgresql": map[string]any{ "parameters": map[string]any{ - "something": "str", - "another": float64(5), + "something": intstr.FromString("str"), + "another": intstr.FromInt(5), "unrelated": "default", }, "pg_hba": []string{}, @@ -419,16 +449,12 @@ func TestDynamicConfiguration(t *testing.T) { }, }, { - name: "postgresql.parameters: mandatory overrides input", + name: "config.parameters: mandatory overrides input", spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - parameters: { - something: str, - another: 5, - }, - }, + config: { + parameters: { + something: str, + another: 5, }, }, }`, @@ -444,7 +470,7 @@ func TestDynamicConfiguration(t *testing.T) { "postgresql": map[string]any{ "parameters": map[string]any{ "something": "overrides", - "another": float64(5), + "another": intstr.FromInt(5), "unrelated": "setting", }, "pg_hba": []string{}, @@ -454,15 +480,11 @@ func TestDynamicConfiguration(t *testing.T) { }, }, { - name: "postgresql.parameters: mandatory shared_preload_libraries", + name: "config.parameters: mandatory shared_preload_libraries", spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - parameters: { - shared_preload_libraries: given, - }, - }, + config: { + parameters: { + shared_preload_libraries: given, }, }, }`, @@ -485,15 +507,11 @@ func TestDynamicConfiguration(t *testing.T) { }, }, { - name: "postgresql.parameters: mandatory shared_preload_libraries wrong-type is ignored", + name: "config.parameters: mandatory shared_preload_libraries wrong-type is ignored", spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - parameters: { - shared_preload_libraries: 1, - }, - }, + config: { + parameters: { + shared_preload_libraries: 1, }, }, }`, @@ -516,15 +534,11 @@ func TestDynamicConfiguration(t *testing.T) { }, }, { - name: "postgresql.parameters: shared_preload_libraries order", + name: "config.parameters: shared_preload_libraries order", spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - parameters: { - shared_preload_libraries: "given, citus, more", - }, - }, + config: { + parameters: { + shared_preload_libraries: "given, citus, more", }, }, }`, @@ -839,7 +853,30 @@ func TestDynamicConfiguration(t *testing.T) { }, }, { - name: "tde enabled", + name: "config.parameters: tde enabled", + spec: `{ + config: { + parameters: { + encryption_key_command: echo one, + }, + }, + }`, + expected: map[string]any{ + "loop_wait": int32(10), + "ttl": int32(30), + "postgresql": map[string]any{ + "bin_name": map[string]any{"pg_rewind": string("/tmp/pg_rewind_tde.sh")}, + "parameters": map[string]any{ + "encryption_key_command": intstr.FromString("echo one"), + }, + "pg_hba": []string{}, + "use_pg_rewind": bool(true), + "use_slots": bool(false), + }, + }, + }, + { + name: "postgresql.parameters: tde enabled", spec: `{ patroni: { dynamicConfiguration: { diff --git a/internal/pgbackrest/reconcile.go b/internal/pgbackrest/reconcile.go index d22bccc3c0..86060d3c2b 100644 --- a/internal/pgbackrest/reconcile.go +++ b/internal/pgbackrest/reconcile.go @@ -214,7 +214,7 @@ func AddConfigToRestorePod( // mount any provided configuration files to the restore Job Pod if len(cluster.Spec.Config.Files) != 0 { - additionalConfigVolumeMount := postgres.AdditionalConfigVolumeMount() + additionalConfigVolumeMount := postgres.ConfigVolumeMount() additionalConfigVolume := corev1.Volume{Name: additionalConfigVolumeMount.Name} additionalConfigVolume.Projected = &corev1.ProjectedVolumeSource{ Sources: append(sources, cluster.Spec.Config.Files...), diff --git a/internal/postgres/config.go b/internal/postgres/config.go index db46ea3ba7..fbf0f14c4d 100644 --- a/internal/postgres/config.go +++ b/internal/postgres/config.go @@ -70,7 +70,7 @@ safelink() ( // for streaming replication and for `pg_rewind`. ReplicationUser = "_crunchyrepl" - // configMountPath is where to mount additional config files + // configMountPath is where to mount config files. configMountPath = "/etc/postgres" ) diff --git a/internal/postgres/reconcile.go b/internal/postgres/reconcile.go index 779a0f5677..c25b8b23ee 100644 --- a/internal/postgres/reconcile.go +++ b/internal/postgres/reconcile.go @@ -46,8 +46,8 @@ func DownwardAPIVolumeMount() corev1.VolumeMount { } } -// AdditionalConfigVolumeMount returns the name and mount path of the additional config files. -func AdditionalConfigVolumeMount() corev1.VolumeMount { +// ConfigVolumeMount returns the name and mount path of PostgreSQL config files. +func ConfigVolumeMount() corev1.VolumeMount { return corev1.VolumeMount{ Name: "postgres-config", MountPath: configMountPath, @@ -233,7 +233,7 @@ func InstancePod(ctx context.Context, } if len(inCluster.Spec.Config.Files) != 0 { - additionalConfigVolumeMount := AdditionalConfigVolumeMount() + additionalConfigVolumeMount := ConfigVolumeMount() additionalConfigVolume := corev1.Volume{Name: additionalConfigVolumeMount.Name} additionalConfigVolume.Projected = &corev1.ProjectedVolumeSource{ Sources: append([]corev1.VolumeProjection{}, inCluster.Spec.Config.Files...), diff --git a/internal/testing/validation/postgrescluster_test.go b/internal/testing/validation/postgrescluster_test.go index e71ff22b2e..f4f8516364 100644 --- a/internal/testing/validation/postgrescluster_test.go +++ b/internal/testing/validation/postgrescluster_test.go @@ -11,6 +11,7 @@ import ( "gotest.tools/v3/assert" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" @@ -19,6 +20,178 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestPostgresConfigParameters(t *testing.T) { + ctx := context.Background() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1beta1.NewPostgresCluster() + + // Start with a bunch of required fields. + assert.NilError(t, yaml.Unmarshal([]byte(`{ + postgresVersion: 16, + backups: { + pgbackrest: { + repos: [{ name: repo1 }], + }, + }, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`), &base.Spec)) + + base.Namespace = namespace.Name + base.Name = "postgres-config-parameters" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + t.Run("Disallowed", func(t *testing.T) { + for _, tt := range []struct { + key string + value intstr.IntOrString + }{ + {key: "cluster_name", value: intstr.FromString("asdf")}, + {key: "config_file", value: intstr.FromString("asdf")}, + {key: "data_directory", value: intstr.FromString("")}, + {key: "external_pid_file", value: intstr.FromString("")}, + {key: "hba_file", value: intstr.FromString("one")}, + {key: "hot_standby", value: intstr.FromString("off")}, + {key: "ident_file", value: intstr.FromString("two")}, + {key: "listen_addresses", value: intstr.FromString("")}, + {key: "log_file_mode", value: intstr.FromString("")}, + {key: "logging_collector", value: intstr.FromString("off")}, + {key: "port", value: intstr.FromInt(5)}, + {key: "wal_log_hints", value: intstr.FromString("off")}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + tt.key: tt.value, + } + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + //nolint:errorlint // This is a test, and a panic is unlikely. + status := err.(apierrors.APIStatus).Status() + assert.Assert(t, status.Details != nil) + assert.Equal(t, len(status.Details.Causes), 1) + + // TODO(k8s-1.30) TODO(validation): Move the parameter name from the message to the field path. + assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters") + assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, tt.key)) + }) + } + }) + + t.Run("NoConnections", func(t *testing.T) { + for _, tt := range []struct { + key string + value intstr.IntOrString + }{ + {key: "ssl", value: intstr.FromString("off")}, + {key: "ssl_ca_file", value: intstr.FromString("")}, + {key: "unix_socket_directories", value: intstr.FromString("one")}, + {key: "unix_socket_group", value: intstr.FromString("two")}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + tt.key: tt.value, + } + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + }) + } + }) + + t.Run("NoWriteAheadLog", func(t *testing.T) { + for _, tt := range []struct { + key string + value intstr.IntOrString + }{ + {key: "archive_mode", value: intstr.FromString("off")}, + {key: "archive_command", value: intstr.FromString("true")}, + {key: "restore_command", value: intstr.FromString("true")}, + {key: "recovery_target", value: intstr.FromString("immediate")}, + {key: "recovery_target_name", value: intstr.FromString("doot")}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + tt.key: tt.value, + } + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + }) + } + }) + + t.Run("wal_level", func(t *testing.T) { + t.Run("Valid", func(t *testing.T) { + cluster := base.DeepCopy() + + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + "wal_level": intstr.FromString("logical"), + } + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + "wal_level": intstr.FromString("replica"), + } + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) + + t.Run("Invalid", func(t *testing.T) { + cluster := base.DeepCopy() + + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + "wal_level": intstr.FromString("minimal"), + } + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, `"replica" or higher`) + + //nolint:errorlint // This is a test, and a panic is unlikely. + status := err.(apierrors.APIStatus).Status() + assert.Assert(t, status.Details != nil) + assert.Equal(t, len(status.Details.Causes), 1) + assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters") + assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, "wal_level")) + }) + }) + + t.Run("NoReplication", func(t *testing.T) { + for _, tt := range []struct { + key string + value intstr.IntOrString + }{ + {key: "synchronous_standby_names", value: intstr.FromString("")}, + {key: "primary_conninfo", value: intstr.FromString("")}, + {key: "primary_slot_name", value: intstr.FromString("")}, + {key: "recovery_min_apply_delay", value: intstr.FromString("")}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + cluster.Spec.Config.Parameters = map[string]intstr.IntOrString{ + tt.key: tt.value, + } + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + }) + } + }) +} + func TestPostgresUserOptions(t *testing.T) { ctx := context.Background() cc := require.Kubernetes(t) diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go index b7baa72942..b9b8f72004 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go @@ -4,6 +4,70 @@ package v1beta1 +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +type PostgresConfig struct { + + // Files to mount under "/etc/postgres". + // --- + // +kubebuilder:validation:Optional + Files []corev1.VolumeProjection `json:"files,omitempty"` + + // 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 + // --- + // + // Postgres 17 has something like 350+ built-in parameters, but typically + // an administrator will change only a handful of these. + // +kubebuilder:validation:MaxProperties=50 + // + // # File Locations + // - https://www.postgresql.org/docs/current/runtime-config-file-locations.html + // + // +kubebuilder:validation:XValidation:rule=`!has(self.config_file) && !has(self.data_directory)`,message=`cannot change PGDATA path: config_file, data_directory` + // +kubebuilder:validation:XValidation:rule=`!has(self.external_pid_file)`,message=`cannot change external_pid_file` + // +kubebuilder:validation:XValidation:rule=`!has(self.hba_file) && !has(self.ident_file)`,message=`cannot change authentication path: hba_file, ident_file` + // + // # Connections + // - https://www.postgresql.org/docs/current/runtime-config-connection.html + // + // +kubebuilder:validation:XValidation:rule=`!has(self.listen_addresses)`,message=`network connectivity is always enabled: listen_addresses` + // +kubebuilder:validation:XValidation:rule=`!has(self.port)`,message=`change port using .spec.port instead` + // +kubebuilder:validation:XValidation:rule=`!has(self.ssl) && !self.exists(k, k.startsWith("ssl_"))` + // +kubebuilder:validation:XValidation:rule=`!self.exists(k, k.startsWith("unix_socket_"))`,message=`domain socket paths cannot be changed` + // + // # Write Ahead Log + // - https://www.postgresql.org/docs/current/runtime-config-wal.html + // + // +kubebuilder:validation:XValidation:rule=`!has(self.wal_level) || self.wal_level in ["replica","logical"]`,message=`wal_level must be "replica" or higher` + // +kubebuilder:validation:XValidation:rule=`!has(self.wal_log_hints)`,message=`wal_log_hints are always enabled` + // +kubebuilder:validation:XValidation:rule=`!has(self.archive_mode) && !has(self.archive_command) && !has(self.restore_command)` + // +kubebuilder:validation:XValidation:rule=`!has(self.recovery_target) && !self.exists(k, k.startsWith("recovery_target_"))` + // + // # Replication + // - https://www.postgresql.org/docs/current/runtime-config-replication.html + // + // +kubebuilder:validation:XValidation:rule=`!has(self.hot_standby)`,message=`hot_standby is always enabled` + // +kubebuilder:validation:XValidation:rule=`!has(self.synchronous_standby_names)` + // +kubebuilder:validation:XValidation:rule=`!has(self.primary_conninfo) && !has(self.primary_slot_name)` + // +kubebuilder:validation:XValidation:rule=`!has(self.recovery_min_apply_delay)`,message=`delayed replication is not supported at this time` + // + // # Logging + // - https://www.postgresql.org/docs/current/runtime-config-logging.html + // + // +kubebuilder:validation:XValidation:rule=`!has(self.cluster_name)`,message=`cluster_name is derived from the PostgresCluster name` + // +kubebuilder:validation:XValidation:rule=`!has(self.logging_collector)`,message=`disabling logging_collector is unsafe` + // +kubebuilder:validation:XValidation:rule=`!has(self.log_file_mode)`,message=`log_file_mode cannot be changed` + // + // +optional + Parameters map[string]intstr.IntOrString `json:"parameters,omitempty"` +} + // PostgreSQL identifiers are limited in length but may contain any character. // More info: https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS // diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go index 54e42baa3b..c7dbafc0fb 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go @@ -178,7 +178,7 @@ type PostgresClusterSpec struct { // +optional Users []PostgresUserSpec `json:"users,omitempty"` - Config PostgresAdditionalConfig `json:"config,omitempty"` + Config PostgresConfig `json:"config,omitempty"` } // DataSource defines data sources for a new PostgresCluster. @@ -670,10 +670,6 @@ type PostgresUserInterfaceStatus struct { PGAdmin PGAdminPodStatus `json:"pgAdmin,omitempty"` } -type PostgresAdditionalConfig struct { - Files []corev1.VolumeProjection `json:"files,omitempty"` -} - // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +operator-sdk:csv:customresourcedefinitions:resources={{ConfigMap,v1},{Secret,v1},{Service,v1},{CronJob,v1beta1},{Deployment,v1},{Job,v1},{StatefulSet,v1},{PersistentVolumeClaim,v1}} diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go index e8d8826c22..3163a61d6e 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go @@ -1558,28 +1558,6 @@ func (in *PatroniSwitchover) DeepCopy() *PatroniSwitchover { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PostgresAdditionalConfig) DeepCopyInto(out *PostgresAdditionalConfig) { - *out = *in - if in.Files != nil { - in, out := &in.Files, &out.Files - *out = make([]corev1.VolumeProjection, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresAdditionalConfig. -func (in *PostgresAdditionalConfig) DeepCopy() *PostgresAdditionalConfig { - if in == nil { - return nil - } - out := new(PostgresAdditionalConfig) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresCluster) DeepCopyInto(out *PostgresCluster) { *out = *in @@ -1855,6 +1833,35 @@ func (in *PostgresClusterStatus) DeepCopy() *PostgresClusterStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresConfig) DeepCopyInto(out *PostgresConfig) { + *out = *in + if in.Files != nil { + in, out := &in.Files, &out.Files + *out = make([]corev1.VolumeProjection, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Parameters != nil { + in, out := &in.Parameters, &out.Parameters + *out = make(map[string]intstr.IntOrString, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresConfig. +func (in *PostgresConfig) DeepCopy() *PostgresConfig { + if in == nil { + return nil + } + out := new(PostgresConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresInstanceSetSpec) DeepCopyInto(out *PostgresInstanceSetSpec) { *out = *in diff --git a/testing/kuttl/e2e/major-upgrade-missing-image/10--cluster.yaml b/testing/kuttl/e2e/major-upgrade-missing-image/10--cluster.yaml index f5ef8c029e..8a0e57bab6 100644 --- a/testing/kuttl/e2e/major-upgrade-missing-image/10--cluster.yaml +++ b/testing/kuttl/e2e/major-upgrade-missing-image/10--cluster.yaml @@ -8,11 +8,9 @@ metadata: spec: # postgres version that is no longer available postgresVersion: 11 - patroni: - dynamicConfiguration: - postgresql: - parameters: - shared_preload_libraries: pgaudit, set_user, pg_stat_statements, pgnodemx, pg_cron + config: + parameters: + shared_preload_libraries: pgaudit, set_user, pg_stat_statements, pgnodemx, pg_cron instances: - dataVolumeClaimSpec: { accessModes: [ReadWriteOnce], resources: { requests: { storage: 1Gi } } } backups: diff --git a/testing/kuttl/e2e/major-upgrade/30--cluster.yaml b/testing/kuttl/e2e/major-upgrade/30--cluster.yaml index 01e1ef6175..07546c384e 100644 --- a/testing/kuttl/e2e/major-upgrade/30--cluster.yaml +++ b/testing/kuttl/e2e/major-upgrade/30--cluster.yaml @@ -6,11 +6,9 @@ metadata: name: major-upgrade spec: postgresVersion: ${KUTTL_PG_UPGRADE_FROM_VERSION} - patroni: - dynamicConfiguration: - postgresql: - parameters: - shared_preload_libraries: pgaudit, set_user, pg_stat_statements, pgnodemx, pg_cron + config: + parameters: + shared_preload_libraries: pgaudit, set_user, pg_stat_statements, pgnodemx, pg_cron instances: - dataVolumeClaimSpec: { accessModes: [ReadWriteOnce], resources: { requests: { storage: 1Gi } } } replicas: 3 diff --git a/testing/kuttl/e2e/pgbackrest-restore/01--create-cluster.yaml b/testing/kuttl/e2e/pgbackrest-restore/01--create-cluster.yaml index c414806892..5c562189f4 100644 --- a/testing/kuttl/e2e/pgbackrest-restore/01--create-cluster.yaml +++ b/testing/kuttl/e2e/pgbackrest-restore/01--create-cluster.yaml @@ -8,11 +8,9 @@ metadata: labels: { postgres-operator-test: kuttl } spec: postgresVersion: ${KUTTL_PG_VERSION} - patroni: - dynamicConfiguration: - postgresql: - parameters: - max_connections: 200 + config: + parameters: + max_connections: 200 instances: - dataVolumeClaimSpec: { accessModes: [ReadWriteOnce], resources: { requests: { storage: 1Gi } } } replicas: 2 diff --git a/testing/kuttl/e2e/pgbackrest-restore/07--update-cluster.yaml b/testing/kuttl/e2e/pgbackrest-restore/07--update-cluster.yaml index f83a02c7c6..0c8cb99b98 100644 --- a/testing/kuttl/e2e/pgbackrest-restore/07--update-cluster.yaml +++ b/testing/kuttl/e2e/pgbackrest-restore/07--update-cluster.yaml @@ -7,11 +7,9 @@ metadata: labels: { postgres-operator-test: kuttl } spec: postgresVersion: ${KUTTL_PG_VERSION} - patroni: - dynamicConfiguration: - postgresql: - parameters: - max_connections: 1000 + config: + parameters: + max_connections: 1000 instances: - dataVolumeClaimSpec: { accessModes: [ReadWriteOnce], resources: { requests: { storage: 1Gi } } } replicas: 2