Skip to content

Commit 1fcccfb

Browse files
committed
fix e2e tests, pass ProcessAnnotation in bundle downloaders
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
1 parent d97f5d4 commit 1fcccfb

10 files changed

Lines changed: 92 additions & 10 deletions

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ The resulting decision log entry will contain:
4949

5050
Embedding projects can change the default by setting `config.DefaultIncludeRuleMetadata = true`.
5151

52+
Note: when using `include_rule_metadata` via the configuration file, policies must be loaded
53+
via the bundle plugin (i.e. configured through `services` and `bundles` in the config) for
54+
annotations to be processed correctly. Policies loaded from command-line paths are parsed
55+
before the configuration is available.
56+
5257
## 1.16.1
5358

5459
This is a patch release addressing a regression ([#8590](https://github.com/open-policy-agent/opa/pull/8590)) in the plugin manager that may cause the service to hang on shutdown.

e2e/cli/evaluated_rules.txtar

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22
# have metadata annotations with id fields, and in the response when
33
# the ?ids query parameter is used.
44

5-
exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --set decision_logs.console=true ./policy/ &opa&
6-
retry curl -sf --unix-socket opa.sock 'http://localhost/health'
5+
exec $OPA build -o bundles/bundle.tar.gz --bundle policy/
6+
7+
http_server bundles
8+
9+
exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml &opa&
10+
retry curl -sf --unix-socket opa.sock 'http://localhost/health?bundles'
711

812
# POST to v1/data — rule with id "allow-admin" should be evaluated.
913
exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow
@@ -31,6 +35,21 @@ wait opa
3135
# All requests that matched allow-admin should have ids in the decision log.
3236
jq -s stderr '[.[] | select(.msg == "Decision Log" and .ids == ["allow-admin"])] | length == 3'
3337

38+
-- config.yaml --
39+
include_rule_metadata: true
40+
services:
41+
bundle-server:
42+
url: "http://${HTTP_ADDR}"
43+
bundles:
44+
test:
45+
service: bundle-server
46+
resource: bundle.tar.gz
47+
polling:
48+
min_delay_seconds: 300
49+
max_delay_seconds: 300
50+
decision_logs:
51+
console: true
52+
3453
-- policy/authz.rego --
3554
package authz
3655

@@ -58,3 +77,5 @@ violations contains msg if {
5877
input.dept == "eng"
5978
msg := "admin not allowed in eng"
6079
}
80+
81+
-- bundles/.gitkeep --

e2e/cli/evaluated_rules_file_logger.txtar

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
# Verify that evaluated rule IDs appear in decision logs written via the
22
# file_logger plugin (slog-based path).
33

4-
exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml ./policy/ &opa&
5-
retry curl -sf --unix-socket opa.sock 'http://localhost/health'
4+
exec $OPA build -o bundles/bundle.tar.gz --bundle policy/
5+
6+
http_server bundles
7+
8+
exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml &opa&
9+
retry curl -sf --unix-socket opa.sock 'http://localhost/health?bundles'
610

711
# POST to v1/data — rule with id "allow-admin" should be evaluated.
812
exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow
@@ -22,6 +26,17 @@ jq -s decisions.log '[.[] | select(.ids == ["allow-admin"])] | length == 1'
2226
jq -s decisions.log '[.[] | select(.path == "authz/allow" and (.ids == null))] | length == 1'
2327

2428
-- config.yaml --
29+
include_rule_metadata: true
30+
services:
31+
bundle-server:
32+
url: "http://${HTTP_ADDR}"
33+
bundles:
34+
test:
35+
service: bundle-server
36+
resource: bundle.tar.gz
37+
polling:
38+
min_delay_seconds: 300
39+
max_delay_seconds: 300
2540
plugins:
2641
file_logger:
2742
path: decisions.log
@@ -40,3 +55,5 @@ allow if input.role == "admin"
4055
# METADATA
4156
# id: allow-editor
4257
allow if input.role == "editor"
58+
59+
-- bundles/.gitkeep --

e2e/cli/rule_metadata_dl.txtar

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
# Verify that custom metadata from evaluated rule annotations appears in
22
# decision logs when include_rule_metadata is enabled.
33

4-
exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --set decision_logs.console=true --set include_rule_metadata=true ./policy/ &opa&
5-
retry curl -sf --unix-socket opa.sock 'http://localhost/health'
4+
exec $OPA build -o bundles/bundle.tar.gz --bundle policy/
5+
6+
http_server bundles
7+
8+
exec $OPA run --server --addr unix://opa.sock --log-format json --log-level info --config-file config.yaml &opa&
9+
retry curl -sf --unix-socket opa.sock 'http://localhost/health?bundles'
610

711
# Evaluate allow — triggers allow-admin rule.
812
exec curl -sf --unix-socket opa.sock -H 'Content-Type: application/json' -d '{"input": {"role": "admin"}}' http://localhost/v1/data/authz/allow
@@ -21,6 +25,21 @@ jq -s stderr '[.[] | select(.msg == "Decision Log" and .path == "authz/allow")]
2125
# Extract the decision log for violations and compare sorted rule_metadata.
2226
jq -s stderr '[.[] | select(.msg == "Decision Log" and .path == "authz/violations")] | .[0].custom.rule_metadata | sort_by(.id) == [{"id": "no-admin-in-eng", "severity": "critical", "category": "org-policy"}, {"id": "no-admin-role", "severity": "high", "category": "compliance"}]'
2327

28+
-- config.yaml --
29+
include_rule_metadata: true
30+
services:
31+
bundle-server:
32+
url: "http://${HTTP_ADDR}"
33+
bundles:
34+
test:
35+
service: bundle-server
36+
resource: bundle.tar.gz
37+
polling:
38+
min_delay_seconds: 300
39+
max_delay_seconds: 300
40+
decision_logs:
41+
console: true
42+
2443
-- policy/authz.rego --
2544
package authz
2645

@@ -54,3 +73,5 @@ violations contains "admin not allowed in eng" if {
5473
input.role == "admin"
5574
input.dept == "eng"
5675
}
76+
77+
-- bundles/.gitkeep --

v1/download/download.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ func (d *Downloader) download(ctx context.Context, m metrics.Metrics) (*download
347347

348348
reader := bundle.NewCustomReader(loader).
349349
WithRegoVersion(d.bundleParserOpts.RegoVersion).
350+
WithProcessAnnotations(d.bundleParserOpts.ProcessAnnotation).
350351
WithMetrics(m).
351352
WithBundleVerificationConfig(d.bvc).
352353
WithBundleEtag(etag).

v1/download/oci_download.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ func (d *OCIDownloader) download(ctx context.Context, m metrics.Metrics) (*downl
276276
WithMetrics(m).
277277
WithBundleVerificationConfig(d.bvc).
278278
WithBundleEtag(etag).
279-
WithRegoVersion(d.bundleParserOpts.RegoVersion)
279+
WithRegoVersion(d.bundleParserOpts.RegoVersion).
280+
WithProcessAnnotations(d.bundleParserOpts.ProcessAnnotation)
280281
bundleInfo, err := reader.Read()
281282
if err != nil {
282283
return &downloaderResponse{}, fmt.Errorf("unexpected error %w", err)

v1/plugins/bundle/plugin.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,7 @@ func (fl *fileLoader) oneShot(ctx context.Context) (err error) {
880880
WithLazyLoadingMode(bundle.HasExtension()).
881881
WithSizeLimitBytes(fl.sizeLimitBytes).
882882
WithRegoVersion(fl.bundleParserOpts.RegoVersion).
883+
WithProcessAnnotations(fl.bundleParserOpts.ProcessAnnotation).
883884
Read()
884885
u.Error = err
885886
if err == nil {

v1/rego/rego.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,7 @@ type Rego struct {
691691
enablePrintStatements bool
692692
distributedTracingOpts tracing.Options
693693
strict bool
694+
processAnnotations bool
694695
targetPrepState TargetPluginEval
695696
regoVersion ast.RegoVersion
696697
compilerHook func(*ast.Compiler)
@@ -1397,6 +1398,13 @@ func EvaluatedRuleTracker(t *topdown.EvaluatedRuleTracker) func(r *Rego) {
13971398
}
13981399
}
13991400

1401+
// ProcessAnnotations enables metadata annotation processing during module parsing.
1402+
func ProcessAnnotations(yes bool) func(r *Rego) {
1403+
return func(r *Rego) {
1404+
r.processAnnotations = yes
1405+
}
1406+
}
1407+
14001408
// New returns a new Rego object.
14011409
func New(options ...func(r *Rego)) *Rego {
14021410
r := &Rego{
@@ -2000,8 +2008,9 @@ func (r *Rego) parseModules(ctx context.Context, txn storage.Transaction, m metr
20002008
var errs Errors
20012009

20022010
popts := ast.ParserOptions{
2003-
RegoVersion: r.regoVersion,
2004-
Capabilities: r.capabilities,
2011+
ProcessAnnotation: r.processAnnotations,
2012+
RegoVersion: r.regoVersion,
2013+
Capabilities: r.capabilities,
20052014
}
20062015

20072016
// Parse any modules that are saved to the store, but only if

v1/rego/rego_evaluated_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ p if input.bar`
2323
Query("data.test.p"),
2424
Module("test.rego", module),
2525
Input(map[string]any{"foo": true}),
26+
ProcessAnnotations(true),
2627
EvaluatedRuleTracker(tracker),
2728
).Eval(t.Context())
2829
if err != nil {
@@ -42,6 +43,7 @@ p if input.bar`
4243
Query("data.test.p"),
4344
Module("test.rego", module),
4445
Input(map[string]any{"baz": true}),
46+
ProcessAnnotations(true),
4547
EvaluatedRuleTracker(tracker),
4648
).Eval(t.Context())
4749

@@ -55,6 +57,7 @@ p if input.bar`
5557
Query("data.test.p"),
5658
Module("test.rego", module),
5759
Input(map[string]any{"foo": true}),
60+
ProcessAnnotations(true),
5861
EvaluatedRuleTracker(nil),
5962
).Eval(t.Context())
6063
if err != nil {
@@ -66,6 +69,7 @@ p if input.bar`
6669
pq, err := New(
6770
Query("data.test.p"),
6871
Module("test.rego", module),
72+
ProcessAnnotations(true),
6973
).PrepareForEval(t.Context())
7074
if err != nil {
7175
t.Fatal(err)
@@ -103,6 +107,7 @@ result if {
103107
rs, err := New(
104108
Query("data.test.result"),
105109
Module("test.rego", mod),
110+
ProcessAnnotations(true),
106111
EvaluatedRuleTracker(tracker),
107112
).Eval(t.Context())
108113
if err != nil {

v1/sdk/opa_evaluated_rules_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ xs contains "b" if false
7676
},
7777
"decision_logs": {
7878
"console": true
79-
}
79+
},
80+
"include_rule_metadata": true
8081
}`, server.URL())
8182

8283
testLogger := test.New()

0 commit comments

Comments
 (0)