From 72560311a27bb957576416739cca5e71c8665980 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Mon, 3 Sep 2018 17:07:51 +0200 Subject: [PATCH 1/3] MM-11728: Avoid Archived channels editions throught Patch (#9335) --- i18n/en.json | 4 ++++ store/sqlstore/channel_store.go | 5 +++++ store/storetest/channel_store.go | 6 ++++++ 3 files changed, 15 insertions(+) diff --git a/i18n/en.json b/i18n/en.json index a0733827aa..5345fa0cfa 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5006,6 +5006,10 @@ "id": "store.sql_channel.update.app_error", "translation": "We couldn't update the channel" }, + { + "id": "store.sql_channel.update.archived_channel.app_error", + "translation": "You can not modify an archived channel" + }, { "id": "store.sql_channel.update.exists.app_error", "translation": "A channel with that handle already exists" diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 6d0c7353f8..fba37d7cba 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -463,6 +463,11 @@ func (s SqlChannelStore) Update(channel *model.Channel) store.StoreChannel { return store.Do(func(result *store.StoreResult) { channel.PreUpdate() + if channel.DeleteAt != 0 { + result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.archived_channel.app_error", nil, "", http.StatusBadRequest) + return + } + if result.Err = channel.IsValid(); result.Err != nil { return } diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index e2f91e0c9f..c827a4226f 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -220,6 +220,12 @@ func testChannelStoreUpdate(t *testing.T, ss store.Store) { t.Fatal(err) } + o1.DeleteAt = 100 + if err := (<-ss.Channel().Update(&o1)).Err; err == nil { + t.Fatal("Update should have failed because channel is archived") + } + + o1.DeleteAt = 0 o1.Id = "missing" if err := (<-ss.Channel().Update(&o1)).Err; err == nil { t.Fatal("Update should have failed because of missing key") From b98ef658ad217fafe98af088f516746860b2a7b8 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Mon, 3 Sep 2018 11:08:25 -0400 Subject: [PATCH 2/3] MM-11720: disable loading plugin specific config from the environment (#9334) There are numerous issues here, including some non-determinism in the viper library (fixable) and some annoying behaviour regarding periods in keys, often used by plugin ids (fix unknown). Let's defer the handling of same until we can get our config loading library to do what we need it to do vs. having to hack around viper all the time. --- utils/config.go | 14 +++++ utils/config_test.go | 141 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 133 insertions(+), 22 deletions(-) diff --git a/utils/config.go b/utils/config.go index 3ce2215a38..7e5a42faa0 100644 --- a/utils/config.go +++ b/utils/config.go @@ -247,6 +247,16 @@ func ReadConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, map var config model.Config unmarshalErr := v.Unmarshal(&config) + // https://github.com/spf13/viper/issues/324 + // https://github.com/spf13/viper/issues/348 + if unmarshalErr == nil { + config.PluginSettings.Plugins = make(map[string]map[string]interface{}) + unmarshalErr = v.UnmarshalKey("pluginsettings.plugins", &config.PluginSettings.Plugins) + } + if unmarshalErr == nil { + config.PluginSettings.PluginStates = make(map[string]*model.PluginState) + unmarshalErr = v.UnmarshalKey("pluginsettings.pluginstates", &config.PluginSettings.PluginStates) + } envConfig := v.EnvSettings() @@ -274,6 +284,10 @@ func newViper(allowEnvironmentOverrides bool) *viper.Viper { defaults := getDefaultsFromStruct(model.Config{}) for key, value := range defaults { + if key == "PluginSettings.Plugins" || key == "PluginSettings.PluginStates" { + continue + } + v.SetDefault(key, value) } diff --git a/utils/config_test.go b/utils/config_test.go index a08b73632c..77705192dd 100644 --- a/utils/config_test.go +++ b/utils/config_test.go @@ -52,32 +52,64 @@ func TestReadConfig_PluginSettings(t *testing.T) { "abc": 123, "def": "456" } - } + }, + "jira": { + "number": 2, + "string": "123", + "boolean": true, + "abc.def.ghi": { + "abc": 456, + "def": "123" + } + } }, "PluginStates": { "com.example.plugin": { "enable": true - } + }, + "jira": { + "enable": false + } } } }`)), false) require.Nil(t, err) assert.Equal(t, "/temp/mattermost-plugins", *config.PluginSettings.Directory) - assert.Contains(t, config.PluginSettings.Plugins, "com.example.plugin") - assert.Equal(t, map[string]interface{}{ - "number": float64(1), - "string": "abc", - "boolean": false, - "abc.def.ghi": map[string]interface{}{ - "abc": float64(123), - "def": "456", - }, - }, config.PluginSettings.Plugins["com.example.plugin"]) - assert.Contains(t, config.PluginSettings.PluginStates, "com.example.plugin") - assert.Equal(t, model.PluginState{ - Enable: true, - }, *config.PluginSettings.PluginStates["com.example.plugin"]) + + if assert.Contains(t, config.PluginSettings.Plugins, "com.example.plugin") { + assert.Equal(t, map[string]interface{}{ + "number": float64(1), + "string": "abc", + "boolean": false, + "abc.def.ghi": map[string]interface{}{ + "abc": float64(123), + "def": "456", + }, + }, config.PluginSettings.Plugins["com.example.plugin"]) + } + if assert.Contains(t, config.PluginSettings.PluginStates, "com.example.plugin") { + assert.Equal(t, model.PluginState{ + Enable: true, + }, *config.PluginSettings.PluginStates["com.example.plugin"]) + } + + if assert.Contains(t, config.PluginSettings.Plugins, "jira") { + assert.Equal(t, map[string]interface{}{ + "number": float64(2), + "string": "123", + "boolean": true, + "abc.def.ghi": map[string]interface{}{ + "abc": float64(456), + "def": "123", + }, + }, config.PluginSettings.Plugins["jira"]) + } + if assert.Contains(t, config.PluginSettings.PluginStates, "jira") { + assert.Equal(t, model.PluginState{ + Enable: false, + }, *config.PluginSettings.PluginStates["jira"]) + } } func TestTimezoneConfig(t *testing.T) { @@ -381,6 +413,20 @@ func TestConfigFromEnviroVars(t *testing.T) { }, "SupportSettings": { "TermsOfServiceLink": "https://about.mattermost.com/default-terms/" + }, + "PluginSettings": { + "Enable": true, + "Plugins": { + "jira": { + "enabled": "true", + "secret": "config-secret" + } + }, + "PluginStates": { + "jira": { + "Enable": true + } + } } }` @@ -517,20 +563,19 @@ func TestConfigFromEnviroVars(t *testing.T) { }) t.Run("plugin directory settings", func(t *testing.T) { + os.Setenv("MM_PLUGINSETTINGS_ENABLE", "false") os.Setenv("MM_PLUGINSETTINGS_DIRECTORY", "/temp/plugins") os.Setenv("MM_PLUGINSETTINGS_CLIENTDIRECTORY", "/temp/clientplugins") + defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLE") defer os.Unsetenv("MM_PLUGINSETTINGS_DIRECTORY") defer os.Unsetenv("MM_PLUGINSETTINGS_CLIENTDIRECTORY") cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) require.Nil(t, err) - if *cfg.PluginSettings.Directory != "/temp/plugins" { - t.Fatal("Couldn't read Directory from environment var") - } - if *cfg.PluginSettings.ClientDirectory != "/temp/clientplugins" { - t.Fatal("Couldn't read ClientDirectory from environment var") - } + assert.Equal(t, false, *cfg.PluginSettings.Enable) + assert.Equal(t, "/temp/plugins", *cfg.PluginSettings.Directory) + assert.Equal(t, "/temp/clientplugins", *cfg.PluginSettings.ClientDirectory) if pluginSettings, ok := envCfg["PluginSettings"]; !ok { t.Fatal("PluginSettings is missing from envConfig") @@ -545,6 +590,58 @@ func TestConfigFromEnviroVars(t *testing.T) { } } }) + + t.Run("plugin specific settings cannot be overridden via environment", func(t *testing.T) { + os.Setenv("MM_PLUGINSETTINGS_PLUGINS_JIRA_ENABLED", "false") + os.Setenv("MM_PLUGINSETTINGS_PLUGINS_JIRA_SECRET", "env-secret") + os.Setenv("MM_PLUGINSETTINGS_PLUGINSTATES_JIRA_ENABLE", "false") + defer os.Unsetenv("MM_PLUGINSETTINGS_PLUGINS_JIRA_ENABLED") + defer os.Unsetenv("MM_PLUGINSETTINGS_PLUGINS_JIRA_SECRET") + defer os.Unsetenv("MM_PLUGINSETTINGS_PLUGINSTATES_JIRA_ENABLE") + + cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) + require.Nil(t, err) + + if pluginsJira, ok := cfg.PluginSettings.Plugins["jira"]; !ok { + t.Fatal("PluginSettings.Plugins.jira is missing from config") + } else { + if enabled, ok := pluginsJira["enabled"]; !ok { + t.Fatal("PluginSettings.Plugins.jira.enabled is missing from config") + } else { + assert.Equal(t, "true", enabled) + } + + if secret, ok := pluginsJira["secret"]; !ok { + t.Fatal("PluginSettings.Plugins.jira.secret is missing from config") + } else { + assert.Equal(t, "config-secret", secret) + } + } + + if pluginStatesJira, ok := cfg.PluginSettings.PluginStates["jira"]; !ok { + t.Fatal("PluginSettings.PluginStates.jira is missing from config") + } else { + require.Equal(t, true, pluginStatesJira.Enable) + } + + if pluginSettings, ok := envCfg["PluginSettings"]; !ok { + t.Fatal("PluginSettings is missing from envConfig") + } else if pluginSettingsAsMap, ok := pluginSettings.(map[string]interface{}); !ok { + t.Fatal("PluginSettings is not a map in envConfig") + } else { + if plugins, ok := pluginSettingsAsMap["Plugins"].(map[string]interface{}); !ok { + t.Fatal("PluginSettings.Plugins is not a map in envConfig") + } else if _, ok := plugins["jira"].(map[string]interface{}); ok { + t.Fatal("PluginSettings.Plugins.jira should not be a map in envConfig") + } + + if pluginStates, ok := pluginSettingsAsMap["PluginStates"].(map[string]interface{}); !ok { + t.Fatal("PluginSettings.PluginStates is missing from envConfig") + } else if _, ok := pluginStates["jira"].(map[string]interface{}); ok { + t.Fatal("PluginSettings.PluginStates.jira should not be a map in envConfig") + } + } + }) } func TestValidateLocales(t *testing.T) { From d2190527eafdae31e14ba0832dcfb79a0a1e223b Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Tue, 4 Sep 2018 15:42:43 +0200 Subject: [PATCH 3/3] Uncomment upgrade for 5.3.0 (#9354) * Uncomment upgrade for 5.3.0 * add missing version --- model/version.go | 1 + store/sqlstore/upgrade.go | 8 +++----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/model/version.go b/model/version.go index d8753b3b10..6e45211afc 100644 --- a/model/version.go +++ b/model/version.go @@ -13,6 +13,7 @@ import ( // It should be maintained in chronological order with most current // release at the front of the list. var versions = []string{ + "5.3.0", "5.2.0", "5.1.0", "5.0.0", diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index ab3bd202bc..cd45dfcb3c 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -484,9 +484,7 @@ func UpgradeDatabaseToVersion52(sqlStore SqlStore) { } func UpgradeDatabaseToVersion53(sqlStore SqlStore) { - // TODO: Uncomment following condition when version 5.3.0 is released - // if shouldPerformUpgrade(sqlStore, VERSION_5_2_0, VERSION_5_3_0) { - - // saveSchemaVersion(sqlStore, VERSION_5_3_0) - // } + if shouldPerformUpgrade(sqlStore, VERSION_5_2_0, VERSION_5_3_0) { + saveSchemaVersion(sqlStore, VERSION_5_3_0) + } }