diff --git a/runtime/loader.go b/runtime/loader.go index adb67a0639..05c1224855 100644 --- a/runtime/loader.go +++ b/runtime/loader.go @@ -18,6 +18,26 @@ import ( "github.com/pkg/errors" ) +type loaderErrors []error + +func (e loaderErrors) Error() string { + if len(e) == 0 { + return "no error(s)" + } + if len(e) == 1 { + return "1 error occurred during loading: " + e[0].Error() + } + buf := make([]string, len(e)) + for i := range buf { + buf[i] = e[i].Error() + } + return fmt.Sprintf("%v errors occured during loading:\n", len(e)) + strings.Join(buf, "\n") +} + +func (e *loaderErrors) Add(err error) { + *e = append(*e, err) +} + type loaded struct { Documents map[string]interface{} Modules map[string]*loadedModule @@ -47,14 +67,14 @@ func (l *loaded) WithParent(p string) *loaded { type unsupportedDocumentType string -func (u unsupportedDocumentType) Error() string { - return "unsupported document type: " + string(u) +func (path unsupportedDocumentType) Error() string { + return string(path) + ": bad document type" } type unrecognizedFile string -func (u unrecognizedFile) Error() string { - return "unrecognized file: " + string(u) +func (path unrecognizedFile) Error() string { + return string(path) + ": can't recognize file type" } func isUnrecognizedFile(err error) bool { @@ -62,18 +82,30 @@ func isUnrecognizedFile(err error) bool { return ok } +type mergeError string + +func (e mergeError) Error() string { + return string(e) + ": merge error" +} + +type emptyModuleError string + +func (e emptyModuleError) Error() string { + return string(e) + ": empty policy" +} + func (l *loaded) Merge(path string, result interface{}) error { switch result := result.(type) { case *loadedModule: l.Modules[normalizeModuleID(path)] = result default: - obj, err := makeDir(l.path, result) - if err != nil { - return err + obj, ok := makeDir(l.path, result) + if !ok { + return unsupportedDocumentType(path) } - merged, err := mergeDocs(l.Documents, obj) - if err != nil { - return err + merged, ok := mergeDocs(l.Documents, obj) + if !ok { + return mergeError(path) } for k := range merged { l.Documents[k] = merged[k] @@ -85,6 +117,7 @@ func (l *loaded) Merge(path string, result interface{}) error { func loadAllPaths(paths []string) (*loaded, error) { root := newLoaded() + errors := loaderErrors{} for _, path := range paths { @@ -98,56 +131,59 @@ func loadAllPaths(paths []string) (*loaded, error) { info, err := os.Stat(path) if err != nil { - return nil, err + errors.Add(err) + continue } if info.IsDir() { - if err := loadDirRecursive(path, loaded.WithParent(info.Name())); err != nil { - return nil, err - } + loadDirRecursive(&errors, path, loaded.WithParent(info.Name())) } else { result, err := loadFile(path) if err != nil { - return nil, err - } - if err := loaded.Merge(path, result); err != nil { - return nil, err + errors.Add(err) + } else { + if err := loaded.Merge(path, result); err != nil { + errors.Add(err) + } } } } + if len(errors) > 0 { + return nil, errors + } + return root, nil } -func loadDirRecursive(dirPath string, loaded *loaded) error { +func loadDirRecursive(errors *loaderErrors, dirPath string, loaded *loaded) { files, err := ioutil.ReadDir(dirPath) if err != nil { - return err + errors.Add(err) + return } for _, file := range files { filePath := filepath.Join(dirPath, file.Name()) info, err := os.Stat(filePath) if err != nil { - return err - } - if info.IsDir() { - if err := loadDirRecursive(filePath, loaded.WithParent(info.Name())); err != nil { - return err - } + errors.Add(err) } else { - result, err := loadFileForKnownTypes(filePath) - if err != nil { - if _, ok := err.(unrecognizedFile); !ok { - return err - } + if info.IsDir() { + loadDirRecursive(errors, filePath, loaded.WithParent(info.Name())) } else { - if err := loaded.Merge(filePath, result); err != nil { - return err + result, err := loadFileForKnownTypes(filePath) + if err != nil { + if _, ok := err.(unrecognizedFile); !ok { + errors.Add(err) + } + } else { + if err := loaded.Merge(filePath, result); err != nil { + errors.Add(err) + } } } } } - return nil } func loadFileForKnownTypes(path string) (interface{}, error) { @@ -197,7 +233,10 @@ func jsonLoad(path string) (interface{}, error) { defer f.Close() decoder := util.NewJSONDecoder(f) var x interface{} - return x, decoder.Decode(&x) + if err = decoder.Decode(&x); err != nil { + return nil, errors.Wrapf(err, path) + } + return x, nil } func regoLoad(path string) (interface{}, error) { @@ -209,6 +248,9 @@ func regoLoad(path string) (interface{}, error) { if err != nil { return nil, err } + if module == nil { + return nil, emptyModuleError(path) + } result := &loadedModule{ Parsed: module, Raw: bs, @@ -228,13 +270,13 @@ func yamlLoad(path string) (interface{}, error) { return x, nil } -func makeDir(path []string, x interface{}) (map[string]interface{}, error) { +func makeDir(path []string, x interface{}) (map[string]interface{}, bool) { if len(path) == 0 { obj, ok := x.(map[string]interface{}) if !ok { - return nil, unsupportedDocumentType(fmt.Sprintf("%T", x)) + return nil, false } - return obj, nil + return obj, true } return makeDir(path[:len(path)-1], map[string]interface{}{path[len(path)-1]: x}) } diff --git a/runtime/loader_test.go b/runtime/loader_test.go index 2f760341bd..cbe36364a2 100644 --- a/runtime/loader_test.go +++ b/runtime/loader_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "reflect" "sort" + "strings" "testing" "github.com/ghodss/yaml" @@ -177,6 +178,42 @@ func TestLoadRooted(t *testing.T) { }) } +func TestLoadErrors(t *testing.T) { + files := map[string]string{ + "/x1.json": `{"x": [1,2,3]}`, + "/x2.json": `{"x": {"y": 1}}`, + "/empty.rego": ` `, + "/dir/a.json": ``, + "/dir/b.yaml": ` + foo: + - bar: + `, + "/bad_doc.json": "[1,2,3]", + } + withTempFS(files, func(rootDir string) { + paths := mustListPaths(rootDir, false)[1:] + sort.Strings(paths) + _, err := loadAllPaths(paths) + if err == nil { + t.Fatalf("Expected failure") + } + + expected := []string{ + "bad_doc.json: bad document type", + "a.json: EOF", + "b.yaml: error converting YAML to JSON", + "empty.rego: empty policy", + "x2.json: merge error", + } + + for _, s := range expected { + if !strings.Contains(err.Error(), s) { + t.Fatalf("Expected error to contain %v but got:\n%v", s, err) + } + } + }) +} + func withTempFS(files map[string]string, f func(string)) { rootDir, cleanup, err := makeTempFS(files) if err != nil { diff --git a/runtime/merge.go b/runtime/merge.go index e75efdc28c..6a059928d1 100644 --- a/runtime/merge.go +++ b/runtime/merge.go @@ -4,43 +4,35 @@ package runtime -import ( - "fmt" - - "github.com/pkg/errors" -) - // mergeDocs returns the result of merging a and b. If a and b cannot be merged -// because of conflicting key-value pairs, an error is returned. -func mergeDocs(a map[string]interface{}, b map[string]interface{}) (map[string]interface{}, error) { +// because of conflicting key-value pairs, ok is false. +func mergeDocs(a map[string]interface{}, b map[string]interface{}) (c map[string]interface{}, ok bool) { - merged := map[string]interface{}{} + c = map[string]interface{}{} for k := range a { - merged[k] = a[k] + c[k] = a[k] } for k := range b { add := b[k] - exist, ok := merged[k] + exist, ok := c[k] if !ok { - merged[k] = add + c[k] = add continue } existObj, existOk := exist.(map[string]interface{}) addObj, addOk := add.(map[string]interface{}) if !existOk || !addOk { - return nil, fmt.Errorf("%v: merge error: %T cannot merge into %T", k, add, exist) + return nil, false } - mergedObj, err := mergeDocs(existObj, addObj) - if err != nil { - return nil, errors.Wrapf(err, k) + c[k], ok = mergeDocs(existObj, addObj) + if !ok { + return nil, false } - - merged[k] = mergedObj } - return merged, nil + return c, true } diff --git a/runtime/merge_test.go b/runtime/merge_test.go index 4d3dc7b538..a04876e8c4 100644 --- a/runtime/merge_test.go +++ b/runtime/merge_test.go @@ -5,7 +5,6 @@ package runtime import ( - "fmt" "reflect" "testing" @@ -15,14 +14,15 @@ import ( func TestMergeDocs(t *testing.T) { tests := []struct { - a string - b string - c interface{} + a string + b string + c string + ok bool }{ - {`{"x": 1, "y": 2}`, `{"z": 3}`, `{"x": 1, "y": 2, "z": 3}`}, - {`{"x": {"y": 2}}`, `{"z": 3, "x": {"q": 4}}`, `{"x": {"y": 2, "q": 4}, "z": 3}`}, - {`{"x": 1}`, `{"x": 1}`, fmt.Errorf("x: merge error: json.Number cannot merge into json.Number")}, - {`{"x": {"y": [{"z": 2}]}}`, `{"x": {"y": [{"z": 3}]}}`, fmt.Errorf("x: y: merge error: []interface {} cannot merge into []interface {}")}, + {`{"x": 1, "y": 2}`, `{"z": 3}`, `{"x": 1, "y": 2, "z": 3}`, true}, + {`{"x": {"y": 2}}`, `{"z": 3, "x": {"q": 4}}`, `{"x": {"y": 2, "q": 4}, "z": 3}`, true}, + {`{"x": 1}`, `{"x": 1}`, "", false}, + {`{"x": {"y": [{"z": 2}]}}`, `{"x": {"y": [{"z": 3}]}}`, "", false}, } for _, tc := range tests { @@ -36,27 +36,23 @@ func TestMergeDocs(t *testing.T) { panic(err) } - switch c := tc.c.(type) { - case error: - _, err := mergeDocs(a, b) - if !reflect.DeepEqual(err.Error(), c.Error()) { - t.Errorf("Expected error to be exactly %v but got: %v", c, err) + if len(tc.c) == 0 { + + c, ok := mergeDocs(a, b) + if ok { + t.Errorf("Expected merge(%v,%v) == false but got: %v", a, b, c) } - case string: + } else { + expected := map[string]interface{}{} - if err := util.UnmarshalJSON([]byte(c), &expected); err != nil { + if err := util.UnmarshalJSON([]byte(tc.c), &expected); err != nil { panic(err) } - result, err := mergeDocs(a, b) - if err != nil { - t.Errorf("Unexpected error on merge(%v, %v): %v", a, b, err) - continue - } - - if !reflect.DeepEqual(result, expected) { - t.Errorf("Expected merge(%v, %v) to be %v but got: %v", a, b, expected, result) + c, ok := mergeDocs(a, b) + if !ok || !reflect.DeepEqual(c, expected) { + t.Errorf("Expected merge(%v, %v) == %v but got: %v (ok: %v)", a, b, expected, c, ok) } } }