Add optional tfe-token flag. Get working with 0.12.

cmd/server.go

@@ -61,6 +61,7 @@ const (
 	SilenceWhitelistErrorsFlag = "silence-whitelist-errors"
 	SSLCertFileFlag            = "ssl-cert-file"
 	SSLKeyFileFlag             = "ssl-key-file"
+	TFETokenFlag               = "tfe-token"
 
 	// Flag defaults.
 	DefaultBitbucketBaseURL = bitbucketcloud.BaseURL
@@ -167,6 +168,12 @@ var stringFlags = []stringFlag{
 		name:        SSLKeyFileFlag,
 		description: fmt.Sprintf("File containing x509 private key matching --%s.", SSLCertFileFlag),
 	},
+	{
+		name: TFETokenFlag,
+		description: "API token for Terraform Enterprise. This will be used to generate a ~/.terraformrc file." +
+			" Only set if using TFE as a backend." +
+			" Should be specified via the ATLANTIS_TFE_TOKEN environment variable for security.",
+	},
 }
 var boolFlags = []boolFlag{
 	{

cmd/server_test.go

@@ -349,6 +349,7 @@ func TestExecute_Defaults(t *testing.T) {
 	Equals(t, false, passedConfig.RequireMergeable)
 	Equals(t, "", passedConfig.SSLCertFile)
 	Equals(t, "", passedConfig.SSLKeyFile)
+	Equals(t, "", passedConfig.TFEToken)
 }
 
 func TestExecute_ExpandHomeInDataDir(t *testing.T) {
@@ -447,6 +448,7 @@ func TestExecute_Flags(t *testing.T) {
 		cmd.RequireMergeableFlag:       true,
 		cmd.SSLCertFileFlag:            "cert-file",
 		cmd.SSLKeyFileFlag:             "key-file",
+		cmd.TFETokenFlag:               "my-token",
 	})
 	err := c.Execute()
 	Ok(t, err)
@@ -474,6 +476,7 @@ func TestExecute_Flags(t *testing.T) {
 	Equals(t, true, passedConfig.RequireMergeable)
 	Equals(t, "cert-file", passedConfig.SSLCertFile)
 	Equals(t, "key-file", passedConfig.SSLKeyFile)
+	Equals(t, "my-token", passedConfig.TFEToken)
 }
 
 func TestExecute_ConfigFile(t *testing.T) {
@@ -502,6 +505,7 @@ require-approval: true
 require-mergeable: true
 ssl-cert-file: cert-file
 ssl-key-file: key-file
+tfe-token: my-token
 `)
 	defer os.Remove(tmpFile) // nolint: errcheck
 	c := setup(map[string]interface{}{
@@ -533,6 +537,7 @@ ssl-key-file: key-file
 	Equals(t, true, passedConfig.RequireMergeable)
 	Equals(t, "cert-file", passedConfig.SSLCertFile)
 	Equals(t, "key-file", passedConfig.SSLKeyFile)
+	Equals(t, "my-token", passedConfig.TFEToken)
 }
 
 func TestExecute_EnvironmentOverride(t *testing.T) {
@@ -560,6 +565,7 @@ repo-whitelist: "github.com/runatlantis/atlantis"
 require-approval: true
 ssl-cert-file: cert-file
 ssl-key-file: key-file
+ssl-key-file: my-token
 `)
 	defer os.Remove(tmpFile) // nolint: errcheck
 
@@ -588,6 +594,7 @@ ssl-key-file: key-file
 		"REQUIRE_MERGEABLE":        "false",
 		"SSL_CERT_FILE":            "override-cert-file",
 		"SSL_KEY_FILE":             "override-key-file",
+		"TFE_TOKEN":                "override-my-token",
 	} {
 		os.Setenv("ATLANTIS_"+name, value) // nolint: errcheck
 	}
@@ -619,6 +626,7 @@ ssl-key-file: key-file
 	Equals(t, false, passedConfig.RequireMergeable)
 	Equals(t, "override-cert-file", passedConfig.SSLCertFile)
 	Equals(t, "override-key-file", passedConfig.SSLKeyFile)
+	Equals(t, "override-my-token", passedConfig.TFEToken)
 }
 
 func TestExecute_FlagConfigOverride(t *testing.T) {
@@ -647,6 +655,7 @@ require-approval: true
 require-mergeable: true
 ssl-cert-file: cert-file
 ssl-key-file: key-file
+tfe-token: my-token
 `)
 
 	defer os.Remove(tmpFile) // nolint: errcheck
@@ -674,6 +683,7 @@ ssl-key-file: key-file
 		cmd.RequireMergeableFlag:       false,
 		cmd.SSLCertFileFlag:            "override-cert-file",
 		cmd.SSLKeyFileFlag:             "override-key-file",
+		cmd.TFETokenFlag:               "override-my-token",
 	})
 	err := c.Execute()
 	Ok(t, err)
@@ -699,6 +709,7 @@ ssl-key-file: key-file
 	Equals(t, false, passedConfig.RequireMergeable)
 	Equals(t, "override-cert-file", passedConfig.SSLCertFile)
 	Equals(t, "override-key-file", passedConfig.SSLKeyFile)
+	Equals(t, "override-my-token", passedConfig.TFEToken)
 }
 
 func TestExecute_FlagEnvVarOverride(t *testing.T) {
@@ -728,6 +739,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) {
 		"REQUIRE_MERGEABLE":        "true",
 		"SSL_CERT_FILE":            "cert-file",
 		"SSL_KEY_FILE":             "key-file",
+		"TFE_TOKEN":                "my-token",
 	}
 	for name, value := range envVars {
 		os.Setenv("ATLANTIS_"+name, value) // nolint: errcheck
@@ -763,6 +775,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) {
 		cmd.RequireMergeableFlag:       false,
 		cmd.SSLCertFileFlag:            "override-cert-file",
 		cmd.SSLKeyFileFlag:             "override-key-file",
+		cmd.TFETokenFlag:               "override-my-token",
 	})
 	err := c.Execute()
 	Ok(t, err)
@@ -790,6 +803,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) {
 	Equals(t, false, passedConfig.RequireMergeable)
 	Equals(t, "override-cert-file", passedConfig.SSLCertFile)
 	Equals(t, "override-key-file", passedConfig.SSLKeyFile)
+	Equals(t, "override-my-token", passedConfig.TFEToken)
 }
 
 // If using bitbucket cloud, webhook secrets are not supported.

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

@@ -23,9 +23,11 @@ import (
 	"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"
-	"github.com/runatlantis/atlantis/server/logging"
 )
 
 // Repo is a VCS repository.
@@ -274,33 +276,33 @@ func (h VCSHostType) String() string {
 }
 
 type ProjectCommandContext struct {
+	// ApplyCmd is the command that users should run to apply this plan. If
+	// this is an apply then this will be empty.
+	ApplyCmd string
 	// BaseRepo is the repository that the pull request will be merged into.
 	BaseRepo Repo
+	// CommentArgs are the extra arguments appended to comment,
+	// ex. atlantis plan -- -target=resource
+	CommentArgs  []string
+	GlobalConfig *valid.Config
 	// HeadRepo is the repository that is getting merged into the BaseRepo.
 	// If the pull request branch is from the same repository then HeadRepo will
 	// be the same as BaseRepo.
 	// See https://help.github.com/articles/about-pull-request-merges/.
-	HeadRepo Repo
-	Pull     PullRequest
-	// User is the user that triggered this command.
-	User          User
+	HeadRepo      Repo
 	Log           *logging.SimpleLogger
-	RepoRelDir    string
+	Pull          PullRequest
 	ProjectConfig *valid.Project
-	GlobalConfig  *valid.Config
-
-	// CommentArgs are the extra arguments appended to comment,
-	// ex. atlantis plan -- -target=resource
-	CommentArgs []string
-	Workspace   string
-	// Verbose is true when the user would like verbose output.
-	Verbose bool
 	// RePlanCmd is the command that users should run to re-plan this project.
 	// If this is an apply then this will be empty.
-	RePlanCmd string
-	// ApplyCmd is the command that users should run to apply this plan. If
-	// this is an apply then this will be empty.
-	ApplyCmd string
+	RePlanCmd        string
+	RepoRelDir       string
+	TerraformVersion *version.Version
+	// User is the user that triggered this command.
+	User User
+	// Verbose is true when the user would like verbose output.
+	Verbose   bool
+	Workspace string
 }
 
 // SplitRepoFullName splits a repo full name up into its owner and repo name

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