[pull] master from mattermost:master

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.

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)

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)