Plugins: Ensure plugin uninstall success for "pre-installed" community plugins (#55322)

* track pre-installed external plugins in store

* fix tests

pkg/plugins/manager/fakes/fakes.go

@@ -207,14 +207,22 @@ func (r *FakePluginRepo) GetPluginDownloadOptions(ctx context.Context, pluginID,
 }
 
 type FakePluginStorage struct {
-	AddFunc    func(_ context.Context, pluginID string, z *zip.ReadCloser) (*storage.ExtractedPluginArchive, error)
+	AddFunc      func(_ context.Context, pluginID string, z *zip.ReadCloser) (*storage.ExtractedPluginArchive, error)
-	RemoveFunc func(_ context.Context, pluginID string) error
+	RegisterFunc func(_ context.Context, pluginID, pluginDir string) error
-	Added      map[string]string
+	RemoveFunc   func(_ context.Context, pluginID string) error
-	Removed    map[string]int
+	Added        map[string]string
+	Removed      map[string]int
+}
+
+func (s *FakePluginStorage) Register(ctx context.Context, pluginID, pluginDir string) error {
+	s.Added[pluginID] = pluginDir
+	if s.RegisterFunc != nil {
+		return s.RegisterFunc(ctx, pluginID, pluginDir)
+	}
+	return nil
 }
 
 func (s *FakePluginStorage) Add(ctx context.Context, pluginID string, z *zip.ReadCloser) (*storage.ExtractedPluginArchive, error) {
-	s.Added[pluginID] = z.File[0].Name
 	if s.AddFunc != nil {
 		return s.AddFunc(ctx, pluginID, z)
 	}

pkg/plugins/manager/manager.go

@@ -242,6 +242,12 @@ func (m *PluginManager) registerAndStart(ctx context.Context, p *plugins.Plugin)
 		m.log.Info("Plugin registered", "pluginID", p.ID)
 	}
 
+	if p.IsExternalPlugin() {
+		if err := m.pluginStorage.Register(ctx, p.ID, p.PluginDir); err != nil {
+			return err
+		}
+	}
+
 	return m.processManager.Start(ctx, p.ID)
 }
 

pkg/plugins/manager/manager_test.go

@@ -22,11 +22,13 @@ func TestPluginManager_Add_Remove(t *testing.T) {
 		const (
 			pluginID, v1 = "test-panel", "1.0.0"
 			zipNameV1    = "test-panel-1.0.0.zip"
+			pluginDirV1  = "/data/plugin/test-panel-1.0.0"
 		)
 
 		// mock a plugin to be returned automatically by the plugin loader
 		pluginV1 := createPlugin(t, pluginID, plugins.External, true, true, func(plugin *plugins.Plugin) {
 			plugin.Info.Version = v1
+			plugin.PluginDir = pluginDirV1
 		})
 		mockZipV1 := &zip.ReadCloser{Reader: zip.Reader{File: []*zip.File{{
 			FileHeader: zip.FileHeader{Name: zipNameV1},
@@ -57,6 +59,11 @@ func TestPluginManager_Add_Remove(t *testing.T) {
 					Path: zipNameV1,
 				}, nil
 			},
+			RegisterFunc: func(_ context.Context, pluginID, pluginDir string) error {
+				require.Equal(t, pluginV1.ID, pluginID)
+				require.Equal(t, pluginV1.PluginDir, pluginDir)
+				return nil
+			},
 			Added:   make(map[string]string),
 			Removed: make(map[string]int),
 		}
@@ -66,7 +73,7 @@ func TestPluginManager_Add_Remove(t *testing.T) {
 		err := pm.Add(context.Background(), pluginID, v1, plugins.CompatOpts{})
 		require.NoError(t, err)
 
-		require.Equal(t, zipNameV1, fs.Added[pluginID])
+		require.Equal(t, pluginV1.PluginDir, fs.Added[pluginID])
 		require.Equal(t, 0, fs.Removed[pluginID])
 		require.Equal(t, 1, proc.Started[pluginID])
 		require.Equal(t, 0, proc.Stopped[pluginID])
@@ -86,12 +93,14 @@ func TestPluginManager_Add_Remove(t *testing.T) {
 
 		t.Run("Update plugin to different version", func(t *testing.T) {
 			const (
-				v2        = "2.0.0"
+				v2          = "2.0.0"
-				zipNameV2 = "test-panel-2.0.0.zip"
+				zipNameV2   = "test-panel-2.0.0.zip"
+				pluginDirV2 = "/data/plugin/test-panel-2.0.0"
 			)
 			// mock a plugin to be returned automatically by the plugin loader
 			pluginV2 := createPlugin(t, pluginID, plugins.External, true, true, func(plugin *plugins.Plugin) {
 				plugin.Info.Version = v2
+				plugin.PluginDir = pluginDirV2
 			})
 
 			mockZipV2 := &zip.ReadCloser{Reader: zip.Reader{File: []*zip.File{{
@@ -121,11 +130,16 @@ func TestPluginManager_Add_Remove(t *testing.T) {
 					Path: zipNameV2,
 				}, nil
 			}
+			fs.RegisterFunc = func(_ context.Context, pluginID, pluginDir string) error {
+				require.Equal(t, pluginV2.ID, pluginID)
+				require.Equal(t, pluginV2.PluginDir, pluginDir)
+				return nil
+			}
 
 			err = pm.Add(context.Background(), pluginID, v2, plugins.CompatOpts{})
 			require.NoError(t, err)
 
-			require.Equal(t, zipNameV2, fs.Added[pluginID])
+			require.Equal(t, pluginDirV2, fs.Added[pluginID])
 			require.Equal(t, 1, fs.Removed[pluginID])
 			require.Equal(t, 2, proc.Started[pluginID])
 			require.Equal(t, 1, proc.Stopped[pluginID])

pkg/plugins/storage/fs.go

@@ -61,10 +61,6 @@ func (fs *FS) Add(ctx context.Context, pluginID string, pluginArchive *zip.ReadC
 		})
 	}
 
-	fs.mu.Lock()
-	fs.store[pluginID] = pluginDir
-	fs.mu.Unlock()
-
 	return &ExtractedPluginArchive{
 		ID:           res.ID,
 		Version:      res.Info.Version,
@@ -73,6 +69,14 @@ func (fs *FS) Add(ctx context.Context, pluginID string, pluginArchive *zip.ReadC
 	}, nil
 }
 
+func (fs *FS) Register(_ context.Context, pluginID, pluginDir string) error {
+	fs.mu.Lock()
+	fs.store[pluginID] = pluginDir
+	fs.mu.Unlock()
+
+	return nil
+}
+
 func (fs *FS) Remove(_ context.Context, pluginID string) error {
 	fs.mu.RLock()
 	pluginDir, exists := fs.store[pluginID]

pkg/plugins/storage/ifaces.go

@@ -7,5 +7,6 @@ import (
 
 type Manager interface {
 	Add(ctx context.Context, pluginID string, rc *zip.ReadCloser) (*ExtractedPluginArchive, error)
+	Register(ctx context.Context, pluginID, pluginDir string) error
 	Remove(ctx context.Context, pluginID string) error
 }