From a8d01e3940b406a3c974bfaffd0ca5f534363cc7 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 18 Oct 2019 10:11:18 -0700 Subject: [PATCH] backend/remote: Report invalid variables only remotely The remote backend uses backend.ParseVariableValues locally only to decide if the user seems to be trying to use -var or -var-file options locally, since those are not supported for the remote backend. Other than detecting those, we don't actually have any need to use the results of backend.ParseVariableValues, and so it's better for us to ignore any errors it produces itself and prefer to just send a potentially-invalid request to the remote system and let the remote system be responsible for validating it. This then avoids issues caused by the fact that when remote operations are in use the local system does not have all of the required context: it can't see which environment variables will be set in the remote execution context nor which variables the remote system will set using its own generated -var-file based on the workspace stored variables. --- backend/remote/backend_apply.go | 5 +--- backend/remote/backend_common.go | 43 +++++++++++++++++++------------- backend/remote/backend_plan.go | 5 +--- backend/unparsed_value.go | 5 +++- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/backend/remote/backend_apply.go b/backend/remote/backend_apply.go index 6e187fc34..ef5b1511c 100644 --- a/backend/remote/backend_apply.go +++ b/backend/remote/backend_apply.go @@ -75,10 +75,7 @@ func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operati )) } - variables, parseDiags := b.parseVariableValues(op) - diags = diags.Append(parseDiags) - - if len(variables) > 0 { + if b.hasExplicitVariableValues(op) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Run variables are currently not supported", diff --git a/backend/remote/backend_common.go b/backend/remote/backend_common.go index 930e01646..c2d6602dd 100644 --- a/backend/remote/backend_common.go +++ b/backend/remote/backend_common.go @@ -14,7 +14,6 @@ import ( tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/terraform" - "github.com/hashicorp/terraform/tfdiags" ) var ( @@ -201,32 +200,42 @@ func (b *Remote) waitForRun(stopCtx, cancelCtx context.Context, op *backend.Oper } } -func (b *Remote) parseVariableValues(op *backend.Operation) (terraform.InputValues, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - result := make(terraform.InputValues) - +// 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 *Remote) hasExplicitVariableValues(op *backend.Operation) bool { // Load the configuration using the caller-provided configuration loader. config, _, configDiags := op.ConfigLoader.LoadConfigWithSnapshot(op.ConfigDir) - diags = diags.Append(configDiags) - if diags.HasErrors() { - return nil, diags + 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 } - variables, varDiags := backend.ParseVariableValues(op.Variables, config.Module.Variables) - diags = diags.Append(varDiags) - if diags.HasErrors() { - return nil, diags - } + // 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) - // Save only the explicitly defined variables. - for k, v := range variables { + // Check for explicitly-defined (-var and -var-file) variables, which the + // remote backend 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: - result[k] = v + return true } } - return result, diags + return false } func (b *Remote) costEstimate(stopCtx, cancelCtx context.Context, op *backend.Operation, r *tfe.Run) error { diff --git a/backend/remote/backend_plan.go b/backend/remote/backend_plan.go index f4a3d7118..c73c63f18 100644 --- a/backend/remote/backend_plan.go +++ b/backend/remote/backend_plan.go @@ -78,10 +78,7 @@ func (b *Remote) opPlan(stopCtx, cancelCtx context.Context, op *backend.Operatio )) } - variables, parseDiags := b.parseVariableValues(op) - diags = diags.Append(parseDiags) - - if len(variables) > 0 { + if b.hasExplicitVariableValues(op) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Run variables are currently not supported", diff --git a/backend/unparsed_value.go b/backend/unparsed_value.go index a1f0468a0..1b2318c97 100644 --- a/backend/unparsed_value.go +++ b/backend/unparsed_value.go @@ -37,7 +37,10 @@ type UnparsedVariableValue interface { // // If this function returns without any errors in the diagnostics, the // resulting input values map is guaranteed to be valid and ready to pass -// to terraform.NewContext. +// to terraform.NewContext. If the diagnostics contains errors, the returned +// InputValues may be incomplete but will include the subset of variables +// that were successfully processed, allowing for careful analysis of the +// partial result. func ParseVariableValues(vv map[string]UnparsedVariableValue, decls map[string]*configs.Variable) (terraform.InputValues, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics ret := make(terraform.InputValues, len(vv))