azurerm_firewall: allow multiple ip_configuration blocks

[houkms]

Allow enable_ip_configuration to accept multiple ip_configurations.

(fixes #4045)

[WodansSon]

@houkms

Thanks for the PR, this mostly LGTM, but I left a few comments I would like to see addressed. Once you fix those up we can get this merged. 🚀

[houkms]

@WodansSon

Thanks for your suggestions. All the ip_configurations are reverted to ip_configuration now.
I addressed the reason about ConflictsWith inline, waiting for your advice.

[WodansSon]

@houkms this is looking good, left a few comments. Thanks for the updates! 🚀

[houkms]

@WodansSon Thanks for your suggestions. I added the validate function for ip_configuration and addressed the reason for the change of subnet_id.

[tombuildsstuff]

hi @houkms

Thanks for pushing those changes - I've taken a look through and left some comments inline.

Thanks!

[houkms]

@WodansSon I addressed the comments, would you please help review the codes?

[katbyte]

Hi @houkms

I have a question before i continue reviewing this: what is the difference between public_ip_address_id and internal_public_ip_address_id ? Do they do different things or is this just renaming the property?

thanks 🙂

[houkms]

Hi @katbyte

It's a property renaming. I think it's a way to deal with breaking change, which was already introduced before this PR.

[timja]

I missed this PR and implemented it myself as well, I implemented pretty much the same thing except there's more validation here, and I implemented a test.

I've submitted the test as a PR to your fork @houkms, if you're interested in taking it to give more confidence in this PR:
houkms#1

Either way would love to get this in,
ping @katbyte , @tombuildsstuff , @WodansSon

[houkms]

@timja Thanks for your test codes. It looks good to me. Lets wait for the reviews.

[katbyte]

Thanks for the revisions @houkms,

IN addtion to one comment i've left inline i think your going to need to update some of the tag tests:

------- Stdout: -------
=== RUN   TestAccAzureRMFirewallApplicationRuleCollection_updateFirewallTags
=== PAUSE TestAccAzureRMFirewallApplicationRuleCollection_updateFirewallTags
=== CONT  TestAccAzureRMFirewallApplicationRuleCollection_updateFirewallTags
--- FAIL: TestAccAzureRMFirewallApplicationRuleCollection_updateFirewallTags (1096.26s)
    testing.go:569: Step 1 error: errors during apply:
        
        Error: Error validating Firewall "acctestfirewall191110000748156502" (Resource Group "acctestRG-191110000748156502"): The "ip_configuration" is invalid, both "public_ip_address_id" and "internal_public_ip_address_id" have been set, all defined "ip_configuration" blocks must use the same attribute name.
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test745137549/main.tf line 30:
          (source code not available)
        
        
FAIL

------- Stderr: -------
2019/11/10 00:07:48 [DEBUG] Registering Data Sources for "Compute"..

these ones failed too

TestAccAzureRMFirewallNatRuleCollection_updateFirewallTags |  
TestAccAzureRMFirewallNetworkRuleCollection_updateFirewallTags |  
TestAccAzureRMFirewall_withTags

[houkms]

The tests fail because my validation function doesn't allow internal_public_ip_address_id and public_ip_address_id both exist before creating or updating.

While in the Read function, both of them will be set (see here) if we use any one of them, which will make they both appear in tfstate thus result in validation error in the next time we call create/update.

CONCLUSION

• I removed the conflicts validation of the two properties (follow an existing resource), and the tests work well now.

[katbyte]

Thanks for the revisions @houkms! LGTM now 👍

[ghost]

This has been released in version 1.37.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 = "~> 1.37.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 👉 hashibot-feedback@hashicorp.com. Thanks!