This commit adds a new API endpoint to fetch OPA's
active configuration. When the discovery feature is enabled,
this API can be used to fetch the discovered configuration
in the last evaluated discovery bundle.
Fixes: #2020
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
This commit refactors how the server manages the bundle revisions that
are included in decision logs and provenance results for the
API. Previously the revisions were cached on the server struct,
outside of the store. The server would read the revisions from the
store on reload() to keep them consistent.
While it is more performant to keep the revisions cached outside of
the store, it requires that the server perform reads on the
transaction that has already been committed. The inmem store
implementation allows for this however it's not going to be possible
to support that with other transaction implementations in the future.
This commit updates the server to simply read the revisions out of the
store in the handlers that require them. This adds a small amount of
overhead to the handlers that wasn't present before however in
practice this is not a concern (the overhead measured on my machine
was approx. 5 microseconds compared to the entire server handler that
was taking approx. 75 microseconds.)
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Policy IDs will be decoded in GET, PUT, and DELETE requests to the
/policies endpoint. This will enable users to include non-alphanumeric
characters in policy IDs, as well as leading forward slashes, by URL
encoding the path component of their requests.
Fixes#2116
Signed-off-by: Matthew Mahnke <mmahnke18@gmail.com>
* Authorization policy with deny reason
Currently the authorization policy only returns boolean result. There
are no way to give more context to denied requests. This adds the ability
to policies to return a reason for denying the request.
Applying backward compatible policy evaluation logic.
Fixes#3056
Signed-off-by: Ajanthan Balachandran <balaajanthan@gmail.com>
* docs/security:: make object-response the extra, not the default; update tests
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Co-authored-by: Ajanthan Balachandran <balaajanthan@gmail.com>
This change adds a server_handler timer to the metrics for any POSTs to /v1/compile.
So `curl -X POST localhost:8181/v1/compile?metrics ...` will result in:
"metrics": {
"timer_rego_partial_eval_ns": 145020,
"timer_rego_query_compile_ns": 86415,
"timer_rego_query_parse_ns": 56104,
"timer_server_handler_ns": 377557 #this line is new
}
Fixes#3096.
Signed-off-by: Jakob Schmid <jakob.schmid@sap.com>
InterQueryBuiltinCacheConfig now responds to the plugin manager's reconfigure event, which allows cache config to exist in discovery config. Previously, cache config would be ignored if it was only declared in discovery config.
Related to #2978.
Signed-off-by: Grant Shively <gshively@godaddy.com>
Fixes#3000.
The assertions on the response metrics object should be enough to
cover the bug -- depending on what is happening during eval, the
keys of that object may differ. (E.g. if there's a ref to be resolved,
that operation is timed; if there are none, there's no timer data.)
Small change to test/e2e: close some request bodies
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit updates the server's basic authorizer to include the
deserialized message body in the input to the authorization policy so
that the latter can make decisions based on policy query input
documents. The authorizer caches the parsed message body on the
request context and the server retrieves the value to avoid parsing twice.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
We can set the resolvers on the base rego objects (which avoids
having them reloaded from the store) and they get propagated to any
prepared evals automatically.
Signed-off-by: Patrick East <east.patrick@gmail.com>
This plumbs through metrics to the wasm evaluation, adding several
new timers. They will show up when using a Wasm bundle with any of the
usual evaluation mechanisms (eg opa eval, bench, server requests etc)
Signed-off-by: Patrick East <east.patrick@gmail.com>
The wasm binaries support >1 entrypoint per module, this makes changes
to reflect that in the various data structures we keep references to
the modules and resolvers, mapping them to entrypoints.
Signed-off-by: Patrick East <east.patrick@gmail.com>
This is largely plumbing changes required to get Wasm modules loaded
from bundles and configured as external resolvers for evaluations.
Signed-off-by: Patrick East <east.patrick@gmail.com>
This commit removes the deprecated watch package and server
feature. This feature was never adopted and since the implementation
was not incremental, it would inevitably encounter
performance/scalability issues.
Fixes#2265
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
This commit adds a new inter-query cache that built-in
functions can use to cache responses across queries.
The OPA config includes a new "caching" field that can be used
to set the size of the cache. By default there is no limit.
This change also updates `http.send` to optionally utilize the
inter-query cache.
Fixes#1753
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
At one point the in-memory store implemented an indexing strategy so
that variable bindings could be returned for non-ground references to
base documents. However, we eventually disabled in-memory indexing
and we have not re-added it since
3ebbeede6c. At this point, indexing can
be removed completely.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
There was a potential race with initializing a listener and saving the
reference to it and using the reference when checking on the address
for the listener.
This change is doing two things:
* Getting the address from the listener a single time _after_ it has
been initialized.
* Using a mutex to protect access to the address (common use-case
being to start the listener on a background thread and check for
the address it is listening on from another one).
`go test -tags=slow -race -vet=off ./cmd/` is now passing with these
changes.
Signed-off-by: Patrick East <east.patrick@gmail.com>
This will deprecate the older API's that used the `topdown.Tracer` in
favor of the newer `topdown.QueryTracer` interface. Usages of the old
API have been swapped, although some of the testing is left with them
to ensure we still support them (until we remove the deprecated API).
Signed-off-by: Patrick East <east.patrick@gmail.com>
Previously the rego package would construct rules for each of the
PE query results when PartialResult() was invoked--however, it would not
check if those rules would be recursive. If the user queried for
`data` (or `data.<partialnamespace>`) then PE would return an
(essentially) unmodified copy of the query and the rego package would
happily construct a rule from it--since the rule is namespaced under
data this leads to a recursion error.
The problem is that the recursion error was caught by running the
compiler--however, since the compiler is shared by the server and
other components and since compile operations do not rollback changes
on error, this approach left OPA in an inconsistent state. Some of the
compiler data structures like the rule tree would include the PE
result but any structures built by stages after the recursion check
would be incomplete. This causes issues for the evaluator because it
(rightfully) assumes that the compiler data structures are consistent.
Since we can assume that PE results are valid and do not contain
semantic errors and we do not intend to support the lazy PE API in the
future, this commit fixes the issue/panic by modifying the rego
package to check for recursion in the rules that it constructs. This
is relatively simple since it merely has to check for prefixes in the
refs contained in the PE query result. If recursion is caught, the
rego package returns an error signalling that PE was ineffective. In
this case, the server just falls back to normal eval.
Fixes#2197
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
The location info on rules parsed from exprs was not ideal--it was
being set to the value of the rhs term (which is not accurate.)
This commit updates the parser helper function to reset the location
to the original expr location.
This commit also fixes an issue in the parser helper test case that
was introduced in 49a963f16a when we
used a tool to roundtrip the policy examples in the OPA codebase
to move away from the old datalog if-then syntax (":-").
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
If there was a nil status value it was still being added to the
status map, but the server didn't check before dereferencing it.
We now just check if the pointer is nil before trying to use it.
Fixes: #2396
Signed-off-by: Patrick East <east.patrick@gmail.com>
This adds a new config option for the OPA server (along with plumbing
from `opa run` downward to the server) to configure separate
diagnostic addresses to listen on. These will only be configured to
serve the /metrics and /health.
This will allow for more secure OPA deployments with the normal "data"
or "policies" API's made to be only accessible on localhost.
Fixes: #2002
Signed-off-by: Patrick East <east.patrick@gmail.com>
This test case does not need to have the manager started and since the
manager now performs at least one read on start, the mock used in the
test case would cause the test to fail.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
This commit fixes another data race found by running go test
-race. The server was reading state modified by the trigger handler
without using a mutex or being inside of a transaction.
Note, we should think about how we can move away from relying on the
server transactions to implement critical sections in the server as
this is bound to bite us in the future.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Recently we improved the trace pretty printing to include location
information on events. Unless there's a good reason we should use this
tracer printing throughout.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Any time we do a compilation in the Rego object (or helpers) we need
to be careful to setup a conflict check that is valid for the current
context (both literal golang ctx and current storage transaction).
This isn't much of a concern if the Rego instance owns the compiler,
but if an external one was provided we need to be careful to update
the conflict check before compiling.
This was already doing the "right thing" when activating bundles, but
for partial evaluation results that were being updated on the compiler
it was not.
Fixes: #2197
Signed-off-by: Patrick East <east.patrick@gmail.com>
Previously we let it use the default namespace, which meant that
every cached evaluation would use the same query on the compiler..
which isn't correct. They need to be unique per path.
We'll now use a hash of the path (since it needs to be a valid var).
While doing this the logic for the Rego opts was refactored in
`makeRego` to only define the list a single time.. this should help
reduce the risk of any regressions in the future.
Fixes: #2247
Signed-off-by: Patrick East <east.patrick@gmail.com>
Previously we would iterate over all modules on the compiler and then
look them up by id in the store. This causes problems when there are
partial compilation results on the compiler that have not been put in
the store.
To correct this we will now reverse how it iterates, instead iterating
over a list of policies from the store and looking up their AST from
the compiler as needed.
Fixes: #2036
Signed-off-by: Patrick East <east.patrick@gmail.com>
This commit replaces the existing PEG generated parser with a parser
implemented by hand. The new parser is more efficient (avoiding old
problems with pathological input cases like {{{{{{{{{}}}}}}}} and
deeply-nested composites in general) and offers better opportunities
for improved error reporting (which has been improved already but
there is still room to grow.)
During the test process of implementing the new parser, we identified
a few issues that were present in the old parser. Those issues are
fixed by this commit.
Fixes#1251Fixes#501Fixes#2198Fixes#2199Fixes#2200Fixes#2201Fixes#2202Fixes#2203
Co-authored-by: Torin Sandall <torinsandall@gmail.com>
Co-authored-by: Patrick East <east.patrick@gmail.com>
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Signed-off-by: Patrick East <east.patrick@gmail.com>
This deprecates `/health?bundle` in favor of (the plural)
`/health?bundles`.
Both have the same API result, taking into account *all* bundle
statuses. This just lessen any confusion with regards to whether
`bundle` means a single bundle or all. It fits better with the new
`plugins` option.
Signed-off-by: Patrick East <east.patrick@gmail.com>
The health check now supports a `?plugin` option which will make the
response depend on whether or not all configured plugins are in an OK
state.
The `bundle` parameter will now use the bundle *and* discovery plugin
statuses to determine if the bundles are ready. This corrects an issue
where discovery bundles, and bundles defined by the discovery dynamic
config, were not included with `/health?bundle=true` checks.
The URL parameter parsing has also changed to allow for omitting the
value for the `bundle` option. It will default to `true` so that
`/health?bundle=true` can be shortened to `/health?bundle`.
Fixes: #2010Fixes: #2015
Signed-off-by: Patrick East <east.patrick@gmail.com>
The logger was swapping `.`'s with `/`'s but this isn't safe when a
valid path should be /foo/a.b.c/main. The server was already doing
the right thing by passing in the url path where applicable, or only
specifying a query instead of the path.
This might affect anyone using the decision logger golang API passing
in something in dot-notation and expecting it to come out with paths.
Anyone using the HTTP server should be unaffected.
Fixes: #2031
Signed-off-by: Patrick East <east.patrick@gmail.com>
Certain portions of Rego cannot be easily inlined when they depend on
unknowns (e.g., comprehensions). In the past, if PE encountered
statements that could not be inlined, PE would simply save the
original statement regardless of whether that statement (or any of
it's dependencies) depended on unknowns. This was the safest way to
introduce PE initially and worked in many cases as long as authors
were aware of the fragment of Rego that could be PE-ed. Of course,
this places a burden on authors and also fails when constructs like
comprehensions are required.
This commit enhances the evaluator so that PE will evaluate all terms
(e.g., comprehensions, references to full extent of partial sets/objects,
etc.) as long as they do not depend on unknowns. This commit also
enhances the evaluato to support PE on expressions that include with
statements.
Support for with statements is handled by disabling inlining on any
references that are contained in the expression modified by the with
statement. This approach was chosen over #1956 because while it
generates support rules it avoids the need to re-apply with statements
to inlined expressions.
This commit also addresses #1417 because negated expressions that can
be completely evaluated will be now.
As part of this change, the evaluator has been refactored to maintain
the inlining controls as a stack (this makes with statements easier to
handle.)
name old time/op new time/op delta
InliningFullScan/1000-8 5.89ms ± 0% 5.92ms ± 3% ~ (p=0.548 n=5+5)
InliningFullScan/10000-8 65.8ms ± 2% 65.1ms ± 0% ~ (p=0.095 n=5+5)
InliningFullScan/300000-8 2.06s ± 2% 2.07s ± 5% ~ (p=0.841 n=5+5)
name old alloc/op new alloc/op delta
InliningFullScan/1000-8 2.68MB ± 0% 2.68MB ± 0% +0.01% (p=0.008 n=5+5)
InliningFullScan/10000-8 27.4MB ± 0% 27.4MB ± 0% +0.00% (p=0.008 n=5+5)
InliningFullScan/300000-8 825MB ± 0% 825MB ± 0% ~ (p=0.087 n=5+5)
name old allocs/op new allocs/op delta
InliningFullScan/1000-8 81.0k ± 0% 81.0k ± 0% +0.01% (p=0.008 n=5+5)
InliningFullScan/10000-8 810k ± 0% 810k ± 0% +0.00% (p=0.008 n=5+5)
InliningFullScan/300000-8 24.3M ± 0% 24.3M ± 0% +0.00% (p=0.008 n=5+5)
Fixes#1069Fixes#653Fixes#1421Fixes#1417
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
We had a number of helper methods that would start a timer, defer
stopping it, and then do something to setup a Rego object for doing
its thing.
Many of these would have a check and then skip the step, but the timer
would get a little bit of time accounted for it anyway for the `if`
check and handling the deferred stop function.
This refactors as many of these as I could find to do the if check
and shortcut out before starting the timer. This will prevent any of
the "no-op" steps from showing up in metrics.
Signed-off-by: Patrick East <east.patrick@gmail.com>
The metric was using `metrics.RegoQueryParse` but it isn't actually
doing the query parse there. It is parsing the input document.
Signed-off-by: Patrick East <east.patrick@gmail.com>
This adds in caching of prepared queries for versioned and unversioned
data queries (POST/GET to `/`, `/data`, and `/v1/data`).
The cache has a max size of 100 and just starts acting as a circular
buffer for queries (FIFO, no smarts for LRU caching or anything). It
seems like it would be unlikely for these API's to hit the cache max
size. Most OPA use-cases have a single query that is re-used over
and over with different inputs.
There is a new metric `counter_server_query_cache_hit` which will
show whether or not a request used the query cache or not. It is there
primarily to help explain away why sometimes a handful of the other
metrics aren't there (the query parse/compile/etc).
In the future this could be added to the query API's too. This change
does not touch anything other than the "data" API's.
Closes: #1567
Signed-off-by: Patrick East <east.patrick@gmail.com>
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: #1726Fixes: #1724
Signed-off-by: Patrick East <east.patrick@gmail.com>
The issue was that with bundles loaded from the file system we would
not initialize the mutex used for checking bundle status.
This fixes the initialization and prevents the error. Health status
works as expected now.
Fixes: #1703
Signed-off-by: Patrick East <east.patrick@gmail.com>
Previously the server didn't wrap the error messages which made it
hard to determine the source of internal errors coming back from OPA
when deployed with a custom decision logger.
Fixes#1367
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
* Remove metric provider config to avoid introducing new public
interfaces. Since there is only one provider (prometheus) and it
doesn't have any configurable settings, remove the configuration
changes for now. We can always add these in the future.
* Remove dummy metric provider implementation. This isn't needed now
that we're using the metrics.Metrics interface instead of
metrics.GlobalMetrics.
* Remove metrics.GlobalMetrics in favour of metrics.Metrics. Move the
HTTP handler instrumentation interfaces into the server package to
avoid coupling the metrics package to the net/http package.
* Refactor the prometheus provider to implement the metrics.Metrics
interface. Since the prometheus registry can error on Gather()
calls, the provider has been updated to accept a logger and use ti
when the Gather() call fails. This doesn't affect any public
interfaces so it can be revisited in future if needed. Alteratnively
we could add a Gather() interface onto metrics.Metrics which could
return the error.
* Refactor status plugin to include metrics in status update by
default. Users implementing the status API are likely to need
performance metrics to gauge the OPA's health. Moreover if they are
implementing the status API it's unlikely they will want to poll the
/metrics endpoint on the OPA HTTP API (which may not even be
exposed.)
* Move the prometheus endpoint test case into the e2e package so the
server package has no dependencies on prometheus anymore.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Ashutosh Narkar