Fixing conflicting resources by supporting larger name suffix

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.11.5"
+__version__ = "0.11.6"

api_app/service_bus/helpers.py

@@ -41,17 +41,24 @@ async def send_deployment_message(content, correlation_id, session_id, action):
 
 
 async def update_resource_for_step(operation_step: OperationStep, resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, primary_resource: Resource, resource_to_update_id: str, primary_action: str, user: User) -> Resource:
-    # if this is main, just leave it alone and return it
-    if operation_step.stepId == "main":
-        return primary_resource
-
     # get the template for the primary resource, to get all the step details for substitutions
     primary_parent_service_name = ""
+    primary_parent_workspace = None
+    primary_parent_workspace_service = None
     if primary_resource.resourceType == ResourceType.UserResource:
         primary_parent_workspace_service = await resource_repo.get_resource_by_id(primary_resource.parentWorkspaceServiceId)
         primary_parent_service_name = primary_parent_workspace_service.templateName
+        primary_parent_workspace = await resource_repo.get_resource_by_id(primary_resource.workspaceId)
+
+    if primary_resource.resourceType == ResourceType.WorkspaceService:
+        primary_parent_workspace = await resource_repo.get_resource_by_id(primary_resource.workspaceId)
+
     primary_template = await resource_template_repo.get_template_by_name_and_version(primary_resource.templateName, primary_resource.templateVersion, primary_resource.resourceType, primary_parent_service_name)
 
+    # if there are no pipelines, no need to continue with substitutions.
+    if primary_template.pipeline is None:
+        return primary_resource
+
     # get the template step
     template_step = None
     for step in primary_template.pipeline.dict()[primary_action]:
@@ -62,40 +69,40 @@ async def update_resource_for_step(operation_step: OperationStep, resource_repo:
     if template_step is None:
         raise Exception(f"Cannot find step with id of {operation_step.stepId} in template {primary_resource.templateName} for action {primary_action}")
 
-    if template_step.resourceAction == "upgrade":
-        resource_to_send = await try_upgrade_with_retries(
-            num_retries=3,
-            attempt_count=0,
-            resource_repo=resource_repo,
-            resource_template_repo=resource_template_repo,
-            resource_history_repo=resource_history_repo,
-            user=user,
-            resource_to_update_id=resource_to_update_id,
-            template_step=template_step,
-            primary_resource=primary_resource
-        )
-
-        return resource_to_send
+    resource_to_send = await try_update_with_retries(
+        num_retries=3,
+        attempt_count=0,
+        resource_repo=resource_repo,
+        resource_template_repo=resource_template_repo,
+        resource_history_repo=resource_history_repo,
+        user=user,
+        resource_to_update_id=resource_to_update_id,
+        template_step=template_step,
+        primary_resource=primary_resource,
+        primary_parent_workspace=primary_parent_workspace,
+        primary_parent_workspace_svc=primary_parent_workspace_service
+    )
 
-    else:
-        raise Exception("Only upgrade is currently supported for pipeline steps")
+    return resource_to_send
 
 
-async def try_upgrade_with_retries(num_retries: int, attempt_count: int, resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_to_update_id: str, template_step: PipelineStep, primary_resource: Resource) -> Resource:
+async def try_update_with_retries(num_retries: int, attempt_count: int, resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_to_update_id: str, template_step: PipelineStep, primary_resource: Resource, primary_parent_workspace: Resource = None, primary_parent_workspace_svc: Resource = None) -> Resource:
     try:
-        return await try_upgrade(
+        return await try_patch(
             resource_repo=resource_repo,
             resource_template_repo=resource_template_repo,
             resource_history_repo=resource_history_repo,
             user=user,
             resource_to_update_id=resource_to_update_id,
             template_step=template_step,
-            primary_resource=primary_resource
+            primary_resource=primary_resource,
+            primary_parent_workspace=primary_parent_workspace,
+            primary_parent_workspace_svc=primary_parent_workspace_svc
         )
     except CosmosAccessConditionFailedError as e:
         logging.warning(f"Etag mismatch for {resource_to_update_id}. Retrying.")
         if attempt_count < num_retries:
-            await try_upgrade_with_retries(
+            await try_update_with_retries(
                 num_retries=num_retries,
                 attempt_count=(attempt_count + 1),
                 resource_repo=resource_repo,
@@ -104,22 +111,23 @@ async def try_upgrade_with_retries(num_retries: int, attempt_count: int, resourc
                 user=user,
                 resource_to_update_id=resource_to_update_id,
                 template_step=template_step,
-                primary_resource=primary_resource
+                primary_resource=primary_resource,
+                primary_parent_workspace=primary_parent_workspace
             )
         else:
             raise e
 
 
-async def try_upgrade(resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_to_update_id: str, template_step: PipelineStep, primary_resource: Resource) -> Resource:
+async def try_patch(resource_repo: ResourceRepository, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_to_update_id: str, template_step: PipelineStep, primary_resource: Resource, primary_parent_workspace: Resource, primary_parent_workspace_svc: Resource) -> Resource:
     resource_to_update = await 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)
+    properties = substitute_properties(template_step, primary_resource, primary_parent_workspace, primary_parent_workspace_svc, resource_to_update)
 
     # get the template for the resource to upgrade
     parent_service_name = ""
     if resource_to_update.resourceType == ResourceType.UserResource:
-        parent_service_name = resource_to_update["parentWorkspaceServiceId"]
+        parent_service_name = primary_parent_workspace_svc.templateName
 
     resource_template_to_send = await resource_template_repo.get_template_by_name_and_version(resource_to_update.templateName, resource_to_update.templateVersion, resource_to_update.resourceType, parent_service_name)
 

api_app/service_bus/substitutions.py

@@ -3,14 +3,23 @@
 from models.domain.resource import Resource
 
 
-def substitute_properties(template_step: PipelineStep, primary_resource: Resource, resource_to_update: Resource) -> dict:
+def substitute_properties(template_step: PipelineStep, primary_resource: Resource, primary_parent_workspace: Resource, primary_parent_workspace_svc: Resource, resource_to_update: Resource) -> dict:
     properties = {}
+    parent_ws_dict = {}
+    parent_ws_svc_dict = {}
     primary_resource_dict = primary_resource.dict()
+    if primary_parent_workspace is not None:
+        parent_ws_dict = primary_parent_workspace.dict()
+    if primary_parent_workspace_svc is not None:
+        parent_ws_svc_dict = primary_parent_workspace_svc.dict()
+
+    if template_step is None or template_step.properties is None:
+        return properties
 
     for prop in template_step.properties:
         val = prop.value
         if isinstance(prop.value, dict):
-            val = recurse_object(prop.value, primary_resource_dict)
+            val = recurse_object(prop.value, primary_resource_dict, parent_ws_dict, parent_ws_svc_dict)
 
             if prop.type == 'array':
                 if prop.name in resource_to_update.properties:
@@ -42,7 +51,7 @@ def substitute_properties(template_step: PipelineStep, primary_resource: Resourc
                 properties[prop.name] = val
 
         else:
-            val = substitute_value(val, primary_resource_dict)
+            val = substitute_value(val, primary_resource_dict, parent_ws_dict, parent_ws_svc_dict)
             properties[prop.name] = val
 
     return properties
@@ -55,23 +64,23 @@ def find_item_index(array: list, arrayMatchField: str, val: dict) -> int:
     return -1
 
 
-def recurse_object(obj: dict, primary_resource_dict: dict) -> dict:
+def recurse_object(obj: dict, resource_dict: dict, parent_ws_dict: dict, parent_ws_svc_dict: dict) -> dict:
     for prop in obj:
         if isinstance(obj[prop], list):
             for i in range(0, len(obj[prop])):
                 if isinstance(obj[prop][i], list) or isinstance(obj[prop][i], dict):
-                    obj[prop][i] = recurse_object(obj[prop][i], primary_resource_dict)
+                    obj[prop][i] = recurse_object(obj[prop][i], resource_dict, parent_ws_dict, parent_ws_svc_dict)
                 else:
-                    obj[prop][i] = substitute_value(obj[prop][i], primary_resource_dict)
+                    obj[prop][i] = substitute_value(obj[prop][i], resource_dict, parent_ws_dict, parent_ws_svc_dict)
         if isinstance(obj[prop], dict):
-            obj[prop] = recurse_object(obj[prop], primary_resource_dict)
+            obj[prop] = recurse_object(obj[prop], resource_dict, parent_ws_dict, parent_ws_svc_dict)
         else:
-            obj[prop] = substitute_value(obj[prop], primary_resource_dict)
+            obj[prop] = substitute_value(obj[prop], resource_dict, parent_ws_dict, parent_ws_svc_dict)
 
     return obj
 
 
-def substitute_value(val: str, primary_resource_dict: dict) -> Union[dict, list, str]:
+def substitute_value(val: str, primary_resource_dict: dict, primary_parent_ws_dict: dict, primary_parent_ws_svc_dict: dict) -> Union[dict, list, str]:
     if "{{" not in val:
         return val
 
@@ -89,12 +98,26 @@ def substitute_value(val: str, primary_resource_dict: dict) -> Union[dict, list,
             t = p[0:p.index("}}")]
             tokens.append(t)
 
+    dict_to_use = None
     for t in tokens:
         # t = "resource.properties.prop_1"
         p = t.split(".")
+        # decide on which dictionary to use (parents support)
+        if p[0] == "resource":
+            dict_to_use = primary_resource_dict
+        if p[0] == "parent_workspace" and p[1] == "resource":
+            dict_to_use = primary_parent_ws_dict
+            del p[0]
+        if p[0] == "parent_workspace_service" and p[1] == "resource":
+            dict_to_use = primary_parent_ws_svc_dict
+            del p[0]
+
         if p[0] == "resource":
-            prop_to_get = primary_resource_dict
+            prop_to_get = dict_to_use
             for i in range(1, len(p)):
+                # instead of failing, if the value is not found, return empty string. Used for backward compatability
+                if p[i] not in prop_to_get:
+                    return ""
                 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

api_app/tests_ma/conftest.py

@@ -285,6 +285,33 @@ def multi_step_resource_template(basic_shared_service_template) -> ResourceTempl
                         )
                     ],
                 ),
+            ],
+            uninstall=[
+                PipelineStep(
+                    stepId="pre-step-1",
+                    stepTitle="Title for pre-step-1",
+                    resourceTemplateName=basic_shared_service_template.name,
+                    resourceType=basic_shared_service_template.resourceType,
+                    resourceAction="upgrade",
+                    properties=[
+                        PipelineStepProperty(
+                            name="display_name", type="string", value="new name"
+                        )
+                    ],
+                ),
+                PipelineStep(stepId="main"),
+                PipelineStep(
+                    stepId="post-step-1",
+                    stepTitle="Title for post-step-1",
+                    resourceTemplateName=basic_shared_service_template.name,
+                    resourceType=basic_shared_service_template.resourceType,
+                    resourceAction="upgrade",
+                    properties=[
+                        PipelineStepProperty(
+                            name="display_name", type="string", value="old name"
+                        )
+                    ],
+                ),
             ]
         ),
     )
@@ -394,6 +421,44 @@ def primary_resource() -> Resource:
     )
 
 
+@pytest.fixture
+def resource_ws_parent() -> Resource:
+    return Resource(
+        id="234",
+        name="ws test resource",
+        isEnabled=True,
+        templateName="ws template name",
+        templateVersion="8",
+        resourceType="workspace",
+        _etag="",
+        properties={
+            "display_name": "ImTheParentWS",
+            "address_prefix": ["172.1.1.1", "192.168.1.1"],
+            "fqdn": ["*pypi.org", "security.ubuntu.com"],
+            "my_protocol": "MyWSCoolProtocol",
+        },
+    )
+
+
+@pytest.fixture
+def resource_ws_svc_parent() -> Resource:
+    return Resource(
+        id="345",
+        name="ws svc test resource",
+        isEnabled=True,
+        templateName="svc template name",
+        templateVersion="9",
+        resourceType="workspace-service",
+        _etag="",
+        properties={
+            "display_name": "ImTheParentWSSvc",
+            "address_prefix": ["172.2.2.2", "192.168.2.2"],
+            "fqdn": ["*pypi.org", "files.pythonhosted.org"],
+            "my_protocol": "MyWSSvcCoolProtocol",
+        },
+    )
+
+
 @pytest.fixture
 def resource_to_update() -> Resource:
     return Resource(

api_app/tests_ma/test_api/test_routes/test_resource_helpers.py

@@ -279,6 +279,9 @@ async def test_save_and_deploy_masks_secrets(self, send_deployment_message_mock,
 
         resource_repo.save_item = AsyncMock(return_value=None)
         operations_repo.create_operation_item = AsyncMock(return_value=operation)
+
+        resource_template_repo.get_template_by_name_and_version = AsyncMock(return_value=basic_resource_template)
+
         user = create_test_user()
 
         await save_and_deploy_resource(

api_app/tests_ma/test_service_bus/test_resource_request_sender.py

@@ -7,7 +7,7 @@
 from mock import AsyncMock, patch
 from models.schemas.resource import ResourcePatch
 from service_bus.helpers import (
-    try_upgrade_with_retries,
+    try_update_with_retries,
     update_resource_for_step,
 )
 from tests_ma.test_api.conftest import create_test_user
@@ -61,6 +61,8 @@ async def test_resource_request_message_generated_correctly(
     resource_repo.get_resource_by_id.return_value = resource
     resource_template_repo.get_template_by_name_and_version.return_value = multi_step_resource_template
 
+    resource_repo.patch_resource.return_value = (resource, multi_step_resource_template)
+
     await send_resource_request_message(
         resource=resource,
         operations_repo=operations_repo_mock,
@@ -168,7 +170,7 @@ async def test_multi_step_document_retries(
 
     num_retries = 5
     try:
-        await try_upgrade_with_retries(
+        await try_update_with_retries(
             num_retries=num_retries,
             attempt_count=0,
             resource_repo=resource_repo,
@@ -177,7 +179,9 @@ async def test_multi_step_document_retries(
             resource_to_update_id="resource-id",
             template_step=multi_step_resource_template.pipeline.install[0],
             resource_history_repo=resource_history_repo,
-            primary_resource=primary_resource
+            primary_resource=primary_resource,
+            primary_parent_workspace=None,
+            primary_parent_workspace_svc=None
         )
     except CosmosAccessConditionFailedError:
         pass