This commit fixes a request handling bug introduced in #6868, which
caused OPA to treat all incoming chunked requests as if they had
zero-length request bodies.
The fix detects cases where the request body size is unknown in the
DecodingLimits handler, and propagates a request context key down to
the `util.ReadMaybeCompressedBody` function, allowing it to correctly
select between using the original `io.ReadAll` style for chunked
requests, or the newer preallocated buffers approach (for requests of
known size).
This change has a small, but barely visible performance impact for large
requests (<5% increase in GC pauses for a 1GB request JSON blob), and
minimal, if any, effect on RPS under load.
Fixes: #6904
Signed-off-by: Philip Conrad <philip@chariot-chaser.net>
This commit introduces a few major changes:
- (Breaking change) Limits now exist for maximum request body sizes.
- Buffers are preallocated for reading request bodies.
- Buffers are preallocated for decompressing request bodies.
- Gzip decoder instances are reused in a `sync.Pool` across requests.
The effect on garbage collection is dramatically fewer GC pauses, giving
a roughly 9% RPS improvement in load tests with gzipped request bodies.
For larger request sizes, the number of GC pauses is dramatically
reduced, although the peak pause time may increase by a few percent.
Implementation notes:
- The DecodingLimits handler enforces the max request body size both
through a Content-Length check, and a MaxBytesReader wrapper around
the payload.
- The DecodingLimits handler passes the gzip payload size limit down
using a context key.
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
This PR fixes an issue where an OPA running authorization policies would
be unable to handle gzipped request bodies.
Example OPA CLI setup:
opa run -s --authorization=basic
Example request:
echo -n '{}' | gzip | curl -H "Content-Encoding: gzip" --data-binary @- http://127.0.0.1:8181/v1/data
This would result in unhelpful error messages, like:
```json
{
"code": "invalid_parameter",
"message": "invalid character '\\x1f' looking for beginning of value"
}
```
The cause was that the request body handling system in the
`server/authorizer` package did not take gzipped payloads into
account. The fix was to borrow the gzip request body handling function
from `server/server.go`, to transparently decompress the body when
needed.
Fixes: #6804
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
It would be useful if users had the ability to enhance the
decision log with info from the incoming HTTP request such as
headers. This change allows users to configure headers whose
values if present in the incoming HTTP request would be
surfaced via the decision log. This can be extended in the
future to include more context from the request.
Fixes: #6693
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
This change attempts to keep the default decision path used by the server
in sync with the one defined on the manager's config. Currently the
server only updates the default decision path when it's initialized and
when there is a commit on the store. The issue happens when the default
decision path is updated via the discovered config. In this case, the
manager's config is updated but there could be no store txn. Hence
the updated value of default decision path is not taken into account by
the server.
Fixes: #6697
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
In workflow runs like this:
https://github.com/open-policy-agent/opa/actions/runs/7803493290/job/21283458848#step:3:317
We can see two problems. This commit is meant to address them.
First, the test failed with this message:
```
expected unknown certificate authority error but got: Get "https://127.0.0.1:38699/v1/data": write tcp 127.0.0.1:52786->127.0.0.1:38699: write: connection reset by peer
```
Now this step in the test is retried like the other steps in the test
since it can fail too.
Second, the error `failed to reload TLS config` appears many times in
the logs for that test. This issue is caused by the server attempting to
read the new cert, key, and CA contents from disk while they are still
being written to. This PR also introduces a 100ms pause between upto 5
attempts to reload the config for any given change to the state on disk.
This should mean that the error is seen only when is is actually an
issue and the reload has failed after a reasonable time. In most cases,
running locally, the reload happens without error on the first run.
Signed-off-by: Charlie Egan <charlie@styra.com>
The partial query parameter has been marked as deprecated since
v0.23.0. It's also removed from the docs since that time.
Fixes: #2266
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
http_request_duration has fixed, hardcoded number of buckets with no possibility to tweak them
For cases when the most of the latencies are above 1ms, with only 4 available buckets there's no good insight on OPA's performance.
This implementation:
- adds the possibility for the buckets to be configurable in ```server.metrics.prom.http_request_duration_seconds.buckets``` key
- it's not a breaking change, if the buckets are not present in the configuration, the metric is configured with the existing values as a fallback
Signed-off-by: aarnautu <aarnautu@adobe.com>
This test case exercises the scenario when a bundle is being written
to the store (ie. active write transaction) and the OPA server handling
a policy eval request at the same time. The former should not block
the later.
Fixes: #4792
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Currently we parse store modules irrespective of whether
there are modules on the Rego object. This will result in
the compilation of those modules which triggers the bundle
activation flow. Now as part of the module compilation
we interact with the compiler's modules and run compilation
on the input modules. If let's say there are concurrent health
check requests (ie. /v1/health), this could result in a race
during the compilation process while working with the compiler's
modules.
This change avoids this situation by skipping parsing of the store
modules when none are set on the Rego object. The assumption this
change makes is that while using the rego package the compiler and
store are kept in-sync.
Fixes: #5868
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Currently when OPA's HTTP server rejects requests per
the authz policy, this is not accounted for via the management APIs.
This change adds that count in the metric registry that is
part of the Status API for more visibility.
Fixes: #3378
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
It is available for the following REST API endpoints:
- GET & POST HTTP methods on /v0/data & /v1/data endpoints
- POST HTTP method on /v1/compile endpoint
HTTP clients can optionally:
- send 'Accept-Encoding: gzip' header and expect a gzip compressed body and a Content-Encoding: gzip response header. The server will send the content encoded as gzip only after a threshold defined by server.encoding.gzip.min_length (default value is 1024). If the size is below the threshold, the body is not compressed
- send 'Content-Encoding: gzip' header and a gzip compressed body and expect the server to correctly interpret the request
Fixes#5310
Signed-off-by: aarnautu <aarnautu@adobe.com>
When sending JSON back to the client — and we do a lot of that, use
the streaming implementation of json.Encode rather than marshalling
the data into an intermediate byte array.
One curious detail here is that the streaming implementation
uses newlines to mark the end of the stream, so a few unit tests had
to be updated to expect this. Previously we would only emit a trailing
newline if "pretty" was configured.
Signed-off-by: Anders Eknert <anders@styra.com>
* [rego] Check store modules before skipping parsing
Fixes https://github.com/open-policy-agent/opa/issues/5511
This change will cause the operation to be timed if the store
modules have already all been compiled and there are no
rawModules. This might be undesirable.
Signed-off-by: Charlie Egan <charlie@styra.com>
Since "document missing or undefined" caused confusion, we can be
more helpful and report whether the default decision document is
missing, or whether it's there but undefined. I would probably have
preferred for undefined to just result in an empty/undefined result,
but that'd be quite a breaking change at this point in time, so
hopefully this can at least make things a little less convoluted.
Fixes#5344
Signed-off-by: Anders Eknert <anders@eknert.com>
This is the OPA side of #4290. It will allow the envoy plugin to wire
the TraceProvider into the gRPC handlers.
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
This would be useful for us for two immediate use cases:
1. Show how and why rules failed in more detail in verbose tooling,
we can show the unification happening step by step.
2. We can trace which parts of the input document were used, if we
add `Location` info those terms.
However, I think it's generally useful for debugging tools.
This increases verbosity in the explain logs, so we decided to add a new explain
mode `debug` in addition to the existing `full`, `notes`, `fails`, `off` modes.
This can be set using the `--explain=debug` flag on the CLI, or by using `trace
debug` in the REPL.
Signed-off-by: Jasper Van der Jeugt <m@jaspervdj.be>
Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.
Fixes#4846
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Before, the option InterQueryCache(...) passed to the authorizer's config
had set it to `nil` -- it wasn't set up yet.
Now, the ordering allows for caching in the system authz policies.
Fixes#4829.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Currently bundles are loaded into memory entirely
even when disk storage is used. Then the parsed content
is written to the store. Deserializing data into Go structs
is memory consuming and even if user has configured disk
storage, OPA is still bound by the amount of memory
assigned to it. This change adds a new lazy loading mode
wherein the entire data is not deserialized while bundle
reading and hence if the bundle contains large data files
and the user has enabled disk storage, OPA should be
able to handle this scenario w/o running OOM.
Fixes: #4539
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
A bunch of smaller follow-up tasks to #4381.
* storage/disk_test: check invalid patches with wildcard partition, too
* docs/disk: add caveat re: bundles loaded into memory
* storage/disk: auto-manage /system partitions
If these are found in the user-provided partitions, we'll error out.
* storage/disk: pretty-print partitions with "*" instead of %2A
* storage/disk: respect wildcard-replacement in partition validation
It is now allowed to replace a partition like
/foo/bar
by
/foo/*
also if multiple wildcards are used.
Caveats:
You cannot add a wildcard partition like /*/*, since it would overlap
the managed "/system/*" partition.
When attempting to go back from /foo/* to /foo/bar, an error is
raised _unconditionally_ -- we could check the existing data, but
currently don't.
* storage/disk: check prefix when adding wildcard partitions
The previously done check would have falsely returned that there is no problem
when adding a wildcard partition: lookup of "/foo/*" with '*' not interpreted
as a wildcard, but as a string, would yield a not-found, even if there was any
data under /foo/.
Now, we'll check the prefix-until-wildcard. It's more cautious than
theoretically necessary, but safe.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
With this change, the disk backend (badger) becomes available for
use with the OPA runtime properly:
It can be configured using the `storage.disk` key in OPA's config
(see included documentation).
When enabled,
- any data or policies stored with OPA will persist over restarts
- per-query metrics related to disk usage are reported
- Prometheus metrics per storage operation are exported
The main intention behind this feature is to optimize memory usage:
OPA can now operate on more data than fits into the allotted memory
resources. It is NOT meant to be used as a primary source of truth:
there are no backup/restore or desaster recovery procedures -- you
MUST secure the means to restore the data stored with OPA's disk
storage by yourself.
See also #4014. Future improvements around bundle loading are
planned.
Some notes on details:
storage/disk: impose same locking regime used with inmem
With this setup, we'll ensure:
- there is only one open write txn at a time
- there are any number of open read txns at a time
- writes are blocked when reads are inflight
- during a commit (and triggers being run), no read txns can be created
This is to ensure the same atomic policy update semantics when using
'disk" as we have with "inmem". We're basically opting out of badger's
currency control and transactionality guarantees. This is because we
cannot piggy back on that to ensure the atomic update we want.
There might be other ways -- using subscribers, and blocking in some
other place -- but this one seems preferrable since it mirrors inmem.
Part of the problem is ErrTxnTooLarge, and committing and renewing
txns when it occurs: that, which is the prescribed solution to txns
growing too big, also means that reads can see half of the "logical"
transaction having been committed, while the rest is still getting
processed.
Another approach would have been using `WriteBatch`, but that won't
let us read from the batch, only apply Set and Delete operations.
We currently need to read (via an iterator) to figure out if we
need to delete keys to replace something in the store. There is
no DropPrefix operation on the badger txn, or the WriteBatch API.
storage/disk: remove commit-and-renew-txn code for txn-too-big errors
This would break transactional guarantees we care about: while there
can be only one write transaction at a time, read transactions may
happen while a write txn is underway -- with this commit-and-reset
logic, those would read partial data.
Now, the error will be returned to the caller. The maximum txn size
depends on the size of memtables, and could be tweaked manually.
In general, the caller should try to push multiple smaller increments
of the data.
storage/disk: implement noop MakeDir
The MakeDir operation as implemented in the backend-agnostic storage
code has become an issue with the disk store: to write /foo/bar/baz,
we'd have to read /foo (among other subdirs), and that can be _much_
work for the disk backend. With inmem, it's cheap, so this wasn't
problematic before.
Some of the storage/disk/txn.go logic had to be adjusted to properly
do the MakeDir steps implicitly.
The index argument addition to patch() in storage/disk/txn.go was
necessary to keep the error messages conforming to the previous
code path: previously, conflicts (arrays indexed as objects) would
be surfaced in the MakeDir step, now it's entangled with the patch
calculation.
storage/disk: check ctx.Err() in List/Get operations
This won't abort reading a single key, but it will abort iterations.
storage/disk: support patterns in partitions
There is a potential clash here: "*", the path wildcard, is
a valid path section. However, it only affects the case when
a user would want to have a partition at
/foo/*/bar
and would really mean "*", and not the wildcard.
Storing data at /foo/*/bar with a literal "*" won't be treated
differently than storing something at /fo/xyz/bar.
storage/disk: keep per-txn-type histograms of stats
This is done by reading off the metrics on commit, and shovelling
their numbers into the prometheus collector.
NOTE: if you were to share a metrics object among multiple transactions,
the results would be skewed, as it's not reset. However, our server
handlers don't do that.
storage/disk: opt out of badger's conflict detection
With only one write transaction in flight at any time, the situation
that badger guards against cannot happen:
A transaction has written to a key after the current, to-be-committed
transaction has last read that key from the store.
Since it can't happen, we can ignore the bookkeeping involved. This
improves the time it takes to overwrite existing keys.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This change adds an api_usage_warning when input attribute is missing in POST requests to /v1/data
Fixes: #4386
Signed-off-by: Alam <afaaqalam@gmail.com>
This change enables consumers of Compile API to specify packages
those should not be inlined in partial evaluation response.
Fixes: #4357
Signed-off-by: skosunda <skosunda@adobe.com>
... so applications can perform more informed error handling, e.g. refresh credentials on 403.
Fixes#4259.
Signed-off-by: Jakob Schmid <jakob.schmid@sap.com>
Earlier a snapshot bundle would describe the full state of OPA's
policy/data and any update would require first erasing the state from
the existing bundle and then activating the new bundle.
This commit introduces a new bundle type called "delta".
Delta bundles contain patches to data instead of snapshots.
They allow users to efficiently make updates to OPA's data
cache.
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
1. I think an extra section on OT is warranted, even if it's brief.
2. The config now expects "type" to be set to "grpc" to enable the current
feature set. More options may follow in the future.
This is meant to future-proof the config. Future values might include
"http", where the address, TLS and sampling related settings would be
reused; and "global", where the setup would use whatever global trace
provider was configured with otel. The latter use case would be for
embedding OPA via the SDK.
3. Decision ID attributes are only added to spans if decision logging is enabled.
Fixes#4128.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit implements tracing using the net/http automatic
instrumentation wrappers on the server and topdown/http packages.
Fixes#1469
Signed-off-by: Rien Valkenaers <rien.valkenaers@gmail.com>
This commit adds a new server endpoint for pulling the status
information from the running OPA. Normally status is pushed by OPA to
remote locations but in some cases users may need to pull it.
The docs changes move the config and health API sections up into the
right location in the REST API page.
Fixes#4089
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
If the following conditions hold for a set of rules returned by the indexer,
it will set EarlyExit: true, and change how the complete virtual doc or
function is evaluated:
- all rule head values are ground
- all rule head values match
This implies that some cases where early exit would be possible will not be
covered:
p = x {
x := true
input.foo == "bar"
}
p = x {
x := true
input.baz == "quz"
}
To indicate that "early exit" is possible, the indexer result message is
amended. Also, the "Exit" trace event will have a message of "early" when
"early exit" actually happens in eval:
$ echo '{"x":"x", "y":"y"}' | opa eval -I -fpretty --explain=full -d r.rego data.r.r
query:1 Enter data.r.r = _
query:1 | Eval data.r.r = _
query:1 | Index data.r.r (matched 2 rules, early exit)
r.rego:11 | Enter data.r.r
r.rego:12 | | Eval input.y = "y"
r.rego:11 | | Exit data.r.r
query:1 | Exit data.r.r = _
query:1 Redo data.r.r = _
query:1 | Redo data.r.r = _
r.rego:11 | Redo data.r.r
r.rego:12 | | Redo input.y = "y"
r.rego:11 | Exit data.r.r early
With `r.rego` as
package r
r {
input.x = "x"
}
r = 2 {
input.z = "z"
}
r {
input.y = "y"
}
This is done in in a way such that early-exit will abort array/set/object
iterations on data:
r {
data.i[_] = "one"
data.j[_] = "four"
}
f(x, y) {
data.i[_] = x
data.j[_] = y
}
Complete rules (r) and functions (f) that iterate over sets, arrays, and
objects from either data (evalTree) or a term that's returned by some
other rule etc (evalTerm).
The CLI and golang packages expose ways to disable 'early-exit':
This is in line with how indexing can be disabled. It's supposed to be
used as a debugging measure, so it's only exposed as a CLI flag to
`opa eval`.
Co-authored-by: Torin Sandall <torinsandall@gmail.com>
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
* Wrap entire plugin state get/set operation with mutex
* Tweak error messages returned to clients
* Tweak docs explaining use case for custom checks
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
- Add a new /health/{path} route to the server
- This route will eval policy defined at system.health with a query
based on the url path.
- If the policy returns true, the route will respond with a 200.
Otherwise, a 500.
- Log health check errors to provide clarity when health check fails.
Fixes#3759
Signed-off-by: Grant Shively <gshively@godaddy.com>
- Added a new querystring param to the /health route: `exclude-plugin`
- Can be specified multiple times
- Value is the name of a plugin that should be excluded from the OK
status check performed when the `plugins` qs param is specified
Fixes#3713
Signed-off-by: Grant Shively <gshively@godaddy.com>
This commit refactors the serve to avoid using simple HTML forms to
execute queries against the server. This avoids having the GET /
handler run queries. With this change, the index.html page is now
static (except for templating that happens to inject the OPA version
info on the page.)
This commit drops support for the "explain" option on the form. This
feature is not very useful in it's current state so it should not be missed.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
golint is deprecated. The author of the code no longer supports the
codebase. golangci-lint is faster than golint, and is in use by other
opa repositories (e.g. Gatekeeper).
This commit changes tools.go to reference golangci (so it ends up in
vendor) and modifies check-lint to use golangci instead.
Breaking API Changes:
- plugins/rest/rest.go: Fix typo "AllowInsureTLS" -> "AllowInsecureTLS"
- storage/errors.go: Removed unused IndexingNotSupportedErr
Signed-off-by: Will Beason <willbeason@google.com>
- Allows extending the REST API by creating a custom runtime object
- Allows overriding existing routes since Router matches routes in order of registration
Signed-off-by: Jakob Schmid <jakob.schmid@sap.com>
This commit just tweaks the new v1/config response to include a
wrapper that nests the config object under a "result" key in the
response. This makes GET v1/config consistent with other OPA APIs and
future-proofs the API to some extent (e.g., if we need to include
additional fields in the future, they do not necessarily have to be
included in the config object itself.)
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
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>
Philip Conrad