From d757599be2879377c1865ac71b577c5648482f3a Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Fri, 15 Nov 2019 22:20:53 +0200 Subject: [PATCH] r/aws_wafregional_rule - add tagging support + expose arn + read after create (#10895) Output from acceptance testing: ``` --- PASS: TestAccAWSWafRegionalRule_disappears (36.53s) --- PASS: TestAccAWSWafRegionalRule_noPredicates (39.85s) --- PASS: TestAccAWSWafRegionalRule_tags (46.46s) --- PASS: TestAccAWSWafRegionalRule_changePredicates (72.39s) --- PASS: TestAccAWSWafRegionalRule_changeNameForceNew (74.27s) --- PASS: TestAccAWSWafRegionalRule_basic (92.20s) ``` --- aws/resource_aws_wafregional_rule.go | 56 +++++- aws/resource_aws_wafregional_rule_test.go | 168 ++++++++++++++---- website/docs/r/wafregional_rule.html.markdown | 2 + 3 files changed, 185 insertions(+), 41 deletions(-) diff --git a/aws/resource_aws_wafregional_rule.go b/aws/resource_aws_wafregional_rule.go index 7b139c555f4..b16703e3468 100644 --- a/aws/resource_aws_wafregional_rule.go +++ b/aws/resource_aws_wafregional_rule.go @@ -4,12 +4,13 @@ import ( "fmt" "log" - "github.com/aws/aws-sdk-go/service/wafregional" - "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/waf" + "github.com/aws/aws-sdk-go/service/wafregional" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsWafRegionalRule() *schema.Resource { @@ -55,6 +56,11 @@ func resourceAwsWafRegionalRule() *schema.Resource { }, }, }, + "tags": tagsSchema(), + "arn": { + Type: schema.TypeString, + Computed: true, + }, }, } } @@ -62,6 +68,7 @@ func resourceAwsWafRegionalRule() *schema.Resource { func resourceAwsWafRegionalRuleCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).wafregionalconn region := meta.(*AWSClient).region + tags := keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().WafregionalTags() wr := newWafRegionalRetryer(conn, region) out, err := wr.RetryWithToken(func(token *string) (interface{}, error) { @@ -71,6 +78,10 @@ func resourceAwsWafRegionalRuleCreate(d *schema.ResourceData, meta interface{}) Name: aws.String(d.Get("name").(string)), } + if len(tags) > 0 { + params.Tags = tags + } + return conn.CreateRule(params) }) if err != nil { @@ -78,7 +89,16 @@ func resourceAwsWafRegionalRuleCreate(d *schema.ResourceData, meta interface{}) } resp := out.(*waf.CreateRuleOutput) d.SetId(*resp.Rule.RuleId) - return resourceAwsWafRegionalRuleUpdate(d, meta) + + newPredicates := d.Get("predicate").(*schema.Set).List() + if len(newPredicates) > 0 { + noPredicates := []interface{}{} + err := updateWafRegionalRuleResource(d.Id(), noPredicates, newPredicates, meta) + if err != nil { + return fmt.Errorf("Error Updating WAF Regional Rule: %s", err) + } + } + return resourceAwsWafRegionalRuleRead(d, meta) } func resourceAwsWafRegionalRuleRead(d *schema.ResourceData, meta interface{}) error { @@ -99,6 +119,25 @@ func resourceAwsWafRegionalRuleRead(d *schema.ResourceData, meta interface{}) er return err } + arn := arn.ARN{ + AccountID: meta.(*AWSClient).accountid, + Partition: meta.(*AWSClient).partition, + Region: meta.(*AWSClient).region, + Resource: fmt.Sprintf("rule/%s", d.Id()), + Service: "waf-regional", + }.String() + d.Set("arn", arn) + + tags, err := keyvaluetags.WafregionalListTags(conn, arn) + + if err != nil { + return fmt.Errorf("error listing tags for WAF Regional Rule (%s): %s", arn, err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + d.Set("predicate", flattenWafPredicates(resp.Rule.Predicates)) d.Set("name", resp.Rule.Name) d.Set("metric_name", resp.Rule.MetricName) @@ -107,6 +146,8 @@ func resourceAwsWafRegionalRuleRead(d *schema.ResourceData, meta interface{}) er } func resourceAwsWafRegionalRuleUpdate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).wafregionalconn + if d.HasChange("predicate") { o, n := d.GetChange("predicate") oldP, newP := o.(*schema.Set).List(), n.(*schema.Set).List() @@ -116,6 +157,15 @@ func resourceAwsWafRegionalRuleUpdate(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error Updating WAF Rule: %s", err) } } + + if d.HasChange("tags") { + o, n := d.GetChange("tags") + + if err := keyvaluetags.WafregionalUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { + return fmt.Errorf("error updating tags: %s", err) + } + } + return resourceAwsWafRegionalRuleRead(d, meta) } diff --git a/aws/resource_aws_wafregional_rule_test.go b/aws/resource_aws_wafregional_rule_test.go index 236700d9397..899b4da9374 100644 --- a/aws/resource_aws_wafregional_rule_test.go +++ b/aws/resource_aws_wafregional_rule_test.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -130,12 +131,10 @@ func TestAccAWSWafRegionalRule_basic(t *testing.T) { Config: testAccAWSWafRegionalRuleConfig(wafRuleName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSWafRegionalRuleExists(resourceName, &v), - resource.TestCheckResourceAttr( - resourceName, "name", wafRuleName), - resource.TestCheckResourceAttr( - resourceName, "predicate.#", "1"), - resource.TestCheckResourceAttr( - resourceName, "metric_name", wafRuleName), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "waf-regional", regexp.MustCompile(`rule/.+`)), + resource.TestCheckResourceAttr(resourceName, "name", wafRuleName), + resource.TestCheckResourceAttr(resourceName, "predicate.#", "1"), + resource.TestCheckResourceAttr(resourceName, "metric_name", wafRuleName), ), }, { @@ -147,6 +146,50 @@ func TestAccAWSWafRegionalRule_basic(t *testing.T) { }) } +func TestAccAWSWafRegionalRule_tags(t *testing.T) { + var v waf.Rule + wafRuleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) + resourceName := "aws_wafregional_rule.wafrule" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSWafRegionalRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSWafRegionalRuleConfigTags1(wafRuleName, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSWafRegionalRuleExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSWafRegionalRuleConfigTags2(wafRuleName, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSWafRegionalRuleExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSWafRegionalRuleConfigTags1(wafRuleName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSWafRegionalRuleExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + }, + }) +} + func TestAccAWSWafRegionalRule_changeNameForceNew(t *testing.T) { var before, after waf.Rule wafRuleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) @@ -162,24 +205,18 @@ func TestAccAWSWafRegionalRule_changeNameForceNew(t *testing.T) { Config: testAccAWSWafRegionalRuleConfig(wafRuleName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSWafRegionalRuleExists(resourceName, &before), - resource.TestCheckResourceAttr( - resourceName, "name", wafRuleName), - resource.TestCheckResourceAttr( - resourceName, "predicate.#", "1"), - resource.TestCheckResourceAttr( - resourceName, "metric_name", wafRuleName), + resource.TestCheckResourceAttr(resourceName, "name", wafRuleName), + resource.TestCheckResourceAttr(resourceName, "predicate.#", "1"), + resource.TestCheckResourceAttr(resourceName, "metric_name", wafRuleName), ), }, { Config: testAccAWSWafRegionalRuleConfigChangeName(wafRuleNewName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSWafRegionalRuleExists(resourceName, &after), - resource.TestCheckResourceAttr( - resourceName, "name", wafRuleNewName), - resource.TestCheckResourceAttr( - resourceName, "predicate.#", "1"), - resource.TestCheckResourceAttr( - resourceName, "metric_name", wafRuleNewName), + resource.TestCheckResourceAttr(resourceName, "name", wafRuleNewName), + resource.TestCheckResourceAttr(resourceName, "predicate.#", "1"), + resource.TestCheckResourceAttr(resourceName, "metric_name", wafRuleNewName), ), }, { @@ -227,10 +264,8 @@ func TestAccAWSWafRegionalRule_noPredicates(t *testing.T) { Config: testAccAWSWafRegionalRule_noPredicates(wafRuleName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSWafRegionalRuleExists(resourceName, &v), - resource.TestCheckResourceAttr( - resourceName, "name", wafRuleName), - resource.TestCheckResourceAttr( - resourceName, "predicate.#", "0"), + resource.TestCheckResourceAttr(resourceName, "name", wafRuleName), + resource.TestCheckResourceAttr(resourceName, "predicate.#", "0"), ), }, { @@ -414,7 +449,31 @@ func testAccCheckAWSWafRegionalRuleExists(n string, v *waf.Rule) resource.TestCh func testAccAWSWafRegionalRuleConfig(name string) string { return fmt.Sprintf(` resource "aws_wafregional_ipset" "ipset" { - name = "%s" + name = %[1]q + + ip_set_descriptor { + type = "IPV4" + value = "192.0.7.0/24" + } +} + +resource "aws_wafregional_rule" "wafrule" { + name = %[1]q + metric_name = %[1]q + + predicate { + data_id = "${aws_wafregional_ipset.ipset.id}" + negated = false + type = "IPMatch" + } +} +`, name) +} + +func testAccAWSWafRegionalRuleConfigTags1(name, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_wafregional_ipset" "ipset" { + name = %[1]q ip_set_descriptor { type = "IPV4" @@ -423,22 +482,55 @@ resource "aws_wafregional_ipset" "ipset" { } resource "aws_wafregional_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicate { data_id = "${aws_wafregional_ipset.ipset.id}" negated = false type = "IPMatch" } + + tags = { + %[2]q = %[3]q + } +} +`, name, tagKey1, tagValue1) +} + +func testAccAWSWafRegionalRuleConfigTags2(name, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_wafregional_ipset" "ipset" { + name = %[1]q + + ip_set_descriptor { + type = "IPV4" + value = "192.0.7.0/24" + } +} + +resource "aws_wafregional_rule" "wafrule" { + name = %[1]q + metric_name = %[1]q + + predicate { + data_id = "${aws_wafregional_ipset.ipset.id}" + negated = false + type = "IPMatch" + } + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } } -`, name, name, name) +`, name, tagKey1, tagValue1, tagKey2, tagValue2) } func testAccAWSWafRegionalRuleConfigChangeName(name string) string { return fmt.Sprintf(` resource "aws_wafregional_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptor { type = "IPV4" @@ -447,8 +539,8 @@ resource "aws_wafregional_ipset" "ipset" { } resource "aws_wafregional_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicate { data_id = "${aws_wafregional_ipset.ipset.id}" @@ -456,22 +548,22 @@ resource "aws_wafregional_rule" "wafrule" { type = "IPMatch" } } -`, name, name, name) +`, name) } func testAccAWSWafRegionalRule_noPredicates(name string) string { return fmt.Sprintf(` resource "aws_wafregional_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q } -`, name, name) +`, name) } func testAccAWSWafRegionalRule_changePredicates(name string) string { return fmt.Sprintf(` resource "aws_wafregional_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptor { type = "IPV4" @@ -480,7 +572,7 @@ resource "aws_wafregional_ipset" "ipset" { } resource "aws_wafregional_xss_match_set" "xss_match_set" { - name = "%s" + name = %[1]q xss_match_tuple { text_transformation = "NONE" @@ -492,8 +584,8 @@ resource "aws_wafregional_xss_match_set" "xss_match_set" { } resource "aws_wafregional_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicate { data_id = "${aws_wafregional_xss_match_set.xss_match_set.id}" @@ -507,5 +599,5 @@ resource "aws_wafregional_rule" "wafrule" { type = "IPMatch" } } -`, name, name, name, name) +`, name) } diff --git a/website/docs/r/wafregional_rule.html.markdown b/website/docs/r/wafregional_rule.html.markdown index 3c7eff003b7..df887bcf187 100644 --- a/website/docs/r/wafregional_rule.html.markdown +++ b/website/docs/r/wafregional_rule.html.markdown @@ -41,6 +41,7 @@ The following arguments are supported: * `name` - (Required) The name or description of the rule. * `metric_name` - (Required) The name or description for the Amazon CloudWatch metric of this rule. * `predicate` - (Optional) The objects to include in a rule (documented below). +* `tags` - (Optional) Key-value mapping of resource tags ## Nested Fields @@ -61,6 +62,7 @@ See the [WAF Documentation](https://docs.aws.amazon.com/waf/latest/APIReference/ In addition to all arguments above, the following attributes are exported: * `id` - The ID of the WAF Regional Rule. +* `arn` - The ARN of the WAF Regional Rule. ## Import