Module Expansion: Part 2 (#24154)

* WIP: dynamic expand

* WIP: add variable and local support

* WIP: outputs

* WIP: Add referencer

* String representation, fixing tests it impacts

* Fixes TestContext2Apply_outputOrphanModule

* Fix TestContext2Apply_plannedDestroyInterpolatedCount

* Update DestroyOutputTransformer and associated types to reflect PlannableOutputs

* Remove comment about locals

* Remove module count enablement

* Removes allowing count for modules, and reverts the test,
while adding a Skip()'d test that works when you re-enable
the config

* update TargetDownstream signature to match master

* remove unnecessary method

Co-authored-by: James Bardin <j.bardin@gmail.com>

addrs/input_variable.go

@@ -14,6 +14,15 @@ func (v InputVariable) String() string {
 	return "var." + v.Name
 }
 
+// Absolute converts the receiver into an absolute address within the given
+// module instance.
+func (v InputVariable) Absolute(m ModuleInstance) AbsInputVariableInstance {
+	return AbsInputVariableInstance{
+		Module:   m,
+		Variable: v,
+	}
+}
+
 // AbsInputVariableInstance is the address of an input variable within a
 // particular module instance.
 type AbsInputVariableInstance struct {

addrs/module.go

@@ -33,7 +33,11 @@ func (m Module) String() string {
 	if len(m) == 0 {
 		return ""
 	}
-	return strings.Join([]string(m), ".")
+	var steps []string
+	for _, s := range m {
+		steps = append(steps, "module", s)
+	}
+	return strings.Join(steps, ".")
 }
 
 // Child returns the address of a child call in the receiver, identified by the

command/testdata/show-json/modules/output.json

@@ -274,8 +274,8 @@
             }
         },
         "provider_config": {
-            "module_test_foo:test": {
+            "module.module_test_foo:test": {
-                "module_address": "module_test_foo",
+                "module_address": "module.module_test_foo",
                 "name": "test"
             }
         }

internal/modsdir/manifest.go

@@ -8,6 +8,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"strings"
 
 	version "github.com/hashicorp/go-version"
 
@@ -48,7 +49,11 @@ type Record struct {
 type Manifest map[string]Record
 
 func (m Manifest) ModuleKey(path addrs.Module) string {
-	return path.String()
+	if len(path) == 0 {
+		return ""
+	}
+	return strings.Join([]string(path), ".")
+
 }
 
 // manifestSnapshotFile is an internal struct used only to assist in our JSON

terraform/context_apply_test.go

@@ -767,6 +767,7 @@ aws_instance.foo:
 }
 
 func TestContext2Apply_emptyModule(t *testing.T) {
+	// A module with only outputs (no resources)
 	m := testModule(t, "apply-empty-module")
 	p := testProvider("aws")
 	p.ApplyFn = testApplyFn

terraform/context_plan_test.go

@@ -429,6 +429,81 @@ func TestContext2Plan_modules(t *testing.T) {
 		checkVals(t, expected, ric.After)
 	}
 }
+func TestContext2Plan_moduleCount(t *testing.T) {
+	// This test is skipped with count disabled.
+	t.Skip()
+	//FIXME: add for_each and single modules to this test
+
+	m := testModule(t, "plan-modules-count")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[addrs.Provider]providers.Factory{
+				addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p),
+			},
+		),
+	})
+
+	plan, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors: %s", diags.Err())
+	}
+
+	if len(plan.Changes.Resources) != 6 {
+		t.Error("expected 6 resource in plan, got", len(plan.Changes.Resources))
+	}
+
+	schema := p.GetSchemaReturn.ResourceTypes["aws_instance"]
+	ty := schema.ImpliedType()
+
+	expectFoo := objectVal(t, schema, map[string]cty.Value{
+		"id":   cty.UnknownVal(cty.String),
+		"foo":  cty.StringVal("2"),
+		"type": cty.StringVal("aws_instance")},
+	)
+
+	expectNum := objectVal(t, schema, map[string]cty.Value{
+		"id":   cty.UnknownVal(cty.String),
+		"num":  cty.NumberIntVal(2),
+		"type": cty.StringVal("aws_instance"),
+	})
+
+	expectExpansion := objectVal(t, schema, map[string]cty.Value{
+		"bar":  cty.StringVal("baz"),
+		"id":   cty.UnknownVal(cty.String),
+		"num":  cty.NumberIntVal(2),
+		"type": cty.StringVal("aws_instance"),
+	})
+
+	for _, res := range plan.Changes.Resources {
+		if res.Action != plans.Create {
+			t.Fatalf("expected resource creation, got %s", res.Action)
+		}
+		ric, err := res.Decode(ty)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		var expected cty.Value
+		switch i := ric.Addr.String(); i {
+		case "aws_instance.bar":
+			expected = expectFoo
+		case "aws_instance.foo":
+			expected = expectNum
+		case "module.child[0].aws_instance.foo[0]",
+			"module.child[0].aws_instance.foo[1]",
+			"module.child[1].aws_instance.foo[0]",
+			"module.child[1].aws_instance.foo[1]":
+			expected = expectExpansion
+		default:
+			t.Fatal("unknown instance:", i)
+		}
+
+		checkVals(t, expected, ric.After)
+	}
+}
 
 // GH-1475
 func TestContext2Plan_moduleCycle(t *testing.T) {

terraform/eval_state.go

@@ -490,15 +490,18 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
 
 	// We'll record our expansion decision in the shared "expander" object
 	// so that later operations (i.e. DynamicExpand and expression evaluation)
-	// can refer to it.
+	// 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()
+	for _, module := range expander.ExpandModule(ctx.Path().Module()) {
 		switch eachMode {
 		case states.EachList:
-		expander.SetResourceCount(ctx.Path(), n.Addr, count)
+			expander.SetResourceCount(module, n.Addr, count)
 		case states.EachMap:
-		expander.SetResourceForEach(ctx.Path(), n.Addr, forEach)
+			expander.SetResourceForEach(module, n.Addr, forEach)
 		default:
-		expander.SetResourceSingle(ctx.Path(), n.Addr)
+			expander.SetResourceSingle(module, n.Addr)
+		}
 	}
 
 	return nil, nil

terraform/node_module_expand.go

@@ -5,6 +5,8 @@ import (
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/lang"
+	"github.com/hashicorp/terraform/states"
 )
 
 // nodeExpandModule represents a module call in the configuration that
@@ -12,8 +14,10 @@ import (
 // configured.
 type nodeExpandModule struct {
 	CallerAddr addrs.ModuleInstance
+	Addr       addrs.Module
 	Call       addrs.ModuleCall
 	Config     *configs.Module
+	ModuleCall *configs.ModuleCall
 }
 
 var (
@@ -29,14 +33,20 @@ func (n *nodeExpandModule) Name() string {
 
 // GraphNodeSubPath implementation
 func (n *nodeExpandModule) Path() addrs.ModuleInstance {
-	// Notice that the node represents the module call and so we report
+	// This node represents the module call within a module,
-	// the parent module as the path. The module call we're representing
+	// so return the CallerAddr as the path as the module
-	// might expand into multiple child module instances during our work here.
+	// call may expand into multiple child instances
 	return n.CallerAddr
 }
 
 // GraphNodeReferencer implementation
 func (n *nodeExpandModule) References() []*addrs.Reference {
+	var refs []*addrs.Reference
+
+	if n.ModuleCall == nil {
+		return nil
+	}
+
 	// Expansion only uses the count and for_each expressions, so this
 	// particular graph node only refers to those.
 	// Individual variable values in the module call definition might also
@@ -47,18 +57,14 @@ func (n *nodeExpandModule) References() []*addrs.Reference {
 	// our call, these references will be correctly interpreted as being
 	// in the calling module's namespace, not the namespaces of any of the
 	// child module instances we might expand to during our evaluation.
-	var ret []*addrs.Reference
+
-	// TODO: Once count and for_each are actually supported, analyze their
+	if n.ModuleCall.Count != nil {
-	// expressions for references here.
+		refs, _ = lang.ReferencesInExpr(n.ModuleCall.Count)
-	/*
-		if n.Config.Count != nil {
-			ret = append(ret, n.Config.Count.References()...)
 	}
-		if n.Config.ForEach != nil {
+	if n.ModuleCall.ForEach != nil {
-			ret = append(ret, n.Config.ForEach.References()...)
+		refs, _ = lang.ReferencesInExpr(n.ModuleCall.ForEach)
 	}
-	*/
+	return appendResourceDestroyReferences(refs)
-	return ret
 }
 
 // RemovableIfNotTargeted implementation
@@ -74,23 +80,56 @@ func (n *nodeExpandModule) EvalTree() EvalNode {
 		CallerAddr: n.CallerAddr,
 		Call:       n.Call,
 		Config:     n.Config,
+		ModuleCall: n.ModuleCall,
 	}
 }
 
+// evalPrepareModuleExpansion is an EvalNode implementation
+// that sets the count or for_each on the instance expander
 type evalPrepareModuleExpansion struct {
 	CallerAddr addrs.ModuleInstance
 	Call       addrs.ModuleCall
 	Config     *configs.Module
+	ModuleCall *configs.ModuleCall
 }
 
 func (n *evalPrepareModuleExpansion) Eval(ctx EvalContext) (interface{}, error) {
-	// Modules don't support any of the repetition arguments yet, so their
+	eachMode := states.NoEach
-	// expansion type is always "single". We just record this here to make
+	expander := ctx.InstanceExpander()
-	// the expander data structure consistent for now.
+
-	// FIXME: Once the rest of Terraform Core is ready to support expanding
+	if n.ModuleCall == nil {
-	// modules, evaluate the "count" and "for_each" arguments here in a
+		// FIXME: should we have gotten here with no module call?
-	// similar way as in EvalWriteResourceState.
 		log.Printf("[TRACE] evalPrepareModuleExpansion: %s is a singleton", n.CallerAddr.Child(n.Call.Name, addrs.NoKey))
-	ctx.InstanceExpander().SetModuleSingle(n.CallerAddr, n.Call)
+		expander.SetModuleSingle(n.CallerAddr, n.Call)
+		return nil, nil
+	}
+
+	count, countDiags := evaluateResourceCountExpression(n.ModuleCall.Count, ctx)
+	if countDiags.HasErrors() {
+		return nil, countDiags.Err()
+	}
+
+	if count >= 0 { // -1 signals "count not set"
+		eachMode = states.EachList
+	}
+
+	forEach, forEachDiags := evaluateResourceForEachExpression(n.ModuleCall.ForEach, ctx)
+	if forEachDiags.HasErrors() {
+		return nil, forEachDiags.Err()
+	}
+
+	if forEach != nil {
+		eachMode = states.EachMap
+	}
+
+	switch eachMode {
+	case states.EachList:
+		expander.SetModuleCount(ctx.Path(), n.Call, count)
+	case states.EachMap:
+		expander.SetModuleForEach(ctx.Path(), n.Call, forEach)
+	default:
+		expander.SetModuleSingle(n.CallerAddr, n.Call)
+	}
+
 	return nil, nil
 }

terraform/node_module_removed.go

@@ -39,12 +39,12 @@ func (n *NodeModuleRemoved) EvalTree() EvalNode {
 	}
 }
 
-func (n *NodeModuleRemoved) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) {
+func (n *NodeModuleRemoved) ReferenceOutside() (selfPath, referencePath addrs.Module) {
 	// Our "References" implementation indicates that this node depends on
 	// the call to the module it represents, which implicitly depends on
 	// everything inside the module. That reference must therefore be
 	// interpreted in terms of our parent module.
-	return n.Addr, n.Addr.Parent()
+	return n.Addr.Module(), n.Addr.Parent().Module()
 }
 
 func (n *NodeModuleRemoved) References() []*addrs.Reference {

terraform/node_module_variable.go

@@ -1,6 +1,8 @@
 package terraform
 
 import (
+	"fmt"
+
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
@@ -9,6 +11,101 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
+// NodePlannableModuleVariable is the placeholder for an variable that has not yet had
+// its module path expanded.
+type NodePlannableModuleVariable struct {
+	Addr   addrs.InputVariable
+	Module addrs.Module
+	Config *configs.Variable
+	Expr   hcl.Expression
+}
+
+var (
+	_ GraphNodeDynamicExpandable = (*NodePlannableModuleVariable)(nil)
+	_ GraphNodeReferenceOutside  = (*NodePlannableModuleVariable)(nil)
+	_ GraphNodeReferenceable     = (*NodePlannableModuleVariable)(nil)
+	_ GraphNodeReferencer        = (*NodePlannableModuleVariable)(nil)
+	_ GraphNodeSubPath           = (*NodePlannableModuleVariable)(nil)
+	_ RemovableIfNotTargeted     = (*NodePlannableModuleVariable)(nil)
+)
+
+func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) {
+	var g Graph
+	expander := ctx.InstanceExpander()
+	for _, module := range expander.ExpandModule(ctx.Path().Module()) {
+		o := &NodeApplyableModuleVariable{
+			Addr:   n.Addr.Absolute(module),
+			Config: n.Config,
+			Expr:   n.Expr,
+		}
+		g.Add(o)
+	}
+	return &g, nil
+}
+
+func (n *NodePlannableModuleVariable) Name() string {
+	return fmt.Sprintf("%s.%s", n.Module, n.Addr.String())
+}
+
+// GraphNodeSubPath
+func (n *NodePlannableModuleVariable) Path() addrs.ModuleInstance {
+	// Return an UnkeyedInstanceShim as our placeholder,
+	// given that modules will be unexpanded at this point in the walk
+	return n.Module.UnkeyedInstanceShim()
+}
+
+// GraphNodeReferencer
+func (n *NodePlannableModuleVariable) 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.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
+}
+
+// GraphNodeReferenceOutside implementation
+func (n *NodePlannableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
+	return n.Module, n.Module.Parent()
+}
+
+// GraphNodeReferenceable
+func (n *NodePlannableModuleVariable) 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
+	_, call := n.Module.Call()
+	return []addrs.Referenceable{n.Addr, call}
+}
+
+// RemovableIfNotTargeted
+func (n *NodePlannableModuleVariable) RemoveIfNotTargeted() bool {
+	return true
+}
+
+// GraphNodeTargetDownstream
+func (n *NodePlannableModuleVariable) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool {
+	return true
+}
+
 // NodeApplyableModuleVariable represents a module variable input during
 // the apply step.
 type NodeApplyableModuleVariable struct {
@@ -48,15 +145,15 @@ func (n *NodeApplyableModuleVariable) RemoveIfNotTargeted() bool {
 }
 
 // GraphNodeReferenceOutside implementation
-func (n *NodeApplyableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) {
+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()
+	referencePath = n.Addr.Module.Parent().Module()
 
 	// Input variables are _referenced_ from their own module, though.
-	selfPath = n.Addr.Module
+	selfPath = n.Addr.Module.Module()
 
 	return // uses named return values
 }

terraform/node_output.go

@@ -9,6 +9,98 @@ import (
 	"github.com/hashicorp/terraform/lang"
 )
 
+// NodePlannableOutput is the placeholder for an output that has not yet had
+// its module path expanded.
+type NodePlannableOutput struct {
+	Addr   addrs.OutputValue
+	Module addrs.Module
+	Config *configs.Output
+}
+
+var (
+	_ GraphNodeSubPath       = (*NodePlannableOutput)(nil)
+	_ RemovableIfNotTargeted = (*NodePlannableOutput)(nil)
+	_ GraphNodeReferenceable = (*NodePlannableOutput)(nil)
+	//_ GraphNodeEvalable          = (*NodePlannableOutput)(nil)
+	_ GraphNodeReferencer        = (*NodePlannableOutput)(nil)
+	_ GraphNodeDynamicExpandable = (*NodePlannableOutput)(nil)
+)
+
+func (n *NodePlannableOutput) DynamicExpand(ctx EvalContext) (*Graph, error) {
+	var g Graph
+	expander := ctx.InstanceExpander()
+	for _, module := range expander.ExpandModule(ctx.Path().Module()) {
+		o := &NodeApplyableOutput{
+			Addr:   n.Addr.Absolute(module),
+			Config: n.Config,
+		}
+		// log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o)
+		g.Add(o)
+	}
+	return &g, nil
+}
+
+func (n *NodePlannableOutput) Name() string {
+	return n.Addr.Absolute(n.Module.UnkeyedInstanceShim()).String()
+}
+
+// GraphNodeSubPath
+func (n *NodePlannableOutput) Path() addrs.ModuleInstance {
+	// Return an UnkeyedInstanceShim as our placeholder,
+	// given that modules will be unexpanded at this point in the walk
+	return n.Module.UnkeyedInstanceShim()
+}
+
+// GraphNodeReferenceable
+func (n *NodePlannableOutput) ReferenceableAddrs() []addrs.Referenceable {
+	// An output in the root module can't be referenced at all.
+	if n.Module.IsRoot() {
+		return nil
+	}
+
+	// the output is referenced through the module call, and via the
+	// module itself.
+	_, call := n.Module.Call()
+
+	// FIXME: make something like ModuleCallOutput for this type of reference
+	// that doesn't need an instance shim
+	callOutput := addrs.ModuleCallOutput{
+		Call: call.Instance(addrs.NoKey),
+		Name: n.Addr.Name,
+	}
+
+	// Otherwise, we can reference the output via the
+	// module call itself
+	return []addrs.Referenceable{call, callOutput}
+}
+
+// GraphNodeReferenceOutside implementation
+func (n *NodePlannableOutput) ReferenceOutside() (selfPath, referencePath addrs.Module) {
+	// Output values have their expressions resolved in the context of the
+	// module where they are defined.
+	referencePath = n.Module
+
+	// ...but they are referenced in the context of their calling module.
+	selfPath = referencePath.Parent()
+
+	return // uses named return values
+}
+
+// GraphNodeReferencer
+func (n *NodePlannableOutput) References() []*addrs.Reference {
+	return appendResourceDestroyReferences(referencesForOutput(n.Config))
+}
+
+// RemovableIfNotTargeted
+func (n *NodePlannableOutput) RemoveIfNotTargeted() bool {
+	return true
+}
+
+// GraphNodeTargetDownstream
+func (n *NodePlannableOutput) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool {
+	return true
+}
+
 // NodeApplyableOutput represents an output that is "applyable":
 // it is ready to be applied.
 type NodeApplyableOutput struct {
@@ -51,21 +143,19 @@ func (n *NodeApplyableOutput) TargetDownstream(targetedDeps, untargetedDeps dag.
 	return true
 }
 
-func referenceOutsideForOutput(addr addrs.AbsOutputValue) (selfPath, referencePath addrs.ModuleInstance) {
+func referenceOutsideForOutput(addr addrs.AbsOutputValue) (selfPath, referencePath addrs.Module) {
-
 	// Output values have their expressions resolved in the context of the
 	// module where they are defined.
-	referencePath = addr.Module
+	referencePath = addr.Module.Module()
 
 	// ...but they are referenced in the context of their calling module.
-	selfPath = addr.Module.Parent()
+	selfPath = addr.Module.Parent().Module()
 
 	return // uses named return values
-
 }
 
 // GraphNodeReferenceOutside implementation
-func (n *NodeApplyableOutput) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) {
+func (n *NodeApplyableOutput) ReferenceOutside() (selfPath, referencePath addrs.Module) {
 	return referenceOutsideForOutput(n.Addr)
 }
 
@@ -83,8 +173,8 @@ func referenceableAddrsForOutput(addr addrs.AbsOutputValue) []addrs.Referenceabl
 	// was declared.
 	_, outp := addr.ModuleCallOutput()
 	_, call := addr.Module.CallInstance()
-	return []addrs.Referenceable{outp, call}
 
+	return []addrs.Referenceable{outp, call}
 }
 
 // GraphNodeReferenceable
@@ -141,7 +231,8 @@ func (n *NodeApplyableOutput) DotNode(name string, opts *dag.DotOpts) *dag.DotNo
 // NodeDestroyableOutput represents an output that is "destroybale":
 // its application will remove the output from the state.
 type NodeDestroyableOutput struct {
-	Addr   addrs.AbsOutputValue
+	Addr   addrs.OutputValue
+	Module addrs.Module
 	Config *configs.Output // Config is the output in the config
 }
 
@@ -160,7 +251,7 @@ func (n *NodeDestroyableOutput) Name() string {
 
 // GraphNodeSubPath
 func (n *NodeDestroyableOutput) Path() addrs.ModuleInstance {
-	return n.Addr.Module
+	return n.Module.UnkeyedInstanceShim()
 }
 
 // RemovableIfNotTargeted
@@ -184,7 +275,7 @@ func (n *NodeDestroyableOutput) References() []*addrs.Reference {
 // GraphNodeEvalable
 func (n *NodeDestroyableOutput) EvalTree() EvalNode {
 	return &EvalDeleteOutput{
-		Addr: n.Addr.OutputValue,
+		Addr: n.Addr,
 	}
 }
 

terraform/node_output_orphan.go

@@ -23,7 +23,7 @@ func (n *NodeOutputOrphan) Name() string {
 }
 
 // GraphNodeReferenceOutside implementation
-func (n *NodeOutputOrphan) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) {
+func (n *NodeOutputOrphan) ReferenceOutside() (selfPath, referencePath addrs.Module) {
 	return referenceOutsideForOutput(n.Addr)
 }
 

terraform/node_resource_abstract.go

@@ -43,6 +43,10 @@ type GraphNodeResourceInstance interface {
 // operations. It registers all the interfaces for a resource that common
 // across multiple operation types.
 type NodeAbstractResource struct {
+	//FIXME: AbstractResources are no longer absolute, because modules are not expanded.
+	// Addr addrs.Resource
+	// Module addrs.Module
+
 	Addr addrs.AbsResource // Addr is the address for this resource
 
 	// The fields below will be automatically set using the Attach

terraform/node_resource_refresh.go

@@ -14,7 +14,7 @@ import (
 	"github.com/hashicorp/terraform/tfdiags"
 )
 
-// NodeRefreshableManagedResource represents a resource that is expanabled into
+// NodeRefreshableManagedResource represents a resource that is expandable into
 // NodeRefreshableManagedResourceInstance. Resource count orphans are also added.
 type NodeRefreshableManagedResource struct {
 	*NodeAbstractResource
@@ -61,13 +61,16 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph,
 	// 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()
+
+	for _, module := range expander.ExpandModule(ctx.Path().Module()) {
 		switch {
 		case count >= 0:
-		expander.SetResourceCount(ctx.Path(), n.ResourceAddr().Resource, count)
+			expander.SetResourceCount(module, n.ResourceAddr().Resource, count)
 		case forEachMap != nil:
-		expander.SetResourceForEach(ctx.Path(), n.ResourceAddr().Resource, forEachMap)
+			expander.SetResourceForEach(module, n.ResourceAddr().Resource, forEachMap)
 		default:
-		expander.SetResourceSingle(ctx.Path(), n.ResourceAddr().Resource)
+			expander.SetResourceSingle(module, n.ResourceAddr().Resource)
+		}
 	}
 	instanceAddrs := expander.ExpandResource(ctx.Path().Module(), n.ResourceAddr().Resource)
 

terraform/testdata/plan-modules-count/child/main.tf

@@ -0,0 +1,12 @@
+variable "foo" {}
+variable "bar" {}
+
+resource "aws_instance" "foo" {
+  count = 2
+  num = var.foo
+  bar = "baz" #var.bar
+}
+
+output "out" {
+  value = aws_instance.foo[0].id
+}

terraform/testdata/plan-modules-count/main.tf

@@ -0,0 +1,28 @@
+locals {
+  val = 2
+  bar = "baz"
+}
+
+variable "myvar" {
+  default = "baz"
+}
+
+
+module "child" {
+  count = local.val
+  foo = 2
+  bar = var.myvar
+  source = "./child"
+}
+
+output "out" {
+  value = module.child[*].out
+}
+
+resource "aws_instance" "foo" {
+  num = 2
+}
+
+resource "aws_instance" "bar" {
+  foo = "${aws_instance.foo.num}"
+}

terraform/transform_module_expansion.go

@@ -43,10 +43,12 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare
 	fullAddr := c.Path.UnkeyedInstanceShim()
 	callerAddr, callAddr := fullAddr.Call()
 
+	modulecall := c.Parent.Module.ModuleCalls["child"]
 	v := &nodeExpandModule{
 		CallerAddr: callerAddr,
 		Call:       callAddr,
 		Config:     c.Module,
+		ModuleCall: modulecall,
 	}
 	g.Add(v)
 	log.Printf("[TRACE] ModuleExpansionTransformer: Added %s as %T", fullAddr, v)

terraform/transform_module_variable.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/zclconf/go-cty/cty"
 
@@ -110,15 +111,17 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs
 			}
 		}
 
-		// For now we treat all module variables as "applyable", even though
+		// Add a plannable node, as the variable may expand
-		// such nodes are valid to use on other walks too. We may specialize
+		// during module expansion
-		// this in future if we find reasons to employ different behaviors
+		node := &NodePlannableModuleVariable{
-		// in different scenarios.
+			Addr: addrs.InputVariable{
-		node := &NodeApplyableModuleVariable{
+				Name: v.Name,
-			Addr:   path.InputVariable(v.Name),
+			},
+			Module: c.Path,
 			Config: v,
 			Expr:   expr,
 		}
+
 		g.Add(node)
 	}
 

terraform/transform_output.go

@@ -3,6 +3,7 @@ package terraform
 import (
 	"log"
 
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/dag"
 )
@@ -36,20 +37,16 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error {
 		}
 	}
 
-	// Our addressing system distinguishes between modules and module instances,
+	// Add plannable outputs to the graph, which will be dynamically expanded
-	// but we're not yet ready to make that distinction here (since we don't
+	// into NodeApplyableOutputs to reflect possible expansion
-	// support "count"/"for_each" on modules) and so we just do a naive
+	// through the presence of "count" or "for_each" on the modules.
-	// transform of the module path into a module instance path, assuming that
-	// no keys are in use. This should be removed when "count" and "for_each"
-	// are implemented for modules.
-	path := c.Path.UnkeyedInstanceShim()
-
 	for _, o := range c.Module.Outputs {
-		addr := path.OutputValue(o.Name)
+		node := &NodePlannableOutput{
-		node := &NodeApplyableOutput{
+			Addr:   addrs.OutputValue{Name: o.Name},
-			Addr:   addr,
+			Module: c.Path,
 			Config: o,
 		}
+		log.Printf("[TRACE] OutputTransformer: adding %s as %T", o.Name, node)
 		g.Add(node)
 	}
 
@@ -70,7 +67,7 @@ func (t *DestroyOutputTransformer) Transform(g *Graph) error {
 	}
 
 	for _, v := range g.Vertices() {
-		output, ok := v.(*NodeApplyableOutput)
+		output, ok := v.(*NodePlannableOutput)
 		if !ok {
 			continue
 		}
@@ -78,6 +75,7 @@ func (t *DestroyOutputTransformer) Transform(g *Graph) error {
 		// create the destroy node for this output
 		node := &NodeDestroyableOutput{
 			Addr:   output.Addr,
+			Module: output.Module,
 			Config: output.Config,
 		}
 

terraform/transform_reference.go

@@ -66,7 +66,7 @@ type GraphNodeAttachDependencies interface {
 type GraphNodeReferenceOutside interface {
 	// ReferenceOutside returns a path in which any references from this node
 	// are resolved.
-	ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance)
+	ReferenceOutside() (selfPath, referencePath addrs.Module)
 }
 
 // ReferenceTransformer is a GraphTransformer that connects all the
@@ -85,8 +85,7 @@ func (t *ReferenceTransformer) Transform(g *Graph) error {
 			// use their own state.
 			continue
 		}
-
+		parents := m.References(v)
-		parents, _ := m.References(v)
 		parentsDbg := make([]string, len(parents))
 		for i, v := range parents {
 			parentsDbg[i] = dag.VertexName(v)
@@ -196,7 +195,12 @@ func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
 				if v.Addr.Module.IsRoot() && !t.Destroy {
 					continue
 				}
-			case *NodeLocal, *NodeApplyableModuleVariable:
+			case *NodePlannableOutput:
+				// Have similar guardrails for plannable outputs as applyable above
+				if v.Module.IsRoot() && !t.Destroy {
+					continue
+				}
+			case *NodeLocal, *NodeApplyableModuleVariable, *NodePlannableModuleVariable:
 				// OK
 			default:
 				// We're only concerned with variables, locals and outputs
@@ -239,27 +243,20 @@ type ReferenceMap struct {
 	// A particular reference key might actually identify multiple vertices,
 	// e.g. in situations where one object is contained inside another.
 	vertices map[string][]dag.Vertex
-
-	// edges is a map whose keys are a subset of the internal reference keys
-	// from "vertices", and whose values are the nodes that refer to each
-	// key. The values in this map are the referrers, while values in
-	// "verticies" are the referents. The keys in both cases are referents.
-	edges map[string][]dag.Vertex
 }
 
 // References returns the set of vertices that the given vertex refers to,
 // and any referenced addresses that do not have corresponding vertices.
-func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []addrs.Referenceable) {
+func (m *ReferenceMap) References(v dag.Vertex) []dag.Vertex {
 	rn, ok := v.(GraphNodeReferencer)
 	if !ok {
-		return nil, nil
+		return nil
 	}
 	if _, ok := v.(GraphNodeSubPath); !ok {
-		return nil, nil
+		return nil
 	}
 
 	var matches []dag.Vertex
-	var missing []addrs.Referenceable
 
 	for _, ref := range rn.References() {
 		subject := ref.Subject
@@ -278,7 +275,6 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []addrs.Reference
 			}
 			key = m.referenceMapKey(v, subject)
 		}
-
 		vertices := m.vertices[key]
 		for _, rv := range vertices {
 			// don't include self-references
@@ -287,47 +283,6 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []addrs.Reference
 			}
 			matches = append(matches, rv)
 		}
-		if len(vertices) == 0 {
-			missing = append(missing, ref.Subject)
-		}
-	}
-
-	return matches, missing
-}
-
-// Referrers returns the set of vertices that refer to the given vertex.
-func (m *ReferenceMap) Referrers(v dag.Vertex) []dag.Vertex {
-	rn, ok := v.(GraphNodeReferenceable)
-	if !ok {
-		return nil
-	}
-	sp, ok := v.(GraphNodeSubPath)
-	if !ok {
-		return nil
-	}
-
-	var matches []dag.Vertex
-	for _, addr := range rn.ReferenceableAddrs() {
-		key := m.mapKey(sp.Path(), addr)
-		referrers, ok := m.edges[key]
-		if !ok {
-			continue
-		}
-
-		// If the referrer set includes our own given vertex then we skip,
-		// since we don't want to return self-references.
-		selfRef := false
-		for _, p := range referrers {
-			if p == v {
-				selfRef = true
-				break
-			}
-		}
-		if selfRef {
-			continue
-		}
-
-		matches = append(matches, referrers...)
 	}
 
 	return matches
@@ -354,7 +309,7 @@ func (m *ReferenceMap) vertexReferenceablePath(v dag.Vertex) addrs.ModuleInstanc
 		// Vertex is referenced from a different module than where it was
 		// declared.
 		path, _ := outside.ReferenceOutside()
-		return path
+		return path.UnkeyedInstanceShim()
 	}
 
 	// Vertex is referenced from the same module as where it was declared.
@@ -373,12 +328,11 @@ func vertexReferencePath(referrer dag.Vertex) addrs.ModuleInstance {
 		panic(fmt.Errorf("vertexReferencePath on vertex type %T which doesn't implement GraphNodeSubPath", sp))
 	}
 
-	var path addrs.ModuleInstance
 	if outside, ok := referrer.(GraphNodeReferenceOutside); ok {
 		// Vertex makes references to objects in a different module than where
 		// it was declared.
-		_, path = outside.ReferenceOutside()
+		_, path := outside.ReferenceOutside()
-		return path
+		return path.UnkeyedInstanceShim()
 	}
 
 	// Vertex makes references to objects in the same module as where it
@@ -443,34 +397,7 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap {
 		}
 	}
 
-	// Build the lookup table for referenced by
-	edges := make(map[string][]dag.Vertex)
-	for _, v := range vs {
-		_, ok := v.(GraphNodeSubPath)
-		if !ok {
-			// Only nodes with paths can participate in a reference map.
-			continue
-		}
-
-		rn, ok := v.(GraphNodeReferencer)
-		if !ok {
-			// We're only looking for referenceable nodes
-			continue
-		}
-
-		// Go through and cache them
-		for _, ref := range rn.References() {
-			if ref.Subject == nil {
-				// Should never happen
-				panic(fmt.Sprintf("%T.References returned reference with nil subject", rn))
-			}
-			key := m.referenceMapKey(v, ref.Subject)
-			edges[key] = append(edges[key], v)
-		}
-	}
-
 	m.vertices = vertices
-	m.edges = edges
 	return &m
 }
 
@@ -503,6 +430,8 @@ func appendResourceDestroyReferences(refs []*addrs.Reference) []*addrs.Reference
 			newRef.Subject = tr.Phase(addrs.ResourceInstancePhaseDestroy)
 			refs = append(refs, &newRef)
 		}
+		// FIXME: Using this method in module expansion references,
+		// May want to refactor this method beyond resources
 	}
 	return refs
 }

terraform/transform_reference_test.go

@@ -113,55 +113,7 @@ func TestReferenceMapReferences(t *testing.T) {
 	for tn, tc := range cases {
 		t.Run(tn, func(t *testing.T) {
 			rm := NewReferenceMap(tc.Nodes)
-			result, _ := rm.References(tc.Check)
+			result := rm.References(tc.Check)
-
-			var resultStr []string
-			for _, v := range result {
-				resultStr = append(resultStr, dag.VertexName(v))
-			}
-
-			sort.Strings(resultStr)
-			sort.Strings(tc.Result)
-			if !reflect.DeepEqual(resultStr, tc.Result) {
-				t.Fatalf("bad: %#v", resultStr)
-			}
-		})
-	}
-}
-
-func TestReferenceMapReferencedBy(t *testing.T) {
-	cases := map[string]struct {
-		Nodes  []dag.Vertex
-		Check  dag.Vertex
-		Result []string
-	}{
-		"simple": {
-			Nodes: []dag.Vertex{
-				&graphNodeRefChildTest{
-					NameValue: "A",
-					Refs:      []string{"A"},
-				},
-				&graphNodeRefChildTest{
-					NameValue: "B",
-					Refs:      []string{"A"},
-				},
-				&graphNodeRefChildTest{
-					NameValue: "C",
-					Refs:      []string{"B"},
-				},
-			},
-			Check: &graphNodeRefParentTest{
-				NameValue: "foo",
-				Names:     []string{"A"},
-			},
-			Result: []string{"A", "B"},
-		},
-	}
-
-	for tn, tc := range cases {
-		t.Run(tn, func(t *testing.T) {
-			rm := NewReferenceMap(tc.Nodes)
-			result := rm.Referrers(tc.Check)
 
 			var resultStr []string
 			for _, v := range result {