Closed davidkarlsen closed 2 years ago
Thank you for logging this question @davidkarlsen... and great work on the advanced configuration!!
Unfortunately, the issue above is something which will be difficult to resolve in the module without a complete change to the naming convention used for our resources. This is something which is in consideration but will need to wait for a significant re-design of the module as it will require resource migrations within the Terraform state to avoid a complete re-deployment of critical resources (something which Hashicorp are in the process of making easier in the latest releases of Terraform, but isn't ready for us to rely on).
For now, I think the best way to address this is to remove the following resource reference and replace it with a literal value, as I believe this is the item causing the problem:
firewall_policy_id = azurerm_firewall_policy.firewall_policy.id
I appreciate this isn't ideal, but until we can address #233 this is an unfortunately limitation of our current design.
You can also find more details relating to your scenario within issue #214 where another customer came across this challenge.
@krowlandson that's not it, doing that change will still give:
Error: Invalid for_each argument β β on .terraform/modules/caf-enterprise-scale/resources.connectivity.tf line 2, in resource "azurerm_resource_group" "connectivity": β 2: for_each = local.azurerm_resource_group_connectivity β βββββββββββββββββ β β local.azurerm_resource_group_connectivity will be known only after apply β β The "for_each" value depends on resource attributes that cannot be β determined until apply, so Terraform cannot predict how many instances will β be created. To work around this, use the -target argument to first apply β only the resources that the for_each depends on. β΅ Releasing state lock. This may take a few moments...
Interesting, thank you for the update @davidkarlsen.
It's possible it may also be due to the resource references within each of the IP configurations π
Most likely this one:
public_ip_address_id = azurerm_public_ip.vpn_gw[0].id
but also possibly this one:
subnet_id = data.azurerm_subnet.gateway_subnet.id
As we've already discussed with you in #232, we are planning to add native support for active-active configuration by just setting active_active = true
in release v1.2.0
which should remove the need to create a custom IP configuration and partially mitigate this.
@krowlandson
No, I don't think so, because all of that is working, it's first when I add:
git diff main
diff --git a/main.tf b/main.tf
index b237b40..4a1bcba 100644
--- a/main.tf
+++ b/main.tf
@@ -47,6 +47,13 @@ provider "azurerm" {
data "azurerm_client_config" "core" {}
+resource "azurerm_user_assigned_identity" "vpn_client_gw" {
+ location = var.default_location
+ name = "isfss-baas-dev-vpngw-client"
+ #resource_group_name = local.comms_resource_group_name
+ resource_group_name = "es-connectivity-norwayeast"
+}
+
module "caf-enterprise-scale" {
source = "Azure/caf-enterprise-scale/azurerm"
version = "1.1.2"
@@ -97,6 +104,14 @@ module "caf-enterprise-scale" {
subnet_id = data.azurerm_subnet.gateway_subnet.id
}
]
+ vpn_client_configuration = {
+ address_space = ["192.168.168.0/24"]
+ vpn_auth_types = ["AAD"]
+ vpn_client_protocols = ["OpenVPN"]
+ aad_issuer = "https://sts.windows.net/${data.azurerm_client_config.core.tenant_id}/"
+ aad_audience = azurerm_user_assigned_identity.vpn_client_gw.client_id
+ aad_tenant = "https://login.microsoftonline.com/${data.azurerm_client_config.core.tenant_id}"
+ }
}
}
}
that it fails.
Have you ever tested the module doing S2S and P2S at the same time?
As we've already discussed with you in #232, we are planning to add native support for active-active configuration by just setting
active_active = true
in releasev1.2.0
which should remove the need to create a custom IP configuration and partially mitigate this.
But please leave the option to pass an ip_configuration, that means we can allocate the IPs outside of the module and keep them fixed, even if the VPN-gw would be re-created.
But please leave the option to pass an ip_configuration, that means we can allocate the IPs outside of the module and keep them fixed, even if the VPN-gw would be re-created.
Custom provided values will always take precedence over default config settings, so this won't be a problem π
Have you ever tested the module doing S2S and P2S at the same time?
In short, no. But if you have narrowed it down to that specific change, I would see whether the following line is causing the issue:
aad_audience = azurerm_user_assigned_identity.vpn_client_gw.client_id
As per the suggestions above, we have a number of issues where parsing in values which cannot be determined until after apply can cause the error you've observed.
This is one of the factors we consider when recommending to decompose your deployment into multiple stages so you're inputs to the module are already known.
We now have the variables to configure this surfaced in module version 2.0.0. @davidkarlsen please can you confirm if this is still an issue?
As we've already discussed with you in #232, we are planning to add native support for active-active configuration by just setting
active_active = true
in releasev1.2.0
which should remove the need to create a custom IP configuration and partially mitigate this.But please leave the option to pass an ip_configuration, that means we can allocate the IPs outside of the module and keep them fixed, even if the VPN-gw would be re-created.
Note that we have left the option to specify a custom ip_configuration
as requested:
We have also used the presence of this value as a control to determine whether to create azurerm_public_ip
resource(s) for the associated resource, as shown here:
As an FYI, I don't believe any changes we made in this release will make any difference to the original error you were getting for depends on resource attributes that cannot be determined until apply
and I'm not sure it would be possible either with the current module architecture. This is something I hope to look at in the future though.
We now have the variables to configure this surfaced in module version 2.0.0. @davidkarlsen please can you confirm if this is still an issue?
I have a WIP branch, but I need to sort out some errors before we can merge it. I hope to do so over the coming weeks. I'll get back to you.
Thanks for the release!
Anything we can help with @davidkarlsen?
Hopefully the upgrade guide covers everything but good to know if we missed anything.
Picking up this again. I've added some of the params that are now required:
module "caf-enterprise-scale" {
source = "Azure/caf-enterprise-scale/azurerm"
- version = "1.1.4"
+ version = "2.0.0"
providers = {
azurerm = azurerm
azurerm.connectivity = azurerm
@@ -166,8 +166,11 @@ module "caf-enterprise-scale" {
azure_firewall = {
enabled = true
config = {
- address_prefix = var.azure_firewall_range
- enable_dns_proxy = true
+ address_prefix = var.azure_firewall_range
+ sku_tier = "Standard"
+ threat_intelligence_mode = "Deny"
+ base_policy_id = azurerm_firewall_policy.firewall_policy.id
+ enable_dns_proxy = true
availability_zones = {
zone_1 = true
zone_2 = true
and now end up with a shorter list of:
The given value is not suitable for child module variable
β "configure_connectivity_resources" defined at
β .terraform/modules/caf-enterprise-scale/variables.tf:203,1-44: attribute
β "settings": attribute "hub_networks": element 0: attribute "config":
β attribute "azure_firewall": attribute "config": attributes "dns_servers",
β "private_ip_ranges", and "threat_intelligence_allowlist" are required.
β΅
Seems dns-servers needs to be explicitly listed now, I can't use "Default" ? Unsure about private_ip_ranges and threat_intelligence_allowlist
@krowlandson sorry, i see that is actually covered in the migration guide, closing the gaps now.
No worries @davidkarlsen... Please let us know if anything is missing!
Moving parameters around and reading migration guide, but it's still a bit unclear to me where I should place ip_configuration
for my VNG:
# module.caf-enterprise-scale.azurerm_virtual_network_gateway.connectivity["/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/virtualNetworkGateways/es-vpngw-norwayeast"] will be updated in-place
~ resource "azurerm_virtual_network_gateway" "connectivity" {
id = "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/virtualNetworkGateways/es-vpngw-norwayeast"
name = "es-vpngw-norwayeast"
~ tags = {
~ "deployedBy" = "terraform/azure/caf-enterprise-scale/v1.1.4" -> "terraform/azure/caf-enterprise-scale/v2.0.0"
# (4 unchanged elements hidden)
}
# (9 unchanged attributes hidden)
~ ip_configuration {
~ name = "vnetGatewayConfig1" -> "es-vpngw-norwayeast-pip"
~ public_ip_address_id = "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/publicIPAddresses/VpnGw0" -> "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/publicIPAddresses/es-vpngw-norwayeast-pip"
# (2 unchanged attributes hidden)
}
~ ip_configuration {
~ name = "vnetGatewayConfig2" -> "es-vpngw-norwayeast-pip2"
~ public_ip_address_id = "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/publicIPAddresses/VpnGw1" -> "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/publicIPAddresses/es-vpngw-norwayeast-pip2"
# (2 unchanged attributes hidden)
}
- ip_configuration {
- name = "vnetGatewayConfig3" -> null
- private_ip_address_allocation = "Dynamic" -> null
- public_ip_address_id = "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/publicIPAddresses/VpnGw2" -> null
- subnet_id = "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/virtualNetworks/es-hub-norwayeast/subnets/GatewaySubnet" -> null
}
I can't find any place for it in https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/blob/246cf7de44369b2a40a1b07fafdd8b8b792e7c35/tests/modules/settings/settings.connectivity.tf#L17
π¦ Confused myself - I notice the settings are now spread over two different sections, and the "old" section is now also renamed - made it all a bit hard to follow (what is the logic behind this) - but I am closing in on the diffs now.
OK, having applied it all, it now fails at:
Error: Creating/Updating Virtual Network Gateway: (Name "es-vpngw-norwayeast" / Resource Group "es-connectivity-norwayeast"): network.VirtualNetworkGatewaysClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="VirtualNetworkGatewayBgpPeeringAddressCannotBeModified" Message="The BgpPeeringAddress for the virtual network gateway /subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/virtualNetworkGateways/es-vpngw-norwayeast cannot be modified" Details=[]
how can I preserve my bgp setup? My diffs is:
diff --git a/main.tf b/main.tf
index d777433..e105380 100644
--- a/main.tf
+++ b/main.tf
@@ -8,7 +8,7 @@ terraform {
required_providers {
azurerm = {
source = "hashicorp/azurerm"
- version = "~> 2.99.0"
+ version = "~> 3.0.2"
}
cloudflare = {
source = "cloudflare/cloudflare"
@@ -56,7 +56,7 @@ data "azurerm_client_config" "core" {}
module "caf-enterprise-scale" {
source = "Azure/caf-enterprise-scale/azurerm"
- version = "1.1.4"
+ version = "2.0.1"
providers = {
azurerm = azurerm
azurerm.connectivity = azurerm
@@ -81,44 +81,28 @@ module "caf-enterprise-scale" {
}
}
azurerm_virtual_network_gateway = {
- connectivity = {
- vpngw = {
- (var.default_location) = {
- enable_bgp = true
- active_active = true
- bgp_settings = [{
- asn = var.az_bgp_asn
- peering_addresses = []
- }]
- ip_configuration = [
- {
- name = "vnetGatewayConfig1"
- public_ip_address_id = azurerm_public_ip.vpn_gw[0].id
- private_ip_address_allocation = "Dynamic"
- subnet_id = data.azurerm_subnet.gateway_subnet.id
- },
- {
- name = "vnetGatewayConfig2"
- public_ip_address_id = azurerm_public_ip.vpn_gw[1].id
- private_ip_address_allocation = "Dynamic"
- subnet_id = data.azurerm_subnet.gateway_subnet.id
- },
- {
- name = "vnetGatewayConfig3"
- public_ip_address_id = azurerm_public_ip.vpn_gw[2].id
- private_ip_address_allocation = "Dynamic"
- subnet_id = data.azurerm_subnet.gateway_subnet.id
- }
- ]
- vpn_client_configuration = [{
- address_space = [var.client_vpn_ipaddress_range]
- vpn_auth_types = ["AAD"]
- vpn_client_protocols = ["OpenVPN"]
- aad_issuer = "XXX
- aad_audience = var.client_vpn_audience
- aad_tenant = "XXXX"
- }]
- }
+ connectivity_vpn = {
+ (var.default_location) = {
+ ip_configuration = [
+ {
+ name = "vnetGatewayConfig1"
+ public_ip_address_id = azurerm_public_ip.vpn_gw[0].id
+ private_ip_address_allocation = "Dynamic"
+ subnet_id = data.azurerm_subnet.gateway_subnet.id
+ },
+ {
+ name = "vnetGatewayConfig2"
+ public_ip_address_id = azurerm_public_ip.vpn_gw[1].id
+ private_ip_address_allocation = "Dynamic"
+ subnet_id = data.azurerm_subnet.gateway_subnet.id
+ },
+ {
+ name = "vnetGatewayConfig3"
+ public_ip_address_id = azurerm_public_ip.vpn_gw[2].id
+ private_ip_address_allocation = "Dynamic"
+ subnet_id = data.azurerm_subnet.gateway_subnet.id
+ }
+ ]
}
}
}
@@ -161,13 +145,43 @@ module "caf-enterprise-scale" {
address_prefix = var.virtual_network_gateway_range
gateway_sku_expressroute = ""
gateway_sku_vpn = var.vpn_gw_sku
+ advanced_vpn_settings = {
+ enable_bgp = true
+ active_active = true
+ private_ip_address_allocation = ""
+ default_local_network_gateway_id = ""
+ bgp_settings = [{
+ asn = var.az_bgp_asn
+ peer_weight = 32768
+ peering_addresses = []
+ }]
+ vpn_client_configuration = [{
+ address_space = [var.client_vpn_ipaddress_range]
+ vpn_auth_types = ["AAD"]
+ vpn_client_protocols = ["OpenVPN"]
+ aad_issuer = "XXX"
+ aad_audience = var.client_vpn_audience
+ aad_tenant = "XXX"
+ radius_server_address = null
+ radius_server_secret = null
+ revoked_certificate = []
+ root_certificate = []
+ }]
+ custom_route = []
+ }
}
}
azure_firewall = {
enabled = true
config = {
- address_prefix = var.azure_firewall_range
- enable_dns_proxy = true
+ address_prefix = var.azure_firewall_range
+ sku_tier = "Standard"
+ private_ip_ranges = []
+ threat_intelligence_mode = "Deny"
+ threat_intelligence_allowlist = []
+ base_policy_id = azurerm_firewall_policy.firewall_policy.id
+ dns_servers = []
+ enable_dns_proxy = true
availability_zones = {
zone_1 = true
zone_2 = true
Hi @davidkarlsen
I'm back in the office on Monday. I'll have a proper look at this then if that's OK although it sounds like something in your configuration is still resulting in an unexpected change so it might be helpful to have a copy of your full before and after config if that's possible? Could you share via a private repo using branches for each?
@krowlandson I can share most of it privately, although the diff above should show it. What's really strange too, is that the vpn gw shouldn't really change according to the plan:
# module.caf-enterprise-scale.azurerm_virtual_network_gateway.connectivity["/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/virtualNetworkGateways/es-vpngw-norwayeast"] will be updated in-place
~ resource "azurerm_virtual_network_gateway" "connectivity" {
id = "/subscriptions/***/resourceGroups/es-connectivity-norwayeast/providers/Microsoft.Network/virtualNetworkGateways/es-vpngw-norwayeast"
name = "es-vpngw-norwayeast"
~ tags = {
~ "deployedBy" = "terraform/azure/caf-enterprise-scale/v1.1.4" -> "terraform/azure/caf-enterprise-scale/v2.0.1"
# (4 unchanged elements hidden)
}
# (9 unchanged attributes hidden)
# (5 unchanged blocks hidden)
}
This code https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/blob/246cf7de44369b2a40a1b07fafdd8b8b792e7c35/resources.connectivity.tf#L216 does not seem to match what you can give as input: https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/blob/main/variables.tf#L263
@krowlandson any update here?
This code
does not seem to match what you can give as input: https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/blob/main/variables.tf#L263
Thank you for this... it seems we didn't clean this one up correctly. I'll get a PR raised to fix this.
I'm also taking a look at the rest of this issue today.
@davidkarlsen, as discussed offline we will close this issue as the original ask is now resolved.
Regarding the error you experienced on update, we have been unable to reproduce this behaviour but believe this to be an external issue. As you were able to resolve this by ensuring that the target resource configuration (tags) matched those in code (thereby negating the need to update the resource), we believe this may be either the AzureRM Provider, the SDK, or something on the Azure resource provider side causing this. If anyone else experiences this, we'll continue investigation as necessary.
Thank you for your support and patience working through this.
Community Note
Versions
terraform: 1.1.4
azure provider: "~> 2.92.0"
module: 1.1.1
Description
I have the module running with site-to-site vpn, but altered it to also add config for
vpn_client_configuration
, it then fails on:Steps to Reproduce
Screenshots
Additional context
config: