Compare commits
4 Commits
cc094765fd
..
main
| Author | SHA1 | Date | |
|---|---|---|---|
| 6fea44067f | |||
| bf8ff91b31 | |||
| 315ed04979 | |||
| 9a04ab262b |
@@ -19,11 +19,6 @@ what would send podman down the `newuidmap` path that cannot work here.
|
||||
The agent still talks to `docker` and `docker compose`; those speak to
|
||||
podman's Docker-compatible API socket, so nothing in the agent's habits
|
||||
changes.
|
||||
|
||||
Nested containers run *within* the bottle boundary, not inside a new one: the
|
||||
single-UID mapping means `root` in a nested container is the agent user
|
||||
outside it. This is for build and test workloads, not for sandboxing
|
||||
untrusted code.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -65,9 +60,11 @@ def build_image(
|
||||
) -> str:
|
||||
"""Layer the nested-container tooling onto an already-built agent image.
|
||||
|
||||
Only what the flag is meant to gate lands here. Podman itself is already
|
||||
in every built-in agent image (issue #451); the storage/network helpers,
|
||||
the Docker CLI, and the compose plugin are the ~100MB this flag buys.
|
||||
Podman and its networking stack live here rather than in the base agent
|
||||
images so that bottles without the flag pay no image-size cost.
|
||||
|
||||
# TODO(#394): replace this hand-rolled Dockerfile with a docker-layer
|
||||
# abstraction once that infrastructure exists.
|
||||
"""
|
||||
image = f"{base_image}{IMAGE_SUFFIX}"
|
||||
init_script = Path(__file__).with_name("nested-containers-init.sh")
|
||||
@@ -82,9 +79,11 @@ def build_image(
|
||||
"COPY --from=docker_cli /usr/local/libexec/docker/cli-plugins/"
|
||||
"docker-compose /usr/local/libexec/docker/cli-plugins/docker-compose\n"
|
||||
"RUN apt-get update \\\n"
|
||||
# podman 5's networking stack, installed explicitly because the
|
||||
# base image's `--no-install-recommends` podman does not pull it
|
||||
# in and each piece fails at a different, misleading layer:
|
||||
# podman 5's networking stack, installed explicitly because
|
||||
# --no-install-recommends omits it and each missing piece fails
|
||||
# at a different, misleading layer:
|
||||
# podman -> moved here from the base agent images so that
|
||||
# bottles without nested_containers pay no cost
|
||||
# passt -> `pasta`, the default rootless netns helper
|
||||
# (podman 4 used slirp4netns); without it
|
||||
# nothing starts: "could not find pasta"
|
||||
@@ -95,7 +94,7 @@ def build_image(
|
||||
# looks healthy
|
||||
# slirp4netns stays as the documented fallback for pasta.
|
||||
" && apt-get install -y --no-install-recommends "
|
||||
"aardvark-dns fuse-overlayfs netavark nftables passt "
|
||||
"aardvark-dns fuse-overlayfs netavark nftables passt podman "
|
||||
"slirp4netns uidmap \\\n"
|
||||
" && rm -rf /var/lib/apt/lists/* \\\n"
|
||||
# Deliberate: an empty subordinate range keeps podman on the
|
||||
|
||||
@@ -26,7 +26,6 @@ RUN apt-get update \
|
||||
ca-certificates \
|
||||
curl \
|
||||
openssh-client \
|
||||
podman \
|
||||
ripgrep \
|
||||
iproute2 \
|
||||
dnsutils \
|
||||
|
||||
@@ -11,7 +11,6 @@ RUN apt-get update \
|
||||
ca-certificates \
|
||||
curl \
|
||||
openssh-client \
|
||||
podman \
|
||||
procps \
|
||||
ripgrep \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
@@ -11,7 +11,6 @@ RUN apt-get update \
|
||||
curl \
|
||||
fd-find \
|
||||
openssh-client \
|
||||
podman \
|
||||
ripgrep \
|
||||
&& ln -s /usr/bin/fdfind /usr/local/bin/fd \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
@@ -49,6 +49,10 @@ class ManifestBottle:
|
||||
# costs image weight, a resident service, and relaxed guest device modes
|
||||
# that the majority of bottles never need.
|
||||
nested_containers: bool = False
|
||||
# Source fields retained across extends/runtime composition. Boolean
|
||||
# defaults otherwise erase the distinction between "omitted" and an
|
||||
# explicitly declared value (especially False).
|
||||
declared_fields: frozenset[str] = frozenset()
|
||||
|
||||
@classmethod
|
||||
def from_dict(cls, name: str, raw: object) -> "ManifestBottle":
|
||||
@@ -139,4 +143,5 @@ class ManifestBottle:
|
||||
env=env, agent_provider=agent_provider, git=git,
|
||||
git_user=git_user, egress=egress, supervise=supervise_raw,
|
||||
nested_containers=nested_raw,
|
||||
declared_fields=frozenset(d),
|
||||
)
|
||||
|
||||
@@ -8,6 +8,17 @@ from .manifest_git import ManifestGitUser, parse_git_gate_config
|
||||
from .manifest_util import ManifestError, as_json_object
|
||||
|
||||
|
||||
def _overlay_declared_bool(
|
||||
base: ManifestBottle,
|
||||
override: ManifestBottle,
|
||||
field: str,
|
||||
) -> bool:
|
||||
"""Overlay a defaulted boolean only when override declared it."""
|
||||
value = getattr(override if field in override.declared_fields else base, field)
|
||||
assert isinstance(value, bool)
|
||||
return value
|
||||
|
||||
|
||||
def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle":
|
||||
"""Merge an ordered list of pre-resolved ManifestBottle objects.
|
||||
|
||||
@@ -15,16 +26,13 @@ def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle":
|
||||
the same field-merge rules as the file-based extends machinery:
|
||||
env: dict merge, later wins; git_user: per-field overlay, later
|
||||
wins on non-empty; git (repos): union by name, later wins; egress
|
||||
routes: concatenate; agent_provider, supervise: later
|
||||
replaces; nested_containers: OR (see below).
|
||||
routes: concatenate; agent_provider, supervise, nested_containers:
|
||||
later replaces (presence-aware).
|
||||
|
||||
nested_containers is OR'd rather than replaced because these objects
|
||||
are already resolved: a bottle that never mentions the key is
|
||||
indistinguishable from one that sets it false, so "later replaces"
|
||||
would let any bottle composed after a container-enabled one silently
|
||||
drop the capability. The file-based `extends:` path still sees the
|
||||
raw keys, so there an explicit `nested_containers: false` in a child
|
||||
turns it back off.
|
||||
Defaulted booleans use presence-aware replacement: if the later bottle
|
||||
was loaded from a source that explicitly declared the key, its value
|
||||
wins (so an explicit `false` can override an earlier `true`). If the
|
||||
later bottle never mentioned the key, the earlier value is preserved.
|
||||
"""
|
||||
if not bottles:
|
||||
raise ValueError("merge_bottles_runtime requires at least one bottle")
|
||||
@@ -62,8 +70,11 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle
|
||||
git=merged_git,
|
||||
git_user=merged_git_user,
|
||||
egress=merged_egress,
|
||||
supervise=override.supervise,
|
||||
nested_containers=base.nested_containers or override.nested_containers,
|
||||
supervise=_overlay_declared_bool(base, override, "supervise"),
|
||||
nested_containers=_overlay_declared_bool(
|
||||
base, override, "nested_containers"
|
||||
),
|
||||
declared_fields=base.declared_fields | override.declared_fields,
|
||||
)
|
||||
|
||||
|
||||
@@ -215,8 +226,11 @@ def _fold_two_bottles(
|
||||
git=merged_git,
|
||||
git_user=merged_git_user,
|
||||
egress=merged_egress,
|
||||
supervise=later.supervise,
|
||||
nested_containers=earlier.nested_containers or later.nested_containers,
|
||||
supervise=_overlay_declared_bool(earlier, later, "supervise"),
|
||||
nested_containers=_overlay_declared_bool(
|
||||
earlier, later, "nested_containers"
|
||||
),
|
||||
declared_fields=earlier.declared_fields | later.declared_fields,
|
||||
), merged_repos_raw
|
||||
|
||||
|
||||
@@ -274,13 +288,9 @@ def _merge_bottles(
|
||||
if "agent_provider" in child_raw
|
||||
else parent.agent_provider
|
||||
)
|
||||
merged_supervise = (
|
||||
child.supervise if "supervise" in child_raw else parent.supervise
|
||||
)
|
||||
merged_nested_containers = (
|
||||
child.nested_containers
|
||||
if "nested_containers" in child_raw
|
||||
else parent.nested_containers
|
||||
merged_supervise = _overlay_declared_bool(parent, child, "supervise")
|
||||
merged_nested_containers = _overlay_declared_bool(
|
||||
parent, child, "nested_containers"
|
||||
)
|
||||
validate_egress_routes(name, merged_egress.routes)
|
||||
|
||||
@@ -292,6 +302,7 @@ def _merge_bottles(
|
||||
egress=merged_egress,
|
||||
supervise=merged_supervise,
|
||||
nested_containers=merged_nested_containers,
|
||||
declared_fields=parent.declared_fields | child.declared_fields,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -21,10 +21,12 @@ class TestBuiltinAgentImages(unittest.TestCase):
|
||||
r"(?m)^FROM node:22-trixie-slim\s*$",
|
||||
)
|
||||
|
||||
def test_all_install_podman(self):
|
||||
def test_none_install_podman(self):
|
||||
# podman lives in the nested-containers derived layer (nested_containers.py),
|
||||
# not in the base agent images, so bottles without the flag pay no cost.
|
||||
for dockerfile in _AGENT_DOCKERFILES:
|
||||
with self.subTest(provider=dockerfile.parent.name):
|
||||
self.assertRegex(
|
||||
self.assertNotRegex(
|
||||
dockerfile.read_text(),
|
||||
re.compile(r"(?m)^\s*podman(?:\s|\\|$)"),
|
||||
)
|
||||
|
||||
@@ -108,7 +108,7 @@ class TestNestedContainersImage(unittest.TestCase):
|
||||
calls.append((image, context, dockerfile))
|
||||
text = Path(dockerfile).read_text(encoding="utf-8")
|
||||
self.assertIn("FROM agent:base", text)
|
||||
self.assertIn("aardvark-dns fuse-overlayfs netavark nftables passt", text)
|
||||
self.assertIn("aardvark-dns fuse-overlayfs netavark nftables passt podman", text)
|
||||
self.assertIn("USER node", text)
|
||||
self.assertTrue((Path(context) / "nested-containers-init.sh").is_file())
|
||||
|
||||
@@ -128,7 +128,7 @@ class TestNestedContainersImage(unittest.TestCase):
|
||||
seen.append(Path(dockerfile).read_text(encoding="utf-8"))
|
||||
|
||||
nested_containers.build_image("agent:base", build)
|
||||
for package in ("passt", "nftables", "aardvark-dns"):
|
||||
for package in ("podman", "passt", "nftables", "aardvark-dns"):
|
||||
self.assertIn(package, seen[0])
|
||||
|
||||
def test_strips_subordinate_ranges_so_podman_avoids_newuidmap(self) -> None:
|
||||
|
||||
@@ -56,16 +56,56 @@ class TestMergeBottlesRuntime(unittest.TestCase):
|
||||
result = merge_bottles_runtime([base, override])
|
||||
self.assertFalse(result.supervise)
|
||||
|
||||
def test_nested_containers_survives_a_later_bottle(self):
|
||||
"""OR, not replace: a resolved bottle that never mentioned the key is
|
||||
indistinguishable from one that set it false, so `--bottle with-docker
|
||||
--bottle claude-dev` must not silently drop the capability."""
|
||||
def test_supervise_survives_a_later_bottle_that_omits_the_key(self):
|
||||
disabled = _bottle(supervise=False)
|
||||
quiet = _bottle(env={"X": "1"})
|
||||
self.assertFalse(merge_bottles_runtime([disabled, quiet]).supervise)
|
||||
|
||||
def test_supervise_explicit_true_overrides_earlier_false(self):
|
||||
disabled = _bottle(supervise=False)
|
||||
enabled = _bottle(supervise=True)
|
||||
self.assertTrue(merge_bottles_runtime([disabled, enabled]).supervise)
|
||||
|
||||
def test_nested_containers_survives_a_later_bottle_that_omits_the_key(self):
|
||||
"""A bottle that never mentions nested_containers must not silently
|
||||
drop the capability: `--bottle with-docker --bottle claude-dev`."""
|
||||
enabled = _bottle(nested_containers=True)
|
||||
quiet = _bottle(env={"X": "1"})
|
||||
self.assertTrue(merge_bottles_runtime([enabled, quiet]).nested_containers)
|
||||
self.assertTrue(merge_bottles_runtime([quiet, enabled]).nested_containers)
|
||||
self.assertFalse(merge_bottles_runtime([quiet, quiet]).nested_containers)
|
||||
|
||||
def test_nested_containers_explicit_false_overrides_earlier_true(self):
|
||||
"""An explicit nested_containers: false in a later bottle must win
|
||||
over an earlier true (last-wins, presence-aware)."""
|
||||
enabled = _bottle(nested_containers=True)
|
||||
disabled = _bottle(nested_containers=False)
|
||||
self.assertFalse(merge_bottles_runtime([enabled, disabled]).nested_containers)
|
||||
|
||||
def test_nested_containers_explicit_true_overrides_earlier_false(self):
|
||||
enabled = _bottle(nested_containers=True)
|
||||
disabled = _bottle(nested_containers=False)
|
||||
self.assertTrue(merge_bottles_runtime([disabled, enabled]).nested_containers)
|
||||
|
||||
def test_extended_boolean_values_remain_explicit_at_runtime(self):
|
||||
idx = _index(
|
||||
bottles={
|
||||
"enabled": {"nested_containers": True, "supervise": True},
|
||||
"parent": {},
|
||||
"disabled_child": {
|
||||
"extends": "parent",
|
||||
"nested_containers": False,
|
||||
"supervise": False,
|
||||
},
|
||||
},
|
||||
agents={"impl": {"bottle": "enabled", "skills": [], "prompt": ""}},
|
||||
)
|
||||
result = idx.load_for_agent(
|
||||
"impl", ("enabled", "disabled_child")
|
||||
).bottle
|
||||
self.assertFalse(result.nested_containers)
|
||||
self.assertFalse(result.supervise)
|
||||
|
||||
def test_three_bottles_merged_left_to_right(self):
|
||||
b1 = _bottle(env={"A": "1", "B": "1", "C": "1"})
|
||||
b2 = _bottle(env={"B": "2", "C": "2"})
|
||||
|
||||
@@ -68,6 +68,23 @@ class TestExtendsBasic(unittest.TestCase):
|
||||
self.assertTrue(m.bottles["base"].supervise)
|
||||
self.assertFalse(m.bottles["off"].supervise)
|
||||
|
||||
def test_child_overrides_nested_containers_scalar(self):
|
||||
m = _build(
|
||||
base={"nested_containers": True},
|
||||
off={"extends": "base", "nested_containers": False},
|
||||
)
|
||||
self.assertTrue(m.bottles["base"].nested_containers)
|
||||
self.assertFalse(m.bottles["off"].nested_containers)
|
||||
|
||||
def test_inherited_boolean_declaration_is_preserved(self):
|
||||
m = _build(
|
||||
base={"nested_containers": True, "supervise": False},
|
||||
child={"extends": "base"},
|
||||
)
|
||||
child = m.bottles["child"]
|
||||
self.assertIn("nested_containers", child.declared_fields)
|
||||
self.assertIn("supervise", child.declared_fields)
|
||||
|
||||
def test_parent_resolved_once_for_multiple_children(self):
|
||||
# Two children sharing one parent: both inherit; the parent
|
||||
# is resolved once + cached. (Cache behavior is internal; we
|
||||
@@ -477,6 +494,26 @@ class TestExtendsMultiParent(unittest.TestCase):
|
||||
)
|
||||
self.assertTrue(m.bottles["child"].supervise)
|
||||
|
||||
def test_later_parent_omitting_boole_preserves_earlier_values(self):
|
||||
m = _build(
|
||||
p1={"nested_containers": True, "supervise": False},
|
||||
p2={"env": {"FROM_P2": "1"}},
|
||||
child={"extends": ["p1", "p2"]},
|
||||
)
|
||||
child = m.bottles["child"]
|
||||
self.assertTrue(child.nested_containers)
|
||||
self.assertFalse(child.supervise)
|
||||
|
||||
def test_later_parent_explicit_boole_override_earlier_values(self):
|
||||
m = _build(
|
||||
p1={"nested_containers": True, "supervise": False},
|
||||
p2={"nested_containers": False, "supervise": True},
|
||||
child={"extends": ["p1", "p2"]},
|
||||
)
|
||||
child = m.bottles["child"]
|
||||
self.assertFalse(child.nested_containers)
|
||||
self.assertTrue(child.supervise)
|
||||
|
||||
def test_child_supervise_overrides_all_parents(self):
|
||||
m = _build(
|
||||
p1={"supervise": True},
|
||||
|
||||
Reference in New Issue
Block a user