Skip to content

Commit

Permalink
resource/aws_waf(regional)_ipset: Properly handle updates and deletio…
Browse files Browse the repository at this point in the history 
…ns over 1000 IP set descriptors

* Perform batching of 1000 Updates with UpdateIPSet

Previously:

--- FAIL: TestAccAWSWafIPSet_IpSetDescriptors_1000UpdateLimit (8.02s)
	testing.go:527: Step 0 error: Error applying: 1 error occurred:
			* aws_waf_ipset.ipset: 1 error occurred:
			* aws_waf_ipset.ipset: Error Updating WAF IPSet: Error Updating WAF IPSet: WAFLimitsExceededException: Operation would result in exceeding resource limits.
--- FAIL: TestAccAWSWafRegionalIPSet_IpSetDescriptors_1000UpdateLimit (10.55s)
	testing.go:527: Step 0 error: Error applying: 1 error occurred:
			* aws_wafregional_ipset.ipset: 1 error occurred:
			* aws_wafregional_ipset.ipset: Error Updating WAF IPSet: Error Updating WAF IPSet: WAFLimitsExceededException: Operation would result in exceeding resource limits.

Now passes:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSWaf\(Regional\)\?IPSet_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSWaf\(Regional\)\?IPSet_ -timeout 120m
=== RUN   TestAccAWSWafIPSet_basic
--- PASS: TestAccAWSWafIPSet_basic (11.79s)
=== RUN   TestAccAWSWafIPSet_disappears
--- PASS: TestAccAWSWafIPSet_disappears (7.02s)
=== RUN   TestAccAWSWafIPSet_changeNameForceNew
--- PASS: TestAccAWSWafIPSet_changeNameForceNew (18.54s)
=== RUN   TestAccAWSWafIPSet_changeDescriptors
--- PASS: TestAccAWSWafIPSet_changeDescriptors (15.31s)
=== RUN   TestAccAWSWafIPSet_noDescriptors
--- PASS: TestAccAWSWafIPSet_noDescriptors (8.89s)
=== RUN   TestAccAWSWafIPSet_IpSetDescriptors_1000UpdateLimit
--- PASS: TestAccAWSWafIPSet_IpSetDescriptors_1000UpdateLimit (26.55s)
=== RUN   TestAccAWSWafRegionalIPSet_basic
--- PASS: TestAccAWSWafRegionalIPSet_basic (13.73s)
=== RUN   TestAccAWSWafRegionalIPSet_disappears
--- PASS: TestAccAWSWafRegionalIPSet_disappears (13.02s)
=== RUN   TestAccAWSWafRegionalIPSet_changeNameForceNew
--- PASS: TestAccAWSWafRegionalIPSet_changeNameForceNew (26.81s)
=== RUN   TestAccAWSWafRegionalIPSet_changeDescriptors
--- PASS: TestAccAWSWafRegionalIPSet_changeDescriptors (22.95s)
=== RUN   TestAccAWSWafRegionalIPSet_IpSetDescriptors_1000UpdateLimit
--- PASS: TestAccAWSWafRegionalIPSet_IpSetDescriptors_1000UpdateLimit (33.62s)
=== RUN   TestAccAWSWafRegionalIPSet_noDescriptors
--- PASS: TestAccAWSWafRegionalIPSet_noDescriptors (10.92s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	209.782s
  • Loading branch information
@bflad
bflad committed Aug 17, 2018
1 parent 5ee91f7 commit fe8f004
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 29 deletions.
47 changes: 32 additions & 15 deletions aws/resource_aws_waf_ipset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"github.com/hashicorp/terraform/helper/schema"
)

// WAF requires UpdateIPSet operations be split into batches of 1000 Updates
const wafUpdateIPSetUpdatesLimit = 1000

func resourceAwsWafIPSet() *schema.Resource {
return &schema.Resource{
Create: resourceAwsWafIPSetCreate,
Expand Down Expand Up @@ -138,7 +141,7 @@ func resourceAwsWafIPSetDelete(d *schema.ResourceData, meta interface{}) error {
noDescriptors := []interface{}{}
err := updateWafIpSetDescriptors(d.Id(), oldDescriptors, noDescriptors, conn)
if err != nil {
return fmt.Errorf("Error updating IPSetDescriptors: %s", err)
return fmt.Errorf("Error Deleting IPSetDescriptors: %s", err)
}
}

Expand All @@ -159,25 +162,28 @@ func resourceAwsWafIPSetDelete(d *schema.ResourceData, meta interface{}) error {
}

func updateWafIpSetDescriptors(id string, oldD, newD []interface{}, conn *waf.WAF) error {
wr := newWafRetryer(conn, "global")
_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
req := &waf.UpdateIPSetInput{
ChangeToken: token,
IPSetId: aws.String(id),
Updates: diffWafIpSetDescriptors(oldD, newD),
for _, ipSetUpdates := range diffWafIpSetDescriptors(oldD, newD) {
wr := newWafRetryer(conn, "global")
_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
req := &waf.UpdateIPSetInput{
ChangeToken: token,
IPSetId: aws.String(id),
Updates: ipSetUpdates,
}
log.Printf("[INFO] Updating IPSet descriptors: %s", req)
return conn.UpdateIPSet(req)
})
if err != nil {
return fmt.Errorf("Error Updating WAF IPSet: %s", err)
}
log.Printf("[INFO] Updating IPSet descriptors: %s", req)
return conn.UpdateIPSet(req)
})
if err != nil {
return fmt.Errorf("Error Updating WAF IPSet: %s", err)
}

return nil
}

func diffWafIpSetDescriptors(oldD, newD []interface{}) []*waf.IPSetUpdate {
updates := make([]*waf.IPSetUpdate, 0)
func diffWafIpSetDescriptors(oldD, newD []interface{}) [][]*waf.IPSetUpdate {
updates := make([]*waf.IPSetUpdate, 0, wafUpdateIPSetUpdatesLimit)
updatesBatches := make([][]*waf.IPSetUpdate, 0)

for _, od := range oldD {
descriptor := od.(map[string]interface{})
Expand All @@ -187,6 +193,11 @@ func diffWafIpSetDescriptors(oldD, newD []interface{}) []*waf.IPSetUpdate {
continue
}

if len(updates) == wafUpdateIPSetUpdatesLimit {
updatesBatches = append(updatesBatches, updates)
updates = make([]*waf.IPSetUpdate, 0, wafUpdateIPSetUpdatesLimit)
}

updates = append(updates, &waf.IPSetUpdate{
Action: aws.String(waf.ChangeActionDelete),
IPSetDescriptor: &waf.IPSetDescriptor{
Expand All @@ -199,6 +210,11 @@ func diffWafIpSetDescriptors(oldD, newD []interface{}) []*waf.IPSetUpdate {
for _, nd := range newD {
descriptor := nd.(map[string]interface{})

if len(updates) == wafUpdateIPSetUpdatesLimit {
updatesBatches = append(updatesBatches, updates)
updates = make([]*waf.IPSetUpdate, 0, wafUpdateIPSetUpdatesLimit)
}

updates = append(updates, &waf.IPSetUpdate{
Action: aws.String(waf.ChangeActionInsert),
IPSetDescriptor: &waf.IPSetDescriptor{
Expand All @@ -207,5 +223,6 @@ func diffWafIpSetDescriptors(oldD, newD []interface{}) []*waf.IPSetUpdate {
},
})
}
return updates
updatesBatches = append(updatesBatches, updates)
return updatesBatches
}
49 changes: 49 additions & 0 deletions aws/resource_aws_waf_ipset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package aws

import (
"fmt"
"net"
"reflect"
"regexp"
"strings"
"testing"

"github.com/hashicorp/terraform/helper/resource"
Expand Down Expand Up @@ -169,6 +171,46 @@ func TestAccAWSWafIPSet_noDescriptors(t *testing.T) {
})
}

func TestAccAWSWafIPSet_IpSetDescriptors_1000UpdateLimit(t *testing.T) {
var ipset waf.IPSet
ipsetName := fmt.Sprintf("ip-set-%s", acctest.RandString(5))
resourceName := "aws_waf_ipset.ipset"

incrementIP := func(ip net.IP) {
for j := len(ip) - 1; j >= 0; j-- {
ip[j]++
if ip[j] > 0 {
break
}
}
}

// Generate 2048 IPs
ip, ipnet, err := net.ParseCIDR("10.0.0.0/21")
if err != nil {
t.Fatal(err)
}
ipSetDescriptors := make([]string, 0, 2048)
for ip := ip.Mask(ipnet.Mask); ipnet.Contains(ip); incrementIP(ip) {
ipSetDescriptors = append(ipSetDescriptors, fmt.Sprintf("ip_set_descriptors {\ntype=\"IPV4\"\nvalue=\"%s/32\"\n}", ip))
}

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSWafIPSetDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSWafIPSetConfig_IpSetDescriptors(ipsetName, strings.Join(ipSetDescriptors, "\n")),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSWafIPSetExists(resourceName, &ipset),
resource.TestCheckResourceAttr(resourceName, "ip_set_descriptors.#", "2048"),
),
},
},
})
}

func TestDiffWafIpSetDescriptors(t *testing.T) {
testCases := []struct {
Old []interface{}
Expand Down Expand Up @@ -401,6 +443,13 @@ func testAccAWSWafIPSetConfigChangeIPSetDescriptors(name string) string {
}`, name)
}

func testAccAWSWafIPSetConfig_IpSetDescriptors(name, ipSetDescriptors string) string {
return fmt.Sprintf(`resource "aws_waf_ipset" "ipset" {
name = "%s"
%s
}`, name, ipSetDescriptors)
}

func testAccAWSWafIPSetConfig_noDescriptors(name string) string {
return fmt.Sprintf(`resource "aws_waf_ipset" "ipset" {
name = "%s"
Expand Down
29 changes: 15 additions & 14 deletions aws/resource_aws_wafregional_ipset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func resourceAwsWafRegionalIPSetDelete(d *schema.ResourceData, meta interface{})
err := updateIPSetResourceWR(d.Id(), oldD, noD, conn, region)

if err != nil {
return fmt.Errorf("Error Removing IPSetDescriptors: %s", err)
return fmt.Errorf("Error Deleting IPSetDescriptors: %s", err)
}
}

Expand All @@ -164,20 +164,21 @@ func resourceAwsWafRegionalIPSetDelete(d *schema.ResourceData, meta interface{})
}

func updateIPSetResourceWR(id string, oldD, newD []interface{}, conn *wafregional.WAFRegional, region string) error {

wr := newWafRegionalRetryer(conn, region)
_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
req := &waf.UpdateIPSetInput{
ChangeToken: token,
IPSetId: aws.String(id),
Updates: diffWafIpSetDescriptors(oldD, newD),
for _, ipSetUpdates := range diffWafIpSetDescriptors(oldD, newD) {
wr := newWafRegionalRetryer(conn, region)
_, err := wr.RetryWithToken(func(token *string) (interface{}, error) {
req := &waf.UpdateIPSetInput{
ChangeToken: token,
IPSetId: aws.String(id),
Updates: ipSetUpdates,
}
log.Printf("[INFO] Updating IPSet descriptor: %s", req)

return conn.UpdateIPSet(req)
})
if err != nil {
return fmt.Errorf("Error Updating WAF IPSet: %s", err)
}
log.Printf("[INFO] Updating IPSet descriptor: %s", req)

return conn.UpdateIPSet(req)
})
if err != nil {
return fmt.Errorf("Error Updating WAF IPSet: %s", err)
}

return nil
Expand Down
49 changes: 49 additions & 0 deletions aws/resource_aws_wafregional_ipset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package aws

import (
"fmt"
"net"
"reflect"
"regexp"
"strings"
"testing"

"github.com/hashicorp/terraform/helper/resource"
Expand Down Expand Up @@ -142,6 +144,46 @@ func TestAccAWSWafRegionalIPSet_changeDescriptors(t *testing.T) {
})
}

func TestAccAWSWafRegionalIPSet_IpSetDescriptors_1000UpdateLimit(t *testing.T) {
var ipset waf.IPSet
ipsetName := fmt.Sprintf("ip-set-%s", acctest.RandString(5))
resourceName := "aws_wafregional_ipset.ipset"

incrementIP := func(ip net.IP) {
for j := len(ip) - 1; j >= 0; j-- {
ip[j]++
if ip[j] > 0 {
break
}
}
}

// Generate 2048 IPs
ip, ipnet, err := net.ParseCIDR("10.0.0.0/21")
if err != nil {
t.Fatal(err)
}
ipSetDescriptors := make([]string, 0, 2048)
for ip := ip.Mask(ipnet.Mask); ipnet.Contains(ip); incrementIP(ip) {
ipSetDescriptors = append(ipSetDescriptors, fmt.Sprintf("ip_set_descriptor {\ntype=\"IPV4\"\nvalue=\"%s/32\"\n}", ip))
}

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSWafRegionalIPSetDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSWafRegionalIPSetConfig_IpSetDescriptors(ipsetName, strings.Join(ipSetDescriptors, "\n")),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSWafRegionalIPSetExists(resourceName, &ipset),
resource.TestCheckResourceAttr(resourceName, "ip_set_descriptor.#", "2048"),
),
},
},
})
}

func TestAccAWSWafRegionalIPSet_noDescriptors(t *testing.T) {
var ipset waf.IPSet
ipsetName := fmt.Sprintf("ip-set-%s", acctest.RandString(5))
Expand Down Expand Up @@ -398,6 +440,13 @@ func testAccAWSWafRegionalIPSetConfigChangeIPSetDescriptors(name string) string
}`, name)
}

func testAccAWSWafRegionalIPSetConfig_IpSetDescriptors(name, ipSetDescriptors string) string {
return fmt.Sprintf(`resource "aws_wafregional_ipset" "ipset" {
name = "%s"
%s
}`, name, ipSetDescriptors)
}

func testAccAWSWafRegionalIPSetConfig_noDescriptors(name string) string {
return fmt.Sprintf(`resource "aws_wafregional_ipset" "ipset" {
name = "%s"
Expand Down

0 comments on commit fe8f004

Please sign in to comment.