Merge pull request #22454 from pselle/nokeymoregraph

Add edge when calculating for_each orphans

states/sync.go

@@ -283,7 +283,7 @@ func (s *SyncState) MaybeFixUpResourceInstanceAddressForCount(addr addrs.AbsReso
 }
 
 // SetResourceInstanceCurrent saves the given instance object as the current
-// generation of the resource instance with the given address, simulataneously
+// generation of the resource instance with the given address, simultaneously
 // updating the recorded provider configuration address, dependencies, and
 // resource EachMode.
 //

terraform/transform_orphan_count.go

@@ -50,7 +50,29 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error {
 }
 
 func (t *OrphanResourceCountTransformer) transformForEach(haveKeys map[addrs.InstanceKey]struct{}, g *Graph) error {
+	// If there is a NoKey node, add this to the graph first,
+	// so that we can create edges to it in subsequent (StringKey) nodes.
+	// This is because the last item determines the resource mode for the whole resource,
+	// (see SetResourceInstanceCurrent for more information) and we need to evaluate
+	// an orphaned (NoKey) resource before the in-memory state is updated
+	// to deal with a new for_each resource
+	_, hasNoKeyNode := haveKeys[addrs.NoKey]
+	var noKeyNode dag.Vertex
+	if hasNoKeyNode {
+		abstract := NewNodeAbstractResourceInstance(t.Addr.Instance(addrs.NoKey))
+		noKeyNode = abstract
+		if f := t.Concrete; f != nil {
+			noKeyNode = f(abstract)
+		}
+		g.Add(noKeyNode)
+	}
+
 	for key := range haveKeys {
+		// If the key is no-key, we have already added it, so skip
+		if key == addrs.NoKey {
+			continue
+		}
+
 		s, _ := key.(addrs.StringKey)
 		// If the key is present in our current for_each, carry on
 		if _, ok := t.ForEach[string(s)]; ok {
@@ -64,6 +86,11 @@ func (t *OrphanResourceCountTransformer) transformForEach(haveKeys map[addrs.Ins
 		}
 		log.Printf("[TRACE] OrphanResourceCount(non-zero): adding %s as %T", t.Addr, node)
 		g.Add(node)
+
+		// Add edge to noKeyNode if it exists
+		if hasNoKeyNode {
+			g.Connect(dag.BasicEdge(node, noKeyNode))
+		}
 	}
 	return nil
 }

terraform/transform_orphan_count_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/states"
+	"github.com/zclconf/go-cty/cty"
 )
 
 func TestOrphanResourceCountTransformer(t *testing.T) {
@@ -331,6 +333,74 @@ func TestOrphanResourceCountTransformer_zeroAndNoneCount(t *testing.T) {
 	}
 }
 
+// When converting from a NoEach mode to an EachMap via a switch to for_each,
+// an edge is necessary to ensure that the map-key'd instances
+// are evaluated after the NoKey resource, because the final instance evaluated
+// sets the whole resource's EachMode.
+func TestOrphanResourceCountTransformer_ForEachEdgesAdded(t *testing.T) {
+	state := states.BuildState(func(s *states.SyncState) {
+		// "bar" key'd resource
+		s.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "aws_instance",
+				Name: "foo",
+			}.Instance(addrs.StringKey("bar")).Absolute(addrs.RootModuleInstance),
+			&states.ResourceInstanceObjectSrc{
+				AttrsFlat: map[string]string{
+					"id": "foo",
+				},
+				Status: states.ObjectReady,
+			},
+			addrs.ProviderConfig{
+				Type: "aws",
+			}.Absolute(addrs.RootModuleInstance),
+		)
+
+		// NoKey'd resource
+		s.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "aws_instance",
+				Name: "foo",
+			}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+			&states.ResourceInstanceObjectSrc{
+				AttrsFlat: map[string]string{
+					"id": "foo",
+				},
+				Status: states.ObjectReady,
+			},
+			addrs.ProviderConfig{
+				Type: "aws",
+			}.Absolute(addrs.RootModuleInstance),
+		)
+	})
+
+	g := Graph{Path: addrs.RootModuleInstance}
+
+	{
+		tf := &OrphanResourceCountTransformer{
+			Concrete: testOrphanResourceConcreteFunc,
+			// No keys in this ForEach ensure both our resources end
+			// up orphaned in this test
+			ForEach: map[string]cty.Value{},
+			Addr: addrs.RootModuleInstance.Resource(
+				addrs.ManagedResourceMode, "aws_instance", "foo",
+			),
+			State: state,
+		}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformOrphanResourceForEachStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
 const testTransformOrphanResourceCountBasicStr = `
 aws_instance.foo[2] (orphan)
 `
@@ -355,3 +425,9 @@ aws_instance.foo[0] (orphan)
 const testTransformOrphanResourceCountZeroAndNoneCountStr = `
 aws_instance.foo (orphan)
 `
+
+const testTransformOrphanResourceForEachStr = `
+aws_instance.foo (orphan)
+aws_instance.foo["bar"] (orphan)
+  aws_instance.foo (orphan)
+`