Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/aws_cloudformation_stack: Sets outputs as Computed when other fields change #33059

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/33059.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_cloudformation_stack: Marks `outputs` as Computed when there are potential changes.
```
34 changes: 33 additions & 1 deletion internal/service/cloudformation/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/aws/aws-sdk-go/service/cloudformation"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"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/id"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -132,7 +133,10 @@ func ResourceStack() *schema.Resource {
},
},

CustomizeDiff: verify.SetTagsDiff,
CustomizeDiff: customdiff.All(
verify.SetTagsDiff,
customdiff.ComputedIf("outputs", stackHasActualChanges),
),
}
}

Expand Down Expand Up @@ -690,3 +694,31 @@ func isStackDeletionEvent(event *cloudformation.StackEvent) bool {
func reasonFromEvent(event *cloudformation.StackEvent) string {
return aws.StringValue(event.ResourceStatusReason)
}

func stackHasActualChanges(ctx context.Context, d *schema.ResourceDiff, meta any) bool {
if d.Id() == "" {
return false
}

for k, attr := range ResourceStack().Schema {
if attr.ForceNew {
continue
}
if attr.Computed && !attr.Optional {
continue
}

if d.HasChange(k) {
if attr.StateFunc == nil {
return true
}
o, n := d.GetChange(k)
on := attr.StateFunc(o)
nn := attr.StateFunc(n)
if on != nn {
return true
}
}
}
return false
}
159 changes: 140 additions & 19 deletions internal/service/cloudformation/stack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
"github.com/aws/aws-sdk-go/service/cloudformation"
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-plugin-testing/tfjsonpath"
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
tfcloudformation "github.com/hashicorp/terraform-provider-aws/internal/service/cloudformation"
Expand All @@ -33,17 +35,26 @@ func TestAccCloudFormationStack_basic(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "name", rName),
resource.TestCheckNoResourceAttr(resourceName, "on_failure"),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "2"),
resource.TestCheckResourceAttrSet(resourceName, "outputs.DefaultSgId"),
resource.TestCheckResourceAttrSet(resourceName, "outputs.VpcID"),
resource.TestCheckResourceAttr(resourceName, "parameters.%", "0"),
resource.TestCheckNoResourceAttr(resourceName, "template_url"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccStackConfig_basic(rName),
PlanOnly: true,
},
},
})
}
Expand Down Expand Up @@ -119,7 +130,7 @@ func TestAccCloudFormationStack_updateFailure(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_params(rName, vpcCidrInitial),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -145,7 +156,7 @@ func TestAccCloudFormationStack_disappears(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
acctest.CheckResourceDisappears(ctx, acctest.Provider, tfcloudformation.ResourceStack(), resourceName),
),
Expand All @@ -169,7 +180,7 @@ func TestAccCloudFormationStack_yaml(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_yaml(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -196,7 +207,7 @@ func TestAccCloudFormationStack_defaultParams(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_defaultParams(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand Down Expand Up @@ -225,7 +236,7 @@ func TestAccCloudFormationStack_allAttributes(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_allAttributesBodies(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "name", rName),
resource.TestCheckResourceAttr(resourceName, "capabilities.#", "1"),
Expand All @@ -249,7 +260,7 @@ func TestAccCloudFormationStack_allAttributes(t *testing.T) {
},
{
Config: testAccStackConfig_allAttributesBodiesModified(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "name", rName),
resource.TestCheckResourceAttr(resourceName, "capabilities.#", "1"),
Expand Down Expand Up @@ -287,10 +298,11 @@ func TestAccCloudFormationStack_withParams(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_params(rName, vpcCidrInitial),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "parameters.%", "1"),
resource.TestCheckResourceAttr(resourceName, "parameters.VpcCIDR", vpcCidrInitial),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "0"),
),
},
{
Expand All @@ -301,10 +313,11 @@ func TestAccCloudFormationStack_withParams(t *testing.T) {
},
{
Config: testAccStackConfig_params(rName, vpcCidrUpdated),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "parameters.%", "1"),
resource.TestCheckResourceAttr(resourceName, "parameters.VpcCIDR", vpcCidrUpdated),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "0"),
),
},
},
Expand All @@ -326,7 +339,7 @@ func TestAccCloudFormationStack_WithURL_withParams(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_templateURLParams(rName, "tf-cf-stack.json", "11.0.0.0/16"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -338,7 +351,7 @@ func TestAccCloudFormationStack_WithURL_withParams(t *testing.T) {
},
{
Config: testAccStackConfig_templateURLParams(rName, "tf-cf-stack.json", "13.0.0.0/16"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -360,7 +373,7 @@ func TestAccCloudFormationStack_WithURLWithParams_withYAML(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_templateURLParamsYAML(rName, "tf-cf-stack.test", "13.0.0.0/16"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand Down Expand Up @@ -389,7 +402,7 @@ func TestAccCloudFormationStack_WithURLWithParams_noUpdate(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_templateURLParams(rName, "tf-cf-stack-1.json", "11.0.0.0/16"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -401,7 +414,7 @@ func TestAccCloudFormationStack_WithURLWithParams_noUpdate(t *testing.T) {
},
{
Config: testAccStackConfig_templateURLParams(rName, "tf-cf-stack-2.json", "11.0.0.0/16"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -423,14 +436,14 @@ func TestAccCloudFormationStack_withTransform(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_transform(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
{
PlanOnly: true,
Config: testAccStackConfig_transform(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
),
},
Expand All @@ -453,7 +466,7 @@ func TestAccCloudFormationStack_onFailure(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_onFailure(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "disable_rollback", "false"),
resource.TestCheckResourceAttr(resourceName, "on_failure", cloudformation.OnFailureDoNothing),
Expand All @@ -477,24 +490,83 @@ func TestAccCloudFormationStack_outputsUpdated(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccStackConfig_parametersAndOutputs(rName, "in1"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "parameters.%", "1"),
resource.TestCheckResourceAttr(resourceName, "parameters.DataIn", "in1"),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "1"),
resource.TestCheckResourceAttr(resourceName, "outputs.DataOut", "pre-in1-post"),
resource.TestCheckOutput("stack_DataOut", "pre-in1-post"),
),
},
{
Config: testAccStackConfig_parametersAndOutputs(rName, "in2"),
Check: resource.ComposeTestCheckFunc(
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
plancheck.ExpectUnknownValue(resourceName, tfjsonpath.New("outputs")),
},
},
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "parameters.%", "1"),
resource.TestCheckResourceAttr(resourceName, "parameters.DataIn", "in2"),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "1"),
resource.TestCheckResourceAttr(resourceName, "outputs.DataOut", "pre-in2-post"),
resource.TestCheckOutput("stack_DataOut", "pre-in2-post"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"parameters"},
},
},
})
}

func TestAccCloudFormationStack_templateUpdate(t *testing.T) {
ctx := acctest.Context(t)
var stack cloudformation.Stack
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_cloudformation_stack.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, cloudformation.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckStackDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccStackConfig_templateUpdate(rName, "out1", "value1"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "1"),
resource.TestCheckResourceAttr(resourceName, "outputs.out1", "value1"),
resource.TestCheckOutput("stack_output", "out1:value1"),
),
},
{
Config: testAccStackConfig_templateUpdate(rName, "out2", "value2"),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
plancheck.ExpectUnknownValue(resourceName, tfjsonpath.New("outputs")),
},
},
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckStackExists(ctx, resourceName, &stack),
resource.TestCheckResourceAttr(resourceName, "outputs.%", "1"),
resource.TestCheckResourceAttr(resourceName, "outputs.out2", "value2"),
resource.TestCheckOutput("stack_output", "out2:value2"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand Down Expand Up @@ -1109,5 +1181,54 @@ STACK
DataIn = %[2]q
}
}

output "stack_DataOut" {
value = aws_cloudformation_stack.test.outputs["DataOut"]
}
`, rName, dataIn)
}

func testAccStackConfig_templateUpdate(rName, name, value string) string {
return fmt.Sprintf(`
resource "aws_cloudformation_stack" "test" {
name = %[1]q

template_body = jsonencode(
merge(
jsondecode(local.template),
{ "Outputs" = local.outputs },
)
)
}

locals {
template = <<STACK
{
"AWSTemplateFormatVersion": "2010-09-09",
"Description": "AWS CloudFormation template that returns a constant value",
"Conditions": {
"NeverTrue": {"Fn::Equals": ["true","false"]}
},
"Resources": {
"nullRes": {
"Type": "Custom::NullResource",
"Condition": "NeverTrue",
"Properties": {
"ServiceToken": ""
}
}
}
}
STACK
outputs = {
%[2]s = {
Value = %[3]q
}
}
}

output "stack_output" {
value = format("%%s:%%s", %[2]q, aws_cloudformation_stack.test.outputs[%[2]q])
}
`, rName, name, value)
}
Loading