From f9dfa98533c51e766b0cc60fe0c94c882f2d9516 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 3 May 2019 16:23:32 -0400 Subject: [PATCH 1/3] test to make sure a data source is planned The data source here needs to be re-read during apply, because its config is changing. --- builtin/providers/test/data_source_test.go | 50 ++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/builtin/providers/test/data_source_test.go b/builtin/providers/test/data_source_test.go index 6e809cabc..c0a1ae57c 100644 --- a/builtin/providers/test/data_source_test.go +++ b/builtin/providers/test/data_source_test.go @@ -239,3 +239,53 @@ data "test_data_source" "two" { }, }) } + +func TestDataSource_planUpdate(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: strings.TrimSpace(` +resource "test_resource" "a" { + required = "first" + required_map = { + key = "1" + } + optional_force_new = "first" +} + +data "test_data_source" "a" { + input = "${test_resource.a.computed_from_required}" +} + +output "out" { + value = "${data.test_data_source.a.output}" +} + `), + }, + { + Config: strings.TrimSpace(` +resource "test_resource" "a" { + required = "second" + required_map = { + key = "1" + } + optional_force_new = "second" +} + +data "test_data_source" "a" { + input = "${test_resource.a.computed_from_required}" +} + +output "out" { + value = "${data.test_data_source.a.output}" +} + `), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("data.test_data_source.a", "output", "second"), + resource.TestCheckOutput("out", "second"), + ), + }, + }, + }) +} From 35e087fe5b54b869f5dbbafe8bd0282e16db76c9 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 3 May 2019 16:40:02 -0400 Subject: [PATCH 2/3] always re-read datasource if there are dep changes If a datasource's dependencies have planned changes, then we need to plan a read for the data source, because the config may change once the dependencies have been applied. --- terraform/node_resource_plan_instance.go | 27 ++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index d69472f82..75e0bcd34 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -81,8 +81,31 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou // here. &EvalIf{ If: func(ctx EvalContext) (bool, error) { - if state != nil && state.Status != states.ObjectPlanned { - return true, EvalEarlyExitError{} + depChanges := false + + // Check and see if any of our dependencies have changes. + changes := ctx.Changes() + for _, d := range n.StateReferences() { + ri, ok := d.(addrs.ResourceInstance) + if !ok { + continue + } + change := changes.GetResourceInstanceChange(ri.Absolute(ctx.Path()), states.CurrentGen) + if change != nil && change.Action != plans.NoOp { + depChanges = true + break + } + } + + refreshed := state != nil && state.Status != states.ObjectPlanned + + // If there are no dependency changes, and it's not a forced + // read because we there was no Refresh, then we don't need + // to re-read. If any dependencies have changes, it means + // our config may also have changes and we need to Read the + // data source again. + if !depChanges && refreshed { + return false, EvalEarlyExitError{} } return true, nil }, From 06babb6cc3b30818ba8ec30f00707a1a1b7bdf82 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 3 May 2019 16:46:19 -0400 Subject: [PATCH 3/3] remove dead code for the next reader --- terraform/eval_read_data.go | 119 ------------------------------------ 1 file changed, 119 deletions(-) diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index d89d02ba9..34f2d60ad 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -291,125 +291,6 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.ErrWithWarnings() } -// EvalReadDataDiff is an EvalNode implementation that executes a data -// resource's ReadDataDiff method to discover what attributes it exports. -type EvalReadDataDiff struct { - Addr addrs.ResourceInstance - Config *configs.Resource - ProviderAddr addrs.AbsProviderConfig - ProviderSchema **ProviderSchema - - Output **plans.ResourceInstanceChange - OutputValue *cty.Value - OutputConfigValue *cty.Value - OutputState **states.ResourceInstanceObject - - // Set Previous when re-evaluating diff during apply, to ensure that - // the "Destroy" flag is preserved. - Previous **plans.ResourceInstanceChange -} - -func (n *EvalReadDataDiff) Eval(ctx EvalContext) (interface{}, error) { - absAddr := n.Addr.Absolute(ctx.Path()) - - if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return nil, fmt.Errorf("provider schema not available for %s", n.Addr) - } - - var diags tfdiags.Diagnostics - var change *plans.ResourceInstanceChange - var configVal cty.Value - - if n.Previous != nil && *n.Previous != nil && (*n.Previous).Action == plans.Delete { - // If we're re-diffing for a diff that was already planning to - // destroy, then we'll just continue with that plan. - - nullVal := cty.NullVal(cty.DynamicPseudoType) - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreDiff(absAddr, states.CurrentGen, nullVal, nullVal) - }) - if err != nil { - return nil, err - } - - change = &plans.ResourceInstanceChange{ - Addr: absAddr, - ProviderAddr: n.ProviderAddr, - Change: plans.Change{ - Action: plans.Delete, - Before: nullVal, - After: nullVal, - }, - } - } else { - config := *n.Config - providerSchema := *n.ProviderSchema - schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) - if schema == nil { - // Should be caught during validation, so we don't bother with a pretty error here - return nil, fmt.Errorf("provider does not support data source %q", n.Addr.Resource.Type) - } - - objTy := schema.ImpliedType() - priorVal := cty.NullVal(objTy) // for data resources, prior is always null because we start fresh every time - - keyData := EvalDataForInstanceKey(n.Addr.Key) - - var configDiags tfdiags.Diagnostics - configVal, _, configDiags = ctx.EvaluateBlock(config.Config, schema, nil, keyData) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { - return nil, diags.Err() - } - - proposedNewVal := objchange.ProposedNewObject(schema, priorVal, configVal) - - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) - }) - if err != nil { - return nil, err - } - - change = &plans.ResourceInstanceChange{ - Addr: absAddr, - ProviderAddr: n.ProviderAddr, - Change: plans.Change{ - Action: plans.Read, - Before: priorVal, - After: proposedNewVal, - }, - } - } - - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostDiff(absAddr, states.CurrentGen, change.Action, change.Before, change.After) - }) - if err != nil { - return nil, err - } - - if n.Output != nil { - *n.Output = change - } - if n.OutputValue != nil { - *n.OutputValue = change.After - } - if n.OutputConfigValue != nil { - *n.OutputConfigValue = configVal - } - - if n.OutputState != nil { - state := &states.ResourceInstanceObject{ - Value: change.After, - Status: states.ObjectReady, - } - *n.OutputState = state - } - - return nil, diags.ErrWithWarnings() -} - // EvalReadDataApply is an EvalNode implementation that executes a data // resource's ReadDataApply method to read data from the data source. type EvalReadDataApply struct {