Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_firewall_policy_rule_collection_group - support description,destination_addresses,destination_urls,terminate_tls,web_categories #13190

Merged

Conversation

gro1m
Copy link
Contributor

@gro1m gro1m commented Aug 31, 2021

Fixes #11438
Fixes #12944
Fixes #12086

@gro1m gro1m changed the title enhance firewall policy rule collection group to support Azure Firewall Premium features enhance azurerm_firewall_policy_rule_collection_group to support Azure Firewall Premium features Aug 31, 2021
@gro1m
Copy link
Contributor Author

gro1m commented Aug 31, 2021

@manicminer Unfortunately, the azurerm_firewall_policy update is not enough to use the Azure Firewall Premium features, also the azurerm_firewall_policy_rule_collection_group has to be updated this is the reason for this PR.

@gro1m
Copy link
Contributor Author

gro1m commented Aug 31, 2021

@katbyte Should one of the other issues also be drawn into this PR or shall they be addressed in another PR. Setting the protocol to optional should not be a problem, if this would be general... About the other 2 not yet considered issues, I think it'd be probably better to address them separately, right?

@katbyte
Copy link
Collaborator

katbyte commented Aug 31, 2021

@gro1m - as long as the PR is only a couple 100 lines and the changes are small i'm not to concerned with resolving more issues in a single PR

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have some test failures @gro1m

------- Stdout: -------
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_complete
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_complete
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_complete
    testcase.go:88: Step 1/2 error: Error running apply: exit status 1
        
        Error: creating Firewall Policy Rule Collection Group "acctest-fwpolicy-RCG-210831222724613425" (Resource Group "acctestRG-fwpolicy-RCG-210831222724613425" / Policy: "acctest-fwpolicy-RCG-210831222724613425"): network.FirewallPolicyRuleCollectionGroupsClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code="Failed" Message="The async operation failed." AdditionalInfo=[{"Message":"The request is invalid.","ModelState":{"resource":["{\"Status\":\"Failed\",\"Error\":{\"Code\":\"FirewallPolicyApplicationRuleInvalidWebCategory\",\"Message\":\"Firewall Policy Application Rule app_rule_collection1_rule3 web category 'Liability' is invalid\",\"Target\":null}}"]}}]
        
          with azurerm_firewall_policy_rule_collection_group.test,
          on terraform_plugin_test.tf line 35, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          35: resource "azurerm_firewall_policy_rule_collection_group" "test" {
        
--- FAIL: TestAccFirewallPolicyRuleCollectionGroup_complete (145.57s)
FAIL

@gro1m
Copy link
Contributor Author

gro1m commented Sep 1, 2021

@katbyte Tests should be passing :)

make acctests SERVICE='firewall' TESTARGS='-run=TestAccFirewallPolicyRuleCollectionGroup_complete'
==> 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 -v ./internal/services/firewall -run=TestAccFirewallPolicyRuleCollectionGroup_complete -timeout 180m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_complete
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_complete
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_complete
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_complete (110.51s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/firewall      111.669s

@gro1m gro1m requested a review from katbyte September 1, 2021 16:28
@katbyte
Copy link
Collaborator

katbyte commented Sep 2, 2021

Thanks @gro1m - update test still needs a fix it seems:

------- Stdout: -------
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_update
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_update
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_update
    testcase.go:88: Step 3/6 error: Error running pre-apply refresh: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 43, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          43:     rule {
        
        The argument "description" is required, but no definition was found.
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 56, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          56:     rule {
        
        The argument "description" is required, but no definition was found.
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 65, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          65:     rule {
        
        The argument "description" is required, but no definition was found.
    testing_new.go:70: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 43, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          43:     rule {
        
        The argument "description" is required, but no definition was found.
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 56, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          56:     rule {
        
        The argument "description" is required, but no definition was found.
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 65, in resource "azurerm_firewall_policy_rule_collection_group" "test":
          65:     rule {
        
        The argument "description" is required, but no definition was found.
--- FAIL: TestAccFirewallPolicyRuleCollectionGroup_update (102.22s)
FAIL

@github-actions github-actions bot added size/XL and removed size/M labels Sep 2, 2021
@gro1m
Copy link
Contributor Author

gro1m commented Sep 2, 2021

Tests are fixed:

make acctests SERVICE='firewall' TESTARGS='-run=TestAccFirewallPolicyRuleCollectionGroup_'
==> 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 -v ./internal/services/firewall -run=TestAccFirewallPolicyRuleCollectionGroup_ -timeout 180m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_basic
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_basic
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_complete
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_complete
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_update
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_update
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_requiresImport
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_requiresImport
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_basic
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_requiresImport
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_update
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_complete
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_basic (104.72s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_complete (119.63s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_requiresImport (119.71s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_update (219.90s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/firewall      221.043s

@gro1m
Copy link
Contributor Author

gro1m commented Sep 2, 2021

@katbyte Everything should be fixed now, see tests above. I also removed lifecycle ignore_changes blocks that I needed to do the testing.

@gro1m
Copy link
Contributor Author

gro1m commented Sep 2, 2021

Thanks for your review, @katbyte. And now everything should be o.k. - now also the Premium tests should be implemented and the TEST SUMMARY looks as follows:

make acctests SERVICE='firewall' TESTARGS='-run=TestAccFirewallPolicyRuleCollectionGroup_'
==> 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 -v ./internal/services/firewall -run=TestAccFirewallPolicyRuleCollectionGroup_ -timeout 180m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_basic
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_basic
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_complete
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_complete
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_completePremium
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_completePremium
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_update
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_update
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_updatePremium
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_updatePremium
=== RUN   TestAccFirewallPolicyRuleCollectionGroup_requiresImport
=== PAUSE TestAccFirewallPolicyRuleCollectionGroup_requiresImport
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_basic
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_updatePremium
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_completePremium
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_update
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_requiresImport
=== CONT  TestAccFirewallPolicyRuleCollectionGroup_complete
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_completePremium (120.77s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_complete (126.77s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_requiresImport (127.19s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_basic (177.49s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_updatePremium (228.57s)
--- PASS: TestAccFirewallPolicyRuleCollectionGroup_update (232.89s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/firewall      234.042s

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gro1m - LGTM 🚀

@katbyte katbyte changed the title enhance azurerm_firewall_policy_rule_collection_group to support Azure Firewall Premium features azurerm_firewall_policy_rule_collection_group - support description,destination_addresses,destination_urls,terminate_tls,web_categories Sep 3, 2021
@katbyte katbyte merged commit cc28a21 into hashicorp:main Sep 3, 2021
katbyte added a commit that referenced this pull request Sep 3, 2021
@github-actions
Copy link

This functionality has been released in v2.76.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.