Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically run go-cty.convert after applying defaults #560

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions ext/typeexpr/defaults.go
Expand Up @@ -2,6 +2,7 @@ package typeexpr

import (
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/convert"
)

// Defaults represents a type tree which may contain default values for
Expand Down Expand Up @@ -103,9 +104,57 @@ func (t *defaultsTransformer) Enter(p cty.Path, v cty.Value) (cty.Value, error)
}

func (t *defaultsTransformer) Exit(p cty.Path, v cty.Value) (cty.Value, error) {
v, err := convert.Convert(v, t.defaults.traverseType(p))
if err != nil {
return cty.DynamicVal, err
}
return v, nil
}

func (d *Defaults) traverseType(path cty.Path) cty.Type {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function and its collaborator traverseChildType are so similar to traverse and traverseChild that I think we should consider merging them. Could we extend traverse and traverseChild to return (cty.Value, cty.Type) so that the recursion logic is shared?

if len(path) == 0 {
return d.Type
}

switch s := path[0].(type) {
case cty.GetAttrStep:
if d.Type.IsObjectType() {
// Attribute path steps are normally applied to objects, where each
// attribute may have different defaults.
return d.traverseChildType(s.Name, path)
} else if d.Type.IsMapType() {
// Literal values for maps can result in attribute path steps, in which
// case we need to disregard the attribute name, as maps can have only
// one child.
return d.traverseChildType("", path)
}

return cty.DynamicPseudoType
case cty.IndexStep:
if d.Type.IsTupleType() {
// Tuples can have different types for each element, so we look
// up the defaults based on the index key.
return d.traverseChildType(s.Key.AsBigFloat().String(), path)
} else if d.Type.IsCollectionType() {
// Defaults for collection element types are stored with a blank
// key, so we disregard the index key.
return d.traverseChildType("", path)
}
return cty.DynamicPseudoType
default:
// At time of writing there are no other path step types.
return cty.DynamicPseudoType
}

}

func (d *Defaults) traverseChildType(name string, path cty.Path) cty.Type {
if child, ok := d.Children[name]; ok {
return child.traverseType(path[1:])
}
return cty.DynamicPseudoType
}

// traverse walks the abstract defaults structure for a given path, returning
// a set of default values (if any are present) or nil (if not). This operation
// differs from applying a path to a value because we need to customize the
Expand Down
119 changes: 110 additions & 9 deletions ext/typeexpr/defaults_test.go
Expand Up @@ -68,7 +68,10 @@ func TestDefaults_Apply(t *testing.T) {
},
},
value: cty.UnknownVal(cty.Map(cty.String)),
want: cty.UnknownVal(cty.Map(cty.String)),
want: cty.UnknownVal(cty.Object(map[string]cty.Type{
"a": cty.String,
"b": cty.Bool,
})),
},
// Defaults do not override attributes which are present in the given
// value.
Expand All @@ -85,7 +88,7 @@ func TestDefaults_Apply(t *testing.T) {
}),
want: cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
"b": cty.StringVal("false"),
"b": cty.False,
}),
},
// Defaults will replace explicit nulls.
Expand Down Expand Up @@ -188,7 +191,7 @@ func TestDefaults_Apply(t *testing.T) {
"a": cty.StringVal("bar"),
}),
}),
want: cty.ObjectVal(map[string]cty.Value{
want: cty.MapVal(map[string]cty.Value{
"f": cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
"b": cty.True,
Expand Down Expand Up @@ -221,7 +224,7 @@ func TestDefaults_Apply(t *testing.T) {
"a": cty.StringVal("bar"),
}),
}),
want: cty.TupleVal([]cty.Value{
want: cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
"b": cty.True,
Expand Down Expand Up @@ -307,7 +310,7 @@ func TestDefaults_Apply(t *testing.T) {
"d": cty.NumberIntVal(7),
}),
}),
want: cty.TupleVal([]cty.Value{
want: cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"c": cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
Expand All @@ -317,7 +320,11 @@ func TestDefaults_Apply(t *testing.T) {
}),
cty.ObjectVal(map[string]cty.Value{
// No default value for "c" specified, so none applied. The
// convert stage will fill in a null.
// convert stage will have filled in a null.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little more distinct from the previous logic:

Suggested change
// convert stage will have filled in a null.
// apply process converts to the target type, filling in a null.

"c": cty.NullVal(cty.Object(map[string]cty.Type{
"a": cty.String,
"b": cty.Bool,
})),
"d": cty.NumberIntVal(7),
}),
}),
Expand All @@ -339,6 +346,7 @@ func TestDefaults_Apply(t *testing.T) {
"c": {
Type: simpleObject,
DefaultValues: map[string]cty.Value{
"a": cty.StringVal("bar"),
"b": cty.True,
},
},
Expand All @@ -357,7 +365,7 @@ func TestDefaults_Apply(t *testing.T) {
"d": cty.NumberIntVal(7),
}),
}),
want: cty.TupleVal([]cty.Value{
want: cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"c": cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
Expand All @@ -369,6 +377,7 @@ func TestDefaults_Apply(t *testing.T) {
"c": cty.ObjectVal(map[string]cty.Value{
// Default value for "b" is applied to the empty object
// specified as the default for "c"
"a": cty.StringVal("bar"),
"b": cty.True,
}),
"d": cty.NumberIntVal(7),
Expand Down Expand Up @@ -413,7 +422,7 @@ func TestDefaults_Apply(t *testing.T) {
"d": cty.NumberIntVal(7),
}),
}),
want: cty.TupleVal([]cty.Value{
want: cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"c": cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
Expand All @@ -433,6 +442,98 @@ func TestDefaults_Apply(t *testing.T) {
}),
}),
},
"my_test_case": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A guess at a slightly better test case name:

Suggested change
"my_test_case": {
"nested set of objects with optional attributes, with maps as inputs": {

defaults: &Defaults{
Type: cty.Set(cty.Object(map[string]cty.Type{
"name": cty.String,
"schedules": cty.Set(cty.ObjectWithOptionalAttrs(
map[string]cty.Type{
"name": cty.String,
"cold_storage_after": cty.Number,
}, []string{"cold_storage_after"})),
})),
DefaultValues: nil,
Children: map[string]*Defaults{
"": {
Type: cty.Object(map[string]cty.Type{
"name": cty.String,
"schedules": cty.Set(cty.ObjectWithOptionalAttrs(
map[string]cty.Type{
"name": cty.String,
"cold_storage_after": cty.Number,
}, []string{"cold_storage_after"})),
}),
Children: map[string]*Defaults{
"schedules": {
Type: cty.Set(cty.ObjectWithOptionalAttrs(
map[string]cty.Type{
"name": cty.String,
"cold_storage_after": cty.Number,
}, []string{"cold_storage_after"})),
Children: map[string]*Defaults{
"": {
Type: cty.ObjectWithOptionalAttrs(
map[string]cty.Type{
"name": cty.String,
"cold_storage_after": cty.Number,
}, []string{"cold_storage_after"}),
DefaultValues: map[string]cty.Value{
"cold_storage_after": cty.NumberIntVal(10),
},
},
},
},
},
},
},
},
value: cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("test1"),
"schedules": cty.SetVal([]cty.Value{
cty.MapVal(map[string]cty.Value{
"name": cty.StringVal("daily"),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("test2"),
"schedules": cty.SetVal([]cty.Value{
cty.MapVal(map[string]cty.Value{
"name": cty.StringVal("daily"),
}),
cty.MapVal(map[string]cty.Value{
"name": cty.StringVal("weekly"),
"cold_storage_after": cty.StringVal("0"),
}),
}),
}),
}),
want: cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("test1"),
"schedules": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("daily"),
"cold_storage_after": cty.NumberIntVal(10),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("test2"),
"schedules": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("daily"),
"cold_storage_after": cty.NumberIntVal(10),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("weekly"),
"cold_storage_after": cty.NumberIntVal(0),
}),
}),
}),
}),
},
"set of nested objects, nulls in default sub-object overridden": {
defaults: &Defaults{
Type: cty.Set(nestedObject),
Expand Down Expand Up @@ -472,7 +573,7 @@ func TestDefaults_Apply(t *testing.T) {
"d": cty.NumberIntVal(7),
}),
}),
want: cty.TupleVal([]cty.Value{
want: cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"c": cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("foo"),
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Expand Up @@ -14,7 +14,7 @@ require (
github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7
github.com/sergi/go-diff v1.0.0
github.com/spf13/pflag v1.0.2
github.com/zclconf/go-cty v1.8.0
github.com/zclconf/go-cty v1.11.1
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167
)
Expand All @@ -25,5 +25,5 @@ require (
github.com/stretchr/testify v1.2.2 // indirect
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 // indirect
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
golang.org/x/text v0.3.6 // indirect
golang.org/x/text v0.3.7 // indirect
)
23 changes: 4 additions & 19 deletions go.sum
Expand Up @@ -2,7 +2,6 @@ github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tj
github.com/agext/levenshtein v1.2.1/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558=
github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3 h1:ZSTrOEhiM5J5RFxEaFvMZVEAM1KvT1YzbEOwB2EAGjA=
github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3/go.mod h1:oL81AME2rN47vu18xqj1S1jPIPuN7afo62yKTNn3XMM=
github.com/apparentlymart/go-textseg v1.0.0 h1:rRmlIsPEEhUTIKQb7T++Nz/A5Q6C9IuX2wFoYVvnCs0=
github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/Nj9VFpLOpjS5yuumk=
github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw=
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
Expand All @@ -11,8 +10,6 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68=
github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.4/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw=
github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
Expand All @@ -33,34 +30,22 @@ github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4=
github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI=
github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8=
github.com/zclconf/go-cty v1.8.0 h1:s4AvqaeQzJIu3ndv4gVIhplVD0krU+bgrcLSVUnaWuA=
github.com/zclconf/go-cty v1.8.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk=
github.com/zclconf/go-cty v1.11.1 h1:UMMYDL4riBFaPdzjEWcDdWG7x/Adz8E8f9OX/MGR7V4=
github.com/zclconf/go-cty v1.11.1/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA=
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI=
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 h1:O8uGbHCqlTp2P6QJSLmCojM4mN6UemYv8K+dCnmHmu0=
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 h1:SrN+KX8Art/Sf4HNj6Zcz06G7VEz+7w9tdXTPOZ7+l4=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=