From 4122ba86fcefb8f0edc5d605490d095c87bf9136 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 27 Jul 2023 10:22:53 +0200 Subject: [PATCH] terraform test: fix crash when using nested modules from test run blocks (#33589) --- internal/command/test_test.go | 55 +++++++++++++ .../test/with_nested_setup_modules/main.tf | 2 + .../with_nested_setup_modules/main.tftest.hcl | 14 ++++ .../with_nested_setup_modules/setup/main.tf | 14 ++++ .../setup/other/main.tf | 12 +++ internal/configs/config_build.go | 34 +++++++- internal/configs/config_build_test.go | 81 +++++++++++++++++++ .../with-tests-nested-module/main.tf | 2 + .../with-tests-nested-module/main.tftest.hcl | 14 ++++ .../with-tests-nested-module/setup/main.tf | 14 ++++ .../setup/other/main.tf | 12 +++ 11 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 internal/command/testdata/test/with_nested_setup_modules/main.tf create mode 100644 internal/command/testdata/test/with_nested_setup_modules/main.tftest.hcl create mode 100644 internal/command/testdata/test/with_nested_setup_modules/setup/main.tf create mode 100644 internal/command/testdata/test/with_nested_setup_modules/setup/other/main.tf create mode 100644 internal/configs/testdata/valid-modules/with-tests-nested-module/main.tf create mode 100644 internal/configs/testdata/valid-modules/with-tests-nested-module/main.tftest.hcl create mode 100644 internal/configs/testdata/valid-modules/with-tests-nested-module/setup/main.tf create mode 100644 internal/configs/testdata/valid-modules/with-tests-nested-module/setup/other/main.tf diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 67b7d452dc..4501c6fc69 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -615,3 +615,58 @@ variable can be declared with a variable "not_real" {} block. t.Errorf("should have deleted all resources on completion but left %v", provider.ResourceString()) } } + +func TestTest_NestedSetupModules(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath(path.Join("test", "with_nested_setup_modules")), td) + defer testChdir(t, td)() + + provider := testing_command.NewProvider(nil) + + providerSource, close := newMockProviderSource(t, map[string][]string{ + "test": {"1.0.0"}, + }) + defer close() + + 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(nil) + output := done(t) + + printedOutput := false + + if code != 0 { + printedOutput = true + t.Errorf("expected status code 0 but got %d: %s", code, output.All()) + } + + if provider.ResourceCount() > 0 { + if !printedOutput { + t.Errorf("should have deleted all resources on completion but left %s\n\n%s", provider.ResourceString(), output.All()) + } else { + t.Errorf("should have deleted all resources on completion but left %s", provider.ResourceString()) + } + } +} diff --git a/internal/command/testdata/test/with_nested_setup_modules/main.tf b/internal/command/testdata/test/with_nested_setup_modules/main.tf new file mode 100644 index 0000000000..f304def8b1 --- /dev/null +++ b/internal/command/testdata/test/with_nested_setup_modules/main.tf @@ -0,0 +1,2 @@ + +resource "test_resource" "resource" {} diff --git a/internal/command/testdata/test/with_nested_setup_modules/main.tftest.hcl b/internal/command/testdata/test/with_nested_setup_modules/main.tftest.hcl new file mode 100644 index 0000000000..3172b8c6dc --- /dev/null +++ b/internal/command/testdata/test/with_nested_setup_modules/main.tftest.hcl @@ -0,0 +1,14 @@ +variables { + value = "Hello, world!" +} + +run "load_module" { + module { + source = "./setup" + } + + assert { + condition = output.value == "Hello, world!" + error_message = "invalid value" + } +} \ No newline at end of file diff --git a/internal/command/testdata/test/with_nested_setup_modules/setup/main.tf b/internal/command/testdata/test/with_nested_setup_modules/setup/main.tf new file mode 100644 index 0000000000..54e468b7db --- /dev/null +++ b/internal/command/testdata/test/with_nested_setup_modules/setup/main.tf @@ -0,0 +1,14 @@ + +variable "value" { + type = string +} + +module "child" { + source = "./other" + + value = var.value +} + +output "value" { + value = module.child.value +} diff --git a/internal/command/testdata/test/with_nested_setup_modules/setup/other/main.tf b/internal/command/testdata/test/with_nested_setup_modules/setup/other/main.tf new file mode 100644 index 0000000000..e1f6e52f3f --- /dev/null +++ b/internal/command/testdata/test/with_nested_setup_modules/setup/other/main.tf @@ -0,0 +1,12 @@ + +variable "value" { + type = string +} + +resource "test_resource" "resource" { + value = var.value +} + +output "value" { + value = test_resource.resource.value +} diff --git a/internal/configs/config_build.go b/internal/configs/config_build.go index 829e169138..6f99189a5f 100644 --- a/internal/configs/config_build.go +++ b/internal/configs/config_build.go @@ -93,9 +93,18 @@ func buildTestModules(root *Config, walker ModuleWalker) hcl.Diagnostics { // In actuality, when this is executed it will be as if the // module was the root. So, we'll post-process some things to // get it to behave as expected later. - cfg.Path = addrs.RootModule + + // First, update the main module for this test run to behave as + // if it is the root module. cfg.Parent = nil - cfg.Root = cfg + + // Then we need to update the paths for this config and all + // children, so they think they are all relative to the root + // module we just created. + rebaseChildModule(cfg, cfg) + + // Finally, link the new config back into our test run so + // it can be retrieved later. run.ConfigUnderTest = cfg } } @@ -194,6 +203,27 @@ func loadModule(root *Config, req *ModuleRequest, walker ModuleWalker) (*Config, return cfg, diags } +// rebaseChildModule updates cfg to make it act as if root is the base of the +// module tree. +// +// This is used for modules loaded directly from test files. In order to load +// them properly, and reuse the code for loading modules from normal +// configuration files, we pretend they are children of the main configuration +// object. Later, when it comes time for them to execute they will act as if +// they are the root module directly. +// +// This function updates cfg so that it treats the provided root as the actual +// root of this module tree. It then recurses into all the child modules and +// does the same for them. +func rebaseChildModule(cfg *Config, root *Config) { + for _, child := range cfg.Children { + rebaseChildModule(child, root) + } + + cfg.Path = cfg.Path[len(root.Path):] + cfg.Root = root +} + // A ModuleWalker knows how to find and load a child module given details about // the module to be loaded and a reference to its partially-loaded parent // Config. diff --git a/internal/configs/config_build_test.go b/internal/configs/config_build_test.go index 1b0af9ce4e..d2868c220c 100644 --- a/internal/configs/config_build_test.go +++ b/internal/configs/config_build_test.go @@ -6,6 +6,7 @@ package configs import ( "fmt" "io/ioutil" + "path" "path/filepath" "reflect" "sort" @@ -283,6 +284,86 @@ func TestBuildConfigInvalidModules(t *testing.T) { } } +func TestBuildConfig_WithNestedTestModules(t *testing.T) { + parser := NewParser(nil) + mod, diags := parser.LoadConfigDirWithTests("testdata/valid-modules/with-tests-nested-module", "tests") + assertNoDiagnostics(t, diags) + if mod == nil { + t.Fatal("got nil root module; want non-nil") + } + + cfg, diags := BuildConfig(mod, ModuleWalkerFunc( + func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) { + + // Bit of a hack to get the test working, but we know all the source + // addresses in this test are locals, so we can just treat them as + // paths in the filesystem. + + addr := req.SourceAddr.String() + current := req.Parent + for current.SourceAddr != nil { + addr = path.Join(current.SourceAddr.String(), addr) + current = current.Parent + } + sourcePath := filepath.Join("testdata/valid-modules/with-tests-nested-module", addr) + + mod, diags := parser.LoadConfigDir(sourcePath) + version, _ := version.NewVersion("1.0.0") + return mod, version, diags + }, + )) + assertNoDiagnostics(t, diags) + if cfg == nil { + t.Fatal("got nil config; want non-nil") + } + + // We should have loaded our test case, and one of the test runs should + // have loaded an alternate module. + + if len(cfg.Module.Tests) != 1 { + t.Fatalf("expected exactly one test case but found %d", len(cfg.Module.Tests)) + } + + test := cfg.Module.Tests["main.tftest.hcl"] + if len(test.Runs) != 1 { + t.Fatalf("expected two test runs but found %d", len(test.Runs)) + } + + run := test.Runs[0] + if run.ConfigUnderTest == nil { + t.Fatalf("the first test run should have loaded config but did not") + } + + if run.ConfigUnderTest.Parent != nil { + t.Errorf("config under test should not have a parent") + } + + if run.ConfigUnderTest.Root != run.ConfigUnderTest { + t.Errorf("config under test root should be itself") + } + + if len(run.ConfigUnderTest.Path) > 0 { + t.Errorf("config under test path should be the root module") + } + + // We should also have loaded a single child underneath the config under + // test, and it should have valid paths. + + child := run.ConfigUnderTest.Children["child"] + + if child.Parent != run.ConfigUnderTest { + t.Errorf("child should point back to root") + } + + if len(child.Path) != 1 || child.Path[0] != "child" { + t.Errorf("child should have rebased against virtual root") + } + + if child.Root != run.ConfigUnderTest { + t.Errorf("child root should be main config under test") + } +} + func TestBuildConfig_WithTestModule(t *testing.T) { parser := NewParser(nil) mod, diags := parser.LoadConfigDirWithTests("testdata/valid-modules/with-tests-module", "tests") diff --git a/internal/configs/testdata/valid-modules/with-tests-nested-module/main.tf b/internal/configs/testdata/valid-modules/with-tests-nested-module/main.tf new file mode 100644 index 0000000000..f304def8b1 --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-nested-module/main.tf @@ -0,0 +1,2 @@ + +resource "test_resource" "resource" {} diff --git a/internal/configs/testdata/valid-modules/with-tests-nested-module/main.tftest.hcl b/internal/configs/testdata/valid-modules/with-tests-nested-module/main.tftest.hcl new file mode 100644 index 0000000000..3172b8c6dc --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-nested-module/main.tftest.hcl @@ -0,0 +1,14 @@ +variables { + value = "Hello, world!" +} + +run "load_module" { + module { + source = "./setup" + } + + assert { + condition = output.value == "Hello, world!" + error_message = "invalid value" + } +} \ No newline at end of file diff --git a/internal/configs/testdata/valid-modules/with-tests-nested-module/setup/main.tf b/internal/configs/testdata/valid-modules/with-tests-nested-module/setup/main.tf new file mode 100644 index 0000000000..54e468b7db --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-nested-module/setup/main.tf @@ -0,0 +1,14 @@ + +variable "value" { + type = string +} + +module "child" { + source = "./other" + + value = var.value +} + +output "value" { + value = module.child.value +} diff --git a/internal/configs/testdata/valid-modules/with-tests-nested-module/setup/other/main.tf b/internal/configs/testdata/valid-modules/with-tests-nested-module/setup/other/main.tf new file mode 100644 index 0000000000..e1f6e52f3f --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-nested-module/setup/other/main.tf @@ -0,0 +1,12 @@ + +variable "value" { + type = string +} + +resource "test_resource" "resource" { + value = var.value +} + +output "value" { + value = test_resource.resource.value +}