From 5f9d3d3d9e0c3098bafe05cad16178523de44827 Mon Sep 17 00:00:00 2001 From: Mark Dietzer Date: Thu, 15 Sep 2022 06:14:57 -0700 Subject: [PATCH] types: Add element type validation to ListType, MapType, and SetType (#481) Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/467 --- .changelog/481.txt | 3 + .../fwserver/attribute_validation_test.go | 6 +- .../fwserver/schema_plan_modification_test.go | 4 +- internal/reflect/map.go | 2 +- types/list.go | 51 ++++++++++++++++ types/list_test.go | 58 +++++++++++++++++++ types/map.go | 51 ++++++++++++++++ types/map_test.go | 58 +++++++++++++++++++ types/set.go | 19 +++++- types/set_test.go | 15 +++++ 10 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 .changelog/481.txt diff --git a/.changelog/481.txt b/.changelog/481.txt new file mode 100644 index 000000000..db09d6e98 --- /dev/null +++ b/.changelog/481.txt @@ -0,0 +1,3 @@ +```release-note:bug +types: Ensured `List`, `Map`, and `Set` types with `xattr.TypeWithValidate` elements run validation on those elements +``` diff --git a/internal/fwserver/attribute_validation_test.go b/internal/fwserver/attribute_validation_test.go index 4a966cd3c..423080c48 100644 --- a/internal/fwserver/attribute_validation_test.go +++ b/internal/fwserver/attribute_validation_test.go @@ -143,9 +143,9 @@ func TestAttributeValidate(t *testing.T) { Diagnostics: diag.Diagnostics{ diag.NewAttributeErrorDiagnostic( path.Root("test"), - "Configuration Read Error", - "An unexpected error was encountered trying to convert an attribute value from the configuration. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - "Error: can't use tftypes.String<\"testvalue\"> as value of List with ElementType types.primitive, can only use tftypes.String values", + "List Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "expected List value, received tftypes.Value with value: tftypes.String<\"testvalue\">", ), }, }, diff --git a/internal/fwserver/schema_plan_modification_test.go b/internal/fwserver/schema_plan_modification_test.go index f9f642848..e66e47c1b 100644 --- a/internal/fwserver/schema_plan_modification_test.go +++ b/internal/fwserver/schema_plan_modification_test.go @@ -89,7 +89,7 @@ func TestSchemaModifyPlan(t *testing.T) { Diagnostics: diag.Diagnostics{ diag.NewAttributeErrorDiagnostic( path.Root("test"), - "Configuration Read Error", + "List Type Validation Error", "An unexpected error was encountered trying to convert an attribute value from the configuration. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ "Error: can't use tftypes.String<\"testvalue\"> as value of List with ElementType types.primitive, can only use tftypes.String values", ), @@ -175,7 +175,7 @@ func TestSchemaModifyPlan(t *testing.T) { ), diag.NewAttributeErrorDiagnostic( path.Root("test"), - "Configuration Read Error", + "List Type Validation Error", "An unexpected error was encountered trying to convert an attribute value from the configuration. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ "Error: can't use tftypes.String<\"testvalue\"> as value of List with ElementType types.primitive, can only use tftypes.String values", ), diff --git a/internal/reflect/map.go b/internal/reflect/map.go index ccd5597e0..db2b6fe63 100644 --- a/internal/reflect/map.go +++ b/internal/reflect/map.go @@ -145,7 +145,7 @@ func FromMap(ctx context.Context, typ attr.TypeWithElementType, val reflect.Valu return nil, append(diags, toTerraformValueErrorDiag(err, path)) } - if typeWithValidate, ok := typ.(xattr.TypeWithValidate); ok { + if typeWithValidate, ok := elemType.(xattr.TypeWithValidate); ok { diags.Append(typeWithValidate.Validate(ctx, tfVal, path.AtMapKey(key.String()))...) if diags.HasError() { diff --git a/types/list.go b/types/list.go index c035bf000..6f47efc34 100644 --- a/types/list.go +++ b/types/list.go @@ -6,8 +6,10 @@ import ( "strings" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/reflect" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -111,6 +113,55 @@ func (l ListType) String() string { return "types.ListType[" + l.ElemType.String() + "]" } +// Validate validates all elements of the list that are of type +// xattr.TypeWithValidate. +func (l ListType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { + var diags diag.Diagnostics + + if in.Type() == nil { + return diags + } + + if !in.Type().Is(tftypes.List{}) { + err := fmt.Errorf("expected List value, received %T with value: %v", in, in) + diags.AddAttributeError( + path, + "List Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return diags + } + + if !in.IsKnown() || in.IsNull() { + return diags + } + + var elems []tftypes.Value + + if err := in.As(&elems); err != nil { + diags.AddAttributeError( + path, + "List Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return diags + } + + validatableType, isValidatable := l.ElemType.(xattr.TypeWithValidate) + if !isValidatable { + return diags + } + + for index, elem := range elems { + if !elem.IsFullyKnown() { + continue + } + diags = append(diags, validatableType.Validate(ctx, elem, path.AtListIndex(index))...) + } + + return diags +} + // List represents a list of attr.Values, all of the same type, indicated // by ElemType. type List struct { diff --git a/types/list_test.go b/types/list_test.go index 5cb71b4dc..a1ebe027e 100644 --- a/types/list_test.go +++ b/types/list_test.go @@ -6,6 +6,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -768,3 +770,59 @@ func TestListString(t *testing.T) { }) } } + +func TestListTypeValidate(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + listType ListType + tfValue tftypes.Value + path path.Path + expectedDiags diag.Diagnostics + }{ + "wrong-value-type": { + listType: ListType{ + ElemType: StringType, + }, + tfValue: tftypes.NewValue(tftypes.Set{ + ElementType: tftypes.String, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "testvalue"), + }), + path: path.Root("test"), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "List Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "expected List value, received tftypes.Value with value: tftypes.Set[tftypes.String]>", + ), + }, + }, + "no-validation": { + listType: ListType{ + ElemType: StringType, + }, + tfValue: tftypes.NewValue(tftypes.List{ + ElementType: tftypes.String, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "testvalue"), + }), + path: path.Root("test"), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + diags := testCase.listType.Validate(context.Background(), testCase.tfValue, testCase.path) + + if diff := cmp.Diff(diags, testCase.expectedDiags); diff != "" { + t.Errorf("unexpected diagnostics difference: %s", diff) + } + }) + } +} diff --git a/types/map.go b/types/map.go index 174b581dc..fa44068b7 100644 --- a/types/map.go +++ b/types/map.go @@ -7,8 +7,10 @@ import ( "strings" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/reflect" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -115,6 +117,55 @@ func (m MapType) String() string { return "types.MapType[" + m.ElemType.String() + "]" } +// Validate validates all elements of the map that are of type +// xattr.TypeWithValidate. +func (m MapType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { + var diags diag.Diagnostics + + if in.Type() == nil { + return diags + } + + if !in.Type().Is(tftypes.Map{}) { + err := fmt.Errorf("expected Map value, received %T with value: %v", in, in) + diags.AddAttributeError( + path, + "Map Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return diags + } + + if !in.IsKnown() || in.IsNull() { + return diags + } + + var elems map[string]tftypes.Value + + if err := in.As(&elems); err != nil { + diags.AddAttributeError( + path, + "Map Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return diags + } + + validatableType, isValidatable := m.ElemType.(xattr.TypeWithValidate) + if !isValidatable { + return diags + } + + for index, elem := range elems { + if !elem.IsFullyKnown() { + continue + } + diags = append(diags, validatableType.Validate(ctx, elem, path.AtMapKey(index))...) + } + + return diags +} + // Map represents a map of attr.Values, all of the same type, indicated by // ElemType. Keys for the map will always be strings. type Map struct { diff --git a/types/map_test.go b/types/map_test.go index 90bc5d24e..9a58e6cab 100644 --- a/types/map_test.go +++ b/types/map_test.go @@ -7,6 +7,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -782,3 +784,59 @@ func TestMapString(t *testing.T) { }) } } + +func TestMapTypeValidate(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + mapType MapType + tfValue tftypes.Value + path path.Path + expectedDiags diag.Diagnostics + }{ + "wrong-value-type": { + mapType: MapType{ + ElemType: StringType, + }, + tfValue: tftypes.NewValue(tftypes.List{ + ElementType: tftypes.String, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "testvalue"), + }), + path: path.Root("test"), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Map Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "expected Map value, received tftypes.Value with value: tftypes.List[tftypes.String]>", + ), + }, + }, + "no-validation": { + mapType: MapType{ + ElemType: StringType, + }, + tfValue: tftypes.NewValue(tftypes.Map{ + ElementType: tftypes.String, + }, map[string]tftypes.Value{ + "testkey": tftypes.NewValue(tftypes.String, "testvalue"), + }), + path: path.Root("test"), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + diags := testCase.mapType.Validate(context.Background(), testCase.tfValue, testCase.path) + + if diff := cmp.Diff(diags, testCase.expectedDiags); diff != "" { + t.Errorf("unexpected diagnostics difference: %s", diff) + } + }) + } +} diff --git a/types/set.go b/types/set.go index 064330bc3..b7295c4c4 100644 --- a/types/set.go +++ b/types/set.go @@ -148,15 +148,32 @@ func (st SetType) Validate(ctx context.Context, in tftypes.Value, path path.Path return diags } + validatableType, isValidatable := st.ElemType.(xattr.TypeWithValidate) + // Attempting to use map[tftypes.Value]struct{} for duplicate detection yields: // panic: runtime error: hash of unhashable type tftypes.primitive // Instead, use for loops. for indexOuter, elemOuter := range elems { - // Only evaluate fully known values for duplicates. + // Only evaluate fully known values for duplicates and validation. if !elemOuter.IsFullyKnown() { continue } + // Validate the element first + if isValidatable { + elemValue, err := st.ElemType.ValueFromTerraform(ctx, elemOuter) + if err != nil { + diags.AddAttributeError( + path, + "Set Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return diags + } + diags = append(diags, validatableType.Validate(ctx, elemOuter, path.AtSetValue(elemValue))...) + } + + // Then check for duplicates for indexInner := indexOuter + 1; indexInner < len(elems); indexInner++ { elemInner := elems[indexInner] diff --git a/types/set_test.go b/types/set_test.go index d46ba7817..6ba3075d1 100644 --- a/types/set_test.go +++ b/types/set_test.go @@ -540,6 +540,21 @@ func TestSetTypeValidate(t *testing.T) { ), }, }, + "wrong-value-type": { + in: tftypes.NewValue(tftypes.List{ + ElementType: tftypes.String, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "testvalue"), + }), + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("test"), + "Set Type Validation Error", + "An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "expected Set value, received tftypes.Value with value: tftypes.List[tftypes.String]>", + ), + }, + }, } for name, testCase := range testCases { name, testCase := name, testCase