diff --git a/CHANGELOG.md b/CHANGELOG.md index 117476b83e..7c75cc15b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ UPGRADE NOTES: ENHANCEMENTS: +* Add implicit moves for modules using meta-arguments. ([#3327](https://github.com/opentofu/opentofu/pull/3327)) * The conditional `enabled` field is now supported for modules within the `lifecycle` block. ([#3244](https://github.com/opentofu/opentofu/pull/3244)) * The conditional `enabled` field is now supported for all types of resources within the `lifecycle` block. ([#3042](https://github.com/opentofu/opentofu/pull/3042)) * OpenTofu will now suggest using `-exclude` if a provider reports that it cannot create a plan for a particular resource instance due to values that won't be known until the apply phase. ([#2643](https://github.com/opentofu/opentofu/pull/2643)) diff --git a/internal/addrs/module_instance.go b/internal/addrs/module_instance.go index b267a5a747..f834930add 100644 --- a/internal/addrs/module_instance.go +++ b/internal/addrs/module_instance.go @@ -505,7 +505,7 @@ func (m ModuleInstance) HasSameModule(other ModuleInstance) bool { // 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. +// a [Module] value from the receiver just to compare it and then throw it away. func (m ModuleInstance) IsForModule(module Module) bool { if len(m) != len(module) { return false diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index b681365668..e9be1da4b6 100644 --- a/internal/addrs/move_endpoint_module.go +++ b/internal/addrs/move_endpoint_module.go @@ -67,11 +67,11 @@ type MoveEndpointInModule struct { // have somewhat-related-but-imprecise source ranges, typically referring to // some general configuration construct that implied the statement, because // by definition there is no explicit move endpoint expression in this case. -func ImpliedMoveStatementEndpoint(addr AbsResourceInstance, rng tfdiags.SourceRange) *MoveEndpointInModule { +func ImpliedMoveStatementEndpoint(addr AbsMoveable, rng tfdiags.SourceRange) *MoveEndpointInModule { // implied move endpoints always belong to the root module, because each - // one refers to a single resource instance inside a specific module - // instance, rather than all instances of the module where the resource - // was declared. + // one refers to a single resource instance inside a specific module instance + // or a single module instance, rather than all instances of the module where + // the resource was declared. return &MoveEndpointInModule{ SourceRange: rng, module: RootModule, diff --git a/internal/refactoring/move_statement.go b/internal/refactoring/move_statement.go index 445d343bff..49459f99e4 100644 --- a/internal/refactoring/move_statement.go +++ b/internal/refactoring/move_statement.go @@ -85,12 +85,17 @@ func findMoveStatements(cfg *configs.Config, into []MoveStatement) []MoveStateme // // We should think very hard before adding any _new_ implication rules for // moved statements. -func ImpliedMoveStatements(rootCfg *configs.Config, prevRunState *states.State, explicitStmts []MoveStatement) []MoveStatement { - return impliedMoveStatements(rootCfg, prevRunState, explicitStmts, nil) -} - -func impliedMoveStatements(cfg *configs.Config, prevRunState *states.State, explicitStmts []MoveStatement, into []MoveStatement) []MoveStatement { +func ImpliedMoveStatements(cfg *configs.Config, prevRunState *states.State, explicitStmts []MoveStatement) []MoveStatement { modAddr := cfg.Path + into := make([]MoveStatement, 0) + + // Create implied move statements for module calls. We're typically + // looking for module where meta-arguments were changed to see if they + // can be moved without an explicit moved block. If there's an existing + // explicit move statement for the module, we don't create an implied move statement. + for modCallName, modCallCfg := range cfg.Module.ModuleCalls { + into = append(into, impliedMoveStatementsForModules(prevRunState, cfg, modCallName, modCallCfg, explicitStmts)...) + } // There can be potentially many instances of the module, so we need // to consider each of them separately. @@ -100,66 +105,144 @@ func impliedMoveStatements(cfg *configs.Config, prevRunState *states.State, expl // instance where the configuration _doesn't_ have count set. // If so, we'll generate a statement replacing no-key with zero-key or // vice-versa. - for _, rState := range modState.Resources { - rAddr := rState.Addr - rCfg := cfg.Module.ResourceByAddr(rAddr.Resource) - if rCfg == nil { - // If there's no configuration at all then there can't be any - // automatic move fixup to do. - continue - } - approxSrcRange := tfdiags.SourceRangeFromHCL(rCfg.DeclRange) - - // NOTE: We're intentionally not checking to see whether the - // "to" addresses in our implied statements already have - // instances recorded in state, because ApplyMoves should - // deal with such conflicts in a deterministic way for both - // explicit and implicit moves, and we'd rather have that - // handled all in one place. - - var fromKey, toKey addrs.InstanceKey - - switch { - case rCfg.Count != nil: - // If we have a count expression then we'll use _that_ as - // a slightly-more-precise approximate source range. - approxSrcRange = tfdiags.SourceRangeFromHCL(rCfg.Count.Range()) - - if riState := rState.Instances[addrs.NoKey]; riState != nil { - fromKey = addrs.NoKey - toKey = addrs.IntKey(0) - } - case rCfg.Count == nil && rCfg.ForEach == nil: // no repetition at all - if riState := rState.Instances[addrs.IntKey(0)]; riState != nil { - fromKey = addrs.IntKey(0) - toKey = addrs.NoKey - } - } - - if fromKey != toKey { - // We mustn't generate an implied statement if the user already - // wrote an explicit statement referring to this resource, - // because they may wish to select an instance key other than - // zero as the one to retain. - if !haveMoveStatementForResource(rAddr, explicitStmts) { - into = append(into, MoveStatement{ - From: addrs.ImpliedMoveStatementEndpoint(rAddr.Instance(fromKey), approxSrcRange), - To: addrs.ImpliedMoveStatementEndpoint(rAddr.Instance(toKey), approxSrcRange), - DeclRange: approxSrcRange, - Implied: true, - }) - } - } - } + into = append(into, impliedMoveStatementsForModuleResources(cfg, modState, explicitStmts)...) } for _, childCfg := range cfg.Children { - into = impliedMoveStatements(childCfg, prevRunState, explicitStmts, into) + into = append(into, ImpliedMoveStatements(childCfg, prevRunState, explicitStmts)...) } return into } +// impliedMoveStatementsForModules creates implied move statements for module calls. +func impliedMoveStatementsForModules( + prevRunState *states.State, + parentCfg *configs.Config, + modCallName string, + modCallCfg *configs.ModuleCall, + explicitStmts []MoveStatement, +) []MoveStatement { + var into []MoveStatement + var toKey addrs.InstanceKey + approxSrcRange := tfdiags.SourceRangeFromHCL(modCallCfg.DeclRange) + + // Use the configuration to determine the instance key to + // use for the implied move `To statement. + switch { + case modCallCfg.Count != nil: + // If we have a count expression then we'll use _that_ as + // a slightly-more-precise approximate source range. + approxSrcRange = tfdiags.SourceRangeFromHCL(modCallCfg.Count.Range()) + toKey = addrs.IntKey(0) + case modCallCfg.Count == nil && modCallCfg.ForEach == nil: // no repetition at all + toKey = addrs.NoKey + default: + // Other combinations of meta-arguments are not supported. + return into + } + + // Get the module address + modAddr := parentCfg.Path.Child(modCallName) + + // Iterate over all module instances, that can be generated by + // meta-arguments like count or for_each on the saved state for the module. + for _, modState := range prevRunState.ModuleInstances(modAddr) { + callerAddr, callAddr := modState.Addr.CallInstance() + absCallAddr := addrs.AbsModuleCall{ + Module: callerAddr, + Call: callAddr.Call, + } + + // Only one instance of the module can be dealt, because moving from single + // to repeated can't support multiple instances. The other instances are going + // to be removed or created, depending on the direction of the change. + fromKey := callAddr.Key + if fromKey != addrs.NoKey && fromKey != addrs.IntKey(0) { + continue + } + + // We mustn't generate an implied statement if the user already + // wrote an explicit statement referring to this module, + // because they may wish to select an instance key other than + // zero as the one to retain. If the instance key from the state + // equals the instance key from the configuration, then we don't + // need to generate an implied statement. + if fromKey == toKey || haveMoveStatementForModule(modState.Addr, explicitStmts) { + continue + } + + fromAddr := absCallAddr.Instance(fromKey) + toAddr := absCallAddr.Instance(toKey) + + into = append(into, MoveStatement{ + From: addrs.ImpliedMoveStatementEndpoint(fromAddr, approxSrcRange), + To: addrs.ImpliedMoveStatementEndpoint(toAddr, approxSrcRange), + DeclRange: approxSrcRange, + Implied: true, + }) + } + return into +} + +// impliedMoveStatementsForModuleResources creates implied move statements for module resources. +func impliedMoveStatementsForModuleResources(cfg *configs.Config, modState *states.Module, explicitStmts []MoveStatement) []MoveStatement { + var into []MoveStatement + + for _, rState := range modState.Resources { + rAddr := rState.Addr + rCfg := cfg.Module.ResourceByAddr(rAddr.Resource) + if rCfg == nil { + // If there's no configuration at all then there can't be any + // automatic move fixup to do. + continue + } + approxSrcRange := tfdiags.SourceRangeFromHCL(rCfg.DeclRange) + + // NOTE: We're intentionally not checking to see whether the + // "to" addresses in our implied statements already have + // instances recorded in state, because ApplyMoves should + // deal with such conflicts in a deterministic way for both + // explicit and implicit moves, and we'd rather have that + // handled all in one place. + + var fromKey, toKey addrs.InstanceKey + + switch { + case rCfg.Count != nil: + // If we have a count expression then we'll use _that_ as + // a slightly-more-precise approximate source range. + approxSrcRange = tfdiags.SourceRangeFromHCL(rCfg.Count.Range()) + + if riState := rState.Instances[addrs.NoKey]; riState != nil { + fromKey = addrs.NoKey + toKey = addrs.IntKey(0) + } + case rCfg.Count == nil && rCfg.ForEach == nil: // no repetition at all + if riState := rState.Instances[addrs.IntKey(0)]; riState != nil { + fromKey = addrs.IntKey(0) + toKey = addrs.NoKey + } + } + + if fromKey != toKey { + // We mustn't generate an implied statement if the user already + // wrote an explicit statement referring to this resource, + // because they may wish to select an instance key other than + // zero as the one to retain. + if !haveMoveStatementForResource(rAddr, explicitStmts) { + into = append(into, MoveStatement{ + From: addrs.ImpliedMoveStatementEndpoint(rAddr.Instance(fromKey), approxSrcRange), + To: addrs.ImpliedMoveStatementEndpoint(rAddr.Instance(toKey), approxSrcRange), + DeclRange: approxSrcRange, + Implied: true, + }) + } + } + } + return into +} + func (s *MoveStatement) ObjectKind() addrs.MoveEndpointKind { // addrs.UnifyMoveEndpoints guarantees that both of our addresses have // the same kind, so we can just arbitrary use From and assume To will @@ -189,3 +272,21 @@ func haveMoveStatementForResource(addr addrs.AbsResource, stmts []MoveStatement) } return false } + +func haveMoveStatementForModule(addr addrs.ModuleInstance, stmts []MoveStatement) bool { + // This is not a particularly optimal way to answer this question, + // particularly since our caller calls this function in a loop already, + // but we expect the total number of explicit statements to be small + // in any reasonable OpenTofu configuration and so a more complicated + // approach wouldn't be justified here. + + for _, stmt := range stmts { + if stmt.From.SelectsModule(addr) { + return true + } + if stmt.To.SelectsModule(addr) { + return true + } + } + return false +} diff --git a/internal/refactoring/move_statement_test.go b/internal/refactoring/move_statement_test.go index 1c500df00d..bf17788631 100644 --- a/internal/refactoring/move_statement_test.go +++ b/internal/refactoring/move_statement_test.go @@ -77,6 +77,12 @@ func TestImpliedMoveStatements(t *testing.T) { }.Absolute(addrs.RootModuleInstance) } + moduleAddr := func(name string) addrs.AbsModuleCall { + return addrs.ModuleCall{ + Name: name, + }.Absolute(addrs.RootModuleInstance) + } + nestedResourceAddr := func(mod, name string) addrs.AbsResource { return addrs.Resource{ Mode: addrs.ManagedResourceMode, @@ -178,6 +184,13 @@ func TestImpliedMoveStatements(t *testing.T) { providerAddr, addrs.NoKey, ) + + s.SetResourceInstanceCurrent( + nestedResourceAddr("child_count_one", "now_count").Instance(addrs.NoKey), + instObjState(), + providerAddr, + addrs.IntKey(0), + ) }) explicitStmts := FindMoveStatements(rootCfg) @@ -244,6 +257,17 @@ func TestImpliedMoveStatements(t *testing.T) { End: tfdiags.SourcePos{Line: 46, Column: 27, Byte: 832}, }, }, + // Implied move from an enabled module to a count module + { + From: addrs.ImpliedMoveStatementEndpoint(moduleAddr("child_count_one").Instance(addrs.NoKey), tfdiags.SourceRange{}), + To: addrs.ImpliedMoveStatementEndpoint(moduleAddr("child_count_one").Instance(addrs.IntKey(0)), tfdiags.SourceRange{}), + Implied: true, + DeclRange: tfdiags.SourceRange{ + Filename: filepath.Join("testdata", "move-statement-implied", "move-statement-implied.tf"), + Start: tfdiags.SourcePos{Line: 58, Column: 12, Byte: 1088}, + End: tfdiags.SourcePos{Line: 58, Column: 13, Byte: 1089}, + }, + }, } sort.Slice(got, func(i, j int) bool { diff --git a/internal/refactoring/testdata/move-statement-implied/move-statement-implied.tf b/internal/refactoring/testdata/move-statement-implied/move-statement-implied.tf index 4ea628ea65..a7dd682f60 100644 --- a/internal/refactoring/testdata/move-statement-implied/move-statement-implied.tf +++ b/internal/refactoring/testdata/move-statement-implied/move-statement-implied.tf @@ -52,3 +52,8 @@ resource "foo" "ambiguous" { module "child" { source = "./child" } + +module "child_count_one" { + source = "./no-move-child" + count = 1 +} diff --git a/internal/refactoring/testdata/move-statement-implied/no-move-child/main.tf b/internal/refactoring/testdata/move-statement-implied/no-move-child/main.tf new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/tofu/context_plan2_test.go b/internal/tofu/context_plan2_test.go index 20af5756f0..cb20c21ec9 100644 --- a/internal/tofu/context_plan2_test.go +++ b/internal/tofu/context_plan2_test.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/checks" + "github.com/opentofu/opentofu/internal/configs" "github.com/zclconf/go-cty/cty" "github.com/opentofu/opentofu/internal/configs/configschema" @@ -3360,52 +3361,165 @@ output "output" { } } -func TestContext2Plan_moduleExpandOrphansResourceInstance(t *testing.T) { - // This test deals with the situation where a user has changed the - // repetition/expansion mode for a module call while there are already - // resource instances from the previous declaration in the state. - // - // This is conceptually just the same as removing the resources - // from the module configuration only for that instance, but the - // implementation of it ends up a little different because it's - // an entry in the resource address's _module path_ that we'll find - // missing, rather than the resource's own instance key, and so - // our analyses need to handle that situation by indicating that all - // of the resources under the missing module instance have zero - // instances, regardless of which resource in that module we might - // be asking about, and do so without tripping over any missing - // registrations in the instance expander that might lead to panics - // if we aren't careful. - // - // (For some history here, see https://github.com/hashicorp/terraform/issues/30110 ) - - addrNoKey := mustResourceInstanceAddr("module.child.test_object.a[0]") - addrZeroKey := mustResourceInstanceAddr("module.child[0].test_object.a[0]") - m := testModuleInline(t, map[string]string{ - "main.tf": ` - module "child" { - source = "./child" - count = 1 - } - `, - "child/main.tf": ` - resource "test_object" "a" { - count = 1 - } - `, - }) - - state := states.BuildState(func(s *states.SyncState) { - // Notice that addrNoKey is the address which lacks any instance key - // for module.child, and so that module instance doesn't match the - // call declared above with count = 1, and therefore the resource - // inside is "orphaned" even though the resource block actually - // still exists there. - s.SetResourceInstanceCurrent(addrNoKey, &states.ResourceInstanceObjectSrc{ - AttrsJSON: []byte(`{}`), - Status: states.ObjectReady, - }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) - }) +func TestContext2Plan_moduleImplicitMove(t *testing.T) { + // Modules are being moved implicitly to use the `enabled` field when nothing + // is declared on the block. Alternatively, they are implicitly being moved from + // using `enabled` as true or without declaring `enabled` to use count. + var tests = map[string]struct { + name string + expectedAddr addrs.AbsResourceInstance + prevAddr addrs.AbsResourceInstance + config *configs.Config + prevState *states.State + }{ + "from count-module single-resource to enabled-module single-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { source = "./child" }`, + "child/main.tf": `resource "test_object" "a" {}`, + }), + expectedAddr: mustResourceInstanceAddr("module.child.test_object.a"), + prevAddr: mustResourceInstanceAddr("module.child[0].test_object.a"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child[0].test_object.a"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.IntKey(0)) + }), + }, + "from count-module single-resource to enabled-module multiple-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { source = "./child" }`, + "child/main.tf": `resource "test_object" "a" { count = 1}`, + }), + expectedAddr: mustResourceInstanceAddr("module.child.test_object.a[0]"), + prevAddr: mustResourceInstanceAddr("module.child[0].test_object.a"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child[0].test_object.a"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.IntKey(0)) + }), + }, + "from count-module repeated-resource to enabled-module single-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { source = "./child" }`, + "child/main.tf": `resource "test_object" "a" {}`, + }), + expectedAddr: mustResourceInstanceAddr("module.child.test_object.a"), + prevAddr: mustResourceInstanceAddr("module.child[0].test_object.a[0]"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child[0].test_object.a[0]"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.IntKey(0)) + }), + }, + "from count-module repeated-resource to enabled-module multiple-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { source = "./child" }`, + "child/main.tf": `resource "test_object" "a" { count = 1 }`, + }), + expectedAddr: mustResourceInstanceAddr("module.child.test_object.a[0]"), + prevAddr: mustResourceInstanceAddr("module.child[0].test_object.a[0]"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child[0].test_object.a[0]"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.IntKey(0)) + }), + }, + "from enabled-module single-resource to count-module single-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { + source = "./child" + count = 1 + }`, + "child/main.tf": `resource "test_object" "a" {}`, + }), + expectedAddr: mustResourceInstanceAddr("module.child[0].test_object.a"), + prevAddr: mustResourceInstanceAddr("module.child.test_object.a"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child.test_object.a"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) + }), + }, + "from enabled-module single-resource to count-module multiple-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { + source = "./child" + count = 1 + }`, + "child/main.tf": `resource "test_object" "a" { count = 1 }`, + }), + expectedAddr: mustResourceInstanceAddr("module.child[0].test_object.a[0]"), + prevAddr: mustResourceInstanceAddr("module.child.test_object.a"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child.test_object.a"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) + }), + }, + "from enabled-module multiple-resource to count-module multiple-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": `module "child" { + source = "./child" + count = 1 + }`, + "child/main.tf": `resource "test_object" "a" { count = 1 }`, + }), + expectedAddr: mustResourceInstanceAddr("module.child[0].test_object.a[0]"), + prevAddr: mustResourceInstanceAddr("module.child.test_object.a[0]"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child.test_object.a[0]"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) + }), + }, + "from enabled-module multiple-resource to count-module single-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": ` + module "child" { + source = "./child" + count = 1 + }`, + "child/main.tf": `resource "test_object" "a" {}`, + }), + expectedAddr: mustResourceInstanceAddr("module.child[0].test_object.a"), + prevAddr: mustResourceInstanceAddr("module.child.test_object.a[0]"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child.test_object.a[0]"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) + }), + }, + "from nested enabled-module multiple-resource to count-module single-resource": { + config: testModuleInline(t, map[string]string{ + "main.tf": ` + module "child" { + source = "./child" + }`, + "child/main.tf": ` + module "grandchild" { + source = "./grandchild" + count = 1 + }`, + "child/grandchild/main.tf": `resource "test_object" "a" {}`, + }), + expectedAddr: mustResourceInstanceAddr("module.child.module.grandchild[0].test_object.a"), + prevAddr: mustResourceInstanceAddr("module.child.module.grandchild.test_object.a"), + prevState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("module.child.module.grandchild.test_object.a"), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) + }), + }, + } p := simpleMockProvider() ctx := testContext2(t, &ContextOpts{ @@ -3414,52 +3528,32 @@ func TestContext2Plan_moduleExpandOrphansResourceInstance(t *testing.T) { }, }) - plan, diags := ctx.Plan(context.Background(), m, state, &PlanOpts{ - Mode: plans.NormalMode, - }) - if diags.HasErrors() { - t.Fatalf("unexpected errors\n%s", diags.Err().Error()) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + plan, diags := ctx.Plan(context.Background(), test.config, test.prevState, DefaultPlanOpts) + if diags.HasErrors() { + t.Fatalf("unexpected errors\n%s", diags.Err().Error()) + } + + gotPlan := plan.Changes.ResourceInstance(test.expectedAddr) + if gotPlan == nil { + t.Fatalf("no plan for %s at all", test.expectedAddr) + } + + if got, want := gotPlan.Addr, test.expectedAddr; !got.Equal(want) { + t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want) + } + if got, want := gotPlan.PrevRunAddr, test.prevAddr; !got.Equal(want) { + t.Errorf("wrong previous run address\ngot: %s\nwant: %s", got, want) + } + if got, want := gotPlan.Action, plans.NoOp; got != want { + t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want) + } + if got, want := gotPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) + } + }) } - - t.Run(addrNoKey.String(), func(t *testing.T) { - instPlan := plan.Changes.ResourceInstance(addrNoKey) - if instPlan == nil { - t.Fatalf("no plan for %s at all", addrNoKey) - } - - if got, want := instPlan.Addr, addrNoKey; !got.Equal(want) { - t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want) - } - if got, want := instPlan.PrevRunAddr, addrNoKey; !got.Equal(want) { - t.Errorf("wrong previous run address\ngot: %s\nwant: %s", got, want) - } - if got, want := instPlan.Action, plans.Delete; got != want { - t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want) - } - if got, want := instPlan.ActionReason, plans.ResourceInstanceDeleteBecauseNoModule; got != want { - t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) - } - }) - - t.Run(addrZeroKey.String(), func(t *testing.T) { - instPlan := plan.Changes.ResourceInstance(addrZeroKey) - if instPlan == nil { - t.Fatalf("no plan for %s at all", addrZeroKey) - } - - if got, want := instPlan.Addr, addrZeroKey; !got.Equal(want) { - t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want) - } - if got, want := instPlan.PrevRunAddr, addrZeroKey; !got.Equal(want) { - t.Errorf("wrong previous run address\ngot: %s\nwant: %s", got, want) - } - if got, want := instPlan.Action, plans.Create; got != want { - t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want) - } - if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { - t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) - } - }) } func TestContext2Plan_resourcePreconditionPostcondition(t *testing.T) {