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>
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>
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>
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>
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>
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>
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>
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>
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>
Prometheus metrics can give much of insight into OPA's health.
Run-time metrics are a natural part of the application state
so having them in status update seems like a right change
that can help server understand what's going in with the OPA
instance.
The commit also encapsulates all prometheus-related code in one package
and abstracts it with generic interface so that it would be possible
to add other metrics providers
Addresses #1606
Signed-off-by: Stan Lagun <stan@styra.com>
This commit removes the deprecated diagnostic feature from the
server. The feature has been deprecated since November 2018 and it was
essentially unused at the time so it should be safe to
remove. Removing the diagnostic support from the server saves having
to perform an extra policy evaluation in the server.
Once the buffer is removed from the runtime.Params struct the related
issue can be closed (there is still one known user of that so it has
been left intact for backwards compatibility.)
Ref #1052
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
This change brings in support for multiple bundles to be downloaded
and activated OPA.
This is enabled by using the new config option `bundles` to define
the bundles, and deprecates the older `bundle` option.
The new `bundles` keyword and structure is propagated through to the
decision logs, status API, provenance, stored manifests, etc. Check
out the doc changes for all the updated structures.
That being said any existing configuration using `bundle` will *not*
see the new structure, everything is intended to be backwards
compatible (almost to a fault).
Fixes: #721
Signed-off-by: Patrick East <east.patrick@gmail.com>
This will allow others consuming OPA as a library to have easier
access to manage/query the bundle manifests.
Signed-off-by: Patrick East <east.patrick@gmail.com>
This patch moves the check for unsafe built-ins from the server to the
compiler, so it can be used by the Go API as well as the HTTP API.
This was previously discussed in #1570.
Signed-off-by: Jasper Van der Jeugt <jasper@fugue.co>
To do this we needed a way to get the actual address that was bound.
To do that we needed to refactor the server and runtime code a tad
to let us create our own `net.Listener`s and get their address _after_
they had been started. The code is pretty much 1:1 with what is in the
official `http` package.
Now when the tests run using the helpers to stand up server runtimes
they should all be on separate ports.. in theory we could run the
unit tests in parallel without concern (for the e2e parts anyway).
Fixes: #1533
Signed-off-by: Patrick East <east.patrick@gmail.com>
The server was leaking the write transaction when parsing failed which
caused subsequent updates to block. In future we should refactor the
handler implementations to avoid having to manually abort on each
error path.
Fixes#1478
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
There is a new parameter for /health REST API which will include the
Configured bundle activation in the response. Example:
GET /health?bundle=true HTTP/1.1
Without the parameter the behavior stays the same, with it the server
will respond with 500’s until the status has been updated with an
activation time.
The docs for kubernetes ready probe has been updated to show this as
it makes for a better ready check than the original behavior when
remote bundles are being used.
Fixes: #1153
Signed-off-by: Patrick East <east.patrick@gmail.com>
Add the new 'provenance' query parameter to the v1DataGet() and
v1DataPost() routines.
Set the fields of the DataResponseV1 struct with the version/build
values for possible return with the result of the data operations.
Add a unit test by setting version info, than ensuring we get the
strings in the response. Note that the revision is left unset.
Signed-off-by: Peter W. Morreale <pmorreale@statestreet.com>
These changes update the server to compare the body of the HTTP
request with the existing policy in the store. If the bytes are equal,
the server returns immediately.
These changes are being made to workaround poor parser performance
given deeply nested JSON objects that are commonly found in Kubernetes
use cases. Specifically, we've observed very high parse times (e.g.,
over a second) when processing policies in production
environments. The problem becomes worse when kube-mgmt is being used
because it periodically resyncs the policies causing load even if the
policies are not changing.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
To make this work it adds in a new shim interface for the server
to interact with the `http.Server`(s). We mock one out and can then
have it return errors on `Shutdown`.
Signed-off-by: Patrick East <east.patrick@gmail.com>
There is a new CLI option to configure the grace period:
`--shutdown-grace-period`
The option defaults to 10 seconds.
When a SIGINT or SIGTERM is sent the runtime will catch them and
attempt to gracefully shutdown the http servers. If the timeout is reached
it will log an error and continue with exiting (in a less graceful way).
Unit testing is a little bit light on this. Its touching some parts of the
code that aren't super easy to mock out. We can probably refactor
things in the future to make it easier.
Fixes: #1291
Signed-off-by: Patrick East <east.patrick@gmail.com>
In the Data API, the `path` attribute of `PATCH` operations was
not handling escaped `~` and `/` values (escaped as `~0` and `~1`,
respectively) according to the JSON-Pointer RFC.
The API was using (and continues to support) URL escaping of these
characters (so `%7E` and `%2F`, respectively), but this was not
documented and is not strictly correct according to the documentation.
However, this is being left unchanged to preserve backwards compatibility
for anyone who stumbled across this behaviour.
With this change, `~0` and `~1` in the `path` attribute of `PATCH`
operations will be unescaped into `~` and `/` as defined in the RFC.
Signed-off-by: Geoff Baskwill <me@geoffbaskwill.ca>
Previously, the decision logger interface did not allow plugin
implementations to return an error. In some cases, implementations may
prefer to make OPA fail-closed if the event cannot be emitted.
This is a backwards incompatible change to the custom decision logger
API that was added in v0.10.3 and it deprecates the old diagnostic
interface as well.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Previously the build version was recorded in the version package and
then different components would report it in an ad-hoc manner, e.g.,
the REPL has a module that generates a virtual doc with the version
info in it, the server was using templating to do the same, etc.
These changes remove the special code from the REPL and server
implementations to report the version. Instead the runtime writes the
version into /system/version at boot.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
This change appends a newline to pretty printed API results to improve readability on the command line. Also, fix tests that were asserting on string equality instead of JSON value equality.
Signed-off-by: Josh Marshall <joshua.r.marshall.1991@gmail.com>
These changes update the server to check the bundle scope before
writing any policy and data. This way the caller won't accidentally
write policy and data into OPA only to have it overwritten the next
time a bundle is downloaded and activated.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Previously there were no checks in place to ensure that base and
virtual documents do not overlap. As a result, if users loaded raw
JSON and rules into OPA that overlapped, the evaluation results were
not well defined. With these changes, we can detect the overlap and
reject updates (to policies or data) that would cause inconsistent
results.
Fixes#1207
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Matthew Mahnke