From 36b5207d2bd35ebefc2292999e523f78dd12c78d Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 13 Mar 2026 15:47:18 +0100 Subject: [PATCH] Still require const variables when AllowUnsetVariables is set We never want to stub const variables, so we will always try to get values for them. The cloud backend now always fetches variable values so const vars can be satisfied. --- internal/backend/local/backend_local.go | 6 +- internal/cloud/backend_context.go | 138 ++++++++++++------------ 2 files changed, 69 insertions(+), 75 deletions(-) diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 768b94355a..90f63a6f25 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -496,8 +496,8 @@ func (b *Local) interactiveCollectVariables(ctx context.Context, existing map[st func (b *Local) stubUnsetRequiredVariables(existing map[string]arguments.UnparsedVariableValue, vcs map[string]*configs.Variable) map[string]arguments.UnparsedVariableValue { var missing bool // Do we need to add anything? for name, vc := range vcs { - if !vc.Required() { - continue // We only stub required variables + if !vc.Required() || vc.Const { + continue // We only stub non-const required variables } if _, exists := existing[name]; !exists { missing = true @@ -512,7 +512,7 @@ func (b *Local) stubUnsetRequiredVariables(existing map[string]arguments.Unparse maps.Copy(ret, existing) // don't use clone here, so we can return a non-nil map for name, vc := range vcs { - if !vc.Required() { + if !vc.Required() || vc.Const { continue } if _, exists := existing[name]; !exists { diff --git a/internal/cloud/backend_context.go b/internal/cloud/backend_context.go index e3fc587a33..4b39db0a6a 100644 --- a/internal/cloud/backend_context.go +++ b/internal/cloud/backend_context.go @@ -86,63 +86,41 @@ func (b *Cloud) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, statem return nil, nil, diags } - if op.AllowUnsetVariables { - // If we're not going to use the variables in an operation we'll be - // more lax about them, stubbing out any unset ones as unknown. - // This gives us enough information to produce a consistent context, - // but not enough information to run a real operation (plan, apply, etc) - ret.PlanOpts.SetVariables = stubAllVariables(op.Variables, rootMod.Variables) - } else { - // The underlying API expects us to use the opaque workspace id to request - // variables, so we'll need to look that up using our organization name - // and workspace name. - remoteWorkspaceID, err := b.getRemoteWorkspaceID(context.Background(), op.Workspace) - if err != nil { - diags = diags.Append(fmt.Errorf("error finding remote workspace: %w", err)) - return nil, nil, diags - } - w, err := b.fetchWorkspace(context.Background(), b.Organization, op.Workspace) - if err != nil { - diags = diags.Append(fmt.Errorf("error loading workspace: %w", err)) - return nil, nil, diags - } - - if isLocalExecutionMode(w.ExecutionMode) { - log.Printf("[TRACE] skipping retrieving variables from workspace %s/%s (%s), workspace is in Local Execution mode", remoteWorkspaceName, b.Organization, remoteWorkspaceID) - } else { - log.Printf("[TRACE] cloud: retrieving variables from workspace %s/%s (%s)", remoteWorkspaceName, b.Organization, remoteWorkspaceID) - tfeVariables, err := b.client.Variables.ListAll(context.Background(), remoteWorkspaceID, nil) - if err != nil && err != tfe.ErrResourceNotFound { - diags = diags.Append(fmt.Errorf("error loading variables: %w", err)) - return nil, nil, diags - } - - if tfeVariables != nil { - if op.Variables == nil { - op.Variables = make(map[string]arguments.UnparsedVariableValue) - } - - for _, v := range tfeVariables.Items { - if v.Category == tfe.CategoryTerraform { - if _, ok := op.Variables[v.Key]; !ok { - op.Variables[v.Key] = &remoteStoredVariableValue{ - definition: v, - } - } - } - } - } + // If we're not going to use the variables in an operation we'll be + // more lax about them, stubbing out any unset ones as unknown. + // This gives us enough information to produce a consistent context, + // but not enough information to run a real operation (plan, apply, etc). + // + // However, const variables must always be resolved since they're + // needed during early configuration loading (e.g. module sources). + // We fetch backend variables so const vars can be satisfied. + fetchedVars, fetchDiags := b.FetchVariables(context.Background(), op.Workspace) + diags = diags.Append(fetchDiags) + if fetchDiags.HasErrors() { + return nil, nil, diags + } + if len(fetchedVars) > 0 { + if op.Variables == nil { + op.Variables = make(map[string]arguments.UnparsedVariableValue) } - - if op.Variables != nil { - variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) - diags = diags.Append(varDiags) - if diags.HasErrors() { - return nil, nil, diags + for k, v := range fetchedVars { + if _, ok := op.Variables[k]; !ok { + op.Variables[k] = v } - ret.PlanOpts.SetVariables = variables } } + var variables terraform.InputValues + var varDiags tfdiags.Diagnostics + if op.AllowUnsetVariables { + variables, varDiags = backendrun.ParseConstVariableValues(op.Variables, rootMod.Variables) + } else { + variables, varDiags = backendrun.ParseVariableValues(op.Variables, rootMod.Variables) + } + diags = diags.Append(varDiags) + if diags.HasErrors() { + return nil, nil, diags + } + ret.PlanOpts.SetVariables = variables tfCtx, ctxDiags := terraform.NewContext(&opts) diags = diags.Append(ctxDiags) @@ -202,31 +180,47 @@ func (b *Cloud) getRemoteWorkspaceID(ctx context.Context, localWorkspaceName str return remoteWorkspace.ID, nil } -func stubAllVariables(vv map[string]arguments.UnparsedVariableValue, decls map[string]*configs.Variable) terraform.InputValues { - ret := make(terraform.InputValues, len(decls)) +// FetchVariables implements backendrun.ConstVariableSupplier by retrieving +// Terraform variables from the HCP Terraform or Terraform Enterprise workspace. +func (b *Cloud) FetchVariables(ctx context.Context, workspace string) (map[string]arguments.UnparsedVariableValue, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics - for name, cfg := range decls { - raw, exists := vv[name] - if !exists { - ret[name] = &terraform.InputValue{ - Value: cty.UnknownVal(cfg.Type), - SourceType: terraform.ValueFromConfig, - } - continue - } + remoteWorkspaceID, err := b.getRemoteWorkspaceID(ctx, workspace) + if err != nil { + diags = diags.Append(fmt.Errorf("error finding remote workspace: %w", err)) + return nil, diags + } - val, diags := raw.ParseVariableValue(cfg.ParsingMode) - if diags.HasErrors() { - ret[name] = &terraform.InputValue{ - Value: cty.UnknownVal(cfg.Type), - SourceType: terraform.ValueFromConfig, + w, err := b.fetchWorkspace(ctx, b.Organization, workspace) + if err != nil { + diags = diags.Append(fmt.Errorf("error loading workspace: %w", err)) + return nil, diags + } + + if isLocalExecutionMode(w.ExecutionMode) { + log.Printf("[TRACE] cloud: skipping variable fetch for workspace %s/%s (%s), workspace is in Local Execution mode", b.getRemoteWorkspaceName(workspace), b.Organization, remoteWorkspaceID) + return nil, nil + } + + log.Printf("[TRACE] cloud: retrieving variables from workspace %s/%s (%s)", b.getRemoteWorkspaceName(workspace), b.Organization, remoteWorkspaceID) + tfeVariables, err := b.client.Variables.ListAll(ctx, remoteWorkspaceID, nil) + if err != nil && err != tfe.ErrResourceNotFound { + diags = diags.Append(fmt.Errorf("error loading variables: %w", err)) + return nil, diags + } + + result := make(map[string]arguments.UnparsedVariableValue) + if tfeVariables != nil { + for _, v := range tfeVariables.Items { + if v.Category == tfe.CategoryTerraform { + result[v.Key] = &remoteStoredVariableValue{ + definition: v, + } } - continue } - ret[name] = val } - return ret + return result, nil } // remoteStoredVariableValue is a backendrun.UnparsedVariableValue implementation