From 7e7ee94b8318cf3981e36fa7e4333f4f8b809a08 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Sun, 5 Apr 2026 21:39:24 +0300 Subject: [PATCH 01/24] Add Unix socket listener with proper HBA support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Client connections via Unix domain socket (unix_socket_dir config option). Bypasses TCP stack entirely — useful for benchmarking and local deployments. Changes: - unix_socket_dir config creates .s.PGSQL. socket (pgbench-compatible) - HBA local rules now correctly match Unix socket connections (previously local rules were always skipped) - HBA host/hostssl/hostnossl rules only match TCP connections - max_connections enforced for Unix socket clients - Socket file cleaned up on shutdown - is_unix flag threaded through check_hba -> Client::startup --- pg_doorman.toml | 5 ++ pg_doorman.yaml | 5 ++ src/app/generate/annotated.rs | 12 +++++ src/app/generate/fields.yaml | 7 +++ src/app/server.rs | 92 ++++++++++++++++++++++++++++++++++- src/auth/hba.rs | 54 ++++++++++++-------- src/auth/hba_eval_tests.rs | 4 +- src/client/entrypoint.rs | 91 +++++++++++++++++++++++++++++++++- src/client/mod.rs | 4 +- src/client/startup.rs | 4 ++ src/config/general.rs | 4 ++ src/config/mod.rs | 7 ++- 12 files changed, 262 insertions(+), 27 deletions(-) diff --git a/pg_doorman.toml b/pg_doorman.toml index 69c17517..43fac98e 100644 --- a/pg_doorman.toml +++ b/pg_doorman.toml @@ -115,6 +115,11 @@ tcp_user_timeout = 60 # Default: 1048576 (1048576 bytes) unix_socket_buffer_size = 1048576 +# Directory for Unix domain socket listener. When set, pg_doorman accepts connections via .s.PGSQL. socket file. +# Directory for Unix domain socket listener (.s.PGSQL.) +# When set, pg_doorman also accepts connections via Unix socket. +# unix_socket_dir = "/var/run/pg_doorman" + # -------------------------------------------------------------------------- # Connection Limits # -------------------------------------------------------------------------- diff --git a/pg_doorman.yaml b/pg_doorman.yaml index 400f019e..e96e1d7b 100644 --- a/pg_doorman.yaml +++ b/pg_doorman.yaml @@ -152,6 +152,11 @@ general: # Default: "1MB" (1048576 bytes) unix_socket_buffer_size: "1MB" + # Directory for Unix domain socket listener. When set, pg_doorman accepts connections via .s.PGSQL. socket file. + # Directory for Unix domain socket listener (.s.PGSQL.) + # When set, pg_doorman also accepts connections via Unix socket. + # unix_socket_dir: "/var/run/pg_doorman" + # -------------------------------------------------------------------------- # Connection Limits # -------------------------------------------------------------------------- diff --git a/src/app/generate/annotated.rs b/src/app/generate/annotated.rs index 4250fb0c..7a10d06d 100644 --- a/src/app/generate/annotated.rs +++ b/src/app/generate/annotated.rs @@ -593,6 +593,18 @@ fn write_general_section(w: &mut ConfigWriter, config: &Config) { "1048576 bytes", ); + write_field_desc(w, fi, "general", "unix_socket_dir"); + w.comment( + fi, + "Directory for Unix domain socket listener (.s.PGSQL.)", + ); + w.comment( + fi, + "When set, pg_doorman also accepts connections via Unix socket.", + ); + w.commented_kv(fi, "unix_socket_dir", &w.str_val("/var/run/pg_doorman")); + w.blank(); + // --- Connection Limits --- w.separator(fi, f.section_title("limits").get(w.russian)); w.blank(); diff --git a/src/app/generate/fields.yaml b/src/app/generate/fields.yaml index e534ba40..bd2798f2 100644 --- a/src/app/generate/fields.yaml +++ b/src/app/generate/fields.yaml @@ -482,6 +482,13 @@ fields: doc: "Buffer size for read and write operations when connecting to PostgreSQL via a unix socket." default: "1048576" + unix_socket_dir: + config: + en: "Directory for Unix domain socket listener. When set, pg_doorman accepts connections via .s.PGSQL. socket file." + ru: "Директория для Unix domain socket. При указании pg_doorman принимает подключения через файл .s.PGSQL.." + doc: "Directory for Unix domain socket listener. Creates .s.PGSQL. file for client connections. Compatible with pgbench -h ." + default: "null" + max_connections: config: en: | diff --git a/src/app/server.rs b/src/app/server.rs index feff45c6..522948c5 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -17,7 +17,7 @@ use tokio::{runtime::Builder, sync::mpsc}; use crate::app::args::Args; use crate::config::{get_config, reload_config, Config}; use crate::daemon; -use crate::messages::configure_tcp_socket; +use crate::messages::{configure_tcp_socket, configure_unix_socket}; use crate::pool::{retain, ClientServerMap, ConnectionPool}; use crate::prometheus::start_prometheus_server; use crate::stats::{Collector, Reporter, REPORTER, TOTAL_CONNECTION_COUNTER}; @@ -206,6 +206,24 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { + info!("Unix socket listening on {path}"); + Some(l) + } + Err(err) => { + error!("Failed to bind Unix socket {path}: {err}"); + None + } + } + } else { + None + }; + config.show(); // Tracks which client is connected to which server for query cancellation. @@ -465,12 +483,84 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { + let (socket, _unix_addr) = match new_unix { + Ok(pair) => pair, + Err(err) => { + error!("Failed to accept Unix connection: {err}"); + continue; + } + }; + if admin_only { + drop(socket); + continue; + } + configure_unix_socket(&socket); + let client_server_map = client_server_map.clone(); + let config = get_config(); + let log_client_disconnections = config.general.log_client_disconnections; + let max_connections = config.general.max_connections; + + tokio::task::spawn(async move { + let connection_id = TOTAL_CONNECTION_COUNTER.fetch_add(1, Ordering::Relaxed) as u64 + 1; + let current_clients = CURRENT_CLIENT_COUNT.fetch_add(1, Ordering::SeqCst); + if current_clients as u64 > max_connections { + warn!("[#c{connection_id}] unix client rejected: too many clients (current={current_clients}, max={max_connections})"); + CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); + return; + } + let start = Utc::now().naive_utc(); + + match crate::client::client_entrypoint_unix( + socket, + client_server_map, + admin_only, + connection_id, + ) + .await + { + Ok(session_info) => { + if log_client_disconnections { + let session = format_duration(&(Utc::now().naive_utc() - start)); + let identity = match &session_info { + Some(si) => format!("[{}@{} #c{}]", si.username, si.pool_name, si.connection_id), + None => format!("[#c{connection_id}]"), + }; + info!("{identity} unix client disconnected, session={session}"); + } + } + Err(err) => { + let session = format_duration(&(Utc::now().naive_utc() - start)); + warn!("[#c{connection_id}] unix client disconnected with error: {err}, session={session}"); + } + }; + CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); + }); + } + _ = exit_rx.recv() => { break; } } } + // Cleanup Unix socket file + if let Some(ref dir) = get_config().general.unix_socket_dir { + let path = format!("{dir}/.s.PGSQL.{}", get_config().general.port); + if let Err(err) = std::fs::remove_file(&path) { + if err.kind() != std::io::ErrorKind::NotFound { + warn!("Failed to remove Unix socket {path}: {err}"); + } + } + } + info!("Shutting down..."); }); diff --git a/src/auth/hba.rs b/src/auth/hba.rs index fc141427..382bd053 100644 --- a/src/auth/hba.rs +++ b/src/auth/hba.rs @@ -360,6 +360,7 @@ impl PgHba { &self, ip: IpAddr, ssl: bool, + is_unix: bool, type_auth: &str, username: &str, database: &str, @@ -371,16 +372,26 @@ impl PgHba { }; for rule in &self.rules { - // Skip local rules; they are intended only for unix-socket connections - if matches!(rule.host_type, HostType::Local) { - continue; - } - if !rule.host_type.matches_ssl(ssl) { - continue; - } - if let Some(net) = &rule.address { - if !net.contains(&ip) { - continue; + match rule.host_type { + HostType::Local => { + // local rules match only Unix socket connections + if !is_unix { + continue; + } + } + _ => { + // host/hostssl/hostnossl rules match only TCP connections + if is_unix { + continue; + } + if !rule.host_type.matches_ssl(ssl) { + continue; + } + if let Some(net) = &rule.address { + if !net.contains(&ip) { + continue; + } + } } } // Database and user must match as well (supporting keyword `all`). @@ -461,29 +472,29 @@ hostnossl all all 127.0.0.1/32 trust // md5 allowed for 10.1.2.3 over non-ssl and ssl (host matches both) let ip = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); assert_eq!( - hba.check_hba(ip, false, "md5", "alice", "app"), + hba.check_hba(ip, false, false, "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - hba.check_hba(ip, true, "md5", "alice", "app"), + hba.check_hba(ip, true, false, "md5", "alice", "app"), CheckResult::Allow ); // scram allowed for 192.168.1.10 only with ssl let ip2 = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 10)); assert_eq!( - hba.check_hba(ip2, true, "scram-sha-256", "alice", "app"), + hba.check_hba(ip2, true, false, "scram-sha-256", "alice", "app"), CheckResult::Allow ); assert_eq!( - hba.check_hba(ip2, false, "scram-sha-256", "alice", "app"), + hba.check_hba(ip2, false, false, "scram-sha-256", "alice", "app"), CheckResult::NotMatched ); // trust on localhost without ssl let ip3 = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); assert_eq!( - hba.check_hba(ip3, false, "md5", "alice", "app"), + hba.check_hba(ip3, false, false, "md5", "alice", "app"), CheckResult::Trust ); } @@ -509,13 +520,13 @@ hostnossl all all 127.0.0.1/32 trust // First rule trust for 127.0.0.1 let ip = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); assert_eq!( - cfg.hba.check_hba(ip, false, "md5", "alice", "app"), + cfg.hba.check_hba(ip, false, false, "md5", "alice", "app"), CheckResult::Trust ); // Second rule md5 for 10.1.2.3 let ip2 = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); assert_eq!( - cfg.hba.check_hba(ip2, false, "md5", "alice", "app"), + cfg.hba.check_hba(ip2, false, false, "md5", "alice", "app"), CheckResult::Allow ); } @@ -528,11 +539,11 @@ hostnossl all all 127.0.0.1/32 trust let cfg: Wrapper = toml::from_str(toml_in).expect("toml parse map content"); let ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); assert_eq!( - cfg.hba.check_hba(ip, false, "md5", "alice", "app"), + cfg.hba.check_hba(ip, false, false, "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - cfg.hba.check_hba(ip, true, "md5", "alice", "app"), + cfg.hba.check_hba(ip, true, false, "md5", "alice", "app"), CheckResult::Allow ); } @@ -555,7 +566,8 @@ hostnossl all all 127.0.0.1/32 trust let cfg: Wrapper = toml::from_str(&toml_in).expect("toml parse map path"); let ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 2)); assert_eq!( - cfg.hba.check_hba(ip, true, "scram-sha-256", "alice", "app"), + cfg.hba + .check_hba(ip, true, false, "scram-sha-256", "alice", "app"), CheckResult::Allow ); @@ -593,7 +605,7 @@ hostnossl all all 127.0.0.1/32 trust let cfg: Wrapper = toml::from_str(toml_in).expect("toml parse both fields"); let ip = IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)); assert_eq!( - cfg.hba.check_hba(ip, false, "md5", "alice", "app"), + cfg.hba.check_hba(ip, false, false, "md5", "alice", "app"), CheckResult::Allow ); } diff --git a/src/auth/hba_eval_tests.rs b/src/auth/hba_eval_tests.rs index 5a72c813..ede7bf35 100644 --- a/src/auth/hba_eval_tests.rs +++ b/src/auth/hba_eval_tests.rs @@ -22,8 +22,8 @@ fn ci_from_hba(hba_text: &str, ssl: bool) -> ClientIdentifier { let ip: std::net::IpAddr = "127.0.0.1".parse().unwrap(); let username = "user"; let database = "db"; - let hba_scram = hba.check_hba(ip, ssl, "scram-sha-256", username, database); - let hba_md5 = hba.check_hba(ip, ssl, "md5", username, database); + let hba_scram = hba.check_hba(ip, ssl, false, "scram-sha-256", username, database); + let hba_md5 = hba.check_hba(ip, ssl, false, "md5", username, database); let mut ci = base_ci(); ci.hba_scram = hba_scram; ci.hba_md5 = hba_md5; diff --git a/src/client/entrypoint.rs b/src/client/entrypoint.rs index 39560134..d5dc3e1d 100644 --- a/src/client/entrypoint.rs +++ b/src/client/entrypoint.rs @@ -1,7 +1,7 @@ use log::{error, info, warn}; use std::sync::atomic::Ordering; use tokio::io::split; -use tokio::net::TcpStream; +use tokio::net::{TcpStream, UnixStream}; use crate::config::get_config; use crate::errors::Error; @@ -168,6 +168,7 @@ pub async fn client_entrypoint( admin_only, false, connection_id, + false, ) .await { @@ -228,6 +229,7 @@ pub async fn client_entrypoint( admin_only, false, connection_id, + false, ) .await { @@ -286,3 +288,90 @@ pub async fn client_entrypoint( } } } + +/// Unix socket client entrypoint. No TLS, no peer_addr — uses fake 127.0.0.1:0. +pub async fn client_entrypoint_unix( + mut stream: UnixStream, + client_server_map: ClientServerMap, + admin_only: bool, + connection_id: u64, +) -> Result, Error> { + let fake_addr: std::net::SocketAddr = ([127, 0, 0, 1], 0).into(); + let config = get_config(); + let log_client_connections = config.general.log_client_connections; + + match get_startup::(&mut stream).await { + Ok((ClientConnectionType::Startup, bytes)) => { + PLAIN_CONNECTION_COUNTER.fetch_add(1, Ordering::Relaxed); + let (read, write) = split(stream); + + match Client::startup( + read, + write, + fake_addr, + bytes, + client_server_map, + admin_only, + false, + connection_id, + true, + ) + .await + { + Ok(mut client) => { + if log_client_connections { + info!( + "[{}@{} #c{}] client connected via unix socket", + client.username, client.pool_name, client.connection_id + ); + } + let session_info = ClientSessionInfo { + username: client.username.clone(), + pool_name: client.pool_name.clone(), + connection_id: client.connection_id, + }; + let result = client.handle().await; + if !client.is_admin() && result.is_err() { + client.disconnect_stats(); + } + result.map(|_| Some(session_info)) + } + Err(err) => Err(err), + } + } + + Ok((ClientConnectionType::Tls, _)) => { + error_response_terminal( + &mut stream, + "TLS is not supported on Unix socket connections", + "08P01", + ) + .await?; + Err(Error::ProtocolSyncError( + "TLS requested on Unix socket".into(), + )) + } + + Ok((ClientConnectionType::CancelQuery, bytes)) => { + CANCEL_CONNECTION_COUNTER.fetch_add(1, Ordering::Relaxed); + let (read, write) = split(stream); + + match Client::cancel(read, write, fake_addr, bytes, client_server_map).await { + Ok(mut client) => { + info!("Cancel request via unix socket"); + let result = client.handle().await; + if !client.is_admin() && result.is_err() { + client.disconnect_stats(); + } + result.map(|_| None) + } + Err(err) => Err(err), + } + } + + Err(err) => { + error!("#c{connection_id} unix client startup failed: {err}"); + Err(err) + } + } +} diff --git a/src/client/mod.rs b/src/client/mod.rs index 3d37d5e2..e8080f82 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -9,6 +9,8 @@ mod transaction; mod util; pub use core::Client; -pub use entrypoint::{client_entrypoint, client_entrypoint_too_many_clients_already}; +pub use entrypoint::{ + client_entrypoint, client_entrypoint_too_many_clients_already, client_entrypoint_unix, +}; pub use startup::startup_tls; pub use util::PREPARED_STATEMENT_COUNTER; diff --git a/src/client/startup.rs b/src/client/startup.rs index 4a0c896a..e196cef7 100644 --- a/src/client/startup.rs +++ b/src/client/startup.rs @@ -137,6 +137,7 @@ pub async fn startup_tls( admin_only, true, connection_id, + false, ) .await } @@ -173,6 +174,7 @@ where admin_only: bool, use_tls: bool, connection_id: u64, + is_unix: bool, ) -> Result, Error> { let parameters = parse_startup(bytes)?; @@ -205,6 +207,7 @@ where client_identifier.hba_md5 = check_hba( addr.ip(), use_tls, + is_unix, "md5", username_from_parameters, &pool_name, @@ -212,6 +215,7 @@ where client_identifier.hba_scram = check_hba( addr.ip(), use_tls, + is_unix, "scram-sha-256", username_from_parameters, &pool_name, diff --git a/src/config/general.rs b/src/config/general.rs index 856c28df..25582583 100644 --- a/src/config/general.rs +++ b/src/config/general.rs @@ -55,6 +55,9 @@ pub struct General { #[serde(default = "General::default_unix_socket_buffer_size")] pub unix_socket_buffer_size: ByteSize, + #[serde(default)] + pub unix_socket_dir: Option, + #[serde(default)] // True pub log_client_connections: bool, @@ -416,6 +419,7 @@ impl Default for General { tcp_no_delay: Self::default_tcp_no_delay(), tcp_user_timeout: Self::default_tcp_user_timeout(), unix_socket_buffer_size: Self::default_unix_socket_buffer_size(), + unix_socket_dir: None, log_client_connections: true, log_client_disconnections: true, sync_server_parameters: Self::default_sync_server_parameters(), diff --git a/src/config/mod.rs b/src/config/mod.rs index 33ecc92e..7579c1e9 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -559,13 +559,18 @@ pub async fn reload_config(client_server_map: ClientServerMap) -> Result CheckResult { let config = get_config(); if let Some(ref pg) = config.general.pg_hba { - return pg.check_hba(ip, ssl, type_auth, username, database); + return pg.check_hba(ip, ssl, is_unix, type_auth, username, database); + } + // Legacy hba list has no unix concept — allow all unix connections + if is_unix { + return CheckResult::Allow; } if config.general.hba.is_empty() { return CheckResult::Allow; From 86531eedaf9899957c098daa16f68e5846dab1ae Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Sun, 5 Apr 2026 21:56:53 +0300 Subject: [PATCH 02/24] Add BDD tests, CHANGELOG, fix annotated config comments - BDD test: psql via Unix socket reaches PostgreSQL backend (pg_backend_pid proves real backend, not synthetic response) - CHANGELOG.md with known limitations - Fix duplicate comment in annotated config for unix_socket_dir - Add ${PG_TEMP_DIR} placeholder for BDD tests --- documentation/en/src/changelog.md | 14 ++++ pg_doorman.toml | 5 +- pg_doorman.yaml | 5 +- src/app/generate/annotated.rs | 6 +- src/app/generate/fields.yaml | 6 +- src/app/server.rs | 2 +- src/client/entrypoint.rs | 2 +- tests/bdd/features/unix-socket.feature | 36 ++++++++++ tests/bdd/postgres_helper.rs | 93 ++++++++++++++++++++++++++ tests/bdd/world.rs | 5 ++ 10 files changed, 158 insertions(+), 16 deletions(-) create mode 100644 tests/bdd/features/unix-socket.feature diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index bbf79f58..3f752c5c 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -1,5 +1,19 @@ # Changelog +### Unreleased + +**New features:** + +- **Unix socket listener.** `unix_socket_dir` creates `.s.PGSQL.` socket file. Connect with `psql -h ` or `pgbench -h `. No TCP overhead on local connections. + +- **HBA `local` rule matching.** `local` rules in pg_hba now apply to Unix socket connections. `host`/`hostssl`/`hostnossl` rules apply only to TCP. Previously `local` rules were parsed but ignored. + +**Known limitations (Unix socket):** + +- No `unix_socket_permissions`. Socket file inherits umask. Restrict access through directory permissions. +- Unix listener not handed off during `SIGUSR2` binary upgrade. New process re-creates the socket; connections refused for ~100ms. +- `only_ssl_connections` does not reject Unix socket connections. Unix sockets do not need TLS for transport security. + ### 3.4.1 Apr 3, 2026 **Improvements:** diff --git a/pg_doorman.toml b/pg_doorman.toml index 43fac98e..74cc7cfb 100644 --- a/pg_doorman.toml +++ b/pg_doorman.toml @@ -115,9 +115,8 @@ tcp_user_timeout = 60 # Default: 1048576 (1048576 bytes) unix_socket_buffer_size = 1048576 -# Directory for Unix domain socket listener. When set, pg_doorman accepts connections via .s.PGSQL. socket file. -# Directory for Unix domain socket listener (.s.PGSQL.) -# When set, pg_doorman also accepts connections via Unix socket. +# Directory for Unix socket listener. Creates .s.PGSQL. file. Use psql -h or pgbench -h to connect. +# When set, pg_doorman also accepts connections via Unix socket (.s.PGSQL.). # unix_socket_dir = "/var/run/pg_doorman" # -------------------------------------------------------------------------- diff --git a/pg_doorman.yaml b/pg_doorman.yaml index e96e1d7b..81bcb45b 100644 --- a/pg_doorman.yaml +++ b/pg_doorman.yaml @@ -152,9 +152,8 @@ general: # Default: "1MB" (1048576 bytes) unix_socket_buffer_size: "1MB" - # Directory for Unix domain socket listener. When set, pg_doorman accepts connections via .s.PGSQL. socket file. - # Directory for Unix domain socket listener (.s.PGSQL.) - # When set, pg_doorman also accepts connections via Unix socket. + # Directory for Unix socket listener. Creates .s.PGSQL. file. Use psql -h or pgbench -h to connect. + # When set, pg_doorman also accepts connections via Unix socket (.s.PGSQL.). # unix_socket_dir: "/var/run/pg_doorman" # -------------------------------------------------------------------------- diff --git a/src/app/generate/annotated.rs b/src/app/generate/annotated.rs index 7a10d06d..f869d373 100644 --- a/src/app/generate/annotated.rs +++ b/src/app/generate/annotated.rs @@ -596,11 +596,7 @@ fn write_general_section(w: &mut ConfigWriter, config: &Config) { write_field_desc(w, fi, "general", "unix_socket_dir"); w.comment( fi, - "Directory for Unix domain socket listener (.s.PGSQL.)", - ); - w.comment( - fi, - "When set, pg_doorman also accepts connections via Unix socket.", + "When set, pg_doorman also accepts connections via Unix socket (.s.PGSQL.).", ); w.commented_kv(fi, "unix_socket_dir", &w.str_val("/var/run/pg_doorman")); w.blank(); diff --git a/src/app/generate/fields.yaml b/src/app/generate/fields.yaml index bd2798f2..6b8a59b6 100644 --- a/src/app/generate/fields.yaml +++ b/src/app/generate/fields.yaml @@ -484,9 +484,9 @@ fields: unix_socket_dir: config: - en: "Directory for Unix domain socket listener. When set, pg_doorman accepts connections via .s.PGSQL. socket file." - ru: "Директория для Unix domain socket. При указании pg_doorman принимает подключения через файл .s.PGSQL.." - doc: "Directory for Unix domain socket listener. Creates .s.PGSQL. file for client connections. Compatible with pgbench -h ." + en: "Directory for Unix socket listener. Creates .s.PGSQL. file. Use psql -h or pgbench -h to connect." + ru: "Директория для Unix socket. Создаёт файл .s.PGSQL.. Подключение: psql -h или pgbench -h ." + doc: "Directory for Unix domain socket listener. Creates .s.PGSQL. socket file for local client connections." default: "null" max_connections: diff --git a/src/app/server.rs b/src/app/server.rs index 522948c5..63e19058 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -206,7 +206,7 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box 0 proves the query was executed by a real PostgreSQL backend + Then psql query "SELECT pg_backend_pid() > 0" via pg_doorman unix socket as user "postgres" to database "postgres" returns "t" diff --git a/tests/bdd/postgres_helper.rs b/tests/bdd/postgres_helper.rs index dff82fa8..02f37a60 100644 --- a/tests/bdd/postgres_helper.rs +++ b/tests/bdd/postgres_helper.rs @@ -659,6 +659,99 @@ pub async fn store_password_hash(world: &mut DoormanWorld, pg_user: String, var_ world.vars.insert(var_name, hash); } +/// Build psql command connecting via Unix socket directory +fn build_psql_via_doorman_unix( + socket_dir: &str, + port: u16, + user: &str, + database: &str, + query: &str, +) -> Command { + let mut cmd = Command::new("psql"); + cmd.args([ + "-h", + socket_dir, + "-p", + &port.to_string(), + "-U", + user, + "-d", + database, + "-c", + query, + ]); + cmd.env("PGSSLMODE", "disable"); + cmd.arg("-w"); + cmd.env_remove("PGPASSWORD"); + cmd +} + +#[then( + expr = "psql connection to pg_doorman via unix socket as user {string} to database {string} succeeds" +)] +pub async fn psql_unix_connection_succeeds( + world: &mut DoormanWorld, + user: String, + database: String, +) { + let port = world.doorman_port.expect("pg_doorman not started"); + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir not set") + .path() + .to_str() + .unwrap() + .to_string(); + let output = build_psql_via_doorman_unix(&socket_dir, port, &user, &database, "SELECT 1") + .output() + .expect("Failed to run psql"); + assert!( + output.status.success(), + "psql unix socket connection failed: stdout={}, stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); +} + +#[then( + expr = "psql query {string} via pg_doorman unix socket as user {string} to database {string} returns {string}" +)] +pub async fn psql_unix_query_returns( + world: &mut DoormanWorld, + query: String, + user: String, + database: String, + expected: String, +) { + let port = world.doorman_port.expect("pg_doorman not started"); + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir not set") + .path() + .to_str() + .unwrap() + .to_string(); + let mut cmd = build_psql_via_doorman_unix(&socket_dir, port, &user, &database, &query); + cmd.args(["-t", "-A"]); + let output = cmd.output().expect("Failed to run psql"); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "psql unix query failed: stderr: {}", + stderr, + ); + assert!( + stdout.contains(&expected), + "Expected '{}', got: '{}' (stderr: {})", + expected, + stdout.trim(), + stderr.trim(), + ); +} + /// Stop PostgreSQL and pg_doorman when the world is dropped impl Drop for DoormanWorld { fn drop(&mut self) { diff --git a/tests/bdd/world.rs b/tests/bdd/world.rs index 3ed97af5..8a184769 100644 --- a/tests/bdd/world.rs +++ b/tests/bdd/world.rs @@ -123,6 +123,11 @@ impl DoormanWorld { result = result.replace("${ODYSSEY_PORT}", &port.to_string()); } + // Replace temp dir placeholder (for unix_socket_dir) + if let Some(ref tmp_dir) = self.pg_tmp_dir { + result = result.replace("${PG_TEMP_DIR}", tmp_dir.path().to_str().unwrap()); + } + // Replace file path placeholders if let Some(ref hba_file) = self.doorman_hba_file { result = result.replace("${DOORMAN_HBA_FILE}", hba_file.path().to_str().unwrap()); From b27392be16e79245fc2a2c99e318781554c413af Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Sun, 5 Apr 2026 23:37:34 +0300 Subject: [PATCH 03/24] Add Unix socket benchmarks to AWS Fargate bench suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: compare all three poolers (pg_doorman, pgbouncer, odyssey) via Unix domain socket alongside existing TCP benchmarks. What changed: bench.feature runs Unix socket tests first — simple, extended, prepared protocols with 1/40/120/500/10000 clients, with and without reconnect. Odyssey gets a dedicated unix-socket-only instance (it cannot bind TCP and Unix socket in one process). Benchmark tables now use ▲/▼ arrows for faster visual scanning of results. --- tests/bdd/features/bench.feature | 196 ++++++++++++++++++++++++++++++- tests/bdd/main.rs | 5 + tests/bdd/odyssey_helper.rs | 62 +++++++++- tests/bdd/pgbench_helper.rs | 64 ++++++++-- tests/bdd/service_helper.rs | 33 ++++++ tests/bdd/world.rs | 14 +++ 6 files changed, 364 insertions(+), 10 deletions(-) diff --git a/tests/bdd/features/bench.feature b/tests/bdd/features/bench.feature index e90718c7..f95cdba0 100644 --- a/tests/bdd/features/bench.feature +++ b/tests/bdd/features/bench.feature @@ -14,6 +14,7 @@ Feature: Benchmarking environment setup with SSL """ And pg_doorman hba file contains: """ + local all all trust host all all 0.0.0.0/0 trust hostssl all all 0.0.0.0/0 trust """ @@ -29,6 +30,7 @@ Feature: Benchmarking environment setup with SSL tls_private_key = "${DOORMAN_SSL_KEY}" tls_certificate = "${DOORMAN_SSL_CERT}" max_connections = 11000 + unix_socket_dir = "${PG_TEMP_DIR}" [pools.postgres] server_host = "127.0.0.1" @@ -52,7 +54,7 @@ Feature: Benchmarking environment setup with SSL [pgbouncer] listen_addr = 127.0.0.1 listen_port = ${PGBOUNCER_PORT} - unix_socket_dir = + unix_socket_dir = ${PG_TEMP_DIR} auth_type = trust auth_file = ${PGBOUNCER_USERLIST} pool_mode = transaction @@ -106,6 +108,198 @@ Feature: Benchmarking environment setup with SSL tls_key_file "${DOORMAN_SSL_KEY}" } """ + And odyssey unix socket instance started with config: + """ + unix_socket_dir "${PG_TEMP_DIR}" + workers ${ODYSSEY_WORKERS} + log_to_stdout no + log_file "/dev/null" + log_format "%p %t %l [%i %s] (%c) %m\n" + log_debug no + log_config no + log_session no + log_query no + log_stats no + + storage "postgres_server" { + type "remote" + host "127.0.0.1" + port ${PG_PORT} + } + + database "postgres" { + user "postgres" { + authentication "none" + storage "postgres_server" + pool "transaction" + pool_size 40 + pool_discard no + pool_reserve_prepared_statement yes + } + } + + listen { + port ${ODYSSEY_UNIX_PORT} + } + """ + + # ==================== UNIX SOCKET - SIMPLE PROTOCOL ==================== + + # --- 1 client, simple protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_simple_c1" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_c1" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_c1" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 40 clients, simple protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_simple_c40" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_c40" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_c40" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 500 clients, simple protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_simple_c500" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_c500" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_c500" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 10000 clients, simple protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_simple_c10000" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_c10000" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_c10000" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 120 clients, simple protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_simple_c120" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_c120" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_c120" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=simple postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # ==================== UNIX SOCKET - EXTENDED PROTOCOL ==================== + + # --- 1 client, extended protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_extended_c1" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_c1" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_c1" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 40 clients, extended protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_extended_c40" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_c40" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_c40" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 500 clients, extended protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_extended_c500" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_c500" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_c500" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 10000 clients, extended protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_extended_c10000" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_c10000" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_c10000" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 120 clients, extended protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_extended_c120" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_c120" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_c120" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=extended postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # ==================== UNIX SOCKET - PREPARED PROTOCOL ==================== + + # --- 1 client, prepared protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_prepared_c1" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_c1" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_c1" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 40 clients, prepared protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_prepared_c40" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_c40" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_c40" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 500 clients, prepared protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_prepared_c500" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_c500" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_c500" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 10000 clients, prepared protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_prepared_c10000" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_c10000" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_c10000" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 120 clients, prepared protocol, unix socket --- + When I run pgbench for "pg_doorman_unix_prepared_c120" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_c120" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_c120" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=prepared postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # ==================== UNIX SOCKET + CONNECT ==================== + + # --- 1 client, simple protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_simple_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 40 clients, simple protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_simple_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 500 clients, simple protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_simple_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 10000 clients, simple protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_simple_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 120 clients, simple protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_simple_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_simple_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_simple_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=simple --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 1 client, extended protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_extended_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 40 clients, extended protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_extended_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 500 clients, extended protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_extended_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 10000 clients, extended protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_extended_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 120 clients, extended protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_extended_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_extended_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_extended_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=extended --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 1 client, prepared protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_prepared_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_connect_c1" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 1 -j ${PGBENCH_JOBS_C1} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 40 clients, prepared protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_prepared_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_connect_c40" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 40 -j ${PGBENCH_JOBS_C40} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 500 clients, prepared protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_prepared_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_connect_c500" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 500 -j ${PGBENCH_JOBS_C500} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 10000 clients, prepared protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_prepared_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_connect_c10000" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 10000 -j ${PGBENCH_JOBS_C10000} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + + # --- 120 clients, prepared protocol, unix socket, with connect --- + When I run pgbench for "pg_doorman_unix_prepared_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${DOORMAN_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "odyssey_unix_prepared_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${ODYSSEY_UNIX_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" + When I run pgbench for "pgbouncer_unix_prepared_connect_c120" with "-n -h ${PG_TEMP_DIR} -p ${PGBOUNCER_PORT} -U postgres -c 120 -j ${PGBENCH_JOBS_C120} -T 30 -P 1 --protocol=prepared --connect postgres -f ${PGBENCH_FILE}" and env "PGSSLMODE=disable" # ==================== SIMPLE PROTOCOL ==================== diff --git a/tests/bdd/main.rs b/tests/bdd/main.rs index c0d6c566..9a6e1aea 100644 --- a/tests/bdd/main.rs +++ b/tests/bdd/main.rs @@ -133,6 +133,11 @@ fn main() { odyssey_helper::stop_odyssey(child); } w.odyssey_process = None; + + if let Some(ref mut child) = w.odyssey_unix_process { + odyssey_helper::stop_odyssey(child); + } + w.odyssey_unix_process = None; } Box::pin(async {}) }) diff --git a/tests/bdd/odyssey_helper.rs b/tests/bdd/odyssey_helper.rs index 45864f00..551cb461 100644 --- a/tests/bdd/odyssey_helper.rs +++ b/tests/bdd/odyssey_helper.rs @@ -1,4 +1,6 @@ -use crate::service_helper::{stop_process_immediate, wait_for_service_ready}; +use crate::service_helper::{ + stop_process_immediate, wait_for_service_ready, wait_for_unix_socket_ready, +}; use crate::utils::{create_temp_file, get_stdio_config}; use crate::world::DoormanWorld; use cucumber::{gherkin::Step, given}; @@ -49,6 +51,64 @@ pub async fn start_odyssey_with_config(world: &mut DoormanWorld, step: &Step) { .await; } +#[given("odyssey unix socket instance started with config:")] +pub async fn start_odyssey_unix_with_config(world: &mut DoormanWorld, step: &Step) { + if let Some(ref mut child) = world.odyssey_unix_process { + stop_odyssey(child); + } + world.odyssey_unix_process = None; + + let config_content = step + .docstring + .as_ref() + .expect("config_content not found") + .to_string(); + + let odyssey_unix_port = pick_unused_port().expect("No free ports for odyssey unix"); + + world.odyssey_unix_port = Some(odyssey_unix_port); + let config_content = world.replace_placeholders(&config_content); + + let config_file = create_temp_file(&config_content); + let config_path = config_file.path().to_path_buf(); + world.odyssey_unix_config_file = Some(config_file); + + let (stdout_cfg, stderr_cfg) = get_stdio_config(); + + let child = Command::new("odyssey") + .arg("--log_to_stdout") + .arg("--console") + .arg(&config_path) + .stdout(stdout_cfg) + .stderr(stderr_cfg) + .spawn() + .expect("Failed to start odyssey unix socket instance"); + + world.odyssey_unix_process = Some(child); + + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir must be set before odyssey unix socket instance") + .path() + .to_str() + .unwrap() + .to_string(); + + if let Err(e) = wait_for_unix_socket_ready( + "odyssey-unix", + &socket_dir, + odyssey_unix_port, + world.odyssey_unix_process.as_mut().unwrap(), + 20, + 500, + ) + .await + { + panic!("{}", e); + } +} + pub fn stop_odyssey(child: &mut std::process::Child) { stop_process_immediate(child); } diff --git a/tests/bdd/pgbench_helper.rs b/tests/bdd/pgbench_helper.rs index 2cdf088c..84c2e823 100644 --- a/tests/bdd/pgbench_helper.rs +++ b/tests/bdd/pgbench_helper.rs @@ -469,18 +469,16 @@ pub async fn generate_benchmark_markdown_table(world: &mut DoormanWorld) { if percent.abs() < 3.0 { "≈0%".to_string() } else if percent > 100.0 { - // More than 2x faster - show as multiplier - format!("x{:.1}", ratio) + format!("x{:.1} ▲", ratio) } else if percent < -50.0 { - // More than 2x slower - show as multiplier - format!("x{:.1}", ratio) + format!("x{:.1} ▼", ratio) } else if percent > 0.0 { - format!("+{:.0}%", percent) + format!("+{:.0}% ▲", percent) } else { - format!("{:.0}%", percent) + format!("{:.0}% ▼", percent) } } - Some(_) if doorman > 0.0 => "∞".to_string(), // competitor failed, pg_doorman wins + Some(_) if doorman > 0.0 => "∞".to_string(), Some(_) => "N/A".to_string(), None => "-".to_string(), } @@ -583,6 +581,53 @@ pub async fn generate_benchmark_markdown_table(world: &mut DoormanWorld) { ("ssl_prepared_c10000", "10,000 clients + SSL"), ]; + // Unix Socket tests (pg_doorman vs pgbouncer only, odyssey does not support simultaneous TCP + unix socket) + let unix_simple_configs: Vec<(&str, &str)> = vec![ + ("unix_simple_c1", "1 client"), + ("unix_simple_c40", "40 clients"), + ("unix_simple_c120", "120 clients"), + ("unix_simple_c500", "500 clients"), + ("unix_simple_c10000", "10,000 clients"), + ("unix_simple_connect_c1", "1 client + Reconnect"), + ("unix_simple_connect_c40", "40 clients + Reconnect"), + ("unix_simple_connect_c120", "120 clients + Reconnect"), + ("unix_simple_connect_c500", "500 clients + Reconnect"), + ("unix_simple_connect_c10000", "10,000 clients + Reconnect"), + ]; + + let unix_extended_configs: Vec<(&str, &str)> = vec![ + ("unix_extended_c1", "1 client"), + ("unix_extended_c40", "40 clients"), + ("unix_extended_c120", "120 clients"), + ("unix_extended_c500", "500 clients"), + ("unix_extended_c10000", "10,000 clients"), + ("unix_extended_connect_c1", "1 client + Reconnect"), + ("unix_extended_connect_c40", "40 clients + Reconnect"), + ("unix_extended_connect_c120", "120 clients + Reconnect"), + ("unix_extended_connect_c500", "500 clients + Reconnect"), + ("unix_extended_connect_c10000", "10,000 clients + Reconnect"), + ]; + + let unix_prepared_configs: Vec<(&str, &str)> = vec![ + ("unix_prepared_c1", "1 client"), + ("unix_prepared_c40", "40 clients"), + ("unix_prepared_c120", "120 clients"), + ("unix_prepared_c500", "500 clients"), + ("unix_prepared_c10000", "10,000 clients"), + ("unix_prepared_connect_c1", "1 client + Reconnect"), + ("unix_prepared_connect_c40", "40 clients + Reconnect"), + ("unix_prepared_connect_c120", "120 clients + Reconnect"), + ("unix_prepared_connect_c500", "500 clients + Reconnect"), + ("unix_prepared_connect_c10000", "10,000 clients + Reconnect"), + ]; + + // Unix socket table uses the same format as TCP table (vs pgbouncer + vs odyssey) + // Odyssey runs as a separate unix-socket-only instance since it cannot listen on TCP and unix socket simultaneously + + let unix_simple_table = generate_table(&unix_simple_configs, &world.bench_results); + let unix_extended_table = generate_table(&unix_extended_configs, &world.bench_results); + let unix_prepared_table = generate_table(&unix_prepared_configs, &world.bench_results); + let simple_table = generate_table(&simple_configs, &world.bench_results); let extended_table = generate_table(&extended_configs, &world.bench_results); let prepared_table = generate_table(&prepared_configs, &world.bench_results); @@ -690,8 +735,11 @@ These benchmarks are automatically generated by the CI pipeline using `pgbench`. // Combine sections let full_markdown = format!( - "{}\n---\n\n## Simple Protocol\n\n{}\n\n---\n\n## Extended Protocol\n\n{}\n\n---\n\n## Prepared Protocol\n\n{}\n\n---\n\n### Notes\n\n- Odyssey has poor support for extended query protocol in transaction pooling mode, resulting in significantly lower performance compared to pg_doorman and pgbouncer\n- **Important**: The values shown are **relative performance ratios**, not absolute TPS numbers. While absolute TPS values may vary depending on hardware and system load, the relative ratios between poolers should remain consistent when tests are run sequentially in a short timeframe (30 seconds each). This allows for fair comparison across different connection poolers under identical conditions\n", + "{}\n---\n\n## Unix Socket\n\nAll three poolers connect to clients via Unix domain socket. The pooler-to-PostgreSQL leg is TCP in all cases. Odyssey runs as a separate unix-socket-only instance (it cannot listen on TCP and Unix socket in the same process).\n\n### Simple Protocol (Unix Socket)\n\n{}\n\n### Extended Protocol (Unix Socket)\n\n{}\n\n### Prepared Protocol (Unix Socket)\n\n{}\n\n---\n\n## Simple Protocol\n\n{}\n\n---\n\n## Extended Protocol\n\n{}\n\n---\n\n## Prepared Protocol\n\n{}\n\n---\n\n### Notes\n\n- Odyssey has poor support for extended query protocol in transaction pooling mode, resulting in significantly lower performance compared to pg_doorman and pgbouncer\n- Unix socket tests: pg_doorman and pgbouncer reuse the same instances as TCP tests (they listen on both). Odyssey requires a dedicated unix-socket-only instance because it cannot bind TCP and unix socket in one process\n- **Important**: The values shown are **relative performance ratios**, not absolute TPS numbers. While absolute TPS values may vary depending on hardware and system load, the relative ratios between poolers should remain consistent when tests are run sequentially in a short timeframe (30 seconds each). This allows for fair comparison across different connection poolers under identical conditions\n", markdown_content, + unix_simple_table.join("\n"), + unix_extended_table.join("\n"), + unix_prepared_table.join("\n"), simple_table.join("\n"), extended_table.join("\n"), prepared_table.join("\n") diff --git a/tests/bdd/service_helper.rs b/tests/bdd/service_helper.rs index 1be687f0..54d1af77 100644 --- a/tests/bdd/service_helper.rs +++ b/tests/bdd/service_helper.rs @@ -63,6 +63,39 @@ pub async fn wait_for_tcp_ready( )) } +/// Wait for a service to be ready by checking Unix socket file +pub async fn wait_for_unix_socket_ready( + service_name: &str, + socket_dir: &str, + port: u16, + child: &mut Child, + max_attempts: u32, + interval_ms: u64, +) -> Result<(), String> { + let socket_path = format!("{}/.s.PGSQL.{}", socket_dir, port); + for _ in 0..max_attempts { + match child.try_wait() { + Ok(Some(status)) => { + return Err(format!("{} exited with status: {:?}", service_name, status)); + } + Ok(None) => { + if std::os::unix::net::UnixStream::connect(&socket_path).is_ok() { + return Ok(()); + } + } + Err(e) => { + return Err(format!("Error checking {} process: {}", service_name, e)); + } + } + sleep(Duration::from_millis(interval_ms)).await; + } + + Err(format!( + "{} failed to start on unix socket {} (timeout after {} attempts)", + service_name, socket_path, max_attempts + )) +} + /// Wait for a service to be ready, panic on failure /// This is a convenience wrapper around wait_for_tcp_ready pub async fn wait_for_service_ready(service_name: &str, port: u16, child: &mut Child) { diff --git a/tests/bdd/world.rs b/tests/bdd/world.rs index 8a184769..f7790552 100644 --- a/tests/bdd/world.rs +++ b/tests/bdd/world.rs @@ -60,6 +60,12 @@ pub struct DoormanWorld { pub odyssey_port: Option, /// odyssey config file pub odyssey_config_file: Option, + /// odyssey unix socket instance process handle (separate instance for unix socket benchmarks) + pub odyssey_unix_process: Option, + /// odyssey unix socket instance port + pub odyssey_unix_port: Option, + /// odyssey unix socket instance config file + pub odyssey_unix_config_file: Option, /// Accumulated messages from PG pub pg_accumulated_messages: Vec<(char, Vec)>, /// Accumulated messages from Doorman @@ -122,6 +128,9 @@ impl DoormanWorld { if let Some(port) = self.odyssey_port { result = result.replace("${ODYSSEY_PORT}", &port.to_string()); } + if let Some(port) = self.odyssey_unix_port { + result = result.replace("${ODYSSEY_UNIX_PORT}", &port.to_string()); + } // Replace temp dir placeholder (for unix_socket_dir) if let Some(ref tmp_dir) = self.pg_tmp_dir { @@ -255,6 +264,11 @@ impl std::fmt::Debug for DoormanWorld { &self.odyssey_process.as_ref().map(|p| p.id()), ) .field("odyssey_port", &self.odyssey_port) + .field( + "odyssey_unix_process", + &self.odyssey_unix_process.as_ref().map(|p| p.id()), + ) + .field("odyssey_unix_port", &self.odyssey_unix_port) .finish() } } From b9801e1aa9c4fea00bb09ca5ca2556c7703dd82e Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 07:39:38 +0300 Subject: [PATCH 04/24] Fix odyssey unix socket instance startup in bench suite Needed: AWS Fargate bench run failed because odyssey refused to start the unix-socket-only instance with "unix_socket_mode is not set". Added unix_socket_mode "0644" to the dedicated odyssey instance config. --- tests/bdd/features/bench.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bdd/features/bench.feature b/tests/bdd/features/bench.feature index f95cdba0..cd567086 100644 --- a/tests/bdd/features/bench.feature +++ b/tests/bdd/features/bench.feature @@ -111,6 +111,7 @@ Feature: Benchmarking environment setup with SSL And odyssey unix socket instance started with config: """ unix_socket_dir "${PG_TEMP_DIR}" + unix_socket_mode "0644" workers ${ODYSSEY_WORKERS} log_to_stdout no log_file "/dev/null" From 8ba673c308d96bb49b05fbc77e1ac61d14e3fb22 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 08:11:03 +0300 Subject: [PATCH 05/24] Tail Fargate task logs while waiting for completion Needed: during the long benchmark run the polling loop only printed "Task status is RUNNING" with no insight into what the bench was doing. Hard to tell if it was making progress or stuck. Now every 5th poll (~2.5 minutes) the loop fetches new log events from CloudWatch and prints them inline, so the workflow output shows the bench output as it happens. --- .github/workflows/bench-aws-fargate.yml | 30 +++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/bench-aws-fargate.yml b/.github/workflows/bench-aws-fargate.yml index 6201da1a..a44da417 100644 --- a/.github/workflows/bench-aws-fargate.yml +++ b/.github/workflows/bench-aws-fargate.yml @@ -327,25 +327,47 @@ jobs: echo "Waiting for task to complete..." MAX_ATTEMPTS=240 # 240 * 30s = 120 minutes ATTEMPT=0 + LOG_GROUP="/ecs/${CLUSTER}" + TASK_ID=$(echo "${TASK_ARN}" | awk -F/ '{print $NF}') + LOG_STREAM="benchmark/benchmark-container/${TASK_ID}" + LAST_LOG_TS=0 while [ $ATTEMPT -lt $MAX_ATTEMPTS ]; do STATUS=$(aws ecs describe-tasks \ --cluster "${CLUSTER}" \ --tasks "${TASK_ARN}" \ --query 'tasks[0].lastStatus' \ --output text) - + echo "Attempt $((ATTEMPT+1))/${MAX_ATTEMPTS}: Task status is ${STATUS}" - + if [ "${STATUS}" == "STOPPED" ]; then echo "✅ Task has stopped." break fi - + if [ "${STATUS}" == "None" ]; then echo "❌ Error: Task not found." exit 1 fi - + + # Tail recent log events every ~5 attempts (~2.5 minutes) + if [ "${STATUS}" == "RUNNING" ] && [ $((ATTEMPT % 5)) -eq 0 ]; then + LOG_JSON=$(aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --start-time "${LAST_LOG_TS}" \ + --start-from-head \ + --limit 50 \ + --output json 2>/dev/null || echo '{"events":[]}') + EVENT_COUNT=$(echo "${LOG_JSON}" | jq '.events | length') + if [ "${EVENT_COUNT}" -gt 0 ]; then + echo "----- recent logs (last ${EVENT_COUNT} lines) -----" + echo "${LOG_JSON}" | jq -r '.events[] | .message' + echo "---------------------------------------------------" + LAST_LOG_TS=$(echo "${LOG_JSON}" | jq '.events[-1].timestamp + 1') + fi + fi + sleep 30 ATTEMPT=$((ATTEMPT+1)) done From 4f3056f3602ec9be4db135bbcf2a19e1563ce486 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 08:55:51 +0300 Subject: [PATCH 06/24] Add unix_socket_mode config option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the .s.PGSQL. socket file inherited the process umask after bind, so the access surface depended on whoever launched pg_doorman. There was no way to declare the intended permissions in the config; operators had to umask-wrap systemd units or rely on parent directory permissions. What changed: a new general.unix_socket_mode setting (octal string, default "0600") fixes the permission bits on the socket file immediately after bind. The mode is validated at config load — invalid octal, empty strings, and bits above 0o777 (setuid/setgid/sticky) are rejected upfront rather than failing later inside the listener. The applied mode is logged at startup. To grant a Unix group, set "0660"; to allow any local user, set "0666". --- documentation/en/src/changelog.md | 3 +- pg_doorman.toml | 5 ++ pg_doorman.yaml | 5 ++ src/app/generate/annotated.rs | 4 + src/app/generate/docs.rs | 2 + src/app/generate/fields.yaml | 14 ++++ src/app/server.rs | 27 ++++++- src/config/general.rs | 126 ++++++++++++++++++++++++++++++ src/config/mod.rs | 5 ++ 9 files changed, 189 insertions(+), 2 deletions(-) diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index 3f752c5c..940f7951 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -8,9 +8,10 @@ - **HBA `local` rule matching.** `local` rules in pg_hba now apply to Unix socket connections. `host`/`hostssl`/`hostnossl` rules apply only to TCP. Previously `local` rules were parsed but ignored. +- **`unix_socket_mode` controls socket file permissions.** New `[general]` setting fixes the permission bits on `.s.PGSQL.` after bind, so the access surface no longer depends on the process umask. Octal string, default `"0600"` (owner only). Set to `"0660"` to grant a Unix group, or `"0666"` to allow any local user. Validated at config load — invalid octal values, setuid/setgid/sticky bits, and overflow into bits above `0o777` are rejected upfront. + **Known limitations (Unix socket):** -- No `unix_socket_permissions`. Socket file inherits umask. Restrict access through directory permissions. - Unix listener not handed off during `SIGUSR2` binary upgrade. New process re-creates the socket; connections refused for ~100ms. - `only_ssl_connections` does not reject Unix socket connections. Unix sockets do not need TLS for transport security. diff --git a/pg_doorman.toml b/pg_doorman.toml index 74cc7cfb..b29fd41e 100644 --- a/pg_doorman.toml +++ b/pg_doorman.toml @@ -119,6 +119,11 @@ unix_socket_buffer_size = 1048576 # When set, pg_doorman also accepts connections via Unix socket (.s.PGSQL.). # unix_socket_dir = "/var/run/pg_doorman" +# Permission mode applied to the .s.PGSQL. socket file after bind. +# Octal string. Only the lowest 9 bits (0o777) are honored. +# Default: "0600" +unix_socket_mode = "0600" + # -------------------------------------------------------------------------- # Connection Limits # -------------------------------------------------------------------------- diff --git a/pg_doorman.yaml b/pg_doorman.yaml index 81bcb45b..0d0b7a2c 100644 --- a/pg_doorman.yaml +++ b/pg_doorman.yaml @@ -156,6 +156,11 @@ general: # When set, pg_doorman also accepts connections via Unix socket (.s.PGSQL.). # unix_socket_dir: "/var/run/pg_doorman" + # Permission mode applied to the .s.PGSQL. socket file after bind. + # Octal string. Only the lowest 9 bits (0o777) are honored. + # Default: "0600" + unix_socket_mode: "0600" + # -------------------------------------------------------------------------- # Connection Limits # -------------------------------------------------------------------------- diff --git a/src/app/generate/annotated.rs b/src/app/generate/annotated.rs index f869d373..4f4bd2bc 100644 --- a/src/app/generate/annotated.rs +++ b/src/app/generate/annotated.rs @@ -601,6 +601,10 @@ fn write_general_section(w: &mut ConfigWriter, config: &Config) { w.commented_kv(fi, "unix_socket_dir", &w.str_val("/var/run/pg_doorman")); w.blank(); + write_field_comment(w, fi, "general", "unix_socket_mode"); + w.kv(fi, "unix_socket_mode", &w.str_val(&g.unix_socket_mode)); + w.blank(); + // --- Connection Limits --- w.separator(fi, f.section_title("limits").get(w.russian)); w.blank(); diff --git a/src/app/generate/docs.rs b/src/app/generate/docs.rs index 12c081c5..407bef3e 100644 --- a/src/app/generate/docs.rs +++ b/src/app/generate/docs.rs @@ -237,6 +237,8 @@ fn write_general_fields(out: &mut String, f: &FieldsData) { "tcp_keepalives_interval", "tcp_user_timeout", "unix_socket_buffer_size", + "unix_socket_dir", + "unix_socket_mode", "admin_username", "admin_password", "prepared_statements", diff --git a/src/app/generate/fields.yaml b/src/app/generate/fields.yaml index 6b8a59b6..71986807 100644 --- a/src/app/generate/fields.yaml +++ b/src/app/generate/fields.yaml @@ -489,6 +489,20 @@ fields: doc: "Directory for Unix domain socket listener. Creates .s.PGSQL. socket file for local client connections." default: "null" + unix_socket_mode: + config: + en: | + Permission mode applied to the .s.PGSQL. socket file after bind. + Octal string. Only the lowest 9 bits (0o777) are honored. + ru: | + Права доступа на файл .s.PGSQL., выставляемые сразу после bind. + Восьмеричная строка. Учитываются только младшие 9 бит (0o777). + doc: | + Permission mode applied to the Unix domain socket file `.s.PGSQL.` immediately after `bind()`. Specified as an octal string (e.g. `"0600"`, `"0660"`, `"0666"`). Only the lowest 9 bits are honored — setuid/setgid/sticky bits are rejected. + + The default `"0600"` restricts socket access to the user running pg_doorman. To let other local users connect, set a more permissive mode such as `"0660"` (group access) or `"0666"` (any local user). When loosening the mode, ensure the parent directory permissions allow traversal by the intended group. + default: "\"0600\"" + max_connections: config: en: | diff --git a/src/app/server.rs b/src/app/server.rs index 63e19058..4aeb3699 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -212,7 +212,32 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { - info!("Unix socket listening on {path}"); + // Apply the configured permission mode after bind so the socket + // file does not depend on the process umask. The mode string was + // already validated at config load time. + let mode = crate::config::General::parse_unix_socket_mode( + &config.general.unix_socket_mode, + ) + .expect("unix_socket_mode validated at config load"); + use std::os::unix::fs::PermissionsExt; + match std::fs::set_permissions( + &path, + std::fs::Permissions::from_mode(mode), + ) { + Ok(()) => { + info!( + "Unix socket listening on {path} (mode={:#o})", + mode + ); + } + Err(err) => { + error!( + "Failed to set mode {:#o} on Unix socket {path}: {err}", + mode + ); + std::process::exit(exitcode::OSERR); + } + } Some(l) } Err(err) => { diff --git a/src/config/general.rs b/src/config/general.rs index 25582583..9725873c 100644 --- a/src/config/general.rs +++ b/src/config/general.rs @@ -58,6 +58,13 @@ pub struct General { #[serde(default)] pub unix_socket_dir: Option, + /// Permission mode applied to the Unix socket file `.s.PGSQL.` after bind. + /// Specified as an octal string (e.g. `"0600"`, `"0660"`, `"0666"`). + /// Only the lowest 9 bits (`0o777`) are honored. + /// Default: `"0600"` (owner read/write only). + #[serde(default = "General::default_unix_socket_mode")] + pub unix_socket_mode: String, + #[serde(default)] // True pub log_client_connections: bool, @@ -240,6 +247,38 @@ impl General { ByteSize::from_mb(1) // 1mb } + /// Default permission mode for the Unix socket file: `0600` (owner read/write only). + pub fn default_unix_socket_mode() -> String { + "0600".to_string() + } + + /// Parse a Unix socket permission mode from its octal string form. + /// + /// Accepts strings like `"0600"`, `"600"`, `"0o660"`, `"0644"`. Only the lowest + /// 9 bits (`0o777`) are honored — extra bits are rejected because they would + /// silently enable setuid/setgid/sticky semantics on the socket file. + /// + /// Returns the parsed mode on success or a human-readable error otherwise. + pub fn parse_unix_socket_mode(raw: &str) -> Result { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Err("unix_socket_mode must not be empty".to_string()); + } + let digits = trimmed + .strip_prefix("0o") + .or_else(|| trimmed.strip_prefix("0O")) + .unwrap_or(trimmed); + let parsed = u32::from_str_radix(digits, 8).map_err(|err| { + format!("unix_socket_mode {raw:?} is not a valid octal number: {err}") + })?; + if parsed & !0o777 != 0 { + return Err(format!( + "unix_socket_mode {raw:?} sets bits outside 0o777; only standard rwx permissions are allowed" + )); + } + Ok(parsed) + } + pub fn default_worker_cpu_affinity_pinning() -> bool { false } @@ -420,6 +459,7 @@ impl Default for General { tcp_user_timeout: Self::default_tcp_user_timeout(), unix_socket_buffer_size: Self::default_unix_socket_buffer_size(), unix_socket_dir: None, + unix_socket_mode: Self::default_unix_socket_mode(), log_client_connections: true, log_client_disconnections: true, sync_server_parameters: Self::default_sync_server_parameters(), @@ -451,3 +491,89 @@ impl Default for General { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_unix_socket_mode_accepts_owner_only() { + assert_eq!(General::parse_unix_socket_mode("0600").unwrap(), 0o600); + } + + #[test] + fn parse_unix_socket_mode_accepts_group_readable() { + assert_eq!(General::parse_unix_socket_mode("0660").unwrap(), 0o660); + } + + #[test] + fn parse_unix_socket_mode_accepts_world_writable() { + assert_eq!(General::parse_unix_socket_mode("0666").unwrap(), 0o666); + } + + #[test] + fn parse_unix_socket_mode_accepts_full_octal() { + assert_eq!(General::parse_unix_socket_mode("0777").unwrap(), 0o777); + } + + #[test] + fn parse_unix_socket_mode_accepts_three_digit_form() { + assert_eq!(General::parse_unix_socket_mode("644").unwrap(), 0o644); + } + + #[test] + fn parse_unix_socket_mode_accepts_zero() { + // Mode 0 is technically valid (no access for anyone). The kernel will reject + // any subsequent connect(), so this is a self-inflicted footgun, not a parser + // bug — accept it to keep the parser scope tight to syntax + bit checks. + assert_eq!(General::parse_unix_socket_mode("0000").unwrap(), 0); + } + + #[test] + fn parse_unix_socket_mode_accepts_0o_prefix() { + assert_eq!(General::parse_unix_socket_mode("0o640").unwrap(), 0o640); + } + + #[test] + fn parse_unix_socket_mode_default_matches_owner_only() { + let parsed = General::parse_unix_socket_mode(&General::default_unix_socket_mode()).unwrap(); + assert_eq!(parsed, 0o600); + } + + #[test] + fn parse_unix_socket_mode_rejects_non_octal_digit() { + let err = General::parse_unix_socket_mode("0900").unwrap_err(); + assert!(err.contains("octal"), "unexpected error: {err}"); + } + + #[test] + fn parse_unix_socket_mode_rejects_alphabetic() { + let err = General::parse_unix_socket_mode("abc").unwrap_err(); + assert!(err.contains("octal"), "unexpected error: {err}"); + } + + #[test] + fn parse_unix_socket_mode_rejects_overflow_into_setuid_bit() { + // 01600 sets the setuid bit (0o4000 family is masked out by !0o777 → bit 0o1000 is rejected). + let err = General::parse_unix_socket_mode("01600").unwrap_err(); + assert!(err.contains("0o777"), "unexpected error: {err}"); + } + + #[test] + fn parse_unix_socket_mode_rejects_far_overflow() { + let err = General::parse_unix_socket_mode("12345").unwrap_err(); + assert!(err.contains("0o777"), "unexpected error: {err}"); + } + + #[test] + fn parse_unix_socket_mode_rejects_empty() { + let err = General::parse_unix_socket_mode("").unwrap_err(); + assert!(err.contains("empty"), "unexpected error: {err}"); + } + + #[test] + fn parse_unix_socket_mode_rejects_whitespace_only() { + let err = General::parse_unix_socket_mode(" ").unwrap_err(); + assert!(err.contains("empty"), "unexpected error: {err}"); + } +} diff --git a/src/config/mod.rs b/src/config/mod.rs index 7579c1e9..6f19ba57 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -364,6 +364,11 @@ impl Config { )); } + // Validate unix_socket_mode upfront so misconfigurations fail at startup + // rather than at the moment the listener tries to chmod the socket file. + General::parse_unix_socket_mode(&self.general.unix_socket_mode) + .map_err(|err| Error::BadConfig(format!("general.{err}")))?; + // Validate mutual exclusion for HBA settings if self.general.pg_hba.is_some() && !self.general.hba.is_empty() { return Err(Error::BadConfig( From c97a50c90dfd75765a6c8dea80176632a3b4d1d8 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 09:59:11 +0300 Subject: [PATCH 07/24] Fix clippy warnings in tests Needed: cargo clippy --tests --deny "warnings" failed with 34 errors across BDD test helpers and unit-test modules, blocking CI lint gate. What changed: applied clippy's recommended fixes - converted manual range checks to RangeInclusive::contains, replaced loop+match patterns with while let, swapped &[b'X'] for b"X" byte strings, removed useless format!/return/lifetimes, derived Default where trivial, used struct update syntax instead of post-construction field assignment, dropped unnecessary i32 casts, and switched from DoubleEndedIterator::last() to next_back(). No test logic or assertions were modified. --- src/bin/patroni_proxy/port.rs | 47 +++++++------------ .../patroni_proxy/tests/bdd/port_allocator.rs | 15 +++--- .../patroni_proxy/tests/bdd/proxy_helper.rs | 2 +- src/bin/patroni_proxy/tests/bdd/world.rs | 19 +------- src/config/tests.rs | 12 +++-- src/messages/socket.rs | 8 ++-- src/stats/address.rs | 16 +++---- src/utils/debug_messages.rs | 6 +-- tests/bdd/doorman_helper.rs | 1 - tests/bdd/extended/helpers.rs | 2 +- tests/bdd/fuzz_client.rs | 6 +-- tests/bdd/generate_helper.rs | 8 ++-- 12 files changed, 54 insertions(+), 88 deletions(-) diff --git a/src/bin/patroni_proxy/port.rs b/src/bin/patroni_proxy/port.rs index 3798c9ec..6cd2d7d4 100644 --- a/src/bin/patroni_proxy/port.rs +++ b/src/bin/patroni_proxy/port.rs @@ -683,17 +683,12 @@ mod tests { // Start backend that keeps connection open let backend_task = tokio::spawn(async move { - loop { - match backend_listener.accept().await { - Ok((mut stream, _)) => { - // Keep connection open until it's closed - tokio::spawn(async move { - tokio::time::sleep(Duration::from_secs(60)).await; - let _ = stream.shutdown().await; - }); - } - Err(_) => break, - } + while let Ok((mut stream, _)) = backend_listener.accept().await { + // Keep connection open until it's closed + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(60)).await; + let _ = stream.shutdown().await; + }); } }); @@ -779,30 +774,20 @@ mod tests { // Start backends that keep connections open let backend1_task = tokio::spawn(async move { - loop { - match backend1_listener.accept().await { - Ok((mut stream, _)) => { - tokio::spawn(async move { - tokio::time::sleep(Duration::from_secs(60)).await; - let _ = stream.shutdown().await; - }); - } - Err(_) => break, - } + while let Ok((mut stream, _)) = backend1_listener.accept().await { + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(60)).await; + let _ = stream.shutdown().await; + }); } }); let backend2_task = tokio::spawn(async move { - loop { - match backend2_listener.accept().await { - Ok((mut stream, _)) => { - tokio::spawn(async move { - tokio::time::sleep(Duration::from_secs(60)).await; - let _ = stream.shutdown().await; - }); - } - Err(_) => break, - } + while let Ok((mut stream, _)) = backend2_listener.accept().await { + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(60)).await; + let _ = stream.shutdown().await; + }); } }); diff --git a/src/bin/patroni_proxy/tests/bdd/port_allocator.rs b/src/bin/patroni_proxy/tests/bdd/port_allocator.rs index 2419914d..ab76accb 100644 --- a/src/bin/patroni_proxy/tests/bdd/port_allocator.rs +++ b/src/bin/patroni_proxy/tests/bdd/port_allocator.rs @@ -39,14 +39,11 @@ pub fn allocate_port() -> u16 { #[cfg(test)] mod tests { - use super::allocate_port; - use std::net::{SocketAddr, TcpListener}; - #[test] fn test_allocate_port_returns_different_ports() { - let port1 = allocate_port(); - let port2 = allocate_port(); - let port3 = allocate_port(); + let port1 = super::allocate_port(); + let port2 = super::allocate_port(); + let port3 = super::allocate_port(); assert_ne!(port1, port2); assert_ne!(port2, port3); @@ -55,11 +52,11 @@ mod tests { #[test] fn test_allocated_port_is_bindable() { - let port = allocate_port(); - let addr: SocketAddr = format!("127.0.0.1:{}", port).parse().unwrap(); + let port = super::allocate_port(); + let addr: std::net::SocketAddr = format!("127.0.0.1:{}", port).parse().unwrap(); // Should be able to bind to the allocated port - let listener = TcpListener::bind(addr); + let listener = std::net::TcpListener::bind(addr); assert!(listener.is_ok()); } } diff --git a/src/bin/patroni_proxy/tests/bdd/proxy_helper.rs b/src/bin/patroni_proxy/tests/bdd/proxy_helper.rs index 9077c20b..611bd9dd 100644 --- a/src/bin/patroni_proxy/tests/bdd/proxy_helper.rs +++ b/src/bin/patroni_proxy/tests/bdd/proxy_helper.rs @@ -48,7 +48,7 @@ pub async fn start_proxy_with_config(world: &mut PatroniProxyWorld, step: &Step) if let Some(ref api_addr) = world.api_listen_address { let port = api_addr .split(':') - .last() + .next_back() .and_then(|p| p.parse::().ok()) .expect("Invalid API listen address"); wait_for_proxy_ready(port, world.proxy_process.as_mut().unwrap()).await; diff --git a/src/bin/patroni_proxy/tests/bdd/world.rs b/src/bin/patroni_proxy/tests/bdd/world.rs index 116ec8ed..9889c013 100644 --- a/src/bin/patroni_proxy/tests/bdd/world.rs +++ b/src/bin/patroni_proxy/tests/bdd/world.rs @@ -7,7 +7,7 @@ use std::sync::{Arc, RwLock}; use tempfile::NamedTempFile; /// The World struct holds the state shared across all steps in a scenario. -#[derive(World)] +#[derive(World, Default)] pub struct PatroniProxyWorld { /// patroni_proxy process handle pub proxy_process: Option, @@ -31,23 +31,6 @@ pub struct PatroniProxyWorld { pub mock_backends: HashMap, } -impl Default for PatroniProxyWorld { - fn default() -> Self { - Self { - proxy_process: None, - proxy_config_file: None, - mock_patroni_shutdowns: HashMap::new(), - mock_patroni_ports: Vec::new(), - mock_patroni_names: HashMap::new(), - mock_patroni_responses: HashMap::new(), - proxy_listen_addresses: HashMap::new(), - api_listen_address: None, - active_connections: HashMap::new(), - mock_backends: HashMap::new(), - } - } -} - impl PatroniProxyWorld { /// Replace all known placeholders in the given text pub fn replace_placeholders(&self, text: &str) -> String { diff --git a/src/config/tests.rs b/src/config/tests.rs index 1ded47a1..cc1bb40b 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -1321,8 +1321,10 @@ async fn test_resolve_scaling_config_pool_override() { general.scaling_fast_retries = 10; general.scaling_cooldown_sleep = Duration::from_millis(10); - let mut pool = Pool::default(); - pool.scaling_warm_pool_ratio = Some(50); + let pool = Pool { + scaling_warm_pool_ratio: Some(50), + ..Pool::default() + }; let scaling = pool.resolve_scaling_config(&general); assert!((scaling.warm_pool_ratio - 0.5).abs() < f32::EPSILON); @@ -1364,8 +1366,10 @@ async fn test_validate_scaling_warm_pool_ratio_general_out_of_range() { #[tokio::test] async fn test_validate_scaling_warm_pool_ratio_pool_out_of_range() { let mut config = Config::default(); - let mut pool = Pool::default(); - pool.scaling_warm_pool_ratio = Some(101); + let mut pool = Pool { + scaling_warm_pool_ratio: Some(101), + ..Pool::default() + }; pool.users.push(User { username: "user1".to_string(), password: "pass1".to_string(), diff --git a/src/messages/socket.rs b/src/messages/socket.rs index 463cdb90..09d8fa65 100644 --- a/src/messages/socket.rs +++ b/src/messages/socket.rs @@ -310,7 +310,7 @@ mod tests { /// DBA sees this after every successful query. Must parse correctly. #[tokio::test] async fn reuse_ready_for_query() { - let data = wire_msg(b'Z', &[b'I']); + let data = wire_msg(b'Z', b"I"); let mut stream = Cursor::new(data); let mut buf = BytesMut::with_capacity(READ_BUF_DEFAULT_CAPACITY); @@ -438,7 +438,7 @@ mod tests { /// Resets counter to isolate from parallel tests sharing the global atomic. #[tokio::test] async fn reuse_memory_counter_balanced_on_success() { - let data = wire_msg(b'Z', &[b'I']); + let data = wire_msg(b'Z', b"I"); let mut stream = Cursor::new(data); let mut buf = BytesMut::with_capacity(READ_BUF_DEFAULT_CAPACITY); @@ -475,9 +475,9 @@ mod tests { /// This is the steady-state: after warmup, zero allocations per message. #[tokio::test] async fn reuse_sequential_messages_stable_capacity() { - let msg1 = wire_msg(b'Z', &[b'I']); + let msg1 = wire_msg(b'Z', b"I"); let msg2 = wire_msg(b'C', b"SELECT 1\0"); - let msg3 = wire_msg(b'Z', &[b'T']); + let msg3 = wire_msg(b'Z', b"T"); let mut all = Vec::new(); all.extend_from_slice(&msg1); diff --git a/src/stats/address.rs b/src/stats/address.rs index 7f0b2dbc..0887f279 100644 --- a/src/stats/address.rs +++ b/src/stats/address.rs @@ -632,22 +632,22 @@ mod tests { let (p50, p90, p95, p99) = stats.get_xact_percentiles(); // p50 should be around 50, p90 around 90, p95 around 95, p99 around 99 assert!( - p50 >= 45 && p50 <= 55, + (45..=55).contains(&p50), "p50 xact should be ~50, got {}", p50 ); assert!( - p90 >= 85 && p90 <= 95, + (85..=95).contains(&p90), "p90 xact should be ~90, got {}", p90 ); assert!( - p95 >= 90 && p95 <= 100, + (90..=100).contains(&p95), "p95 xact should be ~95, got {}", p95 ); assert!( - p99 >= 95 && p99 <= 105, + (95..=105).contains(&p99), "p99 xact should be ~99, got {}", p99 ); @@ -655,22 +655,22 @@ mod tests { // Verify percentiles for queries let (p50, p90, p95, p99) = stats.get_query_percentiles(); assert!( - p50 >= 45 && p50 <= 55, + (45..=55).contains(&p50), "p50 query should be ~50, got {}", p50 ); assert!( - p90 >= 85 && p90 <= 95, + (85..=95).contains(&p90), "p90 query should be ~90, got {}", p90 ); assert!( - p95 >= 90 && p95 <= 100, + (90..=100).contains(&p95), "p95 query should be ~95, got {}", p95 ); assert!( - p99 >= 95 && p99 <= 105, + (95..=105).contains(&p99), "p99 query should be ~99, got {}", p99 ); diff --git a/src/utils/debug_messages.rs b/src/utils/debug_messages.rs index 504d3d0d..a5a961e0 100644 --- a/src/utils/debug_messages.rs +++ b/src/utils/debug_messages.rs @@ -720,7 +720,7 @@ mod tests { // Execute: E + len + portal + max_rows buf.push(b'E'); - let e_len = (4 + 1 + 4) as i32; // len + empty portal + max_rows + let e_len: i32 = 4 + 1 + 4; // len + empty portal + max_rows buf.extend_from_slice(&e_len.to_be_bytes()); buf.push(0); // empty portal buf.extend_from_slice(&0i32.to_be_bytes()); // max_rows = 0 @@ -741,8 +741,8 @@ mod tests { // Add 5 identical DataRow messages for _ in 0..5 { buf.push(b'D'); - let len = 4 + 2 + 4 + 1; // len + num_cols + col_len + data - buf.extend_from_slice(&(len as i32).to_be_bytes()); + let len: i32 = 4 + 2 + 4 + 1; // len + num_cols + col_len + data + buf.extend_from_slice(&len.to_be_bytes()); buf.extend_from_slice(&1i16.to_be_bytes()); // 1 column buf.extend_from_slice(&1i32.to_be_bytes()); // col length = 1 buf.push(b'X'); // data diff --git a/tests/bdd/doorman_helper.rs b/tests/bdd/doorman_helper.rs index 0a762c5b..fb8e2f38 100644 --- a/tests/bdd/doorman_helper.rs +++ b/tests/bdd/doorman_helper.rs @@ -193,7 +193,6 @@ pub fn stop_doorman(child: &mut Child) { // Process already exited, just clean up pipes drop(child.stdout.take()); drop(child.stderr.take()); - return; } Ok(None) => { // Process still running, stop it diff --git a/tests/bdd/extended/helpers.rs b/tests/bdd/extended/helpers.rs index 9a53c41e..aa13f8ff 100644 --- a/tests/bdd/extended/helpers.rs +++ b/tests/bdd/extended/helpers.rs @@ -131,7 +131,7 @@ pub(crate) fn find_column_index(header_line: &str, column_name: &str) -> (usize, } /// Split a table row using the detected separator. -pub(crate) fn split_row<'a>(line: &'a str, use_pipe: bool) -> Vec<&'a str> { +pub(crate) fn split_row(line: &str, use_pipe: bool) -> Vec<&str> { if use_pipe { line.split('|').map(|s| s.trim()).collect() } else { diff --git a/tests/bdd/fuzz_client.rs b/tests/bdd/fuzz_client.rs index 18812d97..acc26e4d 100644 --- a/tests/bdd/fuzz_client.rs +++ b/tests/bdd/fuzz_client.rs @@ -138,7 +138,7 @@ impl FuzzClient { let stmt_name = b"test_stmt\0"; let query = b"SELECT 1\0"; let parse_len = 4 + stmt_name.len() + query.len() + 2; // +2 for param count (i16) - stream.write_all(&[b'P']).await?; + stream.write_all(b"P").await?; stream.write_all(&(parse_len as i32).to_be_bytes()).await?; stream.write_all(stmt_name).await?; stream.write_all(query).await?; @@ -147,7 +147,7 @@ impl FuzzClient { // Skip Bind, go directly to Execute - protocol violation! let portal = b"\0"; // unnamed portal let execute_len = 4 + portal.len() + 4; // +4 for max_rows - stream.write_all(&[b'E']).await?; + stream.write_all(b"E").await?; stream .write_all(&(execute_len as i32).to_be_bytes()) .await?; @@ -169,7 +169,7 @@ impl FuzzClient { let portal = b"\0"; // unnamed portal let stmt_name = b"nonexistent_statement\0"; let bind_len = 4 + portal.len() + stmt_name.len() + 2 + 2 + 2; // format codes + params + result formats - stream.write_all(&[b'B']).await?; + stream.write_all(b"B").await?; stream.write_all(&(bind_len as i32).to_be_bytes()).await?; stream.write_all(portal).await?; stream.write_all(stmt_name).await?; diff --git a/tests/bdd/generate_helper.rs b/tests/bdd/generate_helper.rs index 6888a64a..879d6147 100644 --- a/tests/bdd/generate_helper.rs +++ b/tests/bdd/generate_helper.rs @@ -133,13 +133,11 @@ fn patch_toml_config(content: &str, port: u16) -> String { // Add pg_hba trust rule at the end of the [general] section // Find where to insert: after [general] block, before [pools] if let Some(pools_pos) = output.find("\n[pools]") { - let insert = format!("\n[general.pg_hba]\ncontent = \"host all all 127.0.0.1/32 trust\"\n"); - output.insert_str(pools_pos, &insert); + let insert = "\n[general.pg_hba]\ncontent = \"host all all 127.0.0.1/32 trust\"\n"; + output.insert_str(pools_pos, insert); } else { // Fallback: append at the end - output.push_str(&format!( - "\n[general.pg_hba]\ncontent = \"host all all 127.0.0.1/32 trust\"\n" - )); + output.push_str("\n[general.pg_hba]\ncontent = \"host all all 127.0.0.1/32 trust\"\n"); } output From 34e0d9a23848ee1b6f9437aff94f953ecc65bdf0 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 10:17:45 +0300 Subject: [PATCH 08/24] Extend Fargate bench timeout to 3 hours and download full log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: previous run hit the 120-minute polling timeout at 124 minutes of ECS RUNNING state — the bench had finished test 226/240 and only needed ~7 more minutes. Adding the unix socket suite (90 new pgbench runs at -T 30s each) pushed the wall-clock past the old 120-min ceiling. The pre-unix-socket bench took ~1.2 hours, so a 3-hour ceiling gives comfortable headroom for future additions. Also: aws logs get-log-events was called without --start-from-head and without pagination, returning only the tail of CloudWatch events. For a multi-MB log stream this could chop off the ===BEGIN_BENCHMARK_RESULTS=== marker even on a successful run, so the markdown file was at risk of silent truncation. Now we paginate from the start until nextForwardToken stops advancing. --- .github/workflows/bench-aws-fargate.yml | 40 ++++++++++++++++++++----- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/.github/workflows/bench-aws-fargate.yml b/.github/workflows/bench-aws-fargate.yml index a44da417..8cd799d3 100644 --- a/.github/workflows/bench-aws-fargate.yml +++ b/.github/workflows/bench-aws-fargate.yml @@ -325,7 +325,7 @@ jobs: # Wait for task to complete with a custom loop (longer timeout) echo "Waiting for task to complete..." - MAX_ATTEMPTS=240 # 240 * 30s = 120 minutes + MAX_ATTEMPTS=360 # 360 * 30s = 180 minutes (3 hours) ATTEMPT=0 LOG_GROUP="/ecs/${CLUSTER}" TASK_ID=$(echo "${TASK_ARN}" | awk -F/ '{print $NF}') @@ -373,7 +373,7 @@ jobs: done if [ $ATTEMPT -eq $MAX_ATTEMPTS ]; then - echo "❌ Error: Timeout waiting for task to complete after 120 minutes." + echo "❌ Error: Timeout waiting for task to complete after 180 minutes." # Try to stop the task if it's still running aws ecs stop-task --cluster "${CLUSTER}" --task "${TASK_ARN}" --reason "Timeout in GitHub Actions" || true exit 1 @@ -426,11 +426,37 @@ jobs: LOG_ATTEMPT=$((LOG_ATTEMPT+1)) done - # Download logs using jq to ensure each event is on a new line - aws logs get-log-events \ - --log-group-name "${LOG_GROUP}" \ - --log-stream-name "${LOG_STREAM}" \ - --output json | jq -r '.events[].message' > benchmark-results/stdout.txt + # Download ALL logs from the start, paginated, since a 2+ hour bench + # produces several MB of CloudWatch events. Without --start-from-head and + # without paginating, aws-cli only returns the tail, which can chop off + # the ===BEGIN_BENCHMARK_RESULTS=== marker that lives in the middle. + : > benchmark-results/stdout.txt + NEXT_TOKEN="" + while :; do + if [ -z "${NEXT_TOKEN}" ]; then + PAGE=$(aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --start-from-head \ + --output json) + else + PAGE=$(aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --start-from-head \ + --next-token "${NEXT_TOKEN}" \ + --output json) + fi + EVENTS=$(echo "${PAGE}" | jq '.events | length') + echo "${PAGE}" | jq -r '.events[].message' >> benchmark-results/stdout.txt + NEW_TOKEN=$(echo "${PAGE}" | jq -r '.nextForwardToken') + # CloudWatch returns the same forward token when there are no more events + if [ "${EVENTS}" -eq 0 ] || [ "${NEW_TOKEN}" = "${NEXT_TOKEN}" ]; then + break + fi + NEXT_TOKEN="${NEW_TOKEN}" + done + echo "Downloaded $(wc -l < benchmark-results/stdout.txt) log lines" echo "Extracting benchmark results from logs..." From 7c96ac6dcc2d33ca87390bb55ac2789d28939d99 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 12:39:02 +0300 Subject: [PATCH 09/24] Filter Fargate logs server-side and add time window Needed: a 2-hour bench produces hundreds of MB of CloudWatch events (the unix-socket odyssey instance alone logs every connection accept and disconnect). Paginating the whole stream from the start with get-log-events takes 30+ minutes and times out the workflow before the artifact is uploaded. What changed: switched to filter-log-events which accepts a CloudWatch filter pattern and a time window. Two new optional inputs let the caller skip the noisy setup phase or grab a specific minute range. With a focused pattern (e.g. "?Scenario ?pgbench ?error") the workflow finishes in under a minute even on the largest streams. --- .github/workflows/fetch-fargate-logs.yml | 111 +++++++++++++++++++---- 1 file changed, 93 insertions(+), 18 deletions(-) diff --git a/.github/workflows/fetch-fargate-logs.yml b/.github/workflows/fetch-fargate-logs.yml index 55835eff..31346f2d 100644 --- a/.github/workflows/fetch-fargate-logs.yml +++ b/.github/workflows/fetch-fargate-logs.yml @@ -10,11 +10,27 @@ on: description: 'ECS task ID (the UUID part, e.g. 8f1d948c60c74239998e8be8dee87ea7)' required: true type: string + filter_pattern: + description: 'CloudWatch Logs filter pattern (empty = no filter, e.g. "?ERROR ?pgbench ?Scenario")' + required: false + default: '' + type: string + start_minute: + description: 'Skip first N minutes after task start (helps avoid setup spam)' + required: false + default: '0' + type: string + end_minute: + description: 'Stop fetching after N minutes from task start (0 = no limit)' + required: false + default: '0' + type: string jobs: fetch-logs: name: Fetch CloudWatch Logs runs-on: ubuntu-latest + timeout-minutes: 30 steps: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@v4 @@ -23,8 +39,14 @@ jobs: aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Download full log stream from CloudWatch + - name: Download log stream from CloudWatch + env: + FILTER_PATTERN: ${{ inputs.filter_pattern }} + START_MINUTE: ${{ inputs.start_minute }} + END_MINUTE: ${{ inputs.end_minute }} run: | + set -euo pipefail + CLUSTER="${{ secrets.ECS_CLUSTER_NAME }}" TASK_ID="${{ inputs.task_id }}" LOG_GROUP="/ecs/${CLUSTER}" @@ -36,35 +58,88 @@ jobs: echo "Fetching ${LOG_GROUP}/${LOG_STREAM}" + # Find the timestamp of the first event in the stream so start/end offsets + # are interpreted relative to "task start" rather than wall-clock epoch. + FIRST_TS=$(aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --start-from-head \ + --limit 1 \ + --output json | jq '.events[0].timestamp // 0') + if [ "${FIRST_TS}" = "0" ]; then + echo "❌ Log stream is empty or unreachable" + exit 1 + fi + echo "First event timestamp: ${FIRST_TS} ($(date -u -d @$((FIRST_TS / 1000)) +%FT%TZ))" + + START_TS=$((FIRST_TS + START_MINUTE * 60 * 1000)) + if [ "${END_MINUTE}" -gt 0 ]; then + END_TS=$((FIRST_TS + END_MINUTE * 60 * 1000)) + END_ARG=(--end-time "${END_TS}") + echo "Window: [+${START_MINUTE}min, +${END_MINUTE}min] (start=${START_TS}, end=${END_TS})" + else + END_ARG=() + echo "Window: [+${START_MINUTE}min, end] (start=${START_TS})" + fi + + # Use filter-log-events when a pattern is supplied — it pre-filters on the + # AWS side, so we don't pull half a gigabyte of odyssey spam through the + # paginator. Without a pattern, fall back to filter-log-events with no + # pattern, which still respects the time window. NEXT_TOKEN="" PAGE_NUM=0 + TOTAL=0 while :; do - if [ -z "${NEXT_TOKEN}" ]; then - PAGE=$(aws logs get-log-events \ - --log-group-name "${LOG_GROUP}" \ - --log-stream-name "${LOG_STREAM}" \ - --start-from-head \ - --output json) + if [ -n "${NEXT_TOKEN}" ]; then + if [ -n "${FILTER_PATTERN}" ]; then + PAGE=$(aws logs filter-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-names "${LOG_STREAM}" \ + --start-time "${START_TS}" \ + "${END_ARG[@]}" \ + --filter-pattern "${FILTER_PATTERN}" \ + --next-token "${NEXT_TOKEN}" \ + --output json) + else + PAGE=$(aws logs filter-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-names "${LOG_STREAM}" \ + --start-time "${START_TS}" \ + "${END_ARG[@]}" \ + --next-token "${NEXT_TOKEN}" \ + --output json) + fi else - PAGE=$(aws logs get-log-events \ - --log-group-name "${LOG_GROUP}" \ - --log-stream-name "${LOG_STREAM}" \ - --start-from-head \ - --next-token "${NEXT_TOKEN}" \ - --output json) + if [ -n "${FILTER_PATTERN}" ]; then + PAGE=$(aws logs filter-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-names "${LOG_STREAM}" \ + --start-time "${START_TS}" \ + "${END_ARG[@]}" \ + --filter-pattern "${FILTER_PATTERN}" \ + --output json) + else + PAGE=$(aws logs filter-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-names "${LOG_STREAM}" \ + --start-time "${START_TS}" \ + "${END_ARG[@]}" \ + --output json) + fi fi EVENTS=$(echo "${PAGE}" | jq '.events | length') echo "${PAGE}" | jq -r '.events[] | "\(.timestamp) \(.message)"' >> "${OUT}" - NEW_TOKEN=$(echo "${PAGE}" | jq -r '.nextForwardToken') - PAGE_NUM=$((PAGE_NUM+1)) - echo "page ${PAGE_NUM}: +${EVENTS} events (total $(wc -l < "${OUT}"))" - if [ "${EVENTS}" -eq 0 ] || [ "${NEW_TOKEN}" = "${NEXT_TOKEN}" ]; then + TOTAL=$((TOTAL + EVENTS)) + PAGE_NUM=$((PAGE_NUM + 1)) + echo "page ${PAGE_NUM}: +${EVENTS} events (total ${TOTAL})" + NEW_TOKEN=$(echo "${PAGE}" | jq -r '.nextToken // empty') + if [ -z "${NEW_TOKEN}" ]; then break fi NEXT_TOKEN="${NEW_TOKEN}" done - echo "Downloaded $(wc -l < "${OUT}") log lines into ${OUT}" + echo "Downloaded ${TOTAL} log lines into ${OUT}" echo "File size: $(du -h "${OUT}" | cut -f1)" - name: Upload logs as artifact From 1f9b791d2e02f4b9348ae865c1273fb09fe25e7a Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 13:53:13 +0300 Subject: [PATCH 10/24] Locate benchmark markers via filter-log-events instead of paginating Needed: the previous fix paginated the entire CloudWatch stream from the start to avoid truncating the markdown result. For a 2-hour bench the stream is hundreds of MB (mostly odyssey accept/disconnect spam), so the download step crawled for 30+ minutes and could exceed the GitHub Actions runner timeout. What changed: a single filter-log-events call locates the timestamp of the ===BEGIN_BENCHMARK_RESULTS=== marker server-side, then we only fetch events from that timestamp onward and stop as soon as the END marker lands in the file. The download finishes in seconds regardless of stream size while still capturing the full markdown payload. If the marker is missing (bench did not finish) we fall back to the last 200 events for context. --- .github/workflows/bench-aws-fargate.yml | 82 ++++++++++++++++--------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/.github/workflows/bench-aws-fargate.yml b/.github/workflows/bench-aws-fargate.yml index 8cd799d3..9d423062 100644 --- a/.github/workflows/bench-aws-fargate.yml +++ b/.github/workflows/bench-aws-fargate.yml @@ -426,36 +426,60 @@ jobs: LOG_ATTEMPT=$((LOG_ATTEMPT+1)) done - # Download ALL logs from the start, paginated, since a 2+ hour bench - # produces several MB of CloudWatch events. Without --start-from-head and - # without paginating, aws-cli only returns the tail, which can chop off - # the ===BEGIN_BENCHMARK_RESULTS=== marker that lives in the middle. + # The bench writes the markdown file as a base64 block bracketed by + # ===BEGIN_BENCHMARK_RESULTS=== / ===END_BENCHMARK_RESULTS===, and these + # markers always sit at the very end of the run. Locate the BEGIN marker + # via filter-log-events (server-side, single request) and then download + # only the events after that timestamp. This avoids paginating hundreds + # of MB of CloudWatch events from the noisy odyssey logs. : > benchmark-results/stdout.txt - NEXT_TOKEN="" - while :; do - if [ -z "${NEXT_TOKEN}" ]; then - PAGE=$(aws logs get-log-events \ - --log-group-name "${LOG_GROUP}" \ - --log-stream-name "${LOG_STREAM}" \ - --start-from-head \ - --output json) - else - PAGE=$(aws logs get-log-events \ - --log-group-name "${LOG_GROUP}" \ - --log-stream-name "${LOG_STREAM}" \ - --start-from-head \ - --next-token "${NEXT_TOKEN}" \ - --output json) - fi - EVENTS=$(echo "${PAGE}" | jq '.events | length') - echo "${PAGE}" | jq -r '.events[].message' >> benchmark-results/stdout.txt - NEW_TOKEN=$(echo "${PAGE}" | jq -r '.nextForwardToken') - # CloudWatch returns the same forward token when there are no more events - if [ "${EVENTS}" -eq 0 ] || [ "${NEW_TOKEN}" = "${NEXT_TOKEN}" ]; then - break - fi - NEXT_TOKEN="${NEW_TOKEN}" - done + MARKER_TS=$(aws logs filter-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-names "${LOG_STREAM}" \ + --filter-pattern '"===BEGIN_BENCHMARK_RESULTS==="' \ + --output json | jq '.events[0].timestamp // empty') + + if [ -z "${MARKER_TS}" ]; then + echo "❌ ===BEGIN_BENCHMARK_RESULTS=== marker not found in log stream" + echo "The benchmark scenario likely did not finish — fetching last 200 events for context" + aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --limit 200 \ + --output json | jq -r '.events[].message' >> benchmark-results/stdout.txt + else + echo "Found BEGIN marker at timestamp ${MARKER_TS}" + NEXT_TOKEN="" + while :; do + if [ -z "${NEXT_TOKEN}" ]; then + PAGE=$(aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --start-time "${MARKER_TS}" \ + --start-from-head \ + --output json) + else + PAGE=$(aws logs get-log-events \ + --log-group-name "${LOG_GROUP}" \ + --log-stream-name "${LOG_STREAM}" \ + --start-time "${MARKER_TS}" \ + --start-from-head \ + --next-token "${NEXT_TOKEN}" \ + --output json) + fi + EVENTS=$(echo "${PAGE}" | jq '.events | length') + echo "${PAGE}" | jq -r '.events[].message' >> benchmark-results/stdout.txt + # Stop as soon as the END marker is in the file + if grep -q "===END_BENCHMARK_RESULTS===" benchmark-results/stdout.txt; then + break + fi + NEW_TOKEN=$(echo "${PAGE}" | jq -r '.nextForwardToken') + if [ "${EVENTS}" -eq 0 ] || [ "${NEW_TOKEN}" = "${NEXT_TOKEN}" ]; then + break + fi + NEXT_TOKEN="${NEW_TOKEN}" + done + fi echo "Downloaded $(wc -l < benchmark-results/stdout.txt) log lines" echo "Extracting benchmark results from logs..." From e938b3c9371833fc8c7b8311147999c77d765658 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 17:15:28 +0300 Subject: [PATCH 11/24] Merge 3.4.1 section into 3.4.0 in changelog Needed: 3.4.0 is not released yet, so a 3.4.1 entry on top of it was incorrect. The items attributed to 3.4.1 (log readability, auth failure IP logging, replenish noise, runtime log level control, etc.) all belong to the same unreleased 3.4.0 train. --- documentation/en/src/changelog.md | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index 940f7951..8c4052f9 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -15,7 +15,19 @@ - Unix listener not handed off during `SIGUSR2` binary upgrade. New process re-creates the socket; connections refused for ~100ms. - `only_ssl_connections` does not reject Unix socket connections. Unix sockets do not need TLS for transport security. -### 3.4.1 Apr 3, 2026 +### 3.4.0 Apr 1, 2026 + +**New Features:** + +- **Pool Coordinator — database-level connection limits.** New `max_db_connections` setting caps total server connections per database across all user pools. When the limit is reached, the coordinator evicts idle connections from users with the largest surplus (respecting `min_guaranteed_pool_size`), then waits for a connection to be returned, and falls back to a reserve pool as last resort. Disabled by default (`max_db_connections = 0`) — zero overhead when not configured. Five new pool-level config fields: `max_db_connections`, `min_connection_lifetime` (eviction age threshold), `reserve_pool_size` (extra slots beyond the limit), `reserve_pool_timeout` (wait before using reserve), `min_guaranteed_pool_size` (per-user eviction protection independent of `min_pool_size`). + +- **`SHOW POOL_COORDINATOR` admin command.** Displays per-database coordinator status: configured limits, current connection count, reserve usage, cumulative evictions, reserve acquisitions, and client exhaustion errors. + +- **Pool Coordinator Prometheus metrics.** Seven new metrics under `pg_doorman_pool_coordinator{type, database}`: `connections` (current), `reserve_in_use` (current), `max_connections` (configured limit), `reserve_pool_size` (configured reserve), `evictions_total`, `reserve_acquisitions_total`, `exhaustions_total` (client errors from full exhaustion — primary pager signal). + +- **Reserve pressure relief.** Idle reserve connections (created under `max_db_connections` pressure) are closed early by the retain cycle once idle longer than `min_connection_lifetime`, returning reserve capacity before the regular `idle_timeout` fires. + +- **Runtime log level control via admin `SET` command.** Change log level without restarting the pooler: `SET log_level = 'debug'` for global, `SET log_level = 'warn,pg_doorman::pool::pool_coordinator=debug'` for per-module (RUST_LOG syntax). View current level with `SHOW LOG_LEVEL`. Changes are ephemeral (lost on restart). Zero overhead on the hot path at production log levels — filtering uses lock-free `ArcSwap` instead of `RwLock`. **Improvements:** @@ -29,26 +41,10 @@ - **Prepared statement cache eviction log.** Shows truncated query text and current cache size (`size=99/100`) to help diagnose cache sizing issues. -**New features:** - -- **Runtime log level control via admin `SET` command.** Change log level without restarting the pooler: `SET log_level = 'debug'` for global, `SET log_level = 'warn,pg_doorman::pool::pool_coordinator=debug'` for per-module (RUST_LOG syntax). View current level with `SHOW LOG_LEVEL`. Changes are ephemeral (lost on restart). Zero overhead on the hot path at production log levels — filtering uses lock-free `ArcSwap` instead of `RwLock`. - **Security:** - **Removed password hash from logs.** The "unsupported password type" warning no longer includes the password hash value. -### 3.4.0 Apr 1, 2026 - -**New Features:** - -- **Pool Coordinator — database-level connection limits.** New `max_db_connections` setting caps total server connections per database across all user pools. When the limit is reached, the coordinator evicts idle connections from users with the largest surplus (respecting `min_guaranteed_pool_size`), then waits for a connection to be returned, and falls back to a reserve pool as last resort. Disabled by default (`max_db_connections = 0`) — zero overhead when not configured. Five new pool-level config fields: `max_db_connections`, `min_connection_lifetime` (eviction age threshold), `reserve_pool_size` (extra slots beyond the limit), `reserve_pool_timeout` (wait before using reserve), `min_guaranteed_pool_size` (per-user eviction protection independent of `min_pool_size`). - -- **`SHOW POOL_COORDINATOR` admin command.** Displays per-database coordinator status: configured limits, current connection count, reserve usage, cumulative evictions, reserve acquisitions, and client exhaustion errors. - -- **Pool Coordinator Prometheus metrics.** Seven new metrics under `pg_doorman_pool_coordinator{type, database}`: `connections` (current), `reserve_in_use` (current), `max_connections` (configured limit), `reserve_pool_size` (configured reserve), `evictions_total`, `reserve_acquisitions_total`, `exhaustions_total` (client errors from full exhaustion — primary pager signal). - -- **Reserve pressure relief.** Idle reserve connections (created under `max_db_connections` pressure) are closed early by the retain cycle once idle longer than `min_connection_lifetime`, returning reserve capacity before the regular `idle_timeout` fires. - ### 3.3.5 Mar 31, 2026 **Bug Fixes:** From 50d61085253761c391a5e2141324c2b784689b29 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 17:18:03 +0300 Subject: [PATCH 12/24] Move Unreleased changelog items into 3.4.0 Needed: 3.4.0 is still unreleased, so a separate Unreleased section above it was redundant. The Unix socket listener, HBA local rule matching, and unix_socket_mode entries all belong to the same 3.4.0 release train. --- documentation/en/src/changelog.md | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index 8c4052f9..654c1454 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -1,8 +1,8 @@ # Changelog -### Unreleased +### 3.4.0 Apr 1, 2026 -**New features:** +**New Features:** - **Unix socket listener.** `unix_socket_dir` creates `.s.PGSQL.` socket file. Connect with `psql -h ` or `pgbench -h `. No TCP overhead on local connections. @@ -10,15 +10,6 @@ - **`unix_socket_mode` controls socket file permissions.** New `[general]` setting fixes the permission bits on `.s.PGSQL.` after bind, so the access surface no longer depends on the process umask. Octal string, default `"0600"` (owner only). Set to `"0660"` to grant a Unix group, or `"0666"` to allow any local user. Validated at config load — invalid octal values, setuid/setgid/sticky bits, and overflow into bits above `0o777` are rejected upfront. -**Known limitations (Unix socket):** - -- Unix listener not handed off during `SIGUSR2` binary upgrade. New process re-creates the socket; connections refused for ~100ms. -- `only_ssl_connections` does not reject Unix socket connections. Unix sockets do not need TLS for transport security. - -### 3.4.0 Apr 1, 2026 - -**New Features:** - - **Pool Coordinator — database-level connection limits.** New `max_db_connections` setting caps total server connections per database across all user pools. When the limit is reached, the coordinator evicts idle connections from users with the largest surplus (respecting `min_guaranteed_pool_size`), then waits for a connection to be returned, and falls back to a reserve pool as last resort. Disabled by default (`max_db_connections = 0`) — zero overhead when not configured. Five new pool-level config fields: `max_db_connections`, `min_connection_lifetime` (eviction age threshold), `reserve_pool_size` (extra slots beyond the limit), `reserve_pool_timeout` (wait before using reserve), `min_guaranteed_pool_size` (per-user eviction protection independent of `min_pool_size`). - **`SHOW POOL_COORDINATOR` admin command.** Displays per-database coordinator status: configured limits, current connection count, reserve usage, cumulative evictions, reserve acquisitions, and client exhaustion errors. @@ -45,6 +36,11 @@ - **Removed password hash from logs.** The "unsupported password type" warning no longer includes the password hash value. +**Known limitations (Unix socket):** + +- Unix listener not handed off during `SIGUSR2` binary upgrade. New process re-creates the socket; connections refused for ~100ms. +- `only_ssl_connections` does not reject Unix socket connections. Unix sockets do not need TLS for transport security. + ### 3.3.5 Mar 31, 2026 **Bug Fixes:** From 5e59254d87a4b5dea3cf98485ff5ade974cbcd5b Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 18:30:01 +0300 Subject: [PATCH 13/24] Cover check_hba is_unix branch with unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the new is_unix parameter turned check_hba into a security-critical matcher (local rules gate Unix sockets, host* rules gate TCP). The only existing tests exercised TCP paths, so any regression in the unix_socket listener's authentication path would have passed CI. What changed: thirteen unit tests spread across src/auth/hba.rs and src/config/tests.rs pin down the new semantics — local vs host rule transport matching, reject-before-trust ordering via Unix, scram-only rules returning NotMatched for md5 clients, and the legacy general.hba fallback allowing all Unix clients while still enforcing CIDR limits on TCP clients. The legacy branch became reachable from tests after extracting check_hba_with_general, a pure helper that takes an explicit General snapshot instead of pulling from the global config. --- src/auth/hba.rs | 131 ++++++++++++++++++++++++++++++++++++++++++++ src/config/mod.rs | 31 ++++++++++- src/config/tests.rs | 62 +++++++++++++++++++++ 3 files changed, 221 insertions(+), 3 deletions(-) diff --git a/src/auth/hba.rs b/src/auth/hba.rs index 382bd053..cae66636 100644 --- a/src/auth/hba.rs +++ b/src/auth/hba.rs @@ -617,4 +617,135 @@ hostnossl all all 127.0.0.1/32 trust let expected = "host all all 10.0.0.0/8 md5\nhostssl all all 192.168.0.0/16 scram-sha-256\nhostnossl all all 127.0.0.1/32 trust"; assert_eq!(s, expected); } + + // ---- is_unix semantics: local rules match Unix sockets, host* rules match TCP ---- + + // Placeholder IP supplied by the client entrypoint for Unix connections. + // `check_hba` ignores the IP entirely when `is_unix=true`, but callers still + // have to pass something; we document the convention here. + fn unix_ip() -> IpAddr { + IpAddr::V4(Ipv4Addr::LOCALHOST) + } + + #[test] + fn local_trust_matches_unix_client() { + let hba = PgHba::from_content("local all all trust"); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::Trust + ); + } + + #[test] + fn local_trust_does_not_match_tcp_client() { + // Complement to local_trust_matches_unix_client: the same rule must + // stay invisible to TCP clients so the existing hba_eval_tests remain + // correct and nobody can shadow host rules by adding a local entry. + let hba = PgHba::from_content("local all all trust"); + let ip = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); + assert_eq!( + hba.check_hba(ip, false, false, "md5", "alice", "app"), + CheckResult::NotMatched + ); + } + + #[test] + fn host_rule_does_not_match_unix_client() { + // host* rules may contain a matching CIDR, but Unix connections must + // never be authenticated by them — only by `local` entries. + let hba = PgHba::from_content("host all all 0.0.0.0/0 md5"); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::NotMatched + ); + } + + #[test] + fn hostssl_rule_ignored_for_unix_even_when_ssl_true() { + // Defensive: the entrypoint never passes ssl=true together with + // is_unix=true, but the matcher should still reject the rule rather + // than honour it silently if that invariant ever leaks. + let hba = PgHba::from_content("hostssl all all 0.0.0.0/0 trust"); + assert_eq!( + hba.check_hba(unix_ip(), true, true, "scram-sha-256", "alice", "app"), + CheckResult::NotMatched + ); + } + + #[test] + fn local_reject_blocks_named_user_via_unix() { + // Rule order matches PostgreSQL: the first rule that applies wins, + // so bob is rejected while alice falls through to the trust rule. + let hba = PgHba::from_content("local all bob reject\nlocal all all trust"); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "bob", "app"), + CheckResult::Deny + ); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::Trust + ); + } + + #[test] + fn local_md5_rule_allows_unix_md5_client() { + let hba = PgHba::from_content("local all all md5"); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::Allow + ); + } + + #[test] + fn local_scram_rule_returns_not_matched_for_md5_request() { + // A local rule with a stricter auth method must not grant access to a + // weaker requested method — we return NotMatched so the caller can + // fall through to the next rule or deny. + let hba = PgHba::from_content("local all all scram-sha-256"); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::NotMatched + ); + } + + #[test] + fn local_and_host_rules_are_independent_decisions() { + // Single file, both transports: unix follows `local`, TCP follows `host`. + let hba = PgHba::from_content("local all all trust\nhost all all 0.0.0.0/0 md5"); + let ip = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::Trust + ); + assert_eq!( + hba.check_hba(ip, false, false, "md5", "alice", "app"), + CheckResult::Allow + ); + } + + #[test] + fn local_database_filter_narrows_match() { + // Database scoping is orthogonal to the transport check: a local rule + // for "admin" must not authenticate a client connecting to "app". + let hba = PgHba::from_content("local admin all trust\nlocal all all reject"); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "admin"), + CheckResult::Trust + ); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::Deny + ); + } + + #[test] + fn empty_hba_returns_not_matched_for_unix() { + // A configured-but-empty PgHba must fall through — the upstream + // caller decides what to do, it should not be silently promoted. + let hba = PgHba::from_content(""); + assert_eq!( + hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + CheckResult::NotMatched + ); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 6f19ba57..9caadbf0 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -570,17 +570,42 @@ pub fn check_hba( database: &str, ) -> CheckResult { let config = get_config(); - if let Some(ref pg) = config.general.pg_hba { + check_hba_with_general( + &config.general, + ip, + ssl, + is_unix, + type_auth, + username, + database, + ) +} + +/// Pure evaluation of HBA rules against an explicit [`General`] snapshot. +/// +/// Split out of [`check_hba`] so that unit tests can exercise the legacy +/// `general.hba` branches — including the `is_unix` bypass — without +/// touching the global config. +pub(crate) fn check_hba_with_general( + general: &General, + ip: IpAddr, + ssl: bool, + is_unix: bool, + type_auth: &str, + username: &str, + database: &str, +) -> CheckResult { + if let Some(ref pg) = general.pg_hba { return pg.check_hba(ip, ssl, is_unix, type_auth, username, database); } // Legacy hba list has no unix concept — allow all unix connections if is_unix { return CheckResult::Allow; } - if config.general.hba.is_empty() { + if general.hba.is_empty() { return CheckResult::Allow; } - if config.general.hba.iter().any(|net| net.contains(&ip)) { + if general.hba.iter().any(|net| net.contains(&ip)) { CheckResult::Allow } else { CheckResult::NotMatched diff --git a/src/config/tests.rs b/src/config/tests.rs index cc1bb40b..95a5d6e3 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -1660,3 +1660,65 @@ async fn test_validate_reserve_pool_timeout_skipped_when_coordinator_disabled() // Coordinator disabled → no cross-config check assert!(config.validate().await.is_ok()); } + +// ---- check_hba_with_general: legacy general.hba + is_unix semantics ---- + +#[test] +fn check_hba_legacy_empty_allows_unix() { + // Legacy branch, nothing configured: any transport is allowed. Kept as a + // baseline so the next two tests document what changes once `is_unix` + // enters the picture. + let general = General::default(); + let ip: IpAddr = "10.0.0.5".parse().unwrap(); + assert_eq!( + check_hba_with_general(&general, ip, false, true, "md5", "alice", "app"), + CheckResult::Allow + ); + assert_eq!( + check_hba_with_general(&general, ip, false, false, "md5", "alice", "app"), + CheckResult::Allow + ); +} + +#[test] +fn check_hba_legacy_list_bypassed_for_unix() { + // Reproduces the "silent privilege expansion" case from review: the + // operator restricts TCP access with a CIDR whitelist, but Unix clients + // must still be allowed because the legacy list has no transport concept. + let mut general = General::default(); + general.hba = vec!["10.0.0.0/8".parse().unwrap()]; + let ip: IpAddr = "192.168.1.10".parse().unwrap(); + + // Unix: Allow regardless of source IP + assert_eq!( + check_hba_with_general(&general, ip, false, true, "md5", "alice", "app"), + CheckResult::Allow + ); + // TCP from an IP outside the whitelist: NotMatched + assert_eq!( + check_hba_with_general(&general, ip, false, false, "md5", "alice", "app"), + CheckResult::NotMatched + ); + // TCP from an IP inside the whitelist: Allow + let ip_inside: IpAddr = "10.1.2.3".parse().unwrap(); + assert_eq!( + check_hba_with_general(&general, ip_inside, false, false, "md5", "alice", "app"), + CheckResult::Allow + ); +} + +#[test] +fn check_hba_pg_hba_takes_precedence_over_legacy_for_unix() { + // When pg_hba is configured the legacy list must be ignored entirely; + // `local` rules drive the decision for Unix clients. + use crate::auth::hba::PgHba; + let mut general = General::default(); + general.hba = vec!["10.0.0.0/8".parse().unwrap()]; + general.pg_hba = Some(PgHba::from_content("local all all reject")); + let ip: IpAddr = "127.0.0.1".parse().unwrap(); + + assert_eq!( + check_hba_with_general(&general, ip, false, true, "md5", "alice", "app"), + CheckResult::Deny + ); +} From 0d3c709da7de05983a90995d86de4377e5a38d75 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 19:23:18 +0300 Subject: [PATCH 14/24] Close chmod race on Unix socket startup via umask guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the listener bind path created the socket file at umask-derived rights (usually 0644), then ratcheted them down to unix_socket_mode via set_permissions. The window between bind and chmod allowed a concurrent local client to reach the listener with weaker permissions than the operator configured, silently bypassing the intended restriction and contradicting the changelog promise that the socket "does not depend on the process umask". What changed: before bind() the process temporarily tightens its umask to cover every bit the configured mode forbids, then restores the old umask once the file is in place. set_permissions still runs afterwards so operators can loosen the mode (e.g. 0660 with a group bit) — the guard only enforces a floor. Two serial unit tests pin the guard's restore-on-drop and bit-preservation behaviour, and two new BDD scenarios stat the socket file to verify both the 0600 default and the 0660 override are honoured end-to-end. --- src/app/server.rs | 103 ++++++++++++++++++++++--- tests/bdd/doorman_helper.rs | 33 ++++++++ tests/bdd/features/unix-socket.feature | 67 ++++++++++++++++ 3 files changed, 192 insertions(+), 11 deletions(-) diff --git a/src/app/server.rs b/src/app/server.rs index 4aeb3699..a30848e9 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -210,15 +210,24 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { - // Apply the configured permission mode after bind so the socket - // file does not depend on the process umask. The mode string was - // already validated at config load time. - let mode = crate::config::General::parse_unix_socket_mode( - &config.general.unix_socket_mode, - ) - .expect("unix_socket_mode validated at config load"); + drop(_umask_guard); use std::os::unix::fs::PermissionsExt; match std::fs::set_permissions( &path, @@ -226,14 +235,12 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { info!( - "Unix socket listening on {path} (mode={:#o})", - mode + "Unix socket listening on {path} (mode={mode:#o})" ); } Err(err) => { error!( - "Failed to set mode {:#o} on Unix socket {path}: {err}", - mode + "Failed to set mode {mode:#o} on Unix socket {path}: {err}" ); std::process::exit(exitcode::OSERR); } @@ -873,3 +880,77 @@ fn spawn_shutdown_timer(exit_tx: mpsc::Sender<()>, shutdown_timeout: Duration) { } }); } + +/// Temporarily tighten the process umask for the lifetime of the guard. +/// +/// The Unix listener startup needs the socket inode to be created with no +/// weaker permissions than the configured `unix_socket_mode`. Since `bind()` +/// applies `0666 & ~umask` at the moment the file appears in the filesystem, +/// we ratchet the umask up, perform the bind, then restore the original +/// value on drop. The guard is also safe to drop explicitly once the socket +/// is in place and `set_permissions` has run. +#[cfg(unix)] +struct UmaskGuard { + previous: libc::mode_t, +} + +#[cfg(unix)] +impl UmaskGuard { + /// Ensure the process umask masks at least `additional_bits` on top of + /// whatever was already set. + fn restrict(additional_bits: libc::mode_t) -> Self { + // SAFETY: umask is a process-global knob; we snapshot the current + // value by setting a known mask, OR in our extra bits, and restore + // it on drop. No Rust invariants are touched. + let previous = unsafe { libc::umask(0o777) }; + unsafe { libc::umask(previous | additional_bits) }; + Self { previous } + } +} + +#[cfg(unix)] +impl Drop for UmaskGuard { + fn drop(&mut self) { + // SAFETY: same rationale as `restrict`; we only touch the umask. + unsafe { libc::umask(self.previous) }; + } +} + +#[cfg(test)] +mod umask_guard_tests { + use super::UmaskGuard; + use serial_test::serial; + + #[test] + #[serial] + fn restore_previous_umask_on_drop() { + let prior = unsafe { libc::umask(0o022) }; + { + let _guard = UmaskGuard::restrict(0o077); + let inside = unsafe { libc::umask(0o777) }; + unsafe { libc::umask(inside) }; + assert_eq!( + inside & 0o077, + 0o077, + "guard must ensure the restrict bits are set" + ); + } + let after = unsafe { libc::umask(0o022) }; + assert_eq!(after, 0o022, "drop must restore the original umask"); + unsafe { libc::umask(prior) }; + } + + #[test] + #[serial] + fn restrict_preserves_existing_bits() { + let prior = unsafe { libc::umask(0o027) }; + { + let _guard = UmaskGuard::restrict(0o050); + let inside = unsafe { libc::umask(0o777) }; + unsafe { libc::umask(inside) }; + // Prior bits (027) AND new bits (050) must both be present. + assert_eq!(inside & 0o077, 0o077); + } + unsafe { libc::umask(prior) }; + } +} diff --git a/tests/bdd/doorman_helper.rs b/tests/bdd/doorman_helper.rs index fb8e2f38..0a4c317b 100644 --- a/tests/bdd/doorman_helper.rs +++ b/tests/bdd/doorman_helper.rs @@ -312,6 +312,39 @@ async fn wait_for_daemon_port_ready(port: u16) { } /// Check if PID file contains correct daemon PID +/// Assert the permission bits on the pg_doorman Unix socket file match the +/// expected octal mode. Uses the temp dir + doorman port the world already +/// knows to locate the socket, so the step works for any scenario that +/// configured `unix_socket_dir = "${PG_TEMP_DIR}"`. +#[then(regex = r#"pg_doorman unix socket file has mode "([0-7]{3,4})""#)] +pub async fn verify_unix_socket_mode(world: &mut DoormanWorld, expected_mode: String) { + use std::os::unix::fs::PermissionsExt; + + let dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir must be set") + .path(); + let port = world.doorman_port.expect("doorman_port must be set"); + let path = dir.join(format!(".s.PGSQL.{port}")); + + let metadata = + std::fs::metadata(&path).unwrap_or_else(|e| panic!("stat {}: {e}", path.display())); + let actual_mode = metadata.permissions().mode() & 0o777; + + let expected = u32::from_str_radix(expected_mode.trim_start_matches('0'), 8) + .expect("expected mode must be a valid octal literal"); + + assert_eq!( + actual_mode, + expected, + "unix socket {} has mode {:#o}, expected {:#o}", + path.display(), + actual_mode, + expected + ); +} + #[then(regex = r#"PID file "([^"]+)" should contain running daemon PID"#)] pub async fn verify_pid_file(_world: &mut DoormanWorld, pid_path: String) { let pid_content = std::fs::read_to_string(&pid_path).expect("Failed to read PID file"); diff --git a/tests/bdd/features/unix-socket.feature b/tests/bdd/features/unix-socket.feature index cd6a296e..b4694e68 100644 --- a/tests/bdd/features/unix-socket.feature +++ b/tests/bdd/features/unix-socket.feature @@ -34,3 +34,70 @@ Feature: Unix socket connections """ # pg_backend_pid > 0 proves the query was executed by a real PostgreSQL backend Then psql query "SELECT pg_backend_pid() > 0" via pg_doorman unix socket as user "postgres" to database "postgres" returns "t" + + Scenario: Unix socket file gets the default 0600 permission bits + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And pg_doorman hba file contains: + """ + local all all trust + host all all 0.0.0.0/0 trust + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + unix_socket_dir = "${PG_TEMP_DIR}" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.postgres.users]] + username = "postgres" + password = "" + pool_size = 10 + """ + Then pg_doorman unix socket file has mode "0600" + + Scenario: Unix socket file honours configured unix_socket_mode + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And pg_doorman hba file contains: + """ + local all all trust + host all all 0.0.0.0/0 trust + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + unix_socket_dir = "${PG_TEMP_DIR}" + unix_socket_mode = "0660" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.postgres.users]] + username = "postgres" + password = "" + pool_size = 10 + """ + Then pg_doorman unix socket file has mode "0660" From 1240615e159d46f3307f43f72845771640162527 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 20:05:13 +0300 Subject: [PATCH 15/24] Refuse to unlink a live Unix socket at startup Needed: the listener bring-up called remove_file unconditionally, so pointing unix_socket_dir at a shared directory such as /var/run/postgresql would silently delete another service's live socket and cut off every local client before pg_doorman even bound its own. What changed: a dedicated prepare_unix_socket_path helper stats the target, probes it with a local connect, and only unlinks the file when the probe is refused (stale leftover from a crash). If a process is still listening, the helper returns a descriptive error and run_server exits with OSERR instead of clobbering the foreign inode. Three unit tests cover the missing-file, stale-file, and live-listener branches. --- src/app/server.rs | 83 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/src/app/server.rs b/src/app/server.rs index a30848e9..cec77820 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -209,7 +209,10 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box, shutdown_timeout: Duration) { }); } +/// Prepare a Unix socket path for bind() by clearing any stale file without +/// clobbering a live peer. +/// +/// The previous implementation called `remove_file` unconditionally, which +/// meant pointing pg_doorman at a shared directory like `/var/run/postgresql` +/// could silently delete another process's live socket. This helper instead: +/// +/// 1. Returns Ok if nothing exists at the path. +/// 2. Attempts a connect — if it succeeds, a live peer owns the socket and +/// we refuse to touch it so the caller can fail loudly. +/// 3. Otherwise removes the stale inode (typical case after a crash). +/// +/// Errors are returned as strings with enough context for the caller to log +/// and exit; unit tests exercise the three branches without touching the +/// process umask or the real server bring-up. +#[cfg(unix)] +fn prepare_unix_socket_path(path: &str) -> Result<(), String> { + use std::os::unix::net::UnixStream; + + match std::fs::symlink_metadata(path) { + Ok(_) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(err) => return Err(format!("stat failed: {err}")), + } + + // Short probe: a local Unix connect that would succeed does so in + // microseconds. If it refuses, the socket is stale (no listener bound to + // the inode) and we can reclaim it. + match UnixStream::connect(path) { + Ok(_) => Err(format!( + "another process is already listening on {path}; refusing to remove it" + )), + Err(_) => std::fs::remove_file(path) + .map_err(|err| format!("failed to remove stale socket {path}: {err}")), + } +} + /// Temporarily tighten the process umask for the lifetime of the guard. /// /// The Unix listener startup needs the socket inode to be created with no @@ -916,6 +956,47 @@ impl Drop for UmaskGuard { } } +#[cfg(test)] +mod prepare_unix_socket_path_tests { + use super::prepare_unix_socket_path; + use std::os::unix::net::UnixListener; + use tempfile::tempdir; + + #[test] + fn missing_path_is_ok() { + let dir = tempdir().unwrap(); + let path = dir.path().join("missing.sock"); + assert!(prepare_unix_socket_path(path.to_str().unwrap()).is_ok()); + } + + #[test] + fn stale_file_is_removed() { + // A regular file (not a live listener) simulates a post-crash leftover + // — prepare_unix_socket_path should clean it up silently. + let dir = tempdir().unwrap(); + let path = dir.path().join("stale.sock"); + std::fs::write(&path, b"leftover").unwrap(); + assert!(path.exists()); + + prepare_unix_socket_path(path.to_str().unwrap()).expect("stale file must be removable"); + assert!(!path.exists(), "stale socket file must be removed"); + } + + #[test] + fn live_listener_is_preserved() { + // Bind a real UnixListener in a temp dir; the helper must refuse to + // touch it and return a descriptive error. + let dir = tempdir().unwrap(); + let path = dir.path().join("live.sock"); + let _listener = UnixListener::bind(&path).unwrap(); + + let err = prepare_unix_socket_path(path.to_str().unwrap()) + .expect_err("live socket must trigger an error"); + assert!(err.contains("already listening"), "unexpected error: {err}"); + assert!(path.exists(), "live socket file must stay on disk"); + } +} + #[cfg(test)] mod umask_guard_tests { use super::UmaskGuard; From 8426c34379011f86ac8cf65949227b24d270ba84 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 20:10:51 +0300 Subject: [PATCH 16/24] Warn when legacy general.hba cannot protect Unix socket clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: when an operator migrates to the Unix listener but keeps the legacy general.hba CIDR whitelist, the IP-based rule no longer covers local clients — check_hba_with_general returns Allow for every Unix connection. The silent privilege expansion is only visible by reading the HBA matcher source, so an operator can easily miss it. What changed: Config::validate now calls legacy_hba_bypassed_by_unix_socket and emits a warn! that tells the operator to move to pg_hba with explicit `local` rules if they want the whitelist to keep working. The check lives in a dedicated helper so unit tests cover the four branches (bypass detected, no unix dir, empty legacy list, pg_hba overrides) without touching the log stream. --- src/config/mod.rs | 22 +++++++++++++++++++++- src/config/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index 9caadbf0..e371e906 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -4,7 +4,7 @@ //! for the connection pooler. use arc_swap::ArcSwap; -use log::{error, info}; +use log::{error, info, warn}; use once_cell::sync::Lazy; use serde_derive::{Deserialize, Serialize}; use std::collections::HashMap; @@ -376,6 +376,18 @@ impl Config { )); } + // Legacy general.hba is an IP-based whitelist and has no transport + // concept, so Unix socket clients unconditionally fall through to + // Allow in check_hba_with_general. Warn the operator loudly rather + // than silently granting access to anyone with filesystem reach. + if legacy_hba_bypassed_by_unix_socket(&self.general) { + warn!( + "general.hba restricts TCP clients by CIDR but does not apply to Unix socket \ + clients — any local process able to connect to the socket file will bypass the \ + IP whitelist. Switch to pg_hba with explicit `local` rules to cover this path." + ); + } + // Validate prepared_statements if self.general.prepared_statements && self.general.prepared_statements_cache_size == 0 { return Err(Error::BadConfig("The value of prepared_statements_cache should be greater than 0 if prepared_statements are enabled".to_string())); @@ -581,6 +593,14 @@ pub fn check_hba( ) } +/// True when the operator enabled a Unix listener alongside the legacy +/// IP-based `general.hba` whitelist, without a `pg_hba` snippet to cover +/// the `local` transport. In this shape Unix clients bypass the CIDR +/// check entirely — see `check_hba_with_general`. +pub(crate) fn legacy_hba_bypassed_by_unix_socket(general: &General) -> bool { + general.unix_socket_dir.is_some() && general.pg_hba.is_none() && !general.hba.is_empty() +} + /// Pure evaluation of HBA rules against an explicit [`General`] snapshot. /// /// Split out of [`check_hba`] so that unit tests can exercise the legacy diff --git a/src/config/tests.rs b/src/config/tests.rs index 95a5d6e3..d371ed60 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -1722,3 +1722,40 @@ fn check_hba_pg_hba_takes_precedence_over_legacy_for_unix() { CheckResult::Deny ); } + +// ---- legacy_hba_bypassed_by_unix_socket: silent privilege expansion detector ---- + +#[test] +fn legacy_hba_bypass_detected_when_unix_dir_set_and_legacy_hba_present() { + let mut general = General::default(); + general.unix_socket_dir = Some("/tmp".to_string()); + general.hba = vec!["10.0.0.0/8".parse().unwrap()]; + assert!(legacy_hba_bypassed_by_unix_socket(&general)); +} + +#[test] +fn legacy_hba_bypass_quiet_without_unix_socket_dir() { + let mut general = General::default(); + general.hba = vec!["10.0.0.0/8".parse().unwrap()]; + // No unix listener → operator's CIDR whitelist applies to every client. + assert!(!legacy_hba_bypassed_by_unix_socket(&general)); +} + +#[test] +fn legacy_hba_bypass_quiet_without_legacy_entries() { + let mut general = General::default(); + general.unix_socket_dir = Some("/tmp".to_string()); + // Empty legacy hba means there is no rule to bypass in the first place. + assert!(!legacy_hba_bypassed_by_unix_socket(&general)); +} + +#[test] +fn legacy_hba_bypass_quiet_when_pg_hba_present() { + use crate::auth::hba::PgHba; + let mut general = General::default(); + general.unix_socket_dir = Some("/tmp".to_string()); + general.hba = vec!["10.0.0.0/8".parse().unwrap()]; + general.pg_hba = Some(PgHba::from_content("local all all trust")); + // pg_hba takes precedence and has explicit local rules — no silent bypass. + assert!(!legacy_hba_bypassed_by_unix_socket(&general)); +} From d5327a5ed14f859c9d95bbc5e7e794c6b682675c Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 20:24:51 +0300 Subject: [PATCH 17/24] Reply with PostgreSQL error when a Unix client hits max_connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the Unix accept branch dropped the socket the moment CURRENT_CLIENT_COUNT exceeded max_connections, so psql and similar clients saw "server closed the connection unexpectedly" and operators went hunting for a segfault or an OOM. The TCP branch had the right behaviour for years — it consumes the startup packet and returns a proper 53300 ErrorResponse — and Unix needs to match it. What changed: a new client_entrypoint_too_many_clients_already_unix reads the startup message (refusing a stray SSL request with a clear 08P01) and writes the same "sorry, too many clients already" error response that TCP emits. The Unix accept branch calls it instead of silently returning, so max_connections rejection is now observable end-to-end for both transports. Test coverage for the rejection BDD scenario is intentionally deferred: the scenario needs two concurrent Unix clients to cross the limit deterministically, and the current BDD harness has no helper for parallel psql processes. Follow-up will land once that helper exists. --- src/app/server.rs | 8 ++++++++ src/client/entrypoint.rs | 41 ++++++++++++++++++++++++++++++++++++++++ src/client/mod.rs | 3 ++- 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/app/server.rs b/src/app/server.rs index cec77820..b7c3bddb 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -548,6 +548,14 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box max_connections { warn!("[#c{connection_id}] unix client rejected: too many clients (current={current_clients}, max={max_connections})"); + if let Err(err) = crate::client::client_entrypoint_too_many_clients_already_unix( + socket, + connection_id, + ) + .await + { + warn!("[#c{connection_id}] unix client rejection response failed: {err}"); + } CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); return; } diff --git a/src/client/entrypoint.rs b/src/client/entrypoint.rs index 30df3687..b2d3d4b2 100644 --- a/src/client/entrypoint.rs +++ b/src/client/entrypoint.rs @@ -71,6 +71,47 @@ pub async fn client_entrypoint_too_many_clients_already( Ok(()) } +/// Reject an inbound Unix socket client with a proper PostgreSQL ErrorResponse. +/// +/// The TCP rejection path above also handles TLS and cancel-request startups, +/// neither of which applies here: Unix sockets do not negotiate TLS, and +/// cancel requests through Unix are not forwarded to a backend at startup +/// time because they carry no addr to pair with the running client. We still +/// consume the startup bytes so the client reads our ErrorResponse instead +/// of seeing a bare EOF — the symptom the operator saw in the max_connections +/// regression. +pub async fn client_entrypoint_too_many_clients_already_unix( + mut stream: UnixStream, + connection_id: u64, +) -> Result<(), Error> { + match get_startup::(&mut stream).await { + Ok((ClientConnectionType::Tls, _)) => { + // Unix sockets never negotiate TLS; mirror the main Unix entrypoint + // and refuse the SSL request with the same error message. + error_response_terminal( + &mut stream, + "TLS is not supported on Unix socket connections", + "08P01", + ) + .await?; + return Ok(()); + } + Ok((ClientConnectionType::Startup, _)) => (), + Ok((ClientConnectionType::CancelQuery, _)) => { + // A cancel request arriving while the server is at capacity is a + // no-op: we have no worker slot to forward it to. Report back the + // same "too many clients" error so the client sees a structured + // response instead of EOF. + } + Err(err) => { + warn!("[#c{connection_id}] unix client bad startup: {err}"); + return Err(err); + } + } + error_response_terminal(&mut stream, "sorry, too many clients already", "53300").await?; + Ok(()) +} + /// Client entrypoint. Returns session identity on success for disconnect logging. pub async fn client_entrypoint( mut stream: TcpStream, diff --git a/src/client/mod.rs b/src/client/mod.rs index e8080f82..a4313e62 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -10,7 +10,8 @@ mod util; pub use core::Client; pub use entrypoint::{ - client_entrypoint, client_entrypoint_too_many_clients_already, client_entrypoint_unix, + client_entrypoint, client_entrypoint_too_many_clients_already, + client_entrypoint_too_many_clients_already_unix, client_entrypoint_unix, }; pub use startup::startup_tls; pub use util::PREPARED_STATEMENT_COUNTER; From f56fb0401db22ac8684163f78beaf64445f9eb1a Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 20:32:29 +0300 Subject: [PATCH 18/24] Skip Unix socket cleanup when another process already owns the inode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: on SIGUSR2 binary upgrade the successor rebinds the same .s.PGSQL. file before the parent reaches its shutdown path. The parent then unlinked the path unconditionally, wiping the successor's inode and leaving the new listener nameless — every subsequent Unix client would see ENOENT. The changelog admitted a ~100ms refusal gap, but the real outcome was losing the Unix listener until a full restart. What changed: right after bind+chmod the listener startup captures the (dev, ino) of the socket file via a new UnixSocketOwnership helper. At shutdown we stat the path again and only call remove_file when the inode still matches; if it has changed we log that another process owns the path and leave the file alone. The decision lives in a pure UnixSocketOwnership::inspect helper covered by six unit tests — match, replacement, missing, skip-on-stat-error, plus a round-trip that actually re-binds the same path to exercise the full "successor replaces inode" scenario. --- src/app/server.rs | 303 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 253 insertions(+), 50 deletions(-) diff --git a/src/app/server.rs b/src/app/server.rs index b7c3bddb..00dea44c 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -207,57 +207,65 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { - drop(_umask_guard); - use std::os::unix::fs::PermissionsExt; - match std::fs::set_permissions( - &path, - std::fs::Permissions::from_mode(mode), - ) { - Ok(()) => { - info!( - "Unix socket listening on {path} (mode={mode:#o})" - ); - } - Err(err) => { + // + // `unix_socket_ownership` captures the dev/ino of the inode we + // create here so the shutdown path can tell our socket apart from + // one bound by a successor process (binary upgrade via SIGUSR2: + // the child re-creates the file before the parent reaches cleanup). + let (unix_listener, unix_socket_ownership) = + if let Some(ref dir) = config.general.unix_socket_dir { + let path = format!("{dir}/.s.PGSQL.{}", config.general.port); + if let Err(err) = prepare_unix_socket_path(&path) { + error!("Cannot reuse Unix socket path {path}: {err}"); + std::process::exit(exitcode::OSERR); + } + let mode = crate::config::General::parse_unix_socket_mode( + &config.general.unix_socket_mode, + ) + .expect("unix_socket_mode validated at config load"); + + // Clamp the umask so the socket inode created by bind() never + // exists with weaker permissions than `mode`. Without this a + // concurrent client connecting in the window between bind() + // and set_permissions() would land on the umask-derived + // rights (typically 0644) and bypass the configured + // restriction. set_permissions() still runs afterwards so + // callers can loosen the mode (e.g. 0660 with a group bit). + let restrict_bits = !(mode & 0o777) & 0o777; + let _umask_guard = UmaskGuard::restrict(restrict_bits as libc::mode_t); + + match tokio::net::UnixListener::bind(&path) { + Ok(l) => { + drop(_umask_guard); + use std::os::unix::fs::PermissionsExt; + if let Err(err) = + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)) + { error!( "Failed to set mode {mode:#o} on Unix socket {path}: {err}" ); std::process::exit(exitcode::OSERR); } + let ownership = match UnixSocketOwnership::capture(&path) { + Ok(o) => o, + Err(err) => { + error!( + "Failed to stat Unix socket {path} after bind: {err}" + ); + std::process::exit(exitcode::OSERR); + } + }; + info!("Unix socket listening on {path} (mode={mode:#o})"); + (Some(l), Some(ownership)) + } + Err(err) => { + error!("Failed to bind Unix socket {path}: {err}"); + (None, None) } - Some(l) - } - Err(err) => { - error!("Failed to bind Unix socket {path}: {err}"); - None } - } - } else { - None - }; + } else { + (None, None) + }; config.show(); @@ -594,12 +602,22 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box {} + UnixSocketCleanup::Missing => {} + UnixSocketCleanup::Skipped { reason } => { + info!( + "Leaving Unix socket {} in place: {reason}", + ownership.path + ); + } + UnixSocketCleanup::Failed { err } => { + warn!("Failed to remove Unix socket {}: {err}", ownership.path); } } } @@ -892,6 +910,94 @@ fn spawn_shutdown_timer(exit_tx: mpsc::Sender<()>, shutdown_timeout: Duration) { }); } +/// Identity of a Unix socket file this process bound to, captured as +/// `(dev, ino)` plus the original path. Used to decide at shutdown whether +/// the inode on disk is still ours or has been replaced by a successor +/// process during a binary upgrade. +#[cfg(unix)] +#[derive(Debug, Clone)] +struct UnixSocketOwnership { + path: String, + dev: u64, + ino: u64, +} + +#[cfg(unix)] +#[derive(Debug, PartialEq, Eq)] +enum UnixSocketCleanup { + /// The inode matched; the file has been removed. + Removed, + /// Nothing was on disk at the captured path. + Missing, + /// A different inode sits at the path — a successor rebound it. + Skipped { reason: String }, + /// Removal was attempted but the syscall returned an error. + Failed { err: String }, +} + +#[cfg(unix)] +impl UnixSocketOwnership { + /// Stat the path and remember `(dev, ino)` so future cleanup can verify + /// the inode has not been replaced. + fn capture(path: &str) -> Result { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::metadata(path)?; + Ok(Self { + path: path.to_string(), + dev: meta.dev(), + ino: meta.ino(), + }) + } + + /// Remove the socket file only if the inode on disk still matches the + /// one captured at `capture` time. + fn cleanup_if_ours(&self) -> UnixSocketCleanup { + match Self::inspect(&self.path, self.dev, self.ino) { + CleanupDecision::Remove => match std::fs::remove_file(&self.path) { + Ok(()) => UnixSocketCleanup::Removed, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + UnixSocketCleanup::Missing + } + Err(err) => UnixSocketCleanup::Failed { + err: err.to_string(), + }, + }, + CleanupDecision::Missing => UnixSocketCleanup::Missing, + CleanupDecision::Skip(reason) => UnixSocketCleanup::Skipped { reason }, + } + } + + /// Pure decision function: given a path and the expected `(dev, ino)`, + /// should the caller proceed to unlink the file? Split out so the logic + /// can be unit-tested without touching real filesystem state. + fn inspect(path: &str, expected_dev: u64, expected_ino: u64) -> CleanupDecision { + use std::os::unix::fs::MetadataExt; + match std::fs::symlink_metadata(path) { + Ok(meta) => { + let dev = meta.dev(); + let ino = meta.ino(); + if dev == expected_dev && ino == expected_ino { + CleanupDecision::Remove + } else { + CleanupDecision::Skip(format!( + "inode changed (expected dev={expected_dev} ino={expected_ino}, found dev={dev} ino={ino}); another process owns the path now" + )) + } + } + Err(err) if err.kind() == std::io::ErrorKind::NotFound => CleanupDecision::Missing, + Err(err) => CleanupDecision::Skip(format!("stat failed: {err}")), + } + } +} + +#[cfg(unix)] +#[derive(Debug, PartialEq, Eq)] +enum CleanupDecision { + Remove, + Missing, + Skip(String), +} + /// Prepare a Unix socket path for bind() by clearing any stale file without /// clobbering a live peer. /// @@ -964,6 +1070,103 @@ impl Drop for UmaskGuard { } } +#[cfg(test)] +mod unix_socket_ownership_tests { + use super::{CleanupDecision, UnixSocketCleanup, UnixSocketOwnership}; + use std::os::unix::net::UnixListener; + use tempfile::tempdir; + + #[test] + fn capture_and_cleanup_round_trip() { + let dir = tempdir().unwrap(); + let path = dir.path().join("owned.sock"); + let _listener = UnixListener::bind(&path).unwrap(); + + let ownership = UnixSocketOwnership::capture(path.to_str().unwrap()) + .expect("capture must succeed right after bind"); + assert_eq!(ownership.cleanup_if_ours(), UnixSocketCleanup::Removed); + assert!(!path.exists(), "our socket file must be removed"); + } + + #[test] + fn cleanup_skips_replaced_inode() { + // Simulate a binary upgrade: capture the current inode, then swap + // the file out of the way and create a new one at the same path. + // The successor's inode must NOT be removed by the parent. + let dir = tempdir().unwrap(); + let path = dir.path().join("shared.sock"); + let _original = UnixListener::bind(&path).unwrap(); + let ownership = UnixSocketOwnership::capture(path.to_str().unwrap()).unwrap(); + + drop(_original); + std::fs::remove_file(&path).unwrap(); + let successor = UnixListener::bind(&path).unwrap(); + + match ownership.cleanup_if_ours() { + UnixSocketCleanup::Skipped { reason } => { + assert!( + reason.contains("inode changed"), + "unexpected reason: {reason}" + ); + } + other => panic!("expected Skipped, got {other:?}"), + } + assert!(path.exists(), "successor's socket must be preserved"); + drop(successor); + } + + #[test] + fn cleanup_reports_missing_when_already_removed() { + let dir = tempdir().unwrap(); + let path = dir.path().join("gone.sock"); + let _listener = UnixListener::bind(&path).unwrap(); + let ownership = UnixSocketOwnership::capture(path.to_str().unwrap()).unwrap(); + + std::fs::remove_file(&path).unwrap(); + assert_eq!(ownership.cleanup_if_ours(), UnixSocketCleanup::Missing); + } + + #[test] + fn inspect_remove_on_exact_match() { + let dir = tempdir().unwrap(); + let path = dir.path().join("inspect.sock"); + let _listener = UnixListener::bind(&path).unwrap(); + let ownership = UnixSocketOwnership::capture(path.to_str().unwrap()).unwrap(); + + assert_eq!( + UnixSocketOwnership::inspect(path.to_str().unwrap(), ownership.dev, ownership.ino), + CleanupDecision::Remove + ); + } + + #[test] + fn inspect_skip_on_mismatched_ino() { + let dir = tempdir().unwrap(); + let path = dir.path().join("inspect2.sock"); + let _listener = UnixListener::bind(&path).unwrap(); + let ownership = UnixSocketOwnership::capture(path.to_str().unwrap()).unwrap(); + + // Pretend we captured a different inode to simulate replacement. + let fake_ino = ownership.ino.wrapping_add(1); + match UnixSocketOwnership::inspect(path.to_str().unwrap(), ownership.dev, fake_ino) { + CleanupDecision::Skip(reason) => { + assert!(reason.contains("inode changed"), "unexpected: {reason}"); + } + other => panic!("expected Skip, got {other:?}"), + } + } + + #[test] + fn inspect_missing_when_no_file() { + let dir = tempdir().unwrap(); + let path = dir.path().join("nope.sock"); + assert_eq!( + UnixSocketOwnership::inspect(path.to_str().unwrap(), 0, 0), + CleanupDecision::Missing + ); + } +} + #[cfg(test)] mod prepare_unix_socket_path_tests { use super::prepare_unix_socket_path; From 030fcda3a565692fa6eb667b84e379ed42402f4a Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 20:47:05 +0300 Subject: [PATCH 19/24] Extract Unix listener startup into a testable function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: run_server inlined ~60 lines of bind+chmod+ownership+umask plumbing, which made the failure modes (missing directory, stale socket, chmod denied) unreachable from unit tests and noisy to read. What changed: create_unix_listener now owns the whole bring-up — prepare path, raise the umask, bind, drop the guard, chmod, capture ownership — and returns a Result so the caller keeps the exit-on- error policy without leaking OS exit codes into the helper. Three tokio tests exercise the happy path, the missing-directory error, and the group-readable mode override in a tempdir, so regressions in any of those branches trip CI instead of a production bring-up. --- src/app/server.rs | 161 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 112 insertions(+), 49 deletions(-) diff --git a/src/app/server.rs b/src/app/server.rs index 00dea44c..2ada523c 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -206,66 +206,33 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { let path = format!("{dir}/.s.PGSQL.{}", config.general.port); - if let Err(err) = prepare_unix_socket_path(&path) { - error!("Cannot reuse Unix socket path {path}: {err}"); - std::process::exit(exitcode::OSERR); - } let mode = crate::config::General::parse_unix_socket_mode( &config.general.unix_socket_mode, ) .expect("unix_socket_mode validated at config load"); - - // Clamp the umask so the socket inode created by bind() never - // exists with weaker permissions than `mode`. Without this a - // concurrent client connecting in the window between bind() - // and set_permissions() would land on the umask-derived - // rights (typically 0644) and bypass the configured - // restriction. set_permissions() still runs afterwards so - // callers can loosen the mode (e.g. 0660 with a group bit). - let restrict_bits = !(mode & 0o777) & 0o777; - let _umask_guard = UmaskGuard::restrict(restrict_bits as libc::mode_t); - - match tokio::net::UnixListener::bind(&path) { - Ok(l) => { - drop(_umask_guard); - use std::os::unix::fs::PermissionsExt; - if let Err(err) = - std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)) - { - error!( - "Failed to set mode {mode:#o} on Unix socket {path}: {err}" - ); - std::process::exit(exitcode::OSERR); - } - let ownership = match UnixSocketOwnership::capture(&path) { - Ok(o) => o, - Err(err) => { - error!( - "Failed to stat Unix socket {path} after bind: {err}" - ); - std::process::exit(exitcode::OSERR); - } - }; + match create_unix_listener(&path, mode) { + Ok((listener, ownership)) => { info!("Unix socket listening on {path} (mode={mode:#o})"); - (Some(l), Some(ownership)) + (Some(listener), Some(ownership)) } Err(err) => { - error!("Failed to bind Unix socket {path}: {err}"); - (None, None) + error!("{err}"); + std::process::exit(exitcode::OSERR); } } - } else { - (None, None) - }; + } + None => (None, None), + }; config.show(); @@ -998,6 +965,46 @@ enum CleanupDecision { Skip(String), } +/// Create a Tokio Unix socket listener at `path` with the given permission +/// `mode`. +/// +/// This is the whole bring-up sequence the pooler runs at startup, factored +/// out of `run_server` so unit tests can reproduce the failure modes (stale +/// file, dead-end directory, chmod failure) in a tempdir without launching a +/// full server. On success the returned [`UnixSocketOwnership`] records the +/// (dev, ino) of the inode so the shutdown path can decide whether the +/// successor of a binary upgrade has already replaced it. +#[cfg(unix)] +fn create_unix_listener( + path: &str, + mode: u32, +) -> Result<(tokio::net::UnixListener, UnixSocketOwnership), String> { + prepare_unix_socket_path(path) + .map_err(|err| format!("Cannot reuse Unix socket path {path}: {err}"))?; + + // Clamp the umask so the socket inode created by bind() never exists with + // weaker permissions than `mode`. Without this a concurrent client + // connecting in the window between bind() and set_permissions() would + // land on the umask-derived rights (typically 0644) and bypass the + // configured restriction. set_permissions() still runs afterwards so + // callers can loosen the mode (e.g. 0660 with a group bit). + let restrict_bits = !(mode & 0o777) & 0o777; + let _umask_guard = UmaskGuard::restrict(restrict_bits as libc::mode_t); + + let listener = tokio::net::UnixListener::bind(path) + .map_err(|err| format!("Failed to bind Unix socket {path}: {err}"))?; + drop(_umask_guard); + + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)) + .map_err(|err| format!("Failed to set mode {mode:#o} on Unix socket {path}: {err}"))?; + + let ownership = UnixSocketOwnership::capture(path) + .map_err(|err| format!("Failed to stat Unix socket {path} after bind: {err}"))?; + + Ok((listener, ownership)) +} + /// Prepare a Unix socket path for bind() by clearing any stale file without /// clobbering a live peer. /// @@ -1070,6 +1077,62 @@ impl Drop for UmaskGuard { } } +#[cfg(test)] +mod create_unix_listener_tests { + use super::create_unix_listener; + use std::os::unix::fs::PermissionsExt; + use tempfile::tempdir; + + #[tokio::test] + async fn binds_and_applies_mode() { + let dir = tempdir().unwrap(); + let path = dir.path().join(".s.PGSQL.6432"); + let path_str = path.to_str().unwrap(); + + let (listener, ownership) = + create_unix_listener(path_str, 0o600).expect("bind must succeed in empty tempdir"); + + let meta = std::fs::metadata(path_str).unwrap(); + assert_eq!(meta.permissions().mode() & 0o777, 0o600); + assert_eq!(ownership.path, path_str); + + drop(listener); + } + + #[tokio::test] + async fn bind_fails_when_directory_missing() { + // Directory we never created → bind must return a structured error + // instead of panicking or exiting the process. + let dir = tempdir().unwrap(); + let path = dir + .path() + .join("does") + .join("not") + .join("exist") + .join(".s.PGSQL.6432"); + + let err = create_unix_listener(path.to_str().unwrap(), 0o600) + .expect_err("bind must fail when parent directory is missing"); + assert!(err.contains("Failed to bind"), "unexpected error: {err}"); + } + + #[tokio::test] + async fn group_readable_mode_is_applied() { + // 0660 exercises the path where set_permissions *loosens* the bits + // the umask guard masked off; if we mess that up the file stays + // owner-only and client groups lose access silently. + let dir = tempdir().unwrap(); + let path = dir.path().join(".s.PGSQL.6432"); + + let (listener, _ownership) = + create_unix_listener(path.to_str().unwrap(), 0o660).expect("bind must succeed"); + + let meta = std::fs::metadata(&path).unwrap(); + assert_eq!(meta.permissions().mode() & 0o777, 0o660); + drop(listener); + } +} + #[cfg(test)] mod unix_socket_ownership_tests { use super::{CleanupDecision, UnixSocketCleanup, UnixSocketOwnership}; From c2f587cf1c52874224466871977e2536cc2e642a Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 21:48:49 +0300 Subject: [PATCH 20/24] Replace is_unix bool with ClientTransport enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the HBA matcher and Client::startup took a (SocketAddr, ssl, is_unix) triplet with two unrelated booleans sitting next to each other — a classic swap-me footgun. On the Unix path callers had to manufacture a 127.0.0.1:0 "fake_addr" to satisfy the TCP-shaped signature, which then leaked into SHOW CLIENTS rows and HBA error messages and made operators think unix clients were localhost TCP. What changed: a new top-level ClientTransport enum (Tcp { peer, ssl } or Unix) is threaded through PgHba::check_hba, check_hba_with_general, the public config::check_hba wrapper, Client::startup, and both client_entrypoint variants. is_unix(), is_tls(), hba_ip() and peer_display() keep the call sites readable. Unix clients no longer construct fake_addr at the entrypoint layer — the sentinel lives exclusively inside Client::startup for struct-level compatibility while a follow-up teaches Client to store a typed peer. Existing HBA unit tests and config legacy-hba tests were ported to the new API (nothing lost), the hba_eval_tests suite now builds its ClientIdentifier via ClientTransport::Tcp, and test modules that touch the filesystem were marked `#[serial]` to coexist with the umask guard tests. 423 lib tests stay green. --- src/app/server.rs | 18 +++++++ src/auth/hba.rs | 97 ++++++++++++++++++----------------- src/auth/hba_eval_tests.rs | 8 +-- src/client/entrypoint.rs | 30 +++++++---- src/client/startup.rs | 47 +++++++++-------- src/config/mod.rs | 27 +++------- src/config/tests.rs | 31 +++++++----- src/lib.rs | 1 + src/transport.rs | 101 +++++++++++++++++++++++++++++++++++++ 9 files changed, 246 insertions(+), 114 deletions(-) create mode 100644 src/transport.rs diff --git a/src/app/server.rs b/src/app/server.rs index 2ada523c..b212c85b 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -1080,10 +1080,15 @@ impl Drop for UmaskGuard { #[cfg(test)] mod create_unix_listener_tests { use super::create_unix_listener; + use serial_test::serial; use std::os::unix::fs::PermissionsExt; use tempfile::tempdir; + // Serialised because the umask_guard_tests in this crate flip the + // process umask to 0o777 while running; any concurrent tempdir-backed + // bind() would land on an inaccessible file and fail with EACCES. #[tokio::test] + #[serial] async fn binds_and_applies_mode() { let dir = tempdir().unwrap(); let path = dir.path().join(".s.PGSQL.6432"); @@ -1100,6 +1105,7 @@ mod create_unix_listener_tests { } #[tokio::test] + #[serial] async fn bind_fails_when_directory_missing() { // Directory we never created → bind must return a structured error // instead of panicking or exiting the process. @@ -1117,6 +1123,7 @@ mod create_unix_listener_tests { } #[tokio::test] + #[serial] async fn group_readable_mode_is_applied() { // 0660 exercises the path where set_permissions *loosens* the bits // the umask guard masked off; if we mess that up the file stays @@ -1136,10 +1143,12 @@ mod create_unix_listener_tests { #[cfg(test)] mod unix_socket_ownership_tests { use super::{CleanupDecision, UnixSocketCleanup, UnixSocketOwnership}; + use serial_test::serial; use std::os::unix::net::UnixListener; use tempfile::tempdir; #[test] + #[serial] fn capture_and_cleanup_round_trip() { let dir = tempdir().unwrap(); let path = dir.path().join("owned.sock"); @@ -1152,6 +1161,7 @@ mod unix_socket_ownership_tests { } #[test] + #[serial] fn cleanup_skips_replaced_inode() { // Simulate a binary upgrade: capture the current inode, then swap // the file out of the way and create a new one at the same path. @@ -1179,6 +1189,7 @@ mod unix_socket_ownership_tests { } #[test] + #[serial] fn cleanup_reports_missing_when_already_removed() { let dir = tempdir().unwrap(); let path = dir.path().join("gone.sock"); @@ -1190,6 +1201,7 @@ mod unix_socket_ownership_tests { } #[test] + #[serial] fn inspect_remove_on_exact_match() { let dir = tempdir().unwrap(); let path = dir.path().join("inspect.sock"); @@ -1203,6 +1215,7 @@ mod unix_socket_ownership_tests { } #[test] + #[serial] fn inspect_skip_on_mismatched_ino() { let dir = tempdir().unwrap(); let path = dir.path().join("inspect2.sock"); @@ -1220,6 +1233,7 @@ mod unix_socket_ownership_tests { } #[test] + #[serial] fn inspect_missing_when_no_file() { let dir = tempdir().unwrap(); let path = dir.path().join("nope.sock"); @@ -1233,10 +1247,12 @@ mod unix_socket_ownership_tests { #[cfg(test)] mod prepare_unix_socket_path_tests { use super::prepare_unix_socket_path; + use serial_test::serial; use std::os::unix::net::UnixListener; use tempfile::tempdir; #[test] + #[serial] fn missing_path_is_ok() { let dir = tempdir().unwrap(); let path = dir.path().join("missing.sock"); @@ -1244,6 +1260,7 @@ mod prepare_unix_socket_path_tests { } #[test] + #[serial] fn stale_file_is_removed() { // A regular file (not a live listener) simulates a post-crash leftover // — prepare_unix_socket_path should clean it up silently. @@ -1257,6 +1274,7 @@ mod prepare_unix_socket_path_tests { } #[test] + #[serial] fn live_listener_is_preserved() { // Bind a real UnixListener in a temp dir; the helper must refuse to // touch it and return a descriptive error. diff --git a/src/auth/hba.rs b/src/auth/hba.rs index cae66636..5916f53b 100644 --- a/src/auth/hba.rs +++ b/src/auth/hba.rs @@ -1,9 +1,10 @@ -use std::net::IpAddr; use std::path::Path; use std::{fs, str::FromStr}; use ipnet::IpNet; +use crate::transport::ClientTransport; + /// Authentication method supported by our checker. #[derive(Debug, Clone, PartialEq, Eq)] pub enum AuthMethod { @@ -346,8 +347,8 @@ impl PgHba { /// Evaluate given connection parameters against parsed HBA rules. /// - /// - `ip`: client IP address - /// - `ssl`: whether the client connection is over SSL + /// - `transport`: how the client reached the pooler (TCP + SSL state, + /// or a Unix socket). Drives `local` vs `host*` rule matching. /// - `type_auth`: requested auth method name, e.g. "md5" or "scram-sha-256" /// - `username`: database user name /// - `database`: target database name @@ -358,9 +359,7 @@ impl PgHba { /// otherwise `CheckResult::NotMatched`. pub fn check_hba( &self, - ip: IpAddr, - ssl: bool, - is_unix: bool, + transport: &ClientTransport, type_auth: &str, username: &str, database: &str, @@ -375,20 +374,20 @@ impl PgHba { match rule.host_type { HostType::Local => { // local rules match only Unix socket connections - if !is_unix { + if !transport.is_unix() { continue; } } _ => { // host/hostssl/hostnossl rules match only TCP connections - if is_unix { + if transport.is_unix() { continue; } - if !rule.host_type.matches_ssl(ssl) { + if !rule.host_type.matches_ssl(transport.is_tls()) { continue; } if let Some(net) = &rule.address { - if !net.contains(&ip) { + if !net.contains(&transport.hba_ip()) { continue; } } @@ -455,7 +454,7 @@ fn parse_address(token: &str) -> Option { #[cfg(test)] mod tests { use super::*; - use std::net::{IpAddr, Ipv4Addr}; + use std::net::{IpAddr, Ipv4Addr, SocketAddr}; const SAMPLE: &str = r#" # comment @@ -464,6 +463,17 @@ hostssl all all 192.168.0.0/16 scram-sha-256 hostnossl all all 127.0.0.1/32 trust "#; + fn tcp(peer: IpAddr, ssl: bool) -> ClientTransport { + ClientTransport::Tcp { + peer: SocketAddr::new(peer, 54321), + ssl, + } + } + + fn unix_transport() -> ClientTransport { + ClientTransport::Unix + } + #[test] fn parse_and_check() { let hba = PgHba::from_content(SAMPLE); @@ -472,29 +482,29 @@ hostnossl all all 127.0.0.1/32 trust // md5 allowed for 10.1.2.3 over non-ssl and ssl (host matches both) let ip = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); assert_eq!( - hba.check_hba(ip, false, false, "md5", "alice", "app"), + hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - hba.check_hba(ip, true, false, "md5", "alice", "app"), + hba.check_hba(&tcp(ip, true), "md5", "alice", "app"), CheckResult::Allow ); // scram allowed for 192.168.1.10 only with ssl let ip2 = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 10)); assert_eq!( - hba.check_hba(ip2, true, false, "scram-sha-256", "alice", "app"), + hba.check_hba(&tcp(ip2, true), "scram-sha-256", "alice", "app"), CheckResult::Allow ); assert_eq!( - hba.check_hba(ip2, false, false, "scram-sha-256", "alice", "app"), + hba.check_hba(&tcp(ip2, false), "scram-sha-256", "alice", "app"), CheckResult::NotMatched ); // trust on localhost without ssl let ip3 = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); assert_eq!( - hba.check_hba(ip3, false, false, "md5", "alice", "app"), + hba.check_hba(&tcp(ip3, false), "md5", "alice", "app"), CheckResult::Trust ); } @@ -520,13 +530,13 @@ hostnossl all all 127.0.0.1/32 trust // First rule trust for 127.0.0.1 let ip = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); assert_eq!( - cfg.hba.check_hba(ip, false, false, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Trust ); // Second rule md5 for 10.1.2.3 let ip2 = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); assert_eq!( - cfg.hba.check_hba(ip2, false, false, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip2, false), "md5", "alice", "app"), CheckResult::Allow ); } @@ -539,11 +549,11 @@ hostnossl all all 127.0.0.1/32 trust let cfg: Wrapper = toml::from_str(toml_in).expect("toml parse map content"); let ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); assert_eq!( - cfg.hba.check_hba(ip, false, false, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - cfg.hba.check_hba(ip, true, false, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, true), "md5", "alice", "app"), CheckResult::Allow ); } @@ -567,7 +577,7 @@ hostnossl all all 127.0.0.1/32 trust let ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 2)); assert_eq!( cfg.hba - .check_hba(ip, true, false, "scram-sha-256", "alice", "app"), + .check_hba(&tcp(ip, true), "scram-sha-256", "alice", "app"), CheckResult::Allow ); @@ -605,7 +615,7 @@ hostnossl all all 127.0.0.1/32 trust let cfg: Wrapper = toml::from_str(toml_in).expect("toml parse both fields"); let ip = IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)); assert_eq!( - cfg.hba.check_hba(ip, false, false, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); } @@ -618,20 +628,13 @@ hostnossl all all 127.0.0.1/32 trust assert_eq!(s, expected); } - // ---- is_unix semantics: local rules match Unix sockets, host* rules match TCP ---- - - // Placeholder IP supplied by the client entrypoint for Unix connections. - // `check_hba` ignores the IP entirely when `is_unix=true`, but callers still - // have to pass something; we document the convention here. - fn unix_ip() -> IpAddr { - IpAddr::V4(Ipv4Addr::LOCALHOST) - } + // ---- ClientTransport semantics: local rules match Unix, host* rules match TCP ---- #[test] fn local_trust_matches_unix_client() { let hba = PgHba::from_content("local all all trust"); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::Trust ); } @@ -644,7 +647,7 @@ hostnossl all all 127.0.0.1/32 trust let hba = PgHba::from_content("local all all trust"); let ip = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); assert_eq!( - hba.check_hba(ip, false, false, "md5", "alice", "app"), + hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::NotMatched ); } @@ -655,19 +658,19 @@ hostnossl all all 127.0.0.1/32 trust // never be authenticated by them — only by `local` entries. let hba = PgHba::from_content("host all all 0.0.0.0/0 md5"); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::NotMatched ); } #[test] - fn hostssl_rule_ignored_for_unix_even_when_ssl_true() { - // Defensive: the entrypoint never passes ssl=true together with - // is_unix=true, but the matcher should still reject the rule rather - // than honour it silently if that invariant ever leaks. + fn hostssl_rule_ignored_for_unix() { + // The ClientTransport enum makes it impossible to build a + // `Unix + ssl=true` state, so the matcher only needs to confirm the + // hostssl rule itself does not leak through for Unix transport. let hba = PgHba::from_content("hostssl all all 0.0.0.0/0 trust"); assert_eq!( - hba.check_hba(unix_ip(), true, true, "scram-sha-256", "alice", "app"), + hba.check_hba(&unix_transport(), "scram-sha-256", "alice", "app"), CheckResult::NotMatched ); } @@ -678,11 +681,11 @@ hostnossl all all 127.0.0.1/32 trust // so bob is rejected while alice falls through to the trust rule. let hba = PgHba::from_content("local all bob reject\nlocal all all trust"); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "bob", "app"), + hba.check_hba(&unix_transport(), "md5", "bob", "app"), CheckResult::Deny ); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::Trust ); } @@ -691,7 +694,7 @@ hostnossl all all 127.0.0.1/32 trust fn local_md5_rule_allows_unix_md5_client() { let hba = PgHba::from_content("local all all md5"); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::Allow ); } @@ -703,7 +706,7 @@ hostnossl all all 127.0.0.1/32 trust // fall through to the next rule or deny. let hba = PgHba::from_content("local all all scram-sha-256"); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::NotMatched ); } @@ -714,11 +717,11 @@ hostnossl all all 127.0.0.1/32 trust let hba = PgHba::from_content("local all all trust\nhost all all 0.0.0.0/0 md5"); let ip = IpAddr::V4(Ipv4Addr::new(10, 1, 2, 3)); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::Trust ); assert_eq!( - hba.check_hba(ip, false, false, "md5", "alice", "app"), + hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); } @@ -729,11 +732,11 @@ hostnossl all all 127.0.0.1/32 trust // for "admin" must not authenticate a client connecting to "app". let hba = PgHba::from_content("local admin all trust\nlocal all all reject"); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "admin"), + hba.check_hba(&unix_transport(), "md5", "alice", "admin"), CheckResult::Trust ); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::Deny ); } @@ -744,7 +747,7 @@ hostnossl all all 127.0.0.1/32 trust // caller decides what to do, it should not be silently promoted. let hba = PgHba::from_content(""); assert_eq!( - hba.check_hba(unix_ip(), false, true, "md5", "alice", "app"), + hba.check_hba(&unix_transport(), "md5", "alice", "app"), CheckResult::NotMatched ); } diff --git a/src/auth/hba_eval_tests.rs b/src/auth/hba_eval_tests.rs index ede7bf35..bc229524 100644 --- a/src/auth/hba_eval_tests.rs +++ b/src/auth/hba_eval_tests.rs @@ -18,12 +18,14 @@ fn base_ci() -> ClientIdentifier { // Build ClientIdentifier with HBA decisions computed from concrete pg_hba rules fn ci_from_hba(hba_text: &str, ssl: bool) -> ClientIdentifier { + use crate::transport::ClientTransport; let hba = PgHba::from_content(hba_text); - let ip: std::net::IpAddr = "127.0.0.1".parse().unwrap(); + let peer = std::net::SocketAddr::new("127.0.0.1".parse().unwrap(), 12345); + let transport = ClientTransport::Tcp { peer, ssl }; let username = "user"; let database = "db"; - let hba_scram = hba.check_hba(ip, ssl, false, "scram-sha-256", username, database); - let hba_md5 = hba.check_hba(ip, ssl, false, "md5", username, database); + let hba_scram = hba.check_hba(&transport, "scram-sha-256", username, database); + let hba_md5 = hba.check_hba(&transport, "md5", username, database); let mut ci = base_ci(); ci.hba_scram = hba_scram; ci.hba_md5 = hba_md5; diff --git a/src/client/entrypoint.rs b/src/client/entrypoint.rs index b2d3d4b2..80cbe55d 100644 --- a/src/client/entrypoint.rs +++ b/src/client/entrypoint.rs @@ -11,6 +11,8 @@ use crate::pool::ClientServerMap; use crate::stats::{CANCEL_CONNECTION_COUNTER, PLAIN_CONNECTION_COUNTER, TLS_CONNECTION_COUNTER}; use crate::utils::rate_limit::RateLimiter; +use crate::transport::ClientTransport; + use super::core::Client; use super::startup::{get_startup, startup_tls, ClientConnectionType}; @@ -203,13 +205,14 @@ pub async fn client_entrypoint( match Client::startup( read, write, - addr, + ClientTransport::Tcp { + peer: addr, + ssl: false, + }, bytes, client_server_map, admin_only, - false, connection_id, - false, ) .await { @@ -264,13 +267,14 @@ pub async fn client_entrypoint( match Client::startup( read, write, - addr, + ClientTransport::Tcp { + peer: addr, + ssl: false, + }, bytes, client_server_map, admin_only, - false, connection_id, - false, ) .await { @@ -337,7 +341,6 @@ pub async fn client_entrypoint_unix( admin_only: bool, connection_id: u64, ) -> Result, Error> { - let fake_addr: std::net::SocketAddr = ([127, 0, 0, 1], 0).into(); let config = get_config(); let log_client_connections = config.general.log_client_connections; @@ -349,13 +352,11 @@ pub async fn client_entrypoint_unix( match Client::startup( read, write, - fake_addr, + ClientTransport::Unix, bytes, client_server_map, admin_only, - false, connection_id, - true, ) .await { @@ -397,7 +398,14 @@ pub async fn client_entrypoint_unix( CANCEL_CONNECTION_COUNTER.fetch_add(1, Ordering::Relaxed); let (read, write) = split(stream); - match Client::cancel(read, write, fake_addr, bytes, client_server_map).await { + // Unix clients have no peer addr; use the loopback sentinel the + // TCP path would have seen so Client::cancel can still do its + // client_server_map lookup by process_id + secret_key. + let sentinel_addr = std::net::SocketAddr::from(( + std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST), + 0, + )); + match Client::cancel(read, write, sentinel_addr, bytes, client_server_map).await { Ok(mut client) => { info!("Cancel request via unix socket"); let result = client.handle().await; diff --git a/src/client/startup.rs b/src/client/startup.rs index e196cef7..eaff3ade 100644 --- a/src/client/startup.rs +++ b/src/client/startup.rs @@ -20,6 +20,7 @@ use crate::messages::{ use crate::pool::ClientServerMap; use crate::server::ServerParameters; use crate::stats::{ClientStats, CANCEL_CONNECTION_COUNTER}; +use crate::transport::ClientTransport; use super::buffer_pool::PooledBuffer; use super::core::{Client, PreparedStatementState}; @@ -131,13 +132,14 @@ pub async fn startup_tls( Client::startup( read, write, - addr, + ClientTransport::Tcp { + peer: addr, + ssl: true, + }, bytes, client_server_map, admin_only, - true, connection_id, - false, ) .await } @@ -168,14 +170,23 @@ where pub async fn startup( mut read: S, mut write: T, - addr: std::net::SocketAddr, + transport: ClientTransport, bytes: BytesMut, // The rest of the startup message. client_server_map: ClientServerMap, admin_only: bool, - use_tls: bool, connection_id: u64, - is_unix: bool, ) -> Result, Error> { + // Unix sockets have no peer address; we pin a sentinel loopback + // value into the Client struct so the many transaction-level log + // lines that interpolate `self.addr` keep compiling. A follow-up + // refactor should replace this with a typed PeerAddress field. + let addr = match transport { + ClientTransport::Tcp { peer, .. } => peer, + ClientTransport::Unix => { + std::net::SocketAddr::from((std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST), 0)) + } + }; + let use_tls = transport.is_tls(); let parameters = parse_startup(bytes)?; // This parameter is mandatory by the protocol. @@ -202,20 +213,12 @@ where application_name, username_from_parameters, &pool_name, - addr.to_string().as_str(), - ); - client_identifier.hba_md5 = check_hba( - addr.ip(), - use_tls, - is_unix, - "md5", - username_from_parameters, - &pool_name, + transport.peer_display().as_str(), ); + client_identifier.hba_md5 = + check_hba(&transport, "md5", username_from_parameters, &pool_name); client_identifier.hba_scram = check_hba( - addr.ip(), - use_tls, - is_unix, + &transport, "scram-sha-256", username_from_parameters, &pool_name, @@ -317,15 +320,15 @@ where if !hba_ok_final { error_response_terminal( &mut write, - format!("Connection from IP address {} to {}@{} (TLS: {}) is not permitted by HBA configuration. Please contact your database administrator.", - addr.ip(), username_from_parameters, pool_name, use_tls).as_str(), + format!("Connection from {} to {}@{} (TLS: {}) is not permitted by HBA configuration. Please contact your database administrator.", + transport.peer_display(), username_from_parameters, pool_name, use_tls).as_str(), "28000" ) .await?; return Err(Error::HbaForbiddenError(format!( - "Connection not permitted by HBA configuration for client: {} from address: {:?}", + "Connection not permitted by HBA configuration for client: {} from {}", client_identifier, - addr.ip() + transport.peer_display() ))); } diff --git a/src/config/mod.rs b/src/config/mod.rs index e371e906..1be8022c 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -8,7 +8,6 @@ use log::{error, info, warn}; use once_cell::sync::Lazy; use serde_derive::{Deserialize, Serialize}; use std::collections::HashMap; -use std::net::IpAddr; use std::path::Path; use std::sync::Arc; use tokio::fs::File; @@ -18,6 +17,7 @@ use self::tls::{load_identity, TLSMode}; use crate::auth::hba::CheckResult; use crate::errors::Error; use crate::pool::{ClientServerMap, ConnectionPool}; +use crate::transport::ClientTransport; use crate::utils::format_duration_ms; // Sub-modules @@ -574,23 +574,13 @@ pub async fn reload_config(client_server_map: ClientServerMap) -> Result CheckResult { let config = get_config(); - check_hba_with_general( - &config.general, - ip, - ssl, - is_unix, - type_auth, - username, - database, - ) + check_hba_with_general(&config.general, transport, type_auth, username, database) } /// True when the operator enabled a Unix listener alongside the legacy @@ -604,27 +594,26 @@ pub(crate) fn legacy_hba_bypassed_by_unix_socket(general: &General) -> bool { /// Pure evaluation of HBA rules against an explicit [`General`] snapshot. /// /// Split out of [`check_hba`] so that unit tests can exercise the legacy -/// `general.hba` branches — including the `is_unix` bypass — without +/// `general.hba` branches — including the Unix-socket bypass — without /// touching the global config. pub(crate) fn check_hba_with_general( general: &General, - ip: IpAddr, - ssl: bool, - is_unix: bool, + transport: &ClientTransport, type_auth: &str, username: &str, database: &str, ) -> CheckResult { if let Some(ref pg) = general.pg_hba { - return pg.check_hba(ip, ssl, is_unix, type_auth, username, database); + return pg.check_hba(transport, type_auth, username, database); } // Legacy hba list has no unix concept — allow all unix connections - if is_unix { + if transport.is_unix() { return CheckResult::Allow; } if general.hba.is_empty() { return CheckResult::Allow; } + let ip = transport.hba_ip(); if general.hba.iter().any(|net| net.contains(&ip)) { CheckResult::Allow } else { diff --git a/src/config/tests.rs b/src/config/tests.rs index d371ed60..43aac9aa 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -1661,21 +1661,25 @@ async fn test_validate_reserve_pool_timeout_skipped_when_coordinator_disabled() assert!(config.validate().await.is_ok()); } -// ---- check_hba_with_general: legacy general.hba + is_unix semantics ---- +// ---- check_hba_with_general: legacy general.hba + Unix socket semantics ---- + +fn tcp_transport(ip: &str) -> ClientTransport { + let peer = std::net::SocketAddr::new(ip.parse().unwrap(), 12345); + ClientTransport::Tcp { peer, ssl: false } +} #[test] fn check_hba_legacy_empty_allows_unix() { // Legacy branch, nothing configured: any transport is allowed. Kept as a - // baseline so the next two tests document what changes once `is_unix` + // baseline so the next two tests document what changes once Unix // enters the picture. let general = General::default(); - let ip: IpAddr = "10.0.0.5".parse().unwrap(); assert_eq!( - check_hba_with_general(&general, ip, false, true, "md5", "alice", "app"), + check_hba_with_general(&general, &ClientTransport::Unix, "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - check_hba_with_general(&general, ip, false, false, "md5", "alice", "app"), + check_hba_with_general(&general, &tcp_transport("10.0.0.5"), "md5", "alice", "app"), CheckResult::Allow ); } @@ -1687,22 +1691,26 @@ fn check_hba_legacy_list_bypassed_for_unix() { // must still be allowed because the legacy list has no transport concept. let mut general = General::default(); general.hba = vec!["10.0.0.0/8".parse().unwrap()]; - let ip: IpAddr = "192.168.1.10".parse().unwrap(); // Unix: Allow regardless of source IP assert_eq!( - check_hba_with_general(&general, ip, false, true, "md5", "alice", "app"), + check_hba_with_general(&general, &ClientTransport::Unix, "md5", "alice", "app"), CheckResult::Allow ); // TCP from an IP outside the whitelist: NotMatched assert_eq!( - check_hba_with_general(&general, ip, false, false, "md5", "alice", "app"), + check_hba_with_general( + &general, + &tcp_transport("192.168.1.10"), + "md5", + "alice", + "app" + ), CheckResult::NotMatched ); // TCP from an IP inside the whitelist: Allow - let ip_inside: IpAddr = "10.1.2.3".parse().unwrap(); assert_eq!( - check_hba_with_general(&general, ip_inside, false, false, "md5", "alice", "app"), + check_hba_with_general(&general, &tcp_transport("10.1.2.3"), "md5", "alice", "app"), CheckResult::Allow ); } @@ -1715,10 +1723,9 @@ fn check_hba_pg_hba_takes_precedence_over_legacy_for_unix() { let mut general = General::default(); general.hba = vec!["10.0.0.0/8".parse().unwrap()]; general.pg_hba = Some(PgHba::from_content("local all all reject")); - let ip: IpAddr = "127.0.0.1".parse().unwrap(); assert_eq!( - check_hba_with_general(&general, ip, false, true, "md5", "alice", "app"), + check_hba_with_general(&general, &ClientTransport::Unix, "md5", "alice", "app"), CheckResult::Deny ); } diff --git a/src/lib.rs b/src/lib.rs index 171cdfd1..394a96d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,5 +15,6 @@ pub mod pool; pub mod prometheus; pub mod server; pub mod stats; +pub mod transport; pub mod utils; pub use config::tls; diff --git a/src/transport.rs b/src/transport.rs new file mode 100644 index 00000000..ded53209 --- /dev/null +++ b/src/transport.rs @@ -0,0 +1,101 @@ +//! Transport descriptor used by the client authentication pipeline. +//! +//! Replaces the `(SocketAddr, ssl: bool, is_unix: bool)` triplet that used +//! to be threaded through the HBA matcher and `Client::startup`. Having a +//! single enum removes the "two booleans in a row" footgun and gives us a +//! natural place to hang transport-specific helpers like a display string +//! for logs. + +use std::net::SocketAddr; + +/// How a client reached the pooler. +#[derive(Debug, Clone)] +pub enum ClientTransport { + /// Classic TCP (optionally over TLS). + Tcp { + peer: SocketAddr, + /// True when the client completed the TLS handshake before sending + /// its startup packet. Drives hostssl rule matching and the + /// `ClientStats::is_tls` counter. + ssl: bool, + }, + /// Unix domain socket. Peer address is not meaningful for these + /// connections — the kernel does not expose a remote endpoint and + /// `SO_PEERCRED` is not currently threaded through. + Unix, +} + +impl ClientTransport { + /// True when the client is connected over a TLS-upgraded TCP socket. + pub fn is_tls(&self) -> bool { + matches!(self, ClientTransport::Tcp { ssl: true, .. }) + } + + /// True when the client is connected over a Unix domain socket. + pub fn is_unix(&self) -> bool { + matches!(self, ClientTransport::Unix) + } + + /// Short display string used in logs and in `ClientStats` / `SHOW + /// CLIENTS` rows. TCP clients carry their `peer.to_string()`; Unix + /// clients render as `unix:` so operators can tell them apart from + /// localhost TCP at a glance. + pub fn peer_display(&self) -> String { + match self { + ClientTransport::Tcp { peer, .. } => peer.to_string(), + ClientTransport::Unix => "unix:".to_string(), + } + } + + /// IP that the HBA matcher should use when checking `host`/`hostssl` + /// rules. Unix transport has no meaningful IP, so we return a sentinel + /// loopback value — the matcher ignores the IP for Unix clients + /// anyway (see `src/auth/hba.rs`). + pub fn hba_ip(&self) -> std::net::IpAddr { + match self { + ClientTransport::Tcp { peer, .. } => peer.ip(), + ClientTransport::Unix => std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::net::Ipv4Addr; + + #[test] + fn tcp_is_tls_reflects_ssl_flag() { + let peer = SocketAddr::from((Ipv4Addr::new(10, 0, 0, 1), 5432)); + assert!(!ClientTransport::Tcp { peer, ssl: false }.is_tls()); + assert!(ClientTransport::Tcp { peer, ssl: true }.is_tls()); + assert!(!ClientTransport::Tcp { peer, ssl: true }.is_unix()); + } + + #[test] + fn unix_is_unix_and_never_tls() { + assert!(ClientTransport::Unix.is_unix()); + assert!(!ClientTransport::Unix.is_tls()); + } + + #[test] + fn peer_display_distinguishes_transports() { + let peer = SocketAddr::from((Ipv4Addr::new(127, 0, 0, 1), 54321)); + assert_eq!( + ClientTransport::Tcp { peer, ssl: false }.peer_display(), + "127.0.0.1:54321" + ); + assert_eq!(ClientTransport::Unix.peer_display(), "unix:"); + } + + #[test] + fn hba_ip_for_unix_is_loopback_sentinel() { + // The HBA matcher drops the IP entirely for Unix clients, so the + // exact value does not matter — but we pin loopback here so a + // regression is easy to spot. + assert_eq!( + ClientTransport::Unix.hba_ip(), + std::net::IpAddr::V4(Ipv4Addr::LOCALHOST) + ); + } +} From 5efb46efeb73e78eab0f9b2fbbca8c92e082b386 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Mon, 6 Apr 2026 23:15:59 +0300 Subject: [PATCH 21/24] Drop client_entrypoint copy-paste into one helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the TCP and Unix entrypoints each inlined the same "call Client::startup, log connected, run handle, flush disconnect_stats" sequence. Same four locals, same branch shape, only the ClientTransport variant and the log label differed. The plain-continue-after-rejected- TLS branch in the TCP path was a third copy. Any change to the lifecycle meant hunting down three mirrored places. What changed: a new drive_authenticated_client generic helper takes the split stream halves, the transport descriptor, and a log label, then runs the full post-startup pipeline. The three former copies collapse to single call sites and client_entrypoint_unix shrinks to a straight dispatch on the three ClientConnectionType variants. No behavioural change — tests (incl. the 13 HBA unit tests) still pass. --- src/client/entrypoint.rs | 138 +++++++++++++++++++-------------------- 1 file changed, 67 insertions(+), 71 deletions(-) diff --git a/src/client/entrypoint.rs b/src/client/entrypoint.rs index 80cbe55d..a5341a67 100644 --- a/src/client/entrypoint.rs +++ b/src/client/entrypoint.rs @@ -23,6 +23,64 @@ pub struct ClientSessionInfo { pub connection_id: u64, } +/// Drive the authenticated-client lifecycle for any transport. +/// +/// Three places (plain TCP startup, TCP plain-continue after rejected TLS, +/// and Unix socket startup) used to inline the same sequence: call +/// `Client::startup`, log "client connected", run `client.handle()`, and +/// flush `disconnect_stats` on a late error. Centralising it here keeps +/// the three call sites down to a single generic hop and removes ~90 +/// lines of copy-paste. +#[allow(clippy::too_many_arguments)] +async fn drive_authenticated_client( + read: S, + write: T, + transport: ClientTransport, + bytes: bytes::BytesMut, + client_server_map: ClientServerMap, + admin_only: bool, + connection_id: u64, + log_client_connections: bool, + log_label: &'static str, +) -> Result, Error> +where + S: tokio::io::AsyncRead + Unpin + Send + 'static, + T: tokio::io::AsyncWrite + Unpin + Send + 'static, +{ + let peer = transport.peer_display(); + match Client::startup( + read, + write, + transport, + bytes, + client_server_map, + admin_only, + connection_id, + ) + .await + { + Ok(mut client) => { + if log_client_connections { + info!( + "[{}@{} #c{}] client connected from {} ({})", + client.username, client.pool_name, client.connection_id, peer, log_label, + ); + } + let session_info = ClientSessionInfo { + username: client.username.clone(), + pool_name: client.pool_name.clone(), + connection_id: client.connection_id, + }; + let result = client.handle().await; + if !client.is_admin() && result.is_err() { + client.disconnect_stats(); + } + result.map(|_| Some(session_info)) + } + Err(err) => Err(err), + } +} + pub async fn client_entrypoint_too_many_clients_already( mut stream: TcpStream, client_server_map: ClientServerMap, @@ -200,9 +258,7 @@ pub async fn client_entrypoint( // Client accepted unencrypted connection. Ok((ClientConnectionType::Startup, bytes)) => { let (read, write) = split(stream); - - // Continue with regular startup. - match Client::startup( + drive_authenticated_client( read, write, ClientTransport::Tcp { @@ -213,29 +269,10 @@ pub async fn client_entrypoint( client_server_map, admin_only, connection_id, + log_client_connections, + "plain", ) .await - { - Ok(mut client) => { - if log_client_connections { - info!( - "[{}@{} #c{}] client connected from {addr} (plain)", - client.username, client.pool_name, client.connection_id - ); - } - let session_info = ClientSessionInfo { - username: client.username.clone(), - pool_name: client.pool_name.clone(), - connection_id: client.connection_id, - }; - let result = client.handle().await; - if !client.is_admin() && result.is_err() { - client.disconnect_stats(); - } - result.map(|_| Some(session_info)) - } - Err(err) => Err(err), - } } // Client probably disconnected rejecting our plain text connection. @@ -262,9 +299,7 @@ pub async fn client_entrypoint( } PLAIN_CONNECTION_COUNTER.fetch_add(1, Ordering::Relaxed); let (read, write) = split(stream); - - // Continue with regular startup. - match Client::startup( + drive_authenticated_client( read, write, ClientTransport::Tcp { @@ -275,29 +310,10 @@ pub async fn client_entrypoint( client_server_map, admin_only, connection_id, + log_client_connections, + "plain", ) .await - { - Ok(mut client) => { - if log_client_connections { - info!( - "[{}@{} #c{}] client connected from {addr} (plain)", - client.username, client.pool_name, client.connection_id - ); - } - let session_info = ClientSessionInfo { - username: client.username.clone(), - pool_name: client.pool_name.clone(), - connection_id: client.connection_id, - }; - let result = client.handle().await; - if !client.is_admin() && result.is_err() { - client.disconnect_stats(); - } - result.map(|_| Some(session_info)) - } - Err(err) => Err(err), - } } // Client wants to cancel a query. @@ -348,8 +364,7 @@ pub async fn client_entrypoint_unix( Ok((ClientConnectionType::Startup, bytes)) => { PLAIN_CONNECTION_COUNTER.fetch_add(1, Ordering::Relaxed); let (read, write) = split(stream); - - match Client::startup( + drive_authenticated_client( read, write, ClientTransport::Unix, @@ -357,29 +372,10 @@ pub async fn client_entrypoint_unix( client_server_map, admin_only, connection_id, + log_client_connections, + "unix", ) .await - { - Ok(mut client) => { - if log_client_connections { - info!( - "[{}@{} #c{}] client connected via unix socket", - client.username, client.pool_name, client.connection_id - ); - } - let session_info = ClientSessionInfo { - username: client.username.clone(), - pool_name: client.pool_name.clone(), - connection_id: client.connection_id, - }; - let result = client.handle().await; - if !client.is_admin() && result.is_err() { - client.disconnect_stats(); - } - result.map(|_| Some(session_info)) - } - Err(err) => Err(err), - } } Ok((ClientConnectionType::Tls, _)) => { From ed1195fc5ab83cee6a134a9cf715d5207496dba2 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Tue, 7 Apr 2026 16:07:26 +0300 Subject: [PATCH 22/24] Share post-disconnect logging between TCP and Unix accept branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: both accept sites inlined the same match on Result, Error> — same identity formatting, same elapsed-time rendering, same info-vs-warn split. Any tweak to session logging meant changing two places in lockstep. What changed: a new log_session_end helper takes the entrypoint result, the connection_id, a peer label, the session start time, and the config flag, and produces the single-line disconnect log. The TCP accept branch passes addr.to_string(), the Unix branch passes the "unix:" sentinel — the only transport-specific bit left in the select arms. Spawned tasks shrink from ~45 lines each to ~25 with no behavioural change; ClientSessionInfo became pub to cross the module boundary. 423 lib tests green. --- src/app/server.rs | 106 +++++++++++++++++++++++----------------------- src/client/mod.rs | 2 +- 2 files changed, 55 insertions(+), 53 deletions(-) diff --git a/src/app/server.rs b/src/app/server.rs index b212c85b..0610a51b 100644 --- a/src/app/server.rs +++ b/src/app/server.rs @@ -442,22 +442,17 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box max_connections { warn!("[#c{connection_id}] client {addr} rejected: too many clients (current={current_clients}, max={max_connections})"); - match crate::client::client_entrypoint_too_many_clients_already( + if let Err(err) = crate::client::client_entrypoint_too_many_clients_already( socket, client_server_map).await { - Ok(()) => (), - Err(err) => { - error!("[#c{connection_id}] client {addr} disconnected with error: {err}"); - } + error!("[#c{connection_id}] client {addr} disconnected with error: {err}"); } CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); - return + return; } let start = Utc::now().naive_utc(); - - match crate::client::client_entrypoint( + let result = crate::client::client_entrypoint( socket, client_server_map, admin_only, @@ -465,30 +460,14 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { - if log_client_disconnections - || log::log_enabled!(log::Level::Debug) - { - let session = format_duration( - &(Utc::now().naive_utc() - start), - ); - let identity = match &session_info { - Some(si) => format!("[{}@{} #c{}]", si.username, si.pool_name, si.connection_id), - None => format!("[#c{connection_id}]"), - }; - info!("{identity} client disconnected from {addr}, session={session}"); - } - } - - Err(err) => { - // Pre-auth failures: identity unknown, only connection_id available. - // Post-auth failures already logged with [user@pool #cN] inside entrypoint. - let session = format_duration(&(Utc::now().naive_utc() - start)); - warn!("[#c{connection_id}] client {addr} disconnected with error: {err}, session={session}"); - } - }; + .await; + log_session_end( + result, + connection_id, + &addr.to_string(), + start, + log_client_disconnections, + ); CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); }); } @@ -535,30 +514,20 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { - if log_client_disconnections { - let session = format_duration(&(Utc::now().naive_utc() - start)); - let identity = match &session_info { - Some(si) => format!("[{}@{} #c{}]", si.username, si.pool_name, si.connection_id), - None => format!("[#c{connection_id}]"), - }; - info!("{identity} unix client disconnected, session={session}"); - } - } - Err(err) => { - let session = format_duration(&(Utc::now().naive_utc() - start)); - warn!("[#c{connection_id}] unix client disconnected with error: {err}, session={session}"); - } - }; + .await; + log_session_end( + result, + connection_id, + "unix:", + start, + log_client_disconnections, + ); CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); }); } @@ -965,6 +934,39 @@ enum CleanupDecision { Skip(String), } +/// Log the end of a client session using a shared format string. Both the +/// TCP and Unix accept branches used to inline the same match on +/// `Result, Error>` — same identity string, +/// same elapsed-time rendering, same warn vs info split. Centralising +/// it keeps the two remaining accept sites down to a single call. +fn log_session_end( + result: Result, crate::errors::Error>, + connection_id: u64, + peer_label: &str, + session_start: chrono::NaiveDateTime, + log_disconnections: bool, +) { + let session = format_duration(&(Utc::now().naive_utc() - session_start)); + match result { + Ok(session_info) => { + if log_disconnections || log::log_enabled!(log::Level::Debug) { + let identity = match &session_info { + Some(si) => { + format!("[{}@{} #c{}]", si.username, si.pool_name, si.connection_id) + } + None => format!("[#c{connection_id}]"), + }; + info!("{identity} client disconnected from {peer_label}, session={session}"); + } + } + Err(err) => { + // Pre-auth failures: identity unknown, only connection_id available. + // Post-auth failures already logged with [user@pool #cN] inside entrypoint. + warn!("[#c{connection_id}] client {peer_label} disconnected with error: {err}, session={session}"); + } + } +} + /// Create a Tokio Unix socket listener at `path` with the given permission /// `mode`. /// diff --git a/src/client/mod.rs b/src/client/mod.rs index a4313e62..96c30423 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -11,7 +11,7 @@ mod util; pub use core::Client; pub use entrypoint::{ client_entrypoint, client_entrypoint_too_many_clients_already, - client_entrypoint_too_many_clients_already_unix, client_entrypoint_unix, + client_entrypoint_too_many_clients_already_unix, client_entrypoint_unix, ClientSessionInfo, }; pub use startup::startup_tls; pub use util::PREPARED_STATEMENT_COUNTER; From 911c26353d897e80f8b40bdf94dc11cf8f81938a Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Tue, 7 Apr 2026 16:13:29 +0300 Subject: [PATCH 23/24] Add Unix socket BDD coverage for HBA and TLS interactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the unix-socket feature only carried the happy-path scenario plus the file-mode assertions added with the chmod fix. Critical HBA and TLS rules that reach the matcher only via the new ClientTransport plumbing had no end-to-end coverage, so a regression in is_unix handling could pass CI silently. What changed: three new scenarios on top of the existing happy path — "local reject blocks Unix", "host rule does not match Unix", and "only_ssl_connections does not block Unix". Each spins up pg_doorman with the relevant pg_hba snippet (or tls_mode = require) and asserts the expected outcome via psql over the socket. Two new step definitions in postgres_helper exercise the rejection path: psql_unix_connection_fails (no error string check) and psql_unix_connection_fails_with (substring match for structured error verification). --- tests/bdd/features/unix-socket.feature | 107 +++++++++++++++++++++++++ tests/bdd/postgres_helper.rs | 65 +++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/tests/bdd/features/unix-socket.feature b/tests/bdd/features/unix-socket.feature index b4694e68..acfb6084 100644 --- a/tests/bdd/features/unix-socket.feature +++ b/tests/bdd/features/unix-socket.feature @@ -101,3 +101,110 @@ Feature: Unix socket connections pool_size = 10 """ Then pg_doorman unix socket file has mode "0660" + + Scenario: HBA local reject blocks Unix socket connection + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And pg_doorman hba file contains: + """ + local all all reject + host all all 0.0.0.0/0 trust + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + unix_socket_dir = "${PG_TEMP_DIR}" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.postgres.users]] + username = "postgres" + password = "" + pool_size = 10 + """ + Then psql connection to pg_doorman via unix socket as user "postgres" to database "postgres" fails + + Scenario: HBA host rule does not match Unix socket connection + # Only a `host` rule for 127.0.0.1 — Unix clients have no peer IP, so + # this rule must NOT authenticate them. Without a `local` rule the + # connection should be rejected by the matcher. + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And pg_doorman hba file contains: + """ + host all all 127.0.0.1/32 trust + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + unix_socket_dir = "${PG_TEMP_DIR}" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.postgres.users]] + username = "postgres" + password = "" + pool_size = 10 + """ + Then psql connection to pg_doorman via unix socket as user "postgres" to database "postgres" fails + + Scenario: only_ssl_connections does not block Unix socket clients + # only_ssl_connections rejects plain TCP, but Unix sockets are inherently + # local-only and should never be subject to the TLS-required check. + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And self-signed SSL certificates are generated + And pg_doorman hba file contains: + """ + local all all trust + hostssl all all 0.0.0.0/0 trust + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + tls_private_key = "${DOORMAN_SSL_KEY}" + tls_certificate = "${DOORMAN_SSL_CERT}" + tls_mode = "require" + unix_socket_dir = "${PG_TEMP_DIR}" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.postgres.users]] + username = "postgres" + password = "" + pool_size = 10 + """ + Then psql query "SELECT 1" via pg_doorman unix socket as user "postgres" to database "postgres" returns "1" diff --git a/tests/bdd/postgres_helper.rs b/tests/bdd/postgres_helper.rs index 02f37a60..5eda066c 100644 --- a/tests/bdd/postgres_helper.rs +++ b/tests/bdd/postgres_helper.rs @@ -752,6 +752,71 @@ pub async fn psql_unix_query_returns( ); } +/// Verify that a Unix socket psql connection fails — without checking the +/// exact error string. Used by feature scenarios that rely on HBA reject +/// or "no rule matched" wording, both of which produce different error +/// codes depending on which auth method the matcher fell through to. +#[then( + expr = "psql connection to pg_doorman via unix socket as user {string} to database {string} fails" +)] +pub async fn psql_unix_connection_fails(world: &mut DoormanWorld, user: String, database: String) { + let port = world.doorman_port.expect("pg_doorman not started"); + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir not set") + .path() + .to_str() + .unwrap() + .to_string(); + let output = build_psql_via_doorman_unix(&socket_dir, port, &user, &database, "SELECT 1") + .output() + .expect("Failed to run psql"); + assert!( + !output.status.success(), + "psql unix socket connection unexpectedly succeeded: stdout={}", + String::from_utf8_lossy(&output.stdout), + ); +} + +/// Verify a Unix socket psql connection fails AND its stderr contains a +/// substring. Used to assert that pg_doorman replies with a structured +/// PostgreSQL ErrorResponse and not a bare connection drop. +#[then( + expr = "psql connection to pg_doorman via unix socket as user {string} to database {string} fails with {string}" +)] +pub async fn psql_unix_connection_fails_with( + world: &mut DoormanWorld, + user: String, + database: String, + expected_error: String, +) { + let port = world.doorman_port.expect("pg_doorman not started"); + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir not set") + .path() + .to_str() + .unwrap() + .to_string(); + let output = build_psql_via_doorman_unix(&socket_dir, port, &user, &database, "SELECT 1") + .output() + .expect("Failed to run psql"); + assert!( + !output.status.success(), + "psql unix socket connection unexpectedly succeeded: stdout={}", + String::from_utf8_lossy(&output.stdout), + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains(&expected_error), + "expected stderr to contain '{}', got: {}", + expected_error, + stderr, + ); +} + /// Stop PostgreSQL and pg_doorman when the world is dropped impl Drop for DoormanWorld { fn drop(&mut self) { From 8a9ee6f75abc5898df89988ba8a328100ed3d534 Mon Sep 17 00:00:00 2001 From: dmitrivasilyev Date: Tue, 7 Apr 2026 16:55:20 +0300 Subject: [PATCH 24/24] Cover Unix socket md5 authentication in BDD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed: the BDD suite checked the trust path and the bare-rejection path through the new HBA local-rule plumbing, but never exercised a password-authenticated client coming in over the Unix socket. md5 is the most common production setup, so a regression in the auth pipeline for Unix would still pass CI. What changed: two new scenarios in unix-socket.feature reuse the existing tests/fixture.sql user (example_user_1 / md5...test) — one asserts SELECT 1 succeeds when the password matches, the other asserts the connection fails with the wrong password. Two new step definitions in postgres_helper supply the PGPASSWORD env var for both the success and failure paths. --- tests/bdd/features/unix-socket.feature | 68 ++++++++++++++ tests/bdd/postgres_helper.rs | 118 +++++++++++++++++++++++++ 2 files changed, 186 insertions(+) diff --git a/tests/bdd/features/unix-socket.feature b/tests/bdd/features/unix-socket.feature index acfb6084..339c45d8 100644 --- a/tests/bdd/features/unix-socket.feature +++ b/tests/bdd/features/unix-socket.feature @@ -170,6 +170,74 @@ Feature: Unix socket connections """ Then psql connection to pg_doorman via unix socket as user "postgres" to database "postgres" fails + Scenario: HBA local md5 authenticates Unix socket clients with the right password + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman hba file contains: + """ + local all all md5 + host all all 0.0.0.0/0 md5 + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + unix_socket_dir = "${PG_TEMP_DIR}" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 10 + """ + Then psql query "SELECT 1" via pg_doorman unix socket as user "example_user_1" to database "example_db" with password "test" returns "1" + + Scenario: HBA local md5 rejects Unix socket clients with the wrong password + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman hba file contains: + """ + local all all md5 + host all all 0.0.0.0/0 md5 + """ + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba = {path = "${DOORMAN_HBA_FILE}"} + unix_socket_dir = "${PG_TEMP_DIR}" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 10 + """ + Then psql connection to pg_doorman via unix socket as user "example_user_1" to database "example_db" with password "wrong-password" fails + Scenario: only_ssl_connections does not block Unix socket clients # only_ssl_connections rejects plain TCP, but Unix sockets are inherently # local-only and should never be subject to the TLS-required check. diff --git a/tests/bdd/postgres_helper.rs b/tests/bdd/postgres_helper.rs index 5eda066c..3d0cecff 100644 --- a/tests/bdd/postgres_helper.rs +++ b/tests/bdd/postgres_helper.rs @@ -686,6 +686,36 @@ fn build_psql_via_doorman_unix( cmd } +/// Same as `build_psql_via_doorman_unix` but supplies a password through +/// `PGPASSWORD` so the scenarios that exercise password-based auth over +/// Unix sockets do not need to drive the prompt manually. +fn build_psql_via_doorman_unix_with_password( + socket_dir: &str, + port: u16, + user: &str, + database: &str, + query: &str, + password: &str, +) -> Command { + let mut cmd = Command::new("psql"); + cmd.args([ + "-h", + socket_dir, + "-p", + &port.to_string(), + "-U", + user, + "-d", + database, + "-c", + query, + ]); + cmd.env("PGSSLMODE", "disable"); + cmd.arg("-w"); + cmd.env("PGPASSWORD", password); + cmd +} + #[then( expr = "psql connection to pg_doorman via unix socket as user {string} to database {string} succeeds" )] @@ -752,6 +782,94 @@ pub async fn psql_unix_query_returns( ); } +/// Run a query through the Unix socket using a password-authenticated +/// connection and assert it succeeds. Pairs with the password-authenticated +/// failure step below to round out HBA `local md5`/`local scram-sha-256` +/// scenarios. +#[then( + expr = "psql query {string} via pg_doorman unix socket as user {string} to database {string} with password {string} returns {string}" +)] +pub async fn psql_unix_query_with_password_returns( + world: &mut DoormanWorld, + query: String, + user: String, + database: String, + password: String, + expected: String, +) { + let port = world.doorman_port.expect("pg_doorman not started"); + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir not set") + .path() + .to_str() + .unwrap() + .to_string(); + let mut cmd = build_psql_via_doorman_unix_with_password( + &socket_dir, + port, + &user, + &database, + &query, + &password, + ); + cmd.args(["-t", "-A"]); + let output = cmd.output().expect("Failed to run psql"); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "psql unix query failed: stderr: {}", + stderr, + ); + assert!( + stdout.contains(&expected), + "Expected '{}', got: '{}' (stderr: {})", + expected, + stdout.trim(), + stderr.trim(), + ); +} + +/// Run a query through the Unix socket with a wrong password and assert +/// authentication fails. Both ends accept any error message — psql exits +/// non-zero either way once auth refuses the credentials. +#[then( + expr = "psql connection to pg_doorman via unix socket as user {string} to database {string} with password {string} fails" +)] +pub async fn psql_unix_connection_with_password_fails( + world: &mut DoormanWorld, + user: String, + database: String, + password: String, +) { + let port = world.doorman_port.expect("pg_doorman not started"); + let socket_dir = world + .pg_tmp_dir + .as_ref() + .expect("pg_tmp_dir not set") + .path() + .to_str() + .unwrap() + .to_string(); + let output = build_psql_via_doorman_unix_with_password( + &socket_dir, + port, + &user, + &database, + "SELECT 1", + &password, + ) + .output() + .expect("Failed to run psql"); + assert!( + !output.status.success(), + "psql unix socket connection unexpectedly succeeded: stdout={}", + String::from_utf8_lossy(&output.stdout), + ); +} + /// Verify that a Unix socket psql connection fails — without checking the /// exact error string. Used by feature scenarios that rely on HBA reject /// or "no rule matched" wording, both of which produce different error