From 27b0d44dae418a15c37412dc9485276dc96712e1 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 20 Jul 2018 12:40:22 -0400 Subject: [PATCH 1/2] don't set Unknown for computed values Any value that is not set in the configuration should decode as a Null value. Unknown should still be returned if a computed value expression is unknown. --- configs/configschema/decoder_spec.go | 34 ++++------------------- configs/configschema/decoder_spec_test.go | 14 ++++++++-- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/configs/configschema/decoder_spec.go b/configs/configschema/decoder_spec.go index 2b1b0cacbf..9a9b70acc6 100644 --- a/configs/configschema/decoder_spec.go +++ b/configs/configschema/decoder_spec.go @@ -2,7 +2,6 @@ package configschema import ( "github.com/hashicorp/hcl2/hcldec" - "github.com/zclconf/go-cty/cty" ) var mapLabelNames = []string{"key"} @@ -11,9 +10,8 @@ var mapLabelNames = []string{"key"} // using the facilities in the hcldec package. // // The returned specification is guaranteed to return a value of the same type -// returned by method ImpliedType, but it may contain null or unknown values if -// any of the block attributes are defined as optional and/or computed -// respectively. +// returned by method ImpliedType, but it may contain null values if any of the +// block attributes are defined as optional and/or computed respectively. func (b *Block) DecoderSpec() hcldec.Spec { ret := hcldec.ObjectSpec{} if b == nil { @@ -21,30 +19,10 @@ func (b *Block) DecoderSpec() hcldec.Spec { } for name, attrS := range b.Attributes { - switch { - case attrS.Computed && attrS.Optional: - // In this special case we use an unknown value as a default - // to get the intended behavior that the result is computed - // unless it has been explicitly set in config. - ret[name] = &hcldec.DefaultSpec{ - Primary: &hcldec.AttrSpec{ - Name: name, - Type: attrS.Type, - }, - Default: &hcldec.LiteralSpec{ - Value: cty.UnknownVal(attrS.Type), - }, - } - case attrS.Computed: - ret[name] = &hcldec.LiteralSpec{ - Value: cty.UnknownVal(attrS.Type), - } - default: - ret[name] = &hcldec.AttrSpec{ - Name: name, - Type: attrS.Type, - Required: attrS.Required, - } + ret[name] = &hcldec.AttrSpec{ + Name: name, + Type: attrS.Type, + Required: attrS.Required, } } diff --git a/configs/configschema/decoder_spec_test.go b/configs/configschema/decoder_spec_test.go index 50cfbecf07..87047a6828 100644 --- a/configs/configschema/decoder_spec_test.go +++ b/configs/configschema/decoder_spec_test.go @@ -55,6 +55,11 @@ func TestBlockDecoderSpec(t *testing.T) { Optional: true, Computed: true, }, + "optional_computed_unknown": { + Type: cty.String, + Optional: true, + Computed: true, + }, }, }, hcltest.MockBody(&hcl.BodyContent{ @@ -67,14 +72,19 @@ func TestBlockDecoderSpec(t *testing.T) { Name: "optional_computed_overridden", Expr: hcltest.MockExprLiteral(cty.True), }, + "optional_computed_unknown": { + Name: "optional_computed_overridden", + Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.String)), + }, }, }), cty.ObjectVal(map[string]cty.Value{ "optional": cty.NullVal(cty.Number), "required": cty.StringVal("5"), // converted from number to string - "computed": cty.UnknownVal(cty.List(cty.Bool)), - "optional_computed": cty.UnknownVal(cty.Map(cty.Bool)), + "computed": cty.NullVal(cty.List(cty.Bool)), + "optional_computed": cty.NullVal(cty.Map(cty.Bool)), "optional_computed_overridden": cty.True, + "optional_computed_unknown": cty.UnknownVal(cty.String), }), 0, }, From 8a4931d334a6174588af53e03bfcb4558b8dabb3 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 20 Jul 2018 12:59:50 -0400 Subject: [PATCH 2/2] CoerceValue should insert Null for unset attrs This matches the decoder spec, where a value unset in the configuration is always Null. --- configs/configschema/coerce_value.go | 4 +--- configs/configschema/coerce_value_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/configs/configschema/coerce_value.go b/configs/configschema/coerce_value.go index 1f3793ad11..9c54f158aa 100644 --- a/configs/configschema/coerce_value.go +++ b/configs/configschema/coerce_value.go @@ -58,9 +58,7 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) { switch { case ty.HasAttribute(name): val = in.GetAttr(name) - case attrS.Computed: - val = cty.UnknownVal(attrS.Type) - case attrS.Optional: + case attrS.Computed || attrS.Optional: val = cty.NullVal(attrS.Type) default: return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("attribute %q is required", name) diff --git a/configs/configschema/coerce_value_test.go b/configs/configschema/coerce_value_test.go index d9b7a17f4a..f0e3552d97 100644 --- a/configs/configschema/coerce_value_test.go +++ b/configs/configschema/coerce_value_test.go @@ -342,6 +342,22 @@ func TestCoerceValue(t *testing.T) { cty.DynamicVal, `.foo: number required`, }, + "unset computed value": { + &Block{ + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.NullVal(cty.String), + }), + ``, + }, } for name, test := range tests {