Skip to content

Commit 1a874c8

Browse files
committed
runtime+config: drive annotation parsing from new configurable
...not from an extra config flag. Restores the default to `false` for anyone not setting `opa_config.DefaultIncludeRuleMetadata` or not configuring `include_rule_metadata: true`. Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
1 parent a022a7d commit 1a874c8

7 files changed

Lines changed: 20 additions & 48 deletions

File tree

CHANGELOG.md

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,6 @@ project adheres to [Semantic Versioning](http://semver.org/).
55

66
## Unreleased
77

8-
### Runtime Defaults to Parsing Metadata Annotations
9-
10-
The OPA runtime (server mode) now processes rule metadata annotations by default.
11-
Previously, annotations were only parsed when explicitly enabled or when a rule
12-
contained an `id` field. This ensures annotations are always available during
13-
evaluation, regardless of how policies are loaded (bundles, files, or watch mode).
14-
15-
To restore the previous behavior, set `skip_annotation_processing` in the configuration:
16-
17-
```yaml
18-
skip_annotation_processing: true
19-
```
20-
218
### Evaluated Rules in Decision Logs and API Responses
229

2310
Rules can now be annotated with a metadata `id` field. When any loaded policy contains

docs/docs/configuration.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,8 +1036,7 @@ The `server` configuration sets:
10361036
| `persistence_directory` | `string` | No (default `$PWD/.opa`) | Set directory to use for persistence with options like `bundles[_].persist`. |
10371037
| `plugins` | `object` | No (default: `{}`) | Location for custom plugin configuration. |
10381038
| `nd_builtin_cache` | `boolean` | No (default: `false`) | Enable the non-deterministic builtins caching system during policy evaluation, and include the contents of the cache in decision logs. Note that decision logs that are larger than `upload_size_limit_bytes` will drop the `nd_builtin_cache` key from the log entry before uploading. |
1039-
| `skip_annotation_processing` | `boolean` | No (default: `false`) | Disable metadata annotation processing during policy parsing. By default, the runtime processes annotations so they are available during evaluation. |
1040-
| `include_rule_metadata` | `boolean` | No (default: `false`) | Include custom metadata from evaluated rule annotations in decision logs. When enabled, the `custom` field of each decision log entry will contain a `rule_metadata` key with the custom annotation fields of all successfully evaluated rules. |
1039+
| `include_rule_metadata` | `boolean` | No (default: `false`) | Include custom metadata from evaluated rule annotations in decision logs. When enabled, the `custom` field of each decision log entry will contain a `rule_metadata` key with the custom annotation fields of all successfully evaluated rules. Implies annotation processing. |
10411040

10421041
## Using Environment Variables in Configuration
10431042

v1/config/config.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ type Config struct {
102102
DefaultAuthorizationDecision *string `json:"default_authorization_decision,omitempty"`
103103
Caching json.RawMessage `json:"caching,omitempty"`
104104
NDBuiltinCache bool `json:"nd_builtin_cache,omitempty"`
105-
SkipAnnotationProcessing bool `json:"skip_annotation_processing,omitempty"`
106105
IncludeRuleMetadata bool `json:"include_rule_metadata,omitempty"`
107106
PersistenceDirectory *string `json:"persistence_directory,omitempty"`
108107
DistributedTracing json.RawMessage `json:"distributed_tracing,omitempty"`
@@ -185,11 +184,6 @@ func (c Config) NDBuiltinCacheEnabled() bool {
185184
return c.NDBuiltinCache
186185
}
187186

188-
// AnnotationProcessingSkipped returns true if annotation processing should be disabled.
189-
func (c Config) AnnotationProcessingSkipped() bool {
190-
return c.SkipAnnotationProcessing
191-
}
192-
193187
// GetPersistenceDirectory returns the configured persistence directory, or $PWD/.opa if none is configured
194188
func (c Config) GetPersistenceDirectory() (string, error) {
195189
if c.PersistenceDirectory == nil {
@@ -240,12 +234,11 @@ func (c *Config) Clone() *Config {
240234
}
241235

242236
clone := &Config{
243-
NDBuiltinCache: c.NDBuiltinCache,
244-
SkipAnnotationProcessing: c.SkipAnnotationProcessing,
245-
IncludeRuleMetadata: c.IncludeRuleMetadata,
246-
Server: c.Server.Clone(),
247-
Storage: c.Storage.Clone(),
248-
Labels: maps.Clone(c.Labels),
237+
NDBuiltinCache: c.NDBuiltinCache,
238+
IncludeRuleMetadata: c.IncludeRuleMetadata,
239+
Server: c.Server.Clone(),
240+
Storage: c.Storage.Clone(),
241+
Labels: maps.Clone(c.Labels),
249242
}
250243

251244
if c.Services != nil {

v1/config/default.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package config
66

77
// DefaultIncludeRuleMetadata controls whether custom metadata from rule annotations
8-
// is included in decision logs by default. Embedding projects can set this to true
9-
// via an init() function or linker flag to change the default.
8+
// is included in decision logs by default. When true, this also enables annotation
9+
// processing during policy parsing. Embedding projects can set this to true via an
10+
// init() function to change the default.
1011
var DefaultIncludeRuleMetadata bool

v1/plugins/plugins.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,10 @@ func New(raw []byte, id string, store storage.Store, opts ...func(*Manager)) (*M
562562
m.parserOptions.RegoVersion = ast.DefaultRegoVersion
563563
}
564564

565+
if parsedConfig.IncludeRuleMetadata {
566+
m.parserOptions.ProcessAnnotation = true
567+
}
568+
565569
if m.logger == nil {
566570
m.logger = logging.Get()
567571
}

v1/runtime/runtime.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,6 @@ type Params struct {
291291
// NDBCacheEnabled allows enabling the non-deterministic builtin cache globally.
292292
NDBCacheEnabled bool
293293

294-
// SkipAnnotationProcessing disables metadata annotation processing during policy parsing.
295-
// By default, the runtime processes annotations. Set this to true to restore the old behavior.
296-
SkipAnnotationProcessing bool
297-
298294
// IncludeRuleMetadataInDecisionLogs enables carry-over of custom metadata fields
299295
// from evaluated rule annotations into the decision log "custom" field.
300296
IncludeRuleMetadataInDecisionLogs bool
@@ -313,9 +309,9 @@ func (p *Params) regoVersion() ast.RegoVersion {
313309
return ast.DefaultRegoVersion
314310
}
315311

316-
func (p *Params) parserOptions(skipAnnotations bool) ast.ParserOptions {
312+
func (p *Params) parserOptions() ast.ParserOptions {
317313
return ast.ParserOptions{
318-
ProcessAnnotation: !skipAnnotations,
314+
ProcessAnnotation: opa_config.DefaultIncludeRuleMetadata || p.IncludeRuleMetadataInDecisionLogs,
319315
RegoVersion: p.regoVersion(),
320316
}
321317
}
@@ -441,7 +437,7 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) {
441437

442438
regoVersion := params.regoVersion()
443439

444-
loaded, err := initload.LoadPathsForRegoVersion(regoVersion, params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, params.BundleLazyLoadingMode, !params.SkipAnnotationProcessing, false, nil, nil)
440+
loaded, err := initload.LoadPathsForRegoVersion(regoVersion, params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, params.BundleLazyLoadingMode, opa_config.DefaultIncludeRuleMetadata || params.IncludeRuleMetadataInDecisionLogs, false, nil, nil)
445441
if err != nil {
446442
return nil, fmt.Errorf("load error: %w", err)
447443
}
@@ -533,7 +529,7 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) {
533529
plugins.WithPrometheusRegister(metrics),
534530
plugins.WithTracerProvider(tracerProvider),
535531
plugins.WithEnableVersionCheck(params.EnableVersionCheck),
536-
plugins.WithParserOptions(params.parserOptions(params.SkipAnnotationProcessing)),
532+
plugins.WithParserOptions(params.parserOptions()),
537533
plugins.WithDistributedTracingOpts(params.DistributedTracingOpts),
538534
plugins.WithBundleActivatorPlugin(params.BundleActivatorPlugin),
539535
plugins.WithHooks(params.Hooks),

v1/server/server.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -450,14 +450,10 @@ func (s *Server) WithIncludeRuleMetadata(include bool) *Server {
450450
}
451451

452452
func (s *Server) newEvaluatedRuleTracker(force bool) *topdown.EvaluatedRuleTracker {
453-
if !force && !s.evaluatedRulesEnabled && !s.hasExternalSources() && !s.includeRuleMetadata {
454-
return nil
455-
}
456-
t := &topdown.EvaluatedRuleTracker{}
457-
if s.includeRuleMetadata {
458-
t.CollectCustom = true
453+
if force || s.evaluatedRulesEnabled || s.includeRuleMetadata {
454+
return &topdown.EvaluatedRuleTracker{CollectCustom: s.includeRuleMetadata}
459455
}
460-
return t
456+
return nil
461457
}
462458

463459
func compilerHasRuleIDs(c *ast.Compiler) bool {
@@ -476,10 +472,6 @@ func compilerHasRuleIDs(c *ast.Compiler) bool {
476472
return false
477473
}
478474

479-
func (s *Server) hasExternalSources() bool {
480-
return s.manager.GetExternalSources() != nil && s.manager.GetExternalSources().Len() > 0
481-
}
482-
483475
func evaluatedRuleIDs(t *topdown.EvaluatedRuleTracker) []string {
484476
if t == nil || len(t.IDs) == 0 {
485477
return nil

0 commit comments

Comments
 (0)