diff --git a/.github/workflows/bdd-tests.yml b/.github/workflows/bdd-tests.yml index 890089e34..f2298bc36 100644 --- a/.github/workflows/bdd-tests.yml +++ b/.github/workflows/bdd-tests.yml @@ -204,13 +204,10 @@ jobs: # Single matrixed job replaces the 21 nearly-identical per-language / # per-suite jobs that used to live here. Each suite differs only in - # the cargo command it runs; share everything else. - # - # Why no `nick-fields/retry` around the cargo step: the legacy jobs - # wrapped `cargo test` in max_attempts: 2, which masks intermittent - # BDD failures rather than surfaces them. Retry stays on the network - # `docker pull` step, where it covers GHCR manifest visibility lag - # right after a fresh push (the typical real-world flake). + # the cargo command it runs; share everything else. See the cargo + # step below for the retry policy and why three attempts beat two + # on the dominant GHA flake families (DNS to crates.io, timing- + # sensitive lifecycle scenarios). bdd-tests: name: 'BDD: ${{ matrix.suite.name }}' needs: prepare-tests @@ -222,6 +219,12 @@ jobs: packages: read strategy: fail-fast: false + # GitHub-hosted runners share CPU under heavy matrix fan-out, and + # the timing-sensitive scenarios (sleep-based retain/lifetime + # waits, SCRAM passthrough reconnect) lose their margin when 20+ + # BDD jobs run in parallel. Cap concurrency so each suite gets a + # less contested runner. + max-parallel: 4 matrix: suite: - { name: "Go", cargo: "test --test bdd -- --tags @go" } @@ -245,6 +248,7 @@ jobs: - { name: "Patroni-assisted fallback", cargo: "test --test bdd -- --tags @patroni_fallback" } - { name: "Server TLS", cargo: "test --test bdd -- --tags @server-tls" } - { name: "TLS migration (vendored OpenSSL)", cargo: "test --features tls-migration --test bdd -- --tags @tls-migration" } + - { name: "Startup parameters", cargo: "test --test bdd -- --tags @startup-parameters" } steps: - name: Checkout repository uses: actions/checkout@v4 @@ -269,9 +273,44 @@ jobs: command: docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.prepare-tests.outputs.image-tag }} - name: Run BDD suite (${{ matrix.suite.name }}) - run: | - docker run --rm \ - -v ${{ github.workspace }}:/workspace \ - -w /workspace \ - ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.prepare-tests.outputs.image-tag }} \ - cargo ${{ matrix.suite.cargo }} + # `--network=host` is the load-bearing flag. With the default + # bridge network, the container inherits DNS from the docker + # daemon, which on a GitHub-hosted runner does not always + # re-export the host's systemd-resolved stub at 127.0.0.53. + # When the bridge resolver is wedged for a job, every cargo + # attempt inside the container fails with + # `Could not resolve host: index.crates.io` and stays wedged + # for minutes — both cargo's own retries and an outer step + # retry hit the same dead resolver. Outer retries do not + # rescue that case (we tried 3×30 s in a previous commit and + # the job still drained all three attempts on the wedged + # resolver). Sharing the host's network stack short-circuits + # the bridge resolver entirely; this is safe because each + # matrix entry runs on its own ephemeral runner, so no two + # BDD suites contend for the same loopback ports. + # + # `CARGO_NET_RETRY=10` (default 2) and `CARGO_HTTP_TIMEOUT=60` + # (default 30 s) widen cargo's internal network retry loop for + # the residual case where the host's own resolver works but + # `index.crates.io` itself rate-limits or 5xx's mid-fetch. + # + # The outer 2-attempt retry remains for the timing-sensitive + # BDD flake family (SCRAM passthrough reconnect after retain, + # sleep-based lifecycle waits) that occasionally loses its + # margin under cross-job contention. It is not a network + # safety net — that responsibility moved to `--network=host` + # plus the cargo env vars above. + uses: nick-fields/retry@v3 + with: + timeout_minutes: 30 + max_attempts: 2 + retry_wait_seconds: 5 + command: | + docker run --rm \ + --network=host \ + -e CARGO_NET_RETRY=10 \ + -e CARGO_HTTP_TIMEOUT=60 \ + -v ${{ github.workspace }}:/workspace \ + -w /workspace \ + ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.prepare-tests.outputs.image-tag }} \ + cargo ${{ matrix.suite.cargo }} diff --git a/.github/workflows/dashboard-validation.yml b/.github/workflows/dashboard-validation.yml index 3baa276e0..0d95ad127 100644 --- a/.github/workflows/dashboard-validation.yml +++ b/.github/workflows/dashboard-validation.yml @@ -95,7 +95,24 @@ jobs: > grafana/demo/grafana/provisioning/dashboards/pg_doorman.json - name: Run smoke + ground-truth against grafana/demo - run: make dashboard-validate-ci + # Demo TPS warms up at different speed across shared GitHub + # runners, so a single attempt occasionally hits a panel that + # has not yet populated its rate window. Retry twice with a + # cleanup tear-down between attempts before failing the job. + run: | + set -e + attempt=1 + max_attempts=3 + until make dashboard-validate-ci; do + if [ "$attempt" -ge "$max_attempts" ]; then + echo "dashboard-validate-ci failed after $attempt attempts" >&2 + exit 1 + fi + echo "dashboard-validate-ci attempt $attempt failed; tearing down and retrying" >&2 + (cd grafana/demo && docker compose -f docker-compose.yml -f docker-compose.ci.yml down -v) || true + sleep 5 + attempt=$((attempt + 1)) + done - name: Collect demo logs on failure if: failure() diff --git a/.gitignore b/.gitignore index b44bddaac..ec19ade37 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,7 @@ .idea .junie .claude -/target +target/ .DS_Store .direnv .pre-commit-config.yaml @@ -57,6 +57,12 @@ flamegraph-output/ # Superpowers brainstorm session workdir (VC mockups, server state) .superpowers/ +# Superpowers spec drafts (private session artifacts) +/docs/superpowers/ + +# Internal planning notes (private session artifacts) +/docs/internal-plans/ + # Per-session agent handoff scratchpads — not part of public history. .local/ diff --git a/Cargo.lock b/Cargo.lock index 4aa2fe0e4..c4d783d2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1900,7 +1900,7 @@ checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e" [[package]] name = "pg_doorman" -version = "3.8.5" +version = "3.9.0" dependencies = [ "ahash", "arc-swap", diff --git a/Cargo.toml b/Cargo.toml index 7f6694966..8aadd7d9b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pg_doorman" -version = "3.8.5" +version = "3.9.0" edition = "2021" rust-version = "1.87.0" license = "MIT" diff --git a/documentation/en/src/SUMMARY.md b/documentation/en/src/SUMMARY.md index 4a8d39e26..ac7e2ea36 100644 --- a/documentation/en/src/SUMMARY.md +++ b/documentation/en/src/SUMMARY.md @@ -30,6 +30,7 @@ - [Pool Modes](concepts/pool-modes.md) - [Pool Coordinator](concepts/pool-coordinator.md) - [Anonymous Parse Caching](tutorials/prepared-statements.md) +- [PostgreSQL startup parameters](tutorials/startup-parameters.md) - [Pool Pressure (advanced)](tutorials/pool-pressure.md) # High Availability diff --git a/documentation/en/src/authentication/auth-query.md b/documentation/en/src/authentication/auth-query.md index f8157dbac..6a64933ae 100644 --- a/documentation/en/src/authentication/auth-query.md +++ b/documentation/en/src/authentication/auth-query.md @@ -28,7 +28,11 @@ pools: cache_failure_ttl: "30s" ``` -The query must return a column named `passwd` or `password` containing the MD5 or SCRAM hash. Extra columns are ignored. +The query must return a column named `passwd` or `password` containing +the MD5 or SCRAM hash. Extra columns are ignored except for +`startup_parameters`. In passthrough mode, pg_doorman reads that column +as a `text` JSON object with per-user PostgreSQL startup parameters. +Dedicated mode ignores the column and logs a warning. `user` and `password` are the credentials PgDoorman uses to run the lookup query. They must have permission to read the credential column. Either grant access to a custom view (recommended) or use a user in `pg_read_server_files` group. diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index 515935e79..2fd50dd56 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -1,5 +1,87 @@ # Changelog +### 3.9.0 + +Per-pool PostgreSQL startup parameters. pg_doorman can now add +configured GUCs to each backend `StartupMessage`. Values apply in +three layers: `general.startup_parameters`, `pools..startup_parameters`, +and the optional `startup_parameters` column returned by passthrough +`auth_query`. + +PostgreSQL stores these values as the session reset defaults, so +client-side `RESET ALL` and `DISCARD ALL` return to the configured value. +This gives one pool a different `plan_cache_mode`, `statement_timeout`, +`work_mem`, or `idle_in_transaction_session_timeout` without changing +`postgresql.conf`, `ALTER ROLE`, or `ALTER DATABASE`. + +#### Cascade resolution + +- `general.startup_parameters`, `pools..startup_parameters`, and + the optional `startup_parameters` text column on an `auth_query` row + are applied in order. Later layers override earlier ones per key. +- Dedicated `auth_query` mode uses a shared `server_user`, so + pg_doorman ignores the per-user column there and logs one warning per + pool and username. +- A reload that changes startup parameters recycles the affected pools. + Idle backends with the old reset defaults are not reused. + +#### Validation and protocol safety + +- Reserved protocol keys (`user`, `database`, `replication`, + `options`, the `_pq_.*` extension prefix) are refused at config load. +- Keys must match the PG GUC naming shape `[A-Za-z_][A-Za-z0-9_.]*`, + values must not contain null bytes, and each level fits the startup-parameter + budget of `MAX_STARTUP_PACKET_LENGTH - 512` bytes. +- The resolved parameter set is checked before each backend startup + against PG's 10 000-byte `MAX_STARTUP_PACKET_LENGTH`. If only the + auth_query layer overflows the packet, pg_doorman drops that layer and + keeps the general/pool baseline. If the baseline itself does not fit, + pg_doorman skips all configured keys for that spawn and logs the + byte counts. + +#### Behaviour on PG-side rejection + +- If PostgreSQL rejects a configured startup parameter at backend + startup, pg_doorman returns PostgreSQL's `ErrorResponse` to the + client unchanged. pg_doorman does not retry without the key and does + not disable the key automatically for the pool. Fix the parameter in + the config; until then, backend startup for that pool fails with + PostgreSQL's own SQLSTATE and message. +- SQLSTATEs with the `57P` prefix (server unavailable) keep mapping to + `ServerUnavailableError` first so the Patroni-assisted fallback + path can route around the failed node before the startup-parameter + log line fires. +- The configured parameter wins over the client sync path: + even if the client connect string carries an `application_name` + (or another tracked GUC like `TimeZone`), the per-checkout + `sync_parameters` call no longer overrides the configured value on + the backend. That default stands until an + explicit `SET` statement on the client session changes it. + +#### RELOAD coherence + +- A SIGHUP that changes `general.startup_parameters` drains pools that + inherit that baseline. The per-pool config hash includes the general + startup map, and carried-over dynamic `auth_query` pools are recycled + when the baseline changes. + +#### Observability + +- `pg_doorman_backend_startup_parameter_errors_total{pool, sqlstate}` + counts backend startups PostgreSQL rejected because of an + configured startup parameter. The failing parameter name and username are + written to the warning log line, not to metric labels. +- `SHOW STARTUP_PARAMETERS` (admin SQL console) lists the per-pool + resolved parameters with the source of each value. `psql` tab + completion on `SHOW ` now includes the command. +- The Web UI pool detail page shows the same rows in a "Startup + parameters (configured)" section, driven by the new + `startup_parameters[]` field on `/api/pools`. + +See [PostgreSQL startup parameters](tutorials/startup-parameters.md) +for the configuration walkthrough, plus [General Settings](reference/general.md) +and [Pool Settings](reference/pool.md) for the full parameter list. + ### 3.8.5 The web console now accepts JWTs issued by an external SSO proxy @@ -108,7 +190,7 @@ live in [`guides/web-ui.md`](guides/web-ui.md). **Built-in operator dashboard.** pg_doorman exposes a single-page diagnostic console on the same port as `/metrics`, served from inside the binary and gated on `[web].ui = true` plus a non-default -`admin_password`. Reaching the same view through the existing psql +`admin_password`. Getting comparable detail from the existing psql admin console means running `SHOW POOLS`, `SHOW CLIENTS`, `SHOW STATS` and friends in a loop, computing rates by hand between two snapshots, and joining the rows mentally. The dashboard does diff --git a/documentation/en/src/comparison.md b/documentation/en/src/comparison.md index e512a63f0..e026f5c37 100644 --- a/documentation/en/src/comparison.md +++ b/documentation/en/src/comparison.md @@ -77,6 +77,7 @@ See [Patroni-assisted fallback](tutorials/patroni-assisted-fallback.md), [`patro | LISTEN / NOTIFY pinning in transaction mode | No | No | Experimental | | Cross-rule connection cap (`shared_pool`) | No | No | Yes (since 1.5.1) | | `PAUSE` / `RESUME` / `RECONNECT` admin commands | Yes | Yes | Yes (since 1.4.1) | +| Configured PostgreSQL GUCs in backend `StartupMessage` per pool | Yes (`startup_parameters`, applied as `general` → pool → passthrough `auth_query`; client `RESET ALL` / `DISCARD ALL` returns to those values; PG startup errors reach the client unchanged) | No equivalent configured defaults; selected client startup parameters can be tracked or ignored | No (`maintain_params` preserves client-side parameters across rebind; no configured GUCs) | See [Pool Coordinator](concepts/pool-coordinator.md), [Pool pressure](tutorials/pool-pressure.md). diff --git a/documentation/en/src/guides/web-ui.md b/documentation/en/src/guides/web-ui.md index e86c26ee8..47851675c 100644 --- a/documentation/en/src/guides/web-ui.md +++ b/documentation/en/src/guides/web-ui.md @@ -129,7 +129,8 @@ proxy: | `sso_allowed_users` | Allowlist on the `preferred_username` (or `sub`) claim. `["*"]` accepts every valid JWT; a literal list restricts access to those usernames. | `["*"]` | | `sso_groups_claim` | Name of the JWT claim that carries the user's group memberships. Read together with `sso_admin_groups`. | `"groups"` | | `sso_admin_groups` | Group names that promote an SSO user to `Admin`. Empty keeps every SSO login at the read-only `Sso` role. | `[]` | -| `trusted_proxies` | CIDR ranges trusted to set `X-Forwarded-For` / `Forwarded`. Empty trusts only the listener's own peer. See [Access log](#access-log). | `[]` | +| `sso_require_https` | Reject Bearer/cookie/query SSO credentials presented over plain HTTP. The listener treats a request as secure only when the TCP peer is in `trusted_proxies` and `X-Forwarded-Proto: https` is forwarded. Defaults to off so SSO keeps working through a TLS-terminating proxy that reaches pg_doorman over a private HTTP leg. | `false` | +| `trusted_proxies` | CIDR ranges trusted to set `X-Forwarded-For` / `Forwarded` / `X-Forwarded-Proto`. Empty trusts only the listener's own peer. See [Access log](#access-log). | `[]` | ### Promoting SSO users to Admin via group claim diff --git a/documentation/en/src/observability/admin-commands.md b/documentation/en/src/observability/admin-commands.md index bf374e050..c1481a2ad 100644 --- a/documentation/en/src/observability/admin-commands.md +++ b/documentation/en/src/observability/admin-commands.md @@ -34,6 +34,7 @@ Admin commands are read with `SHOW ` or executed with bare verbs (`P | `SHOW LISTS` | Counts by category (databases, users, pools, clients, servers). | | `SHOW USERS` | List of users and their pool modes. | | `SHOW AUTH_QUERY` | `auth_query` cache hit/miss/refetch rates, auth success/failure, executor errors, dynamic pool counts. | +| `SHOW STARTUP_PARAMETERS` | Resolved `startup_parameters` per pool: parameter, value, source, and application state. | | `SHOW SOCKETS` | TCP and Unix socket counts by state (Linux only — reads `/proc/net/`). | | `SHOW LOG_LEVEL` | Current log level. | | `SHOW VERSION` | PgDoorman version. | @@ -68,6 +69,19 @@ mydb | app | 12 | 4 | 0 | 4 | 36 | 0 - `sv_idle` matches free backends; `sv_active` is in-use; `sv_used` is reserved by the coordinator (see below). - `maxwait` is the longest current wait in seconds. If it grows beyond `query_wait_timeout`, clients get errors. +### `SHOW STARTUP_PARAMETERS` + +``` +user | database | parameter | value | source | state +app | mydb | statement_timeout | 5s | general | applied +app | mydb | plan_cache_mode | force_custom_plan | pool | applied +``` + +- `source` shows where the value came from: `general`, `pool`, or + `auth_query`. +- `state` shows whether the next backend `StartupMessage` will carry + the value: `applied`, `dropped_due_to_budget`, or `stale`. + ### `SHOW POOL_COORDINATOR` ``` @@ -109,7 +123,7 @@ Admin connections do not pass through `pg_hba.conf` rules — they go directly t ## Where to next -- [Prometheus reference](../reference/prometheus.md) — same data, machine-readable. +- [Prometheus reference](../reference/prometheus.md) — the metric form of the same state. - [Pool Coordinator](../concepts/pool-coordinator.md) — what `SHOW POOL_COORDINATOR` is telling you. - [Pool Pressure](../tutorials/pool-pressure.md) — what `SHOW POOL_SCALING` is telling you. - [Troubleshooting](../tutorials/troubleshooting.md) — common failure modes and their `SHOW` output. diff --git a/documentation/en/src/tutorials/startup-parameters.md b/documentation/en/src/tutorials/startup-parameters.md new file mode 100644 index 000000000..1a3f0709b --- /dev/null +++ b/documentation/en/src/tutorials/startup-parameters.md @@ -0,0 +1,174 @@ +# PostgreSQL startup parameters + +Use `startup_parameters` when a pool needs PostgreSQL GUC defaults at +backend startup and you do not want to change `postgresql.conf`, +`ALTER ROLE`, or `ALTER DATABASE`. + +- A hot OLTP pool gets stuck on a generic plan after the + `plan_cache_mode = auto` heuristic flips. Setting + `force_custom_plan` on the role would affect every workload using + that role; setting it on one pool keeps the change local. +- An application that does not set its own `statement_timeout` or + `idle_in_transaction_session_timeout` and cannot be patched fast + enough. The DBA needs a server-side default that survives the + application's own session resets. +- A single application that should announce a stable + `application_name` regardless of what the connecting driver + negotiates, so `pg_stat_activity` and audit logs stay legible. + +## Configuration + +Values apply in three layers. The more specific layer wins per key: + +```toml +[general.startup_parameters] +statement_timeout = "5s" + +[pools.checkout.startup_parameters] +plan_cache_mode = "force_custom_plan" +work_mem = "64MB" +``` + +After `SIGHUP` (or `RELOAD` on the admin console) every new backend +for the `checkout` pool starts with `statement_timeout = 5s`, +`plan_cache_mode = force_custom_plan`, and `work_mem = 64MB`. Other +pools keep `statement_timeout = 5s` from `general` and the PG default +for the rest. Already-open backends are not affected; the change takes +hold as the pool rotates connections. + +When `auth_query` runs in passthrough mode (no `server_user`), the +lookup SQL may return an optional `startup_parameters` text column +holding a JSON object. Values from that column override both +`general` and per-pool settings for that user only: + +```sql +SELECT + rolpassword AS passwd, + CASE rolname + WHEN 'vip' THEN '{"work_mem":"256MB"}'::text + ELSE NULL::text + END AS startup_parameters +FROM pg_authid +WHERE rolname = $1; +``` + +The column must serialize as `text`. If the SQL returns `json` or +`jsonb`, add an explicit `::text` cast. pg_doorman reads the column +as `text` and logs a warning for that fetched row when the type does +not match. + +Dedicated `auth_query` mode (`server_user` set) ignores the per-user +column and logs once per (pool, username): one shared backend serves +many users, so a per-user override cannot apply. + +## What pg_doorman does with the values + +pg_doorman adds the resolved parameter set to the PostgreSQL +`StartupMessage` for each new backend. PostgreSQL records each value as +the session default for that setting (`pg_settings.reset_val` and +`pg_settings.source = 'client'`), so client-side `RESET ALL` and +`DISCARD ALL` return to the configured value. Operators get a stable +session default without editing `postgresql.conf` or running +`ALTER ROLE`. + +The values can be observed from the client: + +```text +checkout=> SHOW plan_cache_mode; + plan_cache_mode +------------------- + force_custom_plan + +checkout=> SET plan_cache_mode = 'auto'; RESET ALL; SHOW plan_cache_mode; + plan_cache_mode +------------------- + force_custom_plan +``` + +## Validation + +At config load: + +- Keys must match PG GUC naming `^[A-Za-z_][A-Za-z0-9_.]*$`. Namespaced + names like `auto_explain.log_min_duration` are accepted; arbitrary + punctuation is not. +- Reserved keys (`user`, `database`, `replication`, `options`, and + anything starting with `_pq_.`) are refused. pg_doorman manages + them itself or PG treats them specially in the StartupMessage. +- Values must not contain null bytes. +- Each level (general or per-pool) must fit within the startup-parameter + budget: `MAX_STARTUP_PACKET_LENGTH` (10 000 bytes) minus 512 bytes + reserved for pg_doorman-managed keys. + +Before each backend spawn pg_doorman checks the resolved parameter set +against the same cap. Two layers that fit on their own can overflow once +`auth_query` adds a third layer. If only the `auth_query` layer pushes +the set over the cap, pg_doorman drops that layer and keeps the +general/pool baseline. If the baseline itself or the full startup packet +does not fit, pg_doorman skips all configured parameters for that +spawn and logs the byte counts. + +## What happens when PG rejects a parameter + +If PostgreSQL rejects a configured parameter at backend startup, +pg_doorman returns PostgreSQL's `ErrorResponse` to the client unchanged. +The client sees the same sqlstate (`22023`, `42704`, `42501`, `55P02`, +or any other code under the startup family) and the same message it +would have seen when connecting to PostgreSQL directly. + +pg_doorman does not retry with the parameter removed and does not +automatically disable that key for the pool. The next client connection +sends the same `StartupMessage` and gets the same error until the +operator fixes the config. + +## Observability + +The admin SQL console shows the resolved parameters for each pool: + +```text +admin> SHOW STARTUP_PARAMETERS; + user | database | parameter | value | source | state +------+----------+-------------------+-------------------+---------+-------- + shop | checkout | plan_cache_mode | force_custom_plan | pool | applied + shop | reports | statement_timeout | 10s | general | applied +``` + +The Web UI shows the same rows on the pool detail page in the "Startup +parameters (configured)" section. + +Prometheus exports counters for both failure points: + +- `pg_doorman_backend_startup_parameter_errors_total{pool, sqlstate}` + counts every backend startup PostgreSQL rejected because of an + configured parameter. The failing parameter name and + username are written to the warning log line, not to metric labels. +- `pg_doorman_startup_parameters_dropped_total{pool, reason}` counts + parameter sets pg_doorman dropped before sending `StartupMessage`. + +Alert when `pg_doorman_backend_startup_parameter_errors_total` keeps +growing for the same pool for several minutes. That usually means new +backend startups for the pool are failing on the same configured GUC. + +## When not to use this + +- The application already sets the parameter on every connection. + Duplicating the value in `startup_parameters` adds another config path + and does not change runtime behavior. +- Per-transaction tuning (`SET LOCAL`). `startup_parameters` is for + session defaults; transaction-scoped tuning belongs in the + application. +- Anything that needs to depend on which query the application is + running. Startup parameters apply to every transaction on every + backend for the lifetime of that backend; there is no + per-statement variant. + +## Reference + +- [General Settings](../reference/general.md): `startup_parameters`. +- [Pool Settings](../reference/pool.md): + `pools..startup_parameters`. +- [auth_query](../authentication/auth-query.md): passthrough vs + dedicated modes, where the `startup_parameters` column is read. +- [Admin Commands](../observability/admin-commands.md): + `SHOW STARTUP_PARAMETERS`. +- [Prometheus](../reference/prometheus.md): full metric list. diff --git a/documentation/ru/src/SUMMARY.md b/documentation/ru/src/SUMMARY.md index 292690582..7cb3a0db2 100644 --- a/documentation/ru/src/SUMMARY.md +++ b/documentation/ru/src/SUMMARY.md @@ -30,6 +30,7 @@ - [Режимы пула](concepts/pool-modes.md) - [Координатор пулов](concepts/pool-coordinator.md) - [Кеш Parse для анонимных prepared statements](tutorials/prepared-statements.md) +- [Параметры запуска PostgreSQL](tutorials/startup-parameters.md) - [Пул под нагрузкой (продвинутое)](tutorials/pool-pressure.md) # Высокая доступность diff --git a/documentation/ru/src/authentication/auth-query.md b/documentation/ru/src/authentication/auth-query.md index 45c7427e1..338d112c1 100644 --- a/documentation/ru/src/authentication/auth-query.md +++ b/documentation/ru/src/authentication/auth-query.md @@ -28,7 +28,12 @@ pools: cache_failure_ttl: "30s" ``` -Запрос должен возвращать колонку с именем `passwd` или `password`, содержащую хеш MD5 или SCRAM. Дополнительные колонки игнорируются. +Запрос должен возвращать колонку с именем `passwd` или `password`, +содержащую хеш MD5 или SCRAM. Дополнительные колонки игнорируются, кроме +необязательной `startup_parameters`. В passthrough-режиме pg_doorman +читает её как JSON-объект в `text` с пользовательскими параметрами +запуска PostgreSQL. Dedicated-режим игнорирует её и пишет +предупреждение. `user` и `password` — это учётные данные, под которыми pg_doorman выполняет lookup-запрос. У них должно быть право читать колонку с учётными данными. Либо выдайте доступ к специально созданному представлению (рекомендуется), либо используйте пользователя из группы `pg_read_server_files`. diff --git a/documentation/ru/src/authentication/hba.md b/documentation/ru/src/authentication/hba.md index b38d5379f..d461ca78a 100644 --- a/documentation/ru/src/authentication/hba.md +++ b/documentation/ru/src/authentication/hba.md @@ -107,7 +107,7 @@ host all all 0.0.0.0/0 reject ## Отличия от pg_hba.conf PostgreSQL -- Нет ключевого слова `replication` (pg_doorman не пробрасывает соединения репликации). +- Нет ключевого слова `replication` (pg_doorman не обслуживает соединения репликации). - Нет методов `peer`, `ident`, `cert`, `gss`, `sspi`, `pam`. PAM настраивается на пользователя через `auth_pam_service`, не через HBA. - Нет префикса `+groupname` для пользователя. - Нет регулярных выражений (синтаксис `/regex`). diff --git a/documentation/ru/src/authentication/jwt.md b/documentation/ru/src/authentication/jwt.md index 86eba17e4..f3a79674a 100644 --- a/documentation/ru/src/authentication/jwt.md +++ b/documentation/ru/src/authentication/jwt.md @@ -1,6 +1,6 @@ # Аутентификация JWT -Аутентифицируйте клиентов JSON Web Token, подписанным внешним поставщиком идентификации. pg_doorman проверяет подпись токена RSA-SHA256 по публичному ключу с диска, сверяет claim `preferred_username` и пробрасывает соединение в PostgreSQL под заданной идентичностью бэкенда. +Аутентифицируйте клиентов JSON Web Token, подписанным внешним поставщиком идентификации. pg_doorman проверяет подпись токена RSA-SHA256 по публичному ключу с диска, сверяет claim `preferred_username` и открывает соединение PostgreSQL под заданной идентичностью бэкенда. Этот метод подходит для доступа сервиса к базе, когда короткоживущие токены выпускает OIDC-провайдер, Vault или внутренний токен-сервис. diff --git a/documentation/ru/src/comparison.md b/documentation/ru/src/comparison.md index b387eec0f..43624238d 100644 --- a/documentation/ru/src/comparison.md +++ b/documentation/ru/src/comparison.md @@ -77,6 +77,7 @@ PgCat намеренно опущен: у него центр тяжести — | LISTEN / NOTIFY pinning в transaction mode | Нет | Нет | Экспериментально | | Cross-rule connection cap (`shared_pool`) | Нет | Нет | Да (с 1.5.1) | | Команды администратора `PAUSE` / `RESUME` / `RECONNECT` | Да | Да | Да (с 1.4.1) | +| GUC PostgreSQL на уровне пула в backend `StartupMessage` | Да (`startup_parameters`: `general` → пул → passthrough `auth_query`; клиентские `RESET ALL` / `DISCARD ALL` возвращают эти значения; ошибки PG при запуске бэкенда доходят до клиента без переписывания) | Нет эквивалентных операторских значений по умолчанию; отдельные клиентские startup-параметры можно отслеживать или игнорировать | Нет (`maintain_params` сохраняет клиентские параметры при rebind; операторских GUC нет) | См. [Координатор пулов](concepts/pool-coordinator.md), [Пул под нагрузкой](tutorials/pool-pressure.md). diff --git a/documentation/ru/src/concepts/pool-modes.md b/documentation/ru/src/concepts/pool-modes.md index 26b30ac93..3027a7e7f 100644 --- a/documentation/ru/src/concepts/pool-modes.md +++ b/documentation/ru/src/concepts/pool-modes.md @@ -76,7 +76,7 @@ pools: Что сбрасывается, когда сработал флаг: - Флаг `SET` → `RESET ALL` сбрасывает session-level GUCs и неявно вызывает `pg_advisory_unlock_all`. -- Флаг `PREPARE` → `DEALLOCATE ALL` удаляет PostgreSQL-side prepared statements, которые драйвер именовал явно. Собственный кеш prepared statements pg_doorman переживает сброс — он индексируется текстом запроса, а не backend-именем. +- Флаг `PREPARE` → `DEALLOCATE ALL` удаляет PostgreSQL-side prepared statements, которые драйвер именовал явно. Собственный кеш prepared statements pg_doorman сохраняется после сброса: он индексируется текстом запроса, а не backend-именем. - Флаг `DECLARE CURSOR` → `CLOSE ALL` закрывает курсоры. `DEALLOCATE ALL` и `DISCARD ALL` со стороны клиента очищают prepared-statement-кеш именно этого клиента (следующий `Parse` зарегистрируется заново). Pool-level shared cache не затрагивается; у других клиентов их записи сохраняются. diff --git a/documentation/ru/src/guides/web-ui.md b/documentation/ru/src/guides/web-ui.md index 99ea7253c..580fa4992 100644 --- a/documentation/ru/src/guides/web-ui.md +++ b/documentation/ru/src/guides/web-ui.md @@ -54,7 +54,7 @@ log_tap_max_entries = 8192 Сервер на каждом запросе вычисляет одну из трёх ролей. Проверка работает на стороне сервера; SPA дублирует её на клиенте только для того, чтобы -скрыть кнопки, которыми оператор всё равно не может воспользоваться. +не показывать действия, недоступные текущему оператору. | Роль | Как запрос её получает | Что роль даёт | |---|---|---| @@ -129,7 +129,8 @@ SSO опциональный. По умолчанию (`[web].sso_enabled = fals | `sso_allowed_users` | Allowlist по claim `preferred_username` (или `sub`). `["*"]` принимает любого. Иначе пропускаются только перечисленные имена. | `["*"]` | | `sso_groups_claim` | Имя JWT-claim, в котором лежат группы пользователя. Читается вместе с `sso_admin_groups`. | `"groups"` | | `sso_admin_groups` | Группы, которые поднимают SSO-пользователя до `Admin`. Пустой список оставляет каждый SSO-логин на роли `Sso` только для чтения. | `[]` | -| `trusted_proxies` | CIDR доверенных обратных прокси. Пустой список — доверять только непосредственному TCP-peer. См. [Журнал доступа](#журнал-доступа). | `[]` | +| `sso_require_https` | Отклонять Bearer/cookie/query SSO-credentials, пришедшие по plain HTTP. Запрос считается защищённым только если TCP-peer входит в `trusted_proxies` и прокси прислал `X-Forwarded-Proto: https`. По умолчанию выключено, чтобы SSO продолжал работать в схеме «TLS терминирует прокси → pg_doorman слушает HTTP во внутренней сети». | `false` | +| `trusted_proxies` | CIDR доверенных обратных прокси (используется для `X-Forwarded-For` / `Forwarded` / `X-Forwarded-Proto`). Пустой список — доверять только непосредственному TCP-peer. См. [Журнал доступа](#журнал-доступа). | `[]` | ### Поднятие SSO-пользователя до Admin через claim с группами @@ -247,7 +248,7 @@ cookie `sso_access_token` существует для сайдкаров, curl Basic-пароль по умолчанию живёт только в памяти React и пропадает после полной перезагрузки страницы. Галочка **Remember me on this device** в форме входа -сохраняет его в `localStorage`, и консоль переживает перезагрузку. +сохраняет его в `localStorage`, поэтому консоль открывается без повторного ввода. Очистка хранилища сайта в браузере удаляет и Basic, и SSO-запись. ## Журнал доступа diff --git a/documentation/ru/src/observability/admin-commands.md b/documentation/ru/src/observability/admin-commands.md index 6269abb1e..b3c277678 100644 --- a/documentation/ru/src/observability/admin-commands.md +++ b/documentation/ru/src/observability/admin-commands.md @@ -34,6 +34,7 @@ psql "host=127.0.0.1 port=6432 user=admin dbname=pgdoorman" | `SHOW LISTS` | Счётчики по категориям (databases, users, pools, clients, servers). | | `SHOW USERS` | Список пользователей и их режимы пула. | | `SHOW AUTH_QUERY` | Кэш `auth_query`: попадания/промахи/перезапросы, успехи/отказы аутентификации, ошибки исполнителя, счётчики динамических пулов. | +| `SHOW STARTUP_PARAMETERS` | Итоговые `startup_parameters` по каждому пулу: параметр, значение, источник и состояние применения. | | `SHOW SOCKETS` | Счётчики TCP- и Unix-сокетов по состоянию (только Linux — читает `/proc/net/`). | | `SHOW LOG_LEVEL` | Текущий уровень логирования. | | `SHOW VERSION` | Версия pg_doorman. | @@ -68,6 +69,19 @@ mydb | app | 12 | 4 | 0 | 4 | 36 | 0 - `sv_idle` соответствует свободным серверным соединениям; `sv_active` — занятым; `sv_used` — зарезервированным координатором (см. ниже). - `maxwait` — самое долгое текущее ожидание в секундах. Если оно вырастает за `query_wait_timeout`, клиенты получают ошибки. +### `SHOW STARTUP_PARAMETERS` + +``` +user | database | parameter | value | source | state +app | mydb | statement_timeout | 5s | general | applied +app | mydb | plan_cache_mode | force_custom_plan | pool | applied +``` + +- `source` показывает источник значения: `general`, `pool` или + `auth_query`. +- `state` показывает, будет ли значение отправлено в ближайший + `StartupMessage`: `applied`, `dropped_due_to_budget` или `stale`. + ### `SHOW POOL_COORDINATOR` ``` diff --git a/documentation/ru/src/operations/monitoring-interner.md b/documentation/ru/src/operations/monitoring-interner.md index 6cdcd8b88..fabce80b3 100644 --- a/documentation/ru/src/operations/monitoring-interner.md +++ b/documentation/ru/src/operations/monitoring-interner.md @@ -31,7 +31,7 @@ pg_doorman. Хранилище разделено на две независим `rate(pg_doorman_query_interner_synthetic_misses_total[5m])`. Норма — плоский ноль. Любой всплеск означает: либо TTL вытеснил запись, на которую сослался клиент, либо драйвер рассчитывает - на безымянный prepared statement, переживший Sync. + на безымянный prepared statement, который остаётся доступным после Sync. ### Детализация diff --git a/documentation/ru/src/reference/general.md b/documentation/ru/src/reference/general.md index 43b0e64be..badb9ba8f 100644 --- a/documentation/ru/src/reference/general.md +++ b/documentation/ru/src/reference/general.md @@ -673,6 +673,32 @@ hostnossl all all 192.168.1.0/24 trust - Для методов аутентификации, отличных от `trust`, PgDoorman выполняет соответствующий challenge/response с клиентом. - Для потоков Talos/JWT/PAM, настроенных на уровне пула или пользователя, `trust` всё равно обходит запрос пароля у клиента; однако эти режимы могут использоваться, если `trust` не совпал. +### startup_parameters + +Базовые параметры PostgreSQL, которые pg_doorman добавляет в +`StartupMessage` каждого нового бэкенда. `startup_parameters` уровня +пула переопределяют эти значения по ключу, а passthrough `auth_query` +может переопределить их для конкретного пользователя. + +При загрузке конфигурации pg_doorman проверяет зарезервированные +протокольные ключи (`user`, `database`, `replication`, `options`, +`_pq_.*`), имена GUC, нулевые байты и размер этого уровня. Перед каждым +запуском бэкенда объединённый набор параметров снова проверяется по +лимиту `MAX_STARTUP_PACKET_LENGTH` PostgreSQL; если он не помещается, +pg_doorman пропускает операторские параметры для этого запуска и пишет +предупреждение. + +Если PostgreSQL отвергает параметр при запуске бэкенда, pg_doorman +возвращает клиенту `ErrorResponse` PostgreSQL без изменений: повторной +попытки без этого ключа не будет, сам ключ автоматически не отключается. +Накопительный счётчик отказов экспортируется как +`pg_doorman_backend_startup_parameter_errors_total{pool, sqlstate}`; +имя параметра и пользователя остаются в строке лога уровня `warn`. +Итоговые параметры по каждому пулу видны через `SHOW STARTUP_PARAMETERS` +в административной SQL-консоли и через `/api/pools` в веб-интерфейсе. + +По умолчанию: `{}`. + ### pooler_check_query Когда клиент отправляет ровно этот запрос как SimpleQuery, pg_doorman отвечает немедленно diff --git a/documentation/ru/src/reference/pool.md b/documentation/ru/src/reference/pool.md index 5932c3f44..d7f06e2b9 100644 --- a/documentation/ru/src/reference/pool.md +++ b/documentation/ru/src/reference/pool.md @@ -132,6 +132,23 @@ По умолчанию: `0 (no protection)`. +### startup_parameters + +Параметры PostgreSQL уровня пула, которые pg_doorman добавляет в +`StartupMessage` каждого нового бэкенда. Значения переопределяют +`general.startup_parameters` по ключу. В passthrough-пулах `auth_query` +колонка `startup_parameters` может переопределить и этот уровень для +конкретного пользователя. + +При загрузке конфигурации pg_doorman проверяет зарезервированные +протокольные ключи, имена GUC, нулевые байты и размер этого уровня. Перед +каждым запуском бэкенда объединённый набор параметров снова проверяется +по лимиту `MAX_STARTUP_PACKET_LENGTH` PostgreSQL; если он не помещается, +pg_doorman пропускает операторские параметры для этого запуска и пишет +предупреждение. + +По умолчанию: `{}`. + ## Настройки auth_query Секция `auth_query` включает динамическую аутентификацию пользователей через запрос учётных данных @@ -186,7 +203,16 @@ auth_query: ### query -SQL-запрос для получения учётных данных. Должен возвращать колонку с именем `passwd` или `password`, содержащую MD5- или SCRAM-хеш. Если запрос возвращает ровно одну колонку, она используется независимо от имени. Любые лишние колонки игнорируются. В качестве плейсхолдера для имени пользователя используйте `$1`. +SQL-запрос для получения учётных данных. Он должен возвращать колонку с +именем `passwd` или `password`, содержащую MD5- или SCRAM-хеш. Если +запрос возвращает ровно одну колонку, pg_doorman использует её независимо +от имени. + +Дополнительные колонки игнорируются, кроме необязательной +`startup_parameters` типа `text`. В passthrough-режиме pg_doorman читает +её как JSON-объект с параметрами запуска PostgreSQL для этого пользователя. +Dedicated-режим игнорирует эту колонку и пишет предупреждение. В +качестве плейсхолдера для имени пользователя используйте `$1`. Пример: `"SELECT passwd FROM pg_shadow WHERE usename = $1"` diff --git a/documentation/ru/src/reference/prometheus.md b/documentation/ru/src/reference/prometheus.md index 6fc0adc2f..18e2ff016 100644 --- a/documentation/ru/src/reference/prometheus.md +++ b/documentation/ru/src/reference/prometheus.md @@ -66,6 +66,8 @@ pg_doorman экспортирует следующие метрики: | `pg_doorman_pools_bytes_total` | Накопительный счётчик байт, переданных через пулы соединений, по направлению (`received`/`sent`), пользователю и базе. Для пропускной способности используйте `rate(pg_doorman_pools_bytes_total[5m])`. | | `pg_doorman_pools_bytes` | Устаревшая gauge-версия `pg_doorman_pools_bytes_total`; будет удалена в 3.10. | | `pg_doorman_pool_size` | Сконфигурированный максимальный размер пула на пользователя и базу. Полезен для расчёта оставшейся ёмкости пула вместе с pg_doorman_pools_servers. | +| `pg_doorman_backend_startup_parameter_errors_total` | Накопительный счётчик запусков бэкенда, которые PostgreSQL отклонил из-за `startup_parameters`. Лейблы: пул и SQLSTATE. Отклонённый параметр и имя пользователя пишутся в строку лога уровня `warn`, а не в лейблы метрики. | +| `pg_doorman_startup_parameters_dropped_total` | Накопительный счётчик событий, когда pg_doorman отбросил `startup_parameters` до отправки `StartupMessage`. Лейблы: пул и причина (`cascade_budget_exceeded`, `packet_cap_exceeded`, `auth_query_oversize`, `auth_query_overlay_oversize`, `auth_query_bad_type`, `auth_query_invalid_json`, `auth_query_invalid_shape`, `auth_query_invalid_entry`, `dedicated_mode`). | ### Метрики запросов и транзакций diff --git a/documentation/ru/src/tutorials/binary-upgrade.md b/documentation/ru/src/tutorials/binary-upgrade.md index 91eaafd49..ad9682f84 100644 --- a/documentation/ru/src/tutorials/binary-upgrade.md +++ b/documentation/ru/src/tutorials/binary-upgrade.md @@ -184,10 +184,10 @@ now`), и старый процесс выходит. - Если `client_anonymous_prepared_cache_size` нового конфига меньше, лишние anonymous-записи вытесняются по LRU. Именованная часть не - ограничена и переживает миграцию полностью. Оставшиеся записи + ограничена и переносится полностью. Оставшиеся записи работают нормально. -- Anonymous prepared statements (`Parse` с пустым именем) переживают - миграцию, но требуют повторного `Parse` перед `Bind` в новом процессе. +- Anonymous prepared statements (`Parse` с пустым именем) переносятся в + новый процесс, но требуют повторного `Parse` перед `Bind`. - `DEALLOCATE ALL` после миграции очищает переданный кеш. Повторный `Parse` с тем же именем использует новый текст запроса. diff --git a/documentation/ru/src/tutorials/patroni-proxy.md b/documentation/ru/src/tutorials/patroni-proxy.md index 0091a37e3..ea62c59ba 100644 --- a/documentation/ru/src/tutorials/patroni-proxy.md +++ b/documentation/ru/src/tutorials/patroni-proxy.md @@ -6,7 +6,7 @@ - **Открытие членов кластера** через опрос `/cluster` с интервалом `cluster_update_interval` (по умолчанию 3 с) и по запросу `GET /update_clusters`. - **Маршрутизация по ролям.** Каждый listen-порт привязан к одной или нескольким ролям (`leader`, `sync`, `async`, `any`). Соединения с этого порта попадают на члена, у которого совпадает одна из указанных ролей. -- **Least-connections.** Для портов с несколькими допустимыми членами прокси держит счётчик соединений на каждого члена и отправляет новое соединение туда, где их меньше. Счётчики переживают cluster update. +- **Least-connections.** Для портов с несколькими допустимыми членами прокси держит счётчик соединений на каждого члена и отправляет новое соединение туда, где их меньше. Обновление кластера не сбрасывает эти счётчики. - **Отбрасывает реплики со старыми данными.** `max_lag_in_bytes` per-port исключает членов, у которых `replication_lag` (из `/cluster`) выше порога. Leader по лагу никогда не исключается. - **Пропускает не-running.** Допускаются только члены со `state: "running"`; `starting`, `stopped`, `crashed` и узлы с тегом `noloadbalance` фильтруются. diff --git a/documentation/ru/src/tutorials/pool-pressure.md b/documentation/ru/src/tutorials/pool-pressure.md index 884ab5b11..205bc66bb 100644 --- a/documentation/ru/src/tutorials/pool-pressure.md +++ b/documentation/ru/src/tutorials/pool-pressure.md @@ -906,7 +906,7 @@ herd) возвращается. с 50 ms и неделю наблюдайте `reserve_acq` и `evictions`. 4. Оставьте `min_connection_lifetime` на дефолте 30 000 ms, если у вас нет явной цели ускорить кросс-пуловую ребалансировку; - понижение увеличивает частоту eviction и churn соединений. + понижение увеличивает частоту выселений и повторных подключений. За чем следить после каждого изменения (все в `SHOW POOL_COORDINATOR`): @@ -1336,7 +1336,7 @@ PgBouncer и pg_doorman оба пулят соединения, но давле | Одновременные `connect()` к бэкенду в одном пуле | Однопоточный, обрабатывает события последовательно в пределах пула, вызовы `connect()` выпускаются по одному. | Ограничено `scaling_max_parallel_creates` (по умолчанию 2 на пул): не больше N одновременных подключений к PostgreSQL в пуле, лишние задачи ждут на ограничитель всплесков. | | Anticipation возвратов | Нет. Клиенты ждут следующего доступного соединения в порядке прихода, в пределах `wait_timeout`. | Event-driven anticipation: возвращающееся соединение будит ровно одного из ожидающих в очереди, часто ещё до того, как будет выпущен хоть один новый `connect()`. | | Прогрев `min_pool_size` | Поддерживается на каждом такте event loop (без отдельной задачи replenish). | Периодический фоновый replenish (`retain_connections_time`, по умолчанию 30 s), который отступает, когда ограничитель всплесков занят. | -| Повторный логин после ошибки | `server_login_retry` (по умолчанию 15 s) блокирует новые попытки логина после отказа бэкенда. | Аналога нет. Ошибки логина бэкенда пробрасываются клиенту на каждую попытку. | +| Повторный логин после ошибки | `server_login_retry` (по умолчанию 15 s) блокирует новые попытки логина после отказа бэкенда. | Аналога нет. Ошибки логина бэкенда возвращаются клиенту на каждую попытку. | | Jitter на lifetime | Нет. `server_lifetime` точный. | ±20% jitter на `server_lifetime` и `idle_timeout`, чтобы избежать одновременного массового закрытия. | | Ключ поиска пула | `(database, user, auth_type)` | `(database, user)` | | Честность между пользователями на общем лимите | First come first served на `max_db_connections`. | Reserve arbiter оценивает запросы по `(starving, queued_clients)`. | diff --git a/documentation/ru/src/tutorials/prepared-statements.md b/documentation/ru/src/tutorials/prepared-statements.md index 63375f0c6..154e02cbd 100644 --- a/documentation/ru/src/tutorials/prepared-statements.md +++ b/documentation/ru/src/tutorials/prepared-statements.md @@ -18,8 +18,8 @@ PgDoorman прозрачно переписывает каждый аноним Это уникальная возможность PgDoorman. PgBouncer (1.21+) и Odyssey поддерживают prepared statements в transaction mode, но только для -**именованных** statement; анонимный `Parse` пробрасывается без -изменений и перепланируется при каждом обращении. Ограничения кеша, +**именованных** statement; анонимный `Parse` проходит без +изменений и планируется заново при каждом обращении. Ограничения кеша, LRU, TTL и observability нужны не вместо производительности, а чтобы эта оптимизация оставалась управляемой под динамическим SQL. @@ -69,7 +69,7 @@ psycopg. Прикладной код выглядит как обычный па ## Почему это проблема в transaction-mode В transaction pooling один backend по очереди обслуживает разных -клиентов. Если пулер пробрасывает пустой `Parse` как есть, каждый +клиентов. Если пулер отправляет пустой `Parse` как есть, каждый `Bind` клиента приходит на backend, у которого плана для этого запроса нет. Горячие OLTP-пути платят CPU планировщика на каждом обращении. @@ -225,12 +225,12 @@ PgDoorman держит состояние prepared statements на трёх ур | Пулер | Кеш Parse/плана для анонимного prepared statement | | --------------- | :--------------------------------------------------- | | **PgDoorman** | Да: прозрачная подмена на `DOORMAN_` | -| PgBouncer 1.21+ | Нет: только named, анонимный пробрасывается as-is | +| PgBouncer 1.21+ | Нет: только named, анонимный проходит as-is | | Odyssey | Нет: только named, `pool_reserve_prepared_statement` | | PgCat | Нет: только named | В PgBouncer поддержка prepared statements появилась в 1.21, но -ограничена **именованными**: анонимный `Parse` пробрасывается без +ограничена **именованными**: анонимный `Parse` проходит без изменений, и каждый `Bind` запускает планировщик. Флаг `pool_reserve_prepared_statement` в Odyssey требует именованных statement; на анонимный трафик он не влияет. PgCat ведёт себя diff --git a/documentation/ru/src/tutorials/startup-parameters.md b/documentation/ru/src/tutorials/startup-parameters.md new file mode 100644 index 000000000..7c1fdd0ae --- /dev/null +++ b/documentation/ru/src/tutorials/startup-parameters.md @@ -0,0 +1,184 @@ +# Параметры запуска PostgreSQL + +PgDoorman может задавать параметры PostgreSQL при открытии серверного +соединения, не меняя `postgresql.conf`, `ALTER ROLE` или +`ALTER DATABASE`. Это полезно, например, в таких случаях: + +- В горячем OLTP-пуле план переключается на generic после решения + эвристики `plan_cache_mode = auto` и обратно уже не возвращается. + `ALTER ROLE SET plan_cache_mode = force_custom_plan` затронет любую + другую нагрузку под этой ролью, а изменить нужно только один пул. +- Приложение не задаёт `statement_timeout` или + `idle_in_transaction_session_timeout`, а быстро доработать его нельзя. + Администратору БД нужно сессионное значение по умолчанию, которое + сохранится после клиентского `RESET ALL`. +- Одно приложение должно стабильно показывать конкретный + `application_name`, независимо от значения, которое передаст драйвер, + чтобы `pg_stat_activity` и аудит оставались читаемыми. + +Для этого используется `startup_parameters`: набор GUC PostgreSQL, +который pg_doorman добавляет в `StartupMessage` новых серверных +соединений пула. + +## Конфигурация + +Значения применяются в три слоя. Более узкий слой переопределяет ключ +из предыдущего. + +```toml +[general.startup_parameters] +statement_timeout = "5s" + +[pools.checkout.startup_parameters] +plan_cache_mode = "force_custom_plan" +work_mem = "64MB" +``` + +После `SIGHUP` или `RELOAD` через консоль администратора каждое новое +серверное соединение пула `checkout` открывается со значениями +`statement_timeout = 5s`, `plan_cache_mode = force_custom_plan` и +`work_mem = 64MB`. В других пулах остаётся только +`statement_timeout = 5s` из `general`; остальные значения берутся из +настроек PostgreSQL по умолчанию. Уже открытые бэкенды не меняются: +новые значения вступают в силу по мере ротации соединений. + +В passthrough-режиме `auth_query`, когда `server_user` не задан, запрос +аутентификации может вернуть необязательную колонку `startup_parameters` +типа `text` с JSON-объектом. Значения из этой колонки переопределяют +`general` и настройки пула только для конкретного пользователя. + +```sql +SELECT + rolpassword AS passwd, + CASE rolname + WHEN 'vip' THEN '{"work_mem":"256MB"}'::text + ELSE NULL::text + END AS startup_parameters +FROM pg_authid +WHERE rolname = $1; +``` + +Колонка должна возвращаться как `text`. Если SQL отдаёт `json` или +`jsonb`, добавьте явное приведение типа `::text`. pg_doorman читает её +именно как `text` и пишет предупреждение для полученной строки, если +тип не совпал. + +Dedicated-режим `auth_query`, когда `server_user` задан, игнорирует эту +колонку и один раз пишет предупреждение на пару `(пул, пользователь)`. +В этом режиме один серверный пул обслуживает разных пользователей, +поэтому per-user значения применить нельзя. + +## Что pg_doorman делает со значениями + +pg_doorman добавляет итоговый набор параметров в `StartupMessage` +каждого нового бэкенда. PostgreSQL сохраняет эти значения как +сессионные значения по умолчанию (`pg_settings.reset_val` и +`pg_settings.source = 'client'`). Поэтому клиентские `RESET ALL` и +`DISCARD ALL` возвращают операторские значения, а не исходные +значения PostgreSQL. + +Значение видно со стороны клиента: + +```text +checkout=> SHOW plan_cache_mode; + plan_cache_mode +------------------- + force_custom_plan + +checkout=> SET plan_cache_mode = 'auto'; RESET ALL; SHOW plan_cache_mode; + plan_cache_mode +------------------- + force_custom_plan +``` + +## Валидация + +При загрузке конфигурации pg_doorman проверяет: + +- Имена ключей должны соответствовать маске GUC PostgreSQL: + `^[A-Za-z_][A-Za-z0-9_.]*$`. Составные имена вроде + `auto_explain.log_min_duration` допустимы; произвольная пунктуация + нет. +- Зарезервированные ключи (`user`, `database`, `replication`, `options` + и всё, что начинается с `_pq_.`) отклоняются. pg_doorman управляет + ими сам, либо PostgreSQL обрабатывает их в `StartupMessage` особым + образом. +- Значения не должны содержать нулевой байт. +- Каждый уровень (`general` или `pool`) должен помещаться в лимит для + операторских параметров: `MAX_STARTUP_PACKET_LENGTH` (10000 байт) + минус 512 байт, зарезервированных под служебные ключи pg_doorman. + +Перед запуском каждого бэкенда pg_doorman заново проверяет объединённый +набор параметров по тому же лимиту. Два уровня, которые помещались по +отдельности, могут вместе выйти за лимит, особенно когда `auth_query` +добавляет третий слой. Если лимит превышает только слой `auth_query`, +pg_doorman отбрасывает этот слой и сохраняет baseline из `general` и +пула. Если не помещается сам baseline или полный startup-пакет, +pg_doorman пропускает все операторские параметры для этого +запуска и пишет размеры в лог. + +## Что происходит, если PG отвергает параметр + +Если PostgreSQL отвергает заданный оператором параметр при запуске +бэкенда, pg_doorman возвращает клиенту `ErrorResponse` PostgreSQL без +изменений. Клиент видит тот же sqlstate (`22023`, `42704`, `42501`, +`55P02` или любой другой код из стартового семейства) и то же сообщение, +что увидел бы при прямом подключении к PostgreSQL. + +pg_doorman не пробует повторить подключение без отклонённого параметра +и не отключает этот ключ автоматически для пула. Следующее подключение +клиента отправит тот же `StartupMessage` и получит ту же ошибку, пока +оператор не исправит конфигурацию. + +## Наблюдаемость + +Итоговые параметры по каждому пулу видны через административную +SQL-консоль: + +```text +admin> SHOW STARTUP_PARAMETERS; + user | database | parameter | value | source | state +------+----------+-------------------+-------------------+---------+-------- + shop | checkout | plan_cache_mode | force_custom_plan | pool | applied + shop | reports | statement_timeout | 10s | general | applied +``` + +Веб-интерфейс показывает эти же строки на странице пула в секции +«Startup parameters (operator-injected)». + +В Prometheus: + +- `pg_doorman_backend_startup_parameter_errors_total{pool, sqlstate}` + считает попытки запуска бэкенда, которые PostgreSQL отклонил из-за + параметра, заданного оператором. Имя параметра и пользователя + пишутся в строку лога уровня `warn`; в лейблы они не включены, чтобы + динамические `auth_query`-пулы не раздували количество серий. + +Разумная отправная точка для алерта: если +`pg_doorman_backend_startup_parameter_errors_total` растёт по одному и +тому же пулу несколько минут подряд, новые подключения к этому пулу +падают на одном и том же GUC. Конфигурацию нужно исправить до возврата +трафика. + +## Когда это не нужно + +- Приложение само задаёт параметр на каждом подключении. Дублирование в + `startup_parameters` добавляет ещё одну настройку без изменения + поведения. +- Тюнинг на одну транзакцию (`SET LOCAL`). `startup_parameters` задают + сессионные значения по умолчанию; параметры уровня транзакции должно + выставлять приложение. +- Значения, которые зависят от текущего запроса. Параметры запуска + действуют для всех транзакций бэкенда на протяжении его жизни; + режима «на один statement» нет. + +## Справочник + +- [Общие настройки](../reference/general.md): `startup_parameters`. +- [Настройки пула](../reference/pool.md): + `pools..startup_parameters`. +- [auth_query](../authentication/auth-query.md): passthrough- и + dedicated-режимы, чтение колонки `startup_parameters`. +- [Команды администратора](../observability/admin-commands.md): + `SHOW STARTUP_PARAMETERS`. +- [Метрики Prometheus](../reference/prometheus.md): полный список. diff --git a/documentation/ru/src/tutorials/troubleshooting.md b/documentation/ru/src/tutorials/troubleshooting.md index 1342c129d..ab73289fc 100644 --- a/documentation/ru/src/tutorials/troubleshooting.md +++ b/documentation/ru/src/tutorials/troubleshooting.md @@ -18,7 +18,7 @@ SELECT usename, passwd FROM pg_shadow WHERE usename = 'your_user'; ### Когда username пула отличается от роли на backend -Когда обращённый к клиенту `username` в PgDoorman не совпадает с реальной ролью PostgreSQL, passthrough работать не может — нечего пробрасывать. Дайте явные credentials: +Когда обращённый к клиенту `username` в PgDoorman не совпадает с реальной ролью PostgreSQL, passthrough работать не может: у pg_doorman нет пароля для backend-роли. Дайте явные credentials: ```yaml users: @@ -29,7 +29,7 @@ users: pool_size: 40 ``` -Это же путь для JWT-аутентификации, где клиент не присылает пароль и пробрасывать нечего. +Это же путь для JWT-аутентификации, где клиент не присылает пароль. ```admonish tip title="Где взять хеш пароля" `pg_doorman generate --host …` интроспектирует PostgreSQL и собирает конфиг с уже подставленными хешами. Быстрее, чем копировать руками из `pg_shadow`. diff --git a/frontend/dist/.source-hash b/frontend/dist/.source-hash index a917e499e..30b03cc7b 100644 --- a/frontend/dist/.source-hash +++ b/frontend/dist/.source-hash @@ -1 +1 @@ -814730d21a0d7e29a720341a7e95ab7920719785ae0d25c506c66d0703113e2d +c354cdb48204d35e96a6e6ea90ef3743de01fc3841c0516829bb812c40c924c8 diff --git a/frontend/dist/assets/index-ANwga4xD.css.gz b/frontend/dist/assets/index-ANwga4xD.css.gz deleted file mode 100644 index 4af9830e5..000000000 Binary files a/frontend/dist/assets/index-ANwga4xD.css.gz and /dev/null differ diff --git a/frontend/dist/assets/index-BPAqXFlE.js.gz b/frontend/dist/assets/index-BPAqXFlE.js.gz new file mode 100644 index 000000000..51d504fac Binary files /dev/null and b/frontend/dist/assets/index-BPAqXFlE.js.gz differ diff --git a/frontend/dist/assets/index-DYwzK8Nu.css.gz b/frontend/dist/assets/index-DYwzK8Nu.css.gz new file mode 100644 index 000000000..64f4e9f74 Binary files /dev/null and b/frontend/dist/assets/index-DYwzK8Nu.css.gz differ diff --git a/frontend/dist/assets/index-Dl7lSMbo.js.gz b/frontend/dist/assets/index-Dl7lSMbo.js.gz deleted file mode 100644 index 1032fb455..000000000 Binary files a/frontend/dist/assets/index-Dl7lSMbo.js.gz and /dev/null differ diff --git a/frontend/dist/index.html.gz b/frontend/dist/index.html.gz index 8af13ee11..5ecf0ea58 100644 Binary files a/frontend/dist/index.html.gz and b/frontend/dist/index.html.gz differ diff --git a/frontend/src/components/AuthGate.tsx b/frontend/src/components/AuthGate.tsx index 534207791..ed6ba7d81 100644 --- a/frontend/src/components/AuthGate.tsx +++ b/frontend/src/components/AuthGate.tsx @@ -265,6 +265,12 @@ function AuthModal({ onSubmit({ username, password }, remember); }; + // Bloomberg-Terminal sign-in surface: full-page, no modal chrome. + // The whole canvas reads as a single carved console, with the amber + // accent reserved for the primary SSO action and the live status + // line at the bottom. We deliberately avoid the gradient/glass + // cliché — the rest of the SPA never uses it, and an operator + // landing here should feel they are still in the same console. return (
-
-

- Sign in -

- {ssoConfigError && ( -
+ {/* Amber tick line above the heading: the same accent used on + live cells across the dashboard, anchoring the sign-in + surface to the rest of the SPA. */} + - )} - {ssoProxyUrl && ( -
- -

- {ssoAdminPossible - ? "SSO can grant read or admin access depending on your group memberships." - : "SSO grants read-only access including logs and SQL text."} -

-
- - or - -
-
- )} -
-

- {currentBasic - ? "That user/password did not work. Check [general].admin_username and [general].admin_password in pg_doorman.toml." - : "Sign in with the admin_username / admin_password from [general] in pg_doorman.toml."} -

-
+
+ + + {ssoProxyUrl ? "sso + basic" : "basic only"} + +
+ +
+ ); +} + +function SsoBlock({ + proxyUrl, + ssoAdminPossible, + redirecting, + onRedirect, +}: { + proxyUrl: string; + ssoAdminPossible: boolean; + redirecting: boolean; + onRedirect: () => void; +}) { + // Trim the proxy host out of the URL so the operator sees where SSO + // will route them before clicking. The try/catch handles a typo'd + // sso_proxy_url at render time — the runtime `safeProxyUrl` check + // only fires when the operator actually clicks the button. + let host: string | null = null; + try { + host = new URL(proxyUrl).host; + } catch { + host = null; + } + return ( +
+

+ Single sign-on +

+ +

+ {host ? ( + <> + Routes via {host}.{" "} + + ) : null} + {ssoAdminPossible + ? "Group membership in the JWT decides whether you land in read-only Sso or full Admin." + : "SSO grants read-only access including logs and SQL text."} +

+
+ ); +} + +function BasicBlock({ + currentBasic, + username, + password, + remember, + onUsername, + onPassword, + onRemember, + onSubmit, + ssoVisible, +}: { + currentBasic: { username: string; password: string } | null; + username: string; + password: string; + remember: boolean; + onUsername: (next: string) => void; + onPassword: (next: string) => void; + onRemember: (next: boolean) => void; + onSubmit: (e: FormEvent) => void; + ssoVisible: boolean; +}) { + return ( +
+ {ssoVisible && ( + + )} + {!ssoVisible && ( +

+ Local admin +

+ )} + {currentBasic && ( +

+ That user/password was rejected. Recheck{" "} + [general].admin_username and{" "} + [general].admin_password in{" "} + pg_doorman.toml. +

+ )} + +
+ setUsername(e.target.value)} - className="mb-3 w-full rounded border border-border-strong bg-surface-2 px-2 py-1.5 text-sm text-text" + onChange={(e) => onUsername(e.target.value)} + className="block h-10 w-full border border-border-strong bg-surface-2 px-3 text-sm text-text focus:border-accent focus:outline-none" /> -
+
+ setPassword(e.target.value)} - className="mb-3 w-full rounded border border-border-strong bg-surface-2 px-2 py-1.5 text-sm text-text" + onChange={(e) => onPassword(e.target.value)} + className="block h-10 w-full border border-border-strong bg-surface-2 px-3 text-sm text-text focus:border-accent focus:outline-none" /> - - - -
+
+ + +
); } + +function TransportChip() { + // Read the live protocol so the operator can tell at a glance + // whether they are about to hand a Bearer JWT to a plain-HTTP + // listener. Falls back to the insecure rendering when `window` is + // not present (SSR / test render) — there is no honest signal in + // that context, and "http" is the safer default for a chip that + // exists to warn about insecure transport. + const protocol = + typeof window !== "undefined" ? window.location.protocol : ""; + const secure = protocol === "https:"; + const className = secure + ? "border-success/40 text-success" + : "border-warning/40 text-warning"; + return ( + + + transport · {secure ? "https" : "http"} + + ); +} diff --git a/frontend/src/lib/sso.ts b/frontend/src/lib/sso.ts index e99b420d3..e81672521 100644 --- a/frontend/src/lib/sso.ts +++ b/frontend/src/lib/sso.ts @@ -59,10 +59,12 @@ export function captureTokenFromUrl(): string | null { /** * Send the user agent to the SSO proxy with the current href as - * redirect target. Validates the proxy URL: must parse, must use https - * (or be localhost for development). A bad URL logs to the console - * and aborts the redirect, so a typo in `pg_doorman.toml` shows in - * devtools instead of leaving the SPA stuck on a half-redirect. + * redirect target. Validates that the URL parses; protocol choice is + * left to the operator (an internal HTTPS-terminating proxy reaching + * pg_doorman over a private HTTP leg is a supported deployment, see + * [web].sso_require_https). A non-parseable URL logs to the console + * and aborts so a typo in `pg_doorman.toml` shows in devtools + * instead of leaving the SPA stuck on a half-redirect. * * Returns `true` when navigation was scheduled, `false` when the URL * was rejected — the caller can use this to clear a "Redirecting…" @@ -77,25 +79,12 @@ export function redirectToSso(proxyUrl: string): boolean { } function safeProxyUrl(proxyUrl: string): URL | null { - let url: URL; try { - url = new URL(proxyUrl); + return new URL(proxyUrl); } catch { console.error("sso_proxy_url is not a valid URL:", proxyUrl); return null; } - const isLocal = url.hostname === "localhost" || url.hostname === "127.0.0.1"; - if (url.protocol !== "https:" && !isLocal) { - console.error( - "sso_proxy_url must use https (got", - url.protocol, - "for", - url.hostname, - ")", - ); - return null; - } - return url; } interface SsoTokenMessage { diff --git a/frontend/src/pages/PoolDetail.tsx b/frontend/src/pages/PoolDetail.tsx index 141cd0845..b3e85243d 100644 --- a/frontend/src/pages/PoolDetail.tsx +++ b/frontend/src/pages/PoolDetail.tsx @@ -25,6 +25,7 @@ import type { PoolScalingDto, PoolScalingRowDto, PoolsDto, + StartupParameter, } from "../types"; interface AdminActionResponse { @@ -312,6 +313,12 @@ export default function PoolDetail() { + {pool.startup_parameters && pool.startup_parameters.length > 0 && ( +
+ +
+ )} + {evalResult && evalResult.reasons.length > 0 && (
    @@ -674,6 +681,69 @@ function ScalingBlock({ row }: { row: PoolScalingRowDto | null }) { ); } +// Operator-supplied PostgreSQL GUCs that pg_doorman injects into each new +// backend's StartupMessage. Source is the cascade layer (`general`, `pool`, +// `auth_query`) that contributed the winning value. If PG rejects one of +// these at backend startup, the client sees the PG error directly — +// pg_doorman does not retry or silently strip the key. +function StartupParametersBlock({ + parameters, +}: { + parameters?: StartupParameter[]; +}) { + const rows = parameters ?? []; + if (rows.length === 0) { + return ( +

    + No operator-supplied startup parameters configured for this pool. +

    + ); + } + return ( +
      + {rows.map((p) => { + // Anonymous viewers receive `value` undefined - the backend + // redacts operator-supplied values for read-only callers. Render + // it as "***" so the row still shows the parameter and the + // cascade source the operator configured. + const displayValue = p.value ?? "***"; + const state = p.state ?? "applied"; + // `applied` is the steady state - the row matches what backends + // ship. Anything else is operator-visible information that needs + // attention: a stale snapshot (RELOAD has not propagated) or a + // cascade that runs over the StartupMessage budget. + const stateClass = + state === "applied" + ? "text-text-dim" + : state === "dropped_due_to_budget" + ? "text-error" + : "text-warning"; + return ( +
    • + + + {p.parameter} = {displayValue} + + + source: {p.source} + {state !== "applied" ? ( + <> + {" · "} + state: {state} + + ) : null} + + +
      • + ); + })} +
    + ); +} + function SqlstateBreakdown({ errors }: { errors?: Record }) { const entries = errors ? Object.entries(errors).sort((a, b) => b[1] - a[1]) : []; if (entries.length === 0) diff --git a/frontend/src/types.ts b/frontend/src/types.ts index fedf634bf..6d1acbd74 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -86,6 +86,28 @@ export interface PoolDto { tls_handshake_errors_total: number; // Live TLS-encrypted backend connections held by the pool. tls_backend_connections: number; + // Operator-supplied PostgreSQL startup parameters this pool injects into + // each new backend StartupMessage. Backend omits this field when the + // cascade is empty for the pool's user. + startup_parameters?: StartupParameter[]; +} + +export interface StartupParameter { + parameter: string; + // Backend omits `value` for anonymous viewers so the public read-only + // UI does not leak operator-supplied tenant identifiers, audit tags, + // or accidental secrets. Admin and SSO callers receive the full + // value. When undefined, render as "***". + value?: string; + // "general" | "pool" | "auth_query" — cascade layer that contributed the + // winning value. + source: string; + // "applied" | "dropped_due_to_budget" | "stale" — wire-application + // state cross-checked against the pool's frozen snapshot. `applied` + // means the next backend spawn ships this key/value; the other two + // states tell the operator that the pool needs a RELOAD or that the + // operator cascade is over the StartupMessage budget. + state: string; } export interface PoolsDto { diff --git a/grafana/generate_dashboard.py b/grafana/generate_dashboard.py index 1f5a41c92..f0bbcaac4 100644 --- a/grafana/generate_dashboard.py +++ b/grafana/generate_dashboard.py @@ -141,6 +141,11 @@ def expanded_row(title: str): # Selector shorthand S = 'instance=~"$instance", user=~"$user", database=~"$database"' SD = 'instance=~"$instance", database=~"$database"' +# Selector for metrics that carry only `pool` (not `user`/`database`), +# e.g. startup_parameters counters. Filtering on `user`/`database` +# yields an empty series even when traffic exists, because those +# labels are not in the metric's label set. +SI = 'instance=~"$instance"' # --------------------------------------------------------------------------- # Row 1: Overview @@ -722,6 +727,39 @@ def expanded_row(title: str): desc="Backend connections completing each phase per second. tcp_connect rate equals total backend creates; gaps to tls/auth/startup mark drop-offs at each step.", ) +# --------------------------------------------------------------------------- +# Row 19: Startup Parameters (collapsed) — PG-rejected and pre-wire-dropped GUCs +# --------------------------------------------------------------------------- +row19 = collapsed_row("Startup Parameters") + +p_sp_errors_by_sqlstate = ts_panel( + "PG-Side Rejections by SQLSTATE", [ + prom( + f'sum by (sqlstate) (rate(pg_doorman_backend_startup_parameter_errors_total{{{SI}}}[$__rate_interval]))', + "{{sqlstate}}", + ), + ], unit="ops", w=12, + desc="Rate of backend startups PostgreSQL rejected because of a configured startup parameter. Split by SQLSTATE: 22023 invalid_value, 42704 undefined_object, 42501 insufficient_privilege, 55P02 cant_change_runtime_param. Growth for the same pool over several minutes means new connects through that pool fail on the same GUC; fix general/pool/auth_query. Filters on $user/$database do not apply: the counter has only the `pool` label.", +) +p_sp_errors_by_pool = ts_panel( + "PG-Side Rejections by Pool", [ + prom( + f'sum by (pool) (rate(pg_doorman_backend_startup_parameter_errors_total{{{SI}}}[$__rate_interval]))', + "{{pool}}", + ), + ], unit="ops", w=12, + desc="Same counter aggregated by pool. The pool name shows which logical pool is affected; per-user attribution lives in the pg_doorman warn log.", +) +p_sp_dropped_by_reason = ts_panel( + "Pre-Wire Drops by Reason", [ + prom( + f'sum by (reason) (rate(pg_doorman_startup_parameters_dropped_total{{{SI}}}[$__rate_interval]))', + "{{reason}}", + ), + ], unit="ops", w=24, + desc="Startup parameter drop events before pg_doorman sends StartupMessage. Reasons: cascade_budget_exceeded (resolved set above 9 488 bytes), packet_cap_exceeded (full packet above PG MAX_STARTUP_PACKET_LENGTH 10 000 bytes), auth_query_oversize (per-user JSON column above the startup-parameter budget), auth_query_overlay_oversize (auth_query overlay overflows but the general/pool baseline still fits), auth_query_bad_type / auth_query_invalid_json / auth_query_invalid_shape (column type, JSON parse, or non-object payload), auth_query_invalid_entry (one or more JSON entries failed validation), dedicated_mode (per-user GUC ignored because the pool shares one backend across users). Any increase should be investigated. Filters on $user/$database do not apply: the counter has only `pool` and `reason` labels.", +) + # --------------------------------------------------------------------------- # Build dashboard # --------------------------------------------------------------------------- @@ -831,6 +869,11 @@ def expanded_row(title: str): .with_panel(p_backend_phase_p99) .with_panel(p_backend_phase_p50) .with_panel(p_backend_phase_rate) + # Row 19: Startup Parameters + .with_row(row19) + .with_panel(p_sp_errors_by_sqlstate) + .with_panel(p_sp_errors_by_pool) + .with_panel(p_sp_dropped_by_reason) ) dashboard_obj = d.build() diff --git a/grafana/pg_doorman.json b/grafana/pg_doorman.json index 5cf4ce850..7f77e562d 100644 --- a/grafana/pg_doorman.json +++ b/grafana/pg_doorman.json @@ -3038,6 +3038,157 @@ "y": 182 }, "repeatDirection": "h" + }, + { + "type": "row", + "collapsed": true, + "id": 0, + "panels": [], + "title": "Startup Parameters", + "gridPos": { + "h": 1, + "w": 24, + "x": 0, + "y": 190 + } + }, + { + "type": "timeseries", + "transparent": false, + "transformations": [], + "options": { + "legend": { + "displayMode": "table", + "placement": "bottom", + "showLegend": true, + "calcs": [ + "min", + "max", + "lastNotNull" + ] + }, + "tooltip": { + "mode": "single", + "sort": "asc" + } + }, + "fieldConfig": { + "defaults": { + "unit": "ops" + }, + "overrides": [] + }, + "targets": [ + { + "expr": "sum by (sqlstate) (rate(pg_doorman_backend_startup_parameter_errors_total{instance=~\"$instance\"}[$__rate_interval]))", + "refId": "", + "legendFormat": "{{sqlstate}}" + } + ], + "title": "PG-Side Rejections by SQLSTATE", + "description": "Per-pool rate of backend startups PG rejected because of an operator-supplied parameter. Split by SQLSTATE: 22023 invalid_value, 42704 undefined_object, 42501 insufficient_privilege, 55P02 cant_change_runtime_param. Non-zero for the same pool over a few minutes means every connect through that pool fails on the same operator GUC \u2014 fix general/pool/auth_query. Filters on $user/$database do not apply \u2014 the counter has only the `pool` label.", + "datasource": { + "uid": "prometheus" + }, + "gridPos": { + "h": 8, + "w": 12, + "x": 0, + "y": 191 + }, + "repeatDirection": "h" + }, + { + "type": "timeseries", + "transparent": false, + "transformations": [], + "options": { + "legend": { + "displayMode": "table", + "placement": "bottom", + "showLegend": true, + "calcs": [ + "min", + "max", + "lastNotNull" + ] + }, + "tooltip": { + "mode": "single", + "sort": "asc" + } + }, + "fieldConfig": { + "defaults": { + "unit": "ops" + }, + "overrides": [] + }, + "targets": [ + { + "expr": "sum by (pool) (rate(pg_doorman_backend_startup_parameter_errors_total{instance=~\"$instance\"}[$__rate_interval]))", + "refId": "", + "legendFormat": "{{pool}}" + } + ], + "title": "PG-Side Rejections by Pool", + "description": "Same counter aggregated by pool. The pool name shows which logical pool is affected; per-user attribution lives in the pg_doorman warn log.", + "datasource": { + "uid": "prometheus" + }, + "gridPos": { + "h": 8, + "w": 12, + "x": 12, + "y": 191 + }, + "repeatDirection": "h" + }, + { + "type": "timeseries", + "transparent": false, + "transformations": [], + "options": { + "legend": { + "displayMode": "table", + "placement": "bottom", + "showLegend": true, + "calcs": [ + "min", + "max", + "lastNotNull" + ] + }, + "tooltip": { + "mode": "single", + "sort": "asc" + } + }, + "fieldConfig": { + "defaults": { + "unit": "ops" + }, + "overrides": [] + }, + "targets": [ + { + "expr": "sum by (reason) (rate(pg_doorman_startup_parameters_dropped_total{instance=~\"$instance\"}[$__rate_interval]))", + "refId": "", + "legendFormat": "{{reason}}" + } + ], + "title": "Pre-Wire Drops by Reason", + "description": "Operator-supplied entries pg_doorman dropped BEFORE the StartupMessage went on the wire \u2014 the failure mode the PG-side counter above cannot see. Reasons: cascade_budget_exceeded (merged map past 9 488 bytes), packet_cap_exceeded (full packet past PG MAX_STARTUP_PACKET_LENGTH 10 000 bytes), auth_query_oversize (per-user JSON column past operator budget), auth_query_overlay_oversize (overlay pushes cascade over budget but baseline alone fits \u2014 pg_doorman ships the baseline), auth_query_bad_type / auth_query_invalid_json / auth_query_invalid_shape (column type, JSON parse, or non-object payload), auth_query_invalid_entry (one or more JSON entries failed validation), dedicated_mode (per-user GUC ignored because the pool shares one backend across users). Non-zero on any reason needs operator attention. Filters on $user/$database do not apply \u2014 the counter has only `pool` and `reason` labels.", + "datasource": { + "uid": "prometheus" + }, + "gridPos": { + "h": 8, + "w": 24, + "x": 0, + "y": 199 + }, + "repeatDirection": "h" } ] } diff --git a/pg_doorman.toml b/pg_doorman.toml index 4a64f3861..574813c5f 100644 --- a/pg_doorman.toml +++ b/pg_doorman.toml @@ -406,6 +406,22 @@ hba = [] # host all all 0.0.0.0/0 reject # """ +# -------------------------------------------------------------------------- +# PostgreSQL Startup GUCs +# -------------------------------------------------------------------------- + +# Baseline PostgreSQL GUCs that pg_doorman adds to each new +# backend StartupMessage. Pools override values per key; +# passthrough auth_query can override them per user. Config load +# validates reserved keys, GUC names, null bytes, and this level's +# size. Before backend startup, pg_doorman checks the resolved +# parameter set again; if it does not fit PG's +# MAX_STARTUP_PACKET_LENGTH (10000 bytes), pg_doorman skips +# configured GUCs for that startup and logs a warning. +# Example: startup_parameters = { plan_cache_mode = "force_custom_plan" } +# Default: {} (empty) +# startup_parameters = { plan_cache_mode = "force_custom_plan", work_mem = "64MB" } + # ############################################################################ # WEB UI / METRICS # ############################################################################ @@ -466,6 +482,10 @@ sso_groups_claim = "groups" # Default: [] # sso_admin_groups = ["pg-doorman-admins"] +# Reject SSO credentials presented over plain HTTP. +# Default: false +sso_require_https = false + # ############################################################################ # TALOS AUTHENTICATION (Optional) # ############################################################################ @@ -553,7 +573,7 @@ cleanup_server_connections = true # max_db_connections = 0 # Don't evict connections younger than this (milliseconds). -# Protects freshly created connections from eviction churn +# Protects freshly created connections from repeated evictions # between user pools sharing the same database. # min_connection_lifetime = 30000 @@ -601,6 +621,17 @@ cleanup_server_connections = true # Default: false log_client_parameter_status_changes = false +# Per-pool overrides for PostgreSQL configuration parameters in +# backend StartupMessage. Wins over general.startup_parameters +# per key; auth_query in passthrough mode wins over this. +# Config load validates reserved keys, GUC names, null bytes, and +# this level's size. Before backend startup, pg_doorman checks the +# resolved parameter set again against PG's +# MAX_STARTUP_PACKET_LENGTH (10000 bytes). +# Example: startup_parameters = { plan_cache_mode = "force_custom_plan" } +# Default: {} (empty) +# startup_parameters = { plan_cache_mode = "force_custom_plan" } + # -------------------------------------------------------------------------- # Users Configuration (TOML uses indexed format) # -------------------------------------------------------------------------- diff --git a/pg_doorman.yaml b/pg_doorman.yaml index f5edc640d..f8f3082e1 100644 --- a/pg_doorman.yaml +++ b/pg_doorman.yaml @@ -446,6 +446,24 @@ general: # # Reject all other connections # host all all 0.0.0.0/0 reject + # -------------------------------------------------------------------------- + # PostgreSQL Startup GUCs + # -------------------------------------------------------------------------- + + # Baseline PostgreSQL GUCs that pg_doorman adds to each new + # backend StartupMessage. Pools override values per key; + # passthrough auth_query can override them per user. Config load + # validates reserved keys, GUC names, null bytes, and this level's + # size. Before backend startup, pg_doorman checks the resolved + # parameter set again; if it does not fit PG's + # MAX_STARTUP_PACKET_LENGTH (10000 bytes), pg_doorman skips + # configured GUCs for that startup and logs a warning. + # Example: startup_parameters = { plan_cache_mode = "force_custom_plan" } + # Default: {} (empty) + # startup_parameters: + # plan_cache_mode: force_custom_plan + # work_mem: 64MB + # ############################################################################ # WEB UI / METRICS # ############################################################################ @@ -506,6 +524,10 @@ web: # Default: [] # sso_admin_groups: ["pg-doorman-admins"] + # Reject SSO credentials presented over plain HTTP. + # Default: false + sso_require_https: false + # ############################################################################ # TALOS AUTHENTICATION (Optional) # ############################################################################ @@ -597,7 +619,7 @@ pools: # max_db_connections: 0 # Don't evict connections younger than this (milliseconds). - # Protects freshly created connections from eviction churn + # Protects freshly created connections from repeated evictions # between user pools sharing the same database. # min_connection_lifetime: 30000 @@ -645,6 +667,18 @@ pools: # Default: false log_client_parameter_status_changes: false + # Per-pool overrides for PostgreSQL configuration parameters in + # backend StartupMessage. Wins over general.startup_parameters + # per key; auth_query in passthrough mode wins over this. + # Config load validates reserved keys, GUC names, null bytes, and + # this level's size. Before backend startup, pg_doorman checks the + # resolved parameter set again against PG's + # MAX_STARTUP_PACKET_LENGTH (10000 bytes). + # Example: startup_parameters = { plan_cache_mode = "force_custom_plan" } + # Default: {} (empty) + # startup_parameters: + # plan_cache_mode: force_custom_plan + # -------------------------------------------------------------------------- # Users Configuration # -------------------------------------------------------------------------- diff --git a/scripts/dashboard-smoke.expected.yaml b/scripts/dashboard-smoke.expected.yaml index 88eca5a2f..1130e9fe0 100644 --- a/scripts/dashboard-smoke.expected.yaml +++ b/scripts/dashboard-smoke.expected.yaml @@ -69,6 +69,13 @@ allow_empty: # are zero on a -M simple workload so the ratio expression returns # an empty vector. - "Prepared Statement Hit Ratio" + # Startup_parameters panels track operator-supplied GUCs that PG + # rejected or that pg_doorman dropped before the wire. The demo + # ships no startup_parameters config, so no backend ever sees an + # operator GUC and no rejection or drop counter ever increments. + - "PG-Side Rejections by SQLSTATE" + - "PG-Side Rejections by Pool" + - "Pre-Wire Drops by Reason" bounds: - name: pool_size_app_user diff --git a/scripts/docker-smoke.sh b/scripts/docker-smoke.sh index 7842d330e..8aee56165 100755 --- a/scripts/docker-smoke.sh +++ b/scripts/docker-smoke.sh @@ -116,11 +116,26 @@ docker run -d --name "$PG_NAME" --network "$NET_NAME" \ # Wait for postgres to accept connections. 90s ceiling: typical fresh # postgres:17 boot is <5s; the headroom covers cold-cache image pulls # and slow runners (Ubicloud cold VM, congested GHA pool). +# +# The official postgres image cycles through an init listener and then +# restarts before the final listener comes up. `pg_isready` answers YES +# on the init listener too, so a single success is not enough: the very +# next query can hit "FATAL: the database system is shutting down" and +# the smoke fails for a reason that has nothing to do with pg_doorman. +# Require two consecutive successful `psql SELECT 1` runs to ride past +# the restart window. echo "waiting for postgres readiness..." +prev_ok=0 for i in $(seq 1 90); do - if docker exec "$PG_NAME" pg_isready -U "$PG_USER" >/dev/null 2>&1; then - echo "postgres ready after ${i}s" - break + if docker exec -e "PGPASSWORD=$PG_PASSWORD" "$PG_NAME" \ + psql -U "$PG_USER" -d "$PG_DB" -At -c "SELECT 1" >/dev/null 2>&1; then + if [ "$prev_ok" -eq 1 ]; then + echo "postgres ready after ~${i}s" + break + fi + prev_ok=1 + else + prev_ok=0 fi if [ "$i" -eq 90 ]; then echo "::error::postgres did not become ready in 90s" diff --git a/src/admin/mod.rs b/src/admin/mod.rs index 1648104eb..7b74ef0ab 100644 --- a/src/admin/mod.rs +++ b/src/admin/mod.rs @@ -41,6 +41,7 @@ pub(crate) const SHOW_SUBCOMMANDS: &[&str] = &[ "version", "users", "auth_query", + "startup_parameters", "log_level", "lists", #[cfg(target_os = "linux")] @@ -56,7 +57,8 @@ use show::{ reset_interner, show_auth_query, show_clients, show_config, show_connections, show_databases, show_help, show_interner, show_interner_top, show_lists, show_log_level, show_pool_coordinator, show_pool_scaling, show_pools, show_pools_extended, show_pools_memory, - show_prepared_statements, show_servers, show_stats, show_users, show_version, + show_prepared_statements, show_servers, show_startup_parameters, show_stats, show_users, + show_version, }; /// Handle admin client. @@ -136,6 +138,7 @@ where "VERSION" => show_version(stream).await, "USERS" => show_users(stream).await, "AUTH_QUERY" => show_auth_query(stream).await, + "STARTUP_PARAMETERS" => show_startup_parameters(stream).await, "POOL_COORDINATOR" => show_pool_coordinator(stream).await, "POOL_SCALING" => show_pool_scaling(stream).await, "LOG_LEVEL" => show_log_level(stream).await, @@ -270,3 +273,22 @@ where } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn show_subcommands_contains_startup_parameters() { + // Tab completion on `SHOW ` returns SHOW_SUBCOMMANDS, and the + // dispatch above routes `SHOW STARTUP_PARAMETERS` to + // `show_startup_parameters`. The entry has to exist in this constant + // for psql's autocomplete to surface the command, since the + // canonical list is the single source of truth shared between + // dispatch, `SHOW HELP`, and `handle_tab_completion`. + assert!( + SHOW_SUBCOMMANDS.contains(&"startup_parameters"), + "SHOW_SUBCOMMANDS missing startup_parameters: {SHOW_SUBCOMMANDS:?}" + ); + } +} diff --git a/src/admin/show.rs b/src/admin/show.rs index 4833fe616..f19a91478 100644 --- a/src/admin/show.rs +++ b/src/admin/show.rs @@ -819,6 +819,58 @@ where write_all_half(stream, &res).await } +/// Show the operator-supplied PostgreSQL startup parameters that pg_doorman +/// will inject into the `StartupMessage` of each new backend connection, +/// resolved per pool through the `general` -> pool -> auth_query cascade. +/// +/// Each row lists one parameter for one pool, with the layer that contributed +/// the winning value (`general`, `pool`, `auth_query`). +pub async fn show_startup_parameters(stream: &mut T) -> Result<(), Error> +where + T: tokio::io::AsyncWrite + std::marker::Unpin, +{ + let columns = vec![ + ("user", DataType::Text), + ("database", DataType::Text), + ("parameter", DataType::Text), + ("value", DataType::Text), + ("source", DataType::Text), + // applied | dropped_due_to_budget | stale — `applied` means the + // value lands on the wire, `dropped_due_to_budget` means the + // runtime cascade overflows, and `stale` means the pool's + // frozen snapshot is behind the live config (RELOAD or auth_query + // refetch will catch up). + ("state", DataType::Text), + ]; + + let mut res = BytesMut::new(); + res.put(row_description(&columns)); + + let pools = get_all_pools(); + let mut entries: Vec<_> = pools.iter().collect(); + entries.sort_by(|a, b| (&a.0.db, &a.0.user).cmp(&(&b.0.db, &b.0.user))); + + for (identifier, pool) in entries { + let effective = pool.database.effective_startup_parameters_with_sources(); + for (parameter, (value, source, state)) in effective { + res.put(data_row(&[ + identifier.user.clone(), + identifier.db.clone(), + parameter, + value, + source.as_str().to_string(), + state.as_str().to_string(), + ])); + } + } + + res.put(command_complete("SHOW")); + res.put_u8(b'Z'); + res.put_i32(5); + res.put_u8(b'I'); + write_all_half(stream, &res).await +} + /// Show pool coordinator status per database. /// Displays connection limits, current usage, and cumulative counters. pub async fn show_pool_coordinator(stream: &mut T) -> Result<(), Error> diff --git a/src/app/errors.rs b/src/app/errors.rs index 798a832e9..60c02993d 100644 --- a/src/app/errors.rs +++ b/src/app/errors.rs @@ -21,6 +21,19 @@ pub enum Error { /// FATAL with SQLSTATE 57P01/57P02/57P03: backend accepted the connection /// but is shutting down or starting up. ServerUnavailableError(String, ServerIdentifier), + /// PostgreSQL rejected the StartupMessage because of an operator-supplied + /// `startup_parameters` entry — typo, bad value, insufficient privilege, + /// or postmaster-only knob. Carries the verbatim PG `SQLSTATE` and + /// `M`-field so the client receives the PG-native error rather than the + /// generic `53300` connection-pool fallback. SQLSTATE class `57P` does + /// NOT use this variant — those go through `ServerUnavailableError` to + /// drive the Patroni-assisted fallback path before this branch is + /// reached. + ServerStartupParameterRejection { + sqlstate: String, + message: String, + server_identifier: ServerIdentifier, + }, ServerStartupReadParameters(String), BadConfig(String), AllServersDown, @@ -144,6 +157,14 @@ impl std::fmt::Display for Error { Error::ServerUnavailableError(error, server_identifier) => { write!(f, "Backend unavailable: {error} for {server_identifier}") } + Error::ServerStartupParameterRejection { + sqlstate, + message, + server_identifier, + } => write!( + f, + "PostgreSQL rejected operator-supplied startup parameter (sqlstate {sqlstate}): {message} for {server_identifier}" + ), Error::ServerStartupReadParameters(msg) => { write!(f, "Failed to read server parameters: {msg}") } diff --git a/src/app/generate/annotated.rs b/src/app/generate/annotated.rs index 18490e5d8..04a3447a1 100644 --- a/src/app/generate/annotated.rs +++ b/src/app/generate/annotated.rs @@ -175,6 +175,7 @@ pub fn generate_reference_config(format: ConfigFormat, russian: bool) -> String server_tls_certificate: None, server_tls_private_key: None, auth_query: None, + startup_parameters: std::collections::BTreeMap::new(), users: vec![User { username: "app_user".to_string(), password: "md5dd9a0f26a4302744db881776a09bbfad".to_string(), @@ -1027,6 +1028,26 @@ fn write_general_section(w: &mut ConfigWriter, config: &Config) { w.comment(fi, ""); write_pg_hba_rule_examples(w, fi); w.blank(); + + // --- PostgreSQL Startup Parameters (operator-defined GUCs) --- + w.separator(fi, f.section_title("startup_parameters").get(w.russian)); + w.blank(); + + write_field_comment(w, fi, "general", "startup_parameters"); + match w.format { + ConfigFormat::Toml => { + w.comment( + fi, + "startup_parameters = { plan_cache_mode = \"force_custom_plan\", work_mem = \"64MB\" }", + ); + } + ConfigFormat::Yaml => { + w.comment(fi, "startup_parameters:"); + w.comment(fi, " plan_cache_mode: force_custom_plan"); + w.comment(fi, " work_mem: 64MB"); + } + } + w.blank(); } fn write_pg_hba_examples(w: &mut ConfigWriter, fi: usize) { @@ -1218,6 +1239,10 @@ fn write_web_section(w: &mut ConfigWriter, web: &Web) { w.kv(fi, "sso_admin_groups", &format!("[{rendered}]")); } w.blank(); + + write_field_comment(w, fi, "web", "sso_require_https"); + w.kv(fi, "sso_require_https", &web.sso_require_https.to_string()); + w.blank(); } fn write_talos_section(w: &mut ConfigWriter) { @@ -1487,6 +1512,22 @@ fn write_single_pool(w: &mut ConfigWriter, pool_name: &str, pool: &Pool) { ); w.blank(); + // --- Per-pool Startup Parameters --- + write_field_comment(w, fi, "pool", "startup_parameters"); + match w.format { + ConfigFormat::Toml => { + w.comment( + fi, + "startup_parameters = { plan_cache_mode = \"force_custom_plan\" }", + ); + } + ConfigFormat::Yaml => { + w.comment(fi, "startup_parameters:"); + w.comment(fi, " plan_cache_mode: force_custom_plan"); + } + } + w.blank(); + write_pool_users(w, pool_name, &pool.users); write_auth_query_commented_example(w); } diff --git a/src/app/generate/docs.rs b/src/app/generate/docs.rs index d3a3d4bf8..621884d6b 100644 --- a/src/app/generate/docs.rs +++ b/src/app/generate/docs.rs @@ -261,6 +261,7 @@ fn write_general_fields(out: &mut String, f: &FieldsData) { "hba", "pg_hba", "pooler_check_query", + "startup_parameters", ]; for name in &fields { @@ -291,6 +292,7 @@ fn write_pool_fields(out: &mut String, f: &FieldsData) { "reserve_pool_size", "reserve_pool_timeout", "min_guaranteed_pool_size", + "startup_parameters", ]; for name in &fields { @@ -305,7 +307,7 @@ fn write_auth_query_section(out: &mut String) { let _ = writeln!(out, "The `auth_query` section enables dynamic user authentication by querying a PostgreSQL database for credentials at connection time. This allows pg_doorman to authenticate users without listing them statically in the configuration file.\n"); let _ = writeln!(out, "```yaml\npools:\n mydb:\n auth_query:\n query: \"SELECT passwd FROM pg_shadow WHERE usename = $1\"\n user: \"doorman_auth\"\n password: \"auth_password\"\n```\n"); let _ = writeln!(out, "There are two modes of operation:\n"); - let _ = writeln!(out, "- **Dedicated mode** (`server_user` is set): All dynamically authenticated users share a single connection pool that connects to PostgreSQL as `server_user`. This is the simplest setup and works well when all users need the same backend access."); + let _ = writeln!(out, "- **Dedicated mode** (`server_user` is set): all dynamically authenticated users share one backend pool that connects to PostgreSQL as `server_user`. Use it when backend identity does not need to match the client user."); let _ = writeln!(out, "- **Passthrough mode** (`server_user` is not set): Each dynamically authenticated user gets their own connection pool that connects to PostgreSQL using their own credentials (MD5 pass-the-hash or SCRAM ClientKey passthrough). This preserves per-user identity on the backend.\n"); let _ = writeln!(out, "Static users (defined in the `users` section) are always checked first. The auth_query is only used when the username is not found among static users.\n"); let _ = writeln!(out, "```admonish warning title=\"Security Recommendation\""); @@ -468,6 +470,17 @@ fn write_prometheus_metrics_section(out: &mut String) { let _ = writeln!(out, "| `pg_doorman_auth_query_executor` | DEPRECATED, removed in 3.10. Gauge mirror of `pg_doorman_auth_query_executor_total`. |"); let _ = writeln!(out, "| `pg_doorman_auth_query_dynamic_pools` | Auth query dynamic pool lifecycle metrics by type and database. Types include: `current` (currently active dynamic pools), `created` (total pools created since startup), `destroyed` (total pools garbage-collected or removed on RELOAD). Only relevant in passthrough mode. |\n"); + // Configured startup_parameters + let _ = writeln!(out, "### Configured startup_parameters\n"); + let _ = writeln!( + out, + "These metrics cover two failure points for configured startup parameters. `pg_doorman_backend_startup_parameter_errors_total` counts backend startups PostgreSQL rejected after pg_doorman sent the `StartupMessage`. `pg_doorman_startup_parameters_dropped_total` counts drop events before `StartupMessage`, either because the resolved parameter set was too large or because an auth_query JSON value was invalid.\n" + ); + let _ = writeln!(out, "| Metric | Description |"); + let _ = writeln!(out, "|--------|-------------|"); + let _ = writeln!(out, "| `pg_doorman_backend_startup_parameter_errors_total` | Counter by `(pool, sqlstate)`. Increments when PostgreSQL rejects a backend startup and the `ErrorResponse` names a startup parameter sent by pg_doorman. SQLSTATEs with the `57P` prefix are excluded because Patroni-assisted fallback handles those errors. The failing parameter name and username are written to the warning log line, not to labels. pg_doorman first parses the common `parameter \"\"` phrase, then scans the message for any sent key in double quotes. If neither lookup finds a key, the counter is not incremented. |"); + let _ = writeln!(out, "| `pg_doorman_startup_parameters_dropped_total` | Counter by `(pool, reason)`. Increments when pg_doorman drops startup parameters before sending `StartupMessage`. Reasons: `cascade_budget_exceeded`, `packet_cap_exceeded`, `auth_query_oversize`, `auth_query_overlay_oversize`, `auth_query_bad_type`, `auth_query_invalid_json`, `auth_query_invalid_shape`, `auth_query_invalid_entry`, `dedicated_mode`. |\n"); + // Server Metrics let _ = writeln!(out, "### Server Metrics\n"); let _ = writeln!(out, "| Metric | Description |"); diff --git a/src/app/generate/fields.yaml b/src/app/generate/fields.yaml index fd4d8dd4c..01ad67927 100644 --- a/src/app/generate/fields.yaml +++ b/src/app/generate/fields.yaml @@ -53,6 +53,9 @@ sections: hba: en: "Access Control (pg_hba - Recommended)" ru: "Контроль доступа (pg_hba — рекомендуется)" + startup_parameters: + en: "PostgreSQL Startup GUCs" + ru: "Параметры запуска PostgreSQL (GUC, задаваемые оператором)" pool_server: en: "Server Connection Settings" ru: "Настройки подключения к серверу" @@ -467,8 +470,8 @@ fields: is actively sending data but the remote end has become unreachable (e.g., network failure, client crash). When set to a non-zero value, if data remains unacknowledged for this duration, the connection will - be terminated. This is particularly useful to avoid 15-16 minute delays caused by TCP retransmission - timeout when keepalive cannot help (e.g., during active data transmission). + be terminated. Use it to avoid 15-16 minute delays caused by TCP retransmission timeout when + keepalive cannot help (e.g., during active data transmission). **Note:** This option is only supported on Linux. On other operating systems, this setting is ignored. @@ -937,7 +940,7 @@ fields: ru: | Путь к файлу TLS-сертификата для входящих клиентских подключений. Должен использоваться вместе с tls_private_key. - doc: "The path to the certificate file for TLS connections. This is required to enable TLS for incoming client connections. Must be used together with `tls_private_key`." + doc: "Path to the certificate file for TLS connections. Required to enable TLS for incoming client connections. Must be used together with `tls_private_key`." default: "None" tls_private_key: @@ -948,7 +951,7 @@ fields: ru: | Путь к файлу приватного ключа TLS для входящих клиентских подключений. Должен использоваться вместе с tls_certificate. - doc: "The path to the private key file for TLS connections. This is required to enable TLS for incoming client connections. Must be used together with `tls_certificate`." + doc: "Path to the private key file for TLS connections. Required to enable TLS for incoming client connections. Must be used together with `tls_certificate`." default: "None" tls_ca_cert: @@ -959,7 +962,7 @@ fields: ru: | Путь к CA-сертификату для верификации клиентских сертификатов. Используется с tls_mode = "verify-full" - doc: "The file containing the CA certificate to verify the client certificate. This is required when `tls_mode` is set to `verify-full`." + doc: "CA certificate file used to verify client certificates. Required when `tls_mode` is set to `verify-full`." default: "None" tls_mode: @@ -1146,6 +1149,42 @@ fields: - For authentication methods other than `trust`, PgDoorman performs the corresponding challenge/response with the client. - For Talos/JWT/PAM flows configured at the pool/user level, `trust` still bypasses the client password prompt; however, those modes may be used when `trust` does not match. + startup_parameters: + config: + en: | + Baseline PostgreSQL GUCs that pg_doorman adds to each new + backend StartupMessage. Pools override values per key; + passthrough auth_query can override them per user. Config load + validates reserved keys, GUC names, null bytes, and this level's + size. Before backend startup, pg_doorman checks the resolved + parameter set again; if it does not fit PG's + MAX_STARTUP_PACKET_LENGTH (10000 bytes), pg_doorman skips + configured GUCs for that startup and logs a warning. + Example: startup_parameters = { plan_cache_mode = "force_custom_plan" } + ru: | + Базовые GUC PostgreSQL, которые pg_doorman добавляет в + StartupMessage каждого нового бэкенда. Пулы могут + переопределять значения по ключу; auth_query в режиме + passthrough может переопределять их для конкретного + пользователя. При загрузке конфигурации проверяются + зарезервированные ключи, имена GUC, нулевые байты и размер + этого уровня. Объединённый набор параметров снова проверяется + при запуске бэкенда; если он не помещается в лимит PG + MAX_STARTUP_PACKET_LENGTH (10000 байт), pg_doorman пропускает + операторские GUC для этого запуска и пишет предупреждение. + Пример: startup_parameters = { plan_cache_mode = "force_custom_plan" } + doc: | + Map of PostgreSQL configuration parameter names to string values. pg_doorman writes them into each new backend `StartupMessage`; PostgreSQL stores them as the session reset defaults, so client `RESET ALL` / `DISCARD ALL` returns to these values. + + Cascade order: `general.startup_parameters`, then `pools..startup_parameters`, then the optional `startup_parameters` JSON column returned by passthrough `auth_query`. Later layers win per key. Dedicated-mode `auth_query` pools ignore the per-user column because one shared backend serves multiple roles. + + Validation at config load rejects reserved protocol keys (`user`, `database`, `replication`, `options`, anything starting with `_pq_.`), invalid GUC names, null bytes, and per-level maps that exceed the startup-parameter budget. Before each backend startup, pg_doorman checks the resolved parameter set against PG's `MAX_STARTUP_PACKET_LENGTH` (10 000 bytes); if it does not fit, pg_doorman drops the auth_query overlay when the baseline still fits, otherwise it drops all configured keys for that startup and logs the event. + + If PostgreSQL rejects a parameter at backend startup, pg_doorman returns PostgreSQL's `ErrorResponse` to the client unchanged. There is no retry with the key removed, and pg_doorman does not automatically disable that key for the pool. The cumulative count is exported as `pg_doorman_backend_startup_parameter_errors_total{pool, sqlstate}`; the parameter name and username are written to the corresponding warning log line. + + Inspect the resolved per-pool values with `SHOW STARTUP_PARAMETERS` or the `/api/pools` REST endpoint. + default: "{} (empty)" + pool: server_host: config: @@ -1285,11 +1324,11 @@ fields: config: en: | Don't evict connections younger than this (milliseconds). - Protects freshly created connections from eviction churn + Protects freshly created connections from repeated evictions between user pools sharing the same database. ru: | Не изымать соединения моложе этого значения (миллисекунды). - Защищает свежесозданные соединения от постоянного churn'а + Защищает свежесозданные соединения от постоянных выселений между user-пулами, разделяющими одну базу данных. doc: | Minimum age (in milliseconds) a connection must reach before it can be evicted by the @@ -1349,7 +1388,7 @@ fields: from coordinator eviction. When the coordinator needs to free a connection slot for another user, it will not evict connections from a user who is at or below this count. - This is separate from `min_pool_size` (user-level): `min_pool_size` controls prewarm + Separate from `min_pool_size` (user-level): `min_pool_size` controls prewarm and replenish (proactively creating connections), while `min_guaranteed_pool_size` only affects eviction decisions (never creates connections). @@ -1445,6 +1484,33 @@ fields: doc: "Per-pool override of `server_tls_private_key`." default: "None (uses global setting)" + startup_parameters: + config: + en: | + Per-pool overrides for PostgreSQL configuration parameters in + backend StartupMessage. Wins over general.startup_parameters + per key; auth_query in passthrough mode wins over this. + Config load validates reserved keys, GUC names, null bytes, and + this level's size. Before backend startup, pg_doorman checks the + resolved parameter set again against PG's + MAX_STARTUP_PACKET_LENGTH (10000 bytes). + Example: startup_parameters = { plan_cache_mode = "force_custom_plan" } + ru: | + Переопределения параметров PostgreSQL для этого пула. pg_doorman + добавляет их в StartupMessage каждого нового бэкенда. Значения + имеют приоритет над general.startup_parameters по ключу; + auth_query в режиме passthrough имеет приоритет над настройками + пула. При загрузке конфигурации проверяются зарезервированные + ключи, имена GUC, нулевые байты и размер этого уровня. + Объединённый набор параметров снова проверяется при запуске + бэкенда по лимиту PG MAX_STARTUP_PACKET_LENGTH (10000 байт). + Пример: startup_parameters = { plan_cache_mode = "force_custom_plan" } + doc: | + Per-pool map of PostgreSQL configuration parameters. Validation rules match those documented for [`general.startup_parameters`](#startup-parameters): reserved keys, GUC naming, null bytes, and the startup-parameter budget within PG's `MAX_STARTUP_PACKET_LENGTH` (10 000-byte) `StartupMessage` cap. + + In the cascade `general` → `pool` → `auth_query`, this layer overrides `general` per key, and a passthrough auth_query entry overrides this layer. Dedicated-mode `auth_query` pools ignore the per-user column because one shared backend serves multiple users. See [`general.startup_parameters`](#startup-parameters) for validation rules, failure behavior, and observability. + default: "{} (empty)" + user: username: config: @@ -1573,7 +1639,9 @@ fields: Если запрос возвращает ровно один столбец, он используется независимо от имени. Используйте $1 как параметр для имени пользователя. doc: | - SQL query to fetch credentials. Must return a column named `passwd` or `password` containing the MD5 or SCRAM hash. If the query returns exactly one column, it is used regardless of name. Any extra columns are ignored. Use `$1` as the placeholder for the username parameter. + SQL query to fetch credentials. It must return a column named `passwd` or `password` containing the MD5 or SCRAM hash. If the query returns exactly one column, it is used regardless of name. + + Extra columns are ignored except for the optional `startup_parameters` text column. In passthrough mode, pg_doorman reads that column as a JSON object with per-user PostgreSQL startup parameters. Dedicated mode ignores it and logs a warning. Use `$1` as the placeholder for the username parameter. Example: `"SELECT passwd FROM pg_shadow WHERE usename = $1"` @@ -1771,5 +1839,12 @@ fields: config: en: "Group names that map onto the Admin role." ru: "Список групп, которые получают роль Admin (могут управлять пулами)." - doc: "An SSO user whose JWT carries any of these names in sso_groups_claim gets full admin access (POST /api/admin/* allowed). Empty (default) keeps the SSO surface read-only." + doc: "An SSO user whose JWT carries any of these names in sso_groups_claim gets full admin access (POST /api/admin/* allowed). Empty (default) keeps SSO users read-only." default: "[]" + + sso_require_https: + config: + en: "Reject SSO credentials presented over plain HTTP." + ru: "Отклонять SSO credentials, пришедшие по plain HTTP." + doc: "When true, Bearer/cookie/query SSO tokens are accepted only if the request peer is in trusted_proxies and the proxy forwarded X-Forwarded-Proto: https. Defaults to false so SSO works through a TLS-terminating proxy reaching pg_doorman over a private HTTP leg. Enable on shared networks where the proxy → pg_doorman hop is exposed." + default: "false" diff --git a/src/app/generate/mod.rs b/src/app/generate/mod.rs index 2eb45acfc..0203400e1 100644 --- a/src/app/generate/mod.rs +++ b/src/app/generate/mod.rs @@ -168,6 +168,7 @@ pub fn generate_config_with_client( server_tls_certificate: None, server_tls_private_key: None, auth_query: None, + startup_parameters: std::collections::BTreeMap::new(), users: users.clone(), }, ); diff --git a/src/app/generate/tests.rs b/src/app/generate/tests.rs index c78099637..1f4d0713a 100644 --- a/src/app/generate/tests.rs +++ b/src/app/generate/tests.rs @@ -91,6 +91,7 @@ pub fn generate_config_with_client< patroni_api_timeout: None, fallback_connect_timeout: None, fallback_lifetime: None, + startup_parameters: std::collections::BTreeMap::new(), users: users_vec.clone(), }, ); diff --git a/src/auth/auth_query.rs b/src/auth/auth_query.rs index 7a3778b0d..9a0eebd87 100644 --- a/src/auth/auth_query.rs +++ b/src/auth/auth_query.rs @@ -33,13 +33,34 @@ const MAX_USERNAME_LEN: usize = 63; // PasswordFetcher trait (allows mocking AuthQueryExecutor in unit tests) // --------------------------------------------------------------------------- -/// Trait for fetching password hashes from PostgreSQL. +/// Password hash plus the per-user startup_parameters map returned by +/// auth_query. The map is empty when the optional column is absent, NULL, +/// empty, or fully rejected by validation. +pub type Credentials = (String, std::collections::HashMap); + +/// Trait for fetching credentials from PostgreSQL. /// `AuthQueryExecutor` implements this; tests and benchmarks can substitute a mock. +/// +/// `fetch` returns the password hash. `fetch_credentials` also returns the +/// optional per-user startup parameter map. Fetchers that do not support that +/// column use the default empty map. pub trait PasswordFetcher: Send + Sync { fn fetch<'a>( &'a self, username: &'a str, ) -> impl Future, Error>> + Send + 'a; + + fn fetch_credentials<'a>( + &'a self, + username: &'a str, + ) -> impl Future, Error>> + Send + 'a { + async move { + Ok(self + .fetch(username) + .await? + .map(|p| (p, std::collections::HashMap::new()))) + } + } } impl PasswordFetcher for AuthQueryExecutor { @@ -49,6 +70,13 @@ impl PasswordFetcher for AuthQueryExecutor { ) -> impl Future, Error>> + Send + 'a { self.fetch_password(username) } + + fn fetch_credentials<'a>( + &'a self, + username: &'a str, + ) -> impl Future, Error>> + Send + 'a { + AuthQueryExecutor::fetch_credentials(self, username) + } } // --------------------------------------------------------------------------- @@ -183,11 +211,12 @@ impl AuthQueryExecutor { Ok(client) } - /// Fetch password hash for a username from PostgreSQL. - /// Returns `Some(password_hash)` or `None` if user not found. - pub async fn fetch_password(&self, username: &str) -> Result, Error> { + /// Fetch credentials (password hash plus the optional per-user + /// startup_parameters map) for a username from PostgreSQL. + /// Returns `Some((password_hash, params))` or `None` if user not found. + pub async fn fetch_credentials(&self, username: &str) -> Result, Error> { debug!( - "[{username}@{}] auth_query: fetching password", + "[{username}@{}] auth_query: fetching credentials", self.pool_name ); @@ -195,7 +224,7 @@ impl AuthQueryExecutor { let mut rx = self.rx.lock().await; rx.recv().await.ok_or_else(|| { error!( - "[{username}@{}] auth_query: executor pool closed, cannot fetch password", + "[{username}@{}] auth_query: executor pool closed, cannot fetch credentials", self.pool_name ); Error::AuthQueryPoolClosed @@ -242,6 +271,12 @@ impl AuthQueryExecutor { result } + /// Backwards-compatible password-only accessor. Discards any per-user + /// startup_parameters returned alongside the password. + pub async fn fetch_password(&self, username: &str) -> Result, Error> { + Ok(self.fetch_credentials(username).await?.map(|(p, _)| p)) + } + async fn try_reconnect(&self) { let database = self .config @@ -282,7 +317,7 @@ impl AuthQueryExecutor { &self, client: &Client, username: &str, - ) -> Result, Error> { + ) -> Result, Error> { let rows = client .query( &self.config.query, @@ -297,7 +332,15 @@ impl AuthQueryExecutor { match rows.len() { 0 => Ok(None), - 1 => Self::extract_password(&rows[0], username, &self.pool_name), + 1 => { + let row = &rows[0]; + let pw_opt = Self::extract_password(row, username, &self.pool_name)?; + let Some(pw) = pw_opt else { + return Ok(None); + }; + let params = Self::extract_startup_parameters(row, username, &self.pool_name); + Ok(Some((pw, params))) + } n => Err(Error::AuthQueryConfigError(format!( "query returned {n} rows for user '{username}', expected 0 or 1" ))), @@ -342,6 +385,140 @@ impl AuthQueryExecutor { } } } + + /// Read the optional `startup_parameters` text column from the auth_query + /// row and parse it as a JSON object. A missing column yields an empty + /// map; a present column whose type does not coerce to `Option` + /// logs a warning and yields an empty map. Actual JSON parsing and + /// per-entry validation happen in `parse_startup_parameters_text`. + fn extract_startup_parameters( + row: &tokio_postgres::Row, + username: &str, + pool_name: &str, + ) -> std::collections::HashMap { + let column = row + .columns() + .iter() + .find(|c| c.name() == "startup_parameters"); + let Some(column) = column else { + return std::collections::HashMap::new(); + }; + let raw: Option = match row.try_get::<_, Option>("startup_parameters") { + Ok(v) => v, + Err(e) => { + warn!( + "[{username}@{pool_name}] auth_query startup_parameters column has type \ + `{ty}` but pg_doorman reads it as `text`: {e}. If the SELECT returns \ + json or jsonb, add `::text` (for example: \ + `jsonb_build_object(...)::text AS startup_parameters`); per-user \ + parameters are ignored for this row.", + ty = column.type_().name() + ); + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[pool_name, "auth_query_bad_type"]) + .inc(); + return std::collections::HashMap::new(); + } + }; + Self::parse_startup_parameters_text(raw.as_deref(), username, pool_name) + } + + /// Parse the optional `startup_parameters` JSON object returned by + /// auth_query. Valid string entries become per-user GUCs. Invalid keys, + /// non-string values, malformed JSON, and non-object JSON are logged and + /// ignored; authentication still continues. + fn parse_startup_parameters_text( + text: Option<&str>, + username: &str, + pool_name: &str, + ) -> std::collections::HashMap { + let Some(text) = text else { + return std::collections::HashMap::new(); + }; + if text.is_empty() { + return std::collections::HashMap::new(); + } + // Reject oversize input before serde_json allocates the Value tree. + // A single auth_query row above the operator budget cannot produce a + // sendable startup map. + let max_bytes = crate::config::startup_parameters::MAX_OPERATOR_BUDGET; + if text.len() > max_bytes { + warn!( + "[{username}@{pool_name}] auth_query startup_parameters: raw column is {} bytes, \ + exceeding operator budget {max_bytes}; parameters ignored", + text.len() + ); + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[pool_name, "auth_query_oversize"]) + .inc(); + return std::collections::HashMap::new(); + } + let parsed: serde_json::Value = match serde_json::from_str(text) { + Ok(v) => v, + Err(e) => { + warn!( + "[{username}@{pool_name}] auth_query startup_parameters: JSON parse failed: \ + {e}; parameters ignored" + ); + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[pool_name, "auth_query_invalid_json"]) + .inc(); + return std::collections::HashMap::new(); + } + }; + let serde_json::Value::Object(obj) = parsed else { + warn!( + "[{username}@{pool_name}] auth_query startup_parameters: top-level value is not a \ + JSON object; ignored" + ); + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[pool_name, "auth_query_invalid_shape"]) + .inc(); + return std::collections::HashMap::new(); + }; + let mut out = std::collections::HashMap::with_capacity(obj.len()); + let scope = format!("auth_query.startup_parameters[user={username}]"); + let mut had_invalid_entry = false; + for (k, v) in obj { + match v { + serde_json::Value::String(s) => { + if let Err(e) = + crate::config::startup_parameters::validate_entry(&k, &s, &scope) + { + warn!("[{pool_name}] {e}"); + had_invalid_entry = true; + continue; + } + out.insert(k, s); + } + other => { + let kind = match other { + serde_json::Value::Null => "null", + serde_json::Value::Bool(_) => "boolean", + serde_json::Value::Number(_) => "number", + serde_json::Value::Array(_) => "array", + serde_json::Value::Object(_) => "object", + serde_json::Value::String(_) => unreachable!(), + }; + warn!( + "[{username}@{pool_name}] auth_query startup_parameters: value for '{k}' \ + is {kind}, not string; ignored" + ); + had_invalid_entry = true; + } + } + } + // One increment per parsed row that contained at least one + // invalid entry, matching every other reason on this counter + // so `rate by(reason)` is dimensionally consistent. Per-entry + // detail stays in the warn log. + if had_invalid_entry { + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[pool_name, "auth_query_invalid_entry"]) + .inc(); + } + out + } } // --------------------------------------------------------------------------- @@ -364,6 +541,15 @@ pub struct CacheEntry { /// for SCRAM passthrough to backend PG (Step 6). /// None for MD5 users or before first SCRAM auth. pub client_key: Option>, + /// Per-user startup parameters returned by the optional auth_query + /// `startup_parameters` JSON column. Empty when the column is absent, + /// empty/NULL, or filtered out in dedicated auth_query mode. + /// + /// Wrapped in `Arc` so cache hits do not clone the underlying map. + /// For a user with a wide row (a dozen extension GUCs) every + /// `cache.get_or_fetch` previously paid an `O(map)` clone; the + /// `Arc::clone` here is two atomic increments instead. + pub startup_parameters: Arc>, } impl CacheEntry { @@ -374,6 +560,7 @@ impl CacheEntry { is_negative: false, last_refetch_at: None, client_key: None, + startup_parameters: Arc::new(std::collections::HashMap::new()), } } @@ -384,6 +571,7 @@ impl CacheEntry { is_negative: true, last_refetch_at: None, client_key: None, + startup_parameters: Arc::new(std::collections::HashMap::new()), } } @@ -420,7 +608,7 @@ pub struct AuthQueryCache { /// Per-username locks for request coalescing. /// First request acquires lock + fetches; others wait + get cache hit. locks: DashMap>>, - /// Fetcher for cache miss → PG fetch. + /// Fetcher for cache miss to PG. executor: Arc, /// TTL for positive cache entries (user found). cache_ttl: Duration, @@ -430,6 +618,14 @@ pub struct AuthQueryCache { min_interval: Duration, /// Optional stats for observability (None in unit tests). stats: Option>, + /// True when auth_query runs in dedicated mode (server_user is set). + /// In that mode every backend connection shares a single backend + /// identity, so per-user startup_parameters cannot be honored. + is_dedicated: bool, + /// Usernames already warned about dropped per-user startup_parameters + /// in dedicated mode. Ensures the warning fires at most once per + /// (pool, user) until the cache is cleared by a config reload. + dedicated_warnings: DashMap, } impl AuthQueryCache { @@ -448,6 +644,76 @@ impl AuthQueryCache { cache_failure_ttl: config.cache_failure_ttl, min_interval: config.min_interval, stats, + is_dedicated: config.is_dedicated_mode(), + dedicated_warnings: DashMap::new(), + } + } + + /// In dedicated auth_query mode (`server_user` set) every backend + /// connection shares a single identity, so per-user startup_parameters + /// cannot be honored: pg_doorman has no per-user backend on which to + /// apply them. Drop the parsed map before it reaches downstream code + /// and warn once per (pool, username) so the operator notices. + fn dedicated_mode_filter(&self, entry: &mut CacheEntry, username: &str) { + if !self.is_dedicated || entry.startup_parameters.is_empty() { + return; + } + // One increment per drop event (a single fetched row whose + // overlay was dropped because of dedicated mode), matching every + // other reason on this counter so `rate by(reason)` is + // dimensionally consistent. The warn log carries the same + // once-per-(pool, user) shape. + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[self.pool_name.as_str(), "dedicated_mode"]) + .inc(); + if self + .dedicated_warnings + .insert(username.to_string(), ()) + .is_none() + { + warn!( + "[{username}@{pool}] per-user startup_parameters ignored in dedicated \ + auth_query mode; use pool-level startup_parameters instead", + pool = self.pool_name + ); + } + entry.startup_parameters = Arc::new(std::collections::HashMap::new()); + } + + /// When a fresh auth_query fetch produces a per-user + /// `startup_parameters` map that differs from the snapshot frozen + /// into the live dynamic pool at creation time, drop the pool so + /// the next client connection rebuilds against the new overlay. + /// Without this, an operator-side change to the row (`UPDATE + /// pgbouncer.users SET startup_parameters = ...`) only takes effect + /// for new dynamic-pool spawns, not for existing pools. Dedicated + /// mode and the dedicated-mode warning path land here with an empty + /// map; that compares equal to the empty-overlay hash that + /// dedicated pools store, so nothing is dropped on that path. + fn drop_dynamic_pool_if_overlay_drifted( + &self, + username: &str, + new_overlay: &std::collections::HashMap, + ) { + let identifier = crate::pool::PoolIdentifier::new(&self.pool_name, username); + if !crate::pool::is_dynamic_pool(&identifier) { + return; + } + let new_hash = crate::pool::per_user_overlay_hash(new_overlay.iter()); + let live_hash = crate::pool::POOLS + .load() + .get(&identifier) + .map(|p| p.per_user_startup_overlay_hash); + match live_hash { + Some(h) if h != new_hash => { + if crate::pool::drop_dynamic_pool(&identifier) { + info!( + "[{username}@{}] auth_query overlay drift on refetch — dynamic pool dropped, next connect will rebuild", + self.pool_name + ); + } + } + _ => {} } } @@ -507,13 +773,23 @@ impl AuthQueryCache { } } - // Cache miss — fetch from PG + // Cache miss: fetch credentials from PG. self.inc(|s| &s.executor_queries); - match self.executor.fetch(username).await { - Ok(Some(password_hash)) => { + match self.executor.fetch_credentials(username).await { + Ok(Some((password_hash, startup_params))) => { self.inc(|s| &s.cache_misses); - let entry = CacheEntry::positive(password_hash); + let mut entry = CacheEntry::positive(password_hash); + entry.startup_parameters = Arc::new(startup_params); + self.dedicated_mode_filter(&mut entry, username); + // Publish the fresh entry first so any concurrent + // create_dynamic_pool peeks the new overlay, then drop the + // pool whose snapshot drifted. Reversing the order would + // open a window where the drop runs against the live pool + // while the cache still holds the old map, and a racing + // create_dynamic_pool would rebuild against that stale + // map and immediately drift again. self.entries.insert(username.to_string(), entry.clone()); + self.drop_dynamic_pool_if_overlay_drifted(username, &entry.startup_parameters); Ok(Some(entry)) } Ok(None) => { @@ -572,14 +848,18 @@ impl AuthQueryCache { } } - // Fetch fresh from PG + // Fetch fresh from PG. self.inc(|s| &s.executor_queries); self.inc(|s| &s.cache_refetches); - match self.executor.fetch(username).await { - Ok(Some(password_hash)) => { + match self.executor.fetch_credentials(username).await { + Ok(Some((password_hash, startup_params))) => { let mut entry = CacheEntry::positive(password_hash); + entry.startup_parameters = Arc::new(startup_params); entry.last_refetch_at = Some(Instant::now()); + self.dedicated_mode_filter(&mut entry, username); + // Insert before drop — see comment in get_or_fetch. self.entries.insert(username.to_string(), entry.clone()); + self.drop_dynamic_pool_if_overlay_drifted(username, &entry.startup_parameters); Ok(Some(entry)) } Ok(None) => { @@ -596,9 +876,11 @@ impl AuthQueryCache { } /// Clear all entries (called on RELOAD when auth_query config changes). + /// Also resets dedicated-mode warning suppression after reload. pub fn clear(&self) { self.entries.clear(); self.locks.clear(); + self.dedicated_warnings.clear(); } /// Store ClientKey for a cached user (called after successful SCRAM auth). @@ -615,6 +897,31 @@ impl AuthQueryCache { .and_then(|e| e.client_key.clone()) } + /// Synchronous lookup of the cached per-user startup_parameters map. + /// Returns `None` when there is no positive, unexpired cache entry. This + /// never queries PostgreSQL or initializes the executor. + /// + /// The TTL check prevents replenishment and anticipation from using stale + /// per-user GUCs after the auth_query row should have expired. + pub fn peek_startup_parameters( + &self, + username: &str, + f: impl FnOnce(&std::collections::HashMap) -> R, + ) -> Option { + // Closure-based to avoid cloning the cached HashMap on every + // backend spawn. The DashMap shard read-lock is held only for the + // duration of `f`; consumers merge the overlay directly into their + // owned destination map instead of through an intermediate clone. + let entry = self.entries.get(username)?; + if entry.is_negative { + return None; + } + if entry.is_expired(&self.cache_ttl, &self.cache_failure_ttl) { + return None; + } + Some(f(&entry.startup_parameters)) + } + /// Number of cached entries (for metrics/admin). pub fn len(&self) -> usize { self.entries.len() @@ -639,6 +946,10 @@ mod tests { /// Pre-configure responses; fetch calls are counted. struct MockFetcher { responses: DashMap>, + /// Optional per-user startup_parameters map. Surfaced via + /// `fetch_credentials` so cache-side wiring can be exercised + /// without standing up a real PG. + params: DashMap>, fetch_count: AtomicUsize, /// Optional delay to simulate slow PG queries (for concurrency tests). delay: std::time::Duration, @@ -648,6 +959,7 @@ mod tests { fn new() -> Self { Self { responses: DashMap::new(), + params: DashMap::new(), fetch_count: AtomicUsize::new(0), delay: std::time::Duration::ZERO, } @@ -656,6 +968,7 @@ mod tests { fn with_delay(delay: std::time::Duration) -> Self { Self { responses: DashMap::new(), + params: DashMap::new(), fetch_count: AtomicUsize::new(0), delay, } @@ -666,6 +979,21 @@ mod tests { .insert(username.to_string(), Some(password_hash.to_string())); } + fn add_user_with_params( + &self, + username: &str, + password_hash: &str, + params: &[(&str, &str)], + ) { + self.responses + .insert(username.to_string(), Some(password_hash.to_string())); + let map: std::collections::HashMap = params + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(); + self.params.insert(username.to_string(), map); + } + fn fetch_count(&self) -> usize { self.fetch_count.load(Ordering::SeqCst) } @@ -690,6 +1018,30 @@ mod tests { Ok(result) } } + + fn fetch_credentials<'a>( + &'a self, + username: &'a str, + ) -> impl Future, Error>> + Send + 'a { + self.fetch_count.fetch_add(1, Ordering::SeqCst); + let pw = self + .responses + .get(username) + .map(|r| r.clone()) + .unwrap_or(None); + let params = self + .params + .get(username) + .map(|r| r.clone()) + .unwrap_or_default(); + let delay = self.delay; + async move { + if !delay.is_zero() { + tokio::time::sleep(delay).await; + } + Ok(pw.map(|p| (p, params))) + } + } } fn test_config() -> AuthQueryConfig { @@ -950,4 +1302,261 @@ mod tests { assert_eq!(stats.cache_rate_limited.load(Ordering::Relaxed), 1); assert_eq!(stats.executor_queries.load(Ordering::Relaxed), 2); // no new query } + + // -- parse_startup_parameters_text: pure-parser unit tests -- + + #[test] + fn parse_startup_parameters_absent_column_returns_empty() { + let r = AuthQueryExecutor::parse_startup_parameters_text(None, "u", "p"); + assert!(r.is_empty()); + } + + #[test] + fn parse_startup_parameters_empty_string_returns_empty() { + let r = AuthQueryExecutor::parse_startup_parameters_text(Some(""), "u", "p"); + assert!(r.is_empty()); + } + + #[test] + fn parse_startup_parameters_simple_json_object() { + let r = AuthQueryExecutor::parse_startup_parameters_text( + Some(r#"{"plan_cache_mode":"force_custom_plan","work_mem":"64MB"}"#), + "u", + "p", + ); + assert_eq!( + r.get("plan_cache_mode").map(String::as_str), + Some("force_custom_plan") + ); + assert_eq!(r.get("work_mem").map(String::as_str), Some("64MB")); + assert_eq!(r.len(), 2); + } + + #[test] + fn parse_startup_parameters_reserved_key_dropped() { + // 'user' is reserved by pg_doorman; the valid sibling key survives. + let r = AuthQueryExecutor::parse_startup_parameters_text( + Some(r#"{"user":"x","work_mem":"64MB"}"#), + "u", + "p", + ); + assert!(!r.contains_key("user")); + assert_eq!(r.get("work_mem").map(String::as_str), Some("64MB")); + } + + #[test] + fn parse_startup_parameters_non_string_values_dropped() { + // number, boolean, null, array, object on the right-hand side are + // all rejected; only string-valued entries survive. + let r = AuthQueryExecutor::parse_startup_parameters_text( + Some( + r#"{"work_mem":64,"on":true,"off":null,"arr":[1],"obj":{},"plan_cache_mode":"force_custom_plan"}"#, + ), + "u", + "p", + ); + assert_eq!(r.len(), 1); + assert_eq!( + r.get("plan_cache_mode").map(String::as_str), + Some("force_custom_plan") + ); + } + + #[test] + fn parse_startup_parameters_malformed_json_returns_empty() { + let r = AuthQueryExecutor::parse_startup_parameters_text(Some("not-json"), "u", "p"); + assert!(r.is_empty()); + } + + #[test] + fn parse_startup_parameters_non_object_returns_empty() { + let r = AuthQueryExecutor::parse_startup_parameters_text(Some("[1,2,3]"), "u", "p"); + assert!(r.is_empty()); + } + + #[test] + fn parse_startup_parameters_oversize_text_returns_empty() { + // HIGH #9 regression guard: pathological auth_query row should not + // make serde_json walk megabytes of JSON. The raw text cap matches + // `MAX_OPERATOR_BUDGET`, so anything past that returns empty before + // we even start parsing. Drop the same value into a giant string + // so the byte length crosses the cap independently of JSON shape. + let cap = crate::config::startup_parameters::MAX_OPERATOR_BUDGET; + let bytes = "a".repeat(cap + 1); + let r = AuthQueryExecutor::parse_startup_parameters_text(Some(&bytes), "u", "p"); + assert!( + r.is_empty(), + "oversize raw column must be rejected before serde_json walks it" + ); + } + + #[test] + fn parse_startup_parameters_invalid_guc_name_dropped() { + // Keys with spaces fail the shared `is_valid_guc_name` check used + // for operator-supplied parameter maps. + let r = AuthQueryExecutor::parse_startup_parameters_text( + Some(r#"{"bad name":"x","plan_cache_mode":"force_custom_plan"}"#), + "u", + "p", + ); + assert!(!r.contains_key("bad name")); + assert!(r.contains_key("plan_cache_mode")); + } + + #[test] + fn parse_startup_parameters_null_byte_value_dropped() { + // A null byte in the value fails the shared validator; the good + // neighbor still survives. + let r = AuthQueryExecutor::parse_startup_parameters_text( + Some("{\"work_mem\":\"64\\u0000MB\",\"plan_cache_mode\":\"force_custom_plan\"}"), + "u", + "p", + ); + assert!(!r.contains_key("work_mem")); + assert!(r.contains_key("plan_cache_mode")); + } + + // -- dedicated_mode_filter: drops params + warns once per username -- + + #[tokio::test] + async fn dedicated_mode_filter_drops_params_and_warns_once() { + let fetcher = Arc::new(MockFetcher::new()); + fetcher.add_user_with_params("alice", "md5abc123", &[("work_mem", "64MB")]); + let mut config = test_config(); + // Mark the config as dedicated by providing a server_user. + config.server_user = Some("doorman_backend".to_string()); + + let cache = make_cache(fetcher.clone(), &config); + + // Cache miss path applies the filter: per-user params are dropped + // because the backend identity is shared in dedicated mode. + let entry = cache.get_or_fetch("alice").await.unwrap().unwrap(); + assert!( + entry.startup_parameters.is_empty(), + "params must be cleared in dedicated mode" + ); + + // The warning fires at most once per username: subsequent calls do + // not insert into dedicated_warnings again. We assert that the + // tracker still holds exactly one entry after a second miss-and-fill. + cache.invalidate("alice"); + let entry = cache.get_or_fetch("alice").await.unwrap().unwrap(); + assert!(entry.startup_parameters.is_empty()); + assert_eq!(cache.dedicated_warnings.len(), 1); + + // clear() resets the warning tracker so a config reload re-arms it. + cache.clear(); + assert_eq!(cache.dedicated_warnings.len(), 0); + } + + #[tokio::test] + async fn non_dedicated_mode_keeps_params() { + let fetcher = Arc::new(MockFetcher::new()); + fetcher.add_user_with_params("alice", "md5abc123", &[("work_mem", "64MB")]); + let config = test_config(); // server_user = None: passthrough mode + + let cache = make_cache(fetcher.clone(), &config); + let entry = cache.get_or_fetch("alice").await.unwrap().unwrap(); + assert_eq!( + entry.startup_parameters.get("work_mem").map(String::as_str), + Some("64MB") + ); + } + + // --------------------------------------------------------------------- + // peek_startup_parameters: sync, non-fetching lookup used by backend spawn + // --------------------------------------------------------------------- + + // Closure-based API tested by snapshotting the borrowed HashMap into + // an owned one when an existing assertion needs to inspect contents. + // Generic over the cache's fetcher because the test harness uses a + // `MockFetcher` rather than the production `AuthQueryExecutor`. + fn peek_snapshot( + cache: &AuthQueryCache, + username: &str, + ) -> Option> + where + F: PasswordFetcher, + { + cache.peek_startup_parameters(username, |m| m.clone()) + } + + #[tokio::test] + async fn peek_startup_parameters_missing_user_returns_none() { + let fetcher = Arc::new(MockFetcher::new()); + let config = test_config(); + let cache = make_cache(fetcher, &config); + assert!(peek_snapshot(&cache, "alice").is_none()); + } + + #[tokio::test] + async fn peek_startup_parameters_negative_entry_returns_none() { + let fetcher = Arc::new(MockFetcher::new()); + // No user added; first lookup populates a negative cache entry. + let config = test_config(); + let cache = make_cache(fetcher, &config); + assert!(cache.get_or_fetch("ghost").await.unwrap().is_none()); + assert!(peek_snapshot(&cache, "ghost").is_none()); + } + + #[tokio::test] + async fn peek_startup_parameters_returns_none_for_expired_entry() { + // HIGH #7 regression guard: a positive cache entry that has lived + // past `cache_ttl` must not pin a stale per-user startup parameter + // onto a backend the replenishment loop spawns later. Mirrors + // `test_cache_ttl_expiration` but exercises the peek path the + // backend-spawn hot path uses. + let fetcher = Arc::new(MockFetcher::new()); + fetcher.add_user_with_params("alice", "md5abc123", &[("work_mem", "64MB")]); + let mut config = test_config(); + config.cache_ttl = Duration::from_millis(50); + + let cache = make_cache(fetcher, &config); + cache.get_or_fetch("alice").await.unwrap().unwrap(); + // Verify that peek sees the entry before it expires. + assert!(peek_snapshot(&cache, "alice").is_some()); + + tokio::time::sleep(std::time::Duration::from_millis(80)).await; + + assert!( + peek_snapshot(&cache, "alice").is_none(), + "peek must return None once cache_ttl has elapsed for the entry" + ); + } + + #[tokio::test] + async fn peek_startup_parameters_positive_entry_returns_map() { + let fetcher = Arc::new(MockFetcher::new()); + fetcher.add_user_with_params( + "alice", + "md5abc123", + &[("work_mem", "64MB"), ("statement_timeout", "10s")], + ); + let config = test_config(); + let cache = make_cache(fetcher, &config); + cache.get_or_fetch("alice").await.unwrap().unwrap(); + + let params = peek_snapshot(&cache, "alice").unwrap(); + assert_eq!(params.get("work_mem").map(String::as_str), Some("64MB")); + assert_eq!( + params.get("statement_timeout").map(String::as_str), + Some("10s") + ); + } + + #[tokio::test] + async fn peek_startup_parameters_dedicated_mode_returns_empty() { + // Dedicated mode keeps the user cached but removes per-user params. + let fetcher = Arc::new(MockFetcher::new()); + fetcher.add_user_with_params("alice", "md5abc123", &[("work_mem", "64MB")]); + let mut config = test_config(); + config.server_user = Some("shared".to_string()); + config.server_password = Some("secret".to_string()); + + let cache = make_cache(fetcher, &config); + cache.get_or_fetch("alice").await.unwrap().unwrap(); + + let params = peek_snapshot(&cache, "alice").unwrap(); + assert!(params.is_empty()); + } } diff --git a/src/auth/mod.rs b/src/auth/mod.rs index 0bed5d2f1..8bd6fd9c7 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -11,6 +11,7 @@ pub mod talos; // Standard library imports use std::marker::Unpin; use std::sync::atomic::Ordering; +use std::sync::Arc; // External crate imports use crate::auth::hba::CheckResult; @@ -325,6 +326,22 @@ where let server_parameters = match pool.get_server_parameters().await { Ok(params) => params, Err(err) => { + // PG-side rejection of an operator-supplied startup + // parameter already carries the real sqlstate and message + // from PostgreSQL. Forward them verbatim — same contract + // the transaction checkout path in + // src/client/transaction.rs honours — instead of collapsing + // into the generic 3D000 wrapper. + if let Error::ServerStartupParameterRejection { + sqlstate, + message: pg_message, + .. + } = &err + { + error!("[{username_from_parameters}@{pool_name}] PG rejected operator-supplied startup parameter: {pg_message}"); + error_response(write, pg_message, sqlstate).await?; + return Err(err); + } error!("[{username_from_parameters}@{pool_name}] failed to retrieve server parameters: {err}"); error_response( write, @@ -879,6 +896,18 @@ where let server_parameters = match pool.get_server_parameters().await { Ok(params) => params, Err(err) => { + // Forward PG-rejected operator startup parameter + // verbatim, same as the static-user path above. + if let Error::ServerStartupParameterRejection { + sqlstate, + message: pg_message, + .. + } = &err + { + error!("[{username}@{pool_name}] auth_query: PG rejected operator-supplied startup parameter: {pg_message}"); + error_response(write, pg_message, sqlstate).await?; + return Err(err); + } error!( "[{username}@{pool_name}] auth_query: failed to get server parameters: {err}" ); @@ -908,8 +937,13 @@ where auth_client_key.map(BackendAuthMethod::ScramPassthrough) }; - let mut pool = - create_dynamic_pool(pool_name, username, backend_auth).map_err(|err| { + // Use the overlay from the same auth_query row that + // authenticated this user. That keeps dynamic-pool creation + // tied to this login instead of reading the global cache + // again while TTL expiry or a concurrent refetch is changing it. + let fetched_overlay = Arc::clone(&cache_entry.startup_parameters); + let mut pool = create_dynamic_pool(pool_name, username, backend_auth, fetched_overlay) + .map_err(|err| { error!( "[{username}@{pool_name}] auth_query: failed to create dynamic pool: {err}" ); @@ -926,6 +960,16 @@ where let server_parameters = match pool.get_server_parameters().await { Ok(params) => params, Err(err) => { + if let Error::ServerStartupParameterRejection { + sqlstate, + message: pg_message, + .. + } = &err + { + error!("[{username}@{pool_name}] auth_query passthrough: PG rejected operator-supplied startup parameter: {pg_message}"); + error_response(write, pg_message, sqlstate).await?; + return Err(err); + } error!("[{username}@{pool_name}] auth_query: passthrough pool failed: {err}"); error_response( write, diff --git a/src/client/transaction.rs b/src/client/transaction.rs index 903fbe520..4c0aa5487 100644 --- a/src/client/transaction.rs +++ b/src/client/transaction.rs @@ -759,6 +759,45 @@ where // Mirrors the SQLSTATE in the ErrorResponse below // so the per-pool breakdown reflects checkout // failures alongside PG-side errors. + // + // Special case: PG itself rejected the + // operator-supplied startup_parameters cascade. + // Forward the verbatim sqlstate/message so the + // client receives the same PG-native error it + // would have seen connecting to PG directly, + // instead of the generic 53300 + // (too_many_connections) checkout-fallback. + // Same shape as the rest of the branch (reset + // buffered state on 'S', error_response, log, + // return), only the SQLSTATE and message differ. + if let crate::pool::PoolError::Backend( + Error::ServerStartupParameterRejection { + sqlstate, + message: pg_message, + .. + }, + ) = &err + { + current_pool.address.stats.error_with_sqlstate(sqlstate); + self.stats.checkout_error(); + + if message[0] as char == 'S' { + self.reset_buffered_state(); + } + + error_response(&mut self.write, pg_message, sqlstate).await?; + + error!( + "[{}@{} #c{}] PG rejected startup_parameters: sqlstate={} {}", + self.username, + self.pool_name, + self.connection_id, + sqlstate, + pg_message, + ); + return Err(Error::AllServersDown); + } + current_pool.address.stats.error_with_sqlstate("53300"); self.stats.checkout_error(); diff --git a/src/config/general.rs b/src/config/general.rs index d4154da9a..d5406b1a6 100644 --- a/src/config/general.rs +++ b/src/config/general.rs @@ -268,6 +268,18 @@ pub struct General { // New pg_hba rules: either inline content or a file path (see `PgHba` deserialization). #[serde(default, skip_serializing)] pub pg_hba: Option, + + /// Operator-supplied PostgreSQL configuration parameters added to + /// backend `StartupMessage`s. The general map is the baseline; + /// pool-level settings override per key, and passthrough `auth_query` + /// rows can override per user. Config load validates reserved keys, + /// GUC names, null bytes, and this level's size; the merged cascade is + /// checked again before each backend startup. If PostgreSQL rejects an + /// operator-supplied parameter at backend startup, the client receives + /// the PG error unchanged — pg_doorman never substitutes its own + /// retry, fallback, or per-key quarantine for the backend's verdict. + #[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")] + pub startup_parameters: std::collections::BTreeMap, } impl General { @@ -585,6 +597,7 @@ impl Default for General { Self::default_query_interner_anon_idle_ttl_seconds(), hba: Self::default_hba(), pg_hba: None, + startup_parameters: std::collections::BTreeMap::new(), daemon_pid_file: Self::default_daemon_pid_file(), syslog_prog_name: None, pooler_check_query: Self::default_pooler_check_query(), diff --git a/src/config/mod.rs b/src/config/mod.rs index f4bb9c34a..567e49cae 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -27,6 +27,7 @@ mod duration; mod general; mod include; mod pool; +pub mod startup_parameters; mod talos; pub mod tls; mod user; @@ -421,6 +422,13 @@ impl Config { // Validate Talos self.talos.validate().await?; + // Validate operator-supplied PostgreSQL startup parameters at the + // general level; per-pool maps are validated inside `Pool::validate`. + startup_parameters::validate( + &self.general.startup_parameters, + "general.startup_parameters", + )?; + if self.general.tls_rate_limit_per_second < 100 && self.general.tls_rate_limit_per_second != 0 { diff --git a/src/config/pool.rs b/src/config/pool.rs index 325cd5345..3da2665d1 100644 --- a/src/config/pool.rs +++ b/src/config/pool.rs @@ -176,6 +176,15 @@ pub struct Pool { #[serde(skip_serializing_if = "Option::is_none")] pub auth_query: Option, + /// Pool-level PostgreSQL configuration parameters added to backend + /// `StartupMessage`s. These values override general settings per key; + /// passthrough `auth_query` rows can override them per user. Config + /// load validates reserved keys, GUC names, null bytes, and this + /// level's size; the merged cascade is checked again before each + /// backend startup. + #[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")] + pub startup_parameters: std::collections::BTreeMap, + #[serde( default = "Pool::default_users", deserialize_with = "deserialize_users" @@ -232,6 +241,11 @@ impl Pool { } pub async fn validate(&mut self) -> Result<(), Error> { + crate::config::startup_parameters::validate( + &self.startup_parameters, + "pool.startup_parameters", + )?; + // Validate scaling_warm_pool_ratio if let Some(ratio) = self.scaling_warm_pool_ratio { if ratio > 100 { @@ -457,6 +471,7 @@ impl Default for Pool { server_tls_certificate: None, server_tls_private_key: None, auth_query: None, + startup_parameters: std::collections::BTreeMap::new(), } } } diff --git a/src/config/startup_parameters.rs b/src/config/startup_parameters.rs new file mode 100644 index 000000000..f8ea832fc --- /dev/null +++ b/src/config/startup_parameters.rs @@ -0,0 +1,395 @@ +//! Validation of operator-supplied PostgreSQL startup parameters. +//! +//! Used by [`crate::config::General`] and [`crate::config::Pool`] to refuse +//! configs that try to inject reserved protocol keys (user, database, +//! replication, options, `_pq_.*`), or that would exceed PG's +//! `MAX_STARTUP_PACKET_LENGTH` (10 000 bytes) StartupMessage body cap once +//! concatenated with `user`+`database`+`application_name`. + +use std::collections::BTreeMap; + +use crate::errors::Error; + +/// PostgreSQL caps the StartupMessage body at 10 000 bytes +/// (`MAX_STARTUP_PACKET_LENGTH` in `src/include/libpq/pqcomm.h`) to prevent +/// memory-exhaustion attacks via oversize packets. pg_doorman reserves a +/// modest slice for its own `user`/`database`/`application_name` triple +/// and the protocol's per-pair NUL terminators; the rest is the budget +/// available to operator-supplied parameters. +pub const MAX_STARTUP_PACKET_SIZE: usize = 10_000; +pub const RESERVED_HEADROOM: usize = 512; +pub const MAX_OPERATOR_BUDGET: usize = MAX_STARTUP_PACKET_SIZE - RESERVED_HEADROOM; + +/// Keys pg_doorman manages itself or that PG treats specially in the startup +/// packet. Operator must not put them in `startup_parameters`. +/// +/// `role` and `session_authorization` are blocked because they affect +/// PostgreSQL authorization state, not session defaults: they become the +/// `reset_val` for that backend, so a `RESET ROLE` after some `SET ROLE` +/// returns to the operator-injected role instead of the login role +/// pg_doorman authenticated as. Letting these through `startup_parameters` +/// would break the contract that the cascade only configures benign +/// session defaults. +pub const RESERVED_KEYS: &[&str] = &[ + "user", + "database", + "replication", + "options", + "role", + "session_authorization", +]; +pub const RESERVED_PREFIX: &str = "_pq_."; + +/// Allowed GUC name shape: ASCII letter / underscore, then letters / +/// digits / underscores / dots (for namespaced GUC like +/// `auto_explain.log_min_duration`). Equivalent to the regex +/// `^[A-Za-z_][A-Za-z0-9_.]*$`; hand-rolled to keep `regex` out of the +/// runtime dependency set. +fn is_valid_guc_name(key: &str) -> bool { + let mut bytes = key.bytes(); + let Some(first) = bytes.next() else { + return false; + }; + if !(first.is_ascii_alphabetic() || first == b'_') { + return false; + } + bytes.all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'.') +} + +/// Validate one map (general or per-pool). +/// +/// * `scope` — human-friendly label used in error messages, e.g. +/// `"general.startup_parameters"` or `"pool.startup_parameters"`. +pub fn validate(map: &BTreeMap, scope: &str) -> Result<(), Error> { + for (k, v) in map { + validate_key(k, scope)?; + validate_value(k, v, scope)?; + } + validate_total_size(map, scope) +} + +/// Validate a single borrowed `(key, value)` pair the same way [`validate`] +/// would. Used by the auth_query JSON parser to check entries inline +/// without building a one-element `BTreeMap` for each one. The total-size +/// check is *not* applied here — that gate runs once over the parent +/// map in [`validate`] (config load) or against the merged cascade at +/// runtime in `ServerPool::resolved_startup_parameters`. +pub fn validate_entry(key: &str, value: &str, scope: &str) -> Result<(), Error> { + validate_key(key, scope)?; + validate_value(key, value, scope) +} + +fn validate_key(key: &str, scope: &str) -> Result<(), Error> { + if key.is_empty() { + return Err(Error::BadConfig(format!("{scope}: empty key"))); + } + if RESERVED_KEYS.iter().any(|r| r.eq_ignore_ascii_case(key)) { + return Err(Error::BadConfig(format!( + "{scope}: '{key}' is reserved and managed by pg_doorman" + ))); + } + if key.starts_with(RESERVED_PREFIX) { + return Err(Error::BadConfig(format!( + "{scope}: '{key}' uses the reserved '_pq_.' prefix" + ))); + } + if !is_valid_guc_name(key) { + return Err(Error::BadConfig(format!( + "{scope}: '{key}' is not a valid GUC name (expected [A-Za-z_][A-Za-z0-9_.]*)" + ))); + } + Ok(()) +} + +fn validate_value(key: &str, value: &str, scope: &str) -> Result<(), Error> { + if value.as_bytes().contains(&b'\0') { + return Err(Error::BadConfig(format!( + "{scope}: value for '{key}' contains a null byte" + ))); + } + Ok(()) +} + +fn validate_total_size(map: &BTreeMap, scope: &str) -> Result<(), Error> { + let total = serialized_bytes(map); + if total > MAX_OPERATOR_BUDGET { + return Err(Error::BadConfig(format!( + "{scope}: serialized size {total} bytes exceeds operator budget {MAX_OPERATOR_BUDGET} \ + (PG StartupMessage cap is {MAX_STARTUP_PACKET_SIZE} bytes per \ + MAX_STARTUP_PACKET_LENGTH; {RESERVED_HEADROOM} reserved for \ + pg_doorman-managed keys)" + ))); + } + Ok(()) +} + +/// Bytes the operator-supplied map will occupy on the StartupMessage wire, +/// per the PG layout where each pair contributes `key\0value\0`. +pub fn serialized_bytes(map: &BTreeMap) -> usize { + map.iter().map(|(k, v)| k.len() + 1 + v.len() + 1).sum() +} + +/// Exact byte length of the full StartupMessage pg_doorman will put on the +/// wire for one backend spawn, *including* the 4-byte length prefix. The +/// layout mirrors `crate::messages::protocol::startup`: +/// +/// * 4 bytes - length prefix (the wire field itself) +/// * 4 bytes - protocol version +/// * `"user\0\0"`, `"application_name\0\0"`, `"database\0\0"` +/// (`application_name` from `extras` wins over the pg_doorman default) +/// * each remaining `(key, value)` pair as `key\0value\0` +/// * 1 byte - parameter-list terminator (`\0`) +/// +/// The per-level config validation only sees `extras`; this helper is what +/// the runtime path uses to ensure the *full* packet still fits under PG's +/// `MAX_STARTUP_PACKET_LENGTH` cap once user / database / application_name +/// are included. +pub fn full_packet_bytes( + user: &str, + database: &str, + application_name: &str, + extras: &BTreeMap, +) -> usize { + packet_and_body_bytes(user, database, application_name, extras).0 +} + +/// Single-pass variant that returns both the full StartupMessage byte +/// length and the body-only byte count (what `serialized_bytes` reports +/// for the operator-supplied map). Used by the runtime budget/packet +/// guard to avoid walking the map three times per backend spawn. +pub fn packet_and_body_bytes( + user: &str, + database: &str, + application_name: &str, + extras: &BTreeMap, +) -> (usize, usize) { + let mut packet = 4usize + 4; // length prefix + protocol version + packet += b"user\0".len() + user.len() + 1; + packet += b"database\0".len() + database.len() + 1; + let effective_app_name = extras + .get("application_name") + .map(String::as_str) + .unwrap_or(application_name); + packet += b"application_name\0".len() + effective_app_name.len() + 1; + let mut body = 0usize; + for (key, value) in extras { + // `serialized_bytes` counts every operator-supplied pair, + // including `application_name` — keep the same accounting so + // the budget check stays comparable across callers. + body += key.len() + 1 + value.len() + 1; + if key == "application_name" { + continue; + } + packet += key.len() + 1 + value.len() + 1; + } + packet += 1; // parameter-list terminator + (packet, body) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn m(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect() + } + + #[test] + fn empty_map_is_valid() { + assert!(validate(&BTreeMap::new(), "general.startup_parameters").is_ok()); + } + + #[test] + fn plain_guc_is_valid() { + let map = m(&[ + ("plan_cache_mode", "force_custom_plan"), + ("work_mem", "64MB"), + ]); + assert!(validate(&map, "general.startup_parameters").is_ok()); + } + + #[test] + fn namespaced_guc_is_valid() { + let map = m(&[("auto_explain.log_min_duration", "100ms")]); + assert!(validate(&map, "pools.foo.startup_parameters").is_ok()); + } + + #[test] + fn reserved_user_rejected() { + let err = validate(&m(&[("user", "x")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn reserved_database_rejected_case_insensitive() { + let err = validate(&m(&[("DATABASE", "x")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn reserved_role_rejected() { + // `role` changes PG authorization state, not a session default. + // Letting it through startup_parameters would mean RESET ROLE + // restores the operator-injected role, not the login role. + let err = validate(&m(&[("role", "admin")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn reserved_role_rejected_case_insensitive() { + let err = validate(&m(&[("ROLE", "admin")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn reserved_session_authorization_rejected() { + let err = validate(&m(&[("session_authorization", "admin")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn reserved_session_authorization_rejected_case_insensitive() { + let err = validate(&m(&[("Session_Authorization", "admin")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn validate_entry_rejects_role() { + // auth_query JSON entries go through validate_entry, not validate. + let err = validate_entry("role", "admin", "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn validate_entry_rejects_session_authorization() { + let err = validate_entry("session_authorization", "admin", "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref msg) if msg.contains("reserved"))); + } + + #[test] + fn pq_prefix_rejected() { + let err = validate(&m(&[("_pq_.fancy_ext", "x")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(_))); + } + + #[test] + fn empty_key_rejected() { + let err = validate(&m(&[("", "x")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref m) if m.contains("empty key"))); + } + + #[test] + fn weird_chars_rejected() { + let err = validate(&m(&[("bad name", "x")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(_))); + } + + #[test] + fn null_byte_in_value_rejected() { + let err = validate(&m(&[("work_mem", "64\0MB")]), "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref m) if m.contains("null byte"))); + } + + #[test] + fn oversize_rejected() { + // 16 keys × 1 KiB value still overruns the 9 488-byte operator budget. + let big: BTreeMap = (0..16) + .map(|i| (format!("key{i}"), "a".repeat(1024))) + .collect(); + let err = validate(&big, "scope").unwrap_err(); + assert!(matches!(err, Error::BadConfig(ref m) if m.contains("exceeds operator budget"))); + } + + #[test] + fn application_name_is_not_reserved() { + // application_name is explicitly allowed in startup_parameters; the + // operator-wins merge against pg_doorman's default happens at the + // wire layer, not here. + let map = m(&[("application_name", "my_app")]); + assert!(validate(&map, "scope").is_ok()); + } + + #[test] + fn budget_matches_pg_startup_packet_cap() { + // Locks the constants in place — PG's MAX_STARTUP_PACKET_LENGTH + // (src/include/libpq/pqcomm.h) is 10 000; pg_doorman reserves + // 512 bytes for its own keys, leaving 9 488 for the operator. + // A future careless edit that drifts back to a 16 KiB ceiling + // would re-introduce silently-rejected configs on every backend + // startup; this assertion is the trip-wire. + assert_eq!(MAX_STARTUP_PACKET_SIZE, 10_000); + assert_eq!(RESERVED_HEADROOM, 512); + assert_eq!( + MAX_OPERATOR_BUDGET, + MAX_STARTUP_PACKET_SIZE - RESERVED_HEADROOM + ); + } + + #[test] + fn serialized_bytes_counts_per_pair_nuls() { + let map = m(&[("k1", "v1"), ("plan_cache_mode", "force_custom_plan")]); + // "k1\0v1\0" = 2 + 1 + 2 + 1 = 6 bytes + // "plan_cache_mode\0force_custom_plan\0" = 15 + 1 + 17 + 1 = 34 bytes + assert_eq!(serialized_bytes(&map), 6 + 34); + } + + #[test] + fn serialized_bytes_empty_map_is_zero() { + assert_eq!(serialized_bytes(&BTreeMap::new()), 0); + } + + #[test] + fn full_packet_bytes_matches_pg_layout() { + let extras = m(&[]); + // 4 + 4 + ("user\0"=5 + 4 + 1) + ("database\0"=9 + 4 + 1) + + // ("application_name\0"=17 + 10 + 1) + 1 = 61 + let n = full_packet_bytes("usr1", "db01", "pg_doorman", &extras); + assert_eq!(n, 4 + 4 + (5 + 4 + 1) + (9 + 4 + 1) + (17 + 10 + 1) + 1); + } + + #[test] + fn full_packet_bytes_overrides_application_name_from_extras() { + let extras = m(&[("application_name", "checkout_pool")]); + let n = full_packet_bytes("usr1", "db01", "pg_doorman", &extras); + // Same as above but with "checkout_pool" (13 bytes) instead of + // "pg_doorman" (10 bytes): 61 + 3 = 64. + assert_eq!(n, 4 + 4 + (5 + 4 + 1) + (9 + 4 + 1) + (17 + 13 + 1) + 1); + } + + #[test] + fn full_packet_bytes_counts_each_extra_pair() { + let extras = m(&[("plan_cache_mode", "force_custom_plan")]); + // Base 61 + key("plan_cache_mode"=15 + 1) + value("force_custom_plan"=17 + 1) = 95. + let n = full_packet_bytes("usr1", "db01", "pg_doorman", &extras); + assert_eq!(n, 61 + (15 + 1) + (17 + 1)); + } + + #[test] + fn cascade_overflow_detectable_after_merge() { + // Each level fits the per-level budget on its own (every map below is + // ~3 KiB), but the union of all three pushes past 9 488 bytes and + // would trip the post-resolve guard in `server_pool.rs`. + let general: BTreeMap = (0..32) + .map(|i| (format!("g_key_{i}"), "a".repeat(100))) + .collect(); + let pool: BTreeMap = (0..32) + .map(|i| (format!("p_key_{i}"), "b".repeat(100))) + .collect(); + let auth: BTreeMap = (0..32) + .map(|i| (format!("a_key_{i}"), "c".repeat(100))) + .collect(); + // Each map ~ 32 * (8 + 1 + 100 + 1) = 32 * 110 = 3 520 bytes < 9 488. + assert!(serialized_bytes(&general) < MAX_OPERATOR_BUDGET); + assert!(serialized_bytes(&pool) < MAX_OPERATOR_BUDGET); + assert!(serialized_bytes(&auth) < MAX_OPERATOR_BUDGET); + + let mut merged: BTreeMap = BTreeMap::new(); + merged.extend(general.iter().map(|(k, v)| (k.clone(), v.clone()))); + merged.extend(pool.iter().map(|(k, v)| (k.clone(), v.clone()))); + merged.extend(auth.iter().map(|(k, v)| (k.clone(), v.clone()))); + assert!(serialized_bytes(&merged) > MAX_OPERATOR_BUDGET); + } +} diff --git a/src/config/tests.rs b/src/config/tests.rs index edc014187..8924cffbc 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -2034,3 +2034,43 @@ sso_allowed_users = ["alice", "bob"] vec!["alice".to_string(), "bob".to_string()] ); } + +#[tokio::test] +async fn reject_reserved_in_general_startup_parameters() { + let mut cfg = Config::default(); + cfg.general + .startup_parameters + .insert("user".to_string(), "x".to_string()); + let err = cfg.validate().await.unwrap_err(); + match err { + Error::BadConfig(msg) => assert!( + msg.contains("general.startup_parameters") && msg.contains("reserved"), + "unexpected message: {msg}" + ), + other => panic!("expected BadConfig, got {other:?}"), + } +} + +#[tokio::test] +async fn reject_reserved_in_pool_startup_parameters() { + let mut cfg = Config::default(); + cfg.general.tls_rate_limit_per_second = 0; + let mut pool = Pool::default(); + pool.startup_parameters + .insert("database".to_string(), "x".to_string()); + pool.users.push(User { + username: "u".to_string(), + password: "p".to_string(), + pool_size: 1, + ..User::default() + }); + cfg.pools.insert("p".to_string(), pool); + let err = cfg.validate().await.unwrap_err(); + match err { + Error::BadConfig(msg) => assert!( + msg.contains("pool.startup_parameters") && msg.contains("reserved"), + "unexpected message: {msg}" + ), + other => panic!("expected BadConfig, got {other:?}"), + } +} diff --git a/src/config/web.rs b/src/config/web.rs index e10b5b3ef..a6d5c56db 100644 --- a/src/config/web.rs +++ b/src/config/web.rs @@ -74,6 +74,18 @@ pub struct Web { /// SSO user resolves to `Sso`. #[serde(default)] pub sso_admin_groups: Vec, + + /// Reject Bearer/cookie/query SSO credentials when the request did + /// not arrive over HTTPS. The listener treats a request as secure + /// only when its TCP peer is in `trusted_proxies` and the proxy + /// forwarded `X-Forwarded-Proto: https`. Defaults to `false` so + /// existing deployments where the SSO proxy terminates TLS on a + /// different host (and reaches pg_doorman over a private network) + /// keep working without configuration changes. Enable on multi- + /// tenant networks where an attacker could observe the HTTP leg + /// between the proxy and pg_doorman. + #[serde(default)] + pub sso_require_https: bool, } impl Web { @@ -93,6 +105,7 @@ impl Web { trusted_proxies: Vec::new(), sso_groups_claim: Self::default_sso_groups_claim(), sso_admin_groups: Vec::new(), + sso_require_https: false, } } diff --git a/src/messages/protocol.rs b/src/messages/protocol.rs index 403de58d0..b8b53a7ba 100644 --- a/src/messages/protocol.rs +++ b/src/messages/protocol.rs @@ -7,6 +7,7 @@ use bytes::{Buf, BufMut, BytesMut}; use md5::{Digest, Md5}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; // Internal crate imports +use crate::config::startup_parameters::full_packet_bytes; use crate::errors::Error; use crate::messages::socket::{write_all, write_all_flush}; use crate::messages::types::DataType; @@ -160,42 +161,70 @@ pub fn simple_query(query: &str) -> BytesMut { } /// Send startup message to the server. +/// +/// Required parameters (`user`, `application_name`, `database`) keep their +/// historical wire order when `extra_params` is empty. Additional parameters +/// are appended in BTreeMap order. `extra_params["application_name"]` +/// overrides the default application name. pub async fn startup( stream: &mut S, - user: String, + user: &str, database: &str, - application_name: String, + application_name: &str, + extra_params: &std::collections::BTreeMap, ) -> Result<(), Error> where S: tokio::io::AsyncWrite + std::marker::Unpin, { - let mut bytes = BytesMut::new(); - - // Protocol version - bytes.put_i32(196608); // Version 3.0 - - // User - bytes.put(&b"user\0"[..]); - bytes.put_slice(user.as_bytes()); - bytes.put_u8(0); - - // Application name - bytes.put(&b"application_name\0"[..]); - bytes.put_slice(application_name.as_bytes()); - bytes.put_u8(0); - - // Database - bytes.put(&b"database\0"[..]); - bytes.put_slice(database.as_bytes()); - bytes.put_u8(0); - bytes.put_u8(0); // Null terminator - - let len = bytes.len() as i32 + 4i32; + // Pre-compute the wire size so the StartupMessage fits in one allocation + // and the length prefix can be written in place. + let total_size = full_packet_bytes(user, database, application_name, extra_params); + let mut startup = BytesMut::with_capacity(total_size); + + // Length prefix (includes itself). + startup.put_i32(total_size as i32); + // Protocol version 3.0. + startup.put_i32(196608); + + // User. + startup.put(&b"user\0"[..]); + startup.put_slice(user.as_bytes()); + startup.put_u8(0); + + // Application name. Operator-supplied value in `extra_params` wins over + // the pg_doorman-managed default. + let effective_app_name = extra_params + .get("application_name") + .map(String::as_str) + .unwrap_or(application_name); + startup.put(&b"application_name\0"[..]); + startup.put_slice(effective_app_name.as_bytes()); + startup.put_u8(0); + + // Database. + startup.put(&b"database\0"[..]); + startup.put_slice(database.as_bytes()); + startup.put_u8(0); + + // Operator-supplied extras (already-handled application_name is skipped). + for (key, value) in extra_params { + if key == "application_name" { + continue; + } + startup.put_slice(key.as_bytes()); + startup.put_u8(0); + startup.put_slice(value.as_bytes()); + startup.put_u8(0); + } - let mut startup = BytesMut::with_capacity(len as usize); + // Parameter-list terminator. + startup.put_u8(0); - startup.put_i32(len); - startup.put(bytes); + debug_assert_eq!( + startup.len(), + total_size, + "full_packet_bytes drifted from the actual startup serializer" + ); match stream.write_all(&startup).await { Ok(_) => Ok(()), @@ -305,7 +334,7 @@ pub fn md5_hash_second_pass(hash: &str, salt: &[u8]) -> Vec { } /// Send password challenge response to the server. -/// This is the MD5 challenge. +/// Handles the MD5 challenge. pub async fn md5_password( stream: &mut S, user: &str, @@ -817,7 +846,7 @@ pub fn insert_close_complete_after_last_close_complete( } /// Insert ParseComplete messages before each ParameterDescription ('t') message. -/// This is used for the Describe flow when Parse was skipped due to caching. +/// Used by the Describe flow when Parse was skipped due to caching. /// /// Describe response for a statement is: /// - ParameterDescription ('t') followed by RowDescription ('T') or NoData ('n') @@ -938,3 +967,99 @@ pub fn insert_close_complete_before_ready_for_query(mut buffer: BytesMut, count: buffer } } + +#[cfg(test)] +mod startup_tests { + use super::*; + use std::collections::BTreeMap; + + #[tokio::test] + async fn startup_with_extra_params_includes_them_in_order() { + let mut buf: Vec = Vec::new(); + let mut params = BTreeMap::new(); + params.insert( + "plan_cache_mode".to_string(), + "force_custom_plan".to_string(), + ); + params.insert("work_mem".to_string(), "64MB".to_string()); + + startup(&mut buf, "alice", "appdb", "myapp", ¶ms) + .await + .expect("startup"); + + // Skip the 4-byte length prefix and 4-byte protocol version. + let body = &buf[8..]; + let body_str = String::from_utf8_lossy(body); + assert!( + body_str.contains("user\0alice"), + "user pair not on wire: {body_str:?}" + ); + assert!( + body_str.contains("database\0appdb"), + "database pair not on wire: {body_str:?}" + ); + assert!( + body_str.contains("application_name\0myapp"), + "default application_name not on wire: {body_str:?}" + ); + assert!(body_str.contains("plan_cache_mode\0force_custom_plan")); + // `\x00` (explicit hex) instead of `\0` here because the following + // digits `64` would otherwise look like an octal escape to clippy. + assert!(body_str.contains("work_mem\x0064MB")); + // The parameter list terminates with a single NUL byte right before the + // end of the packet body. Buffer always ends with that terminator. + assert_eq!(*buf.last().expect("non-empty buffer"), 0); + } + + #[tokio::test] + async fn startup_with_empty_params_keeps_pre_feature_format() { + let mut buf: Vec = Vec::new(); + startup(&mut buf, "alice", "appdb", "myapp", &BTreeMap::new()) + .await + .expect("startup"); + let body = &buf[8..]; + let s = String::from_utf8_lossy(body); + assert!(s.contains("user\0alice")); + assert!(s.contains("database\0appdb")); + assert!(s.contains("application_name\0myapp")); + } + + #[tokio::test] + async fn startup_application_name_operator_override_wins() { + let mut buf: Vec = Vec::new(); + let mut params = BTreeMap::new(); + params.insert("application_name".to_string(), "operator_app".to_string()); + + startup(&mut buf, "alice", "appdb", "ignored", ¶ms) + .await + .expect("startup"); + + let body = &buf[8..]; + let s = String::from_utf8_lossy(body); + // Operator-supplied wins over the pg_doorman-managed default. + assert!(s.contains("application_name\0operator_app")); + // pg_doorman's own application_name argument value must not be on the wire. + assert!(!s.contains("application_name\0ignored")); + } + + /// The length prefix must match the byte count PostgreSQL reads. + #[tokio::test] + async fn startup_length_prefix_matches_body() { + let mut buf: Vec = Vec::new(); + let mut params = BTreeMap::new(); + params.insert("k1".to_string(), "v1".to_string()); + params.insert("k2".to_string(), "v2".to_string()); + + startup(&mut buf, "u", "d", "a", ¶ms) + .await + .expect("startup"); + + let claimed_len = i32::from_be_bytes(buf[0..4].try_into().expect("4 bytes")); + assert_eq!( + claimed_len as usize, + buf.len(), + "claimed length {claimed_len} != actual {} bytes", + buf.len() + ); + } +} diff --git a/src/pool/auth_query_state.rs b/src/pool/auth_query_state.rs index b9c4068f0..9b9928732 100644 --- a/src/pool/auth_query_state.rs +++ b/src/pool/auth_query_state.rs @@ -20,6 +20,22 @@ use super::PoolIdentifier; pub struct AuthQueryState { cache_cell: tokio::sync::OnceCell, pub(crate) config: AuthQueryConfig, + /// Hash of the pool-level `startup_parameters` map captured at the + /// moment this state was built. RELOAD compares it against the new + /// `pool_config.startup_parameters` hash and drains dynamic pools + + /// rebuilds the dedicated shared pool when they differ — otherwise + /// dynamic backends would keep starting with the previous baseline's + /// `reset_val`. + pub(crate) pool_startup_hash: u64, + /// Fingerprint of every other parent input the dedicated shared + /// pool was built from: `pool_config.hash_value()` (which folds in + /// host/port/TLS/timeouts/fallback/app_name/users/startup_parameters) + /// combined with the `general.startup_parameters` hash. RELOAD + /// compares this on reuse so a SIGHUP that changed the parent pool + /// host, TLS material, or timeouts (without touching the + /// `auth_query` config itself) still rebuilds the shared pool + /// against the new parent config. + pub(crate) parent_fingerprint: u64, pool_name: String, server_host: String, server_port: u16, @@ -31,8 +47,11 @@ pub struct AuthQueryState { impl AuthQueryState { /// Create a new AuthQueryState. + #[allow(clippy::too_many_arguments)] pub(crate) fn new( config: AuthQueryConfig, + pool_startup_hash: u64, + parent_fingerprint: u64, pool_name: String, server_host: String, server_port: u16, @@ -42,6 +61,8 @@ impl AuthQueryState { Self { cache_cell: tokio::sync::OnceCell::new(), config, + pool_startup_hash, + parent_fingerprint, pool_name, server_host, server_port, @@ -100,4 +121,23 @@ impl AuthQueryState { pub fn cache_len(&self) -> usize { self.cache_cell.get().map_or(0, |c| c.len()) } + + /// Sync, non-fetching peek of the per-user startup_parameters map. + /// Returns `None` if the auth_query executor was never initialized + /// (no client has authenticated through this pool yet) or if the + /// username has no cached entry. Used on the backend-spawn hot path + /// where blocking on a PG roundtrip would defeat the point of the + /// cache; cold lookups intentionally surface as "no per-user override". + /// Pass the cached per-user startup_parameters map (when present and + /// fresh) to `f` and return its result. `f` borrows the HashMap; no + /// clone happens on the backend-spawn hot path. Returns `None` if the + /// auth_query executor hasn't been lazily initialized yet, the entry + /// is absent / negative, or the entry's TTL has elapsed. + pub fn peek_startup_parameters( + &self, + username: &str, + f: impl FnOnce(&std::collections::HashMap) -> R, + ) -> Option { + self.cache_cell.get()?.peek_startup_parameters(username, f) + } } diff --git a/src/pool/dynamic.rs b/src/pool/dynamic.rs index dc2f7fc82..1d95dfc23 100644 --- a/src/pool/dynamic.rs +++ b/src/pool/dynamic.rs @@ -28,10 +28,14 @@ use super::{ /// /// On RELOAD, dynamic pools are dropped (not in config) and recreated /// on the next client connection with fresh settings. +/// `fetched_overlay` is the per-user `startup_parameters` map from the +/// auth_query row that authenticated this user. Passing it in ties pool +/// creation to that row instead of reading the cache again. pub fn create_dynamic_pool( pool_name: &str, username: &str, backend_auth: Option, + fetched_overlay: Arc>, ) -> Result { // Fast path: pool already exists if let Some(existing) = get_pool(pool_name, username) { @@ -122,6 +126,40 @@ pub fn create_dynamic_pool( let fallback_state = super::build_fallback_state(pool_name, pool_config, &config.general); + // Merge general+pool startup_parameters baseline from the same config + // snapshot. Dynamic auth_query pools follow the same lifecycle as + // static pools: rebuilt on RELOAD when the underlying base changes + // (see `general_startup_parameters_changed` in pool/mod.rs). + let base_startup_parameters = { + let mut merged: std::collections::BTreeMap = + config.general.startup_parameters.clone(); + for (k, v) in &pool_config.startup_parameters { + merged.insert(k.clone(), v.clone()); + } + std::sync::Arc::new(merged) + }; + + // Convert the caller's HashMap snapshot into the BTreeMap shape + // ServerPool stores. The snapshot comes from the auth_query row used + // for this login, so TTL expiry or an interleaved refetch cannot + // change the overlay while the pool is created. Dedicated-mode pools + // should not reach this path, but keep the guard so a future caller + // cannot attach a per-user overlay to a shared backend pool. + let per_user_startup_overlay: std::sync::Arc> = { + let is_dedicated = super::get_auth_query_state(pool_name) + .map(|state| state.config.is_dedicated_mode()) + .unwrap_or(false); + if is_dedicated || fetched_overlay.is_empty() { + std::sync::Arc::new(std::collections::BTreeMap::new()) + } else { + let map: std::collections::BTreeMap = fetched_overlay + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); + std::sync::Arc::new(map) + } + }; + let manager = ServerPool::new( address.clone(), user.clone(), @@ -143,8 +181,16 @@ pub fn create_dynamic_pool( config.general.query_wait_timeout.as_std(), pool_mode == PoolMode::Session, fallback_state, + base_startup_parameters, + per_user_startup_overlay.clone(), ); + // Snapshot the overlay hash before the Arc moves into ServerPool. + // The auth_query cache compares the new fetched per-user map against + // this value after every refetch; a mismatch drops the dynamic pool + // so the next connect rebuilds with the new reset_val. + let overlay_hash = super::per_user_overlay_hash(per_user_startup_overlay.iter()); + let queue_strategy = match config.general.server_round_robin { true => QueueMode::Fifo, false => QueueMode::Lifo, @@ -170,6 +216,7 @@ pub fn create_dynamic_pool( database: pool, address, config_hash: 0, // dynamic pools don't participate in hash-based reload + per_user_startup_overlay_hash: overlay_hash, original_server_parameters: Arc::new(tokio::sync::Mutex::new(ServerParameters::new())), settings: PoolSettings { pool_mode, diff --git a/src/pool/fallback.rs b/src/pool/fallback.rs index 525afb4c3..982ffe96b 100644 --- a/src/pool/fallback.rs +++ b/src/pool/fallback.rs @@ -53,6 +53,12 @@ pub enum FailureReason { ServerUnavailable, /// `startup_with_timeout` deadline elapsed. Timeout, + /// PostgreSQL rejected an operator-supplied startup parameter (the + /// real SQLSTATE lives in the carried Error). Distinct from + /// StartupError because the candidate host is healthy — only the + /// operator config is wrong — so it must not enter the per-host + /// cooldown that StartupError implies. + StartupParameterRejection, /// Anything else — should normally not happen on the fallback path. Other, } @@ -64,9 +70,16 @@ impl FailureReason { FailureReason::StartupError => "startup_error", FailureReason::ServerUnavailable => "server_unavailable", FailureReason::Timeout => "timeout", + FailureReason::StartupParameterRejection => "startup_parameter_rejection", FailureReason::Other => "other", } } + + /// Whether `mark_unhealthy` should record a cooldown entry. Operator + /// config errors must not blacklist a healthy candidate. + pub fn warrants_host_cooldown(self) -> bool { + !matches!(self, FailureReason::StartupParameterRejection) + } } impl From<&Error> for FailureReason { @@ -81,6 +94,9 @@ impl From<&Error> for FailureReason { Error::ConnectError(_) => FailureReason::ConnectError, Error::ServerUnavailableError(_, _) => FailureReason::ServerUnavailable, Error::ServerStartupError(_, _) => FailureReason::StartupError, + Error::ServerStartupParameterRejection { .. } => { + FailureReason::StartupParameterRejection + } _ => FailureReason::Other, } } @@ -296,6 +312,14 @@ impl FallbackState { .with_label_values(&[self.pool_name.as_str(), reason.as_str()]) .inc(); + // Operator config errors (e.g. invalid startup_parameter) must not + // blacklist a healthy candidate — the same misconfiguration will + // fail against every host until the operator fixes the config. + // Count it in the failure metric (above), then return. + if !reason.warrants_host_cooldown() { + return; + } + let now = Instant::now(); let base = self.connect_timeout; let mut guard = self.unhealthy_candidates.lock(); @@ -887,6 +911,50 @@ mod tests { )), FailureReason::ServerUnavailable ); + assert_eq!( + FailureReason::from(&Error::ServerStartupParameterRejection { + sqlstate: "22023".into(), + message: "invalid_value".into(), + server_identifier: id.clone(), + }), + FailureReason::StartupParameterRejection + ); + } + + #[test] + fn warrants_host_cooldown_skips_only_startup_parameter_rejection() { + // The whole point of the new helper: every host-related failure + // still cools the host down, only operator-config errors do not. + assert!(FailureReason::ConnectError.warrants_host_cooldown()); + assert!(FailureReason::StartupError.warrants_host_cooldown()); + assert!(FailureReason::ServerUnavailable.warrants_host_cooldown()); + assert!(FailureReason::Timeout.warrants_host_cooldown()); + assert!(FailureReason::Other.warrants_host_cooldown()); + assert!(!FailureReason::StartupParameterRejection.warrants_host_cooldown()); + } + + #[test] + fn mark_unhealthy_skips_cooldown_for_startup_parameter_rejection() { + let state = FallbackState::new( + "test_pool_op_config_no_cooldown".to_string(), + vec![], + Duration::from_secs(10), + Duration::from_millis(50), + Duration::from_secs(2), + 30_000, + ) + .unwrap(); + state.mark_unhealthy("10.0.0.1", 5432, FailureReason::StartupParameterRejection); + // The candidate must remain eligible — the failure was the + // operator's config, not the host. + assert!( + state + .unhealthy_candidates + .lock() + .get(&("10.0.0.1".to_string(), 5432)) + .is_none(), + "operator-config rejection must not record a cooldown entry" + ); } #[test] diff --git a/src/pool/inner.rs b/src/pool/inner.rs index 5f2db1dc5..6618bac40 100644 --- a/src/pool/inner.rs +++ b/src/pool/inner.rs @@ -1545,6 +1545,25 @@ impl Pool { self.inner.server_pool.is_paused() } + /// Effective merged startup_parameters cascade keyed by parameter, with + /// the layer that contributed each winning value. Delegates to + /// `ServerPool` so admin `SHOW STARTUP_PARAMETERS` and the + /// `/api/pools` JSON share one resolver. + pub fn effective_startup_parameters_with_sources( + &self, + ) -> std::collections::BTreeMap< + String, + ( + String, + super::startup_resolver::ParameterSource, + super::startup_resolver::ApplicationState, + ), + > { + self.inner + .server_pool + .effective_startup_parameters_with_sources() + } + /// Bumps reconnect epoch and drains all idle connections. /// Returns the new epoch value. pub fn reconnect(&self) -> u32 { @@ -2081,6 +2100,8 @@ mod tests { Duration::from_secs(5), false, None, + Arc::new(std::collections::BTreeMap::new()), + Arc::new(std::collections::BTreeMap::new()), ); Pool::builder(server_pool) .coordinator(Some(coord)) @@ -2230,6 +2251,8 @@ mod tests { Duration::from_secs(5), false, None, + Arc::new(std::collections::BTreeMap::new()), + Arc::new(std::collections::BTreeMap::new()), ); let pool = Pool::builder(server_pool) .pool_name("test_db".to_string()) diff --git a/src/pool/mod.rs b/src/pool/mod.rs index 83b03d225..c96f52b6b 100644 --- a/src/pool/mod.rs +++ b/src/pool/mod.rs @@ -5,7 +5,7 @@ use once_cell::sync::{Lazy, OnceCell}; use parking_lot::{Mutex, RwLock}; use std::collections::{HashMap, HashSet}; use std::fmt::{Display, Formatter}; -use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; use std::sync::Arc; use crate::config::{ @@ -35,6 +35,7 @@ pub mod gc; pub mod pool_coordinator; pub mod retain; mod server_pool; +pub mod startup_resolver; pub mod fallback; @@ -67,6 +68,13 @@ pub type PoolMap = HashMap; /// This is atomic and safe and read-optimized. /// The pool is recreated dynamically when the config is reloaded. pub static POOLS: Lazy> = Lazy::new(|| ArcSwap::from_pointee(HashMap::default())); + +/// Hash of the previous reload's `general.startup_parameters` map. Used by +/// `ConnectionPool::from_config` to recognize when a SIGHUP changed the +/// general-level baseline so dynamic auth_query pools can be drained — the +/// per-pool reuse hash already folds in the baseline, but dynamic pools are +/// carried over by identifier rather than rebuilt from the same path. +static PREVIOUS_GENERAL_STARTUP_HASH: AtomicU64 = AtomicU64::new(0); pub static CANCELED_PIDS: Lazy>>> = Lazy::new(|| Arc::new(Mutex::new(HashSet::new()))); @@ -92,6 +100,37 @@ pub fn get_client_server_map() -> Option { CLIENT_SERVER_MAP.get().cloned() } +/// Stable hash of a per-user auth_query `startup_parameters` overlay. +/// Used to detect overlay drift after `auth_query` refetches: if the +/// new row's hash differs from `ConnectionPool::per_user_startup_overlay_hash`, +/// the dynamic pool is dropped so the next client connection rebuilds +/// against the new overlay. Accepts both `HashMap` (auth_query cache +/// shape) and `BTreeMap` (the immutable snapshot stored on the pool) +/// via a borrowed iterator, normalising key order so the hash is shape- +/// independent. +pub(crate) fn per_user_overlay_hash<'a, I>(entries: I) -> u64 +where + I: IntoIterator, +{ + use std::hash::{Hash, Hasher}; + let mut sorted: Vec<(&str, &str)> = entries + .into_iter() + .map(|(k, v)| (k.as_str(), v.as_str())) + .collect(); + sorted.sort_by(|a, b| a.0.cmp(b.0)); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + sorted.hash(&mut hasher); + hasher.finish() +} + +/// Hash that `per_user_overlay_hash` produces for the empty overlay. +/// Computed once and reused by every static / dedicated-mode pool so +/// drift comparisons against dynamic pools' real overlay hashes are +/// shape-stable across the codebase. +pub(crate) fn empty_overlay_hash() -> u64 { + per_user_overlay_hash(std::iter::empty::<(&String, &String)>()) +} + /// Build a `ServerTlsConfig` for a pool, merging pool-level overrides with general defaults. pub(crate) fn build_server_tls_for_pool( pool_config: &ConfigPool, @@ -157,6 +196,31 @@ pub fn is_dynamic_pool(id: &PoolIdentifier) -> bool { DYNAMIC_POOLS.load().contains(id) } +/// Drop a dynamic pool from `POOLS` and `DYNAMIC_POOLS`. No-op for +/// static pools — overlay drift only applies to auth_query passthrough. +/// Used by the auth_query cache after a refetch when the new per-user +/// `startup_parameters` map no longer matches the snapshot frozen in +/// the live pool: the next client connection rebuilds the dynamic pool +/// against the new overlay. +pub fn drop_dynamic_pool(id: &PoolIdentifier) -> bool { + if !is_dynamic_pool(id) { + return false; + } + let pools = POOLS.load(); + let mut new_pools = (**pools).clone(); + let removed = new_pools.remove(id).is_some(); + if removed { + POOLS.store(Arc::new(new_pools)); + } + let dynamics = DYNAMIC_POOLS.load(); + if dynamics.contains(id) { + let mut new_set = (**dynamics).clone(); + new_set.remove(id); + DYNAMIC_POOLS.store(Arc::new(new_set)); + } + removed +} + /// Get auth_query state for a database pool. pub fn get_auth_query_state(db: &str) -> Option> { AUTH_QUERY_STATE.load().get(db).cloned() @@ -250,6 +314,14 @@ pub struct ConnectionPool { /// the pool after a RELOAD command pub config_hash: u64, + /// Hash of the per-user auth_query overlay frozen into this pool at + /// creation time. After a refetch, the auth_query cache compares the + /// new per-user startup_parameters map against this value; a mismatch + /// drops the dynamic pool so the next client connection rebuilds + /// against the new overlay. Static pools and dedicated-mode shared + /// pools both pin this to the empty-map hash. + pub per_user_startup_overlay_hash: u64, + /// Cache pub prepared_statement_cache: Option, @@ -318,8 +390,39 @@ impl ConnectionPool { ); } + // Hashing each pool's effective config against (Pool, general + // startup_parameters baseline) folds general-level GUC changes into + // the same reuse decision pg_doorman already uses for pool-level + // changes. Without this, a SIGHUP that only edits + // `general.startup_parameters` would leave every idle backend + // pinned to the previous `reset_val` until the connection rotates + // through `lifetime_ms`, so clients would see mixed defaults from + // the same pool depending on which backend they got. + let general_startup_hash = { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + config.general.startup_parameters.hash(&mut hasher); + hasher.finish() + }; + // Load only; the hash is not advanced until the new pool map has + // been committed at the bottom of from_config. Otherwise a reload + // that fails halfway poisons the hash, and the next reload of the + // *same* config silently skips the recycle of dynamic pools that + // still carry the old reset_val. + let previous_general_startup_hash = PREVIOUS_GENERAL_STARTUP_HASH.load(Ordering::Relaxed); + // The static defaults to `0`, which collides with the empty-map + // hash on a fresh process; treat that special case as "no prior + // value" so the first reload never falsely claims a change. + let general_startup_parameters_changed = previous_general_startup_hash != 0 + && previous_general_startup_hash != general_startup_hash; for (pool_name, pool_config) in &config.pools { - let new_pool_hash_value = pool_config.hash_value(); + let new_pool_hash_value = { + use std::hash::Hasher; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + hasher.write_u64(pool_config.hash_value()); + hasher.write_u64(general_startup_hash); + hasher.finish() + }; let server_tls_config = build_server_tls_for_pool(pool_config, &config.general)?; // There is one pool per database/user pair. @@ -422,6 +525,25 @@ impl ConnectionPool { let fallback_state = build_fallback_state(pool_name, pool_config, &config.general); + // Merge general+pool startup_parameters from the same + // `config` snapshot we hashed above. ServerPool keeps this + // as Arc for the rest of its life — the reload + // path rebuilds the pool whenever either layer's hash + // changes, so the snapshot stays valid until then. Passing + // it in explicitly (rather than letting ServerPool::new + // call config_arc() again) closes a narrow race where a + // second reload between this iteration and constructor + // execution would write a different baseline to the pool + // than the one the reuse hash captured. + let base_startup_parameters = { + let mut merged: std::collections::BTreeMap = + config.general.startup_parameters.clone(); + for (k, v) in &pool_config.startup_parameters { + merged.insert(k.clone(), v.clone()); + } + Arc::new(merged) + }; + let manager = ServerPool::new( address.clone(), user.clone(), @@ -443,6 +565,9 @@ impl ConnectionPool { config.general.query_wait_timeout.as_std(), pool_mode == PoolMode::Session, fallback_state, + base_startup_parameters, + // Static pools carry no per-user auth_query overlay. + Arc::new(std::collections::BTreeMap::new()), ); let queue_strategy = match config.general.server_round_robin { @@ -471,6 +596,11 @@ impl ConnectionPool { database: pool, address, config_hash: new_pool_hash_value, + // Static and dedicated-mode shared pools carry no + // per-user overlay, so they pin to the empty-map + // hash. Dynamic passthrough pools set this from the + // captured overlay in dynamic.rs. + per_user_startup_overlay_hash: empty_overlay_hash(), original_server_parameters: Arc::new(tokio::sync::Mutex::new( ServerParameters::new(), )), @@ -514,9 +644,36 @@ impl ConnectionPool { for (pool_name, pool_config) in &config.pools { if let Some(ref aq_config) = pool_config.auth_query { - // RELOAD: reuse state when config unchanged (preserves cache, executor, stats) + let pool_startup_hash = { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + pool_config.startup_parameters.hash(&mut hasher); + hasher.finish() + }; + // Parent fingerprint folds every other parent input the + // dedicated shared pool depends on into one hash: + // `pool_config.hash_value()` covers host/port/TLS/ + // timeouts/fallback/app_name/users; the general + // startup hash covers the operator-wide baseline that + // also flows into the shared pool's reset_val. A SIGHUP + // that changes any of these without touching the + // auth_query config still rebuilds the shared pool. + let parent_fingerprint = pool_config.hash_value() ^ general_startup_hash; + // RELOAD: reuse state when the auth_query config AND + // the pool-level startup_parameters AND the parent + // fingerprint are unchanged. Any other parent edit + // (host/port/TLS/timeouts/fallback/app_name change, + // general.startup_parameters edit) must drop the cache + // and recycle the shared/dynamic pools: their backends + // were started with the old parent inputs as + // `reset_val` and TLS identity, and those survive + // client-side `RESET ALL` / `DISCARD ALL` unless the + // backend is recreated. if let Some(old_state) = old_aq_states_for_reuse.get(pool_name) { - if old_state.config == *aq_config { + if old_state.config == *aq_config + && old_state.pool_startup_hash == pool_startup_hash + && old_state.parent_fingerprint == parent_fingerprint + { info!("[pool: {pool_name}] auth_query config unchanged — reusing state"); auth_query_states.insert(pool_name.clone(), old_state.clone()); // Still need to ensure shared pool exists in new_pools @@ -596,6 +753,15 @@ impl ConnectionPool { let fallback_state = build_fallback_state(pool_name, pool_config, &config.general); + let base_startup_parameters = { + let mut merged: std::collections::BTreeMap = + config.general.startup_parameters.clone(); + for (k, v) in &pool_config.startup_parameters { + merged.insert(k.clone(), v.clone()); + } + Arc::new(merged) + }; + let manager = ServerPool::new( address.clone(), shared_user.clone(), @@ -617,6 +783,10 @@ impl ConnectionPool { config.general.query_wait_timeout.as_std(), pool_mode == PoolMode::Session, fallback_state, + base_startup_parameters, + // Dedicated-mode shared pool serves multiple + // dynamic users — no single per-user override. + Arc::new(std::collections::BTreeMap::new()), ); let queue_strategy = match config.general.server_round_robin { @@ -650,6 +820,11 @@ impl ConnectionPool { database: pool, address, config_hash: new_pool_hash_value, + // Static and dedicated-mode shared pools carry no + // per-user overlay, so they pin to the empty-map + // hash. Dynamic passthrough pools set this from the + // captured overlay in dynamic.rs. + per_user_startup_overlay_hash: empty_overlay_hash(), original_server_parameters: Arc::new(tokio::sync::Mutex::new( ServerParameters::new(), )), @@ -692,6 +867,8 @@ impl ConnectionPool { pool_name.clone(), Arc::new(AuthQueryState::new( aq_config.clone(), + pool_startup_hash, + parent_fingerprint, pool_name.clone(), pool_config.server_host.clone(), pool_config.server_port, @@ -706,18 +883,40 @@ impl ConnectionPool { let old_aq_states = old_aq_states_for_reuse; let mut pools_to_remove: Vec = Vec::new(); - // 1. Compare old vs new auth_query configs + // 1. Compare old vs new auth_query configs, plus pool-level + // startup_parameters: either change must drain dynamic pools + // for this pool_name so the next auth_query lookup builds + // fresh backends with the new baseline reset_val. for (pool_name, old_state) in old_aq_states.iter() { - let new_aq = config - .pools - .get(pool_name) - .and_then(|p| p.auth_query.as_ref()); - let changed = match new_aq { + let new_pool_config = config.pools.get(pool_name); + let new_aq = new_pool_config.and_then(|p| p.auth_query.as_ref()); + let aq_changed = match new_aq { None => true, // auth_query removed Some(new) => *new != old_state.config, // config changed }; - if changed { - info!("[pool: {pool_name}] auth_query config changed — collecting dynamic pools for removal"); + let new_pool_startup_hash = new_pool_config.map(|p| { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + p.startup_parameters.hash(&mut hasher); + hasher.finish() + }); + let new_parent_fingerprint = + new_pool_config.map(|p| p.hash_value() ^ general_startup_hash); + let pool_startup_changed = new_pool_startup_hash + .map(|h| h != old_state.pool_startup_hash) + .unwrap_or(false); + let parent_fingerprint_changed = new_parent_fingerprint + .map(|h| h != old_state.parent_fingerprint) + .unwrap_or(false); + if aq_changed || pool_startup_changed || parent_fingerprint_changed { + let reason = if aq_changed { + "auth_query config changed" + } else if pool_startup_changed { + "pool.startup_parameters changed" + } else { + "parent pool/general config changed" + }; + info!("[pool: {pool_name}] {reason} — collecting dynamic pools for removal"); for id in DYNAMIC_POOLS.load().iter() { if id.db == *pool_name { pools_to_remove.push(id.clone()); @@ -741,6 +940,21 @@ impl ConnectionPool { } } + // 2b. general.startup_parameters changed: drain every dynamic pool + // so the next auth_query lookup builds fresh backends with the + // new baseline. Static pools are already handled by the pool + // reuse hash above, which folds in `general_startup_hash`. + if general_startup_parameters_changed { + info!( + "general.startup_parameters changed on reload — collecting all dynamic pools for recycle" + ); + for id in DYNAMIC_POOLS.load().iter() { + if !pools_to_remove.contains(id) { + pools_to_remove.push(id.clone()); + } + } + } + // 3. Carry over surviving dynamic pools let old_pools = POOLS.load(); for id in DYNAMIC_POOLS.load().iter() { @@ -778,6 +992,11 @@ impl ConnectionPool { COORDINATORS.store(Arc::new(coordinators)); AUTH_QUERY_STATE.store(Arc::new(auth_query_states)); POOLS.store(Arc::new(new_pools.clone())); + // Advance the recycle-watcher hash only after the new state is + // published; a failure path above (Err returned via `?`) leaves + // PREVIOUS_GENERAL_STARTUP_HASH alone so the next reload still + // sees the old value and re-evaluates the change correctly. + PREVIOUS_GENERAL_STARTUP_HASH.store(general_startup_hash, Ordering::Relaxed); Ok(()) } @@ -835,6 +1054,18 @@ impl ConnectionPool { { let conn = match self.database.get().await { Ok(conn) => conn, + // PG-side rejection of an operator-supplied startup + // parameter must keep its typed shape so the cold auth + // path returns the same `ErrorResponse` (real SQLSTATE + // and PG message) to the client that the transaction + // checkout path already returns through + // src/client/transaction.rs. Stringifying the + // PoolError here collapses the carried sqlstate/message + // into a generic 58000/3D000 — which contradicts the + // "rejection forwarded verbatim" contract. + Err(PoolError::Backend(err @ Error::ServerStartupParameterRejection { .. })) => { + return Err(err); + } Err(err) => return Err(Error::ServerStartupReadParameters(err.to_string())), }; guard.set_from_hashmap(&conn.server_parameters_as_hashmap(), true); @@ -999,6 +1230,75 @@ pub fn get_coordinator(db: &str) -> Option::new(); + assert_eq!( + per_user_overlay_hash(empty_map.iter()), + empty_overlay_hash() + ); + } + + #[test] + fn per_user_overlay_hash_ignores_input_order() { + // HashMap with the same key/value pairs but inserted in different + // orders must hash identically. Without the internal sort the + // hash would depend on HashMap iteration order, which is + // randomized per process and would falsely flag overlay drift on + // every refetch. + let mut a = std::collections::HashMap::new(); + a.insert("work_mem".to_string(), "64MB".to_string()); + a.insert("statement_timeout".to_string(), "30s".to_string()); + let mut b = std::collections::HashMap::new(); + b.insert("statement_timeout".to_string(), "30s".to_string()); + b.insert("work_mem".to_string(), "64MB".to_string()); + assert_eq!( + per_user_overlay_hash(a.iter()), + per_user_overlay_hash(b.iter()) + ); + } + + #[test] + fn per_user_overlay_hash_changes_when_value_changes() { + let mut a = std::collections::HashMap::new(); + a.insert("work_mem".to_string(), "64MB".to_string()); + let mut b = std::collections::HashMap::new(); + b.insert("work_mem".to_string(), "128MB".to_string()); + assert_ne!( + per_user_overlay_hash(a.iter()), + per_user_overlay_hash(b.iter()) + ); + } + + #[test] + fn per_user_overlay_hash_changes_when_key_added() { + let mut a = std::collections::HashMap::new(); + a.insert("work_mem".to_string(), "64MB".to_string()); + let mut b = a.clone(); + b.insert("statement_timeout".to_string(), "30s".to_string()); + assert_ne!( + per_user_overlay_hash(a.iter()), + per_user_overlay_hash(b.iter()) + ); + } + + #[test] + fn per_user_overlay_hash_matches_across_hashmap_and_btreemap() { + // The auth_query cache stores HashMap; the pool freezes a + // BTreeMap snapshot. Drift detection compares the two — they + // must hash to the same value for identical content. + let mut h = std::collections::HashMap::new(); + h.insert("work_mem".to_string(), "64MB".to_string()); + let mut b = std::collections::BTreeMap::new(); + b.insert("work_mem".to_string(), "64MB".to_string()); + assert_eq!( + per_user_overlay_hash(h.iter()), + per_user_overlay_hash(b.iter()) + ); + } + // --- compute_spare tests --- #[test] diff --git a/src/pool/retain.rs b/src/pool/retain.rs index 3cf77863a..995681f1a 100644 --- a/src/pool/retain.rs +++ b/src/pool/retain.rs @@ -322,6 +322,8 @@ mod tests { Duration::from_secs(5), false, None, + Arc::new(std::collections::BTreeMap::new()), + Arc::new(std::collections::BTreeMap::new()), ); let database = Pool::builder(server_pool) .pool_name("test_db".to_string()) @@ -342,6 +344,7 @@ mod tests { min_guaranteed_pool_size: 0, }, config_hash: 0, + per_user_startup_overlay_hash: crate::pool::empty_overlay_hash(), prepared_statement_cache: None, coordinator: None, replenish_failures: Arc::new(AtomicU32::new(0)), diff --git a/src/pool/server_pool.rs b/src/pool/server_pool.rs index 55772b798..5f8ae3d9d 100644 --- a/src/pool/server_pool.rs +++ b/src/pool/server_pool.rs @@ -4,6 +4,7 @@ //! server connections. It handles connect timeouts, lifetime checks, alive //! checks, pause/resume, and reconnect epoch management. +use std::collections::BTreeMap; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use std::time::Duration; @@ -11,6 +12,7 @@ use std::time::Duration; use log::{debug, info, warn}; use tokio::sync::{Notify, Semaphore}; +use crate::config::startup_parameters as sp; use crate::config::{Address, User}; use crate::errors::Error; use crate::patroni::types::Role; @@ -19,9 +21,44 @@ use crate::stats::ServerStats; use crate::utils::format_duration_ms; use super::errors::{RecycleError, RecycleResult}; +use super::startup_resolver::ApplicationState; use super::types::Metrics; use super::ClientServerMap; +/// Decision returned by `ServerPool::classify_startup_parameters`. Used +/// by the spawn path to drive counter/log side effects and by the +/// read-only admin/API view to label each entry without touching the +/// metric. Carries the packet/body byte counts so the caller can log +/// them without recomputing. +#[derive(Debug, Clone, Copy)] +enum BudgetDecision { + FullCascade, + OverlayDroppedBaselineKept { + body_bytes: usize, + packet_bytes: usize, + }, + EmptyDueToBudget { + reason: BudgetReason, + body_bytes: usize, + packet_bytes: usize, + }, +} + +#[derive(Debug, Clone, Copy)] +enum BudgetReason { + CascadeBudgetExceeded, + PacketCapExceeded, +} + +impl BudgetReason { + fn as_str(self) -> &'static str { + match self { + BudgetReason::CascadeBudgetExceeded => "cascade_budget_exceeded", + BudgetReason::PacketCapExceeded => "packet_cap_exceeded", + } + } +} + /// Wrapper for the connection pool. pub struct ServerPool { /// Server address. @@ -83,6 +120,27 @@ pub struct ServerPool { /// Notify to wake up clients blocked on PAUSE. resume_notify: Notify, + + /// `general.startup_parameters` merged with this pool's + /// `pool.startup_parameters` — the baseline that every backend spawn + /// from this pool ships in `StartupMessage`, before the optional + /// per-user auth_query overlay is applied. Cached once at pool + /// construction: the reload path rebuilds the pool whenever either + /// layer's hash changes (see `ConnectionPool::from_config`), so this + /// view is immutable for the lifetime of the pool object. Shared as + /// `Arc` so backend spawns can borrow it without per-call cloning. + base_startup_parameters: Arc>, + + /// Per-user auth_query overlay captured at pool construction. Dynamic + /// passthrough pools populate this from a fresh `cache.get_or_fetch` + /// snapshot taken right after auth, so every backend spawn from this + /// pool sees the same overlay even when the auth_query cache TTL has + /// since expired. Empty for static pools and for the dedicated-mode + /// shared pool (which intentionally has no per-user override). + /// Refetches that change the overlay are handled by the reload / + /// drain logic in `pool/mod.rs`; this field is immutable for the + /// lifetime of the pool object. + per_user_startup_overlay: Arc>, } impl std::fmt::Debug for ServerPool { @@ -128,6 +186,8 @@ impl ServerPool { query_wait_timeout: Duration, session_mode: bool, fallback_state: Option>, + base_startup_parameters: Arc>, + per_user_startup_overlay: Arc>, ) -> ServerPool { ServerPool { address, @@ -149,6 +209,8 @@ impl ServerPool { resume_notify: Notify::new(), session_mode, fallback_state, + base_startup_parameters, + per_user_startup_overlay, } } @@ -217,6 +279,12 @@ impl ServerPool { stats.register(stats.clone()); + // Resolve once for this spawn attempt: the plain attempt and the + // optional sslmode=allow TLS retry must see the same parameter set, + // otherwise a config RELOAD landing between the two would silently + // ship different StartupMessages for the same client request. + let startup_parameters = self.resolved_startup_parameters(); + let result = startup_with_timeout( self.connect_timeout, &self.address.host, @@ -232,22 +300,24 @@ impl ServerPool { self.prepared_statement_cache_size, self.application_name.clone(), self.session_mode, + &startup_parameters, ), ) .await; // libpq sslmode=allow: PostgreSQL has no protocol-level "TLS required" - // signal — pg_hba rejects plain connections via FATAL 28000 only after - // StartupMessage. The socket is dead after FATAL, so retry needs a fresh - // TCP connection. We retry on any startup failure (matching libpq), but - // skip retry on transport-level errors (ConnectError, ServerUnavailableError) - // since TLS cannot help when the server was never reached. + // signal. pg_hba rejects plain connections with FATAL 28000 after the + // StartupMessage, and the socket cannot be reused after that. Match + // libpq by retrying startup failures over TLS, but skip transport + // failures where no PostgreSQL startup response was received. // // Reference: PostgreSQL docs, "SSL Support" → sslmode parameter. let should_tls_retry = match &result { Err(err) if self.address.server_tls.mode.retries_with_tls() => !matches!( err, - Error::ConnectError(_) | Error::ServerUnavailableError(_, _) + Error::ConnectError(_) + | Error::ServerUnavailableError(_, _) + | Error::ServerStartupParameterRejection { .. } ), _ => false, }; @@ -287,6 +357,7 @@ impl ServerPool { self.prepared_statement_cache_size, self.application_name.clone(), self.session_mode, + &startup_parameters, ), ) .await; @@ -329,6 +400,248 @@ impl ServerPool { &self.address } + /// Return the effective startup parameter cascade with the winning + /// source layer **and** the application state for each key. Used by + /// `SHOW STARTUP_PARAMETERS` and `/api/pools`. + /// + /// The cascade view comes from the live config and the auth_query + /// cache, so it reflects what the operator just edited. The + /// application state cross-checks each key against + /// `resolved_startup_parameters()` — the same wire-ready map that + /// `Server::startup` will ship for the next backend spawn — so the + /// admin view can flag keys that look configured but will not + /// actually leave the wire: + /// + /// * `Applied` — same key/value in both views; the next spawn + /// ships it. + /// * `DroppedDueToBudget` — the wire map omits the key. The runtime + /// budget/packet check dropped the operator cascade (or the + /// overlay) on the most recent spawn; same will happen on the + /// next one until the operator shrinks the config. + /// * `Stale` — the key is in the wire map but with a different + /// value, which means the frozen `base_startup_parameters` / + /// `per_user_startup_overlay` Arc on this pool was captured + /// before the operator's latest edit. RELOAD has not yet + /// recycled the pool (general/pool change) or the auth_query + /// cache has not refetched (per-user change). The next spawn + /// ships the stale value, not the configured one. + pub fn effective_startup_parameters_with_sources( + &self, + ) -> std::collections::BTreeMap< + String, + ( + String, + super::startup_resolver::ParameterSource, + ApplicationState, + ), + > { + let cfg = crate::config::config_arc(); + let pool_params = cfg + .pools + .get(&self.address.pool_name) + .map(|p| &p.startup_parameters) + .cloned() + .unwrap_or_default(); + let auth_query_params: Option> = + match super::get_auth_query_state(&self.address.pool_name) { + Some(state) if !state.config.is_dedicated_mode() => { + state.peek_startup_parameters(&self.user.username, |m| m.clone()) + } + _ => None, + }; + let configured = super::startup_resolver::resolve_with_sources( + &cfg.general.startup_parameters, + &pool_params, + auth_query_params.as_ref(), + ); + // Use the pure classifier so this admin/API path does not + // increment STARTUP_PARAMETERS_DROPPED_TOTAL or emit warn logs + // — both are side effects of the spawn path + // (resolved_startup_parameters). SHOW polling and /api/pools + // page refreshes are safe to call repeatedly. + let (wire_cow, _decision) = self.classify_startup_parameters(); + let wire = wire_cow.as_ref(); + let mut out: std::collections::BTreeMap< + String, + ( + String, + super::startup_resolver::ParameterSource, + ApplicationState, + ), + > = configured + .into_iter() + .map(|(k, (v, src))| { + let state = match wire.get(&k) { + Some(wire_v) if wire_v == &v => ApplicationState::Applied, + Some(_) => ApplicationState::Stale, + None => ApplicationState::DroppedDueToBudget, + }; + (k, (v, src, state)) + }) + .collect(); + // Surface wire-only keys: a key that the live config no longer + // mentions but that the pool's frozen baseline / overlay still + // ships is invisible without this loop. The operator needs to + // see that "I deleted plan_cache_mode from pool.startup_parameters + // but my backends are still getting force_custom_plan" — that is + // exactly the stale snapshot RELOAD has not yet recycled (or that + // the auth_query cache has not yet refetched). + for (k, wire_v) in wire { + if !out.contains_key(k) { + let frozen_source = if self.per_user_startup_overlay.contains_key(k) { + super::startup_resolver::ParameterSource::AuthQuery + } else { + super::startup_resolver::ParameterSource::Pool + }; + out.insert( + k.clone(), + (wire_v.clone(), frozen_source, ApplicationState::Stale), + ); + } + } + out + } + + /// Resolve the operator-supplied startup_parameters map that this pool + /// Pure classifier shared between the spawn path and the read-only + /// admin/API views. Returns the wire-ready map and the budget + /// decision the runtime would make for this spawn, **without** + /// touching `STARTUP_PARAMETERS_DROPPED_TOTAL` or emitting warn + /// logs. The spawn-side `resolved_startup_parameters` wraps this + /// with the counter + log; admin/API callers call this directly so + /// `SHOW STARTUP_PARAMETERS` polling cannot inflate the drop counter + /// or spam the warn log. + fn classify_startup_parameters( + &self, + ) -> ( + std::borrow::Cow<'_, BTreeMap>, + BudgetDecision, + ) { + let merged: std::borrow::Cow<'_, BTreeMap> = + if self.per_user_startup_overlay.is_empty() { + std::borrow::Cow::Borrowed(&*self.base_startup_parameters) + } else { + let mut owned = (*self.base_startup_parameters).clone(); + for (k, v) in self.per_user_startup_overlay.iter() { + owned.insert(k.clone(), v.clone()); + } + std::borrow::Cow::Owned(owned) + }; + + let username_for_wire = self + .user + .server_username + .as_deref() + .unwrap_or(self.user.username.as_str()); + + let (packet_bytes, body_bytes) = sp::packet_and_body_bytes( + username_for_wire, + &self.database, + &self.application_name, + &merged, + ); + + let over_budget = body_bytes > sp::MAX_OPERATOR_BUDGET; + let over_packet = packet_bytes > sp::MAX_STARTUP_PACKET_SIZE; + if !over_budget && !over_packet { + return (merged, BudgetDecision::FullCascade); + } + + // The merged cascade is over a limit. If we have an overlay, try + // dropping just the overlay and keep the baseline. + let has_overlay = !self.per_user_startup_overlay.is_empty(); + if has_overlay { + let baseline = &*self.base_startup_parameters; + let (baseline_packet, baseline_body) = sp::packet_and_body_bytes( + username_for_wire, + &self.database, + &self.application_name, + baseline, + ); + if baseline_body <= sp::MAX_OPERATOR_BUDGET + && baseline_packet <= sp::MAX_STARTUP_PACKET_SIZE + { + return ( + std::borrow::Cow::Borrowed(baseline), + BudgetDecision::OverlayDroppedBaselineKept { + body_bytes, + packet_bytes, + }, + ); + } + } + + let reason = if over_packet { + BudgetReason::PacketCapExceeded + } else { + BudgetReason::CascadeBudgetExceeded + }; + ( + std::borrow::Cow::Owned(BTreeMap::new()), + BudgetDecision::EmptyDueToBudget { + reason, + body_bytes, + packet_bytes, + }, + ) + } + + /// will hand to `Server::startup` for one backend spawn. + /// + /// Without a per-user auth_query overlay, this borrows the cached base + /// map. With an overlay, it clones the base map and applies the user + /// values. + /// + /// The merged map is checked against the operator budget and the full + /// PostgreSQL startup-packet limit. On overflow, pg_doorman logs and sends + /// no operator-supplied parameters for this backend startup. + fn resolved_startup_parameters(&self) -> std::borrow::Cow<'_, BTreeMap> { + let (map, decision) = self.classify_startup_parameters(); + match decision { + BudgetDecision::FullCascade => map, + BudgetDecision::OverlayDroppedBaselineKept { + body_bytes, + packet_bytes, + } => { + warn!( + "[{}@{}] auth_query per-user startup_parameters pushes the cascade \ + over the operator budget (merged {} bytes, packet {} bytes); \ + dropping the per-user overlay and keeping the general/pool \ + baseline for this backend spawn", + self.user.username, self.address.pool_name, body_bytes, packet_bytes, + ); + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[ + self.address.pool_name.as_str(), + "auth_query_overlay_oversize", + ]) + .inc(); + map + } + BudgetDecision::EmptyDueToBudget { + reason, + body_bytes, + packet_bytes, + } => { + warn!( + "[{}@{}] effective startup_parameters serialize to {} bytes \ + (packet {} bytes), exceeding operator budget {} / PG cap {}; \ + all operator-supplied parameters dropped for this backend spawn", + self.user.username, + self.address.pool_name, + body_bytes, + packet_bytes, + sp::MAX_OPERATOR_BUDGET, + sp::MAX_STARTUP_PACKET_SIZE, + ); + crate::web::metrics::STARTUP_PARAMETERS_DROPPED_TOTAL + .with_label_values(&[self.address.pool_name.as_str(), reason.as_str()]) + .inc(); + map + } + } + } + /// Establish a fallback connection by iterating through Patroni-discovered /// candidates. Per-candidate failures (auth error, "database is starting up", /// startup timeout, etc.) mark the candidate unhealthy and proceed to the @@ -381,8 +694,8 @@ impl ServerPool { (Ok(conn), _) => Ok(conn), (Err(err), super::fallback::TargetSource::WhitelistCache) => { // Cached host was stale; wipe it and try with full discovery - // exactly once more. Bounded retry — discovery round failure - // surfaces directly without a third try. + // exactly once more. If discovery fails too, return that + // failure without a third attempt. info!( "[{}@{}] fallback: whitelist round failed ({err}), retrying with fresh discovery", self.address.username, self.address.pool_name, @@ -451,6 +764,15 @@ impl ServerPool { } }; + // Resolve the startup_parameters cascade once for the whole + // fallback round. Without this, a wave of N candidates would + // do N×{auth_query peek, BTreeMap clone, validation walk} + // for one client checkout — and the merge result is host- + // independent, so the per-candidate work was pure waste. + // `try_fallback_target` borrows this map for both the plain + // attempt and the optional sslmode=allow TLS retry. + let startup_parameters_round = self.resolved_startup_parameters(); + // Whitelist-cache hit: single target, race-of-one is just a startup. if matches!(source, super::fallback::TargetSource::WhitelistCache) { let target = match targets.into_iter().next() { @@ -475,7 +797,10 @@ impl ServerPool { crate::web::metrics::FALLBACK_CONNECTIONS_TOTAL .with_label_values(&[&self.address.pool_name]) .inc(); - return match self.try_fallback_target(&target).await { + return match self + .try_fallback_target(&target, &startup_parameters_round) + .await + { Ok(server) => { fallback.set_whitelisted(target.host, target.port, target.role); (Ok(server), source) @@ -506,7 +831,13 @@ impl ServerPool { format_target_list(&sync_targets), ); if let Some(server) = self - .race_wave(fallback, &sync_targets, &mut summary, source) + .race_wave( + fallback, + &sync_targets, + &mut summary, + source, + &startup_parameters_round, + ) .await { return (Ok(server), source); @@ -534,7 +865,13 @@ impl ServerPool { format_target_list(&other_targets), ); if let Some(server) = self - .race_wave(fallback, &other_targets, &mut summary, source) + .race_wave( + fallback, + &other_targets, + &mut summary, + source, + &startup_parameters_round, + ) .await { return (Ok(server), source); @@ -546,6 +883,17 @@ impl ServerPool { "[{}@{}] fallback: all fallback candidates rejected ({summary_str})", self.address.username, self.address.pool_name, ); + // If every candidate failed solely on operator-supplied startup + // parameter rejection, surface PG's actual sqlstate/message so the + // client gets the real error instead of a generic 53300. Healthy + // hosts are not blacklisted (mark_unhealthy skips this category), + // so the same misconfiguration will keep failing until the + // operator fixes the config. + if summary.all_startup_parameter_rejection() { + if let Some(err) = summary.into_last_err() { + return (Err(err), source); + } + } ( Err(Error::ConnectError(format!( "all fallback candidates rejected ({summary_str})" @@ -557,14 +905,15 @@ impl ServerPool { /// Race `Server::startup` against `targets` in parallel. On first Ok /// return `Some(server)` (winner is whitelisted as a side effect). On /// full exhaustion mark every loser unhealthy, record reasons into - /// `summary`, and return `None` — the caller advances to the next wave - /// or surfaces the aggregate. + /// `summary`, and return `None`; the caller advances to the next wave or + /// returns the aggregate error. async fn race_wave( &self, fallback: &super::fallback::FallbackState, targets: &[super::fallback::FallbackTarget], summary: &mut FailureSummary, source: super::fallback::TargetSource, + startup_parameters: &BTreeMap, ) -> Option { // We only count "we attempted to use fallback" once per wave, on // entry — not per candidate. The metric measures fallback usage @@ -577,7 +926,7 @@ impl ServerPool { let futures: Vec>> = targets .iter() - .map(|t| Box::pin(self.try_fallback_target(t)) as _) + .map(|t| Box::pin(self.try_fallback_target(t, startup_parameters)) as _) .collect(); match race_first_success(futures).await { @@ -631,6 +980,7 @@ impl ServerPool { async fn try_fallback_target( &self, target: &super::fallback::FallbackTarget, + startup_parameters: &BTreeMap, ) -> Result { // Use the fallback_connect_timeout for fallback startup deadlines — // the same scale as the TCP-probe and per-candidate cooldown window. @@ -650,6 +1000,13 @@ impl ServerPool { )); stats.register(stats.clone()); + // `startup_parameters` is resolved once per fallback round by + // the caller (`run_fallback_round` / `race_wave`), so a wave + // of N candidates does N×0 cascade resolves and merges instead + // of N×1. The fallback target's pool name matches `self`, so + // the per-pool cascade still applies even though we are talking + // to a different physical host than `self.address.host`. + let result = startup_with_timeout( fallback_timeout, &fallback_address.host, @@ -665,6 +1022,7 @@ impl ServerPool { self.prepared_statement_cache_size, self.application_name.clone(), self.session_mode, + startup_parameters, ), ) .await; @@ -674,7 +1032,9 @@ impl ServerPool { let should_tls_retry = match &result { Err(err) if fallback_address.server_tls.mode.retries_with_tls() => !matches!( err, - Error::ConnectError(_) | Error::ServerUnavailableError(_, _) + Error::ConnectError(_) + | Error::ServerUnavailableError(_, _) + | Error::ServerStartupParameterRejection { .. } ), _ => false, }; @@ -713,6 +1073,7 @@ impl ServerPool { self.prepared_statement_cache_size, self.application_name.clone(), self.session_mode, + startup_parameters, ), ) .await; @@ -897,6 +1258,22 @@ impl FailureSummary { self.last_err = Some(err); } + /// True when the recorded failures are non-empty and contain only + /// `StartupParameterRejection`. Used to decide whether to surface the + /// original PG error to the client instead of the aggregate + /// "all candidates rejected" wrapper. + fn all_startup_parameter_rejection(&self) -> bool { + !self.counts.is_empty() + && self + .counts + .keys() + .all(|r| matches!(r, super::fallback::FailureReason::StartupParameterRejection)) + } + + fn into_last_err(self) -> Option { + self.last_err + } + fn format(&self) -> String { if self.counts.is_empty() { return "no candidates".to_string(); @@ -915,12 +1292,12 @@ impl FailureSummary { /// Race `futures` and return the first `Ok`, together with its index in the /// input slice. If every future yields `Err`, return all errors with their -/// original indices — the caller decides how to surface them (per-host -/// cooldown, log aggregation). Pending futures are dropped on first +/// original indices; the caller decides how to use them for per-host cooldown +/// and log aggregation. Pending futures are dropped on first /// success, which cancels the in-flight `Server::startup` for the losing /// candidates: their TCP sockets go away under us; the kernel finishes the -/// half-open handshake asynchronously. This is intentional — the -/// user-facing requirement is "first successful sync wins", and chasing +/// half-open handshake asynchronously. The user-facing requirement is +/// "first successful sync wins", and chasing /// graceful disconnect on every loser would gate the winner on the slowest /// loser. async fn race_first_success<'a, T: 'a, E: 'a>( @@ -1007,11 +1384,85 @@ mod tests { Metrics::new(lifetime_ms, 0, 0) } + fn rejection_err() -> Error { + Error::ServerStartupParameterRejection { + sqlstate: "22023".to_string(), + message: "invalid_value".to_string(), + server_identifier: crate::app::errors::ServerIdentifier::new( + "alice".to_string(), + "db", + "pool_a", + ), + } + } + + fn timeout_err() -> Error { + Error::ConnectError("server startup timed out after 5s".to_string()) + } + + #[test] + fn all_startup_parameter_rejection_false_when_empty() { + let s = FailureSummary::default(); + assert!( + !s.all_startup_parameter_rejection(), + "empty summary must not claim everyone rejected on startup parameter" + ); + } + + #[test] + fn all_startup_parameter_rejection_true_when_only_rejections() { + let mut s = FailureSummary::default(); + s.record( + rejection_err(), + super::super::fallback::FailureReason::StartupParameterRejection, + ); + s.record( + rejection_err(), + super::super::fallback::FailureReason::StartupParameterRejection, + ); + assert!(s.all_startup_parameter_rejection()); + } + + #[test] + fn all_startup_parameter_rejection_false_when_mixed() { + let mut s = FailureSummary::default(); + s.record( + rejection_err(), + super::super::fallback::FailureReason::StartupParameterRejection, + ); + s.record( + timeout_err(), + super::super::fallback::FailureReason::Timeout, + ); + assert!( + !s.all_startup_parameter_rejection(), + "a single non-rejection cause must veto the shortcut" + ); + } + + #[test] + fn into_last_err_returns_most_recently_recorded() { + let mut s = FailureSummary::default(); + s.record( + timeout_err(), + super::super::fallback::FailureReason::Timeout, + ); + s.record( + rejection_err(), + super::super::fallback::FailureReason::StartupParameterRejection, + ); + match s.into_last_err() { + Some(Error::ServerStartupParameterRejection { sqlstate, .. }) => { + assert_eq!(sqlstate, "22023") + } + other => panic!("expected the last-recorded rejection, got {other:?}"), + } + } + #[test] fn lifetime_exceeded_skipped_when_under_pressure() { // A connection well past its budget is kept alive when the caller - // signals pressure. This is the whole point of the new flag: a - // working connection must not be closed mid-storm. + // signals pressure. A working connection must not be closed mid-storm. let metrics = metrics_with_lifetime(1); thread::sleep(Duration::from_millis(5)); assert!(lifetime_exceeded(&metrics, true).is_none()); @@ -1048,9 +1499,9 @@ mod tests { async fn startup_with_timeout_returns_connect_error_on_deadline() { // Simulates a server that opened TCP but never replies to // StartupMessage: the inner future never resolves. We expect - // `startup_with_timeout` to surface this as `ConnectError`, which is - // what callers treat as a transport-level failure (triggers fallback - // on the main path; marks the candidate unhealthy on the fallback path). + // `startup_with_timeout` to return `ConnectError`, which callers treat + // as a transport-level failure (triggers fallback on the main path; + // marks the candidate unhealthy on the fallback path). let pending = std::future::pending::>(); let result = startup_with_timeout(Duration::from_millis(20), "1.2.3.4", 5432, pending).await; diff --git a/src/pool/startup_resolver.rs b/src/pool/startup_resolver.rs new file mode 100644 index 000000000..895da0408 --- /dev/null +++ b/src/pool/startup_resolver.rs @@ -0,0 +1,200 @@ +//! Pure cascade resolver for operator-supplied PostgreSQL startup parameters. +//! +//! Three levels merge by union; per-key, the more specific level wins +//! (auth_query > pool > general). The result is what pg_doorman sends in +//! `StartupMessage` for one backend connection. + +use std::collections::{BTreeMap, HashMap}; + +/// Merge cascade and return the map pg_doorman will put on the wire. +/// +/// `auth_query_params` is `None` for connections that don't go through +/// `auth_query` (static user), and also for dedicated-mode auth_query pools +/// where one shared backend serves multiple dynamic users so per-user +/// parameters cannot be honoured. +/// +/// The production hot path goes through +/// [`ServerPool::resolved_startup_parameters`] using a cached +/// `Arc` for the general+pool base; this function is the +/// pure-cascade variant kept as the canonical reference of the merge +/// rule and exercised by unit tests. +#[allow(dead_code)] +pub fn resolve( + general: &BTreeMap, + pool: &BTreeMap, + auth_query_params: Option<&HashMap>, +) -> BTreeMap { + let mut merged: BTreeMap = BTreeMap::new(); + merged.extend(general.iter().map(|(k, v)| (k.clone(), v.clone()))); + merged.extend(pool.iter().map(|(k, v)| (k.clone(), v.clone()))); + if let Some(extra) = auth_query_params { + merged.extend(extra.iter().map(|(k, v)| (k.clone(), v.clone()))); + } + merged +} + +/// Layer in the cascade that contributed the winning value for a key. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ParameterSource { + General, + Pool, + AuthQuery, +} + +impl ParameterSource { + pub fn as_str(self) -> &'static str { + match self { + ParameterSource::General => "general", + ParameterSource::Pool => "pool", + ParameterSource::AuthQuery => "auth_query", + } + } +} + +/// Wire-application state for an entry returned by +/// `ServerPool::effective_startup_parameters_with_sources`. Lets the +/// admin/Web UI flag entries that the operator configured but the +/// runtime will not actually ship in `StartupMessage`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ApplicationState { + /// Configured value matches the wire-ready map; the next backend + /// spawn will ship this key/value. + Applied, + /// Runtime dropped the key (operator cascade exceeded the budget + /// or the packet cap on the most recent backend spawn — the + /// `*_dropped_total` counter ticked on the same spawn). + DroppedDueToBudget, + /// Wire map ships a different value than the live config has. The + /// pool's frozen baseline / overlay snapshot is stale — RELOAD or + /// auth_query cache refetch has not yet recycled this pool. + Stale, +} + +impl ApplicationState { + pub fn as_str(self) -> &'static str { + match self { + ApplicationState::Applied => "applied", + ApplicationState::DroppedDueToBudget => "dropped_due_to_budget", + ApplicationState::Stale => "stale", + } + } +} + +/// Same cascade as [`resolve`], but carries the layer that contributed each +/// winning value. Used by `SHOW STARTUP_PARAMETERS` and `/api/pools` so an +/// operator can see "this `work_mem` came from the pool, that `lock_timeout` +/// from auth_query" without re-reading config plus the auth_query cache. +pub fn resolve_with_sources( + general: &BTreeMap, + pool: &BTreeMap, + auth_query_params: Option<&HashMap>, +) -> BTreeMap { + let mut out: BTreeMap = BTreeMap::new(); + for (k, v) in general { + out.insert(k.clone(), (v.clone(), ParameterSource::General)); + } + for (k, v) in pool { + out.insert(k.clone(), (v.clone(), ParameterSource::Pool)); + } + if let Some(extra) = auth_query_params { + for (k, v) in extra { + out.insert(k.clone(), (v.clone(), ParameterSource::AuthQuery)); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn b(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + fn h(pairs: &[(&str, &str)]) -> HashMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + #[test] + fn empty_cascade_yields_empty() { + let r = resolve(&BTreeMap::new(), &BTreeMap::new(), None); + assert!(r.is_empty()); + } + + #[test] + fn general_baseline_passes_through() { + let g = b(&[("statement_timeout", "10s")]); + let r = resolve(&g, &BTreeMap::new(), None); + assert_eq!(r.get("statement_timeout").map(String::as_str), Some("10s")); + } + + #[test] + fn pool_overrides_general_per_key() { + let g = b(&[("plan_cache_mode", "auto"), ("statement_timeout", "10s")]); + let p = b(&[("plan_cache_mode", "force_custom_plan")]); + let r = resolve(&g, &p, None); + assert_eq!(r.get("plan_cache_mode").unwrap(), "force_custom_plan"); + assert_eq!(r.get("statement_timeout").unwrap(), "10s"); + } + + #[test] + fn auth_query_overrides_pool() { + let p = b(&[("work_mem", "64MB")]); + let a = h(&[("work_mem", "256MB"), ("lock_timeout", "5s")]); + let r = resolve(&BTreeMap::new(), &p, Some(&a)); + assert_eq!(r.get("work_mem").unwrap(), "256MB"); + assert_eq!(r.get("lock_timeout").unwrap(), "5s"); + } + + #[test] + fn dedicated_mode_signaled_by_none_auth_query() { + let p = b(&[("work_mem", "64MB")]); + let r = resolve(&BTreeMap::new(), &p, None); + assert_eq!(r.get("work_mem").unwrap(), "64MB"); + assert!(!r.contains_key("lock_timeout")); + } + + #[test] + fn resolve_with_sources_attributes_each_layer() { + let g = b(&[("statement_timeout", "10s"), ("plan_cache_mode", "auto")]); + let p = b(&[ + ("plan_cache_mode", "force_custom_plan"), + ("work_mem", "64MB"), + ]); + let a = h(&[("work_mem", "256MB"), ("lock_timeout", "5s")]); + let r = resolve_with_sources(&g, &p, Some(&a)); + assert_eq!( + r.get("statement_timeout"), + Some(&("10s".to_string(), ParameterSource::General)) + ); + assert_eq!( + r.get("plan_cache_mode"), + Some(&("force_custom_plan".to_string(), ParameterSource::Pool)) + ); + assert_eq!( + r.get("work_mem"), + Some(&("256MB".to_string(), ParameterSource::AuthQuery)) + ); + assert_eq!( + r.get("lock_timeout"), + Some(&("5s".to_string(), ParameterSource::AuthQuery)) + ); + } + + #[test] + fn application_name_can_cascade_too() { + // operator-wins on application_name extends through the cascade: + // pool overrides general's baseline; auth_query overrides pool. + let g = b(&[("application_name", "tier-default")]); + let p = b(&[("application_name", "checkout-pool")]); + let a = h(&[("application_name", "vip-user-app")]); + let r = resolve(&g, &p, Some(&a)); + assert_eq!(r.get("application_name").unwrap(), "vip-user-app"); + } +} diff --git a/src/server/parameters.rs b/src/server/parameters.rs index e6feaf95e..b5f98318e 100644 --- a/src/server/parameters.rs +++ b/src/server/parameters.rs @@ -14,6 +14,24 @@ static TRACKED_PARAMETERS: Lazy> = Lazy::new(|| { set }); +/// Canonicalise a PostgreSQL session parameter name so that startup-time +/// lowercase forms (`timezone`, `datestyle`) match the +/// `ParameterStatus` casing PG sends back (`TimeZone`, `DateStyle`). +/// Used both by `ServerParameters::set_param` (where it has lived since +/// day one) and by `Server::startup` when it captures the operator- +/// managed key set: without the canonical form, `sync_parameters` +/// filters by exact-string match and a client startup value reported +/// as `TimeZone` would overwrite an operator value set as `timezone`. +pub fn canonicalize_param_name(key: String) -> String { + if key == "timezone" { + "TimeZone".to_string() + } else if key == "datestyle" { + "DateStyle".to_string() + } else { + key + } +} + #[derive(Debug, Clone)] pub struct ServerParameters { // Kept `pub(crate)` to preserve current internal usage patterns during refactor. @@ -57,16 +75,9 @@ impl ServerParameters { /// If `startup` is false, then only tracked parameters will be set. pub fn set_param(&mut self, key: impl Into, value: impl Into, startup: bool) { - let mut key = key.into(); + let key = canonicalize_param_name(key.into()); let value = value.into(); - // Startup parameters may come uncapitalized, while ParameterStatus uses canonical keys. - if key == "timezone" { - key = "TimeZone".to_string(); - } else if key == "datestyle" { - key = "DateStyle".to_string(); - }; - if TRACKED_PARAMETERS.contains(&key) || startup { self.parameters.insert(key, value); } diff --git a/src/server/server_backend.rs b/src/server/server_backend.rs index 2ee998ab9..344bbd0aa 100644 --- a/src/server/server_backend.rs +++ b/src/server/server_backend.rs @@ -1,7 +1,7 @@ // Implementation of the PostgreSQL server (database) protocol. // Standard library imports -use std::collections::{HashMap, VecDeque}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::num::NonZeroUsize; use std::string::ToString; use std::sync::Arc; @@ -27,7 +27,6 @@ use crate::stats::ServerStats; use super::authentication::handle_authentication; use super::cleanup::CleanupState; use super::parameters::ServerParameters; -use super::startup_error::handle_startup_error; use super::stream::{create_tcp_stream_inner, create_unix_stream_inner, StreamInner}; use super::{prepared_statements, protocol_io, startup_cancel}; @@ -164,6 +163,23 @@ pub struct Server { /// Per-connection lifetime override (ms). Set on fallback connections so /// they expire before the local backend recovers. pub(crate) override_lifetime_ms: Option, + + /// Names of GUCs that pg_doorman injected through `startup_parameters` + /// for this backend. They become `pg_settings.reset_val` for the + /// session, so `sync_parameters` must not push a client-side value over + /// them on checkout — the operator decision wins over the client. + /// `Arc` because every spawn from the same pool sees the same set; + /// without the share each backend allocated its own `HashSet`. + operator_managed_startup_keys: Arc>, +} + +/// Shared empty key set so pools that don't use `startup_parameters` +/// hand every backend the same `Arc` instead of allocating a +/// new empty `HashSet` per spawn. +fn empty_operator_keys() -> Arc> { + static EMPTY: once_cell::sync::Lazy>> = + once_cell::sync::Lazy::new(|| Arc::new(HashSet::new())); + EMPTY.clone() } impl std::fmt::Display for Server { @@ -305,7 +321,7 @@ impl Server { } /// Drains any remaining data from the server that hasn't been read yet. - /// This is used to synchronize the connection state when data is unexpectedly available. + /// Used to synchronize connection state when data is unexpectedly available. /// All received data is discarded (sent to a sink). pub async fn wait_available(&mut self) { if !self.is_data_available() { @@ -502,7 +518,7 @@ impl Server { } /// Sets the number of expected responses in async mode. - /// This is calculated from the batch operations before sending to server. + /// Calculated from the batch operations before sending to the server. #[inline(always)] pub fn set_expected_responses(&mut self, count: u32) { self.expected_responses = count; @@ -692,7 +708,13 @@ impl Server { } pub async fn sync_parameters(&mut self, parameters: &ServerParameters) -> Result<(), Error> { - let parameter_diff = self.server_parameters.compare_params(parameters); + let mut parameter_diff = self.server_parameters.compare_params(parameters); + + // Do not let values from the client startup packet overwrite + // operator-supplied startup defaults during checkout sync. + if !self.operator_managed_startup_keys.is_empty() { + parameter_diff.retain(|k, _| !self.operator_managed_startup_keys.contains(k)); + } if parameter_diff.is_empty() { return Ok(()); @@ -741,6 +763,11 @@ impl Server { /// Pretend to be the Postgres client and connect to the server given host, port and credentials. /// Perform the authentication and return the server in a ready for query state. + /// + /// `startup_parameters` is the resolved cascade + /// (`general` -> pool -> auth_query). It is sent in the backend + /// `StartupMessage`. If PostgreSQL rejects a value, pg_doorman forwards + /// the `ErrorResponse` unchanged. #[allow(clippy::too_many_arguments)] pub async fn startup( address: &Address, @@ -753,6 +780,7 @@ impl Server { server_prepared_statement_cache_size: usize, application_name: String, session_mode: bool, + startup_parameters: &std::collections::BTreeMap, ) -> Result { let config = get_config(); @@ -797,11 +825,13 @@ impl Server { // code 0); see the `'R'` branch below. let auth_started = Instant::now(); let mut startup_started: Option = None; + startup( &mut stream, - username.clone(), + username.as_str(), database, - application_name.clone(), + application_name.as_str(), + startup_parameters, ) .await?; @@ -902,11 +932,67 @@ impl Server { } } - // ErrorResponse + // ErrorResponse during startup. Keep SQLSTATE class 57P + // on the fallback path; other startup errors are forwarded + // to the client as PostgreSQL returned them. 'E' => { - return handle_startup_error(&mut stream, len, &server_identifier) - .await - .map(|_| unreachable!()); + let mut bytes = read_message_data(&mut stream, code as u8, len).await?; + let _ = bytes.get_u8(); + let _ = bytes.get_i32(); + let Ok(msg) = PgErrorMsg::parse(&bytes) else { + return Err(Error::ServerStartupError( + "startup ErrorResponse".to_string(), + server_identifier.clone(), + )); + }; + + if msg.code.starts_with("57P") { + return Err(Error::ServerUnavailableError( + msg.message, + server_identifier.clone(), + )); + } + + // Identify the failing parameter for logs and metrics. + // + // First parse the common English `parameter ""` + // phrase, then fall back to looking for any sent key in + // double quotes. The fallback covers translated + // `lc_messages` where PostgreSQL still quotes the name. + let matched_key = if startup_parameters.is_empty() { + None + } else { + crate::server::startup_error::extract_parameter_name(&msg.message) + .filter(|n| startup_parameters.contains_key(n)) + .or_else(|| { + crate::server::startup_error::match_sent_key_in_message( + &msg.message, + startup_parameters.keys(), + ) + }) + }; + if let Some(param_name) = matched_key { + warn!( + "[{}@{}] PG rejected operator-supplied startup \ + parameter=\"{}\" sqlstate={} message=\"{}\"; the \ + error is being forwarded to the client. Fix the \ + parameter in general/pool/auth_query.", + address.username, address.pool_name, param_name, msg.code, msg.message, + ); + crate::web::metrics::BACKEND_STARTUP_PARAMETER_ERRORS_TOTAL + .with_label_values(&[&address.pool_name, &msg.code]) + .inc(); + return Err(Error::ServerStartupParameterRejection { + sqlstate: msg.code, + message: msg.message, + server_identifier: server_identifier.clone(), + }); + } + + return Err(Error::ServerStartupError( + format!("{}: {}", msg.code, msg.message), + server_identifier.clone(), + )); } // Notice @@ -972,6 +1058,36 @@ impl Server { phase_started.elapsed().as_secs_f64(), ); + // The empty case is a shared `Arc` static, so + // pools that don't use the feature pay zero allocation + // per spawn. The non-empty case still allocates once per + // spawn — the caller in pool/server_pool.rs knows the + // map shape but does not currently pass an already-Arc'd + // HashSet through, and lifting the construction up there + // is a larger refactor than this commit warrants. + let operator_managed_startup_keys: Arc> = if startup_parameters + .is_empty() + { + empty_operator_keys() + } else { + // Canonicalize every operator key the same way + // ServerParameters::set_param does on the + // sync_parameters path. Without this an operator + // value configured as `timezone` would not match + // a client-startup value reported as `TimeZone` + // in compare_params(), letting the client + // override the operator default. See codex + // MED #7 (fresh review). + Arc::new( + startup_parameters + .keys() + .map(|k| { + crate::server::parameters::canonicalize_param_name(k.clone()) + }) + .collect(), + ) + }; + let server = Server { address: address.to_owned(), stream: BufStream::new(stream), @@ -1010,6 +1126,7 @@ impl Server { pending_large_message: None, close_reason: None, override_lifetime_ms: None, + operator_managed_startup_keys, }; server.stats.update_process_id(process_id); server.stats.set_tls(connected_with_tls); diff --git a/src/server/startup_error.rs b/src/server/startup_error.rs index eaaab0513..133560bc5 100644 --- a/src/server/startup_error.rs +++ b/src/server/startup_error.rs @@ -9,7 +9,119 @@ use crate::messages::PgErrorMsg; use super::stream::StreamInner; -/// Handles error response during server startup. +/// Extract the GUC name from the human-readable M-field of a PG +/// ErrorResponse. Matches messages of the form: +/// `unrecognized configuration parameter "foobar"` +/// `invalid value for parameter "work_mem": "abc"` +/// `permission denied to set parameter "session_preload_libraries"` +/// +/// Returns `None` if the message does not contain the literal substring +/// `parameter "..."` (e.g. it is unrelated to a configuration parameter). +/// +/// Hand-rolled rather than using `regex` to keep that crate out of the +/// runtime dependency set; the pattern is fixed (`parameter "([^"]+)"`) +/// and trivial to scan for. The same constraint shaped +/// `crate::config::startup_parameters::is_valid_guc_name`. +pub fn extract_parameter_name(message: &str) -> Option { + const NEEDLE: &str = r#"parameter ""#; + let start = message.find(NEEDLE)? + NEEDLE.len(); + let rest = &message[start..]; + let end = rest.find('"')?; + if end == 0 { + return None; + } + Some(rest[..end].to_owned()) +} + +/// Classify a PostgreSQL `ErrorResponse` received during backend startup +/// into the right pg_doorman `Error` variant. Centralizes the SQLSTATE → +/// Error mapping so the rule stays in one place across `Server::startup`, +/// `handle_startup_error`, and anywhere else the startup path needs to +/// react to a PG-side rejection. +/// +/// Decision order: +/// +/// 1. SQLSTATE class `57P*` (server unavailable / shutting down / starting +/// up / cannot connect now) → `ServerUnavailableError`. Drives the +/// Patroni-assisted fallback path; must win over the startup-parameter +/// branch so a node-down rejection cannot be misclassified as a bad +/// operator GUC. +/// 2. If `sent_keys` is non-empty AND the PG message names a key in that +/// set (English `parameter ""` template, with a +/// locale-independent fallback that scans for any sent key wrapped in +/// double quotes) → `ServerStartupParameterRejection { sqlstate, +/// message, server_identifier }`. Lets the checkout site forward the +/// PG sqlstate verbatim to the client, instead of the generic 53300 +/// pool-exhausted fallback. +/// 3. Otherwise → `ServerStartupError(: , …)`. +pub fn classify_pg_startup_error<'a, I>( + sqlstate: String, + message: String, + server_identifier: &ServerIdentifier, + sent_keys: I, +) -> Error +where + I: IntoIterator, +{ + if sqlstate.starts_with("57P") { + return Error::ServerUnavailableError(message, server_identifier.clone()); + } + let mut sent_iter = sent_keys.into_iter().peekable(); + if sent_iter.peek().is_some() { + // Collect lazily so we walk the sent set at most once: try the + // English template first; on miss, scan once for any quoted key. + let parsed_key = extract_parameter_name(&message); + let collected: Vec<&'a String> = sent_iter.collect(); + let matched = parsed_key + .as_ref() + .filter(|n| collected.iter().any(|k| k.as_str() == n.as_str())) + .cloned() + .or_else(|| match_sent_key_in_message(&message, collected.iter().copied())); + if matched.is_some() { + return Error::ServerStartupParameterRejection { + sqlstate, + message, + server_identifier: server_identifier.clone(), + }; + } + } + Error::ServerStartupError(format!("{sqlstate}: {message}"), server_identifier.clone()) +} + +/// Locale-independent fallback for `extract_parameter_name`: scan the +/// PG ErrorResponse `M` field for any of the operator-supplied keys +/// pg_doorman actually sent, looking for the standard PG double-quoted +/// form (`"key"`). PG quotes the parameter name in every locale even when +/// the surrounding prose is translated, so a hit on `"name"` is a +/// reliable signal that the failing key is `name`. +/// +/// Returns the first match in iteration order of `sent_keys`. Used by +/// `Server::startup` to keep the +/// `pg_doorman_backend_startup_parameter_errors_total` counter usable +/// against PG servers with non-English `lc_messages`. +pub fn match_sent_key_in_message<'a, I>(message: &str, sent_keys: I) -> Option +where + I: IntoIterator, +{ + for key in sent_keys { + // Match the GUC name surrounded by double quotes. Bare substring + // search would false-positive on prose like "key is wrong" when + // `key` happens to be the parameter name; the quote markers in + // PG ErrorResponse messages are stable across locales. + let needle = format!("\"{key}\""); + if message.contains(&needle) { + return Some(key.clone()); + } + } + None +} + +/// Handles error response during server startup. Surfaces the PG sqlstate +/// through [`classify_pg_startup_error`] so non-`Server::startup` callers +/// (currently none in production; kept for symmetry and future code paths +/// that need verbatim PG-error-passthrough behaviour) follow the same +/// `ServerUnavailableError` / `ServerStartupError` mapping. +#[allow(dead_code)] pub(crate) async fn handle_startup_error( stream: &mut StreamInner, len: i32, @@ -47,17 +159,15 @@ pub(crate) async fn handle_startup_error( f.code, f.message ); - if f.code.starts_with("57P") { - Err(Error::ServerUnavailableError( - f.message, - server_identifier.clone(), - )) - } else { - Err(Error::ServerStartupError( - f.message, - server_identifier.clone(), - )) - } + // No sent-key set is available here; the helper falls + // back to plain `ServerStartupError` for non-57P codes, + // matching the previous behaviour exactly. + Err(classify_pg_startup_error( + f.code, + f.message, + server_identifier, + std::iter::empty::<&String>(), + )) } Err(err) => { error!( @@ -73,3 +183,85 @@ pub(crate) async fn handle_startup_error( } } } + +#[cfg(test)] +mod parameter_extractor_tests { + use super::*; + + #[test] + fn unknown_parameter_extracted() { + assert_eq!( + extract_parameter_name(r#"unrecognized configuration parameter "foobar""#), + Some("foobar".into()) + ); + } + + #[test] + fn invalid_value_extracted() { + assert_eq!( + extract_parameter_name(r#"invalid value for parameter "work_mem": "abc""#), + Some("work_mem".into()) + ); + } + + #[test] + fn permission_denied_extracted() { + assert_eq!( + extract_parameter_name( + r#"permission denied to set parameter "session_preload_libraries""# + ), + Some("session_preload_libraries".into()) + ); + } + + #[test] + fn no_quoted_parameter_returns_none() { + assert_eq!(extract_parameter_name("connection refused by peer"), None); + } + + #[test] + fn namespaced_parameter_extracted() { + assert_eq!( + extract_parameter_name( + r#"unrecognized configuration parameter "auto_explain.log_min_duration""# + ), + Some("auto_explain.log_min_duration".into()) + ); + } + + #[test] + fn match_sent_key_finds_quoted_key_in_localized_message() { + // Hypothetical Russian lc_messages output: prose is translated, + // PG still wraps the parameter name in double quotes. + let sent = vec!["plan_cache_mode".to_string(), "work_mem".to_string()]; + let msg = r#"параметр "plan_cache_mode" не существует"#; + assert_eq!( + match_sent_key_in_message(msg, &sent), + Some("plan_cache_mode".into()) + ); + } + + #[test] + fn match_sent_key_returns_none_when_no_quoted_match() { + let sent = vec!["plan_cache_mode".to_string()]; + // The key appears in prose but not quoted — must not match, + // otherwise unrelated PG errors mentioning the word would + // poison the counter. + let msg = "some unrelated startup error mentions plan_cache_mode somewhere"; + assert!(match_sent_key_in_message(msg, &sent).is_none()); + } + + #[test] + fn match_sent_key_skips_keys_not_in_message() { + let sent = vec![ + "first_key".to_string(), + "second_key".to_string(), + "third_key".to_string(), + ]; + let msg = r#"FATAL: invalid value for parameter "second_key""#; + assert_eq!( + match_sent_key_in_message(msg, &sent), + Some("second_key".into()) + ); + } +} diff --git a/src/stats/pool.rs b/src/stats/pool.rs index f7e85ee80..ed31ffd42 100644 --- a/src/stats/pool.rs +++ b/src/stats/pool.rs @@ -828,6 +828,31 @@ mod tests { assert_eq!(row[14].as_ref(), "5", "column 14 should be avg_errors"); } + /// SHOW POOLS row width must match the header width on every config, + /// otherwise the admin console renders misaligned columns. The risk + /// shows up when only one of header or row is touched when a new + /// column is added. + #[test] + fn show_pools_row_and_header_have_same_width() { + let percentile = Percentile { + p99: 0, + p95: 0, + p90: 0, + p50: 0, + }; + let stats = PoolStats::new_with_percentiles( + PoolIdentifier::new("shop", "alice"), + PoolMode::Transaction, + percentile.clone(), + percentile.clone(), + percentile, + ); + + let header = PoolStats::generate_show_pools_header(); + let row = stats.generate_show_pools_row(); + assert_eq!(header.len(), row.len(), "header/row width mismatch"); + } + /// Both entry points must agree on shape when fed the same global /// POOLS state and equivalent client/server maps. Validates that /// `construct_pool_lookup_from` is a structural extract of diff --git a/src/web/auth.rs b/src/web/auth.rs index efcc7048a..d8c1b6347 100644 --- a/src/web/auth.rs +++ b/src/web/auth.rs @@ -59,6 +59,32 @@ impl AuthOutcome { } } +/// Whether the listener should accept SSO credentials presented over a +/// plain-HTTP request. `request_is_secure` is the listener's verdict +/// after combining the TCP peer with `X-Forwarded-Proto` (see +/// [`crate::web::peer::request_is_secure`]); `require_https` is the +/// operator's `[web].sso_require_https` knob. +/// +/// Default policy (`require_https=false`) keeps backward compatibility: +/// every deployment where a TLS-terminating proxy reaches pg_doorman +/// over a private HTTP leg keeps working without configuration changes. +/// Opt-in `require_https=true` rejects SSO credentials on plain HTTP so +/// the JWT cannot leak between the proxy and pg_doorman. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SsoTransportPolicy { + pub request_is_secure: bool, + pub require_https: bool, +} + +impl SsoTransportPolicy { + /// Permits SSO credentials when the operator has not opted in to + /// HTTPS-only SSO, or when the request actually arrived over a + /// trusted HTTPS hop. + fn permits_sso(self) -> bool { + !self.require_https || self.request_is_secure + } +} + /// Classify an inbound HTTP request into an `AuthOutcome`. Recognises: /// /// - `Authorization: Basic ` against the admin @@ -76,6 +102,15 @@ impl AuthOutcome { /// credentials to deny timing oracles. Both username and password legs /// are checked together without short-circuit (see the `&` operator /// inside the implementation). +/// +/// `sso_transport` decides whether the SSO branches run at all. When +/// the operator set `[web].sso_require_https = true` and the request +/// did not arrive over a trusted HTTPS hop, every SSO source is +/// skipped and the function falls through to either `Rejected` (an +/// SSO credential was attempted) or `Anonymous` (no credentials at +/// all). Basic credentials are unaffected — they are scheme-bound by +/// `Authorization: Basic` and live or die on the constant-time +/// compare above, regardless of transport. pub fn classify( authorization_header: Option<&str>, cookie_header: Option<&str>, @@ -83,6 +118,7 @@ pub fn classify( admin_username: &str, admin_password: &str, sso: Option<&crate::web::sso::SsoRuntime>, + sso_transport: SsoTransportPolicy, ) -> AuthOutcome { let mut tried = false; @@ -128,22 +164,42 @@ pub fn classify( // credential attempt when SSO is actually configured to consume // them; otherwise an Anonymous request to a public endpoint that // happens to carry such a cookie should still pass. + // + // Transport gate: when `sso_require_https` is on and the request + // did not arrive over a trusted HTTPS hop, skip every SSO branch + // and record one telemetry sample per blocked attempt. A request + // that carried only `Authorization: Bearer …` still sets + // `tried = true` above, so the caller falls through to a 401 — + // important so an operator chasing a misconfigured proxy gets a + // real failure instead of silent Anonymous behaviour. if let Some(rt) = sso { - if let Some(token) = bearer_token { - if let Ok(id) = rt.validate(token) { - return sso_outcome(id); + if sso_transport.permits_sso() { + if let Some(token) = bearer_token { + if let Ok(id) = rt.validate(token) { + return sso_outcome(id); + } } - } - if let Some(token) = query_token { - tried = true; - if let Ok(id) = rt.validate(token) { - return sso_outcome(id); + if let Some(token) = query_token { + tried = true; + if let Ok(id) = rt.validate(token) { + return sso_outcome(id); + } } - } - if let Some(token) = cookie_header.and_then(find_sso_cookie) { - tried = true; - if let Ok(id) = rt.validate(token) { - return sso_outcome(id); + if let Some(token) = cookie_header.and_then(find_sso_cookie) { + tried = true; + if let Ok(id) = rt.validate(token) { + return sso_outcome(id); + } + } + } else { + let presented = bearer_token.is_some() + || query_token.is_some() + || cookie_header.and_then(find_sso_cookie).is_some(); + if presented { + tried = true; + crate::web::metrics::WEB_SSO_VALIDATION_ERRORS + .with_label_values(&["insecure_transport"]) + .inc(); } } } @@ -227,10 +283,43 @@ mod tests { .unwrap() } + fn permissive_transport() -> SsoTransportPolicy { + SsoTransportPolicy { + request_is_secure: false, + require_https: false, + } + } + + fn https_only_transport(is_secure: bool) -> SsoTransportPolicy { + SsoTransportPolicy { + request_is_secure: is_secure, + require_https: true, + } + } + + fn classify_default( + auth: Option<&str>, + cookie: Option<&str>, + query: Option<&str>, + admin_user: &str, + admin_pass: &str, + sso: Option<&SsoRuntime>, + ) -> AuthOutcome { + classify( + auth, + cookie, + query, + admin_user, + admin_pass, + sso, + permissive_transport(), + ) + } + #[test] fn anonymous_when_header_missing() { assert_eq!( - classify(None, None, None, "admin", "secret", None), + classify_default(None, None, None, "admin", "secret", None), AuthOutcome::Anonymous ); } @@ -239,7 +328,7 @@ mod tests { fn admin_when_credentials_match() { let header = format!("Basic {}", b64("admin:secret")); assert_eq!( - classify(Some(&header), None, None, "admin", "secret", None), + classify_default(Some(&header), None, None, "admin", "secret", None), admin("admin") ); } @@ -248,7 +337,7 @@ mod tests { fn rejected_when_password_wrong() { let header = format!("Basic {}", b64("admin:wrong")); assert_eq!( - classify(Some(&header), None, None, "admin", "secret", None), + classify_default(Some(&header), None, None, "admin", "secret", None), AuthOutcome::Rejected ); } @@ -257,7 +346,7 @@ mod tests { fn rejected_when_username_wrong() { let header = format!("Basic {}", b64("evil:secret")); assert_eq!( - classify(Some(&header), None, None, "admin", "secret", None), + classify_default(Some(&header), None, None, "admin", "secret", None), AuthOutcome::Rejected ); } @@ -266,7 +355,7 @@ mod tests { fn rejected_when_scheme_not_basic_and_no_sso() { let header = format!("Bearer {}", b64("admin:secret")); assert_eq!( - classify(Some(&header), None, None, "admin", "secret", None), + classify_default(Some(&header), None, None, "admin", "secret", None), AuthOutcome::Rejected ); } @@ -274,7 +363,7 @@ mod tests { #[test] fn rejected_when_base64_invalid() { assert_eq!( - classify( + classify_default( Some("Basic !!!not-base64!!!"), None, None, @@ -290,7 +379,7 @@ mod tests { fn rejected_when_decoded_has_no_colon() { let header = format!("Basic {}", b64("adminsecret")); assert_eq!( - classify(Some(&header), None, None, "admin", "secret", None), + classify_default(Some(&header), None, None, "admin", "secret", None), AuthOutcome::Rejected ); } @@ -300,7 +389,7 @@ mod tests { let raw = base64::engine::general_purpose::STANDARD.encode([0xff, 0xfe, 0xfd]); let header = format!("Basic {}", raw); assert_eq!( - classify(Some(&header), None, None, "admin", "secret", None), + classify_default(Some(&header), None, None, "admin", "secret", None), AuthOutcome::Rejected ); } @@ -310,7 +399,7 @@ mod tests { // Per RFC 7617 only the FIRST colon is the separator. let header = format!("Basic {}", b64("admin:p:a:s:s")); assert_eq!( - classify(Some(&header), None, None, "admin", "p:a:s:s", None), + classify_default(Some(&header), None, None, "admin", "p:a:s:s", None), admin("admin") ); } @@ -333,7 +422,7 @@ mod tests { let token = mint(600, "alice"); let header = format!("Bearer {}", token); let rt = sso_rt(AllowedUsers::Any); - let out = classify(Some(&header), None, None, "admin", "secret", Some(&rt)); + let out = classify_default(Some(&header), None, None, "admin", "secret", Some(&rt)); match out { AuthOutcome::Sso(id) => { assert_eq!(id.username, "alice"); @@ -347,7 +436,7 @@ mod tests { fn sso_query_token_works() { let token = mint(600, "alice"); let rt = sso_rt(AllowedUsers::Any); - let out = classify(None, None, Some(&token), "admin", "secret", Some(&rt)); + let out = classify_default(None, None, Some(&token), "admin", "secret", Some(&rt)); assert!(matches!(out, AuthOutcome::Sso(_))); } @@ -356,7 +445,7 @@ mod tests { let token = mint(600, "alice"); let cookie = format!("foo=bar; sso_access_token={}; baz=qux", token); let rt = sso_rt(AllowedUsers::Any); - let out = classify(None, Some(&cookie), None, "admin", "secret", Some(&rt)); + let out = classify_default(None, Some(&cookie), None, "admin", "secret", Some(&rt)); assert!(matches!(out, AuthOutcome::Sso(_))); } @@ -368,7 +457,7 @@ mod tests { let basic = format!("Basic {}", b64("admin:secret")); let cookie = format!("sso_access_token={}", token); let rt = sso_rt(AllowedUsers::Any); - let out = classify( + let out = classify_default( Some(&basic), Some(&cookie), None, @@ -390,7 +479,7 @@ mod tests { let basic = format!("Basic {}", b64("admin:wrong")); let cookie = format!("sso_access_token={}", token); let rt = sso_rt(AllowedUsers::Any); - let out = classify( + let out = classify_default( Some(&basic), Some(&cookie), None, @@ -408,7 +497,7 @@ mod tests { let token = mint(-600, "alice"); let header = format!("Bearer {}", token); let rt = sso_rt(AllowedUsers::Any); - let out = classify(Some(&header), None, None, "admin", "secret", Some(&rt)); + let out = classify_default(Some(&header), None, None, "admin", "secret", Some(&rt)); assert_eq!(out, AuthOutcome::Rejected); } @@ -419,7 +508,7 @@ mod tests { let rt = sso_rt(AllowedUsers::List( ["alice".to_string()].into_iter().collect(), )); - let out = classify(Some(&header), None, None, "admin", "secret", Some(&rt)); + let out = classify_default(Some(&header), None, None, "admin", "secret", Some(&rt)); assert_eq!(out, AuthOutcome::Rejected); } @@ -430,7 +519,99 @@ mod tests { // so a credential was attempted but nothing took it: Rejected. let token = mint(600, "alice"); let header = format!("Bearer {}", token); - let out = classify(Some(&header), None, None, "admin", "secret", None); + let out = classify_default(Some(&header), None, None, "admin", "secret", None); assert_eq!(out, AuthOutcome::Rejected); } + + #[test] + fn require_https_rejects_bearer_on_plain_http() { + // sso_require_https = true and the transport is not secure → the + // Bearer JWT is treated as a credential attempt that failed, so + // the caller gets a 401 instead of an Anonymous fall-through. + let token = mint(600, "alice"); + let header = format!("Bearer {}", token); + let rt = sso_rt(AllowedUsers::Any); + let out = classify( + Some(&header), + None, + None, + "admin", + "secret", + Some(&rt), + https_only_transport(false), + ); + assert_eq!(out, AuthOutcome::Rejected); + } + + #[test] + fn require_https_accepts_bearer_on_secure_hop() { + let token = mint(600, "alice"); + let header = format!("Bearer {}", token); + let rt = sso_rt(AllowedUsers::Any); + let out = classify( + Some(&header), + None, + None, + "admin", + "secret", + Some(&rt), + https_only_transport(true), + ); + assert!(matches!(out, AuthOutcome::Sso(_))); + } + + #[test] + fn require_https_does_not_block_basic() { + // Basic credentials live or die on the constant-time compare; + // sso_require_https only gates the SSO branches. + let header = format!("Basic {}", b64("admin:secret")); + let out = classify( + Some(&header), + None, + None, + "admin", + "secret", + None, + https_only_transport(false), + ); + assert_eq!(out, admin("admin")); + } + + #[test] + fn require_https_leaves_anonymous_alone_when_no_sso_attempt() { + // sso_require_https is on but the request carries no SSO source + // at all — it stays Anonymous so the caller can still hit a + // public read-only endpoint over plain HTTP. + let rt = sso_rt(AllowedUsers::Any); + let out = classify( + None, + None, + None, + "admin", + "secret", + Some(&rt), + https_only_transport(false), + ); + assert_eq!(out, AuthOutcome::Anonymous); + } + + #[test] + fn require_https_off_keeps_plain_http_sso_working() { + // The default policy (require_https = false) preserves + // backward-compatible behaviour for the SSO-proxy-fronts-pg_doorman + // deployment where the proxy → pg_doorman hop is private HTTP. + let token = mint(600, "alice"); + let header = format!("Bearer {}", token); + let rt = sso_rt(AllowedUsers::Any); + let out = classify( + Some(&header), + None, + None, + "admin", + "secret", + Some(&rt), + permissive_transport(), + ); + assert!(matches!(out, AuthOutcome::Sso(_))); + } } diff --git a/src/web/metrics/mod.rs b/src/web/metrics/mod.rs index effa42827..e6e6ce894 100644 --- a/src/web/metrics/mod.rs +++ b/src/web/metrics/mod.rs @@ -457,6 +457,130 @@ pub(crate) static LISTENER_REJECTIONS_TOTAL: Lazy = Lazy::new(|| counter }); +/// Counts backend startup attempts pg_doorman aborted because PostgreSQL +/// returned an `ErrorResponse` that names a key the pool actually sent in +/// `StartupMessage`. Labels: +/// +/// * `pool` — pool name as it appears in `pools.` of the config +/// (in the default mapping this is the PostgreSQL database name). +/// The label is *not* `@`: pg_doorman emits one +/// series per pool name, so a multi-user database collapses into a +/// single row. Per-user attribution lives in the warn log line. +/// * `sqlstate` — PG SQLSTATE on the rejection (`22023`, `42704`, +/// `42501`, `55P02`, or any other code under the startup-parameter +/// family — pg_doorman does not pre-filter by SQLSTATE). +/// +/// SQLSTATEs with the `57P` prefix (server unavailable) are excluded: those +/// `ErrorResponse`s are surfaced as `ServerUnavailableError` to drive the +/// Patroni-assisted fallback path before the counter branch is reached. +/// +/// Identification of the failing key is best-effort: pg_doorman first +/// parses the canonical English `parameter ""` phrase, then falls +/// back to scanning the M-field for any sent key wrapped in double +/// quotes (PG keeps the quote markers across all `lc_messages` locales). +/// If both heuristics fail — typically because the PG error is unrelated +/// to the sent map at all — the counter does NOT increment. Operator +/// reading a non-zero rate can be confident the issue is on a key they +/// configured; the per-line warn log carries the parameter name and +/// username for triage. +/// +/// The parameter name and username are intentionally NOT in the label +/// set so a dynamic `auth_query` pool that mints per-tenant roles cannot +/// blow up Prometheus series count by reading user input into labels. +/// Counts cases where pg_doorman dropped configured +/// `startup_parameters` *before* the StartupMessage went on the wire — +/// the failure mode the per-pool `*_errors_total` counter cannot see +/// because PG never had a chance to reject. Every reason increments +/// the counter by 1 per drop event (one backend spawn that dropped +/// the resolved set, one parsed row that contained invalid entries, one +/// row whose overlay was ignored because of dedicated mode), so +/// `rate by(reason)` is dimensionally consistent regardless of how +/// many individual keys the offending row carried. Per-entry detail +/// goes to the warn log only. Labels: +/// +/// * `pool` — pool name as it appears in `pools.` of the config +/// (in the default mapping this is the PostgreSQL database name). +/// The label is *not* `@`: pg_doorman emits one +/// series per pool name, so a multi-user database collapses into a +/// single row. Per-user attribution lives in the warn log line. +/// * `reason` — bounded enum: +/// * `cascade_budget_exceeded` — the resolved general+pool+auth_query +/// map exceeded the startup-parameter budget (`MAX_OPERATOR_BUDGET`, 9 488 +/// bytes). Every configured key was dropped for that spawn +/// and the backend got PG defaults instead. +/// * `packet_cap_exceeded` — the full StartupMessage including user, +/// application_name and database would exceed PG's +/// `MAX_STARTUP_PACKET_LENGTH` (10 000 bytes). Same drop-all +/// behaviour. +/// * `auth_query_oversize` — the auth_query `startup_parameters` +/// text column for some username exceeded the operator budget at +/// parse time, so the per-user overlay is ignored. +/// * `auth_query_overlay_oversize` — the merged baseline+overlay was +/// over budget, but the baseline alone fits. Keeps general/pool +/// defaults (statement_timeout, lock_timeout, ...) for that user +/// instead of stripping the whole configured parameter set. +/// * `auth_query_bad_type` — the auth_query `startup_parameters` +/// column has a non-text type (likely `json`/`jsonb`); pg_doorman +/// reads it as text, so the row's overlay is dropped. Cast to +/// `::text` in the auth_query SELECT to fix. +/// * `auth_query_invalid_json` — the column value is not valid JSON. +/// * `auth_query_invalid_shape` — the column parses but the +/// top-level value is not a JSON object. +/// * `auth_query_invalid_entry` — at least one entry in the parsed +/// auth_query JSON object failed validation (reserved key, bad +/// GUC name, null byte, non-string value). Incremented once per +/// parsed row that had any invalid entry. +/// * `dedicated_mode` — a per-user auth_query row carried +/// startup_parameters, but the pool runs in dedicated auth_query +/// mode (one shared backend across users) so the per-user overlay +/// was dropped. Incremented once per such row. +/// +/// All cases also emit a `warn!` log line for human triage; the +/// counter exists so dashboards and alerts can spot the silent drop +/// without log scraping. +pub(crate) static STARTUP_PARAMETERS_DROPPED_TOTAL: Lazy = Lazy::new(|| { + let counter = IntCounterVec::new( + Opts::new( + "pg_doorman_startup_parameters_dropped_total", + "Cumulative count of startup_parameters drop events before \ + pg_doorman sends StartupMessage. \ + Labels: pool, reason (cascade_budget_exceeded, \ + packet_cap_exceeded, auth_query_oversize, \ + auth_query_overlay_oversize, auth_query_bad_type, \ + auth_query_invalid_json, auth_query_invalid_shape, \ + auth_query_invalid_entry, dedicated_mode). Distinct from \ + pg_doorman_backend_startup_parameter_errors_total which \ + counts PG-side rejections after StartupMessage.", + ), + &["pool", "reason"], + ) + .unwrap(); + REGISTRY.register(Box::new(counter.clone())).unwrap(); + counter +}); + +pub(crate) static BACKEND_STARTUP_PARAMETER_ERRORS_TOTAL: Lazy = Lazy::new(|| { + let counter = IntCounterVec::new( + Opts::new( + "pg_doorman_backend_startup_parameter_errors_total", + "Cumulative count of backend startup attempts pg_doorman \ + aborted because PostgreSQL ErrorResponse identified a key \ + this pool sent in StartupMessage (configured \ + startup_parameters). Labels: pool, sqlstate. \ + SQLSTATEs with the 57P prefix (server unavailable) are excluded — \ + those rejections take the Patroni-assisted fallback path \ + instead. The failing parameter name and username are in \ + the corresponding warn log line; kept out of the label set \ + so dynamic auth_query roles cannot inflate Prometheus \ + series count.", + ), + &["pool", "sqlstate"], + ) + .unwrap(); + REGISTRY.register(Box::new(counter.clone())).unwrap(); + counter +}); + /// Counter for protocol-level large-message streaming events. pg_doorman /// drops to byte-stream forwarding when a server message of type DataRow /// ('D') or CopyData ('d') exceeds max_message_size — see @@ -1232,7 +1356,7 @@ pub(crate) static WEB_SSO_VALIDATION_ERRORS: Lazy = Lazy::new(|| let counter = IntCounterVec::new( Opts::new( "pg_doorman_web_sso_validation_errors_total", - "JWT validation failures by reason: signature, expired, audience, no_username, allowlist. A sustained signature spike means the SSO proxy rotated keys without updating sso_public_key_file; allowlist spikes mean someone outside the allowlist is trying to log in.", + "JWT validation failures by reason: signature, expired, audience, no_username, allowlist, insecure_transport. A sustained signature spike means the SSO proxy rotated keys without updating sso_public_key_file; allowlist spikes mean someone outside the allowlist is trying to log in; insecure_transport means [web].sso_require_https is on and a JWT arrived without the trusted-proxy + X-Forwarded-Proto: https hop required to accept it.", ), &["reason"], ) diff --git a/src/web/metrics/tests.rs b/src/web/metrics/tests.rs index 3b47b0f4a..f968c1ede 100644 --- a/src/web/metrics/tests.rs +++ b/src/web/metrics/tests.rs @@ -41,6 +41,7 @@ async fn test_prometheus_server_basic() { sso_config_error: None, trusted_proxies: Vec::new(), sso_admin_groups_configured: false, + sso_require_https: false, }, ) .await; @@ -245,6 +246,7 @@ async fn test_prometheus_server_integration() { sso_config_error: None, trusted_proxies: Vec::new(), sso_admin_groups_configured: false, + sso_require_https: false, }, ) .await; diff --git a/src/web/peer.rs b/src/web/peer.rs index fbd35d1a5..4a92c7216 100644 --- a/src/web/peer.rs +++ b/src/web/peer.rs @@ -48,6 +48,48 @@ fn is_trusted(addr: IpAddr, trusted: &[IpNet]) -> bool { trusted.iter().any(|net| net.contains(&addr)) } +/// Decide whether the inbound request reached pg_doorman over a secure +/// transport. The listener itself terminates plain HTTP, so the only +/// honest signal is the proxy's `X-Forwarded-Proto` — and we trust it +/// only when the TCP peer is in `trusted_proxies`. An attacker who +/// connects directly without going through the proxy controls every +/// header they want to send; the trusted-peer gate is what keeps them +/// from forging `https` to bypass `sso_require_https`. +/// +/// Multi-hop chains (`X-Forwarded-Proto: https, http`) are read +/// left-to-right and accepted only when every hop is `https` — +/// any `http` segment downgrades the result. This matches the access +/// log's right-to-left walk in spirit: the inner-most hop closest to +/// pg_doorman is the one we cannot verify, so a chain that includes +/// a plain leg is treated as plain. +pub fn request_is_secure( + peer_addr: Option, + x_forwarded_proto: Option<&str>, + trusted_proxies: &[IpNet], +) -> bool { + let Some(peer) = peer_addr else { + return false; + }; + if !is_trusted(peer.ip(), trusted_proxies) { + return false; + } + let Some(value) = x_forwarded_proto else { + return false; + }; + let mut seen_any = false; + for hop in value.split(',') { + let hop = hop.trim(); + if hop.is_empty() { + continue; + } + seen_any = true; + if !hop.eq_ignore_ascii_case("https") { + return false; + } + } + seen_any +} + /// Walks `X-Forwarded-For` right-to-left, skipping trusted IPs. /// Returns the first untrusted address found, or `None`. fn walk_xff(xff: Option<&str>, trusted: &[IpNet]) -> Option { @@ -197,4 +239,85 @@ mod tests { ); assert_eq!(out, "198.51.100.42"); } + + #[test] + fn request_is_secure_requires_trusted_peer_and_https() { + assert!(request_is_secure( + Some(sock("10.0.0.1:443")), + Some("https"), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_rejects_untrusted_peer() { + assert!(!request_is_secure( + Some(sock("203.0.113.7:443")), + Some("https"), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_rejects_missing_header() { + assert!(!request_is_secure( + Some(sock("10.0.0.1:443")), + None, + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_rejects_http_value() { + assert!(!request_is_secure( + Some(sock("10.0.0.1:443")), + Some("http"), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_case_insensitive() { + assert!(request_is_secure( + Some(sock("10.0.0.1:443")), + Some("HTTPS"), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_downgrades_on_mixed_chain() { + assert!(!request_is_secure( + Some(sock("10.0.0.1:443")), + Some("https, http"), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_accepts_chain_of_https() { + assert!(request_is_secure( + Some(sock("10.0.0.1:443")), + Some("https, https"), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_rejects_empty_header() { + assert!(!request_is_secure( + Some(sock("10.0.0.1:443")), + Some(" , "), + &[net("10.0.0.0/8")], + )); + } + + #[test] + fn request_is_secure_rejects_when_no_peer() { + assert!(!request_is_secure( + None, + Some("https"), + &[net("10.0.0.0/8")] + )); + } } diff --git a/src/web/routes/collect/config.rs b/src/web/routes/collect/config.rs index 234181a5f..b4fcc64ed 100644 --- a/src/web/routes/collect/config.rs +++ b/src/web/routes/collect/config.rs @@ -21,6 +21,16 @@ fn is_secret_key(key: &str) -> bool { || last_segment.ends_with("_key") } +/// Returns `true` for keys that live inside a `startup_parameters` +/// cascade (`general.startup_parameters.` or +/// `pools..startup_parameters.`). These values are +/// operator-supplied and may carry tenant identifiers, audit routing +/// tags, or accidental secrets - the same redaction contract that +/// applies to `/api/pools` for anonymous viewers also applies here. +fn is_startup_parameter_key(key: &str) -> bool { + key.contains(".startup_parameters.") || key.starts_with("startup_parameters.") +} + /// Bind-address fields require a restart; everything else takes effect on /// the next backend or `RELOAD`. Listed precisely so the UI can render /// the right "restart_required" pill instead of marking everything @@ -78,7 +88,7 @@ fn json_leaf_to_string(value: &serde_json::Value) -> String { } } -pub(crate) fn collect_config() -> ConfigDto { +pub(crate) fn collect_config(reveal_startup_values: bool) -> ConfigDto { let config = get_config(); let mut flat: HashMap = HashMap::new(); @@ -100,10 +110,12 @@ pub(crate) fn collect_config() -> ConfigDto { .into_iter() .map(|(key, value)| { let secret = is_secret_key(&key); - let value = if secret { "***".to_string() } else { value }; + let startup_redact = !reveal_startup_values && is_startup_parameter_key(&key); + let mask = secret || startup_redact; + let value = if mask { "***".to_string() } else { value }; let default = defaults .get(&key) - .map(|d| if secret { "***".to_string() } else { d.clone() }) + .map(|d| if mask { "***".to_string() } else { d.clone() }) .unwrap_or_else(|| "-".to_string()); let changeable = if IMMUTABLES.iter().any(|c| *c == key) { "no" @@ -227,7 +239,7 @@ mod tests { /// refactor that quietly trims keys gets caught. #[test] fn collect_config_exposes_operationally_relevant_fields() { - let dto = super::collect_config(); + let dto = super::collect_config(true); let keys: std::collections::HashSet<&str> = dto.config.iter().map(|e| e.key.as_str()).collect(); // Spot-check four orthogonal areas DBA P3#7 called out: @@ -241,7 +253,7 @@ mod tests { #[test] fn collect_config_drops_internal_keys() { - let dto = super::collect_config(); + let dto = super::collect_config(true); for entry in &dto.config { assert!( !super::is_internal_key(&entry.key), diff --git a/src/web/routes/collect/pools.rs b/src/web/routes/collect/pools.rs index 25ff272b8..a3cf180ba 100644 --- a/src/web/routes/collect/pools.rs +++ b/src/web/routes/collect/pools.rs @@ -2,11 +2,11 @@ use crate::pool::get_all_pools; use crate::web::metrics::{ FALLBACK_ACTIVE, SHOW_SERVER_TLS_CONNECTIONS, SHOW_SERVER_TLS_HANDSHAKE_ERRORS, }; -use crate::web::routes::dto::{PoolDto, PoolsDto}; +use crate::web::routes::dto::{PoolDto, PoolsDto, StartupParameterDto}; use super::{now_unix_ms, snapshot}; -pub(crate) fn collect_pools() -> PoolsDto { +pub(crate) fn collect_pools(reveal_startup_values: bool) -> PoolsDto { let snap = snapshot(); let pool_lookup = &snap.pool_lookup; let pools_map = get_all_pools(); @@ -70,6 +70,10 @@ pub(crate) fn collect_pools() -> PoolsDto { fallback_active, tls_handshake_errors_total, tls_backend_connections, + startup_parameters: StartupParameterDto::from_resolved( + pool.database.effective_startup_parameters_with_sources(), + reveal_startup_values, + ), }; pools.push(dto); } diff --git a/src/web/routes/config.rs b/src/web/routes/config.rs index 682df3625..3b91fd0d3 100644 --- a/src/web/routes/config.rs +++ b/src/web/routes/config.rs @@ -1,10 +1,16 @@ //! GET /api/config handler. +use crate::web::auth::Role; use crate::web::routes::collect::collect_config; use crate::web::server::Response; -pub(crate) fn handle_config() -> Response { - Response::ok_json(&collect_config()) +pub(crate) fn handle_config(role: Role) -> Response { + // Mirror the /api/pools contract: operator-supplied + // startup_parameter values are masked for anonymous readers because + // they may carry tenant identifiers, audit routing tags, or + // accidental secrets. SSO and Admin keep the full view. + let reveal_startup_values = role >= Role::Sso; + Response::ok_json(&collect_config(reveal_startup_values)) } #[cfg(test)] @@ -13,10 +19,36 @@ mod tests { #[test] fn config_response_is_200_with_envelope() { - let r = handle_config(); + let r = handle_config(Role::Admin); assert_eq!(r.status, 200); let body = std::str::from_utf8(&r.body).unwrap(); assert!(body.contains("\"ts\"")); assert!(body.contains("\"config\"")); } + + #[test] + fn anonymous_config_masks_startup_parameter_values() { + // Set general.startup_parameters via the in-process config so + // collect_config has something to redact. The Lazy + // config defaults to an empty `Config`, so we rely on the + // serialized JSON containing `*.startup_parameters` as a + // nested object only when something is set there. The masked + // value `"***"` should appear in the response for any such key. + let r = handle_config(Role::Anonymous); + let body = std::str::from_utf8(&r.body).unwrap(); + // Bare-minimum invariant: no occurrence of any unmasked + // startup_parameters value can appear under the anonymous + // viewer for any key path ending in `startup_parameters.*`. + // The key path itself is fine (operators want to see *which* + // GUCs are configured), only the value is hidden. + let lower = body.to_lowercase(); + if lower.contains("startup_parameters.") { + // If the test environment configures any startup_parameter, + // the response must mask its value. + assert!( + body.contains("\"***\""), + "anonymous /api/config has a startup_parameters entry but no masked '***' value, body={body}" + ); + } + } } diff --git a/src/web/routes/dto.rs b/src/web/routes/dto.rs index 4b4b68ac8..222933d56 100644 --- a/src/web/routes/dto.rs +++ b/src/web/routes/dto.rs @@ -7,7 +7,7 @@ //! tests are a candidate follow-up. use serde::Serialize; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; #[derive(Debug, Serialize)] pub(crate) struct VersionDto { @@ -148,6 +148,122 @@ pub(crate) struct PoolDto { /// Live TLS-encrypted backend connections held by the pool. Mirrors /// `pg_doorman_server_tls_connections`. pub tls_backend_connections: u64, + + /// Operator-supplied PostgreSQL startup parameters this pool injects into + /// each new backend `StartupMessage`, in the order produced by the + /// `general` -> pool -> auth_query cascade. Each entry carries the layer + /// that contributed the winning value. Omitted from JSON when the pool + /// has no operator overrides for this user. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub startup_parameters: Vec, +} + +/// One entry in `PoolDto.startup_parameters`. The `source` field tells the +/// operator which cascade layer contributed the value — `"general"`, `"pool"` +/// or `"auth_query"`. `value` is omitted for anonymous viewers because +/// operator-supplied values can include tenant identifiers, audit tags, +/// or extension-specific GUC payloads the public read-only UI must not +/// expose; admin/SSO callers and the admin SQL command keep the full +/// value. +#[derive(Debug, Serialize)] +pub(crate) struct StartupParameterDto { + pub parameter: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub value: Option, + pub source: &'static str, + /// `applied` / `dropped_due_to_budget` / `stale` — see + /// `pool::startup_resolver::ApplicationState`. Tells an operator + /// whether the configured cascade entry will actually ship in the + /// next backend `StartupMessage` for this pool. + pub state: &'static str, +} + +impl StartupParameterDto { + pub fn from_resolved( + merged: BTreeMap< + String, + ( + String, + crate::pool::startup_resolver::ParameterSource, + crate::pool::startup_resolver::ApplicationState, + ), + >, + reveal_values: bool, + ) -> Vec { + merged + .into_iter() + .map(|(parameter, (value, source, state))| StartupParameterDto { + parameter, + value: if reveal_values { Some(value) } else { None }, + source: source.as_str(), + state: state.as_str(), + }) + .collect() + } +} + +#[cfg(test)] +mod startup_parameter_dto_tests { + use super::*; + use crate::pool::startup_resolver::{ApplicationState, ParameterSource}; + + fn sample_merged() -> BTreeMap { + let mut m = BTreeMap::new(); + m.insert( + "application_name".to_string(), + ( + "tenant-a-audit".to_string(), + ParameterSource::Pool, + ApplicationState::Applied, + ), + ); + m.insert( + "statement_timeout".to_string(), + ( + "30s".to_string(), + ParameterSource::General, + ApplicationState::Applied, + ), + ); + m + } + + #[test] + fn reveal_values_true_serializes_value_field() { + let dtos = StartupParameterDto::from_resolved(sample_merged(), true); + let json = serde_json::to_string(&dtos).expect("serialize"); + assert!( + json.contains("\"value\":\"tenant-a-audit\""), + "expected admin/SSO view to include the operator-supplied value, got {json}" + ); + assert!( + json.contains("\"value\":\"30s\""), + "expected admin/SSO view to include the general baseline value, got {json}" + ); + } + + #[test] + fn reveal_values_false_omits_value_field_but_keeps_parameter_and_source() { + let dtos = StartupParameterDto::from_resolved(sample_merged(), false); + let json = serde_json::to_string(&dtos).expect("serialize"); + assert!( + !json.contains("\"value\""), + "anonymous view must not include any startup_parameter value, got {json}" + ); + assert!( + json.contains("\"parameter\":\"application_name\""), + "anonymous view must preserve parameter name, got {json}" + ); + assert!( + json.contains("\"source\":\"pool\""), + "anonymous view must preserve source label, got {json}" + ); + assert_eq!( + dtos.len(), + 2, + "redaction must not drop entries — anonymous view still tells the operator which keys are set" + ); + } } #[derive(Debug, Serialize)] diff --git a/src/web/routes/pools.rs b/src/web/routes/pools.rs index 94fcb3f8b..0c4e70bc7 100644 --- a/src/web/routes/pools.rs +++ b/src/web/routes/pools.rs @@ -1,10 +1,16 @@ //! GET /api/pools handler. +use crate::web::auth::Role; use crate::web::routes::collect::collect_pools; use crate::web::server::Response; -pub(crate) fn handle_pools() -> Response { - Response::ok_json(&collect_pools()) +pub(crate) fn handle_pools(role: Role) -> Response { + // Anonymous /api/pools must not leak operator-supplied + // startup_parameter values: they can carry tenant identifiers, audit + // tags, or accidental secrets. SSO/Admin callers keep the full view; + // anonymous viewers get parameter+source only. + let reveal_startup_values = role >= Role::Sso; + Response::ok_json(&collect_pools(reveal_startup_values)) } #[cfg(test)] @@ -13,10 +19,25 @@ mod tests { #[test] fn pools_response_is_200_json_with_array() { - let r = handle_pools(); + let r = handle_pools(Role::Admin); assert_eq!(r.status, 200); let body = std::str::from_utf8(&r.body).unwrap(); assert!(body.contains("\"ts\""), "body={body}"); assert!(body.contains("\"pools\""), "body={body}"); } + + #[test] + fn pools_response_hides_startup_value_for_anonymous() { + // Smoke check on the wired path: anonymous /api/pools must not + // surface a "value" field anywhere in the response body. The + // actual redaction logic lives in `StartupParameterDto::from_resolved` + // and is covered by the dedicated unit test in `dto.rs`. + let r = handle_pools(Role::Anonymous); + assert_eq!(r.status, 200); + let body = std::str::from_utf8(&r.body).unwrap(); + assert!( + !body.contains("\"value\""), + "anonymous /api/pools must not include any startup_parameter value, body={body}" + ); + } } diff --git a/src/web/server/http.rs b/src/web/server/http.rs index f9cdd3de6..1a21bbbc1 100644 --- a/src/web/server/http.rs +++ b/src/web/server/http.rs @@ -11,7 +11,7 @@ use tokio::io::{AsyncReadExt, BufReader, BufWriter}; use tokio::net::tcp::OwnedReadHalf; use tokio::net::TcpStream; -use crate::web::auth::{classify, AuthOutcome, Role}; +use crate::web::auth::{classify, AuthOutcome, Role, SsoTransportPolicy}; use crate::web::metrics::write_metrics_response; use super::router::{dispatch, unauthorized_for}; @@ -102,6 +102,15 @@ pub(super) async fn handle_connection(stream: TcpStream, opts: Arc) -> Response { } } -fn route_api(req: &ParsedRequest<'_>) -> Response { +fn route_api(req: &ParsedRequest<'_>, role: Role) -> Response { // ParsedRequest already split path on `?` — no further work here. let query = parse_query(req.query.unwrap_or("")); @@ -71,7 +71,7 @@ fn route_api(req: &ParsedRequest<'_>) -> Response { match req.path { "/api/version" => routes::version::handle_version(), "/api/overview" => routes::overview::handle_overview(), - "/api/pools" => routes::pools::handle_pools(), + "/api/pools" => routes::pools::handle_pools(role), "/api/clients" => routes::clients::handle_clients(&query), "/api/connections" => routes::connections::handle_connections(), "/api/databases" => routes::databases::handle_databases(), @@ -79,7 +79,7 @@ fn route_api(req: &ParsedRequest<'_>) -> Response { "/api/stats" => routes::stats::handle_stats(), "/api/users" => routes::users::handle_users(), "/api/auth_query" => routes::auth_query::handle_auth_query(), - "/api/config" => routes::config::handle_config(), + "/api/config" => routes::config::handle_config(role), "/api/log_level" => routes::log_level::handle_log_level(), "/api/pool_coordinator" => routes::pool_coordinator::handle_pool_coordinator(), "/api/pool_scaling" => routes::pool_scaling::handle_pool_scaling(), @@ -138,7 +138,7 @@ pub(super) fn dispatch( _ => unauthorized_for(req), }; } - return route_api(req); + return route_api(req, actual); } // SPA shell: serve the embedded bundle. Anything that is not /api or diff --git a/src/web/server/state.rs b/src/web/server/state.rs index 7b8ee8381..77506f1f0 100644 --- a/src/web/server/state.rs +++ b/src/web/server/state.rs @@ -41,6 +41,13 @@ pub struct WebServerOptions { /// "SSO grants read-only access" when the operator may actually /// land in Admin via group membership. pub sso_admin_groups_configured: bool, + /// `true` when `[web].sso_require_https = true`. The mux rejects + /// SSO credentials (Bearer / cookie / query) on requests that did + /// not arrive over HTTPS, identified by a trusted-proxy peer plus + /// `X-Forwarded-Proto: https`. Defaults to `false` so deployments + /// where the SSO proxy reaches pg_doorman over a private HTTP leg + /// keep working without configuration changes. + pub sso_require_https: bool, } impl WebServerOptions { @@ -69,6 +76,7 @@ impl WebServerOptions { sso_config_error, trusted_proxies: cfg.web.trusted_proxies.clone(), sso_admin_groups_configured: !cfg.web.sso_admin_groups.is_empty(), + sso_require_https: cfg.web.sso_require_https, } } } diff --git a/src/web/server/tests.rs b/src/web/server/tests.rs index ef4bbd58b..8a9b2cb90 100644 --- a/src/web/server/tests.rs +++ b/src/web/server/tests.rs @@ -15,6 +15,7 @@ fn opts(ui_active: bool, ui_anonymous: bool) -> WebServerOptions { sso_config_error: None, trusted_proxies: Vec::new(), sso_admin_groups_configured: false, + sso_require_https: false, } } @@ -41,6 +42,7 @@ fn req<'a>(method: &'a str, raw_path: &'a str) -> ParsedRequest<'a> { authorization: None, cookie: None, x_forwarded_for: None, + x_forwarded_proto: None, forwarded: None, accepts_gzip: false, accepts_json: false, @@ -57,6 +59,7 @@ fn req_json<'a>(method: &'a str, raw_path: &'a str) -> ParsedRequest<'a> { authorization: None, cookie: None, x_forwarded_for: None, + x_forwarded_proto: None, forwarded: None, accepts_gzip: false, accepts_json: true, diff --git a/src/web/server/wire.rs b/src/web/server/wire.rs index b73b1c210..c3057abcc 100644 --- a/src/web/server/wire.rs +++ b/src/web/server/wire.rs @@ -49,6 +49,11 @@ pub(super) struct ParsedRequest<'a> { /// Raw value of the `Forwarded:` header (RFC 7239). Same role as /// `x_forwarded_for`; both are walked. pub(super) forwarded: Option<&'a str>, + /// Raw value of the `X-Forwarded-Proto:` header, if present. Only + /// trusted when the TCP peer is in `[web].trusted_proxies`; used to + /// gate SSO credentials behind HTTPS when + /// `[web].sso_require_https = true`. + pub(super) x_forwarded_proto: Option<&'a str>, pub(super) accepts_gzip: bool, /// True when the request advertises `Accept: application/json`. The SPA /// `fetch()` wrapper sets this on every call; a browser hitting the URL @@ -80,6 +85,7 @@ impl<'a> ParsedRequest<'a> { let mut authorization = None; let mut cookie = None; let mut x_forwarded_for = None; + let mut x_forwarded_proto = None; let mut forwarded = None; let mut accepts_gzip = false; let mut accepts_json = false; @@ -98,6 +104,8 @@ impl<'a> ParsedRequest<'a> { cookie = Some(value); } else if let Some(value) = strip_header_prefix(line, "X-Forwarded-For") { x_forwarded_for = Some(value); + } else if let Some(value) = strip_header_prefix(line, "X-Forwarded-Proto") { + x_forwarded_proto = Some(value); } else if let Some(value) = strip_header_prefix(line, "Forwarded") { forwarded = Some(value); } else if let Some(value) = strip_header_prefix(line, "Accept-Encoding") { @@ -121,6 +129,7 @@ impl<'a> ParsedRequest<'a> { authorization, cookie, x_forwarded_for, + x_forwarded_proto, forwarded, accepts_gzip, accepts_json, diff --git a/src/web/tests.rs b/src/web/tests.rs index f8a28e021..20694b381 100644 --- a/src/web/tests.rs +++ b/src/web/tests.rs @@ -33,6 +33,7 @@ fn opts(ui_active: bool, ui_anonymous: bool) -> WebServerOptions { sso_config_error: None, trusted_proxies: Vec::new(), sso_admin_groups_configured: false, + sso_require_https: false, } } @@ -54,6 +55,7 @@ fn opts_with_sso(ui_anonymous: bool) -> WebServerOptions { sso_config_error: None, trusted_proxies: Vec::new(), sso_admin_groups_configured: false, + sso_require_https: false, } } diff --git a/tests/auth_query_startup_params_fixture.sql b/tests/auth_query_startup_params_fixture.sql new file mode 100644 index 000000000..c58fb0487 --- /dev/null +++ b/tests/auth_query_startup_params_fixture.sql @@ -0,0 +1,34 @@ +-- Fixtures for auth_query scenarios that exercise the optional +-- startup_parameters JSON column. +-- +-- Layout matches auth_query_passthrough_fixture.sql with one extra +-- column. Existing scenarios that select only (username, password) keep +-- working: the new column is opt-in at the auth_query SQL level, not +-- at the table schema level. + +CREATE TABLE IF NOT EXISTS auth_users ( + username TEXT NOT NULL, + password TEXT, + startup_parameters TEXT +); + +SET password_encryption = 'md5'; + +-- User whose per-user startup_parameters override pool defaults. +-- Used by the passthrough-cascade scenario: pool sets plan_cache_mode +-- to 'auto', auth_query column overrides it to 'force_custom_plan' +-- for this user only. +CREATE USER sp_tuned_user WITH PASSWORD 'tuned_pass'; +INSERT INTO auth_users + SELECT rolname, rolpassword, '{"plan_cache_mode":"force_custom_plan"}' + FROM pg_authid WHERE rolname = 'sp_tuned_user'; + +-- User with NULL startup_parameters: same fixture covers the +-- "column present but no per-user override" baseline. +CREATE USER sp_plain_user WITH PASSWORD 'plain_pass'; +INSERT INTO auth_users + SELECT rolname, rolpassword, NULL + FROM pg_authid WHERE rolname = 'sp_plain_user'; + +GRANT ALL ON DATABASE postgres TO sp_tuned_user; +GRANT ALL ON DATABASE postgres TO sp_plain_user; diff --git a/tests/bdd/features/startup-parameters.feature b/tests/bdd/features/startup-parameters.feature new file mode 100644 index 000000000..15b047c06 --- /dev/null +++ b/tests/bdd/features/startup-parameters.feature @@ -0,0 +1,554 @@ +@startup-parameters +Feature: Per-pool startup_parameters + pg_doorman sends configured PostgreSQL runtime parameters in every + backend StartupMessage. Values come from general defaults, per-pool + overrides, and (in auth_query passthrough mode) per-user overrides + from an optional JSON column. + + Scenario: general.startup_parameters apply on a fresh backend + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [general.startup_parameters] + statement_timeout = "12345" + + [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 = 2 + """ + Then psql query "SHOW statement_timeout" via pg_doorman as user "example_user_1" to database "example_db" with password "test" returns "12345ms" + + Scenario: pool.startup_parameters overrides general per-key + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [general.startup_parameters] + statement_timeout = "10001" + lock_timeout = "5001" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + statement_timeout = "23456" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + Then psql query "SHOW statement_timeout" via pg_doorman as user "example_user_1" to database "example_db" with password "test" returns "23456ms" + And psql query "SHOW lock_timeout" via pg_doorman as user "example_user_1" to database "example_db" with password "test" returns "5001ms" + + Scenario: auth_query passthrough per-user JSON column overrides pool default + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all postgres 127.0.0.1/32 trust + host all all 127.0.0.1/32 md5 + host all all ::1/128 trust + """ + And fixtures from "tests/auth_query_startup_params_fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.postgres.startup_parameters] + plan_cache_mode = "auto" + + [pools.postgres.auth_query] + query = "SELECT username, password, startup_parameters FROM auth_users WHERE username = $1" + user = "postgres" + password = "" + workers = 1 + pool_size = 5 + cache_ttl = "1h" + cache_failure_ttl = "30s" + min_interval = "0s" + """ + Then psql query "SHOW plan_cache_mode" via pg_doorman as user "sp_tuned_user" to database "postgres" with password "tuned_pass" returns "force_custom_plan" + And psql query "SHOW plan_cache_mode" via pg_doorman as user "sp_plain_user" to database "postgres" with password "plain_pass" returns "auto" + + Scenario: startup_parameters application_name wins over pool default + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + application_name = "doorman_default_app" + + [pools.example_db.startup_parameters] + application_name = "operator_supplied_app" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + Then psql query "SHOW application_name" via pg_doorman as user "example_user_1" to database "example_db" with password "test" returns "operator_supplied_app" + + Scenario: reserved-key in startup_parameters is rejected at config validation + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [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 = 2 + """ + # pg_doorman is running with a valid config. Now overwrite the file with a + # reserved-key violation and re-run the binary in -t mode to confirm config + # validation rejects the change before any worker can pick it up. + When we overwrite pg_doorman config file with: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + user = "evil_override" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + # `pg_doorman -t` exits non-zero when validation fails; the precise + # reserved-key message lands in a logger that has not been initialised at + # this point in startup, so we assert on the exit code only. The narrower + # reserved-key behaviour is exercised by unit tests in src/config/tests.rs. + And I run shell command "${DOORMAN_BINARY} -t ${DOORMAN_CONFIG_FILE}" + Then the command should fail + + Scenario: unknown GUC produces a backend startup error with warn log + Given pg_doorman log capture enabled + And PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + nonexistent_guc_zzz = "value" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + Then psql connection to pg_doorman as user "example_user_1" to database "example_db" with password "test" fails + And pg_doorman log contains "PG rejected operator-supplied startup parameter" + And pg_doorman log contains "nonexistent_guc_zzz" + + Scenario: every subsequent connect fails the same way until the operator fixes the config + Given pg_doorman log capture enabled + And PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + nonexistent_guc_yyy = "value" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + # PostgreSQL rejects the unknown GUC at backend startup; pg_doorman + # returns that rejection to the client. The next connect repeats the + # same failure: pg_doorman does not strip the key or disable it for the + # pool. The operator must fix the parameter in the config. + Then psql connection to pg_doorman as user "example_user_1" to database "example_db" with password "test" fails + Then psql connection to pg_doorman as user "example_user_1" to database "example_db" with password "test" fails + And pg_doorman log contains "nonexistent_guc_yyy" + + Scenario: RESET ALL restores startup_parameters defaults + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "session" + + [pools.example_db.startup_parameters] + plan_cache_mode = "force_custom_plan" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + When I run shell command: + """ + PGPASSWORD=test PGSSLMODE=disable psql -h 127.0.0.1 -p ${DOORMAN_PORT} \ + -U example_user_1 -d example_db -A -t <<'SQL' + SET plan_cache_mode = 'auto'; + SHOW plan_cache_mode; + RESET ALL; + SHOW plan_cache_mode; + SQL + """ + Then the command should succeed + # The first SHOW returns the client-set value, the second returns the + # Startup default configured by pg_doorman (RESET ALL falls back to reset_val, + # which is the value PG saw in StartupMessage). + And the command output should contain "auto" + And the command output should contain "force_custom_plan" + + Scenario: DISCARD ALL restores startup_parameters defaults + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "session" + + [pools.example_db.startup_parameters] + plan_cache_mode = "force_custom_plan" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + When I run shell command: + """ + PGPASSWORD=test PGSSLMODE=disable psql -h 127.0.0.1 -p ${DOORMAN_PORT} \ + -U example_user_1 -d example_db -A -t <<'SQL' + SET plan_cache_mode = 'auto'; + SHOW plan_cache_mode; + DISCARD ALL; + SHOW plan_cache_mode; + SQL + """ + Then the command should succeed + And the command output should contain "auto" + And the command output should contain "force_custom_plan" + + Scenario: RELOAD recycles pool when pool.startup_parameters changes + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + statement_timeout = "11111" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + Then psql query "SHOW statement_timeout" via pg_doorman as user "example_user_1" to database "example_db" with password "test" returns "11111ms" + When we overwrite pg_doorman config file with: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + statement_timeout = "22222" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + And we create admin session "adm1" to pg_doorman as "admin" with password "admin" + And we execute "RELOAD" on admin session "adm1" + And we sleep for 500 milliseconds + Then psql query "SHOW statement_timeout" via pg_doorman as user "example_user_1" to database "example_db" with password "test" returns "22222ms" + + Scenario: dedicated auth_query mode logs a warning and ignores per-user startup_parameters + Given pg_doorman log capture enabled + And PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all postgres 127.0.0.1/32 trust + host all all 127.0.0.1/32 md5 + host all all ::1/128 trust + """ + And fixtures from "tests/auth_query_startup_params_fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.postgres.auth_query] + query = "SELECT username, password, startup_parameters FROM auth_users WHERE username = $1" + user = "postgres" + password = "" + workers = 1 + pool_size = 5 + cache_ttl = "1h" + cache_failure_ttl = "30s" + min_interval = "0s" + server_user = "postgres" + server_password = "" + """ + # Authentication still succeeds. Dedicated mode ignores the per-user + # JSON-column value because every dynamic user shares one backend + # identity, and the warning makes that ignored value visible to the + # operator. + Then psql connection to pg_doorman as user "sp_tuned_user" to database "postgres" with password "tuned_pass" succeeds + And pg_doorman log contains "per-user startup_parameters ignored in dedicated" + + Scenario: auth_query SQL without the startup_parameters column keeps working + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all postgres 127.0.0.1/32 trust + host all all 127.0.0.1/32 md5 + host all all ::1/128 trust + """ + And fixtures from "tests/auth_query_passthrough_fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [pools.postgres] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.postgres.auth_query] + query = "SELECT username, password FROM auth_users WHERE username = $1" + user = "postgres" + password = "" + workers = 1 + pool_size = 5 + cache_ttl = "1h" + cache_failure_ttl = "30s" + min_interval = "0s" + """ + Then psql connection to pg_doorman as user "pt_md5_user" to database "postgres" with password "md5_pass" succeeds + + Scenario: admin SHOW STARTUP_PARAMETERS lists resolved parameters per pool + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + host all all ::1/128 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 md5" + + [general.startup_parameters] + statement_timeout = "10s" + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "transaction" + + [pools.example_db.startup_parameters] + plan_cache_mode = "force_custom_plan" + + [[pools.example_db.users]] + username = "example_user_1" + password = "md58a67a0c805a5ee0384ea28e0dea557b6" + pool_size = 2 + """ + When I run shell command: + """ + PGPASSWORD=admin PGSSLMODE=disable psql -h 127.0.0.1 -p ${DOORMAN_PORT} \ + -U admin -d pgdoorman -A -t -c 'SHOW STARTUP_PARAMETERS' + """ + Then the command should succeed + # Default psql -A -t output is pipe-delimited: + # user|database|parameter|value|source|state. + # The general baseline shows up as source=general, the pool override + # as source=pool, and the runtime state stays `applied` because the + # cascade fits the operator budget for this fixture. + And the command output should contain "statement_timeout|10s|general|applied" + And the command output should contain "plan_cache_mode|force_custom_plan|pool|applied" diff --git a/tests/bdd/pool_bench_helper.rs b/tests/bdd/pool_bench_helper.rs index a7423eaec..3fa8736fe 100644 --- a/tests/bdd/pool_bench_helper.rs +++ b/tests/bdd/pool_bench_helper.rs @@ -75,6 +75,8 @@ async fn setup_internal_pool(world: &mut DoormanWorld, size: usize, _mode: Strin Duration::from_secs(10), // query_wait_timeout false, // session_mode None, // fallback_state + std::sync::Arc::new(std::collections::BTreeMap::new()), + std::sync::Arc::new(std::collections::BTreeMap::new()), ); // Create Pool with configuration @@ -427,6 +429,8 @@ async fn setup_internal_pool_with_lifetimes( Duration::from_secs(10), false, None, + std::sync::Arc::new(std::collections::BTreeMap::new()), + std::sync::Arc::new(std::collections::BTreeMap::new()), ); let config = PoolConfig {