diff --git a/internal/sdk/resource_decode.go b/internal/sdk/resource_decode.go index cb35f9122102..4e35f51a97e5 100644 --- a/internal/sdk/resource_decode.go +++ b/internal/sdk/resource_decode.go @@ -6,7 +6,6 @@ package sdk import ( "fmt" "reflect" - "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -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 } @@ -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 } diff --git a/internal/sdk/resource_encode.go b/internal/sdk/resource_encode.go index 9a93345f22d1..b54e792d629e 100644 --- a/internal/sdk/resource_encode.go +++ b/internal/sdk/resource_encode.go @@ -6,7 +6,6 @@ package sdk import ( "fmt" "reflect" - "strings" "github.com/hashicorp/terraform-provider-azurerm/internal/features" ) @@ -54,28 +53,37 @@ 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() @@ -83,42 +91,42 @@ func recurse(objType reflect.Type, objVal reflect.Value, fieldName string, debug 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: @@ -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) } } } diff --git a/internal/sdk/resource_helpers.go b/internal/sdk/resource_helpers.go new file mode 100644 index 000000000000..a4b891bc693e --- /dev/null +++ b/internal/sdk/resource_helpers.go @@ -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 +} diff --git a/internal/sdk/resource_helpers_test.go b/internal/sdk/resource_helpers_test.go new file mode 100644 index 000000000000..0599436293b0 --- /dev/null +++ b/internal/sdk/resource_helpers_test.go @@ -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) + } + } +} diff --git a/internal/sdk/wrapper_validate.go b/internal/sdk/wrapper_validate.go index 1009d77d9193..2f749a7dcc98 100644 --- a/internal/sdk/wrapper_validate.go +++ b/internal/sdk/wrapper_validate.go @@ -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) } }