merge: file-level mount overlays for ResolvedMount
This commit is contained in:
@@ -494,13 +494,16 @@ def apply_resolved_profile(
|
||||
full_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
full_path.write_text(content)
|
||||
|
||||
volume_mounts.append(
|
||||
{
|
||||
"source": str(mount_dir),
|
||||
"target": expanded_target,
|
||||
"type": "bind",
|
||||
}
|
||||
)
|
||||
# Mount each file individually so sibling files from other mounts
|
||||
# (e.g. git repo directories) are preserved.
|
||||
file_target = os.path.join(expanded_target, file_path)
|
||||
volume_mounts.append(
|
||||
{
|
||||
"source": str(full_path),
|
||||
"target": file_target,
|
||||
"type": "bind",
|
||||
}
|
||||
)
|
||||
|
||||
return env_vars, files, volume_mounts, resolved.runtime_hints
|
||||
|
||||
|
||||
@@ -6,6 +6,9 @@ from src.models.config_profile import ConfigProfile, ConfigProfileInclude
|
||||
from src.services.config_profile_resolver import (
|
||||
ConfigProfileCycleError,
|
||||
ConfigProfileNotFoundError,
|
||||
ResolvedMount,
|
||||
ResolvedProfile,
|
||||
apply_resolved_profile,
|
||||
check_include_cycle,
|
||||
resolve_profile,
|
||||
_merge_env_vars,
|
||||
@@ -479,6 +482,81 @@ class TestResolveProfile:
|
||||
await resolve_profile(db_session, uuid.uuid4())
|
||||
|
||||
|
||||
class TestApplyResolvedProfile:
|
||||
"""Unit tests for apply_resolved_profile file-level mount behavior."""
|
||||
|
||||
def test_mounts_individual_files_not_directory(self, tmp_path) -> None:
|
||||
"""Each file in a ResolvedMount should be mounted individually, not the staging dir."""
|
||||
resolved = ResolvedProfile(
|
||||
profile_id=uuid.uuid4(),
|
||||
profile_name="test",
|
||||
mounts={
|
||||
"/app": ResolvedMount(
|
||||
target="/app",
|
||||
mode="rw",
|
||||
files={"config.json": '{"key": "value"}', "nested/file.txt": "hello"},
|
||||
)
|
||||
},
|
||||
)
|
||||
env, files, volumes, hints = apply_resolved_profile(str(tmp_path), resolved)
|
||||
|
||||
assert len(volumes) == 2
|
||||
targets = {v["target"] for v in volumes}
|
||||
assert "/app/config.json" in targets
|
||||
assert "/app/nested/file.txt" in targets
|
||||
# No directory-level mount
|
||||
assert "/app" not in targets
|
||||
|
||||
def test_file_mount_preserves_sibling_files(self, tmp_path) -> None:
|
||||
"""File-level mounts should not hide sibling files from other mounts."""
|
||||
resolved = ResolvedProfile(
|
||||
profile_id=uuid.uuid4(),
|
||||
profile_name="test",
|
||||
mounts={
|
||||
"/workspace/x/y": ResolvedMount(
|
||||
target="/workspace/x/y",
|
||||
mode="rw",
|
||||
files={"z.json": "override"},
|
||||
)
|
||||
},
|
||||
)
|
||||
env, files, volumes, hints = apply_resolved_profile(str(tmp_path), resolved)
|
||||
|
||||
assert len(volumes) == 1
|
||||
assert volumes[0]["target"] == "/workspace/x/y/z.json"
|
||||
assert volumes[0]["source"].endswith("z.json")
|
||||
|
||||
def test_empty_mount_produces_no_volumes(self, tmp_path) -> None:
|
||||
"""A mount with no files should not produce any volume entries."""
|
||||
resolved = ResolvedProfile(
|
||||
profile_id=uuid.uuid4(),
|
||||
profile_name="test",
|
||||
mounts={
|
||||
"/app": ResolvedMount(target="/app", mode="rw", files={})
|
||||
},
|
||||
)
|
||||
env, files, volumes, hints = apply_resolved_profile(str(tmp_path), resolved)
|
||||
assert volumes == []
|
||||
|
||||
def test_home_expansion_in_file_mount_target(self, tmp_path) -> None:
|
||||
"""~ in mount target should be expanded to home_dir for file mounts."""
|
||||
resolved = ResolvedProfile(
|
||||
profile_id=uuid.uuid4(),
|
||||
profile_name="test",
|
||||
mounts={
|
||||
"~/.config": ResolvedMount(
|
||||
target="~/.config",
|
||||
mode="rw",
|
||||
files={"app.toml": "setting = 1"},
|
||||
)
|
||||
},
|
||||
)
|
||||
env, files, volumes, hints = apply_resolved_profile(
|
||||
str(tmp_path), resolved, home_dir="/home/user"
|
||||
)
|
||||
assert volumes[0]["target"] == "/home/user/.config/app.toml"
|
||||
|
||||
|
||||
class TestCheckIncludeCycle:
|
||||
"""Unit tests for include cycle checking."""
|
||||
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
# Exploration: File-Level Mount Overlays
|
||||
|
||||
## Follow-up to Mount Specificity Ordering
|
||||
|
||||
The sorting fix (parent paths before child paths) was correct for directory mounts,
|
||||
but it exposed a deeper bug: `ResolvedMount` always mounts its staging directory
|
||||
as a single bind mount. When a config profile mount targets `/workspace/x/y` and
|
||||
contains a single file `z.json`, the staging directory (containing only `z.json`)
|
||||
replaces the ENTIRE `/workspace/x/y` directory, hiding all sibling files from the
|
||||
git repo.
|
||||
|
||||
## Root Cause
|
||||
|
||||
In `apply_resolved_profile`:
|
||||
```python
|
||||
volume_mounts.append({
|
||||
"source": str(mount_dir), # staging dir with ONLY z.json
|
||||
"target": expanded_target, # /workspace/x/y
|
||||
"type": "bind",
|
||||
})
|
||||
```
|
||||
|
||||
This mounts a directory. Docker bind mounts at a directory path completely replace
|
||||
the target directory. There is no merge.
|
||||
|
||||
## What the User Expects
|
||||
|
||||
Git repo mount: `/repo/x` → `/workspace/x` (directory with many files)
|
||||
Config profile mount: `z.json` → `/workspace/x/y/z.json` (single file overlay)
|
||||
|
||||
Expected: `/workspace/x/y/` contains all repo files PLUS the overlaid `z.json`.
|
||||
Actual (before sorting): parent mount hides child mount (child never visible).
|
||||
Actual (after sorting): child directory mount replaces parent subdirectory
|
||||
(`/workspace/x/y` now contains ONLY `z.json`).
|
||||
|
||||
## Solution
|
||||
|
||||
Mount each file individually instead of the staging directory.
|
||||
|
||||
```python
|
||||
for file_path, content in mount.files.items():
|
||||
full_path = mount_dir / file_path
|
||||
full_path.write_text(content)
|
||||
volume_mounts.append({
|
||||
"source": str(full_path),
|
||||
"target": os.path.join(expanded_target, file_path),
|
||||
"type": "bind",
|
||||
})
|
||||
```
|
||||
|
||||
This creates file-level bind mounts. Docker mounts a single file without affecting
|
||||
sibling files in the parent directory.
|
||||
|
||||
## Edge Cases
|
||||
|
||||
- Empty `files` dict: skip, no mounts created.
|
||||
- Nested file paths (`a/b/c.txt`): mount `staging/a/b/c.txt` → `target/a/b/c.txt`.
|
||||
Docker creates parent directories as needed.
|
||||
- File target already exists in git repo: file mount wins (desired override behavior).
|
||||
- No git repo mount (directory doesn't exist in image): Docker creates parent dirs
|
||||
for the first file mount.
|
||||
|
||||
## Sorting Fix Status
|
||||
|
||||
KEEP the sorting. It is still correct and necessary for cases where directory
|
||||
mounts genuinely override parent directories (e.g., a git mount to `/workspace/x`
|
||||
and another git mount to `/workspace/x/sub`). File-level mounts also benefit from
|
||||
sorting because the parent directory mount must exist before file mounts inside it.
|
||||
Reference in New Issue
Block a user