refactor(pipelock): PipelockProxy.prepare takes a Bottle, not (manifest, name)
test / run tests/run_tests.py (pull_request) Successful in 14s
test / run tests/run_tests.py (pull_request) Successful in 14s
Matches the allowlist-resolution helpers' shape: the caller resolves the bottle once and passes it in. Signature drops from (manifest, bottle_name, slug, yaml_path) to (bottle, slug, yaml_path). DockerBottleBackend.prepare_proxy uses manifest.bottle_for(agent_name) to get the bottle directly. Tests pass fixture.bottles[name]. prepare's docstring also explains what `slug` is: the lowercased, hyphen-normalized agent identifier used as the suffix in every per-agent resource name (agent container, pipelock container, the internal/egress networks). It's stored on the plan so start can derive the sidecar's container name. Top-level pipelock.py drops the Manifest import — no longer used.
This commit is contained in:
@@ -136,9 +136,9 @@ class DockerBottleBackend(BottleBackend):
|
|||||||
PipelockProxyPlan for the launch step to consume. Stage-only:
|
PipelockProxyPlan for the launch step to consume. Stage-only:
|
||||||
no Docker resources created yet."""
|
no Docker resources created yet."""
|
||||||
yaml_path = stage_dir / "pipelock.yaml"
|
yaml_path = stage_dir / "pipelock.yaml"
|
||||||
bottle_name = spec.manifest.agents[spec.agent_name].bottle
|
bottle = spec.manifest.bottle_for(spec.agent_name)
|
||||||
slug = docker_mod.slugify(spec.agent_name)
|
slug = docker_mod.slugify(spec.agent_name)
|
||||||
return self._proxy.prepare(spec.manifest, bottle_name, slug, yaml_path)
|
return self._proxy.prepare(bottle, slug, yaml_path)
|
||||||
|
|
||||||
@contextmanager
|
@contextmanager
|
||||||
def launch(self, plan: BottlePlan) -> Iterator[DockerBottle]:
|
def launch(self, plan: BottlePlan) -> Iterator[DockerBottle]:
|
||||||
|
|||||||
+15
-10
@@ -16,7 +16,7 @@ from abc import ABC, abstractmethod
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from .manifest import Bottle, Manifest
|
from .manifest import Bottle
|
||||||
from .util import is_ipv4_literal
|
from .util import is_ipv4_literal
|
||||||
|
|
||||||
# Baked-in default allowlist for hosts Claude Code itself needs.
|
# Baked-in default allowlist for hosts Claude Code itself needs.
|
||||||
@@ -110,8 +110,21 @@ class PipelockProxy(ABC):
|
|||||||
and lives on concrete subclasses (e.g. DockerPipelockProxy)."""
|
and lives on concrete subclasses (e.g. DockerPipelockProxy)."""
|
||||||
|
|
||||||
def prepare(
|
def prepare(
|
||||||
self, manifest: Manifest, bottle_name: str, slug: str, yaml_path: Path
|
self, bottle: Bottle, slug: str, yaml_path: Path
|
||||||
) -> PipelockProxyPlan:
|
) -> PipelockProxyPlan:
|
||||||
|
"""Write the pipelock yaml config (mode 600) to `yaml_path`
|
||||||
|
and return the plan for `.start`.
|
||||||
|
|
||||||
|
`slug` is the agent-derived identifier (lowercased,
|
||||||
|
hyphen-normalized) used as the suffix in every per-agent
|
||||||
|
resource name — the agent container, the pipelock container
|
||||||
|
(`claude-bottle-pipelock-<slug>`), the internal/egress
|
||||||
|
networks. It's stored on the returned plan so the backend's
|
||||||
|
start step can derive the sidecar's container name."""
|
||||||
|
self._build_pipelock_yaml(bottle, yaml_path)
|
||||||
|
return PipelockProxyPlan(yaml_path=yaml_path, slug=slug)
|
||||||
|
|
||||||
|
def _build_pipelock_yaml(self, bottle: Bottle, yaml_path: Path):
|
||||||
"""Write the pipelock yaml config (mode 600) to `yaml_path`
|
"""Write the pipelock yaml config (mode 600) to `yaml_path`
|
||||||
for the sidecar to consume when it boots. Carries the
|
for the sidecar to consume when it boots. Carries the
|
||||||
effective allowlist (bottle.egress.allowlist UNION
|
effective allowlist (bottle.egress.allowlist UNION
|
||||||
@@ -119,12 +132,6 @@ class PipelockProxy(ABC):
|
|||||||
port, strict mode + forward_proxy + DLP defaults + scan_env.
|
port, strict mode + forward_proxy + DLP defaults + scan_env.
|
||||||
Deliberately contains no env values, no secrets, no per-agent
|
Deliberately contains no env values, no secrets, no per-agent
|
||||||
customization beyond the hostname list."""
|
customization beyond the hostname list."""
|
||||||
return self._build_pipelock_yaml(manifest, bottle_name, slug, yaml_path)
|
|
||||||
|
|
||||||
def _build_pipelock_yaml(
|
|
||||||
self, manifest: Manifest, bottle_name: str, slug: str, yaml_path: Path
|
|
||||||
) -> PipelockProxyPlan:
|
|
||||||
bottle = manifest.bottles[bottle_name]
|
|
||||||
allowlist = pipelock_effective_allowlist(bottle)
|
allowlist = pipelock_effective_allowlist(bottle)
|
||||||
trusted = pipelock_bottle_ssh_trusted_domains(bottle)
|
trusted = pipelock_bottle_ssh_trusted_domains(bottle)
|
||||||
ip_cidrs = pipelock_bottle_ssh_ip_cidrs(bottle)
|
ip_cidrs = pipelock_bottle_ssh_ip_cidrs(bottle)
|
||||||
@@ -161,8 +168,6 @@ class PipelockProxy(ABC):
|
|||||||
yaml_path.write_text("\n".join(lines) + "\n")
|
yaml_path.write_text("\n".join(lines) + "\n")
|
||||||
yaml_path.chmod(0o600)
|
yaml_path.chmod(0o600)
|
||||||
|
|
||||||
return PipelockProxyPlan(yaml_path=yaml_path, slug=slug)
|
|
||||||
|
|
||||||
@abstractmethod
|
@abstractmethod
|
||||||
def start(self, plan: PipelockProxyPlan) -> str:
|
def start(self, plan: PipelockProxyPlan) -> str:
|
||||||
"""Bring up the pipelock sidecar according to `plan`. Returns
|
"""Bring up the pipelock sidecar according to `plan`. Returns
|
||||||
|
|||||||
@@ -41,7 +41,7 @@ class TestPipelockSidecarSmoke(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
def test_smoke(self):
|
def test_smoke(self):
|
||||||
yaml_path = self.work_dir / "pipelock.yaml"
|
yaml_path = self.work_dir / "pipelock.yaml"
|
||||||
DockerPipelockProxy().prepare(fixture_minimal(), "dev", "demo", yaml_path)
|
DockerPipelockProxy().prepare(fixture_minimal().bottles["dev"], "demo", yaml_path)
|
||||||
|
|
||||||
create = subprocess.run(
|
create = subprocess.run(
|
||||||
[
|
[
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ class TestPipelockProxyPrepare(unittest.TestCase):
|
|||||||
|
|
||||||
def test_minimal(self):
|
def test_minimal(self):
|
||||||
yaml_path = self.out_dir / "min.yaml"
|
yaml_path = self.out_dir / "min.yaml"
|
||||||
self.proxy.prepare(fixture_minimal(), "dev", "demo", yaml_path)
|
self.proxy.prepare(fixture_minimal().bottles["dev"], "demo", yaml_path)
|
||||||
content = yaml_path.read_text()
|
content = yaml_path.read_text()
|
||||||
self.assertIn("mode: strict", content)
|
self.assertIn("mode: strict", content)
|
||||||
self.assertIn("enforce: true", content)
|
self.assertIn("enforce: true", content)
|
||||||
@@ -41,7 +41,7 @@ class TestPipelockProxyPrepare(unittest.TestCase):
|
|||||||
|
|
||||||
def test_ssh_blocks(self):
|
def test_ssh_blocks(self):
|
||||||
yaml_path = self.out_dir / "ssh.yaml"
|
yaml_path = self.out_dir / "ssh.yaml"
|
||||||
self.proxy.prepare(fixture_with_ssh(), "dev", "demo", yaml_path)
|
self.proxy.prepare(fixture_with_ssh().bottles["dev"], "demo", yaml_path)
|
||||||
content = yaml_path.read_text()
|
content = yaml_path.read_text()
|
||||||
self.assertIn("trusted_domains:", content)
|
self.assertIn("trusted_domains:", content)
|
||||||
self.assertIn("github.com", content)
|
self.assertIn("github.com", content)
|
||||||
@@ -65,7 +65,7 @@ class TestPipelockProxyPrepare(unittest.TestCase):
|
|||||||
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||||
})
|
})
|
||||||
yaml_path = self.out_dir / "secret.yaml"
|
yaml_path = self.out_dir / "secret.yaml"
|
||||||
self.proxy.prepare(manifest, "dev", "demo", yaml_path)
|
self.proxy.prepare(manifest.bottles["dev"], "demo", yaml_path)
|
||||||
content = yaml_path.read_text()
|
content = yaml_path.read_text()
|
||||||
self.assertNotIn("literal-value-should-not-appear", content)
|
self.assertNotIn("literal-value-should-not-appear", content)
|
||||||
self.assertNotIn("MY_SECRET", content)
|
self.assertNotIn("MY_SECRET", content)
|
||||||
@@ -73,7 +73,7 @@ class TestPipelockProxyPrepare(unittest.TestCase):
|
|||||||
|
|
||||||
def test_file_mode_is_600(self):
|
def test_file_mode_is_600(self):
|
||||||
yaml_path = self.out_dir / "min.yaml"
|
yaml_path = self.out_dir / "min.yaml"
|
||||||
self.proxy.prepare(fixture_minimal(), "dev", "demo", yaml_path)
|
self.proxy.prepare(fixture_minimal().bottles["dev"], "demo", yaml_path)
|
||||||
mode = os.stat(yaml_path).st_mode & 0o777
|
mode = os.stat(yaml_path).st_mode & 0o777
|
||||||
self.assertEqual(0o600, mode)
|
self.assertEqual(0o600, mode)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user