From 4db9ab8536003776e7be2c771f50a930b8eeafc3 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 5 Apr 2024 23:44:17 +0200
Subject: [PATCH] stacks: check if resource instance is deferred

before marking it as deferred
---
 internal/plans/deferring/deferred.go          | 19 ++++++++++++++++++-
 .../terraform/context_apply_deferred_test.go  |  3 ---
 .../terraform/node_resource_plan_instance.go  |  8 ++++++--
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/internal/plans/deferring/deferred.go b/internal/plans/deferring/deferred.go
index 89e45ae19f..7c19b9375e 100644
--- a/internal/plans/deferring/deferred.go
+++ b/internal/plans/deferring/deferred.go
@@ -177,6 +177,22 @@ func (d *Deferred) HaveAnyDeferrals() bool {
 			len(d.partialExpandedModulesDeferred) != 0)
 }
 
+// IsResourceInstanceDeferred returns true if the receiver knows some reason
+// why the resource instance with the given address should have its planned
+// action deferred for a future plan/apply round.
+func (d *Deferred) IsResourceInstanceDeferred(addr addrs.AbsResourceInstance) bool {
+	if d.externalDependencyDeferred {
+		return true
+	}
+
+	// Our resource graph describes relationships between the static resource
+	// configuration blocks, not their dynamic instances, so we need to start
+	// with the config address that the given instance belongs to.
+	configAddr := addr.ConfigResource()
+
+	return d.resourceInstancesDeferred.Get(configAddr).Has(addr)
+}
+
 // ShouldDeferResourceInstanceChanges returns true if the receiver knows some
 // reason why the resource instance with the given address should have its
 // planned action deferred for a future plan/apply round.
@@ -196,7 +212,8 @@ func (d *Deferred) HaveAnyDeferrals() bool {
 // It's invalid to call this method for an address that was already reported
 // as deferred using [Deferred.ReportResourceInstanceDeferred], and so this
 // method will panic in that case. Callers should always test whether a resource
-// instance action should be deferred _before_ reporting that it has been.
+// instance action should be deferred _before_ reporting that it has been by calling
+// [Deferred.IsResourceInstanceDeferred].
 func (d *Deferred) ShouldDeferResourceInstanceChanges(addr addrs.AbsResourceInstance) bool {
 	d.mu.Lock()
 	defer d.mu.Unlock()
diff --git a/internal/terraform/context_apply_deferred_test.go b/internal/terraform/context_apply_deferred_test.go
index 701b989988..f1d311b5aa 100644
--- a/internal/terraform/context_apply_deferred_test.go
+++ b/internal/terraform/context_apply_deferred_test.go
@@ -1351,7 +1351,6 @@ output "a" {
 		}),
 		stages: []deferredActionsTestStage{
 			{
-
 				buildOpts: func(opts *PlanOpts) {
 					opts.Mode = plans.RefreshOnlyMode
 				},
@@ -1613,9 +1612,7 @@ func (provider *deferredActionsProvider) Provider() providers.Interface {
 			},
 		},
 		ReadResourceFn: func(req providers.ReadResourceRequest) providers.ReadResourceResponse {
-			fmt.Println("ReadResourceFn called")
 			if key := req.PriorState.GetAttr("name"); key.IsKnown() && key.AsString() == "deferred_read" {
-				fmt.Println("Deferring read")
 				return providers.ReadResourceResponse{
 					NewState: req.PriorState,
 					Deferred: &providers.Deferred{
diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go
index 2bc635da9a..9a7b7c12fd 100644
--- a/internal/terraform/node_resource_plan_instance.go
+++ b/internal/terraform/node_resource_plan_instance.go
@@ -298,7 +298,11 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
 		}
 
 		deferrals := ctx.Deferrals()
-		if !deferrals.ShouldDeferResourceInstanceChanges(n.Addr) {
+
+		if deferrals.IsResourceInstanceDeferred(n.Addr) {
+			// This resource instance is already deferred, probably because it
+			// was deferred during the refresh or import step.
+		} else if !deferrals.ShouldDeferResourceInstanceChanges(n.Addr) {
 			// We intentionally write the change before the subsequent checks, because
 			// all of the checks below this point are for problems caused by the
 			// context surrounding the change, rather than the change itself, and
@@ -614,7 +618,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 	}
 
 	// verify the existence of the imported resource
-	if instanceRefreshState.Value.IsNull() {
+	if instanceRefreshState.Value.IsNull() && deferred == nil {
 		var diags tfdiags.Diagnostics
 		diags = diags.Append(tfdiags.Sourceless(
 			tfdiags.Error,