refactor(sidecars): drop vestigial start/stop methods (PRD 0024 chunk 3)
Compose-up has owned per-container lifecycle since PRD 0018 ch3;
the .start() / .stop() methods on DockerPipelockProxy /
DockerEgress / DockerGitGate / DockerSupervise (and their
abstractmethod declarations in the four base ABCs) were already
documented as vestigial. With the bundle path in flight
(PRD 0024 ch2), they are truly dead — collapse to nothing.
Changes:
- Removed start/stop methods from the four DockerSidecar
classes. Plan dataclasses, image/path constants,
container-name helpers, and the .prepare() methods all stay
(the renderer + apply path still need them).
- Removed the matching @abstractmethod declarations in the
base ABCs so concrete subclasses don't have to stub them.
- launch.launch() and prepare.resolve_plan() no longer take
proxy/git_gate/egress/supervise instance parameters. backend.py
loses the four instance attributes it threaded through.
prepare.resolve_plan() instantiates the four classes itself
to call their .prepare() methods.
- Deleted four integration tests that only exercised the
removed lifecycle: test_pipelock_sidecar_smoke,
test_supervise_sidecar, test_git_gate_sidecar,
test_git_gate_mirror.
- Dropped the .stop-idempotency case in test_orphan_cleanup;
the network-cleanup cases stay (those test real production
code).
- Marked test_pipelock_apply @skip pending chunk 4 — its
bringup helper used .start; chunk 4 rewrites it with direct
`docker run`.
Dockerfile deletion deferred to chunk 5 (when the bundle flag
default flips) — the legacy compose path still needs
Dockerfile.{egress,git-gate,supervise} until then.
Net: 708 lines removed, 80 added.
533 unit tests + 27 integration tests passing (5 skipped: the
chunk-4-pending case + existing GITEA_ACTIONS guards).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -13,14 +13,8 @@ import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
from ...egress import (
|
||||
EGRESS_HOSTNAME,
|
||||
EGRESS_ROUTES_IN_CONTAINER,
|
||||
Egress,
|
||||
EgressPlan,
|
||||
egress_resolve_token_values,
|
||||
)
|
||||
from ...log import die, info, warn
|
||||
from ...egress import Egress
|
||||
from ...log import die
|
||||
from . import util as docker_mod
|
||||
|
||||
|
||||
@@ -166,214 +160,6 @@ def egress_tls_init(stage_dir: Path) -> tuple[Path, Path]:
|
||||
|
||||
|
||||
class DockerEgress(Egress):
|
||||
"""Brings the egress sidecar up and down via Docker."""
|
||||
|
||||
def start(self, plan: EgressPlan) -> str:
|
||||
"""Boot the egress sidecar:
|
||||
1. Resolve every host TokenRef env var into a concrete
|
||||
value. Fails early if any are unset.
|
||||
2. Build the egress image (no-op when cache is hot).
|
||||
3. `docker create` on the internal network with
|
||||
`--network-alias egress`, the `HTTPS_PROXY=pipelock`
|
||||
env (so the upstream leg traverses pipelock), the
|
||||
`EGRESS_UPSTREAM_CA` env pointing at the in-container
|
||||
pipelock-CA path (so mitmproxy trusts pipelock's MITM),
|
||||
and one `-e EGRESS_TOKEN_N` flag per token slot.
|
||||
Secret values arrive via subprocess env, never argv.
|
||||
4. `docker cp` the routes.yaml, mitmproxy CA (cert+key
|
||||
concat), and pipelock CA (cert only) into the container.
|
||||
5. Attach to the per-agent egress network so the proxy can
|
||||
reach pipelock.
|
||||
6. `docker start`.
|
||||
Returns the container name (the target passed to `.stop`)."""
|
||||
if not plan.routes:
|
||||
die("DockerEgress.start called with no routes; caller should skip")
|
||||
if not plan.internal_network or not plan.egress_network:
|
||||
die(
|
||||
"DockerEgress.start: internal_network / egress_network must be "
|
||||
"populated on the plan before start"
|
||||
)
|
||||
if not plan.routes_path.is_file():
|
||||
die(
|
||||
f"egress routes file missing at {plan.routes_path}; "
|
||||
f"Egress.prepare must run first"
|
||||
)
|
||||
if plan.mitmproxy_ca_host_path == Path() or not plan.mitmproxy_ca_host_path.is_file():
|
||||
die(
|
||||
f"DockerEgress.start: mitmproxy CA missing at "
|
||||
f"{plan.mitmproxy_ca_host_path}; egress_tls_init must run first"
|
||||
)
|
||||
# pipelock CA + upstream proxy URL: both must be present (we
|
||||
# use HTTPS_PROXY=pipelock with pipelock's own MITM CA on the
|
||||
# upstream leg) or both absent (egress goes direct, for
|
||||
# standalone integration tests that don't bring pipelock up).
|
||||
route_via_pipelock = bool(plan.pipelock_proxy_url) or plan.pipelock_ca_host_path != Path()
|
||||
if route_via_pipelock:
|
||||
if not plan.pipelock_proxy_url:
|
||||
die(
|
||||
"DockerEgress.start: pipelock_ca_host_path is set but "
|
||||
"pipelock_proxy_url is empty; populate both or neither."
|
||||
)
|
||||
if not plan.pipelock_ca_host_path.is_file():
|
||||
die(
|
||||
f"DockerEgress.start: pipelock CA missing at "
|
||||
f"{plan.pipelock_ca_host_path}; pipelock_tls_init must run first"
|
||||
)
|
||||
|
||||
# Resolve host env vars into concrete values. Must happen at
|
||||
# start time (not prepare) — the values flow into the sidecar's
|
||||
# environ via subprocess env. The plan never holds them.
|
||||
token_values = egress_resolve_token_values(
|
||||
plan.token_env_map, dict(os.environ),
|
||||
)
|
||||
|
||||
build_egress_image()
|
||||
|
||||
name = egress_container_name(plan.slug)
|
||||
info(f"starting egress sidecar {name} on network {plan.internal_network}")
|
||||
|
||||
create_args = [
|
||||
"docker", "create",
|
||||
"--name", name,
|
||||
"--network", plan.internal_network,
|
||||
"--network-alias", EGRESS_HOSTNAME,
|
||||
]
|
||||
if route_via_pipelock:
|
||||
# Route egress's outbound traffic through pipelock
|
||||
# so the egress allowlist + DLP body scanner apply to
|
||||
# the egress → upstream leg. Pipelock MITMs each
|
||||
# handshake with its per-bottle CA, which is docker-cp'd
|
||||
# in below and pointed to via the EGRESS_UPSTREAM_CA
|
||||
# env (entrypoint conditionally adds the matching --set
|
||||
# flag).
|
||||
#
|
||||
# EGRESS_UPSTREAM_PROXY is the mechanism: mitmproxy
|
||||
# does NOT honor HTTPS_PROXY env vars on its outbound
|
||||
# side (it's a proxy server, not a client). The
|
||||
# entrypoint reads this env and switches mitmdump to
|
||||
# `--mode upstream:<URL>` so all post-MITM traffic
|
||||
# CONNECTs to pipelock instead of going direct. The
|
||||
# HTTPS/HTTP_PROXY env vars below are kept for any
|
||||
# bundled client libraries (mitmproxy plugin requests,
|
||||
# etc.) that might honor them — harmless if ignored.
|
||||
create_args.extend([
|
||||
"-e", f"EGRESS_UPSTREAM_PROXY={plan.pipelock_proxy_url}",
|
||||
"-e", f"HTTPS_PROXY={plan.pipelock_proxy_url}",
|
||||
"-e", f"HTTP_PROXY={plan.pipelock_proxy_url}",
|
||||
"-e", "NO_PROXY=localhost,127.0.0.1",
|
||||
"-e", f"EGRESS_UPSTREAM_CA={EGRESS_PIPELOCK_CA_IN_CONTAINER}",
|
||||
])
|
||||
# One -e flag per token slot; values arrive via subprocess env.
|
||||
# docker create with `-e NAME` (no =VALUE) reads NAME from the
|
||||
# current process env at create time. We pass `env=child_env`
|
||||
# to subprocess.run so the value comes from token_values, not
|
||||
# the host's os.environ directly — keeps the resolver in one
|
||||
# place and lets egress_resolve_token_values surface
|
||||
# missing-env errors with a clear hint.
|
||||
for token_env in sorted(plan.token_env_map.keys()):
|
||||
create_args.extend(["-e", token_env])
|
||||
create_args.append(EGRESS_IMAGE)
|
||||
|
||||
child_env: dict[str, str] = {**os.environ, **token_values}
|
||||
|
||||
create_result = subprocess.run(
|
||||
create_args, capture_output=True, text=True, env=child_env, check=False,
|
||||
)
|
||||
if create_result.returncode != 0:
|
||||
die(
|
||||
f"failed to create egress sidecar {name}: "
|
||||
f"{create_result.stderr.strip()}"
|
||||
)
|
||||
|
||||
# routes.yaml also lands inside the container; bump to 644
|
||||
# for the same reason as the CAs — mitmproxy user (uid 1000)
|
||||
# has to read it. Host stage_dir is mode 700 so the file
|
||||
# isn't actually exposed to other host users.
|
||||
plan.routes_path.chmod(0o644)
|
||||
# Pipelock CA: pipelock itself runs as root so its in-pipelock
|
||||
# copy doesn't care about mode, but egress's mitmproxy
|
||||
# user does. Bump on the host so docker cp into egress
|
||||
# carries world-readable.
|
||||
if route_via_pipelock:
|
||||
plan.pipelock_ca_host_path.chmod(0o644)
|
||||
cps: list[tuple[Path, str, str]] = [
|
||||
(plan.routes_path, EGRESS_ROUTES_IN_CONTAINER, "routes.yaml"),
|
||||
(plan.mitmproxy_ca_host_path, EGRESS_CA_IN_CONTAINER, "mitmproxy CA"),
|
||||
]
|
||||
if route_via_pipelock:
|
||||
cps.append((
|
||||
plan.pipelock_ca_host_path,
|
||||
EGRESS_PIPELOCK_CA_IN_CONTAINER,
|
||||
"pipelock CA",
|
||||
))
|
||||
for src, dst, label in cps:
|
||||
cp_result = subprocess.run(
|
||||
["docker", "cp", str(src), f"{name}:{dst}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if cp_result.returncode != 0:
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", name],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
)
|
||||
die(
|
||||
f"failed to copy {label} into {name}: "
|
||||
f"{cp_result.stderr.strip()}"
|
||||
)
|
||||
|
||||
connect_result = subprocess.run(
|
||||
["docker", "network", "connect", plan.egress_network, name],
|
||||
capture_output=True, text=True, check=False,
|
||||
)
|
||||
if connect_result.returncode != 0:
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", name],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
)
|
||||
die(
|
||||
f"failed to attach egress sidecar {name} to egress network "
|
||||
f"{plan.egress_network}: {connect_result.stderr.strip()}"
|
||||
)
|
||||
|
||||
start_result = subprocess.run(
|
||||
["docker", "start", name], capture_output=True, text=True, check=False,
|
||||
)
|
||||
if start_result.returncode != 0:
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", name],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
)
|
||||
die(
|
||||
f"failed to start egress sidecar {name}: "
|
||||
f"{start_result.stderr.strip()}"
|
||||
)
|
||||
|
||||
return name
|
||||
|
||||
def stop(self, target: str) -> None:
|
||||
"""Idempotent: missing container is success. `target` is the
|
||||
container name returned by `.start`."""
|
||||
if subprocess.run(
|
||||
["docker", "inspect", target],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
).returncode == 0:
|
||||
if subprocess.run(
|
||||
["docker", "rm", "-f", target],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
).returncode != 0:
|
||||
warn(
|
||||
f"failed to remove egress sidecar {target}; "
|
||||
f"clean up with 'docker rm -f {target}'"
|
||||
)
|
||||
"""Docker-flavored Egress: inherits `.prepare()` from the base.
|
||||
Container lifecycle is owned by compose; per-container
|
||||
`.start()` / `.stop()` were removed in PRD 0024 chunk 3."""
|
||||
|
||||
Reference in New Issue
Block a user