Merge pull request #26317 from hashicorp/jbardin/remove-refresh-walk

Replace internal Refresh command with Plan

backend/local/backend_refresh_test.go

@@ -51,76 +51,11 @@ test_instance.foo:
 	assertBackendStateUnlocked(t, b)
 }
 
-func TestLocal_refreshNoConfig(t *testing.T) {
-	b, cleanup := TestLocal(t)
-	defer cleanup()
-	p := TestLocalProvider(t, b, "test", refreshFixtureSchema())
-	testStateFile(t, b.StatePath, testRefreshState())
-	p.ReadResourceFn = nil
-	p.ReadResourceResponse = providers.ReadResourceResponse{NewState: cty.ObjectVal(map[string]cty.Value{
-		"id": cty.StringVal("yes"),
-	})}
-
-	op, configCleanup := testOperationRefresh(t, "./testdata/empty")
-	defer configCleanup()
-
-	run, err := b.Operation(context.Background(), op)
-	if err != nil {
-		t.Fatalf("bad: %s", err)
-	}
-	<-run.Done()
-
-	if !p.ReadResourceCalled {
-		t.Fatal("ReadResource should be called")
-	}
-
-	checkState(t, b.StateOutPath, `
-test_instance.foo:
-  ID = yes
-  provider = provider["registry.terraform.io/hashicorp/test"]
-	`)
-}
-
-// GH-12174
-func TestLocal_refreshNilModuleWithInput(t *testing.T) {
-	b, cleanup := TestLocal(t)
-	defer cleanup()
-	p := TestLocalProvider(t, b, "test", refreshFixtureSchema())
-	testStateFile(t, b.StatePath, testRefreshState())
-	p.ReadResourceFn = nil
-	p.ReadResourceResponse = providers.ReadResourceResponse{NewState: cty.ObjectVal(map[string]cty.Value{
-		"id": cty.StringVal("yes"),
-	})}
-
-	b.OpInput = true
-
-	op, configCleanup := testOperationRefresh(t, "./testdata/empty")
-	defer configCleanup()
-
-	run, err := b.Operation(context.Background(), op)
-	if err != nil {
-		t.Fatalf("bad: %s", err)
-	}
-	<-run.Done()
-
-	if !p.ReadResourceCalled {
-		t.Fatal("ReadResource should be called")
-	}
-
-	checkState(t, b.StateOutPath, `
-test_instance.foo:
-  ID = yes
-  provider = provider["registry.terraform.io/hashicorp/test"]
-	`)
-}
-
 func TestLocal_refreshInput(t *testing.T) {
 	b, cleanup := TestLocal(t)
 	defer cleanup()
-	p := TestLocalProvider(t, b, "test", refreshFixtureSchema())
-	testStateFile(t, b.StatePath, testRefreshState())
 
-	p.GetSchemaReturn = &terraform.ProviderSchema{
+	schema := &terraform.ProviderSchema{
 		Provider: &configschema.Block{
 			Attributes: map[string]*configschema.Attribute{
 				"value": {Type: cty.String, Optional: true},
@@ -129,12 +64,17 @@ func TestLocal_refreshInput(t *testing.T) {
 		ResourceTypes: map[string]*configschema.Block{
 			"test_instance": {
 				Attributes: map[string]*configschema.Attribute{
+					"id":  {Type: cty.String, Computed: true},
 					"foo": {Type: cty.String, Optional: true},
-					"id":  {Type: cty.String, Optional: true},
+					"ami": {Type: cty.String, Optional: true},
 				},
 			},
 		},
 	}
+
+	p := TestLocalProvider(t, b, "test", schema)
+	testStateFile(t, b.StatePath, testRefreshState())
+
 	p.ReadResourceFn = nil
 	p.ReadResourceResponse = providers.ReadResourceResponse{NewState: cty.ObjectVal(map[string]cty.Value{
 		"id": cty.StringVal("yes"),

backend/local/testdata/refresh-var-unset/main.tf

@@ -1,7 +1,7 @@
 variable "should_ask" {}
 
 provider "test" {
-  value = "${var.should_ask}"
+  value = var.should_ask
 }
 
 resource "test_instance" "foo" {

command/command_test.go

@@ -466,9 +466,11 @@ func testStateOutput(t *testing.T, path string, expected string) {
 
 func testProvider() *terraform.MockProvider {
 	p := new(terraform.MockProvider)
-	p.PlanResourceChangeResponse = providers.PlanResourceChangeResponse{
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
-		PlannedState: cty.EmptyObjectVal,
+		resp.PlannedState = req.ProposedNewState
+		return resp
 	}
+
 	p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse {
 		return providers.ReadResourceResponse{
 			NewState: req.PriorState,

command/testdata/show-json/basic-create/output.json

@@ -53,7 +53,18 @@
             ]
         }
     },
-    "prior_state": {},
+    "prior_state": {
+        "format_version": "0.1",
+        "values": {
+            "outputs": {
+                "test": {
+                    "sensitive": false,
+                    "value": "bar"
+                }
+            },
+            "root_module": {}
+        }
+    },
     "resource_changes": [
         {
             "address": "test_instance.test[0]",

command/testdata/show-json/basic-delete/terraform.tfstate

@@ -3,12 +3,7 @@
   "terraform_version": "0.12.0",
   "serial": 7,
   "lineage": "configuredUnchanged",
-  "outputs": {
+  "outputs": {},
-    "test": {
-      "value": "bar",
-      "type": "string"
-    }
-  },
   "resources": [
     {
       "mode": "managed",

command/testdata/show-json/basic-update/terraform.tfstate

@@ -3,12 +3,7 @@
     "terraform_version": "0.12.0",
     "serial": 7,
     "lineage": "configuredUnchanged",
-    "outputs": {
+    "outputs": {},
-        "test": {
-            "value": "bar",
-            "type": "string"
-        }
-    },
     "resources": [
         {
             "mode": "managed",

command/testdata/show-json/modules/output.json

@@ -69,7 +69,18 @@
             ]
         }
     },
-    "prior_state": {},
+    "prior_state": {
+        "format_version": "0.1",
+        "values": {
+            "outputs": {
+                "test": {
+                    "sensitive": false,
+                    "value": "baz"
+                }
+            },
+            "root_module": {}
+        }
+    },
     "resource_changes": [
         {
             "address": "module.module_test_bar.test_instance.test",

command/testdata/show-json/multi-resource-update/output.json

@@ -101,14 +101,20 @@
         "format_version": "0.1",
         "terraform_version": "0.13.0",
         "values": {
+            "outputs": {
+                "test": {
+                    "sensitive": false,
+                    "value": "bar"
+                }
+            },
             "root_module": {
                 "resources": [
                     {
                         "address": "test_instance.test[0]",
-                        "index": 0,
                         "mode": "managed",
                         "type": "test_instance",
                         "name": "test",
+                        "index": 0,
                         "provider_name": "registry.terraform.io/hashicorp/test",
                         "schema_version": 0,
                         "values": {

terraform/context.go

@@ -567,7 +567,8 @@ The -target option is not for routine use, and is provided only for exceptional
 	}
 	p.Changes = c.changes
 
-	p.State = c.refreshState
+	c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects()
+	p.State = c.refreshState.DeepCopy()
 
 	// replace the working state with the updated state, so that immediate calls
 	// to Apply work as expected.
@@ -577,51 +578,17 @@ The -target option is not for routine use, and is provided only for exceptional
 }
 
 // Refresh goes through all the resources in the state and refreshes them
-// to their latest state. This will update the state that this context
+// to their latest state. This is done by executing a plan, and retaining the
-// works with, along with returning it.
+// state while discarding the change set.
 //
-// Even in the case an error is returned, the state may be returned and
+// In the case of an error, there is no state returned.
-// will potentially be partially updated.
 func (c *Context) Refresh() (*states.State, tfdiags.Diagnostics) {
-	defer c.acquireRun("refresh")()
+	p, diags := c.Plan()
-
-	// Copy our own state
-	c.state = c.state.DeepCopy()
-
-	// Refresh builds a partial changeset as part of its work because it must
-	// create placeholder stubs for any resource instances that'll be created
-	// in subsequent plan so that provider configurations and data resources
-	// can interpolate from them. This plan is always thrown away after
-	// the operation completes, restoring any existing changeset.
-	oldChanges := c.changes
-	defer func() { c.changes = oldChanges }()
-	c.changes = plans.NewChanges()
-
-	// Build the graph.
-	graph, diags := c.Graph(GraphTypeRefresh, nil)
 	if diags.HasErrors() {
 		return nil, diags
 	}
 
-	// Do the walk
+	return p.State, diags
-	_, walkDiags := c.walk(graph, walkRefresh)
-	diags = diags.Append(walkDiags)
-	if walkDiags.HasErrors() {
-		return nil, diags
-	}
-
-	// During our walk we will have created planned object placeholders in
-	// state for resource instances that are in configuration but not yet
-	// created. These were created only to allow expression evaluation to
-	// work properly in provider and data blocks during the walk and must
-	// now be discarded, since a subsequent plan walk is responsible for
-	// creating these "for real".
-	// TODO: Consolidate refresh and plan into a single walk, so that the
-	// refresh walk doesn't need to emulate various aspects of the plan
-	// walk in order to properly evaluate provider and data blocks.
-	c.state.SyncWrapper().RemovePlannedResourceInstanceObjects()
-
-	return c.state, diags
 }
 
 // Stop stops the running task.

terraform/context_apply_test.go

@@ -3987,77 +3987,6 @@ func TestContext2Apply_nilDiff(t *testing.T) {
 	}
 }
 
-func TestContext2Apply_outputDependsOn(t *testing.T) {
-	m := testModule(t, "apply-output-depends-on")
-	p := testProvider("aws")
-	p.DiffFn = testDiffFn
-
-	{
-		// Create a custom apply function that sleeps a bit (to allow parallel
-		// graph execution) and then returns an error to force a partial state
-		// return. We then verify the output is NOT there.
-		p.ApplyFn = func(
-			info *InstanceInfo,
-			is *InstanceState,
-			id *InstanceDiff) (*InstanceState, error) {
-			// Sleep to allow parallel execution
-			time.Sleep(50 * time.Millisecond)
-
-			// Return error to force partial state
-			return nil, fmt.Errorf("abcd")
-		}
-
-		ctx := testContext2(t, &ContextOpts{
-			Config: m,
-			Providers: map[addrs.Provider]providers.Factory{
-				addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
-			},
-		})
-
-		if _, diags := ctx.Plan(); diags.HasErrors() {
-			t.Fatalf("diags: %s", diags.Err())
-		}
-
-		state, diags := ctx.Apply()
-		if !diags.HasErrors() || !strings.Contains(diags.Err().Error(), "abcd") {
-			t.Fatalf("err: %s", diags.Err())
-		}
-
-		checkStateString(t, state, `<no state>`)
-	}
-
-	{
-		// Create the standard apply function and verify we get the output
-		p.ApplyFn = testApplyFn
-
-		ctx := testContext2(t, &ContextOpts{
-			Config: m,
-			Providers: map[addrs.Provider]providers.Factory{
-				addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
-			},
-		})
-
-		if _, diags := ctx.Plan(); diags.HasErrors() {
-			t.Fatalf("diags: %s", diags.Err())
-		}
-
-		state, diags := ctx.Apply()
-		if diags.HasErrors() {
-			t.Fatalf("diags: %s", diags.Err())
-		}
-
-		checkStateString(t, state, `
-aws_instance.foo:
-  ID = foo
-  provider = provider["registry.terraform.io/hashicorp/aws"]
-
-Outputs:
-
-value = result
-		`)
-	}
-}
-
 func TestContext2Apply_outputOrphan(t *testing.T) {
 	m := testModule(t, "apply-output-orphan")
 	p := testProvider("aws")
@@ -8604,6 +8533,16 @@ func TestContext2Apply_ignoreChangesWithDep(t *testing.T) {
 		&states.ResourceInstanceObjectSrc{
 			Status:    states.ObjectReady,
 			AttrsJSON: []byte(`{"id":"eip-abc123","instance":"i-abc123"}`),
+			Dependencies: []addrs.ConfigResource{
+				addrs.ConfigResource{
+					Resource: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "foo",
+					},
+					Module: addrs.RootModule,
+				},
+			},
 		},
 		mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
 	)
@@ -8612,6 +8551,16 @@ func TestContext2Apply_ignoreChangesWithDep(t *testing.T) {
 		&states.ResourceInstanceObjectSrc{
 			Status:    states.ObjectReady,
 			AttrsJSON: []byte(`{"id":"eip-bcd234","instance":"i-bcd234"}`),
+			Dependencies: []addrs.ConfigResource{
+				addrs.ConfigResource{
+					Resource: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "foo",
+					},
+					Module: addrs.RootModule,
+				},
+			},
 		},
 		mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
 	)
@@ -8621,7 +8570,7 @@ func TestContext2Apply_ignoreChangesWithDep(t *testing.T) {
 		Providers: map[addrs.Provider]providers.Factory{
 			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
 		},
-		State: state,
+		State: state.DeepCopy(),
 	})
 
 	_, diags := ctx.Plan()
@@ -8801,10 +8750,7 @@ resource "null_instance" "depends" {
 		}
 	}
 
-	_, diags := ctx.Refresh()
+	_, diags := ctx.Plan()
-	assertNoErrors(t, diags)
-
-	_, diags = ctx.Plan()
 	assertNoErrors(t, diags)
 
 	state, diags := ctx.Apply()
@@ -10594,45 +10540,6 @@ func TestContext2Apply_ProviderMeta_refresh_set(t *testing.T) {
 	}
 }
 
-func TestContext2Apply_ProviderMeta_refresh_unset(t *testing.T) {
-	m := testModule(t, "provider-meta-unset")
-	p := testProvider("test")
-	p.ApplyFn = testApplyFn
-	p.DiffFn = testDiffFn
-	schema := p.GetSchemaReturn
-	schema.ProviderMeta = &configschema.Block{
-		Attributes: map[string]*configschema.Attribute{
-			"baz": {
-				Type:     cty.String,
-				Required: true,
-			},
-		},
-	}
-	p.GetSchemaReturn = schema
-	ctx := testContext2(t, &ContextOpts{
-		Config: m,
-		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
-		},
-	})
-
-	_, diags := ctx.Plan()
-	assertNoErrors(t, diags)
-
-	_, diags = ctx.Apply()
-	assertNoErrors(t, diags)
-
-	_, diags = ctx.Refresh()
-	assertNoErrors(t, diags)
-
-	if !p.ReadResourceCalled {
-		t.Fatalf("ReadResource not called")
-	}
-	if !p.ReadResourceRequest.ProviderMeta.IsNull() {
-		t.Fatalf("Expected null ProviderMeta in ReadResource, got %v", p.ReadResourceRequest.ProviderMeta)
-	}
-}
-
 func TestContext2Apply_ProviderMeta_refresh_setNoSchema(t *testing.T) {
 	m := testModule(t, "provider-meta-set")
 	p := testProvider("test")
@@ -10924,10 +10831,7 @@ func TestContext2Apply_ProviderMeta_refreshdata_unset(t *testing.T) {
 		}
 	}
 
-	_, diags := ctx.Refresh()
+	_, diags := ctx.Plan()
-	assertNoErrors(t, diags)
-
-	_, diags = ctx.Plan()
 	assertNoErrors(t, diags)
 
 	_, diags = ctx.Apply()
@@ -11228,12 +11132,7 @@ func TestContext2Apply_moduleDependsOn(t *testing.T) {
 		},
 	})
 
-	_, diags := ctx.Refresh()
+	_, diags := ctx.Plan()
-	if diags.HasErrors() {
-		t.Fatal(diags.ErrWithWarnings())
-	}
-
-	_, diags = ctx.Plan()
 	if diags.HasErrors() {
 		t.Fatal(diags.ErrWithWarnings())
 	}
@@ -11243,11 +11142,6 @@ func TestContext2Apply_moduleDependsOn(t *testing.T) {
 		t.Fatal(diags.ErrWithWarnings())
 	}
 
-	// run the plan again to ensure that data sources are not going to be re-read
-	_, diags = ctx.Refresh()
-	if diags.HasErrors() {
-		t.Fatal(diags.ErrWithWarnings())
-	}
 	plan, diags := ctx.Plan()
 	if diags.HasErrors() {
 		t.Fatal(diags.ErrWithWarnings())
@@ -11799,10 +11693,6 @@ output "outputs" {
 			Destroy: true,
 		})
 
-		if _, diags := ctx.Refresh(); diags.HasErrors() {
-			t.Fatalf("destroy plan errors: %s", diags.Err())
-		}
-
 		if _, diags := ctx.Plan(); diags.HasErrors() {
 			t.Fatalf("destroy plan errors: %s", diags.Err())
 		}

terraform/context_refresh_test.go

@@ -52,6 +52,7 @@ func TestContext2Refresh(t *testing.T) {
 	p.ReadResourceResponse = providers.ReadResourceResponse{
 		NewState: readState,
 	}
+	p.DiffFn = testDiffFn
 
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -118,6 +119,10 @@ func TestContext2Refresh_dynamicAttr(t *testing.T) {
 			NewState: readStateVal,
 		}
 	}
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
+		resp.PlannedState = req.ProposedNewState
+		return resp
+	}
 
 	ctx := testContext2(t, &ContextOpts{
 		Config: m,
@@ -153,18 +158,15 @@ func TestContext2Refresh_dynamicAttr(t *testing.T) {
 func TestContext2Refresh_dataComputedModuleVar(t *testing.T) {
 	p := testProvider("aws")
 	m := testModule(t, "refresh-data-module-var")
-	ctx := testContext2(t, &ContextOpts{
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
-		Config: m,
+		obj := req.ProposedNewState.AsValueMap()
-		Providers: map[addrs.Provider]providers.Factory{
+		obj["id"] = cty.UnknownVal(cty.String)
-			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+		resp.PlannedState = cty.ObjectVal(obj)
-		},
+		return resp
-	})
+	}
-
+	p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) {
-	p.ReadResourceFn = nil
+		resp.State = req.Config
-	p.ReadResourceResponse = providers.ReadResourceResponse{
+		return resp
-		NewState: cty.ObjectVal(map[string]cty.Value{
-			"id": cty.StringVal("foo"),
-		}),
 	}
 
 	p.GetSchemaReturn = &ProviderSchema{
@@ -190,11 +192,22 @@ func TestContext2Refresh_dataComputedModuleVar(t *testing.T) {
 						Type:     cty.String,
 						Optional: true,
 					},
+					"output": {
+						Type:     cty.String,
+						Computed: true,
+					},
 				},
 			},
 		},
 	}
 
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+		},
+	})
+
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
 		t.Fatalf("refresh errors: %s", diags.Err())
@@ -269,6 +282,7 @@ func TestContext2Refresh_targeted(t *testing.T) {
 			NewState: req.PriorState,
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	_, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -347,6 +361,7 @@ func TestContext2Refresh_targetedCount(t *testing.T) {
 			NewState: req.PriorState,
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	_, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -433,6 +448,7 @@ func TestContext2Refresh_targetedCountIndex(t *testing.T) {
 			NewState: req.PriorState,
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	_, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -464,6 +480,7 @@ func TestContext2Refresh_moduleComputedVar(t *testing.T) {
 			},
 		},
 	}
+	p.DiffFn = testDiffFn
 
 	m := testModule(t, "refresh-module-computed-var")
 	ctx := testContext2(t, &ContextOpts{
@@ -500,6 +517,7 @@ func TestContext2Refresh_delete(t *testing.T) {
 	p.ReadResourceResponse = providers.ReadResourceResponse{
 		NewState: cty.NullVal(p.GetSchemaReturn.ResourceTypes["aws_instance"].ImpliedType()),
 	}
+	p.DiffFn = testDiffFn
 
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -529,6 +547,7 @@ func TestContext2Refresh_ignoreUncreated(t *testing.T) {
 			"id": cty.StringVal("foo"),
 		}),
 	}
+	p.DiffFn = testDiffFn
 
 	_, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -542,6 +561,7 @@ func TestContext2Refresh_ignoreUncreated(t *testing.T) {
 func TestContext2Refresh_hook(t *testing.T) {
 	h := new(MockHook)
 	p := testProvider("aws")
+	p.DiffFn = testDiffFn
 	m := testModule(t, "refresh-basic")
 
 	state := states.NewState()
@@ -603,6 +623,7 @@ func TestContext2Refresh_modules(t *testing.T) {
 			NewState: new,
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -628,6 +649,7 @@ func TestContext2Refresh_moduleInputComputedOutput(t *testing.T) {
 					"foo": {
 						Type:     cty.String,
 						Optional: true,
+						Computed: true,
 					},
 					"compute": {
 						Type:     cty.String,
@@ -683,6 +705,7 @@ func TestContext2Refresh_noState(t *testing.T) {
 			"id": cty.StringVal("foo"),
 		}),
 	}
+	p.DiffFn = testDiffFn
 
 	if _, diags := ctx.Refresh(); diags.HasErrors() {
 		t.Fatalf("refresh errs: %s", diags.Err())
@@ -702,12 +725,13 @@ func TestContext2Refresh_output(t *testing.T) {
 					},
 					"foo": {
 						Type:     cty.String,
-						Computed: true,
+						Optional: true,
 					},
 				},
 			},
 		},
 	}
+	p.DiffFn = testDiffFn
 
 	m := testModule(t, "refresh-output")
 
@@ -815,6 +839,7 @@ func TestContext2Refresh_stateBasic(t *testing.T) {
 	}
 
 	p.ReadResourceFn = nil
+	p.DiffFn = testDiffFn
 	p.ReadResourceResponse = providers.ReadResourceResponse{
 		NewState: readStateVal,
 	}
@@ -843,25 +868,18 @@ func TestContext2Refresh_dataCount(t *testing.T) {
 	p := testProvider("test")
 	m := testModule(t, "refresh-data-count")
 
-	// This test is verifying that a data resource count can refer to a
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
-	// resource attribute that can't be known yet during refresh (because
+		m := req.ProposedNewState.AsValueMap()
-	// the resource in question isn't in the state at all). In that case,
+		m["things"] = cty.ListVal([]cty.Value{cty.StringVal("foo")})
-	// we skip the data resource during refresh and process it during the
+		resp.PlannedState = cty.ObjectVal(m)
-	// subsequent plan step instead.
+		return resp
-	//
+	}
-	// Normally it's an error for "count" to be computed, but during the
-	// refresh step we allow it because we _expect_ to be working with an
-	// incomplete picture of the world sometimes, particularly when we're
-	// creating object for the first time against an empty state.
-	//
-	// For more information, see:
-	//    https://github.com/hashicorp/terraform/issues/21047
-
 	p.GetSchemaReturn = &ProviderSchema{
 		ResourceTypes: map[string]*configschema.Block{
 			"test": {
 				Attributes: map[string]*configschema.Attribute{
-					"things": {Type: cty.List(cty.String), Optional: true},
+					"id":     {Type: cty.String, Computed: true},
+					"things": {Type: cty.List(cty.String), Computed: true},
 				},
 			},
 		},
@@ -870,6 +888,12 @@ func TestContext2Refresh_dataCount(t *testing.T) {
 		},
 	}
 
+	p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse {
+		return providers.ReadDataSourceResponse{
+			State: req.Config,
+		}
+	}
+
 	ctx := testContext2(t, &ContextOpts{
 		Providers: map[addrs.Provider]providers.Factory{
 			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
@@ -878,43 +902,14 @@ func TestContext2Refresh_dataCount(t *testing.T) {
 	})
 
 	s, diags := ctx.Refresh()
-	if p.ReadResourceCalled {
-		// The managed resource doesn't exist in the state yet, so there's
-		// nothing to refresh.
-		t.Errorf("ReadResource was called, but should not have been")
-	}
-	if p.ReadDataSourceCalled {
-		// The data resource should've been skipped because its count cannot
-		// be determined yet.
-		t.Errorf("ReadDataSource was called, but should not have been")
-	}
 
 	if diags.HasErrors() {
 		t.Fatalf("refresh errors: %s", diags.Err())
 	}
 
-	checkStateString(t, s, `<no state>`)
+	checkStateString(t, s, `data.test.foo.0:
-}
+  ID = 
-
+  provider = provider["registry.terraform.io/hashicorp/test"]`)
-func TestContext2Refresh_dataOrphan(t *testing.T) {
-	p := testProvider("null")
-	state := states.NewState()
-	root := state.EnsureModule(addrs.RootModuleInstance)
-	testSetResourceInstanceCurrent(root, "data.null_data_source.bar", `{"id":"foo"}`, `provider["registry.terraform.io/hashicorp/null"]`)
-
-	ctx := testContext2(t, &ContextOpts{
-		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewDefaultProvider("null"): testProviderFuncFixed(p),
-		},
-		State: state,
-	})
-
-	s, diags := ctx.Refresh()
-	if diags.HasErrors() {
-		t.Fatalf("refresh errors: %s", diags.Err())
-	}
-
-	checkStateString(t, s, `<no state>`)
 }
 
 func TestContext2Refresh_dataState(t *testing.T) {
@@ -955,6 +950,7 @@ func TestContext2Refresh_dataState(t *testing.T) {
 			State: readStateVal,
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -1020,6 +1016,7 @@ func TestContext2Refresh_dataStateRefData(t *testing.T) {
 			State: cty.ObjectVal(m),
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -1057,6 +1054,7 @@ func TestContext2Refresh_tainted(t *testing.T) {
 			NewState: cty.ObjectVal(m),
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	s, diags := ctx.Refresh()
 	if diags.HasErrors() {
@@ -1144,6 +1142,7 @@ func TestContext2Refresh_vars(t *testing.T) {
 	}
 
 	p.ReadResourceFn = nil
+	p.DiffFn = testDiffFn
 	p.ReadResourceResponse = providers.ReadResourceResponse{
 		NewState: readStateVal,
 	}
@@ -1197,6 +1196,7 @@ func TestContext2Refresh_orphanModule(t *testing.T) {
 			NewState: req.PriorState,
 		}
 	}
+	p.DiffFn = testDiffFn
 
 	state := states.NewState()
 	root := state.EnsureModule(addrs.RootModuleInstance)
@@ -1266,6 +1266,7 @@ func TestContext2Validate(t *testing.T) {
 			},
 		},
 	}
+	p.DiffFn = testDiffFn
 
 	m := testModule(t, "validate-good")
 	c := testContext2(t, &ContextOpts{
@@ -1281,47 +1282,6 @@ func TestContext2Validate(t *testing.T) {
 	}
 }
 
-// TestContext2Refresh_noDiffHookOnScaleOut tests to make sure that
-// pre/post-diff hooks are not called when running EvalDiff on scale-out nodes
-// (nodes with no state). The effect here is to make sure that the diffs -
-// which only exist for interpolation of parallel resources or data sources -
-// do not end up being counted in the UI.
-func TestContext2Refresh_noDiffHookOnScaleOut(t *testing.T) {
-	h := new(MockHook)
-	p := testProvider("aws")
-	m := testModule(t, "refresh-resource-scale-inout")
-
-	// Refresh creates a partial plan for any instances that don't have
-	// remote objects yet, to get stub values for interpolation. Therefore
-	// we need to make DiffFn available to let that complete.
-	p.DiffFn = testDiffFn
-
-	state := states.NewState()
-	root := state.EnsureModule(addrs.RootModuleInstance)
-	testSetResourceInstanceCurrent(root, "aws_instance.foo[0]", `{"id":"foo"}`, `provider["registry.terraform.io/hashicorp/aws"]`)
-	testSetResourceInstanceCurrent(root, "aws_instance.foo[1]", `{"id":"foo"}`, `provider["registry.terraform.io/hashicorp/aws"]`)
-
-	ctx := testContext2(t, &ContextOpts{
-		Config: m,
-		Hooks:  []Hook{h},
-		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
-		},
-		State: state,
-	})
-
-	_, diags := ctx.Refresh()
-	if diags.HasErrors() {
-		t.Fatalf("refresh errors: %s", diags.Err())
-	}
-	if h.PreDiffCalled {
-		t.Fatal("PreDiff should not have been called")
-	}
-	if h.PostDiffCalled {
-		t.Fatal("PostDiff should not have been called")
-	}
-}
-
 func TestContext2Refresh_updateProviderInState(t *testing.T) {
 	m := testModule(t, "update-resource-provider")
 	p := testProvider("aws")
@@ -1357,7 +1317,7 @@ aws_instance.bar:
 }
 
 func TestContext2Refresh_schemaUpgradeFlatmap(t *testing.T) {
-	m := testModule(t, "empty")
+	m := testModule(t, "refresh-schema-upgrade")
 	p := testProvider("test")
 	p.GetSchemaReturn = &ProviderSchema{
 		ResourceTypes: map[string]*configschema.Block{
@@ -1379,6 +1339,7 @@ func TestContext2Refresh_schemaUpgradeFlatmap(t *testing.T) {
 			"name": cty.StringVal("foo"),
 		}),
 	}
+	p.DiffFn = testDiffFn
 
 	s := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(
@@ -1443,7 +1404,7 @@ test_thing.bar:
 }
 
 func TestContext2Refresh_schemaUpgradeJSON(t *testing.T) {
-	m := testModule(t, "empty")
+	m := testModule(t, "refresh-schema-upgrade")
 	p := testProvider("test")
 	p.GetSchemaReturn = &ProviderSchema{
 		ResourceTypes: map[string]*configschema.Block{
@@ -1465,6 +1426,7 @@ func TestContext2Refresh_schemaUpgradeJSON(t *testing.T) {
 			"name": cty.StringVal("foo"),
 		}),
 	}
+	p.DiffFn = testDiffFn
 
 	s := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(
@@ -1542,6 +1504,7 @@ data "aws_data_source" "foo" {
 		resp.State = req.Config
 		return
 	}
+	p.DiffFn = testDiffFn
 
 	ctx := testContext2(t, &ContextOpts{
 		Config: m,

terraform/eval_state.go

@@ -549,12 +549,7 @@ func (n *EvalRefreshDependencies) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, nil
 	}
 
-	depMap := make(map[string]addrs.ConfigResource)
+	// We already have dependencies in state, so we need to trust those for
-	for _, d := range *n.Dependencies {
-		depMap[d.String()] = d
-	}
-
-	// We have already dependencies in state, so we need to trust those for
 	// refresh. We can't write out new dependencies until apply time in case
 	// the configuration has been changed in a manner the conflicts with the
 	// stored dependencies.
@@ -563,6 +558,11 @@ func (n *EvalRefreshDependencies) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, nil
 	}
 
+	depMap := make(map[string]addrs.ConfigResource)
+	for _, d := range *n.Dependencies {
+		depMap[d.String()] = d
+	}
+
 	deps := make([]addrs.ConfigResource, 0, len(depMap))
 	for _, d := range depMap {
 		deps = append(deps, d)

terraform/graph_builder_plan.go

@@ -146,6 +146,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		// Connect so that the references are ready for targeting. We'll
 		// have to connect again later for providers and so on.
 		&ReferenceTransformer{},
+		&AttachDependenciesTransformer{},
 
 		// Make sure data sources are aware of any depends_on from the
 		// configuration

terraform/node_output.go

@@ -230,6 +230,13 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error {
 			return diags.Err()
 		}
 		n.setValue(state, changes, val)
+
+		// If we were able to evaluate a new value, we can update that in the
+		// refreshed state as well.
+		if state = ctx.RefreshState(); state != nil && val.IsWhollyKnown() {
+			n.setValue(state, changes, val)
+		}
+
 		return nil
 	default:
 		return nil

terraform/node_resource_plan.go

@@ -20,6 +20,10 @@ type nodeExpandPlannableResource struct {
 	// during graph construction, if dependencies require us to force this
 	// on regardless of what the configuration says.
 	ForceCreateBeforeDestroy *bool
+
+	// We attach dependencies to the Resource during refresh, since the
+	// instances are instantiated during DynamicExpand.
+	dependencies []addrs.ConfigResource
 }
 
 var (
@@ -29,6 +33,7 @@ var (
 	_ GraphNodeReferencer           = (*nodeExpandPlannableResource)(nil)
 	_ GraphNodeConfigResource       = (*nodeExpandPlannableResource)(nil)
 	_ GraphNodeAttachResourceConfig = (*nodeExpandPlannableResource)(nil)
+	_ GraphNodeAttachDependencies   = (*nodeExpandPlannableResource)(nil)
 	_ GraphNodeTargetable           = (*nodeExpandPlannableResource)(nil)
 )
 
@@ -36,6 +41,11 @@ func (n *nodeExpandPlannableResource) Name() string {
 	return n.NodeAbstractResource.Name() + " (expand)"
 }
 
+// GraphNodeAttachDependencies
+func (n *nodeExpandPlannableResource) AttachDependencies(deps []addrs.ConfigResource) {
+	n.dependencies = deps
+}
+
 // GraphNodeDestroyerCBD
 func (n *nodeExpandPlannableResource) CreateBeforeDestroy() bool {
 	if n.ForceCreateBeforeDestroy != nil {
@@ -71,6 +81,7 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 			NodeAbstractResource:     n.NodeAbstractResource,
 			Addr:                     resAddr,
 			ForceCreateBeforeDestroy: n.ForceCreateBeforeDestroy,
+			dependencies:             n.dependencies,
 		})
 	}
 
@@ -101,6 +112,7 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 		a.Schema = n.Schema
 		a.ProvisionerSchemas = n.ProvisionerSchemas
 		a.ProviderMetas = n.ProviderMetas
+		a.Dependencies = n.dependencies
 
 		return &NodePlannableResourceInstanceOrphan{
 			NodeAbstractResourceInstance: a,
@@ -131,6 +143,8 @@ type NodePlannableResource struct {
 	// during graph construction, if dependencies require us to force this
 	// on regardless of what the configuration says.
 	ForceCreateBeforeDestroy *bool
+
+	dependencies []addrs.ConfigResource
 }
 
 var (
@@ -220,6 +234,7 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) {
 		a.ProvisionerSchemas = n.ProvisionerSchemas
 		a.ProviderMetas = n.ProviderMetas
 		a.dependsOn = n.dependsOn
+		a.Dependencies = n.dependencies
 
 		return &NodePlannableResourceInstance{
 			NodeAbstractResourceInstance: a,

terraform/node_resource_plan_instance.go

@@ -160,6 +160,7 @@ func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
 				State:          &instanceRefreshState,
 				ProviderSchema: &providerSchema,
 				targetState:    refreshState,
+				Dependencies:   &n.Dependencies,
 			},
 
 			// Plan the instance

terraform/testdata/apply-output-depends-on/main.tf

@@ -1,7 +0,0 @@
-resource "aws_instance" "foo" {}
-
-output "value" {
-    value = "result"
-
-    depends_on = ["aws_instance.foo"]
-}

terraform/testdata/refresh-data-count/refresh-data-count.tf

@@ -1,5 +1,4 @@
 resource "test" "foo" {
-  things = ["foo"]
 }
 
 data "test" "foo" {

terraform/testdata/refresh-module-input-computed-output/child/main.tf

@@ -1,11 +1,11 @@
 variable "input" {
-    type = list(string)
+    type = string
 }
 
 resource "aws_instance" "foo" {
-    foo = "${var.input}"
+    foo = var.input
 }
 
 output "foo" {
-    value = "${aws_instance.foo.foo}"
+    value = aws_instance.foo.foo
 }

terraform/testdata/refresh-module-input-computed-output/main.tf

@@ -1,5 +1,5 @@
 module "child" {
-    input = "${aws_instance.bar.foo}"
+    input = aws_instance.bar.foo
     source = "./child"
 }
 

terraform/testdata/refresh-schema-upgrade/main.tf

@@ -0,0 +1,2 @@
+resource "test_thing" "bar" {
+}