diff --git a/.changes/unreleased/ENHANCEMENTS-20240722-175116.yaml b/.changes/unreleased/ENHANCEMENTS-20240722-175116.yaml new file mode 100644 index 000000000..d700cd106 --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20240722-175116.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: 'all: Added embedded struct support for object to struct conversions with `tfsdk` + tags' +time: 2024-07-22T17:51:16.833264-04:00 +custom: + Issue: "1021" diff --git a/.changes/unreleased/NOTES-20240801-171654.yaml b/.changes/unreleased/NOTES-20240801-171654.yaml new file mode 100644 index 000000000..f75f016d1 --- /dev/null +++ b/.changes/unreleased/NOTES-20240801-171654.yaml @@ -0,0 +1,40 @@ +kind: NOTES +body: | + Framework reflection logic (`Config.Get`, `Plan.Get`, etc.) for structs with + `tfsdk` field tags has been updated to support embedded structs that promote exported + fields. For existing structs that embed unexported structs with exported fields, a tfsdk + ignore tag (``tfsdk:"-"``) can be added to ignore all promoted fields. + + For example, the following struct will now return an error diagnostic: + ```go + type thingResourceModel struct { + Attr1 types.String `tfsdk:"attr_1"` + Attr2 types.Bool `tfsdk:"attr_2"` + + // Previously, this embedded struct was ignored, will now promote underlying fields + embeddedModel + } + + type embeddedModel struct { + // No `tfsdk` tag + ExportedField string + } + ``` + + To preserve the original behavior, a tfsdk ignore tag can be added to ignore the entire embedded struct: + ```go + type thingResourceModel struct { + Attr1 types.String `tfsdk:"attr_1"` + Attr2 types.Bool `tfsdk:"attr_2"` + + // This embedded struct will now be ignored + embeddedModel `tfsdk:"-"` + } + + type embeddedModel struct { + ExportedField string + } + ``` +time: 2024-08-01T17:16:54.043432-04:00 +custom: + Issue: "1021" diff --git a/internal/reflect/helpers.go b/internal/reflect/helpers.go index 52634dfb6..675fa8df8 100644 --- a/internal/reflect/helpers.go +++ b/internal/reflect/helpers.go @@ -46,36 +46,87 @@ func commaSeparatedString(in []string) string { } // getStructTags returns a map of Terraform field names to their position in -// the tags of the struct `in`. `in` must be a struct. -func getStructTags(_ context.Context, in reflect.Value, path path.Path) (map[string]int, error) { - tags := map[string]int{} - typ := trueReflectValue(in).Type() +// the fields of the struct `in`. `in` must be a struct. +// +// The position of the field in a struct is represented as an index sequence to support type embedding +// in structs. This index sequence can be used to retrieve the field with the Go "reflect" package FieldByIndex methods: +// - https://pkg.go.dev/reflect#Type.FieldByIndex +// - https://pkg.go.dev/reflect#Value.FieldByIndex +// - https://pkg.go.dev/reflect#Value.FieldByIndexErr +// +// The following are not supported and will return an error if detected in a struct (including embedded structs): +// - Duplicate "tfsdk" tags +// - Exported fields without a "tfsdk" tag +// - Exported fields with an invalid "tfsdk" tag (must be a valid Terraform identifier) +func getStructTags(ctx context.Context, typ reflect.Type, path path.Path) (map[string][]int, error) { //nolint:unparam // False positive, ctx is used below. + tags := make(map[string][]int, 0) + + if typ.Kind() == reflect.Pointer { + typ = typ.Elem() + } if typ.Kind() != reflect.Struct { - return nil, fmt.Errorf("%s: can't get struct tags of %s, is not a struct", path, in.Type()) + return nil, fmt.Errorf("%s: can't get struct tags of %s, is not a struct", path, typ) } + for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) - if field.PkgPath != "" { - // skip unexported fields + if !field.IsExported() && !field.Anonymous { + // Skip unexported fields. Unexported embedded structs (anonymous fields) are allowed because they may + // contain exported fields that are promoted; which means they can be read/set. continue } - tag := field.Tag.Get(`tfsdk`) + + // This index sequence is the location of the field within the struct. + // For promoted fields from an embedded struct, the length of this sequence will be > 1 + fieldIndexSequence := []int{i} + tag, tagExists := field.Tag.Lookup(`tfsdk`) + + // "tfsdk" tags with "-" are being explicitly excluded if tag == "-" { - // skip explicitly excluded fields continue } - if tag == "" { + + // Handle embedded structs + if field.Anonymous { + if tagExists { + return nil, fmt.Errorf(`%s: embedded struct field %s cannot have tfsdk tag`, path.AtName(tag), field.Name) + } + + embeddedTags, err := getStructTags(ctx, field.Type, path) + if err != nil { + return nil, fmt.Errorf(`error retrieving embedded struct %q field tags: %w`, field.Name, err) + } + for k, v := range embeddedTags { + if other, ok := tags[k]; ok { + otherField := typ.FieldByIndex(other) + return nil, fmt.Errorf("embedded struct %q promotes a field with a duplicate tfsdk tag %q, conflicts with %q tfsdk tag", field.Name, k, otherField.Name) + } + + tags[k] = append(fieldIndexSequence, v...) + } + continue + } + + // All non-embedded fields must have a tfsdk tag + if !tagExists { return nil, fmt.Errorf(`%s: need a struct tag for "tfsdk" on %s`, path, field.Name) } + + // Ensure the tfsdk tag has a valid name path := path.AtName(tag) if !isValidFieldName(tag) { - return nil, fmt.Errorf("%s: invalid field name, must only use lowercase letters, underscores, and numbers, and must start with a letter", path) + return nil, fmt.Errorf("%s: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter", path) } + + // Ensure there are no duplicate tfsdk tags if other, ok := tags[tag]; ok { - return nil, fmt.Errorf("%s: can't use field name for both %s and %s", path, typ.Field(other).Name, field.Name) + otherField := typ.FieldByIndex(other) + return nil, fmt.Errorf("%s: can't use tfsdk tag %q for both %s and %s fields", path, tag, otherField.Name, field.Name) } - tags[tag] = i + + tags[tag] = fieldIndexSequence } + return tags, nil } diff --git a/internal/reflect/helpers_test.go b/internal/reflect/helpers_test.go index 63abba23e..ce6f35763 100644 --- a/internal/reflect/helpers_test.go +++ b/internal/reflect/helpers_test.go @@ -4,11 +4,265 @@ package reflect import ( + "context" + "errors" "fmt" "reflect" + "strings" "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/path" ) +type ExampleStruct struct { + StrField string `tfsdk:"str_field"` + IntField int `tfsdk:"int_field"` + BoolField bool `tfsdk:"bool_field"` + IgnoreMe string `tfsdk:"-"` + + unexported string //nolint:structcheck,unused + unexportedAndTagged string `tfsdk:"unexported_and_tagged"` +} + +type NestedEmbed struct { + ListField []string `tfsdk:"list_field"` + DoubleNestedEmbed +} + +type DoubleNestedEmbed struct { + Map map[string]string `tfsdk:"map_field"` + ExampleStruct +} + +type EmbedWithDuplicates struct { + StrField1 string `tfsdk:"str_field"` + StrField2 string `tfsdk:"str_field"` +} + +type StructWithInvalidTag struct { + InvalidField string `tfsdk:"*()-"` +} + +func TestGetStructTags(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + in any + expectedTags map[string][]int + expectedErr error + }{ + "struct": { + in: ExampleStruct{}, + expectedTags: map[string][]int{ + "str_field": {0}, + "int_field": {1}, + "bool_field": {2}, + }, + }, + "struct-err-duplicate-fields": { + in: struct { + StrField string `tfsdk:"str_field"` + IntField string `tfsdk:"str_field"` + }{}, + expectedErr: errors.New(`str_field: can't use tfsdk tag "str_field" for both StrField and IntField fields`), + }, + "embedded-struct-err-duplicate-fields": { + in: struct { + EmbedWithDuplicates + }{}, + expectedErr: errors.New(`error retrieving embedded struct "EmbedWithDuplicates" field tags: str_field: can't use tfsdk tag "str_field" for both StrField1 and StrField2 fields`), + }, + "embedded-struct-err-duplicate-fields-from-promote": { + in: struct { + StrField string `tfsdk:"str_field"` + ExampleStruct // Contains a `tfsdk:"str_field"` + }{}, + expectedErr: errors.New(`embedded struct "ExampleStruct" promotes a field with a duplicate tfsdk tag "str_field", conflicts with "StrField" tfsdk tag`), + }, + "struct-err-invalid-field": { + in: StructWithInvalidTag{}, + expectedErr: errors.New(`*()-: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter`), + }, + "struct-err-missing-tfsdk-tag": { + in: struct { + ExampleField string + }{}, + expectedErr: errors.New(`: need a struct tag for "tfsdk" on ExampleField`), + }, + "struct-err-empty-tfsdk-tag": { + in: struct { + ExampleField string `tfsdk:""` + }{}, + expectedErr: errors.New(`: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter`), + }, + "ignore-embedded-struct": { + in: struct { + ExampleStruct `tfsdk:"-"` + Field5 string `tfsdk:"field5"` + }{}, + expectedTags: map[string][]int{ + "field5": {1}, + }, + }, + "embedded-struct": { + in: struct { + ExampleStruct + Field5 string `tfsdk:"field5"` + }{}, + expectedTags: map[string][]int{ + "str_field": {0, 0}, + "int_field": {0, 1}, + "bool_field": {0, 2}, + "field5": {1}, + }, + }, + "nested-embedded-struct": { + in: struct { + NestedEmbed + Field5 string `tfsdk:"field5"` + }{}, + expectedTags: map[string][]int{ + "list_field": {0, 0}, + "map_field": {0, 1, 0}, + "str_field": {0, 1, 1, 0}, + "int_field": {0, 1, 1, 1}, + "bool_field": {0, 1, 1, 2}, + "field5": {1}, + }, + }, + "embedded-struct-unexported": { + in: struct { + ExampleStruct + Field5 string `tfsdk:"field5"` + + unexported string //nolint:structcheck,unused + unexportedAndTagged string `tfsdk:"unexported_and_tagged"` + }{}, + expectedTags: map[string][]int{ + "str_field": {0, 0}, + "int_field": {0, 1}, + "bool_field": {0, 2}, + "field5": {1}, + }, + }, + "embedded-struct-err-cannot-have-empty-tfsdk-tag": { + in: struct { + ExampleStruct `tfsdk:""` // Can't put a tfsdk tag here + }{}, + expectedErr: errors.New(`: embedded struct field ExampleStruct cannot have tfsdk tag`), + }, + "embedded-struct-err-cannot-have-tfsdk-tag": { + in: struct { + ExampleStruct `tfsdk:"example_field"` // Can't put a tfsdk tag here + }{}, + expectedErr: errors.New(`example_field: embedded struct field ExampleStruct cannot have tfsdk tag`), + }, + "embedded-struct-err-invalid": { + in: struct { + StructWithInvalidTag // Contains an invalid "tfsdk" tag + }{}, + expectedErr: errors.New(`error retrieving embedded struct "StructWithInvalidTag" field tags: *()-: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter`), + }, + // NOTE: The following tests are for embedded struct pointers, despite them not being explicitly supported by the framework reflect package. + // Embedded struct pointers still produce a valid field index, but are later rejected when retrieving them. These tests just ensure that there + // are no panics when retrieving the field index for an embedded struct pointer field + "ignore-embedded-struct-ptr": { + in: struct { + *ExampleStruct `tfsdk:"-"` + Field5 string `tfsdk:"field5"` + }{}, + expectedTags: map[string][]int{ + "field5": {1}, + }, + }, + "embedded-struct-ptr": { + in: struct { + *ExampleStruct + Field5 string `tfsdk:"field5"` + }{}, + expectedTags: map[string][]int{ + "str_field": {0, 0}, + "int_field": {0, 1}, + "bool_field": {0, 2}, + "field5": {1}, + }, + }, + "embedded-struct-ptr-unexported": { + in: struct { + *ExampleStruct + Field5 string `tfsdk:"field5"` + + unexported string //nolint:structcheck,unused + unexportedAndTagged string `tfsdk:"unexported_and_tagged"` + }{}, + expectedTags: map[string][]int{ + "str_field": {0, 0}, + "int_field": {0, 1}, + "bool_field": {0, 2}, + "field5": {1}, + }, + }, + "embedded-struct-ptr-err-cannot-have-empty-tfsdk-tag": { + in: struct { + *ExampleStruct `tfsdk:""` // Can't put a tfsdk tag here + }{}, + expectedErr: errors.New(`: embedded struct field ExampleStruct cannot have tfsdk tag`), + }, + "embedded-struct-ptr-err-cannot-have-tfsdk-tag": { + in: struct { + *ExampleStruct `tfsdk:"example_field"` // Can't put a tfsdk tag here + }{}, + expectedErr: errors.New(`example_field: embedded struct field ExampleStruct cannot have tfsdk tag`), + }, + "embedded-struct-ptr-err-duplicate-fields": { + in: struct { + *EmbedWithDuplicates + }{}, + expectedErr: errors.New(`error retrieving embedded struct "EmbedWithDuplicates" field tags: str_field: can't use tfsdk tag "str_field" for both StrField1 and StrField2 fields`), + }, + "embedded-struct-ptr-err-duplicate-fields-from-promote": { + in: struct { + StrField string `tfsdk:"str_field"` + *ExampleStruct // Contains a `tfsdk:"str_field"` + }{}, + expectedErr: errors.New(`embedded struct "ExampleStruct" promotes a field with a duplicate tfsdk tag "str_field", conflicts with "StrField" tfsdk tag`), + }, + "embedded-struct-ptr-err-invalid": { + in: struct { + *StructWithInvalidTag // Contains an invalid "tfsdk" tag + }{}, + expectedErr: errors.New(`error retrieving embedded struct "StructWithInvalidTag" field tags: *()-: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter`), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + tags, err := getStructTags(context.Background(), reflect.TypeOf(testCase.in), path.Empty()) + if err != nil { + if testCase.expectedErr == nil { + t.Fatalf("expected no error, got: %s", err) + } + + if !strings.Contains(err.Error(), testCase.expectedErr.Error()) { + t.Fatalf("expected error %q, got: %s", testCase.expectedErr, err) + } + } + + if err == nil && testCase.expectedErr != nil { + t.Fatalf("got no error, expected: %s", testCase.expectedErr) + } + + if diff := cmp.Diff(tags, testCase.expectedTags); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + func TestTrueReflectValue(t *testing.T) { t.Parallel() diff --git a/internal/reflect/struct.go b/internal/reflect/struct.go index d48ff2d31..7e1198d0e 100644 --- a/internal/reflect/struct.go +++ b/internal/reflect/struct.go @@ -75,7 +75,7 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref // collect a map of fields that are defined in the tags of the struct // passed in - targetFields, err := getStructTags(ctx, target, path) + targetFields, err := getStructTags(ctx, target.Type(), path) if err != nil { diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, @@ -120,7 +120,7 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref // now that we know they match perfectly, fill the struct with the // values in the object result := reflect.New(target.Type()).Elem() - for field, structFieldPos := range targetFields { + for field, fieldIndex := range targetFields { attrType, ok := attrTypes[field] if !ok { diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ @@ -130,7 +130,33 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref })) return target, diags } - structField := result.Field(structFieldPos) + + structField, err := result.FieldByIndexErr(fieldIndex) + if err != nil { + if len(fieldIndex) > 1 { + // The field index that triggered the error is in an embedded struct. The most likely cause for the error + // is because the embedded struct is a pointer, which we explicitly don't support. We'll create a more tailored + // error message to nudge provider developers to use a value embedded struct. + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ + Val: object, + TargetType: target.Type(), + Err: fmt.Errorf( + "%s contains a struct embedded by a pointer which is not supported. Switch any embedded structs to be embedded by value.\n\nError: %s", + target.Type(), + err, + ), + })) + return target, diags + } + + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ + Val: object, + TargetType: target.Type(), + Err: fmt.Errorf("error retrieving field index %v in struct %s: %w", fieldIndex, target.Type(), err), + })) + return target, diags + } + fieldVal, fieldValDiags := BuildValue(ctx, attrType, objectFields[field], structField, opts, path.AtName(field)) diags.Append(fieldValDiags...) @@ -156,7 +182,7 @@ func FromStruct(ctx context.Context, typ attr.TypeWithAttributeTypes, val reflec // collect a map of fields that are defined in the tags of the struct // passed in - targetFields, err := getStructTags(ctx, val, path) + targetFields, err := getStructTags(ctx, val.Type(), path) if err != nil { err = fmt.Errorf("error retrieving field names from struct tags: %w", err) diags.AddAttributeError( @@ -211,9 +237,26 @@ func FromStruct(ctx context.Context, typ attr.TypeWithAttributeTypes, val reflec return nil, diags } - for name, fieldNo := range targetFields { + for name, fieldIndex := range targetFields { path := path.AtName(name) - fieldValue := val.Field(fieldNo) + fieldValue, err := val.FieldByIndexErr(fieldIndex) + if err != nil { + if len(fieldIndex) > 1 { + // The field index that triggered the error is in an embedded struct. The most likely cause for the error + // is because the embedded struct is a pointer, which we explicitly don't support. We'll create a more tailored + // error message to nudge provider developers to use a value embedded struct. + err = fmt.Errorf("%s contains a struct embedded by a pointer which is not supported. Switch any embedded structs to be embedded by value.\n\nError: %s", val.Type(), err) + } else { + err = fmt.Errorf("error retrieving field index %v in struct %s: %w", fieldIndex, val.Type(), err) + } + + diags.AddAttributeError( + path, + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return nil, diags + } // If the attr implements xattr.ValidateableAttribute, or xattr.TypeWithValidate, // and the attr does not validate then diagnostics will be added here and returned diff --git a/internal/reflect/struct_test.go b/internal/reflect/struct_test.go index 7d9836a3b..a79df6e24 100644 --- a/internal/reflect/struct_test.go +++ b/internal/reflect/struct_test.go @@ -22,6 +22,24 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) +type EmbedSingleField struct { + Attr1 types.String `tfsdk:"attr_1"` +} + +type embedSingleField struct { + Attr1 types.String `tfsdk:"attr_1"` +} + +type embedNoFields struct{} + +type embedWithUntaggedExportedField struct { + ExportedAndUntagged types.String +} + +type embedWithInvalidTag struct { + A types.String `tfsdk:"invalidTag"` +} + func TestNewStruct_errors(t *testing.T) { t.Parallel() @@ -62,6 +80,14 @@ func TestNewStruct_errors(t *testing.T) { }{}), expectedError: errors.New("mismatch between struct and object: Struct defines fields not found in object: a."), }, + "embedded-object-missing-fields": { + typ: types.ObjectType{}, + objVal: tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}), + targetVal: reflect.ValueOf(struct { + embedSingleField + }{}), + expectedError: errors.New("mismatch between struct and object: Struct defines fields not found in object: attr_1."), + }, "struct-missing-fields": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -78,12 +104,30 @@ func TestNewStruct_errors(t *testing.T) { targetVal: reflect.ValueOf(struct{}{}), expectedError: errors.New("mismatch between struct and object: Object defines fields not found in struct: a."), }, - "object-and-struct-missing-fields": { + "embedded-struct-missing-fields": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ "a": types.StringType, }, }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "a": tftypes.String, + }, + }, map[string]tftypes.Value{ + "a": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + embedNoFields + }{}), + expectedError: errors.New("mismatch between struct and object: Object defines fields not found in struct: a."), + }, + "object-and-struct-missing-fields": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "b": types.StringType, + }, + }, objVal: tftypes.NewValue(tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "b": tftypes.String, @@ -96,6 +140,24 @@ func TestNewStruct_errors(t *testing.T) { }{}), expectedError: errors.New("mismatch between struct and object: Struct defines fields not found in object: a. Object defines fields not found in struct: b."), }, + "embedded-object-and-struct-missing-fields": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "b": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "b": tftypes.String, + }, + }, map[string]tftypes.Value{ + "b": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + embedSingleField + }{}), + expectedError: errors.New("mismatch between struct and object: Struct defines fields not found in object: attr_1. Object defines fields not found in struct: b."), + }, "struct-has-untagged-fields": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -117,6 +179,30 @@ func TestNewStruct_errors(t *testing.T) { "error retrieving field names from struct tags: %w", errors.New(`: need a struct tag for "tfsdk" on ExportedAndUntagged`)), }, + "embedded-struct-has-untagged-fields": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "a": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "a": tftypes.String, + }, + }, map[string]tftypes.Value{ + "a": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + A string `tfsdk:"a"` + embedWithUntaggedExportedField + }{}), + expectedError: fmt.Errorf( + "error retrieving field names from struct tags: %w", + fmt.Errorf(`error retrieving embedded struct "embedWithUntaggedExportedField" field tags: %w`, + errors.New(`: need a struct tag for "tfsdk" on ExportedAndUntagged`), + ), + ), + }, "struct-has-invalid-tags": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -135,7 +221,50 @@ func TestNewStruct_errors(t *testing.T) { }{}), expectedError: fmt.Errorf( "error retrieving field names from struct tags: %w", - errors.New("invalidTag: invalid field name, must only use lowercase letters, underscores, and numbers, and must start with a letter")), + errors.New("invalidTag: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter")), + }, + "struct-has-empty-tag": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "a": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "a": tftypes.String, + }, + }, map[string]tftypes.Value{ + "a": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + A string `tfsdk:""` + }{}), + expectedError: fmt.Errorf( + "error retrieving field names from struct tags: %w", + errors.New(": invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter")), + }, + "embedded-struct-has-invalid-tags": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "a": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "a": tftypes.String, + }, + }, map[string]tftypes.Value{ + "a": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + embedWithInvalidTag + }{}), + expectedError: fmt.Errorf( + "error retrieving field names from struct tags: %w", + fmt.Errorf(`error retrieving embedded struct "embedWithInvalidTag" field tags: %w`, + errors.New("invalidTag: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter"), + ), + ), }, "struct-has-duplicate-tags": { typ: types.ObjectType{ @@ -156,7 +285,112 @@ func TestNewStruct_errors(t *testing.T) { }{}), expectedError: fmt.Errorf( "error retrieving field names from struct tags: %w", - errors.New("a: can't use field name for both A and B")), + errors.New(`a: can't use tfsdk tag "a" for both A and B fields`)), + }, + "embedded-struct-has-duplicate-tags": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "a": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "a": tftypes.String, + }, + }, map[string]tftypes.Value{ + "a": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + FirstAttr1 types.String `tfsdk:"attr_1"` + // Also contains an attr_1 tfsdk tag + embedSingleField + }{}), + expectedError: fmt.Errorf( + "error retrieving field names from struct tags: %w", + errors.New(`embedded struct "embedSingleField" promotes a field with a duplicate tfsdk tag "attr_1", conflicts with "FirstAttr1" tfsdk tag`), + ), + }, + "embedded-exported-struct-with-pointer-not-supported": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "attr_1": tftypes.String, + }, + }, map[string]tftypes.Value{ + "attr_1": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + *EmbedSingleField + }{}), + expectedError: errors.New( + "struct { *reflect_test.EmbedSingleField } contains a struct embedded by a pointer which is not supported. Switch any embedded structs to be embedded by value.\n\n" + + "Error: reflect: indirection through nil pointer to embedded struct field EmbedSingleField", + ), + }, + "embedded-unexported-struct-with-pointer-not-supported": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "attr_1": tftypes.String, + }, + }, map[string]tftypes.Value{ + "attr_1": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + *embedSingleField + }{}), + expectedError: errors.New( + "struct { *reflect_test.embedSingleField } contains a struct embedded by a pointer which is not supported. Switch any embedded structs to be embedded by value.\n\n" + + "Error: reflect: indirection through nil pointer to embedded struct field embedSingleField", + ), + }, + "embedded-struct-has-empty-tfsdk-tag": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "attr_1": tftypes.String, + }, + }, map[string]tftypes.Value{ + "attr_1": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + embedSingleField `tfsdk:""` + }{}), + expectedError: errors.New( + "error retrieving field names from struct tags: : embedded struct field embedSingleField cannot have tfsdk tag", + ), + }, + "embedded-struct-has-tfsdk-tag": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + objVal: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "attr_1": tftypes.String, + }, + }, map[string]tftypes.Value{ + "attr_1": tftypes.NewValue(tftypes.String, "hello"), + }), + targetVal: reflect.ValueOf(struct { + embedSingleField `tfsdk:"attr_2"` + }{}), + expectedError: errors.New( + "error retrieving field names from struct tags: attr_2: embedded struct field embedSingleField cannot have tfsdk tag", + ), }, } @@ -226,6 +460,54 @@ func TestNewStruct_primitives(t *testing.T) { } } +func TestNewStruct_embedded_primitives(t *testing.T) { + t.Parallel() + + type S3 struct { + C bool `tfsdk:"c"` + } + type s2 struct { + B *big.Float `tfsdk:"b"` + S3 + } + type s1 struct { + A string `tfsdk:"a"` + s2 + } + var s s1 + + result, diags := refl.Struct(context.Background(), types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "a": types.StringType, + "b": types.NumberType, + "c": types.BoolType, + }, + }, tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "a": tftypes.String, + "b": tftypes.Number, + "c": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + "a": tftypes.NewValue(tftypes.String, "hello"), + "b": tftypes.NewValue(tftypes.Number, 123), + "c": tftypes.NewValue(tftypes.Bool, true), + }), reflect.ValueOf(s), refl.Options{}, path.Empty()) + if diags.HasError() { + t.Errorf("Unexpected error: %v", diags) + } + reflect.ValueOf(&s).Elem().Set(result) + if s.A != "hello" { + t.Errorf("Expected s.A to be %q, was %q", "hello", s.A) + } + if s.B.Cmp(big.NewFloat(123)) != 0 { + t.Errorf("Expected s.B to be %v, was %v", big.NewFloat(123), s.B) + } + if s.C != true { + t.Errorf("Expected s.C to be %v, was %v", true, s.C) + } +} + func TestNewStruct_complex(t *testing.T) { t.Parallel() @@ -565,6 +847,56 @@ func TestNewStruct_structtags_ignores(t *testing.T) { } } +func TestNewStruct_embedded_structtags_ignores(t *testing.T) { + t.Parallel() + + type s1 struct { + ExportedAndTagged string `tfsdk:"exported_and_tagged"` + unexported string //nolint:structcheck,unused + unexportedAndTagged string `tfsdk:"unexported_and_tagged"` + ExportedAndExcluded string `tfsdk:"-"` + } + + type s2 struct { + unexportedField string //nolint:structcheck,unused + } + + var s struct { + s1 + *s2 `tfsdk:"-"` + } + result, diags := refl.Struct(context.Background(), types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "exported_and_tagged": types.StringType, + }, + }, tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "exported_and_tagged": tftypes.String, + }, + }, map[string]tftypes.Value{ + "exported_and_tagged": tftypes.NewValue(tftypes.String, "hello"), + }), reflect.ValueOf(s), refl.Options{}, path.Empty()) + if diags.HasError() { + t.Errorf("Unexpected error: %v", diags) + } + reflect.ValueOf(&s).Elem().Set(result) + if s.ExportedAndTagged != "hello" { + t.Errorf("Expected s.ExportedAndTagged to be %q, was %q", "hello", s.ExportedAndTagged) + } + + if s.unexported != "" { + t.Errorf("Expected s.unexported to be empty, was %q", s.unexported) + } + + if s.unexportedAndTagged != "" { + t.Errorf("Expected s.unexportedAndTagged to be empty, was %q", s.unexportedAndTagged) + } + + if s.ExportedAndExcluded != "" { + t.Errorf("Expected s.ExportedAndExcluded to be empty, was %q", s.ExportedAndExcluded) + } +} + func TestFromStruct_primitives(t *testing.T) { t.Parallel() @@ -608,10 +940,65 @@ func TestFromStruct_primitives(t *testing.T) { } } -func TestFromStruct_complex(t *testing.T) { +func TestFromStruct_embedded_primitives(t *testing.T) { t.Parallel() - type myStruct struct { + type embedFields struct { + Name string `tfsdk:"name"` + Age int `tfsdk:"age"` + } + + type EmbedFields struct { + OptedIn bool `tfsdk:"opted_in"` + } + + type disk struct { + embedFields + EmbedFields + } + disk1 := disk{ + embedFields{ + Name: "myfirstdisk", + Age: 30, + }, + EmbedFields{ + OptedIn: true, + }, + } + + actualVal, diags := refl.FromStruct(context.Background(), types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "name": types.StringType, + "age": types.NumberType, + "opted_in": types.BoolType, + }, + }, reflect.ValueOf(disk1), path.Empty()) + if diags.HasError() { + t.Fatalf("Unexpected error: %v", diags) + } + + expectedVal := types.ObjectValueMust( + map[string]attr.Type{ + "name": types.StringType, + "age": types.NumberType, + "opted_in": types.BoolType, + }, + map[string]attr.Value{ + "name": types.StringValue("myfirstdisk"), + "age": types.NumberValue(big.NewFloat(30)), + "opted_in": types.BoolValue(true), + }, + ) + + if diff := cmp.Diff(expectedVal, actualVal); diff != "" { + t.Errorf("Unexpected diff (+wanted, -got): %s", diff) + } +} + +func TestFromStruct_complex(t *testing.T) { + t.Parallel() + + type myStruct struct { ListSlice []string `tfsdk:"list_slice"` ListSliceOfStructs []struct { A string `tfsdk:"a"` @@ -970,6 +1357,29 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-field-mismatch": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "test": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct into an object. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "Mismatch between struct and object type: Struct defines fields not found in object: attr_1. Object defines fields not found in struct: test.\n"+ + `Struct: struct { reflect_test.embedSingleField }`+"\n"+ + `Object type: types.ObjectType["test":basetypes.StringType]`, + ), + }, + }, "struct-type-mismatch": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -993,6 +1403,29 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-type-mismatch": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.BoolType, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField // intentionally not types.Bool + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test").AtName("attr_1"), + "Value Conversion Error", + "An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "Expected framework type from provider logic: basetypes.BoolType / underlying type: tftypes.Bool\n"+ + "Received framework type from provider logic: basetypes.StringType / underlying type: tftypes.String\n"+ + "Path: test.attr_1", + ), + }, + }, "struct-validate-error": { typ: testtypes.ObjectTypeWithValidateError{ ObjectType: types.ObjectType{ @@ -1014,6 +1447,27 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-validate-error": { + typ: testtypes.ObjectTypeWithValidateError{ + ObjectType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Error Diagnostic", + "This is an error.", + ), + }, + }, "struct-validate-attribute-error": { typ: testtypes.ObjectTypeWithValidateAttributeError{ ObjectType: types.ObjectType{ @@ -1035,6 +1489,27 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-validate-attribute-error": { + typ: testtypes.ObjectTypeWithValidateAttributeError{ + ObjectType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Error Diagnostic", + "This is an error.", + ), + }, + }, "struct-validate-warning": { typ: testtypes.ObjectTypeWithValidateWarning{ ObjectType: types.ObjectType{ @@ -1066,6 +1541,39 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-validate-warning": { + typ: testtypes.ObjectTypeWithValidateWarning{ + ObjectType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField + }{ + embedSingleField{ + Attr1: types.StringValue("test"), + }, + }, + ), + expected: types.ObjectValueMust( + map[string]attr.Type{ + "attr_1": types.StringType, + }, + map[string]attr.Value{ + "attr_1": types.StringValue("test"), + }, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeWarningDiagnostic( + path.Root("test"), + "Warning Diagnostic", + "This is a warning.", + ), + }, + }, "struct-validate-attribute-warning": { typ: testtypes.ObjectTypeWithValidateAttributeWarning{ ObjectType: types.ObjectType{ @@ -1099,6 +1607,41 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-validate-attribute-warning": { + typ: testtypes.ObjectTypeWithValidateAttributeWarning{ + ObjectType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField + }{ + embedSingleField: embedSingleField{ + Attr1: types.StringValue("test"), + }, + }, + ), + expected: testtypes.ObjectValueWithValidateAttributeWarning{ + Object: types.ObjectValueMust( + map[string]attr.Type{ + "attr_1": types.StringType, + }, + map[string]attr.Value{ + "attr_1": types.StringValue("test"), + }, + ), + }, + expectedDiags: diag.Diagnostics{ + diag.NewAttributeWarningDiagnostic( + path.Root("test"), + "Warning Diagnostic", + "This is a warning.", + ), + }, + }, "struct-field-validate-error": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -1118,6 +1661,25 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-field-validate-error": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": testtypes.StringTypeWithValidateError{}, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test").AtName("attr_1"), + "Error Diagnostic", + "This is an error.", + ), + }, + }, "struct-field-validate-attribute-error": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -1237,6 +1799,29 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-struct-has-untagged-fields": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "test": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + Test types.String `tfsdk:"test"` + embedWithUntaggedExportedField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "error retrieving field names from struct tags: error retrieving embedded struct \"embedWithUntaggedExportedField\" field tags: "+ + "test: need a struct tag for \"tfsdk\" on ExportedAndUntagged", + ), + }, + }, "struct-has-invalid-tags": { typ: types.ObjectType{ AttrTypes: map[string]attr.Type{ @@ -1254,7 +1839,50 @@ func TestFromStruct_errors(t *testing.T) { "Value Conversion Error", "An unexpected error was encountered trying to convert from struct value. "+ "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - "error retrieving field names from struct tags: test.invalidTag: invalid field name, must only use lowercase letters, underscores, and numbers, and must start with a letter", + "error retrieving field names from struct tags: test.invalidTag: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter", + ), + }, + }, + "struct-has-empty-tag": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "test": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + Test types.String `tfsdk:""` + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "error retrieving field names from struct tags: test.: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter", + ), + }, + }, + "embedded-struct-has-invalid-tags": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "test": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + embedWithInvalidTag + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "error retrieving field names from struct tags: error retrieving embedded struct \"embedWithInvalidTag\" field tags: "+ + "test.invalidTag: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter", ), }, }, @@ -1276,7 +1904,29 @@ func TestFromStruct_errors(t *testing.T) { "Value Conversion Error", "An unexpected error was encountered trying to convert from struct value. "+ "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - "error retrieving field names from struct tags: test.test: can't use field name for both Test and Test2", + "error retrieving field names from struct tags: test.test: can't use tfsdk tag \"test\" for both Test and Test2 fields", + ), + }, + }, + "embedded-struct-has-duplicate-tags": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + Attr1 types.String `tfsdk:"attr_1"` + embedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "error retrieving field names from struct tags: embedded struct \"embedSingleField\" promotes a field with a duplicate tfsdk tag \"attr_1\", conflicts with \"Attr1\" tfsdk tag", ), }, }, @@ -1383,6 +2033,92 @@ func TestFromStruct_errors(t *testing.T) { ), }, }, + "embedded-exported-struct-with-pointer-not-supported": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + *EmbedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test").AtName("attr_1"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "struct { *reflect_test.EmbedSingleField } contains a struct embedded by a pointer which is not supported. Switch any embedded structs to be embedded by value.\n\n"+ + "Error: reflect: indirection through nil pointer to embedded struct field EmbedSingleField", + ), + }, + }, + "embedded-unexported-struct-with-pointer-not-supported": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + *embedSingleField + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test").AtName("attr_1"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "struct { *reflect_test.embedSingleField } contains a struct embedded by a pointer which is not supported. Switch any embedded structs to be embedded by value.\n\n"+ + "Error: reflect: indirection through nil pointer to embedded struct field embedSingleField", + ), + }, + }, + "embedded-struct-has-empty-tfsdk-tag": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField `tfsdk:""` + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "error retrieving field names from struct tags: test.: embedded struct field embedSingleField cannot have tfsdk tag", + ), + }, + }, + "embedded-struct-has-tfsdk-tag": { + typ: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "attr_1": types.StringType, + }, + }, + val: reflect.ValueOf( + struct { + embedSingleField `tfsdk:"attr_2"` + }{}, + ), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from struct value. "+ + "This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "error retrieving field names from struct tags: test.attr_2: embedded struct field embedSingleField cannot have tfsdk tag", + ), + }, + }, } for name, testCase := range testCases { @@ -1450,3 +2186,54 @@ func TestFromStruct_structtags_ignores(t *testing.T) { t.Errorf("Unexpected diff (+wanted, -got): %s", diff) } } + +func TestFromStruct_embedded_structtags_ignores(t *testing.T) { + t.Parallel() + + type s1 struct { + ExportedAndTagged string `tfsdk:"exported_and_tagged"` + unexported string //nolint:structcheck,unused + unexportedAndTagged string `tfsdk:"unexported_and_tagged"` + ExportedAndExcluded string `tfsdk:"-"` + } + + type s2 struct { + unexportedField string //nolint:structcheck,unused + } + + type s struct { + s1 + *s2 `tfsdk:"-"` + } + testStruct := s{ + s1{ + ExportedAndTagged: "thisshouldstay", + unexported: "shouldntcopy", + unexportedAndTagged: "shouldntcopy", + ExportedAndExcluded: "shouldntcopy", + }, + &s2{}, + } + + actualVal, diags := refl.FromStruct(context.Background(), types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "exported_and_tagged": types.StringType, + }, + }, reflect.ValueOf(testStruct), path.Empty()) + if diags.HasError() { + t.Fatalf("Unexpected error: %v", diags) + } + + expectedVal := types.ObjectValueMust( + map[string]attr.Type{ + "exported_and_tagged": types.StringType, + }, + map[string]attr.Value{ + "exported_and_tagged": types.StringValue("thisshouldstay"), + }, + ) + + if diff := cmp.Diff(expectedVal, actualVal); diff != "" { + t.Errorf("Unexpected diff (+wanted, -got): %s", diff) + } +} diff --git a/website/docs/plugin/framework/handling-data/types/object.mdx b/website/docs/plugin/framework/handling-data/types/object.mdx index 5c02451a9..3c62b2e0b 100644 --- a/website/docs/plugin/framework/handling-data/types/object.mdx +++ b/website/docs/plugin/framework/handling-data/types/object.mdx @@ -121,10 +121,12 @@ Otherwise, for certain framework functionality that does not require `types` imp * [`types.ObjectValueFrom()`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/types#ObjectValueFrom) * [`types.SetValueFrom()`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/types#SetValueFrom) -Objects can be automatically converted to any Go struct type that follows these constraints to prevent accidental data loss: +## Automatic conversion with `tfsdk` struct tags +Objects can be automatically converted to and from any Go struct type that follows these constraints to prevent accidental data loss: + +* Every struct field must have a `tfsdk` struct tag and every attribute in the object must have a corresponding struct tag. The `tfsdk` struct tag must name an attribute in the object that it is being mapped or be set to `-` to explicitly declare it does not map to an attribute in the object. Duplicate `tfsdk` struct tags are not allowed. * Every struct type must be an acceptable conversion type according to the type documentation, such as `*string` being acceptable for a string type. However, it is recommended to use framework types to simplify data modeling (one model type for accessing and setting data) and prevent errors when encountering unknown values from Terraform. -* Every struct field must have a `tfsdk` struct tag and every attribute in the object must have a corresponding struct tag. The `tfsdk` struct tag must name an attribute in the object that it is being mapped or be set to `-` to explicitly declare it does not map to an attribute in the object. In this example, a struct is directly used to set an object attribute value: @@ -165,6 +167,77 @@ value := ExampleAttributeModel{ objectValue, diags := types.ObjectValueFrom(ctx, value.AttributeTypes(), value) ``` +### Struct Embedding + +Go struct types that utilize [struct embedding](https://go.dev/doc/effective_go#embedding) to promote fields with `tfsdk` tags are supported when converting to and from object types. + +In this example, a `types.Object` is created from a struct that embeds another struct type: + +```go +type ExampleAttributeModel struct { + Attr1 types.String `tfsdk:"attr_1"` + Attr2 types.Bool `tfsdk:"attr_2"` + CommonModel // This embedded struct type promotes the Attr3 field +} + +type CommonModel struct { + Attr3 types.Int64 `tfsdk:"attr_3"` +} + +func (m ExampleAttributeModel) AttributeTypes() map[string]attr.Type { + return map[string]attr.Type{ + "attr_1": types.StringType, + "attr_2": types.BoolType, + "attr_3": types.Int64Type, + } +} + +value := ExampleAttributeModel{ + Attr1: types.StringValue("example"), + Attr2: types.BoolValue(true), +} +// This field is promoted from CommonModel +value.Attr3 = types.Int64Value(123) + +objectValue, diags := types.ObjectValueFrom(ctx, value.AttributeTypes(), value) +``` + +#### Restrictions + +In addition to the constraints [listed above for object conversions](#automatic-conversion-with-tfsdk-struct-tags) using `tfsdk` tagged fields, embedded struct types have these additional restrictions: + +* Promoted fields cannot have duplicate `tfsdk` struct tags that conflict with any fields of structs they are embedded within. +```go +type thingResourceModel struct { + Attr1 types.String `tfsdk:"attr_1"` + Attr2 types.Bool `tfsdk:"attr_2"` + CommonModel +} + +type CommonModel struct { + // This field has a duplicate tfsdk tag, conflicting with (thingResourceModel).Attr1 + // and will raise an error diagnostic during conversion. + ConflictingAttr types.Int64 `tfsdk:"attr_1"` +} +``` +* Struct types embedded by pointers are not supported. +```go +type thingResourceModel struct { + Attr1 types.String `tfsdk:"attr_1"` + Attr2 types.Bool `tfsdk:"attr_2"` + // This type of struct embedding is not supported and will raise + // an error diagnostic during conversion. + // + // Remove the pointer to embed the struct by value. + *CommonModel +} + +type CommonModel struct { + Attr3 types.Int64 `tfsdk:"attr_3"` + Attr4 types.List `tfsdk:"attr_4"` +} +``` + ## Extending The framework supports extending its base type implementations with [custom types](/terraform/plugin/framework/handling-data/types/custom). These can adjust expected provider code usage depending on their implementation.