From 41a8eb1a832c649e964861fb4bcc242d8e412892 Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Sun, 4 Aug 2019 16:00:33 +0900 Subject: [PATCH 1/2] refactor error handling --- ...esource_aws_wafregional_rate_based_rule.go | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/aws/resource_aws_wafregional_rate_based_rule.go b/aws/resource_aws_wafregional_rate_based_rule.go index 5cfc40f1148..2081786857f 100644 --- a/aws/resource_aws_wafregional_rate_based_rule.go +++ b/aws/resource_aws_wafregional_rate_based_rule.go @@ -82,7 +82,7 @@ func resourceAwsWafRegionalRateBasedRuleCreate(d *schema.ResourceData, meta inte return conn.CreateRateBasedRule(params) }) if err != nil { - return err + return fmt.Errorf("Error creating WAF Regional Rate Based Rule (%s): %s", d.Id(), err) } resp := out.(*waf.CreateRateBasedRuleOutput) d.SetId(*resp.Rule.RuleId) @@ -97,14 +97,13 @@ func resourceAwsWafRegionalRateBasedRuleRead(d *schema.ResourceData, meta interf } resp, err := conn.GetRateBasedRule(params) + if isAWSErr(err, wafregional.ErrCodeWAFNonexistentItemException, "") { + log.Printf("[WARN] WAF Regional Rate Based Rule (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } if err != nil { - if isAWSErr(err, wafregional.ErrCodeWAFNonexistentItemException, "") { - log.Printf("[WARN] WAF Regional Rate Based Rule (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - - return err + return fmt.Errorf("Error getting WAF Regional Rate Based Rule (%s): %s", d.Id(), err) } var predicates []map[string]interface{} @@ -136,8 +135,13 @@ func resourceAwsWafRegionalRateBasedRuleUpdate(d *schema.ResourceData, meta inte rateLimit := d.Get("rate_limit") err := updateWafRateBasedRuleResourceWR(d.Id(), oldP, newP, rateLimit, conn, region) + if isAWSErr(err, wafregional.ErrCodeWAFNonexistentItemException, "") { + log.Printf("[WARN] WAF Regional Rate Based Rule (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } if err != nil { - return fmt.Errorf("Error Updating WAF Rule: %s", err) + return fmt.Errorf("Error updating WAF Regional Rate Based Rule Predicates (%s): %s", d.Id(), err) } } @@ -154,8 +158,11 @@ func resourceAwsWafRegionalRateBasedRuleDelete(d *schema.ResourceData, meta inte rateLimit := d.Get("rate_limit") err := updateWafRateBasedRuleResourceWR(d.Id(), oldPredicates, noPredicates, rateLimit, conn, region) + if isAWSErr(err, wafregional.ErrCodeWAFNonexistentItemException, "") { + return nil + } if err != nil { - return fmt.Errorf("Error updating WAF Regional Rate Based Rule Predicates: %s", err) + return fmt.Errorf("Error updating WAF Regional Rate Based Rule Predicates (%s): %s", d.Id(), err) } } @@ -168,8 +175,11 @@ func resourceAwsWafRegionalRateBasedRuleDelete(d *schema.ResourceData, meta inte log.Printf("[INFO] Deleting WAF Regional Rate Based Rule") return conn.DeleteRateBasedRule(req) }) + if isAWSErr(err, wafregional.ErrCodeWAFNonexistentItemException, "") { + return nil + } if err != nil { - return fmt.Errorf("Error deleting WAF Regional Rate Based Rule: %s", err) + return fmt.Errorf("Error deleting WAF Regional Rate Based Rule (%s): %s", d.Id(), err) } return nil @@ -187,9 +197,6 @@ func updateWafRateBasedRuleResourceWR(id string, oldP, newP []interface{}, rateL return conn.UpdateRateBasedRule(req) }) - if err != nil { - return fmt.Errorf("Error Updating WAF Regional Rate Based Rule: %s", err) - } - return nil + return err } From 16712e273c87277bcc414c3b38ce30a1beabf891 Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Sun, 4 Aug 2019 16:00:57 +0900 Subject: [PATCH 2/2] Add support import for aws_wafregional_rate_based_rule resource --- ...esource_aws_wafregional_rate_based_rule.go | 3 + ...ce_aws_wafregional_rate_based_rule_test.go | 95 ++++++++++++------- .../wafregional_rate_based_rule.html.markdown | 8 ++ 3 files changed, 74 insertions(+), 32 deletions(-) diff --git a/aws/resource_aws_wafregional_rate_based_rule.go b/aws/resource_aws_wafregional_rate_based_rule.go index 2081786857f..3a6299d6a05 100644 --- a/aws/resource_aws_wafregional_rate_based_rule.go +++ b/aws/resource_aws_wafregional_rate_based_rule.go @@ -17,6 +17,9 @@ func resourceAwsWafRegionalRateBasedRule() *schema.Resource { Read: resourceAwsWafRegionalRateBasedRuleRead, Update: resourceAwsWafRegionalRateBasedRuleUpdate, Delete: resourceAwsWafRegionalRateBasedRuleDelete, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, Schema: map[string]*schema.Schema{ "name": { diff --git a/aws/resource_aws_wafregional_rate_based_rule_test.go b/aws/resource_aws_wafregional_rate_based_rule_test.go index 3155d5e489f..d3acfbbf93e 100644 --- a/aws/resource_aws_wafregional_rate_based_rule_test.go +++ b/aws/resource_aws_wafregional_rate_based_rule_test.go @@ -119,6 +119,7 @@ func testSweepWafRegionalRateBasedRules(region string) error { func TestAccAWSWafRegionalRateBasedRule_basic(t *testing.T) { var v waf.RateBasedRule + resourceName := "aws_wafregional_rate_based_rule.wafrule" wafRuleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -128,21 +129,27 @@ func TestAccAWSWafRegionalRateBasedRule_basic(t *testing.T) { { Config: testAccAWSWafRegionalRateBasedRuleConfig(wafRuleName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &v), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &v), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "name", wafRuleName), + resourceName, "name", wafRuleName), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "predicate.#", "1"), + resourceName, "predicate.#", "1"), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "metric_name", wafRuleName), + resourceName, "metric_name", wafRuleName), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } func TestAccAWSWafRegionalRateBasedRule_changeNameForceNew(t *testing.T) { var before, after waf.RateBasedRule + resourceName := "aws_wafregional_rate_based_rule.wafrule" wafRuleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) wafRuleNewName := fmt.Sprintf("wafrulenew%s", acctest.RandString(5)) @@ -154,34 +161,40 @@ func TestAccAWSWafRegionalRateBasedRule_changeNameForceNew(t *testing.T) { { Config: testAccAWSWafRegionalRateBasedRuleConfig(wafRuleName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &before), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &before), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "name", wafRuleName), + resourceName, "name", wafRuleName), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "predicate.#", "1"), + resourceName, "predicate.#", "1"), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "metric_name", wafRuleName), + resourceName, "metric_name", wafRuleName), ), }, { Config: testAccAWSWafRegionalRateBasedRuleConfigChangeName(wafRuleNewName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &after), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &after), testAccCheckAWSWafRateBasedRuleIdDiffers(&before, &after), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "name", wafRuleNewName), + resourceName, "name", wafRuleNewName), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "predicate.#", "1"), + resourceName, "predicate.#", "1"), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "metric_name", wafRuleNewName), + resourceName, "metric_name", wafRuleNewName), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } func TestAccAWSWafRegionalRateBasedRule_disappears(t *testing.T) { var v waf.RateBasedRule + resourceName := "aws_wafregional_rate_based_rule.wafrule" wafRuleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -191,7 +204,7 @@ func TestAccAWSWafRegionalRateBasedRule_disappears(t *testing.T) { { Config: testAccAWSWafRegionalRateBasedRuleConfig(wafRuleName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &v), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &v), testAccCheckAWSWafRegionalRateBasedRuleDisappears(&v), ), ExpectNonEmptyPlan: true, @@ -206,6 +219,7 @@ func TestAccAWSWafRegionalRateBasedRule_changePredicates(t *testing.T) { var before, after waf.RateBasedRule var idx int + resourceName := "aws_wafregional_rate_based_rule.wafrule" ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) resource.ParallelTest(t, resource.TestCase{ @@ -217,32 +231,38 @@ func TestAccAWSWafRegionalRateBasedRule_changePredicates(t *testing.T) { Config: testAccAWSWafRegionalRateBasedRuleConfig(ruleName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSWafRegionalIPSetExists("aws_wafregional_ipset.ipset", &ipset), - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &before), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "name", ruleName), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "predicate.#", "1"), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &before), + resource.TestCheckResourceAttr(resourceName, "name", ruleName), + resource.TestCheckResourceAttr(resourceName, "predicate.#", "1"), computeWafRegionalRateBasedRulePredicateWithIpSet(&ipset, false, "IPMatch", &idx), - testCheckResourceAttrWithIndexesAddr("aws_wafregional_rate_based_rule.wafrule", "predicate.%d.negated", &idx, "false"), - testCheckResourceAttrWithIndexesAddr("aws_wafregional_rate_based_rule.wafrule", "predicate.%d.type", &idx, "IPMatch"), + testCheckResourceAttrWithIndexesAddr(resourceName, "predicate.%d.negated", &idx, "false"), + testCheckResourceAttrWithIndexesAddr(resourceName, "predicate.%d.type", &idx, "IPMatch"), ), }, { Config: testAccAWSWafRegionalRateBasedRuleConfig_changePredicates(ruleName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSWafRegionalByteMatchSetExists("aws_wafregional_byte_match_set.set", &byteMatchSet), - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &after), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "name", ruleName), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "predicate.#", "1"), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &after), + resource.TestCheckResourceAttr(resourceName, "name", ruleName), + resource.TestCheckResourceAttr(resourceName, "predicate.#", "1"), computeWafRegionalRateBasedRulePredicateWithByteMatchSet(&byteMatchSet, true, "ByteMatch", &idx), - testCheckResourceAttrWithIndexesAddr("aws_wafregional_rate_based_rule.wafrule", "predicate.%d.negated", &idx, "true"), - testCheckResourceAttrWithIndexesAddr("aws_wafregional_rate_based_rule.wafrule", "predicate.%d.type", &idx, "ByteMatch"), + testCheckResourceAttrWithIndexesAddr(resourceName, "predicate.%d.negated", &idx, "true"), + testCheckResourceAttrWithIndexesAddr(resourceName, "predicate.%d.type", &idx, "ByteMatch"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } func TestAccAWSWafRegionalRateBasedRule_changeRateLimit(t *testing.T) { var before, after waf.RateBasedRule + resourceName := "aws_wafregional_rate_based_rule.wafrule" ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) rateLimitBefore := "2000" rateLimitAfter := "2001" @@ -255,19 +275,24 @@ func TestAccAWSWafRegionalRateBasedRule_changeRateLimit(t *testing.T) { { Config: testAccAWSWafRegionalRateBasedRuleWithRateLimitConfig(ruleName, rateLimitBefore), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &before), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "name", ruleName), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "rate_limit", rateLimitBefore), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &before), + resource.TestCheckResourceAttr(resourceName, "name", ruleName), + resource.TestCheckResourceAttr(resourceName, "rate_limit", rateLimitBefore), ), }, { Config: testAccAWSWafRegionalRateBasedRuleWithRateLimitConfig(ruleName, rateLimitAfter), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &after), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "name", ruleName), - resource.TestCheckResourceAttr("aws_wafregional_rate_based_rule.wafrule", "rate_limit", rateLimitAfter), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &after), + resource.TestCheckResourceAttr(resourceName, "name", ruleName), + resource.TestCheckResourceAttr(resourceName, "rate_limit", rateLimitAfter), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -312,6 +337,7 @@ func computeWafRegionalRateBasedRulePredicateWithByteMatchSet(set *waf.ByteMatch func TestAccAWSWafRegionalRateBasedRule_noPredicates(t *testing.T) { var rule waf.RateBasedRule + resourceName := "aws_wafregional_rate_based_rule.wafrule" ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) resource.ParallelTest(t, resource.TestCase{ @@ -322,13 +348,18 @@ func TestAccAWSWafRegionalRateBasedRule_noPredicates(t *testing.T) { { Config: testAccAWSWafRegionalRateBasedRuleConfig_noPredicates(ruleName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckAWSWafRegionalRateBasedRuleExists("aws_wafregional_rate_based_rule.wafrule", &rule), + testAccCheckAWSWafRegionalRateBasedRuleExists(resourceName, &rule), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "name", ruleName), + resourceName, "name", ruleName), resource.TestCheckResourceAttr( - "aws_wafregional_rate_based_rule.wafrule", "predicate.#", "0"), + resourceName, "predicate.#", "0"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } diff --git a/website/docs/r/wafregional_rate_based_rule.html.markdown b/website/docs/r/wafregional_rate_based_rule.html.markdown index af55a94c142..dc33a7fe0d3 100644 --- a/website/docs/r/wafregional_rate_based_rule.html.markdown +++ b/website/docs/r/wafregional_rate_based_rule.html.markdown @@ -68,3 +68,11 @@ 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 rate based rule. + +## Import + +WAF Regional Rate Based Rule can be imported using the id, e.g. + +``` +$ terraform import aws_wafregional_rate_based_rule.wafrule a1b2c3d4-d5f6-7777-8888-9999aaaabbbbcccc +``` \ No newline at end of file