diff --git a/internal/addrs/module.go b/internal/addrs/module.go index 3bfa818212..503adc356c 100644 --- a/internal/addrs/module.go +++ b/internal/addrs/module.go @@ -9,6 +9,8 @@ import ( "strings" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/opentofu/opentofu/internal/tfdiags" ) @@ -196,6 +198,22 @@ func ParseModule(traversal hcl.Traversal) (Module, tfdiags.Diagnostics) { return mod, diags } +// ParseModuleStr is a helper wrapper around [ParseModule] that first tries +// to parse the given string as HCL traversal syntax. +func ParseModuleStr(str string) (Module, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1}) + diags = diags.Append(parseDiags) + if parseDiags.HasErrors() { + return nil, diags + } + + addr, addrDiags := ParseModule(traversal) + diags = diags.Append(addrDiags) + return addr, diags +} + // parseModulePrefix parses a module address from the given traversal, // returning the module address and the remaining traversal. // For example, if the input traversal is ["module","a","module","b", diff --git a/internal/addrs/module_instance.go b/internal/addrs/module_instance.go index 1f051708af..3c573de69a 100644 --- a/internal/addrs/module_instance.go +++ b/internal/addrs/module_instance.go @@ -471,6 +471,42 @@ func (m ModuleInstance) Module() Module { return ret } +// HasSameModule returns true if calling [ModuleInstance.Module] on both +// the receiver and the other given address would produce equal [Module] +// addresses. +// +// This is here only as an optimization to avoid the overhead of constructing +// two [Module] values just to compare them and then throw them away. +func (m ModuleInstance) HasSameModule(other ModuleInstance) bool { + if len(m) != len(other) { + return false + } + for i := range m { + if m[i].Name != other[i].Name { + return false + } + } + return true +} + +// HasSameModule returns true if calling [ModuleInstance.Module] on the +// receiver would return a [Module] address equal to the one given as +// an argument. +// +// This is here only as an optimization to avoid the overhead of constructing +// a [Module] value from the reciever just to compare it and then throw it away. +func (m ModuleInstance) IsForModule(module Module) bool { + if len(m) != len(module) { + return false + } + for i := range m { + if m[i].Name != module[i] { + return false + } + } + return true +} + func (m ModuleInstance) AddrType() TargetableAddrType { return ModuleInstanceAddrType } diff --git a/internal/addrs/module_instance_test.go b/internal/addrs/module_instance_test.go index a8de77f8b6..5da6d4433c 100644 --- a/internal/addrs/module_instance_test.go +++ b/internal/addrs/module_instance_test.go @@ -83,6 +83,126 @@ func TestModuleInstanceEqual_false(t *testing.T) { } } +func TestHasSameModule(t *testing.T) { + tests := []struct { + a string + b string + + wantSame bool + }{ + { + "module.foo", + "module.bar", + false, + }, + { + "module.foo", + "module.foo.module.bar", + false, + }, + { + "module.foo[1]", + "module.bar[1]", + false, + }, + { + `module.foo[1]`, + `module.foo["1"]`, + true, + }, + { + "module.foo.module.bar", + "module.foo[1].module.bar", + true, + }, + { + `module.foo.module.bar`, + `module.foo["a"].module.bar`, + true, + }, + } + for _, test := range tests { + t.Run(fmt.Sprintf("%#v.HasSameModule(%#v)", test.a, test.b), func(t *testing.T) { + a, diags := ParseModuleInstanceStr(test.a) + if len(diags) > 0 { + t.Fatalf("invalid module instance address %s: %s", test.a, diags.Err()) + } + b, diags := ParseModuleInstanceStr(test.b) + if len(diags) > 0 { + t.Fatalf("invalid module instance address %s: %s", test.b, diags.Err()) + } + + // "HasSameModule" is commutative, so we'll test it both ways at once + gotAB := a.HasSameModule(b) + gotBA := b.HasSameModule(a) + + if gotAB != test.wantSame { + t.Errorf("wrong result\n1st: %s\n2nd: %s\ngot: %t\nwant: %t", a, b, gotAB, test.wantSame) + } + if gotBA != test.wantSame { + t.Errorf("wrong result\n1st: %s\n2nd: %s\ngot: %t\nwant: %t", b, a, gotBA, test.wantSame) + } + }) + } +} + +func TestIsForModule(t *testing.T) { + tests := []struct { + inst string + mod string + + want bool + }{ + { + "module.foo", + "module.bar", + false, + }, + { + "module.foo", + "module.foo.module.bar", + false, + }, + { + "module.foo[1]", + "module.bar", + false, + }, + { + `module.foo[1]`, + `module.foo`, + true, + }, + { + "module.foo[1].module.bar", + "module.foo.module.bar", + true, + }, + { + `module.foo["a"].module.bar`, + `module.foo.module.bar`, + true, + }, + } + for _, test := range tests { + t.Run(fmt.Sprintf("%#v.IsForModule(%#v)", test.inst, test.mod), func(t *testing.T) { + inst, diags := ParseModuleInstanceStr(test.inst) + if len(diags) > 0 { + t.Fatalf("invalid module instance address %s: %s", test.inst, diags.Err()) + } + mod, diags := ParseModuleStr(test.mod) + if len(diags) > 0 { + t.Fatalf("invalid module address %s: %s", test.mod, diags.Err()) + } + + got := inst.IsForModule(mod) + if got != test.want { + t.Errorf("wrong result\ninstance: %s\nmodule: %s\ngot: %t\nwant: %t", inst, mod, got, test.want) + } + }) + } +} + func BenchmarkStringShort(b *testing.B) { addr, _ := ParseModuleInstanceStr(`module.foo`) for n := 0; n < b.N; n++ { diff --git a/internal/addrs/module_test.go b/internal/addrs/module_test.go index 9ef6b6576d..d07b1af3a6 100644 --- a/internal/addrs/module_test.go +++ b/internal/addrs/module_test.go @@ -176,3 +176,11 @@ func TestParseModule(t *testing.T) { }) } } + +func mustParseModuleStr(str string) Module { + m, diags := ParseModuleStr(str) + if diags.HasErrors() { + panic(diags.ErrWithWarnings()) + } + return m +} diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index 1a7e838b83..b681365668 100644 --- a/internal/addrs/move_endpoint_module.go +++ b/internal/addrs/move_endpoint_module.go @@ -727,17 +727,17 @@ func (from *MoveEndpointInModule) IsModuleReIndex(to *MoveEndpointInModule) bool // the module call. We're not actually comparing indexes, so the // instance doesn't matter. callAddr := f.Instance(NoKey).Module() - return callAddr.Equal(t.Module()) + return t.IsForModule(callAddr) } case ModuleInstance: switch t := to.relSubject.(type) { case AbsModuleCall: callAddr := t.Instance(NoKey).Module() - return callAddr.Equal(f.Module()) + return f.IsForModule(callAddr) case ModuleInstance: - return t.Module().Equal(f.Module()) + return t.HasSameModule(f) } } diff --git a/internal/addrs/move_endpoint_module_test.go b/internal/addrs/move_endpoint_module_test.go index a5e6cdc8ee..55dabd9e44 100644 --- a/internal/addrs/move_endpoint_module_test.go +++ b/internal/addrs/move_endpoint_module_test.go @@ -1375,7 +1375,7 @@ func TestSelectsModule(t *testing.T) { }, { Endpoint: &MoveEndpointInModule{ - module: mustParseModuleInstanceStr("module.foo").Module(), + module: mustParseModuleStr("module.foo"), relSubject: AbsModuleCall{ Module: mustParseModuleInstanceStr("module.bar[2]"), Call: ModuleCall{Name: "baz"}, @@ -1386,7 +1386,7 @@ func TestSelectsModule(t *testing.T) { }, { Endpoint: &MoveEndpointInModule{ - module: mustParseModuleInstanceStr("module.foo").Module(), + module: mustParseModuleStr("module.foo"), relSubject: AbsModuleCall{ Module: mustParseModuleInstanceStr("module.bar[2]"), Call: ModuleCall{Name: "baz"}, @@ -1407,7 +1407,7 @@ func TestSelectsModule(t *testing.T) { }, { Endpoint: &MoveEndpointInModule{ - module: mustParseModuleInstanceStr("module.foo").Module(), + module: mustParseModuleStr("module.foo"), relSubject: mustParseAbsResourceInstanceStr(`module.bar.resource.name["key"]`), }, Addr: mustParseModuleInstanceStr(`module.foo[1].module.bar`), @@ -1429,7 +1429,7 @@ func TestSelectsModule(t *testing.T) { }, { Endpoint: &MoveEndpointInModule{ - module: mustParseModuleInstanceStr("module.nope").Module(), + module: mustParseModuleStr("module.nope"), relSubject: mustParseAbsResourceInstanceStr(`module.bar.resource.name["key"]`), }, Addr: mustParseModuleInstanceStr(`module.foo[1].module.bar`), diff --git a/internal/states/state.go b/internal/states/state.go index 3747a8eb7d..3b8f9e5e6c 100644 --- a/internal/states/state.go +++ b/internal/states/state.go @@ -94,7 +94,7 @@ func (s *State) Module(addr addrs.ModuleInstance) *Module { func (s *State) ModuleInstances(addr addrs.Module) []*Module { var ms []*Module for _, m := range s.Modules { - if m.Addr.Module().Equal(addr) { + if m.Addr.IsForModule(addr) { ms = append(ms, m) } } diff --git a/internal/tofu/eval_context_builtin.go b/internal/tofu/eval_context_builtin.go index 3b747f9d9c..5e2aebc354 100644 --- a/internal/tofu/eval_context_builtin.go +++ b/internal/tofu/eval_context_builtin.go @@ -212,7 +212,7 @@ func (ctx *BuiltinEvalContext) CloseProvider(addr addrs.AbsProviderConfig) error func (ctx *BuiltinEvalContext) ConfigureProvider(addr addrs.AbsProviderConfig, providerKey addrs.InstanceKey, cfg cty.Value) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - if !addr.Module.Equal(ctx.Path().Module()) { + if !ctx.Path().IsForModule(addr.Module) { // This indicates incorrect use of ConfigureProvider: it should be used // only from the module that the provider configuration belongs to. panic(fmt.Sprintf("%s configured by wrong module %s", addr, ctx.Path())) @@ -237,7 +237,7 @@ func (ctx *BuiltinEvalContext) ProviderInput(pc addrs.AbsProviderConfig) map[str ctx.ProviderLock.Lock() defer ctx.ProviderLock.Unlock() - if !pc.Module.Equal(ctx.Path().Module()) { + if !ctx.Path().IsForModule(pc.Module) { // This indicates incorrect use of InitProvider: it should be used // only from the module that the provider configuration belongs to. panic(fmt.Sprintf("%s initialized by wrong module %s", pc, ctx.Path())) @@ -346,7 +346,7 @@ func (ctx *BuiltinEvalContext) EvaluateReplaceTriggeredBy(expr hcl.Expression, r } // Do some validation to make sure we are expecting a change at all - cfg := ctx.Evaluator.Config.Descendent(ctx.Path().Module()) + cfg := ctx.Evaluator.Config.DescendentForInstance(ctx.Path()) resCfg := cfg.Module.ResourceByAddr(resourceAddr) if resCfg == nil { diags = diags.Append(&hcl.Diagnostic{ @@ -470,7 +470,7 @@ func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, source var providerKey addrs.InstanceKey if providedBy.KeyExpression != nil && ctx.Evaluator.Operation != walkValidate { moduleInstanceForKey := ctx.PathValue[:len(providedBy.KeyModule)] - if !moduleInstanceForKey.Module().Equal(providedBy.KeyModule) { + if !moduleInstanceForKey.IsForModule(providedBy.KeyModule) { panic(fmt.Sprintf("Invalid module key expression location %s in function %s", providedBy.KeyModule, pf.String())) } diff --git a/internal/tofu/node_resource_abstract.go b/internal/tofu/node_resource_abstract.go index dd72875761..838d4b7cdf 100644 --- a/internal/tofu/node_resource_abstract.go +++ b/internal/tofu/node_resource_abstract.go @@ -632,9 +632,7 @@ func graphNodesAreResourceInstancesInDifferentInstancesOfSameModule(a, b dag.Ver } aModInst := aRI.ResourceInstanceAddr().Module bModInst := bRI.ResourceInstanceAddr().Module - aMod := aModInst.Module() - bMod := bModInst.Module() - if !aMod.Equal(bMod) { + if !aModInst.HasSameModule(bModInst) { return false } return !aModInst.Equal(bModInst) diff --git a/internal/tofu/node_resource_abstract_instance.go b/internal/tofu/node_resource_abstract_instance.go index afe1bf6afb..896405cf9a 100644 --- a/internal/tofu/node_resource_abstract_instance.go +++ b/internal/tofu/node_resource_abstract_instance.go @@ -146,7 +146,7 @@ func (n *NodeAbstractResourceInstance) resolveProvider(ctx EvalContext, hasExpan } else { // Resolved from module instance moduleInstanceForKey := n.Addr.Module[:len(n.ResolvedProvider.KeyModule)] - if !moduleInstanceForKey.Module().Equal(n.ResolvedProvider.KeyModule) { + if !moduleInstanceForKey.IsForModule(n.ResolvedProvider.KeyModule) { panic(fmt.Sprintf("Invalid module key expression location %s in resource %s", n.ResolvedProvider.KeyModule, n.Addr)) } @@ -1974,9 +1974,8 @@ func (n *NodeAbstractResourceInstance) dependenciesHavePendingChanges(ctx EvalCo for _, change := range changes.GetChangesForConfigResource(d) { changeModInst := change.Addr.Module - changeMod := changeModInst.Module() - if changeMod.Equal(nMod) && !changeModInst.Equal(nModInst) { + if changeModInst.IsForModule(nMod) && !changeModInst.Equal(nModInst) { // Dependencies are tracked by configuration address, which // means we may have changes from other instances of parent // modules. The actual reference can only take effect within diff --git a/internal/tofu/node_resource_deposed_test.go b/internal/tofu/node_resource_deposed_test.go index 2b0623a63c..abdfbeaef8 100644 --- a/internal/tofu/node_resource_deposed_test.go +++ b/internal/tofu/node_resource_deposed_test.go @@ -249,7 +249,7 @@ func initMockEvalContext(resourceAddrs string, deposedKey states.DeposedKey) (*M state := states.NewState() absResource := mustResourceInstanceAddr(resourceAddrs) - if !absResource.Module.Module().Equal(addrs.RootModule) { + if !absResource.Module.IsRoot() { state.EnsureModule(addrs.RootModuleInstance.Child(absResource.Module[0].Name, absResource.Module[0].InstanceKey)) } diff --git a/internal/tofu/node_resource_plan_orphan_test.go b/internal/tofu/node_resource_plan_orphan_test.go index 8f81f50da3..2c5c389bc8 100644 --- a/internal/tofu/node_resource_plan_orphan_test.go +++ b/internal/tofu/node_resource_plan_orphan_test.go @@ -92,7 +92,7 @@ func TestNodeResourcePlanOrphan_Execute(t *testing.T) { state := states.NewState() absResource := mustResourceInstanceAddr(test.nodeAddress) - if !absResource.Module.Module().Equal(addrs.RootModule) { + if !absResource.Module.IsRoot() { state.EnsureModule(addrs.RootModuleInstance.Child(absResource.Module[0].Name, absResource.Module[0].InstanceKey)) } diff --git a/internal/tofu/transform_removed_modules.go b/internal/tofu/transform_removed_modules.go index f9405a0cc8..fa24b6f0cc 100644 --- a/internal/tofu/transform_removed_modules.go +++ b/internal/tofu/transform_removed_modules.go @@ -33,7 +33,8 @@ func (t *RemovedModuleTransformer) Transform(g *Graph) error { if cc != nil { continue } - removed[m.Addr.Module().String()] = m.Addr.Module() + mod := m.Addr.Module() + removed[mod.String()] = mod log.Printf("[DEBUG] %s is no longer in configuration\n", m.Addr) }