refactor(gateway): fail closed without an orchestrator URL; drop stale single-tenant comments
Follow-ups from the #402 review of the single-tenant data-plane teardown. - egress_entrypoint.sh: refuse to launch mitmdump when BOT_BOTTLE_ORCHESTRATOR_URL is unset, so the fail-closed guarantee no longer rests solely on mitmproxy's errorcheck addon exiting on the addon's load-time raise. A misconfigured gateway can never come up as a bare TLS-bumping open proxy with no policy. - orchestrator/gateway.py: ensure_running() raises GatewayError on an empty orchestrator URL — a URL-less launch would only crash-loop the now-resolver-only daemons (egress raises, git-http exits 1, supervise exits 2). The env-injection branch is now unconditional. - Drop stale "single-tenant" / "reads routes.yaml" comments in gateway.py and egress_entrypoint.sh, and the /etc/egress/routes.yaml layout line in Dockerfile.gateway. - Tests: gateway fixtures supply an orchestrator URL; add a refuse-without-URL test and assert the URL env is injected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit is contained in:
@@ -20,7 +20,6 @@
|
|||||||
# /app/egress-entrypoint.sh mitmdump launcher
|
# /app/egress-entrypoint.sh mitmdump launcher
|
||||||
# /app/supervise_server.py + .py supervise MCP server
|
# /app/supervise_server.py + .py supervise MCP server
|
||||||
# /app/gateway_init.py PID 1 supervisor
|
# /app/gateway_init.py PID 1 supervisor
|
||||||
# /etc/egress/routes.yaml bind-mounted at run time
|
|
||||||
# /etc/git-gate/pre-receive docker-cp'd at start time
|
# /etc/git-gate/pre-receive docker-cp'd at start time
|
||||||
# /git-gate-entrypoint.sh docker-cp'd at start time
|
# /git-gate-entrypoint.sh docker-cp'd at start time
|
||||||
# /git-gate/creds/* docker-cp'd at start time
|
# /git-gate/creds/* docker-cp'd at start time
|
||||||
|
|||||||
@@ -15,11 +15,23 @@
|
|||||||
# mitmproxy at it. The option REPLACES mitmproxy's default
|
# mitmproxy at it. The option REPLACES mitmproxy's default
|
||||||
# trust store, so passing the upstream CA alone would break
|
# trust store, so passing the upstream CA alone would break
|
||||||
# non-chained hosts.
|
# non-chained hosts.
|
||||||
# * `-s /app/egress_addon.py` loads the addon that reads
|
# * `-s /app/egress_addon.py` loads the addon that resolves each
|
||||||
# /etc/egress/routes.yaml.
|
# request's policy from the orchestrator control plane by source
|
||||||
|
# IP (PRD 0070). There is no static routes file.
|
||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
|
# Fail closed on a missing policy source. The addon itself raises at
|
||||||
|
# load when BOT_BOTTLE_ORCHESTRATOR_URL is unset (so mitmdump exits via
|
||||||
|
# its errorcheck addon), but that leaves the fail-closed guarantee at the
|
||||||
|
# mercy of a mitmproxy version keeping that behavior. Refuse here too, so
|
||||||
|
# a misconfigured gateway can never come up as a bare TLS-bumping open
|
||||||
|
# proxy with no policy — independent of mitmproxy's startup-error handling.
|
||||||
|
if [ -z "$BOT_BOTTLE_ORCHESTRATOR_URL" ]; then
|
||||||
|
echo "egress: BOT_BOTTLE_ORCHESTRATOR_URL is required (no static routes fallback)" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
# Pin mitmproxy's config dir to the bind-mount location of its CA
|
# Pin mitmproxy's config dir to the bind-mount location of its CA
|
||||||
# regardless of which user mitmdump runs as. In the legacy
|
# regardless of which user mitmdump runs as. In the legacy
|
||||||
# four-daemon setup (Dockerfile.egress, USER mitmproxy) this
|
# four-daemon setup (Dockerfile.egress, USER mitmproxy) this
|
||||||
|
|||||||
@@ -126,7 +126,10 @@ class DockerGateway(Gateway):
|
|||||||
self.network = network
|
self.network = network
|
||||||
# The control-plane URL the gateway's data plane resolves per bottle
|
# The control-plane URL the gateway's data plane resolves per bottle
|
||||||
# against — reached by container name over docker DNS on the shared
|
# against — reached by container name over docker DNS on the shared
|
||||||
# network (container↔container, no host firewall). Empty → single-tenant.
|
# network (container↔container, no host firewall). Mandatory to *run*
|
||||||
|
# the gateway (see `ensure_running`); empty is tolerated only for the
|
||||||
|
# construct-then-read-CA path (`ca_cert_pem` on an already-running
|
||||||
|
# container), which never launches a container.
|
||||||
self._orchestrator_url = orchestrator_url
|
self._orchestrator_url = orchestrator_url
|
||||||
self._build_context = build_context or _REPO_ROOT
|
self._build_context = build_context or _REPO_ROOT
|
||||||
self._dockerfile = dockerfile
|
self._dockerfile = dockerfile
|
||||||
@@ -193,6 +196,16 @@ class DockerGateway(Gateway):
|
|||||||
)
|
)
|
||||||
|
|
||||||
def ensure_running(self) -> None:
|
def ensure_running(self) -> None:
|
||||||
|
# Fail closed on a missing policy source. The data-plane daemons are
|
||||||
|
# resolver-only now (PRD 0070) — without an orchestrator URL egress
|
||||||
|
# raises, git-http exits 1, and supervise exits 2 — so launching a
|
||||||
|
# gateway without one would only crash-loop its daemons. Refuse here so
|
||||||
|
# the misconfiguration surfaces as a clear error, not a broken container.
|
||||||
|
if not self._orchestrator_url:
|
||||||
|
raise GatewayError(
|
||||||
|
"gateway requires an orchestrator URL to run "
|
||||||
|
"(resolver-only data plane; no single-tenant fallback)"
|
||||||
|
)
|
||||||
# Recreate when the running container's image is stale (a rebuild),
|
# Recreate when the running container's image is stale (a rebuild),
|
||||||
# so source changes to the gateway's flat daemons take effect — not
|
# so source changes to the gateway's flat daemons take effect — not
|
||||||
# just when the container is absent.
|
# just when the container is absent.
|
||||||
@@ -220,16 +233,15 @@ class DockerGateway(Gateway):
|
|||||||
for port in self._host_port_bindings:
|
for port in self._host_port_bindings:
|
||||||
argv += ["--publish", f"0.0.0.0:{port}:{port}"]
|
argv += ["--publish", f"0.0.0.0:{port}:{port}"]
|
||||||
run_env = dict(os.environ)
|
run_env = dict(os.environ)
|
||||||
if self._orchestrator_url:
|
# The gateway's egress / git / supervise daemons resolve source-IP ->
|
||||||
# Makes the gateway's egress / git / supervise daemons multi-tenant:
|
# policy against the control plane per request (guaranteed non-empty by
|
||||||
# each request resolves source-IP -> policy against the control plane.
|
# the check above).
|
||||||
argv += ["--env", f"BOT_BOTTLE_ORCHESTRATOR_URL={self._orchestrator_url}"]
|
argv += ["--env", f"BOT_BOTTLE_ORCHESTRATOR_URL={self._orchestrator_url}"]
|
||||||
# ...and presents the control-plane secret on those /resolve calls
|
# ...and present the control-plane secret on those /resolve calls (the
|
||||||
# (the control plane requires it). Bare `--env NAME` keeps the value
|
# control plane requires it). Bare `--env NAME` keeps the value off argv
|
||||||
# off argv / `docker inspect`; only the gateway (not the agent) is
|
# / `docker inspect`; only the gateway (not the agent) is given it.
|
||||||
# given it. Only needed in multi-tenant mode, where /resolve is used.
|
argv += ["--env", CONTROL_PLANE_TOKEN_ENV]
|
||||||
argv += ["--env", CONTROL_PLANE_TOKEN_ENV]
|
run_env[CONTROL_PLANE_TOKEN_ENV] = host_control_plane_token()
|
||||||
run_env[CONTROL_PLANE_TOKEN_ENV] = host_control_plane_token()
|
|
||||||
argv.append(self.image_ref)
|
argv.append(self.image_ref)
|
||||||
proc = run_docker(argv, env=run_env)
|
proc = run_docker(argv, env=run_env)
|
||||||
if proc.returncode != 0:
|
if proc.returncode != 0:
|
||||||
|
|||||||
@@ -20,7 +20,11 @@ IMAGE = "busybox"
|
|||||||
class TestDockerGatewayIntegration(unittest.TestCase):
|
class TestDockerGatewayIntegration(unittest.TestCase):
|
||||||
def setUp(self) -> None:
|
def setUp(self) -> None:
|
||||||
self.name = "bot-bottle-orch-gateway-itest-" + secrets.token_hex(4)
|
self.name = "bot-bottle-orch-gateway-itest-" + secrets.token_hex(4)
|
||||||
self.sc = DockerGateway(IMAGE, name=self.name)
|
# Resolver-only data plane (PRD 0070) requires an orchestrator URL to
|
||||||
|
# run; busybox never dials it, so a placeholder is enough here.
|
||||||
|
self.sc = DockerGateway(
|
||||||
|
IMAGE, name=self.name, orchestrator_url="http://orchestrator:9000",
|
||||||
|
)
|
||||||
self.addCleanup(self.sc.stop)
|
self.addCleanup(self.sc.stop)
|
||||||
|
|
||||||
def _count(self) -> int:
|
def _count(self) -> int:
|
||||||
|
|||||||
@@ -22,13 +22,27 @@ def _proc(returncode: int = 0, stdout: str = "", stderr: str = "") -> Mock:
|
|||||||
return Mock(returncode=returncode, stdout=stdout, stderr=stderr)
|
return Mock(returncode=returncode, stdout=stdout, stderr=stderr)
|
||||||
|
|
||||||
|
|
||||||
|
_ORCH_URL = "http://orchestrator:9000"
|
||||||
|
|
||||||
|
|
||||||
class TestDockerGateway(unittest.TestCase):
|
class TestDockerGateway(unittest.TestCase):
|
||||||
def setUp(self) -> None:
|
def setUp(self) -> None:
|
||||||
self.sc = DockerGateway("bot-bottle-gateway:latest")
|
# Resolver-only data plane (PRD 0070): running the gateway requires an
|
||||||
|
# orchestrator URL, so the fixture supplies one.
|
||||||
|
self.sc = DockerGateway("bot-bottle-gateway:latest", orchestrator_url=_ORCH_URL)
|
||||||
|
|
||||||
def test_default_name(self) -> None:
|
def test_default_name(self) -> None:
|
||||||
self.assertEqual(GATEWAY_NAME, self.sc.name)
|
self.assertEqual(GATEWAY_NAME, self.sc.name)
|
||||||
|
|
||||||
|
def test_ensure_running_refuses_without_orchestrator_url(self) -> None:
|
||||||
|
# No policy source → the data-plane daemons would only crash-loop, so
|
||||||
|
# the launch must fail closed with a clear error rather than start one.
|
||||||
|
sc = DockerGateway("bot-bottle-gateway:latest")
|
||||||
|
with patch(_RUN_DOCKER) as m:
|
||||||
|
with self.assertRaises(GatewayError):
|
||||||
|
sc.ensure_running()
|
||||||
|
m.assert_not_called()
|
||||||
|
|
||||||
def test_is_running_reads_docker_ps(self) -> None:
|
def test_is_running_reads_docker_ps(self) -> None:
|
||||||
with patch(_RUN_DOCKER, return_value=_proc(stdout=self.sc.name + "\n")):
|
with patch(_RUN_DOCKER, return_value=_proc(stdout=self.sc.name + "\n")):
|
||||||
self.assertTrue(self.sc.is_running())
|
self.assertTrue(self.sc.is_running())
|
||||||
@@ -98,6 +112,8 @@ class TestDockerGateway(unittest.TestCase):
|
|||||||
for a in runs[0]))
|
for a in runs[0]))
|
||||||
self.assertTrue(any(
|
self.assertTrue(any(
|
||||||
a.endswith(":/run/supervise") for a in runs[0]))
|
a.endswith(":/run/supervise") for a in runs[0]))
|
||||||
|
# Data plane resolves policy against the orchestrator control plane.
|
||||||
|
self.assertIn(f"BOT_BOTTLE_ORCHESTRATOR_URL={_ORCH_URL}", runs[0])
|
||||||
|
|
||||||
def test_ensure_running_creates_network_when_missing(self) -> None:
|
def test_ensure_running_creates_network_when_missing(self) -> None:
|
||||||
calls: list[list[str]] = []
|
calls: list[list[str]] = []
|
||||||
|
|||||||
Reference in New Issue
Block a user