Skip to content

Commit d9335a1

Browse files
committed
fix(server): fix sandbox port expose bug under windows and egress
1 parent aa37dc2 commit d9335a1

3 files changed

Lines changed: 85 additions & 7 deletions

File tree

server/opensandbox_server/services/docker.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@
6363
)
6464
from opensandbox_server.config import AppConfig, get_config
6565
from opensandbox_server.services.docker_diagnostics import DockerDiagnosticsMixin
66-
from opensandbox_server.services.docker_port_allocator import allocate_port_bindings
66+
from opensandbox_server.services.docker_port_allocator import (
67+
allocate_port_bindings,
68+
normalize_container_port_spec,
69+
normalize_port_bindings,
70+
)
6771
from opensandbox_server.services.docker_windows_profile import (
6872
apply_windows_runtime_host_config_defaults,
6973
fetch_execd_install_bat,
@@ -1235,7 +1239,7 @@ def _provision_sandbox(
12351239
port_bindings = allocate_port_bindings(exposed_ports)
12361240
host_execd_port = port_bindings["44772"][1]
12371241
host_http_port = port_bindings["8080"][1]
1238-
host_config_kwargs["port_bindings"] = port_bindings
1242+
host_config_kwargs["port_bindings"] = normalize_port_bindings(port_bindings)
12391243
labels[SANDBOX_EMBEDDING_PROXY_PORT_LABEL] = str(host_execd_port)
12401244
labels[SANDBOX_HTTP_PORT_LABEL] = str(host_http_port)
12411245
else:
@@ -2276,7 +2280,7 @@ def _start_egress_sidecar(
22762280
sidecar_host_config_kwargs: dict[str, Any] = {
22772281
"network_mode": BRIDGE_NETWORK_MODE,
22782282
"cap_add": ["NET_ADMIN"],
2279-
"port_bindings": sidecar_port_bindings,
2283+
"port_bindings": normalize_port_bindings(sidecar_port_bindings),
22802284
}
22812285
if self.app_config.egress.disable_ipv6:
22822286
# Optional: disable IPv6 in the shared namespace when egress.disable_ipv6 is set.
@@ -2301,7 +2305,7 @@ def _start_egress_sidecar(
23012305
labels=sidecar_labels,
23022306
environment=sidecar_env,
23032307
# Expose the ports that have host bindings so Docker publishes them in bridge mode.
2304-
ports=list(sidecar_port_bindings.keys()),
2308+
ports=[normalize_container_port_spec(p) for p in sidecar_port_bindings.keys()],
23052309
)
23062310
sidecar_container_id = sidecar_resp.get("Id")
23072311
if not sidecar_container_id:
@@ -2373,7 +2377,11 @@ def _create_and_start_container(
23732377
container_kwargs = {
23742378
"image": image_uri,
23752379
"command": bootstrap_command,
2376-
"ports": exposed_ports,
2380+
"ports": (
2381+
[normalize_container_port_spec(p) for p in exposed_ports]
2382+
if exposed_ports
2383+
else None
2384+
),
23772385
"name": f"sandbox-{sandbox_id}",
23782386
"environment": environment,
23792387
"labels": labels,

server/opensandbox_server/services/docker_port_allocator.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,28 @@
2323
from opensandbox_server.services.constants import SandboxErrorCodes
2424

2525

26+
def normalize_container_port_spec(port_spec: str) -> str:
27+
token = str(port_spec).strip()
28+
if token.endswith("/tcp"):
29+
return token[:-4]
30+
return token
31+
32+
33+
def normalize_port_bindings(
34+
port_bindings: dict[str, tuple[str, int]],
35+
) -> dict[str, tuple[str, int]]:
36+
"""
37+
Normalize binding keys to docker-py canonical forms.
38+
39+
Docker port bindings accept "port" for tcp and "port/udp" for udp.
40+
"""
41+
normalized: dict[str, tuple[str, int]] = {}
42+
for container_port, binding in port_bindings.items():
43+
normalized_key = normalize_container_port_spec(container_port)
44+
normalized[normalized_key] = binding
45+
return normalized
46+
47+
2648
def allocate_host_port(
2749
min_port: int = 40000,
2850
max_port: int = 60000,

server/tests/test_docker_service.py

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,54 @@ def host_cfg_side_effect(**kwargs):
928928
hc2 = mock_client.api.create_host_config.call_args.kwargs
929929
assert hc2["sysctls"]["net.ipv6.conf.all.disable_ipv6"] == 1
930930

931+
932+
@patch("opensandbox_server.services.docker.docker")
933+
def test_egress_sidecar_normalizes_windows_port_bindings(mock_docker):
934+
mock_client = MagicMock()
935+
mock_client.containers.list.return_value = []
936+
937+
def host_cfg_side_effect(**kwargs):
938+
return kwargs
939+
940+
sidecar_container = MagicMock()
941+
mock_client.api.create_host_config.side_effect = host_cfg_side_effect
942+
mock_client.api.create_container.return_value = {"Id": "sidecar-id"}
943+
mock_client.containers.get.return_value = sidecar_container
944+
mock_docker.from_env.return_value = mock_client
945+
946+
cfg = _app_config()
947+
cfg.docker.network_mode = "bridge"
948+
cfg.egress = EgressConfig(image="egress:latest", disable_ipv6=False)
949+
service = DockerSandboxService(config=cfg)
950+
951+
with (
952+
patch.object(service, "_ensure_image_available"),
953+
patch.object(service, "_docker_operation") as mock_op,
954+
):
955+
mock_op.return_value.__enter__.return_value = None
956+
mock_op.return_value.__exit__.return_value = None
957+
service._start_egress_sidecar(
958+
"sandbox-id",
959+
NetworkPolicy(defaultAction="deny", egress=[]),
960+
egress_token="egress-token",
961+
host_execd_port=44772,
962+
host_http_port=8080,
963+
extra_port_bindings={
964+
"3389/tcp": ("0.0.0.0", 53389),
965+
"3389/udp": ("0.0.0.0", 53390),
966+
"8006/tcp": ("0.0.0.0", 58006),
967+
},
968+
)
969+
970+
hc_kwargs = mock_client.api.create_host_config.call_args.kwargs
971+
assert "3389" in hc_kwargs["port_bindings"]
972+
assert "3389/udp" in hc_kwargs["port_bindings"]
973+
assert "8006" in hc_kwargs["port_bindings"]
974+
sidecar_kwargs = mock_client.api.create_container.call_args.kwargs
975+
assert "3389" in sidecar_kwargs["ports"]
976+
assert "3389/udp" in sidecar_kwargs["ports"]
977+
assert "8006" in sidecar_kwargs["ports"]
978+
931979
def test_expire_cleans_sidecar():
932980
service = DockerSandboxService(config=_app_config())
933981
mock_container = MagicMock()
@@ -1293,9 +1341,9 @@ async def test_create_sandbox_windows_profile_injects_runtime_defaults(mock_dock
12931341
port_bindings = host_config_kwargs["port_bindings"]
12941342
assert "44772" in port_bindings
12951343
assert "8080" in port_bindings
1296-
assert "3389/tcp" in port_bindings
1344+
assert "3389" in port_bindings
12971345
assert "3389/udp" in port_bindings
1298-
assert "8006/tcp" in port_bindings
1346+
assert "8006" in port_bindings
12991347

13001348

13011349
@pytest.mark.asyncio

0 commit comments

Comments
 (0)