Add run variables to cloud plan operations from non-file sources

Run variables allow the run API to accept input variables not found
in the configuration slug (the file-based ones plus workspace vars)

go.mod

@@ -40,9 +40,9 @@ require (
 	github.com/hashicorp/go-hclog v0.15.0
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-plugin v1.4.3
-	github.com/hashicorp/go-retryablehttp v0.5.2
+	github.com/hashicorp/go-retryablehttp v0.7.0
-	github.com/hashicorp/go-tfe v0.19.1-0.20210922134841-a2c1784e9c00
+	github.com/hashicorp/go-tfe v0.19.1-0.20211001235029-ff29186e11db
-	github.com/hashicorp/go-uuid v1.0.1
+	github.com/hashicorp/go-uuid v1.0.2
 	github.com/hashicorp/go-version v1.2.1
 	github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f
 	github.com/hashicorp/hcl/v2 v2.10.1

go.sum

@@ -352,6 +352,7 @@ github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9n
 github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
 github.com/hashicorp/go-getter v1.5.2 h1:XDo8LiAcDisiqZdv0TKgz+HtX3WN7zA2JD1R1tjsabE=
 github.com/hashicorp/go-getter v1.5.2/go.mod h1:orNH3BTYLu/fIxGIdLjLoAJHWMDQ/UKQr5O4m3iBuoo=
+github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
 github.com/hashicorp/go-hclog v0.12.0/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39EeqMAyrmvFQ=
 github.com/hashicorp/go-hclog v0.14.1/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39EeqMAyrmvFQ=
 github.com/hashicorp/go-hclog v0.15.0 h1:qMuK0wxsoW4D0ddCCYwPSTm4KQv1X1ke3WmPWZ0Mvsk=
@@ -367,8 +368,8 @@ github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+l
 github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
 github.com/hashicorp/go-plugin v1.4.3 h1:DXmvivbWD5qdiBts9TpBC7BYL1Aia5sxbRgQB+v6UZM=
 github.com/hashicorp/go-plugin v1.4.3/go.mod h1:5fGEH17QVwTTcR0zV7yhDPLLmFX9YSZ38b18Udy6vYQ=
-github.com/hashicorp/go-retryablehttp v0.5.2 h1:AoISa4P4IsW0/m4T6St8Yw38gTl5GtBAgfkhYh1xAz4=
+github.com/hashicorp/go-retryablehttp v0.7.0 h1:eu1EI/mbirUgP5C8hVsTNaGZreBDlYiwC1FZWkvQPQ4=
-github.com/hashicorp/go-retryablehttp v0.5.2/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es7NDiJgTc6Er0maI1Xs=
+github.com/hashicorp/go-retryablehttp v0.7.0/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
 github.com/hashicorp/go-rootcerts v1.0.2 h1:jzhAVGtqPKbwpyCPELlgNWhE1znq+qwJtW5Oi2viEzc=
 github.com/hashicorp/go-rootcerts v1.0.2/go.mod h1:pqUvnprVnM5bf7AOirdbb01K4ccR319Vf4pU3K5EGc8=
 github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo=
@@ -378,11 +379,12 @@ github.com/hashicorp/go-slug v0.7.0/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu41
 github.com/hashicorp/go-sockaddr v1.0.0 h1:GeH6tui99pF4NJgfnhp+L6+FfobzVW3Ah46sLo0ICXs=
 github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=
 github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4=
-github.com/hashicorp/go-tfe v0.19.1-0.20210922134841-a2c1784e9c00 h1:51ARk47jO4piKzhhbwk6u67ErvSuBj4cu2f2VS9HkgI=
+github.com/hashicorp/go-tfe v0.19.1-0.20211001235029-ff29186e11db h1:e7oSI8NO5bAJBGI8pTxvLkI/0/LMxRaBkJS9hFdKuMk=
-github.com/hashicorp/go-tfe v0.19.1-0.20210922134841-a2c1784e9c00/go.mod h1:U5Iy307L+MazGg0uF8annDtaxAbPp4ElFZ9uPMrjw/I=
+github.com/hashicorp/go-tfe v0.19.1-0.20211001235029-ff29186e11db/go.mod h1:gyXLXbpBVxA2F/6opah8XBsOkZJxHYQmghl0OWi8keI=
 github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
-github.com/hashicorp/go-uuid v1.0.1 h1:fv1ep09latC32wFoVwnqcnKJGnMSdBanPczbHAYm1BE=
 github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
+github.com/hashicorp/go-uuid v1.0.2 h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE=
+github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-version v1.0.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/hashicorp/go-version v1.1.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=

internal/cloud/backend_apply.go

@@ -3,7 +3,6 @@ package cloud
 import (
 	"bufio"
 	"context"
-	"fmt"
 	"io"
 	"log"
 
@@ -59,22 +58,6 @@ func (b *Cloud) opApply(stopCtx, cancelCtx context.Context, op *backend.Operatio
 		))
 	}
 
-	if b.hasExplicitVariableValues(op) {
-		diags = diags.Append(tfdiags.Sourceless(
-			tfdiags.Error,
-			"Run variables are currently not supported",
-			fmt.Sprintf(
-				"Terraform Cloud does not support setting run variables from command line arguments at this time. "+
-					"Currently the only to way to pass variables is by "+
-					"creating a '*.auto.tfvars' variables file. This file will automatically "+
-					"be loaded when the workspace is configured to use "+
-					"Terraform v0.10.0 or later.\n\nAdditionally you can also set variables on "+
-					"the workspace in the web UI:\nhttps://%s/app/%s/%s/variables",
-				b.hostname, b.organization, op.Workspace,
-			),
-		))
-	}
-
 	if !op.HasConfig() && op.PlanMode != plans.DestroyMode {
 		diags = diags.Append(tfdiags.Sourceless(
 			tfdiags.Error,

internal/cloud/backend_apply_test.go

@@ -436,14 +436,15 @@ func TestCloud_applyWithReplace(t *testing.T) {
 	}
 }
 
-func TestCloud_applyWithVariables(t *testing.T) {
+func TestCloud_applyWithRequiredVariables(t *testing.T) {
 	b, bCleanup := testBackendWithName(t)
 	defer bCleanup()
 
 	op, configCleanup, done := testOperationApply(t, "./testdata/apply-variables")
 	defer configCleanup()
+	defer done(t)
 
-	op.Variables = testVariables(terraform.ValueFromNamedFile, "foo", "bar")
+	op.Variables = testVariables(terraform.ValueFromNamedFile, "foo") // "bar" variable value missing
 	op.Workspace = testBackendSingleWorkspaceName
 
 	run, err := b.Operation(context.Background(), op)
@@ -452,14 +453,15 @@ func TestCloud_applyWithVariables(t *testing.T) {
 	}
 
 	<-run.Done()
-	output := done(t)
+	// The usual error of a required variable being missing is deferred and the operation
-	if run.Result == backend.OperationSuccess {
+	// is successful
-		t.Fatal("expected apply operation to fail")
+	if run.Result != backend.OperationSuccess {
+		t.Fatal("expected plan operation to succeed")
 	}
 
-	errOutput := output.Stderr()
+	output := b.CLI.(*cli.MockUi).OutputWriter.String()
-	if !strings.Contains(errOutput, "variables are currently not supported") {
+	if !strings.Contains(output, "Running apply in Terraform Cloud") {
-		t.Fatalf("expected a variables error, got: %v", errOutput)
+		t.Fatalf("unexpected TFC header in output: %s", output)
 	}
 }
 

internal/cloud/backend_common.go

@@ -208,44 +208,6 @@ func (b *Cloud) waitForRun(stopCtx, cancelCtx context.Context, op *backend.Opera
 	}
 }
 
-// hasExplicitVariableValues is a best-effort check to determine whether the
-// user has provided -var or -var-file arguments to a remote operation.
-//
-// The results may be inaccurate if the configuration is invalid or if
-// individual variable values are invalid. That's okay because we only use this
-// result to hint the user to set variables a different way. It's always the
-// remote system's responsibility to do final validation of the input.
-func (b *Cloud) hasExplicitVariableValues(op *backend.Operation) bool {
-	// Load the configuration using the caller-provided configuration loader.
-	config, _, configDiags := op.ConfigLoader.LoadConfigWithSnapshot(op.ConfigDir)
-	if configDiags.HasErrors() {
-		// If we can't load the configuration then we'll assume no explicit
-		// variable values just to let the remote operation start and let
-		// the remote system return the same set of configuration errors.
-		return false
-	}
-
-	// We're intentionally ignoring the diagnostics here because validation
-	// of the variable values is the responsibilty of the remote system. Our
-	// goal here is just to make a best effort count of how many variable
-	// values are coming from -var or -var-file CLI arguments so that we can
-	// hint the user that those are not supported for remote operations.
-	variables, _ := backend.ParseVariableValues(op.Variables, config.Module.Variables)
-
-	// Check for explicitly-defined (-var and -var-file) variables, which the
-	// Terraform Cloud currently does not support. All other source types are okay,
-	// because they are implicit from the execution context anyway and so
-	// their final values will come from the _remote_ execution context.
-	for _, v := range variables {
-		switch v.SourceType {
-		case terraform.ValueFromCLIArg, terraform.ValueFromNamedFile:
-			return true
-		}
-	}
-
-	return false
-}
-
 func (b *Cloud) costEstimate(stopCtx, cancelCtx context.Context, op *backend.Operation, r *tfe.Run) error {
 	if r.CostEstimate == nil {
 		return nil

internal/cloud/backend_plan.go

@@ -64,22 +64,6 @@ func (b *Cloud) opPlan(stopCtx, cancelCtx context.Context, op *backend.Operation
 		))
 	}
 
-	if b.hasExplicitVariableValues(op) {
-		diags = diags.Append(tfdiags.Sourceless(
-			tfdiags.Error,
-			"Run variables are currently not supported",
-			fmt.Sprintf(
-				"Terraform Cloud does not support setting run variables from command line arguments at this time. "+
-					"Currently the only to way to pass variables is by "+
-					"creating a '*.auto.tfvars' variables file. This file will automatically "+
-					"be loaded when the workspace is configured to use "+
-					"Terraform v0.10.0 or later.\n\nAdditionally you can also set variables on "+
-					"the workspace in the web UI:\nhttps://%s/app/%s/%s/variables",
-				b.hostname, b.organization, op.Workspace,
-			),
-		))
-	}
-
 	if !op.HasConfig() && op.PlanMode != plans.DestroyMode {
 		diags = diags.Append(tfdiags.Sourceless(
 			tfdiags.Error,
@@ -241,6 +225,25 @@ in order to capture the filesystem context the remote workspace expects:
 		}
 	}
 
+	config, _, configDiags := op.ConfigLoader.LoadConfigWithSnapshot(op.ConfigDir)
+	if configDiags.HasErrors() {
+		return nil, fmt.Errorf("error loading config with snapshot: %w", configDiags.Errs()[0])
+	}
+	variables, varDiags := ParseCloudRunVariables(op.Variables, config.Module.Variables)
+
+	if varDiags.HasErrors() {
+		return nil, varDiags.Err()
+	}
+
+	runVariables := make([]*tfe.RunVariable, len(variables))
+	for name, value := range variables {
+		runVariables = append(runVariables, &tfe.RunVariable{
+			Key:   name,
+			Value: value,
+		})
+	}
+	runOptions.Variables = runVariables
+
 	r, err := b.client.Runs.Create(stopCtx, runOptions)
 	if err != nil {
 		return r, generalError("Failed to create run", err)

internal/cloud/backend_plan_test.go

@@ -469,14 +469,15 @@ func TestCloud_planWithReplace(t *testing.T) {
 	}
 }
 
-func TestCloud_planWithVariables(t *testing.T) {
+func TestCloud_planWithRequiredVariables(t *testing.T) {
 	b, bCleanup := testBackendWithName(t)
 	defer bCleanup()
 
 	op, configCleanup, done := testOperationPlan(t, "./testdata/plan-variables")
 	defer configCleanup()
+	defer done(t)
 
-	op.Variables = testVariables(terraform.ValueFromCLIArg, "foo", "bar")
+	op.Variables = testVariables(terraform.ValueFromCLIArg, "foo") // "bar" variable value missing
 	op.Workspace = testBackendSingleWorkspaceName
 
 	run, err := b.Operation(context.Background(), op)
@@ -485,14 +486,15 @@ func TestCloud_planWithVariables(t *testing.T) {
 	}
 
 	<-run.Done()
-	output := done(t)
+	// The usual error of a required variable being missing is deferred and the operation
-	if run.Result == backend.OperationSuccess {
+	// is successful
-		t.Fatal("expected plan operation to fail")
+	if run.Result != backend.OperationSuccess {
+		t.Fatal("expected plan operation to succeed")
 	}
 
-	errOutput := output.Stderr()
+	output := b.CLI.(*cli.MockUi).OutputWriter.String()
-	if !strings.Contains(errOutput, "variables are currently not supported") {
+	if !strings.Contains(output, "Running plan in Terraform Cloud") {
-		t.Fatalf("expected a variables error, got: %v", errOutput)
+		t.Fatalf("unexpected TFC header in output: %s", output)
 	}
 }
 

internal/cloud/cloud_variables.go

@@ -0,0 +1,52 @@
+package cloud
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/configs"
+	"github.com/hashicorp/terraform/internal/terraform"
+	"github.com/hashicorp/terraform/internal/tfdiags"
+	ctyjson "github.com/zclconf/go-cty/cty/json"
+)
+
+func allowedSourceType(source terraform.ValueSourceType) bool {
+	return source == terraform.ValueFromNamedFile || source == terraform.ValueFromCLIArg || source == terraform.ValueFromEnvVar
+}
+
+// ParseCloudRunVariables accepts a mapping of unparsed values and a mapping of variable
+// declarations and returns a name/value variable map appropriate for an API run context,
+// that is, containing declared string variables only sourced from non-file inputs like CLI args
+// and environment variables. However, all variable parsing diagnostics are returned
+// in order to allow callers to short circuit cloud runs that contain variable
+// declaration or parsing errors. The only exception is that missing required values are not
+// considered errors because they may be defined within the cloud workspace.
+func ParseCloudRunVariables(vv map[string]backend.UnparsedVariableValue, decls map[string]*configs.Variable) (map[string]string, tfdiags.Diagnostics) {
+	declared, diags := backend.ParseDeclaredVariableValues(vv, decls)
+	_, undedeclaredDiags := backend.ParseUndeclaredVariableValues(vv, decls)
+	diags = diags.Append(undedeclaredDiags)
+
+	ret := make(map[string]string, len(declared))
+
+	// Even if there are parsing or declaration errors, populate the return map with the
+	// variables that could be used for cloud runs
+	for name, v := range declared {
+		if !allowedSourceType(v.SourceType) {
+			continue
+		}
+
+		valueData, err := ctyjson.Marshal(v.Value, v.Value.Type())
+		if err != nil {
+			return nil, diags.Append(fmt.Errorf("error marshaling input variable value as json: %w", err))
+		}
+		var variableValue string
+		if err = json.Unmarshal(valueData, &variableValue); err != nil {
+			// This should never happen since cty marshaled the value to begin with without error
+			return nil, diags.Append(fmt.Errorf("error unmarshaling run variable: %w", err))
+		}
+		ret[name] = variableValue
+	}
+
+	return ret, diags
+}

internal/cloud/cloud_variables_test.go

@@ -0,0 +1,119 @@
+package cloud
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/configs"
+	"github.com/hashicorp/terraform/internal/terraform"
+	"github.com/hashicorp/terraform/internal/tfdiags"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestParseCloudRunVariables(t *testing.T) {
+	t.Run("populates variables from allowed sources", func(t *testing.T) {
+		vv := map[string]backend.UnparsedVariableValue{
+			"undeclared":            testUnparsedVariableValue{source: terraform.ValueFromCLIArg, value: "0"},
+			"declaredFromConfig":    testUnparsedVariableValue{source: terraform.ValueFromConfig, value: "1"},
+			"declaredFromNamedFile": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: "2"},
+			"declaredFromCLIArg":    testUnparsedVariableValue{source: terraform.ValueFromCLIArg, value: "3"},
+			"declaredFromEnvVar":    testUnparsedVariableValue{source: terraform.ValueFromEnvVar, value: "4"},
+		}
+
+		decls := map[string]*configs.Variable{
+			"declaredFromConfig": {
+				Name:           "declaredFromConfig",
+				Type:           cty.String,
+				ConstraintType: cty.String,
+				ParsingMode:    configs.VariableParseLiteral,
+				DeclRange: hcl.Range{
+					Filename: "fake.tf",
+					Start:    hcl.Pos{Line: 2, Column: 1, Byte: 0},
+					End:      hcl.Pos{Line: 2, Column: 1, Byte: 0},
+				},
+			},
+			"declaredFromNamedFile": {
+				Name:           "declaredFromNamedFile",
+				Type:           cty.String,
+				ConstraintType: cty.String,
+				ParsingMode:    configs.VariableParseLiteral,
+				DeclRange: hcl.Range{
+					Filename: "fake.tf",
+					Start:    hcl.Pos{Line: 2, Column: 1, Byte: 0},
+					End:      hcl.Pos{Line: 2, Column: 1, Byte: 0},
+				},
+			},
+			"declaredFromCLIArg": {
+				Name:           "declaredFromCLIArg",
+				Type:           cty.String,
+				ConstraintType: cty.String,
+				ParsingMode:    configs.VariableParseLiteral,
+				DeclRange: hcl.Range{
+					Filename: "fake.tf",
+					Start:    hcl.Pos{Line: 2, Column: 1, Byte: 0},
+					End:      hcl.Pos{Line: 2, Column: 1, Byte: 0},
+				},
+			},
+			"declaredFromEnvVar": {
+				Name:           "declaredFromEnvVar",
+				Type:           cty.String,
+				ConstraintType: cty.String,
+				ParsingMode:    configs.VariableParseLiteral,
+				DeclRange: hcl.Range{
+					Filename: "fake.tf",
+					Start:    hcl.Pos{Line: 2, Column: 1, Byte: 0},
+					End:      hcl.Pos{Line: 2, Column: 1, Byte: 0},
+				},
+			},
+			"missing": {
+				Name:           "missing",
+				Type:           cty.String,
+				ConstraintType: cty.String,
+				Default:        cty.StringVal("2"),
+				ParsingMode:    configs.VariableParseLiteral,
+				DeclRange: hcl.Range{
+					Filename: "fake.tf",
+					Start:    hcl.Pos{Line: 2, Column: 1, Byte: 0},
+					End:      hcl.Pos{Line: 2, Column: 1, Byte: 0},
+				},
+			},
+		}
+		wantVals := make(map[string]string)
+
+		wantVals["declaredFromNamedFile"] = "2"
+		wantVals["declaredFromCLIArg"] = "3"
+		wantVals["declaredFromEnvVar"] = "4"
+
+		gotVals, diags := ParseCloudRunVariables(vv, decls)
+		if diff := cmp.Diff(wantVals, gotVals, cmp.Comparer(cty.Value.RawEquals)); diff != "" {
+			t.Errorf("wrong result\n%s", diff)
+		}
+
+		if got, want := len(diags), 1; got != want {
+			t.Fatalf("expected 1 variable error: %v, got %v", diags.Err(), want)
+		}
+
+		if got, want := diags[0].Description().Summary, "Value for undeclared variable"; got != want {
+			t.Errorf("wrong summary for diagnostic 0\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+}
+
+type testUnparsedVariableValue struct {
+	source terraform.ValueSourceType
+	value  string
+}
+
+func (v testUnparsedVariableValue) ParseVariableValue(mode configs.VariableParsingMode) (*terraform.InputValue, tfdiags.Diagnostics) {
+	return &terraform.InputValue{
+		Value:      cty.StringVal(v.value),
+		SourceType: v.source,
+		SourceRange: tfdiags.SourceRange{
+			Filename: "fake.tfvars",
+			Start:    tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
+			End:      tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
+		},
+	}, nil
+}