IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

MM-29033: Modify Test S3 connection to handle path prefix (#15659)

Browse Source 
* MM-29033: Modify Test S3 connection to handle path prefix

When a IAM user is just allowed access to a specific path prefix,
the bucket exists test will fail because that is applicable to the entire bucket.

We modify that check to just list objects under that path prefix and return
on the first non-nil error.

Another consideration was to do a StatObject, but it is only applicable to actual
objects and not at a directory level. Path prefixes are entirely logical and depends
on the underlying implementation. Therefore, ListObjects is a better choice.

https://mattermost.atlassian.net/browse/MM-29033

* Improve logic

* tmp

* Only receive
This commit is contained in:
Agniva De Sarker
2020-10-02 00:10:28 +05:30
committed by GitHub
parent 2403aae0c4
commit e15308e840
2 changed files with 33 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
i18n/en.json
View File

@@ -1409,7 +1409,7 @@
"translation": "Don't have permissions to write to local path specified or other error."
},
{
"id": "api.file.test_connection.s3.bucked_create.app_error",
"id": "api.file.test_connection.s3.bucket_create.app_error",
"translation": "Unable to create bucket."
},
{
@@ -1420,6 +1420,10 @@
"id": "api.file.test_connection.s3.connection.app_error",
"translation": "Bad connection to S3 or minio."
},
{
"id": "api.file.test_connection.s3.list_objects.app_error",
"translation": "Error trying to list objects."
},
{
"id": "api.file.upload_file.incorrect_channelId.app_error",
"translation": "Unable to upload the file. Incorrect channel ID: {{.channelId}}"

35
services/filesstore/s3store.go
View File

@@ -34,6 +34,11 @@ type S3FileBackend struct {
trace bool
}
const (
// This is not exported by minio. See: https://github.com/minio/minio-go/issues/1339
bucketNotFound = "NoSuchBucket"
)
// Similar to s3.New() but allows initialization of signature v2 or signature v4 client.
// If signV2 input is false, function always returns signature v4.
//
@@ -73,20 +78,36 @@ func (b *S3FileBackend) TestConnection() *model.AppError {
return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.connection.app_error", nil, err.Error(), http.StatusInternalServerError)
}
exists, err := s3Clnt.BucketExists(context.Background(), b.bucket)
if err != nil {
return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.bucket_exists.app_error", nil, err.Error(), http.StatusInternalServerError)
exists := true
// If a path prefix is present, we attempt to test the bucket by listing objects under the path
// and just checking the first response. This is because the BucketExists call is only at a bucket level
// and sometimes the user might only be allowed access to the specified path prefix.
if b.pathPrefix != "" {
obj := <-s3Clnt.ListObjects(context.Background(), b.bucket, s3.ListObjectsOptions{Prefix: b.pathPrefix})
if obj.Err != nil {
typedErr := s3.ToErrorResponse(obj.Err)
if typedErr.Code != bucketNotFound {
return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.list_objects.app_error", nil, obj.Err.Error(), http.StatusInternalServerError)
}
exists = false
}
} else {
exists, err = s3Clnt.BucketExists(context.Background(), b.bucket)
if err != nil {
return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.bucket_exists.app_error", nil, err.Error(), http.StatusInternalServerError)
}
}
if !exists {
if exists {
mlog.Debug("Connection to S3 or minio is good. Bucket exists.")
} else {
mlog.Warn("Bucket specified does not exist. Attempting to create...")
err := s3Clnt.MakeBucket(context.Background(), b.bucket, s3.MakeBucketOptions{Region: b.region})
if err != nil {
mlog.Error("Unable to create bucket.")
return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.bucked_create.app_error", nil, err.Error(), http.StatusInternalServerError)
return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.bucket_create.app_error", nil, err.Error(), http.StatusInternalServerError)
}
}
mlog.Debug("Connection to S3 or minio is good. Bucket exists.")
return nil
}