diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go index 1f59e6f41..15565a400 100644 --- a/plans/objchange/objchange.go +++ b/plans/objchange/objchange.go @@ -32,6 +32,32 @@ func ProposedNewObject(schema *configschema.Block, prior, config cty.Value) cty. // below by giving us one non-null level of object to pull values from. prior = AllAttributesNull(schema) } + return proposedNewObject(schema, prior, config) +} + +// PlannedDataResourceObject is similar to ProposedNewObject but tailored for +// planning data resources in particular. Specifically, it replaces the values +// of any Computed attributes not set in the configuration with an unknown +// value, which serves as a placeholder for a value to be filled in by the +// provider when the data resource is finally read. +// +// Data resources are different because the planning of them is handled +// entirely within Terraform Core and not subject to customization by the +// provider. This function is, in effect, producing an equivalent result to +// passing the ProposedNewObject result into a provider's PlanResourceChange +// function, assuming a fixed implementation of PlanResourceChange that just +// fills in unknown values as needed. +func PlannedDataResourceObject(schema *configschema.Block, config cty.Value) cty.Value { + // Our trick here is to run the ProposedNewObject logic with an + // entirely-unknown prior value. Because of cty's unknown short-circuit + // behavior, any operation on prior returns another unknown, and so + // unknown values propagate into all of the parts of the resulting value + // that would normally be filled in by preserving the prior state. + prior := cty.UnknownVal(schema.ImpliedType()) + return proposedNewObject(schema, prior, config) +} + +func proposedNewObject(schema *configschema.Block, prior, config cty.Value) cty.Value { if config.IsNull() || !config.IsKnown() { // This is a weird situation, but we'll allow it anyway to free // callers from needing to specifically check for these cases. diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index f95f59068..a58035233 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -4990,8 +4990,20 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { }, } p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + computedVal := req.ProposedNewState.GetAttr("computed") + if computedVal.IsNull() { + computedVal = cty.UnknownVal(cty.String) + } return providers.PlanResourceChangeResponse{ - PlannedState: req.ProposedNewState, + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "num": req.ProposedNewState.GetAttr("num"), + "computed": computedVal, + }), + } + } + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("ReadDataSource called, but should not have been")), } } @@ -5004,11 +5016,20 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { ), }) + // We're skipping ctx.Refresh here, which simulates what happens when + // running "terraform plan -refresh=false". As a result, we don't get our + // usual opportunity to read the data source during the refresh step and + // thus the plan call below is forced to produce a deferred read action. + plan, diags := ctx.Plan() + if p.ReadDataSourceCalled { + t.Errorf("ReadDataSource was called on the provider, but should not have been because we didn't refresh") + } if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } + seenAddrs := make(map[string]struct{}) for _, res := range plan.Changes.Resources { var schema *configschema.Block switch res.Addr.Resource.Resource.Mode { @@ -5023,6 +5044,8 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { t.Fatal(err) } + seenAddrs[ric.Addr.String()] = struct{}{} + t.Run(ric.Addr.String(), func(t *testing.T) { switch i := ric.Addr.String(); i { case "aws_instance.foo[0]": @@ -5046,6 +5069,10 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { t.Fatalf("resource %s should be read, got %s", ric.Addr, ric.Action) } checkVals(t, objectVal(t, schema, map[string]cty.Value{ + // In a normal flow we would've read an exact value in + // ReadDataSource, but because this test doesn't run + // cty.Refresh we have no opportunity to do that lookup + // and a deferred read is forced. "id": cty.UnknownVal(cty.String), "foo": cty.StringVal("0"), }), ric.After) @@ -5054,6 +5081,10 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { t.Fatalf("resource %s should be read, got %s", ric.Addr, ric.Action) } checkVals(t, objectVal(t, schema, map[string]cty.Value{ + // In a normal flow we would've read an exact value in + // ReadDataSource, but because this test doesn't run + // cty.Refresh we have no opportunity to do that lookup + // and a deferred read is forced. "id": cty.UnknownVal(cty.String), "foo": cty.StringVal("1"), }), ric.After) @@ -5062,6 +5093,16 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { } }) } + + wantAddrs := map[string]struct{}{ + "aws_instance.foo[0]": struct{}{}, + "aws_instance.foo[1]": struct{}{}, + "data.aws_vpc.bar[0]": struct{}{}, + "data.aws_vpc.bar[1]": struct{}{}, + } + if !cmp.Equal(seenAddrs, wantAddrs) { + t.Errorf("incorrect addresses in changeset:\n%s", cmp.Diff(wantAddrs, seenAddrs)) + } } // interpolated lists need to be stored in the original order. diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 54d59eea4..d89d02ba9 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -104,7 +104,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } - proposedNewVal := objchange.ProposedNewObject(schema, priorVal, configVal) + proposedNewVal := objchange.PlannedDataResourceObject(schema, configVal) // If our configuration contains any unknown values then we must defer the // read to the apply phase by producing a "Read" change for this resource, @@ -119,7 +119,11 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { absAddr, ) } - log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr) + if n.ForcePlanRead { + log.Printf("[TRACE] EvalReadData: %s configuration is fully known, but we're forcing a read plan to be created", absAddr) + } else { + log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr) + } err := ctx.Hook(func(h Hook) (HookAction, error) { return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal)