From 8f206a418025570c8fdac6a9ceb5333e481a0aaa Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Thu, 19 Dec 2024 09:45:03 -0600 Subject: [PATCH 1/2] Consolidate arguments to a Patroni function When called at runtime, the second argument is always derived from the first. This simplifies those call sites and clarifies the behavior in tests. --- .../controller/postgrescluster/patroni.go | 9 +- internal/patroni/config.go | 50 ++- internal/patroni/config_test.go | 310 +++++++++++------- 3 files changed, 207 insertions(+), 162 deletions(-) diff --git a/internal/controller/postgrescluster/patroni.go b/internal/controller/postgrescluster/patroni.go index 1c5ac93eed..fb6df0a6ac 100644 --- a/internal/controller/postgrescluster/patroni.go +++ b/internal/controller/postgrescluster/patroni.go @@ -204,14 +204,9 @@ func (r *Reconciler) reconcilePatroniDynamicConfiguration( return r.PodExec(ctx, pod.Namespace, pod.Name, naming.ContainerDatabase, stdin, stdout, stderr, command...) } - var configuration map[string]any - if cluster.Spec.Patroni != nil { - configuration = cluster.Spec.Patroni.DynamicConfiguration - } - configuration = patroni.DynamicConfiguration(cluster, configuration, pgHBAs, pgParameters) - return errors.WithStack( - patroni.Executor(exec).ReplaceConfiguration(ctx, configuration)) + patroni.Executor(exec).ReplaceConfiguration(ctx, + patroni.DynamicConfiguration(&cluster.Spec, pgHBAs, pgParameters))) } // generatePatroniLeaderLeaseService returns a v1.Service that exposes the diff --git a/internal/patroni/config.go b/internal/patroni/config.go index 65b1f8d239..64645ec2dd 100644 --- a/internal/patroni/config.go +++ b/internal/patroni/config.go @@ -179,13 +179,8 @@ func clusterYAML( // Patroni has not yet bootstrapped. Populate the "bootstrap.dcs" field to // facilitate it. When Patroni is already bootstrapped, this field is ignored. - var configuration map[string]any - if cluster.Spec.Patroni != nil { - configuration = cluster.Spec.Patroni.DynamicConfiguration - } - root["bootstrap"] = map[string]any{ - "dcs": DynamicConfiguration(cluster, configuration, pgHBAs, pgParameters), + "dcs": DynamicConfiguration(&cluster.Spec, pgHBAs, pgParameters), // Missing here is "users" which runs *after* "post_bootstrap". It is // not possible to use roles created by the former in the latter. @@ -200,20 +195,19 @@ func clusterYAML( // DynamicConfiguration combines configuration with some PostgreSQL settings // and returns a value that can be marshaled to JSON. func DynamicConfiguration( - cluster *v1beta1.PostgresCluster, - configuration map[string]any, + spec *v1beta1.PostgresClusterSpec, pgHBAs postgres.HBAs, pgParameters postgres.Parameters, ) map[string]any { // Copy the entire configuration before making any changes. - root := make(map[string]any, len(configuration)) - for k, v := range configuration { - root[k] = v + root := make(map[string]any) + if spec.Patroni != nil && spec.Patroni.DynamicConfiguration != nil { + root = spec.Patroni.DynamicConfiguration.DeepCopy() } - root["ttl"] = *cluster.Spec.Patroni.LeaderLeaseDurationSeconds - root["loop_wait"] = *cluster.Spec.Patroni.SyncPeriodSeconds + // NOTE: These are always populated due to [v1beta1.PatroniSpec.Default] + root["ttl"] = *spec.Patroni.LeaderLeaseDurationSeconds + root["loop_wait"] = *spec.Patroni.SyncPeriodSeconds - // Copy the "postgresql" section before making any changes. postgresql := map[string]any{ // TODO(cbandy): explain this. requires an archive, perhaps. "use_slots": false, @@ -221,12 +215,13 @@ func DynamicConfiguration( // When TDE is configured, override the pg_rewind binary name to point // to the wrapper script. - if config.FetchKeyCommand(&cluster.Spec) != "" { + if config.FetchKeyCommand(spec) != "" { postgresql["bin_name"] = map[string]any{ "pg_rewind": "/tmp/pg_rewind_tde.sh", } } + // Copy the "postgresql" section over the above defaults. if section, ok := root["postgresql"].(map[string]any); ok { for k, v := range section { postgresql[k] = v @@ -300,15 +295,12 @@ func DynamicConfiguration( // Recent versions of `pg_rewind` can run with limited permissions granted // by Patroni to the user defined in "postgresql.authentication.rewind". // PostgreSQL v10 and earlier require superuser access over the network. - postgresql["use_pg_rewind"] = cluster.Spec.PostgresVersion > 10 - - if cluster.Spec.Standby != nil && cluster.Spec.Standby.Enabled { - // Copy the "standby_cluster" section before making any changes. - standby := make(map[string]any) - if section, ok := root["standby_cluster"].(map[string]any); ok { - for k, v := range section { - standby[k] = v - } + postgresql["use_pg_rewind"] = spec.PostgresVersion > 10 + + if spec.Standby != nil && spec.Standby.Enabled { + standby, _ := root["standby_cluster"].(map[string]any) + if standby == nil { + standby = make(map[string]any) } // Unset any previous value for restore_command - we will set it later if needed @@ -316,16 +308,16 @@ func DynamicConfiguration( // Populate replica creation methods based on options provided in the standby spec: methods := []string{} - if cluster.Spec.Standby.Host != "" { - standby["host"] = cluster.Spec.Standby.Host - if cluster.Spec.Standby.Port != nil { - standby["port"] = *cluster.Spec.Standby.Port + if spec.Standby.Host != "" { + standby["host"] = spec.Standby.Host + if spec.Standby.Port != nil { + standby["port"] = *spec.Standby.Port } methods = append([]string{basebackupCreateReplicaMethod}, methods...) } - if cluster.Spec.Standby.RepoName != "" { + if spec.Standby.RepoName != "" { // Append pgbackrest as the first choice when creating the standby methods = append([]string{pgBackRestCreateReplicaMethod}, methods...) diff --git a/internal/patroni/config_test.go b/internal/patroni/config_test.go index 38ade680b7..3470392bae 100644 --- a/internal/patroni/config_test.go +++ b/internal/patroni/config_test.go @@ -229,8 +229,7 @@ func TestDynamicConfiguration(t *testing.T) { for _, tt := range []struct { name string - cluster *v1beta1.PostgresCluster - input map[string]any + spec v1beta1.PostgresClusterSpec hbas postgres.HBAs params postgres.Parameters expected map[string]any @@ -250,13 +249,17 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "top-level passes through", - input: map[string]any{ - "retry_timeout": 5, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "retry_timeout": int64(5), + }, + }, }, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), - "retry_timeout": 5, + "retry_timeout": int64(5), "postgresql": map[string]any{ "parameters": map[string]any{}, "pg_hba": []string{}, @@ -267,18 +270,16 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "top-level: spec overrides input", - cluster: &v1beta1.PostgresCluster{ - Spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - LeaderLeaseDurationSeconds: initialize.Int32(99), - SyncPeriodSeconds: initialize.Int32(8), + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + LeaderLeaseDurationSeconds: initialize.Int32(99), + SyncPeriodSeconds: initialize.Int32(8), + DynamicConfiguration: map[string]any{ + "loop_wait": int64(3), + "ttl": "nope", }, }, }, - input: map[string]any{ - "loop_wait": 3, - "ttl": "nope", - }, expected: map[string]any{ "loop_wait": int32(8), "ttl": int32(99), @@ -292,8 +293,12 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql: wrong-type is ignored", - input: map[string]any{ - "postgresql": true, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": true, + }, + }, }, expected: map[string]any{ "loop_wait": int32(10), @@ -308,10 +313,14 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql: defaults and overrides", - input: map[string]any{ - "postgresql": map[string]any{ - "use_pg_rewind": "overridden", - "use_slots": "input", + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "use_pg_rewind": "overridden", + "use_slots": "input", + }, + }, }, }, expected: map[string]any{ @@ -327,9 +336,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: wrong-type is ignored", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": true, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": true, + }, + }, }, }, expected: map[string]any{ @@ -345,11 +358,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: input passes through", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "something": "str", - "another": 5, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "something": "str", + "another": int64(5), + }, + }, }, }, }, @@ -359,7 +376,7 @@ func TestDynamicConfiguration(t *testing.T) { "postgresql": map[string]any{ "parameters": map[string]any{ "something": "str", - "another": 5, + "another": int64(5), }, "pg_hba": []string{}, "use_pg_rewind": true, @@ -369,11 +386,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: input overrides default", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "something": "str", - "another": 5, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "something": "str", + "another": int64(5), + }, + }, }, }, }, @@ -389,7 +410,7 @@ func TestDynamicConfiguration(t *testing.T) { "postgresql": map[string]any{ "parameters": map[string]any{ "something": "str", - "another": 5, + "another": int64(5), "unrelated": "default", }, "pg_hba": []string{}, @@ -400,11 +421,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: mandatory overrides input", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "something": "str", - "another": 5, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "something": "str", + "another": int64(5), + }, + }, }, }, }, @@ -420,7 +445,7 @@ func TestDynamicConfiguration(t *testing.T) { "postgresql": map[string]any{ "parameters": map[string]any{ "something": "overrides", - "another": 5, + "another": int64(5), "unrelated": "setting", }, "pg_hba": []string{}, @@ -431,10 +456,14 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: mandatory shared_preload_libraries", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "shared_preload_libraries": "given", + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "shared_preload_libraries": "given", + }, + }, }, }, }, @@ -458,10 +487,14 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: mandatory shared_preload_libraries wrong-type is ignored", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "shared_preload_libraries": 1, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "shared_preload_libraries": int64(1), + }, + }, }, }, }, @@ -485,10 +518,14 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: shared_preload_libraries order", - input: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "shared_preload_libraries": "given, citus, more", + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "shared_preload_libraries": "given, citus, more", + }, + }, }, }, }, @@ -512,9 +549,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: wrong-type is ignored", - input: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": true, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "pg_hba": true, + }, + }, }, }, expected: map[string]any{ @@ -530,9 +571,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: default when no input", - input: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": nil, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "pg_hba": nil, + }, + }, }, }, hbas: postgres.HBAs{ @@ -555,9 +600,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: no default when input", - input: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": []any{"custom"}, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "pg_hba": []any{"custom"}, + }, + }, }, }, hbas: postgres.HBAs{ @@ -580,9 +629,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: mandatory before others", - input: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": []any{"custom"}, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "pg_hba": []any{"custom"}, + }, + }, }, }, hbas: postgres.HBAs{ @@ -606,9 +659,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: ignore non-string types", - input: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": []any{1, true, "custom", map[string]string{}, []string{}}, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "pg_hba": []any{int64(1), true, "custom", map[string]any{}, []any{}}, + }, + }, }, }, hbas: postgres.HBAs{ @@ -632,9 +689,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: input passes through", - input: map[string]any{ - "standby_cluster": map[string]any{ - "primary_slot_name": "str", + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "standby_cluster": map[string]any{ + "primary_slot_name": "str", + }, + }, }, }, expected: map[string]any{ @@ -653,18 +714,18 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: repo only", - cluster: &v1beta1.PostgresCluster{ - Spec: v1beta1.PostgresClusterSpec{ - Standby: &v1beta1.PostgresStandbySpec{ - Enabled: true, - RepoName: "repo", - }, + spec: v1beta1.PostgresClusterSpec{ + Standby: &v1beta1.PostgresStandbySpec{ + Enabled: true, + RepoName: "repo", }, - }, - input: map[string]any{ - "standby_cluster": map[string]any{ - "restore_command": "overridden", - "unrelated": "input", + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "standby_cluster": map[string]any{ + "restore_command": "overridden", + "unrelated": "input", + }, + }, }, }, params: postgres.Parameters{ @@ -692,21 +753,21 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: basebackup for streaming", - cluster: &v1beta1.PostgresCluster{ - Spec: v1beta1.PostgresClusterSpec{ - Standby: &v1beta1.PostgresStandbySpec{ - Enabled: true, - Host: "0.0.0.0", - Port: initialize.Int32(5432), - }, + spec: v1beta1.PostgresClusterSpec{ + Standby: &v1beta1.PostgresStandbySpec{ + Enabled: true, + Host: "0.0.0.0", + Port: initialize.Int32(5432), }, - }, - input: map[string]any{ - "standby_cluster": map[string]any{ - "host": "overridden", - "port": int32(0000), - "restore_command": "overridden", - "unrelated": "input", + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "standby_cluster": map[string]any{ + "host": "overridden", + "port": int64(0000), + "restore_command": "overridden", + "unrelated": "input", + }, + }, }, }, params: postgres.Parameters{ @@ -735,22 +796,22 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: both repo and streaming", - cluster: &v1beta1.PostgresCluster{ - Spec: v1beta1.PostgresClusterSpec{ - Standby: &v1beta1.PostgresStandbySpec{ - Enabled: true, - Host: "0.0.0.0", - Port: initialize.Int32(5432), - RepoName: "repo", - }, + spec: v1beta1.PostgresClusterSpec{ + Standby: &v1beta1.PostgresStandbySpec{ + Enabled: true, + Host: "0.0.0.0", + Port: initialize.Int32(5432), + RepoName: "repo", }, - }, - input: map[string]any{ - "standby_cluster": map[string]any{ - "host": "overridden", - "port": int32(9999), - "restore_command": "overridden", - "unrelated": "input", + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "standby_cluster": map[string]any{ + "host": "overridden", + "port": int64(9999), + "restore_command": "overridden", + "unrelated": "input", + }, + }, }, }, params: postgres.Parameters{ @@ -780,14 +841,12 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "tde enabled", - cluster: &v1beta1.PostgresCluster{ - Spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "encryption_key_command": "echo test", - }, + spec: v1beta1.PostgresClusterSpec{ + Patroni: &v1beta1.PatroniSpec{ + DynamicConfiguration: map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "encryption_key_command": "echo test", }, }, }, @@ -797,8 +856,10 @@ func TestDynamicConfiguration(t *testing.T) { "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{}, + "bin_name": map[string]any{"pg_rewind": string("/tmp/pg_rewind_tde.sh")}, + "parameters": map[string]any{ + "encryption_key_command": "echo test", + }, "pg_hba": []string{}, "use_pg_rewind": bool(true), "use_slots": bool(false), @@ -807,15 +868,12 @@ func TestDynamicConfiguration(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - cluster := tt.cluster - if cluster == nil { - cluster = new(v1beta1.PostgresCluster) - } + cluster := &v1beta1.PostgresCluster{Spec: tt.spec} if cluster.Spec.PostgresVersion == 0 { cluster.Spec.PostgresVersion = 14 } cluster.Default() - actual := DynamicConfiguration(cluster, tt.input, tt.hbas, tt.params) + actual := DynamicConfiguration(&cluster.Spec, tt.hbas, tt.params) assert.DeepEqual(t, tt.expected, actual) }) } From dbec3943ea2be1787c4f48b9d7d982a1b577b07c Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Thu, 19 Dec 2024 11:41:43 -0600 Subject: [PATCH 2/2] FIXUP: unmarshal spec --- internal/patroni/config_test.go | 326 ++++++++++++++++---------------- 1 file changed, 163 insertions(+), 163 deletions(-) diff --git a/internal/patroni/config_test.go b/internal/patroni/config_test.go index 3470392bae..eb8b12918f 100644 --- a/internal/patroni/config_test.go +++ b/internal/patroni/config_test.go @@ -17,7 +17,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/yaml" - "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/internal/testing/cmp" "github.com/crunchydata/postgres-operator/internal/testing/require" @@ -229,7 +228,7 @@ func TestDynamicConfiguration(t *testing.T) { for _, tt := range []struct { name string - spec v1beta1.PostgresClusterSpec + spec string hbas postgres.HBAs params postgres.Parameters expected map[string]any @@ -249,17 +248,17 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "top-level passes through", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "retry_timeout": int64(5), + spec: `{ + patroni: { + dynamicConfiguration: { + retry_timeout: 5, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), - "retry_timeout": int64(5), + "retry_timeout": float64(5), "postgresql": map[string]any{ "parameters": map[string]any{}, "pg_hba": []string{}, @@ -270,16 +269,16 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "top-level: spec overrides input", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - LeaderLeaseDurationSeconds: initialize.Int32(99), - SyncPeriodSeconds: initialize.Int32(8), - DynamicConfiguration: map[string]any{ - "loop_wait": int64(3), - "ttl": "nope", + spec: `{ + patroni: { + leaderLeaseDurationSeconds: 99, + syncPeriodSeconds: 8, + dynamicConfiguration: { + loop_wait: 3, + ttl: nope, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(8), "ttl": int32(99), @@ -293,13 +292,13 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql: wrong-type is ignored", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": true, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: true, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), @@ -313,16 +312,16 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql: defaults and overrides", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "use_pg_rewind": "overridden", - "use_slots": "input", + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + use_pg_rewind: overidden, + use_slots: input, }, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), @@ -336,15 +335,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: wrong-type is ignored", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": true, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: true, }, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), @@ -358,25 +357,25 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: input passes through", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "something": "str", - "another": int64(5), + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + something: str, + another: 5, }, }, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), "postgresql": map[string]any{ "parameters": map[string]any{ "something": "str", - "another": int64(5), + "another": float64(5), }, "pg_hba": []string{}, "use_pg_rewind": true, @@ -386,18 +385,18 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: input overrides default", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "something": "str", - "another": int64(5), + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + something: str, + another: 5, }, }, }, }, - }, + }`, params: postgres.Parameters{ Default: parameters(map[string]string{ "something": "overridden", @@ -410,7 +409,7 @@ func TestDynamicConfiguration(t *testing.T) { "postgresql": map[string]any{ "parameters": map[string]any{ "something": "str", - "another": int64(5), + "another": float64(5), "unrelated": "default", }, "pg_hba": []string{}, @@ -421,18 +420,18 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: mandatory overrides input", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "something": "str", - "another": int64(5), + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + something: str, + another: 5, }, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "something": "overrides", @@ -445,7 +444,7 @@ func TestDynamicConfiguration(t *testing.T) { "postgresql": map[string]any{ "parameters": map[string]any{ "something": "overrides", - "another": int64(5), + "another": float64(5), "unrelated": "setting", }, "pg_hba": []string{}, @@ -456,17 +455,17 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: mandatory shared_preload_libraries", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "shared_preload_libraries": "given", + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + shared_preload_libraries: given, }, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "shared_preload_libraries": "mandatory", @@ -487,17 +486,17 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: mandatory shared_preload_libraries wrong-type is ignored", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "shared_preload_libraries": int64(1), + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + shared_preload_libraries: 1, }, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "shared_preload_libraries": "mandatory", @@ -518,17 +517,17 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.parameters: shared_preload_libraries order", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "shared_preload_libraries": "given, citus, more", + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + shared_preload_libraries: "given, citus, more", }, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "shared_preload_libraries": "mandatory", @@ -549,15 +548,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: wrong-type is ignored", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": true, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + pg_hba: true, }, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), @@ -571,15 +570,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: default when no input", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": nil, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + pg_hba: null, }, }, }, - }, + }`, hbas: postgres.HBAs{ Default: []*postgres.HostBasedAuthentication{ postgres.NewHBA().Local().Method("peer"), @@ -600,15 +599,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: no default when input", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": []any{"custom"}, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + pg_hba: [custom], }, }, }, - }, + }`, hbas: postgres.HBAs{ Default: []*postgres.HostBasedAuthentication{ postgres.NewHBA().Local().Method("peer"), @@ -629,15 +628,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: mandatory before others", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": []any{"custom"}, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + pg_hba: [custom], }, }, }, - }, + }`, hbas: postgres.HBAs{ Mandatory: []*postgres.HostBasedAuthentication{ postgres.NewHBA().Local().Method("peer"), @@ -659,15 +658,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "postgresql.pg_hba: ignore non-string types", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "pg_hba": []any{int64(1), true, "custom", map[string]any{}, []any{}}, + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + pg_hba: [1, true, custom, {}, []], }, }, }, - }, + }`, hbas: postgres.HBAs{ Mandatory: []*postgres.HostBasedAuthentication{ postgres.NewHBA().Local().Method("peer"), @@ -689,15 +688,15 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: input passes through", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "standby_cluster": map[string]any{ - "primary_slot_name": "str", + spec: `{ + patroni: { + dynamicConfiguration: { + standby_cluster: { + primary_slot_name: str, }, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), @@ -714,20 +713,20 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: repo only", - spec: v1beta1.PostgresClusterSpec{ - Standby: &v1beta1.PostgresStandbySpec{ - Enabled: true, - RepoName: "repo", + spec: `{ + standby: { + enabled: true, + repoName: repo, }, - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "standby_cluster": map[string]any{ - "restore_command": "overridden", - "unrelated": "input", + patroni: { + dynamicConfiguration: { + standby_cluster: { + restore_command: overridden, + unrelated: input, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "restore_command": "mandatory", @@ -753,23 +752,23 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: basebackup for streaming", - spec: v1beta1.PostgresClusterSpec{ - Standby: &v1beta1.PostgresStandbySpec{ - Enabled: true, - Host: "0.0.0.0", - Port: initialize.Int32(5432), + spec: `{ + standby: { + enabled: true, + host: 0.0.0.0, + port: 5432, }, - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "standby_cluster": map[string]any{ - "host": "overridden", - "port": int64(0000), - "restore_command": "overridden", - "unrelated": "input", + patroni: { + dynamicConfiguration: { + standby_cluster: { + host: overridden, + port: 0000, + restore_command: overridden, + unrelated: input, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "restore_command": "mandatory", @@ -796,24 +795,24 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "standby_cluster: both repo and streaming", - spec: v1beta1.PostgresClusterSpec{ - Standby: &v1beta1.PostgresStandbySpec{ - Enabled: true, - Host: "0.0.0.0", - Port: initialize.Int32(5432), - RepoName: "repo", + spec: `{ + standby: { + enabled: true, + host: 0.0.0.0, + port: 5432, + repoName: repo, }, - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "standby_cluster": map[string]any{ - "host": "overridden", - "port": int64(9999), - "restore_command": "overridden", - "unrelated": "input", + patroni: { + dynamicConfiguration: { + standby_cluster: { + host: overridden, + port: 9999, + restore_command: overridden, + unrelated: input, }, }, }, - }, + }`, params: postgres.Parameters{ Mandatory: parameters(map[string]string{ "restore_command": "mandatory", @@ -841,17 +840,17 @@ func TestDynamicConfiguration(t *testing.T) { }, { name: "tde enabled", - spec: v1beta1.PostgresClusterSpec{ - Patroni: &v1beta1.PatroniSpec{ - DynamicConfiguration: map[string]any{ - "postgresql": map[string]any{ - "parameters": map[string]any{ - "encryption_key_command": "echo test", + spec: `{ + patroni: { + dynamicConfiguration: { + postgresql: { + parameters: { + encryption_key_command: echo test, }, }, }, }, - }, + }`, expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), @@ -868,7 +867,8 @@ func TestDynamicConfiguration(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - cluster := &v1beta1.PostgresCluster{Spec: tt.spec} + cluster := new(v1beta1.PostgresCluster) + assert.NilError(t, yaml.Unmarshal([]byte(tt.spec), &cluster.Spec)) if cluster.Spec.PostgresVersion == 0 { cluster.Spec.PostgresVersion = 14 }