Merge pull request #10118 from hashicorp/b-config-race

config: maintain slice index accounting for computed keys

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