From 801b220dc75408dd3618f42d6106feecf308cf22 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 18 Aug 2014 16:54:30 -0700
Subject: [PATCH] helper/schema: can diff lists more correctly

---
 helper/schema/schema.go      | 74 ++++++++++++++++++++++--------------
 helper/schema/schema_test.go | 39 +++++++++++++++++++
 2 files changed, 84 insertions(+), 29 deletions(-)

diff --git a/helper/schema/schema.go b/helper/schema/schema.go
index 43fb2b050..1452d4712 100644
--- a/helper/schema/schema.go
+++ b/helper/schema/schema.go
@@ -3,6 +3,7 @@ package schema
 import (
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/hashicorp/terraform/terraform"
@@ -213,29 +214,19 @@ func (m schemaMap) diffList(
 	diff *terraform.ResourceDiff,
 	s *terraform.ResourceState,
 	c *terraform.ResourceConfig) error {
+	var vs []interface{}
+
 	v, ok := c.Get(k)
-	if !ok {
-		// We don't have a value, if it is required then it is an error
-		if schema.Required {
-			return fmt.Errorf("%s: required field not set", k)
+	if ok {
+		// We have to use reflection to build the []interface{} list
+		rawV := reflect.ValueOf(v)
+		if rawV.Kind() != reflect.Slice {
+			return fmt.Errorf("%s: must be a list", k)
 		}
-
-		// We don't have a configuration value.
-		if !schema.Computed {
-			return nil
+		vs = make([]interface{}, rawV.Len())
+		for i, _ := range vs {
+			vs[i] = rawV.Index(i).Interface()
 		}
-
-		return nil
-	}
-
-	// We have to use reflection to build the []interface{} list
-	rawV := reflect.ValueOf(v)
-	if rawV.Kind() != reflect.Slice {
-		return fmt.Errorf("%s: must be a list", k)
-	}
-	vs := make([]interface{}, rawV.Len())
-	for i, _ := range vs {
-		vs[i] = rawV.Index(i).Interface()
 	}
 
 	// If this field is required, then it must also be non-empty
@@ -243,12 +234,37 @@ func (m schemaMap) diffList(
 		return fmt.Errorf("%s: required field is not set", k)
 	}
 
-	// Diff the count no matter what
-	countSchema := &Schema{
-		Type:     TypeInt,
-		ForceNew: schema.ForceNew,
+	// Get the counts
+	var oldLen, newLen int
+	if s != nil {
+		if v, ok := s.Attributes[k+".#"]; ok {
+			old64, err := strconv.ParseInt(v, 0, 0)
+			if err != nil {
+				return err
+			}
+			oldLen = int(old64)
+		}
+	}
+	newLen = len(vs)
+
+	// If the counts are not the same, then record that diff
+	if oldLen != newLen {
+		countSchema := &Schema{
+			Type:     TypeInt,
+			ForceNew: schema.ForceNew,
+		}
+
+		diff.Attributes[k+".#"] = countSchema.finalizeDiff(&terraform.ResourceAttrDiff{
+			Old: strconv.FormatInt(int64(oldLen), 10),
+			New: strconv.FormatInt(int64(newLen), 10),
+		})
+	}
+
+	// Figure out the maximum
+	maxLen := oldLen
+	if newLen > maxLen {
+		maxLen = newLen
 	}
-	m.diffString(k+".#", countSchema, diff, s, c)
 
 	switch t := schema.Elem.(type) {
 	case *Schema:
@@ -260,7 +276,7 @@ func (m schemaMap) diffList(
 
 		// This is just a primitive element, so go through each and
 		// just diff each.
-		for i, _ := range vs {
+		for i := 0; i < maxLen; i++ {
 			subK := fmt.Sprintf("%s.%d", k, i)
 			err := m.diff(subK, &t2, diff, s, c)
 			if err != nil {
@@ -269,7 +285,7 @@ func (m schemaMap) diffList(
 		}
 	case *Resource:
 		// This is a complex resource
-		for i, _ := range vs {
+		for i := 0; i < maxLen; i++ {
 			for k2, schema := range t.Schema {
 				subK := fmt.Sprintf("%s.%d.%s", k, i, k2)
 				err := m.diff(subK, schema, diff, s, c)
@@ -358,8 +374,8 @@ func (m schemaMap) diffString(
 			return fmt.Errorf("%s: required field not set", k)
 		}
 
-		// We don't have a configuration value.
-		if !schema.Computed {
+		// If we don't have an old value, just return
+		if old == "" && !schema.Computed {
 			return nil
 		}
 	} else {
diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index a2b0ce477..30cacaff2 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -591,9 +591,48 @@ func TestSchemaMap_Diff(t *testing.T) {
 
 			Err: false,
 		},
+
+		{
+			Schema: map[string]*Schema{
+				"config_vars": &Schema{
+					Type: TypeList,
+					Elem: &Schema{Type: TypeMap},
+				},
+			},
+
+			State: &terraform.ResourceState{
+				Attributes: map[string]string{
+					"config_vars.#":     "1",
+					"config_vars.0.foo": "bar",
+					"config_vars.0.bar": "baz",
+				},
+			},
+
+			Config: map[string]interface{}{},
+
+			Diff: &terraform.ResourceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"config_vars.#": &terraform.ResourceAttrDiff{
+						Old: "1",
+						New: "0",
+					},
+					"config_vars.0.foo": &terraform.ResourceAttrDiff{
+						Old:        "bar",
+						NewRemoved: true,
+					},
+					"config_vars.0.bar": &terraform.ResourceAttrDiff{
+						Old:        "baz",
+						NewRemoved: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
 	}
 
 	for i, tc := range cases {
+		if i != 1 { continue }
 		c, err := config.NewRawConfig(tc.Config)
 		if err != nil {
 			t.Fatalf("err: %s", err)