Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expander.ExpandResource cannot expand all modules #24454

Merged
merged 1 commit into from
Mar 25, 2020
Merged

Expander.ExpandResource cannot expand all modules #24454

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 37 additions & 14 deletions instances/expander.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (e *Expander) ExpandModuleResource(moduleAddr addrs.Module, resourceAddr ad
// (moduleInstances does plenty of allocations itself, so the benefit of
// pre-allocating this is marginal but it's not hard to do.)
moduleInstanceAddr := make(addrs.ModuleInstance, 0, 4)
ret := e.exps.resourceInstances(moduleAddr, resourceAddr, moduleInstanceAddr)
ret := e.exps.moduleResourceInstances(moduleAddr, resourceAddr, moduleInstanceAddr)
sort.SliceStable(ret, func(i, j int) bool {
return ret[i].Less(ret[j])
})
Expand All @@ -150,16 +150,14 @@ func (e *Expander) ExpandModuleResource(moduleAddr addrs.Module, resourceAddr ad
// itself must already have had their expansion registered using one of the
// SetModule*/SetResource* methods before calling, or this method will panic.
func (e *Expander) ExpandResource(resourceAddr addrs.AbsResource) []addrs.AbsResourceInstance {
var ret []addrs.AbsResourceInstance
e.mu.RLock()
defer e.mu.RUnlock()

// Take the full set of expanded resource instances and filter for only
// those within our module instance.
for _, r := range e.ExpandModuleResource(resourceAddr.Module.Module(), resourceAddr.Resource) {
if !r.Module.Equal(resourceAddr.Module) {
continue
}
ret = append(ret, r)
}
moduleInstanceAddr := make(addrs.ModuleInstance, 0, 4)
ret := e.exps.resourceInstances(resourceAddr.Module, resourceAddr.Resource, moduleInstanceAddr)
sort.SliceStable(ret, func(i, j int) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these getting sorted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for consistency, so that things like output and test fixtures are stable.
It also matched the behavior of the original method above, now called ExpandModuleResource.

return ret[i].Less(ret[j])
})
return ret
}

Expand Down Expand Up @@ -297,10 +295,9 @@ func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.Mod
return ret
}

func (m *expanderModule) resourceInstances(moduleAddr addrs.Module, resourceAddr addrs.Resource, parentAddr addrs.ModuleInstance) []addrs.AbsResourceInstance {
var ret []addrs.AbsResourceInstance

func (m *expanderModule) moduleResourceInstances(moduleAddr addrs.Module, resourceAddr addrs.Resource, parentAddr addrs.ModuleInstance) []addrs.AbsResourceInstance {
if len(moduleAddr) > 0 {
var ret []addrs.AbsResourceInstance
// We need to traverse through the module levels first, so we can
// then iterate resource expansions in the context of each module
// path leading to them.
Expand All @@ -317,11 +314,37 @@ func (m *expanderModule) resourceInstances(moduleAddr addrs.Module, resourceAddr
continue
}
moduleInstAddr := append(parentAddr, step)
ret = append(ret, inst.resourceInstances(moduleAddr[1:], resourceAddr, moduleInstAddr)...)
ret = append(ret, inst.moduleResourceInstances(moduleAddr[1:], resourceAddr, moduleInstAddr)...)
}
return ret
}

return m.onlyResourceInstances(resourceAddr, parentAddr)
}

func (m *expanderModule) resourceInstances(moduleAddr addrs.ModuleInstance, resourceAddr addrs.Resource, parentAddr addrs.ModuleInstance) []addrs.AbsResourceInstance {
if len(moduleAddr) > 0 {
// We need to traverse through the module levels first, using only the
// module instances for our specific resource, as the resource may not
// yet be expanded in all module instances.
step := moduleAddr[0]
callName := step.Name
if _, ok := m.moduleCalls[addrs.ModuleCall{Name: callName}]; !ok {
// 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.
panic(fmt.Sprintf("no expansion has been registered for %s", parentAddr.Child(callName, addrs.NoKey)))
}

inst := m.childInstances[step]
moduleInstAddr := append(parentAddr, step)
return inst.resourceInstances(moduleAddr[1:], resourceAddr, moduleInstAddr)
}
return m.onlyResourceInstances(resourceAddr, parentAddr)
}

func (m *expanderModule) onlyResourceInstances(resourceAddr addrs.Resource, parentAddr addrs.ModuleInstance) []addrs.AbsResourceInstance {
var ret []addrs.AbsResourceInstance
exp, ok := m.resources[resourceAddr]
if !ok {
panic(fmt.Sprintf("no expansion has been registered for %s", resourceAddr.Absolute(parentAddr)))
Expand Down