fix: provider not initialized in some cases (mostly, deposed) (#2335)

Signed-off-by: ollevche <ollevche@gmail.com>
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Co-authored-by: Martin Atkins <mart@degeneration.co.uk>

CHANGELOG.md

@@ -16,9 +16,7 @@ ENHANCEMENTS:
 * State encryption now supports using external programs as key providers. Additionally, the PBKDF2 key provider now supports chaining via the `chain` parameter. ([#2023](https://github.com/opentofu/opentofu/pull/2023))
 
 BUG FIXES:
-* `tofu init` command does not attempt to read encryption keys when `-backend=false` flag is set. (https://github.com/opentofu/opentofu/pull/2293)
 
-* Changes in `create_before_destroy` for resources which require replacement are now properly handled when refresh is disabled. ([#2248](https://github.com/opentofu/opentofu/pull/2248))
 
 ## Previous Releases
 

internal/tofu/context_apply2_test.go

@@ -621,6 +621,61 @@ resource "test_object" "x" {
 
 }
 
+// This test is a copy and paste from TestContext2Apply_destroyWithDeposed
+// with modifications to test the same scenario with a dynamic provider instance.
+func TestContext2Apply_destroyWithDeposedWithDynamicProvider(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+provider "test" {
+  alias = "for_eached"
+  for_each = {a: {}}
+}
+
+resource "test_object" "x" {
+  test_string = "ok"
+  lifecycle {
+    create_before_destroy = true
+  }
+  provider = test.for_eached["a"]
+}`,
+	})
+
+	p := simpleMockProvider()
+
+	deposedKey := states.NewDeposedKey()
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceDeposed(
+		mustResourceInstanceAddr("test_object.x").Resource,
+		deposedKey,
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectTainted,
+			AttrsJSON: []byte(`{"test_string":"deposed"}`),
+		},
+		mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"].for_eached`),
+		addrs.StringKey("a"),
+	)
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(context.Background(), m, state, &PlanOpts{
+		Mode: plans.DestroyMode,
+	})
+	if diags.HasErrors() {
+		t.Fatalf("plan: %s", diags.Err())
+	}
+
+	_, diags = ctx.Apply(context.Background(), plan, m)
+	if diags.HasErrors() {
+		t.Fatalf("apply: %s", diags.Err())
+	}
+}
+
 func TestContext2Apply_nullableVariables(t *testing.T) {
 	m := testModule(t, "apply-nullable-variables")
 	state := states.NewState()
@@ -2379,6 +2434,67 @@ func TestContext2Apply_forgetOrphanAndDeposed(t *testing.T) {
 	}
 }
 
+// This test is a copy and paste from TestContext2Apply_forgetOrphanAndDeposed
+// with modifications to test the same scenario with  a dynamic provider instance.
+func TestContext2Apply_forgetOrphanAndDeposedWithDynamicProvider(t *testing.T) {
+	desposedKey := states.DeposedKey("deposed")
+	addr := "aws_instance.baz"
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+			provider aws {
+				alias = "for_eached"
+				for_each = {a: {}}
+			}
+
+			removed {
+				from = aws_instance.baz
+			}
+		`,
+	})
+	p := testProvider("aws")
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr(addr).Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"bar"}`),
+		},
+		mustProviderConfig(`provider["registry.opentofu.org/hashicorp/aws"].for_eached`),
+		addrs.StringKey("a"),
+	)
+	root.SetResourceInstanceDeposed(
+		mustResourceInstanceAddr(addr).Resource,
+		desposedKey,
+		&states.ResourceInstanceObjectSrc{
+			Status:       states.ObjectTainted,
+			AttrsJSON:    []byte(`{"id":"bar"}`),
+			Dependencies: []addrs.ConfigResource{},
+		},
+		mustProviderConfig(`provider["registry.opentofu.org/hashicorp/aws"].for_eached`),
+		addrs.StringKey("a"),
+	)
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+		},
+	})
+
+	p.PlanResourceChangeFn = testDiffFn
+
+	plan, diags := ctx.Plan(context.Background(), m, state, DefaultPlanOpts)
+	assertNoErrors(t, diags)
+
+	s, diags := ctx.Apply(context.Background(), plan, m)
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+
+	if !s.Empty() {
+		t.Fatalf("State should be empty")
+	}
+}
+
 func TestContext2Apply_providerExpandWithTargetOrExclude(t *testing.T) {
 	// This test is covering a potentially-tricky interaction between the
 	// logic that updates the provider instance references for resource

internal/tofu/context_plan2_test.go

@@ -7042,6 +7042,88 @@ import {
 	}
 }
 
+func TestContext2Plan_providerForEachWithOrphanResourceInstanceNotUsingForEach(t *testing.T) {
+	// This test is to cover the bug reported in this issue:
+	//    https://github.com/opentofu/opentofu/issues/2334
+	//
+	// The bug there was that OpenTofu was failing to evaluate the provider
+	// instance key expression for a graph node representing a "orphaned"
+	// resource instance, and was thus always treating them as belonging to
+	// the no-key instance of the given provider. Since a for_each provider
+	// has no instance without an instance key, that caused an error trying
+	// to use a non-existent provider instance.
+
+	instAddr := addrs.Resource{
+		Mode: addrs.ManagedResourceMode,
+		Type: "test_thing",
+		Name: "a",
+	}.Instance(addrs.StringKey("orphaned")).Absolute(addrs.RootModuleInstance)
+	providerConfigAddr := addrs.AbsProviderConfig{
+		Module:   addrs.RootModule,
+		Provider: addrs.NewBuiltInProvider("test"),
+		Alias:    "multi",
+	}
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+			terraform {
+				required_providers {
+					test = {
+						source = "terraform.io/builtin/test"
+					}
+				}
+			}
+
+			provider "test" {
+				alias    = "multi"
+				for_each = toset(["a"])
+			}
+
+			resource "test_thing" "a" {
+				for_each = toset([])
+				provider = test.multi["a"]
+			}
+		`,
+	})
+	s := states.BuildState(func(ss *states.SyncState) {
+		ss.SetResourceInstanceCurrent(
+			instAddr,
+			&states.ResourceInstanceObjectSrc{
+				Status:    states.ObjectReady,
+				AttrsJSON: []byte(`{}`),
+			},
+			providerConfigAddr,
+			addrs.NoKey, // NOTE: The prior state object is associated with a no-key instance of provider.test
+		)
+	})
+	p := &MockProvider{}
+	p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
+		Provider: providers.Schema{
+			Block: &configschema.Block{},
+		},
+		ResourceTypes: map[string]providers.Schema{
+			"test_thing": {
+				Block: &configschema.Block{},
+			},
+		},
+	}
+
+	tofuCtx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			providerConfigAddr.Provider: testProviderFuncFixed(p),
+		},
+	})
+	_, diags := tofuCtx.Plan(context.Background(), m, s, DefaultPlanOpts)
+	err := diags.Err()
+	if err == nil {
+		t.Fatalf("unexpected success; want an error about %s not being declared", providerConfigAddr.InstanceString(addrs.NoKey))
+	}
+	got := err.Error()
+	wantSubstring := `To proceed, return to your previous single-instance configuration for provider["terraform.io/builtin/test"].multi and ensure that test_thing.a["orphaned"] has been destroyed or forgotten before using for_each with this provider, or change the resource configuration to still declare an instance with the key ["orphaned"].`
+	if !strings.Contains(got, wantSubstring) {
+		t.Errorf("missing expected error message\ngot:\n%s\nwant substring: %s", got, wantSubstring)
+	}
+}
+
 func TestContext2Plan_plannedState(t *testing.T) {
 	addr := mustResourceInstanceAddr("test_object.a")
 	m := testModuleInline(t, map[string]string{

internal/tofu/node_resource_abstract_instance.go

@@ -107,7 +107,7 @@ func (n *NodeAbstractResourceInstance) References() []*addrs.Reference {
 	return nil
 }
 
-func (n *NodeAbstractResourceInstance) resolveProvider(ctx EvalContext, hasExpansionData bool) tfdiags.Diagnostics {
+func (n *NodeAbstractResourceInstance) resolveProvider(ctx EvalContext, hasExpansionData bool, deposedKey states.DeposedKey) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
 
 	log.Printf("[TRACE] Resolving provider key for %s", n.Addr)
@@ -204,19 +204,75 @@ func (n *NodeAbstractResourceInstance) resolveProvider(ctx EvalContext, hasExpan
 		return nil
 	}
 
-	if n.ResolvedProviderKey == nil {
-		// Probably an OpenTofu bug
-		return diags.Append(fmt.Errorf("provider %s not initialized for resource %s", n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr))
+	// If we get here then the provider instance address tracked in the state refers to
+	// an instance of the provider configuration that is no longer declared in the
+	// configuration. This could either mean that the provider was previously using
+	// for_each but one of the keys has been removed, or that the "for_each"-ness
+	// of the provider configuration has changed since this state snapshot was created.
+	// There are therefore two different error cases to handle, although we need
+	// slightly different messaging for deposed vs. orphaned instances.
+	if deposedKey == states.NotDeposed {
+		if n.ResolvedProviderKey != nil {
+			// We're associated with an for_each instance key that isn't declared anymore.
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Provider instance not present",
+				fmt.Sprintf(
+					"To work with %s its original provider instance at %s is required, but it has been removed. This occurs when an element is removed from the provider configuration's for_each collection while objects created by that the associated provider instance still exist in the state. Re-add the for_each element to destroy %s, after which you can remove the provider configuration again.\n\nThis is commonly caused by using the same for_each collection both for a resource (or its containing module) and its associated provider configuration. To successfully remove an instance of a resource it must be possible to remove the corresponding element from the resource's for_each collection while retaining the corresponding element in the provider's for_each collection.",
+					n.Addr, n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr,
+				),
+			))
+		} else {
+			// We're associated with the no-key instance of a provider configuration, which
+			// suggests that someone is in the process of adopting provider for_each for
+			// a provider configuration that didn't previously use it but has some
+			// orphaned resource instance objects in the state that need to have
+			// their destroy completed first.
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Provider instance not present",
+				fmt.Sprintf(
+					"To work with %s its original provider instance at %s is required, but it has been removed. This suggests that you've added for_each to this provider configuration while there are existing instances of %s that need to be destroyed by the original single-instance provider configuration.\n\nTo proceed, return to your previous single-instance configuration for %s and ensure that %s has been destroyed or forgotten before using for_each with this provider, or change the resource configuration to still declare an instance with the key %s.",
+					n.Addr, n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr.ContainingResource(),
+					n.ResolvedProvider.ProviderConfig, n.Addr, n.Addr.Resource.Key,
+				),
+			))
+		}
+	} else {
+		if n.ResolvedProviderKey != nil {
+			// We're associated with an for_each instance key that isn't declared anymore.
+			// This particualr case is similar to the non-deposed variant above, but we
+			// mention the deposed key in the message and drop the irrelevant note about
+			// using the same for_each for the resource and the provider, since deposed
+			// objects are caused by a failed create_before_destroy (a kind of "replace")
+			// rather than by entirely removing an instance.
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Provider instance not present",
+				fmt.Sprintf(
+					"To work with %s's deposed object %s its original provider instance at %s is required, but it has been removed. This occurs when an element is removed from the provider configuration's for_each collection while objects created by that the associated provider instance still exist in the state. Re-add the for_each element to destroy this deposed object for %s, after which you can remove the provider configuration again.",
+					n.Addr, deposedKey, n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr,
+				),
+			))
+		} else {
+			// We're associated with the no-key instance of a provider configuration, which
+			// suggests that someone is in the process of adopting provider for_each for
+			// a provider configuration that didn't previously use it but has some
+			// deposed resource instance objects in the state that need to have
+			// their destroy completed first.
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Provider instance not present",
+				fmt.Sprintf(
+					"To work with %s's deposed object %s its original provider instance at %s is required, but it has been removed. This suggests that you've added for_each to this provider configuration while there are existing deposed objects of %s that need to be destroyed by the original single-instance provider configuration.\n\nTo proceed, return to your previous single-instance configuration for %s and ensure that all deposed instances of %s have been destroyed before using for_each with this provider.",
+					n.Addr, deposedKey, n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey),
+					n.Addr,
+					n.ResolvedProvider.ProviderConfig, n.Addr,
+				),
+			))
+		}
 	}
-
-	return diags.Append(tfdiags.Sourceless(
-		tfdiags.Error,
-		"Provider instance not present",
-		fmt.Sprintf(
-			"To work with %s its original provider instance at %s is required, but it has been removed. This occurs when an element is removed from the provider configuration's for_each collection while objects created by that the associated provider instance still exist in the state. Re-add the for_each element to destroy %s, after which you can remove the provider configuration again.\n\nThis is commonly caused by using the same for_each collection both for a resource (or its containing module) and its associated provider configuration. To successfully remove an instance of a resource it must be possible to remove the corresponding element from the resource's for_each collection while retaining the corresponding element in the provider's for_each collection.",
-			n.Addr, n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr,
-		),
-	))
+	return diags
 }
 
 // StateDependencies returns the dependencies which will be saved in the state

internal/tofu/node_resource_apply_instance.go

@@ -141,7 +141,7 @@ func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperatio
 		return diags
 	}
 
-	diags = n.resolveProvider(ctx, true)
+	diags = n.resolveProvider(ctx, true, states.NotDeposed)
 	if diags.HasErrors() {
 		return diags
 	}

internal/tofu/node_resource_deposed.go

@@ -87,6 +87,11 @@ func (n *NodePlanDeposedResourceInstanceObject) References() []*addrs.Reference
 func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
 	log.Printf("[TRACE] NodePlanDeposedResourceInstanceObject: planning %s deposed object %s", n.Addr, n.DeposedKey)
 
+	diags = n.resolveProvider(ctx, false, n.DeposedKey)
+	if diags.HasErrors() {
+		return diags
+	}
+
 	// Read the state for the deposed resource instance
 	state, err := n.readResourceInstanceStateDeposed(ctx, n.Addr, n.DeposedKey)
 	diags = diags.Append(err)
@@ -252,6 +257,11 @@ func (n *NodeDestroyDeposedResourceInstanceObject) ModifyCreateBeforeDestroy(v b
 func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
 	var change *plans.ResourceInstanceChange
 
+	diags = n.resolveProvider(ctx, false, n.DeposedKey)
+	if diags.HasErrors() {
+		return diags
+	}
+
 	// Read the state for the deposed resource instance
 	state, err := n.readResourceInstanceStateDeposed(ctx, n.Addr, n.DeposedKey)
 	if err != nil {
@@ -399,6 +409,11 @@ func (n *NodeForgetDeposedResourceInstanceObject) References() []*addrs.Referenc
 
 // GraphNodeExecutable impl.
 func (n *NodeForgetDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
+	diags = n.resolveProvider(ctx, false, n.DeposedKey)
+	if diags.HasErrors() {
+		return diags
+	}
+
 	// Read the state for the deposed resource instance
 	state, err := n.readResourceInstanceStateDeposed(ctx, n.Addr, n.DeposedKey)
 	if err != nil {

internal/tofu/node_resource_destroy.go

@@ -143,7 +143,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 	// Eval info is different depending on what kind of resource this is
 	switch addr.Resource.Resource.Mode {
 	case addrs.ManagedResourceMode:
-		diags = n.resolveProvider(ctx, false)
+		diags = n.resolveProvider(ctx, false, states.NotDeposed)
 		if diags.HasErrors() {
 			return diags
 		}

internal/tofu/node_resource_forget.go

@@ -53,6 +53,11 @@ func (n *NodeForgetResourceInstance) Execute(ctx EvalContext, op walkOperation)
 		log.Printf("[WARN] NodeForgetResourceInstance for %s with no state", addr)
 	}
 
+	diags = n.resolveProvider(ctx, false, states.NotDeposed)
+	if diags.HasErrors() {
+		return diags
+	}
+
 	var state *states.ResourceInstanceObject
 
 	state, readDiags := n.readResourceInstanceState(ctx, addr)

internal/tofu/node_resource_import.go

@@ -92,7 +92,7 @@ func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) (diags
 			ResolvedProvider: n.ResolvedProvider,
 		},
 	}
-	diags = diags.Append(asAbsNode.resolveProvider(ctx, true))
+	diags = diags.Append(asAbsNode.resolveProvider(ctx, true, states.NotDeposed))
 	if diags.HasErrors() {
 		return diags
 	}

internal/tofu/node_resource_plan_destroy.go

@@ -47,7 +47,7 @@ func (n *NodePlanDestroyableResourceInstance) DestroyAddr() *addrs.AbsResourceIn
 func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
 	addr := n.ResourceInstanceAddr()
 
-	diags = diags.Append(n.resolveProvider(ctx, false))
+	diags = diags.Append(n.resolveProvider(ctx, false, states.NotDeposed))
 	if diags.HasErrors() {
 		return diags
 	}

internal/tofu/node_resource_plan_instance.go

@@ -86,7 +86,7 @@ var (
 func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics {
 	addr := n.ResourceInstanceAddr()
 
-	diags := n.resolveProvider(ctx, true)
+	diags := n.resolveProvider(ctx, true, states.NotDeposed)
 	if diags.HasErrors() {
 		return diags
 	}

internal/tofu/node_resource_plan_orphan.go

@@ -57,7 +57,7 @@ func (n *NodePlannableResourceInstanceOrphan) Execute(ctx EvalContext, op walkOp
 	// Eval info is different depending on what kind of resource this is
 	switch addr.Resource.Resource.Mode {
 	case addrs.ManagedResourceMode:
-		diags := n.resolveProvider(ctx, true)
+		diags := n.resolveProvider(ctx, true, states.NotDeposed)
 		if diags.HasErrors() {
 			return diags
 		}