From 4d733b4d2dc16cd5461905da06e00db95bb09698 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 26 Jul 2021 16:05:05 -0700 Subject: [PATCH] addrs: Implement ModuleInstance.MoveDestination This method encapsulates the move-processing rules for applying move statements to ModuleInstance addresses. It honors both module call moves and module instance moves by trying to find a subsequence of the input that matches the "from" endpoint and then, if found, replacing it with the "to" endpoint while preserving the prefix and suffix around the match, if any. --- internal/addrs/move_endpoint_module.go | 85 +++++- internal/addrs/move_endpoint_module_test.go | 305 ++++++++++++++++++++ 2 files changed, 389 insertions(+), 1 deletion(-) create mode 100644 internal/addrs/move_endpoint_module_test.go diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index 0b6461bd9..2c7ad2276 100644 --- a/internal/addrs/move_endpoint_module.go +++ b/internal/addrs/move_endpoint_module.go @@ -157,7 +157,90 @@ func (e *MoveEndpointInModule) NestedWithin(other *MoveEndpointInModule) bool { // Both of the given endpoints must be from the same move statement and thus // must have matching object types. If not, MoveDestination will panic. func (m ModuleInstance) MoveDestination(fromMatch, toMatch *MoveEndpointInModule) (ModuleInstance, bool) { - return nil, false + // NOTE: This implementation assumes the invariant that fromMatch and + // toMatch both belong to the same configuration statement, and thus they + // will both have the same address type and the same declaration module. + + // The root module instance is not itself moveable. + if m.IsRoot() { + return nil, false + } + + // The two endpoints must either be module call or module instance + // addresses, or else this statement can never match. + if fromMatch.ObjectKind() != MoveEndpointModule { + return nil, false + } + + // The given module instance must have a prefix that matches the + // declaration module of the two endpoints. + if len(fromMatch.module) > len(m) { + return nil, false // too short to possibly match + } + for i := range fromMatch.module { + if fromMatch.module[i] != m[i].Name { + return nil, false // this step doesn't match + } + } + + // The rest of our work will be against the part of the reciever that's + // relative to the declaration module. mRel is a weird abuse of + // ModuleInstance that represents a relative module address, similar to + // what we do for MoveEndpointInModule.relSubject. + mPrefix, mRel := m[:len(fromMatch.module)], m[len(fromMatch.module):] + + // Our next goal is to split mRel into two parts: the match (if any) and + // the suffix. Our result will then replace the match with the replacement + // in toMatch while preserving the prefix and suffix. + var mSuffix, mNewMatch ModuleInstance + + switch relSubject := fromMatch.relSubject.(type) { + case ModuleInstance: + if len(relSubject) > len(mRel) { + return nil, false // too short to possibly match + } + for i := range relSubject { + if relSubject[i] != mRel[i] { + return nil, false // this step doesn't match + } + } + // If we get to here then we've found a match. Since the statement + // addresses are already themselves ModuleInstance fragments we can + // just slice out the relevant parts. + mNewMatch = toMatch.relSubject.(ModuleInstance) + mSuffix = mRel[len(relSubject):] + case AbsModuleCall: + // The module instance part of relSubject must be a prefix of + // mRel, and mRel must be at least one step longer to account for + // the call step itself. + if len(relSubject.Module) > len(mRel)-1 { + return nil, false + } + for i := range relSubject.Module { + if relSubject.Module[i] != mRel[i] { + return nil, false // this step doesn't match + } + } + // The call name must also match the next step of mRel, after + // the relSubject.Module prefix. + callStep := mRel[len(relSubject.Module)] + if callStep.Name != relSubject.Call.Name { + return nil, false + } + // If we get to here then we've found a match. We need to construct + // a new mNewMatch that's an instance of the "new" relSubject with + // the same key as our call. + mNewMatch = toMatch.relSubject.(AbsModuleCall).Instance(callStep.InstanceKey) + mSuffix = mRel[len(relSubject.Module)+1:] + default: + panic("invalid address type for module-kind move endpoint") + } + + ret := make(ModuleInstance, 0, len(mPrefix)+len(mNewMatch)+len(mSuffix)) + ret = append(ret, mPrefix...) + ret = append(ret, mNewMatch...) + ret = append(ret, mSuffix...) + return ret, true } // MoveDestination considers a an address representing a resource diff --git a/internal/addrs/move_endpoint_module_test.go b/internal/addrs/move_endpoint_module_test.go new file mode 100644 index 000000000..981c50ee5 --- /dev/null +++ b/internal/addrs/move_endpoint_module_test.go @@ -0,0 +1,305 @@ +package addrs + +import ( + "fmt" + "strings" + "testing" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +func TestModuleInstanceMoveDestination(t *testing.T) { + tests := []struct { + DeclModule string + StmtFrom, StmtTo string + Reciever string + WantMatch bool + WantResult string + }{ + { + ``, + `module.foo`, + `module.bar`, + `module.foo`, + true, + `module.bar`, + }, + { + ``, + `module.foo`, + `module.bar`, + `module.foo[1]`, + true, + `module.bar[1]`, + }, + { + ``, + `module.foo`, + `module.bar`, + `module.foo["a"]`, + true, + `module.bar["a"]`, + }, + { + ``, + `module.foo`, + `module.bar.module.foo`, + `module.foo`, + true, + `module.bar.module.foo`, + }, + { + ``, + `module.foo.module.bar`, + `module.bar`, + `module.foo.module.bar`, + true, + `module.bar`, + }, + { + ``, + `module.foo[1]`, + `module.foo[2]`, + `module.foo[1]`, + true, + `module.foo[2]`, + }, + { + ``, + `module.foo[1]`, + `module.foo`, + `module.foo[1]`, + true, + `module.foo`, + }, + { + ``, + `module.foo`, + `module.foo[1]`, + `module.foo`, + true, + `module.foo[1]`, + }, + { + ``, + `module.foo`, + `module.foo[1]`, + `module.foo.module.bar`, + true, + `module.foo[1].module.bar`, + }, + { + ``, + `module.foo`, + `module.foo[1]`, + `module.foo.module.bar[0]`, + true, + `module.foo[1].module.bar[0]`, + }, + { + ``, + `module.foo`, + `module.bar.module.foo`, + `module.foo[0]`, + true, + `module.bar.module.foo[0]`, + }, + { + ``, + `module.foo.module.bar`, + `module.bar`, + `module.foo.module.bar[0]`, + true, + `module.bar[0]`, + }, + { + `foo`, + `module.bar`, + `module.baz`, + `module.foo.module.bar`, + true, + `module.foo.module.baz`, + }, + { + `foo`, + `module.bar`, + `module.baz`, + `module.foo[1].module.bar`, + true, + `module.foo[1].module.baz`, + }, + { + `foo`, + `module.bar`, + `module.bar[1]`, + `module.foo[1].module.bar`, + true, + `module.foo[1].module.bar[1]`, + }, + { + ``, + `module.foo[1]`, + `module.foo[2]`, + `module.foo`, + false, // the receiver has a non-matching instance key (NoKey) + ``, + }, + { + ``, + `module.foo[1]`, + `module.foo[2]`, + `module.foo[2]`, + false, // the receiver is already the "to" address + ``, + }, + { + ``, + `module.foo`, + `module.bar`, + ``, + false, // the root module can never be moved + ``, + }, + { + `foo`, + `module.bar`, + `module.bar[1]`, + `module.boz`, + false, // the receiver is outside the declaration module + ``, + }, + { + `foo.bar`, + `module.bar`, + `module.bar[1]`, + `module.boz`, + false, // the receiver is outside the declaration module + ``, + }, + { + `foo.bar`, + `module.a`, + `module.b`, + `module.boz`, + false, // the receiver is outside the declaration module + ``, + }, + { + ``, + `module.a1.module.a2`, + `module.b1.module.b2`, + `module.c`, + false, // the receiver is outside the declaration module + ``, + }, + { + ``, + `module.a1.module.a2[0]`, + `module.b1.module.b2[1]`, + `module.c`, + false, // the receiver is outside the declaration module + ``, + }, + { + ``, + `module.a1.module.a2`, + `module.b1.module.b2`, + `module.a1.module.b2`, + false, // the receiver is outside the declaration module + ``, + }, + { + ``, + `module.a1.module.a2`, + `module.b1.module.b2`, + `module.b1.module.a2`, + false, // the receiver is outside the declaration module + ``, + }, + { + ``, + `module.a1.module.a2[0]`, + `module.b1.module.b2[1]`, + `module.a1.module.b2[0]`, + false, // the receiver is outside the declaration module + ``, + }, + { + ``, + `foo_instance.bar`, + `foo_instance.baz`, + `module.foo`, + false, // a resource address can never match a module instance + ``, + }, + } + + for _, test := range tests { + t.Run( + fmt.Sprintf( + "%s: %s to %s with %s", + test.DeclModule, + test.StmtFrom, test.StmtTo, + test.Reciever, + ), + func(t *testing.T) { + + parseStmtEP := func(t *testing.T, input string) *MoveEndpoint { + t.Helper() + + traversal, hclDiags := hclsyntax.ParseTraversalAbs([]byte(input), "", hcl.InitialPos) + if hclDiags.HasErrors() { + // We're not trying to test the HCL parser here, so any + // failures at this point are likely to be bugs in the + // test case itself. + t.Fatalf("syntax error: %s", hclDiags.Error()) + } + + moveEp, diags := ParseMoveEndpoint(traversal) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err().Error()) + } + return moveEp + } + + fromEPLocal := parseStmtEP(t, test.StmtFrom) + toEPLocal := parseStmtEP(t, test.StmtTo) + + declModule := RootModule + if test.DeclModule != "" { + declModule = strings.Split(test.DeclModule, ".") + } + fromEP, toEP := UnifyMoveEndpoints(declModule, fromEPLocal, toEPLocal) + if fromEP == nil || toEP == nil { + t.Fatalf("invalid test case: non-unifyable endpoints\nfrom: %s\nto: %s", fromEPLocal, toEPLocal) + } + + receiverAddr := RootModuleInstance + if test.Reciever != "" { + var diags tfdiags.Diagnostics + receiverAddr, diags = ParseModuleInstanceStr(test.Reciever) + 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) + } + return + } + + if !gotMatch { + t.Errorf("unexpected non-match\nreciever: %s\nfrom: %s\nto: %s", test.Reciever, fromEP, toEP) + } + + if gotStr, wantStr := gotAddr.String(), test.WantResult; gotStr != wantStr { + t.Errorf("wrong result\ngot: %s\nwant: %s", gotStr, wantStr) + } + }, + ) + } +}