Skip to content

Commit cac484d

Browse files
committed
don't carry all replace addresses through instances
Once we have a resource instance we no longer need to have all possible forceReplace addresses available within that instance. We can determine immediately if the instance address is in the set if replacement addresses. This avoids bugs caused by sharing the `-replace` flag addresses across all resource nodes.
1 parent a7e2c9f commit cac484d

File tree

5 files changed

+17
-37
lines changed

5 files changed

+17
-37
lines changed

internal/terraform/graph_builder_apply.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package terraform
55

66
import (
7+
"slices"
8+
79
"github.com/hashicorp/terraform/internal/addrs"
810
"github.com/hashicorp/terraform/internal/configs"
911
"github.com/hashicorp/terraform/internal/dag"
@@ -110,7 +112,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
110112
concreteResourceInstance := func(a *NodeAbstractResourceInstance) dag.Vertex {
111113
return &NodeApplyableResourceInstance{
112114
NodeAbstractResourceInstance: a,
113-
forceReplace: b.ForceReplace,
115+
forceReplace: slices.ContainsFunc(b.ForceReplace, a.Addr.Equal),
114116
}
115117
}
116118

internal/terraform/node_resource_abstract_instance.go

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ func (n *NodeAbstractResourceInstance) plan(
795795
plannedChange *plans.ResourceInstanceChange,
796796
currentState *states.ResourceInstanceObject,
797797
createBeforeDestroy bool,
798-
forceReplace []addrs.AbsResourceInstance,
798+
forceReplace bool,
799799
) (*plans.ResourceInstanceChange, *states.ResourceInstanceObject, *providers.Deferred, instances.RepetitionData, tfdiags.Diagnostics) {
800800
var diags tfdiags.Diagnostics
801801
var keyData instances.RepetitionData
@@ -2975,34 +2975,15 @@ func resourceInstancePrevRunAddr(ctx EvalContext, currentAddr addrs.AbsResourceI
29752975
return table.OldAddr(currentAddr)
29762976
}
29772977

2978-
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) {
2979-
// The user might also ask us to force replacing a particular resource
2980-
// instance, regardless of whether the provider thinks it needs replacing.
2981-
// For example, users typically do this if they learn a particular object
2982-
// has become degraded in an immutable infrastructure scenario and so
2983-
// replacing it with a new object is a viable repair path.
2984-
matchedForceReplace := false
2985-
for _, candidateAddr := range forceReplace {
2986-
if candidateAddr.Equal(addr) {
2987-
matchedForceReplace = true
2988-
break
2989-
}
2990-
2991-
// For "force replace" purposes we require an exact resource instance
2992-
// address to match. If a user forgets to include the instance key
2993-
// for a multi-instance resource then it won't match here, but we
2994-
// have an earlier check in NodePlannableResource.Execute that should
2995-
// prevent us from getting here in that case.
2996-
}
2997-
2978+
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) {
29982979
// Unmark for this test for value equality.
29992980
eqV := plannedNewVal.Equals(priorVal)
30002981
eq := eqV.IsKnown() && eqV.True()
30012982

30022983
switch {
30032984
case priorVal.IsNull():
30042985
action = plans.Create
3005-
case matchedForceReplace || !reqRep.Empty() || !writeOnly.Intersection(reqRep).Empty():
2986+
case forceReplace || !reqRep.Empty() || !writeOnly.Intersection(reqRep).Empty():
30062987
// If the user "forced replace" of this instance of if there are any
30072988
// "requires replace" paths left _after our filtering above_ then this
30082989
// is a replace action.
@@ -3012,12 +2993,12 @@ func getAction(addr addrs.AbsResourceInstance, priorVal, plannedNewVal cty.Value
30122993
action = plans.DeleteThenCreate
30132994
}
30142995
switch {
3015-
case matchedForceReplace:
2996+
case forceReplace:
30162997
actionReason = plans.ResourceInstanceReplaceByRequest
30172998
case !reqRep.Empty():
30182999
actionReason = plans.ResourceInstanceReplaceBecauseCannotUpdate
30193000
}
3020-
case eq && !matchedForceReplace:
3001+
case eq && !forceReplace:
30213002
action = plans.NoOp
30223003
default:
30233004
action = plans.Update

internal/terraform/node_resource_apply_instance.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,9 @@ type NodeApplyableResourceInstance struct {
3131

3232
graphNodeDeposer // implementation of GraphNodeDeposerConfig
3333

34-
// forceReplace are resource instance addresses where the user wants to
35-
// force generating a replace action. This set isn't pre-filtered, so
36-
// it might contain addresses that have nothing to do with the resource
37-
// that this node represents, which the node itself must therefore ignore.
38-
forceReplace []addrs.AbsResourceInstance
34+
// forceReplace indicates that this resource is being replaced for external
35+
// reasons, like a -replace flag or via replace_triggered_by.
36+
forceReplace bool
3937
}
4038

4139
var (

internal/terraform/node_resource_plan.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package terraform
66
import (
77
"fmt"
88
"log"
9+
"slices"
910
"strings"
1011

1112
"github.com/hashicorp/hcl/v2"
@@ -586,7 +587,7 @@ func (n *nodeExpandPlannableResource) concreteResource(ctx EvalContext, knownImp
586587
ForceCreateBeforeDestroy: n.CreateBeforeDestroy(),
587588
skipRefresh: n.skipRefresh,
588589
skipPlanChanges: skipPlanChanges,
589-
forceReplace: n.forceReplace,
590+
forceReplace: slices.ContainsFunc(n.forceReplace, a.Addr.Equal),
590591
}
591592

592593
if importID, ok := knownImports.GetOk(a.Addr); ok {

internal/terraform/node_resource_plan_instance.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,9 @@ type NodePlannableResourceInstance struct {
4040
// for any instances.
4141
skipPlanChanges bool
4242

43-
// forceReplace are resource instance addresses where the user wants to
44-
// force generating a replace action. This set isn't pre-filtered, so
45-
// it might contain addresses that have nothing to do with the resource
46-
// that this node represents, which the node itself must therefore ignore.
47-
forceReplace []addrs.AbsResourceInstance
43+
// forceReplace indicates that this resource is being replaced for external
44+
// reasons, like a -replace flag or via replace_triggered_by.
45+
forceReplace bool
4846

4947
// replaceTriggeredBy stores references from replace_triggered_by which
5048
// triggered this instance to be replaced.
@@ -568,7 +566,7 @@ func (n *NodePlannableResourceInstance) replaceTriggered(ctx EvalContext, repDat
568566
// triggered the replacement in the plan.
569567
// Rather than further complicating the plan method with more
570568
// options, we can refactor both of these features later.
571-
n.forceReplace = append(n.forceReplace, n.Addr)
569+
n.forceReplace = true
572570
log.Printf("[DEBUG] ReplaceTriggeredBy forcing replacement of %s due to change in %s", n.Addr, ref.DisplayString())
573571

574572
n.replaceTriggeredBy = append(n.replaceTriggeredBy, ref)

0 commit comments

Comments
 (0)