From 09ab95268304ff802decba099dd45f2ce81aaac6 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 2 Aug 2021 17:20:06 -0400 Subject: [PATCH 1/9] fix ApplyMoves tests Add empty result value, since ApplyMoves does not return nil. Fix the desired addresses for moves. --- internal/refactoring/move_execute.go | 5 +++-- internal/refactoring/move_execute_test.go | 16 +++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/internal/refactoring/move_execute.go b/internal/refactoring/move_execute.go index 1fc60f85f..26eef65c0 100644 --- a/internal/refactoring/move_execute.go +++ b/internal/refactoring/move_execute.go @@ -28,6 +28,8 @@ type MoveResult struct { // ApplyMoves expects exclusive access to the given state while it's running. // Don't read or write any part of the state structure until ApplyMoves returns. func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]MoveResult { + results := make(map[addrs.UniqueKey]MoveResult) + // The methodology here is to construct a small graph of all of the move // statements where the edges represent where a particular statement // is either chained from or nested inside the effect of another statement. @@ -40,7 +42,7 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey] // at all. The separate validation step should detect this and return // an error. if len(g.Cycles()) != 0 { - return nil + return results } // The starting nodes are the ones that don't depend on any other nodes. @@ -51,7 +53,6 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey] } } - results := make(map[addrs.UniqueKey]MoveResult) g.DepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error { stmt := v.(*MoveStatement) diff --git a/internal/refactoring/move_execute_test.go b/internal/refactoring/move_execute_test.go index eed56e911..4041bedff 100644 --- a/internal/refactoring/move_execute_test.go +++ b/internal/refactoring/move_execute_test.go @@ -15,10 +15,6 @@ import ( ) func TestApplyMoves(t *testing.T) { - // TODO: Renable this once we're ready to implement the intended behaviors - // it is describing. - t.Skip("ApplyMoves is not yet fully implemented") - providerAddr := addrs.AbsProviderConfig{ Module: addrs.RootModule, Provider: addrs.MustParseProviderSourceString("example.com/foo/bar"), @@ -48,6 +44,8 @@ func TestApplyMoves(t *testing.T) { }.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance), } + emptyResults := map[addrs.UniqueKey]MoveResult{} + tests := map[string]struct { Stmts []MoveStatement State *states.State @@ -58,7 +56,7 @@ func TestApplyMoves(t *testing.T) { "no moves and empty state": { []MoveStatement{}, states.NewState(), - nil, + emptyResults, nil, }, "no moves": { @@ -73,7 +71,7 @@ func TestApplyMoves(t *testing.T) { providerAddr, ) }), - nil, + emptyResults, []string{ `foo.from`, }, @@ -98,7 +96,7 @@ func TestApplyMoves(t *testing.T) { To: rootNoKeyResourceAddr[1], }, rootNoKeyResourceAddr[1].UniqueKey(): { - From: rootNoKeyResourceAddr[1], + From: rootNoKeyResourceAddr[0], To: rootNoKeyResourceAddr[1], }, }, @@ -121,11 +119,11 @@ func TestApplyMoves(t *testing.T) { ) }), map[addrs.UniqueKey]MoveResult{ - rootNoKeyResourceAddr[0].UniqueKey(): { + rootIntKeyResourceAddr[0].UniqueKey(): { From: rootIntKeyResourceAddr[0], To: rootIntKeyResourceAddr[1], }, - rootNoKeyResourceAddr[1].UniqueKey(): { + rootIntKeyResourceAddr[1].UniqueKey(): { From: rootIntKeyResourceAddr[0], To: rootIntKeyResourceAddr[1], }, From 6401022bc8e629ca34ba9851648e41317878ed24 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 3 Aug 2021 16:53:45 -0400 Subject: [PATCH 2/9] don't take the address of a range variable --- internal/refactoring/move_execute.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/refactoring/move_execute.go b/internal/refactoring/move_execute.go index 26eef65c0..011fc9f64 100644 --- a/internal/refactoring/move_execute.go +++ b/internal/refactoring/move_execute.go @@ -148,9 +148,9 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey] // may contain cycles and other sorts of invalidity. func buildMoveStatementGraph(stmts []MoveStatement) *dag.AcyclicGraph { g := &dag.AcyclicGraph{} - for _, stmt := range stmts { + for i := range stmts { // The graph nodes are pointers to the actual statements directly. - g.Add(&stmt) + g.Add(&stmts[i]) } // Now we'll add the edges representing chaining and nesting relationships. From 789317dc05157a3d8d8b76aaf88aa9f8d82a3053 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 4 Aug 2021 09:36:22 -0400 Subject: [PATCH 3/9] additional test --- internal/refactoring/move_execute_test.go | 40 ++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/internal/refactoring/move_execute_test.go b/internal/refactoring/move_execute_test.go index 4041bedff..5943445cb 100644 --- a/internal/refactoring/move_execute_test.go +++ b/internal/refactoring/move_execute_test.go @@ -30,6 +30,11 @@ func TestApplyMoves(t *testing.T) { Type: "foo", Name: "to", }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "mid", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), } rootIntKeyResourceAddr := [...]addrs.AbsResourceInstance{ addrs.Resource{ @@ -132,13 +137,46 @@ func TestApplyMoves(t *testing.T) { `foo.to[0]`, }, }, + "chained move of whole singleton resource": { + []MoveStatement{ + testMoveStatement(t, "", "foo.from", "foo.mid"), + testMoveStatement(t, "", "foo.mid", "foo.to"), + }, + states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + rootNoKeyResourceAddr[0], + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{}`), + }, + providerAddr, + ) + }), + map[addrs.UniqueKey]MoveResult{ + rootNoKeyResourceAddr[0].UniqueKey(): { + From: rootNoKeyResourceAddr[0], + To: rootNoKeyResourceAddr[2], + }, + rootNoKeyResourceAddr[2].UniqueKey(): { + From: rootNoKeyResourceAddr[2], + To: rootNoKeyResourceAddr[1], + }, + rootNoKeyResourceAddr[1].UniqueKey(): { + From: rootNoKeyResourceAddr[2], + To: rootNoKeyResourceAddr[1], + }, + }, + []string{ + `foo.to`, + }, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { var stmtsBuf strings.Builder for _, stmt := range test.Stmts { - fmt.Fprintf(&stmtsBuf, "- from: %s\n to: %s", stmt.From, stmt.To) + fmt.Fprintf(&stmtsBuf, "- from: %s\n to: %s\n", stmt.From, stmt.To) } t.Logf("move statements:\n%s", stmtsBuf.String()) From 08edb022708e6093517c7748426603f53c112a4c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 5 Aug 2021 15:54:36 -0400 Subject: [PATCH 4/9] MoveStatement.Name() makes the graph printable for debugging --- internal/refactoring/move_statement.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/refactoring/move_statement.go b/internal/refactoring/move_statement.go index 59f7f55de..9edafc7b4 100644 --- a/internal/refactoring/move_statement.go +++ b/internal/refactoring/move_statement.go @@ -50,3 +50,8 @@ func (s *MoveStatement) ObjectKind() addrs.MoveEndpointKind { // match it. return s.From.ObjectKind() } + +// Name is used internally for displaying the statement graph +func (s *MoveStatement) Name() string { + return fmt.Sprintf("%s->%s", s.From, s.To) +} From 88ad938cc670d43496c60bc9c3948786a448dd80 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 5 Aug 2021 15:55:28 -0400 Subject: [PATCH 5/9] Equal methods for move AbsMoveable Make sure all the types are comparable --- internal/addrs/module_call.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/addrs/module_call.go b/internal/addrs/module_call.go index 4e8ce34f2..709b1e302 100644 --- a/internal/addrs/module_call.go +++ b/internal/addrs/module_call.go @@ -37,6 +37,10 @@ func (c ModuleCall) Absolute(moduleAddr ModuleInstance) AbsModuleCall { } } +func (c ModuleCall) Equal(other ModuleCall) bool { + return c.Name == other.Name +} + // AbsModuleCall is the address of a "module" block relative to the root // of the configuration. // @@ -70,6 +74,10 @@ func (c AbsModuleCall) Instance(key InstanceKey) ModuleInstance { return ret } +func (c AbsModuleCall) Equal(other AbsModuleCall) bool { + return c.Module.Equal(other.Module) && c.Call.Equal(other.Call) +} + type absModuleCallInstanceKey string func (c AbsModuleCall) UniqueKey() UniqueKey { From 493ec4e6c527f7f8e00dd621714c2cd79073af58 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 5 Aug 2021 15:57:44 -0400 Subject: [PATCH 6/9] correct the direction and walk order of the graph --- internal/refactoring/move_execute.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/refactoring/move_execute.go b/internal/refactoring/move_execute.go index 011fc9f64..800810981 100644 --- a/internal/refactoring/move_execute.go +++ b/internal/refactoring/move_execute.go @@ -48,12 +48,12 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey] // The starting nodes are the ones that don't depend on any other nodes. startNodes := make(dag.Set, len(stmts)) for _, v := range g.Vertices() { - if len(g.UpEdges(v)) == 0 { + if len(g.DownEdges(v)) == 0 { startNodes.Add(v) } } - g.DepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error { + g.ReverseDepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error { stmt := v.(*MoveStatement) for _, ms := range state.Modules { From 6087b1bdb9ab99fd6170ca02c0f1a5472efa2f1b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 5 Aug 2021 15:56:31 -0400 Subject: [PATCH 7/9] CanChainFrom and NestedWithin Add implementations of CanChainFrom and NestedWithin for MoveEndpointInModule. CanChainFrom allows the linking of move statements of the same address, which means the prior destination address must equal the following source address. If the destination and source addresses are of different types, they must be covered by NestedWithin rather than CanChainFrom. NestedWithin checks if the destination contains the source address. Any matching types would be covered by CanChainFrom. --- internal/addrs/move_endpoint_module.go | 77 ++++++++- internal/addrs/move_endpoint_module_test.go | 176 ++++++++++++++++++++ 2 files changed, 251 insertions(+), 2 deletions(-) diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index 1ed6fbee1..aea1e643a 100644 --- a/internal/addrs/move_endpoint_module.go +++ b/internal/addrs/move_endpoint_module.go @@ -218,7 +218,35 @@ func (e *MoveEndpointInModule) SelectsModule(addr ModuleInstance) bool { // the reciever is the "to" from one statement and the other given address // is the "from" of another statement. func (e *MoveEndpointInModule) CanChainFrom(other *MoveEndpointInModule) bool { - // TODO: implement + eSub := e.relSubject + oSub := other.relSubject + + switch oSub := oSub.(type) { + case AbsModuleCall: + switch eSub := eSub.(type) { + case AbsModuleCall: + return eSub.Equal(oSub) + } + + case ModuleInstance: + switch eSub := eSub.(type) { + case ModuleInstance: + return eSub.Equal(oSub) + } + + case AbsResource: + switch eSub := eSub.(type) { + case AbsResource: + return eSub.Equal(oSub) + } + + case AbsResourceInstance: + switch eSub := eSub.(type) { + case AbsResourceInstance: + return eSub.Equal(oSub) + } + } + return false } @@ -226,7 +254,52 @@ func (e *MoveEndpointInModule) CanChainFrom(other *MoveEndpointInModule) bool { // contained within one of the objects that the given other address could // select. func (e *MoveEndpointInModule) NestedWithin(other *MoveEndpointInModule) bool { - // TODO: implement + eSub := e.relSubject + oSub := other.relSubject + + switch oSub := oSub.(type) { + case AbsModuleCall: + withinModuleCall := func(mod ModuleInstance, call AbsModuleCall) bool { + // parent modules don't match at all + if !call.Module.IsAncestor(mod) { + return false + } + + rem := mod[len(call.Module):] + return rem[0].Name == call.Call.Name + } + + // Module calls can contain module instances, resources, and resource + // instances. + switch eSub := eSub.(type) { + case AbsResource: + return withinModuleCall(eSub.Module, oSub) + + case AbsResourceInstance: + return withinModuleCall(eSub.Module, oSub) + + case ModuleInstance: + return withinModuleCall(eSub, oSub) + } + + case ModuleInstance: + // Module instances can contain resources and resource instances. + switch eSub := eSub.(type) { + case AbsResource: + return eSub.Module.Equal(oSub) || oSub.IsAncestor(eSub.Module) + + case AbsResourceInstance: + return eSub.Module.Equal(oSub) || oSub.IsAncestor(eSub.Module) + } + + case AbsResource: + // A resource can only contain a resource instance. + switch eSub := eSub.(type) { + case AbsResourceInstance: + return eSub.ContainingResource().Equal(oSub) + } + } + return false } diff --git a/internal/addrs/move_endpoint_module_test.go b/internal/addrs/move_endpoint_module_test.go index 1e52eea23..ff772aba1 100644 --- a/internal/addrs/move_endpoint_module_test.go +++ b/internal/addrs/move_endpoint_module_test.go @@ -1074,3 +1074,179 @@ func TestAbsResourceMoveDestination(t *testing.T) { ) } } + +func TestMoveEndpointChainAndNested(t *testing.T) { + tests := []struct { + Endpoint, Other AbsMoveable + CanChainFrom, NestedWithin bool + }{ + { + Endpoint: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + Other: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + CanChainFrom: true, + NestedWithin: false, + }, + + { + Endpoint: mustParseModuleInstanceStr("module.foo[2]"), + Other: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + CanChainFrom: false, + NestedWithin: false, + }, + + { + Endpoint: mustParseModuleInstanceStr("module.foo[2].module.bar[2]"), + Other: AbsModuleCall{ + Module: RootModuleInstance, + Call: ModuleCall{Name: "foo"}, + }, + CanChainFrom: false, + NestedWithin: true, + }, + + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].module.bar.resource.baz").ContainingResource(), + Other: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + CanChainFrom: false, + NestedWithin: true, + }, + + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].module.bar[3].resource.baz[2]"), + Other: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + CanChainFrom: false, + NestedWithin: true, + }, + + { + Endpoint: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + Other: mustParseModuleInstanceStr("module.foo[2]"), + CanChainFrom: false, + NestedWithin: false, + }, + + { + Endpoint: mustParseModuleInstanceStr("module.foo[2]"), + Other: mustParseModuleInstanceStr("module.foo[2]"), + CanChainFrom: true, + NestedWithin: false, + }, + + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(), + Other: mustParseModuleInstanceStr("module.foo[2]"), + CanChainFrom: false, + NestedWithin: true, + }, + + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].module.bar.resource.baz"), + Other: mustParseModuleInstanceStr("module.foo[2]"), + CanChainFrom: false, + NestedWithin: true, + }, + + { + Endpoint: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(), + CanChainFrom: false, + NestedWithin: false, + }, + + { + Endpoint: mustParseModuleInstanceStr("module.foo[2]"), + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(), + CanChainFrom: false, + NestedWithin: false, + }, + + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(), + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(), + CanChainFrom: true, + NestedWithin: false, + }, + + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"), + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz[2]").ContainingResource(), + CanChainFrom: false, + NestedWithin: true, + }, + + { + Endpoint: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"), + CanChainFrom: false, + }, + { + Endpoint: mustParseModuleInstanceStr("module.foo[2]"), + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"), + CanChainFrom: false, + }, + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(), + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"), + CanChainFrom: false, + }, + { + Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"), + Other: mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"), + CanChainFrom: true, + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("[%02d]%s.CanChainFrom(%s)", i, test.Endpoint, test.Other), + func(t *testing.T) { + endpoint := &MoveEndpointInModule{ + relSubject: test.Endpoint, + } + + other := &MoveEndpointInModule{ + relSubject: test.Other, + } + + if endpoint.CanChainFrom(other) != test.CanChainFrom { + t.Errorf("expected %s CanChainFrom %s == %t", test.Endpoint, test.Other, test.CanChainFrom) + } + + if endpoint.NestedWithin(other) != test.NestedWithin { + t.Errorf("expected %s NestedWithin %s == %t", test.Endpoint, test.Other, test.NestedWithin) + } + }, + ) + } +} + +func mustParseAbsResourceInstanceStr(s string) AbsResourceInstance { + r, diags := ParseAbsResourceInstanceStr(s) + if diags.HasErrors() { + panic(diags.ErrWithWarnings().Error()) + } + return r +} From 2dff0481c83f2b9378e5b1631f1ea2d3429fd2af Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 19 Aug 2021 11:42:58 -0400 Subject: [PATCH 8/9] missed relMatch for AbsModuleCall in SelectsModule --- internal/addrs/move_endpoint_module.go | 4 + internal/addrs/move_endpoint_module_test.go | 105 ++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index aea1e643a..5d53ae770 100644 --- a/internal/addrs/move_endpoint_module.go +++ b/internal/addrs/move_endpoint_module.go @@ -192,6 +192,8 @@ func (e *MoveEndpointInModule) SelectsModule(addr ModuleInstance) bool { if callPart.Name != relAddr.Call.Name { return false } + + relMatch = relAddr.Module.Child(relAddr.Call.Name, callPart.InstanceKey) case AbsResource: relMatch = relAddr.Module case AbsResourceInstance: @@ -203,11 +205,13 @@ func (e *MoveEndpointInModule) SelectsModule(addr ModuleInstance) bool { if len(relPart) != len(relMatch) { return false } + for i := range relMatch { if relPart[i] != relMatch[i] { return false } } + return true } diff --git a/internal/addrs/move_endpoint_module_test.go b/internal/addrs/move_endpoint_module_test.go index ff772aba1..2dd61e261 100644 --- a/internal/addrs/move_endpoint_module_test.go +++ b/internal/addrs/move_endpoint_module_test.go @@ -1243,6 +1243,111 @@ func TestMoveEndpointChainAndNested(t *testing.T) { } } +func TestSelectsModule(t *testing.T) { + tests := []struct { + Endpoint *MoveEndpointInModule + Addr ModuleInstance + Selects bool + }{ + { + Endpoint: &MoveEndpointInModule{ + relSubject: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.foo[2]"), + Call: ModuleCall{Name: "bar"}, + }, + }, + Addr: mustParseModuleInstanceStr("module.foo[2].module.bar[1]"), + Selects: true, + }, + { + Endpoint: &MoveEndpointInModule{ + module: mustParseModuleInstanceStr("module.foo").Module(), + relSubject: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.bar[2]"), + Call: ModuleCall{Name: "baz"}, + }, + }, + Addr: mustParseModuleInstanceStr("module.foo[2].module.bar[2].module.baz"), + Selects: true, + }, + { + Endpoint: &MoveEndpointInModule{ + module: mustParseModuleInstanceStr("module.foo").Module(), + relSubject: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.bar[2]"), + Call: ModuleCall{Name: "baz"}, + }, + }, + Addr: mustParseModuleInstanceStr("module.foo[2].module.bar[1].module.baz"), + Selects: false, + }, + { + Endpoint: &MoveEndpointInModule{ + relSubject: AbsModuleCall{ + Module: mustParseModuleInstanceStr("module.bar"), + Call: ModuleCall{Name: "baz"}, + }, + }, + Addr: mustParseModuleInstanceStr("module.bar[1].module.baz"), + Selects: false, + }, + { + Endpoint: &MoveEndpointInModule{ + module: mustParseModuleInstanceStr("module.foo").Module(), + relSubject: mustParseAbsResourceInstanceStr(`module.bar.resource.name["key"]`), + }, + Addr: mustParseModuleInstanceStr(`module.foo[1].module.bar`), + Selects: true, + }, + { + Endpoint: &MoveEndpointInModule{ + relSubject: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`), + }, + Addr: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`), + Selects: true, + }, + { + Endpoint: &MoveEndpointInModule{ + relSubject: mustParseAbsResourceInstanceStr(`module.bar.module.baz["key"].resource.name`).ContainingResource(), + }, + Addr: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`), + Selects: true, + }, + { + Endpoint: &MoveEndpointInModule{ + module: mustParseModuleInstanceStr("module.nope").Module(), + relSubject: mustParseAbsResourceInstanceStr(`module.bar.resource.name["key"]`), + }, + Addr: mustParseModuleInstanceStr(`module.foo[1].module.bar`), + Selects: false, + }, + { + Endpoint: &MoveEndpointInModule{ + relSubject: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`), + }, + Addr: mustParseModuleInstanceStr(`module.bar.module.baz["nope"]`), + Selects: false, + }, + { + Endpoint: &MoveEndpointInModule{ + relSubject: mustParseAbsResourceInstanceStr(`module.nope.module.baz["key"].resource.name`).ContainingResource(), + }, + Addr: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`), + Selects: false, + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("[%02d]%s.SelectsModule(%s)", i, test.Endpoint, test.Addr), + func(t *testing.T) { + if test.Endpoint.SelectsModule(test.Addr) != test.Selects { + t.Errorf("expected %s SelectsModule %s == %t", test.Endpoint, test.Addr, test.Selects) + } + }, + ) + } +} + func mustParseAbsResourceInstanceStr(s string) AbsResourceInstance { r, diags := ParseAbsResourceInstanceStr(s) if diags.HasErrors() { From 553d6525d22939764867c0fd39f8d3fc95e87715 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 19 Aug 2021 11:44:22 -0400 Subject: [PATCH 9/9] more move tests --- internal/refactoring/move_execute_test.go | 287 +++++++++++++++++++--- 1 file changed, 250 insertions(+), 37 deletions(-) diff --git a/internal/refactoring/move_execute_test.go b/internal/refactoring/move_execute_test.go index 5943445cb..126a56ee7 100644 --- a/internal/refactoring/move_execute_test.go +++ b/internal/refactoring/move_execute_test.go @@ -19,34 +19,100 @@ func TestApplyMoves(t *testing.T) { Module: addrs.RootModule, Provider: addrs.MustParseProviderSourceString("example.com/foo/bar"), } - rootNoKeyResourceAddr := [...]addrs.AbsResourceInstance{ - addrs.Resource{ + + moduleBoo, _ := addrs.ParseModuleInstanceStr("module.boo") + moduleBarKey, _ := addrs.ParseModuleInstanceStr("module.bar[0]") + + instAddrs := map[string]addrs.AbsResourceInstance{ + "foo.from": addrs.Resource{ Mode: addrs.ManagedResourceMode, Type: "foo", Name: "from", }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "foo", - Name: "to", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - addrs.Resource{ + + "foo.mid": addrs.Resource{ Mode: addrs.ManagedResourceMode, Type: "foo", Name: "mid", }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - } - rootIntKeyResourceAddr := [...]addrs.AbsResourceInstance{ - addrs.Resource{ + + "foo.to": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "to", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + + "foo.from[0]": addrs.Resource{ Mode: addrs.ManagedResourceMode, Type: "foo", Name: "from", }.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance), - addrs.Resource{ + + "foo.to[0]": addrs.Resource{ Mode: addrs.ManagedResourceMode, Type: "foo", Name: "to", }.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance), + + "module.boo.foo.from": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "from", + }.Instance(addrs.NoKey).Absolute(moduleBoo), + + "module.boo.foo.mid": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "mid", + }.Instance(addrs.NoKey).Absolute(moduleBoo), + + "module.boo.foo.to": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "to", + }.Instance(addrs.NoKey).Absolute(moduleBoo), + + "module.boo.foo.from[0]": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "from", + }.Instance(addrs.IntKey(0)).Absolute(moduleBoo), + + "module.boo.foo.to[0]": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "to", + }.Instance(addrs.IntKey(0)).Absolute(moduleBoo), + + "module.bar[0].foo.from": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "from", + }.Instance(addrs.NoKey).Absolute(moduleBarKey), + + "module.bar[0].foo.mid": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "mid", + }.Instance(addrs.NoKey).Absolute(moduleBarKey), + + "module.bar[0].foo.to": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "to", + }.Instance(addrs.NoKey).Absolute(moduleBarKey), + + "module.bar[0].foo.from[0]": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "from", + }.Instance(addrs.IntKey(0)).Absolute(moduleBarKey), + + "module.bar[0].foo.to[0]": addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "foo", + Name: "to", + }.Instance(addrs.IntKey(0)).Absolute(moduleBarKey), } emptyResults := map[addrs.UniqueKey]MoveResult{} @@ -68,7 +134,7 @@ func TestApplyMoves(t *testing.T) { []MoveStatement{}, states.BuildState(func(s *states.SyncState) { s.SetResourceInstanceCurrent( - rootNoKeyResourceAddr[0], + instAddrs["foo.from"], &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte(`{}`), @@ -87,7 +153,7 @@ func TestApplyMoves(t *testing.T) { }, states.BuildState(func(s *states.SyncState) { s.SetResourceInstanceCurrent( - rootNoKeyResourceAddr[0], + instAddrs["foo.from"], &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte(`{}`), @@ -96,13 +162,13 @@ func TestApplyMoves(t *testing.T) { ) }), map[addrs.UniqueKey]MoveResult{ - rootNoKeyResourceAddr[0].UniqueKey(): { - From: rootNoKeyResourceAddr[0], - To: rootNoKeyResourceAddr[1], + instAddrs["foo.from"].UniqueKey(): { + From: instAddrs["foo.from"], + To: instAddrs["foo.to"], }, - rootNoKeyResourceAddr[1].UniqueKey(): { - From: rootNoKeyResourceAddr[0], - To: rootNoKeyResourceAddr[1], + instAddrs["foo.to"].UniqueKey(): { + From: instAddrs["foo.from"], + To: instAddrs["foo.to"], }, }, []string{ @@ -115,7 +181,7 @@ func TestApplyMoves(t *testing.T) { }, states.BuildState(func(s *states.SyncState) { s.SetResourceInstanceCurrent( - rootIntKeyResourceAddr[0], + instAddrs["foo.from[0]"], &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte(`{}`), @@ -124,13 +190,13 @@ func TestApplyMoves(t *testing.T) { ) }), map[addrs.UniqueKey]MoveResult{ - rootIntKeyResourceAddr[0].UniqueKey(): { - From: rootIntKeyResourceAddr[0], - To: rootIntKeyResourceAddr[1], + instAddrs["foo.from[0]"].UniqueKey(): { + From: instAddrs["foo.from[0]"], + To: instAddrs["foo.to[0]"], }, - rootIntKeyResourceAddr[1].UniqueKey(): { - From: rootIntKeyResourceAddr[0], - To: rootIntKeyResourceAddr[1], + instAddrs["foo.to[0]"].UniqueKey(): { + From: instAddrs["foo.from[0]"], + To: instAddrs["foo.to[0]"], }, }, []string{ @@ -144,7 +210,7 @@ func TestApplyMoves(t *testing.T) { }, states.BuildState(func(s *states.SyncState) { s.SetResourceInstanceCurrent( - rootNoKeyResourceAddr[0], + instAddrs["foo.from"], &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte(`{}`), @@ -153,23 +219,170 @@ func TestApplyMoves(t *testing.T) { ) }), map[addrs.UniqueKey]MoveResult{ - rootNoKeyResourceAddr[0].UniqueKey(): { - From: rootNoKeyResourceAddr[0], - To: rootNoKeyResourceAddr[2], + instAddrs["foo.from"].UniqueKey(): { + From: instAddrs["foo.from"], + To: instAddrs["foo.mid"], }, - rootNoKeyResourceAddr[2].UniqueKey(): { - From: rootNoKeyResourceAddr[2], - To: rootNoKeyResourceAddr[1], + instAddrs["foo.mid"].UniqueKey(): { + From: instAddrs["foo.mid"], + To: instAddrs["foo.to"], }, - rootNoKeyResourceAddr[1].UniqueKey(): { - From: rootNoKeyResourceAddr[2], - To: rootNoKeyResourceAddr[1], + instAddrs["foo.to"].UniqueKey(): { + From: instAddrs["foo.mid"], + To: instAddrs["foo.to"], }, }, []string{ `foo.to`, }, }, + + "move whole resource into module": { + []MoveStatement{ + testMoveStatement(t, "", "foo.from", "module.boo.foo.to"), + }, + states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + instAddrs["foo.from[0]"], + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{}`), + }, + providerAddr, + ) + }), + map[addrs.UniqueKey]MoveResult{ + instAddrs["foo.from[0]"].UniqueKey(): { + From: instAddrs["foo.from[0]"], + To: instAddrs["module.boo.foo.to[0]"], + }, + instAddrs["module.boo.foo.to[0]"].UniqueKey(): { + From: instAddrs["foo.from[0]"], + To: instAddrs["module.boo.foo.to[0]"], + }, + }, + []string{ + `module.boo.foo.to[0]`, + }, + }, + + "move resource instance between modules": { + []MoveStatement{ + testMoveStatement(t, "", "module.boo.foo.from[0]", "module.bar[0].foo.to[0]"), + }, + states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + instAddrs["module.boo.foo.from[0]"], + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{}`), + }, + providerAddr, + ) + }), + map[addrs.UniqueKey]MoveResult{ + instAddrs["module.boo.foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.to[0]"], + }, + instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.to[0]"], + }, + }, + []string{ + `module.bar[0].foo.to[0]`, + }, + }, + + "move whole single module to indexed module": { + []MoveStatement{ + testMoveStatement(t, "", "module.boo", "module.bar[0]"), + }, + states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + instAddrs["module.boo.foo.from[0]"], + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{}`), + }, + providerAddr, + ) + }), + map[addrs.UniqueKey]MoveResult{ + instAddrs["module.boo.foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.from[0]"], + }, + instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.from[0]"], + }, + }, + []string{ + `module.bar[0].foo.from[0]`, + }, + }, + + "move whole module to within indexed module and instance chained": { + []MoveStatement{ + testMoveStatement(t, "", "module.boo", "module.bar[0]"), + testMoveStatement(t, "module.bar[0]", "foo.from[0]", "foo.too[0]"), + }, + states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + instAddrs["module.boo.foo.from[0]"], + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{}`), + }, + providerAddr, + ) + }), + map[addrs.UniqueKey]MoveResult{ + instAddrs["module.boo.foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.from[0]"], + }, + instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.from[0]"], + }, + }, + []string{ + `module.bar[0].foo.from[0]`, + }, + }, + + "move instance to indexed module and instance chained": { + []MoveStatement{ + testMoveStatement(t, "", "module.boo.foo.from[0]", "module.bar[0].foo.from[0]"), + testMoveStatement(t, "module.bar[0]", "foo.from[0]", "foo.too[0]"), + }, + states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + instAddrs["module.boo.foo.from[0]"], + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{}`), + }, + providerAddr, + ) + }), + map[addrs.UniqueKey]MoveResult{ + instAddrs["module.boo.foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.from[0]"], + }, + instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): { + From: instAddrs["module.boo.foo.from[0]"], + To: instAddrs["module.bar[0].foo.from[0]"], + }, + }, + []string{ + `module.bar[0].foo.from[0]`, + }, + }, } for name, test := range tests {