From dda6ab679025b5a045ba402839be041598514902 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Thu, 3 Apr 2025 14:27:17 +0000 Subject: [PATCH 01/17] Add ability to reuse workspace across parallel test proceses --- .../devcontainer_run_command/action.yml | 2 +- .github/workflows/deploy_tre.yml | 6 +- .github/workflows/pr_comment_bot.yml | 6 +- .../setup-instructions/workflows.md | 1 + e2e_tests/conftest.py | 71 ++++++++++++++++--- 5 files changed, 69 insertions(+), 17 deletions(-) diff --git a/.github/actions/devcontainer_run_command/action.yml b/.github/actions/devcontainer_run_command/action.yml index c7a72479aa..7a34b9a3d7 100644 --- a/.github/actions/devcontainer_run_command/action.yml +++ b/.github/actions/devcontainer_run_command/action.yml @@ -270,7 +270,7 @@ runs: && inputs.ENABLE_CMK_ENCRYPTION) || 'false' }}" \ -e TF_VAR_encryption_kv_name="${{ inputs.ENCRYPTION_KV_NAME }}" \ -e TF_VAR_external_key_store_id="${{ inputs.EXTERNAL_KEY_STORE_ID }}" \ - -e E2E_TESTS_NUMBER_PROCESSES="${{ inputs.E2E_TESTS_NUMBER_PROCESSES }}" \ + -e E2E_TESTS_NUMBER_PROCESSES="${{ inputs.E2E_TESTS_NUMBER_PROCESSES || 1 }}" \ '${{ inputs.CI_CACHE_ACR_NAME }}${{ env.ACR_DOMAIN_SUFFIX }}/tredev:${{ inputs.DEVCONTAINER_TAG }}' \ bash -c -x "./command.sh" diff --git a/.github/workflows/deploy_tre.yml b/.github/workflows/deploy_tre.yml index c9ea4e3e0a..d725ba80ea 100644 --- a/.github/workflows/deploy_tre.yml +++ b/.github/workflows/deploy_tre.yml @@ -3,7 +3,7 @@ name: Deploy Azure TRE # This workflow is the integration build run for pushes to the main branch # It also runs on a schedule, serving as the nightly build -on: # yamllint disable-line rule:truthy +on: # yamllint disable-line rule:truthy schedule: # midnight every day https://crontab.guru/#0_0_*_*_* - cron: "0 0 * * *" @@ -36,8 +36,8 @@ jobs: ${{ (github.event_name == 'push' && 'extended or extended_aad') || 'extended or extended_aad or shared_services or airlock' }} environmentName: ${{ github.event.inputs.environment || 'CICD' }} - E2E_TESTS_NUMBER_PROCESSES: 1 - DEVCONTAINER_TAG: 'latest' + E2E_TESTS_NUMBER_PROCESSES: ${{ vars.e2e_tests_number_processes }} + DEVCONTAINER_TAG: "latest" secrets: AAD_TENANT_ID: ${{ secrets.AAD_TENANT_ID }} ACR_NAME: ${{ secrets.ACR_NAME }} diff --git a/.github/workflows/pr_comment_bot.yml b/.github/workflows/pr_comment_bot.yml index 3771dfbbfb..b27dd89005 100644 --- a/.github/workflows/pr_comment_bot.yml +++ b/.github/workflows/pr_comment_bot.yml @@ -1,9 +1,9 @@ --- name: pr_comment_bot -on: # yamllint disable-line rule:truthy +on: # yamllint disable-line rule:truthy issue_comment: - types: [created] # only run on new comments + types: [created] # only run on new comments # https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment # https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#issue_comment @@ -164,7 +164,7 @@ jobs: (needs.pr_comment.outputs.command == 'run-tests-shared-services' && 'shared_services') || (needs.pr_comment.outputs.command == 'run-tests' && '') }} environmentName: CICD - E2E_TESTS_NUMBER_PROCESSES: 1 + E2E_TESTS_NUMBER_PROCESSES: ${{ vars.e2e_tests_number_processes }} DEVCONTAINER_TAG: ${{ needs.pr_comment.outputs.prRefId }} secrets: AAD_TENANT_ID: ${{ secrets.AAD_TENANT_ID }} diff --git a/docs/tre-admins/setup-instructions/workflows.md b/docs/tre-admins/setup-instructions/workflows.md index eabc62332d..af6ecef094 100644 --- a/docs/tre-admins/setup-instructions/workflows.md +++ b/docs/tre-admins/setup-instructions/workflows.md @@ -143,6 +143,7 @@ Configure variables used in the deployment workflow: | `AZURE_ENVIRONMENT` | Optional. The name of the Azure environment. Supported values are `AzureCloud` and `AzureUSGovernment`. Default value is `AzureCloud`. | | `CORE_APP_SERVICE_PLAN_SKU` | Optional. The SKU used for AppService plan for core infrastructure. Default value is `P1v2`. | | `WORKSPACE_APP_SERVICE_PLAN_SKU` | Optional. The SKU used for AppService plan used in E2E tests. Default value is `P1v2`. | +| `E2E_TESTS_NUMBER_PROCESSES` | Optional. The number of pytest processes to instantiate when the E2E tests start. Defaults to `1`. | | `RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE` | Optional. The number of processes to instantiate when the Resource Processor starts. Equates to the number of parallel deployment operations possible in your TRE. Defaults to `5`. | | `ENABLE_SWAGGER` | Optional. Determines whether the Swagger interface for the API will be available. Default value is `false`. | | `FIREWALL_SKU` | Optional. The SKU of the Azure Firewall instance. Default value is `Standard`. Allowed values [`Basic`, `Standard`, `Premium`]. See [Azure Firewall SKU feature comparison](https://learn.microsoft.com/en-us/azure/firewall/choose-firewall-sku). | diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index 39589e1696..361186db29 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -1,4 +1,8 @@ +import json +import os +from pathlib import Path import random +from filelock import FileLock import pytest import asyncio from typing import Tuple @@ -19,6 +23,16 @@ def pytest_addoption(parser): parser.addoption("--verify", action="store", default="true") +def pytest_configure(config): + worker_id = os.environ.get("PYTEST_XDIST_WORKER") + if worker_id is not None: + logging.basicConfig( + format=config.getini("log_file_format"), + filename=f"tests_{worker_id}.log", + level=config.getini("log_file_level"), + ) + + @pytest.fixture(scope="session") def verify(pytestconfig): if pytestconfig.getoption("verify").lower() == "true": @@ -102,21 +116,60 @@ async def clean_up_test_workspace_service(pre_created_workspace_service_id: str, await disable_and_delete_ws_resource(workspace_service_path, workspace_id, verify) -# Session scope isn't in effect with python-xdist: https://github.com/microsoft/AzureTRE/issues/2868 +def read_json_file(file_path: Path) -> dict: + return json.loads(file_path.read_text().replace("'", '"')) + + +def write_json_file(file_path: Path, data: dict): + file_path.write_text(json.dumps(data)) + + @pytest.fixture(scope="session") -async def setup_test_workspace(verify) -> Tuple[str, str, str]: - pre_created_workspace_id = config.TEST_WORKSPACE_ID - # Set up - uses a pre created app reg as has appropriate roles assigned - workspace_path, workspace_id = await create_or_get_test_workspace( - auth_type="Manual", verify=verify, pre_created_workspace_id=pre_created_workspace_id, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) +async def setup_test_workspace(verify, worker_id, tmp_path_factory) -> Tuple[str, str, str]: + LOCK_SUFFIX = ".lock" + WORKSPACE_DETAILS_FILE = "setup_test_workspace.json" + + root_tmp_dir = tmp_path_factory.getbasetemp().parent + fn = root_tmp_dir / WORKSPACE_DETAILS_FILE + lock_fn = root_tmp_dir / (WORKSPACE_DETAILS_FILE + LOCK_SUFFIX) + worker_lock = root_tmp_dir / "worker_locks" / worker_id + + worker_lock.parent.mkdir(parents=True, exist_ok=True) + worker_lock.touch() + + if worker_id == "master": + # Set up - uses a pre-created app reg as it has appropriate roles assigned + workspace_path, workspace_id = await create_or_get_test_workspace( + auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", verify=verify, pre_created_workspace_id=config.TEST_WORKSPACE_ID, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) + else: + with FileLock(lock_fn): + if fn.is_file(): + data = read_json_file(fn) + workspace_path = data["workspace_path"] + workspace_id = data["workspace_id"] + else: + # Set up - uses a pre-created app reg as it has appropriate roles assigned + workspace_path, workspace_id = await create_or_get_test_workspace( + auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", verify=verify, pre_created_workspace_id=config.TEST_WORKSPACE_ID, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) yield workspace_path, workspace_id # Tear-down - await clean_up_test_workspace(pre_created_workspace_id=pre_created_workspace_id, workspace_path=workspace_path, verify=verify) + # Check if there are any other worker locks + if len(list(worker_lock.glob("*"))) > 1: + (root_tmp_dir / "worker_locks" / (worker_id)).unlink(missing_ok=True) + while True: + # Wait for other workers to finish + if len(list(worker_lock.glob("*"))) > 0: + await asyncio.sleep(10) + else: + break + # This is the last worker, clean up the workspace + elif len(list(worker_lock.glob("*"))) == 1: + await clean_up_test_workspace(pre_created_workspace_id=config.TEST_WORKSPACE_ID, workspace_path=workspace_path, verify=verify) + (root_tmp_dir / "worker_locks" / (worker_id)).unlink(missing_ok=True) -# Session scope isn't in effect with python-xdist: https://github.com/microsoft/AzureTRE/issues/2868 @pytest.fixture(scope="session") async def setup_test_workspace_and_guacamole_service(setup_test_workspace, verify): # Set up @@ -135,7 +188,6 @@ async def setup_test_workspace_and_guacamole_service(setup_test_workspace, verif await clean_up_test_workspace_service(pre_created_workspace_service_id, workspace_service_path, workspace_id, verify) -# Session scope isn't in effect with python-xdist: https://github.com/microsoft/AzureTRE/issues/2868 @pytest.fixture(scope="session") async def setup_test_aad_workspace(verify) -> Tuple[str, str, str]: pre_created_workspace_id = config.TEST_AAD_WORKSPACE_ID @@ -164,7 +216,6 @@ async def disable_and_delete_tre_resource(resource_path, verify): await disable_and_delete_resource(f'/api{resource_path}', admin_token, verify) -# 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) -> Tuple[str, str, str, str, str]: pre_created_workspace_id = config.TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID From 9dbf25dfe8d4fe9dabca733e2083bbcaabd27641 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Wed, 14 May 2025 07:03:48 +0000 Subject: [PATCH 02/17] WIP on E2E --- devops/scripts/get_access_token.sh | 26 ++--- .../setup-instructions/workflows.md | 12 --- docs/tre-developers/end-to-end-tests.md | 4 +- e2e_tests/.env.sample | 25 ++--- e2e_tests/config.py | 3 - e2e_tests/conftest.py | 96 +++++++++++-------- e2e_tests/helpers.py | 12 +-- 7 files changed, 76 insertions(+), 102 deletions(-) diff --git a/devops/scripts/get_access_token.sh b/devops/scripts/get_access_token.sh index 7dc0a358c2..7f7a5cae93 100755 --- a/devops/scripts/get_access_token.sh +++ b/devops/scripts/get_access_token.sh @@ -7,24 +7,14 @@ set -o pipefail activeDirectoryUri="$(az cloud show --query endpoints.activeDirectory --output tsv)" -if [ -n "${TEST_ACCOUNT_CLIENT_ID:-}" ] && [ -n "${TEST_ACCOUNT_CLIENT_SECRET:-}" ] && [ -n "${AAD_TENANT_ID:-}" ] && [ -n "${API_CLIENT_ID:-}" ] -then - # Use client credentials flow with TEST_ACCOUNT_CLIENT_ID/SECRET - echo "Using TEST_ACCOUNT_CLIENT_ID to get token via client credential flow" - token_response=$(curl -X POST -H 'Content-Type: application/x-www-form-urlencoded' \ - "${activeDirectoryUri}/${AAD_TENANT_ID}"/oauth2/v2.0/token \ - -d "client_id=${TEST_ACCOUNT_CLIENT_ID}" \ - -d 'grant_type=client_credentials' \ - -d "scope=api://${API_CLIENT_ID}/.default" \ - -d "client_secret=${TEST_ACCOUNT_CLIENT_SECRET}") -elif [ -n "${API_CLIENT_ID:-}" ] && [ -n "${TEST_APP_ID:-}" ] && [ -n "${TEST_USER_NAME:-}" ] && [ -n "${TEST_USER_PASSWORD:-}" ] && [ -n "${AAD_TENANT_ID:-}" ] -then - # Use resource owner password credentials flow with USERNAME/PASSWORD - echo "Using TEST_USER_NAME to get token via resource owner password credential flow" - token_response=$(curl -X POST -H "Content-Type: application/x-www-form-urlencoded" -d \ - "grant_type=password&resource=""${API_CLIENT_ID}""&client_id=""${TEST_APP_ID}""&username=""${TEST_USER_NAME}""&password=""${TEST_USER_PASSWORD}""&scope=default)" \ - "${activeDirectoryUri}/${AAD_TENANT_ID}"/oauth2/token) -fi +# Use client credentials flow with TEST_ACCOUNT_CLIENT_ID/SECRET +echo "Using TEST_ACCOUNT_CLIENT_ID to get token via client credential flow" +token_response=$(curl -X POST -H 'Content-Type: application/x-www-form-urlencoded' \ + "${activeDirectoryUri}/${AAD_TENANT_ID}"/oauth2/v2.0/token \ + -d "client_id=${TEST_ACCOUNT_CLIENT_ID}" \ + -d 'grant_type=client_credentials' \ + -d "scope=api://${API_CLIENT_ID}/.default" \ + -d "client_secret=${TEST_ACCOUNT_CLIENT_SECRET}") if [ -n "${token_response:-}" ] then diff --git a/docs/tre-admins/setup-instructions/workflows.md b/docs/tre-admins/setup-instructions/workflows.md index af6ecef094..d58082688e 100644 --- a/docs/tre-admins/setup-instructions/workflows.md +++ b/docs/tre-admins/setup-instructions/workflows.md @@ -79,18 +79,6 @@ Configure the TRE API and Swagger UI repository secrets | `API_CLIENT_ID` | The application (client) ID of the TRE API app. | | `API_CLIENT_SECRET` | The application password (client secret) of the TRE API app. | -### Create an app registration and a user for the E2E tests - -Follow the instructions to [create an app registration and a test user for the E2E tests in the Authentication and Authorization](../auth.md#tre-e2e-test) document. - -Configure the E2E Test repository secrets - -|
Secret name
| Description | -| ----------- | ----------- | -| `TEST_APP_ID` | The application (client) ID of the E2E Test app | -| `TEST_USER_NAME` | The username of the E2E Test User | -| `TEST_USER_PASSWORD` | The password of the E2E Test User | - ### Create a workspace app registration for setting up workspaces (for the E2E tests) Follow the [instructions to create a workspace app registration](../auth.md#workspaces) (used for the E2E tests) - and make the E2E test user a **WorkspaceOwner** for the app registration. diff --git a/docs/tre-developers/end-to-end-tests.md b/docs/tre-developers/end-to-end-tests.md index b17fcf84dd..c4ef966392 100644 --- a/docs/tre-developers/end-to-end-tests.md +++ b/docs/tre-developers/end-to-end-tests.md @@ -21,6 +21,6 @@ make prepare-for-e2e Use the "Run and Debug" panel within Visual Studio Code, select "E2E Extended", "E2E Smoke" or "E2E Performance" in the drop down box and click play. -- This will copy `config.yaml` settings to `/workspaces/AzureTRE/e2e_tests/.env` for you which supplies your authentciation details +- Values from `config.yaml` are copied to `/workspaces/AzureTRE/e2e_tests/.env`. -- This will also use `/workspaces/AzureTRE/core/private.env` file for other values. +- Outputs from the core deployment are read from `/workspaces/AzureTRE/core/private.env`. diff --git a/e2e_tests/.env.sample b/e2e_tests/.env.sample index 504651cfda..1e2645f607 100644 --- a/e2e_tests/.env.sample +++ b/e2e_tests/.env.sample @@ -1,24 +1,11 @@ +# shellcheck disable=SC2148,SC2034 # API parameters # -------------- -# These keys should be copied into /workspaces/AzureTRE/e2e_tests/.env for you -# TRE_ID=cse-msr-dev -# AAD_TENANT_ID= -# API_CLIENT_ID= -# TEST_ACCOUNT_CLIENT_ID= -# TEST_ACCOUNT_CLIENT_SECRET= +# Values will be copied into /workspaces/AzureTRE/e2e_tests/.env when running tests -RESOURCE_LOCATION=westeurope -TEST_APP_ID= -TEST_USER_PASSWORD= -TEST_WORKSPACE_APP_ID= -TEST_WORKSPACE_APP_SECRET= - -# TODO: move to RP default with https://github.com/microsoft/AzureTRE/pull/2634 -WORKSPACE_APP_SERVICE_PLAN_SKU="P1v2" - -TEST_WORKSPACE_ID= -TEST_AAD_WORKSPACE_ID=ID of pre-created Microsoft Entra ID workspace> +# If you want to use precreated workspace for tests, set the following values +TEST_WORKSPACE_ID="" +TEST_AAD_WORKSPACE_ID="" # Run tests sequentially. Change this value if you want to run tests in parallel locally -E2E_TESTS_NUMBER_PROCESSES=1 +E2E_TESTS_NUMBER_PROCESSES=5 diff --git a/e2e_tests/config.py b/e2e_tests/config.py index 31ccb34c37..fdf58ea019 100644 --- a/e2e_tests/config.py +++ b/e2e_tests/config.py @@ -10,9 +10,6 @@ TRE_ID: str = config("TRE_ID", default="") TRE_URL: str = config("TRE_URL", default="") API_CLIENT_ID: str = config("API_CLIENT_ID", default="") -TEST_USER_NAME: str = config("TEST_USER_NAME", default="") -TEST_USER_PASSWORD: str = config("TEST_USER_PASSWORD", default="") -TEST_APP_ID: str = config("TEST_APP_ID", default="") AAD_TENANT_ID: str = config("AAD_TENANT_ID", default="") TEST_ACCOUNT_CLIENT_ID: str = config("TEST_ACCOUNT_CLIENT_ID", default="") TEST_ACCOUNT_CLIENT_SECRET: str = config("TEST_ACCOUNT_CLIENT_SECRET", default="") diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index 361186db29..edeef600f6 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -5,7 +5,7 @@ from filelock import FileLock import pytest import asyncio -from typing import Tuple +from typing import Tuple, AsyncGenerator import config import logging @@ -124,50 +124,66 @@ def write_json_file(file_path: Path, data: dict): file_path.write_text(json.dumps(data)) -@pytest.fixture(scope="session") -async def setup_test_workspace(verify, worker_id, tmp_path_factory) -> Tuple[str, str, str]: - LOCK_SUFFIX = ".lock" - WORKSPACE_DETAILS_FILE = "setup_test_workspace.json" +# Constants for repeated strings +LOCK_SUFFIX = ".lock" +WORKSPACE_DETAILS_FILE = "setup_test_workspace.json" +WORKER_LOCKS_DIR = "worker_locks" + +@pytest.fixture(scope="session") +async def setup_test_workspace(verify: bool, worker_id: str, tmp_path_factory) -> AsyncGenerator[Tuple[str, str], None]: root_tmp_dir = tmp_path_factory.getbasetemp().parent fn = root_tmp_dir / WORKSPACE_DETAILS_FILE lock_fn = root_tmp_dir / (WORKSPACE_DETAILS_FILE + LOCK_SUFFIX) - worker_lock = root_tmp_dir / "worker_locks" / worker_id + worker_lock_dir = root_tmp_dir / WORKER_LOCKS_DIR + worker_lock = worker_lock_dir / worker_id - worker_lock.parent.mkdir(parents=True, exist_ok=True) + # Ensure worker lock directory exists + worker_lock_dir.mkdir(parents=True, exist_ok=True) worker_lock.touch() - if worker_id == "master": - # Set up - uses a pre-created app reg as it has appropriate roles assigned - workspace_path, workspace_id = await create_or_get_test_workspace( - auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", verify=verify, pre_created_workspace_id=config.TEST_WORKSPACE_ID, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) - else: - with FileLock(lock_fn): - if fn.is_file(): - data = read_json_file(fn) - workspace_path = data["workspace_path"] - workspace_id = data["workspace_id"] - else: - # Set up - uses a pre-created app reg as it has appropriate roles assigned - workspace_path, workspace_id = await create_or_get_test_workspace( - auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", verify=verify, pre_created_workspace_id=config.TEST_WORKSPACE_ID, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) - - yield workspace_path, workspace_id - - # Tear-down - # Check if there are any other worker locks - if len(list(worker_lock.glob("*"))) > 1: - (root_tmp_dir / "worker_locks" / (worker_id)).unlink(missing_ok=True) - while True: - # Wait for other workers to finish - if len(list(worker_lock.glob("*"))) > 0: - await asyncio.sleep(10) - else: - break - # This is the last worker, clean up the workspace - elif len(list(worker_lock.glob("*"))) == 1: - await clean_up_test_workspace(pre_created_workspace_id=config.TEST_WORKSPACE_ID, workspace_path=workspace_path, verify=verify) - (root_tmp_dir / "worker_locks" / (worker_id)).unlink(missing_ok=True) + try: + if worker_id == "master": + workspace_path, workspace_id = await create_or_get_test_workspace( + auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", + verify=verify, + pre_created_workspace_id=config.TEST_WORKSPACE_ID, + client_id=config.TEST_WORKSPACE_APP_ID, + client_secret=config.TEST_WORKSPACE_APP_SECRET + ) + else: + with FileLock(lock_fn): + if fn.is_file(): + data = read_json_file(fn) + workspace_path = data["workspace_path"] + workspace_id = data["workspace_id"] + else: + workspace_path, workspace_id = await create_or_get_test_workspace( + auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", + verify=verify, + pre_created_workspace_id=config.TEST_WORKSPACE_ID, + client_id=config.TEST_WORKSPACE_APP_ID, + client_secret=config.TEST_WORKSPACE_APP_SECRET + ) + + yield workspace_path, workspace_id + + finally: + # Tear-down logic + try: + if len(list(worker_lock_dir.glob("*"))) > 1: + worker_lock.unlink(missing_ok=True) + while len(list(worker_lock_dir.glob("*"))) > 0: + await asyncio.sleep(10) + elif len(list(worker_lock_dir.glob("*"))) == 1: + await clean_up_test_workspace( + pre_created_workspace_id=config.TEST_WORKSPACE_ID, + workspace_path=workspace_path, + verify=verify + ) + worker_lock.unlink(missing_ok=True) + except Exception as e: + LOGGER.error(f"Error during workspace cleanup: {e}") @pytest.fixture(scope="session") @@ -189,7 +205,7 @@ async def setup_test_workspace_and_guacamole_service(setup_test_workspace, verif @pytest.fixture(scope="session") -async def setup_test_aad_workspace(verify) -> Tuple[str, str, str]: +async def setup_test_aad_workspace(verify) -> AsyncGenerator[Tuple[str, str], None]: pre_created_workspace_id = config.TEST_AAD_WORKSPACE_ID # Set up workspace_path, workspace_id = await create_or_get_test_workspace(auth_type="Automatic", verify=verify, pre_created_workspace_id=pre_created_workspace_id) @@ -217,7 +233,7 @@ async def disable_and_delete_tre_resource(resource_path, verify): @pytest.fixture(scope="session") -async def setup_test_airlock_import_review_workspace_and_guacamole_service(verify) -> Tuple[str, str, str, str, str]: +async def setup_test_airlock_import_review_workspace_and_guacamole_service(verify) -> AsyncGenerator[Tuple[str, str, str, str, str], None]: pre_created_workspace_id = config.TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID # Set up workspace_path, workspace_id = await create_or_get_test_workspace(auth_type="Automatic", verify=verify, template_name=resource_strings.AIRLOCK_IMPORT_REVIEW_WORKSPACE, pre_created_workspace_id=pre_created_workspace_id) diff --git a/e2e_tests/helpers.py b/e2e_tests/helpers.py index 307fb88933..9c582f9dce 100644 --- a/e2e_tests/helpers.py +++ b/e2e_tests/helpers.py @@ -110,14 +110,10 @@ async def get_admin_token(verify) -> str: def get_token(scope_uri, verify) -> str: - if config.TEST_ACCOUNT_CLIENT_ID != "" and config.TEST_ACCOUNT_CLIENT_SECRET != "": - # Logging in as an Enterprise Application: Use Client Credentials flow - credential = ClientSecretCredential(config.AAD_TENANT_ID, config.TEST_ACCOUNT_CLIENT_ID, config.TEST_ACCOUNT_CLIENT_SECRET, connection_verify=verify, authority=cloud.get_aad_authority_fqdn()) - token = credential.get_token(f'{scope_uri}/.default') - else: - # Logging in as a User: Use Resource Owner Password Credentials flow - credential = UsernamePasswordCredential(config.TEST_APP_ID, config.TEST_USER_NAME, config.TEST_USER_PASSWORD, connection_verify=verify, authority=cloud.get_aad_authority_fqdn(), tenant_id=config.AAD_TENANT_ID) - token = credential.get_token(f'{scope_uri}/user_impersonation') + + # Logging in as an Enterprise Application: Use Client Credentials flow + credential = ClientSecretCredential(config.AAD_TENANT_ID, config.TEST_ACCOUNT_CLIENT_ID, config.TEST_ACCOUNT_CLIENT_SECRET, connection_verify=verify, authority=cloud.get_aad_authority_fqdn()) + token = credential.get_token(f'{scope_uri}/.default') return token.token From 88990cfefaeff3359c721d243010b0f916fa7b39 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Thu, 15 May 2025 10:21:01 +0000 Subject: [PATCH 03/17] Fix linting. --- .github/workflows/deploy_tre.yml | 2 +- .github/workflows/pr_comment_bot.yml | 2 +- e2e_tests/helpers.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/deploy_tre.yml b/.github/workflows/deploy_tre.yml index 5b424d1920..20c4adf629 100644 --- a/.github/workflows/deploy_tre.yml +++ b/.github/workflows/deploy_tre.yml @@ -36,7 +36,7 @@ jobs: ${{ (github.event_name == 'push' && 'extended or extended_aad') || 'extended or extended_aad or shared_services or airlock' }} environmentName: ${{ github.event.inputs.environment || 'CICD' }} - E2E_TESTS_NUMBER_PROCESSES: ${{ vars.e2e_tests_number_processes }} + E2E_TESTS_NUMBER_PROCESSES: ${{ fromJSON(vars.e2e_tests_number_processes) }} DEVCONTAINER_TAG: "latest" secrets: AAD_TENANT_ID: ${{ secrets.AAD_TENANT_ID }} diff --git a/.github/workflows/pr_comment_bot.yml b/.github/workflows/pr_comment_bot.yml index 82bbea7958..1872a790e8 100644 --- a/.github/workflows/pr_comment_bot.yml +++ b/.github/workflows/pr_comment_bot.yml @@ -164,7 +164,7 @@ jobs: (needs.pr_comment.outputs.command == 'run-tests-shared-services' && 'shared_services') || (needs.pr_comment.outputs.command == 'run-tests' && '') }} environmentName: CICD - E2E_TESTS_NUMBER_PROCESSES: ${{ vars.e2e_tests_number_processes }} + E2E_TESTS_NUMBER_PROCESSES: ${{ fromJSON(vars.e2e_tests_number_processes) }} DEVCONTAINER_TAG: ${{ needs.pr_comment.outputs.prRefId }} secrets: AAD_TENANT_ID: ${{ secrets.AAD_TENANT_ID }} diff --git a/e2e_tests/helpers.py b/e2e_tests/helpers.py index 9c582f9dce..6195436c35 100644 --- a/e2e_tests/helpers.py +++ b/e2e_tests/helpers.py @@ -4,7 +4,7 @@ from httpx import AsyncClient, Timeout, Response import logging from starlette import status -from azure.identity import ClientSecretCredential, UsernamePasswordCredential +from azure.identity import ClientSecretCredential import config from e2e_tests import cloud From e4fdf2c030eca1180485ea8322cebf8bbbc6d3d6 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Mon, 19 May 2025 13:15:11 +0000 Subject: [PATCH 04/17] Threads for workspace creation, remove AAD specific tests, add airlcok manager assignemnt. --- .devcontainer/devcontainer.json | 19 +--- .github/workflows/deploy_tre.yml | 4 +- .github/workflows/pr_comment_bot.yml | 1 - Makefile | 24 ++-- config.sample.yaml | 9 ++ .../scripts/consolidate_env.sh | 0 devops/scripts/setup_local_debugging.sh | 4 + e2e_tests/.env.sample | 11 -- e2e_tests/config.py | 5 +- e2e_tests/conftest.py | 107 +++++++++++------- e2e_tests/helpers.py | 12 +- e2e_tests/pytest.ini | 2 +- e2e_tests/resources/resource.py | 3 +- e2e_tests/resources/workspace.py | 54 ++++++++- e2e_tests/test_airlock.py | 61 ++++++---- e2e_tests/test_performance.py | 5 +- e2e_tests/test_provisioned_health_api.py | 4 +- e2e_tests/test_ui.py | 6 +- e2e_tests/test_workspace_services.py | 28 +---- e2e_tests/test_workspace_templates.py | 15 +++ 20 files changed, 229 insertions(+), 145 deletions(-) rename {.devcontainer => devops}/scripts/consolidate_env.sh (100%) delete mode 100644 e2e_tests/.env.sample diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 68cd00e034..04aca6bc6d 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -101,21 +101,6 @@ "false" ] }, - { - "name": "E2E Extended AAD", - "type": "python", - "request": "launch", - "module": "pytest", - "justMyCode": true, - "cwd": "${workspaceFolder}/e2e_tests/", - "preLaunchTask": "Copy_env_file_for_e2e_debug", - "args": [ - "-m", - "extended_aad", - "--verify", - "false" - ] - }, { "name": "E2E Shared Services", "type": "python", @@ -223,12 +208,12 @@ "tasks": [ { "label": "Copy_env_file_for_api_debug", - "command": "./.devcontainer/scripts/consolidate_env.sh ${workspaceFolder} ${workspaceFolder}/api_app/.env", + "command": "./devops/scripts/consolidate_env.sh ${workspaceFolder} ${workspaceFolder}/api_app/.env", "type": "shell" }, { "label": "Copy_env_file_for_e2e_debug", - "command": "./.devcontainer/scripts/consolidate_env.sh ${workspaceFolder} ${workspaceFolder}/e2e_tests/.env", + "command": "./devops/scripts/consolidate_env.sh ${workspaceFolder} ${workspaceFolder}/e2e_tests/.env", "type": "shell" }, { diff --git a/.github/workflows/deploy_tre.yml b/.github/workflows/deploy_tre.yml index 20c4adf629..9e6ea903d7 100644 --- a/.github/workflows/deploy_tre.yml +++ b/.github/workflows/deploy_tre.yml @@ -33,8 +33,8 @@ jobs: with: ciGitRef: ${{ github.ref }} e2eTestsCustomSelector: >- - ${{ (github.event_name == 'push' && 'extended or extended_aad') - || 'extended or extended_aad or shared_services or airlock' }} + ${{ (github.event_name == 'push' && 'extended') + || 'extended or shared_services or airlock' }} environmentName: ${{ github.event.inputs.environment || 'CICD' }} E2E_TESTS_NUMBER_PROCESSES: ${{ fromJSON(vars.e2e_tests_number_processes) }} DEVCONTAINER_TAG: "latest" diff --git a/.github/workflows/pr_comment_bot.yml b/.github/workflows/pr_comment_bot.yml index 1872a790e8..eef54e1552 100644 --- a/.github/workflows/pr_comment_bot.yml +++ b/.github/workflows/pr_comment_bot.yml @@ -160,7 +160,6 @@ jobs: ciGitRef: ${{ needs.pr_comment.outputs.ciGitRef }} e2eTestsCustomSelector: >- ${{ (needs.pr_comment.outputs.command == 'run-tests-extended' && 'extended') || - (needs.pr_comment.outputs.command == 'run-tests-extended-aad' && 'extended_aad') || (needs.pr_comment.outputs.command == 'run-tests-shared-services' && 'shared_services') || (needs.pr_comment.outputs.command == 'run-tests' && '') }} environmentName: CICD diff --git a/Makefile b/Makefile index 7f14180ba7..e9ad60f078 100644 --- a/Makefile +++ b/Makefile @@ -467,10 +467,13 @@ build-and-deploy-ui: # Example: make prepare-for-e2e prepare-for-e2e: $(MAKE) workspace_bundle BUNDLE=base + $(MAKE) workspace_bundle BUNDLE=unrestricted + $(MAKE) workspace_bundle BUNDLE=airlock-import-review $(MAKE) workspace_service_bundle BUNDLE=guacamole $(MAKE) shared_service_bundle BUNDLE=gitea $(MAKE) user_resource_bundle WORKSPACE_SERVICE=guacamole BUNDLE=guacamole-azure-windowsvm $(MAKE) user_resource_bundle WORKSPACE_SERVICE=guacamole BUNDLE=guacamole-azure-linuxvm + $(MAKE) user_resource_bundle WORKSPACE_SERVICE=guacamole BUNDLE=guacamole-azure-import-reviewvm # Description: Run E2E smoke tests # The E2E smoke tests include: @@ -494,13 +497,14 @@ test-e2e-extended: ## 🧪 Run E2E extended tests $(call target_title, "Running E2E extended tests") && \ $(MAKE) test-e2e-custom SELECTOR=extended -# Description: Run E2E extended AAD tests -# # The E2E extended AAD tests include: -# # - test_create_guacamole_service_into_aad_workspace: This test will create a Guacamole service but will create a workspace and automatically register the AAD Application -# Example: make test-e2e-extended-aad -test-e2e-extended-aad: ## 🧪 Run E2E extended AAD tests - $(call target_title, "Running E2E extended AAD tests") && \ - $(MAKE) test-e2e-custom SELECTOR=extended_aad +# Description: Run E2E airlock tests +# # The E2E airlock tests include: +# # - test_airlock_flow: test import request creation and approval flow +# # - test_airlock_review_vm_flow: test import request creation and approval and creation of review VM +test-e2e-airlock: ## 🧪 Run E2E airlock tests + $(call target_title, "Running E2E airlock tests") && \ + $(MAKE) test-e2e-custom SELECTOR=airlock + # Description: Run E2E shared service tests # # The E2E shared service tests include: @@ -512,12 +516,18 @@ test-e2e-shared-services: ## 🧪 Run E2E shared service tests $(call target_title, "Running E2E shared service tests") && \ $(MAKE) test-e2e-custom SELECTOR=shared_services +test-e2e-workspace-services: ## 🧪 Run E2E workspace services tests + $(call target_title, "Running E2E workspace services tests") && \ + $(MAKE) test-e2e-custom SELECTOR=workspace_services + + # Description: Run E2E tests with custom selector # Arguments: SELECTOR - the selector to run the tests with # Example: make test-e2e-custom SELECTOR=smoke test-e2e-custom: ## 🧪 Run E2E tests with custom selector (SELECTOR=) $(call target_title, "Running E2E tests with custom selector ${SELECTOR}") \ && . ${MAKEFILE_DIR}/devops/scripts/check_dependencies.sh env,auth \ + && . ${MAKEFILE_DIR}/devops/scripts/consolidate_env.sh ${MAKEFILE_DIR} ${MAKEFILE_DIR}/e2e_tests/.env \ && . ${MAKEFILE_DIR}/devops/scripts/load_env.sh ${MAKEFILE_DIR}/e2e_tests/.env \ && cd ${MAKEFILE_DIR}/e2e_tests \ && \ diff --git a/config.sample.yaml b/config.sample.yaml index d50c7b9c66..b7a342cd95 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -116,3 +116,12 @@ developer_settings: # If you want to use TRE_URL to point to your local TRE API instance or be configured to another cloud provider # uncomment and set this variable # tre_url: __CHANGE_ME__ + +e2e_tests: +# # The number of processes to start in the resource processor VMSS image +# e2e_tests_number_processes_per_instance: 5 +# # Preexisting resource IDs to use for e2e tests +# test_workspace_id: __CHANGE_ME__ +# test_airlock_import_review_workspace_id: __CHANGE_ME__ +# test_airlock_import_review_workspace_service_id: __CHANGE_ME__ +# test_guacamole_workspace_service_id: __CHANGE_ME__ diff --git a/.devcontainer/scripts/consolidate_env.sh b/devops/scripts/consolidate_env.sh similarity index 100% rename from .devcontainer/scripts/consolidate_env.sh rename to devops/scripts/consolidate_env.sh diff --git a/devops/scripts/setup_local_debugging.sh b/devops/scripts/setup_local_debugging.sh index 2bf70a63d0..39ed547af4 100755 --- a/devops/scripts/setup_local_debugging.sh +++ b/devops/scripts/setup_local_debugging.sh @@ -162,6 +162,10 @@ sed -i '/ARM_CLIENT_SECRET/d' "${private_env_path}" sed -i '/AAD_TENANT_ID/d' "${private_env_path}" sed -i '/APPLICATION_ADMIN_CLIENT_ID/d' "${private_env_path}" sed -i '/APPLICATION_ADMIN_CLIENT_SECRET/d' "${private_env_path}" +if [[ -z ${WORKSPACE_API_CLIENT_ID:-} ]]; then + sed -i '/WORKSPACE_API_CLIENT_ID/d' "${private_env_path}" + sed -i '/WORKSPACE_API_CLIENT_SECRET/d' "${private_env_path}" +fi sed -i '/TEST_WORKSPACE_APP_ID/d' "${private_env_path}" sed -i '/TEST_WORKSPACE_APP_SECRET/d' "${private_env_path}" diff --git a/e2e_tests/.env.sample b/e2e_tests/.env.sample deleted file mode 100644 index 1e2645f607..0000000000 --- a/e2e_tests/.env.sample +++ /dev/null @@ -1,11 +0,0 @@ -# shellcheck disable=SC2148,SC2034 -# API parameters -# -------------- -# Values will be copied into /workspaces/AzureTRE/e2e_tests/.env when running tests - -# If you want to use precreated workspace for tests, set the following values -TEST_WORKSPACE_ID="" -TEST_AAD_WORKSPACE_ID="" - -# Run tests sequentially. Change this value if you want to run tests in parallel locally -E2E_TESTS_NUMBER_PROCESSES=5 diff --git a/e2e_tests/config.py b/e2e_tests/config.py index fdf58ea019..a57f920e02 100644 --- a/e2e_tests/config.py +++ b/e2e_tests/config.py @@ -11,6 +11,8 @@ TRE_URL: str = config("TRE_URL", default="") API_CLIENT_ID: str = config("API_CLIENT_ID", default="") AAD_TENANT_ID: str = config("AAD_TENANT_ID", default="") +APPLICATION_ADMIN_CLIENT_ID: str = config("APPLICATION_ADMIN_CLIENT_ID", default="") +APPLICATION_ADMIN_CLIENT_SECRET: str = config("APPLICATION_ADMIN_CLIENT_SECRET", default="") TEST_ACCOUNT_CLIENT_ID: str = config("TEST_ACCOUNT_CLIENT_ID", default="") TEST_ACCOUNT_CLIENT_SECRET: str = config("TEST_ACCOUNT_CLIENT_SECRET", default="") TEST_WORKSPACE_APP_ID: str = config("TEST_WORKSPACE_APP_ID", default="") @@ -19,7 +21,6 @@ # Set workspace id of an existing workspace to skip creation of a workspace during E2E tests TEST_WORKSPACE_ID: str = config("TEST_WORKSPACE_ID", default="") -TEST_WORKSPACE_SERVICE_ID: str = config("TEST_WORKSPACE_SERVICE_ID", default="") -TEST_AAD_WORKSPACE_ID: str = config("TEST_AAD_WORKSPACE_ID", default="") +TEST_GUACAMOLE_WORKSPACE_SERVICE_ID: str = config("TEST_GUACAMOLE_WORKSPACE_SERVICE_ID", default="") TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID: str = config("TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID", default="") TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_SERVICE_ID: str = config("TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_SERVICE_ID", default="") diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index edeef600f6..46db2b0bd5 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -2,6 +2,7 @@ import os from pathlib import Path import random +import uuid from filelock import FileLock import pytest import asyncio @@ -24,11 +25,22 @@ def pytest_addoption(parser): def pytest_configure(config): + if os.environ.get("PYTEST_RUN_NUMBER") is None: + pytest_run_number = str(uuid.uuid4()) + os.environ["PYTEST_RUN_NUMBER"] = pytest_run_number + else: + pytest_run_number = os.environ.get("PYTEST_RUN_NUMBER") worker_id = os.environ.get("PYTEST_XDIST_WORKER") if worker_id is not None: logging.basicConfig( format=config.getini("log_file_format"), - filename=f"tests_{worker_id}.log", + filename=f"pytest_log_{pytest_run_number}_{worker_id}.log", + level=config.getini("log_file_level"), + ) + else: + logging.basicConfig( + format=config.getini("log_file_format"), + filename=f"pytest_log_{pytest_run_number}.log", level=config.getini("log_file_level"), ) @@ -69,10 +81,13 @@ async def create_or_get_test_workspace( if auth_type == "Manual": payload["properties"]["client_id"] = client_id payload["properties"]["client_secret"] = client_secret + else: + payload["properties"]["create_aad_groups"] = True admin_token = await get_admin_token(verify=verify) # 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) LOGGER.info(f"Workspace {workspace_id} {template_name} created") @@ -124,14 +139,11 @@ def write_json_file(file_path: Path, data: dict): file_path.write_text(json.dumps(data)) -# Constants for repeated strings -LOCK_SUFFIX = ".lock" -WORKSPACE_DETAILS_FILE = "setup_test_workspace.json" -WORKER_LOCKS_DIR = "worker_locks" - +async def setup_test_base_workspace_with_locks(worker_id: str, tmp_path_factory, verify) -> AsyncGenerator[Tuple[str, str], None]: + LOCK_SUFFIX = ".lock" + WORKSPACE_DETAILS_FILE = "setup_test_workspace.json" + WORKER_LOCKS_DIR = "worker_locks" -@pytest.fixture(scope="session") -async def setup_test_workspace(verify: bool, worker_id: str, tmp_path_factory) -> AsyncGenerator[Tuple[str, str], None]: root_tmp_dir = tmp_path_factory.getbasetemp().parent fn = root_tmp_dir / WORKSPACE_DETAILS_FILE lock_fn = root_tmp_dir / (WORKSPACE_DETAILS_FILE + LOCK_SUFFIX) @@ -145,11 +157,11 @@ async def setup_test_workspace(verify: bool, worker_id: str, tmp_path_factory) - try: if worker_id == "master": workspace_path, workspace_id = await create_or_get_test_workspace( - auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", - verify=verify, + auth_type="Automatic", pre_created_workspace_id=config.TEST_WORKSPACE_ID, client_id=config.TEST_WORKSPACE_APP_ID, - client_secret=config.TEST_WORKSPACE_APP_SECRET + client_secret=config.TEST_WORKSPACE_APP_SECRET, + verify=verify ) else: with FileLock(lock_fn): @@ -159,11 +171,11 @@ async def setup_test_workspace(verify: bool, worker_id: str, tmp_path_factory) - workspace_id = data["workspace_id"] else: workspace_path, workspace_id = await create_or_get_test_workspace( - auth_type="Manual" if config.TEST_WORKSPACE_APP_ID else "Automatic", - verify=verify, + auth_type="Automatic", pre_created_workspace_id=config.TEST_WORKSPACE_ID, client_id=config.TEST_WORKSPACE_APP_ID, - client_secret=config.TEST_WORKSPACE_APP_SECRET + client_secret=config.TEST_WORKSPACE_APP_SECRET, + verify=verify ) yield workspace_path, workspace_id @@ -187,12 +199,12 @@ async def setup_test_workspace(verify: bool, worker_id: str, tmp_path_factory) - @pytest.fixture(scope="session") -async def setup_test_workspace_and_guacamole_service(setup_test_workspace, verify): +async def setup_test_base_workspace_and_guacamole_service(setup_test_base_workspace, verify): # Set up - workspace_path, workspace_id = setup_test_workspace + workspace_path, workspace_id = setup_test_base_workspace workspace_owner_token = await get_workspace_owner_token(workspace_id, verify) - pre_created_workspace_service_id = config.TEST_WORKSPACE_SERVICE_ID + pre_created_workspace_service_id = config.TEST_GUACAMOLE_WORKSPACE_SERVICE_ID workspace_service_path, workspace_service_id = await create_or_get_test_workpace_service( workspace_path, workspace_owner_token=workspace_owner_token, @@ -205,34 +217,31 @@ async def setup_test_workspace_and_guacamole_service(setup_test_workspace, verif @pytest.fixture(scope="session") -async def setup_test_aad_workspace(verify) -> AsyncGenerator[Tuple[str, str], None]: - pre_created_workspace_id = config.TEST_AAD_WORKSPACE_ID - # Set up - workspace_path, workspace_id = await create_or_get_test_workspace(auth_type="Automatic", verify=verify, pre_created_workspace_id=pre_created_workspace_id) - - yield workspace_path, workspace_id - - # Tear-down - await clean_up_test_workspace(pre_created_workspace_id=pre_created_workspace_id, workspace_path=workspace_path, verify=verify) - - -async def get_workspace_owner_token(workspace_id, verify): - 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) - return workspace_owner_token - - -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) +async def setup_airlock_test_workspaces_and_guacamole_service(worker_id, tmp_path_factory, verify): + # Start both async generators concurrently and get their first yielded values + base_workspace_function = setup_test_base_workspace_with_locks(worker_id, tmp_path_factory, verify) + airlock_function = setup_test_airlock_import_review_workspace_and_guacamole_service(verify) + base_result, airlock_result = await asyncio.gather(base_workspace_function.__anext__(), airlock_function.__anext__()) + try: + yield (*base_result, *airlock_result) + finally: + await asyncio.gather( + base_workspace_function.aclose(), + airlock_function.aclose(), + ) -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) +@pytest.fixture(scope="session") +async def setup_test_base_workspace(worker_id, tmp_path_factory, verify) -> AsyncGenerator[Tuple[str, str], None]: + # Set up + base_workspace_function = setup_test_base_workspace_with_locks(worker_id, tmp_path_factory, verify) + base_result = await base_workspace_function.__anext__() + try: + yield base_result + finally: + await base_workspace_function.aclose() -@pytest.fixture(scope="session") async def setup_test_airlock_import_review_workspace_and_guacamole_service(verify) -> AsyncGenerator[Tuple[str, str, str, str, str], None]: pre_created_workspace_id = config.TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID # Set up @@ -252,3 +261,19 @@ async def setup_test_airlock_import_review_workspace_and_guacamole_service(verif # 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) + + +async def get_workspace_owner_token(workspace_id, verify): + admin_token = await get_admin_token(verify) + workspace_owner_token, _ = await get_workspace_auth_details(admin_token=admin_token, workspace_id=workspace_id, verify=verify) + return workspace_owner_token + + +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) + + +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) diff --git a/e2e_tests/helpers.py b/e2e_tests/helpers.py index 6195436c35..df42227aa8 100644 --- a/e2e_tests/helpers.py +++ b/e2e_tests/helpers.py @@ -106,18 +106,24 @@ async def check_aad_auth_redirect(endpoint, verify) -> None: async def get_admin_token(verify) -> str: scope_uri = f"api://{config.API_CLIENT_ID}" - return get_token(scope_uri, verify) + return get_token(scope_uri) -def get_token(scope_uri, verify) -> str: +def get_token(scope_uri) -> str: # Logging in as an Enterprise Application: Use Client Credentials flow - credential = ClientSecretCredential(config.AAD_TENANT_ID, config.TEST_ACCOUNT_CLIENT_ID, config.TEST_ACCOUNT_CLIENT_SECRET, connection_verify=verify, authority=cloud.get_aad_authority_fqdn()) + credential = ClientSecretCredential(config.AAD_TENANT_ID, config.TEST_ACCOUNT_CLIENT_ID, config.TEST_ACCOUNT_CLIENT_SECRET, authority=cloud.get_aad_authority_fqdn()) token = credential.get_token(f'{scope_uri}/.default') return token.token +def get_msgraph_token() -> str: + credential = ClientSecretCredential(config.AAD_TENANT_ID, config.APPLICATION_ADMIN_CLIENT_ID, config.APPLICATION_ADMIN_CLIENT_SECRET, authority=cloud.get_aad_authority_fqdn()) + token = credential.get_token('https://graph.microsoft.com/.default') + return token.token + + def assert_status(response: Response, expected_status: List[int] = [200], message_prefix: str = "Unexpected HTTP Status"): assert response.status_code in expected_status, \ f"{message_prefix}. Expected: {expected_status}. Actual: {response.status_code}. Response text: {response.text}" diff --git a/e2e_tests/pytest.ini b/e2e_tests/pytest.ini index 3e3cf490e3..7476b313db 100644 --- a/e2e_tests/pytest.ini +++ b/e2e_tests/pytest.ini @@ -2,7 +2,7 @@ markers = smoke: marks tests as smoke (run sometimes, relatively fast) extended: marks tests as extended (run less frequently, relatively slow) - extended_aad + manual shared_services performance: marks tests for performance evaluation timeout: used to set test timeout with pytest-timeout diff --git a/e2e_tests/resources/resource.py b/e2e_tests/resources/resource.py index d684ace4fd..d57307a82b 100644 --- a/e2e_tests/resources/resource.py +++ b/e2e_tests/resources/resource.py @@ -1,5 +1,6 @@ import asyncio import logging +from typing import Tuple from httpx import AsyncClient, Timeout from starlette import status from e2e_tests.helpers import assert_status, get_auth_header, get_full_endpoint @@ -22,7 +23,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) -> Tuple[str, str]: async with AsyncClient(verify=verify, timeout=30.0) as client: full_endpoint = get_full_endpoint(endpoint) diff --git a/e2e_tests/resources/workspace.py b/e2e_tests/resources/workspace.py index 2518ba9a07..97fc62ad8d 100644 --- a/e2e_tests/resources/workspace.py +++ b/e2e_tests/resources/workspace.py @@ -1,7 +1,7 @@ import logging from httpx import AsyncClient, Timeout from typing import Tuple -from e2e_tests.helpers import get_auth_header, get_full_endpoint, get_token +from e2e_tests.helpers import get_auth_header, get_full_endpoint, get_msgraph_token, get_token LOGGER = logging.getLogger(__name__) TIMEOUT = Timeout(10, read=30) @@ -29,13 +29,61 @@ async def get_identifier_uri(client, workspace_id: str, auth_headers) -> str: raise Exception("Scope Id not found in workspace properties.") # Cope with the fact that scope id can have api:// at the front. - return f"api://{workspace['properties']['scope_id'].replace('api://','')}" + return f"api://{workspace['properties']['scope_id'].replace('api://', '')}" async def get_workspace_auth_details(admin_token, workspace_id, verify) -> Tuple[str, str]: async with AsyncClient(verify=verify) as client: auth_headers = get_auth_header(admin_token) scope_uri = await get_identifier_uri(client, workspace_id, auth_headers) - access_token = get_token(scope_uri, verify) + access_token = get_token(scope_uri) return access_token, scope_uri + + +async def assign_airlock_manager_role(admin_token, workspace_id, verify) -> None: + async with AsyncClient(verify=verify) as client: + access_token = get_msgraph_token() + auth_headers = get_auth_header(admin_token) + workspace = await get_workspace(client, workspace_id, auth_headers) + + app_sp_id = workspace.get("properties", {}).get("sp_id") + app_role_id = workspace.get("properties", {}).get("app_role_id_workspace_airlock_manager") + user_object_id = workspace["user"]["id"] + + graph_api_url = "https://graph.microsoft.com/v1.0" + headers = { + "Authorization": f"Bearer {access_token}", + "Content-Type": "application/json" + } + + # check if the role is already assigned + check_response = await client.get( + f"{graph_api_url}/servicePrincipals/{user_object_id}/appRoleAssignments", + headers=headers + ) + if check_response.status_code != 200: + LOGGER.error(f"Failed to check role assignment for user {user_object_id}: {check_response.status_code}") + raise Exception(f"Failed to check role assignment for user {user_object_id}: {check_response.status_code}") + + assignments = check_response.json().get('value', []) + + if any(a['appRoleId'] == app_role_id for a in assignments): + LOGGER.info(f"Role Airlock Manager already assigned to user {user_object_id} in workspace {workspace_id}") + return + + payload = { + "principalId": user_object_id, + "resourceId": app_sp_id, + "appRoleId": app_role_id + } + + response = await client.post( + f"{graph_api_url}/servicePrincipals/{user_object_id}/appRoleAssignments", + headers=headers, + json=payload + ) + + if response.status_code != 201: + LOGGER.error(f"Failed to assign Airlock Manager to workspace {workspace_id}: {response.status_code}") + raise Exception(f"Failed to assign role Airlock Manager to workspace {workspace_id}: {response.status_code}") diff --git a/e2e_tests/test_airlock.py b/e2e_tests/test_airlock.py index 051a5c9d81..326a2bf4bd 100644 --- a/e2e_tests/test_airlock.py +++ b/e2e_tests/test_airlock.py @@ -8,9 +8,9 @@ from airlock.request import post_request, get_request, upload_blob_using_sas, wait_for_status from resources.resource import get_resource, post_resource -from resources.workspace import get_workspace_auth_details +from resources.workspace import assign_airlock_manager_role, get_workspace_auth_details from airlock import strings as airlock_strings -from e2e_tests.conftest import get_workspace_owner_token +from conftest import get_workspace_owner_token from helpers import get_admin_token @@ -60,7 +60,7 @@ async def submit_airlock_import_request(workspace_path: str, workspace_owner_tok raise Exception("upload failed") except ResourceNotFoundError: i += 1 - LOGGER.info(f"sleeping for {wait_time} sec until container would be created") + LOGGER.info(f"Waiting {wait_time}s for container creation. If it does not create check airlock processor.") await asyncio.sleep(wait_time) pass except Exception as e: @@ -79,11 +79,19 @@ async def submit_airlock_import_request(workspace_path: str, workspace_owner_tok @pytest.mark.timeout(50 * 60) @pytest.mark.airlock -async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_import_review_workspace_and_guacamole_service, verify): - workspace_path, workspace_id = setup_test_workspace - workspace_owner_token = await get_workspace_owner_token(workspace_id, verify) +async def test_airlock_review_vm_flow(setup_airlock_test_workspaces_and_guacamole_service, verify): - _, import_review_workspace_id, _, import_review_workspace_service_id = setup_test_airlock_import_review_workspace_and_guacamole_service + base_workspace_path, base_workspace_id, import_review_workspace_path, import_review_workspace_id, guacamole_workspace_service_path, guacamole_workspace_service_id = setup_airlock_test_workspaces_and_guacamole_service + + # Preparation: Assign the airlock manager role to the user + await assign_airlock_manager_role( + admin_token=await get_admin_token(verify), + workspace_id=base_workspace_id, + verify=verify + ) + + # Preparation: Get the workspace owner token + workspace_owner_token = await get_workspace_owner_token(base_workspace_id, verify) # Preparation: Update the research workspace so that it has the import review details patch_payload = { @@ -93,7 +101,7 @@ async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_i "airlock_review_config": { "import": { "import_vm_workspace_id": import_review_workspace_id, - "import_vm_workspace_service_id": import_review_workspace_service_id, + "import_vm_workspace_service_id": guacamole_workspace_service_id, "import_vm_user_resource_template_name": "tre-service-guacamole-import-reviewvm" }, "export": { @@ -104,12 +112,12 @@ async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_i } } # Get workspace to get the etag - workspace = await get_resource(f"/api{workspace_path}", workspace_owner_token, verify) + workspace = await get_resource(f"/api{base_workspace_path}", workspace_owner_token, verify) admin_token = await get_admin_token(verify) await post_resource( payload=patch_payload, - endpoint=f"/api{workspace_path}", + endpoint=f"/api{base_workspace_path}", access_token=admin_token, verify=verify, method="PATCH", @@ -119,7 +127,7 @@ async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_i # IMPORT FLOW # Submit the request - request_id, _ = await submit_airlock_import_request(workspace_path, workspace_owner_token, verify) + request_id, _ = await submit_airlock_import_request(base_workspace_path, workspace_owner_token, verify) LOGGER.info(f'Airlock Request ID {request_id} has been created') @@ -128,7 +136,7 @@ async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_i import_workspace_owner_token, _ = await get_workspace_auth_details(admin_token=admin_token, workspace_id=import_review_workspace_id, verify=verify) user_resource_path, user_resource_id = await post_resource( payload={}, - endpoint=f"/api{workspace_path}/requests/{request_id}/review-user-resource", + endpoint=f"/api{base_workspace_path}/requests/{request_id}/review-user-resource", access_token=workspace_owner_token, verify=verify, method="POST", @@ -144,10 +152,10 @@ async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_i "approval": "True", "decisionExplanation": "the reason why this request was approved/rejected" } - request_result = await post_request(payload, f'/api{workspace_path}/requests/{request_id}/review', workspace_owner_token, verify, 200) + request_result = await post_request(payload, f'/api{base_workspace_path}/requests/{request_id}/review', workspace_owner_token, verify, 200) assert request_result["airlockRequest"]["reviews"][0]["decisionExplanation"] == "the reason why this request was approved/rejected" - await wait_for_status(airlock_strings.APPROVED_STATUS, workspace_owner_token, workspace_path, request_id, verify) + await wait_for_status(airlock_strings.APPROVED_STATUS, workspace_owner_token, base_workspace_path, request_id, verify) LOGGER.info("Airlock request has been approved") # Check that deletion for user resource has started @@ -162,13 +170,20 @@ async def test_airlock_review_vm_flow(setup_test_workspace, setup_test_airlock_i @pytest.mark.airlock @pytest.mark.extended @pytest.mark.timeout(35 * 60) -async def test_airlock_flow(setup_test_workspace, verify) -> None: +async def test_airlock_flow(setup_test_base_workspace, verify) -> None: # 1. Get the workspace set up - workspace_path, workspace_id = setup_test_workspace - workspace_owner_token = await get_workspace_owner_token(workspace_id, verify) + base_workspace_path, base_workspace_id = setup_test_base_workspace + workspace_owner_token = await get_workspace_owner_token(base_workspace_id, verify) + + # Preparation: Assign the airlock manager role to the user + await assign_airlock_manager_role( + admin_token=await get_admin_token(verify), + workspace_id=base_workspace_id, + verify=verify + ) # 2. create and submit airlock request - request_id, container_url = await submit_airlock_import_request(workspace_path, workspace_owner_token, verify) + request_id, container_url = await submit_airlock_import_request(base_workspace_path, workspace_owner_token, verify) # 3. approve request LOGGER.info("Approving airlock request") @@ -176,10 +191,10 @@ async def test_airlock_flow(setup_test_workspace, verify) -> None: "approval": "True", "decisionExplanation": "the reason why this request was approved/rejected" } - request_result = await post_request(payload, f'/api{workspace_path}/requests/{request_id}/review', workspace_owner_token, verify, 200) + request_result = await post_request(payload, f'/api{base_workspace_path}/requests/{request_id}/review', workspace_owner_token, verify, 200) assert request_result["airlockRequest"]["reviews"][0]["decisionExplanation"] == "the reason why this request was approved/rejected" - await wait_for_status(airlock_strings.APPROVED_STATUS, workspace_owner_token, workspace_path, request_id, verify) + await wait_for_status(airlock_strings.APPROVED_STATUS, workspace_owner_token, base_workspace_path, request_id, verify) # 4. check the file has been deleted from the source # NOTE: We should really be checking that the file is deleted from in progress location too, @@ -198,7 +213,7 @@ async def test_airlock_flow(setup_test_workspace, verify) -> None: # 5. get a link to the blob in the approved location. # For a full E2E we should try to download it, but can't without special networking setup. # So at the very least we check that we get the link for it. - request_result = await get_request(f'/api{workspace_path}/requests/{request_id}/link', workspace_owner_token, verify, 200) + request_result = await get_request(f'/api{base_workspace_path}/requests/{request_id}/link', workspace_owner_token, verify, 200) container_url = request_result["containerUrl"] # 6. create airlock export request @@ -209,7 +224,7 @@ async def test_airlock_flow(setup_test_workspace, verify) -> None: "businessJustification": justification } - request_result = await post_request(payload, f'/api{workspace_path}/requests', workspace_owner_token, verify, 201) + request_result = await post_request(payload, f'/api{base_workspace_path}/requests', workspace_owner_token, verify, 201) assert request_result["airlockRequest"]["type"] == airlock_strings.EXPORT assert request_result["airlockRequest"]["businessJustification"] == justification @@ -219,7 +234,7 @@ async def test_airlock_flow(setup_test_workspace, verify) -> None: # 7. get container link LOGGER.info("Getting airlock request container URL") - request_result = await get_request(f'/api{workspace_path}/requests/{request_id}/link', workspace_owner_token, verify, 200) + request_result = await get_request(f'/api{base_workspace_path}/requests/{request_id}/link', workspace_owner_token, verify, 200) container_url = request_result["containerUrl"] # we can't test any more the export flow since we don't have the network # access to upload the file from within the workspace. diff --git a/e2e_tests/test_performance.py b/e2e_tests/test_performance.py index 7fa0208a7c..576f069b52 100644 --- a/e2e_tests/test_performance.py +++ b/e2e_tests/test_performance.py @@ -27,8 +27,7 @@ async def test_parallel_resource_creations(verify) -> None: "display_name": f'Perf Test Workspace {i}', "description": "workspace for perf test", "address_space_size": "small", - "auth_type": "Manual", - "client_id": f"{config.TEST_WORKSPACE_APP_ID}" + "auth_type": "Automatic" } } @@ -80,7 +79,7 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(verify) -> workspace_owner_token, scope_uri = await get_workspace_auth_details(admin_token=admin_token, workspace_id=workspace_id, verify=verify) - workspace_service_id = config.TEST_WORKSPACE_SERVICE_ID + workspace_service_id = config.TEST_GUACAMOLE_WORKSPACE_SERVICE_ID if workspace_service_id == "": # create a guac service diff --git a/e2e_tests/test_provisioned_health_api.py b/e2e_tests/test_provisioned_health_api.py index 92636d12dc..ac4f911c9c 100644 --- a/e2e_tests/test_provisioned_health_api.py +++ b/e2e_tests/test_provisioned_health_api.py @@ -9,8 +9,8 @@ @pytest.mark.smoke -async def test_health() -> None: - async with AsyncClient(verify=False) as client: +async def test_health(verify) -> None: + async with AsyncClient(verify) as client: url = f"{config.TRE_URL}{strings.API_HEALTH}" response = await client.get(url) assert response.status_code == 200 diff --git a/e2e_tests/test_ui.py b/e2e_tests/test_ui.py index 8d71827fdb..a1b7f48cf5 100644 --- a/e2e_tests/test_ui.py +++ b/e2e_tests/test_ui.py @@ -8,10 +8,10 @@ @pytest.mark.smoke -async def test_ui() -> None: +async def test_ui(verify) -> None: endpoint = f"{config.TRE_URL}" - async with AsyncClient(verify=False) as client: - response = await client.get(endpoint) + async with AsyncClient(verify) as client: + response = await client.get(endpoint, verify=verify) assert response.status_code == 200 assert "Azure TRE" in response.text diff --git a/e2e_tests/test_workspace_services.py b/e2e_tests/test_workspace_services.py index c4b67036aa..a4d7c5619f 100644 --- a/e2e_tests/test_workspace_services.py +++ b/e2e_tests/test_workspace_services.py @@ -10,18 +10,16 @@ workspace_services = [ strings.AZUREML_SERVICE, strings.GITEA_SERVICE, - strings.MLFLOW_SERVICE, strings.MYSQL_SERVICE, strings.HEALTH_SERVICE, - strings.AZURESQL_SERVICE, - strings.OPENAI_SERVICE + strings.AZURESQL_SERVICE ] @pytest.mark.extended @pytest.mark.timeout(75 * 60) -async def test_create_guacamole_service_into_base_workspace(setup_test_workspace_and_guacamole_service, verify) -> None: - _, workspace_id, workspace_service_path, _ = setup_test_workspace_and_guacamole_service +async def test_create_guacamole_service_into_base_workspace(setup_test_base_workspace_and_guacamole_service, verify) -> None: + _, workspace_id, workspace_service_path, _ = setup_test_base_workspace_and_guacamole_service workspace_owner_token = await get_workspace_owner_token(workspace_id, verify) await ping_guacamole_workspace_service(workspace_service_path, workspace_owner_token, verify) @@ -39,26 +37,6 @@ async def test_create_guacamole_service_into_base_workspace(setup_test_workspace _, _ = await post_resource(user_resource_payload, f'/api{workspace_service_path}/{strings.API_USER_RESOURCES}', workspace_owner_token, verify, method="POST") -@pytest.mark.extended_aad -@pytest.mark.timeout(75 * 60) -async def test_create_guacamole_service_into_aad_workspace(setup_test_aad_workspace, verify) -> None: - """This test will create a Guacamole service but will create a workspace and automatically register the AAD Application""" - workspace_path, workspace_id = setup_test_aad_workspace - workspace_owner_token = await get_workspace_owner_token(workspace_id, verify) - - workspace_service_payload = { - "templateName": strings.GUACAMOLE_SERVICE, - "properties": { - "display_name": "Workspace service test", - "description": "Workspace service for E2E test" - } - } - - workspace_service_path, _ = await post_resource(workspace_service_payload, f'/api{workspace_path}/{strings.API_WORKSPACE_SERVICES}', workspace_owner_token, verify) - - await ping_guacamole_workspace_service(workspace_service_path, workspace_owner_token, verify) - - async def ping_guacamole_workspace_service(workspace_service_path, access_token, verify) -> None: workspace_service = await get_resource(f"/api{workspace_service_path}", access_token, verify) endpoint = workspace_service["workspaceService"]["properties"]["admin_connection_uri"] diff --git a/e2e_tests/test_workspace_templates.py b/e2e_tests/test_workspace_templates.py index 1fed11daaa..1fdb8a0e6e 100644 --- a/e2e_tests/test_workspace_templates.py +++ b/e2e_tests/test_workspace_templates.py @@ -59,3 +59,18 @@ async def test_create_workspace_templates(template_name, verify) -> None: # Tear-down in a cascaded way await clean_up_test_workspace(pre_created_workspace_id="", workspace_path=workspace_path, verify=verify) + + +@pytest.mark.manual +async def test_manual_workspace(verify) -> None: + + workspace_path, workspace_id = await create_or_get_test_workspace(auth_type="Manual", template_name=strings.BASE_WORKSPACE, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) + async with AsyncClient() as client: + admin_token = await get_admin_token(verify) + auth_headers = get_auth_header(admin_token) + workspace = await get_workspace(client, workspace_id, auth_headers) + + assert workspace["deploymentStatus"] == strings.RESOURCE_STATUS_DEPLOYED + + # Tear-down + await clean_up_test_workspace(pre_created_workspace_id="", workspace_path=workspace_path, verify=verify) From 5ff91a81b5d369b317e062b125177e8a7709e9a9 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Mon, 19 May 2025 14:53:22 +0000 Subject: [PATCH 05/17] Get token after role assignment. --- e2e_tests/test_airlock.py | 4 +++- templates/workspaces/base/terraform/providers.tf | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/e2e_tests/test_airlock.py b/e2e_tests/test_airlock.py index 326a2bf4bd..4d31089ef9 100644 --- a/e2e_tests/test_airlock.py +++ b/e2e_tests/test_airlock.py @@ -173,7 +173,6 @@ async def test_airlock_review_vm_flow(setup_airlock_test_workspaces_and_guacamol async def test_airlock_flow(setup_test_base_workspace, verify) -> None: # 1. Get the workspace set up base_workspace_path, base_workspace_id = setup_test_base_workspace - workspace_owner_token = await get_workspace_owner_token(base_workspace_id, verify) # Preparation: Assign the airlock manager role to the user await assign_airlock_manager_role( @@ -182,6 +181,9 @@ async def test_airlock_flow(setup_test_base_workspace, verify) -> None: verify=verify ) + # Preparation: Get the workspace owner token after assigning the airlock manager role + workspace_owner_token = await get_workspace_owner_token(base_workspace_id, verify) + # 2. create and submit airlock request request_id, container_url = await submit_airlock_import_request(base_workspace_path, workspace_owner_token, verify) diff --git a/templates/workspaces/base/terraform/providers.tf b/templates/workspaces/base/terraform/providers.tf index 8d151c7b9c..a20f810930 100644 --- a/templates/workspaces/base/terraform/providers.tf +++ b/templates/workspaces/base/terraform/providers.tf @@ -31,6 +31,9 @@ provider "azurerm" { recover_soft_deleted_certificates = true recover_soft_deleted_keys = true } + resource_group { + prevent_deletion_if_contains_resources = false + } } storage_use_azuread = true } From 98714a44724b7af3c14c57c3cd2275cd250a01a3 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Mon, 19 May 2025 15:18:38 +0000 Subject: [PATCH 06/17] Revert workspace change. --- templates/workspaces/base/terraform/providers.tf | 3 --- 1 file changed, 3 deletions(-) diff --git a/templates/workspaces/base/terraform/providers.tf b/templates/workspaces/base/terraform/providers.tf index a20f810930..8d151c7b9c 100644 --- a/templates/workspaces/base/terraform/providers.tf +++ b/templates/workspaces/base/terraform/providers.tf @@ -31,9 +31,6 @@ provider "azurerm" { recover_soft_deleted_certificates = true recover_soft_deleted_keys = true } - resource_group { - prevent_deletion_if_contains_resources = false - } } storage_use_azuread = true } From 84f8dfd56338ff20c597a7a53af3bbc15936b218 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Mon, 19 May 2025 15:27:35 +0000 Subject: [PATCH 07/17] Fix writing workspace ID --- e2e_tests/conftest.py | 5 +++++ e2e_tests/test_workspace_services.py | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index 46db2b0bd5..160ad96a60 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -177,6 +177,11 @@ async def setup_test_base_workspace_with_locks(worker_id: str, tmp_path_factory, client_secret=config.TEST_WORKSPACE_APP_SECRET, verify=verify ) + data = { + "workspace_path": workspace_path, + "workspace_id": workspace_id + } + write_json_file(fn, data) yield workspace_path, workspace_id diff --git a/e2e_tests/test_workspace_services.py b/e2e_tests/test_workspace_services.py index a4d7c5619f..507c5c11a3 100644 --- a/e2e_tests/test_workspace_services.py +++ b/e2e_tests/test_workspace_services.py @@ -46,8 +46,8 @@ async def ping_guacamole_workspace_service(workspace_service_path, access_token, @pytest.mark.workspace_services @pytest.mark.timeout(45 * 60) @pytest.mark.parametrize("template_name", workspace_services) -async def test_install_workspace_service(template_name, verify, setup_test_workspace) -> None: - workspace_path, workspace_id = setup_test_workspace +async def test_install_workspace_service(template_name, verify, setup_test_base_workspace) -> None: + workspace_path, workspace_id = setup_test_base_workspace workspace_owner_token = await get_workspace_owner_token(workspace_id, verify) service_payload = { From 51d8c5db8afbb6f016e1b780e330dcf44276dc58 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 08:34:50 +0000 Subject: [PATCH 08/17] Fix az login --- core/terraform/resource_processor/vmss_porter/cloud-config.yaml | 2 +- core/version.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/terraform/resource_processor/vmss_porter/cloud-config.yaml b/core/terraform/resource_processor/vmss_porter/cloud-config.yaml index d9bd4411da..e8e57ed054 100644 --- a/core/terraform/resource_processor/vmss_porter/cloud-config.yaml +++ b/core/terraform/resource_processor/vmss_porter/cloud-config.yaml @@ -84,7 +84,7 @@ runcmd: - printf '\nalias rpstatus='\''tmux new-session -d "watch docker ps"; tmux split-window -p 100 -v "docker logs --since 1m --follow resource_processor1"; tmux split-window -v -p 90; tmux -2 attach-session -d'\''\n' >> /etc/bash.bashrc - export DEBIAN_FRONTEND=noninteractive - az cloud set --name ${azure_environment} - - az login --identity -u ${vmss_msi_id} + - az login --identity --client-id ${vmss_msi_id} - az acr login --name ${docker_registry_server} - docker run -d -p 8080:8080 -v /var/run/docker.sock:/var/run/docker.sock --restart always --env-file .env diff --git a/core/version.txt b/core/version.txt index f2c260cc17..e1130451f1 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.14.6" +__version__ = "0.14.7" From 6b728b65b84c81f4fdd4dc7e95a2eb8189b239ca Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 11:23:28 +0000 Subject: [PATCH 09/17] Skip writing to file if set to use env vars --- Makefile | 2 +- devops/scripts/consolidate_env.sh | 28 ++++++++++++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index e9ad60f078..4f46fd1400 100644 --- a/Makefile +++ b/Makefile @@ -527,7 +527,7 @@ test-e2e-workspace-services: ## 🧪 Run E2E workspace services tests test-e2e-custom: ## 🧪 Run E2E tests with custom selector (SELECTOR=) $(call target_title, "Running E2E tests with custom selector ${SELECTOR}") \ && . ${MAKEFILE_DIR}/devops/scripts/check_dependencies.sh env,auth \ - && . ${MAKEFILE_DIR}/devops/scripts/consolidate_env.sh ${MAKEFILE_DIR} ${MAKEFILE_DIR}/e2e_tests/.env \ + && . ${MAKEFILE_DIR}/devops/scripts/consolidate_env.sh ${MAKEFILE_DIR} ${MAKEFILE_DIR}/e2e_tests/.env \ && . ${MAKEFILE_DIR}/devops/scripts/load_env.sh ${MAKEFILE_DIR}/e2e_tests/.env \ && cd ${MAKEFILE_DIR}/e2e_tests \ && \ diff --git a/devops/scripts/consolidate_env.sh b/devops/scripts/consolidate_env.sh index f64465b78d..6e52f8e055 100755 --- a/devops/scripts/consolidate_env.sh +++ b/devops/scripts/consolidate_env.sh @@ -10,17 +10,21 @@ set -o nounset WORKDIR=${1:-"automatic"} FILE=${2:-"automatic"} -# YQ query to get leaf keys -GET_LEAF_KEYS=".. | select(. == \"*\") | {(path | .[-1]): .} " -# YQ query to uppercase keys -UPCASE_KEYS="with_entries(.key |= upcase)" -# YQ query to map yaml entries to the following format: key=value -# needed for later env export -FORMAT_TO_ENV_FILE="to_entries| map(.key + \"=\" + .value)|.[]" +if [ -z "${USE_ENV_VARS_NOT_FILES:-}" ]; then + # YQ query to get leaf keys + GET_LEAF_KEYS=".. | select(. == \"*\") | {(path | .[-1]): .} " + # YQ query to uppercase keys + UPCASE_KEYS="with_entries(.key |= upcase)" + # YQ query to map yaml entries to the following format: key=value + # needed for later env export + FORMAT_TO_ENV_FILE="to_entries| map(.key + \"=\" + .value)|.[]" -# Export as UPPERCASE keys to file -# shellcheck disable=SC2086 -yq e "$GET_LEAF_KEYS|$UPCASE_KEYS| $FORMAT_TO_ENV_FILE" config.yaml > $FILE + # Export as UPPERCASE keys to file + # shellcheck disable=SC2086 + yq e "$GET_LEAF_KEYS|$UPCASE_KEYS| $FORMAT_TO_ENV_FILE" config.yaml > $FILE -# shellcheck disable=SC2086 -cat $WORKDIR/core/private.env >> $FILE + if [ -f $WORKDIR/core/private.env ]; then + # shellcheck disable=SC2086 + cat $WORKDIR/core/private.env >> $FILE + fi +fi From ab28fbbe08cfd5a8413fcc0466a266a266b3a8d4 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 11:54:17 +0000 Subject: [PATCH 10/17] Remove verify. --- e2e_tests/test_ui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e_tests/test_ui.py b/e2e_tests/test_ui.py index a1b7f48cf5..9cc94251bb 100644 --- a/e2e_tests/test_ui.py +++ b/e2e_tests/test_ui.py @@ -12,6 +12,6 @@ async def test_ui(verify) -> None: endpoint = f"{config.TRE_URL}" async with AsyncClient(verify) as client: - response = await client.get(endpoint, verify=verify) + response = await client.get(endpoint) assert response.status_code == 200 assert "Azure TRE" in response.text From 263d08daf3aabf83054a25bb1021c84987538dea Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 13:44:46 +0100 Subject: [PATCH 11/17] fix linting --- devops/scripts/consolidate_env.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/devops/scripts/consolidate_env.sh b/devops/scripts/consolidate_env.sh index 6e52f8e055..9ae085ef66 100755 --- a/devops/scripts/consolidate_env.sh +++ b/devops/scripts/consolidate_env.sh @@ -20,11 +20,9 @@ if [ -z "${USE_ENV_VARS_NOT_FILES:-}" ]; then FORMAT_TO_ENV_FILE="to_entries| map(.key + \"=\" + .value)|.[]" # Export as UPPERCASE keys to file - # shellcheck disable=SC2086 yq e "$GET_LEAF_KEYS|$UPCASE_KEYS| $FORMAT_TO_ENV_FILE" config.yaml > $FILE - if [ -f $WORKDIR/core/private.env ]; then - # shellcheck disable=SC2086 - cat $WORKDIR/core/private.env >> $FILE + if [ -f "$WORKDIR/core/private.env" ]; then + cat "$WORKDIR/core/private.env" >> $FILE fi fi From d9577532656c7a293e5e3b5ec132fb3c555dbc2f Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 14:16:54 +0000 Subject: [PATCH 12/17] Add APPLICATION_ADMIN_CLIENT_ID and secret to e2e tests workflow. --- .github/workflows/deploy_tre_reusable.yml | 302 +++++++++++++++------- 1 file changed, 202 insertions(+), 100 deletions(-) diff --git a/.github/workflows/deploy_tre_reusable.yml b/.github/workflows/deploy_tre_reusable.yml index 09437893df..f698bc45a3 100644 --- a/.github/workflows/deploy_tre_reusable.yml +++ b/.github/workflows/deploy_tre_reusable.yml @@ -290,7 +290,12 @@ jobs: strategy: fail-fast: true matrix: - target: [build-and-push-api, build-and-push-resource-processor, build-and-push-airlock-processor] + target: + [ + build-and-push-api, + build-and-push-resource-processor, + build-and-push-airlock-processor, + ] steps: - name: Checkout @@ -404,38 +409,70 @@ jobs: strategy: matrix: include: - - {BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/base"} - - {BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/unrestricted"} - - {BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/airlock-import-review"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/guacamole"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azureml"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/gitea"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/mysql"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/health-services"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/databricks"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/ohdsi"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azuresql"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/openai"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm"} + - { + BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/base", + } + - { + BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/unrestricted", + } + - { + BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/airlock-import-review", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/guacamole", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azureml", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/gitea", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/mysql", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/health-services", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/databricks", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/ohdsi", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azuresql", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/openai", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm", + } environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -471,22 +508,38 @@ jobs: strategy: matrix: include: - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/firewall/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/gitea/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/admin-vm/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/airlock_notifier/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/certs/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/cyclecloud/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/databricks-auth/"} + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/firewall/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/gitea/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/admin-vm/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/airlock_notifier/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/certs/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/cyclecloud/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/databricks-auth/", + } environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -522,22 +575,38 @@ jobs: strategy: matrix: include: - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/firewall"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/gitea"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/admin-vm/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/airlock_notifier/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/certs/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/cyclecloud/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/"} - - {BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/databricks-auth/"} + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/firewall", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/gitea", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/admin-vm/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/airlock_notifier/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/certs/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/cyclecloud/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/", + } + - { + BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/databricks-auth/", + } environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -580,28 +649,50 @@ jobs: matrix: include: # bundles type can be inferred from the bundle dir (but this is more explicit) - - {BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/base"} - - {BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/unrestricted"} - - {BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/airlock-import-review"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/guacamole"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azureml"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/gitea"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/mysql"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/health-services"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/databricks"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/ohdsi"} - - {BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azuresql"} + - { + BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/base", + } + - { + BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/unrestricted", + } + - { + BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/airlock-import-review", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/guacamole", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azureml", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/gitea", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/mysql", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/health-services", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/databricks", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/ohdsi", + } + - { + BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azuresql", + } environment: ${{ inputs.environmentName }} steps: @@ -645,18 +736,26 @@ jobs: strategy: matrix: include: - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} - - {BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole", + } + - { + BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole", + } environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -822,7 +921,8 @@ jobs: if: ${{ inputs.e2eTestsCustomSelector != '' }} runs-on: ubuntu-latest environment: ${{ inputs.environmentName }} - needs: [deploy_shared_services, register_bundles, register_user_resource_bundles] + needs: + [deploy_shared_services, register_bundles, register_user_resource_bundles] timeout-minutes: 300 steps: - name: Checkout @@ -849,6 +949,8 @@ jobs: TEST_WORKSPACE_APP_SECRET: "${{ secrets.TEST_WORKSPACE_APP_SECRET }}" TEST_ACCOUNT_CLIENT_ID: "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}" TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}" + APPLICATION_ADMIN_CLIENT_ID: "${{ secrets.APPLICATION_ADMIN_CLIENT_ID }}" + APPLICATION_ADMIN_CLIENT_SECRET: "${{ secrets.APPLICATION_ADMIN_CLIENT_SECRET }}" TRE_ID: ${{ secrets.TRE_ID }} IS_API_SECURED: false WORKSPACE_APP_SERVICE_PLAN_SKU: ${{ vars.WORKSPACE_APP_SERVICE_PLAN_SKU }} From fbdca203a4309c9add99072055285196c0198c33 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 14:46:42 +0000 Subject: [PATCH 13/17] make verify keyword argument. --- e2e_tests/test_ui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e_tests/test_ui.py b/e2e_tests/test_ui.py index 9cc94251bb..cfef4f2fea 100644 --- a/e2e_tests/test_ui.py +++ b/e2e_tests/test_ui.py @@ -11,7 +11,7 @@ async def test_ui(verify) -> None: endpoint = f"{config.TRE_URL}" - async with AsyncClient(verify) as client: + async with AsyncClient(verify=verify) as client: response = await client.get(endpoint) assert response.status_code == 200 assert "Azure TRE" in response.text From ba500a7439fb1e785321360726c9e22022c33501 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 17:16:21 +0000 Subject: [PATCH 14/17] Fix linting --- .github/workflows/deploy_tre_reusable.yml | 300 ++++++++-------------- 1 file changed, 100 insertions(+), 200 deletions(-) diff --git a/.github/workflows/deploy_tre_reusable.yml b/.github/workflows/deploy_tre_reusable.yml index f698bc45a3..8cf7048d0e 100644 --- a/.github/workflows/deploy_tre_reusable.yml +++ b/.github/workflows/deploy_tre_reusable.yml @@ -290,12 +290,7 @@ jobs: strategy: fail-fast: true matrix: - target: - [ - build-and-push-api, - build-and-push-resource-processor, - build-and-push-airlock-processor, - ] + target: [build-and-push-api, build-and-push-resource-processor, build-and-push-airlock-processor] steps: - name: Checkout @@ -409,70 +404,38 @@ jobs: strategy: matrix: include: - - { - BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/base", - } - - { - BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/unrestricted", - } - - { - BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/airlock-import-review", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/guacamole", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azureml", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/gitea", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/mysql", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/health-services", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/databricks", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/ohdsi", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azuresql", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/openai", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm", - } + - {BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/base"} + - {BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/unrestricted"} + - {BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/airlock-import-review"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/guacamole"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azureml"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/gitea"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/mysql"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/health-services"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/databricks"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/ohdsi"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azuresql"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/openai"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm"} environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -508,38 +471,22 @@ jobs: strategy: matrix: include: - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/firewall/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/gitea/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/admin-vm/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/airlock_notifier/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/certs/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/cyclecloud/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/databricks-auth/", - } + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/firewall/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/gitea/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/admin-vm/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/airlock_notifier/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/certs/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/cyclecloud/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/databricks-auth/"} environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -575,38 +522,22 @@ jobs: strategy: matrix: include: - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/firewall", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/gitea", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/admin-vm/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/airlock_notifier/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/certs/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/cyclecloud/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/", - } - - { - BUNDLE_TYPE: "shared_service", - BUNDLE_DIR: "./templates/shared_services/databricks-auth/", - } + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/firewall"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/gitea"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/admin-vm/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/airlock_notifier/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/certs/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/cyclecloud/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/sonatype-nexus-vm/"} + - {BUNDLE_TYPE: "shared_service", + BUNDLE_DIR: "./templates/shared_services/databricks-auth/"} environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -649,50 +580,28 @@ jobs: matrix: include: # bundles type can be inferred from the bundle dir (but this is more explicit) - - { - BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/base", - } - - { - BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/unrestricted", - } - - { - BUNDLE_TYPE: "workspace", - BUNDLE_DIR: "./templates/workspaces/airlock-import-review", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/guacamole", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azureml", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/gitea", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/mysql", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/health-services", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/databricks", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/ohdsi", - } - - { - BUNDLE_TYPE: "workspace_service", - BUNDLE_DIR: "./templates/workspace_services/azuresql", - } + - {BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/base"} + - {BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/unrestricted"} + - {BUNDLE_TYPE: "workspace", + BUNDLE_DIR: "./templates/workspaces/airlock-import-review"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/guacamole"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azureml"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/gitea"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/mysql"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/health-services"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/databricks"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/ohdsi"} + - {BUNDLE_TYPE: "workspace_service", + BUNDLE_DIR: "./templates/workspace_services/azuresql"} environment: ${{ inputs.environmentName }} steps: @@ -736,26 +645,18 @@ jobs: strategy: matrix: include: - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole", - } - - { - BUNDLE_TYPE: "user_resource", - BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm", - WORKSPACE_SERVICE_NAME: "tre-service-guacamole", - } + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} + - {BUNDLE_TYPE: "user_resource", + BUNDLE_DIR: "./templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm", + WORKSPACE_SERVICE_NAME: "tre-service-guacamole"} environment: ${{ inputs.environmentName }} steps: - name: Checkout @@ -921,8 +822,7 @@ jobs: if: ${{ inputs.e2eTestsCustomSelector != '' }} runs-on: ubuntu-latest environment: ${{ inputs.environmentName }} - needs: - [deploy_shared_services, register_bundles, register_user_resource_bundles] + needs: [deploy_shared_services, register_bundles, register_user_resource_bundles] timeout-minutes: 300 steps: - name: Checkout From 1dcea7bdd51727b0ab913d6704aab0adc9591bfd Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 18:23:50 +0000 Subject: [PATCH 15/17] fix linting --- devops/scripts/consolidate_env.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devops/scripts/consolidate_env.sh b/devops/scripts/consolidate_env.sh index 9ae085ef66..139af47ae2 100755 --- a/devops/scripts/consolidate_env.sh +++ b/devops/scripts/consolidate_env.sh @@ -20,9 +20,9 @@ if [ -z "${USE_ENV_VARS_NOT_FILES:-}" ]; then FORMAT_TO_ENV_FILE="to_entries| map(.key + \"=\" + .value)|.[]" # Export as UPPERCASE keys to file - yq e "$GET_LEAF_KEYS|$UPCASE_KEYS| $FORMAT_TO_ENV_FILE" config.yaml > $FILE + yq e "$GET_LEAF_KEYS|$UPCASE_KEYS| $FORMAT_TO_ENV_FILE" config.yaml > "$FILE" if [ -f "$WORKDIR/core/private.env" ]; then - cat "$WORKDIR/core/private.env" >> $FILE + cat "$WORKDIR/core/private.env" >> "$FILE" fi fi From 8d3993e426645c74b986c7289d4a9eecd88076c5 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Tue, 20 May 2025 18:26:59 +0000 Subject: [PATCH 16/17] Prevent deadlock. --- e2e_tests/conftest.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index 160ad96a60..6c2f60069d 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -190,8 +190,14 @@ async def setup_test_base_workspace_with_locks(worker_id: str, tmp_path_factory, try: if len(list(worker_lock_dir.glob("*"))) > 1: worker_lock.unlink(missing_ok=True) + timeout = 7200 # Maximum time to wait in seconds - 120 minutes + elapsed = 0 while len(list(worker_lock_dir.glob("*"))) > 0: + if elapsed >= timeout: + LOGGER.warning(f"Timeout reached while waiting for worker locks to be released in {worker_lock_dir}.") + break await asyncio.sleep(10) + elapsed += 10 elif len(list(worker_lock_dir.glob("*"))) == 1: await clean_up_test_workspace( pre_created_workspace_id=config.TEST_WORKSPACE_ID, From ed9782ef48acc45aa62d9bf9a26b4803ff77b66a Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Wed, 21 May 2025 07:24:27 +0000 Subject: [PATCH 17/17] add verify keyword --- e2e_tests/test_provisioned_health_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e_tests/test_provisioned_health_api.py b/e2e_tests/test_provisioned_health_api.py index ac4f911c9c..7ff7e21ebf 100644 --- a/e2e_tests/test_provisioned_health_api.py +++ b/e2e_tests/test_provisioned_health_api.py @@ -10,7 +10,7 @@ @pytest.mark.smoke async def test_health(verify) -> None: - async with AsyncClient(verify) as client: + async with AsyncClient(verify=verify) as client: url = f"{config.TRE_URL}{strings.API_HEALTH}" response = await client.get(url) assert response.status_code == 200