From 6c857a4525f18b0e10d552232e0cf9c7656f3339 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Mon, 16 Nov 2020 15:25:48 +0100 Subject: [PATCH] [MM-30436] Add support for configuration custom defaults (#16265) * [MM-30436] Add support for configuration custom defaults * Addressing review comments * Addressing PR comments * Soft fail if the configuration defaults cannot be read * Directly print error in log message * Fix linter Co-authored-by: Mattermod --- api4/apitestlib.go | 2 +- api4/config_test.go | 4 +- app/options.go | 10 +- app/server.go | 2 +- app/server_test.go | 4 +- cmd/mattermost/commands/config.go | 2 +- cmd/mattermost/commands/config_test.go | 4 +- cmd/mattermost/commands/init.go | 2 +- cmd/mattermost/commands/plugin_test.go | 4 +- cmd/mattermost/commands/server.go | 35 +++++- config/common_test.go | 14 ++- config/database_test.go | 163 +++++++++++++++++-------- config/file_test.go | 162 ++++++++++++++++++------ config/migrate.go | 4 +- config/migrate_test.go | 8 +- config/store.go | 31 +++-- config/store_test.go | 8 +- services/mailservice/mail_test.go | 6 +- 18 files changed, 334 insertions(+), 131 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 868ac5453b..e9d9e9efb8 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -92,7 +92,7 @@ func setupTestHelper(dbStore store.Store, searchEngine *searchengine.Broker, ent } memoryStore.Set(memoryConfig) - configStore, err := config.NewStoreFromBacking(memoryStore) + configStore, err := config.NewStoreFromBacking(memoryStore, nil) if err != nil { panic(err) } diff --git a/api4/config_test.go b/api4/config_test.go index 75d71b2cb0..7b3dcbc889 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -686,11 +686,11 @@ func TestMigrateConfig(t *testing.T) { }) th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - f, err := config.NewStore("from.json", false) + f, err := config.NewStore("from.json", false, nil) require.NoError(t, err) defer f.RemoveFile("from.json") - _, err = config.NewStore("to.json", false) + _, err = config.NewStore("to.json", false, nil) require.NoError(t, err) defer f.RemoveFile("to.json") diff --git a/app/options.go b/app/options.go index 2abf3cc52f..7a5533e312 100644 --- a/app/options.go +++ b/app/options.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/mattermost/mattermost-server/v5/config" + "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" ) @@ -38,10 +39,13 @@ func StoreOverride(override interface{}) Option { } } -// Config applies the given config dsn, whether a path to config.json or a database connection string. -func Config(dsn string, watch bool) Option { +// Config applies the given config dsn, whether a path to config.json +// 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 bool, configDefaults *model.Config) Option { return func(s *Server) error { - configStore, err := config.NewStore(dsn, watch) + configStore, err := config.NewStore(dsn, watch, configDefaults) if err != nil { return errors.Wrap(err, "failed to apply Config option") } diff --git a/app/server.go b/app/server.go index e42936c5af..fb3fc8434d 100644 --- a/app/server.go +++ b/app/server.go @@ -210,7 +210,7 @@ func NewServer(options ...Option) (*Server, error) { if err != nil { return nil, errors.Wrap(err, "failed to load config") } - configStore, err := config.NewStoreFromBacking(innerStore) + configStore, err := config.NewStoreFromBacking(innerStore, nil) 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 85442dc842..bdc6119020 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -386,7 +386,7 @@ func TestSentry(t *testing.T) { s, err := NewServer(func(server *Server) error { configStore, _ := config.NewFileStore("config.json", true) - store, _ := config.NewStoreFromBacking(configStore) + store, _ := config.NewStoreFromBacking(configStore, nil) server.configStore = store server.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = ":0" @@ -437,7 +437,7 @@ func TestSentry(t *testing.T) { s, err := NewServer(func(server *Server) error { configStore, _ := config.NewFileStore("config.json", true) - store, _ := config.NewStoreFromBacking(configStore) + store, _ := config.NewStoreFromBacking(configStore, nil) server.configStore = store server.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = ":0" diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index 19733a3f1d..05035c5fde 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -145,7 +145,7 @@ func getConfigStore(command *cobra.Command) (*config.Store, error) { return nil, errors.Wrap(err, "failed to initialize i18n") } - configStore, err := config.NewStore(getConfigDSN(command, config.GetEnvironment()), false) + configStore, err := config.NewStore(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 f82d0cd5f4..b557d08b49 100644 --- a/cmd/mattermost/commands/config_test.go +++ b/cmd/mattermost/commands/config_test.go @@ -543,9 +543,9 @@ func TestConfigMigrate(t *testing.T) { sqlDSN := getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource) fileDSN := "config.json" - ds, err := config.NewStore(sqlDSN, false) + ds, err := config.NewStore(sqlDSN, false, nil) require.NoError(t, err) - fs, err := config.NewStore(fileDSN, false) + fs, err := config.NewStore(fileDSN, false, nil) require.NoError(t, err) defer ds.Close() diff --git a/cmd/mattermost/commands/init.go b/cmd/mattermost/commands/init.go index 689b255f2b..93d2661ece 100644 --- a/cmd/mattermost/commands/init.go +++ b/cmd/mattermost/commands/init.go @@ -32,7 +32,7 @@ func InitDBCommandContext(configDSN string) (*app.App, error) { model.AppErrorInit(utils.T) s, err := app.NewServer( - app.Config(configDSN, false), + app.Config(configDSN, false, nil), app.StartSearchEngine, ) if err != nil { diff --git a/cmd/mattermost/commands/plugin_test.go b/cmd/mattermost/commands/plugin_test.go index 025415e89f..7ce44f8653 100644 --- a/cmd/mattermost/commands/plugin_test.go +++ b/cmd/mattermost/commands/plugin_test.go @@ -37,7 +37,7 @@ func TestPlugin(t *testing.T) { fs, err := config.NewFileStore(th.ConfigPath(), false) require.Nil(t, err) - cfsStore, err := config.NewStoreFromBacking(fs) + cfsStore, err := config.NewStoreFromBacking(fs, nil) require.Nil(t, err) require.NotNil(t, cfsStore.Get().PluginSettings.PluginStates["testplugin"]) assert.True(t, cfsStore.Get().PluginSettings.PluginStates["testplugin"].Enable) @@ -47,7 +47,7 @@ func TestPlugin(t *testing.T) { assert.Contains(t, output, "Disabled plugin: testplugin") fs, err = config.NewFileStore(th.ConfigPath(), false) require.Nil(t, err) - cfsStore, err = config.NewStoreFromBacking(fs) + cfsStore, err = config.NewStoreFromBacking(fs, nil) require.Nil(t, err) require.NotNil(t, cfsStore.Get().PluginSettings.PluginStates["testplugin"]) assert.False(t, cfsStore.Get().PluginSettings.PluginStates["testplugin"].Enable) diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index b59130ed3b..d14da0c02a 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -4,6 +4,7 @@ package commands import ( + "encoding/json" "net" "os" "os/signal" @@ -15,6 +16,7 @@ import ( "github.com/mattermost/mattermost-server/v5/config" "github.com/mattermost/mattermost-server/v5/manualtesting" "github.com/mattermost/mattermost-server/v5/mlog" + "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/utils" "github.com/mattermost/mattermost-server/v5/web" "github.com/mattermost/mattermost-server/v5/wsapi" @@ -22,6 +24,8 @@ import ( "github.com/spf13/cobra" ) +const CUSTOM_DEFAULTS_ENV_VAR = "MM_CUSTOM_DEFAULTS_PATH" + var serverCmd = &cobra.Command{ Use: "server", Short: "Run the Mattermost server", @@ -34,6 +38,27 @@ func init() { RootCmd.RunE = serverCmdF } +func loadCustomDefaults() (*model.Config, error) { + customDefaultsPath := os.Getenv(CUSTOM_DEFAULTS_ENV_VAR) + if customDefaultsPath == "" { + return nil, nil + } + + file, err := os.Open(customDefaultsPath) + if err != nil { + return nil, errors.Wrapf(err, "unable to open custom defaults file at %q", customDefaultsPath) + } + defer file.Close() + + var customDefaults *model.Config + err = json.NewDecoder(file).Decode(&customDefaults) + if err != nil { + return nil, errors.Wrap(err, "unable to decode custom defaults configuration") + } + + return customDefaults, nil +} + func serverCmdF(command *cobra.Command, args []string) error { disableConfigWatch, _ := command.Flags().GetBool("disableconfigwatch") usedPlatform, _ := command.Flags().GetBool("platform") @@ -41,9 +66,15 @@ func serverCmdF(command *cobra.Command, args []string) error { interruptChan := make(chan os.Signal, 1) if err := utils.TranslationsPreInit(); err != nil { - return errors.Wrapf(err, "unable to load Mattermost translation files") + return errors.Wrap(err, "unable to load Mattermost translation files") } - configStore, err := config.NewStore(getConfigDSN(command, config.GetEnvironment()), !disableConfigWatch) + + customDefaults, err := loadCustomDefaults() + if err != nil { + mlog.Error("Error loading custom configuration defaults: " + err.Error()) + } + + configStore, err := config.NewStore(getConfigDSN(command, config.GetEnvironment()), !disableConfigWatch, customDefaults) if err != nil { return errors.Wrap(err, "failed to load configuration") } diff --git a/config/common_test.go b/config/common_test.go index c2ea63a812..a3cc7f4133 100644 --- a/config/common_test.go +++ b/config/common_test.go @@ -14,7 +14,7 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) -var emptyConfig, readOnlyConfig, minimalConfig, invalidConfig, fixesRequiredConfig, ldapConfig, testConfig *model.Config +var emptyConfig, readOnlyConfig, minimalConfig, invalidConfig, fixesRequiredConfig, ldapConfig, testConfig, customConfigDefaults *model.Config func init() { emptyConfig = &model.Config{} @@ -72,6 +72,14 @@ func init() { SiteURL: sToP("http://TestStoreNew"), }, } + customConfigDefaults = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewString("http://custom.com"), + }, + DisplaySettings: model.DisplaySettings{ + ExperimentalTimezone: model.NewBool(false), + }, + } } func TestMergeConfigs(t *testing.T) { @@ -132,7 +140,7 @@ func TestMergeConfigs(t *testing.T) { func TestConfigEnvironmentOverrides(t *testing.T) { memstore, err := config.NewMemoryStore() require.NoError(t, err) - base, err := config.NewStoreFromBacking(memstore) + base, err := config.NewStoreFromBacking(memstore, nil) require.NoError(t, err) originalConfig := &model.Config{} originalConfig.ServiceSettings.SiteURL = newString("http://notoverriden.ca") @@ -161,7 +169,7 @@ func TestRemoveEnvironmentOverrides(t *testing.T) { memstore, err := config.NewMemoryStore() require.NoError(t, err) - base, err := config.NewStoreFromBacking(memstore) + base, err := config.NewStoreFromBacking(memstore, nil) require.NoError(t, err) oldCfg := base.Get() assert.Equal(t, "http://overridden.ca", *oldCfg.ServiceSettings.SiteURL) diff --git a/config/database_test.go b/config/database_test.go index 1a1db3bbfd..127572945a 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -115,12 +115,12 @@ func assertDatabaseNotEqualsConfig(t *testing.T, expectedCfg *model.Config) { assert.NotEqual(t, expectedCfg, actualCfg) } -func newTestDatabaseStore(t *testing.T) (*config.Store, error) { +func newTestDatabaseStore(t *testing.T, customDefaults *model.Config) (*config.Store, error) { sqlSettings := mainHelper.GetSQLSettings() dss, err := config.NewDatabaseStore(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource)) require.NoError(t, err) - cStore, err := config.NewStoreFromBacking(dss) + cStore, err := config.NewStoreFromBacking(dss, customDefaults) require.NoError(t, err) return cStore, nil @@ -133,18 +133,28 @@ func TestDatabaseStoreNew(t *testing.T) { sqlSettings := mainHelper.GetSQLSettings() t.Run("no existing configuration - initialization required", func(t *testing.T) { - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() assert.Equal(t, "", *ds.Get().ServiceSettings.SiteURL) }) + t.Run("no existing configuration with custom defaults", func(t *testing.T) { + truncateTables(t) + ds, err := newTestDatabaseStore(t, customConfigDefaults) + require.NoError(t, err) + defer ds.Close() + + assert.Equal(t, *customConfigDefaults.ServiceSettings.SiteURL, *ds.Get().ServiceSettings.SiteURL) + assert.Equal(t, *customConfigDefaults.DisplaySettings.ExperimentalTimezone, *ds.Get().DisplaySettings.ExperimentalTimezone) + }) + t.Run("existing config, initialization required", func(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -152,11 +162,28 @@ func TestDatabaseStoreNew(t *testing.T) { assertDatabaseNotEqualsConfig(t, testConfig) }) + t.Run("existing config with custom defaults, initialization required", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, testConfig, nil) + defer tearDown() + + ds, err := newTestDatabaseStore(t, customConfigDefaults) + require.NoError(t, err) + defer ds.Close() + + // already existing value should not be overwritten by the + // custom default value + assert.Equal(t, "http://TestStoreNew", *ds.Get().ServiceSettings.SiteURL) + // not existing value should be overwritten by the custom + // default value + assert.Equal(t, *customConfigDefaults.DisplaySettings.ExperimentalTimezone, *ds.Get().DisplaySettings.ExperimentalTimezone) + assertDatabaseNotEqualsConfig(t, testConfig) + }) + t.Run("already minimally configured", func(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -164,6 +191,21 @@ func TestDatabaseStoreNew(t *testing.T) { assertDatabaseEqualsConfig(t, minimalConfig) }) + t.Run("already minimally configured with custom defaults", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := newTestDatabaseStore(t, customConfigDefaults) + require.NoError(t, err) + defer ds.Close() + + // as the whole config has default values already, custom + // defaults should have no effect + assert.Equal(t, "http://minimal", *ds.Get().ServiceSettings.SiteURL) + assert.NotEqual(t, *customConfigDefaults.DisplaySettings.ExperimentalTimezone, *ds.Get().DisplaySettings.ExperimentalTimezone) + assertDatabaseEqualsConfig(t, minimalConfig) + }) + t.Run("invalid url", func(t *testing.T) { _, err := config.NewDatabaseStore("") require.Error(t, err) @@ -187,7 +229,7 @@ func TestDatabaseStoreGet(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -205,7 +247,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -215,7 +257,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - ds, err = newTestDatabaseStore(t) + ds, err = newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -223,11 +265,34 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ds.GetEnvironmentOverrides()) }) + t.Run("get override for a string variable with a custom default value", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, testConfig, nil) + defer tearDown() + + ds, err := newTestDatabaseStore(t, customConfigDefaults) + require.NoError(t, err) + defer ds.Close() + + assert.Equal(t, "http://TestStoreNew", *ds.Get().ServiceSettings.SiteURL) + assert.Empty(t, ds.GetEnvironmentOverrides()) + + os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") + defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") + + ds, err = newTestDatabaseStore(t, customConfigDefaults) + require.NoError(t, err) + defer ds.Close() + + // environment override should take priority over the custom default value + assert.Equal(t, "http://override", *ds.Get().ServiceSettings.SiteURL) + assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ds.GetEnvironmentOverrides()) + }) + t.Run("get override for a bool variable", func(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -237,7 +302,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true") defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLEUPLOADS") - ds, err = newTestDatabaseStore(t) + ds, err = newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -249,7 +314,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -259,7 +324,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000") defer os.Unsetenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM") - ds, err = newTestDatabaseStore(t) + ds, err = newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -271,7 +336,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -281,7 +346,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456") defer os.Unsetenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE") - ds, err = newTestDatabaseStore(t) + ds, err = newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -293,7 +358,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -303,7 +368,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - ds, err = newTestDatabaseStore(t) + ds, err = newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -318,7 +383,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -328,7 +393,7 @@ func TestDatabaseStoreGetEnivironmentOverrides(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") - ds, err = newTestDatabaseStore(t) + ds, err = newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -348,7 +413,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -362,7 +427,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -378,7 +443,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, ldapConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -395,7 +460,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -414,7 +479,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -433,7 +498,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, readOnlyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -453,7 +518,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -476,7 +541,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -501,7 +566,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -518,7 +583,7 @@ func TestDatabaseStoreSet(t *testing.T) { activeID, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -549,7 +614,7 @@ func TestDatabaseStoreLoad(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -564,7 +629,7 @@ func TestDatabaseStoreLoad(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -586,7 +651,7 @@ func TestDatabaseStoreLoad(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://overridePersistEnvVariables") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -607,7 +672,7 @@ func TestDatabaseStoreLoad(t *testing.T) { os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true") defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLEUPLOADS") - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -630,7 +695,7 @@ func TestDatabaseStoreLoad(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000") defer os.Unsetenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM") - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -653,7 +718,7 @@ func TestDatabaseStoreLoad(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456") defer os.Unsetenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE") - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -676,7 +741,7 @@ func TestDatabaseStoreLoad(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -701,7 +766,7 @@ func TestDatabaseStoreLoad(t *testing.T) { os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db") defer os.Unsetenv("MM_SQLSETTINGS_DATASOURCEREPLICAS") - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -721,7 +786,7 @@ func TestDatabaseStoreLoad(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -749,7 +814,7 @@ func TestDatabaseStoreLoad(t *testing.T) { _, tearDown := setupConfigDatabase(t, fixesRequiredConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -763,7 +828,7 @@ func TestDatabaseStoreLoad(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -787,7 +852,7 @@ func TestDatabaseGetFile(t *testing.T) { }) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -818,7 +883,7 @@ func TestDatabaseSetFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -871,7 +936,7 @@ func TestDatabaseHasFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -884,7 +949,7 @@ func TestDatabaseHasFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -902,7 +967,7 @@ func TestDatabaseHasFile(t *testing.T) { }) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -915,7 +980,7 @@ func TestDatabaseHasFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -930,7 +995,7 @@ func TestDatabaseRemoveFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -942,7 +1007,7 @@ func TestDatabaseRemoveFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -966,7 +1031,7 @@ func TestDatabaseRemoveFile(t *testing.T) { }) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) defer ds.Close() @@ -989,7 +1054,7 @@ func TestDatabaseStoreString(t *testing.T) { _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() - ds, err := newTestDatabaseStore(t) + ds, err := newTestDatabaseStore(t, nil) require.NoError(t, err) require.NotNil(t, ds) defer ds.Close() diff --git a/config/file_test.go b/config/file_test.go index ef17a43d74..13448cdd20 100644 --- a/config/file_test.go +++ b/config/file_test.go @@ -91,7 +91,7 @@ func TestFileStoreNew(t *testing.T) { fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.NoError(t, err) defer configStore.Close() @@ -99,13 +99,32 @@ func TestFileStoreNew(t *testing.T) { assertFileNotEqualsConfig(t, testConfig, path) }) + t.Run("absolute path, initialization required, with custom defaults", func(t *testing.T) { + path, tearDown := setupConfigFile(t, testConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + configStore, err := config.NewStoreFromBacking(fs, customConfigDefaults) + require.NoError(t, err) + defer configStore.Close() + + // already existing value should not be affected by the custom + // defaults + assert.Equal(t, "http://TestStoreNew", *configStore.Get().ServiceSettings.SiteURL) + // nonexisting value should be overwritten by the custom + // defaults + assert.Equal(t, *customConfigDefaults.DisplaySettings.ExperimentalTimezone, *configStore.Get().DisplaySettings.ExperimentalTimezone) + assertFileNotEqualsConfig(t, testConfig, path) + }) + t.Run("absolute path, already minimally configured", func(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.NoError(t, err) defer configStore.Close() @@ -113,6 +132,23 @@ func TestFileStoreNew(t *testing.T) { assertFileEqualsConfig(t, minimalConfig, path) }) + t.Run("absolute path, already minimally configured, with custom defaults", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + configStore, err := config.NewStoreFromBacking(fs, customConfigDefaults) + require.NoError(t, err) + defer configStore.Close() + + // as the whole config has default values already, custom + // defaults should have no effect + assert.Equal(t, "http://minimal", *configStore.Get().ServiceSettings.SiteURL) + assert.NotEqual(t, *customConfigDefaults.DisplaySettings.ExperimentalTimezone, *configStore.Get().DisplaySettings.ExperimentalTimezone) + assertFileEqualsConfig(t, minimalConfig, path) + }) + t.Run("absolute path, file does not exist", func(t *testing.T) { _, tearDown := setupConfigFile(t, nil) defer tearDown() @@ -124,7 +160,7 @@ func TestFileStoreNew(t *testing.T) { path := filepath.Join(tempDir, "does_not_exist") fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.NoError(t, err) defer configStore.Close() @@ -132,6 +168,25 @@ func TestFileStoreNew(t *testing.T) { assertFileNotEqualsConfig(t, testConfig, path) }) + t.Run("absolute path, file does not exist, with custom defaults", func(t *testing.T) { + _, tearDown := setupConfigFile(t, nil) + defer tearDown() + + tempDir, err := ioutil.TempDir("", "TestFileStoreNew") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + path := filepath.Join(tempDir, "does_not_exist") + fs, err := config.NewFileStore(path, false) + require.NoError(t, err) + configStore, err := config.NewStoreFromBacking(fs, customConfigDefaults) + require.NoError(t, err) + defer configStore.Close() + + assert.Equal(t, *customConfigDefaults.ServiceSettings.SiteURL, *configStore.Get().ServiceSettings.SiteURL) + assert.Equal(t, *customConfigDefaults.DisplaySettings.ExperimentalTimezone, *configStore.Get().DisplaySettings.ExperimentalTimezone) + }) + t.Run("absolute path, path to file does not exist", func(t *testing.T) { _, tearDown := setupConfigFile(t, nil) defer tearDown() @@ -143,7 +198,7 @@ func TestFileStoreNew(t *testing.T) { path := filepath.Join(tempDir, "does/not/exist") fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.Nil(t, configStore) require.Error(t, err) }) @@ -165,7 +220,7 @@ func TestFileStoreNew(t *testing.T) { fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.NoError(t, err) defer configStore.Close() @@ -184,7 +239,7 @@ func TestFileStoreNew(t *testing.T) { path := "TestFileStoreNew/a/b/c/config.json" fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.NoError(t, err) defer configStore.Close() @@ -199,7 +254,7 @@ func TestFileStoreGet(t *testing.T) { fs, err := config.NewFileStore(path, false) require.NoError(t, err) - configStore, err := config.NewStoreFromBacking(fs) + configStore, err := config.NewStoreFromBacking(fs, nil) require.NoError(t, err) defer configStore.Close() @@ -225,7 +280,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -237,7 +292,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err = config.NewFileStore(path, false) require.NoError(t, err) - fs, err = config.NewStoreFromBacking(fsInner) + fs, err = config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -245,13 +300,40 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, fs.GetEnvironmentOverrides()) }) + t.Run("get override for a string variable, with custom defaults", func(t *testing.T) { + path, tearDown := setupConfigFile(t, testConfig) + defer tearDown() + + fsInner, err := config.NewFileStore(path, false) + require.NoError(t, err) + fs, err := config.NewStoreFromBacking(fsInner, customConfigDefaults) + require.NoError(t, err) + defer fs.Close() + + assert.Equal(t, "http://TestStoreNew", *fs.Get().ServiceSettings.SiteURL) + assert.Empty(t, fs.GetEnvironmentOverrides()) + + os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") + defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") + + fsInner, err = config.NewFileStore(path, false) + require.NoError(t, err) + fs, err = config.NewStoreFromBacking(fsInner, customConfigDefaults) + require.NoError(t, err) + defer fs.Close() + + // environment override should take priority over the custom default value + assert.Equal(t, "http://override", *fs.Get().ServiceSettings.SiteURL) + assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, fs.GetEnvironmentOverrides()) + }) + t.Run("get override for a bool variable", func(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -263,7 +345,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err = config.NewFileStore(path, false) require.NoError(t, err) - fs, err = config.NewStoreFromBacking(fsInner) + fs, err = config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -277,7 +359,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -289,7 +371,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err = config.NewFileStore(path, false) require.NoError(t, err) - fs, err = config.NewStoreFromBacking(fsInner) + fs, err = config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -303,7 +385,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -315,7 +397,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err = config.NewFileStore(path, false) require.NoError(t, err) - fs, err = config.NewStoreFromBacking(fsInner) + fs, err = config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -329,7 +411,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -341,7 +423,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err = config.NewFileStore(path, false) require.NoError(t, err) - fs, err = config.NewStoreFromBacking(fsInner) + fs, err = config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -358,7 +440,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -370,7 +452,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { fsInner, err = config.NewFileStore(path, false) require.NoError(t, err) - fs, err = config.NewStoreFromBacking(fsInner) + fs, err = config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -386,7 +468,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -407,7 +489,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -426,7 +508,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -447,7 +529,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -466,7 +548,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -488,7 +570,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -516,7 +598,7 @@ func TestFileStoreSet(t *testing.T) { fsInner, err := config.NewFileStore(path, true) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -548,7 +630,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -565,7 +647,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -589,7 +671,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -614,7 +696,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -639,7 +721,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -664,7 +746,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -689,7 +771,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -716,7 +798,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -738,7 +820,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -759,7 +841,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -778,7 +860,7 @@ func TestFileStoreLoad(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -808,7 +890,7 @@ func TestFileStoreWatcherEmitter(t *testing.T) { t.Run("disabled", func(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -832,7 +914,7 @@ func TestFileStoreWatcherEmitter(t *testing.T) { t.Run("enabled", func(t *testing.T) { fsInner, err := config.NewFileStore(path, true) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() @@ -857,7 +939,7 @@ func TestFileStoreSave(t *testing.T) { fsInner, err := config.NewFileStore(path, false) require.NoError(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.NoError(t, err) defer fs.Close() diff --git a/config/migrate.go b/config/migrate.go index 4ab917c479..d3f66bbf3a 100644 --- a/config/migrate.go +++ b/config/migrate.go @@ -7,13 +7,13 @@ import "github.com/pkg/errors" // 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 := NewStore(from, false) + source, err := NewStore(from, false, nil) if err != nil { return errors.Wrapf(err, "failed to access source config %s", from) } defer source.Close() - destination, err := NewStore(to, false) + destination, err := NewStore(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 ffa13d658d..0b1d1f0ba0 100644 --- a/config/migrate_test.go +++ b/config/migrate_test.go @@ -102,7 +102,7 @@ func TestMigrate(t *testing.T) { sourcedb, err := config.NewDatabaseStore(sourceDSN) require.NoError(t, err) - source, err := config.NewStoreFromBacking(sourcedb) + source, err := config.NewStoreFromBacking(sourcedb, nil) require.NoError(t, err) defer source.Close() @@ -112,7 +112,7 @@ func TestMigrate(t *testing.T) { destinationfile, err := config.NewFileStore(destinationDSN, false) require.NoError(t, err) - destination, err := config.NewStoreFromBacking(destinationfile) + destination, err := config.NewStoreFromBacking(destinationfile, nil) require.NoError(t, err) defer destination.Close() @@ -131,7 +131,7 @@ func TestMigrate(t *testing.T) { sourcefile, err := config.NewFileStore(sourceDSN, false) require.NoError(t, err) - source, err := config.NewStoreFromBacking(sourcefile) + source, err := config.NewStoreFromBacking(sourcefile, nil) require.NoError(t, err) defer source.Close() @@ -141,7 +141,7 @@ func TestMigrate(t *testing.T) { destinationdb, err := config.NewDatabaseStore(destinationDSN) require.NoError(t, err) - destination, err := config.NewStoreFromBacking(destinationdb) + destination, err := config.NewStoreFromBacking(destinationdb, nil) require.NoError(t, err) defer destination.Close() diff --git a/config/store.go b/config/store.go index e0701f67da..775ec8d2e0 100644 --- a/config/store.go +++ b/config/store.go @@ -48,19 +48,19 @@ type BackingStore interface { } // NewStore creates a database or file store given a data source name by which to connect. -func NewStore(dsn string, watch bool) (*Store, error) { +func NewStore(dsn string, watch bool, customDefaults *model.Config) (*Store, error) { backingStore, err := getBackingStore(dsn, watch) if err != nil { return nil, err } - return NewStoreFromBacking(backingStore) - + return NewStoreFromBacking(backingStore, customDefaults) } -func NewStoreFromBacking(backingStore BackingStore) (*Store, error) { +func NewStoreFromBacking(backingStore BackingStore, customDefaults *model.Config) (*Store, error) { store := &Store{ - backingStore: backingStore, + backingStore: backingStore, + configCustomDefaults: customDefaults, } if err := store.Load(); err != nil { @@ -90,7 +90,7 @@ func NewTestMemoryStore() *Store { panic("failed to initialize memory store: " + err.Error()) } - configStore, err := NewStoreFromBacking(memoryStore) + configStore, err := NewStoreFromBacking(memoryStore, nil) if err != nil { panic("failed to initialize config store: " + err.Error()) } @@ -102,9 +102,10 @@ type Store struct { emitter backingStore BackingStore - configLock sync.RWMutex - config *model.Config - configNoEnv *model.Config + configLock sync.RWMutex + config *model.Config + configNoEnv *model.Config + configCustomDefaults *model.Config persistFeatureFlags bool } @@ -195,6 +196,18 @@ func (s *Store) loadLockedWithOld(oldCfg *model.Config, unlockOnce *sync.Once) e } } + // If we have custom defaults set, the initial config is merged on + // top of them and we delete them not to be used again in the + // configuration reloads + if s.configCustomDefaults != nil { + var mErr error + loadedConfig, mErr = Merge(s.configCustomDefaults, loadedConfig, nil) + if mErr != nil { + return errors.Wrap(mErr, "failed to merge custom config defaults") + } + s.configCustomDefaults = nil + } + loadedConfig.SetDefaults() s.configNoEnv = loadedConfig.Clone() diff --git a/config/store_test.go b/config/store_test.go index fbaf300dad..eaf9253bef 100644 --- a/config/store_test.go +++ b/config/store_test.go @@ -28,25 +28,25 @@ func TestNewStore(t *testing.T) { require.NoError(t, os.Mkdir(filepath.Join(tempDir, "config"), 0700)) t.Run("database dsn", func(t *testing.T) { - ds, err := config.NewStore(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), false) + ds, err := config.NewStore(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), false, nil) require.NoError(t, err) ds.Close() }) t.Run("database dsn, watch ignored", func(t *testing.T) { - ds, err := config.NewStore(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), true) + ds, err := config.NewStore(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), true, nil) require.NoError(t, err) ds.Close() }) t.Run("file dsn", func(t *testing.T) { - fs, err := config.NewStore("config.json", false) + fs, err := config.NewStore("config.json", false, nil) require.NoError(t, err) fs.Close() }) t.Run("file dsn, watch", func(t *testing.T) { - fs, err := config.NewStore("config.json", true) + fs, err := config.NewStore("config.json", true, nil) require.NoError(t, err) fs.Close() }) diff --git a/services/mailservice/mail_test.go b/services/mailservice/mail_test.go index 1a92ca8d1f..b503a46c44 100644 --- a/services/mailservice/mail_test.go +++ b/services/mailservice/mail_test.go @@ -129,7 +129,7 @@ func TestSendMailUsingConfig(t *testing.T) { fsInner, err := config.NewFileStore("config.json", false) require.Nil(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.Nil(t, err) cfg := fs.Get() @@ -170,7 +170,7 @@ func TestSendMailWithEmbeddedFilesUsingConfig(t *testing.T) { fsInner, err := config.NewFileStore("config.json", false) require.Nil(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.Nil(t, err) cfg := fs.Get() @@ -217,7 +217,7 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { fsInner, err := config.NewFileStore("config.json", false) require.Nil(t, err) - fs, err := config.NewStoreFromBacking(fsInner) + fs, err := config.NewStoreFromBacking(fsInner, nil) require.Nil(t, err) cfg := fs.Get()