search

Azure / terraform-azurerm-avm-ptn-network-private-link-private-dns-zones

The AVM pattern module to deploy all the required private link private DNS zones for supported services.
https://registry.terraform.io/modules/Azure/avm-ptn-network-private-link-private-dns-zones/azurerm/latest
MIT License
8 stars 5 forks source link

[AVM Module Issue]: Remove resource group data source #40

Closed jaredfholgate closed 1 week ago

jaredfholgate commented 1 month ago

Check for previous/existing GitHub issues

Issue Type?

I'm not sure

(Optional) Module Version

latest

(Optional) Correlation Id

No response

Description

Remove the data source for the resource group: https://github.com/Azure/terraform-azurerm-avm-ptn-network-private-link-private-dns-zones/blob/3acbfd844ec3e3e735da9bb15d8ec3f25b1c9192/main.tf#L9

I have seen an edge case where the data source is read even though it is dependent on an input that is unknown causing an error as the resource group does not exist yet.

I appreciate this is down to the consumer, but the data source can be removed without impacting the module and would remove the need to add explicit dependencies in this scenario.

Example:

│ Error: Error: Resource Group "rg-dns-uksouth" was not found
│
│   with module.private_dns_zones["ukwest"].data.azurerm_resource_group.this[0],
│   on .terraform\modules\private_dns_zones\main.tf line 9, in data "azurerm_resource_group" "this":
│    9: data "azurerm_resource_group" "this" {
│
╵
╷
│ Error: Error: Resource Group "rg-dns-uksouth" was not found
│
│   with module.private_dns_zones["uksouth"].data.azurerm_resource_group.this[0],
│   on .terraform\modules\private_dns_zones\main.tf line 9, in data "azurerm_resource_group" "this":
│    9: data "azurerm_resource_group" "this" {
module "private_dns_zones_resource_group" {
  source  = "Azure/avm-res-resources-resourcegroup/azurerm"
  version = "0.1.0"

  count = local.private_dns_enabled ? 1 : 0

  name = try(local.module_private_dns.resource_group_name, "rg-dns-${var.starter_locations[0]}")
  location = try(local.module_private_dns.location, var.starter_locations[0])

  providers = {
    azurerm = azurerm.connectivity
  }
}

module "private_dns_zones" {
  source  = "Azure/avm-ptn-network-private-link-private-dns-zones/azurerm"
  version = "0.4.0"

  for_each = local.private_dns_location_map

  location = each.key
  resource_group_name = module.private_dns_zones_resource_group[0].name
  resource_group_creation_enabled = false
  virtual_network_resource_ids_to_link_to = local.private_dns_virtual_networks
  private_link_private_dns_zones = each.value.is_primary ? null : local.private_dns_secondary_zones

  # depends_on = [ module.private_dns_zones_resource_group ]  # NEED this for it to work.

  providers = {
    azurerm = azurerm.connectivity
  }
}
jtracey93 commented 1 month ago

Thanks @jaredfholgate, is this not needed due to this line

https://github.com/Azure/terraform-azurerm-avm-ptn-network-private-link-private-dns-zones/blob/3acbfd844ec3e3e735da9bb15d8ec3f25b1c9192/main.tf#L50

jtracey93 commented 1 month ago

RR

matt-FFFFFF commented 1 month ago

In general data sources are a bad idea. Instead accept a resource id for an existing RG (string, default null), or create one.

I am also going to remove this from alz mgmt

matt-FFFFFF commented 1 month ago

Ah, as you are using azurerm the client config data source is the best approach as you've done in #40