Merge pull request #41222 from hashicorp/td-framework-wrapper-modifyplan

Tech debt: Remove transparent tagging-only `ModifyPlan` implementations for Plugin Framework resources

docs/resource-tagging.md

@@ -163,15 +163,8 @@ The `tags_all` attribute contains a union of the tags set directly on the resour
     }
     ```
 
-Add a plan modifier (Terraform Plugin Framework) or a `CustomizeDiff` function (Terraform Plugin SDK V2) to ensure tagging diffs are handled appropriately.
-These functions handle the combination of tags set on the resource and default tags, and must be set for tagging to function properly.
-
-=== "Terraform Plugin Framework (Preferred)"
-    ```go
-    func (r *resourceExample) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-        r.SetTagsAll(ctx, req, resp)
-    }
-    ```
+Add a `CustomizeDiff` function (Terraform Plugin SDK V2) to ensure tagging diffs are handled appropriately.
+This function handles the combination of tags set on the resource and default tags, and must be set for tagging to function properly.
 
 === "Terraform Plugin SDK V2"
     ```go
@@ -183,8 +176,6 @@ These functions handle the combination of tags set on the resource and default t
     }
     ```
 
-If the resource already implements `ModifyPlan`, simply include the `SetTagsAll` function at the end of the method body.
-
 ### Transparent Tagging
 
 Most services can use a facility we call _transparent_ (or _implicit_) _tagging_, where the majority of resource tagging functionality is implemented using code located in the provider's runtime packages (see `internal/provider/intercept.go` and `internal/provider/fwprovider/intercept.go` for details) and not in the resource's CRUD handler functions. Resource implementers opt-in to transparent tagging by adding an _annotation_ (a specially formatted Go comment) to the resource's factory function (similar to the [resource self-registration mechanism](add-a-new-resource.md)).

docs/terraform-plugin-migrations.md

@@ -77,20 +77,12 @@ func (r *resourceExampleResource) Schema(ctx context.Context, request resource.S
 
 ## Tagging
 
-Tagging in the Plugin Framework is done by implementing the `ModifyPlan()` method on a resource.
-
-```go
-func (r *resourceExampleResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-```
-
 Transparent Tagging that is used in SDKv2 also applies to the Framework by using the `@Tags` decorator when defining the resource.
 
 ```go
 // @FrameworkResource("aws_service_example", name="Example Resource")
 // @Tags(identifierAttribute="arn")
-func newResourceExampleResrouce(_ context.Context) (resource.ResourceWithConfigure, error) {
+func newResourceExampleResource(_ context.Context) (resource.ResourceWithConfigure, error) {
 	r := resourceExampleResource{}
 	return &r, nil
 }

internal/framework/resource_with_configure.go

@@ -6,13 +6,8 @@ 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-provider-aws/internal/conns"
-	"github.com/hashicorp/terraform-provider-aws/internal/framework/flex"
-	tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
-	"github.com/hashicorp/terraform-provider-aws/names"
 )
 
 // ResourceWithConfigure is a structure to be embedded within a Resource that implements the ResourceWithConfigure interface.
@@ -34,49 +29,3 @@ func (r *ResourceWithConfigure) Configure(_ context.Context, request resource.Co
 		r.meta = v
 	}
 }
-
-// SetTagsAll calculates the new value for the `tags_all` attribute.
-func (r *ResourceWithConfigure) SetTagsAll(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	// If the entire plan is null, the resource is planned for destruction.
-	if request.Plan.Raw.IsNull() {
-		return
-	}
-
-	defaultTagsConfig := r.Meta().DefaultTagsConfig(ctx)
-	ignoreTagsConfig := r.Meta().IgnoreTagsConfig(ctx)
-
-	var planTags tftags.Map
-
-	response.Diagnostics.Append(request.Plan.GetAttribute(ctx, path.Root(names.AttrTags), &planTags)...)
-
-	if response.Diagnostics.HasError() {
-		return
-	}
-
-	if !planTags.IsUnknown() {
-		if !mapHasUnknownElements(planTags) {
-			resourceTags := tftags.New(ctx, planTags)
-			allTags := defaultTagsConfig.MergeTags(resourceTags).IgnoreConfig(ignoreTagsConfig)
-
-			response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root(names.AttrTagsAll), flex.FlattenFrameworkStringValueMapLegacy(ctx, allTags.Map()))...)
-		} else {
-			response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root(names.AttrTagsAll), tftags.Unknown)...)
-		}
-	} else {
-		response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root(names.AttrTagsAll), tftags.Unknown)...)
-	}
-}
-
-type mapValueElementsable interface {
-	Elements() map[string]attr.Value
-}
-
-func mapHasUnknownElements(m mapValueElementsable) bool {
-	for _, v := range m.Elements() {
-		if v.IsUnknown() {
-			return true
-		}
-	}
-
-	return false
-}

internal/provider/fwprovider/provider.go

@@ -442,8 +442,9 @@ func (p *fwprovider) Resources(ctx context.Context) []func() resource.Resource {
 
 					return ctx
 				},
-				interceptors: interceptors,
-				typeName:     typeName,
+				interceptors:           interceptors,
+				typeName:               typeName,
+				usesTransparentTagging: v.Tags != nil,
 			}
 			resources = append(resources, func() resource.Resource {
 				return newWrappedResource(inner, opts)

internal/provider/fwprovider/wrap.go

@@ -9,8 +9,12 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/ephemeral"
+	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
+	fwflex "github.com/hashicorp/terraform-provider-aws/internal/framework/flex"
+	tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
+	"github.com/hashicorp/terraform-provider-aws/names"
 )
 
 // contextFunc augments Context.
@@ -165,9 +169,10 @@ func (w *wrappedEphemeralResource) ValidateConfig(ctx context.Context, request e
 
 type wrappedResourceOptions struct {
 	// bootstrapContext is run on all wrapped methods before any interceptors.
-	bootstrapContext contextFunc
-	interceptors     resourceInterceptors
-	typeName         string
+	bootstrapContext       contextFunc
+	interceptors           resourceInterceptors
+	typeName               string
+	usesTransparentTagging bool
 }
 
 // wrappedResource represents an interceptor dispatcher for a Plugin Framework resource.
@@ -257,8 +262,16 @@ func (w *wrappedResource) ImportState(ctx context.Context, request resource.Impo
 }
 
 func (w *wrappedResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
+	ctx = w.opts.bootstrapContext(ctx, w.meta)
+
+	if w.opts.usesTransparentTagging {
+		w.setTagsAll(ctx, request, response)
+		if response.Diagnostics.HasError() {
+			return
+		}
+	}
+
 	if v, ok := w.inner.(resource.ResourceWithModifyPlan); ok {
-		ctx = w.opts.bootstrapContext(ctx, w.meta)
 		v.ModifyPlan(ctx, request, response)
 	}
 }
@@ -298,3 +311,24 @@ func (w *wrappedResource) MoveState(ctx context.Context) []resource.StateMover {
 
 	return nil
 }
+
+// setTagsAll is a plan modifier that calculates the new value for the `tags_all` attribute.
+func (w *wrappedResource) setTagsAll(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
+	// If the entire plan is null, the resource is planned for destruction.
+	if request.Plan.Raw.IsNull() {
+		return
+	}
+
+	var planTags tftags.Map
+	response.Diagnostics.Append(request.Plan.GetAttribute(ctx, path.Root(names.AttrTags), &planTags)...)
+	if response.Diagnostics.HasError() {
+		return
+	}
+
+	if planTags.IsWhollyKnown() {
+		allTags := w.meta.DefaultTagsConfig(ctx).MergeTags(tftags.New(ctx, planTags)).IgnoreConfig(w.meta.IgnoreTagsConfig(ctx))
+		response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root(names.AttrTagsAll), fwflex.FlattenFrameworkStringValueMapLegacy(ctx, allTags.Map()))...)
+	} else {
+		response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root(names.AttrTagsAll), tftags.Unknown)...)
+	}
+}

internal/service/amp/scraper.go

@@ -390,10 +390,6 @@ func (r *scraperResource) Delete(ctx context.Context, req resource.DeleteRequest
 	}
 }
 
-func (r *scraperResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, req, resp)
-}
-
 type scraperResourceModel struct {
 	Alias               types.String                                             `tfsdk:"alias"`
 	ARN                 types.String                                             `tfsdk:"arn"`

internal/service/apigateway/domain_name_access_association.go

@@ -167,10 +167,6 @@ func (r *domainNameAccessAssociationResource) Delete(ctx context.Context, reques
 	}
 }
 
-func (r *domainNameAccessAssociationResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func findDomainNameAccessAssociationByARN(ctx context.Context, conn *apigateway.Client, arn string) (*awstypes.DomainNameAccessAssociation, error) {
 	input := apigateway.GetDomainNameAccessAssociationsInput{
 		ResourceOwner: awstypes.ResourceOwnerSelf,

internal/service/appconfig/environment.go

@@ -298,10 +298,6 @@ func (r *resourceEnvironment) ImportState(ctx context.Context, request resource.
 	response.Diagnostics.Append(response.State.SetAttribute(ctx, path.Root(names.AttrApplicationID), parts[1])...)
 }
 
-func (r *resourceEnvironment) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 type resourceEnvironmentData struct {
 	ApplicationID types.String `tfsdk:"application_id"`
 	ARN           types.String `tfsdk:"arn"`

internal/service/appfabric/app_authorization.go

@@ -402,10 +402,6 @@ func (r *appAuthorizationResource) Delete(ctx context.Context, request resource.
 	}
 }
 
-func (r *appAuthorizationResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func findAppAuthorizationByTwoPartKey(ctx context.Context, conn *appfabric.Client, appAuthorizationARN, appBundleIdentifier string) (*awstypes.AppAuthorization, error) {
 	in := &appfabric.GetAppAuthorizationInput{
 		AppAuthorizationIdentifier: aws.String(appAuthorizationARN),

internal/service/appfabric/app_bundle.go

@@ -159,10 +159,6 @@ func (r *appBundleResource) Delete(ctx context.Context, request resource.DeleteR
 	}
 }
 
-func (r *appBundleResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func findAppBundleByID(ctx context.Context, conn *appfabric.Client, arn string) (*awstypes.AppBundle, error) {
 	input := &appfabric.GetAppBundleInput{
 		AppBundleIdentifier: aws.String(arn),

internal/service/appfabric/ingestion.go

@@ -204,10 +204,6 @@ func (r *ingestionResource) Delete(ctx context.Context, request resource.DeleteR
 	}
 }
 
-func (r *ingestionResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func findIngestionByTwoPartKey(ctx context.Context, conn *appfabric.Client, appBundleARN, arn string) (*awstypes.Ingestion, error) {
 	input := &appfabric.GetIngestionInput{
 		AppBundleIdentifier: aws.String(appBundleARN),

internal/service/appfabric/ingestion_destination.go

@@ -383,10 +383,6 @@ func (r *ingestionDestinationResource) ConfigValidators(context.Context) []resou
 	}
 }
 
-func (r *ingestionDestinationResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func findIngestionDestinationByThreePartKey(ctx context.Context, conn *appfabric.Client, appBundleARN, ingestionARN, arn string) (*awstypes.IngestionDestination, error) {
 	input := &appfabric.GetIngestionDestinationInput{
 		AppBundleIdentifier:            aws.String(appBundleARN),

internal/service/auditmanager/assessment.go

@@ -365,10 +365,6 @@ func (r *resourceAssessment) ImportState(ctx context.Context, req resource.Impor
 	resource.ImportStatePassthroughID(ctx, path.Root(names.AttrID), req, resp)
 }
 
-func (r *resourceAssessment) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, req, resp)
-}
-
 func FindAssessmentByID(ctx context.Context, conn *auditmanager.Client, id string) (*awstypes.Assessment, error) {
 	in := &auditmanager.GetAssessmentInput{
 		AssessmentId: aws.String(id),

internal/service/auditmanager/control.go

@@ -333,8 +333,6 @@ func (r *resourceControl) ModifyPlan(ctx context.Context, req resource.ModifyPla
 			}
 		}
 	}
-
-	r.SetTagsAll(ctx, req, resp)
 }
 
 func FindControlByID(ctx context.Context, conn *auditmanager.Client, id string) (*awstypes.Control, error) {

internal/service/auditmanager/framework.go

@@ -296,8 +296,6 @@ func (r *resourceFramework) ModifyPlan(ctx context.Context, req resource.ModifyP
 			}
 		}
 	}
-
-	r.SetTagsAll(ctx, req, resp)
 }
 
 func FindFrameworkByID(ctx context.Context, conn *auditmanager.Client, id string) (*awstypes.Framework, error) {

internal/service/backup/logically_air_gapped_vault.go

@@ -194,10 +194,6 @@ func (r *logicallyAirGappedVaultResource) Delete(ctx context.Context, request re
 	}
 }
 
-func (r *logicallyAirGappedVaultResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 type logicallyAirGappedVaultResourceModel struct {
 	BackupVaultARN   types.String   `tfsdk:"arn"`
 	BackupVaultName  types.String   `tfsdk:"name"`

internal/service/backup/restore_testing_plan.go

@@ -297,10 +297,6 @@ func (r *restoreTestingPlanResource) Delete(ctx context.Context, request resourc
 	}
 }
 
-func (r *restoreTestingPlanResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func (r *restoreTestingPlanResource) ImportState(ctx context.Context, request resource.ImportStateRequest, response *resource.ImportStateResponse) {
 	response.Diagnostics.Append(response.State.SetAttribute(ctx, path.Root(names.AttrName), request.ID)...)
 }

internal/service/batch/job_queue.go

@@ -386,10 +386,6 @@ func (r *jobQueueResource) Delete(ctx context.Context, request resource.DeleteRe
 	}
 }
 
-func (r *jobQueueResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func (r *jobQueueResource) ConfigValidators(_ context.Context) []resource.ConfigValidator {
 	return []resource.ConfigValidator{
 		resourcevalidator.ExactlyOneOf(

internal/service/bcmdataexports/export.go

@@ -377,10 +377,6 @@ func (r *resourceExport) Delete(ctx context.Context, req resource.DeleteRequest,
 	}
 }
 
-func (r *resourceExport) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, req, resp)
-}
-
 func waitExportCreated(ctx context.Context, conn *bcmdataexports.Client, id string, timeout time.Duration) (*bcmdataexports.GetExportOutput, error) {
 	stateConf := &retry.StateChangeConf{
 		Pending:                   []string{},

internal/service/bedrock/custom_model.go

@@ -472,10 +472,6 @@ func (r *customModelResource) Delete(ctx context.Context, request resource.Delet
 	}
 }
 
-func (r *customModelResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func findCustomModelByID(ctx context.Context, conn *bedrock.Client, id string) (*bedrock.GetCustomModelOutput, error) {
 	input := &bedrock.GetCustomModelInput{
 		ModelIdentifier: aws.String(id),

internal/service/bedrock/guardrail.go

@@ -547,10 +547,6 @@ func (r *resourceGuardrail) ImportState(ctx context.Context, req resource.Import
 	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root(names.AttrVersion), parts[1])...)
 }
 
-func (r *resourceGuardrail) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, req, resp)
-}
-
 func waitGuardrailCreated(ctx context.Context, conn *bedrock.Client, id string, version string, timeout time.Duration) (*bedrock.GetGuardrailOutput, error) { //nolint:unparam
 	stateConf := &retry.StateChangeConf{
 		Pending:                   enum.Slice(awstypes.GuardrailStatusCreating),

internal/service/bedrock/inference_profile.go

@@ -274,10 +274,6 @@ func (r *resourceInferenceProfile) Delete(ctx context.Context, req resource.Dele
 	}
 }
 
-func (r *resourceInferenceProfile) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, req, resp)
-}
-
 func (r *resourceInferenceProfile) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
 	resource.ImportStatePassthroughID(ctx, path.Root(names.AttrID), req, resp)
 }

internal/service/bedrock/provisioned_model_throughput.go

@@ -197,10 +197,6 @@ func (r *resourceProvisionedModelThroughput) Delete(ctx context.Context, request
 	}
 }
 
-func (r *resourceProvisionedModelThroughput) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, req, resp)
-}
-
 func findProvisionedModelThroughputByID(ctx context.Context, conn *bedrock.Client, id string) (*bedrock.GetProvisionedModelThroughputOutput, error) {
 	input := &bedrock.GetProvisionedModelThroughputInput{
 		ProvisionedModelId: aws.String(id),

internal/service/bedrockagent/agent.go

@@ -413,10 +413,6 @@ func (r *agentResource) ImportState(ctx context.Context, req resource.ImportStat
 	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("prepare_agent"), true)...)
 }
 
-func (r *agentResource) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) {
-	r.SetTagsAll(ctx, request, response)
-}
-
 func prepareAgent(ctx context.Context, conn *bedrockagent.Client, id string, timeout time.Duration) (*awstypes.Agent, error) {
 	input := &bedrockagent.PrepareAgentInput{
 		AgentId: aws.String(id),