ENG-3566: Refactor engine creation to use SQLAlchemy creator pattern

[erosselli]

Part of ENG-3566

Description Of Changes

Refactors all 6 database engines to use SQLAlchemy's creator pattern instead of baking credentials into connection URIs. The creator callable is invoked by the pool on every new connection, resolving credentials at connect time rather than engine construction time. This is the structural seam required for dynamic credential rotation via AWS Secrets Manager (design doc: #8016).

Today the creators read from static config (CONFIG.database). In follow-up work, the credential helpers will be swapped to call provider.get_secret() — no further engine changes needed.

Also updates the design doc to remove the lazy factory requirement, since the creator closure captures a provider reference (not credentials), making it independent of when the engine is constructed.

Code Changes

• database_settings.py — Add raw_password / raw_readonly_password properties that reverse quote_plus escaping for direct psycopg2.connect() / asyncpg.connect() use
• engine_creators.py (new) — make_sync_creator / make_async_creator factories that capture per-engine config (keepalives, SSL) in closures while resolving credentials per-connection. Includes credential helpers and asyncpg param/SSL conversion
• session.py — Add optional creator parameter to get_db_engine(). When provided, uses dialect-only URL and skips connect_args (creator handles them)
• ctl_session.py — Switch all 3 engines (async, readonly async, sync) from URI-based to creator-based. SSL context and asyncpg param handling moved into engine_creators.py
• session_management.py — Switch API and readonly sync engines to use make_sync_creator with keepalive connect_args
• tasks/__init__.py — Switch Celery task engine to use make_sync_creator with keepalive connect_args
• dynamic-database-credentials.md — Remove lazy factory requirement from design doc

Steps to Confirm

1. Verify all existing database connections work: hit GET /health/database and confirm all pools are healthy
2. Run existing test suites to confirm no regressions (public API surface is unchanged)
3. Verify that the async creator correctly wraps connections using SA internals (AsyncAdapt_asyncpg_connection)

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
  • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • No UX review needed
• Followup issues:
  • Followup issues created
• Database migrations:
  • No migrations
• Documentation:
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	May 11, 2026 9:17pm
fides-privacy-center	Ignored		May 11, 2026 9:17pm

[codecov]

Codecov Report

❌ Patch coverage is 93.68421% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.45%. Comparing base (26cf81d) to head (37ce205).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/fides/common/engine_creators.py	89.83%	3 Missing and 3 partials ⚠️

❌ Your patch check has failed because the patch coverage (93.68%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8148      +/-   ##
==========================================
+ Coverage   85.40%   85.45%   +0.04%     
==========================================
  Files         649      650       +1     
  Lines       42283    42363      +80     
  Branches     4960     4971      +11     
==========================================
+ Hits        36112    36201      +89     
+ Misses       5063     5054       -9     
  Partials     1108     1108              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[adamsachs]

nice work, this looks good to me! nice job carving out a clean abstraction/hook for this.

a couple of nits and then my claude review does have one callout that i think is worth looking into, will post that as a quick followup

[adamsachs]

nice documentation here.

nit: maybe worth mentioning that they must stay lightweight? (or at least i assume so 😄)

[adamsachs]

interesting, claude is calling something out here and i think it's valid? in that we should probably at least call this out as a change to how we expect encoded passwords to be handled...

will post directly from my claude review

[adamsachs]

[Claude]: escape_password runs quote_plus unconditionally, and raw_password reverses it with unquote_plus. For passwords containing literal @, #, %, , etc., this round-trips correctly — the parametrized tests confirm that.

But the previous URI-based code path treated password as the already-escaped value to be embedded in a URI. If a deployment has historically set FIDES__DATABASE__PASSWORD=foo%40bar (i.e., the user manually URL-encoded @ as %40), the old URI path used foo%40bar in the URI, which psycopg2/asyncpg decode to foo@bar at parse time — correct.

With the new creator path, that same value flows through:

1. escape_password runs quote_plus("foo%40bar") → "foo%2540bar" (the % itself gets escaped).
2. raw_password runs unquote_plus("foo%2540bar") → "foo%40bar" (the literal string).
3. psycopg2.connect(password="foo%40bar") sends foo%40bar as the password, not foo@bar.

This is a silent behavior change for any user who was already pre-encoding their password in env vars or config files. It will only manifest as auth failures in production.

Options:

• Document this clearly as a breaking change for pre-encoded passwords, OR
• Make escape_password idempotent (e.g., detect already-encoded values), OR
• Add a release note + migration step.

At minimum, add a test that documents the round-trip behavior for a password literally containing %XX sequences so the contract is explicit.

[erosselli]

This isn't a behavior change. Both the old URI path and the new creator path produce the same result for all cases, including the pre-encoded foo%40bar scenario.

escape_password has always run quote_plus unconditionally on the raw config value. PostgresDsn.build() does not re-encode, so the old URI path was:

1. escape_password("foo%40bar") → "foo%2540bar" (stored)
2. PostgresDsn.build(password="foo%2540bar") → embeds as-is in URI
3. psycopg2 parses URI, URL-decodes → "foo%40bar" sent to Postgres

The new creator path:

1. Same escape_password → "foo%2540bar" (stored)
2. raw_password → unquote_plus("foo%2540bar") → "foo%40bar"
3. psycopg2.connect(password="foo%40bar") → "foo%40bar" sent to Postgres

Same result in both paths. The pre-encoding case was already "broken" (or rather: passwords are expected to be raw, not pre-encoded). Will add a test documenting this contract.