Extend S3 configuration for other S3 API compatible services

Feature
21

Why not add force_path_style as a separate option?

22

Yes, I already did that!
You can view the PR here.

23

Thank You!

Any plans on adding nginx template to docker_discourse repo?
I think its common thing for minio/s3/spaces storage - caching is absolutely needed.

24

No problem, I will need to learn more about nginx caching before adding anything but I will look into it.

This :+1:

1 Like
25

Yes, and any form of nginx caching is a PR to another repo, so let’s get this done first.

4 Likes
26

Shouldn’t this setting be a checkbox (boolean)?

So this:

s3_force_path_style:
     default: 'false'
     regex: '^(true|false)$'

becomes:

s3_force_path_style:
    default: false
1 Like
27

Yes, absolutely.
I’ve updated and tested the PR :+1:

Whenever this is merged, may I write guides on Meta for setting up Spaces/Minio?
(Similar to Setting up file and image uploads to S3)

6 Likes
28

Will there be a migrate script to and fro discourse—spaces?

29

I just realised enable_s3_uploads and enable_s3_backups go through very different code paths. I initially assumed that all s3 objects are uploaded the same way. (Sorry, I’m super new here)

The PR currently breaks enable_s3_uploads because the final url is hardcoded for AWS uploads here:

In site_setting.rb:

def self.absolute_base_url
      bucket = SiteSetting.enable_s3_uploads ? Discourse.store.s3_bucket_name : GlobalSetting.s3_bucket_name

      # cf. http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region
      if SiteSetting.Upload.s3_region == "us-east-1"
        "//#{bucket}.s3.amazonaws.com"
      elsif SiteSetting.Upload.s3_region == 'cn-north-1'
        "//#{bucket}.s3.cn-north-1.amazonaws.com.cn"
      else
        "//#{bucket}.s3-#{SiteSetting.Upload.s3_region}.amazonaws.com"
      end
    end

Note: I’m working on it, do not merge.

1 Like
30

Wouldn’t the existing rake task work? The new code just point to a new storage location.

31

No because those tasks in their current state are made to work specifically with AWS with hardcoded parameters!

1 Like
32

Yeah, I’ll have to be very careful when changing the absolute_base_url return values because it’s being used all over the codebase (almost 40 usages) and I don’t want the change to break anything else :smiley:

1 Like
33

Or you can perhaps copy that task and create a separate task to handle DO?

34

I’ve kept the older AWS region logic as it is (didn’t want to refactor unless there was a need to) and just added support for a custom endpoint if the default endpoint isn’t used:

def self.absolute_base_url
      url_basename = SiteSetting.s3_endpoint.split('/')[-1]
      bucket = SiteSetting.enable_s3_uploads ? Discourse.store.s3_bucket_name : GlobalSetting.s3_bucket_name

      # cf. http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region
      if SiteSetting.s3_endpoint == "https://s3.amazonaws.com"
        if SiteSetting.Upload.s3_region == "us-east-1"
          "//#{bucket}.s3.amazonaws.com"
        elsif SiteSetting.Upload.s3_region == 'cn-north-1'
          "//#{bucket}.s3.cn-north-1.amazonaws.com.cn"
        else
          "//#{bucket}.s3-#{SiteSetting.Upload.s3_region}.amazonaws.com"
        end
      else
        "//#{bucket}.#{url_basename}"
      end
    end

Update:
I’m happy to report that uploads and backups are working 100% as expected for DigitalOcean Spaces with the PR. :tada: (image uploads, bucket creation, backup uploads and deletion :+1:) The code will work for platforms that are fully S3-compatible.

Note: uploads and backups work well for Minio too, it’s just that the images don’t show up on Discourse because Minio routes their object urls differently. It’s a minio configuration issue and I’m working on it but does the code look okay till now? @Falco

11 Likes
35

Hey, is the PR in effect so we can use digital ocean spaces?

36

Not yet.

We have hundreds of people using S3 currently, so we must be very careful here so we don’t break current setups.

7 Likes
37

Update: Minio now works perfectly for uploads/backups :tada:

I can refactor the absolute_base_url method in various ways but I’ll wait till I get a review from the team (because we have to be careful with code/logic changes).

Let me know if I can do anything to help :+1:

3 Likes
38

the PR is merged.

@rishabh can you confirm if it works with uploads?

I have a question. If we enable this feature, what would happen to previous uploads? Those that are on our own disk. Do they automatically transfer? On the other hand, if some day we might want to move back the uploads to our own server and turn off this feature, How can we migrate data back?

39

Yes, it works with uploads! :+1:

No, previous uploads would be not be affected. They don’t transfer automatically and only the future uploads will be uploaded to the new provider when you switch. I will check if it’s possible/feasible to migrate from one method to another.

1 Like
40

awesome, so how do we setup digitalocean spaces any tutorials out there?