Merge pull request #30233 from hashicorp/alisdair/move-nested-modules

refactoring: Move nested modules
hashicorp · Dec 22, 2021 · f772cb0 · f772cb0
2 parents a72d2d4 + d7ef123
commit f772cb0
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 8 deletions.
diff --git a/internal/refactoring/move_execute.go b/internal/refactoring/move_execute.go
@@ -88,16 +88,13 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
 
 		for _, ms := range state.Modules {
 			modAddr := ms.Addr
-			if !stmt.From.SelectsModule(modAddr) {
-				continue
-			}
 
-			// We now know that the current module is relevant but what
-			// we'll do with it depends on the object kind.
+			// We don't yet know that the current module is relevant, and
+			// we determine that differently for each the object kind.
 			switch kind := stmt.ObjectKind(); kind {
 			case addrs.MoveEndpointModule:
 				// For a module endpoint we just try the module address
-				// directly.
+				// directly, and execute the moves if it matches.
 				if newAddr, matches := modAddr.MoveDestination(stmt.From, stmt.To); matches {
 					log.Printf("[TRACE] refactoring.ApplyMoves: %s has moved to %s", modAddr, newAddr)
 
@@ -125,8 +122,15 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
 					continue
 				}
 			case addrs.MoveEndpointResource:
-				// For a resource endpoint we need to search each of the
-				// resources and resource instances in the module.
+				// For a resource endpoint we require an exact containing
+				// module match, because by definition a matching resource
+				// cannot be nested any deeper than that.
+				if !stmt.From.SelectsModule(modAddr) {
+					continue
+				}
+
+				// We then need to search each of the resources and resource
+				// instances in the module.
 				for _, rs := range ms.Resources {
 					rAddr := rs.Addr
 					if newAddr, matches := rAddr.MoveDestination(stmt.From, stmt.To); matches {

diff --git a/internal/refactoring/move_execute_test.go b/internal/refactoring/move_execute_test.go
@@ -21,7 +21,10 @@ func TestApplyMoves(t *testing.T) {
 	}
 
 	moduleBoo, _ := addrs.ParseModuleInstanceStr("module.boo")
+	moduleBar, _ := addrs.ParseModuleInstanceStr("module.bar")
 	moduleBarKey, _ := addrs.ParseModuleInstanceStr("module.bar[0]")
+	moduleBooHoo, _ := addrs.ParseModuleInstanceStr("module.boo.module.hoo")
+	moduleBarHoo, _ := addrs.ParseModuleInstanceStr("module.bar.module.hoo")
 
 	instAddrs := map[string]addrs.AbsResourceInstance{
 		"foo.from": addrs.Resource{
@@ -84,6 +87,12 @@ func TestApplyMoves(t *testing.T) {
 			Name: "to",
 		}.Instance(addrs.IntKey(0)).Absolute(moduleBoo),
 
+		"module.bar.foo.from": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.NoKey).Absolute(moduleBar),
+
 		"module.bar[0].foo.from": addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
 			Type: "foo",
@@ -113,6 +122,18 @@ func TestApplyMoves(t *testing.T) {
 			Type: "foo",
 			Name: "to",
 		}.Instance(addrs.IntKey(0)).Absolute(moduleBarKey),
+
+		"module.boo.module.hoo.foo.from": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.NoKey).Absolute(moduleBooHoo),
+
+		"module.bar.module.hoo.foo.from": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.NoKey).Absolute(moduleBarHoo),
 	}
 
 	emptyResults := MoveResults{
@@ -289,6 +310,47 @@ func TestApplyMoves(t *testing.T) {
 			},
 		},
 
+		"module move with child module": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "module.boo", "module.bar"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["module.boo.foo.from"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+				s.SetResourceInstanceCurrent(
+					instAddrs["module.boo.module.hoo.foo.from"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			MoveResults{
+				Changes: map[addrs.UniqueKey]MoveSuccess{
+					instAddrs["module.bar.foo.from"].UniqueKey(): {
+						From: instAddrs["module.boo.foo.from"],
+						To:   instAddrs["module.bar.foo.from"],
+					},
+					instAddrs["module.bar.module.hoo.foo.from"].UniqueKey(): {
+						From: instAddrs["module.boo.module.hoo.foo.from"],
+						To:   instAddrs["module.bar.module.hoo.foo.from"],
+					},
+				},
+				Blocked: map[addrs.UniqueKey]MoveBlocked{},
+			},
+			[]string{
+				`module.bar.foo.from`,
+				`module.bar.module.hoo.foo.from`,
+			},
+		},
+
 		"move whole single module to indexed module": {
 			[]MoveStatement{
 				testMoveStatement(t, "", "module.boo", "module.bar[0]"),