Retry on AWS retry error and throttle codes (#11)

* Retry on AWS retry error and throttle codes

Copy some internal, inaccessible retry logic from the AWS SDK

* Streamline tags in terraform test fixtures

README.md

@@ -41,11 +41,11 @@ Happy (terra)dozing!
 It's recommended to install a specific version of terradozer available on the
 [releases page](https://github.com/jckuester/terradozer/releases).
 
-Here is the recommended way to install terradozer v0.1.1:
+Here is the recommended way to install terradozer v0.1.2:
 
 ```bash
 # install it into ./bin/
-curl -sSfL https://raw.githubusercontent.com/jckuester/terradozer/master/install.sh | sh -s v0.1.1
+curl -sSfL https://raw.githubusercontent.com/jckuester/terradozer/master/install.sh | sh -s v0.1.2
 ```
 
 ## Usage

pkg/provider/provider.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strings"
 	"time"
 
 	"github.com/mitchellh/cli"
@@ -28,8 +27,6 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-const requestError = "RequestError"
-
 // provider is the interface that every Terraform Provider Plugin implements.
 type provider interface {
 	Configure(providers.ConfigureRequest) providers.ConfigureResponse
@@ -138,7 +135,7 @@ func (p TerraformProvider) ImportResource(terraformType string, id string) ([]pr
 		})
 
 		if response.Diagnostics.HasErrors() {
-			if strings.Contains(response.Diagnostics.Err().Error(), requestError) {
+			if shouldRetry(response.Diagnostics.Err()) {
 				log.WithError(response.Diagnostics.Err()).Debug("retrying to import resource")
 
 				return resource.RetryableError(response.Diagnostics.Err())
@@ -171,7 +168,7 @@ func (p TerraformProvider) ReadResource(terraformType string, state cty.Value) (
 		})
 
 		if response.Diagnostics.HasErrors() {
-			if strings.Contains(response.Diagnostics.Err().Error(), requestError) {
+			if shouldRetry(response.Diagnostics.Err()) {
 				log.WithError(response.Diagnostics.Err()).Debug("retrying to read current state of resource")
 
 				return resource.RetryableError(response.Diagnostics.Err())
@@ -206,7 +203,7 @@ func (p TerraformProvider) DestroyResource(terraformType string, currentState ct
 		})
 
 		if response.Diagnostics.HasErrors() {
-			if strings.Contains(response.Diagnostics.Err().Error(), requestError) {
+			if shouldRetry(response.Diagnostics.Err()) {
 				log.WithError(response.Diagnostics.Err()).Debug("retrying to destroy resource")
 
 				return resource.RetryableError(response.Diagnostics.Err())

pkg/provider/provider_test.go

@@ -229,7 +229,10 @@ func TestTerraformProvider_ReadResource(t *testing.T) {
 	require.NoError(t, err)
 
 	assert.Equal(t, currentResourceState.GetAttr("tags"),
-		cty.MapVal(map[string]cty.Value{"Name": cty.StringVal(terraformOptions.Vars["name"].(string))}))
+		cty.MapVal(map[string]cty.Value{
+			"Name":       cty.StringVal(terraformOptions.Vars["name"].(string)),
+			"terradozer": cty.StringVal("test-acc"),
+		}))
 
 	assert.Equal(t, currentResourceState.GetAttr("cidr_block"),
 		cty.StringVal("10.0.0.0/16"))

pkg/provider/retry.go

@@ -0,0 +1,77 @@
+package provider
+
+import (
+	"strings"
+
+	"github.com/aws/aws-sdk-go/aws/request"
+)
+
+//nolint:gochecknoglobals
+var (
+	// copied from github.com/aws-sdk-go/aws/request/retryer.go
+	retryableCodes = map[string]struct{}{
+		request.ErrCodeRequestError:    {},
+		"RequestTimeout":               {},
+		request.ErrCodeResponseTimeout: {},
+		"RequestTimeoutException":      {}, // Glacier's flavor of RequestTimeout
+	}
+
+	// copied from github.com/aws-sdk-go/aws/request/retryer.go
+	throttleCodes = map[string]struct{}{
+		"ProvisionedThroughputExceededException": {},
+		"ThrottledException":                     {}, // SNS, XRay, ResourceGroupsTagging API
+		"Throttling":                             {},
+		"ThrottlingException":                    {},
+		"RequestLimitExceeded":                   {},
+		"RequestThrottled":                       {},
+		"RequestThrottledException":              {},
+		"TooManyRequestsException":               {}, // Lambda functions
+		"PriorRequestNotComplete":                {}, // Route53
+		"TransactionInProgressException":         {},
+		"EC2ThrottledException":                  {}, // EC2
+	}
+
+	// copied from github.com/aws-sdk-go/aws/request/retryer.go
+	credsExpiredCodes = map[string]struct{}{
+		"ExpiredToken":          {},
+		"ExpiredTokenException": {},
+		"RequestExpired":        {}, // EC2 Only
+	}
+)
+
+// shouldRetry returns true if the request should be retried.
+// Note: the given error is checked against retryable error codes of the AWS SDK API v1,
+// since Terraform AWS Provider also uses v1.
+func shouldRetry(err error) bool {
+	return isCodeRetryable(err) || isCodeThrottle(err)
+}
+
+func isCodeThrottle(err error) bool {
+	for throttleCode := range throttleCodes {
+		if strings.Contains(err.Error(), throttleCode) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func isCodeRetryable(err error) bool {
+	for retryableCode := range retryableCodes {
+		if strings.Contains(err.Error(), retryableCode) {
+			return true
+		}
+	}
+
+	return isCodeExpiredCreds(err)
+}
+
+func isCodeExpiredCreds(err error) bool {
+	for credsExpiredCode := range credsExpiredCodes {
+		if strings.Contains(err.Error(), credsExpiredCode) {
+			return true
+		}
+	}
+
+	return false
+}

pkg/provider/retry_test.go

@@ -0,0 +1,43 @@
+package provider
+
+import (
+	"fmt"
+	"testing"
+)
+
+func Test_shouldRetry(t *testing.T) {
+	tests := []struct {
+		name string
+		arg  error
+		want bool
+	}{
+		{
+			name: "a 'Throttling' error that is retryable",
+			arg:  fmt.Errorf("ThrottlingException: Rate exceeded"),
+			want: true,
+		},
+
+		{
+			name: "a 'RequestExpired' error that is retryable",
+			arg:  fmt.Errorf("RequestExpired: request has expired"),
+			want: true,
+		},
+		{
+			name: "a 'RequestError' error that is retryable",
+			arg:  fmt.Errorf("RequestError: send request failed"),
+			want: true,
+		},
+		{
+			name: "some error that is not retryable",
+			arg:  fmt.Errorf("SomeError: foo bar"),
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := shouldRetry(tt.arg); got != tt.want {
+				t.Errorf("shouldRetry() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

test/test-fixtures/attached-policy/main.tf

@@ -12,15 +12,16 @@ terraform {
 }
 
 resource "aws_vpc" "test" {
-  cidr_block       = "10.0.0.0/16"
+  cidr_block = "10.0.0.0/16"
 
   tags = {
-    Name = var.name
+    Name       = var.name
+    terradozer = "test-acc"
   }
 }
 
 resource "aws_iam_role" "test" {
-  name = "test_role"
+  name = var.name
 
   assume_role_policy = <<EOF
 {
@@ -39,14 +40,14 @@ resource "aws_iam_role" "test" {
 EOF
 
   tags = {
-    tag-key = "tag-value"
+    terradozer = "test-acc"
   }
 }
 
 resource "aws_iam_policy" "test" {
-  name        = "test_policy"
+  name        = var.name
   path        = "/"
-  description = "My test policy"
+  description = "A test policy"
 
   policy = <<EOF
 {

test/test-fixtures/dependent-resources/main.tf

@@ -12,10 +12,11 @@ terraform {
 }
 
 resource "aws_vpc" "test" {
-  cidr_block       = "10.0.0.0/16"
+  cidr_block = "10.0.0.0/16"
 
   tags = {
-    Name = var.name
+    Name       = var.name
+    terradozer = "test-acc"
   }
 }
 
@@ -25,7 +26,7 @@ resource "aws_iam_role" "test" {
   assume_role_policy = data.aws_iam_policy_document.role.json
 
   tags = {
-    tag-key = var.name
+    terradozer = "test-acc"
   }
 }
 
@@ -41,8 +42,8 @@ data "aws_iam_policy_document" "role" {
 }
 
 resource "aws_iam_policy" "test" {
-  name        = var.name
-  path        = "/"
+  name = var.name
+  path = "/"
 
   policy = data.aws_iam_policy_document.policy.json
 }

test/test-fixtures/non-empty-bucket/main.tf

@@ -15,7 +15,8 @@ resource "aws_s3_bucket" "test" {
   bucket = var.name
 
   tags = {
-    Name = var.name
+    Name       = var.name
+    terradozer = "test-acc"
   }
 }
 

test/test-fixtures/single-resource/aws-ecs-cluster/main.tf

@@ -14,6 +14,6 @@ terraform {
 resource "aws_ecs_cluster" "test" {
   name = var.name
   tags = {
-    awsweeper = "test-acc"
+    terradozer = "test-acc"
   }
 }

test/test-fixtures/single-resource/aws-lambda-function/main.tf

@@ -29,6 +29,10 @@ resource "aws_iam_role" "test" {
   ]
 }
 EOF
+
+  tags = {
+    terradozer = "test-acc"
+  }
 }
 
 resource "aws_lambda_function" "test" {
@@ -43,7 +47,7 @@ resource "aws_lambda_function" "test" {
 
   environment {
     variables = {
-      awsweeper = "test-acc"
+      terradozer = "test-acc"
     }
   }
 }

test/test-fixtures/single-resource/aws-vpc/dependency/main.tf

@@ -18,6 +18,7 @@ resource "aws_subnet" "test" {
   cidr_block = "10.0.1.0/24"
 
   tags = {
-    Name = var.name
+    Name       = var.name
+    terradozer = "test-acc"
   }
 }

test/test-fixtures/single-resource/aws-vpc/main.tf

@@ -15,6 +15,7 @@ resource "aws_vpc" "test" {
   cidr_block = "10.0.0.0/16"
 
   tags = {
-    Name = var.name
+    Name       = var.name
+    terradozer = "test-acc"
   }
 }

test/test-fixtures/unsupported-provider/main.tf

@@ -12,14 +12,15 @@ terraform {
 }
 
 resource "aws_vpc" "test" {
-  cidr_block       = "10.0.0.0/16"
+  cidr_block = "10.0.0.0/16"
 
   tags = {
-    Name = var.name
+    Name       = var.name
+    terradozer = "test-acc"
   }
 }
 
 resource "random_integer" "test" {
-  min     = 1
-  max     = 50000
+  min = 1
+  max = 50000
 }