diff --git a/api_app/_version.py b/api_app/_version.py index dc1bba865d..8966e1c9a6 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.3.14" +__version__ = "0.3.15" diff --git a/api_app/models/domain/resource.py b/api_app/models/domain/resource.py index 416d53fdbc..a48723e93d 100644 --- a/api_app/models/domain/resource.py +++ b/api_app/models/domain/resource.py @@ -1,5 +1,5 @@ from enum import Enum -from typing import List, Optional +from typing import List, Optional, Union from pydantic import Field from models.domain.azuretremodel import AzureTREModel from models.domain.request_action import RequestAction @@ -70,4 +70,4 @@ def get_resource_request_message_payload(self, operation_id: str, step_id: str, class Output(AzureTREModel): Name: str = Field(title="", description="") - Value: str = Field(title="", description="") + Value: Union[list, dict, str] = Field(None, title="", description="") diff --git a/api_app/models/domain/resource_template.py b/api_app/models/domain/resource_template.py index d04633d875..ddebdbf83c 100644 --- a/api_app/models/domain/resource_template.py +++ b/api_app/models/domain/resource_template.py @@ -1,4 +1,4 @@ -from typing import Dict, Any, List, Optional +from typing import Dict, Any, List, Optional, Union from pydantic import Field @@ -34,7 +34,9 @@ class CustomAction(AzureTREModel): class PipelineStepProperty(AzureTREModel): name: str = Field(title="name", description="name of the property to update") type: str = Field(title="type", description="data type of the property to update") - value: str = Field(title="value", description="value to use in substitution for the property to update") + value: Union[dict, str] = Field(None, title="value", description="value to use in substitution for the property to update") + arraySubstitutionAction: Optional[str] = Field("", title="Array Substitution Action", description="How to treat existing values of this property in an array [overwrite | append | replace | remove]") + arrayMatchField: Optional[str] = Field("", title="Array match field", description="Name of the field to use for finding an item in an array - to replace/remove it") class PipelineStep(AzureTREModel): diff --git a/api_app/service_bus/deployment_status_update.py b/api_app/service_bus/deployment_status_update.py index a58dacb659..c4cd730200 100644 --- a/api_app/service_bus/deployment_status_update.py +++ b/api_app/service_bus/deployment_status_update.py @@ -112,7 +112,7 @@ def create_updated_resource_document(resource: dict, message: DeploymentStatusUp # although outputs are likely to be relevant when resources are moving to "deployed" status, # lets not limit when we update them and have the resource process make that decision. - output_dict = {output.Name: output.Value.strip("'").strip('"') for output in message.outputs} + output_dict = {output.Name: output.Value.strip("'").strip('"') if isinstance(output.Value, str) else output.Value for output in message.outputs} resource["properties"].update(output_dict) # if deleted - mark as isActive = False diff --git a/api_app/service_bus/helpers.py b/api_app/service_bus/helpers.py index a6c68a13dd..f38d216b8e 100644 --- a/api_app/service_bus/helpers.py +++ b/api_app/service_bus/helpers.py @@ -2,8 +2,8 @@ from azure.servicebus import ServiceBusMessage from azure.servicebus.aio import ServiceBusClient from contextlib import asynccontextmanager - from pydantic import parse_obj_as +from service_bus.substitutions import substitute_properties from models.domain.resource_template import PipelineStep from models.domain.operation import OperationStep from models.domain.resource import Resource, ResourceType @@ -52,7 +52,7 @@ async def send_deployment_message(content, correlation_id, session_id, action): def update_resource_for_step(operation_step: OperationStep, resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, primary_resource_id: str, resource_to_update_id: str, primary_action: str, user: User) -> Resource: - # create properties dict - for now we create a basic, string only dict to use as a patch + # get primary resource to use in substitutions primary_resource = resource_repo.get_resource_by_id(primary_resource_id) @@ -77,20 +77,16 @@ def update_resource_for_step(operation_step: OperationStep, resource_repo: Resou if template_step is None: raise f"Cannot find step with id of {operation_step.stepId} in template {primary_resource.templateName} for action {primary_action}" - # TODO: actual substitution logic #1679 - properties = {} - for prop in template_step.properties: - properties[prop.name] = prop.value - if template_step.resourceAction == "upgrade": resource_to_send = try_upgrade_with_retries( num_retries=3, attempt_count=0, resource_repo=resource_repo, resource_template_repo=resource_template_repo, - properties=properties, user=user, - resource_to_update_id=resource_to_update_id + resource_to_update_id=resource_to_update_id, + template_step=template_step, + primary_resource=primary_resource ) return resource_to_send @@ -99,14 +95,15 @@ def update_resource_for_step(operation_step: OperationStep, resource_repo: Resou raise Exception("Only upgrade is currently supported for pipeline steps") -def try_upgrade_with_retries(num_retries: int, attempt_count: int, resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, properties: dict, user: User, resource_to_update_id: str) -> Resource: +def try_upgrade_with_retries(num_retries: int, attempt_count: int, resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, user: User, resource_to_update_id: str, template_step: PipelineStep, primary_resource: Resource) -> Resource: try: return try_upgrade( resource_repo=resource_repo, resource_template_repo=resource_template_repo, - properties=properties, user=user, - resource_to_update_id=resource_to_update_id + resource_to_update_id=resource_to_update_id, + template_step=template_step, + primary_resource=primary_resource ) except CosmosAccessConditionFailedError as e: logging.warn(f"Etag mismatch for {resource_to_update_id}. Retrying.") @@ -116,17 +113,21 @@ def try_upgrade_with_retries(num_retries: int, attempt_count: int, resource_repo attempt_count=(attempt_count + 1), resource_repo=resource_repo, resource_template_repo=resource_template_repo, - properties=properties, user=user, - resource_to_update_id=resource_to_update_id + resource_to_update_id=resource_to_update_id, + template_step=template_step, + primary_resource=primary_resource ) else: raise e -def try_upgrade(resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, properties: dict, user: User, resource_to_update_id: str) -> Resource: +def try_upgrade(resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, user: User, resource_to_update_id: str, template_step: PipelineStep, primary_resource: Resource) -> Resource: resource_to_update = resource_repo.get_resource_by_id(resource_to_update_id) + # substitute values into new property bag for update + properties = substitute_properties(template_step, primary_resource, resource_to_update) + # get the template for the resource to upgrade parent_service_name = "" if resource_to_update.resourceType == ResourceType.UserResource: diff --git a/api_app/service_bus/substitutions.py b/api_app/service_bus/substitutions.py new file mode 100644 index 0000000000..53847edd34 --- /dev/null +++ b/api_app/service_bus/substitutions.py @@ -0,0 +1,103 @@ +from typing import Union +from models.domain.resource_template import PipelineStep +from models.domain.resource import Resource + + +def substitute_properties(template_step: PipelineStep, primary_resource: Resource, resource_to_update: Resource) -> dict: + properties = {} + primary_resource_dict = primary_resource.dict() + + for prop in template_step.properties: + val = prop.value + if isinstance(prop.value, dict): + val = recurse_object(prop.value, primary_resource_dict) + + if prop.type == 'array': + if prop.name in resource_to_update.properties: + existing_arr = resource_to_update.properties[prop.name] + else: + existing_arr = [] + + if prop.arraySubstitutionAction == 'overwrite': + existing_arr = [val] + + if prop.arraySubstitutionAction == 'append': + existing_arr.append(val) + + if prop.arraySubstitutionAction == 'remove': + item_index = find_item_index(existing_arr, prop.arrayMatchField, val) + if item_index > -1: + del existing_arr[item_index] + + if prop.arraySubstitutionAction == 'replace': + item_index = find_item_index(existing_arr, prop.arrayMatchField, val) + if item_index > -1: + existing_arr[item_index] = val + else: + existing_arr.append(val) + + properties[prop.name] = existing_arr + + else: + properties[prop.name] = val + + else: + val = substitute_value(val, primary_resource_dict) + properties[prop.name] = val + + return properties + + +def find_item_index(array: list, arrayMatchField: str, val: dict) -> int: + for i in range(0, len(array)): + if array[i][arrayMatchField] == val[arrayMatchField]: + return i + return -1 + + +def recurse_object(obj: dict, primary_resource_dict: dict) -> dict: + for prop in obj: + if isinstance(obj[prop], list): + for i in range(0, len(obj[prop])): + obj[prop][i] = recurse_object(obj[prop][i], primary_resource_dict) + if isinstance(obj[prop], dict): + obj[prop] = recurse_object(obj[prop]) + else: + obj[prop] = substitute_value(obj[prop], primary_resource_dict) + + return obj + + +def substitute_value(val: str, primary_resource_dict: dict) -> Union[dict, list, str]: + if "{{" not in val: + return val + + val = val.replace("{{ ", "{{").replace(" }}", "}}") + + # if the value being substituted in is a simple type, we can return it in the string, to allow for concatenation + # like "This was deployed by {{ resource.id }}" + # else if the value being injected in is a dict/list - we shouldn't try to concatenate that, we'll return the true value and drop any surrounding text + + # extract the tokens to replace + tokens = [] + parts = val.split("{{") + for p in parts: + if len(p) > 0 and "}}" in p: + t = p[0:p.index("}}")] + tokens.append(t) + + for t in tokens: + # t = "resource.properties.prop_1" + p = t.split(".") + if p[0] == "resource": + prop_to_get = primary_resource_dict + for i in range(1, len(p)): + prop_to_get = prop_to_get[p[i]] + + # if the value to inject is actually an object / list - just return it, else replace the value in the string + if isinstance(prop_to_get, dict) or isinstance(prop_to_get, list): + return prop_to_get + else: + val = val.replace("{{" + t + "}}", str(prop_to_get)) + + return val diff --git a/api_app/tests_ma/conftest.py b/api_app/tests_ma/conftest.py index 91e3fba083..8273ffa2f0 100644 --- a/api_app/tests_ma/conftest.py +++ b/api_app/tests_ma/conftest.py @@ -1,5 +1,6 @@ import uuid import pytest +from models.domain.resource import Resource from models.domain.user_resource import UserResource from models.domain.shared_service import SharedService from tests_ma.test_api.test_routes.test_resource_helpers import FAKE_CREATE_TIMESTAMP @@ -192,7 +193,7 @@ def user_resource_template_in_response(input_user_resource_template): @pytest.fixture -def multi_step_resource_template(basic_shared_service_template): +def multi_step_resource_template(basic_shared_service_template) -> ResourceTemplate: return ResourceTemplate( id="123", name="template1", @@ -316,3 +317,89 @@ def multi_step_operation(test_user, basic_shared_service_template, basic_shared_ ) ] ) + + +@pytest.fixture +def primary_resource() -> Resource: + return Resource( + id="123", + name="test resource", + isEnabled=True, + templateName="template name", + templateVersion="7", + resourceType="workspace", + _etag="", + properties={ + "display_name": "test_resource name", + "address_prefix": ["172.0.0.1", "192.168.0.1"], + "fqdn": ["*pypi.org", "files.pythonhosted.org", "security.ubuntu.com"], + "my_protocol": "MyCoolProtocol" + }, + ) + + +@pytest.fixture +def resource_to_update() -> Resource: + return Resource( + id="123", + name="Firewall", + isEnabled=True, + templateName="template name", + templateVersion="7", + resourceType="workspace", + _etag="", + properties={}, + ) + + +@pytest.fixture +def pipeline_step() -> PipelineStep: + return PipelineStep( + properties=[ + PipelineStepProperty( + name="rule_collections", + type="array", + arraySubstitutionAction="overwrite", + arrayMatchField="name", + value={ + "name": "arc-web_app_subnet_nexus_api", + "action": "Allow", + "rules": [ + { + "name": "nexus-package-sources-api", + "description": "Deployed by {{ resource.id }}", + "protocols": [ + {"port": "443", "type": "Https"}, + {"port": "80", "type": "{{ resource.properties.my_protocol }}"}, + ], + "target_fqdns": "{{ resource.properties.fqdn }}", + "source_addresses": "{{ resource.properties.address_prefix }}", + } + ] + } + ) + ] + ) + + +@pytest.fixture +def simple_pipeline_step() -> PipelineStep: + return PipelineStep( + properties=[ + PipelineStepProperty( + name="just_text", + type="string", + value="Updated by {{resource.id}}" + ), + PipelineStepProperty( + name="just_text_2", + type="string", + value="No substitution, just a fixed string here" + ), + PipelineStepProperty( + name="just_text_3", + type="string", + value="Multiple substitutions -> {{resource.id}} and {{resource.templateName}}" + ) + ] + ) 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 f1a760530c..b7544a6c59 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 @@ -1,3 +1,4 @@ + import json import pytest import uuid @@ -5,13 +6,21 @@ from azure.servicebus import ServiceBusMessage from mock import AsyncMock, MagicMock, patch from models.schemas.resource import ResourcePatch -from service_bus.helpers import try_upgrade_with_retries, update_resource_for_step +from service_bus.helpers import ( + try_upgrade_with_retries, + update_resource_for_step, +) from models.schemas.workspace_template import get_sample_workspace_template_object from tests_ma.test_api.conftest import create_test_user -from tests_ma.test_service_bus.test_deployment_status_update import create_sample_operation +from tests_ma.test_service_bus.test_deployment_status_update import ( + create_sample_operation, +) from models.domain.workspace_service import WorkspaceService from models.domain.resource import Resource, ResourceType -from service_bus.resource_request_sender import send_resource_request_message, RequestAction +from service_bus.resource_request_sender import ( + send_resource_request_message, + RequestAction, +) from azure.cosmos.exceptions import CosmosAccessConditionFailedError pytestmark = pytest.mark.asyncio @@ -23,18 +32,26 @@ def create_test_resource(): resourceType=ResourceType.Workspace, templateName="Test resource template name", templateVersion="2.718", - etag='', + etag="", properties={"testParameter": "testValue"}, - resourcePath="test" + resourcePath="test", ) -@pytest.mark.parametrize('request_action', [RequestAction.Install, RequestAction.UnInstall]) -@patch('service_bus.resource_request_sender.OperationRepository') -@patch('service_bus.helpers.ServiceBusClient') -@patch('service_bus.resource_request_sender.ResourceRepository') -@patch('service_bus.resource_request_sender.ResourceTemplateRepository') -async def test_resource_request_message_generated_correctly(resource_template_repo, resource_repo, service_bus_client_mock, operations_repo_mock, request_action): +@pytest.mark.parametrize( + "request_action", [RequestAction.Install, RequestAction.UnInstall] +) +@patch("service_bus.resource_request_sender.OperationRepository") +@patch("service_bus.helpers.ServiceBusClient") +@patch("service_bus.resource_request_sender.ResourceRepository") +@patch("service_bus.resource_request_sender.ResourceTemplateRepository") +async def test_resource_request_message_generated_correctly( + resource_template_repo, + resource_repo, + service_bus_client_mock, + operations_repo_mock, + request_action, +): service_bus_client_mock().get_queue_sender().send_messages = AsyncMock() resource = create_test_resource() operation = create_sample_operation(resource.id, request_action) @@ -49,7 +66,8 @@ async def test_resource_request_message_generated_correctly(resource_template_re user=create_test_user(), resource_template=template, resource_template_repo=resource_template_repo, - action=request_action) + action=request_action + ) args = service_bus_client_mock().get_queue_sender().send_messages.call_args.args assert len(args) == 1 @@ -62,23 +80,43 @@ async def test_resource_request_message_generated_correctly(resource_template_re assert sent_message_as_json["action"] == request_action -@patch('service_bus.resource_request_sender.OperationRepository.create_operation_item') -@patch('service_bus.resource_request_sender.ResourceRepository') -@patch('service_bus.resource_request_sender.ResourceTemplateRepository') -async def test_multi_step_document_sends_first_step(resource_template_repo, resource_repo, create_op_item_mock, multi_step_operation, basic_shared_service, basic_shared_service_template, multi_step_resource_template, user_resource_multi, test_user): +@patch("service_bus.resource_request_sender.OperationRepository.create_operation_item") +@patch("service_bus.resource_request_sender.ResourceRepository") +@patch("service_bus.resource_request_sender.ResourceTemplateRepository") +async def test_multi_step_document_sends_first_step( + resource_template_repo, + resource_repo, + create_op_item_mock, + multi_step_operation, + basic_shared_service, + basic_shared_service_template, + multi_step_resource_template, + user_resource_multi, + test_user, +): create_op_item_mock.return_value = multi_step_operation temp_workspace_service = WorkspaceService( - id="123", - templateName="template-name-here", - templateVersion="0.1.0", - etag="" + id="123", templateName="template-name-here", templateVersion="0.1.0", etag="" ) # return the primary resource, a 'parent' workspace service, then the shared service to patch - resource_repo.get_resource_by_id.side_effect = [user_resource_multi, temp_workspace_service, basic_shared_service] - resource_template_repo.get_current_template.side_effect = [multi_step_resource_template, basic_shared_service_template] + resource_repo.get_resource_by_id.side_effect = [ + user_resource_multi, + temp_workspace_service, + basic_shared_service, + ] + resource_template_repo.get_current_template.side_effect = [ + multi_step_resource_template, + basic_shared_service_template, + ] + + resource_repo.patch_resource = MagicMock( + return_value=(basic_shared_service, basic_shared_service_template) + ) - resource_repo.patch_resource = MagicMock(return_value=(basic_shared_service, basic_shared_service_template)) + resource_repo.get_resource_by_id = MagicMock( + return_value=basic_shared_service + ) _ = update_resource_for_step( operation_step=multi_step_operation.steps[0], @@ -87,14 +125,10 @@ async def test_multi_step_document_sends_first_step(resource_template_repo, reso primary_resource_id="resource-id", resource_to_update_id=basic_shared_service.id, primary_action="install", - user=test_user + user=test_user, ) - expected_patch = ResourcePatch( - properties={ - "display_name": "new name" - } - ) + expected_patch = ResourcePatch(properties={"display_name": "new name"}) # expect the patch for step 1 resource_repo.patch_resource.assert_called_once_with( @@ -107,15 +141,27 @@ async def test_multi_step_document_sends_first_step(resource_template_repo, reso ) -@patch('service_bus.resource_request_sender.ResourceRepository') -@patch('service_bus.resource_request_sender.ResourceTemplateRepository') -async def test_multi_step_document_retries(resource_template_repo, resource_repo, basic_shared_service, basic_shared_service_template, test_user): +@patch("service_bus.resource_request_sender.ResourceRepository") +@patch("service_bus.resource_request_sender.ResourceTemplateRepository") +async def test_multi_step_document_retries( + resource_template_repo, + resource_repo, + basic_shared_service, + basic_shared_service_template, + test_user, + multi_step_resource_template, + primary_resource +): resource_repo.get_resource_by_id.return_value = basic_shared_service - resource_template_repo.get_current_template.return_value = basic_shared_service_template + resource_template_repo.get_current_template.return_value = ( + basic_shared_service_template + ) # simulate an etag mismatch - resource_repo.patch_resource = MagicMock(side_effect=CosmosAccessConditionFailedError) + resource_repo.patch_resource = MagicMock( + side_effect=CosmosAccessConditionFailedError + ) num_retries = 5 try: @@ -124,9 +170,10 @@ async def test_multi_step_document_retries(resource_template_repo, resource_repo attempt_count=0, resource_repo=resource_repo, resource_template_repo=resource_template_repo, - properties={}, user=test_user, - resource_to_update_id="resource-id" + resource_to_update_id="resource-id", + template_step=multi_step_resource_template.pipeline.install[0], + primary_resource=primary_resource ) except CosmosAccessConditionFailedError: pass diff --git a/api_app/tests_ma/test_service_bus/test_substitutions.py b/api_app/tests_ma/test_service_bus/test_substitutions.py new file mode 100644 index 0000000000..5c2b6e8dc4 --- /dev/null +++ b/api_app/tests_ma/test_service_bus/test_substitutions.py @@ -0,0 +1,176 @@ +import copy +import pytest +from service_bus.substitutions import substitute_properties, substitute_value + +pytestmark = pytest.mark.asyncio + + +def test_substitution(primary_resource): + resource_dict = primary_resource.dict() + + # single array val + val_to_sub = "{{ resource.properties.address_prefix }}" + val = substitute_value(val_to_sub, resource_dict) + assert val == ["172.0.0.1", "192.168.0.1"] + + # array val to inject, with text. Text will be dropped. + val_to_sub = "{{ resource.properties.fqdn }} - this text will be removed because fqdn is a list and shouldn't be concatenated into a string" + val = substitute_value(val_to_sub, resource_dict) + assert val == ["*pypi.org", "files.pythonhosted.org", "security.ubuntu.com"] + + # single string val, with text. Will be concatenated into text. + val_to_sub = "I think {{ resource.templateName }} is the best template!" + val = substitute_value(val_to_sub, resource_dict) + assert val == "I think template name is the best template!" + + # multiple string vals, with text. Will be concatenated. + val_to_sub = "I think {{ resource.templateName }} is the best template, and {{ resource.templateVersion }} is a good version!" + val = substitute_value(val_to_sub, resource_dict) + assert val == "I think template name is the best template, and 7 is a good version!" + + +def test_simple_substitution(simple_pipeline_step, primary_resource, resource_to_update): + obj = substitute_properties(simple_pipeline_step, primary_resource, resource_to_update) + + assert obj['just_text'] == 'Updated by 123' + assert obj['just_text_2'] == 'No substitution, just a fixed string here' + assert obj['just_text_3'] == 'Multiple substitutions -> 123 and template name' + + +def test_substitution_props(pipeline_step, primary_resource, resource_to_update): + obj = substitute_properties(pipeline_step, primary_resource, resource_to_update) + + assert obj["rule_collections"][0]["rules"][0]["target_fqdns"] == ["*pypi.org", "files.pythonhosted.org", "security.ubuntu.com"] + assert obj["rule_collections"][0]["rules"][0]["source_addresses"] == ["172.0.0.1", "192.168.0.1"] + assert obj["rule_collections"][0]["rules"][0]["protocols"][1]["type"] == "MyCoolProtocol" + assert obj["rule_collections"][0]["rules"][0]["description"] == "Deployed by 123" + + +def test_substitution_array_append_remove(pipeline_step, primary_resource, resource_to_update): + + # do the first substitution, and assert there's a single rule collection + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "append" + step.properties[0].value['name'] = "object 1" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 1 + + # in effect the RP will do this: + resource_to_update.properties = obj + + # now append another substitution, and check we've got both rules + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "append" + step.properties[0].value['name'] = "object 2" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 2 + + # the RP makes the change again... + resource_to_update.properties = obj + + # now append another substitution, and check we've got all 3 rules + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "append" + step.properties[0].value['name'] = "object 3" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 3 + + # the RP makes the change again... + resource_to_update.properties = obj + + # now remove object 2... + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "remove" + step.properties[0].value['name'] = "object 2" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 2 + assert obj['rule_collections'][0]['name'] == "object 1" + assert obj['rule_collections'][1]['name'] == "object 3" + + # the RP makes the change again... + resource_to_update.properties = obj + + # now remove object 1... + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "remove" + step.properties[0].value['name'] = "object 1" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 1 + assert obj['rule_collections'][0]['name'] == "object 3" + + # the RP makes the change again... + resource_to_update.properties = obj + + # now remove object 3... + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "remove" + step.properties[0].value['name'] = "object 3" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 0 + + # the RP makes the change again... + resource_to_update.properties = obj + + # now remove another one, even though the array is empty... + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "remove" + step.properties[0].value['name'] = "object 1" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 0 + + +def test_substitution_array_append_replace(pipeline_step, primary_resource, resource_to_update): + + # add object 1 + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "append" + step.properties[0].value['name'] = "Object 1" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 1 + assert obj["rule_collections"][0]["name"] == "Object 1" + + # the RP does this: + resource_to_update.properties = obj + + # add object 2 + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "append" + step.properties[0].value['name'] = "Object 2" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 2 + assert obj["rule_collections"][1]["name"] == "Object 2" + + # the RP does this: + resource_to_update.properties = obj + + # replace object 1 + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "replace" + step.properties[0].value['name'] = "Object 1" + step.properties[0].value['action'] = "Deny Object 1" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 2 + assert obj["rule_collections"][0]["action"] == "Deny Object 1" + + # the RP does this: + resource_to_update.properties = obj + + # replace the next one + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "replace" + step.properties[0].value['name'] = "Object 2" + step.properties[0].value['action'] = "Deny Object 2" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 2 + assert obj["rule_collections"][1]["action"] == "Deny Object 2" + + +def test_substitution_array_replace_not_found(pipeline_step, primary_resource, resource_to_update): + + # try to replace an item not there - it should just append + step = copy.deepcopy(pipeline_step) + step.properties[0].arraySubstitutionAction = "replace" + step.properties[0].value['name'] = "Object 1" + obj = substitute_properties(step, primary_resource, resource_to_update) + assert len(obj["rule_collections"]) == 1 + assert obj["rule_collections"][0]["name"] == "Object 1" diff --git a/docs/tre-templates/pipeline-templates/overview.md b/docs/tre-templates/pipeline-templates/overview.md index db3bab6c49..ce8b5a94eb 100644 --- a/docs/tre-templates/pipeline-templates/overview.md +++ b/docs/tre-templates/pipeline-templates/overview.md @@ -1,7 +1,5 @@ # Pipeline Templates -> NOTE: This feature is under development, and not currently functional. These documents will be updated alongside the code. - Occasionally there will be a need for the deployment / update of one resource to affect a change in another. This section outlines how that can be achieved with Pipeline Templates. ## Overview @@ -44,16 +42,14 @@ Consider the following `template_schema.json`: ``` When a user deploys this resource, the API will read the `install: []` array within the `pipeline: {}` block, and will: -- Orchestrate the `upgrade` of the `tre-shared-service-firewall`. +- Orchestrate the `upgrade` of the `tre-shared-service-firewall`, changing the `display_name` property to `A new name here!`. - Run the `main` (primary resource) install - Complete the next step A single `Operation` document will be used to keep track of which steps in the deployment chain have completed. ## Current Limitations -This feature is very much a work in progress, and is currently limited in the following ways: -- Only statically addressable resources can be referred to - such as `shared_services`, as these are singletons and can be referenced by a template name. -- Only the `upgrade` action for each secondary resource is supported. Support for `install` / `uninstall` is planned. -- Properties only accept static values. Runtime value substitution is planned. -- Only `install` is supported for the 'trigger' for the primary resource. `upgrade` and `uninstall` support is planned. +This feature is undergoing active development, and is currently limited in the following ways: +- Only statically addressable resources can be referred to - `shared_services`, as these are singletons and can be referenced by a template name. +- Only the `upgrade` action for each secondary resource is supported. Support for `install` / `uninstall` of secondary resources is planned. - No current planned support for `customActions`. diff --git a/docs/tre-templates/pipeline-templates/pipeline-schema.md b/docs/tre-templates/pipeline-templates/pipeline-schema.md index 7b5536bddc..9ff62cef67 100644 --- a/docs/tre-templates/pipeline-templates/pipeline-schema.md +++ b/docs/tre-templates/pipeline-templates/pipeline-schema.md @@ -1,15 +1,15 @@ # Pipeline Template Schema This document will help you write a valid `pipeline: {}` block in your template. -> For a working example, see `./templates/workspace_services/guacamole/user_resources/guacamole-dev-vm/template_schema.json`. +> For a working example, see `./templates/shared-services/sonatype-nexus/template_schema.json`. ## Schema ```json "pipeline": { - "install": [ // <-- currently only install supported + "install": [ // <-- [install | upgrade | uninstall] { "stepId": "a unique string value here", - "stepTitle": "Friendly description for the user here", + "stepTitle": "Friendly description of the step here - will be displayed in the UI", "resourceTemplateName": "name of the resource template to update", "resourceType": "shared_service", // <-- currently only shared_service types supported "resourceAction": "upgrade", // <-- currently only upgrade supported @@ -17,7 +17,7 @@ This document will help you write a valid `pipeline: {}` block in your template. { "name": "display_name", "type": "string", - "value": "A new name here!" // <-- currently only static strings supported + "value": "A new name here!" }] }, { @@ -26,6 +26,51 @@ This document will help you write a valid `pipeline: {}` block in your template. ``` +## Substituting Resource Property Values +It's possible to refer to properties from the primary resource (the resource that triggered this pipeline) in the template steps. The values will be substituted in at runtime. + +The syntax is `{{ resource.propertyName }}`. For example: `"{{ resource.properties.display_name }}"`. + +Example pipeline in `template_schema.json`: +The below example references 2 properties from the primary resource to be used in updating the firewall shared service. + +```json +"pipeline": { + "upgrade": [ + { + "stepId": "1234567-87654-2345-6543", + "stepTitle": "Update a firewall rule", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared_service", + "resourceAction": "upgrade", + "arraySubstitutionAction": "replace", // <-- [append | remove | replace] + "arrayMatchField": "name", // <-- name of the field in the array object to match on, for remove / replace + "properties": [ + { + "name": "rule_collections", + "type": "array", // <-- More on array types below + "value": { // <-- value can be string or object + "name": "my-firewall-rule-collection", + "action": "Allow", + "rules": [ + { + "name": "my-rules", + "target_fqdns": "{{ resource.properties.fqdns_list }}", + "source_addresses": "{{ resource.properties.address_prefixes }}" + } + } + }] + }, +``` + +## Working with Properties Containing Arrays +It's possible that a resource property would actually be an array. As an example, the firewall shared service has the `rule_collections` property. This single property contains +an array of objects. Since the values inside this array may have been sourced from different resources, it's important to leave other values in tact when modifying the property. +To do so, the `arraySubstitutionAction` field supports the following values: +- `append` - just append this object into the array +- `replace` - find this object in the array (using the `arrayMatchField` value), and replace it with this value +- `remove` - remove this property from the array (useful for `uninstall` actions) + ## Notes - Each step is executed in serial, in the order defined in the template - Theoretically any number of steps could be created diff --git a/resource_processor/version.txt b/resource_processor/version.txt index 260c070a89..f9aa3e1109 100644 --- a/resource_processor/version.txt +++ b/resource_processor/version.txt @@ -1 +1 @@ -__version__ = "0.3.1" +__version__ = "0.3.2" diff --git a/resource_processor/vmss_porter/runner.py b/resource_processor/vmss_porter/runner.py index c03ee85e93..ec90084fcd 100644 --- a/resource_processor/vmss_porter/runner.py +++ b/resource_processor/vmss_porter/runner.py @@ -202,6 +202,12 @@ async def get_porter_outputs(msg_body: dict, message_logger_adapter: logging.Log outputs_json = {} try: outputs_json = json.loads(stdout) + + # loop props individually to try to deserialise to dict/list, as all TF outputs are strings, but we want the pure value + for i in range(0, len(outputs_json)): + if "{" in outputs_json[i]['Value'] or "[" in outputs_json[i]['Value']: + outputs_json[i]['Value'] = json.loads(outputs_json[i]['Value']) + message_logger_adapter.info(f"Got outputs as json: {outputs_json}") except ValueError: message_logger_adapter.error(f"Got outputs invalid json: {stdout}") diff --git a/templates/shared_services/gitea/porter.yaml b/templates/shared_services/gitea/porter.yaml index 885af2ad13..453c608ad4 100644 --- a/templates/shared_services/gitea/porter.yaml +++ b/templates/shared_services/gitea/porter.yaml @@ -1,6 +1,6 @@ --- name: tre-shared-service-gitea -version: 0.3.6 +version: 0.3.8 description: "A Gitea shared service" registry: azuretre @@ -42,6 +42,20 @@ mixins: - terraform: clientVersion: 1.1.5 +outputs: + - name: gitea_allowed_fqdns_list + type: string + default: "" + applyTo: + - install + - upgrade + - name: address_prefixes + type: string + default: "" + applyTo: + - install + - upgrade + install: - terraform: description: "Deploy shared service" @@ -56,12 +70,28 @@ install: storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" container_name: "{{ bundle.parameters.tfstate_container_name }}" key: "{{ bundle.parameters.tre_id }}-shared-service-gitea" + outputs: + - name: gitea_allowed_fqdns_list + - name: address_prefixes + upgrade: - - exec: + - terraform: description: "Upgrade shared service" - command: echo - arguments: - - "This shared service does not implement upgrade action" + input: false + vars: + tre_id: "{{ bundle.parameters.tre_id }}" + tre_resource_id: "{{ bundle.parameters.id }}" + mgmt_resource_group_name: "{{ bundle.parameters.tfstate_resource_group_name }}" + acr_name: "{{ bundle.parameters.mgmt_acr_name }}" + backendConfig: + resource_group_name: "{{ bundle.parameters.tfstate_resource_group_name }}" + storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" + container_name: "{{ bundle.parameters.tfstate_container_name }}" + key: "{{ bundle.parameters.tre_id }}-shared-service-gitea" + outputs: + - name: gitea_allowed_fqdns_list + - name: address_prefixes + uninstall: - terraform: description: "Tear down shared service" diff --git a/templates/shared_services/gitea/template_schema.json b/templates/shared_services/gitea/template_schema.json index 1ecd81066d..64a73dbb18 100644 --- a/templates/shared_services/gitea/template_schema.json +++ b/templates/shared_services/gitea/template_schema.json @@ -5,5 +5,114 @@ "title": "Gitea Shared Service", "description": "Provides Gitea shared service", "required": [], - "properties": {} + "properties": {}, + "pipeline": { + "install": [ + { + "stepId": "main" + }, + { + "stepId": "260421b3-7308-491f-b531-e007cdc0ff46", + "stepTitle": "Add gitea rule collection to firewall", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared-service", + "resourceAction": "upgrade", + "properties": [ + { + "name": "rule_collections", + "type": "array", + "arraySubstitutionAction": "replace", + "arrayMatchField": "name", + "value": { + "name": "arc_web_app_subnet_gitea_v2", + "action": "Allow", + "rules": [ + { + "name": "nexus-package-sources", + "description": "Nexus Package Sources", + "protocols": [ + { + "port": "443", + "type": "Https" + }, + { + "port": "80", + "type": "Http" + } + ], + "target_fqdns": "{{ resource.properties.gitea_allowed_fqdns_list }}", + "source_addresses": "{{ resource.properties.address_prefixes }}" + } + ] + } + } + ] + } + ], + "upgrade": [ + { + "stepId": "main" + }, + { + "stepId": "360421b3-7308-491f-b531-e007cdc0ff47", + "stepTitle": "Update gitea rule collection in firewall", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared-service", + "resourceAction": "upgrade", + "properties": [ + { + "name": "rule_collections", + "type": "array", + "arraySubstitutionAction": "replace", + "arrayMatchField": "name", + "value": { + "name": "arc_web_app_subnet_gitea_v2", + "action": "Allow", + "rules": [ + { + "name": "nexus-package-sources", + "description": "Nexus Package Sources", + "protocols": [ + { + "port": "443", + "type": "Https" + }, + { + "port": "80", + "type": "Http" + } + ], + "target_fqdns": "{{ resource.properties.gitea_allowed_fqdns_list }}", + "source_addresses": "{{ resource.properties.address_prefixes }}" + } + ] + } + } + ] + } + ], + "uninstall": [ + { + "stepId": "460421b3-7308-491f-b531-e007cdc0ff48", + "stepTitle": "Remove gitea rule collection from firewall", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared-service", + "resourceAction": "upgrade", + "properties": [ + { + "name": "rule_collections", + "type": "array", + "arraySubstitutionAction": "remove", + "arrayMatchField": "name", + "value": { + "name": "arc_web_app_subnet_gitea_v2" + } + } + ] + }, + { + "stepId": "main" + } + ] + } } diff --git a/templates/shared_services/gitea/terraform/firewall.tf b/templates/shared_services/gitea/terraform/firewall.tf deleted file mode 100644 index 2371ffb83e..0000000000 --- a/templates/shared_services/gitea/terraform/firewall.tf +++ /dev/null @@ -1,23 +0,0 @@ -resource "azurerm_firewall_application_rule_collection" "web_app_subnet_gitea" { - name = "arc-web_app_subnet_gitea" - azure_firewall_name = data.azurerm_firewall.fw.name - resource_group_name = data.azurerm_firewall.fw.resource_group_name - priority = 103 - action = "Allow" - - rule { - name = "gitea-sources" - protocol { - port = "443" - type = "Https" - } - protocol { - port = "80" - type = "Http" - } - - target_fqdns = local.gitea_allowed_fqdns_list - source_addresses = data.azurerm_subnet.web_app.address_prefixes - } -} - diff --git a/templates/shared_services/gitea/terraform/output.tf b/templates/shared_services/gitea/terraform/output.tf index b1094d6406..bb04614e83 100644 --- a/templates/shared_services/gitea/terraform/output.tf +++ b/templates/shared_services/gitea/terraform/output.tf @@ -1,3 +1,11 @@ output "gitea_fqdn" { value = azurerm_app_service.gitea.default_site_hostname } + +output "address_prefixes" { + value = data.azurerm_subnet.web_app.address_prefixes +} + +output "gitea_allowed_fqdns_list" { + value = local.gitea_allowed_fqdns_list +} diff --git a/templates/shared_services/gitea/version.txt b/templates/shared_services/gitea/version.txt index 4ad67eb7ab..771bc6e629 100644 --- a/templates/shared_services/gitea/version.txt +++ b/templates/shared_services/gitea/version.txt @@ -1 +1 @@ -__version__ = "0.3.8" +__version__ = "0.3.9" diff --git a/templates/shared_services/sonatype-nexus/porter.yaml b/templates/shared_services/sonatype-nexus/porter.yaml index 102004696d..8612fa7505 100644 --- a/templates/shared_services/sonatype-nexus/porter.yaml +++ b/templates/shared_services/sonatype-nexus/porter.yaml @@ -1,6 +1,6 @@ --- name: tre-shared-service-nexus -version: 0.3.3 +version: 0.3.4 description: "A Sonatype Nexus shared service" registry: azuretre dockerfile: Dockerfile.tmpl @@ -35,6 +35,19 @@ parameters: env: ARM_USE_MSI type: boolean default: false +outputs: + - name: nexus_allowed_fqdns_list + type: string + default: "" + applyTo: + - install + - upgrade + - name: address_prefixes + type: string + default: "" + applyTo: + - install + - upgrade mixins: - exec - terraform: @@ -51,12 +64,24 @@ install: storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" container_name: "{{ bundle.parameters.tfstate_container_name }}" key: "{{ bundle.parameters.tre_id }}-shared-service-sonatype-nexus" + outputs: + - name: nexus_allowed_fqdns_list + - name: address_prefixes upgrade: - - exec: + - terraform: description: "Upgrade shared service" - command: echo - arguments: - - "This shared service does not implement upgrade action" + input: false + vars: + tre_id: "{{ bundle.parameters.tre_id }}" + tre_resource_id: "{{ bundle.parameters.id }}" + backendConfig: + resource_group_name: "{{ bundle.parameters.tfstate_resource_group_name }}" + storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" + container_name: "{{ bundle.parameters.tfstate_container_name }}" + key: "{{ bundle.parameters.tre_id }}-shared-service-sonatype-nexus" + outputs: + - name: nexus_allowed_fqdns_list + - name: address_prefixes uninstall: - terraform: description: "Tear down shared service" diff --git a/templates/shared_services/sonatype-nexus/template_schema.json b/templates/shared_services/sonatype-nexus/template_schema.json index 13e66df1a7..111dd79f63 100644 --- a/templates/shared_services/sonatype-nexus/template_schema.json +++ b/templates/shared_services/sonatype-nexus/template_schema.json @@ -5,5 +5,114 @@ "title": "Nexus Shared Service", "description": "Provides Nexus shared service", "required": [], - "properties": {} + "properties": {}, + "pipeline": { + "install": [ + { + "stepId": "main" + }, + { + "stepId": "a60421b3-7308-491f-b531-e007cdc0ff46", + "stepTitle": "Add nexus rule collection to firewall", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared-service", + "resourceAction": "upgrade", + "properties": [ + { + "name": "rule_collections", + "type": "array", + "arraySubstitutionAction": "replace", + "arrayMatchField": "name", + "value": { + "name": "arc-web_app_subnet_nexus_v2", + "action": "Allow", + "rules": [ + { + "name": "nexus-package-sources", + "description": "Nexus Package Sources", + "protocols": [ + { + "port": "443", + "type": "Https" + }, + { + "port": "80", + "type": "Http" + } + ], + "target_fqdns": "{{ resource.properties.nexus_allowed_fqdns_list }}", + "source_addresses": "{{ resource.properties.address_prefixes }}" + } + ] + } + } + ] + } + ], + "upgrade": [ + { + "stepId": "main" + }, + { + "stepId": "a60421b3-7308-491f-b531-e007cdc0ff47", + "stepTitle": "Update nexus rule collection in firewall", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared-service", + "resourceAction": "upgrade", + "properties": [ + { + "name": "rule_collections", + "type": "array", + "arraySubstitutionAction": "replace", + "arrayMatchField": "name", + "value": { + "name": "arc-web_app_subnet_nexus_v2", + "action": "Allow", + "rules": [ + { + "name": "nexus-package-sources", + "description": "Nexus Package Sources", + "protocols": [ + { + "port": "443", + "type": "Https" + }, + { + "port": "80", + "type": "Http" + } + ], + "target_fqdns": "{{ resource.properties.nexus_allowed_fqdns_list }}", + "source_addresses": "{{ resource.properties.address_prefixes }}" + } + ] + } + } + ] + } + ], + "uninstall": [ + { + "stepId": "a60421b3-7308-491f-b531-e007cdc0ff48", + "stepTitle": "Remove nexus rule collection from firewall", + "resourceTemplateName": "tre-shared-service-firewall", + "resourceType": "shared-service", + "resourceAction": "upgrade", + "properties": [ + { + "name": "rule_collections", + "type": "array", + "arraySubstitutionAction": "remove", + "arrayMatchField": "name", + "value": { + "name": "arc-web_app_subnet_nexus_v2" + } + } + ] + }, + { + "stepId": "main" + } + ] + } } diff --git a/templates/shared_services/sonatype-nexus/terraform/firewall.tf b/templates/shared_services/sonatype-nexus/terraform/firewall.tf deleted file mode 100644 index 17113d48db..0000000000 --- a/templates/shared_services/sonatype-nexus/terraform/firewall.tf +++ /dev/null @@ -1,22 +0,0 @@ -resource "azurerm_firewall_application_rule_collection" "web_app_subnet_nexus" { - name = "arc-web_app_subnet_nexus" - azure_firewall_name = data.azurerm_firewall.fw.name - resource_group_name = data.azurerm_firewall.fw.resource_group_name - priority = 104 - action = "Allow" - - rule { - name = "nexus-package-sources" - protocol { - port = "443" - type = "Https" - } - protocol { - port = "80" - type = "Http" - } - - target_fqdns = local.nexus_allowed_fqdns_list - source_addresses = data.azurerm_subnet.web_app.address_prefixes - } -} diff --git a/templates/shared_services/sonatype-nexus/terraform/output.tf b/templates/shared_services/sonatype-nexus/terraform/output.tf index d69d8570a5..f98d8b99fe 100644 --- a/templates/shared_services/sonatype-nexus/terraform/output.tf +++ b/templates/shared_services/sonatype-nexus/terraform/output.tf @@ -1,3 +1,11 @@ output "nexus_fqdn" { value = azurerm_app_service.nexus.default_site_hostname -} \ No newline at end of file +} + +output "address_prefixes" { + value = jsonencode(data.azurerm_subnet.web_app.address_prefixes) +} + +output "nexus_allowed_fqdns_list" { + value = jsonencode(local.nexus_allowed_fqdns_list) +}