diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ea8093846..ee95c8c7ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ In future versions, public access will be fully removed, and the ACR will become ENHANCEMENTS: * Add ability to pass values to install stage on pipleine ([#4451](https://github.com/microsoft/AzureTRE/pull/4451)) * Format the error message in the Operations panel for enhanced readability ([#4493](https://github.com/microsoft/AzureTRE/issues/4493)) +* Modify the AML workspace service so it does not use local authentication keys for storage access ([#4341](https://github.com/microsoft/AzureTRE/issues/4341)) * Enhanced the logout message to emphasize session security. ([#4410](https://github.com/AzureTRE/AzureTRE/issues/4410)) * Added ability to assign VMs to other users at creation time ([#1179](https://github.com/microsoft/AzureTRE/issues/1179)) * Add shutdown schedule to Windows VMs ([#4211](https://github.com/microsoft/AzureTRE/pull/4211)) @@ -29,6 +30,7 @@ ENHANCEMENTS: * Allow UI_SITE_NAME and UI_FOOTER_TEXT to be dynamically calculated passed in deploy_tre_reusable.yaml ([#4575](https://github.com/microsoft/AzureTRE/pull/4575)) * Enable diagnostic settings for Databricks workspaces ([#4576](https://github.com/microsoft/AzureTRE/pull/4576)) + BUG FIXES: * Letsencrypt.yml fails with "Invalid reference in variable validation" ([#4506](https://github.com/microsoft/AzureTRE/4506)) * Intermittent management storage account access failure during core deployment ([#4505](https://github.com/microsoft/AzureTRE/4505)) diff --git a/templates/workspace_services/azureml/porter.yaml b/templates/workspace_services/azureml/porter.yaml index 2bbaa5d3b2..0cf012d630 100644 --- a/templates/workspace_services/azureml/porter.yaml +++ b/templates/workspace_services/azureml/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-azureml -version: 0.9.3 +version: 0.10.0 description: "An Azure TRE service for Azure Machine Learning" registry: azuretre dockerfile: Dockerfile.tmpl @@ -69,6 +69,8 @@ parameters: - name: key_store_id type: string default: "" + - name: log_analytics_workspace_name + type: string outputs: - name: azureml_workspace_name @@ -147,6 +149,7 @@ install: azure_environment: ${ bundle.parameters.azure_environment } enable_cmk_encryption: ${ bundle.parameters.enable_cmk_encryption } key_store_id: ${ bundle.parameters.key_store_id } + log_analytics_workspace_name: ${ bundle.parameters.log_analytics_workspace_name } backendConfig: use_azuread_auth: "true" use_oidc: "true" @@ -185,6 +188,7 @@ upgrade: azure_environment: ${ bundle.parameters.azure_environment } enable_cmk_encryption: ${ bundle.parameters.enable_cmk_encryption } key_store_id: ${ bundle.parameters.key_store_id } + log_analytics_workspace_name: ${ bundle.parameters.log_analytics_workspace_name } backendConfig: use_azuread_auth: "true" use_oidc: "true" @@ -223,6 +227,7 @@ uninstall: azure_environment: ${ bundle.parameters.azure_environment } enable_cmk_encryption: ${ bundle.parameters.enable_cmk_encryption } key_store_id: ${ bundle.parameters.key_store_id } + log_analytics_workspace_name: ${ bundle.parameters.log_analytics_workspace_name } backendConfig: use_azuread_auth: "true" use_oidc: "true" diff --git a/templates/workspace_services/azureml/template_schema.json b/templates/workspace_services/azureml/template_schema.json index 8a36a3761c..581b8993df 100644 --- a/templates/workspace_services/azureml/template_schema.json +++ b/templates/workspace_services/azureml/template_schema.json @@ -39,11 +39,19 @@ "type": "string", "title": "Address space", "description": "The address space for use by AML subnets" + }, + "log_analytics_workspace_name": { + "$id": "#/properties/log_analytics_workspace_name", + "type": "string", + "title": "Log Analytics Workspace Name" } }, "uiSchema": { "address_space": { "classNames": "tre-hidden" + }, + "log_analytics_workspace_name": { + "classNames": "tre-hidden" } }, "pipeline": { @@ -53,11 +61,17 @@ "stepTitle": "Upgrade to ensure aware of address space", "resourceType": "workspace", "resourceAction": "upgrade", - "properties": [ - ] + "properties": [] }, { - "stepId": "main" + "stepId": "main", + "properties": [ + { + "name": "log_analytics_workspace_name", + "type": "string", + "value": "{{ resource.parent.properties.log_analytics_workspace_name }}" + } + ] }, { "stepId": "260421b3-7308-491f-b531-e007cdc0ff46", @@ -128,7 +142,6 @@ "AzureResourceManager", "{{ resource.properties.mcr_tag }}", "AzureFrontDoor.FirstParty" - ], "destination_ports": [ "443" @@ -275,7 +288,6 @@ "AzureResourceManager", "{{ resource.properties.mcr_tag }}", "AzureFrontDoor.FirstParty" - ], "destination_ports": [ "443" diff --git a/templates/workspace_services/azureml/terraform/compute.tf b/templates/workspace_services/azureml/terraform/compute.tf index 7e4f125158..e66e6017ad 100644 --- a/templates/workspace_services/azureml/terraform/compute.tf +++ b/templates/workspace_services/azureml/terraform/compute.tf @@ -25,7 +25,7 @@ resource "azapi_resource" "compute_cluster" { type = "Microsoft.MachineLearningServices/workspaces/computes@2022-10-01" name = "cp-${local.short_service_id}" location = data.azurerm_resource_group.ws.location - parent_id = azurerm_machine_learning_workspace.aml_workspace.id + parent_id = azapi_resource.aml_workspace.output.id tags = local.tre_workspace_service_tags lifecycle { ignore_changes = [tags] } @@ -69,16 +69,9 @@ resource "azapi_resource" "compute_cluster" { } -# This seems to be added automatically -# resource "azurerm_role_assignment" "compute_cluster_acr_pull" { -# scope = azurerm_container_registry.acr.id -# role_definition_name = "AcrPull" -# principal_id = jsondecode(azapi_resource.compute_cluster.output).identity.principalId -# } - resource "azapi_update_resource" "set_image_build_compute" { type = "Microsoft.MachineLearningServices/workspaces@2022-10-01" - name = azurerm_machine_learning_workspace.aml_workspace.name + name = azapi_resource.aml_workspace.output.name parent_id = data.azurerm_resource_group.ws.id body = { @@ -89,7 +82,5 @@ resource "azapi_update_resource" "set_image_build_compute" { depends_on = [ azapi_resource.compute_cluster - #, - #azurerm_role_assignment.compute_cluster_acr_pull ] } diff --git a/templates/workspace_services/azureml/terraform/data.tf b/templates/workspace_services/azureml/terraform/data.tf index d9db7e09b9..aee13899c0 100644 --- a/templates/workspace_services/azureml/terraform/data.tf +++ b/templates/workspace_services/azureml/terraform/data.tf @@ -7,18 +7,13 @@ data "azurerm_virtual_network" "ws" { resource_group_name = data.azurerm_resource_group.ws.name } -resource "azurerm_application_insights" "ai" { - name = "ai-${local.service_resource_name_suffix}" - location = data.azurerm_resource_group.ws.location +data "azurerm_key_vault" "ws" { + name = local.keyvault_name resource_group_name = data.azurerm_resource_group.ws.name - application_type = "web" - tags = local.tre_workspace_service_tags - - lifecycle { ignore_changes = [tags] } } -data "azurerm_key_vault" "ws" { - name = local.keyvault_name +data "azurerm_log_analytics_workspace" "ws" { + name = var.log_analytics_workspace_name resource_group_name = data.azurerm_resource_group.ws.name } @@ -59,3 +54,15 @@ data "azurerm_user_assigned_identity" "ws_encryption_identity" { name = local.encryption_identity_name resource_group_name = data.azurerm_resource_group.ws.name } + +data "azurerm_role_definition" "reader" { + name = "Reader" +} + +data "azurerm_role_definition" "storage_blob_data_contributor" { + name = "Storage Blob Data Contributor" +} + +data "azurerm_role_definition" "storage_file_data_contributor" { + name = "Storage File Data Privileged Contributor" +} diff --git a/templates/workspace_services/azureml/terraform/main.tf b/templates/workspace_services/azureml/terraform/main.tf index da6996243f..8d35407179 100644 --- a/templates/workspace_services/azureml/terraform/main.tf +++ b/templates/workspace_services/azureml/terraform/main.tf @@ -1,29 +1,61 @@ -resource "azurerm_machine_learning_workspace" "aml_workspace" { - name = local.workspace_name - resource_group_name = data.azurerm_resource_group.ws.name - location = data.azurerm_resource_group.ws.location - application_insights_id = azurerm_application_insights.ai.id - container_registry_id = azurerm_container_registry.acr.id - friendly_name = var.display_name - description = var.description - high_business_impact = true - key_vault_id = data.azurerm_key_vault.ws.id - public_network_access_enabled = var.is_exposed_externally ? true : false - storage_account_id = azurerm_storage_account.aml.id - tags = local.tre_workspace_service_tags - - identity { - type = "SystemAssigned" +resource "azapi_resource" "aml_workspace" { + type = "Microsoft.MachineLearningServices/workspaces@2024-10-01-preview" + name = local.workspace_name + location = data.azurerm_resource_group.ws.location + parent_id = data.azurerm_resource_group.ws.id + tags = local.tre_workspace_service_tags + + lifecycle { ignore_changes = [tags, body.properties.imageBuildCompute] } + + + dynamic "identity" { + for_each = var.enable_cmk_encryption ? [] : [1] + content { + type = "SystemAssigned" + } + } + + + dynamic "identity" { + for_each = var.enable_cmk_encryption ? [1] : [] + content { + type = "SystemAssigned, UserAssigned" + identity_ids = [data.azurerm_user_assigned_identity.ws_encryption_identity[0].id] + } } - lifecycle { - ignore_changes = [ - tags, - image_build_compute_name, - public_network_access_enabled - ] + body = { + properties = { + applicationInsights = azurerm_application_insights.ai.id + containerRegistry = azurerm_container_registry.acr.id + description = var.description + friendlyName = var.display_name + hbiWorkspace = true + keyVault = data.azurerm_key_vault.ws.id + publicNetworkAccess = var.is_exposed_externally ? "Enabled" : "Disabled" + storageAccount = azurerm_storage_account.aml.id + systemDatastoresAuthMode = "identity" + + workspaceHubConfig = { + additionalWorkspaceStorageAccounts = [] + } + + + encryption = { + status = var.enable_cmk_encryption ? "Enabled" : "Disabled" + identity = { + userAssignedIdentity = var.enable_cmk_encryption ? data.azurerm_user_assigned_identity.ws_encryption_identity[0].id : null + } + keyVaultProperties = { + keyIdentifier = var.enable_cmk_encryption ? data.azurerm_key_vault_key.ws_encryption_key[0].versionless_id : null + keyVaultArmId = var.enable_cmk_encryption ? var.key_store_id : null + identityClientId = var.enable_cmk_encryption ? data.azurerm_user_assigned_identity.ws_encryption_identity[0].client_id : null + } + } + } } + response_export_values = ["id", "name", "identity.principalId"] } resource "azurerm_private_endpoint" "mlpe" { @@ -42,7 +74,7 @@ resource "azurerm_private_endpoint" "mlpe" { private_service_connection { name = "mlpesc-${local.service_resource_name_suffix}" - private_connection_resource_id = azurerm_machine_learning_workspace.aml_workspace.id + private_connection_resource_id = azapi_resource.aml_workspace.output.id is_manual_connection = false subresource_names = ["amlworkspace"] } @@ -53,3 +85,14 @@ resource "azurerm_private_endpoint" "mlpe" { ] } + +resource "azurerm_application_insights" "ai" { + name = "ai-${local.service_resource_name_suffix}" + location = data.azurerm_resource_group.ws.location + resource_group_name = data.azurerm_resource_group.ws.name + application_type = "web" + workspace_id = data.azurerm_log_analytics_workspace.ws.id + tags = local.tre_workspace_service_tags + + lifecycle { ignore_changes = [tags] } +} diff --git a/templates/workspace_services/azureml/terraform/outputs.tf b/templates/workspace_services/azureml/terraform/outputs.tf index 6f75c6547d..f889ab5f17 100644 --- a/templates/workspace_services/azureml/terraform/outputs.tf +++ b/templates/workspace_services/azureml/terraform/outputs.tf @@ -1,5 +1,5 @@ output "azureml_workspace_name" { - value = azurerm_machine_learning_workspace.aml_workspace.name + value = azapi_resource.aml_workspace.output.name } output "azureml_acr_id" { @@ -15,7 +15,7 @@ output "aml_fqdn" { } output "connection_uri" { - value = format("%s/?wsid=%s&tid=%s", module.terraform_azurerm_environment_configuration.aml_studio_endpoint, azurerm_machine_learning_workspace.aml_workspace.id, var.arm_tenant_id) + value = format("%s/?wsid=%s&tid=%s", module.terraform_azurerm_environment_configuration.aml_studio_endpoint, azapi_resource.aml_workspace.output.id, var.arm_tenant_id) } output "workspace_address_spaces" { diff --git a/templates/workspace_services/azureml/terraform/roles.tf b/templates/workspace_services/azureml/terraform/roles.tf index af965cae2d..3d8c91d325 100644 --- a/templates/workspace_services/azureml/terraform/roles.tf +++ b/templates/workspace_services/azureml/terraform/roles.tf @@ -22,7 +22,28 @@ data "azurerm_role_definition" "azure_ml_data_scientist" { resource "azurerm_role_assignment" "app_role_members_aml_data_scientist" { for_each = (data.external.app_role_members.result.principals == "") ? [] : toset(split("\n", data.external.app_role_members.result.principals)) - scope = azurerm_machine_learning_workspace.aml_workspace.id + scope = azapi_resource.aml_workspace.output.id role_definition_id = data.azurerm_role_definition.azure_ml_data_scientist.id principal_id = each.value } + +resource "azurerm_role_assignment" "app_role_members_reader" { + for_each = (data.external.app_role_members.result.principals == "") ? [] : toset(split("\n", data.external.app_role_members.result.principals)) + scope = azapi_resource.aml_workspace.output.id + role_definition_id = data.azurerm_role_definition.reader.id + principal_id = each.value +} + +resource "azurerm_role_assignment" "app_role_members_storage_blob_data_contributor" { + for_each = (data.external.app_role_members.result.principals == "") ? [] : toset(split("\n", data.external.app_role_members.result.principals)) + scope = azurerm_storage_account.aml.id + role_definition_id = data.azurerm_role_definition.storage_blob_data_contributor.id + principal_id = each.value +} + +resource "azurerm_role_assignment" "app_role_members_storage_file_data_contributor" { + for_each = (data.external.app_role_members.result.principals == "") ? [] : toset(split("\n", data.external.app_role_members.result.principals)) + scope = azurerm_storage_account.aml.id + role_definition_id = data.azurerm_role_definition.storage_file_data_contributor.id + principal_id = each.value +} diff --git a/templates/workspace_services/azureml/terraform/variables.tf b/templates/workspace_services/azureml/terraform/variables.tf index ef994567b2..66d82055d1 100644 --- a/templates/workspace_services/azureml/terraform/variables.tf +++ b/templates/workspace_services/azureml/terraform/variables.tf @@ -48,3 +48,6 @@ variable "enable_cmk_encryption" { variable "key_store_id" { type = string } +variable "log_analytics_workspace_name" { + type = string +} diff --git a/templates/workspaces/base/porter.yaml b/templates/workspaces/base/porter.yaml index 4c123b232f..18abbb8b26 100644 --- a/templates/workspaces/base/porter.yaml +++ b/templates/workspaces/base/porter.yaml @@ -181,6 +181,11 @@ outputs: applyTo: - install - upgrade + - name: log_analytics_workspace_name + type: string + applyTo: + - install + - upgrade - name: workspace_owners_group_id type: string applyTo: @@ -254,6 +259,7 @@ install: - name: client_id - name: scope_id - name: sp_id + - name: log_analytics_workspace_name - name: workspace_owners_group_id - name: workspace_researchers_group_id - name: workspace_airlock_managers_group_id @@ -308,6 +314,7 @@ upgrade: - name: client_id - name: scope_id - name: sp_id + - name: log_analytics_workspace_name - name: workspace_owners_group_id - name: workspace_researchers_group_id - name: workspace_airlock_managers_group_id diff --git a/templates/workspaces/base/terraform/outputs.tf b/templates/workspaces/base/terraform/outputs.tf index 933c033be8..70986fcd06 100644 --- a/templates/workspaces/base/terraform/outputs.tf +++ b/templates/workspaces/base/terraform/outputs.tf @@ -29,6 +29,10 @@ output "scope_id" { value = var.register_aad_application ? module.aad[0].scope_id : var.scope_id } +output "log_analytics_workspace_name" { + value = module.azure_monitor.log_analytics_workspace_name +} + output "workspace_owners_group_id" { value = var.register_aad_application ? module.aad[0].workspace_owners_group_id : "" }