fix(server): make resourceLimits/image/entrypoint optional when poolRef is set (#883)

* fix(server): make resourceLimits/image/entrypoint optional when poolRef is set

When creating a sandbox from a pre-configured Pool (via extensions.poolRef),
the image, entrypoint, and resourceLimits are all defined in the Pool CRD
template. Requiring callers to provide dummy values for these fields is
unnecessary and error-prone.

Changes:
- Make resource_limits Optional with None default in CreateSandboxRequest
- Skip image/snapshotId/entrypoint validation when poolRef is present
- Add explicit resourceLimits required check for non-pool requests
- Guard against None resource_limits in Docker provider code paths

* fix(server): address review feedback for pool mode optional fields

- Skip image/entrypoint resolution in K8s service layer when poolRef is set
- Reject poolRef on Docker provider (unsupported)
- Reject snapshotId when poolRef is set (conflicting fields)
- Update specs/sandbox-lifecycle.yml: remove required constraint on
  resourceLimits, document pool mode behavior
- All 1038 tests pass

* fix: guard _ensure_image_auth_support against None image, align spec docs

- Fix AttributeError when image is None in pool mode (P1)
- Clarify in spec that snapshotId is rejected (not optional) with poolRef

* test: add pool mode validation tests for poolRef, snapshotId rejection, Docker guard, and image auth

- Schema: poolRef-only happy path, poolRef+snapshotId rejection, resourceLimits
  still required without poolRef, blank poolRef ignored
- Docker: rejects poolRef with SANDBOX::UNSUPPORTED_POOL_REF
- K8s: pool mode skips image/entrypoint validation, image auth guard handles
  None image without AttributeError

All 1046 tests pass (8 new).

* fix: normalize blank snapshotId to None in pool mode

When poolRef is set and snapshotId is whitespace-only (e.g. '   '),
the validator now clears it to None before returning. This prevents
downstream code from treating a truthy whitespace string as a real
snapshot ID (e.g. writing an invalid Kubernetes label).

Adds test_pool_mode_normalizes_blank_snapshot_id to cover this edge case.

---------

Co-authored-by: longsuizhi <longsuizhi@xiaomi.com>

Author: longsuizhi

server/opensandbox_server/api/schema.py

@@ -403,10 +403,10 @@ class CreateSandboxRequest(BaseModel):
             "null timeout when the workload provider does not support non-expiring sandboxes."
         ),
     )
-    resource_limits: ResourceLimits = Field(
-        ...,
+    resource_limits: Optional[ResourceLimits] = Field(
+        None,
         alias="resourceLimits",
-        description="Runtime resource constraints for the sandbox instance",
+        description="Runtime resource constraints for the sandbox instance. Optional when poolRef is provided.",
     )
     env: Optional[Dict[str, Optional[str]]] = Field(
         None,
@@ -457,6 +457,19 @@ class CreateSandboxRequest(BaseModel):
 
     @model_validator(mode="after")
     def validate_source_and_entrypoint(self) -> "CreateSandboxRequest":
+        # When poolRef is set, image/snapshotId/entrypoint/resourceLimits are
+        # all defined in the Pool CRD and not required from the caller.
+        has_pool_ref = bool((self.extensions or {}).get("poolRef", "").strip())
+        if has_pool_ref:
+            # Reject conflicting fields that would be ignored in pool mode
+            if bool((self.snapshot_id or "").strip()):
+                raise ValueError("snapshotId cannot be used together with poolRef.")
+            # Normalize blank snapshotId so downstream code won't see
+            # a truthy whitespace string (e.g. "   ") as a real value.
+            if self.snapshot_id is not None and not self.snapshot_id.strip():
+                self.snapshot_id = None
+            return self
+
         has_image = self.image is not None and bool(self.image.uri.strip())
         has_snapshot = bool((self.snapshot_id or "").strip())
 
@@ -472,6 +485,9 @@ def validate_source_and_entrypoint(self) -> "CreateSandboxRequest":
         if self.snapshot_id is not None and not has_snapshot:
             self.snapshot_id = None
 
+        if self.resource_limits is None:
+            raise ValueError("resourceLimits is required when poolRef is not provided.")
+
         return self
 
     class Config:

server/opensandbox_server/services/docker/container_ops.py

@@ -329,7 +329,7 @@ def _resolve_image_auth(
     def _resolve_resource_limits(
         self, request: CreateSandboxRequest
     ) -> tuple[Optional[int], Optional[int], Optional[int]]:
-        resource_limits = request.resource_limits.root or {}
+        resource_limits = (request.resource_limits.root if request.resource_limits else None) or {}
         mem_limit = parse_memory_limit(resource_limits.get("memory"))
         nano_cpus = parse_nano_cpus(resource_limits.get("cpu"))
         gpu_count = parse_gpu_request(resource_limits.get("gpu"))

server/opensandbox_server/services/docker/docker_service.py

@@ -607,6 +607,14 @@ async def create_sandbox(self, request: CreateSandboxRequest) -> CreateSandboxRe
         Raises:
             HTTPException: If sandbox creation fails
         """
+        if (request.extensions or {}).get("poolRef", "").strip():
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail={
+                    "code": "SANDBOX::UNSUPPORTED_POOL_REF",
+                    "message": "poolRef is not supported by the Docker provider. Use Kubernetes BatchSandbox provider instead.",
+                },
+            )
         request = resolve_sandbox_image_from_request(request)
         ensure_entrypoint(request.entrypoint or [])
         ensure_metadata_labels(request.metadata)
@@ -761,7 +769,7 @@ def _provision_sandbox(
         requested_windows_profile = is_windows_platform(request.platform)
 
         if requested_windows_profile:
-            validate_windows_resource_limits(request.resource_limits.root or {})
+            validate_windows_resource_limits((request.resource_limits.root if request.resource_limits else None) or {})
             validate_windows_runtime_prerequisites()
 
         # Prepare OSSFS mounts first so binds can reference mounted host paths.
@@ -855,7 +863,7 @@ def _provision_sandbox(
                 )
                 environment = inject_windows_resource_limits_env(
                     environment,
-                    request.resource_limits.root or {},
+                    (request.resource_limits.root if request.resource_limits else None) or {},
                 )
                 environment = inject_windows_user_ports(environment, exposed_ports)
 

server/opensandbox_server/services/k8s/kubernetes_service.py

@@ -264,7 +264,7 @@ def _ensure_image_auth_support(self, request: CreateSandboxRequest) -> None:
 
         Raises HTTP 400 if the provider does not support per-request image auth.
         """
-        if request.image.auth is None:
+        if request.image is None or request.image.auth is None:
             return
         if self.workload_provider.supports_image_auth():
             return
@@ -404,8 +404,11 @@ async def create_sandbox(self, request: CreateSandboxRequest) -> CreateSandboxRe
         Raises:
             HTTPException: If creation fails, timeout, or invalid parameters
         """
-        request = resolve_sandbox_image_from_request(request)
-        ensure_entrypoint(request.entrypoint or [])
+        has_pool_ref = bool((request.extensions or {}).get("poolRef", "").strip())
+
+        if not has_pool_ref:
+            request = resolve_sandbox_image_from_request(request)
+            ensure_entrypoint(request.entrypoint or [])
         ensure_metadata_labels(request.metadata)
         ensure_platform_valid(request.platform)
         ensure_timeout_within_limit(

server/tests/k8s/test_kubernetes_service.py

@@ -525,6 +525,67 @@ async def test_create_sandbox_rejects_timeout_above_configured_maximum(
         assert "configured maximum of 3600s" in exc_info.value.detail["message"]
         k8s_service.workload_provider.create_workload.assert_not_called()
 
+    @pytest.mark.asyncio
+    async def test_create_sandbox_pool_mode_skips_image_and_entrypoint_validation(
+        self, k8s_service, mock_workload
+    ):
+        """Pool mode: poolRef only, no image/entrypoint/resourceLimits — should succeed."""
+        from opensandbox_server.api.schema import CreateSandboxRequest
+
+        pool_request = CreateSandboxRequest(
+            extensions={"poolRef": "my-pool"},
+        )
+
+        k8s_service.workload_provider.create_workload.return_value = {
+            "name": "test-sandbox-pool",
+            "uid": "pool-123",
+        }
+        k8s_service.workload_provider.get_workload.return_value = mock_workload
+        k8s_service.workload_provider.get_status.return_value = {
+            "state": "Running",
+            "reason": "",
+            "message": "Pod is running",
+            "last_transition_at": datetime.now(timezone.utc),
+        }
+        k8s_service.workload_provider.get_endpoint_info.return_value = "10.244.0.5:8080"
+        k8s_service.workload_provider.get_expiration.return_value = datetime.now(timezone.utc) + timedelta(hours=1)
+
+        response = await k8s_service.create_sandbox(pool_request)
+
+        assert response.id is not None
+        assert response.status.state == "Running"
+        k8s_service.workload_provider.create_workload.assert_called_once()
+
+    @pytest.mark.asyncio
+    async def test_create_sandbox_pool_mode_image_auth_guard_no_error(
+        self, k8s_service, mock_workload
+    ):
+        """Pool mode with image=None should not raise AttributeError in _ensure_image_auth_support."""
+        from opensandbox_server.api.schema import CreateSandboxRequest
+
+        pool_request = CreateSandboxRequest(
+            extensions={"poolRef": "my-pool"},
+        )
+        assert pool_request.image is None
+
+        k8s_service.workload_provider.create_workload.return_value = {
+            "name": "test-sandbox-pool2",
+            "uid": "pool-456",
+        }
+        k8s_service.workload_provider.get_workload.return_value = mock_workload
+        k8s_service.workload_provider.get_status.return_value = {
+            "state": "Running",
+            "reason": "",
+            "message": "Pod is running",
+            "last_transition_at": datetime.now(timezone.utc),
+        }
+        k8s_service.workload_provider.get_endpoint_info.return_value = "10.244.0.6:8080"
+        k8s_service.workload_provider.get_expiration.return_value = datetime.now(timezone.utc) + timedelta(hours=1)
+
+        # Should not raise AttributeError on None.auth
+        response = await k8s_service.create_sandbox(pool_request)
+        assert response.id is not None
+
 class TestWaitForSandboxReady:
     """_wait_for_sandbox_ready method tests"""
 

server/tests/test_docker_service.py

@@ -330,6 +330,29 @@ async def test_create_sandbox_rejects_invalid_metadata(mock_docker):
     assert exc.value.detail["code"] == SandboxErrorCodes.INVALID_METADATA_LABEL
     mock_client.containers.create.assert_not_called()
 
+@pytest.mark.asyncio
+@patch("opensandbox_server.services.docker.docker_service.docker")
+async def test_create_sandbox_rejects_pool_ref_on_docker(mock_docker):
+    mock_client = MagicMock()
+    mock_client.containers.list.return_value = []
+    mock_docker.from_env.return_value = mock_client
+
+    service = DockerSandboxService(config=_app_config())
+
+    request = CreateSandboxRequest(
+        image=ImageSpec(uri="python:3.11"),
+        entrypoint=["python"],
+        resourceLimits=ResourceLimits(root={}),
+        extensions={"poolRef": "my-pool"},
+    )
+
+    with pytest.raises(HTTPException) as exc:
+        await service.create_sandbox(request)
+
+    assert exc.value.status_code == status.HTTP_400_BAD_REQUEST
+    assert exc.value.detail["code"] == "SANDBOX::UNSUPPORTED_POOL_REF"
+    mock_client.containers.create.assert_not_called()
+
 @pytest.mark.asyncio
 @patch("opensandbox_server.services.docker.docker_service.docker")
 async def test_create_sandbox_rejects_timeout_above_configured_maximum(mock_docker):

server/tests/test_schema.py

@@ -601,3 +601,61 @@ def test_request_allows_timeout_above_previous_hardcoded_limit(self):
         assert request.timeout == 172800
 
 
+class TestCreateSandboxRequestPoolMode:
+    """Tests for pool mode (extensions.poolRef) validation."""
+
+    def test_pool_mode_accepts_only_pool_ref(self):
+        """Happy path: poolRef only, no image/entrypoint/resourceLimits required."""
+        request = CreateSandboxRequest(
+            extensions={"poolRef": "my-pool"},
+        )
+        assert request.image is None
+        assert request.entrypoint is None
+        assert request.resource_limits is None
+        assert request.extensions["poolRef"] == "my-pool"
+
+    def test_pool_mode_accepts_pool_ref_with_optional_fields(self):
+        """poolRef with optional env/metadata/timeout should be valid."""
+        request = CreateSandboxRequest(
+            extensions={"poolRef": "my-pool"},
+            env={"KEY": "value"},
+            metadata={"team": "test"},
+            timeout=600,
+        )
+        assert request.extensions["poolRef"] == "my-pool"
+        assert request.env == {"KEY": "value"}
+
+    def test_pool_mode_rejects_snapshot_id_with_pool_ref(self):
+        """snapshotId and poolRef cannot be used together."""
+        with pytest.raises(ValidationError) as exc_info:
+            CreateSandboxRequest(
+                snapshotId="snap-001",
+                extensions={"poolRef": "my-pool"},
+            )
+        errors = exc_info.value.errors()
+        assert any("snapshotId" in str(e) and "poolRef" in str(e) for e in errors)
+
+    def test_resource_limits_required_without_pool_ref(self):
+        """Without poolRef, resourceLimits is still required (image mode)."""
+        with pytest.raises(ValidationError):
+            CreateSandboxRequest(
+                image=ImageSpec(uri="python:3.11"),
+                entrypoint=["python"],
+            )
+
+    def test_pool_mode_normalizes_blank_snapshot_id(self):
+        """Blank snapshotId (e.g. whitespace) should be normalized to None in pool mode."""
+        req = CreateSandboxRequest(
+            extensions={"poolRef": "my-pool"},
+            snapshotId="   ",
+        )
+        assert req.snapshot_id is None
+
+    def test_pool_mode_ignores_blank_pool_ref(self):
+        """Blank poolRef should not trigger pool mode."""
+        with pytest.raises(ValidationError):
+            CreateSandboxRequest(
+                extensions={"poolRef": "   "},
+            )
+
+

specs/sandbox-lifecycle.yml

@@ -1155,15 +1155,22 @@ components:
 
     CreateSandboxRequest:
       type: object
-      required: [resourceLimits]
       description: |
-        Request to create a new sandbox from either a container image or a snapshot.
-        Exactly one of `image` or `snapshotId` must be provided.
+        Request to create a new sandbox from either a container image, a snapshot,
+        or a pre-configured pool (via `extensions.poolRef`).
+
+        **Standard mode**: Exactly one of `image` or `snapshotId` must be provided,
+        and `resourceLimits` is required.
 
         When `image` is provided, `entrypoint` is required. When `snapshotId` is
         provided, `entrypoint` is optional. If omitted, the server defaults the
         sandbox entrypoint to `["tail", "-f", "/dev/null"]`.
 
+        **Pool mode**: When `extensions.poolRef` is set, the sandbox is created from
+        a pre-configured pool. In this case `image`, `entrypoint`, and
+        `resourceLimits` are all optional (defined by the Pool CRD template).
+        `snapshotId` must not be provided together with `poolRef`.
+
         **Note**: API Key authentication is required via the `OPEN-SANDBOX-API-KEY` header.
       properties:
         image:
@@ -1204,6 +1211,8 @@ components:
           $ref: '#/components/schemas/ResourceLimits'
           description: |
             Runtime resource constraints for the sandbox instance.
+            Required when `extensions.poolRef` is not set.
+            Optional when using pool mode (resource limits are defined by the Pool CRD template).
             SDK clients should provide sensible defaults (e.g., cpu: "500m", memory: "512Mi").
 
         env: