surface registry errors to the user

Now that we can enforce local modules being relative or absolute paths,
we can be assured that any module source matching a registry pattern
must be found in the registry. This allows us to surface more useful
errors to the user, rather than simply stating that a source string
isn't valid.
This commit is contained in:
James Bardin 2017-10-18 12:46:04 -04:00
parent 40c36b48dc
commit 0962792257
2 changed files with 20 additions and 62 deletions

View File

@ -3,9 +3,7 @@ package module
import (
"fmt"
"io/ioutil"
"log"
"net/http"
"net/url"
"os"
"regexp"
"strings"
@ -88,7 +86,7 @@ var detectors = []getter.Detector{
new(getter.BitBucketDetector),
new(getter.S3Detector),
new(registryDetector),
new(localDetector),
new(getter.FileDetector),
}
// these prefixes can't be registry IDs
@ -136,28 +134,24 @@ 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.Printf("[WARN] error looking up module %q: %s", src, err)
return "", false, nil
return "", false, fmt.Errorf("error looking up module %q: %s", src, err)
}
defer resp.Body.Close()
// there should be no body, but save it for logging
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
fmt.Printf("[WARN] error reading response body from registry: %s", err)
return "", false, nil
return "", false, fmt.Errorf("error reading response body from registry: %s", err)
}
switch resp.StatusCode {
case http.StatusOK, http.StatusNoContent:
// OK
case http.StatusNotFound:
log.Printf("[INFO] module %q not found in registry", src)
return "", false, nil
return "", false, fmt.Errorf("module %q not found in registry", src)
default:
// anything else is an error:
log.Printf("[WARN] error getting download location for %q: %s resp:%s", src, resp.Status, body)
return "", false, nil
return "", false, fmt.Errorf("error getting download location for %q: %s resp:%s", src, resp.Status, body)
}
// the download location is in the X-Terraform-Get header
@ -168,40 +162,3 @@ 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
}

View File

@ -113,7 +113,7 @@ func setResetRegDetector(server *httptest.Server) func() {
new(getter.BitBucketDetector),
new(getter.S3Detector),
regDetector,
new(localDetector),
new(getter.FileDetector),
}
return func() {
@ -146,10 +146,10 @@ func TestDetectRegistry(t *testing.T) {
location: testMods["registry/foo/baz"].location,
found: true,
},
// this should not be found, but not stop detection
// this should not be found, and is no longer valid as a local source
{
source: "registry/foo/notfound",
found: false,
err: true,
},
// a full url should not be detected
@ -178,7 +178,7 @@ func TestDetectRegistry(t *testing.T) {
t.Run(tc.source, func(t *testing.T) {
loc, ok, err := detector.Detect(tc.source, "")
if (err == nil) == tc.err {
t.Fatalf("expected error? %t; got error :%v", tc.err, err)
t.Fatalf("expected error? %t; got error: %v", tc.err, err)
}
if ok != tc.found {
@ -215,7 +215,7 @@ func TestDetectors(t *testing.T) {
source: "registry/foo/bar",
location: "file:///download/registry/foo/bar/0.2.3//*?archive=tar.gz",
},
// this should not be found, but not stop detection
// this should not be found, and is no longer a valid local source
{
source: "registry/foo/notfound",
err: true,
@ -237,26 +237,27 @@ func TestDetectors(t *testing.T) {
// local paths should be detected as such, even if they're match
// registry modules.
{
source: "./registry/foo/bar",
err: true,
source: "./registry/foo/bar",
location: "file://" + filepath.Join(wd, "registry/foo/bar"),
},
{
source: "/registry/foo/bar",
err: true,
source: "/registry/foo/bar",
location: "file:///registry/foo/bar",
},
// wrong number of parts can't be regisry IDs
// Wrong number of parts can't be registry IDs.
// This is returned as a local path for now, but may return an error at
// some point.
{
source: "something/registry/foo/notfound",
err: true,
source: "something/here/registry/foo/notfound",
location: "file://" + filepath.Join(wd, "something/here/registry/foo/notfound"),
},
// make sure a local module that looks like a registry id can be found
{
source: "namespace/identifier/provider",
fixture: "discover-subdirs",
// this should be found locally
location: "file://" + filepath.Join(wd, fixtureDir, "discover-subdirs/namespace/identifier/provider"),
err: true,
},
// The registry takes precedence over local paths if they don't start