Fix for the issue when OPA throws misleading error (storage_not_found…

…_error) message while loading the delta bundle when persist property in config is true.

The fix is to prevent the loading of delta bundle when persist is true and give a more clear error message.

Fixes open-policy-agent#5959

bundle/bundle.go

@@ -399,6 +399,7 @@ type Reader struct {
 	etag                  string
 	lazyLoadingMode       bool
 	name                  string
+	persist               bool
 }
 
 // NewReader is deprecated. Use NewCustomReader instead.
@@ -496,6 +497,12 @@ func (r *Reader) WithLazyLoadingMode(yes bool) *Reader {
 	return r
 }
 
+// WithBundlePersistence specifies if the downloaded bundle will eventually be persisted to disk.
+func (r *Reader) WithBundlePersistence(persist bool) *Reader {
+	r.persist = persist
+	return r
+}
+
 func (r *Reader) ParserOptions() ast.ParserOptions {
 	return ast.ParserOptions{
 		ProcessAnnotation: r.processAnnotations,
@@ -659,6 +666,10 @@ func (r *Reader) Read() (Bundle, error) {
 		if len(bundle.WasmModules) != 0 {
 			return bundle, fmt.Errorf("delta bundle expected to contain only patch file but wasm files found")
 		}
+
+		if r.persist {
+			return bundle, fmt.Errorf("'persist' property is true in config. persisting delta bundle to disk is not supported")
+		}
 	}
 
 	// check if the bundle signatures specify any files that weren't found in the bundle

bundle/bundle_test.go

@@ -578,6 +578,51 @@ func TestReadWithPatchExtraFiles(t *testing.T) {
 
 }
 
+func TestReadWithPatchPersistProperty(t *testing.T) {
+	cases := []struct {
+		note    string
+		files   [][2]string
+		persist bool
+		err     string
+	}{
+		{
+			note: "persist true property",
+			files: [][2]string{
+				{"/patch.json", `{"data": [{"op": "add", "path": "/a/b/d", "value": "foo"}, {"op": "remove", "path": "a/b/c"}]}`},
+			},
+			persist: true,
+			err:     "'persist' property is true in config. persisting delta bundle to disk is not supported",
+		},
+		{
+			note: "persist false property",
+			files: [][2]string{
+				{"/patch.json", `{"data": [{"op": "add", "path": "/a/b/d", "value": "foo"}, {"op": "remove", "path": "a/b/c"}]}`},
+			},
+			persist: false,
+			err:     "",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.note, func(t *testing.T) {
+			buf := archive.MustWriteTarGz(tc.files)
+			loader := NewTarballLoaderWithBaseURL(buf, "/foo/bar")
+			reader := NewCustomReader(loader).
+				WithBundlePersistence(tc.persist).WithBaseDir("/foo/bar")
+			_, err := reader.Read()
+			if tc.err == "" && err != nil {
+				t.Fatal("Unexpected error occurred:", err)
+			} else if tc.err != "" && err == nil {
+				t.Fatal("Expected error but got success")
+			} else if tc.err != "" && err != nil {
+				if tc.err != err.Error() {
+					t.Fatalf("Expected error to contain %q but got: %v", tc.err, err)
+				}
+			}
+		})
+	}
+}
+
 func TestReadWithSignaturesExtraFiles(t *testing.T) {
 	signedTokenHS256 := `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImZvbyJ9.eyJmaWxlcyI6W3sibmFtZSI6Ii5tYW5pZmVzdCIsImhhc2giOiI1MDdhMmMzOGExNDQxZGI1OGQyY2I4Nzk4MmM0MmFhOTFhNDM0MmVmNDIyYTZiNTQyZWRkZWJlZWY2ZjA0MTJmIiwiYWxnb3JpdGhtIjoiU0hBLTI1NiJ9LHsibmFtZSI6ImEvYi9jL2RhdGEuanNvbiIsImhhc2giOiI0MmNmZTY3NjhiNTdiYjVmNzUwM2MxNjVjMjhkZDA3YWM1YjgxMzU1NGViYzg1MGYyY2MzNTg0M2U3MTM3YjFkIiwiYWxnb3JpdGhtIjoiU0hBLTI1NiJ9LHsibmFtZSI6Imh0dHAvcG9saWN5L3BvbGljeS5yZWdvIiwiaGFzaCI6ImE2MTVlZWFlZTIxZGU1MTc5ZGUwODBkZThjMzA1MmM4ZGE5MDExMzg0MDZiYTcxYzM4YzAzMjg0NWY3ZDU0ZjQiLCJhbGdvcml0aG0iOiJTSEEtMjU2In1dLCJpYXQiOjE1OTIyNDgwMjcsImlzcyI6IkpXVFNlcnZpY2UiLCJzY29wZSI6IndyaXRlIn0.Vmm9UDiInUnXXlk-OOjiCy3rR7EVvXS-OFst1rbh3Zo`
 

download/download.go

@@ -333,7 +333,8 @@ func (d *Downloader) download(ctx context.Context, m metrics.Metrics) (*download
 				WithBundleVerificationConfig(d.bvc).
 				WithBundleEtag(etag).
 				WithLazyLoadingMode(d.lazyLoadingMode).
-				WithBundleName(d.bundleName)
+				WithBundleName(d.bundleName).
+				WithBundlePersistence(d.persist)
 			if d.sizeLimitBytes != nil {
 				reader = reader.WithSizeLimitBytes(*d.sizeLimitBytes)
 			}