PR review comments

mrlockstar · mrlockstar · commit 5f40854a7915 · 2026-01-27T16:11:15.000Z
diff --git a/.azuredevops/pipelines/hub-infrastructure-dev.yaml b/.azuredevops/pipelines/hub-infrastructure-dev.yaml
@@ -5,7 +5,6 @@ pr: none
 
 pool:
   name: private-pool-hub-nonlive-uks
-  # vmImage: ubuntu-latest
 
 resources:
   repositories:
diff --git a/infrastructure/bootstrap/environments/live/hub.bicepparam b/infrastructure/bootstrap/environments/live/hub.bicepparam
@@ -5,4 +5,3 @@ param vnetAddressPrefixes = [
   '10.21.0.0/16'
 ]
 param devopsSubnetAddressPrefix = '10.21.1.0/24'
-//param devopsInfrastructureId = ''
diff --git a/infrastructure/bootstrap/environments/nonlive/hub.bicepparam b/infrastructure/bootstrap/environments/nonlive/hub.bicepparam
@@ -7,4 +7,3 @@ param vnetAddressPrefixes = [
 param devopsSubnetAddressPrefix = '10.11.1.0/24'
 param privateEndpointSubnetAddressPrefix = '10.11.2.0/24'
 param enableSoftDelete = true
-// param devopsInfrastructureId = ''
diff --git a/infrastructure/terraform/hub/azure_monitor_private_link_scope.tf b/infrastructure/terraform/hub/azure_monitor_private_link_scope.tf
@@ -26,8 +26,6 @@ module "private_endpoint_ampls" {
       module.private_dns_zones["${each.key}-automation"].id,
       module.private_dns_zones["${each.key}-operations_data_store"].id,
       module.private_dns_zones["${each.key}-operations_management_suite"].id,
-      # created in the bootstrap
-      # module.private_dns_zones["${each.key}-storage_blob"].id
     ]
   }
 
diff --git a/infrastructure/terraform/hub/data.tf b/infrastructure/terraform/hub/data.tf
@@ -9,10 +9,6 @@ data "azurerm_client_config" "current" {}
 #   display_name = var.avd_admins_group_name
 # }
 
-# data "azuread_group" "avd_platform_users" {
-#   display_name = "DToS-platform-team-Dev"
-# }
-
 # This client id is the same for all Azure customers - it is not a secret.
 # https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/app_service_certificate
 # data "azuread_service_principal" "MicrosoftAzureAppService" {
@@ -22,19 +18,3 @@ data "azurerm_client_config" "current" {}
 # data "azuread_service_principal" "MicrosoftAzureFrontDoorCdn" {
 #   client_id = "205478c0-bd83-4e1b-a9d6-db63a3e1e1c8"
 # }
-
-# data "azurerm_key_vault_secret" "object-id" {
-#   for_each     = var.regions
-#   name         = "dtos-apim-object-id"
-#   key_vault_id = module.key_vault[each.key].key_vault_id
-
-#   depends_on = [azurerm_key_vault_access_policy.terraform-mi]
-# }
-
-# data "azurerm_key_vault_secret" "secret" {
-#   for_each     = var.regions
-#   name         = "dtos-apim-secret"
-#   key_vault_id = module.key_vault[each.key].key_vault_id
-
-#   depends_on = [azurerm_key_vault_access_policy.terraform-mi]
-# }
diff --git a/infrastructure/terraform/hub/diagnostic_settings.tf b/infrastructure/terraform/hub/diagnostic_settings.tf
@@ -1,8 +1,4 @@
 locals {
-  # APIM
-  monitor_diagnostic_setting_apim_enabled_logs = ["GatewayLogs", "WebSocketConnectionLogs", "DeveloperPortalAuditLogs"]
-  monitor_diagnostic_setting_apim_metrics      = ["AllMetrics"]
-
   # FRONTDOOR
   monitor_diagnostic_setting_frontdoor_enabled_logs = ["FrontDoorAccessLog", "FrontDoorHealthProbeLog", "FrontDoorWebApplicationFirewallLog"]
   monitor_diagnostic_setting_frontdoor_metrics      = ["AllMetrics"]
diff --git a/infrastructure/terraform/hub/dns_private.tf b/infrastructure/terraform/hub/dns_private.tf
@@ -40,8 +40,6 @@ module "private_dns_resolver" {
 
 locals {
   private_dns_zones = {
-    # national_screening          = var.dns_zone_name_private.nationalscreening
-    # screening                   = var.dns_zone_name_private.screening
     container_registry          = "privatelink.azurecr.io"
     app_insights                = var.private_dns_zones.is_app_insights_private_dns_zone_enabled ? "privatelink.monitor.azure.com" : null
     automation                  = var.private_dns_zones.is_app_insights_private_dns_zone_enabled ? "privatelink.agentsvc.azure-automation.net" : null
@@ -52,8 +50,6 @@ locals {
     event_grid                  = var.private_dns_zones.is_event_grid_enabled_dns_zone_enabled ? "privatelink.eventgrid.azure.net" : null
     azure_sql                   = var.private_dns_zones.is_azure_sql_private_dns_zone_enabled ? "privatelink.database.windows.net" : null
     postgres_sql                = var.private_dns_zones.is_postgres_sql_private_dns_zone_enabled ? "privatelink.postgres.database.azure.com" : null
-    # key_vault                   = var.private_dns_zones.is_key_vault_private_dns_zone_enabled ? "privatelink.vaultcore.azure.net" : null
-    # storage_blob                = var.private_dns_zones.is_storage_private_dns_zone_enabled ? "privatelink.blob.core.windows.net" : null
     storage_queue               = var.private_dns_zones.is_storage_private_dns_zone_enabled ? "privatelink.queue.core.windows.net" : null
     storage_table               = var.private_dns_zones.is_storage_private_dns_zone_enabled ? "privatelink.table.core.windows.net" : null
     event_hub                   = var.private_dns_zones.is_event_hub_private_dns_zone_enabled ? "privatelink.servicebus.windows.net" : null
diff --git a/infrastructure/terraform/hub/firewall.tf b/infrastructure/terraform/hub/firewall.tf
@@ -4,7 +4,6 @@ module "firewall" {
   source = "../../../../dtos-devops-templates/infrastructure/modules/firewall"
 
   firewall_name       = module.config[each.key].names.firewall
-  # resource_group_name = azurerm_resource_group.rg_hub[each.key].name
   resource_group_name = var.vnet_resource_group
   location            = each.key
 
diff --git a/infrastructure/terraform/hub/networking_hub.tf b/infrastructure/terraform/hub/networking_hub.tf
@@ -46,7 +46,6 @@ module "subnets_hub" {
   network_security_group_name       = each.value.nsg_name
   network_security_group_nsg_rules  = each.value.nsg_rules
   create_nsg                        = coalesce(each.value.create_nsg, true)
-  # resource_group_name               = azurerm_resource_group.rg_hub[each.value.vnet_key].name
   resource_group_name               = var.vnet_resource_group
   vnet_name                         = var.vnet_name
   address_prefixes                  = [each.value.address_prefixes]
diff --git a/infrastructure/terraform/hub/outputs.tf b/infrastructure/terraform/hub/outputs.tf
@@ -30,10 +30,6 @@ output "private_endpoint_rg_name" {
   value = { for k, v in azurerm_resource_group.rg_private_endpoints : k => v.name }
 }
 
-# output "public_dns_zone_rg_name" {
-#   value = var.dns_zone_rg_name_public
-# }
-
 output "subnets_hub" {
   value = module.subnets_hub
 }
diff --git a/infrastructure/terraform/hub/rbac.tf b/infrastructure/terraform/hub/rbac.tf
diff --git a/infrastructure/terraform/hub/variables.tf b/infrastructure/terraform/hub/variables.tf
@@ -131,16 +131,6 @@ variable "firewall_config" {
   default = {}
 }
 
-# variable "key_vault" {
-#   description = "Configuration for the key vault"
-#   type = object({
-#     disk_encryption   = optional(bool, true)
-#     soft_del_ret_days = optional(number, 7)
-#     purge_prot        = optional(bool, false)
-#     sku_name          = optional(string, "standard")
-#   })
-# }
-
 variable "law" {
   description = "Configuration of the Log Analytics Workspace"
   type = object({
@@ -250,7 +240,6 @@ variable "AVD_APPLICATION_ID" {
   type        = string
 }
 
-
 variable "virtual_desktop_group_active" {
   description = <<-EOT
     This can either be 'blue', 'green', 'both-with-blue-primary',  'both-with-green-primary', 'both-with-blue-primary-but-equal-vms' or 'both-with-green-primary-but-equal-vms'.
diff --git a/infrastructure/terraform/hub/virtual_desktop.tf b/infrastructure/terraform/hub/virtual_desktop.tf
@@ -20,6 +20,7 @@ locals {
   )
 }
 
+# Once issues with the Data Reader is resolved this will be re-added.
 # data "azuread_service_principal" "avd" {
 #   application_id = local.principal_id
 # }
@@ -53,35 +54,27 @@ module "virtual-desktop-blue" {
   host_pool_name        = module.config[each.key].names.avd-host-pool
   location              = each.key
 
-  # entra_users_group_id = (
-  #   local.blue_avd_primary
-  #   ? data.azuread_group.avd_users.id
-  #   : data.azuread_group.avd_users.id
-  # )
-
   entra_users_group_id = var.ENTRA_USERS_GROUP_ID
-
-  # entra_admins_group_id = (
-  #   local.blue_avd_primary
-  #   ? data.azuread_group.avd_admins.id
-  #   : data.azuread_group.avd_admins.id
-  # )
-
   entra_admins_group_id = var.ENTRA_ADMINS_GROUP_ID
 
   maximum_sessions_allowed  = var.avd_maximum_sessions_allowed
   resource_group_name       = azurerm_resource_group.avd_blue[each.key].name
   resource_group_id         = azurerm_resource_group.avd_blue[each.key].id
   scaling_plan_name         = module.config[each.key].names.avd-scaling-plan
+
+  source_image_id           = var.AVD_SOURCE_IMAGE_ID
+  # left as example of source image reference Example 1
   # source_image_reference = {
   #   publisher = "MicrosoftWindowsDesktop"
   #   offer     = "windows-11"
   #   sku       = "win11-23h2-avd"
   #   version   = "latest"
   # }
-  source_image_id           = var.AVD_SOURCE_IMAGE_ID
+
+  # left as example of source image reference Example 2
   # source_image_reference    = var.avd_source_image_reference
   # source_image_from_gallery = var.avd_source_image_from_gallery
+
   subnet_id                 = module.subnets_hub["${module.config[each.key].names.subnet}-virtual-desktop"].id
   vm_count                  = local.blue_avd_primary || local.equal_vm_counts ? var.avd_vm_count : 1
   vm_name_prefix            = module.config[each.key].names.avd-host
@@ -115,23 +108,8 @@ module "virtual-desktop-green" {
   dag_name              = module.config[each.key].names.avd-dag
   host_pool_name        = "${module.config[each.key].names.avd-host-pool}-v2"
   location              = each.key
-
-  # entra_users_group_id = (
-  #   local.green_avd_primary
-  #   ? data.azuread_group.avd_users.id
-  #   : data.azuread_group.avd_users.id
-  # )
-
-  entra_users_group_id = var.ENTRA_USERS_GROUP_ID
-
-  # entra_admins_group_id = (
-  #   local.green_avd_primary
-  #   ? data.azuread_group.avd_admins.id
-  #   : data.azuread_group.avd_admins.id
-  # )
-
+  entra_users_group_id  = var.ENTRA_USERS_GROUP_ID
   entra_admins_group_id = var.ENTRA_ADMINS_GROUP_ID
-
   maximum_sessions_allowed  = var.avd_maximum_sessions_allowed
   resource_group_name       = azurerm_resource_group.avd_green[each.key].name
   resource_group_id         = azurerm_resource_group.avd_green[each.key].id
@@ -146,7 +124,6 @@ module "virtual-desktop-green" {
   vm_size                   = var.avd_vm_size
   vm_license_type           = "Windows_Client"
   workspace_name            = "lungcs-${each.key}"
-  # workspace_name            = module.config[each.key].names.avd-workspace
 
   principal_id              = var.AVD_OBJECT_ID
   tags = var.tags

Original file line number	Diff line number	Diff line change
`@@ -5,4 +5,3 @@ param vnetAddressPrefixes = [`
`5`	`5`	`'10.21.0.0/16'`
`6`	`6`	`]`
`7`	`7`	`param devopsSubnetAddressPrefix = '10.21.1.0/24'`
`8`		`-//param devopsInfrastructureId = ''`
Original file line number	Diff line number	Diff line change
`@@ -26,8 +26,6 @@ module "private_endpoint_ampls" {`
`26`	`26`	`module.private_dns_zones["${each.key}-automation"].id,`
`27`	`27`	`module.private_dns_zones["${each.key}-operations_data_store"].id,`
`28`	`28`	`module.private_dns_zones["${each.key}-operations_management_suite"].id,`
`29`		`- # created in the bootstrap`
`30`		`- # module.private_dns_zones["${each.key}-storage_blob"].id`
`31`	`29`	`]`
`32`	`30`	`}`
`33`	`31`
Original file line number	Diff line number	Diff line change
`@@ -30,10 +30,6 @@ output "private_endpoint_rg_name" {`
`30`	`30`	`value = { for k, v in azurerm_resource_group.rg_private_endpoints : k => v.name }`
`31`	`31`	`}`
`32`	`32`
`33`		`-# output "public_dns_zone_rg_name" {`
`34`		`-# value = var.dns_zone_rg_name_public`
`35`		`-# }`
`36`		`-`
`37`	`33`	`output "subnets_hub" {`
`38`	`34`	`value = module.subnets_hub`
`39`	`35`	`}`