diff --git a/config/hcl2shim/paths.go b/config/hcl2shim/paths.go index c80b12ba0..5d2fb02d9 100644 --- a/config/hcl2shim/paths.go +++ b/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 } diff --git a/config/hcl2shim/paths_test.go b/config/hcl2shim/paths_test.go index 167b57a64..ff52c92d9 100644 --- a/config/hcl2shim/paths_test.go +++ b/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) } }) }