fix: preserve the default url in the session spec (#514)

Co-authored-by: Ralf Grubenmann <ralf.grubenmann@sdsc.ethz.ch>

Author: olevski

components/renku_data_services/notebooks/blueprints.py

@@ -4,7 +4,7 @@
 import os
 from dataclasses import dataclass
 from pathlib import PurePosixPath
-from typing import Any
+from typing import Any, cast
 from urllib.parse import urljoin, urlparse
 
 import httpx
@@ -267,7 +267,7 @@ def start(self) -> BlueprintFactoryResponse:
         @authenticate_2(self.authenticator, self.internal_gitlab_authenticator)
         @validate(json=apispec.SessionPostRequest)
         async def _handler(
-            _: Request,
+            request: Request,
             user: AuthenticatedAPIUser | AnonymousAPIUser,
             internal_gitlab_user: APIUser,
             body: apispec.SessionPostRequest,
@@ -381,14 +381,19 @@ async def _handler(
 
             base_server_url = self.nb_config.sessions.ingress.base_url(server_name)
             base_server_path = self.nb_config.sessions.ingress.base_path(server_name)
+            ui_path: str = (
+                f"{base_server_path.rstrip("/")}/{environment.default_url.lstrip("/")}"
+                if len(environment.default_url) > 0
+                else base_server_path
+            )
             annotations: dict[str, str] = {
                 "renku.io/project_id": str(launcher.project_id),
                 "renku.io/launcher_id": body.launcher_id,
                 "renku.io/resource_class_id": str(body.resource_class_id or default_resource_class.id),
             }
             requests: dict[str, str | int] = {
                 "cpu": str(round(resource_class.cpu * 1000)) + "m",
-                "memory": resource_class.memory,
+                "memory": f"{resource_class.memory}Gi",
             }
             if resource_class.gpu > 0:
                 gpu_name = GpuKind.NVIDIA.value + "/gpu"
@@ -409,7 +414,7 @@ async def _handler(
                     hibernated=False,
                     session=Session(
                         image=image,
-                        urlPath=base_server_path,
+                        urlPath=ui_path,
                         port=environment.port,
                         storage=Storage(
                             className=self.nb_config.sessions.storage.pvs_storage_class,
@@ -436,6 +441,7 @@ async def _handler(
                         tlsSecret=TlsSecret(adopt=False, name=self.nb_config.sessions.ingress.tls_secret)
                         if self.nb_config.sessions.ingress.tls_secret is not None
                         else None,
+                        pathPrefix=base_server_path,
                     ),
                     extraContainers=extra_containers,
                     initContainers=session_init_containers,
@@ -454,7 +460,14 @@ async def _handler(
                         else AuthenticationType.token,
                         secretRef=SecretRefKey(name=server_name, key="auth", adopt=True),
                         extraVolumeMounts=[
-                            ExtraVolumeMount(name="renku-authorized-emails", mountPath="/authorized_emails")
+                            # NOTE: Without subpath k8s keeps updating the secret and this can lead to
+                            # the oauth2proxy restarting intermittently even when the secret does not change
+                            # because the oauth2proxy watches this file and restarts on changes
+                            ExtraVolumeMount(
+                                name="renku-authorized-emails",
+                                mountPath="/authorized_emails",
+                                subPath="authorized_emails",
+                            )
                         ]
                         if isinstance(user, AuthenticatedAPIUser)
                         else [],
@@ -477,19 +490,31 @@ async def _handler(
                         "redirect_url": urljoin(base_server_url + "/", "oauth2/callback"),
                         "cookie_path": base_server_path,
                         "proxy_prefix": parsed_proxy_url.path,
-                        "authenticated_emails_file": "/authorized_emails/authorized_emails",
+                        "authenticated_emails_file": "/authorized_emails",
                         "client_secret": self.nb_config.sessions.oidc.client_secret,
                         "cookie_secret": base64.urlsafe_b64encode(os.urandom(32)).decode(),
                         "insecure_oidc_allow_unverified_email": self.nb_config.sessions.oidc.allow_unverified_email,
                     }
                 )
                 secret_data["authorized_emails"] = user.email
             else:
+                # NOTE: We extract the session cookie value here in order to avoid creating a cookie.
+                # The gateway encrypts and signs cookies so the user ID injected in the request headers does not
+                # match the value of the session cookie.
+                session_id = cast(str | None, request.cookies.get(self.nb_config.session_id_cookie_name))
+                if not session_id:
+                    raise errors.UnauthorizedError(
+                        message=f"You have to have a renku session cookie at {self.nb_config.session_id_cookie_name} "
+                        "in order to launch an anonymous session."
+                    )
+                # NOTE: Amalthea looks for the token value first in the cookie and then in the authorization header
                 secret_data["auth"] = safe_dump(
                     {
-                        "token": user.id,
-                        "cookie_key": "Renku-Auth-Anon-Id",
-                        "verbose": True,
+                        "authproxy": {
+                            "token": session_id,
+                            "cookie_key": self.nb_config.session_id_cookie_name,
+                            "verbose": True,
+                        }
                     }
                 )
             secrets_to_create.append(V1Secret(metadata=V1ObjectMeta(name=server_name), string_data=secret_data))

components/renku_data_services/notebooks/config/__init__.py

@@ -121,6 +121,7 @@ class NotebooksConfig:
     session_get_endpoint_annotations: _ServersGetEndpointAnnotations = field(
         default_factory=_ServersGetEndpointAnnotations
     )
+    session_id_cookie_name: str = "_renku_session"  # NOTE: This cookie name is set and controlled by the gateway
 
     @classmethod
     def from_env(cls, db_config: DBConfig) -> Self:

components/renku_data_services/notebooks/cr_amalthea_session.py

@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 #   filename:  <stdin>
-#   timestamp: 2024-10-24T01:41:50+00:00
+#   timestamp: 2024-11-05T01:48:51+00:00
 
 from __future__ import annotations
 
@@ -2487,6 +2487,10 @@ class Ingress(BaseCRD):
     annotations: Optional[Dict[str, str]] = None
     host: str
     ingressClassName: Optional[str] = None
+    pathPrefix: str = Field(
+        default="/",
+        description="The path prefix that will be used in the ingress. If this is explicitly set, then the\nurlPath value should be a subpath of this value.",
+    )
     tlsSecret: Optional[TlsSecret] = Field(
         default=None,
         description="The name of the TLS secret, same as what is specified in a regular Kubernetes Ingress.",
@@ -3116,7 +3120,7 @@ class Session(BaseCRD):
     )
     image: str
     port: int = Field(
-        ...,
+        default=8000,
         description="The TCP port on the pod where the session can be accessed.\nIf the session has authentication enabled then the ingress and service will point to the authentication container\nand the authentication proxy container will proxy to this port. If authentication is disabled then the ingress and service\nroute directly to this port. Note that renku reserves the highest TCP value 65535 to run the authentication proxy.",
         gt=0,
         lt=65535,
@@ -3137,7 +3141,7 @@ class Session(BaseCRD):
     storage: Storage = {}
     urlPath: str = Field(
         default="/",
-        description="The path where the session can be accessed. If an ingress is enabled then this will be\nthe path prefix for the ingress.",
+        description="The path where the session can be accessed, if an ingress is used this should be a subpath\nof the ingress.pathPrefix field. For example if the pathPrefix is /foo, this should be /foo or /foo/bar,\nbut it cannot be /baz.",
     )
     workingDir: Optional[str] = Field(
         default=None,
@@ -3278,6 +3282,10 @@ class Status(BaseCRD):
         default=None,
         description="Counts of the total and ready containers, can represent either regular or init containers.",
     )
+    error: Optional[str] = Field(
+        default=None,
+        description="If the state is failed then the message will contain information about what went wrong, otherwise it is empty",
+    )
     failingSince: Optional[datetime] = None
     hibernatedSince: Optional[datetime] = None
     idle: bool = False

components/renku_data_services/notebooks/crs.py

@@ -239,6 +239,7 @@ def as_apispec(self) -> apispec.SessionResponse:
                 total_containers=total_containers,
                 will_hibernate_at=will_hibernate_at,
                 will_delete_at=will_delete_at,
+                message=self.status.error,
             ),
             url=url,
             project_id=str(self.project_id),
@@ -256,6 +257,8 @@ def base_url(self) -> str | None:
         scheme = "https" if self.spec and self.spec.ingress and self.spec.ingress.tlsSecret else "http"
         host = self.spec.ingress.host
         path = self.spec.session.urlPath if self.spec.session.urlPath else "/"
+        if not path.endswith("/"):
+            path += "/"
         params = None
         query = None
         fragment = None