open-policy-agent / opa Goto Github PK

It'd be great if we included 'data' as a command in the help screen in the repl so that people can see all their data. Couple of use cases.

1. show all data currently available. data
2. show top-level keys in the data currently available (for when there's too much data). data[x] = _
3. drill into data. data.foo.bar

Would be nice if all of those were available from help (maybe with subhelp commands like 'help data').

It would be nice if the request parameter value did not require a ":" character to indicate the start of the value. It should be possible to check if the value starts with ":" and parse accordingly. This is only a small change in server.go:parseRequest function (along with an update to the REST API docs).

Today, if a query contains a nested reference, e.g., data.foo[data.bar[0]], the storage layer will receive a read for the entire ref. This requires all storage plugins recursively process references to handle the nested case.

Nested references should be flattened either by storage.Storage or during evaluation in topdown.

Ran across the following error.

data.clair[layers][index].Name = data.docker.history[index][app].Id
storage error; bad path; object key must be a string, not 0

Presumably the problem is that 'index' is used for both an array and a dictionary.

It'd be great if somewhere in the docs we had a "reference guide" for people who are familiar with the basics but need clarity on something specific. For this issue, it'd be great if we had a reference guide for the builtins that Rego supports (e.g. addition, string-concatenation). Perhaps a handful of tables, one for each type of scalar value and the builtins Rego supports for that type.

If a set literal is referred to with a lookup value containing references to base documents, the evaluation is incorrect. Specifically it will fail because the lookup value will not have embedded references resolved. As a result, the lookup is performed with the reference itself contained in the lookup value, e.g., in pseudocode: set.contains(["a",1,data.foo[2]) instead of set.contains(["a",1,<value-referred-to-by-data.foo[2]).

For example, below pair1 contains a reference data.foo[2] which refers to a string value in the x.json file.

(master)torin:~/go/src/github.com/open-policy-agent/opa$ ./opa_darwin_amd64 run ~/x.json
OPA 0.2.1-dev (commit f56ae0d, built at 2016-11-14T22:07:08Z)

Run 'help' to see a list of commands.

> v = {["a","b"],["c","d"]}
> x = "d"
> pair1 = [data.foo[2], x]
> pair1
[
  "c",
  "d"
]
> v[pair1]
undefined

But lookup values that only contain ground values are fine:

> pair2 = ["c", "d"]
> v[pair2]
true

As are lookup values that contain references to virtual docs:

> y = "c"
> pair3 = [y, x]
> v[pair3]
true

As are lookup values that contain variables with bindings:

> a="c", b="d", pair4=[a,b], v[pair4]
+-----+-----+-----------+
|  a  |  b  |   pair4   |
+-----+-----+-----------+
| "c" | "d" | ["c","d"] |
+-----+-----+-----------+

The errors returned by the 1-arity arithmetic functions refer to the wrong term ("abs" below):

> abs("hello", 1)
error: 1:1: expected number (operand abs is not a number): illegal argument: "hello"

Rules can be written in the form <var> = <ground term>. This is useful for defining constants; it would be nice if we could do the same with terms containing refs. Currently, the parser will return an error because it deems the term as being non-ground (since references have not been resolved yet).

One solution would be to update the parser to allow non-ground terms. If the user did include a variable in the value and that variable was not eventually resolved, the compiler should catch the error when performing the safety checks on the head.

Example parse error:

torin:~$ opa run test.rego
test.rego:7: expected rule (annotations_versioned = {
    "metadata": {
        "annotations": annotations,
        "resourceVersion": object.metadata.resourceVersion
    }
} must be declared inside a rule)

Example input:

package io.k8s

import object

annotations = {}

annotations_versioned = {
    "metadata": {
        "annotations": annotations,
        "resourceVersion": object.metadata.resourceVersion
    }
}

If a package path overlaps with a rule name (or vice versa), OPA should handle it better.

It seems reasonable to treat this kind of overlap as a conflict and reject at compile-time.

Simple example:

test1.rego...

package x

p = "hello"

test2.rego...

package x.p

q = "goodbye"

queries...

[venv/go](master)[*]torin:~/go/src/github.com/open-policy-agent/opa$ ./opa_darwin_amd64 run ~/test*
OPA 0.1.1-dev (commit b033f78-dirty, built at 2016-11-03T21:24:35Z)

Run 'help' to see a list of commands.

> data.x
{
  "p": "hello"
}
> data.x.p
"hello"
> data.x.p.q
"goodbye"

Currently, the compiler mutates input modules as part of the compilation process. This leads to problems in the REPL and other places where modules are re-compiled multiple times. The compiler ought to deep-copy the input modules so that user input can be re-compiled consistently.

Example REPL issue:

(master)torin:~/go/src/github.com/open-policy-agent/opa$ opa run
OPA 0.2.1-dev (commit f56ae0d, built at 2016-11-22T18:31:35Z)

Run 'help' to see a list of commands.

> import data.abc
> abc
undefined
> abc
error: 1 error occurred: 1:1: repl1: abc is unsafe (variable abc must appear in the output position of at least one non-negated expression)

This also means that the Server should not insert the compiled version of the modules into storage.

It'd be really great if we had some additional string manipulation builtins and functions to convert between scalar types. Use case is checking if 1 docker image name is a previous version of another docker image name, e.g. given the following two strings determine that one is a previous version of the other.

"ubuntu/ubuntu:16.04"
"ubuntu/ubuntu:14.04"

Logic for this would be ...

1. break each string into an array, such as ["ubuntu", "ubuntu", "16.04"]
2. convert last element of each array into a number: ["ubuntu", "ubuntu", 16.04]
3. check if the first 2 elements of the array are the same for the 2 strings
4. check that element 3 of array 1 is greater than element 3 of array 2

Different ways to implement that of course, but here are some builtins that came to mind.

indexof(string, substring, output)
substring(string, startindex, length, output)
len(string, output)
lower(string, lower-case-output)
upper(string, upper-case-output)

to_num(string-or-bool, output)
to_string(num-or-bool, output)
to_bool(string-or-num, output)

The REPL now allows users to specify query inputs by defining a documents under the repl.globals namespace. E.g.,

OPA 0.2.2-dev (commit e83a083, built at 2016-11-25T01:40:36Z)

Run 'help' to see a list of commands.

> package repl.globals
>
> request = {"method": "GET", "path": "/foo"}
>
>
> package abc
> import request
> request
+--------------------------------+
|            request             |
+--------------------------------+
| {"method":"GET","path":"/foo"} |
+--------------------------------+

It would be good to include this in the REPL example so people know the behaviour exists.

The parser currently allows modules to include multiple package directives. Only the first package directive will be processed, others are ignored. This is confusing and should not be allowed. Modules must contain exactly one package directive.

For example:

test.rego:

package xyz
package abc

opa run -s

curl -X PUT localhost:8181/v1/policies/test --data-binary @test.rego
{
  "ID": "test",
  "Module": {
    "Package": {
      "Path": [
        {
          "Type": "var",
          "Value": "data"
        },
        {
          "Type": "string",
          "Value": "xyz"
        }
      ]
    },
    "Imports": null,
    "Rules": []
  }
}

As I've been writing Rego recently I've tried to embed terms inside each other, which seems to not be allowed. Besides it being natural to write rules with embedded terms, embedding terms should allow for improved efficiency during evaluation. I've been defining virtual docs that have dictionaries/arrays as outputs (e.g. {id: ..., name: ...}), and then want to use those virtual docs to ask if one of the outputs has a particular value for one of its fields (e.g. name: "foo").

Here's a contrived example of how we do that today. Notice that when we query p, we are iterating over all the values of p and checking if the first element is an "a".

q["a"] = 1 :- true
q["b"] = 2 :- true
p[x] :- q[y] = z, x=[y,z]
p[x], x=["a", y]
Enter data.repl.p[x], eq(x, ["a", y])
| Eval data.repl.p[x]
| Enter p[x]
| | Eval eq(data.repl.q[y], z)
| | Enter q["a"] = 1
| | | Eval true
| | | Exit q["a"] = 1
| | Eval eq(x, ["a", 1])
| | Exit p[["a", 1]]
| Eval eq(["a", 1], ["a", y])
| Exit data.repl.p[x], eq(x, ["a", y])
Redo data.repl.p[x], eq(x, ["a", y])
| Redo true
| Redo p[["a", 1]]
| | Redo eq(1, z)
| | Redo q["b"] = 2
| | | Eval true
| | | Exit q["b"] = 2
| | Eval eq(x, ["b", 2])
| | Exit p[["b", 2]]
| Eval eq(["b", 2], ["a", y])
| Fail eq(["b", 2], ["a", y])
+---------+---+
| x | y |
+---------+---+
| ["a",1] | 1 |
+---------+---+

It would be more efficient if the fact that the first value in the output is an "a" is pushed down into the computation of p so that we only compute the slice of p where every array is of the form ["a", y]. Here is how I would imagine this working in the future. I embedded 3 comments with // explaining the difference with the trace above.

q["a"] = 1 :- true
q["b"] = 2 :- true
p[[y,z]] :- q[y] = z // moved x=[y,z] into the head of the rule
p[["a", y]] // moved x=["a", y] into head of query
Enter data.repl.p[["a", y]]
| Eval data.repl.p[["a", y]]
| Enter p[["a", y]]
| | Eval eq(data.repl.q["a"], z)
| | Enter q["a"] = 1
| | | Eval true
| | | Exit q["a"] = 1
| | Eval eq(x, ["a", 1])
| | Exit p[["a", 1]]
| Eval eq(["a", 1], ["a", y])
| Exit data.repl.p[x], eq(x, ["a", y])
// used index to lookup just the q["a"] values, so not scanning thru all q data
Redo data.repl.p[x], eq(x, ["a", y])
| Redo true
| Redo p[["a", 1]]
| | Redo q["a"] = 1
| | Fail
| Fail data.repl.p[x], eq(x, ["a", y])
+---------+---+
| x | y |
+---------+---+
| ["a",1] | 1 |
+---------+---+

If the parser fails to find a matching rule, it stops immediately and returns an error message as follows:

1 error occurred: /Users/torin/test.rego:3: no match found, unexpected ','

It would be nice if the error message contained more context such as the surrounding text.

In some cases, the error message is slightly misleading, for instance:

test.rego:

package ex

limit :- input.value >= .5

error:

1 error occurred: /Users/torin/test.rego:3: no match found, unexpected '>'

The floating point number doesn't parse however because of how expressions are matched, it fails at >.

It's possible the parser rules could be restructured to improve error handling however this seems like something the parser should be able to deal with.

Creating this ticket as a placeholder for more investigation into improvements.

Rules with the name "request" and "data" may be defined for test purposes to mock out the normal request/data documents. The parser currently does not recognize expressions of the form "request = term" or "data = term" as rules.

ParseRuleFromBody should be updated to handle the "request" and "data" refs.

Example:

package ex

request = {"foo":1,"bar":2}

or

package ex

data = {"foo": 1, "data": 2}

A simple workaround is to declare the rule body, e.g., request = {"foo": 1} :- true.

It would be nice if composite terms could be used as reference operands.

Today we don't support this in the parser:

data.example.p[[x,y]]

Authors have to write:

v = [x,y], data.example.p[v]

When policies need to be optimized by hand, the first step is to understand where time is spent in evaluating the policy. Without a query profiler this task can be quite difficult because it requires deep knowledge of the policy (which may be quite large, e.g., hundreds of lines split across multiple files.)

To start, we should add basic query profiling support to the eval subcommand.

For example:

opa eval --profile --data somedir 'data.example.allow == true'

The profiler should provide a per-expression breakdown of time spent, # of invocations, etc. The the command should also be able to sort and return the top-N expressions.

The code that accumulates results for ad-hoc queries in the server/REST API is not handling errors correctly (see

As a result, API calls are succeeding when they should be returning an error.

OPA currently uses float64 as the underlying number representation in storage and the AST. Float64 was used initially because this is the default return type for numbers in encoding/json. Unfortunately this leads to loss of precision when dealing with large int64 values.

One solution would be to switch to json.Number (string) as the underlying representation.

OPA 0.2.1 (commit b3f62a1, built at 2016-11-23T16:33:13Z)

Run 'help' to see a list of commands.

>
>
>
> p["a"] = 1 :- true
> p["a"] = 1 :- true
> p
error: evaluation error (code: 2): multiple values for data.repl.p: rules must produce exactly one value for object document keys: check rule definition(s): p

The conflict check should allow for duplicate key/value pairs.

Busy or long-lived REPL sessions would benefit from a command to dump rules that are defined in the REPL environment. All REPL input is evaluated within a module, so the REPL environment is essentially the currently active module.

The simplest option would be a REPL command to "show" the currently active module, for example:

> p :- true
> show
package repl

p :- true

Another, more complex option, would allow the REPL to drop into the user's $EDITOR with an "edit" command. The editor would receive the text representation of the REPL's currently active module. This would be nice because then users could create and update complex rules, on the fly, with their choice of editor. (something which is a bit of a pain today).

It would be nice if OPA returned errors with more actionable information.

Currently the core packages (ast, storage, topdown) all return structured errors that include a code and a message. In all cases, the messages attempt to provide enough information to understand and fix the problem. In some cases, it's difficult to encode all the necessary information in a simple string

It would be nice if we could link to documentation articles that help the user understand why the error occurred.

Updating the errors to include a link field would be relatively straightforward. The interface could require either a URL or a path. For paths, the URL would be constructed with a linker flag containing the base URL of the documentation.

data
error: unbound value: _
data[x]
error: 1 error occurred: 1:1: repl1: recursive reference: repl1 -> repl1 (recursion is not allowed)

The parser requires that floating point numbers include a leading zero digit, for example:

torin:~$ opa run
OPA 0.3.2-dev (commit ad52c56-dirty, built at 2017-01-16T17:34:27Z)

Run 'help' to see a list of commands.

> 0.5
0.5
> .5
|
error: 1 error occurred: 1:2: no match found, unexpected '.'

The parser should be updated to support FP numbers with and without the leading zero digit.

When support for directory loading and path prefixes was added, the code that watches the files for changes was not updated. As a result, it tries to open the entire path with the entire command line string.

torin:~$ opa run -w foo:x.json
error opening watch: lstat foo:x.json: no such file or directory

In some cases, it's counter-intuitive that OPA returns 404 to indicate an undefined result. The most common example is for authorization policies that are typically of the form:

package ex

import request

allow :- request.user = "admin"
allow :- request.user = "bob", request.method = "GET"
# ... possibly more allow rules ...

In this case, a query to /data/ex/allow with request = {"user": "bob", "method": "POST"} would return 404.

It would be more intuitive if /data/ex/allow returned 200 with true or false in the body.

More generally, it would be useful if users could define default values for complete documents. The default value would only be produced if all of the rules that defined the complete document were undefined.

For example:

package ex

import request

default allow = false

allow :- request.user = "admin"
allow :- request.user = "bob", request.method = "GET"

Note, this is never an issue with partially defined documents because a query to the document would simply return an empty set/object if all of the rules were undefined.

So, the syntax for the new keyword would be:

"default" var "=" <term>

The default keyword could only be used with the name of a rule in the same package. The name must refer to an existing rule.

Open Questions:

• Should there be any restrictions on the type of term? Specifically, are references allowed?

Currently, policy authors can use built-in names for local variables and the compiler will not complain.

However, the compiler should not automatically consider these variables safe:

> abs(eq, 1)
error: 1:1: expected number (operand abs is not a number): unbound value: eq

It would be nice if we could run a Rego query from the command line and have it output json so that we could pipe results to something like 'less'. For example...

$ opa run foo.json bar.json "data.foo[x].name = data.bar.baz[y].id, data.foo[x]=output" | less

Or give people access to unix utilities from inside the repl

If I want to run multiple, redundant OPAs? Is there a way to replicate the OPA 'document store'?

The output for references that iterate complete sets should not include the binding for the reference itself. This is already handled for partial sets. When set literals were added, we forgot to update this part of the REPL. For example:

torin:~$ opa run
OPA 0.2.1-dev (commit 6228136-dirty, built at 2016-11-10T21:14:12Z)

Run 'help' to see a list of commands.

> p[1] :- true
> p[2] :- true
> p[x]
+---+
| x |
+---+
| 1 |
| 2 |
+---+
>
>
>
> q = {1,2} :- true
> q[x]
+---+------+
| x | q[x] |
+---+------+
| 1 | true |
| 2 | true |
+---+------+

$ cat test.rego
package foo

bar["a"] :- true

bar = {"a"}

$ opa run test.rego
OPA 0.2.1-dev (commit 47199b9, built at 2016-11-14T17:09:08Z)

Run 'help' to see a list of commands.

data.foo.bar
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x2eacb9]

goroutine 1 [running]:
panic(0x3fa040, 0xc4200100a0)
/usr/local/go/src/runtime/panic.go:500 +0x1a1
github.com/open-policy-agent/opa/topdown.evalRefRulePartialSetDocFull.func1(0xc420140780, 0x10da8, 0x80)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1522 +0x59
github.com/open-policy-agent/opa/topdown.evalContext(0xc420140780, 0xc420183440, 0xc42013f19f, 0xc42013f100)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:748 +0x206
github.com/open-policy-agent/opa/topdown.evalContext.func1.1(0xc420140700, 0x644101, 0xc42013f19f)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:772 +0x94
github.com/open-policy-agent/opa/topdown.evalExpr(0xc420140700, 0xc420183480, 0xc4201887b0, 0x28)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:848 +0x24b
github.com/open-policy-agent/opa/topdown.evalContext.func1(0xc420140700, 0xc42011c8e8, 0x1057e)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:773 +0xbb
github.com/open-policy-agent/opa/topdown.evalTermsRec(0xc420140700, 0xc420183460, 0xc4200de2c8, 0x0, 0x0, 0x0, 0x0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1773 +0x66b
github.com/open-policy-agent/opa/topdown.evalTermsRec(0xc420140700, 0xc420183460, 0xc4200de2c8, 0x1, 0x1, 0xc4200de2c8, 0x0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1795 +0x18e
github.com/open-policy-agent/opa/topdown.evalTerms(0xc420140700, 0xc420183460, 0xc4201887e0, 0x41d0c0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1720 +0x424
github.com/open-policy-agent/opa/topdown.evalContext(0xc420140700, 0xc420183440, 0xc42010c240, 0x1)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:786 +0x121
github.com/open-policy-agent/opa/topdown.evalRefRulePartialSetDocFull(0xc420140580, 0xc4201832a0, 0x3, 0x4, 0xc42013ee20, 0x2, 0x2, 0xc420189770, 0x660780, 0xc42011cd70)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1527 +0x29f
github.com/open-policy-agent/opa/topdown.evalRefRule(0xc420140580, 0xc4201832a0, 0x3, 0x4, 0xc420183340, 0x3, 0x3, 0xc42013ee20, 0x2, 0x2, ...)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1181 +0x2a5
github.com/open-policy-agent/opa/topdown.evalRefRec(0xc420140580, 0xc4201832a0, 0x3, 0x4, 0xc420189770, 0x0, 0xc42011cf18)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:936 +0x39d
github.com/open-policy-agent/opa/topdown.evalRef(0xc420140580, 0xc4201822b8, 0x0, 0x1, 0xc4201832a0, 0x3, 0x4, 0xc420189770, 0xc42013f2a0, 0xc4200de298)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:873 +0x205
github.com/open-policy-agent/opa/topdown.evalRef(0xc420140580, 0xc4201822b0, 0x1, 0x2, 0xc4201832a0, 0x3, 0x4, 0xc420189770, 0xc4200de298, 0x6809e0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:880 +0x3b2
github.com/open-policy-agent/opa/topdown.evalRef(0xc420140580, 0xc4201822a8, 0x2, 0x3, 0xc42013f2a0, 0x2, 0x2, 0xc420189770, 0xc420189770, 0x0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:880 +0x3b2
github.com/open-policy-agent/opa/topdown.evalRef(0xc420140580, 0xc4201822a0, 0x3, 0x4, 0xc4200de298, 0x1, 0x1, 0xc420189770, 0x10da8, 0x18)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:880 +0x3b2
github.com/open-policy-agent/opa/topdown.evalTermsRec(0xc420140580, 0xc4201831c0, 0xc420183108, 0x2, 0x3, 0x10, 0x409d00)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1785 +0x55a
github.com/open-policy-agent/opa/topdown.evalTermsRec(0xc420140580, 0xc4201831c0, 0xc420183100, 0x3, 0x4, 0x0, 0x1)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1795 +0x18e
github.com/open-policy-agent/opa/topdown.evalTerms(0xc420140580, 0xc4201831c0, 0xc420189680, 0x40bd80)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:1720 +0x424
github.com/open-policy-agent/opa/topdown.evalContext(0xc420140580, 0xc42013f230, 0xc4201831a0, 0x436560)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:786 +0x121
github.com/open-policy-agent/opa/topdown.Eval(0xc420140580, 0xc420183180, 0x1, 0xc4200e1040)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/topdown/topdown.go:380 +0xb6
github.com/open-policy-agent/opa/repl.(*REPL).evalTermSingleValue(0xc4200d60c0, 0xc4200e1040, 0xc4200de158, 0x1, 0x1, 0x1f2b49)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/repl/repl.go:640 +0x275
github.com/open-policy-agent/opa/repl.(*REPL).evalBody(0xc4200d60c0, 0xc4200e1040, 0xc4200de158, 0x1, 0x1, 0x1)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/repl/repl.go:494 +0x65b
github.com/open-policy-agent/opa/repl.(*REPL).evalStatement(0xc4200d60c0, 0x436560, 0xc4200dbce0, 0xc)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/repl/repl.go:473 +0x6d3
github.com/open-policy-agent/opa/repl.(*REPL).evalBufferOne(0xc4200d60c0, 0x0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/repl/repl.go:408 +0x152
github.com/open-policy-agent/opa/repl.(*REPL).OneShot(0xc4200d60c0, 0xc42013e360, 0xc, 0xc42013e300)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/repl/repl.go:153 +0x415
github.com/open-policy-agent/opa/repl.(*REPL).Loop(0xc4200d60c0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/repl/repl.go:105 +0x1c1
github.com/open-policy-agent/opa/runtime.(*Runtime).startRepl(0xc42002e078, 0xc420018720)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/runtime/runtime.go:158 +0xe2
github.com/open-policy-agent/opa/runtime.(*Runtime).Start(0xc42002e078, 0xc420018720)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/runtime/runtime.go:78 +0x8d
github.com/open-policy-agent/opa/cmd.init.1.func1(0xc42008efc0, 0xc420011ba0, 0x1, 0x1)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/cmd/run.go:68 +0x82
github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra.(*Command).execute(0xc42008efc0, 0xc420011b50, 0x1, 0x1, 0xc42008efc0, 0xc420011b50)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra/command.go:636 +0x443
github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x661220, 0x0, 0x661220, 0xc42004fef8)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra/command.go:722 +0x367
github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra.(*Command).Execute(0x661220, 0x0, 0x0)
/Users/tim/gocode/src/github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra/command.go:681 +0x2b
main.main()
/Users/tim/gocode/src/github.com/open-policy-agent/opa/main.go:12 +0x31

It'd be great if OPA could give more informative error messages when loading data from files. Below is the current behavior, where there's a problem with the bar.json file. If the error message could include the file that generated the error, that'd be helpful.

$ ./opa_darwin_amd64 run foo.rego bar.json baz.json
invalid character '}' looking for beginning of object key string

Evaluation encounters an index out of range error if the ref being evaluated is bound to another ref that has a longer ground prefix than the original ref. For instance:

{
    "a": {
        "b": {
            "c": {
                "d": "e"
            }
        }
    }
}

package ex

p = x :- data.a.b.c.d = x

q :- p, p

When the second ref to "p" is evaluated, the ground prefix is obtained from the binding for p (which is is data.a.b.c.d).

Example:

OPA 0.4.1-dev (commit 89d13d0-dirty, built at 2017-02-02T02:41:04Z)

Run 'help' to see a list of commands.

> data
panic: runtime error: index out of range

goroutine 1 [running]:
panic(0x478480, 0xc4200100f0)
	/usr/local/Cellar/go/1.7.5/libexec/src/runtime/panic.go:500 +0x1a1
github.com/open-policy-agent/opa/topdown.evalRefRecNonGround(0xc420188640, 0xc4201f8600, 0x3, 0x4, 0xc4201f6de0, 0x5, 0x5, 0xc4201f6db0, 0x4, 0x715700)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1220 +0x11ec
github.com/open-policy-agent/opa/topdown.evalRefRec(0xc420188640, 0xc4201f8600, 0x3, 0x4, 0xc4201f6db0, 0x1, 0x1)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1086 +0x2f9
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5b8, 0x0, 0x1, 0xc4201f8600, 0x3, 0x4, 0xc4201f6db0, 0x715680, 0x4)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:998 +0x5a0
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5b0, 0x1, 0x2, 0xc4201f8600, 0x3, 0x4, 0xc4201f6db0, 0xc4201f84c0, 0x344101)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1022 +0x1bc
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5a8, 0x2, 0x3, 0xc4201ed360, 0x2, 0x2, 0xc4201f6db0, 0x49c700, 0x1)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1022 +0x1bc
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5a0, 0x3, 0x4, 0xc4200b4848, 0x1, 0x1, 0xc4201f6db0, 0x0, 0xc4201bb788)

Creating a ticket to track work needed to move the REST API models over to snake_case.

Example:

torin:~$ docker run -it --rm openpolicyagent/opa:0.4.0
OPA 0.4.0 (commit 4ea2b27, built at 2017-01-25T18:34:56Z)

Run 'help' to see a list of commands.

> input = 1
error: 1:1: missing input document

The problem is that the statement (input = 1) is compiled as a query before checking if it's parse-able as a rule. The compile step fails and so an error is printed.

The REST API doesn't allow callers to load dependent modules one-by-one:

test1.rego:

package ex

import request.req1

x :- req1 > 1

test2.rego:

package ex

y :- x with request.req1 as 1000

Example:

torin:~$ curl -X PUT localhost:8181/v1/policies/test1 --data-binary @test1.rego
{
  "ID": "test1",
  "Module": {
    "Package": {
      "Path": [
        {
          "Type": "var",
          "Value": "data"
        },
        {
          "Type": "string",
          "Value": "ex"
        }
      ]
    },
    "Imports": null,
    "Rules": [
      {
        "Name": "x",
        "Value": {
          "Type": "boolean",
          "Value": true
        },
        "Body": [
          {
            "Index": 0,
            "Terms": [
              {
                "Type": "var",
                "Value": "gt"
              },
              {
                "Type": "ref",
                "Value": [
                  {
                    "Type": "var",
                    "Value": "request"
                  },
                  {
                    "Type": "string",
                    "Value": "req1"
                  }
                ]
              },
              {
                "Type": "number",
                "Value": 1
              }
            ]
          }
        ]
      }
    ]
  }
}torin:~$ curl -X PUT localhost:8181/v1/policies/test1 --data-binary @test2.rego
{
  "Code": 400,
  "Message": "error(s) occurred while compiling module(s), see Errors",
  "Errors": [
    {
      "Code": 2,
      "Location": {
        "File": "test1",
        "Row": 3,
        "Col": 1
      },
      "Message": "y: x is unsafe (variable x must appear in the output position of at least one non-negated expression)"
    }
  ]
}

However, if we run OPA and pass the modules via the command line, everything works fine:

Run OPA:

./opa_darwin_amd64 run -s --v=3 --logtostderr=1 ~/test1.rego ~/test2.rego

List policies:

torin:~$ curl localhost:8181/v1/policies --silent | jq '.[].ID'
"/Users/torin/test1.rego"
"/Users/torin/test2.rego"

Query for data:

torin:~$ curl localhost:8181/v1/data
{"ex":{"y":true}}

Didn't expect the following results. Bug?

re_match("d*", "cat")
true

re_match("ec2*", "ec3")
true

The parser panics when the "request" parameter is not set to a value.
http://localhost:8181/v1/data?request

2017/01/16 09:28:36 http: panic serving 127.0.0.1:42608: runtime error: index out of range
goroutine 45 [running]:
net/http.(*conn).serve.func1(0xc4201b0380)
/usr/local/go/src/net/http/server.go:1491 +0x12a
panic(0x85a3e0, 0xc42000e110)
/usr/local/go/src/runtime/panic.go:458 +0x243
github.com/open-policy-agent/opa/server.parseRequest(0xc4202be310, 0x1, 0x1, 0x7, 0xc4201b5aa8, 0xf0, 0xc4202de960, 0x0)
/go/src/github.com/open-policy-agent/opa/server/server.go:1074 +0x80a
github.com/open-policy-agent/opa/server.(*Server).v1DataGet(0xc42000ae60, 0xadfce0, 0xc4202b43a0, 0xc4202de960)
/go/src/github.com/open-policy-agent/opa/server/server.go:471 +0x229
github.com/open-policy-agent/opa/server.(*Server).(github.com/open-policy-agent/opa/server.v1DataGet)-fm(0xadfce0, 0xc4202b43a0, 0xc4202de960)
/go/src/github.com/open-policy-agent/opa/server/server.go:310 +0x48
net/http.HandlerFunc.ServeHTTP(0xc420184590, 0xadfce0, 0xc4202b43a0, 0xc4202de960)
/usr/local/go/src/net/http/server.go:1726 +0x44
github.com/open-policy-agent/opa/vendor/github.com/gorilla/mux.(*Router).ServeHTTP(0xc42000aeb0, 0xadfce0, 0xc4202b43a0, 0xc4202de960)
/go/src/github.com/open-policy-agent/opa/vendor/github.com/gorilla/mux/mux.go:114 +0x10d
github.com/open-policy-agent/opa/runtime.(*LoggingHandler).ServeHTTP(0xc420185e50, 0xae05e0, 0xc4201bc1a0, 0xc4202de780)
/go/src/github.com/open-policy-agent/opa/runtime/logging.go:30 +0xf8
net/http.serverHandler.ServeHTTP(0xc42001b700, 0xae05e0, 0xc4201bc1a0, 0xc4202de780)
/usr/local/go/src/net/http/server.go:2202 +0x7d
net/http.(*conn).serve(0xc4201b0380, 0xae0da0, 0xc4202b8280)
/usr/local/go/src/net/http/server.go:1579 +0x4b7
created by net/http.(*Server).Serve
/usr/local/go/src/net/http/server.go:2293 +0x44d
2017/01/16 09:28:36 http: panic serving 127.0.0.1:42610: runtime error: index out of range
goroutine 46 [running]:
net/http.(*conn).serve.func1(0xc4201b0480)
/usr/local/go/src/net/http/server.go:1491 +0x12a
panic(0x85a3e0, 0xc42000e110)

People find it annoying when their REPL sessions come to an abrupt stop.

It would be nice if ctrl+c cleared the line and ctrl+d presented a prompt asking if the user wants to quit.

Errors returned by the API currently use integer codes as type hints.

The reason for this is simply that Go makes it relatively easy to define a suite of error codes as integers. It would be nicer if the errors returned in the REST API were human readable, e.g., instead of {"code": 1, ...} the error was {"code": "rego_runtime_error", ...} etc.

JSON files can be loaded on startup (e.g., opa run one.json two.json). Today, OPA requires that these files contain an object so that they can be merged and stored at the root of the data store. In some cases it would be helpful if (1) the files did not have to contain objects and (2) the documents in the files could loaded at specific locations in the data hierarchy. For example:

opa run foo.bar:one.json two.json

The above command would load the one.json file at /data/foo/bar. two.json would be loaded normally.

Sometimes I find myself writing virtual-documents that are technically unsafe but are safe the way I use them. It would be nice if we could declare a virtual-doc unsafe, and have the evaluation work properly so long as the document is always provided with ground arguments when evaluated.

For example, suppose we are declaring which API paths are permitted.

# permit all subpaths of 'foo' that are not explicitly prohibited
permit :- 
    request.path[0] = "foo", 
    not prohibited[request.path]

prohibited[path] :-
    path[0] = "bar",
    path[1] = "baz"

Here 'prohibited' is unsafe, but because we will only ever be evaluating 'permit', we see that 'prohibited' will never be problematic for evaluation. If something ever did ask for 'prohibited' directly, we could return an error message.

We can think of these unsafe virtual-docs as user-written builtins. Their arguments (or maybe more generally some of their arguments) must be ground before evaluation.

It would be nice if OPA could load YAML files provided via the command line the same way it handles JSON and Rego files.

The topdown and storage modules should be updated to accept a Context argument. The Context should be propagated all the way down to storage plugins.

Note, because the name "context" is heavily associated with the Golang construct, we should strongly consider renaming topdown.Context to avoid confusion.

OPA is returning 200 OK when evaluating queries that depend on an input document even if an input document has not been supplied. Per the documentation, this should result in a 400 Bad Request response.

Update documents to reflect required indentation with package definitions for the REST examples
http://www.openpolicyagent.org/documentation/references/rest/

working example:
package opa.examples
<white space>import request.example.a

failing example:
package opa.examples
import request.example.a
error output:
{ "Code": 400, "Message": "opaexample:1: expected rule (request.example.a must be declared inside a rule)" }

If a rule head does not contaIn vars, evaluation should stop after it finds the first proof for the body. Currently, evaluation/topdown continues to find all proofs. For instance:

OPA 0.4.0 (commit 4ea2b27, built at 2017-01-25T18:34:39Z)

Run 'help' to see a list of commands.

> p :- a = [1,2,3], a[_] > 0
> p
true
> trace
> p
Enter eq(data.repl.p, _)
| Eval eq(data.repl.p, _)
| Enter p = true
| | Eval eq(a, [1, 2, 3])
| | Eval gt(a[_], 0)
| | Exit p = true
| Redo p = true
| | Redo gt(1, 0)
| | Exit p = true
| Redo p = true
| | Redo gt(2, 0)
| | Exit p = true
| Exit eq(data.repl.p, _)
true

A pattern I ran across is to

1. Define virtual doc p as set S
2. Define virtual doc q as a subset S1 of S
3. Define virtual doc r as S - S1

Step 3 is error-prone because we need to write logic for the body of r that is the negation of the logic in the body of rules for q. And whenever I update the rules for q I need to modify the rules for r (sometimes significantly).

It would be nice if I could write something like the following..
p[x] :- ...
q[x] :- p[x], ...
r = setdifference(p, q)