1
0
mirror of https://github.com/sredevopsorg/deepseek-kubernetes-sandbox.git synced 2026-11-03 09:23:25 +00:00

fix(gateway): grant sandboxclaims patch and map API errors to 503 (#45)

* fix(gateway): grant patch on sandboxclaims for token rotation

The SSO landing path calls K8sClient.patch_token(), which patches the
`dsh-<user>` SandboxClaim's token-hash annotation whenever the browser has
no valid gateway session: first visit, an expired or foreign cookie, or a
gateway restart that drops the in-memory session store. The gateway Role
only granted get/list/create/delete on sandboxclaims, so the patch returned
403 and, because ApiException is not handled, every such GET / answered a
bare 500 "Internal Server Error".

Grant the missing `patch` verb on
extensions.agents.x-k8s.io/sandboxclaims, and update the least-privilege
comment plus the two docs that enumerate the Role verbs.

Verified with `make check` (versions, example layout, manifests) and
`yamllint`; the 403 and the resulting 500 were reproduced from a running
install and cleared by applying the equivalent Role patch (RBAC is evaluated
per request, so no gateway restart is needed).

* fix(gateway): return 503 instead of a bare 500 on Kubernetes API errors

Every gateway route talks to the Kubernetes API, and ApiException used to be
unhandled: Starlette's ServerErrorMiddleware turned it into a bare
500 "Internal Server Error" plus an ERROR traceback. That masked a missing
RBAC verb as a client-visible crash on GET / (the SSO landing path), and it
would equally hide an API outage or throttling.

Register an ApiException handler that logs one credential-free line
(status/reason) and answers 503 + Retry-After with a fixed body, so the
Kubernetes Status payload (which can name cluster objects) is never proxied.
Guard the first get_claim() in the WebSocket route the same way: WebSocket
scopes bypass Starlette's HTTP exception handlers, so it now closes with 1011
instead of escaping as a traceback.

Tests assert the 503 + Retry-After response, cover the SSO landing path that
produced the reported 500 (forbidden claim patch), and assert the WebSocket
close code. The failure-semantics tables document the new behaviour.

---------

Co-authored-by: SREDevOpsAgents <agents@sredevops.org>
This commit is contained in:
SREDevOps.org Agent
2026-11-01 21:38:49 -03:00
committed by GitHub
parent c5b8afde48
commit fbe886f3b9
5 changed files with 67 additions and 9 deletions

View File

@@ -175,9 +175,11 @@ oauth2-proxy's egress to the gateway, DNS and public HTTPS.
| Sandbox not provisioned, wake timeout, handshake failure | `503` + `Retry-After: 10`. The handshake error body is a fixed message: it never contains the launch token, the handshake URL or pod/credential details. |
| User over `MAX_IN_FLIGHT_PER_USER` | `503` + `Retry-After` (HTTP); the existing in-flight requests are untouched |
| Upstream connection/protocol failure | `502` |
| Kubernetes API error (API unreachable/throttled, RBAC `403`, `5xx`) | `503` + `Retry-After: 10`. The body is a fixed message; the Kubernetes `Status` payload (which can name cluster objects) is never proxied, and the API `status`/`reason` is logged once |
| WS auth failure (unknown user, bad token, cross-origin) | accept, then close `4401` |
| WS per-user concurrency cap | accept, then close `1011` |
| WS upstream/handshake failure | accept, then close `1011` |
| WS Kubernetes API error | accept, then close `1011` |
| WS message from DSH over `WS_MAX_MESSAGE_BYTES` | the upstream/relay closes the connection (websockets' message-too-big path) |
A close code can only be delivered after the WebSocket upgrade is accepted;
@@ -230,7 +232,8 @@ would never be sent). HTTPS deployments must set `COOKIE_SECURE=true`.
The gateway's ServiceAccount needs, in its namespace:
* `sandboxclaims` (`extensions.agents.x-k8s.io`): get, list, create, delete
* `sandboxclaims` (`extensions.agents.x-k8s.io`): get, list, create, patch, delete
(patch rotates the token-hash annotation when a session is minted)
* `sandboxes` (`agents.x-k8s.io`): get, list, patch
* `pods`: get, list
* `pods/log`: get

View File

@@ -1510,7 +1510,13 @@ against the JSON API. The browser session is an HttpOnly cookie.</p>
user = websocket.path_params["user"]
if not valid_user(user):
return await self._ws_reject(websocket, 4401)
claim = await self.k8s.get_claim(user)
try:
claim = await self.k8s.get_claim(user)
except ApiException:
# Starlette has no exception handler for WebSocket scopes, so an
# unhandled API error would escape as a traceback and an unclean
# close. 1011 keeps the HTTP 503 meaning on the WS channel.
return await self._ws_reject(websocket, 1011)
if claim is None:
return await self._ws_reject(websocket, 4401)
token, cookie_user = self.credentials(websocket.headers, websocket.cookies)
@@ -1746,6 +1752,28 @@ async def client_gone(request: Request, exc: ClientDisconnect) -> Response:
return Response(status_code=CLIENT_CLOSED_REQUEST)
async def k8s_api_error(request: Request, exc: ApiException) -> Response:
"""Answer a Kubernetes API failure with 503 instead of a bare 500.
The gateway is the only path between the browser and the cluster, so an
API outage, a throttle or a missing RBAC verb is a server-side
availability problem, not a client error. A 503 keeps ingress and
browser retry semantics honest, and the body is a fixed message: the
Kubernetes ``Status`` payload can name cluster objects and is never
proxied. The API ``status``/``reason`` is logged once, credential-free.
"""
print(
"gateway: kubernetes API error: "
f"status={getattr(exc, 'status', None)} reason={getattr(exc, 'reason', None)}",
flush=True,
)
return JSONResponse(
{"error": "cluster API unavailable, retry shortly"},
status_code=503,
headers={"Retry-After": "10"},
)
def create_app(
settings: Settings | None = None,
k8s=None,
@@ -1778,7 +1806,7 @@ def create_app(
]
app = Starlette(
routes=routes,
exception_handlers={ClientDisconnect: client_gone},
exception_handlers={ClientDisconnect: client_gone, ApiException: k8s_api_error},
lifespan=_lifespan(gateway, http_client if owns_http_client else None),
)
app.state.gateway = gateway

View File

@@ -1176,12 +1176,14 @@ async def test_delete_missing_claim_is_404(env):
@pytest.mark.asyncio
async def test_unexpected_api_error_propagates(env):
async def test_unexpected_api_error_returns_503(env):
add_user(env.k8s)
env.k8s.get_claim_error = ApiException(status=500)
async with asgi_client(env) as client:
with pytest.raises(ApiException):
await client.get("/users/alice")
response = await client.get("/users/alice")
assert response.status_code == 503
assert response.headers["retry-after"] == "10"
assert response.json() == {"error": "cluster API unavailable, retry shortly"}
@pytest.mark.asyncio
@@ -1831,6 +1833,16 @@ def test_ws_unauthenticated_closes_4401(env):
assert exc.value.code == 4401
def test_ws_kubernetes_api_error_closes_1011(env):
add_user(env.k8s)
env.k8s.get_claim_error = ApiException(status=503)
with TestClient(env.app) as client:
with client.websocket_connect("/u/alice/api/remote.mux") as ws:
with pytest.raises(WebSocketDisconnect) as exc:
ws.receive_text()
assert exc.value.code == 1011
def test_ws_cookie_cross_origin_closes_4401(env):
add_user(env.k8s)
with TestClient(env.app) as client:
@@ -2149,6 +2161,19 @@ def test_remote_user_returns_valid_and_rejects_invalid(env):
assert g.remote_user({}) is None
@pytest.mark.asyncio
async def test_sso_landing_returns_503_when_claim_patch_is_forbidden(env):
"""A missing RBAC verb on ``patch sandboxclaims`` used to escape as a 500."""
enable_sso(env)
env.k8s.claims["alice"] = {"metadata": {"name": claim_name("alice")}}
env.k8s.patch_error = ApiException(status=403)
async with asgi_client(env) as client:
response = await client.get("/", headers={"X-Forwarded-User": "alice"})
assert response.status_code == 503
assert response.headers["retry-after"] == "10"
assert response.json() == {"error": "cluster API unavailable, retry shortly"}
@pytest.mark.asyncio
async def test_sso_landing_new_user_shows_provisioning(env):
enable_sso(env)