refactor: add validation to crc, user and secrets endpoints

Author: Ralf Grubenmann

components/renku_data_services/crc/blueprints.py

@@ -11,6 +11,7 @@
 from renku_data_services.base_api.auth import authenticate, only_admins
 from renku_data_services.base_api.blueprint import BlueprintFactoryResponse, CustomBlueprint
 from renku_data_services.base_api.misc import validate_db_ids
+from renku_data_services.base_models.validation import validated_json
 from renku_data_services.crc import apispec, models
 from renku_data_services.crc.apispec_base import ResourceClassesFilter
 from renku_data_services.crc.db import ResourcePoolRepository, UserRepository
@@ -34,9 +35,7 @@ def get_all(self) -> BlueprintFactoryResponse:
         async def _get_all(request: Request, user: base_models.APIUser) -> HTTPResponse:
             res_filter = ResourceClassesFilter.model_validate(dict(request.query_args))
             rps = await self.rp_repo.filter_resource_pools(api_user=user, **res_filter.dict())
-            return json(
-                [apispec.ResourcePoolWithIdFiltered.model_validate(r).model_dump(exclude_none=True) for r in rps]
-            )
+            return validated_json(apispec.ResourcePoolWithIdFiltered, rps)
 
         return "/resource_pools", ["GET"], _get_all
 
@@ -49,7 +48,7 @@ def post(self) -> BlueprintFactoryResponse:
         async def _post(_: Request, user: base_models.APIUser, body: apispec.ResourcePool) -> HTTPResponse:
             rp = models.ResourcePool.from_dict(body.model_dump(exclude_none=True))
             res = await self.rp_repo.insert_resource_pool(api_user=user, resource_pool=rp)
-            return json(apispec.ResourcePoolWithId.model_validate(res).model_dump(exclude_none=True), 201)
+            return validated_json(apispec.ResourcePoolWithId, res, status=201)
 
         return "/resource_pools", ["POST"], _post
 
@@ -68,7 +67,7 @@ async def _get_one(request: Request, user: base_models.APIUser, resource_pool_id
                     message=f"The resource pool with id {resource_pool_id} cannot be found."
                 )
             rp = rps[0]
-            return json(apispec.ResourcePoolWithId.model_validate(rp).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourcePoolWithId, rp)
 
         return "/resource_pools/<resource_pool_id>", ["GET"], _get_one
 
@@ -103,7 +102,7 @@ async def _put(
                 raise errors.MissingResourceError(
                     message=f"The resource pool with ID {resource_pool_id} cannot be found."
                 )
-            return json(apispec.ResourcePoolWithId.model_validate(res).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourcePoolWithId, res)
 
         return "/resource_pools/<resource_pool_id>", ["PUT"], _put
 
@@ -126,7 +125,7 @@ async def _patch(
                 raise errors.MissingResourceError(
                     message=f"The resource pool with ID {resource_pool_id} cannot be found."
                 )
-            return json(apispec.ResourcePoolWithId.model_validate(res).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourcePoolWithId, res)
 
         return "/resource_pools/<resource_pool_id>", ["PATCH"], _patch
 
@@ -147,13 +146,9 @@ def get_all(self) -> BlueprintFactoryResponse:
         @validate_db_ids
         async def _get_all(_: Request, user: base_models.APIUser, resource_pool_id: int) -> HTTPResponse:
             res = await self.repo.get_resource_pool_users(api_user=user, resource_pool_id=resource_pool_id)
-            return json(
-                [
-                    apispec.PoolUserWithId(id=r.keycloak_id, no_default_access=r.no_default_access).model_dump(
-                        exclude_none=True
-                    )
-                    for r in res.allowed
-                ]
+            return validated_json(
+                apispec.PoolUserWithId,
+                [dict(id=r.keycloak_id, no_default_access=r.no_default_access) for r in res.allowed],
             )
 
         return "/resource_pools/<resource_pool_id>/users", ["GET"], _get_all
@@ -199,13 +194,9 @@ async def _put_post(
             user_ids=user_ids_to_add,
             append=post,
         )
-        return json(
-            [
-                apispec.PoolUserWithId(id=r.keycloak_id, no_default_access=r.no_default_access).model_dump(
-                    exclude_none=True
-                )
-                for r in updated_users
-            ],
+        return validated_json(
+            apispec.PoolUserWithId,
+            [dict(id=r.keycloak_id, no_default_access=r.no_default_access) for r in updated_users],
             status=201 if post else 200,
         )
 
@@ -219,10 +210,9 @@ async def _get(_: Request, user: base_models.APIUser, resource_pool_id: int, use
                 keycloak_id=user_id, resource_pool_id=resource_pool_id, api_user=user
             )
             if len(res.allowed) > 0:
-                return json(
-                    apispec.PoolUserWithId(
-                        id=res.allowed[0].keycloak_id, no_default_access=res.allowed[0].no_default_access
-                    ).model_dump(exclude_none=True)
+                return validated_json(
+                    apispec.PoolUserWithId,
+                    dict(id=res.allowed[0].keycloak_id, no_default_access=res.allowed[0].no_default_access),
                 )
             raise errors.MissingResourceError(
                 message=f"The user with id {user_id} or resource pool with id {resource_pool_id} cannot be found."
@@ -275,7 +265,7 @@ async def _get_all(request: Request, user: base_models.APIUser, resource_pool_id
             res = await self.repo.get_classes(
                 api_user=user, resource_pool_id=resource_pool_id, name=request.args.get("name")
             )
-            return json([apispec.ResourceClassWithId.model_validate(r).model_dump(exclude_none=True) for r in res])
+            return validated_json(apispec.ResourceClassWithId, res)
 
         return "/resource_pools/<resource_pool_id>/classes", ["GET"], _get_all
 
@@ -293,7 +283,7 @@ async def _post(
             res = await self.repo.insert_resource_class(
                 api_user=user, resource_class=cls, resource_pool_id=resource_pool_id
             )
-            return json(apispec.ResourceClassWithId.model_validate(res).model_dump(exclude_none=True), 201)
+            return validated_json(apispec.ResourceClassWithId, res, 201)
 
         return "/resource_pools/<resource_pool_id>/classes", ["POST"], _post
 
@@ -308,7 +298,7 @@ async def _get(_: Request, user: base_models.APIUser, resource_pool_id: int, cla
                 raise errors.MissingResourceError(
                     message=f"The class with id {class_id} or resource pool with id {resource_pool_id} cannot be found."
                 )
-            return json(apispec.ResourceClassWithId.model_validate(res[0]).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourceClassWithId, res[0])
 
         return "/resource_pools/<resource_pool_id>/classes/<class_id>", ["GET"], _get
 
@@ -320,7 +310,7 @@ async def _get_no_pool(_: Request, class_id: int) -> HTTPResponse:
             res = await self.repo.get_classes(api_user=None, id=class_id)
             if len(res) < 1:
                 raise errors.MissingResourceError(message=f"The class with id {class_id} cannot be found.")
-            return json(apispec.ResourceClassWithId.model_validate(res[0]).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourceClassWithId, res[0])
 
         return "/classes/<class_id>", ["GET"], _get_no_pool
 
@@ -343,14 +333,15 @@ def put(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         @validate(json=apispec.ResourceClass)
         async def _put(
             _: Request, user: base_models.APIUser, body: apispec.ResourceClass, resource_pool_id: int, class_id: int
         ) -> HTTPResponse:
             res = await self.repo.update_resource_class(
                 user, resource_pool_id, class_id, put=True, **body.model_dump(exclude_none=True)
             )
-            return json(apispec.ResourceClassWithId.model_validate(res).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourceClassWithId, res)
 
         return "/resource_pools/<resource_pool_id>/classes/<class_id>", ["PUT"], _put
 
@@ -359,6 +350,7 @@ def patch(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         @validate(json=apispec.ResourceClassPatch)
         async def _patch(
             _: Request,
@@ -370,7 +362,7 @@ async def _patch(
             res = await self.repo.update_resource_class(
                 user, resource_pool_id, class_id, put=False, **body.model_dump(exclude_none=True)
             )
-            return json(apispec.ResourceClassWithId.model_validate(res).model_dump(exclude_none=True))
+            return validated_json(apispec.ResourceClassWithId, res)
 
         return "/resource_pools/<resource_pool_id>/classes/<class_id>", ["PATCH"], _patch
 
@@ -379,6 +371,7 @@ def get_tolerations(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         async def _get_tolerations(
             _: Request, user: base_models.APIUser, resource_pool_id: int, class_id: int
         ) -> HTTPResponse:
@@ -392,6 +385,7 @@ def delete_tolerations(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         async def _delete_tolerations(
             _: Request, user: base_models.APIUser, resource_pool_id: int, class_id: int
         ) -> HTTPResponse:
@@ -405,11 +399,12 @@ def get_affinities(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         async def _get_affinities(
             _: Request, user: base_models.APIUser, resource_pool_id: int, class_id: int
         ) -> HTTPResponse:
             res = await self.repo.get_affinities(user, resource_pool_id, class_id)
-            return json([apispec.NodeAffinity.model_validate(i).model_dump(exclude_none=True) for i in res])
+            return validated_json(apispec.NodeAffinity, res)
 
         return "/resource_pools/<resource_pool_id>/classes/<class_id>/node_affinities", ["GET"], _get_affinities
 
@@ -418,6 +413,7 @@ def delete_affinities(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         async def _delete_affinities(
             _: Request, user: base_models.APIUser, resource_pool_id: int, class_id: int
         ) -> HTTPResponse:
@@ -451,7 +447,7 @@ async def _get(_: Request, user: base_models.APIUser, resource_pool_id: int) ->
                 raise errors.MissingResourceError(
                     message=f"The resource pool with ID {resource_pool_id} does not have a quota."
                 )
-            return json(apispec.QuotaWithId.model_validate(rp.quota).model_dump(exclude_none=True))
+            return validated_json(apispec.QuotaWithId, rp.quota)
 
         return "/resource_pools/<resource_pool_id>/quota", ["GET"], _get
 
@@ -460,6 +456,7 @@ def put(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         @validate(json=apispec.QuotaWithId)
         async def _put(
             _: Request, user: base_models.APIUser, resource_pool_id: int, body: apispec.QuotaWithId
@@ -473,6 +470,7 @@ def patch(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         @only_admins
+        @validate_db_ids
         @validate(json=apispec.QuotaPatch)
         async def _patch(
             _: Request, user: base_models.APIUser, resource_pool_id: int, body: apispec.QuotaPatch
@@ -500,7 +498,7 @@ async def _put_patch(
                     message=f"The quota {new_quota} is not compatible with the resource class {rc}."
                 )
         new_quota = self.quota_repo.update_quota(new_quota)
-        return json(apispec.QuotaWithId.model_validate(new_quota).model_dump(exclude_none=True))
+        return validated_json(apispec.QuotaWithId, new_quota)
 
 
 @dataclass(kw_only=True)
@@ -518,7 +516,7 @@ def get(self) -> BlueprintFactoryResponse:
         @only_admins
         async def _get(_: Request, user: base_models.APIUser, user_id: str) -> HTTPResponse:
             rps = await self.repo.get_user_resource_pools(keycloak_id=user_id, api_user=user)
-            return json([apispec.ResourcePoolWithId.model_validate(rp).model_dump(exclude_none=True) for rp in rps])
+            return validated_json(apispec.ResourcePoolWithId, rps)
 
         return "/users/<user_id>/resource_pools", ["GET"], _get
 
@@ -553,7 +551,8 @@ async def _post_put(
         rps = await self.repo.update_user_resource_pools(
             keycloak_id=user_id, resource_pool_ids=resource_pool_ids.root, append=post, api_user=api_user
         )
-        return json(
-            [apispec.ResourcePoolWithId.model_validate(rp).model_dump(exclude_none=True) for rp in rps],
+        return validated_json(
+            apispec.ResourcePoolWithId,
+            rps,
             status=201 if post else 200,
         )

components/renku_data_services/secrets/blueprints.py

@@ -6,6 +6,7 @@
 from sanic import Request, json
 from sanic.response import JSONResponse
 from sanic_ext import validate
+from ulid import ULID
 
 import renku_data_services.base_models as base_models
 from renku_data_services.base_api.auth import authenticate, only_authenticated
@@ -37,7 +38,7 @@ async def _post(_: Request, user: base_models.APIUser, body: apispec.K8sSecret)
             owner_references = []
             if body.owner_references:
                 owner_references = [OwnerReference.from_dict(o) for o in body.owner_references]
-            secret_ids = [id.root for id in body.secret_ids]
+            secret_ids = [ULID.from_str(id.root) for id in body.secret_ids]
             await create_k8s_secret(
                 user,
                 body.name,

components/renku_data_services/secrets/core.py

@@ -7,6 +7,7 @@
 from kubernetes import client as k8s_client
 from prometheus_client import Counter, Enum
 from sanic.log import logger
+from ulid import ULID
 
 from renku_data_services import base_models, errors
 from renku_data_services.base_models.core import InternalServiceAdmin
@@ -26,7 +27,7 @@ async def create_k8s_secret(
     user: base_models.APIUser,
     secret_name: str,
     namespace: str,
-    secret_ids: list[str],
+    secret_ids: list[ULID],
     owner_references: list[OwnerReference],
     secrets_repo: UserSecretsRepo,
     secret_service_private_key: rsa.RSAPrivateKey,
@@ -36,7 +37,7 @@ async def create_k8s_secret(
     """Creates a single k8s secret from a list of user secrets stored in the DB."""
     secrets = await secrets_repo.get_secrets_by_ids(requested_by=user, secret_ids=secret_ids)
     found_secret_ids = {str(s.id) for s in secrets}
-    requested_secret_ids = set(secret_ids)
+    requested_secret_ids = set(map(str, secret_ids))
     missing_secret_ids = requested_secret_ids - found_secret_ids
     if len(missing_secret_ids) > 0:
         raise errors.MissingResourceError(message=f"Couldn't find secrets with ids {', '.join(missing_secret_ids)}")

components/renku_data_services/secrets/db.py

@@ -7,6 +7,7 @@
 from sqlalchemy import delete, select
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.ext.asyncio import AsyncSession
+from ulid import ULID
 
 from renku_data_services.base_api.auth import APIUser, only_authenticated
 from renku_data_services.base_models.core import InternalServiceAdmin, ServiceAdminId
@@ -34,21 +35,22 @@ async def get_user_secrets(self, requested_by: APIUser, kind: SecretKind) -> lis
             return [o.dump() for o in orm]
 
     @only_authenticated
-    async def get_secret_by_id(self, requested_by: APIUser, secret_id: str) -> Secret | None:
+    async def get_secret_by_id(self, requested_by: APIUser, secret_id: ULID) -> Secret | None:
         """Get a specific user secret from the database."""
         async with self.session_maker() as session:
-            stmt = select(SecretORM).where(SecretORM.user_id == requested_by.id).where(SecretORM.id == secret_id)
+            stmt = select(SecretORM).where(SecretORM.user_id == requested_by.id).where(SecretORM.id == str(secret_id))
             res = await session.execute(stmt)
             orm = res.scalar_one_or_none()
             if orm is None:
                 return None
             return orm.dump()
 
     @only_authenticated
-    async def get_secrets_by_ids(self, requested_by: APIUser, secret_ids: list[str]) -> list[Secret]:
+    async def get_secrets_by_ids(self, requested_by: APIUser, secret_ids: list[ULID]) -> list[Secret]:
         """Get a specific user secrets from the database."""
+        secret_ids_str = map(str, secret_ids)
         async with self.session_maker() as session:
-            stmt = select(SecretORM).where(SecretORM.user_id == requested_by.id).where(SecretORM.id.in_(secret_ids))
+            stmt = select(SecretORM).where(SecretORM.user_id == requested_by.id).where(SecretORM.id.in_(secret_ids_str))
             res = await session.execute(stmt)
             orms = res.scalars()
             return [orm.dump() for orm in orms]
@@ -83,13 +85,13 @@ async def insert_secret(self, requested_by: APIUser, secret: Secret) -> Secret:
 
     @only_authenticated
     async def update_secret(
-        self, requested_by: APIUser, secret_id: str, encrypted_value: bytes, encrypted_key: bytes
+        self, requested_by: APIUser, secret_id: ULID, encrypted_value: bytes, encrypted_key: bytes
     ) -> Secret:
         """Update a secret."""
 
         async with self.session_maker() as session, session.begin():
             result = await session.execute(
-                select(SecretORM).where(SecretORM.id == secret_id).where(SecretORM.user_id == requested_by.id)
+                select(SecretORM).where(SecretORM.id == str(secret_id)).where(SecretORM.user_id == requested_by.id)
             )
             secret = result.scalar_one_or_none()
             if secret is None:
@@ -101,12 +103,12 @@ async def update_secret(
         return secret.dump()
 
     @only_authenticated
-    async def delete_secret(self, requested_by: APIUser, secret_id: str) -> None:
+    async def delete_secret(self, requested_by: APIUser, secret_id: ULID) -> None:
         """Delete a secret."""
 
         async with self.session_maker() as session, session.begin():
             result = await session.execute(
-                select(SecretORM).where(SecretORM.id == secret_id).where(SecretORM.user_id == requested_by.id)
+                select(SecretORM).where(SecretORM.id == str(secret_id)).where(SecretORM.user_id == requested_by.id)
             )
             secret = result.scalar_one_or_none()
             if secret is None: