From b52bb7dd4fcfa36065fab0fe0ed1b7ca33d75a68 Mon Sep 17 00:00:00 2001
From: Anton Tayanovskyy <anton@pulumi.com>
Date: Wed, 10 Nov 2021 12:47:03 -0500
Subject: [PATCH] DependingOn transitivity

---
 pkg/resource/graph/dependency_graph.go      | 26 +++++++-------
 pkg/resource/graph/dependency_graph_test.go | 38 +++++++++++++++++++++
 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/pkg/resource/graph/dependency_graph.go b/pkg/resource/graph/dependency_graph.go
index 3fbd9908c..117dc5834 100644
--- a/pkg/resource/graph/dependency_graph.go
+++ b/pkg/resource/graph/dependency_graph.go
@@ -32,21 +32,23 @@ func (dg *DependencyGraph) DependingOn(res *resource.State,
 	dependentSet[res.URN] = true
 
 	isDependent := func(candidate *resource.State) bool {
-		if ignore[candidate.URN] {
-			return false
-		}
-		if includeChildren && candidate.Parent == res.URN {
-			return true
-		}
-		for _, dependency := range candidate.Dependencies {
-			if dependentSet[dependency] {
-				return true
-			}
-		}
+		// Direct deps include explicit `Dependencies`,
+		// provider, and parent (under `includeChildren=true`
+		// semantic).
+		directDeps := candidate.Dependencies
 		if candidate.Provider != "" {
 			ref, err := providers.ParseReference(candidate.Provider)
 			contract.Assert(err == nil)
-			if dependentSet[ref.URN()] {
+			directDeps = append(directDeps, ref.URN())
+		}
+		if includeChildren && candidate.Parent != "" {
+			directDeps = append(directDeps, candidate.Parent)
+		}
+
+		// We are computing a transitive closure of direct
+		// deps; therefore check in `dependentSet`.
+		for _, dependency := range directDeps {
+			if dependentSet[dependency] {
 				return true
 			}
 		}
diff --git a/pkg/resource/graph/dependency_graph_test.go b/pkg/resource/graph/dependency_graph_test.go
index 73d594c72..247f7567d 100644
--- a/pkg/resource/graph/dependency_graph_test.go
+++ b/pkg/resource/graph/dependency_graph_test.go
@@ -229,3 +229,41 @@ func TestDependenciesOfRemoteComponentsNoCycle(t *testing.T) {
 	assert.True(t, rDependencies[parent])
 	assert.False(t, rDependencies[child])
 }
+
+func NewCustomResource(name string, provider *resource.State, deps ...resource.URN) *resource.State {
+	r := NewResource(name, provider, deps...)
+	r.Custom = true
+	return r
+}
+
+func TestDependingOnIndirect(t *testing.T) {
+	var noProvider *resource.State
+
+	d2 := NewCustomResource("d2", noProvider)
+	d3 := NewCustomResource("d3", noProvider, d2.URN)
+	c3 := NewCustomResource("c3", noProvider)
+	c3.Parent = d3.URN
+
+	dg := NewDependencyGraph([]*resource.State{
+		d2, d3, c3,
+	})
+
+	for r := range dg.DependenciesOf(c3) {
+		t.Logf("dg.DependenciesOf(c3) includes %v", r.URN)
+	}
+
+	for r := range dg.DependenciesOf(d3) {
+		t.Logf("dg.DependenciesOf(d3) includes %v", r.URN)
+	}
+
+	var foundC3 bool
+	for _, r := range dg.DependingOn(d2, nil, true) {
+		if r == c3 {
+			foundC3 = true
+		}
+		t.Logf("DependingOn(d2) includes %v", r.URN)
+	}
+	if !foundC3 {
+		t.Errorf("DependingOn(d2, nil) should have included c3")
+	}
+}