From e52043921574a7927c9378c4518085ab612a7789 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Fri, 28 Apr 2017 20:25:58 +0100 Subject: [PATCH 1/2] provider/aws: Allow updating predicates in WAF Rule --- .../providers/aws/resource_aws_waf_rule.go | 77 +++++++---- .../aws/resource_aws_waf_rule_test.go | 121 ++++++++++++++++++ 2 files changed, 175 insertions(+), 23 deletions(-) diff --git a/builtin/providers/aws/resource_aws_waf_rule.go b/builtin/providers/aws/resource_aws_waf_rule.go index 543299879..c62b889c7 100644 --- a/builtin/providers/aws/resource_aws_waf_rule.go +++ b/builtin/providers/aws/resource_aws_waf_rule.go @@ -127,7 +127,12 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error { } func resourceAwsWafRuleUpdate(d *schema.ResourceData, meta interface{}) error { - err := updateWafRuleResource(d, meta, waf.ChangeActionInsert) + conn := meta.(*AWSClient).wafconn + + o, n := d.GetChange("predicates") + oldP, newP := o.(*schema.Set).List(), n.(*schema.Set).List() + + err := updateWafRuleResource(d.Id(), oldP, newP, conn) if err != nil { return fmt.Errorf("Error Updating WAF Rule: %s", err) } @@ -136,13 +141,18 @@ func resourceAwsWafRuleUpdate(d *schema.ResourceData, meta interface{}) error { func resourceAwsWafRuleDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).wafconn - err := updateWafRuleResource(d, meta, waf.ChangeActionDelete) - if err != nil { - return fmt.Errorf("Error Removing WAF Rule Predicates: %s", err) + + oldPredicates := d.Get("predicates").(*schema.Set).List() + if len(oldPredicates) > 0 { + noPredicates := []interface{}{} + err := updateWafRuleResource(d.Id(), oldPredicates, noPredicates, conn) + if err != nil { + return fmt.Errorf("Error updating WAF Rule Predicates: %s", err) + } } wr := newWafRetryer(conn, "global") - _, err = wr.RetryWithToken(func(token *string) (interface{}, error) { + _, err := wr.RetryWithToken(func(token *string) (interface{}, error) { req := &waf.DeleteRuleInput{ ChangeToken: token, RuleId: aws.String(d.Id()), @@ -157,28 +167,13 @@ func resourceAwsWafRuleDelete(d *schema.ResourceData, meta interface{}) error { return nil } -func updateWafRuleResource(d *schema.ResourceData, meta interface{}, ChangeAction string) error { - conn := meta.(*AWSClient).wafconn - +func updateWafRuleResource(id string, oldP, newP []interface{}, conn *waf.WAF) error { wr := newWafRetryer(conn, "global") _, err := wr.RetryWithToken(func(token *string) (interface{}, error) { req := &waf.UpdateRuleInput{ ChangeToken: token, - RuleId: aws.String(d.Id()), - } - - predicatesSet := d.Get("predicates").(*schema.Set) - for _, predicateI := range predicatesSet.List() { - predicate := predicateI.(map[string]interface{}) - updatePredicate := &waf.RuleUpdate{ - Action: aws.String(ChangeAction), - Predicate: &waf.Predicate{ - Negated: aws.Bool(predicate["negated"].(bool)), - Type: aws.String(predicate["type"].(string)), - DataId: aws.String(predicate["data_id"].(string)), - }, - } - req.Updates = append(req.Updates, updatePredicate) + RuleId: aws.String(id), + Updates: diffWafRulePredicates(oldP, newP), } return conn.UpdateRule(req) @@ -189,3 +184,39 @@ func updateWafRuleResource(d *schema.ResourceData, meta interface{}, ChangeActio return nil } + +func diffWafRulePredicates(oldP, newP []interface{}) []*waf.RuleUpdate { + updates := make([]*waf.RuleUpdate, 0) + + for _, op := range oldP { + predicate := op.(map[string]interface{}) + + if idx, contains := sliceContainsMap(newP, predicate); contains { + newP = append(newP[:idx], newP[idx+1:]...) + continue + } + + updates = append(updates, &waf.RuleUpdate{ + Action: aws.String(waf.ChangeActionDelete), + Predicate: &waf.Predicate{ + Negated: aws.Bool(predicate["negated"].(bool)), + Type: aws.String(predicate["type"].(string)), + DataId: aws.String(predicate["data_id"].(string)), + }, + }) + } + + for _, np := range newP { + predicate := np.(map[string]interface{}) + + updates = append(updates, &waf.RuleUpdate{ + Action: aws.String(waf.ChangeActionInsert), + Predicate: &waf.Predicate{ + Negated: aws.Bool(predicate["negated"].(bool)), + Type: aws.String(predicate["type"].(string)), + DataId: aws.String(predicate["data_id"].(string)), + }, + }) + } + return updates +} diff --git a/builtin/providers/aws/resource_aws_waf_rule_test.go b/builtin/providers/aws/resource_aws_waf_rule_test.go index c8e6bafbf..ddf5e005d 100644 --- a/builtin/providers/aws/resource_aws_waf_rule_test.go +++ b/builtin/providers/aws/resource_aws_waf_rule_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" "github.com/aws/aws-sdk-go/aws" @@ -95,6 +96,91 @@ func TestAccAWSWafRule_disappears(t *testing.T) { }) } +func TestAccAWSWafRule_changePredicates(t *testing.T) { + var ipset waf.IPSet + var byteMatchSet waf.ByteMatchSet + + var before, after waf.Rule + var idx int + ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSWafRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSWafRuleConfig(ruleName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSWafIPSetExists("aws_waf_ipset.ipset", &ipset), + testAccCheckAWSWafRuleExists("aws_waf_rule.wafrule", &before), + resource.TestCheckResourceAttr("aws_waf_rule.wafrule", "name", ruleName), + resource.TestCheckResourceAttr("aws_waf_rule.wafrule", "predicates.#", "1"), + computeWafRulePredicateWithIpSet(&ipset, false, "IPMatch", &idx), + testCheckResourceAttrWithIndexesAddr("aws_waf_rule.wafrule", "predicates.%d.negated", &idx, "false"), + testCheckResourceAttrWithIndexesAddr("aws_waf_rule.wafrule", "predicates.%d.type", &idx, "IPMatch"), + ), + }, + { + Config: testAccAWSWafRuleConfig_changePredicates(ruleName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSWafByteMatchSetExists("aws_waf_byte_match_set.set", &byteMatchSet), + testAccCheckAWSWafRuleExists("aws_waf_rule.wafrule", &after), + resource.TestCheckResourceAttr("aws_waf_rule.wafrule", "name", ruleName), + resource.TestCheckResourceAttr("aws_waf_rule.wafrule", "predicates.#", "1"), + computeWafRulePredicateWithByteMatchSet(&byteMatchSet, true, "ByteMatch", &idx), + testCheckResourceAttrWithIndexesAddr("aws_waf_rule.wafrule", "predicates.%d.negated", &idx, "true"), + testCheckResourceAttrWithIndexesAddr("aws_waf_rule.wafrule", "predicates.%d.type", &idx, "ByteMatch"), + ), + }, + }, + }) +} + +// computeWafRulePredicateWithIpSet calculates index +// which isn't static because dataId is generated as part of the test +func computeWafRulePredicateWithIpSet(ipSet *waf.IPSet, negated bool, pType string, idx *int) resource.TestCheckFunc { + return func(s *terraform.State) error { + predicateResource := resourceAwsWafRule().Schema["predicates"].Elem.(*schema.Resource) + + m := map[string]interface{}{ + "data_id": *ipSet.IPSetId, + "negated": negated, + "type": pType, + } + + f := schema.HashResource(predicateResource) + *idx = f(m) + + return nil + } +} + +// computeWafRulePredicateWithByteMatchSet calculates index +// which isn't static because dataId is generated as part of the test +func computeWafRulePredicateWithByteMatchSet(set *waf.ByteMatchSet, negated bool, pType string, idx *int) resource.TestCheckFunc { + return func(s *terraform.State) error { + predicateResource := resourceAwsWafRule().Schema["predicates"].Elem.(*schema.Resource) + + m := map[string]interface{}{ + "data_id": *set.ByteMatchSetId, + "negated": negated, + "type": pType, + } + + f := schema.HashResource(predicateResource) + *idx = f(m) + + return nil + } +} + +func testCheckResourceAttrWithIndexesAddr(name, format string, idx *int, value string) resource.TestCheckFunc { + return func(s *terraform.State) error { + return resource.TestCheckResourceAttr(name, fmt.Sprintf(format, *idx), value)(s) + } +} + func testAccCheckAWSWafRuleDisappears(v *waf.Rule) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).wafconn @@ -241,3 +327,38 @@ resource "aws_waf_rule" "wafrule" { } }`, name, name, name) } + +func testAccAWSWafRuleConfig_changePredicates(name string) string { + return fmt.Sprintf(` +resource "aws_waf_ipset" "ipset" { + name = "%s" + ip_set_descriptors { + type = "IPV4" + value = "192.0.7.0/24" + } +} + +resource "aws_waf_byte_match_set" "set" { + name = "%s" + byte_match_tuples { + text_transformation = "NONE" + target_string = "badrefer1" + positional_constraint = "CONTAINS" + + field_to_match { + type = "HEADER" + data = "referer" + } + } +} + +resource "aws_waf_rule" "wafrule" { + name = "%s" + metric_name = "%s" + predicates { + data_id = "${aws_waf_byte_match_set.set.id}" + negated = true + type = "ByteMatch" + } +}`, name, name, name, name) +} From 73839a6e0c9852fecb86a703725ff488597ede9e Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Sat, 29 Apr 2017 08:39:18 +0100 Subject: [PATCH 2/2] provider/aws: Allow WAF Rule with no predicates --- .../providers/aws/resource_aws_waf_rule.go | 13 +++++--- .../aws/resource_aws_waf_rule_test.go | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/builtin/providers/aws/resource_aws_waf_rule.go b/builtin/providers/aws/resource_aws_waf_rule.go index c62b889c7..e7d44d7be 100644 --- a/builtin/providers/aws/resource_aws_waf_rule.go +++ b/builtin/providers/aws/resource_aws_waf_rule.go @@ -129,13 +129,16 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error { func resourceAwsWafRuleUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).wafconn - o, n := d.GetChange("predicates") - oldP, newP := o.(*schema.Set).List(), n.(*schema.Set).List() + if d.HasChange("predicates") { + o, n := d.GetChange("predicates") + oldP, newP := o.(*schema.Set).List(), n.(*schema.Set).List() - err := updateWafRuleResource(d.Id(), oldP, newP, conn) - if err != nil { - return fmt.Errorf("Error Updating WAF Rule: %s", err) + err := updateWafRuleResource(d.Id(), oldP, newP, conn) + if err != nil { + return fmt.Errorf("Error Updating WAF Rule: %s", err) + } } + return resourceAwsWafRuleRead(d, meta) } diff --git a/builtin/providers/aws/resource_aws_waf_rule_test.go b/builtin/providers/aws/resource_aws_waf_rule_test.go index ddf5e005d..456b1f5aa 100644 --- a/builtin/providers/aws/resource_aws_waf_rule_test.go +++ b/builtin/providers/aws/resource_aws_waf_rule_test.go @@ -181,6 +181,29 @@ func testCheckResourceAttrWithIndexesAddr(name, format string, idx *int, value s } } +func TestAccAWSWafRule_noPredicates(t *testing.T) { + var rule waf.Rule + ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSWafRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSWafRuleConfig_noPredicates(ruleName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSWafRuleExists("aws_waf_rule.wafrule", &rule), + resource.TestCheckResourceAttr( + "aws_waf_rule.wafrule", "name", ruleName), + resource.TestCheckResourceAttr( + "aws_waf_rule.wafrule", "predicates.#", "0"), + ), + }, + }, + }) +} + func testAccCheckAWSWafRuleDisappears(v *waf.Rule) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).wafconn @@ -362,3 +385,11 @@ resource "aws_waf_rule" "wafrule" { } }`, name, name, name, name) } + +func testAccAWSWafRuleConfig_noPredicates(name string) string { + return fmt.Sprintf(` +resource "aws_waf_rule" "wafrule" { + name = "%s" + metric_name = "%s" +}`, name, name) +}