move local module detection ahead of the registry

The getter.FileDetector was intended to be the final detector, only
converting a path to a file URL and returning a true in all cases. We
want to check for a local module before checking the registry so no
local modules that happen to match a registry module are broken.

Wrap the getter.FileDetector to check the module source's existence
before delegating the search to the registry.

config/module/get.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
+	"net/url"
 	"os"
 	"regexp"
 	"strings"
@@ -86,8 +87,8 @@ var detectors = []getter.Detector{
 	new(getter.GitHubDetector),
 	new(getter.BitBucketDetector),
 	new(getter.S3Detector),
+	new(localDetector),
 	new(registryDetector),
-	new(getter.FileDetector),
 }
 
 // these prefixes can't be registry IDs
@@ -121,8 +122,6 @@ func (d registryDetector) Detect(src, _ string) (string, bool, error) {
 }
 
 // Lookup the module in the registry.
-// Since existing module sources may match a registry ID format, we only log
-// registry errors and continue discovery.
 func (d registryDetector) lookupModule(src string) (string, bool, error) {
 	if d.api == "" {
 		d.api = registryAPI
@@ -137,7 +136,7 @@ func (d registryDetector) lookupModule(src string) (string, bool, error) {
 	// determine if it's truly valid.
 	resp, err := d.client.Get(fmt.Sprintf("%s/%s/download", d.api, src))
 	if err != nil {
-		log.Println("[WARN] error looking up module %q: %s", src, err)
+		log.Printf("[WARN] error looking up module %q: %s", src, err)
 		return "", false, nil
 	}
 	defer resp.Body.Close()
@@ -145,7 +144,7 @@ func (d registryDetector) lookupModule(src string) (string, bool, error) {
 	// there should be no body, but save it for logging
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		fmt.Println("[WARN] error reading response body from registry: %s", err)
+		fmt.Printf("[WARN] error reading response body from registry: %s", err)
 		return "", false, nil
 	}
 
@@ -169,3 +168,40 @@ func (d registryDetector) lookupModule(src string) (string, bool, error) {
 
 	return location, true, nil
 }
+
+// localDetector wraps the default getter.FileDetector and checks if the module
+// exists in the local filesystem. The default FileDetector only converts paths
+// into file URLs, and returns found. We want to first check for a local module
+// before passing it off to the registryDetector so we don't inadvertently
+// replace a local module with a registry module of the same name.
+type localDetector struct{}
+
+func (d localDetector) Detect(src, wd string) (string, bool, error) {
+	localSrc, ok, err := new(getter.FileDetector).Detect(src, wd)
+	if err != nil {
+		return src, ok, err
+	}
+
+	if !ok {
+		return "", false, nil
+	}
+
+	u, err := url.Parse(localSrc)
+	if err != nil {
+		return "", false, err
+	}
+
+	_, err = os.Stat(u.Path)
+
+	// just continue detection if it doesn't exist
+	if os.IsNotExist(err) {
+		return "", false, nil
+	}
+
+	// return any other errors
+	if err != nil {
+		return "", false, err
+	}
+
+	return localSrc, true, nil
+}

config/module/get_test.go

@@ -4,11 +4,14 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"regexp"
 	"sort"
 	"strings"
 	"testing"
 
+	getter "github.com/hashicorp/go-getter"
 	version "github.com/hashicorp/go-version"
 )
 
@@ -79,52 +82,52 @@ func TestDetectRegistry(t *testing.T) {
 	}
 
 	for _, tc := range []struct {
-		module   string
+		source   string
 		location string
 		found    bool
 		err      bool
 	}{
 		{
-			module:   "registry/foo/bar",
+			source:   "registry/foo/bar",
 			location: "download/registry/foo/bar/0.2.3",
 			found:    true,
 		},
 		{
-			module:   "registry/foo/baz",
+			source:   "registry/foo/baz",
 			location: "download/registry/foo/baz/1.10.0",
 			found:    true,
 		},
 		// this should not be found, but not stop detection
 		{
-			module: "registry/foo/notfound",
+			source: "registry/foo/notfound",
 			found:  false,
 		},
 
 		// a full url should not be detected
 		{
-			module: "http://example.com/registry/foo/notfound",
+			source: "http://example.com/registry/foo/notfound",
 			found:  false,
 		},
 
 		// paths should not be detected
 		{
-			module: "./local/foo/notfound",
+			source: "./local/foo/notfound",
 			found:  false,
 		},
 		{
-			module: "/local/foo/notfound",
+			source: "/local/foo/notfound",
 			found:  false,
 		},
 
 		// wrong number of parts can't be regisry IDs
 		{
-			module: "something/registry/foo/notfound",
+			source: "something/registry/foo/notfound",
 			found:  false,
 		},
 	} {
 
-		t.Run(tc.module, func(t *testing.T) {
+		t.Run(tc.source, func(t *testing.T) {
-			loc, ok, err := detector.Detect(tc.module, "")
+			loc, ok, err := detector.Detect(tc.source, "")
 			if (err == nil) == tc.err {
 				t.Fatalf("expected error? %t; got error :%v", tc.err, err)
 			}
@@ -141,3 +144,105 @@ func TestDetectRegistry(t *testing.T) {
 
 	}
 }
+
+// check that the full set of detectors works as expected
+func TestDetectors(t *testing.T) {
+	server := mockRegistry()
+	defer server.Close()
+
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	regDetector := &registryDetector{
+		api:    server.URL + "/v1/modules/",
+		client: server.Client(),
+	}
+
+	detectors := []getter.Detector{
+		new(getter.GitHubDetector),
+		new(getter.BitBucketDetector),
+		new(getter.S3Detector),
+		new(localDetector),
+		regDetector,
+	}
+
+	for _, tc := range []struct {
+		source   string
+		location string
+		fixture  string
+		err      bool
+	}{
+		{
+			source:   "registry/foo/bar",
+			location: "download/registry/foo/bar/0.2.3",
+		},
+		// this should not be found, but not stop detection
+		{
+			source: "registry/foo/notfound",
+			err:    true,
+		},
+		// a full url should be unchanged
+		{
+			source: "http://example.com/registry/foo/notfound?" +
+				"checksum=sha256:f19056b80a426d797ff9e470da069c171a6c6befa83e2da7f6c706207742acab",
+			location: "http://example.com/registry/foo/notfound?" +
+				"checksum=sha256:f19056b80a426d797ff9e470da069c171a6c6befa83e2da7f6c706207742acab",
+		},
+
+		// forced getters will return untouched
+		{
+			source:   "git::http://example.com/registry/foo/notfound?param=value",
+			location: "git::http://example.com/registry/foo/notfound?param=value",
+		},
+
+		// local paths should be detected as such, even if they're match
+		// registry modules.
+		{
+			source: "./registry/foo/bar",
+			err:    true,
+		},
+		{
+			source: "/registry/foo/bar",
+			err:    true,
+		},
+
+		// wrong number of parts can't be regisry IDs
+		{
+			source: "something/registry/foo/notfound",
+			err:    true,
+		},
+
+		// make sure a local module that looks like a registry id takes precedence
+		{
+			source:  "namespace/identifier/provider",
+			fixture: "discover-subdirs",
+			// this should be found locally
+			location: "file://" + filepath.Join(wd, fixtureDir, "discover-subdirs/namespace/identifier/provider"),
+		},
+	} {
+
+		t.Run(tc.source, func(t *testing.T) {
+			dir := wd
+			if tc.fixture != "" {
+				dir = filepath.Join(wd, fixtureDir, tc.fixture)
+				if err := os.Chdir(dir); err != nil {
+					t.Fatal(err)
+				}
+				defer os.Chdir(wd)
+			}
+
+			loc, err := getter.Detect(tc.source, dir, detectors)
+			if (err == nil) == tc.err {
+				t.Fatalf("expected error? %t; got error :%v", tc.err, err)
+			}
+
+			loc = strings.TrimPrefix(loc, server.URL+"/")
+			if strings.TrimPrefix(loc, server.URL) != tc.location {
+				t.Fatalf("expected location: %q, got %q", tc.location, loc)
+			}
+		})
+
+	}
+}

config/module/module_test.go

@@ -13,6 +13,7 @@ import (
 const fixtureDir = "./test-fixtures"
 
 func tempDir(t *testing.T) string {
+	t.Helper()
 	dir, err := ioutil.TempDir("", "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -25,6 +26,7 @@ func tempDir(t *testing.T) string {
 }
 
 func testConfig(t *testing.T, n string) *config.Config {
+	t.Helper()
 	c, err := config.LoadDir(filepath.Join(fixtureDir, n))
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -34,5 +36,6 @@ func testConfig(t *testing.T, n string) *config.Config {
 }
 
 func testStorage(t *testing.T) getter.Storage {
+	t.Helper()
 	return &getter.FolderStorage{StorageDir: tempDir(t)}
 }