search

mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
127 stars 41 forks source link

Remove `Accept-Language` from CDN cache policy configuration #1344

Open diox opened 3 years ago

diox commented 3 years ago

We've hardcoded the Accept-Language header in the CDN request cache policy, so it always ends up in the cache key, but I don't think that's correct. Instead we want to make it part of the origin cache policy (so that it's forwarded to requests going to the origin), and let pages that depend on it indicate it through Vary, because most won't need to do that at all. Then it will only end up being part of the cache key when needed.

We need to double-check addons-frontend and addons-server do the right thing and add it to Vary in their responses when they use it, though, and of course test that changing the configuration works fine on dev and that we don't end up caching things with the wrong language.

┆Issue is synchronized with this Jira Task

diox commented 3 years ago

We need to double-check addons-frontend and addons-server do the right thing and add it to Vary in their responses when they use it, though, and of course test that changing the configuration works fine on dev and that we don't end up caching things with the wrong language.

Both set Vary to include Accept-Language on pages that depend on it (usually redirects). In addons-server we even set it unconditionally in API responses because of https://github.com/mozilla/addons/issues/8121.

I did notice an issue with it on addons-frontend: we're only varying on Accept-Language if it was present in the request, which is probably wrong. We need to depend on it as soon as the response for a specific URL potentially depends on it, even if it's not used for that particular request. Otherwise you could end up caching a redirection to en-US (because the header is absent) that would later be used by upstream caches for requests containing Accept-Language: fr.

diox commented 3 years ago

I changed addons-frontend to return Vary: Accept-Language even when it wasn't present in the request, but that doesn't seem to be enough. If I proceed as planned and remove Accept-Language from the cache key, and set it in the origin request policy like I wanted to, invalidate everything and make some requests, sometimes get the wrong cached redirection. The origin seems fine (I can see the Vary header being set if I bypass Cloudfront) but somehow CloudFront doesn't care. It's possible it's an issue with that header specifically. I reverted the policy changes, we might have to live with that.

diox commented 1 month ago

We'll revisit this once we're on a new CDN.