Tags: Add custom Map type with semantic equality for tags and tags_all attributes

.changelog/38869.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+many resources: Fixes perpetual diff when tag has a `null` value.
+```

.ci/.golangci.yml

@@ -1,5 +1,5 @@
 issues:
-  max-per-linter: 0
+  max-issues-per-linter: 0
   max-same-issues: 0
 
 linters:

.ci/semgrep/framework/tags.yml

@@ -0,0 +1,8 @@
+rules:
+  - id: model-tag-types
+    languages: [go]
+    message: Use type `tags.Map` for $1
+    patterns:
+      - pattern-regex: (\w+)\s+types\.Map\s+`tfsdk:"tags(_all)?"`
+      - pattern-inside: type $TYPE struct { ... }
+    severity: ERROR

.teamcity/components/generated/services_all.kt

@@ -26,7 +26,7 @@ val services = mapOf(
     "autoscalingplans" to ServiceSpec("Auto Scaling Plans"),
     "backup" to ServiceSpec("Backup"),
     "batch" to ServiceSpec("Batch", vpcLock = true),
-    "bcmdataexports" to ServiceSpec("BCM Data Exports"),
+    "bcmdataexports" to ServiceSpec("BCM Data Exports", parallelismOverride = 5),
     "bedrock" to ServiceSpec("Bedrock"),
     "bedrockagent" to ServiceSpec("Bedrock Agents"),
     "budgets" to ServiceSpec("Web Services Budgets"),

GNUmakefile

@@ -864,6 +864,7 @@ yamllint: ## [CI] YAML Linting / yamllint
 	go-misspell \
 	golangci-lint1 \
 	golangci-lint2 \
+	golangci-lint3 \
 	golangci-lint \
 	help \
 	import-lint \

docs/resource-tagging.md

@@ -550,6 +550,9 @@ For example, 3 minutes and 30 seconds is `3m30s`.
 Some services do not support tags with an empty string value.
 In that case, use the annotation `@Testing(skipEmptyTags=true)`.
 
+Some services do not support tags with an null string value.
+In that case, use the annotation `@Testing(skipNullTags=true)`.
+
 Some resource types use the no-op `CheckDestroy` function `acctest.CheckDestroyNoop`.
 Use the annotation `@Testing(checkDestroyNoop=true)`.
 

internal/framework/resource_with_configure.go

@@ -6,9 +6,9 @@ package framework
 import (
 	"context"
 
+	"github.com/hashicorp/terraform-plugin-framework/attr"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
-	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/framework/flex"
 	tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
@@ -38,7 +38,7 @@ func (r *ResourceWithConfigure) SetTagsAll(ctx context.Context, request resource
 	defaultTagsConfig := r.Meta().DefaultTagsConfig
 	ignoreTagsConfig := r.Meta().IgnoreTagsConfig
 
-	var planTags types.Map
+	var planTags tftags.Map
 
 	response.Diagnostics.Append(request.Plan.GetAttribute(ctx, path.Root(names.AttrTags), &planTags)...)
 
@@ -60,7 +60,11 @@ func (r *ResourceWithConfigure) SetTagsAll(ctx context.Context, request resource
 	}
 }
 
-func mapHasUnknownElements(m types.Map) bool {
+type mapValueElementsable interface {
+	Elements() map[string]attr.Value
+}
+
+func mapHasUnknownElements(m mapValueElementsable) bool {
 	for _, v := range m.Elements() {
 		if v.IsUnknown() {
 			return true

internal/generate/tagstests/main.go

@@ -332,6 +332,7 @@ type ResourceDatum struct {
 	SerializeDelay            bool
 	PreCheck                  bool
 	SkipEmptyTags             bool // TODO: Remove when we have a strategy for resources that have a minimum tag value length of 1
+	SkipNullTags              bool
 	NoRemoveTags              bool
 	GoImports                 []goImport
 	GenerateConfig            bool
@@ -646,6 +647,14 @@ func (v *visitor) processFuncDecl(funcDecl *ast.FuncDecl) {
 						d.SkipEmptyTags = b
 					}
 				}
+				if attr, ok := args.Keyword["skipNullTags"]; ok {
+					if b, err := strconv.ParseBool(attr); err != nil {
+						v.errs = append(v.errs, fmt.Errorf("invalid skipNullTags value: %q at %s. Should be boolean value.", attr, fmt.Sprintf("%s.%s", v.packageName, v.functionName)))
+						continue
+					} else {
+						d.SkipNullTags = b
+					}
+				}
 				if attr, ok := args.Keyword["noRemoveTags"]; ok {
 					if b, err := strconv.ParseBool(attr); err != nil {
 						v.errs = append(v.errs, fmt.Errorf("invalid noRemoveTags value: %q at %s. Should be boolean value.", attr, fmt.Sprintf("%s.%s", v.packageName, v.functionName)))

internal/generate/tagstests/resource_test.go.gtpl

@@ -46,7 +46,7 @@ plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), know
 {{- end }}
 {{- end }}
 
-{{ define "ImportBody" }}
+{{ define "CommonImportBody" -}}
 	ResourceName: resourceName,
 	ImportState:  true,
 {{ if gt (len .ImportStateID) 0 -}}
@@ -56,13 +56,49 @@ plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), know
 	ImportStateIdFunc: {{ .ImportStateIDFunc }}(resourceName),
 {{ end -}}
 	ImportStateVerify: true,
+{{- end }}
+
+{{ define "ImportBody" }}
+{{ template "CommonImportBody" . }}
 {{ if gt (len .ImportIgnore) 0 -}}
 	ImportStateVerifyIgnore: []string{
 	{{ range $i, $v := .ImportIgnore }}{{ $v }},{{ end }}
 	},
 {{- end }}
 {{ end }}
 
+{{ define "ImportBodyIgnoreKey1" }}
+{{ template "CommonImportBody" . }}
+{{ if eq .Implementation "framework" -}}
+	ImportStateVerifyIgnore: []string{
+        acctest.CtTagsKey1, // The canonical value returned by the AWS API is ""
+		{{ if gt (len .ImportIgnore) 0 -}}
+		{{ range $i, $v := .ImportIgnore }}{{ $v }},{{ end }}
+		{{ end -}}
+	},
+{{- else if gt (len .ImportIgnore) 0 -}}
+	ImportStateVerifyIgnore: []string{
+		{{ range $i, $v := .ImportIgnore }}{{ $v }},{{ end }}
+	},
+{{- end }}
+{{ end }}
+
+{{ define "ImportBodyIgnoreResourceKey1" }}
+{{ template "CommonImportBody" . }}
+{{ if eq .Implementation "framework" -}}
+	ImportStateVerifyIgnore: []string{
+        "tags.resourcekey1", // The canonical value returned by the AWS API is ""
+		{{ if gt (len .ImportIgnore) 0 -}}
+		{{ range $i, $v := .ImportIgnore }}{{ $v }},{{ end }}
+		{{ end -}}
+	},
+{{ else if gt (len .ImportIgnore) 0 -}}
+	ImportStateVerifyIgnore: []string{
+		{{ range $i, $v := .ImportIgnore }}{{ $v }},{{ end }}
+	},
+{{- end }}
+{{ end }}
+
 {{ define "testname" -}}
 {{ if .Serialize }}testAcc{{ else }}TestAcc{{ end }}{{ .ResourceProviderNameUpper }}{{ .Name }}
 {{- end }}
@@ -347,8 +383,8 @@ func {{ template "testname" . }}_tags(t *testing.T) {
 }
 
 func {{ template "testname" . }}_tags_null(t *testing.T) {
-{{- if eq .Implementation "framework" }}
-	t.Skip("Tags with null values are not correctly handled with the Plugin Framework")
+{{- if .SkipNullTags }}
+	t.Skip("Resource {{ .Name }} does not support null tags")
 {{ end }}
 	{{- template "Init" . }}
 
@@ -371,16 +407,30 @@ func {{ template "testname" . }}_tags_null(t *testing.T) {
 					{{- template "ExistsCheck" . -}}
 				),
 				ConfigStateChecks: []statecheck.StateCheck{
-					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
+					{{ if eq .Implementation "framework" -}}
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.MapExact(map[string]knownvalue.Check{
+						acctest.CtKey1: knownvalue.Null(),
+					})),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
+						acctest.CtKey1: knownvalue.StringExact(""),
+					})),
+					{{- else -}}
+                    statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{})),
+					{{- end }}
 				},
 				ConfigPlanChecks: resource.ConfigPlanChecks{
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
-						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 						{{ if eq .Implementation "framework" -}}
-						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{})),
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.MapExact(map[string]knownvalue.Check{
+							acctest.CtKey1: knownvalue.Null(),
+						})),
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
+							acctest.CtKey1: knownvalue.StringExact(""),
+						})),
 						{{- else -}}
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 						// TODO: Should be known
 						plancheck.ExpectUnknownValue(resourceName, tfjsonpath.New(names.AttrTagsAll)),
 						{{- end }}
@@ -400,9 +450,10 @@ func {{ template "testname" . }}_tags_null(t *testing.T) {
 					}),
 					{{ template "AdditionalTfVars" . }}
 				},
-				{{- template "ImportBody" . -}}
+				{{- template "ImportBodyIgnoreKey1" . -}}
 			},
 			{{- end }}
+			{{ if eq .Implementation "sdk" -}}
 			{
 				{{ if .AlternateRegionProvider -}}
 				ProtoV5ProviderFactories: acctest.ProtoV5FactoriesAlternate(ctx, t),
@@ -416,6 +467,7 @@ func {{ template "testname" . }}_tags_null(t *testing.T) {
 				PlanOnly:           true,
 				ExpectNonEmptyPlan: false,
 			},
+			{{- end }}
 		},
 	})
 }
@@ -1854,8 +1906,8 @@ func {{ template "testname" . }}_tags_DefaultTags_emptyProviderOnlyTag(t *testin
 }
 
 func {{ template "testname" . }}_tags_DefaultTags_nullOverlappingResourceTag(t *testing.T) {
-{{- if eq .Implementation "framework" }}
-	t.Skip("Tags with null values are not correctly handled with the Plugin Framework")
+{{- if .SkipNullTags }}
+	t.Skip("Resource {{ .Name }} does not support null tags")
 {{ end }}
 	{{- template "Init" . }}
 
@@ -1883,18 +1935,36 @@ func {{ template "testname" . }}_tags_DefaultTags_nullOverlappingResourceTag(t *
 					{{- template "ExistsCheck" . -}}
 				),
 				ConfigStateChecks: []statecheck.StateCheck{
+					{{ if eq .Implementation "framework" -}}
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.MapExact(map[string]knownvalue.Check{
+						acctest.CtKey1: knownvalue.Null(),
+					})),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
+						acctest.CtKey1: knownvalue.StringExact(""),
+					})),
+					{{- else -}}
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
 						acctest.CtKey1: knownvalue.StringExact(acctest.CtProviderValue1),
 					})),
+					{{- end }}
 				},
 				ConfigPlanChecks: resource.ConfigPlanChecks{
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+						{{ if eq .Implementation "framework" -}}
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.MapExact(map[string]knownvalue.Check{
+							acctest.CtKey1: knownvalue.Null(),
+						})),
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
+							acctest.CtKey1: knownvalue.StringExact(""),
+						})),
+						{{- else -}}
 						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
 							acctest.CtKey1: knownvalue.StringExact(acctest.CtProviderValue1),
 						})),
+						{{- end }}
 					},
 				},
 			},
@@ -1916,16 +1986,16 @@ func {{ template "testname" . }}_tags_DefaultTags_nullOverlappingResourceTag(t *
 					}),
 					{{ template "AdditionalTfVars" . }}
 				},
-				{{- template "ImportBody" . -}}
+				{{- template "ImportBodyIgnoreKey1" . -}}
 			},
 			{{- end }}
 		},
 	})
 }
 
 func {{ template "testname" . }}_tags_DefaultTags_nullNonOverlappingResourceTag(t *testing.T) {
-{{- if eq .Implementation "framework" }}
-	t.Skip("Tags with null values are not correctly handled with the Plugin Framework")
+{{- if .SkipNullTags }}
+	t.Skip("Resource {{ .Name }} does not support null tags")
 {{ end }}
 	{{- template "Init" . }}
 
@@ -1953,18 +2023,38 @@ func {{ template "testname" . }}_tags_DefaultTags_nullNonOverlappingResourceTag(
 					{{- template "ExistsCheck" . -}}
 				),
 				ConfigStateChecks: []statecheck.StateCheck{
+					{{ if eq .Implementation "framework" -}}
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.MapExact(map[string]knownvalue.Check{
+						acctest.CtResourceKey1: knownvalue.Null(),
+					})),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
+						acctest.CtResourceKey1: knownvalue.StringExact(""),
+						acctest.CtProviderKey1: knownvalue.StringExact(acctest.CtProviderValue1),
+					})),
+					{{- else -}}
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
 						acctest.CtProviderKey1: knownvalue.StringExact(acctest.CtProviderValue1),
 					})),
+					{{- end }}
 				},
 				ConfigPlanChecks: resource.ConfigPlanChecks{
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+						{{ if eq .Implementation "framework" -}}
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.MapExact(map[string]knownvalue.Check{
+							acctest.CtResourceKey1: knownvalue.Null(),
+						})),
+						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
+							acctest.CtResourceKey1: knownvalue.StringExact(""),
+							acctest.CtProviderKey1: knownvalue.StringExact(acctest.CtProviderValue1),
+						})),
+						{{- else -}}
 						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTags), knownvalue.Null()),
 						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrTagsAll), knownvalue.MapExact(map[string]knownvalue.Check{
 							acctest.CtProviderKey1: knownvalue.StringExact(acctest.CtProviderValue1),
 						})),
+						{{- end }}
 					},
 				},
 			},
@@ -1986,7 +2076,7 @@ func {{ template "testname" . }}_tags_DefaultTags_nullNonOverlappingResourceTag(
 					}),
 					{{ template "AdditionalTfVars" . }}
 				},
-				{{- template "ImportBody" . -}}
+				{{- template "ImportBodyIgnoreResourceKey1" . -}}
 			},
 			{{- end }}
 		},

internal/generate/teamcity/acctest_services.hcl

@@ -35,6 +35,10 @@ service "batch" {
   vpc_lock = true
 }
 
+service "bcmdataexports" {
+  parallelism = 5
+}
+
 service "cloudformation" {
   vpc_lock = true
 }