diff --git a/CHANGELOG.md b/CHANGELOG.md index fb084892d2..46cc7a1d0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ UPGRADE NOTES: NEW FEATURES: ENHANCEMENTS: +* Added user input prompt for static variables. ([#1792](https://github.com/opentofu/opentofu/issues/1792)) * Added `-show-sensitive` flag to tofu plan, apply, state-show and output commands to display sensitive data in output. ([#1554](https://github.com/opentofu/opentofu/pull/1554)) * Improved performance for large graphs when debug logs are not enabled. ([#1810](https://github.com/opentofu/opentofu/pull/1810)) * Improved performance for large graphs with many submodules. ([#1809](https://github.com/opentofu/opentofu/pull/1809)) diff --git a/internal/command/test.go b/internal/command/test.go index 8932d3279d..e9490573b3 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -530,8 +530,9 @@ func (runner *TestFileRunner) ExecuteTestRun(run *moduletest.Run, file *modulete planCtx, plan, planDiags := runner.plan(config, state, run, file) if run.Config.Command == configs.PlanTestCommand { + expectedFailures, sourceRanges := run.BuildExpectedFailuresAndSourceMaps() // Then we want to assess our conditions and diagnostics differently. - planDiags = run.ValidateExpectedFailures(planDiags) + planDiags = run.ValidateExpectedFailures(expectedFailures, sourceRanges, planDiags) run.Diagnostics = run.Diagnostics.Append(planDiags) if planDiags.HasErrors() { run.Status = moduletest.Error @@ -576,6 +577,10 @@ func (runner *TestFileRunner) ExecuteTestRun(run *moduletest.Run, file *modulete return state, false } + expectedFailures, sourceRanges := run.BuildExpectedFailuresAndSourceMaps() + + planDiags = checkProblematicPlanErrors(expectedFailures, planDiags) + // Otherwise any error during the planning prevents our apply from // continuing which is an error. run.Diagnostics = run.Diagnostics.Append(planDiags) @@ -601,7 +606,7 @@ func (runner *TestFileRunner) ExecuteTestRun(run *moduletest.Run, file *modulete applyCtx, updated, applyDiags := runner.apply(plan, state, config, run, file) // Remove expected diagnostics, and add diagnostics in case anything that should have failed didn't. - applyDiags = run.ValidateExpectedFailures(applyDiags) + applyDiags = run.ValidateExpectedFailures(expectedFailures, sourceRanges, applyDiags) run.Diagnostics = run.Diagnostics.Append(applyDiags) if applyDiags.HasErrors() { @@ -1266,3 +1271,26 @@ func (runner *TestFileRunner) prepareInputVariablesForAssertions(config *configs config.Module.Variables = currentVars }, diags } + +// checkProblematicPlanErrors checks for plan errors that are also "expected" by the tests. In some cases we expect an error, however, +// what causes the error might not be what we expected. So we try to warn about that here. +func checkProblematicPlanErrors(expectedFailures addrs.Map[addrs.Referenceable, bool], planDiags tfdiags.Diagnostics) tfdiags.Diagnostics { + for _, diag := range planDiags { + rule, ok := addrs.DiagnosticOriginatesFromCheckRule(diag) + if !ok { + continue + } + if rule.Container.CheckableKind() != addrs.CheckableInputVariable { + continue + } + + addr, ok := rule.Container.(addrs.AbsInputVariableInstance) + if ok && expectedFailures.Has(addr.Variable) { + planDiags = planDiags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Invalid Variable in test file", + fmt.Sprintf("Variable %s, has an invalid value within the test. Although this was an expected failure, it has meant the apply stage was unable to run so the overall test will fail.", rule.Container.String()))) + } + } + return planDiags +} diff --git a/internal/command/test_test.go b/internal/command/test_test.go index a3908e0693..a85d9ad698 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -1241,6 +1241,100 @@ Success! 1 passed, 0 failed. } } +func TestTest_InvalidLocalVariables(t *testing.T) { + tcs := map[string]struct { + contains []string + notContains []string + code int + skip bool + }{ + "invalid_variable_warning_expected": { + contains: []string{ + "Warning: Invalid Variable in test file", + "Error: Invalid value for variable", + "This was checked by the validation rule at main.tf:5,3-13.", + "This was checked by the validation rule at main.tf:14,3-13.", + }, + code: 1, + }, + "invalid_variable_warning_no_expected": { + contains: []string{ + "Error: Invalid value for variable", + "This was checked by the validation rule at main.tf:5,3-13.", + "This was checked by the validation rule at main.tf:14,3-13.", + }, + notContains: []string{ + "Warning: Invalid Variable in test file", + }, + code: 1, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + if tc.skip { + t.Skip() + } + + file := name + + td := t.TempDir() + testCopyDir(t, testFixturePath(path.Join("test", file)), td) + defer testChdir(t, td)() + + provider := testing_command.NewProvider(nil) + providerSource, providerClose := newMockProviderSource(t, map[string][]string{ + "test": {"1.0.0"}, + }) + defer providerClose() + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + ui := new(cli.MockUi) + meta := Meta{ + testingOverrides: metaOverridesForProvider(provider.Provider), + Ui: ui, + View: view, + Streams: streams, + ProviderSource: providerSource, + } + + init := &InitCommand{ + Meta: meta, + } + + if code := init.Run(nil); code != 0 { + t.Fatalf("expected status code 0 but got %d: %s", code, ui.ErrorWriter) + } + + command := &TestCommand{ + Meta: meta, + } + + code := command.Run([]string{"-verbose", "-no-color"}) + output := done(t) + + if code != tc.code { + t.Errorf("expected status code %d but got %d: %s", tc.code, code, output.All()) + } + + actual := output.All() + + for _, containsString := range tc.contains { + if !strings.Contains(actual, containsString) { + t.Errorf("expected '%s' in output but didn't find it: \n%s", containsString, output.All()) + } + } + + for _, notContainsString := range tc.notContains { + if strings.Contains(actual, notContainsString) { + t.Errorf("expected not to find '%s' in output: \n%s", notContainsString, output.All()) + } + } + }) + } +} + func TestTest_RunBlock(t *testing.T) { tcs := map[string]struct { expected string diff --git a/internal/command/testdata/test/invalid_variable_warning_expected/main.tf b/internal/command/testdata/test/invalid_variable_warning_expected/main.tf new file mode 100644 index 0000000000..4ce1518c25 --- /dev/null +++ b/internal/command/testdata/test/invalid_variable_warning_expected/main.tf @@ -0,0 +1,19 @@ + +variable "variable1" { + type = string + + validation { + condition = var.variable1 == "foobar" + error_message = "The variable1 value must be: foobar" + } +} + +variable "variable2" { + type = string + + validation { + condition = var.variable2 == "barfoo" + error_message = "The variable2 value must be: barfoo" + } +} + diff --git a/internal/command/testdata/test/invalid_variable_warning_expected/main.tftest.hcl b/internal/command/testdata/test/invalid_variable_warning_expected/main.tftest.hcl new file mode 100644 index 0000000000..2ecff89923 --- /dev/null +++ b/internal/command/testdata/test/invalid_variable_warning_expected/main.tftest.hcl @@ -0,0 +1,10 @@ +run "invalid_object" { + variables { + variable1 = "lalalala" + variable2 = "lalalala" + } + + expect_failures = [ + var.variable1, + ] +} diff --git a/internal/command/testdata/test/invalid_variable_warning_no_expected/main.tf b/internal/command/testdata/test/invalid_variable_warning_no_expected/main.tf new file mode 100644 index 0000000000..4ce1518c25 --- /dev/null +++ b/internal/command/testdata/test/invalid_variable_warning_no_expected/main.tf @@ -0,0 +1,19 @@ + +variable "variable1" { + type = string + + validation { + condition = var.variable1 == "foobar" + error_message = "The variable1 value must be: foobar" + } +} + +variable "variable2" { + type = string + + validation { + condition = var.variable2 == "barfoo" + error_message = "The variable2 value must be: barfoo" + } +} + diff --git a/internal/command/testdata/test/invalid_variable_warning_no_expected/main.tftest.hcl b/internal/command/testdata/test/invalid_variable_warning_no_expected/main.tftest.hcl new file mode 100644 index 0000000000..b5e5080679 --- /dev/null +++ b/internal/command/testdata/test/invalid_variable_warning_no_expected/main.tftest.hcl @@ -0,0 +1,9 @@ +run "invalid_object" { + variables { + variable1 = "lalalala" + variable2 = "lalalala" + } + + expect_failures = [ + ] +} diff --git a/internal/moduletest/run.go b/internal/moduletest/run.go index 1f9195051b..c8ffb08055 100644 --- a/internal/moduletest/run.go +++ b/internal/moduletest/run.go @@ -130,25 +130,9 @@ func (run *Run) GetReferences() ([]*addrs.Reference, tfdiags.Diagnostics) { // diagnostics were generated by custom conditions. OpenTofu adds the // addrs.CheckRule that generated each diagnostic to the diagnostic itself so we // can tell which diagnostics can be expected. -func (run *Run) ValidateExpectedFailures(originals tfdiags.Diagnostics) tfdiags.Diagnostics { - - // We're going to capture all the checkable objects that are referenced - // from the expected failures. - expectedFailures := addrs.MakeMap[addrs.Referenceable, bool]() - sourceRanges := addrs.MakeMap[addrs.Referenceable, tfdiags.SourceRange]() - - for _, traversal := range run.Config.ExpectFailures { - // Ignore the diagnostics returned from the reference parsing, these - // references will have been checked earlier in the process by the - // validate stage so we don't need to do that again here. - reference, _ := addrs.ParseRefFromTestingScope(traversal) - expectedFailures.Put(reference.Subject, false) - sourceRanges.Put(reference.Subject, reference.SourceRange) - } - +func (run *Run) ValidateExpectedFailures(expectedFailures addrs.Map[addrs.Referenceable, bool], sourceRanges addrs.Map[addrs.Referenceable, tfdiags.SourceRange], originals tfdiags.Diagnostics) tfdiags.Diagnostics { //nolint: funlen,gocognit,cyclop // legacy code causes this to trigger, we actually reduced the statements var diags tfdiags.Diagnostics for _, diag := range originals { - if rule, ok := addrs.DiagnosticOriginatesFromCheckRule(diag); ok { switch rule.Container.CheckableKind() { case addrs.CheckableOutputValue: @@ -340,3 +324,20 @@ func (run *Run) ValidateExpectedFailures(originals tfdiags.Diagnostics) tfdiags. return diags } + +// BuildExpectedFailuresAndSourceMaps captures all the checkable objects that are referenced +// from the expected failures. +func (run *Run) BuildExpectedFailuresAndSourceMaps() (addrs.Map[addrs.Referenceable, bool], addrs.Map[addrs.Referenceable, tfdiags.SourceRange]) { + expectedFailures := addrs.MakeMap[addrs.Referenceable, bool]() + sourceRanges := addrs.MakeMap[addrs.Referenceable, tfdiags.SourceRange]() + + for _, traversal := range run.Config.ExpectFailures { + // Ignore the diagnostics returned from the reference parsing, these + // references will have been checked earlier in the process by the + // validate stage so we don't need to do that again here. + reference, _ := addrs.ParseRefFromTestingScope(traversal) + expectedFailures.Put(reference.Subject, false) + sourceRanges.Put(reference.Subject, reference.SourceRange) + } + return expectedFailures, sourceRanges +} diff --git a/internal/moduletest/run_test.go b/internal/moduletest/run_test.go index 5e32152aec..bef93bf65c 100644 --- a/internal/moduletest/run_test.go +++ b/internal/moduletest/run_test.go @@ -767,8 +767,9 @@ func TestRun_ValidateExpectedFailures(t *testing.T) { ExpectFailures: traversals, }, } + expectedFailures, sourceRanges := run.BuildExpectedFailuresAndSourceMaps() - out := run.ValidateExpectedFailures(tc.Input) + out := run.ValidateExpectedFailures(expectedFailures, sourceRanges, tc.Input) ix := 0 for ; ix < len(tc.Output); ix++ { expected := tc.Output[ix]