terraform: multi-var interpolation should use state for count

Related to #5254

If the count of a resource is interpolated (i.e. `${var.c}`), then it
must be interpolated before any splat variable using that resource can
be used (i.e. `type.name.*.attr`). The original fix for #5254 is to
always ensure that this is the case.

While working on a new apply builder based on the diff in
`f-apply-builder`, this truth no longer always holds. Rather than always
include such a resource, I believe the correct behavior instead is to
use the state as a source of truth during `walkApply` operations.

This change specifically is scoped to `walkApply` operation
interpolations since we know the state of any multi-variable should be
available. The behavior is less clear for other operations so I left the
logic unchanged from prior versions.

terraform/interpolate.go

@@ -490,17 +490,14 @@ func (i *Interpolater) computeResourceMultiVariable(
 		return nil, err
 	}
 
-	// Get the count so we know how many to iterate over
-	count, err := cr.Count()
+	// Get the keys for all the resources that are created for this resource
+	resourceKeys, err := i.resourceCountKeys(module, cr, v)
 	if err != nil {
-		return nil, fmt.Errorf(
-			"Error reading %s count: %s",
-			v.ResourceId(),
-			err)
+		return nil, err
 	}
 
 	// If count is zero, we return an empty list
-	if count == 0 {
+	if len(resourceKeys) == 0 {
 		return &ast.Variable{Type: ast.TypeList, Value: []ast.Variable{}}, nil
 	}
 
@@ -510,13 +507,15 @@ func (i *Interpolater) computeResourceMultiVariable(
 	}
 
 	var values []interface{}
-	for j := 0; j < count; j++ {
-		id := fmt.Sprintf("%s.%d", v.ResourceId(), j)
-
-		// If we're dealing with only a single resource, then the
-		// ID doesn't have a trailing index.
-		if count == 1 {
-			id = v.ResourceId()
+	for _, id := range resourceKeys {
+		// ID doesn't have a trailing index. We try both here, but if a value
+		// without a trailing index is found we prefer that. This choice
+		// is for legacy reasons: older versions of TF preferred it.
+		if id == v.ResourceId()+".0" {
+			potential := v.ResourceId()
+			if _, ok := module.Resources[potential]; ok {
+				id = potential
+			}
 		}
 
 		r, ok := module.Resources[id]
@@ -678,3 +677,44 @@ func (i *Interpolater) resourceVariableInfo(
 	module := i.State.ModuleByPath(scope.Path)
 	return module, cr, nil
 }
+
+func (i *Interpolater) resourceCountKeys(
+	ms *ModuleState,
+	cr *config.Resource,
+	v *config.ResourceVariable) ([]string, error) {
+	id := v.ResourceId()
+
+	// If we're NOT applying, then we assume we can read the count
+	// from the state. Plan and so on may not have any state yet so
+	// we do a full interpolation.
+	if i.Operation != walkApply {
+		count, err := cr.Count()
+		if err != nil {
+			return nil, err
+		}
+
+		result := make([]string, count)
+		for i := 0; i < count; i++ {
+			result[i] = fmt.Sprintf("%s.%d", id, i)
+		}
+
+		return result, nil
+	}
+
+	// We need to determine the list of resource keys to get values from.
+	// This needs to be sorted so the order is deterministic. We used to
+	// use "cr.Count()" but that doesn't work if the count is interpolated
+	// and we can't guarantee that so we instead depend on the state.
+	var resourceKeys []string
+	for k, _ := range ms.Resources {
+		// If we don't have the right prefix then ignore it
+		if k != id && !strings.HasPrefix(k, id+".") {
+			continue
+		}
+
+		// Add it to the list
+		resourceKeys = append(resourceKeys, k)
+	}
+	sort.Strings(resourceKeys)
+	return resourceKeys, nil
+}

terraform/interpolate_test.go

@@ -358,6 +358,49 @@ func TestInterpolater_resourceVariableMulti(t *testing.T) {
 	})
 }
 
+func TestInterpolater_resourceVariableMulti_interpolated(t *testing.T) {
+	lock := new(sync.RWMutex)
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.web.0": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID:         "a",
+							Attributes: map[string]string{"foo": "a"},
+						},
+					},
+
+					"aws_instance.web.1": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID:         "b",
+							Attributes: map[string]string{"foo": "b"},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	i := &Interpolater{
+		Operation: walkApply,
+		Module:    testModule(t, "interpolate-multi-interp"),
+		State:     state,
+		StateLock: lock,
+	}
+
+	scope := &InterpolationScope{
+		Path: rootModulePath,
+	}
+
+	expected := []interface{}{"a", "b"}
+	testInterpolate(t, i, scope, "aws_instance.web.*.foo",
+		interfaceToVariableSwallowError(expected))
+}
+
 func interfaceToVariableSwallowError(input interface{}) ast.Variable {
 	variable, _ := hil.InterfaceToVariable(input)
 	return variable

terraform/test-fixtures/interpolate-multi-interp/main.tf

@@ -0,0 +1,3 @@
+resource "aws_instance" "web" {
+    count = "${var.c}"
+}