Skip to content

Commit

Permalink
[Internal] Remove TF annotations on TFSDK models (#4383)
Browse files Browse the repository at this point in the history
## Changes
<!-- Summary of your changes that are easy to understand -->
This PR changes how schemas are defined for plugin framework resources
and data sources. Instead of encoding the schema attributes in a `tf`
tag, which involves reflection and code that's harder to reason about,
structs must now implement the `ApplySchemaCustomizations` method and
declare schema customizations explicitly there. All TFSDK structs will
have this method generated for them, and if you want to declare a schema
for your own struct, you should do so as follows:

```go
// BEFORE
type TestDeprecatedTfTags struct {
	Foo types.String `tfsdk:"foo" tf:"computed,optional"`
}

// AFTER
type TestDeprecatedTfTags struct {
	Foo types.String `tfsdk:"foo"`
}

func (TestNestedMapTfSdk) ApplySchemaCustomizations(attrs map[string]AttributeBuilder) map[string]AttributeBuilder {
	attrs["foo"] = attrs["foo"].SetComputed().SetOptional()
	return attrs
}
```

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] using Go SDK
- [ ] using TF Plugin Framework

---------

Co-authored-by: Omer Lachish <rauchy@users.noreply.github.com>
  • Loading branch information
rauchy and rauchy authored Jan 10, 2025
1 parent 6011b67 commit 1411692
Show file tree
Hide file tree
Showing 58 changed files with 26,488 additions and 19,240 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ We are migrating the resource from SDKv2 to Plugin Framework provider and hence
2. Create a file with resource_resource-name.go and write the CRUD methods, schema for that resource. For reference, please take a look at existing resources eg: `resource_app.go`.
- Make sure to set the user agent in all the CRUD methods.
- In the `Metadata()`, use the method `GetDatabricksProductionName()`.
- In the `Schema()` method, import the appropriate struct from the `internal/service/{package}_tf` package and use the `ResourceStructToSchema` method to convert the struct to schema. Use the struct that does not have the `_SdkV2` suffix.
- In the `Schema()` method, import the appropriate struct from the `internal/service/{package}_tf` package and use the `ResourceStructToSchema` method to convert the struct to schema. Use the struct that does not have the `_SdkV2` suffix. The schema for the struct is automatically generated and maintained within the `ApplySchemaCustomizations` method of that struct. If you need to customize the schema further, pass in a `CustomizableSchema` to `ResourceStructToSchema` and customize the schema there. If you need to use a manually crafted struct in place of the auto-generated one, you must implement the `ApplySchemaCustomizations` method in a similar way.
3. Create a file with `resource_resource-name_acc_test.go` and add integration tests here.
4. Create a file with `resource_resource-name_test.go` and add unit tests here. Note: Please make sure to abstract specific method of the resource so they are unit test friendly and not testing internal part of terraform plugin framework library. You can compare the diagnostics, for example: please take a look at: `data_cluster_test.go`
5. Add the resource under `internal/providers/pluginfw/pluginfw.go` in `Resources()` method. Please update the list so that it stays in alphabetically sorted order.
Expand Down
46 changes: 23 additions & 23 deletions internal/providers/pluginfw/converters/converters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,28 @@ import (
)

type DummyTfSdk struct {
Enabled types.Bool `tfsdk:"enabled" tf:"optional"`
Workers types.Int64 `tfsdk:"workers" tf:""`
Floats types.Float64 `tfsdk:"floats" tf:""`
Description types.String `tfsdk:"description" tf:""`
Tasks types.String `tfsdk:"task" tf:"optional"`
NoPointerNested types.List `tfsdk:"no_pointer_nested" tf:"optional"`
NestedList types.List `tfsdk:"nested_list" tf:"optional"`
NestedPointerList types.List `tfsdk:"nested_pointer_list" tf:"optional"`
Map types.Map `tfsdk:"map" tf:"optional"`
NestedMap types.Map `tfsdk:"nested_map" tf:"optional"`
Repeated types.List `tfsdk:"repeated" tf:"optional"`
Attributes types.Map `tfsdk:"attributes" tf:"optional"`
EnumField types.String `tfsdk:"enum_field" tf:"optional"`
AdditionalField types.String `tfsdk:"additional_field" tf:"optional"`
DistinctField types.String `tfsdk:"distinct_field" tf:"optional"`
SliceStructPtr types.List `tfsdk:"slice_struct_ptr" tf:"optional"`
Enabled types.Bool `tfsdk:"enabled"`
Workers types.Int64 `tfsdk:"workers"`
Floats types.Float64 `tfsdk:"floats"`
Description types.String `tfsdk:"description"`
Tasks types.String `tfsdk:"task"`
NoPointerNested types.List `tfsdk:"no_pointer_nested"`
NestedList types.List `tfsdk:"nested_list"`
NestedPointerList types.List `tfsdk:"nested_pointer_list"`
Map types.Map `tfsdk:"map"`
NestedMap types.Map `tfsdk:"nested_map"`
Repeated types.List `tfsdk:"repeated"`
Attributes types.Map `tfsdk:"attributes"`
EnumField types.String `tfsdk:"enum_field"`
AdditionalField types.String `tfsdk:"additional_field"`
DistinctField types.String `tfsdk:"distinct_field"`
SliceStructPtr types.List `tfsdk:"slice_struct_ptr"`
Irrelevant types.String `tfsdk:"-"`
Object types.Object `tfsdk:"object" tf:"optional"`
ObjectPtr types.Object `tfsdk:"object_ptr" tf:"optional"`
Type_ types.String `tfsdk:"type" tf:""` // Test Type_ renaming
EmptyStructList types.List `tfsdk:"empty_struct_list" tf:"optional"`
EmptyStructObject types.Object `tfsdk:"empty_struct_object" tf:"optional"`
Object types.Object `tfsdk:"object"`
ObjectPtr types.Object `tfsdk:"object_ptr"`
Type_ types.String `tfsdk:"type"` // Test Type_ renaming
EmptyStructList types.List `tfsdk:"empty_struct_list"`
EmptyStructObject types.Object `tfsdk:"empty_struct_object"`
}

func (DummyTfSdk) GetComplexFieldTypes(ctx context.Context) map[string]reflect.Type {
Expand Down Expand Up @@ -84,8 +84,8 @@ func (f *TestEnum) Type() string {
}

type DummyNestedTfSdk struct {
Name types.String `tfsdk:"name" tf:"optional"`
Enabled types.Bool `tfsdk:"enabled" tf:"optional"`
Name types.String `tfsdk:"name"`
Enabled types.Bool `tfsdk:"enabled"`
}

type DummyNestedTfSdkEmpty struct{}
Expand Down
9 changes: 8 additions & 1 deletion internal/providers/pluginfw/products/app/data_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ type dataSourceApp struct {

type dataApp struct {
Name types.String `tfsdk:"name"`
App types.Object `tfsdk:"app" tf:"computed"`
App types.Object `tfsdk:"app"`
}

func (dataApp) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["name"] = attrs["name"].SetRequired()
attrs["app"] = attrs["app"].SetComputed()

return attrs
}

func (dataApp) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
8 changes: 7 additions & 1 deletion internal/providers/pluginfw/products/app/data_apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ type dataSourceApps struct {
}

type dataApps struct {
Apps types.List `tfsdk:"app" tf:"computed"`
Apps types.List `tfsdk:"app"`
}

func (dataApps) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["app"] = attrs["app"].SetComputed()

return attrs
}

func (dataApps) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
13 changes: 11 additions & 2 deletions internal/providers/pluginfw/products/catalog/data_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,17 @@ type FunctionsDataSource struct {
type FunctionsData struct {
CatalogName types.String `tfsdk:"catalog_name"`
SchemaName types.String `tfsdk:"schema_name"`
IncludeBrowse types.Bool `tfsdk:"include_browse" tf:"optional"`
Functions types.List `tfsdk:"functions" tf:"optional,computed"`
IncludeBrowse types.Bool `tfsdk:"include_browse"`
Functions types.List `tfsdk:"functions"`
}

func (FunctionsData) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["catalog_name"] = attrs["catalog_name"].SetRequired()
attrs["schema_name"] = attrs["schema_name"].SetRequired()
attrs["include_browse"] = attrs["include_browse"].SetOptional()
attrs["functions"] = attrs["functions"].SetOptional().SetComputed()

return attrs
}

func (FunctionsData) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
14 changes: 11 additions & 3 deletions internal/providers/pluginfw/products/cluster/data_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,17 @@ type ClusterDataSource struct {
}

type ClusterInfo struct {
ClusterId types.String `tfsdk:"cluster_id" tf:"optional,computed"`
Name types.String `tfsdk:"cluster_name" tf:"optional,computed"`
ClusterInfo types.List `tfsdk:"cluster_info" tf:"optional,computed"`
ClusterId types.String `tfsdk:"cluster_id"`
Name types.String `tfsdk:"cluster_name"`
ClusterInfo types.List `tfsdk:"cluster_info"`
}

func (ClusterInfo) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["cluster_id"] = attrs["cluster_id"].SetOptional().SetComputed()
attrs["cluster_name"] = attrs["cluster_name"].SetOptional().SetComputed()
attrs["cluster_info"] = attrs["cluster_info"].SetOptional().SetComputed()

return attrs
}

func (ClusterInfo) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func readLibrary(ctx context.Context, w *databricks.WorkspaceClient, waitParams
type LibraryExtended struct {
compute_tf.Library_SdkV2
ClusterId types.String `tfsdk:"cluster_id"`
ID types.String `tfsdk:"id" tf:"optional,computed"` // Adding ID field to stay compatible with SDKv2
ID types.String `tfsdk:"id"` // Adding ID field to stay compatible with SDKv2
}

type LibraryResource struct {
Expand Down Expand Up @@ -103,6 +103,8 @@ func (r *LibraryResource) Schema(ctx context.Context, req resource.SchemaRequest
c.AddPlanModifier(listplanmodifier.RequiresReplace(), field)
}
}
c.SetRequired("cluster_id")
c.SetOptional("id")
c.SetComputed("id")
return c
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,17 @@ type NotificationDestinationsDataSource struct {
}

type NotificationDestinationsInfo struct {
DisplayNameContains types.String `tfsdk:"display_name_contains" tf:"optional"`
Type types.String `tfsdk:"type" tf:"optional"`
NotificationDestinations types.List `tfsdk:"notification_destinations" tf:"computed"`
DisplayNameContains types.String `tfsdk:"display_name_contains"`
Type types.String `tfsdk:"type"`
NotificationDestinations types.List `tfsdk:"notification_destinations"`
}

func (NotificationDestinationsInfo) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["display_name_contains"] = attrs["display_name_contains"].SetOptional()
attrs["type"] = attrs["type"].SetOptional()
attrs["notification_destinations"] = attrs["notification_destinations"].SetComputed()

return attrs
}

func (NotificationDestinationsInfo) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ func waitForMonitor(ctx context.Context, w *databricks.WorkspaceClient, monitor

type MonitorInfoExtended struct {
catalog_tf.MonitorInfo_SdkV2
WarehouseId types.String `tfsdk:"warehouse_id" tf:"optional"`
SkipBuiltinDashboard types.Bool `tfsdk:"skip_builtin_dashboard" tf:"optional"`
ID types.String `tfsdk:"id" tf:"optional,computed"` // Adding ID field to stay compatible with SDKv2
WarehouseId types.String `tfsdk:"warehouse_id"`
SkipBuiltinDashboard types.Bool `tfsdk:"skip_builtin_dashboard"`
ID types.String `tfsdk:"id"` // Adding ID field to stay compatible with SDKv2
}

var _ pluginfwcommon.ComplexFieldTypeProvider = MonitorInfoExtended{}
Expand Down Expand Up @@ -87,7 +87,10 @@ func (r *QualityMonitorResource) Schema(ctx context.Context, req resource.Schema
c.SetReadOnly("status")
c.SetReadOnly("dashboard_id")
c.SetReadOnly("schedule", "pause_status")
c.SetOptional("warehouse_id")
c.SetOptional("skip_builtin_dashboard")
c.SetComputed("id")
c.SetOptional("id")
return c
})
resp.Schema = schema.Schema{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,17 @@ type RegisteredModelDataSource struct {

type RegisteredModelData struct {
FullName types.String `tfsdk:"full_name"`
IncludeAliases types.Bool `tfsdk:"include_aliases" tf:"optional"`
IncludeBrowse types.Bool `tfsdk:"include_browse" tf:"optional"`
ModelInfo types.List `tfsdk:"model_info" tf:"optional,computed"`
IncludeAliases types.Bool `tfsdk:"include_aliases"`
IncludeBrowse types.Bool `tfsdk:"include_browse"`
ModelInfo types.List `tfsdk:"model_info"`
}

func (RegisteredModelData) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["full_name"] = attrs["full_name"].SetRequired()
attrs["include_aliases"] = attrs["include_aliases"].SetOptional()
attrs["include_browse"] = attrs["include_browse"].SetOptional()
attrs["model_info"] = attrs["model_info"].SetOptional().SetComputed()
return attrs
}

func (RegisteredModelData) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ type RegisteredModelVersionsDataSource struct {

type RegisteredModelVersionsData struct {
FullName types.String `tfsdk:"full_name"`
ModelVersions types.List `tfsdk:"model_versions" tf:"optional,computed"`
ModelVersions types.List `tfsdk:"model_versions"`
}

func (RegisteredModelVersionsData) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["full_name"] = attrs["full_name"].SetRequired()
attrs["model_versions"] = attrs["model_versions"].SetOptional().SetComputed()
return attrs
}

func (RegisteredModelVersionsData) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ type ServingEndpointsDataSource struct {
}

type ServingEndpointsData struct {
Endpoints types.List `tfsdk:"endpoints" tf:"optional,computed"`
Endpoints types.List `tfsdk:"endpoints"`
}

func (ServingEndpointsData) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["endpoints"] = attrs["endpoints"].SetOptional().SetComputed()

return attrs
}

func (ServingEndpointsData) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
8 changes: 7 additions & 1 deletion internal/providers/pluginfw/products/sharing/data_shares.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import (
const dataSourceNameShares = "shares"

type SharesList struct {
Shares types.List `tfsdk:"shares" tf:"computed,optional,slice_set"`
Shares types.List `tfsdk:"shares"`
}

func (SharesList) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["shares"] = attrs["shares"].SetComputed().SetOptional()

return attrs
}

func (SharesList) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ func (r *ShareResource) Schema(ctx context.Context, req resource.SchemaRequest,
c.SetRequired("object", "data_object_type")
c.SetRequired("object", "partition", "value", "op")
c.SetRequired("object", "partition", "value", "name")
c.SetOptional("owner")

return c
})
Expand Down
9 changes: 8 additions & 1 deletion internal/providers/pluginfw/products/volume/data_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@ type VolumesDataSource struct {
type VolumesList struct {
CatalogName types.String `tfsdk:"catalog_name"`
SchemaName types.String `tfsdk:"schema_name"`
Ids types.List `tfsdk:"ids" tf:"optional,computed"`
Ids types.List `tfsdk:"ids"`
}

func (VolumesList) ApplySchemaCustomizations(attrs map[string]tfschema.AttributeBuilder) map[string]tfschema.AttributeBuilder {
attrs["catalog_name"] = attrs["catalog_name"].SetRequired()
attrs["schema_name"] = attrs["schema_name"].SetRequired()
attrs["ids"] = attrs["ids"].SetOptional().SetComputed()
return attrs
}

func (VolumesList) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand Down
19 changes: 13 additions & 6 deletions internal/providers/pluginfw/tfschema/customizable_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"reflect"
"testing"

"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
Expand All @@ -15,10 +16,16 @@ import (
)

type TestTfSdk struct {
Description types.String `tfsdk:"description" tf:""`
Nested types.List `tfsdk:"nested" tf:"optional"`
NestedSliceObject types.List `tfsdk:"nested_slice_object" tf:"optional,object"`
Map types.Map `tfsdk:"map" tf:"optional"`
Description types.String `tfsdk:"description"`
Nested types.List `tfsdk:"nested"`
NestedSliceObject types.List `tfsdk:"nested_slice_object"`
Map types.Map `tfsdk:"map"`
}

func (TestTfSdk) ApplySchemaCustomizations(attrs map[string]AttributeBuilder) map[string]AttributeBuilder {
attrs["nested_slice_object"] = attrs["nested_slice_object"].(ListNestedAttributeBuilder).AddValidator(listvalidator.SizeAtMost(1)).(AttributeBuilder)

return attrs
}

func (TestTfSdk) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
Expand All @@ -30,8 +37,8 @@ func (TestTfSdk) GetComplexFieldTypes(context.Context) map[string]reflect.Type {
}

type NestedTfSdk struct {
Name types.String `tfsdk:"name" tf:"optional"`
Enabled types.Bool `tfsdk:"enabled" tf:"optional"`
Name types.String `tfsdk:"name"`
Enabled types.Bool `tfsdk:"enabled"`
}

type stringLengthBetweenValidator struct {
Expand Down
Loading

0 comments on commit 1411692

Please sign in to comment.