From fb2072b7664ce00020157c8d0dc0476eb552f5bd Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 19 Aug 2025 17:06:45 -0400 Subject: [PATCH 01/26] service cat debuggery --- .../servicecatalog/provisioned_product.go | 37 ++- .../provisioned_product_test.go | 229 ++++++++++++++++++ internal/service/servicecatalog/wait.go | 25 +- 3 files changed, 286 insertions(+), 5 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 0694b1512b82..d78f97d8edee 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -438,24 +438,35 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, // but this can interfere complete reads of this resource when an error occurs after initial creation // or after an invalid update that returns a 'FAILED' record state. Thus, waiters are now present in the CREATE and UPDATE methods of this resource instead. // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/24574#issuecomment-1126339193 + + // For TAINTED resources, we need to get parameters from the last successful record + // not the last provisioning record which may have failed + var recordIdToUse *string + if detail.Status == awstypes.ProvisionedProductStatusTainted && detail.LastSuccessfulProvisioningRecordId != nil { + recordIdToUse = detail.LastSuccessfulProvisioningRecordId + log.Printf("[DEBUG] Service Catalog Provisioned Product (%s) is TAINTED, using last successful record %s for parameter values", d.Id(), aws.ToString(recordIdToUse)) + } else { + recordIdToUse = detail.LastProvisioningRecordId + } + recordInput := &servicecatalog.DescribeRecordInput{ - Id: detail.LastProvisioningRecordId, + Id: recordIdToUse, AcceptLanguage: aws.String(acceptLanguage), } recordOutput, err := conn.DescribeRecord(ctx, recordInput) if !d.IsNewResource() && errs.IsA[*awstypes.ResourceNotFoundException](err) { - log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(detail.LastProvisioningRecordId)) + log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(recordIdToUse)) return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(detail.LastProvisioningRecordId), err) + return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(recordIdToUse), err) } if recordOutput == nil || recordOutput.RecordDetail == nil { - return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(detail.LastProvisioningRecordId)) + return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(recordIdToUse)) } // To enable debugging of potential v, log as a warning @@ -554,6 +565,24 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat } if _, err := waitProvisionedProductReady(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutUpdate)); err != nil { + // Check if this is a state inconsistency error + var failureErr *ProvisionedProductFailureError + if errors.As(err, &failureErr) && failureErr.IsStateInconsistent() { + // Force a state refresh to get actual AWS values before returning error + log.Printf("[WARN] Service Catalog Provisioned Product (%s) update failed with status %s, refreshing state", d.Id(), failureErr.Status) + + // Perform state refresh to get actual current values from AWS + refreshDiags := resourceProvisionedProductRead(ctx, d, meta) + if refreshDiags.HasError() { + // If refresh fails, return both errors + return append(refreshDiags, sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err)...) + } + + // Return the original failure error after state is corrected + return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) + } + + // For other errors, proceed as before return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) } diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 048b9294b70b..642c86198312 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -15,6 +15,7 @@ import ( awstypes "github.com/aws/aws-sdk-go-v2/service/servicecatalog/types" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -988,3 +989,231 @@ resource "aws_s3_bucket" "conflict" { } `, rName, conflictingBucketName, tagValue)) } + +// TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate reproduces the exact bug scenario: +// +// When a Service Catalog provisioned product update fails, the resource becomes TAINTED. +// The bug is that subsequent `terraform apply` shows "no changes" instead of retrying +// the failed update automatically. +// +// Expected behavior: TAINTED resources should always trigger an update attempt. +// Current (buggy) behavior: TAINTED resources show "no changes" in plan. +// +// This test should FAIL at Step 4 with the current implementation, proving the bug exists. +// Step 4 uses ConfigPlanChecks to verify that an update action should be planned. +func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_servicecatalog_provisioned_product.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + var pprod awstypes.ProvisionedProductDetail + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ServiceCatalogServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckProvisionedProductDestroy(ctx), + Steps: []resource.TestStep{ + // Step 1: Create with working configuration using simple template + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_working(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), + resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), + ), + }, + // Step 2: Update to failing configuration - this should fail and leave resource TAINTED + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_failing(rName), + ExpectError: regexache.MustCompile(`Properties validation failed`), + }, + // Step 3: Verify resource is now TAINTED after the failed update + // Use RefreshOnly to avoid triggering any plan changes due to config differences + { + RefreshState: true, + Check: resource.ComposeTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), + testAccCheckProvisionedProductStatus(ctx, resourceName, "TAINTED"), + ), + }, + // Step 4: CRITICAL TEST - Apply the same failing config again + // BUG: Currently this shows "no changes" but should retry the update + // ConfigPlanChecks should FAIL with current implementation, demonstrating the bug + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_failing(rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + ExpectError: regexache.MustCompile(`Properties validation failed`), + }, + // Step 5: Clean up by applying a working config + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_working(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), + resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), + ), + }, + }, + }) +} + +// Helper function to check provisioned product status +func testAccCheckProvisionedProductStatus(ctx context.Context, resourceName, expectedStatus string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("resource not found: %s", resourceName) + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).ServiceCatalogClient(ctx) + + input := &servicecatalog.DescribeProvisionedProductInput{ + Id: aws.String(rs.Primary.ID), + AcceptLanguage: aws.String(tfservicecatalog.AcceptLanguageEnglish), + } + + output, err := conn.DescribeProvisionedProduct(ctx, input) + if err != nil { + return fmt.Errorf("describing Service Catalog Provisioned Product (%s): %w", rs.Primary.ID, err) + } + + if output.ProvisionedProductDetail == nil { + return fmt.Errorf("Service Catalog Provisioned Product (%s) not found", rs.Primary.ID) + } + + actualStatus := string(output.ProvisionedProductDetail.Status) + if actualStatus != expectedStatus { + return fmt.Errorf("Service Catalog Provisioned Product (%s) status: expected %s, got %s", + rs.Primary.ID, expectedStatus, actualStatus) + } + + return nil + } +} + +// testAccProvisionedProductConfig_retryTaintedUpdate_working creates a simple working CloudFormation template +// This avoids the complex conditional logic that was causing issues in the basic config +func testAccProvisionedProductConfig_retryTaintedUpdate_working(rName string) string { + return acctest.ConfigCompose( + testAccProvisionedProductPortfolioBaseConfig(rName), + fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + force_destroy = true +} + +resource "aws_s3_object" "test" { + bucket = aws_s3_bucket.test.id + key = "%[1]s.json" + + content = jsonencode({ + AWSTemplateFormatVersion = "2010-09-09" + + Resources = { + TestBucket = { + Type = "AWS::S3::Bucket" + } + } + + Outputs = { + BucketName = { + Description = "Bucket Name" + Value = { + Ref = "TestBucket" + } + } + } + }) +} + +resource "aws_servicecatalog_product" "test" { + description = %[1]q + distributor = "distributör" + name = %[1]q + owner = "ägare" + type = "CLOUD_FORMATION_TEMPLATE" + support_description = %[1]q + + provisioning_artifact_parameters { + description = "artefaktbeskrivning" + disable_template_validation = true + name = %[1]q + template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" + type = "CLOUD_FORMATION_TEMPLATE" + } +} + +resource "aws_servicecatalog_provisioned_product" "test" { + name = %[1]q + product_id = aws_servicecatalog_product.test.id + provisioning_artifact_name = %[1]q + path_id = data.aws_servicecatalog_launch_paths.test.summaries[0].path_id +} +`, rName)) +} + +// testAccProvisionedProductConfig_retryTaintedUpdate_failing creates a CloudFormation template that will fail +// This uses an invalid S3 bucket name to trigger a CloudFormation validation error +func testAccProvisionedProductConfig_retryTaintedUpdate_failing(rName string) string { + return acctest.ConfigCompose( + testAccProvisionedProductPortfolioBaseConfig(rName), + fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + force_destroy = true +} + +resource "aws_s3_object" "test" { + bucket = aws_s3_bucket.test.id + key = "%[1]s.json" + + content = jsonencode({ + AWSTemplateFormatVersion = "2010-09-09" + + Resources = { + TestBucket = { + Type = "AWS::S3::Bucket" + Properties = { + BucketName = "INVALID_BUCKET_NAME_WITH_UPPERCASE_AND_UNDERSCORES" + } + } + } + + Outputs = { + BucketName = { + Description = "Bucket Name" + Value = { + Ref = "TestBucket" + } + } + } + }) +} + +resource "aws_servicecatalog_product" "test" { + description = %[1]q + distributor = "distributör" + name = %[1]q + owner = "ägare" + type = "CLOUD_FORMATION_TEMPLATE" + support_description = %[1]q + + provisioning_artifact_parameters { + description = "artefaktbeskrivning" + disable_template_validation = true + name = %[1]q + template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" + type = "CLOUD_FORMATION_TEMPLATE" + } +} + +resource "aws_servicecatalog_provisioned_product" "test" { + name = %[1]q + product_id = aws_servicecatalog_product.test.id + provisioning_artifact_name = %[1]q + path_id = data.aws_servicecatalog_launch_paths.test.summaries[0].path_id +} +`, rName)) +} diff --git a/internal/service/servicecatalog/wait.go b/internal/service/servicecatalog/wait.go index d3600429438a..4fa01d21bdfb 100644 --- a/internal/service/servicecatalog/wait.go +++ b/internal/service/servicecatalog/wait.go @@ -75,6 +75,24 @@ const ( organizationAccessStatusError = "ERROR" ) +// ProvisionedProductFailureError represents a provisioned product operation failure +// that requires state refresh to recover from inconsistent state. +type ProvisionedProductFailureError struct { + StatusMessage string + Status string + NeedsRefresh bool +} + +func (e *ProvisionedProductFailureError) Error() string { + return e.StatusMessage +} + +// IsStateInconsistent returns true if this error indicates state inconsistency +// that requires a refresh to recover. +func (e *ProvisionedProductFailureError) IsStateInconsistent() bool { + return e.NeedsRefresh +} + func waitProductReady(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, productID string, timeout time.Duration) (*servicecatalog.DescribeProductAsAdminOutput, error) { stateConf := &retry.StateChangeConf{ Pending: enum.Slice(awstypes.StatusCreating, statusNotFound, statusUnavailable), @@ -485,7 +503,12 @@ func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Clien // The difference is that, in the case of `TAINTED`, there is a previous version to roll back to. status := string(detail.Status) if status == string(awstypes.ProvisionedProductStatusError) || status == string(awstypes.ProvisionedProductStatusTainted) { - return output, errors.New(aws.ToString(detail.StatusMessage)) + // Create a custom error type that signals state refresh is needed + return output, &ProvisionedProductFailureError{ + StatusMessage: aws.ToString(detail.StatusMessage), + Status: status, + NeedsRefresh: true, + } } } } From 9dfca5088d33483a517cffa47bb04e061632c70f Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 21 Aug 2025 07:56:23 -0700 Subject: [PATCH 02/26] Allows deletion when in `TAINTED` status --- internal/service/servicecatalog/provisioned_product.go | 4 +++- internal/service/servicecatalog/wait.go | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index d78f97d8edee..fd9943b16999 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -602,7 +602,9 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat input.AcceptLanguage = aws.String(v.(string)) } - if v, ok := d.GetOk("ignore_errors"); ok { + if v, ok := d.Get(names.AttrStatus).(string); ok && v == string(awstypes.ProvisionedProductStatusTainted) { + input.IgnoreErrors = true + } else if v, ok := d.GetOk("ignore_errors"); ok { input.IgnoreErrors = v.(bool) } diff --git a/internal/service/servicecatalog/wait.go b/internal/service/servicecatalog/wait.go index 4fa01d21bdfb..e62f42c5e299 100644 --- a/internal/service/servicecatalog/wait.go +++ b/internal/service/servicecatalog/wait.go @@ -520,7 +520,11 @@ func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Clien func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) error { stateConf := &retry.StateChangeConf{ - Pending: enum.Slice(awstypes.ProvisionedProductStatusAvailable, awstypes.ProvisionedProductStatusUnderChange), + Pending: enum.Slice( + awstypes.ProvisionedProductStatusAvailable, + awstypes.ProvisionedProductStatusTainted, + awstypes.ProvisionedProductStatusUnderChange, + ), Target: []string{}, Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), Timeout: timeout, From 75ba85081b5c177d57aa5fa9e29b7d68bb5baa21 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 21 Aug 2025 10:27:53 -0700 Subject: [PATCH 03/26] Retries creating `aws_servicecatalog_constraint` while waiting for IAM permissions to propagate --- internal/service/servicecatalog/constraint.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/service/servicecatalog/constraint.go b/internal/service/servicecatalog/constraint.go index 41c08c672867..ece51a3d4549 100644 --- a/internal/service/servicecatalog/constraint.go +++ b/internal/service/servicecatalog/constraint.go @@ -117,6 +117,10 @@ func resourceConstraintCreate(ctx context.Context, d *schema.ResourceData, meta return retry.RetryableError(err) } + if errs.IsAErrorMessageContains[*awstypes.InvalidParametersException](err, "Access denied while assuming the role") { + return retry.RetryableError(err) + } + if errs.IsA[*awstypes.ResourceNotFoundException](err) { return retry.RetryableError(err) } From da402c438693ae7d9cb06dd80e0e4b9408a5cc70 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 21 Aug 2025 12:57:39 -0700 Subject: [PATCH 04/26] Handles deletion if `TAINTED` or becomes `TAINTED` during deletion --- .../servicecatalog/provisioned_product.go | 25 +++++++++++++++---- internal/service/servicecatalog/wait.go | 22 ++++++++++++++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index fd9943b16999..b052986e9c27 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -593,7 +593,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat var diags diag.Diagnostics conn := meta.(*conns.AWSClient).ServiceCatalogClient(ctx) - input := &servicecatalog.TerminateProvisionedProductInput{ + input := servicecatalog.TerminateProvisionedProductInput{ TerminateToken: aws.String(id.UniqueId()), ProvisionedProductId: aws.String(d.Id()), } @@ -602,9 +602,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat input.AcceptLanguage = aws.String(v.(string)) } - if v, ok := d.Get(names.AttrStatus).(string); ok && v == string(awstypes.ProvisionedProductStatusTainted) { - input.IgnoreErrors = true - } else if v, ok := d.GetOk("ignore_errors"); ok { + if v, ok := d.GetOk("ignore_errors"); ok { input.IgnoreErrors = v.(bool) } @@ -612,7 +610,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat input.RetainPhysicalResources = v.(bool) } - _, err := conn.TerminateProvisionedProduct(ctx, input) + _, err := conn.TerminateProvisionedProduct(ctx, &input) if errs.IsA[*awstypes.ResourceNotFoundException](err) { return diags @@ -624,6 +622,23 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat err = waitProvisionedProductTerminated(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutDelete)) + if failureErr, ok := errs.As[*ProvisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { + input.IgnoreErrors = true + + _, err = conn.TerminateProvisionedProduct(ctx, &input) + + if errs.IsA[*awstypes.ResourceNotFoundException](err) { + return diags + } + + if err != nil { + return sdkdiag.AppendErrorf(diags, "terminating Service Catalog Provisioned Product (%s): %s", d.Id(), err) + } + + err = waitProvisionedProductTerminated(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutDelete)) + + } + if errs.IsA[*awstypes.ResourceNotFoundException](err) { return diags } diff --git a/internal/service/servicecatalog/wait.go b/internal/service/servicecatalog/wait.go index e62f42c5e299..4795fe778a8b 100644 --- a/internal/service/servicecatalog/wait.go +++ b/internal/service/servicecatalog/wait.go @@ -522,7 +522,6 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. stateConf := &retry.StateChangeConf{ Pending: enum.Slice( awstypes.ProvisionedProductStatusAvailable, - awstypes.ProvisionedProductStatusTainted, awstypes.ProvisionedProductStatusUnderChange, ), Target: []string{}, @@ -530,7 +529,26 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. Timeout: timeout, } - _, err := stateConf.WaitForStateContext(ctx) + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { + if detail := output.ProvisionedProductDetail; detail != nil { + var foo *retry.UnexpectedStateError + if errors.As(err, &foo) { + // If the status is `TAINTED`, we can retry with `IgnoreErrors` + status := string(detail.Status) + if status == string(awstypes.ProvisionedProductStatusTainted) { + // Create a custom error type that signals state refresh is needed + return &ProvisionedProductFailureError{ + StatusMessage: aws.ToString(detail.StatusMessage), + Status: status, + NeedsRefresh: true, + } + } + } + } + return err + } return err } From fb7fe0293d138a42e11a06c049476a9eac199dea Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 21 Aug 2025 13:38:16 -0700 Subject: [PATCH 05/26] Updates test for current behaviour --- .../provisioned_product_test.go | 208 ++++++++++-------- .../testdata/foo/product_template.yaml | 167 ++++++++++++++ 2 files changed, 280 insertions(+), 95 deletions(-) create mode 100644 internal/service/servicecatalog/testdata/foo/product_template.yaml diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 642c86198312..2787732ee32d 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1004,6 +1004,9 @@ resource "aws_s3_bucket" "conflict" { func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { ctx := acctest.Context(t) resourceName := "aws_servicecatalog_provisioned_product.test" + const artifactsDataSourceName = "data.aws_servicecatalog_provisioning_artifacts.product_artifacts" + const initialArtifactID = "provisioning_artifact_details.0.id" + const newArtifactID = "provisioning_artifact_details.1.id" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) var pprod awstypes.ProvisionedProductDetail @@ -1015,44 +1018,66 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { Steps: []resource.TestStep{ // Step 1: Create with working configuration using simple template { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_working(rName), - Check: resource.ComposeTestCheckFunc( + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "none"), + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), + resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, initialArtifactID), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "false"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "none"), ), }, + // Step 2: Update to failing configuration - this should fail and leave resource TAINTED { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_failing(rName), - ExpectError: regexache.MustCompile(`Properties validation failed`), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "changed_once"), + ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), }, + // Step 3: Verify resource is now TAINTED after the failed update // Use RefreshOnly to avoid triggering any plan changes due to config differences { RefreshState: true, - Check: resource.ComposeTestCheckFunc( - testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), + Check: resource.ComposeAggregateTestCheckFunc( + // testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), // Can't use this because it fails on TAINTED testAccCheckProvisionedProductStatus(ctx, resourceName, "TAINTED"), + resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "TAINTED"), + resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "true"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_once"), ), }, + // Step 4: CRITICAL TEST - Apply the same failing config again // BUG: Currently this shows "no changes" but should retry the update // ConfigPlanChecks should FAIL with current implementation, demonstrating the bug { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_failing(rName), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "changed_once"), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), }, }, - ExpectError: regexache.MustCompile(`Properties validation failed`), }, + // Step 5: Clean up by applying a working config { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_working(rName), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "changed_to_force_an_update"), Check: resource.ComposeTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), + resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "false"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_to_force_an_update"), ), }, }, @@ -1093,73 +1118,72 @@ func testAccCheckProvisionedProductStatus(ctx context.Context, resourceName, exp } } -// testAccProvisionedProductConfig_retryTaintedUpdate_working creates a simple working CloudFormation template +// testAccProvisionedProductConfig_retryTaintedUpdate creates a simple working CloudFormation template // This avoids the complex conditional logic that was causing issues in the basic config -func testAccProvisionedProductConfig_retryTaintedUpdate_working(rName string) string { +func testAccProvisionedProductConfig_retryTaintedUpdate(rName string, useNewVersion bool, simulateFailure bool, extraParam string) string { return acctest.ConfigCompose( testAccProvisionedProductPortfolioBaseConfig(rName), fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q - force_destroy = true -} - -resource "aws_s3_object" "test" { - bucket = aws_s3_bucket.test.id - key = "%[1]s.json" +resource "aws_servicecatalog_provisioned_product" "test" { + name = %[1]q + product_id = aws_servicecatalog_product.test.id + provisioning_artifact_id = %[2]t ? local.new_provisioning_artifact.id : local.initial_provisioning_artifact.id - content = jsonencode({ - AWSTemplateFormatVersion = "2010-09-09" + provisioning_parameters { + key = "FailureSimulation" + value = "%[3]t" + } - Resources = { - TestBucket = { - Type = "AWS::S3::Bucket" - } - } + provisioning_parameters { + key = "ExtraParam" + value = %[4]q + } - Outputs = { - BucketName = { - Description = "Bucket Name" - Value = { - Ref = "TestBucket" - } - } - } - }) + depends_on = [ + aws_servicecatalog_constraint.launch_constraint, + ] } resource "aws_servicecatalog_product" "test" { description = %[1]q - distributor = "distributör" name = %[1]q owner = "ägare" type = "CLOUD_FORMATION_TEMPLATE" - support_description = %[1]q provisioning_artifact_parameters { - description = "artefaktbeskrivning" - disable_template_validation = true - name = %[1]q - template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" - type = "CLOUD_FORMATION_TEMPLATE" + name = "%[1]s - Initial" + description = "Initial" + template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" + type = "CLOUD_FORMATION_TEMPLATE" } } -resource "aws_servicecatalog_provisioned_product" "test" { - name = %[1]q - product_id = aws_servicecatalog_product.test.id - provisioning_artifact_name = %[1]q - path_id = data.aws_servicecatalog_launch_paths.test.summaries[0].path_id +resource "aws_servicecatalog_provisioning_artifact" "new_version" { + product_id = aws_servicecatalog_product.test.id + + name = "%[1]s - New" + description = "New" + template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" + type = "CLOUD_FORMATION_TEMPLATE" + + # Force a new version to be created when MPI version changes + # Is this needed? + lifecycle { + create_before_destroy = true + } } -`, rName)) + +data "aws_servicecatalog_provisioning_artifacts" "product_artifacts" { + product_id = aws_servicecatalog_product.test.id + + depends_on = [aws_servicecatalog_provisioning_artifact.new_version] +} + +locals { + initial_provisioning_artifact = data.aws_servicecatalog_provisioning_artifacts.product_artifacts.provisioning_artifact_details[0] + new_provisioning_artifact = data.aws_servicecatalog_provisioning_artifacts.product_artifacts.provisioning_artifact_details[1] } -// testAccProvisionedProductConfig_retryTaintedUpdate_failing creates a CloudFormation template that will fail -// This uses an invalid S3 bucket name to trigger a CloudFormation validation error -func testAccProvisionedProductConfig_retryTaintedUpdate_failing(rName string) string { - return acctest.ConfigCompose( - testAccProvisionedProductPortfolioBaseConfig(rName), - fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q force_destroy = true @@ -1167,53 +1191,47 @@ resource "aws_s3_bucket" "test" { resource "aws_s3_object" "test" { bucket = aws_s3_bucket.test.id - key = "%[1]s.json" + key = "product_template.yaml" - content = jsonencode({ - AWSTemplateFormatVersion = "2010-09-09" + source = "${path.module}/testdata/foo/product_template.yaml" +} - Resources = { - TestBucket = { - Type = "AWS::S3::Bucket" - Properties = { - BucketName = "INVALID_BUCKET_NAME_WITH_UPPERCASE_AND_UNDERSCORES" - } - } - } +# Required to validate provisioned product on update +resource "aws_servicecatalog_constraint" "launch_constraint" { + description = "Launch constraint for test product" + portfolio_id = aws_servicecatalog_portfolio.test.id + product_id = aws_servicecatalog_product.test.id + type = "LAUNCH" - Outputs = { - BucketName = { - Description = "Bucket Name" - Value = { - Ref = "TestBucket" - } - } - } + parameters = jsonencode({ + "RoleArn" = aws_iam_role.launch_role.arn }) -} -resource "aws_servicecatalog_product" "test" { - description = %[1]q - distributor = "distributör" - name = %[1]q - owner = "ägare" - type = "CLOUD_FORMATION_TEMPLATE" - support_description = %[1]q + depends_on = [aws_iam_role_policy_attachment.launch_role] +} - provisioning_artifact_parameters { - description = "artefaktbeskrivning" - disable_template_validation = true - name = %[1]q - template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" - type = "CLOUD_FORMATION_TEMPLATE" - } +# IAM role for Service Catalog launch constraint +resource "aws_iam_role" "launch_role" { + name = "%[1]s-launch-role" + + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Action = "sts:AssumeRole" + Effect = "Allow" + Principal = { + Service = "servicecatalog.amazonaws.com" + } + } + ] + }) } -resource "aws_servicecatalog_provisioned_product" "test" { - name = %[1]q - product_id = aws_servicecatalog_product.test.id - provisioning_artifact_name = %[1]q - path_id = data.aws_servicecatalog_launch_paths.test.summaries[0].path_id +# Attach admin policy to launch role (for demo purposes only) +resource "aws_iam_role_policy_attachment" "launch_role" { + role = aws_iam_role.launch_role.name + policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess" } -`, rName)) +`, rName, useNewVersion, simulateFailure, extraParam)) } diff --git a/internal/service/servicecatalog/testdata/foo/product_template.yaml b/internal/service/servicecatalog/testdata/foo/product_template.yaml new file mode 100644 index 000000000000..cfcb21370405 --- /dev/null +++ b/internal/service/servicecatalog/testdata/foo/product_template.yaml @@ -0,0 +1,167 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: 'Test product template for taint reproduction' + +Parameters: + ExtraParam: + Type: String + Description: Extra parameter + Default: 'none' + + FailureSimulation: + Type: String + Description: Boolean to simulate failure + Default: 'false' + AllowedValues: + - 'true' + - 'false' + +Resources: + TestLambdaFunction: + Type: AWS::Lambda::Function + Properties: + FunctionName: !Sub '${AWS::StackName}-test-function' + Handler: index.handler + Role: !GetAtt LambdaExecutionRole.Arn + Runtime: nodejs18.x + Code: + ZipFile: | + // cfnresponse module inline + const cfnresponse = (() => { + const SUCCESS = "SUCCESS"; + const FAILED = "FAILED"; + + function send(event, context, responseStatus, responseData, physicalResourceId, noEcho) { + return new Promise((resolve, reject) => { + const responseBody = JSON.stringify({ + Status: responseStatus, + Reason: `See the details in CloudWatch Log Stream: ${context.logStreamName}`, + PhysicalResourceId: physicalResourceId || context.logStreamName, + StackId: event.StackId, + RequestId: event.RequestId, + LogicalResourceId: event.LogicalResourceId, + NoEcho: noEcho || false, + Data: responseData || {} + }); + + console.log("Response body:", responseBody); + + const https = require("https"); + const url = require("url"); + + const parsedUrl = url.parse(event.ResponseURL); + const options = { + hostname: parsedUrl.hostname, + port: 443, + path: parsedUrl.path, + method: "PUT", + headers: { + "content-type": "", + "content-length": responseBody.length + } + }; + + const request = https.request(options, (response) => { + console.log(`Status code: ${response.statusCode}`); + console.log(`Status message: ${response.statusMessage}`); + resolve(); + }); + + request.on("error", (error) => { + console.log(`Send CFN response failed: ${error}`); + reject(error); + }); + + request.write(responseBody); + request.end(); + }); + } + + return { + SUCCESS, + FAILED, + send + }; + })(); + + exports.handler = async (event, context) => { + console.log('Event:', JSON.stringify(event)); + + // For CloudFormation custom resources + if (event.RequestType) { + try { + // Simulate failure if parameter is set to true + if (event.ResourceProperties.FailureSimulation === 'true' && + (event.RequestType === 'Create' || event.RequestType === 'Update')) { + console.log('Simulating failure as requested'); + await cfnresponse.send(event, context, cfnresponse.FAILED, { + Message: 'Simulated failure' + }); + return; + } + + // Process the request based on the RequestType + let responseData = {}; + + if (event.RequestType === 'Create' || event.RequestType === 'Update') { + // Perform create or update actions + responseData = { + Message: 'Resource created/updated successfully', + Timestamp: new Date().toISOString() + }; + } else if (event.RequestType === 'Delete') { + // Perform delete actions + responseData = { + Message: 'Resource deleted successfully' + }; + } + + // Send success response + await cfnresponse.send(event, context, cfnresponse.SUCCESS, responseData); + } catch (error) { + console.error('Error:', error); + await cfnresponse.send(event, context, cfnresponse.FAILED, { + Error: error.message + }); + } + } else { + // For direct Lambda invocations + // Simulate failure if parameter is set to true + if (event.FailureSimulation === 'true') { + throw new Error('Simulated failure'); + } + + return { + statusCode: 200, + body: JSON.stringify('Success'), + }; + } + }; + Environment: + Variables: + # MPIVersion: !Ref MPIVersion + FailureSimulation: !Ref FailureSimulation + ExtraParam: !Ref ExtraParam + + LambdaExecutionRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Principal: + Service: lambda.amazonaws.com + Action: 'sts:AssumeRole' + ManagedPolicyArns: + - 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole' + + TestInvocation: + Type: Custom::TestInvocation + Properties: + ServiceToken: !GetAtt TestLambdaFunction.Arn + FailureSimulation: !Ref FailureSimulation + +Outputs: + LambdaArn: + Description: ARN of the Lambda function + Value: !GetAtt TestLambdaFunction.Arn From 0c92f86c5b101ee70ed623b7d440e178c1296a95 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 21 Aug 2025 14:17:44 -0700 Subject: [PATCH 06/26] Reverts resource changes from "service cat debuggery" --- .../servicecatalog/provisioned_product.go | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index b052986e9c27..534eabbd3787 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -439,34 +439,38 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, // or after an invalid update that returns a 'FAILED' record state. Thus, waiters are now present in the CREATE and UPDATE methods of this resource instead. // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/24574#issuecomment-1126339193 - // For TAINTED resources, we need to get parameters from the last successful record - // not the last provisioning record which may have failed - var recordIdToUse *string - if detail.Status == awstypes.ProvisionedProductStatusTainted && detail.LastSuccessfulProvisioningRecordId != nil { - recordIdToUse = detail.LastSuccessfulProvisioningRecordId - log.Printf("[DEBUG] Service Catalog Provisioned Product (%s) is TAINTED, using last successful record %s for parameter values", d.Id(), aws.ToString(recordIdToUse)) - } else { - recordIdToUse = detail.LastProvisioningRecordId - } + // // For TAINTED resources, we need to get parameters from the last successful record + // // not the last provisioning record which may have failed + // var recordIdToUse *string + // if detail.Status == awstypes.ProvisionedProductStatusTainted && detail.LastSuccessfulProvisioningRecordId != nil { + // recordIdToUse = detail.LastSuccessfulProvisioningRecordId + // log.Printf("[DEBUG] Service Catalog Provisioned Product (%s) is TAINTED, using last successful record %s for parameter values", d.Id(), aws.ToString(recordIdToUse)) + // } else { + // recordIdToUse = detail.LastProvisioningRecordId + // } recordInput := &servicecatalog.DescribeRecordInput{ - Id: recordIdToUse, + Id: detail.LastProvisioningRecordId, + // Id: recordIdToUse, AcceptLanguage: aws.String(acceptLanguage), } recordOutput, err := conn.DescribeRecord(ctx, recordInput) if !d.IsNewResource() && errs.IsA[*awstypes.ResourceNotFoundException](err) { - log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(recordIdToUse)) + log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(detail.LastProvisioningRecordId)) + // log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(recordIdToUse)) return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(recordIdToUse), err) + return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(detail.LastProvisioningRecordId), err) + // return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(recordIdToUse), err) } if recordOutput == nil || recordOutput.RecordDetail == nil { - return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(recordIdToUse)) + return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(detail.LastProvisioningRecordId)) + // return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(recordIdToUse)) } // To enable debugging of potential v, log as a warning @@ -565,24 +569,24 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat } if _, err := waitProvisionedProductReady(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutUpdate)); err != nil { - // Check if this is a state inconsistency error - var failureErr *ProvisionedProductFailureError - if errors.As(err, &failureErr) && failureErr.IsStateInconsistent() { - // Force a state refresh to get actual AWS values before returning error - log.Printf("[WARN] Service Catalog Provisioned Product (%s) update failed with status %s, refreshing state", d.Id(), failureErr.Status) - - // Perform state refresh to get actual current values from AWS - refreshDiags := resourceProvisionedProductRead(ctx, d, meta) - if refreshDiags.HasError() { - // If refresh fails, return both errors - return append(refreshDiags, sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err)...) - } - - // Return the original failure error after state is corrected - return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) - } - - // For other errors, proceed as before + // // Check if this is a state inconsistency error + // var failureErr *ProvisionedProductFailureError + // if errors.As(err, &failureErr) && failureErr.IsStateInconsistent() { + // // Force a state refresh to get actual AWS values before returning error + // log.Printf("[WARN] Service Catalog Provisioned Product (%s) update failed with status %s, refreshing state", d.Id(), failureErr.Status) + + // // Perform state refresh to get actual current values from AWS + // refreshDiags := resourceProvisionedProductRead(ctx, d, meta) + // if refreshDiags.HasError() { + // // If refresh fails, return both errors + // return append(refreshDiags, sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err)...) + // } + + // // Return the original failure error after state is corrected + // return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) + // } + + // // For other errors, proceed as before return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) } From c8d9f45b5cfc4399195743b6d59c100f8ce4a38f Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 21 Aug 2025 14:18:10 -0700 Subject: [PATCH 07/26] Updates tests --- .../provisioned_product_test.go | 58 +++++++++++-------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 2787732ee32d..819509c221cc 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1016,9 +1016,9 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckProvisionedProductDestroy(ctx), Steps: []resource.TestStep{ - // Step 1: Create with working configuration using simple template + // Step 1: Create with working configuration { - Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "none"), + Config: testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), @@ -1031,44 +1031,44 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { ), }, - // Step 2: Update to failing configuration - this should fail and leave resource TAINTED + // Step 2: Update to failing configuration { - Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "changed_once"), + Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), }, - // Step 3: Verify resource is now TAINTED after the failed update - // Use RefreshOnly to avoid triggering any plan changes due to config differences - { - RefreshState: true, - Check: resource.ComposeAggregateTestCheckFunc( - // testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), // Can't use this because it fails on TAINTED - testAccCheckProvisionedProductStatus(ctx, resourceName, "TAINTED"), - resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "TAINTED"), - resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "true"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_once"), - ), - }, + // // Step 3: Verify resource is now TAINTED after the failed update + // // Use RefreshState to avoid triggering any plan changes due to config differences + // { + // RefreshState: true, + // Check: resource.ComposeAggregateTestCheckFunc( + // // testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), // Can't use this because it fails on TAINTED + // testAccCheckProvisionedProductStatus(ctx, resourceName, "TAINTED"), + // resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "TAINTED"), + // resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), + // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), + // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), + // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "true"), + // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), + // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_once"), + // ), + // }, // Step 4: CRITICAL TEST - Apply the same failing config again // BUG: Currently this shows "no changes" but should retry the update // ConfigPlanChecks should FAIL with current implementation, demonstrating the bug { - Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "changed_once"), + Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), }, }, }, // Step 5: Clean up by applying a working config { - Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "changed_to_force_an_update"), + Config: testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName), Check: resource.ComposeTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), @@ -1118,6 +1118,18 @@ func testAccCheckProvisionedProductStatus(ctx context.Context, resourceName, exp } } +func testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName string) string { + return testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "none") +} + +func testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName string) string { + return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "changed_once") +} + +func testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName string) string { + return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "changed_to_force_an_update") +} + // testAccProvisionedProductConfig_retryTaintedUpdate creates a simple working CloudFormation template // This avoids the complex conditional logic that was causing issues in the basic config func testAccProvisionedProductConfig_retryTaintedUpdate(rName string, useNewVersion bool, simulateFailure bool, extraParam string) string { From 1039db41a619496107fa84c1344da1bb3bf54263 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 27 Aug 2025 14:02:16 -0400 Subject: [PATCH 08/26] Add copyright header. --- .../service/servicecatalog/testdata/foo/product_template.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/service/servicecatalog/testdata/foo/product_template.yaml b/internal/service/servicecatalog/testdata/foo/product_template.yaml index cfcb21370405..e63f55771e3e 100644 --- a/internal/service/servicecatalog/testdata/foo/product_template.yaml +++ b/internal/service/servicecatalog/testdata/foo/product_template.yaml @@ -1,3 +1,6 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + AWSTemplateFormatVersion: '2010-09-09' Description: 'Test product template for taint reproduction' From d7d8d27c51e807bd902e4eac04b4727bead9f5c7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 27 Aug 2025 14:03:36 -0400 Subject: [PATCH 09/26] Run 'make fix-constants PKG=servicecatalog'. --- internal/service/servicecatalog/provisioned_product_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 819509c221cc..5d66ed93b181 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1025,7 +1025,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, initialArtifactID), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "false"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", acctest.CtFalse), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "none"), ), @@ -1075,7 +1075,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "false"), + resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", acctest.CtFalse), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_to_force_an_update"), ), From 848d84cbb34b6b8573f480784c82368c38704ef2 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 27 Aug 2025 14:06:42 -0400 Subject: [PATCH 10/26] Fix providerlint 'AWSAT005: avoid hardcoded ARN AWS partitions, use aws_partition data source'. --- internal/service/servicecatalog/provisioned_product_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 5d66ed93b181..b4a0716c869d 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1240,10 +1240,12 @@ resource "aws_iam_role" "launch_role" { }) } +data "aws_partition" "current" {} + # Attach admin policy to launch role (for demo purposes only) resource "aws_iam_role_policy_attachment" "launch_role" { role = aws_iam_role.launch_role.name - policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess" + policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/AdministratorAccess" } `, rName, useNewVersion, simulateFailure, extraParam)) } From c2be5c35bdf1183e069e5d023f23be6763da4bad Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 27 Aug 2025 14:07:26 -0400 Subject: [PATCH 11/26] Fix golangci-lint 'whitespace'. --- internal/service/servicecatalog/provisioned_product.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 534eabbd3787..1ea7178f2c56 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -640,7 +640,6 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat } err = waitProvisionedProductTerminated(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutDelete)) - } if errs.IsA[*awstypes.ResourceNotFoundException](err) { From 59e2323b167f58998d86b31ee7c8d3133f6d2218 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 27 Aug 2025 14:08:54 -0400 Subject: [PATCH 12/26] Use 'testAccCheckProvisionedProductStatus'. --- internal/service/servicecatalog/provisioned_product_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index b4a0716c869d..3cf51a0ad6dd 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1021,6 +1021,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { Config: testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), + testAccCheckProvisionedProductStatus(ctx, resourceName, "AVAILABLE"), resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, initialArtifactID), resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), From 8c2329921216d4336587665275bc492927594bfa Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 27 Aug 2025 14:10:03 -0400 Subject: [PATCH 13/26] Fix terrafmt errors. --- .../service/servicecatalog/provisioned_product_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 3cf51a0ad6dd..e84ec23061cc 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1158,10 +1158,10 @@ resource "aws_servicecatalog_provisioned_product" "test" { } resource "aws_servicecatalog_product" "test" { - description = %[1]q - name = %[1]q - owner = "ägare" - type = "CLOUD_FORMATION_TEMPLATE" + description = %[1]q + name = %[1]q + owner = "ägare" + type = "CLOUD_FORMATION_TEMPLATE" provisioning_artifact_parameters { name = "%[1]s - Initial" From c21a1b0d531a821948c0474e66ead09fe977d120 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 28 Aug 2025 12:17:09 -0400 Subject: [PATCH 14/26] Add 'findProvisionedProduct' and 'findRecord'. --- .../service/servicecatalog/exports_test.go | 7 +- .../servicecatalog/provisioned_product.go | 354 +++++++++++++----- .../provisioned_product_test.go | 26 +- internal/service/servicecatalog/status.go | 33 -- internal/service/servicecatalog/wait.go | 95 ----- 5 files changed, 272 insertions(+), 243 deletions(-) diff --git a/internal/service/servicecatalog/exports_test.go b/internal/service/servicecatalog/exports_test.go index fe83cfeeba24..420e157fed30 100644 --- a/internal/service/servicecatalog/exports_test.go +++ b/internal/service/servicecatalog/exports_test.go @@ -18,9 +18,10 @@ var ( ResourceTagOption = resourceTagOption ResourceTagOptionResourceAssociation = resourceTagOptionResourceAssociation - FindPortfolioByID = findPortfolioByID - FindPortfolioShare = findPortfolioShare - FindPrincipalPortfolioAssociation = findPrincipalPortfolioAssociation + FindPortfolioByID = findPortfolioByID + FindPortfolioShare = findPortfolioShare + FindPrincipalPortfolioAssociation = findPrincipalPortfolioAssociation + FindProvisionedProductByTwoPartKey = findProvisionedProductByTwoPartKey BudgetResourceAssociationParseID = budgetResourceAssociationParseID ProductPortfolioAssociationParseID = productPortfolioAssociationParseID diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 1ea7178f2c56..a7f3b85ca233 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/enum" "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" @@ -44,10 +45,9 @@ func resourceProvisionedProduct() *schema.Resource { }, Timeouts: &schema.ResourceTimeout{ - Create: schema.DefaultTimeout(ProvisionedProductReadyTimeout), - Read: schema.DefaultTimeout(ProvisionedProductReadTimeout), - Update: schema.DefaultTimeout(ProvisionedProductUpdateTimeout), - Delete: schema.DefaultTimeout(ProvisionedProductDeleteTimeout), + Create: schema.DefaultTimeout(30 * time.Minute), + Update: schema.DefaultTimeout(30 * time.Minute), + Delete: schema.DefaultTimeout(30 * time.Minute), }, Schema: map[string]*schema.Schema{ @@ -281,9 +281,10 @@ func resourceProvisionedProductCreate(ctx context.Context, d *schema.ResourceDat var diags diag.Diagnostics conn := meta.(*conns.AWSClient).ServiceCatalogClient(ctx) - input := &servicecatalog.ProvisionProductInput{ + name := d.Get(names.AttrName).(string) + input := servicecatalog.ProvisionProductInput{ ProvisionToken: aws.String(id.UniqueId()), - ProvisionedProductName: aws.String(d.Get(names.AttrName).(string)), + ProvisionedProductName: aws.String(name), Tags: getTagsIn(ctx), } @@ -327,42 +328,25 @@ func resourceProvisionedProductCreate(ctx context.Context, d *schema.ResourceDat input.ProvisioningPreferences = expandProvisioningPreferences(v.([]any)[0].(map[string]any)) } - var output *servicecatalog.ProvisionProductOutput - - err := retry.RetryContext(ctx, d.Timeout(schema.TimeoutCreate), func() *retry.RetryError { - var err error - - output, err = conn.ProvisionProduct(ctx, input) - - if errs.IsAErrorMessageContains[*awstypes.InvalidParametersException](err, "profile does not exist") { - return retry.RetryableError(err) - } - - if errs.IsA[*awstypes.ResourceNotFoundException](err) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } + output, err := tfresource.RetryWhen(ctx, d.Timeout(schema.TimeoutCreate), + func(ctx context.Context) (*servicecatalog.ProvisionProductOutput, error) { + return conn.ProvisionProduct(ctx, &input) + }, + func(err error) (bool, error) { + if errs.IsAErrorMessageContains[*awstypes.InvalidParametersException](err, "profile does not exist") { + return true, err + } - return nil - }) + if errs.IsA[*awstypes.ResourceNotFoundException](err) { + return true, err + } - if tfresource.TimedOut(err) { - output, err = conn.ProvisionProduct(ctx, input) - } + return false, err + }, + ) if err != nil { - return sdkdiag.AppendErrorf(diags, "provisioning Service Catalog Product: %s", err) - } - - if output == nil { - return sdkdiag.AppendErrorf(diags, "provisioning Service Catalog Product: empty response") - } - - if output.RecordDetail == nil { - return sdkdiag.AppendErrorf(diags, "provisioning Service Catalog Product: no product view detail or summary") + return sdkdiag.AppendErrorf(diags, "provisioning Service Catalog Product (%s): %s", name, err) } d.SetId(aws.ToString(output.RecordDetail.ProvisionedProductId)) @@ -386,43 +370,30 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, // DescribeRecord is available in the data source aws_servicecatalog_record. acceptLanguage := acceptLanguageEnglish - if v, ok := d.GetOk("accept_language"); ok { acceptLanguage = v.(string) } - input := &servicecatalog.DescribeProvisionedProductInput{ - Id: aws.String(d.Id()), - AcceptLanguage: aws.String(acceptLanguage), - } - - output, err := conn.DescribeProvisionedProduct(ctx, input) + outputDPP, err := findProvisionedProductByTwoPartKey(ctx, conn, d.Id(), acceptLanguage) - if !d.IsNewResource() && errs.IsA[*awstypes.ResourceNotFoundException](err) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Service Catalog Provisioned Product (%s) not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "reading Service Catalog Provisioned Product (%s): %s", d.Id(), err) } - if output == nil || output.ProvisionedProductDetail == nil { - return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s): empty response", d.Id()) - } - - detail := output.ProvisionedProductDetail - + detail := outputDPP.ProvisionedProductDetail d.Set(names.AttrARN, detail.Arn) - d.Set("cloudwatch_dashboard_names", flattenCloudWatchDashboards(output.CloudWatchDashboards)) - + d.Set("cloudwatch_dashboard_names", flattenCloudWatchDashboards(outputDPP.CloudWatchDashboards)) if detail.CreatedTime != nil { d.Set(names.AttrCreatedTime, detail.CreatedTime.Format(time.RFC3339)) } else { d.Set(names.AttrCreatedTime, nil) } - d.Set("last_provisioning_record_id", detail.LastProvisioningRecordId) d.Set("last_record_id", detail.LastRecordId) d.Set("last_successful_provisioning_record_id", detail.LastSuccessfulProvisioningRecordId) @@ -449,35 +420,23 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, // recordIdToUse = detail.LastProvisioningRecordId // } - recordInput := &servicecatalog.DescribeRecordInput{ - Id: detail.LastProvisioningRecordId, - // Id: recordIdToUse, - AcceptLanguage: aws.String(acceptLanguage), - } - - recordOutput, err := conn.DescribeRecord(ctx, recordInput) + recordIdToUse := aws.ToString(detail.LastProvisioningRecordId) + outputDR, err := findRecordByTwoPartKey(ctx, conn, recordIdToUse, acceptLanguage) - if !d.IsNewResource() && errs.IsA[*awstypes.ResourceNotFoundException](err) { - log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(detail.LastProvisioningRecordId)) - // log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), aws.ToString(recordIdToUse)) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Service Catalog Provisioned Product (%s) Record (%s) not found, unable to set tags", d.Id(), recordIdToUse) return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(detail.LastProvisioningRecordId), err) - // return sdkdiag.AppendErrorf(diags, "describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(recordIdToUse), err) - } - - if recordOutput == nil || recordOutput.RecordDetail == nil { - return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(detail.LastProvisioningRecordId)) - // return sdkdiag.AppendErrorf(diags, "getting Service Catalog Provisioned Product (%s) Record (%s): empty response", d.Id(), aws.ToString(recordIdToUse)) + return sdkdiag.AppendErrorf(diags, "reading Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), recordIdToUse, err) } // To enable debugging of potential v, log as a warning // instead of exiting prematurely with an error, e.g. // v can be present after update to a new version failed and the stack // rolled back to the current version. - if v := recordOutput.RecordDetail.RecordErrors; len(v) > 0 { + if v := outputDR.RecordDetail.RecordErrors; len(v) > 0 { var errs []error for _, err := range v { @@ -487,13 +446,13 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, log.Printf("[WARN] Errors found when describing Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), aws.ToString(detail.LastProvisioningRecordId), errors.Join(errs...)) } - if err := d.Set("outputs", flattenRecordOutputs(recordOutput.RecordOutputs)); err != nil { + if err := d.Set("outputs", flattenRecordOutputs(outputDR.RecordOutputs)); err != nil { return sdkdiag.AppendErrorf(diags, "setting outputs: %s", err) } - d.Set("path_id", recordOutput.RecordDetail.PathId) + d.Set("path_id", outputDR.RecordDetail.PathId) - setTagsOut(ctx, svcTags(recordKeyValueTags(ctx, recordOutput.RecordDetail.RecordTags))) + setTagsOut(ctx, svcTags(recordKeyValueTags(ctx, outputDR.RecordDetail.RecordTags))) return diags } @@ -502,9 +461,9 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat var diags diag.Diagnostics conn := meta.(*conns.AWSClient).ServiceCatalogClient(ctx) - input := &servicecatalog.UpdateProvisionedProductInput{ - UpdateToken: aws.String(id.UniqueId()), + input := servicecatalog.UpdateProvisionedProductInput{ ProvisionedProductId: aws.String(d.Id()), + UpdateToken: aws.String(id.UniqueId()), } if v, ok := d.GetOk("accept_language"); ok { @@ -546,23 +505,18 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat // to provisioned AWS objects during update if the tags don't change. input.Tags = getTagsIn(ctx) - err := retry.RetryContext(ctx, d.Timeout(schema.TimeoutUpdate), func() *retry.RetryError { - _, err := conn.UpdateProvisionedProduct(ctx, input) - - if errs.IsAErrorMessageContains[*awstypes.InvalidParametersException](err, "profile does not exist") { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } - - return nil - }) + _, err := tfresource.RetryWhen(ctx, d.Timeout(schema.TimeoutUpdate), + func(ctx context.Context) (any, error) { + return conn.UpdateProvisionedProduct(ctx, &input) + }, + func(err error) (bool, error) { + if errs.IsAErrorMessageContains[*awstypes.InvalidParametersException](err, "profile does not exist") { + return true, err + } - if tfresource.TimedOut(err) { - _, err = conn.UpdateProvisionedProduct(ctx, input) - } + return false, err + }, + ) if err != nil { return sdkdiag.AppendErrorf(diags, "updating Service Catalog Provisioned Product (%s): %s", d.Id(), err) @@ -626,7 +580,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat err = waitProvisionedProductTerminated(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutDelete)) - if failureErr, ok := errs.As[*ProvisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { + if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { input.IgnoreErrors = true _, err = conn.TerminateProvisionedProduct(ctx, &input) @@ -647,12 +601,218 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat } if err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) to be terminated: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) terminate: %s", d.Id(), err) } return diags } +func findProvisionedProductByTwoPartKey(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string) (*servicecatalog.DescribeProvisionedProductOutput, error) { + input := servicecatalog.DescribeProvisionedProductInput{ + Id: aws.String(id), + } + if acceptLanguage != "" { + input.AcceptLanguage = aws.String(acceptLanguage) + } + + return findProvisionedProduct(ctx, conn, &input) +} + +func findProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, input *servicecatalog.DescribeProvisionedProductInput) (*servicecatalog.DescribeProvisionedProductOutput, error) { + output, err := conn.DescribeProvisionedProduct(ctx, input) + + if errs.IsA[*awstypes.ResourceNotFoundException](err) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.ProvisionedProductDetail == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} + +func findRecordByTwoPartKey(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string) (*servicecatalog.DescribeRecordOutput, error) { + input := servicecatalog.DescribeRecordInput{ + Id: aws.String(id), + } + if acceptLanguage != "" { + input.AcceptLanguage = aws.String(acceptLanguage) + } + + return findRecord(ctx, conn, &input) +} + +func findRecord(ctx context.Context, conn *servicecatalog.Client, input *servicecatalog.DescribeRecordInput) (*servicecatalog.DescribeRecordOutput, error) { + var output *servicecatalog.DescribeRecordOutput + + for { + page, err := conn.DescribeRecord(ctx, input) + + if errs.IsA[*awstypes.ResourceNotFoundException](err) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if page == nil { + break + } + + if output == nil { + output = page + } else { + output.RecordOutputs = append(output.RecordOutputs, page.RecordOutputs...) + } + + nextPageToken := aws.ToString(page.NextPageToken) + if nextPageToken == "" { + break + } + input.PageToken = aws.String(nextPageToken) + } + + if output == nil || output.RecordDetail == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} + +func statusProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string) retry.StateRefreshFunc { + return func() (any, string, error) { + input := &servicecatalog.DescribeProvisionedProductInput{} + + if acceptLanguage != "" { + input.AcceptLanguage = aws.String(acceptLanguage) + } + + // one or the other but not both + if id != "" { + input.Id = aws.String(id) + } else if name != "" { + input.Name = aws.String(name) + } + + output, err := conn.DescribeProvisionedProduct(ctx, input) + + if errs.IsA[*awstypes.ResourceNotFoundException](err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + if output == nil || output.ProvisionedProductDetail == nil { + return nil, "", nil + } + + return output, string(output.ProvisionedProductDetail.Status), err + } +} + +func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { + stateConf := &retry.StateChangeConf{ + Pending: enum.Slice(awstypes.ProvisionedProductStatusUnderChange, awstypes.ProvisionedProductStatusPlanInProgress), + Target: enum.Slice(awstypes.ProvisionedProductStatusAvailable), + Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), + Timeout: timeout, + ContinuousTargetOccurence: continuousTargetOccurrence, + NotFoundChecks: notFoundChecks, + MinTimeout: minTimeout, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { + if detail := output.ProvisionedProductDetail; detail != nil { + var foo *retry.UnexpectedStateError + if errors.As(err, &foo) { + // The statuses `ERROR` and `TAINTED` are equivalent: the application of the requested change has failed. + // The difference is that, in the case of `TAINTED`, there is a previous version to roll back to. + status := string(detail.Status) + if status == string(awstypes.ProvisionedProductStatusError) || status == string(awstypes.ProvisionedProductStatusTainted) { + // Create a custom error type that signals state refresh is needed + return output, &provisionedProductFailureError{ + StatusMessage: aws.ToString(detail.StatusMessage), + Status: status, + NeedsRefresh: true, + } + } + } + } + return output, err + } + + return nil, err +} + +func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) error { + stateConf := &retry.StateChangeConf{ + Pending: enum.Slice( + awstypes.ProvisionedProductStatusAvailable, + awstypes.ProvisionedProductStatusUnderChange, + ), + Target: []string{}, + Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { + if detail := output.ProvisionedProductDetail; detail != nil { + var foo *retry.UnexpectedStateError + if errors.As(err, &foo) { + // If the status is `TAINTED`, we can retry with `IgnoreErrors` + status := string(detail.Status) + if status == string(awstypes.ProvisionedProductStatusTainted) { + // Create a custom error type that signals state refresh is needed + return &provisionedProductFailureError{ + StatusMessage: aws.ToString(detail.StatusMessage), + Status: status, + NeedsRefresh: true, + } + } + } + } + return err + } + + return err +} + +// provisionedProductFailureError represents a provisioned product operation failure +// that requires state refresh to recover from inconsistent state. +type provisionedProductFailureError struct { + StatusMessage string + Status string + NeedsRefresh bool +} + +func (e *provisionedProductFailureError) Error() string { + return e.StatusMessage +} + +// IsStateInconsistent returns true if this error indicates state inconsistency +// that requires a refresh to recover. +func (e *provisionedProductFailureError) IsStateInconsistent() bool { + return e.NeedsRefresh +} + func expandProvisioningParameter(tfMap map[string]any) awstypes.ProvisioningParameter { apiObject := awstypes.ProvisioningParameter{} diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index e84ec23061cc..e50c37778ef9 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -19,8 +19,8 @@ import ( "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" - "github.com/hashicorp/terraform-provider-aws/internal/errs" tfservicecatalog "github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -467,13 +467,9 @@ func testAccCheckProvisionedProductDestroy(ctx context.Context) resource.TestChe continue } - input := &servicecatalog.DescribeProvisionedProductInput{ - Id: aws.String(rs.Primary.ID), - AcceptLanguage: aws.String(rs.Primary.Attributes["accept_language"]), - } - _, err := conn.DescribeProvisionedProduct(ctx, input) + _, err := tfservicecatalog.FindProvisionedProductByTwoPartKey(ctx, conn, rs.Primary.ID, rs.Primary.Attributes["accept_language"]) - if errs.IsA[*awstypes.ResourceNotFoundException](err) { + if tfresource.NotFound(err) { continue } @@ -481,29 +477,29 @@ func testAccCheckProvisionedProductDestroy(ctx context.Context) resource.TestChe return err } - return fmt.Errorf("Service Catalog Provisioned Product (%s) still exists", rs.Primary.ID) + return fmt.Errorf("Service Catalog Provisioned Product %s still exists", rs.Primary.ID) } return nil } } -func testAccCheckProvisionedProductExists(ctx context.Context, resourceName string, pprod *awstypes.ProvisionedProductDetail) resource.TestCheckFunc { +func testAccCheckProvisionedProductExists(ctx context.Context, n string, v *awstypes.ProvisionedProductDetail) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] - + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("resource not found: %s", resourceName) + return fmt.Errorf("Not found: %s", n) } conn := acctest.Provider.Meta().(*conns.AWSClient).ServiceCatalogClient(ctx) - out, err := tfservicecatalog.WaitProvisionedProductReady(ctx, conn, tfservicecatalog.AcceptLanguageEnglish, rs.Primary.ID, "", tfservicecatalog.ProvisionedProductReadyTimeout) + output, err := tfservicecatalog.FindProvisionedProductByTwoPartKey(ctx, conn, rs.Primary.ID, rs.Primary.Attributes["accept_language"]) + if err != nil { - return fmt.Errorf("describing Service Catalog Provisioned Product (%s): %w", rs.Primary.ID, err) + return err } - *pprod = *out.ProvisionedProductDetail + *v = *output.ProvisionedProductDetail return nil } diff --git a/internal/service/servicecatalog/status.go b/internal/service/servicecatalog/status.go index 08543cedd860..b4ea55caf1fc 100644 --- a/internal/service/servicecatalog/status.go +++ b/internal/service/servicecatalog/status.go @@ -321,39 +321,6 @@ func statusLaunchPaths(ctx context.Context, conn *servicecatalog.Client, acceptL } } -func statusProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string) retry.StateRefreshFunc { - return func() (any, string, error) { - input := &servicecatalog.DescribeProvisionedProductInput{} - - if acceptLanguage != "" { - input.AcceptLanguage = aws.String(acceptLanguage) - } - - // one or the other but not both - if id != "" { - input.Id = aws.String(id) - } else if name != "" { - input.Name = aws.String(name) - } - - output, err := conn.DescribeProvisionedProduct(ctx, input) - - if errs.IsA[*awstypes.ResourceNotFoundException](err) { - return nil, "", nil - } - - if err != nil { - return nil, "", err - } - - if output == nil || output.ProvisionedProductDetail == nil { - return nil, "", nil - } - - return output, string(output.ProvisionedProductDetail.Status), err - } -} - func statusPortfolioConstraints(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, portfolioID, productID string) retry.StateRefreshFunc { return func() (any, string, error) { input := &servicecatalog.ListConstraintsForPortfolioInput{ diff --git a/internal/service/servicecatalog/wait.go b/internal/service/servicecatalog/wait.go index 4795fe778a8b..4e433f892193 100644 --- a/internal/service/servicecatalog/wait.go +++ b/internal/service/servicecatalog/wait.go @@ -5,10 +5,8 @@ package servicecatalog import ( "context" - "errors" "time" - "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/servicecatalog" awstypes "github.com/aws/aws-sdk-go-v2/service/servicecatalog/types" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" @@ -42,10 +40,6 @@ const ( ProductReadTimeout = 10 * time.Minute ProductReadyTimeout = 5 * time.Minute ProductUpdateTimeout = 5 * time.Minute - ProvisionedProductDeleteTimeout = 30 * time.Minute - ProvisionedProductReadTimeout = 10 * time.Minute - ProvisionedProductReadyTimeout = 30 * time.Minute - ProvisionedProductUpdateTimeout = 30 * time.Minute ProvisioningArtifactDeleteTimeout = 3 * time.Minute ProvisioningArtifactReadTimeout = 10 * time.Minute ProvisioningArtifactReadyTimeout = 3 * time.Minute @@ -75,24 +69,6 @@ const ( organizationAccessStatusError = "ERROR" ) -// ProvisionedProductFailureError represents a provisioned product operation failure -// that requires state refresh to recover from inconsistent state. -type ProvisionedProductFailureError struct { - StatusMessage string - Status string - NeedsRefresh bool -} - -func (e *ProvisionedProductFailureError) Error() string { - return e.StatusMessage -} - -// IsStateInconsistent returns true if this error indicates state inconsistency -// that requires a refresh to recover. -func (e *ProvisionedProductFailureError) IsStateInconsistent() bool { - return e.NeedsRefresh -} - func waitProductReady(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, productID string, timeout time.Duration) (*servicecatalog.DescribeProductAsAdminOutput, error) { stateConf := &retry.StateChangeConf{ Pending: enum.Slice(awstypes.StatusCreating, statusNotFound, statusUnavailable), @@ -482,77 +458,6 @@ func waitLaunchPathsReady(ctx context.Context, conn *servicecatalog.Client, acce return nil, err } -func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { - stateConf := &retry.StateChangeConf{ - Pending: enum.Slice(awstypes.ProvisionedProductStatusUnderChange, awstypes.ProvisionedProductStatusPlanInProgress), - Target: enum.Slice(awstypes.ProvisionedProductStatusAvailable), - Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), - Timeout: timeout, - ContinuousTargetOccurence: continuousTargetOccurrence, - NotFoundChecks: notFoundChecks, - MinTimeout: minTimeout, - } - - outputRaw, err := stateConf.WaitForStateContext(ctx) - - if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { - if detail := output.ProvisionedProductDetail; detail != nil { - var foo *retry.UnexpectedStateError - if errors.As(err, &foo) { - // The statuses `ERROR` and `TAINTED` are equivalent: the application of the requested change has failed. - // The difference is that, in the case of `TAINTED`, there is a previous version to roll back to. - status := string(detail.Status) - if status == string(awstypes.ProvisionedProductStatusError) || status == string(awstypes.ProvisionedProductStatusTainted) { - // Create a custom error type that signals state refresh is needed - return output, &ProvisionedProductFailureError{ - StatusMessage: aws.ToString(detail.StatusMessage), - Status: status, - NeedsRefresh: true, - } - } - } - } - return output, err - } - - return nil, err -} - -func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) error { - stateConf := &retry.StateChangeConf{ - Pending: enum.Slice( - awstypes.ProvisionedProductStatusAvailable, - awstypes.ProvisionedProductStatusUnderChange, - ), - Target: []string{}, - Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), - Timeout: timeout, - } - - outputRaw, err := stateConf.WaitForStateContext(ctx) - - if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { - if detail := output.ProvisionedProductDetail; detail != nil { - var foo *retry.UnexpectedStateError - if errors.As(err, &foo) { - // If the status is `TAINTED`, we can retry with `IgnoreErrors` - status := string(detail.Status) - if status == string(awstypes.ProvisionedProductStatusTainted) { - // Create a custom error type that signals state refresh is needed - return &ProvisionedProductFailureError{ - StatusMessage: aws.ToString(detail.StatusMessage), - Status: status, - NeedsRefresh: true, - } - } - } - } - return err - } - - return err -} - func waitPortfolioConstraintsReady(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, portfolioID, productID string, timeout time.Duration) ([]awstypes.ConstraintDetail, error) { stateConf := &retry.StateChangeConf{ Pending: []string{statusNotFound}, From 69fd52878eb14ae0fb39c77a62273b58d5aa1500 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 28 Aug 2025 14:24:26 -0400 Subject: [PATCH 15/26] statusProvisionedProduct: Use 'findProvisionedProductByTwoPartKey'. --- .../service/servicecatalog/exports_test.go | 1 - .../servicecatalog/provisioned_product.go | 39 ++++++------------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/internal/service/servicecatalog/exports_test.go b/internal/service/servicecatalog/exports_test.go index 420e157fed30..f7b830ca5ef5 100644 --- a/internal/service/servicecatalog/exports_test.go +++ b/internal/service/servicecatalog/exports_test.go @@ -37,7 +37,6 @@ var ( WaitOrganizationsAccessStable = waitOrganizationsAccessStable WaitProductPortfolioAssociationDeleted = waitProductPortfolioAssociationDeleted WaitProductPortfolioAssociationReady = waitProductPortfolioAssociationReady - WaitProvisionedProductReady = waitProvisionedProductReady WaitTagOptionResourceAssociationDeleted = waitTagOptionResourceAssociationDeleted WaitTagOptionResourceAssociationReady = waitTagOptionResourceAssociationReady diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index a7f3b85ca233..f760b86241ef 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -351,7 +351,7 @@ func resourceProvisionedProductCreate(ctx context.Context, d *schema.ResourceDat d.SetId(aws.ToString(output.RecordDetail.ProvisionedProductId)) - if _, err := waitProvisionedProductReady(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutCreate)); err != nil { + if _, err := waitProvisionedProductReady(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutCreate)); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) create: %s", d.Id(), err) } @@ -522,7 +522,7 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat return sdkdiag.AppendErrorf(diags, "updating Service Catalog Provisioned Product (%s): %s", d.Id(), err) } - if _, err := waitProvisionedProductReady(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutUpdate)); err != nil { + if _, err := waitProvisionedProductReady(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutUpdate)); err != nil { // // Check if this is a state inconsistency error // var failureErr *ProvisionedProductFailureError // if errors.As(err, &failureErr) && failureErr.IsStateInconsistent() { @@ -578,7 +578,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat return sdkdiag.AppendErrorf(diags, "terminating Service Catalog Provisioned Product (%s): %s", d.Id(), err) } - err = waitProvisionedProductTerminated(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutDelete)) + err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { input.IgnoreErrors = true @@ -593,7 +593,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat return sdkdiag.AppendErrorf(diags, "terminating Service Catalog Provisioned Product (%s): %s", d.Id(), err) } - err = waitProvisionedProductTerminated(ctx, conn, d.Get("accept_language").(string), d.Id(), "", d.Timeout(schema.TimeoutDelete)) + err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) } if errs.IsA[*awstypes.ResourceNotFoundException](err) { @@ -691,24 +691,11 @@ func findRecord(ctx context.Context, conn *servicecatalog.Client, input *service return output, nil } -func statusProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string) retry.StateRefreshFunc { +func statusProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string) retry.StateRefreshFunc { return func() (any, string, error) { - input := &servicecatalog.DescribeProvisionedProductInput{} + output, err := findProvisionedProductByTwoPartKey(ctx, conn, id, acceptLanguage) - if acceptLanguage != "" { - input.AcceptLanguage = aws.String(acceptLanguage) - } - - // one or the other but not both - if id != "" { - input.Id = aws.String(id) - } else if name != "" { - input.Name = aws.String(name) - } - - output, err := conn.DescribeProvisionedProduct(ctx, input) - - if errs.IsA[*awstypes.ResourceNotFoundException](err) { + if tfresource.NotFound(err) { return nil, "", nil } @@ -716,19 +703,15 @@ func statusProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, return nil, "", err } - if output == nil || output.ProvisionedProductDetail == nil { - return nil, "", nil - } - return output, string(output.ProvisionedProductDetail.Status), err } } -func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { +func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { stateConf := &retry.StateChangeConf{ Pending: enum.Slice(awstypes.ProvisionedProductStatusUnderChange, awstypes.ProvisionedProductStatusPlanInProgress), Target: enum.Slice(awstypes.ProvisionedProductStatusAvailable), - Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), + Refresh: statusProvisionedProduct(ctx, conn, id, acceptLanguage), Timeout: timeout, ContinuousTargetOccurence: continuousTargetOccurrence, NotFoundChecks: notFoundChecks, @@ -760,14 +743,14 @@ func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Clien return nil, err } -func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, acceptLanguage, id, name string, timeout time.Duration) error { +func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string, timeout time.Duration) error { stateConf := &retry.StateChangeConf{ Pending: enum.Slice( awstypes.ProvisionedProductStatusAvailable, awstypes.ProvisionedProductStatusUnderChange, ), Target: []string{}, - Refresh: statusProvisionedProduct(ctx, conn, acceptLanguage, id, name), + Refresh: statusProvisionedProduct(ctx, conn, id, acceptLanguage), Timeout: timeout, } From eee6cb8e8ef4497605bbaad447fad4d6fadf1de7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 28 Aug 2025 14:33:05 -0400 Subject: [PATCH 16/26] Acceptance test output: % make testacc TESTARGS='-run=TestAccServiceCatalogProvisionedProduct_basic' PKG=servicecatalog make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.24.6 test ./internal/service/servicecatalog/... -v -count 1 -parallel 20 -run=TestAccServiceCatalogProvisionedProduct_basic -timeout 360m -vet=off 2025/08/28 14:25:42 Creating Terraform AWS Provider (SDKv2-style)... 2025/08/28 14:25:42 Initializing Terraform AWS Provider (SDKv2-style)... === RUN TestAccServiceCatalogProvisionedProduct_basic === PAUSE TestAccServiceCatalogProvisionedProduct_basic === CONT TestAccServiceCatalogProvisionedProduct_basic --- PASS: TestAccServiceCatalogProvisionedProduct_basic (91.58s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog 97.019s From a999fe983be2c4b0d83b03a7d92dff26a8fd2a43 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 28 Aug 2025 14:33:58 -0400 Subject: [PATCH 17/26] Tidy up 'provisionedProductFailureError.Status'. --- .../service/servicecatalog/provisioned_product.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index f760b86241ef..3efe1405e2b3 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -722,12 +722,10 @@ func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Clien if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { if detail := output.ProvisionedProductDetail; detail != nil { - var foo *retry.UnexpectedStateError - if errors.As(err, &foo) { + if errs.IsA[*retry.UnexpectedStateError](err) { // The statuses `ERROR` and `TAINTED` are equivalent: the application of the requested change has failed. // The difference is that, in the case of `TAINTED`, there is a previous version to roll back to. - status := string(detail.Status) - if status == string(awstypes.ProvisionedProductStatusError) || status == string(awstypes.ProvisionedProductStatusTainted) { + if status := detail.Status; status == awstypes.ProvisionedProductStatusError || status == awstypes.ProvisionedProductStatusTainted { // Create a custom error type that signals state refresh is needed return output, &provisionedProductFailureError{ StatusMessage: aws.ToString(detail.StatusMessage), @@ -758,11 +756,9 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok { if detail := output.ProvisionedProductDetail; detail != nil { - var foo *retry.UnexpectedStateError - if errors.As(err, &foo) { + if errs.IsA[*retry.UnexpectedStateError](err) { // If the status is `TAINTED`, we can retry with `IgnoreErrors` - status := string(detail.Status) - if status == string(awstypes.ProvisionedProductStatusTainted) { + if status := detail.Status; status == awstypes.ProvisionedProductStatusTainted { // Create a custom error type that signals state refresh is needed return &provisionedProductFailureError{ StatusMessage: aws.ToString(detail.StatusMessage), @@ -782,7 +778,7 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. // that requires state refresh to recover from inconsistent state. type provisionedProductFailureError struct { StatusMessage string - Status string + Status awstypes.ProvisionedProductStatus NeedsRefresh bool } From 34342d148b9f2093c68d1c5b0d0ad4fe3a21cfc5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 28 Aug 2025 14:49:07 -0400 Subject: [PATCH 18/26] Fix golangci-lint 'unparam'. --- .../service/servicecatalog/provisioned_product.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 3efe1405e2b3..337bac260b18 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -578,7 +578,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat return sdkdiag.AppendErrorf(diags, "terminating Service Catalog Provisioned Product (%s): %s", d.Id(), err) } - err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) + _, err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { input.IgnoreErrors = true @@ -593,7 +593,7 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat return sdkdiag.AppendErrorf(diags, "terminating Service Catalog Provisioned Product (%s): %s", d.Id(), err) } - err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) + _, err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) } if errs.IsA[*awstypes.ResourceNotFoundException](err) { @@ -707,7 +707,7 @@ func statusProvisionedProduct(ctx context.Context, conn *servicecatalog.Client, } } -func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { +func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { //nolint:unparam stateConf := &retry.StateChangeConf{ Pending: enum.Slice(awstypes.ProvisionedProductStatusUnderChange, awstypes.ProvisionedProductStatusPlanInProgress), Target: enum.Slice(awstypes.ProvisionedProductStatusAvailable), @@ -741,7 +741,7 @@ func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Clien return nil, err } -func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string, timeout time.Duration) error { +func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog.Client, id, acceptLanguage string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) { //nolint:unparam stateConf := &retry.StateChangeConf{ Pending: enum.Slice( awstypes.ProvisionedProductStatusAvailable, @@ -760,7 +760,7 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. // If the status is `TAINTED`, we can retry with `IgnoreErrors` if status := detail.Status; status == awstypes.ProvisionedProductStatusTainted { // Create a custom error type that signals state refresh is needed - return &provisionedProductFailureError{ + return output, &provisionedProductFailureError{ StatusMessage: aws.ToString(detail.StatusMessage), Status: status, NeedsRefresh: true, @@ -768,10 +768,10 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. } } } - return err + return output, err } - return err + return nil, err } // provisionedProductFailureError represents a provisioned product operation failure From ef2c3ac9f42ea85ab5491e638af11275763cd254 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 28 Aug 2025 16:30:46 -0400 Subject: [PATCH 19/26] ProvisionedProduct: Force state refresh if Update results in TAINTED state. --- .../servicecatalog/provisioned_product.go | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 337bac260b18..97ffd1475107 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -421,6 +421,10 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, // } recordIdToUse := aws.ToString(detail.LastProvisioningRecordId) + if detail.Status == awstypes.ProvisionedProductStatusTainted && detail.LastSuccessfulProvisioningRecordId != nil { + recordIdToUse = aws.ToString(detail.LastSuccessfulProvisioningRecordId) + log.Printf("[DEBUG] Service Catalog Provisioned Product (%s) is TAINTED, using last successful record %s for parameter values", d.Id(), recordIdToUse) + } outputDR, err := findRecordByTwoPartKey(ctx, conn, recordIdToUse, acceptLanguage) if !d.IsNewResource() && tfresource.NotFound(err) { @@ -523,24 +527,23 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat } if _, err := waitProvisionedProductReady(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutUpdate)); err != nil { - // // Check if this is a state inconsistency error - // var failureErr *ProvisionedProductFailureError - // if errors.As(err, &failureErr) && failureErr.IsStateInconsistent() { - // // Force a state refresh to get actual AWS values before returning error - // log.Printf("[WARN] Service Catalog Provisioned Product (%s) update failed with status %s, refreshing state", d.Id(), failureErr.Status) - - // // Perform state refresh to get actual current values from AWS - // refreshDiags := resourceProvisionedProductRead(ctx, d, meta) - // if refreshDiags.HasError() { - // // If refresh fails, return both errors - // return append(refreshDiags, sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err)...) - // } - - // // Return the original failure error after state is corrected - // return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) - // } - - // // For other errors, proceed as before + // Check if this is a state inconsistency error + if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { + // Force a state refresh to get actual AWS values before returning error + log.Printf("[WARN] Service Catalog Provisioned Product (%s) update failed with status %s, refreshing state", d.Id(), failureErr.Status) + + // Perform state refresh to get actual current values from AWS + refreshDiags := resourceProvisionedProductRead(ctx, d, meta) + if refreshDiags.HasError() { + // If refresh fails, return both errors + return append(refreshDiags, sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err)...) + } + + // Return the original failure error after state is corrected + return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) + } + + // For other errors, proceed as before return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) } From 6d218dc89f24129079a356b0c304a638ae93b7a8 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 13:58:26 -0400 Subject: [PATCH 20/26] r/aws_servicecatalog_provisioned_product(test): add expected error condition --- internal/service/servicecatalog/provisioned_product_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index e50c37778ef9..6dd0f145903d 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1061,11 +1061,17 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), }, }, + ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), }, // Step 5: Clean up by applying a working config { Config: testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, Check: resource.ComposeTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), From 4140e7f1cbc70a06591e8937de077347602d9099 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 14:55:24 -0400 Subject: [PATCH 21/26] r/aws_servicecatalog_provisioned_product: rollback params, artifact ID on failed update ```console % make testacc PKG=servicecatalog TESTS=TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.24.6 test ./internal/service/servicecatalog/... -v -count 1 -parallel 20 -run='TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate' -timeout 360m -vet=off 2025/09/02 14:44:06 Creating Terraform AWS Provider (SDKv2-style)... 2025/09/02 14:44:06 Initializing Terraform AWS Provider (SDKv2-style)... --- PASS: TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate (294.43s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog 301.051s ``` --- .../servicecatalog/provisioned_product.go | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 97ffd1475107..2ab03d2a0f49 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -410,18 +410,10 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, // or after an invalid update that returns a 'FAILED' record state. Thus, waiters are now present in the CREATE and UPDATE methods of this resource instead. // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/24574#issuecomment-1126339193 - // // For TAINTED resources, we need to get parameters from the last successful record - // // not the last provisioning record which may have failed - // var recordIdToUse *string - // if detail.Status == awstypes.ProvisionedProductStatusTainted && detail.LastSuccessfulProvisioningRecordId != nil { - // recordIdToUse = detail.LastSuccessfulProvisioningRecordId - // log.Printf("[DEBUG] Service Catalog Provisioned Product (%s) is TAINTED, using last successful record %s for parameter values", d.Id(), aws.ToString(recordIdToUse)) - // } else { - // recordIdToUse = detail.LastProvisioningRecordId - // } - recordIdToUse := aws.ToString(detail.LastProvisioningRecordId) if detail.Status == awstypes.ProvisionedProductStatusTainted && detail.LastSuccessfulProvisioningRecordId != nil { + // For TAINTED resources, we need to get artifact details from the last successful + // record, as the last provisioned record may have failed. recordIdToUse = aws.ToString(detail.LastSuccessfulProvisioningRecordId) log.Printf("[DEBUG] Service Catalog Provisioned Product (%s) is TAINTED, using last successful record %s for parameter values", d.Id(), recordIdToUse) } @@ -436,10 +428,9 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, return sdkdiag.AppendErrorf(diags, "reading Service Catalog Provisioned Product (%s) Record (%s): %s", d.Id(), recordIdToUse, err) } - // To enable debugging of potential v, log as a warning - // instead of exiting prematurely with an error, e.g. - // v can be present after update to a new version failed and the stack - // rolled back to the current version. + // To enable debugging of potential issues, log as a warning instead of exiting prematurely. + // For example, errors can be present after a failed version update, and the stack rolled back + // to the current version. if v := outputDR.RecordDetail.RecordErrors; len(v) > 0 { var errs []error @@ -455,6 +446,7 @@ func resourceProvisionedProductRead(ctx context.Context, d *schema.ResourceData, } d.Set("path_id", outputDR.RecordDetail.PathId) + d.Set("provisioning_artifact_id", outputDR.RecordDetail.ProvisioningArtifactId) setTagsOut(ctx, svcTags(recordKeyValueTags(ctx, outputDR.RecordDetail.RecordTags))) @@ -539,6 +531,16 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat return append(refreshDiags, sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err)...) } + if d.HasChange("provisioning_parameters") { + // If parameters were changed, rollback to previous values. + // + // The read APIs used to refresh state above do not return parameter values, and therefore + // will not reflect that the planned updates did not take effect. Explicitly rolling back + // ensures the planned parameter changes are attempted again on a subsequent apply. + oldParams, _ := d.GetChange("provisioning_parameters") + d.Set("provisioning_parameters", oldParams) + } + // Return the original failure error after state is corrected return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) } From d861fc17be38c9bf6c12202a1c90ee622d94e33b Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 15:57:23 -0400 Subject: [PATCH 22/26] r/aws_servicecatalog_provisioned_product(test): refine plan, state checks ```console % make testacc PKG=servicecatalog TESTS=TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.24.6 test ./internal/service/servicecatalog/... -v -count 1 -parallel 20 -run='TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate' -timeout 360m -vet=off 2025/09/02 15:41:43 Creating Terraform AWS Provider (SDKv2-style)... 2025/09/02 15:41:43 Initializing Terraform AWS Provider (SDKv2-style)... === RUN TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate === PAUSE TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate === CONT TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate --- PASS: TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate (296.94s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog 303.535s ``` --- .../provisioned_product_test.go | 268 +++++++++--------- 1 file changed, 127 insertions(+), 141 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 6dd0f145903d..00362a925c77 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -11,12 +11,14 @@ import ( "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/servicecatalog" awstypes "github.com/aws/aws-sdk-go-v2/service/servicecatalog/types" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/statecheck" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfservicecatalog "github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog" @@ -458,6 +460,127 @@ func TestAccServiceCatalogProvisionedProduct_productTagUpdateAfterError(t *testi }) } +// Validates that a provisioned product in tainted status properly triggers an update +// on subsequent applies. +// Ref: https://github.com/hashicorp/terraform-provider-aws/issues/42585 +func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_servicecatalog_provisioned_product.test" + artifactsDataSourceName := "data.aws_servicecatalog_provisioning_artifacts.product_artifacts" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + initialArtifactID := "provisioning_artifact_details.0.id" + newArtifactID := "provisioning_artifact_details.1.id" + var v awstypes.ProvisionedProductDetail + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ServiceCatalogServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckProvisionedProductDestroy(ctx), + Steps: []resource.TestStep{ + // Step 1 - Setup + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName, &v), + resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, initialArtifactID), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrStatus), knownvalue.StringExact("AVAILABLE")), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("provisioning_parameters"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("FailureSimulation"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact(acctest.CtFalse), + }), + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("ExtraParam"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact("original"), + }), + })), + }, + }, + // Step 2 - Trigger a failure, leaving the provisioned product tainted + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), + ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + plancheck.ExpectKnownValue(resourceName, tfjsonpath.New("provisioning_parameters"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("FailureSimulation"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact(acctest.CtTrue), + }), + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("ExtraParam"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact("updated"), + }), + })), + }, + }, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrStatus), knownvalue.StringExact("TAINTED")), + // Verify state is rolled back to the paramters from the original setup run + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("provisioning_parameters"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("FailureSimulation"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact(acctest.CtFalse), + }), + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("ExtraParam"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact("original"), + }), + })), + }, + }, + // Step 3 - Verify an update is planned, even without configuration changes + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), + }, + // Step 4 - Resolve the failure, verifying an update is completed + { + Config: testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName, &v), + resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), + ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrStatus), knownvalue.StringExact("AVAILABLE")), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("provisioning_parameters"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("FailureSimulation"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact(acctest.CtFalse), + }), + knownvalue.ObjectExact(map[string]knownvalue.Check{ + names.AttrKey: knownvalue.StringExact("ExtraParam"), + "use_previous_value": knownvalue.Bool(false), + names.AttrValue: knownvalue.StringExact("updated"), + }), + })), + }, + }, + }, + }) +} + func testAccCheckProvisionedProductDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).ServiceCatalogClient(ctx) @@ -986,155 +1109,18 @@ resource "aws_s3_bucket" "conflict" { `, rName, conflictingBucketName, tagValue)) } -// TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate reproduces the exact bug scenario: -// -// When a Service Catalog provisioned product update fails, the resource becomes TAINTED. -// The bug is that subsequent `terraform apply` shows "no changes" instead of retrying -// the failed update automatically. -// -// Expected behavior: TAINTED resources should always trigger an update attempt. -// Current (buggy) behavior: TAINTED resources show "no changes" in plan. -// -// This test should FAIL at Step 4 with the current implementation, proving the bug exists. -// Step 4 uses ConfigPlanChecks to verify that an update action should be planned. -func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { - ctx := acctest.Context(t) - resourceName := "aws_servicecatalog_provisioned_product.test" - const artifactsDataSourceName = "data.aws_servicecatalog_provisioning_artifacts.product_artifacts" - const initialArtifactID = "provisioning_artifact_details.0.id" - const newArtifactID = "provisioning_artifact_details.1.id" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - var pprod awstypes.ProvisionedProductDetail - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, names.ServiceCatalogServiceID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckProvisionedProductDestroy(ctx), - Steps: []resource.TestStep{ - // Step 1: Create with working configuration - { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), - testAccCheckProvisionedProductStatus(ctx, resourceName, "AVAILABLE"), - resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), - resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, initialArtifactID), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", acctest.CtFalse), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "none"), - ), - }, - - // Step 2: Update to failing configuration - { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), - ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), - }, - - // // Step 3: Verify resource is now TAINTED after the failed update - // // Use RefreshState to avoid triggering any plan changes due to config differences - // { - // RefreshState: true, - // Check: resource.ComposeAggregateTestCheckFunc( - // // testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), // Can't use this because it fails on TAINTED - // testAccCheckProvisionedProductStatus(ctx, resourceName, "TAINTED"), - // resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "TAINTED"), - // resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), - // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), - // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), - // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", "true"), - // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), - // resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_once"), - // ), - // }, - - // Step 4: CRITICAL TEST - Apply the same failing config again - // BUG: Currently this shows "no changes" but should retry the update - // ConfigPlanChecks should FAIL with current implementation, demonstrating the bug - { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), - ConfigPlanChecks: resource.ConfigPlanChecks{ - PreApply: []plancheck.PlanCheck{ - plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), - }, - }, - ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), - }, - - // Step 5: Clean up by applying a working config - { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName), - ConfigPlanChecks: resource.ConfigPlanChecks{ - PreApply: []plancheck.PlanCheck{ - plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), - }, - }, - Check: resource.ComposeTestCheckFunc( - testAccCheckProvisionedProductExists(ctx, resourceName, &pprod), - resource.TestCheckResourceAttr(resourceName, names.AttrStatus, "AVAILABLE"), - resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.#", "2"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.key", "FailureSimulation"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.0.value", acctest.CtFalse), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.key", "ExtraParam"), - resource.TestCheckResourceAttr(resourceName, "provisioning_parameters.1.value", "changed_to_force_an_update"), - ), - }, - }, - }) -} - -// Helper function to check provisioned product status -func testAccCheckProvisionedProductStatus(ctx context.Context, resourceName, expectedStatus string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] - if !ok { - return fmt.Errorf("resource not found: %s", resourceName) - } - - conn := acctest.Provider.Meta().(*conns.AWSClient).ServiceCatalogClient(ctx) - - input := &servicecatalog.DescribeProvisionedProductInput{ - Id: aws.String(rs.Primary.ID), - AcceptLanguage: aws.String(tfservicecatalog.AcceptLanguageEnglish), - } - - output, err := conn.DescribeProvisionedProduct(ctx, input) - if err != nil { - return fmt.Errorf("describing Service Catalog Provisioned Product (%s): %w", rs.Primary.ID, err) - } - - if output.ProvisionedProductDetail == nil { - return fmt.Errorf("Service Catalog Provisioned Product (%s) not found", rs.Primary.ID) - } - - actualStatus := string(output.ProvisionedProductDetail.Status) - if actualStatus != expectedStatus { - return fmt.Errorf("Service Catalog Provisioned Product (%s) status: expected %s, got %s", - rs.Primary.ID, expectedStatus, actualStatus) - } - - return nil - } -} - func testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName string) string { - return testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "none") + return testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "original") } func testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName string) string { - return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "changed_once") + return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "updated") } func testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName string) string { - return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "changed_to_force_an_update") + return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "updated") } -// testAccProvisionedProductConfig_retryTaintedUpdate creates a simple working CloudFormation template -// This avoids the complex conditional logic that was causing issues in the basic config func testAccProvisionedProductConfig_retryTaintedUpdate(rName string, useNewVersion bool, simulateFailure bool, extraParam string) string { return acctest.ConfigCompose( testAccProvisionedProductPortfolioBaseConfig(rName), From 5b1f474df54d9d6d7af042f076b2845bfb2ed281 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 16:10:06 -0400 Subject: [PATCH 23/26] r/aws_servicecatalog_provisioned_product(test): simplify test config --- .../provisioned_product_test.go | 76 +++---------------- 1 file changed, 10 insertions(+), 66 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 00362a925c77..b782961c3a51 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -480,7 +480,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { Steps: []resource.TestStep{ // Step 1 - Setup { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "original"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &v), resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, initialArtifactID), @@ -503,7 +503,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { }, // Step 2 - Trigger a failure, leaving the provisioned product tainted { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "updated"), ExpectError: regexache.MustCompile(`The following resource\(s\) failed to update:`), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ @@ -541,7 +541,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { }, // Step 3 - Verify an update is planned, even without configuration changes { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "updated"), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), @@ -551,7 +551,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { }, // Step 4 - Resolve the failure, verifying an update is completed { - Config: testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName), + Config: testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "updated"), Check: resource.ComposeTestCheckFunc( testAccCheckProvisionedProductExists(ctx, resourceName, &v), resource.TestCheckResourceAttrPair(resourceName, "provisioning_artifact_id", artifactsDataSourceName, newArtifactID), @@ -1109,22 +1109,15 @@ resource "aws_s3_bucket" "conflict" { `, rName, conflictingBucketName, tagValue)) } -func testAccProvisionedProductConfig_retryTaintedUpdate_Setup(rName string) string { - return testAccProvisionedProductConfig_retryTaintedUpdate(rName, false, false, "original") -} - -func testAccProvisionedProductConfig_retryTaintedUpdate_WithFailure(rName string) string { - return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, true, "updated") -} - -func testAccProvisionedProductConfig_retryTaintedUpdate_ResolveFailure(rName string) string { - return testAccProvisionedProductConfig_retryTaintedUpdate(rName, true, false, "updated") -} - func testAccProvisionedProductConfig_retryTaintedUpdate(rName string, useNewVersion bool, simulateFailure bool, extraParam string) string { return acctest.ConfigCompose( testAccProvisionedProductPortfolioBaseConfig(rName), fmt.Sprintf(` +locals { + initial_provisioning_artifact = data.aws_servicecatalog_provisioning_artifacts.product_artifacts.provisioning_artifact_details[0] + new_provisioning_artifact = data.aws_servicecatalog_provisioning_artifacts.product_artifacts.provisioning_artifact_details[1] +} + resource "aws_servicecatalog_provisioned_product" "test" { name = %[1]q product_id = aws_servicecatalog_product.test.id @@ -1139,16 +1132,12 @@ resource "aws_servicecatalog_provisioned_product" "test" { key = "ExtraParam" value = %[4]q } - - depends_on = [ - aws_servicecatalog_constraint.launch_constraint, - ] } resource "aws_servicecatalog_product" "test" { description = %[1]q name = %[1]q - owner = "ägare" + owner = "test" type = "CLOUD_FORMATION_TEMPLATE" provisioning_artifact_parameters { @@ -1180,11 +1169,6 @@ data "aws_servicecatalog_provisioning_artifacts" "product_artifacts" { depends_on = [aws_servicecatalog_provisioning_artifact.new_version] } -locals { - initial_provisioning_artifact = data.aws_servicecatalog_provisioning_artifacts.product_artifacts.provisioning_artifact_details[0] - new_provisioning_artifact = data.aws_servicecatalog_provisioning_artifacts.product_artifacts.provisioning_artifact_details[1] -} - resource "aws_s3_bucket" "test" { bucket = %[1]q force_destroy = true @@ -1196,45 +1180,5 @@ resource "aws_s3_object" "test" { source = "${path.module}/testdata/foo/product_template.yaml" } - -# Required to validate provisioned product on update -resource "aws_servicecatalog_constraint" "launch_constraint" { - description = "Launch constraint for test product" - portfolio_id = aws_servicecatalog_portfolio.test.id - product_id = aws_servicecatalog_product.test.id - type = "LAUNCH" - - parameters = jsonencode({ - "RoleArn" = aws_iam_role.launch_role.arn - }) - - depends_on = [aws_iam_role_policy_attachment.launch_role] -} - -# IAM role for Service Catalog launch constraint -resource "aws_iam_role" "launch_role" { - name = "%[1]s-launch-role" - - assume_role_policy = jsonencode({ - Version = "2012-10-17" - Statement = [ - { - Action = "sts:AssumeRole" - Effect = "Allow" - Principal = { - Service = "servicecatalog.amazonaws.com" - } - } - ] - }) -} - -data "aws_partition" "current" {} - -# Attach admin policy to launch role (for demo purposes only) -resource "aws_iam_role_policy_attachment" "launch_role" { - role = aws_iam_role.launch_role.name - policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/AdministratorAccess" -} `, rName, useNewVersion, simulateFailure, extraParam)) } From bfde42d2d7ed83dc16bd93af37e7d14be24e9037 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 16:24:10 -0400 Subject: [PATCH 24/26] r/aws_servicecatalog_provisioned_product(test): tidy test template ```console % make testacc PKG=servicecatalog TESTS=TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.24.6 test ./internal/service/servicecatalog/... -v -count 1 -parallel 20 -run='TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate' -timeout 360m -vet=off 2025/09/02 16:15:09 Creating Terraform AWS Provider (SDKv2-style)... 2025/09/02 16:15:09 Initializing Terraform AWS Provider (SDKv2-style)... --- PASS: TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate (299.97s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog 306.426s ``` --- .../service/servicecatalog/provisioned_product_test.go | 8 +------- .../{foo => retry-tainted-update}/product_template.yaml | 1 - 2 files changed, 1 insertion(+), 8 deletions(-) rename internal/service/servicecatalog/testdata/{foo => retry-tainted-update}/product_template.yaml (99%) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index b782961c3a51..160a9e4e078c 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -1155,12 +1155,6 @@ resource "aws_servicecatalog_provisioning_artifact" "new_version" { description = "New" template_url = "https://${aws_s3_bucket.test.bucket_regional_domain_name}/${aws_s3_object.test.key}" type = "CLOUD_FORMATION_TEMPLATE" - - # Force a new version to be created when MPI version changes - # Is this needed? - lifecycle { - create_before_destroy = true - } } data "aws_servicecatalog_provisioning_artifacts" "product_artifacts" { @@ -1178,7 +1172,7 @@ resource "aws_s3_object" "test" { bucket = aws_s3_bucket.test.id key = "product_template.yaml" - source = "${path.module}/testdata/foo/product_template.yaml" + source = "${path.module}/testdata/retry-tainted-update/product_template.yaml" } `, rName, useNewVersion, simulateFailure, extraParam)) } diff --git a/internal/service/servicecatalog/testdata/foo/product_template.yaml b/internal/service/servicecatalog/testdata/retry-tainted-update/product_template.yaml similarity index 99% rename from internal/service/servicecatalog/testdata/foo/product_template.yaml rename to internal/service/servicecatalog/testdata/retry-tainted-update/product_template.yaml index e63f55771e3e..af531995bdfa 100644 --- a/internal/service/servicecatalog/testdata/foo/product_template.yaml +++ b/internal/service/servicecatalog/testdata/retry-tainted-update/product_template.yaml @@ -141,7 +141,6 @@ Resources: }; Environment: Variables: - # MPIVersion: !Ref MPIVersion FailureSimulation: !Ref FailureSimulation ExtraParam: !Ref ExtraParam From 52778c5ec0a3b9a2a21ca7c37c1fed1a9bde2adc Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 16:25:57 -0400 Subject: [PATCH 25/26] chore: changelog --- .changelog/43956.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/43956.txt diff --git a/.changelog/43956.txt b/.changelog/43956.txt new file mode 100644 index 000000000000..d3254c1a25ae --- /dev/null +++ b/.changelog/43956.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_servicecatalog_provisioned_product: Set `provisioning_parameters` and `provisioning_artifact_id` to the values from the last successful deployment when update fails +``` From ed015b65fa97cc5a30e862b57101a3d9bbfc2228 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 2 Sep 2025 16:47:40 -0400 Subject: [PATCH 26/26] r/aws_servicecatalog_provisioned_product: simplify custom error type ```console % make testacc PKG=servicecatalog TESTS=TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.24.6 test ./internal/service/servicecatalog/... -v -count 1 -parallel 20 -run='TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate' -timeout 360m -vet=off 2025/09/02 16:40:44 Creating Terraform AWS Provider (SDKv2-style)... 2025/09/02 16:40:44 Initializing Terraform AWS Provider (SDKv2-style)... --- PASS: TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate (305.21s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog 311.774s ``` --- .../servicecatalog/provisioned_product.go | 23 ++----------------- .../provisioned_product_test.go | 2 +- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 2ab03d2a0f49..8551698fd9f8 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -519,12 +519,8 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat } if _, err := waitProvisionedProductReady(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutUpdate)); err != nil { - // Check if this is a state inconsistency error - if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { - // Force a state refresh to get actual AWS values before returning error + if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok { log.Printf("[WARN] Service Catalog Provisioned Product (%s) update failed with status %s, refreshing state", d.Id(), failureErr.Status) - - // Perform state refresh to get actual current values from AWS refreshDiags := resourceProvisionedProductRead(ctx, d, meta) if refreshDiags.HasError() { // If refresh fails, return both errors @@ -540,12 +536,8 @@ func resourceProvisionedProductUpdate(ctx context.Context, d *schema.ResourceDat oldParams, _ := d.GetChange("provisioning_parameters") d.Set("provisioning_parameters", oldParams) } - - // Return the original failure error after state is corrected - return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) } - // For other errors, proceed as before return sdkdiag.AppendErrorf(diags, "waiting for Service Catalog Provisioned Product (%s) update: %s", d.Id(), err) } @@ -585,15 +577,13 @@ func resourceProvisionedProductDelete(ctx context.Context, d *schema.ResourceDat _, err = waitProvisionedProductTerminated(ctx, conn, d.Id(), d.Get("accept_language").(string), d.Timeout(schema.TimeoutDelete)) - if failureErr, ok := errs.As[*provisionedProductFailureError](err); ok && failureErr.IsStateInconsistent() { + if errs.IsA[*provisionedProductFailureError](err) { input.IgnoreErrors = true _, err = conn.TerminateProvisionedProduct(ctx, &input) - if errs.IsA[*awstypes.ResourceNotFoundException](err) { return diags } - if err != nil { return sdkdiag.AppendErrorf(diags, "terminating Service Catalog Provisioned Product (%s): %s", d.Id(), err) } @@ -735,7 +725,6 @@ func waitProvisionedProductReady(ctx context.Context, conn *servicecatalog.Clien return output, &provisionedProductFailureError{ StatusMessage: aws.ToString(detail.StatusMessage), Status: status, - NeedsRefresh: true, } } } @@ -768,7 +757,6 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. return output, &provisionedProductFailureError{ StatusMessage: aws.ToString(detail.StatusMessage), Status: status, - NeedsRefresh: true, } } } @@ -784,19 +772,12 @@ func waitProvisionedProductTerminated(ctx context.Context, conn *servicecatalog. type provisionedProductFailureError struct { StatusMessage string Status awstypes.ProvisionedProductStatus - NeedsRefresh bool } func (e *provisionedProductFailureError) Error() string { return e.StatusMessage } -// IsStateInconsistent returns true if this error indicates state inconsistency -// that requires a refresh to recover. -func (e *provisionedProductFailureError) IsStateInconsistent() bool { - return e.NeedsRefresh -} - func expandProvisioningParameter(tfMap map[string]any) awstypes.ProvisioningParameter { apiObject := awstypes.ProvisioningParameter{} diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 160a9e4e078c..2984edf4b08d 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -524,7 +524,7 @@ func TestAccServiceCatalogProvisionedProduct_retryTaintedUpdate(t *testing.T) { }, ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrStatus), knownvalue.StringExact("TAINTED")), - // Verify state is rolled back to the paramters from the original setup run + // Verify state is rolled back to the parameters from the original setup run statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("provisioning_parameters"), knownvalue.ListExact([]knownvalue.Check{ knownvalue.ObjectExact(map[string]knownvalue.Check{ names.AttrKey: knownvalue.StringExact("FailureSimulation"),