From 6b458160b931b6ea6ff3536b756337dfb1a5a86e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 8 Dec 2016 23:01:51 -0500 Subject: [PATCH 1/3] config: disallow names starting with ints Fixes #10597 This disallows any names for variables, modules, etc. starting with ints. This causes parse errors with the new HIL parser and actually causes long term ambiguities if we allow this. I've also updated the upgrade guide to note this as a backwards compatibility and how people can fix this going forward. --- config/config.go | 10 +++++++++- config/loader_hcl.go | 5 +++++ config/loader_test.go | 14 ++++++++++++++ config/test-fixtures/var_int.tf | 1 + config/test-fixtures/var_int_bare.tf | 1 + 5 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 config/test-fixtures/var_int.tf create mode 100644 config/test-fixtures/var_int_bare.tf diff --git a/config/config.go b/config/config.go index da981019ca..eb2adb8ab2 100644 --- a/config/config.go +++ b/config/config.go @@ -18,7 +18,7 @@ import ( // NameRegexp is the regular expression that all names (modules, providers, // resources, etc.) must follow. -var NameRegexp = regexp.MustCompile(`\A[A-Za-z0-9\-\_]+\z`) +var NameRegexp = regexp.MustCompile(`(?i)\A[A-Z_][A-Z0-9\-\_]+\z`) // Config is the configuration that comes from loading a collection // of Terraform templates. @@ -282,6 +282,14 @@ func (c *Config) Validate() error { varMap[v.Name] = v } + for k, _ := range varMap { + if !NameRegexp.MatchString(k) { + errs = append(errs, fmt.Errorf( + "variable %q: variable name must match regular expresion %s", + k, NameRegexp)) + } + } + for _, v := range c.Variables { if v.Type() == VariableTypeUnknown { errs = append(errs, fmt.Errorf( diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 0a025913d2..881a983120 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -404,6 +404,11 @@ func loadVariablesHcl(list *ast.ObjectList) ([]*Variable, error) { } n := item.Keys[0].Token.Value().(string) + if !NameRegexp.MatchString(n) { + return nil, fmt.Errorf( + "position %s: 'variable' name must match regular expression: %s", + item.Pos(), NameRegexp) + } /* // TODO: catch extra fields diff --git a/config/loader_test.go b/config/loader_test.go index f7fad33c03..7dae362cd7 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -447,6 +447,20 @@ func TestLoadFile_variables(t *testing.T) { } } +func TestLoadFile_varInt(t *testing.T) { + _, err := LoadFile(filepath.Join(fixtureDir, "var_int.tf")) + if err == nil { + t.Fatal("should have error") + } +} + +func TestLoadFile_varIntBare(t *testing.T) { + _, err := LoadFile(filepath.Join(fixtureDir, "var_int_bare.tf")) + if err == nil { + t.Fatal("should have error") + } +} + func TestLoadDir_basic(t *testing.T) { dir := filepath.Join(fixtureDir, "dir-basic") c, err := LoadDir(dir) diff --git a/config/test-fixtures/var_int.tf b/config/test-fixtures/var_int.tf new file mode 100644 index 0000000000..cd3dca18c5 --- /dev/null +++ b/config/test-fixtures/var_int.tf @@ -0,0 +1 @@ +variable "1x1" {} diff --git a/config/test-fixtures/var_int_bare.tf b/config/test-fixtures/var_int_bare.tf new file mode 100644 index 0000000000..b08e613642 --- /dev/null +++ b/config/test-fixtures/var_int_bare.tf @@ -0,0 +1 @@ +variable 1x1 {} From ef2500932d7d16453802e35cd0c7f5dfcba34c96 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 8 Dec 2016 23:09:18 -0500 Subject: [PATCH 2/3] website: update upgrade guide for number backwards incompat --- .../source/upgrade-guides/0-8.html.markdown | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/website/source/upgrade-guides/0-8.html.markdown b/website/source/upgrade-guides/0-8.html.markdown index c856718d9d..3c16af6d50 100644 --- a/website/source/upgrade-guides/0-8.html.markdown +++ b/website/source/upgrade-guides/0-8.html.markdown @@ -57,6 +57,46 @@ EOF **Action:** Use heredocs or escape sequences when you have a string with newlines. +## Names Cannot Start with Integers or Hyphens + +Names of variables, resources, modules, etc. may no longer start with +numbers or hyphens. These will now fail at the validation step. + +This change was necessary to remove ambiguities from parsing the +interpolations. Most languages do not allow identifiers starting with +these characters with good reason. We now follow that as well. + +An example of a configuration that no longer works: + +``` +variable "1x1" {} +``` + +This must now be changed to start with a letter or underscore: + +``` +variable "foo-1x1" {} +variable "_1x1" {} +``` + +And so on... + +If you're changing resource names, this can cause Terraform to consider it +a new resource and plan to destroy it. To work around these scenarios, +please use the `terraform state mv` command: + +``` +terraform state mv aws_instance.1x1 aws_instance.foo-1x1 +``` + +The `terraform state mv` command creates a backup for each operation, +but to be safe you can still back up your state prior to doing these +changes. + +**Action:** Rename variable, resources, modules, providers, outputs that +start with a number or hyphen. If you rename a resource or module, use +`terraform state mv` to fix the state. + ## Math Order of Operations Math operations now follow standard mathematical order of operations. From 8b9e2c17cc0210b43e5bde6b7eec3e8113524fb5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 8 Dec 2016 23:13:19 -0500 Subject: [PATCH 3/3] config: fix NameRegexp validation to allow single-char names --- config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index eb2adb8ab2..da39051c19 100644 --- a/config/config.go +++ b/config/config.go @@ -18,7 +18,7 @@ import ( // NameRegexp is the regular expression that all names (modules, providers, // resources, etc.) must follow. -var NameRegexp = regexp.MustCompile(`(?i)\A[A-Z_][A-Z0-9\-\_]+\z`) +var NameRegexp = regexp.MustCompile(`(?i)\A[A-Z_][A-Z0-9\-\_]*\z`) // Config is the configuration that comes from loading a collection // of Terraform templates.