From d7ef123c127307fb24abb4a71422689b375ca9a4 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 21 Dec 2021 14:50:47 -0500 Subject: [PATCH] refactoring: Move nested modules When applying module `moved` statements by iterating through modules in state, we previously required an exact match from the `moved` statement's `from` field and the module address. This permitted moving resources directly inside a module, but did not recur into module calls within those moved modules. This commit moves that exact match requirement so that it only applies to `moved` statements targeting resources. In turn this allows nested modules to be moved. --- internal/refactoring/move_execute.go | 20 +++++--- internal/refactoring/move_execute_test.go | 62 +++++++++++++++++++++++ 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/internal/refactoring/move_execute.go b/internal/refactoring/move_execute.go index 7f5fbae23..b99da1072 100644 --- 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 index ab71f5b0f..3f568ee79 100644 --- 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]"),