ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

Ensure all errors are in JSON formatted CLI output

Browse Source 
Previously if the errors passed into the presentation Output were not
structured w/ JSON tags for marshaling the error would be an empty
string.

This changes to wrap the errors with a struct in cases where they
would otherwise not be formatted. We do this by forcing every error
into a structure and translating known error types into it.

Fixes: #1726
Fixes: #1724
Signed-off-by: Patrick East <east.patrick@gmail.com>
This commit is contained in:
Patrick East
2019-09-11 16:13:58 -07:00
committed by Torin Sandall
parent e5428406c4
commit f1b9c7586b
13 changed files with 562 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ast/parser_ext.go
+13 -11
View File
@@ -763,12 +763,13 @@ func (vt *varToRefTransformer) Transform(x interface{}) (interface{}, error) {
return x, nil
}
type parserErrorDetail struct {
line string
idx int
// ParserErrorDetail holds additional details for parser errors.
type ParserErrorDetail struct {
Line string `json:"line"`
Idx int `json:"idx"`
}
func newParserErrorDetail(bs []byte, pos position) *parserErrorDetail {
func newParserErrorDetail(bs []byte, pos position) *ParserErrorDetail {
offset := pos.offset
@@ -809,16 +810,17 @@ func newParserErrorDetail(bs []byte, pos position) *parserErrorDetail {
line := bs[begin:end]
index := offset - begin
return &parserErrorDetail{
line: string(line),
idx: index,
return &ParserErrorDetail{
Line: string(line),
Idx: index,
}
}
func (d parserErrorDetail) Lines() []string {
line := strings.TrimLeft(d.line, "\t") // remove leading tabs
tabCount := len(d.line) - len(line)
return []string{line, strings.Repeat(" ", d.idx-tabCount) + "^"}
// Lines returns the pretty formatted line output for the error details.
func (d ParserErrorDetail) Lines() []string {
line := strings.TrimLeft(d.Line, "\t") // remove leading tabs
tabCount := len(d.Line) - len(line)
return []string{line, strings.Repeat(" ", d.Idx-tabCount) + "^"}
}
func isNewLineChar(b byte) bool {
ast/parser_test.go
+25 -25
View File
@@ -1542,15 +1542,15 @@ func TestParseErrorDetails(t *testing.T) {
tests := []struct {
note string
exp *parserErrorDetail
exp *ParserErrorDetail
err string
input string
}{
{
note: "no match: bad rule name",
exp: &parserErrorDetail{
line: ".",
idx: 0,
exp: &ParserErrorDetail{
Line: ".",
Idx: 0,
},
input: `
package test
@@ -1558,54 +1558,54 @@ package test
},
{
note: "no match: bad termination for comprehension",
exp: &parserErrorDetail{
line: "p = [true | true}",
idx: 16,
exp: &ParserErrorDetail{
Line: "p = [true | true}",
Idx: 16,
},
input: `
package test
p = [true | true}`},
{
note: "no match: non-terminated comprehension",
exp: &parserErrorDetail{
line: "p = [true | true",
idx: 15,
exp: &ParserErrorDetail{
Line: "p = [true | true",
Idx: 15,
},
input: `
package test
p = [true | true`},
{
note: "no match: expected expression",
exp: &parserErrorDetail{
line: "p { true; }",
idx: 10,
exp: &ParserErrorDetail{
Line: "p { true; }",
Idx: 10,
},
input: `
package test
p { true; }`},
{
note: "empty body",
exp: &parserErrorDetail{
line: "p { }",
idx: 2,
exp: &ParserErrorDetail{
Line: "p { }",
Idx: 2,
},
input: `
package test
p { }`},
{
note: "non-terminated string",
exp: &parserErrorDetail{
line: `p = "foo`,
idx: 4,
exp: &ParserErrorDetail{
Line: `p = "foo`,
Idx: 4,
},
input: `
package test
p = "foo`},
{
note: "rule with error begins with one tab",
exp: &parserErrorDetail{
line: "\tas",
idx: 2,
exp: &ParserErrorDetail{
Line: "\tas",
Idx: 2,
},
input: `
package test
@@ -1615,9 +1615,9 @@ package test
^`},
{
note: "rule term with error begins with two tabs",
exp: &parserErrorDetail{
line: "\t\tas",
idx: 3,
exp: &ParserErrorDetail{
Line: "\t\tas",
Idx: 3,
},
input: `
package test
cmd/check.go
+11 -6
View File
@@ -5,13 +5,14 @@
package cmd
import (
"encoding/json"
"fmt"
"io"
"os"
"github.com/spf13/cobra"
"github.com/open-policy-agent/opa/ast"
pr "github.com/open-policy-agent/opa/internal/presentation"
"github.com/open-policy-agent/opa/loader"
"github.com/open-policy-agent/opa/util"
)
@@ -99,14 +100,18 @@ func checkModules(args []string) int {
func outputErrors(err error) {
switch checkParams.format.String() {
case checkFormatJSON:
result := map[string]error{
"errors": err,
result := pr.Output{
Errors: pr.NewOutputErrors(err),
}
bs, err := json.MarshalIndent(result, "", " ")
var out io.Writer
if err != nil {
fmt.Fprintln(os.Stderr, err)
out = os.Stderr
} else {
fmt.Fprintln(os.Stdout, string(bs))
out = os.Stdout
}
err := pr.JSON(out, result)
if err != nil {
fmt.Fprintln(os.Stderr, err.Error())
}
default:
fmt.Fprintln(os.Stdout, err)
cmd/eval.go
+10 -7
View File
@@ -330,25 +330,28 @@ func eval(args []string, params evalCommandParams, w io.Writer) (bool, error) {
eval := rego.New(regoArgs...)
var result pr.Output
var resultErr error
var parsedModules map[string]*ast.Module
if !params.partial {
var pq rego.PreparedEvalQuery
pq, result.Error = eval.PrepareForEval(ctx)
if result.Error == nil {
pq, resultErr = eval.PrepareForEval(ctx)
if resultErr == nil {
parsedModules = pq.Modules()
result.Result, result.Error = pq.Eval(ctx)
result.Result, resultErr = pq.Eval(ctx)
}
} else {
var pq rego.PreparedPartialQuery
pq, result.Error = eval.PrepareForPartial(ctx)
if result.Error == nil {
pq, resultErr = eval.PrepareForPartial(ctx)
if resultErr == nil {
parsedModules = pq.Modules()
result.Partial, result.Error = eval.Partial(ctx)
result.Partial, resultErr = eval.Partial(ctx)
}
}
result.Errors = pr.NewOutputErrors(resultErr)
switch params.explain.String() {
case explainModeFull:
result.Explanation = *tracer
@@ -390,7 +393,7 @@ func eval(args []string, params evalCommandParams, w io.Writer) (bool, error) {
if err != nil {
return false, err
} else if result.Error != nil {
} else if len(result.Errors) > 0 {
// If the rego package returned an error, return a special error here so
// that the command doesn't print the same error twice. The error will
// have been printed above by the presentation package.
cmd/eval_test.go
+1 -1
View File
@@ -288,7 +288,7 @@ func TestEvalErrorJSONOutput(t *testing.T) {
t.Fatal(err)
}
if output["error"] == nil {
if output["errors"] == nil {
t.Fatalf("Expected error to be non-nil")
}
cmd/parse.go
+6 -4
View File
@@ -12,6 +12,7 @@ import (
"github.com/spf13/cobra"
"github.com/open-policy-agent/opa/ast"
pr "github.com/open-policy-agent/opa/internal/presentation"
"github.com/open-policy-agent/opa/loader"
"github.com/open-policy-agent/opa/util"
)
@@ -49,13 +50,14 @@ func parse(args []string) int {
}
result, err := loader.Rego(args[0])
if err != nil {
fmt.Fprintln(os.Stderr, err)
return 1
}
switch parseParams.format.String() {
case parseFormatJSON:
if err != nil {
pr.JSON(os.Stderr, pr.Output{Errors: pr.NewOutputErrors(err)})
return 1
}
bs, err := json.MarshalIndent(result.Parsed, "", " ")
if err != nil {
fmt.Fprintln(os.Stderr, err)
internal/presentation/presentation.go
+117 -7
View File
@@ -21,9 +21,11 @@ import (
"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/cover"
"github.com/open-policy-agent/opa/format"
"github.com/open-policy-agent/opa/loader"
"github.com/open-policy-agent/opa/metrics"
"github.com/open-policy-agent/opa/profiler"
"github.com/open-policy-agent/opa/rego"
"github.com/open-policy-agent/opa/storage"
"github.com/open-policy-agent/opa/topdown"
)
@@ -107,7 +109,7 @@ func (o DepAnalysisOutput) sort() {
// Output contains the result of evaluation to be presented.
type Output struct {
Error error `json:"error,omitempty"`
Errors OutputErrors `json:"errors,omitempty"`
Result rego.ResultSet `json:"result,omitempty"`
Partial *rego.PartialQueries `json:"partial,omitempty"`
Metrics metrics.Metrics `json:"metrics,omitempty"`
@@ -127,6 +129,114 @@ func (e Output) undefined() bool {
return len(e.Result) == 0 && (e.Partial == nil || len(e.Partial.Queries) == 0)
}
// NewOutputErrors creates a new slice of OutputError's based
// on the type of error passed in. Known structured types will
// be translated as appropriate, while unknown errors are
// placed into a structured format with their string value.
func NewOutputErrors(err error) []OutputError {
var errs []OutputError
if err != nil {
// Handle known structured errors
switch typedErr := err.(type) {
case *ast.Error:
oe := OutputError{
Code: typedErr.Code,
Message: typedErr.Message,
Details: typedErr.Details,
err: typedErr,
}
// TODO(patrick-east): Why does the JSON marshaller marshal
// location as `null` when err.location == nil?!
if typedErr.Location != nil {
oe.Location = typedErr.Location
}
errs = []OutputError{oe}
case *topdown.Error:
errs = []OutputError{{
Code: typedErr.Code,
Message: typedErr.Message,
Location: typedErr.Location,
err: typedErr,
}}
case *storage.Error:
errs = []OutputError{{
Code: typedErr.Code,
Message: typedErr.Message,
err: typedErr,
}}
// The cases below are wrappers for other errors, format errors
// recursively on them.
case ast.Errors:
for _, e := range typedErr {
if e != nil {
errs = append(errs, NewOutputErrors(e)...)
}
}
case rego.Errors:
for _, e := range typedErr {
if e != nil {
errs = append(errs, NewOutputErrors(e)...)
}
}
case loader.Errors:
{
for _, e := range typedErr {
if e != nil {
errs = append(errs, NewOutputErrors(e)...)
}
}
}
default:
// Any errors which don't have a structure we know about
// are converted to their string representation only.
errs = []OutputError{{
Message: err.Error(),
err: typedErr,
}}
}
}
return errs
}
// OutputErrors is a list of errors encountered
// which are to presented.
type OutputErrors []OutputError
func (e OutputErrors) Error() string {
if len(e) == 0 {
return "no error(s)"
}
if len(e) == 1 {
return fmt.Sprintf("1 error occurred: %v", e[0].Error())
}
var s []string
for _, err := range e {
s = append(s, err.Error())
}
return fmt.Sprintf("%d errors occurred:\n%s", len(e), strings.Join(s, "\n"))
}
// OutputError provides a common structure for all OPA
// library errors so that the JSON output given by the
// presentation package is consistent and parsable.
type OutputError struct {
Message string `json:"message"`
Code string `json:"code,omitempty"`
Location interface{} `json:"location,omitempty"`
Details interface{} `json:"details,omitempty"`
err error
}
func (j OutputError) Error() string {
return j.err.Error()
}
// JSON writes x to w with indentation.
func JSON(w io.Writer, x interface{}) error {
encoder := json.NewEncoder(w)
@@ -136,8 +246,8 @@ func JSON(w io.Writer, x interface{}) error {
// Bindings prints the bindings from r to w.
func Bindings(w io.Writer, r Output) error {
if r.Error != nil {
return prettyError(w, r.Error)
if r.Errors != nil {
return prettyError(w, r.Errors)
}
for _, rs := range r.Result {
if err := JSON(w, rs.Bindings); err != nil {
@@ -149,8 +259,8 @@ func Bindings(w io.Writer, r Output) error {
// Values prints the values from r to w.
func Values(w io.Writer, r Output) error {
if r.Error != nil {
return prettyError(w, r.Error)
if r.Errors != nil {
return prettyError(w, r.Errors)
}
for _, rs := range r.Result {
line := make([]interface{}, len(rs.Expressions))
@@ -171,8 +281,8 @@ func Pretty(w io.Writer, r Output) error {
return err
}
}
if r.Error != nil {
if err := prettyError(w, r.Error); err != nil {
if r.Errors != nil {
if err := prettyError(w, r.Errors); err != nil {
return err
}
} else if r.undefined() {
internal/presentation/presentation_test.go
+360
View File
@@ -0,0 +1,360 @@
// Copyright 2019 The OPA Authors. All rights reserved.
// Use of this source code is governed by an Apache2
// license that can be found in the LICENSE file.
package presentation
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"testing"
"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/loader"
"github.com/open-policy-agent/opa/rego"
"github.com/open-policy-agent/opa/storage"
"github.com/open-policy-agent/opa/storage/inmem"
"github.com/open-policy-agent/opa/util/test"
)
type testErrorWithMarshaller struct {
msg string
}
func (t *testErrorWithMarshaller) Error() string {
return t.msg
}
func (t *testErrorWithMarshaller) MarshalJSON() ([]byte, error) {
return json.Marshal(struct {
Text string `json:"text"`
}{
Text: t.msg,
})
}
func validateJSONOutput(t *testing.T, testErr error, expected string) {
t.Helper()
output := Output{Errors: NewOutputErrors(testErr)}
var buf bytes.Buffer
err := JSON(&buf, output)
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}
if buf.String() != expected {
t.Fatalf("Unexpected marshalled error value.\n Expected (len=%d):\n>>>\n%s\n<<<\n\nActual (len=%d):\n>>>>\n%s\n<<<<\n",
len(expected), expected, len(buf.String()), buf.String())
}
}
func TestOutputJSONErrorUnstructured(t *testing.T) {
err := errors.New("some text")
expected := `{
"errors": [
{
"message": "some text"
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorCustomMarshaller(t *testing.T) {
err := &testErrorWithMarshaller{
msg: "custom message",
}
expected := `{
"errors": [
{
"message": "custom message"
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredASTErr(t *testing.T) {
err := &ast.Error{
Code: "1",
Message: "error message",
}
expected := `{
"errors": [
{
"message": "error message",
"code": "1"
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredStorageErr(t *testing.T) {
store := inmem.New()
txn := storage.NewTransactionOrDie(context.Background(), store)
err := store.Write(context.Background(), txn, storage.AddOp, storage.Path{}, map[string]interface{}{"foo": 1})
expected := `{
"errors": [
{
"message": "data write during read transaction",
"code": "storage_invalid_txn_error"
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredTopdownErr(t *testing.T) {
mod := `
package test
p(x) = y {
y = x[_]
}
z := p([1, 2, 3])
`
_, err := rego.New(
rego.Module("test.rego", mod),
rego.Query("data.test.z"),
).Eval(context.Background())
expected := `{
"errors": [
{
"message": "functions must not produce multiple outputs for same inputs",
"code": "eval_conflict_error",
"location": {
"file": "test.rego",
"row": 4,
"col": 3
}
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredAstErr(t *testing.T) {
_, err := rego.New(rego.Query("count(0)")).Eval(context.Background())
expected := `{
"errors": [
{
"message": "count: invalid argument(s)",
"code": "rego_type_error",
"location": {
"file": "",
"row": 1,
"col": 1
},
"details": {
"have": [
{
"type": "number"
},
null
],
"want": [
{
"of": [
{
"of": {
"of": [],
"type": "any"
},
"type": "set"
},
{
"dynamic": {
"of": [],
"type": "any"
},
"type": "array"
},
{
"dynamic": {
"key": {
"of": [],
"type": "any"
},
"value": {
"of": [],
"type": "any"
}
},
"type": "object"
},
{
"type": "string"
}
],
"type": "any"
},
{
"type": "number"
}
]
}
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredAstParseErr(t *testing.T) {
_, err := rego.New(
rego.Module("parse-err.rego", "!!!"),
rego.Query("!!!"),
).Eval(context.Background())
expected := `{
"errors": [
{
"message": "no match found",
"code": "rego_parse_error",
"location": {
"file": "parse-err.rego",
"row": 1,
"col": 1
},
"details": {
"line": "!!!",
"idx": 0
}
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredASTErrList(t *testing.T) {
c := ast.NewCompiler()
c.Compile(map[string]*ast.Module{
"error.rego": ast.MustParseModule(`
package test
q {
bad[reference]
}
`)})
c.Errors.Sort()
err := c.Errors
expected := `{
"errors": [
{
"message": "var bad is unsafe",
"code": "rego_unsafe_var_error",
"location": {
"file": "",
"row": 5,
"col": 2
}
},
{
"message": "var reference is unsafe",
"code": "rego_unsafe_var_error",
"location": {
"file": "",
"row": 5,
"col": 2
}
}
]
}
`
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredLoaderErrList(t *testing.T) {
files := map[string]string{
// bundle a
"a/data.json": "{{{",
"b/data.json": "...",
}
var err error
var tmpPath string
test.WithTempFS(files, func(path string) {
tmpPath = path
_, err = loader.All([]string{path})
})
expected := fmt.Sprintf(`{
"errors": [
{
"message": "%s/a/data.json: invalid character '{' looking for beginning of object key string"
},
{
"message": "%s/b/data.json: invalid character '.' looking for beginning of value"
}
]
}
`, tmpPath, tmpPath)
validateJSONOutput(t, err, expected)
}
func TestOutputJSONErrorStructuredRegoErrList(t *testing.T) {
mod := `
package test
p {
bad_func1()
}
q {
bad_func2()
}
`
_, err := rego.New(
rego.Module("error.rego", mod),
rego.Query("data"),
).PrepareForEval(context.Background())
expected := `{
"errors": [
{
"message": "undefined function bad_func1",
"code": "rego_type_error",
"location": {
"file": "error.rego",
"row": 5,
"col": 2
}
},
{
"message": "undefined function bad_func2",
"code": "rego_type_error",
"location": {
"file": "error.rego",
"row": 9,
"col": 2
}
}
]
}
`
validateJSONOutput(t, err, expected)
}
loader/errors.go
+4 -3
View File
@@ -11,9 +11,10 @@ import (
"github.com/open-policy-agent/opa/ast"
)
type loaderErrors []error
// Errors is a wrapper for multiple loader errors.
type Errors []error
func (e loaderErrors) Error() string {
func (e Errors) Error() string {
if len(e) == 0 {
return "no error(s)"
}
@@ -27,7 +28,7 @@ func (e loaderErrors) Error() string {
return fmt.Sprintf("%v errors occurred during loading:\n", len(e)) + strings.Join(buf, "\n")
}
func (e *loaderErrors) Add(err error) {
func (e *Errors) add(err error) {
if errs, ok := err.(ast.Errors); ok {
for i := range errs {
*e = append(*e, errs[i])
loader/loader.go
+6 -6
View File
@@ -248,7 +248,7 @@ func (l *Result) withParent(p string) *Result {
}
func all(paths []string, filter Filter, f func(*Result, string, int) error) (*Result, error) {
errors := loaderErrors{}
errors := Errors{}
root := newResult()
for _, path := range paths {
@@ -274,17 +274,17 @@ func all(paths []string, filter Filter, f func(*Result, string, int) error) (*Re
return root, nil
}
func allRec(path string, filter Filter, errors *loaderErrors, loaded *Result, depth int, f func(*Result, string, int) error) {
func allRec(path string, filter Filter, errors *Errors, loaded *Result, depth int, f func(*Result, string, int) error) {
path, err := fileurl.Clean(path)
if err != nil {
errors.Add(err)
errors.add(err)
return
}
info, err := os.Stat(path)
if err != nil {
errors.Add(err)
errors.add(err)
return
}
@@ -294,7 +294,7 @@ func allRec(path string, filter Filter, errors *loaderErrors, loaded *Result, de
if !info.IsDir() {
if err := f(loaded, path, depth); err != nil {
errors.Add(err)
errors.add(err)
}
return
}
@@ -307,7 +307,7 @@ func allRec(path string, filter Filter, errors *loaderErrors, loaded *Result, de
files, err := ioutil.ReadDir(path)
if err != nil {
errors.Add(err)
errors.add(err)
return
}
rego/rego.go
+3 -3
View File
@@ -1254,7 +1254,7 @@ func (r *Rego) parseModules(ctx context.Context, txn storage.Transaction, m metr
}
if len(errs) > 0 {
return errors.New(errs.Error())
return errs
}
return nil
@@ -1266,7 +1266,7 @@ func (r *Rego) loadFiles(ctx context.Context, txn storage.Transaction, m metrics
result, err := loader.Filtered(r.loadPaths.paths, r.loadPaths.filter)
if err != nil {
return fmt.Errorf("error loading paths: %s", err)
return err
}
for name, mod := range result.Modules {
r.parsedModules[name] = mod.Parsed
@@ -1275,7 +1275,7 @@ func (r *Rego) loadFiles(ctx context.Context, txn storage.Transaction, m metrics
if len(result.Documents) > 0 {
err = r.store.Write(ctx, txn, storage.AddOp, storage.Path{}, result.Documents)
if err != nil {
return fmt.Errorf("error writing loaded documents to store: %s", err)
return err
}
}
return nil
repl/repl.go
+2 -2
View File
@@ -864,7 +864,7 @@ func (r *REPL) evalBody(ctx context.Context, compiler *ast.Compiler, input ast.V
rs, err := eval.Eval(ctx)
output := pr.Output{
Error: err,
Errors: pr.NewOutputErrors(err),
Result: rs,
Metrics: r.metrics,
}
@@ -920,7 +920,7 @@ func (r *REPL) evalPartial(ctx context.Context, compiler *ast.Compiler, input as
output := pr.Output{
Metrics: r.metrics,
Partial: pq,
Error: err,
Errors: pr.NewOutputErrors(err),
}
switch r.explain {
server/server_test.go
+4 -1
View File
@@ -3027,7 +3027,10 @@ func TestBadQueryV1(t *testing.T) {
"row": 1,
"col": 1
},
"details": {}
"details": {
"line": "^ -i",
"idx": 0
}
}
]
}`