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 data source, provider, and resource types into new Go packages #432

Merged
merged 3 commits into from Aug 2, 2022

Conversation

bflad
Copy link
Member

@bflad bflad commented Jul 29, 2022

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

Since the framework's initial release, the tfsdk Go package has been a monolithic collection of various provider concepts, including but not limited to handling of data sources, providers, resources, schemas, schema data (such as Config, Plan, and State), and various other helpers. For framework maintainers, this monolithic package has made implementations difficult for certain enhancements, such as import cycles. For provider developers, this monolithic package has been difficult to navigate in the Go documentation. This change represents the first major provider coding paradigm shift to colocate related concepts into their own Go packages, starting with new top-level datasource, provider, and resource packages.

This particular change and method (no deprecation period) was not desired, but it was unavoidable due to the interconnectedness of the tfsdk package and due to the amount of effort it would require to attempt to support both the old and new types. This change is necessary to complete before there are additional code compatibility promises added to this Go module, such as a version 1.0.0 release or release candidate.

This type of change is not taken lightly, as it is quite disruptive for existing provider codebases. On the surface, this change may look fairly unbeneficial for provider developers other than the easier discoverability and reduced wordiness, however it is required to unblock other future refactoring and enhancement efforts in the framework. It is unclear at the time of writing whether splitting out the other concepts from the tfsdk package, such as schemas, will present the same issues. Regardless, any other major changes will be spread across releases.

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

To reduce framework maintainer review burden, all code migrations were lift and shift operations while most code and documentation updates were find and replace operations.

Prior tfsdk Package Type New provider Package Type
tfsdk.ConfigureProviderRequest provider.ConfigureRequest
tfsdk.ConfigureProviderResponse provider.ConfigureResponse
tfsdk.Provider provider.Provider
tfsdk.ProviderConfigValidator provider.ConfigValidator
tfsdk.ProviderWithConfigValidators provider.ProviderWithConfigValidators
tfsdk.ProviderWithProviderMeta provider.ProviderWithMetaSchema (note naming realignment)
tfsdk.ProviderWithValidateConfig provider.ProviderWithValidateConfig
tfsdk.ValidateProviderConfigRequest provider.ValidateConfigRequest
tfsdk.ValidateProviderConfigResponse provider.ValidateConfigResponse

The DataSourceType abstraction migrates to the provider package since it relates to data that must be populated before the provider is configured as well as being passed the Provider interface, which can be converted into a concrete Go type when creating a DataSource. Other data source concept types are migrated to a new datasource package.

Prior tfsdk Package Type New provider Package Type
tfsdk.DataSourceType provider.DataSourceType
Prior tfsdk Package Type New datasource Package Type
tfsdk.DataSource datasource.DataSource
tfsdk.DataSourceConfigValidator datasource.ConfigValidator
tfsdk.DataSourceWithConfigValidators datasource.DataSourceWithConfigValidators
tfsdk.DataSourceWithValidateConfig datasource.DataSourceWithValidateConfig
tfsdk.ReadDataSourceRequest datasource.ReadRequest
tfsdk.ReadDataSourceResponse datasource.ReadResponse
tfsdk.ValidateDataSourceConfigRequest datasource.ValidateConfigRequest
tfsdk.ValidateDataSourceConfigResponse datasource.ValidateConfigResponse

The ResourceType abstraction migrates to the provider package since it relates to data that must be populated before the provider is configured as well as being passed the Provider interface, which can be converted into a concrete Go type when creating a Resource. Other resource concept types are migrated to a new resource package.

Additionally, the tfsdk.ResourceImportStatePassthroughID() function has been migrated to resource.ImportStatePassthroughID().

Prior tfsdk Package Type New provider Package Type
tfsdk.ResourceType provider.ResourceType
Prior tfsdk Package Type New resource Package Type
tfsdk.CreateResourceRequest resource.CreateRequest
tfsdk.CreateResourceResponse resource.CreateResponse
tfsdk.DeleteResourceRequest resource.DeleteRequest
tfsdk.DeleteResourceResponse resource.DeleteResponse
tfsdk.ImportResourceStateRequest resource.ImportStateRequest
tfsdk.ImportResourceStateResponse resource.ImportStateResponse
tfsdk.ModifyResourcePlanRequest resource.ModifyPlanRequest
tfsdk.ModifyResourcePlanResponse resource.ModifyPlanResponse
tfsdk.ReadResourceRequest resource.ReadRequest
tfsdk.ReadResourceResponse resource.ReadResponse
tfsdk.Resource resource.Resource
tfsdk.ResourceConfigValidator resource.ConfigValidator
tfsdk.ResourceWithConfigValidators resource.ResourceWithConfigValidators
tfsdk.ResourceWithImportState resource.ResourceWithImportState
tfsdk.ResourceWithModifyPlan resource.ResourceWithModifyPlan
tfsdk.ResourceWithUpgradeState resource.ResourceWithUpgradeState
tfsdk.ResourceWithValidateConfig resource.ResourceWithValidateConfig
tfsdk.UpdateResourceRequest resource.UpdateRequest
tfsdk.UpdateResourceResponse resource.UpdateResponse
tfsdk.UpgradeResourceStateRequest resource.UpgradeStateRequest
tfsdk.UpgradeResourceStateResponse resource.UpgradeStateResponse
tfsdk.ValidateResourceConfigRequest resource.ValidateConfigRequest
tfsdk.ValidateResourceConfigResponse resource.ValidateConfigResponse

@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 Jul 29, 2022
@bflad bflad requested a review from a team as a code owner July 29, 2022 03:45
@bflad
Copy link
Member Author

bflad commented Jul 29, 2022

Go documentation for latest commit: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework@v0.10.1-0.20220729031038-a5443541a405

Real world updates to terraform-provider-corner: hashicorp/terraform-provider-corner#79
Real world updates to terraform-provider-scaffolding-framework: hashicorp/terraform-provider-scaffolding-framework#72

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.

The detailed rationale and description make sense to me.

The examples using terraform-provider-corner and terraform-provider-scaffolding-framework are also really useful to see how the changes will look.

LGTM

Copy link
Contributor

@detro detro left a comment

Choose a reason for hiding this comment

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

Reviewing every change would be impossible, so thank you so much for the summary of what happened (and why).

I like a lot the tables you provided to quickly reference "what it was" and "what's now".

Also, to seal the deal, I looked at how the new godoc would look like, and I think there is little to no discussion: a massive improvement for the developer to consult.

🚀

bflad added a commit that referenced this pull request Jul 29, 2022
…packages

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

Since the framework's initial release, the `tfsdk` Go package has been a monolithic collection of various provider concepts, including but not limited to handling of data sources, providers, resources, schemas, schema data (such as `Config`, `Plan`, and `State`), and various other helpers. For framework maintainers, this monolithic package has made implementations difficult for certain enhancements, such as import cycles. For provider developers, this monolithic package has been difficult to navigate in the Go documentation. This change represents the first major provider coding paradigm shift to colocate related concepts into their own Go packages, starting with new top-level `datasource`, `provider`, and `resource` packages.

This particular change and method (no deprecation period) was not desired, but it was unavoidable due to the interconnectedness of the `tfsdk` package and due to the amount of effort it would require to attempt to support both the old and new types. This change is necessary to complete before there are additional code compatibility promises added to this Go module, such as a version 1.0.0 release or release candidate.

This type of change is not taken lightly, as it is quite disruptive for existing provider codebases. On the surface, this change may look fairly unbeneficial for provider developers other than the easier discoverability and reduced wordiness, however it is required to unblock other future refactoring and enhancement efforts in the framework. It is unclear at the time of writing whether splitting out the other concepts from the `tfsdk` package, such as schemas, will present the same issues. Regardless, any other major changes will be spread across releases.

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

To reduce framework maintainer review burden, all code migrations were lift and shift operations while most code and documentation updates were find and replace operations.

| Prior tfsdk Package Type | New provider Package Type |
| --- | --- |
| `tfsdk.ConfigureProviderRequest` | `provider.ConfigureRequest` |
| `tfsdk.ConfigureProviderResponse` | `provider.ConfigureResponse` |
| `tfsdk.Provider` | `provider.Provider` |
| `tfsdk.ProviderConfigValidator` | `provider.ConfigValidator` |
| `tfsdk.ProviderWithConfigValidators` | `provider.ProviderWithConfigValidators` |
| `tfsdk.ProviderWithProviderMeta` | `provider.ProviderWithMetaSchema` (note naming realignment) |
| `tfsdk.ProviderWithValidateConfig` | `provider.ProviderWithValidateConfig` |
| `tfsdk.ValidateProviderConfigRequest` | `provider.ValidateConfigRequest` |
| `tfsdk.ValidateProviderConfigResponse` | `provider.ValidateConfigResponse` |

The `DataSourceType` abstraction migrates to the `provider` package since it relates to data that must be populated before the provider is configured as well as being passed the `Provider` interface, which can be converted into a concrete Go type when creating a `DataSource`. Other data source concept types are migrated to a new `datasource` package.

| Prior tfsdk Package Type | New provider Package Type |
| --- | --- |
| `tfsdk.DataSourceType` | `provider.DataSourceType` |

| Prior tfsdk Package Type | New datasource Package Type |
| --- | --- |
| `tfsdk.DataSource` | `datasource.DataSource` |
| `tfsdk.DataSourceConfigValidator` | `datasource.ConfigValidator` |
| `tfsdk.DataSourceWithConfigValidators` | `datasource.DataSourceWithConfigValidators` |
| `tfsdk.DataSourceWithValidateConfig` | `datasource.DataSourceWithValidateConfig` |
| `tfsdk.ReadDataSourceRequest` | `datasource.ReadRequest` |
| `tfsdk.ReadDataSourceResponse` | `datasource.ReadResponse` |
| `tfsdk.ValidateDataSourceConfigRequest` | `datasource.ValidateConfigRequest` |
| `tfsdk.ValidateDataSourceConfigResponse` | `datasource.ValidateConfigResponse` |

The `ResourceType` abstraction migrates to the `provider` package since it relates to data that must be populated before the provider is configured as well as being passed the `Provider` interface, which can be converted into a concrete Go type when creating a `Resource`. Other resource concept types are migrated to a new `resource` package.

Additionally, the `tfsdk.ResourceImportStatePassthroughID()` function has been migrated to `resource.ImportStatePassthroughID()`.

| Prior tfsdk Package Type | New provider Package Type |
| --- | --- |
| `tfsdk.ResourceType` | `provider.ResourceType` |

| Prior tfsdk Package Type | New resource Package Type |
| --- | --- |
| `tfsdk.CreateResourceRequest` | `resource.CreateRequest` |
| `tfsdk.CreateResourceResponse` | `resource.CreateResponse` |
| `tfsdk.DeleteResourceRequest` | `resource.DeleteRequest` |
| `tfsdk.DeleteResourceResponse` | `resource.DeleteResponse` |
| `tfsdk.ImportResourceStateRequest` | `resource.ImportStateRequest` |
| `tfsdk.ImportResourceStateResponse` | `resource.ImportStateResponse` |
| `tfsdk.ModifyResourcePlanRequest` | `resource.ModifyPlanRequest` |
| `tfsdk.ModifyResourcePlanResponse` | `resource.ModifyPlanResponse` |
| `tfsdk.ReadResourceRequest` | `resource.ReadRequest` |
| `tfsdk.ReadResourceResponse` | `resource.ReadResponse` |
| `tfsdk.Resource` | `resource.Resource` |
| `tfsdk.ResourceConfigValidator` | `resource.ConfigValidator` |
| `tfsdk.ResourceWithConfigValidators` | `resource.ResourceWithConfigValidators` |
| `tfsdk.ResourceWithImportState` | `resource.ResourceWithImportState` |
| `tfsdk.ResourceWithModifyPlan` | `resource.ResourceWithModifyPlan` |
| `tfsdk.ResourceWithUpgradeState` | `resource.ResourceWithUpgradeState` |
| `tfsdk.ResourceWithValidateConfig` | `resource.ResourceWithValidateConfig` |
| `tfsdk.UpdateResourceRequest` | `resource.UpdateRequest` |
| `tfsdk.UpdateResourceResponse` | `resource.UpdateResponse` |
| `tfsdk.UpgradeResourceStateRequest` | `resource.UpgradeStateRequest` |
| `tfsdk.UpgradeResourceStateResponse` | `resource.UpgradeStateResponse` |
| `tfsdk.ValidateResourceConfigRequest` | `resource.ValidateConfigRequest` |
| `tfsdk.ValidateResourceConfigResponse` | `resource.ValidateConfigResponse` |
@ewbankkit
Copy link
Contributor

Overall this LGTM to me 🚀.

@bflad
Copy link
Member Author

bflad commented Aug 2, 2022

Discussed with other internal stakeholders and merging so we can continue other efforts.

@bflad bflad merged commit 3b0c519 into main Aug 2, 2022
@bflad bflad deleted the bflad/dspr-refactoring branch August 2, 2022 13:10
@bflad bflad added this to the v0.11.0 milestone Aug 2, 2022
bflad added a commit that referenced this pull request Aug 2, 2022
…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 added a commit that referenced this pull request Aug 2, 2022
…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.
bflad added a commit that referenced this pull request Aug 2, 2022
bflad added a commit that referenced this pull request Aug 3, 2022
gonzolino added a commit to gonzolino/terraform-provider-tado that referenced this pull request Aug 16, 2022
gonzolino added a commit to gonzolino/terraform-provider-tado that referenced this pull request Aug 16, 2022
gonzolino added a commit to gonzolino/terraform-provider-powerdns that referenced this pull request Aug 17, 2022
ewbankkit added a commit to hashicorp/terraform-provider-aws that referenced this pull request Aug 17, 2022
ewbankkit added a commit to hashicorp/terraform-provider-aws that referenced this pull request Aug 17, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
ewbankkit added a commit to hashicorp/terraform-provider-awscc that referenced this pull request Aug 18, 2022
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

4 participants