terraform: multi-var ordering is by count

Fixes #9444

This appears to be a regression from 0.7.0, but there were no tests
covering it so we missed it and changed the behavior at some point! Oh
no.

This PR make the ordering of multi-var access: `resource.name.*.attr`
consistent: it is the ordering of the count, not the lexical ordering of
the value. This allows behavior where two lists are indexed by count
index and can be assumed to be related (for example user data for an aws
instance, as shown in the above referenced issue).

Two new context tests added to cover this case.

terraform/context_apply_test.go

@@ -2039,6 +2039,74 @@ func TestContext2Apply_multiVar(t *testing.T) {
 	}
 }
 
+// Test that multi-var (splat) access is ordered by count, not by
+// value.
+func TestContext2Apply_multiVarOrder(t *testing.T) {
+	m := testModule(t, "apply-multi-var-order")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	// First, apply with a count of 3
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	t.Logf("State: %s", state.String())
+
+	actual := state.RootModule().Outputs["should-be-11"]
+	expected := "index-11"
+	if actual == nil || actual.Value != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
+// Test that multi-var (splat) access is ordered by count, not by
+// value, through interpolations.
+func TestContext2Apply_multiVarOrderInterp(t *testing.T) {
+	m := testModule(t, "apply-multi-var-order-interp")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	// First, apply with a count of 3
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	t.Logf("State: %s", state.String())
+
+	actual := state.RootModule().Outputs["should-be-11"]
+	expected := "baz-index-11"
+	if actual == nil || actual.Value != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
 func TestContext2Apply_nilDiff(t *testing.T) {
 	m := testModule(t, "apply-good")
 	p := testProvider("aws")

terraform/interpolate.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -491,13 +492,13 @@ func (i *Interpolater) computeResourceMultiVariable(
 	}
 
 	// Get the keys for all the resources that are created for this resource
-	resourceKeys, err := i.resourceCountKeys(module, cr, v)
+	countMax, err := i.resourceCountMax(module, cr, v)
 	if err != nil {
 		return nil, err
 	}
 
 	// If count is zero, we return an empty list
-	if len(resourceKeys) == 0 {
+	if countMax == 0 {
 		return &ast.Variable{Type: ast.TypeList, Value: []ast.Variable{}}, nil
 	}
 
@@ -507,7 +508,9 @@ func (i *Interpolater) computeResourceMultiVariable(
 	}
 
 	var values []interface{}
-	for _, id := range resourceKeys {
+	for idx := 0; idx < countMax; idx++ {
+		id := fmt.Sprintf("%s.%d", v.ResourceId(), idx)
+
 		// 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.
@@ -678,10 +681,10 @@ func (i *Interpolater) resourceVariableInfo(
 	return module, cr, nil
 }
 
-func (i *Interpolater) resourceCountKeys(
+func (i *Interpolater) resourceCountMax(
 	ms *ModuleState,
 	cr *config.Resource,
-	v *config.ResourceVariable) ([]string, error) {
+	v *config.ResourceVariable) (int, error) {
 	id := v.ResourceId()
 
 	// If we're NOT applying, then we assume we can read the count
@@ -690,31 +693,58 @@ func (i *Interpolater) resourceCountKeys(
 	if i.Operation != walkApply {
 		count, err := cr.Count()
 		if err != nil {
-			return nil, err
+			return 0, err
 		}
 
-		result := make([]string, count)
+		return count, nil
-		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
+	max := -1
 	for k, _ := range ms.Resources {
-		// If we don't have the right prefix then ignore it
+		// Get the index number for this resource
-		if k != id && !strings.HasPrefix(k, id+".") {
+		index := ""
+		if k == id {
+			// If the key is the id, then its just 0 (no explicit index)
+			index = "0"
+		} else if strings.HasPrefix(k, id+".") {
+			// Grab the index number out of the state
+			index = k[len(id+"."):]
+			if idx := strings.IndexRune(index, '.'); idx >= 0 {
+				index = index[:idx]
+			}
+		}
+
+		// If there was no index then this resource didn't match
+		// the one we're looking for, exit.
+		if index == "" {
 			continue
 		}
 
-		// Add it to the list
+		// Turn the index into an int
-		resourceKeys = append(resourceKeys, k)
+		raw, err := strconv.ParseInt(index, 0, 0)
+		if err != nil {
+			return 0, fmt.Errorf(
+				"%s: error parsing index %q as int: %s",
+				id, index, err)
+		}
+
+		// Keep track of this index if its the max
+		if new := int(raw); new > max {
+			max = new
+		}
 	}
-	sort.Strings(resourceKeys)
+
-	return resourceKeys, nil
+	// If we never found any matching resources in the state, we
+	// have zero.
+	if max == -1 {
+		return 0, nil
+	}
+
+	// The result value is "max+1" because we're returning the
+	// max COUNT, not the max INDEX, and we zero-index.
+	return max + 1, nil
 }

terraform/test-fixtures/apply-multi-var-order-interp/main.tf

@@ -0,0 +1,15 @@
+variable "count" { default = 15 }
+
+resource "aws_instance" "bar" {
+    count = "${var.count}"
+    foo = "index-${count.index}"
+}
+
+resource "aws_instance" "baz" {
+    count = "${var.count}"
+    foo = "baz-${element(aws_instance.bar.*.foo, count.index)}"
+}
+
+output "should-be-11" {
+    value = "${element(aws_instance.baz.*.foo, 11)}"
+}

terraform/test-fixtures/apply-multi-var-order/main.tf

@@ -0,0 +1,10 @@
+variable "count" { default = 15 }
+
+resource "aws_instance" "bar" {
+    count = "${var.count}"
+    foo = "index-${count.index}"
+}
+
+output "should-be-11" {
+    value = "${element(aws_instance.bar.*.foo, 11)}"
+}