sdk: fix bug while activating the bundle plugin

plugins/bundle/plugin.go

@@ -597,13 +597,14 @@ func (p *Plugin) activate(ctx context.Context, name string, b *bundle.Bundle) er
 		var activateErr error
 
 		opts := &bundle.ActivateOpts{
-			Ctx:      ctx,
-			Store:    p.manager.Store,
-			Txn:      txn,
-			TxnCtx:   params.Context,
-			Compiler: compiler,
-			Metrics:  p.status[name].Metrics,
-			Bundles:  map[string]*bundle.Bundle{name: b},
+			Ctx:           ctx,
+			Store:         p.manager.Store,
+			Txn:           txn,
+			TxnCtx:        params.Context,
+			Compiler:      compiler,
+			Metrics:       p.status[name].Metrics,
+			Bundles:       map[string]*bundle.Bundle{name: b},
+			ParserOptions: p.manager.ParserOptions(),
 		}
 
 		if p.manager.Info != nil {

plugins/bundle/plugin_test.go

@@ -1912,51 +1912,126 @@ func TestLoadAndActivateBundlesFromDisk(t *testing.T) {
 }
 
 func TestLoadAndActivateBundlesFromDiskV1Compatible(t *testing.T) {
+	type update struct {
+		modules map[string]string
+		expErrs []string
+	}
 	// Note: modules are parsed before passed to plugin, so any expected errors must be triggered by the compiler stage.
 	tests := []struct {
-		note              string
-		v1Compatible      bool
-		bundleRegoVersion *ast.RegoVersion
-		module            string
-		expErrs           []string
+		note         string
+		v1Compatible bool
+		updates      []update
 	}{
 		{
 			note: "v0.x",
-			module: `package foo
+			updates: []update{
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
 import future.keywords
 corge contains 1 if {
 	input.x == 2
 }`,
+					},
+				},
+			},
 		},
 		{
 			note: "v0.x, shadowed import (no error)",
-			module: `package foo
+			updates: []update{
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
 import future.keywords
 import data.foo
 import data.bar as foo
 corge contains 1 if {
 	input.x == 2
 }`,
+					},
+				},
+			},
 		},
 		{
 			note:         "v1.0",
 			v1Compatible: true,
-			module: `package foo
+			updates: []update{
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
 corge contains 1 if {
 	input.x == 2
 }`,
+					},
+				},
+			},
 		},
 		{
 			note:         "v1.0, shadowed import",
 			v1Compatible: true,
-			module: `package foo
+			updates: []update{
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
 import data.foo
 import data.bar as foo
 corge contains 1 if {
 	input.x == 2
 }`,
-			expErrs: []string{
-				"rego_compile_error: import must not shadow import data.foo",
+					},
+					expErrs: []string{
+						"rego_compile_error: import must not shadow import data.foo",
+					},
+				},
+			},
+		},
+		{
+			note:         "v1.0, module updated",
+			v1Compatible: true,
+			updates: []update{
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
+corge contains 1 if {
+	input.x == 2
+}`,
+					},
+				},
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
+corge contains 2 if {
+	input.x == 3
+}`,
+					},
+				},
+			},
+		},
+		{
+			note:         "v1.0, module updated, shadowed import",
+			v1Compatible: true,
+			updates: []update{
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
+corge contains 1 if {
+	input.x == 2
+}`,
+					},
+				},
+				{
+					modules: map[string]string{
+						"/foo/bar.rego": `package foo
+import data.foo
+import data.bar as foo
+corge contains 2 if {
+	input.x == 3
+}`,
+					},
+					expErrs: []string{
+						"rego_compile_error: import must not shadow import data.foo",
+					},
+				},
 			},
 		},
 	}
@@ -1995,80 +2070,88 @@ corge contains 1 if {
 
 			plugin.loadAndActivateBundlesFromDisk(ctx)
 
-			// persist a bundle to disk and then load it
-			b := bundle.Bundle{
-				Manifest: bundle.Manifest{Revision: "quickbrownfaux"},
-				Data:     util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}}`)).(map[string]interface{}),
-				Modules: []bundle.ModuleFile{
-					{
-						URL:    "/foo/bar.rego",
-						Path:   "/foo/bar.rego",
-						Parsed: ast.MustParseModuleWithOpts(tc.module, popts),
-						Raw:    []byte(tc.module),
-					},
-				},
-			}
+			for _, update := range tc.updates {
+				// persist a bundle to disk and then load it
+				b := bundle.Bundle{
+					Manifest: bundle.Manifest{Revision: "quickbrownfaux"},
+					Data:     util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}}`)).(map[string]interface{}),
+				}
+				for url, module := range update.modules {
+					b.Modules = append(b.Modules, bundle.ModuleFile{
+						URL:    url,
+						Path:   url,
+						Parsed: ast.MustParseModuleWithOpts(module, popts),
+						Raw:    []byte(module),
+					})
+				}
 
-			b.Manifest.Init()
+				b.Manifest.Init()
 
-			var buf bytes.Buffer
-			if err := bundle.NewWriter(&buf).UseModulePath(true).Write(b); err != nil {
-				t.Fatal("unexpected error:", err)
-			}
-
-			err = plugin.saveBundleToDisk(bundleName, &buf)
-			if err != nil {
-				t.Fatalf("unexpected error %v", err)
-			}
+				var buf bytes.Buffer
+				if err := bundle.NewWriter(&buf).UseModulePath(true).Write(b); err != nil {
+					t.Fatal("unexpected error:", err)
+				}
 
-			plugin.loadAndActivateBundlesFromDisk(ctx)
+				err = plugin.saveBundleToDisk(bundleName, &buf)
+				if err != nil {
+					t.Fatalf("unexpected error %v", err)
+				}
 
-			if tc.expErrs != nil {
-				if status, ok := plugin.status[bundleName]; !ok {
-					t.Fatalf("Expected to find status for %s, found nil", bundleName)
-				} else if status.Type != bundle.SnapshotBundleType {
-					t.Fatalf("expected snapshot bundle but got %v", status.Type)
-				} else if errs := status.Errors; len(errs) != len(tc.expErrs) {
-					t.Fatalf("expected errors:\n\n%v\n\nbut got:\n\n%v", tc.expErrs, errs)
-				} else {
-					for _, expErr := range tc.expErrs {
-						found := false
-						for _, err := range errs {
-							if strings.Contains(err.Error(), expErr) {
-								found = true
-								break
+				plugin.loadAndActivateBundlesFromDisk(ctx)
+
+				if update.expErrs != nil {
+					if status, ok := plugin.status[bundleName]; !ok {
+						t.Fatalf("Expected to find status for %s, found nil", bundleName)
+					} else if status.Type != bundle.SnapshotBundleType {
+						t.Fatalf("expected snapshot bundle but got %v", status.Type)
+					} else if errs := status.Errors; len(errs) != len(update.expErrs) {
+						t.Fatalf("expected errors:\n\n%v\n\nbut got:\n\n%v", update.expErrs, errs)
+					} else {
+						for _, expErr := range update.expErrs {
+							found := false
+							for _, err := range errs {
+								if strings.Contains(err.Error(), expErr) {
+									found = true
+									break
+								}
+							}
+							if !found {
+								t.Fatalf("expected error:\n\n%v\n\nbut got:\n\n%v", expErr, errs)
 							}
-						}
-						if !found {
-							t.Fatalf("expected error:\n\n%v\n\nbut got:\n\n%v", expErr, errs)
 						}
 					}
-				}
-			} else {
-				txn := storage.NewTransactionOrDie(ctx, manager.Store)
-				defer manager.Store.Abort(ctx, txn)
+				} else {
+					txn := storage.NewTransactionOrDie(ctx, manager.Store)
+					fatal := func(args ...any) {
+						manager.Store.Abort(ctx, txn)
+						t.Fatal(args...)
+					}
 
-				ids, err := manager.Store.ListPolicies(ctx, txn)
-				if err != nil {
-					t.Fatal(err)
-				} else if len(ids) != 1 {
-					t.Fatal("Expected 1 policy")
-				}
+					ids, err := manager.Store.ListPolicies(ctx, txn)
+					if err != nil {
+						fatal(err)
+					}
+					for _, id := range ids {
+						bs, err := manager.Store.GetPolicy(ctx, txn, id)
+						p, _ := strings.CutPrefix(id, bundleName)
+						module := update.modules[p]
+						exp := []byte(module)
+						if err != nil {
+							fatal(err)
+						} else if !bytes.Equal(bs, exp) {
+							fatal("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
+						}
+					}
 
-				bs, err := manager.Store.GetPolicy(ctx, txn, ids[0])
-				exp := []byte(tc.module)
-				if err != nil {
-					t.Fatal(err)
-				} else if !bytes.Equal(bs, exp) {
-					t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
-				}
+					data, err := manager.Store.Read(ctx, txn, storage.Path{})
+					expData := util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}}}`))
+					if err != nil {
+						fatal(err)
+					} else if !reflect.DeepEqual(data, expData) {
+						fatal("Bad data content. Exp:\n%v\n\nGot:\n\n%v", expData, data)
+					}
 
-				data, err := manager.Store.Read(ctx, txn, storage.Path{})
-				expData := util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}}}`))
-				if err != nil {
-					t.Fatal(err)
-				} else if !reflect.DeepEqual(data, expData) {
-					t.Fatalf("Bad data content. Exp:\n%v\n\nGot:\n\n%v", expData, data)
+					manager.Store.Abort(ctx, txn)
 				}
 			}
 		})

sdk/opa_test.go

@@ -2844,3 +2844,52 @@ func toMetricMap(metrics []*promdto.MetricFamily) map[string]bool {
 	}
 	return metricMap
 }
+
+func TestActivateV1Bundles(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100)
+	defer cancel()
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.FileServer(http.Dir("testdata")).ServeHTTP(w, r)
+	}))
+
+	config := fmt.Sprintf(`{
+		"services": {
+			"test": {
+				"url": %q
+			}
+		},
+		"bundles": {
+			"v1bundle": {
+				"resource": "/v1bundle.tar.gz"
+			}
+		}
+	}`, server.URL)
+
+	opa, err := sdk.New(ctx, sdk.Options{
+		ID:           "sdk-id-0",
+		Config:       strings.NewReader(config),
+		Logger:       logging.New(),
+		V1Compatible: true,
+	})
+
+	defer opa.Stop(ctx)
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	d, err := opa.Decision(context.Background(), sdk.DecisionOptions{
+		Path: "v1bundle/authz",
+		Input: map[string]interface{}{
+			"role": "admin",
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if d.Result != true {
+		t.Errorf("expected result to be true, got %v", d.Result)
+	}
+}

sdk/testdata/Makefile

@@ -1,3 +1,7 @@
 disco.tar.gz: bundle/data.json
 	opa build bundle
 	mv bundle.tar.gz disco.tar.gz
+
+v1bundle.tar.gz: v1bundle/.manifest v1bundle/policy.rego
+	opa build --v1-compatible -o v1bundle.tar.gz -b v1bundle/
+

sdk/testdata/v1bundle/.manifest

@@ -0,0 +1,3 @@
+{
+  "roots": ["v1bundle"]
+}

sdk/testdata/v1bundle/policy.rego

@@ -0,0 +1,9 @@
+package v1bundle
+
+default authz := false
+
+# METADATA
+# entrypoint: true
+authz if {
+    input.role == "admin"
+}