From 347690a73e449e1d5fdbbce2de8d4081837ecf2f Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Tue, 14 Apr 2015 19:42:23 -0500 Subject: [PATCH 1/2] core: don't crash when count.index is used in the wrong context It's bad manners! :) Also adds a validation error up at the configuration layer so the user sees the case from #1528 as an error message. fixes #1528 --- config/config.go | 7 +++++ config/config_test.go | 10 +++++++ .../validate-count-bad-context/main.tf | 6 +++++ terraform/interpolate.go | 3 +++ terraform/interpolate_test.go | 26 +++++++++++++++++++ 5 files changed, 52 insertions(+) create mode 100644 config/test-fixtures/validate-count-bad-context/main.tf diff --git a/config/config.go b/config/config.go index 5814141f8..064bbaf7d 100644 --- a/config/config.go +++ b/config/config.go @@ -472,6 +472,13 @@ func (c *Config) Validate() error { errs = append(errs, fmt.Errorf( "%s: output should only have 'value' field", o.Name)) } + + for _, v := range o.RawConfig.Variables { + if _, ok := v.(*CountVariable); ok { + errs = append(errs, fmt.Errorf( + "%s: count variables are only valid within resources", o.Name)) + } + } } // Check that all variables are in the proper context diff --git a/config/config_test.go b/config/config_test.go index 0503d2e66..871ed0743 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "path/filepath" "reflect" + "strings" "testing" ) @@ -60,6 +61,15 @@ func TestConfigValidate_countInt(t *testing.T) { } } +func TestConfigValidate_countBadContext(t *testing.T) { + c := testConfig(t, "validate-count-bad-context") + expected := "count variables are only valid within resources" + err := c.Validate() + if !strings.Contains(err.Error(), expected) { + t.Fatalf("expected: %q,\nto contain: %q", err, expected) + } +} + func TestConfigValidate_countCountVar(t *testing.T) { c := testConfig(t, "validate-count-count-var") if err := c.Validate(); err == nil { diff --git a/config/test-fixtures/validate-count-bad-context/main.tf b/config/test-fixtures/validate-count-bad-context/main.tf new file mode 100644 index 000000000..3baa98041 --- /dev/null +++ b/config/test-fixtures/validate-count-bad-context/main.tf @@ -0,0 +1,6 @@ +resource "aws_instance" "foo" { +} + +output "notgood" { + value = "${count.index}" +} diff --git a/terraform/interpolate.go b/terraform/interpolate.go index a1e6d37af..aee283a78 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -85,6 +85,9 @@ func (i *Interpolater) valueCountVar( result map[string]ast.Variable) error { switch v.Type { case config.CountValueIndex: + if scope.Resource == nil { + return fmt.Errorf("%s: count.index is only valid within resources", n) + } result[n] = ast.Variable{ Value: scope.Resource.CountIndex, Type: ast.TypeInt, diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 13d56fffb..52896a54b 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -1,6 +1,7 @@ package terraform import ( + "fmt" "os" "reflect" "sync" @@ -24,6 +25,31 @@ func TestInterpolater_countIndex(t *testing.T) { }) } +func TestInterpolater_countIndexInWrongContext(t *testing.T) { + i := &Interpolater{} + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + n := "count.index" + + v, err := config.NewInterpolatedVariable(n) + if err != nil { + t.Fatalf("err: %s", err) + } + + expectedErr := fmt.Errorf("foo: count.index is only valid within resources") + + _, err = i.Values(scope, map[string]config.InterpolatedVariable{ + "foo": v, + }) + + if !reflect.DeepEqual(expectedErr, err) { + t.Fatalf("expected: %#v, got %#v", expectedErr, err) + } +} + func TestInterpolater_moduleVariable(t *testing.T) { lock := new(sync.RWMutex) state := &State{ From 975a96f271d66763d77b6644c367a0f6444274a0 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 15 Apr 2015 10:41:56 -0500 Subject: [PATCH 2/2] core: protect against count.index in modules Modules should get a validation error just like outputs do. refs #1528 --- config/config.go | 8 ++++++++ config/config_test.go | 13 ++++++++++--- .../validate-count-bad-context/main.tf | 7 ++++++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index 064bbaf7d..ed4aaaf88 100644 --- a/config/config.go +++ b/config/config.go @@ -290,6 +290,14 @@ func (c *Config) Validate() error { raw[k] = strVal } + // Check for invalid count variables + for _, v := range m.RawConfig.Variables { + if _, ok := v.(*CountVariable); ok { + errs = append(errs, fmt.Errorf( + "%s: count variables are only valid within resources", m.Name)) + } + } + // Update the raw configuration to only contain the string values m.RawConfig, err = NewRawConfig(raw) if err != nil { diff --git a/config/config_test.go b/config/config_test.go index 871ed0743..3f67bdbf6 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -63,10 +63,17 @@ func TestConfigValidate_countInt(t *testing.T) { func TestConfigValidate_countBadContext(t *testing.T) { c := testConfig(t, "validate-count-bad-context") - expected := "count variables are only valid within resources" + err := c.Validate() - if !strings.Contains(err.Error(), expected) { - t.Fatalf("expected: %q,\nto contain: %q", err, expected) + + expected := []string{ + "no_count_in_output: count variables are only valid within resources", + "no_count_in_module: count variables are only valid within resources", + } + for _, exp := range expected { + if !strings.Contains(err.Error(), exp) { + t.Fatalf("expected: %q,\nto contain: %q", err, exp) + } } } diff --git a/config/test-fixtures/validate-count-bad-context/main.tf b/config/test-fixtures/validate-count-bad-context/main.tf index 3baa98041..2d3833b65 100644 --- a/config/test-fixtures/validate-count-bad-context/main.tf +++ b/config/test-fixtures/validate-count-bad-context/main.tf @@ -1,6 +1,11 @@ resource "aws_instance" "foo" { } -output "notgood" { +output "no_count_in_output" { value = "${count.index}" } + +module "no_count_in_module" { + source = "./child" + somevar = "${count.index}" +}