Fix Airlock prerequisites and owned-resource cleanup

CHANGELOG.md

@@ -27,6 +27,7 @@ See [Legacy Airlock & migration](docs/azure-tre-overview/airlock.md#legacy-airlo
 * Add weekly and manual builds for Dockerfiles and Porter bundles, with selected pull request builds. Retry a failed target once and report final results. ([#5088](https://github.com/microsoft/AzureTRE/issues/5088))
 
 BUG FIXES:
+* Create Airlock prerequisites with explicit consent. Track owned review resources and wait for deletion. Preserve certificates if Nexus cleanup fails. ([#5183](https://github.com/microsoft/AzureTRE/issues/5183))
 * Handle expected plain-text Airlock errors and give fixture cleanup a separate, bounded timeout. ([#5174](https://github.com/microsoft/AzureTRE/issues/5174))
 * Install and verify required Azure CLI tools before configuring Windows review VMs. ([#5182](https://github.com/microsoft/AzureTRE/issues/5182))
 * Use supported Dsv6 sizes for Guacamole Linux and Airlock review VMs. ([#5172](https://github.com/microsoft/AzureTRE/issues/5172))

e2e_tests/airlock/request.py

@@ -4,6 +4,9 @@
 import os
 from urllib.parse import urlparse
 from azure.storage.blob import BlobClient
+from azure.storage.blob.aio import BlobClient as AsyncBlobClient
+from azure.core.exceptions import ResourceNotFoundError, ResourceExistsError
+from uuid import uuid4
 from airlock import strings
 from e2e_tests.helpers import get_auth_header, get_full_endpoint
 
@@ -83,6 +86,35 @@ async def delete_blob_using_sas(file_path: str, sas_url: str):
     client.delete_blob()
 
 
+async def wait_for_draft_container_deletion(sas_url: str, *, timeout=60, interval=2):
+    """Require ContainerNotFound after Azure's asynchronous container deletion."""
+    started = asyncio.get_running_loop().time()
+    parsed = urlparse(sas_url)
+    # A new name and overwrite avoid mistaking BlobAlreadyExists for revoked access.
+    blob_url = parsed._replace(path=f"{parsed.path.rstrip('/')}/revocation-probe-{uuid4().hex}").geturl()
+    async with AsyncBlobClient.from_blob_url(blob_url, retry_total=0, connection_timeout=5, read_timeout=10) as client:
+        try:
+            async with asyncio.timeout(timeout):
+                while True:
+                    try:
+                        await client.upload_blob(b"post-submission draft probe", overwrite=True)
+                    except ResourceNotFoundError as error:
+                        if getattr(error, "error_code", None) != "ContainerNotFound":
+                            raise
+                        LOGGER.info(
+                            "Draft container deletion confirmed after %.1f seconds (ContainerNotFound)",
+                            asyncio.get_running_loop().time() - started,
+                        )
+                        return
+                    except ResourceExistsError as error:
+                        # Azure can reject writes while deletion is still in progress.
+                        if getattr(error, "error_code", None) != "ContainerBeingDeleted":
+                            raise
+                    await asyncio.sleep(interval)
+        except TimeoutError:
+            raise AssertionError(f"Draft container deletion was not confirmed within {timeout} seconds") from None
+
+
 async def wait_for_status(request_status: str, workspace_owner_token, workspace_path, request_id, verify):
 
     while True:

e2e_tests/conftest.py

@@ -5,12 +5,14 @@
 from typing import Tuple
 import config
 import logging
-from contextlib import asynccontextmanager, contextmanager
+from contextlib import AsyncExitStack, asynccontextmanager, contextmanager
 
 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, get_template
+from e2e_tests.resources.nexus import nexus_prerequisites
+from e2e_tests.timeouts import async_test_timeout, cleanup_deadline
 
 
 LOGGER = logging.getLogger(__name__)
@@ -72,12 +74,12 @@ def pause_test_timeout_for_cleanup(request):
 @asynccontextmanager
 async def resource_cleanup_timeout(resource_path, request=None):
     with pause_test_timeout_for_cleanup(request):
-        timeout = asyncio.timeout(CLEANUP_TIMEOUT_SECONDS)
+        timeout = None
         try:
-            async with timeout:
+            async with cleanup_deadline(CLEANUP_TIMEOUT_SECONDS) as timeout:
                 yield
         except TimeoutError as error:
-            if not timeout.expired():
+            if timeout is None or not timeout.expired():
                 raise
             message = f"Cleanup of {resource_path} exceeded {CLEANUP_TIMEOUT_SECONDS} seconds"
             LOGGER.error("%s. Check the operation polling logs for its last observed state.", message)
@@ -96,6 +98,7 @@ def verify(pytestconfig):
         return False
 
 
+@async_test_timeout(60 * 60)
 async def create_or_get_test_workspace(
     auth_type: str,
     verify: bool,
@@ -139,13 +142,18 @@ async def create_or_get_test_workspace(
     # TODO: Temp fix to solve creation of workspaces - https://github.com/microsoft/AzureTRE/issues/2986
     await asyncio.sleep(random.uniform(1, 9))
     workspace_path, workspace_id = await post_resource(
-        payload, resource_strings.API_WORKSPACES, access_token=admin_token, verify=verify
+        payload,
+        resource_strings.API_WORKSPACES,
+        access_token=admin_token,
+        verify=verify,
+        cleanup_failed_create=True,
     )
 
     LOGGER.info(f"Workspace {workspace_id} {template_name} created")
     return workspace_path, workspace_id
 
 
+@async_test_timeout(60 * 60)
 async def create_or_get_test_workpace_service(
     workspace_path, workspace_owner_token, pre_created_workspace_service_id, verify
 ):
@@ -165,6 +173,7 @@ async def create_or_get_test_workpace_service(
         endpoint=f"/api{workspace_path}/{resource_strings.API_WORKSPACE_SERVICES}",
         access_token=workspace_owner_token,
         verify=verify,
+        cleanup_failed_create=True,
     )
 
     return workspace_service_path, workspace_service_id
@@ -257,19 +266,32 @@ 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)
+    await disable_and_delete_resource(f"/api{resource_path}", workspace_owner_token, verify, allow_failed_disable=True)
 
 
 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)
+    await disable_and_delete_resource(f"/api{resource_path}", admin_token, verify, allow_failed_disable=True)
+
+
+@pytest.fixture(scope="session")
+async def setup_nexus_prerequisites(verify, request):
+    # Dependencies must outlive review VMs and their workspace finaliser.
+    resources = AsyncExitStack()
+    try:
+        async with asyncio.timeout(60 * 60):
+            await resources.enter_async_context(nexus_prerequisites(verify))
+        yield
+    finally:
+        async with resource_cleanup_timeout("Nexus prerequisites", request):
+            await resources.aclose()
 
 
 # Session scope isn't in effect with python-xdist: https://github.com/microsoft/AzureTRE/issues/2868
 @pytest.fixture(scope="session")
 async def setup_test_airlock_import_review_workspace_and_guacamole_service(
-    verify, request
-) -> Tuple[str, str, str, str, str]:
+    setup_nexus_prerequisites, verify, request
+) -> Tuple[str, str, str, str]:
     pre_created_workspace_id = config.TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID
     # Set up
     workspace_path, workspace_id = await create_or_get_test_workspace(
@@ -279,22 +301,39 @@ async def setup_test_airlock_import_review_workspace_and_guacamole_service(
         pre_created_workspace_id=pre_created_workspace_id,
     )
 
-    admin_token = await get_admin_token(verify=verify)
-    workspace_owner_token, _ = await get_workspace_auth_details(
-        admin_token=admin_token, workspace_id=workspace_id, verify=verify
-    )
+    workspace_service_path = None
     pre_created_workspace_service_id = config.TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_SERVICE_ID
+    try:
+        admin_token = await get_admin_token(verify=verify)
+        workspace_owner_token, _ = await get_workspace_auth_details(
+            admin_token=admin_token, workspace_id=workspace_id, verify=verify
+        )
 
-    workspace_service_path, workspace_service_id = await create_or_get_test_workpace_service(
-        workspace_path,
-        workspace_owner_token=workspace_owner_token,
-        pre_created_workspace_service_id=pre_created_workspace_service_id,
-        verify=verify,
-    )
+        workspace_service_path, workspace_service_id = await create_or_get_test_workpace_service(
+            workspace_path,
+            workspace_owner_token=workspace_owner_token,
+            pre_created_workspace_service_id=pre_created_workspace_service_id,
+            verify=verify,
+        )
 
-    yield workspace_path, workspace_id, workspace_service_path, workspace_service_id
+        yield workspace_path, workspace_id, workspace_service_path, workspace_service_id
 
-    # Tear-down in a cascaded way
-    await clean_up_test_workspace(
-        pre_created_workspace_id=pre_created_workspace_id, workspace_path=workspace_path, verify=verify, request=request
-    )
+    finally:
+        try:
+            # A reused parent has no cascading cleanup. Remove only a child
+            # created by this fixture. Preserve any pre-existing service.
+            if pre_created_workspace_id and workspace_service_path is not None:
+                await clean_up_test_workspace_service(
+                    pre_created_workspace_service_id,
+                    workspace_service_path,
+                    workspace_id,
+                    verify,
+                    request=request,
+                )
+        finally:
+            await clean_up_test_workspace(
+                pre_created_workspace_id=pre_created_workspace_id,
+                workspace_path=workspace_path,
+                verify=verify,
+                request=request,
+            )

e2e_tests/pytest.ini

@@ -9,6 +9,7 @@ markers =
     performance: marks tests for performance evaluation
     timeout: used to set test timeout with pytest-timeout
     airlock: only airlock related
+    airlock_validation: draft-container deletion and review-VM flow, including Nexus prerequisites and cleanup
     workspace_services
 
 asyncio_mode = auto

e2e_tests/resources/nexus.py

@@ -0,0 +1,111 @@
+"""Nexus prerequisites shared by Linux and Airlock review VM tests."""
+
+from contextlib import AsyncExitStack, asynccontextmanager
+from uuid import uuid4
+
+from packaging.version import Version
+
+from e2e_tests import config
+from e2e_tests.helpers import get_admin_token
+from e2e_tests.resources import strings
+from e2e_tests.resources.resource import (
+    FAILED_CREATE_CLEANUP_TIMEOUT_SECONDS,
+    disable_and_delete_resource,
+    get_resource,
+    post_resource,
+    temporary_resource,
+)
+from e2e_tests.timeouts import cleanup_deadline
+
+
+async def cleanup_certificate(resource_path, access_token, verify):
+    async with cleanup_deadline(FAILED_CREATE_CLEANUP_TIMEOUT_SECONDS):
+        services = (await get_resource(strings.API_SHARED_SERVICES, access_token, verify))["sharedServices"]
+        if any(service["templateName"] == strings.NEXUS_SHARED_SERVICE for service in services):
+            raise RuntimeError(f"Retaining certificate dependency {resource_path}: Nexus is still present")
+        await disable_and_delete_resource(f"/api{resource_path}", access_token, verify, allow_failed_disable=True)
+
+
+@asynccontextmanager
+async def certificate_prerequisite(payload, endpoint, access_token, verify):
+    resource_path, _ = await post_resource(payload, endpoint, access_token, verify, cleanup_failed_create=True)
+    try:
+        yield resource_path
+    except BaseException as original_error:
+        try:
+            await cleanup_certificate(resource_path, access_token, verify)
+        except Exception as cleanup_error:
+            original_error.add_note(f"Cleanup of {resource_path} also failed: {cleanup_error!r}")
+        raise
+    else:
+        await cleanup_certificate(resource_path, access_token, verify)
+
+
+@asynccontextmanager
+async def nexus_prerequisites(verify):
+    """Reuse a suitable Nexus instance or create explicitly authorised prerequisites."""
+    admin_token = await get_admin_token(verify)
+    services = (await get_resource(strings.API_SHARED_SERVICES, admin_token, verify))["sharedServices"]
+    nexus_services = [service for service in services if service["templateName"] == strings.NEXUS_SHARED_SERVICE]
+    assert len(nexus_services) <= 1, "This test requires at most one Nexus shared service"
+    if nexus_services:
+        nexus = nexus_services[0]
+        assert nexus["isEnabled"] and nexus["deploymentStatus"] in ("deployed", "updated"), (
+            "Nexus must be deployed and enabled"
+        )
+        assert Version(nexus["templateVersion"]) >= Version("3.11.0"), "Upgrade Nexus to bundle 3.11.0 before this test"
+        yield
+        return
+
+    assert config.TEST_ACCEPT_NEXUS_EULA, (
+        "This test requires Nexus. Deploy Nexus 3.11.0 first, or explicitly accept its CE EULA "
+        "with TEST_ACCEPT_NEXUS_EULA=true (acceptNexusEula in the branch workflow)."
+    )
+    cert_services = [service for service in services if service["templateName"] == strings.CERTS_SHARED_SERVICE]
+    assert len(cert_services) <= 1, "This test requires at most one certificate shared service"
+    async with AsyncExitStack() as resources:
+        if cert_services:
+            certs = cert_services[0]
+            assert certs["isEnabled"] and certs["deploymentStatus"] in ("deployed", "updated"), (
+                "Repair the existing Nexus certificate service before this test"
+            )
+            assert certs["properties"].get("domain_prefix") == "nexus", (
+                "The existing certificate service belongs to another domain. Deploy Nexus separately before this test."
+            )
+            cert_name = certs["properties"]["cert_name"]
+        else:
+            cert_name = f"nexus-e2e-{uuid4().hex[:8]}"
+            await resources.enter_async_context(
+                certificate_prerequisite(
+                    {
+                        "templateName": strings.CERTS_SHARED_SERVICE,
+                        "properties": {
+                            "display_name": "E2E Nexus certificate",
+                            "description": "Certificate prerequisite for E2E Nexus",
+                            "domain_prefix": "nexus",
+                            "cert_name": cert_name,
+                        },
+                    },
+                    strings.API_SHARED_SERVICES,
+                    admin_token,
+                    verify,
+                )
+            )
+        await resources.enter_async_context(
+            temporary_resource(
+                {
+                    "templateName": strings.NEXUS_SHARED_SERVICE,
+                    "properties": {
+                        "display_name": "E2E Nexus",
+                        "description": "Nexus prerequisite for E2E workspace resources",
+                        "ssl_cert_name": cert_name,
+                        "accept_nexus_eula": config.TEST_ACCEPT_NEXUS_EULA,
+                        "vm_size": "Standard_D2s_v3",
+                    },
+                },
+                strings.API_SHARED_SERVICES,
+                admin_token,
+                verify,
+            )
+        )
+        yield