From 45b5c289a050613bb076a70b991a057712e9b242 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 24 May 2021 17:02:15 -0400 Subject: [PATCH] no drift with only deposed changes Changes to deposed instances should not triggers any drift to be rendered, as they will have nothing to display. --- internal/command/views/operation_test.go | 27 ++++++++++++++++++ internal/command/views/plan.go | 35 ++++++++++++++++++------ 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/internal/command/views/operation_test.go b/internal/command/views/operation_test.go index ae0dc0fda..a3b3392ea 100644 --- a/internal/command/views/operation_test.go +++ b/internal/command/views/operation_test.go @@ -112,6 +112,33 @@ func TestOperation_planNoChanges(t *testing.T) { }, "No objects need to be destroyed.", }, + "no drift to display with only deposed instances": { + // changes in deposed instances will cause a change in state, but + // have nothing to display to the user + func(schemas *terraform.Schemas) *plans.Plan { + return &plans.Plan{ + UIMode: plans.NormalMode, + Changes: plans.NewChanges(), + PrevRunState: states.BuildState(func(state *states.SyncState) { + state.SetResourceInstanceDeposed( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "somewhere", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + states.NewDeposedKey(), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"foo": "ok", "bars":[]}`), + }, + addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), + ) + }), + PriorState: states.NewState(), + } + }, + "no differences, so no changes are needed.", + }, "drift detected in normal mode": { func(schemas *terraform.Schemas) *plans.Plan { return &plans.Plan{ diff --git a/internal/command/views/plan.go b/internal/command/views/plan.go index 0c3f92fc1..544508588 100644 --- a/internal/command/views/plan.go +++ b/internal/command/views/plan.go @@ -339,17 +339,16 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { // line of output, and guarantees to always produce whole lines terminated // by newline characters. func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraform.Schemas, view *View) bool { + // ManagedResourceEqual checks that the state es exactly equal for all + // managed resources; but semantically equivalent states, or changes to + // deposed instances may not actually represent changes we need to present + // to the user, so for now this only serves as a short-circuit to skip + // attempting to render the diffs below. if after.ManagedResourcesEqual(before) { return false } - view.streams.Print( - 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", - view.outputColumns(), - )) + var diffs []string for _, bms := range before.Modules { for _, brs := range bms.Resources { @@ -390,13 +389,31 @@ func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraf view.colorize, ) if diff != "" { - view.streams.Print(diff) + diffs = append(diffs, diff) } } } } - return true + // If we only have changes regarding deposed instances, or the diff + // renderer is suppressing irrelevant changes from the legacy SDK, there + // may not have been anything to display to the user. + if len(diffs) > 0 { + view.streams.Print( + 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", + view.outputColumns(), + )) + + for _, diff := range diffs { + view.streams.Print(diff) + } + return true + } + + return false } const planHeaderIntro = `