Repository: microsoft/AzureTRE (python)

Issue:
Pipeline substitution only works when values are marked as updatable so not for install.
When passing values into a resource in a pipeline, the code only allows input for values marked as updatable. This means if need to pass values into the app at install it does not work.

For example `subscription_id` into a workspace, needed by #1073 cc @askew 

And `log_analytics_workspace_name` as needed by #4341 

PR to come.

Pull request:
Add ability to pass values to install stage on pipleine.
Fixes #4450

This allows values to be validated when the action is `install`.

It feels wrong using patch methods for install rather than update, but the database operation is a patch...

It is needed to enable:

For example subscription_id into a workspace, needed by https://github.com/microsoft/AzureTRE/issues/1073 cc @askew

And log_analytics_workspace_name as needed by https://github.com/microsoft/AzureTRE/pull/4341

https://github.com/microsoft/AzureTRE/blob/835891870427718eeac92de154d8ca9121439c07/templates/workspace_services/azureml/template_schema.json#L59-L75


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index efb08fd1..67b26ec1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 **BREAKING CHANGES & MIGRATIONS**:
 
 ENHANCEMENTS:
+* Add ability to pass values to install stage on pipleine [#4451](https://github.com/microsoft/AzureTRE/pull/4451)
 * Format the error message in the Operations panel for enhanced readability ([#4493](https://github.com/microsoft/AzureTRE/issues/4493))
 
 BUG FIXES:
diff --git a/api_app/_version.py b/api_app/_version.py
index 8b301a77..770bb8d0 100644
--- a/api_app/_version.py
+++ b/api_app/_version.py
@@ -1,1 +1,1 @@
-__version__ = "0.23.0"
+__version__ = "0.24.0"
diff --git a/api_app/api/routes/resource_helpers.py b/api_app/api/routes/resource_helpers.py
index 34cc5006..d3dff659 100644
--- a/api_app/api/routes/resource_helpers.py
+++ b/api_app/api/routes/resource_helpers.py
@@ -52,7 +52,7 @@ async def cascaded_update_resource(resource_patch: ResourcePatch, parent_resourc
             primary_parent_service_name = primary_parent_workspace_service.templateName
 
         child_resource_template = await resource_template_repo.get_template_by_name_and_version(child_resource.templateName, child_resource.templateVersion, child_resource.resourceType, parent_service_name=primary_parent_service_name)
-        await resource_repo.patch_resource(child_resource, resource_patch, child_resource_template, child_etag, resource_template_repo, resource_history_repo, user, force_version_update)
+        await resource_repo.patch_resource(child_resource, resource_patch, child_resource_template, child_etag, resource_template_repo, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE, force_version_update)
 
 
 async def save_and_deploy_resource(
diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py
index e1753889..04596988 100644
--- a/api_app/db/repositories/resources.py
+++ b/api_app/db/repositories/resources.py
@@ -4,6 +4,7 @@ from datetime import datetime
 from typing import Optional, Tuple, List
 
 from azure.cosmos.exceptions import CosmosResourceNotFoundError
+from resources.strings import RESOURCE_ACTION_INSTALL
 from core import config
 from db.errors import VersionDowngradeDenied, EntityDoesNotExist, MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate
 from db.repositories.resources_history import ResourceHistoryRepository
@@ -100,7 +101,7 @@ class ResourceRepository(BaseRepository):
 
         return parse_obj_as(ResourceTemplate, template)
 
-    async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch, resource_template: ResourceTemplate, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, force_version_update: bool = False) -> Tuple[Resource, ResourceTemplate]:
+    async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch, resource_template: ResourceTemplate, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_action: str, force_version_update: bool = False) -> Tuple[Resource, ResourceTemplate]:
         await resource_history_repo.create_resource_history_item(resource)
         # now update the resource props
         resource.resourceVersion = resource.resourceVersion + 1
@@ -115,7 +116,7 @@ class ResourceRepository(BaseRepository):
             resource.templateVersion = resource_patch.templateVersion
 
         if resource_patch.properties is not None and len(resource_patch.properties) > 0:
-            self.validate_patch(resource_patch, resource_template_repo, resource_template)
+            self.validate_patch(resource_patch, resource_template_repo, resource_template, resource_action)
 
             # if we're here then we're valid - update the props + persist
             resource.properties.update(resource_patch.properties)
@@ -168,7 +169,7 @@ class ResourceRepository(BaseRepository):
         except EntityDoesNotExist:
             raise TargetTemplateVersionDoesNotExist(f"Template '{resource_template.name}' not found for resource type '{resource_template.resourceType}' with target template version '{resource_patch.templateVersion}'")
 
-    def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate):
+    def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate, resource_action: str):
         # get the enriched (combined) template
         enriched_template = resource_template_repo.enrich_template(resource_template, is_update=True)
 
@@ -177,7 +178,7 @@ class ResourceRepository(BaseRepository):
         update_template["required"] = []
         update_template["properties"] = {}
         for prop_name, prop in enriched_template["properties"].items():
-            if ("updateable" in prop.keys() and prop["updateable"] is True):
+            if (resource_action == RESOURCE_ACTION_INSTALL or prop.get("updateable", False) is True):
                 update_template["properties"][prop_name] = prop
 
         self._validate_resource_parameters(resource_patch.dict(), update_template)
diff --git a/api_app/db/repositories/shared_services.py b/api_app/db/repositories/shared_services.py
index af83ab31..9b8b1963 100644
--- a/api_app/db/repositories/shared_services.py
+++ b/api_app/db/repositories/shared_services.py
@@ -3,6 +3,7 @@ from typing import List, Tuple
 import uuid
 
 from pydantic import parse_obj_as
+import resources.strings as strings
 from models.domain.resource_template import ResourceTemplate
 from models.domain.authentication import User
 from db.repositories.resource_templates import ResourceTemplateRepository
@@ -80,4 +81,4 @@ class SharedServiceRepository(ResourceRepository):
     async def patch_shared_service(self, shared_service: SharedService, shared_service_patch: ResourcePatch, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, force_version_update: bool) -> Tuple[SharedService, ResourceTemplate]:
         # get shared service template
         shared_service_template = await resource_template_repo.get_template_by_name_and_version(shared_service.templateName, shared_service.templateVersion, ResourceType.SharedService)
-        return await self.patch_resource(shared_service, shared_service_patch, shared_service_template, etag, resource_template_repo, resource_history_repo, user, force_version_update)
+        return await self.patch_resource(shared_service, shared_service_patch, shared_service_template, etag, resource_template_repo, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE, force_version_update)
diff --git a/api_app/db/repositories/user_resources.py b/api_app/db/repositories/user_resources.py
index dd093e24..c14e31b7 100644
--- a/api_app/db/repositories/user_resources.py
+++ b/api_app/db/repositories/user_resources.py
@@ -6,6 +6,7 @@ from db.repositories.resources_history import ResourceHistoryRepository
 from models.domain.resource_template import ResourceTemplate
 from models.domain.authentication import User
 
+import resources.strings as strings
 from db.errors import EntityDoesNotExist
 from db.repositories.resource_templates import ResourceTemplateRepository
 from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE
@@ -73,4 +74,4 @@ class UserResourceRepository(ResourceRepository):
     async def patch_user_resource(self, user_resource: UserResource, user_resource_patch: ResourcePatch, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, parent_template_name: str, user: User, force_version_update: bool) -> Tuple[UserResource, ResourceTemplate]:
         # get user resource template
         user_resource_template = await resource_template_repo.get_template_by_name_and_version(user_resource.templateName, user_resource.templateVersion, ResourceType.UserResource, parent_service_name=parent_template_name)
-        return await self.patch_resource(user_resource, user_resource_patch, user_resource_template, etag, resource_template_repo, resource_history_repo, user, force_version_update)
+        return await self.patch_resource(user_resource, user_resource_patch, user_resource_template, etag, resource_template_repo, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE, force_version_update)
diff --git a/api_app/db/repositories/workspace_services.py b/api_app/db/repositories/workspace_services.py
index 5f614aaa..218699d3 100644
--- a/api_app/db/repositories/workspace_services.py
+++ b/api_app/db/repositories/workspace_services.py
@@ -7,6 +7,7 @@ from models.domain.resource_template import ResourceTemplate
 from models.domain.authentication import User
 from db.repositories.resource_templates import ResourceTemplateRepository
 
+import resources.strings as strings
 from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE
 from db.repositories.operations import OperationRepository
 from models.domain.workspace_service import WorkspaceService
@@ -80,4 +81,4 @@ class WorkspaceServiceRepository(ResourceRepository):
     async def patch_workspace_service(self, workspace_service: WorkspaceService, workspace_service_patch: ResourcePatch, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, force_version_update: bool) -> Tuple[WorkspaceService, ResourceTemplate]:
         # get workspace service template
         workspace_service_template = await resource_template_repo.get_template_by_name_and_version(workspace_service.templateName, workspace_service.templateVersion, ResourceType.WorkspaceService)
-        return await self.patch_resource(workspace_service, workspace_service_patch, workspace_service_template, etag, resource_template_repo, resource_history_repo, user, force_version_update)
+        return await self.patch_resource(workspace_service, workspace_service_patch, workspace_service_template, etag, resource_template_repo, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE, force_version_update)
diff --git a/api_app/db/repositories/workspaces.py b/api_app/db/repositories/workspaces.py
index c1cd4069..77317109 100644
--- a/api_app/db/repositories/workspaces.py
+++ b/api_app/db/repositories/workspaces.py
@@ -6,6 +6,7 @@ from db.repositories.resources_history import ResourceHistoryRepository
 from models.domain.resource_template import ResourceTemplate
 from models.domain.authentication import User
 
+import resources.strings as strings
 from core import config, credentials
 from db.errors import EntityDoesNotExist, InvalidInput, ResourceIsNotDeployed
 from db.repositories.resource_templates import ResourceTemplateRepository
@@ -159,7 +160,7 @@ class WorkspaceRepository(ResourceRepository):
     async def patch_workspace(self, workspace: Workspace, workspace_patch: ResourcePatch, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, force_version_update: bool) -> Tuple[Workspace, ResourceTemplate]:
         # get the workspace template
         workspace_template = await resource_template_repo.get_template_by_name_and_version(workspace.templateName, workspace.templateVersion, ResourceType.Workspace)
-        return await self.patch_resource(workspace, workspace_patch, workspace_template, etag, resource_template_repo, resource_history_repo, user, force_version_update)
+        return await self.patch_resource(workspace, workspace_patch, workspace_template, etag, resource_template_repo, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE, force_version_update)
 
     def get_workspace_spec_params(self, full_workspace_id: str):
         params = self.get_resource_base_spec_params()
diff --git a/api_app/resources/strings.py b/api_app/resources/strings.py
index 221c21a1..d6dcff73 100644
--- a/api_app/resources/strings.py
+++ b/api_app/resources/strings.py
@@ -176,6 +176,12 @@ RESOURCE_STATUS_UPDATING = "updating"
 RESOURCE_STATUS_UPDATED = "updated"
 RESOURCE_STATUS_UPDATING_FAILED = "updating_failed"
 
+# Resource Action
+RESOURCE_ACTION_INSTALL = "install"
+RESOURCE_ACTION_UPDATE = "upgrade"
+RESOURCE_ACTION_UNINSTALL = "uninstall"
+
+
 # Resource Action Status
 RESOURCE_STATUS_AWAITING_ACTION = "awaiting_action"
 RESOURCE_ACTION_STATUS_INVOKING = "invoking_action"
diff --git a/api_app/service_bus/helpers.py b/api_app/service_bus/helpers.py
index 77f71279..65acff1e 100644
--- a/api_app/service_bus/helpers.py
+++ b/api_app/service_bus/helpers.py
@@ -1,6 +1,7 @@
 from azure.servicebus import ServiceBusMessage
 from azure.servicebus.aio import ServiceBusClient
 from pydantic import parse_obj_as
+from resources import strings
 from db.repositories.resources_history import ResourceHistoryRepository
 from service_bus.substitutions import substitute_properties
 from models.domain.resource_template import PipelineStep
@@ -83,6 +84,8 @@ async def update_resource_for_step(operation_step: OperationStep, resource_repo:
     for step in parent_template_pipeline_dict[primary_action]:
         if step["stepId"] == operation_step.templateStepId:
             template_step = parse_obj_as(PipelineStep, step)
+            if (template_step.resourceAction is None and primary_action == strings.RESOURCE_ACTION_INSTALL):
+                template_step.resourceAction = strings.RESOURCE_ACTION_INSTALL
             break
 
     if template_step is None:
@@ -164,6 +167,7 @@ async def try_patch(resource_repo: ResourceRepository, resource_template_repo: R
         etag=resource_to_update.etag,
         resource_template_repo=resource_template_repo,
         resource_history_repo=resource_history_repo,
-        user=user)
+        user=user,
+        resource_action=template_step.resourceAction)
 
     return resource_to_send


Test patch:
diff --git a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py
index a308b788..60b8569c 100644
--- a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py
+++ b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py
@@ -6,6 +6,7 @@ import pytest_asyncio
 from mock import patch, MagicMock
 
 from jsonschema.exceptions import ValidationError
+from resources import strings
 from db.repositories.resources_history import ResourceHistoryRepository
 from tests_ma.test_api.test_routes.test_resource_helpers import FAKE_CREATE_TIMESTAMP, FAKE_UPDATE_TIMESTAMP
 from tests_ma.test_api.conftest import create_test_user
@@ -354,7 +355,7 @@ async def test_patch_resource_preserves_property_history(_, __, ___, resource_re
     expected_resource.user = user
     expected_resource.updatedWhen = FAKE_UPDATE_TIMESTAMP
 
-    await resource_repo.patch_resource(resource, resource_patch, None, etag, None, resource_history_repo, user)
+    await resource_repo.patch_resource(resource, resource_patch, None, etag, None, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE)
     resource_repo.update_item_with_etag.assert_called_once_with(expected_resource, etag)
 
     # now patch again
@@ -365,7 +366,7 @@ async def test_patch_resource_preserves_property_history(_, __, ___, resource_re
     expected_resource.isEnabled = False
     expected_resource.user = user
 
-    await resource_repo.patch_resource(new_resource, new_patch, None, etag, None, resource_history_repo, user)
+    await resource_repo.patch_resource(new_resource, new_patch, None, etag, None, resource_history_repo, user, strings.RESOURCE_ACTION_UPDATE)
     resource_repo.update_item_with_etag.assert_called_with(expected_resource, etag)
 
 
@@ -380,7 +381,7 @@ def test_validate_patch_with_good_fields_passes(template_repo, resource_repo):
 
     # check it's valid when updating a single updateable prop
     patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large'})
-    resource_repo.validate_patch(patch, template_repo, template)
+    resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE)
 
 
 @patch('db.repositories.resources.ResourceTemplateRepository.enrich_template')
@@ -395,14 +396,14 @@ def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo):
     # check it's invalid when sending an unexpected field
     patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large', 'unexpected_field': 'surprise!'})
     with pytest.raises(ValidationError):
-        resource_repo.validate_patch(patch, template_repo, template)
+        resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL)
 
     # check it's invalid when sending a bad value
     patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'huge'})
     with pytest.raises(ValidationError):
-        resource_repo.validate_patch(patch, template_repo, template)
+        resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL)
 
     # check it's invalid when trying to update a non-updateable field
     patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large', 'os_image': 'linux'})
     with pytest.raises(ValidationError):
-        resource_repo.validate_patch(patch, template_repo, template)
+        resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL)
diff --git a/api_app/tests_ma/test_service_bus/test_resource_request_sender.py b/api_app/tests_ma/test_service_bus/test_resource_request_sender.py
index 71e37851..5f51770f 100644
--- a/api_app/tests_ma/test_service_bus/test_resource_request_sender.py
+++ b/api_app/tests_ma/test_service_bus/test_resource_request_sender.py
@@ -5,6 +5,7 @@ import uuid
 
 from azure.servicebus import ServiceBusMessage
 from mock import AsyncMock, patch
+from resources import strings
 from models.schemas.resource import ResourcePatch
 from service_bus.helpers import (
     try_update_with_retries,
@@ -128,7 +129,7 @@ async def test_multi_step_document_sends_first_step(
         step_resource=user_resource_multi,
         root_resource=None,
         resource_to_update_id=basic_shared_service.id,
-        primary_action="install",
+        primary_action=strings.RESOURCE_ACTION_INSTALL,
         user=test_user,
     )
 
@@ -142,7 +143,8 @@ async def test_multi_step_document_sends_first_step(
         resource_history_repo=resource_history_repo_mock,
         etag=basic_shared_service.etag,
         resource_template_repo=resource_template_repo,
-        user=test_user
+        user=test_user,
+        resource_action=strings.RESOURCE_ACTION_UPDATE
     )