From 8372267739c80a15b468a6c61a160ad5ae2fb98d Mon Sep 17 00:00:00 2001
From: Jesse Hallam <jesse.hallam@gmail.com>
Date: Tue, 8 Aug 2023 18:29:57 -0300
Subject: [PATCH] MM-53355: tidy up plugins (#24194)

* remove feature flag managed plugins

* remove unneeded plugin blocklist

* remove unnecessary wrappers

* documentation and logging improvements

* avoid use of global logger
* leverage wrapped loggers (e.g. consistently log `plugin_id`)
* promote some logs from `Debug` to `Info` for better visibility.
* extract installPluginToFilestore
* rename some variables for consistency / clarity

* make generated
---
 server/channels/app/app_iface.go           |   3 +-
 server/channels/app/cluster_handlers.go    |   4 +-
 server/channels/app/plugin.go              | 161 +++++----------
 server/channels/app/plugin_install.go      | 217 ++++++++++++---------
 server/channels/app/plugin_install_test.go |  11 --
 server/i18n/en.json                        |   4 -
 server/public/model/feature_flags.go       |  28 ---
 server/public/plugin/environment.go        |  27 +--
 8 files changed, 187 insertions(+), 268 deletions(-)

diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go
index 58dc603fe6..720e8aed52 100644
--- a/server/channels/app/app_iface.go
+++ b/server/channels/app/app_iface.go
@@ -248,7 +248,8 @@ type AppIface interface {
 	HubRegister(webConn *platform.WebConn)
 	// HubUnregister unregisters a connection from a hub.
 	HubUnregister(webConn *platform.WebConn)
-	// InstallPlugin unpacks and installs a plugin but does not enable or activate it.
+	// InstallPlugin unpacks and installs a plugin but does not enable or activate it unless the the
+	// plugin was already enabled.
 	InstallPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError)
 	// LogAuditRec logs an audit record using default LvlAuditCLI.
 	LogAuditRec(rec *audit.Record, err error)
diff --git a/server/channels/app/cluster_handlers.go b/server/channels/app/cluster_handlers.go
index d26ea11a68..d8db2b0302 100644
--- a/server/channels/app/cluster_handlers.go
+++ b/server/channels/app/cluster_handlers.go
@@ -16,7 +16,7 @@ func (s *Server) clusterInstallPluginHandler(msg *model.ClusterMessage) {
 	if jsonErr := json.Unmarshal(msg.Data, &data); jsonErr != nil {
 		mlog.Warn("Failed to decode from JSON", mlog.Err(jsonErr))
 	}
-	s.Channels().installPluginFromData(data)
+	s.Channels().installPluginFromClusterMessage(data.Id)
 }
 
 func (s *Server) clusterRemovePluginHandler(msg *model.ClusterMessage) {
@@ -24,7 +24,7 @@ func (s *Server) clusterRemovePluginHandler(msg *model.ClusterMessage) {
 	if jsonErr := json.Unmarshal(msg.Data, &data); jsonErr != nil {
 		mlog.Warn("Failed to decode from JSON", mlog.Err(jsonErr))
 	}
-	s.Channels().removePluginFromData(data)
+	s.Channels().removePluginFromClusterMessage(data.Id)
 }
 
 func (s *Server) clusterPluginEventHandler(msg *model.ClusterMessage) {
diff --git a/server/channels/app/plugin.go b/server/channels/app/plugin.go
index 64156ed64f..6c2ef31b43 100644
--- a/server/channels/app/plugin.go
+++ b/server/channels/app/plugin.go
@@ -30,11 +30,14 @@ import (
 	"github.com/mattermost/mattermost/server/v8/platform/shared/filestore"
 )
 
+// prepackagedPluginsDir is the hard-coded folder name where prepackaged plugins are bundled
+// alongside the server.
 const prepackagedPluginsDir = "prepackaged_plugins"
 
+// pluginSignaturePath tracks the path to the plugin bundle and signature for the given plugin.
 type pluginSignaturePath struct {
 	pluginID      string
-	path          string
+	bundlePath    string
 	signaturePath string
 }
 
@@ -161,16 +164,18 @@ func (ch *Channels) syncPluginsActiveState() {
 				defer wg.Done()
 
 				pluginID := plugin.Manifest.Id
+				logger := ch.srv.Log().With(mlog.String("plugin_id", pluginID), mlog.String("bundle_path", plugin.Path))
+
 				updatedManifest, activated, err := pluginsEnvironment.Activate(pluginID)
 				if err != nil {
-					plugin.WrapLogger(ch.srv.Log()).Error("Unable to activate plugin", mlog.Err(err))
+					logger.Error("Unable to activate plugin", mlog.Err(err))
 					return
 				}
 
 				if activated {
 					// Notify all cluster clients if ready
 					if err := ch.notifyPluginEnabled(updatedManifest); err != nil {
-						ch.srv.Log().Error("Failed to notify cluster on plugin enable", mlog.Err(err))
+						logger.Error("Failed to notify cluster on plugin enable", mlog.Err(err))
 					}
 				}
 			}(plugin)
@@ -181,7 +186,7 @@ func (ch *Channels) syncPluginsActiveState() {
 	}
 
 	if err := ch.notifyPluginStatusesChanged(); err != nil {
-		mlog.Warn("failed to notify plugin status changed", mlog.Err(err))
+		ch.srv.Log().Warn("failed to notify plugin status changed", mlog.Err(err))
 	}
 }
 
@@ -213,12 +218,12 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s
 	ch.srv.Log().Info("Starting up plugins")
 
 	if err := os.Mkdir(pluginDir, 0744); err != nil && !os.IsExist(err) {
-		mlog.Error("Failed to start up plugins", mlog.Err(err))
+		ch.srv.Log().Error("Failed to start up plugins", mlog.Err(err))
 		return
 	}
 
 	if err := os.Mkdir(webappPluginDir, 0744); err != nil && !os.IsExist(err) {
-		mlog.Error("Failed to start up plugins", mlog.Err(err))
+		ch.srv.Log().Error("Failed to start up plugins", mlog.Err(err))
 		return
 	}
 
@@ -235,7 +240,7 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s
 		ch.srv.GetMetrics(),
 	)
 	if err != nil {
-		mlog.Error("Failed to start up plugins", mlog.Err(err))
+		ch.srv.Log().Error("Failed to start up plugins", mlog.Err(err))
 		return
 	}
 	ch.pluginsLock.Lock()
@@ -245,19 +250,17 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s
 	ch.pluginsEnvironment.TogglePluginHealthCheckJob(*ch.cfgSvc.Config().PluginSettings.EnableHealthCheck)
 
 	if err := ch.syncPlugins(); err != nil {
-		mlog.Error("Failed to sync plugins from the file store", mlog.Err(err))
+		ch.srv.Log().Error("Failed to sync plugins from the file store", mlog.Err(err))
 	}
 
 	plugins := ch.processPrepackagedPlugins(prepackagedPluginsDir)
 	pluginsEnvironment = ch.GetPluginsEnvironment()
 	if pluginsEnvironment == nil {
-		mlog.Info("Plugins environment not found, server is likely shutting down")
+		ch.srv.Log().Info("Plugins environment not found, server is likely shutting down")
 		return
 	}
 	pluginsEnvironment.SetPrepackagedPlugins(plugins)
 
-	ch.installFeatureFlagPlugins()
-
 	// Sync plugin active state when config changes. Also notify plugins.
 	ch.pluginsLock.Lock()
 	ch.RemoveConfigListener(ch.pluginConfigListenerID)
@@ -265,7 +268,6 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s
 		// If plugin status remains unchanged, only then run this.
 		// Because (*App).InitPlugins is already run as a config change hook.
 		if *old.PluginSettings.Enable == *new.PluginSettings.Enable {
-			ch.installFeatureFlagPlugins()
 			ch.syncPluginsActiveState()
 		}
 
@@ -290,7 +292,7 @@ func (a *App) SyncPlugins() *model.AppError {
 // SyncPlugins synchronizes the plugins installed locally
 // with the plugin bundles available in the file store.
 func (ch *Channels) syncPlugins() *model.AppError {
-	mlog.Info("Syncing plugins from the file store")
+	ch.srv.Log().Info("Syncing plugins from the file store")
 
 	pluginsEnvironment := ch.GetPluginsEnvironment()
 	if pluginsEnvironment == nil {
@@ -307,16 +309,19 @@ func (ch *Channels) syncPlugins() *model.AppError {
 		wg.Add(1)
 		go func(pluginID string) {
 			defer wg.Done()
+
+			logger := ch.srv.Log().With(mlog.String("plugin_id", pluginID))
+
 			// Only handle managed plugins with .filestore flag file.
 			_, err := os.Stat(filepath.Join(*ch.cfgSvc.Config().PluginSettings.Directory, pluginID, managedPluginFileName))
 			if os.IsNotExist(err) {
-				mlog.Warn("Skipping sync for unmanaged plugin", mlog.String("plugin_id", pluginID))
+				logger.Warn("Skipping sync for unmanaged plugin")
 			} else if err != nil {
-				mlog.Error("Skipping sync for plugin after failure to check if managed", mlog.String("plugin_id", pluginID), mlog.Err(err))
+				logger.Error("Skipping sync for plugin after failure to check if managed", mlog.Err(err))
 			} else {
-				mlog.Debug("Removing local installation of managed plugin before sync", mlog.String("plugin_id", pluginID))
+				logger.Info("Removing local installation of managed plugin before sync")
 				if err := ch.removePluginLocally(pluginID); err != nil {
-					mlog.Error("Failed to remove local installation of managed plugin before sync", mlog.String("plugin_id", pluginID), mlog.Err(err))
+					logger.Error("Failed to remove local installation of managed plugin before sync", mlog.Err(err))
 				}
 			}
 		}(plugin.Manifest.Id)
@@ -333,26 +338,28 @@ func (ch *Channels) syncPlugins() *model.AppError {
 		wg.Add(1)
 		go func(plugin *pluginSignaturePath) {
 			defer wg.Done()
-			reader, appErr := ch.srv.fileReader(plugin.path)
+			logger := ch.srv.Log().With(mlog.String("plugin_id", plugin.pluginID), mlog.String("bundle_path", plugin.bundlePath))
+
+			bundle, appErr := ch.srv.fileReader(plugin.bundlePath)
 			if appErr != nil {
-				mlog.Error("Failed to open plugin bundle from file store.", mlog.String("bundle", plugin.path), mlog.Err(appErr))
+				logger.Error("Failed to open plugin bundle from file store.", mlog.Err(appErr))
 				return
 			}
-			defer reader.Close()
+			defer bundle.Close()
 
 			var signature filestore.ReadCloseSeeker
 			if *ch.cfgSvc.Config().PluginSettings.RequirePluginSignature {
 				signature, appErr = ch.srv.fileReader(plugin.signaturePath)
 				if appErr != nil {
-					mlog.Error("Failed to open plugin signature from file store.", mlog.Err(appErr))
+					logger.Error("Failed to open plugin signature from file store.", mlog.Err(appErr))
 					return
 				}
 				defer signature.Close()
 			}
 
-			mlog.Info("Syncing plugin from file store", mlog.String("bundle", plugin.path))
-			if _, err := ch.installPluginLocally(reader, signature, installPluginLocallyAlways); err != nil && err.Id != "app.plugin.blocked.app_error" && err.Id != "app.plugin.skip_installation.app_error" {
-				mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(err))
+			logger.Info("Syncing plugin from file store")
+			if _, err := ch.installPluginLocally(bundle, signature, installPluginLocallyAlways); err != nil && err.Id != "app.plugin.skip_installation.app_error" {
+				logger.Error("Failed to sync plugin from file store", mlog.Err(err))
 			}
 		}(plugin)
 	}
@@ -370,7 +377,7 @@ func (ch *Channels) ShutDownPlugins() {
 		return
 	}
 
-	mlog.Info("Shutting down plugins")
+	ch.srv.Log().Info("Shutting down plugins")
 
 	pluginsEnvironment.Shutdown()
 
@@ -383,7 +390,7 @@ func (ch *Channels) ShutDownPlugins() {
 	if ch.pluginsEnvironment == pluginsEnvironment {
 		ch.pluginsEnvironment = nil
 	} else {
-		mlog.Warn("Another PluginsEnvironment detected while shutting down plugins.")
+		ch.srv.Log().Warn("Another PluginsEnvironment detected while shutting down plugins.")
 	}
 }
 
@@ -735,7 +742,7 @@ func (a *App) mergeLocalPlugins(remoteMarketplacePlugins map[string]*model.Marke
 		if plugin.Manifest.IconPath != "" {
 			iconData, err = getIcon(filepath.Join(plugin.Path, plugin.Manifest.IconPath))
 			if err != nil {
-				mlog.Warn("Error loading local plugin icon", mlog.String("plugin", plugin.Manifest.Id), mlog.String("icon_path", plugin.Manifest.IconPath), mlog.Err(err))
+				a.Log().Warn("Error loading local plugin icon", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("icon_path", plugin.Manifest.IconPath), mlog.Err(err))
 			}
 		}
 
@@ -850,7 +857,7 @@ func (ch *Channels) notifyPluginEnabled(manifest *model.Manifest) error {
 	// which may result in a 404.
 	for _, status := range statuses {
 		if status.PluginId == manifest.Id && status.Version != manifest.Version {
-			mlog.Debug("Not ready to notify webclients", mlog.String("cluster_id", status.ClusterId), mlog.String("plugin_id", manifest.Id))
+			ch.srv.Log().Debug("Not ready to notify webclients", mlog.String("cluster_id", status.ClusterId), mlog.String("plugin_id", manifest.Id))
 			return nil
 		}
 	}
@@ -879,7 +886,7 @@ func (ch *Channels) getPluginsFromFilePaths(fileStorePaths []string) map[string]
 			id := strings.TrimSuffix(filepath.Base(path), ".tar.gz")
 			helper := &pluginSignaturePath{
 				pluginID:      id,
-				path:          path,
+				bundlePath:    path,
 				signaturePath: "",
 			}
 			pluginSignaturePathMap[id] = helper
@@ -911,7 +918,7 @@ func (ch *Channels) processPrepackagedPlugins(pluginsDir string) []*plugin.Prepa
 		return nil
 	})
 	if err != nil {
-		mlog.Error("Failed to walk prepackaged plugins", mlog.Err(err))
+		ch.srv.Log().Error("Failed to walk prepackaged plugins", mlog.Err(err))
 		return nil
 	}
 
@@ -927,11 +934,10 @@ func (ch *Channels) processPrepackagedPlugins(pluginsDir string) []*plugin.Prepa
 			p, err := ch.processPrepackagedPlugin(psPath)
 			if err != nil {
 				var appErr *model.AppError
-				// A log line already appears if the plugin is on the blocklist
-				if errors.As(err, &appErr) && (appErr.Id == "app.plugin.blocked.app_error" || appErr.Id == "app.plugin.skip_installation.app_error") {
+				if errors.As(err, &appErr) && appErr.Id == "app.plugin.skip_installation.app_error" {
 					return
 				}
-				mlog.Error("Failed to install prepackaged plugin", mlog.String("path", psPath.path), mlog.Err(err))
+				ch.srv.Log().Error("Failed to install prepackaged plugin", mlog.String("plugin_id", p.Manifest.Id), mlog.String("bundle_path", psPath.bundlePath), mlog.Err(err))
 				return
 			}
 			prepackagedPlugins <- p
@@ -951,11 +957,13 @@ func (ch *Channels) processPrepackagedPlugins(pluginsDir string) []*plugin.Prepa
 // processPrepackagedPlugin will return the prepackaged plugin metadata and will also
 // install the prepackaged plugin if it had been previously enabled and AutomaticPrepackagedPlugins is true.
 func (ch *Channels) processPrepackagedPlugin(pluginPath *pluginSignaturePath) (*plugin.PrepackagedPlugin, error) {
-	mlog.Debug("Processing prepackaged plugin", mlog.String("path", pluginPath.path))
+	logger := ch.srv.Log().With(mlog.String("plugin_id", pluginPath.pluginID), mlog.String("bundle_path", pluginPath.bundlePath))
 
-	fileReader, err := os.Open(pluginPath.path)
+	logger.Info("Processing prepackaged plugin")
+
+	fileReader, err := os.Open(pluginPath.bundlePath)
 	if err != nil {
-		return nil, errors.Wrapf(err, "Failed to open prepackaged plugin %s", pluginPath.path)
+		return nil, errors.Wrapf(err, "Failed to open prepackaged plugin %s", pluginPath.bundlePath)
 	}
 	defer fileReader.Close()
 
@@ -965,9 +973,9 @@ func (ch *Channels) processPrepackagedPlugin(pluginPath *pluginSignaturePath) (*
 	}
 	defer os.RemoveAll(tmpDir)
 
-	plugin, pluginDir, err := getPrepackagedPlugin(pluginPath, fileReader, tmpDir)
+	plugin, pluginDir, err := ch.buildPrepackagedPlugin(pluginPath, fileReader, tmpDir)
 	if err != nil {
-		return nil, errors.Wrapf(err, "Failed to get prepackaged plugin %s", pluginPath.path)
+		return nil, errors.Wrapf(err, "Failed to get prepackaged plugin %s", pluginPath.bundlePath)
 	}
 
 	// Skip installing the plugin at all if automatic prepackaged plugins is disabled
@@ -981,87 +989,24 @@ func (ch *Channels) processPrepackagedPlugin(pluginPath *pluginSignaturePath) (*
 		return plugin, nil
 	}
 
-	mlog.Debug("Installing prepackaged plugin", mlog.String("path", pluginPath.path))
+	logger.Info("Installing prepackaged plugin")
 	if _, err := ch.installExtractedPlugin(plugin.Manifest, pluginDir, installPluginLocallyOnlyIfNewOrUpgrade); err != nil {
-		return nil, errors.Wrapf(err, "Failed to install extracted prepackaged plugin %s", pluginPath.path)
+		return nil, errors.Wrapf(err, "Failed to install extracted prepackaged plugin %s", pluginPath.bundlePath)
 	}
 
 	return plugin, nil
 }
 
-// installFeatureFlagPlugins handles the automatic installation/upgrade of plugins from feature flags
-func (ch *Channels) installFeatureFlagPlugins() {
-	ffControledPlugins := ch.cfgSvc.Config().FeatureFlags.Plugins()
-
-	// Respect the automatic prepackaged disable setting
-	if !*ch.cfgSvc.Config().PluginSettings.AutomaticPrepackagedPlugins {
-		return
-	}
-
-	for pluginID, version := range ffControledPlugins {
-		// Skip installing if the plugin has been previously disabled.
-		pluginState := ch.cfgSvc.Config().PluginSettings.PluginStates[pluginID]
-		if pluginState != nil && !pluginState.Enable {
-			ch.srv.Log().Debug("Not auto installing/upgrade because plugin was disabled", mlog.String("plugin_id", pluginID), mlog.String("version", version))
-			continue
-		}
-
-		// Check if we already installed this version as InstallMarketplacePlugin can't handle re-installs well.
-		pluginStatus, err := ch.GetPluginStatus(pluginID)
-		pluginExists := err == nil
-		if pluginExists && pluginStatus.Version == version {
-			continue
-		}
-
-		if version != "" && version != "control" {
-			// If we are on-prem skip installation if this is a downgrade
-			license := ch.srv.License()
-			inCloud := license != nil && *license.Features.Cloud
-			if !inCloud && pluginExists {
-				parsedVersion, err := semver.Parse(version)
-				if err != nil {
-					ch.srv.Log().Debug("Bad version from feature flag", mlog.String("plugin_id", pluginID), mlog.Err(err), mlog.String("version", version))
-					return
-				}
-				parsedExistingVersion, err := semver.Parse(pluginStatus.Version)
-				if err != nil {
-					ch.srv.Log().Debug("Bad version from plugin manifest", mlog.String("plugin_id", pluginID), mlog.Err(err), mlog.String("version", pluginStatus.Version))
-					return
-				}
-
-				if parsedVersion.LTE(parsedExistingVersion) {
-					ch.srv.Log().Debug("Skip installation because given version was a downgrade and on-prem installations should not downgrade.", mlog.String("plugin_id", pluginID), mlog.Err(err), mlog.String("version", pluginStatus.Version))
-					return
-				}
-			}
-
-			_, err := ch.InstallMarketplacePlugin(&model.InstallMarketplacePluginRequest{
-				Id:      pluginID,
-				Version: version,
-			})
-			if err != nil {
-				ch.srv.Log().Debug("Unable to install plugin from FF manifest", mlog.String("plugin_id", pluginID), mlog.Err(err), mlog.String("version", version))
-			} else {
-				if err := ch.enablePlugin(pluginID); err != nil {
-					ch.srv.Log().Debug("Unable to enable plugin installed from feature flag.", mlog.String("plugin_id", pluginID), mlog.Err(err), mlog.String("version", version))
-				} else {
-					ch.srv.Log().Debug("Installed and enabled plugin.", mlog.String("plugin_id", pluginID), mlog.String("version", version))
-				}
-			}
-		}
-	}
-}
-
-// getPrepackagedPlugin builds a PrepackagedPlugin from the plugin at the given path, additionally returning the directory in which it was extracted.
-func getPrepackagedPlugin(pluginPath *pluginSignaturePath, pluginFile io.ReadSeeker, tmpDir string) (*plugin.PrepackagedPlugin, string, error) {
+// buildPrepackagedPlugin builds a PrepackagedPlugin from the plugin at the given path, additionally returning the directory in which it was extracted.
+func (ch *Channels) buildPrepackagedPlugin(pluginPath *pluginSignaturePath, pluginFile io.ReadSeeker, tmpDir string) (*plugin.PrepackagedPlugin, string, error) {
 	manifest, pluginDir, appErr := extractPlugin(pluginFile, tmpDir)
 	if appErr != nil {
-		return nil, "", errors.Wrapf(appErr, "Failed to extract plugin with path %s", pluginPath.path)
+		return nil, "", errors.Wrapf(appErr, "Failed to extract plugin with path %s", pluginPath.bundlePath)
 	}
 
 	plugin := new(plugin.PrepackagedPlugin)
 	plugin.Manifest = manifest
-	plugin.Path = pluginPath.path
+	plugin.Path = pluginPath.bundlePath
 
 	if pluginPath.signaturePath != "" {
 		sig := pluginPath.signaturePath
@@ -1079,7 +1024,7 @@ func getPrepackagedPlugin(pluginPath *pluginSignaturePath, pluginFile io.ReadSee
 	if manifest.IconPath != "" {
 		iconData, err := getIcon(filepath.Join(pluginDir, manifest.IconPath))
 		if err != nil {
-			mlog.Warn("Error loading local plugin icon", mlog.String("plugin", plugin.Manifest.Id), mlog.String("icon_path", plugin.Manifest.IconPath), mlog.Err(err))
+			ch.srv.Log().Warn("Error loading local plugin icon", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("icon_path", plugin.Manifest.IconPath), mlog.Err(err))
 		}
 		plugin.IconData = iconData
 	}
diff --git a/server/channels/app/plugin_install.go b/server/channels/app/plugin_install.go
index 5f99c86bb9..90e645e545 100644
--- a/server/channels/app/plugin_install.go
+++ b/server/channels/app/plugin_install.go
@@ -46,7 +46,6 @@ import (
 	"github.com/blang/semver"
 
 	"github.com/mattermost/mattermost/server/public/model"
-	"github.com/mattermost/mattermost/server/public/plugin"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/mattermost/mattermost/server/public/utils"
 	"github.com/mattermost/mattermost/server/v8/platform/shared/filestore"
@@ -59,88 +58,93 @@ const managedPluginFileName = ".filestore"
 // fileStorePluginFolder is the folder name in the file store of the plugin bundles installed.
 const fileStorePluginFolder = "plugins"
 
-func (ch *Channels) installPluginFromData(data model.PluginEventData) {
-	mlog.Debug("Installing plugin as per cluster message", mlog.String("plugin_id", data.Id))
+// installPluginFromClusterMessage is called when a peer activates a plugin in the filestore,
+// signalling all other servers to do the same.
+func (ch *Channels) installPluginFromClusterMessage(pluginID string) {
+	logger := ch.srv.Log().With(mlog.String("plugin_id", pluginID))
+
+	logger.Info("Installing plugin as per cluster message")
 
 	pluginSignaturePathMap, appErr := ch.getPluginsFromFolder()
 	if appErr != nil {
-		mlog.Error("Failed to get plugin signatures from filestore. Can't install plugin from data.", mlog.Err(appErr))
+		logger.Error("Failed to get plugin signatures from filestore.", mlog.Err(appErr))
 		return
 	}
-	plugin, ok := pluginSignaturePathMap[data.Id]
+	plugin, ok := pluginSignaturePathMap[pluginID]
 	if !ok {
-		mlog.Error("Failed to get plugin signature from filestore. Can't install plugin from data.", mlog.String("plugin id", data.Id))
+		logger.Error("Failed to get plugin signature from filestore.")
 		return
 	}
 
-	reader, appErr := ch.srv.fileReader(plugin.path)
+	bundle, appErr := ch.srv.fileReader(plugin.bundlePath)
 	if appErr != nil {
-		mlog.Error("Failed to open plugin bundle from file store.", mlog.String("bundle", plugin.path), mlog.Err(appErr))
+		logger.Error("Failed to open plugin bundle from file store.", mlog.Err(appErr))
 		return
 	}
-	defer reader.Close()
+	defer bundle.Close()
 
 	var signature filestore.ReadCloseSeeker
 	if *ch.cfgSvc.Config().PluginSettings.RequirePluginSignature {
 		signature, appErr = ch.srv.fileReader(plugin.signaturePath)
 		if appErr != nil {
-			mlog.Error("Failed to open plugin signature from file store.", mlog.Err(appErr))
+			logger.Error("Failed to open plugin signature from file store.", mlog.Err(appErr))
 			return
 		}
 		defer signature.Close()
 	}
 
-	manifest, appErr := ch.installPluginLocally(reader, signature, installPluginLocallyAlways)
+	manifest, appErr := ch.installPluginLocally(bundle, signature, installPluginLocallyAlways)
 	if appErr != nil {
 		// A log line already appears if the plugin is on the blocklist or skipped
 		if appErr.Id != "app.plugin.blocked.app_error" && appErr.Id != "app.plugin.skip_installation.app_error" {
-			mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(appErr))
+			logger.Error("Failed to sync plugin from file store", mlog.Err(appErr))
 		}
 		return
 	}
 
 	if err := ch.notifyPluginEnabled(manifest); err != nil {
-		mlog.Error("Failed notify plugin enabled", mlog.Err(err))
+		logger.Error("Failed notify plugin enabled", mlog.Err(err))
 	}
 
 	if err := ch.notifyPluginStatusesChanged(); err != nil {
-		mlog.Error("Failed to notify plugin status changed", mlog.Err(err))
+		logger.Error("Failed to notify plugin status changed", mlog.Err(err))
 	}
 }
 
-func (ch *Channels) removePluginFromData(data model.PluginEventData) {
-	mlog.Debug("Removing plugin as per cluster message", mlog.String("plugin_id", data.Id))
+// removePluginFromClusterMessage is called when a peer removes a plugin, signalling all other
+// servers to do the same.
+func (ch *Channels) removePluginFromClusterMessage(pluginID string) {
+	logger := ch.srv.Log().With(mlog.String("plugin_id", pluginID))
 
-	if err := ch.removePluginLocally(data.Id); err != nil {
-		mlog.Warn("Failed to remove plugin locally", mlog.Err(err), mlog.String("id", data.Id))
+	logger.Info("Removing plugin as per cluster message")
+
+	if err := ch.removePluginLocally(pluginID); err != nil {
+		logger.Error("Failed to remove plugin locally", mlog.Err(err))
 	}
 
 	if err := ch.notifyPluginStatusesChanged(); err != nil {
-		mlog.Warn("failed to notify plugin status changed", mlog.Err(err))
+		logger.Error("failed to notify plugin status changed", mlog.Err(err))
 	}
 }
 
-// InstallPluginWithSignature verifies and installs plugin.
-func (ch *Channels) installPluginWithSignature(pluginFile, signature io.ReadSeeker) (*model.Manifest, *model.AppError) {
-	return ch.installPlugin(pluginFile, signature, installPluginLocallyAlways)
-}
-
-// InstallPlugin unpacks and installs a plugin but does not enable or activate it.
+// InstallPlugin unpacks and installs a plugin but does not enable or activate it unless the the
+// plugin was already enabled.
 func (a *App) InstallPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError) {
 	installationStrategy := installPluginLocallyOnlyIfNew
 	if replace {
 		installationStrategy = installPluginLocallyAlways
 	}
 
-	return a.installPlugin(pluginFile, nil, installationStrategy)
+	return a.ch.installPlugin(pluginFile, nil, installationStrategy)
 }
 
-func (a *App) installPlugin(pluginFile, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) {
-	return a.ch.installPlugin(pluginFile, signature, installationStrategy)
-}
-
-func (ch *Channels) installPlugin(pluginFile, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) {
-	manifest, appErr := ch.installPluginLocally(pluginFile, signature, installationStrategy)
+// installPlugin extracts and installs the given plugin bundle (optionally signed) for the
+// current server, activating the plugin if already enabled, installs it to the filestore for
+// cluster peers to use, and then broadcasts the change to connected websockets.
+//
+// The given installation strategy decides how to handle upgrade scenarios.
+func (ch *Channels) installPlugin(bundle, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) {
+	manifest, appErr := ch.installPluginLocally(bundle, signature, installationStrategy)
 	if appErr != nil {
 		return nil, appErr
 	}
@@ -149,17 +153,42 @@ func (ch *Channels) installPlugin(pluginFile, signature io.ReadSeeker, installat
 		return nil, nil
 	}
 
+	logger := ch.srv.Log().With(mlog.String("plugin_id", manifest.Id))
+
+	appErr = ch.installPluginToFilestore(manifest, bundle, signature)
+	if appErr != nil {
+		return nil, appErr
+	}
+
+	if err := ch.notifyPluginEnabled(manifest); err != nil {
+		logger.Warn("Failed notify plugin enabled", mlog.Err(err))
+	}
+
+	if err := ch.notifyPluginStatusesChanged(); err != nil {
+		logger.Warn("Failed to notify plugin status changed", mlog.Err(err))
+	}
+
+	return manifest, nil
+}
+
+// installPluginToFilestore saves the given plugin bundle (optionally signed) to the filestore,
+// notifying cluster peers accordingly.
+func (ch *Channels) installPluginToFilestore(manifest *model.Manifest, bundle, signature io.ReadSeeker) *model.AppError {
 	if signature != nil {
-		signature.Seek(0, 0)
-		if _, appErr = ch.srv.writeFile(signature, getSignatureStorePath(manifest.Id)); appErr != nil {
-			return nil, model.NewAppError("saveSignature", "app.plugin.store_signature.app_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
+		_, err := signature.Seek(0, 0)
+		if err != nil {
+			return model.NewAppError("saveSignature", "app.plugin.store_signature.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+		}
+
+		if _, appErr := ch.srv.writeFile(signature, getSignatureStorePath(manifest.Id)); appErr != nil {
+			return model.NewAppError("saveSignature", "app.plugin.store_signature.app_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
 		}
 	}
 
 	// Store bundle in the file store to allow access from other servers.
-	pluginFile.Seek(0, 0)
-	if _, appErr := ch.srv.writeFile(pluginFile, getBundleStorePath(manifest.Id)); appErr != nil {
-		return nil, model.NewAppError("uploadPlugin", "app.plugin.store_bundle.app_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
+	bundle.Seek(0, 0)
+	if _, appErr := ch.srv.writeFile(bundle, getBundleStorePath(manifest.Id)); appErr != nil {
+		return model.NewAppError("uploadPlugin", "app.plugin.store_bundle.app_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
 	}
 
 	ch.notifyClusterPluginEvent(
@@ -169,20 +198,15 @@ func (ch *Channels) installPlugin(pluginFile, signature io.ReadSeeker, installat
 		},
 	)
 
-	if err := ch.notifyPluginEnabled(manifest); err != nil {
-		mlog.Warn("Failed notify plugin enabled", mlog.Err(err))
-	}
-
-	if err := ch.notifyPluginStatusesChanged(); err != nil {
-		mlog.Warn("Failed to notify plugin status changed", mlog.Err(err))
-	}
-
-	return manifest, nil
+	return nil
 }
 
-// InstallMarketplacePlugin installs a plugin listed in the marketplace server. It will get the plugin bundle
-// from the prepackaged folder, if available, or remotely if EnableRemoteMarketplace is true.
+// InstallMarketplacePlugin installs a plugin listed in the marketplace server. It will get the
+// plugin bundle from the prepackaged folder, if available, or remotely if EnableRemoteMarketplace
+// is true.
 func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePluginRequest) (*model.Manifest, *model.AppError) {
+	logger := ch.srv.Log().With(mlog.String("plugin_id", request.Id))
+
 	var pluginFile, signatureFile io.ReadSeeker
 
 	prepackagedPlugin, appErr := ch.getPrepackagedPlugin(request.Id, request.Version)
@@ -205,7 +229,7 @@ func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePl
 		plugin, appErr = ch.getRemoteMarketplacePlugin(request.Id, request.Version)
 		// The plugin might only be prepackaged and not on the Marketplace.
 		if appErr != nil && appErr.Id != "app.plugin.marketplace_plugins.not_found.app_error" {
-			mlog.Warn("Failed to reach Marketplace to install plugin", mlog.String("plugin_id", request.Id), mlog.Err(appErr))
+			logger.Warn("Failed to reach Marketplace to install plugin", mlog.Err(appErr))
 		}
 
 		if plugin != nil {
@@ -245,7 +269,7 @@ func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePl
 		return nil, model.NewAppError("InstallMarketplacePlugin", "app.plugin.marketplace_plugins.signature_not_found.app_error", nil, "", http.StatusInternalServerError)
 	}
 
-	manifest, appErr := ch.installPluginWithSignature(pluginFile, signatureFile)
+	manifest, appErr := ch.installPlugin(pluginFile, signatureFile, installPluginLocallyAlways)
 	if appErr != nil {
 		return nil, appErr
 	}
@@ -264,7 +288,11 @@ const (
 	installPluginLocallyAlways
 )
 
-func (ch *Channels) installPluginLocally(pluginFile, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) {
+// installPluginLocally extracts and installs the given plugin bundle (optionally signed) for the
+// current server, activating the plugin if already enabled.
+//
+// The given installation strategy decides how to handle upgrade scenarios.
+func (ch *Channels) installPluginLocally(bundle, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) {
 	pluginsEnvironment := ch.GetPluginsEnvironment()
 	if pluginsEnvironment == nil {
 		return nil, model.NewAppError("installPluginLocally", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented)
@@ -272,7 +300,7 @@ func (ch *Channels) installPluginLocally(pluginFile, signature io.ReadSeeker, in
 
 	// verify signature
 	if signature != nil {
-		if err := ch.verifyPlugin(pluginFile, signature); err != nil {
+		if err := ch.verifyPlugin(bundle, signature); err != nil {
 			return nil, err
 		}
 	}
@@ -283,7 +311,7 @@ func (ch *Channels) installPluginLocally(pluginFile, signature io.ReadSeeker, in
 	}
 	defer os.RemoveAll(tmpDir)
 
-	manifest, pluginDir, appErr := extractPlugin(pluginFile, tmpDir)
+	manifest, pluginDir, appErr := extractPlugin(bundle, tmpDir)
 	if appErr != nil {
 		return nil, appErr
 	}
@@ -296,9 +324,10 @@ func (ch *Channels) installPluginLocally(pluginFile, signature io.ReadSeeker, in
 	return manifest, nil
 }
 
-func extractPlugin(pluginFile io.ReadSeeker, extractDir string) (*model.Manifest, string, *model.AppError) {
-	pluginFile.Seek(0, 0)
-	if err := extractTarGz(pluginFile, extractDir); err != nil {
+// extractPlugin unpacks the given plugin bundle into the specified directory.
+func extractPlugin(bundle io.ReadSeeker, extractDir string) (*model.Manifest, string, *model.AppError) {
+	bundle.Seek(0, 0)
+	if err := extractTarGz(bundle, extractDir); err != nil {
 		return nil, "", model.NewAppError("extractPlugin", "app.plugin.extract.app_error", nil, "", http.StatusBadRequest).Wrap(err)
 	}
 
@@ -307,6 +336,8 @@ func extractPlugin(pluginFile io.ReadSeeker, extractDir string) (*model.Manifest
 		return nil, "", model.NewAppError("extractPlugin", "app.plugin.filesystem.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
+	// If the root of the plugin bundle consists of exactly one directory, assume the plugin
+	// is contained therein. Otherwise the root directory is expected to contain the plugin.
 	if len(dir) == 1 && dir[0].IsDir() {
 		extractDir = filepath.Join(extractDir, dir[0].Name())
 	}
@@ -317,13 +348,21 @@ func extractPlugin(pluginFile io.ReadSeeker, extractDir string) (*model.Manifest
 	}
 
 	if !model.IsValidPluginId(manifest.Id) {
-		return nil, "", model.NewAppError("installPluginLocally", "app.plugin.invalid_id.app_error", map[string]any{"Min": model.MinIdLength, "Max": model.MaxIdLength, "Regex": model.ValidIdRegex}, "", http.StatusBadRequest)
+		return nil, "", model.NewAppError("extractPlugin", "app.plugin.invalid_id.app_error", map[string]any{"Min": model.MinIdLength, "Max": model.MaxIdLength, "Regex": model.ValidIdRegex}, "", http.StatusBadRequest)
 	}
 
 	return manifest, extractDir, nil
 }
 
+// installExtractedPlugin installs a plugin previously extracted to a temporary directory,
+// activating the plugin automatically if already enabled by the server configuration.
+//
+// The given installation strategy decides how to handle upgrade scenarios.
 func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginDir string, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) {
+	logger := ch.srv.Log().With(mlog.String("plugin_id", manifest.Id))
+
+	logger.Info("Installing extracted plugin", mlog.String("version", manifest.Version))
+
 	pluginsEnvironment := ch.GetPluginsEnvironment()
 	if pluginsEnvironment == nil {
 		return nil, model.NewAppError("installExtractedPlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented)
@@ -334,12 +373,6 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD
 		return nil, model.NewAppError("installExtractedPlugin", "app.plugin.install.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
-	// Check plugin id is not blocked
-	if plugin.PluginIDIsBlocked(manifest.Id) {
-		mlog.Debug("Skipping installation of plugin since plugin is on blocklist. Some plugins are blocked because they are built into this version of Mattermost.", mlog.String("plugin_id", manifest.Id))
-		return nil, model.NewAppError("installExtractedPlugin", "app.plugin.blocked.app_error", map[string]any{"Id": manifest.Id}, "", http.StatusInternalServerError)
-	}
-
 	// Check for plugins installed with the same ID.
 	var existingManifest *model.Manifest
 	for _, bundle := range bundles {
@@ -366,30 +399,30 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD
 
 			existingVersion, err = semver.Parse(existingManifest.Version)
 			if err != nil {
-				return nil, model.NewAppError("installExtractedPlugin", "app.plugin.invalid_version.app_error", nil, "", http.StatusBadRequest)
+				return nil, model.NewAppError("installExtractedPlugin", "app.plugin.invalid_version.app_error", nil, "", http.StatusInternalServerError)
 			}
 
 			if version.LTE(existingVersion) {
-				mlog.Debug("Skipping local installation of plugin since existing version is newer", mlog.String("plugin_id", manifest.Id))
+				logger.Warn("Skipping local installation of plugin since existing version is newer", mlog.String("version", version.String()), mlog.String("existing_version", existingVersion.String()))
 				return nil, model.NewAppError("installExtractedPlugin", "app.plugin.skip_installation.app_error", map[string]any{"Id": manifest.Id}, "", http.StatusInternalServerError)
 			}
 		}
 
-		// Otherwise remove the existing installation prior to install below.
-		mlog.Debug("Removing existing installation of plugin before local install", mlog.String("plugin_id", existingManifest.Id), mlog.String("version", existingManifest.Version))
+		// Otherwise remove the existing installation prior to installing below.
+		logger.Info("Removing existing installation of plugin before local install", mlog.String("existing_version", existingManifest.Version))
 		if err := ch.removePluginLocally(existingManifest.Id); err != nil {
-			return nil, model.NewAppError("installExtractedPlugin", "app.plugin.install_id_failed_remove.app_error", nil, "", http.StatusBadRequest)
+			return nil, model.NewAppError("installExtractedPlugin", "app.plugin.install_id_failed_remove.app_error", nil, "", http.StatusInternalServerError)
 		}
 	}
 
-	pluginPath := filepath.Join(*ch.cfgSvc.Config().PluginSettings.Directory, manifest.Id)
-	err = utils.CopyDir(fromPluginDir, pluginPath)
+	bundlePath := filepath.Join(*ch.cfgSvc.Config().PluginSettings.Directory, manifest.Id)
+	err = utils.CopyDir(fromPluginDir, bundlePath)
 	if err != nil {
 		return nil, model.NewAppError("installExtractedPlugin", "app.plugin.mvdir.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
 	// Flag plugin locally as managed by the filestore.
-	f, err := os.Create(filepath.Join(pluginPath, managedPluginFileName))
+	f, err := os.Create(filepath.Join(bundlePath, managedPluginFileName))
 	if err != nil {
 		return nil, model.NewAppError("installExtractedPlugin", "app.plugin.flag_managed.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
@@ -419,12 +452,13 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD
 		manifest = updatedManifest
 	}
 
-	mlog.Debug("Installing plugin", mlog.String("plugin_id", manifest.Id), mlog.String("version", manifest.Version))
-
 	return manifest, nil
 }
 
+// RemovePlugin removes a plugin from all servers.
 func (ch *Channels) RemovePlugin(id string) *model.AppError {
+	logger := ch.srv.Log().With(mlog.String("plugin_id", id))
+
 	// Disable plugin before removal to make sure this
 	// plugin remains disabled on re-install.
 	if err := ch.disablePlugin(id); err != nil {
@@ -436,19 +470,19 @@ func (ch *Channels) RemovePlugin(id string) *model.AppError {
 	}
 
 	// Remove bundle from the file store.
-	storePluginFileName := getBundleStorePath(id)
-	bundleExist, err := ch.srv.fileExists(storePluginFileName)
+	bundlePath := getBundleStorePath(id)
+	bundleExists, err := ch.srv.fileExists(bundlePath)
 	if err != nil {
 		return model.NewAppError("removePlugin", "app.plugin.remove_bundle.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
-	if !bundleExist {
+	if !bundleExists {
 		return nil
 	}
-	if err = ch.srv.removeFile(storePluginFileName); err != nil {
+	if err := ch.srv.removeFile(bundlePath); err != nil {
 		return model.NewAppError("removePlugin", "app.plugin.remove_bundle.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
-	if err = ch.removeSignature(id); err != nil {
-		mlog.Warn("Can't remove signature", mlog.Err(err))
+	if err := ch.removeSignature(id); err != nil {
+		logger.Warn("Can't remove signature", mlog.Err(err))
 	}
 
 	ch.notifyClusterPluginEvent(
@@ -459,12 +493,13 @@ func (ch *Channels) RemovePlugin(id string) *model.AppError {
 	)
 
 	if err := ch.notifyPluginStatusesChanged(); err != nil {
-		mlog.Warn("Failed to notify plugin status changed", mlog.Err(err))
+		logger.Warn("Failed to notify plugin status changed", mlog.Err(err))
 	}
 
 	return nil
 }
 
+// removePluginLocally removes the given plugin from the current server.
 func (ch *Channels) removePluginLocally(id string) *model.AppError {
 	pluginsEnvironment := ch.GetPluginsEnvironment()
 	if pluginsEnvironment == nil {
@@ -477,11 +512,11 @@ func (ch *Channels) removePluginLocally(id string) *model.AppError {
 	}
 
 	var manifest *model.Manifest
-	var pluginPath string
+	var unpackedBundlePath string
 	for _, p := range plugins {
 		if p.Manifest != nil && p.Manifest.Id == id {
 			manifest = p.Manifest
-			pluginPath = filepath.Dir(p.ManifestPath)
+			unpackedBundlePath = filepath.Dir(p.ManifestPath)
 			break
 		}
 	}
@@ -494,33 +529,39 @@ func (ch *Channels) removePluginLocally(id string) *model.AppError {
 	pluginsEnvironment.RemovePlugin(id)
 	ch.unregisterPluginCommands(id)
 
-	if err := os.RemoveAll(pluginPath); err != nil {
+	if err := os.RemoveAll(unpackedBundlePath); err != nil {
 		return model.NewAppError("removePlugin", "app.plugin.remove.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
 	return nil
 }
 
+// removeSignature removes the signature file installed alongside the plugin.
 func (ch *Channels) removeSignature(pluginID string) *model.AppError {
-	filePath := getSignatureStorePath(pluginID)
-	exists, err := ch.srv.fileExists(filePath)
+	logger := ch.srv.Log().With(mlog.String("plugin_id", pluginID))
+
+	signaturePath := getSignatureStorePath(pluginID)
+	exists, err := ch.srv.fileExists(signaturePath)
 	if err != nil {
 		return model.NewAppError("removeSignature", "app.plugin.remove_bundle.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 	if !exists {
-		mlog.Debug("no plugin signature to remove", mlog.String("plugin_id", pluginID))
+		logger.Debug("no plugin signature to remove")
 		return nil
 	}
-	if err = ch.srv.removeFile(filePath); err != nil {
+	if err = ch.srv.removeFile(signaturePath); err != nil {
 		return model.NewAppError("removeSignature", "app.plugin.remove_bundle.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 	return nil
 }
 
+// getBundleStorePath maps the given plugin id to the file path of the corresponding plugin bundle.
 func getBundleStorePath(id string) string {
 	return filepath.Join(fileStorePluginFolder, fmt.Sprintf("%s.tar.gz", id))
 }
 
+// getSignatureStorePath maps the given plugin id to the file path of the corresponding plugin
+// signature, if one exists.
 func getSignatureStorePath(id string) string {
 	return filepath.Join(fileStorePluginFolder, fmt.Sprintf("%s.tar.gz.sig", id))
 }
diff --git a/server/channels/app/plugin_install_test.go b/server/channels/app/plugin_install_test.go
index 0a5cf901d6..c3e0b31c91 100644
--- a/server/channels/app/plugin_install_test.go
+++ b/server/channels/app/plugin_install_test.go
@@ -167,17 +167,6 @@ func TestInstallPluginLocally(t *testing.T) {
 		assertBundleInfoManifests(t, th, []*model.Manifest{manifest})
 	})
 
-	t.Run("doesn't install if ID on block list", func(t *testing.T) {
-		th := Setup(t)
-		defer th.TearDown()
-		cleanExistingBundles(t, th)
-
-		_, appErr := installPlugin(t, th, "com.mattermost.plugin-incident-response", "0.0.1", installPluginLocallyAlways)
-		require.NotNil(t, appErr)
-		require.Equal(t, "app.plugin.blocked.app_error", appErr.Id)
-		assertBundleInfoManifests(t, th, []*model.Manifest{})
-	})
-
 	t.Run("different plugin already installed", func(t *testing.T) {
 		th := Setup(t)
 		defer th.TearDown()
diff --git a/server/i18n/en.json b/server/i18n/en.json
index afcf77a824..3aeec89829 100644
--- a/server/i18n/en.json
+++ b/server/i18n/en.json
@@ -6023,10 +6023,6 @@
     "id": "app.oauth.update_app.updating.app_error",
     "translation": "We encountered an error updating the app."
   },
-  {
-    "id": "app.plugin.blocked.app_error",
-    "translation": "Plugin {{.Id}} is on the block list. Some plugins are blocked because they are built into this version of Mattermost."
-  },
   {
     "id": "app.plugin.cluster.save_config.app_error",
     "translation": "The plugin configuration in your config.json file must be updated manually when using ReadOnlyConfig with clustering enabled."
diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go
index 442f47de24..a820acd289 100644
--- a/server/public/model/feature_flags.go
+++ b/server/public/model/feature_flags.go
@@ -22,12 +22,6 @@ type FeatureFlags struct {
 	// AppsEnabled toggles the Apps framework functionalities both in server and client side
 	AppsEnabled bool
 
-	// Feature flags to control plugin versions
-	PluginPlaybooks  string `plugin_id:"playbooks"`
-	PluginApps       string `plugin_id:"com.mattermost.apps"`
-	PluginFocalboard string `plugin_id:"focalboard"`
-	PluginCalls      string `plugin_id:"com.mattermost.calls"`
-
 	PermalinkPreviews bool
 
 	// CallsEnabled controls whether or not the Calls plugin should be enabled
@@ -64,8 +58,6 @@ func (f *FeatureFlags) SetDefaults() {
 	f.TestBoolFeature = false
 	f.EnableRemoteClusterService = false
 	f.AppsEnabled = true
-	f.PluginApps = ""
-	f.PluginFocalboard = ""
 	f.BoardsDataRetention = false
 	f.NormalizeLdapDNs = false
 	f.GraphQL = false
@@ -79,26 +71,6 @@ func (f *FeatureFlags) SetDefaults() {
 	f.DataRetentionConcurrencyEnabled = true
 }
 
-func (f *FeatureFlags) Plugins() map[string]string {
-	rFFVal := reflect.ValueOf(f).Elem()
-	rFFType := reflect.TypeOf(f).Elem()
-
-	pluginVersions := make(map[string]string)
-	for i := 0; i < rFFVal.NumField(); i++ {
-		rFieldVal := rFFVal.Field(i)
-		rFieldType := rFFType.Field(i)
-
-		pluginId, hasPluginId := rFieldType.Tag.Lookup("plugin_id")
-		if !hasPluginId {
-			continue
-		}
-
-		pluginVersions[pluginId] = rFieldVal.String()
-	}
-
-	return pluginVersions
-}
-
 // ToMap returns the feature flags as a map[string]string
 // Supports boolean and string feature flags.
 func (f *FeatureFlags) ToMap() map[string]string {
diff --git a/server/public/plugin/environment.go b/server/public/plugin/environment.go
index cb9e70316f..32fd819a55 100644
--- a/server/public/plugin/environment.go
+++ b/server/public/plugin/environment.go
@@ -103,34 +103,9 @@ func scanSearchPath(path string) ([]*model.BundleInfo, error) {
 	return ret, nil
 }
 
-var pluginIDBlocklist = map[string]bool{
-	"com.mattermost.plugin-incident-response":   true,
-	"com.mattermost.plugin-incident-management": true,
-}
-
-func PluginIDIsBlocked(id string) bool {
-	_, ok := pluginIDBlocklist[id]
-	return ok
-}
-
 // Returns a list of all plugins within the environment.
 func (env *Environment) Available() ([]*model.BundleInfo, error) {
-	rawList, err := scanSearchPath(env.pluginDir)
-	if err != nil {
-		return nil, err
-	}
-
-	// Filter any plugins that match the blocklist
-	filteredList := make([]*model.BundleInfo, 0, len(rawList))
-	for _, bundleInfo := range rawList {
-		if PluginIDIsBlocked(bundleInfo.Manifest.Id) {
-			env.logger.Debug("Plugin ignored by blocklist", mlog.String("plugin_id", bundleInfo.Manifest.Id))
-		} else {
-			filteredList = append(filteredList, bundleInfo)
-		}
-	}
-
-	return filteredList, nil
+	return scanSearchPath(env.pluginDir)
 }
 
 // Returns a list of prepackaged plugins available in the local prepackaged_plugins folder.