diff --git a/.changes/v1.14/BUG FIXES-20251029-175958.yaml b/.changes/v1.14/BUG FIXES-20251029-175958.yaml new file mode 100644 index 000000000000..e9f7d9d2ae2c --- /dev/null +++ b/.changes/v1.14/BUG FIXES-20251029-175958.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: Combinations of replace_triggered_by and -replace could result in some instances not being replaced +time: 2025-10-29T17:59:58.326396-04:00 +custom: + Issue: "37833" diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index f835b82f41fb..8835a434e5c7 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -4,6 +4,8 @@ package terraform import ( + "slices" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" @@ -110,7 +112,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { concreteResourceInstance := func(a *NodeAbstractResourceInstance) dag.Vertex { return &NodeApplyableResourceInstance{ NodeAbstractResourceInstance: a, - forceReplace: b.ForceReplace, + forceReplace: slices.ContainsFunc(b.ForceReplace, a.Addr.Equal), } } diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index e15a5593c332..a1f951c34baf 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -795,7 +795,7 @@ func (n *NodeAbstractResourceInstance) plan( plannedChange *plans.ResourceInstanceChange, currentState *states.ResourceInstanceObject, createBeforeDestroy bool, - forceReplace []addrs.AbsResourceInstance, + forceReplace bool, ) (*plans.ResourceInstanceChange, *states.ResourceInstanceObject, *providers.Deferred, instances.RepetitionData, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics var keyData instances.RepetitionData @@ -2975,26 +2975,7 @@ func resourceInstancePrevRunAddr(ctx EvalContext, currentAddr addrs.AbsResourceI return table.OldAddr(currentAddr) } -func getAction(addr addrs.AbsResourceInstance, priorVal, plannedNewVal cty.Value, createBeforeDestroy bool, writeOnly cty.PathSet, forceReplace []addrs.AbsResourceInstance, reqRep cty.PathSet) (action plans.Action, actionReason plans.ResourceInstanceChangeActionReason) { - // The user might also ask us to force replacing a particular resource - // instance, regardless of whether the provider thinks it needs replacing. - // For example, users typically do this if they learn a particular object - // has become degraded in an immutable infrastructure scenario and so - // replacing it with a new object is a viable repair path. - matchedForceReplace := false - for _, candidateAddr := range forceReplace { - if candidateAddr.Equal(addr) { - matchedForceReplace = true - break - } - - // For "force replace" purposes we require an exact resource instance - // address to match. If a user forgets to include the instance key - // for a multi-instance resource then it won't match here, but we - // have an earlier check in NodePlannableResource.Execute that should - // prevent us from getting here in that case. - } - +func getAction(addr addrs.AbsResourceInstance, priorVal, plannedNewVal cty.Value, createBeforeDestroy bool, writeOnly cty.PathSet, forceReplace bool, reqRep cty.PathSet) (action plans.Action, actionReason plans.ResourceInstanceChangeActionReason) { // Unmark for this test for value equality. eqV := plannedNewVal.Equals(priorVal) eq := eqV.IsKnown() && eqV.True() @@ -3002,7 +2983,7 @@ func getAction(addr addrs.AbsResourceInstance, priorVal, plannedNewVal cty.Value switch { case priorVal.IsNull(): action = plans.Create - case matchedForceReplace || !reqRep.Empty() || !writeOnly.Intersection(reqRep).Empty(): + case forceReplace || !reqRep.Empty() || !writeOnly.Intersection(reqRep).Empty(): // If the user "forced replace" of this instance of if there are any // "requires replace" paths left _after our filtering above_ then this // is a replace action. @@ -3012,12 +2993,12 @@ func getAction(addr addrs.AbsResourceInstance, priorVal, plannedNewVal cty.Value action = plans.DeleteThenCreate } switch { - case matchedForceReplace: + case forceReplace: actionReason = plans.ResourceInstanceReplaceByRequest case !reqRep.Empty(): actionReason = plans.ResourceInstanceReplaceBecauseCannotUpdate } - case eq && !matchedForceReplace: + case eq && !forceReplace: action = plans.NoOp default: action = plans.Update diff --git a/internal/terraform/node_resource_apply_instance.go b/internal/terraform/node_resource_apply_instance.go index fbaecc29bc54..ee551a1e49a6 100644 --- a/internal/terraform/node_resource_apply_instance.go +++ b/internal/terraform/node_resource_apply_instance.go @@ -31,11 +31,9 @@ type NodeApplyableResourceInstance struct { graphNodeDeposer // implementation of GraphNodeDeposerConfig - // forceReplace are resource instance addresses where the user wants to - // force generating a replace action. This set isn't pre-filtered, so - // it might contain addresses that have nothing to do with the resource - // that this node represents, which the node itself must therefore ignore. - forceReplace []addrs.AbsResourceInstance + // forceReplace indicates that this resource is being replaced for external + // reasons, like a -replace flag or via replace_triggered_by. + forceReplace bool } var ( diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index 2a6e3590bd35..1b8763c2af35 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -6,6 +6,7 @@ package terraform import ( "fmt" "log" + "slices" "strings" "github.com/hashicorp/hcl/v2" @@ -586,7 +587,7 @@ func (n *nodeExpandPlannableResource) concreteResource(ctx EvalContext, knownImp ForceCreateBeforeDestroy: n.CreateBeforeDestroy(), skipRefresh: n.skipRefresh, skipPlanChanges: skipPlanChanges, - forceReplace: n.forceReplace, + forceReplace: slices.ContainsFunc(n.forceReplace, a.Addr.Equal), } if importID, ok := knownImports.GetOk(a.Addr); ok { diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 4dec19172fa1..74f7140a37c9 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -40,11 +40,9 @@ type NodePlannableResourceInstance struct { // for any instances. skipPlanChanges bool - // forceReplace are resource instance addresses where the user wants to - // force generating a replace action. This set isn't pre-filtered, so - // it might contain addresses that have nothing to do with the resource - // that this node represents, which the node itself must therefore ignore. - forceReplace []addrs.AbsResourceInstance + // forceReplace indicates that this resource is being replaced for external + // reasons, like a -replace flag or via replace_triggered_by. + forceReplace bool // replaceTriggeredBy stores references from replace_triggered_by which // triggered this instance to be replaced. @@ -568,7 +566,7 @@ func (n *NodePlannableResourceInstance) replaceTriggered(ctx EvalContext, repDat // triggered the replacement in the plan. // Rather than further complicating the plan method with more // options, we can refactor both of these features later. - n.forceReplace = append(n.forceReplace, n.Addr) + n.forceReplace = true log.Printf("[DEBUG] ReplaceTriggeredBy forcing replacement of %s due to change in %s", n.Addr, ref.DisplayString()) n.replaceTriggeredBy = append(n.replaceTriggeredBy, ref)