Goget Fetcher should error out if gobinpath is not valid (gomods#377)

* Goget fetcher should error out if gobinpath is not valid

* Propogating the error from goget initialisation to main, to stop application

* wrapping errors with op, using exec.command(gobin).Run to verify gobin, clean up

* Inlining afero fs to goget call, and new go getfetcher

* Revert "Inlining afero fs to goget call, and new go getfetcher"

This reverts commit ae31fe6.

* Fixing example test

cmd/olympus/actions/app.go

@@ -113,7 +113,11 @@ func App(config *AppConfig) (*buffalo.App, error) {
 	app.POST("/push", pushNotificationHandler(w))
 
 	// Download Protocol
-	dp := download.New(goget.New(), config.Storage)
+	gg, err := goget.New()
+	if err != nil {
+		return nil, err
+	}
+	dp := download.New(gg, config.Storage)
 	app.GET(download.PathList, download.ListHandler(dp, lggr, renderEng))
 	app.GET(download.PathVersionInfo, download.VersionInfoHandler(dp, lggr, renderEng))
 	app.GET(download.PathVersionModule, download.VersionModuleHandler(dp, lggr, renderEng))

cmd/proxy/actions/app_proxy.go

@@ -17,7 +17,11 @@ func addProxyRoutes(
 ) error {
 	app.GET("/", proxyHomeHandler)
 
-	dp := download.New(goget.New(), storage)
+	gg, err := goget.New()
+	if err != nil {
+		return err
+	}
+	dp := download.New(gg, storage)
 	// Download Protocol
 	app.GET(download.PathList, download.ListHandler(dp, lggr, proxy))
 	app.GET(download.PathLatest, download.LatestHandler(dp, lggr, proxy))

pkg/download/goget/goget.go

@@ -22,15 +22,24 @@ import (
 // New returns a download protocol by using
 // go get. You must have a modules supported
 // go binary for this to work.
-func New() download.Protocol {
-	return &goget{
-		goBinPath: env.GoBinPath(),
-		fs:        afero.NewOsFs(),
+func New() (download.Protocol, error) {
+	const op errors.Op = "goget.New"
+	goBin := env.GoBinPath()
+	fs := afero.NewOsFs()
+	mf, err := module.NewGoGetFetcher(goBin, fs)
+	if err != nil {
+		return nil, errors.E(op, err)
 	}
+	return &goget{
+		goBinPath: goBin,
+		fetcher:   mf,
+		fs:        fs,
+	}, nil
 }
 
 type goget struct {
 	goBinPath string
+	fetcher   module.Fetcher
 	fs        afero.Fs
 }
 
@@ -139,8 +148,7 @@ func (gg *goget) Zip(ctx context.Context, mod, ver string) (io.ReadCloser, error
 
 func (gg *goget) Version(ctx context.Context, mod, ver string) (*storage.Version, error) {
 	const op errors.Op = "goget.Version"
-	fetcher := module.NewGoGetFetcher(gg.goBinPath, gg.fs)
-	ref, err := fetcher.Fetch(mod, ver)
+	ref, err := gg.fetcher.Fetch(mod, ver)
 	if err != nil {
 		return nil, errors.E(op, err)
 	}

pkg/download/goget/goget_test.go

@@ -3,6 +3,8 @@ package goget
 import (
 	"context"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 type testCase struct {
@@ -23,7 +25,8 @@ var tt = []testCase{
 }
 
 func TestList(t *testing.T) {
-	dp := New()
+	dp, err := New()
+	require.NoError(t, err, "failed to create protocol")
 	ctx := context.Background()
 
 	for _, tc := range tt {

pkg/module/go_get_fetcher.go

@@ -19,11 +19,15 @@ type goGetFetcher struct {
 }
 
 // NewGoGetFetcher creates fetcher which uses go get tool to fetch modules
-func NewGoGetFetcher(goBinaryName string, fs afero.Fs) Fetcher {
+func NewGoGetFetcher(goBinaryName string, fs afero.Fs) (Fetcher, error) {
+	const op errors.Op = "module.NewGoGetFetcher"
+	if err := validGoBinary(goBinaryName); err != nil {
+		return nil, errors.E(op, err)
+	}
 	return &goGetFetcher{
 		fs:           fs,
 		goBinaryName: goBinaryName,
-	}
+	}, nil
 }
 
 // Fetch downloads the sources and returns path where it can be found. Make sure to call Clear
@@ -148,3 +152,13 @@ func getRepoDirName(repoURI, version string) string {
 func getPackagePath(gopath, module string) string {
 	return filepath.Join(gopath, "src", "mod", "cache", "download", module, "@v")
 }
+
+func validGoBinary(name string) error {
+	const op errors.Op = "module.validGoBinary"
+	err := exec.Command(name).Run()
+	_, ok := err.(*exec.ExitError)
+	if err != nil && !ok {
+		return errors.E(op, err)
+	}
+	return nil
+}

pkg/module/go_get_fetcher_test.go

@@ -3,26 +3,37 @@ package module
 import (
 	"fmt"
 	"io/ioutil"
+	"log"
 
 	"github.com/gomods/athens/pkg/config/env"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/spf13/afero"
 )
 
 func (s *ModuleSuite) TestNewGoGetFetcher() {
 	r := s.Require()
-	fetcher := NewGoGetFetcher(s.goBinaryName, s.fs)
+	fetcher, err := NewGoGetFetcher(s.goBinaryName, s.fs)
+	r.NoError(err)
 	_, ok := fetcher.(*goGetFetcher)
 	r.True(ok)
 }
 
+func (s *ModuleSuite) TestGoGetFetcherError() {
+	fetcher, err := NewGoGetFetcher("invalidpath", afero.NewOsFs())
+
+	assert.Nil(s.T(), fetcher)
+	assert.EqualError(s.T(), err, "exec: \"invalidpath\": executable file not found in $PATH")
+}
+
 func (s *ModuleSuite) TestGoGetFetcherFetch() {
 	r := s.Require()
 	// we need to use an OS filesystem because fetch executes vgo on the command line, which
 	// always writes to the filesystem
-	fetcher := NewGoGetFetcher(s.goBinaryName, afero.NewOsFs())
-	ref, err := fetcher.Fetch(repoURI, version)
+	fetcher, err := NewGoGetFetcher(s.goBinaryName, afero.NewOsFs())
 	r.NoError(err)
+	ref, err := fetcher.Fetch(repoURI, version)
+	r.NoError(err, "fetch shouldn't error")
 	ver, err := ref.Read()
 	r.NoError(err)
 	defer ver.Zip.Close()
@@ -46,7 +57,10 @@ func ExampleFetch() {
 	repoURI := "github.com/arschles/assert"
 	version := "v1.0.0"
 	goBinaryName := env.GoBinPath()
-	fetcher := NewGoGetFetcher(goBinaryName, afero.NewOsFs())
+	fetcher, err := NewGoGetFetcher(goBinaryName, afero.NewOsFs())
+	if err != nil {
+		log.Fatal(err)
+	}
 	ref, err := fetcher.Fetch(repoURI, version)
 	// handle errors if any
 	if err != nil {