Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Validate() to all collections #481

Merged
merged 13 commits into from Sep 15, 2022
3 changes: 3 additions & 0 deletions .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
```
6 changes: 3 additions & 3 deletions internal/fwserver/attribute_validation_test.go
Expand Up @@ -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\">",
),
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/fwserver/schema_plan_modification_test.go
Expand Up @@ -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",
),
Expand Down Expand Up @@ -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",
),
Expand Down
2 changes: 1 addition & 1 deletion internal/reflect/map.go
Expand Up @@ -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() {
Expand Down
51 changes: 51 additions & 0 deletions types/list.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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(),
)
Doridian marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
58 changes: 58 additions & 0 deletions types/list_test.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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]<tftypes.String<\"testvalue\">>",
),
},
},
"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)
}
})
}
}
51 changes: 51 additions & 0 deletions types/map.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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{}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ensure there is a covering unit test for this new conditional. 👍

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(),
)
Doridian marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
58 changes: 58 additions & 0 deletions types/map_test.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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]<tftypes.String<\"testvalue\">>",
),
},
},
"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)
}
})
}
}
19 changes: 18 additions & 1 deletion types/set.go
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ensure there is a covering unit test for this change. 👍


// 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]

Expand Down
15 changes: 15 additions & 0 deletions types/set_test.go
Expand Up @@ -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]<tftypes.String<\"testvalue\">>",
),
},
},
}
for name, testCase := range testCases {
name, testCase := name, testCase
Expand Down