From 4cbe6cabfc30cf7dabda73376f6e2361f20a4a62 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 29 Jun 2021 19:03:59 -0700 Subject: [PATCH] addrs: AbsMoveable, ConfigMoveable, and MoveableEndpoint These three types represent the three different address representations we need to represent different stages of analysis for "moved" blocks in the configuration. The goal here is to encapsulate all of the static address wrangling inside these types so that users of these types elsewhere would have to work pretty hard to use them incorrectly. In particular, the MovableEndpoint type intentionally fully encapsulates the weird relative addresses we use in configuration so that code elsewhere in Terraform can never end up holding an address of a type that suggests absolute when it's actually relative. That situation only occurs in the internals of MoveableEndpoint where we use not-really-absolute AbsMoveable address types to represent the not-yet-resolved relative addresses. This only takes care of the static address wrangling. There's lots of other rules for what makes a "moved" block valid which will need to be checked elsewhere because they require more context than just the content of the address itself. --- internal/addrs/module.go | 4 + internal/addrs/module_call.go | 8 + internal/addrs/module_instance.go | 4 + internal/addrs/move_endpoint.go | 260 +++++++++ internal/addrs/move_endpoint_test.go | 752 +++++++++++++++++++++++++++ internal/addrs/moveable.go | 44 ++ internal/addrs/parse_target.go | 78 ++- internal/addrs/resource.go | 12 + 8 files changed, 1148 insertions(+), 14 deletions(-) create mode 100644 internal/addrs/move_endpoint.go create mode 100644 internal/addrs/move_endpoint_test.go create mode 100644 internal/addrs/moveable.go diff --git a/internal/addrs/module.go b/internal/addrs/module.go index 80f394ad75..4ae72b4e42 100644 --- a/internal/addrs/module.go +++ b/internal/addrs/module.go @@ -150,3 +150,7 @@ func (m Module) Ancestors() []Module { } return ret } + +func (m Module) configMoveableSigil() { + // ModuleInstance is moveable +} diff --git a/internal/addrs/module_call.go b/internal/addrs/module_call.go index 8d69207745..c55433ac67 100644 --- a/internal/addrs/module_call.go +++ b/internal/addrs/module_call.go @@ -42,6 +42,14 @@ type AbsModuleCall struct { Call ModuleCall } +func (c AbsModuleCall) absMoveableSigil() { + // AbsModuleCall is "moveable". +} + +func (c AbsModuleCall) String() string { + return fmt.Sprintf("%s.%s", c.Module, c.Call.Name) +} + func (c AbsModuleCall) Instance(key InstanceKey) ModuleInstance { ret := make(ModuleInstance, len(c.Module), len(c.Module)+1) copy(ret, c.Module) diff --git a/internal/addrs/module_instance.go b/internal/addrs/module_instance.go index 5162fb3c17..dfaacc418e 100644 --- a/internal/addrs/module_instance.go +++ b/internal/addrs/module_instance.go @@ -492,6 +492,10 @@ func (m ModuleInstance) targetableSigil() { // ModuleInstance is targetable } +func (m ModuleInstance) absMoveableSigil() { + // ModuleInstance is moveable +} + func (s ModuleInstanceStep) String() string { if s.InstanceKey != NoKey { return s.Name + s.InstanceKey.String() diff --git a/internal/addrs/move_endpoint.go b/internal/addrs/move_endpoint.go new file mode 100644 index 0000000000..70172a080a --- /dev/null +++ b/internal/addrs/move_endpoint.go @@ -0,0 +1,260 @@ +package addrs + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// MoveEndpoint is to AbsMoveable and ConfigMoveable what Target is to +// Targetable: a wrapping struct that captures the result of decoding an HCL +// traversal representing a relative path from the current module to +// a moveable object. +// +// Its name reflects that its primary purpose is for the "from" and "to" +// addresses in a "moved" statement in the configuration, but it's also +// valid to use MoveEndpoint for other similar mechanisms that give +// Terraform hints about historical configuration changes that might +// prompt creating a different plan than Terraform would by default. +// +// To obtain a full address from a MoveEndpoint you must use +// either the package function UnifyMoveEndpoints (to get an AbsMovable) or +// the method ConfigMoveable (to get a ConfigMoveable). +type MoveEndpoint struct { + // SourceRange is the location of the physical endpoint address + // in configuration, if this MoveEndpoint was decoded from a + // configuration expresson. + SourceRange tfdiags.SourceRange + + // Internally we (ab)use AbsMovable as the representation of our + // relative address, even though everywhere else in Terraform + // AbsMovable always represents a fully-absolute address. + // In practice, due to the implementation of ParseMoveEndpoint, + // this is always either a ModuleInstance or an AbsResourceInstance, + // and we only consider the possibility of interpreting it as + // a AbsModuleCall or an AbsResource in UnifyMoveEndpoints. + // This is intentionally unexported to encapsulate this unusual + // meaning of AbsMovable. + relSubject AbsMoveable +} + +func (e *MoveEndpoint) String() string { + // Our internal pseudo-AbsMovable representing the relative + // address (either ModuleInstance or AbsResourceInstance) is + // a good enough proxy for the relative move endpoint address + // serialization. + return e.relSubject.String() +} + +// ConfigMovable transforms the reciever into a ConfigMovable by resolving it +// relative to the given base module, which should be the module where +// the MoveEndpoint expression was found. +// +// The result is useful for finding the target object in the configuration, +// but it's not sufficient for fully interpreting a move statement because +// it lacks the specific module and resource instance keys. +func (e *MoveEndpoint) ConfigMoveable(baseModule Module) ConfigMoveable { + addr := e.relSubject + switch addr := addr.(type) { + case ModuleInstance: + ret := make(Module, 0, len(baseModule)+len(addr)) + ret = append(ret, baseModule...) + ret = append(ret, addr.Module()...) + return ret + case AbsResourceInstance: + moduleAddr := make(Module, 0, len(baseModule)+len(addr.Module)) + moduleAddr = append(moduleAddr, baseModule...) + moduleAddr = append(moduleAddr, addr.Module.Module()...) + return ConfigResource{ + Module: moduleAddr, + Resource: addr.Resource.Resource, + } + default: + // The above should be exhaustive for all of the types + // that ParseMoveEndpoint produces as our intermediate + // address representation. + panic(fmt.Sprintf("unsupported address type %T", addr)) + } + +} + +// ParseMoveEndpoint attempts to interpret the given traversal as a +// "move endpoint" address, which is a relative path from the module containing +// the traversal to a movable object in either the same module or in some +// child module. +// +// This deals only with the syntactic element of a move endpoint expression +// in configuration. Before the result will be useful you'll need to combine +// it with the address of the module where it was declared in order to get +// an absolute address relative to the root module. +func ParseMoveEndpoint(traversal hcl.Traversal) (*MoveEndpoint, tfdiags.Diagnostics) { + path, remain, diags := parseModuleInstancePrefix(traversal) + if diags.HasErrors() { + return nil, diags + } + + rng := tfdiags.SourceRangeFromHCL(traversal.SourceRange()) + + if len(remain) == 0 { + return &MoveEndpoint{ + relSubject: path, + SourceRange: rng, + }, diags + } + + riAddr, moreDiags := parseResourceInstanceUnderModule(path, remain) + diags = diags.Append(moreDiags) + if diags.HasErrors() { + return nil, diags + } + + return &MoveEndpoint{ + relSubject: riAddr, + SourceRange: rng, + }, diags +} + +// UnifyMoveEndpoints takes a pair of MoveEndpoint objects representing the +// "from" and "to" addresses in a moved block, and returns a pair of +// AbsMoveable addresses guaranteed to be of the same dynamic type +// that represent what the two MoveEndpoint addresses refer to. +// +// moduleAddr must be the address of the module instance where the move +// was declared. +// +// This function deals both with the conversion from relative to absolute +// addresses and with resolving the ambiguity between no-key instance +// addresses and whole-object addresses, returning the least specific +// address type possible. +// +// Not all combinations of addresses are unifyable: the two addresses must +// either both include resources or both just be modules. If the two +// given addresses are incompatible then UnifyMoveEndpoints returns (nil, nil), +// in which case the caller should typically report an error to the user +// stating the unification constraints. +func UnifyMoveEndpoints(moduleAddr ModuleInstance, relFrom, relTo *MoveEndpoint) (absFrom, absTo AbsMoveable) { + + // First we'll make a decision about which address type we're + // ultimately trying to unify to. For our internal purposes + // here we're going to borrow TargetableAddrType just as a + // convenient way to talk about our address types, even though + // targetable address types are not 100% aligned with moveable + // address types. + fromType := relFrom.internalAddrType() + toType := relTo.internalAddrType() + var wantType TargetableAddrType + + // Our goal here is to choose the whole-resource or whole-module-call + // addresses if both agree on it, but to use specific instance addresses + // otherwise. This is a somewhat-arbitrary way to resolve syntactic + // ambiguity between the two situations which allows both for renaming + // whole resources and for switching from a single-instance object to + // a multi-instance object. + switch { + case fromType == AbsResourceInstanceAddrType || toType == AbsResourceInstanceAddrType: + wantType = AbsResourceInstanceAddrType + case fromType == AbsResourceAddrType || toType == AbsResourceAddrType: + wantType = AbsResourceAddrType + case fromType == ModuleInstanceAddrType || toType == ModuleInstanceAddrType: + wantType = ModuleInstanceAddrType + case fromType == ModuleAddrType || toType == ModuleAddrType: + // NOTE: We're fudging a little here and using + // ModuleAddrType to represent AbsModuleCall rather + // than Module. + wantType = ModuleAddrType + default: + panic("unhandled move address types") + } + + absFrom = relFrom.prepareAbsMoveable(moduleAddr, wantType) + absTo = relTo.prepareAbsMoveable(moduleAddr, wantType) + if absFrom == nil || absTo == nil { + // if either of them failed then they both failed, to make the + // caller's life a little easier. + return nil, nil + } + return absFrom, absTo +} + +func (e *MoveEndpoint) prepareAbsMoveable(moduleAddr ModuleInstance, wantType TargetableAddrType) AbsMoveable { + // relAddr can only be either AbsResourceInstance or ModuleInstance, the + // internal intermediate representation produced by ParseMoveEndpoint. + relAddr := e.relSubject + + switch relAddr := relAddr.(type) { + case ModuleInstance: + switch wantType { + case ModuleInstanceAddrType: + ret := make(ModuleInstance, 0, len(moduleAddr)+len(relAddr)) + ret = append(ret, moduleAddr...) + ret = append(ret, relAddr...) + return ret + case ModuleAddrType: + // NOTE: We're fudging a little here and using + // ModuleAddrType to represent AbsModuleCall rather + // than Module. + callerAddr := make(ModuleInstance, 0, len(moduleAddr)+len(relAddr)-1) + callerAddr = append(callerAddr, moduleAddr...) + callerAddr = append(callerAddr, relAddr[:len(relAddr)-1]...) + return AbsModuleCall{ + Module: callerAddr, + Call: ModuleCall{ + Name: relAddr[len(relAddr)-1].Name, + }, + } + default: + return nil // can't make any other types from a ModuleInstance + } + case AbsResourceInstance: + callerAddr := make(ModuleInstance, 0, len(moduleAddr)+len(relAddr.Module)) + callerAddr = append(callerAddr, moduleAddr...) + callerAddr = append(callerAddr, relAddr.Module...) + switch wantType { + case AbsResourceInstanceAddrType: + return AbsResourceInstance{ + Module: callerAddr, + Resource: relAddr.Resource, + } + case AbsResourceAddrType: + return AbsResource{ + Module: callerAddr, + Resource: relAddr.Resource.Resource, + } + default: + return nil // can't make any other types from an AbsResourceInstance + } + default: + panic(fmt.Sprintf("unhandled address type %T", relAddr)) + } +} + +// internalAddrType helps facilitate our slight abuse of TargetableAddrType +// as a way to talk about our different possible result address types in +// UnifyMoveEndpoints. +// +// It's not really correct to use TargetableAddrType in this way, because +// it's for Targetable rather than for AbsMoveable, but as long as the two +// remain aligned enough it saves introducing yet another enumeration with +// similar members that would be for internal use only anyway. +func (e *MoveEndpoint) internalAddrType() TargetableAddrType { + switch addr := e.relSubject.(type) { + case ModuleInstance: + if !addr.IsRoot() && addr[len(addr)-1].InstanceKey == NoKey { + // NOTE: We're fudging a little here and using + // ModuleAddrType to represent AbsModuleCall rather + // than Module. + return ModuleAddrType + } + return ModuleInstanceAddrType + case AbsResourceInstance: + if addr.Resource.Key == NoKey { + return AbsResourceAddrType + } + return AbsResourceInstanceAddrType + default: + // The above should cover all of the address types produced + // by ParseMoveEndpoint. + panic(fmt.Sprintf("unsupported address type %T", addr)) + } +} diff --git a/internal/addrs/move_endpoint_test.go b/internal/addrs/move_endpoint_test.go new file mode 100644 index 0000000000..a54b79c85b --- /dev/null +++ b/internal/addrs/move_endpoint_test.go @@ -0,0 +1,752 @@ +package addrs + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" +) + +func TestParseMoveEndpoint(t *testing.T) { + tests := []struct { + Input string + WantRel AbsMoveable // funny intermediate subset of AbsMovable + WantErr string + }{ + { + `foo.bar`, + AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: NoKey, + }, + }, + ``, + }, + { + `foo.bar[0]`, + AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: IntKey(0), + }, + }, + ``, + }, + { + `foo.bar["a"]`, + AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("a"), + }, + }, + ``, + }, + { + `module.boop.foo.bar`, + AbsResourceInstance{ + Module: ModuleInstance{ + ModuleInstanceStep{Name: "boop"}, + }, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: NoKey, + }, + }, + ``, + }, + { + `module.boop.foo.bar[0]`, + AbsResourceInstance{ + Module: ModuleInstance{ + ModuleInstanceStep{Name: "boop"}, + }, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: IntKey(0), + }, + }, + ``, + }, + { + `module.boop.foo.bar["a"]`, + AbsResourceInstance{ + Module: ModuleInstance{ + ModuleInstanceStep{Name: "boop"}, + }, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("a"), + }, + }, + ``, + }, + { + `data.foo.bar`, + AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: DataResourceMode, + Type: "foo", + Name: "bar", + }, + Key: NoKey, + }, + }, + ``, + }, + { + `data.foo.bar[0]`, + AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: DataResourceMode, + Type: "foo", + Name: "bar", + }, + Key: IntKey(0), + }, + }, + ``, + }, + { + `data.foo.bar["a"]`, + AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: DataResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("a"), + }, + }, + ``, + }, + { + `module.boop.data.foo.bar`, + AbsResourceInstance{ + Module: ModuleInstance{ + ModuleInstanceStep{Name: "boop"}, + }, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: DataResourceMode, + Type: "foo", + Name: "bar", + }, + Key: NoKey, + }, + }, + ``, + }, + { + `module.boop.data.foo.bar[0]`, + AbsResourceInstance{ + Module: ModuleInstance{ + ModuleInstanceStep{Name: "boop"}, + }, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: DataResourceMode, + Type: "foo", + Name: "bar", + }, + Key: IntKey(0), + }, + }, + ``, + }, + { + `module.boop.data.foo.bar["a"]`, + AbsResourceInstance{ + Module: ModuleInstance{ + ModuleInstanceStep{Name: "boop"}, + }, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: DataResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("a"), + }, + }, + ``, + }, + { + `module.foo`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo"}, + }, + ``, + }, + { + `module.foo[0]`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo", InstanceKey: IntKey(0)}, + }, + ``, + }, + { + `module.foo["a"]`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo", InstanceKey: StringKey("a")}, + }, + ``, + }, + { + `module.foo.module.bar`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo"}, + ModuleInstanceStep{Name: "bar"}, + }, + ``, + }, + { + `module.foo[1].module.bar`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo", InstanceKey: IntKey(1)}, + ModuleInstanceStep{Name: "bar"}, + }, + ``, + }, + { + `module.foo.module.bar[1]`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo"}, + ModuleInstanceStep{Name: "bar", InstanceKey: IntKey(1)}, + }, + ``, + }, + { + `module.foo[0].module.bar[1]`, + ModuleInstance{ + ModuleInstanceStep{Name: "foo", InstanceKey: IntKey(0)}, + ModuleInstanceStep{Name: "bar", InstanceKey: IntKey(1)}, + }, + ``, + }, + { + `module`, + nil, + `Invalid address operator: Prefix "module." must be followed by a module name.`, + }, + { + `module[0]`, + nil, + `Invalid address operator: Prefix "module." must be followed by a module name.`, + }, + { + `module.foo.data`, + nil, + `Invalid address: Resource specification must include a resource type and name.`, + }, + { + `module.foo.data.bar`, + nil, + `Invalid address: Resource specification must include a resource type and name.`, + }, + { + `module.foo.data[0]`, + nil, + `Invalid address: Resource specification must include a resource type and name.`, + }, + { + `module.foo.data.bar[0]`, + nil, + `Invalid address: A resource name is required.`, + }, + { + `module.foo.bar`, + nil, + `Invalid address: Resource specification must include a resource type and name.`, + }, + { + `module.foo.bar[0]`, + nil, + `Invalid address: A resource name is required.`, + }, + } + + for _, test := range tests { + t.Run(test.Input, func(t *testing.T) { + traversal, hclDiags := hclsyntax.ParseTraversalAbs([]byte(test.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) + + switch { + case test.WantErr != "": + if !diags.HasErrors() { + t.Fatalf("unexpected success\nwant error: %s", test.WantErr) + } + gotErr := diags.Err().Error() + if gotErr != test.WantErr { + t.Fatalf("wrong error\ngot: %s\nwant: %s", gotErr, test.WantErr) + } + default: + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err().Error()) + } + if diff := cmp.Diff(test.WantRel, moveEp.relSubject); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + } + }) + } +} + +func TestUnifyMoveEndpoints(t *testing.T) { + tests := []struct { + InputFrom, InputTo string + Module ModuleInstance + WantFrom, WantTo AbsMoveable + }{ + { + InputFrom: `foo.bar`, + InputTo: `foo.baz`, + Module: RootModuleInstance, + WantFrom: AbsResource{ + Module: RootModuleInstance, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + WantTo: AbsResource{ + Module: RootModuleInstance, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "baz", + }, + }, + }, + { + InputFrom: `foo.bar`, + InputTo: `foo.baz`, + Module: RootModuleInstance.Child("a", NoKey), + WantFrom: AbsResource{ + Module: RootModuleInstance.Child("a", NoKey), + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + WantTo: AbsResource{ + Module: RootModuleInstance.Child("a", NoKey), + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "baz", + }, + }, + }, + { + InputFrom: `foo.bar`, + InputTo: `module.b[0].foo.baz`, + Module: RootModuleInstance.Child("a", NoKey), + WantFrom: AbsResource{ + Module: RootModuleInstance.Child("a", NoKey), + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + WantTo: AbsResource{ + Module: RootModuleInstance.Child("a", NoKey).Child("b", IntKey(0)), + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "baz", + }, + }, + }, + { + InputFrom: `foo.bar`, + InputTo: `foo.bar["thing"]`, + Module: RootModuleInstance, + WantFrom: AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + }, + WantTo: AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("thing"), + }, + }, + }, + { + InputFrom: `foo.bar["thing"]`, + InputTo: `foo.bar`, + Module: RootModuleInstance, + WantFrom: AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("thing"), + }, + }, + WantTo: AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + }, + }, + { + InputFrom: `foo.bar["a"]`, + InputTo: `foo.bar["b"]`, + Module: RootModuleInstance, + WantFrom: AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("a"), + }, + }, + WantTo: AbsResourceInstance{ + Module: RootModuleInstance, + Resource: ResourceInstance{ + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + Key: StringKey("b"), + }, + }, + }, + { + InputFrom: `module.foo`, + InputTo: `module.bar`, + Module: RootModuleInstance, + WantFrom: AbsModuleCall{ + Module: RootModuleInstance, + Call: ModuleCall{Name: "foo"}, + }, + WantTo: AbsModuleCall{ + Module: RootModuleInstance, + Call: ModuleCall{Name: "bar"}, + }, + }, + { + InputFrom: `module.foo`, + InputTo: `module.bar.module.baz`, + Module: RootModuleInstance, + WantFrom: AbsModuleCall{ + Module: RootModuleInstance, + Call: ModuleCall{Name: "foo"}, + }, + WantTo: AbsModuleCall{ + Module: RootModuleInstance.Child("bar", NoKey), + Call: ModuleCall{Name: "baz"}, + }, + }, + { + InputFrom: `module.foo`, + InputTo: `module.bar.module.baz`, + Module: RootModuleInstance.Child("bloop", StringKey("hi")), + WantFrom: AbsModuleCall{ + Module: RootModuleInstance.Child("bloop", StringKey("hi")), + Call: ModuleCall{Name: "foo"}, + }, + WantTo: AbsModuleCall{ + Module: RootModuleInstance.Child("bloop", StringKey("hi")).Child("bar", NoKey), + Call: ModuleCall{Name: "baz"}, + }, + }, + { + InputFrom: `module.foo[0]`, + InputTo: `module.foo["a"]`, + Module: RootModuleInstance, + WantFrom: RootModuleInstance.Child("foo", IntKey(0)), + WantTo: RootModuleInstance.Child("foo", StringKey("a")), + }, + { + InputFrom: `module.foo`, + InputTo: `module.foo["a"]`, + Module: RootModuleInstance, + WantFrom: RootModuleInstance.Child("foo", NoKey), + WantTo: RootModuleInstance.Child("foo", StringKey("a")), + }, + { + InputFrom: `module.foo[0]`, + InputTo: `module.foo`, + Module: RootModuleInstance, + WantFrom: RootModuleInstance.Child("foo", IntKey(0)), + WantTo: RootModuleInstance.Child("foo", NoKey), + }, + { + InputFrom: `module.foo[0]`, + InputTo: `module.foo`, + Module: RootModuleInstance.Child("bloop", NoKey), + WantFrom: RootModuleInstance.Child("bloop", NoKey).Child("foo", IntKey(0)), + WantTo: RootModuleInstance.Child("bloop", NoKey).Child("foo", NoKey), + }, + { + InputFrom: `module.foo`, + InputTo: `foo.bar`, + Module: RootModuleInstance, + WantFrom: nil, // Can't unify module call with resource + WantTo: nil, + }, + { + InputFrom: `module.foo[0]`, + InputTo: `foo.bar`, + Module: RootModuleInstance, + WantFrom: nil, // Can't unify module instance with resource + WantTo: nil, + }, + { + InputFrom: `module.foo`, + InputTo: `foo.bar[0]`, + Module: RootModuleInstance, + WantFrom: nil, // Can't unify module call with resource instance + WantTo: nil, + }, + { + InputFrom: `module.foo[0]`, + InputTo: `foo.bar[0]`, + Module: RootModuleInstance, + WantFrom: nil, // Can't unify module instance with resource instance + WantTo: nil, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%s to %s in %s", test.InputFrom, test.InputTo, test.Module), func(t *testing.T) { + parseInput := func(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 + } + + fromEp := parseInput(test.InputFrom) + toEp := parseInput(test.InputTo) + + diffOpts := cmpopts.IgnoreUnexported(ModuleCall{}) + gotFrom, gotTo := UnifyMoveEndpoints(test.Module, fromEp, toEp) + if diff := cmp.Diff(test.WantFrom, gotFrom, diffOpts); diff != "" { + t.Errorf("wrong 'from' address\n%s", diff) + } + if diff := cmp.Diff(test.WantTo, gotTo, diffOpts); diff != "" { + t.Errorf("wrong 'to' address\n%s", diff) + } + }) + } +} + +func TestMoveEndpointConfigMoveable(t *testing.T) { + tests := []struct { + Input string + Module Module + Want ConfigMoveable + }{ + { + `foo.bar`, + RootModule, + ConfigResource{ + Module: RootModule, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + }, + { + `foo.bar[0]`, + RootModule, + ConfigResource{ + Module: RootModule, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + }, + { + `module.foo.bar.baz`, + RootModule, + ConfigResource{ + Module: Module{"foo"}, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "bar", + Name: "baz", + }, + }, + }, + { + `module.foo[0].bar.baz`, + RootModule, + ConfigResource{ + Module: Module{"foo"}, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "bar", + Name: "baz", + }, + }, + }, + { + `foo.bar`, + Module{"boop"}, + ConfigResource{ + Module: Module{"boop"}, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + }, + { + `module.bloop.foo.bar`, + Module{"bleep"}, + ConfigResource{ + Module: Module{"bleep", "bloop"}, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "foo", + Name: "bar", + }, + }, + }, + { + `module.foo.bar.baz`, + RootModule, + ConfigResource{ + Module: Module{"foo"}, + Resource: Resource{ + Mode: ManagedResourceMode, + Type: "bar", + Name: "baz", + }, + }, + }, + { + `module.foo`, + RootModule, + Module{"foo"}, + }, + { + `module.foo[0]`, + RootModule, + Module{"foo"}, + }, + { + `module.bloop`, + Module{"bleep"}, + Module{"bleep", "bloop"}, + }, + { + `module.bloop[0]`, + Module{"bleep"}, + Module{"bleep", "bloop"}, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%s in %s", test.Input, test.Module), func(t *testing.T) { + traversal, hclDiags := hclsyntax.ParseTraversalAbs([]byte(test.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()) + } + + got := moveEp.ConfigMoveable(test.Module) + if diff := cmp.Diff(test.Want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + } +} diff --git a/internal/addrs/moveable.go b/internal/addrs/moveable.go new file mode 100644 index 0000000000..cac6eceb89 --- /dev/null +++ b/internal/addrs/moveable.go @@ -0,0 +1,44 @@ +package addrs + +// AbsMoveable is an interface implemented by address types that can be either +// the source or destination of a "moved" statement in configuration, along +// with any other similar cross-module state refactoring statements we might +// allow. +// +// Note that AbsMovable represents an absolute address relative to the root +// of the configuration, which is different than the direct representation +// of these in configuration where the author gives an address relative to +// the current module where the address is defined. The type MoveEndpoint + +type AbsMoveable interface { + absMoveableSigil() + + String() string +} + +// The following are all of the possible AbsMovable address types: +var ( + _ AbsMoveable = AbsResource{} + _ AbsMoveable = AbsResourceInstance{} + _ AbsMoveable = ModuleInstance(nil) + _ AbsMoveable = AbsModuleCall{} +) + +// ConfigMoveable is similar to AbsMoveable but represents a static object in +// the configuration, rather than an instance of that object created by +// module expansion. +// +// Note that ConfigMovable represents an absolute address relative to the root +// of the configuration, which is different than the direct representation +// of these in configuration where the author gives an address relative to +// the current module where the address is defined. The type MoveEndpoint +// represents the relative form given directly in configuration. +type ConfigMoveable interface { + configMoveableSigil() +} + +// The following are all of the possible ConfigMovable address types: +var ( + _ ConfigMoveable = ConfigResource{} + _ ConfigMoveable = Module(nil) +) diff --git a/internal/addrs/parse_target.go b/internal/addrs/parse_target.go index f75e652369..378e4de6a5 100644 --- a/internal/addrs/parse_target.go +++ b/internal/addrs/parse_target.go @@ -39,6 +39,36 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { }, diags } + riAddr, moreDiags := parseResourceInstanceUnderModule(path, remain) + diags = diags.Append(moreDiags) + if diags.HasErrors() { + return nil, diags + } + + var subject Targetable + switch { + case riAddr.Resource.Key == NoKey: + // We always assume that a no-key instance is meant to + // be referring to the whole resource, because the distinction + // doesn't really matter for targets anyway. + subject = riAddr.ContainingResource() + default: + subject = riAddr + } + + return &Target{ + Subject: subject, + SourceRange: rng, + }, diags +} + +func parseResourceInstanceUnderModule(moduleAddr ModuleInstance, remain hcl.Traversal) (AbsResourceInstance, tfdiags.Diagnostics) { + // Note that this helper is used as part of both ParseTarget and + // ParseMoveEndpoint, so its error messages should be generic + // enough to suit both situations. + + var diags tfdiags.Diagnostics + mode := ManagedResourceMode if remain.RootName() == "data" { mode = DataResourceMode @@ -52,7 +82,7 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { Detail: "Resource specification must include a resource type and name.", Subject: remain.SourceRange().Ptr(), }) - return nil, diags + return AbsResourceInstance{}, diags } var typeName, name string @@ -80,7 +110,7 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { default: panic("unknown mode") } - return nil, diags + return AbsResourceInstance{}, diags } switch tt := remain[1].(type) { @@ -93,14 +123,13 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { Detail: "A resource name is required.", Subject: remain[1].SourceRange().Ptr(), }) - return nil, diags + return AbsResourceInstance{}, diags } - var subject Targetable remain = remain[2:] switch len(remain) { case 0: - subject = path.Resource(mode, typeName, name) + return moduleAddr.ResourceInstance(mode, typeName, name, NoKey), diags case 1: if tt, ok := remain[0].(hcl.TraverseIndex); ok { key, err := ParseInstanceKey(tt.Key) @@ -111,10 +140,10 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { Detail: fmt.Sprintf("Invalid resource instance key: %s.", err), Subject: remain[0].SourceRange().Ptr(), }) - return nil, diags + return AbsResourceInstance{}, diags } - subject = path.ResourceInstance(mode, typeName, name, key) + return moduleAddr.ResourceInstance(mode, typeName, name, key), diags } else { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -122,7 +151,7 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { Detail: "Resource instance key must be given in square brackets.", Subject: remain[0].SourceRange().Ptr(), }) - return nil, diags + return AbsResourceInstance{}, diags } default: diags = diags.Append(&hcl.Diagnostic{ @@ -131,13 +160,8 @@ func ParseTarget(traversal hcl.Traversal) (*Target, tfdiags.Diagnostics) { Detail: "Unexpected extra operators after address.", Subject: remain[1].SourceRange().Ptr(), }) - return nil, diags + return AbsResourceInstance{}, diags } - - return &Target{ - Subject: subject, - SourceRange: rng, - }, diags } // ParseTargetStr is a helper wrapper around ParseTarget that takes a string @@ -316,3 +340,29 @@ func ParseAbsResourceInstanceStr(str string) (AbsResourceInstance, tfdiags.Diagn diags = diags.Append(addrDiags) return addr, diags } + +// ModuleAddr returns the module address portion of the subject of +// the recieving target. +// +// Regardless of specific address type, all targets always include +// a module address. They might also include something in that +// module, which this method always discards if so. +func (t *Target) ModuleAddr() ModuleInstance { + switch addr := t.Subject.(type) { + case ModuleInstance: + return addr + case Module: + // We assume that a module address is really + // referring to a module path containing only + // single-instance modules. + return addr.UnkeyedInstanceShim() + case AbsResourceInstance: + return addr.Module + case AbsResource: + return addr.Module + default: + // The above cases should be exhaustive for all + // implementations of Targetable. + panic(fmt.Sprintf("unsupported target address type %T", addr)) + } +} diff --git a/internal/addrs/resource.go b/internal/addrs/resource.go index d34fec97cd..97b7f5dd99 100644 --- a/internal/addrs/resource.go +++ b/internal/addrs/resource.go @@ -178,6 +178,10 @@ func (r AbsResource) Equal(o AbsResource) bool { return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource) } +func (r AbsResource) absMoveableSigil() { + // AbsResource is moveable +} + // AbsResourceInstance is an absolute address for a resource instance under a // given module path. type AbsResourceInstance struct { @@ -276,6 +280,10 @@ func (r AbsResourceInstance) Less(o AbsResourceInstance) bool { } } +func (r AbsResourceInstance) absMoveableSigil() { + // AbsResourceInstance is moveable +} + // ConfigResource is an address for a resource within a configuration. type ConfigResource struct { targetable @@ -335,6 +343,10 @@ func (r ConfigResource) Equal(o ConfigResource) bool { return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource) } +func (r ConfigResource) configMoveableSigil() { + // AbsResource is moveable +} + // ResourceMode defines which lifecycle applies to a given resource. Each // resource lifecycle has a slightly different address format. type ResourceMode rune