Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/microsoft/AzureTRE into fea…
Browse files Browse the repository at this point in the history
…ture/workspace_user_management
  • Loading branch information
Matthew Fortunka committed Feb 27, 2025
2 parents 2ce6937 + 0ec539a commit 6bf481e
Show file tree
Hide file tree
Showing 28 changed files with 265 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
**BREAKING CHANGES & MIGRATIONS**:

ENHANCEMENTS:
* Deny public access to TRE management storage account, and add private endpoint for TRE core [#4353](https://github.com/microsoft/AzureTRE/issues/4353)

BUG FIXES:

Expand All @@ -24,6 +25,7 @@ ENHANCEMENTS:
* Migrate UI to Vite build engine and update dependencies ([#4368](https://github.com/microsoft/AzureTRE/pull/4368))
* Add Windows image field to the Admin VM template ([#4274](https://github.com/microsoft/AzureTRE/pull/4274))
* Update TLS to the latest version for web apps / function apps ([#4351](https://github.com/microsoft/AzureTRE/issues/4351))
* Set `stairlockp` Airlock Processor storage account firewall to "Enabled from selected virtual networks and IP addresses" ([#4386](https://github.com/microsoft/AzureTRE/issues/4386))

BUG FIXES:
* Fix upgrade when porter install has failed ([#4338](https://github.com/microsoft/AzureTRE/pull/4338))
Expand Down
6 changes: 6 additions & 0 deletions core/terraform/airlock/airlock_processor.tf
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,14 @@ resource "azurerm_storage_account" "sa_airlock_processor_func_app" {
cross_tenant_replication_enabled = false
local_user_enabled = false
shared_access_key_enabled = false
public_network_access_enabled = true
tags = var.tre_core_tags

network_rules {
default_action = var.enable_local_debugging ? "Allow" : "Deny"
bypass = ["AzureServices"]
}

dynamic "identity" {
for_each = var.enable_cmk_encryption ? [1] : []
content {
Expand Down
2 changes: 1 addition & 1 deletion core/terraform/data.tf
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ data "azurerm_monitor_diagnostic_categories" "sb" {
depends_on = [
azurerm_servicebus_namespace.sb
]
}
}
1 change: 1 addition & 0 deletions core/terraform/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ module "resource_processor_vmss_porter" {
acr_id = data.azurerm_container_registry.mgmt_acr.id
app_insights_connection_string = module.azure_monitor.app_insights_connection_string
resource_processor_subnet_id = module.network.resource_processor_subnet_id
blob_core_dns_zone_id = module.network.blob_core_dns_zone_id
docker_registry_server = local.docker_registry_server
resource_processor_vmss_porter_image_repository = var.resource_processor_vmss_porter_image_repository
service_bus_namespace_id = azurerm_servicebus_namespace.sb.id
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/migrate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ set -o pipefail
set -o nounset
# set -o xtrace

# shellcheck disable=SC1091
source ../../devops/scripts/mgmtstorage_enable_public_access.sh

get_resource_id() {
local json_data="$1"
local resource_addr="$2"
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/outputs.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#!/bin/bash
set -e

# shellcheck disable=SC1091
source ../../devops/scripts/mgmtstorage_enable_public_access.sh

if [ ! -f ../tre_output.json ] || [ ! -s ../tre_output.json ]; then
# Connect to the remote backend of Terraform
export TF_LOG=""
Expand Down
21 changes: 21 additions & 0 deletions core/terraform/resource_processor/vmss_porter/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,27 @@ resource "azurerm_role_assignment" "vmss_kv_encryption_key_user" {
principal_id = azurerm_user_assigned_identity.vmss_msi.principal_id
}

resource "azurerm_private_endpoint" "mgmtblobpe" {
name = "pe-mgmt-blob-${var.tre_id}"
location = var.location
resource_group_name = var.mgmt_resource_group_name
subnet_id = var.resource_processor_subnet_id
tags = local.tre_core_tags
lifecycle { ignore_changes = [tags] }

private_dns_zone_group {
name = "private-dns-zone-group-blobcore"
private_dns_zone_ids = [var.blob_core_dns_zone_id]
}

private_service_connection {
name = "psc-mgmt-${var.tre_id}"
private_connection_resource_id = data.azurerm_storage_account.mgmt_storage.id
is_manual_connection = false
subresource_names = ["Blob"]
}
}

module "terraform_azurerm_environment_configuration" {
source = "git::https://github.com/microsoft/terraform-azurerm-environment-configuration.git?ref=0.2.0"
arm_environment = var.arm_environment
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/resource_processor/vmss_porter/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ variable "core_api_client_id" {
variable "resource_processor_subnet_id" {
type = string
}
variable "blob_core_dns_zone_id" {
type = string
}
variable "resource_processor_vmss_porter_image_repository" {
type = string
}
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/update_tags.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ set -o pipefail
set -o nounset
# set -o xtrace

# shellcheck disable=SC1091
source ../../devops/scripts/mgmtstorage_enable_public_access.sh

script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

core_rg_rid=$(terraform show -json | jq -r '.values.root_module.resources[] | select(.address=="azurerm_resource_group.core") | .values.id')
Expand Down
130 changes: 130 additions & 0 deletions devops/scripts/mgmtstorage_enable_public_access.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
#!/bin/bash

#
# Add an exception to the TRE management storage account by making it public for deployment, and remove it on script exit.
#
# Note: Ensure you "source" this script, or else the EXIT trap won't fire at the right time.
#

function mgmtstorage_enable_public_access() {
local RESOURCE_GROUP
RESOURCE_GROUP=$(get_resource_group_name)

local SA_NAME
SA_NAME=$(get_storage_account_name)

# Check that the storage account exists before making changes
if ! does_storage_account_exist "$SA_NAME"; then
echo -e "Error: Storage account $SA_NAME does not exist.\n" >&2
exit 1
fi

# Pre-check: if public access is already enabled, no need to update.
if is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Storage account $SA_NAME is already publicly accessible\n"
return
fi

echo -e "\nEnabling public access on storage account $SA_NAME"

# Enable public network access with explicit default action allow
az storage account update --resource-group "$RESOURCE_GROUP" --name "$SA_NAME" --public-network-access Enabled --default-action Allow --output none

for ATTEMPT in {1..10}; do
if is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Storage account $SA_NAME is now publicly accessible\n"
return
fi

echo " Unable to confirm public access on storage account $SA_NAME after $ATTEMPT/10. Waiting for update to take effect..."
sleep 10
done

echo -e "Error: Could not enable public access for $SA_NAME after 10 attempts.\n"
exit 1
}

function mgmtstorage_disable_public_access() {
local RESOURCE_GROUP
RESOURCE_GROUP=$(get_resource_group_name)

local SA_NAME
SA_NAME=$(get_storage_account_name)

# Check that the storage account exists before making changes
if ! does_storage_account_exist "$SA_NAME"; then
echo -e "Error: Storage account $SA_NAME does not exist.\n" >&2
exit 1
fi

# Pre-check: if public access is already disabled, no need to update.
if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Storage account $SA_NAME is already not publicly accessible\n"
return
fi

echo -e "\nDisabling public access on storage account $SA_NAME"

# Disable public network access with explicit default action deny
az storage account update --resource-group "$RESOURCE_GROUP" --name "$SA_NAME" --public-network-access Disabled --default-action Deny --output none

for ATTEMPT in {1..10}; do
if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Public access has been disabled successfully\n"
return
fi

echo " Unable to confirm public access is disabled on storage account $SA_NAME after $ATTEMPT/10. Waiting for update to take effect..."
sleep 10
done

echo -e "Error: Could not disable public access for $SA_NAME after 10 attempts.\n"
exit 1
}

function get_resource_group_name() {
if [[ -z "${TF_VAR_mgmt_resource_group_name:-}" ]]; then
echo -e "Error: TF_VAR_mgmt_resource_group_name is not set\nExiting...\n" >&2
exit 1
fi
echo "$TF_VAR_mgmt_resource_group_name"
}

function get_storage_account_name() {
if [[ -z "${TF_VAR_mgmt_storage_account_name:-}" ]]; then
echo -e "Error: TF_VAR_mgmt_storage_account_name is not set\nExiting...\n" >&2
exit 1
fi
echo "$TF_VAR_mgmt_storage_account_name"
}

function does_storage_account_exist() {
[[ -n "$(az storage account show --name "$1" --query "id" --output tsv)" ]]
}

function is_public_access_enabled() {
local RESOURCE_GROUP="$1"
local SA_NAME="$2"

# Try listing containers
local containers
if ! containers=$(az storage container list --account-name "$SA_NAME" --auth-mode login --query "[].name" --output tsv); then
return 1
fi

# For each container found, check blob listing
for container in $containers; do
if ! az storage blob list --container-name "$container" --account-name "$SA_NAME" --auth-mode login --output none; then
return 1
fi
done

# If container list succeeded (even if empty) and blob list (if any) succeeded, public access is enabled
return 0
}

# Setup the trap to disable public access on exit
trap mgmtstorage_disable_public_access EXIT

# Enable public access for deployment
mgmtstorage_enable_public_access "$@"
3 changes: 3 additions & 0 deletions devops/scripts/terraform_wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ if [[ -z ${tf_logfile+x} ]]; then
echo -e "No logfile provided, using ${tf_logfile}\n"
fi

# shellcheck disable=SC1091
source "$(dirname "$0")/mgmtstorage_enable_public_access.sh"

terraform init -input=false -backend=true -reconfigure \
-backend-config="resource_group_name=${mgmt_resource_group_name}" \
-backend-config="storage_account_name=${mgmt_storage_account_name}" \
Expand Down
3 changes: 3 additions & 0 deletions devops/scripts/upgrade.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ else
KEY="${TRE_ID?}_${PARENT_DIR}"
fi

# shellcheck disable=SC1091
source "$(dirname "$0")/mgmtstorage_enable_public_access.sh"

# Run terraform init with upgrade and reconfigure options
terraform -chdir="$DIR/terraform" init -upgrade -reconfigure -input=false -backend=true \
-backend-config="resource_group_name=${TF_VAR_mgmt_resource_group_name}" \
Expand Down
3 changes: 3 additions & 0 deletions devops/terraform/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ else
az storage account show --resource-group "$TF_VAR_mgmt_resource_group_name" --name "$TF_VAR_mgmt_storage_account_name" --output table
fi

# shellcheck disable=SC1091
source ../scripts/mgmtstorage_enable_public_access.sh

# Grant user blob data contributor permissions
echo -e "\n\e[34m»»» 🔑 \e[96mGranting Storage Blob Data Contributor role to the current user\e[0m..."
if [ -n "${ARM_CLIENT_ID:-}" ]; then
Expand Down
3 changes: 3 additions & 0 deletions devops/terraform/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ set -o pipefail
set -o nounset
# set -o xtrace

# shellcheck disable=SC1091
source ../scripts/mgmtstorage_enable_public_access.sh

PLAN_FILE="devops.tfplan"

terraform init -input=false -backend=true -reconfigure
Expand Down
12 changes: 11 additions & 1 deletion devops/terraform/destroy.sh
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
terraform destroy -auto-approve
#!/bin/bash

set -o errexit
set -o pipefail
set -o nounset
# set -o xtrace

# shellcheck disable=SC1091
source ../scripts/mgmtstorage_enable_public_access.sh

terraform destroy -auto-approve
2 changes: 1 addition & 1 deletion devops/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.5.5"
__version__ = "0.5.6"
3 changes: 2 additions & 1 deletion e2e_tests/test_performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(verify) ->
"properties": {
"display_name": "Perf test VM",
"description": "",
"os_image": "Ubuntu 22.04 LTS"
"os_image": "Ubuntu 22.04 LTS",
"admin_username": "researcher"
}
}

Expand Down
3 changes: 2 additions & 1 deletion e2e_tests/test_workspace_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ async def test_create_guacamole_service_into_base_workspace(setup_test_workspace
"properties": {
"display_name": "My VM",
"description": "Will be using this VM for my research",
"os_image": "Windows 10"
"os_image": "Windows 10",
"admin_username": "researcher"
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ parameters:
- name: os_image
type: string
default: "Ubuntu 22.04 LTS"
- name: admin_username
type: string
default: ""
- name: vm_size
type: string
default: "2 CPU | 8GB RAM"
Expand Down Expand Up @@ -166,6 +169,7 @@ install:
parent_service_id: ${ bundle.parameters.parent_service_id }
tre_resource_id: ${ bundle.parameters.id }
image: ${ bundle.parameters.os_image }
admin_username: ${ bundle.parameters.admin_username }
vm_size: ${ bundle.parameters.vm_size }
shared_storage_access: ${ bundle.parameters.shared_storage_access }
shared_storage_name: ${ bundle.parameters.shared_storage_name }
Expand Down Expand Up @@ -201,6 +205,7 @@ upgrade:
parent_service_id: ${ bundle.parameters.parent_service_id }
tre_resource_id: ${ bundle.parameters.id }
image: ${ bundle.parameters.os_image }
admin_username: ${ bundle.parameters.admin_username }
vm_size: ${ bundle.parameters.vm_size }
shared_storage_access: ${ bundle.parameters.shared_storage_access }
shared_storage_name: ${ bundle.parameters.shared_storage_name }
Expand Down Expand Up @@ -248,6 +253,7 @@ uninstall:
parent_service_id: ${ bundle.parameters.parent_service_id }
tre_resource_id: ${ bundle.parameters.id }
image: ${ bundle.parameters.os_image }
admin_username: ${ bundle.parameters.admin_username }
vm_size: ${ bundle.parameters.vm_size }
shared_storage_access: ${ bundle.parameters.shared_storage_access }
shared_storage_name: ${ bundle.parameters.shared_storage_name }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
"Ubuntu 22.04 LTS"
]
},
"admin_username": {
"type": "string",
"title": "Admin username",
"description": "Overide automatic admin username generation.",
"default": ""
},
"vm_size": {
"$id": "#/properties/vm_size",
"type": "string",
Expand Down Expand Up @@ -126,5 +132,10 @@
]
}
}
]
],
"uiSchema": {
"admin_username": {
"classNames": "tre-hidden"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,6 @@ data "azurerm_storage_account" "stg" {
}

data "azuread_user" "user" {
count = var.admin_username == "" ? 1 : 0
object_id = var.owner_id
}
Loading

0 comments on commit 6bf481e

Please sign in to comment.