From 494a7a475822e1582b13e58f147ae527df24b268 Mon Sep 17 00:00:00 2001 From: Edvin N Date: Wed, 8 Mar 2023 17:16:15 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 4 ++++ modules/azure/core/README.md | 7 +++---- modules/azure/core/locals.tf | 18 ++++++------------ modules/azure/core/subnets.tf | 29 ++++++++++++++--------------- modules/azure/core/variables.tf | 23 ++++++----------------- 5 files changed, 33 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78614e3ec..cb543f06d 100644 --- 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 index 357ceb9ec..1afff0d1c 100644 --- a/modules/azure/core/README.md +++ b/modules/azure/core/README.md @@ -60,12 +60,11 @@ No modules. | [location\_short](#input\_location\_short) | The Azure region short name | `string` | n/a | yes | | [name](#input\_name) | The commonName to use for the deploy | `string` | n/a | yes | | [notification\_email](#input\_notification\_email) | Email address to send alert notifications | `string` | n/a | yes | -| [peering\_config](#input\_peering\_config) | Network peering configuration |
list(object({
name = string
remote_virtual_network_id = string
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.
}))
| `[]` | no | -| [route\_config](#input\_route\_config) | Route configuration. Not applied to AKS subnets |
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
routes = list(object({
name = string # Name of the route
address_prefix = string # Example: 192.168.0.0/24
next_hop_type = string # VirtualNetworkGateway, VnetLocal, Internet, VirtualAppliance and None
next_hop_in_ip_address = string # Only set if next_hop_type is VirtualAppliance
}))

}))
| `[]` | no | -| [subnet\_private\_endpoints](#input\_subnet\_private\_endpoints) | Enable private enpoint for specific subnet names | `map(bool)` | `{}` | no | +| [peering\_config](#input\_peering\_config) | Network peering configuration |
list(object({
name = string
remote_virtual_network_id = string
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.
}))
| `[]` | no | +| [route\_config](#input\_route\_config) | Route configuration. Not applied to AKS subnets |
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
routes = list(object({
name = string # Name of the route
address_prefix = string # Example: 192.168.0.0/24
next_hop_type = string # VirtualNetworkGateway, VnetLocal, Internet, VirtualAppliance and None
next_hop_in_ip_address = string # Only set if next_hop_type is VirtualAppliance
}))

}))
| `[]` | no | | [subscription\_name](#input\_subscription\_name) | The subscription commonName to use for the deploy | `string` | n/a | yes | | [unique\_suffix](#input\_unique\_suffix) | Unique suffix that is used in globally unique resources names | `string` | n/a | yes | -| [vnet\_config](#input\_vnet\_config) | Address spaces used by virtual network |
object({
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)
}))
})
| n/a | yes | +| [vnet\_config](#input\_vnet\_config) | Address spaces used by virtual network |
object({
address_space = list(string)
dns_servers = list(string)
subnets = list(object({
name = string
cidr = string
service_endpoints = list(string)
aks_subnet = bool
}))
})
| n/a | yes | ## Outputs diff --git a/modules/azure/core/locals.tf b/modules/azure/core/locals.tf index c2946687e..a65c03583 100644 --- 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 index ae31c42fb..23a10868e 100644 --- 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 index c4e4c5a02..ef876cb62 100644 --- 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 = [] }