Merge pull request #329 from hashicorp/f-module-subdir

Module source should support subdir

config/module/copy_dir.go

@@ -0,0 +1,51 @@
+package module
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+)
+
+// copyDir copies the src directory contents into dst. Both directories
+// should already exist.
+func copyDir(dst, src string) error {
+	walkFn := func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		dstPath := filepath.Join(dst, filepath.Base(path))
+
+		// If we have a directory, make that subdirectory, then continue
+		// the walk.
+		if info.IsDir() {
+			if err := os.MkdirAll(dstPath, 0755); err != nil {
+				return err
+			}
+
+			return copyDir(dstPath, path)
+		}
+
+		// If we have a file, copy the contents.
+		srcF, err := os.Open(path)
+		if err != nil {
+			return err
+		}
+		defer srcF.Close()
+
+		dstF, err := os.Create(dstPath)
+		if err != nil {
+			return err
+		}
+		defer dstF.Close()
+
+		if _, err := io.Copy(dstF, srcF); err != nil {
+			return err
+		}
+
+		// Chmod it
+		return os.Chmod(dstPath, info.Mode())
+	}
+
+	return filepath.Walk(src, walkFn)
+}

config/module/detect_github.go

@@ -25,7 +25,13 @@ func (d *GitHubDetector) Detect(src, _ string) (string, bool, error) {
 }
 
 func (d *GitHubDetector) detectHTTP(src string) (string, bool, error) {
-	urlStr := fmt.Sprintf("https://%s", src)
+	parts := strings.Split(src, "/")
+	if len(parts) < 3 {
+		return "", false, fmt.Errorf(
+			"GitHub URLs should be github.com/username/repo")
+	}
+
+	urlStr := fmt.Sprintf("https://%s", strings.Join(parts[:3], "/"))
 	url, err := url.Parse(urlStr)
 	if err != nil {
 		return "", true, fmt.Errorf("error parsing GitHub URL: %s", err)
@@ -35,6 +41,10 @@ func (d *GitHubDetector) detectHTTP(src string) (string, bool, error) {
 		url.Path += ".git"
 	}
 
+	if len(parts) > 3 {
+		url.Path += "//" + strings.Join(parts[3:], "/")
+	}
+
 	return "git::" + url.String(), true, nil
 }
 

config/module/detect_github_test.go

@@ -12,6 +12,10 @@ func TestGitHubDetector(t *testing.T) {
 		// HTTP
 		{"github.com/hashicorp/foo", "git::https://github.com/hashicorp/foo.git"},
 		{"github.com/hashicorp/foo.git", "git::https://github.com/hashicorp/foo.git"},
+		{
+			"github.com/hashicorp/foo/bar",
+			"git::https://github.com/hashicorp/foo.git//bar",
+		},
 		{
 			"github.com/hashicorp/foo?foo=bar",
 			"git::https://github.com/hashicorp/foo.git?foo=bar",
@@ -23,6 +27,10 @@ func TestGitHubDetector(t *testing.T) {
 
 		// SSH
 		{"git@github.com:hashicorp/foo.git", "git::ssh://git@github.com/hashicorp/foo.git"},
+		{
+			"git@github.com:hashicorp/foo.git//bar",
+			"git::ssh://git@github.com/hashicorp/foo.git//bar",
+		},
 		{
 			"git@github.com:hashicorp/foo.git?foo=bar",
 			"git::ssh://git@github.com/hashicorp/foo.git?foo=bar",

config/module/get.go

@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"os/exec"
 	"regexp"
+	"strings"
 	"syscall"
 )
 
@@ -96,6 +97,37 @@ func getRunCommand(cmd *exec.Cmd) error {
 	return fmt.Errorf("error running %s: %s", cmd.Path, buf.String())
 }
 
+// getDirSubdir takes a source and returns a tuple of the URL without
+// the subdir and the URL with the subdir.
+func getDirSubdir(src string) (string, string) {
+	// Calcaulate an offset to avoid accidentally marking the scheme
+	// as the dir.
+	var offset int
+	if idx := strings.Index(src, "://"); idx > -1 {
+		offset = idx + 3
+	}
+
+	// First see if we even have an explicit subdir
+	idx := strings.Index(src[offset:], "//")
+	if idx == -1 {
+		return src, ""
+	}
+
+	idx += offset
+	subdir := src[idx+2:]
+	src = src[:idx]
+
+	// Next, check if we have query parameters and push them onto the
+	// URL.
+	if idx = strings.Index(subdir, "?"); idx > -1 {
+		query := subdir[idx:]
+		subdir = subdir[:idx]
+		src += query
+	}
+
+	return src, subdir
+}
+
 // getForcedGetter takes a source and returns the tuple of the forced
 // getter and the raw URL (without the force syntax).
 func getForcedGetter(src string) (string, string) {

config/module/get_http.go

@@ -4,8 +4,11 @@ import (
 	"encoding/xml"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/url"
+	"os"
+	"path/filepath"
 	"strings"
 )
 
@@ -62,8 +65,50 @@ func (g *HttpGetter) Get(dst string, u *url.URL) error {
 		return fmt.Errorf("no source URL was returned")
 	}
 
-	// Get it!
-	return Get(dst, source)
+	// If there is a subdir component, then we download the root separately
+	// into a temporary directory, then copy over the proper subdir.
+	source, subDir := getDirSubdir(source)
+	if subDir == "" {
+		return Get(dst, source)
+	}
+
+	// We have a subdir, time to jump some hoops
+	return g.getSubdir(dst, source, subDir)
+}
+
+// getSubdir downloads the source into the destination, but with
+// the proper subdir.
+func (g *HttpGetter) getSubdir(dst, source, subDir string) error {
+	// Create a temporary directory to store the full source
+	td, err := ioutil.TempDir("", "tf")
+	if err != nil {
+		return err
+	}
+	defer os.RemoveAll(td)
+
+	// Download that into the given directory
+	if err := Get(td, source); err != nil {
+		return err
+	}
+
+	// Make sure the subdir path actually exists
+	sourcePath := filepath.Join(td, subDir)
+	if _, err := os.Stat(sourcePath); err != nil {
+		return fmt.Errorf(
+			"Error downloading %s: %s", source, err)
+	}
+
+	// Copy the subdirectory into our actual destination.
+	if err := os.RemoveAll(dst); err != nil {
+		return err
+	}
+
+	// Make the final destination
+	if err := os.MkdirAll(dst, 0755); err != nil {
+		return err
+	}
+
+	return copyDir(dst, sourcePath)
 }
 
 // parseMeta looks for the first meta tag in the given reader that

config/module/get_http_test.go

@@ -62,6 +62,30 @@ func TestHttpGetter_meta(t *testing.T) {
 	}
 }
 
+func TestHttpGetter_metaSubdir(t *testing.T) {
+	ln := testHttpServer(t)
+	defer ln.Close()
+
+	g := new(HttpGetter)
+	dst := tempDir(t)
+
+	var u url.URL
+	u.Scheme = "http"
+	u.Host = ln.Addr().String()
+	u.Path = "/meta-subdir"
+
+	// Get it!
+	if err := g.Get(dst, &u); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Verify the main file exists
+	mainPath := filepath.Join(dst, "sub.tf")
+	if _, err := os.Stat(mainPath); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func TestHttpGetter_none(t *testing.T) {
 	ln := testHttpServer(t)
 	defer ln.Close()
@@ -89,6 +113,7 @@ func testHttpServer(t *testing.T) net.Listener {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/header", testHttpHandlerHeader)
 	mux.HandleFunc("/meta", testHttpHandlerMeta)
+	mux.HandleFunc("/meta-subdir", testHttpHandlerMetaSubdir)
 
 	var server http.Server
 	server.Handler = mux
@@ -106,6 +131,10 @@ func testHttpHandlerMeta(w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(fmt.Sprintf(testHttpMetaStr, testModuleURL("basic").String())))
 }
 
+func testHttpHandlerMetaSubdir(w http.ResponseWriter, r *http.Request) {
+	w.Write([]byte(fmt.Sprintf(testHttpMetaStr, testModuleURL("basic//subdir").String())))
+}
+
 func testHttpHandlerNone(w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(testHttpNoneStr))
 }

config/module/get_test.go

@@ -45,3 +45,37 @@ func TestGet_fileForced(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 }
+
+func TestGetDirSubdir(t *testing.T) {
+	cases := []struct {
+		Input string
+		Dir, Sub string
+	}{
+		{
+			"hashicorp.com",
+			"hashicorp.com", "",
+		},
+		{
+			"hashicorp.com//foo",
+			"hashicorp.com", "foo",
+		},
+		{
+			"hashicorp.com//foo?bar=baz",
+			"hashicorp.com?bar=baz", "foo",
+		},
+		{
+			"file://foo//bar",
+			"file://foo", "bar",
+		},
+	}
+
+	for i, tc := range cases {
+		adir, asub := getDirSubdir(tc.Input)
+		if adir != tc.Dir {
+			t.Fatalf("%d: bad dir: %#v", i, adir)
+		}
+		if asub != tc.Sub {
+			t.Fatalf("%d: bad sub: %#v", i, asub)
+		}
+	}
+}

config/module/storage.go

@@ -11,3 +11,15 @@ type Storage interface {
 	// Get will download and optionally update the given module.
 	Get(string, bool) error
 }
+
+func getStorage(s Storage, 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(src, mode == GetModeUpdate); err != nil {
+			return "", false, err
+		}
+	}
+
+	// Get the directory where the module is.
+	return s.Dir(src)
+}

config/module/test-fixtures/basic-git/DOTgit/COMMIT_EDITMSG

@@ -1,7 +1,7 @@
-Branch
+add subdir
 # Please enter the commit message for your changes. Lines starting
 # with '#' will be ignored, and an empty message aborts the commit.
-# On branch test-branch
+# On branch master
 # Changes to be committed:
-#	new file:   main_branch.tf
+#	new file:   subdir/sub.tf
 #

config/module/test-fixtures/basic-git/DOTgit/logs/HEAD

@@ -4,3 +4,4 @@
 1f31e97f053caeb5d6b7bffa3faf82941c99efa2 1f31e97f053caeb5d6b7bffa3faf82941c99efa2 Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1410886909 -0700	checkout: moving from master to test-branch
 1f31e97f053caeb5d6b7bffa3faf82941c99efa2 7b7614f8759ac8b5e4b02be65ad8e2667be6dd87 Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1410886913 -0700	commit: Branch
 7b7614f8759ac8b5e4b02be65ad8e2667be6dd87 1f31e97f053caeb5d6b7bffa3faf82941c99efa2 Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1410886916 -0700	checkout: moving from test-branch to master
+1f31e97f053caeb5d6b7bffa3faf82941c99efa2 146492b04efe0aae2b8288c5c0aef6a951030fde Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1411767116 -0700	commit: add subdir

config/module/test-fixtures/basic-git/DOTgit/logs/refs/heads/master

@@ -1,3 +1,4 @@
 0000000000000000000000000000000000000000 497bc37401eb3c9b11865b1768725b64066eccee Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1410850637 -0700	commit (initial): A commit
 497bc37401eb3c9b11865b1768725b64066eccee 243f0fc5c4e586d1a3daa54c981b6f34e9ab1085 Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1410886526 -0700	commit: tag1
 243f0fc5c4e586d1a3daa54c981b6f34e9ab1085 1f31e97f053caeb5d6b7bffa3faf82941c99efa2 Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1410886536 -0700	commit: remove tag1
+1f31e97f053caeb5d6b7bffa3faf82941c99efa2 146492b04efe0aae2b8288c5c0aef6a951030fde Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 1411767116 -0700	commit: add subdir

config/module/test-fixtures/basic-git/DOTgit/refs/heads/master

@@ -1 +1 @@
-1f31e97f053caeb5d6b7bffa3faf82941c99efa2
+146492b04efe0aae2b8288c5c0aef6a951030fde

config/module/test-fixtures/basic-subdir/foo/sub/main.tf

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

config/module/test-fixtures/basic-subdir/main.tf

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

config/module/tree.go

@@ -2,6 +2,7 @@ package module
 
 import (
 	"bufio"
+	"path/filepath"
 	"bytes"
 	"fmt"
 	"strings"
@@ -131,27 +132,28 @@ func (t *Tree) Load(s Storage, mode GetMode) error {
 	children := make(map[string]*Tree)
 
 	// Go through all the modules and get the directory for them.
-	update := mode == GetModeUpdate
 	for _, m := range modules {
 		if _, ok := children[m.Name]; ok {
 			return fmt.Errorf(
 				"module %s: duplicated. module names must be unique", m.Name)
 		}
 
-		source, err := Detect(m.Source, t.config.Dir)
+		// Split out the subdir if we have one
+		source, subDir := getDirSubdir(m.Source)
+
+		source, err := Detect(source, t.config.Dir)
 		if err != nil {
 			return fmt.Errorf("module %s: %s", m.Name, err)
 		}
 
-		if mode > GetModeNone {
-			// Get the module since we specified we should
-			if err := s.Get(source, update); err != nil {
-				return err
-			}
+		// Check if the detector introduced something new.
+		source, subDir2 := getDirSubdir(source)
+		if subDir2 != "" {
+			subDir = filepath.Join(subDir2, subDir)
 		}
 
 		// Get the directory where this module is so we can load it
-		dir, ok, err := s.Dir(source)
+		dir, ok, err := getStorage(s, source, mode)
 		if err != nil {
 			return err
 		}
@@ -160,7 +162,12 @@ func (t *Tree) Load(s Storage, mode GetMode) error {
 				"module %s: not found, may need to be downloaded", m.Name)
 		}
 
-		// Load the configuration
+		// If we have a subdirectory, then merge that in
+		if subDir != "" {
+			dir = filepath.Join(dir, subDir)
+		}
+
+		// Load the configurations.Dir(source)
 		children[m.Name], err = NewTreeModule(m.Name, dir)
 		if err != nil {
 			return fmt.Errorf(

config/module/tree_test.go

@@ -58,6 +58,44 @@ func TestTreeLoad_duplicate(t *testing.T) {
 	}
 }
 
+func TestTreeLoad_subdir(t *testing.T) {
+	storage := testStorage(t)
+	tree := NewTree("", testConfig(t, "basic-subdir"))
+
+	if tree.Loaded() {
+		t.Fatal("should not be loaded")
+	}
+
+	// This should error because we haven't gotten things yet
+	if err := tree.Load(storage, GetModeNone); err == nil {
+		t.Fatal("should error")
+	}
+
+	if tree.Loaded() {
+		t.Fatal("should not be loaded")
+	}
+
+	// This should get things
+	if err := tree.Load(storage, GetModeGet); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if !tree.Loaded() {
+		t.Fatal("should be loaded")
+	}
+
+	// This should no longer error
+	if err := tree.Load(storage, GetModeNone); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(tree.String())
+	expected := strings.TrimSpace(treeLoadSubdirStr)
+	if actual != expected {
+		t.Fatalf("bad: \n\n%s", actual)
+	}
+}
+
 func TestTreeModules(t *testing.T) {
 	tree := NewTree("", testConfig(t, "basic"))
 	actual := tree.Modules()
@@ -164,3 +202,9 @@ const treeLoadStr = `
 root
   foo
 `
+
+const treeLoadSubdirStr = `
+root
+  foo
+    bar
+`

website/source/docs/modules/sources.html.markdown

@@ -64,10 +64,14 @@ Subdirectories within the repository can also be referenced:
 
 ```
 module "consul" {
-	source = "github.com/hashicorp/example/subdir"
+	source = "github.com/hashicorp/example//subdir"
 }
 ```
 
+**Note:** The double-slash is important. It is what tells Terraform that
+that is the separator for a subdirectory, and not part of the repository
+itself.
+
 GitHub source URLs will require that Git is installed on your system
 and that you have the proper access to the repository.
 
@@ -90,10 +94,14 @@ Subdirectories within the repository can also be referenced:
 
 ```
 module "consul" {
-	source = "bitbucket.org/hashicorp/example/subdir"
+	source = "bitbucket.org/hashicorp/example//subdir"
 }
 ```
 
+**Note:** The double-slash is important. It is what tells Terraform that
+that is the separator for a subdirectory, and not part of the repository
+itself.
+
 BitBucket URLs will require that Git or Mercurial is installed on your
 system, depending on the source URL.