From cbabb37c659a725a2bd0267df1afa284fc342abc Mon Sep 17 00:00:00 2001
From: Brian Flad <bflad417@gmail.com>
Date: Mon, 30 Jul 2018 09:54:18 -0400
Subject: [PATCH] resource/aws_waf_web_acl: Properly update rules

This also includes other function refactoring to make the WAF and WAF Regional Web ACL resources behave similarly.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSWaf\(Regional\)\?WebAcl_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSWaf\(Regional\)\?WebAcl_ -timeout 120m
=== RUN   TestAccAWSWafWebAcl_basic
--- PASS: TestAccAWSWafWebAcl_basic (14.46s)
=== RUN   TestAccAWSWafWebAcl_changeNameForceNew
--- PASS: TestAccAWSWafWebAcl_changeNameForceNew (22.92s)
=== RUN   TestAccAWSWafWebAcl_DefaultAction
--- PASS: TestAccAWSWafWebAcl_DefaultAction (20.89s)
=== RUN   TestAccAWSWafWebAcl_Rules
--- PASS: TestAccAWSWafWebAcl_Rules (41.19s)
=== RUN   TestAccAWSWafWebAcl_disappears
--- PASS: TestAccAWSWafWebAcl_disappears (10.02s)
=== RUN   TestAccAWSWafRegionalWebAcl_basic
--- PASS: TestAccAWSWafRegionalWebAcl_basic (21.19s)
=== RUN   TestAccAWSWafRegionalWebAcl_createRateBased
--- PASS: TestAccAWSWafRegionalWebAcl_createRateBased (22.72s)
=== RUN   TestAccAWSWafRegionalWebAcl_createGroup
--- PASS: TestAccAWSWafRegionalWebAcl_createGroup (23.46s)
=== RUN   TestAccAWSWafRegionalWebAcl_changeNameForceNew
--- PASS: TestAccAWSWafRegionalWebAcl_changeNameForceNew (42.56s)
=== RUN   TestAccAWSWafRegionalWebAcl_changeDefaultAction
--- PASS: TestAccAWSWafRegionalWebAcl_changeDefaultAction (41.41s)
=== RUN   TestAccAWSWafRegionalWebAcl_disappears
--- PASS: TestAccAWSWafRegionalWebAcl_disappears (21.20s)
=== RUN   TestAccAWSWafRegionalWebAcl_noRules
--- PASS: TestAccAWSWafRegionalWebAcl_noRules (17.18s)
=== RUN   TestAccAWSWafRegionalWebAcl_changeRules
--- PASS: TestAccAWSWafRegionalWebAcl_changeRules (33.23s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	333.027s
---
 aws/resource_aws_waf_web_acl.go         | 148 +++-------
 aws/resource_aws_waf_web_acl_test.go    | 377 +++++++++++-------------
 aws/resource_aws_wafregional_web_acl.go | 169 +++--------
 aws/structure.go                        | 109 +++++++
 4 files changed, 366 insertions(+), 437 deletions(-)

diff --git a/aws/resource_aws_waf_web_acl.go b/aws/resource_aws_waf_web_acl.go
index af7203eb7bd..a4c130bd13b 100644
--- a/aws/resource_aws_waf_web_acl.go
+++ b/aws/resource_aws_waf_web_acl.go
@@ -108,7 +108,7 @@ func resourceAwsWafWebAclCreate(d *schema.ResourceData, meta interface{}) error
 	out, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
 		params := &waf.CreateWebACLInput{
 			ChangeToken:   token,
-			DefaultAction: expandDefaultAction(d),
+			DefaultAction: expandWafAction(d.Get("default_action").(*schema.Set).List()),
 			MetricName:    aws.String(d.Get("metric_name").(string)),
 			Name:          aws.String(d.Get("name").(string)),
 		}
@@ -140,11 +140,14 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
-	defaultAction := flattenDefaultAction(resp.WebACL.DefaultAction)
-	if defaultAction != nil {
-		if err := d.Set("default_action", defaultAction); err != nil {
-			return fmt.Errorf("error setting default_action: %s", err)
-		}
+	if resp == nil || resp.WebACL == nil {
+		log.Printf("[WARN] WAF ACL (%s) not found, removing from state", d.Id())
+		d.SetId("")
+		return nil
+	}
+
+	if err := d.Set("default_action", flattenWafAction(resp.WebACL.DefaultAction)); err != nil {
+		return fmt.Errorf("error setting default_action: %s", err)
 	}
 	d.Set("name", resp.WebACL.Name)
 	d.Set("metric_name", resp.WebACL.MetricName)
@@ -156,22 +159,53 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error {
 }
 
 func resourceAwsWafWebAclUpdate(d *schema.ResourceData, meta interface{}) error {
-	err := updateWebAclResource(d, meta, waf.ChangeActionInsert)
-	if err != nil {
-		return fmt.Errorf("Error Updating WAF ACL: %s", err)
+	conn := meta.(*AWSClient).wafconn
+
+	if d.HasChange("default_action") || d.HasChange("rules") {
+		o, n := d.GetChange("rules")
+		oldR, newR := o.(*schema.Set).List(), n.(*schema.Set).List()
+
+		wr := newWafRetryer(conn, "global")
+		_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
+			req := &waf.UpdateWebACLInput{
+				ChangeToken:   token,
+				DefaultAction: expandWafAction(d.Get("default_action").(*schema.Set).List()),
+				Updates:       diffWafWebAclRules(oldR, newR),
+				WebACLId:      aws.String(d.Id()),
+			}
+			return conn.UpdateWebACL(req)
+		})
+		if err != nil {
+			return fmt.Errorf("Error Updating WAF ACL: %s", err)
+		}
 	}
+
 	return resourceAwsWafWebAclRead(d, meta)
 }
 
 func resourceAwsWafWebAclDelete(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).wafconn
-	err := updateWebAclResource(d, meta, waf.ChangeActionDelete)
-	if err != nil {
-		return fmt.Errorf("Error Removing WAF ACL Rules: %s", err)
+
+	// First, need to delete all rules
+	rules := d.Get("rules").(*schema.Set).List()
+	if len(rules) > 0 {
+		wr := newWafRetryer(conn, "global")
+		_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
+			req := &waf.UpdateWebACLInput{
+				ChangeToken:   token,
+				DefaultAction: expandWafAction(d.Get("default_action").(*schema.Set).List()),
+				Updates:       diffWafWebAclRules(rules, []interface{}{}),
+				WebACLId:      aws.String(d.Id()),
+			}
+			return conn.UpdateWebACL(req)
+		})
+		if err != nil {
+			return fmt.Errorf("Error Removing WAF Regional ACL Rules: %s", err)
+		}
 	}
 
 	wr := newWafRetryer(conn, "global")
-	_, err = wr.RetryWithToken(func(token *string) (interface{}, error) {
+	_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
 		req := &waf.DeleteWebACLInput{
 			ChangeToken: token,
 			WebACLId:    aws.String(d.Id()),
@@ -185,91 +219,3 @@ func resourceAwsWafWebAclDelete(d *schema.ResourceData, meta interface{}) error
 	}
 	return nil
 }
-
-func updateWebAclResource(d *schema.ResourceData, meta interface{}, ChangeAction string) error {
-	conn := meta.(*AWSClient).wafconn
-
-	wr := newWafRetryer(conn, "global")
-	_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
-		req := &waf.UpdateWebACLInput{
-			ChangeToken: token,
-			WebACLId:    aws.String(d.Id()),
-		}
-
-		if d.HasChange("default_action") {
-			req.DefaultAction = expandDefaultAction(d)
-		}
-
-		rules := d.Get("rules").(*schema.Set)
-		for _, rule := range rules.List() {
-			aclRule := rule.(map[string]interface{})
-
-			var aclRuleUpdate *waf.WebACLUpdate
-			switch aclRule["type"].(string) {
-			case waf.WafRuleTypeGroup:
-				overrideAction := aclRule["override_action"].([]interface{})[0].(map[string]interface{})
-				aclRuleUpdate = &waf.WebACLUpdate{
-					Action: aws.String(ChangeAction),
-					ActivatedRule: &waf.ActivatedRule{
-						Priority:       aws.Int64(int64(aclRule["priority"].(int))),
-						RuleId:         aws.String(aclRule["rule_id"].(string)),
-						Type:           aws.String(aclRule["type"].(string)),
-						OverrideAction: &waf.WafOverrideAction{Type: aws.String(overrideAction["type"].(string))},
-					},
-				}
-			default:
-				action := aclRule["action"].([]interface{})[0].(map[string]interface{})
-				aclRuleUpdate = &waf.WebACLUpdate{
-					Action: aws.String(ChangeAction),
-					ActivatedRule: &waf.ActivatedRule{
-						Priority: aws.Int64(int64(aclRule["priority"].(int))),
-						RuleId:   aws.String(aclRule["rule_id"].(string)),
-						Type:     aws.String(aclRule["type"].(string)),
-						Action:   &waf.WafAction{Type: aws.String(action["type"].(string))},
-					},
-				}
-			}
-
-			req.Updates = append(req.Updates, aclRuleUpdate)
-		}
-		return conn.UpdateWebACL(req)
-	})
-	if err != nil {
-		return fmt.Errorf("Error Updating WAF ACL: %s", err)
-	}
-	return nil
-}
-
-func expandDefaultAction(d *schema.ResourceData) *waf.WafAction {
-	set, ok := d.GetOk("default_action")
-	if !ok {
-		return nil
-	}
-
-	s := set.(*schema.Set).List()
-	if s == nil || len(s) == 0 {
-		return nil
-	}
-
-	if s[0] == nil {
-		log.Printf("[ERR] First element of Default Action is set to nil")
-		return nil
-	}
-
-	dA := s[0].(map[string]interface{})
-
-	return &waf.WafAction{
-		Type: aws.String(dA["type"].(string)),
-	}
-}
-
-func flattenDefaultAction(n *waf.WafAction) []map[string]interface{} {
-	if n == nil {
-		return nil
-	}
-
-	m := setMap(make(map[string]interface{}))
-
-	m.SetString("type", n.Type)
-	return m.MapList()
-}
diff --git a/aws/resource_aws_waf_web_acl_test.go b/aws/resource_aws_waf_web_acl_test.go
index 0e1ba7eefb5..a34314f4293 100644
--- a/aws/resource_aws_waf_web_acl_test.go
+++ b/aws/resource_aws_waf_web_acl_test.go
@@ -4,18 +4,17 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/hashicorp/terraform/helper/resource"
-	"github.com/hashicorp/terraform/terraform"
-
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/waf"
 	"github.com/hashicorp/terraform/helper/acctest"
+	"github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/terraform"
 )
 
 func TestAccAWSWafWebAcl_basic(t *testing.T) {
-	var v waf.WebACL
-	wafAclName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
-	resourceName := "aws_waf_web_acl.waf_acl"
+	var webACL waf.WebACL
+	rName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
+	resourceName := "aws_waf_web_acl.test"
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -23,19 +22,14 @@ func TestAccAWSWafWebAcl_basic(t *testing.T) {
 		CheckDestroy: testAccCheckAWSWafWebAclDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSWafWebAclConfig(wafAclName),
+				Config: testAccAWSWafWebAclConfig_Required(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &v),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.4234791575.type", "ALLOW"),
-					resource.TestCheckResourceAttr(
-						resourceName, "name", wafAclName),
-					resource.TestCheckResourceAttr(
-						resourceName, "rules.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "metric_name", wafAclName),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "default_action.4234791575.type", "ALLOW"),
+					resource.TestCheckResourceAttr(resourceName, "metric_name", rName),
+					resource.TestCheckResourceAttr(resourceName, "name", rName),
+					resource.TestCheckResourceAttr(resourceName, "rules.#", "0"),
 				),
 			},
 			{
@@ -47,30 +41,37 @@ func TestAccAWSWafWebAcl_basic(t *testing.T) {
 	})
 }
 
-func TestAccAWSWafWebAcl_group(t *testing.T) {
-	var v waf.WebACL
-	wafAclName := fmt.Sprintf("wafaclgroup%s", acctest.RandString(5))
-	resourceName := "aws_waf_web_acl.waf_acl"
+func TestAccAWSWafWebAcl_changeNameForceNew(t *testing.T) {
+	var webACL waf.WebACL
+	rName1 := fmt.Sprintf("wafacl%s", acctest.RandString(5))
+	rName2 := fmt.Sprintf("wafacl%s", acctest.RandString(5))
+	resourceName := "aws_waf_web_acl.test"
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSWafWebAclDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
-				Config: testAccAWSWafWebAclGroupConfig(wafAclName),
+			{
+				Config: testAccAWSWafWebAclConfig_Required(rName1),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "default_action.4234791575.type", "ALLOW"),
+					resource.TestCheckResourceAttr(resourceName, "metric_name", rName1),
+					resource.TestCheckResourceAttr(resourceName, "name", rName1),
+					resource.TestCheckResourceAttr(resourceName, "rules.#", "0"),
+				),
+			},
+			{
+				Config: testAccAWSWafWebAclConfig_Required(rName2),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &v),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.4234791575.type", "ALLOW"),
-					resource.TestCheckResourceAttr(
-						resourceName, "name", wafAclName),
-					resource.TestCheckResourceAttr(
-						resourceName, "rules.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "metric_name", wafAclName),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "default_action.4234791575.type", "ALLOW"),
+					resource.TestCheckResourceAttr(resourceName, "metric_name", rName2),
+					resource.TestCheckResourceAttr(resourceName, "name", rName2),
+					resource.TestCheckResourceAttr(resourceName, "rules.#", "0"),
 				),
 			},
 			{
@@ -82,11 +83,10 @@ func TestAccAWSWafWebAcl_group(t *testing.T) {
 	})
 }
 
-func TestAccAWSWafWebAcl_changeNameForceNew(t *testing.T) {
-	var before, after waf.WebACL
-	wafAclName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
-	wafAclNewName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
-	resourceName := "aws_waf_web_acl.waf_acl"
+func TestAccAWSWafWebAcl_DefaultAction(t *testing.T) {
+	var webACL waf.WebACL
+	rName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
+	resourceName := "aws_waf_web_acl.test"
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -94,35 +94,19 @@ func TestAccAWSWafWebAcl_changeNameForceNew(t *testing.T) {
 		CheckDestroy: testAccCheckAWSWafWebAclDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSWafWebAclConfig(wafAclName),
+				Config: testAccAWSWafWebAclConfig_DefaultAction(rName, "ALLOW"),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &before),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.4234791575.type", "ALLOW"),
-					resource.TestCheckResourceAttr(
-						resourceName, "name", wafAclName),
-					resource.TestCheckResourceAttr(
-						resourceName, "rules.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "metric_name", wafAclName),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "default_action.4234791575.type", "ALLOW"),
 				),
 			},
 			{
-				Config: testAccAWSWafWebAclConfigChangeName(wafAclNewName),
+				Config: testAccAWSWafWebAclConfig_DefaultAction(rName, "BLOCK"),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &after),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.4234791575.type", "ALLOW"),
-					resource.TestCheckResourceAttr(
-						resourceName, "name", wafAclNewName),
-					resource.TestCheckResourceAttr(
-						resourceName, "rules.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "metric_name", wafAclNewName),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "default_action.2267395054.type", "BLOCK"),
 				),
 			},
 			{
@@ -134,49 +118,41 @@ func TestAccAWSWafWebAcl_changeNameForceNew(t *testing.T) {
 	})
 }
 
-func TestAccAWSWafWebAcl_changeDefaultAction(t *testing.T) {
-	var before, after waf.WebACL
-	wafAclName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
-	wafAclNewName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
-	resourceName := "aws_waf_web_acl.waf_acl"
+func TestAccAWSWafWebAcl_Rules(t *testing.T) {
+	var webACL waf.WebACL
+	rName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
+	resourceName := "aws_waf_web_acl.test"
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSWafWebAclDestroy,
 		Steps: []resource.TestStep{
+			// Test creating with rule
+			{
+				Config: testAccAWSWafWebAclConfig_Rules_Single_Rule(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "rules.#", "1"),
+				),
+			},
+			// Test adding rule
 			{
-				Config: testAccAWSWafWebAclConfig(wafAclName),
+				Config: testAccAWSWafWebAclConfig_Rules_Multiple(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &before),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.4234791575.type", "ALLOW"),
-					resource.TestCheckResourceAttr(
-						resourceName, "name", wafAclName),
-					resource.TestCheckResourceAttr(
-						resourceName, "rules.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "metric_name", wafAclName),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
 				),
 			},
+			// Test removing rule
 			{
-				Config: testAccAWSWafWebAclConfigDefaultAction(wafAclNewName),
+				Config: testAccAWSWafWebAclConfig_Rules_Single_RuleGroup(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &after),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "default_action.2267395054.type", "BLOCK"),
-					resource.TestCheckResourceAttr(
-						resourceName, "name", wafAclNewName),
-					resource.TestCheckResourceAttr(
-						resourceName, "rules.#", "1"),
-					resource.TestCheckResourceAttr(
-						resourceName, "metric_name", wafAclNewName),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					resource.TestCheckResourceAttr(resourceName, "rules.#", "1"),
 				),
 			},
+			// Test import
 			{
 				ResourceName:      resourceName,
 				ImportState:       true,
@@ -187,9 +163,9 @@ func TestAccAWSWafWebAcl_changeDefaultAction(t *testing.T) {
 }
 
 func TestAccAWSWafWebAcl_disappears(t *testing.T) {
-	var v waf.WebACL
-	wafAclName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
-	resourceName := "aws_waf_web_acl.waf_acl"
+	var webACL waf.WebACL
+	rName := fmt.Sprintf("wafacl%s", acctest.RandString(5))
+	resourceName := "aws_waf_web_acl.test"
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -197,10 +173,10 @@ func TestAccAWSWafWebAcl_disappears(t *testing.T) {
 		CheckDestroy: testAccCheckAWSWafWebAclDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSWafWebAclConfig(wafAclName),
+				Config: testAccAWSWafWebAclConfig_Required(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSWafWebAclExists(resourceName, &v),
-					testAccCheckAWSWafWebAclDisappears(&v),
+					testAccCheckAWSWafWebAclExists(resourceName, &webACL),
+					testAccCheckAWSWafWebAclDisappears(&webACL),
 				),
 				ExpectNonEmptyPlan: true,
 			},
@@ -213,31 +189,8 @@ func testAccCheckAWSWafWebAclDisappears(v *waf.WebACL) resource.TestCheckFunc {
 		conn := testAccProvider.Meta().(*AWSClient).wafconn
 
 		wr := newWafRetryer(conn, "global")
-		_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
-			req := &waf.UpdateWebACLInput{
-				ChangeToken: token,
-				WebACLId:    v.WebACLId,
-			}
-
-			for _, ActivatedRule := range v.Rules {
-				WebACLUpdate := &waf.WebACLUpdate{
-					Action: aws.String("DELETE"),
-					ActivatedRule: &waf.ActivatedRule{
-						Priority: ActivatedRule.Priority,
-						RuleId:   ActivatedRule.RuleId,
-						Action:   ActivatedRule.Action,
-					},
-				}
-				req.Updates = append(req.Updates, WebACLUpdate)
-			}
 
-			return conn.UpdateWebACL(req)
-		})
-		if err != nil {
-			return fmt.Errorf("Error Updating WAF ACL: %s", err)
-		}
-
-		_, err = wr.RetryWithToken(func(token *string) (interface{}, error) {
+		_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
 			opts := &waf.DeleteWebACLInput{
 				ChangeToken: token,
 				WebACLId:    v.WebACLId,
@@ -309,135 +262,155 @@ func testAccCheckAWSWafWebAclExists(n string, v *waf.WebACL) resource.TestCheckF
 	}
 }
 
-func testAccAWSWafWebAclConfig(name string) string {
-	return fmt.Sprintf(`resource "aws_waf_ipset" "ipset" {
-  name = "%s"
+func testAccAWSWafWebAclConfig_Required(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_waf_web_acl" "test" {
+  metric_name = %q
+  name        = %q
+
+  default_action {
+    type = "ALLOW"
+  }
+}
+`, rName, rName)
+}
+
+func testAccAWSWafWebAclConfig_DefaultAction(rName, defaultAction string) string {
+	return fmt.Sprintf(`
+resource "aws_waf_web_acl" "test" {
+  metric_name = %q
+  name        = %q
+
+  default_action {
+    type = %q
+  }
+}
+`, rName, rName, defaultAction)
+}
+
+func testAccAWSWafWebAclConfig_Rules_Single_Rule(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_waf_ipset" "test" {
+  name = %q
+
   ip_set_descriptors {
-    type = "IPV4"
+    type  = "IPV4"
     value = "192.0.7.0/24"
   }
 }
 
-resource "aws_waf_rule" "wafrule" {
-  depends_on = ["aws_waf_ipset.ipset"]
-  name = "%s"
-  metric_name = "%s"
+resource "aws_waf_rule" "test" {
+  metric_name = %q
+  name        = %q
+
   predicates {
-    data_id = "${aws_waf_ipset.ipset.id}"
+    data_id = "${aws_waf_ipset.test.id}"
     negated = false
-    type = "IPMatch"
+    type    = "IPMatch"
   }
 }
-resource "aws_waf_web_acl" "waf_acl" {
-  depends_on = ["aws_waf_ipset.ipset", "aws_waf_rule.wafrule"]
-  name = "%s"
-  metric_name = "%s"
+
+resource "aws_waf_web_acl" "test" {
+  metric_name = %q
+  name        = %q
+
   default_action {
     type = "ALLOW"
   }
+
   rules {
+    priority = 1
+    rule_id  = "${aws_waf_rule.test.id}"
+
     action {
        type = "BLOCK"
     }
-    priority = 1 
-    rule_id = "${aws_waf_rule.wafrule.id}"
   }
-}`, name, name, name, name, name)
+}
+`, rName, rName, rName, rName, rName)
 }
 
-func testAccAWSWafWebAclGroupConfig(name string) string {
+func testAccAWSWafWebAclConfig_Rules_Single_RuleGroup(rName string) string {
 	return fmt.Sprintf(`
-
-resource "aws_waf_rule_group" "wafrulegroup" {
-  name = "%s"
-  metric_name = "%s"
+resource "aws_waf_rule_group" "test" {
+  metric_name = %q
+  name        = %q
 }
-resource "aws_waf_web_acl" "waf_acl" {
-  depends_on = ["aws_waf_rule_group.wafrulegroup"]
-  name = "%s"
-  metric_name = "%s"
+
+resource "aws_waf_web_acl" "test" {
+  metric_name = %q
+  name        = %q
+
   default_action {
     type = "ALLOW"
   }
+
   rules {
+    priority = 1
+    rule_id  = "${aws_waf_rule_group.test.id}"
+    type     = "GROUP"
+
     override_action {
        type = "NONE"
     }
-    type = "GROUP"
-    priority = 1
-    rule_id = "${aws_waf_rule_group.wafrulegroup.id}"
   }
-}`, name, name, name, name)
 }
+`, rName, rName, rName, rName)
+}
+
+func testAccAWSWafWebAclConfig_Rules_Multiple(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_waf_ipset" "test" {
+  name = %q
 
-func testAccAWSWafWebAclConfigChangeName(name string) string {
-	return fmt.Sprintf(`resource "aws_waf_ipset" "ipset" {
-  name = "%s"
   ip_set_descriptors {
-    type = "IPV4"
+    type  = "IPV4"
     value = "192.0.7.0/24"
   }
 }
 
-resource "aws_waf_rule" "wafrule" {
-  depends_on = ["aws_waf_ipset.ipset"]
-  name = "%s"
-  metric_name = "%s"
+resource "aws_waf_rule" "test" {
+  metric_name = %q
+  name        = %q
+
   predicates {
-    data_id = "${aws_waf_ipset.ipset.id}"
+    data_id = "${aws_waf_ipset.test.id}"
     negated = false
-    type = "IPMatch"
+    type    = "IPMatch"
   }
 }
-resource "aws_waf_web_acl" "waf_acl" {
-  depends_on = ["aws_waf_ipset.ipset", "aws_waf_rule.wafrule"]
-  name = "%s"
-  metric_name = "%s"
+
+resource "aws_waf_rule_group" "test" {
+  metric_name = %q
+  name        = %q
+}
+
+resource "aws_waf_web_acl" "test" {
+  metric_name = %q
+  name        = %q
+
   default_action {
     type = "ALLOW"
   }
+
   rules {
+    priority = 1
+    rule_id  = "${aws_waf_rule.test.id}"
+
     action {
        type = "BLOCK"
     }
-    priority = 1 
-    rule_id = "${aws_waf_rule.wafrule.id}"
   }
-}`, name, name, name, name, name)
-}
 
-func testAccAWSWafWebAclConfigDefaultAction(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_rule" "wafrule" {
-  depends_on = ["aws_waf_ipset.ipset"]
-  name = "%s"
-  metric_name = "%s"
-  predicates {
-    data_id = "${aws_waf_ipset.ipset.id}"
-    negated = false
-    type = "IPMatch"
-  }
-}
-resource "aws_waf_web_acl" "waf_acl" {
-  depends_on = ["aws_waf_ipset.ipset", "aws_waf_rule.wafrule"]
-  name = "%s"
-  metric_name = "%s"
-  default_action {
-    type = "BLOCK"
-  }
   rules {
-    action {
-       type = "BLOCK"
+    priority = 2
+    rule_id  = "${aws_waf_rule_group.test.id}"
+    type     = "GROUP"
+
+    override_action {
+       type = "NONE"
     }
-    priority = 1 
-    rule_id = "${aws_waf_rule.wafrule.id}"
   }
-}`, name, name, name, name, name)
+}
+`, rName, rName, rName, rName, rName, rName, rName)
 }
diff --git a/aws/resource_aws_wafregional_web_acl.go b/aws/resource_aws_wafregional_web_acl.go
index c9f0f38b3e6..6389d84a549 100644
--- a/aws/resource_aws_wafregional_web_acl.go
+++ b/aws/resource_aws_wafregional_web_acl.go
@@ -106,7 +106,7 @@ func resourceAwsWafRegionalWebAclCreate(d *schema.ResourceData, meta interface{}
 	out, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
 		params := &waf.CreateWebACLInput{
 			ChangeToken:   token,
-			DefaultAction: expandDefaultActionWR(d.Get("default_action").([]interface{})),
+			DefaultAction: expandWafAction(d.Get("default_action").([]interface{})),
 			MetricName:    aws.String(d.Get("metric_name").(string)),
 			Name:          aws.String(d.Get("name").(string)),
 		}
@@ -130,7 +130,7 @@ func resourceAwsWafRegionalWebAclRead(d *schema.ResourceData, meta interface{})
 	resp, err := conn.GetWebACL(params)
 	if err != nil {
 		if isAWSErr(err, wafregional.ErrCodeWAFNonexistentItemException, "") {
-			log.Printf("[WARN] WAF Regional ACL (%s) not found, error code (404)", d.Id())
+			log.Printf("[WARN] WAF Regional ACL (%s) not found, removing from state", d.Id())
 			d.SetId("")
 			return nil
 		}
@@ -138,7 +138,15 @@ func resourceAwsWafRegionalWebAclRead(d *schema.ResourceData, meta interface{})
 		return err
 	}
 
-	d.Set("default_action", flattenDefaultActionWR(resp.WebACL.DefaultAction))
+	if resp == nil || resp.WebACL == nil {
+		log.Printf("[WARN] WAF Regional ACL (%s) not found, removing from state", d.Id())
+		d.SetId("")
+		return nil
+	}
+
+	if err := d.Set("default_action", flattenWafAction(resp.WebACL.DefaultAction)); err != nil {
+		return fmt.Errorf("error setting default_action: %s", err)
+	}
 	d.Set("name", resp.WebACL.Name)
 	d.Set("metric_name", resp.WebACL.MetricName)
 	if err := d.Set("rule", flattenWafWebAclRules(resp.WebACL.Rules)); err != nil {
@@ -149,15 +157,23 @@ func resourceAwsWafRegionalWebAclRead(d *schema.ResourceData, meta interface{})
 }
 
 func resourceAwsWafRegionalWebAclUpdate(d *schema.ResourceData, meta interface{}) error {
-	if d.HasChange("default_action") || d.HasChange("rule") {
-		conn := meta.(*AWSClient).wafregionalconn
-		region := meta.(*AWSClient).region
+	conn := meta.(*AWSClient).wafregionalconn
+	region := meta.(*AWSClient).region
 
-		action := expandDefaultActionWR(d.Get("default_action").([]interface{}))
+	if d.HasChange("default_action") || d.HasChange("rule") {
 		o, n := d.GetChange("rule")
 		oldR, newR := o.(*schema.Set).List(), n.(*schema.Set).List()
 
-		err := updateWebAclResourceWR(d.Id(), action, oldR, newR, conn, region)
+		wr := newWafRegionalRetryer(conn, region)
+		_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
+			req := &waf.UpdateWebACLInput{
+				ChangeToken:   token,
+				DefaultAction: expandWafAction(d.Get("default_action").([]interface{})),
+				Updates:       diffWafWebAclRules(oldR, newR),
+				WebACLId:      aws.String(d.Id()),
+			}
+			return conn.UpdateWebACL(req)
+		})
 		if err != nil {
 			return fmt.Errorf("Error Updating WAF Regional ACL: %s", err)
 		}
@@ -169,11 +185,19 @@ func resourceAwsWafRegionalWebAclDelete(d *schema.ResourceData, meta interface{}
 	conn := meta.(*AWSClient).wafregionalconn
 	region := meta.(*AWSClient).region
 
-	action := expandDefaultActionWR(d.Get("default_action").([]interface{}))
+	// First, need to delete all rules
 	rules := d.Get("rule").(*schema.Set).List()
 	if len(rules) > 0 {
-		noRules := []interface{}{}
-		err := updateWebAclResourceWR(d.Id(), action, rules, noRules, conn, region)
+		wr := newWafRegionalRetryer(conn, region)
+		_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
+			req := &waf.UpdateWebACLInput{
+				ChangeToken:   token,
+				DefaultAction: expandWafAction(d.Get("default_action").([]interface{})),
+				Updates:       diffWafWebAclRules(rules, []interface{}{}),
+				WebACLId:      aws.String(d.Id()),
+			}
+			return conn.UpdateWebACL(req)
+		})
 		if err != nil {
 			return fmt.Errorf("Error Removing WAF Regional ACL Rules: %s", err)
 		}
@@ -194,126 +218,3 @@ func resourceAwsWafRegionalWebAclDelete(d *schema.ResourceData, meta interface{}
 	}
 	return nil
 }
-
-func updateWebAclResourceWR(id string, a *waf.WafAction, oldR, newR []interface{}, conn *wafregional.WAFRegional, region string) error {
-	wr := newWafRegionalRetryer(conn, region)
-	_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
-		req := &waf.UpdateWebACLInput{
-			DefaultAction: a,
-			ChangeToken:   token,
-			WebACLId:      aws.String(id),
-			Updates:       diffWafWebAclRules(oldR, newR),
-		}
-		return conn.UpdateWebACL(req)
-	})
-	if err != nil {
-		return fmt.Errorf("Error Updating WAF Regional ACL: %s", err)
-	}
-	return nil
-}
-
-func expandDefaultActionWR(d []interface{}) *waf.WafAction {
-	if d == nil || len(d) == 0 {
-		return nil
-	}
-
-	if d[0] == nil {
-		log.Printf("[ERR] First element of Default Action is set to nil")
-		return nil
-	}
-
-	dA := d[0].(map[string]interface{})
-
-	return &waf.WafAction{
-		Type: aws.String(dA["type"].(string)),
-	}
-}
-
-func flattenDefaultActionWR(n *waf.WafAction) []map[string]interface{} {
-	if n == nil {
-		return nil
-	}
-
-	m := setMap(make(map[string]interface{}))
-
-	m.SetString("type", n.Type)
-	return m.MapList()
-}
-
-func flattenWafWebAclRules(ts []*waf.ActivatedRule) []map[string]interface{} {
-	out := make([]map[string]interface{}, len(ts), len(ts))
-	for i, r := range ts {
-		m := make(map[string]interface{})
-
-		switch aws.StringValue(r.Type) {
-		case waf.WafRuleTypeGroup:
-			actionMap := map[string]interface{}{
-				"type": aws.StringValue(r.OverrideAction.Type),
-			}
-			m["override_action"] = []map[string]interface{}{actionMap}
-		default:
-			actionMap := map[string]interface{}{
-				"type": aws.StringValue(r.Action.Type),
-			}
-			m["action"] = []map[string]interface{}{actionMap}
-		}
-
-		m["priority"] = int(aws.Int64Value(r.Priority))
-		m["rule_id"] = aws.StringValue(r.RuleId)
-		m["type"] = aws.StringValue(r.Type)
-		out[i] = m
-	}
-	return out
-}
-
-func expandWafWebAclUpdate(updateAction string, aclRule map[string]interface{}) *waf.WebACLUpdate {
-	var rule *waf.ActivatedRule
-
-	switch aclRule["type"].(string) {
-	case waf.WafRuleTypeGroup:
-		ruleAction := aclRule["override_action"].([]interface{})[0].(map[string]interface{})
-
-		rule = &waf.ActivatedRule{
-			OverrideAction: &waf.WafOverrideAction{Type: aws.String(ruleAction["type"].(string))},
-			Priority:       aws.Int64(int64(aclRule["priority"].(int))),
-			RuleId:         aws.String(aclRule["rule_id"].(string)),
-			Type:           aws.String(aclRule["type"].(string)),
-		}
-	default:
-		ruleAction := aclRule["action"].([]interface{})[0].(map[string]interface{})
-
-		rule = &waf.ActivatedRule{
-			Action:   &waf.WafAction{Type: aws.String(ruleAction["type"].(string))},
-			Priority: aws.Int64(int64(aclRule["priority"].(int))),
-			RuleId:   aws.String(aclRule["rule_id"].(string)),
-			Type:     aws.String(aclRule["type"].(string)),
-		}
-	}
-
-	update := &waf.WebACLUpdate{
-		Action:        aws.String(updateAction),
-		ActivatedRule: rule,
-	}
-
-	return update
-}
-
-func diffWafWebAclRules(oldR, newR []interface{}) []*waf.WebACLUpdate {
-	updates := make([]*waf.WebACLUpdate, 0)
-
-	for _, or := range oldR {
-		aclRule := or.(map[string]interface{})
-
-		if idx, contains := sliceContainsMap(newR, aclRule); contains {
-			newR = append(newR[:idx], newR[idx+1:]...)
-			continue
-		}
-		updates = append(updates, expandWafWebAclUpdate(waf.ChangeActionDelete, aclRule))
-	}
-
-	for _, nr := range newR {
-		aclRule := nr.(map[string]interface{})
-		updates = append(updates, expandWafWebAclUpdate(waf.ChangeActionInsert, aclRule))
-	}
-	return updates
-}
diff --git a/aws/structure.go b/aws/structure.go
index 3cc071721d1..f4f0428b904 100644
--- a/aws/structure.go
+++ b/aws/structure.go
@@ -3426,6 +3426,115 @@ func flattenFieldToMatch(fm *waf.FieldToMatch) []interface{} {
 	return []interface{}{m}
 }
 
+func diffWafWebAclRules(oldR, newR []interface{}) []*waf.WebACLUpdate {
+	updates := make([]*waf.WebACLUpdate, 0)
+
+	for _, or := range oldR {
+		aclRule := or.(map[string]interface{})
+
+		if idx, contains := sliceContainsMap(newR, aclRule); contains {
+			newR = append(newR[:idx], newR[idx+1:]...)
+			continue
+		}
+		updates = append(updates, expandWafWebAclUpdate(waf.ChangeActionDelete, aclRule))
+	}
+
+	for _, nr := range newR {
+		aclRule := nr.(map[string]interface{})
+		updates = append(updates, expandWafWebAclUpdate(waf.ChangeActionInsert, aclRule))
+	}
+	return updates
+}
+
+func expandWafWebAclUpdate(updateAction string, aclRule map[string]interface{}) *waf.WebACLUpdate {
+	var rule *waf.ActivatedRule
+
+	switch aclRule["type"].(string) {
+	case waf.WafRuleTypeGroup:
+		rule = &waf.ActivatedRule{
+			OverrideAction: expandWafOverrideAction(aclRule["override_action"].([]interface{})),
+			Priority:       aws.Int64(int64(aclRule["priority"].(int))),
+			RuleId:         aws.String(aclRule["rule_id"].(string)),
+			Type:           aws.String(aclRule["type"].(string)),
+		}
+	default:
+		rule = &waf.ActivatedRule{
+			Action:   expandWafAction(aclRule["action"].([]interface{})),
+			Priority: aws.Int64(int64(aclRule["priority"].(int))),
+			RuleId:   aws.String(aclRule["rule_id"].(string)),
+			Type:     aws.String(aclRule["type"].(string)),
+		}
+	}
+
+	update := &waf.WebACLUpdate{
+		Action:        aws.String(updateAction),
+		ActivatedRule: rule,
+	}
+
+	return update
+}
+
+func expandWafAction(l []interface{}) *waf.WafAction {
+	if l == nil || len(l) == 0 || l[0] == nil {
+		return nil
+	}
+
+	m := l[0].(map[string]interface{})
+
+	return &waf.WafAction{
+		Type: aws.String(m["type"].(string)),
+	}
+}
+
+func expandWafOverrideAction(l []interface{}) *waf.WafOverrideAction {
+	if l == nil || len(l) == 0 || l[0] == nil {
+		return nil
+	}
+
+	m := l[0].(map[string]interface{})
+
+	return &waf.WafOverrideAction{
+		Type: aws.String(m["type"].(string)),
+	}
+}
+
+func flattenWafAction(n *waf.WafAction) []map[string]interface{} {
+	if n == nil {
+		return nil
+	}
+
+	m := setMap(make(map[string]interface{}))
+
+	m.SetString("type", n.Type)
+	return m.MapList()
+}
+
+func flattenWafWebAclRules(ts []*waf.ActivatedRule) []map[string]interface{} {
+	out := make([]map[string]interface{}, len(ts), len(ts))
+	for i, r := range ts {
+		m := make(map[string]interface{})
+
+		switch aws.StringValue(r.Type) {
+		case waf.WafRuleTypeGroup:
+			actionMap := map[string]interface{}{
+				"type": aws.StringValue(r.OverrideAction.Type),
+			}
+			m["override_action"] = []map[string]interface{}{actionMap}
+		default:
+			actionMap := map[string]interface{}{
+				"type": aws.StringValue(r.Action.Type),
+			}
+			m["action"] = []map[string]interface{}{actionMap}
+		}
+
+		m["priority"] = int(aws.Int64Value(r.Priority))
+		m["rule_id"] = aws.StringValue(r.RuleId)
+		m["type"] = aws.StringValue(r.Type)
+		out[i] = m
+	}
+	return out
+}
+
 // escapeJsonPointer escapes string per RFC 6901
 // so it can be used as path in JSON patch operations
 func escapeJsonPointer(path string) string {