Skip to content

Commit

Permalink
core: Placeholder values for local values in partial-expanded modules
Browse files Browse the repository at this point in the history
Since local value evaluation is largely identical between the fully- and
partially-expanded cases, this factors out that handling into a shared
function and makes the two graph node Execute functions just thin wrappers
around it.

The error-handling behavior is intentionally marginally different: it now
always registers the local value result in the named values state, even
on error, but uses cty.DynamicVal as a placeholder when an error prevents
returning anything more precise than that. Nothing depends on that detail
because downstream nodes don't get to execute when an upstream returns an
error, but this approach reduces the number of possible outcomes and thus
makes this marginally easier to follow.

This also includes some modernization of the unit tests for
TestNodeLocalExecute, so that it no longer relies on shimming functions
from the Terraform v0.12 transition.
  • Loading branch information
apparentlymart committed Jan 29, 2024
1 parent 507ccce commit a55b0b3
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 49 deletions.
97 changes: 67 additions & 30 deletions internal/terraform/node_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
32 changes: 13 additions & 19 deletions internal/terraform/node_local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,33 @@ 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"
)

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
},
}
Expand All @@ -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)
Expand All @@ -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)
}
})
}
Expand Down

0 comments on commit a55b0b3

Please sign in to comment.