From 5ab3ad9bfd05f4a0ed59e70b514f917b075b6045 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 25 Nov 2021 10:07:43 +0530 Subject: [PATCH] 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 ``` --- app/app.go | 8 +-- app/channels.go | 29 ++++++++++ app/config.go | 18 +++--- app/enterprise.go | 32 ++++------- app/enterprise_test.go | 2 +- app/server.go | 126 ++++++++++++++++++----------------------- app/server_test.go | 4 ++ 7 files changed, 115 insertions(+), 104 deletions(-) diff --git a/app/app.go b/app/app.go index ca7dd7cc02..90b98a79b4 100644 --- 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 diff --git a/app/channels.go b/app/channels.go index d15d4860b0..7005cf0509 100644 --- 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 } diff --git a/app/config.go b/app/config.go index df080b04c4..4aafe7323f 100644 --- 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() { diff --git a/app/enterprise.go b/app/enterprise.go index 5003d0cfec..b8e0ed0d70 100644 --- 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) } diff --git a/app/enterprise_test.go b/app/enterprise_test.go index d486af5e34..8e05caa05b 100644 --- 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 { diff --git a/app/server.go b/app/server.go index 91777fee01..71d9a3863e 100644 --- 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() diff --git a/app/server_test.go b/app/server_test.go index f23ee95433..7155146759 100644 --- 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)