diff --git a/internal/backend/unparsed_value.go b/internal/backend/unparsed_value.go index 91c982582..e7eadea9a 100644 --- a/internal/backend/unparsed_value.go +++ b/internal/backend/unparsed_value.go @@ -164,13 +164,18 @@ func ParseVariableValues(vv map[string]UnparsedVariableValue, decls map[string]* // 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 their defaults and fail if any of the - // missing ones are required. + // 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.Required() { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -189,8 +194,14 @@ func ParseVariableValues(vv map[string]UnparsedVariableValue, decls map[string]* 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: vc.Default, + Value: cty.NilVal, SourceType: terraform.ValueFromConfig, SourceRange: tfdiags.SourceRangeFromHCL(vc.DeclRange), } diff --git a/internal/backend/unparsed_value_test.go b/internal/backend/unparsed_value_test.go index 981c84a43..8807d243d 100644 --- a/internal/backend/unparsed_value_test.go +++ b/internal/backend/unparsed_value_test.go @@ -204,7 +204,7 @@ func TestUnparsedValue(t *testing.T) { }, }, "missing2": { - Value: cty.StringVal("default for missing2"), + Value: cty.NilVal, // Terraform Core handles substituting the default SourceType: terraform.ValueFromConfig, SourceRange: tfdiags.SourceRange{ Filename: "fake.tf", diff --git a/internal/command/jsonplan/plan.go b/internal/command/jsonplan/plan.go index 64d77c05a..06ed97961 100644 --- a/internal/command/jsonplan/plan.go +++ b/internal/command/jsonplan/plan.go @@ -118,7 +118,7 @@ func Marshal( output := newPlan() output.TerraformVersion = version.String() - err := output.marshalPlanVariables(p.VariableValues, schemas) + err := output.marshalPlanVariables(p.VariableValues, config.Module.Variables) if err != nil { return nil, fmt.Errorf("error in marshalPlanVariables: %s", err) } @@ -183,11 +183,7 @@ func Marshal( return ret, err } -func (p *plan) marshalPlanVariables(vars map[string]plans.DynamicValue, schemas *terraform.Schemas) error { - if len(vars) == 0 { - return nil - } - +func (p *plan) marshalPlanVariables(vars map[string]plans.DynamicValue, decls map[string]*configs.Variable) error { p.Variables = make(variables, len(vars)) for k, v := range vars { @@ -203,6 +199,41 @@ func (p *plan) marshalPlanVariables(vars map[string]plans.DynamicValue, schemas Value: valJSON, } } + + // In Terraform v1.1 and earlier we had some confusion about which subsystem + // of Terraform was the one responsible for substituting in default values + // for unset module variables, with root module variables being handled in + // three different places while child module variables were only handled + // during the Terraform Core graph walk. + // + // For Terraform v1.2 and later we rationalized that by having the Terraform + // Core graph walk always be responsible for selecting defaults regardless + // of root vs. child module, but unfortunately our earlier accidental + // misbehavior bled out into the public interface by making the defaults + // show up in the "vars" map to this function. Those are now correctly + // omitted (so that the plan file only records the variables _actually_ + // set by the caller) but consumers of the JSON plan format may be depending + // on our old behavior and so we'll fake it here just in time so that + // outside consumers won't see a behavior change. + for name, decl := range decls { + if _, ok := p.Variables[name]; ok { + continue + } + if val := decl.Default; val != cty.NilVal { + valJSON, err := ctyjson.Marshal(val, val.Type()) + if err != nil { + return err + } + p.Variables[name] = &variable{ + Value: valJSON, + } + } + } + + if len(p.Variables) == 0 { + p.Variables = nil // omit this property if there are no variables to describe + } + return nil } diff --git a/internal/terraform/context_apply.go b/internal/terraform/context_apply.go index 42520b03d..d3cd9dc2d 100644 --- a/internal/terraform/context_apply.go +++ b/internal/terraform/context_apply.go @@ -87,6 +87,21 @@ func (c *Context) applyGraph(plan *plans.Plan, config *configs.Config, validate return nil, walkApply, diags } + // The plan.VariableValues field only records variables that were actually + // set by the caller in the PlanOpts, so we may need to provide + // placeholders for any other variables that the user didn't set, in + // which case Terraform will once again use the default value from the + // configuration when we visit these variables during the graph walk. + for name := range config.Module.Variables { + if _, ok := variables[name]; ok { + continue + } + variables[name] = &InputValue{ + Value: cty.NilVal, + SourceType: ValueFromPlan, + } + } + graph, moreDiags := (&ApplyGraphBuilder{ Config: config, Changes: plan.Changes, diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index 6b87d409d..512f3dab3 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -426,7 +426,7 @@ resource "test_resource" "b" { }, }) - plan, diags := ctx.Plan(m, state, DefaultPlanOpts) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) _, diags = ctx.Apply(plan, m) @@ -530,14 +530,14 @@ resource "test_object" "y" { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) assertNoErrors(t, diags) // FINAL PLAN: - plan, diags = ctx.Plan(m, state, DefaultPlanOpts) + plan, diags = ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) // make sure the same marks are compared in the next plan as well diff --git a/internal/terraform/context_apply_test.go b/internal/terraform/context_apply_test.go index 28c5c788b..46dcbd58b 100644 --- a/internal/terraform/context_apply_test.go +++ b/internal/terraform/context_apply_test.go @@ -517,7 +517,7 @@ func TestContext2Apply_mapVarBetweenModules(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) @@ -2262,7 +2262,7 @@ func TestContext2Apply_countVariable(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) @@ -2288,7 +2288,7 @@ func TestContext2Apply_countVariableRef(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) @@ -2327,7 +2327,7 @@ func TestContext2Apply_provisionerInterpCount(t *testing.T) { Provisioners: provisioners, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) // We'll marshal and unmarshal the plan here, to ensure that we have @@ -3682,7 +3682,7 @@ func TestContext2Apply_multiVarOrder(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) @@ -3713,7 +3713,7 @@ func TestContext2Apply_multiVarOrderInterp(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) @@ -4704,9 +4704,7 @@ func TestContext2Apply_provisionerDestroy(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, state, &PlanOpts{ - Mode: plans.DestroyMode, - }) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.DestroyMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags = ctx.Apply(plan, m) @@ -4753,9 +4751,7 @@ func TestContext2Apply_provisionerDestroyFail(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, state, &PlanOpts{ - Mode: plans.DestroyMode, - }) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.DestroyMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags = ctx.Apply(plan, m) @@ -5908,7 +5904,7 @@ func TestContext2Apply_destroyWithModuleVariableAndCountNested(t *testing.T) { }) // First plan and apply a create operation - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags = ctx.Apply(plan, m) @@ -5929,9 +5925,7 @@ func TestContext2Apply_destroyWithModuleVariableAndCountNested(t *testing.T) { }) // First plan and apply a create operation - plan, diags := ctx.Plan(m, state, &PlanOpts{ - Mode: plans.DestroyMode, - }) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.DestroyMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("destroy plan err: %s", diags.Err()) } @@ -7561,6 +7555,12 @@ func TestContext2Apply_vars(t *testing.T) { Value: cty.StringVal("us-east-1"), SourceType: ValueFromCaller, }, + "bar": &InputValue{ + // This one is not explicitly set but that's okay because it + // has a declared default, which Terraform Core will use instead. + Value: cty.NilVal, + SourceType: ValueFromCaller, + }, "test_list": &InputValue{ Value: cty.ListVal([]cty.Value{ cty.StringVal("Hello"), @@ -7876,7 +7876,7 @@ func TestContext2Apply_issue7824(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("err: %s", diags.Err()) } @@ -7932,7 +7932,7 @@ func TestContext2Apply_issue5254(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("err: %s", diags.Err()) } @@ -7951,7 +7951,7 @@ func TestContext2Apply_issue5254(t *testing.T) { }, }) - plan, diags = ctx.Plan(m, state, DefaultPlanOpts) + plan, diags = ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("err: %s", diags.Err()) } @@ -8845,7 +8845,7 @@ func TestContext2Apply_plannedInterpolatedCount(t *testing.T) { Providers: Providers, }) - plan, diags := ctx.Plan(m, state, DefaultPlanOpts) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("plan failed: %s", diags.Err()) } @@ -8904,9 +8904,7 @@ func TestContext2Apply_plannedDestroyInterpolatedCount(t *testing.T) { Providers: providers, }) - plan, diags := ctx.Plan(m, state, &PlanOpts{ - Mode: plans.DestroyMode, - }) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.DestroyMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("plan failed: %s", diags.Err()) } @@ -9674,7 +9672,7 @@ func TestContext2Apply_plannedConnectionRefs(t *testing.T) { Hooks: []Hook{hook}, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) diags.HasErrors() if diags.HasErrors() { t.Fatalf("diags: %s", diags.Err()) @@ -11687,7 +11685,7 @@ resource "test_resource" "foo" { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags := ctx.Apply(plan, m) @@ -11702,7 +11700,7 @@ resource "test_resource" "foo" { }, }) - plan, diags = ctx.Plan(m, state, DefaultPlanOpts) + plan, diags = ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) state, diags = ctx.Apply(plan, m) @@ -11720,6 +11718,7 @@ resource "test_resource" "foo" { plan, diags = ctx.Plan(m, state, &PlanOpts{ Mode: plans.NormalMode, SetVariables: InputValues{ + "sensitive_id": &InputValue{Value: cty.NilVal}, "sensitive_var": &InputValue{ Value: cty.StringVal("bar"), }, @@ -11759,7 +11758,7 @@ resource "test_resource" "foo" { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("plan errors: %s", diags.Err()) } @@ -11904,7 +11903,7 @@ resource "test_resource" "foo" { ) }) - plan, diags := ctx.Plan(m, state, DefaultPlanOpts) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) addr := mustResourceInstanceAddr("test_resource.foo") @@ -11954,7 +11953,7 @@ resource "test_resource" "foo" { // but this seems rather suspicious and we should ideally figure out what // this test was originally intending to do and make it do that. oldPlan := plan - _, diags = ctx2.Plan(m2, state, DefaultPlanOpts) + _, diags = ctx2.Plan(m2, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) stateWithoutSensitive, diags := ctx.Apply(oldPlan, m) assertNoErrors(t, diags) @@ -12206,7 +12205,7 @@ func TestContext2Apply_dataSensitive(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("diags: %s", diags.Err()) } else { diff --git a/internal/terraform/context_eval.go b/internal/terraform/context_eval.go index c6af77635..f9d0f6493 100644 --- a/internal/terraform/context_eval.go +++ b/internal/terraform/context_eval.go @@ -45,7 +45,7 @@ func (c *Context) Eval(config *configs.Config, state *states.State, moduleAddr a state = state.DeepCopy() var walker *ContextGraphWalker - variables := mergeDefaultInputVariableValues(opts.SetVariables, config.Module.Variables) + variables := opts.SetVariables // By the time we get here, we should have values defined for all of // the root module variables, even if some of them are "unknown". It's the diff --git a/internal/terraform/context_eval_test.go b/internal/terraform/context_eval_test.go index dff687983..0bd752935 100644 --- a/internal/terraform/context_eval_test.go +++ b/internal/terraform/context_eval_test.go @@ -54,7 +54,9 @@ func TestContextEval(t *testing.T) { }, }) - scope, diags := ctx.Eval(m, states.NewState(), addrs.RootModuleInstance, &EvalOpts{}) + scope, diags := ctx.Eval(m, states.NewState(), addrs.RootModuleInstance, &EvalOpts{ + SetVariables: testInputValuesUnset(m.Module.Variables), + }) if diags.HasErrors() { t.Fatalf("Eval errors: %s", diags.Err()) } diff --git a/internal/terraform/context_import.go b/internal/terraform/context_import.go index b5417405f..d809d6bb9 100644 --- a/internal/terraform/context_import.go +++ b/internal/terraform/context_import.go @@ -53,7 +53,7 @@ func (c *Context) Import(config *configs.Config, prevRunState *states.State, opt log.Printf("[DEBUG] Building and walking import graph") - variables := mergeDefaultInputVariableValues(opts.SetVariables, config.Module.Variables) + variables := opts.SetVariables // Initialize our graph builder builder := &ImportGraphBuilder{ diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 0b3c97f14..f114b2a5f 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -21,10 +21,42 @@ import ( // PlanOpts are the various options that affect the details of how Terraform // will build a plan. type PlanOpts struct { - Mode plans.Mode - SkipRefresh bool + // Mode defines what variety of plan the caller wishes to create. + // Refer to the documentation of the plans.Mode type and its values + // for more information. + Mode plans.Mode + + // SkipRefresh specifies to trust that the current values for managed + // resource instances in the prior state are accurate and to therefore + // disable the usual step of fetching updated values for each resource + // instance using its corresponding provider. + SkipRefresh bool + + // SetVariables are the raw values for root module variables as provided + // by the user who is requesting the run, prior to any normalization or + // substitution of defaults. See the documentation for the InputValue + // type for more information on how to correctly populate this. SetVariables InputValues - Targets []addrs.Targetable + + // If Targets has a non-zero length then it activates targeted planning + // mode, where Terraform will take actions only for resource instances + // mentioned in this set and any other objects those resource instances + // depend on. + // + // Targeted planning mode is intended for exceptional use only, + // and so populating this field will cause Terraform to generate extra + // warnings as part of the planning result. + Targets []addrs.Targetable + + // ForceReplace is a set of resource instance addresses whose corresponding + // objects should be forced planned for replacement if the provider's + // plan would otherwise have been to either update the object in-place or + // to take no action on it at all. + // + // A typical use of this argument is to ask Terraform to replace an object + // which the user has determined is somehow degraded (via information from + // outside of Terraform), thereby hopefully replacing it with a + // fully-functional new object. ForceReplace []addrs.AbsResourceInstance } @@ -99,8 +131,6 @@ func (c *Context) Plan(config *configs.Config, prevRunState *states.State, opts return nil, diags } - variables := mergeDefaultInputVariableValues(opts.SetVariables, config.Module.Variables) - // By the time we get here, we should have values defined for all of // the root module variables, even if some of them are "unknown". It's the // caller's responsibility to have already handled the decoding of these @@ -108,7 +138,7 @@ func (c *Context) Plan(config *configs.Config, prevRunState *states.State, opts // user-friendly error messages if they are not all present, and so // the error message from checkInputVariables should never be seen and // includes language asking the user to report a bug. - varDiags := checkInputVariables(config.Module.Variables, variables) + varDiags := checkInputVariables(config.Module.Variables, opts.SetVariables) diags = diags.Append(varDiags) if len(opts.Targets) > 0 { @@ -139,8 +169,12 @@ The -target option is not for routine use, and is provided only for exceptional } // convert the variables into the format expected for the plan - varVals := make(map[string]plans.DynamicValue, len(variables)) - for k, iv := range variables { + varVals := make(map[string]plans.DynamicValue, len(opts.SetVariables)) + for k, iv := range opts.SetVariables { + if iv.Value == cty.NilVal { + continue // We only record values that the caller actually set + } + // We use cty.DynamicPseudoType here so that we'll save both the // value _and_ its dynamic type in the plan, so we can recover // exactly the same value later. @@ -172,6 +206,25 @@ var DefaultPlanOpts = &PlanOpts{ Mode: plans.NormalMode, } +// SimplePlanOpts is a constructor to help with creating "simple" values of +// PlanOpts which only specify a mode and input variables. +// +// This helper function is primarily intended for use in straightforward +// tests that don't need any of the more "esoteric" planning options. For +// handling real user requests to run Terraform, it'd probably be better +// to construct a *PlanOpts value directly and provide a way for the user +// to set values for all of its fields. +// +// The "mode" and "setVariables" arguments become the values of the "Mode" +// and "SetVariables" fields in the result. Refer to the PlanOpts type +// documentation to learn about the meanings of those fields. +func SimplePlanOpts(mode plans.Mode, setVariables InputValues) *PlanOpts { + return &PlanOpts{ + Mode: mode, + SetVariables: setVariables, + } +} + func (c *Context) plan(config *configs.Config, prevRunState *states.State, opts *PlanOpts) (*plans.Plan, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index 006e9e932..d1771b1f3 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -205,7 +205,7 @@ data "test_data_source" "foo" { }, }) - plan, diags := ctx.Plan(m, state, DefaultPlanOpts) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) assertNoErrors(t, diags) for _, res := range plan.Changes.Resources { diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index cfd51da8c..9cf7e875e 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -405,7 +405,7 @@ func TestContext2Plan_moduleExpand(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } @@ -1175,7 +1175,7 @@ func TestContext2Plan_moduleProviderVar(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } @@ -2242,7 +2242,7 @@ func TestContext2Plan_countModuleStatic(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } @@ -2295,7 +2295,7 @@ func TestContext2Plan_countModuleStaticGrandchild(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } @@ -3938,7 +3938,7 @@ func TestContext2Plan_taintDestroyInterpolatedCountRace(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, state.DeepCopy(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, state.DeepCopy(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } @@ -5481,7 +5481,7 @@ func TestContext2Plan_variableSensitivity(t *testing.T) { }, }) - plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } @@ -5544,6 +5544,7 @@ func TestContext2Plan_variableSensitivityModule(t *testing.T) { plan, diags := ctx.Plan(m, states.NewState(), &PlanOpts{ Mode: plans.NormalMode, SetVariables: InputValues{ + "sensitive_var": {Value: cty.NilVal}, "another_var": &InputValue{ Value: cty.StringVal("boop"), SourceType: ValueFromCaller, @@ -6657,7 +6658,7 @@ resource "test_resource" "foo" { }, ) }) - plan, diags := ctx.Plan(m, state, DefaultPlanOpts) + plan, diags := ctx.Plan(m, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) if diags.HasErrors() { t.Fatal(diags.Err()) } diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 1c16069ad..bbafbe11c 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -45,9 +45,11 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, given c log.Printf("[TRACE] prepareFinalInputVariableValue: %s has no defined value", addr) if cfg.Required() { // NOTE: The CLI layer typically checks for itself whether all of - // the required _root_ module variables are not set, which would - // mask this error. We can get here for child module variables, - // though. + // the required _root_ module variables are set, which would + // mask this error with a more specific one that refers to the + // CLI features for setting such variables. We can get here for + // child module variables, though. + log.Printf("[ERROR] prepareFinalInputVariableValue: %s is required but is not set", addr) diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Required variable not set`, @@ -64,6 +66,7 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, given c val, err := convert.Convert(given, convertTy) if err != nil { + log.Printf("[ERROR] prepareFinalInputVariableValue: %s has unsuitable type\n got: %s\n want: %s", addr, given.Type(), convertTy) diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid value for module argument", @@ -93,6 +96,7 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, given c if defaultVal != cty.NilVal { val = defaultVal } else { + log.Printf("[ERROR] prepareFinalInputVariableValue: %s is non-nullable but set to null, and is required", addr) diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Required variable not set`, diff --git a/internal/terraform/variables.go b/internal/terraform/variables.go index 7a6ace0ee..f5f03d156 100644 --- a/internal/terraform/variables.go +++ b/internal/terraform/variables.go @@ -3,18 +3,50 @@ package terraform import ( "fmt" - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/tfdiags" ) -// InputValue represents a value for a variable in the root module, provided -// as part of the definition of an operation. +// InputValue represents a raw value vor a root module input variable as +// provided by the external caller into a function like terraform.Context.Plan. +// +// InputValue should represent as directly as possible what the user set the +// variable to, without any attempt to convert the value to the variable's +// type constraint or substitute the configured default values for variables +// that wasn't set. Those adjustments will be handled by Terraform Core itself +// as part of performing the requested operation. +// +// A Terraform Core caller must provide an InputValue object for each of the +// variables declared in the root module, even if the end user didn't provide +// an explicit value for some of them. See the Value field documentation for +// how to handle that situation. type InputValue struct { - Value cty.Value + // Value is the raw value as provided by the user as part of the plan + // options, or a corresponding similar data structure for non-plan + // operations. + // + // If a particular variable declared in the root module is _not_ set by + // the user then the caller must still provide an InputValue for it but + // must set Value to cty.NilVal to represent the absense of a value. + // This requirement is to help detect situations where the caller isn't + // correctly detecting and handling all of the declared variables. + // + // For historical reasons it's important that callers distinguish the + // situation of the value not being set at all (cty.NilVal) from the + // situation of it being explicitly set to null (a cty.NullVal result): + // for "nullable" input variables that distinction unfortunately decides + // whether the final value will be the variable's default or will be + // explicitly null. + Value cty.Value + + // SourceType is a high-level category for where the value of Value + // came from, which Terraform Core uses to tailor some of its error + // messages to be more helpful to the user. + // + // Some SourceType values should be accompanied by a populated SourceRange + // value. See that field's documentation below for more information. SourceType ValueSourceType // SourceRange provides source location information for values whose @@ -129,23 +161,6 @@ func (vv InputValues) JustValues() map[string]cty.Value { return ret } -// DefaultVariableValues returns an InputValues map representing the default -// values specified for variables in the given configuration map. -func DefaultVariableValues(configs map[string]*configs.Variable) InputValues { - ret := make(InputValues) - for k, c := range configs { - if c.Default == cty.NilVal { - continue - } - ret[k] = &InputValue{ - Value: c.Default, - SourceType: ValueFromConfig, - SourceRange: tfdiags.SourceRangeFromHCL(c.DeclRange), - } - } - return ret -} - // SameValues returns true if the given InputValues has the same values as // the receiever, disregarding the source types and source ranges. // @@ -227,21 +242,15 @@ func (vv InputValues) Identical(other InputValues) bool { return true } -func mergeDefaultInputVariableValues(setVals InputValues, rootVarsConfig map[string]*configs.Variable) InputValues { - var variables InputValues - - // Default variables from the configuration seed our map. - variables = DefaultVariableValues(rootVarsConfig) - - // Variables provided by the caller (from CLI, environment, etc) can - // override the defaults. - variables = variables.Override(setVals) - - return variables -} - -// checkInputVariables ensures that variable values supplied at the UI conform -// to their corresponding declarations in configuration. +// checkInputVariables ensures that the caller provided an InputValue +// definition for each root module variable declared in the configuration. +// The caller must provide an InputVariables with keys exactly matching +// the declared variables, though some of them may be marked explicitly +// unset by their values being cty.NilVal. +// +// This doesn't perform any type checking, default value substitution, or +// validation checks. Those are all handled during a graph walk when we +// visit the graph nodes representing each root variable. // // The set of values is considered valid only if the returned diagnostics // does not contain errors. A valid set of values may still produce warnings, @@ -249,11 +258,12 @@ func mergeDefaultInputVariableValues(setVals InputValues, rootVarsConfig map[str func checkInputVariables(vcs map[string]*configs.Variable, vs InputValues) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - for name, vc := range vcs { - val, isSet := vs[name] + for name := range vcs { + _, isSet := vs[name] if !isSet { - // Always an error, since the caller should already have included - // default values from the configuration in the values map. + // Always an error, since the caller should have produced an + // item with Value: cty.NilVal to be explicit that it offered + // an opportunity to set this variable. diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Unassigned variable", @@ -261,49 +271,6 @@ func checkInputVariables(vcs map[string]*configs.Variable, vs InputValues) tfdia )) continue } - - // A given value is valid if it can convert to the desired type. - _, err := convert.Convert(val.Value, vc.ConstraintType) - if err != nil { - switch val.SourceType { - case ValueFromConfig, ValueFromAutoFile, ValueFromNamedFile: - // We have source location information for these. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid value for input variable", - Detail: fmt.Sprintf("The given value is not valid for variable %q: %s.", name, err), - Subject: val.SourceRange.ToHCL().Ptr(), - }) - case ValueFromEnvVar: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid value for input variable", - fmt.Sprintf("The environment variable TF_VAR_%s does not contain a valid value for variable %q: %s.", name, name, err), - )) - case ValueFromCLIArg: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid value for input variable", - fmt.Sprintf("The argument -var=\"%s=...\" does not contain a valid value for variable %q: %s.", name, name, err), - )) - case ValueFromInput: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid value for input variable", - fmt.Sprintf("The value entered for variable %q is not valid: %s.", name, err), - )) - default: - // The above gets us good coverage for the situations users - // are likely to encounter with their own inputs. The other - // cases are generally implementation bugs, so we'll just - // use a generic error for these. - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid value for input variable", - fmt.Sprintf("The value provided for variable %q is not valid: %s.", name, err), - )) - } - } } // Check for any variables that are assigned without being configured. diff --git a/internal/terraform/variables_test.go b/internal/terraform/variables_test.go index 41decbae2..6e53a9575 100644 --- a/internal/terraform/variables_test.go +++ b/internal/terraform/variables_test.go @@ -3,166 +3,10 @@ package terraform import ( "testing" - "github.com/davecgh/go-spew/spew" - "github.com/hashicorp/terraform/internal/tfdiags" - - "github.com/go-test/deep" + "github.com/hashicorp/terraform/internal/configs" "github.com/zclconf/go-cty/cty" ) -func TestVariables(t *testing.T) { - tests := map[string]struct { - Module string - Override map[string]cty.Value - Want InputValues - }{ - "config only": { - "vars-basic", - nil, - InputValues{ - "a": &InputValue{ - Value: cty.StringVal("foo"), - SourceType: ValueFromConfig, - SourceRange: tfdiags.SourceRange{ - Filename: "testdata/vars-basic/main.tf", - Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0}, - End: tfdiags.SourcePos{Line: 1, Column: 13, Byte: 12}, - }, - }, - "b": &InputValue{ - Value: cty.ListValEmpty(cty.String), - SourceType: ValueFromConfig, - SourceRange: tfdiags.SourceRange{ - Filename: "testdata/vars-basic/main.tf", - Start: tfdiags.SourcePos{Line: 6, Column: 1, Byte: 55}, - End: tfdiags.SourcePos{Line: 6, Column: 13, Byte: 67}, - }, - }, - "c": &InputValue{ - Value: cty.MapValEmpty(cty.String), - SourceType: ValueFromConfig, - SourceRange: tfdiags.SourceRange{ - Filename: "testdata/vars-basic/main.tf", - Start: tfdiags.SourcePos{Line: 11, Column: 1, Byte: 113}, - End: tfdiags.SourcePos{Line: 11, Column: 13, Byte: 125}, - }, - }, - }, - }, - - "override": { - "vars-basic", - map[string]cty.Value{ - "a": cty.StringVal("bar"), - "b": cty.ListVal([]cty.Value{ - cty.StringVal("foo"), - cty.StringVal("bar"), - }), - "c": cty.MapVal(map[string]cty.Value{ - "foo": cty.StringVal("bar"), - }), - }, - InputValues{ - "a": &InputValue{ - Value: cty.StringVal("bar"), - SourceType: ValueFromCaller, - }, - "b": &InputValue{ - Value: cty.ListVal([]cty.Value{ - cty.StringVal("foo"), - cty.StringVal("bar"), - }), - SourceType: ValueFromCaller, - }, - "c": &InputValue{ - Value: cty.MapVal(map[string]cty.Value{ - "foo": cty.StringVal("bar"), - }), - SourceType: ValueFromCaller, - }, - }, - }, - - "bools: config only": { - "vars-basic-bool", - nil, - InputValues{ - "a": &InputValue{ - Value: cty.True, - SourceType: ValueFromConfig, - SourceRange: tfdiags.SourceRange{ - Filename: "testdata/vars-basic-bool/main.tf", - Start: tfdiags.SourcePos{Line: 4, Column: 1, Byte: 177}, - End: tfdiags.SourcePos{Line: 4, Column: 13, Byte: 189}, - }, - }, - "b": &InputValue{ - Value: cty.False, - SourceType: ValueFromConfig, - SourceRange: tfdiags.SourceRange{ - Filename: "testdata/vars-basic-bool/main.tf", - Start: tfdiags.SourcePos{Line: 8, Column: 1, Byte: 214}, - End: tfdiags.SourcePos{Line: 8, Column: 13, Byte: 226}, - }, - }, - }, - }, - - "bools: override with string": { - "vars-basic-bool", - map[string]cty.Value{ - "a": cty.StringVal("foo"), - "b": cty.StringVal("bar"), - }, - InputValues{ - "a": &InputValue{ - Value: cty.StringVal("foo"), - SourceType: ValueFromCaller, - }, - "b": &InputValue{ - Value: cty.StringVal("bar"), - SourceType: ValueFromCaller, - }, - }, - }, - - "bools: override with bool": { - "vars-basic-bool", - map[string]cty.Value{ - "a": cty.False, - "b": cty.True, - }, - InputValues{ - "a": &InputValue{ - Value: cty.False, - SourceType: ValueFromCaller, - }, - "b": &InputValue{ - Value: cty.True, - SourceType: ValueFromCaller, - }, - }, - }, - } - - for name, test := range tests { - // Wrapped in a func so we can get defers to work - t.Run(name, func(t *testing.T) { - m := testModule(t, test.Module) - fromConfig := DefaultVariableValues(m.Module.Variables) - overrides := InputValuesFromCaller(test.Override) - got := fromConfig.Override(overrides) - - if !got.Identical(test.Want) { - t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(test.Want)) - } - for _, problem := range deep.Equal(got, test.Want) { - t.Errorf(problem) - } - }) - } -} - func TestCheckInputVariables(t *testing.T) { c := testModule(t, "input-variables") @@ -280,3 +124,25 @@ func TestCheckInputVariables(t *testing.T) { } }) } + +// testInputValuesUnset is a helper for constructing InputValues values for +// situations where all of the root module variables are optional and a +// test case intends to just use those default values and not override them +// at all. +// +// In other words, this constructs an InputValues with one entry per given +// input variable declaration where all of them are declared as unset. +func testInputValuesUnset(decls map[string]*configs.Variable) InputValues { + if len(decls) == 0 { + return nil + } + + ret := make(InputValues, len(decls)) + for name := range decls { + ret[name] = &InputValue{ + Value: cty.NilVal, + SourceType: ValueFromUnknown, + } + } + return ret +}