fix(virtual_network_gateway): Fix several issues with VNG module (#117)

modules/virtual_network_gateway/README.md

@@ -339,10 +339,10 @@ Name | Type | Description
 [`zones`](#zones) | `list` | After provider version 3.
 [`edge_zone`](#edge_zone) | `string` | Specifies the Edge Zone within the Azure Region where this Virtual Network Gateway should exist.
 [`private_ip_address_enabled`](#private_ip_address_enabled) | `bool` | Controls whether the private IP is enabled on the Virtual Netowkr Gateway.
-[`default_local_network_gateway_id`](#default_local_network_gateway_id) | `string` | The ID of the local network gateway.
+[`default_local_network_gateway_id`](#default_local_network_gateway_id) | `string` | The ID of the Local Network Gateway.
 [`azure_bgp_peer_addresses`](#azure_bgp_peer_addresses) | `map` | Map of IP addresses used on Azure side for BGP.
 [`bgp`](#bgp) | `object` | A map controlling the BGP configuration used by this Virtual Network Gateway.
-[`local_network_gateways`](#local_network_gateways) | `map` | Map of local network gateways and their connections.
+[`local_network_gateways`](#local_network_gateways) | `map` | Map of Local Network Gateways and their connections.
 [`vpn_clients`](#vpn_clients) | `map` | VPN client configurations (IPSec point-to-site connections).
 
 ### Outputs
@@ -438,10 +438,10 @@ Following properties are available:
                     or `ExpressRoute`.
 - `vpn_type`      - (`string`, optional, defaults to `RouteBased`) the routing type of the Virtual Network Gateway, possible
                     values are: `RouteBased` or `PolicyBased`.
-- `generation`    - (`string`, optional, defaults to `Generation1`) the Generation of the Virtual Network gateway, possible
+- `generation`    - (`string`, optional, defaults to `Generation1`) the Generation of the Virtual Network Gateway, possible
                     values are: `None`, `Generation1` or `Generation2`. This property is ignored when type is set to 
                     `ExpressRoute`.
-- `sku`           - (`string`, optional, defaults to `Basic`) sets the size and capacity of the virtual network gateway.
+- `sku`           - (`string`, optional, defaults to `Basic`) sets the size and capacity of the Virtual Network Gateway.
 - `active_active` - (`bool`, optional, defaults to `false`) when set to true creates an active-active Virtual Network Gateway,
                     active-passive otherwise. Not supported for `Basic` and `Standard` SKUs.
 
@@ -558,10 +558,10 @@ Default value: `false`
 
 #### default_local_network_gateway_id
 
-The ID of the local network gateway.
+The ID of the Local Network Gateway.
 
-When set, the outbound Internet traffic from the virtual network, in which the gateway is created, will be routed through local
-network gateway (forced tunnelling).
+When set, the outbound Internet traffic from the Virtual Network, in which the gateway is created, will be routed through Local
+Network Gateway (forced tunnelling).
 
 
 Type: string
@@ -647,11 +647,11 @@ Default value: `&{}`
 
 #### local_network_gateways
 
-Map of local network gateways and their connections.
+Map of Local Network Gateways and their connections.
 
 Every object in the map contains following attributes:
 
-- `name`                 - (`string`, required) the name of the local network gateway.
+- `name`                 - (`string`, required) the name of the Local Network Gateway.
 - `remote_bgp_settings`  - (`list`, optional, defaults to `[]`) block containing Local Network Gateway's BGP speaker settings:
   - `asn`                 - (`string`, required) the BGP speaker's ASN.
   - `bgp_peering_address` - (`string`, required) the BGP peering address and BGP identifier of this BGP speaker.
@@ -667,7 +667,7 @@ Every object in the map contains following attributes:
                       azurerm_virtual_network_gateway ip_configuration (second one)
 - `connection`           - (`map`, required) a map defining configuration for a VPN connection between Azure VNG and on-premises
                            VPN device. Contains the following properties:
-  - `name`            - (`string`, required) the name of the virtual network gateway connection.
+  - `name`            - (`string`, required) the name of the Virtual Network Gateway connection.
   - `ipsec_policies`  - (`list`, required) list of IPsec policies used for Virtual Network Connection. A single policy consist
                         of the following properties:
     - `dh_group`         - (`string`, required) the DH group used in IKE phase 1 for initial SA.
@@ -753,7 +753,7 @@ Following properties are available:
                             The supported values are SSTP, IkeV2 and OpenVPN. Values SSTP and IkeV2 are incompatible with
                             the use of aad_tenant, aad_audience and aad_issuer.
 - `vpn_auth_types`        - (`list(string)`, optional, defaults to `null`) list of the vpn authentication types for
-                            the virtual network gateway. The supported values are AAD, Radius and Certificate.
+                            the Virtual Network Gateway. The supported values are AAD, Radius and Certificate.
 - `custom_routes`         - (`map`, optional, defaults to `{}`) a map defining custom routes. Each route is a list of address
                             blocks reserved for this Virtual Network (in CIDR notation). Keys in this map are only to identify
                             the CIDR blocks, values are lists of the actual address blocks.

modules/virtual_network_gateway/main.tf

@@ -1,6 +1,6 @@
 # https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/public_ip
 resource "azurerm_public_ip" "this" {
-  for_each = { for k, v in var.ip_configurations : k => v if try(v.create_public_ip, false) }
+  for_each = { for k, v in var.ip_configurations : k => v if v.create_public_ip }
 
   resource_group_name = var.resource_group_name
   location            = var.region
@@ -15,7 +15,7 @@ resource "azurerm_public_ip" "this" {
 
 # https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/data-sources/public_ip
 data "azurerm_public_ip" "this" {
-  for_each = { for k, v in var.ip_configurations : k => v if !try(v.create_public_ip, true) }
+  for_each = { for k, v in var.ip_configurations : k => v if !v.create_public_ip && v.public_ip_name != null }
 
   name                = each.value.public_ip_name
   resource_group_name = coalesce(each.value.public_ip_resource_group_name, var.resource_group_name)
@@ -30,7 +30,7 @@ resource "azurerm_virtual_network_gateway" "this" {
   type                             = var.instance_settings.type
   vpn_type                         = var.instance_settings.vpn_type
   sku                              = var.instance_settings.sku
-  generation                       = var.instance_settings.type == "VPN" ? var.instance_settings.generation : null
+  generation                       = var.instance_settings.type == "Vpn" ? var.instance_settings.generation : null
   active_active                    = var.instance_settings.active_active
   default_local_network_gateway_id = var.default_local_network_gateway_id
   edge_zone                        = var.edge_zone
@@ -43,7 +43,7 @@ resource "azurerm_virtual_network_gateway" "this" {
       name = ip_configuration.value.name
       public_ip_address_id = coalesce(
         ip_configuration.value.public_ip_id,
-        try(azurerm_public_ip.this[ip_configuration.value.name].id, data.azurerm_public_ip.this[ip_configuration.value.name].id)
+        try(azurerm_public_ip.this[ip_configuration.value.name].id, data.azurerm_public_ip.this[ip_configuration.value.name].id, null)
       )
       private_ip_address_allocation = ip_configuration.value.private_ip_address_allocation
       subnet_id                     = var.subnet_id
@@ -59,19 +59,21 @@ resource "azurerm_virtual_network_gateway" "this" {
 
       peering_addresses {
         ip_configuration_name = var.bgp.configuration.primary_peering_addresses.name
-        apipa_addresses = [
-          for i in var.bgp.configuration.primary_peering_addresses.apipa_address_keys : var.azure_bgp_peer_addresses[i]
-        ]
+        apipa_addresses = try(
+          [for i in var.bgp.configuration.primary_peering_addresses.apipa_address_keys : var.azure_bgp_peer_addresses[i]],
+          null
+        )
         default_addresses = var.bgp.configuration.primary_peering_addresses.default_addresses
       }
 
       dynamic "peering_addresses" {
         for_each = var.bgp.configuration.secondary_peering_addresses != null ? [1] : []
         content {
           ip_configuration_name = var.bgp.configuration.secondary_peering_addresses.name
-          apipa_addresses = [
-            for i in var.bgp.configuration.secondary_peering_addresses.apipa_address_keys : var.azure_bgp_peer_addresses[i]
-          ]
+          apipa_addresses = try(
+            [for i in var.bgp.configuration.secondary_peering_addresses.apipa_address_keys : var.azure_bgp_peer_addresses[i]],
+            null
+          )
           default_addresses = var.bgp.configuration.secondary_peering_addresses.default_addresses
         }
       }

modules/virtual_network_gateway/variables.tf

@@ -100,10 +100,10 @@ variable "instance_settings" {
                       or `ExpressRoute`.
   - `vpn_type`      - (`string`, optional, defaults to `RouteBased`) the routing type of the Virtual Network Gateway, possible
                       values are: `RouteBased` or `PolicyBased`.
-  - `generation`    - (`string`, optional, defaults to `Generation1`) the Generation of the Virtual Network gateway, possible
+  - `generation`    - (`string`, optional, defaults to `Generation1`) the Generation of the Virtual Network Gateway, possible
                       values are: `None`, `Generation1` or `Generation2`. This property is ignored when type is set to 
                       `ExpressRoute`.
-  - `sku`           - (`string`, optional, defaults to `Basic`) sets the size and capacity of the virtual network gateway.
+  - `sku`           - (`string`, optional, defaults to `Basic`) sets the size and capacity of the Virtual Network Gateway.
   - `active_active` - (`bool`, optional, defaults to `false`) when set to true creates an active-active Virtual Network Gateway,
                       active-passive otherwise. Not supported for `Basic` and `Standard` SKUs.
   EOF
@@ -263,10 +263,10 @@ variable "private_ip_address_enabled" {
 
 variable "default_local_network_gateway_id" {
   description = <<-EOF
-  The ID of the local network gateway.
+  The ID of the Local Network Gateway.
 
-  When set, the outbound Internet traffic from the virtual network, in which the gateway is created, will be routed through local
-  network gateway (forced tunnelling).
+  When set, the outbound Internet traffic from the Virtual Network, in which the gateway is created, will be routed through Local
+  Network Gateway (forced tunnelling).
   EOF
   default     = null
   type        = string
@@ -366,11 +366,11 @@ variable "bgp" {
 
 variable "local_network_gateways" {
   description = <<-EOF
-  Map of local network gateways and their connections.
+  Map of Local Network Gateways and their connections.
 
   Every object in the map contains following attributes:
   
-  - `name`                 - (`string`, required) the name of the local network gateway.
+  - `name`                 - (`string`, required) the name of the Local Network Gateway.
   - `remote_bgp_settings`  - (`list`, optional, defaults to `[]`) block containing Local Network Gateway's BGP speaker settings:
     - `asn`                 - (`string`, required) the BGP speaker's ASN.
     - `bgp_peering_address` - (`string`, required) the BGP peering address and BGP identifier of this BGP speaker.
@@ -386,7 +386,7 @@ variable "local_network_gateways" {
                         azurerm_virtual_network_gateway ip_configuration (second one)
   - `connection`           - (`map`, required) a map defining configuration for a VPN connection between Azure VNG and on-premises
                              VPN device. Contains the following properties:
-    - `name`            - (`string`, required) the name of the virtual network gateway connection.
+    - `name`            - (`string`, required) the name of the Virtual Network Gateway connection.
     - `ipsec_policies`  - (`list`, required) list of IPsec policies used for Virtual Network Connection. A single policy consist
                           of the following properties:
       - `dh_group`         - (`string`, required) the DH group used in IKE phase 1 for initial SA.
@@ -560,7 +560,7 @@ variable "vpn_clients" {
                               The supported values are SSTP, IkeV2 and OpenVPN. Values SSTP and IkeV2 are incompatible with
                               the use of aad_tenant, aad_audience and aad_issuer.
   - `vpn_auth_types`        - (`list(string)`, optional, defaults to `null`) list of the vpn authentication types for
-                              the virtual network gateway. The supported values are AAD, Radius and Certificate.
+                              the Virtual Network Gateway. The supported values are AAD, Radius and Certificate.
   - `custom_routes`         - (`map`, optional, defaults to `{}`) a map defining custom routes. Each route is a list of address
                               blocks reserved for this Virtual Network (in CIDR notation). Keys in this map are only to identify
                               the CIDR blocks, values are lists of the actual address blocks.

tests/virtual_network_gateway/main.tf

@@ -88,11 +88,11 @@ module "vng" {
   instance_settings = each.value.instance_settings
   ip_configurations = {
     primary = merge(each.value.ip_configurations.primary, {
-      public_ip_key = try(module.public_ip.pip_ids[each.value.ip_configurations.primary.public_ip_key], null)
+      public_ip_id = try(module.public_ip.pip_ids[each.value.ip_configurations.primary.public_ip_key], null)
     })
     secondary = each.value.instance_settings.active_active == true ? merge(each.value.ip_configurations.secondary, {
-      name          = try(each.value.ip_configurations.secondary.name, null)
-      public_ip_key = try(module.public_ip.pip_ids[each.value.ip_configurations.secondary.public_ip_key], null)
+      name         = try(each.value.ip_configurations.secondary.name, null)
+      public_ip_id = try(module.public_ip.pip_ids[each.value.ip_configurations.secondary.public_ip_key], null)
     }) : null
   }
   private_ip_address_enabled       = each.value.private_ip_address_enabled