[TC-5] Add module for private dns zone with vnet links #15
Conversation
mitiko
left a comment
mitiko
left a comment
There was a problem hiding this comment.
Be more consistent. Be considerate that people will us the example as documentation until such is provided and the examples must contain ready-to-go working configuration.
| storage_account_blob = { | ||
| resource_kind = "storage_blob" | ||
| resource_group_ref = "rg_test" | ||
| vnet_ref = ["vnet_test", "vnet_test2"] |
There was a problem hiding this comment.
Rename to vnet_refs. Also the vnet object is not a child of the private dns zone. Rename this field to something that reflects the fact that we're linking with the vnets.
| @@ -0,0 +1,44 @@ | |||
| private_dns_zones = { | |||
| storage_account_blob = { | |||
There was a problem hiding this comment.
Rename to pdns_storage_account_blob
| cidr = ["10.1.0.0/16"] | ||
| subnets = { | ||
| snet_app_02 = { | ||
| name = "snet-app" |
There was a problem hiding this comment.
This example will not work if the subnets are named the same :)
| var.global_settings.inherit_resource_group_tags ? local.resource_group.tags : {}, | ||
| try(var.settings.tags, {}) | ||
| ) | ||
| vnet_ids = { |
There was a problem hiding this comment.
This object is called vnet_ids but it holds names and ids. Reconsider.
| locals { | ||
| # local object used to map possible private dns zoone names | ||
| zone_names = { | ||
| "storage_blob" = "privatelink.blob.core.windows.net" |
There was a problem hiding this comment.
storage_blob is the only name in this map that is not plural. Consistency is key. Reconsider.
| @@ -0,0 +1,5 @@ | |||
| resource "azurerm_private_dns_zone" "main" { | |||
| name = try(local.zone_names[var.settings.resource_kind], var.settings.name) | |||
There was a problem hiding this comment.
Experience shows that the name field is not used in practice. I recommend relying completely on the local object instead. We'll modify the module to be able to override the name, once such a case emerges.
| resource "azurerm_private_dns_zone" "main" { | ||
| name = try(local.zone_names[var.settings.resource_kind], var.settings.name) | ||
| resource_group_name = local.resource_group_name | ||
| tags = try(local.tags, null) |
There was a problem hiding this comment.
this will never fail. local.tags is enough.
| source = "./modules/_networking/private_dns_zone" | ||
| for_each = var.private_dns_zones | ||
|
|
||
| global_settings = var.global_settings |
There was a problem hiding this comment.
this must be local.global_settings for consistency with the rest of the modules
mitiko
changed the title
TC-5 Add module for private dns zone with vnet links