diff --git a/internal/terraform/node_local.go b/internal/terraform/node_local.go index 067878e534a6..895ca28c74e1 100644 --- a/internal/terraform/node_local.go +++ b/internal/terraform/node_local.go @@ -145,35 +145,9 @@ func (n *NodeLocal) References() []*addrs.Reference { // expression for a local value and writes it into a transient part of // the state. func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { - expr := n.Config.Expr - addr := n.Addr.LocalValue - - // We ignore diags here because any problems we might find will be found - // again in EvaluateExpr below. - refs, _ := lang.ReferencesInExpr(addrs.ParseRef, expr) - for _, ref := range refs { - if ref.Subject == addr { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Self-referencing local value", - Detail: fmt.Sprintf("Local value %s cannot use its own result as part of its expression.", addr), - Subject: ref.SourceRange.ToHCL().Ptr(), - Context: expr.Range().Ptr(), - }) - } - } - if diags.HasErrors() { - return diags - } - - val, moreDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) - diags = diags.Append(moreDiags) - if moreDiags.HasErrors() { - return diags - } - - ctx.NamedValues().SetLocalValue(addr.Absolute(ctx.Path()), val) - + namedVals := ctx.NamedValues() + val, diags := evaluateLocalValue(n.Config, n.Addr.LocalValue, n.Addr.String(), ctx) + namedVals.SetLocalValue(n.Addr, val) return diags } @@ -201,8 +175,71 @@ type nodeLocalInPartialModule struct { Config *configs.Local } +// Path implements [GraphNodePartialExpandedModule], meaning that the +// Execute method receives an [EvalContext] that's set up for partial-expanded +// evaluation instead of full evaluation. func (n *nodeLocalInPartialModule) Path() addrs.PartialExpandedModule { return n.Addr.Module } -// TODO: Implement nodeLocalUnexpandedPlaceholder.Execute +func (n *nodeLocalInPartialModule) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { + // Our job here is to make sure that the local value definition is + // valid for all instances of this local value across all of the possible + // module instances under our partially-expanded prefix, and to record + // a placeholder value that captures as precisely as possible what all + // of those results have in common. In the worst case where they have + // absolutely nothing in common cty.DynamicVal is the ultimate fallback, + // but we should try to do better when possible to give operators earlier + // feedback about any problems they would definitely encounter on a + // subsequent plan where the local values get evaluated concretely. + + namedVals := ctx.NamedValues() + val, diags := evaluateLocalValue(n.Config, n.Addr.Local, n.Addr.String(), ctx) + namedVals.SetLocalValuePlaceholder(n.Addr, val) + return diags +} + +// evaluateLocalValue is the common evaluation logic shared between +// [NodeLocal] and [nodeLocalInPartialModule]. +// +// The overall validation and evaluation process is the same for each, with +// the differences encapsulated inside the given [EvalContext], which is +// configured in a different way when doing partial-expanded evaluation. +// +// the addrStr argument should be the canonical string representation of the +// anbsolute address of the object being evaluated, which should either be an +// [addrs.AbsLocalValue] or an [addrs.InPartialEvaluatedModule[addrs.LocalValue]] +// depending on which of the two callers are calling this function. +// +// localAddr should match the local portion of the address that was stringified +// for addrStr, describing the local value relative to the module it's declared +// inside. +func evaluateLocalValue(config *configs.Local, localAddr addrs.LocalValue, addrStr string, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + expr := config.Expr + + // We ignore diags here because any problems we might find will be found + // again in EvaluateExpr below. + refs, _ := lang.ReferencesInExpr(addrs.ParseRef, expr) + for _, ref := range refs { + if ref.Subject == localAddr { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Self-referencing local value", + Detail: fmt.Sprintf("Local value %s cannot use its own result as part of its expression.", addrStr), + Subject: ref.SourceRange.ToHCL().Ptr(), + Context: expr.Range().Ptr(), + }) + } + } + if diags.HasErrors() { + return cty.DynamicVal, diags + } + + val, moreDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) + diags = diags.Append(moreDiags) + if val == cty.NilVal { + val = cty.DynamicVal + } + return val, diags +} diff --git a/internal/terraform/node_local_test.go b/internal/terraform/node_local_test.go index ba28c118e99d..b3ed692cde86 100644 --- a/internal/terraform/node_local_test.go +++ b/internal/terraform/node_local_test.go @@ -8,10 +8,10 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/configs/hcl2shim" "github.com/hashicorp/terraform/internal/namedvals" "github.com/hashicorp/terraform/internal/states" ) @@ -19,22 +19,22 @@ import ( func TestNodeLocalExecute(t *testing.T) { tests := []struct { Value string - Want interface{} + Want cty.Value Err bool }{ { "hello!", - "hello!", + cty.StringVal("hello!"), false, }, { "", - "", + cty.StringVal(""), false, }, { "Hello, ${local.foo}", - nil, + cty.DynamicVal, true, // self-referencing }, } @@ -57,7 +57,7 @@ func TestNodeLocalExecute(t *testing.T) { StateState: states.NewState().SyncWrapper(), NamedValuesState: namedvals.NewState(), - EvaluateExprResult: hcl2shim.HCL2ValueFromConfigValue(test.Want), + EvaluateExprResult: test.Want, } err := n.Execute(ctx, walkApply) @@ -69,19 +69,13 @@ func TestNodeLocalExecute(t *testing.T) { } } - if test.Err { - if ctx.NamedValues().HasLocalValue(localAddr) { - t.Errorf("have value for %s, but wanted none", localAddr) - } - } else { - if !ctx.NamedValues().HasLocalValue(localAddr) { - t.Fatalf("no value for %s", localAddr) - } - got := ctx.NamedValues().GetLocalValue(localAddr) - want := hcl2shim.HCL2ValueFromConfigValue(test.Want) - if !want.RawEquals(got) { - t.Errorf("wrong value for %s\ngot: %#v\nwant: %#v", localAddr, got, want) - } + if !ctx.NamedValues().HasLocalValue(localAddr) { + t.Fatalf("no value for %s", localAddr) + } + got := ctx.NamedValues().GetLocalValue(localAddr) + want := test.Want + if !want.RawEquals(got) { + t.Errorf("wrong value for %s\ngot: %#v\nwant: %#v", localAddr, got, want) } }) } diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 11a053e77224..ea2a59d1ee6c 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -160,6 +160,11 @@ type nodeModuleVariable struct { // ModuleCallArguments, ex. so count.index and each.key can resolve ModuleInstance addrs.ModuleInstance + // ModuleCallConfig is the module call that the expression in field Expr + // came from, which helps decide what [instances.RepetitionData] we should + // use when evaluating Expr. + ModuleCallConfig *configs.ModuleCall + // DestroyApply must be set to true when applying a destroy operation and // false otherwise. DestroyApply bool @@ -241,12 +246,6 @@ func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNod // evalModuleVariable produces the value for a particular variable as will // be used by a child module instance. // -// The result is written into a map, with its key set to the local name of the -// variable, disregarding the module instance address. A map is returned instead -// of a single value as a result of trying to be convenient for use with -// EvalContext.SetModuleCallArguments, which expects a map to merge in with any -// existing arguments. -// // validateOnly indicates that this evaluation is only for config // validation, and we will not have any expansion module instance // repetition data. @@ -261,11 +260,10 @@ func (n *nodeModuleVariable) evalModuleVariable(ctx EvalContext, validateOnly bo case validateOnly: // the instance expander does not track unknown expansion values, so we // have to assume all RepetitionData is unknown. - moduleInstanceRepetitionData = instances.RepetitionData{ - CountIndex: cty.UnknownVal(cty.Number), - EachKey: cty.UnknownVal(cty.String), - EachValue: cty.DynamicVal, - } + // TODO: Ideally we should vary the placeholder we use here based + // on how the module call repetition was configured, but we don't + // have enough information here to decide that. + moduleInstanceRepetitionData = instances.TotallyUnknownRepetitionData default: // Get the repetition data for this module instance, @@ -327,4 +325,33 @@ func (n *nodeModuleVariableInPartialModule) Path() addrs.PartialExpandedModule { return n.Addr.Module } -// TODO: Implement nodeModuleVariableInPartialModule.Execute +func (n *nodeModuleVariableInPartialModule) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { + // Our job here is to make sure that the input variable definition is + // valid for all instances of this input variable across all of the possible + // module instances under our partially-expanded prefix, and to record + // a placeholder value that captures as precisely as possible what all + // of those results have in common. In the worst case where they have + // absolutely nothing in common cty.DynamicVal is the ultimate fallback, + // but we should try to do better when possible to give operators earlier + // feedback about any problems they would definitely encounter on a + // subsequent plan where the input variables get evaluated concretely. + + namedVals := ctx.NamedValues() + + // TODO: Ideally we should vary the placeholder we use here based + // on how the module call repetition was configured, but we don't + // have enough information here to decide that. + moduleInstanceRepetitionData := instances.TotallyUnknownRepetitionData + + // NOTE WELL: Input variables are a little strange in that they announce + // themselves as belonging to the caller of the module they are declared + // in, because that's where their definition expressions get evaluated. + // Therefore this [EvalContext] is in the scope of the parent module, + // while n.Addr describes an object in the child module (where the + // variable declaration appeared). + scope := ctx.EvaluationScope(nil, nil, moduleInstanceRepetitionData) + val, diags := scope.EvalExpr(n.Expr, cty.DynamicPseudoType) + + namedVals.SetInputVariablePlaceholder(n.Addr, val) + return diags +} diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index 3be1fceeee9e..0875f16e1499 100644 --- a/internal/terraform/node_output.go +++ b/internal/terraform/node_output.go @@ -132,6 +132,10 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diagn Config: n.Config, RefreshOnly: n.RefreshOnly, } + // We don't need to handle the module.IsRoot() && n.Destroying case + // seen in the fully-expanded case above, because the root module + // instance is always "fully expanded" (it's always a singleton) + // and so we can't get here for output values in the root module. log.Printf("[TRACE] Expanding output: adding placeholder for all %s as %T", absAddr.String(), node) g.Add(node) }, @@ -464,11 +468,43 @@ type nodeOutputInPartialModule struct { RefreshOnly bool } +// Path implements [GraphNodePartialExpandedModule], meaning that the +// Execute method receives an [EvalContext] that's set up for partial-expanded +// evaluation instead of full evaluation. func (n *nodeOutputInPartialModule) Path() addrs.PartialExpandedModule { return n.Addr.Module } -// TODO: Implement nodeOutputInPartialModule.Execute +func (n *nodeOutputInPartialModule) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { + // Our job here is to make sure that the output value definition is + // valid for all instances of this output value across all of the possible + // module instances under our partially-expanded prefix, and to record + // a placeholder value that captures as precisely as possible what all + // of those results have in common. In the worst case where they have + // absolutely nothing in common cty.DynamicVal is the ultimate fallback, + // but we should try to do better when possible to give operators earlier + // feedback about any problems they would definitely encounter on a + // subsequent plan where the output values get evaluated concretely. + + namedVals := ctx.NamedValues() + + // this "ctx" is preconfigured to evaluate in terms of other placeholder + // values generated in the same unexpanded module prefix, rather than + // from the active state/plan, so this result is likely to be derived + // from unknown value placeholders itself. + val, diags := ctx.EvaluateExpr(n.Config.Expr, cty.DynamicPseudoType, nil) + if val == cty.NilVal { + val = cty.DynamicVal + } + + // We'll also check that the depends_on argument is valid, since that's + // a static concern anyway and so cannot vary between instances of the + // same module. + diags = diags.Append(validateDependsOn(ctx, n.Config.DependsOn)) + + namedVals.SetOutputValuePlaceholder(n.Addr, val) + return diags +} // NodeDestroyableOutput represents an output that is "destroyable": // its application will remove the output from the state.