fix: Raise explicit error if multiple entries found when deleting a v…

…folder (#492)

* Add `TooManyVFoldersFound` API error class
* Raise the error when getting multiple entries from querying vfolder to delete

changes/492.fix.md

@@ -0,0 +1 @@
+Do not delete a virtual folder if there are other folders with the same name (in other folder hosts) and handle by new relevant exception, `TooManyVFoldersFound`, rather than blindly and dangerously deleting the first-queried one.

src/ai/backend/manager/api/exceptions.py

@@ -256,6 +256,11 @@ class VFolderCreationFailed(BackendError, web.HTTPBadRequest):
     error_title = 'Virtual folder creation has failed.'
 
 
+class TooManyVFoldersFound(BackendError, web.HTTPNotFound):
+    error_type = 'https://api.backend.ai/probs/too-many-vfolders'
+    error_title = 'There are two or more matching vfolders.'
+
+
 class VFolderNotFound(ObjectNotFound):
     object_name = 'virtual folder'
 

src/ai/backend/manager/api/vfolder.py

@@ -54,7 +54,7 @@
 )
 from .auth import admin_required, auth_required, superadmin_required
 from .exceptions import (
-    VFolderCreationFailed, VFolderNotFound, VFolderAlreadyExists,
+    VFolderCreationFailed, TooManyVFoldersFound, VFolderNotFound, VFolderAlreadyExists,
     GenericForbidden, ObjectNotFound, InvalidAPIParameters, ServerMisconfiguredError,
     BackendAgentError, InternalServerError, GroupNotFound,
 )
@@ -1608,20 +1608,38 @@ async def delete(request: web.Request) -> web.Response:
             user_role=user_role,
             domain_name=domain_name,
             allowed_vfolder_types=allowed_vfolder_types,
+            extra_vf_conds=(vfolders.c.name == folder_name),
         )
-        for entry in entries:
-            if entry['name'] == folder_name:
-                # 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.')
-                break
-        else:
+        # for entry in entries:
+        #     if entry['name'] == folder_name:
+        #         # 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.')
+        #         break
+        # FIXME: For now, multiple entries on delete vfolder will raise an error. Will be fixed in 22.06
+        if len(entries) > 1:
+            log.error('VFOLDER.DELETE(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('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.')
         folder_host = entry['host']
         folder_id = entry['id']
         query = (sa.delete(vfolders).where(vfolders.c.id == folder_id))