From 728a1e54480030889383f3dd06435197b7bc54d7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 13 Oct 2016 17:57:11 -0700 Subject: [PATCH] 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 | 68 +++++++++++++++---- terraform/interpolate_test.go | 43 ++++++++++++ .../interpolate-multi-interp/main.tf | 3 + 3 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 terraform/test-fixtures/interpolate-multi-interp/main.tf diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 2117fc3f0..7038c1bc8 100644 --- a/terraform/interpolate.go +++ b/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 +} diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 65d716c2e..b13185b42 100644 --- a/terraform/interpolate_test.go +++ b/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 diff --git a/terraform/test-fixtures/interpolate-multi-interp/main.tf b/terraform/test-fixtures/interpolate-multi-interp/main.tf new file mode 100644 index 000000000..1d475e2ee --- /dev/null +++ b/terraform/test-fixtures/interpolate-multi-interp/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "web" { + count = "${var.c}" +}