diff --git a/.changelog/19251.txt b/.changelog/19251.txt new file mode 100644 index 00000000000..835d575b494 --- /dev/null +++ b/.changelog/19251.txt @@ -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 +``` \ No newline at end of file diff --git a/aws/resource_aws_vpc_test.go b/aws/resource_aws_vpc_test.go index 13c63277eb2..42e722158d1 100644 --- a/aws/resource_aws_vpc_test.go +++ b/aws/resource_aws_vpc_test.go @@ -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 @@ -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" diff --git a/aws/tags.go b/aws/tags.go index 1ea9d1d3d47..125b976bd18 100644 --- a/aws/tags.go +++ b/aws/tags.go @@ -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 { @@ -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