Merge pull request opensandbox-group#733 from Pangjiping/fix/server/windows-networkpolicy

fix(server): fix sandbox port expose bug under windows and egress

Author: ninan-nn

server/opensandbox_server/services/docker.py

@@ -63,7 +63,11 @@
 )
 from opensandbox_server.config import AppConfig, get_config
 from opensandbox_server.services.docker_diagnostics import DockerDiagnosticsMixin
-from opensandbox_server.services.docker_port_allocator import allocate_port_bindings
+from opensandbox_server.services.docker_port_allocator import (
+    allocate_port_bindings,
+    normalize_container_port_spec,
+    normalize_port_bindings,
+)
 from opensandbox_server.services.docker_windows_profile import (
     apply_windows_runtime_host_config_defaults,
     fetch_execd_install_bat,
@@ -1197,26 +1201,40 @@ def _provision_sandbox(
             volume_binds = self._build_volume_binds(request.volumes, pvc_inspect_cache)
 
             host_config_kwargs: Dict[str, Any]
-            exposed_ports: Optional[list[str]] = None
+            exposed_ports: Optional[list[str]] = ["44772", "8080"]
+            if requested_windows_profile:
+                # dockur/windows exposes RDP and noVNC/web UI on these ports.
+                # https://github.com/dockur/windows/blob/master/Dockerfile
+                exposed_ports.extend(["3389/tcp", "3389/udp", "8006/tcp"])
+            container_exposed_ports: Optional[list[str]] = exposed_ports
 
             if request.network_policy:
                 egress_token = generate_egress_token()
                 labels[SANDBOX_EGRESS_AUTH_TOKEN_METADATA_KEY] = egress_token
-                sidecar_port_bindings = allocate_port_bindings(["44772", "8080"])
+                sidecar_port_bindings = allocate_port_bindings(exposed_ports)
                 host_execd_port = sidecar_port_bindings["44772"][1]
                 host_http_port = sidecar_port_bindings["8080"][1]
+                extra_sidecar_port_bindings = {
+                    port: binding
+                    for port, binding in sidecar_port_bindings.items()
+                    if port not in {"44772", "8080"}
+                }
                 sidecar_container = self._start_egress_sidecar(
                     sandbox_id=sandbox_id,
                     network_policy=request.network_policy,
                     egress_token=egress_token,
                     host_execd_port=host_execd_port,
                     host_http_port=host_http_port,
+                    extra_port_bindings=extra_sidecar_port_bindings,
                 )
                 labels[SANDBOX_EMBEDDING_PROXY_PORT_LABEL] = str(host_execd_port)
                 labels[SANDBOX_HTTP_PORT_LABEL] = str(host_http_port)
                 host_config_kwargs = self._base_host_config_kwargs(
                     effective_mem_limit, effective_nano_cpus, f"container:{sidecar_container.id}"
                 )
+                # Container network namespace is shared with sidecar. Docker rejects
+                # exposing ports on the main container in "container:<id>" mode.
+                container_exposed_ports = None
                 # Drop NET_ADMIN for the main container; only the sidecar should keep it
                 cap_drop = set(host_config_kwargs.get("cap_drop") or [])
                 cap_drop.add("NET_ADMIN")
@@ -1227,17 +1245,14 @@ def _provision_sandbox(
                     effective_mem_limit, effective_nano_cpus, self.network_mode
                 )
                 if self.network_mode != HOST_NETWORK_MODE:
-                    exposed_ports = ["44772", "8080"]
-                    if requested_windows_profile:
-                        # dockur/windows exposes RDP and noVNC/web UI on these ports.
-                        # https://github.com/dockur/windows/blob/master/Dockerfile
-                        exposed_ports.extend(["3389/tcp", "3389/udp", "8006/tcp"])
                     port_bindings = allocate_port_bindings(exposed_ports)
                     host_execd_port = port_bindings["44772"][1]
                     host_http_port = port_bindings["8080"][1]
-                    host_config_kwargs["port_bindings"] = port_bindings
+                    host_config_kwargs["port_bindings"] = normalize_port_bindings(port_bindings)
                     labels[SANDBOX_EMBEDDING_PROXY_PORT_LABEL] = str(host_execd_port)
                     labels[SANDBOX_HTTP_PORT_LABEL] = str(host_http_port)
+                else:
+                    exposed_ports = None
 
             # Inject volume bind mounts into Docker host config
             if volume_binds:
@@ -1260,7 +1275,7 @@ def _provision_sandbox(
                 labels,
                 environment,
                 host_config_kwargs,
-                exposed_ports,
+                container_exposed_ports,
                 request.platform,
             )
         except Exception:
@@ -2336,6 +2351,7 @@ def _start_egress_sidecar(
         egress_token: str,
         host_execd_port: int,
         host_http_port: int,
+        extra_port_bindings: Optional[dict[str, tuple[str, int]]] = None,
     ):
         sidecar_name = f"sandbox-egress-{sandbox_id}"
         sidecar_labels = {
@@ -2357,13 +2373,17 @@ def _start_egress_sidecar(
             f"{OPENSANDBOX_EGRESS_TOKEN}={egress_token}",
         ]
 
+        sidecar_port_bindings: dict[str, tuple[str, int]] = {
+            "44772": ("0.0.0.0", host_execd_port),
+            "8080": ("0.0.0.0", host_http_port),
+        }
+        if extra_port_bindings:
+            sidecar_port_bindings.update(extra_port_bindings)
+
         sidecar_host_config_kwargs: dict[str, Any] = {
             "network_mode": BRIDGE_NETWORK_MODE,
             "cap_add": ["NET_ADMIN"],
-            "port_bindings": {
-                "44772": ("0.0.0.0", host_execd_port),
-                "8080": ("0.0.0.0", host_http_port),
-            },
+            "port_bindings": normalize_port_bindings(sidecar_port_bindings),
         }
         if self.app_config.egress.disable_ipv6:
             # Optional: disable IPv6 in the shared namespace when egress.disable_ipv6 is set.
@@ -2388,7 +2408,7 @@ def _start_egress_sidecar(
                     labels=sidecar_labels,
                     environment=sidecar_env,
                     # Expose the ports that have host bindings so Docker publishes them in bridge mode.
-                    ports=["44772", "8080"],
+                    ports=[normalize_container_port_spec(p) for p in sidecar_port_bindings.keys()],
                 )
             sidecar_container_id = sidecar_resp.get("Id")
             if not sidecar_container_id:
@@ -2460,7 +2480,11 @@ def _create_and_start_container(
                 container_kwargs = {
                     "image": image_uri,
                     "command": bootstrap_command,
-                    "ports": exposed_ports,
+                    "ports": (
+                        [normalize_container_port_spec(p) for p in exposed_ports]
+                        if exposed_ports
+                        else None
+                    ),
                     "name": f"sandbox-{sandbox_id}",
                     "environment": environment,
                     "labels": labels,

server/opensandbox_server/services/docker_port_allocator.py

@@ -23,6 +23,28 @@
 from opensandbox_server.services.constants import SandboxErrorCodes
 
 
+def normalize_container_port_spec(port_spec: str) -> str:
+    token = str(port_spec).strip()
+    if token.endswith("/tcp"):
+        return token[:-4]
+    return token
+
+
+def normalize_port_bindings(
+    port_bindings: dict[str, tuple[str, int]],
+) -> dict[str, tuple[str, int]]:
+    """
+    Normalize binding keys to docker-py canonical forms.
+
+    Docker port bindings accept "port" for tcp and "port/udp" for udp.
+    """
+    normalized: dict[str, tuple[str, int]] = {}
+    for container_port, binding in port_bindings.items():
+        normalized_key = normalize_container_port_spec(container_port)
+        normalized[normalized_key] = binding
+    return normalized
+
+
 def allocate_host_port(
     min_port: int = 40000,
     max_port: int = 60000,

server/tests/test_docker_service.py

@@ -962,6 +962,54 @@ def host_cfg_side_effect(**kwargs):
     hc2 = mock_client.api.create_host_config.call_args.kwargs
     assert hc2["sysctls"]["net.ipv6.conf.all.disable_ipv6"] == 1
 
+
+@patch("opensandbox_server.services.docker.docker")
+def test_egress_sidecar_normalizes_windows_port_bindings(mock_docker):
+    mock_client = MagicMock()
+    mock_client.containers.list.return_value = []
+
+    def host_cfg_side_effect(**kwargs):
+        return kwargs
+
+    sidecar_container = MagicMock()
+    mock_client.api.create_host_config.side_effect = host_cfg_side_effect
+    mock_client.api.create_container.return_value = {"Id": "sidecar-id"}
+    mock_client.containers.get.return_value = sidecar_container
+    mock_docker.from_env.return_value = mock_client
+
+    cfg = _app_config()
+    cfg.docker.network_mode = "bridge"
+    cfg.egress = EgressConfig(image="egress:latest", disable_ipv6=False)
+    service = DockerSandboxService(config=cfg)
+
+    with (
+        patch.object(service, "_ensure_image_available"),
+        patch.object(service, "_docker_operation") as mock_op,
+    ):
+        mock_op.return_value.__enter__.return_value = None
+        mock_op.return_value.__exit__.return_value = None
+        service._start_egress_sidecar(
+            "sandbox-id",
+            NetworkPolicy(defaultAction="deny", egress=[]),
+            egress_token="egress-token",
+            host_execd_port=44772,
+            host_http_port=8080,
+            extra_port_bindings={
+                "3389/tcp": ("0.0.0.0", 53389),
+                "3389/udp": ("0.0.0.0", 53390),
+                "8006/tcp": ("0.0.0.0", 58006),
+            },
+        )
+
+    hc_kwargs = mock_client.api.create_host_config.call_args.kwargs
+    assert "3389" in hc_kwargs["port_bindings"]
+    assert "3389/udp" in hc_kwargs["port_bindings"]
+    assert "8006" in hc_kwargs["port_bindings"]
+    sidecar_kwargs = mock_client.api.create_container.call_args.kwargs
+    assert "3389" in sidecar_kwargs["ports"]
+    assert "3389/udp" in sidecar_kwargs["ports"]
+    assert "8006" in sidecar_kwargs["ports"]
+
 def test_expire_cleans_sidecar():
     service = DockerSandboxService(config=_app_config())
     mock_container = MagicMock()
@@ -1327,9 +1375,9 @@ async def test_create_sandbox_windows_profile_injects_runtime_defaults(mock_dock
     port_bindings = host_config_kwargs["port_bindings"]
     assert "44772" in port_bindings
     assert "8080" in port_bindings
-    assert "3389/tcp" in port_bindings
+    assert "3389" in port_bindings
     assert "3389/udp" in port_bindings
-    assert "8006/tcp" in port_bindings
+    assert "8006" in port_bindings
 
 
 @pytest.mark.asyncio
@@ -1496,6 +1544,86 @@ async def test_create_sandbox_windows_profile_accepts_dockur_demo_like_request(m
     assert response.platform.os == "windows"
     assert response.platform.arch == "amd64"
 
+
+@pytest.mark.asyncio
+@patch("opensandbox_server.services.docker.docker")
+async def test_create_sandbox_windows_profile_with_network_policy_maps_windows_ports(mock_docker):
+    mock_client = MagicMock()
+    mock_client.containers.list.return_value = []
+    mock_docker.from_env.return_value = mock_client
+
+    cfg = _app_config()
+    cfg.runtime.execd_image = "ghcr.io/opensandbox/execd:v1.0.11"
+    cfg.docker.network_mode = "bridge"
+    cfg.egress = EgressConfig(image="opensandbox/egress:latest")
+    service = DockerSandboxService(config=cfg)
+    request = CreateSandboxRequest(
+        image=ImageSpec(uri="dockurr/windows:latest"),
+        resourceLimits=ResourceLimits(
+            root={
+                "cpu": "4",
+                "memory": "8G",
+                "disk": "64G",
+            }
+        ),
+        env={"VERSION": "11"},
+        entrypoint=["cmd", "/c", "echo ready"],
+        platform=PlatformSpec(os="windows", arch="amd64"),
+        networkPolicy=NetworkPolicy(default_action="deny", egress=[]),
+    )
+    created_container = MagicMock()
+    created_container.image.attrs = {"Os": "windows", "Architecture": "amd64"}
+    sidecar = MagicMock()
+    sidecar.id = "sidecar-123"
+
+    with (
+        patch(
+            "opensandbox_server.services.docker.validate_windows_runtime_prerequisites",
+            return_value=None,
+        ),
+        patch("opensandbox_server.services.docker.generate_egress_token", return_value="egress-token"),
+        patch(
+            "opensandbox_server.services.docker.allocate_port_bindings",
+            return_value={
+                "44772": ("0.0.0.0", 51664),
+                "8080": ("0.0.0.0", 48891),
+                "3389/tcp": ("0.0.0.0", 53389),
+                "3389/udp": ("0.0.0.0", 53390),
+                "8006/tcp": ("0.0.0.0", 58006),
+            },
+        ),
+        patch.object(service, "_start_egress_sidecar", return_value=sidecar) as mock_start_sidecar,
+        patch.object(
+            service,
+            "_create_and_start_container",
+            return_value=created_container,
+        ) as mock_create,
+    ):
+        await service.create_sandbox(request)
+
+    _, start_kwargs = mock_start_sidecar.call_args
+    assert start_kwargs["host_execd_port"] == 51664
+    assert start_kwargs["host_http_port"] == 48891
+    assert start_kwargs["extra_port_bindings"] == {
+        "3389/tcp": ("0.0.0.0", 53389),
+        "3389/udp": ("0.0.0.0", 53390),
+        "8006/tcp": ("0.0.0.0", 58006),
+    }
+
+    forwarded_env = mock_create.call_args.args[4]
+    host_config_kwargs = mock_create.call_args.args[5]
+    forwarded_ports = mock_create.call_args.args[6]
+    labels = mock_create.call_args.args[3]
+
+    assert "USER_PORTS=44772,8080,3389,8006" in forwarded_env
+    assert host_config_kwargs["network_mode"] == "container:sidecar-123"
+    assert "NET_ADMIN" in set(host_config_kwargs.get("cap_add") or [])
+    assert "NET_ADMIN" not in set(host_config_kwargs.get("cap_drop") or [])
+    assert forwarded_ports is None
+    assert labels["opensandbox.io/embedding-proxy-port"] == "51664"
+    assert labels["opensandbox.io/http-port"] == "48891"
+
+
 def test_restore_existing_sandboxes_ignores_manual_cleanup_without_warning():
     service = DockerSandboxService(config=_app_config())
     manual_container = MagicMock()