From 3a7170910358c495f3a48ffa1bb41ea719d4a172 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 12 Feb 2019 08:37:54 -0500 Subject: [PATCH] MM-13893: refactor config (#10230) * refactor utils/config* to config/ * pull validateLdapFilter into app * clean up Config/GetConfig/GetSanitizedConfig usage Eliminate app.GetConfig() in favour of just using app.Config() directly, but expose app.GetSanitizedConfig() for when the old behaviour was required. * web: isolate config setup * TestInvitePeopleProvider: make config explicit * regenerateClientConfig: avoid racey map access * integrate watch flag into app.ConfigFile option * make app.Option return an error * release.mk: only cp static files from config/ * release.mk: fix cp static files from config/ * api4: TestPlugin cleanup * s/c/cfg/ for clarity * fix merge conflict * testlib: allow customization of testlib driver name --- api4/apitestlib.go | 2 +- api4/plugin_test.go | 10 ----- api4/post_test.go | 2 +- api4/system.go | 11 +++--- api4/system_test.go | 6 +-- api4/user.go | 2 +- app/admin.go | 17 ++++++--- app/app_test.go | 4 +- app/command_invite_people_test.go | 9 ++--- app/config.go | 25 +++++++------ app/config_test.go | 2 +- app/enterprise.go | 2 +- app/helper_test.go | 2 +- app/options.go | 37 ++++++++++++------ app/plugin_api.go | 6 +-- app/server.go | 7 +++- build/release.mk | 5 ++- cmd/mattermost/commands/config.go | 3 +- cmd/mattermost/commands/init.go | 2 +- cmd/mattermost/commands/plugin_test.go | 16 ++++---- cmd/mattermost/commands/server.go | 5 +-- {utils => config}/config.go | 52 ++------------------------ {utils => config}/config_test.go | 16 ++++---- migrations/helper_test.go | 2 +- services/mailservice/mail_test.go | 9 +++-- testlib/helper.go | 7 +++- utils/logger.go | 48 ++++++++++++++++++++++++ web/handlers_test.go | 47 +++++++++-------------- web/web_test.go | 24 +++++++++++- web/webhook_test.go | 26 ++++++------- 30 files changed, 221 insertions(+), 185 deletions(-) rename {utils => config}/config.go (95%) rename {utils => config}/config_test.go (98%) create mode 100644 utils/logger.go diff --git a/api4/apitestlib.go b/api4/apitestlib.go index a383d76370..0551c03de3 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -79,7 +79,7 @@ func setupTestHelper(enterprise bool, updateConfig func(*model.Config)) *TestHel panic(err) } - options := []app.Option{app.ConfigFile(tempConfig.Name()), app.DisableConfigWatch} + options := []app.Option{app.ConfigFile(tempConfig.Name(), false)} options = append(options, app.StoreOverride(testStore)) s, err := app.NewServer(options...) diff --git a/api4/plugin_test.go b/api4/plugin_test.go index 4a143ee190..e17d96ecea 100644 --- a/api4/plugin_test.go +++ b/api4/plugin_test.go @@ -20,19 +20,9 @@ func TestPlugin(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - enablePlugins := *th.App.Config().PluginSettings.Enable - enableUploadPlugins := *th.App.Config().PluginSettings.EnableUploads statesJson, _ := json.Marshal(th.App.Config().PluginSettings.PluginStates) states := map[string]*model.PluginState{} json.Unmarshal(statesJson, &states) - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.PluginSettings.Enable = enablePlugins - *cfg.PluginSettings.EnableUploads = enableUploadPlugins - cfg.PluginSettings.PluginStates = states - }) - th.App.SaveConfig(th.App.Config(), false) - }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.PluginSettings.Enable = true *cfg.PluginSettings.EnableUploads = true diff --git a/api4/post_test.go b/api4/post_test.go index e4dc16e904..2e3b14b894 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -741,7 +741,7 @@ func TestPinPost(t *testing.T) { CheckForbiddenStatus(t, resp) t.Run("unable-to-pin-post-in-read-only-town-square", func(t *testing.T) { - townSquareIsReadOnly := *th.App.GetConfig().TeamSettings.ExperimentalTownSquareIsReadOnly + townSquareIsReadOnly := *th.App.Config().TeamSettings.ExperimentalTownSquareIsReadOnly th.App.SetLicense(model.NewTestLicense()) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = true }) diff --git a/api4/system.go b/api4/system.go index e4437d66c1..ac868d360e 100644 --- a/api4/system.go +++ b/api4/system.go @@ -103,7 +103,7 @@ func getConfig(c *Context, w http.ResponseWriter, r *http.Request) { return } - cfg := c.App.GetConfig() + cfg := c.App.GetSanitizedConfig() w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") w.Write([]byte(cfg.ToJson())) @@ -134,12 +134,12 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { } // Do not allow plugin uploads to be toggled through the API - cfg.PluginSettings.EnableUploads = c.App.GetConfig().PluginSettings.EnableUploads + cfg.PluginSettings.EnableUploads = c.App.Config().PluginSettings.EnableUploads // If the Message Export feature has been toggled in the System Console, rewrite the ExportFromTimestamp field to an // appropriate value. The rewriting occurs here to ensure it doesn't affect values written to the config file // directly and not through the System Console UI. - if *cfg.MessageExportSettings.EnableExport != *c.App.GetConfig().MessageExportSettings.EnableExport { + if *cfg.MessageExportSettings.EnableExport != *c.App.Config().MessageExportSettings.EnableExport { if *cfg.MessageExportSettings.EnableExport && *cfg.MessageExportSettings.ExportFromTimestamp == int64(0) { // When the feature is toggled on, use the current timestamp as the start time for future exports. cfg.MessageExportSettings.ExportFromTimestamp = model.NewInt64(model.GetMillis()) @@ -158,7 +158,7 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { c.LogAudit("updateConfig") - cfg = c.App.GetConfig() + cfg = c.App.GetSanitizedConfig() w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") w.Write([]byte(cfg.ToJson())) @@ -477,8 +477,7 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) { m := make(map[string]string) m["location"] = "" - cfg := c.App.GetConfig() - if !*cfg.ServiceSettings.EnableLinkPreviews { + if !*c.App.Config().ServiceSettings.EnableLinkPreviews { w.Write([]byte(model.MapToJson(m))) return } diff --git a/api4/system_test.go b/api4/system_test.go index 252990e030..314f3b3a28 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -131,19 +131,19 @@ func TestUpdateConfig(t *testing.T) { require.Equal(t, SiteName, cfg.TeamSettings.SiteName, "It should update the SiteName") t.Run("Should not be able to modify PluginSettings.EnableUploads", func(t *testing.T) { - oldEnableUploads := *th.App.GetConfig().PluginSettings.EnableUploads + oldEnableUploads := *th.App.Config().PluginSettings.EnableUploads *cfg.PluginSettings.EnableUploads = !oldEnableUploads cfg, resp = th.SystemAdminClient.UpdateConfig(cfg) CheckNoError(t, resp) assert.Equal(t, oldEnableUploads, *cfg.PluginSettings.EnableUploads) - assert.Equal(t, oldEnableUploads, *th.App.GetConfig().PluginSettings.EnableUploads) + assert.Equal(t, oldEnableUploads, *th.App.Config().PluginSettings.EnableUploads) cfg.PluginSettings.EnableUploads = nil cfg, resp = th.SystemAdminClient.UpdateConfig(cfg) CheckNoError(t, resp) assert.Equal(t, oldEnableUploads, *cfg.PluginSettings.EnableUploads) - assert.Equal(t, oldEnableUploads, *th.App.GetConfig().PluginSettings.EnableUploads) + assert.Equal(t, oldEnableUploads, *th.App.Config().PluginSettings.EnableUploads) }) } diff --git a/api4/user.go b/api4/user.go index f6881b45d7..04a630ac5c 100644 --- a/api4/user.go +++ b/api4/user.go @@ -856,7 +856,7 @@ func updateUserActive(c *Context, w http.ResponseWriter, r *http.Request) { } // if EnableUserDeactivation flag is disabled the user cannot deactivate himself. - if isSelfDeactive && !*c.App.GetConfig().TeamSettings.EnableUserDeactivation { + if isSelfDeactive && !*c.App.Config().TeamSettings.EnableUserDeactivation { c.Err = model.NewAppError("updateUserActive", "api.user.update_active.not_enable.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized) return } diff --git a/app/admin.go b/app/admin.go index 14d1abcdfc..7e2a23332a 100644 --- a/app/admin.go +++ b/app/admin.go @@ -6,13 +6,13 @@ package app import ( "io" "os" - "strings" "time" "runtime/debug" "net/http" + "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/mailservice" @@ -149,9 +149,8 @@ func (a *App) InvalidateAllCachesSkipSend() { a.LoadLicense() } -func (a *App) GetConfig() *model.Config { - json := a.Config().ToJson() - cfg := model.ConfigFromJson(strings.NewReader(json)) +func (a *App) GetSanitizedConfig() *model.Config { + cfg := a.Config().Clone() cfg.Sanitize() return cfg @@ -161,6 +160,14 @@ func (a *App) GetEnvironmentConfig() map[string]interface{} { return a.EnvironmentConfig() } +func validateLdapFilter(cfg *model.Config, ldap einterfaces.LdapInterface) *model.AppError { + if !*cfg.LdapSettings.Enable || ldap == nil || *cfg.LdapSettings.UserFilter == "" { + return nil + } + + return ldap.ValidateFilter(*cfg.LdapSettings.UserFilter) +} + func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { oldCfg := a.Config() cfg.SetDefaults() @@ -170,7 +177,7 @@ func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) return err } - if err := utils.ValidateLdapFilter(cfg, a.Ldap); err != nil { + if err := validateLdapFilter(cfg, a.Ldap); err != nil { return err } diff --git a/app/app_test.go b/app/app_test.go index 4ddec64c73..4a622d29c7 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -405,7 +405,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { th.App.DoAdvancedPermissionsMigration() - config := th.App.GetConfig() + config := th.App.Config() assert.Equal(t, -1, *config.ServiceSettings.PostEditTimeLimit) th.ResetRoleMigration() @@ -416,7 +416,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { }) th.App.DoAdvancedPermissionsMigration() - config = th.App.GetConfig() + config = th.App.Config() assert.Equal(t, 300, *config.ServiceSettings.PostEditTimeLimit) } diff --git a/app/command_invite_people_test.go b/app/command_invite_people_test.go index 5cf7aa4127..adff12a55a 100644 --- a/app/command_invite_people_test.go +++ b/app/command_invite_people_test.go @@ -15,11 +15,10 @@ func TestInvitePeopleProvider(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - enableEmailInvitations := *th.App.Config().ServiceSettings.EnableEmailInvitations - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableEmailInvitations = &enableEmailInvitations }) - }() - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = true }) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.SendEmailNotifications = true + *cfg.ServiceSettings.EnableEmailInvitations = true + }) cmd := InvitePeopleProvider{} diff --git a/app/config.go b/app/config.go index 3a6c530281..e024548bb5 100644 --- a/app/config.go +++ b/app/config.go @@ -18,6 +18,7 @@ import ( "strings" "time" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" @@ -63,13 +64,13 @@ func (a *App) UpdateConfig(f func(*model.Config)) { } func (a *App) PersistConfig() { - utils.SaveConfig(a.ConfigFileName(), a.Config()) + config.SaveConfig(a.ConfigFileName(), a.Config()) } func (s *Server) LoadConfig(configFile string) *model.AppError { old := s.Config() - cfg, configPath, envConfig, err := utils.LoadConfig(configFile) + cfg, configPath, envConfig, err := config.LoadConfig(configFile) if err != nil { return err } @@ -117,7 +118,7 @@ func (a *App) LimitedClientConfig() map[string]string { func (s *Server) EnableConfigWatch() { if s.configWatcher == nil && !s.disableConfigWatch { - configWatcher, err := utils.NewConfigWatcher(s.configFile, func() { + configWatcher, err := config.NewConfigWatcher(s.configFile, func() { s.ReloadConfig() }) if err != nil { @@ -280,26 +281,28 @@ func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey { } func (a *App) regenerateClientConfig() { - a.Srv.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId(), a.License()) - a.Srv.limitedClientConfig = utils.GenerateLimitedClientConfig(a.Config(), a.DiagnosticId(), a.License()) + clientConfig := config.GenerateClientConfig(a.Config(), a.DiagnosticId(), a.License()) + limitedClientConfig := config.GenerateLimitedClientConfig(a.Config(), a.DiagnosticId(), a.License()) - if a.Srv.clientConfig["EnableCustomTermsOfService"] == "true" { + if clientConfig["EnableCustomTermsOfService"] == "true" { termsOfService, err := a.GetLatestTermsOfService() if err != nil { mlog.Err(err) } else { - a.Srv.clientConfig["CustomTermsOfServiceId"] = termsOfService.Id - a.Srv.limitedClientConfig["CustomTermsOfServiceId"] = termsOfService.Id + clientConfig["CustomTermsOfServiceId"] = termsOfService.Id + limitedClientConfig["CustomTermsOfServiceId"] = termsOfService.Id } } if key := a.AsymmetricSigningKey(); key != nil { der, _ := x509.MarshalPKIXPublicKey(&key.PublicKey) - a.Srv.clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) - a.Srv.limitedClientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) + clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) + limitedClientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) } - clientConfigJSON, _ := json.Marshal(a.Srv.clientConfig) + clientConfigJSON, _ := json.Marshal(clientConfig) + a.Srv.clientConfig = clientConfig + a.Srv.limitedClientConfig = limitedClientConfig a.Srv.clientConfigHash = fmt.Sprintf("%x", md5.Sum(clientConfigJSON)) } diff --git a/app/config_test.go b/app/config_test.go index f346c2f586..682a146359 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -42,7 +42,7 @@ func TestLoadConfig(t *testing.T) { appErr := a.LoadConfig(tempConfig.Name()) require.Nil(t, appErr) - assert.Equal(t, "http://localhost:8065", *a.GetConfig().ServiceSettings.SiteURL) + assert.Equal(t, "http://localhost:8065", *a.Config().ServiceSettings.SiteURL) } func TestConfigListener(t *testing.T) { diff --git a/app/enterprise.go b/app/enterprise.go index e96e0a3be1..47a88705f0 100644 --- a/app/enterprise.go +++ b/app/enterprise.go @@ -120,7 +120,7 @@ func (s *Server) initEnterprise() { if ldapInterface != nil { s.Ldap = ldapInterface(s.FakeApp()) s.AddConfigListener(func(_, cfg *model.Config) { - if err := utils.ValidateLdapFilter(cfg, s.Ldap); err != nil { + if err := validateLdapFilter(cfg, s.Ldap); err != nil { panic(utils.T(err.Id)) } }) diff --git a/app/helper_test.go b/app/helper_test.go index d8f749b92e..4453b5af25 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -51,7 +51,7 @@ func setupTestHelper(enterprise bool) *TestHelper { panic(err) } - options := []Option{ConfigFile(tempConfig.Name()), DisableConfigWatch} + options := []Option{ConfigFile(tempConfig.Name(), false)} options = append(options, StoreOverride(mainHelper.Store)) s, err := NewServer(options...) diff --git a/app/options.go b/app/options.go index 41b6d00e4d..c6b56808a3 100644 --- a/app/options.go +++ b/app/options.go @@ -4,56 +4,69 @@ package app import ( + "github.com/pkg/errors" + "github.com/mattermost/mattermost-server/store" ) -type Option func(s *Server) +type Option func(s *Server) error // By default, the app will use the store specified by the configuration. This allows you to // construct an app with a different store. // // The override parameter must be either a store.Store or func(App) store.Store. func StoreOverride(override interface{}) Option { - return func(s *Server) { + return func(s *Server) error { switch o := override.(type) { case store.Store: s.newStore = func() store.Store { return o } + return nil + case func(*Server) store.Store: s.newStore = func() store.Store { return o(s) } + return nil + default: - panic("invalid StoreOverride") + return errors.New("invalid StoreOverride") } } } -func ConfigFile(file string) Option { - return func(s *Server) { +func ConfigFile(file string, watch bool) Option { + return func(s *Server) error { s.configFile = file + s.disableConfigWatch = !watch + + return nil } } -func RunJobs(s *Server) { +func RunJobs(s *Server) error { s.runjobs = true + + return nil } -func JoinCluster(s *Server) { +func JoinCluster(s *Server) error { s.joinCluster = true + + return nil } -func StartMetrics(s *Server) { +func StartMetrics(s *Server) error { s.startMetrics = true + + return nil } -func StartElasticsearch(s *Server) { +func StartElasticsearch(s *Server) error { s.startElasticsearch = true -} -func DisableConfigWatch(s *Server) { - s.disableConfigWatch = true + return nil } type AppOption func(a *App) diff --git a/app/plugin_api.go b/app/plugin_api.go index aea9011086..a7f5900072 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -77,7 +77,7 @@ func (api *PluginAPI) GetSession(sessionId string) (*model.Session, *model.AppEr } func (api *PluginAPI) GetConfig() *model.Config { - return api.app.GetConfig() + return api.app.GetSanitizedConfig() } func (api *PluginAPI) SaveConfig(config *model.Config) *model.AppError { @@ -85,7 +85,7 @@ func (api *PluginAPI) SaveConfig(config *model.Config) *model.AppError { } func (api *PluginAPI) GetPluginConfig() map[string]interface{} { - cfg := api.app.GetConfig() + cfg := api.app.GetSanitizedConfig() if pluginConfig, isOk := cfg.PluginSettings.Plugins[api.manifest.Id]; isOk { return pluginConfig } @@ -93,7 +93,7 @@ func (api *PluginAPI) GetPluginConfig() map[string]interface{} { } func (api *PluginAPI) SavePluginConfig(pluginConfig map[string]interface{}) *model.AppError { - cfg := api.app.GetConfig() + cfg := api.app.GetSanitizedConfig() cfg.PluginSettings.Plugins[api.manifest.Id] = pluginConfig return api.app.SaveConfig(cfg, true) } diff --git a/app/server.go b/app/server.go index dc33406537..5ac6acb3cc 100644 --- a/app/server.go +++ b/app/server.go @@ -24,6 +24,7 @@ import ( "github.com/throttled/throttled" "golang.org/x/crypto/acme/autocert" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/jobs" "github.com/mattermost/mattermost-server/mlog" @@ -96,7 +97,7 @@ type Server struct { logListenerId string clusterLeaderListenerId string disableConfigWatch bool - configWatcher *utils.ConfigWatcher + configWatcher *config.ConfigWatcher asymmetricSigningKey *ecdsa.PrivateKey pluginCommands []*PluginCommand @@ -144,7 +145,9 @@ func NewServer(options ...Option) (*Server, error) { clientConfig: make(map[string]string), } for _, option := range options { - option(s) + if err := option(s); err != nil { + return nil, errors.Wrap(err, "failed to apply option") + } } if err := s.LoadConfig(s.configFile); err != nil { diff --git a/build/release.mk b/build/release.mk index 7da9ab8580..3be5b166b2 100644 --- a/build/release.mk +++ b/build/release.mk @@ -32,7 +32,10 @@ package: mkdir -p $(DIST_PATH)/prepackaged_plugins @# Resource directories - cp -RL config $(DIST_PATH) + mkdir -p $(DIST_PATH)/config + cp -L config/README.md $(DIST_PATH)/config + cp -L config/config.json $(DIST_PATH)/config + cp -L config/timezones.json $(DIST_PATH)/config cp -RL fonts $(DIST_PATH) cp -RL templates $(DIST_PATH) cp -RL i18n $(DIST_PATH) diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index bb22447e61..5963ac87d3 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -14,6 +14,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" "github.com/mattermost/mattermost-server/utils/fileutils" @@ -234,7 +235,7 @@ func configSetCmdF(command *cobra.Command, args []string) error { return err } - if err := utils.ValidateLocales(app.Config()); err != nil { + if err := config.ValidateLocales(app.Config()); err != nil { return errors.New("Invalid locale configuration") } diff --git a/cmd/mattermost/commands/init.go b/cmd/mattermost/commands/init.go index a1a12806ef..b2d7229c0d 100644 --- a/cmd/mattermost/commands/init.go +++ b/cmd/mattermost/commands/init.go @@ -36,7 +36,7 @@ func InitDBCommandContext(configFileLocation string) (*app.App, error) { } model.AppErrorInit(utils.T) - s, err := app.NewServer(app.ConfigFile(configFileLocation)) + s, err := app.NewServer(app.ConfigFile(configFileLocation, true)) if err != nil { return nil, err } diff --git a/cmd/mattermost/commands/plugin_test.go b/cmd/mattermost/commands/plugin_test.go index 3c932559a1..b69a37dc2b 100644 --- a/cmd/mattermost/commands/plugin_test.go +++ b/cmd/mattermost/commands/plugin_test.go @@ -5,7 +5,7 @@ import ( "path/filepath" "testing" - "github.com/mattermost/mattermost-server/utils" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,11 +15,11 @@ func TestPlugin(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - config := th.Config() - *config.PluginSettings.EnableUploads = true - *config.PluginSettings.Directory = "./test-plugins" - *config.PluginSettings.ClientDirectory = "./test-client-plugins" - th.SetConfig(config) + cfg := th.Config() + *cfg.PluginSettings.EnableUploads = true + *cfg.PluginSettings.Directory = "./test-plugins" + *cfg.PluginSettings.ClientDirectory = "./test-client-plugins" + th.SetConfig(cfg) os.MkdirAll("./test-plugins", os.ModePerm) os.MkdirAll("./test-client-plugins", os.ModePerm) @@ -31,12 +31,12 @@ func TestPlugin(t *testing.T) { th.CheckCommand(t, "plugin", "add", filepath.Join(path, "testplugin.tar.gz")) th.CheckCommand(t, "plugin", "enable", "testplugin") - cfg, _, _, err := utils.LoadConfig(th.ConfigPath()) + cfg, _, _, err := config.LoadConfig(th.ConfigPath()) require.Nil(t, err) assert.Equal(t, cfg.PluginSettings.PluginStates["testplugin"].Enable, true) th.CheckCommand(t, "plugin", "disable", "testplugin") - cfg, _, _, err = utils.LoadConfig(th.ConfigPath()) + cfg, _, _, err = config.LoadConfig(th.ConfigPath()) require.Nil(t, err) assert.Equal(t, cfg.PluginSettings.PluginStates["testplugin"].Enable, false) diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index 17579f4cea..85be460b32 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -45,15 +45,12 @@ func serverCmdF(command *cobra.Command, args []string) error { func runServer(configFileLocation string, disableConfigWatch bool, usedPlatform bool, interruptChan chan os.Signal) error { options := []app.Option{ - app.ConfigFile(configFileLocation), + app.ConfigFile(configFileLocation, !disableConfigWatch), app.RunJobs, app.JoinCluster, app.StartElasticsearch, app.StartMetrics, } - if disableConfigWatch { - options = append(options, app.DisableConfigWatch) - } server, err := app.NewServer(options...) if err != nil { mlog.Critical(err.Error()) diff --git a/utils/config.go b/config/config.go similarity index 95% rename from utils/config.go rename to config/config.go index df838e3cbf..826a69e1dc 100644 --- a/utils/config.go +++ b/config/config.go @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. -package utils +package config import ( "bytes" @@ -21,18 +21,13 @@ import ( "net/http" - "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/mattermost/mattermost-server/utils/jsonutils" ) -const ( - LOG_ROTATE_SIZE = 10000 - LOG_FILENAME = "mattermost.log" -) - var ( termsOfServiceEnabledAndEmpty = model.NewAppError( "Config.IsValid", @@ -43,36 +38,6 @@ var ( ) ) -func MloggerConfigFromLoggerConfig(s *model.LogSettings) *mlog.LoggerConfiguration { - return &mlog.LoggerConfiguration{ - EnableConsole: *s.EnableConsole, - ConsoleJson: *s.ConsoleJson, - ConsoleLevel: strings.ToLower(*s.ConsoleLevel), - EnableFile: *s.EnableFile, - FileJson: *s.FileJson, - FileLevel: strings.ToLower(*s.FileLevel), - FileLocation: GetLogFileLocation(*s.FileLocation), - } -} - -// DON'T USE THIS Modify the level on the app logger -func DisableDebugLogForTest() { - mlog.GloballyDisableDebugLogForTest() -} - -// DON'T USE THIS Modify the level on the app logger -func EnableDebugLogForTest() { - mlog.GloballyEnableDebugLogForTest() -} - -func GetLogFileLocation(fileLocation string) string { - if fileLocation == "" { - fileLocation, _ = fileutils.FindDir("logs") - } - - return filepath.Join(fileLocation, LOG_FILENAME) -} - func SaveConfig(fileName string, config *model.Config) *model.AppError { b, err := json.MarshalIndent(config, "", " ") if err != nil { @@ -716,18 +681,9 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license * return props } -func ValidateLdapFilter(cfg *model.Config, ldap einterfaces.LdapInterface) *model.AppError { - if *cfg.LdapSettings.Enable && ldap != nil && *cfg.LdapSettings.UserFilter != "" { - if err := ldap.ValidateFilter(*cfg.LdapSettings.UserFilter); err != nil { - return err - } - } - return nil -} - func ValidateLocales(cfg *model.Config) *model.AppError { var err *model.AppError - locales := GetSupportedLocales() + locales := utils.GetSupportedLocales() if _, ok := locales[*cfg.LocalizationSettings.DefaultServerLocale]; !ok { *cfg.LocalizationSettings.DefaultServerLocale = model.DEFAULT_LOCALE err = model.NewAppError("ValidateLocales", "utils.config.supported_server_locale.app_error", nil, "", http.StatusBadRequest) @@ -760,7 +716,7 @@ func ValidateLocales(cfg *model.Config) *model.AppError { err = model.NewAppError("ValidateLocales", "utils.config.add_client_locale.app_error", nil, "", http.StatusBadRequest) } - *cfg.LocalizationSettings.AvailableLocales = strings.Join(RemoveDuplicatesFromStringArray(strings.Split(availableLocales, ",")), ",") + *cfg.LocalizationSettings.AvailableLocales = strings.Join(utils.RemoveDuplicatesFromStringArray(strings.Split(availableLocales, ",")), ",") } return err diff --git a/utils/config_test.go b/config/config_test.go similarity index 98% rename from utils/config_test.go rename to config/config_test.go index b40a5b79be..7be4dfaa7f 100644 --- a/utils/config_test.go +++ b/config/config_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. -package utils +package config import ( "bytes" @@ -14,16 +14,17 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" ) func TestConfig(t *testing.T) { - TranslationsPreInit() + utils.TranslationsPreInit() _, _, _, err := LoadConfig("config.json") require.Nil(t, err) } func TestReadConfig(t *testing.T) { - TranslationsPreInit() + utils.TranslationsPreInit() _, _, err := ReadConfig(bytes.NewReader([]byte(``)), false) require.EqualError(t, err, "parsing error at line 1, character 1: unexpected end of JSON input") @@ -36,7 +37,7 @@ func TestReadConfig(t *testing.T) { } func TestReadConfig_PluginSettings(t *testing.T) { - TranslationsPreInit() + utils.TranslationsPreInit() config, _, err := ReadConfig(bytes.NewReader([]byte(`{ "PluginSettings": { @@ -109,8 +110,9 @@ func TestReadConfig_PluginSettings(t *testing.T) { }, *config.PluginSettings.PluginStates["jira"]) } } + func TestReadConfig_ImageProxySettings(t *testing.T) { - TranslationsPreInit() + utils.TranslationsPreInit() t.Run("deprecated settings should still be read properly", func(t *testing.T) { config, _, err := ReadConfig(bytes.NewReader([]byte(`{ @@ -130,7 +132,7 @@ func TestReadConfig_ImageProxySettings(t *testing.T) { } func TestConfigFromEnviroVars(t *testing.T) { - TranslationsPreInit() + utils.TranslationsPreInit() config := `{ "ServiceSettings": { @@ -375,7 +377,7 @@ func TestConfigFromEnviroVars(t *testing.T) { } func TestValidateLocales(t *testing.T) { - TranslationsPreInit() + utils.TranslationsPreInit() cfg, _, _, err := LoadConfig("config.json") require.Nil(t, err) diff --git a/migrations/helper_test.go b/migrations/helper_test.go index 653a40c716..4bf4469dcf 100644 --- a/migrations/helper_test.go +++ b/migrations/helper_test.go @@ -49,7 +49,7 @@ func setupTestHelper(enterprise bool) *TestHelper { panic(err) } - options := []app.Option{app.ConfigFile(tempConfig.Name()), app.DisableConfigWatch} + options := []app.Option{app.ConfigFile(tempConfig.Name(), false)} options = append(options, app.StoreOverride(mainHelper.Store)) s, err := app.NewServer(options...) diff --git a/services/mailservice/mail_test.go b/services/mailservice/mail_test.go index 6f56e1f20b..04c5035604 100644 --- a/services/mailservice/mail_test.go +++ b/services/mailservice/mail_test.go @@ -12,6 +12,7 @@ import ( "net/mail" "net/smtp" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/filesstore" "github.com/mattermost/mattermost-server/utils" @@ -20,7 +21,7 @@ import ( ) func TestMailConnectionFromConfig(t *testing.T) { - cfg, _, _, err := utils.LoadConfig("config.json") + cfg, _, _, err := config.LoadConfig("config.json") require.Nil(t, err) if conn, err := ConnectToSMTPServer(cfg); err != nil { @@ -43,7 +44,7 @@ func TestMailConnectionFromConfig(t *testing.T) { } func TestMailConnectionAdvanced(t *testing.T) { - cfg, _, _, err := utils.LoadConfig("config.json") + cfg, _, _, err := config.LoadConfig("config.json") require.Nil(t, err) if conn, err := ConnectToSMTPServerAdvanced( @@ -93,7 +94,7 @@ func TestMailConnectionAdvanced(t *testing.T) { } func TestSendMailUsingConfig(t *testing.T) { - cfg, _, _, err := utils.LoadConfig("config.json") + cfg, _, _, err := config.LoadConfig("config.json") require.Nil(t, err) utils.T = utils.GetUserTranslations("en") @@ -135,7 +136,7 @@ func TestSendMailUsingConfig(t *testing.T) { } func TestSendMailUsingConfigAdvanced(t *testing.T) { - cfg, _, _, err := utils.LoadConfig("config.json") + cfg, _, _, err := config.LoadConfig("config.json") require.Nil(t, err) utils.T = utils.GetUserTranslations("en") diff --git a/testlib/helper.go b/testlib/helper.go index adb7177ef8..760cca058c 100644 --- a/testlib/helper.go +++ b/testlib/helper.go @@ -40,7 +40,12 @@ func NewMainHelper() *MainHelper { utils.TranslationsPreInit() - settings := storetest.MakeSqlSettings(model.DATABASE_DRIVER_MYSQL) + driverName := os.Getenv("MM_SQLSETTINGS_DRIVERNAME") + if driverName == "" { + driverName = model.DATABASE_DRIVER_MYSQL + } + + settings := storetest.MakeSqlSettings(driverName) clusterInterface := &FakeClusterInterface{} sqlSupplier := sqlstore.NewSqlSupplier(*settings, nil) diff --git a/utils/logger.go b/utils/logger.go new file mode 100644 index 0000000000..9b071b0841 --- /dev/null +++ b/utils/logger.go @@ -0,0 +1,48 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "path/filepath" + "strings" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils/fileutils" +) + +const ( + LOG_ROTATE_SIZE = 10000 + LOG_FILENAME = "mattermost.log" +) + +func MloggerConfigFromLoggerConfig(s *model.LogSettings) *mlog.LoggerConfiguration { + return &mlog.LoggerConfiguration{ + EnableConsole: *s.EnableConsole, + ConsoleJson: *s.ConsoleJson, + ConsoleLevel: strings.ToLower(*s.ConsoleLevel), + EnableFile: *s.EnableFile, + FileJson: *s.FileJson, + FileLevel: strings.ToLower(*s.FileLevel), + FileLocation: GetLogFileLocation(*s.FileLocation), + } +} + +func GetLogFileLocation(fileLocation string) string { + if fileLocation == "" { + fileLocation, _ = fileutils.FindDir("logs") + } + + return filepath.Join(fileLocation, LOG_FILENAME) +} + +// DON'T USE THIS Modify the level on the app logger +func DisableDebugLogForTest() { + mlog.GloballyDisableDebugLogForTest() +} + +// DON'T USE THIS Modify the level on the app logger +func EnableDebugLogForTest() { + mlog.GloballyEnableDebugLogForTest() +} diff --git a/web/handlers_test.go b/web/handlers_test.go index 252745e974..0eadb1c840 100644 --- a/web/handlers_test.go +++ b/web/handlers_test.go @@ -8,10 +8,8 @@ import ( "net/http/httptest" "testing" - "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func handlerForHTTPErrors(c *Context, w http.ResponseWriter, r *http.Request) { @@ -19,14 +17,10 @@ func handlerForHTTPErrors(c *Context, w http.ResponseWriter, r *http.Request) { } func TestHandlerServeHTTPErrors(t *testing.T) { - s, err := app.NewServer(app.StoreOverride(mainHelper.Store), app.DisableConfigWatch) - require.Nil(t, err) - defer s.Shutdown() + th := Setup().InitBasic() + defer th.TearDown() - web := New(s, s.AppOptions, s.Router) - if err != nil { - panic(err) - } + web := New(th.Server, th.Server.AppOptions, th.Server.Router) handler := web.NewHandler(handlerForHTTPErrors) var flagtests = []struct { @@ -63,21 +57,15 @@ func handlerForHTTPSecureTransport(c *Context, w http.ResponseWriter, r *http.Re } func TestHandlerServeHTTPSecureTransport(t *testing.T) { - s, err := app.NewServer(app.StoreOverride(mainHelper.Store), app.DisableConfigWatch) - require.Nil(t, err) - defer s.Shutdown() + th := Setup().InitBasic() + defer th.TearDown() - a := s.FakeApp() - - a.UpdateConfig(func(config *model.Config) { + th.App.UpdateConfig(func(config *model.Config) { *config.ServiceSettings.TLSStrictTransport = true *config.ServiceSettings.TLSStrictTransportMaxAge = 6000 }) - web := New(s, s.AppOptions, s.Router) - if err != nil { - panic(err) - } + web := New(th.Server, th.Server.AppOptions, th.Server.Router) handler := web.NewHandler(handlerForHTTPSecureTransport) request := httptest.NewRequest("GET", "/api/v4/test", nil) @@ -94,7 +82,7 @@ func TestHandlerServeHTTPSecureTransport(t *testing.T) { t.Errorf("Expected max-age=6000, got %s", header) } - a.UpdateConfig(func(config *model.Config) { + th.App.UpdateConfig(func(config *model.Config) { *config.ServiceSettings.TLSStrictTransport = false }) @@ -109,7 +97,6 @@ func TestHandlerServeHTTPSecureTransport(t *testing.T) { } } - func handlerForCSRFToken(c *Context, w http.ResponseWriter, r *http.Request) { } @@ -117,11 +104,11 @@ func TestHandlerServeCSRFToken(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - session :=&model.Session{ - UserId: th.BasicUser.Id, + session := &model.Session{ + UserId: th.BasicUser.Id, CreateAt: model.GetMillis(), - Roles: model.SYSTEM_USER_ROLE_ID, - IsOAuth: false, + Roles: model.SYSTEM_USER_ROLE_ID, + IsOAuth: false, } session.GenerateCSRF() session.SetExpireInDays(1) @@ -142,15 +129,15 @@ func TestHandlerServeCSRFToken(t *testing.T) { } cookie := &http.Cookie{ - Name: model.SESSION_COOKIE_USER, + Name: model.SESSION_COOKIE_USER, Value: th.BasicUser.Username, } cookie2 := &http.Cookie{ - Name: model.SESSION_COOKIE_TOKEN, + Name: model.SESSION_COOKIE_TOKEN, Value: session.Token, } cookie3 := &http.Cookie{ - Name: model.SESSION_COOKIE_CSRF, + Name: model.SESSION_COOKIE_CSRF, Value: session.GetCSRF(), } @@ -183,7 +170,7 @@ func TestHandlerServeCSRFToken(t *testing.T) { // Fallback Behavior Used - Success expected // ToDo (DSchalla) 2019/01/04: Remove once legacy CSRF Handling is removed - th.App.UpdateConfig(func(config *model.Config){ + th.App.UpdateConfig(func(config *model.Config) { *config.ServiceSettings.ExperimentalStrictCSRFEnforcement = false }) request = httptest.NewRequest("POST", "/api/v4/test", nil) @@ -200,7 +187,7 @@ func TestHandlerServeCSRFToken(t *testing.T) { // Fallback Behavior Used with Strict Enforcement - Failure Expected // ToDo (DSchalla) 2019/01/04: Remove once legacy CSRF Handling is removed - th.App.UpdateConfig(func(config *model.Config){ + th.App.UpdateConfig(func(config *model.Config) { *config.ServiceSettings.ExperimentalStrictCSRFEnforcement = true }) response = httptest.NewRecorder() diff --git a/web/web_test.go b/web/web_test.go index 5e3901af83..fe380e02e8 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -5,10 +5,14 @@ package web import ( "fmt" + "io" + "io/ioutil" + "os" "testing" "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils/fileutils" ) var ApiClient *model.Client4 @@ -28,7 +32,25 @@ type TestHelper struct { func Setup() *TestHelper { mainHelper.Store.DropAllTables() - s, err := app.NewServer(app.StoreOverride(mainHelper.Store), app.DisableConfigWatch) + permConfig, err := os.Open(fileutils.FindConfigFile("config.json")) + if err != nil { + panic(err) + } + defer permConfig.Close() + tempConfig, err := ioutil.TempFile("", "") + if err != nil { + panic(err) + } + _, err = io.Copy(tempConfig, permConfig) + tempConfig.Close() + if err != nil { + panic(err) + } + + options := []app.Option{app.ConfigFile(tempConfig.Name(), false)} + options = append(options, app.StoreOverride(mainHelper.Store)) + + s, err := app.NewServer(options...) if err != nil { panic(err) } diff --git a/web/webhook_test.go b/web/webhook_test.go index f5ab9e8771..873e872e59 100644 --- a/web/webhook_test.go +++ b/web/webhook_test.go @@ -235,13 +235,13 @@ func TestCommandWebhooks(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - cmd, err := th.App.CreateCommand(&model.Command{ + cmd, appErr := th.App.CreateCommand(&model.Command{ CreatorId: th.BasicUser.Id, TeamId: th.BasicTeam.Id, URL: "http://nowhere.com", Method: model.COMMAND_METHOD_POST, Trigger: "delayed"}) - require.Nil(t, err) + require.Nil(t, appErr) args := &model.CommandArgs{ TeamId: th.BasicTeam.Id, @@ -249,22 +249,22 @@ func TestCommandWebhooks(t *testing.T) { ChannelId: th.BasicChannel.Id, } - hook, err := th.App.CreateCommandWebhook(cmd.Id, args) - if err != nil { - t.Fatal(err) + hook, appErr := th.App.CreateCommandWebhook(cmd.Id, args) + if appErr != nil { + t.Fatal(appErr) } - if resp, _ := http.Post(ApiClient.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusNotFound { - t.Fatal("expected not-found for non-existent hook") - } + resp, err := http.Post(ApiClient.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)) + require.NoError(t, err) + assert.Equal(t, http.StatusNotFound, resp.StatusCode, "expected not-found for non-existent hook") - if resp, err := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"invalid`)); err != nil || resp.StatusCode != http.StatusBadRequest { - t.Fatal(err) - } + resp, err = http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"invalid`)) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) for i := 0; i < 5; i++ { - if resp, err := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); err != nil || resp.StatusCode != http.StatusOK { - t.Fatal(err) + if resp, appErr := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); err != nil || resp.StatusCode != http.StatusOK { + t.Fatal(appErr) } }