diff --git a/terraform/context_test.go b/terraform/context_test.go index 43d19f655..6ff63d813 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -115,6 +115,30 @@ func TestContext2Plan_modules(t *testing.T) { } } +// GH-1475 +func TestContext2Plan_moduleCycle(t *testing.T) { + m := testModule(t, "plan-module-cycle") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanModuleCycleStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContext2Plan_moduleInput(t *testing.T) { m := testModule(t, "plan-module-input") p := testProvider("aws") diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index 2f072abab..23d1eb8ba 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -124,13 +124,9 @@ const testBasicGraphBuilderStr = ` const testBuiltinGraphBuilderBasicStr = ` aws_instance.db - aws_instance.db (destroy tainted) -aws_instance.db (destroy tainted) - aws_instance.web (destroy tainted) + provider.aws aws_instance.web aws_instance.db -aws_instance.web (destroy tainted) - provider.aws provider.aws ` diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index cd3a08aae..b1c2cfa92 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -447,11 +447,44 @@ func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { } func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState) bool { - // Always include anything other than the primary destroy - if n.DestroyMode != DestroyPrimary { + switch n.DestroyMode { + case DestroyPrimary: + return n.destroyIncludePrimary(d, s) + case DestroyTainted: + return n.destroyIncludeTainted(d, s) + default: return true } +} +func (n *graphNodeResourceDestroy) destroyIncludeTainted( + d *ModuleDiff, s *ModuleState) bool { + // If there is no state, there can't by any tainted. + if s == nil { + return false + } + + // Grab the ID which is the prefix (in the case count > 0 at some point) + prefix := n.Original.Resource.Id() + + // Go through the resources and find any with our prefix. If there + // are any tainted, we need to keep it. + for k, v := range s.Resources { + if !strings.HasPrefix(k, prefix) { + continue + } + + if len(v.Tainted) > 0 { + return true + } + } + + // We didn't find any tainted nodes, return + return false +} + +func (n *graphNodeResourceDestroy) destroyIncludePrimary( + d *ModuleDiff, s *ModuleState) bool { // Get the count, and specifically the raw value of the count // (with interpolations and all). If the count is NOT a static "1", // then we keep the destroy node no matter what. diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index b3c0f9529..6e80f92f0 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -924,6 +924,19 @@ STATE: ` +const testTerraformPlanModuleCycleStr = ` +DIFF: + +CREATE: aws_instance.b +CREATE: aws_instance.c + some_input: "" => "" + type: "" => "aws_instance" + +STATE: + + +` + const testTerraformPlanModuleDestroyStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-module-cycle/child/main.tf b/terraform/test-fixtures/plan-module-cycle/child/main.tf new file mode 100644 index 000000000..e2e60c1f0 --- /dev/null +++ b/terraform/test-fixtures/plan-module-cycle/child/main.tf @@ -0,0 +1,5 @@ +variable "in" {} + +output "out" { + value = "${var.in}" +} diff --git a/terraform/test-fixtures/plan-module-cycle/main.tf b/terraform/test-fixtures/plan-module-cycle/main.tf new file mode 100644 index 000000000..e9c459721 --- /dev/null +++ b/terraform/test-fixtures/plan-module-cycle/main.tf @@ -0,0 +1,12 @@ +module "a" { + source = "./child" + in = "${aws_instance.b.id}" +} + +resource "aws_instance" "b" {} + +resource "aws_instance" "c" { + some_input = "${module.a.out}" + + depends_on = ["aws_instance.b"] +} diff --git a/terraform/transform_destroy_test.go b/terraform/transform_destroy_test.go index 784ff0669..313d41ff6 100644 --- a/terraform/transform_destroy_test.go +++ b/terraform/transform_destroy_test.go @@ -299,6 +299,54 @@ func TestPruneDestroyTransformer_countState(t *testing.T) { } } +func TestPruneDestroyTransformer_tainted(t *testing.T) { + mod := testModule(t, "transform-destroy-basic") + + diff := &Diff{} + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Tainted: []*InstanceState{ + &InstanceState{ID: "foo"}, + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &DestroyTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &PruneDestroyTransformer{Diff: diff, State: state} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformPruneDestroyTaintedStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + const testTransformDestroyBasicStr = ` aws_instance.bar aws_instance.bar (destroy tainted) @@ -317,63 +365,46 @@ aws_instance.foo (destroy) const testTransformPruneDestroyBasicStr = ` aws_instance.bar - aws_instance.bar (destroy tainted) aws_instance.foo -aws_instance.bar (destroy tainted) aws_instance.foo - aws_instance.foo (destroy tainted) -aws_instance.foo (destroy tainted) - aws_instance.bar (destroy tainted) ` const testTransformPruneDestroyBasicDiffStr = ` aws_instance.bar - aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo -aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo - aws_instance.foo (destroy tainted) -aws_instance.foo (destroy tainted) - aws_instance.bar (destroy tainted) ` const testTransformPruneDestroyCountStr = ` aws_instance.bar - aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo -aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo - aws_instance.foo (destroy tainted) -aws_instance.foo (destroy tainted) - aws_instance.bar (destroy tainted) ` const testTransformPruneDestroyCountDecStr = ` aws_instance.bar - aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo -aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo - aws_instance.foo (destroy tainted) -aws_instance.foo (destroy tainted) - aws_instance.bar (destroy tainted) ` const testTransformPruneDestroyCountStateStr = ` +aws_instance.bar + aws_instance.foo +aws_instance.foo +` + +const testTransformPruneDestroyTaintedStr = ` aws_instance.bar aws_instance.bar (destroy tainted) aws_instance.foo aws_instance.bar (destroy tainted) aws_instance.foo - aws_instance.foo (destroy tainted) -aws_instance.foo (destroy tainted) - aws_instance.bar (destroy tainted) ` const testTransformCreateBeforeDestroyBasicStr = `