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

   ### What changes were proposed in this PR?
   
   `notebook-migration-service` read one Jupyter URL and token from 
configuration as process-wide values. That is safe only under the per-user-pod 
model, where each user runs their own pod. A single global instance would hand 
every user the same Jupyter and the same token. This resolves both per user 
instead.
   
   This is stage 2 of the staged roadmap in the architectural note on #5258. 
Stage 1 (#7390) and stage 4 (#7671) are merged. Stage 3 (#8006) depends on this 
one.
   
   **Registry table**
   A new `user_jupyter` table holds one row per user whose Jupyter has been 
provisioned, keyed on `uid`. Both the internal and public URLs are stored 
rather than derived at read time, matching how `workflow_computing_unit.uri` is 
the recorded source of truth for a computing unit's address. Storing them lets 
the deployment change its addressing scheme without a code change, which is 
what #8006 needs in order to choose between per-user hostnames and a path 
prefix.
   
   **Derived tokens**
   Each user's token is `HMAC-SHA256(secret, uid)`, computed from 
`storage.jupyter.token-secret`. No credential is stored at rest, any replica of 
the service derives the same value, and rotation is a secret change. The 
service refuses to start when per-user Jupyter is on with an empty secret: an 
empty HMAC key is publicly known, so tokens would still look distinct per user 
while anyone could derive anyone else's.
   
   **Fallback gated on a flag, not on row absence**
   With `kubernetes.jupyter-enabled` off, every user resolves to the statically 
configured Jupyter, which is how the single-node (#7932) and local-dev (#7934) 
deployments run one shared JupyterLab. With it on, a user with no row has 
nothing provisioned yet and gets an unavailable response. Falling back to the 
shared server there would hand an unprovisioned user somebody else's notebooks, 
and would let `delete-notebook` remove another user's file. The flag is 
deliberately separate from `kubernetes.enabled`, so a deployment can run 
computing units on Kubernetes without per-user Jupyter.
   
   **Provisioning**
   `JupyterKubernetesClient` mirrors the computing unit's `KubernetesClient`, 
including the hostname and subdomain pair that makes the pod FQDN resolve 
through a headless service. `get-jupyter-iframe-url` and `set-notebook` 
provision on demand; `get-jupyter-url` and `delete-notebook` resolve only, so 
reading a URL or deleting a file never starts a pod. A registered pod that 
stops answering is discarded and rebuilt, since the row would otherwise outlive 
the pod and point every later request at nothing. Two concurrent first requests 
are resolved by the primary key: the loser keeps the winner's row, which holds 
the same uid-derived addresses.
   
   The uid always comes from the authenticated session and never from a request 
body, so one user cannot address another's Jupyter.
   
   Two notes on the implementation. The Kubernetes client is built on demand 
rather than at object initialisation, unlike the computing unit's, because the 
single-node and local-dev deployments have no cluster to build one against and 
never provision. Adding `io.fabric8:kubernetes-client` also raises three 
transitive versions over Dropwizard's pins (`slf4j-api` to 2.0.13, 
`jackson-dataformat-yaml` and `jackson-datatype-jsr310` to 2.17.0); 
`LICENSE-binary` claims are updated to match.
   
   Out of scope, and left to #8006: the Helm templates, the NetworkPolicy, and 
the global Deployment. Also out of scope is reclaiming idle pods. A dead pod 
self-heals on next use, but nothing reaps one that is merely unused, and the 
row's `ON DELETE CASCADE` drops the registration without deleting the pod. 
Until #8006 supplies an ingress, `kubernetes.jupyter-public-url-template` 
defaults to empty and the browser is handed the in-cluster address.
   
   ### Any related issues, documentation, discussions?
   
   Closes #7665
   Parent issue #4301
   
   ### How was this PR tested?
   
   `NotebookMigrationService/test` passes (72) and `Config/test` passes (71). 
`Test/compile` across the whole repo is clean, as is `scalafmtCheckAll`.
   
   New coverage: token derivation is deterministic per uid, distinct across 50 
uids, and changes with the secret; the start-up guard rejects an empty secret 
only when the feature is on. Resolution falls back with the feature off, 
returns the row with it on, returns nothing for an unregistered user, and never 
returns one user's Jupyter to another. Provisioning is exercised against a 
stubbed Kubernetes client across every branch: fresh provision, reuse of a live 
pod, rebuild of a pod that stopped answering, cleanup when readiness never 
arrives, public URL templating, and the concurrent-registration race.
   
   The existing `withFakeJupyter` suite is untouched and still passes on the 
stub bound to `localhost:9100`, which is the point of the flag-gated fallback: 
test users have no registry row and take the configured path exactly as before.
   
   Applied `sql/texera_ddl.sql` to a scratch database the way CI does, 
confirmed the table shape, and confirmed that deleting a user removes their 
registration row. `check_binary_deps.py` passes under both the PR-mode and the 
strict nightly invocations, and `NOTICE-binary` matches generator output byte 
for byte.
   
   Not tested on a live cluster. The chart has no notebook-migration or Jupyter 
templates yet, and the in-cluster pod address does not resolve from outside the 
cluster, so end to end Kubernetes validation belongs to #8006.
   
   ### 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