Implemented AmazonS3PathPrefix (#14917)

* Implemented AmazonS3PathPrefix

* Remove unecessary method

* fix for test

* fix for test which are failing

* fix for test which are failing

* fix for test

Co-authored-by: Dusan Panic <dusan@salestrekker.com>

api4/system_test.go

@@ -404,6 +404,7 @@ func TestS3TestConnection(t *testing.T) {
 			AmazonS3Bucket:          model.NewString(""),
 			AmazonS3Endpoint:        model.NewString(s3Endpoint),
 			AmazonS3Region:          model.NewString(""),
+			AmazonS3PathPrefix:      model.NewString(""),
 			AmazonS3SSL:             model.NewBool(false),
 		},
 	}
@@ -420,6 +421,7 @@ func TestS3TestConnection(t *testing.T) {
 		// If this fails, check the test configuration to ensure minio is setup with the
 		// `mattermost-test` bucket defined by model.MINIO_BUCKET.
 		*config.FileSettings.AmazonS3Bucket = model.MINIO_BUCKET
+		config.FileSettings.AmazonS3PathPrefix = model.NewString("")
 		*config.FileSettings.AmazonS3Region = "us-east-1"
 		_, resp = th.SystemAdminClient.TestS3Connection(&config)
 		CheckOKStatus(t, resp)

app/plugin_test.go

@@ -498,6 +498,7 @@ func TestPluginSync(t *testing.T) {
 				cfg.FileSettings.AmazonS3AccessKeyId = model.NewString(model.MINIO_ACCESS_KEY)
 				cfg.FileSettings.AmazonS3SecretAccessKey = model.NewString(model.MINIO_SECRET_KEY)
 				cfg.FileSettings.AmazonS3Bucket = model.NewString(model.MINIO_BUCKET)
+				cfg.FileSettings.AmazonS3PathPrefix = model.NewString("")
 				cfg.FileSettings.AmazonS3Endpoint = model.NewString(s3Endpoint)
 				cfg.FileSettings.AmazonS3Region = model.NewString("")
 				cfg.FileSettings.AmazonS3SSL = model.NewBool(false)

model/config.go

@@ -1265,6 +1265,7 @@ type FileSettings struct {
 	AmazonS3AccessKeyId     *string `restricted:"true"`
 	AmazonS3SecretAccessKey *string `restricted:"true"`
 	AmazonS3Bucket          *string `restricted:"true"`
+	AmazonS3PathPrefix      *string `restricted:"true"`
 	AmazonS3Region          *string `restricted:"true"`
 	AmazonS3Endpoint        *string `restricted:"true"`
 	AmazonS3SSL             *bool   `restricted:"true"`
@@ -1329,6 +1330,10 @@ func (s *FileSettings) SetDefaults(isUpdate bool) {
 		s.AmazonS3Bucket = NewString("")
 	}
 
+	if s.AmazonS3PathPrefix == nil {
+		s.AmazonS3PathPrefix = NewString("")
+	}
+
 	if s.AmazonS3Region == nil {
 		s.AmazonS3Region = NewString("")
 	}

services/filesstore/filesstore.go

@@ -34,15 +34,16 @@ func NewFileBackend(settings *model.FileSettings, enableComplianceFeatures bool)
 	switch *settings.DriverName {
 	case model.IMAGE_DRIVER_S3:
 		return &S3FileBackend{
-			endpoint:  *settings.AmazonS3Endpoint,
+			endpoint:   *settings.AmazonS3Endpoint,
-			accessKey: *settings.AmazonS3AccessKeyId,
+			accessKey:  *settings.AmazonS3AccessKeyId,
-			secretKey: *settings.AmazonS3SecretAccessKey,
+			secretKey:  *settings.AmazonS3SecretAccessKey,
-			secure:    settings.AmazonS3SSL == nil || *settings.AmazonS3SSL,
+			secure:     settings.AmazonS3SSL == nil || *settings.AmazonS3SSL,
-			signV2:    settings.AmazonS3SignV2 != nil && *settings.AmazonS3SignV2,
+			signV2:     settings.AmazonS3SignV2 != nil && *settings.AmazonS3SignV2,
-			region:    *settings.AmazonS3Region,
+			region:     *settings.AmazonS3Region,
-			bucket:    *settings.AmazonS3Bucket,
+			bucket:     *settings.AmazonS3Bucket,
-			encrypt:   settings.AmazonS3SSE != nil && *settings.AmazonS3SSE && enableComplianceFeatures,
+			pathPrefix: *settings.AmazonS3PathPrefix,
-			trace:     settings.AmazonS3Trace != nil && *settings.AmazonS3Trace,
+			encrypt:    settings.AmazonS3SSE != nil && *settings.AmazonS3SSE && enableComplianceFeatures,
+			trace:      settings.AmazonS3Trace != nil && *settings.AmazonS3Trace,
 		}, nil
 	case model.IMAGE_DRIVER_LOCAL:
 		return &LocalFileBackend{

services/filesstore/filesstore_test.go

@@ -76,6 +76,7 @@ func runBackendTest(t *testing.T, encrypt bool) {
 			AmazonS3Bucket:          model.NewString(model.MINIO_BUCKET),
 			AmazonS3Region:          model.NewString(""),
 			AmazonS3Endpoint:        model.NewString(s3Endpoint),
+			AmazonS3PathPrefix:      model.NewString(""),
 			AmazonS3SSL:             model.NewBool(false),
 			AmazonS3SSE:             model.NewBool(encrypt),
 		},

services/filesstore/s3store.go

@@ -21,15 +21,16 @@ import (
 )
 
 type S3FileBackend struct {
-	endpoint  string
+	endpoint   string
-	accessKey string
+	accessKey  string
-	secretKey string
+	secretKey  string
-	secure    bool
+	secure     bool
-	signV2    bool
+	signV2     bool
-	region    string
+	region     string
-	bucket    string
+	bucket     string
-	encrypt   bool
+	pathPrefix string
-	trace     bool
+	encrypt    bool
+	trace      bool
 }
 
 // Similar to s3.New() but allows initialization of signature v2 or signature v4 client.
@@ -89,10 +90,13 @@ func (b *S3FileBackend) Reader(path string) (ReadCloseSeeker, *model.AppError) {
 	if err != nil {
 		return nil, model.NewAppError("Reader", "api.file.reader.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
+	path = filepath.Join(b.pathPrefix, path)
 	minioObject, err := s3Clnt.GetObject(b.bucket, path, s3.GetObjectOptions{})
 	if err != nil {
 		return nil, model.NewAppError("Reader", "api.file.reader.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
 	return minioObject, nil
 }
 
@@ -101,10 +105,13 @@ func (b *S3FileBackend) ReadFile(path string) ([]byte, *model.AppError) {
 	if err != nil {
 		return nil, model.NewAppError("ReadFile", "api.file.read_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
+	path = filepath.Join(b.pathPrefix, path)
 	minioObject, err := s3Clnt.GetObject(b.bucket, path, s3.GetObjectOptions{})
 	if err != nil {
 		return nil, model.NewAppError("ReadFile", "api.file.read_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
 	defer minioObject.Close()
 	if f, err := ioutil.ReadAll(minioObject); err != nil {
 		return nil, model.NewAppError("ReadFile", "api.file.read_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
@@ -119,6 +126,8 @@ func (b *S3FileBackend) FileExists(path string) (bool, *model.AppError) {
 	if err != nil {
 		return false, model.NewAppError("FileExists", "api.file.file_exists.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
+	path = filepath.Join(b.pathPrefix, path)
 	_, err = s3Clnt.StatObject(b.bucket, path, s3.StatObjectOptions{})
 
 	if err == nil {
@@ -138,11 +147,15 @@ func (b *S3FileBackend) CopyFile(oldPath, newPath string) *model.AppError {
 		return model.NewAppError("copyFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
+	oldPath = filepath.Join(b.pathPrefix, oldPath)
+	newPath = filepath.Join(b.pathPrefix, newPath)
+
 	source := s3.NewSourceInfo(b.bucket, oldPath, nil)
 	destination, err := s3.NewDestinationInfo(b.bucket, newPath, encrypt.NewSSE(), nil)
 	if err != nil {
 		return model.NewAppError("copyFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
 	if err = s3Clnt.CopyObject(destination, source); err != nil {
 		return model.NewAppError("copyFile", "api.file.move_file.copy_within_s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
@@ -155,17 +168,23 @@ func (b *S3FileBackend) MoveFile(oldPath, newPath string) *model.AppError {
 		return model.NewAppError("moveFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
+	oldPath = filepath.Join(b.pathPrefix, oldPath)
+	newPath = filepath.Join(b.pathPrefix, newPath)
+
 	source := s3.NewSourceInfo(b.bucket, oldPath, nil)
 	destination, err := s3.NewDestinationInfo(b.bucket, newPath, encrypt.NewSSE(), nil)
 	if err != nil {
 		return model.NewAppError("moveFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
 	if err = s3Clnt.CopyObject(destination, source); err != nil {
 		return model.NewAppError("moveFile", "api.file.move_file.copy_within_s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
 	if err = s3Clnt.RemoveObject(b.bucket, oldPath); err != nil {
 		return model.NewAppError("moveFile", "api.file.move_file.delete_from_s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
+
 	return nil
 }
 
@@ -176,6 +195,7 @@ func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppE
 	}
 
 	var contentType string
+	path = filepath.Join(b.pathPrefix, path)
 	if ext := filepath.Ext(path); model.IsFileExtImage(ext) {
 		contentType = model.GetImageMimeType(ext)
 	} else {
@@ -202,6 +222,7 @@ func (b *S3FileBackend) RemoveFile(path string) *model.AppError {
 		return model.NewAppError("RemoveFile", "utils.file.remove_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
+	path = filepath.Join(b.pathPrefix, path)
 	if err := s3Clnt.RemoveObject(b.bucket, path); err != nil {
 		return model.NewAppError("RemoveFile", "utils.file.remove_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
@@ -240,11 +261,13 @@ func (b *S3FileBackend) ListDirectory(path string) (*[]string, *model.AppError)
 	doneCh := make(chan struct{})
 	defer close(doneCh)
 
+	path = filepath.Join(b.pathPrefix, path)
 	if !strings.HasSuffix(path, "/") && len(path) > 0 {
 		// s3Clnt returns only the path itself when "/" is not present
 		// appending "/" to make it consistent across all filesstores
 		path = path + "/"
 	}
+
 	for object := range s3Clnt.ListObjects(b.bucket, path, false, doneCh) {
 		if object.Err != nil {
 			return nil, model.NewAppError("ListDirectory", "utils.file.list_directory.s3.app_error", nil, object.Err.Error(), http.StatusInternalServerError)
@@ -263,6 +286,7 @@ func (b *S3FileBackend) RemoveDirectory(path string) *model.AppError {
 
 	doneCh := make(chan struct{})
 
+	path = filepath.Join(b.pathPrefix, path)
 	for err := range s3Clnt.RemoveObjects(b.bucket, getPathsFromObjectInfos(s3Clnt.ListObjects(b.bucket, path, true, doneCh))) {
 		if err.Err != nil {
 			doneCh <- struct{}{}