From dd856f8a1b80bbc8e60aa172339ac8acc312a833 Mon Sep 17 00:00:00 2001 From: Omar Ismail Date: Mon, 23 Aug 2021 13:35:54 -0400 Subject: [PATCH] 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 | 6 +- go.sum | 12 +-- internal/cloud/backend_apply.go | 17 ---- internal/cloud/backend_apply_test.go | 18 ++-- internal/cloud/backend_common.go | 38 -------- internal/cloud/backend_plan.go | 35 ++++---- internal/cloud/backend_plan_test.go | 18 ++-- internal/cloud/cloud_variables.go | 52 +++++++++++ internal/cloud/cloud_variables_test.go | 119 +++++++++++++++++++++++++ 9 files changed, 220 insertions(+), 95 deletions(-) create mode 100644 internal/cloud/cloud_variables.go create mode 100644 internal/cloud/cloud_variables_test.go diff --git a/go.mod b/go.mod index 9910399a3..5c09bc4b6 100644 --- a/go.mod +++ b/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-tfe v0.19.1-0.20210922134841-a2c1784e9c00 - github.com/hashicorp/go-uuid v1.0.1 + github.com/hashicorp/go-retryablehttp v0.7.0 + github.com/hashicorp/go-tfe v0.19.1-0.20211001235029-ff29186e11db + 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 diff --git a/go.sum b/go.sum index 4b6b9daa3..8972a8d9e 100644 --- a/go.sum +++ b/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.5.2/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es7NDiJgTc6Er0maI1Xs= +github.com/hashicorp/go-retryablehttp v0.7.0 h1:eu1EI/mbirUgP5C8hVsTNaGZreBDlYiwC1FZWkvQPQ4= +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.20210922134841-a2c1784e9c00/go.mod h1:U5Iy307L+MazGg0uF8annDtaxAbPp4ElFZ9uPMrjw/I= +github.com/hashicorp/go-tfe v0.19.1-0.20211001235029-ff29186e11db h1:e7oSI8NO5bAJBGI8pTxvLkI/0/LMxRaBkJS9hFdKuMk= +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= diff --git a/internal/cloud/backend_apply.go b/internal/cloud/backend_apply.go index 65f51ff6c..85c447e60 100644 --- a/internal/cloud/backend_apply.go +++ b/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, diff --git a/internal/cloud/backend_apply_test.go b/internal/cloud/backend_apply_test.go index ad01c0d43..0151f39bc 100644 --- a/internal/cloud/backend_apply_test.go +++ b/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) - if run.Result == backend.OperationSuccess { - t.Fatal("expected apply operation to fail") + // The usual error of a required variable being missing is deferred and the operation + // is successful + if run.Result != backend.OperationSuccess { + t.Fatal("expected plan operation to succeed") } - errOutput := output.Stderr() - if !strings.Contains(errOutput, "variables are currently not supported") { - t.Fatalf("expected a variables error, got: %v", errOutput) + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, "Running apply in Terraform Cloud") { + t.Fatalf("unexpected TFC header in output: %s", output) } } diff --git a/internal/cloud/backend_common.go b/internal/cloud/backend_common.go index 92f28a1dd..2ca3fc1a2 100644 --- a/internal/cloud/backend_common.go +++ b/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 diff --git a/internal/cloud/backend_plan.go b/internal/cloud/backend_plan.go index 5455ec06e..4abb22ee3 100644 --- a/internal/cloud/backend_plan.go +++ b/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) diff --git a/internal/cloud/backend_plan_test.go b/internal/cloud/backend_plan_test.go index 3a6d2e74b..6c08c3762 100644 --- a/internal/cloud/backend_plan_test.go +++ b/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) - if run.Result == backend.OperationSuccess { - t.Fatal("expected plan operation to fail") + // The usual error of a required variable being missing is deferred and the operation + // is successful + if run.Result != backend.OperationSuccess { + t.Fatal("expected plan operation to succeed") } - errOutput := output.Stderr() - if !strings.Contains(errOutput, "variables are currently not supported") { - t.Fatalf("expected a variables error, got: %v", errOutput) + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, "Running plan in Terraform Cloud") { + t.Fatalf("unexpected TFC header in output: %s", output) } } diff --git a/internal/cloud/cloud_variables.go b/internal/cloud/cloud_variables.go new file mode 100644 index 000000000..9a9002828 --- /dev/null +++ b/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 +} diff --git a/internal/cloud/cloud_variables_test.go b/internal/cloud/cloud_variables_test.go new file mode 100644 index 000000000..f8a8a5f1c --- /dev/null +++ b/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 +}