Merge pull request #246 from lsst-sqre/tickets/DM-44444

DM-44444: Allow database passwords to be SecretStrs

.github/workflows/ci.yaml

@@ -45,12 +45,13 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      # Temporarily pin requests because 2.32.0 breaks docker 6.1.3.
       - name: Run tox
         uses: lsst-sqre/run-tox@v1
         with:
           python-version: ${{ matrix.python }}
           tox-envs: "py,typing"
-          tox-plugins: "tox-docker"
+          tox-plugins: "tox-docker 'requests<2.32.0'"
 
   docs:
 

Makefile

@@ -11,9 +11,10 @@ clean:
 	rm -rf docs/_build
 	rm -rf docs/api
 
+# Temporarily pin requests because 2.32.0 breaks docker 6.1.3.
 .PHONY: init
 init:
-	pip install --upgrade pip tox tox-docker pre-commit
+	pip install --upgrade pip tox tox-docker pre-commit 'requests<2.32.0'
 	pip install --upgrade -e ".[arq,db,dev,gcs,kubernetes]"
 	pre-commit install
 	rm -rf .tox

changelog.d/20240520_163147_rra_DM_44444.md

@@ -0,0 +1,3 @@
+### New features
+
+- Allow the database password to be passed to `create_database_engine` and `create_sync_session` as a Pydantic `SecretStr`.

docs/user-guide/database.rst

@@ -70,6 +70,7 @@ For applications using `Click`_ (the recommended way to implement a command-line
        await engine.dispose()
 
 This code assumes that ``main`` is the Click entry point and ``.config`` provides a ``config`` object that contains the settings for the application, including the database URL and password as well as the normal Safir configuration settings.
+The database password may be a ``pydantic.SecretStr``, a `str`, or `None` if no password is required by the database.
 
 If it receives a connection error from the database, Safir will attempt the initialization five times, two seconds apart, to allow time for networking or a database proxy to start.
 

pyproject.toml

@@ -47,7 +47,6 @@ dev = [
     "asgi-lifespan",
     "coverage[toml]",
     "fastapi>=0.93.0",
-    "flake8",
     "mypy",
     "pre-commit",
     "psycopg2",
@@ -59,7 +58,7 @@ dev = [
     "sqlalchemy[mypy]",
     "uvicorn",
     # documentation
-    "documenteer[guide]>=1.0.0a7",
+    "documenteer[guide]>=1",
     "autodoc_pydantic",
 ]
 gcs = [

src/safir/database.py

@@ -8,6 +8,7 @@
 from typing import overload
 from urllib.parse import quote, urlparse
 
+from pydantic import SecretStr
 from sqlalchemy import create_engine
 from sqlalchemy.exc import OperationalError
 from sqlalchemy.ext.asyncio import (
@@ -38,7 +39,7 @@ class DatabaseInitializationError(Exception):
 
 
 def _build_database_url(
-    url: str, password: str | None, *, is_async: bool
+    url: str, password: str | SecretStr | None, *, is_async: bool
 ) -> str:
     """Build the authenticated URL for the database.
 
@@ -66,6 +67,8 @@ def _build_database_url(
         if is_async and parsed_url.scheme == "postgresql":
             parsed_url = parsed_url._replace(scheme="postgresql+asyncpg")
         if password:
+            if isinstance(password, SecretStr):
+                password = password.get_secret_value()
             if not parsed_url.username:
                 raise ValueError(f"No username in database URL {url}")
             password = quote(password, safe="")
@@ -143,7 +146,7 @@ def datetime_to_db(time: datetime | None) -> datetime | None:
 
 def create_database_engine(
     url: str,
-    password: str | None,
+    password: str | SecretStr | None,
     *,
     isolation_level: str | None = None,
 ) -> AsyncEngine:
@@ -242,7 +245,7 @@ async def create_async_session(
 
 def create_sync_session(
     url: str,
-    password: str | None,
+    password: str | SecretStr | None,
     logger: BoundLogger | None = None,
     *,
     isolation_level: str | None = None,

tests/database_test.py

@@ -8,7 +8,7 @@
 
 import pytest
 import structlog
-from pydantic import BaseModel
+from pydantic import BaseModel, SecretStr
 from sqlalchemy import Column, MetaData, String, Table
 from sqlalchemy.exc import ProgrammingError
 from sqlalchemy.future import select
@@ -25,7 +25,7 @@
 )
 
 TEST_DATABASE_URL = os.environ["TEST_DATABASE_URL"]
-TEST_DATABASE_PASSWORD = os.getenv("TEST_DATABASE_PASSWORD")
+TEST_DATABASE_PASSWORD = os.environ["TEST_DATABASE_PASSWORD"]
 
 Base = declarative_base()
 
@@ -56,7 +56,10 @@ async def test_database_init() -> None:
         assert result.all() == ["someuser"]
     await session.remove()
 
-    # Reinitializing the database with reset should delete the data.
+    # Reinitializing the database with reset should delete the data. Try
+    # passing in the password as a SecretStr.
+    password = SecretStr(TEST_DATABASE_PASSWORD)
+    engine = create_database_engine(TEST_DATABASE_URL, password)
     await initialize_database(engine, logger, schema=Base.metadata, reset=True)
     session = await create_async_session(engine, logger)
     async with session.begin():