From 36eb40a43263a87868c2fb817e8388b310f7bba5 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 11:29:29 -0400
Subject: [PATCH 1/8] export ModuleStorage and use it for Tree.Load

Exporting ModuleStorage allows us to explicitly pass in the storgae
location rather than extracting it out of the getter.Storage interface,
set a UI for communiating actions back to the user, and accepting a
services Disco for discovery.
---
 config/module/get_test.go      |  21 ++++---
 config/module/module_test.go   |   5 +-
 config/module/storage.go       |  78 +++++++++--------------
 config/module/testing.go       |   6 +-
 config/module/tree.go          |  10 ++-
 config/module/tree_gob_test.go |   3 +-
 config/module/tree_test.go     | 110 ++++++++++++++++++++++-----------
 7 files changed, 127 insertions(+), 106 deletions(-)

diff --git a/config/module/get_test.go b/config/module/get_test.go
index 01b61639d9..e01312b182 100644
--- a/config/module/get_test.go
+++ b/config/module/get_test.go
@@ -196,7 +196,8 @@ func TestRegistryGitHubArchive(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "registry-tar-subdir"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -204,7 +205,8 @@ func TestRegistryGitHubArchive(t *testing.T) {
 		t.Fatal("should be loaded")
 	}
 
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -212,7 +214,8 @@ func TestRegistryGitHubArchive(t *testing.T) {
 	server.Close()
 	tree = NewTree("", testConfig(t, "registry-tar-subdir"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -242,7 +245,8 @@ func TestRegisryModuleSubdir(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "registry-subdir"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -250,7 +254,8 @@ func TestRegisryModuleSubdir(t *testing.T) {
 		t.Fatal("should be loaded")
 	}
 
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -299,7 +304,8 @@ func TestAccRegistryLoad(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "registry-load"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -307,7 +313,8 @@ func TestAccRegistryLoad(t *testing.T) {
 		t.Fatal("should be loaded")
 	}
 
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
diff --git a/config/module/module_test.go b/config/module/module_test.go
index fae2f5b7da..cee27bc42a 100644
--- a/config/module/module_test.go
+++ b/config/module/module_test.go
@@ -7,7 +7,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/config"
 )
 
@@ -42,7 +41,7 @@ func testConfig(t *testing.T, n string) *config.Config {
 	return c
 }
 
-func testStorage(t *testing.T) getter.Storage {
+func testStorage(t *testing.T) *ModuleStorage {
 	t.Helper()
-	return &getter.FolderStorage{StorageDir: tempDir(t)}
+	return &ModuleStorage{StorageDir: tempDir(t)}
 }
diff --git a/config/module/storage.go b/config/module/storage.go
index eaee3746a7..f4cf2bd0f6 100644
--- a/config/module/storage.go
+++ b/config/module/storage.go
@@ -7,10 +7,11 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"reflect"
 
 	getter "github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/registry/regsrc"
+	"github.com/hashicorp/terraform/svchost/disco"
+	"github.com/mitchellh/cli"
 )
 
 const manifestName = "modules.json"
@@ -54,49 +55,22 @@ type moduleRecord struct {
 	registry bool
 }
 
-// moduleStorage 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.
-type moduleStorage struct {
-	getter.Storage
-	storageDir string
-	mode       GetMode
-}
-
-func newModuleStorage(s getter.Storage, mode GetMode) moduleStorage {
-	return moduleStorage{
-		Storage:    s,
-		storageDir: storageDir(s),
-		mode:       mode,
-	}
-}
-
-// The Tree needs to know where to store the module manifest.
-// Th Storage abstraction doesn't provide access to the storage root directory,
-// so we extract it here.
-func storageDir(s getter.Storage) string {
-	// get the StorageDir directly if possible
-	switch t := s.(type) {
-	case *getter.FolderStorage:
-		return t.StorageDir
-	case moduleStorage:
-		return t.storageDir
-	case nil:
-		return ""
-	}
-
-	// this should be our UI wrapper which is exported here, so we need to
-	// extract the FolderStorage via reflection.
-	fs := reflect.ValueOf(s).Elem().FieldByName("Storage").Interface()
-	return storageDir(fs.(getter.Storage))
+type ModuleStorage struct {
+	StorageDir string
+	Services   *disco.Disco
+	Ui         cli.Ui
+	Mode       GetMode
 }
 
 // loadManifest returns the moduleManifest file from the parent directory.
-func (m moduleStorage) loadManifest() (moduleManifest, error) {
+func (m ModuleStorage) loadManifest() (moduleManifest, error) {
 	manifest := moduleManifest{}
 
-	manifestPath := filepath.Join(m.storageDir, manifestName)
+	manifestPath := filepath.Join(m.StorageDir, manifestName)
 	data, err := ioutil.ReadFile(manifestPath)
 	if err != nil && !os.IsNotExist(err) {
 		return manifest, err
@@ -116,7 +90,7 @@ func (m moduleStorage) loadManifest() (moduleManifest, error) {
 // root directory. The storage method loads the entire file and rewrites it
 // each time. This is only done a few times during init, so efficiency is
 // not a concern.
-func (m moduleStorage) recordModule(rec moduleRecord) error {
+func (m ModuleStorage) recordModule(rec moduleRecord) error {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		// if there was a problem with the file, we will attempt to write a new
@@ -146,14 +120,14 @@ func (m moduleStorage) recordModule(rec moduleRecord) error {
 		panic(err)
 	}
 
-	manifestPath := filepath.Join(m.storageDir, manifestName)
+	manifestPath := filepath.Join(m.StorageDir, manifestName)
 	return ioutil.WriteFile(manifestPath, js, 0644)
 }
 
 // load the manifest from dir, and return all module versions matching the
 // provided source. Records with no version info will be skipped, as they need
 // to be uniquely identified by other means.
-func (m moduleStorage) moduleVersions(source string) ([]moduleRecord, error) {
+func (m ModuleStorage) moduleVersions(source string) ([]moduleRecord, error) {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		return manifest.Modules, err
@@ -170,7 +144,7 @@ func (m moduleStorage) moduleVersions(source string) ([]moduleRecord, error) {
 	return matching, nil
 }
 
-func (m moduleStorage) moduleDir(key string) (string, error) {
+func (m ModuleStorage) moduleDir(key string) (string, error) {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		return "", err
@@ -186,7 +160,7 @@ func (m moduleStorage) moduleDir(key string) (string, error) {
 }
 
 // return only the root directory of the module stored in dir.
-func (m moduleStorage) getModuleRoot(dir string) (string, error) {
+func (m ModuleStorage) getModuleRoot(dir string) (string, error) {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		return "", err
@@ -202,7 +176,7 @@ func (m moduleStorage) getModuleRoot(dir string) (string, error) {
 
 // record only the Root directory for the module stored at dir.
 // TODO: remove this compatibility function to store the full moduleRecord.
-func (m moduleStorage) recordModuleRoot(dir, root string) error {
+func (m ModuleStorage) recordModuleRoot(dir, root string) error {
 	rec := moduleRecord{
 		Dir:  dir,
 		Root: root,
@@ -211,24 +185,28 @@ func (m moduleStorage) recordModuleRoot(dir, root string) error {
 	return m.recordModule(rec)
 }
 
-func (m moduleStorage) getStorage(key string, src string) (string, bool, error) {
+func (m ModuleStorage) getStorage(key string, src string) (string, bool, error) {
+	storage := &getter.FolderStorage{
+		StorageDir: m.StorageDir,
+	}
+
 	// Get the module with the level specified if we were told to.
-	if m.mode > GetModeNone {
+	if m.Mode > GetModeNone {
 		log.Printf("[DEBUG] fetching %q with key %q", src, key)
-		if err := m.Storage.Get(key, src, m.mode == GetModeUpdate); err != nil {
+		if err := storage.Get(key, src, m.Mode == GetModeUpdate); err != nil {
 			return "", false, err
 		}
 	}
 
 	// Get the directory where the module is.
-	dir, found, err := m.Storage.Dir(key)
+	dir, found, err := storage.Dir(key)
 	log.Printf("[DEBUG] found %q in %q: %t", src, dir, found)
 	return dir, found, err
 }
 
 // find a stored module that's not from a registry
-func (m moduleStorage) findModule(key string) (string, error) {
-	if m.mode == GetModeUpdate {
+func (m ModuleStorage) findModule(key string) (string, error) {
+	if m.Mode == GetModeUpdate {
 		return "", nil
 	}
 
@@ -236,7 +214,7 @@ func (m moduleStorage) findModule(key string) (string, error) {
 }
 
 // find a registry module
-func (m moduleStorage) findRegistryModule(mSource, constraint string) (moduleRecord, error) {
+func (m ModuleStorage) findRegistryModule(mSource, constraint string) (moduleRecord, error) {
 	rec := moduleRecord{
 		Source: mSource,
 	}
@@ -272,7 +250,7 @@ func (m moduleStorage) findRegistryModule(mSource, constraint string) (moduleRec
 
 	// we need to lookup available versions
 	// Only on Get if it's not found, on unconditionally on Update
-	if (m.mode == GetModeGet && !found) || (m.mode == GetModeUpdate) {
+	if (m.Mode == GetModeGet && !found) || (m.Mode == GetModeUpdate) {
 		resp, err := lookupModuleVersions(nil, mod)
 		if err != nil {
 			return rec, err
diff --git a/config/module/testing.go b/config/module/testing.go
index fc9e7331af..0a78e66d07 100644
--- a/config/module/testing.go
+++ b/config/module/testing.go
@@ -4,8 +4,6 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
-
-	"github.com/hashicorp/go-getter"
 )
 
 // TestTree loads a module at the given path and returns the tree as well
@@ -26,8 +24,8 @@ func TestTree(t *testing.T, path string) (*Tree, func()) {
 	}
 
 	// Get the child modules
-	s := &getter.FolderStorage{StorageDir: dir}
-	if err := mod.Load(s, GetModeGet); err != nil {
+	s := &ModuleStorage{StorageDir: dir, Mode: GetModeGet}
+	if err := mod.Load(s); err != nil {
 		t.Fatalf("err: %s", err)
 		return nil, nil
 	}
diff --git a/config/module/tree.go b/config/module/tree.go
index 24b40cba18..484c62189a 100644
--- a/config/module/tree.go
+++ b/config/module/tree.go
@@ -52,7 +52,7 @@ func NewEmptyTree() *Tree {
 	// We do this dummy load so that the tree is marked as "loaded". It
 	// should never fail because this is just about a no-op. If it does fail
 	// we panic so we can know its a bug.
-	if err := t.Load(nil, GetModeGet); err != nil {
+	if err := t.Load(&ModuleStorage{Mode: GetModeGet}); err != nil {
 		panic(err)
 	}
 
@@ -169,12 +169,10 @@ func (t *Tree) Name() string {
 // module trees inherently require the configuration to be in a reasonably
 // sane state: no circular dependencies, proper module sources, etc. A full
 // suite of validations can be done by running Validate (after loading).
-func (t *Tree) Load(storage getter.Storage, mode GetMode) error {
+func (t *Tree) Load(s *ModuleStorage) error {
 	t.lock.Lock()
 	defer t.lock.Unlock()
 
-	s := newModuleStorage(storage, mode)
-
 	children, err := t.getChildren(s)
 	if err != nil {
 		return err
@@ -182,7 +180,7 @@ func (t *Tree) Load(storage getter.Storage, mode GetMode) error {
 
 	// Go through all the children and load them.
 	for _, c := range children {
-		if err := c.Load(storage, mode); err != nil {
+		if err := c.Load(s); err != nil {
 			return err
 		}
 	}
@@ -198,7 +196,7 @@ func (t *Tree) Load(storage getter.Storage, mode GetMode) error {
 	return nil
 }
 
-func (t *Tree) getChildren(s moduleStorage) (map[string]*Tree, error) {
+func (t *Tree) getChildren(s *ModuleStorage) (map[string]*Tree, error) {
 	children := make(map[string]*Tree)
 
 	// Go through all the modules and get the directory for them.
diff --git a/config/module/tree_gob_test.go b/config/module/tree_gob_test.go
index cee17b42a6..ad0ab4ecdd 100644
--- a/config/module/tree_gob_test.go
+++ b/config/module/tree_gob_test.go
@@ -12,7 +12,8 @@ func TestTreeEncodeDecodeGob(t *testing.T) {
 	tree := NewTree("", testConfig(t, "basic"))
 
 	// This should get things
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
diff --git a/config/module/tree_test.go b/config/module/tree_test.go
index 26a1729775..9470203dcd 100644
--- a/config/module/tree_test.go
+++ b/config/module/tree_test.go
@@ -10,7 +10,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/helper/copy"
 )
@@ -22,8 +21,9 @@ func TestTreeChild(t *testing.T) {
 	}
 
 	storage := testStorage(t)
+	storage.Mode = GetModeGet
 	tree := NewTree("", testConfig(t, "child"))
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -73,7 +73,7 @@ func TestTreeLoad(t *testing.T) {
 	}
 
 	// This should error because we haven't gotten things yet
-	if err := tree.Load(storage, GetModeNone); err == nil {
+	if err := tree.Load(storage); err == nil {
 		t.Fatal("should error")
 	}
 
@@ -82,7 +82,8 @@ func TestTreeLoad(t *testing.T) {
 	}
 
 	// This should get things
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -91,7 +92,8 @@ func TestTreeLoad(t *testing.T) {
 	}
 
 	// This should no longer error
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -111,19 +113,23 @@ func TestTreeLoad_duplicate(t *testing.T) {
 	}
 
 	// This should get things
-	if err := tree.Load(storage, GetModeGet); err == nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err == nil {
 		t.Fatalf("should error")
 	}
 }
 
 func TestTreeLoad_copyable(t *testing.T) {
 	dir := tempDir(t)
-	storage := &getter.FolderStorage{StorageDir: dir}
+	storage := &ModuleStorage{
+		StorageDir: dir,
+		Mode:       GetModeGet,
+	}
 	cfg := testConfig(t, "basic")
 	tree := NewTree("", cfg)
 
 	// This should get things
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -132,7 +138,8 @@ func TestTreeLoad_copyable(t *testing.T) {
 	}
 
 	// This should no longer error
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -160,10 +167,13 @@ func TestTreeLoad_copyable(t *testing.T) {
 		}
 
 		tree := NewTree("", cfg)
-		storage := &getter.FolderStorage{StorageDir: dir2}
+		storage := &ModuleStorage{
+			StorageDir: dir2,
+			Mode:       GetModeNone,
+		}
 
 		// This should not error since we already got it!
-		if err := tree.Load(storage, GetModeNone); err != nil {
+		if err := tree.Load(storage); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 
@@ -182,7 +192,8 @@ func TestTreeLoad_parentRef(t *testing.T) {
 	}
 
 	// This should error because we haven't gotten things yet
-	if err := tree.Load(storage, GetModeNone); err == nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err == nil {
 		t.Fatal("should error")
 	}
 
@@ -191,7 +202,8 @@ func TestTreeLoad_parentRef(t *testing.T) {
 	}
 
 	// This should get things
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -200,7 +212,8 @@ func TestTreeLoad_parentRef(t *testing.T) {
 	}
 
 	// This should no longer error
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -233,7 +246,8 @@ func TestTreeLoad_subdir(t *testing.T) {
 			}
 
 			// This should error because we haven't gotten things yet
-			if err := tree.Load(storage, GetModeNone); err == nil {
+			storage.Mode = GetModeNone
+			if err := tree.Load(storage); err == nil {
 				t.Fatal("should error")
 			}
 
@@ -242,7 +256,8 @@ func TestTreeLoad_subdir(t *testing.T) {
 			}
 
 			// This should get things
-			if err := tree.Load(storage, GetModeGet); err != nil {
+			storage.Mode = GetModeGet
+			if err := tree.Load(storage); err != nil {
 				t.Fatalf("err: %s", err)
 			}
 
@@ -251,7 +266,8 @@ func TestTreeLoad_subdir(t *testing.T) {
 			}
 
 			// This should no longer error
-			if err := tree.Load(storage, GetModeNone); err != nil {
+			storage.Mode = GetModeNone
+			if err := tree.Load(storage); err != nil {
 				t.Fatalf("err: %s", err)
 			}
 
@@ -271,7 +287,7 @@ func TestTree_recordManifest(t *testing.T) {
 	}
 	defer os.RemoveAll(td)
 
-	storage := moduleStorage{storageDir: td}
+	storage := ModuleStorage{StorageDir: td}
 
 	dir := filepath.Join(td, "0131bf0fef686e090b16bdbab4910ddf")
 
@@ -388,7 +404,9 @@ func TestTreeValidate_table(t *testing.T) {
 	for i, tc := range cases {
 		t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) {
 			tree := NewTree("", testConfig(t, tc.Fixture))
-			if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+			storage := testStorage(t)
+			storage.Mode = GetModeGet
+			if err := tree.Load(storage); err != nil {
 				t.Fatalf("err: %s", err)
 			}
 
@@ -409,7 +427,9 @@ func TestTreeValidate_table(t *testing.T) {
 func TestTreeValidate_badChild(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-child-bad"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -421,7 +441,9 @@ func TestTreeValidate_badChild(t *testing.T) {
 func TestTreeValidate_badChildOutput(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-bad-output"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -433,7 +455,9 @@ func TestTreeValidate_badChildOutput(t *testing.T) {
 func TestTreeValidate_badChildOutputToModule(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-bad-output-to-module"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -445,7 +469,9 @@ func TestTreeValidate_badChildOutputToModule(t *testing.T) {
 func TestTreeValidate_badChildVar(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-bad-var"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -457,7 +483,9 @@ func TestTreeValidate_badChildVar(t *testing.T) {
 func TestTreeValidate_badRoot(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-root-bad"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -469,7 +497,9 @@ func TestTreeValidate_badRoot(t *testing.T) {
 func TestTreeValidate_good(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-child-good"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -489,7 +519,9 @@ func TestTreeValidate_notLoaded(t *testing.T) {
 func TestTreeValidate_requiredChildVar(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-required-var"))
 
-	if err := tree.Load(testStorage(t), GetModeGet); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -510,7 +542,9 @@ func TestTreeValidate_requiredChildVar(t *testing.T) {
 func TestTreeValidate_unknownModule(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-module-unknown"))
 
-	if err := tree.Load(testStorage(t), GetModeNone); err != nil {
+	storage := testStorage(t)
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -523,7 +557,8 @@ func TestTreeProviders_basic(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "basic-parent-providers"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -585,7 +620,8 @@ func TestTreeProviders_implicit(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "implicit-parent-providers"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -625,7 +661,8 @@ func TestTreeProviders_implicitMultiLevel(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "implicit-grandparent-providers"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -694,7 +731,8 @@ func TestTreeLoad_conflictingSubmoduleNames(t *testing.T) {
 	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "conficting-submodule-names"))
 
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("load failed: %s", err)
 	}
 
@@ -703,7 +741,8 @@ func TestTreeLoad_conflictingSubmoduleNames(t *testing.T) {
 	}
 
 	// Try to reload
-	if err := tree.Load(storage, GetModeNone); err != nil {
+	storage.Mode = GetModeNone
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("reload failed: %s", err)
 	}
 
@@ -753,13 +792,14 @@ func TestTreeLoad_changeIntermediateSource(t *testing.T) {
 	if err := os.MkdirAll(".terraform/modules", 0777); err != nil {
 		t.Fatal(err)
 	}
-	storage := &getter.FolderStorage{StorageDir: ".terraform/modules"}
+	storage := &ModuleStorage{StorageDir: ".terraform/modules"}
 	cfg, err := config.LoadDir("./")
 	if err != nil {
 		t.Fatal(err)
 	}
 	tree := NewTree("", cfg)
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	storage.Mode = GetModeGet
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("load failed: %s", err)
 	}
 
@@ -774,7 +814,7 @@ func TestTreeLoad_changeIntermediateSource(t *testing.T) {
 		t.Fatal(err)
 	}
 	tree = NewTree("", cfg)
-	if err := tree.Load(storage, GetModeGet); err != nil {
+	if err := tree.Load(storage); err != nil {
 		t.Fatalf("load failed: %s", err)
 	}
 

From f2a7b94692eb03358a129d094c8dcaf4ee576a4a Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 12:58:24 -0400
Subject: [PATCH 2/8] use the new ModuleStorage in the command package

Update the command package to use the new module storage. Move the old
command output strings into the module storage itself. This could be
moved back later either by using ui callbacks, or designing a module
storage interface once we know what the final requirements will look
like.
---
 command/command_test.go        |  8 +++++---
 command/get.go                 |  2 +-
 command/meta.go                | 14 +++++++-------
 command/meta_new.go            |  2 +-
 command/module_storage.go      | 29 -----------------------------
 command/module_storage_test.go | 11 -----------
 config/module/storage.go       |  8 ++++++++
 helper/resource/testing.go     |  6 +++---
 8 files changed, 25 insertions(+), 55 deletions(-)
 delete mode 100644 command/module_storage.go
 delete mode 100644 command/module_storage_test.go

diff --git a/command/command_test.go b/command/command_test.go
index 598805a2cf..e867ae45d1 100644
--- a/command/command_test.go
+++ b/command/command_test.go
@@ -19,7 +19,6 @@ import (
 	"syscall"
 	"testing"
 
-	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/helper/logging"
 	"github.com/hashicorp/terraform/terraform"
@@ -108,8 +107,11 @@ func testModule(t *testing.T, name string) *module.Tree {
 		t.Fatalf("err: %s", err)
 	}
 
-	s := &getter.FolderStorage{StorageDir: tempDir(t)}
-	if err := mod.Load(s, module.GetModeGet); err != nil {
+	s := &module.ModuleStorage{
+		StorageDir: tempDir(t),
+		Mode:       module.GetModeGet,
+	}
+	if err := mod.Load(s); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
diff --git a/command/get.go b/command/get.go
index d51ae39b31..ba8729d79a 100644
--- a/command/get.go
+++ b/command/get.go
@@ -81,7 +81,7 @@ func getModules(m *Meta, path string, mode module.GetMode) error {
 		return fmt.Errorf("Error loading configuration: %s", err)
 	}
 
-	err = mod.Load(m.moduleStorage(m.DataDir()), mode)
+	err = mod.Load(m.moduleStorage(m.DataDir(), mode))
 	if err != nil {
 		return fmt.Errorf("Error loading modules: %s", err)
 	}
diff --git a/command/meta.go b/command/meta.go
index bb58ab6541..24493116e1 100644
--- a/command/meta.go
+++ b/command/meta.go
@@ -16,11 +16,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/backend"
 	"github.com/hashicorp/terraform/backend/local"
 	"github.com/hashicorp/terraform/command/format"
 	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/helper/experiment"
 	"github.com/hashicorp/terraform/helper/variables"
 	"github.com/hashicorp/terraform/helper/wrappedstreams"
@@ -368,12 +368,12 @@ func (m *Meta) flagSet(n string) *flag.FlagSet {
 
 // moduleStorage returns the module.Storage implementation used to store
 // modules for commands.
-func (m *Meta) moduleStorage(root string) getter.Storage {
-	return &uiModuleStorage{
-		Storage: &getter.FolderStorage{
-			StorageDir: filepath.Join(root, "modules"),
-		},
-		Ui: m.Ui,
+func (m *Meta) moduleStorage(root string, mode module.GetMode) *module.ModuleStorage {
+	return &module.ModuleStorage{
+		StorageDir: filepath.Join(root, "modules"),
+		Services:   m.Services,
+		Ui:         m.Ui,
+		Mode:       mode,
 	}
 }
 
diff --git a/command/meta_new.go b/command/meta_new.go
index 9447caf3b8..5fc3cdca36 100644
--- a/command/meta_new.go
+++ b/command/meta_new.go
@@ -45,7 +45,7 @@ func (m *Meta) Module(path string) (*module.Tree, error) {
 		return nil, err
 	}
 
-	err = mod.Load(m.moduleStorage(m.DataDir()), module.GetModeNone)
+	err = mod.Load(m.moduleStorage(m.DataDir(), module.GetModeNone))
 	if err != nil {
 		return nil, errwrap.Wrapf("Error loading modules: {{err}}", err)
 	}
diff --git a/command/module_storage.go b/command/module_storage.go
deleted file mode 100644
index 5bb832897d..0000000000
--- a/command/module_storage.go
+++ /dev/null
@@ -1,29 +0,0 @@
-package command
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/go-getter"
-	"github.com/mitchellh/cli"
-)
-
-// uiModuleStorage implements module.Storage and is just a proxy to output
-// to the UI any Get operations.
-type uiModuleStorage struct {
-	Storage getter.Storage
-	Ui      cli.Ui
-}
-
-func (s *uiModuleStorage) Dir(key string) (string, bool, error) {
-	return s.Storage.Dir(key)
-}
-
-func (s *uiModuleStorage) Get(key string, source string, update bool) error {
-	updateStr := ""
-	if update {
-		updateStr = " (update)"
-	}
-
-	s.Ui.Output(fmt.Sprintf("Get: %s%s", source, updateStr))
-	return s.Storage.Get(key, source, update)
-}
diff --git a/command/module_storage_test.go b/command/module_storage_test.go
deleted file mode 100644
index 97a5ed7ae3..0000000000
--- a/command/module_storage_test.go
+++ /dev/null
@@ -1,11 +0,0 @@
-package command
-
-import (
-	"testing"
-
-	"github.com/hashicorp/go-getter"
-)
-
-func TestUiModuleStorage_impl(t *testing.T) {
-	var _ getter.Storage = new(uiModuleStorage)
-}
diff --git a/config/module/storage.go b/config/module/storage.go
index f4cf2bd0f6..9280ce9ae0 100644
--- a/config/module/storage.go
+++ b/config/module/storage.go
@@ -190,6 +190,14 @@ func (m ModuleStorage) getStorage(key string, src string) (string, bool, error)
 		StorageDir: m.StorageDir,
 	}
 
+	if m.Ui != nil {
+		update := ""
+		if m.Mode == GetModeUpdate {
+			update = " (update)"
+		}
+		m.Ui.Output(fmt.Sprintf("Get: %s%s", src, update))
+	}
+
 	// Get the module with the level specified if we were told to.
 	if m.Mode > GetModeNone {
 		log.Printf("[DEBUG] fetching %q with key %q", src, key)
diff --git a/helper/resource/testing.go b/helper/resource/testing.go
index aa0aa51fba..20fc0145bf 100644
--- a/helper/resource/testing.go
+++ b/helper/resource/testing.go
@@ -15,7 +15,6 @@ import (
 	"testing"
 
 	"github.com/davecgh/go-spew/spew"
-	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/logutils"
 	"github.com/hashicorp/terraform/config/module"
@@ -751,10 +750,11 @@ func testModule(
 	}
 
 	// Load the modules
-	modStorage := &getter.FolderStorage{
+	modStorage := &module.ModuleStorage{
 		StorageDir: filepath.Join(cfgPath, ".tfmodules"),
+		Mode:       module.GetModeGet,
 	}
-	err = mod.Load(modStorage, module.GetModeGet)
+	err = mod.Load(modStorage)
 	if err != nil {
 		return nil, fmt.Errorf("Error downloading modules: %s", err)
 	}

From 70a5b1b7343d90a118b1f918846258290fb5e3d0 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 13:06:07 -0400
Subject: [PATCH 3/8] make terraform work with ModuleStorage

---
 terraform/terraform_test.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go
index 3be9dea163..56af0ff3a9 100644
--- a/terraform/terraform_test.go
+++ b/terraform/terraform_test.go
@@ -12,7 +12,6 @@ import (
 	"sync"
 	"testing"
 
-	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/helper/experiment"
@@ -97,8 +96,11 @@ func testModule(t *testing.T, name string) *module.Tree {
 		t.Fatalf("err: %s", err)
 	}
 
-	s := &getter.FolderStorage{StorageDir: tempDir(t)}
-	if err := mod.Load(s, module.GetModeGet); err != nil {
+	s := &module.ModuleStorage{
+		StorageDir: tempDir(t),
+		Mode:       module.GetModeGet,
+	}
+	if err := mod.Load(s); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -144,10 +146,11 @@ func testModuleInline(t *testing.T, config map[string]string) *module.Tree {
 	}
 
 	// Load the modules
-	modStorage := &getter.FolderStorage{
+	modStorage := &module.ModuleStorage{
 		StorageDir: filepath.Join(cfgPath, ".tfmodules"),
+		Mode:       module.GetModeGet,
 	}
-	err = mod.Load(modStorage, module.GetModeGet)
+	err = mod.Load(modStorage)
 	if err != nil {
 		t.Errorf("Error downloading modules: %s", err)
 	}

From 3a495ffe560301e9e9aa167e069eee4055d414cf Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 13:06:50 -0400
Subject: [PATCH 4/8] rename ModuleStorage to Storage

get rid of stutter and use module.Storage
---
 command/command_test.go      |  2 +-
 command/meta.go              |  4 ++--
 config/module/module_test.go |  4 ++--
 config/module/storage.go     | 32 ++++++++++++++++++--------------
 config/module/testing.go     |  2 +-
 config/module/tree.go        |  6 +++---
 config/module/tree_test.go   |  8 ++++----
 helper/resource/testing.go   |  2 +-
 terraform/terraform_test.go  |  4 ++--
 9 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/command/command_test.go b/command/command_test.go
index e867ae45d1..a789eebabd 100644
--- a/command/command_test.go
+++ b/command/command_test.go
@@ -107,7 +107,7 @@ func testModule(t *testing.T, name string) *module.Tree {
 		t.Fatalf("err: %s", err)
 	}
 
-	s := &module.ModuleStorage{
+	s := &module.Storage{
 		StorageDir: tempDir(t),
 		Mode:       module.GetModeGet,
 	}
diff --git a/command/meta.go b/command/meta.go
index 24493116e1..b00c80d48f 100644
--- a/command/meta.go
+++ b/command/meta.go
@@ -368,8 +368,8 @@ func (m *Meta) flagSet(n string) *flag.FlagSet {
 
 // moduleStorage returns the module.Storage implementation used to store
 // modules for commands.
-func (m *Meta) moduleStorage(root string, mode module.GetMode) *module.ModuleStorage {
-	return &module.ModuleStorage{
+func (m *Meta) moduleStorage(root string, mode module.GetMode) *module.Storage {
+	return &module.Storage{
 		StorageDir: filepath.Join(root, "modules"),
 		Services:   m.Services,
 		Ui:         m.Ui,
diff --git a/config/module/module_test.go b/config/module/module_test.go
index cee27bc42a..13614736c3 100644
--- a/config/module/module_test.go
+++ b/config/module/module_test.go
@@ -41,7 +41,7 @@ func testConfig(t *testing.T, n string) *config.Config {
 	return c
 }
 
-func testStorage(t *testing.T) *ModuleStorage {
+func testStorage(t *testing.T) *Storage {
 	t.Helper()
-	return &ModuleStorage{StorageDir: tempDir(t)}
+	return &Storage{StorageDir: tempDir(t)}
 }
diff --git a/config/module/storage.go b/config/module/storage.go
index 9280ce9ae0..6bec91be9b 100644
--- a/config/module/storage.go
+++ b/config/module/storage.go
@@ -55,19 +55,23 @@ type moduleRecord struct {
 	registry bool
 }
 
-// ModuleStorage implements methods to record and fetch metadata about the
+// Storage 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.
-type ModuleStorage struct {
+type Storage struct {
+	// StorageDir is the directory where all modules will be stored.
 	StorageDir string
-	Services   *disco.Disco
-	Ui         cli.Ui
-	Mode       GetMode
+	// Services is a *Disco which ay have services and credentials pre-loaded.
+	Services *disco.Disco
+	// Ui is an optional cli.Ui for user output
+	Ui cli.Ui
+	// Mode is the GetMode that will be used for various operations.
+	Mode GetMode
 }
 
 // loadManifest returns the moduleManifest file from the parent directory.
-func (m ModuleStorage) loadManifest() (moduleManifest, error) {
+func (m Storage) loadManifest() (moduleManifest, error) {
 	manifest := moduleManifest{}
 
 	manifestPath := filepath.Join(m.StorageDir, manifestName)
@@ -90,7 +94,7 @@ func (m ModuleStorage) loadManifest() (moduleManifest, error) {
 // root directory. The storage method loads the entire file and rewrites it
 // each time. This is only done a few times during init, so efficiency is
 // not a concern.
-func (m ModuleStorage) recordModule(rec moduleRecord) error {
+func (m Storage) recordModule(rec moduleRecord) error {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		// if there was a problem with the file, we will attempt to write a new
@@ -127,7 +131,7 @@ func (m ModuleStorage) recordModule(rec moduleRecord) error {
 // load the manifest from dir, and return all module versions matching the
 // provided source. Records with no version info will be skipped, as they need
 // to be uniquely identified by other means.
-func (m ModuleStorage) moduleVersions(source string) ([]moduleRecord, error) {
+func (m Storage) moduleVersions(source string) ([]moduleRecord, error) {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		return manifest.Modules, err
@@ -144,7 +148,7 @@ func (m ModuleStorage) moduleVersions(source string) ([]moduleRecord, error) {
 	return matching, nil
 }
 
-func (m ModuleStorage) moduleDir(key string) (string, error) {
+func (m Storage) moduleDir(key string) (string, error) {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		return "", err
@@ -160,7 +164,7 @@ func (m ModuleStorage) moduleDir(key string) (string, error) {
 }
 
 // return only the root directory of the module stored in dir.
-func (m ModuleStorage) getModuleRoot(dir string) (string, error) {
+func (m Storage) getModuleRoot(dir string) (string, error) {
 	manifest, err := m.loadManifest()
 	if err != nil {
 		return "", err
@@ -176,7 +180,7 @@ func (m ModuleStorage) getModuleRoot(dir string) (string, error) {
 
 // record only the Root directory for the module stored at dir.
 // TODO: remove this compatibility function to store the full moduleRecord.
-func (m ModuleStorage) recordModuleRoot(dir, root string) error {
+func (m Storage) recordModuleRoot(dir, root string) error {
 	rec := moduleRecord{
 		Dir:  dir,
 		Root: root,
@@ -185,7 +189,7 @@ func (m ModuleStorage) recordModuleRoot(dir, root string) error {
 	return m.recordModule(rec)
 }
 
-func (m ModuleStorage) getStorage(key string, src string) (string, bool, error) {
+func (m Storage) getStorage(key string, src string) (string, bool, error) {
 	storage := &getter.FolderStorage{
 		StorageDir: m.StorageDir,
 	}
@@ -213,7 +217,7 @@ func (m ModuleStorage) getStorage(key string, src string) (string, bool, error)
 }
 
 // find a stored module that's not from a registry
-func (m ModuleStorage) findModule(key string) (string, error) {
+func (m Storage) findModule(key string) (string, error) {
 	if m.Mode == GetModeUpdate {
 		return "", nil
 	}
@@ -222,7 +226,7 @@ func (m ModuleStorage) findModule(key string) (string, error) {
 }
 
 // find a registry module
-func (m ModuleStorage) findRegistryModule(mSource, constraint string) (moduleRecord, error) {
+func (m Storage) findRegistryModule(mSource, constraint string) (moduleRecord, error) {
 	rec := moduleRecord{
 		Source: mSource,
 	}
diff --git a/config/module/testing.go b/config/module/testing.go
index 0a78e66d07..6f1ff050dc 100644
--- a/config/module/testing.go
+++ b/config/module/testing.go
@@ -24,7 +24,7 @@ func TestTree(t *testing.T, path string) (*Tree, func()) {
 	}
 
 	// Get the child modules
-	s := &ModuleStorage{StorageDir: dir, Mode: GetModeGet}
+	s := &Storage{StorageDir: dir, Mode: GetModeGet}
 	if err := mod.Load(s); err != nil {
 		t.Fatalf("err: %s", err)
 		return nil, nil
diff --git a/config/module/tree.go b/config/module/tree.go
index 484c62189a..1ed03d07c5 100644
--- a/config/module/tree.go
+++ b/config/module/tree.go
@@ -52,7 +52,7 @@ func NewEmptyTree() *Tree {
 	// We do this dummy load so that the tree is marked as "loaded". It
 	// should never fail because this is just about a no-op. If it does fail
 	// we panic so we can know its a bug.
-	if err := t.Load(&ModuleStorage{Mode: GetModeGet}); err != nil {
+	if err := t.Load(&Storage{Mode: GetModeGet}); err != nil {
 		panic(err)
 	}
 
@@ -169,7 +169,7 @@ func (t *Tree) Name() string {
 // module trees inherently require the configuration to be in a reasonably
 // sane state: no circular dependencies, proper module sources, etc. A full
 // suite of validations can be done by running Validate (after loading).
-func (t *Tree) Load(s *ModuleStorage) error {
+func (t *Tree) Load(s *Storage) error {
 	t.lock.Lock()
 	defer t.lock.Unlock()
 
@@ -196,7 +196,7 @@ func (t *Tree) Load(s *ModuleStorage) error {
 	return nil
 }
 
-func (t *Tree) getChildren(s *ModuleStorage) (map[string]*Tree, error) {
+func (t *Tree) getChildren(s *Storage) (map[string]*Tree, error) {
 	children := make(map[string]*Tree)
 
 	// Go through all the modules and get the directory for them.
diff --git a/config/module/tree_test.go b/config/module/tree_test.go
index 9470203dcd..780023b7df 100644
--- a/config/module/tree_test.go
+++ b/config/module/tree_test.go
@@ -121,7 +121,7 @@ func TestTreeLoad_duplicate(t *testing.T) {
 
 func TestTreeLoad_copyable(t *testing.T) {
 	dir := tempDir(t)
-	storage := &ModuleStorage{
+	storage := &Storage{
 		StorageDir: dir,
 		Mode:       GetModeGet,
 	}
@@ -167,7 +167,7 @@ func TestTreeLoad_copyable(t *testing.T) {
 		}
 
 		tree := NewTree("", cfg)
-		storage := &ModuleStorage{
+		storage := &Storage{
 			StorageDir: dir2,
 			Mode:       GetModeNone,
 		}
@@ -287,7 +287,7 @@ func TestTree_recordManifest(t *testing.T) {
 	}
 	defer os.RemoveAll(td)
 
-	storage := ModuleStorage{StorageDir: td}
+	storage := Storage{StorageDir: td}
 
 	dir := filepath.Join(td, "0131bf0fef686e090b16bdbab4910ddf")
 
@@ -792,7 +792,7 @@ func TestTreeLoad_changeIntermediateSource(t *testing.T) {
 	if err := os.MkdirAll(".terraform/modules", 0777); err != nil {
 		t.Fatal(err)
 	}
-	storage := &ModuleStorage{StorageDir: ".terraform/modules"}
+	storage := &Storage{StorageDir: ".terraform/modules"}
 	cfg, err := config.LoadDir("./")
 	if err != nil {
 		t.Fatal(err)
diff --git a/helper/resource/testing.go b/helper/resource/testing.go
index 20fc0145bf..9766ff57a8 100644
--- a/helper/resource/testing.go
+++ b/helper/resource/testing.go
@@ -750,7 +750,7 @@ func testModule(
 	}
 
 	// Load the modules
-	modStorage := &module.ModuleStorage{
+	modStorage := &module.Storage{
 		StorageDir: filepath.Join(cfgPath, ".tfmodules"),
 		Mode:       module.GetModeGet,
 	}
diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go
index 56af0ff3a9..15e920796b 100644
--- a/terraform/terraform_test.go
+++ b/terraform/terraform_test.go
@@ -96,7 +96,7 @@ func testModule(t *testing.T, name string) *module.Tree {
 		t.Fatalf("err: %s", err)
 	}
 
-	s := &module.ModuleStorage{
+	s := &module.Storage{
 		StorageDir: tempDir(t),
 		Mode:       module.GetModeGet,
 	}
@@ -146,7 +146,7 @@ func testModuleInline(t *testing.T, config map[string]string) *module.Tree {
 	}
 
 	// Load the modules
-	modStorage := &module.ModuleStorage{
+	modStorage := &module.Storage{
 		StorageDir: filepath.Join(cfgPath, ".tfmodules"),
 		Mode:       module.GetModeGet,
 	}

From 4e8fe97556d56a669247e7c502d516789cec757f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 15:07:24 -0400
Subject: [PATCH 5/8] add credentials to module.Storage

Provide a way to pass in credentials to be used by the module.Storage
when contacting registries.

Remove the mockTLSServer and use a static discovery map pointing to the
http url for tests.
---
 config/module/get_test.go      | 36 ++++----------
 config/module/module_test.go   | 23 ++++++++-
 config/module/registry.go      | 73 +++++++++++++---------------
 config/module/registry_test.go | 70 ++++----------------------
 config/module/storage.go       | 89 ++++++++++++++++++++--------------
 config/module/tree_gob_test.go |  2 +-
 config/module/tree_test.go     | 41 +++++++---------
 7 files changed, 145 insertions(+), 189 deletions(-)

diff --git a/config/module/get_test.go b/config/module/get_test.go
index e01312b182..c468653c2f 100644
--- a/config/module/get_test.go
+++ b/config/module/get_test.go
@@ -16,7 +16,6 @@ import (
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform/registry/regsrc"
 	"github.com/hashicorp/terraform/registry/response"
-	"github.com/hashicorp/terraform/svchost/disco"
 )
 
 // Map of module names and location of test modules.
@@ -162,7 +161,7 @@ func mockRegHandler() http.Handler {
 
 	mux.HandleFunc("/.well-known/terraform.json", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		io.WriteString(w, `{"modules.v1":"/v1/modules/"}`)
+		io.WriteString(w, `{"modules.v1":"http://localhost/v1/modules/"}`)
 	})
 	return mux
 }
@@ -174,26 +173,16 @@ func mockRegistry() *httptest.Server {
 	return server
 }
 
-func mockTLSRegistry() *httptest.Server {
-	server := httptest.NewTLSServer(mockRegHandler())
-	return server
-}
-
 // GitHub archives always contain the module source in a single subdirectory,
 // so the registry will return a path with with a `//*` suffix. We need to make
 // sure this doesn't intefere with our internal handling of `//` subdir.
 func TestRegistryGitHubArchive(t *testing.T) {
-	server := mockTLSRegistry()
+	server := mockRegistry()
 	defer server.Close()
-	d := regDisco
 
-	regDisco = disco.NewDisco()
-	regDisco.Transport = mockTransport(server)
-	defer func() {
-		regDisco = d
-	}()
+	disco := testDisco(server)
+	storage := testStorage(t, disco)
 
-	storage := testStorage(t)
 	tree := NewTree("", testConfig(t, "registry-tar-subdir"))
 
 	storage.Mode = GetModeGet
@@ -232,17 +221,11 @@ func TestRegistryGitHubArchive(t *testing.T) {
 
 // Test that the //subdir notation can be used with registry modules
 func TestRegisryModuleSubdir(t *testing.T) {
-	server := mockTLSRegistry()
+	server := mockRegistry()
 	defer server.Close()
 
-	d := regDisco
-	regDisco = disco.NewDisco()
-	regDisco.Transport = mockTransport(server)
-	defer func() {
-		regDisco = d
-	}()
-
-	storage := testStorage(t)
+	disco := testDisco(server)
+	storage := testStorage(t, disco)
 	tree := NewTree("", testConfig(t, "registry-subdir"))
 
 	storage.Mode = GetModeGet
@@ -277,7 +260,8 @@ func TestAccRegistryDiscover(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	loc, err := lookupModuleLocation(nil, module, "")
+	s := NewStorage("/tmp", nil, nil)
+	loc, err := s.lookupModuleLocation(module, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -301,7 +285,7 @@ func TestAccRegistryLoad(t *testing.T) {
 		t.Skip("skipping ACC test")
 	}
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "registry-load"))
 
 	storage.Mode = GetModeGet
diff --git a/config/module/module_test.go b/config/module/module_test.go
index 13614736c3..9685f7feef 100644
--- a/config/module/module_test.go
+++ b/config/module/module_test.go
@@ -1,13 +1,17 @@
 package module
 
 import (
+	"fmt"
 	"io/ioutil"
 	"log"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/svchost"
+	"github.com/hashicorp/terraform/svchost/disco"
 )
 
 func init() {
@@ -41,7 +45,22 @@ func testConfig(t *testing.T, n string) *config.Config {
 	return c
 }
 
-func testStorage(t *testing.T) *Storage {
+func testStorage(t *testing.T, d *disco.Disco) *Storage {
 	t.Helper()
-	return &Storage{StorageDir: tempDir(t)}
+	return NewStorage(tempDir(t), d, nil)
+}
+
+// test discovery maps registry.terraform.io, localhost, localhost.localdomain,
+// and example.com to the test server.
+func testDisco(s *httptest.Server) *disco.Disco {
+	services := map[string]interface{}{
+		"modules.v1": fmt.Sprintf("%s/v1/modules/", s.URL),
+	}
+	d := disco.NewDisco()
+
+	d.ForceHostServices(svchost.Hostname("registry.terraform.io"), services)
+	d.ForceHostServices(svchost.Hostname("localhost"), services)
+	d.ForceHostServices(svchost.Hostname("localhost.localdomain"), services)
+	d.ForceHostServices(svchost.Hostname("example.com"), services)
+	return d
 }
diff --git a/config/module/registry.go b/config/module/registry.go
index b9ca08c8d2..eb236cc0ef 100644
--- a/config/module/registry.go
+++ b/config/module/registry.go
@@ -16,7 +16,6 @@ import (
 	"github.com/hashicorp/terraform/registry/regsrc"
 	"github.com/hashicorp/terraform/registry/response"
 	"github.com/hashicorp/terraform/svchost"
-	"github.com/hashicorp/terraform/svchost/disco"
 	"github.com/hashicorp/terraform/version"
 )
 
@@ -33,7 +32,6 @@ const (
 var (
 	httpClient *http.Client
 	tfVersion  = version.String()
-	regDisco   = disco.NewDisco()
 )
 
 func init() {
@@ -47,16 +45,8 @@ func (e errModuleNotFound) Error() string {
 	return `module "` + string(e) + `" not found`
 }
 
-func discoverRegURL(d *disco.Disco, module *regsrc.Module) *url.URL {
-	if d == nil {
-		d = regDisco
-	}
-
-	if module.RawHost == nil {
-		module.RawHost = regsrc.NewFriendlyHost(defaultRegistry)
-	}
-
-	regURL := d.DiscoverServiceURL(svchost.Hostname(module.RawHost.Normalized()), serviceID)
+func (s *Storage) discoverRegURL(module *regsrc.Module) *url.URL {
+	regURL := s.Services.DiscoverServiceURL(svchost.Hostname(module.RawHost.Normalized()), serviceID)
 	if regURL == nil {
 		regURL = &url.URL{
 			Scheme: "https",
@@ -72,9 +62,29 @@ func discoverRegURL(d *disco.Disco, module *regsrc.Module) *url.URL {
 	return regURL
 }
 
+func (s *Storage) addRequestCreds(host svchost.Hostname, req *http.Request) {
+	if s.Creds == nil {
+		return
+	}
+
+	creds, err := s.Creds.ForHost(host)
+	if err != nil {
+		log.Printf("[WARNING] Failed to get credentials for %s: %s (ignoring)", host, err)
+		return
+	}
+
+	if creds != nil {
+		creds.PrepareRequest(req)
+	}
+}
+
 // Lookup module versions in the registry.
-func lookupModuleVersions(d *disco.Disco, module *regsrc.Module) (*response.ModuleVersions, error) {
-	service := discoverRegURL(d, module)
+func (s *Storage) lookupModuleVersions(module *regsrc.Module) (*response.ModuleVersions, error) {
+	if module.RawHost == nil {
+		module.RawHost = regsrc.NewFriendlyHost(defaultRegistry)
+	}
+
+	service := s.discoverRegURL(module)
 
 	p, err := url.Parse(path.Join(module.Module(), "versions"))
 	if err != nil {
@@ -90,22 +100,10 @@ func lookupModuleVersions(d *disco.Disco, module *regsrc.Module) (*response.Modu
 		return nil, err
 	}
 
+	s.addRequestCreds(svchost.Hostname(module.RawHost.Normalized()), req)
 	req.Header.Set(xTerraformVersion, tfVersion)
 
-	if d == nil {
-		d = regDisco
-	}
-
-	// if discovery required a custom transport, then we should use that too
-	client := httpClient
-	if d.Transport != nil {
-		client = &http.Client{
-			Transport: d.Transport,
-			Timeout:   requestTimeout,
-		}
-	}
-
-	resp, err := client.Do(req)
+	resp, err := httpClient.Do(req)
 	if err != nil {
 		return nil, err
 	}
@@ -131,8 +129,12 @@ func lookupModuleVersions(d *disco.Disco, module *regsrc.Module) (*response.Modu
 }
 
 // lookup the location of a specific module version in the registry
-func lookupModuleLocation(d *disco.Disco, module *regsrc.Module, version string) (string, error) {
-	service := discoverRegURL(d, module)
+func (s *Storage) lookupModuleLocation(module *regsrc.Module, version string) (string, error) {
+	if module.RawHost == nil {
+		module.RawHost = regsrc.NewFriendlyHost(defaultRegistry)
+	}
+
+	service := s.discoverRegURL(module)
 
 	var p *url.URL
 	var err error
@@ -155,16 +157,7 @@ func lookupModuleLocation(d *disco.Disco, module *regsrc.Module, version string)
 
 	req.Header.Set(xTerraformVersion, tfVersion)
 
-	// if discovery required a custom transport, then we should use that too
-	client := httpClient
-	if regDisco.Transport != nil {
-		client = &http.Client{
-			Transport: regDisco.Transport,
-			Timeout:   requestTimeout,
-		}
-	}
-
-	resp, err := client.Do(req)
+	resp, err := httpClient.Do(req)
 	if err != nil {
 		return "", err
 	}
diff --git a/config/module/registry_test.go b/config/module/registry_test.go
index 6387c18125..ec2ae77a56 100644
--- a/config/module/registry_test.go
+++ b/config/module/registry_test.go
@@ -1,75 +1,19 @@
 package module
 
 import (
-	"context"
-	"net"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
 	"os"
 	"testing"
-	"time"
 
-	cleanhttp "github.com/hashicorp/go-cleanhttp"
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform/registry/regsrc"
 	"github.com/hashicorp/terraform/svchost/disco"
 )
 
-// Return a transport to use for this test server.
-// This not only loads the tls.Config from the test server for proper cert
-// validation, but also inserts a Dialer that resolves localhost and
-// example.com to 127.0.0.1 with the correct port, since 127.0.0.1 on its own
-// isn't a valid registry hostname.
-// TODO: cert validation not working here, so we use don't verify for now.
-func mockTransport(server *httptest.Server) *http.Transport {
-	u, _ := url.Parse(server.URL)
-	_, port, _ := net.SplitHostPort(u.Host)
-
-	transport := cleanhttp.DefaultTransport()
-	transport.TLSClientConfig = server.TLS
-	transport.TLSClientConfig.InsecureSkipVerify = true
-	transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
-		host, _, _ := net.SplitHostPort(addr)
-		switch host {
-		case "example.com", "localhost", "localhost.localdomain", "registry.terraform.io":
-			addr = "127.0.0.1"
-			if port != "" {
-				addr += ":" + port
-			}
-		}
-		return (&net.Dialer{
-			Timeout:   30 * time.Second,
-			KeepAlive: 30 * time.Second,
-			DualStack: true,
-		}).DialContext(ctx, network, addr)
-	}
-	return transport
-}
-
-func TestMockDiscovery(t *testing.T) {
-	server := mockTLSRegistry()
-	defer server.Close()
-
-	regDisco := disco.NewDisco()
-	regDisco.Transport = mockTransport(server)
-
-	regURL := regDisco.DiscoverServiceURL("example.com", serviceID)
-
-	if regURL == nil {
-		t.Fatal("no registry service discovered")
-	}
-
-	if regURL.Host != "example.com" {
-		t.Fatal("expected registry host example.com, got:", regURL.Host)
-	}
-}
-
 func TestLookupModuleVersions(t *testing.T) {
-	server := mockTLSRegistry()
+	server := mockRegistry()
 	defer server.Close()
-	regDisco := disco.NewDisco()
-	regDisco.Transport = mockTransport(server)
+
+	regDisco := testDisco(server)
 
 	// test with and without a hostname
 	for _, src := range []string{
@@ -81,7 +25,8 @@ func TestLookupModuleVersions(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		resp, err := lookupModuleVersions(regDisco, modsrc)
+		s := &Storage{Services: regDisco}
+		resp, err := s.lookupModuleVersions(modsrc)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -125,7 +70,10 @@ func TestAccLookupModuleVersions(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		resp, err := lookupModuleVersions(regDisco, modsrc)
+		s := &Storage{
+			Services: regDisco,
+		}
+		resp, err := s.lookupModuleVersions(modsrc)
 		if err != nil {
 			t.Fatal(err)
 		}
diff --git a/config/module/storage.go b/config/module/storage.go
index 6bec91be9b..318c5d36a4 100644
--- a/config/module/storage.go
+++ b/config/module/storage.go
@@ -10,6 +10,7 @@ import (
 
 	getter "github.com/hashicorp/go-getter"
 	"github.com/hashicorp/terraform/registry/regsrc"
+	"github.com/hashicorp/terraform/svchost/auth"
 	"github.com/hashicorp/terraform/svchost/disco"
 	"github.com/mitchellh/cli"
 )
@@ -55,26 +56,44 @@ type moduleRecord struct {
 	registry bool
 }
 
-// Storage 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.
+// Storage implements methods to manage the storage of modules.
+// This is used by Tree.Load to query registries, authenticate requests, and
+// store modules locally.
 type Storage struct {
-	// StorageDir is the directory where all modules will be stored.
+	// StorageDir is the full path to the directory where all modules will be
+	// stored.
 	StorageDir string
-	// Services is a *Disco which ay have services and credentials pre-loaded.
+	// Services is a required *disco.Disco, which may have services and
+	// credentials pre-loaded.
 	Services *disco.Disco
+	// Creds optionally provides credentials for communicating with service
+	// providers.
+	Creds auth.CredentialsSource
 	// Ui is an optional cli.Ui for user output
 	Ui cli.Ui
 	// Mode is the GetMode that will be used for various operations.
 	Mode GetMode
 }
 
+func NewStorage(dir string, services *disco.Disco, creds auth.CredentialsSource) *Storage {
+	s := &Storage{
+		StorageDir: dir,
+		Services:   services,
+		Creds:      creds,
+	}
+
+	// make sure this isn't nil
+	if s.Services == nil {
+		s.Services = disco.NewDisco()
+	}
+	return s
+}
+
 // loadManifest returns the moduleManifest file from the parent directory.
-func (m Storage) loadManifest() (moduleManifest, error) {
+func (s Storage) loadManifest() (moduleManifest, error) {
 	manifest := moduleManifest{}
 
-	manifestPath := filepath.Join(m.StorageDir, manifestName)
+	manifestPath := filepath.Join(s.StorageDir, manifestName)
 	data, err := ioutil.ReadFile(manifestPath)
 	if err != nil && !os.IsNotExist(err) {
 		return manifest, err
@@ -94,8 +113,8 @@ func (m Storage) loadManifest() (moduleManifest, error) {
 // root directory. The storage method loads the entire file and rewrites it
 // each time. This is only done a few times during init, so efficiency is
 // not a concern.
-func (m Storage) recordModule(rec moduleRecord) error {
-	manifest, err := m.loadManifest()
+func (s Storage) recordModule(rec moduleRecord) error {
+	manifest, err := s.loadManifest()
 	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.
@@ -124,15 +143,15 @@ func (m Storage) recordModule(rec moduleRecord) error {
 		panic(err)
 	}
 
-	manifestPath := filepath.Join(m.StorageDir, manifestName)
+	manifestPath := filepath.Join(s.StorageDir, manifestName)
 	return ioutil.WriteFile(manifestPath, js, 0644)
 }
 
 // load the manifest from dir, and return all module versions matching the
 // provided source. Records with no version info will be skipped, as they need
 // to be uniquely identified by other means.
-func (m Storage) moduleVersions(source string) ([]moduleRecord, error) {
-	manifest, err := m.loadManifest()
+func (s Storage) moduleVersions(source string) ([]moduleRecord, error) {
+	manifest, err := s.loadManifest()
 	if err != nil {
 		return manifest.Modules, err
 	}
@@ -148,8 +167,8 @@ func (m Storage) moduleVersions(source string) ([]moduleRecord, error) {
 	return matching, nil
 }
 
-func (m Storage) moduleDir(key string) (string, error) {
-	manifest, err := m.loadManifest()
+func (s Storage) moduleDir(key string) (string, error) {
+	manifest, err := s.loadManifest()
 	if err != nil {
 		return "", err
 	}
@@ -164,8 +183,8 @@ func (m Storage) moduleDir(key string) (string, error) {
 }
 
 // return only the root directory of the module stored in dir.
-func (m Storage) getModuleRoot(dir string) (string, error) {
-	manifest, err := m.loadManifest()
+func (s Storage) getModuleRoot(dir string) (string, error) {
+	manifest, err := s.loadManifest()
 	if err != nil {
 		return "", err
 	}
@@ -179,33 +198,32 @@ func (m Storage) getModuleRoot(dir string) (string, error) {
 }
 
 // record only the Root directory for the module stored at dir.
-// TODO: remove this compatibility function to store the full moduleRecord.
-func (m Storage) recordModuleRoot(dir, root string) error {
+func (s Storage) recordModuleRoot(dir, root string) error {
 	rec := moduleRecord{
 		Dir:  dir,
 		Root: root,
 	}
 
-	return m.recordModule(rec)
+	return s.recordModule(rec)
 }
 
-func (m Storage) getStorage(key string, src string) (string, bool, error) {
+func (s Storage) getStorage(key string, src string) (string, bool, error) {
 	storage := &getter.FolderStorage{
-		StorageDir: m.StorageDir,
+		StorageDir: s.StorageDir,
 	}
 
-	if m.Ui != nil {
+	if s.Ui != nil {
 		update := ""
-		if m.Mode == GetModeUpdate {
+		if s.Mode == GetModeUpdate {
 			update = " (update)"
 		}
-		m.Ui.Output(fmt.Sprintf("Get: %s%s", src, update))
+		s.Ui.Output(fmt.Sprintf("Get: %s%s", src, update))
 	}
 
 	// Get the module with the level specified if we were told to.
-	if m.Mode > GetModeNone {
+	if s.Mode > GetModeNone {
 		log.Printf("[DEBUG] fetching %q with key %q", src, key)
-		if err := storage.Get(key, src, m.Mode == GetModeUpdate); err != nil {
+		if err := storage.Get(key, src, s.Mode == GetModeUpdate); err != nil {
 			return "", false, err
 		}
 	}
@@ -217,16 +235,16 @@ func (m Storage) getStorage(key string, src string) (string, bool, error) {
 }
 
 // find a stored module that's not from a registry
-func (m Storage) findModule(key string) (string, error) {
-	if m.Mode == GetModeUpdate {
+func (s Storage) findModule(key string) (string, error) {
+	if s.Mode == GetModeUpdate {
 		return "", nil
 	}
 
-	return m.moduleDir(key)
+	return s.moduleDir(key)
 }
 
 // find a registry module
-func (m Storage) findRegistryModule(mSource, constraint string) (moduleRecord, error) {
+func (s Storage) findRegistryModule(mSource, constraint string) (moduleRecord, error) {
 	rec := moduleRecord{
 		Source: mSource,
 	}
@@ -244,7 +262,7 @@ func (m Storage) findRegistryModule(mSource, constraint string) (moduleRecord, e
 
 	log.Printf("[TRACE] %q is a registry module", mod.Module())
 
-	versions, err := m.moduleVersions(mod.String())
+	versions, err := s.moduleVersions(mod.String())
 	if err != nil {
 		log.Printf("[ERROR] error looking up versions for %q: %s", mod.Module(), err)
 		return rec, err
@@ -252,7 +270,6 @@ func (m Storage) findRegistryModule(mSource, constraint string) (moduleRecord, e
 
 	match, err := newestRecord(versions, constraint)
 	if err != nil {
-		// TODO: does this allow previously unversioned modules?
 		log.Printf("[INFO] no matching version for %q<%s>, %s", mod.Module(), constraint, err)
 	}
 
@@ -262,8 +279,8 @@ func (m Storage) findRegistryModule(mSource, constraint string) (moduleRecord, e
 
 	// we need to lookup available versions
 	// Only on Get if it's not found, on unconditionally on Update
-	if (m.Mode == GetModeGet && !found) || (m.Mode == GetModeUpdate) {
-		resp, err := lookupModuleVersions(nil, mod)
+	if (s.Mode == GetModeGet && !found) || (s.Mode == GetModeUpdate) {
+		resp, err := s.lookupModuleVersions(mod)
 		if err != nil {
 			return rec, err
 		}
@@ -283,7 +300,7 @@ func (m Storage) findRegistryModule(mSource, constraint string) (moduleRecord, e
 
 		rec.Version = match.Version
 
-		rec.url, err = lookupModuleLocation(nil, mod, rec.Version)
+		rec.url, err = s.lookupModuleLocation(mod, rec.Version)
 		if err != nil {
 			return rec, err
 		}
diff --git a/config/module/tree_gob_test.go b/config/module/tree_gob_test.go
index ad0ab4ecdd..1d2d7d5b55 100644
--- a/config/module/tree_gob_test.go
+++ b/config/module/tree_gob_test.go
@@ -8,7 +8,7 @@ import (
 )
 
 func TestTreeEncodeDecodeGob(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "basic"))
 
 	// This should get things
diff --git a/config/module/tree_test.go b/config/module/tree_test.go
index 780023b7df..2e23f1aad1 100644
--- a/config/module/tree_test.go
+++ b/config/module/tree_test.go
@@ -20,7 +20,7 @@ func TestTreeChild(t *testing.T) {
 		t.Fatal("child should be nil")
 	}
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	tree := NewTree("", testConfig(t, "child"))
 	if err := tree.Load(storage); err != nil {
@@ -65,7 +65,7 @@ func TestTreeChild(t *testing.T) {
 }
 
 func TestTreeLoad(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "basic"))
 
 	if tree.Loaded() {
@@ -105,7 +105,7 @@ func TestTreeLoad(t *testing.T) {
 }
 
 func TestTreeLoad_duplicate(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "dup"))
 
 	if tree.Loaded() {
@@ -184,7 +184,7 @@ func TestTreeLoad_copyable(t *testing.T) {
 }
 
 func TestTreeLoad_parentRef(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "basic-parent"))
 
 	if tree.Loaded() {
@@ -228,17 +228,12 @@ func TestTreeLoad_subdir(t *testing.T) {
 	fixtures := []string{
 		"basic-subdir",
 		"basic-tar-subdir",
-
-		// Passing a subpath to go getter extracts only this subpath. The old
-		// internal code would keep the entire directory structure, allowing a
-		// top-level module to reference others through its parent directory.
-		// TODO: this can be removed as a breaking change in a major release.
 		"tar-subdir-to-parent",
 	}
 
 	for _, tc := range fixtures {
 		t.Run(tc, func(t *testing.T) {
-			storage := testStorage(t)
+			storage := testStorage(t, nil)
 			tree := NewTree("", testConfig(t, tc))
 
 			if tree.Loaded() {
@@ -404,7 +399,7 @@ func TestTreeValidate_table(t *testing.T) {
 	for i, tc := range cases {
 		t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) {
 			tree := NewTree("", testConfig(t, tc.Fixture))
-			storage := testStorage(t)
+			storage := testStorage(t, nil)
 			storage.Mode = GetModeGet
 			if err := tree.Load(storage); err != nil {
 				t.Fatalf("err: %s", err)
@@ -427,7 +422,7 @@ func TestTreeValidate_table(t *testing.T) {
 func TestTreeValidate_badChild(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-child-bad"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -441,7 +436,7 @@ func TestTreeValidate_badChild(t *testing.T) {
 func TestTreeValidate_badChildOutput(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-bad-output"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -455,7 +450,7 @@ func TestTreeValidate_badChildOutput(t *testing.T) {
 func TestTreeValidate_badChildOutputToModule(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-bad-output-to-module"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -469,7 +464,7 @@ func TestTreeValidate_badChildOutputToModule(t *testing.T) {
 func TestTreeValidate_badChildVar(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-bad-var"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -483,7 +478,7 @@ func TestTreeValidate_badChildVar(t *testing.T) {
 func TestTreeValidate_badRoot(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-root-bad"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -497,7 +492,7 @@ func TestTreeValidate_badRoot(t *testing.T) {
 func TestTreeValidate_good(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-child-good"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -519,7 +514,7 @@ func TestTreeValidate_notLoaded(t *testing.T) {
 func TestTreeValidate_requiredChildVar(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-required-var"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeGet
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -542,7 +537,7 @@ func TestTreeValidate_requiredChildVar(t *testing.T) {
 func TestTreeValidate_unknownModule(t *testing.T) {
 	tree := NewTree("", testConfig(t, "validate-module-unknown"))
 
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	storage.Mode = GetModeNone
 	if err := tree.Load(storage); err != nil {
 		t.Fatalf("err: %s", err)
@@ -554,7 +549,7 @@ func TestTreeValidate_unknownModule(t *testing.T) {
 }
 
 func TestTreeProviders_basic(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "basic-parent-providers"))
 
 	storage.Mode = GetModeGet
@@ -617,7 +612,7 @@ func TestTreeProviders_basic(t *testing.T) {
 }
 
 func TestTreeProviders_implicit(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "implicit-parent-providers"))
 
 	storage.Mode = GetModeGet
@@ -658,7 +653,7 @@ func TestTreeProviders_implicit(t *testing.T) {
 }
 
 func TestTreeProviders_implicitMultiLevel(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "implicit-grandparent-providers"))
 
 	storage.Mode = GetModeGet
@@ -728,7 +723,7 @@ func TestTreeProviders_implicitMultiLevel(t *testing.T) {
 }
 
 func TestTreeLoad_conflictingSubmoduleNames(t *testing.T) {
-	storage := testStorage(t)
+	storage := testStorage(t, nil)
 	tree := NewTree("", testConfig(t, "conficting-submodule-names"))
 
 	storage.Mode = GetModeGet

From 5203c661166fc63e72cbbbee8979a4da3cb3a300 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 15:16:58 -0400
Subject: [PATCH 6/8] pass command credentials into module.Storage

---
 command/command_test.go |  6 ++----
 command/meta.go         | 10 ++++------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/command/command_test.go b/command/command_test.go
index a789eebabd..bf6fee7c24 100644
--- a/command/command_test.go
+++ b/command/command_test.go
@@ -107,10 +107,8 @@ func testModule(t *testing.T, name string) *module.Tree {
 		t.Fatalf("err: %s", err)
 	}
 
-	s := &module.Storage{
-		StorageDir: tempDir(t),
-		Mode:       module.GetModeGet,
-	}
+	s := module.NewStorage(tempDir(t), nil, nil)
+	s.Mode = module.GetModeGet
 	if err := mod.Load(s); err != nil {
 		t.Fatalf("err: %s", err)
 	}
diff --git a/command/meta.go b/command/meta.go
index b00c80d48f..e45c40a771 100644
--- a/command/meta.go
+++ b/command/meta.go
@@ -369,12 +369,10 @@ func (m *Meta) flagSet(n string) *flag.FlagSet {
 // moduleStorage returns the module.Storage implementation used to store
 // modules for commands.
 func (m *Meta) moduleStorage(root string, mode module.GetMode) *module.Storage {
-	return &module.Storage{
-		StorageDir: filepath.Join(root, "modules"),
-		Services:   m.Services,
-		Ui:         m.Ui,
-		Mode:       mode,
-	}
+	s := module.NewStorage(filepath.Join(root, "modules"), m.Services, m.Credentials)
+	s.Ui = m.Ui
+	s.Mode = mode
+	return s
 }
 
 // process will process the meta-parameters out of the arguments. This

From ee56e3226beeb3712c427feab58e3a562f6f3aad Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 15:44:00 -0400
Subject: [PATCH 7/8] test that credentials are added to registry reqs

Add the missing set in lookupModuleLocation
---
 config/module/get_test.go      | 21 ++++++++++++++++++
 config/module/registry.go      |  1 +
 config/module/registry_test.go | 40 ++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/config/module/get_test.go b/config/module/get_test.go
index c468653c2f..336c65e4dc 100644
--- a/config/module/get_test.go
+++ b/config/module/get_test.go
@@ -25,6 +25,10 @@ type testMod struct {
 	version  string
 }
 
+const (
+	testCredentials = "a9564ebc3289b7a14551baf8ad5ec60a"
+)
+
 // All the locationes from the mockRegistry start with a file:// scheme. If
 // the the location string here doesn't have a scheme, the mockRegistry will
 // find the absolute path and return a complete URL.
@@ -51,6 +55,9 @@ var testMods = map[string][]testMod{
 		{version: "1.2.2"},
 		{version: "1.2.1"},
 	},
+	"private/name/provider": {
+		{version: "1.0.0"},
+	},
 }
 
 func latestVersion(versions []string) string {
@@ -81,6 +88,13 @@ func mockRegHandler() http.Handler {
 			return
 		}
 
+		// check for auth
+		if strings.Contains(matches[0], "private/") {
+			if !strings.Contains(r.Header.Get("Authorization"), testCredentials) {
+				http.Error(w, "", http.StatusForbidden)
+			}
+		}
+
 		versions, ok := testMods[matches[1]]
 		if !ok {
 			http.NotFound(w, r)
@@ -110,6 +124,13 @@ func mockRegHandler() http.Handler {
 			return
 		}
 
+		// check for auth
+		if strings.Contains(matches[1], "private/") {
+			if !strings.Contains(r.Header.Get("Authorization"), testCredentials) {
+				http.Error(w, "", http.StatusForbidden)
+			}
+		}
+
 		name := matches[1]
 		versions, ok := testMods[name]
 		if !ok {
diff --git a/config/module/registry.go b/config/module/registry.go
index eb236cc0ef..3741af2040 100644
--- a/config/module/registry.go
+++ b/config/module/registry.go
@@ -155,6 +155,7 @@ func (s *Storage) lookupModuleLocation(module *regsrc.Module, version string) (s
 		return "", err
 	}
 
+	s.addRequestCreds(svchost.Hostname(module.RawHost.Normalized()), req)
 	req.Header.Set(xTerraformVersion, tfVersion)
 
 	resp, err := httpClient.Do(req)
diff --git a/config/module/registry_test.go b/config/module/registry_test.go
index ec2ae77a56..ded0040be8 100644
--- a/config/module/registry_test.go
+++ b/config/module/registry_test.go
@@ -6,6 +6,8 @@ import (
 
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform/registry/regsrc"
+	"github.com/hashicorp/terraform/svchost"
+	"github.com/hashicorp/terraform/svchost/auth"
 	"github.com/hashicorp/terraform/svchost/disco"
 )
 
@@ -54,6 +56,44 @@ func TestLookupModuleVersions(t *testing.T) {
 	}
 }
 
+func TestRegistryAuth(t *testing.T) {
+	server := mockRegistry()
+	defer server.Close()
+
+	regDisco := testDisco(server)
+	storage := testStorage(t, regDisco)
+
+	src := "private/name/provider"
+	mod, err := regsrc.ParseModuleSource(src)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// both should fail without auth
+	_, err = storage.lookupModuleVersions(mod)
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	_, err = storage.lookupModuleLocation(mod, "1.0.0")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	storage.Creds = auth.StaticCredentialsSource(map[svchost.Hostname]map[string]interface{}{
+		svchost.Hostname(defaultRegistry): {"token": testCredentials},
+	})
+
+	_, err = storage.lookupModuleVersions(mod)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = storage.lookupModuleLocation(mod, "1.0.0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+}
+
 func TestAccLookupModuleVersions(t *testing.T) {
 	if os.Getenv("TF_ACC") == "" {
 		t.Skip()

From ac68f9a16bddbf61a6532af9cc59408b1f96e340 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 27 Oct 2017 17:36:30 -0400
Subject: [PATCH 8/8] make testCredentials token obviously fake

---
 config/module/get_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/module/get_test.go b/config/module/get_test.go
index 336c65e4dc..6b661dc01d 100644
--- a/config/module/get_test.go
+++ b/config/module/get_test.go
@@ -26,7 +26,7 @@ type testMod struct {
 }
 
 const (
-	testCredentials = "a9564ebc3289b7a14551baf8ad5ec60a"
+	testCredentials = "test-auth-token"
 )
 
 // All the locationes from the mockRegistry start with a file:// scheme. If