Skip to content

Commit

Permalink
Fixing issue where bundle plugin could panic on reconfiguration (SDK …
Browse files Browse the repository at this point in the history
…use) (#7300)

Fixing issue where bundle plugin would panic on reconfiguration if module rego-version is missing in bundle manifest.

* Passing runtime rego-version to deactivation options
* Preferring to pull rego-version from parsed modules if present

This solves an edge case when using the OPA SDK, and should not affect standalone OPA.

Fixes: #7297
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
  • Loading branch information
johanfylling authored Jan 24, 2025
1 parent e47bd4f commit b032e3b
Show file tree
Hide file tree
Showing 5 changed files with 367 additions and 57 deletions.
24 changes: 19 additions & 5 deletions v1/bundle/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,9 +817,23 @@ func writeEtagToStore(opts *ActivateOpts, name, etag string) error {
}

func writeModuleRegoVersionToStore(ctx context.Context, store storage.Store, txn storage.Transaction, b *Bundle,
bundlePath string, storagePath string, runtimeRegoVersion ast.RegoVersion) error {
if v, err := b.RegoVersionForFile(bundlePath, ast.RegoUndefined); err == nil && v != ast.RegoUndefined && v != runtimeRegoVersion {
if err := write(ctx, store, txn, moduleRegoVersionPath(storagePath), v.Int()); err != nil {
mf ModuleFile, storagePath string, runtimeRegoVersion ast.RegoVersion) error {

var regoVersion ast.RegoVersion
if mf.Parsed != nil {
regoVersion = mf.Parsed.RegoVersion()
}

if regoVersion == ast.RegoUndefined {
var err error
regoVersion, err = b.RegoVersionForFile(mf.Path, ast.RegoUndefined)
if err != nil {
return fmt.Errorf("failed to get rego version for module '%s' in bundle: %w", mf.Path, err)
}
}

if regoVersion != ast.RegoUndefined && regoVersion != runtimeRegoVersion {
if err := write(ctx, store, txn, moduleRegoVersionPath(storagePath), regoVersion.Int()); err != nil {
return fmt.Errorf("failed to write rego version for module '%s': %w", storagePath, err)
}
}
Expand Down Expand Up @@ -853,7 +867,7 @@ func writeDataAndModules(ctx context.Context, store storage.Store, txn storage.T
return err
}

if err := writeModuleRegoVersionToStore(ctx, store, txn, b, mf.Path, path, runtimeRegoVersion); err != nil {
if err := writeModuleRegoVersionToStore(ctx, store, txn, b, mf, path, runtimeRegoVersion); err != nil {
return err
}
}
Expand All @@ -875,7 +889,7 @@ func writeDataAndModules(ctx context.Context, store storage.Store, txn storage.T
if m := f.module; m != nil {
// 'f.module.Path' contains the module's path as it relates to the bundle root, and can be used for looking up the rego-version.
// 'f.Path' can differ, based on how the bundle reader was initialized.
if err := writeModuleRegoVersionToStore(ctx, store, txn, b, f.module.Path, p.String(), runtimeRegoVersion); err != nil {
if err := writeModuleRegoVersionToStore(ctx, store, txn, b, *m, p.String(), runtimeRegoVersion); err != nil {
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion v1/bundle/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1702,7 +1702,7 @@ func TestBundleLifecycle_ModuleRegoVersions(t *testing.T) {
t.Fatalf("unexpected error: %s", err)
}

expectedRaw := deact.expData // `{"system": {"bundles": {}, "modules": {}}}`
expectedRaw := deact.expData
expected := loadExpectedSortedResult(expectedRaw)
if !reflect.DeepEqual(expected, actual) {
t.Errorf("expected:\n\n%s\n\ngot:\n\n%s", expectedRaw, string(util.MustMarshalJSON(actual)))
Expand Down
9 changes: 5 additions & 4 deletions v1/plugins/bundle/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,11 @@ func (p *Plugin) Reconfigure(ctx context.Context, config interface{}) {
params.Context = storage.NewContext() // TODO(sr): metrics?
err := storage.Txn(ctx, p.manager.Store, params, func(txn storage.Transaction) error {
opts := &bundle.DeactivateOpts{
Ctx: ctx,
Store: p.manager.Store,
Txn: txn,
BundleNames: deletedBundles,
Ctx: ctx,
Store: p.manager.Store,
Txn: txn,
BundleNames: deletedBundles,
ParserOptions: p.manager.ParserOptions(),
}
err := bundle.Deactivate(opts)
if err != nil {
Expand Down
Loading

0 comments on commit b032e3b

Please sign in to comment.