Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tfsdk: Migrate RequiresReplace(), RequiresReplaceIf(), and UseStateForUnknown() plan modifier functions to the resource package #434

Merged
merged 3 commits into from Aug 2, 2022

Conversation

bflad
Copy link
Member

@bflad bflad commented Aug 2, 2022

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 to 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.

…ateForUnknown()` plan modifier functions to the `resource` package

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.
@bflad bflad added breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. tech-debt Issues tracking technical debt that we're carrying. labels Aug 2, 2022
@bflad bflad added this to the v0.11.0 milestone Aug 2, 2022
@bflad bflad requested a review from a team as a code owner August 2, 2022 14:41
bflad added a commit to hashicorp/terraform-provider-corner that referenced this pull request Aug 2, 2022
… package

Reference: hashicorp/terraform-plugin-framework#434

Updated via:

```shell
go get github.com/hashicorp/terraform-plugin-framework@a49c78775f1b82a054a4e2a0904375f95134873e
go mod tidy
```
@bflad
Copy link
Member Author

bflad commented Aug 2, 2022

terraform-provider-corner changes: hashicorp/terraform-provider-corner#83

@bflad
Copy link
Member Author

bflad commented Aug 2, 2022

Just to explicitly say it out loud since I didn't mention in the original description, we cannot move tfsdk.AttributePlanModifier to resource.AttributePlanModifier yet until #132 happens, otherwise we'd have an import cycle.

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bflad bflad merged commit 51b944f into main Aug 2, 2022
@bflad bflad deleted the bflad/resource-plan-modifier-refactoring branch August 2, 2022 15:02
bflad added a commit to hashicorp/terraform-provider-corner that referenced this pull request Aug 2, 2022
… package (#83)

Reference: hashicorp/terraform-plugin-framework#434

Updated via:

```shell
go get github.com/hashicorp/terraform-plugin-framework@51b944f8a359e8ca29efc8d22c6843a7deedfad9
go mod tidy
```
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
@github-actions
Copy link

github-actions bot commented Sep 2, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. tech-debt Issues tracking technical debt that we're carrying.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants