ENG-3566: Add DBCredentialProvider for dynamic credential resolution#8175
Merged
Conversation
Add DBCredentialProvider class that sits between SQLAlchemy engine creators and the SecretProvider abstraction. Resolves credentials from static config or AWS Secrets Manager, retries once on auth failure during credential rotation, and sanitizes all connection exceptions to prevent credential leakage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The build_field_documentation function assumed every anyOf entry has a "type" key, but Pydantic uses "$ref" for nested models. Skip $ref entries to avoid KeyError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8175 +/- ##
==========================================
+ Coverage 85.45% 85.54% +0.09%
==========================================
Files 650 654 +4
Lines 42363 42636 +273
Branches 4971 5008 +37
==========================================
+ Hits 36200 36475 +275
- Misses 5054 5055 +1
+ Partials 1109 1106 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- aws_secrets_manager is None when provider is static - region is required (no default) when AWS is configured - Before-validator constructs from env vars when needed - Fix config doc generator for Optional nested model $ref Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace asserts with explicit raises for cases where upstream validators should prevent None but callers may bypass validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 tasks
- Add reset_secret_provider() for test isolation - Preserve original ValidationError with raise...from exc - Add comment explaining time.sleep safety in greenlet context - Add string fallback test coverage for _is_auth_error - Demote fetch log to DEBUG, keep success at INFO - Remove unused Any import from static_provider - Add __all__ to db_credential_provider Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JadeCara
approved these changes
May 13, 2026
Contributor
JadeCara
left a comment
JadeCara
left a comment
There was a problem hiding this comment.
Only one note on some drift between the plan and implementation. Looks clean!
Comment on lines
+112
to
+119
| else: | ||
| if not self.database.credential_secret_id: | ||
| raise ValueError( | ||
| f"secrets.provider is {self.secrets.provider!r} but " | ||
| "database.credential_secret_id is not set. " | ||
| "Provide the secret name/ARN containing database credentials." | ||
| ) | ||
| return self |
Contributor
There was a problem hiding this comment.
I think I remember this being a warning to not block start up in the design doc. Did that change?
Contributor
Author
There was a problem hiding this comment.
ah maybe, but if something is misconfigured I think I'd rather fail noisly than log a silent warning. This should only scream if you've set FIDES__SECRETS__PROVIDER to aws but haven't configured the credential_secret_id for the DB
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
2 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.
Ticket ENG-3566
Description Of Changes
Adds
DBCredentialProvider, the layer between SQLAlchemy engine creators and theSecretProviderabstraction. This is the foundation for dynamic database credentials via AWS Secrets Manager — it resolves credentials, handles auth-failure retry during rotation, and sanitizes connection exceptions to prevent credential leakage.This PR adds the class and all supporting config/provider changes but does not wire it into the engine creators yet. The wiring will follow in a separate PR.
Code Changes
DBCredentialProviderclass withget_credentials(),connect_with_retry(), auth error detection (SQLSTATE + psycopg2 string fallback), andSanitizedConnectionErrorStaticSecretProviderrefactored to read and cache DB credentials from CONFIG at construction timeget_secret_provider()singleton inconfig/secrets/__init__.pyfor shared provider cachecredential_secret_idandreadonly_credential_secret_idfields added toDatabaseSettings@model_validatoronFidesConfigfor cross-section config validation (static + secret_id, aws + missing secret_id)aws_secrets_managermadeOptionalonSecretsSettingswith its own validatorcreate_secret_providerfactory simplified (no morestatic_secretsparam)AWSSecretsManagerProviderfor fetch/cache eventsSteps to Confirm
nox -s "pytest(misc-unit)"— all config, secrets, and db_credential_provider tests passnox -s "pytest(lib)"— engine creator tests passGET /health/databasereturns healthyPre-Merge Checklist
CHANGELOG.mdupdated