Merge pull request #25130 from hashicorp/b-servicecatalog-provisioned…

…-product-record-return-error-on-tainted

r/servicecatalog_provisioned_product: fix error when in the `TAINTED` state

.changelog/25130.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_servicecatalog_provisioned_product: Correctly handle resources in a `TAINTED` state
+```

internal/service/servicecatalog/provisioned_product_test.go

@@ -195,6 +195,38 @@ func TestAccServiceCatalogProvisionedProduct_tags(t *testing.T) {
 	})
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/24574
+func TestAccServiceCatalogProvisionedProduct_tainted(t *testing.T) {
+	resourceName := "aws_servicecatalog_provisioned_product.test"
+
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	domain := fmt.Sprintf("http://%s", acctest.RandomDomainName())
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:          func() { acctest.PreCheck(t) },
+		ErrorCheck:        acctest.ErrorCheck(t, servicecatalog.EndpointsID),
+		ProviderFactories: acctest.ProviderFactories,
+		CheckDestroy:      testAccCheckProvisionedProductDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccProvisionedProductConfig_basic(rName, domain, acctest.DefaultEmailAddress, "10.1.0.0/16"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckProvisionedProductExists(resourceName),
+				),
+			},
+			{
+				Config:      testAccProvisionedProductConfig_updateTainted(rName, domain, acctest.DefaultEmailAddress, "10.1.0.0/16"),
+				ExpectError: regexp.MustCompile(`unexpected state 'TAINTED', wanted target 'AVAILABLE'`),
+			},
+			{
+				// Check we can still run a complete plan after the previous update error
+				Config:   testAccProvisionedProductConfig_updateTainted(rName, domain, acctest.DefaultEmailAddress, "10.1.0.0/16"),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 func testAccCheckProvisionedProductDestroy(s *terraform.State) error {
 	conn := acctest.Provider.Meta().(*conns.AWSClient).ServiceCatalogConn
 
@@ -385,6 +417,28 @@ resource "aws_servicecatalog_provisioned_product" "test" {
 `, rName, vpcCidr))
 }
 
+func testAccProvisionedProductConfig_updateTainted(rName, domain, email, vpcCidr string) string {
+	return acctest.ConfigCompose(testAccProvisionedProductTemplateURLBaseConfig(rName, domain, email),
+		fmt.Sprintf(`
+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
+
+  provisioning_parameters {
+    key   = "VPCPrimaryCIDR"
+    value = %[2]q
+  }
+
+  provisioning_parameters {
+    key   = "LeaveMeEmpty"
+    value = "NotEmpty"
+  }
+}
+`, rName, vpcCidr))
+}
+
 func testAccProvisionedProductConfig_tags(rName, tagKey, tagValue, domain, email string) string {
 	return acctest.ConfigCompose(testAccProvisionedProductTemplateURLBaseConfig(rName, domain, email),
 		fmt.Sprintf(`

internal/service/servicecatalog/wait.go

@@ -502,10 +502,8 @@ func WaitLaunchPathsReady(conn *servicecatalog.ServiceCatalog, acceptLanguage, p
 
 func WaitProvisionedProductReady(conn *servicecatalog.ServiceCatalog, acceptLanguage, id, name string, timeout time.Duration) (*servicecatalog.DescribeProvisionedProductOutput, error) {
 	stateConf := &resource.StateChangeConf{
-		// "TAINTED" is a valid target state as its described as a stable state per API docs, though can result from a failed update
-		// such that the stack rolls back to a previous version.
 		Pending:                   []string{StatusNotFound, StatusUnavailable, servicecatalog.ProvisionedProductStatusUnderChange, servicecatalog.ProvisionedProductStatusPlanInProgress},
-		Target:                    []string{servicecatalog.StatusAvailable, servicecatalog.ProvisionedProductStatusTainted},
+		Target:                    []string{servicecatalog.StatusAvailable},
 		Refresh:                   StatusProvisionedProduct(conn, acceptLanguage, id, name),
 		Timeout:                   timeout,
 		ContinuousTargetOccurence: ContinuousTargetOccurrence,
@@ -516,7 +514,14 @@ func WaitProvisionedProductReady(conn *servicecatalog.ServiceCatalog, acceptLang
 	outputRaw, err := stateConf.WaitForState()
 
 	if output, ok := outputRaw.(*servicecatalog.DescribeProvisionedProductOutput); ok {
-		tfresource.SetLastError(err, errors.New(aws.StringValue(output.ProvisionedProductDetail.StatusMessage)))
+		if detail := output.ProvisionedProductDetail; detail != nil {
+			status := aws.StringValue(detail.Status)
+			// Note: "TAINTED" is described as a stable state per API docs, though can result from a failed update
+			// such that the stack rolls back to a previous version
+			if status == servicecatalog.ProvisionedProductStatusError || status == servicecatalog.ProvisionedProductStatusTainted {
+				tfresource.SetLastError(err, errors.New(aws.StringValue(detail.StatusMessage)))
+			}
+		}
 		return output, err
 	}
 

website/docs/r/servicecatalog_provisioned_product.html.markdown

@@ -105,6 +105,8 @@ In addition to all arguments above, the following attributes are exported:
 
 ### status Meanings
 
+~> **NOTE:** [Enable logging](https://www.terraform.io/plugin/log/managing) to `WARN` verbosity to further investigate error messages associated with a provisioned product in the `ERROR` or `TAINTED` state which can occur during resource creation or update.
+
 * `AVAILABLE` - Stable state, ready to perform any operation. The most recent operation succeeded and completed.
 * `UNDER_CHANGE` - Transitive state. Operations performed might not have
 valid results. Wait for an `AVAILABLE` status before performing operations.