From 25f4c0d3dd006a71bcfc72257892898bc4a42cba Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 4 Feb 2022 16:39:31 -0500 Subject: [PATCH] filter attribute refresh changes from plan UI Filter the refresh changes from the normal plan UI at the attribute level. We do this by constructing fake plans.Change records for diff generation, reverting all attribute changes that do not match any of the plan's ContributingResourceReferences. --- internal/command/views/operation_test.go | 52 ++++++++++++- internal/command/views/plan.go | 97 ++++++++++++++++++++++-- 2 files changed, 141 insertions(+), 8 deletions(-) diff --git a/internal/command/views/operation_test.go b/internal/command/views/operation_test.go index aa86fe144..eca2c3909 100644 --- a/internal/command/views/operation_test.go +++ b/internal/command/views/operation_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" + "github.com/hashicorp/terraform/internal/lang/globalref" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statefile" @@ -107,7 +108,7 @@ func TestOperation_planNoChanges(t *testing.T) { }, "No objects need to be destroyed.", }, - "drift detected in normal mode": { + "no drift detected in normal noop": { func(schemas *terraform.Schemas) *plans.Plan { addr := addrs.Resource{ Mode: addrs.ManagedResourceMode, @@ -146,7 +147,54 @@ func TestOperation_planNoChanges(t *testing.T) { DriftedResources: drs, } }, - "to update the Terraform state to match, create and apply a refresh-only plan", + "No changes", + }, + "drift detected in normal mode": { + func(schemas *terraform.Schemas) *plans.Plan { + addr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "somewhere", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) + schema, _ := schemas.ResourceTypeConfig( + addrs.NewDefaultProvider("test"), + addr.Resource.Resource.Mode, + addr.Resource.Resource.Type, + ) + ty := schema.ImpliedType() + rc := &plans.ResourceInstanceChange{ + Addr: addr, + PrevRunAddr: addr, + ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault( + addrs.NewDefaultProvider("test"), + ), + Change: plans.Change{ + Action: plans.Update, + Before: cty.NullVal(ty), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1234"), + "foo": cty.StringVal("bar"), + }), + }, + } + rcs, err := rc.Encode(ty) + if err != nil { + panic(err) + } + drs := []*plans.ResourceInstanceChangeSrc{rcs} + changes := plans.NewChanges() + changes.Resources = drs + return &plans.Plan{ + UIMode: plans.NormalMode, + Changes: changes, + DriftedResources: drs, + RelevantAttributes: []globalref.ResourceAttr{{ + Resource: addr.ContainingResource(), + Attr: cty.GetAttrPath("id"), + }}, + } + }, + "Objects have changed outside of Terraform", }, "drift detected in refresh-only mode": { func(schemas *terraform.Schemas) *plans.Plan { diff --git a/internal/command/views/plan.go b/internal/command/views/plan.go index 04355e7e7..11cfc872e 100644 --- a/internal/command/views/plan.go +++ b/internal/command/views/plan.go @@ -10,10 +10,12 @@ import ( "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/format" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/lang/globalref" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/objchange" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ) // The Plan view is used for the plan command. @@ -340,16 +342,36 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas relevant[r.Resource.String()] = true } + var changes []*plans.ResourceInstanceChange + for _, rcs := range plan.DriftedResources { + providerSchema := schemas.ProviderSchema(rcs.ProviderAddr.Provider) + if providerSchema == nil { + // Should never happen + view.streams.Printf("(schema missing for %s)\n\n", rcs.ProviderAddr) + continue + } + rSchema, _ := providerSchema.SchemaForResourceAddr(rcs.Addr.Resource.Resource) + if rSchema == nil { + // Should never happen + view.streams.Printf("(schema missing for %s)\n\n", rcs.Addr) + continue + } + + changes = append(changes, decodeChange(rcs, rSchema)) + } + // In refresh-only mode, we show all resources marked as drifted, // including those which have moved without other changes. In other plan // modes, move-only changes will be rendered in the planned changes, so // we skip them here. - var drs []*plans.ResourceInstanceChangeSrc + var drs []*plans.ResourceInstanceChange if plan.UIMode == plans.RefreshOnlyMode { - drs = plan.DriftedResources + drs = changes } else { - for _, dr := range plan.DriftedResources { - if dr.Action != plans.NoOp && relevant[dr.Addr.ContainingResource().String()] { + for _, dr := range changes { + change := filterRefreshChange(dr, plan.RelevantAttributes) + if change.Action != plans.NoOp { + dr.Change = change drs = append(drs, dr) } } @@ -371,7 +393,7 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas view.colorize.Color("[reset]\n[bold][cyan]Note:[reset][bold] Objects have changed outside of Terraform[reset]\n\n"), ) view.streams.Print(format.WordWrap( - "Terraform detected the following changes made outside of Terraform since the last \"terraform apply\":\n\n", + "Terraform detected the following changes made outside of Terraform since the last \"terraform apply\" which may have affected this plan:\n\n", view.outputColumns(), )) @@ -403,7 +425,7 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas } view.streams.Println(format.ResourceChange( - decodeChange(rcs, rSchema), + rcs, rSchema, view.colorize, format.DiffLanguageDetectedDrift, @@ -426,6 +448,69 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas return true } +// Filter individual resource changes for display based on the attributes which +// may have contributed to the plan as a whole. In order to continue to use the +// existing diff renderer, we are going to create a fake change for display, +// only showing the attributes we're interested in. +// The resulting change will be a NoOp if it has nothing relevant to the plan. +func filterRefreshChange(change *plans.ResourceInstanceChange, contributing []globalref.ResourceAttr) plans.Change { + if change.Action == plans.NoOp { + return change.Change + } + + var relevantAttrs []cty.Path + resAddr := change.Addr.ContainingResource() + + for _, attr := range contributing { + if resAddr.Equal(attr.Resource) { + relevantAttrs = append(relevantAttrs, attr.Attr) + } + } + + // If no attributes are relevant in this resource, then we can turn this + // onto a NoOp change for display. + if len(relevantAttrs) == 0 { + return plans.Change{ + Action: plans.NoOp, + Before: change.Before, + After: change.Before, + } + } + + // We have some attributes in this change which were marked as relevant, so + // we are going to take the Before value and add in only those attributes + // from the After value which may have contributed to the plan. + before := change.Before + after, _ := cty.Transform(before, func(path cty.Path, v cty.Value) (cty.Value, error) { + for i, attrPath := range relevantAttrs { + // If the current value is null, but we are only a prefix of the + // affected path, we need to take the value from this point since + // we can't recurse any further into the object. This has the + // possibility of pulling in extra attribute changes we're not + // concerned with, but we can take this as "close enough" for now. + if (v.IsNull() && attrPath.HasPrefix(path)) || attrPath.Equals(path) { + // remove the path from further consideration + relevantAttrs = append(relevantAttrs[:i], relevantAttrs[i+1:]...) + + v, err := path.Apply(change.After) + return v, err + } + } + return v, nil + }) + + action := change.Action + if before.RawEquals(after) { + action = plans.NoOp + } + + return plans.Change{ + Action: action, + Before: before, + After: after, + } +} + func decodeChange(change *plans.ResourceInstanceChangeSrc, schema *configschema.Block) *plans.ResourceInstanceChange { changeV, err := change.Decode(schema.ImpliedType()) if err != nil {