From 931406348fc3b51580604a4c18dabc971c7c77fb Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 10 Jan 2022 16:43:22 -0500 Subject: [PATCH] Handle null variable input with nullable=false Default values are currently being handled in two places, the graph transformation where a synthetic default expression is created if there was no input expression, and in the evaluator when a reference to the variable is evaluated. Unfortunately neither of these covers the case where a non-nullable variable default needs to be checked within a validation statement Rather than try and fix the overall variable handling here, which runs the risk of encountering more unexpected behavior, we are going to fixup this one case to ensure a null value doesn't continue to slip into validation. A more extensive refactoring of variable handling has been completed in v1.2, and this code will only be relevant for the v1.1 branch. --- internal/terraform/context_validate_test.go | 33 +++++++++++++++++++++ internal/terraform/node_module_variable.go | 23 ++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/internal/terraform/context_validate_test.go b/internal/terraform/context_validate_test.go index 1ed5ad4253f2..e582a3d4f562 100644 --- a/internal/terraform/context_validate_test.go +++ b/internal/terraform/context_validate_test.go @@ -2088,3 +2088,36 @@ output "out" { t.Fatal(diags.ErrWithWarnings()) } } + +func TestContext2Validate_nonNullableVariableDefaultValidation(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` + module "first" { + source = "./mod" + input = null + } + `, + + "mod/main.tf": ` + variable "input" { + type = string + default = "default" + nullable = false + + // Validation expressions should receive the default with nullable=false and + // a null input. + validation { + condition = var.input != null + error_message = "Input cannot be null!" + } + } + `, + }) + + ctx := testContext2(t, &ContextOpts{}) + + diags := ctx.Validate(m) + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } +} diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 9f15587eca1b..211ae183e439 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -233,6 +233,29 @@ func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnl scope := ctx.EvaluationScope(nil, moduleInstanceRepetitionData) val, diags := scope.EvalExpr(expr, cty.DynamicPseudoType) + // FIXME: This check is only necessary for v1.1, and is will never be + // present in the v1.2 branch. + // + // Default values are currently being handled in two places, the graph + // transformation where a synthetic default expression is created if there + // was no input expression, and in the evaluator when a reference to the + // variable is evaluated. Unfortunately neither of these covers the case + // where a non-nullable variable default needs to be checked within a + // validation statement + // + // Rather than try and fix the overall variable handling here, which runs + // the risk of encountering more unexpected behavior, we are going to fixup + // this one case to ensure a null value doesn't continue to slip into + // validation. A more extensive refactoring of variable handling has been + // completed in v1.2, and this code will only be relevant for the v1.1 + // branch. + if !diags.HasErrors() && val.IsNull() && + !n.Config.Nullable && + n.Config.Default != cty.NilVal && !n.Config.Default.IsNull() { + // replace the evaluated value with the actual default + val = n.Config.Default + } + // We intentionally passed DynamicPseudoType to EvalExpr above because // now we can do our own local type conversion and produce an error message // with better context if it fails.