Skip to content

Commit

Permalink
add additional if-condition when setting tags_all to new computed
Browse files Browse the repository at this point in the history
  • Loading branch information
@anGie44
anGie44 committed May 5, 2021
1 parent 3aa9b01 commit 7d7dce3
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 3 deletions.
3 changes: 3 additions & 0 deletions .changelog/19251.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
provider: Prevent `Provider produced inconsistent final plan` errors when lifecycle arguments apply to resource `tags` not known until apply
```
150 changes: 150 additions & 0 deletions aws/resource_aws_vpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,114 @@ func TestAccAWSVpc_defaultTags_providerAndResource_duplicateTag(t *testing.T) {
})
}

// TestAccAWSVpc_DynamicResourceTagsMergedWithLocals_IgnoreChanges ensures computed "tags_all"
// attributes are correctly determined when the provider-level default_tags block
// is left unused and resource tags (merged with local.tags) are only known at apply time,
// with additional lifecycle ignore_changes attributes, thereby eliminating "Inconsistent final plan" errors
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/18366
func TestAccAWSVpc_DynamicResourceTagsMergedWithLocals_IgnoreChanges(t *testing.T) {
var providers []*schema.Provider
var vpc ec2.Vpc

resourceName := "aws_vpc.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID),
ProviderFactories: testAccProviderFactoriesInternal(&providers),
CheckDestroy: testAccCheckVpcDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSVPCConfigWithIgnoreChanges_DynamicTagsMergedWithLocals("localkey", "localvalue"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVpcExists(resourceName, &vpc),
resource.TestCheckResourceAttr(resourceName, "tags.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags.localkey", "localvalue"),
resource.TestCheckResourceAttrSet(resourceName, "tags.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags.updated_at"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags_all.localkey", "localvalue"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.updated_at"),
),
// Dynamic tag "updated_at" will cause a perpetual diff but that's OK for this test
// as we want to ensure subsequent applies will not result in "inconsistent final plan errors"
// for the attribute "tags_all"
ExpectNonEmptyPlan: true,
},
{
Config: testAccAWSVPCConfigWithIgnoreChanges_DynamicTagsMergedWithLocals("localkey", "localvalue"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVpcExists(resourceName, &vpc),
resource.TestCheckResourceAttr(resourceName, "tags.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags.localkey", "localvalue"),
resource.TestCheckResourceAttrSet(resourceName, "tags.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags.updated_at"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags_all.localkey", "localvalue"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.updated_at"),
),
// Dynamic tag "updated_at" will cause a perpetual diff but that's OK for this test
// as we wanted to ensure this configuration applies successfully
ExpectNonEmptyPlan: true,
},
},
})
}

// TestAccAWSVpc_DynamicResourceTags_IgnoreChanges ensures computed "tags_all"
// attributes are correctly determined when the provider-level default_tags block
// is left unused and resource tags are only known at apply time,
// with additional lifecycle ignore_changes attributes, thereby eliminating "Inconsistent final plan" errors
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/18366
func TestAccAWSVpc_DynamicResourceTags_IgnoreChanges(t *testing.T) {
var providers []*schema.Provider
var vpc ec2.Vpc

resourceName := "aws_vpc.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID),
ProviderFactories: testAccProviderFactoriesInternal(&providers),
CheckDestroy: testAccCheckVpcDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSVPCConfigWithIgnoreChanges_DynamicTags,
Check: resource.ComposeTestCheckFunc(
testAccCheckVpcExists(resourceName, &vpc),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttrSet(resourceName, "tags.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags.updated_at"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "2"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.updated_at"),
),
// Dynamic tag "updated_at" will cause a perpetual diff but that's OK for this test
// as we want to ensure subsequent applies will not result in "inconsistent final plan errors"
// for the attribute "tags_all"
ExpectNonEmptyPlan: true,
},
{
Config: testAccAWSVPCConfigWithIgnoreChanges_DynamicTags,
Check: resource.ComposeTestCheckFunc(
testAccCheckVpcExists(resourceName, &vpc),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttrSet(resourceName, "tags.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags.updated_at"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "2"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.created_at"),
resource.TestCheckResourceAttrSet(resourceName, "tags_all.updated_at"),
),
// Dynamic tag "updated_at" will cause a perpetual diff but that's OK for this test
// as we wanted to ensure this configuration applies successfully
ExpectNonEmptyPlan: true,
},
},
})
}

func TestAccAWSVpc_defaultAndIgnoreTags(t *testing.T) {
var providers []*schema.Provider
var vpc ec2.Vpc
Expand Down Expand Up @@ -950,6 +1058,48 @@ resource "aws_vpc" "test" {
`, tagKey1, tagValue1, tagKey2, tagValue2)
}

func testAccAWSVPCConfigWithIgnoreChanges_DynamicTagsMergedWithLocals(localTagKey1, localTagValue1 string) string {
return fmt.Sprintf(`
locals {
tags = {
%[1]q = %[2]q
}
}
resource "aws_vpc" "test" {
cidr_block = "10.1.0.0/16"
tags = merge(local.tags, {
"created_at" = timestamp()
"updated_at" = timestamp()
})
lifecycle {
ignore_changes = [
tags["created_at"],
]
}
}
`, localTagKey1, localTagValue1)
}

const testAccAWSVPCConfigWithIgnoreChanges_DynamicTags = `
resource "aws_vpc" "test" {
cidr_block = "10.1.0.0/16"
tags = {
"created_at" = timestamp()
"updated_at" = timestamp()
}
lifecycle {
ignore_changes = [
tags["created_at"],
]
}
}
`

const testAccVpcDedicatedConfig = `
resource "aws_vpc" "test" {
instance_tenancy = "dedicated"
Expand Down
11 changes: 8 additions & 3 deletions aws/tags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ func SetTagsDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{})

allTags := defaultTagsConfig.MergeTags(resourceTags).IgnoreConfig(ignoreTagsConfig)

// To ensure "tags_all" is correctly computed, we explicitly set the attribute
// diff when the merger of resource-level tags onto provider-level tags results in n > 0 tags,
// otherwise we mark the attribute as "Computed" only when their is a known diff for "tags_all"
// To ensure "tags_all" is correctly computed, we explicitly set the attribute diff
// when the merger of resource-level tags onto provider-level tags results in n > 0 tags,
// otherwise we mark the attribute as "Computed" only when their is a known diff (excluding an empty map)
// or a change for "tags_all".
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/18366
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19005
if len(allTags) > 0 {
Expand All @@ -124,6 +125,10 @@ func SetTagsDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{})
if err := diff.SetNewComputed("tags_all"); err != nil {
return fmt.Errorf("error setting tags_all to computed: %w", err)
}
} else if diff.HasChange("tags_all") {
if err := diff.SetNewComputed("tags_all"); err != nil {
return fmt.Errorf("error setting tags_all to computed: %w", err)
}
}

return nil
Expand Down

0 comments on commit 7d7dce3

Please sign in to comment.