From 6cb239f3ca57bbf11d23d91fa8eab9884af5cdfa Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Tue, 10 May 2022 19:29:48 +0200 Subject: [PATCH] [MM-44056] Make Calls plugin's state on Cloud controllable by feature flag (#20161) * Make Calls plugin's state on Cloud controllable by feature flag * Prepackage Calls v0.4.9 (#20148) * Update app/plugin.go Co-authored-by: Jesse Hallam * Update app/plugin_install.go Co-authored-by: Jesse Hallam * Bump Calls version to latest Co-authored-by: Jesse Hallam Co-authored-by: Mattermod (cherry picked from commit aee68c7ae49ac0457e249397448928f72c09386a) --- Makefile | 1 + app/plugin.go | 23 ++++++++++--- app/plugin_install.go | 3 +- app/plugin_test.go | 75 ++++++++++++++++++++++++++++++++++++++++++ build/release.mk | 8 +++++ model/config.go | 5 +++ model/config_test.go | 36 ++++++++++++++++++++ model/feature_flags.go | 5 +++ model/utils.go | 5 +++ 9 files changed, 155 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index a63ca49c74069..a18e9c29d0da2 100644 --- a/Makefile +++ b/Makefile @@ -119,6 +119,7 @@ TEMPLATES_DIR=templates PLUGIN_PACKAGES ?= mattermost-plugin-antivirus-v0.1.2 PLUGIN_PACKAGES += mattermost-plugin-autolink-v1.2.2 PLUGIN_PACKAGES += mattermost-plugin-aws-SNS-v1.2.0 +PLUGIN_PACKAGES += mattermost-plugin-calls-v0.5.1 PLUGIN_PACKAGES += mattermost-plugin-channel-export-v1.0.0 PLUGIN_PACKAGES += mattermost-plugin-custom-attributes-v1.3.0 PLUGIN_PACKAGES += mattermost-plugin-github-v2.0.1 diff --git a/app/plugin.go b/app/plugin.go index 579c83497ba1f..be30cb4ed7e2a 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -98,11 +98,8 @@ func (ch *Channels) syncPluginsActiveState() { pluginEnabled = state.Enable } - // Tie Apps proxy disabled status to the feature flag. - if pluginID == "com.mattermost.apps" { - if !ch.cfgSvc.Config().FeatureFlags.AppsEnabled { - pluginEnabled = false - } + if hasOverride, value := ch.getPluginStateOverride(pluginID); hasOverride { + pluginEnabled = value } if pluginEnabled { @@ -1064,3 +1061,19 @@ func getIcon(iconPath string) (string, error) { return fmt.Sprintf("data:image/svg+xml;base64,%s", base64.StdEncoding.EncodeToString(icon)), nil } + +func (ch *Channels) getPluginStateOverride(pluginID string) (bool, bool) { + switch pluginID { + case "com.mattermost.apps": + // Tie Apps proxy disabled status to the feature flag. + if !ch.cfgSvc.Config().FeatureFlags.AppsEnabled { + return true, false + } + case "com.mattermost.calls": + if model.IsCloud() { + return true, ch.cfgSvc.Config().FeatureFlags.CallsEnabled + } + } + + return false, false +} diff --git a/app/plugin_install.go b/app/plugin_install.go index 35e4154654cec..448c1505a2b57 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -393,9 +393,10 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD // Activate the plugin if enabled. pluginState := ch.cfgSvc.Config().PluginSettings.PluginStates[manifest.Id] if pluginState != nil && pluginState.Enable { - if manifest.Id == "com.mattermost.apps" && !ch.cfgSvc.Config().FeatureFlags.AppsEnabled { + if hasOverride, enabled := ch.getPluginStateOverride(manifest.Id); hasOverride && !enabled { return manifest, nil } + updatedManifest, _, err := pluginsEnvironment.Activate(manifest.Id) if err != nil { return nil, model.NewAppError("installExtractedPlugin", "app.plugin.restart.app_error", nil, err.Error(), http.StatusInternalServerError) diff --git a/app/plugin_test.go b/app/plugin_test.go index f060b473792a6..887538016d676 100644 --- a/app/plugin_test.go +++ b/app/plugin_test.go @@ -965,3 +965,78 @@ func TestProcessPrepackagedPlugins(t *testing.T) { require.Len(t, pluginStatus, 0) }) } + +func TestGetPluginStateOverride(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + t.Run("no override", func(t *testing.T) { + overrides, value := th.App.ch.getPluginStateOverride("focalboard") + require.False(t, overrides) + require.False(t, value) + }) + + t.Run("calls override", func(t *testing.T) { + t.Run("on-prem", func(t *testing.T) { + overrides, value := th.App.ch.getPluginStateOverride("com.mattermost.calls") + require.False(t, overrides) + require.False(t, value) + }) + + t.Run("Cloud, without enabled flag", func(t *testing.T) { + os.Setenv("MM_CLOUD_INSTALLATION_ID", "test") + defer os.Unsetenv("MM_CLOUD_INSTALLATION_ID") + overrides, value := th.App.ch.getPluginStateOverride("com.mattermost.calls") + require.True(t, overrides) + require.False(t, value) + }) + + t.Run("Cloud, with enabled flag set to true", func(t *testing.T) { + os.Setenv("MM_CLOUD_INSTALLATION_ID", "test") + defer os.Unsetenv("MM_CLOUD_INSTALLATION_ID") + os.Setenv("MM_FEATUREFLAGS_CALLSENABLED", "true") + defer os.Unsetenv("MM_FEATUREFLAGS_CALLSENABLED") + + th2 := Setup(t) + defer th2.TearDown() + + overrides, value := th2.App.ch.getPluginStateOverride("com.mattermost.calls") + require.True(t, overrides) + require.True(t, value) + }) + + t.Run("Cloud, with enabled flag set to false", func(t *testing.T) { + os.Setenv("MM_CLOUD_INSTALLATION_ID", "test") + defer os.Unsetenv("MM_CLOUD_INSTALLATION_ID") + os.Setenv("MM_FEATUREFLAGS_CALLSENABLED", "false") + defer os.Unsetenv("MM_FEATUREFLAGS_CALLSENABLED") + + th2 := Setup(t) + defer th2.TearDown() + + overrides, value := th2.App.ch.getPluginStateOverride("com.mattermost.calls") + require.True(t, overrides) + require.False(t, value) + }) + }) + + t.Run("apps override", func(t *testing.T) { + t.Run("without enabled flag", func(t *testing.T) { + overrides, value := th.App.ch.getPluginStateOverride("com.mattermost.apps") + require.False(t, overrides) + require.False(t, value) + }) + + t.Run("with enabled flag set to false", func(t *testing.T) { + os.Setenv("MM_FEATUREFLAGS_APPSENABLED", "false") + defer os.Unsetenv("MM_FEATUREFLAGS_APPSENABLED") + + th2 := Setup(t) + defer th2.TearDown() + + overrides, value := th2.App.ch.getPluginStateOverride("com.mattermost.apps") + require.True(t, overrides) + require.False(t, value) + }) + }) +} diff --git a/build/release.mk b/build/release.mk index faa027239403d..5f10e4f4e8ffc 100644 --- a/build/release.mk +++ b/build/release.mk @@ -170,6 +170,11 @@ else @# Prepackage plugins @for plugin_package in $(PLUGIN_PACKAGES) ; do \ ARCH=$(PLUGIN_ARCH); \ + if [ "$$ARCH" != "linux-amd64" ]; then \ + case $$plugin_package in \ + "mattermost-plugin-calls"*) continue ;; \ + esac; \ + fi; \ cp tmpprepackaged/$$plugin_package-$$ARCH.tar.gz $(DIST_PATH_GENERIC)/prepackaged_plugins; \ cp tmpprepackaged/$$plugin_package-$$ARCH.tar.gz.sig $(DIST_PATH_GENERIC)/prepackaged_plugins; \ HAS_ARCH=`tar -tf $(DIST_PATH_GENERIC)/prepackaged_plugins/$$plugin_package-$$ARCH.tar.gz | grep -oE "dist/plugin-.*"`; \ @@ -234,6 +239,9 @@ endif @# Prepackage plugins @for plugin_package in $(PLUGIN_PACKAGES) ; do \ ARCH="windows-amd64"; \ + case $$plugin_package in \ + "mattermost-plugin-calls"*) continue ;; \ + esac; \ cp tmpprepackaged/$$plugin_package-$$ARCH.tar.gz $(DIST_PATH_WIN)/prepackaged_plugins; \ cp tmpprepackaged/$$plugin_package-$$ARCH.tar.gz.sig $(DIST_PATH_WIN)/prepackaged_plugins; \ HAS_ARCH=`tar -tf $(DIST_PATH_WIN)/prepackaged_plugins/$$plugin_package-$$ARCH.tar.gz | grep -oE "dist/plugin-.*"`; \ diff --git a/model/config.go b/model/config.go index 5972c028a8bc5..c3c6a5b737b29 100644 --- a/model/config.go +++ b/model/config.go @@ -2810,6 +2810,11 @@ func (s *PluginSettings) SetDefaults(ls LogSettings) { s.PluginStates["com.mattermost.apps"] = &PluginState{Enable: true} } + if s.PluginStates["com.mattermost.calls"] == nil && IsCloud() { + // Enable the calls plugin by default on Cloud only + s.PluginStates["com.mattermost.calls"] = &PluginState{Enable: true} + } + if s.EnableMarketplace == nil { s.EnableMarketplace = NewBool(PluginSettingsDefaultEnableMarketplace) } diff --git a/model/config_test.go b/model/config_test.go index 9bb16d94c46f9..afad3e060b7a3 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -6,6 +6,7 @@ package model import ( "encoding/json" "fmt" + "os" "reflect" "testing" @@ -1491,3 +1492,38 @@ func TestConfigServiceSettingsIsValid(t *testing.T) { require.NotNil(t, err) require.Equal(t, "model.config.is_valid.collapsed_threads.autofollow.app_error", err.Id) } + +func TestConfigDefaultCallsPluginState(t *testing.T) { + t.Run("should not enable Calls plugin by default when not in Cloud", func(t *testing.T) { + c1 := Config{} + c1.SetDefaults() + + assert.Nil(t, c1.PluginSettings.PluginStates["com.mattermost.calls"]) + }) + + t.Run("should enable Calls plugin by default on Cloud", func(t *testing.T) { + os.Setenv("MM_CLOUD_INSTALLATION_ID", "test") + defer os.Unsetenv("MM_CLOUD_INSTALLATION_ID") + c1 := Config{} + c1.SetDefaults() + + assert.True(t, c1.PluginSettings.PluginStates["com.mattermost.calls"].Enable) + }) + + t.Run("should not re-enable Calls plugin after it has been disabled", func(t *testing.T) { + os.Setenv("MM_CLOUD_INSTALLATION_ID", "test") + defer os.Unsetenv("MM_CLOUD_INSTALLATION_ID") + c1 := Config{ + PluginSettings: PluginSettings{ + PluginStates: map[string]*PluginState{ + "com.mattermost.calls": { + Enable: false, + }, + }, + }, + } + + c1.SetDefaults() + assert.False(t, c1.PluginSettings.PluginStates["com.mattermost.calls"].Enable) + }) +} diff --git a/model/feature_flags.go b/model/feature_flags.go index 64e85f347e05b..c16867c377293 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -32,12 +32,16 @@ type FeatureFlags struct { 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 // Enable Calls plugin support in the mobile app CallsMobile bool + // CallsEnabled controls whether or not the Calls plugin should be enabled + CallsEnabled bool + // A dash separated list for feature flags to turn on for Boards BoardsFeatureFlags string @@ -93,6 +97,7 @@ func (f *FeatureFlags) SetDefaults() { f.CloudFree = false f.CommandPalette = false } + func (f *FeatureFlags) Plugins() map[string]string { rFFVal := reflect.ValueOf(f).Elem() rFFType := reflect.TypeOf(f).Elem() diff --git a/model/utils.go b/model/utils.go index 636e707ef2ac6..4eeb352b33ad6 100644 --- a/model/utils.go +++ b/model/utils.go @@ -16,6 +16,7 @@ import ( "net/http" "net/mail" "net/url" + "os" "regexp" "sort" "strings" @@ -691,3 +692,7 @@ func filterBlocklist(r rune) rune { return r } + +func IsCloud() bool { + return os.Getenv("MM_CLOUD_INSTALLATION_ID") != "" +}