search

silverstripe / silverstripe-s3

Silverstripe module to store assets in S3 rather than on the local filesystem (SS4/SS5 only)
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

FEATURE: added ability to set a CDN domain for public assets #42

Closed tim-lar closed 2 years ago

tim-lar commented 3 years ago

This PR adds the optional configuration of a CDN to prefix public assets, and I've updated the README to demonstrate how the CDN is to be setup.

I'm using this code in multiple production sites with no issue, it's a very simple implementation.

I'm using a CDN to serve both assets and resources (I've got a task to sync my resources folder in my CI after composer install), hence why I've added the suffix of assets, but this may not reflect everyone's setup.

Any feedback is welcome!

obj63mc commented 3 years ago

Not sure if this should be part of the core package or not. Will leave that up to the actual silverstripe team to decide but yes this is the approach we have recommended to users.

You simply extend the public adapter and then change the method that returns the public url.

samandeggs commented 2 years ago

Is there a simple way (or could there be) via one of the configs (couldn't see this) where it could overwrite the choice of path name of the assets folder utilised on the CDN? The default s3 bucket construction just uses a /public and /protected folders, and there's no simple way to route cdn.com/assets -> public dir as far as I can tell?

Not sure (legitimately not sure, no sass) what the reasoning is to include this by default, rather than allowing for the customisation of this path at the time of setting the CDN url in the .env file. This would have been my preference, or as a configurable variable on the config file.

Simple solution is to extend the function used to exclude or rename the ASSETS_DIR const that's utilised currently to whatever you want, as altering this at a top level is asking for trouble.

Any thoughts/suggestions? Happy to make a PR if the suggestion is a simple overwritable var, or to exclude the ASSETS_DIR from default path construction.

wilr commented 2 years ago

@samandeggs A simple overwritable var sounds like a good option.

obj63mc commented 2 years ago

@samandeggs

You can change the public/protected directories by setting environment variables for their respective prefixes.

Example AWS_PUBLIC_BUCKET_PREFIX="public/site-name/dev" AWS_PROTECTED_BUCKET_PREFIX="protected/site-name/dev"

We use this a lot for qa/prod versions of sites where we use the same bucket for both but different prefixes for dev/qa/prod...

samandeggs commented 2 years ago

@obj63mc oh - I wonder if this will suit me.. I just didn't quite follow the reasoning for forcing the /assets/ onto the cdn path when it can't be assumed to be how it was set up.

I have added a PR which may be superfluous: #56 - and perhaps better handled with an ENV variable, but seemed like a basic way to handle it for those who really need that level of customisation.

Edit: this doesn't suite our current situation, as it's more based around the idea that a single bucket cdn is supporting multiple websites - which is a fair assumption, but not the set up I have to deal with currently. I might be a bit out of my depth, but it does look like the PR could be a useful addition even if rarely utilised.