record the subdirectory in the FolderStorage

Module detection currently requires calling the registry to determine
the subdirectory. Since we're not directly accessing the subdirectory
through FolderStorage, and now handling it within terraform so modules can
reference sibling paths, we need to call out to the registry every
time we load a configuration to verify the subdirectory for the module,
which is returned during the Detect.

Record the subdirectories for each module in the top-level of the
FolderStorage path for retrieval during Tree.Load. This lets us bypass
Detection altogether, modules can be loaded without redetecting.

config/module/module_test.go

@@ -2,6 +2,7 @@ package module
 
 import (
 	"io/ioutil"
+	"log"
 	"os"
 	"path/filepath"
 	"testing"
@@ -10,6 +11,12 @@ import (
 	"github.com/hashicorp/terraform/config"
 )
 
+func init() {
+	if os.Getenv("TF_LOG") == "" {
+		log.SetOutput(ioutil.Discard)
+	}
+}
+
 const fixtureDir = "./test-fixtures"
 
 func tempDir(t *testing.T) string {

config/module/tree.go

@@ -3,8 +3,12 @@ package module
 import (
 	"bufio"
 	"bytes"
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"log"
+	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 
@@ -177,15 +181,52 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error {
 		copy(path, t.path)
 		path = append(path, m.Name)
 
+		// The key is the string that will be hashed to uniquely id the Source.
+		// The leading digit can be incremented to force re-fetch all existing
+		// modules.
+		key := fmt.Sprintf("0.root.%s-%s", strings.Join(path, "."), m.Source)
+
 		log.Printf("[TRACE] module source %q", m.Source)
 		// Split out the subdir if we have one.
-		// Terraform keeps the entire request tree for now, so that modules can
+		// Terraform keeps the entire requested tree for now, so that modules can
 		// reference sibling modules from the same archive or repo.
 		source, subDir := getter.SourceDirSubdir(m.Source)
 
+		// First check if we we need to download anything.
+		// This is also checked by the getter.Storage implementation, but we
+		// want to be able to short-circuit the detection as well, since some
+		// detectors may need to make external calls.
+		dir, found, err := s.Dir(key)
+		if err != nil {
+			return err
+		}
+
+		// looks like we already have it
+		// In order to load the Tree we need to find out if there was another
+		// subDir stored from discovery.
+		if found && mode != GetModeUpdate {
+			subDir, err := t.getSubdir(dir)
+			if err != nil {
+				// If there's a problem with the subdir record, we'll let the
+				// recordSubdir method fix it up.  Any other errors filesystem
+				// errors will turn up again below.
+				log.Println("[WARN] error reading subdir record:", err)
+			} else {
+				dir := filepath.Join(dir, subDir)
+				// Load the configurations.Dir(source)
+				children[m.Name], err = NewTreeModule(m.Name, dir)
+				if err != nil {
+					return fmt.Errorf("module %s: %s", m.Name, err)
+				}
+				// Set the path of this child
+				children[m.Name].path = path
+				continue
+			}
+		}
+
 		log.Printf("[TRACE] module source: %q", source)
 
-		source, err := getter.Detect(source, t.config.Dir, detectors)
+		source, err = getter.Detect(source, t.config.Dir, detectors)
 		if err != nil {
 			return fmt.Errorf("module %s: %s", m.Name, err)
 		}
@@ -208,13 +249,6 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error {
 
 		log.Printf("[TRACE] getting module source %q", source)
 
-		// Get the directory where this module is so we can load it
-		key := strings.Join(path, ".")
-
-		// The key is the string being hashed to uniquely id the Source.  The
-		// leading digit can be incremented to re-fetch all existing modules.
-		key = fmt.Sprintf("0.root.%s-%s", key, m.Source)
-
 		dir, ok, err := getStorage(s, key, source, mode)
 		if err != nil {
 			return err
@@ -224,19 +258,31 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error {
 				"module %s: not found, may need to be downloaded using 'terraform get'", m.Name)
 		}
 
-		// Expand the subDir if required.
+		// expand and record the subDir for later
-		dir, err = getter.SubdirGlob(dir, subDir)
+		if subDir != "" {
+			fullDir, err := getter.SubdirGlob(dir, subDir)
 			if err != nil {
 				return err
 			}
 
+			// +1 to account for the pathsep
+			if len(dir)+1 > len(fullDir) {
+				return fmt.Errorf("invalid module storage path %q", fullDir)
+			}
+
+			subDir = fullDir[len(dir)+1:]
+
+			if err := t.recordSubdir(dir, subDir); err != nil {
+				return err
+			}
+			dir = fullDir
+		}
+
 		// Load the configurations.Dir(source)
 		children[m.Name], err = NewTreeModule(m.Name, dir)
 		if err != nil {
-			return fmt.Errorf(
+			return fmt.Errorf("module %s: %s", m.Name, err)
-				"module %s: %s", m.Name, err)
 		}
-
 		// Set the path of this child
 		children[m.Name].path = path
 	}
@@ -254,6 +300,65 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error {
 	return nil
 }
 
+func subdirRecordsPath(dir string) string {
+	const filename = "module-subdir.json"
+	// Get the parent directory.
+	// The current FolderStorage implementation needed to be able to create
+	// this directory, so we can be reasonably certain we can use it.
+	parent := filepath.Dir(filepath.Clean(dir))
+	return filepath.Join(parent, filename)
+}
+
+// unmarshal the records file in the parent directory. Always returns a valid map.
+func loadSubdirRecords(dir string) (map[string]string, error) {
+	records := map[string]string{}
+
+	recordsPath := subdirRecordsPath(dir)
+	data, err := ioutil.ReadFile(recordsPath)
+	if err != nil && !os.IsNotExist(err) {
+		return records, err
+	}
+
+	if len(data) == 0 {
+		return records, nil
+	}
+
+	if err := json.Unmarshal(data, &records); err != nil {
+		return records, err
+	}
+	return records, nil
+}
+
+func (t *Tree) getSubdir(dir string) (string, error) {
+	records, err := loadSubdirRecords(dir)
+	if err != nil {
+		return "", err
+	}
+
+	return records[dir], nil
+}
+
+// Mark the location of a detected subdir in a top-level file so we
+// can skip detection when not updating the module.
+func (t *Tree) recordSubdir(dir, subdir string) error {
+	records, err := loadSubdirRecords(dir)
+	if err != nil {
+		// if there was a problem with the file, we will attempt to write a new
+		// one. Any non-data related error should surface there.
+		log.Printf("[WARN] error reading subdir records: %s", err)
+	}
+
+	records[dir] = subdir
+
+	js, err := json.Marshal(records)
+	if err != nil {
+		return err
+	}
+
+	recordsPath := subdirRecordsPath(dir)
+	return ioutil.WriteFile(recordsPath, js, 0644)
+}
+
 // Path is the full path to this tree.
 func (t *Tree) Path() []string {
 	return t.path

config/module/tree_test.go

@@ -2,7 +2,9 @@ package module
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -261,6 +263,71 @@ func TestTreeLoad_subdir(t *testing.T) {
 	}
 }
 
+func TestTree_recordSubDir(t *testing.T) {
+	td, err := ioutil.TempDir("", "tf-module")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(td)
+
+	dir := filepath.Join(td, "0131bf0fef686e090b16bdbab4910ddf")
+
+	subDir := "subDirName"
+
+	tree := Tree{}
+
+	// record and read the subdir path
+	if err := tree.recordSubdir(dir, subDir); err != nil {
+		t.Fatal(err)
+	}
+	actual, err := tree.getSubdir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if actual != subDir {
+		t.Fatalf("expected subDir %q, got %q", subDir, actual)
+	}
+
+	// overwrite the path, and nmake sure we get the new one
+	subDir = "newSubDir"
+	if err := tree.recordSubdir(dir, subDir); err != nil {
+		t.Fatal(err)
+	}
+	actual, err = tree.getSubdir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if actual != subDir {
+		t.Fatalf("expected subDir %q, got %q", subDir, actual)
+	}
+
+	// create a fake entry
+	if err := ioutil.WriteFile(subdirRecordsPath(dir), []byte("BAD DATA"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// this should fail because there aare now 2 entries
+	actual, err = tree.getSubdir(dir)
+	if err == nil {
+		t.Fatal("expected multiple subdir entries")
+	}
+
+	// writing the subdir entry should remove the incorrect value
+	if err := tree.recordSubdir(dir, subDir); err != nil {
+		t.Fatal(err)
+	}
+	actual, err = tree.getSubdir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if actual != subDir {
+		t.Fatalf("expected subDir %q, got %q", subDir, actual)
+	}
+}
+
 func TestTreeModules(t *testing.T) {
 	tree := NewTree("", testConfig(t, "basic"))
 	actual := tree.Modules()