From 7d78427d7b452c9023e5e6cc174439956568b54f Mon Sep 17 00:00:00 2001 From: DustinChaloupka Date: Fri, 17 Apr 2015 11:06:52 -0500 Subject: [PATCH 1/4] setting up a failing test for a resource with a set of attributes that does not return all of them as a list --- terraform/interpolate_test.go | 48 +++++++++++++++++++ .../resource-multi-attribute/main.tf | 17 +++++++ 2 files changed, 65 insertions(+) create mode 100644 terraform/test-fixtures/resource-multi-attribute/main.tf diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 52896a54b8b6..95f403086d42 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -136,6 +136,53 @@ func TestInterpolater_pathRoot(t *testing.T) { }) } +func TestInterpolater_multiAttribute(t *testing.T) { + lock := new(sync.RWMutex) + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "resource.name": &ResourceState{ + Type: "resource", + Primary: &InstanceState{ + ID: "qux", + Attributes: map[string]string{ + "foo.#": "4", + "foo.298374.bar": "baz1", + "foo.233489.bar": "baz2", + "foo.872348.bar": "baz3", + "foo.348573.bar": "baz4", + }, + }, + }, + }, + }, + }, + } + + mod := testModule(t, "resource-multi-attribute") + i := &Interpolater{ + Module: mod, + State: state, + StateLock: lock, + } + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + testInterpolate(t, i, scope, "resource.name.foo.#", ast.Variable{ + Value: "4", + Type: ast.TypeString, + }) + + testInterpolate(t, i, scope, "resource.name.foo.*.bar", ast.Variable{ + Value: []string{"baz1", "baz2", "baz3", "baz4"}, + Type: ast.TypeAny, + }) +} + func testInterpolate( t *testing.T, i *Interpolater, scope *InterpolationScope, @@ -155,6 +202,7 @@ func testInterpolate( expected := map[string]ast.Variable{ "foo": expectedVar, } + if !reflect.DeepEqual(actual, expected) { t.Fatalf("bad: %#v", actual) } diff --git a/terraform/test-fixtures/resource-multi-attribute/main.tf b/terraform/test-fixtures/resource-multi-attribute/main.tf new file mode 100644 index 000000000000..626bbb7a4230 --- /dev/null +++ b/terraform/test-fixtures/resource-multi-attribute/main.tf @@ -0,0 +1,17 @@ +resource "resource" "name" { + foo { + bar = "baz1" + } + + foo { + bar = "baz2" + } + + foo { + bar = "baz3" + } + + foo { + bar = "baz4" + } +} From e1a2316498ae259ef242c651fbd3b817655a9e19 Mon Sep 17 00:00:00 2001 From: DustinChaloupka Date: Fri, 17 Apr 2015 15:57:11 -0500 Subject: [PATCH 2/4] adding more tests and a fix --- terraform/interpolate.go | 56 +++++++++++++++++-- terraform/interpolate_test.go | 30 +++++++--- .../resource-multi-attribute/main.tf | 12 ++-- 3 files changed, 81 insertions(+), 17 deletions(-) diff --git a/terraform/interpolate.go b/terraform/interpolate.go index aee283a78c01..0814e431d820 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -5,6 +5,8 @@ import ( "os" "strings" "sync" + "strconv" + "sort" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/lang/ast" @@ -324,6 +326,27 @@ func (i *Interpolater) computeResourceVariable( return attr, nil } + // Check sets and lists for attributes + if parts := strings.Split(v.Field, "."); len(parts) > 1 { + for i := 1; i < len(parts); i++ { + + // Sets and lists have an attribute that is their size + // so use that to check if it has been computed or not yet + key := fmt.Sprintf("%s.#", strings.Join(parts[:i], ".")) + if attr, ok := r.Primary.Attributes[key]; ok { + + // If the number of instances has not been computed yet, don't try + // to find the values + if _, err := strconv.ParseInt(attr, 0, 0); err != nil { + return attr, nil + } + + values := computeValuesFromParts(parts, r.Primary.Attributes) + return strings.Join(values, config.InterpSplitDelim), nil + } + } + } + // At apply time, we can't do the "maybe has it" check below // that we need for plans since parent elements might be computed. // Therefore, it is an error and we're missing the key. @@ -341,14 +364,9 @@ func (i *Interpolater) computeResourceVariable( // a computed list. If so, then the whole thing is computed. if parts := strings.Split(v.Field, "."); len(parts) > 1 { for i := 1; i < len(parts); i++ { - // Lists and sets make this - key := fmt.Sprintf("%s.#", strings.Join(parts[:i], ".")) - if attr, ok := r.Primary.Attributes[key]; ok { - return attr, nil - } // Maps make this - key = fmt.Sprintf("%s", strings.Join(parts[:i], ".")) + key := fmt.Sprintf("%s", strings.Join(parts[:i], ".")) if attr, ok := r.Primary.Attributes[key]; ok { return attr, nil } @@ -372,6 +390,32 @@ MISSING: v.FullKey()) } +func computeValuesFromParts(parts []string, attributes map[string]string) []string { + var values []string + + attributeName := parts[0] + attributeIndex := parts[1] + nestedAttribute := parts[2] + + for attribute, value := range attributes { + if attributeParts := strings.Split(attribute, "."); attributeParts[0] == attributeName && attributeParts[1] != "#" && attributeParts[2] == nestedAttribute { + values = append(values, value) + } + } + + sort.StringSlice(values).Sort() + + if index, err := strconv.ParseInt(attributeIndex, 0, 0); err == nil { + if int(index) < len(values) { + values = []string{values[int(index)]} + } else { + values = []string{} + } + } + + return values +} + func (i *Interpolater) computeResourceMultiVariable( scope *InterpolationScope, v *config.ResourceVariable) (string, error) { diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 95f403086d42..141a96063c2f 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -6,6 +6,7 @@ import ( "reflect" "sync" "testing" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/lang/ast" @@ -149,10 +150,14 @@ func TestInterpolater_multiAttribute(t *testing.T) { ID: "qux", Attributes: map[string]string{ "foo.#": "4", - "foo.298374.bar": "baz1", - "foo.233489.bar": "baz2", - "foo.872348.bar": "baz3", - "foo.348573.bar": "baz4", + "foo.298374.bar1": "baz1", + "foo.298374.bar2": "baz12", + "foo.233489.bar1": "baz2", + "foo.233489.bar2": "baz22", + "foo.872348.bar1": "baz3", + "foo.872348.bar2": "baz32", + "foo.348573.bar1": "baz4", + "foo.348573.bar2": "baz42", }, }, }, @@ -177,9 +182,20 @@ func TestInterpolater_multiAttribute(t *testing.T) { Type: ast.TypeString, }) - testInterpolate(t, i, scope, "resource.name.foo.*.bar", ast.Variable{ - Value: []string{"baz1", "baz2", "baz3", "baz4"}, - Type: ast.TypeAny, + expectedValues := []string{"baz1", "baz2", "baz3", "baz4"} + testInterpolate(t, i, scope, "resource.name.foo.*.bar1", ast.Variable{ + Value: strings.Join(expectedValues, config.InterpSplitDelim), + Type: ast.TypeString, + }) + + testInterpolate(t, i, scope, "resource.name.foo.0.bar1", ast.Variable{ + Value: "baz1", + Type: ast.TypeString, + }) + + testInterpolate(t, i, scope, "resource.name.foo.4.bar1", ast.Variable{ + Value: "", + Type: ast.TypeString, }) } diff --git a/terraform/test-fixtures/resource-multi-attribute/main.tf b/terraform/test-fixtures/resource-multi-attribute/main.tf index 626bbb7a4230..1f72326cc63d 100644 --- a/terraform/test-fixtures/resource-multi-attribute/main.tf +++ b/terraform/test-fixtures/resource-multi-attribute/main.tf @@ -1,17 +1,21 @@ resource "resource" "name" { foo { - bar = "baz1" + bar1 = "baz1" + bar2 = "baz12" } foo { - bar = "baz2" + bar1 = "baz2" + bar2 = "baz22" } foo { - bar = "baz3" + bar1 = "baz3" + bar2 = "baz32" } foo { - bar = "baz4" + bar1 = "baz4" + bar2 = "baz42" } } From 3ac5f3b9eea69737816b74febb248649e9dac239 Mon Sep 17 00:00:00 2001 From: DustinChaloupka Date: Mon, 20 Apr 2015 10:04:18 -0500 Subject: [PATCH 3/4] adding a list test --- terraform/interpolate.go | 9 ++- terraform/interpolate_test.go | 58 ++++++++++++++++++- .../resource-multi-attribute-list/main.tf | 3 + .../main.tf | 0 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 terraform/test-fixtures/resource-multi-attribute-list/main.tf rename terraform/test-fixtures/{resource-multi-attribute => resource-multi-attribute-set}/main.tf (100%) diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 0814e431d820..9b48ebb19159 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -395,11 +395,14 @@ func computeValuesFromParts(parts []string, attributes map[string]string) []stri attributeName := parts[0] attributeIndex := parts[1] - nestedAttribute := parts[2] for attribute, value := range attributes { - if attributeParts := strings.Split(attribute, "."); attributeParts[0] == attributeName && attributeParts[1] != "#" && attributeParts[2] == nestedAttribute { - values = append(values, value) + if attributeParts := strings.Split(attribute, "."); attributeParts[0] == attributeName && attributeParts[1] != "#" { + if len(parts) <= 2 { + values = append(values, value) + } else if nestedAttribute := parts[2]; attributeParts[2] == nestedAttribute { + values = append(values, value) + } } } diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 141a96063c2f..73489b70186c 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -137,7 +137,7 @@ func TestInterpolater_pathRoot(t *testing.T) { }) } -func TestInterpolater_multiAttribute(t *testing.T) { +func TestInterpolater_multiAttributeSet(t *testing.T) { lock := new(sync.RWMutex) state := &State{ Modules: []*ModuleState{ @@ -166,7 +166,7 @@ func TestInterpolater_multiAttribute(t *testing.T) { }, } - mod := testModule(t, "resource-multi-attribute") + mod := testModule(t, "resource-multi-attribute-set") i := &Interpolater{ Module: mod, State: state, @@ -188,6 +188,12 @@ func TestInterpolater_multiAttribute(t *testing.T) { Type: ast.TypeString, }) + expectedValues = []string{"baz12", "baz22", "baz32", "baz42"} + testInterpolate(t, i, scope, "resource.name.foo.*.bar2", ast.Variable{ + Value: strings.Join(expectedValues, config.InterpSplitDelim), + Type: ast.TypeString, + }) + testInterpolate(t, i, scope, "resource.name.foo.0.bar1", ast.Variable{ Value: "baz1", Type: ast.TypeString, @@ -199,6 +205,54 @@ func TestInterpolater_multiAttribute(t *testing.T) { }) } +func TestInterpolater_multiAttributeList(t *testing.T) { + lock := new(sync.RWMutex) + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "resource.name": &ResourceState{ + Type: "resource", + Primary: &InstanceState{ + ID: "qux", + Attributes: map[string]string{ + "foo.#": "4", + "foo.298374": "bar1", + "foo.233489": "bar2", + "foo.872348": "bar3", + "foo.348573": "bar4", + }, + }, + }, + }, + }, + }, + } + + mod := testModule(t, "resource-multi-attribute-list") + i := &Interpolater{ + Module: mod, + State: state, + StateLock: lock, + } + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + testInterpolate(t, i, scope, "resource.name.foo.#", ast.Variable{ + Value: "4", + Type: ast.TypeString, + }) + + expectedValues := []string{"bar1", "bar2", "bar3", "bar4"} + testInterpolate(t, i, scope, "resource.name.foo.*", ast.Variable{ + Value: strings.Join(expectedValues, config.InterpSplitDelim), + Type: ast.TypeString, + }) +} + func testInterpolate( t *testing.T, i *Interpolater, scope *InterpolationScope, diff --git a/terraform/test-fixtures/resource-multi-attribute-list/main.tf b/terraform/test-fixtures/resource-multi-attribute-list/main.tf new file mode 100644 index 000000000000..17c5fdeffc72 --- /dev/null +++ b/terraform/test-fixtures/resource-multi-attribute-list/main.tf @@ -0,0 +1,3 @@ +resource "resource" "name" { + foo = [ "bar1", "bar2", "bar3", "bar4" ] +} diff --git a/terraform/test-fixtures/resource-multi-attribute/main.tf b/terraform/test-fixtures/resource-multi-attribute-set/main.tf similarity index 100% rename from terraform/test-fixtures/resource-multi-attribute/main.tf rename to terraform/test-fixtures/resource-multi-attribute-set/main.tf From 69df6c13ed44debdb1dc40f2933d31952d03ae82 Mon Sep 17 00:00:00 2001 From: DustinChaloupka Date: Mon, 20 Apr 2015 14:41:21 -0500 Subject: [PATCH 4/4] better naming --- terraform/interpolate_test.go | 8 ++++---- .../main.tf | 0 .../main.tf | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename terraform/test-fixtures/{resource-multi-attribute-list => resource-computed-list}/main.tf (100%) rename terraform/test-fixtures/{resource-multi-attribute-set => resource-computed-set}/main.tf (100%) diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 73489b70186c..723646e3ca34 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -137,7 +137,7 @@ func TestInterpolater_pathRoot(t *testing.T) { }) } -func TestInterpolater_multiAttributeSet(t *testing.T) { +func TestInterpolater_computedSet(t *testing.T) { lock := new(sync.RWMutex) state := &State{ Modules: []*ModuleState{ @@ -166,7 +166,7 @@ func TestInterpolater_multiAttributeSet(t *testing.T) { }, } - mod := testModule(t, "resource-multi-attribute-set") + mod := testModule(t, "resource-computed-set") i := &Interpolater{ Module: mod, State: state, @@ -205,7 +205,7 @@ func TestInterpolater_multiAttributeSet(t *testing.T) { }) } -func TestInterpolater_multiAttributeList(t *testing.T) { +func TestInterpolater_computedList(t *testing.T) { lock := new(sync.RWMutex) state := &State{ Modules: []*ModuleState{ @@ -230,7 +230,7 @@ func TestInterpolater_multiAttributeList(t *testing.T) { }, } - mod := testModule(t, "resource-multi-attribute-list") + mod := testModule(t, "resource-computed-list") i := &Interpolater{ Module: mod, State: state, diff --git a/terraform/test-fixtures/resource-multi-attribute-list/main.tf b/terraform/test-fixtures/resource-computed-list/main.tf similarity index 100% rename from terraform/test-fixtures/resource-multi-attribute-list/main.tf rename to terraform/test-fixtures/resource-computed-list/main.tf diff --git a/terraform/test-fixtures/resource-multi-attribute-set/main.tf b/terraform/test-fixtures/resource-computed-set/main.tf similarity index 100% rename from terraform/test-fixtures/resource-multi-attribute-set/main.tf rename to terraform/test-fixtures/resource-computed-set/main.tf