refactor(infrastructure): rename boolean variables to should_ prefix and add missing core variables

infrastructure/terraform/main.tf

@@ -97,19 +97,19 @@ module "platform" {
   should_deploy_postgresql = var.should_deploy_postgresql
   should_deploy_redis      = var.should_deploy_redis
   postgresql_config = {
-    location                  = coalesce(var.postgresql_location, var.location)
-    sku_name                  = var.postgresql_sku_name
-    storage_mb                = var.postgresql_storage_mb
-    version                   = var.postgresql_version
-    databases                 = var.postgresql_databases
-    zone                      = var.postgresql_zone
-    high_availability_enabled = var.postgresql_high_availability.enabled
-    standby_availability_zone = var.postgresql_high_availability.standby_availability_zone
+    location                        = coalesce(var.postgresql_location, var.location)
+    sku_name                        = var.postgresql_sku_name
+    storage_mb                      = var.postgresql_storage_mb
+    version                         = var.postgresql_version
+    databases                       = var.postgresql_databases
+    zone                            = var.postgresql_zone
+    should_enable_high_availability = var.postgresql_high_availability.should_enable
+    standby_availability_zone       = var.postgresql_high_availability.standby_availability_zone
   }
   redis_config = {
-    sku_name                  = var.redis_sku_name
-    clustering_policy         = var.redis_clustering_policy
-    high_availability_enabled = var.should_enable_redis_high_availability
+    sku_name                        = var.redis_sku_name
+    clustering_policy               = var.redis_clustering_policy
+    should_enable_high_availability = var.should_enable_redis_high_availability
   }
 
   // OSMO workload identity
@@ -142,6 +142,7 @@ module "sil" {
   environment     = var.environment
   resource_prefix = var.resource_prefix
   instance        = var.instance
+  location        = var.location
   resource_group  = local.resource_group
 
   // Current user OID for cluster admin role assignments (from Microsoft Graph)
@@ -172,7 +173,7 @@ module "sil" {
     should_enable_system_node_pool_auto_scaling = var.should_enable_system_node_pool_auto_scaling
     system_node_pool_min_count                  = var.system_node_pool_min_count
     system_node_pool_max_count                  = var.system_node_pool_max_count
-    is_private_cluster                          = var.should_enable_private_aks_cluster
+    should_enable_private_cluster               = var.should_enable_private_aks_cluster
     system_node_pool_zones                      = var.system_node_pool_zones
     should_enable_microsoft_defender            = var.should_enable_microsoft_defender
   }

infrastructure/terraform/modules/dataviewer/identity.tf

@@ -11,6 +11,6 @@
 
 resource "azurerm_user_assigned_identity" "dataviewer" {
   name                = "id-dataviewer-${local.resource_name_suffix}"
-  location            = var.resource_group.location
+  location            = var.location
   resource_group_name = var.resource_group.name
 }

infrastructure/terraform/modules/dataviewer/variables.core.tf

@@ -1,7 +1,7 @@
 /**
  * # Core Variables
  *
- * Core variables shared across all modules: environment, resource_prefix, instance, resource_group.
+ * Core variables shared across all modules: environment, resource_prefix, location, instance.
  */
 
 /*
@@ -20,6 +20,11 @@ variable "instance" {
   default     = "001"
 }
 
+variable "location" {
+  type        = string
+  description = "Location for all resources in this module"
+}
+
 variable "resource_group" {
   type = object({
     id       = string

infrastructure/terraform/modules/platform/postgresql.tf

@@ -55,7 +55,7 @@ resource "azurerm_postgresql_flexible_server" "main" {
   public_network_access_enabled = false
 
   dynamic "high_availability" {
-    for_each = var.postgresql_config.high_availability_enabled ? [1] : []
+    for_each = var.postgresql_config.should_enable_high_availability ? [1] : []
     content {
       mode                      = "ZoneRedundant"
       standby_availability_zone = var.postgresql_config.standby_availability_zone

infrastructure/terraform/modules/platform/redis.tf

@@ -18,7 +18,7 @@ resource "azurerm_managed_redis" "main" {
   resource_group_name = var.resource_group.name
   sku_name            = var.redis_config.sku_name
 
-  high_availability_enabled = var.redis_config.high_availability_enabled
+  high_availability_enabled = var.redis_config.should_enable_high_availability
 
   default_database {
     clustering_policy                  = var.redis_config.clustering_policy

infrastructure/terraform/modules/platform/variables.tf

@@ -91,25 +91,25 @@ variable "should_deploy_postgresql" {
 
 variable "postgresql_config" {
   type = object({
-    location                  = string
-    sku_name                  = string
-    storage_mb                = number
-    version                   = string
-    databases                 = map(object({ collation = string, charset = string }))
-    zone                      = optional(string)
-    high_availability_enabled = optional(bool, false)
-    standby_availability_zone = optional(string)
+    location                        = string
+    sku_name                        = string
+    storage_mb                      = number
+    version                         = string
+    databases                       = map(object({ collation = string, charset = string }))
+    zone                            = optional(string)
+    should_enable_high_availability = optional(bool, false)
+    standby_availability_zone       = optional(string)
   })
   description = "PostgreSQL configuration for OSMO including location, SKU, storage, zone, HA settings, and database definitions"
   default = {
-    location                  = "westus3"
-    sku_name                  = "GP_Standard_D2s_v3"
-    storage_mb                = 32768
-    version                   = "16"
-    databases                 = { osmo = { collation = "en_US.utf8", charset = "utf8" } }
-    zone                      = null
-    high_availability_enabled = false
-    standby_availability_zone = null
+    location                        = "westus3"
+    sku_name                        = "GP_Standard_D2s_v3"
+    storage_mb                      = 32768
+    version                         = "16"
+    databases                       = { osmo = { collation = "en_US.utf8", charset = "utf8" } }
+    zone                            = null
+    should_enable_high_availability = false
+    standby_availability_zone       = null
   }
 }
 
@@ -125,15 +125,15 @@ variable "should_deploy_redis" {
 
 variable "redis_config" {
   type = object({
-    sku_name                  = string
-    clustering_policy         = string
-    high_availability_enabled = optional(bool, false)
+    sku_name                        = string
+    clustering_policy               = string
+    should_enable_high_availability = optional(bool, false)
   })
   description = "Redis configuration for OSMO including SKU, clustering policy, and HA settings. EnterpriseCluster recommended for clients that don't support Redis Cluster MOVED redirects"
   default = {
-    sku_name                  = "Balanced_B10"
-    clustering_policy         = "EnterpriseCluster"
-    high_availability_enabled = false
+    sku_name                        = "Balanced_B10"
+    clustering_policy               = "EnterpriseCluster"
+    should_enable_high_availability = false
   }
 }
 

infrastructure/terraform/modules/sil/aks.tf

@@ -22,7 +22,7 @@
 
 resource "azurerm_user_assigned_identity" "aks" {
   name                = "id-aks-${local.resource_name_suffix}"
-  location            = var.resource_group.location
+  location            = var.location
   resource_group_name = var.resource_group.name
 }
 
@@ -38,8 +38,8 @@ resource "azurerm_kubernetes_cluster" "main" {
   kubernetes_version                = null // Use latest stable version
   automatic_upgrade_channel         = "patch"
   sku_tier                          = "Standard"
-  private_cluster_enabled           = var.aks_config.is_private_cluster
-  private_dns_zone_id               = var.aks_config.is_private_cluster && local.pe_enabled ? var.private_dns_zones["aks"].id : null
+  private_cluster_enabled           = var.aks_config.should_enable_private_cluster
+  private_dns_zone_id               = var.aks_config.should_enable_private_cluster && local.pe_enabled ? var.private_dns_zones["aks"].id : null
   local_account_disabled            = true
   azure_policy_enabled              = true
   oidc_issuer_enabled               = true
@@ -129,9 +129,9 @@ resource "azurerm_kubernetes_cluster_node_pool" "gpu" {
   vm_size               = each.value.vm_size
   vnet_subnet_id        = azurerm_subnet.gpu_node_pool[each.key].id
   node_taints           = each.value.node_taints
-  auto_scaling_enabled  = each.value.enable_auto_scaling
-  min_count             = each.value.enable_auto_scaling ? each.value.min_count : null
-  max_count             = each.value.enable_auto_scaling ? each.value.max_count : null
+  auto_scaling_enabled  = each.value.should_enable_auto_scaling
+  min_count             = each.value.should_enable_auto_scaling ? each.value.min_count : null
+  max_count             = each.value.should_enable_auto_scaling ? each.value.max_count : null
   priority              = each.value.priority
   zones                 = each.value.zones
   eviction_policy       = each.value.priority == "Spot" ? each.value.eviction_policy : null
@@ -160,7 +160,7 @@ resource "azurerm_kubernetes_cluster_node_pool" "gpu" {
 resource "azurerm_private_endpoint" "aks" {
   // Use known boolean values for count to avoid plan-time dependency issues
   // pe_enabled ensures the PE subnet exists when this resource is created
-  count = var.aks_config.is_private_cluster && local.pe_enabled ? 1 : 0
+  count = var.aks_config.should_enable_private_cluster && local.pe_enabled ? 1 : 0
 
   name                = "pe-aks-${local.resource_name_suffix}"
   location            = var.resource_group.location

infrastructure/terraform/modules/sil/role-assignments.tf

@@ -39,7 +39,7 @@ resource "azurerm_role_assignment" "aks_rbac_cluster_admin" {
 // Grant AKS identity Private DNS Zone Contributor role for custom DNS zone management
 // This must be created BEFORE the AKS cluster so the identity can manage DNS records
 resource "azurerm_role_assignment" "aks_dns_zone_contributor" {
-  count = var.aks_config.is_private_cluster && local.pe_enabled ? 1 : 0
+  count = var.aks_config.should_enable_private_cluster && local.pe_enabled ? 1 : 0
 
   scope                            = var.private_dns_zones["aks"].id
   role_definition_name             = "Private DNS Zone Contributor"

infrastructure/terraform/modules/sil/variables.core.tf

@@ -20,6 +20,11 @@ variable "instance" {
   default     = "001"
 }
 
+variable "location" {
+  type        = string
+  description = "Location for all resources in this module"
+}
+
 variable "resource_group" {
   type = object({
     id       = string

infrastructure/terraform/modules/sil/variables.tf

@@ -64,7 +64,7 @@ variable "aks_config" {
     should_enable_system_node_pool_auto_scaling = bool
     system_node_pool_min_count                  = optional(number)
     system_node_pool_max_count                  = optional(number)
-    is_private_cluster                          = bool
+    should_enable_private_cluster               = bool
     system_node_pool_zones                      = optional(list(string))
     should_enable_microsoft_defender            = optional(bool, false)
   })
@@ -75,40 +75,40 @@ variable "aks_config" {
     should_enable_system_node_pool_auto_scaling = false
     system_node_pool_min_count                  = null
     system_node_pool_max_count                  = null
-    is_private_cluster                          = true
+    should_enable_private_cluster               = true
     system_node_pool_zones                      = null
   }
 }
 
 variable "node_pools" {
   type = map(object({
-    vm_size                 = string
-    node_count              = optional(number, null)
-    subnet_address_prefixes = list(string)
-    node_taints             = optional(list(string), [])
-    node_labels             = optional(map(string), {})
-    gpu_driver              = optional(string)
-    priority                = optional(string, "Regular")
-    enable_auto_scaling     = optional(bool, false)
-    min_count               = optional(number, null)
-    max_count               = optional(number, null)
-    zones                   = optional(list(string), null)
-    eviction_policy         = optional(string, "Deallocate")
+    vm_size                    = string
+    node_count                 = optional(number, null)
+    subnet_address_prefixes    = list(string)
+    node_taints                = optional(list(string), [])
+    node_labels                = optional(map(string), {})
+    gpu_driver                 = optional(string)
+    priority                   = optional(string, "Regular")
+    should_enable_auto_scaling = optional(bool, false)
+    min_count                  = optional(number, null)
+    max_count                  = optional(number, null)
+    zones                      = optional(list(string), null)
+    eviction_policy            = optional(string, "Deallocate")
   }))
   description = "Additional AKS node pools configuration. Map key is used as the node pool name. Note: Pod subnets are not used with Azure CNI Overlay mode"
   default = {
     gpu = {
-      vm_size                 = "Standard_NV36ads_A10_v5"
-      node_count              = null
-      subnet_address_prefixes = ["10.0.16.0/24"]
-      node_taints             = ["nvidia.com/gpu:NoSchedule", "kubernetes.azure.com/scalesetpriority=spot:NoSchedule"]
-      gpu_driver              = "Install"
-      priority                = "Spot"
-      enable_auto_scaling     = true
-      min_count               = 0
-      max_count               = 1
-      zones                   = []
-      eviction_policy         = "Delete"
+      vm_size                    = "Standard_NV36ads_A10_v5"
+      node_count                 = null
+      subnet_address_prefixes    = ["10.0.16.0/24"]
+      node_taints                = ["nvidia.com/gpu:NoSchedule", "kubernetes.azure.com/scalesetpriority=spot:NoSchedule"]
+      gpu_driver                 = "Install"
+      priority                   = "Spot"
+      should_enable_auto_scaling = true
+      min_count                  = 0
+      max_count                  = 1
+      zones                      = []
+      eviction_policy            = "Delete"
     }
   }
 }

infrastructure/terraform/modules/vpn/main.tf

@@ -15,7 +15,7 @@ locals {
 
   // Determine authentication types based on configuration
   has_certificate_auth = var.root_certificate_public_data != null
-  has_aad_auth         = var.aad_auth_config.enabled
+  has_aad_auth         = var.aad_auth_config.should_enable
 
   // Build vpn_auth_types list based on enabled authentication methods
   vpn_auth_types = compact([

infrastructure/terraform/modules/vpn/variables.tf

@@ -74,15 +74,15 @@ variable "root_certificate_public_data" {
 
 variable "aad_auth_config" {
   type = object({
-    enabled     = bool
-    tenant_id   = optional(string)
-    audience_id = optional(string, "c632b3df-fb67-4d84-bdcf-b95ad541b5c8")
+    should_enable = bool
+    tenant_id     = optional(string)
+    audience_id   = optional(string, "c632b3df-fb67-4d84-bdcf-b95ad541b5c8")
   })
   description = "Azure AD authentication configuration for P2S VPN. tenant_id defaults to current Azure client tenant if not specified. Uses Microsoft-registered Azure VPN application by default. Requires OpenVPN protocol"
   default = {
-    enabled     = false
-    tenant_id   = null
-    audience_id = "c632b3df-fb67-4d84-bdcf-b95ad541b5c8"
+    should_enable = false
+    tenant_id     = null
+    audience_id   = "c632b3df-fb67-4d84-bdcf-b95ad541b5c8"
   }
 }
 

infrastructure/terraform/terraform.tfvars.example

@@ -39,9 +39,9 @@ node_pools = {
     node_labels = {
       "kubernetes.azure.com/scalesetpriority" = "spot"
     }
-    priority                = "Spot"
-    enable_auto_scaling     = true
-    min_count               = 1
+    priority                   = "Spot"
+    should_enable_auto_scaling = true
+    min_count                  = 1
     max_count               = 1
     zones                   = []
     eviction_policy         = "Delete"
@@ -59,9 +59,9 @@ node_pools = {
 //     node_labels = {
 //       "nvidia.com/gpu.deploy.driver" = "false"
 //     }
-//     priority            = "Regular"
-//     enable_auto_scaling = true
-//     min_count           = 1
+//     priority                   = "Regular"
+//     should_enable_auto_scaling = true
+//     min_count                  = 1
 //     max_count           = 1
 //     zones               = []
 //   }
@@ -77,9 +77,9 @@ node_pools = {
 //       "kubernetes.azure.com/scalesetpriority" = "spot"
 //     }
 //     priority            = "Spot"
-//     eviction_policy     = "Delete"
-//     enable_auto_scaling = true
-//     min_count           = 1
+//     eviction_policy            = "Delete"
+//     should_enable_auto_scaling = true
+//     min_count                  = 1
 //     max_count           = 1
 //     zones               = []
 //   }