From cf34b0e6a92a310c864ed6ccf57567c02c0a4172 Mon Sep 17 00:00:00 2001
From: Ronny Orot <ronny.orot@gmail.com>
Date: Mon, 25 Nov 2024 23:52:03 +0200
Subject: [PATCH] Skip imports on `tofu destroy` (#2214)

Signed-off-by: Ronny Orot <ronny.orot@gmail.com>
---
 CHANGELOG.md                        |  1 +
 internal/tofu/context_plan2_test.go | 74 +++++++++++++++++++++++++++++
 internal/tofu/node_resource_plan.go |  7 ++-
 3 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 857522e689..357c06a4b6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -60,6 +60,7 @@ BUG FIXES:
 * A `module` block's `version` argument now accepts prerelease version selections using a "v" prefix before the version number. Previously this was accepted only for non-prerelease selections. ([#2124])(https://github.com/opentofu/opentofu/issues/2124)
 * The `tofu test` command doesn't try to validate mock provider definition by its underlying provider schema now. ([#2140](https://github.com/opentofu/opentofu/pull/2140))
 * Type validation for mocks and overrides are now less strict in `tofu test`. ([#2144](https://github.com/opentofu/opentofu/pull/2144))
+* Skip imports blocks logic on `tofu destroy` ([#2214](https://github.com/opentofu/opentofu/pull/2214))
 
 INTERNAL CHANGES:
 
diff --git a/internal/tofu/context_plan2_test.go b/internal/tofu/context_plan2_test.go
index e410410db3..f8ba3c7133 100644
--- a/internal/tofu/context_plan2_test.go
+++ b/internal/tofu/context_plan2_test.go
@@ -1046,6 +1046,80 @@ resource "test_object" "a" {
 	}
 }
 
+func TestContext2Plan_destroyWithRefresh_skipImport(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_object" "a" {
+}
+
+resource "test_object" "imported" {
+}
+
+import {
+  to   = test_object.imported
+  id   = "123"
+}
+`,
+	})
+
+	p := simpleMockProvider()
+
+	p.ReadResourceResponse = &providers.ReadResourceResponse{
+		NewState: cty.ObjectVal(map[string]cty.Value{
+			"test_string": cty.StringVal("foo"),
+		}),
+	}
+
+	p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{
+		ImportedResources: []providers.ImportedResource{
+			{
+				TypeName: "test_object",
+				State: cty.ObjectVal(map[string]cty.Value{
+					"test_string": cty.StringVal("foo"),
+				}),
+			},
+		},
+	}
+
+	expectedDestroyedAddr := mustResourceInstanceAddr("test_object.a")
+	state := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(expectedDestroyedAddr, &states.ResourceInstanceObjectSrc{
+			AttrsJSON: []byte(`{"arg":"before"}`),
+			Status:    states.ObjectReady,
+		}, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey)
+	})
+
+	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,
+		SkipRefresh: false, // the default
+	})
+	assertNoErrors(t, diags)
+
+	if plan.PriorState == nil {
+		t.Fatal("missing plan state")
+	}
+
+	if len(plan.Changes.Resources) > 1 {
+		t.Fatal("only a single resource should be changed in the plan")
+	}
+
+	changedResource := plan.Changes.Resources[0]
+
+	if changedResource.Action != plans.Delete {
+		t.Errorf("unexpected %s change for %s", changedResource.Action, changedResource.Addr)
+	}
+
+	if !changedResource.Addr.Equal(expectedDestroyedAddr) {
+		t.Errorf("unexpected change for resource %s instead of %s", changedResource.Addr, expectedDestroyedAddr)
+	}
+}
+
 func TestContext2Plan_destroySkipRefresh(t *testing.T) {
 	m := testModuleInline(t, map[string]string{
 		"main.tf": `
diff --git a/internal/tofu/node_resource_plan.go b/internal/tofu/node_resource_plan.go
index 86aec5848f..00c0d7ca38 100644
--- a/internal/tofu/node_resource_plan.go
+++ b/internal/tofu/node_resource_plan.go
@@ -156,7 +156,12 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 	importResolver := ctx.ImportResolver()
 	var diags tfdiags.Diagnostics
 	for _, importTarget := range n.importTargets {
-		if importTarget.IsFromImportBlock() {
+		// If the import target originates from the import command (instead of the import block), we don't need to
+		// resolve the import as it's already in the resolved form
+		// In addition, if PreDestroyRefresh is true, we know we are running as part of a refresh plan, immediately before a destroy
+		// plan. In the destroy plan mode, import blocks are not relevant, that's why we skip resolving imports
+		skipImports := importTarget.IsFromImportBlock() && !n.preDestroyRefresh
+		if skipImports {
 			err := importResolver.ExpandAndResolveImport(importTarget, ctx)
 			diags = diags.Append(err)
 		}