terraform: module variables should be pruned if nothing depends on them

terraform/graph_builder_apply.go

@@ -105,12 +105,12 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Add root variables
 		&RootVariableTransformer{Module: b.Module},
 
-		// Add module variables
-		&ModuleVariableTransformer{Module: b.Module},
-
 		// Add the outputs
 		&OutputTransformer{Module: b.Module},
 
+		// Add module variables
+		&ModuleVariableTransformer{Module: b.Module},
+
 		// Connect references so ordering is correct
 		&ReferenceTransformer{},
 

terraform/transform_module_variable.go

@@ -5,16 +5,18 @@ import (
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
+	"github.com/hashicorp/terraform/dag"
 )
 
 // ModuleVariableTransformer is a GraphTransformer that adds all the variables
 // in the configuration to the graph.
 //
-// This only adds variables that either have no dependencies (and therefore
+// This only adds variables that are referenced by other thigns in the graph.
-// always succeed) or has dependencies that are 100% represented in the
+// If a module variable is not referenced, it won't be added to the graph.
-// graph.
 type ModuleVariableTransformer struct {
 	Module *module.Tree
+
+	DisablePrune bool // True if pruning unreferenced should be disabled
 }
 
 func (t *ModuleVariableTransformer) Transform(g *Graph) error {
@@ -27,18 +29,18 @@ func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree)
 		return nil
 	}
 
-	// If we have a parent, we can determine if a module variable is being
+	// Transform all the children. This must be done BEFORE the transform
-	// used, so we transform this.
+	// above since child module variables can reference parent module variables.
-	if parent != nil {
+	for _, c := range m.Children() {
-		if err := t.transformSingle(g, parent, m); err != nil {
+		if err := t.transform(g, m, c); err != nil {
 			return err
 		}
 	}
 
-	// Transform all the children. This must be done AFTER the transform
+	// If we have a parent, we can determine if a module variable is being
-	// above since child module variables can reference parent module variables.
+	// used, so we transform this.
-	for _, c := range m.Children() {
+	if parent != nil {
-		if err := t.transform(g, m, c); err != nil {
+		if err := t.transformSingle(g, parent, m); err != nil {
 			return err
 		}
 	}
@@ -67,6 +69,9 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, m *module.
 		return nil
 	}
 
+	// Build the reference map so we can determine if we're referencing things.
+	refMap := NewReferenceMap(g.Vertices())
+
 	// Add all variables here
 	for _, v := range vars {
 		// Determine the value of the variable. If it isn't in the
@@ -96,6 +101,17 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, m *module.
 			Module:    t.Module,
 		}
 
+		if !t.DisablePrune {
+			// If the node is not referenced by anything, then we don't need
+			// to include it since it won't be used.
+			if matches := refMap.ReferencedBy(node); len(matches) == 0 {
+				log.Printf(
+					"[INFO] Not including %q in graph, nothing depends on it",
+					dag.VertexName(node))
+				continue
+			}
+		}
+
 		// Add it!
 		g.Add(node)
 	}

terraform/transform_module_variable_test.go

@@ -17,7 +17,7 @@ func TestModuleVariableTransformer(t *testing.T) {
 	}
 
 	{
-		tf := &ModuleVariableTransformer{Module: module}
+		tf := &ModuleVariableTransformer{Module: module, DisablePrune: true}
 		if err := tf.Transform(&g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
@@ -42,7 +42,7 @@ func TestModuleVariableTransformer_nested(t *testing.T) {
 	}
 
 	{
-		tf := &ModuleVariableTransformer{Module: module}
+		tf := &ModuleVariableTransformer{Module: module, DisablePrune: true}
 		if err := tf.Transform(&g); err != nil {
 			t.Fatalf("err: %s", err)
 		}

terraform/transform_reference.go

@@ -66,7 +66,8 @@ func (t *ReferenceTransformer) Transform(g *Graph) error {
 type ReferenceMap struct {
 	// m is the mapping of referenceable name to list of verticies that
 	// implement that name. This is built on initialization.
-	m map[string][]dag.Vertex
+	references   map[string][]dag.Vertex
+	referencedBy map[string][]dag.Vertex
 }
 
 // References returns the list of vertices that this vertex
@@ -82,7 +83,7 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) {
 	prefix := m.prefix(v)
 	for _, n := range rn.References() {
 		n = prefix + n
-		parents, ok := m.m[n]
+		parents, ok := m.references[n]
 		if !ok {
 			missing = append(missing, n)
 			continue
@@ -106,6 +107,41 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) {
 	return matches, missing
 }
 
+// ReferencedBy returns the list of vertices that reference the
+// vertex passed in.
+func (m *ReferenceMap) ReferencedBy(v dag.Vertex) []dag.Vertex {
+	rn, ok := v.(GraphNodeReferenceable)
+	if !ok {
+		return nil
+	}
+
+	var matches []dag.Vertex
+	prefix := m.prefix(v)
+	for _, n := range rn.ReferenceableName() {
+		n = prefix + n
+		children, ok := m.referencedBy[n]
+		if !ok {
+			continue
+		}
+
+		// Make sure this isn't a self reference, which isn't included
+		selfRef := false
+		for _, p := range children {
+			if p == v {
+				selfRef = true
+				break
+			}
+		}
+		if selfRef {
+			continue
+		}
+
+		matches = append(matches, children...)
+	}
+
+	return matches
+}
+
 func (m *ReferenceMap) prefix(v dag.Vertex) string {
 	// If the node is stating it is already fully qualified then
 	// we don't have to create the prefix!
@@ -146,7 +182,25 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap {
 		}
 	}
 
-	m.m = refMap
+	// Build the lookup table for referenced by
+	refByMap := make(map[string][]dag.Vertex)
+	for _, v := range vs {
+		// We're only looking for referenceable nodes
+		rn, ok := v.(GraphNodeReferencer)
+		if !ok {
+			continue
+		}
+
+		// Go through and cache them
+		prefix := m.prefix(v)
+		for _, n := range rn.References() {
+			n = prefix + n
+			refByMap[n] = append(refByMap[n], v)
+		}
+	}
+
+	m.references = refMap
+	m.referencedBy = refByMap
 	return &m
 }
 

terraform/transform_reference_test.go

@@ -112,11 +112,56 @@ func TestReferenceMapReferences(t *testing.T) {
 	for tn, tc := range cases {
 		t.Run(tn, func(t *testing.T) {
 			rm := NewReferenceMap(tc.Nodes)
-			result, err := rm.References(tc.Check)
+			result, _ := rm.References(tc.Check)
-			if err != nil {
+
-				t.Fatalf("err: %s", err)
+			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.ReferencedBy(tc.Check)
+
 			var resultStr []string
 			for _, v := range result {
 				resultStr = append(resultStr, dag.VertexName(v))