From 7951b55f8853afcd13f3f32727b077a2545ad560 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 18 Sep 2020 16:45:39 -0400 Subject: [PATCH 1/7] remove planned resources from refreshed state --- terraform/context.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index dd9889721..6c671c1a0 100644 --- a/terraform/context.go +++ b/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. From f222ed74799e969cead36f87bce699ac24aa13bd Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Sep 2020 09:36:50 -0400 Subject: [PATCH 2/7] write updated outputs to the refresh state If we can evaluate a new output value during plan, write it to the refreshed state as well. --- terraform/node_output.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/terraform/node_output.go b/terraform/node_output.go index 51eb7e156..8423931fd 100644 --- a/terraform/node_output.go +++ b/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 From 3b3ff98356fb9b8dd15d361433b8a84456637544 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Sep 2020 14:39:25 -0400 Subject: [PATCH 3/7] Revert "fix show -json tests" This reverts commit e54949f2e1cbd62d0401f7d2b7b7d2f34735cbfc. Changes incorrectly applied to the planned state tests --- command/testdata/show-json/basic-create/output.json | 13 ++++++++++++- .../show-json/basic-delete/terraform.tfstate | 7 +------ .../show-json/basic-update/terraform.tfstate | 7 +------ command/testdata/show-json/modules/output.json | 13 ++++++++++++- .../show-json/multi-resource-update/output.json | 8 +++++++- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/command/testdata/show-json/basic-create/output.json b/command/testdata/show-json/basic-create/output.json index 19e1846cc..ff83de3fe 100644 --- a/command/testdata/show-json/basic-create/output.json +++ b/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]", diff --git a/command/testdata/show-json/basic-delete/terraform.tfstate b/command/testdata/show-json/basic-delete/terraform.tfstate index 6d2b62237..ac865b864 100644 --- a/command/testdata/show-json/basic-delete/terraform.tfstate +++ b/command/testdata/show-json/basic-delete/terraform.tfstate @@ -3,12 +3,7 @@ "terraform_version": "0.12.0", "serial": 7, "lineage": "configuredUnchanged", - "outputs": { - "test": { - "value": "bar", - "type": "string" - } - }, + "outputs": {}, "resources": [ { "mode": "managed", diff --git a/command/testdata/show-json/basic-update/terraform.tfstate b/command/testdata/show-json/basic-update/terraform.tfstate index bc691aee0..b57f60f84 100644 --- a/command/testdata/show-json/basic-update/terraform.tfstate +++ b/command/testdata/show-json/basic-update/terraform.tfstate @@ -3,12 +3,7 @@ "terraform_version": "0.12.0", "serial": 7, "lineage": "configuredUnchanged", - "outputs": { - "test": { - "value": "bar", - "type": "string" - } - }, + "outputs": {}, "resources": [ { "mode": "managed", diff --git a/command/testdata/show-json/modules/output.json b/command/testdata/show-json/modules/output.json index 76b4f471c..898763aad 100644 --- a/command/testdata/show-json/modules/output.json +++ b/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", diff --git a/command/testdata/show-json/multi-resource-update/output.json b/command/testdata/show-json/multi-resource-update/output.json index ba764b265..cc8f6d1ed 100644 --- a/command/testdata/show-json/multi-resource-update/output.json +++ b/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": { From c3182bd589868c027f298b84286c77321b6f9694 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Sep 2020 14:48:21 -0400 Subject: [PATCH 4/7] mock provider needs to return a valid response --- command/command_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/command/command_test.go b/command/command_test.go index 6c62d1e69..450a36a07 100644 --- a/command/command_test.go +++ b/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{ - PlannedState: cty.EmptyObjectVal, + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + resp.PlannedState = req.ProposedNewState + return resp } + p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse { return providers.ReadResourceResponse{ NewState: req.PriorState, From 8105096ec814248dc10b873f4758036478ba9786 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Sep 2020 15:53:53 -0400 Subject: [PATCH 5/7] attach dependencies during plan This was previously done during refresh alone, now we need to insert these during the refresh portion of plan. --- terraform/eval_state.go | 12 ++++++------ terraform/graph_builder_plan.go | 1 + terraform/node_resource_plan.go | 15 +++++++++++++++ terraform/node_resource_plan_instance.go | 1 + 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/terraform/eval_state.go b/terraform/eval_state.go index b5c596873..50f363b2d 100644 --- a/terraform/eval_state.go +++ b/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) - for _, d := range *n.Dependencies { - depMap[d.String()] = d - } - - // We have already dependencies in state, so we need to trust those for + // We already have 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) diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index a79354a49..082d9721c 100644 --- a/terraform/graph_builder_plan.go +++ b/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 diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index db149328e..d20120715 100644 --- a/terraform/node_resource_plan.go +++ b/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, diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 7cad9d4a4..58e1545fc 100644 --- a/terraform/node_resource_plan_instance.go +++ b/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 From 4a6dac39a547670e114adde1d6e5cf259fda7346 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Sep 2020 15:56:07 -0400 Subject: [PATCH 6/7] Use Plan instead of Refresh Now that the planning process generates a refreshed state, and can handle changes between the configuration and the state which the refresh process cannot, we can use the plan for the refresh command. --- terraform/context.go | 44 +++++--------------------------------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 6c671c1a0..56b5578b6 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -578,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 -// works with, along with returning it. +// to their latest state. This is done by executing a plan, and retaining the +// state while discarding the change set. // -// Even in the case an error is returned, the state may be returned and -// will potentially be partially updated. +// In the case of an error, there is no state returned. func (c *Context) Refresh() (*states.State, tfdiags.Diagnostics) { - defer c.acquireRun("refresh")() - - // 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) + p, diags := c.Plan() if diags.HasErrors() { return nil, diags } - // Do the walk - _, 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 + return p.State, diags } // Stop stops the running task. From bc82347a04e1310fb45fefaae28c767a407e8814 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Sep 2020 15:58:06 -0400 Subject: [PATCH 7/7] fix tests Update tests to match the new behavior. Some were incorrect, some no longer make sense, and some just weren't setup to handle th plan api calls. --- backend/local/backend_refresh_test.go | 74 +------- .../local/testdata/refresh-var-unset/main.tf | 2 +- terraform/context_apply_test.go | 158 +++------------- terraform/context_refresh_test.go | 169 +++++++----------- .../testdata/apply-output-depends-on/main.tf | 7 - .../refresh-data-count/refresh-data-count.tf | 1 - .../child/main.tf | 6 +- .../main.tf | 2 +- .../testdata/refresh-schema-upgrade/main.tf | 2 + 9 files changed, 104 insertions(+), 317 deletions(-) delete mode 100644 terraform/testdata/apply-output-depends-on/main.tf create mode 100644 terraform/testdata/refresh-schema-upgrade/main.tf diff --git a/backend/local/backend_refresh_test.go b/backend/local/backend_refresh_test.go index d6554c47a..7d96b971b 100644 --- a/backend/local/backend_refresh_test.go +++ b/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"), diff --git a/backend/local/testdata/refresh-var-unset/main.tf b/backend/local/testdata/refresh-var-unset/main.tf index 7d6b13014..8e6b73d0d 100644 --- a/backend/local/testdata/refresh-var-unset/main.tf +++ b/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" { diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 22f0d5657..182b678b8 100644 --- a/terraform/context_apply_test.go +++ b/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, ``) - } - - { - // 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() - assertNoErrors(t, diags) - - _, diags = ctx.Plan() + _, 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() - assertNoErrors(t, diags) - - _, diags = ctx.Plan() + _, diags := ctx.Plan() assertNoErrors(t, diags) _, diags = ctx.Apply() @@ -11228,12 +11132,7 @@ func TestContext2Apply_moduleDependsOn(t *testing.T) { }, }) - _, diags := ctx.Refresh() - if diags.HasErrors() { - t.Fatal(diags.ErrWithWarnings()) - } - - _, diags = ctx.Plan() + _, 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()) } diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index 64be9098f..2ea079664 100644 --- a/terraform/context_refresh_test.go +++ b/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{ - Config: m, - Providers: map[addrs.Provider]providers.Factory{ - addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), - }, - }) - - p.ReadResourceFn = nil - p.ReadResourceResponse = providers.ReadResourceResponse{ - NewState: cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("foo"), - }), + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + obj := req.ProposedNewState.AsValueMap() + obj["id"] = cty.UnknownVal(cty.String) + resp.PlannedState = cty.ObjectVal(obj) + return resp + } + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) { + resp.State = req.Config + return resp } 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 - // resource attribute that can't be known yet during refresh (because - // the resource in question isn't in the state at all). In that case, - // we skip the data resource during refresh and process it during the - // subsequent plan step instead. - // - // 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.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + m := req.ProposedNewState.AsValueMap() + m["things"] = cty.ListVal([]cty.Value{cty.StringVal("foo")}) + resp.PlannedState = cty.ObjectVal(m) + return resp + } 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, ``) -} - -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, ``) + checkStateString(t, s, `data.test.foo.0: + ID = + provider = provider["registry.terraform.io/hashicorp/test"]`) } 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, diff --git a/terraform/testdata/apply-output-depends-on/main.tf b/terraform/testdata/apply-output-depends-on/main.tf deleted file mode 100644 index 4923a6f58..000000000 --- a/terraform/testdata/apply-output-depends-on/main.tf +++ /dev/null @@ -1,7 +0,0 @@ -resource "aws_instance" "foo" {} - -output "value" { - value = "result" - - depends_on = ["aws_instance.foo"] -} diff --git a/terraform/testdata/refresh-data-count/refresh-data-count.tf b/terraform/testdata/refresh-data-count/refresh-data-count.tf index ac385a414..ccabdb2c6 100644 --- a/terraform/testdata/refresh-data-count/refresh-data-count.tf +++ b/terraform/testdata/refresh-data-count/refresh-data-count.tf @@ -1,5 +1,4 @@ resource "test" "foo" { - things = ["foo"] } data "test" "foo" { diff --git a/terraform/testdata/refresh-module-input-computed-output/child/main.tf b/terraform/testdata/refresh-module-input-computed-output/child/main.tf index 1e3a16519..ebc1e3ffc 100644 --- a/terraform/testdata/refresh-module-input-computed-output/child/main.tf +++ b/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 } diff --git a/terraform/testdata/refresh-module-input-computed-output/main.tf b/terraform/testdata/refresh-module-input-computed-output/main.tf index 3a0576434..5827a5da2 100644 --- a/terraform/testdata/refresh-module-input-computed-output/main.tf +++ b/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" } diff --git a/terraform/testdata/refresh-schema-upgrade/main.tf b/terraform/testdata/refresh-schema-upgrade/main.tf new file mode 100644 index 000000000..ee0590e3c --- /dev/null +++ b/terraform/testdata/refresh-schema-upgrade/main.tf @@ -0,0 +1,2 @@ +resource "test_thing" "bar" { +}