docs: document pg_hba_conf/pg_ident_conf + e2e#402
Conversation
Document the user-managed `pg_hba_conf` and `pg_ident_conf` fields, and add an end-to-end test that confirms the entries reach the running Postgres. - Add an "Authentication Rules" subsection under "Customizing Database Configuration" in the create-a-database guide, covering both fields, the per-node prepend behavior, the `password_encryption` interaction, and the Swarm source-IP nuance. - Add a single-node e2e test that creates a database with database- and node-level entries and asserts, via Postgres' own pg_hba_file_rules and pg_ident_file_mappings views, that they loaded without error and in the right order, then updates an entry and confirms it applies via a reload (pg_postmaster_start_time unchanged), not a restart. The test stays intentionally small: single-node for speed, no connection matrix, and no replication re-assertion, since those are covered elsewhere. PLAT-629
|
Warning Review limit reached
More reviews will be available in 5 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds documentation and end-to-end test coverage for PostgreSQL client authentication customization. The documentation subsection explains configuring ChangesPostgreSQL Authentication Configuration
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Up to standards ✅🟢 Issues
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/pg_hba_test.go (1)
154-155: ⚡ Quick winClarify the error message.
The assertion checks that the start times are equal (reload occurred), but the error message says "start time changed". This is confusing because if the assertion fails, it means the times are NOT equal, indicating Postgres restarted.
📝 Suggested improvement for clarity
require.True(t, postmasterStartTime.Equal(after), - "Postgres should reload, not restart (start time changed)") + "Postgres restarted instead of reloading (start time changed)")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/pg_hba_test.go` around lines 154 - 155, The assertion using require.True(t, postmasterStartTime.Equal(after), ...) has a confusing message; update the require.True call’s error string (the one paired with postmasterStartTime.Equal(after)) to clearly state that a changed start time means Postgres restarted, e.g. "Postgres restarted: start time changed (expected reload)". Locate the require.True call with postmasterStartTime.Equal(after) in e2e/pg_hba_test.go and replace the current message to reflect that failure indicates a restart rather than a reload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/using/create-db.md`:
- Line 187: The sentence claiming "default `md5`" is ambiguous — change the docs
text to explicitly state that `password_encryption` is defaulted to `md5` by the
control plane (not upstream Postgres), referencing the control-plane default in
server/internal/postgres/gucs.go and the Patroni config generator fallback;
update the phrasing in the create-db documentation to something like "the
control plane defaults `password_encryption` to `md5` (see
server/internal/postgres/gucs.go and Patroni config generator fallback), which
may differ from PostgreSQL upstream defaults" so readers aren’t confused by
upstream Postgres’ `scram-sha-256` default.
---
Nitpick comments:
In `@e2e/pg_hba_test.go`:
- Around line 154-155: The assertion using require.True(t,
postmasterStartTime.Equal(after), ...) has a confusing message; update the
require.True call’s error string (the one paired with
postmasterStartTime.Equal(after)) to clearly state that a changed start time
means Postgres restarted, e.g. "Postgres restarted: start time changed (expected
reload)". Locate the require.True call with postmasterStartTime.Equal(after) in
e2e/pg_hba_test.go and replace the current message to reflect that failure
indicates a restart rather than a reload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35629252-9a52-44b8-ab1a-fa5c4a40cb05
📒 Files selected for processing (2)
docs/using/create-db.mde2e/pg_hba_test.go
2 participants
Document the user-managed
pg_hba_confandpg_ident_conffields, and add an end-to-end test that confirms the entries reach the running Postgres.password_encryptioninteraction, and the Swarm source-IP nuance.The test stays intentionally small: single-node for speed, no connection matrix, and no replication re-assertion, since those are covered elsewhere.
PLAT-629
Summary
Adds user-facing documentation for the
pg_hba_conf/pg_ident_confdatabase-spec fields and a small end-to-end test that proves user entries reach the running Postgres and that updates apply via reload, not restart. This is the docs-and-testing follow-up to the spec and generator work.Changes
docs/using/create-db.md): add an "Authentication Rules" subsection under "Customizing Database Configuration" covering both fields, per-node prepend, thepassword_encryptioninteraction, and the Swarm source-IP nuance, with acurlexample.e2e/pg_hba_test.go): new single-node test that creates a database with database- and node-levelpg_hba_confplus apg_ident_confmapping, then queriespg_hba_file_rules/pg_ident_file_mappingsto confirm Postgres loaded them with no parse error and in prepend order, and updates an entry to confirm a reload (unchangedpg_postmaster_start_time) rather than a restart.Testing
Manual: ran the e2e live against a dev cluster — the entries appear in the container's
pg_hba.conf/pg_ident.conf, load without error, and the update is applied via SIGHUP reload (same container, unchanged postmaster start time).