From 1e2a00dbd3f41704b7ad6f0f36887f91d651e3f0 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Aug 2024 11:02:03 -0400 Subject: [PATCH 01/18] skeleton implementation --- tftypes/refinement/nullness.go | 21 ++++ tftypes/refinement/refinement.go | 31 ++++++ tftypes/value.go | 22 +++++ tftypes/value_msgpack.go | 161 +++++++++++++++++++++++++++++-- 4 files changed, 225 insertions(+), 10 deletions(-) create mode 100644 tftypes/refinement/nullness.go create mode 100644 tftypes/refinement/refinement.go diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go new file mode 100644 index 000000000..d8ce0d6bc --- /dev/null +++ b/tftypes/refinement/nullness.go @@ -0,0 +1,21 @@ +package refinement + +type nullness struct { + Value bool +} + +func (n nullness) Equal(Refinement) bool { + return false +} + +func (n nullness) String() string { + return "todo - nullness" +} + +func (n nullness) unimplementable() {} + +func Nullness(value bool) Refinement { + return nullness{ + Value: value, + } +} diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go new file mode 100644 index 000000000..ed77ab66b --- /dev/null +++ b/tftypes/refinement/refinement.go @@ -0,0 +1,31 @@ +package refinement + +type Key int64 + +func (k Key) String() string { + return "todo" +} + +const ( + KeyNullness = Key(1) + // KeyStringPrefix = Key(2) + // KeyNumberLowerBound = Key(3) + // KeyNumberUpperBound = Key(4) + // KeyCollectionLengthLowerBound = Key(5) + // KeyCollectionLengthUpperBound = Key(6) +) + +type Refinement interface { + Equal(Refinement) bool + String() string + unimplementable() // prevent external implementations +} + +type Refinements map[Key]Refinement + +func (r Refinements) Equal(o Refinements) bool { + return false +} +func (r Refinements) String() string { + return "todo" +} diff --git a/tftypes/value.go b/tftypes/value.go index 63570211f..8c6aa045c 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" + "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" msgpack "github.com/vmihailenco/msgpack/v5" ) @@ -44,6 +45,8 @@ type ValueCreator interface { type Value struct { typ Type value interface{} + + refinements refinement.Refinements //nolint } func (val Value) String() string { @@ -57,6 +60,8 @@ func (val Value) String() string { if val.IsNull() { return typ.String() + "" } + + // TODO: print refinements if !val.IsKnown() { return typ.String() + "" } @@ -221,6 +226,7 @@ func (val Value) Equal(o Value) bool { if !val.Type().Equal(o.Type()) { return false } + // TODO: compare refinements deepEqual, err := val.deepEqual(o) if err != nil { return false @@ -592,3 +598,19 @@ func (val Value) MarshalMsgPack(t Type) ([]byte, error) { func unexpectedValueTypeError(p *AttributePath, expected, got interface{}, typ Type) error { return p.NewErrorf("unexpected value type %T, %s values must be of type %T", got, typ, expected) } + +// TODO: return error? +func (val Value) Refine(refinements refinement.Refinements) Value { + newVal := val.Copy() + + if len(refinements) >= 0 { + newVal.refinements = refinements + } + + return newVal +} + +func (val Value) Refinements() refinement.Refinements { + valCopy := val.Copy() + return valCopy.refinements +} diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 08fb15207..083a2b04d 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -6,14 +6,19 @@ package tftypes import ( "bytes" "fmt" + "io" "math" "math/big" "sort" + "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" msgpack "github.com/vmihailenco/msgpack/v5" msgpackCodes "github.com/vmihailenco/msgpack/v5/msgpcode" ) +// https://github.com/zclconf/go-cty/blob/main/cty/msgpack/unknown.go#L32 +const unknownWithRefinementsExt = 0x0c + type msgPackUnknownType struct{} var msgPackUnknownVal = msgPackUnknownType{} @@ -43,11 +48,7 @@ func msgpackUnmarshal(dec *msgpack.Decoder, typ Type, path *AttributePath) (Valu } if msgpackCodes.IsExt(peek) { // as with go-cty, assume all extensions are unknown values - err := dec.Skip() - if err != nil { - return Value{}, path.NewErrorf("error skipping extension byte: %w", err) - } - return NewValue(typ, UnknownValue), nil + return msgpackUnmarshalUnknown(dec, typ, path) } if typ.Is(DynamicPseudoType) { return msgpackUnmarshalDynamic(dec, path) @@ -344,6 +345,91 @@ func msgpackUnmarshalDynamic(dec *msgpack.Decoder, path *AttributePath) (Value, } return msgpackUnmarshal(dec, typ, path) } +func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath) (Value, error) { + // The value is unknown, but we will check the extension header to see if any + // type-specific refinements are applied to the value. + typeCode, extLen, err := dec.DecodeExtHeader() + if err != nil { + return Value{}, path.NewErrorf("error decoding extension header: %w", err) + } + + if extLen <= 1 { + // If the extension is zero or one-length, this is a wholly unknown value with no + // refinements. + + // TODO: Previous implementations always skipped the body, should we preserve that? + if extLen > 0 { + // Skip the body + err = dec.Skip() + if err != nil { + return Value{}, path.NewErrorf("error skipping extension byte: %w", err) + } + } + + return NewValue(typ, UnknownValue), nil + } + + // Check if the extension is the designated cty unknown refinement code + if typeCode != unknownWithRefinementsExt { + // TODO: cleanup error + return Value{}, path.NewErrorf("unsupported extension type") + } + + if extLen > 1024 { + // A refinement description greater than 1 kiB is unreasonable and + // might be an abusive attempt to allocate large amounts of memory + // in a system consuming this input. + return Value{}, path.NewErrorf("unsupported extension type") + } + + body := make([]byte, extLen) + _, err = io.ReadAtLeast(dec.Buffered(), body, len(body)) + if err != nil { + return Value{}, path.NewErrorf("failed to read msgpack extension body: %s", err) + } + + rfnDec := msgpack.NewDecoder(bytes.NewReader(body)) + entryCount, err := rfnDec.DecodeMapLen() + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: not a map") + } + + // Ignore all refinements for DynamicPseudoType for now, since go-cty also ignores this. + // + // We know that's invalid today but we might find a way to support it in the future and + // if so will want to introduce that in a backward-compatible way. + if typ.Is(DynamicPseudoType) { + return NewValue(typ, UnknownValue), nil + } + + newVal := NewValue(typ, UnknownValue) + newRefinements := make(refinement.Refinements, 0) + + for i := 0; i < entryCount; i++ { + keyCode, err := rfnDec.DecodeInt64() + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: non-integer key in map") + } + + switch keyCode := refinement.Key(keyCode); keyCode { + case refinement.KeyNullness: + isNull, err := rfnDec.DecodeBool() + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: null refinement is not boolean") + } + // The presence of this key means we're refining the null-ness one + // way or another. If nullness is unknown then this key should not + // be present at all. + newRefinements[keyCode] = refinement.Nullness(isNull) + default: + // We don't want to error here, as go-cty could introduce new refinements that we'd + // want to just ignore until this code is updated + continue + } + } + + return newVal.Refine(newRefinements), nil +} func marshalMsgPack(val Value, typ Type, p *AttributePath, enc *msgpack.Encoder) error { if typ.Is(DynamicPseudoType) && !val.Type().Is(DynamicPseudoType) { @@ -351,11 +437,7 @@ func marshalMsgPack(val Value, typ Type, p *AttributePath, enc *msgpack.Encoder) } if !val.IsKnown() { - err := enc.Encode(msgPackUnknownVal) - if err != nil { - return p.NewErrorf("error encoding UnknownValue: %w", err) - } - return nil + return marshalUnknownValue(val, typ, p, enc) } if val.IsNull() { err := enc.EncodeNil() @@ -390,6 +472,65 @@ func marshalMsgPack(val Value, typ Type, p *AttributePath, enc *msgpack.Encoder) return fmt.Errorf("unknown type %s", typ) } +func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Encoder) error { + // Use the representation of a wholly unknown value if there are no refinements. DynamicPseudoType + // cannot have refinements, so it will also use the wholly unknown value. + if len(val.refinements) == 0 || typ.Is(DynamicPseudoType) { + err := enc.Encode(msgPackUnknownVal) + if err != nil { + return p.NewErrorf("error encoding UnknownValue: %w", err) + } + return nil + } + + var refnBuf bytes.Buffer + refnEnc := msgpack.NewEncoder(&refnBuf) + mapLen := 0 + + // TODO: Should the refinement interface define the encoding? + for kind := range val.refinements { + switch kind { + case refinement.KeyNullness: + mapLen++ + refnEnc.EncodeInt(int64(kind)) //nolint + // An value that is definitely null cannot be unknown + refnEnc.EncodeBool(false) //nolint + default: + continue + } + } + + if mapLen == 0 { + // Didn't find any refinements we know how to encode, use the wholly unknown value. + err := enc.Encode(msgPackUnknownVal) + if err != nil { + return p.NewErrorf("error encoding UnknownValue: %w", err) + } + return nil + } + + // If we have at least one refinement to encode then we'll use the new + // representation of unknown values where refinement information is in the + // extension payload. + var lenBuf bytes.Buffer + lenEnc := msgpack.NewEncoder(&lenBuf) + lenEnc.EncodeMapLen(mapLen) //nolint + + err := enc.EncodeExtHeader(unknownWithRefinementsExt, lenBuf.Len()+refnBuf.Len()) + if err != nil { + return p.NewErrorf("failed to write unknown value: %s", err) + } + _, err = enc.Writer().Write(lenBuf.Bytes()) + if err != nil { + return p.NewErrorf("failed to write unknown value: %s", err) + } + _, err = enc.Writer().Write(refnBuf.Bytes()) + if err != nil { + return p.NewErrorf("failed to write unknown value: %s", err) + } + return nil +} + func marshalMsgPackDynamicPseudoType(val Value, _ Type, p *AttributePath, enc *msgpack.Encoder) error { typeJSON, err := val.Type().MarshalJSON() if err != nil { From 55b26ba04c8dca16d07d40bfa348cf532c9b190c Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Aug 2024 17:01:26 -0400 Subject: [PATCH 02/18] add string prefix refinement --- tftypes/refinement/nullness.go | 14 ++++++++++ tftypes/refinement/refinement.go | 7 +++-- tftypes/refinement/string_prefix.go | 43 +++++++++++++++++++++++++++++ tftypes/value_msgpack.go | 35 +++++++++++++++++++---- 4 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 tftypes/refinement/string_prefix.go diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go index d8ce0d6bc..77e2e69b9 100644 --- a/tftypes/refinement/nullness.go +++ b/tftypes/refinement/nullness.go @@ -1,9 +1,21 @@ package refinement +import "github.com/vmihailenco/msgpack/v5" + type nullness struct { Value bool } +func (n nullness) Encode(enc *msgpack.Encoder) error { + err := enc.EncodeInt(int64(KeyNullness)) + if err != nil { + return err + } + + // A value that is definitely null cannot be unknown + return enc.EncodeBool(false) +} + func (n nullness) Equal(Refinement) bool { return false } @@ -14,6 +26,8 @@ func (n nullness) String() string { func (n nullness) unimplementable() {} +// TODO: Should this accept a value? If a value is unknown and the it's refined to be null +// then the value should be a known value of null instead. func Nullness(value bool) Refinement { return nullness{ Value: value, diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go index ed77ab66b..e1b63d8aa 100644 --- a/tftypes/refinement/refinement.go +++ b/tftypes/refinement/refinement.go @@ -1,5 +1,7 @@ package refinement +import "github.com/vmihailenco/msgpack/v5" + type Key int64 func (k Key) String() string { @@ -7,8 +9,8 @@ func (k Key) String() string { } const ( - KeyNullness = Key(1) - // KeyStringPrefix = Key(2) + KeyNullness = Key(1) + KeyStringPrefix = Key(2) // KeyNumberLowerBound = Key(3) // KeyNumberUpperBound = Key(4) // KeyCollectionLengthLowerBound = Key(5) @@ -17,6 +19,7 @@ const ( type Refinement interface { Equal(Refinement) bool + Encode(*msgpack.Encoder) error String() string unimplementable() // prevent external implementations } diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go new file mode 100644 index 000000000..eceaae047 --- /dev/null +++ b/tftypes/refinement/string_prefix.go @@ -0,0 +1,43 @@ +package refinement + +import "github.com/vmihailenco/msgpack/v5" + +type stringPrefix struct { + Value string +} + +// TODO: What if the prefix is empty? Should we skip encoding? Throw an error earlier? Throw an error here? +func (s stringPrefix) Encode(enc *msgpack.Encoder) error { + // Matching go-cty for the max prefix length allowed here + // + // This ensures the total size of the refinements blob does not exceed the limit + // set by the decoder (1024). + maxPrefixLength := 256 + prefix := s.Value + if len(s.Value) > maxPrefixLength { + prefix = prefix[:maxPrefixLength-1] + } + + err := enc.EncodeInt(int64(KeyStringPrefix)) + if err != nil { + return err + } + + return enc.EncodeString(prefix) +} + +func (s stringPrefix) Equal(Refinement) bool { + return false +} + +func (s stringPrefix) String() string { + return "todo - stringPrefix" +} + +func (s stringPrefix) unimplementable() {} + +func StringPrefix(value string) Refinement { + return stringPrefix{ + Value: value, + } +} diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 083a2b04d..d58ff5114 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -10,6 +10,7 @@ import ( "math" "math/big" "sort" + "unicode/utf8" "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" msgpack "github.com/vmihailenco/msgpack/v5" @@ -421,9 +422,22 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath // way or another. If nullness is unknown then this key should not // be present at all. newRefinements[keyCode] = refinement.Nullness(isNull) + case refinement.KeyStringPrefix: + if !typ.Is(String) { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement for non-string type") + } + prefix, err := rfnDec.DecodeString() + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement is not string") + } + if !utf8.ValidString(prefix) { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement is not valid UTF-8") + } + + newRefinements[keyCode] = refinement.StringPrefix(prefix) default: // We don't want to error here, as go-cty could introduce new refinements that we'd - // want to just ignore until this code is updated + // want to just ignore until this logic is updated continue } } @@ -487,14 +501,23 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc refnEnc := msgpack.NewEncoder(&refnBuf) mapLen := 0 - // TODO: Should the refinement interface define the encoding? - for kind := range val.refinements { + // TODO: Should the refinement interface be defining the encoding? Should we export the refinement implementations? + for kind, refn := range val.refinements { switch kind { case refinement.KeyNullness: + err := refn.Encode(refnEnc) + if err != nil { + return p.NewErrorf("error encoding Nullness value refinement: %w", err) + } + mapLen++ + case refinement.KeyStringPrefix: + // TODO: If the prefix is empty, we shouldn't encode a refinement. This should + // probably be reflected in the interface. + err := refn.Encode(refnEnc) + if err != nil { + return p.NewErrorf("error encoding StringPrefix value refinement: %w", err) + } mapLen++ - refnEnc.EncodeInt(int64(kind)) //nolint - // An value that is definitely null cannot be unknown - refnEnc.EncodeBool(false) //nolint default: continue } From 42187731da2f79ea44c7b3b9b1818e2e1c3f1846 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 18 Oct 2024 17:34:52 -0400 Subject: [PATCH 03/18] adding some todos and copy impl --- tftypes/refinement/refinement.go | 19 ++++++++++++++++--- tftypes/refinement/string_prefix.go | 2 ++ tftypes/value.go | 20 +++++++++++++++++--- tftypes/value_msgpack.go | 5 ++++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go index e1b63d8aa..bcab735a4 100644 --- a/tftypes/refinement/refinement.go +++ b/tftypes/refinement/refinement.go @@ -1,11 +1,23 @@ package refinement -import "github.com/vmihailenco/msgpack/v5" +import ( + "fmt" + + "github.com/vmihailenco/msgpack/v5" +) type Key int64 func (k Key) String() string { - return "todo" + // TODO: Not sure when this is used, double check the names + switch k { + case KeyNullness: + return "nullness" + case KeyStringPrefix: + return "string_prefix" + default: + return fmt.Sprintf("unsupported refinement: %d", k) + } } const ( @@ -21,7 +33,7 @@ type Refinement interface { Equal(Refinement) bool Encode(*msgpack.Encoder) error String() string - unimplementable() // prevent external implementations + unimplementable() // prevents external implementations, all refinements are defined in the Terraform/HCL type system go-cty. } type Refinements map[Key]Refinement @@ -30,5 +42,6 @@ func (r Refinements) Equal(o Refinements) bool { return false } func (r Refinements) String() string { + // TODO: Not sure when this is used, should just aggregate and call all underlying refinements.String() method return "todo" } diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go index eceaae047..1eb3c9d9a 100644 --- a/tftypes/refinement/string_prefix.go +++ b/tftypes/refinement/string_prefix.go @@ -7,6 +7,8 @@ type stringPrefix struct { } // TODO: What if the prefix is empty? Should we skip encoding? Throw an error earlier? Throw an error here? +// - I think an empty prefix string is valid, empty string is still more information then wholly unknown? +// - I wonder what Terraform does in this situation today func (s stringPrefix) Encode(enc *msgpack.Encoder) error { // Matching go-cty for the max prefix length allowed here // diff --git a/tftypes/value.go b/tftypes/value.go index 8c6aa045c..4a45ee47e 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -42,11 +42,13 @@ type ValueCreator interface { // The recommended usage of a Value is to check that it is known, using // Value.IsKnown, then to convert it to a Go type, using Value.As. The Go type // can then be manipulated. +// +// TODO: add docs for new refinement data type Value struct { typ Type value interface{} - refinements refinement.Refinements //nolint + refinements refinement.Refinements } func (val Value) String() string { @@ -62,6 +64,7 @@ func (val Value) String() string { } // TODO: print refinements + if !val.IsKnown() { return typ.String() + "" } @@ -226,7 +229,9 @@ func (val Value) Equal(o Value) bool { if !val.Type().Equal(o.Type()) { return false } + // TODO: compare refinements + deepEqual, err := val.deepEqual(o) if err != nil { return false @@ -237,6 +242,9 @@ func (val Value) Equal(o Value) bool { // Copy returns a defensively-copied clone of Value that shares no underlying // data structures with the original Value and can be mutated without // accidentally mutating the original. +// +// TODO: Make sure this actually works for refinements. Consuming packages should not be able to mutate refinements of val +// TODO: Add docs referencing refinements func (val Value) Copy() Value { newVal := val.value switch v := val.value.(type) { @@ -253,7 +261,11 @@ func (val Value) Copy() Value { } newVal = newVals } - return NewValue(val.Type(), newVal) + + newTfValue := NewValue(val.Type(), newVal) + newTfValue.refinements = val.refinements + + return newTfValue } // NewValue returns a Value constructed using the specified Type and stores the @@ -599,7 +611,7 @@ func unexpectedValueTypeError(p *AttributePath, expected, got interface{}, typ T return p.NewErrorf("unexpected value type %T, %s values must be of type %T", got, typ, expected) } -// TODO: return error? +// TODO: do we need to return an error? Like if you attempt to refine a value improperly (like string prefix on a number)? func (val Value) Refine(refinements refinement.Refinements) Value { newVal := val.Copy() @@ -611,6 +623,8 @@ func (val Value) Refine(refinements refinement.Refinements) Value { } func (val Value) Refinements() refinement.Refinements { + // TODO: is this copy really needed? valCopy := val.Copy() + return valCopy.refinements } diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index d58ff5114..0eb8e13e1 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -434,6 +434,8 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement is not valid UTF-8") } + // TODO: If terraform doesn't support an empty string prefix, then neither should we, potentially return an error here. + newRefinements[keyCode] = refinement.StringPrefix(prefix) default: // We don't want to error here, as go-cty could introduce new refinements that we'd @@ -501,7 +503,8 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc refnEnc := msgpack.NewEncoder(&refnBuf) mapLen := 0 - // TODO: Should the refinement interface be defining the encoding? Should we export the refinement implementations? + // TODO: Should the refinement interface be defining the encoding? Should we export the refinement implementation details? + // - If we keep it in the interface, then we can simplify this logic for kind, refn := range val.refinements { switch kind { case refinement.KeyNullness: From 7d11d8d8e233bc516d53821f109ebe83eb50ca40 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 24 Oct 2024 17:19:10 -0400 Subject: [PATCH 04/18] small adjustments to string prefix --- tftypes/refinement/nullness.go | 32 ++++++++++++++++------------- tftypes/refinement/string_prefix.go | 26 +++++++++++++---------- tftypes/value_msgpack.go | 8 ++++++-- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go index 77e2e69b9..5fe13e905 100644 --- a/tftypes/refinement/nullness.go +++ b/tftypes/refinement/nullness.go @@ -2,34 +2,38 @@ package refinement import "github.com/vmihailenco/msgpack/v5" -type nullness struct { - Value bool +type Nullness struct { + value bool } -func (n nullness) Encode(enc *msgpack.Encoder) error { +func (n Nullness) Encode(enc *msgpack.Encoder) error { err := enc.EncodeInt(int64(KeyNullness)) if err != nil { return err } - // A value that is definitely null cannot be unknown - return enc.EncodeBool(false) + // It shouldn't be possible for an unknown value to be definitely null (i.e. nullness.value = true), + // as that should be represented by a known null value instead. This encoding is in place to be compliant + // with Terraform's encoding which uses a definitely null refinement to collapse into a known null value. + return enc.EncodeBool(n.value) } -func (n nullness) Equal(Refinement) bool { +func (n Nullness) Equal(Refinement) bool { return false } -func (n nullness) String() string { - return "todo - nullness" +func (n Nullness) String() string { + return "todo - Nullness" } -func (n nullness) unimplementable() {} +func (n Nullness) NotNull() bool { + return !n.value +} + +func (n Nullness) unimplementable() {} -// TODO: Should this accept a value? If a value is unknown and the it's refined to be null -// then the value should be a known value of null instead. -func Nullness(value bool) Refinement { - return nullness{ - Value: value, +func NewNullness(value bool) Refinement { + return Nullness{ + value: value, } } diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go index 1eb3c9d9a..489873eed 100644 --- a/tftypes/refinement/string_prefix.go +++ b/tftypes/refinement/string_prefix.go @@ -2,21 +2,21 @@ package refinement import "github.com/vmihailenco/msgpack/v5" -type stringPrefix struct { - Value string +type StringPrefix struct { + value string } // TODO: What if the prefix is empty? Should we skip encoding? Throw an error earlier? Throw an error here? // - I think an empty prefix string is valid, empty string is still more information then wholly unknown? // - I wonder what Terraform does in this situation today -func (s stringPrefix) Encode(enc *msgpack.Encoder) error { +func (s StringPrefix) Encode(enc *msgpack.Encoder) error { // Matching go-cty for the max prefix length allowed here // // This ensures the total size of the refinements blob does not exceed the limit // set by the decoder (1024). maxPrefixLength := 256 - prefix := s.Value - if len(s.Value) > maxPrefixLength { + prefix := s.value + if len(s.value) > maxPrefixLength { prefix = prefix[:maxPrefixLength-1] } @@ -28,18 +28,22 @@ func (s stringPrefix) Encode(enc *msgpack.Encoder) error { return enc.EncodeString(prefix) } -func (s stringPrefix) Equal(Refinement) bool { +func (s StringPrefix) Equal(Refinement) bool { return false } -func (s stringPrefix) String() string { +func (s StringPrefix) String() string { return "todo - stringPrefix" } -func (s stringPrefix) unimplementable() {} +func (s StringPrefix) PrefixValue() string { + return s.value +} + +func (s StringPrefix) unimplementable() {} -func StringPrefix(value string) Refinement { - return stringPrefix{ - Value: value, +func NewStringPrefix(value string) Refinement { + return StringPrefix{ + value: value, } } diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 0eb8e13e1..062302627 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -421,7 +421,11 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath // The presence of this key means we're refining the null-ness one // way or another. If nullness is unknown then this key should not // be present at all. - newRefinements[keyCode] = refinement.Nullness(isNull) + // + // isNull should always be false if this refinement is present, but to match Terraform's support + // of this encoding, we will pass along the value. If isNull is true, then the value should not be + // unknown with refinements, it should be a known null value. + newRefinements[keyCode] = refinement.NewNullness(isNull) case refinement.KeyStringPrefix: if !typ.Is(String) { return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement for non-string type") @@ -436,7 +440,7 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath // TODO: If terraform doesn't support an empty string prefix, then neither should we, potentially return an error here. - newRefinements[keyCode] = refinement.StringPrefix(prefix) + newRefinements[keyCode] = refinement.NewStringPrefix(prefix) default: // We don't want to error here, as go-cty could introduce new refinements that we'd // want to just ignore until this logic is updated From 0c191c7cd982fd5fc9d26ca3199635a340aa64a1 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 25 Oct 2024 14:15:32 -0400 Subject: [PATCH 05/18] add skip for unknown extension keys --- tftypes/value_msgpack.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 062302627..812089305 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -442,6 +442,10 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath newRefinements[keyCode] = refinement.NewStringPrefix(prefix) default: + err := rfnDec.Skip() + if err != nil { + return Value{}, path.NewErrorf("error skipping unknown extension body, keycode = %d: %w", keyCode, err) + } // We don't want to error here, as go-cty could introduce new refinements that we'd // want to just ignore until this logic is updated continue From 920d80fb3ca5cc2638e45df146e5aadeaeded795 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 25 Oct 2024 17:58:51 -0400 Subject: [PATCH 06/18] add number bounds --- tftypes/refinement/nullness.go | 18 +--- tftypes/refinement/number_lower_bound.go | 35 ++++++ tftypes/refinement/number_upper_bound.go | 35 ++++++ tftypes/refinement/refinement.go | 15 +-- tftypes/refinement/string_prefix.go | 24 ----- tftypes/value_msgpack.go | 132 +++++++++++++++++++++-- 6 files changed, 201 insertions(+), 58 deletions(-) create mode 100644 tftypes/refinement/number_lower_bound.go create mode 100644 tftypes/refinement/number_upper_bound.go diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go index 5fe13e905..fa54e7b75 100644 --- a/tftypes/refinement/nullness.go +++ b/tftypes/refinement/nullness.go @@ -1,23 +1,9 @@ package refinement -import "github.com/vmihailenco/msgpack/v5" - type Nullness struct { value bool } -func (n Nullness) Encode(enc *msgpack.Encoder) error { - err := enc.EncodeInt(int64(KeyNullness)) - if err != nil { - return err - } - - // It shouldn't be possible for an unknown value to be definitely null (i.e. nullness.value = true), - // as that should be represented by a known null value instead. This encoding is in place to be compliant - // with Terraform's encoding which uses a definitely null refinement to collapse into a known null value. - return enc.EncodeBool(n.value) -} - func (n Nullness) Equal(Refinement) bool { return false } @@ -26,8 +12,8 @@ func (n Nullness) String() string { return "todo - Nullness" } -func (n Nullness) NotNull() bool { - return !n.value +func (n Nullness) Nullness() bool { + return n.value } func (n Nullness) unimplementable() {} diff --git a/tftypes/refinement/number_lower_bound.go b/tftypes/refinement/number_lower_bound.go new file mode 100644 index 000000000..136a0d64b --- /dev/null +++ b/tftypes/refinement/number_lower_bound.go @@ -0,0 +1,35 @@ +package refinement + +import ( + "math/big" +) + +type NumberLowerBound struct { + inclusive bool + value *big.Float +} + +func (n NumberLowerBound) Equal(Refinement) bool { + return false +} + +func (n NumberLowerBound) String() string { + return "todo - NumberLowerBound" +} + +func (n NumberLowerBound) IsInclusive() bool { + return n.inclusive +} + +func (n NumberLowerBound) LowerBound() *big.Float { + return n.value +} + +func (n NumberLowerBound) unimplementable() {} + +func NewNumberLowerBound(value *big.Float, inclusive bool) Refinement { + return NumberLowerBound{ + value: value, + inclusive: inclusive, + } +} diff --git a/tftypes/refinement/number_upper_bound.go b/tftypes/refinement/number_upper_bound.go new file mode 100644 index 000000000..d85150da6 --- /dev/null +++ b/tftypes/refinement/number_upper_bound.go @@ -0,0 +1,35 @@ +package refinement + +import ( + "math/big" +) + +type NumberUpperBound struct { + inclusive bool + value *big.Float +} + +func (n NumberUpperBound) Equal(Refinement) bool { + return false +} + +func (n NumberUpperBound) String() string { + return "todo - NumberUpperBound" +} + +func (n NumberUpperBound) IsInclusive() bool { + return n.inclusive +} + +func (n NumberUpperBound) UpperBound() *big.Float { + return n.value +} + +func (n NumberUpperBound) unimplementable() {} + +func NewNumberUpperBound(value *big.Float, inclusive bool) Refinement { + return NumberUpperBound{ + value: value, + inclusive: inclusive, + } +} diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go index bcab735a4..21bad6f0b 100644 --- a/tftypes/refinement/refinement.go +++ b/tftypes/refinement/refinement.go @@ -2,8 +2,6 @@ package refinement import ( "fmt" - - "github.com/vmihailenco/msgpack/v5" ) type Key int64 @@ -15,23 +13,26 @@ func (k Key) String() string { return "nullness" case KeyStringPrefix: return "string_prefix" + case KeyNumberLowerBound: + return "number_lower_bound" + case KeyNumberUpperBound: + return "number_upper_bound" default: return fmt.Sprintf("unsupported refinement: %d", k) } } const ( - KeyNullness = Key(1) - KeyStringPrefix = Key(2) - // KeyNumberLowerBound = Key(3) - // KeyNumberUpperBound = Key(4) + KeyNullness = Key(1) + KeyStringPrefix = Key(2) + KeyNumberLowerBound = Key(3) + KeyNumberUpperBound = Key(4) // KeyCollectionLengthLowerBound = Key(5) // KeyCollectionLengthUpperBound = Key(6) ) type Refinement interface { Equal(Refinement) bool - Encode(*msgpack.Encoder) error String() string unimplementable() // prevents external implementations, all refinements are defined in the Terraform/HCL type system go-cty. } diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go index 489873eed..935af91bd 100644 --- a/tftypes/refinement/string_prefix.go +++ b/tftypes/refinement/string_prefix.go @@ -1,33 +1,9 @@ package refinement -import "github.com/vmihailenco/msgpack/v5" - type StringPrefix struct { value string } -// TODO: What if the prefix is empty? Should we skip encoding? Throw an error earlier? Throw an error here? -// - I think an empty prefix string is valid, empty string is still more information then wholly unknown? -// - I wonder what Terraform does in this situation today -func (s StringPrefix) Encode(enc *msgpack.Encoder) error { - // Matching go-cty for the max prefix length allowed here - // - // This ensures the total size of the refinements blob does not exceed the limit - // set by the decoder (1024). - maxPrefixLength := 256 - prefix := s.value - if len(s.value) > maxPrefixLength { - prefix = prefix[:maxPrefixLength-1] - } - - err := enc.EncodeInt(int64(KeyStringPrefix)) - if err != nil { - return err - } - - return enc.EncodeString(prefix) -} - func (s StringPrefix) Equal(Refinement) bool { return false } diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 812089305..c1eb393e1 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -441,6 +441,44 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath // TODO: If terraform doesn't support an empty string prefix, then neither should we, potentially return an error here. newRefinements[keyCode] = refinement.NewStringPrefix(prefix) + case refinement.KeyNumberLowerBound, refinement.KeyNumberUpperBound: + if !typ.Is(Number) { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement for non-number type") + } + + // We know these refinements are a tuple of [number, bool] so we can re-use the msgpack decoding logic on this refinement + tfValBound, err := msgpackUnmarshal(rfnDec, Tuple{ElementTypes: []Type{Number, Bool}}, path) + if err != nil || tfValBound.IsNull() || !tfValBound.IsKnown() { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement must be [number, bool] tuple") + } + + tupleVal := []Value{} + err = tfValBound.As(&tupleVal) + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement tuple value conversion failed: %w", err) + } + + if len(tupleVal) != 2 { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement tuple value conversion failed: expected 2 elements, got %d elements", len(tupleVal)) + } + + var boundVal *big.Float + err = tupleVal[0].As(&boundVal) + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement bound value conversion failed: %w", err) + } + + var inclusiveVal bool + err = tupleVal[1].As(&inclusiveVal) + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement inclusive value conversion failed: %w", err) + } + + if keyCode == refinement.KeyNumberLowerBound { + newRefinements[keyCode] = refinement.NewNumberLowerBound(boundVal, inclusiveVal) + } else { + newRefinements[keyCode] = refinement.NewNumberUpperBound(boundVal, inclusiveVal) + } default: err := rfnDec.Skip() if err != nil { @@ -511,23 +549,95 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc refnEnc := msgpack.NewEncoder(&refnBuf) mapLen := 0 - // TODO: Should the refinement interface be defining the encoding? Should we export the refinement implementation details? - // - If we keep it in the interface, then we can simplify this logic - for kind, refn := range val.refinements { - switch kind { - case refinement.KeyNullness: - err := refn.Encode(refnEnc) + for _, refn := range val.refinements { + switch refnVal := refn.(type) { + case refinement.Nullness: + err := refnEnc.EncodeInt(int64(refinement.KeyNullness)) + if err != nil { + return p.NewErrorf("error encoding Nullness value refinement key: %w", err) + } + + // It shouldn't be possible for an unknown value to be definitely null (i.e. nullness.value = true), + // as that should be represented by a known null value instead. This encoding is in place to be compliant + // with Terraform's encoding which uses a definitely null refinement to collapse into a known null value. + err = refnEnc.EncodeBool(refnVal.Nullness()) if err != nil { return p.NewErrorf("error encoding Nullness value refinement: %w", err) } + mapLen++ - case refinement.KeyStringPrefix: - // TODO: If the prefix is empty, we shouldn't encode a refinement. This should - // probably be reflected in the interface. - err := refn.Encode(refnEnc) + case refinement.StringPrefix: + if rawPrefix := refnVal.PrefixValue(); rawPrefix != "" { + // Matching go-cty for the max prefix length allowed here + // + // This ensures the total size of the refinements blob does not exceed the limit + // set by the decoder (1024). + maxPrefixLength := 256 + prefix := rawPrefix + if len(rawPrefix) > maxPrefixLength { + prefix = prefix[:maxPrefixLength-1] + } + + err := refnEnc.EncodeInt(int64(refinement.KeyStringPrefix)) + if err != nil { + return p.NewErrorf("error encoding StringPrefix value refinement key: %w", err) + } + + err = refnEnc.EncodeString(prefix) + if err != nil { + return p.NewErrorf("error encoding StringPrefix value refinement: %w", err) + } + + mapLen++ + } + + case refinement.NumberLowerBound: + // TODO: should check this isn't negative infinity? To match go-cty + boundTfType := Tuple{ElementTypes: []Type{Number, Bool}} + + // TODO: Do we need to do this? Kind of nasty + boundTfVal := NewValue( + boundTfType, + []Value{ + NewValue(Number, refnVal.LowerBound()), + NewValue(Bool, refnVal.IsInclusive()), + }, + ) + + err := refnEnc.EncodeInt(int64(refinement.KeyNumberLowerBound)) + if err != nil { + return p.NewErrorf("error encoding NumberLowerBound value refinement key: %w", err) + } + + err = marshalMsgPack(boundTfVal, boundTfType, p, refnEnc) + if err != nil { + return p.NewErrorf("error encoding NumberLowerBound value refinement: %w", err) + } + + mapLen++ + case refinement.NumberUpperBound: + // TODO: should check this isn't positive infinity? To match go-cty + boundTfType := Tuple{ElementTypes: []Type{Number, Bool}} + + // TODO: Do we need to do this? Kind of nasty + boundTfVal := NewValue( + boundTfType, + []Value{ + NewValue(Number, refnVal.UpperBound()), + NewValue(Bool, refnVal.IsInclusive()), + }, + ) + + err := refnEnc.EncodeInt(int64(refinement.KeyNumberUpperBound)) if err != nil { - return p.NewErrorf("error encoding StringPrefix value refinement: %w", err) + return p.NewErrorf("error encoding NumberUpperBound value refinement key: %w", err) } + + err = marshalMsgPack(boundTfVal, boundTfType, p, refnEnc) + if err != nil { + return p.NewErrorf("error encoding NumberUpperBound value refinement: %w", err) + } + mapLen++ default: continue From d3f8a1bbd006920bd2afd62dcd6d9d736f63324e Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 21 Nov 2024 15:29:38 -0500 Subject: [PATCH 07/18] the rest of the refinements --- .../collection_length_lower_bound.go | 33 +++++++++++++ .../collection_length_upper_bound.go | 33 +++++++++++++ tftypes/refinement/doc.go | 5 ++ tftypes/refinement/nullness.go | 8 ++++ tftypes/refinement/number_lower_bound.go | 9 ++++ tftypes/refinement/number_upper_bound.go | 9 ++++ tftypes/refinement/refinement.go | 47 +++++++++++++++++-- tftypes/refinement/string_prefix.go | 8 ++++ 8 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 tftypes/refinement/collection_length_lower_bound.go create mode 100644 tftypes/refinement/collection_length_upper_bound.go create mode 100644 tftypes/refinement/doc.go diff --git a/tftypes/refinement/collection_length_lower_bound.go b/tftypes/refinement/collection_length_lower_bound.go new file mode 100644 index 000000000..d42af1f90 --- /dev/null +++ b/tftypes/refinement/collection_length_lower_bound.go @@ -0,0 +1,33 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package refinement + +// TODO: doc +type CollectionLengthLowerBound struct { + value int64 +} + +func (n CollectionLengthLowerBound) Equal(Refinement) bool { + // TODO: implement + return false +} + +func (n CollectionLengthLowerBound) String() string { + // TODO: implement + return "todo - CollectionLengthLowerBound" +} + +// TODO: doc +func (n CollectionLengthLowerBound) LowerBound() int64 { + return n.value +} + +func (n CollectionLengthLowerBound) unimplementable() {} + +// TODO: doc +func NewCollectionLengthLowerBound(value int64) Refinement { + return CollectionLengthLowerBound{ + value: value, + } +} diff --git a/tftypes/refinement/collection_length_upper_bound.go b/tftypes/refinement/collection_length_upper_bound.go new file mode 100644 index 000000000..84891bf3d --- /dev/null +++ b/tftypes/refinement/collection_length_upper_bound.go @@ -0,0 +1,33 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package refinement + +// TODO: doc +type CollectionLengthUpperBound struct { + value int64 +} + +func (n CollectionLengthUpperBound) Equal(Refinement) bool { + // TODO: implement + return false +} + +func (n CollectionLengthUpperBound) String() string { + // TODO: implement + return "todo - CollectionLengthUpperBound" +} + +// TODO: doc +func (n CollectionLengthUpperBound) UpperBound() int64 { + return n.value +} + +func (n CollectionLengthUpperBound) unimplementable() {} + +// TODO: doc +func NewCollectionLengthUpperBound(value int64) Refinement { + return CollectionLengthUpperBound{ + value: value, + } +} diff --git a/tftypes/refinement/doc.go b/tftypes/refinement/doc.go new file mode 100644 index 000000000..85283c929 --- /dev/null +++ b/tftypes/refinement/doc.go @@ -0,0 +1,5 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +// TODO: docs +package refinement diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go index fa54e7b75..df88ccf28 100644 --- a/tftypes/refinement/nullness.go +++ b/tftypes/refinement/nullness.go @@ -1,23 +1,31 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package refinement +// TODO: doc type Nullness struct { value bool } func (n Nullness) Equal(Refinement) bool { + // TODO: implement return false } func (n Nullness) String() string { + // TODO: implement return "todo - Nullness" } +// TODO: doc func (n Nullness) Nullness() bool { return n.value } func (n Nullness) unimplementable() {} +// TODO: doc func NewNullness(value bool) Refinement { return Nullness{ value: value, diff --git a/tftypes/refinement/number_lower_bound.go b/tftypes/refinement/number_lower_bound.go index 136a0d64b..a62f20639 100644 --- a/tftypes/refinement/number_lower_bound.go +++ b/tftypes/refinement/number_lower_bound.go @@ -1,32 +1,41 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package refinement import ( "math/big" ) +// TODO: doc type NumberLowerBound struct { inclusive bool value *big.Float } func (n NumberLowerBound) Equal(Refinement) bool { + // TODO: implement return false } func (n NumberLowerBound) String() string { + // TODO: implement return "todo - NumberLowerBound" } +// TODO: doc func (n NumberLowerBound) IsInclusive() bool { return n.inclusive } +// TODO: doc func (n NumberLowerBound) LowerBound() *big.Float { return n.value } func (n NumberLowerBound) unimplementable() {} +// TODO: doc func NewNumberLowerBound(value *big.Float, inclusive bool) Refinement { return NumberLowerBound{ value: value, diff --git a/tftypes/refinement/number_upper_bound.go b/tftypes/refinement/number_upper_bound.go index d85150da6..9f2f0bf64 100644 --- a/tftypes/refinement/number_upper_bound.go +++ b/tftypes/refinement/number_upper_bound.go @@ -1,32 +1,41 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package refinement import ( "math/big" ) +// TODO: doc type NumberUpperBound struct { inclusive bool value *big.Float } func (n NumberUpperBound) Equal(Refinement) bool { + // TODO: implement return false } func (n NumberUpperBound) String() string { + // TODO: implement return "todo - NumberUpperBound" } +// TODO: doc func (n NumberUpperBound) IsInclusive() bool { return n.inclusive } +// TODO: doc func (n NumberUpperBound) UpperBound() *big.Float { return n.value } func (n NumberUpperBound) unimplementable() {} +// TODO: doc func NewNumberUpperBound(value *big.Float, inclusive bool) Refinement { return NumberUpperBound{ value: value, diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go index 21bad6f0b..57506326c 100644 --- a/tftypes/refinement/refinement.go +++ b/tftypes/refinement/refinement.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package refinement import ( @@ -23,20 +26,56 @@ func (k Key) String() string { } const ( - KeyNullness = Key(1) - KeyStringPrefix = Key(2) + // KeyNullness represents a refinement that specifies whether the final value will not be null. + // + // MAINTINAER NOTE: In practice, this refinement data will only contain "false", indicating the final value + // cannot be null. If the refinement data was ever set to "true", that would indicate the final value will be null, in which + // case the value is not unknown, it is known and should not have any refinement data. + // + // This refinement is relevant for all types except tftypes.DynamicPseudoType. + KeyNullness = Key(1) + + // KeyStringPrefix represents a refinement that specifies a known prefix of a final string value. + // + // This refinement is only relevant for tftypes.String. + KeyStringPrefix = Key(2) + + // KeyNumberLowerBound represents a refinement that specifies the lower bound of possible values for a final number value. + // The refinement data contains a boolean which indicates whether the bound is inclusive. + // + // This refinement is only relevant for tftypes.Number. KeyNumberLowerBound = Key(3) + + // KeyNumberUpperBound represents a refinement that specifies the upper bound of possible values for a final number value. + // The refinement data contains a boolean which indicates whether the bound is inclusive. + // + // This refinement is only relevant for tftypes.Number. KeyNumberUpperBound = Key(4) - // KeyCollectionLengthLowerBound = Key(5) - // KeyCollectionLengthUpperBound = Key(6) + + // KeyCollectionLengthLowerBound represents a refinement that specifies the lower bound of possible length for a final collection value. + // + // This refinement is only relevant for tftypes.List, tftypes.Set, and tftypes.Map. + KeyCollectionLengthLowerBound = Key(5) + + // KeyCollectionLengthUpperBound represents a refinement that specifies the upper bound of possible length for a final collection value. + // + // This refinement is only relevant for tftypes.List, tftypes.Set, and tftypes.Map. + KeyCollectionLengthUpperBound = Key(6) ) +// TODO: docs type Refinement interface { + // Equal should return true if the Refinement is considered equivalent to the + // Refinement passed as an argument. Equal(Refinement) bool + + // String should return a human-friendly version of the Refinement. String() string + unimplementable() // prevents external implementations, all refinements are defined in the Terraform/HCL type system go-cty. } +// TODO: docs type Refinements map[Key]Refinement func (r Refinements) Equal(o Refinements) bool { diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go index 935af91bd..246edd5b2 100644 --- a/tftypes/refinement/string_prefix.go +++ b/tftypes/refinement/string_prefix.go @@ -1,23 +1,31 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package refinement +// TODO: doc type StringPrefix struct { value string } func (s StringPrefix) Equal(Refinement) bool { + // TODO: implement return false } func (s StringPrefix) String() string { + // TODO: implement return "todo - stringPrefix" } +// TODO: doc func (s StringPrefix) PrefixValue() string { return s.value } func (s StringPrefix) unimplementable() {} +// TODO: doc func NewStringPrefix(value string) Refinement { return StringPrefix{ value: value, From 43cd678e5adc86620648b4ae43e09fcb997ebcb2 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 21 Nov 2024 17:46:30 -0500 Subject: [PATCH 08/18] implement encoding/decoding --- tftypes/value_msgpack.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index c1eb393e1..a0516050a 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -479,6 +479,21 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath } else { newRefinements[keyCode] = refinement.NewNumberUpperBound(boundVal, inclusiveVal) } + case refinement.KeyCollectionLengthLowerBound, refinement.KeyCollectionLengthUpperBound: + if !typ.Is(List{}) && !typ.Is(Map{}) && !typ.Is(Set{}) { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: length bound refinement for non-collection type") + } + + boundVal, err := rfnDec.DecodeInt() + if err != nil { + return Value{}, path.NewErrorf("failed to decode msgpack extension body: length bound refinement must be integer") + } + + if keyCode == refinement.KeyCollectionLengthLowerBound { + newRefinements[keyCode] = refinement.NewCollectionLengthLowerBound(int64(boundVal)) + } else { + newRefinements[keyCode] = refinement.NewCollectionLengthUpperBound(int64(boundVal)) + } default: err := rfnDec.Skip() if err != nil { @@ -638,6 +653,30 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc return p.NewErrorf("error encoding NumberUpperBound value refinement: %w", err) } + mapLen++ + case refinement.CollectionLengthLowerBound: + err := refnEnc.EncodeInt(int64(refinement.KeyCollectionLengthLowerBound)) + if err != nil { + return p.NewErrorf("error encoding CollectionLengthLowerBound value refinement key: %w", err) + } + + err = refnEnc.EncodeInt(refnVal.LowerBound()) + if err != nil { + return p.NewErrorf("error encoding CollectionLengthLowerBound value refinement: %w", err) + } + + mapLen++ + case refinement.CollectionLengthUpperBound: + err := refnEnc.EncodeInt(int64(refinement.KeyCollectionLengthUpperBound)) + if err != nil { + return p.NewErrorf("error encoding CollectionLengthUpperBound value refinement key: %w", err) + } + + err = refnEnc.EncodeInt(refnVal.UpperBound()) + if err != nil { + return p.NewErrorf("error encoding CollectionLengthUpperBound value refinement: %w", err) + } + mapLen++ default: continue From 78c8d985fc1fac19e65f886b0a5bfdceb0cd1718 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 22 Nov 2024 17:56:01 -0500 Subject: [PATCH 09/18] refactor --- tftypes/value_msgpack.go | 99 ++++++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 28 deletions(-) diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index a0516050a..90b6662d4 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -564,25 +564,38 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc refnEnc := msgpack.NewEncoder(&refnBuf) mapLen := 0 - for _, refn := range val.refinements { - switch refnVal := refn.(type) { - case refinement.Nullness: - err := refnEnc.EncodeInt(int64(refinement.KeyNullness)) - if err != nil { - return p.NewErrorf("error encoding Nullness value refinement key: %w", err) - } + // Nullness refinement applies to all types except for DynamicPseudoType (handled above) + if refnVal, ok := val.refinements[refinement.KeyNullness]; ok { + data, ok := refnVal.(refinement.Nullness) + if !ok { + return p.NewErrorf("error encoding Nullness value refinement: unexpected refinement data of type %T", refnVal) + } - // It shouldn't be possible for an unknown value to be definitely null (i.e. nullness.value = true), - // as that should be represented by a known null value instead. This encoding is in place to be compliant - // with Terraform's encoding which uses a definitely null refinement to collapse into a known null value. - err = refnEnc.EncodeBool(refnVal.Nullness()) - if err != nil { - return p.NewErrorf("error encoding Nullness value refinement: %w", err) + err := refnEnc.EncodeInt(int64(refinement.KeyNullness)) + if err != nil { + return p.NewErrorf("error encoding Nullness value refinement key: %w", err) + } + + // It shouldn't be possible for an unknown value to be definitely null (i.e. nullness.value = true), + // as that should be represented by a known null value instead. This encoding is in place to be compliant + // with Terraform's encoding which uses a definitely null refinement to collapse into a known null value. + err = refnEnc.EncodeBool(data.Nullness()) + if err != nil { + return p.NewErrorf("error encoding Nullness value refinement: %w", err) + } + + mapLen++ + } + + // Refinements for strings + if typ.Is(String) { + if refnVal, ok := val.refinements[refinement.KeyStringPrefix]; ok { + data, ok := refnVal.(refinement.StringPrefix) + if !ok { + return p.NewErrorf("error encoding StringPrefix value refinement: unexpected refinement data of type %T", refnVal) } - mapLen++ - case refinement.StringPrefix: - if rawPrefix := refnVal.PrefixValue(); rawPrefix != "" { + if rawPrefix := data.PrefixValue(); rawPrefix != "" { // Matching go-cty for the max prefix length allowed here // // This ensures the total size of the refinements blob does not exceed the limit @@ -605,8 +618,17 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc mapLen++ } + } + } + + // Refinements for numbers + if typ.Is(Number) { + if refnVal, ok := val.refinements[refinement.KeyNumberLowerBound]; ok { + data, ok := refnVal.(refinement.NumberLowerBound) + if !ok { + return p.NewErrorf("error encoding NumberLowerBound value refinement: unexpected refinement data of type %T", refnVal) + } - case refinement.NumberLowerBound: // TODO: should check this isn't negative infinity? To match go-cty boundTfType := Tuple{ElementTypes: []Type{Number, Bool}} @@ -614,8 +636,8 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc boundTfVal := NewValue( boundTfType, []Value{ - NewValue(Number, refnVal.LowerBound()), - NewValue(Bool, refnVal.IsInclusive()), + NewValue(Number, data.LowerBound()), + NewValue(Bool, data.IsInclusive()), }, ) @@ -630,7 +652,14 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc } mapLen++ - case refinement.NumberUpperBound: + } + + if refnVal, ok := val.refinements[refinement.KeyNumberUpperBound]; ok { + data, ok := refnVal.(refinement.NumberUpperBound) + if !ok { + return p.NewErrorf("error encoding NumberUpperBound value refinement: unexpected refinement data of type %T", refnVal) + } + // TODO: should check this isn't positive infinity? To match go-cty boundTfType := Tuple{ElementTypes: []Type{Number, Bool}} @@ -638,8 +667,8 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc boundTfVal := NewValue( boundTfType, []Value{ - NewValue(Number, refnVal.UpperBound()), - NewValue(Bool, refnVal.IsInclusive()), + NewValue(Number, data.UpperBound()), + NewValue(Bool, data.IsInclusive()), }, ) @@ -654,32 +683,46 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc } mapLen++ - case refinement.CollectionLengthLowerBound: + } + } + + // Refinements for collections + if typ.Is(List{}) || typ.Is(Map{}) || typ.Is(Set{}) { + if refnVal, ok := val.refinements[refinement.KeyCollectionLengthLowerBound]; ok { + data, ok := refnVal.(refinement.CollectionLengthLowerBound) + if !ok { + return p.NewErrorf("error encoding CollectionLengthLowerBound value refinement: unexpected refinement data of type %T", refnVal) + } + err := refnEnc.EncodeInt(int64(refinement.KeyCollectionLengthLowerBound)) if err != nil { return p.NewErrorf("error encoding CollectionLengthLowerBound value refinement key: %w", err) } - err = refnEnc.EncodeInt(refnVal.LowerBound()) + err = refnEnc.EncodeInt(data.LowerBound()) if err != nil { return p.NewErrorf("error encoding CollectionLengthLowerBound value refinement: %w", err) } mapLen++ - case refinement.CollectionLengthUpperBound: + } + + if refnVal, ok := val.refinements[refinement.KeyCollectionLengthUpperBound]; ok { + data, ok := refnVal.(refinement.CollectionLengthUpperBound) + if !ok { + return p.NewErrorf("error encoding CollectionLengthUpperBound value refinement: unexpected refinement data of type %T", refnVal) + } err := refnEnc.EncodeInt(int64(refinement.KeyCollectionLengthUpperBound)) if err != nil { return p.NewErrorf("error encoding CollectionLengthUpperBound value refinement key: %w", err) } - err = refnEnc.EncodeInt(refnVal.UpperBound()) + err = refnEnc.EncodeInt(data.UpperBound()) if err != nil { return p.NewErrorf("error encoding CollectionLengthUpperBound value refinement: %w", err) } mapLen++ - default: - continue } } From b35d1796c152501738928b3cd5c21862af8c8677 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 22 Nov 2024 17:57:25 -0500 Subject: [PATCH 10/18] add some initial tests --- tftypes/value_msgpack_test.go | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tftypes/value_msgpack_test.go b/tftypes/value_msgpack_test.go index 778c073e1..7cff47e1f 100644 --- a/tftypes/value_msgpack_test.go +++ b/tftypes/value_msgpack_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" ) func TestValueFromMsgPack(t *testing.T) { @@ -532,6 +533,60 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: List{ElementType: DynamicPseudoType}, }, + "unknown-string-with-refinements": { + // TODO: How to actually visualize this? So hard to read these tests... + hex: "c70e0c8201c202a97072656669783a2f2f", + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix("prefix://"), + }), + typ: String, + }, + "unknown-number-with-refinements": { + // TODO: How to actually visualize this? So hard to read these tests... + hex: "c70b0c8301c2039201c3049205c3", + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + typ: Number, + }, + "unknown-list-with-refinements": { + // TODO: How to actually visualize this? So hard to read these tests... + hex: "c7070c8301c205010605", + value: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + typ: List{ElementType: String}, + }, + "unknown-map-with-refinements": { + // TODO: How to actually visualize this? So hard to read these tests... + hex: "c7070c8301c205010605", + value: NewValue(Map{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + typ: Map{ElementType: String}, + }, + "unknown-set-with-refinements": { + // TODO: How to actually visualize this? So hard to read these tests... + hex: "c7070c8301c205010605", + value: NewValue(Set{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + typ: Set{ElementType: String}, + }, + // TODO: Test putting refinements with too long of data (string prefix over 256) + // TODO: Test putting refinements with incorrect data (string prefix on nullness key) + // TODO: Test putting refinements on incorrect types (prefix on number or boolean) + // TODO: Test refinement number that doesn't exist? Should just be unknown value + // TODO: Test DynamicPseudoType? } for name, test := range tests { name, test := name, test @@ -550,6 +605,7 @@ func TestValueFromMsgPack(t *testing.T) { if err != nil { t.Fatalf("unexpected error parsing hex: %s", err) } + val, err := ValueFromMsgPack(b, test.typ) if err != nil { t.Fatalf("unexpected error unmarshaling: %s", err) From 219293773aece9a82bc35895c60036015e667b63 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 09:48:27 -0500 Subject: [PATCH 11/18] added tests --- tftypes/value_msgpack_test.go | 242 ++++++++++++++++++++++++++++++---- 1 file changed, 216 insertions(+), 26 deletions(-) diff --git a/tftypes/value_msgpack_test.go b/tftypes/value_msgpack_test.go index 7cff47e1f..1eb0fd230 100644 --- a/tftypes/value_msgpack_test.go +++ b/tftypes/value_msgpack_test.go @@ -5,14 +5,23 @@ package tftypes import ( "encoding/hex" + "errors" "math" "math/big" + "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" ) +// Hex encoding of the long prefix refinements used in this test +var longPrefixRefinement = "c801050c8201c202d9ff7072656669783a2f2f303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d30313233" + func TestValueFromMsgPack(t *testing.T) { t.Parallel() type testCase struct { @@ -533,8 +542,57 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: List{ElementType: DynamicPseudoType}, }, - "unknown-string-with-refinements": { - // TODO: How to actually visualize this? So hard to read these tests... + "unknown-dynamic-refinements-ignored": { + hex: "d40000", + value: NewValue(DynamicPseudoType, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: DynamicPseudoType, + }, + "unknown-bool-with-nullness-refinement": { + hex: "c7030c8101c2", + value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: Bool, + }, + "unknown-number-with-nullness-refinement": { + hex: "c7030c8101c2", + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: Number, + }, + "unknown-string-with-nullness-refinement": { + hex: "c7030c8101c2", + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: String, + }, + "unknown-list-with-nullness-refinement": { + hex: "c7030c8101c2", + value: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: List{ElementType: String}, + }, + "unknown-object-with-nullness-refinement": { + hex: "c7030c8101c2", + value: NewValue(Object{AttributeTypes: map[string]Type{"attr": String}}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: Object{AttributeTypes: map[string]Type{"attr": String}}, + }, + "unknown-string-with-empty-prefix-refinement": { + hex: "c7030c8101c2", + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix(""), + }), + typ: String, + }, + "unknown-string-with-prefix-refinement": { hex: "c70e0c8201c202a97072656669783a2f2f", value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), @@ -542,8 +600,33 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: String, }, - "unknown-number-with-refinements": { - // TODO: How to actually visualize this? So hard to read these tests... + "unknown-string-with-long-prefix-refinement-one": { + // This prefix will be cutoff at 256 bytes, so it will be equal to the other long prefix test. + hex: longPrefixRefinement, + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix( + "prefix://0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-thiswillbecutoff1", + ), + }), + typ: String, + }, + "unknown-string-with-long-prefix-refinement-two": { + // This prefix will be cutoff at 256 bytes, so it will be equal to the other long prefix test. + hex: longPrefixRefinement, + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix( + "prefix://0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-thiswillbecutoff2", + ), + }), + typ: String, + }, + "unknown-number-with-bound-refinements-integers-inclusive": { hex: "c70b0c8301c2039201c3049205c3", value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), @@ -552,8 +635,34 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: Number, }, - "unknown-list-with-refinements": { - // TODO: How to actually visualize this? So hard to read these tests... + "unknown-number-with-bound-refinements-integers-exclusive": { + hex: "c70b0c8301c2039201c2049205c2", + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), false), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), false), + }), + typ: Number, + }, + "unknown-number-with-bound-refinements-float-inclusive": { + hex: "c71b0c8301c20392cb3ff3ae147ae147aec30492cb4016ae147ae147aec3", + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1.23), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5.67), true), + }), + typ: Number, + }, + "unknown-number-with-bound-refinements-float-exclusive": { + hex: "c71b0c8301c20392cb3ff3ae147ae147aec20492cb4016ae147ae147aec2", + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1.23), false), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5.67), false), + }), + typ: Number, + }, + "unknown-list-with-bound-refinements": { hex: "c7070c8301c205010605", value: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), @@ -562,37 +671,46 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: List{ElementType: String}, }, - "unknown-map-with-refinements": { - // TODO: How to actually visualize this? So hard to read these tests... - hex: "c7070c8301c205010605", - value: NewValue(Map{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + "unknown-map-with-bound-refinements": { + hex: "c7070c8301c205000604", + value: NewValue(Map{ElementType: Number}, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), - refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), - refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(0), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(4), }), - typ: Map{ElementType: String}, + typ: Map{ElementType: Number}, }, - "unknown-set-with-refinements": { - // TODO: How to actually visualize this? So hard to read these tests... - hex: "c7070c8301c205010605", - value: NewValue(Set{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + "unknown-set-with-bound-refinements": { + hex: "c7070c8301c205020606", + value: NewValue(Set{ElementType: Bool}, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), - refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), - refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(2), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(6), }), - typ: Set{ElementType: String}, + typ: Set{ElementType: Bool}, + }, + "unknown-with-invalid-refinement-type": { + hex: "d40000", + value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + // This refinement will be ignored since only strings will attempt to encode this + refinement.KeyStringPrefix: refinement.NewStringPrefix("ignored"), + }), + typ: Bool, + }, + "unknown-with-invalid-refinement-data": { + hex: "d40000", + value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + // This refinement will be ignored since we don't know how to encode it + refinement.Key(100): refinement.NewStringPrefix("ignored"), + }), + typ: Bool, }, - // TODO: Test putting refinements with too long of data (string prefix over 256) - // TODO: Test putting refinements with incorrect data (string prefix on nullness key) - // TODO: Test putting refinements on incorrect types (prefix on number or boolean) - // TODO: Test refinement number that doesn't exist? Should just be unknown value - // TODO: Test DynamicPseudoType? } for name, test := range tests { name, test := name, test t.Run(name, func(t *testing.T) { t.Parallel() - got, err := test.value.MarshalMsgPack(test.typ) //nolint:staticcheck + got, err := test.value.MarshalMsgPack(test.typ) if err != nil { t.Fatalf("unexpected error marshaling: %s", err) } @@ -617,3 +735,75 @@ func TestValueFromMsgPack(t *testing.T) { }) } } + +func TestMarshalMsgPack_error(t *testing.T) { + t.Parallel() + tests := map[string]struct { + value Value + typ Type + expectedError error + }{ + "unknown-with-invalid-nullness-refinement": { + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + // String prefix is invalid on KeyNullness + refinement.KeyNullness: refinement.NewStringPrefix("invalid"), + }), + typ: String, + expectedError: errors.New("error encoding Nullness value refinement: unexpected refinement data of type refinement.StringPrefix"), + }, + "unknown-with-invalid-prefix-refinement": { + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + // Nullness is invalid on KeyStringPrefix + refinement.KeyStringPrefix: refinement.NewNullness(false), + }), + typ: String, + expectedError: errors.New("error encoding StringPrefix value refinement: unexpected refinement data of type refinement.Nullness"), + }, + "unknown-with-invalid-number-lowerbound-refinement": { + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + // NumberUpperBound is invalid on KeyNumberLowerBound + refinement.KeyNumberLowerBound: refinement.NewNumberUpperBound(big.NewFloat(1), true), + }), + typ: Number, + expectedError: errors.New("error encoding NumberLowerBound value refinement: unexpected refinement data of type refinement.NumberUpperBound"), + }, + "unknown-with-invalid-number-upperbound-refinement": { + value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + // NumberLowerBound is invalid on KeyNumberUpperBound + refinement.KeyNumberUpperBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + }), + typ: Number, + expectedError: errors.New("error encoding NumberUpperBound value refinement: unexpected refinement data of type refinement.NumberLowerBound"), + }, + "unknown-with-invalid-collection-lowerbound-refinement": { + value: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + // CollectionLengthUpperBound is invalid on KeyCollectionLengthLowerBound + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthUpperBound(1), + }), + typ: List{ElementType: String}, + expectedError: errors.New("error encoding CollectionLengthLowerBound value refinement: unexpected refinement data of type refinement.CollectionLengthUpperBound"), + }, + "unknown-with-invalid-collection-upperbound-refinement": { + value: NewValue(Map{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + // CollectionLengthLowerBound is invalid on KeyCollectionLengthUpperBound + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthLowerBound(1), + }), + typ: Map{ElementType: String}, + expectedError: errors.New("error encoding CollectionLengthUpperBound value refinement: unexpected refinement data of type refinement.CollectionLengthLowerBound"), + }, + } + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + _, err := test.value.MarshalMsgPack(test.typ) + if err == nil { + t.Fatalf("got no error, wanted err: %s", test.expectedError) + } + + if !strings.Contains(err.Error(), test.expectedError.Error()) { + t.Fatalf("wanted error %q, got error: %s", test.expectedError.Error(), err.Error()) + } + }) + } +} From d0fed2c19b80bb4590127d5b4955c9a53cb1965e Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 10:43:58 -0500 Subject: [PATCH 12/18] clean up comments --- tftypes/value_msgpack.go | 92 +++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 53 deletions(-) diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 90b6662d4..96c659320 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -358,12 +358,11 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath // If the extension is zero or one-length, this is a wholly unknown value with no // refinements. - // TODO: Previous implementations always skipped the body, should we preserve that? if extLen > 0 { // Skip the body err = dec.Skip() if err != nil { - return Value{}, path.NewErrorf("error skipping extension byte: %w", err) + return Value{}, path.NewErrorf("error skipping extension body: %w", err) } } @@ -372,33 +371,29 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath // Check if the extension is the designated cty unknown refinement code if typeCode != unknownWithRefinementsExt { - // TODO: cleanup error return Value{}, path.NewErrorf("unsupported extension type") } if extLen > 1024 { - // A refinement description greater than 1 kiB is unreasonable and - // might be an abusive attempt to allocate large amounts of memory - // in a system consuming this input. - return Value{}, path.NewErrorf("unsupported extension type") + // cty refinements cannot be greater than 1 kiB + return Value{}, path.NewErrorf("unknown value refinement too large") } + // Unknown value refinements are always a msgpack-encoded map body := make([]byte, extLen) _, err = io.ReadAtLeast(dec.Buffered(), body, len(body)) if err != nil { - return Value{}, path.NewErrorf("failed to read msgpack extension body: %s", err) + return Value{}, path.NewErrorf("error reading msgpack extension body: %s", err) } rfnDec := msgpack.NewDecoder(bytes.NewReader(body)) entryCount, err := rfnDec.DecodeMapLen() if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: not a map") + return Value{}, path.NewErrorf("error decoding msgpack extension body: not a map") } - // Ignore all refinements for DynamicPseudoType for now, since go-cty also ignores this. - // - // We know that's invalid today but we might find a way to support it in the future and - // if so will want to introduce that in a backward-compatible way. + // cty ignores all refinements for DynamicPseudoType at the moment, this allows them to be introduced + // in the future in a backward compatible way. if typ.Is(DynamicPseudoType) { return NewValue(typ, UnknownValue), nil } @@ -409,69 +404,64 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath for i := 0; i < entryCount; i++ { keyCode, err := rfnDec.DecodeInt64() if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: non-integer key in map") + return Value{}, path.NewErrorf("error decoding msgpack extension body: non-integer key in map") } switch keyCode := refinement.Key(keyCode); keyCode { case refinement.KeyNullness: isNull, err := rfnDec.DecodeBool() if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: null refinement is not boolean") + return Value{}, path.NewErrorf("error decoding msgpack extension body: null refinement is not boolean") } - // The presence of this key means we're refining the null-ness one - // way or another. If nullness is unknown then this key should not - // be present at all. - // - // isNull should always be false if this refinement is present, but to match Terraform's support + + // isNull should always be false if this refinement is present, but to match cty's support // of this encoding, we will pass along the value. If isNull is true, then the value should not be // unknown with refinements, it should be a known null value. newRefinements[keyCode] = refinement.NewNullness(isNull) case refinement.KeyStringPrefix: if !typ.Is(String) { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement for non-string type") + return Value{}, path.NewErrorf("error decoding msgpack extension body: string prefix refinement for non-string type") } prefix, err := rfnDec.DecodeString() if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement is not string") + return Value{}, path.NewErrorf("error decoding msgpack extension body: string prefix refinement is not string") } if !utf8.ValidString(prefix) { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement is not valid UTF-8") + return Value{}, path.NewErrorf("error decoding msgpack extension body: string prefix refinement is not valid UTF-8") } - // TODO: If terraform doesn't support an empty string prefix, then neither should we, potentially return an error here. - newRefinements[keyCode] = refinement.NewStringPrefix(prefix) case refinement.KeyNumberLowerBound, refinement.KeyNumberUpperBound: if !typ.Is(Number) { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement for non-number type") + return Value{}, path.NewErrorf("error decoding msgpack extension body: numeric bound refinement for non-number type") } - // We know these refinements are a tuple of [number, bool] so we can re-use the msgpack decoding logic on this refinement + // Numeric bound refinements are always a tuple of [number, bool] so we re-use the msgpack decoding logic on this refinement. tfValBound, err := msgpackUnmarshal(rfnDec, Tuple{ElementTypes: []Type{Number, Bool}}, path) if err != nil || tfValBound.IsNull() || !tfValBound.IsKnown() { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement must be [number, bool] tuple") + return Value{}, path.NewErrorf("error decoding msgpack extension body: numeric bound refinement must be [number, bool] tuple") } - tupleVal := []Value{} + tupleVal := make([]Value, 2) err = tfValBound.As(&tupleVal) if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement tuple value conversion failed: %w", err) + return Value{}, path.NewErrorf("error decoding msgpack extension body: numeric bound refinement tuple value conversion failed: %w", err) } if len(tupleVal) != 2 { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement tuple value conversion failed: expected 2 elements, got %d elements", len(tupleVal)) + return Value{}, path.NewErrorf("error decoding msgpack extension body: numeric bound refinement tuple value conversion failed: expected 2 elements, got %d elements", len(tupleVal)) } var boundVal *big.Float err = tupleVal[0].As(&boundVal) if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement bound value conversion failed: %w", err) + return Value{}, path.NewErrorf("error decoding msgpack extension body: numeric bound refinement bound value conversion failed: %w", err) } var inclusiveVal bool err = tupleVal[1].As(&inclusiveVal) if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: numeric bound refinement inclusive value conversion failed: %w", err) + return Value{}, path.NewErrorf("error decoding msgpack extension body: numeric bound refinement inclusive value conversion failed: %w", err) } if keyCode == refinement.KeyNumberLowerBound { @@ -481,12 +471,12 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath } case refinement.KeyCollectionLengthLowerBound, refinement.KeyCollectionLengthUpperBound: if !typ.Is(List{}) && !typ.Is(Map{}) && !typ.Is(Set{}) { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: length bound refinement for non-collection type") + return Value{}, path.NewErrorf("error decoding msgpack extension body: length bound refinement for non-collection type") } boundVal, err := rfnDec.DecodeInt() if err != nil { - return Value{}, path.NewErrorf("failed to decode msgpack extension body: length bound refinement must be integer") + return Value{}, path.NewErrorf("error decoding msgpack extension body: length bound refinement must be integer") } if keyCode == refinement.KeyCollectionLengthLowerBound { @@ -499,7 +489,7 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath if err != nil { return Value{}, path.NewErrorf("error skipping unknown extension body, keycode = %d: %w", keyCode, err) } - // We don't want to error here, as go-cty could introduce new refinements that we'd + // We don't want to error here, as cty could introduce new refinements that we'd // want to just ignore until this logic is updated continue } @@ -576,7 +566,7 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc return p.NewErrorf("error encoding Nullness value refinement key: %w", err) } - // It shouldn't be possible for an unknown value to be definitely null (i.e. nullness.value = true), + // It shouldn't be possible for an unknown value to be definitely null (i.e. Nullness.value = true), // as that should be represented by a known null value instead. This encoding is in place to be compliant // with Terraform's encoding which uses a definitely null refinement to collapse into a known null value. err = refnEnc.EncodeBool(data.Nullness()) @@ -595,14 +585,13 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc return p.NewErrorf("error encoding StringPrefix value refinement: unexpected refinement data of type %T", refnVal) } - if rawPrefix := data.PrefixValue(); rawPrefix != "" { - // Matching go-cty for the max prefix length allowed here + if prefix := data.PrefixValue(); prefix != "" { + // Matching cty for the max prefix length allowed here. // // This ensures the total size of the refinements blob does not exceed the limit // set by the decoder (1024). maxPrefixLength := 256 - prefix := rawPrefix - if len(rawPrefix) > maxPrefixLength { + if len(prefix) > maxPrefixLength { prefix = prefix[:maxPrefixLength-1] } @@ -629,10 +618,8 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc return p.NewErrorf("error encoding NumberLowerBound value refinement: unexpected refinement data of type %T", refnVal) } - // TODO: should check this isn't negative infinity? To match go-cty + // Numeric bound refinements are always a tuple of [number, bool] so we re-use the msgpack encoding logic on this refinement. boundTfType := Tuple{ElementTypes: []Type{Number, Bool}} - - // TODO: Do we need to do this? Kind of nasty boundTfVal := NewValue( boundTfType, []Value{ @@ -660,10 +647,8 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc return p.NewErrorf("error encoding NumberUpperBound value refinement: unexpected refinement data of type %T", refnVal) } - // TODO: should check this isn't positive infinity? To match go-cty + // Numeric bound refinements are always a tuple of [number, bool] so we re-use the msgpack encoding logic on this refinement. boundTfType := Tuple{ElementTypes: []Type{Number, Bool}} - - // TODO: Do we need to do this? Kind of nasty boundTfVal := NewValue( boundTfType, []Value{ @@ -735,25 +720,26 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc return nil } - // If we have at least one refinement to encode then we'll use the new - // representation of unknown values where refinement information is in the - // extension payload. + // Encode all of the unknown value refinements var lenBuf bytes.Buffer lenEnc := msgpack.NewEncoder(&lenBuf) lenEnc.EncodeMapLen(mapLen) //nolint err := enc.EncodeExtHeader(unknownWithRefinementsExt, lenBuf.Len()+refnBuf.Len()) if err != nil { - return p.NewErrorf("failed to write unknown value: %s", err) + return p.NewErrorf("error encoding UnknownValue with refinements: %s", err) } + _, err = enc.Writer().Write(lenBuf.Bytes()) if err != nil { - return p.NewErrorf("failed to write unknown value: %s", err) + return p.NewErrorf("error encoding UnknownValue with refinements: %s", err) } + _, err = enc.Writer().Write(refnBuf.Bytes()) if err != nil { - return p.NewErrorf("failed to write unknown value: %s", err) + return p.NewErrorf("error encoding UnknownValue with refinements: %s", err) } + return nil } From f1e26f47232c9b2b8b7b2509bd39a364e2d84060 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 10:45:39 -0500 Subject: [PATCH 13/18] remove comment --- tftypes/value_msgpack.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 96c659320..4ac22b4ab 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -17,7 +17,6 @@ import ( msgpackCodes "github.com/vmihailenco/msgpack/v5/msgpcode" ) -// https://github.com/zclconf/go-cty/blob/main/cty/msgpack/unknown.go#L32 const unknownWithRefinementsExt = 0x0c type msgPackUnknownType struct{} From 54bed957530ad9f637c64d476a5972729ba3daec Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 13:40:27 -0500 Subject: [PATCH 14/18] update the common methods in Value --- .../collection_length_lower_bound.go | 15 +- .../collection_length_upper_bound.go | 15 +- tftypes/refinement/nullness.go | 19 +- tftypes/refinement/number_lower_bound.go | 19 +- tftypes/refinement/number_upper_bound.go | 19 +- tftypes/refinement/refinement.go | 41 ++- tftypes/refinement/string_prefix.go | 15 +- tftypes/value.go | 48 +++- tftypes/value_msgpack_test.go | 154 ++++++----- tftypes/value_test.go | 239 ++++++++++++++++++ 10 files changed, 475 insertions(+), 109 deletions(-) diff --git a/tftypes/refinement/collection_length_lower_bound.go b/tftypes/refinement/collection_length_lower_bound.go index d42af1f90..67c6888bd 100644 --- a/tftypes/refinement/collection_length_lower_bound.go +++ b/tftypes/refinement/collection_length_lower_bound.go @@ -3,19 +3,24 @@ package refinement +import "fmt" + // TODO: doc type CollectionLengthLowerBound struct { value int64 } -func (n CollectionLengthLowerBound) Equal(Refinement) bool { - // TODO: implement - return false +func (n CollectionLengthLowerBound) Equal(other Refinement) bool { + otherVal, ok := other.(CollectionLengthLowerBound) + if !ok { + return false + } + + return n.LowerBound() == otherVal.LowerBound() } func (n CollectionLengthLowerBound) String() string { - // TODO: implement - return "todo - CollectionLengthLowerBound" + return fmt.Sprintf("length lower bound = %d", n.LowerBound()) } // TODO: doc diff --git a/tftypes/refinement/collection_length_upper_bound.go b/tftypes/refinement/collection_length_upper_bound.go index 84891bf3d..e0ffc32d6 100644 --- a/tftypes/refinement/collection_length_upper_bound.go +++ b/tftypes/refinement/collection_length_upper_bound.go @@ -3,19 +3,24 @@ package refinement +import "fmt" + // TODO: doc type CollectionLengthUpperBound struct { value int64 } -func (n CollectionLengthUpperBound) Equal(Refinement) bool { - // TODO: implement - return false +func (n CollectionLengthUpperBound) Equal(other Refinement) bool { + otherVal, ok := other.(CollectionLengthUpperBound) + if !ok { + return false + } + + return n.UpperBound() == otherVal.UpperBound() } func (n CollectionLengthUpperBound) String() string { - // TODO: implement - return "todo - CollectionLengthUpperBound" + return fmt.Sprintf("length upper bound = %d", n.UpperBound()) } // TODO: doc diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go index df88ccf28..342444a15 100644 --- a/tftypes/refinement/nullness.go +++ b/tftypes/refinement/nullness.go @@ -8,14 +8,23 @@ type Nullness struct { value bool } -func (n Nullness) Equal(Refinement) bool { - // TODO: implement - return false +func (n Nullness) Equal(other Refinement) bool { + otherVal, ok := other.(Nullness) + if !ok { + return false + } + + return n.Nullness() == otherVal.Nullness() } func (n Nullness) String() string { - // TODO: implement - return "todo - Nullness" + if n.value { + // This case should never happen, as an unknown value that is definitely null should be + // represented as a known null value. + return "null" + } + + return "not null" } // TODO: doc diff --git a/tftypes/refinement/number_lower_bound.go b/tftypes/refinement/number_lower_bound.go index a62f20639..7f803f721 100644 --- a/tftypes/refinement/number_lower_bound.go +++ b/tftypes/refinement/number_lower_bound.go @@ -4,6 +4,7 @@ package refinement import ( + "fmt" "math/big" ) @@ -13,14 +14,22 @@ type NumberLowerBound struct { value *big.Float } -func (n NumberLowerBound) Equal(Refinement) bool { - // TODO: implement - return false +func (n NumberLowerBound) Equal(other Refinement) bool { + otherVal, ok := other.(NumberLowerBound) + if !ok { + return false + } + + return n.IsInclusive() == otherVal.IsInclusive() && n.LowerBound().Cmp(otherVal.LowerBound()) == 0 } func (n NumberLowerBound) String() string { - // TODO: implement - return "todo - NumberLowerBound" + rangeDescription := "inclusive" + if !n.IsInclusive() { + rangeDescription = "exclusive" + } + + return fmt.Sprintf("lower bound = %s (%s)", n.LowerBound().String(), rangeDescription) } // TODO: doc diff --git a/tftypes/refinement/number_upper_bound.go b/tftypes/refinement/number_upper_bound.go index 9f2f0bf64..036d1d37d 100644 --- a/tftypes/refinement/number_upper_bound.go +++ b/tftypes/refinement/number_upper_bound.go @@ -4,6 +4,7 @@ package refinement import ( + "fmt" "math/big" ) @@ -13,14 +14,22 @@ type NumberUpperBound struct { value *big.Float } -func (n NumberUpperBound) Equal(Refinement) bool { - // TODO: implement - return false +func (n NumberUpperBound) Equal(other Refinement) bool { + otherVal, ok := other.(NumberUpperBound) + if !ok { + return false + } + + return n.IsInclusive() == otherVal.IsInclusive() && n.UpperBound().Cmp(otherVal.UpperBound()) == 0 } func (n NumberUpperBound) String() string { - // TODO: implement - return "todo - NumberUpperBound" + rangeDescription := "inclusive" + if !n.IsInclusive() { + rangeDescription = "exclusive" + } + + return fmt.Sprintf("upper bound = %s (%s)", n.UpperBound().String(), rangeDescription) } // TODO: doc diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go index 57506326c..52f2f14fe 100644 --- a/tftypes/refinement/refinement.go +++ b/tftypes/refinement/refinement.go @@ -5,6 +5,8 @@ package refinement import ( "fmt" + "sort" + "strings" ) type Key int64 @@ -78,10 +80,41 @@ type Refinement interface { // TODO: docs type Refinements map[Key]Refinement -func (r Refinements) Equal(o Refinements) bool { - return false +func (r Refinements) Equal(other Refinements) bool { + if len(r) != len(other) { + return false + } + + for key, refnVal := range r { + otherRefnVal, ok := other[key] + if !ok { + // Didn't find a refinement at the same key + return false + } + + if !refnVal.Equal(otherRefnVal) { + // Refinement data is not equal + return false + } + } + + return true } func (r Refinements) String() string { - // TODO: Not sure when this is used, should just aggregate and call all underlying refinements.String() method - return "todo" + var res strings.Builder + + keys := make([]Key, 0, len(r)) + for k := range r { + keys = append(keys, k) + } + + sort.Slice(keys, func(a, b int) bool { return keys[a] < keys[b] }) + for pos, key := range keys { + if pos != 0 { + res.WriteString(", ") + } + res.WriteString(r[key].String()) + } + + return res.String() } diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go index 246edd5b2..2ced21ccc 100644 --- a/tftypes/refinement/string_prefix.go +++ b/tftypes/refinement/string_prefix.go @@ -3,19 +3,24 @@ package refinement +import "fmt" + // TODO: doc type StringPrefix struct { value string } -func (s StringPrefix) Equal(Refinement) bool { - // TODO: implement - return false +func (s StringPrefix) Equal(other Refinement) bool { + otherVal, ok := other.(StringPrefix) + if !ok { + return false + } + + return s.PrefixValue() == otherVal.PrefixValue() } func (s StringPrefix) String() string { - // TODO: implement - return "todo - stringPrefix" + return fmt.Sprintf("prefix = %q", s.PrefixValue()) } // TODO: doc diff --git a/tftypes/value.go b/tftypes/value.go index 4a45ee47e..f81e16f42 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -63,10 +63,16 @@ func (val Value) String() string { return typ.String() + "" } - // TODO: print refinements - if !val.IsKnown() { - return typ.String() + "" + var res strings.Builder + res.WriteString(typ.String()) + res.WriteString(" 0 { + res.WriteString(", " + val.Refinements().String()) + } + res.WriteString(">") + + return res.String() } // everything else is built up @@ -230,7 +236,13 @@ func (val Value) Equal(o Value) bool { return false } - // TODO: compare refinements + if len(val.refinements) != len(o.refinements) { + return false + } + + if len(val.refinements) > 0 && !val.refinements.Equal(o.refinements) { + return false + } deepEqual, err := val.deepEqual(o) if err != nil { @@ -242,9 +254,6 @@ func (val Value) Equal(o Value) bool { // Copy returns a defensively-copied clone of Value that shares no underlying // data structures with the original Value and can be mutated without // accidentally mutating the original. -// -// TODO: Make sure this actually works for refinements. Consuming packages should not be able to mutate refinements of val -// TODO: Add docs referencing refinements func (val Value) Copy() Value { newVal := val.value switch v := val.value.(type) { @@ -263,7 +272,15 @@ func (val Value) Copy() Value { } newTfValue := NewValue(val.Type(), newVal) - newTfValue.refinements = val.refinements + + if len(val.refinements) > 0 { + newRefinements := make(refinement.Refinements, len(val.refinements)) + for key, refnVal := range val.refinements { + newRefinements[key] = refnVal + } + + newTfValue.refinements = newRefinements + } return newTfValue } @@ -611,19 +628,28 @@ func unexpectedValueTypeError(p *AttributePath, expected, got interface{}, typ T return p.NewErrorf("unexpected value type %T, %s values must be of type %T", got, typ, expected) } -// TODO: do we need to return an error? Like if you attempt to refine a value improperly (like string prefix on a number)? +// TODO: doc, mention returning value if not unknown func (val Value) Refine(refinements refinement.Refinements) Value { newVal := val.Copy() + // Refinements are only relevant for unknown values + if val.IsKnown() { + return newVal + } + if len(refinements) >= 0 { - newVal.refinements = refinements + newRefinements := make(refinement.Refinements, len(refinements)) + for key, refnVal := range refinements { + newRefinements[key] = refnVal + } + newVal.refinements = newRefinements } return newVal } +// TODO: doc, mention copy func (val Value) Refinements() refinement.Refinements { - // TODO: is this copy really needed? valCopy := val.Copy() return valCopy.refinements diff --git a/tftypes/value_msgpack_test.go b/tftypes/value_msgpack_test.go index 1eb0fd230..982ff8787 100644 --- a/tftypes/value_msgpack_test.go +++ b/tftypes/value_msgpack_test.go @@ -15,13 +15,6 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" ) -// Hex encoding of the long prefix refinements used in this test -var longPrefixRefinement = "c801050c8201c202d9ff7072656669783a2f2f303132333435363738392d303132333435363738392d303132333435363738392d30313" + - "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + - "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + - "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + - "2333435363738392d30313233" - func TestValueFromMsgPack(t *testing.T) { t.Parallel() type testCase struct { @@ -542,13 +535,6 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: List{ElementType: DynamicPseudoType}, }, - "unknown-dynamic-refinements-ignored": { - hex: "d40000", - value: NewValue(DynamicPseudoType, UnknownValue).Refine(refinement.Refinements{ - refinement.KeyNullness: refinement.NewNullness(false), - }), - typ: DynamicPseudoType, - }, "unknown-bool-with-nullness-refinement": { hex: "c7030c8101c2", value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ @@ -584,14 +570,6 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: Object{AttributeTypes: map[string]Type{"attr": String}}, }, - "unknown-string-with-empty-prefix-refinement": { - hex: "c7030c8101c2", - value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ - refinement.KeyNullness: refinement.NewNullness(false), - refinement.KeyStringPrefix: refinement.NewStringPrefix(""), - }), - typ: String, - }, "unknown-string-with-prefix-refinement": { hex: "c70e0c8201c202a97072656669783a2f2f", value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ @@ -600,32 +578,6 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: String, }, - "unknown-string-with-long-prefix-refinement-one": { - // This prefix will be cutoff at 256 bytes, so it will be equal to the other long prefix test. - hex: longPrefixRefinement, - value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ - refinement.KeyNullness: refinement.NewNullness(false), - refinement.KeyStringPrefix: refinement.NewStringPrefix( - "prefix://0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + - "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + - "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-thiswillbecutoff1", - ), - }), - typ: String, - }, - "unknown-string-with-long-prefix-refinement-two": { - // This prefix will be cutoff at 256 bytes, so it will be equal to the other long prefix test. - hex: longPrefixRefinement, - value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ - refinement.KeyNullness: refinement.NewNullness(false), - refinement.KeyStringPrefix: refinement.NewStringPrefix( - "prefix://0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + - "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + - "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-thiswillbecutoff2", - ), - }), - typ: String, - }, "unknown-number-with-bound-refinements-integers-inclusive": { hex: "c70b0c8301c2039201c3049205c3", value: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ @@ -689,22 +641,6 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: Set{ElementType: Bool}, }, - "unknown-with-invalid-refinement-type": { - hex: "d40000", - value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ - // This refinement will be ignored since only strings will attempt to encode this - refinement.KeyStringPrefix: refinement.NewStringPrefix("ignored"), - }), - typ: Bool, - }, - "unknown-with-invalid-refinement-data": { - hex: "d40000", - value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ - // This refinement will be ignored since we don't know how to encode it - refinement.Key(100): refinement.NewStringPrefix("ignored"), - }), - typ: Bool, - }, } for name, test := range tests { name, test := name, test @@ -736,6 +672,96 @@ func TestValueFromMsgPack(t *testing.T) { } } +// This test covers certain scenarios where we ignore refinement data during marshalling that are either invalid or not needed. +func TestMarshalMsgPack_refinement_exceptions(t *testing.T) { + t.Parallel() + + // Hex encoding of the long prefix refinements that are eventually truncated in this test + longPrefixRefinement := "c801050c8201c202d9ff7072656669783a2f2f303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d303132333435363738392d30313" + + "2333435363738392d30313233" + + tests := map[string]struct { + value Value + typ Type + expectedHex string + }{ + "unknown-dynamic-refinements-ignored": { + expectedHex: "d40000", + value: NewValue(DynamicPseudoType, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + typ: DynamicPseudoType, + }, + "unknown-string-with-empty-prefix-refinement": { + expectedHex: "c7030c8101c2", + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix(""), + }), + typ: String, + }, + "unknown-string-with-long-prefix-refinement-one": { + // This prefix will be cutoff at 256 bytes, so it will be equal to the other long prefix test. + expectedHex: longPrefixRefinement, + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix( + "prefix://0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-thiswillbecutoff1", + ), + }), + typ: String, + }, + "unknown-string-with-long-prefix-refinement-two": { + // This prefix will be cutoff at 256 bytes, so it will be equal to the other long prefix test. + expectedHex: longPrefixRefinement, + value: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix( + "prefix://0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-" + + "0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-0123456789-thiswillbecutoff2", + ), + }), + typ: String, + }, + "unknown-with-invalid-refinement-type": { + expectedHex: "d40000", + value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + // This refinement will be ignored since only strings will attempt to encode this + refinement.KeyStringPrefix: refinement.NewStringPrefix("ignored"), + }), + typ: Bool, + }, + "unknown-with-invalid-refinement-data": { + expectedHex: "d40000", + value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + // This refinement will be ignored since we don't know how to encode it + refinement.Key(100): refinement.NewStringPrefix("ignored"), + }), + typ: Bool, + }, + } + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + got, err := test.value.MarshalMsgPack(test.typ) + if err != nil { + t.Fatalf("unexpected error marshaling: %s", err) + } + res := hex.EncodeToString(got) + if res != test.expectedHex { + t.Errorf("expected msgpack to be %q, got %q", test.expectedHex, res) + } + }) + } +} + func TestMarshalMsgPack_error(t *testing.T) { t.Parallel() tests := map[string]struct { diff --git a/tftypes/value_test.go b/tftypes/value_test.go index f13d0c5bd..9421962c9 100644 --- a/tftypes/value_test.go +++ b/tftypes/value_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-go/tftypes/refinement" ) func numberComparer(i, j *big.Float) bool { @@ -783,11 +784,168 @@ func TestValueEqual(t *testing.T) { val2: NewValue(String, UnknownValue), equal: true, }, + "unknownEqual-bool-refinements": { + val1: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + val2: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + equal: true, + }, + "unknownEqual-string-refinements": { + val1: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix("hello"), + }), + val2: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix("hello"), + }), + equal: true, + }, + "unknownEqual-number-refinements": { + val1: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), false), + }), + val2: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), false), + }), + equal: true, + }, + "unknownEqual-number-refinements-float": { + val1: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1.23), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5.67), false), + }), + val2: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1.23), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5.67), false), + }), + equal: true, + }, + "unknownEqual-list-refinements": { + val1: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + val2: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + equal: true, + }, "unknownDiff": { val1: NewValue(String, UnknownValue), val2: NewValue(String, "world"), equal: false, }, + "unknownDiff-bool-refinements": { + val1: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + val2: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(true), + }), + equal: false, + }, + "unknownDiff-string-refinements": { + val1: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix("hello"), + }), + val2: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix("world"), + }), + equal: false, + }, + "unknownDiff-number-refinements-lowerInclusiveDiff": { + val1: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + val2: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), false), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + equal: false, + }, + "unknownDiff-number-refinements-upperInclusiveDiff": { + val1: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + val2: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), false), + }), + equal: false, + }, + "unknownDiff-number-refinements-lowerBoundDiff": { + val1: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + val2: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(3), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + equal: false, + }, + "unknownDiff-number-refinements-upperBoundDiff": { + val1: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(5), true), + }), + val2: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(1), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(3), true), + }), + equal: false, + }, + "unknownDiff-list-refinements-lowerBoundDiff": { + val1: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + val2: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(3), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + equal: false, + }, + "unknownDiff-list-refinements-upperBoundDiff": { + val1: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(5), + }), + val2: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(1), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(3), + }), + equal: false, + }, "listEqual": { val1: NewValue(List{ElementType: String}, []Value{ NewValue(String, "hello"), @@ -1721,6 +1879,59 @@ func TestValueString(t *testing.T) { in: Value{}, expected: "invalid typeless tftypes.Value<>", }, + "unknown-bool": { + in: NewValue(Bool, UnknownValue), + expected: "tftypes.Bool", + }, + "unknown-bool-with-nullness-refinement": { + in: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + }), + expected: `tftypes.Bool`, + }, + "unknown-string": { + in: NewValue(String, UnknownValue), + expected: "tftypes.String", + }, + "unknown-string-with-multiple-refinements": { + in: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyStringPrefix: refinement.NewStringPrefix("str://"), + }), + expected: `tftypes.String`, + }, + "unknown-number": { + in: NewValue(Number, UnknownValue), + expected: "tftypes.Number", + }, + "unknown-number-with-multiple-refinements": { + in: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(5), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(10), true), + }), + expected: `tftypes.Number`, + }, + "unknown-number-float-with-multiple-refinements": { + in: NewValue(Number, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyNumberLowerBound: refinement.NewNumberLowerBound(big.NewFloat(5.67), true), + refinement.KeyNumberUpperBound: refinement.NewNumberUpperBound(big.NewFloat(10.123), true), + }), + expected: `tftypes.Number`, + }, + "unknown-list": { + in: NewValue(List{ElementType: String}, UnknownValue), + expected: "tftypes.List[tftypes.String]", + }, + "unknown-list-with-multiple-refinements": { + in: NewValue(List{ElementType: String}, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(false), + refinement.KeyCollectionLengthLowerBound: refinement.NewCollectionLengthLowerBound(2), + refinement.KeyCollectionLengthUpperBound: refinement.NewCollectionLengthUpperBound(7), + }), + expected: `tftypes.List[tftypes.String]`, + }, "string": { in: NewValue(String, "hello"), expected: "tftypes.String<\"hello\">", @@ -1875,3 +2086,31 @@ func TestValueString(t *testing.T) { }) } } + +func TestValueRefine_immutable(t *testing.T) { + t.Parallel() + + originalRefinements := refinement.Refinements{refinement.KeyStringPrefix: refinement.NewStringPrefix("hello")} + originalVal := NewValue(String, UnknownValue).Refine(originalRefinements) + + // Attempt to mutate the original refinements map + originalRefinements[refinement.KeyStringPrefix] = refinement.NewStringPrefix("world") + + if !originalVal.Equal(NewValue(String, UnknownValue).Refine(refinement.Refinements{refinement.KeyStringPrefix: refinement.NewStringPrefix("hello")})) { + t.Fatal("unexpected Refinements mutation") + } +} + +func TestValueRefinements_immutable(t *testing.T) { + t.Parallel() + + originalVal := NewValue(String, UnknownValue).Refine(refinement.Refinements{refinement.KeyStringPrefix: refinement.NewStringPrefix("hello")}) + originalRefinements := originalVal.Refinements() + + // Attempt to mutate the original refinements map + originalRefinements[refinement.KeyStringPrefix] = refinement.NewStringPrefix("world") + + if !originalVal.Equal(NewValue(String, UnknownValue).Refine(refinement.Refinements{refinement.KeyStringPrefix: refinement.NewStringPrefix("hello")})) { + t.Fatal("unexpected Refinements mutation") + } +} From bd66111920cb44641ae0e7c32882ace6e113a666 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 15:54:53 -0500 Subject: [PATCH 15/18] package docs --- .../collection_length_lower_bound.go | 8 ++++--- .../collection_length_upper_bound.go | 8 ++++--- tftypes/refinement/doc.go | 8 ++++++- tftypes/refinement/nullness.go | 22 ++++++++++++++++--- tftypes/refinement/number_lower_bound.go | 11 ++++++---- tftypes/refinement/number_upper_bound.go | 11 ++++++---- tftypes/refinement/refinement.go | 10 ++++++--- tftypes/refinement/string_prefix.go | 10 ++++++--- tftypes/value.go | 10 +++++---- 9 files changed, 70 insertions(+), 28 deletions(-) diff --git a/tftypes/refinement/collection_length_lower_bound.go b/tftypes/refinement/collection_length_lower_bound.go index 67c6888bd..94ddc5541 100644 --- a/tftypes/refinement/collection_length_lower_bound.go +++ b/tftypes/refinement/collection_length_lower_bound.go @@ -5,7 +5,8 @@ package refinement import "fmt" -// TODO: doc +// CollectionLengthLowerBound represents an unknown value refinement which indicates the length of the final collection value will be +// at least the specified int64 value. This refinement can only be applied to List, Map, and Set types. type CollectionLengthLowerBound struct { value int64 } @@ -23,14 +24,15 @@ func (n CollectionLengthLowerBound) String() string { return fmt.Sprintf("length lower bound = %d", n.LowerBound()) } -// TODO: doc +// LowerBound returns the int64 value that the final value's collection length will be at least. func (n CollectionLengthLowerBound) LowerBound() int64 { return n.value } func (n CollectionLengthLowerBound) unimplementable() {} -// TODO: doc +// NewCollectionLengthLowerBound returns the CollectionLengthLowerBound unknown value refinement which indicates the length of the final +// collection value will be at least the specified int64 value. This refinement can only be applied to List, Map, and Set types. func NewCollectionLengthLowerBound(value int64) Refinement { return CollectionLengthLowerBound{ value: value, diff --git a/tftypes/refinement/collection_length_upper_bound.go b/tftypes/refinement/collection_length_upper_bound.go index e0ffc32d6..889250d25 100644 --- a/tftypes/refinement/collection_length_upper_bound.go +++ b/tftypes/refinement/collection_length_upper_bound.go @@ -5,7 +5,8 @@ package refinement import "fmt" -// TODO: doc +// CollectionLengthUpperBound represents an unknown value refinement which indicates the length of the final collection value will be +// at most the specified int64 value. This refinement can only be applied to List, Map, and Set types. type CollectionLengthUpperBound struct { value int64 } @@ -23,14 +24,15 @@ func (n CollectionLengthUpperBound) String() string { return fmt.Sprintf("length upper bound = %d", n.UpperBound()) } -// TODO: doc +// UpperBound returns the int64 value that the final value's collection length will be at most. func (n CollectionLengthUpperBound) UpperBound() int64 { return n.value } func (n CollectionLengthUpperBound) unimplementable() {} -// TODO: doc +// NewCollectionLengthUpperBound returns the CollectionLengthUpperBound unknown value refinement which indicates the length of the final +// collection value will be at most the specified int64 value. This refinement can only be applied to List, Map, and Set types. func NewCollectionLengthUpperBound(value int64) Refinement { return CollectionLengthUpperBound{ value: value, diff --git a/tftypes/refinement/doc.go b/tftypes/refinement/doc.go index 85283c929..eba8d649b 100644 --- a/tftypes/refinement/doc.go +++ b/tftypes/refinement/doc.go @@ -1,5 +1,11 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -// TODO: docs +// The refinement package contains the interfaces and structs that represent unknown value refinement data. Refinements contain +// additional constraints about unknown values and what their eventual known values can be. In certain scenarios, Terraform can +// use these constraints to produce known results from unknown values. (like evaluating a count expression comparing an unknown +// value to "null") +// +// Unknown value refinements can be added to a `tftypes.Value` via the `(tftypes.Value).Refine` method. Refinements on an unknown +// `tftypes.Value` can be retrieved via the `(tftypes.Value).Refinements()` method. package refinement diff --git a/tftypes/refinement/nullness.go b/tftypes/refinement/nullness.go index 342444a15..d74e6440e 100644 --- a/tftypes/refinement/nullness.go +++ b/tftypes/refinement/nullness.go @@ -3,7 +3,12 @@ package refinement -// TODO: doc +// Nullness represents an unknown value refinement that indicates the final value will definitely not be null (Nullness = false). This refinement +// can be applied to a value of any type (excluding DynamicPseudoType). +// +// While an unknown value can be refined to indicate that the final value will definitely be null (Nullness = true), there is no practical reason +// to do this. This option is exposed to maintain parity with Terraform's type system, while all practical usages of this refinement should collapse +// to known null values instead. type Nullness struct { value bool } @@ -27,14 +32,25 @@ func (n Nullness) String() string { return "not null" } -// TODO: doc +// Nullness returns the underlying refinement data indicating: +// - When "false", the final value will definitely not be null. +// - When "true", the final value will definitely be null. +// +// While an unknown value can be refined to indicate that the final value will definitely be null (Nullness = true), there is no practical reason +// to do this. This option is exposed to maintain parity with Terraform's type system, while all practical usages of this refinement should collapse +// to known null values instead. func (n Nullness) Nullness() bool { return n.value } func (n Nullness) unimplementable() {} -// TODO: doc +// NewNullness returns the Nullness unknown value refinement that indicates the final value will definitely not be null (Nullness = false). This refinement +// can be applied to a value of any type (excluding DynamicPseudoType). +// +// While an unknown value can be refined to indicate that the final value will definitely be null (Nullness = true), there is no practical reason +// to do this. This option is exposed to maintain parity with Terraform's type system, while all practical usages of this refinement should collapse +// to known null values instead. func NewNullness(value bool) Refinement { return Nullness{ value: value, diff --git a/tftypes/refinement/number_lower_bound.go b/tftypes/refinement/number_lower_bound.go index 7f803f721..0fe83f783 100644 --- a/tftypes/refinement/number_lower_bound.go +++ b/tftypes/refinement/number_lower_bound.go @@ -8,7 +8,8 @@ import ( "math/big" ) -// TODO: doc +// NumberLowerBound represents an unknown value refinement that indicates the final value will not be less than the specified +// *big.Float value, as well as whether that bound is inclusive or exclusive. This refinement can only be applied to the Number type. type NumberLowerBound struct { inclusive bool value *big.Float @@ -32,19 +33,21 @@ func (n NumberLowerBound) String() string { return fmt.Sprintf("lower bound = %s (%s)", n.LowerBound().String(), rangeDescription) } -// TODO: doc +// IsInclusive returns whether the bound returned by the `LowerBound` method is inclusive or exclusive. func (n NumberLowerBound) IsInclusive() bool { return n.inclusive } -// TODO: doc +// LowerBound returns the *big.Float value that the final value will not be less than. The `IsInclusive` method must also be used during +// comparison to determine whether the bound is inclusive or exclusive. func (n NumberLowerBound) LowerBound() *big.Float { return n.value } func (n NumberLowerBound) unimplementable() {} -// TODO: doc +// NewNumberLowerBound returns the NumberLowerBound unknown value refinement that indicates the final value will not be less than the specified +// *big.Float value, as well as whether that bound is inclusive or exclusive. This refinement can only be applied to the Number type. func NewNumberLowerBound(value *big.Float, inclusive bool) Refinement { return NumberLowerBound{ value: value, diff --git a/tftypes/refinement/number_upper_bound.go b/tftypes/refinement/number_upper_bound.go index 036d1d37d..601e32841 100644 --- a/tftypes/refinement/number_upper_bound.go +++ b/tftypes/refinement/number_upper_bound.go @@ -8,7 +8,8 @@ import ( "math/big" ) -// TODO: doc +// NumberUpperBound represents an unknown value refinement that indicates the final value will not be greater than the specified +// *big.Float value, as well as whether that bound is inclusive or exclusive. This refinement can only be applied to the Number type. type NumberUpperBound struct { inclusive bool value *big.Float @@ -32,19 +33,21 @@ func (n NumberUpperBound) String() string { return fmt.Sprintf("upper bound = %s (%s)", n.UpperBound().String(), rangeDescription) } -// TODO: doc +// IsInclusive returns whether the bound returned by the `UpperBound` method is inclusive or exclusive. func (n NumberUpperBound) IsInclusive() bool { return n.inclusive } -// TODO: doc +// UpperBound returns the *big.Float value that the final value will not be greater than. The `IsInclusive` method must also be used during +// comparison to determine whether the bound is inclusive or exclusive. func (n NumberUpperBound) UpperBound() *big.Float { return n.value } func (n NumberUpperBound) unimplementable() {} -// TODO: doc +// NewNumberUpperBound returns the NumberUpperBound unknown value refinement that indicates the final value will not be greater than the specified +// *big.Float value, as well as whether that bound is inclusive or exclusive. This refinement can only be applied to the Number type. func NewNumberUpperBound(value *big.Float, inclusive bool) Refinement { return NumberUpperBound{ value: value, diff --git a/tftypes/refinement/refinement.go b/tftypes/refinement/refinement.go index 52f2f14fe..0b5b2517a 100644 --- a/tftypes/refinement/refinement.go +++ b/tftypes/refinement/refinement.go @@ -12,7 +12,6 @@ import ( type Key int64 func (k Key) String() string { - // TODO: Not sure when this is used, double check the names switch k { case KeyNullness: return "nullness" @@ -22,6 +21,10 @@ func (k Key) String() string { return "number_lower_bound" case KeyNumberUpperBound: return "number_upper_bound" + case KeyCollectionLengthLowerBound: + return "collection_length_lower_bound" + case KeyCollectionLengthUpperBound: + return "collection_length_upper_bound" default: return fmt.Sprintf("unsupported refinement: %d", k) } @@ -65,7 +68,8 @@ const ( KeyCollectionLengthUpperBound = Key(6) ) -// TODO: docs +// Refinement represents an unknown value refinement with data constraints relevant to the final value. This interface can be asserted further +// with the associated structs in the `refinement` package to extract underlying refinement data. type Refinement interface { // Equal should return true if the Refinement is considered equivalent to the // Refinement passed as an argument. @@ -77,7 +81,7 @@ type Refinement interface { unimplementable() // prevents external implementations, all refinements are defined in the Terraform/HCL type system go-cty. } -// TODO: docs +// Refinements represents a map of unknown value refinement data. type Refinements map[Key]Refinement func (r Refinements) Equal(other Refinements) bool { diff --git a/tftypes/refinement/string_prefix.go b/tftypes/refinement/string_prefix.go index 2ced21ccc..5852262df 100644 --- a/tftypes/refinement/string_prefix.go +++ b/tftypes/refinement/string_prefix.go @@ -5,7 +5,9 @@ package refinement import "fmt" -// TODO: doc +// StringPrefix represents an unknown value refinement that indicates the final value will be prefixed with the specified string value. +// String prefixes that exceed 256 characters in length will be truncated and empty string prefixes will not be encoded. This refinement can +// only be applied to the String type. type StringPrefix struct { value string } @@ -23,14 +25,16 @@ func (s StringPrefix) String() string { return fmt.Sprintf("prefix = %q", s.PrefixValue()) } -// TODO: doc +// PrefixValue returns the string value that the final value will be prefixed with. func (s StringPrefix) PrefixValue() string { return s.value } func (s StringPrefix) unimplementable() {} -// TODO: doc +// NewStringPrefix returns the StringPrefix unknown value refinement that indicates the final value will be prefixed with the specified +// string value. String prefixes that exceed 256 characters in length will be truncated and empty string prefixes will not be encoded. This +// refinement can only be applied to the String type. func NewStringPrefix(value string) Refinement { return StringPrefix{ value: value, diff --git a/tftypes/value.go b/tftypes/value.go index f81e16f42..cae7a8e9a 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -42,8 +42,6 @@ type ValueCreator interface { // The recommended usage of a Value is to check that it is known, using // Value.IsKnown, then to convert it to a Go type, using Value.As. The Go type // can then be manipulated. -// -// TODO: add docs for new refinement data type Value struct { typ Type value interface{} @@ -628,7 +626,10 @@ func unexpectedValueTypeError(p *AttributePath, expected, got interface{}, typ T return p.NewErrorf("unexpected value type %T, %s values must be of type %T", got, typ, expected) } -// TODO: doc, mention returning value if not unknown +// Refine is used to apply unknown refinement data to a Value. This method will return a copy of the Value, fully +// replacing all the existing refinement data with the provided refinements. +// +// If the Value is not unknown, then a copy of the Value will be returned with no refinements. func (val Value) Refine(refinements refinement.Refinements) Value { newVal := val.Copy() @@ -648,8 +649,9 @@ func (val Value) Refine(refinements refinement.Refinements) Value { return newVal } -// TODO: doc, mention copy +// Refinements returns the unknown refinement data for the Value. func (val Value) Refinements() refinement.Refinements { + // Copy the data first to prevent any mutation of the refinement map data. valCopy := val.Copy() return valCopy.refinements From e6d0193339d6b48912f47ee27f48c257358a7909 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 16:05:37 -0500 Subject: [PATCH 16/18] add more tests --- tftypes/value_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tftypes/value_test.go b/tftypes/value_test.go index 9421962c9..a55896bdf 100644 --- a/tftypes/value_test.go +++ b/tftypes/value_test.go @@ -784,6 +784,11 @@ func TestValueEqual(t *testing.T) { val2: NewValue(String, UnknownValue), equal: true, }, + "unknownEqual-empty-refinements": { + val1: NewValue(Bool, UnknownValue), + val2: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{}), + equal: true, + }, "unknownEqual-bool-refinements": { val1: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), @@ -848,6 +853,20 @@ func TestValueEqual(t *testing.T) { val2: NewValue(String, "world"), equal: false, }, + "unknownDiff-nil-refinements": { + val1: NewValue(Bool, UnknownValue), + val2: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(true), + }), + equal: false, + }, + "unknownDiff-empty-refinements": { + val1: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(true), + }), + val2: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{}), + equal: false, + }, "unknownDiff-bool-refinements": { val1: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ refinement.KeyNullness: refinement.NewNullness(false), From fdf5a8ed7db2bca8e4050b9612042d710a9e190f Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 25 Nov 2024 16:32:11 -0500 Subject: [PATCH 17/18] added tests --- tftypes/value_msgpack_test.go | 55 ++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/tftypes/value_msgpack_test.go b/tftypes/value_msgpack_test.go index 982ff8787..03dfaa436 100644 --- a/tftypes/value_msgpack_test.go +++ b/tftypes/value_msgpack_test.go @@ -535,6 +535,14 @@ func TestValueFromMsgPack(t *testing.T) { }), typ: List{ElementType: DynamicPseudoType}, }, + // This encoding, while unlikely in practice, is supported. Terraform should collapse this to a known null value before reaching providers. + "unknown-with-nullness-true": { + hex: "c7030c8101c3", + value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyNullness: refinement.NewNullness(true), + }), + typ: Bool, + }, "unknown-bool-with-nullness-refinement": { hex: "c7030c8101c2", value: NewValue(Bool, UnknownValue).Refine(refinement.Refinements{ @@ -671,9 +679,54 @@ func TestValueFromMsgPack(t *testing.T) { }) } } +func TestValueFromMsgPack_refinements(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + hex string + expectedValue Value + typ Type + }{ + "unsupported-refinement-on-bool": { + // This hex value encodes the Nullness refinement as Key(100), which doesn't exist and should be ignored. + hex: "c7030c8164c2", + expectedValue: NewValue(Bool, UnknownValue), + typ: Bool, + }, + "unsupported-refinement-on-prefixed-string": { + // This hex value encodes the Nullness refinement as Key(100), which doesn't exist and should be ignored. + // The hex value also includes a valid string prefix which should be preserved. + hex: "c70e0c8264c202a97072656669783a2f2f", + expectedValue: NewValue(String, UnknownValue).Refine(refinement.Refinements{ + refinement.KeyStringPrefix: refinement.NewStringPrefix("prefix://"), + }), + typ: String, + }, + } + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + + b, err := hex.DecodeString(test.hex) + if err != nil { + t.Fatalf("unexpected error parsing hex: %s", err) + } + + val, err := ValueFromMsgPack(b, test.typ) + if err != nil { + t.Fatalf("unexpected error unmarshaling: %s", err) + } + + if test.expectedValue.String() != val.String() { + t.Errorf("Unexpected results (-wanted +got): %s", cmp.Diff(test.expectedValue, val)) + } + }) + } +} // This test covers certain scenarios where we ignore refinement data during marshalling that are either invalid or not needed. -func TestMarshalMsgPack_refinement_exceptions(t *testing.T) { +func TestMarshalMsgPack_refinements(t *testing.T) { t.Parallel() // Hex encoding of the long prefix refinements that are eventually truncated in this test From bd716fcfe40719c617fc86ad5e1534e93a731f06 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 26 Nov 2024 15:02:14 -0500 Subject: [PATCH 18/18] doc comment --- tftypes/value.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tftypes/value.go b/tftypes/value.go index cae7a8e9a..8b96b3d51 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -46,6 +46,8 @@ type Value struct { typ Type value interface{} + // refinements represents the unknown value refinement data associated with this Value. + // This field is only populated for unknown values. refinements refinement.Refinements }