From e03384c4d15eeaf1f0a6b1fc26735ff734c3b556 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Mon, 31 Jan 2022 17:15:32 +0300 Subject: [PATCH] [MM-40712] config: Bugfix on path resolution; fail if given config does not exist (#19360) * config: bugfix on path resolution; fail if given config does not exist * reflect review comments Co-authored-by: Mattermod --- api4/config_test.go | 20 +++- app/options.go | 2 +- app/server.go | 2 +- app/server_test.go | 2 +- cmd/mattermost/commands/db.go | 2 +- cmd/mattermost/commands/server.go | 2 +- config/file.go | 17 +++- config/file_test.go | 149 ++++++++++++++++++------------ config/migrate.go | 4 +- config/migrate_test.go | 4 +- config/store.go | 4 +- config/store_test.go | 27 +++--- 12 files changed, 146 insertions(+), 89 deletions(-) diff --git a/api4/config_test.go b/api4/config_test.go index 9b367b23b4..9c0379dc93 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -4,6 +4,7 @@ package api4 import ( + "encoding/json" "fmt" "io/ioutil" "net/http" @@ -776,11 +777,22 @@ func TestMigrateConfig(t *testing.T) { }) th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - f, err := config.NewStoreFromDSN("from.json", false, nil) + cfg := &model.Config{} + cfg.SetDefaults() + + file, err := json.MarshalIndent(cfg, "", " ") + require.NoError(t, err) + + err = ioutil.WriteFile("from.json", file, 0644) + require.NoError(t, err) + + defer os.Remove("from.json") + + f, err := config.NewStoreFromDSN("from.json", false, nil, false) require.NoError(t, err) defer f.RemoveFile("from.json") - _, err = config.NewStoreFromDSN("to.json", false, nil) + _, err = config.NewStoreFromDSN("to.json", false, nil, true) require.NoError(t, err) defer f.RemoveFile("to.json") @@ -791,11 +803,11 @@ func TestMigrateConfig(t *testing.T) { t.Run("Cloud instances should not access to this API", func(t *testing.T) { require.True(t, th.App.Srv().SetLicense(model.NewTestLicense("cloud"))) - f, err := config.NewStoreFromDSN("from.json", false, nil) + f, err := config.NewStoreFromDSN("from.json", false, nil, true) require.NoError(t, err) defer f.RemoveFile("from.json") - _, err = config.NewStoreFromDSN("to.json", false, nil) + _, err = config.NewStoreFromDSN("to.json", false, nil, false) require.NoError(t, err) defer f.RemoveFile("to.json") diff --git a/app/options.go b/app/options.go index 167816fb0f..1dd117cfae 100644 --- a/app/options.go +++ b/app/options.go @@ -45,7 +45,7 @@ func StoreOverride(override interface{}) Option { // config loaded from the dsn on top of the normal defaults func Config(dsn string, readOnly bool, configDefaults *model.Config) Option { return func(s *Server) error { - configStore, err := config.NewStoreFromDSN(dsn, readOnly, configDefaults) + configStore, err := config.NewStoreFromDSN(dsn, readOnly, configDefaults, true) if err != nil { return errors.Wrap(err, "failed to apply Config option") } diff --git a/app/server.go b/app/server.go index d21f0a1212..125f4b18e7 100644 --- a/app/server.go +++ b/app/server.go @@ -210,7 +210,7 @@ func NewServer(options ...Option) (*Server, error) { // // Step 1: Config. if s.configStore == nil { - innerStore, err := config.NewFileStore("config.json") + innerStore, err := config.NewFileStore("config.json", true) 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 7155146759..a9bfe660c3 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -168,7 +168,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") + configStore, _ := config.NewFileStore("config.json", true) store, _ := config.NewStoreFromBacking(configStore, nil, false) server.configStore = store server.UpdateConfig(func(cfg *model.Config) { diff --git a/cmd/mattermost/commands/db.go b/cmd/mattermost/commands/db.go index 88be76eb90..94f8625d62 100644 --- a/cmd/mattermost/commands/db.go +++ b/cmd/mattermost/commands/db.go @@ -68,7 +68,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, customDefaults) + configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, customDefaults, true) if err != nil { return errors.Wrap(err, "failed to load configuration") } diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index b24eae8520..06b8443ecf 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -49,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()), false, customDefaults) + configStore, err := config.NewStoreFromDSN(getConfigDSN(command, config.GetEnvironment()), false, customDefaults, true) if err != nil { return errors.Wrap(err, "failed to load configuration") } diff --git a/config/file.go b/config/file.go index 0bcda57bf2..cbf53cf73e 100644 --- a/config/file.go +++ b/config/file.go @@ -30,12 +30,25 @@ type FileStore struct { } // NewFileStore creates a new instance of a config store backed by the given file path. -func NewFileStore(path string) (fs *FileStore, err error) { +func NewFileStore(path string, createFileIfNotExists bool) (fs *FileStore, err error) { resolvedPath, err := resolveConfigFilePath(path) if err != nil { return nil, err } + f, err := os.Open(resolvedPath) + if err != nil && errors.Is(err, os.ErrNotExist) && createFileIfNotExists { + file, err2 := os.Create(resolvedPath) + if err2 != nil { + return nil, fmt.Errorf("could not create config file: %w", err2) + } + defer file.Close() + } else if err != nil { + return nil, err + } else { + defer f.Close() + } + return &FileStore{ path: resolvedPath, }, nil @@ -63,8 +76,6 @@ func resolveConfigFilePath(path string) (string, error) { return configFile, nil } - // Otherwise, search for the config/ folder using the same heuristics as above, and build - // an absolute path anchored there and joining the given input path (or plain filename). if configFolder, found := fileutils.FindDir("config"); found { return filepath.Join(configFolder, path), nil } diff --git a/config/file_test.go b/config/file_test.go index 2b0a624045..0bc2235a59 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) + fs, err := NewFileStore(path, false) 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) + fs, err := NewFileStore(path, false) 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) + fs, err := NewFileStore(path, false) 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) + fs, err := NewFileStore(path, false) 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) + fs, err := NewFileStore(path, false) 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) + fs, err := NewFileStore(path, true) 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) + fs, err := NewFileStore(path, true) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, customConfigDefaults, false) require.NoError(t, err) @@ -208,10 +208,7 @@ func TestFileStoreNew(t *testing.T) { defer os.RemoveAll(tempDir) path := filepath.Join(tempDir, "does/not/exist") - fs, err := NewFileStore(path) - require.NoError(t, err) - configStore, err := NewStoreFromBacking(fs, nil, false) - require.Nil(t, configStore) + _, err = NewFileStore(path, true) require.Error(t, err) }) @@ -230,7 +227,7 @@ func TestFileStoreNew(t *testing.T) { ioutil.WriteFile(path, cfgData, 0644) - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) configStore, err := NewStoreFromBacking(fs, nil, false) require.NoError(t, err) @@ -249,14 +246,9 @@ func TestFileStoreNew(t *testing.T) { defer os.RemoveAll("config/TestFileStoreNew") path := "TestFileStoreNew/a/b/c/config.json" - fs, err := NewFileStore(path) - require.NoError(t, err) - configStore, err := NewStoreFromBacking(fs, nil, false) - require.NoError(t, err) - defer configStore.Close() - - assert.Equal(t, "", *configStore.Get().ServiceSettings.SiteURL) - assertFileNotEqualsConfig(t, testConfig, filepath.Join("config", path)) + fs, err := NewFileStore(path, false) + require.Error(t, err) + require.Nil(t, fs) }) } @@ -284,7 +276,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -296,7 +288,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - fsInner, err = NewFileStore(path) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -310,7 +302,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, customConfigDefaults, false) require.NoError(t, err) @@ -322,7 +314,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - fsInner, err = NewFileStore(path) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, customConfigDefaults, false) require.NoError(t, err) @@ -337,7 +329,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -349,7 +341,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true") defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLEUPLOADS") - fsInner, err = NewFileStore(path) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -363,7 +355,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -375,7 +367,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000") defer os.Unsetenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM") - fsInner, err = NewFileStore(path) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -389,7 +381,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -401,7 +393,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456") defer os.Unsetenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE") - fsInner, err = NewFileStore(path) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -415,7 +407,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -427,7 +419,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) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -441,7 +433,7 @@ func TestFileStoreGetEnivironmentOverrides(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -453,7 +445,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) + fsInner, err = NewFileStore(path, false) require.NoError(t, err) fs, err = NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -529,7 +521,7 @@ func TestFileStoreSet(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -630,7 +622,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -665,7 +657,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://overridePersistEnvVariables") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -690,7 +682,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true") defer os.Unsetenv("MM_PLUGINSETTINGS_ENABLEUPLOADS") - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -715,7 +707,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000") defer os.Unsetenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM") - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -740,7 +732,7 @@ func TestFileStoreLoad(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456") defer os.Unsetenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE") - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -765,7 +757,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) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -792,7 +784,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) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -814,7 +806,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -848,7 +840,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, fixesRequiredConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -867,7 +859,7 @@ func TestFileStoreLoad(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fsInner, err := NewFileStore(path) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, false) require.NoError(t, err) @@ -959,7 +951,7 @@ func TestFileGetFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1021,7 +1013,7 @@ func TestFileSetFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1072,7 +1064,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1085,7 +1077,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1101,7 +1093,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1124,7 +1116,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1137,7 +1129,7 @@ func TestFileHasFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1156,7 +1148,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1168,7 +1160,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1190,7 +1182,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1219,7 +1211,7 @@ func TestFileRemoveFile(t *testing.T) { path, tearDown := setupConfigFile(t, minimalConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1241,7 +1233,7 @@ func TestFileStoreString(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() - fs, err := NewFileStore(path) + fs, err := NewFileStore(path, false) require.NoError(t, err) defer fs.Close() @@ -1262,7 +1254,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) + fsInner, err := NewFileStore(path, false) require.NoError(t, err) fs, err := NewStoreFromBacking(fsInner, nil, true) require.NoError(t, err) @@ -1317,3 +1309,44 @@ func TestFileStoreSetReadOnlyFF(t *testing.T) { require.Equal(t, newCfg.FeatureFlags, config.FeatureFlags) }) } + +func TestResolveConfigPath(t *testing.T) { + t.Run("should be able to resolve an absolute path", func(t *testing.T) { + cf, err := ioutil.TempFile("", "config-test.json") + require.NoError(t, err) + info, err := cf.Stat() + require.NoError(t, err) + + file := filepath.Join(os.TempDir(), info.Name()) + + defer os.Remove(file) + + resolution, err := resolveConfigFilePath(file) + require.NoError(t, err) + require.Equal(t, file, resolution) + }) + + t.Run("should be able to resolve relative path", func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "resolveconfig") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + err = os.Chdir(tempDir) + require.NoError(t, err) + + file := "config-test-1.json" + _, err = os.Stat(file) + + if os.IsNotExist(err) { + defer os.Remove(file) + + f, err2 := os.Create(file) + require.NoError(t, err2) + defer f.Close() + } + + resolution, err := resolveConfigFilePath(file) + require.NoError(t, err) + require.Contains(t, resolution, filepath.Join(tempDir, file)) + }) +} diff --git a/config/migrate.go b/config/migrate.go index 8b19aabe83..ac4b4e597f 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, nil) + source, err := NewStoreFromDSN(from, false, nil, false) if err != nil { return errors.Wrapf(err, "failed to access source config %s", from) } defer source.Close() - destination, err := NewStoreFromDSN(to, false, nil) + destination, err := NewStoreFromDSN(to, false, nil, true) 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 64e60ad7f9..348f29bd5f 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) + destinationfile, err := NewFileStore(destinationDSN, false) 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) + sourcefile, err := NewFileStore(sourceDSN, true) 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 223bc5153f..9bd5c23548 100644 --- a/config/store.go +++ b/config/store.go @@ -83,13 +83,13 @@ func NewStoreFromBacking(backingStore BackingStore, customDefaults *model.Config // 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, readOnly bool, customDefaults *model.Config) (*Store, error) { +func NewStoreFromDSN(dsn string, readOnly bool, customDefaults *model.Config, createFileIfNotExist bool) (*Store, error) { var err error var backingStore BackingStore if IsDatabaseDSN(dsn) { backingStore, err = NewDatabaseStore(dsn) } else { - backingStore, err = NewFileStore(dsn) + backingStore, err = NewFileStore(dsn, createFileIfNotExist) } if err != nil { return nil, err diff --git a/config/store_test.go b/config/store_test.go index b32d710e21..dc13329522 100644 --- a/config/store_test.go +++ b/config/store_test.go @@ -27,13 +27,14 @@ 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, nil) - require.NoError(t, err) + ds, err2 := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), false, nil, false) + require.NoError(t, err2) ds.Close() }) t.Run("file dsn", func(t *testing.T) { - fs, err := NewStoreFromDSN("config.json", false, nil) + defer os.Remove("config_test.json") + fs, err := NewStoreFromDSN("config_test.json", false, nil, true) require.NoError(t, err) fs.Close() }) @@ -45,23 +46,23 @@ func TestNewStoreReadOnly(t *testing.T) { } sqlSettings := mainHelper.GetSQLSettings() - tempDir, err := ioutil.TempDir("", "TestNewStore") - require.NoError(t, err) + tempDir, tErr := ioutil.TempDir("", "TestNewStore") + require.NoError(t, tErr) - err = os.Chdir(tempDir) - require.NoError(t, err) + tErr = os.Chdir(tempDir) + require.NoError(t, tErr) 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), true, nil) + ds, err := NewStoreFromDSN(getDsn(*sqlSettings.DriverName, *sqlSettings.DataSource), true, nil, false) require.NoError(t, err) t.Run("Set", func(t *testing.T) { - oldCfg, newCfg, err := ds.Set(emptyConfig) + oldCfg, newCfg, err2 := ds.Set(emptyConfig) require.Nil(t, oldCfg) require.Nil(t, newCfg) - require.Equal(t, ErrReadOnlyStore, err) + require.Equal(t, ErrReadOnlyStore, err2) }) t.Run("SetFile", func(t *testing.T) { @@ -78,7 +79,7 @@ func TestNewStoreReadOnly(t *testing.T) { }) t.Run("file dsn", func(t *testing.T) { - fs, err := NewStoreFromDSN("config.json", true, nil) + fs, err := NewStoreFromDSN("config_test.json", true, nil, true) require.NoError(t, err) t.Run("Set", func(t *testing.T) { @@ -89,12 +90,12 @@ func TestNewStoreReadOnly(t *testing.T) { }) t.Run("SetFile", func(t *testing.T) { - err := fs.SetFile("config.json", []byte{}) + err := fs.SetFile("config_test.json", []byte{}) require.Equal(t, ErrReadOnlyStore, err) }) t.Run("RemoveFile", func(t *testing.T) { - err := fs.RemoveFile("config.json") + err := fs.RemoveFile("config_test.json") require.Equal(t, ErrReadOnlyStore, err) })