Skip to content

Commit

Permalink
core: Report reason for deferring data read until apply
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
apparentlymart committed May 9, 2022
1 parent 98f9d64 commit 4cffff2
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 46 deletions.
10 changes: 8 additions & 2 deletions internal/command/format/diff.go
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
))
Expand Down
64 changes: 64 additions & 0 deletions internal/command/format/diff_test.go
Expand Up @@ -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": {
Expand Down
4 changes: 4 additions & 0 deletions internal/command/jsonplan/plan.go
Expand Up @@ -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)
}
Expand Down
6 changes: 6 additions & 0 deletions internal/command/views/json/change.go
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions internal/plans/changes.go
Expand Up @@ -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.
Expand Down
57 changes: 34 additions & 23 deletions internal/plans/internal/planproto/planfile.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions internal/plans/internal/planproto/planfile.proto
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions internal/plans/planfile/tfplan.go
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
32 changes: 20 additions & 12 deletions internal/plans/resourceinstancechangeactionreason_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 21 additions & 1 deletion internal/terraform/context_plan_test.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 4cffff2

Please sign in to comment.