From 5a87cd33a662e4876e25515b013c7ee100156a85 Mon Sep 17 00:00:00 2001 From: Angie Pinilla <angie@hashicorp.com> Date: Wed, 1 Jun 2022 11:37:55 -0400 Subject: [PATCH 1/3] r/servicecatalog_provisioned_product: return error when in the 'TAINTED' state; add error message in waiter --- internal/service/servicecatalog/wait.go | 13 +++++++++---- ...servicecatalog_provisioned_product.html.markdown | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/service/servicecatalog/wait.go b/internal/service/servicecatalog/wait.go index 112c1272452..90b0a9cbb27 100644 --- a/internal/service/servicecatalog/wait.go +++ b/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 } diff --git a/website/docs/r/servicecatalog_provisioned_product.html.markdown b/website/docs/r/servicecatalog_provisioned_product.html.markdown index 98eba8f15b3..e7e5ede18ce 100644 --- a/website/docs/r/servicecatalog_provisioned_product.html.markdown +++ b/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. From 194e66771605bd9272fca985174f728efb192cbd Mon Sep 17 00:00:00 2001 From: Angie Pinilla <angie@hashicorp.com> Date: Thu, 2 Jun 2022 14:09:05 -0400 Subject: [PATCH 2/3] Update CHANGELOG for #25130 --- .changelog/25130.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/25130.txt diff --git a/.changelog/25130.txt b/.changelog/25130.txt new file mode 100644 index 00000000000..5ca2489f924 --- /dev/null +++ b/.changelog/25130.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_servicecatalog_provisioned_product: Correctly handle resources in a `TAINTED` state +``` \ No newline at end of file From bd1774fd5e17efcb7a6862db98b9fbcee5127203 Mon Sep 17 00:00:00 2001 From: Angie Pinilla <angie@hashicorp.com> Date: Thu, 2 Jun 2022 14:35:38 -0400 Subject: [PATCH 3/3] add acceptance test for checking error returned from an incorrect update --- .../provisioned_product_test.go | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index 8fb422a21fb..0c917e7859d 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/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(`