Skip to content

Commit 373a6b3

Browse files
authored
internal/reflect: Use underlying terraform-plugin-go types to validate provider given values (#806)
Reference: #715 Reference: #793 Reference: hashicorp/terraform-provider-aws#32441 The original intention of this logic was to raise a framework error instead of underlying terraform-plugin-go error or panic when providers returned an errant value from their logic. In practice though, comparing the framework types (which may include either base or custom types) was too strict. Technically, as long as the underlying terraform-plugin-go types are equivalent, then this check should pass. Terraform will catch any errant modifications of the actual values, should there be a difference between the base types and custom types.
1 parent fc2db9e commit 373a6b3

File tree

4 files changed

+82
-26
lines changed

4 files changed

+82
-26
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: BUG FIXES
2+
body: 'tfsdk: Prevented `Value Conversion Error` diagnostics when using `Set()` method
3+
with base types instead of custom types'
4+
time: 2023-07-19T14:24:13.806804-04:00
5+
custom:
6+
Issue: "806"

internal/reflect/interfaces.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -320,17 +320,14 @@ func FromAttributeValue(ctx context.Context, typ attr.Type, val attr.Value, path
320320
// compatible. This check will ensure the framework raises its own
321321
// error is there is a mismatch, rather than a terraform-plugin-go
322322
// error or worse a panic.
323-
//
324-
// If this validation causes major issues, another option is to
325-
// validate via tftypes.Type for both the type and value type.
326-
if !typ.Equal(val.Type(ctx)) {
323+
if !typ.TerraformType(ctx).Equal(val.Type(ctx).TerraformType(ctx)) {
327324
diags.AddAttributeError(
328325
path,
329326
"Value Conversion Error",
330327
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
331328
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
332-
fmt.Sprintf("Expected type: %s\n", typ)+
333-
fmt.Sprintf("Value type: %s\n", val.Type(ctx))+
329+
fmt.Sprintf("Expected framework type from provider logic: %s / underlying type: %s\n", typ, typ.TerraformType(ctx))+
330+
fmt.Sprintf("Received framework type from provider logic: %s / underlying type: %s\n", val.Type(ctx), val.Type(ctx).TerraformType(ctx))+
334331
fmt.Sprintf("Path: %s", path),
335332
)
336333

internal/reflect/interfaces_test.go

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -502,16 +502,31 @@ func TestFromAttributeValue(t *testing.T) {
502502
CreatedBy: testtypes.BoolType{},
503503
},
504504
},
505+
"BoolTypable-BoolValue": {
506+
typ: testtypes.BoolTypeWithSemanticEquals{},
507+
val: types.BoolNull(),
508+
expected: types.BoolNull(),
509+
},
505510
"Float64Type-Float64Value": {
506511
typ: types.Float64Type,
507512
val: types.Float64Null(),
508513
expected: types.Float64Null(),
509514
},
515+
"Float64Typable-Float64Value": {
516+
typ: testtypes.Float64TypeWithSemanticEquals{},
517+
val: types.Float64Null(),
518+
expected: types.Float64Null(),
519+
},
510520
"Int64Type-Int64Value": {
511521
typ: types.Int64Type,
512522
val: types.Int64Null(),
513523
expected: types.Int64Null(),
514524
},
525+
"Int64Typable-Int64Value": {
526+
typ: testtypes.Int64TypeWithSemanticEquals{},
527+
val: types.Int64Null(),
528+
expected: types.Int64Null(),
529+
},
515530
"ListType-ListValue-matching-elements": {
516531
typ: types.ListType{ElemType: types.StringType},
517532
val: types.ListNull(types.StringType),
@@ -527,12 +542,19 @@ func TestFromAttributeValue(t *testing.T) {
527542
"Value Conversion Error",
528543
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
529544
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
530-
"Expected type: types.ListType[basetypes.StringType]\n"+
531-
"Value type: types.ListType[basetypes.BoolType]\n"+
545+
"Expected framework type from provider logic: types.ListType[basetypes.StringType] / underlying type: tftypes.List[tftypes.String]\n"+
546+
"Received framework type from provider logic: types.ListType[basetypes.BoolType] / underlying type: tftypes.List[tftypes.Bool]\n"+
532547
"Path: test",
533548
),
534549
},
535550
},
551+
"ListTypable-ListValue-matching-elements": {
552+
typ: testtypes.ListType{
553+
ListType: types.ListType{ElemType: types.StringType},
554+
},
555+
val: types.ListNull(types.StringType),
556+
expected: types.ListNull(types.StringType),
557+
},
536558
"MapType-MapValue-matching-elements": {
537559
typ: types.MapType{ElemType: types.StringType},
538560
val: types.MapNull(types.StringType),
@@ -548,12 +570,19 @@ func TestFromAttributeValue(t *testing.T) {
548570
"Value Conversion Error",
549571
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
550572
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
551-
"Expected type: types.MapType[basetypes.StringType]\n"+
552-
"Value type: types.MapType[basetypes.BoolType]\n"+
573+
"Expected framework type from provider logic: types.MapType[basetypes.StringType] / underlying type: tftypes.Map[tftypes.String]\n"+
574+
"Received framework type from provider logic: types.MapType[basetypes.BoolType] / underlying type: tftypes.Map[tftypes.Bool]\n"+
553575
"Path: test",
554576
),
555577
},
556578
},
579+
"MapTypable-MapValue-matching-elements": {
580+
typ: testtypes.MapType{
581+
MapType: types.MapType{ElemType: types.StringType},
582+
},
583+
val: types.MapNull(types.StringType),
584+
expected: types.MapNull(types.StringType),
585+
},
557586
"NumberType-NumberValue": {
558587
typ: types.NumberType,
559588
val: types.NumberNull(),
@@ -568,6 +597,11 @@ func TestFromAttributeValue(t *testing.T) {
568597
CreatedBy: testtypes.NumberType{},
569598
},
570599
},
600+
"NumberTypable-NumberValue": {
601+
typ: testtypes.NumberTypeWithSemanticEquals{},
602+
val: types.NumberNull(),
603+
expected: types.NumberNull(),
604+
},
571605
"ObjectType-ObjectValue-matching-attributes": {
572606
typ: types.ObjectType{AttrTypes: map[string]attr.Type{"test_attr": types.StringType}},
573607
val: types.ObjectNull(map[string]attr.Type{"test_attr": types.StringType}),
@@ -583,8 +617,8 @@ func TestFromAttributeValue(t *testing.T) {
583617
"Value Conversion Error",
584618
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
585619
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
586-
"Expected type: types.ObjectType[\"test_attr\":basetypes.StringType]\n"+
587-
"Value type: types.ObjectType[\"not_test_attr\":basetypes.StringType]\n"+
620+
"Expected framework type from provider logic: types.ObjectType[\"test_attr\":basetypes.StringType] / underlying type: tftypes.Object[\"test_attr\":tftypes.String]\n"+
621+
"Received framework type from provider logic: types.ObjectType[\"not_test_attr\":basetypes.StringType] / underlying type: tftypes.Object[\"not_test_attr\":tftypes.String]\n"+
588622
"Path: test",
589623
),
590624
},
@@ -599,12 +633,19 @@ func TestFromAttributeValue(t *testing.T) {
599633
"Value Conversion Error",
600634
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
601635
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
602-
"Expected type: types.ObjectType[\"test_attr\":basetypes.StringType]\n"+
603-
"Value type: types.ObjectType[\"test_attr\":basetypes.BoolType]\n"+
636+
"Expected framework type from provider logic: types.ObjectType[\"test_attr\":basetypes.StringType] / underlying type: tftypes.Object[\"test_attr\":tftypes.String]\n"+
637+
"Received framework type from provider logic: types.ObjectType[\"test_attr\":basetypes.BoolType] / underlying type: tftypes.Object[\"test_attr\":tftypes.Bool]\n"+
604638
"Path: test",
605639
),
606640
},
607641
},
642+
"ObjectTypable-ObjectValue-matching-attributes": {
643+
typ: testtypes.ObjectType{
644+
ObjectType: types.ObjectType{AttrTypes: map[string]attr.Type{"test_attr": types.StringType}},
645+
},
646+
val: types.ObjectNull(map[string]attr.Type{"test_attr": types.StringType}),
647+
expected: types.ObjectNull(map[string]attr.Type{"test_attr": types.StringType}),
648+
},
608649
"SetType-SetValue-matching-elements": {
609650
typ: types.SetType{ElemType: types.StringType},
610651
val: types.SetNull(types.StringType),
@@ -620,12 +661,19 @@ func TestFromAttributeValue(t *testing.T) {
620661
"Value Conversion Error",
621662
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
622663
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
623-
"Expected type: types.SetType[basetypes.StringType]\n"+
624-
"Value type: types.SetType[basetypes.BoolType]\n"+
664+
"Expected framework type from provider logic: types.SetType[basetypes.StringType] / underlying type: tftypes.Set[tftypes.String]\n"+
665+
"Received framework type from provider logic: types.SetType[basetypes.BoolType] / underlying type: tftypes.Set[tftypes.Bool]\n"+
625666
"Path: test",
626667
),
627668
},
628669
},
670+
"SetTypable-SetValue-matching-elements": {
671+
typ: testtypes.SetType{
672+
SetType: types.SetType{ElemType: types.StringType},
673+
},
674+
val: types.SetNull(types.StringType),
675+
expected: types.SetNull(types.StringType),
676+
},
629677
"StringType-StringValue-null": {
630678
typ: types.StringType,
631679
val: types.StringNull(),
@@ -650,6 +698,11 @@ func TestFromAttributeValue(t *testing.T) {
650698
CreatedBy: testtypes.StringType{},
651699
},
652700
},
701+
"StringTypable-StringValue": {
702+
typ: testtypes.StringTypeWithSemanticEquals{},
703+
val: types.StringNull(),
704+
expected: types.StringNull(),
705+
},
653706
}
654707

655708
for name, tc := range testCases {

internal/reflect/struct_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -986,8 +986,8 @@ func TestFromStruct_errors(t *testing.T) {
986986
"Value Conversion Error",
987987
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
988988
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
989-
"Expected type: basetypes.StringType\n"+
990-
"Value type: basetypes.BoolType\n"+
989+
"Expected framework type from provider logic: basetypes.StringType / underlying type: tftypes.String\n"+
990+
"Received framework type from provider logic: basetypes.BoolType / underlying type: tftypes.Bool\n"+
991991
"Path: test.string",
992992
),
993993
},
@@ -1076,8 +1076,8 @@ func TestFromStruct_errors(t *testing.T) {
10761076
"Value Conversion Error",
10771077
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
10781078
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
1079-
"Expected type: types.ListType[basetypes.StringType]\n"+
1080-
"Value type: types.ListType[!!! MISSING TYPE !!!]\n"+
1079+
"Expected framework type from provider logic: types.ListType[basetypes.StringType] / underlying type: tftypes.List[tftypes.String]\n"+
1080+
"Received framework type from provider logic: types.ListType[!!! MISSING TYPE !!!] / underlying type: tftypes.List[tftypes.DynamicPseudoType]\n"+
10811081
"Path: test.list",
10821082
),
10831083
},
@@ -1101,8 +1101,8 @@ func TestFromStruct_errors(t *testing.T) {
11011101
"Value Conversion Error",
11021102
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
11031103
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
1104-
"Expected type: types.MapType[basetypes.StringType]\n"+
1105-
"Value type: types.MapType[!!! MISSING TYPE !!!]\n"+
1104+
"Expected framework type from provider logic: types.MapType[basetypes.StringType] / underlying type: tftypes.Map[tftypes.String]\n"+
1105+
"Received framework type from provider logic: types.MapType[!!! MISSING TYPE !!!] / underlying type: tftypes.Map[tftypes.DynamicPseudoType]\n"+
11061106
"Path: test.map",
11071107
),
11081108
},
@@ -1129,8 +1129,8 @@ func TestFromStruct_errors(t *testing.T) {
11291129
"Value Conversion Error",
11301130
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
11311131
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
1132-
"Expected type: types.ObjectType[\"test\":basetypes.StringType]\n"+
1133-
"Value type: types.ObjectType[]\n"+
1132+
"Expected framework type from provider logic: types.ObjectType[\"test\":basetypes.StringType] / underlying type: tftypes.Object[\"test\":tftypes.String]\n"+
1133+
"Received framework type from provider logic: types.ObjectType[] / underlying type: tftypes.Object[]\n"+
11341134
"Path: test.object",
11351135
),
11361136
},
@@ -1154,8 +1154,8 @@ func TestFromStruct_errors(t *testing.T) {
11541154
"Value Conversion Error",
11551155
"An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. "+
11561156
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
1157-
"Expected type: types.SetType[basetypes.StringType]\n"+
1158-
"Value type: types.SetType[!!! MISSING TYPE !!!]\n"+
1157+
"Expected framework type from provider logic: types.SetType[basetypes.StringType] / underlying type: tftypes.Set[tftypes.String]\n"+
1158+
"Received framework type from provider logic: types.SetType[!!! MISSING TYPE !!!] / underlying type: tftypes.Set[tftypes.DynamicPseudoType]\n"+
11591159
"Path: test.set",
11601160
),
11611161
},

0 commit comments

Comments
 (0)