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 ++-- 6 files changed, 186 insertions(+), 27 deletions(-) create mode 100644 app/config_test.go (limited to 'app') 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 } -- cgit v1.2.3-1-g7c22