rename and cleanup use of count/for_each eval func

Stop evaluating count and for each if they aren't set in the config.
Remove "Resource" from the function names, as they are also now used
with modules.
This commit is contained in:
James Bardin 2020-04-08 17:09:17 -04:00
parent d060a3d0e8
commit b1bc7a792b
11 changed files with 165 additions and 156 deletions

View File

@ -61,7 +61,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
configVal := cty.NullVal(cty.DynamicPseudoType)
if n.Config != nil {
var configDiags tfdiags.Diagnostics
forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
configVal, _, configDiags = ctx.EvaluateBlock(n.Config.Config, schema, nil, keyData)
diags = diags.Append(configDiags)
@ -595,7 +595,7 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*configs.Provisio
// in its result. That's okay because each.value is prohibited for
// destroy-time provisioners.
if n.When != configs.ProvisionerWhenDestroy {
m, forEachDiags := evaluateResourceForEachExpression(n.ResourceConfig.ForEach, ctx)
m, forEachDiags := evaluateForEachExpression(n.ResourceConfig.ForEach, ctx)
diags = diags.Append(forEachDiags)
forEach = m
}

View File

@ -11,7 +11,7 @@ import (
"github.com/zclconf/go-cty/cty/gocty"
)
// evaluateResourceCountExpression is our standard mechanism for interpreting an
// evaluateCountExpression is our standard mechanism for interpreting an
// expression given for a "count" argument on a resource. This should be called
// from the DynamicExpand of a node representing a resource in order to
// determine the final count value.
@ -24,9 +24,9 @@ import (
//
// If error diagnostics are returned then the result is always the meaningless
// placeholder value -1.
func evaluateResourceCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
count, known, diags := evaluateResourceCountExpressionKnown(expr, ctx)
if !known {
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
countVal, diags := evaluateCountExpressionValue(expr, ctx)
if !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.
@ -40,22 +40,29 @@ func evaluateResourceCountExpression(expr hcl.Expression, ctx EvalContext) (int,
Subject: expr.Range().Ptr(),
})
}
return count, diags
if countVal.IsNull() || !countVal.IsKnown() {
return -1, diags
}
count, _ := countVal.AsBigFloat().Int64()
return int(count), diags
}
// evaluateResourceCountExpressionKnown is like evaluateResourceCountExpression
// except that it handles an unknown result by returning count = 0 and
// a known = false, rather than by reporting the unknown value as an error
// diagnostic.
func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext) (count int, known bool, diags tfdiags.Diagnostics) {
// evaluateCountExpressionValue is like evaluateCountExpression
// except that it returns a cty.Value which must be a cty.Number and can be
// unknown.
func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
nullCount := cty.NullVal(cty.Number)
if expr == nil {
return -1, true, nil
return nullCount, nil
}
countVal, countDiags := ctx.EvaluateExpr(expr, cty.Number, nil)
diags = diags.Append(countDiags)
if diags.HasErrors() {
return -1, true, diags
return nullCount, diags
}
switch {
@ -66,11 +73,13 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
Detail: `The given "count" argument value is null. An integer is required.`,
Subject: expr.Range().Ptr(),
})
return -1, true, diags
return nullCount, diags
case !countVal.IsKnown():
return 0, false, diags
return cty.UnknownVal(cty.Number), diags
}
var count int
err := gocty.FromCtyValue(countVal, &count)
if err != nil {
diags = diags.Append(&hcl.Diagnostic{
@ -79,7 +88,7 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
Detail: fmt.Sprintf(`The given "count" argument value is unsuitable: %s.`, err),
Subject: expr.Range().Ptr(),
})
return -1, true, diags
return nullCount, diags
}
if count < 0 {
diags = diags.Append(&hcl.Diagnostic{
@ -88,10 +97,10 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
Detail: `The given "count" argument value is unsuitable: negative numbers are not supported.`,
Subject: expr.Range().Ptr(),
})
return -1, true, diags
return nullCount, diags
}
return count, true, diags
return countVal, diags
}
// fixResourceCountSetTransition is a helper function to fix up the state when a
@ -101,7 +110,7 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
//
// The correct time to call this function is in the DynamicExpand method for
// a node representing a resource, just after evaluating the count with
// evaluateResourceCountExpression, and before any other analysis of the
// evaluateCountExpression, and before any other analysis of the
// state such as orphan detection.
//
// This function calls methods on the given EvalContext to update the current

View File

@ -133,7 +133,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
// Should be caught during validation, so we don't bother with a pretty error here
return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type)
}
forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData)
diags = diags.Append(configDiags)

View File

@ -8,14 +8,14 @@ import (
"github.com/zclconf/go-cty/cty"
)
// evaluateResourceForEachExpression interprets a "for_each" argument on a resource.
// evaluateForEachExpression interprets a "for_each" argument on a resource.
//
// Returns a cty.Value map, and diagnostics if necessary. It will return nil if
// the expression is nil, and is used to distinguish between an unset for_each and an
// empty map
func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx)
if !known {
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
forEachVal, diags := evaluateForEachExpressionValue(expr, ctx)
if !forEachVal.IsKnown() {
// Attach a diag as we do with count, with the same downsides
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
@ -24,23 +24,31 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo
Subject: expr.Range().Ptr(),
})
}
return forEachMap, diags
if forEachVal.IsNull() || !forEachVal.IsKnown() || forEachVal.LengthInt() == 0 {
// we check length, because an empty set return a nil map
return map[string]cty.Value{}, diags
}
return forEachVal.AsValueMap(), diags
}
// evaluateResourceForEachExpressionKnown is like evaluateResourceForEachExpression
// except that it handles an unknown result by returning an empty map and
// a known = false, rather than by reporting the unknown value as an error
// diagnostic.
func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) {
// evaluateForEachExpressionValue is like evaluateForEachExpression
// except that it returns a cty.Value map or set which can be unknown.
func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
nullMap := cty.NullVal(cty.Map(cty.DynamicPseudoType))
if expr == nil {
return nil, true, nil
return nullMap, diags
}
forEachVal, forEachDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil)
diags = diags.Append(forEachDiags)
if diags.HasErrors() {
return nil, true, diags
return nullMap, diags
}
ty := forEachVal.Type()
switch {
case forEachVal.IsNull():
@ -50,44 +58,42 @@ func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext
Detail: `The given "for_each" argument value is unsuitable: the given "for_each" argument value is null. A map, or set of strings is allowed.`,
Subject: expr.Range().Ptr(),
})
return nil, true, diags
return nullMap, diags
case !forEachVal.IsKnown():
return map[string]cty.Value{}, false, diags
}
// ensure that we have a map, and not a DynamicValue
return cty.UnknownVal(cty.Map(cty.DynamicPseudoType)), diags
if !forEachVal.CanIterateElements() || forEachVal.Type().IsListType() || forEachVal.Type().IsTupleType() {
case !(ty.IsMapType() || ty.IsSetType() || ty.IsObjectType()):
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type %s.`, forEachVal.Type().FriendlyName()),
Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type %s.`, ty.FriendlyName()),
Subject: expr.Range().Ptr(),
})
return nil, true, diags
return nullMap, diags
case forEachVal.LengthInt() == 0:
// If the map is empty ({}), return an empty map, because cty will
// return nil when representing {} AsValueMap This also covers an empty
// set (toset([]))
return forEachVal, diags
}
// If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap
// This also covers an empty set (toset([]))
if forEachVal.LengthInt() == 0 {
return map[string]cty.Value{}, true, diags
}
if forEachVal.Type().IsSetType() {
if forEachVal.Type().ElementType() != cty.String {
if ty.IsSetType() {
if ty.ElementType() != cty.String {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each set argument",
Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: "for_each" supports maps and sets of strings, but you have provided a set containing type %s.`, forEachVal.Type().ElementType().FriendlyName()),
Subject: expr.Range().Ptr(),
})
return nil, true, diags
return cty.NullVal(ty), diags
}
// A set may contain unknown values that must be
// discovered by checking with IsWhollyKnown (which iterates through the
// structure), while for maps in cty, keys can never be unknown or null,
// thus the earlier IsKnown check suffices for maps
// since we can't use a set values that are unknown, we treat the
// entire set as unknown
if !forEachVal.IsWhollyKnown() {
return map[string]cty.Value{}, false, diags
return cty.UnknownVal(ty), diags
}
// A set of strings may contain null, which makes it impossible to
@ -102,10 +108,10 @@ func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext
Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: "for_each" sets must not contain null values.`),
Subject: expr.Range().Ptr(),
})
return nil, true, diags
return cty.NullVal(ty), diags
}
}
}
return forEachVal.AsValueMap(), true, nil
return forEachVal, nil
}

View File

@ -12,7 +12,7 @@ import (
"github.com/zclconf/go-cty/cty"
)
func TestEvaluateResourceForEachExpression_valid(t *testing.T) {
func TestEvaluateForEachExpression_valid(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
ForEachMap map[string]cty.Value
@ -58,7 +58,7 @@ func TestEvaluateResourceForEachExpression_valid(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
forEachMap, diags := evaluateResourceForEachExpression(test.Expr, ctx)
forEachMap, diags := evaluateForEachExpression(test.Expr, ctx)
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@ -75,7 +75,7 @@ func TestEvaluateResourceForEachExpression_valid(t *testing.T) {
}
}
func TestEvaluateResourceForEachExpression_errors(t *testing.T) {
func TestEvaluateForEachExpression_errors(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
Summary, DetailSubstring string
@ -131,7 +131,7 @@ func TestEvaluateResourceForEachExpression_errors(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
_, diags := evaluateResourceForEachExpression(test.Expr, ctx)
_, diags := evaluateForEachExpression(test.Expr, ctx)
if len(diags) != 1 {
t.Fatalf("got %d diagnostics; want 1", diags)
@ -149,7 +149,7 @@ func TestEvaluateResourceForEachExpression_errors(t *testing.T) {
}
}
func TestEvaluateResourceForEachExpressionKnown(t *testing.T) {
func TestEvaluateForEachExpressionKnown(t *testing.T) {
tests := map[string]hcl.Expression{
"unknown string set": hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
"unknown map": hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
@ -159,23 +159,15 @@ func TestEvaluateResourceForEachExpressionKnown(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx)
forEachVal, diags := evaluateForEachExpressionValue(expr, ctx)
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}
if known {
t.Errorf("got %v known, want false", known)
if forEachVal.IsKnown() {
t.Error("got known, want unknown")
}
if len(forEachMap) != 0 {
t.Errorf(
"expected empty map\ngot: %s",
spew.Sdump(forEachMap),
)
}
})
}
}

View File

@ -96,7 +96,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
objTy := schema.ImpliedType()
priorVal := cty.NullVal(objTy) // for data resources, prior is always null because we start fresh every time
forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
var configDiags tfdiags.Diagnostics

View File

@ -466,36 +466,32 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
// can refer to it. Since this node represents the abstract module, we need
// to expand the module here to create all resources.
expander := ctx.InstanceExpander()
count, countDiags := evaluateResourceCountExpression(n.Config.Count, ctx)
switch {
case n.Config.Count != nil:
count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
diags = diags.Append(countDiags)
if countDiags.HasErrors() {
return nil, diags.Err()
}
eachMode := states.NoEach
if count >= 0 { // -1 signals "count not set"
eachMode = states.EachList
}
state.SetResourceMeta(n.Addr, states.EachList, n.ProviderAddr)
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
forEach, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
case n.Config.ForEach != nil:
forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
diags = diags.Append(forEachDiags)
if forEachDiags.HasErrors() {
return nil, diags.Err()
}
if forEach != nil {
eachMode = states.EachMap
}
// This method takes care of all of the business logic of updating this
// while ensuring that any existing instances are preserved, etc.
state.SetResourceMeta(n.Addr, eachMode, n.ProviderAddr)
switch eachMode {
case states.EachList:
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
case states.EachMap:
state.SetResourceMeta(n.Addr, states.EachMap, n.ProviderAddr)
expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEach)
default:
state.SetResourceMeta(n.Addr, states.NoEach, n.ProviderAddr)
expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
}

View File

@ -375,7 +375,9 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) {
mode := cfg.Mode
keyData := EvalDataForNoInstanceKey
if n.Config.Count != nil {
switch {
case n.Config.Count != nil:
// If the config block has count, we'll evaluate with an unknown
// number as count.index so we can still type check even though
// we won't expand count until the plan phase.
@ -387,9 +389,8 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) {
// of this will happen when we DynamicExpand during the plan walk.
countDiags := n.validateCount(ctx, n.Config.Count)
diags = diags.Append(countDiags)
}
if n.Config.ForEach != nil {
case n.Config.ForEach != nil:
keyData = InstanceKeyEvalData{
EachKey: cty.UnknownVal(cty.String),
EachValue: cty.UnknownVal(cty.DynamicPseudoType),
@ -608,10 +609,10 @@ func (n *EvalValidateResource) validateCount(ctx EvalContext, expr hcl.Expressio
}
func (n *EvalValidateResource) validateForEach(ctx EvalContext, expr hcl.Expression) (diags tfdiags.Diagnostics) {
_, known, forEachDiags := evaluateResourceForEachExpressionKnown(expr, ctx)
val, forEachDiags := evaluateForEachExpressionValue(expr, ctx)
// If the value isn't known then that's the best we can do for now, but
// we'll check more thoroughly during the plan walk
if !known {
if !val.IsKnown() {
return diags
}

View File

@ -65,43 +65,46 @@ func (n *NodeRefreshableDataResource) Path() addrs.ModuleInstance {
func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, error) {
var diags tfdiags.Diagnostics
count, countKnown, countDiags := evaluateResourceCountExpressionKnown(n.Config.Count, ctx)
expander := ctx.InstanceExpander()
switch {
case n.Config.Count != nil:
count, countDiags := evaluateCountExpressionValue(n.Config.Count, ctx)
diags = diags.Append(countDiags)
if countDiags.HasErrors() {
return nil, diags.Err()
}
if !countKnown {
if !count.IsKnown() {
// If the count isn't known yet, we'll skip refreshing and try expansion
// again during the plan walk.
return nil, nil
}
forEachMap, forEachKnown, forEachDiags := evaluateResourceForEachExpressionKnown(n.Config.ForEach, ctx)
c, _ := count.AsBigFloat().Int64()
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, int(c))
case n.Config.ForEach != nil:
forEachVal, forEachDiags := evaluateForEachExpressionValue(n.Config.ForEach, ctx)
diags = diags.Append(forEachDiags)
if forEachDiags.HasErrors() {
return nil, diags.Err()
}
if !forEachKnown {
if !forEachVal.IsKnown() {
// If the for_each isn't known yet, we'll skip refreshing and try expansion
// again during the plan walk.
return nil, nil
}
// Next we need to potentially rename an instance address in the state
// if we're transitioning whether "count" is set at all.
fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1)
expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachVal.AsValueMap())
// Inform our instance expander about our expansion results above,
// and then use it to calculate the instance addresses we'll expand for.
expander := ctx.InstanceExpander()
switch {
case count >= 0:
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
case forEachMap != nil:
expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachMap)
default:
expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
}
// Next we need to potentially rename an instance address in the state
// if we're transitioning whether "count" is set at all.
fixResourceCountSetTransition(ctx, n.ResourceAddr(), n.Config.Count != nil)
instanceAddrs := expander.ExpandResource(n.Addr)
// Our graph transformers require access to the full state, so we'll

View File

@ -229,14 +229,14 @@ func (n *evalPrepareModuleExpansion) Eval(ctx EvalContext) (interface{}, error)
switch {
case n.ModuleCall.Count != nil:
count, diags := evaluateResourceCountExpression(n.ModuleCall.Count, ctx)
count, diags := evaluateCountExpression(n.ModuleCall.Count, ctx)
if diags.HasErrors() {
return nil, diags.Err()
}
expander.SetModuleCount(module, call, count)
case n.ModuleCall.ForEach != nil:
forEach, diags := evaluateResourceForEachExpression(n.ModuleCall.ForEach, ctx)
forEach, diags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx)
if diags.HasErrors() {
return nil, diags.Err()
}

View File

@ -89,32 +89,34 @@ func (n *NodeRefreshableManagedResource) Path() addrs.ModuleInstance {
func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, error) {
var diags tfdiags.Diagnostics
count, countDiags := evaluateResourceCountExpression(n.Config.Count, ctx)
expander := ctx.InstanceExpander()
// Inform our instance expander about our expansion results, and then use
// it to calculate the instance addresses we'll expand for.
switch {
case n.Config.Count != nil:
count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
diags = diags.Append(countDiags)
if countDiags.HasErrors() {
return nil, diags.Err()
}
forEachMap, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
case n.Config.ForEach != nil:
forEachMap, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
if forEachDiags.HasErrors() {
return nil, diags.Err()
}
// Next we need to potentially rename an instance address in the state
// if we're transitioning whether "count" is set at all.
fixResourceCountSetTransition(ctx, n.Addr.Config(), count != -1)
// Inform our instance expander about our expansion results above,
// and then use it to calculate the instance addresses we'll expand for.
expander := ctx.InstanceExpander()
switch {
case count >= 0:
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
case forEachMap != nil:
expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachMap)
default:
expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
}
// Next we need to potentially rename an instance address in the state
// if we're transitioning whether "count" is set at all.
fixResourceCountSetTransition(ctx, n.Addr.Config(), n.Config.Count != nil)
instanceAddrs := expander.ExpandResource(n.Addr)
// Our graph transformers require access to the full state, so we'll