From 63ee9358b80e8769e796d61a9826bc4f7fe0387c Mon Sep 17 00:00:00 2001 From: Jonny Rylands Date: Mon, 24 Jun 2024 10:20:09 +0100 Subject: [PATCH] Expose FIREWALL_SKU as environment variable & support start/stop of SKU Basic Firewall (#3975) * Expose FIREWALL_SKU as environment variable & support start/stop of SKU Basic Firewall * Fix lint / build validation issues * Fix Lint issue & improve documentation on FIREWALL_SKU as per @marrobi * Fix build validation: update core build version number * #3975 Increment minor version numbers as per @tamirkamara review --- .../devcontainer_run_command/action.yml | 5 +++++ .github/workflows/deploy_tre_reusable.yml | 2 ++ config.sample.yaml | 1 + config_schema.json | 4 ++++ core/terraform/main.tf | 1 + .../vmss_porter/cloud-config.yaml | 1 + .../resource_processor/vmss_porter/data.tf | 1 + .../vmss_porter/variables.tf | 3 +++ core/terraform/variables.tf | 6 ++++++ core/version.txt | 2 +- devops/scripts/control_tre.sh | 21 ++++++++++++------- docs/tre-admins/environment-variables.md | 2 ++ .../cicd-pre-deployment-steps.md | 1 + .../setup-instructions/workflows.md | 1 + docs/tre-admins/start-stop.md | 4 +++- resource_processor/_version.py | 2 +- resource_processor/shared/config.py | 1 + .../shared_services/firewall/parameters.json | 4 ++-- .../shared_services/firewall/porter.yaml | 10 ++++----- .../firewall/terraform/firewall.tf | 8 +++---- .../firewall/terraform/locals.tf | 3 +++ .../firewall/terraform/variables.tf | 4 ++-- 22 files changed, 64 insertions(+), 23 deletions(-) diff --git a/.github/actions/devcontainer_run_command/action.yml b/.github/actions/devcontainer_run_command/action.yml index 18b44e2606..d4d9926e9d 100644 --- a/.github/actions/devcontainer_run_command/action.yml +++ b/.github/actions/devcontainer_run_command/action.yml @@ -125,6 +125,10 @@ inputs: description: "A boolean indicating if the purge protection will be enabled on the core keyvault." required: false default: "true" + FIREWALL_SKU: + description: "Firewall SKU" + required: false + default: "" runs: using: composite @@ -234,6 +238,7 @@ runs: && inputs.RP_BUNDLE_VALUES) || '{}' }}' \ -e TF_VAR_resource_processor_number_processes_per_instance="${{ (inputs.RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE != '' && inputs.RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE) || 5 }}" \ + -e TF_VAR_firewall_sku=${{ inputs.FIREWALL_SKU }} \ -e E2E_TESTS_NUMBER_PROCESSES="${{ inputs.E2E_TESTS_NUMBER_PROCESSES }}" \ '${{ inputs.CI_CACHE_ACR_NAME }}${{ env.ACR_DOMAIN_SUFFIX }}/tredev:${{ inputs.DEVCONTAINER_TAG }}' \ bash -c "${{ inputs.COMMAND }}" diff --git a/.github/workflows/deploy_tre_reusable.yml b/.github/workflows/deploy_tre_reusable.yml index a8a3005dc0..4b43636ab6 100644 --- a/.github/workflows/deploy_tre_reusable.yml +++ b/.github/workflows/deploy_tre_reusable.yml @@ -357,6 +357,7 @@ jobs: CORE_APP_SERVICE_PLAN_SKU: ${{ vars.CORE_APP_SERVICE_PLAN_SKU }} RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE: ${{ vars.RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE }} RP_BUNDLE_VALUES: ${{ vars.RP_BUNDLE_VALUES }} + FIREWALL_SKU: ${{ vars.FIREWALL_SKU}} - name: API Healthcheck uses: ./.github/actions/devcontainer_run_command @@ -666,6 +667,7 @@ jobs: TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}" TRE_ID: ${{ secrets.TRE_ID }} LOCATION: ${{ vars.LOCATION }} + FIREWALL_SKU: ${{ vars.FIREWALL_SKU}} - name: State Store Migrations uses: ./.github/actions/devcontainer_run_command diff --git a/config.sample.yaml b/config.sample.yaml index da877d0217..ad2aa6078c 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -41,6 +41,7 @@ tre: # 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__ + firewall_sku: Standard authentication: aad_tenant_id: __CHANGE_ME__ diff --git a/config_schema.json b/config_schema.json index 0d2a781578..a8ae1711d6 100644 --- a/config_schema.json +++ b/config_schema.json @@ -85,6 +85,10 @@ "description": "Url for the TRE environment.", "type": "string", "pattern": "^https?://" + }, + "firewall_sku": { + "description": "SKU of the Azure Firewall.", + "type": "string" } } }, diff --git a/core/terraform/main.tf b/core/terraform/main.tf index 64b3334bfc..3a102b292f 100644 --- a/core/terraform/main.tf +++ b/core/terraform/main.tf @@ -167,6 +167,7 @@ module "resource_processor_vmss_porter" { resource_processor_vmss_sku = var.resource_processor_vmss_sku arm_environment = var.arm_environment logging_level = var.logging_level + firewall_sku = var.firewall_sku rp_bundle_values = var.rp_bundle_values depends_on = [ diff --git a/core/terraform/resource_processor/vmss_porter/cloud-config.yaml b/core/terraform/resource_processor/vmss_porter/cloud-config.yaml index 1422c06e8a..b86b4c883d 100644 --- a/core/terraform/resource_processor/vmss_porter/cloud-config.yaml +++ b/core/terraform/resource_processor/vmss_porter/cloud-config.yaml @@ -57,6 +57,7 @@ write_files: AZURE_ENVIRONMENT=${azure_environment} AAD_AUTHORITY_URL=${aad_authority_url} MICROSOFT_GRAPH_FQDN=${microsoft_graph_fqdn} + FIREWALL_SKU=${firewall_sku} OTEL_RESOURCE_ATTRIBUTES=service.name=resource_processor,service.version=${resource_processor_vmss_porter_image_tag} OTEL_EXPERIMENTAL_RESOURCE_DETECTORS=azure_vm LOGGING_LEVEL=${logging_level} diff --git a/core/terraform/resource_processor/vmss_porter/data.tf b/core/terraform/resource_processor/vmss_porter/data.tf index 4ab89b4367..8f2f2d3ccd 100644 --- a/core/terraform/resource_processor/vmss_porter/data.tf +++ b/core/terraform/resource_processor/vmss_porter/data.tf @@ -29,6 +29,7 @@ data "template_file" "cloudconfig" { azure_environment = local.azure_environment aad_authority_url = module.terraform_azurerm_environment_configuration.active_directory_endpoint microsoft_graph_fqdn = regex("(?:(?P[^:/?#]+):)?(?://(?P[^/?#:]*))?", module.terraform_azurerm_environment_configuration.microsoft_graph_endpoint).fqdn + firewall_sku = var.firewall_sku logging_level = var.logging_level rp_bundle_values = local.rp_bundle_values_formatted } diff --git a/core/terraform/resource_processor/vmss_porter/variables.tf b/core/terraform/resource_processor/vmss_porter/variables.tf index 4ec2f7910e..8c2b835fa0 100644 --- a/core/terraform/resource_processor/vmss_porter/variables.tf +++ b/core/terraform/resource_processor/vmss_porter/variables.tf @@ -69,6 +69,9 @@ variable "subscription_id" { variable "logging_level" { type = string } +variable "firewall_sku" { + type = string +} variable "rp_bundle_values" { type = map(string) } diff --git a/core/terraform/variables.tf b/core/terraform/variables.tf index 724969d0b6..93d8c3a765 100644 --- a/core/terraform/variables.tf +++ b/core/terraform/variables.tf @@ -174,6 +174,12 @@ variable "enable_airlock_malware_scanning" { description = "If False, Airlock requests will skip the malware scanning stage" } +variable "firewall_sku" { + description = "Azure Firewall SKU" + type = string + default = "" +} + variable "rp_bundle_values" { description = "Additional environment values to set on the resource processor that can be supplied to template bundles" type = map(string) diff --git a/core/version.txt b/core/version.txt index 43e38a8b4c..a9b029e7c7 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.9.11" +__version__ = "0.10.0" \ No newline at end of file diff --git a/devops/scripts/control_tre.sh b/devops/scripts/control_tre.sh index c5a04f5a71..a53c6f4166 100755 --- a/devops/scripts/control_tre.sh +++ b/devops/scripts/control_tre.sh @@ -12,6 +12,8 @@ fi core_rg_name="rg-${TRE_ID}" fw_name="fw-${TRE_ID}" agw_name="agw-$TRE_ID" +fw_pip_name="pip-${fw_name}" +vnet_name="vnet-${TRE_ID}" # if the resource group doesn't exist, no need to continue this script. # most likely this is an automated execution before calling make tre-deploy. @@ -27,8 +29,14 @@ if [[ "$1" == *"start"* ]]; then if [[ $(az network firewall list --output json --query "[?resourceGroup=='${core_rg_name}'&&name=='${fw_name}'] | length(@)") != 0 ]]; then CURRENT_PUBLIC_IP=$(az network firewall ip-config list -f "${fw_name}" -g "${core_rg_name}" --query "[0].publicIpAddress" -o tsv) if [ -z "$CURRENT_PUBLIC_IP" ]; then - echo "Starting Firewall - creating ip-config" - az network firewall ip-config create -f "${fw_name}" -g "${core_rg_name}" -n "fw-ip-configuration" --public-ip-address "pip-${fw_name}" --vnet-name "vnet-$TRE_ID" > /dev/null & + FW_SKU_TIER=$(az network firewall show --n "${fw_name}" -g "${core_rg_name}" --query "sku.tier" -o tsv) + if [ "$FW_SKU_TIER" == "Basic" ]; then + echo "Starting Firewall (Basic SKU) - creating ip-config and management-ip-config" + az network firewall ip-config create -f "${fw_name}" -g "${core_rg_name}" -n "fw-ip-configuration" --public-ip-address "${fw_pip_name}" --vnet-name "${vnet_name}" --m-name "fw-management-ip-configuration" --m-public-ip-address "pip-fw-management-$TRE_ID" --m-vnet-name "${vnet_name}"> /dev/null & + else + echo "Starting Firewall - creating ip-config" + az network firewall ip-config create -f "${fw_name}" -g "${core_rg_name}" -n "fw-ip-configuration" --public-ip-address "${fw_pip_name}" --vnet-name "${vnet_name}" > /dev/null & + fi else echo "Firewall ip-config already exists" fi @@ -66,14 +74,13 @@ if [[ "$1" == *"start"* ]]; then elif [[ "$1" == *"stop"* ]]; then if [[ $(az network firewall list --output json --query "[?resourceGroup=='${core_rg_name}'&&name=='${fw_name}'] | length(@)") != 0 ]]; then - fw_sku=$(az network firewall show -n "${fw_name}" -g "${core_rg_name}" --query "sku.tier" -o tsv) IPCONFIG_NAME=$(az network firewall ip-config list -f "${fw_name}" -g "${core_rg_name}" --query "[0].name" -o tsv) - if [ -n "$IPCONFIG_NAME" ] && [ "${fw_sku}" != "Basic" ]; then - echo "Deleting Firewall ip-config: $IPCONFIG_NAME" - az network firewall ip-config delete -f "${fw_name}" -n "$IPCONFIG_NAME" -g "${core_rg_name}" & + if [ -n "$IPCONFIG_NAME" ]; then + echo "Deleting Firewall ip-config" + az network firewall update --name "${fw_name}" --resource-group "${core_rg_name}" --remove ipConfigurations --remove managementIpConfiguration & else - echo "No Firewall ip-config found or SKU (${fw_sku}) doesn't allow deallocation" + echo "No Firewall ip-config found" fi fi diff --git a/docs/tre-admins/environment-variables.md b/docs/tre-admins/environment-variables.md index 55825e8ef9..25f3e73751 100644 --- a/docs/tre-admins/environment-variables.md +++ b/docs/tre-admins/environment-variables.md @@ -39,6 +39,8 @@ | `CORE_APP_SERVICE_PLAN_SKU` | The SKU of AppService plans created for the core infrastructure. | | `WORKSPACE_APP_SERVICE_PLAN_SKU` | Optional. The SKU used for AppService plan used in E2E tests unless otherwise specified. Default value is `P1v2`. | | `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`. | +| `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). | + ## For authentication in `/config.yaml` diff --git a/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md b/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md index 07a231ef0b..af61f035f7 100644 --- a/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md +++ b/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md @@ -83,6 +83,7 @@ Configure the following **variables** in your github environment: | `WORKSPACE_APP_SERVICE_PLAN_SKU` | Optional. The SKU used for AppService plan used in E2E tests. Default value is `P1v2`. | | `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). | ### Configure Authentication Secrets diff --git a/docs/tre-admins/setup-instructions/workflows.md b/docs/tre-admins/setup-instructions/workflows.md index a432e90463..a06ccf46c6 100644 --- a/docs/tre-admins/setup-instructions/workflows.md +++ b/docs/tre-admins/setup-instructions/workflows.md @@ -144,6 +144,7 @@ Configure variables used in the deployment workflow: | `WORKSPACE_APP_SERVICE_PLAN_SKU` | Optional. The SKU used for AppService plan used in E2E tests. Default value is `P1v2`. | | `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). | ### Deploy the TRE using the workflow diff --git a/docs/tre-admins/start-stop.md b/docs/tre-admins/start-stop.md index 2d60b46e60..3d565575fb 100644 --- a/docs/tre-admins/start-stop.md +++ b/docs/tre-admins/start-stop.md @@ -161,7 +161,9 @@ foreach ($Group in $ResourceGroups) { # Find the firewall's public IP and virtual network $pip = Get-AzPublicIpAddress -ResourceGroupName $Group.ResourceGroupName -Name "pip-fw-$azureTreId" $vnet = Get-AzVirtualNetwork -ResourceGroupName $Group.ResourceGroupName -Name "vnet-$azureTreId" - $Firewall.Allocate($vnet, $pip) + # Find the firewall's public management IP - note this will only be present for a firewall with a Basic SKU + $mgmtPip = Get-AzPublicIpAddress -ResourceGroupName "rg-$azureTreId" -Name "pip-fw-management-$azureTreId" -ErrorAction SilentlyContinue + $Firewall.Allocate($vnet, $pip, $mgmtPip) Write-Output "Allocating Firewall '$($Firewall.Name)' with public IP '$($pip.Name)'" Set-AzFirewall -AzureFirewall $Firewall } diff --git a/resource_processor/_version.py b/resource_processor/_version.py index de77196f44..3e2f46a3a3 100644 --- a/resource_processor/_version.py +++ b/resource_processor/_version.py @@ -1 +1 @@ -__version__ = "0.8.6" +__version__ = "0.9.0" diff --git a/resource_processor/shared/config.py b/resource_processor/shared/config.py index e937d24552..8da0ee2ab0 100644 --- a/resource_processor/shared/config.py +++ b/resource_processor/shared/config.py @@ -24,6 +24,7 @@ def get_config() -> dict: config["azure_environment"] = os.environ.get("AZURE_ENVIRONMENT", "AzureCloud") config["aad_authority_url"] = os.environ.get("AAD_AUTHORITY_URL", "https://login.microsoftonline.com") config["microsoft_graph_fqdn"] = os.environ.get("MICROSOFT_GRAPH_FQDN", "graph.microsoft.com") + config["firewall_sku"] = os.environ.get("FIREWALL_SKU", "") try: config["number_processes_int"] = int(config["number_processes"]) diff --git a/templates/shared_services/firewall/parameters.json b/templates/shared_services/firewall/parameters.json index 0923966196..7883f0aa13 100755 --- a/templates/shared_services/firewall/parameters.json +++ b/templates/shared_services/firewall/parameters.json @@ -47,9 +47,9 @@ } }, { - "name": "sku_tier", + "name": "firewall_sku", "source": { - "env": "SKU_TIER" + "env": "FIREWALL_SKU" } }, { diff --git a/templates/shared_services/firewall/porter.yaml b/templates/shared_services/firewall/porter.yaml index fa65582077..d84fcb2cd1 100644 --- a/templates/shared_services/firewall/porter.yaml +++ b/templates/shared_services/firewall/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-shared-service-firewall -version: 1.1.7 +version: 1.2.0 description: "An Azure TRE Firewall shared service" dockerfile: Dockerfile.tmpl registry: azuretre @@ -45,7 +45,7 @@ parameters: type: string default: "W10=" # b64 for [] description: "Network rule collection array" - - name: sku_tier + - name: firewall_sku type: string default: Standard description: The firewall and its policy SKU tier @@ -67,7 +67,7 @@ install: tre_resource_id: ${ bundle.parameters.id } api_driven_rule_collections_b64: ${ bundle.parameters.rule_collections } api_driven_network_rule_collections_b64: ${ bundle.parameters.network_rule_collections } - sku_tier: ${ bundle.parameters.sku_tier } + firewall_sku: ${ bundle.parameters.firewall_sku } microsoft_graph_fqdn: ${ bundle.parameters.microsoft_graph_fqdn } backendConfig: resource_group_name: ${ bundle.parameters.tfstate_resource_group_name } @@ -83,7 +83,7 @@ upgrade: tre_resource_id: ${ bundle.parameters.id } api_driven_rule_collections_b64: ${ bundle.parameters.rule_collections } api_driven_network_rule_collections_b64: ${ bundle.parameters.network_rule_collections } - sku_tier: ${ bundle.parameters.sku_tier } + firewall_sku: ${ bundle.parameters.firewall_sku } microsoft_graph_fqdn: ${ bundle.parameters.microsoft_graph_fqdn } backendConfig: resource_group_name: ${ bundle.parameters.tfstate_resource_group_name } @@ -99,7 +99,7 @@ uninstall: tre_resource_id: ${ bundle.parameters.id } api_driven_rule_collections_b64: ${ bundle.parameters.rule_collections } api_driven_network_rule_collections_b64: ${ bundle.parameters.network_rule_collections } - sku_tier: ${ bundle.parameters.sku_tier } + firewall_sku: ${ bundle.parameters.firewall_sku } microsoft_graph_fqdn: ${ bundle.parameters.microsoft_graph_fqdn } backendConfig: resource_group_name: ${ bundle.parameters.tfstate_resource_group_name } diff --git a/templates/shared_services/firewall/terraform/firewall.tf b/templates/shared_services/firewall/terraform/firewall.tf index fcaefaafdb..ae94aecff0 100644 --- a/templates/shared_services/firewall/terraform/firewall.tf +++ b/templates/shared_services/firewall/terraform/firewall.tf @@ -15,7 +15,7 @@ moved { } resource "azurerm_public_ip" "fwmanagement" { - count = var.sku_tier == "Basic" ? 1 : 0 + count = local.effective_firewall_sku == "Basic" ? 1 : 0 name = "pip-fw-management-${var.tre_id}" resource_group_name = local.core_resource_group_name location = data.azurerm_resource_group.rg.location @@ -31,7 +31,7 @@ resource "azurerm_firewall" "fw" { name = local.firewall_name resource_group_name = local.core_resource_group_name location = data.azurerm_resource_group.rg.location - sku_tier = var.sku_tier + sku_tier = local.effective_firewall_sku sku_name = "AZFW_VNet" firewall_policy_id = azurerm_firewall_policy.root.id tags = local.tre_shared_service_tags @@ -42,7 +42,7 @@ resource "azurerm_firewall" "fw" { } dynamic "management_ip_configuration" { - for_each = var.sku_tier == "Basic" ? [1] : [] + for_each = local.effective_firewall_sku == "Basic" ? [1] : [] content { name = "mgmtconfig" subnet_id = data.azurerm_subnet.firewall_management.id @@ -80,7 +80,7 @@ resource "azurerm_firewall_policy" "root" { name = local.firewall_policy_name resource_group_name = local.core_resource_group_name location = data.azurerm_resource_group.rg.location - sku = var.sku_tier + sku = local.effective_firewall_sku tags = local.tre_shared_service_tags lifecycle { ignore_changes = [tags] } diff --git a/templates/shared_services/firewall/terraform/locals.tf b/templates/shared_services/firewall/terraform/locals.tf index 3eb2a41c33..83762737da 100644 --- a/templates/shared_services/firewall/terraform/locals.tf +++ b/templates/shared_services/firewall/terraform/locals.tf @@ -15,4 +15,7 @@ locals { api_driven_network_rule_collection = jsondecode(base64decode(var.api_driven_network_rule_collections_b64)) firewall_policy_name = "fw-policy-${var.tre_id}" + + default_firewall_sku = "Standard" + effective_firewall_sku = coalesce(var.firewall_sku, local.default_firewall_sku) } diff --git a/templates/shared_services/firewall/terraform/variables.tf b/templates/shared_services/firewall/terraform/variables.tf index 974ac891e6..a1017e157f 100644 --- a/templates/shared_services/firewall/terraform/variables.tf +++ b/templates/shared_services/firewall/terraform/variables.tf @@ -23,7 +23,7 @@ variable "api_driven_network_rule_collections_b64" { default = "W10=" #b64 for [] } -variable "sku_tier" { +variable "firewall_sku" { type = string - default = "Standard" + default = "" }