diff --git a/.changelog/31826.txt b/.changelog/31826.txt new file mode 100644 index 00000000000..2cb06c5aa43 --- /dev/null +++ b/.changelog/31826.txt @@ -0,0 +1,3 @@ +```release-note:bug +provider/default_tags: Fix perpetual diff when identical tags are moved from `default_tags` to resource `tags`, and vice versa +``` \ No newline at end of file diff --git a/internal/service/ec2/vpc_test.go b/internal/service/ec2/vpc_test.go index 571eedf1356..d5258670e23 100644 --- a/internal/service/ec2/vpc_test.go +++ b/internal/service/ec2/vpc_test.go @@ -523,6 +523,52 @@ func TestAccVPC_DefaultTagsProviderAndResource_duplicateTag(t *testing.T) { }) } +func TestAccVPC_DefaultTagsProviderAndResource_moveDuplicateTags(t *testing.T) { + ctx := acctest.Context(t) + var vpc ec2.Vpc + resourceName := "aws_vpc.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: acctest.ConfigCompose( + testAccVPCConfig_tags1("overlapkey", "overlapvalue"), + ), + Check: resource.ComposeTestCheckFunc( + acctest.CheckVPCExists(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"), + ), + }, + { + Config: acctest.ConfigCompose( + testAccVPCConfig_basic, + acctest.ConfigDefaultTags_Tags1("overlapkey", "overlapvalue"), + ), + Check: resource.ComposeTestCheckFunc( + acctest.CheckVPCExists(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"), + ), + }, + { + Config: acctest.ConfigCompose( + testAccVPCConfig_tags1("overlapkey", "overlapvalue"), + ), + Check: resource.ComposeTestCheckFunc( + acctest.CheckVPCExists(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"), + ), + }, + }, + }) +} + // TestAccVPC_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, diff --git a/internal/tags/key_value_tags.go b/internal/tags/key_value_tags.go index dfb8b5de6c4..372e5e70613 100644 --- a/internal/tags/key_value_tags.go +++ b/internal/tags/key_value_tags.go @@ -749,27 +749,37 @@ type schemaResourceData interface { GetRawState() cty.Value } +// tagSource is an enum that identifiers the source of the tag +type tagSource int + +const ( + configuration tagSource = iota + plan + state +) + +// configTag contains the value and source of the incoming tag +type configTag struct { + value string + source tagSource +} + +// ResolveDuplicates resolves differences between incoming tags, defaultTags, and ignoreConfig func (tags KeyValueTags) ResolveDuplicates(ctx context.Context, defaultConfig *DefaultConfig, ignoreConfig *IgnoreConfig, d schemaResourceData) KeyValueTags { // remove default config. t := tags.RemoveDefaultConfig(defaultConfig) + cf := d.GetRawConfig() + configExists := !cf.IsNull() && cf.IsKnown() + result := make(map[string]string) for k, v := range t { result[k] = v.ValueString() } - configTags := make(map[string]string) - if config := d.GetRawPlan(); !config.IsNull() && config.IsKnown() { - c := config.GetAttr("tags") - if !c.IsNull() && c.IsKnown() { - for k, v := range c.AsValueMap() { - configTags[k] = v.AsString() - } - } - } - - if config := d.GetRawConfig(); !config.IsNull() && config.IsKnown() { - c := config.GetAttr("tags") + configTags := make(map[string]configTag) + if configExists { + c := cf.GetAttr(names.AttrTags) // if the config is null just return the incoming tags // no duplicates to calculate @@ -778,33 +788,37 @@ func (tags KeyValueTags) ResolveDuplicates(ctx context.Context, defaultConfig *D } if !c.IsNull() && c.IsKnown() { - for k, v := range c.AsValueMap() { - if _, ok := configTags[k]; !ok { - // config tags can be null values. Ignore. - if !v.IsNull() { - configTags[k] = v.AsString() - } - } - } + normalizeTagsFromRaw(c.AsValueMap(), configTags, configuration) } } - if state := d.GetRawState(); !state.IsNull() && state.IsKnown() { - c := state.GetAttr("tags") + if pl := d.GetRawPlan(); !pl.IsNull() && pl.IsKnown() { + c := pl.GetAttr(names.AttrTags) + if !c.IsNull() && c.IsKnown() { + normalizeTagsFromRaw(c.AsValueMap(), configTags, plan) + } + } + + if st := d.GetRawState(); !st.IsNull() && st.IsKnown() { + c := st.GetAttr(names.AttrTags) if !c.IsNull() { - for k, v := range c.AsValueMap() { - if _, ok := configTags[k]; !ok { - configTags[k] = v.AsString() - } - } + normalizeTagsFromRaw(c.AsValueMap(), configTags, state) } } for k, v := range configTags { if _, ok := result[k]; !ok { if defaultConfig != nil { - if val, ok := defaultConfig.Tags[k]; ok && val.ValueString() == v { - result[k] = v + if val, ok := defaultConfig.Tags[k]; ok && val.ValueString() == v.value { + // config does not exist during a refresh. + // set duplicate values from other sources for refresh diff calculation + if !configExists { + result[k] = v.value + } else { + if v.source == configuration { + result[k] = v.value + } + } } } } @@ -813,6 +827,7 @@ func (tags KeyValueTags) ResolveDuplicates(ctx context.Context, defaultConfig *D return New(ctx, result).IgnoreConfig(ignoreConfig) } +// ResolveDuplicatesFramework resolves differences between incoming tags, defaultTags, and ignoreConfig func (tags KeyValueTags) ResolveDuplicatesFramework(ctx context.Context, defaultConfig *DefaultConfig, ignoreConfig *IgnoreConfig, resp *resource.ReadResponse, diags fwdiag.Diagnostics) KeyValueTags { // remove default config. t := tags.RemoveDefaultConfig(defaultConfig) @@ -860,3 +875,16 @@ func ToSnakeCase(str string) string { result = regexp.MustCompile("([a-z0-9])([A-Z])").ReplaceAllString(result, "${1}_${2}") return strings.ToLower(result) } + +func normalizeTagsFromRaw(m map[string]cty.Value, incoming map[string]configTag, source tagSource) { + for k, v := range m { + if !v.IsNull() { + if _, ok := incoming[k]; !ok { + incoming[k] = configTag{ + value: v.AsString(), + source: source, + } + } + } + } +}