From 0811395e6f0d370c4dc0e026a255951acb86204d Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 23 Oct 2023 17:07:59 -0700 Subject: [PATCH] states: State.AllResourceInstanceObjectAddrs should say "Managed" This method used the more appropriate name AllManagedResourceInstanceObjectAddrs in its docstring, but was incorrectly named just AllResourceInstanceObjectAddrs in its actual definition, obscuring the fact that it filters out everything except objects from managed resources. Now we'll use the originally-documented name to make this clearer. This also factors out the implementation into a separate unexported function that takes the filtering criteria as an argument, since in future commits we're going to add other variations of this which return different subsets of the resource instance objects in the state. --- internal/command/views/test.go | 10 +++---- internal/command/workspace_delete.go | 2 +- internal/states/state.go | 39 ++++++++++++---------------- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/internal/command/views/test.go b/internal/command/views/test.go index b809631552..a12ee15ca1 100644 --- a/internal/command/views/test.go +++ b/internal/command/views/test.go @@ -280,7 +280,7 @@ func (t *TestHuman) DestroySummary(diags tfdiags.Diagnostics, run *moduletest.Ru // FIXME: This message says "resources" but this is actually a list // of resource instance objects. t.view.streams.Eprint(format.WordWrap(fmt.Sprintf("\nTerraform left the following resources in state after executing %s, and they need to be cleaned up manually:\n", identifier), t.view.errorColumns())) - for _, resource := range addrs.SetSortedNatural(state.AllResourceInstanceObjectAddrs()) { + for _, resource := range addrs.SetSortedNatural(state.AllManagedResourceInstanceObjectAddrs()) { if resource.DeposedKey != states.NotDeposed { t.view.streams.Eprintf(" - %s (%s)\n", resource.ResourceInstance, resource.DeposedKey) continue @@ -309,7 +309,7 @@ func (t *TestHuman) FatalInterruptSummary(run *moduletest.Run, file *moduletest. // with a run block. if state, exists := existingStates[nil]; exists && !state.Empty() { t.view.streams.Eprint(format.WordWrap("\nTerraform has already created the following resources from the module under test:\n", t.view.errorColumns())) - for _, resource := range addrs.SetSortedNatural(state.AllResourceInstanceObjectAddrs()) { + for _, resource := range addrs.SetSortedNatural(state.AllManagedResourceInstanceObjectAddrs()) { if resource.DeposedKey != states.NotDeposed { t.view.streams.Eprintf(" - %s (%s)\n", resource.ResourceInstance, resource.DeposedKey) continue @@ -326,7 +326,7 @@ func (t *TestHuman) FatalInterruptSummary(run *moduletest.Run, file *moduletest. } t.view.streams.Eprint(format.WordWrap(fmt.Sprintf("\nTerraform has already created the following resources for %q from %q:\n", run.Name, run.Config.Module.Source), t.view.errorColumns())) - for _, resource := range addrs.SetSortedNatural(state.AllResourceInstanceObjectAddrs()) { + for _, resource := range addrs.SetSortedNatural(state.AllManagedResourceInstanceObjectAddrs()) { if resource.DeposedKey != states.NotDeposed { t.view.streams.Eprintf(" - %s (%s)\n", resource.ResourceInstance, resource.DeposedKey) continue @@ -605,7 +605,7 @@ func (t *TestJSON) Run(run *moduletest.Run, file *moduletest.File, progress modu func (t *TestJSON) DestroySummary(diags tfdiags.Diagnostics, run *moduletest.Run, file *moduletest.File, state *states.State) { if state.HasManagedResourceInstanceObjects() { cleanup := json.TestFileCleanup{} - for _, resource := range addrs.SetSortedNatural(state.AllResourceInstanceObjectAddrs()) { + for _, resource := range addrs.SetSortedNatural(state.AllManagedResourceInstanceObjectAddrs()) { cleanup.FailedResources = append(cleanup.FailedResources, json.TestFailedResource{ Instance: resource.ResourceInstance.String(), DeposedKey: resource.DeposedKey.String(), @@ -663,7 +663,7 @@ func (t *TestJSON) FatalInterruptSummary(run *moduletest.Run, file *moduletest.F } var resources []json.TestFailedResource - for _, resource := range addrs.SetSortedNatural(state.AllResourceInstanceObjectAddrs()) { + for _, resource := range addrs.SetSortedNatural(state.AllManagedResourceInstanceObjectAddrs()) { resources = append(resources, json.TestFailedResource{ Instance: resource.ResourceInstance.String(), DeposedKey: resource.DeposedKey.String(), diff --git a/internal/command/workspace_delete.go b/internal/command/workspace_delete.go index 0b1689a1ad..93b202c365 100644 --- a/internal/command/workspace_delete.go +++ b/internal/command/workspace_delete.go @@ -134,7 +134,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { // We'll collect a list of what's being managed here as extra context // for the message. var buf strings.Builder - for _, obj := range stateMgr.State().AllResourceInstanceObjectAddrs() { + for _, obj := range stateMgr.State().AllManagedResourceInstanceObjectAddrs() { if obj.DeposedKey == states.NotDeposed { fmt.Fprintf(&buf, "\n - %s", obj.ResourceInstance.String()) } else { diff --git a/internal/states/state.go b/internal/states/state.go index 442c5d44c0..8a75896265 100644 --- a/internal/states/state.go +++ b/internal/states/state.go @@ -230,44 +230,39 @@ func (s *State) Resources(addr addrs.ConfigResource) []*Resource { // This method is guaranteed to return at least one item if // s.HasManagedResourceInstanceObjects returns true for the same state, and // to return a zero-length slice if it returns false. -func (s *State) AllResourceInstanceObjectAddrs() addrs.Set[addrs.AbsResourceInstanceObject] { +func (s *State) AllManagedResourceInstanceObjectAddrs() addrs.Set[addrs.AbsResourceInstanceObject] { + return s.allResourceInstanceObjectAddrs(func(addr addrs.AbsResourceInstanceObject) bool { + return addr.ResourceInstance.Resource.Resource.Mode == addrs.ManagedResourceMode + }) +} + +func (s *State) allResourceInstanceObjectAddrs(keepAddr func(addr addrs.AbsResourceInstanceObject) bool) addrs.Set[addrs.AbsResourceInstanceObject] { if s == nil { return nil } - // We use an unnamed return type here just because we currently have no - // general need to return pairs of instance address and deposed key aside - // from this method, and this method itself is only of marginal value - // when producing some error messages. - // - // If that need ends up arising more in future then it might make sense to - // name this as addrs.AbsResourceInstanceObject, although that would require - // moving DeposedKey into the addrs package too. - type ResourceInstanceObject = struct { - Instance addrs.AbsResourceInstance - DeposedKey DeposedKey - } ret := addrs.MakeSet[addrs.AbsResourceInstanceObject]() - for _, ms := range s.Modules { for _, rs := range ms.Resources { - if rs.Addr.Resource.Mode != addrs.ManagedResourceMode { - continue - } - for instKey, is := range rs.Instances { instAddr := rs.Addr.Instance(instKey) if is.Current != nil { - ret.Add(addrs.AbsResourceInstanceObject{ + objAddr := addrs.AbsResourceInstanceObject{ ResourceInstance: instAddr, DeposedKey: addrs.NotDeposed, - }) + } + if keepAddr(objAddr) { + ret.Add(objAddr) + } } for deposedKey := range is.Deposed { - ret.Add(addrs.AbsResourceInstanceObject{ + objAddr := addrs.AbsResourceInstanceObject{ ResourceInstance: instAddr, DeposedKey: deposedKey, - }) + } + if keepAddr(objAddr) { + ret.Add(objAddr) + } } } }