diff --git a/internal/controller/postgrescluster/cluster.go b/internal/controller/postgrescluster/cluster.go index 4cd62f60c8..ead4881b1e 100644 --- a/internal/controller/postgrescluster/cluster.go +++ b/internal/controller/postgrescluster/cluster.go @@ -30,7 +30,7 @@ import ( // files (etc) that apply to the entire cluster. func (r *Reconciler) reconcileClusterConfigMap( ctx context.Context, cluster *v1beta1.PostgresCluster, - pgHBAs postgres.HBAs, pgParameters *postgres.ParameterSet, + pgHBAs *postgres.OrderedHBAs, pgParameters *postgres.ParameterSet, ) (*corev1.ConfigMap, error) { clusterConfigMap := &corev1.ConfigMap{ObjectMeta: naming.ClusterConfigMap(cluster)} clusterConfigMap.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) diff --git a/internal/controller/postgrescluster/controller.go b/internal/controller/postgrescluster/controller.go index 4de285e559..bbe141c0b4 100644 --- a/internal/controller/postgrescluster/controller.go +++ b/internal/controller/postgrescluster/controller.go @@ -33,8 +33,6 @@ import ( "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/kubernetes" "github.com/crunchydata/postgres-operator/internal/logging" - "github.com/crunchydata/postgres-operator/internal/pgbouncer" - "github.com/crunchydata/postgres-operator/internal/pgmonitor" "github.com/crunchydata/postgres-operator/internal/pki" "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/internal/registration" @@ -231,10 +229,7 @@ func (r *Reconciler) Reconcile( } } - pgHBAs := postgres.NewHBAs() - pgmonitor.PostgreSQLHBAs(ctx, cluster, &pgHBAs) - pgbouncer.PostgreSQL(cluster, &pgHBAs) - + pgHBAs := r.generatePostgresHBAs(ctx, cluster) pgParameters := r.generatePostgresParameters(ctx, cluster, backupsSpecFound) otelConfig := collector.NewConfigForPostgresPod(ctx, cluster, pgParameters) diff --git a/internal/controller/postgrescluster/patroni.go b/internal/controller/postgrescluster/patroni.go index 5242169be6..af3a3b8cca 100644 --- a/internal/controller/postgrescluster/patroni.go +++ b/internal/controller/postgrescluster/patroni.go @@ -173,7 +173,7 @@ func (r *Reconciler) reconcilePatroniDistributedConfiguration( func (r *Reconciler) reconcilePatroniDynamicConfiguration( ctx context.Context, cluster *v1beta1.PostgresCluster, instances *observedInstances, - pgHBAs postgres.HBAs, pgParameters *postgres.ParameterSet, + pgHBAs *postgres.OrderedHBAs, pgParameters *postgres.ParameterSet, ) error { if !patroni.ClusterBootstrapped(cluster) { // Patroni has not yet bootstrapped. Dynamic configuration happens through diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index 25ffeefc99..a2f6fb8611 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -33,6 +33,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/patroni" "github.com/crunchydata/postgres-operator/internal/pgaudit" "github.com/crunchydata/postgres-operator/internal/pgbackrest" + "github.com/crunchydata/postgres-operator/internal/pgbouncer" "github.com/crunchydata/postgres-operator/internal/pgmonitor" "github.com/crunchydata/postgres-operator/internal/postgis" "github.com/crunchydata/postgres-operator/internal/postgres" @@ -41,6 +42,35 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +// generatePostgresHBAs produces the HBA rules for cluster that incorporates, +// from highest to lowest precedence: +// 1. mandatory rules determined by controllers +// 2. rules in cluster.spec.patroni.dynamicConfiguration +// 3. default rules, when none were in cluster.spec +func (*Reconciler) generatePostgresHBAs( + ctx context.Context, cluster *v1beta1.PostgresCluster, +) *postgres.OrderedHBAs { + builtin := postgres.NewHBAs() + pgmonitor.PostgreSQLHBAs(ctx, cluster, &builtin) + pgbouncer.PostgreSQL(cluster, &builtin) + + // Postgres processes HBA rules in order. Start with mandatory rules + // so connections are matched against them first. + result := new(postgres.OrderedHBAs) + result.Append(builtin.Mandatory...) + + // Append any rules specified in the Patroni section. + before := result.Length() + result.AppendUnstructured(patroni.PostgresHBAs(cluster.Spec.Patroni)...) + + // When there are no specified rules, include the recommended defaults. + if result.Length() == before { + result.Append(builtin.Default...) + } + + return result +} + // generatePostgresParameters produces the parameter set for cluster that // incorporates, from highest to lowest precedence: // 1. mandatory values determined by controllers diff --git a/internal/controller/postgrescluster/postgres_test.go b/internal/controller/postgrescluster/postgres_test.go index f6da644a09..4f967d1088 100644 --- a/internal/controller/postgrescluster/postgres_test.go +++ b/internal/controller/postgrescluster/postgres_test.go @@ -34,6 +34,42 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestGeneratePostgresHBAs(t *testing.T) { + ctx := context.Background() + reconciler := &Reconciler{} + + builtin := reconciler.generatePostgresHBAs(ctx, v1beta1.NewPostgresCluster()).AsStrings() + assert.Assert(t, len(builtin) > 0, + "expected an empty cluster to have some builtin rules") + + defaults := builtin[len(builtin)-1:] + assert.Assert(t, len(defaults) > 0, + "expected at least one default rule") + + required := builtin[:len(builtin)-len(defaults)] + assert.Assert(t, len(required) > 0, + "expected at least one mandatory rule") + + t.Run("Patroni", func(t *testing.T) { + cluster := v1beta1.NewPostgresCluster() + require.UnmarshalInto(t, &cluster.Spec.Patroni, `{ + dynamicConfiguration: { + postgresql: { pg_hba: [ "first custom", "another" ] }, + }, + }`) + + result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings() + assert.Assert(t, cmp.Len(result, len(required)+2), + "expected two rules from the Patroni section and no defaults") + + // mandatory rules should be first + assert.DeepEqual(t, result[:len(required)], required) + + // specified rules should be last and in their original order + assert.DeepEqual(t, result[len(required):], []string{`first custom`, `another`}) + }) +} + func TestGeneratePostgresParameters(t *testing.T) { ctx := context.Background() reconciler := &Reconciler{} diff --git a/internal/patroni/config.go b/internal/patroni/config.go index 2174607c63..bee17bbb94 100644 --- a/internal/patroni/config.go +++ b/internal/patroni/config.go @@ -39,7 +39,7 @@ const ( // clusterYAML returns Patroni settings that apply to the entire cluster. func clusterYAML( cluster *v1beta1.PostgresCluster, - pgHBAs postgres.HBAs, parameters *postgres.ParameterSet, patroniLogStorageLimit int64, + pgHBAs *postgres.OrderedHBAs, parameters *postgres.ParameterSet, patroniLogStorageLimit int64, ) (string, error) { root := map[string]any{ // The cluster identifier. This value cannot change during the cluster's @@ -208,7 +208,7 @@ func clusterYAML( // and returns a value that can be marshaled to JSON. func DynamicConfiguration( spec *v1beta1.PostgresClusterSpec, - pgHBAs postgres.HBAs, parameters *postgres.ParameterSet, + pgHBAs *postgres.OrderedHBAs, parameters *postgres.ParameterSet, ) map[string]any { // Copy the entire configuration before making any changes. root := make(map[string]any) @@ -239,32 +239,13 @@ func DynamicConfiguration( postgresql[k] = v } } - root["postgresql"] = postgresql - if m := parameters.AsMap(); m != nil { postgresql["parameters"] = m } - - // Copy the "postgresql.pg_hba" section after any mandatory values. - hba := make([]string, 0, len(pgHBAs.Mandatory)) - for i := range pgHBAs.Mandatory { - hba = append(hba, pgHBAs.Mandatory[i].String()) + if pgHBAs != nil { + postgresql["pg_hba"] = pgHBAs.AsStrings() } - if section, ok := postgresql["pg_hba"].([]any); ok { - for i := range section { - // any pg_hba values that are not strings will be skipped - if value, ok := section[i].(string); ok { - hba = append(hba, value) - } - } - } - // When the section is missing or empty, include the recommended defaults. - if len(hba) == len(pgHBAs.Mandatory) { - for i := range pgHBAs.Default { - hba = append(hba, pgHBAs.Default[i].String()) - } - } - postgresql["pg_hba"] = hba + root["postgresql"] = postgresql // Enabling `pg_rewind` allows a former primary to automatically rejoin the // cluster even if it has commits that were not sent to a replica. In other diff --git a/internal/patroni/config_test.go b/internal/patroni/config_test.go index d5ce0eb81d..5386454a47 100644 --- a/internal/patroni/config_test.go +++ b/internal/patroni/config_test.go @@ -32,7 +32,7 @@ func TestClusterYAML(t *testing.T) { cluster.Namespace = "some-namespace" cluster.Name = "cluster-name" - data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.NewParameterSet(), 0) + data, err := clusterYAML(cluster, nil, nil, 0) assert.NilError(t, err) assert.Equal(t, data, strings.TrimSpace(` # Generated by postgres-operator. DO NOT EDIT. @@ -41,8 +41,6 @@ bootstrap: dcs: loop_wait: 10 postgresql: - parameters: {} - pg_hba: [] use_pg_rewind: false use_slots: false ttl: 30 @@ -91,7 +89,7 @@ watchdog: cluster.Name = "cluster-name" cluster.Spec.PostgresVersion = 14 - data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.NewParameterSet(), 0) + data, err := clusterYAML(cluster, nil, nil, 0) assert.NilError(t, err) assert.Equal(t, data, strings.TrimSpace(` # Generated by postgres-operator. DO NOT EDIT. @@ -100,8 +98,6 @@ bootstrap: dcs: loop_wait: 10 postgresql: - parameters: {} - pg_hba: [] use_pg_rewind: true use_slots: false ttl: 30 @@ -159,7 +155,7 @@ watchdog: Level: &logLevel, } - data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.NewParameterSet(), 1000) + data, err := clusterYAML(cluster, nil, nil, 1000) assert.NilError(t, err) assert.Equal(t, data, strings.TrimSpace(` # Generated by postgres-operator. DO NOT EDIT. @@ -168,8 +164,6 @@ bootstrap: dcs: loop_wait: 10 postgresql: - parameters: {} - pg_hba: [] use_pg_rewind: true use_slots: false ttl: 30 @@ -230,10 +224,16 @@ func TestDynamicConfiguration(t *testing.T) { return out } + rules := func(in ...string) *postgres.OrderedHBAs { + out := new(postgres.OrderedHBAs) + out.AppendUnstructured(in...) + return out + } + for _, tt := range []struct { name string spec string - hbas postgres.HBAs + hbas *postgres.OrderedHBAs params *postgres.ParameterSet expected map[string]any }{ @@ -243,7 +243,6 @@ func TestDynamicConfiguration(t *testing.T) { "loop_wait": int32(10), "ttl": int32(30), "postgresql": map[string]any{ - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -263,7 +262,6 @@ func TestDynamicConfiguration(t *testing.T) { "ttl": int32(30), "retry_timeout": int64(5), "postgresql": map[string]any{ - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -285,7 +283,6 @@ func TestDynamicConfiguration(t *testing.T) { "loop_wait": int32(8), "ttl": int32(99), "postgresql": map[string]any{ - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -304,7 +301,6 @@ func TestDynamicConfiguration(t *testing.T) { "loop_wait": int32(10), "ttl": int32(30), "postgresql": map[string]any{ - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -326,7 +322,6 @@ func TestDynamicConfiguration(t *testing.T) { "loop_wait": int32(10), "ttl": int32(30), "postgresql": map[string]any{ - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": "input", }, @@ -359,142 +354,28 @@ func TestDynamicConfiguration(t *testing.T) { "another": "5", "unrelated": "default", }, - "pg_hba": []string{}, - "use_pg_rewind": true, - "use_slots": false, - }, - }, - }, - { - name: "postgresql.pg_hba: wrong-type is ignored", - spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - pg_hba: true, - }, - }, - }, - }`, - expected: map[string]any{ - "loop_wait": int32(10), - "ttl": int32(30), - "postgresql": map[string]any{ - "pg_hba": []string{}, - "use_pg_rewind": true, - "use_slots": false, - }, - }, - }, - { - name: "postgresql.pg_hba: default when no input", - spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - pg_hba: null, - }, - }, - }, - }`, - hbas: postgres.HBAs{ - Default: []*postgres.HostBasedAuthentication{ - postgres.NewHBA().Local().Method("peer"), - }, - }, - expected: map[string]any{ - "loop_wait": int32(10), - "ttl": int32(30), - "postgresql": map[string]any{ - "pg_hba": []string{ - "local all all peer", - }, "use_pg_rewind": true, "use_slots": false, }, }, }, { - name: "postgresql.pg_hba: no default when input", + name: "HBA pass through", spec: `{ patroni: { dynamicConfiguration: { postgresql: { - pg_hba: [custom], + pg_hba: [calculated, elsewhere], }, }, }, }`, - hbas: postgres.HBAs{ - Default: []*postgres.HostBasedAuthentication{ - postgres.NewHBA().Local().Method("peer"), - }, - }, + hbas: rules("function args"), expected: map[string]any{ "loop_wait": int32(10), "ttl": int32(30), "postgresql": map[string]any{ - "pg_hba": []string{ - "custom", - }, - "use_pg_rewind": true, - "use_slots": false, - }, - }, - }, - { - name: "postgresql.pg_hba: mandatory before others", - spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - pg_hba: [custom], - }, - }, - }, - }`, - hbas: postgres.HBAs{ - Mandatory: []*postgres.HostBasedAuthentication{ - postgres.NewHBA().Local().Method("peer"), - }, - }, - expected: map[string]any{ - "loop_wait": int32(10), - "ttl": int32(30), - "postgresql": map[string]any{ - "pg_hba": []string{ - "local all all peer", - "custom", - }, - "use_pg_rewind": true, - "use_slots": false, - }, - }, - }, - { - name: "postgresql.pg_hba: ignore non-string types", - spec: `{ - patroni: { - dynamicConfiguration: { - postgresql: { - pg_hba: [1, true, custom, {}, []], - }, - }, - }, - }`, - hbas: postgres.HBAs{ - Mandatory: []*postgres.HostBasedAuthentication{ - postgres.NewHBA().Local().Method("peer"), - }, - }, - expected: map[string]any{ - "loop_wait": int32(10), - "ttl": int32(30), - "postgresql": map[string]any{ - "pg_hba": []string{ - "local all all peer", - "custom", - }, + "pg_hba": []string{"function args"}, "use_pg_rewind": true, "use_slots": false, }, @@ -515,7 +396,6 @@ func TestDynamicConfiguration(t *testing.T) { "loop_wait": int32(10), "ttl": int32(30), "postgresql": map[string]any{ - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -550,7 +430,6 @@ func TestDynamicConfiguration(t *testing.T) { "parameters": map[string]string{ "restore_command": "mandatory", }, - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -590,7 +469,6 @@ func TestDynamicConfiguration(t *testing.T) { "parameters": map[string]string{ "restore_command": "mandatory", }, - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -632,7 +510,6 @@ func TestDynamicConfiguration(t *testing.T) { "parameters": map[string]string{ "restore_command": "mandatory", }, - "pg_hba": []string{}, "use_pg_rewind": true, "use_slots": false, }, @@ -665,7 +542,6 @@ func TestDynamicConfiguration(t *testing.T) { "parameters": map[string]string{ "encryption_key_command": "echo one", }, - "pg_hba": []string{}, "use_pg_rewind": bool(true), "use_slots": bool(false), }, diff --git a/internal/patroni/postgres.go b/internal/patroni/postgres.go index cb686312fa..519fc30c04 100644 --- a/internal/patroni/postgres.go +++ b/internal/patroni/postgres.go @@ -12,6 +12,33 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +// PostgresHBAs returns the HBA rules in spec, if any. +func PostgresHBAs(spec *v1beta1.PatroniSpec) []string { + var result []string + + if spec != nil { + // DynamicConfiguration lacks an OpenAPI schema, so it may contain any type + // at any depth. Navigate the object and skip HBA values that aren't string. + // + // Patroni expects a list of strings: + // https://github.com/patroni/patroni/blob/v4.0.0/patroni/validator.py#L1170 + // + if root := spec.DynamicConfiguration; root != nil { + if postgresql, ok := root["postgresql"].(map[string]any); ok { + if section, ok := postgresql["pg_hba"].([]any); ok { + for i := range section { + if value, ok := section[i].(string); ok { + result = append(result, value) + } + } + } + } + } + } + + return result +} + // PostgresParameters returns the Postgres parameters in spec, if any. func PostgresParameters(spec *v1beta1.PatroniSpec) *postgres.ParameterSet { result := postgres.NewParameterSet() diff --git a/internal/patroni/postgres_test.go b/internal/patroni/postgres_test.go index 16fdc30fdf..becd1b1743 100644 --- a/internal/patroni/postgres_test.go +++ b/internal/patroni/postgres_test.go @@ -13,6 +13,91 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestPostgresHBAs(t *testing.T) { + t.Run("Zero", func(t *testing.T) { + result := PostgresHBAs(nil) + + assert.Assert(t, result == nil) + }) + + t.Run("NoDynamicConfig", func(t *testing.T) { + spec := new(v1beta1.PatroniSpec) + result := PostgresHBAs(spec) + + assert.Assert(t, result == nil) + }) + + t.Run("NoPostgreSQL", func(t *testing.T) { + spec := new(v1beta1.PatroniSpec) + require.UnmarshalInto(t, spec, `{ + dynamicConfiguration: {}, + }`) + + result := PostgresHBAs(spec) + assert.Assert(t, result == nil) + + t.Run("WrongType", func(t *testing.T) { + require.UnmarshalInto(t, spec, `{ + dynamicConfiguration: { + postgresql: asdf, + }, + }`) + + result := PostgresHBAs(spec) + assert.Assert(t, result == nil) + }) + }) + + t.Run("NoHBAs", func(t *testing.T) { + spec := new(v1beta1.PatroniSpec) + require.UnmarshalInto(t, spec, `{ + dynamicConfiguration: { + postgresql: { + use_pg_rewind: true, + }, + }, + }`) + + result := PostgresHBAs(spec) + assert.Assert(t, result == nil) + + t.Run("WrongType", func(t *testing.T) { + require.UnmarshalInto(t, spec, `{ + dynamicConfiguration: { + postgresql: { + pg_hba: asdf, + }, + }, + }`) + + result := PostgresHBAs(spec) + assert.Assert(t, result == nil) + }) + }) + + t.Run("HBAs", func(t *testing.T) { + spec := new(v1beta1.PatroniSpec) + require.UnmarshalInto(t, spec, `{ + dynamicConfiguration: { + postgresql: { + pg_hba: [ + "host all all all trust", + true, + "total garbage, yikes", + 123, + ], + }, + }, + }`) + + result := PostgresHBAs(spec) + assert.DeepEqual(t, result, []string{ + "host all all all trust", + "total garbage, yikes", + }) + }) +} + func TestPostgresParameters(t *testing.T) { t.Run("Zero", func(t *testing.T) { result := PostgresParameters(nil) diff --git a/internal/patroni/reconcile.go b/internal/patroni/reconcile.go index 394a33d6d5..a8de99f028 100644 --- a/internal/patroni/reconcile.go +++ b/internal/patroni/reconcile.go @@ -29,7 +29,7 @@ func ClusterBootstrapped(postgresCluster *v1beta1.PostgresCluster) bool { // ClusterConfigMap populates the shared ConfigMap with fields needed to run Patroni. func ClusterConfigMap(ctx context.Context, inCluster *v1beta1.PostgresCluster, - inHBAs postgres.HBAs, + inHBAs *postgres.OrderedHBAs, inParameters *postgres.ParameterSet, outClusterConfigMap *corev1.ConfigMap, patroniLogStorageLimit int64, diff --git a/internal/patroni/reconcile_test.go b/internal/patroni/reconcile_test.go index 9a82dfde2d..729bd6573d 100644 --- a/internal/patroni/reconcile_test.go +++ b/internal/patroni/reconcile_test.go @@ -14,7 +14,6 @@ import ( "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/pki" - "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/internal/testing/cmp" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -24,20 +23,18 @@ func TestClusterConfigMap(t *testing.T) { ctx := context.Background() cluster := new(v1beta1.PostgresCluster) - pgHBAs := postgres.HBAs{} - pgParameters := postgres.NewParameterSet() - cluster.Default() + config := new(corev1.ConfigMap) - assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0)) + assert.NilError(t, ClusterConfigMap(ctx, cluster, nil, nil, config, 0)) // The output of clusterYAML should go into config. - data, _ := clusterYAML(cluster, pgHBAs, pgParameters, 0) + data, _ := clusterYAML(cluster, nil, nil, 0) assert.DeepEqual(t, config.Data["patroni.yaml"], data) // No change when called again. before := config.DeepCopy() - assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0)) + assert.NilError(t, ClusterConfigMap(ctx, cluster, nil, nil, config, 0)) assert.DeepEqual(t, config, before) } diff --git a/internal/postgres/hba.go b/internal/postgres/hba.go index 3163b3307b..875a476ee2 100644 --- a/internal/postgres/hba.go +++ b/internal/postgres/hba.go @@ -6,6 +6,7 @@ package postgres import ( "fmt" + "slices" "strings" ) @@ -116,12 +117,6 @@ func (hba *HostBasedAuthentication) Replication() *HostBasedAuthentication { return hba } -// Role makes hba match connections by users that are members of a specific role. -func (hba *HostBasedAuthentication) Role(name string) *HostBasedAuthentication { - hba.user = "+" + hba.quote(name) - return hba -} - // SameNetwork makes hba match connection attempts from IP addresses in any // subnet to which the server is directly connected. func (hba *HostBasedAuthentication) SameNetwork() *HostBasedAuthentication { @@ -157,3 +152,41 @@ func (hba *HostBasedAuthentication) String() string { return strings.TrimSpace(fmt.Sprintf("%s %s %s %s %s %s", hba.origin, hba.database, hba.user, hba.address, hba.method, hba.options)) } + +// OrderedHBAs is an append-only sequence of pg_hba.conf lines. +type OrderedHBAs struct { + records []string +} + +// Append renders and adds pg_hba.conf lines to o. Nil pointers are ignored. +func (o *OrderedHBAs) Append(hbas ...*HostBasedAuthentication) { + for _, hba := range hbas { + if hba != nil { + o.records = append(o.records, hba.String()) + } + } +} + +// AppendUnstructured trims and adds unvalidated pg_hba.conf lines to o. +// Empty lines and lines that are entirely control characters are omitted. +func (o *OrderedHBAs) AppendUnstructured(hbas ...string) { + for _, hba := range hbas { + hba = strings.TrimFunc(hba, func(r rune) bool { + // control characters, space, and backslash + return r > '~' || r < '!' || r == '\\' + }) + if len(hba) > 0 { + o.records = append(o.records, hba) + } + } +} + +// AsStrings returns a copy of o as a slice. +func (o *OrderedHBAs) AsStrings() []string { + return slices.Clone(o.records) +} + +// Length returns the number of records in o. +func (o *OrderedHBAs) Length() int { + return len(o.records) +} diff --git a/internal/postgres/hba_test.go b/internal/postgres/hba_test.go index 7457b7f649..c15cf5cc77 100644 --- a/internal/postgres/hba_test.go +++ b/internal/postgres/hba_test.go @@ -52,11 +52,77 @@ func TestHostBasedAuthentication(t *testing.T) { User("KD6-3.7").Method("scram-sha-256"). String()) - assert.Equal(t, `hostssl "data" +"admin" all md5 clientcert="verify-ca"`, - NewHBA().TLS().Database("data").Role("admin"). + assert.Equal(t, `hostssl "data" all all md5 clientcert="verify-ca"`, + NewHBA().TLS().Database("data"). Method("md5").Options(map[string]string{"clientcert": "verify-ca"}). String()) assert.Equal(t, `hostnossl all all all reject`, NewHBA().NoSSL().Method("reject").String()) } + +func TestOrderedHBAs(t *testing.T) { + ordered := new(OrderedHBAs) + + // The zero value is empty. + assert.Equal(t, ordered.Length(), 0) + assert.Assert(t, cmp.Len(ordered.AsStrings(), 0)) + + // Append can be called without arguments. + ordered.Append() + ordered.AppendUnstructured() + assert.Assert(t, cmp.Len(ordered.AsStrings(), 0)) + + // Append adds to the end of the slice. + ordered.Append(NewHBA()) + assert.Equal(t, ordered.Length(), 1) + assert.DeepEqual(t, ordered.AsStrings(), []string{ + `all all all`, + }) + + // AppendUnstructured adds to the end of the slice. + ordered.AppendUnstructured("could be anything, really") + assert.Equal(t, ordered.Length(), 2) + assert.DeepEqual(t, ordered.AsStrings(), []string{ + `all all all`, + `could be anything, really`, + }) + + // Append and AppendUnstructured do not have a separate order. + ordered.Append(NewHBA().User("zoro")) + assert.Equal(t, ordered.Length(), 3) + assert.DeepEqual(t, ordered.AsStrings(), []string{ + `all all all`, + `could be anything, really`, + `all "zoro" all`, + }) + + t.Run("NilPointersIgnored", func(t *testing.T) { + rules := new(OrderedHBAs) + rules.Append( + NewHBA(), nil, + NewHBA(), nil, + ) + assert.DeepEqual(t, rules.AsStrings(), []string{ + `all all all`, + `all all all`, + }) + }) + + t.Run("SpecialCharactersStripped", func(t *testing.T) { + rules := new(OrderedHBAs) + rules.AppendUnstructured( + " \n\t things \n\n\n", + `with # comment`, + " \n\t \\\\ \f", // entirely special characters + `trailing slashes \\\`, + "multiple \\\n lines okay", + ) + assert.DeepEqual(t, rules.AsStrings(), []string{ + `things`, + `with # comment`, + `trailing slashes`, + "multiple \\\n lines okay", + }) + }) +}