From 2ddee7f8f886e849b2b5836cf62efe0fdc94a8b5 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Thu, 13 Aug 2020 02:56:09 -0400 Subject: [PATCH] fix forced resource recreation due to rule block force-new arguments --- aws/resource_aws_wafv2_rule_group.go | 1 - aws/resource_aws_wafv2_rule_group_test.go | 299 +++++++++++++++++++--- aws/wafv2_helper.go | 1 - 3 files changed, 263 insertions(+), 38 deletions(-) diff --git a/aws/resource_aws_wafv2_rule_group.go b/aws/resource_aws_wafv2_rule_group.go index 4d40a117412..b523161891a 100644 --- a/aws/resource_aws_wafv2_rule_group.go +++ b/aws/resource_aws_wafv2_rule_group.go @@ -95,7 +95,6 @@ func resourceAwsWafv2RuleGroup() *schema.Resource { "name": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validation.StringLenBetween(1, 128), }, "priority": { diff --git a/aws/resource_aws_wafv2_rule_group_test.go b/aws/resource_aws_wafv2_rule_group_test.go index 96f93f840fc..0b2f6737e9e 100644 --- a/aws/resource_aws_wafv2_rule_group_test.go +++ b/aws/resource_aws_wafv2_rule_group_test.go @@ -19,6 +19,43 @@ func TestAccAwsWafv2RuleGroup_basic(t *testing.T) { ruleGroupName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_wafv2_rule_group.test" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsWafv2RuleGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAwsWafv2RuleGroupConfig_Basic(ruleGroupName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsWafv2RuleGroupExists(resourceName, &v), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexp.MustCompile(`regional/rulegroup/.+$`)), + resource.TestCheckResourceAttr(resourceName, "capacity", "2"), + resource.TestCheckResourceAttr(resourceName, "name", ruleGroupName), + resource.TestCheckResourceAttr(resourceName, "description", ruleGroupName), + resource.TestCheckResourceAttr(resourceName, "rule.#", "0"), + resource.TestCheckResourceAttr(resourceName, "scope", wafv2.ScopeRegional), + resource.TestCheckResourceAttr(resourceName, "visibility_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.cloudwatch_metrics_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.metric_name", "friendly-metric-name"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.sampled_requests_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateIdFunc: testAccAwsWafv2RuleGroupImportStateIdFunc(resourceName), + }, + }, + }) +} + +func TestAccAwsWafv2RuleGroup_updateRule(t *testing.T) { + var v wafv2.RuleGroup + ruleGroupName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_wafv2_rule_group.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -55,15 +92,115 @@ func TestAccAwsWafv2RuleGroup_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "visibility_config.0.cloudwatch_metrics_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "visibility_config.0.metric_name", "friendly-metric-name"), resource.TestCheckResourceAttr(resourceName, "visibility_config.0.sampled_requests_enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "rule.#", "2"), + resource.TestCheckResourceAttr(resourceName, "rule.#", "1"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "name": "rule-1", + "priority": "1", + "action.#": "1", + "action.0.allow.#": "0", + "action.0.block.#": "0", + "action.0.count.#": "1", + "statement.#": "1", + "statement.0.geo_match_statement.#": "1", + "statement.0.geo_match_statement.0.country_codes.#": "2", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateIdFunc: testAccAwsWafv2RuleGroupImportStateIdFunc(resourceName), + }, + }, + }) +} + +func TestAccAwsWafv2RuleGroup_updateRuleProperties(t *testing.T) { + var v wafv2.RuleGroup + ruleGroupName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_wafv2_rule_group.test" + ruleName2 := fmt.Sprintf("%s-2", ruleGroupName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsWafv2RuleGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAwsWafv2RuleGroupConfig_BasicUpdate(ruleGroupName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsWafv2RuleGroupExists(resourceName, &v), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexp.MustCompile(`regional/rulegroup/.+$`)), + resource.TestCheckResourceAttr(resourceName, "capacity", "50"), + resource.TestCheckResourceAttr(resourceName, "name", ruleGroupName), + resource.TestCheckResourceAttr(resourceName, "description", "Updated"), + resource.TestCheckResourceAttr(resourceName, "scope", wafv2.ScopeRegional), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.cloudwatch_metrics_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.metric_name", "friendly-metric-name"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.sampled_requests_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "rule.#", "1"), tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ - "name": "rule-2", - "priority": "10", + "name": "rule-1", + "priority": "1", "action.#": "1", "action.0.allow.#": "0", - "action.0.block.#": "1", - "action.0.count.#": "0", - "statement.#": "1", + "action.0.block.#": "0", + "action.0.count.#": "1", + "visibility_config.0.cloudwatch_metrics_enabled": "false", + "visibility_config.0.metric_name": "friendly-rule-metric-name", + "visibility_config.0.sampled_requests_enabled": "false", + "statement.#": "1", + "statement.0.geo_match_statement.#": "1", + "statement.0.geo_match_statement.0.country_codes.#": "2", + }), + ), + }, + { + // Test step verifies addition of a rule block with the first block unchanged + Config: testAccAwsWafv2RuleGroupConfig_UpdateMultipleRules(ruleGroupName, "rule-1", ruleName2, 1, 2), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsWafv2RuleGroupExists(resourceName, &v), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexp.MustCompile(`regional/rulegroup/.+$`)), + resource.TestCheckResourceAttr(resourceName, "capacity", "50"), + resource.TestCheckResourceAttr(resourceName, "name", ruleGroupName), + resource.TestCheckResourceAttr(resourceName, "description", "Updated"), + resource.TestCheckResourceAttr(resourceName, "scope", wafv2.ScopeRegional), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.cloudwatch_metrics_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.metric_name", "friendly-metric-name"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.sampled_requests_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "rule.#", "2"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "name": "rule-1", + "priority": "1", + "action.#": "1", + "action.0.allow.#": "0", + "action.0.block.#": "0", + "action.0.count.#": "1", + "visibility_config.#": "1", + "visibility_config.0.cloudwatch_metrics_enabled": "false", + "visibility_config.0.metric_name": "rule-1", + "visibility_config.0.sampled_requests_enabled": "false", + "statement.#": "1", + "statement.0.geo_match_statement.#": "1", + "statement.0.geo_match_statement.0.country_codes.#": "2", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "name": ruleName2, + "priority": "2", + "action.#": "1", + "action.0.allow.#": "0", + "action.0.block.#": "1", + "action.0.count.#": "0", + "visibility_config.#": "1", + "visibility_config.0.cloudwatch_metrics_enabled": "false", + "visibility_config.0.metric_name": ruleName2, + "visibility_config.0.sampled_requests_enabled": "false", + "statement.#": "1", "statement.0.size_constraint_statement.#": "1", "statement.0.size_constraint_statement.0.comparison_operator": "LT", "statement.0.size_constraint_statement.0.field_to_match.#": "1", @@ -79,13 +216,65 @@ func TestAccAwsWafv2RuleGroup_basic(t *testing.T) { "priority": "5", "type": "NONE", }), + ), + }, + { + // Test step to verify a change in priority for rule #1 and a change in name and priority for rule #2 + Config: testAccAwsWafv2RuleGroupConfig_UpdateMultipleRules(ruleGroupName, "rule-1", "updated", 5, 10), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsWafv2RuleGroupExists(resourceName, &v), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexp.MustCompile(`regional/rulegroup/.+$`)), + resource.TestCheckResourceAttr(resourceName, "capacity", "50"), + resource.TestCheckResourceAttr(resourceName, "name", ruleGroupName), + resource.TestCheckResourceAttr(resourceName, "description", "Updated"), + resource.TestCheckResourceAttr(resourceName, "scope", wafv2.ScopeRegional), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.cloudwatch_metrics_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.metric_name", "friendly-metric-name"), + resource.TestCheckResourceAttr(resourceName, "visibility_config.0.sampled_requests_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "rule.#", "2"), tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ - "name": "rule-1", - "priority": "1", - "action.#": "1", - "action.0.allow.#": "0", - "action.0.block.#": "0", - "action.0.count.#": "1", + "name": "rule-1", + "priority": "5", + "action.#": "1", + "action.0.allow.#": "0", + "action.0.block.#": "0", + "action.0.count.#": "1", + "visibility_config.#": "1", + "visibility_config.0.cloudwatch_metrics_enabled": "false", + "visibility_config.0.metric_name": "rule-1", + "visibility_config.0.sampled_requests_enabled": "false", + "statement.#": "1", + "statement.0.geo_match_statement.#": "1", + "statement.0.geo_match_statement.0.country_codes.#": "2", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "name": "updated", + "priority": "10", + "action.#": "1", + "action.0.allow.#": "0", + "action.0.block.#": "1", + "action.0.count.#": "0", + "visibility_config.#": "1", + "visibility_config.0.cloudwatch_metrics_enabled": "false", + "visibility_config.0.metric_name": "updated", + "visibility_config.0.sampled_requests_enabled": "false", + "statement.#": "1", + "statement.0.size_constraint_statement.#": "1", + "statement.0.size_constraint_statement.0.comparison_operator": "LT", + "statement.0.size_constraint_statement.0.field_to_match.#": "1", + "statement.0.size_constraint_statement.0.field_to_match.0.query_string.#": "1", + "statement.0.size_constraint_statement.0.size": "50", + "statement.0.size_constraint_statement.0.text_transformation.#": "2", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*.statement.0.size_constraint_statement.0.text_transformation.*", map[string]string{ + "priority": "2", + "type": "CMD_LINE", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*.statement.0.size_constraint_statement.0.text_transformation.*", map[string]string{ + "priority": "5", + "type": "NONE", }), ), }, @@ -1235,31 +1424,16 @@ resource "aws_wafv2_rule_group" "test" { scope = "REGIONAL" rule { - name = "rule-2" - priority = 10 + name = "rule-1" + priority = 1 action { - block {} + count {} } statement { - size_constraint_statement { - comparison_operator = "LT" - size = 50 - - field_to_match { - query_string {} - } - - text_transformation { - priority = 5 - type = "NONE" - } - - text_transformation { - priority = 2 - type = "CMD_LINE" - } + geo_match_statement { + country_codes = ["US", "NL"] } } @@ -1270,9 +1444,26 @@ resource "aws_wafv2_rule_group" "test" { } } + visibility_config { + cloudwatch_metrics_enabled = false + metric_name = "friendly-metric-name" + sampled_requests_enabled = false + } +} +`, name) +} + +func testAccAwsWafv2RuleGroupConfig_UpdateMultipleRules(name string, ruleName1, ruleName2 string, priority1, priority2 int) string { + return fmt.Sprintf(` +resource "aws_wafv2_rule_group" "test" { + capacity = 50 + name = "%[1]s" + description = "Updated" + scope = "REGIONAL" + rule { - name = "rule-1" - priority = 1 + name = "%[2]s" + priority = %[3]d action { count {} @@ -1286,7 +1477,43 @@ resource "aws_wafv2_rule_group" "test" { visibility_config { cloudwatch_metrics_enabled = false - metric_name = "friendly-rule-metric-name" + metric_name = "%[2]s" + sampled_requests_enabled = false + } + } + + rule { + name = "%[4]s" + priority = %[5]d + + action { + block {} + } + + statement { + size_constraint_statement { + comparison_operator = "LT" + size = 50 + + field_to_match { + query_string {} + } + + text_transformation { + priority = 5 + type = "NONE" + } + + text_transformation { + priority = 2 + type = "CMD_LINE" + } + } + } + + visibility_config { + cloudwatch_metrics_enabled = false + metric_name = "%[4]s" sampled_requests_enabled = false } } @@ -1297,7 +1524,7 @@ resource "aws_wafv2_rule_group" "test" { sampled_requests_enabled = false } } -`, name) +`, name, ruleName1, priority1, ruleName2, priority2) } func testAccAwsWafv2RuleGroupConfig_UpdateCapacity(name string) string { diff --git a/aws/wafv2_helper.go b/aws/wafv2_helper.go index 41186ff40e5..4c85ed08cbb 100644 --- a/aws/wafv2_helper.go +++ b/aws/wafv2_helper.go @@ -342,7 +342,6 @@ func wafv2VisibilityConfigSchema() *schema.Schema { "metric_name": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validation.All( validation.StringLenBetween(1, 128), validation.StringMatch(regexp.MustCompile(`^[a-zA-Z0-9-_]+$`), "must contain only alphanumeric hyphen and underscore characters"),