Skip to content

Commit b7b798f

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

9 files changed

Lines changed: 87 additions & 17 deletions

File tree

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_evaluated_test.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,26 @@ package rego
33
import (
44
"testing"
55

6+
"github.com/open-policy-agent/opa/v1/ast"
67
"github.com/open-policy-agent/opa/v1/topdown"
78
)
89

910
func TestEvaluatedRules(t *testing.T) {
10-
module := `package test
11+
module := ast.MustParseModuleWithOpts(`package test
1112
1213
# METADATA
1314
# id: rule1
1415
p if input.foo
1516
1617
# METADATA
1718
# id: rule2
18-
p if input.bar`
19+
p if input.bar`, ast.ParserOptions{ProcessAnnotation: true})
1920

2021
t.Run("records matching rule", func(t *testing.T) {
2122
tracker := &topdown.EvaluatedRuleTracker{}
2223
rs, err := New(
2324
Query("data.test.p"),
24-
Module("test.rego", module),
25+
ParsedModule(module),
2526
Input(map[string]any{"foo": true}),
2627
EvaluatedRuleTracker(tracker),
2728
).Eval(t.Context())
@@ -40,7 +41,7 @@ p if input.bar`
4041
tracker := &topdown.EvaluatedRuleTracker{}
4142
_, _ = New(
4243
Query("data.test.p"),
43-
Module("test.rego", module),
44+
ParsedModule(module),
4445
Input(map[string]any{"baz": true}),
4546
EvaluatedRuleTracker(tracker),
4647
).Eval(t.Context())
@@ -53,7 +54,7 @@ p if input.bar`
5354
t.Run("nil tracker is safe", func(t *testing.T) {
5455
_, err := New(
5556
Query("data.test.p"),
56-
Module("test.rego", module),
57+
ParsedModule(module),
5758
Input(map[string]any{"foo": true}),
5859
EvaluatedRuleTracker(nil),
5960
).Eval(t.Context())
@@ -65,7 +66,7 @@ p if input.bar`
6566
t.Run("prepared query", func(t *testing.T) {
6667
pq, err := New(
6768
Query("data.test.p"),
68-
Module("test.rego", module),
69+
ParsedModule(module),
6970
).PrepareForEval(t.Context())
7071
if err != nil {
7172
t.Fatal(err)
@@ -88,7 +89,7 @@ p if input.bar`
8889
})
8990

9091
t.Run("function called multiple times is deduplicated", func(t *testing.T) {
91-
mod := `package test
92+
mod := ast.MustParseModuleWithOpts(`package test
9293
9394
# METADATA
9495
# id: is-allowed
@@ -98,11 +99,12 @@ result if {
9899
is_allowed("alice")
99100
is_allowed("bob")
100101
is_allowed("charlie")
101-
}`
102+
}`, ast.ParserOptions{ProcessAnnotation: true})
103+
102104
tracker := &topdown.EvaluatedRuleTracker{}
103105
rs, err := New(
104106
Query("data.test.result"),
105-
Module("test.rego", mod),
107+
ParsedModule(mod),
106108
EvaluatedRuleTracker(tracker),
107109
).Eval(t.Context())
108110
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)