NVIDIA / NVIDIA/OpenShell

bug: make stable sandbox ID authoritative across compute-driver operations

Open
#2,347 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area:compute state:stale
Dominant language
Rust
Stars
8.7k
Forks
1.3k
Avg merge
2d 11h
Merged PRs (30d)
253

Description

Agent Diagnostic

  • Skills loaded: create-github-issue
  • OpenShell version tested: source at PR #2340, commit ab966edbd868a0a5fd04adbb7c8c67a0580f8f5d
  • Latest release checked: v0.0.86
  • Known fixes reviewed: searched merged compute-driver identity and lifecycle PRs; no fix defining or consistently enforcing stable-ID semantics was found
  • Possible duplicates reviewed: #2183 proposes a driver conformance suite but does not define identifier precedence; #2326 and PR #2340 exposed this existing inconsistency while changing delete synchronization
  • Findings: GetSandboxRequest, StopSandboxRequest, and DeleteSandboxRequest have carried both sandbox_id and sandbox_name since the compute-driver API was introduced. Drivers inconsistently treat the stable ID as authoritative. Kubernetes delete ignores it, Podman stop targets only by name, VM delete falls back to name after an ID miss, and Docker pending-operation matching accepts an ID or name match. Read paths generally validate the returned ID after resolving by name.
  • Remaining reason for filing: a stale request for sandbox A can mutate a replacement sandbox B that reused A's name. The protocol contract and all first-party drivers need one consistent rule.

Description

Actual behavior: Compute-driver mutation requests carry both a stable gateway sandbox ID and a runtime name, but drivers do not consistently verify that the runtime resource selected by name has the requested ID before stopping or deleting it. In an out-of-band deletion, delayed watcher, restart, or multi-gateway sequence, a name can be reused while an operation for the old ID remains active. That operation can then stop or delete the replacement sandbox.

Known inconsistencies include:

  • Kubernetes delete selects and deletes the Sandbox resource by name without checking sandbox_id.
  • Podman stop ignores sandbox_id; delete inspects and removes the container by name even if its ID label disagrees, while using the request ID for associated cleanup.
  • VM delete looks up by ID first but falls back to a matching name when the requested ID is absent.
  • Docker active-container lookup verifies ID and name, but pending sandbox matching currently accepts either an ID match or a name match.
  • Kubernetes and VM stop are currently unimplemented, but should follow the same contract when implemented.

Expected behavior: When a request supplies sandbox_id, that ID is authoritative. sandbox_name may be used as a runtime locator, but the driver must verify the located resource belongs to the requested ID before any mutation. An ID/name mismatch or an absent requested ID must never fall back to mutating a resource that merely has the same name.

Reproduction Steps

  1. Create sandbox A with ID A and runtime name foo.
  2. Retain or delay a stop/delete request containing {sandbox_id: A, sandbox_name: foo}.
  3. Remove A outside that request, then recreate foo as sandbox B with ID B.
  4. Allow the stale operation to reach a driver that selects resources by name.
  5. Observe that the operation can stop or delete B even though the request identifies A.

The exact mechanism for delaying step 2 differs by driver. Unit tests can reproduce the contract violation directly by seeding a runtime resource with name foo and ID B, then sending a request for ID A and name foo.

Proposed Design

  • Define the identifier contract for GetSandbox, StopSandbox, and DeleteSandbox:
    • If sandbox_id is present, it is the authoritative identity.
    • sandbox_name is an additional runtime locator or diagnostic field, not an alternative identity.
    • A located resource whose stable ID differs must produce FAILED_PRECONDITION (or a documented equivalent) before mutation.
    • If the requested ID is absent, do not fall back to another resource with the same name.
  • Audit Docker, Kubernetes, Podman, and VM implementations of get, stop, and delete.
  • After resolving a name, mutate through an immutable backend identifier where available (for example, a container ID or Kubernetes UID precondition).
  • Preserve name-only behavior only for explicitly supported legacy calls where sandbox_id is empty; gateway-originated mutation requests should provide the ID.
  • Add the mismatch and name-reuse cases to the driver conformance work tracked by #2183.

Definition of Done

  • The compute-driver identifier precedence and mismatch behavior are documented in the protobuf contract.
  • All first-party GetSandbox, StopSandbox, and DeleteSandbox implementations follow that contract.
  • No driver mutates a same-name resource after a supplied stable ID fails to match.
  • Driver-level tests cover {requested ID: A, selected resource ID: B, shared name: foo} for every implemented stop/delete path.
  • Name-reuse coverage exercises delayed or stale operations against replacement resources.
  • The driver conformance suite issue #2183 includes an identifier-mismatch scenario or explicitly links to the resulting shared test coverage.

Environment

  • OS: Linux development workspace
  • Docker: not required; finding confirmed through source inspection and existing unit-test infrastructure
  • OpenShell: PR #2340 at ab966edbd868a0a5fd04adbb7c8c67a0580f8f5d
  • Latest release checked: v0.0.86
  • Possible duplicates checked: yes; #2183 is related but not a duplicate

Logs

No runtime error log is required. This issue records an inconsistent compute-driver identity contract found during review of PR #2340.

Related

  • #2326
  • #2183
  • #2340

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the protobuf contract for GetSandbox, StopSandbox, and DeleteSandbox, then trace the corresponding Docker, Kubernetes, Podman, and VM driver implementations. Use the existing unit-test infrastructure to cover a requested ID A, selected resource ID B, and shared name foo; done means the contract, implemented paths, and conformance work in #2183 all reject mismatches before mutation.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, kubernetes, rust
Domain
backend-api-design, infrastructure, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.