provider/aws: Allow updating predicates in WAF Rule

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)
+
+	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 Removing WAF Rule Predicates: %s", err)
+			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 {
+func updateWafRuleResource(id string, oldP, newP []interface{}, conn *waf.WAF) error {
-	conn := meta.(*AWSClient).wafconn
-
 	wr := newWafRetryer(conn, "global")
 	_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
 		req := &waf.UpdateRuleInput{
 			ChangeToken: token,
-			RuleId:      aws.String(d.Id()),
+			RuleId:      aws.String(id),
-		}
+			Updates:     diffWafRulePredicates(oldP, newP),
-
-		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)
 		}
 
 		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
+}

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)
+}