config: maintain slice index accounting for computed keys

Fixes #10075
Fixes #10013

When interpolating, we were only maintaining the last known slice index.
If you had sibling slices then you could lose your slice index when
exiting the slice. The resulting behavior was that no some runs the
computed key would be: "slice.0.attr" and on others would be
"slice.attr", the latter being incorrect.

We now maintain a list of slice indexes so that as we unnest, we
properly restore the old value.

Surprisingly unrelated to the graph but the shadow graph caught this
which is great. :)

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
 }
 

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}",