r/aws_redshift_snapshot_copy_grant: Remove extraneous Exists function (…

…#13052) * r/aws_redshift_snapshot_copy_grant: Remove extraneous Exists function. * Enable providerlint R003 as the last 'Exists' method has been removed. * Fix rebase duplication.
hashicorp · Apr 30, 2020 · 72b775d · 72b775d
1 parent ea1a56a
commit 72b775d
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 134 deletions.
diff --git a/GNUmakefile b/GNUmakefile
@@ -70,6 +70,7 @@ lint:
 		-AWSR001 \
 		-AWSR002 \
 		-R002 \
+		-R003 \
 		-R004 \
 		-R006 \
 		-R007 \

diff --git a/aws/resource_aws_redshift_snapshot_copy_grant.go b/aws/resource_aws_redshift_snapshot_copy_grant.go
@@ -19,7 +19,6 @@ func resourceAwsRedshiftSnapshotCopyGrant() *schema.Resource {
 		Read:   resourceAwsRedshiftSnapshotCopyGrantRead,
 		Update: resourceAwsRedshiftSnapshotCopyGrantUpdate,
 		Delete: resourceAwsRedshiftSnapshotCopyGrantDelete,
-		Exists: resourceAwsRedshiftSnapshotCopyGrantExists,
 
 		Importer: &schema.ResourceImporter{
 			State: schema.ImportStatePassthrough,
@@ -75,6 +74,26 @@ func resourceAwsRedshiftSnapshotCopyGrantCreate(d *schema.ResourceData, meta int
 	log.Printf("[DEBUG] Created new Redshift SnapshotCopyGrant: %s", *out.SnapshotCopyGrant.SnapshotCopyGrantName)
 	d.SetId(grantName)
 
+	err = resource.Retry(3*time.Minute, func() *resource.RetryError {
+		var err error
+		var grant *redshift.SnapshotCopyGrant
+		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName)
+		if isAWSErr(err, redshift.ErrCodeSnapshotCopyGrantNotFoundFault, "") || grant == nil {
+			return resource.RetryableError(err)
+		}
+		if err != nil {
+			return resource.NonRetryableError(err)
+		}
+
+		return nil
+	})
+	if isResourceTimeoutError(err) {
+		_, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName)
+		if err != nil {
+			return err
+		}
+	}
+
 	return resourceAwsRedshiftSnapshotCopyGrantRead(d, meta)
 }
 
@@ -84,18 +103,18 @@ func resourceAwsRedshiftSnapshotCopyGrantRead(d *schema.ResourceData, meta inter
 
 	grantName := d.Id()
 	log.Printf("[DEBUG] Looking for grant: %s", grantName)
-	grant, err := findAwsRedshiftSnapshotCopyGrantWithRetry(conn, grantName)
-
-	if err != nil {
-		return err
-	}
 
-	if grant == nil {
-		log.Printf("[WARN] %s Redshift snapshot copy grant not found, removing from state file", grantName)
+	grant, err := findAwsRedshiftSnapshotCopyGrant(conn, grantName)
+	if isAWSErr(err, redshift.ErrCodeSnapshotCopyGrantNotFoundFault, "") || grant == nil {
+		log.Printf("[WARN] snapshot copy grant (%s) not found, removing from state", grantName)
 		d.SetId("")
 		return nil
 	}
 
+	if err != nil {
+		return err
+	}
+
 	arn := arn.ARN{
 		Partition: meta.(*AWSClient).partition,
 		Service:   "redshift",
@@ -154,91 +173,25 @@ func resourceAwsRedshiftSnapshotCopyGrantDelete(d *schema.ResourceData, meta int
 	return err
 }
 
-func resourceAwsRedshiftSnapshotCopyGrantExists(d *schema.ResourceData, meta interface{}) (bool, error) {
-	conn := meta.(*AWSClient).redshiftconn
-
-	grantName := d.Id()
-
-	log.Printf("[DEBUG] Looking for Grant: %s", grantName)
-	grant, err := findAwsRedshiftSnapshotCopyGrantWithRetry(conn, grantName)
-
-	if err != nil {
-		return false, err
-	}
-	if grant != nil {
-		return true, err
-	}
-
-	return false, nil
-}
-
-func getAwsRedshiftSnapshotCopyGrant(grants []*redshift.SnapshotCopyGrant, grantName string) *redshift.SnapshotCopyGrant {
-	for _, grant := range grants {
-		if *grant.SnapshotCopyGrantName == grantName {
-			return grant
-		}
-	}
-
-	return nil
-}
-
-/*
-In the functions below it is not possible to use retryOnAwsCodes function, as there
-is no get grant call, so an error has to be created if the grant is or isn't returned
-by the describe grants call when expected.
-*/
-
-// NB: This function only retries the grant not being returned and some edge cases, while AWS Errors
-// are handled by the findAwsRedshiftSnapshotCopyGrant function
-func findAwsRedshiftSnapshotCopyGrantWithRetry(conn *redshift.Redshift, grantName string) (*redshift.SnapshotCopyGrant, error) {
-	var grant *redshift.SnapshotCopyGrant
-	err := resource.Retry(3*time.Minute, func() *resource.RetryError {
-		var err error
-		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName, nil)
-
-		if err != nil {
-			if serr, ok := err.(*resource.NotFoundError); ok {
-				// Force a retry if the grant should exist
-				return resource.RetryableError(serr)
-			}
-
-			return resource.NonRetryableError(err)
-		}
-
-		return nil
-	})
-	if isResourceTimeoutError(err) {
-		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName, nil)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("Error finding snapshot copy grant: %s", err)
-	}
-	return grant, nil
-}
-
 // Used by the tests as well
 func waitForAwsRedshiftSnapshotCopyGrantToBeDeleted(conn *redshift.Redshift, grantName string) error {
-	var grant *redshift.SnapshotCopyGrant
 	err := resource.Retry(3*time.Minute, func() *resource.RetryError {
 		var err error
-		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName, nil)
-		if err != nil {
-			if isAWSErr(err, redshift.ErrCodeSnapshotCopyGrantNotFoundFault, "") {
-				return nil
-			}
+		var grant *redshift.SnapshotCopyGrant
+		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName)
+		if isAWSErr(err, redshift.ErrCodeSnapshotCopyGrantNotFoundFault, "") || grant == nil {
+			return nil
 		}
-
-		if grant != nil {
-			// Force a retry if the grant still exists
-			return resource.RetryableError(
-				fmt.Errorf("[DEBUG] Grant still exists while expected to be deleted: %s", *grant.SnapshotCopyGrantName))
+		if err != nil {
+			return resource.NonRetryableError(err)
 		}
 
-		return resource.NonRetryableError(err)
+		return resource.RetryableError(fmt.Errorf("[DEBUG] Grant still exists while expected to be deleted: %s", grantName))
 	})
 	if isResourceTimeoutError(err) {
-		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName, nil)
-		if isAWSErr(err, redshift.ErrCodeSnapshotCopyGrantNotFoundFault, "") {
+		var grant *redshift.SnapshotCopyGrant
+		grant, err = findAwsRedshiftSnapshotCopyGrant(conn, grantName)
+		if isAWSErr(err, redshift.ErrCodeSnapshotCopyGrantNotFoundFault, "") || grant == nil {
 			return nil
 		}
 	}
@@ -248,20 +201,10 @@ func waitForAwsRedshiftSnapshotCopyGrantToBeDeleted(conn *redshift.Redshift, gra
 	return nil
 }
 
-// The DescribeSnapshotCopyGrants API defaults to listing only 100 grants
-// Use a marker to iterate over all grants in "pages"
-// NB: This function only retries on AWS Errors
-func findAwsRedshiftSnapshotCopyGrant(conn *redshift.Redshift, grantName string, marker *string) (*redshift.SnapshotCopyGrant, error) {
+func findAwsRedshiftSnapshotCopyGrant(conn *redshift.Redshift, grantName string) (*redshift.SnapshotCopyGrant, error) {
 
 	input := redshift.DescribeSnapshotCopyGrantsInput{
-		MaxRecords: aws.Int64(int64(100)),
-	}
-
-	// marker and grant name are mutually exclusive
-	if marker != nil {
-		input.Marker = marker
-	} else {
-		input.SnapshotCopyGrantName = aws.String(grantName)
+		SnapshotCopyGrantName: aws.String(grantName),
 	}
 
 	out, err := conn.DescribeSnapshotCopyGrants(&input)
@@ -270,16 +213,9 @@ func findAwsRedshiftSnapshotCopyGrant(conn *redshift.Redshift, grantName string,
 		return nil, err
 	}
 
-	grant := getAwsRedshiftSnapshotCopyGrant(out.SnapshotCopyGrants, grantName)
-	if grant != nil {
-		return grant, nil
-	} else if out.Marker != nil {
-		log.Printf("[DEBUG] Snapshot copy grant not found but marker returned, getting next page via marker: %s", aws.StringValue(out.Marker))
-		return findAwsRedshiftSnapshotCopyGrant(conn, grantName, out.Marker)
+	if out == nil || len(out.SnapshotCopyGrants) == 0 {
+		return nil, nil
 	}
 
-	return nil, &resource.NotFoundError{
-		Message:     fmt.Sprintf("[DEBUG] Grant %s not found", grantName),
-		LastRequest: input,
-	}
+	return out.SnapshotCopyGrants[0], nil
 }
diff --git a/aws/resource_aws_redshift_snapshot_copy_grant_test.go b/aws/resource_aws_redshift_snapshot_copy_grant_test.go
@@ -12,7 +12,7 @@ import (
 )
 
 func TestAccAWSRedshiftSnapshotCopyGrant_Basic(t *testing.T) {
-	resourceName := "aws_redshift_snapshot_copy_grant.basic"
+	resourceName := "aws_redshift_snapshot_copy_grant.test"
 	rName := acctest.RandomWithPrefix("tf-acc-test")
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -25,7 +25,7 @@ func TestAccAWSRedshiftSnapshotCopyGrant_Basic(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSRedshiftSnapshotCopyGrantExists(resourceName),
 					resource.TestCheckResourceAttr(resourceName, "snapshot_copy_grant_name", rName),
-					resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-redshift-snapshot-copy-grant-basic"),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
 					resource.TestCheckResourceAttrSet(resourceName, "kms_key_id"),
 				),
 			},
@@ -39,47 +39,64 @@ func TestAccAWSRedshiftSnapshotCopyGrant_Basic(t *testing.T) {
 }
 
 func TestAccAWSRedshiftSnapshotCopyGrant_Update(t *testing.T) {
-
+	resourceName := "aws_redshift_snapshot_copy_grant.test"
 	rName := acctest.RandomWithPrefix("tf-acc-test")
-	resourceName := "aws_redshift_snapshot_copy_grant.basic"
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSRedshiftSnapshotCopyGrantDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSRedshiftSnapshotCopyGrant_Basic(rName),
+				Config: testAccAWSRedshiftSnapshotCopyGrantWithTags(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSRedshiftSnapshotCopyGrantExists(resourceName),
-					resource.TestCheckResourceAttr(
-						resourceName, "tags.%", "1"),
-					resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-redshift-snapshot-copy-grant-basic"),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
+					resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
+					resource.TestCheckResourceAttr(resourceName, "tags.Env", "Test"),
 				),
 			},
 			{
-				ResourceName:      resourceName,
-				ImportState:       true,
-				ImportStateVerify: true,
+				Config: testAccAWSRedshiftSnapshotCopyGrant_Basic(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSnapshotCopyGrantExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
+				),
 			},
 			{
 				Config: testAccAWSRedshiftSnapshotCopyGrantWithTags(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSRedshiftSnapshotCopyGrantExists(resourceName),
-					resource.TestCheckResourceAttr(
-						resourceName, "tags.%", "2"),
-					resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-redshift-snapshot-copy-grant-basic"),
-					resource.TestCheckResourceAttr(resourceName, "tags.Env", "Production"),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
+					resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
+					resource.TestCheckResourceAttr(resourceName, "tags.Env", "Test"),
 				),
 			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSRedshiftSnapshotCopyGrant_disappears(t *testing.T) {
+	resourceName := "aws_redshift_snapshot_copy_grant.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSRedshiftSnapshotCopyGrantDestroy,
+		Steps: []resource.TestStep{
 			{
 				Config: testAccAWSRedshiftSnapshotCopyGrant_Basic(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSRedshiftSnapshotCopyGrantExists(resourceName),
-					resource.TestCheckResourceAttr(
-						resourceName, "tags.%", "1"),
-					resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-redshift-snapshot-copy-grant-basic"),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsRedshiftSnapshotCopyGrant(), resourceName),
 				),
+				ExpectNonEmptyPlan: true,
 			},
 		},
 	})
@@ -138,24 +155,20 @@ func testAccCheckAWSRedshiftSnapshotCopyGrantExists(name string) resource.TestCh
 
 func testAccAWSRedshiftSnapshotCopyGrant_Basic(rName string) string {
 	return fmt.Sprintf(`
-resource "aws_redshift_snapshot_copy_grant" "basic" {
-  snapshot_copy_grant_name = "%s"
-
-  tags = {
-    Name = "tf-redshift-snapshot-copy-grant-basic"
-  }
+resource "aws_redshift_snapshot_copy_grant" "test" {
+  snapshot_copy_grant_name = %[1]q
 }
 `, rName)
 }
 
 func testAccAWSRedshiftSnapshotCopyGrantWithTags(rName string) string {
 	return fmt.Sprintf(`
-resource "aws_redshift_snapshot_copy_grant" "basic" {
-  snapshot_copy_grant_name = "%s"
+resource "aws_redshift_snapshot_copy_grant" "test" {
+  snapshot_copy_grant_name = %[1]q
 
   tags = {
-	Name = "tf-redshift-snapshot-copy-grant-basic"
-	Env	 = "Production"
+	Name = %[1]q
+	Env	 = "Test"
   }
 }
 `, rName)
-Original file line number
+Diff line change
@@ Expand Up / @@ -70,6 +70,7 @@ lint: @@
     		-AWSR001 \
     		-AWSR002 \
     		-R002 \
+    		-R003 \
     		-R004 \
     		-R006 \
     		-R007 \
@@ Expand Down @@