From d32f36bb2bfe4366bc4e93f091f888319cc76f04 Mon Sep 17 00:00:00 2001 From: didericis Date: Fri, 17 Jul 2026 16:43:07 -0400 Subject: [PATCH] fix(test): actually isolate the docker control-plane auth test's root + network MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs surfaced by a review of the previous commit: - host_control_plane_token() resolves its path via the ambient BOT_BOTTLE_ROOT env var, not the host_root kwarg passed to OrchestratorService (that kwarg only controls the DB bind-mount destination). The test's isolation claim was false as a result: running it read/wrote the developer's real ~/.bot-bottle/control-plane-token instead of the throwaway temp dir — confirmed directly on disk. Fixed by pointing the env var at the same temp dir for the test's duration and restoring it via addCleanup. - ensure_running() creates a per-bottle Docker network but stop() only ever removes containers, never the network — every run of this test leaked one bridge network permanently (found and removed 5 from prior runs via `docker network ls`). Fixed with an explicit `docker network rm` in addCleanup. Verified: re-ran the suite twice: 5/5 pass both times, the real ~/.bot-bottle/control-plane-token timestamp is unchanged across both runs (proving isolation), and `docker network ls` shows zero leaked bot-bottle-net-itest-* networks afterward. pyright clean, pylint 10.00/10. Remaining findings from the same review (missing GITEA_ACTIONS skip guard, root-owned bind-mount cleanup on native Linux, no setUpClass, reinvented OrchestratorClient, gateway_name should be a constructor param rather than a subclassed private-method override) are left for a follow-up — each is a real, separate design/scope call, not a mechanical fix like these two. Co-Authored-By: Claude Sonnet 5 --- ..._orchestrator_docker_control_plane_auth.py | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_orchestrator_docker_control_plane_auth.py b/tests/integration/test_orchestrator_docker_control_plane_auth.py index 45b6a20..740d278 100644 --- a/tests/integration/test_orchestrator_docker_control_plane_auth.py +++ b/tests/integration/test_orchestrator_docker_control_plane_auth.py @@ -14,7 +14,9 @@ orchestrator or gateway. from __future__ import annotations import json +import os import secrets +import subprocess import tempfile import unittest import urllib.error @@ -50,13 +52,42 @@ class _IsolatedOrchestratorService(OrchestratorService): class TestDockerControlPlaneAuthIntegration(unittest.TestCase): def setUp(self) -> None: suffix = secrets.token_hex(4) - self._tmp = tempfile.TemporaryDirectory() + self._tmp = tempfile.TemporaryDirectory() # pylint: disable=consider-using-with self.addCleanup(self._tmp.cleanup) + # host_control_plane_token() — both the test's own call below and the + # one OrchestratorService makes internally to inject the container's + # env var — resolves its path via the *ambient* BOT_BOTTLE_ROOT env + # var, not the host_root kwarg passed to the constructor (that kwarg + # only controls the DB bind-mount destination). Without pointing the + # env var at the same throwaway dir, the "isolated" test reads/writes + # the developer's real ~/.bot-bottle/control-plane-token. + previous_root = os.environ.get("BOT_BOTTLE_ROOT") + + def _restore_root() -> None: + if previous_root is None: + os.environ.pop("BOT_BOTTLE_ROOT", None) + else: + os.environ["BOT_BOTTLE_ROOT"] = previous_root + + os.environ["BOT_BOTTLE_ROOT"] = self._tmp.name + self.addCleanup(_restore_root) + + # ensure_running()/DockerGateway create this network but never remove + # it — stop() only removes containers — so every run would otherwise + # leak one bridge network permanently. + network = f"bot-bottle-net-itest-{suffix}" + self.addCleanup( + lambda: subprocess.run( + ["docker", "network", "rm", network], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, + ) + ) + self.svc = _IsolatedOrchestratorService( orchestrator_name=f"bot-bottle-orch-itest-{suffix}", gateway_name=f"bot-bottle-gw-itest-{suffix}", - network=f"bot-bottle-net-itest-{suffix}", + network=network, port=20000 + secrets.randbelow(10000), host_root=Path(self._tmp.name), )