Update setting of read only fields and include deleted address spaces…

… when assigning new address spaces (#3714)

CHANGELOG.md

@@ -9,6 +9,8 @@ FEATURES:
 ENHANCEMENTS:
 
 BUG FIXES:
+* Fix issue where updates fail as read only is not configured consistently on schema fields ([#3691](https://github.com/microsoft/AzureTRE/issues/3691))
+* When geting avaialble address spaces allow those allocated to deleted workspaces to be reassigned ([#3691](https://github.com/microsoft/AzureTRE/issues/3691))
 * Update Python packages, and fix breaking changes ([#3764](https://github.com/microsoft/AzureTRE/issues/3764))
 * Enabling support for more than 20 users/groups in Workspace API ([#3759](https://github.com/microsoft/AzureTRE/pull/3759  ))
 * Airlock Import Review workspace uses dedicated DNS zone to prevent conflict with core ([#3767](https://github.com/microsoft/AzureTRE/pull/3767))

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.16.7"
+__version__ = "0.16.8"

api_app/api/routes/workspaces.py

@@ -7,7 +7,7 @@
 
 from api.dependencies.database import get_repository
 from api.dependencies.workspaces import get_operation_by_id_from_path, get_workspace_by_id_from_path, get_deployed_workspace_by_id_from_path, get_deployed_workspace_service_by_id_from_path, get_workspace_service_by_id_from_path, get_user_resource_by_id_from_path
-from db.errors import MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate, VersionDowngradeDenied
+from db.errors import InvalidInput, MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate, VersionDowngradeDenied
 from db.repositories.operations import OperationRepository
 from db.repositories.resource_templates import ResourceTemplateRepository
 from db.repositories.resources_history import ResourceHistoryRepository
@@ -107,6 +107,8 @@ async def create_workspace(workspace_create: WorkspaceInCreate, response: Respon
     except UserNotAuthorizedToUseTemplate as e:
         logging.exception("User not authorized to use template")
         raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e))
+    except InvalidInput as e:
+        raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e))
 
     operation = await save_and_deploy_resource(
         resource=workspace,

api_app/db/repositories/workspaces.py

@@ -140,11 +140,11 @@ async def validate_address_space(self, address_space):
         if (address_space is None):
             raise InvalidInput("Missing 'address_space' from properties.")
 
-        allocated_networks = [x.properties["address_space"] for x in await self.get_workspaces()]
+        allocated_networks = [x.properties["address_space"] for x in await self.get_active_workspaces()]
         return is_network_available(allocated_networks, address_space)
 
     async def get_new_address_space(self, cidr_netmask: int = 24):
-        workspaces = await self.get_workspaces()
+        workspaces = await self.get_active_workspaces()
         networks = [[x.properties.get("address_space")] for x in workspaces]
         networks = networks + [x.properties.get("address_spaces", []) for x in workspaces]
         networks = [i for s in networks for i in s if i is not None]

api_app/services/schema_service.py

@@ -50,9 +50,17 @@ def enrich_template(original_template, extra_properties, is_update: bool = False
     # this will help the UI render fields appropriately and know what it can send in a PATCH
     if is_update:
         for prop in template["properties"].values():
-            if "updateable" not in prop.keys() or prop["updateable"] is not True:
+            if not prop.get("updateable", False):
                 prop["readOnly"] = True
 
+        if "allOf" in template:
+            for conditional_property in template["allOf"]:
+                for condition in ["then", "else"]:
+                    if condition in conditional_property and "properties" in conditional_property[condition]:
+                        for prop in conditional_property[condition]["properties"].values():
+                            if not prop.get("updateable", False):
+                                prop["readOnly"] = True
+
     # if there is an 'allOf' property which is empty, the validator fails - so remove the key
     if "allOf" in template and template["allOf"] is None:
         template.pop("allOf")

api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py

@@ -209,29 +209,29 @@ async def test_get_address_space_based_on_size_with_custom_address_space_and_mis
 
 
 @pytest.mark.asyncio
-@patch('db.repositories.workspaces.WorkspaceRepository.get_workspaces')
+@patch('db.repositories.workspaces.WorkspaceRepository.get_active_workspaces')
 @patch('core.config.RESOURCE_LOCATION', "useast2")
 @patch('core.config.TRE_ID', "9876")
 @patch('core.config.CORE_ADDRESS_SPACE', "10.1.0.0/22")
 @patch('core.config.TRE_ADDRESS_SPACE', "10.0.0.0/12")
-async def test_get_address_space_based_on_size_with_address_space_only(get_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
+async def test_get_address_space_based_on_size_with_address_space_only(get_active_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
     workspace_with_address_space = copy.deepcopy(workspace)
     workspace_with_address_space.properties["address_space"] = "10.1.4.0/24"
 
-    get_workspaces_mock.return_value = [workspace_with_address_space]
+    get_active_workspaces_mock.return_value = [workspace_with_address_space]
     workspace_to_create = basic_workspace_request
     address_space = await workspace_repo.get_address_space_based_on_size(workspace_to_create.properties)
 
     assert "10.1.5.0/24" == address_space
 
 
 @pytest.mark.asyncio
-@patch('db.repositories.workspaces.WorkspaceRepository.get_workspaces')
+@patch('db.repositories.workspaces.WorkspaceRepository.get_active_workspaces')
 @patch('core.config.RESOURCE_LOCATION', "useast2")
 @patch('core.config.TRE_ID', "9876")
 @patch('core.config.CORE_ADDRESS_SPACE', "10.1.0.0/22")
 @patch('core.config.TRE_ADDRESS_SPACE', "10.0.0.0/12")
-async def test_get_address_space_based_on_size_with_address_space_and_address_spaces(get_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
+async def test_get_address_space_based_on_size_with_address_space_and_address_spaces(get_active_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
     workspace_with_address_space = copy.deepcopy(workspace)
     workspace_with_address_space.properties["address_space"] = "10.1.4.0/24"
 
@@ -242,7 +242,7 @@ async def test_get_address_space_based_on_size_with_address_space_and_address_sp
     workspace_with_both.properties["address_spaces"] = ["10.1.7.0/24", "10.1.8.0/24"]
     workspace_with_both.properties["address_space"] = "10.1.7.0/24"
 
-    get_workspaces_mock.return_value = [workspace_with_address_space, workspace_with_address_spaces, workspace_with_both]
+    get_active_workspaces_mock.return_value = [workspace_with_address_space, workspace_with_address_spaces, workspace_with_both]
     workspace_to_create = basic_workspace_request
     address_space = await workspace_repo.get_address_space_based_on_size(workspace_to_create.properties)
 

templates/workspaces/airlock-import-review/template_schema.json

@@ -94,13 +94,15 @@
           "client_id": {
             "type": "string",
             "title": "Application (Client) ID",
-            "description": "The AAD Application Registration ID for the workspace."
+            "description": "The AAD Application Registration ID for the workspace.",
+            "updateable": true
           },
           "client_secret": {
             "type": "string",
             "title": "Application (Client) Secret",
             "description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
-            "sensitive": true
+            "sensitive": true,
+            "updateable": true
           }
         },
         "required": [
@@ -113,12 +115,14 @@
             "type": "boolean",
             "title": "Create AAD Groups for each workspace role",
             "description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.",
-            "default": false
+            "default": false,
+            "updateable": true
           },
           "aad_redirect_uris": {
             "type": "array",
             "title": "AAD Redirect URIs",
             "description": "Redirect URIs for the AAD app in Automatic Auth mode",
+            "updateable": true,
             "items": {
               "title": "items",
               "type": "object",

templates/workspaces/base/porter.yaml

@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-workspace-base
-version: 1.5.1
+version: 1.5.3
 description: "A base Azure TRE workspace"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

templates/workspaces/base/template_schema.json

@@ -81,7 +81,8 @@
             "type": "boolean",
             "title": "Configure Review VMs",
             "description": "Allow TRE to automatically create and delete review VMs for airlock approvals",
-            "default": false
+            "default": false,
+            "updateable": true
           }
         }
       }
@@ -108,6 +109,7 @@
             "title": "Airlock Review Config",
             "default": null,
             "description": "Configuration for Airlock Review feature. Needs to be set up after workspace creation",
+            "updateable": true,
             "properties": {
               "import": {
                 "title": "Import Review Settings",
@@ -206,13 +208,15 @@
           "client_id": {
             "type": "string",
             "title": "Application (Client) ID",
-            "description": "The AAD Application Registration ID for the workspace."
+            "description": "The AAD Application Registration ID for the workspace.",
+            "updateable": true
           },
           "client_secret": {
             "type": "string",
             "title": "Application (Client) Secret",
             "description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
-            "sensitive": true
+            "sensitive": true,
+            "updateable": true
           }
         },
         "required": [
@@ -225,12 +229,14 @@
             "type": "boolean",
             "title": "Create AAD Groups for each workspace role",
             "description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.",
-            "default": false
+            "default": false,
+            "updateable": true
           },
           "aad_redirect_uris": {
             "type": "array",
             "title": "AAD Redirect URIs",
             "description": "Redirect URIs for the AAD app in Automatic Auth mode",
+            "updateable": true,
             "items": {
               "title": "items",
               "type": "object",

templates/workspaces/unrestricted/porter.yaml

@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-workspace-unrestricted
-version: 0.11.1
+version: 0.11.4
 description: "A base Azure TRE workspace"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

templates/workspaces/unrestricted/template_schema.json

@@ -206,13 +206,15 @@
           "client_id": {
             "type": "string",
             "title": "Application (Client) ID",
-            "description": "The AAD Application Registration ID for the workspace."
+            "description": "The AAD Application Registration ID for the workspace.",
+            "updateable": true
           },
           "client_secret": {
             "type": "string",
             "title": "Application (Client) Secret",
             "description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
-            "sensitive": true
+            "sensitive": true,
+            "updateable": true
           }
         },
         "required": [
@@ -225,12 +227,14 @@
             "type": "boolean",
             "title": "Create AAD Groups for each workspace role",
             "description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.",
-            "default": false
+            "default": false,
+            "updateable": true
           },
           "aad_redirect_uris": {
             "type": "array",
             "title": "AAD Redirect URIs",
             "description": "Redirect URIs for the AAD app in Automatic Auth mode",
+            "updateable": true,
             "items": {
               "title": "items",
               "type": "object",