Skip to content

Commit

Permalink
Fix NewValue checks to be properly recursive (#79)
Browse files Browse the repository at this point in the history
Fix the checks that NewValue runs to ensure that a value can be used
with a Type to properly handle DynamicPseudoTypes nested in complex
types. Builds on and supersedes #76.

Should we make this part of the behavior for `.Is`? My gut says "no",
because `Is` is supposed to be about checking whether a type is
semantically similar, not that it can be used as another type. Maybe we
want a `Fulfills` method, similar to Is, that does this? I don't know
that this behavior needs to be exported, though...

Fixes a bug in #74 that would panic for complex types containing
DynamicPseudoTypes.

Co-authored-by: Paul Tyng <[email protected]>
  • Loading branch information
paddycarver and paultyng authored Apr 21, 2021
1 parent e0e351e commit 012c7f0
Show file tree
Hide file tree
Showing 12 changed files with 660 additions and 16 deletions.
11 changes: 9 additions & 2 deletions tftypes/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,16 @@ func (l List) supportedGoTypes() []string {
func valueFromList(typ Type, in interface{}) (Value, error) {
switch value := in.(type) {
case []Value:
var valType Type
for pos, v := range value {
if !v.Type().Is(typ) && !typ.Is(DynamicPseudoType) {
return Value{}, fmt.Errorf("tftypes.NewValue can't use type %s as a value in position %d of %s; expected type is %s", v.Type(), pos, List{ElementType: typ}, typ)
if err := useTypeAs(v.Type(), typ, NewAttributePath().WithElementKeyInt(int64(pos))); err != nil {
return Value{}, err
}
if valType == nil {
valType = v.Type()
}
if !v.Type().equals(valType, true) {
return Value{}, fmt.Errorf("lists must only contain one type of element, saw %s and %s", valType, v.Type())
}
}
return Value{
Expand Down
25 changes: 20 additions & 5 deletions tftypes/map.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package tftypes

import "fmt"
import (
"fmt"
"sort"
)

// Map is a Terraform type representing an unordered collection of elements,
// all of the same type, each identifiable with a unique string key.
Expand Down Expand Up @@ -75,10 +78,22 @@ func (m Map) MarshalJSON() ([]byte, error) {
func valueFromMap(typ Type, in interface{}) (Value, error) {
switch value := in.(type) {
case map[string]Value:
for k, v := range value {
if !v.Type().Is(typ) && !typ.Is(DynamicPseudoType) {
// TODO: make this an attribute path error?
return Value{}, fmt.Errorf("tftypes.NewValue can't use type %s as a value for %q in %s; expected type is %s", v.Type(), k, Map{AttributeType: typ}, typ)
keys := make([]string, 0, len(value))
for k := range value {
keys = append(keys, k)
}
sort.Strings(keys)
var elType Type
for _, k := range keys {
v := value[k]
if err := useTypeAs(v.Type(), typ, NewAttributePath().WithElementKeyString(k)); err != nil {
return Value{}, err
}
if elType == nil {
elType = v.Type()
}
if !elType.equals(v.Type(), true) {
return Value{}, fmt.Errorf("maps must only contain one type of element, saw %s and %s", elType, v.Type())
}
}
return Value{
Expand Down
5 changes: 3 additions & 2 deletions tftypes/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,9 @@ func valueFromObject(types map[string]Type, optionalAttrs map[string]struct{}, i
if !ok {
return Value{}, fmt.Errorf("can't set a value on %q in tftypes.NewValue, key not part of the object type %s", k, Object{AttributeTypes: types})
}
if !v.Type().Is(types[k]) && !types[k].Is(DynamicPseudoType) {
return Value{}, fmt.Errorf("tftypes.NewValue can't use type %s as a value for %q in %s; expected type is %s", v.Type(), k, Object{AttributeTypes: types}, typ)
err := useTypeAs(v.Type(), typ, NewAttributePath().WithAttributeName(k))
if err != nil {
return Value{}, err
}
}
}
Expand Down
13 changes: 10 additions & 3 deletions tftypes/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,16 @@ func (s Set) supportedGoTypes() []string {
func valueFromSet(typ Type, in interface{}) (Value, error) {
switch value := in.(type) {
case []Value:
for pos, v := range value {
if !v.Type().Is(typ) && !typ.Is(DynamicPseudoType) {
return Value{}, fmt.Errorf("tftypes.NewValue can't use type %s as a value in position %d of %s; expected type is %s", v.Type(), pos, Set{ElementType: typ}, typ)
var elType Type
for _, v := range value {
if err := useTypeAs(v.Type(), typ, NewAttributePath().WithElementKeyValue(v)); err != nil {
return Value{}, err
}
if elType == nil {
elType = v.Type()
}
if !elType.equals(v.Type(), true) {
return Value{}, fmt.Errorf("sets must only contain one type of element, saw %s and %s", elType, v.Type())
}
}
return Value{
Expand Down
5 changes: 3 additions & 2 deletions tftypes/tuple.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@ func valueFromTuple(types []Type, in interface{}) (Value, error) {
}
for pos, v := range value {
typ := types[pos]
if !v.Type().Is(typ) && !typ.Is(DynamicPseudoType) {
return Value{}, fmt.Errorf("tftypes.NewValue can't use type %s as a value at position %d in %s; expected type is %s", v.Type(), pos, Tuple{ElementTypes: types}, typ)
err := useTypeAs(v.Type(), typ, NewAttributePath().WithElementKeyInt(int64(pos)))
if err != nil {
return Value{}, err
}
}
}
Expand Down
69 changes: 69 additions & 0 deletions tftypes/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,75 @@ func TypeFromElements(elements []Value) (Type, error) {
return typ, nil
}

func useTypeAs(candidate, usedAs Type, path *AttributePath) error {
switch {
case usedAs.Is(DynamicPseudoType):
return nil
case usedAs.Is(String), usedAs.Is(Bool), usedAs.Is(Number):
if candidate.Is(usedAs) {
return nil
}
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
case usedAs.Is(List{}):
if !candidate.Is(List{}) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
return useTypeAs(candidate.(List).ElementType, usedAs.(List).ElementType, path.WithElementKeyInt(0))
case usedAs.Is(Set{}):
if !candidate.Is(Set{}) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
return useTypeAs(candidate.(Set).ElementType, usedAs.(Set).ElementType, path.WithElementKeyValue(NewValue(DynamicPseudoType, UnknownValue)))
case usedAs.Is(Map{}):
if !candidate.Is(Map{}) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
return useTypeAs(candidate.(Map).AttributeType, usedAs.(Map).AttributeType, path.WithElementKeyString(""))
case usedAs.Is(Tuple{}):
if !candidate.Is(Tuple{}) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
cElems := candidate.(Tuple).ElementTypes
uElems := usedAs.(Tuple).ElementTypes
if len(cElems) != len(uElems) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
for pos, cElem := range cElems {
uElem := uElems[pos]
err := useTypeAs(cElem, uElem, path.WithElementKeyInt(int64(pos)))
if err != nil {
return err
}
}
case usedAs.Is(Object{}):
if !candidate.Is(Object{}) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
if len(candidate.(Object).OptionalAttributes) != len(usedAs.(Object).OptionalAttributes) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
for attr := range usedAs.(Object).OptionalAttributes {
if !candidate.(Object).attrIsOptional(attr) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
}
if len(candidate.(Object).AttributeTypes) != len(usedAs.(Object).AttributeTypes) {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
for attr, uAttr := range usedAs.(Object).AttributeTypes {
cAttr, ok := candidate.(Object).AttributeTypes[attr]
if !ok {
return path.NewErrorf("can't use %s as %s", candidate, usedAs)
}
err := useTypeAs(cAttr, uAttr, path.WithAttributeName(attr))
if err != nil {
return err
}
}
}
return nil
}

type jsonType struct {
t Type
}
Expand Down
94 changes: 94 additions & 0 deletions tftypes/value_list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package tftypes

import (
"regexp"
"testing"
)

func Test_newValue_list(t *testing.T) {
t.Parallel()
type testCase struct {
typ Type
val interface{}
err *regexp.Regexp
expected Value
}
tests := map[string]testCase{
"normal": {
typ: List{ElementType: String},
val: []Value{
NewValue(String, "hello"),
NewValue(String, "world"),
},
expected: Value{
typ: List{ElementType: String},
value: []Value{
{
typ: String,
value: "hello",
},
{
typ: String,
value: "world",
},
},
},
err: nil,
},
"dynamic": {
typ: List{ElementType: DynamicPseudoType},
val: []Value{
NewValue(String, "hello"),
NewValue(String, "world"),
},
expected: Value{
typ: List{ElementType: DynamicPseudoType},
value: []Value{
{
typ: String,
value: "hello",
},
{
typ: String,
value: "world",
},
},
},
err: nil,
},
"dynamic-different-types": {
typ: List{ElementType: DynamicPseudoType},
val: []Value{
NewValue(String, "hello"),
NewValue(Number, 123),
},
err: regexp.MustCompile(`lists must only contain one type of element, saw tftypes.String and tftypes.Number`),
},
"wrong-type": {
typ: List{ElementType: String},
val: []Value{
NewValue(String, "foo"),
NewValue(Number, 123),
},
err: regexp.MustCompile(`ElementKeyInt\(1\): can't use tftypes.Number as tftypes.String`),
},
}
for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
t.Parallel()

res, err := newValue(test.typ, test.val)
if err == nil && test.err != nil {
t.Errorf("Expected error to match %q, got nil", test.err)
} else if err != nil && test.err == nil {
t.Errorf("Expected error to be nil, got %q", err)
} else if err != nil && test.err != nil && !test.err.MatchString(err.Error()) {
t.Errorf("Expected error to match %q, got %q", test.err, err.Error())
}
if !res.Equal(test.expected) {
t.Errorf("Expected value to be %s, got %s", test.expected, res)
}
})
}
}
94 changes: 94 additions & 0 deletions tftypes/value_map_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package tftypes

import (
"regexp"
"testing"
)

func Test_newValue_map(t *testing.T) {
t.Parallel()
type testCase struct {
typ Type
val interface{}
err *regexp.Regexp
expected Value
}
tests := map[string]testCase{
"normal": {
typ: Map{AttributeType: String},
val: map[string]Value{
"a": NewValue(String, "hello"),
"b": NewValue(String, "world"),
},
expected: Value{
typ: Map{AttributeType: String},
value: map[string]Value{
"a": {
typ: String,
value: "hello",
},
"b": {
typ: String,
value: "world",
},
},
},
err: nil,
},
"dynamic": {
typ: Map{AttributeType: DynamicPseudoType},
val: map[string]Value{
"a": NewValue(String, "hello"),
"b": NewValue(String, "world"),
},
expected: Value{
typ: Map{AttributeType: DynamicPseudoType},
value: map[string]Value{
"a": {
typ: String,
value: "hello",
},
"b": {
typ: String,
value: "world",
},
},
},
err: nil,
},
"dynamic-different-types": {
typ: Map{AttributeType: DynamicPseudoType},
val: map[string]Value{
"a": NewValue(String, "hello"),
"b": NewValue(Number, 123),
},
err: regexp.MustCompile(`maps must only contain one type of element, saw tftypes.String and tftypes.Number`),
},
"wrong-type": {
typ: Map{AttributeType: String},
val: map[string]Value{
"a": NewValue(String, "foo"),
"b": NewValue(Number, 123),
},
err: regexp.MustCompile(`ElementKeyString\("b"\): can't use tftypes.Number as tftypes.String`),
},
}
for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
t.Parallel()

res, err := newValue(test.typ, test.val)
if err == nil && test.err != nil {
t.Errorf("Expected error to match %q, got nil", test.err)
} else if err != nil && test.err == nil {
t.Errorf("Expected error to be nil, got %q", err)
} else if err != nil && test.err != nil && !test.err.MatchString(err.Error()) {
t.Errorf("Expected error to match %q, got %q", test.err, err.Error())
}
if !res.Equal(test.expected) {
t.Errorf("Expected value to be %s, got %s", test.expected, res)
}
})
}
}
Loading

0 comments on commit 012c7f0

Please sign in to comment.