Merge pull request #24254 from hashicorp/jbardin/state-mv

state mv should always use target address to determine each mode.

command/state_mv.go

@@ -280,31 +280,30 @@ func (c *StateMvCommand) Run(args []string) int {
 				fromResourceAddr := addrFrom.ContainingResource()
 				fromResource := ssFrom.Resource(fromResourceAddr)
 				fromProviderAddr := fromResource.ProviderConfig
-				fromEachMode := fromResource.EachMode
 				ssFrom.ForgetResourceInstanceAll(addrFrom)
 				ssFrom.RemoveResourceIfEmpty(fromResourceAddr)
 
+				// since this is moving an instance, we can infer the target
+				// mode from the address.
+				toEachMode := eachModeForInstanceKey(addrTo.Resource.Key)
+
 				rs := stateTo.Resource(addrTo.ContainingResource())
 				if rs == nil {
 					// If we're moving to an address without an index then that
 					// suggests the user's intent is to establish both the
 					// resource and the instance at the same time (since the
 					// address covers both). If there's an index in the
-					// target then allow creating the new instance here,
-					// inferring the mode from how the new address was parsed.
-					if addrTo.Resource.Key != addrs.NoKey {
-						fromEachMode = eachModeForInstanceKey(addrTo.Resource.Key)
-					}
-
+					// target then allow creating the new instance here.
 					resourceAddr := addrTo.ContainingResource()
 					stateTo.SyncWrapper().SetResourceMeta(
 						resourceAddr,
-						fromEachMode,
+						toEachMode,
 						fromProviderAddr, // in this case, we bring the provider along as if we were moving the whole resource
 					)
 					rs = stateTo.Resource(resourceAddr)
 				}
 
+				rs.EachMode = toEachMode
 				rs.Instances[addrTo.Resource.Key] = is
 			}
 		default:

command/state_mv_test.go

@@ -68,7 +68,7 @@ func TestStateMv(t *testing.T) {
 		"test_instance.bar",
 	}
 	if code := c.Run(args); code != 0 {
-		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+		t.Fatalf("return code: %d\n\n%s", code, ui.ErrorWriter.String())
 	}
 
 	// Test it is correct
@@ -80,6 +80,70 @@ func TestStateMv(t *testing.T) {
 		t.Fatalf("bad: %#v", backups)
 	}
 	testStateOutput(t, backups[0], testStateMvOutputOriginal)
+
+	// Change the single instance to a counted instance
+	args = []string{
+		"-state", statePath,
+		"test_instance.bar",
+		"test_instance.bar[0]",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("return code: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// extract the resource and verify the mode
+	s := testStateRead(t, statePath)
+	addr, diags := addrs.ParseAbsResourceStr("test_instance.bar")
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+	i := s.Resource(addr)
+	if i.EachMode != states.EachList {
+		t.Fatalf("expected each mode List, got %s", i.EachMode)
+	}
+
+	// change from list to map
+	args = []string{
+		"-state", statePath,
+		"test_instance.bar[0]",
+		"test_instance.bar[\"baz\"]",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("return code: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// extract the resource and verify the mode
+	s = testStateRead(t, statePath)
+	addr, diags = addrs.ParseAbsResourceStr("test_instance.bar")
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+	i = s.Resource(addr)
+	if i.EachMode != states.EachMap {
+		t.Fatalf("expected each mode Map, got %s", i.EachMode)
+	}
+
+	// change from from map back to single
+	args = []string{
+		"-state", statePath,
+		"test_instance.bar[\"baz\"]",
+		"test_instance.bar",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("return code: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// extract the resource and verify the mode
+	s = testStateRead(t, statePath)
+	addr, diags = addrs.ParseAbsResourceStr("test_instance.bar")
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+	i = s.Resource(addr)
+	if i.EachMode != states.NoEach {
+		t.Fatalf("expected each mode NoEach, got %s", i.EachMode)
+	}
+
 }
 
 func TestStateMv_resourceToInstance(t *testing.T) {