Fix quadratic marshalPlannedValues (#2324)

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
Signed-off-by: Jon Johnson <jonjohnsonjr@gmail.com>
Co-authored-by: Oleksandr Levchenkov <ollevche@gmail.com>
This commit is contained in:
Jon Johnson 2025-01-06 13:20:41 -08:00 committed by GitHub
parent 45131c4c0c
commit 7ba6e61c69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 67 additions and 41 deletions

View File

@ -98,63 +98,76 @@ func marshalPlannedOutputs(changes *plans.Changes) (map[string]Output, error) {
func marshalPlannedValues(changes *plans.Changes, schemas *tofu.Schemas) (Module, error) {
var ret Module
// build two maps:
// build three maps:
// module name -> [resource addresses]
// module -> [children modules]
// resource addr -> change src
moduleResourceMap := make(map[string][]addrs.AbsResourceInstance)
moduleMap := make(map[string][]addrs.ModuleInstance)
changeMap := make(map[string]*plans.ResourceInstanceChangeSrc)
seenModules := make(map[string]bool)
for _, resource := range changes.Resources {
// If the resource is being deleted, skip over it.
// Deposed instances are always conceptually a destroy, but if they
// were gone during refresh then the change becomes a noop.
if resource.Action != plans.Delete && resource.DeposedKey == states.NotDeposed {
containingModule := resource.Addr.Module.String()
moduleResourceMap[containingModule] = append(moduleResourceMap[containingModule], resource.Addr)
if resource.DeposedKey != states.NotDeposed {
continue
}
// the root module has no parents
if !resource.Addr.Module.IsRoot() {
parent := resource.Addr.Module.Parent().String()
// we expect to see multiple resources in one module, so we
// only need to report the "parent" module for each child module
// once.
if !seenModules[containingModule] {
moduleMap[parent] = append(moduleMap[parent], resource.Addr.Module)
seenModules[containingModule] = true
}
changeMap[resource.Addr.String()] = resource
// If any given parent module has no resources, it needs to be
// added to the moduleMap. This walks through the current
// resources' modules' ancestors, taking advantage of the fact
// that Ancestors() returns an ordered slice, and verifies that
// each one is in the map.
ancestors := resource.Addr.Module.Ancestors()
for i, ancestor := range ancestors[:len(ancestors)-1] {
aStr := ancestor.String()
// If the resource is being deleted, skip over it.
if resource.Action == plans.Delete {
continue
}
// childStr here is the immediate child of the current step
childStr := ancestors[i+1].String()
// we likely will see multiple resources in one module, so we
// only need to report the "parent" module for each child module
// once.
if !seenModules[childStr] {
moduleMap[aStr] = append(moduleMap[aStr], ancestors[i+1])
seenModules[childStr] = true
}
}
containingModule := resource.Addr.Module.String()
moduleResourceMap[containingModule] = append(moduleResourceMap[containingModule], resource.Addr)
// the root module has no parents
if resource.Addr.Module.IsRoot() {
continue
}
parent := resource.Addr.Module.Parent().String()
// we expect to see multiple resources in one module, so we
// only need to report the "parent" module for each child module
// once.
if !seenModules[containingModule] {
moduleMap[parent] = append(moduleMap[parent], resource.Addr.Module)
seenModules[containingModule] = true
}
// If any given parent module has no resources, it needs to be
// added to the moduleMap. This walks through the current
// resources' modules' ancestors, taking advantage of the fact
// that Ancestors() returns an ordered slice, and verifies that
// each one is in the map.
ancestors := resource.Addr.Module.Ancestors()
for i, ancestor := range ancestors[:len(ancestors)-1] {
aStr := ancestor.String()
// childStr here is the immediate child of the current step
childStr := ancestors[i+1].String()
// we likely will see multiple resources in one module, so we
// only need to report the "parent" module for each child module
// once.
if !seenModules[childStr] {
moduleMap[aStr] = append(moduleMap[aStr], ancestors[i+1])
seenModules[childStr] = true
}
}
}
// start with the root module
resources, err := marshalPlanResources(changes, moduleResourceMap[""], schemas)
resources, err := marshalPlanResources(changeMap, moduleResourceMap[""], schemas)
if err != nil {
return ret, err
}
ret.Resources = resources
childModules, err := marshalPlanModules(changes, schemas, moduleMap[""], moduleMap, moduleResourceMap)
childModules, err := marshalPlanModules(changeMap, schemas, moduleMap[""], moduleMap, moduleResourceMap)
if err != nil {
return ret, err
}
@ -168,11 +181,13 @@ func marshalPlannedValues(changes *plans.Changes, schemas *tofu.Schemas) (Module
}
// marshalPlanResources
func marshalPlanResources(changes *plans.Changes, ris []addrs.AbsResourceInstance, schemas *tofu.Schemas) ([]Resource, error) {
func marshalPlanResources(changeMap map[string]*plans.ResourceInstanceChangeSrc, ris []addrs.AbsResourceInstance, schemas *tofu.Schemas) ([]Resource, error) {
var ret []Resource
for _, ri := range ris {
r := changes.ResourceInstance(ri)
// This is equivalent to calling plans.Changes.ResourceInstance(ri),
// but uses the precomputed changeMap to avoid quadratic complexity.
r := changeMap[ri.String()]
if r.Action == plans.Delete {
continue
}
@ -249,7 +264,7 @@ func marshalPlanResources(changes *plans.Changes, ris []addrs.AbsResourceInstanc
// marshalPlanModules iterates over a list of modules to recursively describe
// the full module tree.
func marshalPlanModules(
changes *plans.Changes,
changeMap map[string]*plans.ResourceInstanceChangeSrc,
schemas *tofu.Schemas,
childModules []addrs.ModuleInstance,
moduleMap map[string][]addrs.ModuleInstance,
@ -266,14 +281,14 @@ func marshalPlanModules(
if child.String() != "" {
cm.Address = child.String()
}
rs, err := marshalPlanResources(changes, moduleResources, schemas)
rs, err := marshalPlanResources(changeMap, moduleResources, schemas)
if err != nil {
return nil, err
}
cm.Resources = rs
if len(moduleMap[child.String()]) > 0 {
moreChildModules, err := marshalPlanModules(changes, schemas, moduleMap[child.String()], moduleMap, moduleResourceMap)
moreChildModules, err := marshalPlanModules(changeMap, schemas, moduleMap[child.String()], moduleMap, moduleResourceMap)
if err != nil {
return nil, err
}

View File

@ -16,6 +16,7 @@ import (
"github.com/opentofu/opentofu/internal/configs/configschema"
"github.com/opentofu/opentofu/internal/plans"
"github.com/opentofu/opentofu/internal/providers"
"github.com/opentofu/opentofu/internal/states"
"github.com/opentofu/opentofu/internal/tofu"
)
@ -295,7 +296,7 @@ func TestMarshalPlanResources(t *testing.T) {
ris := testResourceAddrs()
got, err := marshalPlanResources(testChange, ris, testSchemas())
got, err := marshalPlanResources(testChanges(testChange), ris, testSchemas())
if test.Err {
if err == nil {
t.Fatal("succeeded; want error")
@ -366,6 +367,16 @@ func testSchemas() *tofu.Schemas {
}
}
func testChanges(changes *plans.Changes) map[string]*plans.ResourceInstanceChangeSrc {
ret := make(map[string]*plans.ResourceInstanceChangeSrc)
for _, resource := range changes.Resources {
if resource.DeposedKey == states.NotDeposed {
ret[resource.Addr.String()] = resource
}
}
return ret
}
func testResourceAddrs() []addrs.AbsResourceInstance {
return []addrs.AbsResourceInstance{
mustAddr("test_thing.example"),