Skip to content

Commit

Permalink
service/waf: Only wrap errors returned by GetChangeToken in RetryWith…
Browse files Browse the repository at this point in the history
…Token usage

Reference: #9826

Wrapping the errors of the passed in WAF API functions breaks downstream error checking, e.g. `isAWSErr()`

Previously (failure based on eventual consistency):

```
--- FAIL: TestAccAWSWafWebAcl_Rules (31.87s)
    testing.go:569: Step 2 error: errors during apply:

        Error: Error deleting WAF Rule: Error getting WAF change token: WAFReferencedItemException: This entity is still referenced by other entities.
```

Output from acceptance testing:

```console
$ TF_ACC=1 go test ./aws -v -count=10 -timeout 120m -parallel 20 -run='TestAccAWSWafWebAcl_Rules'
...
--- PASS: TestAccAWSWafWebAcl_Rules (36.99s)
--- PASS: TestAccAWSWafWebAcl_Rules (34.34s)
--- PASS: TestAccAWSWafWebAcl_Rules (34.90s)
--- PASS: TestAccAWSWafWebAcl_Rules (35.58s)
--- PASS: TestAccAWSWafWebAcl_Rules (36.87s)
--- PASS: TestAccAWSWafWebAcl_Rules (33.99s)
--- PASS: TestAccAWSWafWebAcl_Rules (35.01s)
--- PASS: TestAccAWSWafWebAcl_Rules (33.60s)
--- PASS: TestAccAWSWafWebAcl_Rules (33.70s)
--- PASS: TestAccAWSWafWebAcl_Rules (35.82s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	353.338s
```

Also verified by running the stuck sweeper:

```console
$ go test ./aws -v -sweep=us-east-1,us-west-2 -sweep-run=aws_wafregional_web_acl -timeout 10h
...
2019/09/25 11:50:25 [INFO] Deleting WAF Regional Web ACL: dc595176-6e50-488d-a9b0-35df984b672f
...
2019/09/25 11:50:26 [INFO] Removing Rules from WAF Regional Web ACL: dc595176-6e50-488d-a9b0-35df984b672f
...
2019/09/25 11:50:27 [INFO] Deleting WAF Regional Web ACL: dc595176-6e50-488d-a9b0-35df984b672f
...
2019/09/25 11:50:28 Sweeper Tests ran:
	- aws_wafregional_web_acl
ok  	github.com/terraform-providers/terraform-provider-aws/aws	8.119s
```
  • Loading branch information
bflad committed Sep 25, 2019
1 parent 401984e commit 6321876
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
9 changes: 6 additions & 3 deletions aws/waf_token_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,15 @@ func (t *WafRetryer) RetryWithToken(f withTokenFunc) (interface{}, error) {
})
if isResourceTimeoutError(err) {
tokenOut, err = t.Connection.GetChangeToken(&waf.GetChangeTokenInput{})
if err == nil {
out, err = f(tokenOut.ChangeToken)

if err != nil {
return nil, fmt.Errorf("error getting WAF change token: %s", err)
}

out, err = f(tokenOut.ChangeToken)
}
if err != nil {
return nil, fmt.Errorf("Error getting WAF change token: %s", err)
return nil, err
}
return out, nil
}
Expand Down
9 changes: 6 additions & 3 deletions aws/wafregionl_token_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,15 @@ func (t *WafRegionalRetryer) RetryWithToken(f withRegionalTokenFunc) (interface{
})
if isResourceTimeoutError(err) {
tokenOut, err = t.Connection.GetChangeToken(&waf.GetChangeTokenInput{})
if err == nil {
out, err = f(tokenOut.ChangeToken)

if err != nil {
return nil, fmt.Errorf("error getting WAF Regional change token: %s", err)
}

out, err = f(tokenOut.ChangeToken)
}
if err != nil {
return nil, fmt.Errorf("Error getting WAF regional change token: %s", err)
return nil, err
}
return out, nil
}
Expand Down

0 comments on commit 6321876

Please sign in to comment.