From 3ebf79355b400aa4b25fce4fed61c8e73b08f3cd Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 7 Jan 2022 13:58:56 -0500 Subject: [PATCH 1/2] tftypes: Allow DynamicPseudoType with known values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reference: https://github.com/hashicorp/terraform-plugin-go/pull/94 Reference: https://github.com/hashicorp/terraform-plugin-go/pull/99 Reference: https://github.com/hashicorp/terraform-plugin-go/pull/100 Reference: https://github.com/hashicorp/terraform-plugin-go/issues/128 Reference: https://github.com/hashicorp/terraform-plugin-go/issues/133 Reverts incorrect logic for handling DynamicPseudoType values in `tftypes`. This type information must be preserved when traversing the protocol, as Terraform CLI decodes values based on the schema information. If a concrete value type is used where DynamicPseudoType is expected, Terraform CLI will return errors such as (given an object of 4 attributes, when DynamicPseudoType is expected): ``` │ Error: ["manifest"]: msgpack: invalid code=84 decoding array length ``` --- .changelog/pending.txt | 3 ++ tftypes/diff.go | 3 ++ tftypes/list.go | 4 +- tftypes/map.go | 4 +- tftypes/object.go | 4 +- tftypes/primitive.go | 53 ++++++++++++++++++++- tftypes/set.go | 4 +- tftypes/tuple.go | 4 +- tftypes/value.go | 11 +++-- tftypes/value_dpt_test.go | 90 +++++++++++++++++++++++++++++++++-- tftypes/value_json.go | 22 ++------- tftypes/value_json_test.go | 6 +-- tftypes/value_msgpack.go | 26 ++-------- tftypes/value_msgpack_test.go | 8 ++-- tftypes/value_test.go | 10 ++++ 15 files changed, 179 insertions(+), 73 deletions(-) create mode 100644 .changelog/pending.txt diff --git a/.changelog/pending.txt b/.changelog/pending.txt new file mode 100644 index 00000000..87893f04 --- /dev/null +++ b/.changelog/pending.txt @@ -0,0 +1,3 @@ +```release-note:bug +tftypes: Fixed regression with DynamicPseudoType handling since v0.4.0, allowing usage of known values again and preventing msgpack decoding errors in Terraform CLI +``` diff --git a/tftypes/diff.go b/tftypes/diff.go index dc89026c..328b1adf 100644 --- a/tftypes/diff.go +++ b/tftypes/diff.go @@ -283,6 +283,9 @@ func (val1 Value) Diff(val2 Value) ([]ValueDiff, error) { // if we have the same keys, we can just let recursion // from the walk check the sub-values match return true, nil + case value1.Type().Is(DynamicPseudoType): + // Let recursion from the walk check the sub-values match + return true, nil } return false, fmt.Errorf("unexpected type %v in Diff at %s", value1.Type(), path) }) diff --git a/tftypes/list.go b/tftypes/list.go index a633ed1e..820efbcd 100644 --- a/tftypes/list.go +++ b/tftypes/list.go @@ -70,9 +70,7 @@ func valueFromList(typ Type, in interface{}) (Value, error) { case []Value: var valType Type for pos, v := range value { - if v.Type().Is(DynamicPseudoType) && v.IsKnown() { - return Value{}, NewAttributePath().WithElementKeyInt(pos).NewErrorf("invalid value %s for %s", v, v.Type()) - } else if !v.Type().Is(DynamicPseudoType) && !v.Type().UsableAs(typ) { + if !v.Type().UsableAs(typ) { return Value{}, NewAttributePath().WithElementKeyInt(pos).NewErrorf("can't use %s as %s", v.Type(), typ) } if valType == nil { diff --git a/tftypes/map.go b/tftypes/map.go index 9188df88..ecc6bed0 100644 --- a/tftypes/map.go +++ b/tftypes/map.go @@ -89,9 +89,7 @@ func valueFromMap(typ Type, in interface{}) (Value, error) { var elType Type for _, k := range keys { v := value[k] - if v.Type().Is(DynamicPseudoType) && v.IsKnown() { - return Value{}, NewAttributePath().WithElementKeyString(k).NewErrorf("invalid value %s for %s", v, v.Type()) - } else if !v.Type().Is(DynamicPseudoType) && !v.Type().UsableAs(typ) { + if !v.Type().UsableAs(typ) { return Value{}, NewAttributePath().WithElementKeyString(k).NewErrorf("can't use %s as %s", v.Type(), typ) } if elType == nil { diff --git a/tftypes/object.go b/tftypes/object.go index 2c9dde76..418b464f 100644 --- a/tftypes/object.go +++ b/tftypes/object.go @@ -183,9 +183,7 @@ func valueFromObject(types map[string]Type, optionalAttrs map[string]struct{}, i if v.Type() == nil { return Value{}, NewAttributePath().WithAttributeName(k).NewErrorf("missing value type") } - if v.Type().Is(DynamicPseudoType) && v.IsKnown() && !v.IsNull() { - return Value{}, NewAttributePath().WithAttributeName(k).NewErrorf("invalid value %s for %s", v, v.Type()) - } else if !v.Type().Is(DynamicPseudoType) && !v.Type().UsableAs(typ) { + if !v.Type().UsableAs(typ) { return Value{}, NewAttributePath().WithAttributeName(k).NewErrorf("can't use %s as %s", v.Type(), typ) } } diff --git a/tftypes/primitive.go b/tftypes/primitive.go index ff0f14e6..a2815c04 100644 --- a/tftypes/primitive.go +++ b/tftypes/primitive.go @@ -102,7 +102,16 @@ func (p primitive) supportedGoTypes() []string { case Bool.name: return []string{"bool", "*bool"} case DynamicPseudoType.name: - return []string{"nil", "UnknownValue"} + // List/Set is covered by Tuple, Map is covered by Object + possibleTypes := []Type{ + String, Bool, Number, + Tuple{}, Object{}, + } + results := []string{} + for _, t := range possibleTypes { + results = append(results, t.supportedGoTypes()...) + } + return results } panic(fmt.Sprintf("unknown primitive type %q", p.name)) } @@ -346,3 +355,45 @@ func valueFromNumber(in interface{}) (Value, error) { return Value{}, fmt.Errorf("tftypes.NewValue can't use %T as a tftypes.Number; expected types are: %s", in, formattedSupportedGoTypes(Number)) } } + +func valueFromDynamicPseudoType(val interface{}) (Value, error) { + switch val := val.(type) { + case string, *string: + v, err := valueFromString(val) + if err != nil { + return Value{}, err + } + v.typ = DynamicPseudoType + return v, nil + case *big.Float, float64, *float64, int, *int, int8, *int8, int16, *int16, int32, *int32, int64, *int64, uint, *uint, uint8, *uint8, uint16, *uint16, uint32, *uint32, uint64, *uint64: + v, err := valueFromNumber(val) + if err != nil { + return Value{}, err + } + v.typ = DynamicPseudoType + return v, nil + case bool, *bool: + v, err := valueFromBool(val) + if err != nil { + return Value{}, err + } + v.typ = DynamicPseudoType + return v, nil + case map[string]Value: + v, err := valueFromObject(nil, nil, val) + if err != nil { + return Value{}, err + } + v.typ = DynamicPseudoType + return v, nil + case []Value: + v, err := valueFromTuple(nil, val) + if err != nil { + return Value{}, err + } + v.typ = DynamicPseudoType + return v, nil + default: + return Value{}, fmt.Errorf("tftypes.NewValue can't use %T as a tftypes.DynamicPseudoType; expected types are: %s", val, formattedSupportedGoTypes(DynamicPseudoType)) + } +} diff --git a/tftypes/set.go b/tftypes/set.go index 3bd3e8a6..28ab23b7 100644 --- a/tftypes/set.go +++ b/tftypes/set.go @@ -70,9 +70,7 @@ func valueFromSet(typ Type, in interface{}) (Value, error) { case []Value: var elType Type for _, v := range value { - if v.Type().Is(DynamicPseudoType) && v.IsKnown() { - return Value{}, NewAttributePath().WithElementKeyValue(v).NewErrorf("invalid value %s for %s", v, v.Type()) - } else if !v.Type().Is(DynamicPseudoType) && !v.Type().UsableAs(typ) { + if !v.Type().UsableAs(typ) { return Value{}, NewAttributePath().WithElementKeyValue(v).NewErrorf("can't use %s as %s", v.Type(), typ) } if elType == nil { diff --git a/tftypes/tuple.go b/tftypes/tuple.go index 088243bb..df57d90d 100644 --- a/tftypes/tuple.go +++ b/tftypes/tuple.go @@ -109,9 +109,7 @@ func valueFromTuple(types []Type, in interface{}) (Value, error) { } for pos, v := range value { typ := types[pos] - if v.Type().Is(DynamicPseudoType) && v.IsKnown() { - return Value{}, NewAttributePath().WithElementKeyInt(pos).NewErrorf("invalid value %s for %s", v, v.Type()) - } else if !v.Type().Is(DynamicPseudoType) && !v.Type().UsableAs(typ) { + if !v.Type().UsableAs(typ) { return Value{}, NewAttributePath().WithElementKeyInt(pos).NewErrorf("can't use %s as %s", v.Type(), typ) } } diff --git a/tftypes/value.go b/tftypes/value.go index 1c0d2545..c2c26feb 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -2,7 +2,6 @@ package tftypes import ( "bytes" - "errors" "fmt" "math/big" "sort" @@ -296,10 +295,6 @@ func newValue(t Type, val interface{}) (Value, error) { }, nil } - if t.Is(DynamicPseudoType) { - return Value{}, errors.New("cannot have DynamicPseudoType with known value, DynamicPseudoType can only contain null or unknown values") - } - if creator, ok := val.(ValueCreator); ok { var err error val, err = creator.ToTerraform5Value() @@ -357,6 +352,12 @@ func newValue(t Type, val interface{}) (Value, error) { return Value{}, err } return v, nil + case t.Is(DynamicPseudoType): + v, err := valueFromDynamicPseudoType(val) + if err != nil { + return Value{}, err + } + return v, nil default: return Value{}, fmt.Errorf("unknown type %s passed to tftypes.NewValue", t) } diff --git a/tftypes/value_dpt_test.go b/tftypes/value_dpt_test.go index c7d78acc..471843a0 100644 --- a/tftypes/value_dpt_test.go +++ b/tftypes/value_dpt_test.go @@ -1,6 +1,7 @@ package tftypes import ( + "math/big" "regexp" "testing" ) @@ -14,10 +15,93 @@ func Test_newValue_dpt(t *testing.T) { expected Value } tests := map[string]testCase{ - "known": { + "*big.Float": { typ: DynamicPseudoType, - val: "hello", - err: regexp.MustCompile(`cannot have DynamicPseudoType with known value, DynamicPseudoType can only contain null or unknown values`), + val: big.NewFloat(123), + expected: Value{ + typ: DynamicPseudoType, + value: Value{ + typ: Number, + value: big.NewFloat(123), + }, + }, + }, + "bool": { + typ: DynamicPseudoType, + val: true, + expected: Value{ + typ: DynamicPseudoType, + value: true, + }, + }, + "float64": { + typ: DynamicPseudoType, + val: float64(123), + expected: Value{ + typ: DynamicPseudoType, + value: Value{ + typ: Number, + value: big.NewFloat(123), + }, + }, + }, + "int": { + typ: DynamicPseudoType, + val: 123, + expected: Value{ + typ: DynamicPseudoType, + value: Value{ + typ: Number, + value: big.NewFloat(123), + }, + }, + }, + "int64": { + typ: DynamicPseudoType, + val: int64(123), + expected: Value{ + typ: DynamicPseudoType, + value: Value{ + typ: Number, + value: big.NewFloat(123), + }, + }, + }, + "object": { + typ: DynamicPseudoType, + val: map[string]Value{ + "testkey": NewValue(String, "testvalue"), + }, + expected: Value{ + typ: DynamicPseudoType, + value: map[string]Value{ + "testkey": { + typ: String, + value: "testvalue", + }, + }, + }, + }, + "string": { + typ: DynamicPseudoType, + val: "test", + expected: Value{ + typ: DynamicPseudoType, + value: "test", + }, + }, + "tuple": { + typ: DynamicPseudoType, + val: []Value{NewValue(String, "test")}, + expected: Value{ + typ: DynamicPseudoType, + value: []Value{ + { + typ: String, + value: "test", + }, + }, + }, }, "null": { typ: DynamicPseudoType, diff --git a/tftypes/value_json.go b/tftypes/value_json.go index c96401ad..6a61ad8d 100644 --- a/tftypes/value_json.go +++ b/tftypes/value_json.go @@ -355,20 +355,8 @@ func jsonUnmarshalMap(buf []byte, attrType Type, p *AttributePath) (Value, error return Value{}, p.NewErrorf("invalid JSON, expected %q, got %q", json.Delim('}'), tok) } - elTyp := attrType - if attrType.Is(DynamicPseudoType) { - var elements []Value - for _, val := range vals { - elements = append(elements, val) - } - elTyp, err = TypeFromElements(elements) - if err != nil { - return Value{}, p.NewErrorf("invalid elements for map: %w", err) - } - } - return NewValue(Map{ - ElementType: elTyp, + ElementType: attrType, }, vals), nil } @@ -393,7 +381,6 @@ func jsonUnmarshalTuple(buf []byte, elementTypes []Type, p *AttributePath) (Valu // while generally in Go it's undesirable to treat empty and nil slices // separately, in this case we're surfacing a non-Go-in-origin // distinction, so we'll allow it. - types := []Type{} vals := []Value{} var idx int @@ -415,7 +402,6 @@ func jsonUnmarshalTuple(buf []byte, elementTypes []Type, p *AttributePath) (Valu if err != nil { return Value{}, err } - types = append(types, val.Type()) vals = append(vals, val) } @@ -432,7 +418,7 @@ func jsonUnmarshalTuple(buf []byte, elementTypes []Type, p *AttributePath) (Valu } return NewValue(Tuple{ - ElementTypes: types, + ElementTypes: elementTypes, }, vals), nil } @@ -447,7 +433,6 @@ func jsonUnmarshalObject(buf []byte, attrTypes map[string]Type, p *AttributePath return Value{}, p.NewErrorf("invalid JSON, expected %q, got %q", json.Delim('{'), tok) } - types := map[string]Type{} vals := map[string]Value{} for dec.More() { innerPath := p.WithElementKeyValue(NewValue(String, UnknownValue)) @@ -474,7 +459,6 @@ func jsonUnmarshalObject(buf []byte, attrTypes map[string]Type, p *AttributePath if err != nil { return Value{}, err } - types[key] = val.Type() vals[key] = val } @@ -494,6 +478,6 @@ func jsonUnmarshalObject(buf []byte, attrTypes map[string]Type, p *AttributePath } return NewValue(Object{ - AttributeTypes: types, + AttributeTypes: attrTypes, }, vals), nil } diff --git a/tftypes/value_json_test.go b/tftypes/value_json_test.go index 22fdadc4..4e552f41 100644 --- a/tftypes/value_json_test.go +++ b/tftypes/value_json_test.go @@ -302,7 +302,7 @@ func TestValueFromJSON(t *testing.T) { }, "tuple-of-dynamic-bools": { value: NewValue(Tuple{ - ElementTypes: []Type{Bool, Bool}, + ElementTypes: []Type{DynamicPseudoType, DynamicPseudoType}, }, []Value{ NewValue(Bool, true), NewValue(Bool, false), @@ -324,7 +324,7 @@ func TestValueFromJSON(t *testing.T) { }, "map-of-dynamic-bools": { value: NewValue(Map{ - ElementType: Bool, + ElementType: DynamicPseudoType, }, map[string]Value{ "true": NewValue(Bool, true), "false": NewValue(Bool, false), @@ -338,7 +338,7 @@ func TestValueFromJSON(t *testing.T) { value: NewValue(Object{ AttributeTypes: map[string]Type{ "static": Bool, - "dynamic": Bool, + "dynamic": DynamicPseudoType, }, }, map[string]Value{ "static": NewValue(Bool, true), diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index d353d86d..3495efe9 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -239,24 +239,8 @@ func msgpackUnmarshalMap(dec *msgpack.Decoder, typ Type, path *AttributePath) (V vals[key] = val } - elTyp := typ - - if typ.Is(DynamicPseudoType) { - var elements []Value - - for _, val := range vals { - elements = append(elements, val) - } - - elTyp, err = TypeFromElements(elements) - - if err != nil { - return Value{}, path.NewErrorf("invalid elements for map: %w", err) - } - } - return NewValue(Map{ - ElementType: elTyp, + ElementType: typ, }, vals), nil } @@ -275,7 +259,6 @@ func msgpackUnmarshalTuple(dec *msgpack.Decoder, types []Type, path *AttributePa return Value{}, path.NewErrorf("error decoding tuple; expected %d items, got %d", len(types), length) } - elTypes := make([]Type, 0, length) vals := make([]Value, 0, length) for i := 0; i < length; i++ { innerPath := path.WithElementKeyInt(i) @@ -284,12 +267,11 @@ func msgpackUnmarshalTuple(dec *msgpack.Decoder, types []Type, path *AttributePa if err != nil { return Value{}, err } - elTypes = append(elTypes, val.Type()) vals = append(vals, val) } return NewValue(Tuple{ - ElementTypes: elTypes, + ElementTypes: types, }, vals), nil } @@ -308,7 +290,6 @@ func msgpackUnmarshalObject(dec *msgpack.Decoder, types map[string]Type, path *A return Value{}, path.NewErrorf("error decoding object; expected %d attributes, got %d", len(types), length) } - attrTypes := make(map[string]Type, length) vals := make(map[string]Value, length) for i := 0; i < length; i++ { key, err := dec.DecodeString() @@ -324,12 +305,11 @@ func msgpackUnmarshalObject(dec *msgpack.Decoder, types map[string]Type, path *A if err != nil { return Value{}, err } - attrTypes[key] = val.Type() vals[key] = val } return NewValue(Object{ - AttributeTypes: attrTypes, + AttributeTypes: types, }, vals), nil } diff --git a/tftypes/value_msgpack_test.go b/tftypes/value_msgpack_test.go index 66c2617b..eaf21087 100644 --- a/tftypes/value_msgpack_test.go +++ b/tftypes/value_msgpack_test.go @@ -127,7 +127,7 @@ func TestValueFromMsgPack(t *testing.T) { hex: "81a86772656574696e6792c40822737472696e6722a568656c6c6f", value: NewValue(Object{ AttributeTypes: map[string]Type{ - "greeting": String, + "greeting": DynamicPseudoType, }, }, map[string]Value{ "greeting": NewValue(String, "hello"), @@ -248,7 +248,7 @@ func TestValueFromMsgPack(t *testing.T) { "map-dynamic": { hex: "81a86772656574696e6792c40822737472696e6722a568656c6c6f", value: NewValue(Map{ - ElementType: String, + ElementType: DynamicPseudoType, }, map[string]Value{ "greeting": NewValue(String, "hello"), }), @@ -291,7 +291,7 @@ func TestValueFromMsgPack(t *testing.T) { "tuple-dynamic": { hex: "9292c40822737472696e6722a568656c6c6f92c40822737472696e6722a5776f726c64", value: NewValue(Tuple{ - ElementTypes: []Type{String, String}, + ElementTypes: []Type{DynamicPseudoType, DynamicPseudoType}, }, []Value{ NewValue(String, "hello"), NewValue(String, "world"), @@ -336,7 +336,7 @@ func TestValueFromMsgPack(t *testing.T) { hex: "81a86772656574696e6792c40822737472696e6722a568656c6c6f", value: NewValue(Object{ AttributeTypes: map[string]Type{ - "greeting": String, + "greeting": DynamicPseudoType, }, }, map[string]Value{ "greeting": NewValue(String, "hello"), diff --git a/tftypes/value_test.go b/tftypes/value_test.go index cae83a61..9f5e5d6f 100644 --- a/tftypes/value_test.go +++ b/tftypes/value_test.go @@ -415,6 +415,16 @@ func TestValueAs(t *testing.T) { }}, []Value{}), }), }, + "dynamic": { + in: NewValue(DynamicPseudoType, "hello"), + as: strPointer(""), + expected: strPointer("hello"), + }, + "dynamic-null": { + in: NewValue(DynamicPseudoType, nil), + as: strPointer(""), + expected: strPointer(""), + }, } for name, test := range tests { From 915b1ba7241b3a675fa55ca918f8c899571c502b Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 7 Jan 2022 15:04:59 -0500 Subject: [PATCH 2/2] Update CHANGELOG for #136 --- .changelog/{pending.txt => 136.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{pending.txt => 136.txt} (100%) diff --git a/.changelog/pending.txt b/.changelog/136.txt similarity index 100% rename from .changelog/pending.txt rename to .changelog/136.txt