From 4d4f9275852c9c5e2039c97f476773194214ca9e Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Mon, 23 Sep 2024 13:52:30 +0200 Subject: [PATCH 01/10] squashme: changes from PR conflicts resolution --- .../notebooks/api.spec.yaml | 1 - .../renku_data_services/notebooks/apispec.py | 18 ++++++++---------- poetry.lock | 6 +++--- projects/renku_data_service/poetry.lock | 6 +++--- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/components/renku_data_services/notebooks/api.spec.yaml b/components/renku_data_services/notebooks/api.spec.yaml index 6f8ac9774..66aae73a0 100644 --- a/components/renku_data_services/notebooks/api.spec.yaml +++ b/components/renku_data_services/notebooks/api.spec.yaml @@ -439,7 +439,6 @@ components: - registered type: object ErrorResponse: - type: object properties: error: type: object diff --git a/components/renku_data_services/notebooks/apispec.py b/components/renku_data_services/notebooks/apispec.py index 331ad10a5..18447f9ea 100644 --- a/components/renku_data_services/notebooks/apispec.py +++ b/components/renku_data_services/notebooks/apispec.py @@ -34,16 +34,10 @@ class DefaultCullingThresholds(BaseAPISpec): registered: CullingThreshold -class Error(BaseAPISpec): - code: int = Field(..., example=1404, gt=0) - detail: Optional[str] = Field( - None, example="A more detailed optional message showing what the problem was" - ) - message: str = Field(..., example="Something went wrong - please try again later") - - -class ErrorResponse(BaseAPISpec): - error: Error +class ErrorResponseNested(BaseAPISpec): + code: int + detail: Optional[str] = None + message: str class Generated(BaseAPISpec): @@ -299,6 +293,10 @@ class SessionsImagesGetParametersQuery(BaseAPISpec): image_url: str +class ErrorResponse(BaseAPISpec): + error: ErrorResponseNested + + class LaunchNotebookRequest(BaseAPISpec): project_id: str launcher_id: str diff --git a/poetry.lock b/poetry.lock index 4bd733d51..dd2866918 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3748,13 +3748,13 @@ pbr = ">=2.0.0,<2.1.0 || >2.1.0" [[package]] name = "tenacity" -version = "9.0.0" +version = "8.5.0" description = "Retry code until it succeeds" optional = false python-versions = ">=3.8" files = [ - {file = "tenacity-9.0.0-py3-none-any.whl", hash = "sha256:93de0c98785b27fcf659856aa9f54bfbd399e29969b0621bc7f762bd441b4539"}, - {file = "tenacity-9.0.0.tar.gz", hash = "sha256:807f37ca97d62aa361264d497b0e31e92b8027044942bfa756160d908320d73b"}, + {file = "tenacity-8.5.0-py3-none-any.whl", hash = "sha256:b594c2a5945830c267ce6b79a166228323ed52718f30302c1359836112346687"}, + {file = "tenacity-8.5.0.tar.gz", hash = "sha256:8bc6c0c8a09b31e6cad13c47afbed1a567518250a9a171418582ed8d9c20ca78"}, ] [package.extras] diff --git a/projects/renku_data_service/poetry.lock b/projects/renku_data_service/poetry.lock index 809edf0bd..6f7ddafad 100644 --- a/projects/renku_data_service/poetry.lock +++ b/projects/renku_data_service/poetry.lock @@ -2785,13 +2785,13 @@ sqlcipher = ["sqlcipher3_binary"] [[package]] name = "tenacity" -version = "9.0.0" +version = "8.5.0" description = "Retry code until it succeeds" optional = false python-versions = ">=3.8" files = [ - {file = "tenacity-9.0.0-py3-none-any.whl", hash = "sha256:93de0c98785b27fcf659856aa9f54bfbd399e29969b0621bc7f762bd441b4539"}, - {file = "tenacity-9.0.0.tar.gz", hash = "sha256:807f37ca97d62aa361264d497b0e31e92b8027044942bfa756160d908320d73b"}, + {file = "tenacity-8.5.0-py3-none-any.whl", hash = "sha256:b594c2a5945830c267ce6b79a166228323ed52718f30302c1359836112346687"}, + {file = "tenacity-8.5.0.tar.gz", hash = "sha256:8bc6c0c8a09b31e6cad13c47afbed1a567518250a9a171418582ed8d9c20ca78"}, ] [package.extras] From e6900d71e54e9e99f9375f85205effe9fc17a4e4 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 4 Sep 2024 23:39:01 +0200 Subject: [PATCH 02/10] feat: add cloud storage via rclone squashme: use the amalthea session cache --- .../notebooks/api.spec.yaml | 1 + .../renku_data_services/notebooks/apispec.py | 18 ++++++++++-------- poetry.lock | 6 +++--- projects/renku_data_service/poetry.lock | 6 +++--- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/components/renku_data_services/notebooks/api.spec.yaml b/components/renku_data_services/notebooks/api.spec.yaml index 66aae73a0..6f8ac9774 100644 --- a/components/renku_data_services/notebooks/api.spec.yaml +++ b/components/renku_data_services/notebooks/api.spec.yaml @@ -439,6 +439,7 @@ components: - registered type: object ErrorResponse: + type: object properties: error: type: object diff --git a/components/renku_data_services/notebooks/apispec.py b/components/renku_data_services/notebooks/apispec.py index 18447f9ea..331ad10a5 100644 --- a/components/renku_data_services/notebooks/apispec.py +++ b/components/renku_data_services/notebooks/apispec.py @@ -34,10 +34,16 @@ class DefaultCullingThresholds(BaseAPISpec): registered: CullingThreshold -class ErrorResponseNested(BaseAPISpec): - code: int - detail: Optional[str] = None - message: str +class Error(BaseAPISpec): + code: int = Field(..., example=1404, gt=0) + detail: Optional[str] = Field( + None, example="A more detailed optional message showing what the problem was" + ) + message: str = Field(..., example="Something went wrong - please try again later") + + +class ErrorResponse(BaseAPISpec): + error: Error class Generated(BaseAPISpec): @@ -293,10 +299,6 @@ class SessionsImagesGetParametersQuery(BaseAPISpec): image_url: str -class ErrorResponse(BaseAPISpec): - error: ErrorResponseNested - - class LaunchNotebookRequest(BaseAPISpec): project_id: str launcher_id: str diff --git a/poetry.lock b/poetry.lock index dd2866918..4bd733d51 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3748,13 +3748,13 @@ pbr = ">=2.0.0,<2.1.0 || >2.1.0" [[package]] name = "tenacity" -version = "8.5.0" +version = "9.0.0" description = "Retry code until it succeeds" optional = false python-versions = ">=3.8" files = [ - {file = "tenacity-8.5.0-py3-none-any.whl", hash = "sha256:b594c2a5945830c267ce6b79a166228323ed52718f30302c1359836112346687"}, - {file = "tenacity-8.5.0.tar.gz", hash = "sha256:8bc6c0c8a09b31e6cad13c47afbed1a567518250a9a171418582ed8d9c20ca78"}, + {file = "tenacity-9.0.0-py3-none-any.whl", hash = "sha256:93de0c98785b27fcf659856aa9f54bfbd399e29969b0621bc7f762bd441b4539"}, + {file = "tenacity-9.0.0.tar.gz", hash = "sha256:807f37ca97d62aa361264d497b0e31e92b8027044942bfa756160d908320d73b"}, ] [package.extras] diff --git a/projects/renku_data_service/poetry.lock b/projects/renku_data_service/poetry.lock index 6f7ddafad..809edf0bd 100644 --- a/projects/renku_data_service/poetry.lock +++ b/projects/renku_data_service/poetry.lock @@ -2785,13 +2785,13 @@ sqlcipher = ["sqlcipher3_binary"] [[package]] name = "tenacity" -version = "8.5.0" +version = "9.0.0" description = "Retry code until it succeeds" optional = false python-versions = ">=3.8" files = [ - {file = "tenacity-8.5.0-py3-none-any.whl", hash = "sha256:b594c2a5945830c267ce6b79a166228323ed52718f30302c1359836112346687"}, - {file = "tenacity-8.5.0.tar.gz", hash = "sha256:8bc6c0c8a09b31e6cad13c47afbed1a567518250a9a171418582ed8d9c20ca78"}, + {file = "tenacity-9.0.0-py3-none-any.whl", hash = "sha256:93de0c98785b27fcf659856aa9f54bfbd399e29969b0621bc7f762bd441b4539"}, + {file = "tenacity-9.0.0.tar.gz", hash = "sha256:807f37ca97d62aa361264d497b0e31e92b8027044942bfa756160d908320d73b"}, ] [package.extras] From f477be9b3a2a1b334b228d587891cd7a8ea81be2 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 4 Sep 2024 23:39:01 +0200 Subject: [PATCH 03/10] squashme: changes from PR conflict resolution --- components/renku_data_services/authn/dummy.py | 2 +- components/renku_data_services/base_api/auth.py | 3 ++- .../renku_data_services/notebooks/util/kubernetes_.py | 2 ++ .../renku_data_services/authz/test_authorization.py | 8 ++++---- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/components/renku_data_services/authn/dummy.py b/components/renku_data_services/authn/dummy.py index d10acedb0..397a95367 100644 --- a/components/renku_data_services/authn/dummy.py +++ b/components/renku_data_services/authn/dummy.py @@ -70,7 +70,7 @@ async def authenticate(self, access_token: str, request: Request) -> base_models ) return base_models.APIUser( - is_admin=user_props.get("is_admin", False), + is_admin_init=user_props.get("is_admin", False), id=user_props.get("id", "some-id") if is_set else None, access_token=access_token, first_name=user_props.get("first_name", "John") if is_set else None, diff --git a/components/renku_data_services/base_api/auth.py b/components/renku_data_services/base_api/auth.py index 16b76b09d..34a57ae46 100644 --- a/components/renku_data_services/base_api/auth.py +++ b/components/renku_data_services/base_api/auth.py @@ -6,7 +6,8 @@ from functools import wraps from typing import Any, Concatenate, ParamSpec, TypeVar, cast -from sanic import Request +from sanic import HTTPResponse, Request +from ulid import ULID from renku_data_services import errors from renku_data_services.base_models import AnyAPIUser, APIUser, Authenticator diff --git a/components/renku_data_services/notebooks/util/kubernetes_.py b/components/renku_data_services/notebooks/util/kubernetes_.py index 7cf289c95..b4a97e706 100644 --- a/components/renku_data_services/notebooks/util/kubernetes_.py +++ b/components/renku_data_services/notebooks/util/kubernetes_.py @@ -27,6 +27,8 @@ from renku_data_services.notebooks.crs import Patch, PatchType +from renku_data_services.notebooks.crs import Patch, PatchType + def renku_1_make_server_name(safe_username: str, namespace: str, project: str, branch: str, commit_sha: str) -> str: """Form a unique server name for Renku 1.0 sessions. diff --git a/test/components/renku_data_services/authz/test_authorization.py b/test/components/renku_data_services/authz/test_authorization.py index a70987ba1..90564c41d 100644 --- a/test/components/renku_data_services/authz/test_authorization.py +++ b/test/components/renku_data_services/authz/test_authorization.py @@ -17,10 +17,10 @@ from renku_data_services.namespace.models import Namespace, NamespaceKind from renku_data_services.project.models import Project -admin_user = APIUser(is_admin=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 -anon_user = APIUser(is_admin=False) -regular_user1 = APIUser(is_admin=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 -regular_user2 = APIUser(is_admin=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 +admin_user = APIUser(is_admin_init=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 +anon_user = APIUser(is_admin_init=False) +regular_user1 = APIUser(is_admin_init=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 +regular_user2 = APIUser(is_admin_init=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 @pytest_asyncio.fixture From 93e364b9ba3f4c1ead45f545ec2dfcceb023220b Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Tue, 17 Sep 2024 17:14:01 +0200 Subject: [PATCH 04/10] feat: use the amalthea session cache --- components/renku_data_services/authn/dummy.py | 2 +- components/renku_data_services/base_api/auth.py | 3 +-- .../renku_data_services/notebooks/util/kubernetes_.py | 2 -- .../renku_data_services/authz/test_authorization.py | 8 ++++---- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/renku_data_services/authn/dummy.py b/components/renku_data_services/authn/dummy.py index 397a95367..d10acedb0 100644 --- a/components/renku_data_services/authn/dummy.py +++ b/components/renku_data_services/authn/dummy.py @@ -70,7 +70,7 @@ async def authenticate(self, access_token: str, request: Request) -> base_models ) return base_models.APIUser( - is_admin_init=user_props.get("is_admin", False), + is_admin=user_props.get("is_admin", False), id=user_props.get("id", "some-id") if is_set else None, access_token=access_token, first_name=user_props.get("first_name", "John") if is_set else None, diff --git a/components/renku_data_services/base_api/auth.py b/components/renku_data_services/base_api/auth.py index 34a57ae46..16b76b09d 100644 --- a/components/renku_data_services/base_api/auth.py +++ b/components/renku_data_services/base_api/auth.py @@ -6,8 +6,7 @@ from functools import wraps from typing import Any, Concatenate, ParamSpec, TypeVar, cast -from sanic import HTTPResponse, Request -from ulid import ULID +from sanic import Request from renku_data_services import errors from renku_data_services.base_models import AnyAPIUser, APIUser, Authenticator diff --git a/components/renku_data_services/notebooks/util/kubernetes_.py b/components/renku_data_services/notebooks/util/kubernetes_.py index b4a97e706..7cf289c95 100644 --- a/components/renku_data_services/notebooks/util/kubernetes_.py +++ b/components/renku_data_services/notebooks/util/kubernetes_.py @@ -27,8 +27,6 @@ from renku_data_services.notebooks.crs import Patch, PatchType -from renku_data_services.notebooks.crs import Patch, PatchType - def renku_1_make_server_name(safe_username: str, namespace: str, project: str, branch: str, commit_sha: str) -> str: """Form a unique server name for Renku 1.0 sessions. diff --git a/test/components/renku_data_services/authz/test_authorization.py b/test/components/renku_data_services/authz/test_authorization.py index 90564c41d..a70987ba1 100644 --- a/test/components/renku_data_services/authz/test_authorization.py +++ b/test/components/renku_data_services/authz/test_authorization.py @@ -17,10 +17,10 @@ from renku_data_services.namespace.models import Namespace, NamespaceKind from renku_data_services.project.models import Project -admin_user = APIUser(is_admin_init=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 -anon_user = APIUser(is_admin_init=False) -regular_user1 = APIUser(is_admin_init=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 -regular_user2 = APIUser(is_admin_init=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 +admin_user = APIUser(is_admin=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 +anon_user = APIUser(is_admin=False) +regular_user1 = APIUser(is_admin=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 +regular_user2 = APIUser(is_admin=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 @pytest_asyncio.fixture From 3f9b99693bacee67b95bd483a1be3a518a3a72a6 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 4 Sep 2024 23:39:01 +0200 Subject: [PATCH 05/10] squashme: changes from PR conflict resolution --- components/renku_data_services/authn/dummy.py | 2 +- components/renku_data_services/base_api/auth.py | 3 ++- .../renku_data_services/notebooks/util/kubernetes_.py | 2 ++ .../renku_data_services/authz/test_authorization.py | 8 ++++---- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/components/renku_data_services/authn/dummy.py b/components/renku_data_services/authn/dummy.py index d10acedb0..397a95367 100644 --- a/components/renku_data_services/authn/dummy.py +++ b/components/renku_data_services/authn/dummy.py @@ -70,7 +70,7 @@ async def authenticate(self, access_token: str, request: Request) -> base_models ) return base_models.APIUser( - is_admin=user_props.get("is_admin", False), + is_admin_init=user_props.get("is_admin", False), id=user_props.get("id", "some-id") if is_set else None, access_token=access_token, first_name=user_props.get("first_name", "John") if is_set else None, diff --git a/components/renku_data_services/base_api/auth.py b/components/renku_data_services/base_api/auth.py index 16b76b09d..34a57ae46 100644 --- a/components/renku_data_services/base_api/auth.py +++ b/components/renku_data_services/base_api/auth.py @@ -6,7 +6,8 @@ from functools import wraps from typing import Any, Concatenate, ParamSpec, TypeVar, cast -from sanic import Request +from sanic import HTTPResponse, Request +from ulid import ULID from renku_data_services import errors from renku_data_services.base_models import AnyAPIUser, APIUser, Authenticator diff --git a/components/renku_data_services/notebooks/util/kubernetes_.py b/components/renku_data_services/notebooks/util/kubernetes_.py index 7cf289c95..b4a97e706 100644 --- a/components/renku_data_services/notebooks/util/kubernetes_.py +++ b/components/renku_data_services/notebooks/util/kubernetes_.py @@ -27,6 +27,8 @@ from renku_data_services.notebooks.crs import Patch, PatchType +from renku_data_services.notebooks.crs import Patch, PatchType + def renku_1_make_server_name(safe_username: str, namespace: str, project: str, branch: str, commit_sha: str) -> str: """Form a unique server name for Renku 1.0 sessions. diff --git a/test/components/renku_data_services/authz/test_authorization.py b/test/components/renku_data_services/authz/test_authorization.py index a70987ba1..90564c41d 100644 --- a/test/components/renku_data_services/authz/test_authorization.py +++ b/test/components/renku_data_services/authz/test_authorization.py @@ -17,10 +17,10 @@ from renku_data_services.namespace.models import Namespace, NamespaceKind from renku_data_services.project.models import Project -admin_user = APIUser(is_admin=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 -anon_user = APIUser(is_admin=False) -regular_user1 = APIUser(is_admin=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 -regular_user2 = APIUser(is_admin=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 +admin_user = APIUser(is_admin_init=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 +anon_user = APIUser(is_admin_init=False) +regular_user1 = APIUser(is_admin_init=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 +regular_user2 = APIUser(is_admin_init=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 @pytest_asyncio.fixture From 9778a5149318f414c92e63a8c77afbd822b6f230 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Tue, 17 Sep 2024 17:14:01 +0200 Subject: [PATCH 06/10] feat: use the amalthea session cache --- components/renku_data_services/authn/dummy.py | 2 +- components/renku_data_services/base_api/auth.py | 3 +-- .../renku_data_services/notebooks/util/kubernetes_.py | 2 -- .../renku_data_services/authz/test_authorization.py | 8 ++++---- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/renku_data_services/authn/dummy.py b/components/renku_data_services/authn/dummy.py index 397a95367..d10acedb0 100644 --- a/components/renku_data_services/authn/dummy.py +++ b/components/renku_data_services/authn/dummy.py @@ -70,7 +70,7 @@ async def authenticate(self, access_token: str, request: Request) -> base_models ) return base_models.APIUser( - is_admin_init=user_props.get("is_admin", False), + is_admin=user_props.get("is_admin", False), id=user_props.get("id", "some-id") if is_set else None, access_token=access_token, first_name=user_props.get("first_name", "John") if is_set else None, diff --git a/components/renku_data_services/base_api/auth.py b/components/renku_data_services/base_api/auth.py index 34a57ae46..16b76b09d 100644 --- a/components/renku_data_services/base_api/auth.py +++ b/components/renku_data_services/base_api/auth.py @@ -6,8 +6,7 @@ from functools import wraps from typing import Any, Concatenate, ParamSpec, TypeVar, cast -from sanic import HTTPResponse, Request -from ulid import ULID +from sanic import Request from renku_data_services import errors from renku_data_services.base_models import AnyAPIUser, APIUser, Authenticator diff --git a/components/renku_data_services/notebooks/util/kubernetes_.py b/components/renku_data_services/notebooks/util/kubernetes_.py index b4a97e706..7cf289c95 100644 --- a/components/renku_data_services/notebooks/util/kubernetes_.py +++ b/components/renku_data_services/notebooks/util/kubernetes_.py @@ -27,8 +27,6 @@ from renku_data_services.notebooks.crs import Patch, PatchType -from renku_data_services.notebooks.crs import Patch, PatchType - def renku_1_make_server_name(safe_username: str, namespace: str, project: str, branch: str, commit_sha: str) -> str: """Form a unique server name for Renku 1.0 sessions. diff --git a/test/components/renku_data_services/authz/test_authorization.py b/test/components/renku_data_services/authz/test_authorization.py index 90564c41d..a70987ba1 100644 --- a/test/components/renku_data_services/authz/test_authorization.py +++ b/test/components/renku_data_services/authz/test_authorization.py @@ -17,10 +17,10 @@ from renku_data_services.namespace.models import Namespace, NamespaceKind from renku_data_services.project.models import Project -admin_user = APIUser(is_admin_init=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 -anon_user = APIUser(is_admin_init=False) -regular_user1 = APIUser(is_admin_init=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 -regular_user2 = APIUser(is_admin_init=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 +admin_user = APIUser(is_admin=True, id="admin-id", access_token="some-token", full_name="admin") # nosec B106 +anon_user = APIUser(is_admin=False) +regular_user1 = APIUser(is_admin=False, id="user1-id", access_token="some-token1", full_name="some-user1") # nosec B106 +regular_user2 = APIUser(is_admin=False, id="user2-id", access_token="some-token2", full_name="some-user2") # nosec B106 @pytest_asyncio.fixture From a920d1d5aa19651086507fe7dab7f1f47c074928 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Fri, 4 Oct 2024 11:39:33 +0200 Subject: [PATCH 07/10] fix: docker image check endpoint and gitlab authn --- .../renku_data_services/authn/gitlab.py | 11 +++-- .../notebooks/api/classes/image.py | 47 ++++++++++--------- .../notebooks/blueprints.py | 31 ++++++++++-- 3 files changed, 56 insertions(+), 33 deletions(-) diff --git a/components/renku_data_services/authn/gitlab.py b/components/renku_data_services/authn/gitlab.py index 2a7d4cc83..9c3686f10 100644 --- a/components/renku_data_services/authn/gitlab.py +++ b/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") diff --git a/components/renku_data_services/notebooks/api/classes/image.py b/components/renku_data_services/notebooks/api/classes/image.py index 6ced400eb..1f0b6ec46 100644 --- a/components/renku_data_services/notebooks/api/classes/image.py +++ b/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,18 @@ 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. + client: httpx.AsyncClient = 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 +56,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 " f"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 +205,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.""" diff --git a/components/renku_data_services/notebooks/blueprints.py b/components/renku_data_services/notebooks/blueprints.py index c0ef15abb..54d7b02f1 100644 --- a/components/renku_data_services/notebooks/blueprints.py +++ b/components/renku_data_services/notebooks/blueprints.py @@ -315,7 +315,7 @@ 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 @@ -323,7 +323,7 @@ async def launch_notebook_helper( 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 @@ -767,7 +767,7 @@ async def _check_docker_image( 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 +1125,24 @@ async def _handler( return json(apispec.SessionLogsResponse.model_validate(logs).model_dump(exclude_none=True)) return "/sessions//logs", ["GET"], _handler + + def check_docker_image(self) -> BlueprintFactoryResponse: + """Return the availability of the docker image.""" + + @authenticate_2(self.authenticator, self.internal_gitlab_authenticator) + async def _check_docker_image( + request: Request, user: AnonymousAPIUser | AuthenticatedAPIUser, internal_gitlab_user: APIUser + ) -> 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) + 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 From 8acb72023fcceff2c99b74d65ad045a689aba746 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Mon, 7 Oct 2024 14:20:27 +0200 Subject: [PATCH 08/10] chore: fix tests --- .devcontainer/devcontainer.json | 2 +- .devcontainer/docker-compose.yml | 1 + .gitignore | 2 ++ .../renku_data_services/notebooks/api/classes/image.py | 4 +++- pyproject.toml | 2 +- .../renku_data_services/data_api/test_notebooks.py | 10 ++++------ .../renku_data_services/data_api/test_sessions.py | 6 ++---- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 9fffd227c..189ef000b 100644 --- a/.devcontainer/devcontainer.json +++ b/.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": [ diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index 1dc6a27a1..65c8ca4c0 100644 --- a/.devcontainer/docker-compose.yml +++ b/.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 diff --git a/.gitignore b/.gitignore index c8729b4e3..ea704bfe3 100644 --- a/.gitignore +++ b/.gitignore @@ -79,3 +79,5 @@ docker-compose.override.yml # nix result *.qcow2 + +.k3d-config.yaml diff --git a/components/renku_data_services/notebooks/api/classes/image.py b/components/renku_data_services/notebooks/api/classes/image.py index 1f0b6ec46..565da3b51 100644 --- a/components/renku_data_services/notebooks/api/classes/image.py +++ b/components/renku_data_services/notebooks/api/classes/image.py @@ -30,7 +30,9 @@ 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. - client: httpx.AsyncClient = httpx.AsyncClient(timeout=10, follow_redirects=True) + # NOTE: If we do not use default_factory to create the client here requests will fail beause 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)) async def _get_docker_token(self, image: "Image") -> Optional[str]: """Get an authorization token from the docker v2 API. diff --git a/pyproject.toml b/pyproject.toml index 52879d7e9..73ac3bcfd 100644 --- a/pyproject.toml +++ b/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" diff --git a/test/bases/renku_data_services/data_api/test_notebooks.py b/test/bases/renku_data_services/data_api/test_notebooks.py index e9e973a8a..a0da0f34e 100644 --- a/test/bases/renku_data_services/data_api/test_notebooks.py +++ b/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") diff --git a/test/bases/renku_data_services/data_api/test_sessions.py b/test/bases/renku_data_services/data_api/test_sessions.py index 2b903bb73..3eb312ec4 100644 --- a/test/bases/renku_data_services/data_api/test_sessions.py +++ b/test/bases/renku_data_services/data_api/test_sessions.py @@ -13,14 +13,12 @@ from renku_data_services.app_config.config import Config from renku_data_services.crc.apispec import ResourcePool from renku_data_services.users.models import UserInfo - -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") def cluster() -> Iterator[K3DCluster]: + os.environ["KUBECONFIG"] = ".k3d-config.yaml" if shutil.which("k3d") is None: pytest.skip("Requires k3d for cluster creation") From 1537547c33641fd051f55bce65a16bb37650ca19 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Tue, 8 Oct 2024 00:30:05 +0200 Subject: [PATCH 09/10] 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 6f8ac9774..fd5fb64fd 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 565da3b51..40e6ee850 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 331ad10a5..00d7a3948 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 54d7b02f1..eed586647 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) From b2b99825288d75ddfb5fe1022c512179fb5bc578 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Fri, 4 Oct 2024 14:55:00 +0200 Subject: [PATCH 10/10] fix: add hibernation and deletion time in status --- .../renku_data_services/notebooks/crs.py | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/components/renku_data_services/notebooks/crs.py b/components/renku_data_services/notebooks/crs.py index 1a548ff75..8d86ed9ad 100644 --- a/components/renku_data_services/notebooks/crs.py +++ b/components/renku_data_services/notebooks/crs.py @@ -1,10 +1,10 @@ """Custom resource definition with proper names from the autogenerated code.""" -from datetime import datetime +from datetime import UTC, datetime from typing import Any, cast from urllib.parse import urljoin, urlparse, urlunparse -from kubernetes.utils import parse_quantity +from kubernetes.utils import parse_duration, parse_quantity from pydantic import BaseModel, Field, field_validator from sanic.log import logger from ulid import ULID @@ -24,6 +24,7 @@ SecretRef, Session, State, + Status, Storage, TlsSecret, ) @@ -190,6 +191,30 @@ def as_apispec(self) -> apispec.SessionResponse: else: state = apispec.State3.starting + will_hibernate_at: datetime | None = None + will_delete_at: datetime | None = None + match self.status, self.spec.culling: + case ( + Status(idle=True, idleSince=idle_since), + Culling(maxIdleDuration=max_idle), + ) if idle_since and max_idle: + will_hibernate_at = idle_since + parse_duration(max_idle) + case ( + Status(state=State.Failed, failingSince=failing_since), + Culling(maxFailedDuration=max_failed), + ) if failing_since and max_failed: + will_hibernate_at = failing_since + parse_duration(max_failed) + case ( + Status(state=State.NotReady), + Culling(maxAge=max_age), + ) if max_age and self.metadata.creationTimestamp: + will_hibernate_at = self.metadata.creationTimestamp + parse_duration(max_age) + case ( + Status(state=State.Hibernated, hibernatedSince=hibernated_since), + Culling(maxHibernatedDuration=max_hibernated), + ) if hibernated_since and max_hibernated: + will_delete_at = hibernated_since + parse_duration(max_hibernated) + return apispec.SessionResponse( image=self.spec.session.image, name=self.metadata.name, @@ -205,6 +230,8 @@ def as_apispec(self) -> apispec.SessionResponse: state=state, ready_containers=ready_containers, total_containers=total_containers, + will_hibernate_at=will_hibernate_at, + will_delete_at=will_delete_at, ), url=url, project_id=str(self.project_id),