diff --git a/pkgs/bay/app/drivers/docker/docker.py b/pkgs/bay/app/drivers/docker/docker.py index b36cb47..681c5f2 100644 --- a/pkgs/bay/app/drivers/docker/docker.py +++ b/pkgs/bay/app/drivers/docker/docker.py @@ -88,6 +88,8 @@ def __init__(self) -> None: self._log = logger.bind(driver="docker") self._client: aiodocker.Docker | None = None + self._cargo_root_path = Path(settings.cargo.root_path) + self._configured_host_root = settings.cargo.host_root_path # Cached host-side cargo root path, resolved once on first use. self._resolved_host_root: str | None | _UNSET = _UNSET @@ -109,15 +111,13 @@ async def _resolve_host_root(self) -> str | None: if self._resolved_host_root is not _UNSET: return self._resolved_host_root # type: ignore[return-value] - settings = get_settings() - # 1. Explicit config always wins. - if settings.cargo.host_root_path: - self._resolved_host_root = settings.cargo.host_root_path + if self._configured_host_root: + self._resolved_host_root = self._configured_host_root return self._resolved_host_root # 2. Auto-detect via /proc/self/mountinfo. - root_path = settings.cargo.root_path.rstrip("/") + root_path = str(self._cargo_root_path).rstrip("/") or "/" try: with open("/proc/self/mountinfo") as f: for line in f: @@ -512,6 +512,29 @@ async def logs(self, container_id: str, tail: int = 100) -> str: # Volume management + def _local_cargo_path(self, volume_ref: str, host_root: str) -> Path: + """Map a validated host bind path or cargo name to Bay's mount.""" + ref_path = Path(volume_ref) + if ref_path.is_absolute(): + try: + relative_ref = ref_path.relative_to(Path(host_root)) + except ValueError as exc: + raise ValueError( + f"Cargo bind path is outside the configured host root: {volume_ref}" + ) from exc + else: + relative_ref = ref_path + + if len(relative_ref.parts) != 1 or not relative_ref.name.startswith( + "bay-cargo-" + ): + raise ValueError(f"Invalid cargo bind reference: {volume_ref}") + + local_path = self._cargo_root_path / relative_ref.name + if local_path.is_symlink(): + raise ValueError(f"Cargo bind path must not be a symbolic link: {volume_ref}") + return local_path + async def create_volume(self, name: str, labels: dict[str, str] | None = None) -> str: """Create a cargo volume. @@ -527,11 +550,18 @@ async def create_volume(self, name: str, labels: dict[str, str] | None = None) - host_root = await self._resolve_host_root() if host_root: - # Bind-mount mode: directory on the Docker host - cargo_path = Path(host_root) / name - cargo_path.mkdir(parents=True, exist_ok=True) - self._log.info("docker.create_volume.bind", name=name, path=str(cargo_path)) - return str(cargo_path) + # Create through Bay's mounted path, then return the equivalent + # Docker-host path for use in the daemon's Binds configuration. + local_path = self._local_cargo_path(name, host_root) + local_path.mkdir(parents=True, exist_ok=True) + host_path = Path(host_root) / Path(name).name + self._log.info( + "docker.create_volume.bind", + name=name, + local_path=str(local_path), + path=str(host_path), + ) + return str(host_path) # Named-volume mode: Docker manages the volume lifecycle client = await self._get_client() @@ -544,11 +574,14 @@ async def delete_volume(self, name: str) -> None: host_root = await self._resolve_host_root() if host_root: - # Bind-mount mode: name is already a host path, delete directory - cargo_path = Path(name) - if cargo_path.exists(): - shutil.rmtree(cargo_path, ignore_errors=True) - self._log.info("docker.delete_volume.bind", path=str(cargo_path)) + local_path = self._local_cargo_path(name, host_root) + if local_path.exists(): + shutil.rmtree(local_path, ignore_errors=True) + self._log.info( + "docker.delete_volume.bind", + local_path=str(local_path), + path=name, + ) else: # Named-volume mode: name is volume name try: @@ -564,8 +597,7 @@ async def volume_exists(self, name: str) -> bool: host_root = await self._resolve_host_root() if host_root: - # Bind-mount mode: name is already the host path - return Path(name).is_dir() + return self._local_cargo_path(name, host_root).is_dir() # Named-volume mode try: diff --git a/pkgs/bay/tests/unit/drivers/test_docker_bind_cargo_paths.py b/pkgs/bay/tests/unit/drivers/test_docker_bind_cargo_paths.py new file mode 100644 index 0000000..501c1b5 --- /dev/null +++ b/pkgs/bay/tests/unit/drivers/test_docker_bind_cargo_paths.py @@ -0,0 +1,78 @@ +from __future__ import annotations + +import tempfile +import unittest +from pathlib import Path +from unittest.mock import AsyncMock, Mock + +from app.drivers.docker.docker import DockerDriver + + +class TestDockerBindCargoPaths(unittest.IsolatedAsyncioTestCase): + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self.temp_dir.cleanup) + + self.local_root = Path(self.temp_dir.name) / "container-cargos" + self.driver = DockerDriver.__new__(DockerDriver) + self.driver._cargo_root_path = self.local_root + self.driver._log = Mock() + self.driver._resolve_host_root = AsyncMock( + return_value="/daemon-only/shipyard-cargos" + ) + + async def test_create_volume_uses_local_mount_and_returns_host_bind_path( + self, + ) -> None: + volume = await self.driver.create_volume("bay-cargo-test") + + self.assertEqual( + volume, + "/daemon-only/shipyard-cargos/bay-cargo-test", + ) + self.assertTrue((self.local_root / "bay-cargo-test").is_dir()) + + async def test_volume_exists_checks_local_mount(self) -> None: + (self.local_root / "bay-cargo-test").mkdir(parents=True) + + exists = await self.driver.volume_exists( + "/daemon-only/shipyard-cargos/bay-cargo-test" + ) + + self.assertTrue(exists) + + async def test_delete_volume_removes_directory_from_local_mount(self) -> None: + local_path = self.local_root / "bay-cargo-test" + local_path.mkdir(parents=True) + (local_path / "artifact.txt").write_text("test") + + await self.driver.delete_volume( + "/daemon-only/shipyard-cargos/bay-cargo-test" + ) + + self.assertFalse(local_path.exists()) + + async def test_delete_volume_rejects_cargo_root(self) -> None: + sentinel = self.local_root / "keep.txt" + self.local_root.mkdir(parents=True) + sentinel.write_text("keep") + + with self.assertRaises(ValueError): + await self.driver.delete_volume("/") + + self.assertTrue(sentinel.is_file()) + + async def test_delete_volume_rejects_path_outside_configured_host_root( + self, + ) -> None: + local_path = self.local_root / "bay-cargo-test" + local_path.mkdir(parents=True) + + with self.assertRaises(ValueError): + await self.driver.delete_volume("/other-root/bay-cargo-test") + + self.assertTrue(local_path.is_dir()) + + +if __name__ == "__main__": + unittest.main()