Don't add extra -var flags when using TF >= 0.12

In Terraform 0.12 and above, you aren't allowed to set -var foo=bar
flags unless the foo variable is actually defined in code. Previously,
we were setting the following variables:
- atlantis_user
- atlantis_repo
- atlantis_repo_owner
- atlantis_repo_name
- atlantis_pull_num

Users could then use these variables if they wanted to, in their code.
The main use case was to name the assume role session in AWS:

provider "aws" {
  assume_role {
    role_arn     = "arn:aws:iam::ACCOUNT_ID:role/ROLE_NAME"
    session_name = "${var.atlantis_user}-${var.atlantis_repo_owner}-${var.atlantis_repo_name}-${var.atlantis_pull_num}"
  }
}

This is longer possible in 0.12.

runatlantis.io/docs/provider-credentials.md

@@ -21,7 +21,7 @@ won't work for multiple accounts since Atlantis wouldn't know which environment
 Terraform with.
 
 ### Assume Role Session Names
-Atlantis injects 5 Terraform variables that can be used to dynamically name the assume role session name.
+If you're using Terraform < 0.12, Atlantis injects 5 Terraform variables that can be used to dynamically name the assume role session name.
 Setting the `session_name` allows you to trace API calls made through Atlantis back to a specific
 user and repo via CloudWatch:
 
@@ -59,3 +59,11 @@ terraform {
   }
 }
 ```
+
+:::tip Why does this not work in TF >= 0.12?
+In Terraform >= 0.12, you're not allowed to set any `-var` flags if those variables
+aren't being used. Since we can't know if you're using these `atlantis_*` variables,
+we can't set the `-var` flag.
+
+You can still set these variables yourself using the `extra_args` configuration.
+:::

server/events/models/models.go

@@ -18,13 +18,14 @@ package models
 
 import (
 	"fmt"
-	"github.com/hashicorp/go-version"
-	"github.com/runatlantis/atlantis/server/logging"
 	"net/url"
 	paths "path"
 	"strings"
 	"time"
 
+	"github.com/hashicorp/go-version"
+	"github.com/runatlantis/atlantis/server/logging"
+
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/events/yaml/valid"
 )

server/events/runtime/plan_step_runner.go

@@ -36,7 +36,7 @@ func (p *PlanStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin
 		return "", err
 	}
 
-	planCmd := p.buildPlanCmd(ctx, extraArgs, path)
+	planCmd := p.buildPlanCmd(ctx, extraArgs, path, tfVersion)
 	output, err := p.TerraformExecutor.RunCommandWithVersion(ctx.Log, filepath.Clean(path), planCmd, tfVersion, ctx.Workspace)
 	if err != nil {
 		return output, err
@@ -94,8 +94,8 @@ func (p *PlanStepRunner) switchWorkspace(ctx models.ProjectCommandContext, path
 	return nil
 }
 
-func (p *PlanStepRunner) buildPlanCmd(ctx models.ProjectCommandContext, extraArgs []string, path string) []string {
-	tfVars := p.tfVars(ctx)
+func (p *PlanStepRunner) buildPlanCmd(ctx models.ProjectCommandContext, extraArgs []string, path string, tfVersion *version.Version) []string {
+	tfVars := p.tfVars(ctx, tfVersion)
 	planFile := filepath.Join(path, GetPlanFilename(ctx.Workspace, ctx.ProjectConfig))
 
 	// Check if env/{workspace}.tfvars exist and include it. This is a use-case
@@ -125,7 +125,15 @@ func (p *PlanStepRunner) buildPlanCmd(ctx models.ProjectCommandContext, extraArg
 // repo this command is running for. This can be used for naming the
 // session name in AWS which will identify in CloudTrail the source of
 // Atlantis API calls.
-func (p *PlanStepRunner) tfVars(ctx models.ProjectCommandContext) []string {
+// If using Terraform >= 0.12 we don't set any of these variables because
+// those versions don't allow setting -var flags for any variables that aren't
+// actually used in the configuration. Since there's no way for us to detect
+// if the configuration is using those variables, we don't set them.
+func (p *PlanStepRunner) tfVars(ctx models.ProjectCommandContext, tfVersion *version.Version) []string {
+	if vTwelveAndUp.Check(tfVersion) {
+		return nil
+	}
+
 	// NOTE: not using maps and looping here because we need to keep the
 	// ordering for testing purposes.
 	// NOTE: quoting the values because in Bitbucket the owner can have
@@ -171,3 +179,5 @@ func (p *PlanStepRunner) fmtPlanOutput(output string) string {
 	output = tildeDiffRegex.ReplaceAllString(output, "~")
 	return minusDiffRegex.ReplaceAllString(output, "-")
 }
+
+var vTwelveAndUp = MustConstraint(">=0.12-a")

server/events/runtime/plan_step_runner_test.go

@@ -606,6 +606,57 @@ func TestRun_OutputOnErr(t *testing.T) {
 	Equals(t, expOutput, actOutput)
 }
 
+// Test that if we're using 0.12, we don't set the optional -var atlantis_repo_name
+// flags because in >= 0.12 you can't set -var flags if those variables aren't
+// being used.
+func TestRun_NoOptionalVarsIn012(t *testing.T) {
+	RegisterMockTestingT(t)
+	terraform := mocks.NewMockClient()
+
+	tfVersion, _ := version.NewVersion("0.12.0")
+	s := runtime.PlanStepRunner{
+		TerraformExecutor: terraform,
+		DefaultTFVersion:  tfVersion,
+	}
+
+	When(terraform.RunCommandWithVersion(
+		matchers.AnyPtrToLoggingSimpleLogger(),
+		AnyString(),
+		AnyStringSlice(),
+		matchers2.AnyPtrToGoVersionVersion(),
+		AnyString())).ThenReturn("output", nil)
+
+	output, err := s.Run(models.ProjectCommandContext{
+		Workspace:   "default",
+		RepoRelDir:  ".",
+		User:        models.User{Username: "username"},
+		CommentArgs: []string{"comment", "args"},
+		Pull: models.PullRequest{
+			Num: 2,
+		},
+		BaseRepo: models.Repo{
+			FullName: "owner/repo",
+			Owner:    "owner",
+			Name:     "repo",
+		},
+	}, []string{"extra", "args"}, "/path")
+	Ok(t, err)
+	Equals(t, "output", output)
+
+	expPlanArgs := []string{"plan",
+		"-input=false",
+		"-refresh",
+		"-no-color",
+		"-out",
+		fmt.Sprintf("%q", "/path/default.tfplan"),
+		"extra",
+		"args",
+		"comment",
+		"args",
+	}
+	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, "/path", expPlanArgs, tfVersion, "default")
+}
+
 func stringSliceEquals(a, b []string) bool {
 	if len(a) != len(b) {
 		return false

server/events/terraform/terraform_client.go

@@ -16,14 +16,15 @@ package terraform
 
 import (
 	"fmt"
-	"github.com/mitchellh/go-homedir"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"regexp"
 	"strings"
 
+	"github.com/mitchellh/go-homedir"
+
 	"github.com/hashicorp/go-version"
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/logging"
@@ -100,22 +101,25 @@ func NewClient(dataDir string, tfeToken string) (*DefaultClient, error) {
 func generateRCFile(tfeToken string, home string) error {
 	const rcFilename = ".terraformrc"
 	rcFile := filepath.Join(home, rcFilename)
+	config := fmt.Sprintf(rcFileContents, tfeToken)
 
 	// If there is already a .terraformrc file and its contents aren't exactly
 	// what we would have written to it, then we error out because we don't
 	// want to overwrite anything.
-	newContents := fmt.Sprintf(rcFileContents, tfeToken)
 	if _, err := os.Stat(rcFile); err == nil {
-		currContents, err := ioutil.ReadFile(rcFile)
+		currContents, err := ioutil.ReadFile(rcFile) // nolint: gosec
 		if err != nil {
 			return errors.Wrapf(err, "trying to read %s to ensure we're not overwriting it", rcFile)
 		}
-		if newContents != string(currContents) {
+		if config != string(currContents) {
 			return fmt.Errorf("can't write TFE token to %s because that file has contents that would be overwritten", rcFile)
 		}
+		// Otherwise we don't need to write the file because it already has
+		// what we need.
+		return nil
 	}
 
-	if err := ioutil.WriteFile(rcFile, []byte(newContents), 0600); err != nil {
+	if err := ioutil.WriteFile(rcFile, []byte(config), 0600); err != nil {
 		return errors.Wrapf(err, "writing generated %s file with TFE token to %s", rcFilename, rcFile)
 	}
 	return nil

server/events/terraform/terraform_client_internal_test.go

@@ -2,10 +2,11 @@ package terraform
 
 import (
 	"fmt"
-	. "github.com/runatlantis/atlantis/testing"
 	"io/ioutil"
 	"path/filepath"
 	"testing"
+
+	. "github.com/runatlantis/atlantis/testing"
 )
 
 // Test that we write the file as expected

server/events_controller_e2e_test.go

@@ -273,7 +273,7 @@ func setupE2E(t *testing.T) (server.EventsController, *vcsmocks.MockClientProxy,
 		GitlabUser:  "gitlab-user",
 		GitlabToken: "gitlab-token",
 	}
-	terraformClient, err := terraform.NewClient(dataDir)
+	terraformClient, err := terraform.NewClient(dataDir, "")
 	Ok(t, err)
 	boltdb, err := boltdb.New(dataDir)
 	Ok(t, err)