From 7e5ce976681e99be6b26d428935ba1106d530efa Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 12 Jan 2018 08:02:11 -0600 Subject: Remove global cfg vars (#8099) * remove global cfg vars * enterprise update --- app/admin.go | 2 +- app/app.go | 30 +++-- app/app_test.go | 4 +- app/config.go | 113 ++++++++++++++++-- app/config_test.go | 56 +++++++++ app/plugin.go | 8 +- cmd/platform/mattermost_test.go | 3 +- cmd/platform/message_export_test.go | 9 +- cmd/platform/server.go | 2 +- jobs/schedulers.go | 3 +- jobs/server.go | 32 +++-- jobs/workers.go | 33 +++--- utils/config.go | 121 ++----------------- utils/config_test.go | 228 ++++++++++-------------------------- utils/logger/logger.go | 9 -- utils/mail_test.go | 8 +- utils/redirect_std_log_test.go | 24 ++++ 17 files changed, 337 insertions(+), 348 deletions(-) create mode 100644 app/config_test.go create mode 100644 utils/redirect_std_log_test.go diff --git a/app/admin.go b/app/admin.go index e7d1eb9ed..b838ed3bd 100644 --- a/app/admin.go +++ b/app/admin.go @@ -159,7 +159,7 @@ func (a *App) GetConfig() *model.Config { func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { oldCfg := a.Config() cfg.SetDefaults() - utils.Desanitize(cfg) + a.Desanitize(cfg) if err := cfg.IsValid(); err != nil { return err diff --git a/app/app.go b/app/app.go index 561942019..1e46d29d0 100644 --- a/app/app.go +++ b/app/app.go @@ -54,8 +54,11 @@ type App struct { Mfa einterfaces.MfaInterface Saml einterfaces.SamlInterface - configFile string - newStore func() store.Store + config atomic.Value + configFile string + configListeners map[string]func(*model.Config, *model.Config) + + newStore func() store.Store htmlTemplateWatcher *utils.HTMLTemplateWatcher sessionCache *utils.Cache @@ -88,9 +91,10 @@ func New(options ...Option) (*App, error) { Srv: &Server{ Router: mux.NewRouter(), }, - sessionCache: utils.NewLru(model.SESSION_CACHE_SIZE), - configFile: "config.json", - clientConfig: make(map[string]string), + sessionCache: utils.NewLru(model.SESSION_CACHE_SIZE), + configFile: "config.json", + configListeners: make(map[string]func(*model.Config, *model.Config)), + clientConfig: make(map[string]string), } for _, option := range options { @@ -103,13 +107,15 @@ func New(options ...Option) (*App, error) { } } model.AppErrorInit(utils.T) - utils.LoadGlobalConfig(app.configFile) + if err := app.LoadConfig(app.configFile); err != nil { + return nil, err + } app.EnableConfigWatch() - if err := utils.InitTranslations(utils.Cfg.LocalizationSettings); err != nil { + if err := utils.InitTranslations(app.Config().LocalizationSettings); err != nil { return nil, errors.Wrapf(err, "unable to load Mattermost translation files") } - app.configListenerId = utils.AddConfigListener(func(_, _ *model.Config) { + app.configListenerId = app.AddConfigListener(func(_, _ *model.Config) { app.configOrLicenseListener() }) app.licenseListenerId = utils.AddLicenseListener(app.configOrLicenseListener) @@ -172,7 +178,7 @@ func (a *App) Shutdown() { a.htmlTemplateWatcher.Close() } - utils.RemoveConfigListener(a.configListenerId) + a.RemoveConfigListener(a.configListenerId) utils.RemoveLicenseListener(a.licenseListenerId) l4g.Info(utils.T("api.server.stop_server.stopped.info")) @@ -302,7 +308,7 @@ func (a *App) initEnterprise() { } if ldapInterface != nil { a.Ldap = ldapInterface(a) - utils.AddConfigListener(func(_, cfg *model.Config) { + a.AddConfigListener(func(_, cfg *model.Config) { if err := utils.ValidateLdapFilter(cfg, a.Ldap); err != nil { panic(utils.T(err.Id)) } @@ -319,7 +325,7 @@ func (a *App) initEnterprise() { } if samlInterface != nil { a.Saml = samlInterface(a) - utils.AddConfigListener(func(_, cfg *model.Config) { + a.AddConfigListener(func(_, cfg *model.Config) { a.Saml.ConfigureSP() }) } @@ -329,7 +335,7 @@ func (a *App) initEnterprise() { } func (a *App) initJobs() { - a.Jobs = jobs.NewJobServer(a.Config, a.Srv.Store) + a.Jobs = jobs.NewJobServer(a, a.Srv.Store) if jobsDataRetentionJobInterface != nil { a.Jobs.DataRetentionJob = jobsDataRetentionJobInterface(a) } diff --git a/app/app_test.go b/app/app_test.go index fd24bdfd7..b686381ea 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -65,11 +65,11 @@ func TestUpdateConfig(t *testing.T) { *cfg.ServiceSettings.SiteURL = prev }) - listener := utils.AddConfigListener(func(old, current *model.Config) { + listener := th.App.AddConfigListener(func(old, current *model.Config) { assert.Equal(t, prev, *old.ServiceSettings.SiteURL) assert.Equal(t, "foo", *current.ServiceSettings.SiteURL) }) - defer utils.RemoveConfigListener(listener) + defer th.App.RemoveConfigListener(listener) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "foo" diff --git a/app/config.go b/app/config.go index 483804c99..3039d4426 100644 --- a/app/config.go +++ b/app/config.go @@ -16,29 +16,59 @@ import ( ) func (a *App) Config() *model.Config { - return utils.Cfg + if cfg := a.config.Load(); cfg != nil { + return cfg.(*model.Config) + } + return &model.Config{} } func (a *App) UpdateConfig(f func(*model.Config)) { - old := utils.Cfg.Clone() - f(utils.Cfg) - utils.InvokeGlobalConfigListeners(old, utils.Cfg) + old := a.Config() + updated := old.Clone() + f(updated) + a.config.Store(updated) + utils.Cfg = updated + a.InvokeConfigListeners(old, updated) } func (a *App) PersistConfig() { utils.SaveConfig(a.ConfigFileName(), a.Config()) } -func (a *App) ReloadConfig() { +func (a *App) LoadConfig(configFile string) *model.AppError { + old := a.Config() + + cfg, configPath, err := utils.LoadConfig(configFile) + if err != nil { + return err + } + + a.configFile = configPath + + utils.ConfigureLog(&cfg.LogSettings) + + a.config.Store(cfg) + utils.Cfg = cfg + + utils.SetSiteURL(*cfg.ServiceSettings.SiteURL) + + a.InvokeConfigListeners(old, cfg) + return nil +} + +func (a *App) ReloadConfig() *model.AppError { debug.FreeOSMemory() - utils.LoadGlobalConfig(a.ConfigFileName()) + if err := a.LoadConfig(a.configFile); err != nil { + return err + } // start/restart email batching job if necessary a.InitEmailBatching() + return nil } func (a *App) ConfigFileName() string { - return utils.CfgFileName + return a.configFile } func (a *App) ClientConfig() map[string]string { @@ -51,7 +81,9 @@ func (a *App) ClientConfigHash() string { func (a *App) EnableConfigWatch() { if a.configWatcher == nil && !a.disableConfigWatch { - configWatcher, err := utils.NewConfigWatcher(utils.CfgFileName) + configWatcher, err := utils.NewConfigWatcher(a.ConfigFileName(), func() { + a.ReloadConfig() + }) if err != nil { l4g.Error(err) } @@ -66,8 +98,73 @@ func (a *App) DisableConfigWatch() { } } +// Registers a function with a given to be called when the config is reloaded and may have changed. The function +// will be called with two arguments: the old config and the new config. AddConfigListener returns a unique ID +// for the listener that can later be used to remove it. +func (a *App) AddConfigListener(listener func(*model.Config, *model.Config)) string { + id := model.NewId() + a.configListeners[id] = listener + return id +} + +// Removes a listener function by the unique ID returned when AddConfigListener was called +func (a *App) RemoveConfigListener(id string) { + delete(a.configListeners, id) +} + +func (a *App) InvokeConfigListeners(old, current *model.Config) { + for _, listener := range a.configListeners { + listener(old, current) + } +} + func (a *App) regenerateClientConfig() { a.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId()) clientConfigJSON, _ := json.Marshal(a.clientConfig) a.clientConfigHash = fmt.Sprintf("%x", md5.Sum(clientConfigJSON)) } + +func (a *App) Desanitize(cfg *model.Config) { + actual := a.Config() + + if cfg.LdapSettings.BindPassword != nil && *cfg.LdapSettings.BindPassword == model.FAKE_SETTING { + *cfg.LdapSettings.BindPassword = *actual.LdapSettings.BindPassword + } + + if *cfg.FileSettings.PublicLinkSalt == model.FAKE_SETTING { + *cfg.FileSettings.PublicLinkSalt = *actual.FileSettings.PublicLinkSalt + } + if cfg.FileSettings.AmazonS3SecretAccessKey == model.FAKE_SETTING { + cfg.FileSettings.AmazonS3SecretAccessKey = actual.FileSettings.AmazonS3SecretAccessKey + } + + if cfg.EmailSettings.InviteSalt == model.FAKE_SETTING { + cfg.EmailSettings.InviteSalt = actual.EmailSettings.InviteSalt + } + if cfg.EmailSettings.SMTPPassword == model.FAKE_SETTING { + cfg.EmailSettings.SMTPPassword = actual.EmailSettings.SMTPPassword + } + + if cfg.GitLabSettings.Secret == model.FAKE_SETTING { + cfg.GitLabSettings.Secret = actual.GitLabSettings.Secret + } + + if *cfg.SqlSettings.DataSource == model.FAKE_SETTING { + *cfg.SqlSettings.DataSource = *actual.SqlSettings.DataSource + } + if cfg.SqlSettings.AtRestEncryptKey == model.FAKE_SETTING { + cfg.SqlSettings.AtRestEncryptKey = actual.SqlSettings.AtRestEncryptKey + } + + if *cfg.ElasticsearchSettings.Password == model.FAKE_SETTING { + *cfg.ElasticsearchSettings.Password = *actual.ElasticsearchSettings.Password + } + + for i := range cfg.SqlSettings.DataSourceReplicas { + cfg.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] + } + + for i := range cfg.SqlSettings.DataSourceSearchReplicas { + cfg.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] + } +} diff --git a/app/config_test.go b/app/config_test.go new file mode 100644 index 000000000..e3d50b958 --- /dev/null +++ b/app/config_test.go @@ -0,0 +1,56 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "testing" + + "github.com/mattermost/mattermost-server/model" +) + +func TestConfigListener(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + originalSiteName := th.App.Config().TeamSettings.SiteName + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.TeamSettings.SiteName = "test123" + }) + + listenerCalled := false + listener := func(oldConfig *model.Config, newConfig *model.Config) { + if listenerCalled { + t.Fatal("listener called twice") + } + + if oldConfig.TeamSettings.SiteName != "test123" { + t.Fatal("old config contains incorrect site name") + } else if newConfig.TeamSettings.SiteName != originalSiteName { + t.Fatal("new config contains incorrect site name") + } + + listenerCalled = true + } + listenerId := th.App.AddConfigListener(listener) + defer th.App.RemoveConfigListener(listenerId) + + listener2Called := false + listener2 := func(oldConfig *model.Config, newConfig *model.Config) { + if listener2Called { + t.Fatal("listener2 called twice") + } + + listener2Called = true + } + listener2Id := th.App.AddConfigListener(listener2) + defer th.App.RemoveConfigListener(listener2Id) + + th.App.ReloadConfig() + + if !listenerCalled { + t.Fatal("listener should've been called") + } else if !listener2Called { + t.Fatal("listener 2 should've been called") + } +} diff --git a/app/plugin.go b/app/plugin.go index 189ca9b09..d96e6e990 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -54,7 +54,7 @@ func (a *App) initBuiltInPlugins() { } p.Initialize(api) } - utils.AddConfigListener(func(before, after *model.Config) { + a.AddConfigListener(func(before, after *model.Config) { for _, p := range plugins { p.OnConfigurationChange() } @@ -407,8 +407,8 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug } } - utils.RemoveConfigListener(a.PluginConfigListenerId) - a.PluginConfigListenerId = utils.AddConfigListener(func(prevCfg, cfg *model.Config) { + a.RemoveConfigListener(a.PluginConfigListenerId) + a.PluginConfigListenerId = a.AddConfigListener(func(prevCfg, cfg *model.Config) { if a.PluginEnv == nil { return } @@ -489,7 +489,7 @@ func (a *App) ShutDownPlugins() { for _, err := range a.PluginEnv.Shutdown() { l4g.Error(err.Error()) } - utils.RemoveConfigListener(a.PluginConfigListenerId) + a.RemoveConfigListener(a.PluginConfigListenerId) a.PluginConfigListenerId = "" a.PluginEnv = nil } diff --git a/cmd/platform/mattermost_test.go b/cmd/platform/mattermost_test.go index eba45801a..7246d620f 100644 --- a/cmd/platform/mattermost_test.go +++ b/cmd/platform/mattermost_test.go @@ -20,7 +20,8 @@ func TestConfigFlag(t *testing.T) { defer os.RemoveAll(dir) utils.TranslationsPreInit() - config := utils.LoadGlobalConfig("config.json") + config, _, err := utils.LoadConfig("config.json") + require.Nil(t, err) configPath := filepath.Join(dir, "foo.json") require.NoError(t, ioutil.WriteFile(configPath, []byte(config.ToJson()), 0600)) diff --git a/cmd/platform/message_export_test.go b/cmd/platform/message_export_test.go index 211c1ca3c..386aa4268 100644 --- a/cmd/platform/message_export_test.go +++ b/cmd/platform/message_export_test.go @@ -4,15 +4,15 @@ package main import ( - "testing" - "io/ioutil" "os" "path/filepath" + "testing" + + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" - "github.com/stretchr/testify/require" ) // There are no tests that actually run the Message Export job, because it can take a long time to complete depending @@ -56,7 +56,8 @@ func writeTempConfig(t *testing.T, isMessageExportEnabled bool) string { require.NoError(t, err) utils.TranslationsPreInit() - config := utils.LoadGlobalConfig("config.json") + config, _, appErr := utils.LoadConfig("config.json") + require.Nil(t, appErr) config.MessageExportSettings.EnableExport = model.NewBool(isMessageExportEnabled) configPath := filepath.Join(dir, "foo.json") require.NoError(t, ioutil.WriteFile(configPath, []byte(config.ToJson()), 0600)) diff --git a/cmd/platform/server.go b/cmd/platform/server.go index cdf12cf71..e36abdaf9 100644 --- a/cmd/platform/server.go +++ b/cmd/platform/server.go @@ -75,7 +75,7 @@ func runServer(configFileLocation string, disableConfigWatch bool) { } a.InitPlugins(*a.Config().PluginSettings.Directory, *a.Config().PluginSettings.ClientDirectory, nil) - utils.AddConfigListener(func(prevCfg, cfg *model.Config) { + a.AddConfigListener(func(prevCfg, cfg *model.Config) { if *cfg.PluginSettings.Enable { a.InitPlugins(*cfg.PluginSettings.Directory, *a.Config().PluginSettings.ClientDirectory, nil) } else { diff --git a/jobs/schedulers.go b/jobs/schedulers.go index bec53a49b..839cbbae8 100644 --- a/jobs/schedulers.go +++ b/jobs/schedulers.go @@ -10,7 +10,6 @@ import ( l4g "github.com/alecthomas/log4go" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" ) type Schedulers struct { @@ -56,7 +55,7 @@ func (srv *JobServer) InitSchedulers() *Schedulers { } func (schedulers *Schedulers) Start() *Schedulers { - schedulers.listenerId = utils.AddConfigListener(schedulers.handleConfigChange) + schedulers.listenerId = schedulers.jobs.ConfigService.AddConfigListener(schedulers.handleConfigChange) go func() { schedulers.startOnce.Do(func() { diff --git a/jobs/server.go b/jobs/server.go index 777b02a26..9a59fad3f 100644 --- a/jobs/server.go +++ b/jobs/server.go @@ -12,11 +12,25 @@ import ( "github.com/mattermost/mattermost-server/utils" ) +type ConfigService interface { + Config() *model.Config + AddConfigListener(func(old, current *model.Config)) string + RemoveConfigListener(string) +} + +type StaticConfigService struct { + Cfg *model.Config +} + +func (s StaticConfigService) Config() *model.Config { return s.Cfg } +func (StaticConfigService) AddConfigListener(func(old, current *model.Config)) string { return "" } +func (StaticConfigService) RemoveConfigListener(string) {} + type JobServer struct { - Config model.ConfigFunc - Store store.Store - Workers *Workers - Schedulers *Schedulers + ConfigService ConfigService + Store store.Store + Workers *Workers + Schedulers *Schedulers DataRetentionJob ejobs.DataRetentionJobInterface MessageExportJob ejobs.MessageExportJobInterface @@ -25,13 +39,17 @@ type JobServer struct { LdapSync ejobs.LdapSyncInterface } -func NewJobServer(config model.ConfigFunc, store store.Store) *JobServer { +func NewJobServer(configService ConfigService, store store.Store) *JobServer { return &JobServer{ - Config: config, - Store: store, + ConfigService: configService, + Store: store, } } +func (srv *JobServer) Config() *model.Config { + return srv.ConfigService.Config() +} + func (srv *JobServer) LoadLicense() { licenseId := "" if result := <-srv.Store.System().Get(); result.Err == nil { diff --git a/jobs/workers.go b/jobs/workers.go index 3abd7131c..ca34855e5 100644 --- a/jobs/workers.go +++ b/jobs/workers.go @@ -8,13 +8,12 @@ import ( l4g "github.com/alecthomas/log4go" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" ) type Workers struct { - startOnce sync.Once - Config model.ConfigFunc - Watcher *Watcher + startOnce sync.Once + ConfigService ConfigService + Watcher *Watcher DataRetention model.Worker MessageExport model.Worker @@ -27,7 +26,7 @@ type Workers struct { func (srv *JobServer) InitWorkers() *Workers { workers := &Workers{ - Config: srv.Config, + ConfigService: srv.ConfigService, } workers.Watcher = srv.MakeWatcher(workers, DEFAULT_WATCHER_POLLING_INTERVAL) @@ -58,30 +57,30 @@ func (workers *Workers) Start() *Workers { l4g.Info("Starting workers") workers.startOnce.Do(func() { - if workers.DataRetention != nil && (*workers.Config().DataRetentionSettings.EnableMessageDeletion || *workers.Config().DataRetentionSettings.EnableFileDeletion) { + if workers.DataRetention != nil && (*workers.ConfigService.Config().DataRetentionSettings.EnableMessageDeletion || *workers.ConfigService.Config().DataRetentionSettings.EnableFileDeletion) { go workers.DataRetention.Run() } - if workers.MessageExport != nil && *workers.Config().MessageExportSettings.EnableExport { + if workers.MessageExport != nil && *workers.ConfigService.Config().MessageExportSettings.EnableExport { go workers.MessageExport.Run() } - if workers.ElasticsearchIndexing != nil && *workers.Config().ElasticsearchSettings.EnableIndexing { + if workers.ElasticsearchIndexing != nil && *workers.ConfigService.Config().ElasticsearchSettings.EnableIndexing { go workers.ElasticsearchIndexing.Run() } - if workers.ElasticsearchAggregation != nil && *workers.Config().ElasticsearchSettings.EnableIndexing { + if workers.ElasticsearchAggregation != nil && *workers.ConfigService.Config().ElasticsearchSettings.EnableIndexing { go workers.ElasticsearchAggregation.Run() } - if workers.LdapSync != nil && *workers.Config().LdapSettings.EnableSync { + if workers.LdapSync != nil && *workers.ConfigService.Config().LdapSettings.EnableSync { go workers.LdapSync.Run() } go workers.Watcher.Start() }) - workers.listenerId = utils.AddConfigListener(workers.handleConfigChange) + workers.listenerId = workers.ConfigService.AddConfigListener(workers.handleConfigChange) return workers } @@ -129,27 +128,27 @@ func (workers *Workers) handleConfigChange(oldConfig *model.Config, newConfig *m } func (workers *Workers) Stop() *Workers { - utils.RemoveConfigListener(workers.listenerId) + workers.ConfigService.RemoveConfigListener(workers.listenerId) workers.Watcher.Stop() - if workers.DataRetention != nil && (*workers.Config().DataRetentionSettings.EnableMessageDeletion || *workers.Config().DataRetentionSettings.EnableFileDeletion) { + if workers.DataRetention != nil && (*workers.ConfigService.Config().DataRetentionSettings.EnableMessageDeletion || *workers.ConfigService.Config().DataRetentionSettings.EnableFileDeletion) { workers.DataRetention.Stop() } - if workers.MessageExport != nil && *workers.Config().MessageExportSettings.EnableExport { + if workers.MessageExport != nil && *workers.ConfigService.Config().MessageExportSettings.EnableExport { workers.MessageExport.Stop() } - if workers.ElasticsearchIndexing != nil && *workers.Config().ElasticsearchSettings.EnableIndexing { + if workers.ElasticsearchIndexing != nil && *workers.ConfigService.Config().ElasticsearchSettings.EnableIndexing { workers.ElasticsearchIndexing.Stop() } - if workers.ElasticsearchAggregation != nil && *workers.Config().ElasticsearchSettings.EnableIndexing { + if workers.ElasticsearchAggregation != nil && *workers.ConfigService.Config().ElasticsearchSettings.EnableIndexing { workers.ElasticsearchAggregation.Stop() } - if workers.LdapSync != nil && *workers.Config().LdapSettings.EnableSync { + if workers.LdapSync != nil && *workers.ConfigService.Config().LdapSettings.EnableSync { workers.LdapSync.Stop() } diff --git a/utils/config.go b/utils/config.go index a692d82d0..7863a0c7c 100644 --- a/utils/config.go +++ b/utils/config.go @@ -4,7 +4,6 @@ package utils import ( - "crypto/md5" "encoding/json" "fmt" "io" @@ -14,7 +13,6 @@ import ( "path/filepath" "strconv" "strings" - "sync" l4g "github.com/alecthomas/log4go" "github.com/fsnotify/fsnotify" @@ -35,12 +33,9 @@ const ( LOG_FILENAME = "mattermost.log" ) -var cfgMutex = &sync.Mutex{} -var Cfg *model.Config = &model.Config{} -var CfgHash = "" -var CfgFileName string = "" var originalDisableDebugLvl l4g.Level = l4g.DEBUG var siteURL = "" +var Cfg *model.Config func GetSiteURL() string { return siteURL @@ -50,22 +45,6 @@ func SetSiteURL(url string) { siteURL = strings.TrimRight(url, "/") } -var cfgListeners = map[string]func(*model.Config, *model.Config){} - -// Registers a function with a given to be called when the config is reloaded and may have changed. The function -// will be called with two arguments: the old config and the new config. AddConfigListener returns a unique ID -// for the listener that can later be used to remove it. -func AddConfigListener(listener func(*model.Config, *model.Config)) string { - id := model.NewId() - cfgListeners[id] = listener - return id -} - -// Removes a listener function by the unique ID returned when AddConfigListener was called -func RemoveConfigListener(id string) { - delete(cfgListeners, id) -} - // FindConfigFile attempts to find an existing configuration file. fileName can be an absolute or // relative path or name such as "/opt/mattermost/config.json" or simply "config.json". An empty // string is returned if no configuration is found. @@ -103,8 +82,6 @@ func FindDir(dir string) (string, bool) { } func DisableDebugLogForTest() { - cfgMutex.Lock() - defer cfgMutex.Unlock() if l4g.Global["stdout"] != nil { originalDisableDebugLvl = l4g.Global["stdout"].Level l4g.Global["stdout"].Level = l4g.ERROR @@ -112,8 +89,6 @@ func DisableDebugLogForTest() { } func EnableDebugLogForTest() { - cfgMutex.Lock() - defer cfgMutex.Unlock() if l4g.Global["stdout"] != nil { l4g.Global["stdout"].Level = originalDisableDebugLvl } @@ -123,12 +98,12 @@ func ConfigureCmdLineLog() { ls := model.LogSettings{} ls.EnableConsole = true ls.ConsoleLevel = "WARN" - configureLog(&ls) + ConfigureLog(&ls) } // TODO: this code initializes console and file logging. It will eventually be replaced by JSON logging in logger/logger.go // See PLT-3893 for more information -func configureLog(s *model.LogSettings) { +func ConfigureLog(s *model.LogSettings) { l4g.Close() @@ -182,9 +157,6 @@ func GetLogFileLocation(fileLocation string) string { } func SaveConfig(fileName string, config *model.Config) *model.AppError { - cfgMutex.Lock() - defer cfgMutex.Unlock() - b, err := json.MarshalIndent(config, "", " ") if err != nil { return model.NewAppError("SaveConfig", "utils.config.save_config.saving.app_error", @@ -206,7 +178,7 @@ type ConfigWatcher struct { closed chan struct{} } -func NewConfigWatcher(cfgFileName string) (*ConfigWatcher, error) { +func NewConfigWatcher(cfgFileName string, f func()) (*ConfigWatcher, error) { watcher, err := fsnotify.NewWatcher() if err != nil { return nil, errors.Wrapf(err, "failed to create config watcher for file: "+cfgFileName) @@ -235,7 +207,7 @@ func NewConfigWatcher(cfgFileName string) (*ConfigWatcher, error) { l4g.Info(fmt.Sprintf("Config file watcher detected a change reloading %v", cfgFileName)) if _, configReadErr := ReadConfigFile(cfgFileName, true); configReadErr == nil { - LoadGlobalConfig(cfgFileName) + f() } else { l4g.Error(fmt.Sprintf("Failed to read while watching config file at %v with err=%v", cfgFileName, configReadErr.Error())) } @@ -322,7 +294,7 @@ func EnsureConfigFile(fileName string) (string, error) { // LoadConfig will try to search around for the corresponding config file. It will search // /tmp/fileName then attempt ./config/fileName, then ../config/fileName and last it will look at // fileName. -func LoadConfig(fileName string) (configPath string, config *model.Config, appErr *model.AppError) { +func LoadConfig(fileName string) (config *model.Config, configPath string, appErr *model.AppError) { if fileName != filepath.Base(fileName) { configPath = fileName } else { @@ -346,7 +318,7 @@ func LoadConfig(fileName string) (configPath string, config *model.Config, appEr config.SetDefaults() if err := config.IsValid(); err != nil { - return "", nil, err + return nil, "", err } if needSave { @@ -368,41 +340,7 @@ func LoadConfig(fileName string) (configPath string, config *model.Config, appEr } } - return configPath, config, nil -} - -// XXX: This is deprecated. Use LoadConfig instead if possible. -func LoadGlobalConfig(fileName string) *model.Config { - configPath, config, err := LoadConfig(fileName) - if err != nil { - fmt.Fprintln(os.Stderr, err.SystemMessage(T)) - os.Exit(1) - } - - cfgMutex.Lock() - defer cfgMutex.Unlock() - - CfgFileName = configPath - - configureLog(&config.LogSettings) - - // Cfg should never be null - oldConfig := *Cfg - - Cfg = config - CfgHash = fmt.Sprintf("%x", md5.Sum([]byte(Cfg.ToJson()))) - - SetSiteURL(*Cfg.ServiceSettings.SiteURL) - - InvokeGlobalConfigListeners(&oldConfig, config) - - return config -} - -func InvokeGlobalConfigListeners(old, current *model.Config) { - for _, listener := range cfgListeners { - listener(old, current) - } + return config, configPath, nil } func GenerateClientConfig(c *model.Config, diagnosticId string) map[string]string { @@ -659,46 +597,3 @@ func ValidateLocales(cfg *model.Config) *model.AppError { return err } - -func Desanitize(cfg *model.Config) { - if cfg.LdapSettings.BindPassword != nil && *cfg.LdapSettings.BindPassword == model.FAKE_SETTING { - *cfg.LdapSettings.BindPassword = *Cfg.LdapSettings.BindPassword - } - - if *cfg.FileSettings.PublicLinkSalt == model.FAKE_SETTING { - *cfg.FileSettings.PublicLinkSalt = *Cfg.FileSettings.PublicLinkSalt - } - if cfg.FileSettings.AmazonS3SecretAccessKey == model.FAKE_SETTING { - cfg.FileSettings.AmazonS3SecretAccessKey = Cfg.FileSettings.AmazonS3SecretAccessKey - } - - if cfg.EmailSettings.InviteSalt == model.FAKE_SETTING { - cfg.EmailSettings.InviteSalt = Cfg.EmailSettings.InviteSalt - } - if cfg.EmailSettings.SMTPPassword == model.FAKE_SETTING { - cfg.EmailSettings.SMTPPassword = Cfg.EmailSettings.SMTPPassword - } - - if cfg.GitLabSettings.Secret == model.FAKE_SETTING { - cfg.GitLabSettings.Secret = Cfg.GitLabSettings.Secret - } - - if *cfg.SqlSettings.DataSource == model.FAKE_SETTING { - *cfg.SqlSettings.DataSource = *Cfg.SqlSettings.DataSource - } - if cfg.SqlSettings.AtRestEncryptKey == model.FAKE_SETTING { - cfg.SqlSettings.AtRestEncryptKey = Cfg.SqlSettings.AtRestEncryptKey - } - - if *cfg.ElasticsearchSettings.Password == model.FAKE_SETTING { - *cfg.ElasticsearchSettings.Password = *Cfg.ElasticsearchSettings.Password - } - - for i := range cfg.SqlSettings.DataSourceReplicas { - cfg.SqlSettings.DataSourceReplicas[i] = Cfg.SqlSettings.DataSourceReplicas[i] - } - - for i := range cfg.SqlSettings.DataSourceSearchReplicas { - cfg.SqlSettings.DataSourceSearchReplicas[i] = Cfg.SqlSettings.DataSourceSearchReplicas[i] - } -} diff --git a/utils/config_test.go b/utils/config_test.go index 8a89940e8..9abc56d5e 100644 --- a/utils/config_test.go +++ b/utils/config_test.go @@ -9,18 +9,16 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "github.com/mattermost/mattermost-server/model" ) func TestConfig(t *testing.T) { TranslationsPreInit() - LoadGlobalConfig("config.json") - InitTranslations(Cfg.LocalizationSettings) + cfg, _, err := LoadConfig("config.json") + require.Nil(t, err) + InitTranslations(cfg.LocalizationSettings) } func TestFindConfigFile(t *testing.T) { @@ -47,21 +45,22 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_READTIMEOUT", "400") TranslationsPreInit() - LoadGlobalConfig("config.json") + cfg, cfgPath, err := LoadConfig("config.json") + require.Nil(t, err) - if Cfg.TeamSettings.SiteName != "From Enviroment" { + if cfg.TeamSettings.SiteName != "From Enviroment" { t.Fatal("Couldn't read config from enviroment var") } - if *Cfg.TeamSettings.CustomBrandText != "Custom Brand" { + if *cfg.TeamSettings.CustomBrandText != "Custom Brand" { t.Fatal("Couldn't read config from enviroment var") } - if *Cfg.ServiceSettings.EnableCommands { + if *cfg.ServiceSettings.EnableCommands { t.Fatal("Couldn't read config from enviroment var") } - if *Cfg.ServiceSettings.ReadTimeout != 400 { + if *cfg.ServiceSettings.ReadTimeout != 400 { t.Fatal("Couldn't read config from enviroment var") } @@ -70,169 +69,67 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Unsetenv("MM_SERVICESETTINGS_ENABLECOMMANDS") os.Unsetenv("MM_SERVICESETTINGS_READTIMEOUT") - Cfg.TeamSettings.SiteName = "Mattermost" - *Cfg.ServiceSettings.SiteURL = "" - *Cfg.ServiceSettings.EnableCommands = true - *Cfg.ServiceSettings.ReadTimeout = 300 - SaveConfig(CfgFileName, Cfg) + cfg.TeamSettings.SiteName = "Mattermost" + *cfg.ServiceSettings.SiteURL = "" + *cfg.ServiceSettings.EnableCommands = true + *cfg.ServiceSettings.ReadTimeout = 300 + SaveConfig(cfgPath, cfg) - LoadGlobalConfig("config.json") + cfg, _, err = LoadConfig("config.json") + require.Nil(t, err) - if Cfg.TeamSettings.SiteName != "Mattermost" { + if cfg.TeamSettings.SiteName != "Mattermost" { t.Fatal("should have been reset") } } -func TestRedirectStdLog(t *testing.T) { - TranslationsPreInit() - LoadGlobalConfig("config.json") - InitTranslations(Cfg.LocalizationSettings) - - log := NewRedirectStdLog("test", false) - - log.Println("[DEBUG] this is a message") - log.Println("[DEBG] this is a message") - log.Println("[WARN] this is a message") - log.Println("[ERROR] this is a message") - log.Println("[EROR] this is a message") - log.Println("[ERR] this is a message") - log.Println("[INFO] this is a message") - log.Println("this is a message") - - time.Sleep(time.Second * 1) -} - -func TestAddRemoveConfigListener(t *testing.T) { - numIntitialCfgListeners := len(cfgListeners) - - id1 := AddConfigListener(func(*model.Config, *model.Config) { - }) - if len(cfgListeners) != numIntitialCfgListeners+1 { - t.Fatal("should now have 1 listener") - } - - id2 := AddConfigListener(func(*model.Config, *model.Config) { - }) - if len(cfgListeners) != numIntitialCfgListeners+2 { - t.Fatal("should now have 2 listeners") - } - - RemoveConfigListener(id1) - if len(cfgListeners) != numIntitialCfgListeners+1 { - t.Fatal("should've removed first listener") - } - - RemoveConfigListener(id2) - if len(cfgListeners) != numIntitialCfgListeners { - t.Fatal("should've removed both listeners") - } -} - -func TestConfigListener(t *testing.T) { - TranslationsPreInit() - LoadGlobalConfig("config.json") - - SiteName := Cfg.TeamSettings.SiteName - defer func() { - Cfg.TeamSettings.SiteName = SiteName - SaveConfig(CfgFileName, Cfg) - }() - Cfg.TeamSettings.SiteName = "test123" - - listenerCalled := false - listener := func(oldConfig *model.Config, newConfig *model.Config) { - if listenerCalled { - t.Fatal("listener called twice") - } - - if oldConfig.TeamSettings.SiteName != "test123" { - t.Fatal("old config contains incorrect site name") - } else if newConfig.TeamSettings.SiteName != "Mattermost" { - t.Fatal("new config contains incorrect site name") - } - - listenerCalled = true - } - listenerId := AddConfigListener(listener) - defer RemoveConfigListener(listenerId) - - listener2Called := false - listener2 := func(oldConfig *model.Config, newConfig *model.Config) { - if listener2Called { - t.Fatal("listener2 called twice") - } - - listener2Called = true - } - listener2Id := AddConfigListener(listener2) - defer RemoveConfigListener(listener2Id) - - LoadGlobalConfig("config.json") - - if !listenerCalled { - t.Fatal("listener should've been called") - } else if !listener2Called { - t.Fatal("listener 2 should've been called") - } -} - func TestValidateLocales(t *testing.T) { TranslationsPreInit() - LoadGlobalConfig("config.json") - - defaultServerLocale := *Cfg.LocalizationSettings.DefaultServerLocale - defaultClientLocale := *Cfg.LocalizationSettings.DefaultClientLocale - availableLocales := *Cfg.LocalizationSettings.AvailableLocales + cfg, _, err := LoadConfig("config.json") + require.Nil(t, err) - defer func() { - *Cfg.LocalizationSettings.DefaultClientLocale = defaultClientLocale - *Cfg.LocalizationSettings.DefaultServerLocale = defaultServerLocale - *Cfg.LocalizationSettings.AvailableLocales = availableLocales - }() + *cfg.LocalizationSettings.DefaultServerLocale = "en" + *cfg.LocalizationSettings.DefaultClientLocale = "en" + *cfg.LocalizationSettings.AvailableLocales = "" - *Cfg.LocalizationSettings.DefaultServerLocale = "en" - *Cfg.LocalizationSettings.DefaultClientLocale = "en" - *Cfg.LocalizationSettings.AvailableLocales = "" - - // t.Logf("*Cfg.LocalizationSettings.DefaultClientLocale: %+v", *Cfg.LocalizationSettings.DefaultClientLocale) - if err := ValidateLocales(Cfg); err != nil { + // t.Logf("*cfg.LocalizationSettings.DefaultClientLocale: %+v", *cfg.LocalizationSettings.DefaultClientLocale) + if err := ValidateLocales(cfg); err != nil { t.Fatal("Should have not returned an error") } // validate DefaultServerLocale - *Cfg.LocalizationSettings.DefaultServerLocale = "junk" - if err := ValidateLocales(Cfg); err != nil { - if *Cfg.LocalizationSettings.DefaultServerLocale != "en" { + *cfg.LocalizationSettings.DefaultServerLocale = "junk" + if err := ValidateLocales(cfg); err != nil { + if *cfg.LocalizationSettings.DefaultServerLocale != "en" { t.Fatal("DefaultServerLocale should have assigned to en as a default value") } } else { - t.Fatal("Should have returned an error validating DefaultServerLocale") } - *Cfg.LocalizationSettings.DefaultServerLocale = "" - if err := ValidateLocales(Cfg); err != nil { - if *Cfg.LocalizationSettings.DefaultServerLocale != "en" { + *cfg.LocalizationSettings.DefaultServerLocale = "" + if err := ValidateLocales(cfg); err != nil { + if *cfg.LocalizationSettings.DefaultServerLocale != "en" { t.Fatal("DefaultServerLocale should have assigned to en as a default value") } } else { t.Fatal("Should have returned an error validating DefaultServerLocale") } - *Cfg.LocalizationSettings.AvailableLocales = "en" - *Cfg.LocalizationSettings.DefaultServerLocale = "de" - if err := ValidateLocales(Cfg); err != nil { - if strings.Contains(*Cfg.LocalizationSettings.AvailableLocales, *Cfg.LocalizationSettings.DefaultServerLocale) { + *cfg.LocalizationSettings.AvailableLocales = "en" + *cfg.LocalizationSettings.DefaultServerLocale = "de" + if err := ValidateLocales(cfg); err != nil { + if strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale) { t.Fatal("DefaultServerLocale should not be added to AvailableLocales") } t.Fatal("Should have not returned an error validating DefaultServerLocale") } // validate DefaultClientLocale - *Cfg.LocalizationSettings.AvailableLocales = "" - *Cfg.LocalizationSettings.DefaultClientLocale = "junk" - if err := ValidateLocales(Cfg); err != nil { - if *Cfg.LocalizationSettings.DefaultClientLocale != "en" { + *cfg.LocalizationSettings.AvailableLocales = "" + *cfg.LocalizationSettings.DefaultClientLocale = "junk" + if err := ValidateLocales(cfg); err != nil { + if *cfg.LocalizationSettings.DefaultClientLocale != "en" { t.Fatal("DefaultClientLocale should have assigned to en as a default value") } } else { @@ -240,19 +137,19 @@ func TestValidateLocales(t *testing.T) { t.Fatal("Should have returned an error validating DefaultClientLocale") } - *Cfg.LocalizationSettings.DefaultClientLocale = "" - if err := ValidateLocales(Cfg); err != nil { - if *Cfg.LocalizationSettings.DefaultClientLocale != "en" { + *cfg.LocalizationSettings.DefaultClientLocale = "" + if err := ValidateLocales(cfg); err != nil { + if *cfg.LocalizationSettings.DefaultClientLocale != "en" { t.Fatal("DefaultClientLocale should have assigned to en as a default value") } } else { t.Fatal("Should have returned an error validating DefaultClientLocale") } - *Cfg.LocalizationSettings.AvailableLocales = "en" - *Cfg.LocalizationSettings.DefaultClientLocale = "de" - if err := ValidateLocales(Cfg); err != nil { - if !strings.Contains(*Cfg.LocalizationSettings.AvailableLocales, *Cfg.LocalizationSettings.DefaultClientLocale) { + *cfg.LocalizationSettings.AvailableLocales = "en" + *cfg.LocalizationSettings.DefaultClientLocale = "de" + if err := ValidateLocales(cfg); err != nil { + if !strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale) { t.Fatal("DefaultClientLocale should have added to AvailableLocales") } } else { @@ -260,34 +157,34 @@ func TestValidateLocales(t *testing.T) { } // validate AvailableLocales - *Cfg.LocalizationSettings.DefaultServerLocale = "en" - *Cfg.LocalizationSettings.DefaultClientLocale = "en" - *Cfg.LocalizationSettings.AvailableLocales = "junk" - if err := ValidateLocales(Cfg); err != nil { - if *Cfg.LocalizationSettings.AvailableLocales != "" { + *cfg.LocalizationSettings.DefaultServerLocale = "en" + *cfg.LocalizationSettings.DefaultClientLocale = "en" + *cfg.LocalizationSettings.AvailableLocales = "junk" + if err := ValidateLocales(cfg); err != nil { + if *cfg.LocalizationSettings.AvailableLocales != "" { t.Fatal("AvailableLocales should have assigned to empty string as a default value") } } else { t.Fatal("Should have returned an error validating AvailableLocales") } - *Cfg.LocalizationSettings.AvailableLocales = "en,de,junk" - if err := ValidateLocales(Cfg); err != nil { - if *Cfg.LocalizationSettings.AvailableLocales != "" { + *cfg.LocalizationSettings.AvailableLocales = "en,de,junk" + if err := ValidateLocales(cfg); err != nil { + if *cfg.LocalizationSettings.AvailableLocales != "" { t.Fatal("AvailableLocales should have assigned to empty string as a default value") } } else { t.Fatal("Should have returned an error validating AvailableLocales") } - *Cfg.LocalizationSettings.DefaultServerLocale = "fr" - *Cfg.LocalizationSettings.DefaultClientLocale = "de" - *Cfg.LocalizationSettings.AvailableLocales = "en" - if err := ValidateLocales(Cfg); err != nil { - if strings.Contains(*Cfg.LocalizationSettings.AvailableLocales, *Cfg.LocalizationSettings.DefaultServerLocale) { + *cfg.LocalizationSettings.DefaultServerLocale = "fr" + *cfg.LocalizationSettings.DefaultClientLocale = "de" + *cfg.LocalizationSettings.AvailableLocales = "en" + if err := ValidateLocales(cfg); err != nil { + if strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale) { t.Fatal("DefaultServerLocale should not be added to AvailableLocales") } - if !strings.Contains(*Cfg.LocalizationSettings.AvailableLocales, *Cfg.LocalizationSettings.DefaultClientLocale) { + if !strings.Contains(*cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale) { t.Fatal("DefaultClientLocale should have added to AvailableLocales") } } else { @@ -297,10 +194,11 @@ func TestValidateLocales(t *testing.T) { func TestGetClientConfig(t *testing.T) { TranslationsPreInit() - LoadGlobalConfig("config.json") + cfg, _, err := LoadConfig("config.json") + require.Nil(t, err) - configMap := GenerateClientConfig(Cfg, "") - if configMap["EmailNotificationContentsType"] != *Cfg.EmailSettings.EmailNotificationContentsType { + configMap := GenerateClientConfig(cfg, "") + if configMap["EmailNotificationContentsType"] != *cfg.EmailSettings.EmailNotificationContentsType { t.Fatal("EmailSettings.EmailNotificationContentsType not exposed to client config") } } diff --git a/utils/logger/logger.go b/utils/logger/logger.go index da549dcc4..558f3fe47 100644 --- a/utils/logger/logger.go +++ b/utils/logger/logger.go @@ -27,15 +27,6 @@ var debugLog = l4g.Debug var infoLog = l4g.Info var errorLog = l4g.Error -func init() { - // listens for configuration changes that we might need to respond to - utils.AddConfigListener(func(oldConfig *model.Config, newConfig *model.Config) { - infoLog("Configuration change detected, reloading log settings") - initL4g(newConfig.LogSettings) - }) - initL4g(utils.Cfg.LogSettings) -} - // assumes that ../config.go::configureLog has already been called, and has in turn called l4g.close() to clean up // any old filters that we might have previously created func initL4g(logSettings model.LogSettings) { diff --git a/utils/mail_test.go b/utils/mail_test.go index a4444eb2e..574f71f46 100644 --- a/utils/mail_test.go +++ b/utils/mail_test.go @@ -6,10 +6,13 @@ package utils import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestMailConnection(t *testing.T) { - cfg := LoadGlobalConfig("config.json") + cfg, _, err := LoadConfig("config.json") + require.Nil(t, err) if conn, err := connectToSMTPServer(cfg); err != nil { t.Log(err) @@ -32,7 +35,8 @@ func TestMailConnection(t *testing.T) { } func TestSendMailUsingConfig(t *testing.T) { - cfg := LoadGlobalConfig("config.json") + cfg, _, err := LoadConfig("config.json") + require.Nil(t, err) T = GetUserTranslations("en") var emailTo string = "test@example.com" diff --git a/utils/redirect_std_log_test.go b/utils/redirect_std_log_test.go new file mode 100644 index 000000000..cbe55c921 --- /dev/null +++ b/utils/redirect_std_log_test.go @@ -0,0 +1,24 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "testing" + "time" +) + +func TestRedirectStdLog(t *testing.T) { + log := NewRedirectStdLog("test", false) + + log.Println("[DEBUG] this is a message") + log.Println("[DEBG] this is a message") + log.Println("[WARN] this is a message") + log.Println("[ERROR] this is a message") + log.Println("[EROR] this is a message") + log.Println("[ERR] this is a message") + log.Println("[INFO] this is a message") + log.Println("this is a message") + + time.Sleep(time.Second * 1) +} -- cgit v1.2.3-1-g7c22