Skip to content

Commit

Permalink
Merge pull request #23568 from hashicorp/f/typed-sdk-consistent-encoding
Browse files Browse the repository at this point in the history
internal/sdk: resource decoding/encoding now parses the struct tags consistently
  • Loading branch information
tombuildsstuff authored Oct 16, 2023
2 parents 778c3eb + 57396df commit 65aff6f
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 37 deletions.
21 changes: 14 additions & 7 deletions internal/sdk/resource_decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package sdk
import (
"fmt"
"reflect"
"strings"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -57,9 +56,13 @@ func decodeReflectedType(input interface{}, stateRetriever stateRetriever, debug
field := objType.Field(i)
debugLogger.Infof("Field", field)

if val, exists := field.Tag.Lookup("tfschema"); exists {
val = strings.TrimSuffix(val, ",removedInNextMajorVersion")
tfschemaValue, valExists := stateRetriever.GetOkExists(val)
structTags, err := parseStructTags(field.Tag)
if err != nil {
return fmt.Errorf("parsing struct tags for %q: %+v", field.Name, err)
}

if structTags != nil {
tfschemaValue, valExists := stateRetriever.GetOkExists(structTags.hclPath)
if !valExists {
continue
}
Expand Down Expand Up @@ -193,9 +196,13 @@ func setListValue(input interface{}, index int, fieldName string, v []interface{
nestedField := elem.Type().Elem().Field(j)
debugLogger.Infof("nestedField ", nestedField)

if val, exists := nestedField.Tag.Lookup("tfschema"); exists {
val = strings.TrimSuffix(val, ",removedInNextMajorVersion")
nestedTFSchemaValue := test[val]
structTags, err := parseStructTags(nestedField.Tag)
if err != nil {
return fmt.Errorf("parsing struct tags for nested field %q: %+v", nestedField.Name, err)
}

if structTags != nil {
nestedTFSchemaValue := test[structTags.hclPath]
if err := setValue(elem.Interface(), nestedTFSchemaValue, j, fieldName, debugLogger); err != nil {
return err
}
Expand Down
62 changes: 35 additions & 27 deletions internal/sdk/resource_encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package sdk
import (
"fmt"
"reflect"
"strings"

"github.com/hashicorp/terraform-provider-azurerm/internal/features"
)
Expand Down Expand Up @@ -54,71 +53,80 @@ func recurse(objType reflect.Type, objVal reflect.Value, fieldName string, debug
for i := 0; i < objType.NumField(); i++ {
field := objType.Field(i)
fieldVal := objVal.Field(i)
if tfschemaTag, exists := field.Tag.Lookup("tfschema"); exists && !(strings.Contains(tfschemaTag, "removedInNextMajorVersion") && features.FourPointOh()) {
tfschemaTag = strings.TrimSuffix(tfschemaTag, ",removedInNextMajorVersion")
structTags, err := parseStructTags(field.Tag)
if err != nil {
return nil, fmt.Errorf("parsing struct tags for %q: %+v", field.Name, err)
}

if structTags != nil {
if structTags.removedInNextMajorVersion && features.FourPointOh() {
debugLogger.Infof("The HCL Path %q is marked as removed - skipping", structTags.hclPath)
continue
}

switch field.Type.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
iv := fieldVal.Int()
debugLogger.Infof("Setting %q to %d", tfschemaTag, iv)
output[tfschemaTag] = iv
debugLogger.Infof("Setting %q to %d", structTags.hclPath, iv)
output[structTags.hclPath] = iv

case reflect.Float32, reflect.Float64:
fv := fieldVal.Float()
debugLogger.Infof("Setting %q to %f", tfschemaTag, fv)
output[tfschemaTag] = fv
debugLogger.Infof("Setting %q to %f", structTags.hclPath, fv)
output[structTags.hclPath] = fv

case reflect.String:
sv := fieldVal.String()
debugLogger.Infof("Setting %q to %q", tfschemaTag, sv)
output[tfschemaTag] = sv
debugLogger.Infof("Setting %q to %q", structTags.hclPath, sv)
output[structTags.hclPath] = sv

case reflect.Bool:
bv := fieldVal.Bool()
debugLogger.Infof("Setting %q to %t", tfschemaTag, bv)
output[tfschemaTag] = bv
debugLogger.Infof("Setting %q to %t", structTags.hclPath, bv)
output[structTags.hclPath] = bv

case reflect.Map:
iter := fieldVal.MapRange()
attr := make(map[string]interface{})
for iter.Next() {
attr[iter.Key().String()] = iter.Value().Interface()
}
output[tfschemaTag] = attr
output[structTags.hclPath] = attr

case reflect.Slice:
sv := fieldVal.Slice(0, fieldVal.Len())
attr := make([]interface{}, sv.Len())
switch sv.Type() {
case reflect.TypeOf([]string{}):
debugLogger.Infof("Setting %q to []string", tfschemaTag)
debugLogger.Infof("Setting %q to []string", structTags.hclPath)
if sv.Len() > 0 {
output[tfschemaTag] = sv.Interface()
output[structTags.hclPath] = sv.Interface()
} else {
output[tfschemaTag] = make([]string, 0)
output[structTags.hclPath] = make([]string, 0)
}

case reflect.TypeOf([]int{}):
debugLogger.Infof("Setting %q to []int", tfschemaTag)
debugLogger.Infof("Setting %q to []int", structTags.hclPath)
if sv.Len() > 0 {
output[tfschemaTag] = sv.Interface()
output[structTags.hclPath] = sv.Interface()
} else {
output[tfschemaTag] = make([]int, 0)
output[structTags.hclPath] = make([]int, 0)
}

case reflect.TypeOf([]float64{}):
debugLogger.Infof("Setting %q to []float64", tfschemaTag)
debugLogger.Infof("Setting %q to []float64", structTags.hclPath)
if sv.Len() > 0 {
output[tfschemaTag] = sv.Interface()
output[structTags.hclPath] = sv.Interface()
} else {
output[tfschemaTag] = make([]float64, 0)
output[structTags.hclPath] = make([]float64, 0)
}

case reflect.TypeOf([]bool{}):
debugLogger.Infof("Setting %q to []bool", tfschemaTag)
debugLogger.Infof("Setting %q to []bool", structTags.hclPath)
if sv.Len() > 0 {
output[tfschemaTag] = sv.Interface()
output[structTags.hclPath] = sv.Interface()
} else {
output[tfschemaTag] = make([]bool, 0)
output[structTags.hclPath] = make([]bool, 0)
}

default:
Expand All @@ -135,11 +143,11 @@ func recurse(objType reflect.Type, objVal reflect.Value, fieldName string, debug
}
attr[i] = serialized
}
debugLogger.Infof("[SLICE] Setting %q to %+v", tfschemaTag, attr)
output[tfschemaTag] = attr
debugLogger.Infof("[SLICE] Setting %q to %+v", structTags.hclPath, attr)
output[structTags.hclPath] = attr
}
default:
return output, fmt.Errorf("unknown type %+v for key %q", field.Type.Kind(), tfschemaTag)
return output, fmt.Errorf("unknown type %+v for key %q", field.Type.Kind(), structTags.hclPath)
}
}
}
Expand Down
55 changes: 55 additions & 0 deletions internal/sdk/resource_helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package sdk

import (
"fmt"
"reflect"
"strings"
)

type decodedStructTags struct {
// hclPath defines the path to this field used for this in the Schema for this Resource
hclPath string

// removedInNextMajorVersion specifies whether this field is deprecated and should not
// be set into the state in the next major version of the Provider
removedInNextMajorVersion bool
}

// parseStructTags parses the struct tags defined in input into a decodedStructTags object
// which allows for the consistent parsing of struct tags across the Typed SDK.
func parseStructTags(input reflect.StructTag) (*decodedStructTags, error) {
tag, ok := input.Lookup("tfschema")
if !ok {
// doesn't exist - ignore it?
return nil, nil
}
if tag == "" {
return nil, fmt.Errorf("the `tfschema` struct tag was defined but empty")
}

components := strings.Split(tag, ",")
output := &decodedStructTags{
// NOTE: `hclPath` has to be the first item in the struct tag
hclPath: strings.TrimSpace(components[0]),
removedInNextMajorVersion: false,
}
if output.hclPath == "" {
return nil, fmt.Errorf("hclPath was empty")
}

if len(components) > 1 {
// remove the hcl field name since it's been parsed
components = components[1:]
for _, item := range components {
item = strings.TrimSpace(item) // allowing for both `foo,bar` and `foo, bar` in struct tags
if strings.EqualFold(item, "removedInNextMajorVersion") {
output.removedInNextMajorVersion = true
continue
}

return nil, fmt.Errorf("internal-error: the struct-tag %q is not implemented - struct tags are %q", item, tag)
}
}

return output, nil
}
111 changes: 111 additions & 0 deletions internal/sdk/resource_helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package sdk

import (
"reflect"
"testing"

"github.com/hashicorp/go-azure-helpers/lang/pointer"
)

func TestParseStructTags_Empty(t *testing.T) {
actual, err := parseStructTags("")
if err != nil {
t.Fatalf("unexpected error %q", err.Error())
}

if actual != nil {
t.Fatalf("expected actual to be nil but got %+v", *actual)
}
}

func TestParseStructTags_WithValue(t *testing.T) {
testData := []struct {
input reflect.StructTag
expected *decodedStructTags
error *string
}{
{
// empty hclPath
input: `tfschema:""`,
expected: nil,
error: pointer.To("the `tfschema` struct tag was defined but empty"),
},
{
// valid, no removedInNextMajorVersion
input: `tfschema:"hello"`,
expected: &decodedStructTags{
hclPath: "hello",
removedInNextMajorVersion: false,
},
},
{
// valid, with removedInNextMajorVersion
input: `tfschema:"hello,removedInNextMajorVersion"`,
expected: &decodedStructTags{
hclPath: "hello",
removedInNextMajorVersion: true,
},
},
{
// valid, with removedInNextMajorVersion and a space before the comma
input: `tfschema:"hello, removedInNextMajorVersion"`,
expected: &decodedStructTags{
hclPath: "hello",
removedInNextMajorVersion: true,
},
},
{
// valid, with removedInNextMajorVersion and a space after the comma
//
// This would be caught in PR review, but would be a confusing error/experience
// during development so it's worth being lenient here since it's non-impactful
input: `tfschema:"hello ,removedInNextMajorVersion"`,
expected: &decodedStructTags{
hclPath: "hello",
removedInNextMajorVersion: true,
},
},
{
// valid, with removedInNextMajorVersion and a space either side
//
// This would be caught in PR review, but would be a confusing error/experience
// during development so it's worth being lenient here since it's non-impactful
input: `tfschema:"hello , removedInNextMajorVersion"`,
expected: &decodedStructTags{
hclPath: "hello",
removedInNextMajorVersion: true,
},
},
{
// invalid, unknown struct tags
input: `tfschema:"hello,world"`,
expected: nil,
error: pointer.To(`internal-error: the struct-tag "world" is not implemented - struct tags are "hello,world"`),
},
}
for i, data := range testData {
t.Logf("Index %d - Input %q", i, data.input)
actual, err := parseStructTags(data.input)
if err != nil {
if data.error != nil {
if err.Error() == *data.error {
continue
}

t.Fatalf("expected the error %q but got %q", *data.error, err.Error())
}

t.Fatalf("unexpected error %q", err.Error())
}
if data.error != nil {
t.Fatalf("expected the error %q but didn't get one", *data.error)
}

if actual == nil {
t.Fatalf("expected actual to have a value but got nil")
}
if !reflect.DeepEqual(*data.expected, *actual) {
t.Fatalf("expected [%+v] and actual [%+v] didn't match", *data.expected, *actual)
}
}
}
10 changes: 7 additions & 3 deletions internal/sdk/wrapper_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ func validateModelObjectRecursively(prefix string, objType reflect.Type, objVal
}
}

if _, exists := field.Tag.Lookup("tfschema"); !exists {
fieldName := strings.TrimPrefix(fmt.Sprintf("%s.%s", prefix, field.Name), ".")
return fmt.Errorf("field %q is missing an `tfschema` label", fieldName)
fieldName := strings.TrimPrefix(fmt.Sprintf("%s.%s", prefix, field.Name), ".")
structTags, err := parseStructTags(field.Tag)
if err != nil {
return fmt.Errorf("parsing struct tags for %q", fieldName)
}
if structTags == nil {
return fmt.Errorf("field %q is missing a struct tag for `tfschema`", fieldName)
}
}

Expand Down

0 comments on commit 65aff6f

Please sign in to comment.