Refactor -tf-download-url flag changes.

* Fix bad ordering of args calling NewClient()
* Make error messages clearer

cmd/server.go

@@ -205,7 +205,7 @@ var stringFlags = map[string]stringFlag{
 		description: fmt.Sprintf("File containing x509 private key matching --%s.", SSLCertFileFlag),
 	},
 	TFDownloadURLFlag: {
-		description:  "URL to download Terraform from.",
+		description:  "Base URL to download Terraform versions from.",
 		defaultValue: DefaultTFDownloadURL,
 	},
 	TFEHostnameFlag: {

server/events/terraform/terraform_client.go

@@ -148,7 +148,7 @@ func NewClient(
 			_, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL)
 			versionsLock.Unlock()
 			if err != nil {
-				log.Err("could not download terraform %s", defaultVersion.String())
+				log.Err("could not download terraform %s: %s", defaultVersion.String(), err)
 			}
 		}()
 	}
@@ -416,8 +416,9 @@ func ensureVersion(log *logging.SimpleLogger, dl Downloader, versions map[string
 	urlPrefix := fmt.Sprintf("%s/terraform/%s/terraform_%s", downloadURL, v.String(), v.String())
 	binURL := fmt.Sprintf("%s_%s_%s.zip", urlPrefix, runtime.GOOS, runtime.GOARCH)
 	checksumURL := fmt.Sprintf("%s_SHA256SUMS", urlPrefix)
-	if err := dl.GetFile(dest, fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL)); err != nil {
-		return "", errors.Wrapf(err, "downloading terraform version %s", v.String())
+	fullSrcURL := fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL)
+	if err := dl.GetFile(dest, fullSrcURL); err != nil {
+		return "", errors.Wrapf(err, "downloading terraform version %s at %q", v.String(), fullSrcURL)
 	}
 
 	log.Info("downloaded terraform %s to %s", v.String(), dest)

server/events/terraform/terraform_client_test.go

@@ -68,7 +68,7 @@ is 0.11.13. You can update by downloading from www.terraform.io/downloads.html
 	Ok(t, err)
 	defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))()
 
-	c, err := terraform.NewClient(nil, tmp, "", "", "", cmd.DefaultTFVersionFlag, "https://releases.hashicorp.com", nil)
+	c, err := terraform.NewClient(nil, tmp, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil)
 	Ok(t, err)
 
 	Ok(t, err)
@@ -96,7 +96,7 @@ is 0.11.13. You can update by downloading from www.terraform.io/downloads.html
 	Ok(t, err)
 	defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))()
 
-	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, nil)
+	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil)
 	Ok(t, err)
 
 	Ok(t, err)
@@ -116,7 +116,7 @@ func TestNewClient_NoTF(t *testing.T) {
 	// Set PATH to only include our empty directory.
 	defer tempSetEnv(t, "PATH", tmp)()
 
-	_, err := terraform.NewClient(nil, tmp, "", "", "", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, nil)
+	_, err := terraform.NewClient(nil, tmp, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil)
 	ErrEquals(t, "terraform not found in $PATH. Set --default-tf-version or download terraform from https://www.terraform.io/downloads.html", err)
 }
 
@@ -133,7 +133,7 @@ func TestNewClient_DefaultTFFlagInPath(t *testing.T) {
 	Ok(t, err)
 	defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))()
 
-	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, nil)
+	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil)
 	Ok(t, err)
 
 	Ok(t, err)
@@ -157,7 +157,7 @@ func TestNewClient_DefaultTFFlagInBinDir(t *testing.T) {
 	Ok(t, err)
 	defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))()
 
-	c, err := terraform.NewClient(logging.NewNoopLogger(), tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, nil)
+	c, err := terraform.NewClient(logging.NewNoopLogger(), tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil)
 	Ok(t, err)
 
 	Ok(t, err)
@@ -207,7 +207,7 @@ func TestNewClient_DefaultTFFlagDownload(t *testing.T) {
 func TestNewClient_BadVersion(t *testing.T) {
 	tmp, cleanup := TempDir(t)
 	defer cleanup()
-	_, err := terraform.NewClient(nil, tmp, "", "", "malformed", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, nil)
+	_, err := terraform.NewClient(nil, tmp, "", "", "malformed", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil)
 	ErrEquals(t, "Malformed version: malformed", err)
 }
 
@@ -219,7 +219,7 @@ func TestRunCommandWithVersion_DLsTF(t *testing.T) {
 
 	mockDownloader := mocks.NewMockDownloader()
 	// Set up our mock downloader to write a fake tf binary when it's called.
-	baseURL := fmt.Sprintf("%s/terraform/99.99.99", cmd.TFDownloadURLFlag)
+	baseURL := fmt.Sprintf("%s/terraform/99.99.99", cmd.DefaultTFDownloadURL)
 	expURL := fmt.Sprintf("%s/terraform_99.99.99_%s_%s.zip?checksum=file:%s/terraform_99.99.99_SHA256SUMS",
 		baseURL,
 		runtime.GOOS,
@@ -230,7 +230,7 @@ func TestRunCommandWithVersion_DLsTF(t *testing.T) {
 		return []pegomock.ReturnValue{err}
 	})
 
-	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, mockDownloader)
+	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader)
 	Ok(t, err)
 	Equals(t, "0.11.10", c.DefaultVersion().String())
 
@@ -249,7 +249,7 @@ func TestEnsureVersion_downloaded(t *testing.T) {
 
 	mockDownloader := mocks.NewMockDownloader()
 
-	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.TFDownloadURLFlag, mockDownloader)
+	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader)
 	Ok(t, err)
 
 	Equals(t, "0.11.10", c.DefaultVersion().String())
@@ -261,7 +261,7 @@ func TestEnsureVersion_downloaded(t *testing.T) {
 
 	Ok(t, err)
 
-	baseURL := fmt.Sprintf("%s/terraform/99.99.99", cmd.TFDownloadURLFlag)
+	baseURL := fmt.Sprintf("%s/terraform/99.99.99", cmd.DefaultTFDownloadURL)
 	expURL := fmt.Sprintf("%s/terraform_99.99.99_%s_%s.zip?checksum=file:%s/terraform_99.99.99_SHA256SUMS",
 		baseURL,
 		runtime.GOOS,

server/events_controller_e2e_test.go

@@ -400,7 +400,7 @@ func setupE2E(t *testing.T, repoDir string) (server.EventsController, *vcsmocks.
 		GithubUser: "github-user",
 		GitlabUser: "gitlab-user",
 	}
-	terraformClient, err := terraform.NewClient(logger, dataDir, "", "", "", "tfdownloadurl", "default-tf-version", &NoopTFDownloader{})
+	terraformClient, err := terraform.NewClient(logger, dataDir, "", "", "", "default-tf-version", "https://releases.hashicorp.com", &NoopTFDownloader{})
 	Ok(t, err)
 	boltdb, err := db.New(dataDir)
 	Ok(t, err)

server/server.go

@@ -211,9 +211,9 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		userConfig.DataDir,
 		userConfig.TFEToken,
 		userConfig.TFEHostname,
-		userConfig.TFDownloadURL,
 		userConfig.DefaultTFVersion,
 		config.DefaultTFVersionFlag,
+		userConfig.TFDownloadURL,
 		&terraform.DefaultDownloader{})
 	// The flag.Lookup call is to detect if we're running in a unit test. If we
 	// are, then we don't error out because we don't have/want terraform