From c02e8bc5b33bfed40fb8826ee52045da0619a208 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 4 Feb 2022 13:34:43 -0500 Subject: [PATCH] change plan to store individual relevant attrs Storing individual contributing attributes will allow finer tuning of the plan rendering. add contributing to outputs --- internal/command/views/plan.go | 4 +-- internal/plans/plan.go | 14 +++----- internal/terraform/context_plan.go | 51 ++++++++++++------------------ 3 files changed, 28 insertions(+), 41 deletions(-) diff --git a/internal/command/views/plan.go b/internal/command/views/plan.go index 61168051f..51226dbdf 100644 --- a/internal/command/views/plan.go +++ b/internal/command/views/plan.go @@ -334,8 +334,8 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas // If this is not a refresh-only plan, we will need to filter out any // non-relevant changes to reduce plan output. relevant := make(map[string]bool) - for _, r := range plan.RelevantResources { - relevant[r.String()] = true + for _, r := range plan.RelevantAttributes { + relevant[r.Resource.String()] = true } // In refresh-only mode, we show all resources marked as drifted, diff --git a/internal/plans/plan.go b/internal/plans/plan.go index 0c90b3e7a..f7a2df1ad 100644 --- a/internal/plans/plan.go +++ b/internal/plans/plan.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/lang/globalref" "github.com/hashicorp/terraform/internal/states" "github.com/zclconf/go-cty/cty" ) @@ -36,20 +37,15 @@ type Plan struct { ForceReplaceAddrs []addrs.AbsResourceInstance Backend Backend - // RelevantResources is a set of resource addresses that are either - // directly affected by proposed changes or may have indirectly contributed - // to them via references in expressions. + // RelevantAttributes is a set of resource addresses and attributes that are + // either directly affected by proposed changes or may have indirectly + // contributed to them via references in expressions. // // This is the result of a heuristic and is intended only as a hint to // the UI layer in case it wants to emphasize or de-emphasize certain // resources. Don't use this to drive any non-cosmetic behavior, especially // including anything that would be subject to compatibility constraints. - // - // FIXME: This result currently doesn't survive round-tripping through a - // saved plan file, and so it'll be populated only for a freshly-created - // plan that has only existed in memory so far. When reloading a saved - // plan it will always appear as if there are no "relevant resources". - RelevantResources []addrs.AbsResource + RelevantAttributes []globalref.ResourceAttr // PrevRunState and PriorState both describe the situation that the plan // was derived from: diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index dcfc8e254..e776e37e8 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -200,10 +200,10 @@ The -target option is not for routine use, and is provided only for exceptional panic("nil plan but no errors") } - relevantResources, rDiags := c.relevantResourcesForPlan(config, plan) + relevantAttrs, rDiags := c.relevantResourceAttrsForPlan(config, plan) diags = diags.Append(rDiags) - plan.RelevantResources = relevantResources + plan.RelevantAttributes = relevantAttrs return plan, diags } @@ -366,10 +366,10 @@ func (c *Context) destroyPlan(config *configs.Config, prevRunState *states.State destroyPlan.PrevRunState = pendingPlan.PrevRunState } - relevantResources, rDiags := c.relevantResourcesForPlan(config, destroyPlan) + relevantAttrs, rDiags := c.relevantResourceAttrsForPlan(config, destroyPlan) diags = diags.Append(rDiags) - destroyPlan.RelevantResources = relevantResources + destroyPlan.RelevantAttributes = relevantAttrs return destroyPlan, diags } @@ -763,47 +763,38 @@ func (c *Context) referenceAnalyzer(config *configs.Config, state *states.State) // relevantResourcesForPlan implements the heuristic we use to populate the // RelevantResources field of returned plans. -func (c *Context) relevantResourcesForPlan(config *configs.Config, plan *plans.Plan) ([]addrs.AbsResource, tfdiags.Diagnostics) { +func (c *Context) relevantResourceAttrsForPlan(config *configs.Config, plan *plans.Plan) ([]globalref.ResourceAttr, tfdiags.Diagnostics) { azr, diags := c.referenceAnalyzer(config, plan.PriorState) if diags.HasErrors() { return nil, diags } - // Our current strategy is that a resource is relevant if it either has - // a proposed change action directly, or if its attributes are used as - // any part of a resource that has a proposed change action. We don't - // consider individual changed attributes for now, because we can't - // really reason about any rules that providers might have about changes - // to one attribute implying a change to another. - - // We'll use the string representation of a resource address as a unique - // key so we can dedupe our results. - relevant := make(map[string]addrs.AbsResource) - var refs []globalref.Reference for _, change := range plan.Changes.Resources { if change.Action == plans.NoOp { continue } - instAddr := change.Addr - addr := instAddr.ContainingResource() - relevant[addr.String()] = addr - moreRefs := azr.ReferencesFromResourceInstance(instAddr) + moreRefs := azr.ReferencesFromResourceInstance(change.Addr) refs = append(refs, moreRefs...) } - contributors := azr.ContributingResources(refs...) - for _, addr := range contributors { - relevant[addr.String()] = addr + for _, change := range plan.Changes.Outputs { + if change.Action == plans.NoOp { + continue + } + + moreRefs := azr.ReferencesFromOutputValue(change.Addr) + refs = append(refs, moreRefs...) } - if len(relevant) == 0 { - return nil, diags + var contributors []globalref.ResourceAttr + + for _, ref := range azr.ContributingResourceReferences(refs...) { + if res, ok := ref.ResourceAttr(); ok { + contributors = append(contributors, res) + } } - ret := make([]addrs.AbsResource, 0, len(relevant)) - for _, addr := range relevant { - ret = append(ret, addr) - } - return ret, diags + + return contributors, diags }