From 509acae519b0656f3232c09ea51c7cb81cec1150 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 21 Feb 2023 16:11:08 -0600 Subject: [PATCH 1/5] r/aws_servicecatalog_provisioned_product: add customdiff to recalculate outputs --- .../servicecatalog/provisioned_product.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/service/servicecatalog/provisioned_product.go b/internal/service/servicecatalog/provisioned_product.go index 1593cb420e5..d0b868a0d27 100644 --- a/internal/service/servicecatalog/provisioned_product.go +++ b/internal/service/servicecatalog/provisioned_product.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -252,10 +253,23 @@ func ResourceProvisionedProduct() *schema.Resource { }, }, - CustomizeDiff: verify.SetTagsDiff, + CustomizeDiff: customdiff.All( + refreshOutputsDiff, + verify.SetTagsDiff, + ), } } +func refreshOutputsDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error { + if diff.HasChanges("provisioning_parameters", "provisioning_artifact_id", "provisioning_artifact_name") { + if err := diff.SetNewComputed("outputs"); err != nil { + return err + } + } + + return nil +} + func resourceProvisionedProductCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).ServiceCatalogConn() From 536ed148e7fab013f52774acd54f2b5e52a417fe Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 21 Feb 2023 20:15:39 -0600 Subject: [PATCH 2/5] r/aws_servicecatalog_provisioned_product: add test for output changes --- .../provisioned_product_test.go | 195 ++++++++++++++++++ 1 file changed, 195 insertions(+) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index cdd82a5f540..b90e02dd9d9 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -142,6 +142,55 @@ func TestAccServiceCatalogProvisionedProduct_update(t *testing.T) { }) } +func TestAccServiceCatalogProvisionedProduct_computedOutputs(t *testing.T) { + ctx := acctest.Context(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), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckProvisionedProductDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccProvisionedProductConfig_computedOutputs(rName, domain, acctest.DefaultEmailAddress, "10.1.0.0/16"), + Check: resource.ComposeTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "outputs.#", "3"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "outputs.*", map[string]string{ + "description": "VPC ID", + "key": "VpcID", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "outputs.*", map[string]string{ + "description": "VPC CIDR", + "key": "VPCPrimaryCIDR", + "value": "10.1.0.0/16", + }), + ), + }, + { + Config: testAccProvisionedProductConfig_computedOutputs(rName, domain, acctest.DefaultEmailAddress, "10.1.0.1/16"), + Check: resource.ComposeTestCheckFunc( + testAccCheckProvisionedProductExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "outputs.#", "3"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "outputs.*", map[string]string{ + "description": "VPC ID", + "key": "VpcID", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "outputs.*", map[string]string{ + "description": "VPC CIDR", + "key": "VPCPrimaryCIDR", + "value": "10.1.0.1/16", + }), + ), + }, + }, + }) +} + func TestAccServiceCatalogProvisionedProduct_disappears(t *testing.T) { ctx := acctest.Context(t) resourceName := "aws_servicecatalog_provisioned_product.test" @@ -403,6 +452,130 @@ data "aws_servicecatalog_launch_paths" "test" { `, rName, domain, email) } +func testAccProvisionedProductPhysicalTemplateIDBaseConfig(rName, domain, email string) string { + return fmt.Sprintf(` +resource "aws_cloudformation_stack" "test" { + name = %[1]q + + template_body = jsonencode({ + AWSTemplateFormatVersion = "2010-09-09" + + Parameters = { + VPCPrimaryCIDR = { + Type = "String" + Default = "10.0.0.0/16" + } + LeaveMeEmpty = { + Type = "String" + Description = "Make sure that empty values come through. Issue #21349" + Default = "" + } + } + + "Conditions" = { + "IsEmptyParameter" = { + "Fn::Equals" = [ + { + "Ref" = "LeaveMeEmpty" + }, + "", + ] + } + } + + Resources = { + MyVPC = { + Type = "AWS::EC2::VPC" + Condition = "IsEmptyParameter" + Properties = { + CidrBlock = { Ref = "VPCPrimaryCIDR" } + } + } + } + + Outputs = { + VpcID = { + Description = "VPC ID" + Value = { + Ref = "MyVPC" + } + } + + VPCPrimaryCIDR = { + Description = "VPC CIDR" + Value = { + Ref = "VPCPrimaryCIDR" + } + } + } + }) +} + +resource "aws_servicecatalog_product" "test" { + description = %[1]q + distributor = "distributör" + name = %[1]q + owner = "ägare" + type = "CLOUD_FORMATION_TEMPLATE" + support_description = %[1]q + support_email = %[3]q + support_url = %[2]q + + provisioning_artifact_parameters { + description = "artefaktbeskrivning" + disable_template_validation = true + name = %[1]q + template_physical_id = aws_cloudformation_stack.test.id + type = "CLOUD_FORMATION_TEMPLATE" + } + + tags = { + Name = %[1]q + } +} + +resource "aws_servicecatalog_portfolio" "test" { + name = %[1]q + description = %[1]q + provider_name = %[1]q +} + +resource "aws_servicecatalog_constraint" "test" { + description = %[1]q + portfolio_id = aws_servicecatalog_product_portfolio_association.test.portfolio_id + product_id = aws_servicecatalog_product_portfolio_association.test.product_id + type = "RESOURCE_UPDATE" + + parameters = jsonencode({ + Version = "2.0" + Properties = { + TagUpdateOnProvisionedProduct = "ALLOWED" + } + }) +} + +resource "aws_servicecatalog_product_portfolio_association" "test" { + portfolio_id = aws_servicecatalog_principal_portfolio_association.test.portfolio_id # avoid depends_on + product_id = aws_servicecatalog_product.test.id +} + +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_servicecatalog_principal_portfolio_association" "test" { + portfolio_id = aws_servicecatalog_portfolio.test.id + principal_arn = data.aws_iam_session_context.current.issuer_arn # unfortunately, you cannot get launch_path for arbitrary role - only caller +} + +data "aws_servicecatalog_launch_paths" "test" { + product_id = aws_servicecatalog_product_portfolio_association.test.product_id # avoid depends_on +} +`, rName, domain, email) +} + func testAccProvisionedProductConfig_basic(rName, domain, email, vpcCidr string) string { return acctest.ConfigCompose(testAccProvisionedProductTemplateURLBaseConfig(rName, domain, email), fmt.Sprintf(` @@ -425,6 +598,28 @@ resource "aws_servicecatalog_provisioned_product" "test" { `, rName, vpcCidr)) } +func testAccProvisionedProductConfig_computedOutputs(rName, domain, email, vpcCidr string) string { + return acctest.ConfigCompose(testAccProvisionedProductPhysicalTemplateIDBaseConfig(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 = "" + } +} +`, rName, vpcCidr)) +} + func testAccProvisionedProductConfig_updateTainted(rName, domain, email, vpcCidr string) string { return acctest.ConfigCompose(testAccProvisionedProductTemplateURLBaseConfig(rName, domain, email), fmt.Sprintf(` From 53902d0f24c73ee1ae5c201c5481969ca7e73576 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 21 Feb 2023 20:27:06 -0600 Subject: [PATCH 3/5] refactor some common test resources --- .../provisioned_product_test.go | 136 +++++++----------- 1 file changed, 52 insertions(+), 84 deletions(-) diff --git a/internal/service/servicecatalog/provisioned_product_test.go b/internal/service/servicecatalog/provisioned_product_test.go index b90e02dd9d9..b48dc3416f7 100644 --- a/internal/service/servicecatalog/provisioned_product_test.go +++ b/internal/service/servicecatalog/provisioned_product_test.go @@ -326,8 +326,54 @@ func testAccCheckProvisionedProductExists(ctx context.Context, resourceName stri } } -func testAccProvisionedProductTemplateURLBaseConfig(rName, domain, email string) string { +func testAccProvisionedProductPortfolioBaseConfig(rName string) string { return fmt.Sprintf(` +resource "aws_servicecatalog_portfolio" "test" { + name = %[1]q + description = %[1]q + provider_name = %[1]q +} + +resource "aws_servicecatalog_constraint" "test" { + description = %[1]q + portfolio_id = aws_servicecatalog_product_portfolio_association.test.portfolio_id + product_id = aws_servicecatalog_product_portfolio_association.test.product_id + type = "RESOURCE_UPDATE" + + parameters = jsonencode({ + Version = "2.0" + Properties = { + TagUpdateOnProvisionedProduct = "ALLOWED" + } + }) +} + +resource "aws_servicecatalog_product_portfolio_association" "test" { + portfolio_id = aws_servicecatalog_principal_portfolio_association.test.portfolio_id # avoid depends_on + product_id = aws_servicecatalog_product.test.id +} + +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_servicecatalog_principal_portfolio_association" "test" { + portfolio_id = aws_servicecatalog_portfolio.test.id + principal_arn = data.aws_iam_session_context.current.issuer_arn # unfortunately, you cannot get launch_path for arbitrary role - only caller +} + +data "aws_servicecatalog_launch_paths" "test" { + product_id = aws_servicecatalog_product_portfolio_association.test.product_id # avoid depends_on +} +`, rName) +} + +func testAccProvisionedProductTemplateURLBaseConfig(rName, domain, email string) string { + return acctest.ConfigCompose( + testAccProvisionedProductPortfolioBaseConfig(rName), + fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q force_destroy = true @@ -409,51 +455,13 @@ resource "aws_servicecatalog_product" "test" { Name = %[1]q } } - -resource "aws_servicecatalog_portfolio" "test" { - name = %[1]q - description = %[1]q - provider_name = %[1]q -} - -resource "aws_servicecatalog_constraint" "test" { - description = %[1]q - portfolio_id = aws_servicecatalog_product_portfolio_association.test.portfolio_id - product_id = aws_servicecatalog_product_portfolio_association.test.product_id - type = "RESOURCE_UPDATE" - - parameters = jsonencode({ - Version = "2.0" - Properties = { - TagUpdateOnProvisionedProduct = "ALLOWED" - } - }) -} - -resource "aws_servicecatalog_product_portfolio_association" "test" { - portfolio_id = aws_servicecatalog_principal_portfolio_association.test.portfolio_id # avoid depends_on - product_id = aws_servicecatalog_product.test.id -} - -data "aws_caller_identity" "current" {} - -data "aws_iam_session_context" "current" { - arn = data.aws_caller_identity.current.arn -} - -resource "aws_servicecatalog_principal_portfolio_association" "test" { - portfolio_id = aws_servicecatalog_portfolio.test.id - principal_arn = data.aws_iam_session_context.current.issuer_arn # unfortunately, you cannot get launch_path for arbitrary role - only caller -} - -data "aws_servicecatalog_launch_paths" "test" { - product_id = aws_servicecatalog_product_portfolio_association.test.product_id # avoid depends_on -} -`, rName, domain, email) +`, rName, domain, email)) } func testAccProvisionedProductPhysicalTemplateIDBaseConfig(rName, domain, email string) string { - return fmt.Sprintf(` + return acctest.ConfigCompose( + testAccProvisionedProductPortfolioBaseConfig(rName), + fmt.Sprintf(` resource "aws_cloudformation_stack" "test" { name = %[1]q @@ -533,47 +541,7 @@ resource "aws_servicecatalog_product" "test" { Name = %[1]q } } - -resource "aws_servicecatalog_portfolio" "test" { - name = %[1]q - description = %[1]q - provider_name = %[1]q -} - -resource "aws_servicecatalog_constraint" "test" { - description = %[1]q - portfolio_id = aws_servicecatalog_product_portfolio_association.test.portfolio_id - product_id = aws_servicecatalog_product_portfolio_association.test.product_id - type = "RESOURCE_UPDATE" - - parameters = jsonencode({ - Version = "2.0" - Properties = { - TagUpdateOnProvisionedProduct = "ALLOWED" - } - }) -} - -resource "aws_servicecatalog_product_portfolio_association" "test" { - portfolio_id = aws_servicecatalog_principal_portfolio_association.test.portfolio_id # avoid depends_on - product_id = aws_servicecatalog_product.test.id -} - -data "aws_caller_identity" "current" {} - -data "aws_iam_session_context" "current" { - arn = data.aws_caller_identity.current.arn -} - -resource "aws_servicecatalog_principal_portfolio_association" "test" { - portfolio_id = aws_servicecatalog_portfolio.test.id - principal_arn = data.aws_iam_session_context.current.issuer_arn # unfortunately, you cannot get launch_path for arbitrary role - only caller -} - -data "aws_servicecatalog_launch_paths" "test" { - product_id = aws_servicecatalog_product_portfolio_association.test.product_id # avoid depends_on -} -`, rName, domain, email) +`, rName, domain, email)) } func testAccProvisionedProductConfig_basic(rName, domain, email, vpcCidr string) string { From 5da4840fd24e613df70bdcd655572546f3098609 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 22 Feb 2023 10:39:51 -0600 Subject: [PATCH 4/5] add CHANGLOG entry --- .changelog/29559.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/29559.txt diff --git a/.changelog/29559.txt b/.changelog/29559.txt new file mode 100644 index 00000000000..ba89294c3cc --- /dev/null +++ b/.changelog/29559.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_servicecatalog_provisioned_product: Fix to allow `outputs` to be `Computed` when the resources changes +``` \ No newline at end of file From 74403b7378ff0c7375a95116af5a18ab11b49e5a Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 22 Feb 2023 10:49:46 -0600 Subject: [PATCH 5/5] update CHANGLOG entry --- .changelog/29559.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/29559.txt b/.changelog/29559.txt index ba89294c3cc..addeeef1c95 100644 --- a/.changelog/29559.txt +++ b/.changelog/29559.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_servicecatalog_provisioned_product: Fix to allow `outputs` to be `Computed` when the resources changes +resource/aws_servicecatalog_provisioned_product: Fix to allow `outputs` to be `Computed` when the resource changes ``` \ No newline at end of file