Extend S3 configuration for other S3 API compatible services


(Oleg Bovykin) #21

Why not add force_path_style as a separate option?


(Rishabh Nambiar) #22

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


(Oleg Bovykin) #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.


(Rishabh Nambiar) #24

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

This :+1:


(Rafael dos Santos Silva) #25

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


(Rafael dos Santos Silva) #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

(Rishabh Nambiar) #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)


(@SenpaiMass) #28

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


(Rishabh Nambiar) #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.


#30

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


(Bhanu Sharma) #31

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


(Rishabh Nambiar) #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:


(Bhanu Sharma) #33

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


(Rishabh Nambiar) #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


(@SenpaiMass) #35

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


(Rafael dos Santos Silva) #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.


(Rishabh Nambiar) #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:


(hosna) #38

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?