From 5e86bab15979c6d1ec1a73ac7b3137d9cccf1985 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 27 Jul 2021 16:47:52 -0700 Subject: [PATCH] addrs: MoveDestination for AbsResource-based move endpoints Previously our MoveDestination methods only honored move statements whose endpoints were module calls or module instances. Now we'll additionally handle when the endpoints are whole resource addresses. This includes both renaming resource blocks and moving resource blocks into or out of child modules. This doesn't yet include endpoints that are specific resource _instances_, which will follow in a subsequent commit. For the moment that situation will always indicate a non-match. --- internal/addrs/move_endpoint_module.go | 50 +++++++- internal/addrs/move_endpoint_module_test.go | 122 +++++++++++++++++--- 2 files changed, 152 insertions(+), 20 deletions(-) diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index f1de3b8616..0615615f88 100644 --- a/internal/addrs/move_endpoint_module.go +++ b/internal/addrs/move_endpoint_module.go @@ -267,8 +267,54 @@ func (r AbsResource) MoveDestination(fromMatch, toMatch *MoveEndpointInModule) ( return r.Resource.Absolute(toMod), true case MoveEndpointResource: - // TODO: Implement - return AbsResource{}, false + fromRelSubject, ok := fromMatch.relSubject.(AbsResource) + if !ok { + // The only other possible type for a resource move is + // AbsResourceInstance, and that can never match an AbsResource. + return AbsResource{}, false + } + + // fromMatch can only possibly match the reciever if the resource + // portions are identical, regardless of the module paths. + if fromRelSubject.Resource != r.Resource { + return AbsResource{}, false + } + + // The module path portion of relSubject must have a prefix that + // matches the module where our endpoints were declared. + if len(fromMatch.module) > len(r.Module) { + return AbsResource{}, false // too short to possibly match + } + for i := range fromMatch.module { + if fromMatch.module[i] != r.Module[i].Name { + return AbsResource{}, false // this step doesn't match + } + } + + // The remaining steps of the module path must _exactly_ match + // the relative module path in the "fromMatch" address. + mPrefix, mRel := r.Module[:len(fromMatch.module)], r.Module[len(fromMatch.module):] + if len(mRel) != len(fromRelSubject.Module) { + return AbsResource{}, false // can't match if lengths are different + } + for i := range mRel { + if mRel[i] != fromRelSubject.Module[i] { + return AbsResource{}, false // all of the steps must match + } + } + + // If we got here then we have a match, and so our result is the + // module instance where the statement was declared (mPrefix) followed + // by the "to" relative address in toMatch. + toRelSubject := toMatch.relSubject.(AbsResource) + var mNew ModuleInstance + if len(mPrefix) > 0 || len(toRelSubject.Module) > 0 { + mNew = make(ModuleInstance, 0, len(mPrefix)+len(toRelSubject.Module)) + mNew = append(mNew, mPrefix...) + mNew = append(mNew, toRelSubject.Module...) + } + ret := toRelSubject.Resource.Absolute(mNew) + return ret, true default: panic("unexpected object kind") diff --git a/internal/addrs/move_endpoint_module_test.go b/internal/addrs/move_endpoint_module_test.go index 60f8044367..3e738bd195 100644 --- a/internal/addrs/move_endpoint_module_test.go +++ b/internal/addrs/move_endpoint_module_test.go @@ -14,7 +14,7 @@ func TestModuleInstanceMoveDestination(t *testing.T) { tests := []struct { DeclModule string StmtFrom, StmtTo string - Reciever string + Receiver string WantMatch bool WantResult string }{ @@ -242,7 +242,7 @@ func TestModuleInstanceMoveDestination(t *testing.T) { "%s: %s to %s with %s", test.DeclModule, test.StmtFrom, test.StmtTo, - test.Reciever, + test.Receiver, ), func(t *testing.T) { @@ -277,9 +277,9 @@ func TestModuleInstanceMoveDestination(t *testing.T) { } receiverAddr := RootModuleInstance - if test.Reciever != "" { + if test.Receiver != "" { var diags tfdiags.Diagnostics - receiverAddr, diags = ParseModuleInstanceStr(test.Reciever) + receiverAddr, diags = ParseModuleInstanceStr(test.Receiver) if diags.HasErrors() { t.Fatalf("invalid reciever address: %s", diags.Err().Error()) } @@ -287,13 +287,13 @@ func TestModuleInstanceMoveDestination(t *testing.T) { gotAddr, gotMatch := receiverAddr.MoveDestination(fromEP, toEP) if !test.WantMatch { if gotMatch { - t.Errorf("unexpected match\nreciever: %s\nfrom: %s\nto: %s\nresult: %s", test.Reciever, fromEP, toEP, gotAddr) + t.Errorf("unexpected match\nreceiver: %s\nfrom: %s\nto: %s\nresult: %s", test.Receiver, fromEP, toEP, gotAddr) } return } if !gotMatch { - t.Errorf("unexpected non-match\nreciever: %s\nfrom: %s\nto: %s", test.Reciever, fromEP, toEP) + t.Errorf("unexpected non-match\nreceiver: %s\nfrom: %s\nto: %s", test.Receiver, fromEP, toEP) } if gotStr, wantStr := gotAddr.String(), test.WantResult; gotStr != wantStr { @@ -308,7 +308,7 @@ func TestAbsResourceInstanceMoveDestination(t *testing.T) { tests := []struct { DeclModule string StmtFrom, StmtTo string - Reciever string + Receiver string WantMatch bool WantResult string }{ @@ -528,7 +528,7 @@ func TestAbsResourceInstanceMoveDestination(t *testing.T) { "%s: %s to %s with %s", test.DeclModule, test.StmtFrom, test.StmtTo, - test.Reciever, + test.Receiver, ), func(t *testing.T) { @@ -562,20 +562,20 @@ func TestAbsResourceInstanceMoveDestination(t *testing.T) { t.Fatalf("invalid test case: non-unifyable endpoints\nfrom: %s\nto: %s", fromEPLocal, toEPLocal) } - receiverAddr, diags := ParseAbsResourceInstanceStr(test.Reciever) + receiverAddr, diags := ParseAbsResourceInstanceStr(test.Receiver) if diags.HasErrors() { t.Fatalf("invalid reciever address: %s", diags.Err().Error()) } gotAddr, gotMatch := receiverAddr.MoveDestination(fromEP, toEP) if !test.WantMatch { if gotMatch { - t.Errorf("unexpected match\nreciever: %s\nfrom: %s\nto: %s\nresult: %s", test.Reciever, fromEP, toEP, gotAddr) + t.Errorf("unexpected match\nreceiver: %s\nfrom: %s\nto: %s\nresult: %s", test.Receiver, fromEP, toEP, gotAddr) } return } if !gotMatch { - t.Errorf("unexpected non-match\nreciever: %s\nfrom: %s\nto: %s", test.Reciever, fromEP, toEP) + t.Fatalf("unexpected non-match\nreceiver: %s (%T)\nfrom: %s\nto: %s\ngot: (no match)\nwant: %s", test.Receiver, receiverAddr, fromEP, toEP, test.WantResult) } if gotStr, wantStr := gotAddr.String(), test.WantResult; gotStr != wantStr { @@ -590,10 +590,95 @@ func TestAbsResourceMoveDestination(t *testing.T) { tests := []struct { DeclModule string StmtFrom, StmtTo string - Reciever string + Receiver string WantMatch bool WantResult string }{ + { + ``, + `test_object.beep`, + `test_object.boop`, + `test_object.beep`, + true, + `test_object.boop`, + }, + { + ``, + `test_object.beep`, + `module.foo.test_object.beep`, + `test_object.beep`, + true, + `module.foo.test_object.beep`, + }, + { + ``, + `test_object.beep`, + `module.foo[0].test_object.beep`, + `test_object.beep`, + true, + `module.foo[0].test_object.beep`, + }, + + { + ``, + `module.foo.test_object.beep`, + `test_object.beep`, + `module.foo.test_object.beep`, + true, + `test_object.beep`, + }, + { + ``, + `module.foo[0].test_object.beep`, + `test_object.beep`, + `module.foo[0].test_object.beep`, + true, + `test_object.beep`, + }, + { + `foo`, + `test_object.beep`, + `test_object.boop`, + `module.foo[0].test_object.beep`, + true, + `module.foo[0].test_object.boop`, + }, + + { + ``, + `test_object.beep`, + `test_object.boop`, + `test_object.boop`, + false, // the reciever is already the "to" address + ``, + }, + { + `foo`, + `test_object.beep`, + `test_object.boop`, + `test_object.beep`, + false, // the receiver is not inside an instance of module "foo" + ``, + }, + { + `foo.bar`, + `test_object.beep`, + `test_object.boop`, + `test_object.beep`, + false, // the receiver is not inside an instance of module "foo.bar" + ``, + }, + { + ``, + `module.foo[0].test_object.beep`, + `test_object.beep`, + `module.foo[1].test_object.beep`, + false, // receiver is in a different instance of module.foo + ``, + }, + + // Moving a module also moves all of the resources declared within it. + // The following tests all cover variations of that rule. { ``, `module.foo`, @@ -804,13 +889,14 @@ func TestAbsResourceMoveDestination(t *testing.T) { }, } - for _, test := range tests { + for i, test := range tests { t.Run( fmt.Sprintf( - "%s: %s to %s with %s", + "[%02d] %s: %s to %s with %s", + i, test.DeclModule, test.StmtFrom, test.StmtTo, - test.Reciever, + test.Receiver, ), func(t *testing.T) { @@ -848,7 +934,7 @@ func TestAbsResourceMoveDestination(t *testing.T) { // AbsResourceParser, and so we'll just cheat and parse this // as a resource instance but fail if it includes an instance // key. - receiverInstanceAddr, diags := ParseAbsResourceInstanceStr(test.Reciever) + receiverInstanceAddr, diags := ParseAbsResourceInstanceStr(test.Receiver) if diags.HasErrors() { t.Fatalf("invalid reciever address: %s", diags.Err().Error()) } @@ -859,13 +945,13 @@ func TestAbsResourceMoveDestination(t *testing.T) { gotAddr, gotMatch := receiverAddr.MoveDestination(fromEP, toEP) if !test.WantMatch { if gotMatch { - t.Errorf("unexpected match\nreciever: %s\nfrom: %s\nto: %s\nresult: %s", test.Reciever, fromEP, toEP, gotAddr) + t.Errorf("unexpected match\nreceiver: %s (%T)\nfrom: %s\nto: %s\nresult: %s", test.Receiver, receiverAddr, fromEP, toEP, gotAddr) } return } if !gotMatch { - t.Errorf("unexpected non-match\nreciever: %s\nfrom: %s\nto: %s", test.Reciever, fromEP, toEP) + t.Fatalf("unexpected non-match\nreceiver: %s (%T)\nfrom: %s\nto: %s\ngot: no match\nwant: %s", test.Receiver, receiverAddr, fromEP, toEP, test.WantResult) } if gotStr, wantStr := gotAddr.String(), test.WantResult; gotStr != wantStr {