From 04f9e7148cd5cc8fa74ccd8af0c8dd2472d3a7f2 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 22 Sep 2021 18:23:35 -0700 Subject: [PATCH] command/format: Include deletion reasons in plan report The core runtime is now able to specify a reason for some situations when Terraform plans to delete a resource instance. This commit makes that information visible in the human-oriented UI. A previous commit already made the underlying data informing these new hints visible as part of the machine-oriented (JSON) plan output. This also removes the bold formatting from the existing "has moved to" hints, because subjectively it seemed like the result was emphasizing too many parts of the output and thus somewhat defeating the benefit of the emphasis in trying to create additional visual hierarchy for sighted users running Terraform in a terminal. Now only the first line containing the main action statement will be in bold, and all of the parenthesized follow-up notes will be unformatted. --- internal/command/format/diff.go | 27 +++- internal/command/format/diff_test.go | 229 ++++++++++++++++++++++++++- 2 files changed, 253 insertions(+), 3 deletions(-) diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index c70918af4..ec777b585 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -98,6 +98,31 @@ func ResourceChange( default: buf.WriteString(fmt.Sprintf(color.Color("[bold] # %s[reset] delete (unknown reason %s)"), dispAddr, language)) } + // We can sometimes give some additional detail about why we're + // proposing to delete. We show this as additional notes, rather than + // as additional wording in the main action statement, in an attempt + // to make the "will be destroyed" message prominent and consistent + // in all cases, for easier scanning of this often-risky action. + switch change.ActionReason { + case plans.ResourceInstanceDeleteBecauseNoResourceConfig: + buf.WriteString(fmt.Sprintf("\n # (because %s is not in configuration)", addr.Resource.Resource)) + case plans.ResourceInstanceDeleteBecauseNoModule: + buf.WriteString(fmt.Sprintf("\n # (because %s is not in configuration)", addr.Module)) + case plans.ResourceInstanceDeleteBecauseWrongRepetition: + // We have some different variations of this one + switch addr.Resource.Key.(type) { + case nil: + buf.WriteString("\n # (because resource uses count or for_each)") + case addrs.IntKey: + buf.WriteString("\n # (because resource does not use count)") + case addrs.StringKey: + buf.WriteString("\n # (because resource does not use for_each)") + } + case plans.ResourceInstanceDeleteBecauseCountIndex: + buf.WriteString(fmt.Sprintf("\n # (because index %s is out of range for count)", addr.Resource.Key)) + case plans.ResourceInstanceDeleteBecauseEachKey: + buf.WriteString(fmt.Sprintf("\n # (because key %s is not in for_each map)", addr.Resource.Key)) + } if change.DeposedKey != states.NotDeposed { // Some extra context about this unusual situation. buf.WriteString(color.Color("\n # (left over from a partially-failed replacement of this instance)")) @@ -115,7 +140,7 @@ func ResourceChange( buf.WriteString(color.Color("[reset]\n")) if change.Moved() && change.Action != plans.NoOp { - buf.WriteString(fmt.Sprintf(color.Color("[bold] # [reset]([bold]%s[reset] has moved to [bold]%s[reset])\n"), change.PrevRunAddr.String(), dispAddr)) + buf.WriteString(fmt.Sprintf(color.Color(" # [reset](moved from %s)\n"), change.PrevRunAddr.String())) } if change.Moved() && change.Action == plans.NoOp { diff --git a/internal/command/format/diff_test.go b/internal/command/format/diff_test.go index b59a3af56..6a720210e 100644 --- a/internal/command/format/diff_test.go +++ b/internal/command/format/diff_test.go @@ -3504,6 +3504,229 @@ func TestResourceChange_nestedMap(t *testing.T) { runTestCases(t, testCases) } +func TestResourceChange_actionReason(t *testing.T) { + emptySchema := &configschema.Block{} + nullVal := cty.NullVal(cty.EmptyObject) + emptyVal := cty.EmptyObjectVal + + testCases := map[string]testCase{ + "delete for no particular reason": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceChangeNoReason, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be destroyed + - resource "test_instance" "example" {} +`, + }, + "delete because of wrong repetition mode (NoKey)": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseWrongRepetition, + Mode: addrs.ManagedResourceMode, + InstanceKey: addrs.NoKey, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be destroyed + # (because resource uses count or for_each) + - resource "test_instance" "example" {} +`, + }, + "delete because of wrong repetition mode (IntKey)": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseWrongRepetition, + Mode: addrs.ManagedResourceMode, + InstanceKey: addrs.IntKey(1), + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example[1] will be destroyed + # (because resource does not use count) + - resource "test_instance" "example" {} +`, + }, + "delete because of wrong repetition mode (StringKey)": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseWrongRepetition, + Mode: addrs.ManagedResourceMode, + InstanceKey: addrs.StringKey("a"), + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example["a"] will be destroyed + # (because resource does not use for_each) + - resource "test_instance" "example" {} +`, + }, + "delete because no resource configuration": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseNoResourceConfig, + ModuleInst: addrs.RootModuleInstance.Child("foo", addrs.NoKey), + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # module.foo.test_instance.example will be destroyed + # (because test_instance.example is not in configuration) + - resource "test_instance" "example" {} +`, + }, + "delete because no module": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseNoModule, + ModuleInst: addrs.RootModuleInstance.Child("foo", addrs.IntKey(1)), + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # module.foo[1].test_instance.example will be destroyed + # (because module.foo[1] is not in configuration) + - resource "test_instance" "example" {} +`, + }, + "delete because out of range for count": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseCountIndex, + Mode: addrs.ManagedResourceMode, + InstanceKey: addrs.IntKey(1), + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example[1] will be destroyed + # (because index [1] is out of range for count) + - resource "test_instance" "example" {} +`, + }, + "delete because out of range for for_each": { + Action: plans.Delete, + ActionReason: plans.ResourceInstanceDeleteBecauseEachKey, + Mode: addrs.ManagedResourceMode, + InstanceKey: addrs.StringKey("boop"), + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example["boop"] will be destroyed + # (because key ["boop"] is not in for_each map) + - resource "test_instance" "example" {} +`, + }, + "replace for no particular reason (delete first)": { + Action: plans.DeleteThenCreate, + ActionReason: plans.ResourceInstanceChangeNoReason, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example must be replaced +-/+ resource "test_instance" "example" {} +`, + }, + "replace for no particular reason (create first)": { + Action: plans.CreateThenDelete, + ActionReason: plans.ResourceInstanceChangeNoReason, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example must be replaced ++/- resource "test_instance" "example" {} +`, + }, + "replace by request (delete first)": { + Action: plans.DeleteThenCreate, + ActionReason: plans.ResourceInstanceReplaceByRequest, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be replaced, as requested +-/+ resource "test_instance" "example" {} +`, + }, + "replace by request (create first)": { + Action: plans.CreateThenDelete, + ActionReason: plans.ResourceInstanceReplaceByRequest, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be replaced, as requested ++/- resource "test_instance" "example" {} +`, + }, + "replace because tainted (delete first)": { + Action: plans.DeleteThenCreate, + ActionReason: plans.ResourceInstanceReplaceBecauseTainted, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example is tainted, so must be replaced +-/+ resource "test_instance" "example" {} +`, + }, + "replace because tainted (create first)": { + Action: plans.CreateThenDelete, + ActionReason: plans.ResourceInstanceReplaceBecauseTainted, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example is tainted, so must be replaced ++/- resource "test_instance" "example" {} +`, + }, + "replace because cannot update (delete first)": { + Action: plans.DeleteThenCreate, + ActionReason: plans.ResourceInstanceReplaceBecauseCannotUpdate, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + // This one has no special message, because the fuller explanation + // typically appears inline as a "# forces replacement" comment. + // (not shown here) + ExpectedOutput: ` # test_instance.example must be replaced +-/+ resource "test_instance" "example" {} +`, + }, + "replace because cannot update (create first)": { + Action: plans.CreateThenDelete, + ActionReason: plans.ResourceInstanceReplaceBecauseCannotUpdate, + Mode: addrs.ManagedResourceMode, + Before: emptyVal, + After: nullVal, + Schema: emptySchema, + RequiredReplace: cty.NewPathSet(), + // This one has no special message, because the fuller explanation + // typically appears inline as a "# forces replacement" comment. + // (not shown here) + ExpectedOutput: ` # test_instance.example must be replaced ++/- resource "test_instance" "example" {} +`, + }, + } + + runTestCases(t, testCases) +} + func TestResourceChange_sensitiveVariable(t *testing.T) { testCases := map[string]testCase{ "creation": { @@ -4479,7 +4702,7 @@ func TestResourceChange_moved(t *testing.T) { }, RequiredReplace: cty.NewPathSet(), ExpectedOutput: ` # test_instance.example will be updated in-place - # (test_instance.previous has moved to test_instance.example) + # (moved from test_instance.previous) ~ resource "test_instance" "example" { ~ bar = "baz" -> "boop" id = "12345" @@ -4524,7 +4747,9 @@ func TestResourceChange_moved(t *testing.T) { type testCase struct { Action plans.Action ActionReason plans.ResourceInstanceChangeActionReason + ModuleInst addrs.ModuleInstance Mode addrs.ResourceMode + InstanceKey addrs.InstanceKey DeposedKey states.DeposedKey Before cty.Value BeforeValMarks []cty.PathValueMarks @@ -4571,7 +4796,7 @@ func runTestCases(t *testing.T, testCases map[string]testCase) { Mode: tc.Mode, Type: "test_instance", Name: "example", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) + }.Instance(tc.InstanceKey).Absolute(tc.ModuleInst) prevRunAddr := tc.PrevRunAddr // If no previous run address is given, reuse the current address