fix: docker image check endpoint and gitlab authn (#439)

Author: olevski

.devcontainer/devcontainer.json

@@ -34,7 +34,7 @@
 		"ghcr.io/devcontainers-contrib/features/poetry",
 		"ghcr.io/devcontainers-contrib/features/bash-command"
 	],
-	"postCreateCommand": "poetry install --with dev",
+	"postCreateCommand": "poetry install --with dev && mkdir -p /home/vscode/.config/k9s",
 	"customizations": {
 		"vscode": {
 			"extensions": [

.devcontainer/docker-compose.yml

@@ -25,6 +25,7 @@ services:
       POETRY_CACHE_DIR: "/poetry_cache"
       NB_SERVER_OPTIONS__DEFAULTS_PATH: /workspace/server_defaults.json
       NB_SERVER_OPTIONS__UI_CHOICES_PATH: /workspace/server_options.json
+      KUBECONFIG: "/workspace/.k3d-config.yaml"
     network_mode: service:db
     depends_on:
       - db

.gitignore

@@ -79,3 +79,5 @@ docker-compose.override.yml
 # nix
 result
 *.qcow2
+
+.k3d-config.yaml

components/renku_data_services/authn/gitlab.py

@@ -46,13 +46,14 @@ async def authenticate(self, access_token: str, request: Request) -> base_models
     async def _get_gitlab_api_user(self, access_token: str, headers: Header) -> base_models.APIUser:
         """Get and validate a Gitlab API User."""
         client = gitlab.Gitlab(self.gitlab_url, oauth_token=access_token)
-        try:
+        with suppress(gitlab.GitlabAuthenticationError):
             client.auth()  # needed for the user property to be set
-        except gitlab.GitlabAuthenticationError:
-            raise errors.UnauthorizedError(message="User not authorized with Gitlab")
+        if client.user is None:
+            # The user is not authenticated with Gitlab so we send out an empty APIUser
+            # Anonymous Renku users will not be able to authenticate with Gitlab
+            return base_models.APIUser()
+
         user = client.user
-        if user is None:
-            raise errors.UnauthorizedError(message="User not authorized with Gitlab")
 
         if user.state != "active":
             raise errors.ForbiddenError(message="User isn't active in Gitlab")

components/renku_data_services/notebooks/api.spec.yaml

@@ -19,6 +19,7 @@ paths:
           required: true
           schema:
             type: string
+            minLength: 1
         responses:
           '200':
             description: The Docker image is available.
@@ -384,6 +385,7 @@ paths:
         required: true
         schema:
           type: string
+          minLength: 1
       responses:
         "200":
           description: The docker image can be found

components/renku_data_services/notebooks/api/classes/image.py

@@ -7,10 +7,10 @@
 from pathlib import PurePosixPath
 from typing import Any, Optional, Self, cast
 
-import requests
+import httpx
 from werkzeug.datastructures import WWWAuthenticate
 
-from ...errors.user import ImageParseError
+from renku_data_services.errors import errors
 
 
 class ManifestTypes(Enum):
@@ -29,16 +29,20 @@ class ImageRepoDockerAPI:
 
     hostname: str
     oauth2_token: Optional[str] = field(default=None, repr=False)
+    # NOTE: We need to follow redirects so that we can authenticate with the image repositories properly.
+    # NOTE: If we do not use default_factory to create the client here requests will fail because it can happen
+    # that the client gets created in the wrong asyncio loop.
+    client: httpx.AsyncClient = field(default_factory=lambda: httpx.AsyncClient(timeout=10, follow_redirects=True))
 
-    def _get_docker_token(self, image: "Image") -> Optional[str]:
+    async def _get_docker_token(self, image: "Image") -> Optional[str]:
         """Get an authorization token from the docker v2 API.
 
         This will return the token provided by the API (or None if no token was found).
         """
         image_digest_url = f"https://{self.hostname}/v2/{image.name}/manifests/{image.tag}"
         try:
-            auth_req = requests.get(image_digest_url, timeout=10)
-        except requests.ConnectionError:
+            auth_req = await self.client.get(image_digest_url)
+        except httpx.ConnectError:
             auth_req = None
         if auth_req is None or not (auth_req.status_code == 401 and "Www-Authenticate" in auth_req.headers):
             # the request status code and header are not what is expected
@@ -54,56 +58,55 @@ def _get_docker_token(self, image: "Image") -> Optional[str]:
         if self.oauth2_token:
             creds = base64.urlsafe_b64encode(f"oauth2:{self.oauth2_token}".encode()).decode()
             headers["Authorization"] = f"Basic {creds}"
-        token_req = requests.get(realm, params=params, headers=headers, timeout=10)
+        token_req = await self.client.get(realm, params=params, headers=headers)
         return str(token_req.json().get("token"))
 
-    def get_image_manifest(self, image: "Image") -> Optional[dict[str, Any]]:
+    async def get_image_manifest(self, image: "Image") -> Optional[dict[str, Any]]:
         """Query the docker API to get the manifest of an image."""
         if image.hostname != self.hostname:
-            raise ImageParseError(
-                f"The image hostname {image.hostname} does not match " f"the image repository {self.hostname}"
+            raise errors.ValidationError(
+                message=f"The image hostname {image.hostname} does not match the image repository {self.hostname}"
             )
-        token = self._get_docker_token(image)
+        token = await self._get_docker_token(image)
         image_digest_url = f"https://{image.hostname}/v2/{image.name}/manifests/{image.tag}"
         headers = {"Accept": ManifestTypes.docker_v2.value}
         if token:
             headers["Authorization"] = f"Bearer {token}"
-        res = requests.get(image_digest_url, headers=headers, timeout=10)
+        res = await self.client.get(image_digest_url, headers=headers)
         if res.status_code != 200:
             headers["Accept"] = ManifestTypes.oci_v1.value
-            res = requests.get(image_digest_url, headers=headers, timeout=10)
+            res = await self.client.get(image_digest_url, headers=headers)
         if res.status_code != 200:
             return None
         return cast(dict[str, Any], res.json())
 
-    def image_exists(self, image: "Image") -> bool:
+    async def image_exists(self, image: "Image") -> bool:
         """Check the docker repo API if the image exists."""
-        return self.get_image_manifest(image) is not None
+        return await self.get_image_manifest(image) is not None
 
-    def get_image_config(self, image: "Image") -> Optional[dict[str, Any]]:
+    async def get_image_config(self, image: "Image") -> Optional[dict[str, Any]]:
         """Query the docker API to get the configuration of an image."""
-        manifest = self.get_image_manifest(image)
+        manifest = await self.get_image_manifest(image)
         if manifest is None:
             return None
         config_digest = manifest.get("config", {}).get("digest")
         if config_digest is None:
             return None
-        token = self._get_docker_token(image)
-        res = requests.get(
+        token = await self._get_docker_token(image)
+        res = await self.client.get(
             f"https://{image.hostname}/v2/{image.name}/blobs/{config_digest}",
             headers={
                 "Accept": "application/json",
                 "Authorization": f"Bearer {token}",
             },
-            timeout=10,
         )
         if res.status_code != 200:
             return None
         return cast(dict[str, Any], res.json())
 
-    def image_workdir(self, image: "Image") -> Optional[PurePosixPath]:
+    async def image_workdir(self, image: "Image") -> Optional[PurePosixPath]:
         """Query the docker API to get the workdir of an image."""
-        config = self.get_image_config(image)
+        config = await self.get_image_config(image)
         if config is None:
             return None
         nested_config = config.get("config", {})
@@ -204,9 +207,9 @@ def build_re(*parts: str) -> re.Pattern:
         if len(matches) == 1:
             return cls(matches[0]["hostname"], matches[0]["image"], matches[0]["tag"])
         elif len(matches) > 1:
-            raise ImageParseError(f"Cannot parse the image {path}, too many interpretations {matches}")
+            raise errors.ValidationError(message=f"Cannot parse the image {path}, too many interpretations {matches}")
         else:
-            raise ImageParseError(f"Cannot parse the image {path}")
+            raise errors.ValidationError(message=f"Cannot parse the image {path}")
 
     def repo_api(self) -> ImageRepoDockerAPI:
         """Get the docker API from the image."""

components/renku_data_services/notebooks/apispec.py

@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 #   filename:  api.spec.yaml
-#   timestamp: 2024-09-24T09:26:37+00:00
+#   timestamp: 2024-10-07T22:25:48+00:00
 
 from __future__ import annotations
 
@@ -273,7 +273,7 @@ class SessionCloudStoragePost(BaseAPISpec):
 
 
 class NotebooksImagesGetParametersQuery(BaseAPISpec):
-    image_url: str
+    image_url: str = Field(..., min_length=1)
 
 
 class NotebooksLogsServerNameGetParametersQuery(BaseAPISpec):
@@ -296,7 +296,7 @@ class SessionsSessionIdLogsGetParametersQuery(BaseAPISpec):
 
 
 class SessionsImagesGetParametersQuery(BaseAPISpec):
-    image_url: str
+    image_url: str = Field(..., min_length=1)
 
 
 class LaunchNotebookRequest(BaseAPISpec):

components/renku_data_services/notebooks/blueprints.py

@@ -315,15 +315,15 @@ async def launch_notebook_helper(
             # A specific image was requested
             parsed_image = Image.from_path(image)
             image_repo = parsed_image.repo_api()
-            image_exists_publicly = image_repo.image_exists(parsed_image)
+            image_exists_publicly = await image_repo.image_exists(parsed_image)
             image_exists_privately = False
             if (
                 not image_exists_publicly
                 and parsed_image.hostname == nb_config.git.registry
                 and internal_gitlab_user.access_token
             ):
                 image_repo = image_repo.with_oauth2_token(internal_gitlab_user.access_token)
-                image_exists_privately = image_repo.image_exists(parsed_image)
+                image_exists_privately = await image_repo.image_exists(parsed_image)
             if not image_exists_privately and not image_exists_publicly:
                 using_default_image = True
                 image = nb_config.sessions.default_image
@@ -349,7 +349,7 @@ async def launch_notebook_helper(
             image_repo = parsed_image.repo_api()
             if is_image_private and internal_gitlab_user.access_token:
                 image_repo = image_repo.with_oauth2_token(internal_gitlab_user.access_token)
-            if not image_repo.image_exists(parsed_image):
+            if not await image_repo.image_exists(parsed_image):
                 raise errors.MissingResourceError(
                     message=(
                         f"Cannot start the session because the following the image {image} does not "
@@ -413,7 +413,7 @@ async def launch_notebook_helper(
         if lfs_auto_fetch is not None:
             parsed_server_options.lfs_auto_fetch = lfs_auto_fetch
 
-        image_work_dir = image_repo.image_workdir(parsed_image) or PurePosixPath("/")
+        image_work_dir = await image_repo.image_workdir(parsed_image) or PurePosixPath("/")
         mount_path = image_work_dir / "work"
 
         server_work_dir = mount_path / gl_project_path
@@ -757,17 +757,18 @@ def check_docker_image(self) -> BlueprintFactoryResponse:
         """Return the availability of the docker image."""
 
         @authenticate_2(self.authenticator, self.internal_gitlab_authenticator)
+        @validate(query=apispec.NotebooksImagesGetParametersQuery)
         async def _check_docker_image(
-            request: Request, user: AnonymousAPIUser | AuthenticatedAPIUser, internal_gitlab_user: APIUser
+            request: Request,
+            user: AnonymousAPIUser | AuthenticatedAPIUser,
+            internal_gitlab_user: APIUser,
+            query: apispec.NotebooksImagesGetParametersQuery,
         ) -> HTTPResponse:
-            image_url = request.get_args().get("image_url")
-            if not isinstance(image_url, str):
-                raise ValueError("required string of image url")
-            parsed_image = Image.from_path(image_url)
+            parsed_image = Image.from_path(query.image_url)
             image_repo = parsed_image.repo_api()
             if parsed_image.hostname == self.nb_config.git.registry and internal_gitlab_user.access_token:
                 image_repo = image_repo.with_oauth2_token(internal_gitlab_user.access_token)
-            if image_repo.image_exists(parsed_image):
+            if await image_repo.image_exists(parsed_image):
                 return HTTPResponse(status=200)
             else:
                 return HTTPResponse(status=404)
@@ -1125,3 +1126,25 @@ async def _handler(
             return json(apispec.SessionLogsResponse.model_validate(logs).model_dump(exclude_none=True))
 
         return "/sessions/<session_id>/logs", ["GET"], _handler
+
+    def check_docker_image(self) -> BlueprintFactoryResponse:
+        """Return the availability of the docker image."""
+
+        @authenticate_2(self.authenticator, self.internal_gitlab_authenticator)
+        @validate(query=apispec.SessionsImagesGetParametersQuery)
+        async def _check_docker_image(
+            request: Request,
+            user: AnonymousAPIUser | AuthenticatedAPIUser,
+            internal_gitlab_user: APIUser,
+            query: apispec.SessionsImagesGetParametersQuery,
+        ) -> HTTPResponse:
+            parsed_image = Image.from_path(query.image_url)
+            image_repo = parsed_image.repo_api()
+            if parsed_image.hostname == self.nb_config.git.registry and internal_gitlab_user.access_token:
+                image_repo = image_repo.with_oauth2_token(internal_gitlab_user.access_token)
+            if await image_repo.image_exists(parsed_image):
+                return HTTPResponse(status=200)
+            else:
+                return HTTPResponse(status=404)
+
+        return "/sessions/images", ["GET"], _check_docker_image

pyproject.toml

@@ -150,7 +150,7 @@ ignore = [
 "components/renku_data_services/notebooks/crs.py" = ["F401"]
 
 [tool.ruff.lint.isort]
-known-first-party = ["renku_data_services"]
+known-first-party = ["renku_data_services", "test"]
 
 [tool.ruff.lint.pydocstyle]
 convention = "google"

test/bases/renku_data_services/data_api/test_notebooks.py

@@ -3,7 +3,7 @@
 import asyncio
 import os
 import shutil
-from collections.abc import AsyncIterator
+from collections.abc import AsyncIterator, Iterator
 from unittest.mock import MagicMock
 from uuid import uuid4
 
@@ -13,14 +13,12 @@
 from sanic_testing.testing import SanicASGITestClient
 
 from renku_data_services.notebooks.api.classes.k8s_client import JupyterServerV1Alpha1Kr8s
-
-from .utils import K3DCluster, setup_amalthea
-
-os.environ["KUBECONFIG"] = ".k3d-config.yaml"
+from test.bases.renku_data_services.data_api.utils import K3DCluster, setup_amalthea
 
 
 @pytest.fixture(scope="module", autouse=True)
-def cluster() -> K3DCluster:
+def cluster() -> Iterator[K3DCluster]:
+    os.environ["KUBECONFIG"] = ".k3d-config.yaml"
     if shutil.which("k3d") is None:
         pytest.skip("Requires k3d for cluster creation")