fix(server): make resourceLimits/image/entrypoint optional when poolRef is set by longsuizhi · Pull Request #883 · opensandbox-group/OpenSandbox

longsuizhi · 2026-05-13T13:03:01Z

Summary

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. Currently the API requires callers to provide dummy values for these fields, which is unnecessary and error-prone.

Fixes #885

Changes

schema.py: Make resource_limits Optional[ResourceLimits] with None default; skip image/snapshotId/entrypoint validation when poolRef is present; reject snapshotId + poolRef combination; add explicit resourceLimits required check for non-pool requests
kubernetes_service.py: Skip resolve_sandbox_image_from_request and ensure_entrypoint when poolRef is set; guard _ensure_image_auth_support against None image
docker_service.py: Reject poolRef on Docker provider (unsupported); guard against None resource_limits
container_ops.py: Guard against None resource_limits
specs/sandbox-lifecycle.yml: Remove required: [resourceLimits], document pool mode behavior, clarify snapshotId is rejected with poolRef

Before (422 error)

curl -X POST /sandboxes -d '{"extensions": {"poolRef": "my-pool"}}'
# => 422: "resourceLimits: Field required"

curl -X POST /sandboxes -d '{"extensions": {"poolRef": "my-pool"}, "resourceLimits": {"cpu": "1", "memory": "1Gi"}}'
# => 422: "Exactly one of image or snapshotId must be provided."

After

curl -X POST /sandboxes -d '{"extensions": {"poolRef": "my-pool"}}'
# => 202: sandbox created from pool

Testing

All 1038 existing tests pass (uv run pytest tests/ -x -q)
Verified on a live K8s cluster: pool-mode sandbox creation works with dummy fields (current workaround) and will work without them after this fix

CLAassistant · 2026-05-13T13:03:18Z

All committers have signed the CLA.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ea9127778

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…ef 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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2abe9a55ed

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- 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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c34c0154a3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

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

Pangjiping · 2026-05-14T05:58:04Z

Thanks for your submission, this is a very valuable fix. I'll review and merge it as soon as possible. 😊

Pangjiping · 2026-05-14T06:19:08Z

Zero new tests for the changed validation paths. The PR modifies 5 files and relaxes a required-field constraint, but no test exercises the new behavior. 1038 existing tests pass, but they only cover the old behavior (everything required).

Specific missing coverage:

Happy path pool mode — POST /sandboxes with only poolRef, no resourceLimits/image/entrypoint. Should return 202. This is the entire point of the PR and it's untested.
snapshotId + poolRef rejection — new validation rule rejects this combination. No test.
resourceLimits still required without poolRef — new explicit ValueError when resource_limits is None and no poolRef. No test.
Docker provider rejects poolRef — new HTTPException with SANDBOX::UNSUPPORTED_POOL_REF. No test.
request.image is None guard in _ensure_image_auth_support — new None-check to avoid AttributeError on None.auth. No test.

…n, 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).

longsuizhi · 2026-05-14T11:56:52Z

Thanks for the detailed review @Pangjiping! I just pushed a commit (0c1356b3) that adds tests for all 5 cases you mentioned:

Pool mode happy path — poolRef only, no image/entrypoint/resourceLimits → validates OK
snapshotId + poolRef rejection — should raise ValidationError
resourceLimits still required without poolRef — standard mode unchanged
Docker provider rejects poolRef — returns 400 with SANDBOX::UNSUPPORTED_POOL_REF
_ensure_image_auth_support None image guard — no AttributeError when request.image is None

Also added a few extra edge cases (blank poolRef ignored, poolRef with optional env/metadata, K8s service-level pool mode skip). All 1046 tests pass now (8 new ones).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c1356b3b2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

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.

Pangjiping

LGTM

hittyt

LGTM

longsuizhi requested review from Generalwin, Pangjiping, hittyt, jwx0925 and ninan-nn as code owners May 13, 2026 13:03

chatgpt-codex-connector Bot reviewed May 13, 2026

View reviewed changes

Comment thread server/opensandbox_server/api/schema.py

Comment thread server/opensandbox_server/api/schema.py

longsuizhi force-pushed the fix/pool-mode-optional-fields branch from 9ea9127 to 2abe9a5 Compare May 13, 2026 13:08

chatgpt-codex-connector Bot reviewed May 13, 2026

View reviewed changes

Comment thread server/opensandbox_server/api/schema.py

Comment thread server/opensandbox_server/api/schema.py

chatgpt-codex-connector Bot reviewed May 13, 2026

View reviewed changes

Comment thread server/opensandbox_server/services/k8s/kubernetes_service.py

Comment thread server/opensandbox_server/api/schema.py

Comment thread specs/sandbox-lifecycle.yml

Comment thread specs/sandbox-lifecycle.yml

fix: guard _ensure_image_auth_support against None image, align spec …

ccb36cb

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

longsuizhi mentioned this pull request May 14, 2026

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

Closed

Pangjiping assigned Pangjiping and ninan-nn May 14, 2026

Pangjiping added bug Something isn't working component/server labels May 14, 2026

chatgpt-codex-connector Bot reviewed May 14, 2026

View reviewed changes

Comment thread specs/sandbox-lifecycle.yml

Comment thread server/opensandbox_server/api/schema.py

Pangjiping reviewed May 17, 2026

View reviewed changes

Pangjiping approved these changes May 17, 2026

View reviewed changes

hittyt approved these changes May 17, 2026

View reviewed changes

hittyt merged commit 5203bef into opensandbox-group:main May 17, 2026
25 checks passed

Conversation

longsuizhi commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Before (422 error)

After

Testing

Uh oh!

CLAassistant commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pangjiping commented May 14, 2026

Uh oh!

Pangjiping commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

longsuizhi commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Pangjiping left a comment

Choose a reason for hiding this comment

Uh oh!

hittyt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

longsuizhi commented May 13, 2026 •

edited

Loading

CLAassistant commented May 13, 2026 •

edited

Loading

Pangjiping commented May 14, 2026 •

edited

Loading

longsuizhi commented May 14, 2026 •

edited

Loading