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

Consider Refactoring tfsdk Schema/Attribute/Block Logic into internal Package(s) #365

Closed
bflad opened this issue Jun 8, 2022 · 4 comments
Assignees
Labels
tech-debt Issues tracking technical debt that we're carrying.
Milestone

Comments

@bflad
Copy link
Contributor

bflad commented Jun 8, 2022

Module version

v0.8.0

Use-cases

While working through some of the framework server refactoring as part of #215, it was necessary to extract some of the tfsdk.Attribute, tfsdk.Block, and tfsdk.Schema unexported methods into temporary functions within the internal/fwserver package so they did not become part of the public tfsdk APIs. For example, SchemaValidate() and SchemaModifyPlan(). This highlighted that a larger portion of the schema handling would benefit as being in internal packages and colocated again, rather than splitting the logic across packages, while still exporting provider developer functionality only as necessary in tfsdk or some other "public" package.

Proposal

To be written up, but a current thought is that there should be an internal/schema or similar package which contains the full framework schema/attribute/block handling, while tfsdk or another package merely exposes anything that should be in the public API.

References

@bflad bflad added tech-debt Issues tracking technical debt that we're carrying. tf-devex-triage labels Jun 8, 2022
bflad added a commit that referenced this issue Jun 22, 2022
Reference: #81
Reference: #161
Reference: #215
Reference: #365

This introduces a native abstraction over terraform-plugin-go's `tftypes.AttributePath`, allowing the framework to own the implementation details and extend the functionality further. Provider developers will be closer to removing a direct dependency on terraform-plugin-go. This is a major breaking change, however it is necessary before 1.0 to prevent compatibility issues in the future.

This functionality is only intended to replace `tftypes.AttributePath` usage and not mess with the `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` type `tftypes.Value` data storage fields or their underlying data manipulation logic. This does leave the framework in an awkward half-state until those are further refactored (likely towards native `attr.Value`), but is done to try and reduce the review complexity of this initial migration. Additional followup changes will introduce the concept of path expressions, which will allow provider developers to match against multiple paths or reference parent paths in schema-based plan modifier and validation functionality.

To prevent import cycles between `attr` and `diag` packages due changing the `attr.TypeWithValidate` type `Validate` method signature from `Validate(context.Context, tftypes.Value, *tftypes.AttributePath) diag.Diagnostics` to `Validate(context.Context, tftypes.Value, path.Path) diag.Diagnostics`, the `TypeWithValidation` interface is moved a new `xattr` package underneath `attr` with Go and website documentation to direct provider developers to the other package. This will also solve a prior issue with trying to implement `TypeWithModifyPlan` support.

Naming and location of anything in this initial implementation can be adjusted as necessary.

Provider developers can migrate to the new path handling by replacing:

```go
tftypes.NewAttributePath().WithAttributeName("example")
```

With the equivalent:

```go
path.RootPath("example")
```

Then using the `(Path).At*` methods to extend the path definition instead of `(*tftypes.AttributePath).With*` methods:

| Current                  | New             |
| ------------------------ | --------------- |
| `WithAttributeName()`    | `AtName()`      |
| `WithElementKeyInt()`    | `AtListIndex()` |
| `WithElementKeyString()` | `AtMapKey()`    |
| `WithElementKeyValue()`  | `AtSetValue()`  |
bflad added a commit that referenced this issue Jun 22, 2022
#390)

* path: Introduce package with initial tftypes.AttributePath abstraction

Reference: #81
Reference: #161
Reference: #172
Reference: #365

This introduces a native abstraction over terraform-plugin-go's `tftypes.AttributePath`, allowing the framework to own the implementation details and extend the functionality further. Provider developers will be closer to removing a direct dependency on terraform-plugin-go. This is a major breaking change, however it is necessary before 1.0 to prevent compatibility issues in the future.

This functionality is only intended to replace `tftypes.AttributePath` usage and not mess with the `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` type `tftypes.Value` data storage fields or their underlying data manipulation logic. This does leave the framework in an awkward half-state until those are further refactored (likely towards native `attr.Value`), but is done to try and reduce the review complexity of this initial migration. Additional followup changes will introduce the concept of path expressions, which will allow provider developers to match against multiple paths or reference parent paths in schema-based plan modifier and validation functionality.

To prevent import cycles between `attr` and `diag` packages due changing the `attr.TypeWithValidate` type `Validate` method signature from `Validate(context.Context, tftypes.Value, *tftypes.AttributePath) diag.Diagnostics` to `Validate(context.Context, tftypes.Value, path.Path) diag.Diagnostics`, the `TypeWithValidation` interface is moved a new `xattr` package underneath `attr` with Go and website documentation to direct provider developers to the other package. This will also solve a prior issue with trying to implement `TypeWithModifyPlan` support.

Naming and location of anything in this initial implementation can be adjusted as necessary.

Provider developers can migrate to the new path handling by replacing:

```go
tftypes.NewAttributePath().WithAttributeName("example")
```

With the equivalent:

```go
path.Root("example")
```

Then using the `(Path).At*` methods to extend the path definition instead of `(*tftypes.AttributePath).With*` methods:

| Current                  | New             |
| ------------------------ | --------------- |
| `WithAttributeName()`    | `AtName()`      |
| `WithElementKeyInt()`    | `AtListIndex()` |
| `WithElementKeyString()` | `AtMapKey()`    |
| `WithElementKeyValue()`  | `AtSetValue()`  |
bflad added a commit that referenced this issue 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. Overall the changes, other than the migration effort, should feel welcome as it will be much easier to discover related functionality and there is some reduced wordiness.

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

### Providers

| 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` |

### Data Sources

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` |

### Resources

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.

| 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 added a commit that referenced this issue 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` |
bflad added a commit that referenced this issue Aug 1, 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` |
bflad added a commit that referenced this issue Aug 2, 2022
…packages (#432)

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 added a commit that referenced this issue 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 issue 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
Copy link
Contributor Author

bflad commented Aug 3, 2022

This type of change is essentially required for #132, which provider developers are asking to be implemented.

@bflad
Copy link
Contributor Author

bflad commented Aug 8, 2022

This issue should be closable early in the v0.12.0 development cycle once we've released deprecations of some existing functionality that shouldn't affect most provider developers.

bflad added a commit that referenced this issue Aug 29, 2022
bflad added a commit that referenced this issue Aug 29, 2022
…cription

Reference: #365
Reference: #366

This change represents another evolutionary step to move away from `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` types directly, where the temporarily duplicated logic in `internal/fwserver` which worked directly with those types is replaced with the shared data implementation.
bflad added a commit that referenced this issue Aug 30, 2022
…cription (#464)

Reference: #365
Reference: #366

This change represents another evolutionary step to move away from `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` types directly, where the temporarily duplicated logic in `internal/fwserver` which worked directly with those types is replaced with the shared data implementation.
iwarapter pushed a commit to iwarapter/terraform-plugin-framework that referenced this issue Sep 4, 2022
Reference: hashicorp#31
Reference: hashicorp#132
Reference: hashicorp#223
Reference: hashicorp#326
Reference: hashicorp#365
Reference: hashicorp#389

The main goals of this change are:

- Prepare the Go module to support multiple packages that implement concept-specific schema declarations, in particular the `datasource`, `provider`, and `resource` packages
- Continue supporting the existing `tfsdk` package schema implementation with minimal provider developer breaking changes, allowing a deprecation period when the concept-specific schemas are introduced
- Migrate unexported or unintentially exported `tfsdk` schema functionality to internal packages

These goals are accomplished by creating new `internal/fwschema` and `internal/fwxschema` packages, which contain interface types that the provider developer facing packages implement. Currently, the `tfsdk` package is the only implementation, until the design of those concept-specific schema declarations is fully determined. Those designs may include changes to schema implementation details, which cannot be accomplished in the existing `tfsdk` implementation without breaking it and provider developers migrating code to the new packages is a great time to make those types of changes.

The internal interface method design here is purposefully similar to the existing `tfsdk` implementations as we cannot name the methods the same as existing fields and to reduce review burden. After the `tfsdk` implementations are removed, we can consider tidying up the interface methods to drop the `Get` and `Is` method name prefixes.

There are some minor followup changes that will happen either between or during concept-specific schema implementation work, namely:

- Swapping calls `tfsdk` package type `TerraformType()` methods for `AttributeType().TerraformType()` to reduce implementation work for new schema implementations
- Updating unit testing that relies on `tfsdk.Schema` to set up sub-tests via `(*testing.T).Run()` to against multiple implementations

These were not included here to reduce the review burden as they are separate details which can be handled later.
iwarapter pushed a commit to iwarapter/terraform-plugin-framework that referenced this issue Sep 4, 2022
…ashicorp#440)

Reference: hashicorp#365

This is a followup to the initial schema logic migration that tries to further prepare the Go module for multiple schema implementations.

- Removes unexported schema-based functions/methods from the `tfsdk` package (`pathMatches` will be handled as part of schema data internalization)
- Removes `tftypes.Type` handling from schemas as `(attr.Type).TerraformType()` already does this
- Tries to standardize on `Type()` for fetching framework type (`attr.Type`) information from schema types (`Attribute` will get fixed once `tfsdk.Attribute` is removed)
- Tries to standardize `Schema` method signatures for framework types/diagnostics and terraform-plugin-go types/errors
- Prepares `NestedAttributes` implementations for unit testing across multiple attribute implementations
@bflad bflad modified the milestones: v0.12.0, v0.13.0 Sep 6, 2022
@bflad bflad modified the milestones: v0.13.0, v1.0.0, v0.12.0 Sep 21, 2022
@bflad
Copy link
Contributor Author

bflad commented Sep 22, 2022

This internal package refactoring was completed previously. Other issues can be used to track additional efforts.

@bflad bflad closed this as completed Sep 22, 2022
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tech-debt Issues tracking technical debt that we're carrying.
Projects
None yet
Development

No branches or pull requests

1 participant