muzix / ghost-s3

MIT License
47 stars 60 forks source link

new option 'pathPrefix' for sharing S3 buckets and CloudFront settings #9

Open garu opened 9 years ago

garu commented 9 years ago

Hello! Me again :)

This PR introduces the pathPrefix option. Without this, the plugin creates several "folders" in the root of the bucket, like "2014", "2015", etc. While I don't really share ghost's bucket with the rest of the app, I do share the CloudFront cdn. This means that I want some prefixes to be forwarded by cloudfront to bucket A and ghost's prefixes to bucket B. If I make a rule for paths starting with "2015" it works, but I don't want to have to worry about this when the year changes :)

With this patch, ghost can coexist with the rest of my website, both sharing the CDN. It also looks much nicer and tidier when browsing the bucket!

This patch is actually very punctual and does not break any compatibilities with the current settings. The prefix, of course, defaults to "".

Anyway, I hope it helps!

muzix commented 9 years ago

Hello, thank you for your PRs ;) I just merged 2 of your PRs. This PR, I still don't have a clear understand of your situation. If you already had multiple S3 buckets, e.g. bucket A is used for only Ghost blog, bucket B is used for the rest of the website, since this module only create folders inside bucket A, why do you have to add prefix into bucket A ?

garu commented 9 years ago

Hi @muzix! Thanks for merging the other PRs and for querying about this one. I'm sorry I didn't reply to this sooner, my commenting alert must have slipped through the inbox.

Like you mentioned, the issue I have has nothing to do with S3 itself, but with how S3 interacts with other Amazon solutions, in my case with CloudFront CDN. I use CloudFront to speed up both my website and my Ghost blog, each using its own S3 bucket. The way I have set up CloudFront for my website is like so:

if there is a hit on cdn.mysite.com, look for bucket "website"

When I decided to use the CDN on the blog as well (with your beautiful ghost-s3) I bumped into this problem: because of the rule above, I could not use the CDN with Ghost: whenever someone tried to hit, for example, "cdn.mysite.com/2015/some_image.png", they would go to bucket "website", not to bucket "blog". What could I do?

The obvious solution is to set up another domain for bucket "blog", so that we could make a rule like:

if there is a hit on blog-cdn.mysite.com, look for bucket "blog"

Unfortunately, that would involve a lot of extra setup on CloudFront and probably cost higher, not to mention I'd need another SSL certificate. There has to be a simpler solution, right?

I was reading through CloudFront docs trying to come up with something, and saw that CloudFront itself solves this issue by allowing users to set path prefix rules where you can point to a given bucket depending on the URL. If we could make ghost-s3 either share a bucket with the main app or share the CloudFront CDN setup, the problem would solve itself! With the inclusion of the "pathPrefix" setting, your users (myself included) are able to do exactly this, and in a non-intrusive way.

In my case, I kept using different buckets (because it felt more organized and more secure in terms of S3 authorizations) but used the "blog" pathPrefix and set up CloudFront CDN with two rules instead of one:

if there is a hit on cdn.mysite.com/blog, look for bucket "blog" otherwise, look for bucket "website"

Of course, the same solution could have been reached without this patch, like so:

if there is a hit on cdn.mysite.com/2015, look for bucket "blog" otherwise, look for bucket "website"

But that would mean the bucket website could not have a "2015" key or subkey (because it would never be reachable) and I would have to pay attention to this setting and create a new one every time the year changed. By creating the pathPrefix setting, I can set it up with full control over what key/subkey I'll use for the blog (it still won't be reachable on the "website" bucket, but at least I'll be able to chose it as I wish) and forget about it. As a side-effect, this organization also improves cache invalidation on CloudFront, where I can just say "invalidate 'blog'" instead of "invalidate everything" or "invalidate 2014, now invalidate 2015, now invalidate 2016, now..."

I hope this helps to clear up why I needed this patch and why I think others might also benefit from it. Please let me know if you have any other questions or concerns.

Oh! Also, please note that I am not attached to this code at all. As long as I am able to solve the issue (which this patch is already doing for me and the solution is live for almost a month without a glitch), I don't mind if you change the config name "pathPrefix" or the underlying patch completely. All I want is to be able to run ghost-s3 sharing a CloudFront setup with my website :)

Cheers! And thanks again for taking the time to review this patch, and for all the work you do on ghost-s3!

garu commented 9 years ago

Hello! Any news on this pull request? :)

Please let me know if there is anything else I can do to help get these changes in.

Cheers!

stevelacey commented 8 years ago

+1 exactly what I want too

stevelacey commented 8 years ago

If anyone else wants this ahead of this merge, I just smashed over the prototype in the index.js that needs adding anyway – allows customisation of the date segments too, I just want it consistent with my old storage:

/content/storage/ghost-s3/index.js

'use strict';
var S3Store = require('ghost-s3-storage');

S3Store.prototype.getTargetDir = function() {
    var now = new Date();
    return 'blog/images/' + now.getFullYear() + '/' + (now.getMonth() + 1) + '/';
};

module.exports = S3Store
garu commented 8 years ago

Hey, @muzix! Just wondering about this PR. It's ok if you just don't want to merge it, but it's not that big a change and looks like it solves other people's issues as well :D If there's any reason why you're hesitant, and if I can help in any way, please let me know.

For the record, I have been successfully using this patch in production since I created it over 4 months ago. It even endured an upgrade to the latest version of Ghost.

Thanks again!

csquared commented 8 years ago

Lol I should have looked at this PR before submitting: https://github.com/muzix/ghost-s3/pull/15 . Either way I'd really like to see this functionality as well.

A perfect use case for this is anywhere a company has multiple users in a single bucket (this can make bucket admin convenient). IAM allows you to issue credentials that are scoped to a bucket prefix, so this becomes a convenient way to control folder-level access on a shared bucket.

Would love to see either of the PRs merged -> no preference on the actual config param name and want to avoid bikeshed territory :)

garu commented 8 years ago

Hahahaha no problem, @csquared! Same here, I just want to stop having to maintain a separate branch for this feature.

If @muzix would rather merge your version instead, I'm cool with that too :)

csquared commented 8 years ago

@garu :) I think yours is the winner right now cus of the readme update and has a more idiomatic implementation. I just went with quick and dirty. TBH I'm considering just pointing my changes to yours and shutting down my PR, but for now hoping having the two open will get us a response from @muzix

csquared commented 8 years ago

@garu: went ahead and did that: but think i got bitten by it not using the public-acl

garu commented 8 years ago

@csquared cool! Let me know if we need to patch anything, ok?