Skip to content

Commit

Permalink
tfsdk: Migrate schema handling to internal interfaces (#438)
Browse files Browse the repository at this point in the history
Reference: #31
Reference: #132
Reference: #223
Reference: #326
Reference: #365
Reference: #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.
  • Loading branch information
bflad committed Aug 5, 2022
1 parent 882780b commit e6bc60e
Show file tree
Hide file tree
Showing 110 changed files with 1,702 additions and 1,086 deletions.
3 changes: 3 additions & 0 deletions .changelog/438.txt
@@ -0,0 +1,3 @@
```release-note:breaking-change
tfsdk: The `Schema` type `AttributeAtPath` method now returns a `fwschema.Attribute` interface instead of a `tfsdk.Attribute` type. Consumers will need to update from direct field usage to similarly named interface method calls.
```
6 changes: 3 additions & 3 deletions internal/fromproto5/applyresourcechange.go
Expand Up @@ -4,15 +4,15 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
)

// ApplyResourceChangeRequest returns the *fwserver.ApplyResourceChangeRequest
// equivalent of a *tfprotov5.ApplyResourceChangeRequest.
func ApplyResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.ApplyResourceChangeRequest, resourceType provider.ResourceType, resourceSchema *tfsdk.Schema, providerMetaSchema *tfsdk.Schema) (*fwserver.ApplyResourceChangeRequest, diag.Diagnostics) {
func ApplyResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.ApplyResourceChangeRequest, resourceType provider.ResourceType, resourceSchema fwschema.Schema, providerMetaSchema fwschema.Schema) (*fwserver.ApplyResourceChangeRequest, diag.Diagnostics) {
if proto5 == nil {
return nil, nil
}
Expand All @@ -35,7 +35,7 @@ func ApplyResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.ApplyReso

fw := &fwserver.ApplyResourceChangeRequest{
PlannedPrivate: proto5.PlannedPrivate,
ResourceSchema: *resourceSchema,
ResourceSchema: resourceSchema,
ResourceType: resourceType,
}

Expand Down
19 changes: 10 additions & 9 deletions internal/fromproto5/applyresourcechange_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
Expand Down Expand Up @@ -45,9 +46,9 @@ func TestApplyResourceChangeRequest(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.ApplyResourceChangeRequest
resourceSchema *tfsdk.Schema
resourceSchema fwschema.Schema
resourceType provider.ResourceType
providerMetaSchema *tfsdk.Schema
providerMetaSchema fwschema.Schema
expected *fwserver.ApplyResourceChangeRequest
expectedDiagnostics diag.Diagnostics
}{
Expand Down Expand Up @@ -93,7 +94,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"plannedstate-missing-schema": {
Expand Down Expand Up @@ -121,7 +122,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"plannedprivate": {
Expand All @@ -131,7 +132,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
resourceSchema: testFwSchema,
expected: &fwserver.ApplyResourceChangeRequest{
PlannedPrivate: []byte("{}"),
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"priorstate-missing-schema": {
Expand Down Expand Up @@ -159,7 +160,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"providermeta-missing-data": {
Expand All @@ -171,7 +172,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
Raw: tftypes.NewValue(testProto5Type, nil),
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"providermeta-missing-schema": {
Expand All @@ -181,7 +182,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
resourceSchema: testFwSchema,
expected: &fwserver.ApplyResourceChangeRequest{
// This intentionally should not include ProviderMeta
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"providermeta": {
Expand All @@ -195,7 +196,7 @@ func TestApplyResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
}
Expand Down
7 changes: 4 additions & 3 deletions internal/fromproto5/config.go
Expand Up @@ -4,13 +4,14 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
)

// Config returns the *tfsdk.Config for a *tfprotov5.DynamicValue and
// *tfsdk.Schema.
func Config(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, schema *tfsdk.Schema) (*tfsdk.Config, diag.Diagnostics) {
// fwschema.Schema.
func Config(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, schema fwschema.Schema) (*tfsdk.Config, diag.Diagnostics) {
if proto5DynamicValue == nil {
return nil, nil
}
Expand Down Expand Up @@ -46,7 +47,7 @@ func Config(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, sch

fw := &tfsdk.Config{
Raw: proto5Value,
Schema: *schema,
Schema: tfsdkSchema(schema),
}

return fw, nil
Expand Down
3 changes: 2 additions & 1 deletion internal/fromproto5/config_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
Expand Down Expand Up @@ -52,7 +53,7 @@ func TestConfig(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.DynamicValue
schema *tfsdk.Schema
schema fwschema.Schema
expected *tfsdk.Config
expectedDiagnostics diag.Diagnostics
}{
Expand Down
4 changes: 2 additions & 2 deletions internal/fromproto5/configureprovider.go
Expand Up @@ -4,14 +4,14 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
)

// ConfigureProviderRequest returns the *fwserver.ConfigureProviderRequest
// equivalent of a *tfprotov5.ConfigureProviderRequest.
func ConfigureProviderRequest(ctx context.Context, proto5 *tfprotov5.ConfigureProviderRequest, providerSchema *tfsdk.Schema) (*provider.ConfigureRequest, diag.Diagnostics) {
func ConfigureProviderRequest(ctx context.Context, proto5 *tfprotov5.ConfigureProviderRequest, providerSchema fwschema.Schema) (*provider.ConfigureRequest, diag.Diagnostics) {
if proto5 == nil {
return nil, nil
}
Expand Down
3 changes: 2 additions & 1 deletion internal/fromproto5/configureprovider_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
Expand Down Expand Up @@ -44,7 +45,7 @@ func TestConfigureProviderRequest(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.ConfigureProviderRequest
providerSchema *tfsdk.Schema
providerSchema fwschema.Schema
expected *provider.ConfigureRequest
expectedDiagnostics diag.Diagnostics
}{
Expand Down
5 changes: 3 additions & 2 deletions internal/fromproto5/importresourcestate.go
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
Expand All @@ -13,7 +14,7 @@ import (

// ImportResourceStateRequest returns the *fwserver.ImportResourceStateRequest
// equivalent of a *tfprotov5.ImportResourceStateRequest.
func ImportResourceStateRequest(ctx context.Context, proto5 *tfprotov5.ImportResourceStateRequest, resourceType provider.ResourceType, resourceSchema *tfsdk.Schema) (*fwserver.ImportResourceStateRequest, diag.Diagnostics) {
func ImportResourceStateRequest(ctx context.Context, proto5 *tfprotov5.ImportResourceStateRequest, resourceType provider.ResourceType, resourceSchema fwschema.Schema) (*fwserver.ImportResourceStateRequest, diag.Diagnostics) {
if proto5 == nil {
return nil, nil
}
Expand All @@ -37,7 +38,7 @@ func ImportResourceStateRequest(ctx context.Context, proto5 *tfprotov5.ImportRes
fw := &fwserver.ImportResourceStateRequest{
EmptyState: tfsdk.State{
Raw: tftypes.NewValue(resourceSchema.TerraformType(ctx), nil),
Schema: *resourceSchema,
Schema: tfsdkSchema(resourceSchema),
},
ID: proto5.ID,
ResourceType: resourceType,
Expand Down
3 changes: 2 additions & 1 deletion internal/fromproto5/importresourcestate_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
Expand Down Expand Up @@ -34,7 +35,7 @@ func TestImportResourceStateRequest(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.ImportResourceStateRequest
resourceSchema *tfsdk.Schema
resourceSchema fwschema.Schema
resourceType provider.ResourceType
expected *fwserver.ImportResourceStateRequest
expectedDiagnostics diag.Diagnostics
Expand Down
7 changes: 4 additions & 3 deletions internal/fromproto5/plan.go
Expand Up @@ -4,13 +4,14 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
)

// Plan returns the *tfsdk.Plan for a *tfprotov5.DynamicValue and
// *tfsdk.Schema.
func Plan(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, schema *tfsdk.Schema) (*tfsdk.Plan, diag.Diagnostics) {
// fwschema.Schema.
func Plan(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, schema fwschema.Schema) (*tfsdk.Plan, diag.Diagnostics) {
if proto5DynamicValue == nil {
return nil, nil
}
Expand Down Expand Up @@ -46,7 +47,7 @@ func Plan(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, schem

fw := &tfsdk.Plan{
Raw: proto5Value,
Schema: *schema,
Schema: tfsdkSchema(schema),
}

return fw, nil
Expand Down
3 changes: 2 additions & 1 deletion internal/fromproto5/plan_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
Expand Down Expand Up @@ -52,7 +53,7 @@ func TestPlan(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.DynamicValue
schema *tfsdk.Schema
schema fwschema.Schema
expected *tfsdk.Plan
expectedDiagnostics diag.Diagnostics
}{
Expand Down
6 changes: 3 additions & 3 deletions internal/fromproto5/planresourcechange.go
Expand Up @@ -4,15 +4,15 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
)

// PlanResourceChangeRequest returns the *fwserver.PlanResourceChangeRequest
// equivalent of a *tfprotov5.PlanResourceChangeRequest.
func PlanResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.PlanResourceChangeRequest, resourceType provider.ResourceType, resourceSchema *tfsdk.Schema, providerMetaSchema *tfsdk.Schema) (*fwserver.PlanResourceChangeRequest, diag.Diagnostics) {
func PlanResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.PlanResourceChangeRequest, resourceType provider.ResourceType, resourceSchema fwschema.Schema, providerMetaSchema fwschema.Schema) (*fwserver.PlanResourceChangeRequest, diag.Diagnostics) {
if proto5 == nil {
return nil, nil
}
Expand All @@ -35,7 +35,7 @@ func PlanResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.PlanResour

fw := &fwserver.PlanResourceChangeRequest{
PriorPrivate: proto5.PriorPrivate,
ResourceSchema: *resourceSchema,
ResourceSchema: resourceSchema,
ResourceType: resourceType,
}

Expand Down
19 changes: 10 additions & 9 deletions internal/fromproto5/planresourcechange_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
Expand Down Expand Up @@ -45,9 +46,9 @@ func TestPlanResourceChangeRequest(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.PlanResourceChangeRequest
resourceSchema *tfsdk.Schema
resourceSchema fwschema.Schema
resourceType provider.ResourceType
providerMetaSchema *tfsdk.Schema
providerMetaSchema fwschema.Schema
expected *fwserver.PlanResourceChangeRequest
expectedDiagnostics diag.Diagnostics
}{
Expand Down Expand Up @@ -93,7 +94,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"priorprivate": {
Expand All @@ -103,7 +104,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
resourceSchema: testFwSchema,
expected: &fwserver.PlanResourceChangeRequest{
PriorPrivate: []byte("{}"),
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"priorstate-missing-schema": {
Expand Down Expand Up @@ -131,7 +132,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"proposednewstate-missing-schema": {
Expand Down Expand Up @@ -159,7 +160,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"providermeta-missing-data": {
Expand All @@ -171,7 +172,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
Raw: tftypes.NewValue(testProto5Type, nil),
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"providermeta-missing-schema": {
Expand All @@ -181,7 +182,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
resourceSchema: testFwSchema,
expected: &fwserver.PlanResourceChangeRequest{
// This intentionally should not include ProviderMeta
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
"providermeta": {
Expand All @@ -195,7 +196,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
Raw: testProto5Value,
Schema: *testFwSchema,
},
ResourceSchema: *testFwSchema,
ResourceSchema: testFwSchema,
},
},
}
Expand Down

0 comments on commit e6bc60e

Please sign in to comment.