configs: explicitly nullable variable values

The current behavior of module input variables is to allow users to
override a default by assigning `null`, which works contrary to the
behavior of resource attributes, and prevents explicitly accepting a
default when the input must be defined in the configuration.

Add a new variable attribute called `nullable` will allow explicitly
defining when a variable can be set to null or not. The current default
behavior is that of `nullable=true`.

Setting `nullable=false` in a variable block indicates that the variable
value can never be null. This either requires a non-null input value, or
a non-null default value. In the case of the latter, we also opt-in to
the new behavior of a `null` input value taking the default rather than
overriding it.

In a future language edition where we make `nullable=false` the default,
setting `nullable=true` will allow the legacy behavior of `null`
overriding a default value. The only future configuration in which this
would be required even if the legacy behavior were not desired is when
setting an optional+nullable value. In that case `default=null` would
also be needed and we could therefor imply `nullable=true` without
requiring it in the configuration.

Author: jbardin

internal/configs/module_merge_test.go

@@ -24,6 +24,7 @@ func TestModuleOverrideVariable(t *testing.T) {
 			Description:    "b_override description",
 			DescriptionSet: true,
 			Default:        cty.StringVal("b_override"),
+			Nullable:       true,
 			Type:           cty.String,
 			ConstraintType: cty.String,
 			ParsingMode:    VariableParseLiteral,
@@ -46,6 +47,7 @@ func TestModuleOverrideVariable(t *testing.T) {
 			Description:    "base description",
 			DescriptionSet: true,
 			Default:        cty.StringVal("b_override partial"),
+			Nullable:       true,
 			Type:           cty.String,
 			ConstraintType: cty.String,
 			ParsingMode:    VariableParseLiteral,

internal/configs/named_values.go

@@ -36,6 +36,11 @@ type Variable struct {
 	DescriptionSet bool
 	SensitiveSet   bool
 
+	// Nullable indicates that null is a valid value for this variable. Setting
+	// Nullable to false means that the module can expect this variable to
+	// never be null.
+	Nullable bool
+
 	DeclRange hcl.Range
 }
 
@@ -110,6 +115,15 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
 		v.SensitiveSet = true
 	}
 
+	if attr, exists := content.Attributes["nullable"]; exists {
+		valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Nullable)
+		diags = append(diags, valDiags...)
+	} else {
+		// The current default is true, which is subject to change in a future
+		// language edition.
+		v.Nullable = true
+	}
+
 	if attr, exists := content.Attributes["default"]; exists {
 		val, valDiags := attr.Expr.Value(nil)
 		diags = append(diags, valDiags...)
@@ -134,6 +148,15 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
 			}
 		}
 
+		if !v.Nullable && val.IsNull() {
+			diags = append(diags, &hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Invalid default value for variable",
+				Detail:   "A null default value is not valid when nullable=false.",
+				Subject:  attr.Expr.Range().Ptr(),
+			})
+		}
+
 		v.Default = val
 	}
 
@@ -556,6 +579,9 @@ var variableBlockSchema = &hcl.BodySchema{
 		{
 			Name: "sensitive",
 		},
+		{
+			Name: "nullable",
+		},
 	},
 	Blocks: []hcl.BlockHeaderSchema{
 		{

internal/configs/testdata/invalid-modules/nullable-with-default-null/main.tf

@@ -0,0 +1,5 @@
+variable "in" {
+  type = number
+  nullable = false
+  default = null
+}

internal/configs/testdata/valid-files/variables.tf

@@ -30,3 +30,15 @@ variable "sensitive_value" {
   }
   sensitive = true
 }
+
+variable "nullable" {
+  type = string
+  nullable = true
+  default = "ok"
+}
+
+variable "nullable_default_null" {
+  type = map(string)
+  nullable = true
+  default = null
+}

internal/terraform/context_apply2_test.go

@@ -596,3 +596,36 @@ resource "test_object" "x" {
 	}
 
 }
+
+func TestContext2Apply_nullableVariables(t *testing.T) {
+	m := testModule(t, "apply-nullable-variables")
+	state := states.NewState()
+	ctx := testContext2(t, &ContextOpts{})
+	plan, diags := ctx.Plan(m, state, &PlanOpts{})
+	if diags.HasErrors() {
+		t.Fatalf("plan: %s", diags.Err())
+	}
+	state, diags = ctx.Apply(plan, m)
+	if diags.HasErrors() {
+		t.Fatalf("apply: %s", diags.Err())
+	}
+
+	outputs := state.Module(addrs.RootModuleInstance).OutputValues
+	// we check for null outputs be seeing that they don't exists
+	if _, ok := outputs["nullable_null_default"]; ok {
+		t.Error("nullable_null_default: expected no output value")
+	}
+	if _, ok := outputs["nullable_non_null_default"]; ok {
+		t.Error("nullable_non_null_default: expected no output value")
+	}
+	if _, ok := outputs["nullable_no_default"]; ok {
+		t.Error("nullable_no_default: expected no output value")
+	}
+
+	if v := outputs["non_nullable_default"].Value; v.AsString() != "ok" {
+		t.Fatalf("incorrect 'non_nullable_default' output value: %#v\n", v)
+	}
+	if v := outputs["non_nullable_no_default"].Value; v.AsString() != "ok" {
+		t.Fatalf("incorrect 'non_nullable_no_default' output value: %#v\n", v)
+	}
+}

internal/terraform/evaluate.go

@@ -268,11 +268,27 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
 	}
 
 	val, isSet := vals[addr.Name]
-	if !isSet {
-		if config.Default != cty.NilVal {
-			return config.Default, diags
-		}
-		return cty.UnknownVal(config.Type), diags
+	switch {
+	case !isSet:
+		// The config loader will ensure there is a default if the value is not
+		// set at all.
+		val = config.Default
+
+	case val.IsNull() && !config.Nullable && config.Default != cty.NilVal:
+		// If nullable=false a null value will use the configured default.
+		val = config.Default
+
+	case val.IsNull() && !config.Nullable:
+		// The value cannot be null, and there is no configured default.
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  `Invalid variable value`,
+			Detail:   fmt.Sprintf(`The resolved value of variable %q cannot be null.`, addr.Name),
+			Subject:  &config.DeclRange,
+		})
+		// Stub out our return value so that the semantic checker doesn't
+		// produce redundant downstream errors.
+		val = cty.UnknownVal(config.Type)
 	}
 
 	var err error
@@ -286,8 +302,6 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
 			Detail:   fmt.Sprintf(`The resolved value of variable %q is not appropriate: %s.`, addr.Name, err),
 			Subject:  &config.DeclRange,
 		})
-		// Stub out our return value so that the semantic checker doesn't
-		// produce redundant downstream errors.
 		val = cty.UnknownVal(config.Type)
 	}
 

internal/terraform/testdata/apply-nullable-variables/main.tf

@@ -0,0 +1,28 @@
+module "mod" {
+  source = "./mod"
+  nullable_null_default = null
+  nullable_non_null_default = null
+  nullable_no_default = null
+  non_nullable_default = null
+  non_nullable_no_default = "ok"
+}
+
+output "nullable_null_default" {
+  value = module.mod.nullable_null_default
+}
+
+output "nullable_non_null_default" {
+  value = module.mod.nullable_non_null_default
+}
+
+output "nullable_no_default" {
+  value = module.mod.nullable_no_default
+}
+
+output "non_nullable_default" {
+  value = module.mod.non_nullable_default
+}
+
+output "non_nullable_no_default" {
+  value = module.mod.non_nullable_no_default
+}

internal/terraform/testdata/apply-nullable-variables/mod/main.tf

@@ -0,0 +1,59 @@
+// optional, and this can take null as an input
+variable "nullable_null_default" {
+  // This is implied now as the default, and probably should be implied even
+  // when nullable=false is the default, so we're leaving this unset for the test.
+  // nullable = true
+
+  default = null
+}
+
+// assigning null can still override the default.
+variable "nullable_non_null_default" {
+  nullable = true
+  default = "ok"
+}
+
+// required, and assigning null is valid.
+variable "nullable_no_default" {
+  nullable = true
+}
+
+
+// this combination is invalid
+//variable "non_nullable_null_default" {
+//  nullable = false
+//  default = null
+//}
+
+
+// assigning null will take the default
+variable "non_nullable_default" {
+  nullable = false
+  default = "ok"
+}
+
+// required, but null is not a valid value
+variable "non_nullable_no_default" {
+  nullable = false
+}
+
+output "nullable_null_default" {
+  value = var.nullable_null_default
+}
+
+output "nullable_non_null_default" {
+  value = var.nullable_non_null_default
+}
+
+output "nullable_no_default" {
+  value = var.nullable_no_default
+}
+
+output "non_nullable_default" {
+  value = var.non_nullable_default
+}
+
+output "non_nullable_no_default" {
+  value = var.non_nullable_no_default
+}
+