Merge pull request #21202 from hashicorp/jbardin/data-plan

core:  always re-read datasource when the dependencies have changes

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"),
+				),
+			},
+		},
+	})
+}

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 {

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 {
+					depChanges := false
-						return true, EvalEarlyExitError{}
+
+					// 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
 				},