From 44d0e8266a4accf3a5a1b89ba2ba4247c4d05156 Mon Sep 17 00:00:00 2001 From: Doridian Date: Tue, 13 Sep 2022 21:47:01 -0700 Subject: [PATCH 01/10] Try validating stuff --- .../fwserver/attribute_validation_test.go | 2 +- .../fwserver/schema_plan_modification_test.go | 4 +- types/list.go | 50 +++++++++++++++++++ types/map.go | 50 +++++++++++++++++++ types/set.go | 10 +++- 5 files changed, 112 insertions(+), 4 deletions(-) diff --git a/internal/fwserver/attribute_validation_test.go b/internal/fwserver/attribute_validation_test.go index 4a966cd3c..c6fd077f7 100644 --- a/internal/fwserver/attribute_validation_test.go +++ b/internal/fwserver/attribute_validation_test.go @@ -143,7 +143,7 @@ func TestAttributeValidate(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", ), 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/types/list.go b/types/list.go index c035bf000..0c7056c97 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,54 @@ 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(), + ) + } + + 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, ok := l.ElemType.(xattr.TypeWithValidate) + if !ok { + 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/map.go b/types/map.go index 174b581dc..9434e27d2 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,54 @@ 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(), + ) + } + + 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, ok := m.ElemType.(xattr.TypeWithValidate) + if !ok { + 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/set.go b/types/set.go index 064330bc3..95565f094 100644 --- a/types/set.go +++ b/types/set.go @@ -148,15 +148,23 @@ 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 { + diags = append(diags, validatableType.Validate(ctx, elemOuter, path.AtListIndex(indexOuter))...) + } + + // Then check for duplicates for indexInner := indexOuter + 1; indexInner < len(elems); indexInner++ { elemInner := elems[indexInner] From 9d56eb0620b26d8fad02f5c8b2f622f6f322b24b Mon Sep 17 00:00:00 2001 From: Doridian Date: Tue, 13 Sep 2022 21:59:22 -0700 Subject: [PATCH 02/10] disable map, list works now --- internal/fwserver/attribute_validation_test.go | 10 ++++++++-- types/map.go | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/fwserver/attribute_validation_test.go b/internal/fwserver/attribute_validation_test.go index c6fd077f7..10b8a74a5 100644 --- a/internal/fwserver/attribute_validation_test.go +++ b/internal/fwserver/attribute_validation_test.go @@ -144,8 +144,14 @@ func TestAttributeValidate(t *testing.T) { diag.NewAttributeErrorDiagnostic( path.Root("test"), "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", + "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\">", + ), + 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"+ + "can't unmarshal tftypes.String into *[]tftypes.Value expected []tftypes.Value", ), }, }, diff --git a/types/map.go b/types/map.go index 9434e27d2..20bbd2f18 100644 --- a/types/map.go +++ b/types/map.go @@ -121,6 +121,9 @@ func (m MapType) String() string { // xattr.TypeWithValidate. func (m MapType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { var diags diag.Diagnostics + if true { + return diags + } if in.Type() == nil { return diags From 06b196f9946b882334d4648db66fe76a51c2d3c7 Mon Sep 17 00:00:00 2001 From: Doridian Date: Tue, 13 Sep 2022 22:08:38 -0700 Subject: [PATCH 03/10] fix bug --- internal/reflect/map.go | 2 +- types/map.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) 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/map.go b/types/map.go index 20bbd2f18..9434e27d2 100644 --- a/types/map.go +++ b/types/map.go @@ -121,9 +121,6 @@ func (m MapType) String() string { // xattr.TypeWithValidate. func (m MapType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { var diags diag.Diagnostics - if true { - return diags - } if in.Type() == nil { return diags From edd36417b519c10f2291576abafe3d8585d3240b Mon Sep 17 00:00:00 2001 From: Doridian Date: Tue, 13 Sep 2022 22:10:43 -0700 Subject: [PATCH 04/10] rename --- types/list.go | 4 ++-- types/map.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/types/list.go b/types/list.go index 0c7056c97..6183c261f 100644 --- a/types/list.go +++ b/types/list.go @@ -146,8 +146,8 @@ func (l ListType) Validate(ctx context.Context, in tftypes.Value, path path.Path return diags } - validatableType, ok := l.ElemType.(xattr.TypeWithValidate) - if !ok { + validatableType, isValidatable := l.ElemType.(xattr.TypeWithValidate) + if !isValidatable { return diags } diff --git a/types/map.go b/types/map.go index 9434e27d2..1a06a5053 100644 --- a/types/map.go +++ b/types/map.go @@ -150,8 +150,8 @@ func (m MapType) Validate(ctx context.Context, in tftypes.Value, path path.Path) return diags } - validatableType, ok := m.ElemType.(xattr.TypeWithValidate) - if !ok { + validatableType, isValidatable := m.ElemType.(xattr.TypeWithValidate) + if !isValidatable { return diags } From d04096d5aca06b2cb80179d93cdc3c00573dc82d Mon Sep 17 00:00:00 2001 From: Mark Dietzer Date: Wed, 14 Sep 2022 07:13:16 -0700 Subject: [PATCH 05/10] Update types/list.go Co-authored-by: Brian Flad --- types/list.go | 1 + 1 file changed, 1 insertion(+) diff --git a/types/list.go b/types/list.go index 6183c261f..6f47efc34 100644 --- a/types/list.go +++ b/types/list.go @@ -129,6 +129,7 @@ func (l ListType) Validate(ctx context.Context, in tftypes.Value, path path.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() { From deb7411d0740c23c984eba84806a183aec08c3ea Mon Sep 17 00:00:00 2001 From: Mark Dietzer Date: Wed, 14 Sep 2022 07:13:24 -0700 Subject: [PATCH 06/10] Update types/set.go Co-authored-by: Brian Flad --- types/set.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/set.go b/types/set.go index 95565f094..89d90fb2a 100644 --- a/types/set.go +++ b/types/set.go @@ -161,7 +161,7 @@ func (st SetType) Validate(ctx context.Context, in tftypes.Value, path path.Path // Validate the element first if isValidatable { - diags = append(diags, validatableType.Validate(ctx, elemOuter, path.AtListIndex(indexOuter))...) + diags = append(diags, validatableType.Validate(ctx, elemOuter, path.AtSetValue(elemOuter))...) } // Then check for duplicates From d34ae7e4572a6144308c99b40b568fb077744bb1 Mon Sep 17 00:00:00 2001 From: Mark Dietzer Date: Wed, 14 Sep 2022 07:13:31 -0700 Subject: [PATCH 07/10] Update types/map.go Co-authored-by: Brian Flad --- types/map.go | 1 + 1 file changed, 1 insertion(+) diff --git a/types/map.go b/types/map.go index 1a06a5053..fa44068b7 100644 --- a/types/map.go +++ b/types/map.go @@ -133,6 +133,7 @@ func (m MapType) Validate(ctx context.Context, in tftypes.Value, path path.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() { From 69866565ffb42aa7a97ed585d129f8c56a77b60d Mon Sep 17 00:00:00 2001 From: Doridian Date: Wed, 14 Sep 2022 07:15:28 -0700 Subject: [PATCH 08/10] add changelog --- .changelog/481.txt | 3 +++ 1 file changed, 3 insertions(+) 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 +``` From 9364d1b2503930945b6cf42919fa141a18618891 Mon Sep 17 00:00:00 2001 From: Doridian Date: Wed, 14 Sep 2022 08:05:20 -0700 Subject: [PATCH 09/10] Fix AtSetValue and fix tests --- internal/fwserver/attribute_validation_test.go | 6 ------ types/set.go | 11 ++++++++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/fwserver/attribute_validation_test.go b/internal/fwserver/attribute_validation_test.go index 10b8a74a5..423080c48 100644 --- a/internal/fwserver/attribute_validation_test.go +++ b/internal/fwserver/attribute_validation_test.go @@ -147,12 +147,6 @@ func TestAttributeValidate(t *testing.T) { "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\">", ), - 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"+ - "can't unmarshal tftypes.String into *[]tftypes.Value expected []tftypes.Value", - ), }, }, }, diff --git a/types/set.go b/types/set.go index 89d90fb2a..b7295c4c4 100644 --- a/types/set.go +++ b/types/set.go @@ -161,7 +161,16 @@ func (st SetType) Validate(ctx context.Context, in tftypes.Value, path path.Path // Validate the element first if isValidatable { - diags = append(diags, validatableType.Validate(ctx, elemOuter, path.AtSetValue(elemOuter))...) + 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 From cd97011cd6c9240cf465baf2ddba6aed2f4c198b Mon Sep 17 00:00:00 2001 From: Doridian Date: Wed, 14 Sep 2022 17:00:37 -0700 Subject: [PATCH 10/10] add test cases --- types/list_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++ types/map_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++ types/set_test.go | 15 ++++++++++++ 3 files changed, 131 insertions(+) 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_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_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