diff --git a/.github/workflows/bench-aws-fargate.yml b/.github/workflows/bench-aws-fargate.yml index 6201da1a..9d423062 100644 --- a/.github/workflows/bench-aws-fargate.yml +++ b/.github/workflows/bench-aws-fargate.yml @@ -325,33 +325,55 @@ 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}') + 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 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 @@ -404,11 +426,61 @@ 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 \ + # 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 + MARKER_TS=$(aws logs filter-log-events \ --log-group-name "${LOG_GROUP}" \ - --log-stream-name "${LOG_STREAM}" \ - --output json | jq -r '.events[].message' > benchmark-results/stdout.txt + --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..." 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 diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index bbf79f58..654c1454 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -1,6 +1,24 @@ # Changelog -### 3.4.1 Apr 3, 2026 +### 3.4.0 Apr 1, 2026 + +**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. + +- **`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. + +- **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:** @@ -14,25 +32,14 @@ - **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. +**Known limitations (Unix socket):** -- **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. +- 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 diff --git a/pg_doorman.toml b/pg_doorman.toml index 69c17517..b29fd41e 100644 --- a/pg_doorman.toml +++ b/pg_doorman.toml @@ -115,6 +115,15 @@ tcp_user_timeout = 60 # Default: 1048576 (1048576 bytes) unix_socket_buffer_size = 1048576 +# 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" + +# 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 400f019e..0d0b7a2c 100644 --- a/pg_doorman.yaml +++ b/pg_doorman.yaml @@ -152,6 +152,15 @@ general: # Default: "1MB" (1048576 bytes) unix_socket_buffer_size: "1MB" + # 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" + + # 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 4250fb0c..4f4bd2bc 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, + "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(); + + 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 e534ba40..71986807 100644 --- a/src/app/generate/fields.yaml +++ b/src/app/generate/fields.yaml @@ -482,6 +482,27 @@ 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 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" + + 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 feff45c6..0610a51b 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,34 @@ pub fn run_server(args: Args, config: Config) -> Result<(), Box { + let path = format!("{dir}/.s.PGSQL.{}", config.general.port); + let mode = crate::config::General::parse_unix_socket_mode( + &config.general.unix_socket_mode, + ) + .expect("unix_socket_mode validated at config load"); + match create_unix_listener(&path, mode) { + Ok((listener, ownership)) => { + info!("Unix socket listening on {path} (mode={mode:#o})"); + (Some(listener), Some(ownership)) + } + Err(err) => { + error!("{err}"); + std::process::exit(exitcode::OSERR); + } + } + } + None => (None, None), + }; + config.show(); // Tracks which client is connected to which server for query cancellation. @@ -414,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, @@ -437,30 +460,74 @@ 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}"); - } - } + .await; + log_session_end( + result, + connection_id, + &addr.to_string(), + start, + log_client_disconnections, + ); + CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); + }); + } - 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}"); + // Unix socket client + new_unix = async { + if let Some(ref l) = unix_listener { + l.accept().await + } else { + std::future::pending().await + } + } => { + 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})"); + 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; + } + let start = Utc::now().naive_utc(); + let result = crate::client::client_entrypoint_unix( + socket, + client_server_map, + admin_only, + connection_id, + ) + .await; + log_session_end( + result, + connection_id, + "unix:", + start, + log_client_disconnections, + ); CURRENT_CLIENT_COUNT.fetch_add(-1, Ordering::SeqCst); }); } @@ -471,6 +538,26 @@ 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); + } + } + } + info!("Shutting down..."); }); @@ -758,3 +845,487 @@ 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), +} + +/// 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`. +/// +/// 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. +/// +/// 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 +/// 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 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"); + 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] + #[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. + 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] + #[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 + // 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}; + 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"); + 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] + #[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. + // 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] + #[serial] + 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] + #[serial] + 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] + #[serial] + 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] + #[serial] + 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; + 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"); + assert!(prepare_unix_socket_path(path.to_str().unwrap()).is_ok()); + } + + #[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. + 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] + #[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. + 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; + 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/src/auth/hba.rs b/src/auth/hba.rs index fc141427..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,8 +359,7 @@ impl PgHba { /// otherwise `CheckResult::NotMatched`. pub fn check_hba( &self, - ip: IpAddr, - ssl: bool, + transport: &ClientTransport, type_auth: &str, username: &str, database: &str, @@ -371,16 +371,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 !transport.is_unix() { + continue; + } + } + _ => { + // host/hostssl/hostnossl rules match only TCP connections + if transport.is_unix() { + continue; + } + if !rule.host_type.matches_ssl(transport.is_tls()) { + continue; + } + if let Some(net) = &rule.address { + if !net.contains(&transport.hba_ip()) { + continue; + } + } } } // Database and user must match as well (supporting keyword `all`). @@ -444,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 @@ -453,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); @@ -461,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, "md5", "alice", "app"), + hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - hba.check_hba(ip, true, "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, "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, "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, "md5", "alice", "app"), + hba.check_hba(&tcp(ip3, false), "md5", "alice", "app"), CheckResult::Trust ); } @@ -509,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, "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, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip2, false), "md5", "alice", "app"), CheckResult::Allow ); } @@ -528,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, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); assert_eq!( - cfg.hba.check_hba(ip, true, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, true), "md5", "alice", "app"), CheckResult::Allow ); } @@ -555,7 +576,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(&tcp(ip, true), "scram-sha-256", "alice", "app"), CheckResult::Allow ); @@ -593,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, "md5", "alice", "app"), + cfg.hba.check_hba(&tcp(ip, false), "md5", "alice", "app"), CheckResult::Allow ); } @@ -605,4 +627,128 @@ 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); } + + // ---- 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_transport(), "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(&tcp(ip, 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_transport(), "md5", "alice", "app"), + CheckResult::NotMatched + ); + } + + #[test] + 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_transport(), "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_transport(), "md5", "bob", "app"), + CheckResult::Deny + ); + assert_eq!( + hba.check_hba(&unix_transport(), "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_transport(), "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_transport(), "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_transport(), "md5", "alice", "app"), + CheckResult::Trust + ); + assert_eq!( + hba.check_hba(&tcp(ip, 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_transport(), "md5", "alice", "admin"), + CheckResult::Trust + ); + assert_eq!( + hba.check_hba(&unix_transport(), "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_transport(), "md5", "alice", "app"), + CheckResult::NotMatched + ); + } } diff --git a/src/auth/hba_eval_tests.rs b/src/auth/hba_eval_tests.rs index 5a72c813..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, "scram-sha-256", username, database); - let hba_md5 = hba.check_hba(ip, ssl, "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/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/client/entrypoint.rs b/src/client/entrypoint.rs index 39560134..a5341a67 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; @@ -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}; @@ -21,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, @@ -71,6 +131,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, @@ -157,40 +258,21 @@ 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, - addr, + ClientTransport::Tcp { + peer: addr, + ssl: false, + }, bytes, client_server_map, admin_only, - false, 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. @@ -217,40 +299,21 @@ 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, - addr, + ClientTransport::Tcp { + peer: addr, + ssl: false, + }, bytes, client_server_map, admin_only, - false, 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. @@ -286,3 +349,74 @@ pub async fn client_entrypoint( } } } + +/// Unix socket client entrypoint. Uses placeholder addr 127.0.0.1:0 (Unix sockets have no peer address). +pub async fn client_entrypoint_unix( + mut stream: UnixStream, + client_server_map: ClientServerMap, + admin_only: bool, + connection_id: u64, +) -> Result, Error> { + 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); + drive_authenticated_client( + read, + write, + ClientTransport::Unix, + bytes, + client_server_map, + admin_only, + connection_id, + log_client_connections, + "unix", + ) + .await + } + + 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); + + // 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; + 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..96c30423 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -9,6 +9,9 @@ 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_too_many_clients_already_unix, client_entrypoint_unix, ClientSessionInfo, +}; 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..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,11 +132,13 @@ 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, ) .await @@ -167,13 +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, ) -> 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. @@ -200,18 +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, - "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, + &transport, "scram-sha-256", username_from_parameters, &pool_name, @@ -313,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/general.rs b/src/config/general.rs index 856c28df..9725873c 100644 --- a/src/config/general.rs +++ b/src/config/general.rs @@ -55,6 +55,16 @@ pub struct General { #[serde(default = "General::default_unix_socket_buffer_size")] pub unix_socket_buffer_size: ByteSize, + #[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, @@ -237,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 } @@ -416,6 +458,8 @@ 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, + unix_socket_mode: Self::default_unix_socket_mode(), log_client_connections: true, log_client_disconnections: true, sync_server_parameters: Self::default_sync_server_parameters(), @@ -447,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 33ecc92e..1be8022c 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -4,11 +4,10 @@ //! 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; -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 @@ -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( @@ -371,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())); @@ -557,20 +574,47 @@ 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); + check_hba_with_general(&config.general, transport, type_auth, username, database) +} + +/// 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 +/// `general.hba` branches — including the Unix-socket bypass — without +/// touching the global config. +pub(crate) fn check_hba_with_general( + general: &General, + transport: &ClientTransport, + type_auth: &str, + username: &str, + database: &str, +) -> CheckResult { + if let Some(ref pg) = general.pg_hba { + return pg.check_hba(transport, type_auth, username, database); + } + // Legacy hba list has no unix concept — allow all unix connections + if transport.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)) { + let ip = transport.hba_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 1ded47a1..43aac9aa 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(), @@ -1656,3 +1660,109 @@ 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 + 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 Unix + // enters the picture. + let general = General::default(); + assert_eq!( + check_hba_with_general(&general, &ClientTransport::Unix, "md5", "alice", "app"), + CheckResult::Allow + ); + assert_eq!( + check_hba_with_general(&general, &tcp_transport("10.0.0.5"), "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()]; + + // Unix: Allow regardless of source IP + assert_eq!( + 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, + &tcp_transport("192.168.1.10"), + "md5", + "alice", + "app" + ), + CheckResult::NotMatched + ); + // TCP from an IP inside the whitelist: Allow + assert_eq!( + check_hba_with_general(&general, &tcp_transport("10.1.2.3"), "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")); + + assert_eq!( + check_hba_with_general(&general, &ClientTransport::Unix, "md5", "alice", "app"), + 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)); +} 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/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/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) + ); + } +} 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..0a4c317b 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 @@ -313,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/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/features/bench.feature b/tests/bdd/features/bench.feature index e90718c7..cd567086 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,199 @@ 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}" + unix_socket_mode "0644" + 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/features/unix-socket.feature b/tests/bdd/features/unix-socket.feature new file mode 100644 index 00000000..339c45d8 --- /dev/null +++ b/tests/bdd/features/unix-socket.feature @@ -0,0 +1,278 @@ +@unix-socket @rust-1 +Feature: Unix socket connections + + Scenario: Query via Unix socket reaches PostgreSQL backend + 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 + """ + # 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" + + 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: 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. + 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/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 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/postgres_helper.rs b/tests/bdd/postgres_helper.rs index dff82fa8..3d0cecff 100644 --- a/tests/bdd/postgres_helper.rs +++ b/tests/bdd/postgres_helper.rs @@ -659,6 +659,282 @@ 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 +} + +/// 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" +)] +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(), + ); +} + +/// 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 +/// 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) { 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 3ed97af5..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,14 @@ 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 { + 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 { @@ -250,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() } }