diff --git a/internal/command/jsonplan/values.go b/internal/command/jsonplan/values.go index 69d04ea205..ffe343d735 100644 --- a/internal/command/jsonplan/values.go +++ b/internal/command/jsonplan/values.go @@ -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 } diff --git a/internal/command/jsonplan/values_test.go b/internal/command/jsonplan/values_test.go index cebde725fe..f739dbb2b8 100644 --- a/internal/command/jsonplan/values_test.go +++ b/internal/command/jsonplan/values_test.go @@ -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"),