Fixed preconfigured_waf_config permadiff causing rules being recreated (GoogleCloudPlatform#11946)

Author: matheusaleixo-cit

mmv1/third_party/terraform/services/compute/resource_compute_security_policy.go.tmpl

@@ -4,8 +4,8 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"reflect"
 	"strings"
-
 	"time"
 
 	"github.com/hashicorp/errwrap"
@@ -24,6 +24,16 @@ import (
 {{- end }}
 )
 
+// IsEmptyValue does not consider a empty PreconfiguredWafConfig object as empty so we check it's nested values
+func preconfiguredWafConfigIsEmptyValue(config *compute.SecurityPolicyRulePreconfiguredWafConfig) bool {
+	if (tpgresource.IsEmptyValue(reflect.ValueOf(config.Exclusions)) &&
+			tpgresource.IsEmptyValue(reflect.ValueOf(config.ForceSendFields)) &&
+			tpgresource.IsEmptyValue(reflect.ValueOf(config.NullFields))) {
+		return true
+	}
+	return false
+}
+
 func ResourceComputeSecurityPolicy() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceComputeSecurityPolicyCreate,
@@ -198,7 +208,6 @@ func ResourceComputeSecurityPolicy() *schema.Resource {
 							Description: `A match condition that incoming traffic is evaluated against. If it evaluates to true, the corresponding action is enforced.`,
 						},
 
-						{{ if ne $.TargetVersionName `ga` -}}
 						"preconfigured_waf_config": {
 							Type:     schema.TypeList,
 							Optional: true,
@@ -246,7 +255,6 @@ func ResourceComputeSecurityPolicy() *schema.Resource {
 							},
 							Description: `Preconfigured WAF configuration to be applied for the rule. If the rule does not evaluate preconfigured WAF rules, i.e., if evaluatePreconfiguredWaf() is not used, this field will have no effect.`,
 						},
-						{{- end }}
 
 						"description": {
 							Type:        schema.TypeString,
@@ -597,7 +605,6 @@ func ResourceComputeSecurityPolicy() *schema.Resource {
 	}
 }
 
-{{ if ne $.TargetVersionName `ga` -}}
 func resourceComputeSecurityPolicyRulePreconfiguredWafConfigExclusionFieldParamsSchema(description string) *schema.Schema {
 	return &schema.Schema{
 		Type:     schema.TypeList,
@@ -620,7 +627,6 @@ func resourceComputeSecurityPolicyRulePreconfiguredWafConfigExclusionFieldParams
 		Description: description,
 	}
 }
-{{- end }}
 
 func rulesCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
 	_, n := diff.GetChange("rule")
@@ -730,7 +736,7 @@ func resourceComputeSecurityPolicyRead(d *schema.ResourceData, meta interface{})
 	if err := d.Set("type", securityPolicy.Type); err != nil {
 		return fmt.Errorf("Error setting type: %s", err)
 	}
-	if err := d.Set("rule", flattenSecurityPolicyRules(securityPolicy.Rules)); err != nil {
+	if err := d.Set("rule", flattenSecurityPolicyRules(securityPolicy.Rules, d)); err != nil {
 		return err
 	}
 	if err := d.Set("fingerprint", securityPolicy.Fingerprint); err != nil {
@@ -1042,9 +1048,7 @@ func expandSecurityPolicyRule(raw interface{}) *compute.SecurityPolicyRule {
 		Action:                 data["action"].(string),
 		Preview:                data["preview"].(bool),
 		Match:                  expandSecurityPolicyMatch(data["match"].([]interface{})),
-	{{- if ne $.TargetVersionName "ga" }}
 		PreconfiguredWafConfig: expandSecurityPolicyPreconfiguredWafConfig(data["preconfigured_waf_config"].([]interface{})),
-	{{- end }}
 		RateLimitOptions:       expandSecurityPolicyRuleRateLimitOptions(data["rate_limit_options"].([]interface{})),
 		RedirectOptions:        expandSecurityPolicyRuleRedirectOptions(data["redirect_options"].([]interface{})),
 		HeaderAction:           expandSecurityPolicyRuleHeaderAction(data["header_action"].([]interface{})),
@@ -1128,7 +1132,6 @@ func expandSecurityPolicyMatchExprOptionsRecaptchaOptions(recaptchaOptions []int
 	}
 }
 
-{{ if ne $.TargetVersionName `ga` -}}
 func expandSecurityPolicyPreconfiguredWafConfig(configured []interface{}) *compute.SecurityPolicyRulePreconfiguredWafConfig {
 	if len(configured) == 0 || configured[0] == nil {
 		return nil
@@ -1175,9 +1178,8 @@ func expandSecurityPolicyRulePreconfiguredWafConfigExclusionFieldParam(raw inter
 		Val: data["value"].(string),
 	}
 }
-{{- end }}
 
-func flattenSecurityPolicyRules(rules []*compute.SecurityPolicyRule) []map[string]interface{} {
+func flattenSecurityPolicyRules(rules []*compute.SecurityPolicyRule, d *schema.ResourceData) []map[string]interface{} {
 	rulesSchema := make([]map[string]interface{}, 0, len(rules))
 	for _, rule := range rules {
 		data := map[string]interface{}{
@@ -1186,9 +1188,7 @@ func flattenSecurityPolicyRules(rules []*compute.SecurityPolicyRule) []map[strin
 			"action":                   rule.Action,
 			"preview":                  rule.Preview,
 			"match":                    flattenMatch(rule.Match),
-		{{- if ne $.TargetVersionName "ga" }}
-			"preconfigured_waf_config": flattenPreconfiguredWafConfig(rule.PreconfiguredWafConfig),
-		{{- end }}
+			"preconfigured_waf_config": flattenPreconfiguredWafConfig(rule.PreconfiguredWafConfig, d, int(rule.Priority)),
 			"rate_limit_options":	      flattenSecurityPolicyRuleRateLimitOptions(rule.RateLimitOptions),
 			"redirect_options":         flattenSecurityPolicyRedirectOptions(rule.RedirectOptions),
 			"header_action":            flattenSecurityPolicyRuleHeaderAction(rule.HeaderAction),
@@ -1266,12 +1266,29 @@ func flattenMatchExpr(match *compute.SecurityPolicyRuleMatcher) []map[string]int
 	return []map[string]interface{}{data}
 }
 
-{{ if ne $.TargetVersionName `ga` -}}
-func flattenPreconfiguredWafConfig(config *compute.SecurityPolicyRulePreconfiguredWafConfig) []map[string]interface{} {
+func flattenPreconfiguredWafConfig(config *compute.SecurityPolicyRulePreconfiguredWafConfig, d *schema.ResourceData, rulePriority int) []map[string]interface{} {
 	if config == nil {
 		return nil
 	}
 
+	// We find the current value for this field in the config and check if its empty, then check if the API is returning a empty non-null value
+	if schemaRules, ok := d.GetOk("rule"); ok {
+		for _, itemRaw := range schemaRules.(*schema.Set).List() {
+			if itemRaw == nil {
+				continue
+			}
+			item := itemRaw.(map[string]interface{})
+
+			schemaPriority := item["priority"].(int)
+			if rulePriority == schemaPriority {
+				if preconfiguredWafConfigIsEmptyValue(config) && tpgresource.IsEmptyValue(reflect.ValueOf(item["preconfigured_waf_config"])) {
+					return nil
+				}
+				break
+			}
+		}
+	}
+
 	data := map[string]interface{}{
 		"exclusion": flattenPreconfiguredWafConfigExclusions(config.Exclusions),
 	}
@@ -1307,7 +1324,6 @@ func flattenPreconfiguredWafConfigExclusionField(fieldParams []*compute.Security
 	}
 	return fieldSchema
 }
-{{- end }}
 
 func expandSecurityPolicyAdvancedOptionsConfig(configured []interface{}) *compute.SecurityPolicyAdvancedOptionsConfig {
 	if len(configured) == 0 || configured[0] == nil {

mmv1/third_party/terraform/services/compute/resource_compute_security_policy_test.go.tmpl

@@ -77,7 +77,6 @@ func TestAccComputeSecurityPolicy_withRuleExpr(t *testing.T) {
 	})
 }
 
-{{ if ne $.TargetVersionName `ga` -}}
 func TestAccComputeSecurityPolicy_withPreconfiguredWafConfig(t *testing.T) {
 	t.Parallel()
 
@@ -112,10 +111,18 @@ func TestAccComputeSecurityPolicy_withPreconfiguredWafConfig(t *testing.T) {
 				ImportState:       true,
 				ImportStateVerify: true,
 			},
+			{
+				Config: testAccComputeSecurityPolicy_withPreconfiguredWafConfig_removed(spName),
+			},
+			{
+				ResourceName:      "google_compute_security_policy.policy",
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{"rule.1.preconfigured_waf_config.#", "rule.1.preconfigured_waf_config.0.%"}, // API will still return a empty object
+			},
 		},
 	})
 }
-{{- end }}
 
 func TestAccComputeSecurityPolicy_update(t *testing.T) {
 	t.Parallel()
@@ -991,7 +998,6 @@ resource "google_compute_security_policy" "policy" {
 `, spName)
 }
 
-{{ if ne $.TargetVersionName `ga` -}}
 func testAccComputeSecurityPolicy_withPreconfiguredWafConfig(spName string) string {
 	return fmt.Sprintf(`
 resource "google_compute_security_policy" "policy" {
@@ -1150,7 +1156,38 @@ resource "google_compute_security_policy" "policy" {
 }
 `, spName)
 }
-{{- end }}
+
+func testAccComputeSecurityPolicy_withPreconfiguredWafConfig_removed(spName string) string {
+	return fmt.Sprintf(`
+resource "google_compute_security_policy" "policy" {
+	name = "%s"
+
+	rule {
+		action   = "allow"
+		priority = "2147483647"
+		match {
+			versioned_expr = "SRC_IPS_V1"
+			config {
+				src_ip_ranges = ["*"]
+			}
+		}
+		description = "default rule"
+	}
+
+	rule {
+		action   = "deny"
+		priority = "1000"
+		match {
+			expr {
+				expression = "evaluatePreconfiguredWaf('rce-stable') || evaluatePreconfiguredWaf('xss-stable')"
+			}
+		}
+		// remove waf config field to test if last step wont cause a permadiff //
+		preview = false
+	}
+}
+`, spName)
+}
 
 func testAccComputeSecurityPolicy_withoutHeadAction(spName string) string {
 	return fmt.Sprintf(`

mmv1/third_party/terraform/website/docs/r/compute_security_policy.html.markdown

@@ -229,7 +229,7 @@ The following arguments are supported:
 * `match` - (Required) A match condition that incoming traffic is evaluated against.
     If it evaluates to true, the corresponding `action` is enforced. Structure is [documented below](#nested_match).
 
-* `preconfigured_waf_config` - (Optional, [Beta](https://terraform.io/docs/providers/google/guides/provider_versions.html)) Preconfigured WAF configuration to be applied for the rule. If the rule does not evaluate preconfigured WAF rules, i.e., if `evaluatePreconfiguredWaf()` is not used, this field will have no effect. Structure is [documented below](#nested_preconfigured_waf_config).
+* `preconfigured_waf_config` - (Optional) Preconfigured WAF configuration to be applied for the rule. If the rule does not evaluate preconfigured WAF rules, i.e., if `evaluatePreconfiguredWaf()` is not used, this field will have no effect. Structure is [documented below](#nested_preconfigured_waf_config).
 
 * `description` - (Optional) An optional description of this rule. Max size is 64.