plugins/discovery: Fix discovery erasing persistence_directory config

Before this change, if a discovery bundle didn't contain configuration
for `persistence_directory`, this would be deleted from the manager's
configuration. When enabling persistence of the discovery bundle this
doesn't make much sense, as the first discovery bundle would erase the
persistence settings.

This change ensures that discovery never erases `persistence_directory`.

Signed-off-by: Benjamin Nørgaard <[email protected]>

Author: blacksails

docs/content/management-discovery.md

@@ -300,5 +300,10 @@ the discovery bundle itself is persisted. The discovered configuration that is g
 policies contained in the discovery bundle will **NOT** be persisted.
 
 {{< info >}}
-By default, the discovery bundle is persisted under the current working directory of the OPA process (e.g., `./.opa/bundles/<discovery.name>/bundle.tar.gz`).
+The discovery bundle is persisted at
+`<persistence_directory>/bundles/<discovery.name>/bundle.tar.gz`. By default
+`persistence_directory` is `.opa` in the working directory of the OPA process.
+If `persistence_directory` is changed through discovery this will not affect
+where the discovery plugin will store the discovery bundles, the boot
+configuration will always be used.
 {{< /info >}}

plugins/discovery/discovery_test.go

@@ -1053,7 +1053,8 @@ func TestReconfigureWithUpdates(t *testing.T) {
 				"key": "some_private_key",
 				"scope": "write"
 			}
-		}
+		},
+		"persistence_directory": "test"
 	}`), "test-id", inmem.New())
 	if err != nil {
 		t.Fatal(err)
@@ -1063,6 +1064,7 @@ func TestReconfigureWithUpdates(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	originalConfig := disco.config
 
 	initialBundle := makeDataBundle(1, `
 		{
@@ -1079,7 +1081,6 @@ func TestReconfigureWithUpdates(t *testing.T) {
 		t.Fatalf("Unexpected error %v", err)
 	}
 
-	originalConfig := disco.config
 	// update the discovery configuration and check
 	// the boot configuration is not overwritten
 	updatedBundle := makeDataBundle(2, `
@@ -1346,6 +1347,40 @@ func TestReconfigureWithUpdates(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}
+
+	// check that not setting persistence_directory doesn't remove boot config
+	updatedBundle = makeDataBundle(13, `
+		{
+			"config": {}
+		}
+	`)
+
+	err = disco.reconfigure(ctx, download.Update{Bundle: updatedBundle})
+	if err != nil {
+		t.Fatalf("Unexpected error %v", err)
+	}
+
+	if manager.Config.PersistenceDirectory == nil {
+		t.Fatal("Erased persistence directory configuration")
+	}
+
+	// update persistence directory
+	updatedBundle = makeDataBundle(14, `
+		{
+			"config": {
+				"persistence_directory": "my_bundles"
+			}
+		}
+	`)
+
+	err = disco.reconfigure(ctx, download.Update{Bundle: updatedBundle})
+	if err != nil {
+		t.Fatalf("Unexpected error %v", err)
+	}
+
+	if manager.Config.PersistenceDirectory == nil || *manager.Config.PersistenceDirectory != "my_bundles" {
+		t.Fatal("Did not update persistence directory")
+	}
 }
 
 func TestProcessBundleWithSigning(t *testing.T) {

plugins/plugins.go

@@ -691,6 +691,11 @@ func (m *Manager) Reconfigure(config *config.Config) error {
 		}
 	}
 
+	// don't erase persistence directory
+	if config.PersistenceDirectory == nil {
+		config.PersistenceDirectory = m.Config.PersistenceDirectory
+	}
+
 	m.Config = config
 	m.interQueryBuiltinCacheConfig = interQueryBuiltinCacheConfig
 	for name, client := range services {