Skip to content

Commit

Permalink
Refactor -tf-download-url flag changes.
Browse files Browse the repository at this point in the history 
* Fix bad ordering of args calling NewClient()
* Make error messages clearer
  • Loading branch information
@lkysow
lkysow committed Jan 20, 2020
1 parent 3b21f31 commit 23c4aab
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 16 deletions.
2 changes: 1 addition & 1 deletion cmd/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
7 changes: 4 additions & 3 deletions server/events/terraform/terraform_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}()
}
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions server/events/terraform/terraform_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}

Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand All @@ -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,
Expand All @@ -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())

Expand All @@ -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())
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion server/events_controller_e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 23c4aab

Please sign in to comment.