Merge pull request #722 from maximede/issue-675

Ensure terraform_version is downloaded when using non built-in step. Fixes #675

.gitignore

@@ -11,3 +11,4 @@ node_modules/
 **/.vuepress/dist
 helm/test-values.yaml
 *.swp
+golangci-lint

server/events/runtime/run_step_runner.go

@@ -13,21 +13,31 @@ import (
 
 // RunStepRunner runs custom commands.
 type RunStepRunner struct {
-	DefaultTFVersion *version.Version
+	TerraformExecutor TerraformExec
+	DefaultTFVersion  *version.Version
 	// TerraformBinDir is the directory where Atlantis downloads Terraform binaries.
 	TerraformBinDir string
 }
 
 func (r *RunStepRunner) Run(ctx models.ProjectCommandContext, command string, path string) (string, error) {
-	cmd := exec.Command("sh", "-c", command) // #nosec
-	cmd.Dir = path
-	tfVersion := r.DefaultTFVersion.String()
+	tfVersion := r.DefaultTFVersion
 	if ctx.TerraformVersion != nil {
-		tfVersion = ctx.TerraformVersion.String()
+		tfVersion = ctx.TerraformVersion
 	}
+
+	err := r.TerraformExecutor.EnsureVersion(ctx.Log, tfVersion)
+	if err != nil {
+		err = fmt.Errorf("%s: Downloading terraform Version %s", err, tfVersion.String())
+		ctx.Log.Debug("error: %s", err)
+		return "", err
+	}
+
+	cmd := exec.Command("sh", "-c", command) // #nosec
+	cmd.Dir = path
+
 	baseEnvVars := os.Environ()
 	customEnvVars := map[string]string{
-		"ATLANTIS_TERRAFORM_VERSION": tfVersion,
+		"ATLANTIS_TERRAFORM_VERSION": tfVersion.String(),
 		"BASE_BRANCH_NAME":           ctx.Pull.BaseBranch,
 		"BASE_REPO_NAME":             ctx.BaseRepo.Name,
 		"BASE_REPO_OWNER":            ctx.BaseRepo.Owner,

server/events/runtime/run_step_runner_test.go

@@ -6,9 +6,13 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/hashicorp/go-version"
+	version "github.com/hashicorp/go-version"
+	. "github.com/petergtz/pegomock"
+	"github.com/runatlantis/atlantis/server/events/mocks/matchers"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/runtime"
+	"github.com/runatlantis/atlantis/server/events/terraform/mocks"
+	matchers2 "github.com/runatlantis/atlantis/server/events/terraform/mocks/matchers"
 	"github.com/runatlantis/atlantis/server/logging"
 	. "github.com/runatlantis/atlantis/testing"
 )
@@ -19,14 +23,17 @@ func TestRunStepRunner_Run(t *testing.T) {
 		ProjectName string
 		ExpOut      string
 		ExpErr      string
+		Version     string
 	}{
 		{
 			Command: "",
 			ExpOut:  "",
+			Version: "v1.2.3",
 		},
 		{
 			Command: "echo hi",
 			ExpOut:  "hi\n",
+			Version: "v2.3.4",
 		},
 		{
 			Command: `printf \'your main.tf file does not provide default region.\\ncheck\'`,
@@ -74,14 +81,34 @@ func TestRunStepRunner_Run(t *testing.T) {
 		},
 	}
 
-	projVersion, err := version.NewVersion("v0.11.0")
-	Ok(t, err)
-	defaultVersion, _ := version.NewVersion("0.8")
-	r := runtime.RunStepRunner{
-		DefaultTFVersion: defaultVersion,
-		TerraformBinDir:  "/bin/dir",
-	}
 	for _, c := range cases {
+
+		var projVersion *version.Version
+		var err error
+
+		projVersion, err = version.NewVersion("v0.11.0")
+
+		if c.Version != "" {
+			projVersion, err = version.NewVersion(c.Version)
+			Ok(t, err)
+		}
+
+		Ok(t, err)
+
+		defaultVersion, _ := version.NewVersion("0.8")
+
+		RegisterMockTestingT(t)
+		terraform := mocks.NewMockClient()
+		When(terraform.EnsureVersion(matchers.AnyPtrToLoggingSimpleLogger(), matchers2.AnyPtrToGoVersionVersion())).
+			ThenReturn(nil)
+
+		logger := logging.NewNoopLogger()
+
+		r := runtime.RunStepRunner{
+			TerraformExecutor: terraform,
+			DefaultTFVersion:  defaultVersion,
+			TerraformBinDir:   "/bin/dir",
+		}
 		t.Run(c.Command, func(t *testing.T) {
 			tmpDir, cleanup := TempDir(t)
 			defer cleanup()
@@ -103,7 +130,7 @@ func TestRunStepRunner_Run(t *testing.T) {
 				User: models.User{
 					Username: "acme-user",
 				},
-				Log:                logging.NewNoopLogger(),
+				Log:                logger,
 				Workspace:          "myworkspace",
 				RepoRelDir:         "mydir",
 				TerraformVersion:   projVersion,
@@ -121,6 +148,10 @@ func TestRunStepRunner_Run(t *testing.T) {
 			// temp dir.
 			expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1)
 			Equals(t, expOut, out)
+
+			terraform.VerifyWasCalledOnce().EnsureVersion(logger, projVersion)
+			terraform.VerifyWasCalled(Never()).EnsureVersion(logger, defaultVersion)
+
 		})
 	}
 }

server/events/runtime/runtime.go

@@ -25,6 +25,7 @@ const (
 // without causing circular imports.
 type TerraformExec interface {
 	RunCommandWithVersion(log *logging.SimpleLogger, path string, args []string, v *version.Version, workspace string) (string, error)
+	EnsureVersion(log *logging.SimpleLogger, v *version.Version) error
 }
 
 // AsyncTFExec brings the interface from TerraformClient into this package

server/events/terraform/terraform_client.go

@@ -41,6 +41,9 @@ type Client interface {
 	// it will use the default Terraform version. workspace is the Terraform
 	// workspace which should be set as an environment variable.
 	RunCommandWithVersion(log *logging.SimpleLogger, path string, args []string, v *version.Version, workspace string) (string, error)
+
+	// EnsureVersion makes sure that terraform version `v` is available to use
+	EnsureVersion(log *logging.SimpleLogger, v *version.Version) error
 }
 
 type DefaultClient struct {
@@ -189,6 +192,23 @@ func (c *DefaultClient) TerraformBinDir() string {
 	return c.binDir
 }
 
+// See Client.EnsureVersion.
+func (c *DefaultClient) EnsureVersion(log *logging.SimpleLogger, v *version.Version) error {
+	if v == nil {
+		v = c.defaultVersion
+	}
+
+	var err error
+	c.versionsLock.Lock()
+	_, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir)
+	c.versionsLock.Unlock()
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // See Client.RunCommandWithVersion.
 func (c *DefaultClient) RunCommandWithVersion(log *logging.SimpleLogger, path string, args []string, v *version.Version, workspace string) (string, error) {
 	tfCmd, cmd, err := c.prepCmd(log, v, workspace, path, args)

server/events/terraform/terraform_client_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/runatlantis/atlantis/cmd"
 	"github.com/runatlantis/atlantis/server/events/terraform"
 	"github.com/runatlantis/atlantis/server/events/terraform/mocks"
+	"github.com/runatlantis/atlantis/server/logging"
 	. "github.com/runatlantis/atlantis/testing"
 )
 
@@ -156,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(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, nil)
+	c, err := terraform.NewClient(logging.NewNoopLogger(), tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, nil)
 	Ok(t, err)
 
 	Ok(t, err)
@@ -240,6 +241,35 @@ func TestRunCommandWithVersion_DLsTF(t *testing.T) {
 	Equals(t, "\nTerraform v99.99.99\n\n", output)
 }
 
+// Test the EnsureVersion downloads terraform.
+func TestEnsureVersion_downloaded(t *testing.T) {
+	RegisterMockTestingT(t)
+	tmp, cleanup := TempDir(t)
+	defer cleanup()
+
+	mockDownloader := mocks.NewMockDownloader()
+
+	c, err := terraform.NewClient(nil, tmp, "", "", "0.11.10", cmd.DefaultTFVersionFlag, mockDownloader)
+	Ok(t, err)
+
+	Equals(t, "0.11.10", c.DefaultVersion().String())
+
+	v, err := version.NewVersion("99.99.99")
+	Ok(t, err)
+
+	err = c.EnsureVersion(nil, v)
+
+	Ok(t, err)
+
+	baseURL := "https://releases.hashicorp.com/terraform/99.99.99"
+	expURL := fmt.Sprintf("%s/terraform_99.99.99_%s_%s.zip?checksum=file:%s/terraform_99.99.99_SHA256SUMS",
+		baseURL,
+		runtime.GOOS,
+		runtime.GOARCH,
+		baseURL)
+	mockDownloader.VerifyWasCalledEventually(Once(), 2*time.Second).GetFile(filepath.Join(tmp, "bin", "terraform99.99.99"), expURL)
+}
+
 // tempSetEnv sets env var key to value. It returns a function that when called
 // will reset the env var to its original value.
 func tempSetEnv(t *testing.T, key string, value string) func() {

server/events_controller_e2e_test.go

@@ -438,7 +438,8 @@ func setupE2E(t *testing.T, repoDir string) (server.EventsController, *vcsmocks.
 				TerraformExecutor: terraformClient,
 			},
 			RunStepRunner: &runtime.RunStepRunner{
-				DefaultTFVersion: defaultTFVersion,
+				TerraformExecutor: terraformClient,
+				DefaultTFVersion:  defaultTFVersion,
 			},
 			PullApprovedChecker: e2eVCSClient,
 			WorkingDir:          workingDir,

server/server.go

@@ -289,8 +289,9 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 				AsyncTFExec:         terraformClient,
 			},
 			RunStepRunner: &runtime.RunStepRunner{
-				DefaultTFVersion: defaultTfVersion,
-				TerraformBinDir:  terraformClient.TerraformBinDir(),
+				TerraformExecutor: terraformClient,
+				DefaultTFVersion:  defaultTfVersion,
+				TerraformBinDir:   terraformClient.TerraformBinDir(),
 			},
 			PullApprovedChecker: vcsClient,
 			WorkingDir:          workingDir,