diff --git a/config/module/test-fixtures/validate-required-var/child/main.tf b/config/module/test-fixtures/validate-required-var/child/main.tf index 618ae3c42..00b6c4e5b 100644 --- a/config/module/test-fixtures/validate-required-var/child/main.tf +++ b/config/module/test-fixtures/validate-required-var/child/main.tf @@ -1 +1,2 @@ variable "memory" {} +variable "feature" {} diff --git a/config/module/tree.go b/config/module/tree.go index d20f163a4..8ce24a321 100644 --- a/config/module/tree.go +++ b/config/module/tree.go @@ -259,7 +259,7 @@ func (t *Tree) Validate() error { } // If something goes wrong, here is our error template - newErr := &TreeError{Name: []string{t.Name()}} + newErr := &treeError{Name: []string{t.Name()}} // Terraform core does not handle root module children named "root". // We plan to fix this in the future but this bug was brought up in @@ -271,15 +271,14 @@ func (t *Tree) Validate() error { // Validate our configuration first. if err := t.config.Validate(); err != nil { - newErr.Err = err - return newErr + newErr.Add(err) } // If we're the root, we do extra validation. This validation usually // requires the entire tree (since children don't have parent pointers). if len(t.path) == 0 { if err := t.validateProviderAlias(); err != nil { - return err + newErr.Add(err) } } @@ -293,7 +292,7 @@ func (t *Tree) Validate() error { continue } - verr, ok := err.(*TreeError) + verr, ok := err.(*treeError) if !ok { // Unknown error, just return... return err @@ -301,7 +300,7 @@ func (t *Tree) Validate() error { // Append ourselves to the error and then return verr.Name = append(verr.Name, t.Name()) - return verr + newErr.AddChild(verr) } // Go over all the modules and verify that any parameters are valid @@ -327,10 +326,9 @@ func (t *Tree) Validate() error { // Compare to the keys in our raw config for the module for k, _ := range m.RawConfig.Raw { if _, ok := varMap[k]; !ok { - newErr.Err = fmt.Errorf( + newErr.Add(fmt.Errorf( "module %s: %s is not a valid parameter", - m.Name, k) - return newErr + m.Name, k)) } // Remove the required @@ -339,10 +337,9 @@ func (t *Tree) Validate() error { // If we have any required left over, they aren't set. for k, _ := range requiredMap { - newErr.Err = fmt.Errorf( - "module %s: required variable %s not set", - m.Name, k) - return newErr + newErr.Add(fmt.Errorf( + "module %s: required variable %q not set", + m.Name, k)) } } @@ -369,33 +366,61 @@ func (t *Tree) Validate() error { } } if !found { - newErr.Err = fmt.Errorf( + newErr.Add(fmt.Errorf( "%s: %s is not a valid output for module %s", - source, mv.Field, mv.Name) - return newErr + source, mv.Field, mv.Name)) } } } + return newErr.ErrOrNil() +} + +// treeError is an error use by Tree.Validate to accumulates all +// validation errors. +type treeError struct { + Name []string + Errs []error + Children []*treeError +} + +func (e *treeError) Add(err error) { + e.Errs = append(e.Errs, err) +} + +func (e *treeError) AddChild(err *treeError) { + e.Children = append(e.Children, err) +} + +func (e *treeError) ErrOrNil() error { + if len(e.Errs) > 0 || len(e.Children) > 0 { + return e + } return nil } -// TreeError is an error returned by Tree.Validate if an error occurs -// with validation. -type TreeError struct { - Name []string - Err error -} +func (e *treeError) Error() string { + name := strings.Join(e.Name, ".") + var out bytes.Buffer + fmt.Fprintf(&out, "module %s: ", name) -func (e *TreeError) Error() string { - // Build up the name - var buf bytes.Buffer - for _, n := range e.Name { - buf.WriteString(n) - buf.WriteString(".") + if len(e.Errs) == 1 { + // single like error + out.WriteString(e.Errs[0].Error()) + } else { + // multi-line error + for _, err := range e.Errs { + fmt.Fprintf(&out, "\n %s", err) + } } - buf.Truncate(buf.Len() - 1) - // Format the value - return fmt.Sprintf("module %s: %s", buf.String(), e.Err) + if len(e.Children) > 0 { + // start the next error on a new line + out.WriteString("\n ") + } + for _, child := range e.Children { + out.WriteString(child.Error()) + } + + return out.String() } diff --git a/config/module/tree_test.go b/config/module/tree_test.go index 6ca5f2a72..475ce7a7c 100644 --- a/config/module/tree_test.go +++ b/config/module/tree_test.go @@ -410,9 +410,18 @@ func TestTreeValidate_requiredChildVar(t *testing.T) { t.Fatalf("err: %s", err) } - if err := tree.Validate(); err == nil { + err := tree.Validate() + if err == nil { t.Fatal("should error") } + + // ensure both variables are mentioned in the output + errMsg := err.Error() + for _, v := range []string{"feature", "memory"} { + if !strings.Contains(errMsg, v) { + t.Fatalf("no mention of missing variable %q", v) + } + } } const treeLoadStr = `