Add support for for_each for data blocks.

This also fixes a few things with resource for_each:

It makes validation more like validation for count.

It makes sure the index is stored in the state properly.
This commit is contained in:
Thayne McCombs 2019-07-25 09:51:55 -06:00 committed by Pam Selle
parent 7d905f6777
commit 7c678d104f
7 changed files with 91 additions and 29 deletions

View File

@ -114,6 +114,11 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) {
hcl.DiagError, hcl.DiagError,
`Invalid combination of "count" and "for_each"`, `Invalid combination of "count" and "for_each"`,
}, },
{
"invalid-files/data-count-and-for_each.tf",
hcl.DiagError,
`Invalid combination of "count" and "for_each"`,
},
{ {
"invalid-files/resource-lifecycle-badbool.tf", "invalid-files/resource-lifecycle-badbool.tf",
hcl.DiagError, hcl.DiagError,

View File

@ -302,14 +302,16 @@ func decodeDataBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) {
if attr, exists := content.Attributes["for_each"]; exists { if attr, exists := content.Attributes["for_each"]; exists {
r.ForEach = attr.Expr r.ForEach = attr.Expr
// We currently parse this, but don't yet do anything with it. // Cannot have count and for_each on the same data block
if r.Count != nil {
diags = append(diags, &hcl.Diagnostic{ diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError, Severity: hcl.DiagError,
Summary: "Reserved argument name in module block", Summary: `Invalid combination of "count" and "for_each"`,
Detail: fmt.Sprintf("The name %q is reserved for use in a future version of Terraform.", attr.Name), Detail: `The "count" and "for_each" meta-arguments are mutually-exclusive, only one should be used to be explicit about the number of resources to be created.`,
Subject: &attr.NameRange, Subject: &attr.NameRange,
}) })
} }
}
if attr, exists := content.Attributes["provider"]; exists { if attr, exists := content.Attributes["provider"]; exists {
var providerDiags hcl.Diagnostics var providerDiags hcl.Diagnostics

View File

@ -0,0 +1,4 @@
data "test" "foo" {
count = 2
for_each = ["a"]
}

View File

@ -14,29 +14,50 @@ import (
// the expression is nil, and is used to distinguish between an unset for_each and an // the expression is nil, and is used to distinguish between an unset for_each and an
// empty map // empty map
func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) { func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx)
if !known {
// 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.
// FIXME: In future, implement a built-in mechanism for deferring changes that
// can't yet be predicted, and use it to guide the user through several
// plan/apply steps until the desired configuration is eventually reached.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid forEach argument",
Detail: `The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.`,
})
panic("uh-oh")
}
return forEachMap, 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) {
if expr == nil { if expr == nil {
return nil, nil return nil, true, nil
} }
forEachVal, forEachDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) forEachVal, forEachDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil)
diags = diags.Append(forEachDiags) diags = diags.Append(forEachDiags)
if diags.HasErrors() { if diags.HasErrors() {
return nil, diags return nil, true, diags
} }
// No-op for dynamic types, so that these pass validation, but are then populated at apply switch {
if forEachVal.Type() == cty.DynamicPseudoType { case forEachVal.IsNull():
return nil, diags
}
if forEachVal.IsNull() {
diags = diags.Append(&hcl.Diagnostic{ diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError, Severity: hcl.DiagError,
Summary: "Invalid for_each argument", Summary: "Invalid for_each argument",
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.`, 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(), Subject: expr.Range().Ptr(),
}) })
return nil, diags return nil, true, diags
case !forEachVal.IsKnown():
return map[string]cty.Value{}, false, diags
} }
if !forEachVal.CanIterateElements() || forEachVal.Type().IsListType() { if !forEachVal.CanIterateElements() || forEachVal.Type().IsListType() {
@ -46,7 +67,7 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo
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.`, forEachVal.Type().FriendlyName()),
Subject: expr.Range().Ptr(), Subject: expr.Range().Ptr(),
}) })
return nil, diags return nil, true, diags
} }
if forEachVal.Type().IsSetType() { if forEachVal.Type().IsSetType() {
@ -57,17 +78,14 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo
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()), 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(), Subject: expr.Range().Ptr(),
}) })
return nil, diags return nil, true, diags
} }
} }
// If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap // If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap
// Also return an empty map if the value is not known -- as this function if forEachVal.LengthInt() == 0 {
// is used to check if the for_each value is valid as well as to apply it, the empty return map[string]cty.Value{}, true, diags
// map will later be filled in.
if !forEachVal.IsKnown() || forEachVal.LengthInt() == 0 {
return map[string]cty.Value{}, diags
} }
return forEachVal.AsValueMap(), nil return forEachVal.AsValueMap(), true, nil
} }

View File

@ -424,15 +424,21 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
return nil, diags.Err() return nil, diags.Err()
} }
// Currently we ony support NoEach and EachList, because for_each support
// is not fully wired up across Terraform. Once for_each support is added,
// we'll need to handle that here too, setting states.EachMap if the
// assigned expression is a map.
eachMode := states.NoEach eachMode := states.NoEach
if count >= 0 { // -1 signals "count not set" if count >= 0 { // -1 signals "count not set"
eachMode = states.EachList eachMode = states.EachList
} }
forEach, forEachDiags := evaluateResourceForEachExpression(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 // This method takes care of all of the business logic of updating this
// while ensuring that any existing instances are preserved, etc. // while ensuring that any existing instances are preserved, etc.
state.SetResourceMeta(absAddr, eachMode, n.ProviderAddr) state.SetResourceMeta(absAddr, eachMode, n.ProviderAddr)

View File

@ -377,7 +377,7 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) {
} }
// Evaluate the for_each expression here so we can expose the diagnostics // Evaluate the for_each expression here so we can expose the diagnostics
_, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx) forEachDiags := n.validateForEach(ctx, n.Config.ForEach)
diags = diags.Append(forEachDiags) diags = diags.Append(forEachDiags)
} }
@ -553,3 +553,18 @@ func (n *EvalValidateResource) validateCount(ctx EvalContext, expr hcl.Expressio
return diags return diags
} }
func (n *EvalValidateResource) validateForEach(ctx EvalContext, expr hcl.Expression) (diags tfdiags.Diagnostics) {
_, known, forEachDiags := evaluateResourceForEachExpressionKnown(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 {
return diags
}
if forEachDiags.HasErrors() {
diags = diags.Append(forEachDiags)
}
return diags
}

View File

@ -38,6 +38,16 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er
return nil, nil return nil, nil
} }
forEachMap, forEachKnown, forEachDiags := evaluateResourceForEachExpressionKnown(n.Config.ForEach, ctx)
if forEachDiags.HasErrors() {
return nil, diags.Err()
}
if !forEachKnown {
// 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 // Next we need to potentially rename an instance address in the state
// if we're transitioning whether "count" is set at all. // if we're transitioning whether "count" is set at all.
fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1) fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1)
@ -77,6 +87,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er
Concrete: concreteResource, Concrete: concreteResource,
Schema: n.Schema, Schema: n.Schema,
Count: count, Count: count,
ForEach: forEachMap,
Addr: n.ResourceAddr(), Addr: n.ResourceAddr(),
}, },
@ -85,6 +96,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er
&OrphanResourceCountTransformer{ &OrphanResourceCountTransformer{
Concrete: concreteResourceDestroyable, Concrete: concreteResourceDestroyable,
Count: count, Count: count,
ForEach: forEachMap,
Addr: n.ResourceAddr(), Addr: n.ResourceAddr(),
State: state, State: state,
}, },