❗ We now parse rego metadata annotations by default.
Rule annotations now support a `labels` field. During policy eval,
labels from all successfully evaluated rules are collected and included
in each decision log entry as a top-level `rule_labels` array. Each
element preserves the label map from one evaluated rule. Exact
duplicates are omitted.
```rego
# METADATA
# labels:
# severity: low
# team: platform
allow if input.role == "admin"
```
The resulting decision log entry will contain:
```json
{"rule_labels": [{"severity": "low", "team": "platform"}]}
```
---------
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Rules can now be annotated with a metadata `id` field. When any
metadata `id` annotations are present in the rego (scope: rule), the IDs
of successfully evaluated rules are included in decision log events.
Additionally, the Data API supports a `?id` query parameter to
include evaluated rule IDs directly in the response payload.
```rego
# METADATA
# id: allow-admin
allow if input.role == "admin"
```
Modules containing `id` annotations will have metadata parsing enabled
automatically.
Fixes#2089
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Wrapping projects can now attach custom metadata to Data API requests
and have evaluation produce response metadata.
Introduce two distinct metadata paths:
- Request (incoming) metadata: parsed from extra top-level keys in the request
body, made available to builtins via `BuiltinContext.RequestMetadata`.
Logged in the decision log under `Custom["request_metadata"]`.
- Response (outgoing) metadata: a separate map (`BuiltinContext.ResponseMetadata`)
that builtins can populate during evaluation. Only included in the
API response and decision log (`Custom["response_metadata"]`)
if non-empty.
In vanilla OPA, no builtins write response metadata, so responses are
unchanged. The request metadata map is only allocated when the request
carries extra fields; the outgoing map is one empty map per request.
To avoid conflicts with future OPA top-level keys, callers should use a
namespaced key: `{"input": {...}, "com.example.opa/md": {...}}`.
```mermaid
flowchart LR
req["POST /v1/data\n{input, com.example.opa/md}"]
parse["readInputPostV1"]
eval["topdown eval"]
resp["API response"]
dl["decision log"]
req --> parse
parse -- "reqMetadata" --> eval
parse -- "reqMetadata" --> dl
eval -- "respMetadata\n(if non-empty)" --> resp
eval -- "respMetadata\n(if non-empty)" --> dl
eval -. "BuiltinContext\n.RequestMetadata\n.ResponseMetadata" .-> eval
```
---------
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Abstract Unix sockets (paths prefixed with @) exist only in the
kernel socket namespace and have no filesystem representation.
Calling os.Chmod on them fails with "no such file or directory".
The --unix-socket-perm flag (added in v0.53.0 via PR #5888) defaults
to "755" and always triggers a chmod on the socket path after the
listener is created. This makes it impossible to use abstract Unix
sockets with OPA >= v0.53.0.
The fix adds a guard to skip chmod when the socket path starts with
"@", matching the existing guard that already skips os.Remove for
abstract sockets a few lines above.
Amp-Thread-ID: https://ampcode.com/threads/T-019d9906-2625-774e-8f1c-a0c288630be4
Signed-off-by: Ben Abderrazak <babderrazak@squareup.com>
Signed-off-by: Ben Apprederisse <bena@squareup.com>
Co-authored-by: Amp <amp@ampcode.com>
* plugins/rest: cache *http.Client and auth plugin
This will require further changes to cert TLS and token auth methods to
stay compatible with the previous behaviour.
* plugins/rest: configurable re-read interval for TLS cert+key
Defaulting to re-reading all the time, more or less like we did before.
(I write "more or less" because we now do it in `GetClientCertificate()`.)
* plugins/rest: document change (code comments, CHANGELOG)
* plugins/rest: set minimum TLS version where `&tls.Config{}` is used
* plugins/rest: ensure min TLS version and ciphersuites are used
...as configured with the server.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
And enable more staticcheck linters. I saw staticcheck failures
mentioned in another PR, so thought I'd check it out.
- `WriteString(fmt.Sprintf)` -> `fmt.Fprintf`
- Rewrite calls to deprecated `*Rule.Path()`
- Don't use `==` to compare `time.Time`
- Use inline ignores over config exclusions of paths
- Remove 'varcheck' ignores as no longer used
- Remove v0 topdown/graphql.go (!)
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
The `json.patch` built-in is quite versatile, and compared to
patching via e.g. `object.union` et. al. often communicates
intent better, IMO. But while it uses some fairly advanced
logic for complex patch operations, it doesn't perform all that
great on simple ones. This is a first and pretty basic attempt
to improve that somewhat by picking the most low-hangig performance
fruits, like avoiding repeated allocations of temporary term pointers.
The main allocation source is the creation of EditTree's, and this
remains a problem. I have created a sync pool but only managed to
get the outermost edit tree to recycle, as I found it really hard
to track where it's safe to release those created in the deeply
nested calls. Additionally, I managed to trigger stack overflows
trying to recycle child trees, so there seems to be some circular
refs? Or I just did something wrong.
If someone wants to look into this and pick up where
I left, that'd be great!
- Add InternedIntRange for testing, primarily
- Intern keys used in json.patch patches
- Clean up json.X built-in benchmarks
- Reduce allocations in edit tree function
- Avoid using intermediate data structures
for JSON patches
- Some unrelated interning fixes to reduce noise
in tests and benchmarks (e.g. do less stuff in
var inits)
Selected benchmark that I used while working on this:
**Before**
```
BenchmarkJSONPatchAddShallowScalar/object-10-16 147853 8008 ns/op 9667 B/op 206 allocs/op
BenchmarkJSONPatchAddShallowScalar/array-10-16 201704 5889 ns/op 7256 B/op 173 allocs/op
BenchmarkJSONPatchAddShallowScalar/set-10-16 182566 6733 ns/op 8103 B/op 156 allocs/op
```
**After**
```
BenchmarkJSONPatchAddShallowScalar/object-10-16 197414 6066 ns/op 7256 B/op 133 allocs/op
BenchmarkJSONPatchAddShallowScalar/array-10-16 278121 4427 ns/op 5285 B/op 100 allocs/op
BenchmarkJSONPatchAddShallowScalar/set-10-16 233884 4839 ns/op 6243 B/op 113 allocs/op
```
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
When `--h2c` is passed, HTTP2 will also be used on the unix domain socket.
Previously, it had no effect on UDS, only on TCP connections.
Fixes#8282.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
- Bump golangci-lint -> 2.6.2
- Fix all `deprecatedComment` "notices should be in a dedicated paragraph, separated from the rest" reports
- Enable `appendCombine` and fix all "appendCombine: can combine chain of X appends into one" notices
- Enable `preferFprint` and fix the few reported issues
- Fix various issues reported only once or twice, like `zeroByteRepeat`
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Have done this some time in the past, but there was a few
new issues this would highlight now that we're on Go 1.24.
Mostly:
- Use `b.Loop()` in benchmarks
- Use `strings.SplitSeq` where possible
- Remove `omitempty` tag for types that can't be empty
Signed-off-by: Anders Eknert <anders@eknert.com>
* server: port compile API
Also adds e2e tests: These include coverage for ucast in the prisma
setting, and thus require some JS runtime.
* e2e: selectively skip e2e Compile API tests
...for macos runs, and for the go-compat suites.
* server: accept timer_rego_external_resolve_ns metrics with value 0
When running the tests in a loop for a while, I would see values of 0ns
for this metric. However, comparing with its non-zero values, which are
often 41 or 42ns, it seems like this is just not happening in this code
path. So if "almost nothing" actually goes below 1ns, it's OK.
* e2e: split dep-heavy e2e tests into their own go module
* Makefile: export DOCKER_RUNNING (make e2e read it)
---------
Co-authored-by: Philip Conrad <philip@chariot-chaser.net>
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
* v1/plugins: Address race in config access
I ran into this race condition on another PR:
https://github.com/open-policy-agent/opa/actions/runs/16655603110/job/47139789057
I have tried to make all manager.Config access thread-safe by adding new
getters for used values. GetConfig is regrettably based on a JSON
roundtrip deep copy of the config. This us used in tests (fine) but also
in the discovery plugin:
https://github.com/open-policy-agent/opa/blob/2d014a89bbbc307d7204817220146ffae992e838/v1/plugins/discovery/discovery.go#L122
getPluginSet is very tightly coupled to the manager.Config and because
of it's dependencies on status and the other plugins packages, it's hard
to break out.
So, for now, I think this is an improvement and worth getting a second
opinion on before more refactoring.
Signed-off-by: Charlie Egan <charlie@styra.com>
* v1/config: Use add Clone to config
This makes the use of the manager's config more thread-safe and
consistent without more API changes.
Signed-off-by: Charlie Egan <charlie@styra.com>
* topdown: Add clone() funcs for config structs
NamedValueCacheConfig.Clone, InterQueryBuiltinValueCacheConfig.Clone and
InterQueryBuiltinCacheConfig.Clone have been added.
All Clone methods return a deep copy of the struct. This is tested for
missed new fields using PopulateAllFields, a generic function that
stuffs structs with values for all fields.
Signed-off-by: Charlie Egan <charlie@styra.com>
* plugins: Clone new config
Signed-off-by: Charlie Egan <charlie@styra.com>
---------
Signed-off-by: Charlie Egan <charlie@styra.com>
This commit adds an experimental "intermediate results" field to
decision logs, and provides some basic plumbing in the server package
for attaching the intermediate results of an eval to the request
context.
Co-authored-by: Teemu Koponen <koponen@styra.com>
Signed-off-by: Philip Conrad <philip@chariot-chaser.net>
This commit provides an extension mechanism for the server authorizer,
allowing plugins and other server extensions to inform the authorizer
about the methods and paths where it should expect and parse request
bodies.
Signed-off-by: Philip Conrad <philip@chariot-chaser.net>
Before we had introduced `http.ServeMux` as "the router", we had been
using github.com/gorilla/mux. Using the latter, it was possible to
inject middlewares using the mux's `.Use()` method. This mechanism
allowed global middlewares to be injected from `runtime.Params`, for
example.
With `http.ServeMux`, that's no longer possible. However, it was never
an intentionally supported feature in the first place.
So this commit introduces HTTP handler middlewares as extension points.
It's modelled after `(*plugins.Manager).ExtraRoute()`.
Signed-off-by: Stephan Renatus <stephan@styra.com>
When an plugin http handler or some other mechanism wants to do rego
evaluations, too, it's beneficial to share the caches with the server.
This change introduces two new hook types to allow retrieving those
caches during server startup.
Signed-off-by: Stephan Renatus <stephan@styra.com>
This way, the extra handler functions are still covered by prometheus
metrics and opentelemetry spans.
The previous method of directly registering routes with the router
bypassed the server's handler wrapping.
Signed-off-by: Stephan Renatus <stephan@styra.com>
I was curious to see how much work this would entail, and it turned out to
be... some :) Particularly porting some of the features exposed as settings
by gorilla mux, like removing trailing slashes, or escaping `/` in matched
paths.
This change is breaking by necessity, as some public functions previously
accepted arguments straight from the mux library. I don't really see any
way around that if we want to get rid of the dependency. I don't think
that too many external projects use code from the server directly though,
so I'm thinking the impact should be minimal? Happy to hear what others
think.
Signed-off-by: Anders Eknert <anders@styra.com>
The /v1/data endpoint's PUT, PATCH, and DELETE methods all reported
incorrect timer information (zeroed out timers). This was caused by the
Timer.Stop() calls being deferred in those method handlers.
The problem is that the timers do not record a time value until you stop
them. So, when the metrics are reported, all of those timers are still
storing their initial zero values.
The fix was manually calling Timer.Stop() right before metrics collection
on each endpoint, similar to how we handle many other endpoints in the
server.
Signed-off-by: Philip Conrad <philip@chariot-chaser.net>
Since we don't use the HTTP API in Regal, I hadn't looked at this from
a performance POV before, and it was a fun side quest :)
A pretty decent reduction of the baseline cost for the most common
request type, v1/data POST. Changes:
- Add NoOp implementation of `Metrics` for when metrics aren't needed
- Cheaper `metrics.New` instantiation avoiding unnecessary lock
- Avoid cost of decision logging if decision logging isn't enabled
- Only call request.URL.Query() if URL.RawQuery isn't empty, avoiding
allocating an empty map with each request
While the target was v1/data POST handling, some of the changes above
have a positive impact on all or most handlers. All changes have been
run through the existing tests, and a new benchmark to measure the impact
of the fixes have been added, showing:
```
13063 ns/op 15162 B/op 195 allocs/op - main
12796 ns/op 14856 B/op 189 allocs/op - avoid r.URL.Query() when no query provided
12541 ns/op 14483 B/op 187 allocs/op - decisionLogger.Log early exit if not enabled
12133 ns/op 14235 B/op 180 allocs/op - get revisions and init logger only if needed
11098 ns/op 14207 B/op 180 allocs/op - more efficient metrics.New() (without locking)
10683 ns/op 13171 B/op 169 allocs/op - use no-op metrics implementation when metrics aren't requested
```
Even if the impact is pretty good, it's worth noting that most of the improvements
above are only seen when decision logging is turned off. While this is the common case
for development, it's not in production. Getting the baseline cost down is important still
as there should be no cost paid for features unused.
Signed-off-by: Anders Eknert <anders@styra.com>
Following up on #7566, and now applying the more exciting
modernizations. fmt.Appendf was new to me! But especially
the contains checks are so much better IMHO. I have reviewed
all changes myself and did a few manual changes where it
became obvious that things could be improved a little further.
(the modernize analyzer still has some issues running against
OPA, and I have manually worked around those for the time being)
Signed-off-by: Anders Eknert <anders@styra.com>
Brace yourselves! For there are many touched files here. No changes
in semantics however.
Spent a long time trying out the various optional rules gocritic
provides, and settled for a few of them. There are more I really
like, but that would take many hours to address across the codebase.
Perhaps others find gocritic too pedantic? If so, we can merge the
fixes without enabling the rule.
Signed-off-by: Anders Eknert <anders@styra.com>
And update code to conform to the rule.
- Replace unnecessary fmt.Sprintf with string concatenation
- Replace fmt.Sprint with more efficient strconv.Itoa
- Replace static fmt.Errorf calls with more efficient errors.New
Thanks @srenatus for pushing me down this rabbit hole!
Signed-off-by: Anders Eknert <anders@styra.com>
* topdown+rego: allow opt-in for evaluating non-det builtins in PE
Some use cases of PE, notably generating queries that are to be translated
into filters of some sort (think SQL), require the evaluation of non-deterministic
builtins. This is because the result of the builtin informs what queries are
returned.
Imagine that the user associated with a request is known at PE-time, but we need
extra information from an HTTP API to determine the filters that should be applied.
Previously, that was just impossible to do. Now, we can opt-in to evaluate non-det
builtins during PE from the Rego API.
Note that it would probably make sense to include this in the inlining controls, as
sent to the Compile API. (Considered out of scope for this PR.)
Also note that this will take highest precedence over the `ast.IgnoreDuringPartialEval`
map and the "Nondeterministic" value of the registered builtin. If the new option is
provided, both of these are ignored.
Signed-off-by: Stephan Renatus <stephan@styra.com>
* server+rego: expose nondeterministicBuiltins via inlining controls
With `foo.rego` as
```rego
package ex
include if input.fruits.name == object.get(http.send(input.req).body, input.path, "unknown")
```
the following queries show the difference:
```interactive
$ curl -v http://127.0.0.1:8181/v1/compile \
-d '{"input": {"req": {"url": "https://httpbin.org/json", "method":"GET"}, "path": ["slideshow", "title"]}, "query": "data.ex.include", "unknowns": ["input.fruits"]}'
{
"result": {
"queries": [
[
{
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "http"
},
{
"type": "string",
"value": "send"
}
]
},
{
"type": "object",
"value": [
[
{
"type": "string",
"value": "method"
},
{
"type": "string",
"value": "GET"
}
],
[
{
"type": "string",
"value": "url"
},
{
"type": "string",
"value": "https://httpbin.org/json"
}
]
]
},
{
"type": "var",
"value": "__local0__1"
}
]
},
{
"index": 1,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "ref",
"value": [
{
"type": "var",
"value": "input"
},
{
"type": "string",
"value": "fruits"
},
{
"type": "string",
"value": "name"
}
]
},
{
"type": "call",
"value": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "object"
},
{
"type": "string",
"value": "get"
}
]
},
{
"type": "ref",
"value": [
{
"type": "var",
"value": "__local0__1"
},
{
"type": "string",
"value": "body"
}
]
},
{
"type": "array",
"value": [
{
"type": "string",
"value": "slideshow"
},
{
"type": "string",
"value": "title"
}
]
},
{
"type": "string",
"value": "unknown"
}
]
}
]
}
]
]
}
}
```
Here, the builtin call to http.send is preserved.
If we also pass `nondeterminsticBuiltins: true` to the options, we get this:
```interactive
$ curl http://127.0.0.1:8181/v1/compile \
-d '{"input": {"req": {"url": "https://httpbin.org/json", "method":"GET"}, "path": ["slideshow", "title"]}, "query": "data.ex.include", "unknowns": ["input.fruits"], "options": {"nondeterministicBuiltins": true}}'
{
"result": {
"queries": [
[
{
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "ref",
"value": [
{
"type": "var",
"value": "input"
},
{
"type": "string",
"value": "fruits"
},
{
"type": "string",
"value": "name"
}
]
},
{
"type": "string",
"value": "Sample Slide Show"
}
]
}
]
]
}
}
```
Here, all args to http.send have been known at PE time and the call was fully
evaluated.
Signed-off-by: Stephan Renatus <stephan@styra.com>
* cmd/eval: expose --nondeterminstic-builtins for new PE control
```interactive
$ echo '{"req": {"url": "https://httpbin.org/json", "method":"GET"}, "path": ["slideshow", "title"]}'| ./opa_darwin_amd64 eval -fpretty -p -I -d foo.rego -u input.fruits data.ex.include
+---------+-------------------------------------------------------------------------------------+
| Query 1 | http.send({"method": "GET", "url": "https://httpbin.org/json"}, __local0__1) |
| | input.fruits.name = object.get(__local0__1.body, ["slideshow", "title"], "unknown") |
+---------+-------------------------------------------------------------------------------------+
$ echo '{"req": {"url": "https://httpbin.org/json", "method":"GET"}, "path": ["slideshow", "title"]}'| ./opa_darwin_amd64 eval -fpretty -p -I -d foo.rego -u input.fruits data.ex.include --nondeterminstic-builtins
+---------+-----------------------------------------+
| Query 1 | input.fruits.name = "Sample Slide Show" |
+---------+-----------------------------------------+
```
Signed-off-by: Stephan Renatus <stephan@styra.com>
---------
Signed-off-by: Stephan Renatus <stephan@styra.com>
All packages, except for `cmd` and `internal`, have been moved into a new `v1` root package.
Old packages are kept for backwards-compatibility reasons. All contained code is replaced with simple type aliases and proxy functions to `v1` implementations.
Old packages default to the Rego v0 syntax, new `v1` packages default to the Rego v1 syntax.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Stephan Renatus