From 17e1c9dd05ed9d95d963c2011e42ae8439691063 Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Wed, 8 Jul 2020 17:09:58 -0400
Subject: [PATCH] command: Fix state mv for only resource in module

When moving a resource block with multiple instances to a new address
within the same module, we need to ensure that the target module is
present as late as possible. Otherwise, deleting the resource from the
original address triggers pruning, and the module is removed just before
we try to add the resource to it, which causes a crash.

Includes regression test which panics without this code change.
---
 command/state_mv.go      |  7 ++--
 command/state_mv_test.go | 76 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/command/state_mv.go b/command/state_mv.go
index 10e9a86f7..812441699 100644
--- a/command/state_mv.go
+++ b/command/state_mv.go
@@ -190,10 +190,7 @@ func (c *StateMvCommand) Run(args []string) int {
 				return 1
 			}
 			diags = diags.Append(c.validateResourceMove(addrFrom, addrTo))
-			if stateTo.Module(addrTo.Module) == nil {
-				// moving something to a mew module, so we need to ensure it exists
-				stateTo.EnsureModule(addrTo.Module)
-			}
+
 			if stateTo.Resource(addrTo) != nil {
 				diags = diags.Append(tfdiags.Sourceless(
 					tfdiags.Error,
@@ -223,7 +220,7 @@ func (c *StateMvCommand) Run(args []string) int {
 
 				// Update the address before adding it to the state.
 				rs.Addr = addrTo
-				stateTo.Module(addrTo.Module).Resources[addrTo.Resource.String()] = rs
+				stateTo.EnsureModule(addrTo.Module).Resources[addrTo.Resource.String()] = rs
 			}
 
 		case addrs.AbsResourceInstance:
diff --git a/command/state_mv_test.go b/command/state_mv_test.go
index f970185b1..38cfe34a8 100644
--- a/command/state_mv_test.go
+++ b/command/state_mv_test.go
@@ -1196,6 +1196,62 @@ func TestStateMv_fromBackendToLocal(t *testing.T) {
 	testStateOutput(t, statePath, testStateMvOriginal_backend)
 }
 
+// This test covers moving the only resource in a module to a new address in
+// that module, which triggers the maybePruneModule functionality. This caused
+// a panic report: https://github.com/hashicorp/terraform/issues/25520
+func TestStateMv_onlyResourceInModule(t *testing.T) {
+	state := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "test_instance",
+				Name: "foo",
+			}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance.Child("foo", addrs.NoKey)),
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"id":"bar","foo":"value","bar":"value"}`),
+				Status:    states.ObjectReady,
+			},
+			addrs.AbsProviderConfig{
+				Provider: addrs.NewLegacyProvider("test"),
+				Module:   addrs.RootModule,
+			},
+		)
+	})
+
+	statePath := testStateFile(t, state)
+	testStateOutput(t, statePath, testStateMvOnlyResourceInModule_original)
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &StateMvCommand{
+		StateMeta{
+			Meta: Meta{
+				testingOverrides: metaOverridesForProvider(p),
+				Ui:               ui,
+			},
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		"module.foo.test_instance.foo",
+		"module.foo.test_instance.bar",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// Test it is correct
+	testStateOutput(t, statePath, testStateMvOnlyResourceInModule_output)
+
+	// Test we have backups
+	backups := testStateBackups(t, filepath.Dir(statePath))
+	if len(backups) != 1 {
+		t.Fatalf("bad: %#v", backups)
+	}
+	testStateOutput(t, backups[0], testStateMvOnlyResourceInModule_original)
+}
+
 const testStateMvOutputOriginal = `
 test_instance.baz:
   ID = foo
@@ -1513,3 +1569,23 @@ test_instance.baz:
   bar = value
   foo = value
 `
+
+const testStateMvOnlyResourceInModule_original = `
+<no state>
+module.foo:
+  test_instance.foo.0:
+    ID = bar
+    provider = provider["registry.terraform.io/-/test"]
+    bar = value
+    foo = value
+`
+
+const testStateMvOnlyResourceInModule_output = `
+<no state>
+module.foo:
+  test_instance.bar.0:
+    ID = bar
+    provider = provider["registry.terraform.io/-/test"]
+    bar = value
+    foo = value
+`