Fixes #497 by adding support for an optional new env variable, S3_BUCKET_URL_BASE

[francisli]

So, with these changes, you can support us-east-1 by just manually setting the S3_BUCKET_URL_BASE to https://s3.amazonaws.com/. Or, if you've got a CNAME mapped to your bucket, you can specify that instead.

Before your pull request is reviewed and merged, make sure you

• there are no linting errors -- npm run lint
• code is in uniquely-named feature branch, and has been rebased on top of latest master. If you're asked to make more changes make sure you rebase onto master then too!
• pull request is descriptively named and links to an issue number, i.e. Fixes #123

Thank you!

[catarak]

thanks for this! can you add a a description of this variable to the README?

[francisli]

Sure thing... added a new section to the README and pushed it up...

[catarak]

thanks!

[simonpfish]

I see that when you set the s3bucket variable you use S3_BUCKET_URL_BASE as the full URL, which should include the bucket name in it:

const s3Bucket = process.env.S3_BUCKET_URL_BASE ||
                 `https://s3-${process.env.AWS_REGION}.amazonaws.com/${process.env.S3_BUCKET}/`;

So maybe we should rename it to S3_BUCKET_URL and update the README to be clear about this, or fix the code so that it uses it only as the base url. What do you think? I can create a new PR for fixing this.

[dhoizner]

This just bit me in the form of a few hours wrestling with what i thought was AWS being stubborn about CORS... @catarak are you hosting with buckets that are using this variable, or do you think it's safe enough to change it to actually be a base rather than the full url?

[francisli]

@dhoizner Sorry to see this conversation so long after it was started (somehow Github notifications aren't always reaching me)...

I think @simonpfish is correct that better naming and documentation of the variable is needed. However, updating the code to always append it with the S3_BUCKET would break its use with custom CNAME mapped buckets.

[dhoizner]

@francisli ah, good call! For now I added a bit to the README in #1475

[catarak]

This just bit me in the form of a few hours wrestling with what i thought was AWS being stubborn about CORS...

Sorry the documentation wasn't clear!! Thank you for updating it.