From bb62382e1886a44f924a9e96f19024f300aafc8d Mon Sep 17 00:00:00 2001 From: Peter van Arkel Date: Thu, 21 May 2026 02:34:09 +0200 Subject: [PATCH] (refactor) Remove YAML config import and export path Drop the pre-Django YAML import/export management commands and remove the file-based config loader fallback from the backup and retention engines. Keep the runtime config bridge backed by Django models, and add tests that ensure engine operations require an explicit Django config source. --- docs/development.md | 18 --- src/pobsync/commands/retention_plan.py | 10 +- src/pobsync/commands/run_scheduled.py | 7 +- src/pobsync/config/load.py | 53 -------- src/pobsync/config/source.py | 14 --- src/pobsync_backend/config_repository.py | 48 +------- .../commands/export_pobsync_configs.py | 23 ---- .../commands/import_pobsync_configs.py | 81 ------------- .../tests/test_config_repository.py | 114 ++++++++---------- .../tests/test_retention_config_source.py | 10 ++ .../tests/test_run_scheduled_config_source.py | 5 + 11 files changed, 82 insertions(+), 301 deletions(-) delete mode 100644 src/pobsync/config/load.py delete mode 100644 src/pobsync_backend/management/commands/export_pobsync_configs.py delete mode 100644 src/pobsync_backend/management/commands/import_pobsync_configs.py diff --git a/docs/development.md b/docs/development.md index a8dcfd7..994235c 100644 --- a/docs/development.md +++ b/docs/development.md @@ -96,23 +96,6 @@ The updater is intentionally a small wrapper around the installer for routine pr non-interactive, preserve the existing environment file, skip OS package installation, skip superuser creation, and still run the Django/runtime refresh steps needed after a code update. -## Migration Helpers - -Import pre-Django YAML configs during a one-time migration: - -``` -python3 manage.py import_pobsync_configs --prefix /opt/pobsync -``` - -Export SQL config back to YAML for inspection or one-off compatibility: - -``` -python3 manage.py export_pobsync_configs --prefix /opt/pobsync -``` - -These commands are migration helpers, not the normal operating model. After import, review and continue operating from -the Django control panel. - ## Docker With SQLite Docker Compose is useful for local development and disposable test installs. Native systemd is preferred for production @@ -192,4 +175,3 @@ Next refactor targets: - Move more snapshot lifecycle details into typed domain objects. - Replace remaining dictionary-shaped config at engine boundaries. -- Remove YAML migration import/export once production migration no longer needs it. diff --git a/src/pobsync/commands/retention_plan.py b/src/pobsync/commands/retention_plan.py index a2fb5f9..2b96d57 100644 --- a/src/pobsync/commands/retention_plan.py +++ b/src/pobsync/commands/retention_plan.py @@ -4,9 +4,8 @@ from datetime import datetime, timezone from pathlib import Path from typing import Any, List -from ..config.source import ConfigSource, FileConfigSource +from ..config.source import ConfigSource from ..errors import ConfigError -from ..paths import PobsyncPaths from ..retention import Snapshot, apply_base_protection, build_retention_plan from ..snapshot_meta import iter_snapshot_dirs, read_snapshot_meta, resolve_host_root from ..util import sanitize_host @@ -40,10 +39,9 @@ def run_retention_plan( if kind not in {"scheduled", "manual", "all"}: raise ConfigError("kind must be scheduled, manual, or all") - paths = PobsyncPaths(home=prefix) - - source = config_source or FileConfigSource(prefix=paths.home) - cfg = source.effective_config_for_host(host) + if config_source is None: + raise ConfigError("A Django config source is required.") + cfg = config_source.effective_config_for_host(host) retention = cfg.get("retention") if not isinstance(retention, dict): diff --git a/src/pobsync/commands/run_scheduled.py b/src/pobsync/commands/run_scheduled.py index 890035d..4a321b0 100644 --- a/src/pobsync/commands/run_scheduled.py +++ b/src/pobsync/commands/run_scheduled.py @@ -3,7 +3,7 @@ from __future__ import annotations from pathlib import Path from typing import Any, Callable -from ..config.source import ConfigSource, FileConfigSource +from ..config.source import ConfigSource from ..errors import ConfigError from ..lock import acquire_host_lock from ..paths import PobsyncPaths @@ -163,8 +163,9 @@ def run_scheduled( host = sanitize_host(host) paths = PobsyncPaths(home=prefix) - source = config_source or FileConfigSource(prefix=paths.home) - cfg = source.effective_config_for_host(host) + if config_source is None: + raise ConfigError("A Django config source is required.") + cfg = config_source.effective_config_for_host(host) backup_root = cfg.get("backup_root") if not isinstance(backup_root, str) or not backup_root.startswith("/"): diff --git a/src/pobsync/config/load.py b/src/pobsync/config/load.py deleted file mode 100644 index 1a0e0a4..0000000 --- a/src/pobsync/config/load.py +++ /dev/null @@ -1,53 +0,0 @@ -from __future__ import annotations - -from pathlib import Path -from typing import Any - -import yaml - -from ..errors import ConfigError, ValidationError -from ..validate import validate_dict -from .schemas import GLOBAL_SCHEMA, HOST_SCHEMA - - -def load_yaml_file(path: Path) -> dict[str, Any]: - if not path.exists(): - raise ConfigError(f"Missing config file: {path}") - try: - raw = path.read_text(encoding="utf-8") - except OSError as e: - raise ConfigError(f"Cannot read config file: {path}: {e}") from e - - try: - data = yaml.safe_load(raw) - except yaml.YAMLError as e: - raise ConfigError(f"Invalid YAML in {path}: {e}") from e - - if data is None: - data = {} - if not isinstance(data, dict): - raise ConfigError(f"Config root must be a mapping in {path}") - return data - - -def load_global_config(path: Path) -> dict[str, Any]: - data = load_yaml_file(path) - try: - return validate_dict(data, GLOBAL_SCHEMA, path="global") - except ValidationError as e: - raise ConfigError(f"Invalid global config at {path}: {format_validation_error(e)}") from e - - -def load_host_config(path: Path) -> dict[str, Any]: - data = load_yaml_file(path) - try: - return validate_dict(data, HOST_SCHEMA, path="host") - except ValidationError as e: - raise ConfigError(f"Invalid host config at {path}: {format_validation_error(e)}") from e - - -def format_validation_error(err: ValidationError) -> str: - if err.path: - return f"{err.path}: {err}" - return str(err) - diff --git a/src/pobsync/config/source.py b/src/pobsync/config/source.py index bcd6ee6..476179a 100644 --- a/src/pobsync/config/source.py +++ b/src/pobsync/config/source.py @@ -1,22 +1,8 @@ from __future__ import annotations -from pathlib import Path from typing import Any, Protocol -from .load import load_global_config, load_host_config -from .merge import build_effective_config - class ConfigSource(Protocol): def effective_config_for_host(self, host: str) -> dict[str, Any]: """Return the fully merged effective config for a host.""" - - -class FileConfigSource: - def __init__(self, prefix: Path) -> None: - self.prefix = prefix - - def effective_config_for_host(self, host: str) -> dict[str, Any]: - global_cfg = load_global_config(self.prefix / "config" / "global.yaml") - host_cfg = load_host_config(self.prefix / "config" / "hosts" / f"{host}.yaml") - return build_effective_config(global_cfg, host_cfg) diff --git a/src/pobsync_backend/config_repository.py b/src/pobsync_backend/config_repository.py index 5cf5f40..7f08e5d 100644 --- a/src/pobsync_backend/config_repository.py +++ b/src/pobsync_backend/config_repository.py @@ -1,13 +1,10 @@ from __future__ import annotations -from pathlib import Path from typing import Any from django.core.exceptions import ObjectDoesNotExist from pobsync.config.schemas import GLOBAL_SCHEMA, HOST_SCHEMA -from pobsync.paths import PobsyncPaths -from pobsync.util import write_yaml_atomic from pobsync.validate import validate_dict from .models import GlobalConfig, HostConfig @@ -17,7 +14,7 @@ class ConfigRepositoryError(RuntimeError): pass -def _global_yaml_data(global_config: GlobalConfig) -> dict[str, Any]: +def _global_runtime_data(global_config: GlobalConfig) -> dict[str, Any]: data = { "backup_root": global_config.backup_root, "pobsync_home": global_config.pobsync_home, @@ -48,7 +45,7 @@ def _global_yaml_data(global_config: GlobalConfig) -> dict[str, Any]: return validate_dict(data, GLOBAL_SCHEMA, path="global") -def _host_yaml_data(host_config: HostConfig) -> dict[str, Any]: +def _host_runtime_data(host_config: HostConfig) -> dict[str, Any]: data: dict[str, Any] = { "host": host_config.host, "address": host_config.address, @@ -78,11 +75,11 @@ def _host_yaml_data(host_config: HostConfig) -> dict[str, Any]: def global_config_object_data(global_config: GlobalConfig) -> dict[str, Any]: - return _global_yaml_data(global_config) + return _global_runtime_data(global_config) def host_config_object_data(host_config: HostConfig) -> dict[str, Any]: - return _host_yaml_data(host_config) + return _host_runtime_data(host_config) def global_config_data(name: str = "default") -> dict[str, Any]: @@ -90,7 +87,7 @@ def global_config_data(name: str = "default") -> dict[str, Any]: global_config = GlobalConfig.objects.get(name=name) except ObjectDoesNotExist as exc: raise ConfigRepositoryError(f"Missing GlobalConfig {name!r}") from exc - return _global_yaml_data(global_config) + return _global_runtime_data(global_config) def host_config_data(host: str) -> dict[str, Any]: @@ -98,37 +95,4 @@ def host_config_data(host: str) -> dict[str, Any]: host_config = HostConfig.objects.get(host=host, enabled=True) except ObjectDoesNotExist as exc: raise ConfigRepositoryError(f"Missing enabled HostConfig {host!r}") from exc - return _host_yaml_data(host_config) - - -def export_global_config(prefix: Path, name: str = "default") -> Path: - try: - global_config = GlobalConfig.objects.get(name=name) - except ObjectDoesNotExist as exc: - raise ConfigRepositoryError(f"Missing GlobalConfig {name!r}") from exc - - paths = PobsyncPaths(home=prefix) - write_yaml_atomic(paths.global_config_path, _global_yaml_data(global_config)) - return paths.global_config_path - - -def export_host_config(prefix: Path, host: str) -> Path: - try: - host_config = HostConfig.objects.get(host=host, enabled=True) - except ObjectDoesNotExist as exc: - raise ConfigRepositoryError(f"Missing enabled HostConfig {host!r}") from exc - - paths = PobsyncPaths(home=prefix) - target = paths.hosts_dir / f"{host_config.host}.yaml" - write_yaml_atomic(target, _host_yaml_data(host_config)) - return target - - -def export_runtime_configs(prefix: Path, host: str | None = None) -> list[Path]: - written = [export_global_config(prefix)] - hosts = HostConfig.objects.filter(enabled=True).order_by("host") - if host is not None: - hosts = hosts.filter(host=host) - for host_config in hosts: - written.append(export_host_config(prefix, host_config.host)) - return written + return _host_runtime_data(host_config) diff --git a/src/pobsync_backend/management/commands/export_pobsync_configs.py b/src/pobsync_backend/management/commands/export_pobsync_configs.py deleted file mode 100644 index 78623bb..0000000 --- a/src/pobsync_backend/management/commands/export_pobsync_configs.py +++ /dev/null @@ -1,23 +0,0 @@ -from __future__ import annotations - -from pathlib import Path -from typing import Any - -from django.conf import settings -from django.core.management.base import BaseCommand - -from pobsync_backend.config_repository import export_runtime_configs - - -class Command(BaseCommand): - help = "Export Django database configs to pobsync runtime YAML files." - - def add_arguments(self, parser) -> None: - parser.add_argument("--prefix", default=settings.POBSYNC_HOME, help="Runtime state root") - parser.add_argument("--host", default=None, help="Export only one enabled host") - - def handle(self, *args: Any, **options: Any) -> None: - written = export_runtime_configs(prefix=Path(options["prefix"]), host=options["host"]) - for path in written: - self.stdout.write(str(path)) - self.stdout.write(self.style.SUCCESS(f"Exported {len(written)} config file(s).")) diff --git a/src/pobsync_backend/management/commands/import_pobsync_configs.py b/src/pobsync_backend/management/commands/import_pobsync_configs.py deleted file mode 100644 index a2de4b4..0000000 --- a/src/pobsync_backend/management/commands/import_pobsync_configs.py +++ /dev/null @@ -1,81 +0,0 @@ -from __future__ import annotations - -from pathlib import Path -from typing import Any - -from django.conf import settings -from django.core.management.base import BaseCommand, CommandError - -from pobsync.config.load import load_global_config, load_host_config -from pobsync.paths import PobsyncPaths -from pobsync_backend.models import GlobalConfig, HostConfig - - -class Command(BaseCommand): - help = "Import pobsync YAML configs into the Django database." - - def add_arguments(self, parser) -> None: - parser.add_argument("--prefix", default=settings.POBSYNC_HOME, help="Runtime state root") - - def handle(self, *args: Any, **options: Any) -> None: - paths = PobsyncPaths(home=Path(options["prefix"])) - if not paths.global_config_path.exists(): - raise CommandError(f"Missing global config: {paths.global_config_path}") - - global_cfg = load_global_config(paths.global_config_path) - global_ssh = global_cfg.get("ssh") or {} - global_rsync = global_cfg.get("rsync") or {} - global_defaults = global_cfg.get("defaults") or {} - retention_defaults = global_cfg.get("retention_defaults") or {} - GlobalConfig.objects.update_or_create( - name="default", - defaults={ - "backup_root": global_cfg["backup_root"], - "pobsync_home": global_cfg.get("pobsync_home", str(paths.home)), - "ssh_user": global_ssh.get("user") or "root", - "ssh_port": global_ssh.get("port") or 22, - "ssh_options": global_ssh.get("options") or [], - "rsync_binary": global_rsync.get("binary") or "rsync", - "rsync_args": global_rsync.get("args") or [], - "rsync_extra_args": global_rsync.get("extra_args") or [], - "rsync_timeout_seconds": global_rsync.get("timeout_seconds") or 0, - "rsync_bwlimit_kbps": global_rsync.get("bwlimit_kbps") or 0, - "default_source_root": global_defaults.get("source_root") or "/", - "default_destination_subdir": global_defaults.get("destination_subdir") or "", - "excludes_default": global_cfg.get("excludes_default") or [], - "retention_daily": retention_defaults.get("daily", 14), - "retention_weekly": retention_defaults.get("weekly", 8), - "retention_monthly": retention_defaults.get("monthly", 12), - "retention_yearly": retention_defaults.get("yearly", 0), - "data": global_cfg, - }, - ) - - count = 0 - for host_path in sorted(paths.hosts_dir.glob("*.yaml")): - host_cfg = load_host_config(host_path) - host_ssh = host_cfg.get("ssh") or {} - host_rsync = host_cfg.get("rsync") or {} - host_retention = host_cfg.get("retention") or {} - HostConfig.objects.update_or_create( - host=host_cfg["host"], - defaults={ - "address": host_cfg["address"], - "ssh_user": host_ssh.get("user") or "", - "ssh_port": host_ssh.get("port"), - "source_root": host_cfg.get("source_root") or "", - "includes": host_cfg.get("includes") or [], - "excludes_add": host_cfg.get("excludes_add") or [], - "excludes_replace": host_cfg.get("excludes_replace"), - "rsync_extra_args": host_rsync.get("extra_args") or [], - "retention_daily": host_retention.get("daily", 14), - "retention_weekly": host_retention.get("weekly", 8), - "retention_monthly": host_retention.get("monthly", 12), - "retention_yearly": host_retention.get("yearly", 0), - "config": host_cfg, - "enabled": True, - }, - ) - count += 1 - - self.stdout.write(self.style.SUCCESS(f"Imported global config and {count} host config(s).")) diff --git a/src/pobsync_backend/tests/test_config_repository.py b/src/pobsync_backend/tests/test_config_repository.py index 312aab2..409294a 100644 --- a/src/pobsync_backend/tests/test_config_repository.py +++ b/src/pobsync_backend/tests/test_config_repository.py @@ -1,71 +1,63 @@ from __future__ import annotations -import tempfile -from pathlib import Path - from django.test import TestCase -from pobsync.config.load import load_global_config, load_host_config -from pobsync_backend.config_repository import export_runtime_configs +from pobsync_backend.config_repository import global_config_data, host_config_data from pobsync_backend.models import GlobalConfig, HostConfig class ConfigRepositoryTests(TestCase): - def test_exports_database_configs_to_engine_yaml(self) -> None: - with tempfile.TemporaryDirectory() as tmp: - prefix = Path(tmp) - GlobalConfig.objects.create( - name="default", - backup_root="/backups", - pobsync_home=str(prefix), - ssh_user="backup", - ssh_port=2222, - rsync_args=["--archive"], - excludes_default=["/proc/***"], - retention_daily=7, - retention_weekly=4, - retention_monthly=3, - retention_yearly=1, - data={ - "backup_root": "/ignored", - "pobsync_home": "/ignored", - "ssh": {"user": "ignored", "port": 22, "options": []}, - "unknown": "must-not-leak", - "retention_defaults": {"daily": 99, "weekly": 99, "monthly": 99, "yearly": 99}, - }, - ) - HostConfig.objects.create( - host="web-01", - address="web-01.example.test", - ssh_user="root", - includes=[], - excludes_add=["/tmp/***"], - retention_daily=7, - retention_weekly=4, - retention_monthly=3, - retention_yearly=1, - config={ - "host": "ignored", - "address": "ignored", - "retention": {"daily": 99, "weekly": 99, "monthly": 99, "yearly": 99}, - "excludes_add": ["/ignored/***"], - "unknown": "must-not-leak", - }, - ) + def test_builds_runtime_config_from_database_fields(self) -> None: + GlobalConfig.objects.create( + name="default", + backup_root="/backups", + pobsync_home="/var/lib/pobsync", + ssh_user="backup", + ssh_port=2222, + rsync_args=["--archive"], + excludes_default=["/proc/***"], + retention_daily=7, + retention_weekly=4, + retention_monthly=3, + retention_yearly=1, + data={ + "backup_root": "/ignored", + "pobsync_home": "/ignored", + "ssh": {"user": "ignored", "port": 22, "options": []}, + "unknown": "must-not-leak", + "retention_defaults": {"daily": 99, "weekly": 99, "monthly": 99, "yearly": 99}, + }, + ) + HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + ssh_user="root", + includes=[], + excludes_add=["/tmp/***"], + retention_daily=7, + retention_weekly=4, + retention_monthly=3, + retention_yearly=1, + config={ + "host": "ignored", + "address": "ignored", + "retention": {"daily": 99, "weekly": 99, "monthly": 99, "yearly": 99}, + "excludes_add": ["/ignored/***"], + "unknown": "must-not-leak", + }, + ) - written = export_runtime_configs(prefix=prefix, host="web-01") + global_cfg = global_config_data() + host_cfg = host_config_data("web-01") - self.assertEqual(len(written), 2) - global_cfg = load_global_config(prefix / "config" / "global.yaml") - host_cfg = load_host_config(prefix / "config" / "hosts" / "web-01.yaml") - self.assertEqual(global_cfg["backup_root"], "/backups") - self.assertEqual(global_cfg["pobsync_home"], str(prefix)) - self.assertEqual(global_cfg["ssh"]["user"], "backup") - self.assertEqual(global_cfg["ssh"]["port"], 2222) - self.assertEqual(global_cfg["retention_defaults"]["daily"], 7) - self.assertEqual(host_cfg["host"], "web-01") - self.assertEqual(host_cfg["address"], "web-01.example.test") - self.assertEqual(host_cfg["retention"]["daily"], 7) - self.assertEqual(host_cfg["excludes_add"], ["/tmp/***"]) - self.assertNotIn("unknown", global_cfg) - self.assertNotIn("unknown", host_cfg) + self.assertEqual(global_cfg["backup_root"], "/backups") + self.assertEqual(global_cfg["pobsync_home"], "/var/lib/pobsync") + self.assertEqual(global_cfg["ssh"]["user"], "backup") + self.assertEqual(global_cfg["ssh"]["port"], 2222) + self.assertEqual(global_cfg["retention_defaults"]["daily"], 7) + self.assertEqual(host_cfg["host"], "web-01") + self.assertEqual(host_cfg["address"], "web-01.example.test") + self.assertEqual(host_cfg["retention"]["daily"], 7) + self.assertEqual(host_cfg["excludes_add"], ["/tmp/***"]) + self.assertNotIn("unknown", global_cfg) + self.assertNotIn("unknown", host_cfg) diff --git a/src/pobsync_backend/tests/test_retention_config_source.py b/src/pobsync_backend/tests/test_retention_config_source.py index 5a28dd7..8df8818 100644 --- a/src/pobsync_backend/tests/test_retention_config_source.py +++ b/src/pobsync_backend/tests/test_retention_config_source.py @@ -7,6 +7,7 @@ from tempfile import TemporaryDirectory from django.test import SimpleTestCase from pobsync.commands.retention_plan import run_retention_plan +from pobsync.errors import ConfigError from pobsync.util import write_yaml_atomic @@ -24,6 +25,15 @@ class FakeConfigSource: class RetentionConfigSourceTests(SimpleTestCase): + def test_retention_plan_requires_explicit_config_source(self) -> None: + with self.assertRaisesMessage(ConfigError, "A Django config source is required."): + run_retention_plan( + prefix=Path("/missing-prefix"), + host="web-01", + kind="scheduled", + protect_bases=False, + ) + def test_retention_plan_uses_injected_config_source(self) -> None: with TemporaryDirectory() as tmp: root = Path(tmp) / "backups" diff --git a/src/pobsync_backend/tests/test_run_scheduled_config_source.py b/src/pobsync_backend/tests/test_run_scheduled_config_source.py index 0d599c5..fce2dc1 100644 --- a/src/pobsync_backend/tests/test_run_scheduled_config_source.py +++ b/src/pobsync_backend/tests/test_run_scheduled_config_source.py @@ -7,6 +7,7 @@ from unittest.mock import patch from django.test import SimpleTestCase from pobsync.commands.run_scheduled import run_scheduled +from pobsync.errors import ConfigError from pobsync.rsync import RsyncResult @@ -34,6 +35,10 @@ class FakeConfigSource: class RunScheduledConfigSourceTests(SimpleTestCase): + def test_requires_explicit_config_source(self) -> None: + with self.assertRaisesMessage(ConfigError, "A Django config source is required."): + run_scheduled(prefix=Path("/missing-prefix"), host="web-01", dry_run=True) + def test_dry_run_uses_injected_config_source(self) -> None: with patch("pobsync.commands.run_scheduled.run_rsync") as run_rsync: run_rsync.return_value = RsyncResult(exit_code=0, command=["rsync", "--archive"])