Airlock UI: support multiple review vms

CHANGELOG.md

@@ -2,6 +2,11 @@
 ## 0.8.0 (Unreleased)
 
 **BREAKING CHANGES & MIGRATIONS**:
+* The model for `reviewUserResources` in airlock requests has changed from being a list to a dictionary. A migration has been added to update your existing requests automatically; please make sure you run the migrations as part of updating your API and UI.
+  * Note that any in-flight requests that have review resources deployed will show `UNKNOWN[i]` for the user key of that resource and in the UI users will be prompted to deploy a new resource. [#2883](https://github.com/microsoft/AzureTRE/pull/2883)
+
+FEATURES:
+* Support review VMs for multiple reviewers for each airlock request [#2883](https://github.com/microsoft/AzureTRE/pull/2883)
 
 ENHANCEMENTS:
 * Remove Porter's Docker mixin as it's not in use ([#2889](https://github.com/microsoft/AzureTRE/pull/2889))

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.5.21"
+__version__ = "0.5.22"

api_app/api/routes/airlock_resource_helpers.py

@@ -7,6 +7,7 @@
 from starlette import status
 
 from api.routes.resource_helpers import send_uninstall_message
+from models.domain.user_resource import UserResource
 from db.repositories.airlock_requests import AirlockRequestRepository
 from db.repositories.resource_templates import ResourceTemplateRepository
 from db.repositories.user_resources import UserResourceRepository
@@ -73,8 +74,9 @@ async def update_and_publish_event_airlock_request(
     except Exception as e:
         logging.error(f'Failed updating airlock_request item {airlock_request}: {e}')
         # If the validation failed, the error was not related to the saving itself
-        if e.status_code == 400:
-            raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strings.AIRLOCK_REQUEST_ILLEGAL_STATUS_CHANGE)
+        if hasattr(e, 'status_code'):
+            if e.status_code == 400:
+                raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strings.AIRLOCK_REQUEST_ILLEGAL_STATUS_CHANGE)
         raise HTTPException(status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=strings.STATE_STORE_ENDPOINT_NOT_RESPONDING)
 
     if not new_status:
@@ -140,39 +142,59 @@ def enrich_requests_with_allowed_actions(requests: List[AirlockRequest], user: U
     return enriched_requests
 
 
-async def delete_review_user_resources(
+async def delete_review_user_resource(
+        user_resource: UserResource,
+        user_resource_repo: UserResourceRepository,
+        workspace_service_repo: WorkspaceServiceRepository,
+        resource_template_repo: ResourceTemplateRepository,
+        operations_repo: OperationRepository,
+        user: User) -> Operation:
+    workspace_service = workspace_service_repo.get_workspace_service_by_id(workspace_id=user_resource.workspaceId,
+                                                                           service_id=user_resource.parentWorkspaceServiceId)
+
+    resource_template = resource_template_repo.get_template_by_name_and_version(
+        user_resource.templateName,
+        user_resource.templateVersion,
+        ResourceType.UserResource,
+        workspace_service.templateName)
+
+    logging.info(f"Deleting user resource {user_resource.id} in workspace service {workspace_service.id}")
+    operation = await send_uninstall_message(
+        resource=user_resource,
+        resource_repo=user_resource_repo,
+        operations_repo=operations_repo,
+        resource_type=ResourceType.UserResource,
+        resource_template_repo=resource_template_repo,
+        user=user,
+        resource_template=resource_template)
+    logging.info(f"Started operation {operation}")
+    return operation
+
+
+async def delete_all_review_user_resources(
         airlock_request: AirlockRequest,
         user_resource_repo: UserResourceRepository,
         workspace_service_repo: WorkspaceServiceRepository,
         resource_template_repo: ResourceTemplateRepository,
         operations_repo: OperationRepository,
         user: User) -> List[Operation]:
     operations: List[Operation] = []
-    for review_ur in airlock_request.reviewUserResources:
+    for review_ur in airlock_request.reviewUserResources.values():
         user_resource = user_resource_repo.get_user_resource_by_id(
             workspace_id=review_ur.workspaceId,
             service_id=review_ur.workspaceServiceId,
             resource_id=review_ur.userResourceId
         )
 
-        workspace_service = workspace_service_repo.get_workspace_service_by_id(workspace_id=user_resource.workspaceId, service_id=user_resource.parentWorkspaceServiceId)
-
-        resource_template = resource_template_repo.get_template_by_name_and_version(
-            user_resource.templateName,
-            user_resource.templateVersion,
-            ResourceType.UserResource,
-            workspace_service.templateName)
-
-        logging.info(f"Deleting user resource {user_resource.id} in workspace service {workspace_service.id}")
-        operations.append(await send_uninstall_message(
-            resource=user_resource,
-            resource_repo=user_resource_repo,
-            operations_repo=operations_repo,
-            resource_type=ResourceType.UserResource,
+        operation = await delete_review_user_resource(
+            user_resource=user_resource,
+            user_resource_repo=user_resource_repo,
+            workspace_service_repo=workspace_service_repo,
             resource_template_repo=resource_template_repo,
-            user=user,
-            resource_template=resource_template))
-        logging.info(f"Started operation {operations[-1]}")
+            operations_repo=operations_repo,
+            user=user
+        )
+        operations.append(operation)
 
     logging.info(f"Started {len(operations)} operations on deleting user resources")
     return operations

api_app/api/routes/migrations.py

@@ -65,6 +65,10 @@ async def migrate_database(resources_repo=Depends(get_repository(ResourceReposit
         num_updated = airlock_migration.add_created_by_and_rename_in_history()
         migrations.append(Migration(issueNumber="2779", status=f'Renamed fields & updated {num_updated} airlock requests with createdBy'))
 
+        logging.info("PR 2883 - Support multiple reviewer VMs per Airlock request")
+        num_updated = airlock_migration.change_review_resources_to_dict()
+        migrations.append(Migration(issueNumber="XXXX", status=f'Updated {num_updated} airlock requests with new reviewUserResources format'))
+
         return MigrationOutList(migrations=migrations)
     except Exception as e:
         logging.error("Failed to migrate database: %s", e, exc_info=True)

api_app/db/migrations/airlock.py

@@ -28,6 +28,22 @@ def add_created_by_and_rename_in_history(self) -> int:
                 request['createdBy'] = request['updatedBy']
 
             self.update_item_dict(request)
-            num_updated = num_updated + 1
+            num_updated += 1
+
+        return num_updated
+
+    def change_review_resources_to_dict(self) -> int:
+        num_updated = 0
+        for request in self.container.read_all_items():
+            # Only migrate if airlockReviewResources property present and is a list
+            if 'reviewUserResources' in request:
+                if type(request['reviewUserResources']) == list:
+                    updated_review_resources = {}
+                    for i, resource in enumerate(request['reviewUserResources']):
+                        updated_review_resources['UNKNOWN' + str(i)] = resource
+
+                    request['reviewUserResources'] = updated_review_resources
+                    self.update_item_dict(request)
+                    num_updated += 1
 
         return num_updated

api_app/db/repositories/airlock_requests.py

@@ -152,7 +152,8 @@ def update_airlock_request(
             request_files=request_files,
             status_message=status_message,
             airlock_review=airlock_review,
-            review_user_resource=review_user_resource)
+            review_user_resource=review_user_resource,
+            updated_by=updated_by)
         try:
             db_response = self.update_airlock_request_item(original_request, updated_request, updated_by, {"previousStatus": original_request.status})
         except CosmosAccessConditionFailedError:
@@ -187,7 +188,8 @@ def _build_updated_request(
             request_files: List[AirlockFile] = None,
             status_message: str = None,
             airlock_review: AirlockReview = None,
-            review_user_resource: AirlockReviewUserResource = None) -> AirlockRequest:
+            review_user_resource: AirlockReviewUserResource = None,
+            updated_by: User = None) -> AirlockRequest:
         updated_request = copy.deepcopy(original_request)
 
         if new_status is not None:
@@ -207,10 +209,7 @@ def _build_updated_request(
                 updated_request.reviews.append(airlock_review)
 
         if review_user_resource is not None:
-            if updated_request.reviewUserResources is None:
-                updated_request.reviewUserResources = [review_user_resource]
-            else:
-                updated_request.reviewUserResources.append(review_user_resource)
+            updated_request.reviewUserResources[updated_by.id] = review_user_resource
 
         return updated_request
 

api_app/models/domain/airlock_request.py

@@ -1,9 +1,10 @@
-from typing import List
 from enum import Enum
+from typing import List, Dict
+
+from models.domain.azuretremodel import AzureTREModel
 from pydantic import Field, validator
 from pydantic.schema import Optional
 from resources import strings
-from models.domain.azuretremodel import AzureTREModel
 
 
 class AirlockRequestStatus(str, Enum):
@@ -95,7 +96,7 @@ class AirlockRequest(AzureTREModel):
     statusMessage: Optional[str] = Field(title="Optional - contains additional information about the current status.")
     reviews: Optional[List[AirlockReview]]
     etag: Optional[str] = Field(title="_etag", alias="_etag")
-    reviewUserResources: List[AirlockReviewUserResource] = Field([], title="User resources created for Airlock Reviews")
+    reviewUserResources: Dict[str, AirlockReviewUserResource] = Field({}, title="User resources created for Airlock Reviews")
 
     # SQL API CosmosDB saves ETag as an escaped string: https://github.com/microsoft/AzureTRE/issues/1931
     @validator("etag", pre=True)

api_app/models/domain/user_resource.py

@@ -5,10 +5,10 @@
 
 class UserResource(Resource):
     """
-    Workspace service request
+    User resource
     """
     workspaceId: str = Field("", title="Workspace ID", description="Service target Workspace id")
     ownerId: str = Field("", title="Owner of the user resource")
     parentWorkspaceServiceId: str = Field("", title="Parent Workspace Service ID", description="Service target Workspace Service id")
-    azureStatus: dict = Field({}, title="Azure Status", description="Azure status, varies per user resoruce")
+    azureStatus: dict = Field({}, title="Azure Status", description="Azure status, varies per user resource")
     resourceType = ResourceType.UserResource

api_app/models/schemas/airlock_request.py

@@ -48,7 +48,7 @@ class AirlockRequestInResponse(BaseModel):
     class Config:
         schema_extra = {
             "example": {
-                "airlock_request": get_sample_airlock_request("933ad738-7265-4b5f-9eae-a1a62928772e", "121e921f-a4aa-44b3-90a9-e8da030495ef")
+                "airlockRequest": get_sample_airlock_request("933ad738-7265-4b5f-9eae-a1a62928772e", "121e921f-a4aa-44b3-90a9-e8da030495ef")
             }
         }
 

api_app/service_bus/resource_request_sender.py

@@ -1,6 +1,5 @@
 import json
 
-
 from db.repositories.resources import ResourceRepository
 from db.repositories.resource_templates import ResourceTemplateRepository
 from service_bus.helpers import send_deployment_message, update_resource_for_step