From 50e5eacc15ec3200be7aa8beefcf43cad1b72bbf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 15:06:04 -0700 Subject: [PATCH 1/5] config: add NameRegexp --- config/config.go | 5 +++++ config/config_test.go | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/config/config.go b/config/config.go index 9d629f5783..273492cb26 100644 --- a/config/config.go +++ b/config/config.go @@ -4,6 +4,7 @@ package config import ( "fmt" + "regexp" "strconv" "strings" @@ -13,6 +14,10 @@ import ( "github.com/mitchellh/reflectwalk" ) +// NameRegexp is the regular expression that all names (modules, providers, +// resources, etc.) must follow. +var NameRegexp = regexp.MustCompile(`\A[A-Za-z0-9\-\_]+\z`) + // Config is the configuration that comes from loading a collection // of Terraform templates. type Config struct { diff --git a/config/config_test.go b/config/config_test.go index 0ea8c28928..b17a32db91 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -228,6 +228,26 @@ func TestConfigValidate_varModuleInvalid(t *testing.T) { } } +func TestNameRegexp(t *testing.T) { + cases := []struct{ + Input string + Match bool + }{ + {"hello", true}, + {"foo-bar", true}, + {"foo_bar", true}, + {"_hello", true}, + {"foo bar", false}, + {"foo.bar", false}, + } + + for _, tc := range cases { + if NameRegexp.Match([]byte(tc.Input)) != tc.Match { + t.Fatalf("Input: %s\n\nExpected: %#v", tc.Input, tc.Match) + } + } +} + func TestProviderConfigName(t *testing.T) { pcs := []*ProviderConfig{ &ProviderConfig{Name: "aw"}, From 36f4a644b69a2ed684314b438176b9176de61662 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 15:59:50 -0700 Subject: [PATCH 2/5] terraform: warn if the name has special characters --- terraform/context.go | 10 ++++++++++ terraform/context_test.go | 19 +++++++++++++++++++ .../validate-resource-name-symbol/main.tf | 3 +++ 3 files changed, 32 insertions(+) create mode 100644 terraform/test-fixtures/validate-resource-name-symbol/main.tf diff --git a/terraform/context.go b/terraform/context.go index 0f24735688..b8fd909f6e 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1049,6 +1049,16 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { return nil } + // If the resouce name doesn't match the name regular + // expression, show a warning. + if !config.NameRegexp.Match([]byte(rn.Config.Name)) { + l.Lock() + meta.Warns = append(meta.Warns, fmt.Sprintf( + "'%s' warning: name can't contain special characters", + rn.Resource.Id)) + l.Unlock() + } + log.Printf("[INFO] Validating resource: %s", rn.Resource.Id) ws, es := rn.Resource.Provider.ValidateResource( rn.Resource.Info.Type, rn.Resource.Config) diff --git a/terraform/context_test.go b/terraform/context_test.go index 11b0e4e7b8..857b923abe 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -312,6 +312,25 @@ func TestContextValidate_resourceConfig_good(t *testing.T) { } } +func TestContextValidate_resourceNameSymbol(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "validate-resource-name-symbol") + c := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + w, e := c.Validate() + if len(w) == 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) > 0 { + t.Fatalf("bad: %#v", e) + } +} + func TestContextValidate_requiredVar(t *testing.T) { m := testModule(t, "validate-required-var") c := testContext(t, &ContextOpts{ diff --git a/terraform/test-fixtures/validate-resource-name-symbol/main.tf b/terraform/test-fixtures/validate-resource-name-symbol/main.tf new file mode 100644 index 0000000000..e89401f7cc --- /dev/null +++ b/terraform/test-fixtures/validate-resource-name-symbol/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "foo bar" { + num = "2" +} From 9ed89dbabdbd78961e64d1867d13647713b0fd0b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 16:01:13 -0700 Subject: [PATCH 3/5] terraform: make wording better --- terraform/context.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index b8fd909f6e..30e2b3e019 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1054,7 +1054,8 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { if !config.NameRegexp.Match([]byte(rn.Config.Name)) { l.Lock() meta.Warns = append(meta.Warns, fmt.Sprintf( - "'%s' warning: name can't contain special characters", + "'%s' warning: name can't contain special characters.\n" + + "this will be an error in Terraform 0.4", rn.Resource.Id)) l.Unlock() } From 67d9188a2907495c90a7c2c9b66284a66a549163 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 16:03:22 -0700 Subject: [PATCH 4/5] config: validate module names are valid --- config/config.go | 11 +++++++++-- config/config_test.go | 7 +++++++ config/test-fixtures/validate-module-name-bad/main.tf | 3 +++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 config/test-fixtures/validate-module-name-bad/main.tf diff --git a/config/config.go b/config/config.go index 273492cb26..6827d58879 100644 --- a/config/config.go +++ b/config/config.go @@ -241,9 +241,9 @@ func (c *Config) Validate() error { } } - // If we haven't seen this module before, check that the - // source has no interpolations. if _, ok := modules[m.Id()]; !ok { + // If we haven't seen this module before, check that the + // source has no interpolations. rc, err := NewRawConfig(map[string]interface{}{ "root": m.Source, }) @@ -256,6 +256,13 @@ func (c *Config) Validate() error { "%s: module source cannot contain interpolations", m.Id())) } + + // Check that the name matches our regexp + if !NameRegexp.Match([]byte(m.Name)) { + errs = append(errs, fmt.Errorf( + "%s: module name cannot contain special characters", + m.Id())) + } } modules[m.Id()] = m diff --git a/config/config_test.go b/config/config_test.go index b17a32db91..ef93e9792b 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -123,6 +123,13 @@ func TestConfigValidate_dupResource(t *testing.T) { } } +func TestConfigValidate_moduleNameBad(t *testing.T) { + c := testConfig(t, "validate-module-name-bad") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_moduleSourceVar(t *testing.T) { c := testConfig(t, "validate-module-source-var") if err := c.Validate(); err == nil { diff --git a/config/test-fixtures/validate-module-name-bad/main.tf b/config/test-fixtures/validate-module-name-bad/main.tf new file mode 100644 index 0000000000..5b3bbcb4e8 --- /dev/null +++ b/config/test-fixtures/validate-module-name-bad/main.tf @@ -0,0 +1,3 @@ +module "foo bar" { + source = "foo" +} From 9dd7618fce1ce093abc0e03d4ca56e20360a5a2e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 16:12:53 -0700 Subject: [PATCH 5/5] config: fix messaging for name symbols --- config/config.go | 3 ++- terraform/context.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index 6827d58879..5de8114876 100644 --- a/config/config.go +++ b/config/config.go @@ -260,7 +260,8 @@ func (c *Config) Validate() error { // Check that the name matches our regexp if !NameRegexp.Match([]byte(m.Name)) { errs = append(errs, fmt.Errorf( - "%s: module name cannot contain special characters", + "%s: module name can only contain letters, numbers, "+ + "dashes, and underscores", m.Id())) } } diff --git a/terraform/context.go b/terraform/context.go index 30e2b3e019..7f1502fa1e 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1054,8 +1054,9 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { if !config.NameRegexp.Match([]byte(rn.Config.Name)) { l.Lock() meta.Warns = append(meta.Warns, fmt.Sprintf( - "'%s' warning: name can't contain special characters.\n" + - "this will be an error in Terraform 0.4", + "%s: module name can only contain letters, numbers, "+ + "dashes, and underscores.\n"+ + "This will be an error in Terraform 0.4", rn.Resource.Id)) l.Unlock() }