ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

bundle: Fixing issue where --v0-compatible isn't respected for custom bundles (#7338)

Browse Source 
where the bundle has been manually constructed containing v0 Rego modules and
no `rego_version`/`file_rego_versions` fields are declared in the bundle manifest.

This affects bundle deactivation in the bundle store lifecycle used when for
`opa run` in server mode (`-s`).

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
This commit is contained in:
Johan Fylling
2025-02-05 12:49:59 +01:00
committed by GitHub
parent 08f98e9527
commit bb12354f58
5 changed files with 254 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
v1/bundle/bundle.go
+5 -6
View File
@@ -715,8 +715,11 @@ func (r *Reader) Read() (Bundle, error) {
popts.RegoVersion = bundle.RegoVersion(popts.EffectiveRegoVersion())
for _, mf := range modules {
modulePopts := popts
if modulePopts.RegoVersion, err = bundle.RegoVersionForFile(mf.RelativePath, popts.EffectiveRegoVersion()); err != nil {
if regoVersion, err := bundle.RegoVersionForFile(mf.RelativePath, popts.EffectiveRegoVersion()); err != nil {
return bundle, err
} else if regoVersion != ast.RegoUndefined {
// We don't expect ast.RegoUndefined here, but don't override configured rego-version if we do just to be extra protective
modulePopts.RegoVersion = regoVersion
}
r.metrics.Timer(metrics.RegoModuleParse).Start()
mf.Parsed, err = ast.ParseModuleWithOpts(mf.Path, string(mf.Raw), modulePopts)
@@ -1204,10 +1207,6 @@ func (b *Bundle) SetRegoVersion(v ast.RegoVersion) {
// If there is no defined version for the given path, the default version def is returned.
// If the version does not correspond to ast.RegoV0 or ast.RegoV1, an error is returned.
func (b *Bundle) RegoVersionForFile(path string, def ast.RegoVersion) (ast.RegoVersion, error) {
if def == ast.RegoUndefined {
def = ast.DefaultRegoVersion
}
version, err := b.Manifest.numericRegoVersionForFile(path)
if err != nil {
return def, err
@@ -1513,7 +1512,7 @@ func bundleRegoVersions(bundle *Bundle, regoVersion ast.RegoVersion, usePath boo
return nil, err
}
// only record the rego version if it's different from one applied globally to the result bundle
if v != regoVersion {
if regoVersion != ast.RegoUndefined && v != regoVersion {
// We store the rego version by the absolute path to the bundle root, as this will be the - possibly new - path
// to the module inside the merged bundle.
fileRegoVersions[bundleAbsolutePath(m, usePath)] = v.Int()
v1/bundle/store.go
+1 -1
View File
@@ -827,7 +827,7 @@ func writeModuleRegoVersionToStore(ctx context.Context, store storage.Store, txn
if regoVersion == ast.RegoUndefined {
var err error
regoVersion, err = b.RegoVersionForFile(mf.Path, ast.RegoUndefined)
regoVersion, err = b.RegoVersionForFile(mf.Path, runtimeRegoVersion)
if err != nil {
return fmt.Errorf("failed to get rego version for module '%s' in bundle: %w", mf.Path, err)
}
v1/bundle/store_test.go
+232 -70
View File
@@ -722,6 +722,232 @@ func TestBundleLifecycle_ModuleRegoVersions(t *testing.T) {
}
}`,
},
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 1337 if { true }`},
},
},
lazy: true,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
deactivation{
bundles: map[string]struct{}{"bundle1": {}},
expData: `{"system":{"bundles":{}}}`,
},
},
},
{
note: "custom bundle without rego-version, lazy, v1 runtime (explicit)",
runtimeRegoVersion: ast.RegoV1,
updates: []interface{}{
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 42 if { true }`},
},
},
lazy: true,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 1337 if { true }`},
},
},
lazy: true,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
deactivation{
bundles: map[string]struct{}{"bundle1": {}},
expData: `{"system":{"bundles":{}}}`,
},
},
},
{
note: "custom bundle without rego-version, lazy, --v0-compatible",
runtimeRegoVersion: ast.RegoV0,
updates: []interface{}{
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p[42] { true }`},
},
},
lazy: true,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p[1337] { true }`},
},
},
lazy: true,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
deactivation{
bundles: map[string]struct{}{"bundle1": {}},
expData: `{"system":{"bundles":{}}}`,
},
},
},
{
note: "custom bundle without rego-version, not lazy",
updates: []interface{}{
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 42 if { true }`},
},
},
lazy: false,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 1337 if { true }`},
},
},
lazy: false,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
deactivation{
bundles: map[string]struct{}{"bundle1": {}},
expData: `{"system":{"bundles":{}}}`,
},
},
},
{
note: "custom bundle without rego-version, not lazy, v1 runtime (explicit)",
runtimeRegoVersion: ast.RegoV1,
updates: []interface{}{
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 42 if { true }`},
},
},
lazy: false,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p contains 1337 if { true }`},
},
},
lazy: false,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
deactivation{
bundles: map[string]struct{}{"bundle1": {}},
expData: `{"system":{"bundles":{}}}`,
},
},
},
{
note: "custom bundle without rego-version, not lazy, --v0-compatible",
runtimeRegoVersion: ast.RegoV0,
updates: []interface{}{
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p[42] { true }`},
},
},
lazy: false,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
activation{
bundles: bundles{
"bundle1": {
{"/.manifest", `{"roots": ["a"]}`},
{"a/policy.rego", `package a
p[1337] { true }`},
},
},
lazy: false,
readWithBundleName: true,
expData: `{
"system":{
"bundles":{"bundle1":{"etag":"bar","manifest":{"revision":"","roots":["a"]}}}
}
}`,
},
deactivation{
bundles: map[string]struct{}{"bundle1": {}},
expData: `{"system":{"bundles":{}}}`,
@@ -2258,11 +2484,6 @@ func TestBundleLazyModeLifecycleRawNoBundleRoots(t *testing.T) {
},
"etag": "foo"
}
},
"modules":{
"example/example.rego":{
"rego_version":1
}
}
}
}
@@ -2455,11 +2676,6 @@ func TestBundleLazyModeLifecycleRawNoBundleRootsDiskStorage(t *testing.T) {
},
"etag": "foo"
}
},
"modules":{
"example/example.rego":{
"rego_version":1
}
}
}
}
@@ -2632,12 +2848,7 @@ func TestBundleLazyModeLifecycleNoBundleRoots(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
}
}
}
}
}`
@@ -2847,12 +3058,7 @@ func TestBundleLazyModeLifecycleNoBundleRootsDiskStorage(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
}
}
}
}
}`
@@ -3085,12 +3291,7 @@ func TestBundleLazyModeLifecycleMixBundleTypeActivationDiskStorage(t *testing.T)
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
}
}
}
}
}`
@@ -3222,12 +3423,7 @@ func TestBundleLazyModeLifecycleOldBundleEraseDiskStorage(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
}
}
}
}
}`
@@ -3439,12 +3635,7 @@ func TestBundleLazyModeLifecycleRestoreBackupDB(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
}
}
}
}
}`
@@ -3523,12 +3714,7 @@ func TestBundleLazyModeLifecycleRestoreBackupDB(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
}
}
}
}
}`
@@ -3814,14 +4000,6 @@ func TestDeltaBundleLazyModeLifecycleDiskStorage(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/a/policy.rego":{
"rego_version":1
},
"bundle2/b/policy.rego":{
"rego_version":1
}
}
}
}`
@@ -4683,14 +4861,6 @@ func TestDeltaBundleLazyModeLifecycle(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/policy.rego":{
"rego_version":1
},
"bundle2/policy.rego":{
"rego_version":1
}
}
}
}`
@@ -4981,14 +5151,6 @@ func TestDeltaBundleLazyModeWithDefaultRules(t *testing.T) {
},
"etag": ""
}
},
"modules":{
"bundle1/policy.rego":{
"rego_version":1
},
"bundle2/policy.rego":{
"rego_version":1
}
}
}
}`
v1/plugins/bundle/plugin_test.go
+11 -20
View File
@@ -109,8 +109,7 @@ func TestPluginOneShot(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}},
"modules": {"test-bundle/foo/bar": {"rego_version": 1}}
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}
}
}`))
if err != nil {
@@ -970,8 +969,7 @@ func TestPluginStartLazyLoadInMem(t *testing.T) {
expected := `{
"p": "x1", "q": "x2",
"system": {
"bundles": {"test-1": {"etag": "", "manifest": {"revision": "", "roots": ["p", "authz"]}}, "test-2": {"etag": "", "manifest": {"revision": "", "roots": ["q"]}}},
"modules": {"test-1/bar/policy.rego": {"rego_version": 1}}
"bundles": {"test-1": {"etag": "", "manifest": {"revision": "", "roots": ["p", "authz"]}}, "test-2": {"etag": "", "manifest": {"revision": "", "roots": ["q"]}}}
}
}`
if rm.readAst {
@@ -1040,11 +1038,11 @@ func TestPluginOneShotDiskStorageMetrics(t *testing.T) {
t.Errorf("%s: expected %v, got %v", name, exp, act)
}
name = "disk_written_keys"
if exp, act := 7, met.Counter(name).Value(); act.(uint64) != uint64(exp) {
if exp, act := 6, met.Counter(name).Value(); act.(uint64) != uint64(exp) {
t.Errorf("%s: expected %v, got %v", name, exp, act)
}
name = "disk_read_keys"
if exp, act := 14, met.Counter(name).Value(); act.(uint64) != uint64(exp) {
if exp, act := 13, met.Counter(name).Value(); act.(uint64) != uint64(exp) {
t.Errorf("%s: expected %v, got %v", name, exp, act)
}
name = "disk_read_bytes"
@@ -1089,8 +1087,7 @@ func TestPluginOneShotDiskStorageMetrics(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}},
"modules": {"test-bundle/foo/bar": {"rego_version": 1}}
"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}
}
}`))
if err != nil {
@@ -1196,8 +1193,7 @@ func TestPluginOneShotDeltaBundle(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"a": {"baz": "bux", "foo": ["hello", "world"]},
"system": {
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "delta", "roots": ["a"]}}},
"modules": {"test-bundle/a/policy.rego": {"rego_version": 1}}
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "delta", "roots": ["a"]}}}
}
}`))
if !reflect.DeepEqual(data, expData) {
@@ -1301,8 +1297,7 @@ func TestPluginOneShotDeltaBundleWithAstStore(t *testing.T) {
expData := ast.MustParseTerm(`{
"a": {"baz": "bux", "foo": ["hello", "world"]},
"system": {
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "delta", "roots": ["a"]}}},
"modules": {"test-bundle/a/policy.rego": {"rego_version": 1}}
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "delta", "roots": ["a"]}}}
}
}`)
if ast.Compare(data, expData) != 0 {
@@ -1488,8 +1483,7 @@ func TestPluginOneShotBundlePersistence(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}},
"modules": {"test-bundle/foo/bar.rego": {"rego_version": 1}}
"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}
}
}`))
if err != nil {
@@ -2171,8 +2165,7 @@ func TestLoadAndActivateBundlesFromDisk(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}},
"modules": {"test-bundle/foo/bar.rego": {"rego_version": 1}}
"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}
}
}`))
if err != nil {
@@ -2257,8 +2250,7 @@ func TestLoadAndActivateBundlesFromDiskReservedChars(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
"bundles": {"test?bundle=opa": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}},
"modules": {"test/foo/bar.rego": {"rego_version": 1}}
"bundles": {"test?bundle=opa": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}
}
}`))
if err != nil {
@@ -6860,8 +6852,7 @@ func TestPluginManualTriggerMultipleDiskStorage(t *testing.T) {
expData := util.MustUnmarshalJSON([]byte(`{
"p": "x1", "q": "x2",
"system": {
"bundles": {"test-1": {"etag": "", "manifest": {"revision": "", "roots": ["p", "authz"]}}, "test-2": {"etag": "", "manifest": {"revision": "", "roots": ["q"]}}},
"modules": {"test-1/bar/policy.rego": {"rego_version": 1}}
"bundles": {"test-1": {"etag": "", "manifest": {"revision": "", "roots": ["p", "authz"]}}, "test-2": {"etag": "", "manifest": {"revision": "", "roots": ["q"]}}}
}
}`))
if err != nil {
v1/plugins/plugins.go
+5
View File
@@ -448,6 +448,11 @@ func New(raw []byte, id string, store storage.Store, opts ...func(*Manager)) (*M
f(m)
}
if m.parserOptions.RegoVersion == ast.RegoUndefined {
// Default to v1 if rego-version is not set through options
m.parserOptions.RegoVersion = ast.DefaultRegoVersion
}
if m.logger == nil {
m.logger = logging.Get()
}