Skip to content

Commit

Permalink
Merge branch 'hashicorp:main' into implement/gohcl-eval_context
Browse files Browse the repository at this point in the history
  • Loading branch information
@incubator4
incubator4 committed Nov 8, 2022
2 parents e2ee2e1 + fa90633 commit 78e36b0
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## v2.15.0 (Unreleased)

### Bugs Fixed

* ext/typeexpr: Skip null objects when applying defaults. This prevents crashes when null objects are creating inside collections, and stops incomplete objects being created with only optional attributes set. ([#567](https://github.com/hashicorp/hcl/pull/567))

### Enhancements

* ext/typeexpr: With the [go-cty](https://github.com/zclconf/go-cty) upstream depenendency updated to v1.12.0, the `Defaults` struct and associated functions can apply additional and more flexible 'unsafe' conversions (examples include tuples into collections such as lists and sets, and additional safety around null and dynamic values). ([#564](https://github.com/hashicorp/hcl/pull/564))
Expand Down
9 changes: 7 additions & 2 deletions ext/typeexpr/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,13 @@ type defaultsTransformer struct {
var _ cty.Transformer = (*defaultsTransformer)(nil)

func (t *defaultsTransformer) Enter(p cty.Path, v cty.Value) (cty.Value, error) {
// Cannot apply defaults to an unknown value
if !v.IsKnown() {
// Cannot apply defaults to an unknown value, and should not apply defaults
// to a null value.
//
// A quick clarification, we should still override null values *inside* the
// object or map with defaults. But if the actual object or map itself is
// null then we skip it.
if !v.IsKnown() || v.IsNull() {
return v, nil
}

Expand Down
158 changes: 158 additions & 0 deletions ext/typeexpr/defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,164 @@ func TestDefaults_Apply(t *testing.T) {
}),
}),
},
"null objects do not get default values inserted": {
defaults: &Defaults{
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"required": cty.String,
"optional": cty.String,
}, []string{"optional"}),
DefaultValues: map[string]cty.Value{
"optional": cty.StringVal("optional"),
},
},
value: cty.NullVal(cty.Object(map[string]cty.Type{
"required": cty.String,
"optional": cty.String,
})),
want: cty.NullVal(cty.Object(map[string]cty.Type{
"required": cty.String,
"optional": cty.String,
})),
},
"defaults with unset defaults are still applied (null)": {
defaults: &Defaults{
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"required": cty.String,
"optional_object": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"nested_required": cty.String,
"nested_optional": cty.String,
}, []string{"nested_optional"}),
}, []string{"optional_object"}),
DefaultValues: map[string]cty.Value{
"optional_object": cty.ObjectVal(map[string]cty.Value{
"nested_required": cty.StringVal("required"),
"nested_optional": cty.NullVal(cty.String),
}),
},
Children: map[string]*Defaults{
"optional_object": {
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"nested_required": cty.String,
"nested_optional": cty.String,
}, []string{"nested_optional"}),
DefaultValues: map[string]cty.Value{
"nested_optional": cty.StringVal("optional"),
},
},
},
},
value: cty.ObjectVal(map[string]cty.Value{
"required": cty.StringVal("required"),
// optional_object is explicitly set to null for this test case.
"optional_object": cty.NullVal(cty.Object(map[string]cty.Type{
"nested_required": cty.String,
"nested_optional": cty.String,
})),
}),
want: cty.ObjectVal(map[string]cty.Value{
"required": cty.StringVal("required"),
"optional_object": cty.ObjectVal(map[string]cty.Value{
"nested_required": cty.StringVal("required"),
"nested_optional": cty.StringVal("optional"),
}),
}),
},
"defaults with unset defaults are still applied (missing)": {
defaults: &Defaults{
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"required": cty.String,
"optional_object": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"nested_required": cty.String,
"nested_optional": cty.String,
}, []string{"nested_optional"}),
}, []string{"optional_object"}),
DefaultValues: map[string]cty.Value{
"optional_object": cty.ObjectVal(map[string]cty.Value{
"nested_required": cty.StringVal("required"),
"nested_optional": cty.NullVal(cty.String),
}),
},
Children: map[string]*Defaults{
"optional_object": {
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"nested_required": cty.String,
"nested_optional": cty.String,
}, []string{"nested_optional"}),
DefaultValues: map[string]cty.Value{
"nested_optional": cty.StringVal("optional"),
},
},
},
},
value: cty.ObjectVal(map[string]cty.Value{
"required": cty.StringVal("required"),
// optional_object is missing but not null for this test case.
}),
want: cty.ObjectVal(map[string]cty.Value{
"required": cty.StringVal("required"),
"optional_object": cty.ObjectVal(map[string]cty.Value{
"nested_required": cty.StringVal("required"),
"nested_optional": cty.StringVal("optional"),
}),
}),
},
// https://discuss.hashicorp.com/t/request-for-feedback-optional-object-type-attributes-with-defaults-in-v1-3-alpha/40550/6?u=alisdair
"all child and nested values are optional with defaults": {
defaults: &Defaults{
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"settings": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"setting_one": cty.String,
"setting_two": cty.Number,
}, []string{"setting_one", "setting_two"}),
}, []string{"settings"}),
DefaultValues: map[string]cty.Value{
"settings": cty.EmptyObjectVal,
},
Children: map[string]*Defaults{
"settings": {
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"setting_one": cty.String,
"setting_two": cty.String,
}, []string{"setting_one", "setting_two"}),
DefaultValues: map[string]cty.Value{
"setting_one": cty.StringVal(""),
"setting_two": cty.NumberIntVal(0),
},
},
},
},
value: cty.EmptyObjectVal,
want: cty.ObjectVal(map[string]cty.Value{
"settings": cty.ObjectVal(map[string]cty.Value{
"setting_one": cty.StringVal(""),
"setting_two": cty.NumberIntVal(0),
}),
}),
},
"all nested values are optional with defaults, but direct child has no default": {
defaults: &Defaults{
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"settings": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"setting_one": cty.String,
"setting_two": cty.Number,
}, []string{"setting_one", "setting_two"}),
}, []string{"settings"}),
Children: map[string]*Defaults{
"settings": {
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"setting_one": cty.String,
"setting_two": cty.String,
}, []string{"setting_one", "setting_two"}),
DefaultValues: map[string]cty.Value{
"setting_one": cty.StringVal(""),
"setting_two": cty.NumberIntVal(0),
},
},
},
},
value: cty.EmptyObjectVal,
want: cty.EmptyObjectVal,
},
}

for name, tc := range testCases {
Expand Down

0 comments on commit 78e36b0

Please sign in to comment.