Skip to content

Commit

Permalink
terraform: count and for_each evaluation permitting unknown values
Browse files Browse the repository at this point in the history
This is currently an opt-in argument, with no callers (except some tests)
using it. Therefore this should not change any externally-visible behavior
yet.

Future commits will gradually incorporate support for modules and resources
whose expansion isn't yet known, which will then cause planning of
anything downstream of them to be deferred to a future run.
  • Loading branch information
apparentlymart committed Jan 24, 2024
1 parent 78e2e20 commit ae2a64a
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 35 deletions.
15 changes: 11 additions & 4 deletions internal/terraform/eval_count.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ package terraform
import (
"fmt"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/gocty"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/tfdiags"
)

// evaluateCountExpression is our standard mechanism for interpreting an
Expand All @@ -20,9 +21,15 @@ import (
// evaluateCountExpression differs from evaluateCountExpressionValue by
// returning an error if the count value is not known, and converting the
// cty.Value to an integer.
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
//
// If allowUnknown is false then this function will return error diagnostics
// whenever the expression returns an unknown value. Setting allowUnknown to
// true instead permits unknown values, indicating them by returning the
// placeholder value -1. Callers can assume that a return value of -1 without
// any error diagnostics represents a valid unknown value.
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext, allowUnknown bool) (int, tfdiags.Diagnostics) {
countVal, diags := evaluateCountExpressionValue(expr, ctx)
if !countVal.IsKnown() {
if !allowUnknown && !countVal.IsKnown() {
// Currently this is a rather bad outcome from a UX standpoint, since we have
// no real mechanism to deal with this situation and all we can do is produce
// an error message.
Expand Down
36 changes: 35 additions & 1 deletion internal/terraform/eval_count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestEvaluateCountExpression(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
countVal, diags := evaluateCountExpression(test.Expr, ctx)
countVal, diags := evaluateCountExpression(test.Expr, ctx, false)

if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
Expand All @@ -47,3 +47,37 @@ func TestEvaluateCountExpression(t *testing.T) {
})
}
}

func TestEvaluateCountExpression_allowUnknown(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
Count int
}{
"unknown number": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Number)),
-1,
},
"dynamicval": {
hcltest.MockExprLiteral(cty.DynamicVal),
-1,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
countVal, diags := evaluateCountExpression(test.Expr, ctx, true)

if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}

if !reflect.DeepEqual(countVal, test.Count) {
t.Errorf(
"wrong result\ngot: %#v\nwant: %#v",
countVal, test.Count,
)
}
})
}
}
39 changes: 25 additions & 14 deletions internal/terraform/eval_for_each.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,21 @@ import (
// evaluateForEachExpression differs from evaluateForEachExpressionValue by
// returning an error if the count value is not known, and converting the
// cty.Value to a map[string]cty.Value for compatibility with other calls.
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
return newForEachEvaluator(expr, ctx).ResourceValue()
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext, allowUnknown bool) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) {
return newForEachEvaluator(expr, ctx, allowUnknown).ResourceValue()
}

// forEachEvaluator is the standard mechanism for interpreting an expression
// given for a "for_each" argument on a resource, module, or import.
func newForEachEvaluator(expr hcl.Expression, ctx EvalContext) *forEachEvaluator {
func newForEachEvaluator(expr hcl.Expression, ctx EvalContext, allowUnknown bool) *forEachEvaluator {
if ctx == nil {
panic("nil EvalContext")
}

return &forEachEvaluator{
ctx: ctx,
expr: expr,
ctx: ctx,
expr: expr,
allowUnknown: allowUnknown,
}
}

Expand All @@ -48,44 +49,54 @@ type forEachEvaluator struct {
ctx EvalContext
expr hcl.Expression

// TEMP: If allowUnknown is set then we skip the usual restriction that
// unknown values are not allowed in for_each. A caller that sets this
// must therefore be ready to deal with the result being unknown.
// This will eventually become the default behavior, once we've updated
// the rest of this package to handle that situation in a reasonable way.
allowUnknown bool

// internal
hclCtx *hcl.EvalContext
}

// ResourceForEachValue returns a known for_each map[string]cty.Value
// appropriate for use within resource expansion.
func (ev *forEachEvaluator) ResourceValue() (map[string]cty.Value, tfdiags.Diagnostics) {
func (ev *forEachEvaluator) ResourceValue() (map[string]cty.Value, bool, tfdiags.Diagnostics) {
res := map[string]cty.Value{}

// no expression always results in an empty map
if ev.expr == nil {
return res, nil
return res, true, nil
}

forEachVal, diags := ev.Value()
if diags.HasErrors() {
return res, diags
return res, false, diags
}

// ensure our value is known for use in resource expansion
diags = diags.Append(ev.ensureKnownForResource(forEachVal))
if diags.HasErrors() {
return res, diags
unknownDiags := ev.ensureKnownForResource(forEachVal)
if unknownDiags.HasErrors() {
if !ev.allowUnknown {
diags = diags.Append(unknownDiags)
}
return res, false, diags
}

// validate the for_each value for use in resource expansion
diags = diags.Append(ev.validateResource(forEachVal))
if diags.HasErrors() {
return res, diags
return res, false, diags
}

if forEachVal.IsNull() || !forEachVal.IsKnown() || markSafeLengthInt(forEachVal) == 0 {
// we check length, because an empty set returns a nil map which will panic below
return res, diags
return res, true, diags
}

res = forEachVal.AsValueMap()
return res, diags
return res, true, diags
}

// ImportValue returns the for_each map for use within an import block,
Expand Down
41 changes: 38 additions & 3 deletions internal/terraform/eval_for_each_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestEvaluateForEachExpression_valid(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
forEachMap, diags := evaluateForEachExpression(test.Expr, ctx)
forEachMap, _, diags := evaluateForEachExpression(test.Expr, ctx, false)

if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
_, diags := evaluateForEachExpression(test.Expr, ctx)
_, _, diags := evaluateForEachExpression(test.Expr, ctx, false)

if len(diags) != 1 {
t.Fatalf("got %d diagnostics; want 1", len(diags))
Expand Down Expand Up @@ -211,6 +211,41 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
}
}

func TestEvaluateForEachExpression_allowUnknown(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
}{
"unknown string set": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
},
"unknown map": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
},
"set containing unknown value": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)})),
},
"set containing dynamic unknown value": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.DynamicPseudoType)})),
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
_, known, diags := evaluateForEachExpression(test.Expr, ctx, true)

// With allowUnknown set, all of these expressions should be treated
// as valid for_each values.
assertNoDiagnostics(t, diags)

if known {
t.Errorf("result is known; want unknown")
}
})
}
}

func TestEvaluateForEachExpressionKnown(t *testing.T) {
tests := map[string]hcl.Expression{
"unknown string set": hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
Expand All @@ -221,7 +256,7 @@ func TestEvaluateForEachExpressionKnown(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
diags := newForEachEvaluator(expr, ctx).ValidateResourceValue()
diags := newForEachEvaluator(expr, ctx, false).ValidateResourceValue()

if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
Expand Down
6 changes: 3 additions & 3 deletions internal/terraform/node_module_expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfd
ctx = ctx.WithPath(module)
switch {
case n.ModuleCall.Count != nil:
count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx)
count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx, false)
diags = diags.Append(ctDiags)
if diags.HasErrors() {
return diags
}
expander.SetModuleCount(module, call, count)

case n.ModuleCall.ForEach != nil:
forEach, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx)
forEach, _, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx, false)
diags = diags.Append(feDiags)
if diags.HasErrors() {
return diags
Expand Down Expand Up @@ -256,7 +256,7 @@ func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) (diags t
diags = diags.Append(countDiags)

case n.ModuleCall.ForEach != nil:
forEachDiags := newForEachEvaluator(n.ModuleCall.ForEach, ctx).ValidateResourceValue()
forEachDiags := newForEachEvaluator(n.ModuleCall.ForEach, ctx, false).ValidateResourceValue()
diags = diags.Append(forEachDiags)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/terraform/node_resource_abstract.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab

switch {
case n.Config != nil && n.Config.Count != nil:
count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
count, countDiags := evaluateCountExpression(n.Config.Count, ctx, false)
diags = diags.Append(countDiags)
if countDiags.HasErrors() {
return diags
Expand All @@ -414,7 +414,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab
expander.SetResourceCount(addr.Module, n.Addr.Resource, count)

case n.Config != nil && n.Config.ForEach != nil:
forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false)
diags = diags.Append(forEachDiags)
if forEachDiags.HasErrors() {
return diags
Expand Down
8 changes: 4 additions & 4 deletions internal/terraform/node_resource_abstract_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ func (n *NodeAbstractResourceInstance) plan(
}

// Evaluate the configuration
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _, _ := evaluateForEachExpression(n.Config.ForEach, ctx, false)

keyData = EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)

Expand Down Expand Up @@ -1709,7 +1709,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule
objTy := schema.ImpliedType()
priorVal := cty.NullVal(objTy)

forEach, _ := evaluateForEachExpression(config.ForEach, ctx)
forEach, _, _ := evaluateForEachExpression(config.ForEach, ctx, false)
keyData = EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)

checkDiags := evalCheckRules(
Expand Down Expand Up @@ -1989,7 +1989,7 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned
return nil, keyData, diags
}

forEach, _ := evaluateForEachExpression(config.ForEach, ctx)
forEach, _, _ := evaluateForEachExpression(config.ForEach, ctx, false)
keyData = EvalDataForInstanceKey(n.Addr.Resource.Key, forEach)

checkDiags := evalCheckRules(
Expand Down Expand Up @@ -2293,7 +2293,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
func (n *NodeAbstractResourceInstance) evalProvisionerConfig(ctx EvalContext, body hcl.Body, self cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false)
diags = diags.Append(forEachDiags)

keyData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)
Expand Down
2 changes: 1 addition & 1 deletion internal/terraform/node_resource_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (n nodeExpandPlannableResource) expandResourceImports(ctx EvalContext, addr
continue
}

forEachData, forEachDiags := newForEachEvaluator(imp.Config.ForEach, ctx).ImportValues()
forEachData, forEachDiags := newForEachEvaluator(imp.Config.ForEach, ctx, false).ImportValues()
diags = diags.Append(forEachDiags)
if forEachDiags.HasErrors() {
return imports, diags
Expand Down
4 changes: 2 additions & 2 deletions internal/terraform/node_resource_plan_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
// values, which could result in a post-condition check relying on that
// value being inaccurate. Unless we decide to store the value of the
// for-each expression in state, this is unavoidable.
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _, _ := evaluateForEachExpression(n.Config.ForEach, ctx, false)
repeatData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)

checkDiags := evalCheckRules(
Expand Down Expand Up @@ -463,7 +463,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
return nil, diags
}

forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _, _ := evaluateForEachExpression(n.Config.ForEach, ctx, false)
keyData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)
configVal, _, configDiags := ctx.EvaluateBlock(n.Config.Config, schema, nil, keyData)
if configDiags.HasErrors() {
Expand Down
2 changes: 1 addition & 1 deletion internal/terraform/node_resource_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag
}

// Evaluate the for_each expression here so we can expose the diagnostics
forEachDiags := newForEachEvaluator(n.Config.ForEach, ctx).ValidateResourceValue()
forEachDiags := newForEachEvaluator(n.Config.ForEach, ctx, false).ValidateResourceValue()
diags = diags.Append(forEachDiags)
}

Expand Down

0 comments on commit ae2a64a

Please sign in to comment.