Compare commits
4 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| cc0c952d0b | |||
| 8c9d4fbc46 | |||
| b9ab1263c2 | |||
| 9282bceaf8 |
@@ -43,7 +43,7 @@ from pathlib import Path
|
|||||||
from typing import Callable, Generator
|
from typing import Callable, Generator
|
||||||
|
|
||||||
from ...egress import egress_resolve_token_values
|
from ...egress import egress_resolve_token_values
|
||||||
from ...log import info
|
from ...log import info, warn
|
||||||
from . import network as network_mod
|
from . import network as network_mod
|
||||||
from . import util as docker_mod
|
from . import util as docker_mod
|
||||||
from .bottle import DockerBottle
|
from .bottle import DockerBottle
|
||||||
@@ -87,10 +87,11 @@ def launch(
|
|||||||
def teardown() -> None:
|
def teardown() -> None:
|
||||||
try:
|
try:
|
||||||
stack.close()
|
stack.close()
|
||||||
except BaseException:
|
except BaseException as exc:
|
||||||
# Teardown must not raise; swallow so the caller's
|
warn(
|
||||||
# __exit__ path can still propagate the original error.
|
f"teardown failed for container {plan.container_name}"
|
||||||
pass
|
f" (compose-down): {exc!r}"
|
||||||
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Step 1: agent image build. Sidecar images get built lazily by
|
# Step 1: agent image build. Sidecar images get built lazily by
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ backend-specific and lives on concrete subclasses (see
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import shlex
|
||||||
from abc import ABC, abstractmethod
|
from abc import ABC, abstractmethod
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -207,10 +208,7 @@ def git_gate_render_entrypoint(upstreams: tuple[GitGateUpstream, ...]) -> str:
|
|||||||
"mkdir -p /git",
|
"mkdir -p /git",
|
||||||
]
|
]
|
||||||
for u in upstreams:
|
for u in upstreams:
|
||||||
# Single-quote args so URL/path content (containing : and /)
|
lines.append(f"init_repo {shlex.quote(u.name)} {shlex.quote(u.upstream_url)}")
|
||||||
# passes through ash unmangled. Names came through the manifest
|
|
||||||
# validator so they don't contain a single quote.
|
|
||||||
lines.append(f"init_repo '{u.name}' '{u.upstream_url}'")
|
|
||||||
lines.extend([
|
lines.extend([
|
||||||
"",
|
"",
|
||||||
"exec git daemon \\",
|
"exec git daemon \\",
|
||||||
|
|||||||
@@ -2,10 +2,16 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
from .manifest_util import ManifestError, as_json_object
|
from .manifest_util import ManifestError, as_json_object
|
||||||
|
|
||||||
|
# Shell-safe characters for git-gate repo names. Names are embedded in
|
||||||
|
# the generated entrypoint shell script (shlex.quote is the primary
|
||||||
|
# defence; this regex is belt-and-suspenders and documents intent).
|
||||||
|
_GIT_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
|
||||||
|
|
||||||
|
|
||||||
def _opt_str(value: object, label: str) -> str:
|
def _opt_str(value: object, label: str) -> str:
|
||||||
if value is None:
|
if value is None:
|
||||||
@@ -94,6 +100,11 @@ class GitEntry:
|
|||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"bottle '{bottle_name}' git-gate.repos has an empty key"
|
f"bottle '{bottle_name}' git-gate.repos has an empty key"
|
||||||
)
|
)
|
||||||
|
if not _GIT_NAME_RE.match(repo_name):
|
||||||
|
raise ManifestError(
|
||||||
|
f"bottle '{bottle_name}' git-gate.repos name {repo_name!r} is invalid; "
|
||||||
|
f"allowed characters: A-Z a-z 0-9 . _ -"
|
||||||
|
)
|
||||||
label = f"git-gate.repos[{repo_name!r}]"
|
label = f"git-gate.repos[{repo_name!r}]"
|
||||||
d = as_json_object(raw, f"bottle '{bottle_name}' {label}")
|
d = as_json_object(raw, f"bottle '{bottle_name}' {label}")
|
||||||
for k in d:
|
for k in d:
|
||||||
|
|||||||
@@ -0,0 +1,145 @@
|
|||||||
|
"""Unit: Docker launch teardown warning on ExitStack failure (issue #156).
|
||||||
|
|
||||||
|
When a callback registered in the ExitStack raises during teardown,
|
||||||
|
the teardown function must emit a WARNING-level message that includes
|
||||||
|
the container name and operation type, rather than silently discarding
|
||||||
|
the exception.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import contextlib
|
||||||
|
import io
|
||||||
|
import tempfile
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
|
from bot_bottle.agent_provider import AgentProvisionPlan
|
||||||
|
from bot_bottle.backend import BottleSpec
|
||||||
|
from bot_bottle.backend.docker import launch as launch_mod
|
||||||
|
from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan
|
||||||
|
from bot_bottle.egress import EgressPlan
|
||||||
|
from bot_bottle.git_gate import GitGatePlan
|
||||||
|
from bot_bottle.manifest import Manifest
|
||||||
|
from bot_bottle.pipelock import PipelockProxyPlan
|
||||||
|
from bot_bottle.workspace import workspace_plan
|
||||||
|
|
||||||
|
|
||||||
|
def _manifest() -> Manifest:
|
||||||
|
return Manifest.from_json_obj({
|
||||||
|
"bottles": {"dev": {}},
|
||||||
|
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||||
|
})
|
||||||
|
|
||||||
|
|
||||||
|
def _plan(tmp: str) -> DockerBottlePlan:
|
||||||
|
stage = Path(tmp)
|
||||||
|
manifest = _manifest()
|
||||||
|
spec = BottleSpec(
|
||||||
|
manifest=manifest,
|
||||||
|
agent_name="demo",
|
||||||
|
copy_cwd=False,
|
||||||
|
user_cwd=tmp,
|
||||||
|
identity="test-teardown-00001",
|
||||||
|
)
|
||||||
|
return DockerBottlePlan(
|
||||||
|
spec=spec,
|
||||||
|
stage_dir=stage,
|
||||||
|
git_gate_plan=GitGatePlan(
|
||||||
|
slug="test-teardown-00001",
|
||||||
|
entrypoint_script=stage / "entrypoint.sh",
|
||||||
|
hook_script=stage / "hook.sh",
|
||||||
|
access_hook_script=stage / "access-hook.sh",
|
||||||
|
upstreams=(),
|
||||||
|
),
|
||||||
|
egress_plan=EgressPlan(
|
||||||
|
slug="test-teardown-00001",
|
||||||
|
routes_path=stage / "egress.yaml",
|
||||||
|
routes=(),
|
||||||
|
token_env_map={},
|
||||||
|
),
|
||||||
|
supervise_plan=None,
|
||||||
|
agent_provision=AgentProvisionPlan(
|
||||||
|
template="claude",
|
||||||
|
command="claude",
|
||||||
|
prompt_mode="append_file",
|
||||||
|
image="",
|
||||||
|
dockerfile="",
|
||||||
|
guest_env={},
|
||||||
|
),
|
||||||
|
workspace_plan=workspace_plan(spec, guest_home="/home/node"),
|
||||||
|
slug="test-teardown-00001",
|
||||||
|
container_name="bot-bottle-test-teardown-abc",
|
||||||
|
container_name_pinned=False,
|
||||||
|
image="bot-bottle-claude:latest",
|
||||||
|
derived_image="",
|
||||||
|
runtime_image="bot-bottle-claude:latest",
|
||||||
|
dockerfile_path="",
|
||||||
|
env_file=stage / "env",
|
||||||
|
forwarded_env={},
|
||||||
|
prompt_file=stage / "prompt.txt",
|
||||||
|
proxy_plan=PipelockProxyPlan(
|
||||||
|
yaml_path=stage / "pipelock.yaml",
|
||||||
|
slug="test-teardown-00001",
|
||||||
|
),
|
||||||
|
use_runsc=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestTeardownWarning(unittest.TestCase):
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self._tmp = tempfile.mkdtemp(prefix="docker-launch-teardown-test.")
|
||||||
|
|
||||||
|
def tearDown(self) -> None:
|
||||||
|
import shutil
|
||||||
|
shutil.rmtree(self._tmp, ignore_errors=True)
|
||||||
|
|
||||||
|
def test_teardown_failure_emits_warning_with_container_and_operation(self):
|
||||||
|
plan = _plan(self._tmp)
|
||||||
|
buf = io.StringIO()
|
||||||
|
|
||||||
|
with mock.patch.object(launch_mod.docker_mod, "build_image"), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod, "pipelock_tls_init",
|
||||||
|
return_value=(Path("/ca.crt"), Path("/ca.key")),
|
||||||
|
), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod, "egress_tls_init",
|
||||||
|
return_value=(Path("/egress_ca"), Path("/egress_cert")),
|
||||||
|
), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod.network_mod, "network_name_for_slug",
|
||||||
|
return_value="bb-internal-test",
|
||||||
|
), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod.network_mod, "network_egress_name_for_slug",
|
||||||
|
return_value="bb-egress-test",
|
||||||
|
), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod, "bottle_plan_to_compose",
|
||||||
|
return_value={"services": {"agent": {}}},
|
||||||
|
), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod, "write_compose_file",
|
||||||
|
return_value=Path("/tmp/compose.yml"),
|
||||||
|
), \
|
||||||
|
mock.patch.object(launch_mod, "compose_up"), \
|
||||||
|
mock.patch.object(launch_mod, "compose_dump_logs"), \
|
||||||
|
mock.patch.object(
|
||||||
|
launch_mod, "compose_down",
|
||||||
|
side_effect=RuntimeError("network remove failed"),
|
||||||
|
), \
|
||||||
|
contextlib.redirect_stderr(buf):
|
||||||
|
provision = mock.Mock(return_value=None)
|
||||||
|
with launch_mod.launch(plan, provision=provision):
|
||||||
|
pass
|
||||||
|
|
||||||
|
output = buf.getvalue()
|
||||||
|
self.assertIn("bot-bottle: warning:", output)
|
||||||
|
self.assertIn("bot-bottle-test-teardown-abc", output)
|
||||||
|
self.assertIn("compose-down", output)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -76,14 +76,18 @@ class TestEntrypointRender(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
script = git_gate_render_entrypoint(ups)
|
script = git_gate_render_entrypoint(ups)
|
||||||
self.assertIn("#!/bin/sh", script)
|
self.assertIn("#!/bin/sh", script)
|
||||||
self.assertIn(
|
# shlex.quote leaves safe strings unquoted; verify via token parse.
|
||||||
"init_repo 'bot-bottle' "
|
import shlex as _shlex
|
||||||
"'ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git'",
|
lines_with_init = [l for l in script.splitlines() if l.startswith("init_repo ")]
|
||||||
script,
|
self.assertEqual(2, len(lines_with_init))
|
||||||
|
self.assertEqual(
|
||||||
|
["init_repo", "bot-bottle",
|
||||||
|
"ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git"],
|
||||||
|
_shlex.split(lines_with_init[0]),
|
||||||
)
|
)
|
||||||
self.assertIn(
|
self.assertEqual(
|
||||||
"init_repo 'foo' 'ssh://git@github.com/didericis/foo.git'",
|
["init_repo", "foo", "ssh://git@github.com/didericis/foo.git"],
|
||||||
script,
|
_shlex.split(lines_with_init[1]),
|
||||||
)
|
)
|
||||||
# Daemon line is what keeps PID 1 alive.
|
# Daemon line is what keeps PID 1 alive.
|
||||||
self.assertIn("exec git daemon", script)
|
self.assertIn("exec git daemon", script)
|
||||||
@@ -108,6 +112,41 @@ class TestEntrypointRender(unittest.TestCase):
|
|||||||
self.assertNotIn("init_repo '", script)
|
self.assertNotIn("init_repo '", script)
|
||||||
self.assertIn("exec git daemon", script)
|
self.assertIn("exec git daemon", script)
|
||||||
|
|
||||||
|
def test_single_quote_in_upstream_url_is_escaped(self):
|
||||||
|
ups = (GitGateUpstream(
|
||||||
|
name="myrepo",
|
||||||
|
upstream_url="ssh://git@host/path'with'quotes.git",
|
||||||
|
upstream_host="host",
|
||||||
|
upstream_port="22",
|
||||||
|
identity_file="/key",
|
||||||
|
known_host_key="",
|
||||||
|
),)
|
||||||
|
script = git_gate_render_entrypoint(ups)
|
||||||
|
self.assertNotIn(
|
||||||
|
"init_repo 'myrepo' 'ssh://git@host/path'with'quotes.git'",
|
||||||
|
script,
|
||||||
|
)
|
||||||
|
self.assertIn("init_repo", script)
|
||||||
|
self.assertIn("path", script)
|
||||||
|
|
||||||
|
def test_space_and_semicolon_in_upstream_url_are_escaped(self):
|
||||||
|
import shlex as _shlex
|
||||||
|
raw_url = "ssh://git@host/path with spaces;evil.git"
|
||||||
|
ups = (GitGateUpstream(
|
||||||
|
name="myrepo",
|
||||||
|
upstream_url=raw_url,
|
||||||
|
upstream_host="host",
|
||||||
|
upstream_port="22",
|
||||||
|
identity_file="/key",
|
||||||
|
known_host_key="",
|
||||||
|
),)
|
||||||
|
script = git_gate_render_entrypoint(ups)
|
||||||
|
line = next(l for l in script.splitlines() if l.startswith("init_repo "))
|
||||||
|
tokens = _shlex.split(line)
|
||||||
|
self.assertEqual(3, len(tokens))
|
||||||
|
self.assertEqual("myrepo", tokens[1])
|
||||||
|
self.assertEqual(raw_url, tokens[2])
|
||||||
|
|
||||||
|
|
||||||
class TestHookRender(unittest.TestCase):
|
class TestHookRender(unittest.TestCase):
|
||||||
def test_pre_receive_hook_has_two_phases(self):
|
def test_pre_receive_hook_has_two_phases(self):
|
||||||
|
|||||||
@@ -185,6 +185,51 @@ class TestGitEntryCrossValidation(unittest.TestCase):
|
|||||||
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||||
})
|
})
|
||||||
|
|
||||||
|
def test_name_with_single_quote_dies(self):
|
||||||
|
with self.assertRaises(ManifestError):
|
||||||
|
Manifest.from_json_obj(_manifest({
|
||||||
|
"o'reilly": {
|
||||||
|
"url": "ssh://git@github.com/foo.git",
|
||||||
|
"identity": "/dev/null",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
def test_name_with_space_dies(self):
|
||||||
|
with self.assertRaises(ManifestError):
|
||||||
|
Manifest.from_json_obj(_manifest({
|
||||||
|
"my repo": {
|
||||||
|
"url": "ssh://git@github.com/foo.git",
|
||||||
|
"identity": "/dev/null",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
def test_name_with_semicolon_dies(self):
|
||||||
|
with self.assertRaises(ManifestError):
|
||||||
|
Manifest.from_json_obj(_manifest({
|
||||||
|
"foo;bar": {
|
||||||
|
"url": "ssh://git@github.com/foo.git",
|
||||||
|
"identity": "/dev/null",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
def test_name_with_dollar_dies(self):
|
||||||
|
with self.assertRaises(ManifestError):
|
||||||
|
Manifest.from_json_obj(_manifest({
|
||||||
|
"foo$bar": {
|
||||||
|
"url": "ssh://git@github.com/foo.git",
|
||||||
|
"identity": "/dev/null",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
def test_valid_name_with_dots_and_hyphens_accepted(self):
|
||||||
|
m = Manifest.from_json_obj(_manifest({
|
||||||
|
"my.repo-name_1": {
|
||||||
|
"url": "ssh://git@github.com/foo.git",
|
||||||
|
"identity": "/dev/null",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
self.assertEqual("my.repo-name_1", m.bottles["dev"].git[0].Name)
|
||||||
|
|
||||||
def test_legacy_git_key_dies_with_hint(self):
|
def test_legacy_git_key_dies_with_hint(self):
|
||||||
msg = ""
|
msg = ""
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user