From 1ced176fc68f5084af7879a6a84c9b5db8d187e8 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 29 Aug 2018 11:25:09 -0700 Subject: [PATCH] plans: Track RequiredReplace as a cty.PathSet We were previously tracking this as a []cty.Path, but having it turned into a pathset on creation makes downstream use of it more convenient and ensures that it'll obey expected invariants like not containing the same path twice. --- plans/changes.go | 2 +- plans/changes_src.go | 8 ++------ terraform/eval_diff.go | 9 ++++----- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/plans/changes.go b/plans/changes.go index bb4ca43bd..6b930cd8a 100644 --- a/plans/changes.go +++ b/plans/changes.go @@ -95,7 +95,7 @@ type ResourceInstanceChange struct { // // This is retained only for UI-plan-rendering purposes and so it does not // currently survive a round-trip through a saved plan file. - RequiredReplace []cty.Path + RequiredReplace cty.PathSet // Private allows a provider to stash any extra data that is opaque to // Terraform that relates to this change. Terraform will save this diff --git a/plans/changes_src.go b/plans/changes_src.go index 7d195d2ae..6c46f9b31 100644 --- a/plans/changes_src.go +++ b/plans/changes_src.go @@ -40,7 +40,7 @@ type ResourceInstanceChangeSrc struct { // // This is retained only for UI-plan-rendering purposes and so it does not // currently survive a round-trip through a saved plan file. - RequiredReplace []cty.Path + RequiredReplace cty.PathSet // Private allows a provider to stash any extra data that is opaque to // Terraform that relates to this change. Terraform will save this @@ -80,11 +80,7 @@ func (rcs *ResourceInstanceChangeSrc) DeepCopy() *ResourceInstanceChangeSrc { } ret := *rcs - if len(ret.RequiredReplace) != 0 { - rr := make([]cty.Path, len(ret.RequiredReplace)) - copy(rr, ret.RequiredReplace) - ret.RequiredReplace = rr - } + ret.RequiredReplace = cty.NewPathSet(ret.RequiredReplace.List()...) if len(ret.Private) != 0 { private := make([]byte, len(ret.Private)) diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index fdec6e30a..639fe47ad 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -182,9 +182,8 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { // actually changed -- particularly after we may have undone some of the // changes in processIgnoreChanges -- so now we'll filter that list to // include only where changes are detected. - var reqRep []cty.Path + reqRep := cty.NewPathSet() if len(resp.RequiresReplace) > 0 { - reqRep = make([]cty.Path, 0, len(resp.RequiresReplace)) for _, path := range resp.RequiresReplace { if priorVal.IsNull() { // If prior is null then we don't expect any RequiresReplace at all, @@ -212,12 +211,12 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { if err != nil { // Should never happen since prior and changed should be of // the same type, but we'll allow it for robustness. - reqRep = append(reqRep, path) + reqRep.Add(path) } if priorChangedVal != cty.NilVal { eqV := plannedChangedVal.Equals(priorChangedVal) if !eqV.IsKnown() || eqV.False() { - reqRep = append(reqRep, path) + reqRep.Add(path) } } } @@ -235,7 +234,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { action = plans.Create case eq: action = plans.NoOp - case len(reqRep) > 0: + case !reqRep.Empty(): // If there are any "requires replace" paths left _after our filtering // above_ then this is a replace action. action = plans.Replace