From def1f9b08427f9ed68d379e8d00e8a60373e3922 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 23 Sep 2020 10:05:32 -0400
Subject: [PATCH] we no longer need EvalRefreshDependencies

This evaluation was required when refresh ran in a separate walk and
managed resources were only partly handled by configuration. Now that we
have the correct dependency information available when refreshing
configured resources, we can update their state accordingly. Since
orphaned resources are not refreshed, they can retain their stored
dependencies for correct ordering.

This also prevents users from introducing cycles with nodes they can't
"see", since only orphaned nodes will retain their stored dependencies,
and the remaining nodes will be updated according to the configuration.
---
 terraform/context_refresh_test.go        | 40 +++++++++++++++++---
 terraform/eval_state.go                  | 47 ------------------------
 terraform/node_resource_plan_instance.go |  4 --
 3 files changed, 35 insertions(+), 56 deletions(-)

diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go
index 4acf6ba69..b7f3b6927 100644
--- a/terraform/context_refresh_test.go
+++ b/terraform/context_refresh_test.go
@@ -1582,7 +1582,7 @@ func TestRefresh_updateDependencies(t *testing.T) {
 			Status:    states.ObjectReady,
 			AttrsJSON: []byte(`{"id":"foo"}`),
 			Dependencies: []addrs.ConfigResource{
-				// Existing dependencies should not be removed during refresh
+				// Existing dependencies should be removed when overridden by the config
 				{
 					Module: addrs.RootModule,
 					Resource: addrs.Resource{
@@ -1598,6 +1598,32 @@ func TestRefresh_updateDependencies(t *testing.T) {
 			Module:   addrs.RootModule,
 		},
 	)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "aws_instance",
+			Name: "baz",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"baz"}`),
+			Dependencies: []addrs.ConfigResource{
+				// Existing dependencies should not be removed from orphaned instances
+				{
+					Module: addrs.RootModule,
+					Resource: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "bam",
+					},
+				},
+			},
+		},
+		addrs.AbsProviderConfig{
+			Provider: addrs.NewDefaultProvider("aws"),
+			Module:   addrs.RootModule,
+		},
+	)
 	root.SetResourceInstanceCurrent(
 		addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
@@ -1621,7 +1647,8 @@ resource "aws_instance" "bar" {
 }
 
 resource "aws_instance" "foo" {
-}`,
+}
+`,
 	})
 
 	p := testProvider("aws")
@@ -1649,12 +1676,15 @@ aws_instance.bar:
 
   Dependencies:
     aws_instance.foo
-aws_instance.foo:
-  ID = foo
+aws_instance.baz:
+  ID = baz
   provider = provider["registry.terraform.io/hashicorp/aws"]
 
   Dependencies:
-    aws_instance.baz
+    aws_instance.bam
+aws_instance.foo:
+  ID = foo
+  provider = provider["registry.terraform.io/hashicorp/aws"]
 `)
 
 	checkStateString(t, result, expect)
diff --git a/terraform/eval_state.go b/terraform/eval_state.go
index 50f363b2d..02f36d42c 100644
--- a/terraform/eval_state.go
+++ b/terraform/eval_state.go
@@ -3,7 +3,6 @@ package terraform
 import (
 	"fmt"
 	"log"
-	"sort"
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
@@ -530,49 +529,3 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
 
 	return nil, nil
 }
-
-// EvalRefreshDependencies is an EvalNode implementation that appends any newly
-// found dependencies to those saved in the state. The existing dependencies
-// are retained, as they may be missing from the config, and will be required
-// for the updates and destroys during the next apply.
-type EvalRefreshDependencies struct {
-	// Prior State
-	State **states.ResourceInstanceObject
-	// Dependencies to write to the new state
-	Dependencies *[]addrs.ConfigResource
-}
-
-func (n *EvalRefreshDependencies) Eval(ctx EvalContext) (interface{}, error) {
-	state := *n.State
-	if state == nil {
-		// no existing state to append
-		return nil, nil
-	}
-
-	// We already have dependencies in state, so we need to trust those for
-	// refresh. We can't write out new dependencies until apply time in case
-	// the configuration has been changed in a manner the conflicts with the
-	// stored dependencies.
-	if len(state.Dependencies) > 0 {
-		*n.Dependencies = state.Dependencies
-		return nil, nil
-	}
-
-	depMap := make(map[string]addrs.ConfigResource)
-	for _, d := range *n.Dependencies {
-		depMap[d.String()] = d
-	}
-
-	deps := make([]addrs.ConfigResource, 0, len(depMap))
-	for _, d := range depMap {
-		deps = append(deps, d)
-	}
-
-	sort.Slice(deps, func(i, j int) bool {
-		return deps[i].String() < deps[j].String()
-	})
-
-	*n.Dependencies = deps
-
-	return nil, nil
-}
diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go
index 58e1545fc..6176eb3c3 100644
--- a/terraform/node_resource_plan_instance.go
+++ b/terraform/node_resource_plan_instance.go
@@ -141,10 +141,6 @@ func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
 				ProviderSchema: &providerSchema,
 				Output:         &instanceRefreshState,
 			},
-			&EvalRefreshDependencies{
-				State:        &instanceRefreshState,
-				Dependencies: &n.Dependencies,
-			},
 			&EvalRefresh{
 				Addr:           addr.Resource,
 				ProviderAddr:   n.ResolvedProvider,