From a332c121bc7f80d38dac96277c094e68a1d40a37 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 28 Oct 2016 21:31:47 -0400 Subject: [PATCH] terraform: prevent_destroy works for decreasing count Fixes #5826 The `prevent_destroy` lifecycle configuration was not being checked when the count was decreased for a resource with a count. It was only checking when attributes changed on pre-existing resources. This fixes that. --- terraform/context_plan_test.go | 121 ++++++++++++++++++ terraform/eval_check_prevent_destroy.go | 12 +- terraform/graph_config_node_resource.go | 5 +- .../plan-prevent-destroy-count-bad/main.tf | 8 ++ .../plan-prevent-destroy-count-good/main.tf | 4 + terraform/transform_orphan.go | 12 ++ 6 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 terraform/test-fixtures/plan-prevent-destroy-count-bad/main.tf create mode 100644 terraform/test-fixtures/plan-prevent-destroy-count-good/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 2cadb5ab8..2eeb25395 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -805,6 +805,127 @@ func TestContext2Plan_preventDestroy_good(t *testing.T) { } } +func TestContext2Plan_preventDestroy_countBad(t *testing.T) { + m := testModule(t, "plan-prevent-destroy-count-bad") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-abc123", + }, + }, + "aws_instance.foo.1": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-abc345", + }, + }, + }, + }, + }, + }, + }) + + plan, err := ctx.Plan() + + expectedErr := "aws_instance.foo.1: the plan would destroy" + if !strings.Contains(fmt.Sprintf("%s", err), expectedErr) { + t.Fatalf("expected err would contain %q\nerr: %s\nplan: %s", + expectedErr, err, plan) + } +} + +func TestContext2Plan_preventDestroy_countGood(t *testing.T) { + m := testModule(t, "plan-prevent-destroy-count-good") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-abc123", + }, + }, + "aws_instance.foo.1": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-abc345", + }, + }, + }, + }, + }, + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + if plan.Diff.Empty() { + t.Fatalf("Expected non-empty plan, got %s", plan.String()) + } +} + +func TestContext2Plan_preventDestroy_countGoodNoChange(t *testing.T) { + m := testModule(t, "plan-prevent-destroy-count-good") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-abc123", + Attributes: map[string]string{ + "current": "0", + "type": "aws_instance", + }, + }, + }, + }, + }, + }, + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + if !plan.Diff.Empty() { + t.Fatalf("Expected empty plan, got %s", plan.String()) + } +} + func TestContext2Plan_preventDestroy_destroyPlan(t *testing.T) { m := testModule(t, "plan-prevent-destroy-good") p := testProvider("aws") diff --git a/terraform/eval_check_prevent_destroy.go b/terraform/eval_check_prevent_destroy.go index aec0ae134..715e79e17 100644 --- a/terraform/eval_check_prevent_destroy.go +++ b/terraform/eval_check_prevent_destroy.go @@ -10,8 +10,9 @@ import ( // error if a resource has PreventDestroy configured and the diff // would destroy the resource. type EvalCheckPreventDestroy struct { - Resource *config.Resource - Diff **InstanceDiff + Resource *config.Resource + ResourceId string + Diff **InstanceDiff } func (n *EvalCheckPreventDestroy) Eval(ctx EvalContext) (interface{}, error) { @@ -23,7 +24,12 @@ func (n *EvalCheckPreventDestroy) Eval(ctx EvalContext) (interface{}, error) { preventDestroy := n.Resource.Lifecycle.PreventDestroy if diff.GetDestroy() && preventDestroy { - return nil, fmt.Errorf(preventDestroyErrStr, n.Resource.Id()) + resourceId := n.ResourceId + if resourceId == "" { + resourceId = n.Resource.Id() + } + + return nil, fmt.Errorf(preventDestroyErrStr, resourceId) } return nil, nil diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 1c45289a9..afd0c6efa 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -168,8 +168,9 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) // expand orphans, which have all the same semantics in a destroy // as a primary or tainted resource. steps = append(steps, &OrphanTransformer{ - State: state, - View: n.Resource.Id(), + Resource: n.Resource, + State: state, + View: n.Resource.Id(), }) steps = append(steps, &DeposedTransformer{ diff --git a/terraform/test-fixtures/plan-prevent-destroy-count-bad/main.tf b/terraform/test-fixtures/plan-prevent-destroy-count-bad/main.tf new file mode 100644 index 000000000..818f93e70 --- /dev/null +++ b/terraform/test-fixtures/plan-prevent-destroy-count-bad/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + count = "1" + current = "${count.index}" + + lifecycle { + prevent_destroy = true + } +} diff --git a/terraform/test-fixtures/plan-prevent-destroy-count-good/main.tf b/terraform/test-fixtures/plan-prevent-destroy-count-good/main.tf new file mode 100644 index 000000000..b6b479078 --- /dev/null +++ b/terraform/test-fixtures/plan-prevent-destroy-count-good/main.tf @@ -0,0 +1,4 @@ +resource "aws_instance" "foo" { + count = "1" + current = "${count.index}" +} diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index 27f032251..83a4f6d57 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -17,6 +17,11 @@ type GraphNodeStateRepresentative interface { // OrphanTransformer is a GraphTransformer that adds orphans to the // graph. This transformer adds both resource and module orphans. type OrphanTransformer struct { + // Resource is resource configuration. This is only non-nil when + // expanding a resource that is in the configuration. It can't be + // dependend on. + Resource *config.Resource + // State is the global state. We require the global state to // properly find module orphans at our path. State *State @@ -80,6 +85,7 @@ func (t *OrphanTransformer) Transform(g *Graph) error { resourceVertexes[i] = g.Add(&graphNodeOrphanResource{ Path: g.Path, ResourceKey: rsk, + Resource: t.Resource, Provider: rs.Provider, dependentOn: rs.Dependencies, }) @@ -159,6 +165,7 @@ func (n *graphNodeOrphanModule) Expand(b GraphBuilder) (GraphNodeSubgraph, error type graphNodeOrphanResource struct { Path []string ResourceKey *ResourceStateKey + Resource *config.Resource Provider string dependentOn []string @@ -283,6 +290,11 @@ func (n *graphNodeOrphanResource) managedResourceEvalNodes(info *InstanceInfo) [ State: &state, Output: &diff, }, + &EvalCheckPreventDestroy{ + Resource: n.Resource, + ResourceId: n.ResourceKey.String(), + Diff: &diff, + }, &EvalWriteDiff{ Name: n.ResourceKey.String(), Diff: &diff,