From 63bede76166ea944e8c98bd2dd0288314438079c Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 5 Apr 2024 10:32:21 +0200 Subject: [PATCH] stacks: return deferral instead of recording it --- .../node_resource_abstract_instance.go | 31 +++++------- .../node_resource_abstract_instance_test.go | 10 ++-- .../node_resource_destroy_deposed.go | 15 +++++- internal/terraform/node_resource_import.go | 48 ++++++++++++------- .../terraform/node_resource_plan_instance.go | 33 +++++++++++-- .../terraform/node_resource_plan_orphan.go | 15 +++++- 6 files changed, 104 insertions(+), 48 deletions(-) diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index c42aec8ffffc..43ca0281d1fb 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -584,8 +584,10 @@ func (n *NodeAbstractResourceInstance) writeChange(ctx EvalContext, change *plan } // refresh does a refresh for a resource -func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey states.DeposedKey, state *states.ResourceInstanceObject) (*states.ResourceInstanceObject, tfdiags.Diagnostics) { +// if the second return value is non-nil, the refresh is deferred +func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey states.DeposedKey, state *states.ResourceInstanceObject) (*states.ResourceInstanceObject, *providers.Deferred, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + var deferred *providers.Deferred absAddr := n.Addr if deposedKey == states.NotDeposed { log.Printf("[TRACE] NodeAbstractResourceInstance.refresh for %s", absAddr) @@ -594,25 +596,25 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state } provider, providerSchema, err := getProvider(ctx, n.ResolvedProvider) if err != nil { - return state, diags.Append(err) + return state, deferred, diags.Append(err) } // If we have no state, we don't do any refreshing if state == nil { log.Printf("[DEBUG] refresh: %s: no state, so not refreshing", absAddr) - return state, diags + return state, deferred, diags } schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.Resource.ContainingResource()) if schema == nil { // Should be caught during validation, so we don't bother with a pretty error here diags = diags.Append(fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Resource.Type)) - return state, diags + return state, deferred, diags } metaConfigVal, metaDiags := n.providerMetas(ctx) diags = diags.Append(metaDiags) if diags.HasErrors() { - return state, diags + return state, deferred, diags } // Call pre-refresh hook @@ -620,7 +622,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state return h.PreRefresh(n.HookResourceIdentity(), deposedKey, state.Value) })) if diags.HasErrors() { - return state, diags + return state, deferred, diags } // Refresh! @@ -649,14 +651,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state }) if resp.Deferred != nil { - deferrals := ctx.Deferrals() - deferrals.ReportResourceInstanceDeferred(absAddr, resp.Deferred.Reason, &plans.ResourceInstanceChange{ - Addr: absAddr, - Change: plans.Change{ - Action: plans.Create, - After: cty.DynamicVal, - }, - }) + deferred = resp.Deferred } } if n.Config != nil { @@ -665,7 +660,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { - return state, diags + return state, deferred, diags } if resp.NewState == cty.NilVal { @@ -699,7 +694,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state )) } if diags.HasErrors() { - return state, diags + return state, deferred, diags } newState := objchange.NormalizeObjectFromLegacySDK(resp.NewState, schema) @@ -732,7 +727,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state return h.PostRefresh(n.HookResourceIdentity(), deposedKey, priorVal, ret.Value) })) if diags.HasErrors() { - return ret, diags + return ret, deferred, diags } // Mark the value with any prior marks from the state, and the marks from @@ -744,7 +739,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state ret.Value = ret.Value.MarkWithPaths(marks) } - return ret, diags + return ret, deferred, diags } func (n *NodeAbstractResourceInstance) plan( diff --git a/internal/terraform/node_resource_abstract_instance_test.go b/internal/terraform/node_resource_abstract_instance_test.go index 82a150cf0c0e..36a0e1d1a5d2 100644 --- a/internal/terraform/node_resource_abstract_instance_test.go +++ b/internal/terraform/node_resource_abstract_instance_test.go @@ -232,7 +232,7 @@ func TestNodeAbstractResourceInstance_refresh_with_deferred_read(t *testing.T) { resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]() evalCtx.DeferralsState = deferring.NewDeferred(resourceGraph, true) - rio, diags := node.refresh(evalCtx, states.NotDeposed, obj) + rio, deferred, diags := node.refresh(evalCtx, states.NotDeposed, obj) if diags.HasErrors() { t.Fatal(diags.Err()) } @@ -242,11 +242,11 @@ func TestNodeAbstractResourceInstance_refresh_with_deferred_read(t *testing.T) { t.Fatalf("value was known: %v", value) } - if !evalCtx.DeferralsCalled { - t.Fatalf("expected deferral to be called") + if deferred == nil { + t.Fatalf("expected deferral to be present") } - if !evalCtx.DeferralsState.HaveAnyDeferrals() { - t.Fatalf("expected deferral to be present") + if deferred.Reason != providers.DeferredReasonAbsentPrereq { + t.Fatalf("expected deferral to be AbsentPrereq, got %s", deferred.Reason) } } diff --git a/internal/terraform/node_resource_destroy_deposed.go b/internal/terraform/node_resource_destroy_deposed.go index 8010dd84ef87..9cd958d8e211 100644 --- a/internal/terraform/node_resource_destroy_deposed.go +++ b/internal/terraform/node_resource_destroy_deposed.go @@ -120,7 +120,7 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk // resource during Delete correctly. If this is a simple refresh, // Terraform is expected to remove the missing resource from the state // entirely - refreshedState, refreshDiags := n.refresh(ctx, n.DeposedKey, state) + refreshedState, deferred, refreshDiags := n.refresh(ctx, n.DeposedKey, state) diags = diags.Append(refreshDiags) if diags.HasErrors() { return diags @@ -133,7 +133,18 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk // If we refreshed then our subsequent planning should be in terms of // the new object, not the original object. - state = refreshedState + if deferred == nil { + state = refreshedState + } else { + ctx.Deferrals().ReportResourceInstanceDeferred(n.Addr, deferred.Reason, &plans.ResourceInstanceChange{ + Addr: n.Addr, + Change: plans.Change{ + Action: plans.Read, + Before: state.Value, + After: refreshedState.Value, + }, + }) + } } if !n.skipPlanChanges { diff --git a/internal/terraform/node_resource_import.go b/internal/terraform/node_resource_import.go index 90949cda162c..a6f1ed7950e6 100644 --- a/internal/terraform/node_resource_import.go +++ b/internal/terraform/node_resource_import.go @@ -8,6 +8,7 @@ import ( "log" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" @@ -228,29 +229,40 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) (di ResolvedProvider: n.ResolvedProvider, }, } - state, refreshDiags := riNode.refresh(ctx, states.NotDeposed, state) + state, deferred, refreshDiags := riNode.refresh(ctx, states.NotDeposed, state) diags = diags.Append(refreshDiags) if diags.HasErrors() { return diags } - // Verify the existance of the imported resource - if state.Value.IsNull() { - var diags tfdiags.Diagnostics - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Cannot import non-existent remote object", - fmt.Sprintf( - "While attempting to import an existing object to %q, "+ - "the provider detected that no object exists with the given id. "+ - "Only pre-existing objects can be imported; check that the id "+ - "is correct and that it is associated with the provider's "+ - "configured region or endpoint, or use \"terraform apply\" to "+ - "create a new remote object for this resource.", - n.TargetAddr, - ), - )) - return diags + // If the refresh is deferred we will need to do another cycle to import the resource + if deferred != nil { + ctx.Deferrals().ReportResourceInstanceDeferred(n.TargetAddr, deferred.Reason, &plans.ResourceInstanceChange{ + Addr: n.TargetAddr, + Change: plans.Change{ + Action: plans.Read, + After: state.Value, + }, + }) + } else { + // Verify the existance of the imported resource + if state.Value.IsNull() { + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Cannot import non-existent remote object", + fmt.Sprintf( + "While attempting to import an existing object to %q, "+ + "the provider detected that no object exists with the given id. "+ + "Only pre-existing objects can be imported; check that the id "+ + "is correct and that it is associated with the provider's "+ + "configured region or endpoint, or use \"terraform apply\" to "+ + "create a new remote object for this resource.", + n.TargetAddr, + ), + )) + return diags + } } diags = diags.Append(riNode.writeResourceInstanceState(ctx, state, workingState)) diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 08a936d1dc0e..69b5814c5e8a 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -205,13 +205,24 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) // Refresh, maybe // The import process handles its own refresh if !n.skipRefresh && !importing { - s, refreshDiags := n.refresh(ctx, states.NotDeposed, instanceRefreshState) + s, deferred, refreshDiags := n.refresh(ctx, states.NotDeposed, instanceRefreshState) diags = diags.Append(refreshDiags) if diags.HasErrors() { return diags } - instanceRefreshState = s + if deferred == nil { + instanceRefreshState = s + } else { + ctx.Deferrals().ReportResourceInstanceDeferred(n.Addr, deferred.Reason, &plans.ResourceInstanceChange{ + Addr: n.Addr, + Change: plans.Change{ + Action: plans.Read, + Before: s.Value, + After: cty.DynamicVal, + }, + }) + } if instanceRefreshState != nil { // When refreshing we start by merging the stored dependencies and @@ -586,12 +597,28 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. }, override: n.override, } - instanceRefreshState, refreshDiags := riNode.refresh(ctx, states.NotDeposed, importedState) + instanceRefreshState, deferred, refreshDiags := riNode.refresh(ctx, states.NotDeposed, importedState) diags = diags.Append(refreshDiags) if diags.HasErrors() { return instanceRefreshState, diags } + if deferred != nil { + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Cannot import deferred remote object", + fmt.Sprintf( + "While attempting to import an existing object to %q, "+ + "the provider detected that the object was deferred. "+ + "Only not-deferred objects can be imported; please make sure "+ + "the provider has all the information needed to plan the change.", + n.Addr, + ), + )) + return instanceRefreshState, diags + } + // verify the existence of the imported resource if instanceRefreshState.Value.IsNull() { var diags tfdiags.Diagnostics diff --git a/internal/terraform/node_resource_plan_orphan.go b/internal/terraform/node_resource_plan_orphan.go index 31f8613d09d1..0cf597e89b87 100644 --- a/internal/terraform/node_resource_plan_orphan.go +++ b/internal/terraform/node_resource_plan_orphan.go @@ -116,7 +116,7 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon // plan before apply, and may not handle a missing resource during // Delete correctly. If this is a simple refresh, Terraform is // expected to remove the missing resource from the state entirely - refreshedState, refreshDiags := n.refresh(ctx, states.NotDeposed, oldState) + refreshedState, deferred, refreshDiags := n.refresh(ctx, states.NotDeposed, oldState) diags = diags.Append(refreshDiags) if diags.HasErrors() { return diags @@ -129,7 +129,18 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon // If we refreshed then our subsequent planning should be in terms of // the new object, not the original object. - oldState = refreshedState + if deferred == nil { + oldState = refreshedState + } else { + ctx.Deferrals().ReportResourceInstanceDeferred(n.Addr, deferred.Reason, &plans.ResourceInstanceChange{ + Addr: n.Addr, + Change: plans.Change{ + Action: plans.Read, + Before: oldState.Value, + After: refreshedState.Value, + }, + }) + } } // If we're skipping planning, all we need to do is write the state. If the