Conditional Templates

CHANGELOG.md

@@ -23,6 +23,7 @@ ENHANCEMENTS:
 * Show error message when Review VMs are not configured in the current workspace
 * CLI: Add missing endpoints and minor bug fixes (#2784)
 * Added optional parameter to allow a client to retrieve a template by name and version ([#2802](https://github.com/microsoft/AzureTRE/pull/2802))
+* Added support for `allOf` usage in Resource Templates - both across the API and the UI. This allows a template author to specify certain fields as being conditionally present / conditionally required, and means we can tidy up some of the resource creation forms substantially. ([#2795](https://github.com/microsoft/AzureTRE/pull/2795))
 
 BUG FIXES:
 * Show the correct createdBy value for airlock requests in UI and in API queries ([#2779](https://github.com/microsoft/AzureTRE/pull/2779))

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.5.10"
+__version__ = "0.5.11"

api_app/api/routes/airlock.py

@@ -130,13 +130,15 @@ async def create_review_user_resource(
         # Getting the review configuration from the airlock request's workspace properties
         if airlock_request.type == AirlockRequestType.Import:
             config = workspace.properties["airlock_review_config"]["import"]
-            workspace_id = config["workspace_id"]
+            workspace_id = config["import_vm_workspace_id"]
+            workspace_service_id = config["import_vm_workspace_service_id"]
+            user_resource_template_name = config["import_vm_user_resource_template_name"]
         else:
             assert airlock_request.type == AirlockRequestType.Export
             config = workspace.properties["airlock_review_config"]["export"]
             workspace_id = workspace.id
-        workspace_service_id = config["workspace_service_id"]
-        user_resource_template_name = config["user_resource_template_name"]
+            workspace_service_id = config["export_vm_orkspace_service_id"]
+            user_resource_template_name = config["export_vm_user_resource_template_name"]
 
         logging.info(f"Going to create a user resource in {workspace_id} {workspace_service_id} {user_resource_template_name}")
     except (KeyError, TypeError) as e:

api_app/db/repositories/resource_templates.py

@@ -116,6 +116,9 @@ def create_template(self, template_input: ResourceTemplateInCreate, resource_typ
         if "pipeline" in template_input.json_schema:
             template["pipeline"] = template_input.json_schema["pipeline"]
 
+        if "allOf" in template_input.json_schema:
+            template["allOf"] = template_input.json_schema["allOf"]
+
         if resource_type == ResourceType.UserResource:
             template["parentWorkspaceService"] = parent_service_name
             template = parse_obj_as(UserResourceTemplate, template)

api_app/db/repositories/workspaces.py

@@ -98,7 +98,7 @@ def get_workspace_owner(self, workspace_properties: dict, workspace_owner_object
         return workspace_owner_object_id if user_defined_workspace_owner_object_id is None else user_defined_workspace_owner_object_id
 
     def automatically_create_application_registration(self, workspace_properties: dict) -> bool:
-        return True if workspace_properties["client_id"] == "auto_create" else False
+        return True if ("auth_type" in workspace_properties and workspace_properties["auth_type"] == "Automatic") else False
 
     def get_address_space_based_on_size(self, workspace_properties: dict):
         # Default the address space to 'small' if not supplied.

api_app/models/domain/resource_template.py

@@ -68,10 +68,11 @@ class ResourceTemplate(AzureTREModel):
     required: List[str] = Field(title="List of properties which must be provided")
     authorizedRoles: Optional[List[str]] = Field(default=[], title="If not empty, the user is required to have one of these roles to install the template")
     properties: Dict[str, Property] = Field(title="Template properties")
+    allOf: Optional[List[dict]] = Field(default=None, title="All Of", description="Used for conditionally showing and validating fields")
     actions: List[CustomAction] = Field(default=[], title="Template actions")
     customActions: List[CustomAction] = Field(default=[], title="Template custom actions")
     pipeline: Optional[Pipeline] = Field(default=None, title="Template pipeline to define updates to other resources")
     uiSchema: Optional[dict] = Field(default={}, title="Dict containing a uiSchema object, if any")
 
     # setting this to false means if extra, unexpected fields are supplied, the request is invalidated
-    additionalProperties: bool = Field(default=False, title="Prevent unspecified properties being applied")
+    unevaluatedProperties: bool = Field(default=False, title="Prevent unspecified properties being applied")

api_app/models/schemas/workspace.py

@@ -82,6 +82,7 @@ class Config:
                 "properties": {
                     "display_name": "the workspace display name",
                     "description": "workspace description",
+                    "auth_type": "Manual",
                     "client_id": "<WORKSPACE_CLIENT_ID>",
                     "client_secret": "<WORKSPACE_CLIENT_SECRET>",
                     "address_space_size": "small"

api_app/schemas/azuread.json

@@ -5,19 +5,8 @@
     "title": "Azure AD Authorisation Schema",
     "default": {},
     "required": [
-        "client_id"
     ],
     "properties": {
-        "client_id": {
-            "type": "string",
-            "title": "Application (Client) ID",
-            "description": "The AAD Application Registration ID for the workspace. Use 'auto_create' if you wish TRE to create this."
-        },
-        "client_secret": {
-          "type": "string",
-          "title": "Application (Client) Secret",
-          "description": "The AAD Application Registration secret for the workspace. Leave blank if using `auto_create` above. This value will be stored in the Workspace Key Vault.",
-          "sensitive": true
-      }
+
     }
 }

api_app/schemas/workspace.json

@@ -27,22 +27,6 @@
       "title": "Workspace Overview",
       "description": "Long form description of the workspace, in markdown syntax.",
       "updateable": true
-    },
-    "address_space_size": {
-      "type": "string",
-      "title": "Address space size",
-      "description": "Network address size (small, medium, large or custom) to be used by the workspace.",
-      "enum": [
-        "small",
-        "medium",
-        "large",
-        "custom"
-      ]
-    },
-    "address_space": {
-      "type": "string",
-      "title": "Address space",
-      "description": "Network address space to be used by the workspace if 'Address space size' is custom."
     }
   }
 }

api_app/services/aad_authentication.py

@@ -55,8 +55,11 @@ async def __call__(self, request: Request) -> User:
             # as we have a workspace_id not given, try decoding token
             logging.debug("Workspace ID was provided. Getting Workspace API app registration")
             try:
+                # get the app reg id - which might be blank if the workspace hasn't fully created yet.
+                # if it's blank, don't use workspace auth, use core auth - and a TRE Admin can still get it
                 app_reg_id = self._fetch_ws_app_reg_id_from_ws_id(request)
-                decoded_token = self._decode_token(token, app_reg_id)
+                if app_reg_id != "":
+                    decoded_token = self._decode_token(token, app_reg_id)
             except HTTPException as h:
                 raise h
             except Exception as e:
@@ -112,7 +115,10 @@ def _fetch_ws_app_reg_id_from_ws_id(request: Request) -> str:
             workspace_id = request.path_params['workspace_id']
             ws_repo = WorkspaceRepository(get_db_client_from_request(request))
             workspace = ws_repo.get_workspace_by_id(workspace_id)
-            ws_app_reg_id = workspace.properties['client_id']
+
+            ws_app_reg_id = ""
+            if "client_id" in workspace.properties:
+                ws_app_reg_id = workspace.properties['client_id']
 
             return ws_app_reg_id
         except EntityDoesNotExist as e:
@@ -294,7 +300,7 @@ def _get_batch_users_by_role_assignments_body(self, roles_graph_data):
 
     # This method is called when you create a workspace and you already have an AAD App Registration
     # to link it to. You pass in the client_id and go and get the extra information you need from AAD
-    # If the client_id is `auto_create`, then these values will be written by Terraform.
+    # If the auth_type is `Automatic`, then these values will be written by Terraform.
     def _get_app_auth_info(self, client_id: str) -> dict:
         graph_data = self._get_app_sp_graph_data(client_id)
         if 'value' not in graph_data or len(graph_data['value']) == 0:
@@ -363,13 +369,13 @@ def _get_identity_type(self, id: str) -> str:
         return object_info["@odata.type"]
 
     def extract_workspace_auth_information(self, data: dict) -> dict:
-        if "client_id" not in data:
+        if ("auth_type" not in data) or (data["auth_type"] != "Automatic" and "client_id" not in data):
             raise AuthConfigValidationError(strings.ACCESS_PLEASE_SUPPLY_CLIENT_ID)
 
         auth_info = {}
         # The user may want us to create the AAD workspace app and therefore they
         # don't know the client_id yet.
-        if data["client_id"] != "auto_create":
+        if data["auth_type"] != "Automatic":
             auth_info = self._get_app_auth_info(data["client_id"])
 
             # Check we've get all our required roles

api_app/services/schema_service.py

@@ -53,6 +53,10 @@ def enrich_template(original_template, extra_properties, is_update: bool = False
             if "updateable" not in prop.keys() or prop["updateable"] is not True:
                 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")
+
     if is_workspace_scope:
         id_field = "workspace_id"
     else:

api_app/tests_ma/test_api/test_routes/test_airlock.py

@@ -105,13 +105,13 @@ def sample_airlock_review_config() -> dict:
     return {
         "airlock_review_config": {
             "import": {
-                "workspace_id": IMPORT_WORKSPACE_ID,
-                "workspace_service_id": WORKSPACE_SERVICE_ID,
-                "user_resource_template_name": "tre-service-guacamole-import-reviewvm"
+                "import_vm_workspace_id": IMPORT_WORKSPACE_ID,
+                "import_vm_workspace_service_id": WORKSPACE_SERVICE_ID,
+                "import_vm_user_resource_template_name": "tre-service-guacamole-import-reviewvm"
             },
             "export": {
-                "workspace_service_id": WORKSPACE_SERVICE_ID,
-                "user_resource_template_name": "tre-service-guacamole-export-reviewvm"
+                "export_vm_workspace_service_id": WORKSPACE_SERVICE_ID,
+                "export_vm_user_resource_template_name": "tre-service-guacamole-export-reviewvm"
             }
         }
     }

api_app/tests_ma/test_db/test_repositories/test_resource_repository.py

@@ -164,15 +164,22 @@ def test_validate_input_against_template_raises_value_error_if_the_user_resource
 
 @patch("db.repositories.resources.ResourceRepository._get_enriched_template")
 def test_validate_input_against_template_raises_value_error_if_payload_is_invalid(enriched_template_mock, resource_repo, workspace_input):
-    enriched_template_mock.return_value = ResourceTemplate(id="123",
-                                                           name="template1",
-                                                           description="description",
-                                                           version="0.1.0",
-                                                           resourceType=ResourceType.Workspace,
-                                                           current=True,
-                                                           required=["display_name"],
-                                                           properties={},
-                                                           customActions=[]).dict()
+    template_dict = ResourceTemplate(
+        id="123",
+        name="template1",
+        description="description",
+        version="0.1.0",
+        resourceType=ResourceType.Workspace,
+        current=True,
+        required=["display_name"],
+        properties={},
+        customActions=[]).dict()
+
+    # the enrich template method does this
+    template_dict.pop("allOf")
+
+    enriched_template_mock.return_value = template_dict
+
     # missing display name
     workspace_input = WorkspaceInCreate(templateName="template1")
 

api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py

@@ -194,7 +194,7 @@ def test_create_workspace_item_raises_value_error_if_template_is_invalid(validat
 
 
 def test_automatically_create_application_registration_returns_true(workspace_repo):
-    dictToTest = {"client_id": "auto_create"}
+    dictToTest = {"auth_type": "Automatic"}
 
     assert workspace_repo.automatically_create_application_registration(dictToTest) is True
 

api_app/tests_ma/test_services/test_aad_access_service.py

@@ -10,7 +10,7 @@
 def test_extract_workspace__raises_error_if_client_id_not_available():
     access_service = AzureADAuthorization()
     with pytest.raises(AuthConfigValidationError):
-        access_service.extract_workspace_auth_information(data={})
+        access_service.extract_workspace_auth_information(data={"auth_type": "Manual"})
 
 
 @patch("services.aad_authentication.AzureADAuthorization._get_app_auth_info",
@@ -62,7 +62,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock):
     }
 
     access_service = AzureADAuthorization()
-    actual_auth_info = access_service.extract_workspace_auth_information(data={"client_id": "1234"})
+    actual_auth_info = access_service.extract_workspace_auth_information(data={"auth_type": "Manual", "client_id": "1234"})
 
     assert actual_auth_info == expected_auth_info
 

e2e_tests/test_airlock.py

@@ -39,6 +39,7 @@ async def test_airlock_flow(verify) -> None:
                 "display_name": "E2E test airlock flow",
                 "description": "workspace for E2E airlock flow",
                 "address_space_size": "small",
+                "auth_type": "Manual",
                 "client_id": f"{config.TEST_WORKSPACE_APP_ID}",
                 "client_secret": f"{config.TEST_WORKSPACE_APP_SECRET}",
             }

e2e_tests/test_performance.py

@@ -26,6 +26,7 @@ async def test_parallel_resource_creations(verify) -> None:
                 "display_name": f'Perf Test Workspace {i}',
                 "description": "workspace for perf test",
                 "address_space_size": "small",
+                "auth_type": "Manual",
                 "client_id": f"{config.TEST_WORKSPACE_APP_ID}"
             }
         }
@@ -67,6 +68,7 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(verify) ->
                 "display_name": "E2E test guacamole service",
                 "description": "",
                 "address_space_size": "small",
+                "auth_type": "Manual",
                 "client_id": f"{config.TEST_WORKSPACE_APP_ID}",
                 "client_secret": f"{config.TEST_WORKSPACE_APP_SECRET}"
             }

e2e_tests/test_workspace_services.py

@@ -20,6 +20,7 @@ async def test_create_guacamole_service_into_base_workspace(verify) -> None:
             "display_name": "E2E test guacamole service",
             "description": "workspace for E2E",
             "address_space_size": "small",
+            "auth_type": "Manual",
             "client_id": f"{config.TEST_WORKSPACE_APP_ID}",
             "client_secret": f"{config.TEST_WORKSPACE_APP_SECRET}",
         }
@@ -83,7 +84,7 @@ async def test_create_guacamole_service_into_aad_workspace(verify) -> None:
             "display_name": "E2E test guacamole service",
             "description": "workspace for E2E AAD",
             "address_space_size": "small",
-            "client_id": "auto_create"
+            "auth_type": "Automatic"
         }
     }
     if config.TEST_WORKSPACE_APP_PLAN != "":

templates/workspaces/airlock-import-review/porter.yaml

@@ -1,6 +1,6 @@
 ---
 name: tre-workspace-airlock-import-review
-version: 0.4.0
+version: 0.5.0
 description: "A workspace to do Airlock Data Import Reviews for Azure TRE"
 dockerfile: Dockerfile.tmpl
 registry: azuretre
@@ -60,6 +60,7 @@ parameters:
     type: string
     description: "The object id of the user that will be granted WorkspaceOwner after it is created."
   - name: client_id
+    default: ""
     type: string
     description:
       "The client id of the workspace in the identity provider. This value is typically provided to you