diff --git a/server/channels/api4/system.go b/server/channels/api4/system.go index e42788b405a..7de04c0676d 100644 --- a/server/channels/api4/system.go +++ b/server/channels/api4/system.go @@ -589,10 +589,12 @@ func testFileStore(c *Context, w http.ResponseWriter, r *http.Request) { // Validate mandatory fields per driver. TestFileStoreConnectionWithConfig // will catch missing fields by failing to construct the backend, but a // dedicated validation step lets us surface a clearer error. - driver := "" - if cfg.FileSettings.DriverName != nil { - driver = *cfg.FileSettings.DriverName - } + // + // When the dedicated export filestore is active the backend that is + // actually built and tested is the export backend, so we have to dispatch + // on ExportDriverName -- otherwise a primary=S3 / export=Azure deployment + // would run the S3 field check while testing the Azure backend. + driver := c.App.ResolvedFileStoreDriverName(&cfg.FileSettings) switch driver { case model.ImageDriverLocal: // Local driver has no mandatory fields beyond the directory, which has a default. diff --git a/server/channels/api4/system_test.go b/server/channels/api4/system_test.go index 4bcd37cbf8b..c838b79575a 100644 --- a/server/channels/api4/system_test.go +++ b/server/channels/api4/system_test.go @@ -826,6 +826,87 @@ func TestS3TestConnection(t *testing.T) { }) } +func TestFileStoreTestConnection(t *testing.T) { + mainHelper.Parallel(t) + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.CloudDedicatedExportUI = true + cfg.FileSettings.DedicatedExportStore = model.NewPointer(true) + }) + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + t.Cleanup(func() { th.App.Srv().SetLicense(nil) }) + + baseSettings := func(primary, export string) model.FileSettings { + fs := model.FileSettings{} + fs.SetDefaults(false) + fs.DriverName = model.NewPointer(primary) + fs.ExportDriverName = model.NewPointer(export) + return fs + } + + t.Run("primary=local, export=azure: missing ExportAzureContainer surfaces azure error", func(t *testing.T) { + fs := baseSettings(model.ImageDriverLocal, model.ImageDriverAzure) + fs.ExportAzureStorageAccount = model.NewPointer("acmemattermost") + fs.ExportAzureAccessKey = model.NewPointer("secret") + fs.ExportAzureContainer = model.NewPointer("") + + resp, err := th.SystemAdminClient.TestS3Connection(context.Background(), &model.Config{FileSettings: fs}) + require.Error(t, err) + CheckErrorID(t, err, "api.admin.test_azure.missing_azure_field") + CheckBadRequestStatus(t, resp) + }) + + t.Run("primary=s3, export=azure: missing ExportAzureStorageAccount surfaces azure error", func(t *testing.T) { + fs := baseSettings(model.ImageDriverS3, model.ImageDriverAzure) + fs.ExportAzureStorageAccount = model.NewPointer("") + fs.ExportAzureContainer = model.NewPointer("mattermost") + fs.ExportAzureAccessKey = model.NewPointer("secret") + + resp, err := th.SystemAdminClient.TestS3Connection(context.Background(), &model.Config{FileSettings: fs}) + require.Error(t, err) + CheckErrorID(t, err, "api.admin.test_azure.missing_azure_field") + CheckBadRequestStatus(t, resp) + }) + + t.Run("primary=local, export=s3: missing ExportAmazonS3Bucket surfaces s3 error", func(t *testing.T) { + fs := baseSettings(model.ImageDriverLocal, model.ImageDriverS3) + fs.ExportAmazonS3Bucket = model.NewPointer("") + + resp, err := th.SystemAdminClient.TestS3Connection(context.Background(), &model.Config{FileSettings: fs}) + require.Error(t, err) + CheckErrorID(t, err, "api.admin.test_s3.missing_s3_bucket") + CheckBadRequestStatus(t, resp) + }) + + t.Run("unsupported export driver", func(t *testing.T) { + fs := baseSettings(model.ImageDriverS3, "bogus") + + resp, err := th.SystemAdminClient.TestS3Connection(context.Background(), &model.Config{FileSettings: fs}) + require.Error(t, err) + CheckErrorID(t, err, "api.file.test_connection_unsupported_driver.app_error") + CheckBadRequestStatus(t, resp) + }) + + t.Run("no license - primary dispatch", func(t *testing.T) { + // No license is set, so UseExportFileStore() must return false. Set an + // ExportDriverName that, if it were honored, would change the dispatch + // outcome -- this asserts that the primary path is taken. + appErr := th.App.Srv().RemoveLicense() + require.Nil(t, appErr) + fs := baseSettings(model.ImageDriverAzure, model.ImageDriverS3) + + // fs.AzureStorageAccount = model.NewPointer("") + // fs.AzureContainer = model.NewPointer("mattermost") + // fs.AzureAccessKey = model.NewPointer("secret") + // fs.ExportAmazonS3Bucket = model.NewPointer("export-bucket") + + resp, err := th.SystemAdminClient.TestS3Connection(context.Background(), &model.Config{FileSettings: fs}) + require.Error(t, err) + CheckErrorID(t, err, "api.admin.test_azure.missing_azure_field") + CheckBadRequestStatus(t, resp) + }) +} + func TestSupportedTimezones(t *testing.T) { mainHelper.Parallel(t) th := Setup(t) diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 1e4750caad9..7608c627bec 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -59,9 +59,41 @@ func (a *App) ExportFileBackend() filestore.FileBackend { return a.ch.exportFilestore } +// UseExportFileStore reports whether the dedicated export filestore is +// active. When true, callers reading the filestore configuration should +// resolve the export-side fields (ExportDriverName, ExportAmazonS3*, +// ExportAzure*, ...) rather than the primary fields. +func (a *App) UseExportFileStore() bool { + if !a.License().IsCloud() { + return false + } + if !a.Config().FeatureFlags.CloudDedicatedExportUI { + return false + } + dedicated := a.Config().FileSettings.DedicatedExportStore + return dedicated != nil && *dedicated +} + +// ResolvedFileStoreDriverName returns the driver name that callers should +// read for the active filestore -- ExportDriverName when the dedicated +// export filestore is enabled, otherwise the primary DriverName. The empty +// string is returned when neither pointer is set so callers can produce a +// dedicated "unsupported driver" error. +func (a *App) ResolvedFileStoreDriverName(settings *model.FileSettings) string { + name := settings.DriverName + if a.UseExportFileStore() { + name = settings.ExportDriverName + } + + if name == nil { + return "" + } + return *name +} + func (a *App) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError { bucket := settings.AmazonS3Bucket - if a.License().IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { + if a.UseExportFileStore() { bucket = settings.ExportAmazonS3Bucket } if bucket == nil || *bucket == "" { @@ -75,7 +107,7 @@ func (a *App) CheckMandatoryAzureFields(settings *model.FileSettings) *model.App authMode := settings.AzureAuthMode accessKey := settings.AzureAccessKey container := settings.AzureContainer - if a.License().IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { + if a.UseExportFileStore() { storageAccount = settings.ExportAzureStorageAccount authMode = settings.ExportAzureAuthMode accessKey = settings.ExportAzureAccessKey @@ -128,11 +160,11 @@ func (a *App) TestFileStoreConnectionWithConfig(cfg *model.FileSettings) *model. var backend filestore.FileBackend var err error complianceEnabled := license != nil && *license.Features.Compliance - if license.IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { - allowInsecure := a.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *a.Config().ServiceSettings.EnableInsecureOutgoingConnections + allowInsecure := insecure != nil && *insecure + if a.UseExportFileStore() { backend, err = filestore.NewFileBackend(filestore.NewExportFileBackendSettingsFromConfig(cfg, complianceEnabled && license.IsCloud(), allowInsecure)) } else { - backend, err = filestore.NewFileBackend(filestore.NewFileBackendSettingsFromConfig(cfg, complianceEnabled, insecure != nil && *insecure)) + backend, err = filestore.NewFileBackend(filestore.NewFileBackendSettingsFromConfig(cfg, complianceEnabled, allowInsecure)) } if err != nil { return model.NewAppError("FileAttachmentBackend", "api.file.no_driver.app_error", nil, "", http.StatusInternalServerError).Wrap(err)