new resource: azurerm_firewall_policy_rule_collection_group

[magodo]

Test Result

💤 make testacc TEST=./azurerm/internal/services/network/tests TESTARGS='-run TestAccAzureRMFirewallPolicyRuleCollectionGroup'

==> 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 TestAccAzureRMFirewallPolicyRuleCollectionGroup -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMFirewallPolicyRuleCollectionGroup_basic
=== PAUSE TestAccAzureRMFirewallPolicyRuleCollectionGroup_basic
=== RUN   TestAccAzureRMFirewallPolicyRuleCollectionGroup_complete
=== PAUSE TestAccAzureRMFirewallPolicyRuleCollectionGroup_complete
=== RUN   TestAccAzureRMFirewallPolicyRuleCollectionGroup_update
=== PAUSE TestAccAzureRMFirewallPolicyRuleCollectionGroup_update
=== RUN   TestAccAzureRMFirewallPolicyRuleCollectionGroup_requiresImport
=== PAUSE TestAccAzureRMFirewallPolicyRuleCollectionGroup_requiresImport
=== CONT  TestAccAzureRMFirewallPolicyRuleCollectionGroup_basic
=== CONT  TestAccAzureRMFirewallPolicyRuleCollectionGroup_requiresImport
=== CONT  TestAccAzureRMFirewallPolicyRuleCollectionGroup_update
=== CONT  TestAccAzureRMFirewallPolicyRuleCollectionGroup_complete
--- PASS: TestAccAzureRMFirewallPolicyRuleCollectionGroup_basic (124.12s)
--- PASS: TestAccAzureRMFirewallPolicyRuleCollectionGroup_complete (124.13s)
--- PASS: TestAccAzureRMFirewallPolicyRuleCollectionGroup_requiresImport (134.30s)
--- PASS: TestAccAzureRMFirewallPolicyRuleCollectionGroup_update (216.12s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/tests       216.163s

Related issue: #7319

[katbyte]

Thanks for the PR @magodo - overall this is looking good and i've left some comments inline.

[magodo]

@katbyte I have updated the code per your review comments, please take another look!

[manicminer]

Hi @magodo, this is looking great. I have one more suggestion and a couple questions below if you could help me understand some of the design decisions. Thanks!

[manicminer]

Could you make this function use azure.ParseAzureResourceID rather than depending on FirewallPolicyID and a regex match?

[magodo]

sure

[manicminer]

Why do we need to check the existence of the linked policy before retrieving the rule collection group?

[magodo]

The reason is that we will need to use the policyResp to set the firewall_policy_id later. Instead, we should opt in to using the ID Formatter to save this API call.

[manicminer]

Why do we accept a single address/CIDR when the API accepts a list?

[magodo]

The API actually doesn't support a list, it will raise error:

Invalid DNat destination address. Multiple values, ranges and * are not supported.

[magodo]

@manicminer I have updated per your commments, please take another look, thank you!

[manicminer]

@magodo Thanks, there's one other issue I noticed - can we store the subscription ID in the FirewallPolicyId and FirewallPolicyRuleCollectionGroupId structs, taking it from the original ID string, instead of passing it in afterwards?

[magodo]

@manicminer I assume you mean this part, this is actually how the Formatter interface defined, and is used prevalently elsewhere.

[manicminer]

@magodo Ah ok thanks. I still prefer the subscriptionId be sourced from the resource rather than from the client, but this looks good to merge 👍

[manicminer]

Test results:

[magodo]

@manicminer TBH, my first adoptions to the id structures imbedded the subscriptionId inside 😅

[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!