plugins/bundle: raise error on root conflict

This changes to detect bundle root conflicts when "activating" a
bundle. The bundle in question will go into an error state and be
prevented from loading its data or policies.

If multiple bundles are being used, and one didn't define roots or
have a manifest (ie they claim all roots), it will conflict with all
other bundles and raise errors.

**Note: This does NOT affect bundles loaded from CLI with --data**

Fixes: #1635
Signed-off-by: Patrick East <[email protected]>

plugins/bundle/plugin.go

@@ -303,6 +303,13 @@ func (p *Plugin) activate(ctx context.Context, name string, b *bundle.Bundle) er
 			}
 		}
 
+		// Before changing anything make sure the roots don't collide with any
+		// other bundles that already are activated.
+		err := p.hasRootsOverlap(ctx, txn, name, newRoots)
+		if err != nil {
+			return err
+		}
+
 		// Erase data and policies at new + old roots, and remove the old
 		// manifest before activating a new bundle.
 		remaining, err := p.deactivate(ctx, txn, name, newRoots)
@@ -520,6 +527,41 @@ func (p *Plugin) configDelta(newConfig *Config) (map[string]*Source, map[string]
 	return newBundles, updatedBundles, deletedBundles
 }
 
+func (p *Plugin) hasRootsOverlap(ctx context.Context, txn storage.Transaction, bundleName string, bundleRoots map[string]struct{}) error {
+	collisions := map[string][]string{}
+	allBundles, err := bundle.ReadBundleNamesFromStore(ctx, p.manager.Store, txn)
+	if err != nil && !storage.IsNotFound(err) {
+		return err
+	}
+	for _, otherBundle := range allBundles {
+		if otherBundle == bundleName {
+			// ignore the bundle we are in the process of activating
+			continue
+		}
+		otherRoots, err := bundle.ReadBundleRootsFromStore(ctx, p.manager.Store, txn, otherBundle)
+		if err != nil && !storage.IsNotFound(err) {
+			return err
+		}
+		for _, existingRoot := range otherRoots {
+			for newRoot := range bundleRoots {
+				if strings.HasPrefix(newRoot, existingRoot) || strings.HasPrefix(existingRoot, newRoot) {
+					collisions[otherBundle] = append(collisions[otherBundle], newRoot)
+				}
+			}
+		}
+	}
+
+	if len(collisions) > 0 {
+		var bundleNames []string
+		for name := range collisions {
+			bundleNames = append(bundleNames, name)
+		}
+		p.logDebug(bundleName, fmt.Sprintf("bundle root collisions: %+v", collisions))
+		return fmt.Errorf("detected overlapping roots in bundle manifest with: %s", bundleNames)
+	}
+	return nil
+}
+
 func lookup(path storage.Path, data map[string]interface{}) (interface{}, bool) {
 	if len(path) == 0 {
 		return data, true

plugins/bundle/plugin_test.go

@@ -226,6 +226,73 @@ func TestPluginOneShotActivationRemovesOld(t *testing.T) {
 	}
 }
 
+func TestPluginOneShotActivationConflictingRoots(t *testing.T) {
+	ctx := context.Background()
+	manager := getTestManager()
+	plugin := Plugin{manager: manager, status: map[string]*Status{}, etags: map[string]string{}}
+	bundleNames := []string{"test-bundle1", "test-bundle2", "test-bundle3"}
+
+	for _, name := range bundleNames {
+		plugin.status[name] = &Status{Name: name}
+	}
+
+	// Start with non-conflicting updates
+	plugin.oneShot(ctx, bundleNames[0], download.Update{Bundle: &bundle.Bundle{
+		Manifest: bundle.Manifest{
+			Roots: &[]string{"a/b"},
+		},
+	}})
+
+	plugin.oneShot(ctx, bundleNames[1], download.Update{Bundle: &bundle.Bundle{
+		Manifest: bundle.Manifest{
+			Roots: &[]string{"a/c"},
+		},
+	}})
+
+	// ensure that both bundles are *not* in error status
+	ensureBundleStatus(t, &plugin, bundleNames, []bool{false, false, false})
+
+	// Add a third bundle that conflicts with one
+	plugin.oneShot(ctx, bundleNames[2], download.Update{Bundle: &bundle.Bundle{
+		Manifest: bundle.Manifest{
+			Roots: &[]string{"a/b/aa"},
+		},
+	}})
+
+	// ensure that both in the conflict go into error state
+	ensureBundleStatus(t, &plugin, bundleNames, []bool{false, false, true})
+
+	// Update to fix conflict
+	plugin.oneShot(ctx, bundleNames[2], download.Update{Bundle: &bundle.Bundle{
+		Manifest: bundle.Manifest{
+			Roots: &[]string{"b"},
+		},
+	}})
+
+	ensureBundleStatus(t, &plugin, bundleNames, []bool{false, false, false})
+
+	// Ensure empty roots conflict with all roots
+	plugin.oneShot(ctx, bundleNames[2], download.Update{Bundle: &bundle.Bundle{
+		Manifest: bundle.Manifest{
+			Roots: &[]string{""},
+		},
+	}})
+
+	ensureBundleStatus(t, &plugin, bundleNames, []bool{false, false, true})
+}
+
+func ensureBundleStatus(t *testing.T, p *Plugin, bundleNames []string, expectedErrs []bool) {
+	t.Helper()
+	for i, name := range bundleNames {
+		hasErr := p.status[name].Message != ""
+		if expectedErrs[i] && !hasErr {
+			t.Fatalf("expected bundle %s to be in an error state", name)
+		} else if !expectedErrs[i] && hasErr {
+			t.Fatalf("unexpected error state for bundle %s", name)
+		}
+	}
+}
+
 func TestPluginListener(t *testing.T) {
 
 	ctx := context.Background()
@@ -378,6 +445,7 @@ func TestPluginBulkListener(t *testing.T) {
 	b := bundle.Bundle{
 		Manifest: bundle.Manifest{
 			Revision: "quickbrownfaux",
+			Roots:    &[]string{"gork"},
 		},
 		Data: map[string]interface{}{},
 		Modules: []bundle.ModuleFile{
@@ -486,6 +554,7 @@ func TestPluginBulkListener(t *testing.T) {
 	b1 := bundle.Bundle{
 		Manifest: bundle.Manifest{
 			Revision: "123",
+			Roots:    &[]string{"p1"},
 		},
 		Data: map[string]interface{}{},
 		Modules: []bundle.ModuleFile{