azurerm_firewall: supports `dns_setting` #8878

magodo · 2020-10-14T06:17:37Z

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.

`azurerm_firewall` supports `dns_setting` and `azurerm_firewall_network_rule_collection` supports `destination_fqdns`.

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!

azurerm/internal/services/network/firewall_network_rule_collection_resource.go

tombuildsstuff · 2020-10-19T05:32:51Z

azurerm/internal/services/network/firewall_resource.go

+
+	return map[string]*string{
+		"Network.DNS.EnableProxy": utils.String(fmt.Sprintf("%t", v["enabled"].(bool))),
+		"Network.DNS.Servers":     utils.String(strings.Join(servers, ",")),


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

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

could we link to this in the code?

Yes, I have added them in my last commit.

tombuildsstuff · 2020-10-19T05:39:24Z

azurerm/internal/services/network/firewall_resource.go

+	}
+
+	return map[string]*string{
+		"Network.DNS.EnableProxy": utils.String(fmt.Sprintf("%t", v["enabled"].(bool))),


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?

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

any update @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?

@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 · 2020-10-29T03:41:27Z

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

manicminer · 2020-11-05T07:00:22Z

Test results:

ghost · 2020-11-05T14:16:24Z

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 · 2020-12-05T14:50:00Z

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!

azurerm_firewall: supports dns_setting

71cca22

`azurerm_firewall` supports `dns_setting` and `azurerm_firewall_network_rule_collection` supports `destination_fqdns`.

ghost added size/L documentation labels Oct 14, 2020

magodo mentioned this pull request Oct 14, 2020

Azure Firewall Network Rule collection Compatibility #8312

Closed

magodo added enhancement service/firewall labels Oct 14, 2020

tombuildsstuff reviewed Oct 19, 2020

View reviewed changes

tombuildsstuff added the waiting-response label Oct 19, 2020

magodo mentioned this pull request Oct 19, 2020

Finalize Firewall DNS Setting in Swagger Azure/azure-rest-api-specs#11278

Open

update flatten logic in network_rule_collection

de7d098

magodo removed the waiting-response label Oct 19, 2020

katbyte added the waiting-response label Oct 28, 2020

simplify the dns_settings to dns_servers

fab06ad

magodo removed the waiting-response label Oct 29, 2020

manicminer approved these changes Nov 5, 2020

View reviewed changes

manicminer merged commit d435272 into hashicorp:master Nov 5, 2020

manicminer added a commit that referenced this pull request Nov 5, 2020

Update changelog for #8878

2961380

jackofallops added this to the v2.35.0 milestone Nov 5, 2020

poddm mentioned this pull request Nov 6, 2020

azurerm_firewall: Enable DNS Proxy + Azure provided DNS #9184

Closed

magodo mentioned this pull request Nov 6, 2020

azurerm_firewall: support for firewall manager (firewall policy) #8879

Merged

ghost locked as resolved and limited conversation to collaborators Dec 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azurerm_firewall: supports `dns_setting` #8878

azurerm_firewall: supports `dns_setting` #8878

magodo commented Oct 14, 2020 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Oct 19, 2020

magodo Oct 19, 2020

katbyte Oct 28, 2020

magodo Oct 29, 2020

tombuildsstuff Oct 19, 2020

magodo Oct 19, 2020

katbyte Oct 28, 2020

magodo Oct 29, 2020 •

edited

Loading

hazzik Nov 20, 2020 •

edited

Loading

magodo commented Oct 29, 2020

manicminer commented Nov 5, 2020

ghost commented Nov 5, 2020

ghost commented Dec 5, 2020

azurerm_firewall: supports dns_setting #8878

azurerm_firewall: supports dns_setting #8878

Conversation

magodo commented Oct 14, 2020 • edited Loading

Test Result

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Oct 19, 2020

Choose a reason for hiding this comment

magodo Oct 19, 2020

Choose a reason for hiding this comment

katbyte Oct 28, 2020

Choose a reason for hiding this comment

magodo Oct 29, 2020

Choose a reason for hiding this comment

tombuildsstuff Oct 19, 2020

Choose a reason for hiding this comment

magodo Oct 19, 2020

Choose a reason for hiding this comment

katbyte Oct 28, 2020

Choose a reason for hiding this comment

magodo Oct 29, 2020 • edited Loading

Choose a reason for hiding this comment

hazzik Nov 20, 2020 • edited Loading

Choose a reason for hiding this comment

magodo commented Oct 29, 2020

manicminer commented Nov 5, 2020

ghost commented Nov 5, 2020

ghost commented Dec 5, 2020

azurerm_firewall: supports `dns_setting` #8878

azurerm_firewall: supports `dns_setting` #8878

magodo commented Oct 14, 2020 •

edited

Loading

magodo Oct 29, 2020 •

edited

Loading

hazzik Nov 20, 2020 •

edited

Loading