Workspace deploy fails because storage account is not unique

Fixes microsoft#2893

.github/linters/.tflint.hcl

@@ -12,7 +12,7 @@ rule "terraform_unused_declarations" {
 }
 
 rule "terraform_typed_variables" {
-  enabled = false
+  enabled = true
 }
 
 rule "terraform_required_providers" {

.github/linters/.tflint_shared_services.hcl

@@ -9,10 +9,6 @@ plugin "azurerm" {
     enabled = true
 }
 
-rule "terraform_typed_variables" {
-  enabled = false
-}
-
 rule "azurerm_resource_missing_tags" {
   enabled = true
   tags = ["tre_id", "tre_shared_service_id"]

.github/linters/.tflint_user_resources.hcl

@@ -9,10 +9,6 @@ plugin "azurerm" {
     enabled = true
 }
 
-rule "terraform_typed_variables" {
-  enabled = false
-}
-
 rule "azurerm_resource_missing_tags" {
   enabled = true
   tags = ["tre_id", "tre_workspace_id", "tre_workspace_service_id", "tre_user_resource_id"]

.github/linters/.tflint_workspace_services.hcl

@@ -9,10 +9,6 @@ plugin "azurerm" {
     enabled = true
 }
 
-rule "terraform_typed_variables" {
-  enabled = false
-}
-
 rule "azurerm_resource_missing_tags" {
   enabled = true
   tags = ["tre_id", "tre_workspace_id", "tre_workspace_service_id"]

CHANGELOG.md

@@ -1,17 +1,21 @@
 <!-- markdownlint-disable MD041 -->
 ## 0.14.0 (Unreleased)
+
 FEATURES:
 
 ENHANCEMENTS:
 
 BUG FIXES:
-- Add temporary workaround for when id with last 4 chars exists ([#3667](https://github.com/microsoft/AzureTRE/pull/3667))
+* Add temporary workaround for when id with last 4 chars exists ([#3667](https://github.com/microsoft/AzureTRE/pull/3667))
+* Apply missing lifecycle blocks. ([#3670](https://github.com/microsoft/AzureTRE/issues/3670))
+* Outputs of type boolean are stored as strings ([#3655](https://github.com/microsoft/AzureTRE/pulls/3655))
 
 
 ## 0.13.0 (August 9, 2023)
 
 BUG FIXES:
 * Custom actions fail on resources with a pipeline ([#3646](https://github.com/microsoft/AzureTRE/issues/3646))
+* Fix ability to debug resource processor locally ([#3426](https://github.com/microsoft/AzureTRE/issues/4426))
 * Upgrade airlock and unrestricted workspaces to base workspace version 0.12.0 ([#3659](https://github.com/microsoft/AzureTRE/pull/3659))
 
 COMPONENTS:
@@ -45,6 +49,7 @@ COMPONENTS:
 | tre-shared-service-sonatype-nexus | 2.5.3 |
 | tre-shared-service-firewall | 1.1.1 |
 
+
 ## 0.12.0 (July 27, 2023)
 
 FEATURES:

api_app/models/domain/resource.py

@@ -84,3 +84,4 @@ def parse_etag_to_remove_escaped_quotes(cls, value):
 class Output(AzureTREModel):
     Name: str = Field(title="", description="", alias="name")
     Value: Union[list, dict, str] = Field(None, title="", description="", alias="value")
+    Type: str = Field(title="", description="", alias="type")

api_app/service_bus/deployment_status_updater.py

@@ -7,6 +7,7 @@
 
 from api.dependencies.database import get_db_client
 from api.routes.resource_helpers import get_timestamp
+from models.domain.resource import Output
 from db.repositories.resources_history import ResourceHistoryRepository
 from models.domain.request_action import RequestAction
 from db.repositories.resource_templates import ResourceTemplateRepository
@@ -244,7 +245,33 @@ def create_updated_resource_document(self, resource: dict, message: DeploymentSt
 
         # 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('"') if isinstance(output.Value, str) else output.Value for output in message.outputs}
+        # need to convert porter outputs to dict so boolean values are converted to bools, not strings
+        output_dict = self.convert_outputs_to_dict(message.outputs)
         resource["properties"].update(output_dict)
 
         return resource
+
+    def convert_outputs_to_dict(self, outputs_list: [Output]):
+        """
+        Convert a list of Porter outputs to a dictionary
+        """
+
+        result_dict = {}
+        for msg in outputs_list:
+            if msg.Value is None:
+                continue
+            name = msg.Name
+            value = msg.Value
+            obj_type = msg.Type
+
+            #
+            if obj_type == 'string' and isinstance(value, str):
+                value = value.strip("'").strip('"')
+            elif obj_type == 'boolean':
+                if isinstance(value, str):
+                    value = value.strip("'").strip('"')
+                value = (value.lower() == 'true')
+
+            result_dict[name] = value
+
+        return result_dict

api_app/tests_ma/test_service_bus/test_deployment_status_update.py

@@ -1,6 +1,7 @@
 import copy
 import json
 from unittest.mock import MagicMock, ANY
+from pydantic import parse_obj_as
 import pytest
 import uuid
 
@@ -11,7 +12,7 @@
 
 from db.errors import EntityDoesNotExist
 from models.domain.workspace import Workspace
-from models.domain.operation import Operation, OperationStep, Status
+from models.domain.operation import DeploymentStatusUpdateMessage, Operation, OperationStep, Status
 from resources import strings
 from service_bus.deployment_status_updater import DeploymentStatusUpdater
 
@@ -41,8 +42,13 @@
     "status": Status.Deployed,
     "message": "test message",
     "outputs": [
-        {"Name": "name1", "Value": "value1", "Type": "type1"},
-        {"Name": "name2", "Value": "\"value2\"", "Type": "type2"}
+        {"Name": "string1", "Value": "value1", "Type": "string"},
+        {"Name": "string2", "Value": "\"value2\"", "Type": "string"},
+        {"Name": "boolean1", "Value": "True", "Type": "boolean"},
+        {"Name": "boolean2", "Value": "true", "Type": "boolean"},
+        {"Name": "boolean3", "Value": "\"true\"", "Type": "boolean"},
+        {"Name": "list1", "Value": "['one', 'two']", "Type": "string"},
+        {"Name": "list2", "Value": ['one', 'two'], "Type": "string"}
     ]
 }
 
@@ -238,7 +244,15 @@ async def test_outputs_are_added_to_resource_item(app, resource_repo, operations
     resource.properties = {"exitingName": "exitingValue"}
     resource_repo.return_value.get_resource_dict_by_id.return_value = resource.dict()
 
-    new_params = {"name1": "value1", "name2": "value2"}
+    new_params = {
+        "string1": "value1",
+        "string2": "value2",
+        "boolean1": True,
+        "boolean2": True,
+        "boolean3": True,
+        "list1": "['one', 'two']",
+        "list2": ["one", "two"],
+    }
 
     expected_resource = resource
     expected_resource.properties = {**resource.properties, **new_params}
@@ -385,3 +399,27 @@ async def test_multi_step_operation_ends_at_last_step(app, sb_sender_client, res
 
     # check it did _not_ enqueue another message
     sb_sender_client().get_queue_sender().send_messages.assert_not_called()
+
+
+@patch('fastapi.FastAPI')
+async def test_convert_outputs_to_dict(app):
+    # Test case 1: Empty list of outputs
+    outputs_list = []
+    expected_result = {}
+
+    status_updater = DeploymentStatusUpdater(app)
+    assert status_updater.convert_outputs_to_dict(outputs_list) == expected_result
+
+    # Test case 2: List of outputs with mixed types
+    deployment_status_update_message = parse_obj_as(DeploymentStatusUpdateMessage, test_sb_message_with_outputs)
+
+    expected_result = {
+        'string1': 'value1',
+        'string2': 'value2',
+        'boolean1': True,
+        'boolean2': True,
+        'boolean3': True,
+        'list1': "['one', 'two']",
+        'list2': ['one', 'two']
+    }
+    assert status_updater.convert_outputs_to_dict(deployment_status_update_message.outputs) == expected_result

core/terraform/airlock/variables.tf

@@ -1,11 +1,27 @@
-variable "tre_id" {}
-variable "location" {}
-variable "resource_group_name" {}
-variable "airlock_storage_subnet_id" {}
-variable "airlock_events_subnet_id" {}
-variable "enable_local_debugging" {}
-variable "myip" {}
-variable "api_principal_id" {}
+variable "tre_id" {
+  type = string
+}
+variable "location" {
+  type = string
+}
+variable "resource_group_name" {
+  type = string
+}
+variable "airlock_storage_subnet_id" {
+  type = string
+}
+variable "airlock_events_subnet_id" {
+  type = string
+}
+variable "enable_local_debugging" {
+  type = bool
+}
+variable "myip" {
+  type = string
+}
+variable "api_principal_id" {
+  type = string
+}
 
 variable "docker_registry_server" {
   type        = string
@@ -33,22 +49,45 @@ variable "airlock_app_service_plan_sku" {
   default = "P1v3"
 }
 
-variable "airlock_processor_subnet_id" {}
+variable "airlock_processor_subnet_id" {
+  type = string
+}
 
-variable "applicationinsights_connection_string" {}
-variable "airlock_servicebus" {}
-variable "tre_core_tags" {}
+variable "applicationinsights_connection_string" {
+  type = string
+}
+variable "airlock_servicebus" {
+  type = object({
+    id                                = string
+    default_primary_connection_string = string
+  })
+}
+variable "tre_core_tags" {
+  type = map(string)
+}
 
 variable "enable_malware_scanning" {
   type        = bool
   description = "If False, Airlock requests will skip the malware scanning stage"
 }
 
-variable "arm_environment" {}
+variable "arm_environment" {
+  type = string
+}
 
-variable "log_analytics_workspace_id" {}
+variable "log_analytics_workspace_id" {
+  type = string
+}
 
-variable "blob_core_dns_zone_id" {}
-variable "file_core_dns_zone_id" {}
-variable "queue_core_dns_zone_id" {}
-variable "table_core_dns_zone_id" {}
+variable "blob_core_dns_zone_id" {
+  type = string
+}
+variable "file_core_dns_zone_id" {
+  type = string
+}
+variable "queue_core_dns_zone_id" {
+  type = string
+}
+variable "table_core_dns_zone_id" {
+  type = string
+}

core/terraform/appgateway/variables.tf

@@ -1,10 +1,28 @@
 
-variable "tre_id" {}
-variable "location" {}
-variable "resource_group_name" {}
-variable "app_gw_subnet" {}
-variable "shared_subnet" {}
-variable "api_fqdn" {}
-variable "keyvault_id" {}
-variable "static_web_dns_zone_id" {}
-variable "log_analytics_workspace_id" {}
+variable "tre_id" {
+  type = string
+}
+variable "location" {
+  type = string
+}
+variable "resource_group_name" {
+  type = string
+}
+variable "app_gw_subnet" {
+  type = string
+}
+variable "shared_subnet" {
+  type = string
+}
+variable "api_fqdn" {
+  type = string
+}
+variable "keyvault_id" {
+  type = string
+}
+variable "static_web_dns_zone_id" {
+  type = string
+}
+variable "log_analytics_workspace_id" {
+  type = string
+}

core/terraform/azure-monitor/query.tf

@@ -3,6 +3,8 @@ resource "azurerm_log_analytics_query_pack" "tre" {
   resource_group_name = var.resource_group_name
   location            = var.location
   tags                = var.tre_core_tags
+
+  lifecycle { ignore_changes = [tags] }
 }
 
 resource "azurerm_log_analytics_query_pack_query" "rp_logs" {

core/terraform/azure-monitor/variables.tf

@@ -1,11 +1,33 @@
-variable "tre_id" {}
-variable "location" {}
-variable "resource_group_name" {}
-variable "shared_subnet_id" {}
-variable "azure_monitor_dns_zone_id" {}
-variable "azure_monitor_oms_opinsights_dns_zone_id" {}
-variable "azure_monitor_ods_opinsights_dns_zone_id" {}
-variable "azure_monitor_agentsvc_dns_zone_id" {}
-variable "blob_core_dns_zone_id" {}
-variable "tre_core_tags" {}
-variable "enable_local_debugging" {}
+variable "tre_id" {
+  type = string
+}
+variable "location" {
+  type = string
+}
+variable "resource_group_name" {
+  type = string
+}
+variable "shared_subnet_id" {
+  type = string
+}
+variable "azure_monitor_dns_zone_id" {
+  type = string
+}
+variable "azure_monitor_oms_opinsights_dns_zone_id" {
+  type = string
+}
+variable "azure_monitor_ods_opinsights_dns_zone_id" {
+  type = string
+}
+variable "azure_monitor_agentsvc_dns_zone_id" {
+  type = string
+}
+variable "blob_core_dns_zone_id" {
+  type = string
+}
+variable "tre_core_tags" {
+  type = map(string)
+}
+variable "enable_local_debugging" {
+  type = bool
+}

core/terraform/cosmos_mongo.tf

@@ -99,4 +99,6 @@ resource "azurerm_key_vault_secret" "cosmos_mongo_connstr" {
   depends_on = [
     azurerm_key_vault_access_policy.deployer
   ]
+
+  lifecycle { ignore_changes = [tags] }
 }