remove s3 region to be mandatory and fix when user call test s3 when the config is saved (#8454)

api4/system.go

@@ -395,6 +395,10 @@ func testS3(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if cfg.FileSettings.AmazonS3SecretAccessKey == model.FAKE_SETTING {
+		cfg.FileSettings.AmazonS3SecretAccessKey = c.App.Config().FileSettings.AmazonS3SecretAccessKey
+	}
+
 	license := c.App.License()
 	backend, appErr := utils.NewFileBackend(&cfg.FileSettings, license != nil && *license.Features.Compliance)
 	if appErr == nil {

api4/system_test.go

@@ -497,7 +497,7 @@ func TestS3TestConnection(t *testing.T) {
 			AmazonS3AccessKeyId:     model.MINIO_ACCESS_KEY,
 			AmazonS3SecretAccessKey: model.MINIO_SECRET_KEY,
 			AmazonS3Bucket:          "",
-			AmazonS3Endpoint:        "",
+			AmazonS3Endpoint:        s3Endpoint,
 			AmazonS3SSL:             model.NewBool(false),
 		},
 	}
@@ -512,23 +512,14 @@ func TestS3TestConnection(t *testing.T) {
 	}
 
 	config.FileSettings.AmazonS3Bucket = model.MINIO_BUCKET
-	_, resp = th.SystemAdminClient.TestS3Connection(&config)
-	CheckBadRequestStatus(t, resp)
-	if resp.Error.Message != "S3 Endpoint is required" {
-		t.Fatal("should return error - missing s3 endpoint")
-	}
-
-	config.FileSettings.AmazonS3Endpoint = s3Endpoint
-	_, resp = th.SystemAdminClient.TestS3Connection(&config)
-	CheckBadRequestStatus(t, resp)
-	if resp.Error.Message != "S3 Region is required" {
-		t.Fatal("should return error - missing s3 region")
-	}
-
 	config.FileSettings.AmazonS3Region = "us-east-1"
 	_, resp = th.SystemAdminClient.TestS3Connection(&config)
 	CheckOKStatus(t, resp)
 
+	config.FileSettings.AmazonS3Region = ""
+	_, resp = th.SystemAdminClient.TestS3Connection(&config)
+	CheckOKStatus(t, resp)
+
 	config.FileSettings.AmazonS3Bucket = "Wrong_bucket"
 	_, resp = th.SystemAdminClient.TestS3Connection(&config)
 	CheckInternalErrorStatus(t, resp)

utils/file_backend_s3.go

@@ -253,12 +253,9 @@ func CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError {
 		return model.NewAppError("S3File", "api.admin.test_s3.missing_s3_bucket", nil, "", http.StatusBadRequest)
 	}
 
+	// if S3 endpoint is not set call the set defaults to set that
 	if len(settings.AmazonS3Endpoint) == 0 {
-		return model.NewAppError("S3File", "api.admin.test_s3.missing_s3_endpoint", nil, "", http.StatusBadRequest)
+		settings.SetDefaults()
-	}
-
-	if len(settings.AmazonS3Region) == 0 {
-		return model.NewAppError("S3File", "api.admin.test_s3.missing_s3_region", nil, "", http.StatusBadRequest)
 	}
 
 	return nil

utils/file_backend_s3_test.go

@@ -19,14 +19,14 @@ func TestCheckMandatoryS3Fields(t *testing.T) {
 
 	cfg.AmazonS3Bucket = "test-mm"
 	err = CheckMandatoryS3Fields(&cfg)
-	if err == nil || err.Message != "api.admin.test_s3.missing_s3_endpoint" {
+	if err != nil {
-		t.Fatal("should've failed with missing s3 endpoint")
+		t.Fatal("should've not failed")
 	}
 
-	cfg.AmazonS3Endpoint = "s3.newendpoint.com"
+	cfg.AmazonS3Endpoint = ""
 	err = CheckMandatoryS3Fields(&cfg)
-	if err == nil || err.Message != "api.admin.test_s3.missing_s3_region" {
+	if err != nil || cfg.AmazonS3Endpoint != "s3.amazonaws.com" {
-		t.Fatal("should've failed with missing s3 region")
+		t.Fatal("should've not failed because it should set the endpoint to the default")
 	}
 
 }