Moving away from goamz to use minio-go instead.

[harshavardhana]

Summary

minio-go does fully managed way of handling S3 API requests

• Automatic bucket location management across all s3 regions.
• Transparently upload large files in multipart if file 64MB
  or larger.
• GetObject() API provides compatibility with
  io.ReadWriteSeeker interface.
• Various other APIs including bulk deletes, server-side object
  copy, bucket policies and bucket notifications.

Ticket Link

#4182

Checklist

• Added or updated unit tests (required for all new features)
• Includes text changes and localization files updated
• Touches critical sections of the codebase (auth, upgrade, etc.)

Fixes #4182

[mattermod]

Thanks @harshavardhana for the pull request!

Per the CONTRIBUTING.md file displayed when you created this pull request, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?

This is a standard procedure for many open source projects. Your form should be processed within 24 hours and reviewers for your pull request will be able to proceed.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

[it33]

Sweet! Thanks @harshavardhana!

[harshavardhana]

@it33 - please let me know how this looks, i haven't removed the goamz deps. Seems like it ends up removing some other deps as well, so was not sure what would you like to do.

Let me know. Thanks for #hacktoberfest

[it33]

Thanks @harshavardhana, and thanks for completing the CLA. I've added the "Dev Review" tag for comments.

[enahum]

Hi @harshavardhana I believe your config.json has errors

[harshavardhana]

Hi @harshavardhana I believe your config.json has errors

Yes seems like fixing them.

[harshavardhana]

@enahum @it33 - ready for review.

[jwilander]

Code looks good, nice work @harshavardhana !

Still need a PM to test this.

[harshavardhana]

Still need a PM to test this.

Can you let me know how to do this.. happy to help.

[jwilander]

Haha no worries @harshavardhana, I've poked a PM to come look at this, setup a test server and give it a test. No action needed from you :)

[esethna]

Hi @harshavardhana, thanks for the PR! I've attempted to test this but there seems to be a bug with the "Amazon S3 Secure" field. The field seems to copy whatever value is typed into the "Amazon S3 Region" field and then cannot be edited, causing an error on save (see screenshot).

Other minor notes regarding the "Amazon S3 Secure" field:

1. If the field only handles true/false values can we make it a radio button selector that defualts to "false" (see example screenshot of radio buttons)?

2. Can we change the setting label from "Amazon S3 Secure" > "Enable Insecure Amazon S3 Connections"? This is more consistent with our UX guidelines and other settings in the console.

3. Can we tweak the help text to "When true, allow insecure connections to Amazon S3. Defaults to secure connections only?

[harshavardhana]

Changed @esethna . let me know if the changes look good.

[it33]

Thanks everyone, really great work!

Just my thoughts high level:

1) Replacing the S3 driver with Minio is a great first step

• We need to test on both S3 and Minio to make sure things work.
• We can document the initial functionality as beta/unofficial for Minio to start

2) Add UI & docs for Minio

It may be an option that says "S3 or Minio" or different drop down options so the admin knows which is being used. Need to add docs too, including link on how to setup Minio, etc.

We can take off the "unofficial" tag at this point.

3) We need to add file scenarios to Mattermost Load Test

• Benchmark both S3 and Minio just to have some baselines.

At this point we can take off the beta tag. It might even be okay to take the beta tag off earlier, just if we're planning to do the file load tests anyway, it could be okay to wait a bit.

Somewhere along the way it probably makes sense to do a Hacktoberfest blog post, as this is a pretty awesome PR.

[harshavardhana]

After taking a look i see that the profile image is not loading there seems to be a 500 Internal Server error.

Are there any pointers where i go about looking at this in the code? does this internally use presigned URL etc? or is it a direct GetObject() operation?

[esethna]

Thanks @harshavardhana, maybe @jwilander can help point you in the right direction?

[harshavardhana]

Thanks @harshavardhana, maybe @jwilander can help point you in the right direction?

Fixed it there was bug in how the path was specified during upload v/s how it is read. Looks like previous library modified this internally by cleaning the prefix '/'.

$ git diff api/
diff --git a/api/user.go b/api/user.go
index ae8136d..6811103 100644
--- a/api/user.go
+++ b/api/user.go
@@ -1174,7 +1174,7 @@ func getProfileImage(c *Context, w http.ResponseWriter, r *http.Request) {
                                return
                        }
                } else {
-                       path := "/users/" + id + "/profile.png"
+                       path := "users/" + id + "/profile.png"

                        if data, err := ReadFile(path); err != nil {

[harshavardhana]

1. The "Enable Insecure Amazon S3 Connections" field seemed to be set to true on the fresh spinmint when I enabled S3 for the first time. Can you please verify that the default is set to false so only secure secure connections are allowed.

Changed the behavior to have this option as "Secure" instead of "InSecure" it is the negation of what is common.

2. There seems to be a bug with uploading profile pictures when the "Enable Insecure Amazon S3 Connections" setting is true. Things work as expected when the setting is false. Reproduction steps:

a) Set "Enable Insecure Amazon S3 Connections" to true and Save
b) Switch back to the team from the system console
c) Account settings > General > Profile Picture
d) Select a new image, then attemp to "Save"

This works now with or without SSL. Pushing the new change.

[harshavardhana]

Is there anything waiting on me for this one?

[jasonblais]

Sorry for the delay @harshavardhana, we had some trouble setting up test servers end of last week.

@esethna can you help test when you get the chance, now that the test servers are working again?

[esethna]

+1, thanks for the changes @harshavardhana! Tested image uploads, downloads and profile pictures with the "Secure Amazon S3 Connections" setting toggled on and off.

[mattermod]

Pull Request queued for Stage 2: Developer Review.

[mattermod]

Pull Request queued for Stage 3: Ready to Merge.

[esethna]

@harshavardhana Excited that we got this PR in! Would you be interested in helping add documentation for this change? This would involve updating the File > Storage section of config-settings.rst doc to mention Minio (appended with (Beta) for this release) and also adding the new setting "Secure Amazon S3 Connections".

[harshavardhana]

Yes will do @esthena

[esethna]

Thanks!