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

the PR is merged.

@rishabhn 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?


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


(@SenpaiMass) #40

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