Merge pull request #24605 from hashicorp/jbardin/validate-module-variable

Allow module variables to pass validation

terraform/context_validate_test.go

@@ -1435,7 +1435,7 @@ resource "aws_instance" "foo" {
 module "nested" {
   count = 2
   source = "./nested"
-  input = 2
+  input = count.index
 }
 `,
 		"mod/nested/main.tf": `

terraform/eval_apply.go

@@ -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
 		}

terraform/eval_count.go

@@ -11,22 +11,17 @@ import (
 	"github.com/zclconf/go-cty/cty/gocty"
 )
 
-// evaluateResourceCountExpression 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.
+// evaluateCountExpression is our standard mechanism for interpreting an
+// expression given for a "count" argument on a resource or a module. This
+// should be called during expansion in order to determine the final count
+// value.
 //
-// If the result is zero or positive and no error diagnostics are returned, then
-// the result is the literal count value to use.
-//
-// If the result is -1, this indicates that the given expression is nil and so
-// the "count" behavior should not be enabled for this resource at all.
-//
-// 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 {
+// 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) {
+	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 +35,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 +68,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 +83,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 +92,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 +105,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

terraform/eval_diff.go

@@ -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)

terraform/eval_for_each.go

@@ -8,14 +8,17 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-// evaluateResourceForEachExpression interprets a "for_each" argument on a resource.
+// evaluateForEachExpression is our standard mechanism for interpreting an
+// expression given for a "for_each" argument on a resource or a module. This
+// should be called during expansion in order to determine the final keys and
+// values.
 //
-// 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 {
+// 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) {
+	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 +27,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 +61,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 +111,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
 }

terraform/eval_for_each_test.go

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

terraform/eval_read_data.go

@@ -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

terraform/eval_state.go

@@ -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)
-	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
-	}
+	switch {
+	case n.Config.Count != nil:
+		count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
+		diags = diags.Append(countDiags)
+		if countDiags.HasErrors() {
+			return nil, diags.Err()
+		}
 
-	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
-	// while ensuring that any existing instances are preserved, etc.
-	state.SetResourceMeta(n.Addr, eachMode, n.ProviderAddr)
-
-	switch eachMode {
-	case states.EachList:
+		state.SetResourceMeta(n.Addr, states.EachList, n.ProviderAddr)
 		expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
-	case states.EachMap:
+
+	case n.Config.ForEach != nil:
+		forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
+		diags = diags.Append(forEachDiags)
+		if forEachDiags.HasErrors() {
+			return nil, diags.Err()
+		}
+
+		// 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, 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)
 	}
 

terraform/eval_validate.go

@@ -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
 	}
 

terraform/eval_variable.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/instances"
 	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
@@ -42,12 +43,12 @@ type EvalModuleCallArgument struct {
 	Expr           hcl.Expression
 	ModuleInstance addrs.ModuleInstance
 
-	// If this flag is set, any diagnostics are discarded and this operation
-	// will always succeed, though may produce an unknown value in the
-	// event of an error.
-	IgnoreDiagnostics bool
-
 	Values map[string]cty.Value
+
+	// validateOnly indicates that this evaluation is only for config
+	// validation, and we will not have any expansion module instance
+	// repetition data.
+	validateOnly bool
 }
 
 func (n *EvalModuleCallArgument) Eval(ctx EvalContext) (interface{}, error) {
@@ -69,9 +70,24 @@ func (n *EvalModuleCallArgument) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, nil
 	}
 
-	// Get the repetition data for this module instance,
-	// so we can create the appropriate scope for evaluating our expression
-	moduleInstanceRepetitionData := ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance)
+	var moduleInstanceRepetitionData instances.RepetitionData
+
+	switch {
+	case n.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,
+		}
+
+	default:
+		// Get the repetition data for this module instance,
+		// so we can create the appropriate scope for evaluating our expression
+		moduleInstanceRepetitionData = ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance)
+	}
+
 	scope := ctx.EvaluationScope(nil, moduleInstanceRepetitionData)
 	val, diags := scope.EvalExpr(expr, cty.DynamicPseudoType)
 
@@ -96,9 +112,6 @@ func (n *EvalModuleCallArgument) Eval(ctx EvalContext) (interface{}, error) {
 	}
 
 	n.Values[name] = val
-	if n.IgnoreDiagnostics {
-		return nil, nil
-	}
 	return nil, diags.ErrWithWarnings()
 }
 
@@ -116,15 +129,10 @@ type evalVariableValidations struct {
 	// This will be nil for root module variables, because their values come
 	// from outside the configuration.
 	Expr hcl.Expression
-
-	// If this flag is set, this node becomes a no-op.
-	// This is here for consistency with EvalModuleCallArgument so that it
-	// can be populated with the same value, where needed.
-	IgnoreDiagnostics bool
 }
 
 func (n *evalVariableValidations) Eval(ctx EvalContext) (interface{}, error) {
-	if n.Config == nil || n.IgnoreDiagnostics || len(n.Config.Validations) == 0 {
+	if n.Config == nil || len(n.Config.Validations) == 0 {
 		log.Printf("[TRACE] evalVariableValidations: not active for %s, so skipping", n.Addr)
 		return nil, nil
 	}

terraform/node_data_refresh.go

@@ -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)
-	diags = diags.Append(countDiags)
-	if countDiags.HasErrors() {
-		return nil, diags.Err()
-	}
-	if !countKnown {
-		// If the count isn't known yet, we'll skip refreshing and try expansion
-		// again during the plan walk.
-		return nil, nil
-	}
+	expander := ctx.InstanceExpander()
 
-	forEachMap, forEachKnown, forEachDiags := evaluateResourceForEachExpressionKnown(n.Config.ForEach, ctx)
-	diags = diags.Append(forEachDiags)
-	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
+	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 !count.IsKnown() {
+			// If the count isn't known yet, we'll skip refreshing and try expansion
+			// again during the plan walk.
+			return nil, nil
+		}
+
+		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 !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
+		}
+
+		expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachVal.AsValueMap())
+
+	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(), count != -1)
+	fixResourceCountSetTransition(ctx, n.ResourceAddr(), n.Config.Count != nil)
 
-	// 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)
-	}
 	instanceAddrs := expander.ExpandResource(n.Addr)
 
 	// Our graph transformers require access to the full state, so we'll

terraform/node_module_expand.go

@@ -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()
 			}

terraform/node_module_variable.go

@@ -11,9 +11,9 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-// NodePlannableModuleVariable is the placeholder for an variable that has not yet had
+// nodeExpandModuleVariable is the placeholder for an variable that has not yet had
 // its module path expanded.
-type NodePlannableModuleVariable struct {
+type nodeExpandModuleVariable struct {
 	Addr   addrs.InputVariable
 	Module addrs.Module
 	Config *configs.Variable
@@ -21,23 +21,23 @@ type NodePlannableModuleVariable struct {
 }
 
 var (
-	_ GraphNodeDynamicExpandable = (*NodePlannableModuleVariable)(nil)
-	_ GraphNodeReferenceOutside  = (*NodePlannableModuleVariable)(nil)
-	_ GraphNodeReferenceable     = (*NodePlannableModuleVariable)(nil)
-	_ GraphNodeReferencer        = (*NodePlannableModuleVariable)(nil)
-	_ graphNodeTemporaryValue    = (*NodePlannableModuleVariable)(nil)
-	_ RemovableIfNotTargeted     = (*NodePlannableModuleVariable)(nil)
+	_ GraphNodeDynamicExpandable = (*nodeExpandModuleVariable)(nil)
+	_ GraphNodeReferenceOutside  = (*nodeExpandModuleVariable)(nil)
+	_ GraphNodeReferenceable     = (*nodeExpandModuleVariable)(nil)
+	_ GraphNodeReferencer        = (*nodeExpandModuleVariable)(nil)
+	_ graphNodeTemporaryValue    = (*nodeExpandModuleVariable)(nil)
+	_ RemovableIfNotTargeted     = (*nodeExpandModuleVariable)(nil)
 )
 
-func (n *NodePlannableModuleVariable) temporaryValue() bool {
+func (n *nodeExpandModuleVariable) temporaryValue() bool {
 	return true
 }
 
-func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) {
+func (n *nodeExpandModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) {
 	var g Graph
 	expander := ctx.InstanceExpander()
 	for _, module := range expander.ExpandModule(n.Module) {
-		o := &NodeApplyableModuleVariable{
+		o := &nodeModuleVariable{
 			Addr:           n.Addr.Absolute(module),
 			Config:         n.Config,
 			Expr:           n.Expr,
@@ -48,17 +48,17 @@ func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, er
 	return &g, nil
 }
 
-func (n *NodePlannableModuleVariable) Name() string {
+func (n *nodeExpandModuleVariable) Name() string {
 	return fmt.Sprintf("%s.%s", n.Module, n.Addr.String())
 }
 
 // GraphNodeModulePath
-func (n *NodePlannableModuleVariable) ModulePath() addrs.Module {
+func (n *nodeExpandModuleVariable) ModulePath() addrs.Module {
 	return n.Module
 }
 
 // GraphNodeReferencer
-func (n *NodePlannableModuleVariable) References() []*addrs.Reference {
+func (n *nodeExpandModuleVariable) References() []*addrs.Reference {
 
 	// If we have no value expression, we cannot depend on anything.
 	if n.Expr == nil {
@@ -86,12 +86,12 @@ func (n *NodePlannableModuleVariable) References() []*addrs.Reference {
 }
 
 // GraphNodeReferenceOutside implementation
-func (n *NodePlannableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
+func (n *nodeExpandModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
 	return n.Module, n.Module.Parent()
 }
 
 // GraphNodeReferenceable
-func (n *NodePlannableModuleVariable) ReferenceableAddrs() []addrs.Referenceable {
+func (n *nodeExpandModuleVariable) ReferenceableAddrs() []addrs.Referenceable {
 	// FIXME: References for module variables probably need to be thought out a bit more
 	// Otherwise, we can reference the output via the address itself, or the
 	// module call
@@ -100,18 +100,18 @@ func (n *NodePlannableModuleVariable) ReferenceableAddrs() []addrs.Referenceable
 }
 
 // RemovableIfNotTargeted
-func (n *NodePlannableModuleVariable) RemoveIfNotTargeted() bool {
+func (n *nodeExpandModuleVariable) RemoveIfNotTargeted() bool {
 	return true
 }
 
 // GraphNodeTargetDownstream
-func (n *NodePlannableModuleVariable) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool {
+func (n *nodeExpandModuleVariable) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool {
 	return true
 }
 
-// NodeApplyableModuleVariable represents a module variable input during
+// nodeModuleVariable represents a module variable input during
 // the apply step.
-type NodeApplyableModuleVariable struct {
+type nodeModuleVariable struct {
 	Addr   addrs.AbsInputVariableInstance
 	Config *configs.Variable // Config is the var in the config
 	Expr   hcl.Expression    // Expr is the value expression given in the call
@@ -123,92 +123,42 @@ type NodeApplyableModuleVariable struct {
 // Ensure that we are implementing all of the interfaces we think we are
 // implementing.
 var (
-	_ GraphNodeModuleInstance   = (*NodeApplyableModuleVariable)(nil)
-	_ RemovableIfNotTargeted    = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeReferenceOutside = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeReferenceable    = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeReferencer       = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeEvalable         = (*NodeApplyableModuleVariable)(nil)
-	_ graphNodeTemporaryValue   = (*NodeApplyableModuleVariable)(nil)
-	_ dag.GraphNodeDotter       = (*NodeApplyableModuleVariable)(nil)
+	_ GraphNodeModuleInstance = (*nodeModuleVariable)(nil)
+	_ RemovableIfNotTargeted  = (*nodeModuleVariable)(nil)
+	_ GraphNodeEvalable       = (*nodeModuleVariable)(nil)
+	_ graphNodeTemporaryValue = (*nodeModuleVariable)(nil)
+	_ dag.GraphNodeDotter     = (*nodeModuleVariable)(nil)
 )
 
-func (n *NodeApplyableModuleVariable) temporaryValue() bool {
+func (n *nodeModuleVariable) temporaryValue() bool {
 	return true
 }
 
-func (n *NodeApplyableModuleVariable) Name() string {
+func (n *nodeModuleVariable) Name() string {
 	return n.Addr.String()
 }
 
 // GraphNodeModuleInstance
-func (n *NodeApplyableModuleVariable) Path() addrs.ModuleInstance {
+func (n *nodeModuleVariable) Path() addrs.ModuleInstance {
 	// We execute in the parent scope (above our own module) because
 	// expressions in our value are resolved in that context.
 	return n.Addr.Module.Parent()
 }
 
 // GraphNodeModulePath
-func (n *NodeApplyableModuleVariable) ModulePath() addrs.Module {
+func (n *nodeModuleVariable) ModulePath() addrs.Module {
 	return n.Addr.Module.Parent().Module()
 }
 
 // RemovableIfNotTargeted
-func (n *NodeApplyableModuleVariable) RemoveIfNotTargeted() bool {
+func (n *nodeModuleVariable) RemoveIfNotTargeted() bool {
 	// We need to add this so that this node will be removed if
 	// it isn't targeted or a dependency of a target.
 	return true
 }
 
-// GraphNodeReferenceOutside implementation
-func (n *NodeApplyableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
-
-	// Module input variables have their value expressions defined in the
-	// context of their calling (parent) module, and so references from
-	// a node of this type should be resolved in the parent module instance.
-	referencePath = n.Addr.Module.Parent().Module()
-
-	// Input variables are _referenced_ from their own module, though.
-	selfPath = n.Addr.Module.Module()
-
-	return // uses named return values
-}
-
-// GraphNodeReferenceable
-func (n *NodeApplyableModuleVariable) ReferenceableAddrs() []addrs.Referenceable {
-	return []addrs.Referenceable{n.Addr.Variable}
-}
-
-// GraphNodeReferencer
-func (n *NodeApplyableModuleVariable) References() []*addrs.Reference {
-
-	// If we have no value expression, we cannot depend on anything.
-	if n.Expr == nil {
-		return nil
-	}
-
-	// Variables in the root don't depend on anything, because their values
-	// are gathered prior to the graph walk and recorded in the context.
-	if len(n.Addr.Module) == 0 {
-		return nil
-	}
-
-	// Otherwise, we depend on anything referenced by our value expression.
-	// We ignore diagnostics here under the assumption that we'll re-eval
-	// all these things later and catch them then; for our purposes here,
-	// we only care about valid references.
-	//
-	// Due to our GraphNodeReferenceOutside implementation, the addresses
-	// returned by this function are interpreted in the _parent_ module from
-	// where our associated variable was declared, which is correct because
-	// our value expression is assigned within a "module" block in the parent
-	// module.
-	refs, _ := lang.ReferencesInExpr(n.Expr)
-	return refs
-}
-
 // GraphNodeEvalable
-func (n *NodeApplyableModuleVariable) EvalTree() EvalNode {
+func (n *nodeModuleVariable) EvalTree() EvalNode {
 	// If we have no value, do nothing
 	if n.Expr == nil {
 		return &EvalNoop{}
@@ -224,15 +174,25 @@ func (n *NodeApplyableModuleVariable) EvalTree() EvalNode {
 		Nodes: []EvalNode{
 			&EvalOpFilter{
 				Ops: []walkOperation{walkRefresh, walkPlan, walkApply,
-					walkDestroy, walkValidate},
+					walkDestroy},
 				Node: &EvalModuleCallArgument{
 					Addr:           n.Addr.Variable,
 					Config:         n.Config,
 					Expr:           n.Expr,
 					ModuleInstance: n.ModuleInstance,
 					Values:         vals,
+				},
+			},
 
-					IgnoreDiagnostics: false,
+			&EvalOpFilter{
+				Ops: []walkOperation{walkValidate},
+				Node: &EvalModuleCallArgument{
+					Addr:           n.Addr.Variable,
+					Config:         n.Config,
+					Expr:           n.Expr,
+					ModuleInstance: n.ModuleInstance,
+					Values:         vals,
+					validateOnly:   true,
 				},
 			},
 
@@ -245,15 +205,13 @@ func (n *NodeApplyableModuleVariable) EvalTree() EvalNode {
 				Addr:   n.Addr,
 				Config: n.Config,
 				Expr:   n.Expr,
-
-				IgnoreDiagnostics: false,
 			},
 		},
 	}
 }
 
 // dag.GraphNodeDotter impl.
-func (n *NodeApplyableModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNode {
+func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNode {
 	return &dag.DotNode{
 		Name: name,
 		Attrs: map[string]string{

terraform/node_module_variable_test.go

@@ -12,9 +12,9 @@ import (
 	"github.com/hashicorp/terraform/configs"
 )
 
-func TestNodeApplyableModuleVariablePath(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).InputVariable("foo"),
+func TestnodeModuleVariablePath(t *testing.T) {
+	n := &nodeModuleVariable{
+		Addr: addrs.RootModuleInstance.InputVariable("foo"),
 		Config: &configs.Variable{
 			Name: "foo",
 		},
@@ -27,9 +27,9 @@ func TestNodeApplyableModuleVariablePath(t *testing.T) {
 	}
 }
 
-func TestNodeApplyableModuleVariableReferenceableName(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).InputVariable("foo"),
+func TestnodeModuleVariableReferenceableName(t *testing.T) {
+	n := &nodeExpandModuleVariable{
+		Addr: addrs.InputVariable{Name: "foo"},
 		Config: &configs.Variable{
 			Name: "foo",
 		},
@@ -59,9 +59,9 @@ func TestNodeApplyableModuleVariableReferenceableName(t *testing.T) {
 
 }
 
-func TestNodeApplyableModuleVariableReference(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).InputVariable("foo"),
+func TestnodeModuleVariableReference(t *testing.T) {
+	n := &nodeExpandModuleVariable{
+		Addr: addrs.InputVariable{Name: "foo"},
 		Config: &configs.Variable{
 			Name: "foo",
 		},
@@ -84,12 +84,9 @@ func TestNodeApplyableModuleVariableReference(t *testing.T) {
 	}
 }
 
-func TestNodeApplyableModuleVariableReference_grandchild(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.
-			Child("child", addrs.NoKey).
-			Child("grandchild", addrs.NoKey).
-			InputVariable("foo"),
+func TestnodeModuleVariableReference_grandchild(t *testing.T) {
+	n := &nodeExpandModuleVariable{
+		Addr: addrs.InputVariable{Name: "foo"},
 		Config: &configs.Variable{
 			Name: "foo",
 		},

terraform/node_resource_refresh.go

@@ -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)
-	diags = diags.Append(countDiags)
-	if countDiags.HasErrors() {
-		return nil, diags.Err()
-	}
+	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)
-	if forEachDiags.HasErrors() {
-		return nil, diags.Err()
+		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()
+		}
+
+		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(), 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)
-	}
+	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

terraform/node_root_variable.go

@@ -15,7 +15,6 @@ type NodeRootVariable struct {
 var (
 	_ GraphNodeModuleInstance = (*NodeRootVariable)(nil)
 	_ GraphNodeReferenceable  = (*NodeRootVariable)(nil)
-	_ dag.GraphNodeDotter     = (*NodeApplyableModuleVariable)(nil)
 )
 
 func (n *NodeRootVariable) Name() string {
@@ -51,8 +50,6 @@ func (n *NodeRootVariable) EvalTree() EvalNode {
 		Addr:   addrs.RootModuleInstance.InputVariable(n.Addr.Name),
 		Config: n.Config,
 		Expr:   nil, // not set for root module variables
-
-		IgnoreDiagnostics: false,
 	}
 }
 

terraform/transform_module_variable.go

@@ -105,7 +105,7 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs
 
 		// Add a plannable node, as the variable may expand
 		// during module expansion
-		node := &NodePlannableModuleVariable{
+		node := &nodeExpandModuleVariable{
 			Addr: addrs.InputVariable{
 				Name: v.Name,
 			},
@@ -113,7 +113,6 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs
 			Config: v,
 			Expr:   expr,
 		}
-
 		g.Add(node)
 	}