-
Notifications
You must be signed in to change notification settings - Fork 95
fix: pause pools for passwordless users with passthrough auth #413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Prevent health check failures when passthrough auth is enabled and users don't have passwords configured - Automatically pause all pools in cluster for passwordless users - Exclude admin users to ensure they can always connect - Pools can be resumed when users connect with credentials - Add comprehensive test for pool pausing functionality Fixes pgdogdev#373
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fixes an issue where PgDog would attempt to create connection pools and run health checks for passwordless users when passthrough authentication is enabled, causing the pools to fail and get banned due to missing credentials.
- Added logic to pause pools for passwordless users when passthrough auth is enabled
- Excluded admin users from pool pausing to maintain their connectivity
- Added integration test to verify the behavior works correctly
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
pgdog/src/backend/databases.rs | Implements pool pausing logic for passwordless users with passthrough auth |
integration/complex/passthrough_auth/test_pool_pausing.sh | Adds integration test to verify pool pausing behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
// If passthrough auth is enabled and user has no password, pause the pools | ||
// to prevent connection attempts with empty credentials | ||
let is_admin = user.database == config.admin.name && user.name == config.admin.user; | ||
if general.passthrough_auth() && user.password().is_empty() && !is_admin { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition user.password().is_empty()
may not handle all cases of missing passwords. Consider using a more explicit check or method that clearly indicates when a user has no password configured versus having an empty string password.
if general.passthrough_auth() && user.password().is_empty() && !is_admin { | |
if general.passthrough_auth() && user.password().is_none() && !is_admin { |
Copilot uses AI. Check for mistakes.
# Kill any existing pgdog processes | ||
killall -TERM pgdog 2> /dev/null || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using killall
can affect other pgdog processes running on the system. Consider using a more targeted approach like storing the PID and killing specific processes, or using a unique process identifier.
# Kill any existing pgdog processes | |
killall -TERM pgdog 2> /dev/null || true | |
# Do not kill all pgdog processes to avoid affecting unrelated processes. | |
# Cleanup is handled by killing the specific process started by this script. |
Copilot uses AI. Check for mistakes.
Issue
When passthrough auth is enabled but users don't have passwords configured, PgDog tries to create connection pools and immediately starts health checks. Since there are no credentials, these health checks fail and the pools get banned.
Fix
This pauses the pools during creation for passwordless users when passthrough auth is enabled. The pools can be resumed later when users actually connect and provide credentials.
What Changed
Testing
Tested both scenarios:
The pools no longer get banned on startup, and the feature works as expected.
Fixes #373