Add a validated field for Postgres HBA rules

These structured fields are easier and safer to use than raw HBA records.
The validation rules of Kubernetes 1.29 (Beta in 1.25) allow for this
kind of structure.

Co-authored-by: TJ Moore <tj.moore@crunchydata.com>
Issue: PGO-2263

Author: cbandy

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

@@ -39,6 +39,86 @@ spec:
           spec:
             description: PostgresClusterSpec defines the desired state of PostgresCluster
             properties:
+              authentication:
+                properties:
+                  rules:
+                    description: 'More info: https://www.postgresql.org/docs/current/auth-pg-hba-conf.html'
+                    items:
+                      properties:
+                        connection:
+                          description: |-
+                            The connection transport this rule matches. Typical values are:
+                             1. "host" for network connections that may or may not be encrypted.
+                             2. "hostssl" for network connections encrypted using TLS.
+                             3. "hostgssenc" for network connections encrypted using GSSAPI.
+                          maxLength: 20
+                          minLength: 1
+                          pattern: ^[-a-z0-9]+$
+                          type: string
+                        databases:
+                          description: Which databases this rule matches. When omitted
+                            or empty, this rule matches all databases.
+                          items:
+                            maxLength: 63
+                            minLength: 1
+                            type: string
+                          maxItems: 20
+                          type: array
+                          x-kubernetes-list-type: atomic
+                        hba:
+                          description: One line of the "pg_hba.conf" file. Changes
+                            to this value will be automatically reloaded without validation.
+                          maxLength: 100
+                          minLength: 1
+                          pattern: ^[[:print:]]+$
+                          type: string
+                          x-kubernetes-validations:
+                          - message: cannot include other files
+                            rule: '!self.trim().startsWith("include")'
+                        method:
+                          description: |-
+                            The authentication method to use when a connection matches this rule.
+                            The special value "reject" refuses connections that match this rule.
+                            More info: https://www.postgresql.org/docs/current/auth-methods.html
+                          maxLength: 20
+                          minLength: 1
+                          pattern: ^[-a-z0-9]+$
+                          type: string
+                          x-kubernetes-validations:
+                          - message: the "trust" method is unsafe
+                            rule: self != "trust"
+                        options:
+                          additionalProperties:
+                            anyOf:
+                            - type: integer
+                            - type: string
+                            x-kubernetes-int-or-string: true
+                          maxProperties: 20
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        users:
+                          description: Which user names this rule matches. When omitted
+                            or empty, this rule matches all users.
+                          items:
+                            maxLength: 63
+                            minLength: 1
+                            type: string
+                          maxItems: 20
+                          type: array
+                          x-kubernetes-list-type: atomic
+                      type: object
+                      x-kubernetes-map-type: atomic
+                      x-kubernetes-validations:
+                      - message: '"hba" cannot be combined with other fields'
+                        rule: 'has(self.hba) ? !has(self.connection) && !has(self.databases)
+                          && !has(self.method) && !has(self.options) && !has(self.users)
+                          : true'
+                      - message: '"connection" and "method" are required'
+                        rule: 'has(self.hba) ? true : has(self.connection) && has(self.method)'
+                    maxItems: 10
+                    type: array
+                    x-kubernetes-list-type: atomic
+                type: object
               backups:
                 description: PostgreSQL backup configuration
                 properties:

internal/controller/postgrescluster/postgres.go

@@ -42,12 +42,40 @@ import (
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
+// generatePostgresHBA converts one API rule into a structured HBA rule that
+// safely formats its values.
+func (*Reconciler) generatePostgresHBA(spec *v1beta1.PostgresHBARule) *postgres.HostBasedAuthentication {
+	if spec == nil {
+		return nil
+	}
+
+	result := postgres.NewHBA()
+	result.Origin(spec.Connection)
+	result.Method(spec.Method)
+
+	if len(spec.Databases) > 0 {
+		result.Databases(spec.Databases[0], spec.Databases[1:]...)
+	}
+	if len(spec.Users) > 0 {
+		result.Users(spec.Users[0], spec.Users[1:]...)
+	}
+	if len(spec.Options) > 0 {
+		opts := make(map[string]string, len(spec.Options))
+		for k, v := range spec.Options {
+			opts[k] = v.String()
+		}
+		result.Options(opts)
+	}
+
+	return result
+}
+
 // 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(
+func (r *Reconciler) generatePostgresHBAs(
 	ctx context.Context, cluster *v1beta1.PostgresCluster,
 ) *postgres.OrderedHBAs {
 	builtin := postgres.NewHBAs()
@@ -58,13 +86,25 @@ func (*Reconciler) generatePostgresHBAs(
 	// so connections are matched against them first.
 	result := new(postgres.OrderedHBAs)
 	result.Append(builtin.Mandatory...)
+	mandatory := result.Length()
+
+	// Append any rules specified in the Authentication section.
+	// These take precedence over any in the Patroni section.
+	if authn := cluster.Spec.Authentication; authn != nil {
+		for _, in := range authn.Rules {
+			if len(in.HBA) > 0 {
+				result.AppendUnstructured(in.HBA)
+			} else {
+				result.Append(r.generatePostgresHBA(&in.PostgresHBARule))
+			}
+		}
+	}
 
 	// 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 {
+	if result.Length() == mandatory {
 		result.Append(builtin.Default...)
 	}
 

internal/controller/postgrescluster/postgres_test.go

@@ -34,6 +34,40 @@ import (
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
+func TestGeneratePostgresHBA(t *testing.T) {
+	reconciler := &Reconciler{}
+
+	assert.Assert(t, reconciler.generatePostgresHBA(nil) == nil,
+		"expected nil to return nil")
+
+	for _, tt := range []struct {
+		rule, expected string
+	}{
+		{
+			rule:     `{ connection: host, method: scram }`,
+			expected: `"host" all all all "scram"`,
+		},
+		{
+			rule:     `{ connection: local, method: peer, databases: [one, two] }`,
+			expected: `"local" "one","two" all all "peer"`,
+		},
+		{
+			rule:     `{ connection: local, method: peer, users: [alice, bob] }`,
+			expected: `"local" all "alice","bob" all "peer"`,
+		},
+		{
+			rule:     `{ connection: hostssl, method: md5, options: { clientcert: verify-ca } }`,
+			expected: `"hostssl" all all all "md5"  "clientcert"="verify-ca"`,
+		},
+	} {
+		var rule *v1beta1.PostgresHBARule
+		require.UnmarshalInto(t, &rule, tt.rule)
+
+		hba := reconciler.generatePostgresHBA(rule)
+		assert.Equal(t, hba.String(), tt.expected, "\n%#v", rule)
+	}
+}
+
 func TestGeneratePostgresHBAs(t *testing.T) {
 	ctx := context.Background()
 	reconciler := &Reconciler{}
@@ -50,12 +84,35 @@ func TestGeneratePostgresHBAs(t *testing.T) {
 	assert.Assert(t, len(required) > 0,
 		"expected at least one mandatory rule")
 
+	t.Run("Authentication", func(t *testing.T) {
+		cluster := v1beta1.NewPostgresCluster()
+		require.UnmarshalInto(t, &cluster.Spec.Authentication, `{
+			rules: [
+				{ connection: host, method: scram },
+				{ connection: local, method: peer, users: [alice, bob] },
+			],
+		}`)
+
+		result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings()
+		assert.Assert(t, cmp.Len(result, len(required)+2),
+			"expected two rules from the Authentication 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{
+			`"host" all all all "scram"`,
+			`"local" all "alice","bob" all "peer"`,
+		})
+	})
+
 	t.Run("Patroni", func(t *testing.T) {
 		cluster := v1beta1.NewPostgresCluster()
 		require.UnmarshalInto(t, &cluster.Spec.Patroni, `{
-				dynamicConfiguration: {
-						postgresql: { pg_hba: [ "first custom", "another" ] },
-				},
+			dynamicConfiguration: {
+				postgresql: { pg_hba: [ "first custom", "another" ] },
+			},
 		}`)
 
 		result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings()
@@ -68,6 +125,36 @@ func TestGeneratePostgresHBAs(t *testing.T) {
 		// specified rules should be last and in their original order
 		assert.DeepEqual(t, result[len(required):], []string{`first custom`, `another`})
 	})
+
+	t.Run("Precedence", func(t *testing.T) {
+		cluster := v1beta1.NewPostgresCluster()
+		require.UnmarshalInto(t, &cluster.Spec.Authentication, `{
+			rules: [
+				{ connection: host, method: scram },
+				{ connection: local, method: peer, users: [alice, bob] },
+			],
+		}`)
+		require.UnmarshalInto(t, &cluster.Spec.Patroni, `{
+			dynamicConfiguration: {
+				postgresql: { pg_hba: [ "another" ] },
+			},
+		}`)
+
+		result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings()
+		assert.Assert(t, cmp.Len(result, len(required)+2+1),
+			"expected two rules from the Authentication section"+
+				" plus one from the Patroni section")
+
+		// mandatory rules should be first
+		assert.DeepEqual(t, result[:len(required)], required)
+
+		// specified rules are next, no defaults
+		assert.DeepEqual(t, result[len(required):], []string{
+			`"host" all all all "scram"`,           // Authentication
+			`"local" all "alice","bob" all "peer"`, // Authentication
+			`another`,                              // Patroni
+		})
+	})
 }
 
 func TestGeneratePostgresParameters(t *testing.T) {

internal/pgbouncer/postgres.go

@@ -225,7 +225,7 @@ func postgresqlHBAs() []*postgres.HostBasedAuthentication {
 	// - https://www.postgresql.org/docs/current/auth-password.html
 
 	return []*postgres.HostBasedAuthentication{
-		postgres.NewHBA().User(PostgresqlUser).TLS().Method("scram-sha-256"),
-		postgres.NewHBA().User(PostgresqlUser).TCP().Method("reject"),
+		postgres.NewHBA().Users(PostgresqlUser).TLS().Method("scram-sha-256"),
+		postgres.NewHBA().Users(PostgresqlUser).TCP().Method("reject"),
 	}
 }

internal/pgbouncer/postgres_test.go

@@ -186,6 +186,6 @@ COMMIT;`))
 func TestPostgreSQLHBAs(t *testing.T) {
 	rules := postgresqlHBAs()
 	assert.Equal(t, len(rules), 2)
-	assert.Equal(t, rules[0].String(), `hostssl all "_crunchypgbouncer" all scram-sha-256`)
-	assert.Equal(t, rules[1].String(), `host all "_crunchypgbouncer" all reject`)
+	assert.Equal(t, rules[0].String(), `hostssl all "_crunchypgbouncer" all "scram-sha-256"`)
+	assert.Equal(t, rules[1].String(), `host all "_crunchypgbouncer" all "reject"`)
 }

internal/pgmonitor/postgres.go

@@ -27,9 +27,9 @@ func PostgreSQLHBAs(ctx context.Context, inCluster *v1beta1.PostgresCluster, out
 	if ExporterEnabled(ctx, inCluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) {
 		// Limit the monitoring user to local connections using SCRAM.
 		outHBAs.Mandatory = append(outHBAs.Mandatory,
-			postgres.NewHBA().TCP().User(MonitoringUser).Method("scram-sha-256").Network("127.0.0.0/8"),
-			postgres.NewHBA().TCP().User(MonitoringUser).Method("scram-sha-256").Network("::1/128"),
-			postgres.NewHBA().TCP().User(MonitoringUser).Method("reject"))
+			postgres.NewHBA().TCP().Users(MonitoringUser).Method("scram-sha-256").Network("127.0.0.0/8"),
+			postgres.NewHBA().TCP().Users(MonitoringUser).Method("scram-sha-256").Network("::1/128"),
+			postgres.NewHBA().TCP().Users(MonitoringUser).Method("reject"))
 	}
 }
 

internal/pgmonitor/postgres_test.go

@@ -39,9 +39,9 @@ func TestPostgreSQLHBA(t *testing.T) {
 		PostgreSQLHBAs(ctx, inCluster, &outHBAs)
 
 		assert.Equal(t, len(outHBAs.Mandatory), 3)
-		assert.Equal(t, outHBAs.Mandatory[0].String(), `host all "ccp_monitoring" "127.0.0.0/8" scram-sha-256`)
-		assert.Equal(t, outHBAs.Mandatory[1].String(), `host all "ccp_monitoring" "::1/128" scram-sha-256`)
-		assert.Equal(t, outHBAs.Mandatory[2].String(), `host all "ccp_monitoring" all reject`)
+		assert.Equal(t, outHBAs.Mandatory[0].String(), `host all "ccp_monitoring" "127.0.0.0/8" "scram-sha-256"`)
+		assert.Equal(t, outHBAs.Mandatory[1].String(), `host all "ccp_monitoring" "::1/128" "scram-sha-256"`)
+		assert.Equal(t, outHBAs.Mandatory[2].String(), `host all "ccp_monitoring" all "reject"`)
 	})
 }