feat(db) Replace the hand-rolled migration runner with Alembic, and allow Postgres

.github/workflows/tests.yml

@@ -67,6 +67,22 @@ jobs:
         ports:
           - 6379:6379
 
+      # SQLite forgives what Postgres rejects: it enforces a foreign key only
+      # when asked to, and works the next id out from the data instead of a
+      # sequence. Anything resting on either can only be proven here.
+      postgres:
+        image: postgres:16-alpine
+        env:
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: borg_test
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 5432:5432
+
     steps:
       - name: Checkout code
         uses: actions/checkout@v7
@@ -122,6 +138,7 @@ jobs:
         env:
           REDIS_HOST: localhost
           REDIS_PORT: 6379
+          BORG_TEST_POSTGRES_URL: postgresql+psycopg://postgres:postgres@localhost:5432/borg_test
           COVERAGE_FILE: coverage-data/unit-${{ matrix.shard }}.coverage
 
       - name: Upload backend coverage data

Dockerfile

@@ -66,7 +66,9 @@ RUN chmod +x /entrypoint.sh
 
 ENV PYTHONPATH=/app
 ENV DATA_DIR=/data
-ENV DATABASE_URL=sqlite:////data/borg.db
+# No DATABASE_URL default: the app derives sqlite:///$DATA_DIR/borg.db itself
+# when none is set. A default here would always win over DB_HOST (DATABASE_URL
+# takes precedence), so the Postgres switch could never take effect.
 ENV BORG_BACKUP_PATH=/backups
 ENV BORG_CACHE_DIR=/home/borg/.cache/borg
 ENV ENABLE_CRON_BACKUPS=false
@@ -134,7 +136,9 @@ RUN chmod +x /entrypoint.sh
 # Set environment variables
 ENV PYTHONPATH=/app
 ENV DATA_DIR=/data
-ENV DATABASE_URL=sqlite:////data/borg.db
+# No DATABASE_URL default: the app derives sqlite:///$DATA_DIR/borg.db itself
+# when none is set. A default here would always win over DB_HOST (DATABASE_URL
+# takes precedence), so the Postgres switch could never take effect.
 ENV BORG_BACKUP_PATH=/backups
 ENV BORG_CACHE_DIR=/home/borg/.cache/borg
 ENV ENABLE_CRON_BACKUPS=false

alembic.ini

@@ -0,0 +1,152 @@
+# A generic, single database configuration.
+
+[alembic]
+# path to migration scripts.
+# this is typically a path given in POSIX (e.g. forward slashes)
+# format, relative to the token %(here)s which refers to the location of this
+# ini file
+script_location = %(here)s/app/database/alembic
+
+# template used to generate migration file names; The default value is %%(rev)s_%%(slug)s
+# Uncomment the line below if you want the files to be prepended with date and time
+# see https://alembic.sqlalchemy.org/en/latest/tutorial.html#editing-the-ini-file
+# for all available tokens
+# file_template = %%(year)d_%%(month).2d_%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s
+# Or organize into date-based subdirectories (requires recursive_version_locations = true)
+# file_template = %%(year)d/%%(month).2d/%%(day).2d_%%(hour).2d%%(minute).2d_%%(second).2d_%%(rev)s_%%(slug)s
+
+# sys.path path, will be prepended to sys.path if present.
+# defaults to the current working directory.  for multiple paths, the path separator
+# is defined by "path_separator" below.
+prepend_sys_path = .
+
+
+# timezone to use when rendering the date within the migration file
+# as well as the filename.
+# If specified, requires the tzdata library which can be installed by adding
+# `alembic[tz]` to the pip requirements.
+# string value is passed to ZoneInfo()
+# leave blank for localtime
+# timezone =
+
+# max length of characters to apply to the "slug" field
+# truncate_slug_length = 40
+
+# set to 'true' to run the environment during
+# the 'revision' command, regardless of autogenerate
+# revision_environment = false
+
+# set to 'true' to allow .pyc and .pyo files without
+# a source .py file to be detected as revisions in the
+# versions/ directory
+# sourceless = false
+
+# version location specification; This defaults
+# to <script_location>/versions.  When using multiple version
+# directories, initial revisions must be specified with --version-path.
+# The path separator used here should be the separator specified by "path_separator"
+# below.
+# version_locations = %(here)s/bar:%(here)s/bat:%(here)s/alembic/versions
+
+# path_separator; This indicates what character is used to split lists of file
+# paths, including version_locations and prepend_sys_path within configparser
+# files such as alembic.ini.
+# The default rendered in new alembic.ini files is "os", which uses os.pathsep
+# to provide os-dependent path splitting.
+#
+# Note that in order to support legacy alembic.ini files, this default does NOT
+# take place if path_separator is not present in alembic.ini.  If this
+# option is omitted entirely, fallback logic is as follows:
+#
+# 1. Parsing of the version_locations option falls back to using the legacy
+#    "version_path_separator" key, which if absent then falls back to the legacy
+#    behavior of splitting on spaces and/or commas.
+# 2. Parsing of the prepend_sys_path option falls back to the legacy
+#    behavior of splitting on spaces, commas, or colons.
+#
+# Valid values for path_separator are:
+#
+# path_separator = :
+# path_separator = ;
+# path_separator = space
+# path_separator = newline
+#
+# Use os.pathsep. Default configuration used for new projects.
+path_separator = os
+
+# set to 'true' to search source files recursively
+# in each "version_locations" directory
+# new in Alembic version 1.10
+# recursive_version_locations = false
+
+# the output encoding used when revision files
+# are written from script.py.mako
+# output_encoding = utf-8
+
+# database URL.  This is consumed by the user-maintained env.py script only.
+# other means of configuring database URLs may be customized within the env.py
+# file.
+# Deliberately empty: env.py takes the URL from the application settings, so
+# there is one source of truth for which database gets migrated. Override only
+# for one-off runs against a throwaway database.
+sqlalchemy.url =
+
+
+[post_write_hooks]
+# post_write_hooks defines scripts or Python functions that are run
+# on newly generated revision scripts.  See the documentation for further
+# detail and examples
+
+# format using "black" - use the console_scripts runner, against the "black" entrypoint
+# hooks = black
+# black.type = console_scripts
+# black.entrypoint = black
+# black.options = -l 79 REVISION_SCRIPT_FILENAME
+
+# lint with attempts to fix using "ruff" - use the module runner, against the "ruff" module
+# hooks = ruff
+# ruff.type = module
+# ruff.module = ruff
+# ruff.options = check --fix REVISION_SCRIPT_FILENAME
+
+# Alternatively, use the exec runner to execute a binary found on your PATH
+# hooks = ruff
+# ruff.type = exec
+# ruff.executable = ruff
+# ruff.options = check --fix REVISION_SCRIPT_FILENAME
+
+# Logging configuration.  This is also consumed by the user-maintained
+# env.py script only.
+[loggers]
+keys = root,sqlalchemy,alembic
+
+[handlers]
+keys = console
+
+[formatters]
+keys = generic
+
+[logger_root]
+level = WARNING
+handlers = console
+qualname =
+
+[logger_sqlalchemy]
+level = WARNING
+handlers =
+qualname = sqlalchemy.engine
+
+[logger_alembic]
+level = INFO
+handlers =
+qualname = alembic
+
+[handler_console]
+class = StreamHandler
+args = (sys.stderr,)
+level = NOTSET
+formatter = generic
+
+[formatter_generic]
+format = %(levelname)-5.5s [%(name)s] %(message)s
+datefmt = %H:%M:%S

app/api/backup_plans.py

@@ -195,7 +195,7 @@ def _unique_backup_plan_name(db: Session, base_name: str) -> str:
     existing_names = {
         name
         for (name,) in db.query(BackupPlan.name)
-        .filter(BackupPlan.name.like(f"{candidate}%"))
+        .filter(BackupPlan.name.ilike(f"{candidate}%"))
         .all()
     }
     if candidate not in existing_names:

app/api/scripts_library.py

@@ -249,8 +249,8 @@ async def list_scripts(
     if search:
         search_pattern = f"%{search}%"
         query = query.filter(
-            (Script.name.like(search_pattern))
-            | (Script.description.like(search_pattern))
+            (Script.name.ilike(search_pattern))
+            | (Script.description.ilike(search_pattern))
         )
 
     scripts = query.order_by(Script.created_at.desc()).all()

app/config.py

@@ -1,7 +1,8 @@
 import os
 import secrets
-from typing import Any, List, Union, Optional
+from typing import Any, List, Union, Optional, Mapping
 from pathlib import Path
+from urllib.parse import quote_plus
 from pydantic_settings import BaseSettings
 
 
@@ -19,6 +20,43 @@ def _packaged_version() -> str:
         return ""
 
 
+def resolve_database_url(env: Mapping[str, str], data_dir: str) -> str:
+    """Work out which database to use, from most explicit to least.
+
+    DATABASE_URL wins, so nothing existing changes. Otherwise DB_HOST switches
+    to Postgres and the DSN is assembled here rather than handed over ready-made:
+    a generated password may contain "/", "@", ":" or "%", any of which silently
+    breaks a URL, and the resulting error points at the host or the credentials
+    rather than at the quoting. Assembling it in one place means it is escaped
+    exactly once.
+
+    With neither, the database is a SQLite file under data_dir, as before.
+    """
+    if env.get("DATABASE_URL"):
+        return env["DATABASE_URL"]
+
+    host = env.get("DB_HOST")
+    if not host:
+        return f"sqlite:///{data_dir}/borg.db"
+
+    missing = [name for name in ("DB_USER", "DB_PASSWORD") if not env.get(name)]
+    if missing:
+        # Falling back to SQLite here would start the application on a local
+        # file while the real database sits untouched, and the two would drift
+        # apart until someone noticed the data was missing.
+        raise RuntimeError(
+            f"DB_HOST is set but {' and '.join(missing)} "
+            f"{'is' if len(missing) == 1 else 'are'} not. Refusing to fall back "
+            "to SQLite, which would quietly run on the wrong database."
+        )
+
+    user = quote_plus(env["DB_USER"])
+    password = quote_plus(env["DB_PASSWORD"])
+    port = env.get("DB_PORT", "5432")
+    name = env.get("DB_NAME", "borg")
+    return f"postgresql+psycopg://{user}:{password}@{host}:{port}/{name}"
+
+
 class Settings(BaseSettings):
     """Application settings"""
 
@@ -197,12 +235,12 @@ class Config:
         env_file = ".env"
         case_sensitive = False
         extra = "ignore"
-        # Map env vars to internal string fields
-        fields = {
-            "_cors_origins_str": {"env": "CORS_ORIGINS"},
-            "_trusted_proxies_str": {"env": "TRUSTED_PROXIES"},
-            "_oidc_allowed_return_origins_str": {"env": "OIDC_ALLOWED_RETURN_ORIGINS"},
-        }
+        # NOTE: pydantic v1 mapped CORS_ORIGINS / TRUSTED_PROXIES /
+        # OIDC_ALLOWED_RETURN_ORIGINS onto the private _*_str fields here via a
+        # `fields = {...}` block. v2 removed that config key (it only warned and
+        # was ignored), so those env vars already have no effect -- the defaults
+        # are always used. Removed to silence the warning; wiring them back up
+        # needs Field(validation_alias=...) on public fields, a separate change.
 
     def model_post_init(self, __context: Any) -> None:
         if not self.rclone_config_root:
@@ -224,13 +262,8 @@ def model_post_init(self, __context: Any) -> None:
 # AUTO-DERIVE all paths from data_dir
 # Users only need to configure data_dir (via volume mount), everything else is automatic
 
-# 1. Database URL - always derived from data_dir
-env_database_url = os.getenv("DATABASE_URL")
-if env_database_url:
-    settings.database_url = env_database_url
-else:
-    # Auto-derive: sqlite:////data/borg.db
-    settings.database_url = f"sqlite:///{settings.data_dir}/borg.db"
+# 1. Database URL
+settings.database_url = resolve_database_url(os.environ, settings.data_dir)
 
 # 2. SSH keys directory - always derived from data_dir
 settings.ssh_keys_dir = f"{settings.data_dir}/ssh_keys"

app/database/alembic/README.md

@@ -0,0 +1,54 @@
+# Database migrations
+
+The schema is managed by Alembic against a single baseline, and the same
+migration body must work on **both SQLite and Postgres**. The engine and URL come
+from the application settings (`env.py` reads `settings.database_url`), and every
+migration is rendered in batch mode, so one migration serves SQLite (table
+rebuild) and Postgres (native `ALTER`) alike.
+
+The CI test matrix applies every migration to SQLite **and** Postgres, so most
+dialect mistakes fail hard on the Postgres leg rather than slipping through. Four
+conventions keep a migration dialect-agnostic:
+
+1. **DDL only through the `op` API** — never a raw SQL string. `op.add_column`,
+   `op.create_index`, `batch_op.alter_column`, and so on. Raw SQL freezes one
+   dialect's grammar.
+2. **Boolean defaults as `sa.false()` / `sa.true()`** — never a literal `0`/`1`.
+   `ADD COLUMN f BOOLEAN DEFAULT 0` is accepted by SQLite and rejected by Postgres
+   with a `DatatypeMismatch`.
+3. **Data migrations through SQLAlchemy Core** — `table.update().values(flag=True)`,
+   never a raw `UPDATE ... SET flag = 1`. Same reason as (2).
+4. **`server_default` must render per dialect** — use `sa.func.now()`, never
+   `sa.text('CURRENT_TIMESTAMP')` or `sa.text('0')`. `autogenerate` **freezes the
+   dialect it ran against** into every `server_default`, so never ship a generated
+   one unreviewed: generated against SQLite it dies loudly on Postgres; generated
+   against Postgres it passes on SQLite and then breaks at the first insert.
+
+**For anything SQLite cannot `ALTER`** — dropping or altering a column, changing a
+constraint, changing a foreign-key action — wrap it in `op.batch_alter_table(...)`.
+On SQLite, Alembic performs the create-copy-drop-rename rebuild automatically; on
+Postgres it emits a native `ALTER`. One body, both dialects.
+
+**When a batch operation touches an AUTOINCREMENT table**, pass
+`table_kwargs={"sqlite_autoincrement": True}`. The rebuild loses AUTOINCREMENT
+otherwise, and reflection can never see that it is gone, so the drift would be
+permanent and invisible. (The baseline's `create_table` renders it correctly;
+only `batch_alter_table` needs the reminder.)
+
+## Workflow
+
+1. Change `models.py`. AUTOINCREMENT is derived from the primary key, so a new
+   single-integer-PK table gets it automatically; a new constraint is named by the
+   metadata naming convention automatically.
+2. `alembic revision --autogenerate -m "<what>"`.
+3. **Review the generated file.** Fix every `server_default` to a dialect-agnostic
+   form (convention 4); confirm any `batch_alter_table` on an AUTOINCREMENT table
+   carries `sqlite_autoincrement`. These two — AUTOINCREMENT on a batch rebuild and
+   a frozen `server_default` — are the things `autogenerate` gets wrong, because it
+   is blind to both.
+4. Run the test suite. It applies the migration to SQLite and to Postgres; a
+   dialect violation fails the Postgres leg.
+
+Rule of thumb: **anything the framework can enforce, it does; the two it cannot
+see — AUTOINCREMENT on a batch rebuild, and a frozen `server_default` — are the two
+you review by hand.**