`azurerm_web_application_firewall_policy`: upgrade API version to 2023-02-01 #22455

wuxu92 · 2023-07-11T09:23:56Z

Draft for TC

--- PASS: TestAccWebApplicationFirewallPolicy_basic (276.18s)
--- PASS: TestAccWebApplicationFirewallPolicy_complete (275.88s)
--- PASS: TestAccWebApplicationFirewallPolicy_update (293.06s)
--- PASS: TestAccWebApplicationFirewallPolicy_updateOverrideRules (336.83s)
--- PASS: TestAccWebApplicationFirewallPolicy_knownCVEs (278.11s)
--- PASS: TestAccWebApplicationFirewallPolicy_OperatorAny (274.98s)
--- PASS: TestAccWebApplicationFirewallPolicy_excludedRules (309.16s)
--- PASS: TestAccWebApplicationFirewallPolicy_updateDisabledRules (312.72s)
PASS

tombuildsstuff

hi @wuxu92

Thanks for this PR - I've taken a look through and left some comments inline, but if we can fix those up then this should otherwise be good to go 👍

Thanks!

tombuildsstuff · 2023-07-11T14:53:53Z

internal/services/network/web_application_firewall_policy_resource.go

+	future, err := client.Delete(ctx, *id)
 	if err != nil {
 		return fmt.Errorf("deleting %s: %+v", *id, err)
 	}

-	if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
+	if err = future.Poller.PollUntilDone(ctx); err != nil {
 		return fmt.Errorf("waiting for the deletion of %s: %+v", *id, err)
 	}


we can reduce this to: `if err := client.DeleteThenPoll(...); err != nil { ... }

internal/services/network/web_application_firewall_policy_resource.go

tombuildsstuff · 2023-07-11T16:45:16Z

internal/services/network/web_application_firewall_policy_resource.go

+		if location := model.Location; location != nil {
+			d.Set("location", azure.NormalizeLocation(*location))
 		}


this can be reduced to:

Suggested change

if location := model.Location; location != nil {

d.Set("location", azure.NormalizeLocation(*location))

}

d.Set("location", location.NormalizeNilable(model.Location))

tombuildsstuff · 2023-07-11T16:46:11Z

internal/services/network/web_application_firewall_policy_resource.go

@@ -680,8 +684,8 @@ func expandWebApplicationFirewallPolicyExclusions(input []interface{}) *[]networ
 	return &results
 }

-func expandWebApplicationFirewallPolicyManagedRuleSet(input []interface{}, d *pluginsdk.ResourceData) (*[]network.ManagedRuleSet, error) {
-	results := make([]network.ManagedRuleSet, 0)
+func expandWebApplicationFirewallPolicyManagedRuleSet(input []interface{}, d *pluginsdk.ResourceData) ([]webapplicationfirewallpolicies.ManagedRuleSet, error) {


we always return a pointer and an error when an error is returned too:

Suggested change

func expandWebApplicationFirewallPolicyManagedRuleSet(input []interface{}, d *pluginsdk.ResourceData) ([]webapplicationfirewallpolicies.ManagedRuleSet, error) {

func expandWebApplicationFirewallPolicyManagedRuleSet(input []interface{}, d *pluginsdk.ResourceData) (*[]webapplicationfirewallpolicies.ManagedRuleSet, error) {

slice can function as a pointer in many instances, though it is not a pointer. I'll update it to a pointer to slice to make it the same as others.

tombuildsstuff · 2023-07-11T16:46:24Z

internal/services/network/web_application_firewall_policy_resource.go

@@ -791,8 +795,8 @@ func expandWebApplicationFirewallPolicyOverrideRules(input []interface{}) *[]net
 	return &results
 }

-func expandWebApplicationFirewallPolicyMatchCondition(input []interface{}) *[]network.MatchCondition {
-	results := make([]network.MatchCondition, 0)
+func expandWebApplicationFirewallPolicyMatchCondition(input []interface{}) []webapplicationfirewallpolicies.MatchCondition {


(same here)

this function will not return with an error, so do not need a pointer to slice?

maybe the expand functions should return a pointer only when the SDK Field is a pointer too? or we always return a pointer no matter what the SDK is?

internal/services/network/web_application_firewall_policy_resource.go

tombuildsstuff · 2023-07-11T16:47:39Z

internal/services/network/web_application_firewall_policy_resource_test.go

 	if err != nil {
 		return nil, fmt.Errorf("reading %s: %+v", *id, err)
 	}

-	return utils.Bool(resp.ID != nil), nil
+	return utils.Bool(resp.Model.Id != nil), nil


Suggested change

return utils.Bool(resp.Model.Id != nil), nil

return utils.Bool(resp.Model != nil), nil

wuxu92 · 2023-07-13T02:26:14Z

Tests pass now:

katbyte

LGTM 🕸️

stale

katbyte · 2023-07-25T15:42:48Z

@wuxu92 - could you fix up the build?

wuxu92 · 2023-07-26T08:06:29Z

@wuxu92 Xu Wu FTE - could you fix up the build?

@katbyte updated.

…-azure-sdk@2023-02-01

tombuildsstuff · 2023-07-28T06:25:11Z

@wuxu92 hope you don't mind but I've rebased this atop of main and updated this to use the Meta Client, since this is now available as of #22676

tombuildsstuff

One minor comment that I'm going to push a commit to fix, but this otherwise LGTM - thanks @wuxu92

internal/services/network/web_application_firewall_policy_data_source.go

wuxu92 · 2023-07-28T06:30:19Z

@tombuildsstuff of course not! thanks!

github-actions · 2024-05-20T02:10:23Z

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 bot added dependencies service/network size/XL labels Jul 11, 2023

tombuildsstuff previously requested changes Jul 11, 2023

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Jul 11, 2023

wuxu92 marked this pull request as ready for review July 13, 2023 02:26

wuxu92 mentioned this pull request Jul 14, 2023

azurerm_web_application_firewall_policy：support log_scrubbing property #22522

Merged

github-actions bot added the documentation label Jul 17, 2023

katbyte approved these changes Jul 25, 2023

View reviewed changes

github-actions bot added the waiting-response label Jul 25, 2023

github-actions bot removed the waiting-response label Jul 26, 2023

wuxu92 and others added 6 commits July 28, 2023 08:20

upgrade webapplication firewall policies resource sdk to hashicorp/go…

0113868

…-azure-sdk@2023-02-01

code lint

78fb019

add state migration for web application firewall policy

e3bb912

remove old resourceid generate cmd

1268bb0

code format

89f8187

r/web_application_firewall_policy: updating to use the meta client

0ccfb22

tombuildsstuff force-pushed the webappfirewall/20230201 branch from b18d732 to 0ccfb22 Compare July 28, 2023 06:24

github-actions bot removed the dependencies label Jul 28, 2023

tombuildsstuff approved these changes Jul 28, 2023

View reviewed changes

internal/services/network/web_application_firewall_policy_data_source.go Outdated Show resolved Hide resolved

fixing an issue found at review time

4517144

tombuildsstuff added this to the v3.68.0 milestone Jul 28, 2023

tombuildsstuff merged commit 17c2a5a into hashicorp:main Jul 28, 2023

tombuildsstuff added a commit that referenced this pull request Jul 28, 2023

updating to include #22455

5597b3e

tombuildsstuff added a commit that referenced this pull request Jul 28, 2023

network: updating to account for #22455

5b695e6

wuxu92 deleted the webappfirewall/20230201 branch July 28, 2023 07:32

tombuildsstuff added a commit that referenced this pull request Jul 28, 2023

network: updating to account for #22455

1d0f523

turkenh mentioned this pull request Aug 16, 2023

azurerm_web_application_firewall_policy - fix handling not found in read #22982

Merged

teowa mentioned this pull request Sep 28, 2023

ApplicationGatewayWebApplicationFirewallPolicies expects a lowercase a starting at version 3.68.0 #23378

Open

1 task

github-actions bot locked as resolved and limited conversation to collaborators May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_web_application_firewall_policy`: upgrade API version to 2023-02-01 #22455

`azurerm_web_application_firewall_policy`: upgrade API version to 2023-02-01 #22455

wuxu92 commented Jul 11, 2023

tombuildsstuff left a comment

tombuildsstuff Jul 11, 2023

tombuildsstuff Jul 11, 2023

tombuildsstuff Jul 11, 2023

wuxu92 Jul 12, 2023

tombuildsstuff Jul 11, 2023

wuxu92 Jul 12, 2023

wuxu92 Jul 14, 2023

tombuildsstuff Jul 11, 2023

wuxu92 commented Jul 13, 2023

katbyte left a comment

katbyte commented Jul 25, 2023

wuxu92 commented Jul 26, 2023

tombuildsstuff commented Jul 28, 2023

tombuildsstuff left a comment

wuxu92 commented Jul 28, 2023

github-actions bot commented May 20, 2024

	func expandWebApplicationFirewallPolicyManagedRuleSet(input []interface{}, d *pluginsdk.ResourceData) ([]webapplicationfirewallpolicies.ManagedRuleSet, error) {
	func expandWebApplicationFirewallPolicyManagedRuleSet(input []interface{}, d pluginsdk.ResourceData) ([]webapplicationfirewallpolicies.ManagedRuleSet, error) {

	return utils.Bool(resp.Model.Id != nil), nil
	return utils.Bool(resp.Model != nil), nil

azurerm_web_application_firewall_policy: upgrade API version to 2023-02-01 #22455

azurerm_web_application_firewall_policy: upgrade API version to 2023-02-01 #22455

Conversation

wuxu92 commented Jul 11, 2023

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wuxu92 commented Jul 13, 2023

katbyte left a comment

Choose a reason for hiding this comment

katbyte commented Jul 25, 2023

wuxu92 commented Jul 26, 2023

tombuildsstuff commented Jul 28, 2023

tombuildsstuff left a comment

Choose a reason for hiding this comment

wuxu92 commented Jul 28, 2023

github-actions bot commented May 20, 2024

`azurerm_web_application_firewall_policy`: upgrade API version to 2023-02-01 #22455

`azurerm_web_application_firewall_policy`: upgrade API version to 2023-02-01 #22455