diff --git a/CHANGELOG.md b/CHANGELOG.md index 70593b3630..a23a023664 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,23 +5,27 @@ project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased -### Evaluated Rules in Decision Logs and API Responses +### Rule Labels in Decision Logs -Rules can now be annotated with a metadata `id` field. When any loaded policy contains -rules with `id` annotations, the IDs of successfully evaluated rules are automatically -included in decision log events (as `ids`). Additionally, the Data API supports a `?ids` -query parameter to include the same information directly in the response payload. -Duplicate IDs from functions called multiple times are suppressed. +Rule annotations now support a `labels` field. 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. ```rego # METADATA -# id: allow-admin +# labels: +# severity: low +# team: platform allow if input.role == "admin" ``` -Modules containing `id` annotations will have metadata parsing enabled automatically. -When external rule sources are registered, rule tracking is always enabled so that -externally-provided rules with `id` annotations are recorded. +The resulting decision log entry will contain: + +```json +{"rule_labels": [{"severity": "low", "team": "platform"}]} +``` + +The runtime now processes metadata annotations by default. ## 1.16.1 diff --git a/cmd/test.go b/cmd/test.go index d57db140e9..8e59705a43 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -317,7 +317,7 @@ func processWatcherUpdate(ctx context.Context, testParams testCommandParams, pat var loadResult *initload.LoadPathsResult - err := pathwatcher.ProcessWatcherUpdateForRegoVersion(ctx, testParams.RegoVersion(), paths, removed, store, filter, testParams.bundleMode, false, + err := pathwatcher.ProcessWatcherUpdateForRegoVersion(ctx, ast.ParserOptions{RegoVersion: testParams.RegoVersion(), ProcessAnnotation: true}, paths, removed, store, filter, testParams.bundleMode, false, func(ctx context.Context, txn storage.Transaction, loaded *initload.LoadPathsResult) error { if len(loaded.Files.Documents) > 0 || removed != "" { if err := store.Write(ctx, txn, storage.AddOp, storage.RootPath, loaded.Files.Documents); err != nil { diff --git a/docs/docs/policy-language.md b/docs/docs/policy-language.md index dbfd9b5c19..6750cad9a3 100644 --- a/docs/docs/policy-language.md +++ b/docs/docs/policy-language.md @@ -2600,18 +2600,18 @@ comment block containing the YAML document is finished ### Annotations -| Name | Type | Description | -| ------------------- | ----------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | -| scope | string; one of `package`, `rule`, `document`, `subpackages` | The scope for which the metadata applies. Read more in the [Metadata Scope section below](#metadata-scope). | -| `id` | string | A unique identifier for the rule, used to track evaluated rules in decision logs. Read more in the [Metadata ID section below](#metadata-id). | -| `title` | string | A human-readable name for the annotation target. Read more in the [Metadata Title section below](#metadata-title). | -| `description` | string | A description of the annotation target. Read more in the [Metadata Description section below](#metadata-description). | -| `related_resources` | list of URLs | A list of URLs pointing to related resources/documentation. Read more in the [Metadata Related Resources section below](#metadata-related_resources). | -| `authors` | list of strings | A list of authors for the annotation target. Read more in the [Metadata Authors section below](#metadata-authors). | -| `organizations` | list of strings | A list of organizations related to the annotation target. Read more in the [Metadata Organizations section below](#metadata-organizations). | -| `schemas` | list of object | A list of associations between value paths and schema definitions. Read more in the [Metadata Schemas section below](#metadata-schemas). | -| `entrypoint` | boolean | Whether or not the annotation target is to be used as a policy entrypoint. Read more in the [Metadata Entrypoint section below](#metadata-entrypoint). | -| `custom` | mapping of arbitrary data | A custom mapping of named parameters holding arbitrary data. Read more in the [Metadata Custom section below](#metadata-custom). | +| Name | Type | Description | +| ------------------- | ----------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| scope | string; one of `package`, `rule`, `document`, `subpackages` | The scope for which the metadata applies. Read more in the [Metadata Scope section below](#metadata-scope). | +| `labels` | mapping of key-value pairs | Arbitrary labels attached to a rule, recorded in decision logs when the rule is evaluated. Read more in the [Metadata Labels section below](#metadata-labels). | +| `title` | string | A human-readable name for the annotation target. Read more in the [Metadata Title section below](#metadata-title). | +| `description` | string | A description of the annotation target. Read more in the [Metadata Description section below](#metadata-description). | +| `related_resources` | list of URLs | A list of URLs pointing to related resources/documentation. Read more in the [Metadata Related Resources section below](#metadata-related_resources). | +| `authors` | list of strings | A list of authors for the annotation target. Read more in the [Metadata Authors section below](#metadata-authors). | +| `organizations` | list of strings | A list of organizations related to the annotation target. Read more in the [Metadata Organizations section below](#metadata-organizations). | +| `schemas` | list of object | A list of associations between value paths and schema definitions. Read more in the [Metadata Schemas section below](#metadata-schemas). | +| `entrypoint` | boolean | Whether or not the annotation target is to be used as a policy entrypoint. Read more in the [Metadata Entrypoint section below](#metadata-entrypoint). | +| `custom` | mapping of arbitrary data | A custom mapping of named parameters holding arbitrary data. Read more in the [Metadata Custom section below](#metadata-custom). | ### Metadata `Scope` @@ -2665,20 +2665,19 @@ allow if { message := "welcome!" if allow ``` -### Metadata `id` +### Metadata `labels` -The `id` annotation is a string value that uniquely identifies a rule. When -any loaded policy contains rules with `id` annotations (or when external rule -sources are registered), the IDs of successfully evaluated rules are -automatically recorded in decision log events. The `id` can also be returned -in the Data API response using the `?ids` query parameter. - -When any module contains a metadata block with an `id` field, annotation -parsing is enabled automatically (even if `ProcessAnnotation` was not set). +The `labels` annotation is a map of arbitrary key-value pairs attached to a +rule (or document). When rules with `labels` are successfully evaluated, their +label sets are automatically recorded in decision log events under the +`rule_labels` field. Labels from document-scoped and rule-scoped annotations +are both collected. ```rego # METADATA -# id: allow-admin +# labels: +# severity: high +# team: platform allow if input.role == "admin" ``` diff --git a/e2e/cli/evaluated_rules.txtar b/e2e/cli/evaluated_rules.txtar deleted file mode 100644 index 88c587519c..0000000000 --- a/e2e/cli/evaluated_rules.txtar +++ /dev/null @@ -1,60 +0,0 @@ -# Verify that evaluated rule IDs appear in decision logs when rules -# have metadata annotations with id fields, and in the response when -# the ?ids query parameter is used. - -exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --set decision_logs.console=true ./policy/ &opa& -retry curl -sf --unix-socket opa.sock 'http://localhost/health' - -# POST to v1/data — rule with id "allow-admin" should be evaluated. -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow -jq stdout '.result == true' - -# POST to v1/data with ?ids — ids should appear in response. -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' 'http://localhost/v1/data/authz/allow?ids' -jq stdout '.result == true and .ids == ["allow-admin"]' - -# POST to v1/data with ?ids — no rules match, field absent. -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "guest"}}' 'http://localhost/v1/data/authz/allow?ids' -jq stdout '.result == false and (has("ids") | not)' - -# POST to v1/data with ?ids — multiple rules evaluate (multi-value rule). -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin", "dept": "eng"}}' 'http://localhost/v1/data/authz/violations?ids' -jq stdout '.ids | sort == ["no-admin-in-eng", "no-admin-role"]' - -# POST without ?ids — ids must be absent. -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow -jq stdout '.result == true and (has("ids") | not)' - -kill -INT opa -wait opa - -# All requests that matched allow-admin should have ids in the decision log. -jq -s stderr '[.[] | select(.msg == "Decision Log" and .ids == ["allow-admin"])] | length == 3' - --- policy/authz.rego -- -package authz - -default allow := false - -# METADATA -# id: allow-admin -allow if input.role == "admin" - -# METADATA -# id: allow-editor -allow if input.role == "editor" - -# METADATA -# id: no-admin-role -violations contains msg if { - input.role == "admin" - msg := "admin role is restricted" -} - -# METADATA -# id: no-admin-in-eng -violations contains msg if { - input.role == "admin" - input.dept == "eng" - msg := "admin not allowed in eng" -} diff --git a/e2e/cli/evaluated_rules_file_logger.txtar b/e2e/cli/evaluated_rules_file_logger.txtar deleted file mode 100644 index a4cab03a71..0000000000 --- a/e2e/cli/evaluated_rules_file_logger.txtar +++ /dev/null @@ -1,42 +0,0 @@ -# Verify that evaluated rule IDs appear in decision logs written via the -# file_logger plugin (slog-based path). - -exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml ./policy/ &opa& -retry curl -sf --unix-socket opa.sock 'http://localhost/health' - -# POST to v1/data — rule with id "allow-admin" should be evaluated. -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow -jq stdout '.result == true' - -# POST to v1/data — no rules match. -exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "guest"}}' http://localhost/v1/data/authz/allow -jq stdout '.result == false' - -kill -INT opa -wait opa - -# Decision log file should contain ids for the admin request. -jq -s decisions.log '[.[] | select(.ids == ["allow-admin"])] | length == 1' - -# Decision log file should have an entry without ids for the guest request. -jq -s decisions.log '[.[] | select(.path == "authz/allow" and (.ids == null))] | length == 1' - --- config.yaml -- -plugins: - file_logger: - path: decisions.log -decision_logs: - plugin: file_logger - --- policy/authz.rego -- -package authz - -default allow := false - -# METADATA -# id: allow-admin -allow if input.role == "admin" - -# METADATA -# id: allow-editor -allow if input.role == "editor" diff --git a/e2e/cli/rule_labels_disk.txtar b/e2e/cli/rule_labels_disk.txtar new file mode 100644 index 0000000000..13e3157326 --- /dev/null +++ b/e2e/cli/rule_labels_disk.txtar @@ -0,0 +1,33 @@ +# Verify that labels from evaluated rule annotations appear as a top-level +# field in decision logs when policies are loaded from disk (no bundle server). + +exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --set decision_logs.console=true ./policy/ &opa& +retry curl -sf --unix-socket opa.sock 'http://localhost/health' + +# Evaluate allow — triggers rule with labels. +exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow +jq stdout '.result == true' + +# Evaluate with no match — no labels expected. +exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "guest"}}' http://localhost/v1/data/authz/allow +jq stdout '.result == false' + +kill -INT opa +wait opa + +# Decision log for the admin request should have rule_labels. +jq -s stderr '[.[] | select(.msg == "Decision Log" and .result == true)] | .[0].rule_labels == [{"severity": "low", "team": "platform"}]' + +# Decision log for the guest request should have no rule_labels. +jq -s stderr '[.[] | select(.msg == "Decision Log" and .result == false)] | .[0] | has("rule_labels") | not' + +-- policy/authz.rego -- +package authz + +default allow := false + +# METADATA +# labels: +# severity: low +# team: platform +allow if input.role == "admin" diff --git a/e2e/cli/rule_labels_file_logger.txtar b/e2e/cli/rule_labels_file_logger.txtar new file mode 100644 index 0000000000..13e4a4b170 --- /dev/null +++ b/e2e/cli/rule_labels_file_logger.txtar @@ -0,0 +1,60 @@ +# Verify that rule labels appear in decision logs written via the +# file_logger plugin (slog-based path). + +exec $OPA build -o bundles/bundle.tar.gz --bundle policy/ + +http_server bundles + +exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml &opa& +retry curl -sf --unix-socket opa.sock 'http://localhost/health?bundles' + +# POST to v1/data — rule with labels should be evaluated. +exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow +jq stdout '.result == true' + +# POST to v1/data — no rules match. +exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "guest"}}' http://localhost/v1/data/authz/allow +jq stdout '.result == false' + +kill -INT opa +wait opa + +# Decision log file should contain rule_labels for the admin request. +jq -s decisions.log '[.[] | select(.rule_labels == [{"severity": "low"}])] | length == 1' + +# Decision log file should have an entry without rule_labels for the guest request. +jq -s decisions.log '[.[] | select(.path == "authz/allow" and (.rule_labels == null))] | length == 1' + +-- config.yaml -- +services: + bundle-server: + url: "http://${HTTP_ADDR}" +bundles: + test: + service: bundle-server + resource: bundle.tar.gz + polling: + min_delay_seconds: 300 + max_delay_seconds: 300 +plugins: + file_logger: + path: decisions.log +decision_logs: + plugin: file_logger + +-- policy/authz.rego -- +package authz + +default allow := false + +# METADATA +# labels: +# severity: low +allow if input.role == "admin" + +# METADATA +# labels: +# severity: low +allow if input.role == "editor" + +-- bundles/.gitkeep -- diff --git a/e2e/cli/rule_metadata_dl.txtar b/e2e/cli/rule_metadata_dl.txtar new file mode 100644 index 0000000000..a5d950a8f8 --- /dev/null +++ b/e2e/cli/rule_metadata_dl.txtar @@ -0,0 +1,72 @@ +# Verify that labels from evaluated rule annotations appear as a top-level +# field in decision logs. + +exec $OPA build -o bundles/bundle.tar.gz --bundle policy/ + +http_server bundles + +exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml &opa& +retry curl -sf --unix-socket opa.sock 'http://localhost/health?bundles' + +# Evaluate allow — triggers allow-admin rule. +exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow +jq stdout '.result == true' + +# Evaluate violations — triggers both violation rules; labels collected per-rule. +exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin", "dept": "eng"}}' http://localhost/v1/data/authz/violations +jq stdout '.result | length == 2' + +kill -INT opa +wait opa + +# Decision log for allow should have rule_labels with the single matching rule's labels. +jq -s stderr '[.[] | select(.msg == "Decision Log" and .path == "authz/allow")] | .[0].rule_labels == [{"severity": "low"}]' + +# Decision log for violations should have one entry per evaluated rule, preserving per-rule grouping. +jq -s stderr '[.[] | select(.msg == "Decision Log" and .path == "authz/violations")] | .[0].rule_labels | sort_by(.category) == [{"severity": "high", "category": "compliance"}, {"severity": "critical", "category": "org-policy"}]' + +-- config.yaml -- +services: + bundle-server: + url: "http://${HTTP_ADDR}" +bundles: + test: + service: bundle-server + resource: bundle.tar.gz + polling: + min_delay_seconds: 300 + max_delay_seconds: 300 +decision_logs: + console: true + +-- policy/authz.rego -- +package authz + +default allow := false + +# METADATA +# labels: +# severity: low +allow if input.role == "admin" + +# METADATA +# labels: +# severity: low +allow if input.role == "editor" + +# METADATA +# labels: +# severity: high +# category: compliance +violations contains "admin role is restricted" if input.role == "admin" + +# METADATA +# labels: +# severity: critical +# category: org-policy +violations contains "admin not allowed in eng" if { + input.role == "admin" + input.dept == "eng" +} + +-- bundles/.gitkeep -- diff --git a/internal/pathwatcher/utils.go b/internal/pathwatcher/utils.go index 8c92524760..e36111a6ab 100644 --- a/internal/pathwatcher/utils.go +++ b/internal/pathwatcher/utils.go @@ -42,12 +42,12 @@ func CreatePathWatcher(rootPaths []string) (*fsnotify.Watcher, error) { // ProcessWatcherUpdate handles an occurrence of a watcher event func ProcessWatcherUpdate(ctx context.Context, paths []string, removed string, store storage.Store, filter loader.Filter, asBundle bool, bundleLazyLoadingMode bool, f func(context.Context, storage.Transaction, *initload.LoadPathsResult) error) error { - return ProcessWatcherUpdateForRegoVersion(ctx, ast.DefaultRegoVersion, paths, removed, store, filter, asBundle, bundleLazyLoadingMode, f) + return ProcessWatcherUpdateForRegoVersion(ctx, ast.ParserOptions{RegoVersion: ast.DefaultRegoVersion, ProcessAnnotation: true}, paths, removed, store, filter, asBundle, bundleLazyLoadingMode, f) } -func ProcessWatcherUpdateForRegoVersion(ctx context.Context, regoVersion ast.RegoVersion, paths []string, removed string, store storage.Store, filter loader.Filter, asBundle bool, bundleLazyLoadingMode bool, +func ProcessWatcherUpdateForRegoVersion(ctx context.Context, popts ast.ParserOptions, paths []string, removed string, store storage.Store, filter loader.Filter, asBundle bool, bundleLazyLoadingMode bool, f func(context.Context, storage.Transaction, *initload.LoadPathsResult) error) error { - loaded, err := initload.LoadPathsForRegoVersion(regoVersion, paths, filter, asBundle, nil, true, bundleLazyLoadingMode, false, false, nil, nil) + loaded, err := initload.LoadPathsForRegoVersion(popts, paths, filter, asBundle, nil, true, bundleLazyLoadingMode, false, nil) if err != nil { return err } @@ -75,7 +75,7 @@ func ProcessWatcherUpdateForRegoVersion(ctx context.Context, regoVersion ast.Reg if err != nil { return err } - module, err := ast.ParseModuleWithOpts(id, string(bs), ast.ParserOptions{RegoVersion: regoVersion}) + module, err := ast.ParseModuleWithOpts(id, string(bs), popts) if err != nil { return err } diff --git a/internal/pathwatcher/utils_test.go b/internal/pathwatcher/utils_test.go index ee05ab5aac..9ef53f8c4f 100644 --- a/internal/pathwatcher/utils_test.go +++ b/internal/pathwatcher/utils_test.go @@ -95,7 +95,7 @@ func TestProcessWatcherUpdateForRegoVersion(t *testing.T) { t.Fatal(err) } - err = ProcessWatcherUpdateForRegoVersion(t.Context(), regoVersion, paths, "", store, filter, false, false, f) + err = ProcessWatcherUpdateForRegoVersion(t.Context(), ast.ParserOptions{RegoVersion: regoVersion, ProcessAnnotation: true}, paths, "", store, filter, false, false, f) if err != nil { t.Fatalf("Unexpected error: %v", err) } diff --git a/internal/runtime/init/init.go b/internal/runtime/init/init.go index 7c9f6b096a..2ba8839605 100644 --- a/internal/runtime/init/init.go +++ b/internal/runtime/init/init.go @@ -143,21 +143,20 @@ func LoadPaths(paths []string, processAnnotations bool, caps *ast.Capabilities, fsys fs.FS) (*LoadPathsResult, error) { - return LoadPathsForRegoVersion(ast.RegoV0, paths, filter, asBundle, bvc, skipVerify, bundleLazyLoading, processAnnotations, false, caps, fsys) + return LoadPathsForRegoVersion(ast.ParserOptions{RegoVersion: ast.RegoV0, ProcessAnnotation: processAnnotations, Capabilities: caps}, paths, filter, asBundle, bvc, skipVerify, bundleLazyLoading, false, fsys) } -func LoadPathsForRegoVersion(regoVersion ast.RegoVersion, +func LoadPathsForRegoVersion(popts ast.ParserOptions, paths []string, filter loader.Filter, asBundle bool, bvc *bundle.VerificationConfig, skipVerify bool, bundleLazyLoading bool, - processAnnotations bool, followSymlinks bool, - caps *ast.Capabilities, fsys fs.FS) (*LoadPathsResult, error) { + caps := popts.Capabilities if caps == nil { caps = ast.CapabilitiesForThisVersion() } @@ -180,9 +179,9 @@ func LoadPathsForRegoVersion(regoVersion ast.RegoVersion, WithSkipBundleVerification(skipVerify). WithBundleLazyLoadingMode(bundleLazyLoading). WithFilter(filter). - WithProcessAnnotation(processAnnotations). + WithProcessAnnotation(popts.ProcessAnnotation). WithCapabilities(caps). - WithRegoVersion(regoVersion). + WithRegoVersion(popts.RegoVersion). WithFollowSymlinks(followSymlinks). AsBundle(path) if err != nil { @@ -198,9 +197,9 @@ func LoadPathsForRegoVersion(regoVersion ast.RegoVersion, files, err := loader.NewFileLoader(). WithFS(fsys). WithBundleLazyLoadingMode(bundleLazyLoading). - WithProcessAnnotation(processAnnotations). + WithProcessAnnotation(popts.ProcessAnnotation). WithCapabilities(caps). - WithRegoVersion(regoVersion). + WithRegoVersion(popts.RegoVersion). Filtered(nonBundlePaths, filter) if err != nil { diff --git a/v1/ast/annotations.go b/v1/ast/annotations.go index a6ff6ab71f..73a938acc9 100644 --- a/v1/ast/annotations.go +++ b/v1/ast/annotations.go @@ -27,7 +27,6 @@ type ( // Annotations represents metadata attached to other AST nodes such as rules. Annotations struct { Scope string `json:"scope"` - ID string `json:"id,omitempty"` Title string `json:"title,omitempty"` Entrypoint bool `json:"entrypoint,omitempty"` Description string `json:"description,omitempty"` @@ -37,6 +36,7 @@ type ( Schemas []*SchemaAnnotation `json:"schemas,omitempty"` Compile *CompileAnnotation `json:"compile,omitempty"` Custom map[string]any `json:"custom,omitempty"` + Labels map[string]any `json:"labels,omitempty"` Location *Location `json:"location,omitempty"` comments []*Comment @@ -134,10 +134,6 @@ func (a *Annotations) Compare(other *Annotations) int { return cmp } - if cmp := strings.Compare(a.ID, other.ID); cmp != 0 { - return cmp - } - if cmp := strings.Compare(a.Title, other.Title); cmp != 0 { return cmp } @@ -177,6 +173,10 @@ func (a *Annotations) Compare(other *Annotations) int { return cmp } + if cmp := util.Compare(a.Labels, other.Labels); cmp != 0 { + return cmp + } + return 0 } @@ -201,10 +201,6 @@ func (a *Annotations) MarshalJSON() ([]byte, error) { "scope": a.Scope, } - if a.ID != "" { - data["id"] = a.ID - } - if a.Title != "" { data["title"] = a.Title } @@ -237,6 +233,10 @@ func (a *Annotations) MarshalJSON() ([]byte, error) { data["custom"] = a.Custom } + if len(a.Labels) > 0 { + data["labels"] = a.Labels + } + if astJSON.GetOptions().MarshalOptions.IncludeLocation.Annotations { if a.Location != nil { data["location"] = a.Location @@ -428,6 +428,10 @@ func (a *Annotations) Copy(node Node) *Annotations { cpy.Custom = deepcopy.Map(a.Custom) } + if a.Labels != nil { + cpy.Labels = deepcopy.Map(a.Labels) + } + cpy.node = node return &cpy @@ -445,10 +449,6 @@ func (a *Annotations) toObject() (*Object, *Error) { obj.Insert(InternedTerm("scope"), InternedTerm(a.Scope)) } - if len(a.ID) > 0 { - obj.Insert(InternedTerm("id"), StringTerm(a.ID)) - } - if len(a.Title) > 0 { obj.Insert(InternedTerm("title"), StringTerm(a.Title)) } @@ -526,6 +526,14 @@ func (a *Annotations) toObject() (*Object, *Error) { obj.Insert(InternedTerm("custom"), NewTerm(c)) } + if len(a.Labels) > 0 { + l, err := InterfaceToValue(a.Labels) + if err != nil { + return nil, NewError(CompileErr, a.Location, "invalid labels annotation %s", err.Error()) + } + obj.Insert(InternedTerm("labels"), NewTerm(l)) + } + return &obj, nil } @@ -599,10 +607,6 @@ func attachAnnotationsNodes(mod *Module) Errors { if err := validateAnnotationEntrypointAttachment(a); err != nil { errs = append(errs, err) } - - if err := validateAnnotationIDAttachment(a); err != nil { - errs = append(errs, err) - } } return errs @@ -635,14 +639,6 @@ func validateAnnotationEntrypointAttachment(a *Annotations) *Error { return nil } -func validateAnnotationIDAttachment(a *Annotations) *Error { - if a.ID != "" && a.Scope != annotationScopeRule { - return NewError( - ParseErr, a.Loc(), "annotation id applied to non-rule scope '%v'", a.Scope) - } - return nil -} - // Copy returns a deep copy of a. func (a *AuthorAnnotation) Copy() *AuthorAnnotation { cpy := *a diff --git a/v1/ast/parser.go b/v1/ast/parser.go index f4d7ddde68..a4e9539c6d 100644 --- a/v1/ast/parser.go +++ b/v1/ast/parser.go @@ -545,10 +545,6 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { break } - if !p.po.ProcessAnnotation && hasMetadataWithID(p.s.comments) { - p.po.ProcessAnnotation = true - } - if p.po.ProcessAnnotation { stmts = p.parseAnnotations(stmts) } @@ -556,19 +552,6 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { return stmts, p.s.comments, p.s.errors } -func hasMetadataWithID(comments []*Comment) bool { - for i := range comments { - if IsMetadataComment(comments[i]) { - for i++; i < len(comments) && !blockBuster(comments[i], comments[i-1]); i++ { - if bytes.HasPrefix(bytes.TrimSpace(comments[i].Text), []byte("id:")) { - return true - } - } - } - } - return false -} - func (p *Parser) parseAnnotations(stmts []Statement) []Statement { annotStmts, errs := parseAnnotations(p.s.comments) for _, err := range errs { @@ -2832,7 +2815,6 @@ func (p *Parser) validateDefaultRuleArgs(rule *Rule) bool { // which isn't handled properly by json for some reason. type rawAnnotation struct { Scope string `yaml:"scope"` - ID string `yaml:"id"` Title string `yaml:"title"` Entrypoint bool `yaml:"entrypoint"` Description string `yaml:"description"` @@ -2842,6 +2824,7 @@ type rawAnnotation struct { Schemas []map[string]any `yaml:"schemas"` Compile map[string]any `yaml:"compile"` Custom map[string]any `yaml:"custom"` + Labels map[string]any `yaml:"labels"` } type metadataParser struct { @@ -2897,7 +2880,6 @@ func (b *metadataParser) Parse() (result *Annotations, err error) { result = &Annotations{ comments: b.comments, Scope: raw.Scope, - ID: raw.ID, Entrypoint: raw.Entrypoint, Title: raw.Title, Description: raw.Description, @@ -2993,6 +2975,15 @@ func (b *metadataParser) Parse() (result *Annotations, err error) { } } + if raw.Labels != nil { + result.Labels = make(map[string]any, len(raw.Labels)) + for k, v := range raw.Labels { + if result.Labels[k], err = convertYAMLMapKeyTypes(v, nil); err != nil { + return nil, err + } + } + } + result.Location = b.loc // recreate original text of entire metadata block for location text attribute diff --git a/v1/ast/parser_test.go b/v1/ast/parser_test.go index 17584b02fa..acb3b22b82 100644 --- a/v1/ast/parser_test.go +++ b/v1/ast/parser_test.go @@ -8197,61 +8197,6 @@ allow if { assertLocationText(t, "# METADATA\n# title: rule", m.Rules[0].Annotations[0].Location) } -func TestAnnotationsAutoEnableWithID(t *testing.T) { - tests := []struct { - note string - module string - processAnnotation bool - expAnnotationsCount int - }{ - { - note: "auto-enable when metadata has id field", - module: `package test -# METADATA -# id: test-rule -allow if true -`, - processAnnotation: false, - expAnnotationsCount: 1, - }, - { - note: "no auto-enable when metadata lacks id field", - module: `# METADATA -# title: Test Rule -package test -allow if true -`, - processAnnotation: false, - expAnnotationsCount: 0, - }, - { - note: "explicit ProcessAnnotation still works", - module: `# METADATA -# title: Test Rule -package test -allow if true -`, - processAnnotation: true, - expAnnotationsCount: 1, - }, - } - - for _, tc := range tests { - t.Run(tc.note, func(t *testing.T) { - m, err := ParseModuleWithOpts("test.rego", tc.module, ParserOptions{ - ProcessAnnotation: tc.processAnnotation, - }) - if err != nil { - t.Fatal(err) - } - - if len(m.Annotations) != tc.expAnnotationsCount { - t.Errorf("expected %d annotations but got %d", tc.expAnnotationsCount, len(m.Annotations)) - } - }) - } -} - func TestMaxParsingRecursionDepth(t *testing.T) { tests := []struct { name string diff --git a/v1/compile/compile.go b/v1/compile/compile.go index ec90426b71..d52f5d898f 100644 --- a/v1/compile/compile.go +++ b/v1/compile/compile.go @@ -489,16 +489,18 @@ func (c *Compiler) initBundle(usePath bool) error { // we can track read and parse times. load, err := initload.LoadPathsForRegoVersion( - c.regoVersion, + ast.ParserOptions{ + RegoVersion: c.regoVersion, + ProcessAnnotation: c.useRegoAnnotationEntrypoints, + Capabilities: c.capabilities, + }, c.paths, c.filter, c.asBundle, c.bvc, false, c.enableBundleLazyLoadingMode, - c.useRegoAnnotationEntrypoints, c.followSymlinks, - c.capabilities, c.fsys) if err != nil { return fmt.Errorf("load error: %w", err) diff --git a/v1/config/config.go b/v1/config/config.go index 1dc16e9f62..5d40898bee 100644 --- a/v1/config/config.go +++ b/v1/config/config.go @@ -6,7 +6,6 @@ package config import ( - "crypto/tls" "encoding/json" "errors" "fmt" @@ -23,11 +22,6 @@ import ( "github.com/open-policy-agent/opa/v1/version" ) -const ( - // DefaultMinTLSVersion is the minimum TLS version used by OPA server and REST clients - DefaultMinTLSVersion = tls.VersionTLS12 -) - // ServerConfig represents the different server configuration options. type ServerConfig struct { Metrics json.RawMessage `json:"metrics,omitempty"` diff --git a/v1/config/default.go b/v1/config/default.go new file mode 100644 index 0000000000..b439cbf942 --- /dev/null +++ b/v1/config/default.go @@ -0,0 +1,12 @@ +// Copyright 2026 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +package config + +import "crypto/tls" + +const ( + // DefaultMinTLSVersion is the minimum TLS version used by OPA server and REST clients + DefaultMinTLSVersion = tls.VersionTLS12 +) diff --git a/v1/download/download.go b/v1/download/download.go index f11a07c428..a901e19b6a 100644 --- a/v1/download/download.go +++ b/v1/download/download.go @@ -347,6 +347,7 @@ func (d *Downloader) download(ctx context.Context, m metrics.Metrics) (*download reader := bundle.NewCustomReader(loader). WithRegoVersion(d.bundleParserOpts.RegoVersion). + WithProcessAnnotations(d.bundleParserOpts.ProcessAnnotation). WithMetrics(m). WithBundleVerificationConfig(d.bvc). WithBundleEtag(etag). diff --git a/v1/download/oci_download.go b/v1/download/oci_download.go index 208200c954..39c8969b13 100644 --- a/v1/download/oci_download.go +++ b/v1/download/oci_download.go @@ -276,7 +276,8 @@ func (d *OCIDownloader) download(ctx context.Context, m metrics.Metrics) (*downl WithMetrics(m). WithBundleVerificationConfig(d.bvc). WithBundleEtag(etag). - WithRegoVersion(d.bundleParserOpts.RegoVersion) + WithRegoVersion(d.bundleParserOpts.RegoVersion). + WithProcessAnnotations(d.bundleParserOpts.ProcessAnnotation) bundleInfo, err := reader.Read() if err != nil { return &downloaderResponse{}, fmt.Errorf("unexpected error %w", err) diff --git a/v1/plugins/bundle/plugin.go b/v1/plugins/bundle/plugin.go index 717da0b9d7..0679a24357 100644 --- a/v1/plugins/bundle/plugin.go +++ b/v1/plugins/bundle/plugin.go @@ -880,6 +880,7 @@ func (fl *fileLoader) oneShot(ctx context.Context) (err error) { WithLazyLoadingMode(bundle.HasExtension()). WithSizeLimitBytes(fl.sizeLimitBytes). WithRegoVersion(fl.bundleParserOpts.RegoVersion). + WithProcessAnnotations(fl.bundleParserOpts.ProcessAnnotation). Read() u.Error = err if err == nil { diff --git a/v1/plugins/logs/plugin.go b/v1/plugins/logs/plugin.go index dbfb0cd423..6cfa751a23 100644 --- a/v1/plugins/logs/plugin.go +++ b/v1/plugins/logs/plugin.go @@ -68,7 +68,7 @@ type EventV1 struct { Timestamp time.Time `json:"timestamp"` Metrics map[string]any `json:"metrics,omitempty"` RequestID uint64 `json:"req_id,omitempty"` - IDs []string `json:"ids,omitempty"` + RuleLabels []map[string]any `json:"rule_labels,omitempty"` RequestContext *RequestContext `json:"request_context,omitempty"` Custom map[string]any `json:"custom,omitempty"` @@ -210,12 +210,11 @@ func (e *EventV1) AST() (ast.Value, error) { event.Insert(ast.InternedTerm("requested_by"), ast.StringTerm(e.RequestedBy)) } - if len(e.IDs) > 0 { - evaluatedRules := make([]*ast.Term, len(e.IDs)) - for i, v := range e.IDs { - evaluatedRules[i] = ast.StringTerm(v) + if len(e.RuleLabels) > 0 { + v, err := ast.InterfaceToValue(e.RuleLabels) + if err == nil { + event.Insert(ast.InternedTerm("rule_labels"), ast.NewTerm(v)) } - event.Insert(ast.InternedTerm("ids"), ast.ArrayTerm(evaluatedRules...)) } // Use the timestamp JSON marshaller to ensure the format is the same as @@ -738,7 +737,7 @@ func (p *Plugin) Log(ctx context.Context, decision *server.Info) error { RequestedBy: decision.RemoteAddr, Timestamp: decision.Timestamp, RequestID: decision.RequestID, - IDs: decision.EvaluatedRuleIDs, + RuleLabels: decision.EvaluatedRuleLabels, inputAST: decision.InputAST, Custom: decision.Custom, } @@ -1233,7 +1232,7 @@ func eventToAttrs(event EventV1) []slog.Attr { attrs = append(attrs, slog.Any("request_context", event.RequestContext)) } - addAttrIfSliceNotEmpty(&attrs, "ids", event.IDs) + addAttrIfSliceNotEmpty(&attrs, "rule_labels", event.RuleLabels) addAttrIfHasLen(&attrs, "custom", event.Custom) return attrs @@ -1354,7 +1353,12 @@ func eventToFields(event EventV1) map[string]any { fields["request_context"] = event.RequestContext } - addIfSliceNotEmpty(fields, "ids", stringsToAny(event.IDs)) + if len(event.RuleLabels) > 0 { + var v any = event.RuleLabels + if err := util.RoundTrip(&v); err == nil { + fields["rule_labels"] = v + } + } if len(event.Custom) > 0 { var v any = event.Custom diff --git a/v1/plugins/logs/plugin_test.go b/v1/plugins/logs/plugin_test.go index 6ea3dabae0..7fe716bfd0 100644 --- a/v1/plugins/logs/plugin_test.go +++ b/v1/plugins/logs/plugin_test.go @@ -4260,7 +4260,11 @@ func TestLogEvaluatedRules(t *testing.T) { // Log decision with evaluated rules if err := plugin.Log(ctx, &server.Info{ - EvaluatedRuleIDs: []string{"rule1", "rule2", "rule3"}, + EvaluatedRuleLabels: []map[string]any{ + {"id": "rule1"}, + {"id": "rule2"}, + {"id": "rule3"}, + }, }); err != nil { t.Fatal(err) } @@ -4275,16 +4279,20 @@ func TestLogEvaluatedRules(t *testing.T) { } // Check first event has evaluated rules - if exp, act := 3, len(backend.events[0].IDs); exp != act { + if exp, act := 3, len(backend.events[0].RuleLabels); exp != act { t.Fatalf("expected %d evaluated rules in event 0, got %d", exp, act) } - expectedRules := []string{"rule1", "rule2", "rule3"} - if diff := cmp.Diff(expectedRules, backend.events[0].IDs); diff != "" { + expectedRules := []map[string]any{ + {"id": "rule1"}, + {"id": "rule2"}, + {"id": "rule3"}, + } + if diff := cmp.Diff(expectedRules, backend.events[0].RuleLabels); diff != "" { t.Errorf("unexpected evaluated rules in event 0 (-want, +got):\n%s", diff) } // Check second event has no evaluated rules - if exp, act := 0, len(backend.events[1].IDs); exp != act { + if exp, act := 0, len(backend.events[1].RuleLabels); exp != act { t.Fatalf("expected %d evaluated rules in event 1, got %d", exp, act) } } diff --git a/v1/rego/rego_evaluated_test.go b/v1/rego/rego_evaluated_test.go deleted file mode 100644 index b7df39b77c..0000000000 --- a/v1/rego/rego_evaluated_test.go +++ /dev/null @@ -1,118 +0,0 @@ -package rego - -import ( - "testing" - - "github.com/open-policy-agent/opa/v1/topdown" -) - -func TestEvaluatedRules(t *testing.T) { - module := `package test - -# METADATA -# id: rule1 -p if input.foo - -# METADATA -# id: rule2 -p if input.bar` - - t.Run("records matching rule", func(t *testing.T) { - tracker := &topdown.EvaluatedRuleTracker{} - rs, err := New( - Query("data.test.p"), - Module("test.rego", module), - Input(map[string]any{"foo": true}), - EvaluatedRuleTracker(tracker), - ).Eval(t.Context()) - if err != nil { - t.Fatal(err) - } - if len(rs) == 0 { - t.Fatal("expected result") - } - if len(tracker.IDs) != 1 || tracker.IDs[0] != "rule1" { - t.Fatalf("expected [rule1], got %v", tracker.IDs) - } - }) - - t.Run("no match yields empty", func(t *testing.T) { - tracker := &topdown.EvaluatedRuleTracker{} - _, _ = New( - Query("data.test.p"), - Module("test.rego", module), - Input(map[string]any{"baz": true}), - EvaluatedRuleTracker(tracker), - ).Eval(t.Context()) - - if len(tracker.IDs) != 0 { - t.Fatalf("expected empty, got %v", tracker.IDs) - } - }) - - t.Run("nil tracker is safe", func(t *testing.T) { - _, err := New( - Query("data.test.p"), - Module("test.rego", module), - Input(map[string]any{"foo": true}), - EvaluatedRuleTracker(nil), - ).Eval(t.Context()) - if err != nil { - t.Fatal(err) - } - }) - - t.Run("prepared query", func(t *testing.T) { - pq, err := New( - Query("data.test.p"), - Module("test.rego", module), - ).PrepareForEval(t.Context()) - if err != nil { - t.Fatal(err) - } - - tracker := &topdown.EvaluatedRuleTracker{} - rs, err := pq.Eval(t.Context(), - EvalInput(map[string]any{"bar": true}), - EvalEvaluatedRuleTracker(tracker), - ) - if err != nil { - t.Fatal(err) - } - if len(rs) == 0 { - t.Fatal("expected result") - } - if len(tracker.IDs) != 1 || tracker.IDs[0] != "rule2" { - t.Fatalf("expected [rule2], got %v", tracker.IDs) - } - }) - - t.Run("function called multiple times is deduplicated", func(t *testing.T) { - mod := `package test - -# METADATA -# id: is-allowed -is_allowed(user) if user != "blocked" - -result if { - is_allowed("alice") - is_allowed("bob") - is_allowed("charlie") -}` - tracker := &topdown.EvaluatedRuleTracker{} - rs, err := New( - Query("data.test.result"), - Module("test.rego", mod), - EvaluatedRuleTracker(tracker), - ).Eval(t.Context()) - if err != nil { - t.Fatal(err) - } - if len(rs) == 0 { - t.Fatal("expected result") - } - if len(tracker.IDs) != 1 || tracker.IDs[0] != "is-allowed" { - t.Fatalf("expected [is-allowed] (deduplicated), got %v", tracker.IDs) - } - }) -} diff --git a/v1/runtime/runtime.go b/v1/runtime/runtime.go index d013ede065..fde2159def 100644 --- a/v1/runtime/runtime.go +++ b/v1/runtime/runtime.go @@ -306,7 +306,10 @@ func (p *Params) regoVersion() ast.RegoVersion { } func (p *Params) parserOptions() ast.ParserOptions { - return ast.ParserOptions{RegoVersion: p.regoVersion()} + return ast.ParserOptions{ + ProcessAnnotation: true, + RegoVersion: p.regoVersion(), + } } // LoggingConfig stores the configuration for OPA's logging behaviour. @@ -428,9 +431,7 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { } } - regoVersion := params.regoVersion() - - loaded, err := initload.LoadPathsForRegoVersion(regoVersion, params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, params.BundleLazyLoadingMode, false, false, nil, nil) + loaded, err := initload.LoadPathsForRegoVersion(params.parserOptions(), params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, params.BundleLazyLoadingMode, false, nil) if err != nil { return nil, fmt.Errorf("load error: %w", err) } @@ -984,7 +985,7 @@ func (rt *Runtime) readWatcher(ctx context.Context, watcher *fsnotify.Watcher, p } func (rt *Runtime) processWatcherUpdate(ctx context.Context, paths []string, removed string) error { - return pathwatcher.ProcessWatcherUpdateForRegoVersion(ctx, rt.Manager.ParserOptions().RegoVersion, paths, removed, rt.Store, rt.Params.Filter, rt.Params.BundleMode, rt.Params.BundleLazyLoadingMode, func(ctx context.Context, txn storage.Transaction, loaded *initload.LoadPathsResult) error { + return pathwatcher.ProcessWatcherUpdateForRegoVersion(ctx, rt.Manager.ParserOptions(), paths, removed, rt.Store, rt.Params.Filter, rt.Params.BundleMode, rt.Params.BundleLazyLoadingMode, func(ctx context.Context, txn storage.Transaction, loaded *initload.LoadPathsResult) error { _, err := initload.InsertAndCompile(ctx, initload.InsertAndCompileOptions{ Store: rt.Store, Txn: txn, diff --git a/v1/sdk/opa.go b/v1/sdk/opa.go index d312b77fda..0f3bc94492 100644 --- a/v1/sdk/opa.go +++ b/v1/sdk/opa.go @@ -172,7 +172,7 @@ func (opa *OPA) configure(ctx context.Context, bs []byte, ready chan struct{}, b plugins.Info(runtimeInfo), plugins.Logger(opa.logger), plugins.ConsoleLogger(opa.console), - plugins.WithParserOptions(ast.ParserOptions{RegoVersion: opa.regoVersion}), + plugins.WithParserOptions(ast.ParserOptions{ProcessAnnotation: true, RegoVersion: opa.regoVersion}), plugins.EnablePrintStatements(opa.logger.GetLevel() >= logging.Info), plugins.PrintHook(loggingPrintHook{logger: opa.logger}), plugins.WithHooks(opa.hooks), @@ -345,7 +345,7 @@ func (opa *OPA) Decision(ctx context.Context, options DecisionOptions) (*Decisio if record.Error == nil { record.Results = &result.Result } - record.EvaluatedRuleIDs = tracker.IDs + record.EvaluatedRuleLabels = tracker.Labels }, ) if err != nil { diff --git a/v1/sdk/opa_evaluated_rules_test.go b/v1/sdk/opa_evaluated_rules_test.go deleted file mode 100644 index 5ea27a2c68..0000000000 --- a/v1/sdk/opa_evaluated_rules_test.go +++ /dev/null @@ -1,233 +0,0 @@ -// Copyright 2026 The OPA Authors. All rights reserved. -// Use of this source code is governed by an Apache2 -// license that can be found in the LICENSE file. - -package sdk_test - -import ( - "fmt" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - - "github.com/open-policy-agent/opa/v1/logging/test" - "github.com/open-policy-agent/opa/v1/sdk" - sdktest "github.com/open-policy-agent/opa/v1/sdk/test" -) - -func TestEvaluatedRules(t *testing.T) { - ctx := t.Context() - - server := sdktest.MustNewServer( - sdktest.RawBundles(true), - sdktest.MockBundle("/bundles/bundle.tar.gz", map[string]string{ - "policy.rego": ` -package test - -# METADATA -# id: rule1 -p if { - input.foo -} - -# METADATA -# id: rule2 -p if { - input.bar -} - -# METADATA -# id: rule3 -q if { - input.baz -} - -# Rule without ID (should still work) -r if { - input.qux -} - -allow if xs != set() - -# METADATA -# id: xs1 -xs contains "a" if true - -# METADATA -# id: xs2 -xs contains "b" if false -`, - }), - ) - - defer server.Stop() - - config := fmt.Sprintf(`{ - "services": { - "test": { - "url": %q - } - }, - "bundles": { - "test": { - "resource": "/bundles/bundle.tar.gz" - } - }, - "decision_logs": { - "console": true - } - }`, server.URL()) - - testLogger := test.New() - opa, err := sdk.New(ctx, sdk.Options{ - Config: strings.NewReader(config), - ConsoleLogger: testLogger, - }) - - if err != nil { - t.Fatal(err) - } - - defer opa.Stop(ctx) - - tests := []struct { - name string - path string - input map[string]any - wantResult bool - wantUndefined bool - wantEvaluatedRules []string - }{ - { - name: "input matches rule1", - path: "/test/p", - input: map[string]any{"foo": true}, - wantResult: true, - wantEvaluatedRules: []string{"rule1"}, - }, - { - name: "input matches rule2", - path: "/test/p", - input: map[string]any{"bar": true}, - wantResult: true, - // Note: Virtual cache may prevent re-evaluation after rule1 was cached - wantEvaluatedRules: nil, - }, - { - name: "input matches both rule1 and rule2", - path: "/test/p", - input: map[string]any{"foo": true, "bar": true}, - wantResult: true, - // Note: Only one rule needed to satisfy 'p', so only one will be evaluated - wantEvaluatedRules: nil, - }, - { - name: "input matches rule3", - path: "/test/q", - input: map[string]any{"baz": true}, - wantResult: true, - wantEvaluatedRules: []string{"rule3"}, - }, - { - name: "input matches rule without ID", - path: "/test/r", - input: map[string]any{"qux": true}, - wantResult: true, - // No ID, so no evaluated_rules - wantEvaluatedRules: nil, - }, - { - name: "no rules match", - path: "/test/p", - input: map[string]any{"other": true}, - wantUndefined: true, - // No rules evaluated, so no evaluated_rules - wantEvaluatedRules: nil, - }, - { - name: "set contains with partial rules", - path: "/test/allow", - input: map[string]any{}, - wantResult: true, - // Only xs1 should be in evaluated_rules since xs2 evaluates to false - wantEvaluatedRules: []string{"xs1"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, err := opa.Decision(ctx, sdk.DecisionOptions{ - Path: tt.path, - Input: tt.input, - }) - - if tt.wantUndefined { - if !sdk.IsUndefinedErr(err) { - t.Fatalf("expected undefined error, got: %v", err) - } - // Still check the log entry was created - entries := testLogger.Entries() - if len(entries) == 0 { - t.Fatal("expected at least one log entry") - } - return - } - - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - resultBool, ok := result.Result.(bool) - if !ok { - t.Fatalf("expected bool result, got: %T", result.Result) - } - if resultBool != tt.wantResult { - t.Fatalf("expected result %v, got %v", tt.wantResult, resultBool) - } - - // Check the decision log entry - entries := testLogger.Entries() - if len(entries) == 0 { - t.Fatal("expected at least one log entry") - } - - lastEntry := entries[len(entries)-1] - - // Verify decision_id is present - if lastEntry.Fields["decision_id"] == "" { - t.Fatal("expected non-empty decision_id") - } - - // Verify evaluated_rules field - evaluatedRules, hasField := lastEntry.Fields["ids"] - if tt.wantEvaluatedRules == nil { - if hasField { - t.Fatalf("expected no evaluated_rules field, but got: %v", evaluatedRules) - } - } else { - if !hasField { - t.Fatal("expected evaluated_rules field in decision log") - } - - // Convert to []string for comparison - rulesSlice, ok := evaluatedRules.([]any) - if !ok { - t.Fatalf("expected evaluated_rules to be []any, got %T", evaluatedRules) - } - - actualRules := make([]string, len(rulesSlice)) - for i, v := range rulesSlice { - actualRules[i], ok = v.(string) - if !ok { - t.Fatalf("expected string rule ID, got %T", v) - } - } - - if diff := cmp.Diff(tt.wantEvaluatedRules, actualRules); diff != "" { - t.Errorf("evaluated_rules mismatch (-want +got):\n%s", diff) - } - } - }) - } -} diff --git a/v1/sdk/opa_test.go b/v1/sdk/opa_test.go index 94cd5b26b1..d5711daefa 100644 --- a/v1/sdk/opa_test.go +++ b/v1/sdk/opa_test.go @@ -1587,6 +1587,80 @@ main = time.now_ns() } +func TestDecisionLoggingWithRuleLabels(t *testing.T) { + + ctx := t.Context() + + server := sdktest.MustNewServer( + sdktest.MockBundle("/bundles/bundle.tar.gz", map[string]string{ + "main.rego": ` +package system + +# METADATA +# labels: +# severity: high +# team: platform +main := true if input.role == "admin" +`, + }), + ) + + defer server.Stop() + + config := fmt.Sprintf(`{ + "services": { + "test": { + "url": %q + } + }, + "bundles": { + "test": { + "resource": "/bundles/bundle.tar.gz" + } + }, + "decision_logs": { + "console": true + } + }`, server.URL()) + + testLogger := loggingtest.New() + opa, err := sdk.New(ctx, sdk.Options{ + Config: strings.NewReader(config), + ConsoleLogger: testLogger, + }) + if err != nil { + t.Fatal(err) + } + + defer opa.Stop(ctx) + + if _, err := opa.Decision(ctx, sdk.DecisionOptions{ + Input: map[string]any{"role": "admin"}, + }); err != nil { + t.Fatal(err) + } + + entries := testLogger.Entries() + if len(entries) != 1 { + t.Fatalf("expected 1 log entry, got %d", len(entries)) + } + + labels, ok := entries[0].Fields["rule_labels"] + if !ok { + t.Fatal("expected rule_labels field in decision log entry") + } + + labelsSlice, ok := labels.([]any) + if !ok { + t.Fatalf("expected rule_labels to be a slice, got %T", labels) + } + + exp := []any{map[string]any{"severity": "high", "team": "platform"}} + if diff := cmp.Diff(exp, labelsSlice); diff != "" { + t.Errorf("unexpected rule_labels (-want, +got):\n%s", diff) + } +} + func TestDecisionLoggingWithMasking(t *testing.T) { ctx := t.Context() diff --git a/v1/server/buffer.go b/v1/server/buffer.go index 315d307f56..b22c27f614 100644 --- a/v1/server/buffer.go +++ b/v1/server/buffer.go @@ -38,7 +38,7 @@ type Info struct { Metrics metrics.Metrics Trace []*topdown.Event RequestID uint64 - EvaluatedRuleIDs []string + EvaluatedRuleLabels []map[string]any Custom map[string]any } diff --git a/v1/server/server.go b/v1/server/server.go index 0544303c4a..63ebd17be4 100644 --- a/v1/server/server.go +++ b/v1/server/server.go @@ -149,7 +149,6 @@ type Server struct { allPluginsOkOnce bool distributedTracingOpts tracing.Options ndbCacheEnabled bool - evaluatedRulesEnabled bool unixSocketPerm *string cipherSuites *[]uint16 hooks hooks.Hooks @@ -232,7 +231,6 @@ func (s *Server) Init(ctx context.Context) (*Server, error) { s.partials = map[string]rego.PartialResult{} s.preparedEvalQueries = newCache(pqMaxCacheSize) s.defaultDecisionPath = s.generateDefaultDecisionPath() - s.evaluatedRulesEnabled = compilerHasRuleIDs(s.getCompiler()) s.manager.RegisterNDCacheTrigger(s.updateNDCache) s.Handler = s.initHandlerAuthn(s.Handler) @@ -441,38 +439,15 @@ func (s *Server) WithNDBCacheEnabled(ndbCacheEnabled bool) *Server { return s } -func (s *Server) newEvaluatedRuleTracker(force bool) *topdown.EvaluatedRuleTracker { - if !force && !s.evaluatedRulesEnabled && !s.hasExternalSources() { - return nil - } +func newEvaluatedRuleTracker() *topdown.EvaluatedRuleTracker { return &topdown.EvaluatedRuleTracker{} } -func compilerHasRuleIDs(c *ast.Compiler) bool { - if c == nil { - return false - } - for _, mod := range c.Modules { - for _, r := range mod.Rules { - for _, a := range r.Annotations { - if a.ID != "" { - return true - } - } - } - } - return false -} - -func (s *Server) hasExternalSources() bool { - return s.manager.GetExternalSources() != nil && s.manager.GetExternalSources().Len() > 0 -} - -func evaluatedRuleIDs(t *topdown.EvaluatedRuleTracker) []string { - if t == nil || len(t.IDs) == 0 { +func evaluatedRuleLabels(t *topdown.EvaluatedRuleTracker) []map[string]any { + if t == nil || len(t.Labels) == 0 { return nil } - return t.IDs + return t.Labels } // WithCipherSuites sets the list of enabled TLS 1.0–1.2 cipher suites. @@ -998,8 +973,7 @@ func (s *Server) execQuery(ctx context.Context, br bundleRevisions, txn storage. ndbCache = builtins.NDBCache{} } - tracker := s.newEvaluatedRuleTracker(false) - + tracker := newEvaluatedRuleTracker() opts := []func(*rego.Rego){ rego.Store(s.store), rego.Transaction(txn), @@ -1047,7 +1021,7 @@ func (s *Server) execQuery(ctx context.Context, br bundleRevisions, txn storage. } var x any = results.Result - if err := logger.Log(ctx, txn, "", parsedQuery.String(), rawInput, input, &x, ndbCache, nil, m, evaluatedRuleIDs(tracker), nil); err != nil { + if err := logger.Log(ctx, txn, "", parsedQuery.String(), rawInput, input, &x, ndbCache, nil, m, evaluatedRuleLabels(tracker), nil); err != nil { return nil, err } return &results, nil @@ -1115,7 +1089,6 @@ func (s *Server) reload(_ context.Context, _ storage.Transaction, evt storage.Tr if evt.PolicyChanged() { s.compileUnknownsCache.Purge() s.compileMaskingRulesCache.Purge() - s.evaluatedRulesEnabled = compilerHasRuleIDs(s.getCompiler()) } } @@ -1200,6 +1173,7 @@ func (s *Server) v0QueryPath(w http.ResponseWriter, r *http.Request, urlPath str s.preparedEvalQueries.Insert(pqID, preparedQuery) } + tracker := newEvaluatedRuleTracker() evalOpts := []rego.EvalOption{ rego.EvalTransaction(txn), rego.EvalParsedInput(input), @@ -1207,11 +1181,7 @@ func (s *Server) v0QueryPath(w http.ResponseWriter, r *http.Request, urlPath str rego.EvalInterQueryBuiltinCache(s.interQueryBuiltinCache), rego.EvalInterQueryBuiltinValueCache(s.interQueryBuiltinValueCache), rego.EvalNDBuiltinCache(ndbCache), - } - - tracker := s.newEvaluatedRuleTracker(false) - if tracker != nil { - evalOpts = append(evalOpts, rego.EvalEvaluatedRuleTracker(tracker)) + rego.EvalEvaluatedRuleTracker(tracker), } rs, err := preparedQuery.Eval( @@ -1248,7 +1218,7 @@ func (s *Server) v0QueryPath(w http.ResponseWriter, r *http.Request, urlPath str writer.Error(w, http.StatusNotFound, errV1) return } - err = logger.Log(ctx, txn, urlPath, "", goInput, input, &rs[0].Expressions[0].Value, ndbCache, nil, m, evaluatedRuleIDs(tracker), nil) + err = logger.Log(ctx, txn, urlPath, "", goInput, input, &rs[0].Expressions[0].Value, ndbCache, nil, m, evaluatedRuleLabels(tracker), nil) if err != nil { writer.ErrorAuto(w, err) return @@ -1554,7 +1524,6 @@ func (s *Server) v1DataGet(w http.ResponseWriter, r *http.Request) { includeInstrumentation := getBoolParam(r.URL, types.ParamInstrumentV1, true) provenance := getBoolParam(r.URL, types.ParamProvenanceV1, true) strictBuiltinErrors := getBoolParam(r.URL, types.ParamStrictBuiltinErrors, true) - includeEvaluatedRules := getBoolParam(r.URL, types.ParamEvaluatedRulesV1, true) m.Timer(metrics.RegoInputParse).Start() @@ -1637,6 +1606,7 @@ func (s *Server) v1DataGet(w http.ResponseWriter, r *http.Request) { s.preparedEvalQueries.Insert(pqID, preparedQuery) } + tracker := newEvaluatedRuleTracker() evalOpts := []rego.EvalOption{ rego.EvalTransaction(txn), rego.EvalParsedInput(input), @@ -1646,11 +1616,7 @@ func (s *Server) v1DataGet(w http.ResponseWriter, r *http.Request) { rego.EvalInterQueryBuiltinValueCache(s.interQueryBuiltinValueCache), rego.EvalInstrument(includeInstrumentation), rego.EvalNDBuiltinCache(ndbCache), - } - - tracker := s.newEvaluatedRuleTracker(includeEvaluatedRules) - if tracker != nil { - evalOpts = append(evalOpts, rego.EvalEvaluatedRuleTracker(tracker)) + rego.EvalEvaluatedRuleTracker(tracker), } rs, err := preparedQuery.Eval( @@ -1702,11 +1668,7 @@ func (s *Server) v1DataGet(w http.ResponseWriter, r *http.Request) { result.Explanation = s.getExplainResponse(explainMode, *buf, pretty(r)) } - if includeEvaluatedRules { - result.IDs = evaluatedRuleIDs(tracker) - } - - if err := logger.Log(ctx, txn, urlPath, "", goInput, input, result.Result, ndbCache, nil, m, evaluatedRuleIDs(tracker), nil); err != nil { + if err := logger.Log(ctx, txn, urlPath, "", goInput, input, result.Result, ndbCache, nil, m, evaluatedRuleLabels(tracker), nil); err != nil { writer.ErrorAuto(w, err) return } @@ -1854,7 +1816,6 @@ func (s *Server) v1DataPost(w http.ResponseWriter, r *http.Request) { strictBuiltinErrors := getBoolParam(r.URL, types.ParamStrictBuiltinErrors, true) includeInstrumentation := getBoolParam(r.URL, types.ParamInstrumentV1, true) - includeEvaluatedRules := getBoolParam(r.URL, types.ParamEvaluatedRulesV1, true) pqID := "v1DataPost::" if strictBuiltinErrors { @@ -1893,6 +1854,7 @@ func (s *Server) v1DataPost(w http.ResponseWriter, r *http.Request) { s.preparedEvalQueries.Insert(pqID, preparedQuery) } + tracker := newEvaluatedRuleTracker() evalOpts := []rego.EvalOption{ rego.EvalTransaction(txn), rego.EvalParsedInput(input), @@ -1903,11 +1865,7 @@ func (s *Server) v1DataPost(w http.ResponseWriter, r *http.Request) { rego.EvalInstrument(includeInstrumentation), rego.EvalNDBuiltinCache(ndbCache), rego.EvalResponseMetadata(respMetadata), - } - - tracker := s.newEvaluatedRuleTracker(includeEvaluatedRules) - if tracker != nil { - evalOpts = append(evalOpts, rego.EvalEvaluatedRuleTracker(tracker)) + rego.EvalEvaluatedRuleTracker(tracker), } if reqMetadata != nil { @@ -1967,11 +1925,7 @@ func (s *Server) v1DataPost(w http.ResponseWriter, r *http.Request) { result.Explanation = s.getExplainResponse(explainMode, *buf, pretty(r)) } - if includeEvaluatedRules { - result.IDs = evaluatedRuleIDs(tracker) - } - - if err := logger.Log(ctx, txn, urlPath, "", goInput, input, result.Result, ndbCache, nil, m, evaluatedRuleIDs(tracker), customLog()); err != nil { + if err := logger.Log(ctx, txn, urlPath, "", goInput, input, result.Result, ndbCache, nil, m, evaluatedRuleLabels(tracker), customLog()); err != nil { writer.ErrorAuto(w, err) return } @@ -3162,7 +3116,7 @@ func (l decisionLogger) Log( ndbCache builtins.NDBCache, err error, m metrics.Metrics, - evaluatedRules []string, + evaluatedRuleLabels []map[string]any, custom map[string]any, ) error { if l.logger == nil { @@ -3188,23 +3142,23 @@ func (l decisionLogger) Log( } info := &Info{ - Txn: txn, - Revision: l.revision, - Bundles: bundles, - Timestamp: time.Now().UTC(), - DecisionID: decisionID, - RemoteAddr: rctx.ClientAddr, - HTTPRequestContext: httpRctx, - Path: path, - Query: query, - Input: goInput, - InputAST: astInput, - Results: goResults, - Error: err, - Metrics: m, - RequestID: rctx.ReqID, - EvaluatedRuleIDs: evaluatedRules, - Custom: custom, + Txn: txn, + Revision: l.revision, + Bundles: bundles, + Timestamp: time.Now().UTC(), + DecisionID: decisionID, + RemoteAddr: rctx.ClientAddr, + HTTPRequestContext: httpRctx, + Path: path, + Query: query, + Input: goInput, + InputAST: astInput, + Results: goResults, + Error: err, + Metrics: m, + RequestID: rctx.ReqID, + EvaluatedRuleLabels: evaluatedRuleLabels, + Custom: custom, } if ndbCache != nil { diff --git a/v1/server/types/types.go b/v1/server/types/types.go index d28cccf883..ddce176bcf 100644 --- a/v1/server/types/types.go +++ b/v1/server/types/types.go @@ -9,6 +9,7 @@ import ( "bytes" "encoding/json" "fmt" + "maps" "strings" "github.com/open-policy-agent/opa/v1/ast" @@ -209,9 +210,7 @@ func (r DataRequestV1) MarshalJSON() ([]byte, error) { return nil, err } - for key, val := range r.Metadata { - base[key] = val - } + maps.Copy(base, r.Metadata) return json.Marshal(base) } @@ -222,7 +221,6 @@ type DataResponseV1 struct { Provenance *ProvenanceV1 `json:"provenance,omitempty"` Explanation TraceV1 `json:"explanation,omitempty"` Metrics MetricsV1 `json:"metrics,omitempty"` - IDs []string `json:"ids,omitempty"` Result *any `json:"result,omitempty"` Warning *Warning `json:"warning,omitempty"` @@ -636,10 +634,6 @@ const ( // ParamStrictBuiltinErrors names the HTTP URL parameter that indicates the client // wants built-in function errors to be treated as fatal. ParamStrictBuiltinErrors = "strict-builtin-errors" - - // ParamEvaluatedRulesV1 names the HTTP URL parameter that indicates the client - // wants evaluated rule IDs included in the response. - ParamEvaluatedRulesV1 = "ids" ) // BadRequestErr represents an error condition raised if the caller passes diff --git a/v1/topdown/evaluated.go b/v1/topdown/evaluated.go index 91ea945f9d..6cfbc0f3f1 100644 --- a/v1/topdown/evaluated.go +++ b/v1/topdown/evaluated.go @@ -1,13 +1,21 @@ +// Copyright 2026 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + package topdown -import "github.com/open-policy-agent/opa/v1/ast" +import ( + "encoding/json" + + "github.com/open-policy-agent/opa/v1/ast" +) -// EvaluatedRuleTracker records rule identifiers from annotations during -// evaluation. It extracts the ID field from each successfully evaluated -// rule's annotations. Duplicate IDs are suppressed. +// EvaluatedRuleTracker records labels from annotations during evaluation. +// Labels from all successfully evaluated rules are aggregated. Exact +// duplicates (same key-value pairs) are suppressed. type EvaluatedRuleTracker struct { - IDs []string - seen map[string]struct{} + Labels []map[string]any + seen map[string]struct{} } func (t *EvaluatedRuleTracker) Record(rule *ast.Rule) { @@ -16,15 +24,16 @@ func (t *EvaluatedRuleTracker) Record(rule *ast.Rule) { } for _, a := range rule.Annotations { - if a.ID != "" { + if len(a.Labels) > 0 { + b, _ := json.Marshal(a.Labels) + key := string(b) if t.seen == nil { t.seen = make(map[string]struct{}) } - if _, dup := t.seen[a.ID]; !dup { - t.seen[a.ID] = struct{}{} - t.IDs = append(t.IDs, a.ID) + if _, dup := t.seen[key]; !dup { + t.seen[key] = struct{}{} + t.Labels = append(t.Labels, a.Labels) } - return } } } diff --git a/v1/topdown/evaluated_test.go b/v1/topdown/evaluated_test.go new file mode 100644 index 0000000000..2dffe518e7 --- /dev/null +++ b/v1/topdown/evaluated_test.go @@ -0,0 +1,232 @@ +// Copyright 2026 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +package topdown_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/open-policy-agent/opa/v1/ast" + "github.com/open-policy-agent/opa/v1/storage" + inmem "github.com/open-policy-agent/opa/v1/storage/inmem/test" + "github.com/open-policy-agent/opa/v1/topdown" +) + +func TestEvaluatedRuleTracker(t *testing.T) { + t.Run("collects labels", func(t *testing.T) { + tracker := &topdown.EvaluatedRuleTracker{} + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{ + {Labels: map[string]any{"severity": "low"}}, + }, + }) + if len(tracker.Labels) != 1 { + t.Fatalf("expected 1 label set, got %d", len(tracker.Labels)) + } + if tracker.Labels[0]["severity"] != "low" { + t.Fatalf("expected severity=low, got %v", tracker.Labels[0]) + } + }) + + t.Run("deduplicates identical labels", func(t *testing.T) { + tracker := &topdown.EvaluatedRuleTracker{} + labels := map[string]any{"team": "platform", "severity": "high"} + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{{Labels: labels}}, + }) + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{{Labels: labels}}, + }) + if len(tracker.Labels) != 1 { + t.Fatalf("expected 1 label set (deduplicated), got %d", len(tracker.Labels)) + } + }) + + t.Run("keeps distinct labels", func(t *testing.T) { + tracker := &topdown.EvaluatedRuleTracker{} + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{ + {Labels: map[string]any{"severity": "low"}}, + }, + }) + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{ + {Labels: map[string]any{"severity": "high"}}, + }, + }) + if len(tracker.Labels) != 2 { + t.Fatalf("expected 2 label sets, got %d", len(tracker.Labels)) + } + }) + + t.Run("nil tracker is safe", func(t *testing.T) { + var tracker *topdown.EvaluatedRuleTracker + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{ + {Labels: map[string]any{"x": "y"}}, + }, + }) + }) + + t.Run("skips rules without labels", func(t *testing.T) { + tracker := &topdown.EvaluatedRuleTracker{} + tracker.Record(&ast.Rule{ + Annotations: []*ast.Annotations{ + {Custom: map[string]any{"foo": "bar"}}, + }, + }) + if len(tracker.Labels) != 0 { + t.Fatalf("expected 0 label sets, got %d", len(tracker.Labels)) + } + }) + + t.Run("skips rules without annotations", func(t *testing.T) { + tracker := &topdown.EvaluatedRuleTracker{} + tracker.Record(&ast.Rule{}) + if len(tracker.Labels) != 0 { + t.Fatalf("expected 0 label sets, got %d", len(tracker.Labels)) + } + }) +} + +func TestEvaluatedRuleLabelsScopes(t *testing.T) { + tests := []struct { + note string + module string + query string + input string + exp []map[string]any + }{ + { + note: "rule scope labels", + module: `package test + +# METADATA +# labels: +# severity: high +# team: security +allow if input.role == "admin" +`, + query: "data.test.allow", + exp: []map[string]any{ + {"severity": "high", "team": "security"}, + }, + }, + { + note: "document scope labels apply to all rules in document", + module: `package test + +# METADATA +# scope: document +# labels: +# component: authz +allow if input.role == "admin" + +allow if input.role == "superuser" +`, + query: "data.test.allow", + exp: []map[string]any{ + {"component": "authz"}, + }, + }, + { + note: "rule and document scope combine", + module: `package test + +# METADATA +# scope: document +# labels: +# component: authz + +# METADATA +# labels: +# severity: high +allow if input.role == "admin" + +# METADATA +# labels: +# severity: low +allow if input.role == "viewer" +`, + query: "data.test.allow", + exp: []map[string]any{ + {"component": "authz"}, + {"severity": "high"}, + }, + }, + { + note: "no labels when rule not satisfied", + module: `package test + +# METADATA +# labels: +# severity: high +allow if input.role == "admin" +`, + query: "data.test.allow", + input: `{"role": "guest"}`, + exp: nil, + }, + { + note: "multiple rules each contribute labels", + module: `package test + +# METADATA +# labels: +# id: allow-admin +allow if input.role == "admin" + +# METADATA +# labels: +# id: allow-viewer +allow if input.role == "viewer" +`, + query: "data.test.allow", + exp: []map[string]any{ + {"id": "allow-admin"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + mod := ast.MustParseModuleWithOpts(tc.module, ast.ParserOptions{ProcessAnnotation: true}) + c := ast.NewCompiler() + c.Compile(map[string]*ast.Module{"test": mod}) + if c.Failed() { + t.Fatal(c.Errors) + } + + inputStr := tc.input + if inputStr == "" { + inputStr = `{"role": "admin"}` + } + input := ast.MustParseTerm(inputStr) + store := inmem.New() + ctx := t.Context() + txn := storage.NewTransactionOrDie(ctx, store) + defer store.Abort(ctx, txn) + + tracker := &topdown.EvaluatedRuleTracker{} + query := topdown.NewQuery(ast.MustParseBody(tc.query)). + WithCompiler(c). + WithStore(store). + WithTransaction(txn). + WithInput(input). + WithEvaluatedRuleTracker(tracker) + + _, err := query.Run(ctx) + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(tc.exp, tracker.Labels, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("unexpected labels (-want, +got):\n%s", diff) + } + }) + } +}