Merge pull request #28796 from hashicorp/jbardin/drift-deposed

plan: handle deposed instances when rendering drift

internal/command/format/diff.go

@@ -172,6 +172,12 @@ func ResourceInstanceDrift(
 	action := plans.Update
 
 	switch {
+	case before == nil || before.Current == nil:
+		// before should never be nil, but before.Current can be if the
+		// instance was deposed. There is nothing to render for a deposed
+		// instance, since we intend to remove it.
+		return ""
+
 	case after == nil || after.Current == nil:
 		// The object was deleted
 		buf.WriteString(color.Color(fmt.Sprintf("[bold]  # %s[reset] has been deleted", dispAddr)))

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{
@@ -121,14 +148,14 @@ func TestOperation_planNoChanges(t *testing.T) {
 						state.SetResourceInstanceCurrent(
 							addrs.Resource{
 								Mode: addrs.ManagedResourceMode,
-								Type: "something",
+								Type: "test_resource",
 								Name: "somewhere",
 							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
 							&states.ResourceInstanceObjectSrc{
 								Status:    states.ObjectReady,
 								AttrsJSON: []byte(`{}`),
 							},
-							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewBuiltInProvider("test")),
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
 						)
 					}),
 					PriorState: states.NewState(),
@@ -136,6 +163,69 @@ func TestOperation_planNoChanges(t *testing.T) {
 			},
 			"to update the Terraform state to match, create and apply a refresh-only plan",
 		},
+		"drift detected with deposed": {
+			func(schemas *terraform.Schemas) *plans.Plan {
+				return &plans.Plan{
+					UIMode:  plans.NormalMode,
+					Changes: plans.NewChanges(),
+					PrevRunState: states.BuildState(func(state *states.SyncState) {
+						state.SetResourceInstanceCurrent(
+							addrs.Resource{
+								Mode: addrs.ManagedResourceMode,
+								Type: "test_resource",
+								Name: "changes",
+							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+							&states.ResourceInstanceObjectSrc{
+								Status:    states.ObjectReady,
+								AttrsJSON: []byte(`{"foo":"b"}`),
+							},
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
+						)
+						state.SetResourceInstanceDeposed(
+							addrs.Resource{
+								Mode: addrs.ManagedResourceMode,
+								Type: "test_resource",
+								Name: "broken",
+							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+							states.NewDeposedKey(),
+							&states.ResourceInstanceObjectSrc{
+								Status:    states.ObjectReady,
+								AttrsJSON: []byte(`{"foo":"c"}`),
+							},
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
+						)
+					}),
+					PriorState: states.BuildState(func(state *states.SyncState) {
+						state.SetResourceInstanceCurrent(
+							addrs.Resource{
+								Mode: addrs.ManagedResourceMode,
+								Type: "test_resource",
+								Name: "changed",
+							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+							&states.ResourceInstanceObjectSrc{
+								Status:    states.ObjectReady,
+								AttrsJSON: []byte(`{"foo":"b"}`),
+							},
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
+						)
+						state.SetResourceInstanceDeposed(
+							addrs.Resource{
+								Mode: addrs.ManagedResourceMode,
+								Type: "test_resource",
+								Name: "broken",
+							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+							states.NewDeposedKey(),
+							&states.ResourceInstanceObjectSrc{
+								Status:    states.ObjectReady,
+								AttrsJSON: []byte(`{"foo":"d"}`),
+							},
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
+						)
+					}),
+				}
+			},
+			"to update the Terraform state to match, create and apply a refresh-only plan",
+		},
 		"drift detected in refresh-only mode": {
 			func(schemas *terraform.Schemas) *plans.Plan {
 				return &plans.Plan{
@@ -145,14 +235,14 @@ func TestOperation_planNoChanges(t *testing.T) {
 						state.SetResourceInstanceCurrent(
 							addrs.Resource{
 								Mode: addrs.ManagedResourceMode,
-								Type: "something",
+								Type: "test_resource",
 								Name: "somewhere",
 							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
 							&states.ResourceInstanceObjectSrc{
 								Status:    states.ObjectReady,
 								AttrsJSON: []byte(`{}`),
 							},
-							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewBuiltInProvider("test")),
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
 						)
 					}),
 					PriorState: states.NewState(),
@@ -169,14 +259,14 @@ func TestOperation_planNoChanges(t *testing.T) {
 						state.SetResourceInstanceCurrent(
 							addrs.Resource{
 								Mode: addrs.ManagedResourceMode,
-								Type: "something",
+								Type: "test_resource",
 								Name: "somewhere",
 							}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
 							&states.ResourceInstanceObjectSrc{
 								Status:    states.ObjectReady,
 								AttrsJSON: []byte(`{}`),
 							},
-							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewBuiltInProvider("test")),
+							addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")),
 						)
 					}),
 					PriorState: states.NewState(),

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 is 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(
+	var diffs []string
-		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 _, bms := range before.Modules {
 		for _, brs := range bms.Resources {
@@ -374,6 +373,10 @@ func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraf
 			}
 
 			for key, bis := range brs.Instances {
+				if bis.Current == nil {
+					// No current instance to render here
+					continue
+				}
 				var pis *states.ResourceInstance
 				if prs != nil {
 					pis = prs.Instance(key)
@@ -386,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 = `