Allow custom config for all cloudfront inputs

[stan-sack]

Background

• A lot of people are asking for this feature
• Ability to configure all cloudfront inputs #270
• feat(component): ability to forward http headers from cloudfront to l… #240
• Handle Cloudfront forwarded headers #198
• want to change _next/* and static/* ttl #375
• How do I cache dynamic pages? #373

Changes

• Allow all options to be passed for @serverless/aws-cloudfront
• Add documentation

Tests

• Added unit tests

[stan-sack]

Your existing markdown is also failing codacy but it looks like it was committed before it was set up:
.

I think you'll need to sort that one out separately to this PR.

[barrysteyn]

So, they way this component does things, there are 4 cloudfront cache behaviors, and my change would (at least for now) only allow for changing the default lambda. It would leave the other cache behaviors alone. Correct me if I am wrong, but this change allows one to cache on any path for all the lambdas. I think that is quite a departure with what I had in mind. At least for now.

[stan-sack]

That's correct. My change allows you to pass in custom cloudfront behaviour for any path.

That's fair enough. It's totally up to you guys.

For my project at least this feature is a necessity. I need to disable caching for user profiles and also pass some custom headers to see if the user is on mobile for some pages. I think these are both common use cases.

For what it's worth I think that it's a low risk change. It exposes the knobs for the users who need them and leaves the default behaviour the same as before. It doesn't break the API.

[skycremaschinin]

Hi, any idea about when this pr would be merged?

[stan-sack]

@danielcondemarin @barrysteyn can we please have some discussion about this? I think that it's not unreasonable that consumers of this library would want to set different CloudFront configurations for different paths.

A major feature of Next 9 is statically generated pages. I think it's a fair assumption that you would want different cache behaviour for these pages than other dynamic pages such as user profiles, or other feed type pages which are frequently updated.

Supporting the latest version of Next.js is pointless if you don't allow users of your library to take full advantage of the features that these new versions offer.

[barrysteyn]

Hi - sorry this has taken me so long to respond. Ultimately, I am happy with your suggestions. But one thing: Read about how Daniel (in the README) defines the 4 caching strategies? I think any alteration to cloudfront must keep those caching logics sacrosanct.

[skycremaschinin]

@barrysteyn hi, yeah we discussed with Daniel about it.
This PR could change this library from something Interesting to production usable.
just think to SEO engine: the library as is can't manage it, but it is absolutely needed to a production env web site.
what do you think about?

[stan-sack]

I've had quite a big think about this and decided that I disagree with you on the 4 caching strategies. Why should the deployment library I use dictate how my site does caching? It doesn't make any sense.

What happens if my boss walks in and says "we need to cache this page for 3 months because the government only releases data quarterly and thats non negotiable"? I literally cant use this lib.

CloudFront allows you to set up to 25 behaviours so why shouldn't I be able to use up to 25? If there were only 4 logical caching strategies then why wouldn't CloudFront just limit this to 4? Or why would the Cache-Control: max-age directive even exist. I think that 4 strategies probably covers 90% of use cases which makes it a great default behaviour but the customisability has to be there for consumers who need it.

I really appreciate the library and all the work that you're doing, but I disagree on this. Please excuse my frustrations coming through, but its becoming harder to pull upstream each time I try to sync the fork.

[danielcondemarin]

I've had quite a big think about this and decided that I disagree with you on the 4 caching strategies. Why should the deployment library I use dictate how my site does caching? It doesn't make any sense.

What happens if my boss walks in and says "we need to cache this page for 3 months because the government only releases data quarterly and thats non negotiable"? I literally cant use this lib.

CloudFront allows you to set up to 25 behaviours so why shouldn't I be able to use up to 25? If there were only 4 logical caching strategies then why wouldn't CloudFront just limit this to 4? Or why would the Cache-Control: max-age directive even exist. I think that 4 strategies probably covers 90% of use cases which makes it a great default behaviour but the customisability has to be there for consumers who need it.

I really appreciate the library and all the work that you're doing, but I disagree on this. Please excuse my frustrations coming through, but its becoming harder to pull upstream each time I try to sync the fork.

@stan-sack Apologies I haven't had the time to review this PR. I have been head down looking into other features in the project which are as important.

I agree that currently the caching options are not flexible enough. I've added this PR to the top of the todos and hopefully should get to it in the next few days. Thanks for the contrib. and your patience 🙏

[stan-sack]

Thanks for communicating. I completely understand. This isnt urgent.

[danielcondemarin]

@stan-sack Something I've realised is that is possible to set Lambda@Edge origin-response on the default path pattern / cache behaviour. Can we prevent that please? The same way is prevented for origin-request, origin-response shouldn't be allowed to be set by users. The reason for that is that I have plans on using origin-response for upcoming features so can't allow it being overridden.

[stan-sack]

that was more difficult than it should have been. I think its ready to go! You might want to pull the error strings out into their own module but I think its OK how it is.

[danielcondemarin]

Right, that's this PR ready to merge 😄 Took a while but is now in a good place for people to try out.
Since it opens up the API surface quite a bit, I'll publish first as a prerelease so we can gather a bit of feedback before considering stable.

[danielcondemarin]

@stan-sack I've added an example app to the examples/ folder. I've kept it quite simple but if you think is worth adding more cache related settings to the example let me know 👍

[stan-sack]

Great stuff! 🎉

[gehan]

🎉

[gehan]

Although won't actually be able to use viewer-response triggers yet until this PR is merged into aws-cloudfront dependency as it sets includebody=true on all trigers even though that's invalid option to response triggers.

serverless-components/aws-cloudfront#19

[danielcondemarin]

Although won't actually be able to use viewer-response triggers yet until this PR is merged into aws-cloudfront dependency as it sets includebody=true on all trigers even though that's invalid option to response triggers.

serverless-components/aws-cloudfront#19

I'll have a look at the PR and nudge the serverless folks to see if we could get it merged 🙂