diff --git a/.changes/unreleased/BUG FIXES-20250123-150746.yaml b/.changes/unreleased/BUG FIXES-20250123-150746.yaml new file mode 100644 index 0000000000..1003a3ec9b --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20250123-150746.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: Refreshed state was not used in the plan for orphaned resource instances +time: 2025-01-23T15:07:46.789595-05:00 +custom: + Issue: "36394" diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index 77cad9e845..f1c5efc976 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -6315,3 +6315,47 @@ resource "aws_instance" "foo" { t.Errorf("unexpected error message\ngot: %s\nwant substring: %s", got, want) } } + +func TestContext2Plan_orphanUpdateInstance(t *testing.T) { + // ean orphaned instance should still reflect the refreshed state in the plan + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "test_object" "a" { + for_each = {} +} +`, + }) + + p := simpleMockProvider() + p.ReadResourceFn = func(req providers.ReadResourceRequest) (resp providers.ReadResourceResponse) { + state := req.PriorState.AsValueMap() + state["test_string"] = cty.StringVal("new") + resp.NewState = cty.ObjectVal(state) + return resp + } + + state := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr(`test_object.a["old"]`), &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"test_string":"old"}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`)) + }) + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + plan, diags := ctx.Plan(m, state, DefaultPlanOpts) + assertNoErrors(t, diags) + + resourceType := p.GetProviderSchemaResponse.ResourceTypes["test_object"].Block.ImpliedType() + change, err := plan.Changes.ResourceInstance(mustResourceInstanceAddr(`test_object.a["old"]`)).Decode(resourceType) + if err != nil { + t.Fatal(err) + } + if change.Before.GetAttr("test_string").AsString() != "new" { + t.Fatalf("resource before value not refreshed in plan: %#v\n", change.Before) + } +} diff --git a/internal/terraform/node_resource_plan_orphan.go b/internal/terraform/node_resource_plan_orphan.go index 9e64977c19..51364049f2 100644 --- a/internal/terraform/node_resource_plan_orphan.go +++ b/internal/terraform/node_resource_plan_orphan.go @@ -122,22 +122,6 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon } } - shouldDefer := ctx.Deferrals().ShouldDeferResourceInstanceChanges(n.Addr, n.Dependencies) - - var change *plans.ResourceInstanceChange - var pDiags tfdiags.Diagnostics - var deferred *providers.Deferred - if forget { - change, pDiags = n.planForget(ctx, oldState, "") - diags = diags.Append(pDiags) - } else { - change, deferred, pDiags = n.planDestroy(ctx, oldState, "") - diags = diags.Append(pDiags) - } - if diags.HasErrors() { - return diags - } - if !n.skipRefresh && !forget { // Refresh this instance even though it is going to be destroyed, in // order to catch missing resources. If this is a normal plan, @@ -153,12 +137,7 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon oldState = refreshedState - if deferred == nil { - // set the overall deferred status if it wasn't already set. - deferred = refreshDeferred - } - - if deferred == nil && !shouldDefer { + if refreshDeferred == nil { // only update the state if we're not deferring the change diags = diags.Append(n.writeResourceInstanceState(ctx, refreshedState, refreshState)) if diags.HasErrors() { @@ -167,6 +146,22 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon } } + shouldDefer := ctx.Deferrals().ShouldDeferResourceInstanceChanges(n.Addr, n.Dependencies) + + var change *plans.ResourceInstanceChange + var pDiags tfdiags.Diagnostics + var deferred *providers.Deferred + if forget { + change, pDiags = n.planForget(ctx, oldState, "") + diags = diags.Append(pDiags) + } else { + change, deferred, pDiags = n.planDestroy(ctx, oldState, "") + diags = diags.Append(pDiags) + } + if diags.HasErrors() { + return diags + } + // We might be able to offer an approximate reason for why we are // planning to delete this object. (This is best-effort; we might // sometimes not have a reason.)