Mildwonkey/eval local (#26182)

* terraform: refactor EvalLocal, remove unused EvalDeleteLocal
* terraform: refactor NodeCountBoundary
* terraform: node_module_expand refactor

terraform/eval_count_boundary.go

@@ -1,76 +0,0 @@
-package terraform
-
-import (
-	"fmt"
-	"log"
-
-	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/configs"
-)
-
-// EvalCountFixZeroOneBoundaryGlobal is an EvalNode that fixes up the state
-// when there is a resource count with zero/one boundary, i.e. fixing
-// a resource named "aws_instance.foo" to "aws_instance.foo.0" and vice-versa.
-//
-// This works on the global state.
-type EvalCountFixZeroOneBoundaryGlobal struct {
-	Config *configs.Config
-}
-
-// TODO: test
-func (n *EvalCountFixZeroOneBoundaryGlobal) Eval(ctx EvalContext) (interface{}, error) {
-	// We'll temporarily lock the state to grab the modules, then work on each
-	// one separately while taking a lock again for each separate resource.
-	// This means that if another caller concurrently adds a module here while
-	// we're working then we won't update it, but that's no worse than the
-	// concurrent writer blocking for our entire fixup process and _then_
-	// adding a new module, and in practice the graph node associated with
-	// this eval depends on everything else in the graph anyway, so there
-	// should not be concurrent writers.
-	state := ctx.State().Lock()
-	moduleAddrs := make([]addrs.ModuleInstance, 0, len(state.Modules))
-	for _, m := range state.Modules {
-		moduleAddrs = append(moduleAddrs, m.Addr)
-	}
-	ctx.State().Unlock()
-
-	for _, addr := range moduleAddrs {
-		cfg := n.Config.DescendentForInstance(addr)
-		if cfg == nil {
-			log.Printf("[WARN] Not fixing up EachModes for %s because it has no config", addr)
-			continue
-		}
-		if err := n.fixModule(ctx, addr); err != nil {
-			return nil, err
-		}
-	}
-
-	return nil, nil
-}
-
-func (n *EvalCountFixZeroOneBoundaryGlobal) fixModule(ctx EvalContext, moduleAddr addrs.ModuleInstance) error {
-	ms := ctx.State().Module(moduleAddr)
-	cfg := n.Config.DescendentForInstance(moduleAddr)
-	if ms == nil {
-		// Theoretically possible for a concurrent writer to delete a module
-		// while we're running, but in practice the graph node that called us
-		// depends on everything else in the graph and so there can never
-		// be a concurrent writer.
-		return fmt.Errorf("[WARN] no state found for %s while trying to fix up EachModes", moduleAddr)
-	}
-	if cfg == nil {
-		return fmt.Errorf("[WARN] no config found for %s while trying to fix up EachModes", moduleAddr)
-	}
-
-	for _, r := range ms.Resources {
-		rCfg := cfg.Module.ResourceByAddr(r.Addr.Resource)
-		if rCfg == nil {
-			log.Printf("[WARN] Not fixing up EachModes for %s because it has no config", r.Addr)
-			continue
-		}
-		hasCount := rCfg.Count != nil
-		fixResourceCountSetTransition(ctx, r.Addr.Config(), hasCount)
-	}
-
-	return nil
-}

terraform/eval_local.go

@@ -1,74 +0,0 @@
-package terraform
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/hcl/v2"
-	"github.com/zclconf/go-cty/cty"
-
-	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/lang"
-	"github.com/hashicorp/terraform/tfdiags"
-)
-
-// EvalLocal is an EvalNode implementation that evaluates the
-// expression for a local value and writes it into a transient part of
-// the state.
-type EvalLocal struct {
-	Addr addrs.LocalValue
-	Expr hcl.Expression
-}
-
-func (n *EvalLocal) Eval(ctx EvalContext) (interface{}, error) {
-	var diags tfdiags.Diagnostics
-
-	// We ignore diags here because any problems we might find will be found
-	// again in EvaluateExpr below.
-	refs, _ := lang.ReferencesInExpr(n.Expr)
-	for _, ref := range refs {
-		if ref.Subject == n.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.", n.Addr),
-				Subject:  ref.SourceRange.ToHCL().Ptr(),
-				Context:  n.Expr.Range().Ptr(),
-			})
-		}
-	}
-	if diags.HasErrors() {
-		return nil, diags.Err()
-	}
-
-	val, moreDiags := ctx.EvaluateExpr(n.Expr, cty.DynamicPseudoType, nil)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return nil, diags.Err()
-	}
-
-	state := ctx.State()
-	if state == nil {
-		return nil, fmt.Errorf("cannot write local value to nil state")
-	}
-
-	state.SetLocalValue(n.Addr.Absolute(ctx.Path()), val)
-
-	return nil, nil
-}
-
-// EvalDeleteLocal is an EvalNode implementation that deletes a Local value
-// from the state. Locals aren't persisted, but we don't need to evaluate them
-// during destroy.
-type EvalDeleteLocal struct {
-	Addr addrs.LocalValue
-}
-
-func (n *EvalDeleteLocal) Eval(ctx EvalContext) (interface{}, error) {
-	state := ctx.State()
-	if state == nil {
-		return nil, nil
-	}
-
-	state.RemoveLocalValue(n.Addr.Absolute(ctx.Path()))
-	return nil, nil
-}

terraform/node_count_boundary.go

@@ -1,6 +1,10 @@
 package terraform
 
 import (
+	"fmt"
+	"log"
+
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
 )
 
@@ -14,9 +18,59 @@ func (n *NodeCountBoundary) Name() string {
 	return "meta.count-boundary (EachMode fixup)"
 }
 
-// GraphNodeEvalable
-func (n *NodeCountBoundary) EvalTree() EvalNode {
-	return &EvalCountFixZeroOneBoundaryGlobal{
-		Config: n.Config,
+// GraphNodeExecutable
+func (n *NodeCountBoundary) Execute(ctx EvalContext, op walkOperation) error {
+	// We'll temporarily lock the state to grab the modules, then work on each
+	// one separately while taking a lock again for each separate resource.
+	// This means that if another caller concurrently adds a module here while
+	// we're working then we won't update it, but that's no worse than the
+	// concurrent writer blocking for our entire fixup process and _then_
+	// adding a new module, and in practice the graph node associated with
+	// this eval depends on everything else in the graph anyway, so there
+	// should not be concurrent writers.
+	state := ctx.State().Lock()
+	moduleAddrs := make([]addrs.ModuleInstance, 0, len(state.Modules))
+	for _, m := range state.Modules {
+		moduleAddrs = append(moduleAddrs, m.Addr)
 	}
+	ctx.State().Unlock()
+
+	for _, addr := range moduleAddrs {
+		cfg := n.Config.DescendentForInstance(addr)
+		if cfg == nil {
+			log.Printf("[WARN] Not fixing up EachModes for %s because it has no config", addr)
+			continue
+		}
+		if err := n.fixModule(ctx, addr); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (n *NodeCountBoundary) fixModule(ctx EvalContext, moduleAddr addrs.ModuleInstance) error {
+	ms := ctx.State().Module(moduleAddr)
+	cfg := n.Config.DescendentForInstance(moduleAddr)
+	if ms == nil {
+		// Theoretically possible for a concurrent writer to delete a module
+		// while we're running, but in practice the graph node that called us
+		// depends on everything else in the graph and so there can never
+		// be a concurrent writer.
+		return fmt.Errorf("[WARN] no state found for %s while trying to fix up EachModes", moduleAddr)
+	}
+	if cfg == nil {
+		return fmt.Errorf("[WARN] no config found for %s while trying to fix up EachModes", moduleAddr)
+	}
+
+	for _, r := range ms.Resources {
+		rCfg := cfg.Module.ResourceByAddr(r.Addr.Resource)
+		if rCfg == nil {
+			log.Printf("[WARN] Not fixing up EachModes for %s because it has no config", r.Addr)
+			continue
+		}
+		hasCount := rCfg.Count != nil
+		fixResourceCountSetTransition(ctx, r.Addr.Config(), hasCount)
+	}
+
+	return nil
 }

terraform/node_count_boundary_test.go

@@ -0,0 +1,72 @@
+package terraform
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2/hcltest"
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/states"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestNodeCountBoundaryExecute(t *testing.T) {
+
+	// Create a state with a single instance (addrs.NoKey) of test_instance.foo
+	state := states.NewState()
+	state.Module(addrs.RootModuleInstance).SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "test_instance",
+			Name: "foo",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"type":"string","value":"hello"}`),
+		},
+		addrs.AbsProviderConfig{
+			Provider: addrs.NewDefaultProvider("test"),
+			Module:   addrs.RootModule,
+		},
+	)
+
+	// Create a config that uses count to create 2 instances of test_instance.foo
+	rc := &configs.Resource{
+		Mode:  addrs.ManagedResourceMode,
+		Type:  "test_instance",
+		Name:  "foo",
+		Count: hcltest.MockExprLiteral(cty.NumberIntVal(2)),
+		Config: configs.SynthBody("", map[string]cty.Value{
+			"test_string": cty.StringVal("hello"),
+		}),
+	}
+	config := &configs.Config{
+		Module: &configs.Module{
+			ManagedResources: map[string]*configs.Resource{
+				"test_instance.foo": rc,
+			},
+		},
+	}
+
+	ctx := &MockEvalContext{
+		StateState: state.SyncWrapper(),
+	}
+	node := NodeCountBoundary{Config: config}
+
+	err := node.Execute(ctx, walkApply)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err.Error())
+	}
+	if !state.HasResources() {
+		t.Fatal("resources missing from state")
+	}
+
+	// verify that the resource changed from test_instance.foo to
+	// test_instance.foo.0 in the state
+	actual := state.String()
+	expected := "test_instance.foo.0:\n  ID = \n  provider = provider[\"registry.terraform.io/hashicorp/test\"]\n  type = string\n  value = hello"
+
+	if actual != expected {
+		t.Fatalf("wrong result: %s", actual)
+	}
+}

terraform/node_local.go

@@ -1,12 +1,16 @@
 package terraform
 
 import (
+	"fmt"
 	"log"
 
+	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/dag"
 	"github.com/hashicorp/terraform/lang"
+	"github.com/hashicorp/terraform/tfdiags"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // nodeExpandLocal represents a named local value in a configuration module,
@@ -85,7 +89,7 @@ var (
 	_ GraphNodeModuleInstance = (*NodeLocal)(nil)
 	_ GraphNodeReferenceable  = (*NodeLocal)(nil)
 	_ GraphNodeReferencer     = (*NodeLocal)(nil)
-	_ GraphNodeEvalable       = (*NodeLocal)(nil)
+	_ GraphNodeExecutable     = (*NodeLocal)(nil)
 	_ graphNodeTemporaryValue = (*NodeLocal)(nil)
 	_ dag.GraphNodeDotter     = (*NodeLocal)(nil)
 )
@@ -120,12 +124,49 @@ func (n *NodeLocal) References() []*addrs.Reference {
 	return refs
 }
 
-// GraphNodeEvalable
-func (n *NodeLocal) EvalTree() EvalNode {
-	return &EvalLocal{
-		Addr: n.Addr.LocalValue,
-		Expr: n.Config.Expr,
+// GraphNodeExecutable
+// NodeLocal.Execute is an Execute implementation that evaluates the
+// expression for a local value and writes it into a transient part of
+// the state.
+func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) error {
+
+	var 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(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.Err()
+	}
+
+	val, moreDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil)
+	diags = diags.Append(moreDiags)
+	if moreDiags.HasErrors() {
+		return diags.Err()
+	}
+
+	state := ctx.State()
+	if state == nil {
+		return fmt.Errorf("cannot write local value to nil state")
+	}
+
+	state.SetLocalValue(addr.Absolute(ctx.Path()), val)
+
+	return nil
 }
 
 // dag.GraphNodeDotter impl.

terraform/node_local_test.go

@@ -10,15 +10,12 @@ import (
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/configs/hcl2shim"
 	"github.com/hashicorp/terraform/states"
 )
 
-func TestEvalLocal_impl(t *testing.T) {
-	var _ EvalNode = new(EvalLocal)
-}
-
-func TestEvalLocal(t *testing.T) {
+func TestNodeLocalExecute(t *testing.T) {
 	tests := []struct {
 		Value string
 		Want  interface{}
@@ -48,9 +45,11 @@ func TestEvalLocal(t *testing.T) {
 				t.Fatal(diags.Error())
 			}
 
-			n := &EvalLocal{
-				Addr: addrs.LocalValue{Name: "foo"},
-				Expr: expr,
+			n := &NodeLocal{
+				Addr: addrs.LocalValue{Name: "foo"}.Absolute(addrs.RootModuleInstance),
+				Config: &configs.Local{
+					Expr: expr,
+				},
 			}
 			ctx := &MockEvalContext{
 				StateState: states.NewState().SyncWrapper(),
@@ -58,7 +57,7 @@ func TestEvalLocal(t *testing.T) {
 				EvaluateExprResult: hcl2shim.HCL2ValueFromConfigValue(test.Want),
 			}
 
-			_, err := n.Eval(ctx)
+			err := n.Execute(ctx, walkApply)
 			if (err != nil) != test.Err {
 				if err != nil {
 					t.Errorf("unexpected error: %s", err)

terraform/node_module_expand.go

@@ -22,7 +22,7 @@ type nodeExpandModule struct {
 }
 
 var (
-	_ GraphNodeEvalable         = (*nodeExpandModule)(nil)
+	_ GraphNodeExecutable       = (*nodeExpandModule)(nil)
 	_ GraphNodeReferencer       = (*nodeExpandModule)(nil)
 	_ GraphNodeReferenceOutside = (*nodeExpandModule)(nil)
 	_ graphNodeExpandsInstances = (*nodeExpandModule)(nil)
@@ -98,13 +98,38 @@ func (n *nodeExpandModule) ReferenceOutside() (selfPath, referencePath addrs.Mod
 	return n.Addr, n.Addr.Parent()
 }
 
-// GraphNodeEvalable
-func (n *nodeExpandModule) EvalTree() EvalNode {
-	return &evalPrepareModuleExpansion{
-		Addr:       n.Addr,
-		Config:     n.Config,
-		ModuleCall: n.ModuleCall,
+// GraphNodeExecutable
+func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) error {
+	expander := ctx.InstanceExpander()
+	_, call := n.Addr.Call()
+
+	// nodeExpandModule itself does not have visibility into how its ancestors
+	// were expanded, so we use the expander here to provide all possible paths
+	// to our module, and register module instances with each of them.
+	for _, module := range expander.ExpandModule(n.Addr.Parent()) {
+		ctx = ctx.WithPath(module)
+		switch {
+		case n.ModuleCall.Count != nil:
+			count, diags := evaluateCountExpression(n.ModuleCall.Count, ctx)
+			if diags.HasErrors() {
+				return diags.Err()
+			}
+			expander.SetModuleCount(module, call, count)
+
+		case n.ModuleCall.ForEach != nil:
+			forEach, diags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx)
+			if diags.HasErrors() {
+				return diags.Err()
+			}
+			expander.SetModuleForEach(module, call, forEach)
+
+		default:
+			expander.SetModuleSingle(module, call)
+		}
 	}
+
+	return nil
+
 }
 
 // nodeCloseModule represents an expanded module during apply, and is visited
@@ -145,88 +170,33 @@ func (n *nodeCloseModule) Name() string {
 	return n.Addr.String() + " (close)"
 }
 
-func (n *nodeCloseModule) EvalTree() EvalNode {
-	return &EvalSequence{
-		Nodes: []EvalNode{
-			&EvalOpFilter{
-				Ops: []walkOperation{walkApply, walkDestroy},
-				Node: &evalCloseModule{
-					Addr: n.Addr,
-				},
-			},
-		},
-	}
-}
+func (n *nodeCloseModule) Execute(ctx EvalContext, op walkOperation) error {
+	switch op {
+	case walkApply, walkDestroy:
+		state := ctx.State().Lock()
+		defer ctx.State().Unlock()
 
-type evalCloseModule struct {
-	Addr addrs.Module
-}
+		for modKey, mod := range state.Modules {
+			if !n.Addr.Equal(mod.Addr.Module()) {
+				continue
+			}
 
-func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) {
-	// We need the full, locked state, because SyncState does not provide a way to
-	// transact over multiple module instances at the moment.
-	state := ctx.State().Lock()
-	defer ctx.State().Unlock()
+			// clean out any empty resources
+			for resKey, res := range mod.Resources {
+				if len(res.Instances) == 0 {
+					delete(mod.Resources, resKey)
+				}
+			}
 
-	for modKey, mod := range state.Modules {
-		if !n.Addr.Equal(mod.Addr.Module()) {
-			continue
-		}
-
-		// clean out any empty resources
-		for resKey, res := range mod.Resources {
-			if len(res.Instances) == 0 {
-				delete(mod.Resources, resKey)
+			// empty child modules are always removed
+			if len(mod.Resources) == 0 && !mod.Addr.IsRoot() {
+				delete(state.Modules, modKey)
 			}
 		}
-
-		// empty child modules are always removed
-		if len(mod.Resources) == 0 && !mod.Addr.IsRoot() {
-			delete(state.Modules, modKey)
-		}
+		return nil
+	default:
+		return nil
 	}
-	return nil, nil
-}
-
-// evalPrepareModuleExpansion is an EvalNode implementation
-// that sets the count or for_each on the instance expander
-type evalPrepareModuleExpansion struct {
-	Addr       addrs.Module
-	Config     *configs.Module
-	ModuleCall *configs.ModuleCall
-}
-
-func (n *evalPrepareModuleExpansion) Eval(ctx EvalContext) (interface{}, error) {
-	expander := ctx.InstanceExpander()
-	_, call := n.Addr.Call()
-
-	// nodeExpandModule itself does not have visibility into how its ancestors
-	// were expanded, so we use the expander here to provide all possible paths
-	// to our module, and register module instances with each of them.
-	for _, module := range expander.ExpandModule(n.Addr.Parent()) {
-		ctx = ctx.WithPath(module)
-
-		switch {
-		case n.ModuleCall.Count != nil:
-			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 := evaluateForEachExpression(n.ModuleCall.ForEach, ctx)
-			if diags.HasErrors() {
-				return nil, diags.Err()
-			}
-			expander.SetModuleForEach(module, call, forEach)
-
-		default:
-			expander.SetModuleSingle(module, call)
-		}
-	}
-
-	return nil, nil
 }
 
 // nodeValidateModule wraps a nodeExpand module for validation, ensuring that
@@ -237,21 +207,7 @@ type nodeValidateModule struct {
 }
 
 // GraphNodeEvalable
-func (n *nodeValidateModule) EvalTree() EvalNode {
-	return &evalValidateModule{
-		Addr:       n.Addr,
-		Config:     n.Config,
-		ModuleCall: n.ModuleCall,
-	}
-}
-
-type evalValidateModule struct {
-	Addr       addrs.Module
-	Config     *configs.Module
-	ModuleCall *configs.ModuleCall
-}
-
-func (n *evalValidateModule) Eval(ctx EvalContext) (interface{}, error) {
+func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) error {
 	_, call := n.Addr.Call()
 	var diags tfdiags.Diagnostics
 	expander := ctx.InstanceExpander()
@@ -283,8 +239,8 @@ func (n *evalValidateModule) Eval(ctx EvalContext) (interface{}, error) {
 	}
 
 	if diags.HasErrors() {
-		return nil, diags.ErrWithWarnings()
+		return diags.ErrWithWarnings()
 	}
 
-	return nil, nil
+	return nil
 }

terraform/node_module_expand_test.go

@@ -0,0 +1,116 @@
+package terraform
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2/hcltest"
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/instances"
+	"github.com/hashicorp/terraform/states"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestNodeExpandModuleExecute(t *testing.T) {
+	ctx := &MockEvalContext{
+		InstanceExpanderExpander: instances.NewExpander(),
+	}
+	ctx.installSimpleEval()
+
+	node := nodeExpandModule{
+		Addr: addrs.Module{"child"},
+		ModuleCall: &configs.ModuleCall{
+			Count: hcltest.MockExprLiteral(cty.NumberIntVal(2)),
+		},
+	}
+
+	err := node.Execute(ctx, walkApply)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	if !ctx.InstanceExpanderCalled {
+		t.Fatal("did not expand")
+	}
+}
+
+func TestNodeCloseModuleExecute(t *testing.T) {
+	t.Run("walkApply", func(t *testing.T) {
+		state := states.NewState()
+		state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
+		ctx := &MockEvalContext{
+			StateState: state.SyncWrapper(),
+		}
+		node := nodeCloseModule{addrs.Module{"child"}}
+		err := node.Execute(ctx, walkApply)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err.Error())
+		}
+
+		// Since module.child has no resources, it should be removed
+		if _, ok := state.Modules["module.child"]; ok {
+			t.Fatal("module.child was not removed from state")
+		}
+	})
+
+	// walkImport is a no-op
+	t.Run("walkImport", func(t *testing.T) {
+		state := states.NewState()
+		state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
+		ctx := &MockEvalContext{
+			StateState: state.SyncWrapper(),
+		}
+		node := nodeCloseModule{addrs.Module{"child"}}
+
+		err := node.Execute(ctx, walkImport)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err.Error())
+		}
+		if _, ok := state.Modules["module.child"]; !ok {
+			t.Fatal("module.child was removed from state, expected no-op")
+		}
+	})
+}
+
+func TestNodeValidateModuleExecute(t *testing.T) {
+	t.Run("success", func(t *testing.T) {
+		ctx := &MockEvalContext{
+			InstanceExpanderExpander: instances.NewExpander(),
+		}
+		ctx.installSimpleEval()
+		node := nodeValidateModule{
+			nodeExpandModule{
+				Addr: addrs.Module{"child"},
+				ModuleCall: &configs.ModuleCall{
+					Count: hcltest.MockExprLiteral(cty.NumberIntVal(2)),
+				},
+			},
+		}
+
+		err := node.Execute(ctx, walkApply)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err.Error())
+		}
+	})
+
+	t.Run("invalid count", func(t *testing.T) {
+		ctx := &MockEvalContext{
+			InstanceExpanderExpander: instances.NewExpander(),
+		}
+		ctx.installSimpleEval()
+		node := nodeValidateModule{
+			nodeExpandModule{
+				Addr: addrs.Module{"child"},
+				ModuleCall: &configs.ModuleCall{
+					Count: hcltest.MockExprLiteral(cty.StringVal("invalid")),
+				},
+			},
+		}
+
+		err := node.Execute(ctx, walkApply)
+		if err == nil {
+			t.Fatal("expected error, got success")
+		}
+	})
+
+}