From 5610c6ceaefaebe58070c7578544930efbf22108 Mon Sep 17 00:00:00 2001 From: jonnyry Date: Mon, 28 Oct 2024 11:16:16 +0000 Subject: [PATCH 1/2] Key Vaults should use RBAC instead of access policies for access control #4000 --- CHANGELOG.md | 2 +- core/terraform/appgateway/certificate.tf | 16 ++---- core/terraform/cosmos_mongo.tf | 2 +- core/terraform/keyvault.tf | 50 ++++++++----------- core/terraform/main.tf | 4 +- core/terraform/modules_move_definitions.tf | 10 ---- .../resource_processor/vmss_porter/main.tf | 11 ++-- core/version.txt | 2 +- templates/shared_services/certs/porter.yaml | 2 +- .../certs/terraform/appgateway.tf | 2 +- .../certs/terraform/certificate.tf | 11 ++-- templates/shared_services/gitea/porter.yaml | 2 +- .../gitea/terraform/gitea-webapp.tf | 12 ++--- .../shared_services/gitea/terraform/mysql.tf | 2 +- .../sonatype-nexus-vm/porter.yaml | 2 +- .../sonatype-nexus-vm/terraform/vm.tf | 12 ++--- .../workspace_services/gitea/porter.yaml | 2 +- .../gitea/terraform/gitea-webapp.tf | 12 ++--- .../gitea/terraform/mysql.tf | 2 +- .../workspace_services/guacamole/porter.yaml | 2 +- .../guacamole/terraform/web_app.tf | 12 ++--- .../workspace_services/mlflow/porter.yaml | 2 +- .../mlflow/terraform/web_app.tf | 12 ++--- .../workspace_services/ohdsi/porter.yaml | 2 +- .../ohdsi/terraform/ohdsi_web_api.tf | 12 ++--- templates/workspaces/base/porter.yaml | 2 +- .../workspaces/base/terraform/keyvault.tf | 35 +++++++------ .../workspaces/base/terraform/workspace.tf | 4 +- 28 files changed, 97 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f972e031..df05cf13b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ FEATURES: ENHANCEMENTS: - +* Key Vaults should use RBAC instead of access policies for access control ([#4000](https://github.com/microsoft/AzureTRE/issues/4000)) BUG FIXES: COMPONENTS: diff --git a/core/terraform/appgateway/certificate.tf b/core/terraform/appgateway/certificate.tf index b2b289ee9..81251167c 100644 --- a/core/terraform/appgateway/certificate.tf +++ b/core/terraform/appgateway/certificate.tf @@ -1,15 +1,7 @@ -resource "azurerm_key_vault_access_policy" "app_gw_managed_identity" { - key_vault_id = var.keyvault_id - tenant_id = azurerm_user_assigned_identity.agw_id.tenant_id - object_id = azurerm_user_assigned_identity.agw_id.principal_id - - key_permissions = [ - "Get", - ] - - secret_permissions = [ - "Get", - ] +resource "azurerm_role_assignment" "keyvault_appgw_role" { + scope = var.keyvault_id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.agw_id.principal_id // id-agw- } resource "azurerm_key_vault_certificate" "tlscert" { diff --git a/core/terraform/cosmos_mongo.tf b/core/terraform/cosmos_mongo.tf index fdb90fbf1..6b4f386d0 100644 --- a/core/terraform/cosmos_mongo.tf +++ b/core/terraform/cosmos_mongo.tf @@ -97,7 +97,7 @@ resource "azurerm_key_vault_secret" "cosmos_mongo_connstr" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_core_tags depends_on = [ - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] lifecycle { ignore_changes = [tags] } diff --git a/core/terraform/keyvault.tf b/core/terraform/keyvault.tf index 659017dfb..60433f1d5 100644 --- a/core/terraform/keyvault.tf +++ b/core/terraform/keyvault.tf @@ -1,34 +1,26 @@ resource "azurerm_key_vault" "kv" { - name = "kv-${var.tre_id}" - tenant_id = data.azurerm_client_config.current.tenant_id - location = azurerm_resource_group.core.location - resource_group_name = azurerm_resource_group.core.name - sku_name = "standard" - purge_protection_enabled = var.kv_purge_protection_enabled - tags = local.tre_core_tags + name = "kv-${var.tre_id}" + tenant_id = data.azurerm_client_config.current.tenant_id + location = azurerm_resource_group.core.location + resource_group_name = azurerm_resource_group.core.name + sku_name = "standard" + enable_rbac_authorization = true + purge_protection_enabled = var.kv_purge_protection_enabled + tags = local.tre_core_tags lifecycle { ignore_changes = [access_policy, tags] } } -resource "azurerm_key_vault_access_policy" "deployer" { - key_vault_id = azurerm_key_vault.kv.id - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id - - key_permissions = ["Get", "List", "Update", "Create", "Import", "Delete", "Recover"] - secret_permissions = ["Get", "List", "Set", "Delete", "Purge", "Recover"] - certificate_permissions = ["Get", "List", "Update", "Create", "Import", "Delete", "Purge", "Recover"] - storage_permissions = ["Get", "List", "Update", "Delete"] +resource "azurerm_role_assignment" "keyvault_deployer_role" { + scope = azurerm_key_vault.kv.id + role_definition_name = "Key Vault Administrator" + principal_id = data.azurerm_client_config.current.object_id // deployer - either CICD service principal or local user } -resource "azurerm_key_vault_access_policy" "managed_identity" { - key_vault_id = azurerm_key_vault.kv.id - tenant_id = azurerm_user_assigned_identity.id.tenant_id - object_id = azurerm_user_assigned_identity.id.principal_id - - key_permissions = ["Get", "List", ] - secret_permissions = ["Get", "List", ] - certificate_permissions = ["Get", "List", ] +resource "azurerm_role_assignment" "keyvault_apiidentity_role" { + scope = azurerm_key_vault.kv.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.id.principal_id // id-api- } data "azurerm_private_dns_zone" "vaultcore" { @@ -68,7 +60,7 @@ resource "azurerm_key_vault_secret" "api_client_id" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_core_tags depends_on = [ - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] lifecycle { ignore_changes = [tags] } @@ -80,7 +72,7 @@ resource "azurerm_key_vault_secret" "api_client_secret" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_core_tags depends_on = [ - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] lifecycle { ignore_changes = [tags] } @@ -92,7 +84,7 @@ resource "azurerm_key_vault_secret" "auth_tenant_id" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_core_tags depends_on = [ - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] lifecycle { ignore_changes = [tags] } @@ -104,7 +96,7 @@ resource "azurerm_key_vault_secret" "application_admin_client_id" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_core_tags depends_on = [ - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] lifecycle { ignore_changes = [tags] } @@ -116,7 +108,7 @@ resource "azurerm_key_vault_secret" "application_admin_client_secret" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_core_tags depends_on = [ - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] lifecycle { ignore_changes = [tags] } diff --git a/core/terraform/main.tf b/core/terraform/main.tf index 2c2f2db8a..8cc0d3a7e 100644 --- a/core/terraform/main.tf +++ b/core/terraform/main.tf @@ -103,7 +103,7 @@ module "appgateway" { depends_on = [ module.network, azurerm_key_vault.kv, - azurerm_key_vault_access_policy.deployer, + azurerm_role_assignment.keyvault_deployer_role, azurerm_private_endpoint.api_private_endpoint ] } @@ -174,7 +174,7 @@ module "resource_processor_vmss_porter" { module.network, module.azure_monitor, azurerm_key_vault.kv, - azurerm_key_vault_access_policy.deployer + azurerm_role_assignment.keyvault_deployer_role ] } diff --git a/core/terraform/modules_move_definitions.tf b/core/terraform/modules_move_definitions.tf index 3634549e2..e0ffe47e3 100644 --- a/core/terraform/modules_move_definitions.tf +++ b/core/terraform/modules_move_definitions.tf @@ -148,16 +148,6 @@ moved { to = azurerm_key_vault.kv } -moved { - from = module.keyvault.azurerm_key_vault_access_policy.deployer - to = azurerm_key_vault_access_policy.deployer -} - -moved { - from = module.keyvault.azurerm_key_vault_access_policy.managed_identity - to = azurerm_key_vault_access_policy.managed_identity -} - moved { from = module.keyvault.azurerm_private_endpoint.kvpe to = azurerm_private_endpoint.kvpe diff --git a/core/terraform/resource_processor/vmss_porter/main.tf b/core/terraform/resource_processor/vmss_porter/main.tf index 83c0e4cad..08ea8b05c 100644 --- a/core/terraform/resource_processor/vmss_porter/main.tf +++ b/core/terraform/resource_processor/vmss_porter/main.tf @@ -183,13 +183,10 @@ resource "azurerm_role_assignment" "subscription_contributor" { principal_id = azurerm_user_assigned_identity.vmss_msi.principal_id } -resource "azurerm_key_vault_access_policy" "resource_processor" { - key_vault_id = var.key_vault_id - tenant_id = azurerm_user_assigned_identity.vmss_msi.tenant_id - object_id = azurerm_user_assigned_identity.vmss_msi.principal_id - - secret_permissions = ["Get", "List", "Set", "Delete", "Purge", "Recover"] - certificate_permissions = ["Get", "Recover", "Import", "Delete", "Purge"] +resource "azurerm_role_assignment" "keyvault_vmss_role" { + scope = var.key_vault_id + role_definition_name = "Key Vault Administrator" + principal_id = azurerm_user_assigned_identity.vmss_msi.principal_id // id-vmss- } module "terraform_azurerm_environment_configuration" { diff --git a/core/version.txt b/core/version.txt index cdf3f1263..5dae1332b 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.10.8" +__version__ = "0.11.8" diff --git a/templates/shared_services/certs/porter.yaml b/templates/shared_services/certs/porter.yaml index f66f3689f..293b76253 100755 --- a/templates/shared_services/certs/porter.yaml +++ b/templates/shared_services/certs/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-shared-service-certs -version: 0.5.2 +version: 0.6.0 description: "An Azure TRE shared service to generate certificates for a specified internal domain using Letsencrypt" registry: azuretre dockerfile: Dockerfile.tmpl diff --git a/templates/shared_services/certs/terraform/appgateway.tf b/templates/shared_services/certs/terraform/appgateway.tf index 730eddf08..909b3cb78 100644 --- a/templates/shared_services/certs/terraform/appgateway.tf +++ b/templates/shared_services/certs/terraform/appgateway.tf @@ -162,6 +162,6 @@ resource "azurerm_application_gateway" "agw" { } depends_on = [ - azurerm_key_vault_access_policy.app_gw_managed_identity, + azurerm_role_assignment.keyvault_appgwcerts_role, ] } diff --git a/templates/shared_services/certs/terraform/certificate.tf b/templates/shared_services/certs/terraform/certificate.tf index 664bbe61d..0a825c491 100644 --- a/templates/shared_services/certs/terraform/certificate.tf +++ b/templates/shared_services/certs/terraform/certificate.tf @@ -1,10 +1,7 @@ -resource "azurerm_key_vault_access_policy" "app_gw_managed_identity" { - key_vault_id = data.azurerm_key_vault.key_vault.id - tenant_id = azurerm_user_assigned_identity.agw_id.tenant_id - object_id = azurerm_user_assigned_identity.agw_id.principal_id - - key_permissions = ["Get"] - secret_permissions = ["Get"] +resource "azurerm_role_assignment" "keyvault_appgwcerts_role" { + scope = data.azurerm_key_vault.key_vault.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.agw_id.principal_id } resource "azurerm_key_vault_certificate" "tlscert" { diff --git a/templates/shared_services/gitea/porter.yaml b/templates/shared_services/gitea/porter.yaml index 384ba69b4..6110403fb 100644 --- a/templates/shared_services/gitea/porter.yaml +++ b/templates/shared_services/gitea/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-shared-service-gitea -version: 1.0.3 +version: 1.1.0 description: "A Gitea shared service" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/shared_services/gitea/terraform/gitea-webapp.tf b/templates/shared_services/gitea/terraform/gitea-webapp.tf index 0fb591877..26d8a6252 100644 --- a/templates/shared_services/gitea/terraform/gitea-webapp.tf +++ b/templates/shared_services/gitea/terraform/gitea-webapp.tf @@ -141,12 +141,10 @@ resource "azurerm_monitor_diagnostic_setting" "webapp_gitea" { } } -resource "azurerm_key_vault_access_policy" "gitea_policy" { - key_vault_id = data.azurerm_key_vault.keyvault.id - tenant_id = azurerm_user_assigned_identity.gitea_id.tenant_id - object_id = azurerm_user_assigned_identity.gitea_id.principal_id - - secret_permissions = ["Get", "List", ] +resource "azurerm_role_assignment" "keyvault_gitea_role" { + scope = data.azurerm_key_vault.keyvault.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.gitea_id.principal_id } resource "azurerm_key_vault_secret" "gitea_password" { @@ -156,7 +154,7 @@ resource "azurerm_key_vault_secret" "gitea_password" { tags = local.tre_shared_service_tags depends_on = [ - azurerm_key_vault_access_policy.gitea_policy + azurerm_role_assignment.keyvault_gitea_role ] lifecycle { ignore_changes = [tags] } diff --git a/templates/shared_services/gitea/terraform/mysql.tf b/templates/shared_services/gitea/terraform/mysql.tf index 61fe2af16..3a1b9425c 100644 --- a/templates/shared_services/gitea/terraform/mysql.tf +++ b/templates/shared_services/gitea/terraform/mysql.tf @@ -65,7 +65,7 @@ resource "azurerm_key_vault_secret" "db_password" { tags = local.tre_shared_service_tags depends_on = [ - azurerm_key_vault_access_policy.gitea_policy + azurerm_role_assignment.keyvault_gitea_role ] lifecycle { ignore_changes = [tags] } diff --git a/templates/shared_services/sonatype-nexus-vm/porter.yaml b/templates/shared_services/sonatype-nexus-vm/porter.yaml index 79ea23c4b..9f1d87459 100644 --- a/templates/shared_services/sonatype-nexus-vm/porter.yaml +++ b/templates/shared_services/sonatype-nexus-vm/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-shared-service-sonatype-nexus -version: 3.0.1 +version: 3.1.0 description: "A Sonatype Nexus shared service" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/shared_services/sonatype-nexus-vm/terraform/vm.tf b/templates/shared_services/sonatype-nexus-vm/terraform/vm.tf index 79dfa0447..e9633eda5 100644 --- a/templates/shared_services/sonatype-nexus-vm/terraform/vm.tf +++ b/templates/shared_services/sonatype-nexus-vm/terraform/vm.tf @@ -87,12 +87,10 @@ resource "azurerm_user_assigned_identity" "nexus_msi" { lifecycle { ignore_changes = [tags] } } -resource "azurerm_key_vault_access_policy" "nexus_msi" { - key_vault_id = data.azurerm_key_vault.kv.id - tenant_id = azurerm_user_assigned_identity.nexus_msi.tenant_id - object_id = azurerm_user_assigned_identity.nexus_msi.principal_id - - secret_permissions = ["Get", "List"] +resource "azurerm_role_assignment" "keyvault_nexus_role" { + scope = data.azurerm_key_vault.kv.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.nexus_msi.principal_id } resource "azurerm_linux_virtual_machine" "nexus" { @@ -134,7 +132,7 @@ resource "azurerm_linux_virtual_machine" "nexus" { } depends_on = [ - azurerm_key_vault_access_policy.nexus_msi + azurerm_role_assignment.keyvault_nexus_role ] connection { diff --git a/templates/workspace_services/gitea/porter.yaml b/templates/workspace_services/gitea/porter.yaml index a95167042..6292a71c9 100644 --- a/templates/workspace_services/gitea/porter.yaml +++ b/templates/workspace_services/gitea/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-service-gitea -version: 1.0.5 +version: 1.1.0 description: "A Gitea workspace service" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspace_services/gitea/terraform/gitea-webapp.tf b/templates/workspace_services/gitea/terraform/gitea-webapp.tf index c354a0ac8..0e25df6c4 100644 --- a/templates/workspace_services/gitea/terraform/gitea-webapp.tf +++ b/templates/workspace_services/gitea/terraform/gitea-webapp.tf @@ -150,12 +150,10 @@ resource "azurerm_monitor_diagnostic_setting" "gitea" { } } -resource "azurerm_key_vault_access_policy" "gitea_policy" { - key_vault_id = data.azurerm_key_vault.ws.id - tenant_id = azurerm_user_assigned_identity.gitea_id.tenant_id - object_id = azurerm_user_assigned_identity.gitea_id.principal_id - - secret_permissions = ["Get", "List", ] +resource "azurerm_role_assignment" "keyvault_gitea_ws_role" { + scope = data.azurerm_key_vault.ws.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.gitea_id.principal_id } resource "azurerm_key_vault_secret" "gitea_password" { @@ -165,7 +163,7 @@ resource "azurerm_key_vault_secret" "gitea_password" { tags = local.workspace_service_tags depends_on = [ - azurerm_key_vault_access_policy.gitea_policy + azurerm_role_assignment.keyvault_gitea_ws_role ] lifecycle { ignore_changes = [tags] } diff --git a/templates/workspace_services/gitea/terraform/mysql.tf b/templates/workspace_services/gitea/terraform/mysql.tf index 760882731..bd823448c 100644 --- a/templates/workspace_services/gitea/terraform/mysql.tf +++ b/templates/workspace_services/gitea/terraform/mysql.tf @@ -65,7 +65,7 @@ resource "azurerm_key_vault_secret" "db_password" { tags = local.workspace_service_tags depends_on = [ - azurerm_key_vault_access_policy.gitea_policy + azurerm_role_assignment.keyvault_gitea_ws_role ] lifecycle { ignore_changes = [tags] } diff --git a/templates/workspace_services/guacamole/porter.yaml b/templates/workspace_services/guacamole/porter.yaml index 4bd942861..e9ed90001 100644 --- a/templates/workspace_services/guacamole/porter.yaml +++ b/templates/workspace_services/guacamole/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-guacamole -version: 0.10.9 +version: 0.11.0 description: "An Azure TRE service for Guacamole" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspace_services/guacamole/terraform/web_app.tf b/templates/workspace_services/guacamole/terraform/web_app.tf index 53d998cb0..5cd938a25 100644 --- a/templates/workspace_services/guacamole/terraform/web_app.tf +++ b/templates/workspace_services/guacamole/terraform/web_app.tf @@ -91,7 +91,7 @@ resource "azurerm_linux_web_app" "guacamole" { depends_on = [ azurerm_role_assignment.guac_acr_pull, - azurerm_key_vault_access_policy.guacamole_policy + azurerm_role_assignment.keyvault_guacamole_ws_role ] } @@ -143,10 +143,8 @@ resource "azurerm_private_endpoint" "guacamole" { lifecycle { ignore_changes = [tags] } } -resource "azurerm_key_vault_access_policy" "guacamole_policy" { - key_vault_id = data.azurerm_key_vault.ws.id - tenant_id = azurerm_user_assigned_identity.guacamole_id.tenant_id - object_id = azurerm_user_assigned_identity.guacamole_id.principal_id - - secret_permissions = ["Get", "List", ] +resource "azurerm_role_assignment" "keyvault_guacamole_ws_role" { + scope = data.azurerm_key_vault.ws.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.guacamole_id.principal_id } diff --git a/templates/workspace_services/mlflow/porter.yaml b/templates/workspace_services/mlflow/porter.yaml index 75a6800c9..e36eb81ce 100644 --- a/templates/workspace_services/mlflow/porter.yaml +++ b/templates/workspace_services/mlflow/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-mlflow -version: 0.7.9 +version: 0.8.0 description: "An Azure TRE service for MLflow machine learning lifecycle" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspace_services/mlflow/terraform/web_app.tf b/templates/workspace_services/mlflow/terraform/web_app.tf index f9aa1b2fe..b2a88acbc 100644 --- a/templates/workspace_services/mlflow/terraform/web_app.tf +++ b/templates/workspace_services/mlflow/terraform/web_app.tf @@ -82,7 +82,7 @@ resource "azurerm_linux_web_app" "mlflow" { depends_on = [ azurerm_role_assignment.mlflow_acr_pull, - azurerm_key_vault_access_policy.mlflow, + azurerm_role_assignment.keyvault_mlflow_ws_role, ] } @@ -131,12 +131,10 @@ resource "azurerm_private_endpoint" "mlflow" { lifecycle { ignore_changes = [tags] } } -resource "azurerm_key_vault_access_policy" "mlflow" { - key_vault_id = data.azurerm_key_vault.ws.id - tenant_id = azurerm_user_assigned_identity.mlflow.tenant_id - object_id = azurerm_user_assigned_identity.mlflow.principal_id - - secret_permissions = ["Get", "List", ] +resource "azurerm_role_assignment" "keyvault_mlflow_ws_role" { + scope = data.azurerm_key_vault.ws.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.mlflow.principal_id } resource "azurerm_user_assigned_identity" "mlflow" { diff --git a/templates/workspace_services/ohdsi/porter.yaml b/templates/workspace_services/ohdsi/porter.yaml index 34b3d8bc7..cb23f165f 100644 --- a/templates/workspace_services/ohdsi/porter.yaml +++ b/templates/workspace_services/ohdsi/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-service-ohdsi -version: 0.2.5 +version: 0.3.0 description: "An OHDSI workspace service" registry: azuretre dockerfile: Dockerfile.tmpl diff --git a/templates/workspace_services/ohdsi/terraform/ohdsi_web_api.tf b/templates/workspace_services/ohdsi/terraform/ohdsi_web_api.tf index cb0b7a970..a3640d467 100644 --- a/templates/workspace_services/ohdsi/terraform/ohdsi_web_api.tf +++ b/templates/workspace_services/ohdsi/terraform/ohdsi_web_api.tf @@ -16,14 +16,10 @@ resource "azurerm_user_assigned_identity" "ohdsi_webapi_id" { lifecycle { ignore_changes = [tags] } } -resource "azurerm_key_vault_access_policy" "ohdsi_webapi" { - key_vault_id = data.azurerm_key_vault.ws.id - tenant_id = azurerm_user_assigned_identity.ohdsi_webapi_id.tenant_id - object_id = azurerm_user_assigned_identity.ohdsi_webapi_id.principal_id - - secret_permissions = [ - "Get", "List" - ] +resource "azurerm_role_assignment" "keyvault_ohdsi_ws_role" { + scope = data.azurerm_key_vault.ws.id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_user_assigned_identity.ohdsi_webapi_id.principal_id } resource "azurerm_linux_web_app" "ohdsi_webapi" { diff --git a/templates/workspaces/base/porter.yaml b/templates/workspaces/base/porter.yaml index 072d22760..2bbe140c7 100644 --- a/templates/workspaces/base/porter.yaml +++ b/templates/workspaces/base/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-base -version: 1.5.7 +version: 1.6.0 description: "A base Azure TRE workspace" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspaces/base/terraform/keyvault.tf b/templates/workspaces/base/terraform/keyvault.tf index acfe387cd..73126a9a4 100644 --- a/templates/workspaces/base/terraform/keyvault.tf +++ b/templates/workspaces/base/terraform/keyvault.tf @@ -5,6 +5,7 @@ resource "azurerm_key_vault" "kv" { location = azurerm_resource_group.ws.location resource_group_name = azurerm_resource_group.ws.name sku_name = "standard" + enable_rbac_authorization = true purge_protection_enabled = true tenant_id = data.azurerm_client_config.current.tenant_id tags = local.tre_workspace_tags @@ -66,22 +67,20 @@ data "azurerm_user_assigned_identity" "resource_processor_vmss_id" { resource_group_name = "rg-${var.tre_id}" } -resource "azurerm_key_vault_access_policy" "resource_processor" { - key_vault_id = azurerm_key_vault.kv.id - tenant_id = data.azurerm_user_assigned_identity.resource_processor_vmss_id.tenant_id - object_id = data.azurerm_user_assigned_identity.resource_processor_vmss_id.principal_id +resource "azurerm_role_assignment" "keyvault_resourceprocessor_ws_role" { + scope = azurerm_key_vault.kv.id + role_definition_name = "Key Vault Administrator" + principal_id = data.azurerm_user_assigned_identity.resource_processor_vmss_id.principal_id + - secret_permissions = ["Get", "List", "Set", "Delete", "Purge", "Recover"] } # If running the terraform locally -resource "azurerm_key_vault_access_policy" "deployer" { - count = var.enable_local_debugging ? 1 : 0 - key_vault_id = azurerm_key_vault.kv.id - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id - - secret_permissions = ["Get", "List", "Set", "Delete", "Purge", "Recover"] +resource "azurerm_role_assignment" "keyvault_deployer_ws_role" { + count = var.enable_local_debugging ? 1 : 0 + scope = azurerm_key_vault.kv.id + role_definition_name = "Key Vault Administrator" + principal_id = data.azurerm_client_config.current.object_id } resource "terraform_data" "wait_for_dns_vault" { @@ -104,8 +103,8 @@ resource "azurerm_key_vault_secret" "aad_tenant_id" { key_vault_id = azurerm_key_vault.kv.id tags = local.tre_workspace_tags depends_on = [ - azurerm_key_vault_access_policy.deployer, - azurerm_key_vault_access_policy.resource_processor, + azurerm_role_assignment.keyvault_deployer_ws_role, + azurerm_role_assignment.keyvault_resourceprocessor_ws_role, terraform_data.wait_for_dns_vault ] @@ -121,8 +120,8 @@ resource "azurerm_key_vault_secret" "client_id" { count = var.register_aad_application ? 0 : 1 tags = local.tre_workspace_tags depends_on = [ - azurerm_key_vault_access_policy.deployer, - azurerm_key_vault_access_policy.resource_processor, + azurerm_role_assignment.keyvault_deployer_ws_role, + azurerm_role_assignment.keyvault_resourceprocessor_ws_role, terraform_data.wait_for_dns_vault ] @@ -144,8 +143,8 @@ resource "azurerm_key_vault_secret" "client_secret" { count = var.register_aad_application ? 0 : 1 tags = local.tre_workspace_tags depends_on = [ - azurerm_key_vault_access_policy.deployer, - azurerm_key_vault_access_policy.resource_processor, + azurerm_role_assignment.keyvault_deployer_ws_role, + azurerm_role_assignment.keyvault_resourceprocessor_ws_role, terraform_data.wait_for_dns_vault ] diff --git a/templates/workspaces/base/terraform/workspace.tf b/templates/workspaces/base/terraform/workspace.tf index bc93f4e34..14f4786e0 100644 --- a/templates/workspaces/base/terraform/workspace.tf +++ b/templates/workspaces/base/terraform/workspace.tf @@ -38,8 +38,8 @@ module "aad" { create_aad_groups = var.create_aad_groups depends_on = [ - azurerm_key_vault_access_policy.deployer, - azurerm_key_vault_access_policy.resource_processor, + azurerm_role_assignment.keyvault_deployer_ws_role, + azurerm_role_assignment.keyvault_resourceprocessor_ws_role, terraform_data.wait_for_dns_vault ] } From d5332c933d6fd032b2e9d05a5a4aa312a280e541 Mon Sep 17 00:00:00 2001 From: jonnyry Date: Mon, 4 Nov 2024 20:11:46 +0000 Subject: [PATCH 2/2] Fix lint issues (terraform formatting) --- core/terraform/appgateway/certificate.tf | 2 +- core/terraform/keyvault.tf | 4 ++-- .../resource_processor/vmss_porter/main.tf | 2 +- templates/workspaces/base/terraform/keyvault.tf | 14 +++++++------- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/core/terraform/appgateway/certificate.tf b/core/terraform/appgateway/certificate.tf index 81251167c..c4f22db14 100644 --- a/core/terraform/appgateway/certificate.tf +++ b/core/terraform/appgateway/certificate.tf @@ -1,7 +1,7 @@ resource "azurerm_role_assignment" "keyvault_appgw_role" { scope = var.keyvault_id role_definition_name = "Key Vault Secrets User" - principal_id = azurerm_user_assigned_identity.agw_id.principal_id // id-agw- + principal_id = azurerm_user_assigned_identity.agw_id.principal_id // id-agw- } resource "azurerm_key_vault_certificate" "tlscert" { diff --git a/core/terraform/keyvault.tf b/core/terraform/keyvault.tf index 60433f1d5..5d75ae917 100644 --- a/core/terraform/keyvault.tf +++ b/core/terraform/keyvault.tf @@ -14,13 +14,13 @@ resource "azurerm_key_vault" "kv" { resource "azurerm_role_assignment" "keyvault_deployer_role" { scope = azurerm_key_vault.kv.id role_definition_name = "Key Vault Administrator" - principal_id = data.azurerm_client_config.current.object_id // deployer - either CICD service principal or local user + principal_id = data.azurerm_client_config.current.object_id // deployer - either CICD service principal or local user } resource "azurerm_role_assignment" "keyvault_apiidentity_role" { scope = azurerm_key_vault.kv.id role_definition_name = "Key Vault Secrets User" - principal_id = azurerm_user_assigned_identity.id.principal_id // id-api- + principal_id = azurerm_user_assigned_identity.id.principal_id // id-api- } data "azurerm_private_dns_zone" "vaultcore" { diff --git a/core/terraform/resource_processor/vmss_porter/main.tf b/core/terraform/resource_processor/vmss_porter/main.tf index 08ea8b05c..68e05d021 100644 --- a/core/terraform/resource_processor/vmss_porter/main.tf +++ b/core/terraform/resource_processor/vmss_porter/main.tf @@ -186,7 +186,7 @@ resource "azurerm_role_assignment" "subscription_contributor" { resource "azurerm_role_assignment" "keyvault_vmss_role" { scope = var.key_vault_id role_definition_name = "Key Vault Administrator" - principal_id = azurerm_user_assigned_identity.vmss_msi.principal_id // id-vmss- + principal_id = azurerm_user_assigned_identity.vmss_msi.principal_id // id-vmss- } module "terraform_azurerm_environment_configuration" { diff --git a/templates/workspaces/base/terraform/keyvault.tf b/templates/workspaces/base/terraform/keyvault.tf index 73126a9a4..e8cba49f1 100644 --- a/templates/workspaces/base/terraform/keyvault.tf +++ b/templates/workspaces/base/terraform/keyvault.tf @@ -1,14 +1,14 @@ data "azurerm_client_config" "current" {} resource "azurerm_key_vault" "kv" { - name = local.keyvault_name - location = azurerm_resource_group.ws.location - resource_group_name = azurerm_resource_group.ws.name - sku_name = "standard" + name = local.keyvault_name + location = azurerm_resource_group.ws.location + resource_group_name = azurerm_resource_group.ws.name + sku_name = "standard" enable_rbac_authorization = true - purge_protection_enabled = true - tenant_id = data.azurerm_client_config.current.tenant_id - tags = local.tre_workspace_tags + purge_protection_enabled = true + tenant_id = data.azurerm_client_config.current.tenant_id + tags = local.tre_workspace_tags network_acls { bypass = "AzureServices"