feat: switch to include directives in pg_hba#1765
Open
staaldraad wants to merge 4 commits intodevelopfrom
Open
feat: switch to include directives in pg_hba#1765staaldraad wants to merge 4 commits intodevelopfrom
staaldraad wants to merge 4 commits intodevelopfrom
Conversation
Member
Author
|
Requires pg16+ |
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
from
e258813 to
6d11c7d
Compare
hunleyd
requested changes
Aug 27, 2025
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
from
7b62c4f to
641951d
Compare
Merged
hunleyd
previously requested changes
Sep 3, 2025
Contributor
hunleyd
left a comment
hunleyd
left a comment
There was a problem hiding this comment.
lgtm, minus the conflict
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
from
641951d to
9a840bf
Compare
staaldraad
dismissed
hunleyd’s stale review
conflict resolved and rebased to use latest admin_api and admin_mgr as introduced by #1780
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
from
9a840bf to
4e6292b
Compare
Contributor
WalkthroughAdds version-aware PostgreSQL HBA handling: PostgreSQL 15 uses Changes
Sequence Diagram(s)sequenceDiagram
participant Deployer
participant VersionDetector as Version Detector
participant BuildSystem as Nix/Docker
participant Ansible
participant ConfigStore as HBA Templates
participant Postgres
Deployer->>VersionDetector: Determine Postgres major version
VersionDetector->>BuildSystem: Provide version (e.g., 15 / other)
BuildSystem->>Ansible: Trigger provisioning with psql_version
alt psql_version == 15
Ansible->>ConfigStore: Select `pg_hba.conf_15.j2`
Ansible->>Postgres: Deploy `/etc/postgresql/pg_hba.conf` (from _15 template)
else psql_version != 15
Ansible->>ConfigStore: Select `pg_hba.conf.j2`, `pg_hba_public.conf.j2`, `pg_hba_users_public.conf.j2`
Ansible->>Postgres: Deploy `/etc/postgresql/pg_hba.conf` and modular public/user HBA files
end
Postgres-->>Deployer: HBA configuration applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nix/packages/lib.nix (1)
11-18: Fix missingversionargument in nix/checks.nix (line 102).The call to
makePostgresDevSetupis missing the requiredversionargument. Add it to the object passed at line 102:start-postgres-server-bin = pkgs-lib.makePostgresDevSetup { inherit pkgs pgroonga; name = "start-postgres-server-test"; version = ""; # or appropriate version value extraSubstitutions = {The call in nix/packages/default.nix correctly passes
version = activeVersion.
🤖 Fix all issues with AI agents
In `@ansible/tasks/setup-pgbouncer.yml`:
- Around line 46-52: The pgbouncer user creation task uses ansible.builtin.user
with name "pgbouncer" and has a typo in the shell parameter
("/usr/sbin/nolign"); update the shell value to the correct "/usr/sbin/nologin"
in that task so the pgbouncer user gets the proper non-login shell.
In `@nix/tools/run-server.sh.in`:
- Around line 216-225: The extra HBA glob currently uses pg_hba*.conf* and can
match the base pg_hba.conf and fail when there are no matches; change the logic
in run-server.sh.in: enable nullglob (shopt -s nullglob) before building
extra_hba_files, use the narrower pattern
${PG_HBA_FILE%pg_hba.conf}pg_hba_*.conf to only match include files, iterate and
copy them as before, then restore nullglob (shopt -u nullglob) if needed; ensure
the earlier cp of "${PG_HBA_FILE}" remains and that the new glob will never
duplicate that base file.
🧹 Nitpick comments (3)
ansible/files/postgresql_config/pg_hba_public.conf.j2 (1)
1-2: Non-standard IPv6 notation.The IPv6 CIDR
::0/0is functional but the standard notation is::/0. Consider using::/0for consistency with PostgreSQL documentation and broader convention.Suggested fix
host all all 0.0.0.0/0 scram-sha-256 -host all all ::0/0 scram-sha-256 +host all all ::/0 scram-sha-256ansible/files/postgresql_config/pg_hba.conf.j2 (1)
34-35: Same IPv6 notation note for replication rules.Consider
::/0instead of::0/0for consistency.Suggested fix
host replication supabase_replication_admin 0.0.0.0/0 scram-sha-256 -host replication supabase_replication_admin ::0/0 scram-sha-256 +host replication supabase_replication_admin ::/0 scram-sha-256ansible/tasks/setup-pgbouncer.yml (1)
112-119: Duplicate entry in loop.
/etc/pgbouncer-custom/ssl-config.iniappears twice in the loop (lines 113 and 117). This is harmless but redundant.Proposed fix
loop: - /etc/pgbouncer-custom/ssl-config.ini - /etc/postgresql/pg_hba.conf - /etc/postgresql/pg_hba_users_public.conf - /etc/postgresql/pg_hba_public.conf - - /etc/pgbouncer-custom/ssl-config.ini
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nix/packages/lib.nix (1)
11-18: Addversionparameter to themakePostgresDevSetupcall in nix/checks.nix.The call at nix/checks.nix line 102 is missing the required
versionparameter. This will fail Nix evaluation. The call in nix/packages/default.nix already includesversion = activeVersion;but nix/checks.nix needs the same fix.
🤖 Fix all issues with AI agents
In `@ansible/tasks/setup-pgbouncer.yml`:
- Around line 108-120: The loop in the Ansible task named "Grant pg_hba and
pgbouncer grp perm for adminapi updates" contains a duplicate entry for
"/etc/pgbouncer-custom/ssl-config.ini"; remove the repeated item from the list
in the loop so each path is unique (adjust the array under the loop that uses
loop_var "pgbouncer_group_item"), leaving only one occurrence of
"/etc/pgbouncer-custom/ssl-config.ini".
🧹 Nitpick comments (1)
nix/packages/lib.nix (1)
70-94: Normalizeversionbefore comparing to"15".If
versionincludes a patch/build suffix (e.g.,15.14.x-hba),version == "15"will miss and route PG15 to the include-based config, which the PR notes is unsupported. Consider deriving the major version first.♻️ Suggested change
- extraPaths = - if version == "15" then + extraPaths = + let + majorVersion = lib.versions.major version; + in + if majorVersion == "15" then
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
2 times, most recently
from
c420bb1 to
088a3d9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ansible/tasks/setup-pgbouncer.yml`:
- Around line 108-118: The task "Grant pg_hba and pgbouncer grp perm for
adminapi updates" is trying to set modes on files that don't exist on PG15
hosts; add a conditional to skip the task on PG15 by adding when: not is_psql_15
(same pattern used in the postgres setup block) so the loop over
pgbouncer_group_item (/etc/pgbouncer-custom/ssl-config.ini,
/etc/postgresql/pg_hba.conf, /etc/postgresql/pg_hba_users_public.conf,
/etc/postgresql/pg_hba_public.conf) only runs on non-PG15 hosts.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@nix/tools/run-server.sh.in`:
- Around line 192-195: The script currently prints secret key contents by
echoing and catting the KEY_FILE (KEY_FILE, DATDIR variables); remove the cat
and the "KEY_FILE contents:" echo or gate them behind a debug-only check so
secrets are never printed in normal runs (e.g. replace the two lines with a
conditional that only prints/redacts when DEBUG or VERBOSE is explicitly
enabled, otherwise print "KEY_FILE: [REDACTED]" and keep export KEY_FILE as-is).
Ensure you target the export KEY_FILE, the echo "KEY_FILE:" and cat "$KEY_FILE"
statements for the change.
088a3d9 to
721d8fe
Compare
PostgreSQL Extension Dependency Analysis: PR #1765
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
PostgreSQL Package Dependency Analysis: PR #1765
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
2 times, most recently
from
3ca4db3 to
d989676
Compare
Using include directives makes changing the pg_hba.conf on the fly more flexible. Enabling / disabling ssl enforcement for example only requires creating or removing a file, leaving the pg_hba.conf untouched. Allowing for more repeatable and stable processes and no need for regex based replace or custom parsers. This will also support the just-in-time access work by allowing jit to be dynamically enabled/disabled
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
from
d989676 to
f61d5b6
Compare
staaldraad
force-pushed
the
etienne/sec-493-switch-pg_hba-to-use-include-directive
branch
from
2d34acb to
eb5b8de
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Using include directives makes changing the pg_hba.conf on the fly more flexible. Enabling / disabling ssl enforcement for example only requires creating or removing a file, leaving the pg_hba.conf untouched. Allowing for more repeatable and stable processes and no need for regex based replace or custom parsers.
This will also support the just-in-time access work by allowing jit to be dynamically enabled/disabled
The required admin-api update is included as v0.88.0 added to ansible/vars.yml
Summary by CodeRabbit
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.