diff --git a/.changes/unreleased/BUG FIXES-20250219-152752.yaml b/.changes/unreleased/BUG FIXES-20250219-152752.yaml new file mode 100644 index 000000000..bbddefb0a --- /dev/null +++ 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 index bd81126f5..52b06688b 100644 --- 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 index 282e223ed..f1607daeb 100644 --- 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{},