Move enterprise features under Channels (#19010)

* Move enterprise features under Channels We move the EE features which are Channels related. While here, we also move some code under *Server.Start() from NewServer. ```release-note NONE ``` * move saml and ldap back to server ```release-note NONE ``` * fix test ```release-note NONE ``` * try again ```release-note NONE ```
2021-11-25 10:07:43 +05:30
--- a/app/app.go
+++ b/app/app.go
@@ -95,16 +95,16 @@ func (a *App) NotificationsLog() *mlog.Logger {
 }

 func (a *App) AccountMigration() einterfaces.AccountMigrationInterface {
-	return a.ch.srv.AccountMigration
+	return a.ch.AccountMigration
 }
 func (a *App) Cluster() einterfaces.ClusterInterface {
 	return a.ch.srv.Cluster
 }
 func (a *App) Compliance() einterfaces.ComplianceInterface {
-	return a.ch.srv.Compliance
+	return a.ch.Compliance
 }
 func (a *App) DataRetention() einterfaces.DataRetentionInterface {
-	return a.ch.srv.DataRetention
+	return a.ch.DataRetention
 }
 func (a *App) SearchEngine() *searchengine.Broker {
 	return a.ch.srv.SearchEngine
@@ -113,7 +113,7 @@ func (a *App) Ldap() einterfaces.LdapInterface {
 	return a.ch.srv.Ldap
 }
 func (a *App) MessageExport() einterfaces.MessageExportInterface {
-	return a.ch.srv.MessageExport
+	return a.ch.MessageExport
 }
 func (a *App) Metrics() einterfaces.MetricsInterface {
 	return a.ch.srv.Metrics
--- a/app/channels.go
+++ b/app/channels.go
@@ -8,6 +8,7 @@ import (
 	"sync/atomic"

 	"github.com/mattermost/mattermost-server/v6/app/request"
+	"github.com/mattermost/mattermost-server/v6/einterfaces"
 	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/mattermost/mattermost-server/v6/plugin"
 	"github.com/mattermost/mattermost-server/v6/services/imageproxy"
@@ -18,6 +19,8 @@ import (
 type Channels struct {
 	srv *Server

+	postActionCookieSecret []byte
+
 	pluginCommandsLock     sync.RWMutex
 	pluginCommands         []*PluginCommand
 	pluginsLock            sync.RWMutex
@@ -37,6 +40,11 @@ type Channels struct {
 	cachedDBMSVersion string
 	// previously fetched notices
 	cachedNotices model.ProductNotices
+
+	AccountMigration einterfaces.AccountMigrationInterface
+	Compliance       einterfaces.ComplianceInterface
+	DataRetention    einterfaces.DataRetentionInterface
+	MessageExport    einterfaces.MessageExportInterface
 }

 func init() {
@@ -50,6 +58,23 @@ func NewChannels(s *Server) (*Channels, error) {
 		srv:        s,
 		imageProxy: imageproxy.MakeImageProxy(s, s.httpService, s.Log),
 	}
+	// We are passing a partially filled Channels struct so that the enterprise
+	// methods can have access to app methods.
+	// Otherwise, passing server would mean it has to call s.Channels(),
+	// which would be nil at this point.
+	if complianceInterface != nil {
+		ch.Compliance = complianceInterface(New(ServerConnector(ch)))
+	}
+	if messageExportInterface != nil {
+		ch.MessageExport = messageExportInterface(New(ServerConnector(ch)))
+	}
+	if dataRetentionInterface != nil {
+		ch.DataRetention = dataRetentionInterface(New(ServerConnector(ch)))
+	}
+	if accountMigrationInterface != nil {
+		ch.AccountMigration = accountMigrationInterface(New(ServerConnector(ch)))
+	}
+
 	// Setup routes.
 	pluginsRoute := ch.srv.Router.PathPrefix("/plugins/{plugin_id:[A-Za-z0-9\\_\\-\\.]+}").Subrouter()
 	pluginsRoute.HandleFunc("", ch.ServePluginRequest)
@@ -75,6 +100,10 @@ func (ch *Channels) Start() error {
 	if err := ch.ensureAsymmetricSigningKey(); err != nil {
 		return errors.Wrapf(err, "unable to ensure asymmetric signing key")
 	}
+
+	if err := ch.ensurePostActionCookieSecret(); err != nil {
+		return errors.Wrapf(err, "unable to ensure PostAction cookie secret")
+	}
 	return nil
 }

--- a/app/config.go
+++ b/app/config.go
@@ -109,14 +109,14 @@ func (a *App) RemoveConfigListener(id string) {
 // ensurePostActionCookieSecret ensures that the key for encrypting PostActionCookie exists
 // and future calls to PostActionCookieSecret will always return a valid key, same on all
 // servers in the cluster
-func (s *Server) ensurePostActionCookieSecret() error {
-	if s.postActionCookieSecret != nil {
+func (ch *Channels) ensurePostActionCookieSecret() error {
+	if ch.postActionCookieSecret != nil {
 		return nil
 	}

 	var secret *model.SystemPostActionCookieSecret

-	value, err := s.Store.System().GetByName(model.SystemPostActionCookieSecretKey)
+	value, err := ch.srv.Store.System().GetByName(model.SystemPostActionCookieSecretKey)
 	if err == nil {
 		if err := json.Unmarshal([]byte(value.Value), &secret); err != nil {
 			return err
@@ -142,7 +142,7 @@ func (s *Server) ensurePostActionCookieSecret() error {
 		}
 		system.Value = string(v)
 		// If we were able to save the key, use it, otherwise log the error.
-		if err = s.Store.System().Save(system); err != nil {
+		if err = ch.srv.Store.System().Save(system); err != nil {
 			mlog.Warn("Failed to save PostActionCookieSecret", mlog.Err(err))
 		} else {
 			secret = newSecret
@@ -152,7 +152,7 @@ func (s *Server) ensurePostActionCookieSecret() error {
 	// If we weren't able to save a new key above, another server must have beat us to it. Get the
 	// key from the database, and if that fails, error out.
 	if secret == nil {
-		value, err := s.Store.System().GetByName(model.SystemPostActionCookieSecretKey)
+		value, err := ch.srv.Store.System().GetByName(model.SystemPostActionCookieSecretKey)
 		if err != nil {
 			return err
 		}
@@ -162,7 +162,7 @@ func (s *Server) ensurePostActionCookieSecret() error {
 		}
 	}

-	s.postActionCookieSecret = secret.Secret
+	ch.postActionCookieSecret = secret.Secret
 	return nil
 }

@@ -294,12 +294,12 @@ func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey {
 	return a.ch.AsymmetricSigningKey()
 }

-func (s *Server) PostActionCookieSecret() []byte {
-	return s.postActionCookieSecret
+func (ch *Channels) PostActionCookieSecret() []byte {
+	return ch.postActionCookieSecret
 }

 func (a *App) PostActionCookieSecret() []byte {
-	return a.Srv().PostActionCookieSecret()
+	return a.ch.PostActionCookieSecret()
 }

 func (ch *Channels) regenerateClientConfig() {
--- a/app/enterprise.go
+++ b/app/enterprise.go
@@ -12,9 +12,9 @@ import (
 	"github.com/mattermost/mattermost-server/v6/shared/mlog"
 )

-var accountMigrationInterface func(*Server) einterfaces.AccountMigrationInterface
+var accountMigrationInterface func(*App) einterfaces.AccountMigrationInterface

-func RegisterAccountMigrationInterface(f func(*Server) einterfaces.AccountMigrationInterface) {
+func RegisterAccountMigrationInterface(f func(*App) einterfaces.AccountMigrationInterface) {
 	accountMigrationInterface = f
 }

@@ -24,9 +24,9 @@ func RegisterClusterInterface(f func(*Server) einterfaces.ClusterInterface) {
 	clusterInterface = f
 }

-var complianceInterface func(*Server) einterfaces.ComplianceInterface
+var complianceInterface func(*App) einterfaces.ComplianceInterface

-func RegisterComplianceInterface(f func(*Server) einterfaces.ComplianceInterface) {
+func RegisterComplianceInterface(f func(*App) einterfaces.ComplianceInterface) {
 	complianceInterface = f
 }

@@ -36,9 +36,9 @@ func RegisterFixCRTChannelUnreadsJobInterface(f func(*Server) tjobs.FixCRTChanne
 	fixCRTChannelUnreadsJobInterface = f
 }

-var dataRetentionInterface func(*Server) einterfaces.DataRetentionInterface
+var dataRetentionInterface func(*App) einterfaces.DataRetentionInterface

-func RegisterDataRetentionInterface(f func(*Server) einterfaces.DataRetentionInterface) {
+func RegisterDataRetentionInterface(f func(*App) einterfaces.DataRetentionInterface) {
 	dataRetentionInterface = f
 }

@@ -163,9 +163,9 @@ func RegisterLdapInterface(f func(*Server) einterfaces.LdapInterface) {
 	ldapInterface = f
 }

-var messageExportInterface func(*Server) einterfaces.MessageExportInterface
+var messageExportInterface func(*App) einterfaces.MessageExportInterface

-func RegisterMessageExportInterface(f func(*Server) einterfaces.MessageExportInterface) {
+func RegisterMessageExportInterface(f func(*App) einterfaces.MessageExportInterface) {
 	messageExportInterface = f
 }

@@ -203,15 +203,7 @@ func (s *Server) initEnterprise() {
 	if metricsInterface != nil {
 		s.Metrics = metricsInterface(s)
 	}
-	if complianceInterface != nil {
-		s.Compliance = complianceInterface(s)
-	}
-	if messageExportInterface != nil {
-		s.MessageExport = messageExportInterface(s)
-	}
-	if dataRetentionInterface != nil {
-		s.DataRetention = dataRetentionInterface(s)
-	}
+
 	if clusterInterface != nil {
 		s.Cluster = clusterInterface(s)
 	}
@@ -223,15 +215,14 @@ func (s *Server) initEnterprise() {
 		s.LicenseManager = licenseInterface(s)
 	}

-	if accountMigrationInterface != nil {
-		s.AccountMigration = accountMigrationInterface(s)
-	}
 	if ldapInterface != nil {
 		s.Ldap = ldapInterface(s)
 	}
+
 	if notificationInterface != nil {
 		s.Notification = notificationInterface(s)
 	}
+
 	if samlInterfaceNew != nil {
 		mlog.Debug("Loading SAML2 library")
 		s.Saml = samlInterfaceNew(s)
@@ -244,6 +235,7 @@ func (s *Server) initEnterprise() {
 			}
 		})
 	}
+
 	if cloudInterface != nil {
 		s.Cloud = cloudInterface(s)
 	}
--- a/app/enterprise_test.go
+++ b/app/enterprise_test.go
@@ -57,7 +57,7 @@ func TestSAMLSettings(t *testing.T) {
 			saml2.Mock.On("ConfigureSP").Return(nil)
 			saml2.Mock.On("GetMetadata").Return("samlTwo", nil)
 			if tc.setNewInterface {
-				RegisterNewSamlInterface(func(s *Server) einterfaces.SamlInterface {
+				RegisterNewSamlInterface(func(_ *Server) einterfaces.SamlInterface {
 					return saml2
 				})
 			} else {
--- a/app/server.go
+++ b/app/server.go
@@ -140,7 +140,6 @@ type Server struct {
 	searchLicenseListenerId string
 	loggerLicenseListenerId string
 	configStore             *config.Store
-	postActionCookieSecret  []byte

 	telemetryService *telemetry.TelemetryService
 	userService      *users.UserService
@@ -163,17 +162,13 @@ type Server struct {

 	SearchEngine *searchengine.Broker

-	AccountMigration einterfaces.AccountMigrationInterface
-	Cluster          einterfaces.ClusterInterface
-	Compliance       einterfaces.ComplianceInterface
-	DataRetention    einterfaces.DataRetentionInterface
-	Ldap             einterfaces.LdapInterface
-	MessageExport    einterfaces.MessageExportInterface
-	Cloud            einterfaces.CloudInterface
-	Metrics          einterfaces.MetricsInterface
-	Notification     einterfaces.NotificationInterface
-	Saml             einterfaces.SamlInterface
-	LicenseManager   einterfaces.LicenseInterface
+	Cluster        einterfaces.ClusterInterface
+	Cloud          einterfaces.CloudInterface
+	Metrics        einterfaces.MetricsInterface
+	Notification   einterfaces.NotificationInterface
+	LicenseManager einterfaces.LicenseInterface
+	Saml           einterfaces.SamlInterface
+	Ldap           einterfaces.LdapInterface

 	CacheProvider cache.Provider

@@ -207,11 +202,14 @@ func NewServer(options ...Option) (*Server, error) {
 		goroutineExitSignal: make(chan struct{}, 1),
 		RootRouter:          rootRouter,
 		LocalRouter:         localRouter,
-		licenseListeners:    map[string]func(*model.License, *model.License){},
-		hashSeed:            maphash.MakeSeed(),
-		uploadLockMap:       map[string]bool{},
-		timezones:           timezones.New(),
-		products:            make(map[string]Product),
+		WebSocketRouter: &WebSocketRouter{
+			handlers: make(map[string]webSocketHandler),
+		},
+		licenseListeners: map[string]func(*model.License, *model.License){},
+		hashSeed:         maphash.MakeSeed(),
+		uploadLockMap:    map[string]bool{},
+		timezones:        timezones.New(),
+		products:         make(map[string]Product),
 	}

 	for _, option := range options {
@@ -523,18 +521,6 @@ func NewServer(options ...Option) (*Server, error) {
 		s.Cluster.StartInterNodeCommunication()
 	}

-	if err = s.ensurePostActionCookieSecret(); err != nil {
-		return nil, errors.Wrapf(err, "unable to ensure PostAction cookie secret")
-	}
-
-	if err = s.ensureInstallationDate(); err != nil {
-		return nil, errors.Wrapf(err, "unable to ensure installation date")
-	}
-
-	if err = s.ensureFirstServerRunTimestamp(); err != nil {
-		return nil, errors.Wrapf(err, "unable to ensure first run timestamp")
-	}
-
 	// If configured with a subpath, redirect 404s at the root back into the subpath.
 	if subpath != "/" {
 		s.RootRouter.NotFoundHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -543,35 +529,10 @@ func NewServer(options ...Option) (*Server, error) {
 		})
 	}

-	s.WebSocketRouter = &WebSocketRouter{
-		handlers: make(map[string]webSocketHandler),
-	}
-
-	mailConfig := s.MailServiceConfig()
-
-	if nErr := mail.TestConnection(mailConfig); nErr != nil {
-		mlog.Error("Mail server connection test is failed", mlog.Err(nErr))
-	}
-
 	if _, err = url.ParseRequestURI(*s.Config().ServiceSettings.SiteURL); err != nil {
 		mlog.Error("SiteURL must be set. Some features will operate incorrectly if the SiteURL is not set. See documentation for details: http://about.mattermost.com/default-site-url")
 	}

-	backend, appErr := s.FileBackend()
-	if appErr != nil {
-		mlog.Error("Problem with file storage settings", mlog.Err(appErr))
-	} else {
-		nErr := backend.TestConnection()
-		if nErr != nil {
-			if _, ok := nErr.(*filestore.S3FileBackendNoBucketError); ok {
-				nErr = backend.(*filestore.S3FileBackend).MakeBucket()
-			}
-			if nErr != nil {
-				mlog.Error("Problem with file storage settings", mlog.Err(nErr))
-			}
-		}
-	}
-
 	// Start email batching because it's not like the other jobs
 	s.AddConfigListener(func(_, _ *model.Config) {
 		s.EmailService.InitEmailBatching()
@@ -596,10 +557,6 @@ func NewServer(options ...Option) (*Server, error) {
 	mlog.Info("Printing current working", mlog.String("directory", pwd))
 	mlog.Info("Loaded config", mlog.String("source", s.configStore.String()))

-	s.checkPushNotificationServerURL()
-
-	s.ReloadConfig()
-
 	license := s.License()
 	allowAdvancedLogging := license != nil && *license.Features.AdvancedLogging

@@ -624,10 +581,6 @@ func NewServer(options ...Option) (*Server, error) {
 		s.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableDeveloper = true })
 	}

-	if err = s.Store.Status().ResetAll(); err != nil {
-		mlog.Error("Error to reset the server status.", mlog.Err(err))
-	}
-
 	if s.startMetrics {
 		s.SetupMetricsServer()
 	}
@@ -663,11 +616,10 @@ func NewServer(options ...Option) (*Server, error) {
 		return s, nil
 	}

-	c := request.EmptyContext()
-	s.AddConfigListener(func(oldConfig *model.Config, newConfig *model.Config) {
+	s.AddConfigListener(func(old, new *model.Config) {
 		appInstance := New(ServerConnector(s.Channels()))
-		if *oldConfig.GuestAccountsSettings.Enable && !*newConfig.GuestAccountsSettings.Enable {
-			if appErr := appInstance.DeactivateGuests(c); appErr != nil {
+		if *old.GuestAccountsSettings.Enable && !*new.GuestAccountsSettings.Enable {
+			if appErr := appInstance.DeactivateGuests(request.EmptyContext()); appErr != nil {
 				mlog.Error("Unable to deactivate guest accounts", mlog.Err(appErr))
 			}
 		}
@@ -676,7 +628,7 @@ func NewServer(options ...Option) (*Server, error) {
 	// Disable active guest accounts on first run if guest accounts are disabled
 	if !*s.Config().GuestAccountsSettings.Enable {
 		appInstance := New(ServerConnector(s.Channels()))
-		if appErr := appInstance.DeactivateGuests(c); appErr != nil {
+		if appErr := appInstance.DeactivateGuests(request.EmptyContext()); appErr != nil {
 			mlog.Error("Unable to deactivate guest accounts", mlog.Err(appErr))
 		}
 	}
@@ -685,7 +637,7 @@ func NewServer(options ...Option) (*Server, error) {
 		s.Go(func() {
 			appInstance := New(ServerConnector(s.Channels()))
 			s.runLicenseExpirationCheckJob()
-			runCheckAdminSupportStatusJob(appInstance, c)
+			runCheckAdminSupportStatusJob(appInstance, request.EmptyContext())
 			runDNDStatusExpireJob(appInstance)
 		})
 		s.runJobs()
@@ -759,7 +711,7 @@ func (s *Server) runJobs() {
 		runCommandWebhookCleanupJob(s)
 	})

-	if complianceI := s.Compliance; complianceI != nil {
+	if complianceI := s.Channels().Compliance; complianceI != nil {
 		complianceI.StartComplianceDailyJob()
 	}

@@ -1208,6 +1160,40 @@ func (s *Server) Start() error {
 		}
 	}

+	if err := s.ensureInstallationDate(); err != nil {
+		return errors.Wrapf(err, "unable to ensure installation date")
+	}
+
+	if err := s.ensureFirstServerRunTimestamp(); err != nil {
+		return errors.Wrapf(err, "unable to ensure first run timestamp")
+	}
+
+	if err := s.Store.Status().ResetAll(); err != nil {
+		mlog.Error("Error to reset the server status.", mlog.Err(err))
+	}
+	if err := mail.TestConnection(s.MailServiceConfig()); err != nil {
+		mlog.Error("Mail server connection test is failed", mlog.Err(err))
+	}
+
+	backend, appErr := s.FileBackend()
+	if appErr != nil {
+		mlog.Error("Problem with file storage settings", mlog.Err(appErr))
+	} else {
+		err := backend.TestConnection()
+		if err != nil {
+			if _, ok := err.(*filestore.S3FileBackendNoBucketError); ok {
+				err = backend.(*filestore.S3FileBackend).MakeBucket()
+			}
+			if err != nil {
+				mlog.Error("Problem with file storage settings", mlog.Err(err))
+			}
+		}
+	}
+
+	s.checkPushNotificationServerURL()
+
+	s.ReloadConfig()
+
 	mlog.Info("Starting Server...")

 	var handler http.Handler = s.RootRouter
@@ -2178,7 +2164,7 @@ func (a *App) generateSupportPacketYaml() (*model.FileData, string) {
 	}

 	// Here we are getting information regarding LDAP
-	ldapInterface := a.Srv().Ldap
+	ldapInterface := a.ch.srv.Ldap
 	var vendorName, vendorVersion string
 	if ldapInterface != nil {
 		vendorName, vendorVersion = ldapInterface.GetVendorNameAndVendorVersion()
--- a/app/server_test.go
+++ b/app/server_test.go
@@ -182,11 +182,15 @@ func TestStartServerNoS3Bucket(t *testing.T) {
 				AmazonS3PathPrefix:      model.NewString(""),
 				AmazonS3SSL:             model.NewBool(false),
 			}
+			*cfg.ServiceSettings.ListenAddress = ":0"
 		})
 		return nil
 	})
 	require.NoError(t, err)

+	require.NoError(t, s.Start())
+	defer s.Shutdown()
+
 	// ensure that a new bucket was created
 	backend, appErr := s.FileBackend()
 	require.Nil(t, appErr)