Skip to content

Commit

Permalink
cty: add CanListVal, CanSetVal, CanMapVal
Browse files Browse the repository at this point in the history
The logic which may result in a panic in List(), Map() and Set() has been extracted into these helper functions, which return bools. The original callers are unchanged, so that they can still return an element-specific panic message.

These new functions are called in the appropriate conversion functions in the convert package to validate that the given elements can be used to create a single Type, to help reduce the number of panics that occur when calling Convert.

For additional background information on the (odd) scenarios that can lead to these panics, see hashicorp/terraform#26195
  • Loading branch information
mildwonkey committed Mar 10, 2021
1 parent fcc7075 commit 8bbb856
Show file tree
Hide file tree
Showing 3 changed files with 383 additions and 24 deletions.
47 changes: 23 additions & 24 deletions cty/convert/conversion_collection.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package convert

import (
"fmt"

"github.com/zclconf/go-cty/cty"
)

Expand Down Expand Up @@ -51,6 +53,10 @@ func conversionCollectionToList(ety cty.Type, conv conversion) conversion {
return cty.ListValEmpty(ety), nil
}

if !cty.CanListVal(elems) {
return cty.NilVal, fmt.Errorf("List elements don't match")
}

return cty.ListVal(elems), nil
}
}
Expand Down Expand Up @@ -96,6 +102,10 @@ func conversionCollectionToSet(ety cty.Type, conv conversion) conversion {
return cty.SetValEmpty(ety), nil
}

if !cty.CanSetVal(elems) {
return cty.NilVal, fmt.Errorf("Set elements don't match")
}

return cty.SetVal(elems), nil
}
}
Expand Down Expand Up @@ -152,8 +162,8 @@ func conversionCollectionToMap(ety cty.Type, conv conversion) conversion {
}
}

if err := conversionCheckMapElementTypes(elems, path); err != nil {
return cty.NilVal, err
if !cty.CanMapVal(elems) {
return cty.NilVal, fmt.Errorf("Map elements don't match")
}

return cty.MapVal(elems), nil
Expand Down Expand Up @@ -237,6 +247,10 @@ func conversionTupleToSet(tupleType cty.Type, setEty cty.Type, unsafe bool) conv
i++
}

if !cty.CanSetVal(elems) {
return cty.NilVal, fmt.Errorf("Set elements don't match")
}

return cty.SetVal(elems), nil
}
}
Expand Down Expand Up @@ -324,6 +338,11 @@ func conversionTupleToList(tupleType cty.Type, listEty cty.Type, unsafe bool) co
if err != nil {
return cty.NilVal, err
}

if !cty.CanListVal(elems) {
return cty.NilVal, fmt.Errorf("List elements don't match")
}

return cty.ListVal(elems), nil
}
}
Expand Down Expand Up @@ -402,8 +421,8 @@ func conversionObjectToMap(objectType cty.Type, mapEty cty.Type, unsafe bool) co
}
}

if err := conversionCheckMapElementTypes(elems, path); err != nil {
return cty.NilVal, err
if !cty.CanMapVal(elems) {
return cty.NilVal, fmt.Errorf("Map elements don't match")
}

return cty.MapVal(elems), nil
Expand Down Expand Up @@ -514,26 +533,6 @@ func conversionUnifyCollectionElements(elems map[string]cty.Value, path cty.Path
return unifiedElems, nil
}

func conversionCheckMapElementTypes(elems map[string]cty.Value, path cty.Path) error {
elementType := cty.NilType
elemPath := append(path.Copy(), nil)

for name, elem := range elems {
if elementType == cty.NilType {
elementType = elem.Type()
continue
}
if !elementType.Equals(elem.Type()) {
elemPath[len(elemPath)-1] = cty.IndexStep{
Key: cty.StringVal(name),
}
return elemPath.NewErrorf("%s is required", elementType.FriendlyName())
}
}

return nil
}

func conversionUnifyListElements(elems []cty.Value, path cty.Path, unsafe bool) ([]cty.Value, error) {
elemTypes := make([]cty.Type, len(elems))
for i, elem := range elems {
Expand Down
48 changes: 48 additions & 0 deletions cty/value_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,20 @@ func ListValEmpty(element Type) Value {
}
}

// CanListVal returns false if the given Values can not be coalesced
// into a single List due to inconsistent element types.
func CanListVal(vals []Value) bool {
elementType := DynamicPseudoType
for _, val := range vals {
if elementType == DynamicPseudoType {
elementType = val.ty
} else if val.ty != DynamicPseudoType && !elementType.Equals(val.ty) {
return false
}
}
return true
}

// MapVal returns a Value of a map type whose element type is defined by
// the types of the given values, which must be homogenous.
//
Expand Down Expand Up @@ -227,6 +241,20 @@ func MapValEmpty(element Type) Value {
}
}

// CanMapVal returns false if the given Values can not be coalesced into a
// single Map due to inconsistent element types.
func CanMapVal(vals map[string]Value) bool {
elementType := DynamicPseudoType
for _, val := range vals {
if elementType == DynamicPseudoType {
elementType = val.ty
} else if val.ty != DynamicPseudoType && !elementType.Equals(val.ty) {
return false
}
}
return true
}

// SetVal returns a Value of set type whose element type is defined by
// the types of the given values, which must be homogenous.
//
Expand Down Expand Up @@ -267,6 +295,26 @@ func SetVal(vals []Value) Value {
}.WithMarks(markSets...)
}

// CanSetVal returns false if the given Values can not be coalesced
// into a single Set due to inconsistent element types.
func CanSetVal(vals []Value) bool {
elementType := DynamicPseudoType
var markSets []ValueMarks

for _, val := range vals {
if unmarkedVal, marks := val.UnmarkDeep(); len(marks) > 0 {
val = unmarkedVal
markSets = append(markSets, marks)
}
if elementType == DynamicPseudoType {
elementType = val.ty
} else if val.ty != DynamicPseudoType && !elementType.Equals(val.ty) {
return false
}
}
return true
}

// SetValFromValueSet returns a Value of set type based on an already-constructed
// ValueSet.
//
Expand Down

0 comments on commit 8bbb856

Please sign in to comment.