diff --git a/i18n/en.json b/i18n/en.json index b475c409b0..6a15967ce6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -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}}" diff --git a/services/filesstore/s3store.go b/services/filesstore/s3store.go index 7750dd4a8b..465973adfe 100644 --- a/services/filesstore/s3store.go +++ b/services/filesstore/s3store.go @@ -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 }