Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to setup cache TTL for the processed images #107

Closed
fabiovasini opened this issue Jun 19, 2019 · 16 comments
Closed

How to setup cache TTL for the processed images #107

fabiovasini opened this issue Jun 19, 2019 · 16 comments

Comments

@fabiovasini
Copy link

I noticed a too short caching TTL for the processed images on CloudFront.
How I can change the TTL?

Also, I noticed that CloudFront is not responding with a Cache-Control header.
How I can set it up?

Thanks,
Fabio

@hayesry
Copy link
Member

hayesry commented Jun 20, 2019

Hey @biosini: here's some references for your questions. The response headers piece looks to be a joint issue between CloudFront and S3, so we will look into the integrations on our end to see if there's anything we can do.
1.) https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/Expiration.html

@fabiovasini
Copy link
Author

Hi @hayesry! So basically in the CloudFormation template I could add the following:

...
"Resources": { 
        "ImageHandlerDistribution": { 
            "Type": "AWS::CloudFront::Distribution", 
            "Properties": {
                "DistributionConfig": {
                    ...
                    "DefaultCacheBehavior": {
                      "AllowedMethods": [
                        "GET",
                        "HEAD"
                      ],
                      "DefaultTTL": 86400,
                      "MaximumTTL": 86400,
                      "MinimumTTL": 86400,
...

I'm a bit confused about the MinimumTTL, should I set it up?
About the response headers for client-side caching, is there anything I can look at to solve this?

Many thanks,
Fabio

@hayesry
Copy link
Member

hayesry commented Jun 20, 2019

@biosini Based on the documentation, I think you can get away with just specifying the DefaultTTL value. The differences MaximumTTL and MinimumTTL aren't made super clear and might step on each other's toes if they're both specified. That's just based on my experience though. Here's what I was reading into: (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-distribution-defaultcachebehavior.html#cfn-cloudfront-distribution-defaultcachebehavior-minttl)

For client-side caching, it appears that this is an ongoing feature request for CloudFront. However, they don't provide ETA's for when features will be implemented. I'll take a look on our end and see if there's anything that can be done to the code to help facilitate this or provide a workaround!

@fabiovasini
Copy link
Author

Sounds good! I'll wait for that. Thanks

@hayesry
Copy link
Member

hayesry commented Jun 26, 2019

Hey @biosini: Not sure if you've found a workaround/solution in the meantime, but I've added the Cache-Control header issue as a feature request to be looked into in a future release of the solution. My guess is that this is something that could be added directly in the Lambda response, so we'll research this a bit more and hopefully have a code-based fix!

@fabiovasini
Copy link
Author

Hi @hayesry! No, I didn't found any solution for the missing Cache-Control header.
But, I investigated better the code of the Lambda function and I discovered the following in the index.js (see the getResponseHeaders function):

const getResponseHeaders = (isErr) => {
    const corsEnabled = (process.env.CORS_ENABLED === "Yes");
    const headers = {
        "Access-Control-Allow-Methods": "GET",
        "Access-Control-Allow-Headers": "Content-Type, Authorization",
        "Access-Control-Allow-Credentials": true,
        "Content-Type": "image"
    }
    if (corsEnabled) {
        headers["Access-Control-Allow-Origin"] = process.env.CORS_ORIGIN;
    }
    if (isErr) {
        headers["Content-Type"] = "application/json"
    }
    return headers;
}
  1. Content-Type has been set to just image, so it's missing the proper MIME description (for example "image/png")
  2. Maybe we could simple add "Cache-Control" in that function set to same value of DefaultTTL and that's it.

What do you think?

@mandys
Copy link

mandys commented Jul 17, 2019

This is an issue for us too. As everytime a request would be sent to cloudfront driving up the bills.
Can we please get this looked into ?

@vlidholt
Copy link

This is a real issue, not just for the bills, but also for the loading times of webpages that include the same images. Can confirm that the original S3 header seems to be ignored.

@mandys
Copy link

mandys commented Jul 23, 2019

We downloaded the source, made the change and redeployed. Now Expires is working for us ( I guess unless they fix it, doing it ourselves was our only option ).

@scadu
Copy link

scadu commented Jul 24, 2019

@shsenior, what do you think about ideas proposed above? It would be cool to pass headers just like before 4.0 👍

@NewToScripting
Copy link

@mandys would you be able to share the code you used? I modified the index.js to include a Cache-Control, but it appears it is still being served from Cloudfront when testing. Here is the change I made -
const headers = { "Access-Control-Allow-Methods": "GET", "Access-Control-Allow-Headers": "Content-Type, Authorization", "Access-Control-Allow-Credentials": true, "Cache-Control": "max-age=2628000", "Content-Type": "image" }

@NewToScripting
Copy link

@mandys, please ignore, the above code change does appear to have worked and the Cache-Control exists and is honored. I was refreshing the same page in chrome which I guess does not honor the cache. Opening the same url in another tab did hit the cache, so it appears that the change above is caching properly.

@brad13x
Copy link

brad13x commented Jan 7, 2020

@NewToScripting where can I update my code in AWS or do i need to rebuild it somehow and upload the new template?

@Lunteer
Copy link

Lunteer commented Jan 9, 2020

Just wanted to add my vote for this feature. It would also be great if we can set it at the setup level like CORS, demo site, etc

@Susan123456789
Copy link

+1

@beomseoklee
Copy link
Member

We have updated our solution, and I believe your issue has been fixed. If you still see the issue with the latest version (v4.2), please feel free to reopen the issue.

You can refer to the recent changes here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests