EBS Snapshots Create Volume Permission: Fail if the account is snapshot owner

.changelog/12103.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_snapshot_create_volume_permission: Error if `account_id` is the snapshot's owner
+```

internal/service/ec2/ebs_snapshot_create_volume_permission.go

@@ -1,16 +1,18 @@
 package ec2
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
+	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 )
 
 func ResourceSnapshotCreateVolumePermission() *schema.Resource {
@@ -19,13 +21,20 @@ func ResourceSnapshotCreateVolumePermission() *schema.Resource {
 		Read:   resourceSnapshotCreateVolumePermissionRead,
 		Delete: resourceSnapshotCreateVolumePermissionDelete,
 
+		CustomizeDiff: resourceSnapshotCreateVolumePermissionCustomizeDiff,
+
+		Timeouts: &schema.ResourceTimeout{
+			Create: schema.DefaultTimeout(20 * time.Minute),
+			Delete: schema.DefaultTimeout(5 * time.Minute),
+		},
+
 		Schema: map[string]*schema.Schema{
-			"snapshot_id": {
+			"account_id": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
-			"account_id": {
+			"snapshot_id": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
@@ -37,37 +46,34 @@ func ResourceSnapshotCreateVolumePermission() *schema.Resource {
 func resourceSnapshotCreateVolumePermissionCreate(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).EC2Conn
 
-	snapshot_id := d.Get("snapshot_id").(string)
-	account_id := d.Get("account_id").(string)
-
-	_, err := conn.ModifySnapshotAttribute(&ec2.ModifySnapshotAttributeInput{
-		SnapshotId: aws.String(snapshot_id),
-		Attribute:  aws.String("createVolumePermission"),
+	snapshotID := d.Get("snapshot_id").(string)
+	accountID := d.Get("account_id").(string)
+	id := EBSSnapshotCreateVolumePermissionCreateResourceID(snapshotID, accountID)
+	input := &ec2.ModifySnapshotAttributeInput{
+		Attribute: aws.String(ec2.SnapshotAttributeNameCreateVolumePermission),
 		CreateVolumePermission: &ec2.CreateVolumePermissionModifications{
 			Add: []*ec2.CreateVolumePermission{
-				{UserId: aws.String(account_id)},
+				{UserId: aws.String(accountID)},
 			},
 		},
-	})
-	if err != nil {
-		return fmt.Errorf("Error adding snapshot createVolumePermission: %s", err)
+		SnapshotId: aws.String(snapshotID),
 	}
 
-	d.SetId(fmt.Sprintf("%s-%s", snapshot_id, account_id))
+	log.Printf("[DEBUG] Creating EBS Snapshot CreateVolumePermission: %s", input)
+	_, err := conn.ModifySnapshotAttribute(input)
 
-	// Wait for the account to appear in the permission list
-	stateConf := &resource.StateChangeConf{
-		Pending:    []string{"denied"},
-		Target:     []string{"granted"},
-		Refresh:    resourceSnapshotCreateVolumePermissionStateRefreshFunc(conn, snapshot_id, account_id),
-		Timeout:    20 * time.Minute,
-		Delay:      10 * time.Second,
-		MinTimeout: 10 * time.Second,
+	if err != nil {
+		return fmt.Errorf("creating EBS Snapshot CreateVolumePermission (%s): %w", id, err)
 	}
-	if _, err := stateConf.WaitForState(); err != nil {
-		return fmt.Errorf(
-			"Error waiting for snapshot createVolumePermission (%s) to be added: %s",
-			d.Id(), err)
+
+	d.SetId(id)
+
+	_, err = tfresource.RetryWhenNotFound(d.Timeout(schema.TimeoutCreate), func() (interface{}, error) {
+		return FindCreateSnapshotCreateVolumePermissionByTwoPartKey(conn, snapshotID, accountID)
+	})
+
+	if err != nil {
+		return fmt.Errorf("waiting for EBS Snapshot CreateVolumePermission create (%s): %w", d.Id(), err)
 	}
 
 	return nil
@@ -76,98 +82,101 @@ func resourceSnapshotCreateVolumePermissionCreate(d *schema.ResourceData, meta i
 func resourceSnapshotCreateVolumePermissionRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).EC2Conn
 
-	snapshotID, accountID, err := SnapshotCreateVolumePermissionParseID(d.Id())
-	if err != nil {
-		return err
-	}
+	snapshotID, accountID, err := EBSSnapshotCreateVolumePermissionParseResourceID(d.Id())
 
-	exists, err := HasCreateVolumePermission(conn, snapshotID, accountID)
 	if err != nil {
 		return err
 	}
-	if !exists {
-		log.Printf("[WARN] snapshot createVolumePermission (%s) not found, removing from state", d.Id())
+
+	_, err = FindCreateSnapshotCreateVolumePermissionByTwoPartKey(conn, snapshotID, accountID)
+
+	if !d.IsNewResource() && tfresource.NotFound(err) {
+		log.Printf("[WARN] EBS Snapshot CreateVolumePermission %s not found, removing from state", d.Id())
 		d.SetId("")
 		return nil
 	}
 
+	if err != nil {
+		return fmt.Errorf("reading EBS Snapshot CreateVolumePermission (%s): %w", d.Id(), err)
+	}
+
 	return nil
 }
 
 func resourceSnapshotCreateVolumePermissionDelete(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).EC2Conn
 
-	snapshotID, accountID, err := SnapshotCreateVolumePermissionParseID(d.Id())
+	snapshotID, accountID, err := EBSSnapshotCreateVolumePermissionParseResourceID(d.Id())
+
 	if err != nil {
 		return err
 	}
 
+	log.Printf("[DEBUG] Deleting EBS Snapshot CreateVolumePermission: %s", d.Id())
 	_, err = conn.ModifySnapshotAttribute(&ec2.ModifySnapshotAttributeInput{
-		SnapshotId: aws.String(snapshotID),
-		Attribute:  aws.String("createVolumePermission"),
+		Attribute: aws.String(ec2.SnapshotAttributeNameCreateVolumePermission),
 		CreateVolumePermission: &ec2.CreateVolumePermissionModifications{
 			Remove: []*ec2.CreateVolumePermission{
 				{UserId: aws.String(accountID)},
 			},
 		},
+		SnapshotId: aws.String(snapshotID),
 	})
-	if err != nil {
-		return fmt.Errorf("Error removing snapshot createVolumePermission: %s", err)
-	}
 
-	// Wait for the account to disappear from the permission list
-	stateConf := &resource.StateChangeConf{
-		Pending:    []string{"granted"},
-		Target:     []string{"denied"},
-		Refresh:    resourceSnapshotCreateVolumePermissionStateRefreshFunc(conn, snapshotID, accountID),
-		Timeout:    5 * time.Minute,
-		Delay:      10 * time.Second,
-		MinTimeout: 10 * time.Second,
+	if tfawserr.ErrCodeEquals(err, errCodeInvalidSnapshotNotFound) {
+		return nil
 	}
-	if _, err := stateConf.WaitForState(); err != nil {
-		return fmt.Errorf(
-			"Error waiting for snapshot createVolumePermission (%s) to be removed: %s",
-			d.Id(), err)
+
+	if err != nil {
+		return fmt.Errorf("deleting EBS Snapshot CreateVolumePermission (%s): %w", d.Id(), err)
 	}
 
-	return nil
-}
+	_, err = tfresource.RetryUntilNotFound(d.Timeout(schema.TimeoutDelete), func() (interface{}, error) {
+		return FindCreateSnapshotCreateVolumePermissionByTwoPartKey(conn, snapshotID, accountID)
+	})
 
-func HasCreateVolumePermission(conn *ec2.EC2, snapshot_id string, account_id string) (bool, error) {
-	_, state, err := resourceSnapshotCreateVolumePermissionStateRefreshFunc(conn, snapshot_id, account_id)()
 	if err != nil {
-		return false, err
-	}
-	if state == "granted" {
-		return true, nil
-	} else {
-		return false, nil
+		return fmt.Errorf("waiting for EBS Snapshot CreateVolumePermission delete (%s): %w", d.Id(), err)
 	}
+
+	return nil
 }
 
-func resourceSnapshotCreateVolumePermissionStateRefreshFunc(conn *ec2.EC2, snapshot_id string, account_id string) resource.StateRefreshFunc {
-	return func() (interface{}, string, error) {
-		attrs, err := conn.DescribeSnapshotAttribute(&ec2.DescribeSnapshotAttributeInput{
-			SnapshotId: aws.String(snapshot_id),
-			Attribute:  aws.String("createVolumePermission"),
-		})
-		if err != nil {
-			return nil, "", fmt.Errorf("Error refreshing snapshot createVolumePermission state: %s", err)
-		}
+func resourceSnapshotCreateVolumePermissionCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error {
+	if diff.Id() == "" {
+		if snapshotID := diff.Get("snapshot_id").(string); snapshotID != "" {
+			conn := meta.(*conns.AWSClient).EC2Conn
 
-		for _, vp := range attrs.CreateVolumePermissions {
-			if aws.StringValue(vp.UserId) == account_id {
-				return attrs, "granted", nil
+			snapshot, err := FindSnapshotByID(conn, snapshotID)
+
+			if err != nil {
+				return fmt.Errorf("reading EBS Snapshot (%s): %w", snapshotID, err)
+			}
+
+			if accountID := diff.Get("account_id").(string); aws.StringValue(snapshot.OwnerId) == accountID {
+				return fmt.Errorf("AWS Account (%s) owns EBS Snapshot (%s)", accountID, snapshotID)
 			}
 		}
-		return attrs, "denied", nil
 	}
+
+	return nil
 }
 
-func SnapshotCreateVolumePermissionParseID(id string) (string, string, error) {
-	idParts := strings.SplitN(id, "-", 3)
-	if len(idParts) != 3 || idParts[0] != "snap" || idParts[1] == "" || idParts[2] == "" {
-		return "", "", fmt.Errorf("unexpected format of ID (%s), expected SNAPSHOT_ID-ACCOUNT_ID", id)
+const ebsSnapshotCreateVolumePermissionIDSeparator = "-"
+
+func EBSSnapshotCreateVolumePermissionCreateResourceID(snapshotID, accountID string) string {
+	parts := []string{snapshotID, accountID}
+	id := strings.Join(parts, ebsSnapshotCreateVolumePermissionIDSeparator)
+
+	return id
+}
+
+func EBSSnapshotCreateVolumePermissionParseResourceID(id string) (string, string, error) {
+	parts := strings.SplitN(id, ebsSnapshotCreateVolumePermissionIDSeparator, 3)
+
+	if len(parts) != 3 || parts[0] != "snap" || parts[1] == "" || parts[2] == "" {
+		return "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected SNAPSHOT_ID%[2]sACCOUNT_ID", id, ebsSnapshotCreateVolumePermissionIDSeparator)
 	}
-	return fmt.Sprintf("%s-%s", idParts[0], idParts[1]), idParts[2], nil
+
+	return strings.Join([]string{parts[0], parts[1]}, ebsSnapshotCreateVolumePermissionIDSeparator), parts[2], nil
 }