diff --git a/GNUmakefile b/GNUmakefile index 6818c573a91..7aa4b3ad62b 100644 --- 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 index 6c51939ba9b..2f6296a6c3a 100644 --- 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 index 809404255d7..6b94094f99e 100644 --- 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,9 +39,8 @@ 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) }, @@ -49,37 +48,55 @@ func TestAccAWSRedshiftSnapshotCopyGrant_Update(t *testing.T) { 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)