diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 01ad3b4f51a..de38a70d784 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -81,7 +81,7 @@ func setupTestHelper(dbStore store.Store, searchEngine *searchengine.Broker, ent if updateConfig != nil { updateConfig(config) } - memoryStore.Set(config) + memoryStore.Set(config, true) var options []app.Option options = append(options, app.ConfigStore(memoryStore)) diff --git a/app/config.go b/app/config.go index 183f6d09a12..3adec5ef566 100644 --- a/app/config.go +++ b/app/config.go @@ -46,11 +46,20 @@ func (a *App) EnvironmentConfig() map[string]interface{} { return a.Srv().EnvironmentConfig() } +func (s *Server) UpdateConfigWithoutDesanitize(f func(*model.Config)) { + old := s.Config() + updated := old.Clone() + f(updated) + if _, err := s.configStore.Set(updated, false); err != nil { + mlog.Error("Failed to update config", mlog.Err(err)) + } +} + func (s *Server) UpdateConfig(f func(*model.Config)) { old := s.Config() updated := old.Clone() f(updated) - if _, err := s.configStore.Set(updated); err != nil { + if _, err := s.configStore.Set(updated, true); err != nil { mlog.Error("Failed to update config", mlog.Err(err)) } } @@ -395,7 +404,7 @@ func (a *App) GetEnvironmentConfig() map[string]interface{} { // SaveConfig replaces the active configuration, optionally notifying cluster peers. func (a *App) SaveConfig(newCfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { - oldCfg, err := a.Srv().configStore.Set(newCfg) + oldCfg, err := a.Srv().configStore.Set(newCfg, true) if errors.Cause(err) == config.ErrReadOnlyConfiguration { return model.NewAppError("saveConfig", "ent.cluster.save_config.error", nil, err.Error(), http.StatusForbidden) } else if err != nil { diff --git a/app/helper_test.go b/app/helper_test.go index b16adea1ef4..533fbb30647 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -59,7 +59,7 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo } *config.PluginSettings.Directory = filepath.Join(tempWorkspace, "plugins") *config.PluginSettings.ClientDirectory = filepath.Join(tempWorkspace, "webapp") - memoryStore.Set(config) + memoryStore.Set(config, true) buffer := &bytes.Buffer{} diff --git a/app/server.go b/app/server.go index 6c6cbb282d7..30a5a918565 100644 --- a/app/server.go +++ b/app/server.go @@ -280,7 +280,7 @@ func NewServer(options ...Option) (*Server, error) { if license == nil && len(s.Config().SqlSettings.DataSourceReplicas) > 0 { mlog.Warn("Read replicas functionality disabled by current license. Please contact your system administrator about upgrading your enterprise license.") - s.UpdateConfig(func(cfg *model.Config) { + s.UpdateConfigWithoutDesanitize(func(cfg *model.Config) { cfg.SqlSettings.DataSourceReplicas = []string{} }) s.Store.Close() @@ -289,7 +289,7 @@ func NewServer(options ...Option) (*Server, error) { if license == nil && len(s.Config().SqlSettings.DataSourceSearchReplicas) > 0 { mlog.Warn("Search replicas functionality disabled by current license. Please contact your system administrator about upgrading your enterprise license.") - s.UpdateConfig(func(cfg *model.Config) { + s.UpdateConfigWithoutDesanitize(func(cfg *model.Config) { cfg.SqlSettings.DataSourceSearchReplicas = []string{} }) s.Store.Close() diff --git a/app/server_test.go b/app/server_test.go index 2e1284b2377..67d2f70492e 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -20,6 +20,7 @@ import ( "github.com/mattermost/mattermost-server/v5/config" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/utils/fileutils" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -38,62 +39,55 @@ func TestStartServerSuccess(t *testing.T) { } func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { + cfg := model.Config{} + cfg.SetDefaults() + cfg.SqlSettings.DataSourceReplicas = []string{*cfg.SqlSettings.DataSource} + cfg.SqlSettings.DataSourceSearchReplicas = []string{*cfg.SqlSettings.DataSource} + t.Run("Read Replicas with no License", func(t *testing.T) { s, err := NewServer(func(server *Server) error { - configStore, _ := config.NewFileStore("config.json", true) + configStore, _ := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: &cfg}) server.configStore = configStore - server.UpdateConfig(func(cfg *model.Config) { - cfg.SqlSettings.DataSourceReplicas = []string{*cfg.SqlSettings.DataSource} - }) return nil }) require.NoError(t, err) - require.Equal(t, s.sqlStore.GetMaster(), s.sqlStore.GetReplica()) - require.Len(t, s.Config().SqlSettings.DataSourceReplicas, 0) + assert.Same(t, s.sqlStore.GetMaster(), s.sqlStore.GetReplica()) + assert.Len(t, s.Config().SqlSettings.DataSourceReplicas, 0) }) t.Run("Read Replicas With License", func(t *testing.T) { s, err := NewServer(func(server *Server) error { - configStore, _ := config.NewFileStore("config.json", true) + configStore, _ := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: &cfg}) server.configStore = configStore server.licenseValue.Store(model.NewTestLicense()) - server.UpdateConfig(func(cfg *model.Config) { - cfg.SqlSettings.DataSourceReplicas = []string{*cfg.SqlSettings.DataSource} - }) return nil }) require.NoError(t, err) - require.NotEqual(t, s.sqlStore.GetMaster(), s.sqlStore.GetReplica()) - require.Len(t, s.Config().SqlSettings.DataSourceReplicas, 1) + assert.NotSame(t, s.sqlStore.GetMaster(), s.sqlStore.GetReplica()) + assert.Len(t, s.Config().SqlSettings.DataSourceReplicas, 1) }) t.Run("Search Replicas with no License", func(t *testing.T) { s, err := NewServer(func(server *Server) error { - configStore, _ := config.NewFileStore("config.json", true) + configStore, _ := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: &cfg}) server.configStore = configStore - server.UpdateConfig(func(cfg *model.Config) { - cfg.SqlSettings.DataSourceSearchReplicas = []string{*cfg.SqlSettings.DataSource} - }) return nil }) require.NoError(t, err) - require.Equal(t, s.sqlStore.GetMaster(), s.sqlStore.GetSearchReplica()) - require.Len(t, s.Config().SqlSettings.DataSourceSearchReplicas, 0) + assert.Same(t, s.sqlStore.GetMaster(), s.sqlStore.GetSearchReplica()) + assert.Len(t, s.Config().SqlSettings.DataSourceSearchReplicas, 0) }) t.Run("Search Replicas With License", func(t *testing.T) { s, err := NewServer(func(server *Server) error { - configStore, _ := config.NewFileStore("config.json", true) + configStore, _ := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: &cfg}) server.configStore = configStore server.licenseValue.Store(model.NewTestLicense()) - server.UpdateConfig(func(cfg *model.Config) { - cfg.SqlSettings.DataSourceSearchReplicas = []string{*cfg.SqlSettings.DataSource} - }) return nil }) require.NoError(t, err) - require.NotEqual(t, s.sqlStore.GetMaster(), s.sqlStore.GetSearchReplica()) - require.Len(t, s.Config().SqlSettings.DataSourceSearchReplicas, 1) + assert.NotSame(t, s.sqlStore.GetMaster(), s.sqlStore.GetSearchReplica()) + assert.Len(t, s.Config().SqlSettings.DataSourceSearchReplicas, 1) }) } @@ -107,7 +101,7 @@ func TestStartServerRateLimiterCriticalError(t *testing.T) { config := ms.Get() *config.RateLimitSettings.Enable = true *config.RateLimitSettings.MaxBurst = -100 - _, err = ms.Set(config) + _, err = ms.Set(config, true) require.NoError(t, err) s, err := NewServer(ConfigStore(ms)) diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index b288c04e77d..24b7fae0fb1 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -251,7 +251,7 @@ func configSetCmdF(command *cobra.Command, args []string) error { return errors.New("Invalid locale configuration") } - if _, errSet := configStore.Set(newConfig); errSet != nil { + if _, errSet := configStore.Set(newConfig, true); errSet != nil { return errors.Wrap(errSet, "failed to set config") } @@ -430,7 +430,7 @@ func configResetCmdF(command *cobra.Command, args []string) error { confirmFlag, _ := command.Flags().GetBool("confirm") if confirmFlag { - if _, err = configStore.Set(defaultConfig); err != nil { + if _, err = configStore.Set(defaultConfig, true); err != nil { return errors.Wrap(err, "failed to set config") } } @@ -440,7 +440,7 @@ func configResetCmdF(command *cobra.Command, args []string) error { CommandPrettyPrintln("Are you sure you want to reset all the configuration settings?(YES/NO): ") fmt.Scanln(&confirmResetAll) if confirmResetAll == "YES" { - if _, err = configStore.Set(defaultConfig); err != nil { + if _, err = configStore.Set(defaultConfig, true); err != nil { return errors.Wrap(err, "failed to set config") } } @@ -469,7 +469,7 @@ func configResetCmdF(command *cobra.Command, args []string) error { return errors.New("Invalid locale configuration") } - if _, errSet := configStore.Set(tempConfig); errSet != nil { + if _, errSet := configStore.Set(tempConfig, true); errSet != nil { return errors.Wrap(errSet, "failed to set config") } diff --git a/config/common.go b/config/common.go index c69ebbc8f27..251e38b5ad6 100644 --- a/config/common.go +++ b/config/common.go @@ -43,7 +43,7 @@ func (cs *commonStore) GetEnvironmentOverrides() map[string]interface{} { // using the persist function argument. // // This function assumes no lock has been acquired, as it acquires a write lock itself. -func (cs *commonStore) set(newCfg *model.Config, allowEnvironmentOverrides bool, validate func(*model.Config) error, persist func(*model.Config) error) (*model.Config, error) { +func (cs *commonStore) set(newCfg *model.Config, allowEnvironmentOverrides bool, validate func(*model.Config) error, persist func(*model.Config) error, shouldDesanitize bool) (*model.Config, error) { cs.configLock.Lock() var unlockOnce sync.Once defer unlockOnce.Do(cs.configLock.Unlock) @@ -69,7 +69,9 @@ func (cs *commonStore) set(newCfg *model.Config, allowEnvironmentOverrides bool, // Sometimes the config is received with "fake" data in sensitive fields. Apply the real // data from the existing config as necessary. - desanitize(oldCfg, newCfg) + if shouldDesanitize { + desanitize(oldCfg, newCfg) + } if validate != nil { if err := validate(newCfg); err != nil { diff --git a/config/common_test.go b/config/common_test.go index 64e2b8897eb..27ac6be5b63 100644 --- a/config/common_test.go +++ b/config/common_test.go @@ -155,7 +155,7 @@ func TestConfigEnvironmentOverrides(t *testing.T) { }) t.Run("setting config should respect environment variable overrides", func(t *testing.T) { - _, err := base.Set(originalConfig) + _, err := base.Set(originalConfig, true) require.NoError(t, err) assert.Equal(t, "http://overridden.ca", *base.Get().ServiceSettings.SiteURL) diff --git a/config/database.go b/config/database.go index e07fd7fe371..465ceaf8590 100644 --- a/config/database.go +++ b/config/database.go @@ -159,8 +159,8 @@ func parseDSN(dsn string) (string, string, error) { } // Set replaces the current configuration in its entirety and updates the backing store. -func (ds *DatabaseStore) Set(newCfg *model.Config) (*model.Config, error) { - return ds.commonStore.set(newCfg, true, ds.commonStore.validate, ds.persist) +func (ds *DatabaseStore) Set(newCfg *model.Config, shouldDesanitize bool) (*model.Config, error) { + return ds.commonStore.set(newCfg, true, ds.commonStore.validate, ds.persist, shouldDesanitize) } // maxLength identifies the maximum length of a configuration or configuration file diff --git a/config/database_test.go b/config/database_test.go index 3e58a0fcd43..e9c7a587fa8 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -189,7 +189,7 @@ func TestDatabaseStoreGet(t *testing.T) { assert.True(t, cfg == cfg2, "Get() returned different configuration instances") newCfg := &model.Config{} - oldCfg, err := ds.Set(newCfg) + oldCfg, err := ds.Set(newCfg, true) require.NoError(t, err) assert.True(t, oldCfg == cfg, "returned config after set() changed original") @@ -355,7 +355,7 @@ func TestDatabaseStoreSet(t *testing.T) { require.NoError(t, err) defer ds.Close() - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) if assert.Error(t, err) { assert.EqualError(t, err, "old configuration modified instead of cloning") } @@ -373,7 +373,7 @@ func TestDatabaseStoreSet(t *testing.T) { newCfg := &model.Config{} - retCfg, err := ds.Set(newCfg) + retCfg, err := ds.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -393,7 +393,7 @@ func TestDatabaseStoreSet(t *testing.T) { newCfg := &model.Config{} newCfg.LdapSettings.BindPassword = sToP(model.FAKE_SETTING) - retCfg, err := ds.Set(newCfg) + retCfg, err := ds.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -411,7 +411,7 @@ func TestDatabaseStoreSet(t *testing.T) { newCfg := &model.Config{} newCfg.ServiceSettings.SiteURL = sToP("invalid") - _, err = ds.Set(newCfg) + _, err = ds.Set(newCfg, true) if assert.Error(t, err) { assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") } @@ -427,11 +427,11 @@ func TestDatabaseStoreSet(t *testing.T) { require.NoError(t, err) defer ds.Close() - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) beforeID, _ := getActualDatabaseConfig(t) - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) afterID, _ := getActualDatabaseConfig(t) @@ -452,7 +452,7 @@ func TestDatabaseStoreSet(t *testing.T) { }, } - _, err = ds.Set(newCfg) + _, err = ds.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, "http://new", *ds.Get().ServiceSettings.SiteURL) @@ -472,7 +472,7 @@ func TestDatabaseStoreSet(t *testing.T) { }, } - _, err = ds.Set(newCfg) + _, err = ds.Set(newCfg, true) require.NoError(t, err) err = ds.Load() @@ -495,7 +495,7 @@ func TestDatabaseStoreSet(t *testing.T) { newCfg := &model.Config{} - _, err = ds.Set(newCfg) + _, err = ds.Set(newCfg, true) require.Error(t, err) assert.True(t, strings.HasPrefix(err.Error(), "failed to persist: failed to query active configuration"), "unexpected error: "+err.Error()) @@ -517,7 +517,7 @@ func TestDatabaseStoreSet(t *testing.T) { newCfg := emptyConfig.Clone() newCfg.ServiceSettings.SiteURL = sToP(longSiteURL) - _, err = ds.Set(newCfg) + _, err = ds.Set(newCfg, true) require.Error(t, err) assert.True(t, strings.HasPrefix(err.Error(), "failed to persist: marshalled configuration failed length check: value is too long"), "unexpected error: "+err.Error()) }) @@ -540,7 +540,7 @@ func TestDatabaseStoreSet(t *testing.T) { newCfg := &model.Config{} - retCfg, err := ds.Set(newCfg) + retCfg, err := ds.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -602,7 +602,7 @@ func TestDatabaseStoreLoad(t *testing.T) { require.NoError(t, err) defer ds.Close() - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) assert.Equal(t, "http://overridePersistEnvVariables", *ds.Get().ServiceSettings.SiteURL) @@ -625,7 +625,7 @@ func TestDatabaseStoreLoad(t *testing.T) { assert.Equal(t, true, *ds.Get().PluginSettings.EnableUploads) - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) assert.Equal(t, true, *ds.Get().PluginSettings.EnableUploads) @@ -648,7 +648,7 @@ func TestDatabaseStoreLoad(t *testing.T) { assert.Equal(t, 3000, *ds.Get().TeamSettings.MaxUsersPerTeam) - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) assert.Equal(t, 3000, *ds.Get().TeamSettings.MaxUsersPerTeam) @@ -671,7 +671,7 @@ func TestDatabaseStoreLoad(t *testing.T) { assert.Equal(t, int64(123456), *ds.Get().ServiceSettings.TLSStrictTransportMaxAge) - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) assert.Equal(t, int64(123456), *ds.Get().ServiceSettings.TLSStrictTransportMaxAge) @@ -694,7 +694,7 @@ func TestDatabaseStoreLoad(t *testing.T) { assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas) - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas) @@ -719,7 +719,7 @@ func TestDatabaseStoreLoad(t *testing.T) { assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas) - _, err = ds.Set(ds.Get()) + _, err = ds.Set(ds.Get(), true) require.NoError(t, err) assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas) diff --git a/config/file.go b/config/file.go index 73ff3cb6f51..8bd6c203298 100644 --- a/config/file.go +++ b/config/file.go @@ -104,14 +104,14 @@ func (fs *FileStore) resolveFilePath(name string) string { } // Set replaces the current configuration in its entirety and updates the backing store. -func (fs *FileStore) Set(newCfg *model.Config) (*model.Config, error) { +func (fs *FileStore) Set(newCfg *model.Config, shouldDesanitize bool) (*model.Config, error) { return fs.commonStore.set(newCfg, true, func(cfg *model.Config) error { if *fs.config.ClusterSettings.Enable && *fs.config.ClusterSettings.ReadOnlyConfig { return ErrReadOnlyConfiguration } return fs.commonStore.validate(cfg) - }, fs.persist) + }, fs.persist, shouldDesanitize) } // persist writes the configuration to the configured file. diff --git a/config/file_test.go b/config/file_test.go index ae5fd29edbe..df3f8fb45e6 100644 --- a/config/file_test.go +++ b/config/file_test.go @@ -197,7 +197,7 @@ func TestFileStoreGet(t *testing.T) { assert.True(t, cfg == cfg2, "Get() returned different configuration instances") newCfg := &model.Config{} - oldCfg, err := fs.Set(newCfg) + oldCfg, err := fs.Set(newCfg, true) require.NoError(t, err) assert.True(t, oldCfg == cfg, "returned config after set() changed original") @@ -352,7 +352,7 @@ func TestFileStoreSet(t *testing.T) { require.NoError(t, err) defer fs.Close() - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) if assert.Error(t, err) { assert.EqualError(t, err, "old configuration modified instead of cloning") } @@ -370,7 +370,7 @@ func TestFileStoreSet(t *testing.T) { newCfg := &model.Config{} - retCfg, err := fs.Set(newCfg) + retCfg, err := fs.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -390,7 +390,7 @@ func TestFileStoreSet(t *testing.T) { newCfg := &model.Config{} newCfg.LdapSettings.BindPassword = sToP(model.FAKE_SETTING) - retCfg, err := fs.Set(newCfg) + retCfg, err := fs.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -408,7 +408,7 @@ func TestFileStoreSet(t *testing.T) { newCfg := &model.Config{} newCfg.ServiceSettings.SiteURL = sToP("invalid") - _, err = fs.Set(newCfg) + _, err = fs.Set(newCfg, true) if assert.Error(t, err) { assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") } @@ -426,7 +426,7 @@ func TestFileStoreSet(t *testing.T) { newCfg := &model.Config{} - _, err = fs.Set(newCfg) + _, err = fs.Set(newCfg, true) if assert.Error(t, err) { assert.Equal(t, config.ErrReadOnlyConfiguration, errors.Cause(err)) } @@ -447,7 +447,7 @@ func TestFileStoreSet(t *testing.T) { newCfg := &model.Config{} - _, err = fs.Set(newCfg) + _, err = fs.Set(newCfg, true) if assert.Error(t, err) { assert.True(t, strings.HasPrefix(err.Error(), "failed to persist: failed to write file")) } @@ -473,7 +473,7 @@ func TestFileStoreSet(t *testing.T) { newCfg := &model.Config{} - retCfg, err := fs.Set(newCfg) + retCfg, err := fs.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -492,7 +492,7 @@ func TestFileStoreSet(t *testing.T) { require.NoError(t, err) defer fs.Close() - _, err = fs.Set(&model.Config{}) + _, err = fs.Set(&model.Config{}, true) require.NoError(t, err) // Let the initial call to invokeConfigListeners finish. @@ -561,7 +561,7 @@ func TestFileStoreLoad(t *testing.T) { assert.Equal(t, "http://overridePersistEnvVariables", *fs.Get().ServiceSettings.SiteURL) - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) require.NoError(t, err) assert.Equal(t, "http://overridePersistEnvVariables", *fs.Get().ServiceSettings.SiteURL) @@ -584,7 +584,7 @@ func TestFileStoreLoad(t *testing.T) { assert.Equal(t, true, *fs.Get().PluginSettings.EnableUploads) - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) require.NoError(t, err) assert.Equal(t, true, *fs.Get().PluginSettings.EnableUploads) @@ -607,7 +607,7 @@ func TestFileStoreLoad(t *testing.T) { assert.Equal(t, 3000, *fs.Get().TeamSettings.MaxUsersPerTeam) - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) require.NoError(t, err) assert.Equal(t, 3000, *fs.Get().TeamSettings.MaxUsersPerTeam) @@ -630,7 +630,7 @@ func TestFileStoreLoad(t *testing.T) { assert.Equal(t, int64(123456), *fs.Get().ServiceSettings.TLSStrictTransportMaxAge) - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) require.NoError(t, err) assert.Equal(t, int64(123456), *fs.Get().ServiceSettings.TLSStrictTransportMaxAge) @@ -653,7 +653,7 @@ func TestFileStoreLoad(t *testing.T) { assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, fs.Get().SqlSettings.DataSourceReplicas) - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) require.NoError(t, err) assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, fs.Get().SqlSettings.DataSourceReplicas) @@ -678,7 +678,7 @@ func TestFileStoreLoad(t *testing.T) { assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, fs.Get().SqlSettings.DataSourceReplicas) - _, err = fs.Set(fs.Get()) + _, err = fs.Set(fs.Get(), true) require.NoError(t, err) assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, fs.Get().SqlSettings.DataSourceReplicas) @@ -812,7 +812,7 @@ func TestFileStoreSave(t *testing.T) { } t.Run("set with automatic save", func(t *testing.T) { - _, err = fs.Set(newCfg) + _, err = fs.Set(newCfg, true) require.NoError(t, err) err = fs.Load() diff --git a/config/memory.go b/config/memory.go index 639a933bdf3..74186bd966c 100644 --- a/config/memory.go +++ b/config/memory.go @@ -67,13 +67,13 @@ func NewMemoryStoreWithOptions(options *MemoryStoreOptions) (*MemoryStore, error } // Set replaces the current configuration in its entirety. -func (ms *MemoryStore) Set(newCfg *model.Config) (*model.Config, error) { +func (ms *MemoryStore) Set(newCfg *model.Config, shouldDesanitize bool) (*model.Config, error) { validate := ms.commonStore.validate if !ms.validate { validate = nil } - return ms.commonStore.set(newCfg, ms.allowEnvironmentOverrides, validate, ms.persist) + return ms.commonStore.set(newCfg, ms.allowEnvironmentOverrides, validate, ms.persist, shouldDesanitize) } // persist copies the active config to the saved config. diff --git a/config/memory_test.go b/config/memory_test.go index d1c8836407e..c80c78e1b2e 100644 --- a/config/memory_test.go +++ b/config/memory_test.go @@ -76,7 +76,7 @@ func TestMemoryStoreGet(t *testing.T) { assert.True(t, cfg == cfg2, "Get() returned different configuration instances") newCfg := &model.Config{} - oldCfg, err := ms.Set(newCfg) + oldCfg, err := ms.Set(newCfg, true) require.NoError(t, err) assert.True(t, oldCfg == cfg, "returned config after set() changed original") @@ -114,7 +114,7 @@ func TestMemoryStoreSet(t *testing.T) { require.NoError(t, err) defer ms.Close() - _, err = ms.Set(ms.Get()) + _, err = ms.Set(ms.Get(), true) if assert.Error(t, err) { assert.EqualError(t, err, "old configuration modified instead of cloning") } @@ -131,7 +131,7 @@ func TestMemoryStoreSet(t *testing.T) { newCfg := &model.Config{} - retCfg, err := ms.Set(newCfg) + retCfg, err := ms.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -150,7 +150,7 @@ func TestMemoryStoreSet(t *testing.T) { newCfg := &model.Config{} newCfg.LdapSettings.BindPassword = sToP(model.FAKE_SETTING) - retCfg, err := ms.Set(newCfg) + retCfg, err := ms.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) @@ -167,7 +167,7 @@ func TestMemoryStoreSet(t *testing.T) { newCfg := &model.Config{} newCfg.ServiceSettings.SiteURL = sToP("invalid") - _, err = ms.Set(newCfg) + _, err = ms.Set(newCfg, true) if assert.Error(t, err) { assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") } @@ -188,7 +188,7 @@ func TestMemoryStoreSet(t *testing.T) { }, } - _, err = ms.Set(newCfg) + _, err = ms.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, "http://new", *ms.Get().ServiceSettings.SiteURL) @@ -211,7 +211,7 @@ func TestMemoryStoreSet(t *testing.T) { newCfg := &model.Config{} - retCfg, err := ms.Set(newCfg) + retCfg, err := ms.Set(newCfg, true) require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) diff --git a/config/migrate.go b/config/migrate.go index a300cebb7f6..1f112916ddf 100644 --- a/config/migrate.go +++ b/config/migrate.go @@ -18,7 +18,7 @@ func Migrate(from, to string) error { } sourceConfig := source.Get() - if _, err = destination.Set(sourceConfig); err != nil { + if _, err = destination.Set(sourceConfig, true); err != nil { return errors.Wrapf(err, "failed to set config") } diff --git a/config/migrate_test.go b/config/migrate_test.go index 618bc78fb6e..d5aec3816a9 100644 --- a/config/migrate_test.go +++ b/config/migrate_test.go @@ -33,14 +33,14 @@ func TestMigrateDatabaseToFile(t *testing.T) { defer func() { defaultCfg := &model.Config{} defaultCfg.SetDefaults() - ds.Set(defaultCfg) + ds.Set(defaultCfg, true) }() require.NoError(t, err) config := ds.Get() config.SamlSettings.IdpCertificateFile = &files[0] config.SamlSettings.PublicCertificateFile = &files[1] config.SamlSettings.PrivateKeyFile = &files[2] - _, err = ds.Set(config) + _, err = ds.Set(config, true) require.NoError(t, err) for _, file := range files { diff --git a/config/store.go b/config/store.go index 6be194a4193..2c82e6f2d83 100644 --- a/config/store.go +++ b/config/store.go @@ -25,7 +25,7 @@ type Store interface { RemoveEnvironmentOverrides(cfg *model.Config) *model.Config // Set replaces the current configuration in its entirety and updates the backing store. - Set(*model.Config) (*model.Config, error) + Set(*model.Config, bool) (*model.Config, error) // Load updates the current configuration from the backing store, possibly initializing. Load() (err error) diff --git a/config/utils.go b/config/utils.go index 50217186950..8b533837f39 100644 --- a/config/utils.go +++ b/config/utils.go @@ -36,18 +36,14 @@ func desanitize(actual, target *model.Config) { *target.SqlSettings.DataSource = *actual.SqlSettings.DataSource } - if len(target.SqlSettings.DataSourceReplicas) == 1 && target.SqlSettings.DataSourceReplicas[0] == model.FAKE_SETTING { - target.SqlSettings.DataSourceReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) - for i := range target.SqlSettings.DataSourceReplicas { - target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] - } + target.SqlSettings.DataSourceReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) + for i := range target.SqlSettings.DataSourceReplicas { + target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] } - if len(target.SqlSettings.DataSourceSearchReplicas) == 1 && target.SqlSettings.DataSourceSearchReplicas[0] == model.FAKE_SETTING { - target.SqlSettings.DataSourceSearchReplicas = make([]string, len(actual.SqlSettings.DataSourceSearchReplicas)) - for i := range target.SqlSettings.DataSourceSearchReplicas { - target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] - } + target.SqlSettings.DataSourceSearchReplicas = make([]string, len(actual.SqlSettings.DataSourceSearchReplicas)) + for i := range target.SqlSettings.DataSourceSearchReplicas { + target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] } if *target.SqlSettings.AtRestEncryptKey == model.FAKE_SETTING { diff --git a/model/config.go b/model/config.go index 57362106f33..be8f0ec5a0e 100644 --- a/model/config.go +++ b/model/config.go @@ -3347,8 +3347,6 @@ func (o *Config) Sanitize() { } *o.SqlSettings.DataSource = FAKE_SETTING - o.SqlSettings.DataSourceReplicas = []string{FAKE_SETTING} - o.SqlSettings.DataSourceSearchReplicas = []string{FAKE_SETTING} *o.SqlSettings.AtRestEncryptKey = FAKE_SETTING *o.ElasticsearchSettings.Password = FAKE_SETTING