diff --git a/internal/configs/module_merge.go b/internal/configs/module_merge.go index 014d2329c..7dc380114 100644 --- a/internal/configs/module_merge.go +++ b/internal/configs/module_merge.go @@ -56,6 +56,10 @@ func (v *Variable) merge(ov *Variable) hcl.Diagnostics { if ov.ParsingMode != 0 { v.ParsingMode = ov.ParsingMode } + if ov.NullableSet { + v.Nullable = ov.Nullable + v.NullableSet = ov.NullableSet + } // If the override file overrode type without default or vice-versa then // it may have created an invalid situation, which we'll catch now by @@ -100,6 +104,16 @@ func (v *Variable) merge(ov *Variable) hcl.Diagnostics { } else { v.Default = val } + + // ensure a null default wasn't merged in when it is not allowed + if !v.Nullable && v.Default.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: &ov.DeclRange, + }) + } } return diags diff --git a/internal/configs/module_merge_test.go b/internal/configs/module_merge_test.go index 0a57e06bc..d95e62c3c 100644 --- a/internal/configs/module_merge_test.go +++ b/internal/configs/module_merge_test.go @@ -24,6 +24,8 @@ func TestModuleOverrideVariable(t *testing.T) { Description: "b_override description", DescriptionSet: true, Default: cty.StringVal("b_override"), + Nullable: false, + NullableSet: true, Type: cty.String, ConstraintType: cty.String, ParsingMode: VariableParseLiteral, @@ -46,6 +48,8 @@ func TestModuleOverrideVariable(t *testing.T) { Description: "base description", DescriptionSet: true, Default: cty.StringVal("b_override partial"), + Nullable: true, + NullableSet: false, Type: cty.String, ConstraintType: cty.String, ParsingMode: VariableParseLiteral, diff --git a/internal/configs/named_values.go b/internal/configs/named_values.go index 21abd33c2..9dcd10b40 100644 --- a/internal/configs/named_values.go +++ b/internal/configs/named_values.go @@ -36,6 +36,12 @@ 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 + NullableSet bool + DeclRange hcl.Range } @@ -110,6 +116,16 @@ 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...) + v.NullableSet = true + } 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 +150,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 +581,9 @@ var variableBlockSchema = &hcl.BodySchema{ { Name: "sensitive", }, + { + Name: "nullable", + }, }, Blocks: []hcl.BlockHeaderSchema{ { diff --git a/internal/configs/testdata/invalid-modules/nullable-with-default-null/main.tf b/internal/configs/testdata/invalid-modules/nullable-with-default-null/main.tf new file mode 100644 index 000000000..bf7a58a40 --- /dev/null +++ b/internal/configs/testdata/invalid-modules/nullable-with-default-null/main.tf @@ -0,0 +1,5 @@ +variable "in" { + type = number + nullable = false + default = null +} diff --git a/internal/configs/testdata/valid-files/variables.tf b/internal/configs/testdata/valid-files/variables.tf index d641966b5..16640c983 100644 --- a/internal/configs/testdata/valid-files/variables.tf +++ b/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 +} diff --git a/internal/configs/testdata/valid-modules/override-variable/b_override.tf b/internal/configs/testdata/valid-modules/override-variable/b_override.tf index 21dbe82e9..f09ce5380 100644 --- a/internal/configs/testdata/valid-modules/override-variable/b_override.tf +++ b/internal/configs/testdata/valid-modules/override-variable/b_override.tf @@ -1,4 +1,5 @@ variable "fully_overridden" { + nullable = false default = "b_override" description = "b_override description" type = string diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index c405f30aa..8708c7228 100644 --- a/internal/terraform/context_apply2_test.go +++ b/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) + } +} diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 5e746b6d1..71e02d969 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -268,11 +268,15 @@ 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 } var err error @@ -286,8 +290,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) } diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 3487f6d08..9f15587ec 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -253,7 +253,23 @@ func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnl val = cty.UnknownVal(n.Config.Type) } + // If there is no default, we have to ensure that a null value is allowed + // for this variable. + if n.Config.Default == cty.NilVal && !n.Config.Nullable && val.IsNull() { + // 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 variable %q is required, but the given value is null.`, n.Addr), + Subject: &n.Config.DeclRange, + }) + // Stub out our return value so that the semantic checker doesn't + // produce redundant downstream errors. + val = cty.UnknownVal(n.Config.Type) + } + vals := make(map[string]cty.Value) vals[name] = val + return vals, diags.ErrWithWarnings() } diff --git a/internal/terraform/testdata/apply-nullable-variables/main.tf b/internal/terraform/testdata/apply-nullable-variables/main.tf new file mode 100644 index 000000000..ed4b6c7f2 --- /dev/null +++ b/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 +} diff --git a/internal/terraform/testdata/apply-nullable-variables/mod/main.tf b/internal/terraform/testdata/apply-nullable-variables/mod/main.tf new file mode 100644 index 000000000..fcac3ba37 --- /dev/null +++ b/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 +} +