resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

refactoring: ApplyMoves new return type 

When we originally stubbed ApplyMoves we didn't know yet how exactly we'd
be using the result, so we made it a double-indexed map allowing looking
up moves in both directions.

However, in practice we only actually need to look up old addresses by new
addresses, and so this commit first removes the double indexing so that
each move is only represented by one element in the map.

We also need to describe situations where a move was blocked, because in
a future commit we'll generate some warnings in those cases. Therefore
ApplyMoves now returns a MoveResults object which contains both a map of
changes and a map of blocks. The map of blocks isn't used yet as of this
commit, but we'll use it in a later commit to produce warnings within
the "terraform" package.
This commit is contained in:
Martin Atkins 2021-09-21 12:57:12 -07:00
parent d054102d38
commit 83f0376673
10 changed files with 321 additions and 180 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

143
internal/refactoring/move_execute.go
View File

@ -10,10 +10,6 @@ import (
"github.com/hashicorp/terraform/internal/states"
)
type MoveResult struct {
From, To addrs.AbsResourceInstance
}
// ApplyMoves modifies in-place the given state object so that any existing
// objects that are matched by a "from" argument of one of the move statements
// will be moved to instead appear at the "to" argument of that statement.
@ -29,8 +25,11 @@ 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)
func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
ret := MoveResults{
Changes: make(map[addrs.UniqueKey]MoveSuccess),
Blocked: make(map[addrs.UniqueKey]MoveBlocked),
}
// The methodology here is to construct a small graph of all of the move
// statements where the edges represent where a particular statement
@ -44,7 +43,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 results
return ret
}
// The starting nodes are the ones that don't depend on any other nodes.
@ -57,11 +56,33 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
if startNodes.Len() == 0 {
log.Println("[TRACE] refactoring.ApplyMoves: No 'moved' statements to consider in this configuration")
return results
return ret
}
log.Printf("[TRACE] refactoring.ApplyMoves: Processing 'moved' statements in the configuration\n%s", logging.Indent(g.String()))
recordOldAddr := func(oldAddr, newAddr addrs.AbsResourceInstance) {
oldAddrKey := oldAddr.UniqueKey()
newAddrKey := newAddr.UniqueKey()
if prevMove, exists := ret.Changes[oldAddrKey]; exists {
// If the old address was _already_ the result of a move then
// we'll replace that entry so that our results summarize a chain
// of moves into a single entry.
delete(ret.Changes, oldAddrKey)
oldAddr = prevMove.From
}
ret.Changes[newAddrKey] = MoveSuccess{
From: oldAddr,
To: newAddr,
}
}
recordBlockage := func(newAddr, wantedAddr addrs.AbsMoveable) {
ret.Blocked[newAddr.UniqueKey()] = MoveBlocked{
Wanted: wantedAddr,
Actual: newAddr,
}
}
g.ReverseDepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error {
stmt := v.(*MoveStatement)
@ -83,11 +104,9 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
// If we already have a module at the new address then
// we'll skip this move and let the existing object take
// priority.
// TODO: This should probably generate a user-visible
// warning, but we'd need to rethink the signature of this
// function to achieve that.
if ms := state.Module(newAddr); ms != nil {
log.Printf("[WARN] Skipped moving %s to %s, because there's already another module instance at the destination", modAddr, newAddr)
recordBlockage(modAddr, newAddr)
continue
}
@ -98,12 +117,7 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
for key := range rs.Instances {
oldInst := relAddr.Instance(key).Absolute(modAddr)
newInst := relAddr.Instance(key).Absolute(newAddr)
result := MoveResult{
From: oldInst,
To: newInst,
}
results[oldInst.UniqueKey()] = result
results[newInst.UniqueKey()] = result
recordOldAddr(oldInst, newInst)
}
}
@ -121,23 +135,16 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
// If we already have a resource at the new address then
// we'll skip this move and let the existing object take
// priority.
// TODO: This should probably generate a user-visible
// warning, but we'd need to rethink the signature of this
// function to achieve that.
if rs := state.Resource(newAddr); rs != nil {
log.Printf("[WARN] Skipped moving %s to %s, because there's already another resource at the destination", rAddr, newAddr)
recordBlockage(rAddr, newAddr)
continue
}
for key := range rs.Instances {
oldInst := rAddr.Instance(key)
newInst := newAddr.Instance(key)
result := MoveResult{
From: oldInst,
To: newInst,
}
results[oldInst.UniqueKey()] = result
results[newInst.UniqueKey()] = result
recordOldAddr(oldInst, newInst)
}
state.MoveAbsResource(rAddr, newAddr)
continue
@ -150,17 +157,13 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
// If we already have a resource instance at the new
// address then we'll skip this move and let the existing
// object take priority.
// TODO: This should probably generate a user-visible
// warning, but we'd need to rethink the signature of this
// function to achieve that.
if is := state.ResourceInstance(newAddr); is != nil {
log.Printf("[WARN] Skipped moving %s to %s, because there's already another resource instance at the destination", iAddr, newAddr)
recordBlockage(iAddr, newAddr)
continue
}
result := MoveResult{From: iAddr, To: newAddr}
results[iAddr.UniqueKey()] = result
results[newAddr.UniqueKey()] = result
recordOldAddr(iAddr, newAddr)
state.MoveAbsResourceInstance(iAddr, newAddr)
continue
@ -175,17 +178,7 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
return nil
})
// FIXME: In the case of either chained or nested moves, "results" will
// be left in a pretty interesting shape where the "old" address will
// refer to a result that describes only the first step, while the "new"
// address will refer to a result that describes only the last step.
// To make that actually useful we'll need a different strategy where
// the result describes the _effective_ source and destination, skipping
// over any intermediate steps we took to get there, so that ultimately
// we'll have enough information to annotate items in the plan with the
// addresses the originally moved from.
return results
return ret
}
// buildMoveStatementGraph constructs a dependency graph of the given move
@ -218,3 +211,67 @@ func buildMoveStatementGraph(stmts []MoveStatement) *dag.AcyclicGraph {
return g
}
// MoveResults describes the outcome of an ApplyMoves call.
type MoveResults struct {
// Changes is a map from the unique keys of the final new resource
// instance addresses to an object describing what changed.
//
// This includes one entry for each resource instance address that was
// the destination of a move statement. It doesn't include resource
// instances that were not affected by moves at all, but it does include
// resource instance addresses that were "blocked" (also recorded in
// BlockedAddrs) if and only if they were able to move at least
// partially along a chain before being blocked.
//
// In the return value from ApplyMoves, all of the keys are guaranteed to
// be unique keys derived from addrs.AbsResourceInstance values.
Changes map[addrs.UniqueKey]MoveSuccess
// Blocked is a map from the unique keys of the final new
// resource instances addresses to information about where they "wanted"
// to move, but were blocked by a pre-existing object at the same address.
//
// "Blocking" can arise in unusual situations where multiple points along
// a move chain were already bound to objects, and thus only one of them
// can actually adopt the final position in the chain. It can also
// occur in other similar situations, such as if a configuration contains
// a move of an entire module and a move of an individual resource into
// that module, such that the individual resource would collide with a
// resource in the whole module that was moved.
//
// In the return value from ApplyMoves, all of the keys are guaranteed to
// be unique keys derived from values of addrs.AbsMoveable types.
Blocked map[addrs.UniqueKey]MoveBlocked
}
type MoveSuccess struct {
From addrs.AbsResourceInstance
To addrs.AbsResourceInstance
}
type MoveBlocked struct {
Wanted addrs.AbsMoveable
Actual addrs.AbsMoveable
}
// AddrMoved returns true if and only if the given resource instance moved to
// a new address in the ApplyMoves call that the receiver is describing.
//
// If AddrMoved returns true, you can pass the same address to method OldAddr
// to find its original address prior to moving.
func (rs MoveResults) AddrMoved(newAddr addrs.AbsResourceInstance) bool {
_, ok := rs.Changes[newAddr.UniqueKey()]
return ok
}
// OldAddr returns the old address of the given resource instance address, or
// just returns back the same address if the given instance wasn't affected by
// any move statements.
func (rs MoveResults) OldAddr(newAddr addrs.AbsResourceInstance) addrs.AbsResourceInstance {
change, ok := rs.Changes[newAddr.UniqueKey()]
if !ok {
return newAddr
}
return change.From
}

288
internal/refactoring/move_execute_test.go
View File

@ -115,13 +115,16 @@ func TestApplyMoves(t *testing.T) {
}.Instance(addrs.IntKey(0)).Absolute(moduleBarKey),
}
emptyResults := map[addrs.UniqueKey]MoveResult{}
emptyResults := MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
}
tests := map[string]struct {
Stmts []MoveStatement
State *states.State
WantResults map[addrs.UniqueKey]MoveResult
WantResults MoveResults
WantInstanceAddrs []string
}{
"no moves and empty state": {
@ -161,15 +164,14 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
}),
map[addrs.UniqueKey]MoveResult{
instAddrs["foo.from"].UniqueKey(): {
From: instAddrs["foo.from"],
To: instAddrs["foo.to"],
},
instAddrs["foo.to"].UniqueKey(): {
From: instAddrs["foo.from"],
To: instAddrs["foo.to"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["foo.to"].UniqueKey(): {
From: instAddrs["foo.from"],
To: instAddrs["foo.to"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`foo.to`,
@ -189,15 +191,14 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
}),
map[addrs.UniqueKey]MoveResult{
instAddrs["foo.from[0]"].UniqueKey(): {
From: instAddrs["foo.from[0]"],
To: instAddrs["foo.to[0]"],
},
instAddrs["foo.to[0]"].UniqueKey(): {
From: instAddrs["foo.from[0]"],
To: instAddrs["foo.to[0]"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["foo.to[0]"].UniqueKey(): {
From: instAddrs["foo.from[0]"],
To: instAddrs["foo.to[0]"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`foo.to[0]`,
@ -218,19 +219,14 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
}),
map[addrs.UniqueKey]MoveResult{
instAddrs["foo.from"].UniqueKey(): {
From: instAddrs["foo.from"],
To: instAddrs["foo.mid"],
},
instAddrs["foo.mid"].UniqueKey(): {
From: instAddrs["foo.mid"],
To: instAddrs["foo.to"],
},
instAddrs["foo.to"].UniqueKey(): {
From: instAddrs["foo.mid"],
To: instAddrs["foo.to"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["foo.to"].UniqueKey(): {
From: instAddrs["foo.from"],
To: instAddrs["foo.to"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`foo.to`,
@ -251,15 +247,14 @@ func TestApplyMoves(t *testing.T) {
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]"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["module.boo.foo.to[0]"].UniqueKey(): {
From: instAddrs["foo.from[0]"],
To: instAddrs["module.boo.foo.to[0]"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`module.boo.foo.to[0]`,
@ -280,15 +275,14 @@ func TestApplyMoves(t *testing.T) {
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]"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): {
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`module.bar[0].foo.to[0]`,
@ -309,15 +303,14 @@ func TestApplyMoves(t *testing.T) {
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]"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): {
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.from[0]"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`module.bar[0].foo.from[0]`,
@ -339,19 +332,14 @@ func TestApplyMoves(t *testing.T) {
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.bar[0].foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
},
instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): {
From: instAddrs["module.bar[0].foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): {
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`module.bar[0].foo.to[0]`,
@ -373,19 +361,14 @@ func TestApplyMoves(t *testing.T) {
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.bar[0].foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
},
instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): {
From: instAddrs["module.bar[0].foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
MoveResults{
Changes: map[addrs.UniqueKey]MoveSuccess{
instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): {
From: instAddrs["module.boo.foo.from[0]"],
To: instAddrs["module.bar[0].foo.to[0]"],
},
},
Blocked: map[addrs.UniqueKey]MoveBlocked{},
},
[]string{
`module.bar[0].foo.to[0]`,
@ -414,9 +397,16 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
}),
map[addrs.UniqueKey]MoveResult{
MoveResults{
// Nothing moved, because the module.b address is already
// occupied by another module.
Changes: map[addrs.UniqueKey]MoveSuccess{},
Blocked: map[addrs.UniqueKey]MoveBlocked{
instAddrs["module.bar[0].foo.from"].Module.UniqueKey(): {
Wanted: instAddrs["module.boo.foo.to[0]"].Module,
Actual: instAddrs["module.bar[0].foo.from"].Module,
},
},
},
[]string{
`module.bar[0].foo.from`,
@ -446,9 +436,16 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
}),
map[addrs.UniqueKey]MoveResult{
// Nothing moved, because the module.b address is already
// occupied by another module.
MoveResults{
// Nothing moved, because the from.to address is already
// occupied by another resource.
Changes: map[addrs.UniqueKey]MoveSuccess{},
Blocked: map[addrs.UniqueKey]MoveBlocked{
instAddrs["foo.from"].ContainingResource().UniqueKey(): {
Wanted: instAddrs["foo.to"].ContainingResource(),
Actual: instAddrs["foo.from"].ContainingResource(),
},
},
},
[]string{
`foo.from`,
@ -478,22 +475,141 @@ func TestApplyMoves(t *testing.T) {
providerAddr,
)
}),
map[addrs.UniqueKey]MoveResult{
// Nothing moved, because the module.b address is already
// occupied by another module.
MoveResults{
// Nothing moved, because the from.to[0] address is already
// occupied by another resource instance.
Changes: map[addrs.UniqueKey]MoveSuccess{},
Blocked: map[addrs.UniqueKey]MoveBlocked{
instAddrs["foo.from"].UniqueKey(): {
Wanted: instAddrs["foo.to[0]"],
Actual: instAddrs["foo.from"],
},
},
},
[]string{
`foo.from`,
`foo.to[0]`,
},
},
// FIXME: This test seems to flap between the result the test case
// currently records and the "more intuitive" results included inline,
// which suggests we have a missing edge in our move dependency graph.
// (The MoveResults commented out below predates some changes to that
// struct, so will need updating once we uncomment this test.)
/*
"move module and then move resource into it": {
[]MoveStatement{
testMoveStatement(t, "", "module.bar[0]", "module.boo"),
testMoveStatement(t, "", "foo.from", "module.boo.foo.from"),
},
states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
instAddrs["module.bar[0].foo.to"],
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
},
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
},
providerAddr,
)
}),
MoveResults{
// FIXME: This result is counter-intuitive, because ApplyMoves
// handled the resource move first and then the module move
// collided with it. It would be arguably more intuitive to
// complete the module move first and then let the "smaller"
// resource move merge into it.
// (The arguably-more-intuitive results are commented out
// in the maps below.)
Changes: map[addrs.UniqueKey]MoveSuccess{
//instAddrs["module.boo.foo.to"].UniqueKey(): instAddrs["module.bar[0].foo.to"],
//instAddrs["module.boo.foo.from"].UniqueKey(): instAddrs["foo.from"],
instAddrs["module.boo.foo.from"].UniqueKey(): instAddrs["foo.from"],
},
Blocked: map[addrs.UniqueKey]MoveBlocked{
// intuitive result: nothing blocked
instAddrs["module.bar[0].foo.to"].Module.UniqueKey(): instAddrs["module.boo.foo.from"].Module,
},
},
[]string{
//`foo.from`,
//`module.boo.foo.from`,
`module.bar[0].foo.to`,
`module.boo.foo.from`,
},
},
*/
// FIXME: This test seems to flap between the result the test case
// currently records and the "more intuitive" results included inline,
// which suggests we have a missing edge in our move dependency graph.
// (The MoveResults commented out below predates some changes to that
// struct, so will need updating once we uncomment this test.)
/*
"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(
instAddrs["module.bar[0].foo.from"],
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
},
providerAddr,
)
s.SetResourceInstanceCurrent(
instAddrs["foo.from"],
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
},
providerAddr,
)
}),
MoveResults{
// FIXME: This result is counter-intuitive, because ApplyMoves
// handled the resource move first and then it was the
// module move that collided, whereas it would arguably have
// been better to let the module take priority and have only
// the one resource move be ignored due to the collision.
// (The arguably-more-intuitive results are commented out
// in the maps below.)
Changes: map[addrs.UniqueKey]MoveSuccess{
//instAddrs["module.boo.foo.from"].UniqueKey(): instAddrs["module.bar[0].foo.from"],
instAddrs["module.boo.foo.from"].UniqueKey(): instAddrs["foo.from"],
},
Blocked: map[addrs.UniqueKey]MoveBlocked{
//instAddrs["foo.from"].UniqueKey(): instAddrs["module.bar[0].foo.from"],
instAddrs["module.bar[0].foo.from"].Module.UniqueKey(): instAddrs["module.boo.foo.from"].Module,
},
},
[]string{
//`foo.from`,
//`module.boo.foo.from`,
`module.bar[0].foo.from`,
`module.boo.foo.from`,
},
},
*/
}
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\n", 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())

23
internal/refactoring/move_validate_test.go
View File

@ -325,29 +325,6 @@ Each resource can have moved from only one source resource.`,
Each resource can have moved from only one source resource.`,
},
/*
// FIXME: This rule requires a deeper analysis to understand that
// module.single already contains a test.single and thus moving
// it to module.foo implicitly also moves module.single.test.single
// module.foo.test.single.
"two different moves to nested test.single by different paths": {
Statements: []MoveStatement{
makeTestMoveStmt(t,
``,
`test.beep`,
`module.foo.test.single`,
),
makeTestMoveStmt(t,
``,
`module.single`,
`module.foo`,
),
},
WantError: `Ambiguous move statements: A statement at test:1,1 declared that test.beep moved to module.foo.test.single, but this statement instead declares that module.single.test.single moved there.
Each resource can have moved from only one source resource.`,
},
*/
"move from resource in another module package": {
Statements: []MoveStatement{
makeTestMoveStmt(t,

12
internal/terraform/context_plan.go
View File

@ -296,7 +296,7 @@ func (c *Context) destroyPlan(config *configs.Config, prevRunState *states.State
return destroyPlan, diags
}
func (c *Context) prePlanFindAndApplyMoves(config *configs.Config, prevRunState *states.State, targets []addrs.Targetable) ([]refactoring.MoveStatement, map[addrs.UniqueKey]refactoring.MoveResult) {
func (c *Context) prePlanFindAndApplyMoves(config *configs.Config, prevRunState *states.State, targets []addrs.Targetable) ([]refactoring.MoveStatement, refactoring.MoveResults) {
explicitMoveStmts := refactoring.FindMoveStatements(config)
implicitMoveStmts := refactoring.ImpliedMoveStatements(config, prevRunState, explicitMoveStmts)
var moveStmts []refactoring.MoveStatement
@ -309,7 +309,7 @@ func (c *Context) prePlanFindAndApplyMoves(config *configs.Config, prevRunState
return moveStmts, moveResults
}
func (c *Context) prePlanVerifyTargetedMoves(moveResults map[addrs.UniqueKey]refactoring.MoveResult, targets []addrs.Targetable) tfdiags.Diagnostics {
func (c *Context) prePlanVerifyTargetedMoves(moveResults refactoring.MoveResults, targets []addrs.Targetable) tfdiags.Diagnostics {
if len(targets) < 1 {
return nil // the following only matters when targeting
}
@ -317,7 +317,7 @@ func (c *Context) prePlanVerifyTargetedMoves(moveResults map[addrs.UniqueKey]ref
var diags tfdiags.Diagnostics
var excluded []addrs.AbsResourceInstance
for _, result := range moveResults {
for _, result := range moveResults.Changes {
fromMatchesTarget := false
toMatchesTarget := false
for _, targetAddr := range targets {
@ -475,10 +475,10 @@ func (c *Context) planGraph(config *configs.Config, prevRunState *states.State,
}
}
func (c *Context) driftedResources(config *configs.Config, oldState, newState *states.State, moves map[addrs.UniqueKey]refactoring.MoveResult) ([]*plans.ResourceInstanceChangeSrc, tfdiags.Diagnostics) {
func (c *Context) driftedResources(config *configs.Config, oldState, newState *states.State, moves refactoring.MoveResults) ([]*plans.ResourceInstanceChangeSrc, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
if newState.ManagedResourcesEqual(oldState) && len(moves) == 0 {
if newState.ManagedResourcesEqual(oldState) && len(moves.Changes) == 0 {
// Nothing to do, because we only detect and report drift for managed
// resource instances.
return nil, diags
@ -510,7 +510,7 @@ func (c *Context) driftedResources(config *configs.Config, oldState, newState *s
// Previous run address defaults to the current address, but
// can differ if the resource moved before refreshing
prevRunAddr := addr
if move, ok := moves[addr.UniqueKey()]; ok {
if move, ok := moves.Changes[addr.UniqueKey()]; ok {
prevRunAddr = move.From
}

3
internal/terraform/context_walk.go
View File

@ -3,7 +3,6 @@ package terraform
import (
"log"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/plans"
@ -25,7 +24,7 @@ type graphWalkOpts struct {
Config *configs.Config
RootVariableValues InputValues
MoveResults map[addrs.UniqueKey]refactoring.MoveResult
MoveResults refactoring.MoveResults
}
func (c *Context) walk(graph *Graph, operation walkOperation, opts *graphWalkOpts) (*ContextGraphWalker, tfdiags.Diagnostics) {

2
internal/terraform/eval_context.go
View File

@ -174,7 +174,7 @@ type EvalContext interface {
// This data structure is created prior to the graph walk and read-only
// thereafter, so callers must not modify the returned map or any other
// objects accessible through it.
MoveResults() map[addrs.UniqueKey]refactoring.MoveResult
MoveResults() refactoring.MoveResults
// WithPath returns a copy of the context with the internal path set to the
// path argument.

4
internal/terraform/eval_context_builtin.go
View File

@ -69,7 +69,7 @@ type BuiltinEvalContext struct {
RefreshStateValue *states.SyncState
PrevRunStateValue *states.SyncState
InstanceExpanderValue *instances.Expander
MoveResultsValue map[addrs.UniqueKey]refactoring.MoveResult
MoveResultsValue refactoring.MoveResults
}
// BuiltinEvalContext implements EvalContext
@ -368,6 +368,6 @@ func (ctx *BuiltinEvalContext) InstanceExpander() *instances.Expander {
return ctx.InstanceExpanderValue
}
func (ctx *BuiltinEvalContext) MoveResults() map[addrs.UniqueKey]refactoring.MoveResult {
func (ctx *BuiltinEvalContext) MoveResults() refactoring.MoveResults {
return ctx.MoveResultsValue
}

4
internal/terraform/eval_context_mock.go
View File

@ -132,7 +132,7 @@ type MockEvalContext struct {
PrevRunStateState *states.SyncState
MoveResultsCalled bool
MoveResultsResults map[addrs.UniqueKey]refactoring.MoveResult
MoveResultsResults refactoring.MoveResults
InstanceExpanderCalled bool
InstanceExpanderExpander *instances.Expander
@ -353,7 +353,7 @@ func (c *MockEvalContext) PrevRunState() *states.SyncState {
return c.PrevRunStateState
}
func (c *MockEvalContext) MoveResults() map[addrs.UniqueKey]refactoring.MoveResult {
func (c *MockEvalContext) MoveResults() refactoring.MoveResults {
c.MoveResultsCalled = true
return c.MoveResultsResults
}

12
internal/terraform/graph_walk_context.go
View File

@ -25,12 +25,12 @@ type ContextGraphWalker struct {
// Configurable values
Context *Context
State *states.SyncState // Used for safe concurrent access to state
RefreshState *states.SyncState // Used for safe concurrent access to state
PrevRunState *states.SyncState // Used for safe concurrent access to state
Changes *plans.ChangesSync // Used for safe concurrent writes to changes
InstanceExpander *instances.Expander // Tracks our gradual expansion of module and resource instances
MoveResults map[addrs.UniqueKey]refactoring.MoveResult // Read-only record of earlier processing of move statements
State *states.SyncState // Used for safe concurrent access to state
RefreshState *states.SyncState // Used for safe concurrent access to state
PrevRunState *states.SyncState // Used for safe concurrent access to state
Changes *plans.ChangesSync // Used for safe concurrent writes to changes
InstanceExpander *instances.Expander // Tracks our gradual expansion of module and resource instances
MoveResults refactoring.MoveResults // Read-only record of earlier processing of move statements
Operation walkOperation
StopContext context.Context
RootVariableValues InputValues

10
internal/terraform/node_resource_abstract_instance.go
View File

@ -2282,13 +2282,5 @@ func (n *NodeAbstractResourceInstance) prevRunAddr(ctx EvalContext) addrs.AbsRes
func resourceInstancePrevRunAddr(ctx EvalContext, currentAddr addrs.AbsResourceInstance) addrs.AbsResourceInstance {
table := ctx.MoveResults()
result, ok := table[currentAddr.UniqueKey()]
if !ok {
// If there's no entry in the table then we'll assume it didn't move
// at all, and so its previous address is the same as the current one.
return currentAddr
}
return result.From
return table.OldAddr(currentAddr)
}