Skip to content

Commit

Permalink
tfsdk: Migrate RequiresReplace(), RequiresReplaceIf(), and `UseSt…
Browse files Browse the repository at this point in the history
…ateForUnknown()` plan modifier functions to the `resource` package (#434)

Reference: #132
Reference: #365
Reference: #366
Reference: #432

Plan modification in the framework is a concept that only applies to managed resources via the protocol `PlanResourceChange` RPC. Following the migration of other `tfsdk` package types into separate `datasource`, `provider`, and `resource` packages, this change migrates the `RequiresReplace()`, `RequiresReplaceIf()` and `UseStateForUnknown()` plan modifier functions into the `resource` package. This change should be bundled in the same release as the package refactoring since it is similar in nature.

This change has two immediate benefits:

- Aligning the Go package placement tof these functions o hint to provider developers that these are only intended for managed resources.
- For future refactoring this removes additional schema-based code imports from the `tfsdk` package, which could enable refactoring the schema logic into internal packages with potentially less breaking changes for provider developers by removing a source of import cycles.

Provider developers should be able to update their code using find and replace operations using the table below:

| Prior tfsdk Package Function | New resource Package Function |
| --- | --- |
| `tfsdk.RequiresReplace` | `resource.RequiresReplace` |
| `tfsdk.RequiresReplaceIf` | `resource.RequiresReplaceIf` |
| `tfsdk.UseStateForUnknown` | `resource.UseStateForUnknown` |

To reduce framework maintainer review burden, this code migrations was primarily a lift and shift operation while most code and documentation updates were find and replace operations. The modifier types were unexported as exposing them should not be beneficial for provider developers and should help reduce the Go documentation surface area. The plan modifier unit testing was updated to use a `_test` package so it is verifying only exported functionality.
  • Loading branch information
bflad authored Aug 2, 2022
1 parent 5fa984e commit 51b944f
Show file tree
Hide file tree
Showing 7 changed files with 521 additions and 491 deletions.
3 changes: 3 additions & 0 deletions .changelog/434.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:breaking-change
tfsdk: The `RequiresReplace()`, `RequiresReplaceIf()`, and `UseStateForUnknown()` plan modifier functions, which only apply to managed resources, have been moved to the `resource` package.
```
25 changes: 13 additions & 12 deletions internal/fwserver/attribute_plan_modification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/internal/testing/planmodifiers"
"github.com/hashicorp/terraform-plugin-framework/internal/totftypes"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tftypes"
Expand Down Expand Up @@ -849,7 +850,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand All @@ -869,7 +870,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand All @@ -889,7 +890,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand Down Expand Up @@ -937,7 +938,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand All @@ -957,7 +958,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand All @@ -977,7 +978,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand Down Expand Up @@ -1039,7 +1040,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
planmodifiers.TestAttrPlanValueModifierOne{},
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
},
},
},
Expand All @@ -1059,7 +1060,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestAttrPlanValueModifierOne{},
},
},
Expand All @@ -1080,7 +1081,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestAttrPlanValueModifierOne{},
},
},
Expand Down Expand Up @@ -1129,7 +1130,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
},
},
Expand All @@ -1150,7 +1151,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
},
},
Expand All @@ -1171,7 +1172,7 @@ func TestAttributeModifyPlan(t *testing.T) {
Type: types.StringType,
Required: true,
PlanModifiers: []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
},
},
Expand Down
33 changes: 17 additions & 16 deletions internal/fwserver/block_plan_modification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/testing/planmodifiers"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tftypes"
Expand Down Expand Up @@ -208,7 +209,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}, nil),
modifyAttributePlanValues{
config: "newtestvalue",
Expand All @@ -221,7 +222,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("newtestvalue"),
Schema: schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}, nil),
},
RequiresReplace: path.Paths{
Expand All @@ -233,7 +234,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}, nil),
modifyAttributePlanValues{
config: "newtestvalue",
Expand All @@ -259,7 +260,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("newtestvalue"),
Schema: schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}, nil),
},
RequiresReplace: path.Paths{
Expand All @@ -271,7 +272,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
testBlockPlanModifierNullList{},
}, nil),
modifyAttributePlanValues{
Expand All @@ -285,7 +286,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaNullTfValue,
Schema: schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
testBlockPlanModifierNullList{},
}, nil),
},
Expand All @@ -298,7 +299,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
}, nil),
modifyAttributePlanValues{
Expand All @@ -312,7 +313,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("newtestvalue"),
Schema: schema([]tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
}, nil),
},
Expand Down Expand Up @@ -531,7 +532,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}),
modifyAttributePlanValues{
config: "newtestvalue",
Expand All @@ -544,7 +545,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("newtestvalue"),
Schema: schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}),
},
RequiresReplace: path.Paths{
Expand All @@ -556,7 +557,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}),
modifyAttributePlanValues{
config: "newtestvalue",
Expand All @@ -582,7 +583,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("newtestvalue"),
Schema: schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
}),
},
RequiresReplace: path.Paths{
Expand All @@ -594,7 +595,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestAttrPlanValueModifierOne{},
}),
modifyAttributePlanValues{
Expand All @@ -608,7 +609,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("TESTATTRTWO"),
Schema: schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestAttrPlanValueModifierOne{},
}),
},
Expand All @@ -621,7 +622,7 @@ func TestBlockModifyPlan(t *testing.T) {
req: modifyAttributePlanRequest(
path.Root("test"),
schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
}),
modifyAttributePlanValues{
Expand All @@ -635,7 +636,7 @@ func TestBlockModifyPlan(t *testing.T) {
Plan: tfsdk.Plan{
Raw: schemaTfValue("newtestvalue"),
Schema: schema(nil, []tfsdk.AttributePlanModifier{
tfsdk.RequiresReplace(),
resource.RequiresReplace(),
planmodifiers.TestRequiresReplaceFalseModifier{},
}),
},
Expand Down
Loading

0 comments on commit 51b944f

Please sign in to comment.