diff --git a/changes/3451.fix.md b/changes/3451.fix.md new file mode 100644 index 0000000000..7c73f4205a --- /dev/null +++ b/changes/3451.fix.md @@ -0,0 +1 @@ +Fix a mis-implementation that has prevented using UUIDs to indicate an exact vfolder when invoking the vfolder REST API diff --git a/docs/manager/rest-reference/openapi.json b/docs/manager/rest-reference/openapi.json index 7f4b35afc1..f6205a8529 100644 --- a/docs/manager/rest-reference/openapi.json +++ b/docs/manager/rest-reference/openapi.json @@ -4804,7 +4804,7 @@ }, "arch": { "type": "string", - "default": "aarch64" + "default": "x86_64" }, "type": { "type": "string", @@ -4969,7 +4969,7 @@ }, "arch": { "type": "string", - "default": "aarch64" + "default": "x86_64" }, "type": { "type": "string", diff --git a/src/ai/backend/manager/api/exceptions.py b/src/ai/backend/manager/api/exceptions.py index 23f381d30b..ab10f59ea0 100644 --- a/src/ai/backend/manager/api/exceptions.py +++ b/src/ai/backend/manager/api/exceptions.py @@ -12,7 +12,8 @@ from __future__ import annotations import json -from typing import Any, Dict, Mapping, Optional, Union, cast +from collections.abc import Mapping, Sequence +from typing import TYPE_CHECKING, Any, Optional, Union, cast from aiohttp import web @@ -21,6 +22,9 @@ from ..exceptions import AgentError +if TYPE_CHECKING: + from ai.backend.manager.api.vfolder import VFolderRow + class BackendError(web.HTTPError): """ @@ -247,7 +251,10 @@ class TooManySessionsMatched(BackendError, web.HTTPNotFound): error_title = "Too many sessions matched." def __init__( - self, extra_msg: Optional[str] = None, extra_data: Optional[Dict[str, Any]] = None, **kwargs + self, + extra_msg: Optional[str] = None, + extra_data: Optional[dict[str, Any]] = None, + **kwargs, ): if extra_data is not None and (matches := extra_data.get("matches", None)) is not None: serializable_matches = [ @@ -288,7 +295,21 @@ class VFolderCreationFailed(BackendError, web.HTTPBadRequest): class TooManyVFoldersFound(BackendError, web.HTTPNotFound): error_type = "https://api.backend.ai/probs/too-many-vfolders" - error_title = "There are two or more matching vfolders." + error_title = "Multiple vfolders found for the operation for a single vfolder." + + def __init__(self, matched_rows: Sequence[VFolderRow]) -> None: + serialized_matches = [ + { + "id": row["id"], + "host": row["host"], + "user": row["user_email"], + "user_id": row["user"], + "group": row["group_name"], + "group_id": row["group"], + } + for row in matched_rows + ] + super().__init__(extra_data={"matches": serialized_matches}) class VFolderNotFound(ObjectNotFound): diff --git a/src/ai/backend/manager/api/vfolder.py b/src/ai/backend/manager/api/vfolder.py index bd8fe36ecc..60a7fe32fe 100644 --- a/src/ai/backend/manager/api/vfolder.py +++ b/src/ai/backend/manager/api/vfolder.py @@ -275,6 +275,8 @@ async def resolve_vfolder_rows( def with_vfolder_rows_resolved( perm: VFolderPermissionSetAlias | VFolderPermission, + *, + allow_privileged_access: bool = False, ) -> Callable[ [Callable[Concatenate[web.Request, Sequence[VFolderRow], P], Awaitable[web.Response]]], Callable[Concatenate[web.Request, P], Awaitable[web.Response]], @@ -294,9 +296,22 @@ def _wrapper( ) -> Callable[Concatenate[web.Request, P], Awaitable[web.Response]]: @functools.wraps(handler) async def _wrapped(request: web.Request, *args: P.args, **kwargs: P.kwargs) -> web.Response: - folder_name = request.match_info["name"] + folder_name_or_id: str | uuid.UUID + piece = request.match_info["name"] + try: + folder_name_or_id = uuid.UUID(piece) + except ValueError: + folder_name_or_id = piece return await handler( - request, await resolve_vfolder_rows(request, perm, folder_name), *args, **kwargs + request, + await resolve_vfolder_rows( + request, + perm, + folder_name_or_id, + allow_privileged_access=allow_privileged_access, + ), + *args, + **kwargs, ) return _wrapped @@ -681,10 +696,12 @@ async def create(request: web.Request, params: CreateRequestModel) -> web.Respon async def list_folders(request: web.Request, params: Any) -> web.Response: resp = [] root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] domain_name = request["user"]["domain_name"] - - log.info("VFOLDER.LIST (email:{}, ak:{})", request["user"]["email"], access_key) + log.info( + "VFOLDER.LIST (email:{}, ak:{})", + request["user"]["email"], + request["keypair"]["access_key"], + ) entries: List[Mapping[str, Any]] | Sequence[Mapping[str, Any]] owner_user_uuid, owner_user_role = await get_user_scopes(request, params) async with root_ctx.db.begin_readonly() as conn: @@ -806,11 +823,10 @@ async def fetch_exposed_volume_fields( ) async def list_hosts(request: web.Request, params: Any) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.LIST_HOSTS (emai:{}, ak:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], ) domain_name = request["user"]["domain_name"] group_id = params["group_id"] @@ -868,11 +884,10 @@ async def list_hosts(request: web.Request, params: Any) -> web.Response: @server_status_required(READ_ALLOWED) async def list_all_hosts(request: web.Request) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.LIST_ALL_HOSTS (email:{}, ak:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], ) all_volumes = await root_ctx.storage_manager.get_all_volumes() all_hosts = {f"{proxy_name}:{volume_data['name']}" for proxy_name, volume_data in all_volumes} @@ -895,11 +910,10 @@ async def list_all_hosts(request: web.Request) -> web.Response: ) async def get_volume_perf_metric(request: web.Request, params: Any) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.VOLUME_PERF_METRIC (email:{}, ak:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], ) proxy_name, volume_name = root_ctx.storage_manager.split_host(params["folder_host"]) async with root_ctx.storage_manager.request( @@ -918,11 +932,10 @@ async def get_volume_perf_metric(request: web.Request, params: Any) -> web.Respo @server_status_required(READ_ALLOWED) async def list_allowed_types(request: web.Request) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.LIST_ALLOWED_TYPES (email:{}, ak:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], ) allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() return web.json_response(allowed_vfolder_types, status=200) @@ -935,13 +948,12 @@ async def list_allowed_types(request: web.Request) -> web.Response: async def get_info(request: web.Request, row: VFolderRow) -> web.Response: root_ctx: RootContext = request.app["_root.context"] resp: Dict[str, Any] = {} - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] log.info( - "VFOLDER.GETINFO (email:{}, ak:{}, vf:{})", + "VFOLDER.GETINFO (email:{}, ak:{}, vf:{} (resolved-from:{!r}))", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], ) if row["permission"] is None: is_owner = True @@ -1202,25 +1214,26 @@ class RenameRequestModel(BaseModel): @server_status_required(ALL_ALLOWED) @pydantic_params_api_handler(RenameRequestModel) @with_vfolder_rows_resolved(VFolderPermission.OWNER_PERM) +@with_vfolder_status_checked(VFolderStatusSet.READABLE) async def rename_vfolder( request: web.Request, - row: Sequence[VFolderRow], + row: VFolderRow, params: RenameRequestModel, ) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - old_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] domain_name = request["user"]["domain_name"] user_role = request["user"]["role"] user_uuid = request["user"]["uuid"] resource_policy = request["keypair"]["resource_policy"] + old_name = row["name"] new_name = params.new_name allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() log.info( - "VFOLDER.RENAME (email:{}, ak:{}, vf.old:{}, vf.new:{})", + "VFOLDER.RENAME (email:{}, ak:{}, vf:{} (resolved-from:{!r}), new-name:{})", request["user"]["email"], - access_key, - old_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], new_name, ) async with root_ctx.db.begin() as conn: @@ -1277,6 +1290,13 @@ async def update_vfolder_options( domain_name = request["user"]["domain_name"] resource_policy = request["keypair"]["resource_policy"] allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() + log.info( + "VFOLDER.UPDATE_OPTIONS (email:{}, ak:{}, vf:{} (resolved-from:{!r}))", + request["user"]["email"], + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], + ) async with root_ctx.db.begin_readonly() as conn: query = sa.select([vfolders.c.host]).select_from(vfolders).where(vfolders.c.id == row["id"]) folder_host = await conn.scalar(query) @@ -1322,13 +1342,12 @@ async def mkdir(request: web.Request, params: Any, row: VFolderRow) -> web.Respo if isinstance(params["path"], list) and len(params["path"]) > 50: raise InvalidAPIParameters("Too many directories specified.") root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] log.info( - "VFOLDER.MKDIR (email:{}, ak:{}, vf:{}, paths:{})", + "VFOLDER.MKDIR (email:{}, ak:{}, vf:{} (resolved-from:{!r}), paths:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], params["path"], ) proxy_name, volume_name = root_ctx.storage_manager.split_host(row["host"]) @@ -1367,14 +1386,14 @@ async def create_download_session( request: web.Request, params: Any, row: VFolderRow ) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - log_fmt = "VFOLDER.CREATE_DOWNLOAD_SESSION(email:{}, ak:{}, vf:{}, path:{})" - log_args = ( + log.info( + "VFOLDER.CREATE_DOWNLOAD_SESSION(email:{}, ak:{}, vf:{} (resolved-from:{!r}), path:{})", request["user"]["email"], request["keypair"]["access_key"], - row["name"], + row["id"], + request.match_info["name"], params["path"], ) - log.info(log_fmt, *log_args) unmanaged_path = row["unmanaged_path"] user_uuid = request["user"]["uuid"] folder_host = row["host"] @@ -1424,11 +1443,14 @@ async def create_download_session( ) async def create_upload_session(request: web.Request, params: Any, row: VFolderRow) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] - log_fmt = "VFOLDER.CREATE_UPLOAD_SESSION (email:{}, ak:{}, vf:{}, path:{})" - log_args = (request["user"]["email"], access_key, folder_name, params["path"]) - log.info(log_fmt, *log_args) + log.info( + "VFOLDER.CREATE_UPLOAD_SESSION (email:{}, ak:{}, vf:{} (resolved-from:{!r}), path:{})", + request["user"]["email"], + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], + params["path"], + ) user_uuid = request["user"]["uuid"] domain_name = request["user"]["domain_name"] folder_host = row["host"] @@ -1477,8 +1499,6 @@ async def create_upload_session(request: web.Request, params: Any, row: VFolderR ) async def rename_file(request: web.Request, params: Any, row: VFolderRow) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] user_uuid = request["user"]["uuid"] domain_name = request["user"]["domain_name"] folder_host = row["host"] @@ -1495,10 +1515,11 @@ async def rename_file(request: web.Request, params: Any, row: VFolderRow) -> web permission=VFolderHostPermission.MODIFY, ) log.info( - "VFOLDER.RENAME_FILE (email:{}, ak:{}, vf:{}, target_path:{}, new_name:{})", + "VFOLDER.RENAME_FILE (email:{}, ak:{}, vf:{} (resolved-from:{!r}), target_path:{}, new_name:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], params["target_path"], params["new_name"], ) @@ -1530,13 +1551,12 @@ async def rename_file(request: web.Request, params: Any, row: VFolderRow) -> web ) async def move_file(request: web.Request, params: Any, row: VFolderRow) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] log.info( - "VFOLDER.MOVE_FILE (email:{}, ak:{}, vf:{}, src:{}, dst:{})", + "VFOLDER.MOVE_FILE (email:{}, ak:{}, vf:{} (resolved-from:{!r}), src:{}, dst:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], params["src"], params["dst"], ) @@ -1568,14 +1588,13 @@ async def move_file(request: web.Request, params: Any, row: VFolderRow) -> web.R ) async def delete_files(request: web.Request, params: Any, row: VFolderRow) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] recursive = params["recursive"] log.info( - "VFOLDER.DELETE_FILES (email:{}, ak:{}, vf:{}, path:{}, recursive:{})", + "VFOLDER.DELETE_FILES (email:{}, ak:{}, vf:{} (resolved-from:{!r}), path:{}, recursive:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], params["files"], recursive, ) @@ -1608,13 +1627,12 @@ async def list_files(request: web.Request, params: Any, row: VFolderRow) -> web. # we can skip check_vfolder_status() guard here since the status is already verified by # vfolder_permission_required() decorator root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] log.info( - "VFOLDER.LIST_FILES (email:{}, ak:{}, vf:{}, path:{})", + "VFOLDER.LIST_FILES (email:{}, ak:{}, vf:{} (resolved-from:{!r}), path:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], params["path"], ) proxy_name, volume_name = root_ctx.storage_manager.split_host(row["host"]) @@ -1660,11 +1678,10 @@ async def list_files(request: web.Request, params: Any, row: VFolderRow) -> web. @server_status_required(READ_ALLOWED) async def list_sent_invitations(request: web.Request) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.LIST_SENT_INVITATIONS (email:{}, ak:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], ) async with root_ctx.db.begin() as conn: j = sa.join(vfolders, vfolder_invitations, vfolders.c.id == vfolder_invitations.c.vfolder) @@ -1707,13 +1724,12 @@ async def update_invitation(request: web.Request, params: Any) -> web.Response: Update sent invitation's permission. Other fields are not allowed to be updated. """ root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] inv_id = request.match_info["inv_id"] perm = params["perm"] log.info( "VFOLDER.UPDATE_INVITATION (email:{}, ak:{}, inv:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], inv_id, ) async with root_ctx.db.begin() as conn: @@ -1743,16 +1759,17 @@ async def update_invitation(request: web.Request, params: Any) -> web.Response: ) async def invite(request: web.Request, params: Any, row: VFolderRow) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] + folder_name = row["name"] access_key = request["keypair"]["access_key"] user_uuid = request["user"]["uuid"] perm = params["perm"] invitee_emails = params["emails"] log.info( - "VFOLDER.INVITE (email:{}, ak:{}, vf:{}, inv.users:{})", + "VFOLDER.INVITE (email:{}, ak:{}, vf:{} (resolved-from:{!r}), inv.users:{})", request["user"]["email"], access_key, - folder_name, + row["id"], + request.match_info["name"], ",".join(invitee_emails), ) domain_name = request["user"]["domain_name"] @@ -1868,11 +1885,10 @@ async def invite(request: web.Request, params: Any, row: VFolderRow) -> web.Resp @server_status_required(READ_ALLOWED) async def invitations(request: web.Request) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.INVITATIONS (email:{}, ak:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], ) async with root_ctx.db.begin() as conn: j = sa.join(vfolders, vfolder_invitations, vfolders.c.id == vfolder_invitations.c.vfolder) @@ -1919,13 +1935,12 @@ async def accept_invitation(request: web.Request, params: Any) -> web.Response: :param inv_id: ID of vfolder_invitations row. """ root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] user_uuid = request["user"]["uuid"] inv_id = params["inv_id"] log.info( "VFOLDER.ACCEPT_INVITATION (email:{}, ak:{}, inv:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], inv_id, ) async with root_ctx.db.begin() as conn: @@ -2004,13 +2019,12 @@ async def accept_invitation(request: web.Request, params: Any) -> web.Response: ) async def delete_invitation(request: web.Request, params: Any) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] request_email = request["user"]["email"] inv_id = params["inv_id"] log.info( "VFOLDER.DELETE_INVITATION (email:{}, ak:{}, inv:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], inv_id, ) try: @@ -2053,7 +2067,7 @@ async def delete_invitation(request: web.Request, params: Any) -> web.Response: @admin_required @server_status_required(ALL_ALLOWED) -@with_vfolder_rows_resolved(VFolderPermission.OWNER_PERM) +@with_vfolder_rows_resolved(VFolderPermission.READ_ONLY) @with_vfolder_status_checked(VFolderStatusSet.UPDATABLE) @check_api_params( t.Dict({ @@ -2070,42 +2084,27 @@ async def share(request: web.Request, params: Any, row: VFolderRow) -> web.Respo be shared directly. """ root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] - folder_name = request.match_info["name"] log.info( - "VFOLDER.SHARE (email:{}, ak:{}, vf:{}, perm:{}, users:{})", + "VFOLDER.SHARE (email:{}, ak:{}, vf:{} (resolved-from:{!r}), perm:{}, users:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], params["permission"], ",".join(params["emails"]), ) user_uuid = request["user"]["uuid"] domain_name = request["user"]["domain_name"] resource_policy = request["keypair"]["resource_policy"] + if row["ownership_type"] != VFolderOwnershipType.GROUP: + raise VFolderNotFound("Only project folders are directly sharable.") async with root_ctx.db.begin() as conn: from ..models import association_groups_users as agus - # Get the group-type virtual folder. - query = ( - sa.select([vfolders.c.id, vfolders.c.host, vfolders.c.ownership_type, vfolders.c.group]) - .select_from(vfolders) - .where( - (vfolders.c.ownership_type == VFolderOwnershipType.GROUP) - & (vfolders.c.name == folder_name), - ) - ) - result = await conn.execute(query) - vf_infos = result.fetchall() - if len(vf_infos) < 1: - raise VFolderNotFound("Only project folders are directly sharable.") - if len(vf_infos) > 1: - raise InternalServerError(f"Multiple project folders found: {folder_name}") - vf_info = vf_infos[0] allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() await ensure_host_permission_allowed( conn, - vf_info["host"], + row["host"], allowed_vfolder_types=allowed_vfolder_types, user_uuid=user_uuid, resource_policy=resource_policy, @@ -2121,7 +2120,7 @@ async def share(request: web.Request, params: Any, row: VFolderRow) -> web.Respo .where( (users.c.email.in_(params["emails"])) & (users.c.email != request["user"]["email"]) - & (agus.c.group_id == vf_info["group"]) + & (agus.c.group_id == row["group"]) & (users.c.status.in_(ACTIVE_USER_STATUSES)), ) ) @@ -2145,7 +2144,7 @@ async def share(request: web.Request, params: Any, row: VFolderRow) -> web.Respo .select_from(vfolder_permissions) .where( (vfolder_permissions.c.user.in_(users_to_share)) - & (vfolder_permissions.c.vfolder == vf_info["id"]), + & (vfolder_permissions.c.vfolder == row["id"]), ) ) result = await conn.execute(query) @@ -2158,7 +2157,7 @@ async def share(request: web.Request, params: Any, row: VFolderRow) -> web.Respo vfolder_permissions, { "permission": params["permission"], - "vfolder": vf_info["id"], + "vfolder": row["id"], "user": _user, }, ) @@ -2168,7 +2167,7 @@ async def share(request: web.Request, params: Any, row: VFolderRow) -> web.Respo query = ( sa.update(vfolder_permissions) .values(permission=params["permission"]) - .where(vfolder_permissions.c.vfolder == vf_info["id"]) + .where(vfolder_permissions.c.vfolder == row["id"]) .where(vfolder_permissions.c.user == _user) ) await conn.execute(query) @@ -2178,7 +2177,7 @@ async def share(request: web.Request, params: Any, row: VFolderRow) -> web.Respo @admin_required @server_status_required(ALL_ALLOWED) -@with_vfolder_rows_resolved(VFolderPermission.OWNER_PERM) +@with_vfolder_rows_resolved(VFolderPermission.READ_ONLY) @with_vfolder_status_checked(VFolderStatusSet.UPDATABLE) @check_api_params( t.Dict({ @@ -2190,39 +2189,24 @@ async def unshare(request: web.Request, params: Any, row: VFolderRow) -> web.Res Unshare a group folder from users. """ root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] - folder_name = request.match_info["name"] log.info( - "VFOLDER.UNSHARE (email:{}, ak:{}, vf:{}, users:{})", + "VFOLDER.UNSHARE (email:{}, ak:{}, vf:{} (resolved-from:{!r}), users:{})", request["user"]["email"], - access_key, - folder_name, + request["keypair"]["access_key"], + row["id"], + request.match_info["name"], ",".join(params["emails"]), ) user_uuid = request["user"]["uuid"] domain_name = request["user"]["domain_name"] resource_policy = request["keypair"]["resource_policy"] + if row["ownership_type"] != VFolderOwnershipType.GROUP: + raise VFolderNotFound("Only project folders are directly unsharable.") async with root_ctx.db.begin() as conn: - # Get the group-type virtual folder. - query = ( - sa.select([vfolders.c.id, vfolders.c.host]) - .select_from(vfolders) - .where( - (vfolders.c.ownership_type == VFolderOwnershipType.GROUP) - & (vfolders.c.name == folder_name), - ) - ) - result = await conn.execute(query) - vf_infos = result.fetchall() - if len(vf_infos) < 1: - raise VFolderNotFound("Only project folders are directly unsharable.") - if len(vf_infos) > 1: - raise InternalServerError(f"Multiple project folders found: {folder_name}") - vf_info = vf_infos[0] allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() await ensure_host_permission_allowed( conn, - vf_info["host"], + row["host"], allowed_vfolder_types=allowed_vfolder_types, user_uuid=user_uuid, resource_policy=resource_policy, @@ -2241,7 +2225,7 @@ async def unshare(request: web.Request, params: Any, row: VFolderRow) -> web.Res # Delete vfolder_permission(s). query = sa.delete(vfolder_permissions).where( - (vfolder_permissions.c.vfolder == vf_info["id"]) + (vfolder_permissions.c.vfolder == row["id"]) & (vfolder_permissions.c.user.in_(users_to_unshare)), ) await conn.execute(query) @@ -2250,47 +2234,29 @@ async def unshare(request: web.Request, params: Any, row: VFolderRow) -> web.Res async def _delete( root_ctx: RootContext, - condition: sa.sql.BinaryExpression, + vfolder_row: VFolderRow, user_uuid: uuid.UUID, user_role: UserRole, domain_name: str, - allowed_vfolder_types: Sequence[str], resource_policy: Mapping[str, Any], ) -> None: + # Only the effective folder owner can delete the folder. + if not vfolder_row["is_owner"]: + raise InvalidAPIParameters("Cannot delete the vfolder that is not owned by myself.") + await check_vfolder_status(vfolder_row, VFolderStatusSet.DELETABLE) async with root_ctx.db.begin_readonly_session() as db_session: - db_conn = db_session.bind - entries = await query_accessible_vfolders( - db_conn, - user_uuid, - allow_privileged_access=True, - user_role=user_role, - domain_name=domain_name, - allowed_vfolder_types=allowed_vfolder_types, - extra_vf_conds=condition, - ) - if len(entries) > 1: - raise TooManyVFoldersFound( - extra_msg="Multiple folders with the same name.", - extra_data=[entry["host"] for entry in entries], - ) - elif len(entries) == 0: - raise InvalidAPIParameters("No such vfolder.") - # query_accesible_vfolders returns list - entry = entries[0] - # Folder owner OR user who have DELETE permission can delete folder. - if not entry["is_owner"] and entry["permission"] != VFolderPermission.RW_DELETE: - raise InvalidAPIParameters("Cannot delete the vfolder that is not owned by myself.") # perform extra check to make sure records of alive model service not removed by foreign key rule - if entry["usage_mode"] == VFolderUsageMode.MODEL: - live_endpoints = await EndpointRow.list_by_model(db_session, entry["id"]) + if vfolder_row["usage_mode"] == VFolderUsageMode.MODEL: + live_endpoints = await EndpointRow.list_by_model(db_session, vfolder_row["id"]) if ( len([e for e in live_endpoints if e.lifecycle_stage == EndpointLifecycle.CREATED]) > 0 ): raise ModelServiceDependencyNotCleared - folder_host = entry["host"] + folder_host = vfolder_row["host"] + allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() await ensure_host_permission_allowed( - db_conn, + db_session.bind, folder_host, allowed_vfolder_types=allowed_vfolder_types, user_uuid=user_uuid, @@ -2299,7 +2265,7 @@ async def _delete( permission=VFolderHostPermission.DELETE, ) - vfolder_row_ids = (entry["id"],) + vfolder_row_ids = (vfolder_row["id"],) async with root_ctx.db.connect() as db_conn: await delete_vfolder_relation_rows(db_conn, root_ctx.db.begin_session, vfolder_row_ids) await update_vfolder_status( @@ -2322,44 +2288,34 @@ class DeleteRequestModel(BaseModel): async def delete_by_id(request: web.Request, params: DeleteRequestModel) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] + domain_name = request["user"]["domain_name"] user_uuid = request["user"]["uuid"] user_role = request["user"]["role"] - domain_name = request["user"]["domain_name"] resource_policy = request["keypair"]["resource_policy"] - allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() folder_id = params.vfolder_id + + rows = await resolve_vfolder_rows( + request, + VFolderPermissionSetAlias.READABLE, + folder_id, + allow_privileged_access=True, + ) + assert len(rows) == 1 + row = rows[0] log.info( "VFOLDER.DELETE_BY_ID (email:{}, ak:{}, vf:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], folder_id, ) - - row = ( - await resolve_vfolder_rows( - request, VFolderPermission.OWNER_PERM, folder_id, allow_privileged_access=True - ) - )[0] - await check_vfolder_status(row, VFolderStatusSet.DELETABLE) - try: - await _delete( - root_ctx, - (vfolders.c.id == folder_id), - user_uuid, - user_role, - domain_name, - allowed_vfolder_types, - resource_policy, - ) - except TooManyVFoldersFound as e: - log.error( - "VFOLDER.DELETE_BY_ID(email: {}, folder id:{}, hosts:{}", - request["user"]["email"], - folder_id, - e.extra_data, - ) - raise + await _delete( + root_ctx, + row, + user_uuid, + user_role, + domain_name, + resource_policy, + ) return web.Response(status=204) @@ -2368,40 +2324,34 @@ async def delete_by_id(request: web.Request, params: DeleteRequestModel) -> web. async def delete_by_name(request: web.Request) -> web.Response: root_ctx: RootContext = request.app["_root.context"] - folder_name = request.match_info["name"] - access_key = request["keypair"]["access_key"] domain_name = request["user"]["domain_name"] user_role = request["user"]["role"] user_uuid = request["user"]["uuid"] - allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() resource_policy = request["keypair"]["resource_policy"] + folder_name = request.match_info["name"] + rows = await resolve_vfolder_rows( + request, + VFolderPermissionSetAlias.READABLE, + folder_name, + allow_privileged_access=True, + ) + if len(rows) > 1: + raise TooManyVFoldersFound(rows) + row = rows[0] log.info( - "VFOLDER.DELETE (email:{}, ak:{}, vf:{})", + "VFOLDER.DELETE_BY_NAME (email:{}, ak:{}, vf:{} (resolved-from:{!r}))", request["user"]["email"], - access_key, + request["keypair"]["access_key"], + row["id"], folder_name, ) - - rows = await resolve_vfolder_rows( - request, VFolderPermission.OWNER_PERM, folder_name, allow_privileged_access=True - ) - for row in rows: - try: - await check_vfolder_status(row, VFolderStatusSet.DELETABLE) - break - except VFolderFilterStatusFailed: - continue - else: - raise VFolderFilterStatusFailed - await _delete( root_ctx, - (vfolders.c.id == row["id"]), + row, user_uuid, user_role, domain_name, - allowed_vfolder_types, resource_policy, ) return web.Response(status=204) @@ -2423,47 +2373,24 @@ class CompactVFolderInfoModel(BaseResponseModel): @server_status_required(ALL_ALLOWED) @pydantic_params_api_handler(IDRequestModel) async def get_vfolder_id(request: web.Request, params: IDRequestModel) -> CompactVFolderInfoModel: - root_ctx: RootContext = request.app["_root.context"] - folder_name = params.name - access_key = request["keypair"]["access_key"] - domain_name = request["user"]["domain_name"] - user_role = request["user"]["role"] - user_uuid = request["user"]["uuid"] - allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() - + rows = await resolve_vfolder_rows( + request, + VFolderPermissionSetAlias.READABLE, + folder_name, + allow_privileged_access=True, + ) + if len(rows) > 1: + raise TooManyVFoldersFound(rows) + row = rows[0] log.info( - "VFOLDER.GET_ID (email:{}, ak:{}, vf:{})", + "VFOLDER.GET_ID (email:{}, ak:{}, vf:{} (resolved-from:{!r}))", request["user"]["email"], - access_key, + request["keypair"]["access_key"], + row["id"], folder_name, ) - async with root_ctx.db.begin_readonly_session() as db_session: - entries = await query_accessible_vfolders( - db_session.bind, - user_uuid, - allow_privileged_access=True, - user_role=user_role, - domain_name=domain_name, - allowed_vfolder_types=allowed_vfolder_types, - extra_vf_conds=(vfolders.c.name == folder_name), - allowed_status_set=VFolderStatusSet.ALL, - ) - if len(entries) > 1: - log.error( - "VFOLDER.GET_ID(folder name:{}, hosts:{}", - folder_name, - [entry["host"] for entry in entries], - ) - raise TooManyVFoldersFound( - extra_msg="Multiple folders with the same name.", - extra_data=None, - ) - elif len(entries) == 0: - raise InvalidAPIParameters(f"No such vfolder (name: {folder_name})") - # query_accesible_vfolders returns list - entry = entries[0] - return CompactVFolderInfoModel(id=entry["id"], name=folder_name) + return CompactVFolderInfoModel(id=row["id"], name=folder_name) class DeleteFromTrashRequestModel(BaseModel): @@ -2484,17 +2411,20 @@ async def delete_from_trash_bin( root_ctx: RootContext = request.app["_root.context"] app_ctx: PrivateContext = request.app["folders.context"] folder_id = params.vfolder_id - access_key = request["keypair"]["access_key"] domain_name = request["user"]["domain_name"] user_role = request["user"]["role"] user_uuid = request["user"]["uuid"] allowed_vfolder_types = await root_ctx.shared_config.get_vfolder_types() + log.info( "VFOLDER.DELETE_FROM_TRASH_BIN (email:{}, ak:{}, vf:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], folder_id, ) + + # TODO: replace with @with_vfolder_rows_resolved + @with_vfolder_status_checked + # --- from here --- row = ( await resolve_vfolder_rows( request, VFolderPermission.OWNER_PERM, folder_id, allow_privileged_access=True @@ -2512,28 +2442,22 @@ async def delete_from_trash_bin( allowed_vfolder_types=allowed_vfolder_types, extra_vf_conds=(vfolders.c.id == folder_id), ) - # FIXME: For now, deleting multiple VFolders at once will raise an error. - # This behavior should be fixed in 24.03 if len(entries) > 1: log.error( "VFOLDER.DELETE_FROM_TRASH_BIN(folder id:{}, hosts:{}", folder_id, [entry["host"] for entry in entries], ) - raise TooManyVFoldersFound( - extra_msg="Multiple folders with the same id.", - extra_data=None, - ) + raise TooManyVFoldersFound(entries) elif len(entries) == 0: raise InvalidAPIParameters("No such vfolder.") - # query_accesible_vfolders returns list - entry = entries[0] + row = entries[0] + # --- until here --- - folder_host = entry["host"] # fs-level deletion may fail or take longer time await initiate_vfolder_deletion( root_ctx.db, - [VFolderDeletionInfo(VFolderID.from_row(entry), folder_host)], + [VFolderDeletionInfo(VFolderID.from_row(row), row["host"])], root_ctx.storage_manager, app_ctx.storage_ptask_group, ) @@ -2556,11 +2480,10 @@ async def purge(request: web.Request, params: PurgeRequestModel) -> web.Response """ root_ctx: RootContext = request.app["_root.context"] folder_id = params.vfolder_id - access_key = request["keypair"]["access_key"] log.info( "VFOLDER.PURGE (email:{}, ak:{}, vf:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], folder_id, ) if request["user"]["role"] not in ( @@ -2597,7 +2520,6 @@ async def restore(request: web.Request, params: RestoreRequestModel) -> web.Resp """ root_ctx: RootContext = request.app["_root.context"] folder_id = params.vfolder_id - access_key = request["keypair"]["access_key"] domain_name = request["user"]["domain_name"] user_role = request["user"]["role"] user_uuid = request["user"]["uuid"] @@ -2605,10 +2527,12 @@ async def restore(request: web.Request, params: RestoreRequestModel) -> web.Resp log.info( "VFOLDER.RESTORE (email: {}, ak:{}, vf:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], folder_id, ) + # TODO: replace with @with_vfolder_rows_resolved + @with_vfolder_status_checked + # --- from here --- row = ( await resolve_vfolder_rows( request, VFolderPermission.OWNER_PERM, folder_id, allow_privileged_access=True @@ -2626,7 +2550,6 @@ async def restore(request: web.Request, params: RestoreRequestModel) -> web.Resp allowed_vfolder_types=allowed_vfolder_types, extra_vf_conds=(vfolders.c.id == folder_id), ) - # FIXME: For now, multiple entries on restore vfolder will raise an error. if len(restore_targets) > 1: log.error( "VFOLDER.RESTORE(email:{}, folder id:{}, hosts:{})", @@ -2634,28 +2557,26 @@ async def restore(request: web.Request, params: RestoreRequestModel) -> web.Resp folder_id, [entry["host"] for entry in restore_targets], ) - raise TooManyVFoldersFound( - extra_msg="Multiple folders with the same name.", - extra_data=None, - ) + raise TooManyVFoldersFound(restore_targets) elif len(restore_targets) == 0: raise InvalidAPIParameters("No such vfolder.") - # query_accesible_vfolders returns list - entry = restore_targets[0] - # Folder owner OR user who have DELETE permission can restore folder. - if not entry["is_owner"] and entry["permission"] != VFolderPermission.RW_DELETE: - raise InvalidAPIParameters("Cannot restore the vfolder that is not owned by myself.") + row = restore_targets[0] + # --- until here --- + + # Folder owner OR user who have DELETE permission can restore folder. + if not row["is_owner"] and row["permission"] != VFolderPermission.RW_DELETE: + raise InvalidAPIParameters("Cannot restore the vfolder that is not owned by myself.") # fs-level mv may fail or take longer time # but let's complete the db transaction to reflect that it's deleted. - await update_vfolder_status(root_ctx.db, (entry["id"],), VFolderOperationStatus.READY) + await update_vfolder_status(root_ctx.db, (row["id"],), VFolderOperationStatus.READY) return web.Response(status=204) @auth_required @server_status_required(ALL_ALLOWED) -@with_vfolder_rows_resolved(VFolderPermissionSetAlias.READABLE) +@with_vfolder_rows_resolved(VFolderPermissionSetAlias.READABLE, allow_privileged_access=True) @with_vfolder_status_checked(VFolderStatusSet.UPDATABLE) @check_api_params( t.Dict({ @@ -2690,10 +2611,11 @@ async def leave(request: web.Request, params: Any, row: VFolderRow) -> web.Respo user_uuid = rqst_user_uuid log.info( - "VFOLDER.LEAVE(email:{}, ak:{}, vfid:{}, uid:{}, perm:{})", + "VFOLDER.LEAVE(email:{}, ak:{}, vf:{} (resolved-from:{!r}), uid:{}, perm:{})", request["user"]["email"], access_key, vfolder_id, + request.match_info["name"], user_uuid, perm, ) @@ -2730,10 +2652,11 @@ async def clone(request: web.Request, params: Any, row: VFolderRow) -> web.Respo resource_policy = request["keypair"]["resource_policy"] domain_name = request["user"]["domain_name"] log.info( - "VFOLDER.CLONE (email:{}, ak:{}, vf:{}, vft:{}, vfh:{}, umod:{}, perm:{})", + "VFOLDER.CLONE (email:{}, ak:{}, vf:{} (resolved-from:{!r}), vft:{}, vfh:{}, umod:{}, perm:{})", request["user"]["email"], access_key, - row["name"], + row["id"], + request.match_info["name"], params["target_name"], params["folder_host"], params["usage_mode"].value, @@ -2896,12 +2819,12 @@ async def list_shared_vfolders(request: web.Request, params: Any) -> web.Respons Not available for group vfolders. """ root_ctx: RootContext = request.app["_root.context"] - access_key = request["keypair"]["access_key"] target_vfid = params["vfolder_id"] log.info( - "VFOLDER.LIST_SHARED_VFOLDERS (email:{}, ak:{})", + "VFOLDER.LIST_SHARED_VFOLDERS (email:{}, ak:{}, vf:{})", request["user"]["email"], - access_key, + request["keypair"]["access_key"], + target_vfid, ) async with root_ctx.db.begin() as conn: j = vfolder_permissions.join(vfolders, vfolders.c.id == vfolder_permissions.c.vfolder).join( @@ -2961,7 +2884,7 @@ async def update_shared_vfolder(request: web.Request, params: Any) -> web.Respon user_uuid = params["user"] perm = params["perm"] log.info( - "VFOLDER.UPDATE_SHARED_VFOLDER(email:{}, ak:{}, vfid:{}, uid:{}, perm:{})", + "VFOLDER.UPDATE_SHARED_VFOLDER(email:{}, ak:{}, vf:{}, uid:{}, perm:{})", request["user"]["email"], access_key, vfolder_id, @@ -3040,7 +2963,7 @@ async def update_vfolder_sharing_status( vfolder_id = params.vfolder_id user_perm_list = params.user_perm_list log.info( - "VFOLDER.UPDATE_VFOLDER_SHARING_STATUS(email:{}, ak:{}, vfid:{}, data:{})", + "VFOLDER.UPDATE_VFOLDER_SHARING_STATUS(email:{}, ak:{}, vf:{}, data:{})", request["user"]["email"], access_key, vfolder_id, @@ -3164,7 +3087,10 @@ async def list_mounts(request: web.Request) -> web.Response: """ root_ctx: RootContext = request.app["_root.context"] access_key = request["keypair"]["access_key"] - log.info("VFOLDER.LIST_MOUNTS(ak:{})", access_key) + log.info( + "VFOLDER.LIST_MOUNTS(ak:{})", + access_key, + ) mount_prefix = await root_ctx.shared_config.get_raw("volumes/_mount") if mount_prefix is None: mount_prefix = "/mnt" diff --git a/src/ai/backend/manager/models/vfolder.py b/src/ai/backend/manager/models/vfolder.py index 1f9f14202a..3131792316 100644 --- a/src/ai/backend/manager/models/vfolder.py +++ b/src/ai/backend/manager/models/vfolder.py @@ -618,9 +618,7 @@ async def _append_entries(_query, _is_owner=True): query = query.where( vfolders.c.status.not_in(vfolder_status_map[VFolderStatusSet.INACCESSIBLE]) ) - if not allow_privileged_access or ( - user_role != UserRole.ADMIN and user_role != UserRole.SUPERADMIN - ): + if not allow_privileged_access or user_role not in (UserRole.ADMIN, UserRole.SUPERADMIN): query = query.where(vfolders.c.user == user_uuid) await _append_entries(query) diff --git a/tests/manager/api/test_vfolder.py b/tests/manager/api/test_vfolder.py new file mode 100644 index 0000000000..bac843fbf1 --- /dev/null +++ b/tests/manager/api/test_vfolder.py @@ -0,0 +1,29 @@ +from unittest.mock import AsyncMock, MagicMock +from uuid import UUID + +import pytest + +from ai.backend.manager.api import vfolder +from ai.backend.manager.api.vfolder import with_vfolder_rows_resolved +from ai.backend.manager.models.vfolder import VFolderPermissionSetAlias + + +@pytest.mark.asyncio +async def test_uuid_or_name_resolution(monkeypatch: pytest.MonkeyPatch) -> None: + mock_resolver = AsyncMock() + monkeypatch.setattr(vfolder, "resolve_vfolder_rows", mock_resolver) + + @with_vfolder_rows_resolved(VFolderPermissionSetAlias.READABLE) # type: ignore + async def dummy_handler(request, row): + return + + mock_request = MagicMock() + mock_request.match_info = {"name": "8e33ca7f-9aa3-4f59-8bbb-526c212da98b"} + await dummy_handler(mock_request) + call = mock_resolver.await_args_list[0] + assert isinstance(call.args[2], UUID) + + mock_request.match_info = {"name": "hello"} + await dummy_handler(mock_request) + call = mock_resolver.await_args_list[1] + assert isinstance(call.args[2], str)