From e80f11d5e4fc4bbf5c6ce94b7951b34974d300c5 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Mon, 17 Aug 2026 05:25:54 +0200 Subject: [PATCH] fix(restore): refuse to write files into an unmounted backing store Writing into a volume's mountpoint only restores it when the mountpoint is the storage. A volume with driver options - NFS, a bind device, tmpfs - keeps the same /var/lib/docker/volumes//_data path, but docker mounts the real backing store over it on demand and unmounts it again when the last consumer stops. Restoring while nothing holds it lands in the empty directory underneath, is hidden by the next mount, and rsync reports success. The declaration decides, not the mount table: the driver and its options are true at every moment, where the mount table is only true while a container happens to hold the volume. Co-Authored-By: Claude Opus 5 (1M context) --- src/baudolo/restore/files.py | 33 ++++- .../test_e2e_restore_files_backing_store.py | 120 ++++++++++++++++++ .../unit/restore/test_files_backing_store.py | 62 +++++++++ 3 files changed, 211 insertions(+), 4 deletions(-) create mode 100644 tests/e2e/test_e2e_restore_files_backing_store.py create mode 100644 tests/unit/restore/test_files_backing_store.py diff --git a/src/baudolo/restore/files.py b/src/baudolo/restore/files.py index e10eed3..7756343 100644 --- a/src/baudolo/restore/files.py +++ b/src/baudolo/restore/files.py @@ -1,9 +1,23 @@ +"""Restore a volume's file tree by writing into its mountpoint. + +That shortcut only holds for a plain local volume, where the mountpoint *is* +the storage. A volume with driver options - NFS, a bind device, tmpfs - keeps +the same ``/var/lib/docker/volumes//_data`` path, but docker mounts the +real backing store over it on demand and unmounts it again when the last +consumer stops. Writing there while nothing has it mounted lands in the empty +directory underneath, is hidden by the next mount, and rsync reports success. +""" + from __future__ import annotations import os import sys -from .run import docker_volume_exists, run +from .run import docker_volume_exists, run, stdout_of + +INSPECT_FORMAT = ( + "{{ .Mountpoint }}|{{ .Driver }}|{{ if .Options }}opts{{ else }}plain{{ end }}" +) def restore_volume_files(volume_name: str, backup_files_dir: str) -> int: @@ -18,11 +32,11 @@ def restore_volume_files(volume_name: str, backup_files_dir: str) -> int: print(f"Volume {volume_name} already exists.") cp = run( - ["docker", "volume", "inspect", "--format", "{{ .Mountpoint }}", volume_name], + ["docker", "volume", "inspect", "--format", INSPECT_FORMAT, volume_name], capture=True, ) - raw = cp.stdout or b"" - mountpoint = (raw.decode() if isinstance(raw, bytes) else raw).strip() + fields = stdout_of(cp).split("|") + mountpoint = fields[0] if fields else "" if not mountpoint: print( f"ERROR: could not resolve mountpoint for volume {volume_name}", @@ -30,6 +44,17 @@ def restore_volume_files(volume_name: str, backup_files_dir: str) -> int: ) return 2 + driver, options = (fields + ["local", "plain"])[1:3] + if (driver != "local" or options == "opts") and not os.path.ismount(mountpoint): + print( + f"ERROR: volume {volume_name} has a backing store of its own " + f"(driver {driver}) but nothing has it mounted; writing to " + f"{mountpoint} now would land under the mount and be lost. " + "Start a container that mounts the volume, then restore again.", + file=sys.stderr, + ) + return 2 + src = os.path.join(backup_files_dir, "") dest = os.path.join(mountpoint, "") run(["rsync", "-avv", "--delete", src, dest]) diff --git a/tests/e2e/test_e2e_restore_files_backing_store.py b/tests/e2e/test_e2e_restore_files_backing_store.py new file mode 100644 index 0000000..3a22c24 --- /dev/null +++ b/tests/e2e/test_e2e_restore_files_backing_store.py @@ -0,0 +1,120 @@ +"""Restoring files into a volume that has a backing store of its own. + +Docker keeps the same ``/var/lib/docker/volumes//_data`` path for such a +volume and mounts the real storage over it only while a container holds it. +Writing there unmounted lands in the empty directory underneath, is hidden by +the next mount, and rsync reports success - so the restore has to refuse. +""" + +import unittest +from pathlib import Path + +from .helpers import ( + backup_path, + cleanup_docker, + ensure_empty_dir, + machine_hash, + require_docker, + run, + unique, +) + +MARKER = "restored-payload" +VERSION = "20260817000000" + + +def mountpoint_of(volume: str) -> Path: + return Path("/var/lib/docker/volumes") / volume / "_data" + + +def contents(directory: Path) -> list[str]: + return sorted(p.name for p in directory.iterdir()) if directory.is_dir() else [] + + +class TestE2ERestoreFilesBackingStore(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + require_docker() + cls.prefix = unique("baudolo-e2e-backing") + cls.repo_name = cls.prefix + cls.backups_dir = f"/tmp/{cls.prefix}/Backups" + cls.backing = Path(f"/tmp/{cls.prefix}/backing") + ensure_empty_dir(cls.backups_dir) + ensure_empty_dir(str(cls.backing)) + + cls.bound_volume = f"{cls.prefix}-bound" + cls.plain_volume = f"{cls.prefix}-plain" + cls.volumes = [cls.bound_volume, cls.plain_volume] + + for volume in cls.volumes: + files = ( + backup_path(cls.backups_dir, cls.repo_name, VERSION, volume) / "files" + ) + files.mkdir(parents=True, exist_ok=True) + (files / "marker.txt").write_text(MARKER, encoding="utf-8") + + run( + [ + "docker", + "volume", + "create", + "--driver", + "local", + "--opt", + "type=none", + "--opt", + "o=bind", + "--opt", + f"device={cls.backing}", + cls.bound_volume, + ] + ) + run(["docker", "volume", "create", cls.plain_volume]) + + cls.refused = cls.restore(cls.bound_volume) + cls.accepted = cls.restore(cls.plain_volume) + + @classmethod + def tearDownClass(cls) -> None: + cleanup_docker(containers=[], volumes=cls.volumes) + + @classmethod + def restore(cls, volume: str): + return run( + [ + "baudolo-restore", + "files", + volume, + machine_hash(), + VERSION, + "--backups-dir", + cls.backups_dir, + "--repo-name", + cls.repo_name, + ], + check=False, + ) + + def test_a_volume_with_its_own_backing_store_is_refused(self) -> None: + self.assertEqual(self.refused.returncode, 2, self.refused.stdout) + self.assertIn("backing store of its own", self.refused.stderr) + + def test_nothing_was_written_into_the_backing_store(self) -> None: + self.assertEqual(contents(self.backing), []) + + def test_nothing_was_written_under_the_mount_either(self) -> None: + self.assertEqual( + contents(mountpoint_of(self.bound_volume)), + [], + "the copy landed in the directory the next mount hides", + ) + + def test_a_plain_volume_is_still_restored(self) -> None: + self.assertEqual(self.accepted.returncode, 0, self.accepted.stderr) + restored = mountpoint_of(self.plain_volume) / "marker.txt" + self.assertTrue(restored.is_file(), f"{restored} missing") + self.assertEqual(restored.read_text(encoding="utf-8"), MARKER) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/restore/test_files_backing_store.py b/tests/unit/restore/test_files_backing_store.py new file mode 100644 index 0000000..a12a247 --- /dev/null +++ b/tests/unit/restore/test_files_backing_store.py @@ -0,0 +1,62 @@ +import tempfile +import unittest +from unittest.mock import MagicMock, patch + +from baudolo.restore import files as files_mod + + +class TestBackingStoreGuard(unittest.TestCase): + def restore(self, inspect: str, mounted: bool) -> tuple[int, list]: + calls = [] + + def _run(cmd, **kwargs): + calls.append(cmd) + return MagicMock(stdout=inspect.encode()) + + with ( + patch.object(files_mod, "docker_volume_exists", return_value=True), + patch.object(files_mod.os.path, "ismount", return_value=mounted), + patch.object(files_mod, "run", side_effect=_run), + ): + code = files_mod.restore_volume_files("app_data", tempfile.mkdtemp()) + return code, calls + + def rsynced(self, calls: list) -> bool: + return any(cmd[0] == "rsync" for cmd in calls) + + def test_plain_local_volume_is_restored_unmounted(self) -> None: + code, calls = self.restore("/var/lib/docker/volumes/a/_data|local|plain", False) + self.assertEqual(code, 0) + self.assertTrue(self.rsynced(calls)) + + def test_volume_with_driver_options_is_refused_while_unmounted(self) -> None: + code, calls = self.restore("/var/lib/docker/volumes/a/_data|local|opts", False) + self.assertEqual(code, 2) + self.assertFalse( + self.rsynced(calls), + "an NFS or bind volume writes under the mount and reports success", + ) + + def test_volume_with_driver_options_is_restored_once_mounted(self) -> None: + code, calls = self.restore("/var/lib/docker/volumes/a/_data|local|opts", True) + self.assertEqual(code, 0) + self.assertTrue(self.rsynced(calls)) + + def test_foreign_driver_is_refused_while_unmounted(self) -> None: + code, calls = self.restore("/mnt/gluster/a|glusterfs|plain", False) + self.assertEqual(code, 2) + self.assertFalse(self.rsynced(calls)) + + def test_an_unresolvable_mountpoint_still_fails_first(self) -> None: + code, calls = self.restore("|local|plain", False) + self.assertEqual(code, 2) + self.assertFalse(self.rsynced(calls)) + + def test_a_format_without_the_new_fields_is_treated_as_plain(self) -> None: + code, calls = self.restore("/var/lib/docker/volumes/a/_data", False) + self.assertEqual(code, 0) + self.assertTrue(self.rsynced(calls)) + + +if __name__ == "__main__": + unittest.main()