From de3e69f84696e397a5bacaf6cfa06928ac516f9d Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 13 Mar 2020 17:28:55 +0100 Subject: [PATCH] s3: add documentation and test cases for upload part size --- docs/account.md | 1 + httpd/api_utils.go | 17 +++++++++++++++++ httpd/httpd_test.go | 21 +++++++++++++++++++-- httpd/internal_test.go | 6 ++++++ httpd/schema/openapi.yaml | 8 +++++++- httpd/web.go | 2 +- scripts/README.md | 5 +++-- scripts/sftpgo_api_cli.py | 24 ++++++++++++++---------- templates/user.html | 9 ++++++--- vfs/s3fs.go | 18 ++++++++++++------ vfs/vfs.go | 4 ++-- 11 files changed, 88 insertions(+), 27 deletions(-) diff --git a/docs/account.md b/docs/account.md index e80afa87..0824d5a6 100644 --- a/docs/account.md +++ b/docs/account.md @@ -46,6 +46,7 @@ For each account, the following properties can be configured: - `s3_endpoint`, specifies a S3 endpoint (server) different from AWS. It is not required if you are connecting to AWS - `s3_storage_class`, leave blank to use the default or specify a valid AWS [storage class](https://docs.aws.amazon.com/AmazonS3/latest/dev/storage-class-intro.html) - `s3_key_prefix`, allows to restrict access to the virtual folder identified by this prefix and its contents +- `s3_upload_part_size`, the buffer size for multipart uploads (MB). Zero means the default (5 MB). Minimum is 5 - `gcs_bucket`, required for GCS filesystem - `gcs_credentials`, Google Cloud Storage JSON credentials base64 encoded - `gcs_automatic_credentials`, integer. Set to 1 to use Application Default Credentials strategy or set to 0 to use explicit credentials via `gcs_credentials` diff --git a/httpd/api_utils.go b/httpd/api_utils.go index 72480755..d194a437 100644 --- a/httpd/api_utils.go +++ b/httpd/api_utils.go @@ -453,6 +453,16 @@ func compareUserFsConfig(expected *dataprovider.User, actual *dataprovider.User) if expected.FsConfig.Provider != actual.FsConfig.Provider { return errors.New("Fs provider mismatch") } + if err := compareS3Config(expected, actual); err != nil { + return err + } + if err := compareGCSConfig(expected, actual); err != nil { + return err + } + return nil +} + +func compareS3Config(expected *dataprovider.User, actual *dataprovider.User) error { if expected.FsConfig.S3Config.Bucket != actual.FsConfig.S3Config.Bucket { return errors.New("S3 bucket mismatch") } @@ -471,10 +481,17 @@ func compareUserFsConfig(expected *dataprovider.User, actual *dataprovider.User) if expected.FsConfig.S3Config.StorageClass != actual.FsConfig.S3Config.StorageClass { return errors.New("S3 storage class mismatch") } + if expected.FsConfig.S3Config.UploadPartSize != actual.FsConfig.S3Config.UploadPartSize { + return errors.New("S3 upload part size class mismatch") + } if expected.FsConfig.S3Config.KeyPrefix != actual.FsConfig.S3Config.KeyPrefix && expected.FsConfig.S3Config.KeyPrefix+"/" != actual.FsConfig.S3Config.KeyPrefix { return errors.New("S3 key prefix mismatch") } + return nil +} + +func compareGCSConfig(expected *dataprovider.User, actual *dataprovider.User) error { if expected.FsConfig.GCSConfig.Bucket != actual.FsConfig.GCSConfig.Bucket { return errors.New("GCS bucket mismatch") } diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go index 0388fa5e..b154e70a 100644 --- a/httpd/httpd_test.go +++ b/httpd/httpd_test.go @@ -394,6 +394,12 @@ func TestAddUserInvalidFsConfig(t *testing.T) { if err != nil { t.Errorf("unexpected error adding user with invalid fs config: %v", err) } + u.FsConfig.S3Config.KeyPrefix = "" + u.FsConfig.S3Config.UploadPartSize = 3 + _, _, err = httpd.AddUser(u, http.StatusBadRequest) + if err != nil { + t.Errorf("unexpected error adding user with invalid fs config: %v", err) + } u = getTestUser() u.FsConfig.Provider = 2 u.FsConfig.GCSConfig.Bucket = "" @@ -2010,7 +2016,7 @@ func TestWebUserS3Mock(t *testing.T) { user.FsConfig.S3Config.Endpoint = "http://127.0.0.1:9000/path?a=b" user.FsConfig.S3Config.StorageClass = "Standard" user.FsConfig.S3Config.KeyPrefix = "somedir/subdir/" - user.FsConfig.S3Config.PartSize = 5 + user.FsConfig.S3Config.UploadPartSize = 5 form := make(url.Values) form.Set("username", user.Username) form.Set("home_dir", user.HomeDir) @@ -2035,13 +2041,21 @@ func TestWebUserS3Mock(t *testing.T) { form.Set("s3_storage_class", user.FsConfig.S3Config.StorageClass) form.Set("s3_endpoint", user.FsConfig.S3Config.Endpoint) form.Set("s3_key_prefix", user.FsConfig.S3Config.KeyPrefix) - form.Set("s3_part_size", strconv.FormatInt(int64(user.FsConfig.S3Config.PartSize), 10)) form.Set("allowed_extensions", "/dir1::.jpg,.png") form.Set("denied_extensions", "/dir2::.zip") + // test invalid s3_upload_part_size + form.Set("s3_upload_part_size", "a") b, contentType, _ := getMultipartFormData(form, "", "") req, _ = http.NewRequest(http.MethodPost, webUserPath+"/"+strconv.FormatInt(user.ID, 10), &b) req.Header.Set("Content-Type", contentType) rr = executeRequest(req) + checkResponseCode(t, http.StatusOK, rr.Code) + // now add the user + form.Set("s3_upload_part_size", strconv.FormatInt(user.FsConfig.S3Config.UploadPartSize, 10)) + b, contentType, _ = getMultipartFormData(form, "", "") + req, _ = http.NewRequest(http.MethodPost, webUserPath+"/"+strconv.FormatInt(user.ID, 10), &b) + req.Header.Set("Content-Type", contentType) + rr = executeRequest(req) checkResponseCode(t, http.StatusSeeOther, rr.Code) req, _ = http.NewRequest(http.MethodGet, userPath+"?limit=1&offset=0&order=ASC&username="+user.Username, nil) rr = executeRequest(req) @@ -2082,6 +2096,9 @@ func TestWebUserS3Mock(t *testing.T) { if updateUser.FsConfig.S3Config.KeyPrefix != user.FsConfig.S3Config.KeyPrefix { t.Error("s3 key prefix mismatch") } + if updateUser.FsConfig.S3Config.UploadPartSize != user.FsConfig.S3Config.UploadPartSize { + t.Error("s3 upload part size mismatch") + } if len(updateUser.Filters.FileExtensions) != 2 { t.Errorf("unexpected extensions filter: %+v", updateUser.Filters.FileExtensions) } diff --git a/httpd/internal_test.go b/httpd/internal_test.go index 4e020b23..c7e7b5c2 100644 --- a/httpd/internal_test.go +++ b/httpd/internal_test.go @@ -363,6 +363,12 @@ func TestCompareUserFsConfig(t *testing.T) { t.Errorf("S3 key prefix does not match") } expected.FsConfig.S3Config.KeyPrefix = "" + expected.FsConfig.S3Config.UploadPartSize = 10 + err = compareUserFsConfig(expected, actual) + if err == nil { + t.Errorf("S3 upload part size does not match") + } + expected.FsConfig.S3Config.UploadPartSize = 0 } func TestCompareUserGCSConfig(t *testing.T) { diff --git a/httpd/schema/openapi.yaml b/httpd/schema/openapi.yaml index 056c4dd9..eae65b42 100644 --- a/httpd/schema/openapi.yaml +++ b/httpd/schema/openapi.yaml @@ -2,7 +2,7 @@ openapi: 3.0.1 info: title: SFTPGo description: 'SFTPGo REST API' - version: 1.8.3 + version: 1.8.4 servers: - url: /api/v1 @@ -1004,6 +1004,12 @@ components: description: optional endpoint storage_class: type: string + upload_part_size: + type: integer + description: the buffer size (in MB) to use for multipart uploads. The minimum allowed part size is 5MB, and if this value is set to zero, the default value (5MB) for the AWS SDK will be used. The minimum allowed value is 5. + upload_concurrency: + type: integer + description: the number of parts to upload in parallel. If this value is set to zero, 2 will be used key_prefix: type: string description: key_prefix is similar to a chroot directory for a local filesystem. If specified the SFTP user will only see contents that starts with this prefix and so you can restrict access to a specific virtual folder. The prefix, if not empty, must not start with "/" and must end with "/". If empty the whole bucket contents will be available diff --git a/httpd/web.go b/httpd/web.go index 69e96167..beb1e36d 100644 --- a/httpd/web.go +++ b/httpd/web.go @@ -329,7 +329,7 @@ func getFsConfigFromUserPostFields(r *http.Request) (dataprovider.Filesystem, er fs.S3Config.Endpoint = r.Form.Get("s3_endpoint") fs.S3Config.StorageClass = r.Form.Get("s3_storage_class") fs.S3Config.KeyPrefix = r.Form.Get("s3_key_prefix") - fs.S3Config.PartSize, err = strconv.ParseInt(r.Form.Get("s3_part_size"), 10, 64) + fs.S3Config.UploadPartSize, err = strconv.ParseInt(r.Form.Get("s3_upload_part_size"), 10, 64) if err != nil { return fs, err } diff --git a/scripts/README.md b/scripts/README.md index b6f17604..af420750 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -44,7 +44,7 @@ Let's see a sample usage for each REST API. Command: ``` -python sftpgo_api_cli.py add-user test_username --password "test_pwd" --home-dir="/tmp/test_home_dir" --uid 33 --gid 1000 --max-sessions 2 --quota-size 0 --quota-files 3 --permissions "list" "download" "upload" "delete" "rename" "create_dirs" "overwrite" --subdirs-permissions "/dir1::list,download" "/dir2::*" --upload-bandwidth 100 --download-bandwidth 60 --status 0 --expiration-date 2019-01-01 --allowed-ip "192.168.1.1/32" --fs S3 --s3-bucket test --s3-region eu-west-1 --s3-access-key accesskey --s3-access-secret secret --s3-endpoint "http://127.0.0.1:9000" --s3-storage-class Standard --s3-key-prefix "vfolder/" --denied-login-methods "password" "keyboard-interactive" --allowed-extensions "/dir1::.jpg,.png" "/dir2::.rar,.png" --denied-extensions "/dir3::.zip,.rar" +python sftpgo_api_cli.py add-user test_username --password "test_pwd" --home-dir="/tmp/test_home_dir" --uid 33 --gid 1000 --max-sessions 2 --quota-size 0 --quota-files 3 --permissions "list" "download" "upload" "delete" "rename" "create_dirs" "overwrite" --subdirs-permissions "/dir1::list,download" "/dir2::*" --upload-bandwidth 100 --download-bandwidth 60 --status 0 --expiration-date 2019-01-01 --allowed-ip "192.168.1.1/32" --fs S3 --s3-bucket test --s3-region eu-west-1 --s3-access-key accesskey --s3-access-secret secret --s3-endpoint "http://127.0.0.1:9000" --s3-storage-class Standard --s3-key-prefix "vfolder/" --s3-upload-part-size 10 --denied-login-methods "password" "keyboard-interactive" --allowed-extensions "/dir1::.jpg,.png" "/dir2::.rar,.png" --denied-extensions "/dir3::.zip,.rar" ``` Output: @@ -63,7 +63,8 @@ Output: "endpoint": "http://127.0.0.1:9000", "key_prefix": "vfolder/", "region": "eu-west-1", - "storage_class": "Standard" + "storage_class": "Standard", + "upload_part_size": 10 } }, "filters": { diff --git a/scripts/sftpgo_api_cli.py b/scripts/sftpgo_api_cli.py index a29dce8b..dbdba6f5 100755 --- a/scripts/sftpgo_api_cli.py +++ b/scripts/sftpgo_api_cli.py @@ -77,7 +77,7 @@ class SFTPGoApiRequests: s3_region='', s3_access_key='', s3_access_secret='', s3_endpoint='', s3_storage_class='', s3_key_prefix='', gcs_bucket='', gcs_key_prefix='', gcs_storage_class='', gcs_credentials_file='', gcs_automatic_credentials='automatic', denied_login_methods=[], virtual_folders=[], - denied_extensions=[], allowed_extensions=[]): + denied_extensions=[], allowed_extensions=[], s3_upload_part_size=0): user = {'id':user_id, 'username':username, 'uid':uid, 'gid':gid, 'max_sessions':max_sessions, 'quota_size':quota_size, 'quota_files':quota_files, 'upload_bandwidth':upload_bandwidth, 'download_bandwidth':download_bandwidth, @@ -101,7 +101,7 @@ class SFTPGoApiRequests: user.update({'filesystem':self.buildFsConfig(fs_provider, s3_bucket, s3_region, s3_access_key, s3_access_secret, s3_endpoint, s3_storage_class, s3_key_prefix, gcs_bucket, gcs_key_prefix, gcs_storage_class, gcs_credentials_file, - gcs_automatic_credentials)}) + gcs_automatic_credentials, s3_upload_part_size)}) return user def buildVirtualFolders(self, vfolders): @@ -204,12 +204,12 @@ class SFTPGoApiRequests: def buildFsConfig(self, fs_provider, s3_bucket, s3_region, s3_access_key, s3_access_secret, s3_endpoint, s3_storage_class, s3_key_prefix, gcs_bucket, gcs_key_prefix, gcs_storage_class, - gcs_credentials_file, gcs_automatic_credentials): + gcs_credentials_file, gcs_automatic_credentials, s3_upload_part_size): fs_config = {'provider':0} if fs_provider == 'S3': s3config = {'bucket':s3_bucket, 'region':s3_region, 'access_key':s3_access_key, 'access_secret': s3_access_secret, 'endpoint':s3_endpoint, 'storage_class':s3_storage_class, 'key_prefix': - s3_key_prefix} + s3_key_prefix, 'upload_part_size':s3_upload_part_size} fs_config.update({'provider':1, 's3config':s3config}) elif fs_provider == 'GCS': gcsconfig = {'bucket':gcs_bucket, 'key_prefix':gcs_key_prefix, 'storage_class':gcs_storage_class} @@ -238,13 +238,14 @@ class SFTPGoApiRequests: subdirs_permissions=[], allowed_ip=[], denied_ip=[], fs_provider='local', s3_bucket='', s3_region='', s3_access_key='', s3_access_secret='', s3_endpoint='', s3_storage_class='', s3_key_prefix='', gcs_bucket='', gcs_key_prefix='', gcs_storage_class='', gcs_credentials_file='', gcs_automatic_credentials='automatic', - denied_login_methods=[], virtual_folders=[], denied_extensions=[], allowed_extensions=[]): + denied_login_methods=[], virtual_folders=[], denied_extensions=[], allowed_extensions=[], + s3_upload_part_size=0): u = self.buildUserObject(0, username, password, public_keys, home_dir, uid, gid, max_sessions, quota_size, quota_files, self.buildPermissions(perms, subdirs_permissions), upload_bandwidth, download_bandwidth, status, expiration_date, allowed_ip, denied_ip, fs_provider, s3_bucket, s3_region, s3_access_key, s3_access_secret, s3_endpoint, s3_storage_class, s3_key_prefix, gcs_bucket, gcs_key_prefix, gcs_storage_class, gcs_credentials_file, gcs_automatic_credentials, denied_login_methods, virtual_folders, denied_extensions, - allowed_extensions) + allowed_extensions, s3_upload_part_size) r = requests.post(self.userPath, json=u, auth=self.auth, verify=self.verify) self.printResponse(r) @@ -254,13 +255,13 @@ class SFTPGoApiRequests: s3_bucket='', s3_region='', s3_access_key='', s3_access_secret='', s3_endpoint='', s3_storage_class='', s3_key_prefix='', gcs_bucket='', gcs_key_prefix='', gcs_storage_class='', gcs_credentials_file='', gcs_automatic_credentials='automatic', denied_login_methods=[], virtual_folders=[], denied_extensions=[], - allowed_extensions=[]): + allowed_extensions=[], s3_upload_part_size=0): u = self.buildUserObject(user_id, username, password, public_keys, home_dir, uid, gid, max_sessions, quota_size, quota_files, self.buildPermissions(perms, subdirs_permissions), upload_bandwidth, download_bandwidth, status, expiration_date, allowed_ip, denied_ip, fs_provider, s3_bucket, s3_region, s3_access_key, s3_access_secret, s3_endpoint, s3_storage_class, s3_key_prefix, gcs_bucket, gcs_key_prefix, gcs_storage_class, gcs_credentials_file, gcs_automatic_credentials, denied_login_methods, virtual_folders, denied_extensions, - allowed_extensions) + allowed_extensions, s3_upload_part_size) r = requests.put(urlparse.urljoin(self.userPath, 'user/' + str(user_id)), json=u, auth=self.auth, verify=self.verify) self.printResponse(r) @@ -537,6 +538,8 @@ def addCommonUserArguments(parser): parser.add_argument('--s3-access-secret', type=str, default='', help='Default: %(default)s') parser.add_argument('--s3-endpoint', type=str, default='', help='Default: %(default)s') parser.add_argument('--s3-storage-class', type=str, default='', help='Default: %(default)s') + parser.add_argument('--s3-upload-part-size', type=int, default=0, help='The buffer size for multipart uploads (MB). ' + + 'Zero means the default (5 MB). Minimum is 5. Default: %(default)s') parser.add_argument('--gcs-bucket', type=str, default='', help='Default: %(default)s') parser.add_argument('--gcs-key-prefix', type=str, default='', help='Virtual root directory. If non empty only this ' + 'directory and its contents will be available. Cannot start with "/". For example "folder/subfolder/".' + @@ -656,7 +659,8 @@ if __name__ == '__main__': args.denied_ip, args.fs, args.s3_bucket, args.s3_region, args.s3_access_key, args.s3_access_secret, args.s3_endpoint, args.s3_storage_class, args.s3_key_prefix, args.gcs_bucket, args.gcs_key_prefix, args.gcs_storage_class, args.gcs_credentials_file, args.gcs_automatic_credentials, - args.denied_login_methods, args.virtual_folders, args.denied_extensions, args.allowed_extensions) + args.denied_login_methods, args.virtual_folders, args.denied_extensions, args.allowed_extensions, + args.s3_upload_part_size) elif args.command == 'update-user': api.updateUser(args.id, args.username, args.password, args.public_keys, args.home_dir, args.uid, args.gid, args.max_sessions, args.quota_size, args.quota_files, args.permissions, args.upload_bandwidth, @@ -665,7 +669,7 @@ if __name__ == '__main__': args.s3_access_key, args.s3_access_secret, args.s3_endpoint, args.s3_storage_class, args.s3_key_prefix, args.gcs_bucket, args.gcs_key_prefix, args.gcs_storage_class, args.gcs_credentials_file, args.gcs_automatic_credentials, args.denied_login_methods, - args.virtual_folders, args.denied_extensions, args.allowed_extensions) + args.virtual_folders, args.denied_extensions, args.allowed_extensions, args.s3_upload_part_size) elif args.command == 'delete-user': api.deleteUser(args.id) elif args.command == 'get-users': diff --git a/templates/user.html b/templates/user.html index ec62f8b5..6ff7fc46 100644 --- a/templates/user.html +++ b/templates/user.html @@ -301,10 +301,13 @@
- +
- + + + The buffer size for multipart uploads. Zero means the default (5 MB). Minimum is 5 +
diff --git a/vfs/s3fs.go b/vfs/s3fs.go index a26189c5..4fe8859c 100644 --- a/vfs/s3fs.go +++ b/vfs/s3fs.go @@ -36,7 +36,13 @@ type S3FsConfig struct { AccessSecret string `json:"access_secret,omitempty"` Endpoint string `json:"endpoint,omitempty"` StorageClass string `json:"storage_class,omitempty"` - PartSize int64 `json:"partsize,omitempty"` + // The buffer size (in MB) to use for multipart uploads. The minimum allowed part size is 5MB, + // and if this value is set to zero, the default value (5MB) for the AWS SDK will be used. + // The minimum allowed value is 5. + // Please note that if the upload bandwidth between the SFTP client and SFTPGo is greater than the + // upload bandwidth between SFTPGo and S3 then the SFTP client will idle wait for the uploads of + // the last parts, and it could timeout. Keep this in mind if you customize these parameters. + UploadPartSize int64 `json:"upload_part_size,omitempty"` } // S3Fs is a Fs implementation for Amazon S3 compatible object storage. @@ -82,10 +88,10 @@ func NewS3Fs(connectionID, localTempDir string, config S3FsConfig) (Fs, error) { awsConfig.S3ForcePathStyle = aws.Bool(true) } - if fs.config.PartSize == 0 { - fs.config.PartSize = s3manager.DefaultUploadPartSize + if fs.config.UploadPartSize == 0 { + fs.config.UploadPartSize = s3manager.DefaultUploadPartSize } else { - fs.config.PartSize *= 1024 * 1024 + fs.config.UploadPartSize *= 1024 * 1024 } sessOpts := session.Options{ @@ -208,10 +214,10 @@ func (fs S3Fs) Create(name string, flag int) (*os.File, *pipeat.PipeWriterAt, fu StorageClass: utils.NilIfEmpty(fs.config.StorageClass), }, func(u *s3manager.Uploader) { u.Concurrency = 2 - u.PartSize = fs.config.PartSize + u.PartSize = fs.config.UploadPartSize }) r.CloseWithError(err) - fsLog(fs, logger.LevelDebug, "upload completed, path: %#v, response: %v, readed bytes: %v, err: %v", + fsLog(fs, logger.LevelDebug, "upload completed, path: %#v, response: %v, readed bytes: %v, err: %+v", name, response, r.GetReadedBytes(), err) metrics.S3TransferCompleted(r.GetReadedBytes(), 0, err) }() diff --git a/vfs/vfs.go b/vfs/vfs.go index 031e246b..759216a4 100644 --- a/vfs/vfs.go +++ b/vfs/vfs.go @@ -103,8 +103,8 @@ func ValidateS3FsConfig(config *S3FsConfig) error { config.KeyPrefix += "/" } } - if config.PartSize != 0 && config.PartSize < 5 { - return errors.New("part_size ret cannot be lower than 5MB") + if config.UploadPartSize != 0 && config.UploadPartSize < 5 { + return errors.New("upload_part_size cannot be != 0 and lower than 5 (MB)") } return nil }