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.
This commit is contained in:
James Bardin 2021-12-16 18:21:06 -05:00
parent a5017bff2f
commit b213386a73
4 changed files with 21 additions and 6 deletions

View File

@ -99,6 +99,13 @@ func (e *Expander) SetResourceForEach(moduleAddr addrs.ModuleInstance, resourceA
// had their expansion registered using one of the SetModule* methods before // had their expansion registered using one of the SetModule* methods before
// calling, or this method will panic. // calling, or this method will panic.
func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance { 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 { if len(addr) == 0 {
// Root module is always a singleton. // Root module is always a singleton.
return singletonRootModule 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 // (moduleInstances does plenty of allocations itself, so the benefit of
// pre-allocating this is marginal but it's not hard to do.) // pre-allocating this is marginal but it's not hard to do.)
parentAddr := make(addrs.ModuleInstance, 0, 4) 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 { sort.SliceStable(ret, func(i, j int) bool {
return ret[i].Less(ret[j]) return ret[i].Less(ret[j])
}) })
@ -344,10 +351,16 @@ func newExpanderModule() *expanderModule {
var singletonRootModule = []addrs.ModuleInstance{addrs.RootModuleInstance} 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] callName := addr[0]
exp, ok := m.moduleCalls[addrs.ModuleCall{Name: callName}] exp, ok := m.moduleCalls[addrs.ModuleCall{Name: callName}]
if !ok { if !ok {
if skipUnknown {
return nil
}
// This is a bug in the caller, because it should always register // This is a bug in the caller, because it should always register
// expansions for an object and all of its ancestors before requesting // expansions for an object and all of its ancestors before requesting
// expansion of it. // expansion of it.
@ -363,7 +376,7 @@ func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.Mod
continue continue
} }
instAddr := append(parentAddr, step) instAddr := append(parentAddr, step)
ret = append(ret, inst.moduleInstances(addr[1:], instAddr)...) ret = append(ret, inst.moduleInstances(addr[1:], instAddr, skipUnknown)...)
} }
return ret return ret
} }

View File

@ -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 // then the result is the full expansion of all combinations of all of their
// declared instance keys. // declared instance keys.
func (s Set) InstancesForModule(modAddr addrs.Module) []addrs.ModuleInstance { func (s Set) InstancesForModule(modAddr addrs.Module) []addrs.ModuleInstance {
return s.exp.ExpandModule(modAddr) return s.exp.expandModule(modAddr, true)
} }

View File

@ -204,4 +204,8 @@ func TestSet(t *testing.T) {
t.Errorf("unexpected %T %s", input, input.String()) 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")
}
} }

View File

@ -631,8 +631,6 @@ func TestContext2Apply_nullableVariables(t *testing.T) {
} }
func TestContext2Apply_targetedDestroyWithMoved(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{ m := testModuleInline(t, map[string]string{
"main.tf": ` "main.tf": `
module "modb" { module "modb" {