diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index 828045ec3e8c..81fa812087f8 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -32,7 +32,7 @@ type interpolationWalker struct { cs []reflect.Value csKey []reflect.Value csData interface{} - sliceIndex int + sliceIndex []int unknownKeys []string } @@ -54,9 +54,6 @@ type interpolationWalkerContextFunc func(reflectwalk.Location, ast.Node) func (w *interpolationWalker) Enter(loc reflectwalk.Location) error { w.loc = loc - if loc == reflectwalk.WalkLoc { - w.sliceIndex = -1 - } return nil } @@ -75,7 +72,7 @@ func (w *interpolationWalker) Exit(loc reflectwalk.Location) error { w.cs = w.cs[:len(w.cs)-1] case reflectwalk.SliceElem: w.csKey = w.csKey[:len(w.csKey)-1] - w.sliceIndex = -1 + w.sliceIndex = w.sliceIndex[:len(w.sliceIndex)-1] } return nil @@ -90,8 +87,8 @@ func (w *interpolationWalker) MapElem(m, k, v reflect.Value) error { w.csData = k w.csKey = append(w.csKey, k) - if w.sliceIndex != -1 { - w.key = append(w.key, fmt.Sprintf("%d.%s", w.sliceIndex, k.String())) + if l := len(w.sliceIndex); l > 0 { + w.key = append(w.key, fmt.Sprintf("%d.%s", w.sliceIndex[l-1], k.String())) } else { w.key = append(w.key, k.String()) } @@ -107,7 +104,7 @@ func (w *interpolationWalker) Slice(s reflect.Value) error { func (w *interpolationWalker) SliceElem(i int, elem reflect.Value) error { w.csKey = append(w.csKey, reflect.ValueOf(i)) - w.sliceIndex = i + w.sliceIndex = append(w.sliceIndex, i) return nil } diff --git a/config/raw_config_test.go b/config/raw_config_test.go index 70ae968e6c6e..4def6a77a346 100644 --- a/config/raw_config_test.go +++ b/config/raw_config_test.go @@ -339,6 +339,49 @@ func TestRawConfig_unknownPartialList(t *testing.T) { } } +// This tests a race found where we were not maintaining the "slice index" +// accounting properly. The result would be that some computed keys would +// look like they had no slice index when they in fact do. This test is not +// very reliable but it did fail before the fix and passed after. +func TestRawConfig_sliceIndexLoss(t *testing.T) { + raw := map[string]interface{}{ + "slice": []map[string]interface{}{ + map[string]interface{}{ + "foo": []interface{}{"foo/${var.unknown}"}, + "bar": []interface{}{"bar"}, + }, + }, + } + + vars := map[string]ast.Variable{ + "var.unknown": ast.Variable{ + Value: UnknownVariableValue, + Type: ast.TypeUnknown, + }, + "var.known": ast.Variable{ + Value: "123456", + Type: ast.TypeString, + }, + } + + // We run it a lot because its fast and we try to get a race out + for i := 0; i < 50; i++ { + rc, err := NewRawConfig(raw) + if err != nil { + t.Fatalf("err: %s", err) + } + + if err := rc.Interpolate(vars); err != nil { + t.Fatalf("err: %s", err) + } + + expectedKeys := []string{"slice.0.foo"} + if !reflect.DeepEqual(rc.UnknownKeys(), expectedKeys) { + t.Fatalf("bad: %#v", rc.UnknownKeys()) + } + } +} + func TestRawConfigValue(t *testing.T) { raw := map[string]interface{}{ "foo": "${var.bar}",