IntenseWebs/opentofu
3
0
Fork 0
mirror of https://github.com/opentofu/opentofu.git synced 2025-02-25 18:45:20 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #31499 from hashicorp/jbardin/dag-walks

Browse Source 
topological sorting for refactoring
This commit is contained in:
James Bardin 2022-07-22 15:41:37 -04:00 committed by GitHub
commit 8b1b05f50f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 417 additions and 272 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

208
internal/dag/dag.go
View File

@ -2,6 +2,7 @@ package dag
import (
"fmt"
"sort"
"strings"
"github.com/hashicorp/terraform/internal/tfdiags"
@ -178,65 +179,135 @@ type vertexAtDepth struct {
Depth int
}
// TopologicalOrder returns a topological sort of the given graph. The nodes
// are not sorted, and any valid order may be returned. This function will
// panic if it encounters a cycle.
func (g *AcyclicGraph) TopologicalOrder() []Vertex {
return g.topoOrder(upOrder)
}
// ReverseTopologicalOrder returns a topological sort of the given graph,
// following each edge in reverse. The nodes are not sorted, and any valid
// order may be returned. This function will panic if it encounters a cycle.
func (g *AcyclicGraph) ReverseTopologicalOrder() []Vertex {
return g.topoOrder(downOrder)
}
func (g *AcyclicGraph) topoOrder(order walkType) []Vertex {
// Use a dfs-based sorting algorithm, similar to that used in
// TransitiveReduction.
sorted := make([]Vertex, 0, len(g.vertices))
// tmp track the current working node to check for cycles
tmp := map[Vertex]bool{}
// perm tracks completed nodes to end the recursion
perm := map[Vertex]bool{}
var visit func(v Vertex)
visit = func(v Vertex) {
if perm[v] {
return
}
if tmp[v] {
panic("cycle found in dag")
}
tmp[v] = true
var next Set
switch {
case order&downOrder != 0:
next = g.downEdgesNoCopy(v)
case order&upOrder != 0:
next = g.upEdgesNoCopy(v)
default:
panic(fmt.Sprintln("invalid order", order))
}
for _, u := range next {
visit(u)
}
tmp[v] = false
perm[v] = true
sorted = append(sorted, v)
}
for _, v := range g.Vertices() {
visit(v)
}
return sorted
}
type walkType uint64
const (
depthFirst walkType = 1 << iota
breadthFirst
downOrder
upOrder
)
// DepthFirstWalk does a depth-first walk of the graph starting from
// the vertices in start.
// The algorithm used here does not do a complete topological sort. To ensure
// correct overall ordering run TransitiveReduction first.
func (g *AcyclicGraph) DepthFirstWalk(start Set, f DepthWalkFunc) error {
seen := make(map[Vertex]struct{})
frontier := make([]*vertexAtDepth, 0, len(start))
for _, v := range start {
frontier = append(frontier, &vertexAtDepth{
Vertex: v,
Depth: 0,
})
}
for len(frontier) > 0 {
// Pop the current vertex
n := len(frontier)
current := frontier[n-1]
frontier = frontier[:n-1]
// Check if we've seen this already and return...
if _, ok := seen[current.Vertex]; ok {
continue
}
seen[current.Vertex] = struct{}{}
// Visit the current node
if err := f(current.Vertex, current.Depth); err != nil {
return err
}
for _, v := range g.downEdgesNoCopy(current.Vertex) {
frontier = append(frontier, &vertexAtDepth{
Vertex: v,
Depth: current.Depth + 1,
})
}
}
return nil
return g.walk(depthFirst|downOrder, false, start, f)
}
// ReverseDepthFirstWalk does a depth-first walk _up_ the graph starting from
// the vertices in start.
// The algorithm used here does not do a complete topological sort. To ensure
// correct overall ordering run TransitiveReduction first.
func (g *AcyclicGraph) ReverseDepthFirstWalk(start Set, f DepthWalkFunc) error {
return g.walk(depthFirst|upOrder, false, start, f)
}
// BreadthFirstWalk does a breadth-first walk of the graph starting from
// the vertices in start.
func (g *AcyclicGraph) BreadthFirstWalk(start Set, f DepthWalkFunc) error {
return g.walk(breadthFirst|downOrder, false, start, f)
}
// ReverseBreadthFirstWalk does a breadth-first walk _up_ the graph starting from
// the vertices in start.
func (g *AcyclicGraph) ReverseBreadthFirstWalk(start Set, f DepthWalkFunc) error {
return g.walk(breadthFirst|upOrder, false, start, f)
}
// Setting test to true will walk sets of vertices in sorted order for
// deterministic testing.
func (g *AcyclicGraph) walk(order walkType, test bool, start Set, f DepthWalkFunc) error {
seen := make(map[Vertex]struct{})
frontier := make([]*vertexAtDepth, 0, len(start))
frontier := make([]vertexAtDepth, 0, len(start))
for _, v := range start {
frontier = append(frontier, &vertexAtDepth{
frontier = append(frontier, vertexAtDepth{
Vertex: v,
Depth: 0,
})
}
if test {
testSortFrontier(frontier)
}
for len(frontier) > 0 {
// Pop the current vertex
n := len(frontier)
current := frontier[n-1]
frontier = frontier[:n-1]
var current vertexAtDepth
switch {
case order&depthFirst != 0:
// depth first, the frontier is used like a stack
n := len(frontier)
current = frontier[n-1]
frontier = frontier[:n-1]
case order&breadthFirst != 0:
// breadth first, the frontier is used like a queue
current = frontier[0]
frontier = frontier[1:]
default:
panic(fmt.Sprint("invalid visit order", order))
}
// Check if we've seen this already and return...
if _, ok := seen[current.Vertex]; ok {
@ -244,18 +315,53 @@ func (g *AcyclicGraph) ReverseDepthFirstWalk(start Set, f DepthWalkFunc) error {
}
seen[current.Vertex] = struct{}{}
for _, t := range g.upEdgesNoCopy(current.Vertex) {
frontier = append(frontier, &vertexAtDepth{
Vertex: t,
Depth: current.Depth + 1,
})
}
// Visit the current node
if err := f(current.Vertex, current.Depth); err != nil {
return err
}
}
var edges Set
switch {
case order&downOrder != 0:
edges = g.downEdgesNoCopy(current.Vertex)
case order&upOrder != 0:
edges = g.upEdgesNoCopy(current.Vertex)
default:
panic(fmt.Sprint("invalid walk order", order))
}
if test {
frontier = testAppendNextSorted(frontier, edges, current.Depth+1)
} else {
frontier = appendNext(frontier, edges, current.Depth+1)
}
}
return nil
}
func appendNext(frontier []vertexAtDepth, next Set, depth int) []vertexAtDepth {
for _, v := range next {
frontier = append(frontier, vertexAtDepth{
Vertex: v,
Depth: depth,
})
}
return frontier
}
func testAppendNextSorted(frontier []vertexAtDepth, edges Set, depth int) []vertexAtDepth {
var newEdges []vertexAtDepth
for _, v := range edges {
newEdges = append(newEdges, vertexAtDepth{
Vertex: v,
Depth: depth,
})
}
testSortFrontier(newEdges)
return append(frontier, newEdges...)
}
func testSortFrontier(f []vertexAtDepth) {
sort.Slice(f, func(i, j int) bool {
return VertexName(f[i].Vertex) < VertexName(f[j].Vertex)
})
}

148
internal/dag/dag_test.go
View File

@ -414,34 +414,136 @@ func BenchmarkDAG(b *testing.B) {
}
}
func TestAcyclicGraph_ReverseDepthFirstWalk_WithRemoval(t *testing.T) {
func TestAcyclicGraphWalkOrder(t *testing.T) {
/* Sample dependency graph,
all edges pointing downwards.
1 2
/ \ / \
3 4 5
/ \ /
6 7
/ | \
8 9 10
\ | /
11
*/
var g AcyclicGraph
g.Add(1)
g.Add(2)
g.Add(3)
g.Connect(BasicEdge(3, 2))
g.Connect(BasicEdge(2, 1))
for i := 1; i <= 11; i++ {
g.Add(i)
}
g.Connect(BasicEdge(1, 3))
g.Connect(BasicEdge(1, 4))
g.Connect(BasicEdge(2, 4))
g.Connect(BasicEdge(2, 5))
g.Connect(BasicEdge(3, 6))
g.Connect(BasicEdge(4, 7))
g.Connect(BasicEdge(5, 7))
g.Connect(BasicEdge(7, 8))
g.Connect(BasicEdge(7, 9))
g.Connect(BasicEdge(7, 10))
g.Connect(BasicEdge(8, 11))
g.Connect(BasicEdge(9, 11))
g.Connect(BasicEdge(10, 11))
var visits []Vertex
var lock sync.Mutex
root := make(Set)
root.Add(1)
start := make(Set)
start.Add(2)
start.Add(1)
reverse := make(Set)
reverse.Add(11)
reverse.Add(6)
err := g.ReverseDepthFirstWalk(root, func(v Vertex, d int) error {
lock.Lock()
defer lock.Unlock()
visits = append(visits, v)
g.Remove(v)
return nil
t.Run("DepthFirst", func(t *testing.T) {
var visits []vertexAtDepth
g.walk(depthFirst|downOrder, true, start, func(v Vertex, d int) error {
visits = append(visits, vertexAtDepth{v, d})
return nil
})
expect := []vertexAtDepth{
{2, 0}, {5, 1}, {7, 2}, {9, 3}, {11, 4}, {8, 3}, {10, 3}, {4, 1}, {1, 0}, {3, 1}, {6, 2},
}
if !reflect.DeepEqual(visits, expect) {
t.Errorf("expected visits:\n%v\ngot:\n%v\n", expect, visits)
}
})
if err != nil {
t.Fatalf("err: %s", err)
}
t.Run("ReverseDepthFirst", func(t *testing.T) {
var visits []vertexAtDepth
g.walk(depthFirst|upOrder, true, reverse, func(v Vertex, d int) error {
visits = append(visits, vertexAtDepth{v, d})
return nil
expected := []Vertex{1, 2, 3}
if !reflect.DeepEqual(visits, expected) {
t.Fatalf("expected: %#v, got: %#v", expected, visits)
}
})
expect := []vertexAtDepth{
{6, 0}, {3, 1}, {1, 2}, {11, 0}, {9, 1}, {7, 2}, {5, 3}, {2, 4}, {4, 3}, {8, 1}, {10, 1},
}
if !reflect.DeepEqual(visits, expect) {
t.Errorf("expected visits:\n%v\ngot:\n%v\n", expect, visits)
}
})
t.Run("BreadthFirst", func(t *testing.T) {
var visits []vertexAtDepth
g.walk(breadthFirst|downOrder, true, start, func(v Vertex, d int) error {
visits = append(visits, vertexAtDepth{v, d})
return nil
})
expect := []vertexAtDepth{
{1, 0}, {2, 0}, {3, 1}, {4, 1}, {5, 1}, {6, 2}, {7, 2}, {10, 3}, {8, 3}, {9, 3}, {11, 4},
}
if !reflect.DeepEqual(visits, expect) {
t.Errorf("expected visits:\n%v\ngot:\n%v\n", expect, visits)
}
})
t.Run("ReverseBreadthFirst", func(t *testing.T) {
var visits []vertexAtDepth
g.walk(breadthFirst|upOrder, true, reverse, func(v Vertex, d int) error {
visits = append(visits, vertexAtDepth{v, d})
return nil
})
expect := []vertexAtDepth{
{11, 0}, {6, 0}, {10, 1}, {8, 1}, {9, 1}, {3, 1}, {7, 2}, {1, 2}, {4, 3}, {5, 3}, {2, 4},
}
if !reflect.DeepEqual(visits, expect) {
t.Errorf("expected visits:\n%v\ngot:\n%v\n", expect, visits)
}
})
t.Run("TopologicalOrder", func(t *testing.T) {
order := g.topoOrder(downOrder)
// Validate the order by checking it against the initial graph. We only
// need to verify that each node has it's direct dependencies
// satisfied.
completed := map[Vertex]bool{}
for _, v := range order {
deps := g.DownEdges(v)
for _, dep := range deps {
if !completed[dep] {
t.Fatalf("walking node %v, but dependency %v was not yet seen", v, dep)
}
}
completed[v] = true
}
})
t.Run("ReverseTopologicalOrder", func(t *testing.T) {
order := g.topoOrder(upOrder)
// Validate the order by checking it against the initial graph. We only
// need to verify that each node has it's direct dependencies
// satisfied.
completed := map[Vertex]bool{}
for _, v := range order {
deps := g.UpEdges(v)
for _, dep := range deps {
if !completed[dep] {
t.Fatalf("walking node %v, but dependency %v was not yet seen", v, dep)
}
}
completed[v] = true
}
})
}
const testGraphTransReductionStr = `

6
internal/refactoring/move_execute.go
View File

@ -87,7 +87,7 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
})
}
g.ReverseDepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error {
for _, v := range g.ReverseTopologicalOrder() {
stmt := v.(*MoveStatement)
for _, ms := range state.Modules {
@ -182,9 +182,7 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
panic(fmt.Sprintf("unhandled move object kind %s", kind))
}
}
return nil
})
}
return ret
}

322
internal/refactoring/move_execute_test.go
View File

@ -10,6 +10,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/states"
)
@ -20,120 +21,12 @@ func TestApplyMoves(t *testing.T) {
Provider: addrs.MustParseProviderSourceString("example.com/foo/bar"),
}
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{
Mode: addrs.ManagedResourceMode,
Type: "foo",
Name: "from",
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
"foo.mid": addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "foo",
Name: "mid",
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
"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),
"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.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",
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),
"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),
mustParseInstAddr := func(s string) addrs.AbsResourceInstance {
addr, err := addrs.ParseAbsResourceInstanceStr(s)
if err != nil {
t.Fatal(err)
}
return addr
}
emptyResults := makeMoveResults()
@ -155,7 +48,7 @@ func TestApplyMoves(t *testing.T) {
[]MoveStatement{},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -174,7 +67,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -184,9 +77,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["foo.to"], MoveSuccess{
From: instAddrs["foo.from"],
To: instAddrs["foo.to"],
addrs.MakeMapElem(mustParseInstAddr("foo.to"), MoveSuccess{
From: mustParseInstAddr("foo.from"),
To: mustParseInstAddr("foo.to"),
}),
),
Blocked: emptyResults.Blocked,
@ -201,7 +94,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from[0]"],
mustParseInstAddr("foo.from[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -211,9 +104,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["foo.to[0]"], MoveSuccess{
From: instAddrs["foo.from[0]"],
To: instAddrs["foo.to[0]"],
addrs.MakeMapElem(mustParseInstAddr("foo.to[0]"), MoveSuccess{
From: mustParseInstAddr("foo.from[0]"),
To: mustParseInstAddr("foo.to[0]"),
}),
),
Blocked: emptyResults.Blocked,
@ -229,7 +122,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -239,9 +132,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["foo.to"], MoveSuccess{
From: instAddrs["foo.from"],
To: instAddrs["foo.to"],
addrs.MakeMapElem(mustParseInstAddr("foo.to"), MoveSuccess{
From: mustParseInstAddr("foo.from"),
To: mustParseInstAddr("foo.to"),
}),
),
Blocked: emptyResults.Blocked,
@ -257,7 +150,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from[0]"],
mustParseInstAddr("foo.from[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -267,9 +160,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.boo.foo.to[0]"], MoveSuccess{
From: instAddrs["foo.from[0]"],
To: instAddrs["module.boo.foo.to[0]"],
addrs.MakeMapElem(mustParseInstAddr("module.boo.foo.to[0]"), MoveSuccess{
From: mustParseInstAddr("foo.from[0]"),
To: mustParseInstAddr("module.boo.foo.to[0]"),
}),
),
Blocked: emptyResults.Blocked,
@ -285,7 +178,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.from[0]"],
mustParseInstAddr("module.boo.foo.from[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -295,9 +188,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.bar[0].foo.to[0]"], MoveSuccess{
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].foo.to[0]"), MoveSuccess{
From: mustParseInstAddr("module.boo.foo.from[0]"),
To: mustParseInstAddr("module.bar[0].foo.to[0]"),
}),
),
Blocked: emptyResults.Blocked,
@ -313,7 +206,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.from"],
mustParseInstAddr("module.boo.foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -321,7 +214,7 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["module.boo.module.hoo.foo.from"],
mustParseInstAddr("module.boo.module.hoo.foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -331,13 +224,13 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.bar.foo.from"], MoveSuccess{
From: instAddrs["module.boo.foo.from"],
To: instAddrs["module.bar.foo.from"],
addrs.MakeMapElem(mustParseInstAddr("module.bar.foo.from"), MoveSuccess{
From: mustParseInstAddr("module.boo.foo.from"),
To: mustParseInstAddr("module.bar.foo.from"),
}),
addrs.MakeMapElem(instAddrs["module.bar.module.hoo.foo.from"], MoveSuccess{
From: instAddrs["module.boo.module.hoo.foo.from"],
To: instAddrs["module.bar.module.hoo.foo.from"],
addrs.MakeMapElem(mustParseInstAddr("module.bar.module.hoo.foo.from"), MoveSuccess{
From: mustParseInstAddr("module.boo.module.hoo.foo.from"),
To: mustParseInstAddr("module.bar.module.hoo.foo.from"),
}),
),
Blocked: emptyResults.Blocked,
@ -354,7 +247,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.from[0]"],
mustParseInstAddr("module.boo.foo.from[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -364,9 +257,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.bar[0].foo.from[0]"], MoveSuccess{
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.from[0]"],
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].foo.from[0]"), MoveSuccess{
From: mustParseInstAddr("module.boo.foo.from[0]"),
To: mustParseInstAddr("module.bar[0].foo.from[0]"),
}),
),
Blocked: emptyResults.Blocked,
@ -383,7 +276,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.from[0]"],
mustParseInstAddr("module.boo.foo.from[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -393,9 +286,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.bar[0].foo.to[0]"], MoveSuccess{
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].foo.to[0]"), MoveSuccess{
From: mustParseInstAddr("module.boo.foo.from[0]"),
To: mustParseInstAddr("module.bar[0].foo.to[0]"),
}),
),
Blocked: emptyResults.Blocked,
@ -412,7 +305,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.from[0]"],
mustParseInstAddr("module.boo.foo.from[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -422,9 +315,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.bar[0].foo.to[0]"], MoveSuccess{
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].foo.to[0]"), MoveSuccess{
From: mustParseInstAddr("module.boo.foo.from[0]"),
To: mustParseInstAddr("module.bar[0].foo.to[0]"),
}),
),
Blocked: emptyResults.Blocked,
@ -440,7 +333,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.bar[0].foo.from"],
mustParseInstAddr("module.bar[0].foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -448,7 +341,7 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.to[0]"],
mustParseInstAddr("module.boo.foo.to[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -462,10 +355,10 @@ func TestApplyMoves(t *testing.T) {
Changes: emptyResults.Changes,
Blocked: addrs.MakeMap(
addrs.MakeMapElem[addrs.AbsMoveable](
instAddrs["module.bar[0].foo.from"].Module,
mustParseInstAddr("module.bar[0].foo.from").Module,
MoveBlocked{
Wanted: instAddrs["module.boo.foo.to[0]"].Module,
Actual: instAddrs["module.bar[0].foo.from"].Module,
Wanted: mustParseInstAddr("module.boo.foo.to[0]").Module,
Actual: mustParseInstAddr("module.bar[0].foo.from").Module,
},
),
),
@ -482,7 +375,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -490,7 +383,7 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["foo.to"],
mustParseInstAddr("foo.to"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -504,10 +397,10 @@ func TestApplyMoves(t *testing.T) {
Changes: emptyResults.Changes,
Blocked: addrs.MakeMap(
addrs.MakeMapElem[addrs.AbsMoveable](
instAddrs["foo.from"].ContainingResource(),
mustParseInstAddr("foo.from").ContainingResource(),
MoveBlocked{
Wanted: instAddrs["foo.to"].ContainingResource(),
Actual: instAddrs["foo.from"].ContainingResource(),
Wanted: mustParseInstAddr("foo.to").ContainingResource(),
Actual: mustParseInstAddr("foo.from").ContainingResource(),
},
),
),
@ -524,7 +417,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -532,7 +425,7 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["foo.to[0]"],
mustParseInstAddr("foo.to[0]"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -546,10 +439,10 @@ func TestApplyMoves(t *testing.T) {
Changes: emptyResults.Changes,
Blocked: addrs.MakeMap(
addrs.MakeMapElem[addrs.AbsMoveable](
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
MoveBlocked{
Wanted: instAddrs["foo.to[0]"],
Actual: instAddrs["foo.from"],
Wanted: mustParseInstAddr("foo.to[0]"),
Actual: mustParseInstAddr("foo.from"),
},
),
),
@ -566,7 +459,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.boo.foo.from"],
mustParseInstAddr("module.boo.foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -576,9 +469,9 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.bar[0].foo.to"], MoveSuccess{
From: instAddrs["module.boo.foo.from"],
To: instAddrs["module.bar[0].foo.to"],
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].foo.to"), MoveSuccess{
From: mustParseInstAddr("module.boo.foo.from"),
To: mustParseInstAddr("module.bar[0].foo.to"),
}),
),
Blocked: emptyResults.Blocked,
@ -595,7 +488,7 @@ func TestApplyMoves(t *testing.T) {
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.bar[0].foo.to"],
mustParseInstAddr("module.bar[0].foo.to"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -603,7 +496,7 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -613,13 +506,13 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.boo.foo.from"], MoveSuccess{
instAddrs["foo.from"],
instAddrs["module.boo.foo.from"],
addrs.MakeMapElem(mustParseInstAddr("module.boo.foo.from"), MoveSuccess{
mustParseInstAddr("foo.from"),
mustParseInstAddr("module.boo.foo.from"),
}),
addrs.MakeMapElem(instAddrs["module.boo.foo.to"], MoveSuccess{
instAddrs["module.bar[0].foo.to"],
instAddrs["module.boo.foo.to"],
addrs.MakeMapElem(mustParseInstAddr("module.boo.foo.to"), MoveSuccess{
mustParseInstAddr("module.bar[0].foo.to"),
mustParseInstAddr("module.boo.foo.to"),
}),
),
Blocked: emptyResults.Blocked,
@ -630,14 +523,15 @@ func TestApplyMoves(t *testing.T) {
},
},
"module move collides with resource move": {
"move resources into module and then move module": {
[]MoveStatement{
testMoveStatement(t, "", "module.bar[0]", "module.boo"),
testMoveStatement(t, "", "foo.from", "module.boo.foo.from"),
testMoveStatement(t, "", "foo.from", "module.boo.foo.to"),
testMoveStatement(t, "", "bar.from", "module.boo.bar.to"),
testMoveStatement(t, "", "module.boo", "module.bar[0]"),
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.bar[0].foo.from"],
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -645,7 +539,7 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
mustParseInstAddr("bar.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -655,17 +549,59 @@ func TestApplyMoves(t *testing.T) {
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(instAddrs["module.boo.foo.from"], MoveSuccess{
instAddrs["module.bar[0].foo.from"],
instAddrs["module.boo.foo.from"],
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].foo.to"), MoveSuccess{
mustParseInstAddr("foo.from"),
mustParseInstAddr("module.bar[0].foo.to"),
}),
addrs.MakeMapElem(mustParseInstAddr("module.bar[0].bar.to"), MoveSuccess{
mustParseInstAddr("bar.from"),
mustParseInstAddr("module.bar[0].bar.to"),
}),
),
Blocked: emptyResults.Blocked,
},
[]string{
`module.bar[0].bar.to`,
`module.bar[0].foo.to`,
},
},
"module move collides with resource move": {
[]MoveStatement{
testMoveStatement(t, "", "module.bar[0]", "module.boo"),
testMoveStatement(t, "", "foo.from", "module.boo.foo.from"),
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
mustParseInstAddr("module.bar[0].foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
},
providerAddr,
)
s.SetResourceInstanceCurrent(
mustParseInstAddr("foo.from"),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
},
providerAddr,
)
}),
MoveResults{
Changes: addrs.MakeMap(
addrs.MakeMapElem(mustParseInstAddr("module.boo.foo.from"), MoveSuccess{
mustParseInstAddr("module.bar[0].foo.from"),
mustParseInstAddr("module.boo.foo.from"),
}),
),
Blocked: addrs.MakeMap(
addrs.MakeMapElem[addrs.AbsMoveable](
instAddrs["foo.from"].ContainingResource(),
mustParseInstAddr("foo.from").ContainingResource(),
MoveBlocked{
Actual: instAddrs["foo.from"].ContainingResource(),
Wanted: instAddrs["module.boo.foo.from"].ContainingResource(),
Actual: mustParseInstAddr("foo.from").ContainingResource(),
Wanted: mustParseInstAddr("module.boo.foo.from").ContainingResource(),
},
),
),

1
internal/refactoring/move_statement_test.go
View File

@ -5,6 +5,7 @@ import (
"testing"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/states"
"github.com/hashicorp/terraform/internal/tfdiags"

1
internal/refactoring/move_validate.go
View File

@ -6,6 +6,7 @@ import (
"strings"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/dag"

3
internal/refactoring/move_validate_test.go
View File

@ -7,6 +7,8 @@ import (
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/zclconf/go-cty/cty/gocty"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/configs/configload"
@ -14,7 +16,6 @@ import (
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/registry"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty/gocty"
)
func TestValidateMoves(t *testing.T) {