update the new RequiresReplace function

Make the function work specifically how we need for RequiresReplace.
Skip index changes, any set changes are only recorded as the set itself,
and filter out duplicate paths.

Add a few more tests to check for various nested structures.

config/hcl2shim/paths.go

@@ -13,7 +13,7 @@ import (
 // InstanceDiff.Attributes along with the corresponding cty.Type, and returns
 // the list of the cty.Paths that are flagged as causing the resource
 // replacement (RequiresNew).
-// This will filter out paths that inadvertently refer to flatmapped indexes
+// This will filter out redundant paths, paths that refer to flatmapped indexes
 // (e.g. "#", "%"), and will return any changes within a set as the path to the
 // set itself.
 func RequiresReplace(attrs []string, ty cty.Type) ([]cty.Path, error) {
@@ -30,7 +30,6 @@ func RequiresReplace(attrs []string, ty cty.Type) ([]cty.Path, error) {
 
 	// There may be redundant paths due to set elements or index attributes
 	// Do some ugly n^2 filtering, but these are always fairly small sets.
-
 	for i := 0; i < len(paths)-1; i++ {
 		for j := i + 1; j < len(paths); j++ {
 			if reflect.DeepEqual(paths[i], paths[j]) {
@@ -48,13 +47,14 @@ func RequiresReplace(attrs []string, ty cty.Type) ([]cty.Path, error) {
 // requiresReplacePath takes a key from a flatmap along with the cty.Type
 // describing the structure, and returns the cty.Path that would be used to
 // reference the nested value in the data structure.
-// This is used specifically to record the RequiresReplace attributes from a ResourceInstanceDiff.
+// This is used specifically to record the RequiresReplace attributes from a
+// ResourceInstanceDiff.
 func requiresReplacePath(k string, ty cty.Type) (cty.Path, error) {
 	if k == "" {
 		return nil, nil
 	}
 	if !ty.IsObjectType() {
-		panic(fmt.Sprintf("RequiresReplacePathFromKey called on %#v", ty))
+		panic(fmt.Sprintf("requires replace path on non-object type: %#v", ty))
 	}
 
 	path, err := pathFromFlatmapKeyObject(k, ty.AttributeTypes())
@@ -81,7 +81,7 @@ func pathFromFlatmapKeyObject(key string, atys map[string]cty.Type) (cty.Path, e
 
 	ty, ok := atys[k]
 	if !ok {
-		return path, fmt.Errorf("attribute %q not found", key)
+		return path, fmt.Errorf("attribute %q not found", k)
 	}
 
 	if rest == "" {
@@ -164,7 +164,10 @@ func pathFromFlatmapKeyMap(key string, ty cty.Type) (cty.Path, error) {
 	var path cty.Path
 	var err error
 
-	k, rest := pathSplit(key)
+	k, rest := key, ""
+	if !ty.ElementType().IsPrimitiveType() {
+		k, rest = pathSplit(key)
+	}
 
 	// we don't need to convert the index keys to paths
 	if k == "%" {
@@ -216,31 +219,7 @@ func pathFromFlatmapKeyList(key string, ty cty.Type) (cty.Path, error) {
 }
 
 func pathFromFlatmapKeySet(key string, ty cty.Type) (cty.Path, error) {
-	var path cty.Path
-	var err error
-
-	k, rest := pathSplit(key)
-
-	// we don't need to convert the index keys to paths
-	if k == "#" {
-		return path, nil
-	}
-
-	idx, err := strconv.Atoi(k)
-	if err != nil {
-		return path, err
-	}
-
-	path = cty.Path{cty.IndexStep{Key: cty.NumberIntVal(int64(idx))}}
-
-	if rest == "" {
-		return path, nil
-	}
-
-	p, err := pathFromFlatmapKeyValue(rest, ty.ElementType())
-	if err != nil {
-		return path, err
-	}
-
-	return append(path, p...), nil
+	// once we hit a set, we can't return consistent paths, so just mark the
+	// set as a whole changed.
+	return nil, nil
 }

config/hcl2shim/paths_test.go

@@ -2,10 +2,10 @@ package hcl2shim
 
 import (
 	"fmt"
+	"reflect"
 	"strings"
 	"testing"
 
-	"github.com/go-test/deep"
 	"github.com/zclconf/go-cty/cty"
 )
 
@@ -69,13 +69,14 @@ func TestPathFromFlatmap(t *testing.T) {
 			},
 		},
 		{
+			// a set index returns the set itself, since this being applied to
+			// a diff and the set is changing.
 			Flatmap: "foo.24534534",
 			Type: cty.Object(map[string]cty.Type{
 				"foo": cty.Set(cty.String),
 			}),
 			Want: cty.Path{
 				cty.GetAttrStep{Name: "foo"},
-				cty.IndexStep{Key: cty.NumberIntVal(24534534)},
 			},
 		},
 		{
@@ -89,7 +90,6 @@ func TestPathFromFlatmap(t *testing.T) {
 		},
 		{
 			Flatmap: "foo.baz",
-			//FlatMap: "foo.bar.baz",
 			Type: cty.Object(map[string]cty.Type{
 				"foo": cty.Map(cty.Bool),
 			}),
@@ -111,6 +111,21 @@ func TestPathFromFlatmap(t *testing.T) {
 				cty.IndexStep{Key: cty.StringVal("baz")},
 			},
 		},
+		{
+			Flatmap: "foo.bar.baz",
+			Type: cty.Object(map[string]cty.Type{
+				"foo": cty.Map(
+					cty.Object(map[string]cty.Type{
+						"baz": cty.String,
+					}),
+				),
+			}),
+			Want: cty.Path{
+				cty.GetAttrStep{Name: "foo"},
+				cty.IndexStep{Key: cty.StringVal("bar")},
+				cty.GetAttrStep{Name: "baz"},
+			},
+		},
 		{
 			Flatmap: "foo.0.bar",
 			Type: cty.Object(map[string]cty.Type{
@@ -135,8 +150,49 @@ func TestPathFromFlatmap(t *testing.T) {
 			}),
 			Want: cty.Path{
 				cty.GetAttrStep{Name: "foo"},
-				cty.IndexStep{Key: cty.NumberIntVal(34534534)},
-				cty.GetAttrStep{Name: "baz"},
+			},
+		},
+		{
+			Flatmap: "foo.bar.bang",
+			Type: cty.Object(map[string]cty.Type{
+				"foo": cty.String,
+			}),
+			WantErr: `invalid step "bar.bang"`,
+		},
+		{
+			// there should not be any attribute names with dots
+			Flatmap: "foo.bar.bang",
+			Type: cty.Object(map[string]cty.Type{
+				"foo.bar": cty.Map(cty.String),
+			}),
+			WantErr: `attribute "foo" not found`,
+		},
+		{
+			// We can only handle key names with dots if the map elements are a
+			// primitive type.
+			Flatmap: "foo.bar.bop",
+			Type: cty.Object(map[string]cty.Type{
+				"foo": cty.Map(cty.String),
+			}),
+			Want: cty.Path{
+				cty.GetAttrStep{Name: "foo"},
+				cty.IndexStep{Key: cty.StringVal("bar.bop")},
+			},
+		},
+		{
+			Flatmap: "foo.bar.0.baz",
+			Type: cty.Object(map[string]cty.Type{
+				"foo": cty.Map(
+					cty.List(
+						cty.Map(cty.String),
+					),
+				),
+			}),
+			Want: cty.Path{
+				cty.GetAttrStep{Name: "foo"},
+				cty.IndexStep{Key: cty.StringVal("bar")},
+				cty.IndexStep{Key: cty.NumberIntVal(0)},
+				cty.IndexStep{Key: cty.StringVal("baz")},
 			},
 		},
 	}
@@ -159,8 +215,8 @@ func TestPathFromFlatmap(t *testing.T) {
 				}
 			}
 
-			for _, problem := range deep.Equal(got, test.Want) {
-				t.Error(problem)
+			if !reflect.DeepEqual(got, test.Want) {
+				t.Fatalf("incorrect path\ngot:  %#v\nwant: %#v\n", got, test.Want)
 			}
 		})
 	}