From 291110fe785bfbaf1c5f022228f6bfb262de2546 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 13 May 2020 13:49:05 -0400
Subject: [PATCH] Don't use plans.Update for data sources

The new data source planning logic no longer needs a separate action,
and the apply status can be determined from whether the After value is
complete or not.
---
 terraform/context_plan_test.go    | 8 ++++----
 terraform/eval_read_data_apply.go | 8 ++++----
 terraform/eval_read_data_plan.go  | 9 +++++++--
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go
index d83ab465b..b7eb83619 100644
--- a/terraform/context_plan_test.go
+++ b/terraform/context_plan_test.go
@@ -5053,16 +5053,16 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) {
 					"computed": cty.StringVal("data_id"),
 				}), ric.After)
 			case "data.aws_vpc.bar[0]":
-				if res.Action != plans.Update {
-					t.Fatalf("resource %s should be update, got %s", ric.Addr, ric.Action)
+				if res.Action != plans.Read {
+					t.Fatalf("resource %s should be read, got %s", ric.Addr, ric.Action)
 				}
 				checkVals(t, objectVal(t, schema, map[string]cty.Value{
 					"id":  cty.StringVal("data_id"),
 					"foo": cty.StringVal("0"),
 				}), ric.After)
 			case "data.aws_vpc.bar[1]":
-				if res.Action != plans.Update {
-					t.Fatalf("resource %s should be update, got %s", ric.Addr, ric.Action)
+				if res.Action != plans.Read {
+					t.Fatalf("resource %s should be read, got %s", ric.Addr, ric.Action)
 				}
 				checkVals(t, objectVal(t, schema, map[string]cty.Value{
 					"id":  cty.StringVal("data_id"),
diff --git a/terraform/eval_read_data_apply.go b/terraform/eval_read_data_apply.go
index ecdd440e6..888d1618d 100644
--- a/terraform/eval_read_data_apply.go
+++ b/terraform/eval_read_data_apply.go
@@ -29,11 +29,11 @@ func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, fmt.Errorf("provider schema not available for %s", n.Addr)
 	}
 
-	if planned != nil && !(planned.Action == plans.Read || planned.Action == plans.Update) {
+	if planned != nil && planned.Action != plans.Read {
 		// If any other action gets in here then that's always a bug; this
 		// EvalNode only deals with reading.
 		return nil, fmt.Errorf(
-			"invalid action %s for %s: only Read or Update is supported (this is a bug in Terraform; please report it!)",
+			"invalid action %s for %s: only Read is supported (this is a bug in Terraform; please report it!)",
 			planned.Action, absAddr,
 		)
 	}
@@ -44,9 +44,9 @@ func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, err
 	}
 
-	// we have a change and it is complete, which means we read the data
+	// We have a change and it is complete, which means we read the data
 	// source during plan and only need to store it in state.
-	if planned.Action == plans.Update {
+	if planned.After.IsWhollyKnown() {
 		if err := ctx.Hook(func(h Hook) (HookAction, error) {
 			return h.PostApply(absAddr, states.CurrentGen, planned.After, nil)
 		}); err != nil {
diff --git a/terraform/eval_read_data_plan.go b/terraform/eval_read_data_plan.go
index 0f86b9f4d..0a6d4c431 100644
--- a/terraform/eval_read_data_plan.go
+++ b/terraform/eval_read_data_plan.go
@@ -81,6 +81,8 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) {
 			return nil, diags.ErrWithWarnings()
 		}
 
+		// Apply detects that the data source will need to be read by the After
+		// value containing unknowns from PlanDataResourceObject.
 		*n.OutputChange = &plans.ResourceInstanceChange{
 			Addr:         absAddr,
 			ProviderAddr: n.ProviderAddr,
@@ -124,12 +126,15 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, diags.ErrWithWarnings()
 	}
 
-	// Produce a change regardless of the outcome.
+	// The returned value from ReadDataSource must be non-nil and known,
+	// which we store in the change. Apply will use the fact that the After
+	// value is wholly kown to save the state directly, rather than reading the
+	// data source again.
 	*n.OutputChange = &plans.ResourceInstanceChange{
 		Addr:         absAddr,
 		ProviderAddr: n.ProviderAddr,
 		Change: plans.Change{
-			Action: plans.Update,
+			Action: plans.Read,
 			Before: priorVal,
 			After:  newVal,
 		},