Merge pull request #41388 from hashicorp/b-cloudformation-retries-exc…

…eeded resource/aws_cloudformation_stack_set_instance: Prevents conflicting creation retries
hashicorp · Feb 13, 2025 · 7c61b60 · 7c61b60
2 parents f8efaa5 + b0408f2
commit 7c61b60
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 133 deletions.
diff --git a/.changelog/41388.txt b/.changelog/41388.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_cloudformation_stack_set_instance: Prevents overly-long creation times and possible `OperationInProgress` errors
+```
diff --git a/.teamcity/components/generated/service_orgacct.kt b/.teamcity/components/generated/service_orgacct.kt
@@ -1,10 +1,7 @@
 val orgacctServices = mapOf(
     "accessanalyzer" to ServiceSpec("IAM Access Analyzer"),
     "backup" to ServiceSpec("Backup", "TestAccBackupGlobalSettings_basic"),
-    "cloudformation" to ServiceSpec(
-        "CloudFormation",
-        "TestAccCloudFormationStackSet_PermissionModel_serviceManaged|TestAccCloudFormationStackSetInstance_deploymentTargets"
-    ),
+    "cloudformation" to ServiceSpec("CloudFormation"),
     "cloudtrail" to ServiceSpec("CloudTrail"),
     "config" to ServiceSpec("Config" /*"TestAccConfig_serial|TestAccConfigConfigurationAggregator_"*/),
     "detective" to ServiceSpec("Detective"),

diff --git a/internal/service/cloudformation/errors.go b/internal/service/cloudformation/errors.go
@@ -3,6 +3,48 @@
 
 package cloudformation
 
+import (
+	"strings"
+)
+
 const (
 	errCodeValidationError = "ValidationError"
 )
+
+func isRetryableIAMPropagationErr(err error) (bool, error) {
+	if err == nil {
+		return false, nil
+	}
+
+	message := err.Error()
+
+	// IAM eventual consistency
+	if strings.Contains(message, "AccountGate check failed") {
+		return true, err
+	}
+
+	// IAM eventual consistency
+	// User: XXX is not authorized to perform: cloudformation:CreateStack on resource: YYY
+	if strings.Contains(message, "is not authorized") {
+		return true, err
+	}
+
+	// IAM eventual consistency
+	// XXX role has insufficient YYY permissions
+	if strings.Contains(message, "role has insufficient") {
+		return true, err
+	}
+
+	// IAM eventual consistency
+	// Account XXX should have YYY role with trust relationship to Role ZZZ
+	if strings.Contains(message, "role with trust relationship") {
+		return true, err
+	}
+
+	// IAM eventual consistency
+	if strings.Contains(message, "The security token included in the request is invalid") {
+		return true, err
+	}
+
+	return false, err
+}
diff --git a/internal/service/cloudformation/stack_instances.go b/internal/service/cloudformation/stack_instances.go
@@ -315,43 +315,7 @@ func resourceStackInstancesCreate(ctx context.Context, d *schema.ResourceData, m
 
 			return operation, nil
 		},
-		func(err error) (bool, error) {
-			if err == nil {
-				return false, nil
-			}
-
-			message := err.Error()
-
-			// IAM eventual consistency
-			if strings.Contains(message, "AccountGate check failed") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// User: XXX is not authorized to perform: cloudformation:CreateStack on resource: YYY
-			if strings.Contains(message, "is not authorized") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// XXX role has insufficient YYY permissions
-			if strings.Contains(message, "role has insufficient") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// Account XXX should have YYY role with trust relationship to Role ZZZ
-			if strings.Contains(message, "role with trust relationship") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			if strings.Contains(message, "The security token included in the request is invalid") {
-				return true, err
-			}
-
-			return false, err
-		},
+		isRetryableIAMPropagationErr,
 	)
 
 	if err != nil {

diff --git a/internal/service/cloudformation/stack_set.go b/internal/service/cloudformation/stack_set.go
@@ -288,43 +288,7 @@ func resourceStackSetCreate(ctx context.Context, d *schema.ResourceData, meta in
 
 			return operation, nil
 		},
-		func(err error) (bool, error) {
-			if err == nil {
-				return false, nil
-			}
-
-			message := err.Error()
-
-			// IAM eventual consistency
-			if strings.Contains(message, "AccountGate check failed") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// User: XXX is not authorized to perform: cloudformation:CreateStack on resource: YYY
-			if strings.Contains(message, "is not authorized") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// XXX role has insufficient YYY permissions
-			if strings.Contains(message, "role has insufficient") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// Account XXX should have YYY role with trust relationship to Role ZZZ
-			if strings.Contains(message, "role with trust relationship") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			if strings.Contains(message, "The security token included in the request is invalid") {
-				return true, err
-			}
-
-			return false, err
-		},
+		isRetryableIAMPropagationErr,
 	)
 
 	if err != nil {

diff --git a/internal/service/cloudformation/stack_set_instance.go b/internal/service/cloudformation/stack_set_instance.go
@@ -279,69 +279,25 @@ func resourceStackSetInstanceCreate(ctx context.Context, d *schema.ResourceData,
 		return create.AppendDiagError(diags, names.CloudFormation, create.ErrActionFlatteningResourceId, ResNameStackSetInstance, id, err)
 	}
 
-	_, err = tfresource.RetryWhen(ctx, propagationTimeout,
-		func() (interface{}, error) {
+	output, err := tfresource.RetryGWhen(ctx, propagationTimeout,
+		func() (*cloudformation.CreateStackInstancesOutput, error) {
 			input.OperationId = aws.String(sdkid.UniqueId())
 
-			output, err := conn.CreateStackInstances(ctx, input)
-
-			if err != nil {
-				return nil, err
-			}
-
-			d.SetId(id)
-
-			operation, err := waitStackSetOperationSucceeded(ctx, conn, stackSetName, aws.ToString(output.OperationId), callAs, d.Timeout(schema.TimeoutCreate))
-
-			if err != nil {
-				return nil, fmt.Errorf("waiting for create: %w", err)
-			}
-
-			return operation, nil
-		},
-		func(err error) (bool, error) {
-			if err == nil {
-				return false, nil
-			}
-
-			message := err.Error()
-
-			// IAM eventual consistency
-			if strings.Contains(message, "AccountGate check failed") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// User: XXX is not authorized to perform: cloudformation:CreateStack on resource: YYY
-			if strings.Contains(message, "is not authorized") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// XXX role has insufficient YYY permissions
-			if strings.Contains(message, "role has insufficient") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			// Account XXX should have YYY role with trust relationship to Role ZZZ
-			if strings.Contains(message, "role with trust relationship") {
-				return true, err
-			}
-
-			// IAM eventual consistency
-			if strings.Contains(message, "The security token included in the request is invalid") {
-				return true, err
-			}
-
-			return false, err
+			return conn.CreateStackInstances(ctx, input)
 		},
+		isRetryableIAMPropagationErr,
 	)
-
 	if err != nil {
 		return sdkdiag.AppendErrorf(diags, "creating CloudFormation StackSet (%s) Instance: %s", stackSetName, err)
 	}
 
+	d.SetId(id)
+
+	_, err = waitStackSetOperationSucceeded(ctx, conn, stackSetName, aws.ToString(output.OperationId), callAs, d.Timeout(schema.TimeoutCreate))
+	if err != nil {
+		return sdkdiag.AppendErrorf(diags, "creating CloudFormation StackSet (%s) Instance: waiting for completion: %s", stackSetName, err)
+	}
+
 	return append(diags, resourceStackSetInstanceRead(ctx, d, meta)...)
 }