MM-13893: refactor config (#10230)

* refactor utils/config* to config/

* pull validateLdapFilter into app

* clean up Config/GetConfig/GetSanitizedConfig usage

Eliminate app.GetConfig() in favour of just using app.Config() directly,
but expose app.GetSanitizedConfig() for when the old behaviour was
required.

* web: isolate config setup

* TestInvitePeopleProvider: make config explicit

* regenerateClientConfig: avoid racey map access

* integrate watch flag into app.ConfigFile option

* make app.Option return an error

* release.mk: only cp static files from config/

* release.mk: fix cp static files from config/

* api4: TestPlugin cleanup

* s/c/cfg/ for clarity

* fix merge conflict

* testlib: allow customization of testlib driver name

app/admin.go

@@ -6,13 +6,13 @@ package app
 import (
 	"io"
 	"os"
-	"strings"
 	"time"
 
 	"runtime/debug"
 
 	"net/http"
 
+	"github.com/mattermost/mattermost-server/einterfaces"
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/services/mailservice"
@@ -149,9 +149,8 @@ func (a *App) InvalidateAllCachesSkipSend() {
 	a.LoadLicense()
 }
 
-func (a *App) GetConfig() *model.Config {
-	json := a.Config().ToJson()
-	cfg := model.ConfigFromJson(strings.NewReader(json))
+func (a *App) GetSanitizedConfig() *model.Config {
+	cfg := a.Config().Clone()
 	cfg.Sanitize()
 
 	return cfg
@@ -161,6 +160,14 @@ func (a *App) GetEnvironmentConfig() map[string]interface{} {
 	return a.EnvironmentConfig()
 }
 
+func validateLdapFilter(cfg *model.Config, ldap einterfaces.LdapInterface) *model.AppError {
+	if !*cfg.LdapSettings.Enable || ldap == nil || *cfg.LdapSettings.UserFilter == "" {
+		return nil
+	}
+
+	return ldap.ValidateFilter(*cfg.LdapSettings.UserFilter)
+}
+
 func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError {
 	oldCfg := a.Config()
 	cfg.SetDefaults()
@@ -170,7 +177,7 @@ func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool)
 		return err
 	}
 
-	if err := utils.ValidateLdapFilter(cfg, a.Ldap); err != nil {
+	if err := validateLdapFilter(cfg, a.Ldap); err != nil {
 		return err
 	}
 

app/app_test.go

@@ -405,7 +405,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
 
 	th.App.DoAdvancedPermissionsMigration()
 
-	config := th.App.GetConfig()
+	config := th.App.Config()
 	assert.Equal(t, -1, *config.ServiceSettings.PostEditTimeLimit)
 
 	th.ResetRoleMigration()
@@ -416,7 +416,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
 	})
 
 	th.App.DoAdvancedPermissionsMigration()
-	config = th.App.GetConfig()
+	config = th.App.Config()
 	assert.Equal(t, 300, *config.ServiceSettings.PostEditTimeLimit)
 }
 

app/command_invite_people_test.go

@@ -15,11 +15,10 @@ func TestInvitePeopleProvider(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()
 
-	enableEmailInvitations := *th.App.Config().ServiceSettings.EnableEmailInvitations
-	defer func() {
-		th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableEmailInvitations = &enableEmailInvitations })
-	}()
-	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = true })
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.EmailSettings.SendEmailNotifications = true
+		*cfg.ServiceSettings.EnableEmailInvitations = true
+	})
 
 	cmd := InvitePeopleProvider{}
 

app/config.go

@@ -18,6 +18,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/mattermost/mattermost-server/config"
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
@@ -63,13 +64,13 @@ func (a *App) UpdateConfig(f func(*model.Config)) {
 }
 
 func (a *App) PersistConfig() {
-	utils.SaveConfig(a.ConfigFileName(), a.Config())
+	config.SaveConfig(a.ConfigFileName(), a.Config())
 }
 
 func (s *Server) LoadConfig(configFile string) *model.AppError {
 	old := s.Config()
 
-	cfg, configPath, envConfig, err := utils.LoadConfig(configFile)
+	cfg, configPath, envConfig, err := config.LoadConfig(configFile)
 	if err != nil {
 		return err
 	}
@@ -117,7 +118,7 @@ func (a *App) LimitedClientConfig() map[string]string {
 
 func (s *Server) EnableConfigWatch() {
 	if s.configWatcher == nil && !s.disableConfigWatch {
-		configWatcher, err := utils.NewConfigWatcher(s.configFile, func() {
+		configWatcher, err := config.NewConfigWatcher(s.configFile, func() {
 			s.ReloadConfig()
 		})
 		if err != nil {
@@ -280,26 +281,28 @@ func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey {
 }
 
 func (a *App) regenerateClientConfig() {
-	a.Srv.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId(), a.License())
-	a.Srv.limitedClientConfig = utils.GenerateLimitedClientConfig(a.Config(), a.DiagnosticId(), a.License())
+	clientConfig := config.GenerateClientConfig(a.Config(), a.DiagnosticId(), a.License())
+	limitedClientConfig := config.GenerateLimitedClientConfig(a.Config(), a.DiagnosticId(), a.License())
 
-	if a.Srv.clientConfig["EnableCustomTermsOfService"] == "true" {
+	if clientConfig["EnableCustomTermsOfService"] == "true" {
 		termsOfService, err := a.GetLatestTermsOfService()
 		if err != nil {
 			mlog.Err(err)
 		} else {
-			a.Srv.clientConfig["CustomTermsOfServiceId"] = termsOfService.Id
-			a.Srv.limitedClientConfig["CustomTermsOfServiceId"] = termsOfService.Id
+			clientConfig["CustomTermsOfServiceId"] = termsOfService.Id
+			limitedClientConfig["CustomTermsOfServiceId"] = termsOfService.Id
 		}
 	}
 
 	if key := a.AsymmetricSigningKey(); key != nil {
 		der, _ := x509.MarshalPKIXPublicKey(&key.PublicKey)
-		a.Srv.clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der)
-		a.Srv.limitedClientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der)
+		clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der)
+		limitedClientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der)
 	}
 
-	clientConfigJSON, _ := json.Marshal(a.Srv.clientConfig)
+	clientConfigJSON, _ := json.Marshal(clientConfig)
+	a.Srv.clientConfig = clientConfig
+	a.Srv.limitedClientConfig = limitedClientConfig
 	a.Srv.clientConfigHash = fmt.Sprintf("%x", md5.Sum(clientConfigJSON))
 }
 

app/config_test.go

@@ -42,7 +42,7 @@ func TestLoadConfig(t *testing.T) {
 	appErr := a.LoadConfig(tempConfig.Name())
 	require.Nil(t, appErr)
 
-	assert.Equal(t, "http://localhost:8065", *a.GetConfig().ServiceSettings.SiteURL)
+	assert.Equal(t, "http://localhost:8065", *a.Config().ServiceSettings.SiteURL)
 }
 
 func TestConfigListener(t *testing.T) {

app/enterprise.go

@@ -120,7 +120,7 @@ func (s *Server) initEnterprise() {
 	if ldapInterface != nil {
 		s.Ldap = ldapInterface(s.FakeApp())
 		s.AddConfigListener(func(_, cfg *model.Config) {
-			if err := utils.ValidateLdapFilter(cfg, s.Ldap); err != nil {
+			if err := validateLdapFilter(cfg, s.Ldap); err != nil {
 				panic(utils.T(err.Id))
 			}
 		})

app/helper_test.go

@@ -51,7 +51,7 @@ func setupTestHelper(enterprise bool) *TestHelper {
 		panic(err)
 	}
 
-	options := []Option{ConfigFile(tempConfig.Name()), DisableConfigWatch}
+	options := []Option{ConfigFile(tempConfig.Name(), false)}
 	options = append(options, StoreOverride(mainHelper.Store))
 
 	s, err := NewServer(options...)

app/options.go

@@ -4,56 +4,69 @@
 package app
 
 import (
+	"github.com/pkg/errors"
+
 	"github.com/mattermost/mattermost-server/store"
 )
 
-type Option func(s *Server)
+type Option func(s *Server) error
 
 // By default, the app will use the store specified by the configuration. This allows you to
 // construct an app with a different store.
 //
 // The override parameter must be either a store.Store or func(App) store.Store.
 func StoreOverride(override interface{}) Option {
-	return func(s *Server) {
+	return func(s *Server) error {
 		switch o := override.(type) {
 		case store.Store:
 			s.newStore = func() store.Store {
 				return o
 			}
+			return nil
+
 		case func(*Server) store.Store:
 			s.newStore = func() store.Store {
 				return o(s)
 			}
+			return nil
+
 		default:
-			panic("invalid StoreOverride")
+			return errors.New("invalid StoreOverride")
 		}
 	}
 }
 
-func ConfigFile(file string) Option {
-	return func(s *Server) {
+func ConfigFile(file string, watch bool) Option {
+	return func(s *Server) error {
 		s.configFile = file
+		s.disableConfigWatch = !watch
+
+		return nil
 	}
 }
 
-func RunJobs(s *Server) {
+func RunJobs(s *Server) error {
 	s.runjobs = true
+
+	return nil
 }
 
-func JoinCluster(s *Server) {
+func JoinCluster(s *Server) error {
 	s.joinCluster = true
+
+	return nil
 }
 
-func StartMetrics(s *Server) {
+func StartMetrics(s *Server) error {
 	s.startMetrics = true
+
+	return nil
 }
 
-func StartElasticsearch(s *Server) {
+func StartElasticsearch(s *Server) error {
 	s.startElasticsearch = true
-}
 
-func DisableConfigWatch(s *Server) {
-	s.disableConfigWatch = true
+	return nil
 }
 
 type AppOption func(a *App)

app/plugin_api.go

@@ -77,7 +77,7 @@ func (api *PluginAPI) GetSession(sessionId string) (*model.Session, *model.AppEr
 }
 
 func (api *PluginAPI) GetConfig() *model.Config {
-	return api.app.GetConfig()
+	return api.app.GetSanitizedConfig()
 }
 
 func (api *PluginAPI) SaveConfig(config *model.Config) *model.AppError {
@@ -85,7 +85,7 @@ func (api *PluginAPI) SaveConfig(config *model.Config) *model.AppError {
 }
 
 func (api *PluginAPI) GetPluginConfig() map[string]interface{} {
-	cfg := api.app.GetConfig()
+	cfg := api.app.GetSanitizedConfig()
 	if pluginConfig, isOk := cfg.PluginSettings.Plugins[api.manifest.Id]; isOk {
 		return pluginConfig
 	}
@@ -93,7 +93,7 @@ func (api *PluginAPI) GetPluginConfig() map[string]interface{} {
 }
 
 func (api *PluginAPI) SavePluginConfig(pluginConfig map[string]interface{}) *model.AppError {
-	cfg := api.app.GetConfig()
+	cfg := api.app.GetSanitizedConfig()
 	cfg.PluginSettings.Plugins[api.manifest.Id] = pluginConfig
 	return api.app.SaveConfig(cfg, true)
 }

app/server.go

@@ -24,6 +24,7 @@ import (
 	"github.com/throttled/throttled"
 	"golang.org/x/crypto/acme/autocert"
 
+	"github.com/mattermost/mattermost-server/config"
 	"github.com/mattermost/mattermost-server/einterfaces"
 	"github.com/mattermost/mattermost-server/jobs"
 	"github.com/mattermost/mattermost-server/mlog"
@@ -96,7 +97,7 @@ type Server struct {
 	logListenerId           string
 	clusterLeaderListenerId string
 	disableConfigWatch      bool
-	configWatcher           *utils.ConfigWatcher
+	configWatcher           *config.ConfigWatcher
 	asymmetricSigningKey    *ecdsa.PrivateKey
 
 	pluginCommands     []*PluginCommand
@@ -144,7 +145,9 @@ func NewServer(options ...Option) (*Server, error) {
 		clientConfig:            make(map[string]string),
 	}
 	for _, option := range options {
-		option(s)
+		if err := option(s); err != nil {
+			return nil, errors.Wrap(err, "failed to apply option")
+		}
 	}
 
 	if err := s.LoadConfig(s.configFile); err != nil {