diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 67a6eddf1e..231b125ed5 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -5,7 +5,6 @@ package api4 import ( "fmt" - "io" "io/ioutil" "net" "net/http" @@ -18,11 +17,11 @@ import ( "time" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/mattermost/mattermost-server/web" "github.com/mattermost/mattermost-server/wsapi" @@ -31,9 +30,9 @@ import ( ) type TestHelper struct { - App *app.App - Server *app.Server - tempConfigPath string + App *app.App + Server *app.Server + ConfigStore config.Store Client *model.Client4 BasicUser *model.User @@ -64,22 +63,13 @@ func UseTestStore(store store.Store) { func setupTestHelper(enterprise bool, updateConfig func(*model.Config)) *TestHelper { testStore.DropAllTables() - permConfig, err := os.Open(fileutils.FindConfigFile("config.json")) + memoryStore, err := config.NewMemoryStore() if err != nil { - panic(err) - } - defer permConfig.Close() - tempConfig, err := ioutil.TempFile("", "") - if err != nil { - panic(err) - } - _, err = io.Copy(tempConfig, permConfig) - tempConfig.Close() - if err != nil { - panic(err) + panic("failed to initialize memory store: " + err.Error()) } - options := []app.Option{app.Config(tempConfig.Name(), false)} + var options []app.Option + options = append(options, app.ConfigStore(memoryStore)) options = append(options, app.StoreOverride(testStore)) s, err := app.NewServer(options...) @@ -88,9 +78,9 @@ func setupTestHelper(enterprise bool, updateConfig func(*model.Config)) *TestHel } th := &TestHelper{ - App: s.FakeApp(), - Server: s, - tempConfigPath: tempConfig.Name(), + App: s.FakeApp(), + Server: s, + ConfigStore: memoryStore, } th.App.UpdateConfig(func(cfg *model.Config) { @@ -180,7 +170,6 @@ func (me *TestHelper) TearDown() { utils.DisableDebugLogForTest() me.ShutdownApp() - os.Remove(me.tempConfigPath) utils.EnableDebugLogForTest() diff --git a/app/admin.go b/app/admin.go index 0723573552..7d54548f62 100644 --- a/app/admin.go +++ b/app/admin.go @@ -17,6 +17,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/mailservice" "github.com/mattermost/mattermost-server/utils" + "github.com/pkg/errors" ) func (a *App) GetLogs(page, perPage int) ([]string, *model.AppError) { @@ -162,7 +163,7 @@ func (a *App) GetEnvironmentConfig() map[string]interface{} { func (a *App) SaveConfig(newCfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { oldCfg, err := a.Srv.configStore.Set(newCfg) - if err == config.ErrReadOnlyConfiguration { + if errors.Cause(err) == config.ErrReadOnlyConfiguration { return model.NewAppError("saveConfig", "ent.cluster.save_config.error", nil, err.Error(), http.StatusForbidden) } else if err != nil { return model.NewAppError("saveConfig", "app.save_config.app_error", nil, err.Error(), http.StatusInternalServerError) @@ -177,7 +178,7 @@ func (a *App) SaveConfig(newCfg *model.Config, sendConfigChangeClusterMessage bo } if a.Cluster != nil { - err := a.Cluster.ConfigChanged(newCfg, oldCfg, sendConfigChangeClusterMessage) + err := a.Cluster.ConfigChanged(oldCfg, newCfg, sendConfigChangeClusterMessage) if err != nil { return err } diff --git a/app/config.go b/app/config.go index 59c4a2453d..8d61422884 100644 --- a/app/config.go +++ b/app/config.go @@ -17,6 +17,8 @@ import ( "strconv" "time" + "github.com/pkg/errors" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" @@ -346,3 +348,13 @@ func (a *App) LimitedClientConfigWithComputed() map[string]string { return respCfg } + +// GetConfigFile proxies access to the given configuration file to the underlying config store. +func (a *App) GetConfigFile(name string) ([]byte, error) { + data, err := a.Srv.configStore.GetFile(name) + if err != nil { + return nil, errors.Wrapf(err, "failed to get config file %s", name) + } + + return data, nil +} diff --git a/app/helper_test.go b/app/helper_test.go index f09ba6bd98..96ce10238c 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -4,7 +4,6 @@ package app import ( - "io" "io/ioutil" "os" "path/filepath" @@ -12,10 +11,10 @@ import ( "testing" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" ) type TestHelper struct { @@ -29,31 +28,21 @@ type TestHelper struct { SystemAdminUser *model.User - tempConfigPath string - tempWorkspace string + tempWorkspace string } func setupTestHelper(enterprise bool, tb testing.TB) *TestHelper { store := mainHelper.GetStore() store.DropAllTables() - permConfig, err := os.Open(fileutils.FindConfigFile("config.json")) + memoryStore, err := config.NewMemoryStore() if err != nil { - panic(err) - } - defer permConfig.Close() - tempConfig, err := ioutil.TempFile("", "") - if err != nil { - panic(err) - } - _, err = io.Copy(tempConfig, permConfig) - tempConfig.Close() - if err != nil { - panic(err) + panic("failed to initialize memory store: " + err.Error()) } - options := []Option{Config(tempConfig.Name(), false)} - options = append(options, StoreOverride(store)) + var options []Option + options = append(options, ConfigStore(memoryStore)) + options = append(options, StoreOverride(mainHelper.Store)) options = append(options, SetLogger(mlog.NewTestingLogger(tb))) s, err := NewServer(options...) @@ -62,9 +51,8 @@ func setupTestHelper(enterprise bool, tb testing.TB) *TestHelper { } th := &TestHelper{ - App: s.FakeApp(), - Server: s, - tempConfigPath: tempConfig.Name(), + App: s.FakeApp(), + Server: s, } th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.MaxUsersPerTeam = 50 }) @@ -418,7 +406,6 @@ func (me *TestHelper) ShutdownApp() { func (me *TestHelper) TearDown() { me.ShutdownApp() - os.Remove(me.tempConfigPath) if err := recover(); err != nil { panic(err) } diff --git a/app/saml.go b/app/saml.go index f985b328a5..089e4a9637 100644 --- a/app/saml.go +++ b/app/saml.go @@ -4,15 +4,11 @@ package app import ( - "io" + "io/ioutil" "mime/multipart" "net/http" - "os" - "path/filepath" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" ) const ( @@ -34,26 +30,28 @@ func (a *App) GetSamlMetadata() (string, *model.AppError) { return result, nil } -func WriteSamlFile(filename string, fileData *multipart.FileHeader) *model.AppError { +func (a *App) writeSamlFile(filename string, fileData *multipart.FileHeader) *model.AppError { file, err := fileData.Open() if err != nil { return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.open.app_error", nil, err.Error(), http.StatusInternalServerError) } defer file.Close() - configDir, _ := fileutils.FindDir("config") - out, err := os.Create(filepath.Join(configDir, filename)) + data, err := ioutil.ReadAll(file) + if err != nil { + return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.saving.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + err = a.Srv.configStore.SetFile(filename, data) if err != nil { return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.saving.app_error", nil, err.Error(), http.StatusInternalServerError) } - defer out.Close() - io.Copy(out, file) return nil } func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.AppError { - if err := WriteSamlFile(SamlPublicCertificateName, fileData); err != nil { + if err := a.writeSamlFile(SamlPublicCertificateName, fileData); err != nil { return err } @@ -70,7 +68,7 @@ func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.Ap } func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.AppError { - if err := WriteSamlFile(SamlPrivateKeyName, fileData); err != nil { + if err := a.writeSamlFile(SamlPrivateKeyName, fileData); err != nil { return err } @@ -87,7 +85,7 @@ func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.A } func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppError { - if err := WriteSamlFile(SamlIdpCertificateName, fileData); err != nil { + if err := a.writeSamlFile(SamlIdpCertificateName, fileData); err != nil { return err } @@ -103,16 +101,16 @@ func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppEr return nil } -func RemoveSamlFile(filename string) *model.AppError { - if err := os.Remove(fileutils.FindConfigFile(filename)); err != nil { - return model.NewAppError("removeCertificate", "api.admin.remove_certificate.delete.app_error", map[string]interface{}{"Filename": filename}, filename+": "+err.Error(), http.StatusInternalServerError) +func (a *App) removeSamlFile(filename string) *model.AppError { + if err := a.Srv.configStore.RemoveFile(filename); err != nil { + return model.NewAppError("RemoveSamlFile", "api.admin.remove_certificate.delete.app_error", map[string]interface{}{"Filename": filename}, err.Error(), http.StatusInternalServerError) } return nil } func (a *App) RemoveSamlPublicCertificate() *model.AppError { - if err := RemoveSamlFile(*a.Config().SamlSettings.PublicCertificateFile); err != nil { + if err := a.removeSamlFile(*a.Config().SamlSettings.PublicCertificateFile); err != nil { return err } @@ -130,7 +128,7 @@ func (a *App) RemoveSamlPublicCertificate() *model.AppError { } func (a *App) RemoveSamlPrivateCertificate() *model.AppError { - if err := RemoveSamlFile(*a.Config().SamlSettings.PrivateKeyFile); err != nil { + if err := a.removeSamlFile(*a.Config().SamlSettings.PrivateKeyFile); err != nil { return err } @@ -148,7 +146,7 @@ func (a *App) RemoveSamlPrivateCertificate() *model.AppError { } func (a *App) RemoveSamlIdpCertificate() *model.AppError { - if err := RemoveSamlFile(*a.Config().SamlSettings.IdpCertificateFile); err != nil { + if err := a.removeSamlFile(*a.Config().SamlSettings.IdpCertificateFile); err != nil { return err } @@ -168,9 +166,9 @@ func (a *App) RemoveSamlIdpCertificate() *model.AppError { func (a *App) GetSamlCertificateStatus() *model.SamlCertificateStatus { status := &model.SamlCertificateStatus{} - status.IdpCertificateFile = utils.FileExistsInConfigFolder(*a.Config().SamlSettings.IdpCertificateFile) - status.PrivateKeyFile = utils.FileExistsInConfigFolder(*a.Config().SamlSettings.PrivateKeyFile) - status.PublicCertificateFile = utils.FileExistsInConfigFolder(*a.Config().SamlSettings.PublicCertificateFile) + status.IdpCertificateFile, _ = a.Srv.configStore.HasFile(*a.Config().SamlSettings.IdpCertificateFile) + status.PrivateKeyFile, _ = a.Srv.configStore.HasFile(*a.Config().SamlSettings.PrivateKeyFile) + status.PublicCertificateFile, _ = a.Srv.configStore.HasFile(*a.Config().SamlSettings.PublicCertificateFile) return status } diff --git a/app/server_test.go b/app/server_test.go index 665fe66201..4ab34b7a31 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -34,10 +34,16 @@ func TestStartServerSuccess(t *testing.T) { func TestStartServerRateLimiterCriticalError(t *testing.T) { // Attempt to use Rate Limiter with an invalid config - ms, err := config.NewMemoryStore(true) + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + SkipValidation: true, + }) + require.NoError(t, err) + + config := ms.Get() + *config.RateLimitSettings.Enable = true + *config.RateLimitSettings.MaxBurst = -100 + _, err = ms.Set(config) require.NoError(t, err) - *ms.Config.RateLimitSettings.Enable = true - *ms.Config.RateLimitSettings.MaxBurst = -100 s, err := NewServer(ConfigStore(ms)) require.NoError(t, err) diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index 17085134b1..009bf00a4e 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -17,7 +17,6 @@ import ( "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" ) var ConfigCmd = &cobra.Command{ @@ -84,33 +83,12 @@ func init() { func configValidateCmdF(command *cobra.Command, args []string) error { utils.TranslationsPreInit() model.AppErrorInit(utils.T) - filePath, err := command.Flags().GetString("config") + + _, err := getConfigStore(command) if err != nil { return err } - filePath = fileutils.FindConfigFile(filePath) - - file, err := os.Open(filePath) - if err != nil { - return err - } - - decoder := json.NewDecoder(file) - config := model.Config{} - err = decoder.Decode(&config) - if err != nil { - return err - } - - if _, err := file.Stat(); err != nil { - return err - } - - if err := config.IsValid(); err != nil { - return errors.New(utils.T(err.Id)) - } - CommandPrettyPrintln("The document is valid") return nil } diff --git a/cmd/mattermost/commands/config_test.go b/cmd/mattermost/commands/config_test.go index 8480219d9f..4886095a21 100644 --- a/cmd/mattermost/commands/config_test.go +++ b/cmd/mattermost/commands/config_test.go @@ -4,12 +4,15 @@ package commands import ( + "io/ioutil" + "os" "reflect" "sort" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" ) @@ -71,7 +74,11 @@ func TestConfigValidate(t *testing.T) { th := Setup() defer th.TearDown() - assert.Error(t, th.RunCommand(t, "--config", "foo.json", "config", "validate")) + tempFile, err := ioutil.TempFile("", "TestConfigValidate") + require.NoError(t, err) + defer os.Remove(tempFile.Name()) + + assert.Error(t, th.RunCommand(t, "--config", tempFile.Name(), "config", "validate")) th.CheckCommand(t, "config", "validate") } diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index 855c92ba48..21bbe4d1e1 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -11,6 +11,7 @@ import ( "github.com/mattermost/mattermost-server/api4" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/manualtesting" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/web" @@ -31,7 +32,7 @@ func init() { } func serverCmdF(command *cobra.Command, args []string) error { - config, err := command.Flags().GetString("config") + configDSN, err := command.Flags().GetString("config") if err != nil { return err } @@ -40,12 +41,18 @@ func serverCmdF(command *cobra.Command, args []string) error { usedPlatform, _ := command.Flags().GetBool("platform") interruptChan := make(chan os.Signal, 1) - return runServer(config, disableConfigWatch, usedPlatform, interruptChan) + + configStore, err := config.NewStore(configDSN, !disableConfigWatch) + if err != nil { + return err + } + + return runServer(configStore, disableConfigWatch, usedPlatform, interruptChan) } -func runServer(configDSN string, disableConfigWatch bool, usedPlatform bool, interruptChan chan os.Signal) error { +func runServer(configStore config.Store, disableConfigWatch bool, usedPlatform bool, interruptChan chan os.Signal) error { options := []app.Option{ - app.Config(configDSN, !disableConfigWatch), + app.ConfigStore(configStore), app.RunJobs, app.JoinCluster, app.StartElasticsearch, diff --git a/cmd/mattermost/commands/server_test.go b/cmd/mattermost/commands/server_test.go index 5771d65400..c34a3358b8 100644 --- a/cmd/mattermost/commands/server_test.go +++ b/cmd/mattermost/commands/server_test.go @@ -10,22 +10,22 @@ import ( "syscall" "testing" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/jobs" - "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/stretchr/testify/require" ) type ServerTestHelper struct { - configPath string + configStore config.Store disableConfigWatch bool interruptChan chan os.Signal originalInterval int } func SetupServerTest() *ServerTestHelper { - // Build a channel that will be used by the server to receive system signals… + // Build a channel that will be used by the server to receive system signals... interruptChan := make(chan os.Signal, 1) - // …and sent it immediately a SIGINT value. + // ...and sent it immediately a SIGINT value. // This will make the server loop stop as soon as it started successfully. interruptChan <- syscall.SIGINT @@ -36,7 +36,6 @@ func SetupServerTest() *ServerTestHelper { jobs.DEFAULT_WATCHER_POLLING_INTERVAL = 200 th := &ServerTestHelper{ - configPath: fileutils.FindConfigFile("config.json"), disableConfigWatch: true, interruptChan: interruptChan, originalInterval: originalInterval, @@ -52,24 +51,11 @@ func TestRunServerSuccess(t *testing.T) { th := SetupServerTest() defer th.TearDownServerTest() - err := runServer(th.configPath, th.disableConfigWatch, false, th.interruptChan) + configStore, err := config.NewMemoryStore() require.NoError(t, err) -} -func TestRunServerInvalidConfigFile(t *testing.T) { - th := SetupServerTest() - defer th.TearDownServerTest() - - // Start the server with an unreadable config file - unreadableConfigFile, err := ioutil.TempFile("", "mattermost-unreadable-config-file-") - if err != nil { - panic(err) - } - os.Chmod(unreadableConfigFile.Name(), 0200) - defer os.Remove(unreadableConfigFile.Name()) - - err = runServer(unreadableConfigFile.Name(), th.disableConfigWatch, false, th.interruptChan) - require.Error(t, err) + err = runServer(configStore, th.disableConfigWatch, false, th.interruptChan) + require.NoError(t, err) } func TestRunServerSystemdNotification(t *testing.T) { @@ -113,8 +99,11 @@ func TestRunServerSystemdNotification(t *testing.T) { ch <- string(data) }(socketReader) + configStore, err := config.NewMemoryStore() + require.NoError(t, err) + // Start and stop the server - err = runServer(th.configPath, th.disableConfigWatch, false, th.interruptChan) + err = runServer(configStore, th.disableConfigWatch, false, th.interruptChan) require.NoError(t, err) // Ensure the notification has been sent on the socket and is correct @@ -131,6 +120,9 @@ func TestRunServerNoSystemd(t *testing.T) { os.Unsetenv("NOTIFY_SOCKET") defer os.Setenv("NOTIFY_SOCKET", originalSocket) - err := runServer(th.configPath, th.disableConfigWatch, false, th.interruptChan) + configStore, err := config.NewMemoryStore() + require.NoError(t, err) + + err = runServer(configStore, th.disableConfigWatch, false, th.interruptChan) require.NoError(t, err) } diff --git a/config/common.go b/config/common.go index 416060074b..a75e17c65f 100644 --- a/config/common.go +++ b/config/common.go @@ -40,7 +40,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, isValid func(*model.Config) error, persist func(*model.Config) error) (*model.Config, error) { +func (cs *commonStore) set(newCfg *model.Config, validate func(*model.Config) error, persist func(*model.Config) error) (*model.Config, error) { cs.configLock.Lock() var unlockOnce sync.Once defer unlockOnce.Do(cs.configLock.Unlock) @@ -57,18 +57,13 @@ func (cs *commonStore) set(newCfg *model.Config, isValid func(*model.Config) err newCfg = newCfg.Clone() newCfg.SetDefaults() - // Sometimes the config is received with "fake" data in sensitive fielcs. Apply the real + // 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 err := newCfg.IsValid(); err != nil { - return nil, errors.Wrap(err, "new configuration is invalid") - } - - // Allow backing-store specific checks. - if isValid != nil { - if err := isValid(newCfg); err != nil { - return nil, err + if validate != nil { + if err := validate(newCfg); err != nil { + return nil, errors.Wrap(err, "new configuration is invalid") } } @@ -90,7 +85,7 @@ func (cs *commonStore) set(newCfg *model.Config, isValid func(*model.Config) err // load updates the current configuration from the given io.ReadCloser. // // This function assumes no lock has been acquired, as it acquires a write lock itself. -func (cs *commonStore) load(f io.ReadCloser, needsSave bool, persist func(*model.Config) error) error { +func (cs *commonStore) load(f io.ReadCloser, needsSave bool, validate func(*model.Config) error, persist func(*model.Config) error) error { allowEnvironmentOverrides := true loadedCfg, environmentOverrides, err := unmarshalConfig(f, allowEnvironmentOverrides) if err != nil { @@ -98,16 +93,17 @@ func (cs *commonStore) load(f io.ReadCloser, needsSave bool, persist func(*model } // SetDefaults generates various keys and salts if not previously configured. Determine if - // such a change will be made before invoking. This method will not effect the save: that - // remains the responsibility of the caller. + // such a change will be made before invoking. needsSave = needsSave || loadedCfg.SqlSettings.AtRestEncryptKey == nil || len(*loadedCfg.SqlSettings.AtRestEncryptKey) == 0 needsSave = needsSave || loadedCfg.FileSettings.PublicLinkSalt == nil || len(*loadedCfg.FileSettings.PublicLinkSalt) == 0 needsSave = needsSave || loadedCfg.EmailSettings.InviteSalt == nil || len(*loadedCfg.EmailSettings.InviteSalt) == 0 loadedCfg.SetDefaults() - if err := loadedCfg.IsValid(); err != nil { - return errors.Wrap(err, "invalid config") + if validate != nil { + if err = validate(loadedCfg); err != nil { + return errors.Wrap(err, "invalid config") + } } if changed := fixConfig(loadedCfg); changed { @@ -118,7 +114,7 @@ func (cs *commonStore) load(f io.ReadCloser, needsSave bool, persist func(*model var unlockOnce sync.Once defer unlockOnce.Do(cs.configLock.Unlock) - if needsSave { + if needsSave && persist != nil { if err = persist(loadedCfg); err != nil { return errors.Wrap(err, "failed to persist required changes after load") } @@ -136,3 +132,12 @@ func (cs *commonStore) load(f io.ReadCloser, needsSave bool, persist func(*model return nil } + +// validate checks if the given configuration is valid +func (cs *commonStore) validate(cfg *model.Config) error { + if err := cfg.IsValid(); err != nil { + return err + } + + return nil +} diff --git a/config/common_test.go b/config/common_test.go index 237020a7e0..b052554916 100644 --- a/config/common_test.go +++ b/config/common_test.go @@ -6,6 +6,71 @@ import ( "github.com/mattermost/mattermost-server/model" ) +var emptyConfig, readOnlyConfig, minimalConfig, invalidConfig, fixesRequiredConfig, ldapConfig, testConfig *model.Config + +func init() { + emptyConfig = &model.Config{} + readOnlyConfig = &model.Config{ + ClusterSettings: model.ClusterSettings{ + Enable: bToP(true), + ReadOnlyConfig: bToP(true), + }, + } + minimalConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://minimal"), + }, + SqlSettings: model.SqlSettings{ + AtRestEncryptKey: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + FileSettings: model.FileSettings{ + PublicLinkSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + EmailSettings: model.EmailSettings{ + InviteSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + LocalizationSettings: model.LocalizationSettings{ + DefaultServerLocale: sToP("en"), + DefaultClientLocale: sToP("en"), + }, + } + invalidConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("invalid"), + }, + } + fixesRequiredConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://trailingslash/"), + }, + SqlSettings: model.SqlSettings{ + AtRestEncryptKey: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + FileSettings: model.FileSettings{ + DriverName: sToP(model.IMAGE_DRIVER_LOCAL), + Directory: sToP("/path/to/directory"), + PublicLinkSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + EmailSettings: model.EmailSettings{ + InviteSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), + }, + LocalizationSettings: model.LocalizationSettings{ + DefaultServerLocale: sToP("garbage"), + DefaultClientLocale: sToP("garbage"), + }, + } + ldapConfig = &model.Config{ + LdapSettings: model.LdapSettings{ + BindPassword: sToP("password"), + }, + } + testConfig = &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://TestStoreNew"), + }, + } +} + func prepareExpectedConfig(t *testing.T, expectedCfg *model.Config) *model.Config { // These fields require special initialization for our tests. expectedCfg = expectedCfg.Clone() diff --git a/config/database.go b/config/database.go index cfc95ae029..1fbd9a64ef 100644 --- a/config/database.go +++ b/config/database.go @@ -75,6 +75,18 @@ func initializeConfigurationsTable(db *sqlx.DB) error { return errors.Wrap(err, "failed to create Configurations table") } + _, err = db.Exec(` + CREATE TABLE IF NOT EXISTS ConfigurationFiles ( + Name VARCHAR(64) PRIMARY KEY, + Data TEXT NOT NULL, + CreateAt BIGINT NOT NULL, + UpdateAt BIGINT NOT NULL + ) + `) + if err != nil { + return errors.Wrap(err, "failed to create ConfigurationFiles table") + } + return nil } @@ -113,8 +125,7 @@ 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, nil, ds.persist) - + return ds.commonStore.set(newCfg, ds.commonStore.validate, ds.persist) } // persist writes the configuration to the configured database. @@ -146,6 +157,16 @@ func (ds *DatabaseStore) persist(cfg *model.Config) error { "key": "ConfigurationId", } + // Skip the persist altogether if we're effectively writing the same configuration. + var oldValue []byte + row := ds.db.QueryRow("SELECT Value FROM Configurations WHERE Active") + if err := row.Scan(&oldValue); err != nil && err != sql.ErrNoRows { + return errors.Wrap(err, "failed to query active configuration") + } + if bytes.Equal(oldValue, b) { + return nil + } + if _, err := tx.Exec("UPDATE Configurations SET Active = NULL WHERE Active"); err != nil { return errors.Wrap(err, "failed to deactivate current configuration") } @@ -175,7 +196,7 @@ func (ds *DatabaseStore) Load() (err error) { if len(configurationData) == 0 { needsSave = true - defaultCfg := model.Config{} + defaultCfg := &model.Config{} defaultCfg.SetDefaults() // Assume the database storing the config is also to be used for the application. @@ -184,13 +205,90 @@ func (ds *DatabaseStore) Load() (err error) { *defaultCfg.SqlSettings.DriverName = ds.driverName *defaultCfg.SqlSettings.DataSource = ds.dataSourceName - configurationData, err = marshalConfig(&defaultCfg) + configurationData, err = marshalConfig(defaultCfg) if err != nil { return errors.Wrap(err, "failed to serialize default config") } } - return ds.commonStore.load(ioutil.NopCloser(bytes.NewReader(configurationData)), needsSave, ds.persist) + return ds.commonStore.load(ioutil.NopCloser(bytes.NewReader(configurationData)), needsSave, ds.commonStore.validate, ds.persist) +} + +// GetFile fetches the contents of a previously persisted configuration file. +func (ds *DatabaseStore) GetFile(name string) ([]byte, error) { + query, args, err := sqlx.Named("SELECT Data FROM ConfigurationFiles WHERE Name = :name", map[string]interface{}{ + "name": name, + }) + if err != nil { + return nil, err + } + + var data []byte + row := ds.db.QueryRowx(query, args...) + if err = row.Scan(&data); err != nil { + return nil, errors.Wrapf(err, "failed to scan data from row for %s", name) + } + + return data, nil +} + +// SetFile sets or replaces the contents of a configuration file. +func (ds *DatabaseStore) SetFile(name string, data []byte) error { + params := map[string]interface{}{ + "name": name, + "data": data, + "create_at": model.GetMillis(), + "update_at": model.GetMillis(), + } + + result, err := ds.db.NamedExec("UPDATE ConfigurationFiles SET Data = :data, UpdateAt = :update_at WHERE Name = :name", params) + if err != nil { + return errors.Wrapf(err, "failed to update row for %s", name) + } + + count, err := result.RowsAffected() + if err != nil { + return errors.Wrapf(err, "failed to count rows affected for %s", name) + } else if count > 0 { + return nil + } + + _, err = ds.db.NamedExec("INSERT INTO ConfigurationFiles (Name, Data, CreateAt, UpdateAt) VALUES (:name, :data, :create_at, :update_at)", params) + if err != nil { + return errors.Wrapf(err, "failed to insert row for %s", name) + } + + return nil +} + +// HasFile returns true if the given file was previously persisted. +func (ds *DatabaseStore) HasFile(name string) (bool, error) { + query, args, err := sqlx.Named("SELECT COUNT(*) FROM ConfigurationFiles WHERE Name = :name", map[string]interface{}{ + "name": name, + }) + if err != nil { + return false, err + } + + var count int + row := ds.db.QueryRowx(query, args...) + if err = row.Scan(&count); err != nil { + return false, errors.Wrapf(err, "failed to scan count of rows for %s", name) + } + + return count != 0, nil +} + +// RemoveFile remoevs a previously persisted configuration file. +func (ds *DatabaseStore) RemoveFile(name string) error { + _, err := ds.db.NamedExec("DELETE FROM ConfigurationFiles WHERE Name = :name", map[string]interface{}{ + "name": name, + }) + if err != nil { + return errors.Wrapf(err, "failed to remove row for %s", name) + } + + return nil } // String returns the path to the database backing the config, masking the password. diff --git a/config/database_test.go b/config/database_test.go index a3c03bdaa4..1e3d89944f 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -20,7 +20,7 @@ import ( "github.com/mattermost/mattermost-server/model" ) -func setupConfigDatabase(t *testing.T, cfg *model.Config) (string, func()) { +func setupConfigDatabase(t *testing.T, cfg *model.Config, files map[string][]byte) (string, func()) { t.Helper() os.Clearenv() truncateTables(t) @@ -40,24 +40,39 @@ func setupConfigDatabase(t *testing.T, cfg *model.Config) (string, func()) { }) require.NoError(t, err) + for name, data := range files { + params := map[string]interface{}{ + "name": name, + "data": data, + "create_at": model.GetMillis(), + "update_at": model.GetMillis(), + } + + _, err = db.NamedExec("INSERT INTO ConfigurationFiles (Name, Data, CreateAt, UpdateAt) VALUES (:name, :data, :create_at, :update_at)", params) + require.NoError(t, err) + } + return id, func() { truncateTables(t) } } // getActualDatabaseConfig returns the active configuration in the database without relying on a config store. -func getActualDatabaseConfig(t *testing.T) *model.Config { +func getActualDatabaseConfig(t *testing.T) (string, *model.Config) { t.Helper() - var actualCfgData []byte + var actual struct { + Id string `db:"Id"` + Value []byte `db:"Value"` + } db := sqlx.NewDb(mainHelper.GetSqlSupplier().GetMaster().Db, *mainHelper.GetSqlSettings().DriverName) - err := db.Get(&actualCfgData, "SELECT Value FROM Configurations WHERE Active") + err := db.Get(&actual, "SELECT Id, Value FROM Configurations WHERE Active") require.NoError(t, err) - actualCfg, _, err := config.UnmarshalConfig(bytes.NewReader(actualCfgData), false) + actualCfg, _, err := config.UnmarshalConfig(bytes.NewReader(actual.Value), false) require.Nil(t, err) - return actualCfg + return actual.Id, actualCfg } // assertDatabaseEqualsConfig verifies the active in-database configuration equals the given config. @@ -65,7 +80,7 @@ func assertDatabaseEqualsConfig(t *testing.T, expectedCfg *model.Config) { t.Helper() expectedCfg = prepareExpectedConfig(t, expectedCfg) - actualCfg := getActualDatabaseConfig(t) + _, actualCfg := getActualDatabaseConfig(t) assert.Equal(t, expectedCfg, actualCfg) } @@ -74,7 +89,7 @@ func assertDatabaseNotEqualsConfig(t *testing.T, expectedCfg *model.Config) { t.Helper() expectedCfg = prepareExpectedConfig(t, expectedCfg) - actualCfg := getActualDatabaseConfig(t) + _, actualCfg := getActualDatabaseConfig(t) assert.NotEqual(t, expectedCfg, actualCfg) } @@ -90,7 +105,7 @@ func TestDatabaseStoreNew(t *testing.T) { }) t.Run("existing config, initialization required", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, testConfig) + _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -102,7 +117,7 @@ func TestDatabaseStoreNew(t *testing.T) { }) t.Run("already minimally configured", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, minimalConfig) + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -125,7 +140,7 @@ func TestDatabaseStoreNew(t *testing.T) { } func TestDatabaseStoreGet(t *testing.T) { - _, tearDown := setupConfigDatabase(t, testConfig) + _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() sqlSettings := mainHelper.GetSqlSettings() @@ -150,7 +165,7 @@ func TestDatabaseStoreGet(t *testing.T) { } func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) { - _, tearDown := setupConfigDatabase(t, testConfig) + _, tearDown := setupConfigDatabase(t, testConfig, nil) defer tearDown() sqlSettings := mainHelper.GetSqlSettings() @@ -177,7 +192,7 @@ func TestDatabaseStoreSet(t *testing.T) { t.Run("set same pointer value", func(t *testing.T) { t.Skip("not yet implemented") - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -191,7 +206,7 @@ func TestDatabaseStoreSet(t *testing.T) { }) t.Run("defaults required", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, minimalConfig) + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -210,7 +225,7 @@ func TestDatabaseStoreSet(t *testing.T) { }) t.Run("desanitization required", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, ldapConfig) + _, tearDown := setupConfigDatabase(t, ldapConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -230,7 +245,7 @@ func TestDatabaseStoreSet(t *testing.T) { }) t.Run("invalid", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -248,8 +263,27 @@ func TestDatabaseStoreSet(t *testing.T) { assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *ds.Get().ServiceSettings.SiteURL) }) + t.Run("duplicate ignored", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + _, err = ds.Set(ds.Get()) + require.NoError(t, err) + + beforeId, _ := getActualDatabaseConfig(t) + _, err = ds.Set(ds.Get()) + require.NoError(t, err) + + afterId, _ := getActualDatabaseConfig(t) + assert.Equal(t, beforeId, afterId, "new record should not have been written") + }) + t.Run("read-only ignored", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, readOnlyConfig) + _, tearDown := setupConfigDatabase(t, readOnlyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -269,7 +303,7 @@ func TestDatabaseStoreSet(t *testing.T) { }) t.Run("set with automatic save", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, minimalConfig) + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -293,7 +327,7 @@ func TestDatabaseStoreSet(t *testing.T) { t.Run("persist failed", func(t *testing.T) { t.Skip("skipping persistence test inside Set") - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -315,7 +349,7 @@ func TestDatabaseStoreSet(t *testing.T) { }) t.Run("listeners notified", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, emptyConfig) + activeId, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -336,6 +370,9 @@ func TestDatabaseStoreSet(t *testing.T) { require.NoError(t, err) assert.Equal(t, oldCfg, retCfg) + id, _ := getActualDatabaseConfig(t) + assert.NotEqual(t, activeId, id, "new record should have been written") + select { case <-called: case <-time.After(5 * time.Second): @@ -348,7 +385,7 @@ func TestDatabaseStoreLoad(t *testing.T) { sqlSettings := mainHelper.GetSqlSettings() t.Run("active configuration no longer exists", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -363,7 +400,7 @@ func TestDatabaseStoreLoad(t *testing.T) { }) t.Run("honour environment", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, minimalConfig) + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -379,7 +416,7 @@ func TestDatabaseStoreLoad(t *testing.T) { }) t.Run("invalid", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -406,7 +443,7 @@ func TestDatabaseStoreLoad(t *testing.T) { }) t.Run("fixes required", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, fixesRequiredConfig) + _, tearDown := setupConfigDatabase(t, fixesRequiredConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -420,7 +457,7 @@ func TestDatabaseStoreLoad(t *testing.T) { }) t.Run("listeners notifed", func(t *testing.T) { - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -444,8 +481,175 @@ func TestDatabaseStoreLoad(t *testing.T) { }) } +func TestDatabaseGetFile(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, map[string][]byte{ + "empty-file": []byte{}, + "test-file": []byte("test"), + }) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + t.Run("get empty filename", func(t *testing.T) { + _, err := ds.GetFile("") + require.Error(t, err) + }) + + t.Run("get non-existent file", func(t *testing.T) { + _, err := ds.GetFile("unknown") + require.Error(t, err) + }) + + t.Run("get empty file", func(t *testing.T) { + data, err := ds.GetFile("empty-file") + require.NoError(t, err) + require.Empty(t, data) + }) + + t.Run("get non-empty file", func(t *testing.T) { + data, err := ds.GetFile("test-file") + require.NoError(t, err) + require.Equal(t, []byte("test"), data) + }) +} + +func TestDatabaseSetFile(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + t.Run("set new file", func(t *testing.T) { + err := ds.SetFile("new", []byte("new file")) + require.NoError(t, err) + + data, err := ds.GetFile("new") + require.NoError(t, err) + require.Equal(t, []byte("new file"), data) + }) + + t.Run("overwrite existing file", func(t *testing.T) { + err := ds.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + err = ds.SetFile("existing", []byte("overwritten file")) + require.NoError(t, err) + + data, err := ds.GetFile("existing") + require.NoError(t, err) + require.Equal(t, []byte("overwritten file"), data) + }) +} + +func TestDatabaseHasFile(t *testing.T) { + t.Run("has non-existent", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + has, err := ds.HasFile("non-existent") + require.NoError(t, err) + require.False(t, has) + }) + + t.Run("has existing", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + err = ds.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + has, err := ds.HasFile("existing") + require.NoError(t, err) + require.True(t, has) + }) + + t.Run("has manually created file", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, map[string][]byte{ + "manual": []byte("manual file"), + }) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + has, err := ds.HasFile("manual") + require.NoError(t, err) + require.True(t, has) + }) +} + +func TestDatabaseRemoveFile(t *testing.T) { + t.Run("remove non-existent", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + err = ds.RemoveFile("non-existent") + require.NoError(t, err) + }) + + t.Run("remove existing", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, nil) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + err = ds.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + err = ds.RemoveFile("existing") + require.NoError(t, err) + + has, err := ds.HasFile("existing") + require.NoError(t, err) + require.False(t, has) + + _, err = ds.GetFile("existing") + require.Error(t, err) + }) + + t.Run("remove manually created file", func(t *testing.T) { + _, tearDown := setupConfigDatabase(t, minimalConfig, map[string][]byte{ + "manual": []byte("manual file"), + }) + defer tearDown() + + ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *mainHelper.Settings.DriverName, *mainHelper.Settings.DataSource)) + require.NoError(t, err) + defer ds.Close() + + err = ds.RemoveFile("manual") + require.NoError(t, err) + + has, err := ds.HasFile("manual") + require.NoError(t, err) + require.False(t, has) + + _, err = ds.GetFile("manual") + require.Error(t, err) + }) +} + func TestDatabaseStoreString(t *testing.T) { - _, tearDown := setupConfigDatabase(t, emptyConfig) + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() sqlSettings := mainHelper.GetSqlSettings() diff --git a/config/export_test.go b/config/export_test.go index e818975c94..b8ef523982 100644 --- a/config/export_test.go +++ b/config/export_test.go @@ -21,3 +21,8 @@ func UnmarshalConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config func InitializeConfigurationsTable(db *sqlx.DB) error { return initializeConfigurationsTable(db) } + +// ResolveConfigFilePath exposes the internal resolveConfigFilePath to test only. +func ResolveConfigFilePath(path string) (string, error) { + return resolveConfigFilePath(path) +} diff --git a/config/file.go b/config/file.go index 6475713e8d..69aa99d63b 100644 --- a/config/file.go +++ b/config/file.go @@ -23,6 +23,8 @@ var ( ) // FileStore is a config store backed by a file such as config/config.json. +// +// It also uses the folder containing the configuration file for storing other configuration files. type FileStore struct { commonStore @@ -67,11 +69,15 @@ func resolveConfigFilePath(path string) (string, error) { return path, nil } - // Search for the given relative path (or plain filename) in various directories, - // resolving to the corresponding absolute path if found. FindConfigFile takes into account - // various common search paths rooted both at the current working directory and relative - // to the executable. - if configFile := fileutils.FindConfigFile(path); configFile != "" { + // Search for the relative path to the file in the config folder, taking into account + // various common starting points. + if configFile := fileutils.FindFile(filepath.Join("config", path)); configFile != "" { + return configFile, nil + } + + // Search for the relative path in the current working directory, also taking into account + // various common starting points. + if configFile := fileutils.FindPath(path, []string{"."}, nil); configFile != "" { return configFile, nil } @@ -93,7 +99,7 @@ func (fs *FileStore) Set(newCfg *model.Config) (*model.Config, error) { return ErrReadOnlyConfiguration } - return nil + return fs.commonStore.validate(cfg) }, fs.persist) } @@ -128,11 +134,11 @@ func (fs *FileStore) Load() (err error) { f, err = os.Open(fs.path) if os.IsNotExist(err) { needsSave = true - defaultCfg := model.Config{} + defaultCfg := &model.Config{} defaultCfg.SetDefaults() var defaultCfgBytes []byte - defaultCfgBytes, err = marshalConfig(&defaultCfg) + defaultCfgBytes, err = marshalConfig(defaultCfg) if err != nil { return errors.Wrap(err, "failed to serialize default config") } @@ -149,7 +155,60 @@ func (fs *FileStore) Load() (err error) { } }() - return fs.commonStore.load(f, needsSave, fs.persist) + return fs.commonStore.load(f, needsSave, fs.commonStore.validate, fs.persist) +} + +// GetFile fetches the contents of a previously persisted configuration file. +func (fs *FileStore) GetFile(name string) ([]byte, error) { + resolvedPath := filepath.Join(filepath.Dir(fs.path), name) + + data, err := ioutil.ReadFile(resolvedPath) + if err != nil { + return nil, errors.Wrapf(err, "failed to read file from %s", resolvedPath) + } + + return data, nil +} + +// SetFile sets or replaces the contents of a configuration file. +func (fs *FileStore) SetFile(name string, data []byte) error { + resolvedPath := filepath.Join(filepath.Dir(fs.path), name) + + err := ioutil.WriteFile(resolvedPath, data, 0777) + if err != nil { + return errors.Wrapf(err, "failed to write file to %s", resolvedPath) + } + + return nil +} + +// HasFile returns true if the given file was previously persisted. +func (fs *FileStore) HasFile(name string) (bool, error) { + resolvedPath := filepath.Join(filepath.Dir(fs.path), name) + + _, err := os.Stat(resolvedPath) + if err != nil && os.IsNotExist(err) { + return false, nil + } else if err != nil { + return false, errors.Wrap(err, "failed to check if file exists") + } + + return true, nil +} + +// RemoveFile removes a previously persisted configuration file. +func (fs *FileStore) RemoveFile(name string) error { + resolvedPath := filepath.Join(filepath.Dir(fs.path), name) + + err := os.Remove(resolvedPath) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return errors.Wrap(err, "failed to remove file") + } + + return err } // startWatcher starts a watcher to monitor for external config file changes. diff --git a/config/file_test.go b/config/file_test.go index 0c75b29218..5395b0b8c1 100644 --- a/config/file_test.go +++ b/config/file_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -19,71 +20,6 @@ import ( "github.com/mattermost/mattermost-server/utils" ) -var emptyConfig, readOnlyConfig, minimalConfig, invalidConfig, fixesRequiredConfig, ldapConfig, testConfig *model.Config - -func init() { - emptyConfig = &model.Config{} - readOnlyConfig = &model.Config{ - ClusterSettings: model.ClusterSettings{ - Enable: bToP(true), - ReadOnlyConfig: bToP(true), - }, - } - minimalConfig = &model.Config{ - ServiceSettings: model.ServiceSettings{ - SiteURL: sToP("http://minimal"), - }, - SqlSettings: model.SqlSettings{ - AtRestEncryptKey: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), - }, - FileSettings: model.FileSettings{ - PublicLinkSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), - }, - EmailSettings: model.EmailSettings{ - InviteSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), - }, - LocalizationSettings: model.LocalizationSettings{ - DefaultServerLocale: sToP("en"), - DefaultClientLocale: sToP("en"), - }, - } - invalidConfig = &model.Config{ - ServiceSettings: model.ServiceSettings{ - SiteURL: sToP("invalid"), - }, - } - fixesRequiredConfig = &model.Config{ - ServiceSettings: model.ServiceSettings{ - SiteURL: sToP("http://trailingslash/"), - }, - SqlSettings: model.SqlSettings{ - AtRestEncryptKey: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), - }, - FileSettings: model.FileSettings{ - DriverName: sToP(model.IMAGE_DRIVER_LOCAL), - Directory: sToP("/path/to/directory"), - PublicLinkSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), - }, - EmailSettings: model.EmailSettings{ - InviteSalt: sToP("abcdefghijklmnopqrstuvwxyz0123456789"), - }, - LocalizationSettings: model.LocalizationSettings{ - DefaultServerLocale: sToP("garbage"), - DefaultClientLocale: sToP("garbage"), - }, - } - ldapConfig = &model.Config{ - LdapSettings: model.LdapSettings{ - BindPassword: sToP("password"), - }, - } - testConfig = &model.Config{ - ServiceSettings: model.ServiceSettings{ - SiteURL: sToP("http://TestStoreNew"), - }, - } -} - func setupConfigFile(t *testing.T, cfg *model.Config) (string, func()) { os.Clearenv() t.Helper() @@ -91,15 +27,23 @@ func setupConfigFile(t *testing.T, cfg *model.Config) (string, func()) { tempDir, err := ioutil.TempDir("", "setupConfigFile") require.NoError(t, err) - f, err := ioutil.TempFile(tempDir, "setupConfigFile") + err = os.Chdir(tempDir) require.NoError(t, err) - cfgData, err := config.MarshalConfig(cfg) - require.NoError(t, err) + var name string + if cfg != nil { + f, err := ioutil.TempFile(tempDir, "setupConfigFile") + require.NoError(t, err) - ioutil.WriteFile(f.Name(), cfgData, 0644) + cfgData, err := config.MarshalConfig(cfg) + require.NoError(t, err) - return f.Name(), func() { + ioutil.WriteFile(f.Name(), cfgData, 0644) + + name = f.Name() + } + + return name, func() { os.RemoveAll(tempDir) } } @@ -120,6 +64,8 @@ func getActualFileConfig(t *testing.T, path string) *model.Config { // assertFileEqualsConfig verifies the on disk contents of the given path equal the given config. func assertFileEqualsConfig(t *testing.T, expectedCfg *model.Config, path string) { + t.Helper() + expectedCfg = prepareExpectedConfig(t, expectedCfg) actualCfg := getActualFileConfig(t, path) @@ -128,6 +74,8 @@ func assertFileEqualsConfig(t *testing.T, expectedCfg *model.Config, path string // assertFileNotEqualsConfig verifies the on disk contents of the given path does not equal the given config. func assertFileNotEqualsConfig(t *testing.T, expectedCfg *model.Config, path string) { + t.Helper() + expectedCfg = prepareExpectedConfig(t, expectedCfg) actualCfg := getActualFileConfig(t, path) @@ -162,6 +110,9 @@ func TestFileStoreNew(t *testing.T) { }) t.Run("absolute path, file does not exist", func(t *testing.T) { + _, tearDown := setupConfigFile(t, nil) + defer tearDown() + tempDir, err := ioutil.TempDir("", "TestFileStoreNew") require.NoError(t, err) defer os.RemoveAll(tempDir) @@ -176,6 +127,9 @@ func TestFileStoreNew(t *testing.T) { }) t.Run("absolute path, path to file does not exist", func(t *testing.T) { + _, tearDown := setupConfigFile(t, nil) + defer tearDown() + tempDir, err := ioutil.TempDir("", "TestFileStoreNew") require.NoError(t, err) defer os.RemoveAll(tempDir) @@ -186,7 +140,8 @@ func TestFileStoreNew(t *testing.T) { }) t.Run("relative path, file exists", func(t *testing.T) { - os.Clearenv() + _, tearDown := setupConfigFile(t, nil) + defer tearDown() err := os.MkdirAll("TestFileStoreNew/a/b/c", 0700) require.NoError(t, err) @@ -208,11 +163,12 @@ func TestFileStoreNew(t *testing.T) { }) t.Run("relative path, file does not exist", func(t *testing.T) { - os.Clearenv() + _, tearDown := setupConfigFile(t, nil) + defer tearDown() - err := os.MkdirAll("TestFileStoreNew/a/b/c", 0700) + err := os.MkdirAll("config/TestFileStoreNew/a/b/c", 0700) require.NoError(t, err) - defer os.RemoveAll("TestFileStoreNew") + defer os.RemoveAll("config/TestFileStoreNew") path := "TestFileStoreNew/a/b/c/config.json" fs, err := config.NewFileStore(path, false) @@ -220,7 +176,7 @@ func TestFileStoreNew(t *testing.T) { defer fs.Close() assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) - assertFileNotEqualsConfig(t, testConfig, path) + assertFileNotEqualsConfig(t, testConfig, filepath.Join("config", path)) }) } @@ -355,7 +311,7 @@ func TestFileStoreSet(t *testing.T) { _, err = fs.Set(newCfg) if assert.Error(t, err) { - assert.Equal(t, err, config.ErrReadOnlyConfiguration) + assert.Equal(t, config.ErrReadOnlyConfiguration, errors.Cause(err)) } assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *fs.Get().ServiceSettings.SiteURL) @@ -626,6 +582,207 @@ func TestFileStoreSave(t *testing.T) { }) } +func TestFileGetFile(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + t.Run("get empty filename", func(t *testing.T) { + _, err := fs.GetFile("") + require.Error(t, err) + }) + + t.Run("get non-existent file", func(t *testing.T) { + _, err := fs.GetFile("unknown") + require.Error(t, err) + }) + + t.Run("get empty file", func(t *testing.T) { + err := os.MkdirAll("config", 0700) + require.NoError(t, err) + + f, err := ioutil.TempFile("config", "empty-file") + require.NoError(t, err) + defer os.Remove(f.Name()) + + err = ioutil.WriteFile(f.Name(), nil, 0777) + require.NoError(t, err) + + data, err := fs.GetFile(f.Name()) + require.NoError(t, err) + require.Empty(t, data) + }) + + t.Run("get non-empty file", func(t *testing.T) { + err := os.MkdirAll("config", 0700) + require.NoError(t, err) + + f, err := ioutil.TempFile("config", "test-file") + require.NoError(t, err) + defer os.Remove(f.Name()) + + err = ioutil.WriteFile(f.Name(), []byte("test"), 0777) + require.NoError(t, err) + + data, err := fs.GetFile(f.Name()) + require.NoError(t, err) + require.Equal(t, []byte("test"), data) + }) +} + +func TestFileSetFile(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + t.Run("set new file", func(t *testing.T) { + err := fs.SetFile("new", []byte("new file")) + require.NoError(t, err) + + data, err := fs.GetFile("new") + require.NoError(t, err) + require.Equal(t, []byte("new file"), data) + }) + + t.Run("overwrite existing file", func(t *testing.T) { + err := fs.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + err = fs.SetFile("existing", []byte("overwritten file")) + require.NoError(t, err) + + data, err := fs.GetFile("existing") + require.NoError(t, err) + require.Equal(t, []byte("overwritten file"), data) + }) +} + +func TestFileHasFile(t *testing.T) { + + t.Run("has non-existent", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + has, err := fs.HasFile("non-existent") + require.NoError(t, err) + require.False(t, has) + }) + + t.Run("has existing", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + err = fs.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + has, err := fs.HasFile("existing") + require.NoError(t, err) + require.True(t, has) + }) + + t.Run("has manually created file", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + err = os.MkdirAll("config", 0700) + require.NoError(t, err) + + f, err := ioutil.TempFile("config", "test-file") + require.NoError(t, err) + defer os.Remove(f.Name()) + + err = ioutil.WriteFile(f.Name(), []byte("test"), 0777) + require.NoError(t, err) + + has, err := fs.HasFile(f.Name()) + require.NoError(t, err) + require.True(t, has) + }) +} + +func TestFileRemoveFile(t *testing.T) { + t.Run("remove non-existent", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + err = fs.RemoveFile("non-existent") + require.NoError(t, err) + }) + + t.Run("remove existing", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + err = fs.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + err = fs.RemoveFile("existing") + require.NoError(t, err) + + has, err := fs.HasFile("existing") + require.NoError(t, err) + require.False(t, has) + + _, err = fs.GetFile("existing") + require.Error(t, err) + }) + + t.Run("remove manually created file", func(t *testing.T) { + path, tearDown := setupConfigFile(t, minimalConfig) + defer tearDown() + + fs, err := config.NewFileStore(path, true) + require.NoError(t, err) + defer fs.Close() + + err = os.MkdirAll("config", 0700) + require.NoError(t, err) + + f, err := ioutil.TempFile("config", "test-file") + require.NoError(t, err) + defer os.Remove(f.Name()) + + err = ioutil.WriteFile(f.Name(), []byte("test"), 0777) + require.NoError(t, err) + + err = fs.RemoveFile(f.Name()) + require.NoError(t, err) + + has, err := fs.HasFile("existing") + require.NoError(t, err) + require.False(t, has) + + _, err = fs.GetFile("existing") + require.Error(t, err) + }) +} + func TestFileStoreString(t *testing.T) { path, tearDown := setupConfigFile(t, emptyConfig) defer tearDown() diff --git a/config/main_test.go b/config/main_test.go index 1c1510ed07..3d7192ff7f 100644 --- a/config/main_test.go +++ b/config/main_test.go @@ -1,6 +1,7 @@ package config_test import ( + "fmt" "testing" "github.com/go-sql-driver/mysql" @@ -22,15 +23,15 @@ func TestMain(m *testing.M) { mainHelper.Main(m) } -// truncateTables clears tables used by the config package for reuse in other tests -func truncateTables(t *testing.T) { +// truncateTable clears the given table +func truncateTable(t *testing.T, table string) { t.Helper() sqlSetting := mainHelper.GetSqlSettings() sqlSupplier := mainHelper.GetSqlSupplier() switch *sqlSetting.DriverName { case model.DATABASE_DRIVER_MYSQL: - _, err := sqlSupplier.GetMaster().Db.Exec("TRUNCATE TABLE Configurations") + _, err := sqlSupplier.GetMaster().Db.Exec(fmt.Sprintf("TRUNCATE TABLE %s", table)) if err != nil { if driverErr, ok := err.(*mysql.MySQLError); ok { // Ignore if the Configurations table does not exist. @@ -42,10 +43,18 @@ func truncateTables(t *testing.T) { require.NoError(t, err) case model.DATABASE_DRIVER_POSTGRES: - _, err := sqlSupplier.GetMaster().Db.Exec("TRUNCATE TABLE Configurations") + _, err := sqlSupplier.GetMaster().Db.Exec(fmt.Sprintf("TRUNCATE TABLE %s", table)) require.NoError(t, err) default: t.Fatalf("unsupported driver name: %s", *sqlSetting.DriverName) } } + +// truncateTables clears tables used by the config package for reuse in other tests +func truncateTables(t *testing.T) { + t.Helper() + + truncateTable(t, "Configurations") + truncateTable(t, "ConfigurationFiles") +} diff --git a/config/memory.go b/config/memory.go index edf8ed2d4f..4fd68b3f58 100644 --- a/config/memory.go +++ b/config/memory.go @@ -5,7 +5,7 @@ package config import ( "bytes" - "encoding/json" + "fmt" "io/ioutil" "github.com/pkg/errors" @@ -15,24 +15,50 @@ import ( // memoryStore implements the Store interface. It is meant primarily for testing. type memoryStore struct { - emitter - - Config *model.Config - EnvironmentOverrides map[string]interface{} + commonStore allowEnvironmentOverrides bool + validate bool + files map[string][]byte + savedConfig *model.Config } -// NewMemoryStore creates a new memoryStore instance. -func NewMemoryStore(allowEnvironmentOverrides bool) (*memoryStore, error) { - defaultCfg := &model.Config{} - defaultCfg.SetDefaults() +// MemoryStoreOptions makes configuration of the memory store explicit. +type MemoryStoreOptions struct { + IgnoreEnvironmentOverrides bool + SkipValidation bool + InitialConfig *model.Config + InitialFiles map[string][]byte +} + +// NewMemoryStore creates a new memoryStore instance with default options. +func NewMemoryStore() (*memoryStore, error) { + return NewMemoryStoreWithOptions(&MemoryStoreOptions{}) +} + +// NewMemoryStoreWithOptions creates a new memoryStore instance. +func NewMemoryStoreWithOptions(options *MemoryStoreOptions) (*memoryStore, error) { + savedConfig := options.InitialConfig + if savedConfig == nil { + savedConfig = &model.Config{} + savedConfig.SetDefaults() + } + + initialFiles := options.InitialFiles + if initialFiles == nil { + initialFiles = make(map[string][]byte) + } ms := &memoryStore{ - Config: defaultCfg, - allowEnvironmentOverrides: allowEnvironmentOverrides, + allowEnvironmentOverrides: !options.IgnoreEnvironmentOverrides, + validate: !options.SkipValidation, + files: initialFiles, + savedConfig: savedConfig, } + ms.commonStore.config = &model.Config{} + ms.commonStore.config.SetDefaults() + if err := ms.Load(); err != nil { return nil, err } @@ -40,59 +66,89 @@ func NewMemoryStore(allowEnvironmentOverrides bool) (*memoryStore, error) { return ms, nil } -// Get fetches the current, cached configuration. -func (ms *memoryStore) Get() *model.Config { - return ms.Config -} - -// GetEnvironmentOverrides fetches the configuration fields overridden by environment variables. -func (ms *memoryStore) GetEnvironmentOverrides() map[string]interface{} { - return ms.EnvironmentOverrides -} - // Set replaces the current configuration in its entirety. func (ms *memoryStore) Set(newCfg *model.Config) (*model.Config, error) { - oldCfg := ms.Config + validate := ms.commonStore.validate + if !ms.validate { + validate = nil + } - newCfg.SetDefaults() - ms.Config = newCfg - - return oldCfg, nil + return ms.commonStore.set(newCfg, validate, ms.persist) } -// serialize converts the given configuration into JSON bytes for persistence. -func (ms *memoryStore) serialize(cfg *model.Config) ([]byte, error) { - return json.MarshalIndent(cfg, "", " ") +// persist copies the active config to the saved config. +func (ms *memoryStore) persist(cfg *model.Config) error { + ms.savedConfig = cfg.Clone() + + return nil } -// Load applies environment overrides to the current config as if a re-load had occurred. +// Load applies environment overrides to the default config as if a re-load had occurred. func (ms *memoryStore) Load() (err error) { var cfgBytes []byte - cfgBytes, err = ms.serialize(ms.Config) + cfgBytes, err = marshalConfig(ms.savedConfig) if err != nil { return errors.Wrap(err, "failed to serialize config") } f := ioutil.NopCloser(bytes.NewReader(cfgBytes)) - allowEnvironmentOverrides := true - loadedCfg, environmentOverrides, err := unmarshalConfig(f, allowEnvironmentOverrides) - if err != nil { - return errors.Wrap(err, "failed to load config") + validate := ms.commonStore.validate + if !ms.validate { + validate = nil } - ms.Config = loadedCfg - ms.EnvironmentOverrides = environmentOverrides + return ms.commonStore.load(f, false, validate, ms.persist) +} + +// GetFile fetches the contents of a previously persisted configuration file. +func (ms *memoryStore) GetFile(name string) ([]byte, error) { + ms.configLock.RLock() + defer ms.configLock.RUnlock() + + data, ok := ms.files[name] + if !ok { + return nil, fmt.Errorf("file %s not stored", name) + } + + return data, nil +} + +// SetFile sets or replaces the contents of a configuration file. +func (ms *memoryStore) SetFile(name string, data []byte) error { + ms.configLock.Lock() + defer ms.configLock.Unlock() + + ms.files[name] = data + + return nil +} + +// HasFile returns true if the given file was previously persisted. +func (ms *memoryStore) HasFile(name string) (bool, error) { + ms.configLock.RLock() + defer ms.configLock.RUnlock() + + _, ok := ms.files[name] + return ok, nil +} + +// RemoveFile removes a previously persisted configuration file. +func (ms *memoryStore) RemoveFile(name string) error { + ms.configLock.Lock() + defer ms.configLock.Unlock() + + delete(ms.files, name) return nil } // String returns a hard-coded description, as there is no backing store. func (ms *memoryStore) String() string { - return "mock://" + return "memory://" } -// Close does nothing for a mock store. +// Close does nothing for a memory store. func (ms *memoryStore) Close() error { return nil } diff --git a/config/memory_test.go b/config/memory_test.go new file mode 100644 index 0000000000..9c76bc3df1 --- /dev/null +++ b/config/memory_test.go @@ -0,0 +1,463 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package config_test + +import ( + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/config" + "github.com/mattermost/mattermost-server/model" +) + +func setupConfigMemory(t *testing.T) { + t.Helper() + os.Clearenv() +} + +func TestMemoryStoreNew(t *testing.T) { + t.Run("no existing configuration - initialization required", func(t *testing.T) { + ms, err := config.NewMemoryStore() + require.NoError(t, err) + defer ms.Close() + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("existing config, initialization required", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: testConfig}) + require.NoError(t, err) + defer ms.Close() + + assert.Equal(t, "http://TestStoreNew", *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("already minimally configured", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: minimalConfig}) + require.NoError(t, err) + defer ms.Close() + + assert.Equal(t, "http://minimal", *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("invalid config, validation enabled", func(t *testing.T) { + _, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: invalidConfig}) + require.Error(t, err) + }) + + t.Run("invalid config, validation disabled", func(t *testing.T) { + _, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: invalidConfig, SkipValidation: true}) + require.NoError(t, err) + }) +} + +func TestMemoryStoreGet(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: testConfig}) + require.NoError(t, err) + defer ms.Close() + + cfg := ms.Get() + assert.Equal(t, "http://TestStoreNew", *cfg.ServiceSettings.SiteURL) + + cfg2 := ms.Get() + assert.Equal(t, "http://TestStoreNew", *cfg.ServiceSettings.SiteURL) + + assert.True(t, cfg == cfg2, "Get() returned different configuration instances") + + newCfg := &model.Config{} + oldCfg, err := ms.Set(newCfg) + require.NoError(t, err) + + assert.True(t, oldCfg == cfg, "returned config after set() changed original") + assert.False(t, newCfg == cfg, "returned config should have been different from original") +} + +func TestMemoryStoreGetEnivironmentOverrides(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: testConfig}) + require.NoError(t, err) + defer ms.Close() + + assert.Equal(t, "http://TestStoreNew", *ms.Get().ServiceSettings.SiteURL) + assert.Empty(t, ms.GetEnvironmentOverrides()) + + os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") + + ms, err = config.NewMemoryStore() + require.NoError(t, err) + defer ms.Close() + + assert.Equal(t, "http://override", *ms.Get().ServiceSettings.SiteURL) + assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ms.GetEnvironmentOverrides()) +} + +func TestMemoryStoreSet(t *testing.T) { + t.Run("set same pointer value", func(t *testing.T) { + t.Skip("not yet implemented") + + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: emptyConfig}) + require.NoError(t, err) + defer ms.Close() + + _, err = ms.Set(ms.Get()) + if assert.Error(t, err) { + assert.EqualError(t, err, "old configuration modified instead of cloning") + } + }) + + t.Run("defaults required", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: minimalConfig}) + require.NoError(t, err) + defer ms.Close() + + oldCfg := ms.Get() + + newCfg := &model.Config{} + + retCfg, err := ms.Set(newCfg) + require.NoError(t, err) + assert.Equal(t, oldCfg, retCfg) + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("desanitization required", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: ldapConfig}) + require.NoError(t, err) + defer ms.Close() + + oldCfg := ms.Get() + + newCfg := &model.Config{} + newCfg.LdapSettings.BindPassword = sToP(model.FAKE_SETTING) + + retCfg, err := ms.Set(newCfg) + require.NoError(t, err) + assert.Equal(t, oldCfg, retCfg) + + assert.Equal(t, "password", *ms.Get().LdapSettings.BindPassword) + }) + + t.Run("invalid", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: emptyConfig}) + require.NoError(t, err) + defer ms.Close() + + newCfg := &model.Config{} + newCfg.ServiceSettings.SiteURL = sToP("invalid") + + _, err = ms.Set(newCfg) + if assert.Error(t, err) { + assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") + } + + assert.Equal(t, model.SERVICE_SETTINGS_DEFAULT_SITE_URL, *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("read-only ignored", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: readOnlyConfig}) + require.NoError(t, err) + defer ms.Close() + + newCfg := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: sToP("http://new"), + }, + } + + _, err = ms.Set(newCfg) + require.NoError(t, err) + + assert.Equal(t, "http://new", *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("listeners notified", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: emptyConfig}) + require.NoError(t, err) + defer ms.Close() + + oldCfg := ms.Get() + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + ms.AddListener(callback) + + newCfg := &model.Config{} + + retCfg, err := ms.Set(newCfg) + require.NoError(t, err) + assert.Equal(t, oldCfg, retCfg) + + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when config written") + } + }) +} + +func TestMemoryStoreLoad(t *testing.T) { + t.Run("honour environment", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: minimalConfig}) + require.NoError(t, err) + defer ms.Close() + + os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override") + + err = ms.Load() + require.NoError(t, err) + assert.Equal(t, "http://override", *ms.Get().ServiceSettings.SiteURL) + assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ms.GetEnvironmentOverrides()) + }) + + t.Run("fixes required", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: fixesRequiredConfig}) + require.NoError(t, err) + defer ms.Close() + + err = ms.Load() + require.NoError(t, err) + assert.Equal(t, "http://trailingslash", *ms.Get().ServiceSettings.SiteURL) + }) + + t.Run("listeners notifed", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: emptyConfig}) + require.NoError(t, err) + defer ms.Close() + + called := make(chan bool, 1) + callback := func(oldfg, newCfg *model.Config) { + called <- true + } + ms.AddListener(callback) + + err = ms.Load() + require.NoError(t, err) + + select { + case <-called: + case <-time.After(5 * time.Second): + t.Fatal("callback should have been called when config loaded") + } + }) +} + +func TestMemoryGetFile(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + InitialFiles: map[string][]byte{ + "empty-file": []byte{}, + "test-file": []byte("test"), + }, + }) + require.NoError(t, err) + defer ms.Close() + + t.Run("get empty filename", func(t *testing.T) { + _, err := ms.GetFile("") + require.Error(t, err) + }) + + t.Run("get non-existent file", func(t *testing.T) { + _, err := ms.GetFile("unknown") + require.Error(t, err) + }) + + t.Run("get empty file", func(t *testing.T) { + data, err := ms.GetFile("empty-file") + require.NoError(t, err) + require.Empty(t, data) + }) + + t.Run("get non-empty file", func(t *testing.T) { + data, err := ms.GetFile("test-file") + require.NoError(t, err) + require.Equal(t, []byte("test"), data) + }) +} + +func TestMemorySetFile(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + }) + require.NoError(t, err) + defer ms.Close() + + t.Run("set new file", func(t *testing.T) { + err := ms.SetFile("new", []byte("new file")) + require.NoError(t, err) + + data, err := ms.GetFile("new") + require.NoError(t, err) + require.Equal(t, []byte("new file"), data) + }) + + t.Run("overwrite existing file", func(t *testing.T) { + err := ms.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + err = ms.SetFile("existing", []byte("overwritten file")) + require.NoError(t, err) + + data, err := ms.GetFile("existing") + require.NoError(t, err) + require.Equal(t, []byte("overwritten file"), data) + }) +} + +func TestMemoryHasFile(t *testing.T) { + t.Run("has non-existent", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + }) + require.NoError(t, err) + defer ms.Close() + + has, err := ms.HasFile("non-existent") + require.NoError(t, err) + require.False(t, has) + }) + + t.Run("has existing", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + }) + require.NoError(t, err) + defer ms.Close() + + err = ms.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + has, err := ms.HasFile("existing") + require.NoError(t, err) + require.True(t, has) + }) + + t.Run("has manually created file", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + InitialFiles: map[string][]byte{ + "manual": []byte("manual file"), + }, + }) + require.NoError(t, err) + defer ms.Close() + + has, err := ms.HasFile("manual") + require.NoError(t, err) + require.True(t, has) + }) +} + +func TestMemoryRemoveFile(t *testing.T) { + t.Run("remove non-existent", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + }) + require.NoError(t, err) + defer ms.Close() + + err = ms.RemoveFile("non-existent") + require.NoError(t, err) + }) + + t.Run("remove existing", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + }) + require.NoError(t, err) + defer ms.Close() + + err = ms.SetFile("existing", []byte("existing file")) + require.NoError(t, err) + + err = ms.RemoveFile("existing") + require.NoError(t, err) + + has, err := ms.HasFile("existing") + require.NoError(t, err) + require.False(t, has) + + _, err = ms.GetFile("existing") + require.Error(t, err) + }) + + t.Run("remove manually created file", func(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ + InitialConfig: minimalConfig, + InitialFiles: map[string][]byte{ + "manual": []byte("manual file"), + }, + }) + require.NoError(t, err) + defer ms.Close() + + err = ms.RemoveFile("manual") + require.NoError(t, err) + + has, err := ms.HasFile("manual") + require.NoError(t, err) + require.False(t, has) + + _, err = ms.GetFile("manual") + require.Error(t, err) + }) +} + +func TestMemoryStoreString(t *testing.T) { + setupConfigMemory(t) + + ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{InitialConfig: emptyConfig}) + require.NoError(t, err) + defer ms.Close() + + assert.Equal(t, "memory://", ms.String()) +} diff --git a/config/store.go b/config/store.go index 4dbc9e6978..e01e36265f 100644 --- a/config/store.go +++ b/config/store.go @@ -32,6 +32,19 @@ type Store interface { // RemoveListener removes a callback function using an id returned from AddListener. RemoveListener(id string) + // GetFile fetches the contents of a previously persisted configuration file. + // If no such file exists, an empty byte array will be returned without error. + GetFile(name string) ([]byte, error) + + // SetFile sets or replaces the contents of a configuration file. + SetFile(name string, data []byte) error + + // HasFile returns true if the given file was previously persisted. + HasFile(name string) (bool, error) + + // RemoveFile removes a previously persisted configuration file. + RemoveFile(name string) error + // String describes the backing store for the config. String() string diff --git a/config/store_test.go b/config/store_test.go index 8b46c6f481..97886885bf 100644 --- a/config/store_test.go +++ b/config/store_test.go @@ -2,6 +2,9 @@ package config_test import ( "fmt" + "io/ioutil" + "os" + "path/filepath" "testing" "github.com/mattermost/mattermost-server/config" @@ -11,6 +14,14 @@ import ( func TestNewStore(t *testing.T) { sqlSettings := mainHelper.GetSqlSettings() + tempDir, err := ioutil.TempDir("", "TestNewStore") + require.NoError(t, err) + + err = os.Chdir(tempDir) + require.NoError(t, err) + + require.NoError(t, os.Mkdir(filepath.Join(tempDir, "config"), 0700)) + t.Run("database dsn", func(t *testing.T) { ds, err := config.NewStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource), false) require.NoError(t, err) diff --git a/migrations/helper_test.go b/migrations/helper_test.go index 574b2a6edc..82d8cb176e 100644 --- a/migrations/helper_test.go +++ b/migrations/helper_test.go @@ -4,16 +4,14 @@ package migrations import ( - "io" - "io/ioutil" "os" "time" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" - "github.com/mattermost/mattermost-server/utils/fileutils" ) type TestHelper struct { @@ -27,31 +25,21 @@ type TestHelper struct { SystemAdminUser *model.User - tempConfigPath string - tempWorkspace string + tempWorkspace string } func setupTestHelper(enterprise bool) *TestHelper { store := mainHelper.GetStore() store.DropAllTables() - permConfig, err := os.Open(fileutils.FindConfigFile("config.json")) + memoryStore, err := config.NewMemoryStore() if err != nil { - panic(err) - } - defer permConfig.Close() - tempConfig, err := ioutil.TempFile("", "") - if err != nil { - panic(err) - } - _, err = io.Copy(tempConfig, permConfig) - tempConfig.Close() - if err != nil { - panic(err) + panic("failed to initialize memory store: " + err.Error()) } - options := []app.Option{app.Config(tempConfig.Name(), false)} - options = append(options, app.StoreOverride(store)) + var options []app.Option + options = append(options, app.ConfigStore(memoryStore)) + options = append(options, app.StoreOverride(mainHelper.Store)) s, err := app.NewServer(options...) if err != nil { @@ -59,9 +47,8 @@ func setupTestHelper(enterprise bool) *TestHelper { } th := &TestHelper{ - App: s.FakeApp(), - Server: s, - tempConfigPath: tempConfig.Name(), + App: s.FakeApp(), + Server: s, } th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.MaxUsersPerTeam = 50 }) @@ -259,7 +246,6 @@ func (me *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel) func (me *TestHelper) TearDown() { me.Server.Shutdown() - os.Remove(me.tempConfigPath) if err := recover(); err != nil { panic(err) } diff --git a/utils/fileutils/fileutils.go b/utils/fileutils/fileutils.go index e1eac469d4..81c37415e5 100644 --- a/utils/fileutils/fileutils.go +++ b/utils/fileutils/fileutils.go @@ -85,15 +85,3 @@ func FindDir(dir string) (string, bool) { return found, true } - -// FindConfigFile attempts to find an existing configuration file. fileName can be an absolute or -// relative path or name such as "/opt/mattermost/config.json" or simply "config.json". An empty -// string is returned if no configuration is found. -func FindConfigFile(fileName string) (path string) { - found := FindFile(filepath.Join("config", fileName)) - if found == "" { - found = FindPath(fileName, []string{"."}, nil) - } - - return found -} diff --git a/utils/fileutils/fileutils_test.go b/utils/fileutils/fileutils_test.go index e4d66fd919..a6b2c51a0a 100644 --- a/utils/fileutils/fileutils_test.go +++ b/utils/fileutils/fileutils_test.go @@ -14,175 +14,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestFindConfigFile(t *testing.T) { - t.Run("config.json in current working directory, not inside config/", func(t *testing.T) { - // Force a unique working directory - cwd, err := ioutil.TempDir("", "") - require.NoError(t, err) - defer os.RemoveAll(cwd) - - prevDir, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(prevDir) - os.Chdir(cwd) - - configJson, err := filepath.Abs("config.json") - require.NoError(t, err) - require.NoError(t, ioutil.WriteFile(configJson, []byte("{}"), 0600)) - - // Relative paths end up getting symlinks fully resolved. - configJsonResolved, err := filepath.EvalSymlinks(configJson) - require.NoError(t, err) - - assert.Equal(t, configJsonResolved, FindConfigFile("config.json")) - }) - - t.Run("config/config.json from various paths", func(t *testing.T) { - // Create the following directory structure: - // tmpDir1/ - // config/ - // config.json - // tmpDir2/ - // tmpDir3/ - // tmpDir4/ - // tmpDir5/ - tmpDir1, err := ioutil.TempDir("", "") - require.NoError(t, err) - defer os.RemoveAll(tmpDir1) - - err = os.Mkdir(filepath.Join(tmpDir1, "config"), 0700) - require.NoError(t, err) - - tmpDir2, err := ioutil.TempDir(tmpDir1, "") - require.NoError(t, err) - - tmpDir3, err := ioutil.TempDir(tmpDir2, "") - require.NoError(t, err) - - tmpDir4, err := ioutil.TempDir(tmpDir3, "") - require.NoError(t, err) - - tmpDir5, err := ioutil.TempDir(tmpDir4, "") - require.NoError(t, err) - - configJson := filepath.Join(tmpDir1, "config", "config.json") - require.NoError(t, ioutil.WriteFile(configJson, []byte("{}"), 0600)) - - // Relative paths end up getting symlinks fully resolved, so use this below as necessary. - configJsonResolved, err := filepath.EvalSymlinks(configJson) - require.NoError(t, err) - - testCases := []struct { - Description string - Cwd *string - FileName string - Expected string - }{ - { - "absolute path to config.json", - nil, - configJson, - configJson, - }, - { - "absolute path to config.json from directory containing config.json", - &tmpDir1, - configJson, - configJson, - }, - { - "relative path to config.json from directory containing config.json", - &tmpDir1, - "config.json", - configJsonResolved, - }, - { - "subdirectory of directory containing config.json", - &tmpDir2, - "config.json", - configJsonResolved, - }, - { - "twice-nested subdirectory of directory containing config.json", - &tmpDir3, - "config.json", - configJsonResolved, - }, - { - "thrice-nested subdirectory of directory containing config.json", - &tmpDir4, - "config.json", - configJsonResolved, - }, - { - "can't find from four nesting levels deep", - &tmpDir5, - "config.json", - "", - }, - } - - for _, testCase := range testCases { - t.Run(testCase.Description, func(t *testing.T) { - if testCase.Cwd != nil { - prevDir, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(prevDir) - os.Chdir(*testCase.Cwd) - } - - assert.Equal(t, testCase.Expected, FindConfigFile(testCase.FileName)) - }) - } - }) - - t.Run("config/config.json relative to executable", func(t *testing.T) { - osExecutable, err := os.Executable() - require.NoError(t, err) - osExecutableDir := filepath.Dir(osExecutable) - - // Force a working directory different than the executable. - cwd, err := ioutil.TempDir("", "") - require.NoError(t, err) - defer os.RemoveAll(cwd) - - prevDir, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(prevDir) - os.Chdir(cwd) - - testCases := []struct { - Description string - RelativePath string - }{ - { - "config/config.json", - ".", - }, - { - "../config/config.json", - "../", - }, - } - - for _, testCase := range testCases { - t.Run(testCase.Description, func(t *testing.T) { - // Install the config in config/config.json relative to the executable - configJson := filepath.Join(osExecutableDir, testCase.RelativePath, "config", "config.json") - require.NoError(t, os.Mkdir(filepath.Dir(configJson), 0700)) - require.NoError(t, ioutil.WriteFile(configJson, []byte("{}"), 0600)) - defer os.RemoveAll(filepath.Dir(configJson)) - - // Relative paths end up getting symlinks fully resolved. - configJsonResolved, err := filepath.EvalSymlinks(configJson) - require.NoError(t, err) - - assert.Equal(t, configJsonResolved, FindConfigFile("config.json")) - }) - } - }) -} - func TestFindFile(t *testing.T) { t.Run("files from various paths", func(t *testing.T) { // Create the following directory structure: diff --git a/utils/license_test.go b/utils/license_test.go index a67d561698..1c909ff847 100644 --- a/utils/license_test.go +++ b/utils/license_test.go @@ -4,9 +4,12 @@ package utils import ( + "io/ioutil" + "os" "testing" - "github.com/mattermost/mattermost-server/utils/fileutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidateLicense(t *testing.T) { @@ -34,17 +37,21 @@ func TestGetLicenseFileLocation(t *testing.T) { } func TestGetLicenseFileFromDisk(t *testing.T) { - fileBytes := GetLicenseFileFromDisk("thisfileshouldnotexist.mattermost-license") - if len(fileBytes) > 0 { - t.Fatal("invalid bytes") - } + t.Run("missing file", func(t *testing.T) { + fileBytes := GetLicenseFileFromDisk("thisfileshouldnotexist.mattermost-license") + assert.Empty(t, fileBytes, "invalid bytes") + }) - fileBytes = GetLicenseFileFromDisk(fileutils.FindConfigFile("config.json")) - if len(fileBytes) == 0 { // a valid bytes but should be a fail license - t.Fatal("invalid bytes") - } + t.Run("not a license file", func(t *testing.T) { + f, err := ioutil.TempFile("", "TestGetLicenseFileFromDisk") + require.NoError(t, err) + defer os.Remove(f.Name()) + ioutil.WriteFile(f.Name(), []byte("not a license"), 0777) - if success, _ := ValidateLicense(fileBytes); success { - t.Fatal("should have been an invalid file") - } + fileBytes := GetLicenseFileFromDisk(f.Name()) + require.NotEmpty(t, fileBytes, "should have read the file") + + success, _ := ValidateLicense(fileBytes) + assert.False(t, success, "should have been an invalid file") + }) } diff --git a/utils/utils.go b/utils/utils.go index 9e35d4a885..3300d25065 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -7,11 +7,9 @@ import ( "net" "net/http" "net/url" - "os" "strings" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils/fileutils" ) func StringInSlice(a string, slice []string) bool { @@ -40,17 +38,6 @@ func StringArrayIntersection(arr1, arr2 []string) []string { return result } -func FileExistsInConfigFolder(filename string) bool { - if len(filename) == 0 { - return false - } - - if _, err := os.Stat(fileutils.FindConfigFile(filename)); err == nil { - return true - } - return false -} - func RemoveDuplicatesFromStringArray(arr []string) []string { result := make([]string, 0, len(arr)) seen := make(map[string]bool) diff --git a/web/web_test.go b/web/web_test.go index c1a4c3538f..1bb6b9ec82 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -5,14 +5,11 @@ package web import ( "fmt" - "io" - "io/ioutil" - "os" "testing" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils/fileutils" ) var ApiClient *model.Client4 @@ -33,23 +30,14 @@ func Setup() *TestHelper { store := mainHelper.GetStore() store.DropAllTables() - permConfig, err := os.Open(fileutils.FindConfigFile("config.json")) + memoryStore, err := config.NewMemoryStore() if err != nil { - panic(err) - } - defer permConfig.Close() - tempConfig, err := ioutil.TempFile("", "") - if err != nil { - panic(err) - } - _, err = io.Copy(tempConfig, permConfig) - tempConfig.Close() - if err != nil { - panic(err) + panic("failed to initialize memory store: " + err.Error()) } - options := []app.Option{app.Config(tempConfig.Name(), false)} - options = append(options, app.StoreOverride(store)) + var options []app.Option + options = append(options, app.ConfigStore(memoryStore)) + options = append(options, app.StoreOverride(mainHelper.Store)) s, err := app.NewServer(options...) if err != nil {