From c85ae29419b72496056f9442afb3bd8e69981b0f Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 12 Aug 2022 10:26:36 -0400 Subject: [PATCH 1/2] typeexpr: Replace null attr values with defaults Previously, when applying defaults to an input variable's given value before type conversion, we would permit `null` attribute values to override a specified default. This behaviour is inconsistent with the intent of the type system underlying Terraform, and represented a divergence from the treatment of `null` as equivalent to unset which exists in resources. The same behaviour exists in top-level variable definitions with `nullable = false`, and we consider this to be the preferred behaviour here too. This commit slightly changes default value application such that an explicit `null` attribute value is treated as equivalent to the attribute being missing. Default values for attributes will now replace explicit nulls. --- internal/typeexpr/defaults.go | 2 +- internal/typeexpr/defaults_test.go | 79 +++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/internal/typeexpr/defaults.go b/internal/typeexpr/defaults.go index fe1d5776b2..851c72fbfc 100644 --- a/internal/typeexpr/defaults.go +++ b/internal/typeexpr/defaults.go @@ -91,7 +91,7 @@ func (t *defaultsTransformer) Enter(p cty.Path, v cty.Value) (cty.Value, error) // Apply defaults where attributes are missing, constructing a new // value with the same marks. for attr, defaultValue := range defaults { - if _, ok := attrs[attr]; !ok { + if attrValue, ok := attrs[attr]; !ok || attrValue.IsNull() { attrs[attr] = defaultValue } } diff --git a/internal/typeexpr/defaults_test.go b/internal/typeexpr/defaults_test.go index b37e92c515..a4da6bb6b2 100644 --- a/internal/typeexpr/defaults_test.go +++ b/internal/typeexpr/defaults_test.go @@ -88,6 +88,23 @@ func TestDefaults_Apply(t *testing.T) { "b": cty.StringVal("false"), }), }, + // Defaults will replace explicit nulls. + "object with explicit null for attribute with default": { + defaults: &Defaults{ + Type: simpleObject, + DefaultValues: map[string]cty.Value{ + "b": cty.True, + }, + }, + value: cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("foo"), + "b": cty.NullVal(cty.String), + }), + want: cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("foo"), + "b": cty.True, + }), + }, // Defaults can be specified at any level of depth and will be applied // so long as there is a parent value to populate. "nested object with defaults applied": { @@ -371,7 +388,7 @@ func TestDefaults_Apply(t *testing.T) { // the child type. "c": cty.ObjectVal(map[string]cty.Value{ "a": cty.StringVal("fallback"), - "b": cty.NullVal(cty.Bool), + "b": cty.False, }), }, Children: map[string]*Defaults{ @@ -410,7 +427,65 @@ func TestDefaults_Apply(t *testing.T) { // default value for "c" includes a non-default value // already. "a": cty.StringVal("fallback"), - "b": cty.NullVal(cty.Bool), + "b": cty.False, + }), + "d": cty.NumberIntVal(7), + }), + }), + }, + "set of nested objects, nulls in default sub-object overridden": { + defaults: &Defaults{ + Type: cty.Set(nestedObject), + Children: map[string]*Defaults{ + "": { + Type: nestedObject, + DefaultValues: map[string]cty.Value{ + // The default value for "c" is used to prepopulate + // the nested object's value if not specified, but + // the null default for its "b" attribute will be + // overridden by the default specified in the child + // type. + "c": cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("fallback"), + "b": cty.NullVal(cty.Bool), + }), + }, + Children: map[string]*Defaults{ + "c": { + Type: simpleObject, + DefaultValues: map[string]cty.Value{ + "b": cty.True, + }, + }, + }, + }, + }, + }, + value: cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "c": cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("foo"), + }), + "d": cty.NumberIntVal(5), + }), + cty.ObjectVal(map[string]cty.Value{ + "d": cty.NumberIntVal(7), + }), + }), + want: cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "c": cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("foo"), + "b": cty.True, + }), + "d": cty.NumberIntVal(5), + }), + cty.ObjectVal(map[string]cty.Value{ + "c": cty.ObjectVal(map[string]cty.Value{ + // The default value for "b" overrides the explicit + // null in the default value for "c". + "a": cty.StringVal("fallback"), + "b": cty.True, }), "d": cty.NumberIntVal(7), }), From 22633a280d5278fad43e0353eb0e68dd7a6c2a6a Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 15 Aug 2022 14:33:15 -0700 Subject: [PATCH 2/2] website: Optional object attributes handling of null Previously we didn't describe the interaction between default values and callers explicitly passing "null". We treat an explicit null as the same as omitting the attribute when applying defaults, because that then allows callers to use the typical pattern for conditional assignment, using explicit null as a fallback to the module's defined default without having to duplicate that default: example = var.foo ? "hello" : null --- .../language/expressions/type-constraints.mdx | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/website/docs/language/expressions/type-constraints.mdx b/website/docs/language/expressions/type-constraints.mdx index d8b7627210..2f5d5b495f 100644 --- a/website/docs/language/expressions/type-constraints.mdx +++ b/website/docs/language/expressions/type-constraints.mdx @@ -260,7 +260,7 @@ value and thus perform no type conversion whatsoever. ## Optional Object Type Attributes --> **Note:** Optional type attributes are supported in Terraform v1.3 and later. +-> **Note:** Optional object type attributes are supported only in Terraform v1.3 and later. Terraform typically returns an error when it does not receive a value for specified object attributes. When you mark an attribute as optional, Terraform instead inserts a default value for the missing attribute. This allows the receiving module to describe an appropriate fallback behavior. @@ -281,6 +281,8 @@ The `optional` modifier takes one or two arguments. specifies the type of the attribute. - **Default:** (Optional) The second argument defines the default value that Terraform should use if the attribute is not present. This must be compatible with the attribute type. If not specified, Terraform uses a `null` value of the appropriate type as the default. +An optional attribute with a non-`null` default value is guaranteed to never have the value `null` within the receiving module. Terraform will substitute the default value both when a caller omits the attribute altogether and when a caller explicitly sets it to `null`, thereby avoiding the need for additional checks to handle a possible null value. + Terraform applies object attribute defaults top-down in nested variable types. This means that Terraform applies the default value you specify in the `optional` modifier first and then later applies any nested default values to that attribute. ### Example: Nested Structures with Optional Attributes and Defaults @@ -383,3 +385,33 @@ tolist([ }, ]) ``` + +### Example: Conditionally setting an optional attribute + +Sometimes the decision about whether or not to set a value for an optional argument needs to be made dynamically based on some other data. In that case, the calling `module` block can use a conditional expression with `null` as one of its result arms to represent dynamically leaving the argument unset. + +With the `variable "buckets"` declaration shown in the previous section, the following example conditionally overrides the `index_document` and `error_document` settings in the `website` object based on a new variable `var.legacy_filenames`: + +```hcl +variable "legacy_filenames" { + type = bool + default = false + nullable = false +} + +module "buckets" { + source = "./modules/buckets" + + buckets = [ + { + name = "maybe_legacy" + website = { + error_document = var.legacy_filenames ? "ERROR.HTM" : null + index_document = var.legacy_filenames ? "INDEX.HTM" : null + } + }, + ] +} +``` + +When `var.legacy_filenames` is set to `true`, the call will override the document filenames. When it is `false`, the call will leave the two filenames unspecified, thereby allowing the module to use its specified default values.