Skip to content

Commit aa81e31

Browse files
johanfyllingjohanfylling
authored
bundle: Setting default rego-version in bundle API (#7630)
Applying default rego-version for `bundle.Activate()` and `bundle.Deactivate()`. Fixes: #7588 Signed-off-by: Johan Fylling <[email protected]>
1 parent 707c43d commit aa81e31

File tree

3 files changed

+573
-2
lines changed

3 files changed

+573
-2
lines changed

‎bundle/store.go‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package bundle
77
import (
88
"context"
99

10+
"github.com/open-policy-agent/opa/ast"
1011
"github.com/open-policy-agent/opa/storage"
1112
v1 "github.com/open-policy-agent/opa/v1/bundle"
1213
)
@@ -87,15 +88,15 @@ type ActivateOpts = v1.ActivateOpts
8788
// Activate the bundle(s) by loading into the given Store. This will load policies, data, and record
8889
// the manifest in storage. The compiler provided will have had the polices compiled on it.
8990
func Activate(opts *ActivateOpts) error {
90-
return v1.Activate(opts)
91+
return v1.Activate(setActivateDefaultRegoVersion(opts))
9192
}
9293

9394
// DeactivateOpts defines options for the Deactivate API call
9495
type DeactivateOpts = v1.DeactivateOpts
9596

9697
// Deactivate the bundle(s). This will erase associated data, policies, and the manifest entry from the store.
9798
func Deactivate(opts *DeactivateOpts) error {
98-
return v1.Deactivate(opts)
99+
return v1.Deactivate(setDeactivateDefaultRegoVersion(opts))
99100
}
100101

101102
// LegacyWriteManifestToStore will write the bundle manifest to the older single (unnamed) bundle manifest location.
@@ -121,3 +122,31 @@ func LegacyReadRevisionFromStore(ctx context.Context, store storage.Store, txn s
121122
func ActivateLegacy(opts *ActivateOpts) error {
122123
return v1.ActivateLegacy(opts)
123124
}
125+
126+
func setActivateDefaultRegoVersion(opts *ActivateOpts) *ActivateOpts {
127+
if opts == nil {
128+
return nil
129+
}
130+
131+
if opts.ParserOptions.RegoVersion == ast.RegoUndefined {
132+
cpy := *opts
133+
cpy.ParserOptions.RegoVersion = ast.DefaultRegoVersion
134+
return &cpy
135+
}
136+
137+
return opts
138+
}
139+
140+
func setDeactivateDefaultRegoVersion(opts *DeactivateOpts) *DeactivateOpts {
141+
if opts == nil {
142+
return nil
143+
}
144+
145+
if opts.ParserOptions.RegoVersion == ast.RegoUndefined {
146+
cpy := *opts
147+
cpy.ParserOptions.RegoVersion = ast.DefaultRegoVersion
148+
return &cpy
149+
}
150+
151+
return opts
152+
}

‎bundle/store_test.go‎

Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ package bundle
66

77
import (
88
"context"
9+
"fmt"
10+
"strings"
911
"testing"
1012

13+
"github.com/open-policy-agent/opa/ast"
1114
"github.com/open-policy-agent/opa/internal/storage/mock"
15+
"github.com/open-policy-agent/opa/metrics"
1216
"github.com/open-policy-agent/opa/storage"
1317
)
1418

@@ -90,3 +94,281 @@ func TestHasRootsOverlap(t *testing.T) {
9094
})
9195
}
9296
}
97+
98+
func TestActivate_DefaultRegoVersion(t *testing.T) {
99+
tests := []struct {
100+
note string
101+
module string
102+
customRegoVersion ast.RegoVersion
103+
expErrs []string
104+
}{
105+
// default rego-version
106+
{
107+
note: "v0 module, no v1 parse-time violations",
108+
module: `package test
109+
p[x] {
110+
x = "a"
111+
}`,
112+
},
113+
{
114+
note: "v0 module, v1 parse-time violations",
115+
module: `package test
116+
117+
contains[x] {
118+
x = "a"
119+
}`,
120+
},
121+
122+
// cross-rego-version
123+
{
124+
note: "rego.v1 import, no v1 parse-time violations",
125+
module: `package test
126+
import rego.v1
127+
128+
p contains x if {
129+
x = "a"
130+
}`,
131+
},
132+
{
133+
note: "rego.v1 import, v1 parse-time violations",
134+
module: `package test
135+
import rego.v1
136+
137+
p contains x {
138+
x = "a"
139+
}`,
140+
expErrs: []string{
141+
"rego_parse_error: `if` keyword is required before rule body",
142+
},
143+
},
144+
145+
// NOT default rego-version
146+
{
147+
note: "v1 module",
148+
module: `package test
149+
150+
p contains x if {
151+
x = "a"
152+
}`,
153+
expErrs: []string{
154+
"rego_parse_error: var cannot be used for rule name",
155+
},
156+
},
157+
158+
// custom rego-version
159+
{
160+
note: "v1 module, v1 custom rego-version",
161+
module: `package test
162+
163+
p contains x if {
164+
x = "a"
165+
}`,
166+
customRegoVersion: ast.RegoV1,
167+
},
168+
{
169+
note: "v1 module, v1 custom rego-version, v1 parse-time violations",
170+
module: `package test
171+
172+
p contains x {
173+
x = "a"
174+
}`,
175+
customRegoVersion: ast.RegoV1,
176+
expErrs: []string{
177+
"rego_parse_error: `if` keyword is required before rule body",
178+
},
179+
},
180+
}
181+
182+
for _, tc := range tests {
183+
t.Run(tc.note, func(t *testing.T) {
184+
ctx := context.Background()
185+
store := mock.New()
186+
txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams)
187+
compiler := ast.NewCompiler().WithDefaultRegoVersion(ast.RegoV0CompatV1)
188+
m := metrics.New()
189+
190+
bundleName := "bundle1"
191+
modulePath := "test/policy.rego"
192+
193+
// We want to make assert that the default rego-version is used, which it is when a module is erased from storage and we don't know what version it has.
194+
// Therefore, we add a module to the store, which is the replaced by the Activate() call, causing an erase.
195+
if err := store.UpsertPolicy(ctx, txn, fmt.Sprintf("%s/%s", bundleName, modulePath), []byte(tc.module)); err != nil {
196+
t.Fatalf("unexpected error: %s", err)
197+
}
198+
199+
newModule := `package test`
200+
bundles := map[string]*Bundle{
201+
bundleName: {
202+
Manifest: Manifest{
203+
Roots: &[]string{"test"},
204+
},
205+
Modules: []ModuleFile{
206+
{
207+
Path: modulePath,
208+
Raw: []byte(newModule),
209+
Parsed: ast.MustParseModule(newModule),
210+
},
211+
},
212+
},
213+
}
214+
215+
opts := ActivateOpts{
216+
Ctx: ctx,
217+
Txn: txn,
218+
Store: store,
219+
Compiler: compiler,
220+
Metrics: m,
221+
Bundles: bundles,
222+
}
223+
224+
if tc.customRegoVersion != ast.RegoUndefined {
225+
opts.ParserOptions.RegoVersion = tc.customRegoVersion
226+
}
227+
228+
err := Activate(&opts)
229+
230+
if len(tc.expErrs) > 0 {
231+
if err == nil {
232+
t.Fatalf("Expected error but got nil for test: %s", tc.note)
233+
}
234+
for _, expErr := range tc.expErrs {
235+
if err := err.Error(); !strings.Contains(err, expErr) {
236+
t.Fatalf("Expected error to contain:\n\n%s\n\nbut got:\n\n%s", expErr, err)
237+
}
238+
}
239+
} else if err != nil {
240+
t.Fatalf("Unexpected error: %v", err)
241+
}
242+
})
243+
}
244+
}
245+
246+
func TestDeactivate_DefaultRegoVersion(t *testing.T) {
247+
tests := []struct {
248+
note string
249+
module string
250+
customRegoVersion ast.RegoVersion
251+
expErrs []string
252+
}{
253+
// default rego-version
254+
{
255+
note: "v0 module, no v1 parse-time violations",
256+
module: `package test
257+
p[x] {
258+
x = "a"
259+
}`,
260+
},
261+
{
262+
note: "v0 module, v1 parse-time violations",
263+
module: `package test
264+
265+
contains[x] {
266+
x = "a"
267+
}`,
268+
},
269+
270+
// cross-rego-version
271+
{
272+
note: "rego.v1 import, no v1 parse-time violations",
273+
module: `package test
274+
import rego.v1
275+
276+
p contains x if {
277+
x = "a"
278+
}`,
279+
},
280+
{
281+
note: "rego.v1 import, v1 parse-time violations",
282+
module: `package test
283+
import rego.v1
284+
285+
p contains x {
286+
x = "a"
287+
}`,
288+
expErrs: []string{
289+
"rego_parse_error: `if` keyword is required before rule body",
290+
},
291+
},
292+
293+
// NOT default rego-version
294+
{
295+
note: "v1 module",
296+
module: `package test
297+
298+
p contains x if {
299+
x = "a"
300+
}`,
301+
expErrs: []string{
302+
"rego_parse_error: var cannot be used for rule name",
303+
},
304+
},
305+
306+
// custom rego-version
307+
{
308+
note: "v1 module, v1 custom rego-version",
309+
module: `package test
310+
311+
p contains x if {
312+
x = "a"
313+
}`,
314+
customRegoVersion: ast.RegoV1,
315+
},
316+
{
317+
note: "v1 module, v1 custom rego-version, v1 parse-time violations",
318+
module: `package test
319+
320+
p contains x {
321+
x = "a"
322+
}`,
323+
customRegoVersion: ast.RegoV1,
324+
expErrs: []string{
325+
"rego_parse_error: `if` keyword is required before rule body",
326+
},
327+
},
328+
}
329+
330+
for _, tc := range tests {
331+
t.Run(tc.note, func(t *testing.T) {
332+
ctx := context.Background()
333+
store := mock.New()
334+
txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams)
335+
336+
bundleName := "bundle1"
337+
modulePath := "test/policy.rego"
338+
339+
// We want to make assert that the default rego-version is used, which it is when a module is erased from storage and we don't know what version it has.
340+
// Therefore, we add a module to the store, which is the replaced by the Activate() call, causing an erase.
341+
if err := store.UpsertPolicy(ctx, txn, fmt.Sprintf("%s/%s", bundleName, modulePath), []byte(tc.module)); err != nil {
342+
t.Fatalf("unexpected error: %s", err)
343+
}
344+
345+
opts := DeactivateOpts{
346+
Ctx: ctx,
347+
Txn: txn,
348+
Store: store,
349+
BundleNames: map[string]struct{}{
350+
fmt.Sprintf("%s/%s", bundleName, modulePath): {},
351+
},
352+
}
353+
354+
if tc.customRegoVersion != ast.RegoUndefined {
355+
opts.ParserOptions.RegoVersion = tc.customRegoVersion
356+
}
357+
358+
err := Deactivate(&opts)
359+
360+
if len(tc.expErrs) > 0 {
361+
if err == nil {
362+
t.Fatalf("Expected error but got nil for test: %s", tc.note)
363+
}
364+
for _, expErr := range tc.expErrs {
365+
if err := err.Error(); !strings.Contains(err, expErr) {
366+
t.Fatalf("Expected error to contain:\n\n%s\n\nbut got:\n\n%s", expErr, err)
367+
}
368+
}
369+
} else if err != nil {
370+
t.Fatalf("Unexpected error: %v", err)
371+
}
372+
})
373+
}
374+
}

0 commit comments

Comments
 (0)