diff --git a/config/loader_hcl.go b/config/loader_hcl.go index a40ad5ba7..9abb1960f 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -327,6 +327,10 @@ func loadAtlasHcl(list *ast.ObjectList) (*AtlasConfig, error) { // represents exactly one module definition in the HCL configuration. // We leave it up to another pass to merge them together. func loadModulesHcl(list *ast.ObjectList) ([]*Module, error) { + if err := assertAllBlocksHaveNames("module", list); err != nil { + return nil, err + } + list = list.Children() if len(list.Items) == 0 { return nil, nil @@ -391,12 +395,12 @@ func loadModulesHcl(list *ast.ObjectList) ([]*Module, error) { // LoadOutputsHcl recurses into the given HCL object and turns // it into a mapping of outputs. func loadOutputsHcl(list *ast.ObjectList) ([]*Output, error) { - list = list.Children() - if len(list.Items) == 0 { - return nil, fmt.Errorf( - "'output' must be followed by exactly one string: a name") + if err := assertAllBlocksHaveNames("output", list); err != nil { + return nil, err } + list = list.Children() + // Go through each object and turn it into an actual result. result := make([]*Output, 0, len(list.Items)) for _, item := range list.Items { @@ -450,12 +454,12 @@ func loadOutputsHcl(list *ast.ObjectList) ([]*Output, error) { // LoadVariablesHcl recurses into the given HCL object and turns // it into a list of variables. func loadVariablesHcl(list *ast.ObjectList) ([]*Variable, error) { - list = list.Children() - if len(list.Items) == 0 { - return nil, fmt.Errorf( - "'variable' must be followed by exactly one strings: a name") + if err := assertAllBlocksHaveNames("variable", list); err != nil { + return nil, err } + list = list.Children() + // hclVariable is the structure each variable is decoded into type hclVariable struct { DeclaredType string `hcl:"type"` @@ -531,6 +535,10 @@ func loadVariablesHcl(list *ast.ObjectList) ([]*Variable, error) { // LoadProvidersHcl recurses into the given HCL object and turns // it into a mapping of provider configs. func loadProvidersHcl(list *ast.ObjectList) ([]*ProviderConfig, error) { + if err := assertAllBlocksHaveNames("provider", list); err != nil { + return nil, err + } + list = list.Children() if len(list.Items) == 0 { return nil, nil @@ -592,6 +600,10 @@ func loadProvidersHcl(list *ast.ObjectList) ([]*ProviderConfig, error) { // represents exactly one data definition in the HCL configuration. // We leave it up to another pass to merge them together. func loadDataResourcesHcl(list *ast.ObjectList) ([]*Resource, error) { + if err := assertAllBlocksHaveNames("data", list); err != nil { + return nil, err + } + list = list.Children() if len(list.Items) == 0 { return nil, nil @@ -901,6 +913,10 @@ func loadManagedResourcesHcl(list *ast.ObjectList) ([]*Resource, error) { } func loadProvisionersHcl(list *ast.ObjectList, connInfo map[string]interface{}) ([]*Provisioner, error) { + if err := assertAllBlocksHaveNames("provisioner", list); err != nil { + return nil, err + } + list = list.Children() if len(list.Items) == 0 { return nil, nil @@ -1023,6 +1039,29 @@ func hclObjectMap(os *hclobj.Object) map[string]ast.ListNode { } */ +// assertAllBlocksHaveNames returns an error if any of the items in +// the given object list are blocks without keys (like "module {}") +// or simple assignments (like "module = 1"). It returns nil if +// neither of these things are true. +// +// The given name is used in any generated error messages, and should +// be the name of the block we're dealing with. The given list should +// be the result of calling .Filter on an object list with that same +// name. +func assertAllBlocksHaveNames(name string, list *ast.ObjectList) error { + if elem := list.Elem(); len(elem.Items) != 0 { + switch et := elem.Items[0].Val.(type) { + case *ast.ObjectType: + pos := et.Lbrace + return fmt.Errorf("%s: %q must be followed by a name", pos, name) + default: + pos := elem.Items[0].Val.Pos() + return fmt.Errorf("%s: %q must be a configuration block", pos, name) + } + } + return nil +} + func checkHCLKeys(node ast.Node, valid []string) error { var list *ast.ObjectList switch n := node.(type) { diff --git a/config/loader_test.go b/config/loader_test.go index a2a2929f2..a3aeb7321 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -314,6 +314,18 @@ func TestLoadFileBasic_modules(t *testing.T) { } } +func TestLoadFile_unnamedModule(t *testing.T) { + _, err := LoadFile(filepath.Join(fixtureDir, "module-unnamed.tf")) + if err == nil { + t.Fatalf("bad: expected error") + } + + errorStr := err.Error() + if !strings.Contains(errorStr, `"module" must be followed`) { + t.Fatalf("bad: expected error has wrong text: %s", errorStr) + } +} + func TestLoadFile_outputDependsOn(t *testing.T) { c, err := LoadFile(filepath.Join(fixtureDir, "output-depends-on.tf")) if err != nil { @@ -696,7 +708,7 @@ func TestLoadFile_variableNoName(t *testing.T) { } errorStr := err.Error() - if !strings.Contains(errorStr, "'variable' must be followed") { + if !strings.Contains(errorStr, `"variable" must be followed`) { t.Fatalf("bad: expected error has wrong text: %s", errorStr) } } @@ -740,7 +752,7 @@ func TestLoadFile_unnamedOutput(t *testing.T) { } errorStr := err.Error() - if !strings.Contains(errorStr, "'output' must be followed") { + if !strings.Contains(errorStr, `"output" must be followed`) { t.Fatalf("bad: expected error has wrong text: %s", errorStr) } } diff --git a/config/test-fixtures/module-unnamed.tf b/config/test-fixtures/module-unnamed.tf new file mode 100644 index 000000000..e285519bf --- /dev/null +++ b/config/test-fixtures/module-unnamed.tf @@ -0,0 +1,7 @@ +module "okay" { + source = "./okay" +} + +module { + source = "./not-okay" +} diff --git a/config/test-fixtures/output-unnamed.tf b/config/test-fixtures/output-unnamed.tf index 7e7529153..7ef8ebe1b 100644 --- a/config/test-fixtures/output-unnamed.tf +++ b/config/test-fixtures/output-unnamed.tf @@ -1,3 +1,7 @@ +output "okay" { + value = "bar" +} + output { value = "foo" } diff --git a/config/test-fixtures/variable-no-name.tf b/config/test-fixtures/variable-no-name.tf index f3856886f..7f09d1e64 100644 --- a/config/test-fixtures/variable-no-name.tf +++ b/config/test-fixtures/variable-no-name.tf @@ -1,3 +1,6 @@ +variable "okay" { +} + variable { name = "test" default = "test_value"