From f4ff263d245863aa5c572018dbc04a14eb5186d7 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Mon, 21 Oct 2024 14:41:58 +0200 Subject: [PATCH 01/13] restructure existing test --- internal/command/apply_test.go | 117 +++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 51 deletions(-) diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index d304095b436f..9082178cc42e 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -23,12 +23,14 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/collections" + "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" testing_provider "github.com/hashicorp/terraform/internal/providers/testing" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statemgr" + "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -992,65 +994,78 @@ func TestApply_planVarsEphemeral_applyTime(t *testing.T) { statePath := testTempFile(t) p := applyFixtureProvider() - view, done := testView(t) - c := &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } + + var view *views.View + var done func(t *testing.T) *terminal.TestOutput + var c *ApplyCommand + var args []string + var code int + var output *terminal.TestOutput // Test first that an apply supplying only the apply-time variable "foo" // succeeds. - args := []string{ - "-state", statePath, - "-var", "foo=bar", - planPath, - } - code := c.Run(args) - output := done(t) - if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) - } + t.Run("only passing ephemeral variable", func(t *testing.T) { + view, done = testView(t) + c = &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + args = []string{ + "-state", statePath, + "-var", "foo=bar", + planPath, + } + code = c.Run(args) + output = done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.Stderr()) + } + }) // Now test that supplying "bar", which is not an apply-time variable, fails. - view, done = testView(t) - c = &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - args = []string{ - "-state", statePath, - "-var", "foo=bar", - "-var", "bar=bar", - planPath, - } - code = c.Run(args) - output = done(t) - if code == 0 { - t.Fatal("should've failed: ", output.Stdout()) - } + t.Run("passing non-ephemeral variable", func(t *testing.T) { + view, done = testView(t) + c = &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + args = []string{ + "-state", statePath, + "-var", "foo=bar", + "-var", "bar=bar", + planPath, + } + code = c.Run(args) + output = done(t) + if code == 0 { + t.Fatal("should've failed: ", output.Stdout()) + } + }) // Finally, test that the apply also fails if we do *not* supply a value for // the apply-time variable foo. - view, done = testView(t) - c = &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - args = []string{ - "-state", statePath, - planPath, - } - code = c.Run(args) - output = done(t) - if code == 0 { - t.Fatal("should've failed: ", output.Stdout()) - } + t.Run("missing ephemeral variable", func(t *testing.T) { + view, done = testView(t) + c = &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + args = []string{ + "-state", statePath, + planPath, + } + code = c.Run(args) + output = done(t) + if code == 0 { + t.Fatal("should've failed: ", output.Stdout()) + } + }) } // we should be able to apply a plan file with no other file dependencies From 756a1a74add5dc76decbc7f848e6c8df828bcf8a Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Wed, 23 Oct 2024 12:37:04 +0200 Subject: [PATCH 02/13] ephemeral: add apply tests --- internal/command/apply_test.go | 43 +++++++++++++++++++++++++++++++- internal/command/command_test.go | 6 +++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index 9082178cc42e..d66aaf8d1f85 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -1046,7 +1046,7 @@ func TestApply_planVarsEphemeral_applyTime(t *testing.T) { } }) - // Finally, test that the apply also fails if we do *not* supply a value for + // Test that the apply also fails if we do *not* supply a value for // the apply-time variable foo. t.Run("missing ephemeral variable", func(t *testing.T) { view, done = testView(t) @@ -1066,6 +1066,47 @@ func TestApply_planVarsEphemeral_applyTime(t *testing.T) { t.Fatal("should've failed: ", output.Stdout()) } }) + + t.Run("passing ephemeral variable through vars file", func(t *testing.T) { + view, done = testView(t) + c = &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + const planVarFile = ` +foo = "bar" +` + + // Write a tfvars file with the variable + tfVarsPath := testVarsFile(t) + err := os.WriteFile(tfVarsPath, []byte(planVarFile), 0600) + if err != nil { + t.Fatalf("Could not write vars file %e", err) + } + + args = []string{ + "-state", statePath, + "-var-file", tfVarsPath, + planPath, + } + code = c.Run(args) + output = done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.Stderr()) + } + }) + + t.Run("passing ephemeral variable through environment variable", func(t *testing.T) { + // https://github.com/hashicorp/terraform/blob/b21a5703bdc0af3d7730c0b8b9f68e41a4bc9645/internal/command/meta_vars.go#L95 + t.Skip("TODO") + }) + + t.Run("passing ephemeral variable through interactive prompts", func(t *testing.T) { + // Look at https://github.com/hashicorp/terraform/blob/b21a5703bdc0af3d7730c0b8b9f68e41a4bc9645/internal/command/plan_test.go#L794 for inspiration + t.Skip("TODO") + }) } // we should be able to apply a plan file with no other file dependencies diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 563c74479a33..023e34962556 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -553,6 +553,12 @@ func testTempFile(t *testing.T) string { return filepath.Join(testTempDir(t), "state.tfstate") } +func testVarsFile(t *testing.T) string { + t.Helper() + + return filepath.Join(testTempDir(t), "variables.tfvars") +} + func testTempDir(t *testing.T) string { t.Helper() d, err := filepath.EvalSymlinks(t.TempDir()) From a87b36a1a894a2dd2e7a72ecef344e90c2ffd96a Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 24 Oct 2024 16:17:44 +0200 Subject: [PATCH 03/13] restructure tests --- internal/command/apply_test.go | 298 ++++++++++++++++----------------- 1 file changed, 147 insertions(+), 151 deletions(-) diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index d66aaf8d1f85..4292b6e3acfe 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -23,7 +23,6 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/collections" - "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" @@ -945,168 +944,165 @@ func TestApply_planVars(t *testing.T) { // that supplying a declared ephemeral input variable that is *not* in the list // of apply-time variables fails. func TestApply_planVarsEphemeral_applyTime(t *testing.T) { - td := t.TempDir() - testCopyDir(t, testFixturePath("apply-ephemeral-variable"), td) - defer testChdir(t, td)() - - _, snap := testModuleWithSnapshot(t, "apply-ephemeral-variable") - plannedVal := cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "ami": cty.StringVal("bar"), - }) - priorValRaw, err := plans.NewDynamicValue(cty.NullVal(plannedVal.Type()), plannedVal.Type()) - if err != nil { - t.Fatal(err) - } - plannedValRaw, err := plans.NewDynamicValue(plannedVal, plannedVal.Type()) - if err != nil { - t.Fatal(err) - } - plan := testPlan(t) - plan.Changes.AppendResourceInstanceChange(&plans.ResourceInstanceChangeSrc{ - Addr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_instance", - Name: "foo", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - ProviderAddr: addrs.AbsProviderConfig{ - Provider: addrs.NewDefaultProvider("test"), - Module: addrs.RootModule, - }, - ChangeSrc: plans.ChangeSrc{ - Action: plans.Create, - Before: priorValRaw, - After: plannedValRaw, + for name, tc := range map[string]func(*testing.T, *ApplyCommand, string, string, func(*testing.T) *terminal.TestOutput){ + // Test first that an apply supplying only the apply-time variable "foo" + // succeeds. + "only passing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + args := []string{ + "-state", statePath, + "-var", "foo=bar", + planPath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.Stderr()) + } }, - }) - applyTimeVariables := collections.NewSetCmp[string]() - applyTimeVariables.Add("foo") - plan.ApplyTimeVariables = applyTimeVariables - planPath := testPlanFileMatchState( - t, - snap, - states.NewState(), - plan, - statemgr.SnapshotMeta{}, - ) + // Now test that supplying "bar", which is not an apply-time variable, fails. + "passing non-ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + args := []string{ + "-state", statePath, + "-var", "foo=bar", + "-var", "bar=bar", + planPath, + } + code := c.Run(args) + output := done(t) + if code == 0 { + t.Fatal("should've failed: ", output.Stdout()) + } + }, - statePath := testTempFile(t) + // Test that the apply also fails if we do *not* supply a value for + // the apply-time variable foo. + "missing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + args := []string{ + "-state", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code == 0 { + t.Fatal("should've failed: ", output.Stdout()) + } + }, - p := applyFixtureProvider() + "passing ephemeral variable through vars file": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + const planVarFile = ` +foo = "bar" +` - var view *views.View - var done func(t *testing.T) *terminal.TestOutput - var c *ApplyCommand - var args []string - var code int - var output *terminal.TestOutput - - // Test first that an apply supplying only the apply-time variable "foo" - // succeeds. - t.Run("only passing ephemeral variable", func(t *testing.T) { - view, done = testView(t) - c = &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - args = []string{ - "-state", statePath, - "-var", "foo=bar", - planPath, - } - code = c.Run(args) - output = done(t) - if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) - } - }) + // Write a tfvars file with the variable + tfVarsPath := testVarsFile(t) + err := os.WriteFile(tfVarsPath, []byte(planVarFile), 0600) + if err != nil { + t.Fatalf("Could not write vars file %e", err) + } - // Now test that supplying "bar", which is not an apply-time variable, fails. - t.Run("passing non-ephemeral variable", func(t *testing.T) { - view, done = testView(t) - c = &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - args = []string{ - "-state", statePath, - "-var", "foo=bar", - "-var", "bar=bar", - planPath, - } - code = c.Run(args) - output = done(t) - if code == 0 { - t.Fatal("should've failed: ", output.Stdout()) - } - }) + args := []string{ + "-state", statePath, + "-var-file", tfVarsPath, + planPath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.Stderr()) + } + }, - // Test that the apply also fails if we do *not* supply a value for - // the apply-time variable foo. - t.Run("missing ephemeral variable", func(t *testing.T) { - view, done = testView(t) - c = &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - args = []string{ - "-state", statePath, - planPath, - } - code = c.Run(args) - output = done(t) - if code == 0 { - t.Fatal("should've failed: ", output.Stdout()) - } - }) + "passing ephemeral variable through environment variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + t.Setenv("TF_VAR_foo", "bar") - t.Run("passing ephemeral variable through vars file", func(t *testing.T) { - view, done = testView(t) - c = &ApplyCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(p), - View: view, - }, - } - const planVarFile = ` -foo = "bar" -` + args := []string{ + "-state", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.Stderr()) + } + }, - // Write a tfvars file with the variable - tfVarsPath := testVarsFile(t) - err := os.WriteFile(tfVarsPath, []byte(planVarFile), 0600) - if err != nil { - t.Fatalf("Could not write vars file %e", err) - } + "passing ephemeral variable through interactive prompts": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + close := testInteractiveInput(t, []string{"bar"}) + defer close() - args = []string{ - "-state", statePath, - "-var-file", tfVarsPath, - planPath, - } - code = c.Run(args) - output = done(t) - if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) - } - }) + args := []string{ + "-state", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.Stderr()) + } + }, + } { + t.Run(name, func(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath("apply-ephemeral-variable"), td) + defer testChdir(t, td)() - t.Run("passing ephemeral variable through environment variable", func(t *testing.T) { - // https://github.com/hashicorp/terraform/blob/b21a5703bdc0af3d7730c0b8b9f68e41a4bc9645/internal/command/meta_vars.go#L95 - t.Skip("TODO") - }) + _, snap := testModuleWithSnapshot(t, "apply-ephemeral-variable") + plannedVal := cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "ami": cty.StringVal("bar"), + }) + priorValRaw, err := plans.NewDynamicValue(cty.NullVal(plannedVal.Type()), plannedVal.Type()) + if err != nil { + t.Fatal(err) + } + plannedValRaw, err := plans.NewDynamicValue(plannedVal, plannedVal.Type()) + if err != nil { + t.Fatal(err) + } + plan := testPlan(t) + plan.Changes.AppendResourceInstanceChange(&plans.ResourceInstanceChangeSrc{ + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ProviderAddr: addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ChangeSrc: plans.ChangeSrc{ + Action: plans.Create, + Before: priorValRaw, + After: plannedValRaw, + }, + }) + applyTimeVariables := collections.NewSetCmp[string]() + applyTimeVariables.Add("foo") + plan.ApplyTimeVariables = applyTimeVariables + + planPath := testPlanFileMatchState( + t, + snap, + states.NewState(), + plan, + statemgr.SnapshotMeta{}, + ) + + statePath := testTempFile(t) + + p := applyFixtureProvider() + view, done := testView(t) + c := &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } - t.Run("passing ephemeral variable through interactive prompts", func(t *testing.T) { - // Look at https://github.com/hashicorp/terraform/blob/b21a5703bdc0af3d7730c0b8b9f68e41a4bc9645/internal/command/plan_test.go#L794 for inspiration - t.Skip("TODO") - }) + tc(t, c, statePath, planPath, done) + }) + } } // we should be able to apply a plan file with no other file dependencies From a4b99ab07d0c051ca15a1ce138b75f3c092b6415 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 25 Oct 2024 14:00:06 +0200 Subject: [PATCH 04/13] allow environment variables to fill in ephemeral values --- internal/backend/local/backend_apply.go | 148 ++++++++++++------------ 1 file changed, 75 insertions(+), 73 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 29aa55d3a4d0..6c6d455a73b5 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -247,16 +247,19 @@ func (b *Local) opApply( continue } - if v.SourceType == terraform.ValueFromCLIArg || v.SourceType == terraform.ValueFromNamedFile { - var rng *hcl.Range - if v.HasSourceRange() { - rng = v.SourceRange.ToHCL().Ptr() - } + var rng *hcl.Range + if v.HasSourceRange() { + rng = v.SourceRange.ToHCL().Ptr() + } - // 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. - decl, ok := lr.Config.Module.Variables[varName] + 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, @@ -266,80 +269,79 @@ func (b *Local) opApply( }) continue } + } - // If the var is declared as ephemeral in config, go ahead and handle it - if decl.Ephemeral { - // Determine whether this is an apply-time variable, i.e. an - // ephemeral variable that was set (non-null) during the - // planning phase. - applyTimeVar := false - for avName := range plan.ApplyTimeVariables.All() { - if varName == avName { - applyTimeVar = true - } - } - - // If this isn't an apply-time variable, it's not valid to - // set it during apply. - if !applyTimeVar { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Ephemeral variable was not set during planning", - Detail: fmt.Sprintf( - "The ephemeral input variable %q was not set during the planning phase, and so must remain unset during the apply phase.", - varName, - ), - Subject: rng, - }) - continue - } - - // Get the value of the variable, because we'll need it for - // the next two steps. - val, valDiags := rawV.ParseVariableValue(decl.ParsingMode) - diags = diags.Append(valDiags) - if valDiags.HasErrors() { - continue + // If the var is declared as ephemeral in config, go ahead and handle it + if decl.Ephemeral { + // Determine whether this is an apply-time variable, i.e. an + // ephemeral variable that was set (non-null) during the + // planning phase. + applyTimeVar := false + for avName := range plan.ApplyTimeVariables.All() { + if varName == avName { + applyTimeVar = true } + } - // If this is an apply-time variable, the user must supply a - // value during apply: it can't be null. - if applyTimeVar && val.Value.IsNull() { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Ephemeral variable must be set for apply", - Detail: fmt.Sprintf( - "The ephemeral input variable %q was set during the planning phase, and so must be set again during the apply phase.", - varName, - ), - }) - continue - } + // If this isn't an apply-time variable, it's not valid to + // set it during apply. + if !applyTimeVar { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Ephemeral variable was not set during planning", + Detail: fmt.Sprintf( + "The ephemeral input variable %q was not set during the planning phase, and so must remain unset during the apply phase.", + varName, + ), + Subject: rng, + }) + continue + } - // If we get here, we are in possession of a non-null - // ephemeral apply-time input variable, and need only pass - // its value on to the ApplyOpts. - applyTimeValues[varName] = val - } else { - // TODO: We should probably actually tolerate this if the new - // value is equal to the value that was saved in the plan, since - // that'd make it possible to, for example, reuse a .tfvars file - // containing a mixture of ephemeral and non-ephemeral definitions - // during the apply phase, rather than having to split ephemeral - // and non-ephemeral definitions into separate files. For initial - // experiment we'll keep things a little simpler, though, and - // just skip this check if we're doing a combined plan/apply where - // the apply phase will therefore always have exactly the same - // inputs as the plan phase. + // Get the value of the variable, because we'll need it for + // the next two steps. + val, valDiags := rawV.ParseVariableValue(decl.ParsingMode) + diags = diags.Append(valDiags) + if valDiags.HasErrors() { + continue + } + // If this is an apply-time variable, the user must supply a + // value during apply: it can't be null. + if applyTimeVar && val.Value.IsNull() { 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 a saved plan includes the variable values that were set when it was created. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName), - Subject: rng, + Summary: "Ephemeral variable must be set for apply", + Detail: fmt.Sprintf( + "The ephemeral input variable %q was set during the planning phase, and so must be set again during the apply phase.", + varName, + ), }) + continue } + // If we get here, we are in possession of a non-null + // ephemeral apply-time input variable, and need only pass + // its value on to the ApplyOpts. + applyTimeValues[varName] = val + } else { + // TODO: We should probably actually tolerate this if the new + // value is equal to the value that was saved in the plan, since + // that'd make it possible to, for example, reuse a .tfvars file + // containing a mixture of ephemeral and non-ephemeral definitions + // during the apply phase, rather than having to split ephemeral + // and non-ephemeral definitions into separate files. For initial + // experiment we'll keep things a little simpler, though, and + // just skip this check if we're doing a combined plan/apply where + // the apply phase will therefore always have exactly the same + // inputs as the plan phase. + + 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 a saved plan includes the variable values that were set when it was created. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName), + Subject: rng, + }) } } applyOpts = &terraform.ApplyOpts{ From 0a71f24453b4543bccc9c1b3118043e9b89fc17f Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 25 Oct 2024 15:44:18 +0200 Subject: [PATCH 05/13] disable interactive prompt test for now --- internal/command/apply_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index 4292b6e3acfe..2d05fb8e2d4f 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -1027,20 +1027,20 @@ foo = "bar" } }, - "passing ephemeral variable through interactive prompts": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { - close := testInteractiveInput(t, []string{"bar"}) - defer close() - - args := []string{ - "-state", statePath, - planPath, - } - code := c.Run(args) - output := done(t) - if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) - } - }, + // "passing ephemeral variable through interactive prompts": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + // close := testInteractiveInput(t, []string{"bar"}) + // defer close() + + // args := []string{ + // "-state", statePath, + // planPath, + // } + // code := c.Run(args) + // output := done(t) + // if code != 0 { + // t.Fatal("should've succeeded: ", output.Stderr()) + // } + // }, } { t.Run(name, func(t *testing.T) { td := t.TempDir() From bf68ac8110843ede04e333e3310d59b5e3dc1100 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 25 Oct 2024 15:57:38 +0200 Subject: [PATCH 06/13] make sure we only access decl if it is present --- internal/backend/local/backend_apply.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 6c6d455a73b5..2a0a4d38baf9 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -272,7 +272,7 @@ func (b *Local) opApply( } // If the var is declared as ephemeral in config, go ahead and handle it - if decl.Ephemeral { + if ok && decl.Ephemeral { // Determine whether this is an apply-time variable, i.e. an // ephemeral variable that was set (non-null) during the // planning phase. From 0e632aae860349c4e2befa04f3be7293e5b656b2 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Mon, 4 Nov 2024 14:32:15 +0100 Subject: [PATCH 07/13] ephemeral: allow setting non-ephemeral values during apply to same value as in plan --- internal/backend/local/backend_apply.go | 53 ++++++++++++++-------- internal/command/apply_test.go | 58 ++++++++++++++++++++++++- internal/terraform/variables.go | 21 +++++++++ 3 files changed, 114 insertions(+), 18 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 2a0a4d38baf9..5c2fa59b24b8 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -11,6 +11,7 @@ import ( "time" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend/backendrun" @@ -325,23 +326,41 @@ func (b *Local) opApply( // its value on to the ApplyOpts. applyTimeValues[varName] = val } else { - // TODO: We should probably actually tolerate this if the new - // value is equal to the value that was saved in the plan, since - // that'd make it possible to, for example, reuse a .tfvars file - // containing a mixture of ephemeral and non-ephemeral definitions - // during the apply phase, rather than having to split ephemeral - // and non-ephemeral definitions into separate files. For initial - // experiment we'll keep things a little simpler, though, and - // just skip this check if we're doing a combined plan/apply where - // the apply phase will therefore always have exactly the same - // inputs as the plan phase. - - 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 a saved plan includes the variable values that were set when it was created. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName), - Subject: rng, - }) + // If a non-ephemeral variable is set differently between plan and apply, we should emit a diagnostic. + value, ok := plan.VariableValues[varName] + if !ok { + if v.Value.IsNull() { + continue + } else { + // TODO: Add test for this case + 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 a saved plan includes the variable values that were set when it was created. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName), + Subject: rng, + }) + } + } + + val, err := value.Decode(cty.DynamicPseudoType) + if err != nil { + // TODO: Reword error message + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Variable was set with a different type when applying a saved plan", + Detail: fmt.Sprintf("The variable %s was set to a different type of value during plan than during apply. Please either don't supply the value or supply the same value if the variable.", varName), + Subject: rng, + }) + } else { + if v.Value.Equals(val) == cty.False { + 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 a saved plan includes the variable values that were set when it was created. The saved plan specifies %q as the value whereas during apply the value %q was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, v.Value.GoString(), val.GoString(), v.SourceType.DiagnosticLabel()), + Subject: rng, + }) + } + } } } applyOpts = &terraform.ApplyOpts{ diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index 2d05fb8e2d4f..7222a0dd3e9a 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -860,7 +860,7 @@ func TestApply_planWithVarFile(t *testing.T) { t.Fatalf("err: %s", err) } - planPath := applyFixturePlanFile(t) + planPath := applyFixturePlanFileWithVariableValue(t, "bar") statePath := testTempFile(t) cwd, err := os.Getwd() @@ -2407,6 +2407,53 @@ func applyFixturePlanFileMatchState(t *testing.T, stateMeta statemgr.SnapshotMet ) } +// applyFixturePlanFileWithVariableValue creates a plan file at a temporary location containing +// 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") + plannedVal := cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "ami": cty.StringVal("bar"), + }) + priorValRaw, err := plans.NewDynamicValue(cty.NullVal(plannedVal.Type()), plannedVal.Type()) + if err != nil { + t.Fatal(err) + } + plannedValRaw, err := plans.NewDynamicValue(plannedVal, plannedVal.Type()) + if err != nil { + t.Fatal(err) + } + plan := testPlan(t) + plan.Changes.AppendResourceInstanceChange(&plans.ResourceInstanceChangeSrc{ + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ProviderAddr: addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ChangeSrc: plans.ChangeSrc{ + Action: plans.Create, + Before: priorValRaw, + After: plannedValRaw, + }, + }) + + plan.VariableValues = map[string]plans.DynamicValue{ + "foo": mustNewDynamicValue(value, cty.DynamicPseudoType), + } + return testPlanFileMatchState( + t, + snap, + states.NewState(), + plan, + statemgr.SnapshotMeta{}, + ) +} + const applyVarFile = ` foo = "bar" ` @@ -2414,3 +2461,12 @@ foo = "bar" const applyVarFileJSON = ` { "foo": "bar" } ` + +func mustNewDynamicValue(val string, ty cty.Type) plans.DynamicValue { + realVal := cty.StringVal(val) + ret, err := plans.NewDynamicValue(realVal, ty) + if err != nil { + panic(err) + } + return ret +} diff --git a/internal/terraform/variables.go b/internal/terraform/variables.go index 4b2984789c49..4adae05a7d79 100644 --- a/internal/terraform/variables.go +++ b/internal/terraform/variables.go @@ -137,6 +137,27 @@ func (v ValueSourceType) GoString() string { return fmt.Sprintf("terraform.%s", v) } +func (v ValueSourceType) DiagnosticLabel() string { + switch v { + case ValueFromConfig: + return "set by the default value in configuration" + case ValueFromAutoFile: + return "set by an automatically loaded .tfvars file" + case ValueFromNamedFile: + return "set by a .tfvars file passed through -var-file argument" + case ValueFromCLIArg: + return "set by a CLI argument" + case ValueFromEnvVar: + return "set by an environment variable" + case ValueFromInput: + return "set by an interactive input" + case ValueFromPlan: + return "set by the plan" + default: + return "unknown" + } +} + //go:generate go run golang.org/x/tools/cmd/stringer -type ValueSourceType // InputValues is a map of InputValue instances. From fa44937977ddf29f7fe443fb97dcb20b3e9e821e Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Mon, 4 Nov 2024 15:09:24 +0100 Subject: [PATCH 08/13] ephemeral: improve diagnostic message through compactValueStr --- internal/backend/local/backend_apply.go | 3 ++- internal/command/views/json/diagnostic.go | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 5c2fa59b24b8..60b63bcf5044 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend/backendrun" "github.com/hashicorp/terraform/internal/command/views" + viewsjson "github.com/hashicorp/terraform/internal/command/views/json" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/internal/plans" @@ -356,7 +357,7 @@ func (b *Local) opApply( 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 a saved plan includes the variable values that were set when it was created. The saved plan specifies %q as the value whereas during apply the value %q was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, v.Value.GoString(), val.GoString(), v.SourceType.DiagnosticLabel()), + Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options when applying a saved plan file, because a saved plan includes the variable values that were set when it was created. The saved plan specifies %s as the value whereas during apply the value %s was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, viewsjson.CompactValueStr(v.Value), viewsjson.CompactValueStr(val), v.SourceType.DiagnosticLabel()), Subject: rng, }) } diff --git a/internal/command/views/json/diagnostic.go b/internal/command/views/json/diagnostic.go index 51f987b61876..4d4032814a08 100644 --- a/internal/command/views/json/diagnostic.go +++ b/internal/command/views/json/diagnostic.go @@ -438,13 +438,17 @@ func parseRange(src []byte, rng hcl.Range) (*hcl.File, int) { return file, offset } -// compactValueStr produces a compact, single-line summary of a given value +// CompactValueStr produces a compact, single-line summary of a given value // that is suitable for display in the UI. // // For primitives it returns a full representation, while for more complex // types it instead summarizes the type, size, etc to produce something // that is hopefully still somewhat useful but not as verbose as a rendering // of the entire data structure. +func CompactValueStr(val cty.Value) string { + return compactValueStr(val) +} + func compactValueStr(val cty.Value) string { // This is a specialized subset of value rendering tailored to producing // helpful but concise messages in diagnostics. It is not comprehensive From 320783672904e1cc809fc0ce6f5237e08b180efb Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Mon, 4 Nov 2024 15:34:14 +0100 Subject: [PATCH 09/13] ephemeral: tests for setting unplanned vars during apply --- internal/backend/local/backend_apply.go | 29 ++++---- internal/command/apply_test.go | 91 +++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 17 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 60b63bcf5044..a87edb29cd85 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -328,35 +328,30 @@ func (b *Local) opApply( applyTimeValues[varName] = val } else { // If a non-ephemeral variable is set differently between plan and apply, we should emit a diagnostic. - value, ok := plan.VariableValues[varName] + plannedVariableValue, ok := plan.VariableValues[varName] if !ok { - if v.Value.IsNull() { - continue - } else { - // TODO: Add test for this case - 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 a saved plan includes the variable values that were set when it was created. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName), - Subject: rng, - }) - } + 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, + }) + continue } - val, err := value.Decode(cty.DynamicPseudoType) + val, err := plannedVariableValue.Decode(cty.DynamicPseudoType) if err != nil { - // TODO: Reword error message diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Variable was set with a different type when applying a saved plan", - Detail: fmt.Sprintf("The variable %s was set to a different type of value during plan than during apply. Please either don't supply the value or supply the same value if the variable.", varName), + Summary: "Could not decode variable value from plan", + Detail: fmt.Sprintf("The variable %s could not be decoded from the plan. %s. This is a bug in Terraform, please report it.", varName, err), Subject: rng, }) } else { if v.Value.Equals(val) == cty.False { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Can't set variable when applying a saved plan", + Summary: "Can't change 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 a saved plan includes the variable values that were set when it was created. The saved plan specifies %s as the value whereas during apply the value %s was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, viewsjson.CompactValueStr(v.Value), viewsjson.CompactValueStr(val), v.SourceType.DiagnosticLabel()), Subject: rng, }) diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index 7222a0dd3e9a..16340d054637 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -860,6 +860,7 @@ func TestApply_planWithVarFile(t *testing.T) { t.Fatalf("err: %s", err) } + // The value of foo is the same as in the var file planPath := applyFixturePlanFileWithVariableValue(t, "bar") statePath := testTempFile(t) @@ -901,6 +902,96 @@ 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 { + t.Fatalf("err: %s", err) + } + + // The value of foo is differnet from the var file + planPath := applyFixturePlanFileWithVariableValue(t, "lorem ipsum") + 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 change 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_planVars(t *testing.T) { // This test ensures that it isn't allowed to set non-ephemeral input // variables when applying from a saved plan file, since in that case the From d550c9755396920bfd4ed412c58eae4b37bbc0f2 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Mon, 4 Nov 2024 15:41:44 +0100 Subject: [PATCH 10/13] ephermal: allow setting var and var files for apply --- internal/command/apply.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/command/apply.go b/internal/command/apply.go index d889f92345da..3bf441ecadd5 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -385,6 +385,15 @@ Options: -state-out=path Path to write state to that is different than "-state". This can be used to preserve the old state. + + -var 'foo=bar' Set a value for one of the input variables in the root + module of the configuration. Use this option more than + once to set more than one variable. + + -var-file=filename Load variable values from the given file, in addition + to the default files terraform.tfvars and *.auto.tfvars. + Use this option more than once to include more than one + variables file. If you don't provide a saved plan file then this command will also accept all of the plan-customization options accepted by the terraform plan command. From 2e026dec0f88c5013b01b2366e992f71e18379b3 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Wed, 6 Nov 2024 14:20:02 +0100 Subject: [PATCH 11/13] ephemeral: set ephemeral variables for combined plans as well they are by definition the same as in the plan, but there is no reason to skip the validation step --- internal/backend/local/backend_apply.go | 4 +- internal/command/apply_test.go | 147 ++++++++++++++---- .../testdata/apply-ephemeral-variable/main.tf | 1 - 3 files changed, 119 insertions(+), 33 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index a87edb29cd85..7859e7377ce0 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -90,10 +90,8 @@ func (b *Local) opApply( stateHook.PersistInterval = time.Duration(op.StatePersistInterval) * time.Second var plan *plans.Plan - combinedPlanApply := false // If we weren't given a plan, then we refresh/plan if op.PlanFile == nil { - combinedPlanApply = true // Perform the plan log.Printf("[INFO] backend/local: apply calling Plan") plan, moreDiags = lr.Core.Plan(lr.Config, lr.InputState, lr.PlanOpts) @@ -235,7 +233,7 @@ func (b *Local) opApply( stateHook.StateMgr = opState var applyOpts *terraform.ApplyOpts - if len(op.Variables) != 0 && !combinedPlanApply { + if len(op.Variables) != 0 { applyTimeValues := make(terraform.InputValues, plan.ApplyTimeVariables.Len()) for varName, rawV := range op.Variables { // We're "parsing" only to get the resulting value's SourceType, diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index 16340d054637..bdc32c43462d 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -1034,11 +1034,12 @@ func TestApply_planVars(t *testing.T) { // Test that an apply supplying all apply-time variables succeeds, and then test // that supplying a declared ephemeral input variable that is *not* in the list // of apply-time variables fails. +// +// In the fixture used for this test foo is a required ephemeral variable, whereas bar is +// an optional one. func TestApply_planVarsEphemeral_applyTime(t *testing.T) { for name, tc := range map[string]func(*testing.T, *ApplyCommand, string, string, func(*testing.T) *terminal.TestOutput){ - // Test first that an apply supplying only the apply-time variable "foo" - // succeeds. - "only passing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + "with planfile only passing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { args := []string{ "-state", statePath, "-var", "foo=bar", @@ -1047,12 +1048,11 @@ func TestApply_planVarsEphemeral_applyTime(t *testing.T) { code := c.Run(args) output := done(t) if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) + t.Fatal("should've succeeded: ", output.All()) } }, - // Now test that supplying "bar", which is not an apply-time variable, fails. - "passing non-ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + "with planfile passing non-ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { args := []string{ "-state", statePath, "-var", "foo=bar", @@ -1062,13 +1062,11 @@ func TestApply_planVarsEphemeral_applyTime(t *testing.T) { code := c.Run(args) output := done(t) if code == 0 { - t.Fatal("should've failed: ", output.Stdout()) + t.Fatal("should've failed: ", output.All()) } }, - // Test that the apply also fails if we do *not* supply a value for - // the apply-time variable foo. - "missing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + "with planfile missing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { args := []string{ "-state", statePath, planPath, @@ -1076,11 +1074,11 @@ func TestApply_planVarsEphemeral_applyTime(t *testing.T) { code := c.Run(args) output := done(t) if code == 0 { - t.Fatal("should've failed: ", output.Stdout()) + t.Fatal("should've failed: ", output.All()) } }, - "passing ephemeral variable through vars file": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + "with planfile passing ephemeral variable through vars file": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { const planVarFile = ` foo = "bar" ` @@ -1100,12 +1098,13 @@ foo = "bar" code := c.Run(args) output := done(t) if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) + t.Fatal("should've succeeded: ", output.All()) } }, - "passing ephemeral variable through environment variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + "with planfile passing ephemeral variable through environment variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { t.Setenv("TF_VAR_foo", "bar") + defer t.Setenv("TF_VAR_foo", "") args := []string{ "-state", statePath, @@ -1114,24 +1113,114 @@ foo = "bar" code := c.Run(args) output := done(t) if code != 0 { - t.Fatal("should've succeeded: ", output.Stderr()) + t.Fatal("should've succeeded: ", output.All()) + } + }, + + "with planfile passing ephemeral variable through interactive prompts": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + close := testInteractiveInput(t, []string{"bar"}) + defer close() + + args := []string{ + "-state", statePath, + planPath, + } + code := c.Run(args) + output := done(t) + if code == 0 { + // We don't support interactive inputs for apply-time variables + t.Fatal("should have failed: ", output.All()) + } + }, + + "without planfile only passing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + args := []string{ + "-state", statePath, + "-var", "foo=bar", + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.All()) + } + }, + + "without planfile passing non-ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + args := []string{ + "-state", statePath, + "-var", "foo=bar", + "-var", "bar=bar", + } + code := c.Run(args) + output := done(t) + + // For a combined plan & apply operation it's okay (and expected) to also be able to pass non-ephemeral variables + if code != 0 { + t.Fatal("should've succeeded: ", output.All()) + } + }, + + "without planfile missing ephemeral variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + args := []string{ + "-state", statePath, + } + code := c.Run(args) + output := done(t) + if code == 0 { + t.Fatal("should've failed: ", output.All()) + } + }, + + "without planfile passing ephemeral variable through vars file": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + const planVarFile = ` +foo = "bar" +` + + // Write a tfvars file with the variable + tfVarsPath := testVarsFile(t) + err := os.WriteFile(tfVarsPath, []byte(planVarFile), 0600) + if err != nil { + t.Fatalf("Could not write vars file %e", err) + } + + args := []string{ + "-state", statePath, + "-var-file", tfVarsPath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.All()) } }, - // "passing ephemeral variable through interactive prompts": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { - // close := testInteractiveInput(t, []string{"bar"}) - // defer close() - - // args := []string{ - // "-state", statePath, - // planPath, - // } - // code := c.Run(args) - // output := done(t) - // if code != 0 { - // t.Fatal("should've succeeded: ", output.Stderr()) - // } - // }, + "without planfile passing ephemeral variable through environment variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + t.Setenv("TF_VAR_foo", "bar") + defer t.Setenv("TF_VAR_foo", "") + + args := []string{ + "-state", statePath, + } + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatal("should've succeeded: ", output.All()) + } + }, + + "without planfile passing ephemeral variable through interactive prompts": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) { + close := testInteractiveInput(t, []string{"bar"}) + defer close() + + args := []string{ + "-state", statePath, + } + code := c.Run(args) + output := done(t) + if code == 0 { + t.Fatal("should've failed: ", output.All()) + } + }, } { t.Run(name, func(t *testing.T) { td := t.TempDir() diff --git a/internal/command/testdata/apply-ephemeral-variable/main.tf b/internal/command/testdata/apply-ephemeral-variable/main.tf index 247cbcbfef7e..f1b83b2052e3 100644 --- a/internal/command/testdata/apply-ephemeral-variable/main.tf +++ b/internal/command/testdata/apply-ephemeral-variable/main.tf @@ -1,6 +1,5 @@ variable "foo" { type = string - default = null ephemeral = true } From f484653765e2a2b8529757aebf508749966492ac Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Wed, 6 Nov 2024 14:48:12 +0100 Subject: [PATCH 12/13] ephemeral: use .False to compare to cty false value --- internal/backend/local/backend_apply.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index 7859e7377ce0..f8aceb9ccd02 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -346,7 +346,7 @@ func (b *Local) opApply( Subject: rng, }) } else { - if v.Value.Equals(val) == cty.False { + if v.Value.Equals(val).False() { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Can't change variable when applying a saved plan", From eccf39aa4577fefef63282fc1e666f418bd7a33d Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 7 Nov 2024 11:48:55 +0100 Subject: [PATCH 13/13] ephemeral: support interactive variable input for ephemeral values --- internal/backend/local/backend_apply.go | 28 +++++++++++++++++++------ internal/command/apply_test.go | 4 ++-- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index f8aceb9ccd02..5c1d4f849bda 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -90,8 +90,10 @@ func (b *Local) opApply( stateHook.PersistInterval = time.Duration(op.StatePersistInterval) * time.Second var plan *plans.Plan + combinedPlanApply := false // If we weren't given a plan, then we refresh/plan if op.PlanFile == nil { + combinedPlanApply = true // Perform the plan log.Printf("[INFO] backend/local: apply calling Plan") plan, moreDiags = lr.Core.Plan(lr.Config, lr.InputState, lr.PlanOpts) @@ -232,9 +234,24 @@ func (b *Local) opApply( // Set up our hook for continuous state updates stateHook.StateMgr = opState - var applyOpts *terraform.ApplyOpts + applyTimeValues := make(terraform.InputValues, plan.ApplyTimeVariables.Len()) + + // In a combined plan/apply run, getting the context already gathers the interactive + // input, therefore we need to make sure to pass the ephemeral variables to the applyOpts. + if combinedPlanApply { + for varName, v := range lr.PlanOpts.SetVariables { + decl, ok := lr.Config.Module.Variables[varName] + if !ok { + continue // This should never happen, but we'll ignore it if it does. + } + + if v.SourceType == terraform.ValueFromInput && decl.Ephemeral { + applyTimeValues[varName] = v + } + } + } + if len(op.Variables) != 0 { - applyTimeValues := make(terraform.InputValues, plan.ApplyTimeVariables.Len()) for varName, rawV := range op.Variables { // We're "parsing" only to get the resulting value's SourceType, // so we'll use configs.VariableParseLiteral just because it's @@ -357,9 +374,6 @@ func (b *Local) opApply( } } } - applyOpts = &terraform.ApplyOpts{ - SetVariables: applyTimeValues, - } if diags.HasErrors() { op.ReportResult(runningOp, diags) return @@ -375,7 +389,9 @@ func (b *Local) opApply( defer close(doneCh) log.Printf("[INFO] backend/local: apply calling Apply") - applyState, applyDiags = lr.Core.Apply(plan, lr.Config, applyOpts) + applyState, applyDiags = lr.Core.Apply(plan, lr.Config, &terraform.ApplyOpts{ + SetVariables: applyTimeValues, + }) }() if b.opWait(doneCh, stopCtx, cancelCtx, lr.Core, opState, op.View) { diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index bdc32c43462d..8a9110a1426f 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -1217,8 +1217,8 @@ foo = "bar" } code := c.Run(args) output := done(t) - if code == 0 { - t.Fatal("should've failed: ", output.All()) + if code != 0 { + t.Fatal("should've succeeded: ", output.All()) } }, } {