diff --git a/internal/backend/backendrun/unparsed_value.go b/internal/backend/backendrun/unparsed_value.go index bf83dc933e..e372457413 100644 --- a/internal/backend/backendrun/unparsed_value.go +++ b/internal/backend/backendrun/unparsed_value.go @@ -146,7 +146,13 @@ func isDefinedAny(name string, maps ...terraform.InputValues) bool { // InputValues may be incomplete but will include the subset of variables // that were successfully processed, allowing for careful analysis of the // partial result. -func ParseVariableValues(vv map[string]arguments.UnparsedVariableValue, decls map[string]*configs.Variable) (terraform.InputValues, tfdiags.Diagnostics) { +// +// constOnly will only raise a diagnostic error if a required variable is +// missing and is marked as const. Since configuration loading will always +// require values for constant variables, this allows us to use this +// function in both configuration loading and plan/apply contexts where all +// variables are required. +func ParseVariableValues(vv map[string]arguments.UnparsedVariableValue, decls map[string]*configs.Variable, constOnly bool) (terraform.InputValues, tfdiags.Diagnostics) { ret, diags := ParseDeclaredVariableValues(vv, decls) undeclared, diagsUndeclared := ParseUndeclaredVariableValues(vv, decls) @@ -166,62 +172,11 @@ func ParseVariableValues(vv map[string]arguments.UnparsedVariableValue, decls ma // specific error message which mentions -var and -var-file command // line options, whereas the one in Terraform Core is more general // due to supporting both root and child module variables. - if vc.Required() { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "No value for required variable", - Detail: fmt.Sprintf("The root module input variable %q is not set, and has no default value. Use a -var or -var-file command line argument to provide a value for this variable.", name), - Subject: vc.DeclRange.Ptr(), - }) - - // We'll include a placeholder value anyway, just so that our - // result is complete for any calling code that wants to cautiously - // analyze it for diagnostic purposes. Since our diagnostics now - // includes an error, normal processing will ignore this result. - ret[name] = &terraform.InputValue{ - Value: cty.DynamicVal, - SourceType: terraform.ValueFromConfig, - SourceRange: tfdiags.SourceRangeFromHCL(vc.DeclRange), - } - } else { - // We're still required to put an entry for this variable - // in the mapping to be explicit to Terraform Core that we - // visited it, but its value will be cty.NilVal to represent - // that it wasn't set at all at this layer, and so Terraform Core - // should substitute a default if available, or generate an error - // if not. - ret[name] = &terraform.InputValue{ - Value: cty.NilVal, - SourceType: terraform.ValueFromConfig, - SourceRange: tfdiags.SourceRangeFromHCL(vc.DeclRange), - } + shouldError := vc.Required() + if constOnly { + shouldError = vc.Const && vc.Required() } - } - - return ret, diags -} - -func ParseConstVariableValues(vv map[string]arguments.UnparsedVariableValue, decls map[string]*configs.Variable) (terraform.InputValues, tfdiags.Diagnostics) { - ret, diags := ParseDeclaredVariableValues(vv, decls) - undeclared, diagsUndeclared := ParseUndeclaredVariableValues(vv, decls) - - diags = diags.Append(diagsUndeclared) - - // By this point we should've gathered all of the required root module - // variables from one of the many possible sources. We'll now populate - // any we haven't gathered as unset placeholders which Terraform Core - // can then react to. - for name, vc := range decls { - if isDefinedAny(name, ret, undeclared) { - continue - } - - // This check is redundant with a check made in Terraform Core when - // processing undeclared variables, but allows us to generate a more - // specific error message which mentions -var and -var-file command - // line options, whereas the one in Terraform Core is more general - // due to supporting both root and child module variables. - if vc.Const && vc.Required() { + if shouldError { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "No value for required variable", diff --git a/internal/backend/backendrun/unparsed_value_test.go b/internal/backend/backendrun/unparsed_value_test.go index 17822487ce..fd970cf2f4 100644 --- a/internal/backend/backendrun/unparsed_value_test.go +++ b/internal/backend/backendrun/unparsed_value_test.go @@ -162,7 +162,7 @@ func TestUnparsedValue(t *testing.T) { }) t.Run("ParseVariableValues", func(t *testing.T) { - gotVals, diags := ParseVariableValues(vv, decls) + gotVals, diags := ParseVariableValues(vv, decls, false) for _, diag := range diags { t.Logf("%s: %s", diag.Description().Summary, diag.Description().Detail) } @@ -221,6 +221,122 @@ func TestUnparsedValue(t *testing.T) { t.Errorf("wrong result\n%s", diff) } }) + + t.Run("ParseVariableValues constOnly", func(t *testing.T) { + vv := map[string]arguments.UnparsedVariableValue{ + "declared1": testUnparsedVariableValue("5"), + } + + decls := map[string]*configs.Variable{ + "declared1": { + Name: "declared1", + Type: cty.String, + ConstraintType: cty.String, + ParsingMode: configs.VariableParseLiteral, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + }, + }, + "missing_const_required": { + Name: "missing_const_required", + Type: cty.String, + ConstraintType: cty.String, + ParsingMode: configs.VariableParseLiteral, + Const: true, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 3, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 0}, + }, + }, + "missing_nonconst_required": { + Name: "missing_nonconst_required", + Type: cty.String, + ConstraintType: cty.String, + ParsingMode: configs.VariableParseLiteral, + Const: false, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 4, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 4, Column: 1, Byte: 0}, + }, + }, + "missing_const_optional": { + Name: "missing_const_optional", + Type: cty.String, + ConstraintType: cty.String, + ParsingMode: configs.VariableParseLiteral, + Const: true, + Default: cty.StringVal("default"), + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 5, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 5, Column: 1, Byte: 0}, + }, + }, + } + + gotVals, diags := ParseVariableValues(vv, decls, true) + + if got, want := len(diags), 1; got != want { + t.Fatalf("wrong number of diagnostics %d; want %d", got, want) + } + + const missingRequired = `No value for required variable` + + if got, want := diags[0].Description().Summary, missingRequired; got != want { + t.Fatalf("wrong summary for diagnostic 0\ngot: %s\nwant: %s", got, want) + } + + if got, want := diags[0].Description().Detail, `"missing_const_required"`; !strings.Contains(got, want) { + t.Fatalf("wrong detail for diagnostic 0\ngot: %s\nmust contain: %s", got, want) + } + + wantVals := terraform.InputValues{ + "declared1": { + Value: cty.StringVal("5"), + SourceType: terraform.ValueFromNamedFile, + SourceRange: tfdiags.SourceRange{ + Filename: "fake.tfvars", + Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0}, + End: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + "missing_const_required": { + Value: cty.DynamicVal, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRange{ + Filename: "fake.tf", + Start: tfdiags.SourcePos{Line: 3, Column: 1, Byte: 0}, + End: tfdiags.SourcePos{Line: 3, Column: 1, Byte: 0}, + }, + }, + "missing_nonconst_required": { + Value: cty.DynamicVal, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRange{ + Filename: "fake.tf", + Start: tfdiags.SourcePos{Line: 4, Column: 1, Byte: 0}, + End: tfdiags.SourcePos{Line: 4, Column: 1, Byte: 0}, + }, + }, + "missing_const_optional": { + Value: cty.NilVal, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRange{ + Filename: "fake.tf", + Start: tfdiags.SourcePos{Line: 5, Column: 1, Byte: 0}, + End: tfdiags.SourcePos{Line: 5, Column: 1, Byte: 0}, + }, + }, + } + + if diff := cmp.Diff(wantVals, gotVals, cmp.Comparer(cty.Value.RawEquals)); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) } type testUnparsedVariableValue string diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 3c25006939..1311229f04 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -266,7 +266,7 @@ func (b *Local) opApply( // same parsing logic from the plan to generate the diagnostics. undeclaredVariables := map[string]arguments.UnparsedVariableValue{} - parsedVars, _ := backendrun.ParseVariableValues(op.Variables, lr.Config.Module.Variables) + parsedVars, _ := backendrun.ParseVariableValues(op.Variables, lr.Config.Module.Variables, false) for varName := range op.Variables { parsedVar, parsed := parsedVars[varName] diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 768b94355a..97c42fc45d 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -167,7 +167,7 @@ func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRu rawVariables = b.interactiveCollectVariables(context.TODO(), op.Variables, rootMod.Variables, op.UIIn) } - variables, varDiags := backendrun.ParseVariableValues(rawVariables, rootMod.Variables) + variables, varDiags := backendrun.ParseVariableValues(rawVariables, rootMod.Variables, false) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags @@ -271,7 +271,7 @@ func (b *Local) localRunForPlanFile(op *backendrun.Operation, pf *planfile.Reade return nil, nil, diags } - variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) + variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables, false) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags diff --git a/internal/backend/remote/backend_common.go b/internal/backend/remote/backend_common.go index 137c01aa41..f76644ac1f 100644 --- a/internal/backend/remote/backend_common.go +++ b/internal/backend/remote/backend_common.go @@ -259,7 +259,7 @@ func (b *Remote) hasExplicitVariableValues(op *backendrun.Operation) bool { // goal here is just to make a best effort count of how many variable // values are coming from -var or -var-file CLI arguments so that we can // hint the user that those are not supported for remote operations. - variables, _ := backendrun.ParseVariableValues(op.Variables, config.Variables) + variables, _ := backendrun.ParseVariableValues(op.Variables, config.Variables, false) // Check for explicitly-defined (-var and -var-file) variables, which the // remote backend does not support. All other source types are okay, diff --git a/internal/backend/remote/backend_context.go b/internal/backend/remote/backend_context.go index db72d709ed..47d68f2b63 100644 --- a/internal/backend/remote/backend_context.go +++ b/internal/backend/remote/backend_context.go @@ -135,7 +135,7 @@ func (b *Remote) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, state } if op.Variables != nil { - variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) + variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables, false) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags diff --git a/internal/cloud/backend_context.go b/internal/cloud/backend_context.go index e3fc587a33..d4df1b9a7f 100644 --- a/internal/cloud/backend_context.go +++ b/internal/cloud/backend_context.go @@ -135,7 +135,7 @@ func (b *Cloud) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, statem } if op.Variables != nil { - variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) + variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables, false) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index 02f0b8415f..5dda59b46c 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -58,7 +58,7 @@ func (m *Meta) loadConfig(rootDir string) (*configs.Config, tfdiags.Diagnostics) cfg.Root = cfg // Root module is self-referential. return cfg, diags } - vars, parseDiags := backendrun.ParseVariableValues(m.VariableValues, rootMod.Variables) + vars, parseDiags := backendrun.ParseVariableValues(m.VariableValues, rootMod.Variables, true) diags = diags.Append(parseDiags) if parseDiags.HasErrors() { return nil, diags @@ -95,7 +95,7 @@ func (m *Meta) loadConfigWithTests(rootDir, testDir string) (*configs.Config, tf cfg.Root = cfg // Root module is self-referential. return cfg, diags } - vars, parseDiags := backendrun.ParseConstVariableValues(m.VariableValues, rootMod.Variables) + vars, parseDiags := backendrun.ParseVariableValues(m.VariableValues, rootMod.Variables, true) diags = diags.Append(parseDiags) if parseDiags.HasErrors() { return nil, diags @@ -243,7 +243,7 @@ func (m *Meta) installModules(ctx context.Context, rootDir, testsDir string, upg } initializer := func(rootMod *configs.Module, walker configs.ModuleWalker) (*configs.Config, tfdiags.Diagnostics) { - variables, diags := backendrun.ParseConstVariableValues(m.VariableValues, rootMod.Variables) + variables, diags := backendrun.ParseVariableValues(m.VariableValues, rootMod.Variables, true) ctx, ctxDiags := terraform.NewContext(&terraform.ContextOpts{ Parallelism: 1, }) diff --git a/internal/command/show.go b/internal/command/show.go index eb7aed4422..f4055eaacb 100644 --- a/internal/command/show.go +++ b/internal/command/show.go @@ -354,7 +354,7 @@ func readConfig(r *planfile.Reader, allowLanguageExperiments bool, variableValue return nil, diags } - variables, varDiags := backendrun.ParseVariableValues(variableValues, rootMod.Variables) + variables, varDiags := backendrun.ParseVariableValues(variableValues, rootMod.Variables, true) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, diags