Enable soft delete for workspace backup Recovery Services vaults, fix core public IP lifecycle issues, and fix e2e token expiry

CHANGELOG.md

@@ -20,6 +20,8 @@ ENHANCEMENTS:
 * Update `picomatch` package to v2.3.2 and v4.0.4 to address security vulnerabilities ([#4887](https://github.com/microsoft/AzureTRE/issues/4887))
 
 BUG FIXES:
+* Enable soft delete on workspace backup Recovery Services vaults and purge protected items on destroy to avoid deployment failures and preserve delete behavior ([#4907](https://github.com/microsoft/AzureTRE/pull/4907))
+* Poll backup container registration status before destroy to prevent `BMSUserErrorContainerNotUndeleted` failures on workspace deletion ([#4907](https://github.com/microsoft/AzureTRE/pull/4907))
 * Fix OpenAPI/schema sample generation for `get_sample_operation` step parameters. ([#4864](https://github.com/microsoft/AzureTRE/issues/4864))
 * Fix test airlock request sample data fields and enum values. ([#4866](https://github.com/microsoft/AzureTRE/issues/4866))
 * Fix property substitution not occuring where there is only a main step in the pipeline ([#4824](https://github.com/microsoft/AzureTRE/issues/4824))
@@ -1703,4 +1705,3 @@ FEATURES:
 * Centrally manage the firewall share service state to enable other services to ask for rule changes
 
 Many more enhancements are listed on the [release page](https://github.com/microsoft/AzureTRE/releases/tag/v0.4)
-

core/terraform/appgateway/appgateway.tf

@@ -7,7 +7,7 @@ resource "azurerm_public_ip" "appgwpip" {
   domain_name_label   = var.tre_id
   tags                = local.tre_core_tags
 
-  lifecycle { ignore_changes = [tags, zones] }
+  lifecycle { ignore_changes = [tags, zones, ip_tags] }
 }
 
 resource "azurerm_user_assigned_identity" "agw_id" {

core/terraform/firewall/firewall.tf

@@ -7,7 +7,7 @@ resource "azurerm_public_ip" "fwtransit" {
   sku                 = "Standard"
   tags                = var.tre_core_tags
 
-  lifecycle { ignore_changes = [tags, zones] }
+  lifecycle { ignore_changes = [tags, zones, ip_tags] }
 }
 
 moved {

core/version.txt

@@ -1 +1 @@
-__version__ = "0.16.16"
+__version__ = "0.16.17"

e2e_tests/conftest.py

@@ -8,7 +8,7 @@
 from resources.resource import post_resource, disable_and_delete_resource
 from resources.workspace import get_workspace_auth_details
 from resources import strings as resource_strings
-from helpers import get_admin_token
+from helpers import get_admin_token, get_auth_header, get_token
 
 
 LOGGER = logging.getLogger(__name__)
@@ -155,13 +155,17 @@ async def get_workspace_owner_token(workspace_id, verify):
 
 
 async def disable_and_delete_ws_resource(resource_path, workspace_id, verify):
-    workspace_owner_token = await get_workspace_owner_token(workspace_id, verify)
-    await disable_and_delete_resource(f'/api{resource_path}', workspace_owner_token, verify)
+    admin_token = await get_admin_token(verify=verify)
+    workspace_owner_token, scope_uri = await get_workspace_auth_details(admin_token=admin_token, workspace_id=workspace_id, verify=verify)
+    token_fn = lambda: get_auth_header(get_token(scope_uri, verify))  # noqa: E731
+    await disable_and_delete_resource(f'/api{resource_path}', workspace_owner_token, verify, token_fn=token_fn)
 
 
 async def disable_and_delete_tre_resource(resource_path, verify):
     admin_token = await get_admin_token(verify)
-    await disable_and_delete_resource(f'/api{resource_path}', admin_token, verify)
+    scope_uri = f"api://{config.API_CLIENT_ID}"
+    token_fn = lambda: get_auth_header(get_token(scope_uri, verify))  # noqa: E731
+    await disable_and_delete_resource(f'/api{resource_path}', admin_token, verify, token_fn=token_fn)
 
 
 # Session scope isn't in effect with python-xdist: https://github.com/microsoft/AzureTRE/issues/2868

e2e_tests/resources/resource.py

@@ -22,7 +22,7 @@ async def get_resource(endpoint, access_token, verify):
         return response.json()
 
 
-async def post_resource(payload, endpoint, access_token, verify, method="POST", wait=True, etag="*", access_token_for_wait=None):
+async def post_resource(payload, endpoint, access_token, verify, method="POST", wait=True, etag="*", access_token_for_wait=None, token_fn=None):
     async with AsyncClient(verify=verify, timeout=30.0) as client:
 
         full_endpoint = get_full_endpoint(endpoint)
@@ -44,12 +44,12 @@ async def post_resource(payload, endpoint, access_token, verify, method="POST",
 
         if wait:
             wait_auth_headers = get_auth_header(access_token_for_wait) if access_token_for_wait else auth_headers
-            await wait_for(check_method, client, operation_endpoint, wait_auth_headers, [strings.RESOURCE_STATUS_DEPLOYMENT_FAILED, strings.RESOURCE_STATUS_UPDATING_FAILED])
+            await wait_for(check_method, client, operation_endpoint, wait_auth_headers, [strings.RESOURCE_STATUS_DEPLOYMENT_FAILED, strings.RESOURCE_STATUS_UPDATING_FAILED], token_fn=token_fn)
 
         return resource_path, resource_id
 
 
-async def disable_and_delete_resource(endpoint, access_token, verify):
+async def disable_and_delete_resource(endpoint, access_token, verify, token_fn=None):
     async with AsyncClient(verify=verify, timeout=TIMEOUT) as client:
 
         full_endpoint = get_full_endpoint(endpoint)
@@ -61,7 +61,7 @@ async def disable_and_delete_resource(endpoint, access_token, verify):
         response = await client.patch(full_endpoint, headers=auth_headers, json=payload, timeout=TIMEOUT)
         assert_status(response, [status.HTTP_202_ACCEPTED], "The resource couldn't be disabled")
         operation_endpoint = response.headers["Location"]
-        await wait_for(patch_done, client, operation_endpoint, auth_headers, [strings.RESOURCE_STATUS_UPDATING_FAILED])
+        await wait_for(patch_done, client, operation_endpoint, auth_headers, [strings.RESOURCE_STATUS_UPDATING_FAILED], token_fn=token_fn)
 
         # delete
         response = await client.delete(full_endpoint, headers=auth_headers, timeout=TIMEOUT)
@@ -70,16 +70,18 @@ async def disable_and_delete_resource(endpoint, access_token, verify):
         resource_id = response.json()["operation"]["resourceId"]
         operation_endpoint = response.headers["Location"]
 
-        await wait_for(delete_done, client, operation_endpoint, auth_headers, [strings.RESOURCE_STATUS_DELETING_FAILED])
+        await wait_for(delete_done, client, operation_endpoint, auth_headers, [strings.RESOURCE_STATUS_DELETING_FAILED], token_fn=token_fn)
         return resource_id
 
 
-async def wait_for(func, client, operation_endpoint, headers, failure_states: list):
+async def wait_for(func, client, operation_endpoint, headers, failure_states: list, token_fn=None):
     done, done_state, message, operation_steps = await func(client, operation_endpoint, headers)
     LOGGER.info(f'WAITING FOR OP: {operation_endpoint}')
     while not done:
         await asyncio.sleep(30)
 
+        if token_fn is not None:
+            headers = token_fn()
         done, done_state, message, operation_steps = await func(client, operation_endpoint, headers)
         LOGGER.info(f"{done}, {done_state}, {message}")
     try:

templates/workspaces/base/porter.yaml

@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-workspace-base
-version: 2.8.3
+version: 2.8.5
 description: "A base Azure TRE workspace"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

templates/workspaces/base/terraform/backup/backup.tf

@@ -4,7 +4,6 @@ resource "azurerm_recovery_services_vault" "vault" {
   location            = var.location
   resource_group_name = var.resource_group_name
   sku                 = "Standard"
-  soft_delete_enabled = false
   storage_mode_type   = "ZoneRedundant" #  Possible values are "GeoRedundant", "LocallyRedundant" and "ZoneRedundant". Defaults to "GeoRedundant".
   tags                = var.tre_workspace_tags
 

templates/workspaces/base/terraform/providers.tf

@@ -2,7 +2,7 @@ terraform {
   required_providers {
     azurerm = {
       source  = "hashicorp/azurerm"
-      version = "= 4.57.0"
+      version = "= 4.73.0"
     }
     azuread = {
       source  = "hashicorp/azuread"
@@ -33,6 +33,10 @@ provider "azurerm" {
       recover_soft_deleted_certificates = true
       recover_soft_deleted_keys         = true
     }
+    recovery_service {
+      purge_protected_items_from_vault_on_destroy          = true
+      vm_backup_stop_protection_and_retain_data_on_destroy = false
+    }
     resource_group {
       prevent_deletion_if_contains_resources = false
     }

templates/workspaces/base/terraform/storage.tf

@@ -219,6 +219,76 @@ EOT
   ]
 }
 
+resource "terraform_data" "prepare_backup_for_destroy" {
+  count = var.enable_backup ? 1 : 0
+
+  input = {
+    resource_group_name  = azurerm_resource_group.ws.name
+    vault_name           = module.backup[0].vault_name
+    storage_account_name = azurerm_storage_account.stg.name
+    subscription_id      = data.azurerm_client_config.current.subscription_id
+  }
+
+  provisioner "local-exec" {
+    when        = destroy
+    interpreter = ["/bin/bash", "-c"]
+    command     = <<EOT
+set -euo pipefail
+az login --identity
+az account set --subscription "${self.input.subscription_id}"
+
+vault="${self.input.vault_name}"
+rg="${self.input.resource_group_name}"
+container_name="StorageContainer;storage;${self.input.resource_group_name};${self.input.storage_account_name}"
+
+echo "Disabling soft delete on Recovery Services vault '$vault' so destroy can hard-delete protected items..."
+az backup vault backup-properties set \
+  --name "$vault" --resource-group "$rg" \
+  --soft-delete-feature-state Disable --output none
+
+for attempt in 1 2 3 4 5 6; do
+  state=$(az backup vault backup-properties show \
+    --name "$vault" --resource-group "$rg" \
+    --query softDeleteFeatureState -o tsv || echo "")
+  echo "Attempt $attempt: softDeleteFeatureState='$state'"
+  [ "$state" = "Disabled" ] && break
+  sleep 10
+done
+
+status=$(az backup container show \
+  --name "$container_name" --resource-group "$rg" --vault-name "$vault" \
+  --backup-management-type AzureStorage \
+  --query properties.registrationStatus -o tsv 2>/dev/null || echo "NotFound")
+echo "Container status before destroy: '$status'"
+
+if [ "$status" = "SoftDeleted" ]; then
+  echo "Container is soft-deleted; re-registering before AzureRM destroy."
+  az backup container re-register \
+    --resource-group "$rg" --vault-name "$vault" \
+    --backup-management-type AzureStorage --workload-type AzureFileShare \
+    --container-name "$container_name" --yes --output none || echo "re-register failed, continuing"
+
+  for attempt in 1 2 3 4 5 6 7 8 9 10; do
+    status=$(az backup container show \
+      --name "$container_name" --resource-group "$rg" --vault-name "$vault" \
+      --backup-management-type AzureStorage \
+      --query properties.registrationStatus -o tsv 2>/dev/null || echo "NotFound")
+    echo "Attempt $attempt: container status='$status'"
+    { [ "$status" = "Registered" ] || [ "$status" = "NotFound" ]; } && break
+    sleep 30
+  done
+fi
+
+echo "Backup vault prepared for destroy."
+EOT
+  }
+
+  depends_on = [
+    azurerm_backup_container_storage_account.storage_account,
+    azurerm_backup_protected_file_share.file_share,
+  ]
+}
+
 resource "azurerm_backup_protected_file_share" "file_share" {
   count                     = var.enable_backup ? 1 : 0
   resource_group_name       = azurerm_resource_group.ws.name
@@ -230,6 +300,6 @@ resource "azurerm_backup_protected_file_share" "file_share" {
   depends_on = [
     azurerm_backup_container_storage_account.storage_account,
     azapi_resource.shared_storage,
-    azurerm_private_endpoint.stgfilepe
+    azurerm_private_endpoint.stgfilepe,
   ]
 }