internal/fwserver: Move write-only nullification to earlier in PlanRe…

…sourceChange (#1097) * Move write-only nullification so that it happens before marking computed attributes as unknown * Add changelog entry
hashicorp · Feb 19, 2025 · 3000d8c · 3000d8c
1 parent 7c9193d
commit 3000d8c
Show file tree

Hide file tree

Showing 3 changed files with 126 additions and 12 deletions.
diff --git a/.changes/unreleased/BUG FIXES-20250219-152752.yaml b/.changes/unreleased/BUG FIXES-20250219-152752.yaml
@@ -0,0 +1,5 @@
+kind: BUG FIXES
+body: 'internal/fwserver: fixed bug where write-only attributes set in configuration would cause perpetual diffs for computed attributes.'
+time: 2025-02-19T15:27:52.52508-05:00
+custom:
+    Issue: "1097"
diff --git a/internal/fwserver/server_planresourcechange.go b/internal/fwserver/server_planresourcechange.go
@@ -155,6 +155,18 @@ func (s *Server) PlanResourceChange(ctx context.Context, req *PlanResourceChange
 		resp.PlannedState.Raw = data.TerraformValue
 	}
 
+	// Set any write-only attributes in the plan to null
+	modifiedPlan, err := tftypes.Transform(resp.PlannedState.Raw, NullifyWriteOnlyAttributes(ctx, resp.PlannedState.Schema))
+	if err != nil {
+		resp.Diagnostics.AddError(
+			"Error Modifying Planned State",
+			"There was an unexpected error modifying the PlannedState. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(),
+		)
+		return
+	}
+
+	resp.PlannedState.Raw = modifiedPlan
+
 	// After ensuring there are proposed changes, mark any computed attributes
 	// that are null in the config as unknown in the plan, so providers have
 	// the choice to update them.
@@ -339,18 +351,6 @@ func (s *Server) PlanResourceChange(ctx context.Context, req *PlanResourceChange
 		)
 		return
 	}
-
-	// Set any write-only attributes in the plan to null
-	modifiedPlan, err := tftypes.Transform(resp.PlannedState.Raw, NullifyWriteOnlyAttributes(ctx, resp.PlannedState.Schema))
-	if err != nil {
-		resp.Diagnostics.AddError(
-			"Error Modifying Planned State",
-			"There was an unexpected error modifying the PlannedState. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(),
-		)
-		return
-	}
-
-	resp.PlannedState.Raw = modifiedPlan
 }
 
 func MarkComputedNilsAsUnknown(ctx context.Context, config tftypes.Value, resourceSchema fwschema.Schema) func(*tftypes.AttributePath, tftypes.Value) (tftypes.Value, error) {

diff --git a/internal/fwserver/server_planresourcechange_test.go b/internal/fwserver/server_planresourcechange_test.go
@@ -437,6 +437,14 @@ func TestServerPlanResourceChange(t *testing.T) {
 		},
 	}
 
+	testSchemaTypeWriteOnly := tftypes.Object{
+		AttributeTypes: map[string]tftypes.Type{
+			"test_computed":   tftypes.String,
+			"test_required":   tftypes.String,
+			"test_write_only": tftypes.String,
+		},
+	}
+
 	testSchemaTypeDefault := tftypes.Object{
 		AttributeTypes: map[string]tftypes.Type{
 			"test_computed_bool":                    tftypes.Bool,
@@ -566,6 +574,21 @@ func TestServerPlanResourceChange(t *testing.T) {
 		},
 	}
 
+	testSchemaWriteOnly := schema.Schema{
+		Attributes: map[string]schema.Attribute{
+			"test_computed": schema.StringAttribute{
+				Computed: true,
+			},
+			"test_required": schema.StringAttribute{
+				Required: true,
+			},
+			"test_write_only": schema.StringAttribute{
+				Optional:  true,
+				WriteOnly: true,
+			},
+		},
+	}
+
 	testSchemaDefault := schema.Schema{
 		Attributes: map[string]schema.Attribute{
 			"test_computed_bool": schema.BoolAttribute{
@@ -1069,6 +1092,11 @@ func TestServerPlanResourceChange(t *testing.T) {
 		Schema: testSchema,
 	}
 
+	testEmptyStateWriteOnly := &tfsdk.State{
+		Raw:    tftypes.NewValue(testSchemaType, nil),
+		Schema: testSchemaWriteOnly,
+	}
+
 	testEmptyStateDefault := &tfsdk.State{
 		Raw:    tftypes.NewValue(testSchemaTypeDefault, nil),
 		Schema: testSchemaDefault,
@@ -1367,6 +1395,43 @@ func TestServerPlanResourceChange(t *testing.T) {
 				PlannedPrivate: testEmptyPrivate,
 			},
 		},
+		"create-mark-computed-config-nils-as-unknown-write-only": {
+			server: &fwserver.Server{
+				Provider: &testprovider.Provider{},
+			},
+			request: &fwserver.PlanResourceChangeRequest{
+				Config: &tfsdk.Config{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, nil),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, "test-write-only-value"),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				ProposedNewState: &tfsdk.Plan{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, nil),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, "test-write-only-value"),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				PriorState:     testEmptyStateWriteOnly,
+				ResourceSchema: testSchemaWriteOnly,
+				Resource:       &testprovider.Resource{},
+			},
+			expectedResponse: &fwserver.PlanResourceChangeResponse{
+				PlannedState: &tfsdk.State{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, tftypes.UnknownValue),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, nil),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				PlannedPrivate: testEmptyPrivate,
+			},
+		},
 		"create-set-default-values": {
 			server: &fwserver.Server{
 				Provider: &testprovider.Provider{},
@@ -3883,6 +3948,50 @@ func TestServerPlanResourceChange(t *testing.T) {
 				PlannedPrivate: testEmptyPrivate,
 			},
 		},
+		"update-mark-computed-config-nils-as-unknown-write-only": {
+			server: &fwserver.Server{
+				Provider: &testprovider.Provider{},
+			},
+			request: &fwserver.PlanResourceChangeRequest{
+				Config: &tfsdk.Config{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, nil),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, "test-write-only-value"),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				ProposedNewState: &tfsdk.Plan{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, "prior-state-val"),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, "test-write-only-value"),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				PriorState: &tfsdk.State{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, "prior-state-val"),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, nil),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				ResourceSchema: testSchemaWriteOnly,
+				Resource:       &testprovider.Resource{},
+			},
+			expectedResponse: &fwserver.PlanResourceChangeResponse{
+				PlannedState: &tfsdk.State{
+					Raw: tftypes.NewValue(testSchemaTypeWriteOnly, map[string]tftypes.Value{
+						"test_computed":   tftypes.NewValue(tftypes.String, "prior-state-val"),
+						"test_required":   tftypes.NewValue(tftypes.String, "test-config-value"),
+						"test_write_only": tftypes.NewValue(tftypes.String, nil),
+					}),
+					Schema: testSchemaWriteOnly,
+				},
+				PlannedPrivate: testEmptyPrivate,
+			},
+		},
 		"update-set-default-values": {
 			server: &fwserver.Server{
 				Provider: &testprovider.Provider{},