From a1a713cf281d9d07ef2dad96d625703051e3e1c6 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 22 Sep 2021 17:58:41 -0700 Subject: [PATCH] core: Report ActionReasons when we plan to delete "orphans" There are a few different reasons why a resource instance tracked in the prior state might be considered an "orphan", but previously we reported them all identically in the planned changes. In order to help users understand the reason for a surprising planned delete, we'll now try to specify an additional reason for the planned deletion, covering all of the main reasons why that could happen. This commit only introduces the new detail to the plans.Changes result, though it also incidentally exposes it as part of the JSON plan result in order to keep that working without returning errors in these new cases. We'll expose this information in the human-oriented UI output in a subsequent commit. --- internal/command/jsonplan/plan.go | 10 ++ .../show-json/basic-delete/output.json | 1 + internal/plans/changes.go | 34 ++++ .../plans/internal/planproto/planfile.pb.go | 54 +++++-- .../plans/internal/planproto/planfile.proto | 5 + internal/plans/planfile/tfplan.go | 20 +++ ...sourceinstancechangeactionreason_string.go | 33 +++- internal/terraform/context_apply_test.go | 147 ++++++++++++++++++ internal/terraform/context_plan_test.go | 37 ++++- .../terraform/node_resource_plan_orphan.go | 112 +++++++++++++ website/docs/internals/json-format.html.md | 14 ++ 11 files changed, 445 insertions(+), 22 deletions(-) diff --git a/internal/command/jsonplan/plan.go b/internal/command/jsonplan/plan.go index b2bf2cceb..c0b726422 100644 --- a/internal/command/jsonplan/plan.go +++ b/internal/command/jsonplan/plan.go @@ -342,6 +342,16 @@ func (p *plan) marshalResourceChanges(resources []*plans.ResourceInstanceChangeS r.ActionReason = "replace_because_tainted" case plans.ResourceInstanceReplaceByRequest: r.ActionReason = "replace_by_request" + case plans.ResourceInstanceDeleteBecauseNoResourceConfig: + r.ActionReason = "delete_because_no_resource_config" + case plans.ResourceInstanceDeleteBecauseWrongRepetition: + r.ActionReason = "delete_because_wrong_repetition" + case plans.ResourceInstanceDeleteBecauseCountIndex: + r.ActionReason = "delete_because_count_index" + case plans.ResourceInstanceDeleteBecauseEachKey: + r.ActionReason = "delete_because_each_key" + case plans.ResourceInstanceDeleteBecauseNoModule: + r.ActionReason = "delete_because_no_module" default: return nil, fmt.Errorf("resource %s has an unsupported action reason %s", r.Address, rc.ActionReason) } diff --git a/internal/command/testdata/show-json/basic-delete/output.json b/internal/command/testdata/show-json/basic-delete/output.json index 9ebea2058..ae6e67f76 100644 --- a/internal/command/testdata/show-json/basic-delete/output.json +++ b/internal/command/testdata/show-json/basic-delete/output.json @@ -60,6 +60,7 @@ "type": "test_instance", "provider_name": "registry.terraform.io/hashicorp/test", "name": "test-delete", + "action_reason": "delete_because_no_resource_config", "change": { "actions": [ "delete" diff --git a/internal/plans/changes.go b/internal/plans/changes.go index c9aa38fd3..510eb2c7c 100644 --- a/internal/plans/changes.go +++ b/internal/plans/changes.go @@ -349,6 +349,40 @@ const ( // the ResourceInstanceChange object to give information about specifically // which arguments changed in a non-updatable way. ResourceInstanceReplaceBecauseCannotUpdate ResourceInstanceChangeActionReason = 'F' + + // ResourceInstanceDeleteBecauseNoResourceConfig indicates that the + // resource instance is planned to be deleted because there's no + // corresponding resource configuration block in the configuration. + ResourceInstanceDeleteBecauseNoResourceConfig ResourceInstanceChangeActionReason = 'N' + + // ResourceInstanceDeleteBecauseWrongRepetition indicates that the + // resource instance is planned to be deleted because the instance key + // type isn't consistent with the repetition mode selected in the + // resource configuration. + ResourceInstanceDeleteBecauseWrongRepetition ResourceInstanceChangeActionReason = 'W' + + // ResourceInstanceDeleteBecauseCountIndex indicates that the resource + // instance is planned to be deleted because its integer instance key + // is out of range for the current configured resource "count" value. + ResourceInstanceDeleteBecauseCountIndex ResourceInstanceChangeActionReason = 'C' + + // ResourceInstanceDeleteBecauseEachKey indicates that the resource + // instance is planned to be deleted because its string instance key + // isn't one of the keys included in the current configured resource + // "for_each" value. + ResourceInstanceDeleteBecauseEachKey ResourceInstanceChangeActionReason = 'E' + + // ResourceInstanceDeleteBecauseNoModule indicates that the resource + // instance is planned to be deleted because it belongs to a module + // instance that's no longer declared in the configuration. + // + // This is less specific than the reasons we return for the various ways + // a resource instance itself can be no longer declared, including both + // the total removal of a module block and changes to its count/for_each + // arguments. This difference in detail is out of pragmatism, because + // potentially multiple nested modules could all contribute conflicting + // specific reasons for a particular instance to no longer be declared. + ResourceInstanceDeleteBecauseNoModule ResourceInstanceChangeActionReason = 'M' ) // OutputChange describes a change to an output value. diff --git a/internal/plans/internal/planproto/planfile.pb.go b/internal/plans/internal/planproto/planfile.pb.go index a612c03a3..beb50852a 100644 --- a/internal/plans/internal/planproto/planfile.pb.go +++ b/internal/plans/internal/planproto/planfile.pb.go @@ -145,10 +145,15 @@ func (Action) EnumDescriptor() ([]byte, []int) { type ResourceInstanceActionReason int32 const ( - ResourceInstanceActionReason_NONE ResourceInstanceActionReason = 0 - ResourceInstanceActionReason_REPLACE_BECAUSE_TAINTED ResourceInstanceActionReason = 1 - ResourceInstanceActionReason_REPLACE_BY_REQUEST ResourceInstanceActionReason = 2 - ResourceInstanceActionReason_REPLACE_BECAUSE_CANNOT_UPDATE ResourceInstanceActionReason = 3 + ResourceInstanceActionReason_NONE ResourceInstanceActionReason = 0 + ResourceInstanceActionReason_REPLACE_BECAUSE_TAINTED ResourceInstanceActionReason = 1 + ResourceInstanceActionReason_REPLACE_BY_REQUEST ResourceInstanceActionReason = 2 + ResourceInstanceActionReason_REPLACE_BECAUSE_CANNOT_UPDATE ResourceInstanceActionReason = 3 + ResourceInstanceActionReason_DELETE_BECAUSE_NO_RESOURCE_CONFIG ResourceInstanceActionReason = 4 + ResourceInstanceActionReason_DELETE_BECAUSE_WRONG_REPETITION ResourceInstanceActionReason = 5 + ResourceInstanceActionReason_DELETE_BECAUSE_COUNT_INDEX ResourceInstanceActionReason = 6 + ResourceInstanceActionReason_DELETE_BECAUSE_EACH_KEY ResourceInstanceActionReason = 7 + ResourceInstanceActionReason_DELETE_BECAUSE_NO_MODULE ResourceInstanceActionReason = 8 ) // Enum value maps for ResourceInstanceActionReason. @@ -158,12 +163,22 @@ var ( 1: "REPLACE_BECAUSE_TAINTED", 2: "REPLACE_BY_REQUEST", 3: "REPLACE_BECAUSE_CANNOT_UPDATE", + 4: "DELETE_BECAUSE_NO_RESOURCE_CONFIG", + 5: "DELETE_BECAUSE_WRONG_REPETITION", + 6: "DELETE_BECAUSE_COUNT_INDEX", + 7: "DELETE_BECAUSE_EACH_KEY", + 8: "DELETE_BECAUSE_NO_MODULE", } ResourceInstanceActionReason_value = map[string]int32{ - "NONE": 0, - "REPLACE_BECAUSE_TAINTED": 1, - "REPLACE_BY_REQUEST": 2, - "REPLACE_BECAUSE_CANNOT_UPDATE": 3, + "NONE": 0, + "REPLACE_BECAUSE_TAINTED": 1, + "REPLACE_BY_REQUEST": 2, + "REPLACE_BECAUSE_CANNOT_UPDATE": 3, + "DELETE_BECAUSE_NO_RESOURCE_CONFIG": 4, + "DELETE_BECAUSE_WRONG_REPETITION": 5, + "DELETE_BECAUSE_COUNT_INDEX": 6, + "DELETE_BECAUSE_EACH_KEY": 7, + "DELETE_BECAUSE_NO_MODULE": 8, } ) @@ -1085,7 +1100,7 @@ var file_planfile_proto_rawDesc = []byte{ 0x12, 0x16, 0x0a, 0x12, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x54, 0x48, 0x45, 0x4e, 0x5f, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x10, 0x06, 0x12, 0x16, 0x0a, 0x12, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x5f, 0x54, 0x48, 0x45, 0x4e, 0x5f, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x10, 0x07, - 0x2a, 0x80, 0x01, 0x0a, 0x1c, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x49, 0x6e, 0x73, + 0x2a, 0xa7, 0x02, 0x0a, 0x1c, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x61, 0x73, 0x6f, 0x6e, 0x12, 0x08, 0x0a, 0x04, 0x4e, 0x4f, 0x4e, 0x45, 0x10, 0x00, 0x12, 0x1b, 0x0a, 0x17, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x54, @@ -1093,11 +1108,22 @@ var file_planfile_proto_rawDesc = []byte{ 0x41, 0x43, 0x45, 0x5f, 0x42, 0x59, 0x5f, 0x52, 0x45, 0x51, 0x55, 0x45, 0x53, 0x54, 0x10, 0x02, 0x12, 0x21, 0x0a, 0x1d, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x43, 0x41, 0x4e, 0x4e, 0x4f, 0x54, 0x5f, 0x55, 0x50, 0x44, 0x41, 0x54, - 0x45, 0x10, 0x03, 0x42, 0x42, 0x5a, 0x40, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, - 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x74, 0x65, 0x72, 0x72, - 0x61, 0x66, 0x6f, 0x72, 0x6d, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, - 0x6c, 0x61, 0x6e, 0x73, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, - 0x61, 0x6e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x45, 0x10, 0x03, 0x12, 0x25, 0x0a, 0x21, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, + 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x4e, 0x4f, 0x5f, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, + 0x45, 0x5f, 0x43, 0x4f, 0x4e, 0x46, 0x49, 0x47, 0x10, 0x04, 0x12, 0x23, 0x0a, 0x1f, 0x44, 0x45, + 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x57, 0x52, 0x4f, + 0x4e, 0x47, 0x5f, 0x52, 0x45, 0x50, 0x45, 0x54, 0x49, 0x54, 0x49, 0x4f, 0x4e, 0x10, 0x05, 0x12, + 0x1e, 0x0a, 0x1a, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, + 0x45, 0x5f, 0x43, 0x4f, 0x55, 0x4e, 0x54, 0x5f, 0x49, 0x4e, 0x44, 0x45, 0x58, 0x10, 0x06, 0x12, + 0x1b, 0x0a, 0x17, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, + 0x45, 0x5f, 0x45, 0x41, 0x43, 0x48, 0x5f, 0x4b, 0x45, 0x59, 0x10, 0x07, 0x12, 0x1c, 0x0a, 0x18, + 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x4e, + 0x4f, 0x5f, 0x4d, 0x4f, 0x44, 0x55, 0x4c, 0x45, 0x10, 0x08, 0x42, 0x42, 0x5a, 0x40, 0x67, 0x69, + 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, + 0x72, 0x70, 0x2f, 0x74, 0x65, 0x72, 0x72, 0x61, 0x66, 0x6f, 0x72, 0x6d, 0x2f, 0x69, 0x6e, 0x74, + 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, 0x61, 0x6e, 0x73, 0x2f, 0x69, 0x6e, 0x74, 0x65, + 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, 0x61, 0x6e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/internal/plans/internal/planproto/planfile.proto b/internal/plans/internal/planproto/planfile.proto index d1427bfbe..1bbe7425e 100644 --- a/internal/plans/internal/planproto/planfile.proto +++ b/internal/plans/internal/planproto/planfile.proto @@ -131,6 +131,11 @@ enum ResourceInstanceActionReason { REPLACE_BECAUSE_TAINTED = 1; REPLACE_BY_REQUEST = 2; REPLACE_BECAUSE_CANNOT_UPDATE = 3; + DELETE_BECAUSE_NO_RESOURCE_CONFIG = 4; + DELETE_BECAUSE_WRONG_REPETITION = 5; + DELETE_BECAUSE_COUNT_INDEX = 6; + DELETE_BECAUSE_EACH_KEY = 7; + DELETE_BECAUSE_NO_MODULE = 8; } message ResourceInstanceChange { diff --git a/internal/plans/planfile/tfplan.go b/internal/plans/planfile/tfplan.go index 8cfd3694f..87d21822e 100644 --- a/internal/plans/planfile/tfplan.go +++ b/internal/plans/planfile/tfplan.go @@ -228,6 +228,16 @@ func resourceChangeFromTfplan(rawChange *planproto.ResourceInstanceChange) (*pla ret.ActionReason = plans.ResourceInstanceReplaceBecauseTainted case planproto.ResourceInstanceActionReason_REPLACE_BY_REQUEST: ret.ActionReason = plans.ResourceInstanceReplaceByRequest + case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_RESOURCE_CONFIG: + ret.ActionReason = plans.ResourceInstanceDeleteBecauseNoResourceConfig + case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_WRONG_REPETITION: + ret.ActionReason = plans.ResourceInstanceDeleteBecauseWrongRepetition + case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_COUNT_INDEX: + ret.ActionReason = plans.ResourceInstanceDeleteBecauseCountIndex + case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_EACH_KEY: + ret.ActionReason = plans.ResourceInstanceDeleteBecauseEachKey + case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_MODULE: + ret.ActionReason = plans.ResourceInstanceDeleteBecauseNoModule default: return nil, fmt.Errorf("resource has invalid action reason %s", rawChange.ActionReason) } @@ -499,6 +509,16 @@ func resourceChangeToTfplan(change *plans.ResourceInstanceChangeSrc) (*planproto ret.ActionReason = planproto.ResourceInstanceActionReason_REPLACE_BECAUSE_TAINTED case plans.ResourceInstanceReplaceByRequest: ret.ActionReason = planproto.ResourceInstanceActionReason_REPLACE_BY_REQUEST + case plans.ResourceInstanceDeleteBecauseNoResourceConfig: + ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_RESOURCE_CONFIG + case plans.ResourceInstanceDeleteBecauseWrongRepetition: + ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_WRONG_REPETITION + case plans.ResourceInstanceDeleteBecauseCountIndex: + ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_COUNT_INDEX + case plans.ResourceInstanceDeleteBecauseEachKey: + ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_EACH_KEY + case plans.ResourceInstanceDeleteBecauseNoModule: + ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_MODULE default: return nil, fmt.Errorf("resource %s has unsupported action reason %s", change.Addr, change.ActionReason) } diff --git a/internal/plans/resourceinstancechangeactionreason_string.go b/internal/plans/resourceinstancechangeactionreason_string.go index 0731f6759..135e6d2c6 100644 --- a/internal/plans/resourceinstancechangeactionreason_string.go +++ b/internal/plans/resourceinstancechangeactionreason_string.go @@ -12,25 +12,46 @@ func _() { _ = x[ResourceInstanceReplaceBecauseTainted-84] _ = x[ResourceInstanceReplaceByRequest-82] _ = x[ResourceInstanceReplaceBecauseCannotUpdate-70] + _ = x[ResourceInstanceDeleteBecauseNoResourceConfig-78] + _ = x[ResourceInstanceDeleteBecauseWrongRepetition-87] + _ = x[ResourceInstanceDeleteBecauseCountIndex-67] + _ = x[ResourceInstanceDeleteBecauseEachKey-69] + _ = x[ResourceInstanceDeleteBecauseNoModule-77] } const ( _ResourceInstanceChangeActionReason_name_0 = "ResourceInstanceChangeNoReason" - _ResourceInstanceChangeActionReason_name_1 = "ResourceInstanceReplaceBecauseCannotUpdate" - _ResourceInstanceChangeActionReason_name_2 = "ResourceInstanceReplaceByRequest" - _ResourceInstanceChangeActionReason_name_3 = "ResourceInstanceReplaceBecauseTainted" + _ResourceInstanceChangeActionReason_name_1 = "ResourceInstanceDeleteBecauseCountIndex" + _ResourceInstanceChangeActionReason_name_2 = "ResourceInstanceDeleteBecauseEachKeyResourceInstanceReplaceBecauseCannotUpdate" + _ResourceInstanceChangeActionReason_name_3 = "ResourceInstanceDeleteBecauseNoModuleResourceInstanceDeleteBecauseNoResourceConfig" + _ResourceInstanceChangeActionReason_name_4 = "ResourceInstanceReplaceByRequest" + _ResourceInstanceChangeActionReason_name_5 = "ResourceInstanceReplaceBecauseTainted" + _ResourceInstanceChangeActionReason_name_6 = "ResourceInstanceDeleteBecauseWrongRepetition" +) + +var ( + _ResourceInstanceChangeActionReason_index_2 = [...]uint8{0, 36, 78} + _ResourceInstanceChangeActionReason_index_3 = [...]uint8{0, 37, 82} ) func (i ResourceInstanceChangeActionReason) String() string { switch { case i == 0: return _ResourceInstanceChangeActionReason_name_0 - case i == 70: + case i == 67: return _ResourceInstanceChangeActionReason_name_1 + case 69 <= i && i <= 70: + i -= 69 + return _ResourceInstanceChangeActionReason_name_2[_ResourceInstanceChangeActionReason_index_2[i]:_ResourceInstanceChangeActionReason_index_2[i+1]] + case 77 <= i && i <= 78: + i -= 77 + return _ResourceInstanceChangeActionReason_name_3[_ResourceInstanceChangeActionReason_index_3[i]:_ResourceInstanceChangeActionReason_index_3[i+1]] case i == 82: - return _ResourceInstanceChangeActionReason_name_2 + return _ResourceInstanceChangeActionReason_name_4 case i == 84: - return _ResourceInstanceChangeActionReason_name_3 + return _ResourceInstanceChangeActionReason_name_5 + case i == 87: + return _ResourceInstanceChangeActionReason_name_6 default: return "ResourceInstanceChangeActionReason(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/internal/terraform/context_apply_test.go b/internal/terraform/context_apply_test.go index babf067a5..a525ab52d 100644 --- a/internal/terraform/context_apply_test.go +++ b/internal/terraform/context_apply_test.go @@ -2127,6 +2127,30 @@ func TestContext2Apply_countDecreaseToOneCorrupted(t *testing.T) { t.Fatalf("wrong plan result\ngot:\n%s\nwant:\n%s", got, want) } } + { + change := plan.Changes.ResourceInstance(mustResourceInstanceAddr("aws_instance.foo[0]")) + if change == nil { + t.Fatalf("no planned change for instance zero") + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for instance zero %s; want %s", got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseWrongRepetition; got != want { + t.Errorf("wrong action reason for instance zero %s; want %s", got, want) + } + } + { + change := plan.Changes.ResourceInstance(mustResourceInstanceAddr("aws_instance.foo")) + if change == nil { + t.Fatalf("no planned change for no-key instance") + } + if got, want := change.Action, plans.NoOp; got != want { + t.Errorf("wrong action for no-key instance %s; want %s", got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + t.Errorf("wrong action reason for no-key instance %s; want %s", got, want) + } + } s, diags := ctx.Apply(plan, m) if diags.HasErrors() { @@ -2562,6 +2586,20 @@ func TestContext2Apply_orphanResource(t *testing.T) { }) plan, diags = ctx.Plan(m, state, DefaultPlanOpts) assertNoErrors(t, diags) + { + addr := mustResourceInstanceAddr("test_thing.one[0]") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseNoResourceConfig; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + } + state, diags = ctx.Apply(plan, m) assertNoErrors(t, diags) @@ -2613,6 +2651,22 @@ func TestContext2Apply_moduleOrphanInheritAlias(t *testing.T) { plan, diags := ctx.Plan(m, state, DefaultPlanOpts) assertNoErrors(t, diags) + { + addr := mustResourceInstanceAddr("module.child.aws_instance.bar") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + // This should ideally be ResourceInstanceDeleteBecauseNoModule, but + // the codepath deciding this doesn't currently have enough information + // to differentiate, and so this is a compromise. + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseNoResourceConfig; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + } state, diags = ctx.Apply(plan, m) if diags.HasErrors() { @@ -8898,6 +8952,43 @@ func TestContext2Apply_scaleInMultivarRef(t *testing.T) { }, }) assertNoErrors(t, diags) + { + addr := mustResourceInstanceAddr("aws_instance.one[0]") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + // This test was originally written with Terraform v0.11 and earlier + // in mind, so it declares a no-key instance of aws_instance.one, + // but its configuration sets count (to zero) and so we end up first + // moving the no-key instance to the zero key and then planning to + // destroy the zero key. + if got, want := change.PrevRunAddr, mustResourceInstanceAddr("aws_instance.one"); !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", addr, got, want) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseCountIndex; got != want { + t.Errorf("wrong action reason for %s %s; want %s", addr, got, want) + } + } + { + addr := mustResourceInstanceAddr("aws_instance.two") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + if got, want := change.PrevRunAddr, mustResourceInstanceAddr("aws_instance.two"); !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", addr, got, want) + } + if got, want := change.Action, plans.Update; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + t.Errorf("wrong action reason for %s %s; want %s", addr, got, want) + } + } // Applying the plan should now succeed _, diags = ctx.Apply(plan, m) @@ -10960,6 +11051,38 @@ locals { if diags.HasErrors() { t.Fatal(diags.ErrWithWarnings()) } + { + addr := mustResourceInstanceAddr("test_instance.a[0]") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + if got, want := change.PrevRunAddr, mustResourceInstanceAddr("test_instance.a[0]"); !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", addr, got, want) + } + if got, want := change.Action, plans.NoOp; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + t.Errorf("wrong action reason for %s %s; want %s", addr, got, want) + } + } + { + addr := mustResourceInstanceAddr("test_instance.a[1]") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + if got, want := change.PrevRunAddr, mustResourceInstanceAddr("test_instance.a[1]"); !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", addr, got, want) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseCountIndex; got != want { + t.Errorf("wrong action reason for %s %s; want %s", addr, got, want) + } + } state, diags = ctx.Apply(plan, m) if diags.HasErrors() { @@ -10991,6 +11114,30 @@ locals { if diags.HasErrors() { t.Fatal(diags.ErrWithWarnings()) } + { + addr := mustResourceInstanceAddr("test_instance.a[0]") + change := plan.Changes.ResourceInstance(addr) + if change == nil { + t.Fatalf("no planned change for %s", addr) + } + if got, want := change.PrevRunAddr, mustResourceInstanceAddr("test_instance.a[0]"); !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", addr, got, want) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", addr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseCountIndex; got != want { + t.Errorf("wrong action reason for %s %s; want %s", addr, got, want) + } + } + { + addr := mustResourceInstanceAddr("test_instance.a[1]") + change := plan.Changes.ResourceInstance(addr) + if change != nil { + // It was already removed in the previous plan/apply + t.Errorf("unexpected planned change for %s", addr) + } + } state, diags = ctx.Apply(plan, m) if diags.HasErrors() { diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index 475bcb661..9cb4c8925 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -3559,7 +3559,7 @@ func TestContext2Plan_orphan(t *testing.T) { if res.Action != plans.Delete { t.Fatalf("resource %s should be removed", i) } - if got, want := ric.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + if got, want := ric.ActionReason, plans.ResourceInstanceDeleteBecauseNoResourceConfig; got != want { t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) } case "aws_instance.foo": @@ -6138,8 +6138,41 @@ resource "test_instance" "b" { }, }) - _, diags := ctx.Plan(m, state, DefaultPlanOpts) + plan, diags := ctx.Plan(m, state, DefaultPlanOpts) assertNoErrors(t, diags) + + t.Run("test_instance.a[0]", func(t *testing.T) { + instAddr := mustResourceInstanceAddr("test_instance.a[0]") + change := plan.Changes.ResourceInstance(instAddr) + if change == nil { + t.Fatalf("no planned change for %s", instAddr) + } + if got, want := change.PrevRunAddr, instAddr; !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", instAddr, got, want) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", instAddr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseWrongRepetition; got != want { + t.Errorf("wrong action reason for %s %s; want %s", instAddr, got, want) + } + }) + t.Run("test_instance.b", func(t *testing.T) { + instAddr := mustResourceInstanceAddr("test_instance.b") + change := plan.Changes.ResourceInstance(instAddr) + if change == nil { + t.Fatalf("no planned change for %s", instAddr) + } + if got, want := change.PrevRunAddr, instAddr; !want.Equal(got) { + t.Errorf("wrong previous run address for %s %s; want %s", instAddr, got, want) + } + if got, want := change.Action, plans.Delete; got != want { + t.Errorf("wrong action for %s %s; want %s", instAddr, got, want) + } + if got, want := change.ActionReason, plans.ResourceInstanceDeleteBecauseWrongRepetition; got != want { + t.Errorf("wrong action reason for %s %s; want %s", instAddr, got, want) + } + }) } func TestContext2Plan_targetedModuleInstance(t *testing.T) { diff --git a/internal/terraform/node_resource_plan_orphan.go b/internal/terraform/node_resource_plan_orphan.go index 2c28d88a3..7f9a683f1 100644 --- a/internal/terraform/node_resource_plan_orphan.go +++ b/internal/terraform/node_resource_plan_orphan.go @@ -134,6 +134,11 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon return diags } + // We might be able to offer an approximate reason for why we are + // planning to delete this object. (This is best-effort; we might + // sometimes not have a reason.) + change.ActionReason = n.deleteActionReason(ctx) + diags = diags.Append(n.writeChange(ctx, change, "")) if diags.HasErrors() { return diags @@ -148,3 +153,110 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon return diags } + +func (n *NodePlannableResourceInstanceOrphan) deleteActionReason(ctx EvalContext) plans.ResourceInstanceChangeActionReason { + cfg := n.Config + if cfg == nil { + // NOTE: We'd ideally detect if the containing module is what's missing + // and then use ResourceInstanceDeleteBecauseNoModule for that case, + // but we don't currently have access to the full configuration here, + // so we need to be less specific. + return plans.ResourceInstanceDeleteBecauseNoResourceConfig + } + + switch n.Addr.Resource.Key.(type) { + case nil: // no instance key at all + if cfg.Count != nil || cfg.ForEach != nil { + return plans.ResourceInstanceDeleteBecauseWrongRepetition + } + case addrs.IntKey: + if cfg.Count == nil { + // This resource isn't using "count" at all, then + return plans.ResourceInstanceDeleteBecauseWrongRepetition + } + + expander := ctx.InstanceExpander() + if expander == nil { + break // only for tests that produce an incomplete MockEvalContext + } + insts := expander.ExpandResource(n.Addr.ContainingResource()) + + declared := false + for _, inst := range insts { + if n.Addr.Equal(inst) { + declared = true + } + } + if !declared { + // This instance key is outside of the configured range + return plans.ResourceInstanceDeleteBecauseCountIndex + } + case addrs.StringKey: + if cfg.ForEach == nil { + // This resource isn't using "for_each" at all, then + return plans.ResourceInstanceDeleteBecauseWrongRepetition + } + + expander := ctx.InstanceExpander() + if expander == nil { + break // only for tests that produce an incomplete MockEvalContext + } + insts := expander.ExpandResource(n.Addr.ContainingResource()) + + declared := false + for _, inst := range insts { + if n.Addr.Equal(inst) { + declared = true + } + } + if !declared { + // This instance key is outside of the configured range + return plans.ResourceInstanceDeleteBecauseEachKey + } + } + + // If we get here then the instance key type matches the configured + // repetition mode, and so we need to consider whether the key itself + // is within the range of the repetition construct. + if expander := ctx.InstanceExpander(); expander != nil { // (sometimes nil in MockEvalContext in tests) + // First we'll check whether our containing module instance still + // exists, so we can talk about that differently in the reason. + declared := false + for _, inst := range expander.ExpandModule(n.Addr.Module.Module()) { + if n.Addr.Module.Equal(inst) { + declared = true + break + } + } + if !declared { + return plans.ResourceInstanceDeleteBecauseNoModule + } + + // Now we've proven that we're in a still-existing module instance, + // we'll see if our instance key matches something actually declared. + declared = false + for _, inst := range expander.ExpandResource(n.Addr.ContainingResource()) { + if n.Addr.Equal(inst) { + declared = true + break + } + } + if !declared { + // Because we already checked that the key _type_ was correct + // above, we can assume that any mismatch here is a range error, + // and thus we just need to decide which of the two range + // errors we're going to return. + switch n.Addr.Resource.Key.(type) { + case addrs.IntKey: + return plans.ResourceInstanceDeleteBecauseCountIndex + case addrs.StringKey: + return plans.ResourceInstanceDeleteBecauseEachKey + } + } + } + + // If we didn't find any specific reason to report, we'll report "no reason" + // as a fallback, which means the UI should just state it'll be deleted + // without any explicit reasoning. + return plans.ResourceInstanceChangeNoReason +} diff --git a/website/docs/internals/json-format.html.md b/website/docs/internals/json-format.html.md index ffe54c2cc..aa4df209a 100644 --- a/website/docs/internals/json-format.html.md +++ b/website/docs/internals/json-format.html.md @@ -149,6 +149,20 @@ For ease of consumption by callers, the plan representation includes a partial r // - "replace_by_request": the user explicitly called for this object // to be replaced as an option when creating the plan, which therefore // overrode what would have been a "no-op" or "update" action otherwise. + // - "delete_because_no_resource_config": Terraform found no resource + // configuration corresponding to this instance. + // - "delete_because_no_module": The resource instance belongs to a + // module instance that's no longer declared, perhaps due to changing + // the "count" or "for_each" argument on one of the containing modules. + // - "delete_because_wrong_repetition": The instance key portion of the + // resource address isn't of a suitable type for the corresponding + // resource's configured repetition mode (count, for_each, or neither). + // - "delete_because_count_index": The corresponding resource uses count, + // but the instance key is out of range for the currently-configured + // count value. + // - "delete_because_each_key": The corresponding resource uses for_each, + // but the instance key doesn't match any of the keys in the + // currently-configured for_each value. // // If there is no special reason to note, Terraform will omit this // property altogether.