provider: Introduce shared IAM propagation timeout, refactor KMS Key …

…creation to that timeout and add KMS Key deletion to internal waiter package (hashicorp#12863)

Reference: hashicorp#7646
Reference: hashicorp#12840

The Terraform AWS Provider codebase contains many varied timeouts for handling IAM propagation retries. Here we introduce a shared constant for this amount of time. The choice of 2 minutes is based on that amount of time being:

- Most widely used across resources
- Based on lack of historical bug reports across those resources that implement that amount of time, most successful
- Ensuring a reasonable user experience (not waiting too long) should there be an actual misconfiguration

As an initial implementation of this IAM propagation timeout and further showing the potential waiter package refactoring, this fixes shorter IAM timeout implementations in the `aws_kms_key` and `aws_kms_external_key` resources, while also refactoring the pending deletion logic. This second change is designed as an inflection point for how we want to handle imports across multiple waiter packages, with the preference of this initial implementation to name the Go import of the outside service, `iamwaiter`, or generically SERVICEwaiter. If agreed, this will be added to the proposal and the refactoring documentation.

NOTE: There is other `StateChangeConf` / `StateRefreshFunc` logic in these KMS resources, but this change is solely focused on highlighting the multiple import situation, and those will be handled later.

Output from acceptance testing:

```
--- PASS: TestAccAWSKmsExternalKey_basic (19.53s)
--- PASS: TestAccAWSKmsExternalKey_DeletionWindowInDays (31.61s)
--- PASS: TestAccAWSKmsExternalKey_Description (32.11s)
--- PASS: TestAccAWSKmsExternalKey_disappears (13.84s)
--- PASS: TestAccAWSKmsExternalKey_Enabled (312.55s)
--- PASS: TestAccAWSKmsExternalKey_KeyMaterialBase64 (104.29s)
--- PASS: TestAccAWSKmsExternalKey_Policy (33.78s)
--- PASS: TestAccAWSKmsExternalKey_Tags (43.70s)
--- PASS: TestAccAWSKmsExternalKey_ValidTo (165.77s)

--- PASS: TestAccAWSKmsKey_asymmetricKey (18.20s)
--- PASS: TestAccAWSKmsKey_basic (21.13s)
--- PASS: TestAccAWSKmsKey_disappears (13.92s)
--- PASS: TestAccAWSKmsKey_isEnabled (236.91s)
--- PASS: TestAccAWSKmsKey_policy (35.34s)
--- PASS: TestAccAWSKmsKey_Policy_IamRole (34.14s)
--- PASS: TestAccAWSKmsKey_Policy_IamServiceLinkedRole (44.80s)
--- PASS: TestAccAWSKmsKey_tags (34.65s)
```

aws/internal/service/iam/waiter/waiter.go

@@ -0,0 +1,14 @@
+package waiter
+
+import (
+	"time"
+)
+
+const (
+	// Maximum amount of time to wait for IAM changes to propagate
+	// This timeout should not be increased without strong consideration
+	// as this will negatively impact user experience when configurations
+	// have incorrect references or permissions.
+	// Reference: https://docs.aws.amazon.com/IAM/latest/UserGuide/troubleshoot_general.html#troubleshoot_general_eventual-consistency
+	PropagationTimeout = 2 * time.Minute
+)

aws/internal/service/kms/waiter/status.go

@@ -0,0 +1,28 @@
+package waiter
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/kms"
+	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
+)
+
+// KeyState fetches the Key and its State
+func KeyState(conn *kms.KMS, keyID string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		input := &kms.DescribeKeyInput{
+			KeyId: aws.String(keyID),
+		}
+
+		output, err := conn.DescribeKey(input)
+
+		if err != nil {
+			return nil, kms.KeyStateUnavailable, err
+		}
+
+		if output == nil || output.KeyMetadata == nil {
+			return output, kms.KeyStateUnavailable, nil
+		}
+
+		return output, aws.StringValue(output.KeyMetadata.KeyState), nil
+	}
+}

aws/internal/service/kms/waiter/waiter.go

@@ -0,0 +1,34 @@
+package waiter
+
+import (
+	"time"
+
+	"github.com/aws/aws-sdk-go/service/kms"
+	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
+)
+
+const (
+	// Maximum amount of time to wait for KeyState to return PendingDeletion
+	KeyStatePendingDeletionTimeout = 20 * time.Minute
+)
+
+// KeyStatePendingDeletion waits for KeyState to return PendingDeletion
+func KeyStatePendingDeletion(conn *kms.KMS, keyID string) (*kms.DescribeKeyOutput, error) {
+	stateConf := &resource.StateChangeConf{
+		Pending: []string{
+			kms.KeyStateDisabled,
+			kms.KeyStateEnabled,
+		},
+		Target:  []string{kms.KeyStatePendingDeletion},
+		Refresh: KeyState(conn, keyID),
+		Timeout: KeyStatePendingDeletionTimeout,
+	}
+
+	outputRaw, err := stateConf.WaitForState()
+
+	if output, ok := outputRaw.(*kms.DescribeKeyOutput); ok {
+		return output, err
+	}
+
+	return nil, err
+}

aws/resource_aws_kms_external_key.go

@@ -17,6 +17,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/structure"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+	iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter"
 )
 
 func resourceAwsKmsExternalKey() *schema.Resource {
@@ -110,7 +112,7 @@ func resourceAwsKmsExternalKeyCreate(d *schema.ResourceData, meta interface{}) e
 	}
 
 	var output *kms.CreateKeyOutput
-	err := resource.Retry(1*time.Minute, func() *resource.RetryError {
+	err := resource.Retry(iamwaiter.PropagationTimeout, func() *resource.RetryError {
 		var err error
 
 		output, err = conn.CreateKey(input)
@@ -332,11 +334,17 @@ func resourceAwsKmsExternalKeyDelete(d *schema.ResourceData, meta interface{}) e
 	}
 
 	if err != nil {
-		return fmt.Errorf("error scheduling KMS External Key (%s) deletion: %s", d.Id(), err)
+		return fmt.Errorf("error scheduling deletion for KMS Key (%s): %w", d.Id(), err)
+	}
+
+	_, err = waiter.KeyStatePendingDeletion(conn, d.Id())
+
+	if isAWSErr(err, kms.ErrCodeNotFoundException, "") {
+		return nil
 	}
 
-	if err := waitForKmsKeyScheduleDeletion(conn, d.Id()); err != nil {
-		return fmt.Errorf("error waiting for KMS External Key (%s) deletion scheduling: %s", d.Id(), err)
+	if err != nil {
+		return fmt.Errorf("error waiting for KMS Key (%s) to schedule deletion: %w", d.Id(), err)
 	}
 
 	return nil
@@ -440,39 +448,3 @@ func importKmsExternalKeyMaterial(conn *kms.KMS, keyID, keyMaterialBase64, valid
 
 	return nil
 }
-
-func waitForKmsKeyScheduleDeletion(conn *kms.KMS, keyID string) error {
-	// Wait for propagation since KMS is eventually consistent
-	input := &kms.DescribeKeyInput{
-		KeyId: aws.String(keyID),
-	}
-
-	wait := resource.StateChangeConf{
-		Pending:                   []string{kms.KeyStateDisabled, kms.KeyStateEnabled},
-		Target:                    []string{kms.KeyStatePendingDeletion},
-		Timeout:                   20 * time.Minute,
-		MinTimeout:                2 * time.Second,
-		ContinuousTargetOccurence: 10,
-		Refresh: func() (interface{}, string, error) {
-			output, err := conn.DescribeKey(input)
-
-			if isAWSErr(err, kms.ErrCodeNotFoundException, "") {
-				return 42, kms.KeyStatePendingDeletion, nil
-			}
-
-			if err != nil {
-				return nil, kms.KeyStateUnavailable, err
-			}
-
-			if output == nil || output.KeyMetadata == nil {
-				return 42, kms.KeyStatePendingDeletion, nil
-			}
-
-			return output, aws.StringValue(output.KeyMetadata.KeyState), nil
-		},
-	}
-
-	_, err := wait.WaitForState()
-
-	return err
-}

aws/resource_aws_kms_external_key_test.go

@@ -10,7 +10,8 @@ import (
 	"github.com/aws/aws-sdk-go/service/kms"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/terraform"
-	"github.com/jen20/awspolicyequivalence"
+	awspolicy "github.com/jen20/awspolicyequivalence"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter"
 )
 
 func TestAccAWSKmsExternalKey_basic(t *testing.T) {
@@ -483,7 +484,9 @@ func testAccCheckAWSKmsExternalKeyDisappears(key *kms.KeyMetadata) resource.Test
 			return err
 		}
 
-		return waitForKmsKeyScheduleDeletion(conn, aws.StringValue(key.KeyId))
+		_, err = waiter.KeyStatePendingDeletion(conn, aws.StringValue(key.KeyId))
+
+		return err
 	}
 }
 

aws/resource_aws_kms_key.go

@@ -13,6 +13,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/structure"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+	iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter"
 )
 
 func resourceAwsKmsKey() *schema.Resource {
@@ -116,7 +118,7 @@ func resourceAwsKmsKeyCreate(d *schema.ResourceData, meta interface{}) error {
 	// The KMS service's awareness of principals is limited by "eventual consistency".
 	// They acknowledge this here:
 	// http://docs.aws.amazon.com/kms/latest/APIReference/API_CreateKey.html
-	err := resource.Retry(30*time.Second, func() *resource.RetryError {
+	err := resource.Retry(iamwaiter.PropagationTimeout, func() *resource.RetryError {
 		var err error
 		resp, err = conn.CreateKey(req)
 		if isAWSErr(err, kms.ErrCodeMalformedPolicyDocumentException, "") {
@@ -471,39 +473,24 @@ func resourceAwsKmsKeyDelete(d *schema.ResourceData, meta interface{}) error {
 		req.PendingWindowInDays = aws.Int64(int64(v.(int)))
 	}
 	_, err := conn.ScheduleKeyDeletion(req)
-	if err != nil {
-		return err
+
+	if isAWSErr(err, kms.ErrCodeNotFoundException, "") {
+		return nil
 	}
 
-	// Wait for propagation since KMS is eventually consistent
-	wait := resource.StateChangeConf{
-		Pending:                   []string{kms.KeyStateEnabled, kms.KeyStateDisabled},
-		Target:                    []string{kms.KeyStatePendingDeletion},
-		Timeout:                   20 * time.Minute,
-		MinTimeout:                2 * time.Second,
-		ContinuousTargetOccurence: 10,
-		Refresh: func() (interface{}, string, error) {
-			log.Printf("[DEBUG] Checking if KMS key %s state is PendingDeletion", keyId)
-			resp, err := conn.DescribeKey(&kms.DescribeKeyInput{
-				KeyId: aws.String(keyId),
-			})
-			if err != nil {
-				return resp, "Failed", err
-			}
+	if err != nil {
+		return fmt.Errorf("error scheduling deletion for KMS Key (%s): %w", d.Id(), err)
+	}
 
-			metadata := *resp.KeyMetadata
-			log.Printf("[DEBUG] KMS key %s state is %s, retrying", keyId, *metadata.KeyState)
+	_, err = waiter.KeyStatePendingDeletion(conn, d.Id())
 
-			return resp, *metadata.KeyState, nil
-		},
+	if isAWSErr(err, kms.ErrCodeNotFoundException, "") {
+		return nil
 	}
 
-	_, err = wait.WaitForState()
 	if err != nil {
-		return fmt.Errorf("Failed deactivating KMS key %s: %s", keyId, err)
+		return fmt.Errorf("error waiting for KMS Key (%s) to schedule deletion: %w", d.Id(), err)
 	}
 
-	log.Printf("[DEBUG] KMS Key %s deactivated.", keyId)
-
 	return nil
 }