diff --git a/Makefile b/Makefile index 24d1dec360..e43d46311f 100644 --- a/Makefile +++ b/Makefile @@ -454,7 +454,7 @@ run-server: prepackaged-binaries validate-go-version start-docker ## Starts the @echo Running mattermost for development mkdir -p $(BUILD_WEBAPP_DIR)/dist/files - $(GO) run $(GOFLAGS) -ldflags '$(LDFLAGS)' $(PLATFORM_FILES) --disableconfigwatch 2>&1 | \ + $(GO) run $(GOFLAGS) -ldflags '$(LDFLAGS)' $(PLATFORM_FILES) 2>&1 | \ $(GO) run $(GOFLAGS) -ldflags '$(LDFLAGS)' $(PLATFORM_FILES) logs --logrus $(RUN_IN_BACKGROUND) debug-server: start-docker ## Compile and start server using delve. @@ -503,11 +503,11 @@ ifeq ($(BUILDER_GOOS_GOARCH),"windows_amd64") wmic process where "Caption='go.exe' and CommandLine like '%go.exe run%'" call terminate wmic process where "Caption='mattermost.exe' and CommandLine like '%go-build%'" call terminate else - @for PID in $$(ps -ef | grep "[g]o run" | grep "disableconfigwatch" | awk '{ print $$2 }'); do \ + @for PID in $$(ps -ef | grep "[g]o run" | awk '{ print $$2 }'); do \ echo stopping go $$PID; \ kill $$PID; \ done - @for PID in $$(ps -ef | grep "[g]o-build" | grep "disableconfigwatch" | awk '{ print $$2 }'); do \ + @for PID in $$(ps -ef | grep "[g]o-build" | awk '{ print $$2 }'); do \ echo stopping mattermost $$PID; \ kill $$PID; \ done @@ -536,7 +536,7 @@ restart-client: | stop-client run-client ## Restarts the webapp. run-job-server: ## Runs the background job server. @echo Running job server for development - $(GO) run $(GOFLAGS) -ldflags '$(LDFLAGS)' $(PLATFORM_FILES) jobserver --disableconfigwatch & + $(GO) run $(GOFLAGS) -ldflags '$(LDFLAGS)' $(PLATFORM_FILES) jobserver & config-ldap: ## Configures LDAP. @echo Setting up configuration for local LDAP diff --git a/api4/config_test.go b/api4/config_test.go index 1c1aa997ee..2c7ea71fa6 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -770,11 +770,11 @@ func TestMigrateConfig(t *testing.T) { }) th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - f, err := config.NewStoreFromDSN("from.json", false, false, nil) + f, err := config.NewStoreFromDSN("from.json", false, nil) require.NoError(t, err) defer f.RemoveFile("from.json") - _, err = config.NewStoreFromDSN("to.json", false, false, nil) + _, err = config.NewStoreFromDSN("to.json", false, nil) require.NoError(t, err) defer f.RemoveFile("to.json") diff --git a/app/options.go b/app/options.go index c09fb3c82d..d8b1ce9891 100644 --- a/app/options.go +++ b/app/options.go @@ -43,9 +43,9 @@ func StoreOverride(override interface{}) Option { // or a database connection string. It receives as well a set of // custom defaults that will be applied for any unset property of the // config loaded from the dsn on top of the normal defaults -func Config(dsn string, watch, readOnly bool, configDefaults *model.Config) Option { +func Config(dsn string, readOnly bool, configDefaults *model.Config) Option { return func(s *Server) error { - configStore, err := config.NewStoreFromDSN(dsn, watch, readOnly, configDefaults) + configStore, err := config.NewStoreFromDSN(dsn, readOnly, configDefaults) if err != nil { return errors.Wrap(err, "failed to apply Config option") } diff --git a/app/server.go b/app/server.go index 75d7a92aa0..af960d4494 100644 --- a/app/server.go +++ b/app/server.go @@ -234,7 +234,7 @@ func NewServer(options ...Option) (*Server, error) { } if s.configStore == nil { - innerStore, err := config.NewFileStore("config.json", true) + innerStore, err := config.NewFileStore("config.json") if err != nil { return nil, errors.Wrap(err, "failed to load config") } diff --git a/app/server_test.go b/app/server_test.go index 1db0ac9043..c32bcb66a3 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -167,7 +167,7 @@ func TestStartServerNoS3Bucket(t *testing.T) { s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port) s, err := NewServer(func(server *Server) error { - configStore, _ := config.NewFileStore("config.json", true) + configStore, _ := config.NewFileStore("config.json") store, _ := config.NewStoreFromBacking(configStore, nil, false) server.configStore = store server.UpdateConfig(func(cfg *model.Config) { diff --git a/cmd/mattermost/commands/cmdtestlib.go b/cmd/mattermost/commands/cmdtestlib.go index 5dc8b34dcd..0945913f0b 100644 --- a/cmd/mattermost/commands/cmdtestlib.go +++ b/cmd/mattermost/commands/cmdtestlib.go @@ -150,7 +150,7 @@ func (h *testHelper) execArgs(t *testing.T, args []string) []string { ret = append(ret, "-test.coverprofile", filepath.Join(dir, strings.Join(baseParts, "."))) } - ret = append(ret, "--", "--disableconfigwatch") + ret = append(ret, "--") // Unless the test passes a `--config` of its own, create a temporary one from the default // configuration with the current test database applied. diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index 7c2df15269..a5d758bf86 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -147,7 +147,7 @@ func getConfigStore(command *cobra.Command) (*config.Store, error) { return nil, errors.Wrap(err, "failed to initialize i18n") } - configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, false, nil) + configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, nil) if err != nil { return nil, errors.Wrap(err, "failed to initialize config store") } diff --git a/cmd/mattermost/commands/config_test.go b/cmd/mattermost/commands/config_test.go index 043b2702d7..81f1ea5d84 100644 --- a/cmd/mattermost/commands/config_test.go +++ b/cmd/mattermost/commands/config_test.go @@ -564,9 +564,9 @@ func TestConfigMigrate(t *testing.T) { sqlDSN := getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource) fileDSN := "config.json" - ds, err := config.NewStoreFromDSN(sqlDSN, false, false, nil) + ds, err := config.NewStoreFromDSN(sqlDSN, false, nil) require.NoError(t, err) - fs, err := config.NewStoreFromDSN(fileDSN, false, false, nil) + fs, err := config.NewStoreFromDSN(fileDSN, false, nil) require.NoError(t, err) defer ds.Close() diff --git a/cmd/mattermost/commands/db.go b/cmd/mattermost/commands/db.go index 1af025bec3..c9f985820c 100644 --- a/cmd/mattermost/commands/db.go +++ b/cmd/mattermost/commands/db.go @@ -57,7 +57,7 @@ func initDbCmdF(command *cobra.Command, _ []string) error { return errors.Wrap(err, "error loading custom configuration defaults") } - configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, false, customDefaults) + configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, customDefaults) if err != nil { return errors.Wrap(err, "failed to load configuration") } diff --git a/cmd/mattermost/commands/init.go b/cmd/mattermost/commands/init.go index 405ed79c9f..62b785db7c 100644 --- a/cmd/mattermost/commands/init.go +++ b/cmd/mattermost/commands/init.go @@ -42,7 +42,7 @@ func initDBCommandContext(configDSN string, readOnlyConfigStore bool) (*app.App, model.AppErrorInit(i18n.T) s, err := app.NewServer( - app.Config(configDSN, false, readOnlyConfigStore, nil), + app.Config(configDSN, readOnlyConfigStore, nil), app.StartSearchEngine, ) if err != nil { diff --git a/cmd/mattermost/commands/plugin_test.go b/cmd/mattermost/commands/plugin_test.go index fd52b99e50..730dd2658e 100644 --- a/cmd/mattermost/commands/plugin_test.go +++ b/cmd/mattermost/commands/plugin_test.go @@ -36,7 +36,7 @@ func TestPlugin(t *testing.T) { output = th.CheckCommand(t, "plugin", "enable", "testplugin") assert.Contains(t, output, "Enabled plugin: testplugin") - fs, err := config.NewFileStore(th.ConfigPath(), false) + fs, err := config.NewFileStore(th.ConfigPath()) require.NoError(t, err) cfsStore, err := config.NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -46,7 +46,7 @@ func TestPlugin(t *testing.T) { output = th.CheckCommand(t, "plugin", "disable", "testplugin") assert.Contains(t, output, "Disabled plugin: testplugin") - fs, err = config.NewFileStore(th.ConfigPath(), false) + fs, err = config.NewFileStore(th.ConfigPath()) require.NoError(t, err) cfsStore, err = config.NewStoreFromBacking(fs, nil, false) require.NoError(t, err) diff --git a/cmd/mattermost/commands/root.go b/cmd/mattermost/commands/root.go index 09b035d085..3dd3dea3e9 100644 --- a/cmd/mattermost/commands/root.go +++ b/cmd/mattermost/commands/root.go @@ -22,5 +22,4 @@ var RootCmd = &cobra.Command{ func init() { RootCmd.PersistentFlags().StringP("config", "c", "", "Configuration file to use.") - RootCmd.PersistentFlags().Bool("disableconfigwatch", false, "When set config.json will not be loaded from disk when the file is changed.") } diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index c4fca1a094..1e7ca9e2e9 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -38,7 +38,6 @@ func init() { } func serverCmdF(command *cobra.Command, args []string) error { - disableConfigWatch, _ := command.Flags().GetBool("disableconfigwatch") interruptChan := make(chan os.Signal, 1) if err := utils.TranslationsPreInit(); err != nil { @@ -50,7 +49,7 @@ func serverCmdF(command *cobra.Command, args []string) error { mlog.Warn("Error loading custom configuration defaults: " + err.Error()) } - configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), !disableConfigWatch, false, customDefaults) + configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, customDefaults) if err != nil { return errors.Wrap(err, "failed to load configuration") } diff --git a/config/database.go b/config/database.go index 713db4a1be..fdb82b038d 100644 --- a/config/database.go +++ b/config/database.go @@ -346,8 +346,3 @@ func (ds *DatabaseStore) String() string { func (ds *DatabaseStore) Close() error { return ds.db.Close() } - -// Watch nothing on memory store -func (ds *DatabaseStore) Watch(_ func()) error { - return nil -} diff --git a/config/file.go b/config/file.go index 3efe8a1ca0..0bcda57bf2 100644 --- a/config/file.go +++ b/config/file.go @@ -26,27 +26,19 @@ var ( // It also uses the folder containing the configuration file for storing other configuration files. // Not to be used directly. Only to be used as a backing store for config.Store type FileStore struct { - path string - watch bool - watcher *watcher - callback func() + path string } // NewFileStore creates a new instance of a config store backed by the given file path. -// -// If watch is true, any external changes to the file will force a reload. -func NewFileStore(path string, watch bool) (fs *FileStore, err error) { +func NewFileStore(path string) (fs *FileStore, err error) { resolvedPath, err := resolveConfigFilePath(path) if err != nil { return nil, err } - fs = &FileStore{ - path: resolvedPath, - watch: watch, - } - - return fs, nil + return &FileStore{ + path: resolvedPath, + }, nil } // resolveConfigFilePath attempts to resolve the given configuration file path to an absolute path. @@ -103,12 +95,6 @@ func (fs *FileStore) Set(newCfg *model.Config) error { // persist writes the configuration to the configured file. func (fs *FileStore) persist(cfg *model.Config) error { - needsRestart := false - if fs.watcher != nil { - fs.stopWatcher() - needsRestart = true - } - b, err := marshalConfig(cfg) if err != nil { return errors.Wrap(err, "failed to serialize") @@ -119,12 +105,6 @@ func (fs *FileStore) persist(cfg *model.Config) error { return errors.Wrap(err, "failed to write file") } - if fs.watch && needsRestart { - if err = fs.Watch(fs.callback); err != nil { - mlog.Error("failed to start config watcher", mlog.String("path", fs.path), mlog.Err(err)) - } - } - return nil } @@ -215,34 +195,6 @@ func (fs *FileStore) RemoveFile(name string) error { return nil } -func (fs *FileStore) Watch(callback func()) error { - if fs.watcher != nil || !fs.watch { - return nil - } - - fs.callback = callback - watcher, err := newWatcher(fs.path, callback) - if err != nil { - return err - } - - fs.watcher = watcher - - return nil -} - -// stopWatcher stops any previously started watcher. -func (fs *FileStore) stopWatcher() { - if fs.watcher == nil { - return - } - - if err := fs.watcher.Close(); err != nil { - mlog.Error("failed to close watcher", mlog.Err(err)) - } - fs.watcher = nil -} - // String returns the path to the file backing the config. func (fs *FileStore) String() string { return "file://" + fs.path @@ -250,7 +202,5 @@ func (fs *FileStore) String() string { // Close cleans up resources associated with the store. func (fs *FileStore) Close() error { - fs.stopWatcher() - return nil } diff --git a/config/file_test.go b/config/file_test.go index 3dd2f7d444..2b0a624045 100644 --- a/config/file_test.go +++ b/config/file_test.go @@ -51,7 +51,7 @@ func setupConfigFile(t *testing.T, cfg *model.Config) (string, func()) { func setupConfigFileStore(t *testing.T, cfg *model.Config) (*Store, func()) { t.Helper() path, tearDown := setupConfigFile(t, cfg) - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -101,7 +101,7 @@ func TestFileStoreNew(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -115,7 +115,7 @@ func TestFileStoreNew(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, customConfigDefaults, false) require.NoError(t, err) @@ -134,7 +134,7 @@ func TestFileStoreNew(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfigNoFF) defer tearDown() - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -148,7 +148,7 @@ func TestFileStoreNew(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfigNoFF) defer tearDown() - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, customConfigDefaults, false) require.NoError(t, err) @@ -170,7 +170,7 @@ func TestFileStoreNew(t *testing.T) { defer os.RemoveAll(tempDir) path := filepath.Join(tempDir, "does_not_exist") - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -189,7 +189,7 @@ func TestFileStoreNew(t *testing.T) { defer os.RemoveAll(tempDir) path := filepath.Join(tempDir, "does_not_exist") - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, customConfigDefaults, false) require.NoError(t, err) @@ -208,7 +208,7 @@ func TestFileStoreNew(t *testing.T) { defer os.RemoveAll(tempDir) path := filepath.Join(tempDir, "does/not/exist") - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.Nil(t, configStore) @@ -230,7 +230,7 @@ func TestFileStoreNew(t *testing.T) { ioutil.WriteFile(path, cfgData, 0644) - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -249,7 +249,7 @@ func TestFileStoreNew(t *testing.T) { defer os.RemoveAll("config/TestFileStoreNew") path := "TestFileStoreNew/a/b/c/config.json" - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -284,7 +284,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -296,7 +296,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -310,7 +310,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, customConfigDefaults, false) require.NoError(t, err) @@ -322,7 +322,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, customConfigDefaults, false) require.NoError(t, err) @@ -337,7 +337,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -349,7 +349,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true") defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLEUPLOADS") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -363,7 +363,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -375,7 +375,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000") defer os.Unsetenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -389,7 +389,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -401,7 +401,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456") defer os.Unsetenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -415,7 +415,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -427,7 +427,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -441,7 +441,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -453,7 +453,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db user:pwd@db2:5433/test-db2 user:pwd@db3:5434/test-db3") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - fsInner, err = NewFileStore(path, false) + fsInner, err = NewFileStore(path) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -529,7 +529,7 @@ func TestFileStoreSet(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -623,40 +623,6 @@ func TestFileStoreSet(t *testing.T) { require.True(t, wasCalled(called, 5*time.Second)) }) - - t.Run("watcher restarted", func(t *testing.T) { - if testing.Short() { - t.Skip("skipping watcher test in short mode") - } - - path, tearDown := setupConfigFile(t, emptyConfig) - defer tearDown() - - fsInner, err := NewFileStore(path, true) - require.NoError(t, err) - fs, err := NewStoreFromBacking(fsInner, nil, false) - require.NoError(t, err) - defer fs.Close() - - _, _, err = fs.Set(minimalConfig) - require.NoError(t, err) - - // Let the initial call to invokeConfigListeners finish. - time.Sleep(1 * time.Second) - - called := make(chan bool, 1) - callback := func(oldCfg, newCfg *model.Config) { - called <- true - } - fs.AddListener(callback) - - // Rewrite the config to the file on disk - cfgData, err := marshalConfig(emptyConfig) - require.NoError(t, err) - - ioutil.WriteFile(path, cfgData, 0644) - require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written") - }) } func TestFileStoreLoad(t *testing.T) { @@ -664,7 +630,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -699,7 +665,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://overridePersistEnvVariables") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -724,7 +690,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true") defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLEUPLOADS") - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -749,7 +715,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000") defer os.Unsetenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM") - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -774,7 +740,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456") defer os.Unsetenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE") - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -799,7 +765,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -826,7 +792,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -848,7 +814,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -882,7 +848,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, fixesRequiredConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -901,7 +867,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path, false) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -968,121 +934,6 @@ func TestFileStoreLoad(t *testing.T) { }) } -func TestFileStoreWatcherEmitter(t *testing.T) { - if testing.Short() { - t.Skip("skipping watcher test in short mode") - } - - t.Run("disabled", func(t *testing.T) { - path, tearDown := setupConfigFile(t, emptyConfig) - defer tearDown() - fsInner, err := NewFileStore(path, false) - require.NoError(t, err) - fs, err := NewStoreFromBacking(fsInner, nil, false) - require.NoError(t, err) - defer fs.Close() - - // Let the initial call to invokeConfigListeners finish. - time.Sleep(1 * time.Second) - - called := make(chan bool, 1) - callback := func(oldCfg, newCfg *model.Config) { - called <- true - } - fs.AddListener(callback) - - // Rewrite the config to the file on disk - cfgData, err := marshalConfig(emptyConfig) - require.NoError(t, err) - - ioutil.WriteFile(path, cfgData, 0644) - require.False(t, wasCalled(called, 1*time.Second), "callback should not have been called since watching disabled") - }) - - t.Run("enabled", func(t *testing.T) { - path, tearDown := setupConfigFile(t, emptyConfig) - defer tearDown() - fsInner, err := NewFileStore(path, true) - require.NoError(t, err) - fs, err := NewStoreFromBacking(fsInner, nil, false) - require.NoError(t, err) - defer fs.Close() - - called := make(chan bool, 1) - callback := func(oldCCfg, newCfg *model.Config) { - called <- true - } - fs.AddListener(callback) - - // Rewrite the config to the file on disk - cfgData, err := marshalConfig(minimalConfig) - require.NoError(t, err) - - f, err := os.OpenFile(path, os.O_WRONLY, 0644) - require.NoError(t, err) - defer f.Close() - _, err = f.Write(cfgData) - require.NoError(t, err) - - require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written") - }) - - t.Run("no change", func(t *testing.T) { - path, tearDown := setupConfigFile(t, minimalConfig) - defer tearDown() - fsInner, err := NewFileStore(path, true) - require.NoError(t, err) - fs, err := NewStoreFromBacking(fsInner, nil, false) - require.NoError(t, err) - defer fs.Close() - - // Let the initial call to invokeConfigListeners finish. - time.Sleep(1 * time.Second) - - called := make(chan bool, 1) - callback := func(oldCfg, newCfg *model.Config) { - called <- true - } - fs.AddListener(callback) - - _, _, err = fs.Set(minimalConfig) - require.NoError(t, err) - - require.False(t, wasCalled(called, 1*time.Second), "callback should not have been called since no change has happened") - }) - - t.Run("env only change", func(t *testing.T) { - path, tearDown := setupConfigFile(t, minimalConfig) - defer tearDown() - fsInner, err := NewFileStore(path, true) - require.NoError(t, err) - fs, err := NewStoreFromBacking(fsInner, nil, false) - require.NoError(t, err) - defer fs.Close() - - // Let the initial call to invokeConfigListeners finish. - time.Sleep(1 * time.Second) - - called := make(chan bool, 1) - callback := func(oldCfg, newCfg *model.Config) { - require.NotEqual(t, oldCfg, newCfg) - expectedConfig := minimalConfig.Clone() - expectedConfig.ServiceSettings.SiteURL = model.NewString("http://override") - require.Equal(t, minimalConfig, oldCfg) - require.Equal(t, expectedConfig, newCfg) - called <- true - } - fs.AddListener(callback) - - os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") - defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - _, _, err = fs.Set(minimalConfig) - require.NoError(t, err) - - require.True(t, wasCalled(called, 5*time.Second), "callback should have been called since no change has happened") - }) -} - func TestFileStoreSave(t *testing.T) { store, tearDown := setupConfigFileStore(t, minimalConfig) defer tearDown() @@ -1108,7 +959,7 @@ func TestFileGetFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1170,7 +1021,7 @@ func TestFileSetFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1221,7 +1072,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1234,7 +1085,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1250,7 +1101,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1273,7 +1124,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1286,7 +1137,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1305,7 +1156,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1317,7 +1168,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1339,7 +1190,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1368,7 +1219,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path, true) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1390,7 +1241,7 @@ func TestFileStoreString(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fs, err := NewFileStore(path, false) + fs, err := NewFileStore(path) require.NoError(t, err) defer fs.Close() @@ -1411,7 +1262,7 @@ func wasCalled(c chan bool, duration time.Duration) bool { func TestFileStoreReadOnly(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path, true) + fsInner, err := NewFileStore(path) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, true) require.NoError(t, err) diff --git a/config/memory.go b/config/memory.go index 5d4f384d4a..d59b34bec8 100644 --- a/config/memory.go +++ b/config/memory.go @@ -118,8 +118,3 @@ func (ms *MemoryStore) String() string { func (ms *MemoryStore) Close() error { return nil } - -// Watch nothing on memory store -func (ms *MemoryStore) Watch(_ func()) error { - return nil -} diff --git a/config/migrate.go b/config/migrate.go index f651921a3d..8b19aabe83 100644 --- a/config/migrate.go +++ b/config/migrate.go @@ -9,13 +9,13 @@ import ( // Migrate migrates SAML keys, certificates, and other config files from one store to another given their data source names. func Migrate(from, to string) error { - source, err := NewStoreFromDSN(from, false, false, nil) + source, err := NewStoreFromDSN(from, false, nil) if err != nil { return errors.Wrapf(err, "failed to access source config %s", from) } defer source.Close() - destination, err := NewStoreFromDSN(to, false, false, nil) + destination, err := NewStoreFromDSN(to, false, nil) if err != nil { return errors.Wrapf(err, "failed to access destination config %s", to) } diff --git a/config/migrate_test.go b/config/migrate_test.go index 7b7f51c41b..64e60ad7f9 100644 --- a/config/migrate_test.go +++ b/config/migrate_test.go @@ -121,7 +121,7 @@ func TestMigrate(t *testing.T) { err = Migrate(sourceDSN, destinationDSN) require.NoError(t, err) - destinationfile, err := NewFileStore(destinationDSN, false) + destinationfile, err := NewFileStore(destinationDSN) require.NoError(t, err) destination, err := NewStoreFromBacking(destinationfile, nil, false) require.NoError(t, err) @@ -141,7 +141,7 @@ func TestMigrate(t *testing.T) { sourceDSN := path.Join(pwd, "config-custom.json") destinationDSN := getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource) - sourcefile, err := NewFileStore(sourceDSN, false) + sourcefile, err := NewFileStore(sourceDSN) require.NoError(t, err) source, err := NewStoreFromBacking(sourcefile, nil, false) require.NoError(t, err) diff --git a/config/store.go b/config/store.go index 29d03d9fbc..223bc5153f 100644 --- a/config/store.go +++ b/config/store.go @@ -61,8 +61,6 @@ type BackingStore interface { // String describes the backing store for the config. String() string - Watch(callback func()) error - // Close cleans up resources associated with the store. Close() error } @@ -80,24 +78,18 @@ func NewStoreFromBacking(backingStore BackingStore, customDefaults *model.Config return nil, errors.Wrap(err, "unable to load on store creation") } - if err := backingStore.Watch(func() { - store.Load() - }); err != nil { - return nil, errors.Wrap(err, "failed to watch backing store") - } - return store, nil } // NewStoreFromDSN creates and returns a new config store backed by either a database or file store // depending on the value of the given data source name string. -func NewStoreFromDSN(dsn string, watch, readOnly bool, customDefaults *model.Config) (*Store, error) { +func NewStoreFromDSN(dsn string, readOnly bool, customDefaults *model.Config) (*Store, error) { var err error var backingStore BackingStore if IsDatabaseDSN(dsn) { backingStore, err = NewDatabaseStore(dsn) } else { - backingStore, err = NewFileStore(dsn, watch) + backingStore, err = NewFileStore(dsn) } if err != nil { return nil, err diff --git a/config/store_test.go b/config/store_test.go index e649cccbfe..b32d710e21 100644 --- a/config/store_test.go +++ b/config/store_test.go @@ -27,25 +27,13 @@ func TestNewStoreFromDSN(t *testing.T) { require.NoError(t, os.Mkdir(filepath.Join(tempDir, "config"), 0700)) t.Run("database dsn", func(t *testing.T) { - ds, err := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), false, false, nil) - require.NoError(t, err) - ds.Close() - }) - - t.Run("database dsn, watch ignored", func(t *testing.T) { - ds, err := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), true, false, nil) + ds, err := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), false, nil) require.NoError(t, err) ds.Close() }) t.Run("file dsn", func(t *testing.T) { - fs, err := NewStoreFromDSN("config.json", false, false, nil) - require.NoError(t, err) - fs.Close() - }) - - t.Run("file dsn, watch", func(t *testing.T) { - fs, err := NewStoreFromDSN("config.json", true, false, nil) + fs, err := NewStoreFromDSN("config.json", false, nil) require.NoError(t, err) fs.Close() }) @@ -66,7 +54,7 @@ func TestNewStoreReadOnly(t *testing.T) { require.NoError(t, os.Mkdir(filepath.Join(tempDir, "config"), 0700)) t.Run("database dsn", func(t *testing.T) { - ds, err := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), false, true, nil) + ds, err := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), true, nil) require.NoError(t, err) t.Run("Set", func(t *testing.T) { @@ -90,7 +78,7 @@ func TestNewStoreReadOnly(t *testing.T) { }) t.Run("file dsn", func(t *testing.T) { - fs, err := NewStoreFromDSN("config.json", false, true, nil) + fs, err := NewStoreFromDSN("config.json", true, nil) require.NoError(t, err) t.Run("Set", func(t *testing.T) {