move getStorage to moduleStorage too

Add a test to ensure we don't store different modules with the same
relative path in the same location.

config/module/get.go

@@ -64,18 +64,6 @@ func GetCopy(dst, src string) error {
 	return copyDir(dst, tmpDir)
 }
 
-func getStorage(s getter.Storage, key string, src string, mode GetMode) (string, bool, error) {
-	// Get the module with the level specified if we were told to.
-	if mode > GetModeNone {
-		if err := s.Get(key, src, mode == GetModeUpdate); err != nil {
-			return "", false, err
-		}
-	}
-
-	// Get the directory where the module is.
-	return s.Dir(key)
-}
-
 const (
 	registryAPI   = "https://registry.terraform.io/v1/modules"
 	xTerraformGet = "X-Terraform-Get"

config/module/storage.go

@@ -44,7 +44,7 @@ type moduleRecord struct {
 	Root string
 }
 
-// moduleStorgae implements methods to record and fetch metadata about the
+// moduleStorage implements methods to record and fetch metadata about the
 // modules that have been fetched and stored locally. The getter.Storgae
 // abstraction doesn't provide the information needed to know which versions of
 // a module have been stored, or their location.
@@ -163,3 +163,18 @@ func (m moduleStorage) recordModuleRoot(dir, root string) error {
 
 	return m.recordModule(rec)
 }
+
+func (m moduleStorage) getStorage(key string, src string, mode GetMode) (string, bool, error) {
+	// Get the module with the level specified if we were told to.
+	if mode > GetModeNone {
+		log.Printf("[DEBUG] fetching %q with key %q", src, key)
+		if err := m.Storage.Get(key, src, mode == GetModeUpdate); err != nil {
+			return "", false, err
+		}
+	}
+
+	// Get the directory where the module is.
+	dir, found, err := m.Storage.Dir(key)
+	log.Printf("[DEBUG] found %q in %q: %t", src, dir, found)
+	return dir, found, err
+}

config/module/test-fixtures/conficting-submodule-names/a/c/main.tf

@@ -0,0 +1 @@
+resource "test_instance" "a-c" {}

config/module/test-fixtures/conficting-submodule-names/a/main.tf

@@ -0,0 +1,3 @@
+module "c" {
+  source = "./c"
+}

config/module/test-fixtures/conficting-submodule-names/b/c/main.tf

@@ -0,0 +1 @@
+resource "test_instance" "b-c" {}

config/module/test-fixtures/conficting-submodule-names/b/main.tf

@@ -0,0 +1,3 @@
+module "c" {
+  source = "./c"
+}

config/module/test-fixtures/conficting-submodule-names/main.tf

@@ -0,0 +1,7 @@
+module "a" {
+  source = "./a"
+}
+
+module "b" {
+  source = "./b"
+}

config/module/tree.go

@@ -248,7 +248,7 @@ func (t *Tree) Load(storage getter.Storage, mode GetMode) error {
 
 		log.Printf("[TRACE] getting module source %q", source)
 
-		dir, ok, err := getStorage(s, key, source, mode)
+		dir, ok, err := s.getStorage(key, source, mode)
 		if err != nil {
 			return err
 		}

config/module/tree_test.go

@@ -690,6 +690,45 @@ func TestTreeProviders_implicitMultiLevel(t *testing.T) {
 	}
 }
 
+func TestTreeLoad_conflictingSubmoduleNames(t *testing.T) {
+	storage := testStorage(t)
+	tree := NewTree("", testConfig(t, "conficting-submodule-names"))
+
+	if err := tree.Load(storage, GetModeGet); err != nil {
+		t.Fatalf("load failed: %s", err)
+	}
+
+	if !tree.Loaded() {
+		t.Fatal("should be loaded")
+	}
+
+	// Try to reload
+	if err := tree.Load(storage, GetModeNone); err != nil {
+		t.Fatalf("reload failed: %s", err)
+	}
+
+	// verify that the grand-children are correctly loaded
+	for _, c := range tree.Children() {
+		for _, gc := range c.Children() {
+			if len(gc.config.Resources) != 1 {
+				t.Fatalf("expected 1 resource in %s, got %d", gc.name, len(gc.config.Resources))
+			}
+			res := gc.config.Resources[0]
+			switch gc.path[0] {
+			case "a":
+				if res.Name != "a-c" {
+					t.Fatal("found wrong resource in a/c:", res.Name)
+				}
+			case "b":
+				if res.Name != "b-c" {
+					t.Fatal("found wrong resource in b/c:", res.Name)
+				}
+			}
+
+		}
+	}
+}
+
 const treeLoadStr = `
 root
   foo (path: foo)