diff --git a/v1/bundle/bundle.go b/v1/bundle/bundle.go index db9bfc4ba4..7d7277de08 100644 --- a/v1/bundle/bundle.go +++ b/v1/bundle/bundle.go @@ -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() diff --git a/v1/bundle/store.go b/v1/bundle/store.go index c9caece8b5..363f7664d7 100644 --- a/v1/bundle/store.go +++ b/v1/bundle/store.go @@ -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) } diff --git a/v1/bundle/store_test.go b/v1/bundle/store_test.go index 582b0352a3..0ef48ef1be 100644 --- a/v1/bundle/store_test.go +++ b/v1/bundle/store_test.go @@ -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 - } } } }` diff --git a/v1/plugins/bundle/plugin_test.go b/v1/plugins/bundle/plugin_test.go index 919caba4d7..c5bfd2afd5 100644 --- a/v1/plugins/bundle/plugin_test.go +++ b/v1/plugins/bundle/plugin_test.go @@ -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 { diff --git a/v1/plugins/plugins.go b/v1/plugins/plugins.go index c9b99ab28b..be9630ee7b 100644 --- a/v1/plugins/plugins.go +++ b/v1/plugins/plugins.go @@ -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() }