From 95d43c912317d3cef3e9f3c7ad00bb9fc880a2f1 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 29 Apr 2022 17:28:43 -0700 Subject: [PATCH] core: Report reason for deferring data read until apply We have two different reasons why a data resource might be read only during apply, rather than during planning as usual: the configuration contains unknown values, or the data resource as a whole depends on a managed resource which itself has a change pending. However, we didn't previously distinguish these two in a way that allowed the UI to describe the difference, and so we confusingly reported both as "config refers to values not yet known", which in turn led to a number of reasonable questions about why Terraform was claiming that but then immediately below showing the configuration entirely known. Now we'll use our existing "ActionReason" mechanism to tell the UI layer which of the two reasons applies to a particular data resource instance. The "dependency pending" situation tends to happen in conjunction with "config unknown", so we'll prefer to refer that the configuration is unknown if both are true. --- internal/command/format/diff.go | 10 ++- internal/command/format/diff_test.go | 64 +++++++++++++++++++ internal/command/jsonplan/plan.go | 4 ++ internal/command/views/json/change.go | 6 ++ internal/plans/changes.go | 12 ++++ .../plans/internal/planproto/planfile.pb.go | 57 ++++++++++------- .../plans/internal/planproto/planfile.proto | 2 + internal/plans/planfile/tfplan.go | 8 +++ ...sourceinstancechangeactionreason_string.go | 32 ++++++---- internal/terraform/context_plan_test.go | 22 ++++++- .../node_resource_abstract_instance.go | 26 +++++--- website/docs/internals/json-format.mdx | 7 ++ 12 files changed, 204 insertions(+), 46 deletions(-) diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index 5abd2c9274ef..63157b5f1959 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -71,7 +71,13 @@ func ResourceChange( case plans.Create: buf.WriteString(fmt.Sprintf(color.Color("[bold] # %s[reset] will be created"), dispAddr)) case plans.Read: - buf.WriteString(fmt.Sprintf(color.Color("[bold] # %s[reset] will be read during apply\n # (config refers to values not yet known)"), dispAddr)) + buf.WriteString(fmt.Sprintf(color.Color("[bold] # %s[reset] will be read during apply"), dispAddr)) + switch change.ActionReason { + case plans.ResourceInstanceReadBecauseConfigUnknown: + buf.WriteString("\n # (config refers to values not yet known)") + case plans.ResourceInstanceReadBecauseDependencyPending: + buf.WriteString("\n # (depends on a resource or a module with changes pending)") + } case plans.Update: switch language { case DiffLanguageProposedChange: @@ -166,7 +172,7 @@ func ResourceChange( )) case addrs.DataResourceMode: buf.WriteString(fmt.Sprintf( - "data %q %q ", + "data %q %q", addr.Resource.Resource.Type, addr.Resource.Resource.Name, )) diff --git a/internal/command/format/diff_test.go b/internal/command/format/diff_test.go index cca13fe158a9..8f8ebfad6e81 100644 --- a/internal/command/format/diff_test.go +++ b/internal/command/format/diff_test.go @@ -532,6 +532,70 @@ new line + forced = "example" # forces replacement name = "name" } +`, + }, + "read during apply because of unknown configuration": { + Action: plans.Read, + ActionReason: plans.ResourceInstanceReadBecauseConfigUnknown, + Mode: addrs.DataResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("name"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("name"), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "name": {Type: cty.String, Optional: true}, + }, + }, + ExpectedOutput: ` # data.test_instance.example will be read during apply + # (config refers to values not yet known) + <= data "test_instance" "example" { + name = "name" + } +`, + }, + "read during apply because of pending changes to upstream dependency": { + Action: plans.Read, + ActionReason: plans.ResourceInstanceReadBecauseDependencyPending, + Mode: addrs.DataResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("name"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("name"), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "name": {Type: cty.String, Optional: true}, + }, + }, + ExpectedOutput: ` # data.test_instance.example will be read during apply + # (depends on a resource or a module with changes pending) + <= data "test_instance" "example" { + name = "name" + } +`, + }, + "read during apply for unspecified reason": { + Action: plans.Read, + Mode: addrs.DataResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("name"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("name"), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "name": {Type: cty.String, Optional: true}, + }, + }, + ExpectedOutput: ` # data.test_instance.example will be read during apply + <= data "test_instance" "example" { + name = "name" + } `, }, "show all identifying attributes even if unchanged": { diff --git a/internal/command/jsonplan/plan.go b/internal/command/jsonplan/plan.go index 5b799b812c1f..0a4f9bcd6e88 100644 --- a/internal/command/jsonplan/plan.go +++ b/internal/command/jsonplan/plan.go @@ -405,6 +405,10 @@ func (p *plan) marshalResourceChanges(resources []*plans.ResourceInstanceChangeS r.ActionReason = "delete_because_each_key" case plans.ResourceInstanceDeleteBecauseNoModule: r.ActionReason = "delete_because_no_module" + case plans.ResourceInstanceReadBecauseConfigUnknown: + r.ActionReason = "read_because_config_unknown" + case plans.ResourceInstanceReadBecauseDependencyPending: + r.ActionReason = "read_because_dependency_pending" default: return nil, fmt.Errorf("resource %s has an unsupported action reason %s", r.Address, rc.ActionReason) } diff --git a/internal/command/views/json/change.go b/internal/command/views/json/change.go index 2036b36761b6..21188bfaf5d3 100644 --- a/internal/command/views/json/change.go +++ b/internal/command/views/json/change.go @@ -80,6 +80,8 @@ const ( ReasonDeleteBecauseCountIndex ChangeReason = "delete_because_count_index" ReasonDeleteBecauseEachKey ChangeReason = "delete_because_each_key" ReasonDeleteBecauseNoModule ChangeReason = "delete_because_no_module" + ReasonReadBecauseConfigUnknown ChangeReason = "read_because_config_unknown" + ReasonReadBecauseDependencyPending ChangeReason = "read_because_dependency_pending" ) func changeReason(reason plans.ResourceInstanceChangeActionReason) ChangeReason { @@ -104,6 +106,10 @@ func changeReason(reason plans.ResourceInstanceChangeActionReason) ChangeReason return ReasonDeleteBecauseEachKey case plans.ResourceInstanceDeleteBecauseNoModule: return ReasonDeleteBecauseNoModule + case plans.ResourceInstanceReadBecauseConfigUnknown: + return ReasonReadBecauseConfigUnknown + case plans.ResourceInstanceReadBecauseDependencyPending: + return ReasonReadBecauseDependencyPending default: // This should never happen, but there's no good way to guarantee // exhaustive handling of the enum, so a generic fall back is better diff --git a/internal/plans/changes.go b/internal/plans/changes.go index 439ecb38a58c..d79ef0fcf473 100644 --- a/internal/plans/changes.go +++ b/internal/plans/changes.go @@ -407,6 +407,18 @@ const ( // potentially multiple nested modules could all contribute conflicting // specific reasons for a particular instance to no longer be declared. ResourceInstanceDeleteBecauseNoModule ResourceInstanceChangeActionReason = 'M' + + // ResourceInstanceReadBecauseConfigUnknown indicates that the resource + // must be read during apply (rather than during planning) because its + // configuration contains unknown values. This reason applies only to + // data resources. + ResourceInstanceReadBecauseConfigUnknown ResourceInstanceChangeActionReason = '?' + + // ResourceInstanceReadBecauseDependencyPending indicates that the resource + // must be read during apply (rather than during planning) because it + // depends on a managed resource instance which has its own changes + // pending. + ResourceInstanceReadBecauseDependencyPending ResourceInstanceChangeActionReason = '!' ) // 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 55cc9cf97833..93b328d39dc6 100644 --- a/internal/plans/internal/planproto/planfile.pb.go +++ b/internal/plans/internal/planproto/planfile.pb.go @@ -150,21 +150,25 @@ const ( ResourceInstanceActionReason_DELETE_BECAUSE_EACH_KEY ResourceInstanceActionReason = 7 ResourceInstanceActionReason_DELETE_BECAUSE_NO_MODULE ResourceInstanceActionReason = 8 ResourceInstanceActionReason_REPLACE_BY_TRIGGERS ResourceInstanceActionReason = 9 + ResourceInstanceActionReason_READ_BECAUSE_CONFIG_UNKNOWN ResourceInstanceActionReason = 10 + ResourceInstanceActionReason_READ_BECAUSE_DEPENDENCY_PENDING ResourceInstanceActionReason = 11 ) // Enum value maps for ResourceInstanceActionReason. var ( ResourceInstanceActionReason_name = map[int32]string{ - 0: "NONE", - 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", - 9: "REPLACE_BY_TRIGGERS", + 0: "NONE", + 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", + 9: "REPLACE_BY_TRIGGERS", + 10: "READ_BECAUSE_CONFIG_UNKNOWN", + 11: "READ_BECAUSE_DEPENDENCY_PENDING", } ResourceInstanceActionReason_value = map[string]int32{ "NONE": 0, @@ -177,6 +181,8 @@ var ( "DELETE_BECAUSE_EACH_KEY": 7, "DELETE_BECAUSE_NO_MODULE": 8, "REPLACE_BY_TRIGGERS": 9, + "READ_BECAUSE_CONFIG_UNKNOWN": 10, + "READ_BECAUSE_DEPENDENCY_PENDING": 11, } ) @@ -1300,7 +1306,7 @@ var file_planfile_proto_rawDesc = []byte{ 0x45, 0x10, 0x05, 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, 0xc0, 0x02, 0x0a, 0x1c, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, + 0x45, 0x10, 0x07, 0x2a, 0x86, 0x03, 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, @@ -1320,18 +1326,23 @@ var file_planfile_proto_rawDesc = []byte{ 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, 0x12, 0x17, 0x0a, 0x13, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x59, 0x5f, 0x54, 0x52, 0x49, 0x47, - 0x47, 0x45, 0x52, 0x53, 0x10, 0x09, 0x2a, 0x6c, 0x0a, 0x0d, 0x43, 0x6f, 0x6e, 0x64, 0x69, 0x74, - 0x69, 0x6f, 0x6e, 0x54, 0x79, 0x70, 0x65, 0x12, 0x0b, 0x0a, 0x07, 0x49, 0x4e, 0x56, 0x41, 0x4c, - 0x49, 0x44, 0x10, 0x00, 0x12, 0x19, 0x0a, 0x15, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, 0x45, - 0x5f, 0x50, 0x52, 0x45, 0x43, 0x4f, 0x4e, 0x44, 0x49, 0x54, 0x49, 0x4f, 0x4e, 0x10, 0x01, 0x12, - 0x1a, 0x0a, 0x16, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, 0x45, 0x5f, 0x50, 0x4f, 0x53, 0x54, - 0x43, 0x4f, 0x4e, 0x44, 0x49, 0x54, 0x49, 0x4f, 0x4e, 0x10, 0x02, 0x12, 0x17, 0x0a, 0x13, 0x4f, - 0x55, 0x54, 0x50, 0x55, 0x54, 0x5f, 0x50, 0x52, 0x45, 0x43, 0x4f, 0x4e, 0x44, 0x49, 0x54, 0x49, - 0x4f, 0x4e, 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, + 0x47, 0x45, 0x52, 0x53, 0x10, 0x09, 0x12, 0x1f, 0x0a, 0x1b, 0x52, 0x45, 0x41, 0x44, 0x5f, 0x42, + 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x43, 0x4f, 0x4e, 0x46, 0x49, 0x47, 0x5f, 0x55, 0x4e, + 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x0a, 0x12, 0x23, 0x0a, 0x1f, 0x52, 0x45, 0x41, 0x44, 0x5f, + 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x44, 0x45, 0x50, 0x45, 0x4e, 0x44, 0x45, 0x4e, + 0x43, 0x59, 0x5f, 0x50, 0x45, 0x4e, 0x44, 0x49, 0x4e, 0x47, 0x10, 0x0b, 0x2a, 0x6c, 0x0a, 0x0d, + 0x43, 0x6f, 0x6e, 0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x54, 0x79, 0x70, 0x65, 0x12, 0x0b, 0x0a, + 0x07, 0x49, 0x4e, 0x56, 0x41, 0x4c, 0x49, 0x44, 0x10, 0x00, 0x12, 0x19, 0x0a, 0x15, 0x52, 0x45, + 0x53, 0x4f, 0x55, 0x52, 0x43, 0x45, 0x5f, 0x50, 0x52, 0x45, 0x43, 0x4f, 0x4e, 0x44, 0x49, 0x54, + 0x49, 0x4f, 0x4e, 0x10, 0x01, 0x12, 0x1a, 0x0a, 0x16, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, + 0x45, 0x5f, 0x50, 0x4f, 0x53, 0x54, 0x43, 0x4f, 0x4e, 0x44, 0x49, 0x54, 0x49, 0x4f, 0x4e, 0x10, + 0x02, 0x12, 0x17, 0x0a, 0x13, 0x4f, 0x55, 0x54, 0x50, 0x55, 0x54, 0x5f, 0x50, 0x52, 0x45, 0x43, + 0x4f, 0x4e, 0x44, 0x49, 0x54, 0x49, 0x4f, 0x4e, 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, } var ( diff --git a/internal/plans/internal/planproto/planfile.proto b/internal/plans/internal/planproto/planfile.proto index fec25c5ca6e5..f12dab4fd47c 100644 --- a/internal/plans/internal/planproto/planfile.proto +++ b/internal/plans/internal/planproto/planfile.proto @@ -147,6 +147,8 @@ enum ResourceInstanceActionReason { DELETE_BECAUSE_EACH_KEY = 7; DELETE_BECAUSE_NO_MODULE = 8; REPLACE_BY_TRIGGERS = 9; + READ_BECAUSE_CONFIG_UNKNOWN = 10; + READ_BECAUSE_DEPENDENCY_PENDING = 11; } message ResourceInstanceChange { diff --git a/internal/plans/planfile/tfplan.go b/internal/plans/planfile/tfplan.go index 882d5fa8f62f..add4ffa7cc99 100644 --- a/internal/plans/planfile/tfplan.go +++ b/internal/plans/planfile/tfplan.go @@ -278,6 +278,10 @@ func resourceChangeFromTfplan(rawChange *planproto.ResourceInstanceChange) (*pla ret.ActionReason = plans.ResourceInstanceDeleteBecauseEachKey case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_MODULE: ret.ActionReason = plans.ResourceInstanceDeleteBecauseNoModule + case planproto.ResourceInstanceActionReason_READ_BECAUSE_CONFIG_UNKNOWN: + ret.ActionReason = plans.ResourceInstanceReadBecauseConfigUnknown + case planproto.ResourceInstanceActionReason_READ_BECAUSE_DEPENDENCY_PENDING: + ret.ActionReason = plans.ResourceInstanceReadBecauseDependencyPending default: return nil, fmt.Errorf("resource has invalid action reason %s", rawChange.ActionReason) } @@ -625,6 +629,10 @@ func resourceChangeToTfplan(change *plans.ResourceInstanceChangeSrc) (*planproto ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_EACH_KEY case plans.ResourceInstanceDeleteBecauseNoModule: ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_MODULE + case plans.ResourceInstanceReadBecauseConfigUnknown: + ret.ActionReason = planproto.ResourceInstanceActionReason_READ_BECAUSE_CONFIG_UNKNOWN + case plans.ResourceInstanceReadBecauseDependencyPending: + ret.ActionReason = planproto.ResourceInstanceActionReason_READ_BECAUSE_DEPENDENCY_PENDING 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 d278977bef27..fbc2abe0c9db 100644 --- a/internal/plans/resourceinstancechangeactionreason_string.go +++ b/internal/plans/resourceinstancechangeactionreason_string.go @@ -18,38 +18,46 @@ func _() { _ = x[ResourceInstanceDeleteBecauseCountIndex-67] _ = x[ResourceInstanceDeleteBecauseEachKey-69] _ = x[ResourceInstanceDeleteBecauseNoModule-77] + _ = x[ResourceInstanceReadBecauseConfigUnknown-63] + _ = x[ResourceInstanceReadBecauseDependencyPending-33] } const ( _ResourceInstanceChangeActionReason_name_0 = "ResourceInstanceChangeNoReason" - _ResourceInstanceChangeActionReason_name_1 = "ResourceInstanceDeleteBecauseCountIndexResourceInstanceReplaceByTriggersResourceInstanceDeleteBecauseEachKeyResourceInstanceReplaceBecauseCannotUpdate" - _ResourceInstanceChangeActionReason_name_2 = "ResourceInstanceDeleteBecauseNoModuleResourceInstanceDeleteBecauseNoResourceConfig" - _ResourceInstanceChangeActionReason_name_3 = "ResourceInstanceReplaceByRequest" - _ResourceInstanceChangeActionReason_name_4 = "ResourceInstanceReplaceBecauseTainted" - _ResourceInstanceChangeActionReason_name_5 = "ResourceInstanceDeleteBecauseWrongRepetition" + _ResourceInstanceChangeActionReason_name_1 = "ResourceInstanceReadBecauseDependencyPending" + _ResourceInstanceChangeActionReason_name_2 = "ResourceInstanceReadBecauseConfigUnknown" + _ResourceInstanceChangeActionReason_name_3 = "ResourceInstanceDeleteBecauseCountIndexResourceInstanceReplaceByTriggersResourceInstanceDeleteBecauseEachKeyResourceInstanceReplaceBecauseCannotUpdate" + _ResourceInstanceChangeActionReason_name_4 = "ResourceInstanceDeleteBecauseNoModuleResourceInstanceDeleteBecauseNoResourceConfig" + _ResourceInstanceChangeActionReason_name_5 = "ResourceInstanceReplaceByRequest" + _ResourceInstanceChangeActionReason_name_6 = "ResourceInstanceReplaceBecauseTainted" + _ResourceInstanceChangeActionReason_name_7 = "ResourceInstanceDeleteBecauseWrongRepetition" ) var ( - _ResourceInstanceChangeActionReason_index_1 = [...]uint8{0, 39, 72, 108, 150} - _ResourceInstanceChangeActionReason_index_2 = [...]uint8{0, 37, 82} + _ResourceInstanceChangeActionReason_index_3 = [...]uint8{0, 39, 72, 108, 150} + _ResourceInstanceChangeActionReason_index_4 = [...]uint8{0, 37, 82} ) func (i ResourceInstanceChangeActionReason) String() string { switch { case i == 0: return _ResourceInstanceChangeActionReason_name_0 + case i == 33: + return _ResourceInstanceChangeActionReason_name_1 + case i == 63: + return _ResourceInstanceChangeActionReason_name_2 case 67 <= i && i <= 70: i -= 67 - return _ResourceInstanceChangeActionReason_name_1[_ResourceInstanceChangeActionReason_index_1[i]:_ResourceInstanceChangeActionReason_index_1[i+1]] + return _ResourceInstanceChangeActionReason_name_3[_ResourceInstanceChangeActionReason_index_3[i]:_ResourceInstanceChangeActionReason_index_3[i+1]] case 77 <= i && i <= 78: i -= 77 - return _ResourceInstanceChangeActionReason_name_2[_ResourceInstanceChangeActionReason_index_2[i]:_ResourceInstanceChangeActionReason_index_2[i+1]] + return _ResourceInstanceChangeActionReason_name_4[_ResourceInstanceChangeActionReason_index_4[i]:_ResourceInstanceChangeActionReason_index_4[i+1]] case i == 82: - return _ResourceInstanceChangeActionReason_name_3 + return _ResourceInstanceChangeActionReason_name_5 case i == 84: - return _ResourceInstanceChangeActionReason_name_4 + return _ResourceInstanceChangeActionReason_name_6 case i == 87: - return _ResourceInstanceChangeActionReason_name_5 + return _ResourceInstanceChangeActionReason_name_7 default: return "ResourceInstanceChangeActionReason(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index 9cf7e875ebe1..3cacc8f0a0e9 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -1786,6 +1786,9 @@ func TestContext2Plan_computedDataResource(t *testing.T) { }), rc.After, ) + if got, want := rc.ActionReason, plans.ResourceInstanceReadBecauseConfigUnknown; got != want { + t.Errorf("wrong ActionReason\ngot: %s\nwant: %s", got, want) + } } func TestContext2Plan_computedInFunction(t *testing.T) { @@ -1987,6 +1990,10 @@ func TestContext2Plan_dataResourceBecomesComputed(t *testing.T) { t.Fatal(err) } + if got, want := rc.ActionReason, plans.ResourceInstanceReadBecauseConfigUnknown; got != want { + t.Errorf("wrong ActionReason\ngot: %s\nwant: %s", got, want) + } + // foo should now be unknown foo := rc.After.GetAttr("foo") if foo.IsKnown() { @@ -6295,8 +6302,21 @@ data "test_data_source" "e" { }, }) - _, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) assertNoErrors(t, diags) + + rc := plan.Changes.ResourceInstance(addrs.Resource{ + Mode: addrs.DataResourceMode, + Type: "test_data_source", + Name: "d", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance)) + if rc != nil { + if got, want := rc.ActionReason, plans.ResourceInstanceReadBecauseDependencyPending; got != want { + t.Errorf("wrong ActionReason\ngot: %s\nwant: %s", got, want) + } + } else { + t.Error("no change for test_data_source.e") + } } func TestContext2Plan_skipRefresh(t *testing.T) { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 6a75274bb1c8..675934935b66 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -1525,15 +1525,23 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule unmarkedConfigVal, configMarkPaths := configVal.UnmarkDeepWithPaths() configKnown := configVal.IsWhollyKnown() + depsPending := n.dependenciesHavePendingChanges(ctx) // If our configuration contains any unknown values, or we depend on any // unknown values then we must defer the read to the apply phase by // producing a "Read" change for this resource, and a placeholder value for // it in the state. - if n.forcePlanReadData(ctx) || !configKnown { - if configKnown { - log.Printf("[TRACE] planDataSource: %s configuration is fully known, but we're forcing a read plan to be created", n.Addr) - } else { + if depsPending || !configKnown { + var reason plans.ResourceInstanceChangeActionReason + switch { + case !configKnown: log.Printf("[TRACE] planDataSource: %s configuration not fully known yet, so deferring to apply phase", n.Addr) + reason = plans.ResourceInstanceReadBecauseConfigUnknown + case depsPending: + // NOTE: depsPending can be true at the same time as configKnown + // is false; configKnown takes precedence because it's more + // specific. + log.Printf("[TRACE] planDataSource: %s configuration is fully known, at least one dependency has changes pending", n.Addr) + reason = plans.ResourceInstanceReadBecauseDependencyPending } proposedNewVal := objchange.PlannedDataResourceObject(schema, unmarkedConfigVal) @@ -1550,6 +1558,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule Before: priorVal, After: proposedNewVal, }, + ActionReason: reason, } plannedNewState := &states.ResourceInstanceObject{ @@ -1580,10 +1589,11 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule return nil, plannedNewState, keyData, diags } -// forcePlanReadData determines if we need to override the usual behavior of -// immediately reading from the data source where possible, instead forcing us -// to generate a plan. -func (n *NodeAbstractResourceInstance) forcePlanReadData(ctx EvalContext) bool { +// dependenciesHavePendingChanges determines whether any managed resource the +// receiver depends on has a change pending in the plan, in which case we'd +// need to override the usual behavior of immediately reading from the data +// source where possible, and instead defer the read until the apply step. +func (n *NodeAbstractResourceInstance) dependenciesHavePendingChanges(ctx EvalContext) bool { nModInst := n.Addr.Module nMod := nModInst.Module() diff --git a/website/docs/internals/json-format.mdx b/website/docs/internals/json-format.mdx index 0476ae274909..ec4c109c53b8 100644 --- a/website/docs/internals/json-format.mdx +++ b/website/docs/internals/json-format.mdx @@ -173,6 +173,13 @@ For ease of consumption by callers, the plan representation includes a partial r // - "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. + // - "read_because_config_unknown": For a data resource, Terraform cannot + // read the data during the plan phase because of values in the + // configuration that won't be known until the apply phase. + // - "read_because_dependency_pending": For a data resource, Terraform + // cannot read the data during the plan phase because the data + // resource depends on at least one managed resource that also has + // a pending change in the same plan. // // If there is no special reason to note, Terraform will omit this // property altogether.