terraform: catch scenario where both "foo" and "foo.0" are in state

terraform/context_test.go

@@ -829,6 +829,64 @@ func TestContext2Plan_countIncreaseFromOne(t *testing.T) {
  }
 }
 
+// https://github.com/PeoplePerHour/terraform/pull/11
+//
+// This tests a case where both a "resource" and "resource.0" are in
+// the state file, which apparently is a reasonable backwards compatibility
+// concern found in the above 3rd party repo.
+func TestContext2Plan_countIncreaseFromOneCorrupted(t *testing.T) {
+ m := testModule(t, "plan-count-inc")
+ p := testProvider("aws")
+ p.DiffFn = testDiffFn
+ s := &State{
+ Modules: []*ModuleState{
+ &ModuleState{
+ Path: rootModulePath,
+ Resources: map[string]*ResourceState{
+ "aws_instance.foo": &ResourceState{
+ Type: "aws_instance",
+ Primary: &InstanceState{
+ ID: "bar",
+ Attributes: map[string]string{
+ "foo": "foo",
+ "type": "aws_instance",
+ },
+ },
+ },
+ "aws_instance.foo.0": &ResourceState{
+ Type: "aws_instance",
+ Primary: &InstanceState{
+ ID: "bar",
+ Attributes: map[string]string{
+ "foo": "foo",
+ "type": "aws_instance",
+ },
+ },
+ },
+ },
+ },
+ },
+ }
+ ctx := testContext2(t, &ContextOpts{
+ Module: m,
+ Providers: map[string]ResourceProviderFactory{
+ "aws": testProviderFuncFixed(p),
+ },
+ State: s,
+ })
+
+ plan, err := ctx.Plan(nil)
+ if err != nil {
+ t.Fatalf("err: %s", err)
+ }
+
+ actual := strings.TrimSpace(plan.String())
+ expected := strings.TrimSpace(testTerraformPlanCountIncreaseFromOneCorruptedStr)
+ if actual != expected {
+ t.Fatalf("bad:\n%s", actual)
+ }
+}
+
 func TestContext2Plan_destroy(t *testing.T) {
  m := testModule(t, "plan-destroy")
  p := testProvider("aws")
@@ -3097,6 +3155,7 @@ func TestContext2Apply_countDecrease(t *testing.T) {
 func TestContext2Apply_countDecreaseToOne(t *testing.T) {
  m := testModule(t, "apply-count-dec-one")
  p := testProvider("aws")
+ p.ApplyFn = testApplyFn
  p.DiffFn = testDiffFn
  s := &State{
  Modules: []*ModuleState{
@@ -3153,6 +3212,70 @@ func TestContext2Apply_countDecreaseToOne(t *testing.T) {
  }
 }
 
+// https://github.com/PeoplePerHour/terraform/pull/11
+//
+// This tests a case where both a "resource" and "resource.0" are in
+// the state file, which apparently is a reasonable backwards compatibility
+// concern found in the above 3rd party repo.
+func TestContext2Apply_countDecreaseToOneCorrupted(t *testing.T) {
+ m := testModule(t, "apply-count-dec-one")
+ p := testProvider("aws")
+ p.ApplyFn = testApplyFn
+ p.DiffFn = testDiffFn
+ s := &State{
+ Modules: []*ModuleState{
+ &ModuleState{
+ Path: rootModulePath,
+ Resources: map[string]*ResourceState{
+ "aws_instance.foo": &ResourceState{
+ Type: "aws_instance",
+ Primary: &InstanceState{
+ ID: "bar",
+ Attributes: map[string]string{
+ "foo": "foo",
+ "type": "aws_instance",
+ },
+ },
+ },
+ "aws_instance.foo.0": &ResourceState{
+ Type: "aws_instance",
+ Primary: &InstanceState{
+ ID: "baz",
+ Attributes: map[string]string{
+ "type": "aws_instance",
+ },
+ },
+ },
+ },
+ },
+ },
+ }
+ ctx := testContext2(t, &ContextOpts{
+ Module: m,
+ Providers: map[string]ResourceProviderFactory{
+ "aws": testProviderFuncFixed(p),
+ },
+ State: s,
+ })
+
+ if p, err := ctx.Plan(nil); err != nil {
+ t.Fatalf("err: %s", err)
+ } else {
+ testStringMatch(t, p, testTerraformApplyCountDecToOneCorruptedPlanStr)
+ }
+
+ state, err := ctx.Apply()
+ if err != nil {
+ t.Fatalf("err: %s", err)
+ }
+
+ actual := strings.TrimSpace(state.String())
+ expected := strings.TrimSpace(testTerraformApplyCountDecToOneCorruptedStr)
+ if actual != expected {
+ t.Fatalf("bad: \n%s", actual)
+ }
+}
+
 func TestContext2Apply_countTainted(t *testing.T) {
  m := testModule(t, "apply-count-tainted")
  p := testProvider("aws")

terraform/eval_count.go

@@ -41,10 +41,18 @@ func (n *EvalCountFixZeroOneBoundary) Eval(ctx EvalContext) (interface{}, error)
  }
 
  // Look for the resource state. If we don't have one, then it is okay.
- if rs, ok := mod.Resources[hunt]; ok {
-  mod.Resources[replace] = rs
- delete(mod.Resources, hunt)
+ rs, ok := mod.Resources[hunt]
+ if !ok {
+ return nil, nil
  }
 
+ // If the replacement key exists, we just keep both
+ if _, ok := mod.Resources[replace]; ok {
+ return nil, nil
+ }
+
+ mod.Resources[replace] = rs
+ delete(mod.Resources, hunt)
+
  return nil, nil
 }

terraform/graph_config_node.go

@@ -481,6 +481,14 @@ func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState)
  return true
  }
  }
+
+ // If we're in the state as _both_ "foo" and "foo.0", then
+ // keep it, since we treat the latter as an orphan.
+ _, okOne := s.Resources[prefix]
+ _, okTwo := s.Resources[prefix+".0"]
+ if okOne && okTwo {
+ return true
+ }
  }
 
  return false

terraform/terraform_test.go

@@ -5,9 +5,11 @@ import (
  "crypto/sha1"
  "encoding/gob"
  "encoding/hex"
+ "fmt"
  "io/ioutil"
  "os"
  "path/filepath"
+ "strings"
  "sync"
  "testing"
 
@@ -68,6 +70,14 @@ func testModule(t *testing.T, name string) *module.Tree {
  return mod
 }
 
+func testStringMatch(t *testing.T, s fmt.Stringer, expected string) {
+ actual := strings.TrimSpace(s.String())
+ expected = strings.TrimSpace(expected)
+ if actual != expected {
+ t.Fatalf("Actual\n\n%s\n\nExpected:\n\n%s", actual, expected)
+ }
+}
+
 func testProviderFuncFixed(rp ResourceProvider) ResourceProviderFactory {
  return func() (ResourceProvider, error) {
  return rp, nil
@@ -246,6 +256,29 @@ aws_instance.foo:
  type = aws_instance
 `
 
+const testTerraformApplyCountDecToOneCorruptedStr = `
+aws_instance.foo:
+ ID = bar
+ foo = foo
+ type = aws_instance
+`
+
+const testTerraformApplyCountDecToOneCorruptedPlanStr = `
+DIFF:
+
+DESTROY: aws_instance.foo.0
+
+STATE:
+
+aws_instance.foo:
+ ID = bar
+ foo = foo
+ type = aws_instance
+aws_instance.foo.0:
+ ID = baz
+ type = aws_instance
+`
+
 const testTerraformApplyCountTaintedStr = `
 <no state>
 `
@@ -796,6 +829,32 @@ aws_instance.foo.0:
  type = aws_instance
 `
 
+const testTerraformPlanCountIncreaseFromOneCorruptedStr = `
+DIFF:
+
+CREATE: aws_instance.bar
+ foo: "" => "bar"
+ type: "" => "aws_instance"
+DESTROY: aws_instance.foo
+CREATE: aws_instance.foo.1
+ foo: "" => "foo"
+ type: "" => "aws_instance"
+CREATE: aws_instance.foo.2
+ foo: "" => "foo"
+ type: "" => "aws_instance"
+
+STATE:
+
+aws_instance.foo:
+ ID = bar
+ foo = foo
+ type = aws_instance
+aws_instance.foo.0:
+ ID = bar
+ foo = foo
+ type = aws_instance
+`
+
 const testTerraformPlanDestroyStr = `
 DIFF:
 

terraform/test-fixtures/apply-count-dec-one/main.tf

@@ -1,7 +1,3 @@
 resource "aws_instance" "foo" {
  foo = "foo"
 }
-
-resource "aws_instance" "bar" {
- foo = "bar"
-}