From bf04503f049147add0600f03aac5138f73d86a29 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Mon, 11 Mar 2019 15:25:21 -0700 Subject: [PATCH] internal/initwd: identify possible relative module sources Identify module sources that look like relative paths ("child" instead of "./child", for example) and surface a helpful error. Previously, such module sources would be passed to go-getter, which would fail because it was expecting an absolute, or properly relative, path. This commit moves the check for improper relative paths sooner so a user-friendly error can be displayed. --- internal/initwd/from_module_test.go | 6 +-- internal/initwd/getter.go | 18 +++++++- internal/initwd/module_install.go | 16 +++++++ internal/initwd/module_install_test.go | 44 +++++++++++++------ .../local-module-error/child_a/main.tf | 8 ++++ .../test-fixtures/local-module-error/main.tf | 8 ++++ 6 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 internal/initwd/test-fixtures/local-module-error/child_a/main.tf create mode 100644 internal/initwd/test-fixtures/local-module-error/main.tf diff --git a/internal/initwd/from_module_test.go b/internal/initwd/from_module_test.go index 5ebebae6b9..d48643e00d 100644 --- a/internal/initwd/from_module_test.go +++ b/internal/initwd/from_module_test.go @@ -52,17 +52,17 @@ func TestDirFromModule_registry(t *testing.T) { Name: "Install", ModuleAddr: "root", Version: v, - LocalPath: ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff", + LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff"), }, { Name: "Install", ModuleAddr: "root.child_a", - LocalPath: ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_a", + LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_a"), }, { Name: "Install", ModuleAddr: "root.child_a.child_b", - LocalPath: ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_b"), }, } diff --git a/internal/initwd/getter.go b/internal/initwd/getter.go index ada192b596..87fe7d82b3 100644 --- a/internal/initwd/getter.go +++ b/internal/initwd/getter.go @@ -78,7 +78,7 @@ type reusingGetter map[string]string // go-getter library, which have very inconsistent quality as // end-user-actionable error messages. At this time we do not have any // reasonable way to improve these error messages at this layer because -// the underlying errors are not separatelyr recognizable. +// the underlying errors are not separately recognizable. func (g reusingGetter) getWithGoGetter(instPath, addr string) (string, error) { packageAddr, subDir := splitAddrSubdir(addr) @@ -186,3 +186,19 @@ func isRegistrySourceAddr(addr string) bool { _, err := regsrc.ParseModuleSource(addr) return err == nil } + +func isMaybeRelativeLocalPath(addr, path string) bool { + realAddr, err := getter.Detect(addr, path, getter.Detectors) + // this error will be handled by the next function + if err != nil { + return false + } else { + if strings.HasPrefix(realAddr, "file://") { + _, err := os.Stat(realAddr[7:]) + if err != nil { + return true + } + } + } + return false +} diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go index 3bb4d0233e..acdb7e0fe4 100644 --- a/internal/initwd/module_install.go +++ b/internal/initwd/module_install.go @@ -199,6 +199,22 @@ func (i *ModuleInstaller) installDescendentModules(rootMod *tfconfig.Module, roo diags = append(diags, mDiags...) return mod, v, diags + case isMaybeRelativeLocalPath(req.SourceAddr, instPath): + log.Printf( + "[TRACE] ModuleInstaller: %s looks like a local path but is missing ./ or ../", + req.SourceAddr, + ) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to locate local module source", + fmt.Sprintf( + "%s looks like a relative path, but Terraform cannot determine the module source. "+ + "Add ./ at the start of the source string if this is a relative path.", + req.SourceAddr, + ), + )) + return nil, nil, diags + default: log.Printf("[TRACE] ModuleInstaller: %s address %q will be handled by go-getter", key, req.SourceAddr) diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index 13c10c181b..9fc952f367 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -93,12 +93,30 @@ func TestModuleInstaller(t *testing.T) { assertResultDeepEqual(t, gotTraces, wantTraces) } +func TestModuleInstaller_error(t *testing.T) { + fixtureDir := filepath.Clean("test-fixtures/local-module-error") + dir, done := tempChdir(t, fixtureDir) + defer done() + + hooks := &testInstallHooks{} + + modulesDir := filepath.Join(dir, ".terraform/modules") + inst := NewModuleInstaller(modulesDir, nil) + _, diags := inst.InstallModules(".", false, hooks) + + if !diags.HasErrors() { + t.Fatal("expected error") + } else { + assertDiagnosticSummary(t, diags, "Failed to locate local module source") + } +} + func TestLoaderInstallModules_registry(t *testing.T) { if os.Getenv("TF_ACC") == "" { t.Skip("this test accesses registry.terraform.io and github.com; set TF_ACC=1 to run it") } - fixtureDir := filepath.Clean("test-fixtures/local-modules") + fixtureDir := filepath.Clean("test-fixtures/registry-modules") dir, done := tempChdir(t, fixtureDir) defer done() @@ -125,7 +143,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { Name: "Install", ModuleAddr: "acctest_child_a", Version: v, - LocalPath: ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a"), }, // acctest_child_a.child_b @@ -133,7 +151,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_child_a.child_b", - LocalPath: ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"), }, // acctest_child_b accesses //modules/child_b directly @@ -147,7 +165,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { Name: "Install", ModuleAddr: "acctest_child_b", Version: v, - LocalPath: ".terraform/modules/acctest_child_b/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_b/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"), }, // acctest_root @@ -161,7 +179,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { Name: "Install", ModuleAddr: "acctest_root", Version: v, - LocalPath: ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038"), }, // acctest_root.child_a @@ -169,7 +187,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root.child_a", - LocalPath: ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a"), }, // acctest_root.child_a.child_b @@ -177,7 +195,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root.child_a.child_b", - LocalPath: ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"), }, } @@ -248,7 +266,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_child_a", - LocalPath: ".terraform/modules/acctest_child_a/modules/child_a", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/modules/child_a"), }, // acctest_child_a.child_b @@ -256,7 +274,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_child_a.child_b", - LocalPath: ".terraform/modules/acctest_child_a/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/modules/child_b"), }, // acctest_child_b accesses //modules/child_b directly @@ -268,7 +286,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_child_b", - LocalPath: ".terraform/modules/acctest_child_b/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_b/modules/child_b"), }, // acctest_root @@ -280,7 +298,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root", - LocalPath: ".terraform/modules/acctest_root", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root"), }, // acctest_root.child_a @@ -288,7 +306,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root.child_a", - LocalPath: ".terraform/modules/acctest_root/modules/child_a", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/modules/child_a"), }, // acctest_root.child_a.child_b @@ -296,7 +314,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { { Name: "Install", ModuleAddr: "acctest_root.child_a.child_b", - LocalPath: ".terraform/modules/acctest_root/modules/child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/modules/child_b"), }, } diff --git a/internal/initwd/test-fixtures/local-module-error/child_a/main.tf b/internal/initwd/test-fixtures/local-module-error/child_a/main.tf new file mode 100644 index 0000000000..03ebd8e476 --- /dev/null +++ b/internal/initwd/test-fixtures/local-module-error/child_a/main.tf @@ -0,0 +1,8 @@ +variable "v" { + description = "in child_ba module" + default = "" +} + +output "hello" { + value = "Hello from child_a!" +} diff --git a/internal/initwd/test-fixtures/local-module-error/main.tf b/internal/initwd/test-fixtures/local-module-error/main.tf new file mode 100644 index 0000000000..a31f913387 --- /dev/null +++ b/internal/initwd/test-fixtures/local-module-error/main.tf @@ -0,0 +1,8 @@ +variable "v" { + description = "in root module" + default = "" +} + +module "child_a" { + source = "child_a" +}