From c46d8ec892d40c50d9a235ac455e082502284c54 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 20 Nov 2018 08:52:51 -0500 Subject: [PATCH] MM-12974: guard app plugins with mutex (#9818) * guard app plugins with mutex Shutting down the app could race with a goroutine that uses the plugins environment, since we shut down the plugins first before cleaning up goroutines. * fix go vet issues --- app/channel.go | 21 ++++----- app/diagnostics.go | 99 +++++++++++++++++++++------------------- app/file.go | 4 +- app/login.go | 6 +-- app/plugin.go | 98 ++++++++++++++++++++++++++------------- app/plugin_api_test.go | 10 ++-- app/plugin_commands.go | 12 +++-- app/plugin_hooks_test.go | 2 +- app/plugin_install.go | 14 +++--- app/plugin_requests.go | 5 +- app/plugin_statuses.go | 10 ++-- app/plugin_test.go | 1 + app/post.go | 16 +++---- app/server.go | 3 +- app/team.go | 8 ++-- 15 files changed, 178 insertions(+), 131 deletions(-) diff --git a/app/channel.go b/app/channel.go index 99265f3408..6e4f1b103a 100644 --- a/app/channel.go +++ b/app/channel.go @@ -211,10 +211,10 @@ func (a *App) CreateChannel(channel *model.Channel, addMember bool) (*model.Chan a.InvalidateCacheForUser(channel.CreatorId) } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.ChannelHasBeenCreated(pluginContext, sc) return true }, plugin.ChannelHasBeenCreatedId) @@ -238,10 +238,10 @@ func (a *App) CreateDirectChannel(userId string, otherUserId string) (*model.Cha a.InvalidateCacheForUser(userId) a.InvalidateCacheForUser(otherUserId) - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.ChannelHasBeenCreated(pluginContext, channel) return true }, plugin.ChannelHasBeenCreatedId) @@ -857,10 +857,10 @@ func (a *App) AddChannelMember(userId string, channel *model.Channel, userReques return nil, err } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.UserHasJoinedChannel(pluginContext, cm, userRequestor) return true }, plugin.UserHasJoinedChannelId) @@ -1247,10 +1247,10 @@ func (a *App) JoinChannel(channel *model.Channel, userId string) *model.AppError return err } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.UserHasJoinedChannel(pluginContext, cm, nil) return true }, plugin.UserHasJoinedChannelId) @@ -1448,8 +1448,7 @@ func (a *App) removeUserFromChannel(userIdToRemove string, removerUserId string, a.InvalidateCacheForUser(userIdToRemove) a.InvalidateCacheForChannelMembers(channel.Id) - if a.PluginsReady() { - + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var actorUser *model.User if removerUserId != "" { actorUser, _ = a.GetUser(removerUserId) @@ -1457,7 +1456,7 @@ func (a *App) removeUserFromChannel(userIdToRemove string, removerUserId string, a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.UserHasLeftChannel(pluginContext, cm, actorUser) return true }, plugin.UserHasLeftChannelId) diff --git a/app/diagnostics.go b/app/diagnostics.go index c706ce4bf2..d75ea5b99b 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -580,62 +580,65 @@ func (a *App) trackLicense() { } func (a *App) trackPlugins() { - if a.PluginsReady() { - totalEnabledCount := 0 - webappEnabledCount := 0 - backendEnabledCount := 0 - totalDisabledCount := 0 - webappDisabledCount := 0 - backendDisabledCount := 0 - brokenManifestCount := 0 - settingsCount := 0 + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { + return + } - pluginStates := a.Config().PluginSettings.PluginStates - plugins, _ := a.Srv.Plugins.Available() + totalEnabledCount := 0 + webappEnabledCount := 0 + backendEnabledCount := 0 + totalDisabledCount := 0 + webappDisabledCount := 0 + backendDisabledCount := 0 + brokenManifestCount := 0 + settingsCount := 0 - if pluginStates != nil && plugins != nil { - for _, plugin := range plugins { - if plugin.Manifest == nil { - brokenManifestCount += 1 - continue + pluginStates := a.Config().PluginSettings.PluginStates + plugins, _ := pluginsEnvironment.Available() + + if pluginStates != nil && plugins != nil { + for _, plugin := range plugins { + if plugin.Manifest == nil { + brokenManifestCount += 1 + continue + } + if state, ok := pluginStates[plugin.Manifest.Id]; ok && state.Enable { + totalEnabledCount += 1 + if plugin.Manifest.HasServer() { + backendEnabledCount += 1 } - if state, ok := pluginStates[plugin.Manifest.Id]; ok && state.Enable { - totalEnabledCount += 1 - if plugin.Manifest.HasServer() { - backendEnabledCount += 1 - } - if plugin.Manifest.HasWebapp() { - webappEnabledCount += 1 - } - } else { - totalDisabledCount += 1 - if plugin.Manifest.HasServer() { - backendDisabledCount += 1 - } - if plugin.Manifest.HasWebapp() { - webappDisabledCount += 1 - } + if plugin.Manifest.HasWebapp() { + webappEnabledCount += 1 } - if plugin.Manifest.SettingsSchema != nil { - settingsCount += 1 + } else { + totalDisabledCount += 1 + if plugin.Manifest.HasServer() { + backendDisabledCount += 1 + } + if plugin.Manifest.HasWebapp() { + webappDisabledCount += 1 } } - } else { - totalEnabledCount = -1 // -1 to indicate disabled or error - totalDisabledCount = -1 // -1 to indicate disabled or error + if plugin.Manifest.SettingsSchema != nil { + settingsCount += 1 + } } - - a.SendDiagnostic(TRACK_PLUGINS, map[string]interface{}{ - "enabled_plugins": totalEnabledCount, - "enabled_webapp_plugins": webappEnabledCount, - "enabled_backend_plugins": backendEnabledCount, - "disabled_plugins": totalDisabledCount, - "disabled_webapp_plugins": webappDisabledCount, - "disabled_backend_plugins": backendDisabledCount, - "plugins_with_settings": settingsCount, - "plugins_with_broken_manifests": brokenManifestCount, - }) + } else { + totalEnabledCount = -1 // -1 to indicate disabled or error + totalDisabledCount = -1 // -1 to indicate disabled or error } + + a.SendDiagnostic(TRACK_PLUGINS, map[string]interface{}{ + "enabled_plugins": totalEnabledCount, + "enabled_webapp_plugins": webappEnabledCount, + "enabled_backend_plugins": backendEnabledCount, + "disabled_plugins": totalDisabledCount, + "disabled_webapp_plugins": webappDisabledCount, + "disabled_backend_plugins": backendDisabledCount, + "plugins_with_settings": settingsCount, + "plugins_with_broken_manifests": brokenManifestCount, + }) } func (a *App) trackServer() { diff --git a/app/file.go b/app/file.go index 410e3ae9b3..dccc177932 100644 --- a/app/file.go +++ b/app/file.go @@ -460,10 +460,10 @@ func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, ra info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg" } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var rejectionError *model.AppError pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { var newBytes bytes.Buffer replacementInfo, rejectionReason := hooks.FileWillBeUploaded(pluginContext, info, bytes.NewReader(data), &newBytes) if rejectionReason != "" { diff --git a/app/login.go b/app/login.go index 28f207f75f..5e97f7f586 100644 --- a/app/login.go +++ b/app/login.go @@ -66,10 +66,10 @@ func (a *App) AuthenticateUserForLogin(id, loginId, password, mfaToken string, l return nil, err } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var rejectionReason string pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { rejectionReason = hooks.UserWillLogIn(pluginContext, user) return rejectionReason == "" }, plugin.UserWillLogInId) @@ -80,7 +80,7 @@ func (a *App) AuthenticateUserForLogin(id, loginId, password, mfaToken string, l a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.UserHasLoggedIn(pluginContext, user) return true }, plugin.UserHasLoggedInId) diff --git a/app/plugin.go b/app/plugin.go index 2c1eb3862a..962d153412 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -15,22 +15,49 @@ import ( "github.com/mattermost/mattermost-server/utils" ) +// GetPluginsEnvironment returns the plugin environment for use if plugins are enabled and +// initialized. +// +// To get the plugins environment when the plugins are disabled, manually acquire the plugins +// lock instead. +func (a *App) GetPluginsEnvironment() *plugin.Environment { + if !*a.Config().PluginSettings.Enable { + return nil + } + + a.Srv.PluginsLock.RLock() + defer a.Srv.PluginsLock.RUnlock() + + return a.Srv.PluginsEnvironment +} + +func (a *App) SetPluginsEnvironment(pluginsEnvironment *plugin.Environment) { + a.Srv.PluginsLock.Lock() + defer a.Srv.PluginsLock.Unlock() + + a.Srv.PluginsEnvironment = pluginsEnvironment +} + func (a *App) SyncPluginsActiveState() { - if a.Srv.Plugins == nil { + a.Srv.PluginsLock.RLock() + pluginsEnvironment := a.Srv.PluginsEnvironment + a.Srv.PluginsLock.RUnlock() + + if pluginsEnvironment == nil { return } config := a.Config().PluginSettings if *config.Enable { - availablePlugins, err := a.Srv.Plugins.Available() + availablePlugins, err := pluginsEnvironment.Available() if err != nil { a.Log.Error("Unable to get available plugins", mlog.Err(err)) return } // Deactivate any plugins that have been disabled. - for _, plugin := range a.Srv.Plugins.Active() { + for _, plugin := range pluginsEnvironment.Active() { // Determine if plugin is enabled pluginId := plugin.Manifest.Id pluginEnabled := false @@ -40,7 +67,7 @@ func (a *App) SyncPluginsActiveState() { // If it's not enabled we need to deactivate it if !pluginEnabled { - deactivated := a.Srv.Plugins.Deactivate(pluginId) + deactivated := pluginsEnvironment.Deactivate(pluginId) if deactivated && plugin.Manifest.HasClient() { message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_DISABLED, "", "", "", nil) message.Add("manifest", plugin.Manifest.ClientManifest()) @@ -65,7 +92,7 @@ func (a *App) SyncPluginsActiveState() { // Activate plugin if enabled if pluginEnabled { - updatedManifest, activated, err := a.Srv.Plugins.Activate(pluginId) + updatedManifest, activated, err := pluginsEnvironment.Activate(pluginId) if err != nil { plugin.WrapLogger(a.Log).Error("Unable to activate plugin", mlog.Err(err)) continue @@ -79,7 +106,7 @@ func (a *App) SyncPluginsActiveState() { } } } else { // If plugins are disabled, shutdown plugins. - a.Srv.Plugins.Shutdown() + pluginsEnvironment.Shutdown() } if err := a.notifyPluginStatusesChanged(); err != nil { @@ -92,7 +119,10 @@ func (a *App) NewPluginAPI(manifest *model.Manifest) plugin.API { } func (a *App) InitPlugins(pluginDir, webappPluginDir string) { - if a.Srv.Plugins != nil || !*a.Config().PluginSettings.Enable { + a.Srv.PluginsLock.RLock() + pluginsEnvironment := a.Srv.PluginsEnvironment + a.Srv.PluginsLock.RUnlock() + if pluginsEnvironment != nil || !*a.Config().PluginSettings.Enable { a.SyncPluginsActiveState() return } @@ -109,12 +139,12 @@ func (a *App) InitPlugins(pluginDir, webappPluginDir string) { return } - if env, err := plugin.NewEnvironment(a.NewPluginAPI, pluginDir, webappPluginDir, a.Log); err != nil { + env, err := plugin.NewEnvironment(a.NewPluginAPI, pluginDir, webappPluginDir, a.Log) + if err != nil { mlog.Error("Failed to start up plugins", mlog.Err(err)) return - } else { - a.Srv.Plugins = env } + a.SetPluginsEnvironment(env) prepackagedPluginsDir, found := utils.FindDir("prepackaged_plugins") if found { @@ -136,39 +166,46 @@ func (a *App) InitPlugins(pluginDir, webappPluginDir string) { } // Sync plugin active state when config changes. Also notify plugins. + a.Srv.PluginsLock.Lock() a.RemoveConfigListener(a.Srv.PluginConfigListenerId) a.Srv.PluginConfigListenerId = a.AddConfigListener(func(*model.Config, *model.Config) { a.SyncPluginsActiveState() - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - hooks.OnConfigurationChange() - return true - }, plugin.OnConfigurationChangeId) + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + hooks.OnConfigurationChange() + return true + }, plugin.OnConfigurationChangeId) + } }) + a.Srv.PluginsLock.Unlock() a.SyncPluginsActiveState() - } func (a *App) ShutDownPlugins() { - if a.Srv.Plugins == nil { + a.Srv.PluginsLock.Lock() + pluginsEnvironment := a.Srv.PluginsEnvironment + defer a.Srv.PluginsLock.Unlock() + if pluginsEnvironment == nil { return } mlog.Info("Shutting down plugins") - a.Srv.Plugins.Shutdown() + pluginsEnvironment.Shutdown() a.RemoveConfigListener(a.Srv.PluginConfigListenerId) a.Srv.PluginConfigListenerId = "" - a.Srv.Plugins = nil + a.Srv.PluginsEnvironment = nil } func (a *App) GetActivePluginManifests() ([]*model.Manifest, *model.AppError) { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return nil, model.NewAppError("GetActivePluginManifests", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - plugins := a.Srv.Plugins.Active() + plugins := pluginsEnvironment.Active() manifests := make([]*model.Manifest, len(plugins)) for i, plugin := range plugins { @@ -181,11 +218,12 @@ func (a *App) GetActivePluginManifests() ([]*model.Manifest, *model.AppError) { // EnablePlugin will set the config for an installed plugin to enabled, triggering asynchronous // activation if inactive anywhere in the cluster. func (a *App) EnablePlugin(id string) *model.AppError { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return model.NewAppError("EnablePlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - plugins, err := a.Srv.Plugins.Available() + plugins, err := pluginsEnvironment.Available() if err != nil { return model.NewAppError("EnablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -221,11 +259,12 @@ func (a *App) EnablePlugin(id string) *model.AppError { // DisablePlugin will set the config for an installed plugin to disabled, triggering deactivation if active. func (a *App) DisablePlugin(id string) *model.AppError { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return model.NewAppError("DisablePlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - plugins, err := a.Srv.Plugins.Available() + plugins, err := pluginsEnvironment.Available() if err != nil { return model.NewAppError("DisablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -255,16 +294,13 @@ func (a *App) DisablePlugin(id string) *model.AppError { return nil } -func (a *App) PluginsReady() bool { - return a.Srv.Plugins != nil && *a.Config().PluginSettings.Enable -} - func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) { - if !a.PluginsReady() { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return nil, model.NewAppError("GetPlugins", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - availablePlugins, err := a.Srv.Plugins.Available() + availablePlugins, err := pluginsEnvironment.Available() if err != nil { return nil, model.NewAppError("GetPlugins", "app.plugin.get_plugins.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -278,7 +314,7 @@ func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) { Manifest: *plugin.Manifest, } - if a.Srv.Plugins.IsActive(plugin.Manifest.Id) { + if pluginsEnvironment.IsActive(plugin.Manifest.Id) { resp.Active = append(resp.Active, info) } else { resp.Inactive = append(resp.Inactive, info) diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index 65cca0abf8..8fc2e1dfd9 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -41,7 +41,7 @@ func setupPluginApiTest(t *testing.T, pluginCode string, pluginManifest string, require.NotNil(t, manifest) require.True(t, activated) - app.Srv.Plugins = env + app.SetPluginsEnvironment(env) } func TestPluginAPIUpdateUserStatus(t *testing.T) { @@ -206,7 +206,7 @@ func TestPluginAPILoadPluginConfiguration(t *testing.T) { } ] }}`, "testloadpluginconfig", th.App) - hooks, err := th.App.Srv.Plugins.HooksForPlugin("testloadpluginconfig") + hooks, err := th.App.GetPluginsEnvironment().HooksForPlugin("testloadpluginconfig") assert.NoError(t, err) _, ret := hooks.MessageWillBePosted(nil, nil) assert.Equal(t, "str32true", ret) @@ -280,7 +280,7 @@ func TestPluginAPILoadPluginConfigurationDefaults(t *testing.T) { } ] }}`, "testloadpluginconfig", th.App) - hooks, err := th.App.Srv.Plugins.HooksForPlugin("testloadpluginconfig") + hooks, err := th.App.GetPluginsEnvironment().HooksForPlugin("testloadpluginconfig") assert.NoError(t, err) _, ret := hooks.MessageWillBePosted(nil, nil) assert.Equal(t, "override35true", ret) @@ -377,9 +377,9 @@ func TestPluginAPIGetPlugins(t *testing.T) { require.True(t, activated) pluginManifests = append(pluginManifests, manifest) } - th.App.Srv.Plugins = env + th.App.SetPluginsEnvironment(env) - // Decative the last one for testing + // Decativate the last one for testing sucess := env.Deactivate(pluginIDs[len(pluginIDs)-1]) require.True(t, sucess) diff --git a/app/plugin_commands.go b/app/plugin_commands.go index cbaeb507c9..911f54cec1 100644 --- a/app/plugin_commands.go +++ b/app/plugin_commands.go @@ -101,12 +101,14 @@ func (a *App) ExecutePluginCommand(args *model.CommandArgs) (*model.Command, *mo for _, pc := range a.Srv.pluginCommands { if (pc.Command.TeamId == "" || pc.Command.TeamId == args.TeamId) && pc.Command.Trigger == trigger { - pluginHooks, err := a.Srv.Plugins.HooksForPlugin(pc.PluginId) - if err != nil { - return pc.Command, nil, model.NewAppError("ExecutePluginCommand", "model.plugin_command.error.app_error", nil, "err="+err.Error(), http.StatusInternalServerError) + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { + pluginHooks, err := pluginsEnvironment.HooksForPlugin(pc.PluginId) + if err != nil { + return pc.Command, nil, model.NewAppError("ExecutePluginCommand", "model.plugin_command.error.app_error", nil, "err="+err.Error(), http.StatusInternalServerError) + } + response, appErr := pluginHooks.ExecuteCommand(&plugin.Context{}, args) + return pc.Command, response, appErr } - response, appErr := pluginHooks.ExecuteCommand(&plugin.Context{}, args) - return pc.Command, response, appErr } } return nil, nil, nil diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go index 9016b84e46..ca3ff46cec 100644 --- a/app/plugin_hooks_test.go +++ b/app/plugin_hooks_test.go @@ -44,7 +44,7 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a env, err := plugin.NewEnvironment(apiFunc, pluginDir, webappPluginDir, app.Log) require.NoError(t, err) - app.Srv.Plugins = env + app.SetPluginsEnvironment(env) pluginIds := []string{} activationErrors := []error{} for _, code := range pluginCode { diff --git a/app/plugin_install.go b/app/plugin_install.go index aaa6e46a53..8fbd0bc623 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -22,7 +22,8 @@ func (a *App) InstallPlugin(pluginFile io.Reader, replace bool) (*model.Manifest } func (a *App) installPlugin(pluginFile io.Reader, replace bool) (*model.Manifest, *model.AppError) { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return nil, model.NewAppError("installPlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } @@ -55,7 +56,7 @@ func (a *App) installPlugin(pluginFile io.Reader, replace bool) (*model.Manifest return nil, model.NewAppError("installPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidIdRegex}, "", http.StatusBadRequest) } - bundles, err := a.Srv.Plugins.Available() + bundles, err := pluginsEnvironment.Available() if err != nil { return nil, model.NewAppError("installPlugin", "app.plugin.install.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -91,11 +92,12 @@ func (a *App) RemovePlugin(id string) *model.AppError { } func (a *App) removePlugin(id string) *model.AppError { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return model.NewAppError("removePlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - plugins, err := a.Srv.Plugins.Available() + plugins, err := pluginsEnvironment.Available() if err != nil { return model.NewAppError("removePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest) } @@ -114,13 +116,13 @@ func (a *App) removePlugin(id string) *model.AppError { return model.NewAppError("removePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest) } - if a.Srv.Plugins.IsActive(id) && manifest.HasClient() { + if pluginsEnvironment.IsActive(id) && manifest.HasClient() { message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_DISABLED, "", "", "", nil) message.Add("manifest", manifest.ClientManifest()) a.Publish(message) } - a.Srv.Plugins.Deactivate(id) + pluginsEnvironment.Deactivate(id) a.UnregisterPluginCommands(id) err = os.RemoveAll(pluginPath) diff --git a/app/plugin_requests.go b/app/plugin_requests.go index 4070a060f4..e3ebc60421 100644 --- a/app/plugin_requests.go +++ b/app/plugin_requests.go @@ -19,7 +19,8 @@ import ( ) func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { err := model.NewAppError("ServePluginRequest", "app.plugin.disabled.app_error", nil, "Enable plugins to serve plugin requests", http.StatusNotImplemented) a.Log.Error(err.Error()) w.WriteHeader(err.StatusCode) @@ -29,7 +30,7 @@ func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) { } params := mux.Vars(r) - hooks, err := a.Srv.Plugins.HooksForPlugin(params["plugin_id"]) + hooks, err := pluginsEnvironment.HooksForPlugin(params["plugin_id"]) if err != nil { a.Log.Error("Access to route for non-existent plugin", mlog.String("missing_plugin_id", params["plugin_id"]), mlog.Err(err)) http.NotFound(w, r) diff --git a/app/plugin_statuses.go b/app/plugin_statuses.go index fc5561bcfc..fe397dd846 100644 --- a/app/plugin_statuses.go +++ b/app/plugin_statuses.go @@ -11,11 +11,12 @@ import ( // GetPluginStatus returns the status for a plugin installed on this server. func (a *App) GetPluginStatus(id string) (*model.PluginStatus, *model.AppError) { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return nil, model.NewAppError("GetPluginStatus", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - pluginStatuses, err := a.Srv.Plugins.Statuses() + pluginStatuses, err := pluginsEnvironment.Statuses() if err != nil { return nil, model.NewAppError("GetPluginStatus", "app.plugin.get_statuses.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -32,11 +33,12 @@ func (a *App) GetPluginStatus(id string) (*model.PluginStatus, *model.AppError) // GetPluginStatuses returns the status for plugins installed on this server. func (a *App) GetPluginStatuses() (model.PluginStatuses, *model.AppError) { - if a.Srv.Plugins == nil || !*a.Config().PluginSettings.Enable { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { return nil, model.NewAppError("GetPluginStatuses", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } - pluginStatuses, err := a.Srv.Plugins.Statuses() + pluginStatuses, err := pluginsEnvironment.Statuses() if err != nil { return nil, model.NewAppError("GetPluginStatuses", "app.plugin.get_statuses.app_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/app/plugin_test.go b/app/plugin_test.go index 8dcae9b16d..0221a763e4 100644 --- a/app/plugin_test.go +++ b/app/plugin_test.go @@ -198,6 +198,7 @@ func TestGetPluginStatusesDisabled(t *testing.T) { }) _, err := th.App.GetPluginStatuses() + require.NotNil(t, err) require.EqualError(t, err, "GetPluginStatuses: Plugins have been disabled. Please check your logs for details., ") } diff --git a/app/post.go b/app/post.go index c4909a9d42..882c3edb65 100644 --- a/app/post.go +++ b/app/post.go @@ -144,10 +144,10 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo return nil, err } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var rejectionError *model.AppError pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post) if rejectionReason != "" { rejectionError = model.NewAppError("createPost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) @@ -170,10 +170,10 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo } rpost := result.Data.(*model.Post) - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.MessageHasBeenPosted(pluginContext, rpost) return true }, plugin.MessageHasBeenPostedId) @@ -356,10 +356,10 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model return nil, err } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var rejectionReason string pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost, oldPost) return post != nil }, plugin.MessageWillBeUpdatedId) @@ -374,10 +374,10 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model } rpost := result.Data.(*model.Post) - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.MessageHasBeenUpdated(pluginContext, newPost, oldPost) return true }, plugin.MessageHasBeenUpdatedId) diff --git a/app/server.go b/app/server.go index 53e5e039da..ed91b92f2a 100644 --- a/app/server.go +++ b/app/server.go @@ -54,8 +54,9 @@ type Server struct { goroutineCount int32 goroutineExitSignal chan struct{} - Plugins *plugin.Environment + PluginsEnvironment *plugin.Environment PluginConfigListenerId string + PluginsLock sync.RWMutex EmailBatching *EmailBatchingJob EmailRateLimiter *throttled.GCRARateLimiter diff --git a/app/team.go b/app/team.go index 0152cbb6af..f8a74a78f6 100644 --- a/app/team.go +++ b/app/team.go @@ -461,7 +461,7 @@ func (a *App) JoinUserToTeam(team *model.Team, user *model.User, userRequestorId return nil } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var actor *model.User if userRequestorId != "" { actor, _ = a.GetUser(userRequestorId) @@ -469,7 +469,7 @@ func (a *App) JoinUserToTeam(team *model.Team, user *model.User, userRequestorId a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.UserHasJoinedTeam(pluginContext, tm, actor) return true }, plugin.UserHasJoinedTeamId) @@ -784,7 +784,7 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string) return result.Err } - if a.PluginsReady() { + if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var actor *model.User if requestorId != "" { actor, _ = a.GetUser(requestorId) @@ -792,7 +792,7 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string) a.Srv.Go(func() { pluginContext := &plugin.Context{} - a.Srv.Plugins.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { hooks.UserHasLeftTeam(pluginContext, teamMember, actor) return true }, plugin.UserHasLeftTeamId)