From 7ede62824b61cb030f2d2947e797946bfb5975bf Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Thu, 4 Jun 2026 22:37:17 +0500 Subject: [PATCH] feat: emit user pg_hba/pg_ident entries Wire pg_hba_conf and pg_ident_conf from the instance spec into both the Swarm and common/systemd Patroni config generators, so the entries now reach the generated `pg_hba.conf` / `pg_ident.conf`. The previous change only accepted, validated, and stored them. - User entries form a zone after the CP's system-user and bridge-isolation rules and before the catch-all, so they cannot affect control-plane-internal connectivity. Node-level entries are already prepended ahead of database-level entries by `NodeInstances()`. - The Block 3 catch-all auth method now follows `password_encryption` (defaults to md5 when unset), so user passwords and the fallback stay in the same auth landscape. - Swarm: add the IPv6 (`::/0`) system-user reject the common path already had, so a permissive user rule can't reach a system user over IPv6 now that the user zone sits below the reject. - Populate `PgIdent` (previously always nil); it is purely user-supplied. - Reload is unchanged (SIGHUP); the generator just emits the entries. Adds golden tests for both generators to guard against divergence. PLAT-628 --- .../user_pg_hba_pg_ident_and_scram.yaml | 125 +++++++++++++++++ .../common/patroni_config_generator.go | 45 ++++++- .../common/patroni_config_generator_test.go | 47 +++++++ .../no_user_entries_defaults_to_md5.yaml | 122 +++++++++++++++++ .../user_pg_hba_pg_ident_and_scram.yaml | 126 ++++++++++++++++++ .../internal/orchestrator/swarm/main_test.go | 15 +++ .../orchestrator/swarm/patroni_config.go | 52 ++++++-- .../swarm/patroni_config_golden_test.go | 119 +++++++++++++++++ 8 files changed, 635 insertions(+), 16 deletions(-) create mode 100644 server/internal/orchestrator/common/golden_test/TestPatroniConfigGenerator/user_pg_hba_pg_ident_and_scram.yaml create mode 100644 server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/no_user_entries_defaults_to_md5.yaml create mode 100644 server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/user_pg_hba_pg_ident_and_scram.yaml create mode 100644 server/internal/orchestrator/swarm/main_test.go create mode 100644 server/internal/orchestrator/swarm/patroni_config_golden_test.go diff --git a/server/internal/orchestrator/common/golden_test/TestPatroniConfigGenerator/user_pg_hba_pg_ident_and_scram.yaml b/server/internal/orchestrator/common/golden_test/TestPatroniConfigGenerator/user_pg_hba_pg_ident_and_scram.yaml new file mode 100644 index 00000000..76ea7dd4 --- /dev/null +++ b/server/internal/orchestrator/common/golden_test/TestPatroniConfigGenerator/user_pg_hba_pg_ident_and_scram.yaml @@ -0,0 +1,125 @@ +name: pghba-n1-689qacsi +namespace: /patroni/ +scope: pghba:n1 +log: + type: json + level: INFO + static_fields: + database_id: pghba + instance_id: pghba-n1-689qacsi + node_name: n1 +bootstrap: + dcs: + loop_wait: 10 + ttl: 30 + retry_timeout: 10 + postgresql: + parameters: + max_connections: 901 + max_replication_slots: 16 + max_wal_senders: 16 + max_worker_processes: 12 + track_commit_timestamp: "on" + wal_level: logical + ignore_slots: + - plugin: spock_output + initdb: + - data-checksums +etcd3: + hosts: + - i-0123456789abcdef.ec2.internal:2379 + protocol: https + username: instance.pghba-n1-689qacsi + password: password + cacert: /opt/pgedge/certificates/etcd/ca.crt + cert: /opt/pgedge/certificates/etcd/client.crt + key: /opt/pgedge/certificates/etcd/client.key +postgresql: + authentication: + superuser: + username: pgedge + sslmode: verify-full + sslkey: /opt/pgedge/certificates/postgres/superuser.key + sslcert: /opt/pgedge/certificates/postgres/superuser.crt + sslrootcert: /opt/pgedge/certificates/postgres/ca.crt + replication: + username: patroni_replicator + sslmode: verify-full + sslkey: /opt/pgedge/certificates/postgres/replication.key + sslcert: /opt/pgedge/certificates/postgres/replication.crt + sslrootcert: /opt/pgedge/certificates/postgres/ca.crt + connect_address: pghba-n1-689qacsi.pghba-database:5432 + data_dir: /opt/pgedge/data/pgdata + listen: "*:5432" + parameters: + archive_command: /bin/true + archive_mode: "on" + autovacuum_max_workers: 3 + autovacuum_vacuum_cost_limit: 200 + autovacuum_work_mem: 262144 + checkpoint_completion_target: "0.9" + checkpoint_timeout: 15min + dynamic_shared_memory_type: posix + effective_cache_size: 524288 + hot_standby_feedback: "on" + log_destination: stderr + log_directory: log + log_filename: postgresql-%a.log + log_line_prefix: "%m [%p] " + log_rotation_age: 1d + log_rotation_size: "0" + log_truncate_on_rotation: "on" + logging_collector: "on" + lolor.node: 1 + maintenance_work_mem: 137518 + max_parallel_workers: 8 + password_encryption: scram-sha-256 + shared_buffers: 262144 + shared_preload_libraries: pg_stat_statements,snowflake,spock + snowflake.node: 1 + spock.allow_ddl_from_functions: "on" + spock.conflict_log_level: DEBUG + spock.conflict_resolution: last_update_wins + spock.enable_ddl_replication: "on" + spock.include_ddl_repset: "on" + spock.save_resolutions: "on" + ssl: "on" + ssl_ca_file: /opt/pgedge/certificates/postgres/ca.crt + ssl_cert_file: /opt/pgedge/certificates/postgres/server.crt + ssl_key_file: /opt/pgedge/certificates/postgres/server.key + track_io_timing: "on" + wal_log_hints: "on" + wal_sender_timeout: 5s + pg_hba: + - local all all trust + - host all all 127.0.0.1/32 trust + - host all all ::1/128 trust + - local replication all trust + - host replication all 127.0.0.1/32 trust + - host replication all ::1/128 trust + - hostssl all pgedge,patroni_replicator 172.17.0.1/32 cert clientcert=verify-full + - hostssl replication pgedge,patroni_replicator 172.17.0.1/32 cert clientcert=verify-full + - hostssl all pgedge,patroni_replicator 10.128.165.128/26 cert clientcert=verify-full + - hostssl replication pgedge,patroni_replicator 10.128.165.128/26 cert clientcert=verify-full + - host all pgedge,patroni_replicator 0.0.0.0/0 reject + - host all pgedge,patroni_replicator ::/0 reject + - host testdb myapp_user 10.0.0.0/8 scram-sha-256 + - hostssl all myapp_user 203.0.113.0/24 scram-sha-256 + - host all all 0.0.0.0/0 scram-sha-256 + - host all all ::/0 scram-sha-256 + pg_ident: + - ssl_users CN=alice,O=example alice + use_pg_rewind: true + remove_data_directory_on_rewind_failure: true + remove_data_directory_on_diverged_timelines: true +restapi: + connect_address: pghba-n1-689qacsi.pghba-database:8888 + listen: 0.0.0.0:8888 + allowlist: + - 172.17.0.1/32 + - 10.128.165.128/26 + - 127.0.0.1 + - localhost + - ::1 +watchdog: + mode: "off" diff --git a/server/internal/orchestrator/common/patroni_config_generator.go b/server/internal/orchestrator/common/patroni_config_generator.go index 3ced18df..07913346 100644 --- a/server/internal/orchestrator/common/patroni_config_generator.go +++ b/server/internal/orchestrator/common/patroni_config_generator.go @@ -56,6 +56,12 @@ type PatroniConfigGenerator struct { PatroniAllowlist []string `json:"patroni_allowlist"` // PatroniPort is the port that Patroni will listen on. PatroniPort int `json:"patroni_port"` + // PgHbaConf are user-supplied pg_hba.conf entries (one rule per element), + // inserted in the user zone after the CP rules and before the catch-all. + PgHbaConf []string `json:"pg_hba_conf,omitempty"` + // PgIdentConf are user-supplied pg_ident.conf entries (one mapping per + // element). The CP writes no pg_ident entries of its own. + PgIdentConf []string `json:"pg_ident_conf,omitempty"` // PostgresCertsDir is the Postgres certificates directory. PostgresCertsDir string `json:"postgres_certs_dir"` // PostgresPort is the port that Postgres will listen on. @@ -138,6 +144,8 @@ func NewPatroniConfigGenerator(opts PatroniConfigGeneratorOptions) *PatroniConfi PostgresPort: opts.PostgresPort, RestoreCommand: restoreCommand, SpecParameters: opts.Instance.PostgreSQLConf, + PgHbaConf: opts.Instance.PgHbaConf, + PgIdentConf: opts.Instance.PgIdentConf, TenantID: opts.Instance.TenantID, // TODO: Add allowlist field to instance and database types // PatroniAllowlist: opts.Instance.PatroniAllowlist, @@ -306,6 +314,14 @@ func (p *PatroniConfigGenerator) postgreSQL( } } + // The catch-all authenticates non-system users by password, so its auth + // method follows password_encryption to match how passwords are stored. It + // defaults to md5 (our DefaultGUCs value) when unset. + passwordAuthMethod := hba.AuthMethodMD5 + if pe, ok := parameters["password_encryption"].(string); ok && pe != "" { + passwordAuthMethod = hba.AuthMethod(pe) + } + return &patroni.PostgreSQL{ ConnectAddress: utils.PointerTo(net.JoinHostPort(p.FQDN, strconv.Itoa(p.PostgresPort))), DataDir: &p.DataDir, @@ -316,7 +332,8 @@ func (p *PatroniConfigGenerator) postgreSQL( RemoveDataDirectoryOnRewindFailure: utils.PointerTo(true), RemoveDataDirectoryOnDivergedTimelines: utils.PointerTo(true), Authentication: p.authentication(), - PgHba: p.pgHba(systemAddresses, extraEntries), + PgHba: p.pgHba(systemAddresses, extraEntries, passwordAuthMethod), + PgIdent: p.pgIdent(), } } @@ -339,7 +356,7 @@ func (p *PatroniConfigGenerator) authentication() *patroni.Authentication { } } -func (p *PatroniConfigGenerator) pgHba(systemAddresses []string, extraEntries []hba.Entry) *[]string { +func (p *PatroniConfigGenerator) pgHba(systemAddresses []string, extraEntries []hba.Entry, passwordAuthMethod hba.AuthMethod) *[]string { entries := []string{ // Trust local connections hba.Entry{ @@ -427,24 +444,40 @@ func (p *PatroniConfigGenerator) pgHba(systemAddresses []string, extraEntries [] entries = append(entries, entry.String()) } - // Use MD5 for non-system users from all other connections - // TODO: Can we safely upgrade this to scram-sha-256? + // User-supplied pg_hba entries form a zone after the CP's system-user rules + // and before the catch-all. By this point system users are already matched + // or rejected, so user rules cannot affect CP-internal connectivity. + // p.PgHbaConf already has node-level entries prepended ahead of the + // database-level entries. + entries = append(entries, p.PgHbaConf...) + + // Catch-all for non-system users. The auth method follows + // password_encryption so it matches how passwords are stored. entries = append(entries, hba.Entry{ Type: hba.EntryTypeHost, Database: "all", User: "all", Address: "0.0.0.0/0", - AuthMethod: hba.AuthMethodMD5, + AuthMethod: passwordAuthMethod, }.String(), hba.Entry{ Type: hba.EntryTypeHost, Database: "all", User: "all", Address: "::/0", - AuthMethod: hba.AuthMethodMD5, + AuthMethod: passwordAuthMethod, }.String(), ) return &entries } + +// pgIdent returns the user-supplied pg_ident.conf entries, or nil when there +// are none. The CP writes no pg_ident entries of its own. +func (p *PatroniConfigGenerator) pgIdent() *[]string { + if len(p.PgIdentConf) == 0 { + return nil + } + return &p.PgIdentConf +} diff --git a/server/internal/orchestrator/common/patroni_config_generator_test.go b/server/internal/orchestrator/common/patroni_config_generator_test.go index b495f51b..2a99737f 100644 --- a/server/internal/orchestrator/common/patroni_config_generator_test.go +++ b/server/internal/orchestrator/common/patroni_config_generator_test.go @@ -361,6 +361,53 @@ func TestPatroniConfigGenerator(t *testing.T) { }, }, }, + { + name: "user pg_hba pg_ident and scram", + options: common.PatroniConfigGeneratorOptions{ + Instance: &database.InstanceSpec{ + InstanceID: "pghba-n1-689qacsi", + DatabaseID: "pghba", + HostID: "host-1", + DatabaseName: "testdb", + NodeName: "n1", + NodeOrdinal: 1, + PgEdgeVersion: ds.MustParsePgEdgeVersion("18.4", "5.0.8"), + ClusterSize: 3, + // password_encryption drives the catch-all auth method. + PostgreSQLConf: map[string]any{ + "password_encryption": "scram-sha-256", + }, + // Node-level entries are already prepended ahead of + // database-level entries by NodeInstances() before reaching + // the generator. + PgHbaConf: []string{ + "host testdb myapp_user 10.0.0.0/8 scram-sha-256", + "hostssl all myapp_user 203.0.113.0/24 scram-sha-256", + }, + PgIdentConf: []string{"ssl_users CN=alice,O=example alice"}, + }, + HostCPUs: 4, + HostMemoryBytes: 1024 * 1024 * 1024 * 8, + FQDN: "pghba-n1-689qacsi.pghba-database", + LogType: patroni.LogTypeJson, + PatroniPort: 8888, + PostgresPort: 5432, + Paths: database.InstancePaths{ + Instance: database.Paths{BaseDir: "/opt/pgedge"}, + Host: database.Paths{BaseDir: "/data/control-plane/instances/pghba-n1-689qacsi"}, + PgBackRestPath: "/usr/bin/pgbackrest", + PatroniPath: "/usr/local/bin/patroni", + }, + }, + etcdHosts: []string{"i-0123456789abcdef.ec2.internal:2379"}, + etcdCreds: &common.EtcdCreds{ + Username: "instance.pghba-n1-689qacsi", + Password: "password", + }, + generateOptions: common.GenerateOptions{ + SystemAddresses: []string{"172.17.0.1/32", "10.128.165.128/26"}, + }, + }, } { t.Run(tc.name, func(t *testing.T) { gen := common.NewPatroniConfigGenerator(tc.options) diff --git a/server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/no_user_entries_defaults_to_md5.yaml b/server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/no_user_entries_defaults_to_md5.yaml new file mode 100644 index 00000000..24e45230 --- /dev/null +++ b/server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/no_user_entries_defaults_to_md5.yaml @@ -0,0 +1,122 @@ +name: plain-n1-689qacsi +namespace: /patroni/ +scope: plain:n1 +log: + type: json + level: INFO + static_fields: + database_id: plain + instance_id: plain-n1-689qacsi + node_name: n1 +bootstrap: + dcs: + loop_wait: 10 + ttl: 30 + retry_timeout: 10 + postgresql: + parameters: + max_connections: 901 + max_replication_slots: 16 + max_wal_senders: 16 + max_worker_processes: 12 + track_commit_timestamp: "on" + wal_level: logical + ignore_slots: + - plugin: spock_output + initdb: + - data-checksums +etcd3: + hosts: + - 10.0.0.1:2379 + protocol: https + username: instance.pghba-n1-689qacsi + password: password + cacert: /opt/pgedge/certificates/etcd/ca.crt + cert: /opt/pgedge/certificates/etcd/client.crt + key: /opt/pgedge/certificates/etcd/client.key +postgresql: + authentication: + superuser: + username: pgedge + sslmode: verify-full + sslkey: /opt/pgedge/certificates/postgres/superuser.key + sslcert: /opt/pgedge/certificates/postgres/superuser.crt + sslrootcert: /opt/pgedge/certificates/postgres/ca.crt + replication: + username: patroni_replicator + sslmode: verify-full + sslkey: /opt/pgedge/certificates/postgres/replication.key + sslcert: /opt/pgedge/certificates/postgres/replication.crt + sslrootcert: /opt/pgedge/certificates/postgres/ca.crt + callbacks: {} + connect_address: postgres-plain-n1-689qacsi:5432 + data_dir: /opt/pgedge/data/pgdata + listen: "*:5432" + parameters: + archive_command: /bin/true + archive_mode: "on" + autovacuum_max_workers: 3 + autovacuum_vacuum_cost_limit: 200 + autovacuum_work_mem: 262144 + checkpoint_completion_target: "0.9" + checkpoint_timeout: 15min + dynamic_shared_memory_type: posix + effective_cache_size: 524288 + hot_standby_feedback: "on" + log_destination: stderr + log_directory: log + log_filename: postgresql-%a.log + log_line_prefix: "%m [%p] " + log_rotation_age: 1d + log_rotation_size: "0" + log_truncate_on_rotation: "on" + logging_collector: "on" + lolor.node: 1 + maintenance_work_mem: 137518 + max_parallel_workers: 8 + password_encryption: md5 + shared_buffers: 262144 + shared_preload_libraries: pg_stat_statements,snowflake,spock,postgis-3 + snowflake.node: 1 + spock.allow_ddl_from_functions: "on" + spock.conflict_log_level: DEBUG + spock.conflict_resolution: last_update_wins + spock.enable_ddl_replication: "on" + spock.include_ddl_repset: "on" + spock.save_resolutions: "on" + ssl: "on" + ssl_ca_file: /opt/pgedge/certificates/postgres/ca.crt + ssl_cert_file: /opt/pgedge/certificates/postgres/server.crt + ssl_key_file: /opt/pgedge/certificates/postgres/server.key + track_io_timing: "on" + wal_log_hints: "on" + wal_sender_timeout: 5s + pg_hba: + - local all all trust + - host all all 127.0.0.1/32 trust + - host all all ::1/128 trust + - local replication all trust + - host replication all 127.0.0.1/32 trust + - host replication all ::1/128 trust + - hostssl all pgedge,patroni_replicator 172.17.0.1/32 cert clientcert=verify-full + - hostssl all pgedge,patroni_replicator 10.128.170.0/26 cert clientcert=verify-full + - hostssl replication pgedge,patroni_replicator 172.17.0.1/32 cert clientcert=verify-full + - hostssl replication pgedge,patroni_replicator 10.128.170.0/26 cert clientcert=verify-full + - host all pgedge,patroni_replicator 0.0.0.0/0 reject + - host all pgedge,patroni_replicator ::/0 reject + - host all all 172.17.0.1/32 md5 + - host all all 172.17.0.0/16 reject + - host all all 0.0.0.0/0 md5 + use_pg_rewind: true + remove_data_directory_on_rewind_failure: true + remove_data_directory_on_diverged_timelines: true +restapi: + connect_address: postgres-plain-n1-689qacsi.plain-database:8888 + listen: 0.0.0.0:8888 + allowlist: + - 172.17.0.1 + - 10.128.170.0/26 + - 127.0.0.1 + - localhost +watchdog: + mode: "off" diff --git a/server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/user_pg_hba_pg_ident_and_scram.yaml b/server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/user_pg_hba_pg_ident_and_scram.yaml new file mode 100644 index 00000000..056907c0 --- /dev/null +++ b/server/internal/orchestrator/swarm/golden_test/TestGeneratePatroniConfig/user_pg_hba_pg_ident_and_scram.yaml @@ -0,0 +1,126 @@ +name: pghba-n1-689qacsi +namespace: /patroni/ +scope: pghba:n1 +log: + type: json + level: INFO + static_fields: + database_id: pghba + instance_id: pghba-n1-689qacsi + node_name: n1 +bootstrap: + dcs: + loop_wait: 10 + ttl: 30 + retry_timeout: 10 + postgresql: + parameters: + max_connections: 901 + max_replication_slots: 16 + max_wal_senders: 16 + max_worker_processes: 12 + track_commit_timestamp: "on" + wal_level: logical + ignore_slots: + - plugin: spock_output + initdb: + - data-checksums +etcd3: + hosts: + - 10.0.0.1:2379 + protocol: https + username: instance.pghba-n1-689qacsi + password: password + cacert: /opt/pgedge/certificates/etcd/ca.crt + cert: /opt/pgedge/certificates/etcd/client.crt + key: /opt/pgedge/certificates/etcd/client.key +postgresql: + authentication: + superuser: + username: pgedge + sslmode: verify-full + sslkey: /opt/pgedge/certificates/postgres/superuser.key + sslcert: /opt/pgedge/certificates/postgres/superuser.crt + sslrootcert: /opt/pgedge/certificates/postgres/ca.crt + replication: + username: patroni_replicator + sslmode: verify-full + sslkey: /opt/pgedge/certificates/postgres/replication.key + sslcert: /opt/pgedge/certificates/postgres/replication.crt + sslrootcert: /opt/pgedge/certificates/postgres/ca.crt + callbacks: {} + connect_address: postgres-pghba-n1-689qacsi:5432 + data_dir: /opt/pgedge/data/pgdata + listen: "*:5432" + parameters: + archive_command: /bin/true + archive_mode: "on" + autovacuum_max_workers: 3 + autovacuum_vacuum_cost_limit: 200 + autovacuum_work_mem: 262144 + checkpoint_completion_target: "0.9" + checkpoint_timeout: 15min + dynamic_shared_memory_type: posix + effective_cache_size: 524288 + hot_standby_feedback: "on" + log_destination: stderr + log_directory: log + log_filename: postgresql-%a.log + log_line_prefix: "%m [%p] " + log_rotation_age: 1d + log_rotation_size: "0" + log_truncate_on_rotation: "on" + logging_collector: "on" + lolor.node: 1 + maintenance_work_mem: 137518 + max_parallel_workers: 8 + password_encryption: scram-sha-256 + shared_buffers: 262144 + shared_preload_libraries: pg_stat_statements,snowflake,spock,postgis-3 + snowflake.node: 1 + spock.allow_ddl_from_functions: "on" + spock.conflict_log_level: DEBUG + spock.conflict_resolution: last_update_wins + spock.enable_ddl_replication: "on" + spock.include_ddl_repset: "on" + spock.save_resolutions: "on" + ssl: "on" + ssl_ca_file: /opt/pgedge/certificates/postgres/ca.crt + ssl_cert_file: /opt/pgedge/certificates/postgres/server.crt + ssl_key_file: /opt/pgedge/certificates/postgres/server.key + track_io_timing: "on" + wal_log_hints: "on" + wal_sender_timeout: 5s + pg_hba: + - local all all trust + - host all all 127.0.0.1/32 trust + - host all all ::1/128 trust + - local replication all trust + - host replication all 127.0.0.1/32 trust + - host replication all ::1/128 trust + - hostssl all pgedge,patroni_replicator 172.17.0.1/32 cert clientcert=verify-full + - hostssl all pgedge,patroni_replicator 10.128.170.0/26 cert clientcert=verify-full + - hostssl replication pgedge,patroni_replicator 172.17.0.1/32 cert clientcert=verify-full + - hostssl replication pgedge,patroni_replicator 10.128.170.0/26 cert clientcert=verify-full + - host all pgedge,patroni_replicator 0.0.0.0/0 reject + - host all pgedge,patroni_replicator ::/0 reject + - host all all 172.17.0.1/32 md5 + - host all all 172.17.0.0/16 reject + - host testdb myapp_user 10.0.0.0/8 scram-sha-256 + - hostssl all myapp_user 203.0.113.0/24 scram-sha-256 + - host all all 0.0.0.0/0 scram-sha-256 + pg_ident: + - ssl_users CN=alice,O=example alice + use_pg_rewind: true + remove_data_directory_on_rewind_failure: true + remove_data_directory_on_diverged_timelines: true +restapi: + connect_address: postgres-pghba-n1-689qacsi.pghba-database:8888 + listen: 0.0.0.0:8888 + allowlist: + - 172.17.0.1 + - 10.128.170.0/26 + - 127.0.0.1 + - localhost +watchdog: + mode: "off" diff --git a/server/internal/orchestrator/swarm/main_test.go b/server/internal/orchestrator/swarm/main_test.go new file mode 100644 index 00000000..2e512c25 --- /dev/null +++ b/server/internal/orchestrator/swarm/main_test.go @@ -0,0 +1,15 @@ +package swarm + +import ( + "flag" + "os" + "testing" +) + +var update bool + +func TestMain(m *testing.M) { + flag.BoolVar(&update, "update", false, "update golden test outputs") + flag.Parse() + os.Exit(m.Run()) +} diff --git a/server/internal/orchestrator/swarm/patroni_config.go b/server/internal/orchestrator/swarm/patroni_config.go index 1438407c..a90c01fd 100644 --- a/server/internal/orchestrator/swarm/patroni_config.go +++ b/server/internal/orchestrator/swarm/patroni_config.go @@ -313,6 +313,15 @@ func generatePatroniConfig( } } + // The gateway and catch-all rules authenticate non-system users by + // password, so their auth method follows password_encryption to match how + // passwords are stored. It defaults to md5 (Postgres' historical default + // and our DefaultGUCs value) when unset. + passwordAuthMethod := hba.AuthMethodMD5 + if pe, ok := parameters["password_encryption"].(string); ok && pe != "" { + passwordAuthMethod = hba.AuthMethod(pe) + } + cfg := &patroni.Config{ Name: utils.PointerTo(spec.InstanceID), Namespace: utils.PointerTo(patroni.Namespace()), @@ -468,6 +477,16 @@ func generatePatroniConfig( Address: "0.0.0.0/0", AuthMethod: hba.AuthMethodReject, }.String(), + // The IPv6 counterpart of the reject above. Without it, a + // permissive user rule in the zone below could authenticate a + // system user over IPv6. + hba.Entry{ + Type: hba.EntryTypeHost, + Database: "all", + User: "pgedge,patroni_replicator", + Address: "::/0", + AuthMethod: hba.AuthMethodReject, + }.String(), // Use MD5 for non-system users from the gateway. External // connections will originate from this address when we publish // a host port. @@ -479,7 +498,8 @@ func generatePatroniConfig( AuthMethod: hba.AuthMethodMD5, }.String(), // Reject all other connections on the bridge network to prevent - // connections from other databases. + // connections from other databases. The user zone and catch-all + // are appended after this literal (see below). hba.Entry{ Type: hba.EntryTypeHost, Database: "all", @@ -487,19 +507,31 @@ func generatePatroniConfig( Address: bridgeInfo.Subnet.String(), AuthMethod: hba.AuthMethodReject, }.String(), - // Use MD5 for non-system users from all other connections - // TODO: Can we upgrade this to scram-sha-256? - hba.Entry{ - Type: hba.EntryTypeHost, - Database: "all", - User: "all", - Address: "0.0.0.0/0", - AuthMethod: hba.AuthMethodMD5, - }.String(), }, }, } + // User-supplied pg_hba entries form a zone after the CP's system-user and + // bridge-isolation rules and before the catch-all. By this point system + // users are already matched or rejected and cross-container traffic is + // blocked, so user rules cannot affect CP-internal connectivity. + // spec.PgHbaConf already has node-level entries prepended ahead of the + // database-level entries. + *cfg.Postgresql.PgHba = append(*cfg.Postgresql.PgHba, spec.PgHbaConf...) + // Catch-all for non-system users; the auth method follows password_encryption. + *cfg.Postgresql.PgHba = append(*cfg.Postgresql.PgHba, hba.Entry{ + Type: hba.EntryTypeHost, + Database: "all", + User: "all", + Address: "0.0.0.0/0", + AuthMethod: passwordAuthMethod, + }.String()) + + // pg_ident mappings are purely user-supplied; the CP writes none of its own. + if len(spec.PgIdentConf) > 0 { + cfg.Postgresql.PgIdent = &spec.PgIdentConf + } + if spec.RestoreConfig != nil { cfg.Bootstrap.Method = utils.PointerTo(patroni.BootstrapMethodNameRestore) diff --git a/server/internal/orchestrator/swarm/patroni_config_golden_test.go b/server/internal/orchestrator/swarm/patroni_config_golden_test.go new file mode 100644 index 00000000..2f93a502 --- /dev/null +++ b/server/internal/orchestrator/swarm/patroni_config_golden_test.go @@ -0,0 +1,119 @@ +package swarm + +import ( + "net/netip" + "testing" + + "github.com/goccy/go-yaml" + "github.com/stretchr/testify/require" + + "github.com/pgEdge/control-plane/server/internal/database" + "github.com/pgEdge/control-plane/server/internal/docker" + "github.com/pgEdge/control-plane/server/internal/ds" + "github.com/pgEdge/control-plane/server/internal/patroni" + "github.com/pgEdge/control-plane/server/internal/testutils" +) + +// TestGeneratePatroniConfig golden-tests the Swarm patroni config generator, +// focused on the pg_hba/pg_ident wiring: the user zone sits between the +// bridge-subnet reject and the catch-all, system users are rejected over both +// IPv4 and IPv6, the catch-all auth method follows password_encryption, and +// pg_ident is populated from the spec. +func TestGeneratePatroniConfig(t *testing.T) { + golden := &testutils.GoldenTest[patroni.Config]{ + Compare: func(t testing.TB, expected, actual patroni.Config) { + // Number types are lost in the YAML round-trip, so normalize the + // actual value the same way before comparing. + raw, err := yaml.Marshal(actual) + require.NoError(t, err) + + var roundTripped patroni.Config + require.NoError(t, yaml.Unmarshal(raw, &roundTripped)) + + require.Equal(t, expected, roundTripped) + }, + FileExtension: ".yaml", + Marshal: yaml.Marshal, + Unmarshal: yaml.Unmarshal, + } + + bridgeInfo := &docker.NetworkInfo{ + Name: "bridge", + Subnet: netip.MustParsePrefix("172.17.0.0/16"), + Gateway: netip.MustParseAddr("172.17.0.1"), + } + dbNetwork := &Network{ + Name: "pghba-database", + Subnet: netip.MustParsePrefix("10.128.170.0/26"), + Gateway: netip.MustParseAddr("10.128.170.1"), + } + etcdCreds := &EtcdCreds{ + Username: "instance.pghba-n1-689qacsi", + Password: "password", + } + + for _, tc := range []struct { + name string + spec *database.InstanceSpec + }{ + { + name: "user pg_hba pg_ident and scram", + spec: &database.InstanceSpec{ + InstanceID: "pghba-n1-689qacsi", + DatabaseID: "pghba", + HostID: "host-1", + DatabaseName: "testdb", + NodeName: "n1", + NodeOrdinal: 1, + PgEdgeVersion: ds.MustParsePgEdgeVersion("18.4", "5.0.8"), + ClusterSize: 3, + CPUs: 4, + MemoryBytes: 1024 * 1024 * 1024 * 8, + // password_encryption drives the gateway and catch-all auth method. + PostgreSQLConf: map[string]any{ + "password_encryption": "scram-sha-256", + }, + // Node-level entries are already prepended ahead of + // database-level entries by NodeInstances() before reaching the + // generator. + PgHbaConf: []string{ + "host testdb myapp_user 10.0.0.0/8 scram-sha-256", + "hostssl all myapp_user 203.0.113.0/24 scram-sha-256", + }, + PgIdentConf: []string{"ssl_users CN=alice,O=example alice"}, + }, + }, + { + name: "no user entries defaults to md5", + spec: &database.InstanceSpec{ + InstanceID: "plain-n1-689qacsi", + DatabaseID: "plain", + HostID: "host-1", + DatabaseName: "testdb", + NodeName: "n1", + NodeOrdinal: 1, + PgEdgeVersion: ds.MustParsePgEdgeVersion("18.4", "5.0.8"), + ClusterSize: 3, + CPUs: 4, + MemoryBytes: 1024 * 1024 * 1024 * 8, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg, err := generatePatroniConfig( + tc.spec, + "postgres-"+tc.spec.InstanceID, + 4, + 1024*1024*1024*8, + []string{"https://10.0.0.1:2379"}, + etcdCreds, + bridgeInfo, + dbNetwork, + false, + ) + require.NoError(t, err) + + golden.Run(t, *cfg, update) + }) + } +}