Merge pull request #35873 from hashicorp/jbardin/ancestors

core: use more efficient methods for finding ancestors

Author: jbardin

internal/dag/dag.go

@@ -51,6 +51,56 @@ func (g *AcyclicGraph) Ancestors(vs ...Vertex) Set {
 	return s
 }
 
+// FirstAncestorsWith returns a Set that includes every Vertex yielded by
+// walking down from the provided starting Vertex v, and stopping each branch when
+// match returns true. This will return the set of all first ancestors
+// encountered which match some criteria.
+func (g *AcyclicGraph) FirstAncestorsWith(v Vertex, match func(Vertex) bool) Set {
+	s := make(Set)
+	searchFunc := func(v Vertex, d int) error {
+		if match(v) {
+			s.Add(v)
+			return errStopWalkBranch
+		}
+
+		return nil
+	}
+
+	start := make(Set)
+	for _, dep := range g.downEdgesNoCopy(v) {
+		start.Add(dep)
+	}
+
+	// our memoFunc doesn't return an error
+	g.DepthFirstWalk(start, searchFunc)
+
+	return s
+}
+
+// MatchAncestor returns true if the given match function returns true for any
+// descendants of the given Vertex.
+func (g *AcyclicGraph) MatchAncestor(v Vertex, match func(Vertex) bool) bool {
+	var ret bool
+	matchFunc := func(v Vertex, d int) error {
+		if match(v) {
+			ret = true
+			return errStopWalk
+		}
+
+		return nil
+	}
+
+	start := make(Set)
+	for _, dep := range g.downEdgesNoCopy(v) {
+		start.Add(dep)
+	}
+
+	// our memoFunc doesn't return an error
+	g.DepthFirstWalk(start, matchFunc)
+
+	return ret
+}
+
 // Descendants returns a Set that includes every Vertex yielded by walking up
 // from the provided starting Vertex v.
 func (g *AcyclicGraph) Descendants(v Vertex) Set {

internal/dag/dag_test.go

@@ -317,7 +317,7 @@ func TestAcyclicGraphFindDescendants(t *testing.T) {
 		return v.(int) == 1
 	})
 	if !foundOne {
-		t.Fatal("did not match 6 in the graph")
+		t.Fatal("did not match 1 in the graph")
 	}
 
 	foundSix := g.MatchDescendant(6, func(v Vertex) bool {
@@ -335,6 +335,57 @@ func TestAcyclicGraphFindDescendants(t *testing.T) {
 	}
 }
 
+func TestAcyclicGraphFindAncestors(t *testing.T) {
+	var g AcyclicGraph
+	g.Add(0)
+	g.Add(1)
+	g.Add(2)
+	g.Add(3)
+	g.Add(4)
+	g.Add(5)
+	g.Add(6)
+	g.Connect(BasicEdge(1, 0))
+	g.Connect(BasicEdge(2, 1))
+	g.Connect(BasicEdge(6, 2))
+	g.Connect(BasicEdge(4, 3))
+	g.Connect(BasicEdge(5, 4))
+	g.Connect(BasicEdge(6, 5))
+
+	actual := g.FirstAncestorsWith(6, func(v Vertex) bool {
+		// looking for first odd ancestors
+		return v.(int)%2 != 0
+	})
+
+	expected := make(Set)
+	expected.Add(1)
+	expected.Add(5)
+
+	if expected.Intersection(actual).Len() != expected.Len() {
+		t.Fatalf("expected %#v, got %#v\n", expected, actual)
+	}
+
+	foundOne := g.MatchAncestor(6, func(v Vertex) bool {
+		return v.(int) == 1
+	})
+	if !foundOne {
+		t.Fatal("did not match 1 in the graph")
+	}
+
+	foundSix := g.MatchAncestor(6, func(v Vertex) bool {
+		return v.(int) == 6
+	})
+	if foundSix {
+		t.Fatal("6 should not be a descendant of itself")
+	}
+
+	foundTen := g.MatchAncestor(6, func(v Vertex) bool {
+		return v.(int) == 10
+	})
+	if foundTen {
+		t.Fatal("10 is not in the graph at all")
+	}
+}
+
 func TestAcyclicGraphWalk(t *testing.T) {
 	var g AcyclicGraph
 	g.Add(1)

internal/terraform/context_apply2_test.go

@@ -3582,3 +3582,53 @@ resource "test_instance" "obj" {
 		t.Fatal("old instance not destroyed")
 	}
 }
+
+// each resource only needs to record the first dependency in a chain
+func TestContext2Apply_limitDependencies(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_object" "a" {
+}
+
+resource "test_object" "b" {
+  test_string = test_object.a.test_string
+}
+
+resource "test_object" "c" {
+  test_string = test_object.b.test_string
+}
+`,
+	})
+
+	p := simpleMockProvider()
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
+	assertNoErrors(t, diags)
+
+	state, diags := ctx.Apply(plan, m, nil)
+	assertNoErrors(t, diags)
+
+	for _, res := range state.RootModule().Resources {
+		deps := res.Instances[addrs.NoKey].Current.Dependencies
+		switch res.Addr.Resource.Name {
+		case "a":
+			if len(deps) > 0 {
+				t.Error(res.Addr, "should have no dependencies")
+			}
+		case "b":
+			if len(deps) != 1 || deps[0].Resource.Name != "a" {
+				t.Error(res.Addr, "should only depend on 'a', got", deps)
+			}
+		case "c":
+			if len(deps) != 1 || deps[0].Resource.Name != "b" {
+				t.Error(res.Addr, "should only record a dependency of 'b', got", deps)
+			}
+		}
+	}
+}

internal/terraform/transform_reference.go

@@ -257,25 +257,30 @@ func (t AttachDependenciesTransformer) Transform(g *Graph) error {
 		// dedupe addrs when there's multiple instances involved, or
 		// multiple paths in the un-reduced graph
 		depMap := map[string]addrs.ConfigResource{}
-		for _, d := range g.Ancestors(v) {
-			var addr addrs.ConfigResource
 
+		// since we need to type-switch over the nodes anyway, we're going to
+		// insert the address directly into depMap and forget about the returned
+		// set.
+		g.FirstAncestorsWith(v, func(d dag.Vertex) bool {
+			var addr addrs.ConfigResource
 			switch d := d.(type) {
-			case GraphNodeResourceInstance:
-				instAddr := d.ResourceInstanceAddr()
+			case GraphNodeCreator:
+				// most of the time we'll hit a GraphNodeConfigResource first since that represents the config structure, but
+				instAddr := d.CreateAddr()
 				addr = instAddr.ContainingResource().Config()
 			case GraphNodeConfigResource:
 				addr = d.ResourceAddr()
 			default:
-				continue
+				return false
 			}
 
 			if matchesSelf(addr) {
-				continue
+				return false
 			}
 
 			depMap[addr.String()] = addr
-		}
+			return true
+		})
 
 		deps := make([]addrs.ConfigResource, 0, len(depMap))
 		for _, d := range depMap {
@@ -381,10 +386,10 @@ func (m ReferenceMap) dependsOn(g *Graph, depender graphNodeDependsOn) ([]dag.Ve
 			// upstream managed resource in order to check for a planned
 			// change.
 			if _, ok := rv.(GraphNodeConfigResource); !ok {
-				for _, v := range g.Ancestors(rv) {
-					if isDependableResource(v) {
-						res = append(res, v)
-					}
+				for _, v := range g.FirstAncestorsWith(rv, func(v dag.Vertex) bool {
+					return isDependableResource(v)
+				}) {
+					res = append(res, v)
 				}
 			}
 		}
@@ -454,8 +459,10 @@ func (m ReferenceMap) parentModuleDependsOn(g *Graph, depender graphNodeDependsO
 			}
 		}
 
-		for _, v := range g.Ancestors(deps...) {
-			if isDependableResource(v) {
+		for _, dep := range deps {
+			for _, v := range g.FirstAncestorsWith(dep, func(v dag.Vertex) bool {
+				return isDependableResource(v)
+			}) {
 				res = append(res, v)
 			}
 		}