diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 5c1d4f849bda..142369dd9128 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -252,7 +252,22 @@ func (b *Local) opApply( } if len(op.Variables) != 0 { + // Undeclared variables cause warnings during plan, but will show up + // again here during apply. Their handling is tricky though, because it + // depends on how they were declared, and is subject to compatibility + // constraints. Collect any suspect values as we go, and then use the + // same parsing logic from the plan to generate the diagnostics. + undeclaredVariables := map[string]backendrun.UnparsedVariableValue{} + for varName, rawV := range op.Variables { + decl, ok := lr.Config.Module.Variables[varName] + if !ok { + // We'll try to parse this and handle diagnostics for missing + // variables with ParseUndeclaredVariableValues after. + undeclaredVariables[varName] = rawV + continue + } + // We're "parsing" only to get the resulting value's SourceType, // so we'll use configs.VariableParseLiteral just because it's // the most liberal interpretation and so least likely to @@ -269,25 +284,6 @@ func (b *Local) opApply( rng = v.SourceRange.ToHCL().Ptr() } - decl, ok := lr.Config.Module.Variables[varName] - - // If the variable isn't declared in config at all, take - // this opportunity to give the user a helpful error, - // rather than waiting for a less helpful one later. - // We are ok with over-supplying variables through environment variables - // since it would be a breaking change to disallow it. - if v.SourceType == terraform.ValueFromCLIArg || v.SourceType == terraform.ValueFromNamedFile { - if !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Value for undeclared variable", - Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options because it is not declared in configuration.", varName), - Subject: rng, - }) - continue - } - } - // If the var is declared as ephemeral in config, go ahead and handle it if ok && decl.Ephemeral { // Determine whether this is an apply-time variable, i.e. an @@ -345,12 +341,8 @@ func (b *Local) opApply( // If a non-ephemeral variable is set differently between plan and apply, we should emit a diagnostic. plannedVariableValue, ok := plan.VariableValues[varName] if !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Can't set variable when applying a saved plan", - Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options when applying a saved plan file, because it is neither ephemeral nor has it been declared during the plan operation. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName), - Subject: rng, - }) + // We'll catch this with ParseUndeclaredVariableValues after + undeclaredVariables[varName] = rawV continue } @@ -373,7 +365,15 @@ func (b *Local) opApply( } } } + + } + _, undeclaredDiags := backendrun.ParseUndeclaredVariableValues(undeclaredVariables, map[string]*configs.Variable{}) + // always add hard errors here, and add warnings if we're not in a + // combined op which just emitted those same warnings already. + if undeclaredDiags.HasErrors() || !combinedPlanApply { + diags = diags.Append(undeclaredDiags) } + if diags.HasErrors() { op.ReportResult(runningOp, diags) return diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index bb9db4f3b93a..89449163a722 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -860,7 +860,7 @@ func TestApply_plan_remoteState(t *testing.T) { func TestApply_planWithVarFile(t *testing.T) { varFileDir := testTempDir(t) varFilePath := filepath.Join(varFileDir, "terraform.tfvars") - if err := ioutil.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { + if err := os.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { t.Fatalf("err: %s", err) } @@ -878,6 +878,19 @@ func TestApply_planWithVarFile(t *testing.T) { defer os.Chdir(cwd) p := applyFixtureProvider() + p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "test_instance": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "value": {Type: cty.String, Optional: true}, + }, + }, + }, + }, + } + view, done := testView(t) c := &ApplyCommand{ Meta: Meta{ @@ -906,59 +919,14 @@ func TestApply_planWithVarFile(t *testing.T) { } } -func TestApply_planWithVarFilePreviouslyUnset(t *testing.T) { - varFileDir := testTempDir(t) - varFilePath := filepath.Join(varFileDir, "terraform.tfvars") - if err := ioutil.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { - t.Fatalf("err: %s", err) - } - - // The value of foo is not set - planPath := applyFixturePlanFile(t) - statePath := testTempFile(t) - - cwd, err := os.Getwd() - if err != nil { - t.Fatalf("err: %s", err) - } - if err := os.Chdir(varFileDir); err != nil { - t.Fatalf("err: %s", err) - } - defer os.Chdir(cwd) - - p := applyFixtureProvider() - view, done := testView(t) - c := &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - - args := []string{ - "-state-out", statePath, - planPath, - } - code := c.Run(args) - output := done(t) - if code == 0 { - t.Fatalf("expected to fail, but succeeded. \n\n%s", output.All()) - } - - expectedTitle := "Can't set variable when applying a saved plan" - if !strings.Contains(output.Stderr(), expectedTitle) { - t.Fatalf("Expected stderr to contain %q, got %q", expectedTitle, output.Stderr()) - } -} - func TestApply_planWithVarFileChangingVariableValue(t *testing.T) { varFileDir := testTempDir(t) varFilePath := filepath.Join(varFileDir, "terraform.tfvars") - if err := ioutil.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { + if err := os.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { t.Fatalf("err: %s", err) } - // The value of foo is differnet from the var file + // The value of foo is different from the var file planPath := applyFixturePlanFileWithVariableValue(t, "lorem ipsum") statePath := testTempFile(t) @@ -1289,6 +1257,114 @@ foo = "bar" } } +// Variables can be passed to apply now for ephemeral usage, but we need to +// ensure that the legacy handling of undeclared variables remains intact +func TestApply_changedVars_applyTime(t *testing.T) { + t.Run("undeclared-config-var", func(t *testing.T) { + // an undeclared config variable is a warning, just like during plan + varFileDir := testTempDir(t) + varFilePath := filepath.Join(varFileDir, "terraform.tfvars") + if err := os.WriteFile(varFilePath, []byte(`undeclared = true`), 0644); err != nil { + t.Fatalf("err: %s", err) + } + + // The value of foo is not set + planPath := applyFixturePlanFile(t) + statePath := testTempFile(t) + + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("err: %s", err) + } + if err := os.Chdir(varFileDir); err != nil { + t.Fatalf("err: %s", err) + } + defer os.Chdir(cwd) + + p := applyFixtureProvider() + view, done := testView(t) + c := &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + + args := []string{ + "-state-out", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("unexpected exit code %d:\n\n%s", code, output.All()) + } + + if !strings.Contains(output.All(), `Value for undeclared variable`) { + t.Fatalf("missing undeclared warning:\n%s", output.All()) + } + }) + + t.Run("undeclared-cli-var", func(t *testing.T) { + // an undeclared cli variable is an error, just like during plan + planPath := applyFixturePlanFile(t) + statePath := testTempFile(t) + + p := applyFixtureProvider() + view, done := testView(t) + c := &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + + args := []string{ + "-var", "undeclared=true", + "-state-out", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code != 1 { + t.Fatalf("unexpected exit code %d:\n\n%s", code, output.All()) + } + + if !strings.Contains(output.Stderr(), `Value for undeclared variable`) { + t.Fatalf("missing undeclared warning:\n%s", output.All()) + } + }) + + t.Run("changed-cli-var", func(t *testing.T) { + planPath := applyFixturePlanFileWithVariableValue(t, "orig") + statePath := testTempFile(t) + + p := applyFixtureProvider() + view, done := testView(t) + c := &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + + args := []string{ + "-var", "foo=new", + "-state-out", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code != 1 { + t.Fatalf("unexpected exit code %d:\n\n%s", code, output.All()) + } + + if !strings.Contains(output.Stderr(), `Can't change variable when applying a saved plan`) { + t.Fatalf("missing undeclared warning:\n%s", output.All()) + } + }) +} + // we should be able to apply a plan file with no other file dependencies func TestApply_planNoModuleFiles(t *testing.T) { // temporary data directory which we can remove between commands @@ -1800,7 +1876,7 @@ func TestApply_varFileDefault(t *testing.T) { defer testChdir(t, td)() varFilePath := filepath.Join(td, "terraform.tfvars") - if err := ioutil.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { + if err := os.WriteFile(varFilePath, []byte(applyVarFile), 0644); err != nil { t.Fatalf("err: %s", err) } @@ -2595,10 +2671,10 @@ func applyFixturePlanFileMatchState(t *testing.T, stateMeta statemgr.SnapshotMet // a single change to create the test_instance.foo and a variable value that is included in the // "apply" test fixture, returning the location of that plan file. func applyFixturePlanFileWithVariableValue(t *testing.T, value string) string { - _, snap := testModuleWithSnapshot(t, "apply") + _, snap := testModuleWithSnapshot(t, "apply-vars") plannedVal := cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "ami": cty.StringVal("bar"), + "id": cty.UnknownVal(cty.String), + "value": cty.StringVal("bar"), }) priorValRaw, err := plans.NewDynamicValue(cty.NullVal(plannedVal.Type()), plannedVal.Type()) if err != nil {