diff --git a/.changes/unreleased/BUG FIXES-20230210-092136.yaml b/.changes/unreleased/BUG FIXES-20230210-092136.yaml new file mode 100644 index 000000000..9d237ac30 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20230210-092136.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'resource: Prevented nested attribute and block plan modifications from being undone' +time: 2023-02-10T09:21:36.848573-05:00 +custom: + Issue: "669" diff --git a/internal/fwserver/attribute_plan_modification.go b/internal/fwserver/attribute_plan_modification.go index 1afbf2c9a..01d876a2e 100644 --- a/internal/fwserver/attribute_plan_modification.go +++ b/internal/fwserver/attribute_plan_modification.go @@ -135,7 +135,9 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req ModifyAt return } - planList, diags := coerceListValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with list + // plan modifiers. + planList, diags := coerceListValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) @@ -220,7 +222,9 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req ModifyAt return } - planSet, diags := coerceSetValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with set + // plan modifiers. + planSet, diags := coerceSetValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) @@ -305,7 +309,9 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req ModifyAt return } - planMap, diags := coerceMapValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with map + // plan modifiers. + planMap, diags := coerceMapValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) @@ -390,7 +396,9 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req ModifyAt return } - planObject, diags := coerceObjectValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with object + // plan modifiers. + planObject, diags := coerceObjectValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) diff --git a/internal/fwserver/attribute_plan_modification_test.go b/internal/fwserver/attribute_plan_modification_test.go index 6c206e63e..de4a9aec3 100644 --- a/internal/fwserver/attribute_plan_modification_test.go +++ b/internal/fwserver/attribute_plan_modification_test.go @@ -20,7 +20,11 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/testing/testschema" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/mapplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" @@ -216,6 +220,75 @@ func TestAttributeModifyPlan(t *testing.T) { Private: testProviderData, }, }, + "attribute-list-nested-usestateforunknown": { + attribute: testschema.NestedAttributeWithListPlanModifiers{ + NestedObject: testschema.NestedAttributeObject{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + }, + Computed: true, + PlanModifiers: []planmodifier.List{ + listplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.ListNull( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.ListUnknown( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributeState: types.ListValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.ListValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + }, "attribute-set-nested-private": { attribute: testschema.NestedAttributeWithSetPlanModifiers{ NestedObject: testschema.NestedAttributeObject{ @@ -310,6 +383,75 @@ func TestAttributeModifyPlan(t *testing.T) { }, }, "attribute-set-nested-usestateforunknown": { + attribute: testschema.NestedAttributeWithSetPlanModifiers{ + NestedObject: testschema.NestedAttributeObject{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + }, + Computed: true, + PlanModifiers: []planmodifier.Set{ + setplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.SetNull( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.SetUnknown( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributeState: types.SetValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.SetValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + }, + "attribute-set-nested-nested-usestateforunknown": { attribute: testschema.NestedAttribute{ NestedObject: testschema.NestedAttributeObject{ Attributes: map[string]fwschema.Attribute{ @@ -547,6 +689,75 @@ func TestAttributeModifyPlan(t *testing.T) { Private: testProviderData, }, }, + "attribute-map-nested-usestateforunknown": { + attribute: testschema.NestedAttributeWithMapPlanModifiers{ + NestedObject: testschema.NestedAttributeObject{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + }, + Computed: true, + PlanModifiers: []planmodifier.Map{ + mapplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.MapNull( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.MapUnknown( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributeState: types.MapValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + map[string]attr.Value{ + "key1": types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.MapValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + map[string]attr.Value{ + "key1": types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + }, "attribute-single-nested-private": { attribute: testschema.NestedAttributeWithObjectPlanModifiers{ NestedObject: testschema.NestedAttributeObject{ @@ -604,6 +815,53 @@ func TestAttributeModifyPlan(t *testing.T) { Private: testProviderData, }, }, + "attribute-single-nested-usestateforunknown": { + attribute: testschema.NestedAttributeWithObjectPlanModifiers{ + NestedObject: testschema.NestedAttributeObject{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + }, + Computed: true, + PlanModifiers: []planmodifier.Object{ + objectplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.ObjectNull( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.ObjectUnknown( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + ), + AttributeState: types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + }, "requires-replacement": { attribute: testschema.AttributeWithStringPlanModifiers{ Required: true, diff --git a/internal/fwserver/block_plan_modification.go b/internal/fwserver/block_plan_modification.go index 91dafc524..5ff5955d9 100644 --- a/internal/fwserver/block_plan_modification.go +++ b/internal/fwserver/block_plan_modification.go @@ -55,7 +55,9 @@ func BlockModifyPlan(ctx context.Context, b fwschema.Block, req ModifyAttributeP return } - planList, diags := coerceListValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with list + // plan modifiers. + planList, diags := coerceListValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) @@ -140,7 +142,9 @@ func BlockModifyPlan(ctx context.Context, b fwschema.Block, req ModifyAttributeP return } - planSet, diags := coerceSetValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with set + // plan modifiers. + planSet, diags := coerceSetValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) @@ -225,7 +229,9 @@ func BlockModifyPlan(ctx context.Context, b fwschema.Block, req ModifyAttributeP return } - planObject, diags := coerceObjectValue(ctx, req.AttributePath, req.AttributePlan) + // Use response as the planned value may have been modified with object + // plan modifiers. + planObject, diags := coerceObjectValue(ctx, req.AttributePath, resp.AttributePlan) resp.Diagnostics.Append(diags...) diff --git a/internal/fwserver/block_plan_modification_test.go b/internal/fwserver/block_plan_modification_test.go index e0477622c..5b58e2a82 100644 --- a/internal/fwserver/block_plan_modification_test.go +++ b/internal/fwserver/block_plan_modification_test.go @@ -17,7 +17,10 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/testing/testplanmodifier" "github.com/hashicorp/terraform-plugin-framework/internal/testing/testschema" "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" @@ -578,6 +581,72 @@ func TestBlockModifyPlan(t *testing.T) { Private: testProviderData, }, }, + "block-list-usestateforunknown": { + block: testschema.BlockWithListPlanModifiers{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + PlanModifiers: []planmodifier.List{ + listplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.ListNull( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.ListUnknown( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributeState: types.ListValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.ListValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + }, "block-set-null-plan": { block: testschema.BlockWithSetPlanModifiers{ Attributes: map[string]fwschema.Attribute{ @@ -1215,6 +1284,72 @@ func TestBlockModifyPlan(t *testing.T) { ), }, }, + "block-set-usestateforunknown": { + block: testschema.BlockWithSetPlanModifiers{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + PlanModifiers: []planmodifier.Set{ + setplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.SetNull( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.SetUnknown( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + ), + AttributeState: types.SetValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.SetValueMust( + types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "nested_computed": types.StringType, + }, + }, + []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + ), + }, + }, "block-single-null-plan": { block: testschema.BlockWithObjectPlanModifiers{ Attributes: map[string]fwschema.Attribute{ @@ -1432,6 +1567,50 @@ func TestBlockModifyPlan(t *testing.T) { ), }, }, + "block-single-usestateforunknown": { + block: testschema.BlockWithObjectPlanModifiers{ + Attributes: map[string]fwschema.Attribute{ + "nested_computed": testschema.Attribute{ + Type: types.StringType, + Computed: true, + }, + }, + PlanModifiers: []planmodifier.Object{ + objectplanmodifier.UseStateForUnknown(), + }, + }, + req: ModifyAttributePlanRequest{ + AttributeConfig: types.ObjectNull( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + ), + AttributePath: path.Root("test"), + AttributePlan: types.ObjectUnknown( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + ), + AttributeState: types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + expectedResp: ModifyAttributePlanResponse{ + AttributePlan: types.ObjectValueMust( + map[string]attr.Type{ + "nested_computed": types.StringType, + }, + map[string]attr.Value{ + "nested_computed": types.StringValue("statevalue1"), + }, + ), + }, + }, "block-requires-replacement": { block: testschema.BlockWithListPlanModifiers{ Attributes: map[string]fwschema.Attribute{