helper/schema: can diff lists more correctly

helper/schema/schema.go

@@ -3,6 +3,7 @@ package schema
 import (
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/hashicorp/terraform/terraform"
@@ -213,42 +214,57 @@ 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)
-		}
-
-		// We don't have a configuration value.
-		if !schema.Computed {
-			return nil
-		}
-
-		return nil
-	}
-
+	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)
 		}
-	vs := make([]interface{}, rawV.Len())
+		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
 	if len(vs) == 0 && schema.Required {
 		return fmt.Errorf("%s: required field is not set", k)
 	}
 
-	// Diff the count no matter what
+	// 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,
 		}
-	m.diffString(k+".#", countSchema, diff, s, c)
+
+		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
+	}
 
 	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 {

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)