zyratlo opened a new pull request, #8073:
URL: https://github.com/apache/texera/pull/8073

   ### What changes were proposed in this PR?
   
   The notebook migration tool runs in the single-node Docker stack (#7931) and 
in local development (#7933), but the Helm chart has no 
`notebook-migration-service` and no JupyterLab. A Kubernetes deployment shows 
the tool in the workspace with nothing serving its endpoints. This adds both, 
following the chart's existing convention that orchestrator services are global 
and stateful resources are per user.
   
   `notebook-migration-service` becomes a global Deployment alongside the other 
services, with a ServiceAccount whose Role is scoped to the JupyterLab 
namespace only. Per-user JupyterLab pods live in their own namespace behind a 
headless Service, addressed as 
`jupyter-<uid>.<service>.<namespace>.svc.cluster.local`, with a ResourceQuota 
bounding the pool and an optional prepull DaemonSet. A NetworkPolicy stops one 
user's pod reaching another's. One `values.yaml` switch, 
`notebookMigrationService.enabled`, gates the Deployment, the routes, the pool, 
and the button in the workspace, and it defaults to off.
   
   **How browser traffic reaches the right pod**
   This is the design decision worth reviewing closely. JupyterLab is loaded in 
an iframe and then issues its own requests for assets, contents and kernel 
websockets. None of those can carry a Texera token, and Texera has no session 
cookie, so the caller cannot be authenticated per request. Each user's 
JupyterLab is therefore served under `/jupyter/<uid>/`, and the gateway's 
existing ExtAuthz hook resolves that uid to the pod's recorded address and 
rewrites `Host`. It reuses `dynamic-routes`, its `DynamicResolver` backend, and 
the `SecurityPolicy` that already forwards `Host`, so no new gateway machinery 
is introduced.
   
   That mechanism routes; it does not authorize. What keeps users apart is the 
per-user JupyterLab token from #7665, derived from a server held secret and 
unguessable. Anyone who can reach the gateway can route to any user's pod and 
will get a 403 from JupyterLab without that user's token. The NetworkPolicy is 
defence in depth on top, closing the one case where a genuinely hostile 
neighbour runs: another user's pod. This is a weaker posture than a per-request 
authorization check, and it is stated plainly in the code rather than left to 
be inferred.
   
   **This PR is not Helm only, unlike what #8006 anticipated.** Four backend 
changes are inseparable from a working deployment:
   
   * JupyterLab serves under a configurable base path, since routing by path 
requires it, and the recorded internal address carries that path because 
JupyterLab serves every endpoint under it, `/api` included.
   * `access-control-service` gains the Jupyter case in `authorize()`.
   * JupyterLab is told which origin may call it. A proxy that rewrites `Host` 
makes JupyterLab's own cross origin check fail on cookie authenticated 
requests, which blocked kernel startup entirely.
   * The browser facing origin is settable directly, not only derived from a 
gateway hostname, because a deployment reached by port forward or NodePort has 
no hostname to derive from.
   
   One change reaches beyond the tool: the gateway route serving `/api/chat` 
and `/api/models` had no explicit timeout, so Envoy's 15 second default 
applied. LLM completions routinely run past that, and the upstream call 
succeeds and is then discarded, so every conversion failed while burning the 
API call. The route now takes its timeout from 
`gatewayConfig.llmRequestTimeout`, defaulting to 10m to match 
`python-notebook-migration-timeout-minutes`. This also fixes the agent chat, 
which shares the route.
   
   Known limitations, both worth their own issues:
   
   * JupyterLab pods have no persistent volume, so a pod restart empties 
`work/`. Notebooks survive in the database but are not re-uploaded 
automatically. The same is true of a single-node container restart, so this is 
not specific to Kubernetes.
   * A dead pod self-heals on the next request, but an idle one is never 
reclaimed.
   
   ### Any related issues, documentation, discussions?
   
   Closes #8006
   Parent issue #4301
   Depends on #7665 (PR #8032), which is still open: without per-user 
resolution a global service would hand every user the same JupyterLab and 
token, which is exactly what this topology exists to prevent. This branch is 
stacked on that work, so the diff will shrink once #8032 merges. Roadmap 
context is the architectural note on #5258.
   
   ### How was this PR tested?
   
   Unit tests pass across the touched modules: `Config/test` (71), 
`NotebookMigrationService/test` (107), `AccessControlService/test` (62), with 
`scalafmtCheckAll` and a repo wide `Test/compile` clean. `helm template` 
renders in both modes, and with the feature disabled the chart emits no Jupyter 
or notebook-migration objects at all.
   
   Deployed and exercised end to end on a local Minikube cluster running 
Calico, chosen because Minikube's default CNI creates NetworkPolicy objects 
without enforcing them, which would make the isolation commit look correct 
while enforcing nothing. Images were built from this branch, since the 
published `latest` images predate #8032.
   
   Verified on that cluster: the feature flag reaches the frontend; opening the 
panel provisions `jupyter-<uid>` and moves the quota; the issued token matches 
an independent HMAC derivation of the same uid; the pod's labels, hostname and 
subdomain match what the headless Service selects; `/jupyter/<uid>/` reaches 
the right pod through the gateway while the same URL without a token is 
redirected to JupyterLab's login; the iframe renders and cell click sync works 
in both directions.
   
   Isolation was tested with an ablation rather than a single observation. With 
the policy in place, `jupyter-1` cannot reach `jupyter-2` on 8888 by pod IP or 
by DNS, in both directions, while the same pod reaches another namespace in the 
same probe, which rules out a broken pod or DNS. Removing the policy makes the 
connection succeed and restoring it blocks again.
   
   The stale row path from #7665 was also confirmed here: deleting a pod 
without touching its registry row causes the next request to log the address as 
unreachable, discard the row and provision a replacement.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   Generated-by: Claude Code (Claude Opus 5)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to