deer-flow/backend/tests/test_provisioner_request_threading.py
Xinmin Zeng f2e832330e
fix(sandbox): enforce disabled skills in filesystem views (#4178)
* fix(sandbox): project enabled skills into sandbox views

* fix(skills): keep projection mutations consistent

* fix(skills): fail closed on projection errors

* fix(skills): isolate per-scope failures during boot projection rebuild

rebuild_all_skill_projections() propagated any exception from the public
rebuild or from a single user's rebuild straight out of the gateway
lifespan startup, uncaught. A single broken user directory (bad
permissions, corrupted _skill_states.json, unreadable content) would
therefore abort gateway boot for every user, not just that one -
_rebuild_*_locked already fails closed internally (clears the view and
re-raises), so the boot loop only needed to stop treating that re-raise
as fatal.

Each scope's rebuild now fails closed independently and boot continues;
a scope left empty by a boot failure self-heals on the next sandbox
acquire via ensure_skill_projections().

Also patches deerflow.skills.projection.rebuild_all_skill_projections in
the memory-flush lifespan test fixture, matching the two sibling
fixtures in the same file — this call is now on the lifespan startup
path and the fixture's minimal SimpleNamespace config predates it.

* test(skills): update authz test for the projection-aware public toggle

_persist_shared_skill_state (introduced earlier in this branch) reads
the shared extensions_config.json fresh from disk under the projection
lock instead of through the cached get_extensions_config() singleton -
that's the whole point of the fix (stale worker caches must not clobber
another worker's concurrent update). The name no longer exists on the
skills router module, so the test's monkeypatch of it started raising
AttributeError instead of exercising the endpoint.

The mock storage in this test isn't a real LocalSkillStorage instance,
so _persist_shared_skill_state's projection-mutation branch is already
skipped (nullcontext) and it falls back to a fresh ExtensionsConfig()
for the nonexistent tmp config_path - no replacement monkeypatch needed.

* fix(sandbox): make skill projection ensure best-effort in acquire

acquire() called _ensure_skills_projection() directly, outside any
try/except, in both LocalSandboxProvider and AioSandboxProvider. Every
other skill-mount setup path in these providers has always caught
exceptions and logged a warning rather than failing sandbox acquire
outright (e.g. when config.yaml can't be resolved) - these two new call
sites broke that contract, so any projection failure (including simply
not having a config.yaml, as in CI's test environment) now failed
acquire() itself instead of just leaving skill mounts off.

_ensure_skills_projection now catches its own exceptions and returns
None; both providers' callers already tolerate that (a None projection
skips the skill-specific mounts, matching the existing degrade path)
after making _append_public_skill_mapping and the custom/legacy mount
block in LocalSandboxProvider explicitly None-safe.

Caught by running the full suite with config.yaml removed, matching
CI's environment - not caught locally because a real config.yaml was
present, masking the failure.

* fix(sandbox): make E2B skill projection mounts best-effort

_skill_projection_mounts called ensure_skill_projections with no guard,
unlike Local/AIO's _ensure_skills_projection. A raise propagated out of
_apply_mounts before the configured-mounts loop ran, so a skills
projection failure dropped the operator's own configured mounts too -
only caught by create()'s outer warning, with nothing applied at all.

Swallow here and return an empty mount list on failure, matching the
Local/AIO pattern: still fail-closed for skills, but no longer widens
the blast radius to unrelated configured mounts.

Review feedback from PR #4178.

* docs(skills): document projection trade-offs flagged in review

- _update_tree_digest: note the metadata-only (not content) hashing
  trade-off and why runtime writes through this codebase are still
  covered regardless (rebuild-under-lock + rename always changes inode).
- LocalSandboxProvider.acquire: note the acquire-time self-heal cost
  (cheap on a fresh manifest, ~400ms rebuild under lock on stale/drift).
- skill_projection_mutation: drop the no-op except-Exception-then-raise;
  a raise from the mutation already propagates past the yield with the
  view left cleared, no explicit re-raise needed.
- provisioner README: spell out that hostPath skills volumes require
  the gateway and K8s node to share DEER_FLOW_HOST_BASE_DIR (single-node
  or shared storage), and that the custom/legacy volumes' hostPath type
  Directory (not DirectoryOrCreate) makes a violation of that assumption
  a visible Pod-creation failure instead of a silent empty mount.

Review feedback from PR #4178.

* fix(skills): lazily repair user projections

* fix(skills): close projection review gaps

* fix(skills): refresh user projection enable state

* fix(skills): close projection review follow-ups

* fix(skills): preserve state across projection writes

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-07-31 17:55:24 +08:00

302 lines
12 KiB
Python

"""Regression tests for provisioner request-path K8s IO threading."""
from __future__ import annotations
import asyncio
import inspect
import threading
import time
from contextlib import contextmanager
from types import SimpleNamespace
import httpx
import pytest
from blockbuster import BlockBuster
from kubernetes.client.rest import ApiException
class _RecordingCoreV1:
def __init__(
self,
*,
event_loop_thread_id: int,
ready_after_service_reads: dict[str, int] | None = None,
service_read_failures: dict[str, list[int]] | None = None,
) -> None:
self.event_loop_thread_id = event_loop_thread_id
self.thread_ids: list[int] = []
self.service_sandboxes: set[str] = {"sandbox-existing"}
self.ready_after_service_reads = ready_after_service_reads or {}
self.service_read_failures = service_read_failures or {}
self.service_read_counts: dict[str, int] = {}
self.created_pods: list[str] = []
self.created_pod_specs: dict[str, object] = {}
self.created_services: list[str] = []
def _record_k8s_call(self) -> None:
thread_id = threading.get_ident()
self.thread_ids.append(thread_id)
time.sleep(0)
if thread_id == self.event_loop_thread_id:
raise AssertionError("Kubernetes client call ran on the ASGI event-loop thread")
try:
asyncio.get_running_loop()
except RuntimeError:
return
raise AssertionError("Kubernetes client call ran inside an asyncio event loop")
def read_namespaced_service(self, _name: str, _namespace: str):
self._record_k8s_call()
sandbox_id = _sandbox_id_from_service_name(_name)
self.service_read_counts[sandbox_id] = self.service_read_counts.get(sandbox_id, 0) + 1
failures = self.service_read_failures.get(sandbox_id) or []
if failures:
raise ApiException(status=failures.pop(0))
ready_after_reads = self.ready_after_service_reads.get(sandbox_id, 1)
if sandbox_id not in self.service_sandboxes or self.service_read_counts[sandbox_id] < ready_after_reads:
raise ApiException(status=404)
return _node_port_service(sandbox_id)
def read_namespaced_pod(self, _name: str, _namespace: str):
self._record_k8s_call()
return SimpleNamespace(status=SimpleNamespace(phase="Running"))
def create_namespaced_pod(self, _namespace: str, pod) -> None:
self._record_k8s_call()
sandbox_id = pod.metadata.labels["sandbox-id"]
self.created_pods.append(sandbox_id)
self.created_pod_specs[sandbox_id] = pod
def create_namespaced_service(self, _namespace: str, service) -> None:
self._record_k8s_call()
sandbox_id = service.metadata.labels["sandbox-id"]
self.created_services.append(sandbox_id)
self.service_sandboxes.add(sandbox_id)
def delete_namespaced_service(self, _name: str, _namespace: str) -> None:
self._record_k8s_call()
def delete_namespaced_pod(self, _name: str, _namespace: str) -> None:
self._record_k8s_call()
def list_namespaced_service(self, _namespace: str, *, label_selector: str):
self._record_k8s_call()
assert label_selector == "app=deer-flow-sandbox"
return SimpleNamespace(items=[_node_port_service("sandbox-listed")])
def _node_port_service(sandbox_id: str):
return SimpleNamespace(
metadata=SimpleNamespace(labels={"sandbox-id": sandbox_id}),
spec=SimpleNamespace(ports=[SimpleNamespace(name="http", port=8080, node_port=32123)]),
)
def _sandbox_id_from_service_name(name: str) -> str:
assert name.startswith("sandbox-")
assert name.endswith("-svc")
return name[len("sandbox-") : -len("-svc")]
@contextmanager
def _detect_provisioner_blocking_io(provisioner_module):
detector = BlockBuster(scanned_modules=[provisioner_module.__name__])
detector.activate()
try:
yield
finally:
detector.deactivate()
def test_sandbox_business_route_handlers_are_sync(provisioner_module) -> None:
"""FastAPI runs sync handlers in its worker pool, away from the event loop."""
for handler in (
provisioner_module.create_sandbox,
provisioner_module.destroy_sandbox,
provisioner_module.get_sandbox,
provisioner_module.list_sandboxes,
):
assert not inspect.iscoroutinefunction(handler)
@pytest.mark.asyncio
@pytest.mark.parametrize(
("method", "path", "json_body", "expected_created_sandbox"),
[
("POST", "/api/sandboxes", {"sandbox_id": "sandbox-existing", "thread_id": "thread-1", "user_id": "user-1"}, None),
("POST", "/api/sandboxes", {"sandbox_id": "sandbox-new", "thread_id": "thread-1", "user_id": "user-1"}, "sandbox-new"),
("DELETE", "/api/sandboxes/sandbox-existing", None, None),
("GET", "/api/sandboxes/sandbox-existing", None, None),
("GET", "/api/sandboxes", None, None),
],
ids=["create-existing", "create-new", "destroy", "get", "list"],
)
async def test_sandbox_business_routes_run_k8s_client_off_event_loop_thread(
method: str,
path: str,
json_body: dict[str, str] | None,
expected_created_sandbox: str | None,
monkeypatch: pytest.MonkeyPatch,
provisioner_module,
) -> None:
fake_core_v1 = _RecordingCoreV1(
event_loop_thread_id=threading.get_ident(),
ready_after_service_reads={"sandbox-new": 3},
)
monkeypatch.setattr(provisioner_module, "core_v1", fake_core_v1)
monkeypatch.setattr(provisioner_module, "PROVISIONER_API_KEY", "test-secret")
with _detect_provisioner_blocking_io(provisioner_module):
transport = httpx.ASGITransport(app=provisioner_module.app)
async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
headers = {"X-API-Key": "test-secret"}
if json_body is None:
response = await client.request(method, path, headers=headers)
else:
response = await client.request(method, path, json=json_body, headers=headers)
assert response.status_code == 200
assert fake_core_v1.thread_ids
if expected_created_sandbox is not None:
assert fake_core_v1.created_pods == [expected_created_sandbox]
assert fake_core_v1.created_services == [expected_created_sandbox]
@pytest.mark.parametrize(
("include_legacy_skills", "expected_mount_names"),
[
(
False,
["skills-public", "skills-custom", "skills-legacy", "user-data"],
),
(
True,
["skills-public", "skills-custom", "skills-legacy", "user-data"],
),
],
ids=["without-legacy", "with-legacy"],
)
def test_create_sandbox_route_builds_expected_skills_mount_layout(
include_legacy_skills: bool,
expected_mount_names: list[str],
monkeypatch: pytest.MonkeyPatch,
provisioner_module,
) -> None:
fake_core_v1 = _RecordingCoreV1(
event_loop_thread_id=-1,
ready_after_service_reads={"sandbox-layout": 1},
)
monkeypatch.setattr(provisioner_module, "core_v1", fake_core_v1)
response = provisioner_module.create_sandbox(
provisioner_module.CreateSandboxRequest(
sandbox_id="sandbox-layout",
thread_id="thread-1",
user_id="user-1",
include_legacy_skills=include_legacy_skills,
)
)
assert response.status == "Running"
pod = fake_core_v1.created_pod_specs["sandbox-layout"]
volume_names = [volume.name for volume in pod.spec.volumes]
mount_names = [mount.name for mount in pod.spec.containers[0].volume_mounts]
assert volume_names == expected_mount_names
assert mount_names == expected_mount_names
def test_create_sandbox_retries_transient_service_read_errors(monkeypatch: pytest.MonkeyPatch, provisioner_module) -> None:
fake_core_v1 = _RecordingCoreV1(
event_loop_thread_id=-1,
ready_after_service_reads={"sandbox-transient": 3},
service_read_failures={"sandbox-transient": [503, 429]},
)
monkeypatch.setattr(provisioner_module, "core_v1", fake_core_v1)
monkeypatch.setattr(provisioner_module.time, "sleep", lambda _seconds: None)
response = provisioner_module.create_sandbox(
provisioner_module.CreateSandboxRequest(
sandbox_id="sandbox-transient",
thread_id="thread-1",
user_id="user-1",
)
)
assert response.status == "Running"
assert response.sandbox_url == provisioner_module._sandbox_url("sandbox-transient", node_port=32123)
assert fake_core_v1.service_read_counts["sandbox-transient"] == 3
def test_sandbox_service_defaults_to_node_port_with_node_host_url(provisioner_module) -> None:
provisioner_module.K8S_NAMESPACE = "mdv-sit"
provisioner_module.SANDBOX_CONTAINER_PORT = 8080
provisioner_module.SANDBOX_SERVICE_TYPE = "NodePort"
provisioner_module.NODE_HOST = "node.example"
service = provisioner_module._build_service("abc123")
assert service.spec.type == "NodePort"
assert service.spec.ports[0].port == 8080
assert service.spec.ports[0].target_port == 8080
assert provisioner_module._sandbox_url("abc123", node_port=32123) == "http://node.example:32123"
def test_sandbox_service_supports_cluster_ip_with_dns_url(provisioner_module) -> None:
provisioner_module.K8S_NAMESPACE = "mdv-sit"
provisioner_module.SANDBOX_CONTAINER_PORT = 8080
provisioner_module.SANDBOX_SERVICE_TYPE = "ClusterIP"
service = provisioner_module._build_service("abc123")
assert service.spec.type == "ClusterIP"
assert service.spec.ports[0].port == 8080
assert service.spec.ports[0].target_port == 8080
assert provisioner_module._sandbox_url("abc123") == ("http://sandbox-abc123-svc.mdv-sit.svc.cluster.local:8080")
@pytest.mark.asyncio
async def test_auth_middleware(monkeypatch: pytest.MonkeyPatch, provisioner_module) -> None:
"""Verify the X-API-Key middleware: /health is open; /api/* requires a correct key."""
monkeypatch.setattr(provisioner_module, "PROVISIONER_API_KEY", "test-secret")
fake_core_v1 = _RecordingCoreV1(event_loop_thread_id=-1)
monkeypatch.setattr(provisioner_module, "core_v1", fake_core_v1)
transport = httpx.ASGITransport(app=provisioner_module.app)
async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
# /health is always open — no key needed
r = await client.get("/health")
assert r.status_code == 200
# /api/* with no header → 401
r = await client.get("/api/sandboxes")
assert r.status_code == 401
# /api/* with wrong key → 401
r = await client.get("/api/sandboxes", headers={"X-API-Key": "wrong-key"})
assert r.status_code == 401
# /api/* with correct key → not 401 (auth passed; handler runs with the K8s mock)
r = await client.get("/api/sandboxes", headers={"X-API-Key": "test-secret"})
assert r.status_code != 401
@pytest.mark.asyncio
async def test_auth_middleware_unset_key(monkeypatch: pytest.MonkeyPatch, provisioner_module) -> None:
"""When PROVISIONER_API_KEY is unset/empty, all /api/* routes return 401."""
monkeypatch.setattr(provisioner_module, "PROVISIONER_API_KEY", "")
fake_core_v1 = _RecordingCoreV1(event_loop_thread_id=-1)
monkeypatch.setattr(provisioner_module, "core_v1", fake_core_v1)
transport = httpx.ASGITransport(app=provisioner_module.app)
async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
# /health is always open even when key is unset
r = await client.get("/health")
assert r.status_code == 200
# /api/* is always 401 when key is unset — even with a header
r = await client.get("/api/sandboxes")
assert r.status_code == 401
r = await client.get("/api/sandboxes", headers={"X-API-Key": "anything"})
assert r.status_code == 401