diff --git a/internal/initwd/from_module.go b/internal/initwd/from_module.go index e944cac981..7249fe1537 100644 --- a/internal/initwd/from_module.go +++ b/internal/initwd/from_module.go @@ -254,21 +254,52 @@ func DirFromModule(rootDir, modulesDir, sourceAddr string, reg *registry.Client, var parentKey string if lastDot := strings.LastIndexByte(newKey, '.'); lastDot != -1 { parentKey = newKey[:lastDot] - } else { - parentKey = "" // parent is the root module } - parentOld := instManifest[initFromModuleRootKeyPrefix+parentKey] + var parentOld modsdir.Record + // "" is the root module; all other modules get `root.` added as a prefix + if parentKey == "" { + parentOld = instManifest[parentKey] + } else { + parentOld = instManifest[initFromModuleRootKeyPrefix+parentKey] + } parentNew := retManifest[parentKey] // We need to figure out which portion of our directory is the // parent package path and which portion is the subdirectory // under that. - baseDirRel, err := filepath.Rel(parentOld.Dir, record.Dir) + var baseDirRel string + baseDirRel, err = filepath.Rel(parentOld.Dir, record.Dir) if err != nil { - // Should never happen, because we constructed both directories - // from the same base and so they must have a common prefix. - panic(err) + // This error may occur when installing a local module with a + // relative path, for e.g. if the source is in a directory above + // the destination ("../") + if parentOld.Dir == "." { + absDir, err := filepath.Abs(parentOld.Dir) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to determine module install directory", + fmt.Sprintf("Error determine relative source directory for module %s: %s.", newKey, err), + )) + continue + } + baseDirRel, err = filepath.Rel(absDir, record.Dir) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to determine relative module source location", + fmt.Sprintf("Error determining relative source for module %s: %s.", newKey, err), + )) + continue + } + } else { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to determine relative module source location", + fmt.Sprintf("Error determining relative source for module %s: %s.", newKey, err), + )) + } } newDir := filepath.Join(parentNew.Dir, baseDirRel) diff --git a/internal/initwd/from_module_test.go b/internal/initwd/from_module_test.go index f45e2b3242..96b7fee34d 100644 --- a/internal/initwd/from_module_test.go +++ b/internal/initwd/from_module_test.go @@ -1,6 +1,8 @@ package initwd import ( + "fmt" + "io/ioutil" "os" "path/filepath" "strings" @@ -9,6 +11,7 @@ import ( version "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configload" + "github.com/hashicorp/terraform/internal/copydir" "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/tfdiags" ) @@ -20,6 +23,7 @@ func TestDirFromModule_registry(t *testing.T) { fixtureDir := filepath.Clean("testdata/empty") tmpDir, done := tempChdir(t, fixtureDir) + defer done() // the module installer runs filepath.EvalSymlinks() on the destination // directory before copying files, and the resultant directory is what is @@ -30,7 +34,6 @@ func TestDirFromModule_registry(t *testing.T) { t.Error(err) } modsDir := filepath.Join(dir, ".terraform/modules") - defer done() hooks := &testInstallHooks{} @@ -38,7 +41,7 @@ func TestDirFromModule_registry(t *testing.T) { diags := DirFromModule(dir, modsDir, "hashicorp/module-installer-acctest/aws//examples/main", reg, hooks) assertNoDiagnostics(t, diags) - v := version.Must(version.NewVersion("0.0.1")) + v := version.Must(version.NewVersion("0.0.2")) wantCalls := []testInstallHookCall{ // The module specified to populate the root directory is not mentioned @@ -61,17 +64,17 @@ func TestDirFromModule_registry(t *testing.T) { Name: "Install", ModuleAddr: "root", Version: v, - LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff"), + LocalPath: filepath.Join(dir, fmt.Sprintf(".terraform/modules/root/terraform-aws-module-installer-acctest-%s", v)), }, { Name: "Install", ModuleAddr: "root.child_a", - LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_a"), + LocalPath: filepath.Join(dir, fmt.Sprintf(".terraform/modules/root/terraform-aws-module-installer-acctest-%s/modules/child_a", v)), }, { Name: "Install", ModuleAddr: "root.child_a.child_b", - LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_b"), + LocalPath: filepath.Join(dir, fmt.Sprintf(".terraform/modules/root/terraform-aws-module-installer-acctest-%s/modules/child_b", v)), }, } @@ -111,3 +114,165 @@ func TestDirFromModule_registry(t *testing.T) { }) assertResultDeepEqual(t, gotTraces, wantTraces) } + +func TestDirFromModule_submodules(t *testing.T) { + fixtureDir := filepath.Clean("testdata/empty") + fromModuleDir, err := filepath.Abs("./testdata/local-modules") + if err != nil { + t.Fatal(err) + } + + tmpDir, done := tempChdir(t, fixtureDir) + defer done() + + hooks := &testInstallHooks{} + dir, err := filepath.EvalSymlinks(tmpDir) + if err != nil { + t.Error(err) + } + modInstallDir := filepath.Join(dir, ".terraform/modules") + + diags := DirFromModule(dir, modInstallDir, fromModuleDir, nil, hooks) + assertNoDiagnostics(t, diags) + wantCalls := []testInstallHookCall{ + { + Name: "Install", + ModuleAddr: "child_a", + LocalPath: filepath.Join(fromModuleDir, "child_a"), + }, + { + Name: "Install", + ModuleAddr: "child_a.child_b", + LocalPath: filepath.Join(fromModuleDir, "child_a/child_b"), + }, + } + + if assertResultDeepEqual(t, hooks.Calls, wantCalls) { + return + } + + loader, err := configload.NewLoader(&configload.Config{ + ModulesDir: modInstallDir, + }) + if err != nil { + t.Fatal(err) + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + config, loadDiags := loader.LoadConfig(".") + if assertNoDiagnostics(t, tfdiags.Diagnostics{}.Append(loadDiags)) { + return + } + wantTraces := map[string]string{ + "": "in root module", + "child_a": "in child_a module", + "child_a.child_b": "in child_b module", + } + gotTraces := map[string]string{} + + config.DeepEach(func(c *configs.Config) { + path := strings.Join(c.Path, ".") + if c.Module.Variables["v"] == nil { + gotTraces[path] = "" + return + } + varDesc := c.Module.Variables["v"].Description + gotTraces[path] = varDesc + }) + assertResultDeepEqual(t, gotTraces, wantTraces) +} + +// TestDirFromModule_rel_submodules is similar to the test above, but the +// from-module is relative to the install dir ("../"): +// https://github.com/hashicorp/terraform/issues/23010 +func TestDirFromModule_rel_submodules(t *testing.T) { + // This test creates a tmpdir with the following directory structure: + // - tmpdir/local-modules (with contents of testdata/local-modules) + // - tmpdir/empty: the workDir we CD into for the test + // - tmpdir/empty/target (target, the destination for init -from-module) + tmpDir, err := ioutil.TempDir("", "terraform-configload") + if err != nil { + t.Fatal(err) + } + fromModuleDir := filepath.Join(tmpDir, "local-modules") + workDir := filepath.Join(tmpDir, "empty") + if err := os.Mkdir(fromModuleDir, os.ModePerm); err != nil { + t.Fatal(err) + } + if err := copydir.CopyDir(fromModuleDir, "testdata/local-modules"); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(workDir, os.ModePerm); err != nil { + t.Fatal(err) + } + + targetDir := filepath.Join(tmpDir, "target") + if err := os.Mkdir(targetDir, os.ModePerm); err != nil { + t.Fatal(err) + } + oldDir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + err = os.Chdir(targetDir) + if err != nil { + t.Fatalf("failed to switch to temp dir %s: %s", tmpDir, err) + } + defer os.Chdir(oldDir) + defer os.RemoveAll(tmpDir) + + hooks := &testInstallHooks{} + + modInstallDir := ".terraform/modules" + sourceDir := "../local-modules" + diags := DirFromModule(".", modInstallDir, sourceDir, nil, hooks) + assertNoDiagnostics(t, diags) + wantCalls := []testInstallHookCall{ + { + Name: "Install", + ModuleAddr: "child_a", + LocalPath: filepath.Join(sourceDir, "child_a"), + }, + { + Name: "Install", + ModuleAddr: "child_a.child_b", + LocalPath: filepath.Join(sourceDir, "child_a/child_b"), + }, + } + + if assertResultDeepEqual(t, hooks.Calls, wantCalls) { + return + } + + loader, err := configload.NewLoader(&configload.Config{ + ModulesDir: modInstallDir, + }) + if err != nil { + t.Fatal(err) + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + config, loadDiags := loader.LoadConfig(".") + if assertNoDiagnostics(t, tfdiags.Diagnostics{}.Append(loadDiags)) { + return + } + wantTraces := map[string]string{ + "": "in root module", + "child_a": "in child_a module", + "child_a.child_b": "in child_b module", + } + gotTraces := map[string]string{} + + config.DeepEach(func(c *configs.Config) { + path := strings.Join(c.Path, ".") + if c.Module.Variables["v"] == nil { + gotTraces[path] = "" + return + } + varDesc := c.Module.Variables["v"].Description + gotTraces[path] = varDesc + }) + assertResultDeepEqual(t, gotTraces, wantTraces) +} diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index d4d7eb1a78..0183e9b5b3 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -268,7 +268,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { Name: "Install", ModuleAddr: "acctest_child_a", Version: v, - LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a"), + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/terraform-aws-module-installer-acctest-0.0.1/modules/child_a"), }, // acctest_child_a.child_b @@ -276,7 +276,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_child_a.child_b", - LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"), + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/terraform-aws-module-installer-acctest-0.0.1/modules/child_b"), }, // acctest_child_b accesses //modules/child_b directly @@ -290,7 +290,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { Name: "Install", ModuleAddr: "acctest_child_b", Version: v, - LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_b/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"), + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_b/terraform-aws-module-installer-acctest-0.0.1/modules/child_b"), }, // acctest_root @@ -304,7 +304,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { Name: "Install", ModuleAddr: "acctest_root", Version: v, - LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038"), + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/terraform-aws-module-installer-acctest-0.0.1"), }, // acctest_root.child_a @@ -312,7 +312,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root.child_a", - LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a"), + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/terraform-aws-module-installer-acctest-0.0.1/modules/child_a"), }, // acctest_root.child_a.child_b @@ -320,7 +320,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root.child_a.child_b", - LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"), + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/terraform-aws-module-installer-acctest-0.0.1/modules/child_b"), }, }