Azure / terraform-azurerm-lz-vending

Terraform module to deploy landing zone subscriptions (and much more) in Azure
https://registry.terraform.io/modules/Azure/lz-vending/azurerm
MIT License
162 stars 73 forks source link

bug: vNet peering gateway transit is backward #255

Closed WibblyDibbler closed 11 months ago

WibblyDibbler commented 12 months ago

Community Note

Versions

Please paste the output of terraform version command from within the initialized directory:

1.5.0

Please enter the module version that you are using:

3.3.0

Description

The azapi_resource references for peering_hub_outbound and peering_hub_inbound do not allow configuration of allowGatewayTransit. Oddly, peering_hub_outbound is hard-coded to false, and peering_hub_inbound is hard-coded to true. A typical hub-and-spoke network should have those swapped, i.e. the spoke should be able to allow gateway transit from networks not in the hub - not the other way around. Either way, let the module consumers decide which is allowed when in the event they have/want some non-standard network topology.

Steps to Reproduce

  1. Set the lz_vending module's virtual_network_enabled = true and provide values for virtual_networks
  2. There is nowhere to propagate configuration to allowGatewayTransit

Additional context

The terraform plan plan for module 3.4.1 shows no changes relative to this, and the module source indicates no support as of commit ID d17ad87e64fa81c0c7c068cde0fa575b4d4a3fef.

matt-FFFFFF commented 11 months ago

Hi @WibblyDibbler

Thanks for reporting. We are looking into this

jtracey93 commented 11 months ago

Hi @WibblyDibbler,

From my investigation into this, I believe the way the peerings are configured are correct as they are.

You are certainly correct about not being able to toggle allowGatewayTransit today in the module however, you can toggle hub_peering_use_remote_gateways to control whether the spoke although I think I may of spotted a bug there.

Just doing some testing but initially this doesn't look to be incorrect.

cc: @matt-FFFFFF

jtracey93 commented 11 months ago

Forgot to add this is the same as Bicep Sub Vending:

https://github.com/Azure/bicep-lz-vending/blob/main/src/self/subResourceWrapper/deploy.bicep#L194-L206

jtracey93 commented 11 months ago

Okay so did some testing and I think how it is today is correct, see below for why.

image

image

image

image

Source: https://learn.microsoft.com/en-us/azure/templates/microsoft.network/virtualnetworks/virtualnetworkpeerings?pivots=deployment-language-bicep

WibblyDibbler commented 11 months ago

I opened this issue because the peerings as displayed in the Azure Portal showed otherwise. Unfortunately, I did not take a screenshot, and the language appears to have changed - I have no idea how to verify that. Either way, this all looks good. Sorry for the run-around.