From b213386a739c80a36e411c7cf6160f78fbfb08fc Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 16 Dec 2021 18:21:06 -0500
Subject: [PATCH] InstancesForModule should not panic

instances.Set is only used after all instances have been processes, so
it should therefor only handle known instances and not panic when given
an address that traverses an unexpanded module.
---
 internal/instances/expander.go            | 19 ++++++++++++++++---
 internal/instances/set.go                 |  2 +-
 internal/instances/set_test.go            |  4 ++++
 internal/terraform/context_apply2_test.go |  2 --
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/internal/instances/expander.go b/internal/instances/expander.go
index 4a0bd936f..2c912c897 100644
--- a/internal/instances/expander.go
+++ b/internal/instances/expander.go
@@ -99,6 +99,13 @@ func (e *Expander) SetResourceForEach(moduleAddr addrs.ModuleInstance, resourceA
 // had their expansion registered using one of the SetModule* methods before
 // calling, or this method will panic.
 func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance {
+	return e.expandModule(addr, false)
+}
+
+// expandModule allows skipping unexpanded module addresses by setting skipUnknown to true.
+// This is used by instances.Set, which is only concerned with the expanded
+// instances, and should not panic when looking up unknown addresses.
+func (e *Expander) expandModule(addr addrs.Module, skipUnknown bool) []addrs.ModuleInstance {
 	if len(addr) == 0 {
 		// Root module is always a singleton.
 		return singletonRootModule
@@ -113,7 +120,7 @@ func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance {
 	// (moduleInstances does plenty of allocations itself, so the benefit of
 	// pre-allocating this is marginal but it's not hard to do.)
 	parentAddr := make(addrs.ModuleInstance, 0, 4)
-	ret := e.exps.moduleInstances(addr, parentAddr)
+	ret := e.exps.moduleInstances(addr, parentAddr, skipUnknown)
 	sort.SliceStable(ret, func(i, j int) bool {
 		return ret[i].Less(ret[j])
 	})
@@ -344,10 +351,16 @@ func newExpanderModule() *expanderModule {
 
 var singletonRootModule = []addrs.ModuleInstance{addrs.RootModuleInstance}
 
-func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.ModuleInstance) []addrs.ModuleInstance {
+// if moduleInstances is being used to lookup known instances after all
+// expansions have been done, set skipUnknown to true which allows addrs which
+// may not have been seen to return with no instances rather than panicking.
+func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.ModuleInstance, skipUnknown bool) []addrs.ModuleInstance {
 	callName := addr[0]
 	exp, ok := m.moduleCalls[addrs.ModuleCall{Name: callName}]
 	if !ok {
+		if skipUnknown {
+			return nil
+		}
 		// This is a bug in the caller, because it should always register
 		// expansions for an object and all of its ancestors before requesting
 		// expansion of it.
@@ -363,7 +376,7 @@ func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.Mod
 				continue
 			}
 			instAddr := append(parentAddr, step)
-			ret = append(ret, inst.moduleInstances(addr[1:], instAddr)...)
+			ret = append(ret, inst.moduleInstances(addr[1:], instAddr, skipUnknown)...)
 		}
 		return ret
 	}
diff --git a/internal/instances/set.go b/internal/instances/set.go
index 571036819..701a2d27e 100644
--- a/internal/instances/set.go
+++ b/internal/instances/set.go
@@ -47,5 +47,5 @@ func (s Set) HasResource(want addrs.AbsResource) bool {
 // then the result is the full expansion of all combinations of all of their
 // declared instance keys.
 func (s Set) InstancesForModule(modAddr addrs.Module) []addrs.ModuleInstance {
-	return s.exp.ExpandModule(modAddr)
+	return s.exp.expandModule(modAddr, true)
 }
diff --git a/internal/instances/set_test.go b/internal/instances/set_test.go
index cf143139a..e255cef1b 100644
--- a/internal/instances/set_test.go
+++ b/internal/instances/set_test.go
@@ -204,4 +204,8 @@ func TestSet(t *testing.T) {
 		t.Errorf("unexpected %T %s", input, input.String())
 	}
 
+	// ensure we can lookup non-existent addrs in a set without panic
+	if set.InstancesForModule(addrs.RootModule.Child("missing")) != nil {
+		t.Error("unexpected instances from missing module")
+	}
 }
diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go
index 36ea1659b..e02c73df1 100644
--- a/internal/terraform/context_apply2_test.go
+++ b/internal/terraform/context_apply2_test.go
@@ -631,8 +631,6 @@ func TestContext2Apply_nullableVariables(t *testing.T) {
 }
 
 func TestContext2Apply_targetedDestroyWithMoved(t *testing.T) {
-	// The impure function call should not cause a planned change with
-	// ignore_changes
 	m := testModuleInline(t, map[string]string{
 		"main.tf": `
 module "modb" {