Fix subnet issue from #943 (#952)

* Fix issues created by #943 - Use the existing for loop logic - Remove var.subnet_private_endpoint_network_policies_enabled - Change core to use new private_endpoint_network_policies_enabled in core subnet config - Solves #950 * fmt * docs * Remove unneeded logic around private policy This to make the code more readable * Add config to locals * Remove network policy variables --------- Co-authored-by: Philip Laine <[email protected]>
XenitAB · Mar 8, 2023 · 494a7a4 · 494a7a4
1 parent 1aff96b
commit 494a7a4
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 48 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [#949](https://github.com/XenitAB/terraform-modules/pull/949) Update audit log alert criteria.
 - [#954](https://github.com/XenitAB/terraform-modules/pull/954) Make audit log alert have bigger window_size and frequency.
 
+### Fixed
+
+- [#952](https://github.com/XenitAB/terraform-modules/pull/952) Fix issues created by #943 and change core to use new private_endpoint_network_policies_enabled in core subnet config.
+
 ## 2023.02.3
 
 ### Changed

diff --git a/modules/azure/core/README.md b/modules/azure/core/README.md
@@ -60,12 +60,11 @@ No modules.
 | <a name="input_location_short"></a> [location\_short](#input\_location\_short) | The Azure region short name | `string` | n/a | yes |
 | <a name="input_name"></a> [name](#input\_name) | The commonName to use for the deploy | `string` | n/a | yes |
 | <a name="input_notification_email"></a> [notification\_email](#input\_notification\_email) | Email address to send alert notifications | `string` | n/a | yes |
-| <a name="input_peering_config"></a> [peering\_config](#input\_peering\_config) | Network peering configuration | <pre>list(object({<br>    name                         = string<br>    remote_virtual_network_id    = string<br>    allow_forwarded_traffic      = bool<br>    use_remote_gateways          = bool<br>    allow_virtual_network_access = bool<br>    allow_gateway_transit        = optional(bool, false) # Controls gatewayLinks can be used in the remote virtual network’s link to the local virtual network. <br>  }))</pre> | `[]` | no |
-| <a name="input_route_config"></a> [route\_config](#input\_route\_config) | Route configuration. Not applied to AKS subnets | <pre>list(object({<br>    subnet_name                   = string                # Short name for the subnet<br>    disable_bgp_route_propagation = optional(bool, false) # Controls propagation of routes learned by BGP on that route table <br>    routes = list(object({<br>      name                   = string # Name of the route<br>      address_prefix         = string # Example: 192.168.0.0/24<br>      next_hop_type          = string # VirtualNetworkGateway, VnetLocal, Internet, VirtualAppliance and None<br>      next_hop_in_ip_address = string # Only set if next_hop_type is VirtualAppliance<br>    }))<br><br>  }))</pre> | `[]` | no |
-| <a name="input_subnet_private_endpoints"></a> [subnet\_private\_endpoints](#input\_subnet\_private\_endpoints) | Enable private enpoint for specific subnet names | `map(bool)` | `{}` | no |
+| <a name="input_peering_config"></a> [peering\_config](#input\_peering\_config) | Network peering configuration | <pre>list(object({<br>    name                         = string<br>    remote_virtual_network_id    = string<br>    allow_forwarded_traffic      = bool<br>    use_remote_gateways          = bool<br>    allow_virtual_network_access = bool<br>    allow_gateway_transit        = optional(bool, false) # Controls gatewayLinks can be used in the remote virtual network’s link to the local virtual network.<br>  }))</pre> | `[]` | no |
+| <a name="input_route_config"></a> [route\_config](#input\_route\_config) | Route configuration. Not applied to AKS subnets | <pre>list(object({<br>    subnet_name                   = string                # Short name for the subnet<br>    disable_bgp_route_propagation = optional(bool, false) # Controls propagation of routes learned by BGP on that route table<br>    routes = list(object({<br>      name                   = string # Name of the route<br>      address_prefix         = string # Example: 192.168.0.0/24<br>      next_hop_type          = string # VirtualNetworkGateway, VnetLocal, Internet, VirtualAppliance and None<br>      next_hop_in_ip_address = string # Only set if next_hop_type is VirtualAppliance<br>    }))<br><br>  }))</pre> | `[]` | no |
 | <a name="input_subscription_name"></a> [subscription\_name](#input\_subscription\_name) | The subscription commonName to use for the deploy | `string` | n/a | yes |
 | <a name="input_unique_suffix"></a> [unique\_suffix](#input\_unique\_suffix) | Unique suffix that is used in globally unique resources names | `string` | n/a | yes |
-| <a name="input_vnet_config"></a> [vnet\_config](#input\_vnet\_config) | Address spaces used by virtual network | <pre>object({<br>    address_space = list(string)<br>    dns_servers   = list(string)<br>    subnets = list(object({<br>      name                                      = string<br>      cidr                                      = string<br>      service_endpoints                         = list(string)<br>      aks_subnet                                = bool<br>      private_endpoint_network_policies_enabled = optional(bool, true)<br>    }))<br>  })</pre> | n/a | yes |
+| <a name="input_vnet_config"></a> [vnet\_config](#input\_vnet\_config) | Address spaces used by virtual network | <pre>object({<br>    address_space = list(string)<br>    dns_servers   = list(string)<br>    subnets = list(object({<br>      name              = string<br>      cidr              = string<br>      service_endpoints = list(string)<br>      aks_subnet        = bool<br>    }))<br>  })</pre> | n/a | yes |
 
 ## Outputs
 

diff --git a/modules/azure/core/locals.tf b/modules/azure/core/locals.tf
@@ -1,21 +1,15 @@
 locals {
   subnets = [
     for subnet in var.vnet_config.subnets : {
-      vnet_resource                                    = "${var.environment}-${var.name}"
-      subnet_full_name                                 = "sn-${var.environment}-${var.location_short}-${var.name}-${subnet.name}"
-      subnet_short_name                                = subnet.name
-      subnet_cidr                                      = subnet.cidr
-      subnet_service_endpoints                         = subnet.service_endpoints
-      subnet_aks_subnet                                = subnet.aks_subnet
-      subnet_private_endpoint_network_policies_enabled = subnet.private_endpoint_network_policies_enabled
+      vnet_resource            = "${var.environment}-${var.name}"
+      subnet_full_name         = "sn-${var.environment}-${var.location_short}-${var.name}-${subnet.name}"
+      subnet_short_name        = subnet.name
+      subnet_cidr              = subnet.cidr
+      subnet_service_endpoints = subnet.service_endpoints
+      subnet_aks_subnet        = subnet.aks_subnet
     }
   ]
 
-  # If subnet is not defined in var.subnet_private_endpoints default it to false
-  subnet_private_endpoints = {
-    for subnet in local.subnets : subnet.subnet_short_name => try(var.subnet_private_endpoints[subnet.subnet_short_name], false)
-  }
-
   peerings = [
     for peering_config in var.peering_config : {
       name           = "${var.environment}-${var.location_short}-${var.name}-${peering_config.name}"

diff --git a/modules/azure/core/subnets.tf b/modules/azure/core/subnets.tf
@@ -5,14 +5,13 @@ resource "azurerm_subnet" "this" {
     if subnet.subnet_aks_subnet == false
   }
 
-  name                                           = each.value.subnet_full_name
-  resource_group_name                            = data.azurerm_resource_group.this.name
-  virtual_network_name                           = azurerm_virtual_network.this.name
-  address_prefixes                               = [each.value.subnet_cidr]
-  service_endpoints                              = each.value.subnet_service_endpoints
-  enforce_private_link_endpoint_network_policies = local.subnet_private_endpoints[each.value.subnet_short_name]
-  enforce_private_link_service_network_policies  = local.subnet_private_endpoints[each.value.subnet_short_name]
-  private_endpoint_network_policies_enabled      = each.value.subnet_private_endpoint_network_policies_enabled
+  name                                          = each.value.subnet_full_name
+  resource_group_name                           = data.azurerm_resource_group.this.name
+  virtual_network_name                          = azurerm_virtual_network.this.name
+  address_prefixes                              = [each.value.subnet_cidr]
+  service_endpoints                             = each.value.subnet_service_endpoints
+  private_link_service_network_policies_enabled = true
+  private_endpoint_network_policies_enabled     = true
 }
 
 resource "azurerm_subnet" "aks" {
@@ -22,11 +21,11 @@ resource "azurerm_subnet" "aks" {
     if subnet.subnet_aks_subnet == true
   }
 
-  name                                           = each.value.subnet_full_name
-  resource_group_name                            = data.azurerm_resource_group.this.name
-  virtual_network_name                           = azurerm_virtual_network.this.name
-  address_prefixes                               = [each.value.subnet_cidr]
-  service_endpoints                              = each.value.subnet_service_endpoints
-  enforce_private_link_endpoint_network_policies = local.subnet_private_endpoints[each.value.subnet_short_name]
-  enforce_private_link_service_network_policies  = local.subnet_private_endpoints[each.value.subnet_short_name]
+  name                                          = each.value.subnet_full_name
+  resource_group_name                           = data.azurerm_resource_group.this.name
+  virtual_network_name                          = azurerm_virtual_network.this.name
+  address_prefixes                              = [each.value.subnet_cidr]
+  service_endpoints                             = each.value.subnet_service_endpoints
+  private_link_service_network_policies_enabled = true
+  private_endpoint_network_policies_enabled     = true
 }
diff --git a/modules/azure/core/variables.tf b/modules/azure/core/variables.tf
@@ -24,30 +24,19 @@ variable "vnet_config" {
     address_space = list(string)
     dns_servers   = list(string)
     subnets = list(object({
-      name                                      = string
-      cidr                                      = string
-      service_endpoints                         = list(string)
-      aks_subnet                                = bool
-      private_endpoint_network_policies_enabled = optional(bool, true)
+      name              = string
+      cidr              = string
+      service_endpoints = list(string)
+      aks_subnet        = bool
     }))
   })
 }
 
-# The following would enable private endpoint on only the "servers" subnet:
-# subnet_private_endpoints = {
-#   servers = true
-# }
-variable "subnet_private_endpoints" {
-  description = "Enable private enpoint for specific subnet names"
-  type        = map(bool)
-  default     = {}
-}
-
 variable "route_config" {
   description = "Route configuration. Not applied to AKS subnets"
   type = list(object({
     subnet_name                   = string                # Short name for the subnet
-    disable_bgp_route_propagation = optional(bool, false) # Controls propagation of routes learned by BGP on that route table 
+    disable_bgp_route_propagation = optional(bool, false) # Controls propagation of routes learned by BGP on that route table
     routes = list(object({
       name                   = string # Name of the route
       address_prefix         = string # Example: 192.168.0.0/24
@@ -67,7 +56,7 @@ variable "peering_config" {
     allow_forwarded_traffic      = bool
     use_remote_gateways          = bool
     allow_virtual_network_access = bool
-    allow_gateway_transit        = optional(bool, false) # Controls gatewayLinks can be used in the remote virtual network’s link to the local virtual network. 
+    allow_gateway_transit        = optional(bool, false) # Controls gatewayLinks can be used in the remote virtual network’s link to the local virtual network.
   }))
   default = []
 }