mengw15 commented on code in PR #8073:
URL: https://github.com/apache/texera/pull/8073#discussion_r3931673108
##########
bin/k8s/values.yaml:
##########
@@ -195,6 +195,61 @@ webserver:
type: ClusterIP
port: 8080
+notebookMigrationService:
+ # Turns the whole notebook migration tool on or off: the service, its route,
the per-user
+ # JupyterLab pool, and the button in the workspace.
+ enabled: false
+ name: notebook-migration-service
+ numOfPods: 1
+ serviceAccountName: notebook-migration-service-service-account
+ imageName: texera-notebook-migration-service
+ service:
+ type: ClusterIP
+ port: 9098
+ # Origin the browser reaches Texera on, used for the JupyterLab iframe URL
and for the
+ # CSP that lets Texera embed it. Required wherever there is no DNS name,
such as a
+ # port-forward or a NodePort. Falls back to the gateway hostname when left
empty.
+ publicOrigin: ""
+ # HMAC key each user's JupyterLab token is derived from. Nothing is stored,
so this must
+ # stay stable across restarts or previously issued tokens stop matching.
+ # Development-only default. Production environments MUST override this with
a different,
+ # securely generated secret.
+ jupyterTokenSecret:
"c4e1f7a9b2d5c8e0f3a6b9d2e5f8a1c4b7d0e3f6a9c2b5d8e1f4a7c0b3d6e9f2"
Review Comment:
This ships a real HMAC key in the public repo as the default. #8032's
startup guard refuses an empty secret precisely because a public key lets
anyone derive any user's token — a published default is exactly as public, but
sails past the guard, and the failure is silent. On any deployment that flips
`enabled` without overriding this, anyone can compute `HMAC(default, uid)` for
every uid, and `/jupyter/<uid>/` is deliberately unauthenticated at the
gateway, so the token is the entire wall.
`required` would move the failure to install time, matching the guard's
philosophy; values-development.yaml (which doesn't set this today) can then
carry an explicitly dev-scoped value so the Minikube flow keeps working.
##########
bin/k8s/templates/base/notebook-migration-service/notebook-migration-service-deployment.yaml:
##########
@@ -0,0 +1,99 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+{{- if .Values.notebookMigrationService.enabled }}
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: {{ .Release.Name }}-{{ .Values.notebookMigrationService.name }}
+ namespace: {{ .Release.Namespace }}
+ labels:
+ app: {{ .Release.Name }}-{{ .Values.notebookMigrationService.name }}
+spec:
+ replicas: {{ .Values.notebookMigrationService.numOfPods | default 1 }}
+ selector:
+ matchLabels:
+ app: {{ .Release.Name }}-{{ .Values.notebookMigrationService.name }}
+ template:
+ metadata:
+ labels:
+ app: {{ .Release.Name }}-{{ .Values.notebookMigrationService.name }}
+ spec:
+ # Needed to create and delete each user's JupyterLab pod in the pool
namespace.
+ serviceAccountName: {{
.Values.notebookMigrationService.serviceAccountName }}
+ containers:
+ - name: {{ .Values.notebookMigrationService.name }}
+ image: {{ .Values.texera.imageRegistry }}/{{
.Values.notebookMigrationService.imageName }}:{{ .Values.texera.imageTag }}
+ imagePullPolicy: {{ .Values.texeraImages.pullPolicy }}
+ ports:
+ - containerPort: {{ .Values.notebookMigrationService.service.port
}}
+ env:
+ - name: STORAGE_JDBC_URL
+ value: jdbc:postgresql://{{ .Release.Name
}}-postgresql:5432/texera_db?currentSchema=texera_db,public
+ - name: STORAGE_JDBC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Release.Name }}-postgresql
+ key: postgres-password
+ # Resolve each user's JupyterLab rather than one shared server.
+ - name: KUBERNETES_JUPYTER_ENABLED
+ value: "true"
+ - name: KUBERNETES_JUPYTER_NAMESPACE
+ value: {{ .Values.jupyterPool.namespace }}
+ - name: KUBERNETES_JUPYTER_SERVICE_NAME
+ value: {{ .Values.jupyterPool.name }}-svc
+ - name: KUBERNETES_JUPYTER_IMAGE_NAME
+ value: {{ .Values.texera.imageRegistry }}/{{
.Values.jupyterPool.imageName }}:{{ .Values.texera.imageTag }}
+ - name: KUBERNETES_JUPYTER_CPU_LIMIT
+ value: "{{ .Values.jupyterPool.resources.cpuLimit }}"
+ - name: KUBERNETES_JUPYTER_MEMORY_LIMIT
+ value: {{ .Values.jupyterPool.resources.memoryLimit }}
+ # The pod's own prefix and the browser-facing address are rendered
from one
+ # basePath, so they cannot drift apart.
+ - name: KUBERNETES_JUPYTER_BASE_URL
+ value: {{ .Values.jupyterPool.basePath }}
+ {{- $origin := .Values.notebookMigrationService.publicOrigin }}
+ {{- if and (not $origin) .Values.gatewayConfig
.Values.gatewayConfig.hostname }}
+ {{- $origin = printf "https://%s" .Values.gatewayConfig.hostname }}
Review Comment:
The scheme can't be inferred from the hostname alone: the chart declares
both listeners unconditionally, so `hostname` without a cert issuer is an HTTP
deployment, and this renders an https:// origin that nothing serves — CSP and
allow_origin then reject the iframe with no error surfaced anywhere. With TLS
terminated at an external LB the guess is right, so it's genuinely undecidable
from here.
Not blocking: gating the inference on TLS actually being configured
(issuer/tlsSecretName), or requiring `publicOrigin` when it isn't, would make
the broken case fail loud instead of silently eating the iframe.
##########
bin/k8s/templates/base/gateway/gateway-routes.yaml:
##########
@@ -135,6 +148,13 @@ spec:
- path:
type: PathPrefix
value: /api/pve
+ {{- if .Values.notebookMigrationService.enabled }}
+ # Per-user JupyterLab. ExtAuthz reads the uid from the path and
rewrites Host to
+ # that user's pod; the per-user Jupyter token is what authorizes the
request.
+ - path:
+ type: PathPrefix
+ value: /jupyter
Review Comment:
`jupyterPool.basePath` drives the pod's base_url, the recorded internal URL
and the public template — but this match is a literal, and so is the uid regex
in AccessControlResource, so any basePath other than /jupyter 404s at the
gateway while the pods faithfully serve the new prefix. A knob half the system
obeys is worse than no knob.
Not blocking: either template this from the value (and derive the regex
prefix from `kubernetes.jupyter-base-url`, which access-control-service already
reads), or drop the knob and hardcode the path with a comment saying so.
--
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]