This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new d94581ade4 feat(notebook-migration-service): compute jupyter iframe
url per request (#7602)
d94581ade4 is described below
commit d94581ade4f421adcf3e8b58ae4be73afe04bdf1
Author: Ryan Zhang <[email protected]>
AuthorDate: Fri Aug 14 23:09:31 2026 +0000
feat(notebook-migration-service): compute jupyter iframe url per request
(#7602)
### What changes were proposed in this PR?
Makes `notebook-migration-service` stateless so it can later run as a
single global instance instead of one instance per user. This is the
first backend stage of moving the service onto Texera's "orchestrator
services are global, stateful resources are per user" pattern.
Today the service keeps a shared `@volatile jupyterIframeURL`:
`set-notebook` writes it and `get-jupyter-iframe-url` reads it back.
That shared state is only safe because each user happens to run their
own pod, and even within one user it lets two browser tabs race. This PR
removes the shared state and builds the URL from the request instead.
**`NotebookMigrationResource.scala`**
- Removes the `@volatile var jupyterIframeURL` singleton and the warning
comment that documented its per-user-pod assumption. Adds a
`defaultNotebookName` constant (`notebook.ipynb`).
- `getJupyterIframeURL` now takes a `notebookName` argument and builds
the URL on each call. The name is validated with the same plain `.ipynb`
regex `setNotebook` uses, since it now flows straight into the returned
URL (blocks path traversal). The argument defaults to
`defaultNotebookName`.
- `setNotebook` no longer mutates any shared state; the assignment that
wrote the singleton is gone. Its upload behavior is unchanged.
- The `/get-jupyter-iframe-url` endpoint accepts an optional
`notebookName` query parameter and falls back to the default when it is
absent.
The change is backward compatible. The existing frontend calls the
endpoint with no query parameter, which resolves to `notebook.ipynb`,
exactly the URL it received before. No frontend, config, or deployment
change is needed in this PR, and no other service or branch consumes the
removed state.
### Any related issues, documentation, discussions?
Closes #7390
Parent-issue #4301
### How was this PR tested?
Extends the existing suite in `NotebookMigrationResourceSpec.scala`:
- Fixed the two call sites that pass through the new endpoint signature.
- Added a test that an explicit `notebookName` is honored in the
returned URL.
- Added a test that an invalid `notebookName` is rejected with 400
before any Jupyter call.
- Added a test pinning the refactor: after `setNotebook` uploads
`other.ipynb`, a parameter-less `getJupyterIframeURL` returns the
default `notebook.ipynb`, proving the result no longer depends on state
left by `setNotebook`.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
---
.../resource/NotebookMigrationResource.scala | 47 ++++++++++++++--------
.../resource/NotebookMigrationResourceSpec.scala | 39 ++++++++++++++++--
2 files changed, 67 insertions(+), 19 deletions(-)
diff --git
a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
index ac41ccd28d..17a0a989d7 100644
---
a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
+++
b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
@@ -73,15 +73,16 @@ object NotebookMigrationResource extends LazyLogging {
}
}
+ // jupyterUrl and jupyterToken are single process-wide values, so this
service still
+ // targets one Jupyter per process (the per-user-pod model) and must not be
deployed as a
+ // shared global instance yet: every user would get the same Jupyter and the
same token.
+ // Resolving these per user is a later stage of the migration (#7665).
private val jupyterUrl = StorageConfig.jupyterURL
private val jupyterToken = StorageConfig.jupyterToken
- // The token is passed as a URL param so the browser iframe can authenticate
when loading the notebook.
- // jupyterIframeURL is process-global state. This is safe ONLY because each
user runs their own pod
- // (own notebook-migration-service JVM + own Jupyter) in the k8s deployment,
so this singleton is
- // effectively per-user. Do NOT deploy this service as a shared multi-user
instance without adding
- // per-user keying here, or one user's upload would overwrite another's
iframe URL.
- @volatile private var jupyterIframeURL =
- s"$jupyterUrl/notebooks/work/notebook.ipynb?token=$jupyterToken"
+
+ // Default notebook name used when a request does not specify one, so a
param-less
+ // getJupyterIframeURL call reproduces the URL from before this service
became stateless.
+ private val defaultNotebookName = "notebook.ipynb"
private def isJupyterAvailable(jupyterUrl: String): Boolean = {
var conn: java.net.HttpURLConnection = null
@@ -104,8 +105,17 @@ object NotebookMigrationResource extends LazyLogging {
}
}
- // Returns the Jupyter iframe reference URL
- def getJupyterIframeURL(): Response = {
+ // Returns the Jupyter iframe reference URL for the given notebook.
+ def getJupyterIframeURL(notebookName: String): Response = {
+ // notebookName flows into the returned URL, so validate it the same way
setNotebook does:
+ // block path traversal and keep it to a plain .ipynb filename.
+ if (!notebookName.matches("[A-Za-z0-9._-]+\\.ipynb")) {
+ return Response
+ .status(Response.Status.BAD_REQUEST)
+ .entity(errorJson(s"Invalid notebook name: $notebookName"))
+ .build()
+ }
+
if (!isJupyterAvailable(jupyterUrl)) {
return Response
.status(500)
@@ -120,7 +130,9 @@ object NotebookMigrationResource extends LazyLogging {
.build()
}
- Response.ok(successUrlJson(jupyterIframeURL)).build()
+ Response
+
.ok(successUrlJson(s"$jupyterUrl/notebooks/work/$notebookName?token=$jupyterToken"))
+ .build()
}
// Returns the URL of Jupyter
@@ -153,8 +165,7 @@ object NotebookMigrationResource extends LazyLogging {
// Allow only a plain ".ipynb" filename. Validated before any network
call so a
// bad name is rejected with a 400 up front. This blocks path traversal
in the
- // Jupyter contents URL (e.g. "../../etc/x.ipynb") and keeps
notebookName out of
- // the raw-interpolated jupyterIframeURL JSON (no quotes/control chars).
+ // Jupyter contents URL (e.g. "../../etc/x.ipynb").
if (!notebookName.matches("[A-Za-z0-9._-]+\\.ipynb")) {
return Response
.status(Response.Status.BAD_REQUEST)
@@ -217,8 +228,6 @@ object NotebookMigrationResource extends LazyLogging {
.build()
}
- jupyterIframeURL =
s"$jupyterUrl/notebooks/work/$notebookName?token=$jupyterToken"
-
Response
.ok(
s"""
@@ -457,9 +466,15 @@ class NotebookMigrationResource extends LazyLogging {
@GET
@Path("/get-jupyter-iframe-url")
- def getJupyterIframeURL(@Auth user: SessionUser): Response = {
+ def getJupyterIframeURL(
+ @QueryParam("notebookName") notebookName: String,
+ @Auth user: SessionUser
+ ): Response = {
logger.info("Getting Jupyter iframe URL")
- NotebookMigrationResource.getJupyterIframeURL()
+ val name = Option(notebookName)
+ .filter(_.nonEmpty)
+ .getOrElse(NotebookMigrationResource.defaultNotebookName)
+ NotebookMigrationResource.getJupyterIframeURL(name)
}
@GET
diff --git
a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
index bb1a29c7e5..15ad27eff1 100644
---
a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
+++
b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
@@ -449,7 +449,7 @@ class NotebookMigrationResourceSpec
resource.setNotebook(validNotebook, user).getStatus shouldBe 500
resource.getJupyterURL(user).getStatus shouldBe 500
- resource.getJupyterIframeURL(user).getStatus shouldBe 500
+ resource.getJupyterIframeURL(null, user).getStatus shouldBe 500
}
it should "return 500 when the request body is malformed JSON" in {
@@ -481,9 +481,42 @@ class NotebookMigrationResourceSpec
urlResp.getStatus shouldBe Response.Status.OK.getStatusCode
urlResp.getEntity.toString should include("localhost:9100")
- val iframeResp = resource.getJupyterIframeURL(sessionUser(writerUid))
+ val iframeResp = resource.getJupyterIframeURL(null,
sessionUser(writerUid))
iframeResp.getStatus shouldBe Response.Status.OK.getStatusCode
- iframeResp.getEntity.toString should include("/notebooks/work/")
+ iframeResp.getEntity.toString should
include("/notebooks/work/notebook.ipynb")
+ }
+ }
+
+ it should "build the iframe URL from an explicit notebook name" in {
+ withFakeJupyter(contentsStatus = 201) {
+ val resp = resource.getJupyterIframeURL("other.ipynb",
sessionUser(writerUid))
+ resp.getStatus shouldBe Response.Status.OK.getStatusCode
+ resp.getEntity.toString should include("/notebooks/work/other.ipynb")
+ }
+ }
+
+ it should "reject an invalid notebook name for the iframe URL with 400" in {
+ // notebookName flows into the URL, so it is validated before any Jupyter
call and
+ // rejected without a running server.
+ NotebookMigrationResource
+ .getJupyterIframeURL("../../etc/evil.ipynb")
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+ }
+
+ it should "not be affected by a prior setNotebook call (no shared iframe
state)" in {
+ // Pins the stateless refactor: getJupyterIframeURL builds its URL from
the request, not
+ // from state left by setNotebook. A param-less iframe request after
uploading other.ipynb
+ // must return the default notebook, not the just-uploaded name.
+ withFakeJupyter(contentsStatus = 201) {
+ val user = sessionUser(writerUid)
+ resource
+ .setNotebook("""{"notebookName": "other.ipynb", "notebookData":
{"cells": []}}""", user)
+ .getStatus shouldBe Response.Status.OK.getStatusCode
+
+ val iframe = resource.getJupyterIframeURL(null, user)
+ iframe.getStatus shouldBe Response.Status.OK.getStatusCode
+ iframe.getEntity.toString should
include("/notebooks/work/notebook.ipynb")
+ iframe.getEntity.toString should not include "other.ipynb"
}
}
