bundle: Fix for overwriting data file keys

This corrects an issue where data.json (or yaml) files in a bundle
would overwrite data if the file was at the root of the bundle
directory and was loaded after other data files.

This also helps avoid any issues with merging data files that
could have overlapped without any problems.

Fixes: #1763
Signed-off-by: Patrick East <east.patrick@gmail.com>

bundle/bundle.go

@@ -18,11 +18,13 @@ import (
 	"strings"
 
 	"github.com/open-policy-agent/opa/internal/file/archive"
+	"github.com/open-policy-agent/opa/internal/merge"
+
+	"github.com/pkg/errors"
 
 	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/internal/file"
 	"github.com/open-policy-agent/opa/util"
-	"github.com/pkg/errors"
 )
 
 // Common file extensions and file names.
@@ -327,38 +329,42 @@ func (b Bundle) Equal(other Bundle) bool {
 }
 
 func (b *Bundle) insert(key []string, value interface{}) error {
-	if len(key) == 0 {
-		obj, ok := value.(map[string]interface{})
-		if !ok {
-			return fmt.Errorf("root value must be object")
-		}
-		b.Data = obj
-		return nil
-	}
-
-	obj, err := b.mkdir(key[:len(key)-1])
+	// Build an object with the full structure for the value
+	obj, err := mktree(key, value)
 	if err != nil {
 		return err
 	}
 
-	obj[key[len(key)-1]] = value
+	// Merge the new data in with the current bundle data object
+	merged, ok := merge.InterfaceMaps(b.Data, obj)
+	if !ok {
+		return fmt.Errorf("failed to insert data file from path %s", filepath.Join(key...))
+	}
+
+	b.Data = merged
+
 	return nil
 }
 
-func (b *Bundle) mkdir(key []string) (map[string]interface{}, error) {
-	obj := b.Data
-	for i := 0; i < len(key); i++ {
-		node, ok := obj[key[i]]
+func mktree(path []string, value interface{}) (map[string]interface{}, error) {
+	if len(path) == 0 {
+		// For 0 length path the value is the full tree.
+		obj, ok := value.(map[string]interface{})
 		if !ok {
-			node = map[string]interface{}{}
-			obj[key[i]] = node
-		}
-		obj, ok = node.(map[string]interface{})
-		if !ok {
-			return nil, fmt.Errorf("non-leaf value must be object")
+			return nil, fmt.Errorf("root value must be object")
 		}
+		return obj, nil
 	}
-	return obj, nil
+
+	dir := map[string]interface{}{}
+	for i := len(path) - 1; i > 0; i-- {
+		dir[path[i]] = value
+		value = dir
+		dir = map[string]interface{}{}
+	}
+	dir[path[0]] = value
+
+	return dir, nil
 }
 
 // RootPathsOverlap takes in two bundle root paths and returns

bundle/bundle_test.go

@@ -11,9 +11,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/open-policy-agent/opa/internal/file/archive"
-
 	"github.com/open-policy-agent/opa/ast"
+	"github.com/open-policy-agent/opa/internal/file/archive"
 )
 
 func TestRead(t *testing.T) {
@@ -23,6 +22,7 @@ func TestRead(t *testing.T) {
 		{"/a/b/d/data.json", "true"},
 		{"/a/b/y/data.yaml", `foo: 1`},
 		{"/example/example.rego", `package example`},
+		{"/data.json", `{"x": {"y": true}, "a": {"b": {"z": true}}}}`},
 	}
 
 	buf := archive.MustWriteTarGz(files)
@@ -42,8 +42,12 @@ func TestRead(t *testing.T) {
 					"y": map[string]interface{}{
 						"foo": json.Number("1"),
 					},
+					"z": true,
 				},
 			},
+			"x": map[string]interface{}{
+				"y": true,
+			},
 		},
 		Modules: []ModuleFile{
 			{
@@ -55,7 +59,7 @@ func TestRead(t *testing.T) {
 	}
 
 	if !exp.Equal(bundle) {
-		t.Fatal("Exp:", exp, "\n\nGot:", bundle)
+		t.Fatal("\nExp:", exp, "\n\nGot:", bundle)
 	}
 }
 
@@ -216,6 +220,10 @@ func TestReadErrorBadContents(t *testing.T) {
 			{"a/b/c/data.json", "true"},
 		}},
 		{[][2]string{{"/test.rego", ""}}},
+		{[][2]string{
+			{"/a/b/data.json", `{"c": "foo"}`},
+			{"/data.json", `{"a": {"b": {"c": [123]}}}`},
+		}},
 	}
 	for _, test := range tests {
 		buf := archive.MustWriteTarGz(test.files)

internal/merge/merge.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by an Apache2
 // license that can be found in the LICENSE file.
 
+// Package merge contains helpers to merge data structures
+// frequently encountered in OPA.
 package merge
 
 // InterfaceMaps returns the result of merging a and b. If a and b cannot be