From 756146bdd416d9a20673d0371e7d585e127882d0 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Tue, 8 Oct 2024 00:30:05 +0200 Subject: [PATCH] squashme: address comments --- .../notebooks/api.spec.yaml | 2 ++ .../notebooks/api/classes/image.py | 4 ++-- .../renku_data_services/notebooks/apispec.py | 6 ++--- .../notebooks/blueprints.py | 22 ++++++++++--------- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/components/renku_data_services/notebooks/api.spec.yaml b/components/renku_data_services/notebooks/api.spec.yaml index 6f8ac977..fd5fb64f 100644 --- a/components/renku_data_services/notebooks/api.spec.yaml +++ b/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 diff --git a/components/renku_data_services/notebooks/api/classes/image.py b/components/renku_data_services/notebooks/api/classes/image.py index 565da3b5..40e6ee85 100644 --- a/components/renku_data_services/notebooks/api/classes/image.py +++ b/components/renku_data_services/notebooks/api/classes/image.py @@ -30,7 +30,7 @@ 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 beause it can happen + # 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)) @@ -65,7 +65,7 @@ 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 errors.ValidationError( - message=f"The image hostname {image.hostname} does not match " f"the image repository {self.hostname}" + message=f"The image hostname {image.hostname} does not match the image repository {self.hostname}" ) token = await self._get_docker_token(image) image_digest_url = f"https://{image.hostname}/v2/{image.name}/manifests/{image.tag}" diff --git a/components/renku_data_services/notebooks/apispec.py b/components/renku_data_services/notebooks/apispec.py index 331ad10a..00d7a394 100644 --- a/components/renku_data_services/notebooks/apispec.py +++ b/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): diff --git a/components/renku_data_services/notebooks/blueprints.py b/components/renku_data_services/notebooks/blueprints.py index 54d7b02f..eed58664 100644 --- a/components/renku_data_services/notebooks/blueprints.py +++ b/components/renku_data_services/notebooks/blueprints.py @@ -757,13 +757,14 @@ 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) @@ -1130,13 +1131,14 @@ 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 + request: Request, + user: AnonymousAPIUser | AuthenticatedAPIUser, + internal_gitlab_user: APIUser, + query: apispec.SessionsImagesGetParametersQuery, ) -> 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)