Skip to content

Commit

Permalink
[Internal] Use Plugin Framework types internally in generated TF SDK …
Browse files Browse the repository at this point in the history
…structures (#4291)

## Changes
Under some circumstances, the generated TFSDK structures may crash when
trying to read the plan during create or update. This can happen
specifically when the generated structure includes a plain Go type, such
as a slice or map, and the corresponding attribute is either null or
unknown in the plan. The issue is that the plugin framework does not
have a way to represent these states in a plain slice or map, so it
simply fails.

Terraform recommends using the plugin framework types for all fields,
including lists, objects, and maps. This PR changes the generated
structures to use `types.List` and `types.Map` for all lists, objects,
and map attributes in all generated code.

Because these types do not include compile-time metadata about the type
of the contained element, the contained element types are accessible at
runtime through the addition of a `GetComplexFieldTypes()` method. This
returns a map from resource field name (specifically, the value of the
`tfsdk` tag) to the `reflect.Type` instance of the contained type. This
must be either a primitive type from the plugin framework type system or
a TF SDK struct type. In this PR, I added support for only 4 primitive
types: `types.String`, `types.Bool`, `types.Int64` and `types.Float64`.

Additional methods are also added via code generation (which accounts
for most of the lines of code in this PR). They are:
* `Type(context.Context) attr.Type` returns the Terraform type for the
current object. This is always a `basetypes.ObjectType`. It should
recursively call the same method on other TF SDK structures that are
contained in list, map, or object fields.
* `ToObjectValue(context.Context) types.ObjectValue` converts the TF SDK
object to an `types.ObjectValue` instance. This makes it simpler to
construct other `attr.Value`s, such as lists and maps, from a TF SDK
object.
* `Get...(context.Context)` and `Set...(context.Context, ...)` are
convenience getters and setters for list, map, and object fields within
your structure.

GoSdkToTfSdkStruct, TfSdkToGoSdkStruct, and ResourceStructToSchema are
all updated to handle the new structure of these TF SDK structs.

This PR does not change the default behavior of treating nested structs
& pointers to structs as list blocks. However, it does add support for
`types.Object`, so when we decide to change such fields to use this
later, it should work as expected. Additionally, support for list
attributes is not implemented here, but the change should be fairly easy
(a small tweak in `typeToSchema`).

Note: I did need to make a manual change to one autogenerated file: the
ComplianceSecurityProfile references ComplianceStandards from the
settings package. This is an enum, so it should be treated as a string,
but our current API spec doesn't provide enough metadata for our
autogeneration templates to determine this, so it is treated as an
object. This should be fixed after @renaudhartert-db's work to add
duplicated structs in the API spec to eliminate cross-package
dependencies.

## Tests
Existing tests should continue to pass. Added a test case covering
`types.Object` fields in conventions and ResourceStructToSchema.

I have been running the integration tests from a feature branch
implementing the `databricks_app` resource, which has succeeded locally.
  • Loading branch information
mgyucht authored Dec 10, 2024
1 parent 89a7a33 commit b1f0847
Show file tree
Hide file tree
Showing 44 changed files with 108,797 additions and 2,198 deletions.
7 changes: 4 additions & 3 deletions internal/providers/pluginfw/common/common_test.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
package common
package common_test

import (
"testing"

"github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/common"
"github.com/stretchr/testify/assert"
)

func TestGetDatabricksStagingName(t *testing.T) {
resourceName := "test"
expected := "databricks_test_pluginframework"
result := GetDatabricksStagingName(resourceName)
result := common.GetDatabricksStagingName(resourceName)
assert.Equal(t, expected, result, "GetDatabricksStagingName should return the expected staging name")
}

func TestGetDatabricksProductionName(t *testing.T) {
resourceName := "test"
expected := "databricks_test"
result := GetDatabricksProductionName(resourceName)
result := common.GetDatabricksProductionName(resourceName)
assert.Equal(t, expected, result, "GetDatabricksProductionName should return the expected production name")
}
27 changes: 27 additions & 0 deletions internal/providers/pluginfw/common/complex_field_type_provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package common

import (
"context"
"reflect"
)

// ComplexFieldTypeProvider must be implemented by any TFSDK structure that contains
// a complex field (list, map, object). Such fields do not include sufficient type
// information to understand the type of the contained elements in the case of a list
// or map, or the fields in the case of an object. This interface enables callers
// to recover that information.
type ComplexFieldTypeProvider interface {
// GetComplexFieldTypes returns a map from field name to the type of the value in
// the list, map or object. The keys of the map must match the value of the
// `tfsdk` tag on the field. There must be one entry in the map for each field
// that has type types.List, types.Map or types.Object.
//
// If the field has type types.List or types.Map, the reflect.Type instance may
// correspond to either a primitive value (e.g. types.String) or a TFSDK structure.
// It is not allowed to return a reflect.Type that corresponds to a type value
// (e.g. types.StringType).
//
// If the field has type types.Object, the reflect.Type instance must correspond
// to a TFSDK structure.
GetComplexFieldTypes(context.Context) map[string]reflect.Type
}
17 changes: 17 additions & 0 deletions internal/providers/pluginfw/common/diag_to_string.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package common

import (
"fmt"
"strings"

"github.com/hashicorp/terraform-plugin-framework/diag"
)

// DiagToString converts a slice of diag.Diagnostics to a string.
func DiagToString(d diag.Diagnostics) string {
b := strings.Builder{}
for _, diag := range d {
b.WriteString(fmt.Sprintf("[%s] %s: %s\n", diag.Severity(), diag.Summary(), diag.Detail()))
}
return b.String()
}
125 changes: 125 additions & 0 deletions internal/providers/pluginfw/common/object_typable.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package common

import (
"context"
"fmt"
"reflect"

"github.com/databricks/terraform-provider-databricks/common"
"github.com/databricks/terraform-provider-databricks/internal/tfreflect"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
)

// An ObjectTypable is an object that has a corresponding attr.Type.
// Note that this is different from the plugin framework's ObjectTypable interface,
// which is used to implement custom types in the plugin framework. Today, the
// serialization to plugin framework types is done in the converters package.
type ObjectTypable interface {
// Type returns the corresponding attr.Type for the object. For TF SDK types,
// this must always return an instance of basetypes.ObjectType.
Type(context.Context) attr.Type
}

type ObjectTyper struct {
// A TF SDK structure.
// If this contains types.List, types.Map, or types.Object, it must implement the
// ComplexFieldTypesProvider interface.
inner any
}

// Construct a new ObjectTyper.
// TFSDK structs automatically implement ObjectTypable, so they are returned as-is.
// Hand-written structs do not necessarily implement ObjectTypable, so this is a
// convenience implementation using reflection.
func NewObjectTyper(inner any) ObjectTypable {
if ov, ok := inner.(ObjectTypable); ok {
return ov
}
return ObjectTyper{inner: inner}
}

// Type implements basetypes.ObjectValuable.
func (o ObjectTyper) Type(ctx context.Context) attr.Type {
attrs := map[string]attr.Type{}

// Tolerate pointers.
rv := reflect.Indirect(reflect.ValueOf(o.inner))
for _, field := range tfreflect.ListAllFields(rv) {
typeField := field.StructField
fieldName := typeField.Tag.Get("tfsdk")
if fieldName == "-" {
continue
}
// If it is a simple type, we can determine the type from the reflect.Type.
if t, ok := getAttrType(field.Value); ok {
attrs[fieldName] = t
continue
}

// Otherwise, additional metadata is required to determine the type of the list elements.
// This is available via the ComplexFieldTypeProvider interface, implemented on the parent type.
provider, ok := o.inner.(ComplexFieldTypeProvider)
if !ok {
panic(fmt.Errorf("complex field types not provided for type: %T. %s", o.inner, common.TerraformBugErrorMessage))
}
complexFieldTypes := provider.GetComplexFieldTypes(ctx)
fieldType, ok := complexFieldTypes[fieldName]
if !ok {
panic(fmt.Errorf("complex field type not found for field %s on type %T. %s", typeField.Name, o.inner, common.TerraformBugErrorMessage))
}

// This is either a "simple" type or a TF SDK structure.
var innerType attr.Type
if t, ok := getAttrType(fieldType); ok {
innerType = t
} else {
// If this is a TF SDK structure, we need to recursively determine the type.
nested := reflect.New(fieldType).Elem().Interface()
ov := NewObjectTyper(nested)
innerType = ov.Type(ctx)
}

switch field.Value.Interface().(type) {
case types.List:
attrs[fieldName] = types.ListType{ElemType: innerType}
case types.Map:
attrs[fieldName] = types.MapType{ElemType: innerType}
case types.Object:
// Objects are only used for nested structures, not primitives, so we must go through
// the else case above.
innerType, ok = innerType.(basetypes.ObjectType)
if !ok {
panic(fmt.Errorf("expected ObjectType, got %T", innerType))
}
attrs[fieldName] = innerType
}
}

return basetypes.ObjectType{
AttrTypes: attrs,
}
}

var simpleTypeMap = map[reflect.Type]attr.Type{
reflect.TypeOf(types.Bool{}): types.BoolType,
reflect.TypeOf(types.Int64{}): types.Int64Type,
reflect.TypeOf(types.Float64{}): types.Float64Type,
reflect.TypeOf(types.String{}): types.StringType,
}

// getAttrType returns the attr.Type for the given value. The value can be a
// reflect.Type instance or a Terraform type instance.
func getAttrType(v any) (attr.Type, bool) {
if r, ok := v.(reflect.Type); ok {
t, ok := simpleTypeMap[r]
return t, ok
}
if rv, ok := v.(reflect.Value); ok {
t, ok := simpleTypeMap[rv.Type()]
return t, ok
}
t, ok := simpleTypeMap[reflect.TypeOf(v)]
return t, ok
}
Loading

0 comments on commit b1f0847

Please sign in to comment.