From 3ab4739ba4a728701a456bac2d322f5d772ee393 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 12 Dec 2018 16:01:18 +0000 Subject: [PATCH] backend/local: Avoid rendering data sources on destroy --- backend/local/backend_plan.go | 9 +- backend/local/backend_plan_test.go | 137 ++++++++++++++++++ .../test-fixtures/destroy-with-ds/main.tf | 7 + backend/local/testing.go | 3 + 4 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 backend/local/test-fixtures/destroy-with-ds/main.tf diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index c007edb35..19f9e3df4 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -8,6 +8,7 @@ import ( "sort" "strings" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/plans" @@ -189,7 +190,14 @@ func (b *Local) opPlan( func (b *Local) renderPlan(plan *plans.Plan, schemas *terraform.Schemas) { counts := map[plans.Action]int{} + var rChanges []*plans.ResourceInstanceChangeSrc for _, change := range plan.Changes.Resources { + if change.Action == plans.Delete && change.Addr.Resource.Resource.Mode == addrs.DataResourceMode { + // Avoid rendering data sources on deletion + continue + } + + rChanges = append(rChanges, change) counts[change.Action]++ } @@ -225,7 +233,6 @@ func (b *Local) renderPlan(plan *plans.Plan, schemas *terraform.Schemas) { // here. The ordering of resource changes in a plan is not significant, // but we can only do this safely here because we can assume that nobody // is concurrently modifying our changes while we're trying to print it. - rChanges := plan.Changes.Resources sort.Slice(rChanges, func(i, j int) bool { iA := rChanges[i].Addr jA := rChanges[j].Addr diff --git a/backend/local/backend_plan_test.go b/backend/local/backend_plan_test.go index 74791456f..f0defcffb 100644 --- a/backend/local/backend_plan_test.go +++ b/backend/local/backend_plan_test.go @@ -212,6 +212,94 @@ func TestLocal_planDestroy(t *testing.T) { } } +func TestLocal_planDestroy_withDataSources(t *testing.T) { + b, cleanup := TestLocal(t) + defer cleanup() + + p := TestLocalProvider(t, b, "test", planFixtureSchema()) + testStateFile(t, b.StatePath, testPlanState_withDataSource()) + + b.CLI = cli.NewMockUi() + + outDir := testTempDir(t) + defer os.RemoveAll(outDir) + planPath := filepath.Join(outDir, "plan.tfplan") + + op, configCleanup := testOperationPlan(t, "./test-fixtures/destroy-with-ds") + defer configCleanup() + op.Destroy = true + op.PlanRefresh = true + op.PlanOutPath = planPath + cfg := cty.ObjectVal(map[string]cty.Value{ + "path": cty.StringVal(b.StatePath), + }) + cfgRaw, err := plans.NewDynamicValue(cfg, cfg.Type()) + if err != nil { + t.Fatal(err) + } + op.PlanOutBackend = &plans.Backend{ + // Just a placeholder so that we can generate a valid plan file. + Type: "local", + Config: cfgRaw, + } + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("bad: %s", err) + } + <-run.Done() + if run.Result != backend.OperationSuccess { + t.Fatalf("plan operation failed") + } + + if !p.ReadResourceCalled { + t.Fatal("ReadResource should be called") + } + + if !p.ReadDataSourceCalled { + t.Fatal("ReadDataSourceCalled should be called") + } + + if run.PlanEmpty { + t.Fatal("plan should not be empty") + } + + // Data source should still exist in the the plan file + plan := testReadPlan(t, planPath) + if len(plan.Changes.Resources) != 2 { + t.Fatalf("Expected exactly 1 resource for destruction, %d given: %q", + len(plan.Changes.Resources), getAddrs(plan.Changes.Resources)) + } + + // Data source should not be rendered in the output + expectedOutput := `Terraform will perform the following actions: + + # test_instance.foo will be destroyed + - resource "test_instance" "foo" { + - ami = "bar" -> null + + - network_interface { + - description = "Main network interface" -> null + - device_index = 0 -> null + } + } + +Plan: 0 to add, 0 to change, 1 to destroy.` + + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, expectedOutput) { + t.Fatalf("Unexpected output (expected no data source):\n%s", output) + } +} + +func getAddrs(resources []*plans.ResourceInstanceChangeSrc) []string { + addrs := make([]string, len(resources), len(resources)) + for i, r := range resources { + addrs[i] = r.Addr.String() + } + return addrs +} + func TestLocal_planOutPathNoChange(t *testing.T) { b, cleanup := TestLocal(t) defer cleanup() @@ -336,6 +424,48 @@ func testPlanState() *states.State { return state } +func testPlanState_withDataSource() *states.State { + state := states.NewState() + rootModule := state.RootModule() + rootModule.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.IntKey(0)), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{ + "ami": "bar", + "network_interface": [{ + "device_index": 0, + "description": "Main network interface" + }] + }`), + }, + addrs.ProviderConfig{ + Type: "test", + }.Absolute(addrs.RootModuleInstance), + ) + rootModule.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.DataResourceMode, + Type: "test_ds", + Name: "bar", + }.Instance(addrs.IntKey(0)), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{ + "filter": "foo" + }`), + }, + addrs.ProviderConfig{ + Type: "test", + }.Absolute(addrs.RootModuleInstance), + ) + return state +} + func testReadPlan(t *testing.T, path string) *plans.Plan { t.Helper() @@ -376,5 +506,12 @@ func planFixtureSchema() *terraform.ProviderSchema { }, }, }, + DataSources: map[string]*configschema.Block{ + "test_ds": { + Attributes: map[string]*configschema.Attribute{ + "filter": {Type: cty.String, Required: true}, + }, + }, + }, } } diff --git a/backend/local/test-fixtures/destroy-with-ds/main.tf b/backend/local/test-fixtures/destroy-with-ds/main.tf new file mode 100644 index 000000000..4ee80ea3d --- /dev/null +++ b/backend/local/test-fixtures/destroy-with-ds/main.tf @@ -0,0 +1,7 @@ +resource "test_instance" "foo" { + ami = "bar" +} + +data "test_ds" "bar" { + filter = "foo" +} diff --git a/backend/local/testing.go b/backend/local/testing.go index 239706057..05f7600f9 100644 --- a/backend/local/testing.go +++ b/backend/local/testing.go @@ -69,6 +69,9 @@ func TestLocalProvider(t *testing.T, b *Local, name string, schema *terraform.Pr p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse { return providers.ReadResourceResponse{NewState: req.PriorState} } + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{State: req.Config} + } // Initialize the opts if b.ContextOpts == nil {