Add db-migrate to container path to meet infra template requirements (#…

…176)

* Add db-migrate executable command to application container PATH
* Use consistent method for creating db connection between migrations
and application
* Remove code for make_connection_uri
* Remove support for offline migrations

## Context
These changes fulfills upcoming application requirements in
template-infra for using the database. The new requirement is that there
is a "db-migrate" command available in the container PATH.

The previous migrations code connects to the database using a different
method than what the application uses, which means that the migrations
code does not have the functionality of connecting to the DB using IAM
auth, which is a requirement for the infra template. In this change, we
make the migrations code in env.py use the existing PostgresDBClient
class and remove the extraneous make_connection_uri class.

As part of the change, we also remove support for offline migrations,
which is not needed in general. This simplifies the code paths in env.py
and reduces maintenance burden.

app/Dockerfile

@@ -65,7 +65,6 @@ CMD ["poetry", "run", "python", "-m", "src"]
 
 FROM base AS release
 ARG RUN_USER
-USER ${RUN_USER}
 WORKDIR /app
 
 # TODO(https://github.com/navapbc/template-application-flask/issues/23) Productionize the Docker image
@@ -95,9 +94,15 @@ RUN poetry install --no-root --only main
 # or the application will not build
 RUN poetry build --format wheel && poetry run pip install 'dist/template_application_flask-0.1.0-py3-none-any.whl'
 
+# Add project's virtual env to the PATH so we can directly run poetry scripts
+# defiend in pyproject.toml
+ENV PATH="/app/.venv/bin:$PATH"
+
 # Set the host to 0.0.0.0 to make the server available external
 # to the Docker container that it's running in.
 ENV HOST=0.0.0.0
 
+USER ${RUN_USER}
+
 # Run the application.
 CMD ["poetry", "run", "python", "-m", "src"]

app/Makefile

@@ -132,7 +132,7 @@ alembic_config := ./src/db/migrations/alembic.ini
 alembic_cmd := $(PY_RUN_CMD) alembic --config $(alembic_config)
 
 db-upgrade: ## Apply pending migrations to db
-	$(PY_RUN_CMD) db-migrate-up
+	$(PY_RUN_CMD) db-migrate
 
 db-downgrade: ## Rollback last migration in db
 	$(PY_RUN_CMD) db-migrate-down

app/pyproject.toml

@@ -45,7 +45,7 @@ build-backend = "poetry.core.masonry.api"
 
 [tool.poetry.scripts]
 app-start = "src.__main__:main"
-db-migrate-up = "src.db.migrations.run:up"
+db-migrate = "src.db.migrations.run:up"
 db-migrate-down = "src.db.migrations.run:down"
 db-migrate-down-all = "src.db.migrations.run:downall"
 

app/src/adapters/db/client.py

@@ -2,10 +2,6 @@
 This module contains the DBClient class, which is used to manage database connections
 
 For usage information look at the package docstring in __init__.py
-
-This module also contains lower level connection related functions such as
-make_connection_uri that can be used outside of the application context such as for
-database migrations.
 """
 import abc
 import logging

app/src/adapters/db/clients/postgres_client.py

@@ -1,5 +1,4 @@
 import logging
-import urllib.parse
 from typing import Any
 
 import boto3
@@ -122,40 +121,6 @@ def generate_iam_auth_token(aws_region: str, host: str, port: int, user: str) ->
     return token
 
 
-def make_connection_uri(config: PostgresDBConfig) -> str:
-    """Construct PostgreSQL connection URI
-
-    More details at:
-    https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
-    """
-    host = config.host
-    db_name = config.name
-    username = config.username
-    password = urllib.parse.quote(config.password) if config.password else None
-    schema = config.db_schema
-    port = config.port
-
-    netloc_parts = []
-
-    if username and password:
-        netloc_parts.append(f"{username}:{password}@")
-    elif username:
-        netloc_parts.append(f"{username}@")
-    elif password:
-        netloc_parts.append(f":{password}@")
-
-    netloc_parts.append(host)
-
-    if port:
-        netloc_parts.append(f":{port}")
-
-    netloc = "".join(netloc_parts)
-
-    uri = f"postgresql://{netloc}/{db_name}?options=-csearch_path={schema}"
-
-    return uri
-
-
 def verify_ssl(connection_info: Any) -> None:
     """Verify that the database connection is encrypted and log a warning if not."""
     if connection_info.ssl_in_use:

app/src/db/migrations/env.py

@@ -1,24 +1,12 @@
 import logging
-import sys
 from typing import Any
 
+import alembic.context as context
 import sqlalchemy
-from alembic import context
 
-# Alembic cli seems to reset the path on load causing issues with local module imports.
-# Workaround is to force set the path to the current run directory (top level src folder)
-# See database migrations section in `./database/database-migrations.md` for details about running migrations.
-sys.path.insert(0, ".")  # noqa: E402
-
-# Load env vars before anything further
-from src.util.local import load_local_env_vars  # noqa: E402 isort:skip
-
-load_local_env_vars()
-
-from src.adapters.db.clients.postgres_client import make_connection_uri  # noqa: E402 isort:skip
-from src.adapters.db.clients.postgres_config import get_db_config  # noqa: E402 isort:skip
-from src.db.models import metadata  # noqa: E402 isort:skip
-import src.logging  # noqa: E402 isort:skip
+import src.adapters.db as db
+import src.logging
+from src.db.models import metadata
 
 # this is the Alembic Config object, which provides
 # access to the values within the .ini file in use.
@@ -29,13 +17,6 @@
 # Initialize logging
 with src.logging.init("migrations"):
 
-    if not config.get_main_option("sqlalchemy.url"):
-        uri = make_connection_uri(get_db_config())
-
-        # Escape percentage signs in the URI.
-        # https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option
-        config.set_main_option("sqlalchemy.url", uri.replace("%", "%%"))
-
     # add your model's MetaData object here
     # for 'autogenerate' support
     # from myapp import mymodel
@@ -59,33 +40,6 @@ def include_object(
         else:
             return True
 
-    def run_migrations_offline() -> None:
-        """Run migrations in 'offline' mode.
-
-        This configures the context with just a URL
-        and not an Engine, though an Engine is acceptable
-        here as well.  By skipping the Engine creation
-        we don't even need a DBAPI to be available.
-
-        Calls to context.execute() here emit the given string to the
-        script output.
-
-        """
-
-        url = config.get_main_option("sqlalchemy.url")
-        context.configure(
-            url=url,
-            target_metadata=target_metadata,
-            literal_binds=True,
-            dialect_opts={"paramstyle": "named"},
-            include_schemas=False,
-            include_object=include_object,
-            compare_type=True,
-        )
-
-        with context.begin_transaction():
-            context.run_migrations()
-
     def run_migrations_online() -> None:
         """Run migrations in 'online' mode.
 
@@ -94,10 +48,9 @@ def run_migrations_online() -> None:
 
         """
 
-        url = config.get_main_option("sqlalchemy.url", "SQLAlchemy URL not set")
-        connectable = sqlalchemy.create_engine(url)
+        db_client = db.PostgresDBClient()
 
-        with connectable.connect() as connection:
+        with db_client.get_connection() as connection:
             context.configure(
                 connection=connection,
                 target_metadata=target_metadata,
@@ -108,7 +61,7 @@ def run_migrations_online() -> None:
             with context.begin_transaction():
                 context.run_migrations()
 
-    if context.is_offline_mode():
-        run_migrations_offline()
-    else:
-        run_migrations_online()
+    # No need to support running migrations in offline mode.
+    # When running locally we have the local containerized database.
+    # When running in the cloud we'll have the actual cloud database.
+    run_migrations_online()

app/src/db/migrations/run.py

@@ -6,8 +6,9 @@
 import os
 from typing import Optional
 
+import alembic.command as command
+import alembic.script as script
 import sqlalchemy
-from alembic import command, script
 from alembic.config import Config
 from alembic.operations.ops import MigrationScript
 from alembic.runtime import migration

app/tests/src/adapters/db/clients/test_postgres_client.py

@@ -1,14 +1,9 @@
-import logging  # noqa: B1
-from itertools import product
+import logging
 
 import pytest
 
-from src.adapters.db.clients.postgres_client import (
-    get_connection_parameters,
-    make_connection_uri,
-    verify_ssl,
-)
-from src.adapters.db.clients.postgres_config import PostgresDBConfig, get_db_config
+from src.adapters.db.clients.postgres_client import get_connection_parameters, verify_ssl
+from src.adapters.db.clients.postgres_config import get_db_config
 
 
 class DummyConnectionInfo:
@@ -43,36 +38,6 @@ def test_verify_ssl_not_in_use(caplog):
     assert caplog.records[0].levelname == "WARNING"
 
 
-@pytest.mark.parametrize(
-    "username_password_port,expected",
-    zip(
-        # Test all combinations of username, password, and port
-        product(["testuser", ""], ["testpass", None], [5432]),
-        [
-            "postgresql://testuser:testpass@localhost:5432/dbname?options=-csearch_path=public",
-            "postgresql://testuser@localhost:5432/dbname?options=-csearch_path=public",
-            "postgresql://:testpass@localhost:5432/dbname?options=-csearch_path=public",
-            "postgresql://localhost:5432/dbname?options=-csearch_path=public",
-        ],
-    ),
-)
-def test_make_connection_uri(username_password_port, expected):
-    username, password, port = username_password_port
-    assert (
-        make_connection_uri(
-            PostgresDBConfig(
-                host="localhost",
-                name="dbname",
-                username=username,
-                password=password,
-                db_schema="public",
-                port=port,
-            )
-        )
-        == expected
-    )
-
-
 def test_get_connection_parameters(monkeypatch: pytest.MonkeyPatch):
     monkeypatch.delenv("DB_SSL_MODE")
     db_config = get_db_config()

app/tests/src/db/test_migrations.py

@@ -1,7 +1,7 @@
 import logging  # noqa: B1
 
+import alembic.command as command
 import pytest
-from alembic import command
 from alembic.script import ScriptDirectory
 from alembic.script.revision import MultipleHeads
 from alembic.util.exc import CommandError