azurerm_firewall: supports dns_setting

[magodo]

azurerm_firewall supports dns_setting and azurerm_firewall_network_rule_collection supports destination_fqdns.

Test Result

💢 make testacc TEST=./azurerm/internal/services/network/tests TESTARGS='-run "TestAccAzureRMFirewallNetworkRuleCollection_fqdns|TestAccAzureRMFirewallNetworkRuleCollection_noDestination|TestAccAzureRMFirewall_enableDNS"'

==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test ./azurerm/internal/services/network/tests -v -run "TestAccAzureRMFirewallNetworkRuleCollection_fqdns|TestAccAzureRMFirewallNetworkRuleCollection_noDestination|TestAccAzureRMFirewall_enableDNS" -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMFirewallNetworkRuleCollection_fqdns
=== PAUSE TestAccAzureRMFirewallNetworkRuleCollection_fqdns
=== RUN   TestAccAzureRMFirewallNetworkRuleCollection_noDestination
=== PAUSE TestAccAzureRMFirewallNetworkRuleCollection_noDestination
=== RUN   TestAccAzureRMFirewall_enableDNS
=== PAUSE TestAccAzureRMFirewall_enableDNS
=== CONT  TestAccAzureRMFirewallNetworkRuleCollection_fqdns
=== CONT  TestAccAzureRMFirewall_enableDNS
=== CONT  TestAccAzureRMFirewallNetworkRuleCollection_noDestination
--- PASS: TestAccAzureRMFirewallNetworkRuleCollection_fqdns (2213.69s)
--- PASS: TestAccAzureRMFirewall_enableDNS (2222.99s)
--- PASS: TestAccAzureRMFirewallNetworkRuleCollection_noDestination (2234.54s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/tests       2234.580s

Fixes #8312, fixes #7743.

[tombuildsstuff]

hey @magodo

Thanks for this PR - I've taken a look through and left a few comments inline but if we can fix those up then this is otherwise looking good 👍

Thanks!

[tombuildsstuff]

Can we file a Swagger bug about these? Since this is a dictionary there's no guarantees there won't be breaking changes here

[magodo]

Sure, Azure/azure-rest-api-specs#11278

[katbyte]

could we link to this in the code?

[magodo]

Yes, I have added them in my last commit.

[tombuildsstuff]

presumably this is a String to handle the Tri-state (True, False, Unset) here - is this defaulted/planned to be defaulted going forward? In general we're removing "enabled" fields and using the presence/omission of the block to infer that - but that won't work for tri-state fields, can we reach out to the service team and confirm the intention here?

[magodo]

I will reply here later once I got the update from service team.

[katbyte]

any update @magodo ?

[magodo]

Just got the update that the absense of Network.DNS.EnableProxy is identical to setting it to false, which means there is no "Unset" state. So I guess we can use the precense of the dns_servers as an indicator whether to enable the proxy or not?

[hazzik]

@magodo no, it is not. Firewall supports DNS Proxy to "nowhere" aka Azure default DNS. Also DNS proxy could be disabled even if the proxy servers are set. These options are really independent of each other. The first implementation was more correct than the one which was committed.

[magodo]

@katbyte I have updated the dns_settings to be simply a dns_servers list. Please take another look. Thanks!

[manicminer]

Test results:

[ghost]

This has been released in version 2.35.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.35.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!