From e21d5d36f46fd548d7f1456cd8d86cf4f6aa596a Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 26 Jan 2022 15:16:09 -0500 Subject: [PATCH] core: Fix plan write/state write ordering bug --- .../terraform/node_resource_plan_instance.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 87d54d720..200635566 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -206,6 +206,22 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } + // FIXME: it is currently important that we write resource changes to + // the plan (n.writeChange) before we write the corresponding state + // (n.writeResourceInstanceState). + // + // This is because the planned resource state will normally have the + // status of states.ObjectPlanned, which causes later logic to refer to + // the contents of the plan to retrieve the resource data. Because + // there is no shared lock between these two data structures, reversing + // the order of these writes will cause a brief window of inconsistency + // which can lead to a failed safety check. + // + // Future work should adjust these APIs such that it is impossible to + // update these two data structures incorrectly through any objects + // reachable via the terraform.EvalContext API. + diags = diags.Append(n.writeChange(ctx, change, "")) + diags = diags.Append(n.writeResourceInstanceState(ctx, instancePlanState, workingState)) if diags.HasErrors() { return diags @@ -224,8 +240,6 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } } - - diags = diags.Append(n.writeChange(ctx, change, "")) } else { // Even if we don't plan changes, we do still need to at least update // the working state to reflect the refresh result. If not, then e.g.