Merge pull request #9883 from hashicorp/b-multi-order

terraform: multi-var ordering is by count

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
-		}
-
-		result := make([]string, count)
-		for i := 0; i < count; i++ {
-			result[i] = fmt.Sprintf("%s.%d", id, i)
+			return 0, err
 		}
 
-		return result, nil
+		return count, 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
-		if k != id && !strings.HasPrefix(k, id+".") {
+		// Get the index number for this resource
+		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
-		resourceKeys = append(resourceKeys, k)
+		// Turn the index into an int
+		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)}"
+}