search

wodby / varnish

Varnish docker container image
https://wodby.com/stacks/varnish
MIT License
59 stars 29 forks source link

VARNISH_KEEP_ALL_COOKIES does not work #16

Closed urbangrz closed 5 years ago

urbangrz commented 5 years ago

We have a problem with VARNISH_KEEP_ALL_COOKIES and wordpress preset. We set VARNISH_KEEP_ALL_COOKIES to 1 but none of there cookies except ones compliant with VARNISH_WP_PRESERVED_COOKIES are accessible in php. We also tried a pure php page outside of wordpress code and still getting the save results, no cookies are preserved.

urbangrz commented 5 years ago

Some more info varnish implementation: varnish:4.1-4.1.2. Wodby infrastructure version: 5.5.3 Wordpress stack version: wordpress-php:7.2-4.11.0

csandanov commented 5 years ago

Our README for WordPress preset states clearly:

VARNISH_WP_PRESERVED_COOKIES: Not affected by $VARNISH_KEEP_ALL_COOKIES

and

Cookies ($VARNISH_STRIP_COOKIES) stripped unless $VARNISH_KEEP_ALL_COOKIES is set

and

VARNISH_STRIP_COOKIES Ignored if $VARNISH_KEEP_ALL_COOKIES is set

Also, the search by $VARNISH_KEEP_ALL_COOKIES in the repository will show you that it clearly has nothing to do with WordPress preset:

https://github.com/wodby/varnish/blob/948d487571f8c1d24df7f8432b4676eaf79faa7f/templates/defaults/vcl_recv.vcl.tmpl#L29-L42

kuveljicdev commented 5 years ago

Chingis we saw this stuff and it appears that you guys decided to remove from PHP all cookies except the ones that are preserved in Varnish.

Even dough we set the varnish keep all cookies the cookies are not in PHP except the varnish preserved ones but than the page is not cached which is not an ideal approach. What if like in our case u need a cookie to be reached in PHP but not to affect on the cache, we were unable to configure that with the available configs.

Is it possible at all to achive that now with the provided params and avoid huge costs?

kuveljicdev commented 5 years ago

@csandanov what about this part here

https://github.com/wodby/varnish/blob/8a4e91d940c61175d8373a16340c913278437ec5/templates/presets/wordpress.vcl.tmpl#L17-L49

If there is a cookie here and it's not in the preserved ones it would be stripped and then missing in PHP. Here the VARNISH_KEEP_ALL_COOKIES is not even mentioned....

csandanov commented 5 years ago

If it does not find the cookies from the preserved it would unset them and they would not be in PHP, sounds like a bug to me.

What makes you think it's a bug? WordPress preset description states:

If a cookie from $VARNISH_WP_PRESERVED_COOKIES is set a page will not be cached. All other cookies stripped

VARNISH_WP_PRESERVED_COOKIES:

Not affected by $VARNISH_KEEP_ALL_COOKIES

Also see https://varnish-cache.org/docs/3.0/tutorial/cookies.html:

Also, if the client sends a Cookie header, Varnish will bypass the cache and go directly to the backend.

kuveljicdev commented 5 years ago

@csandanov obviously you don’t have a full coverage of cases and that’s why you are asking why do we consider this a bug, if you change something that has a negative impact without a backwards compatibility than it must be a buggy approach (clean coding principle) -the way it was before it worked for us perfectly. If a cookie in VARNISH_WP_PRESERVED_COOKIES is set the page would not be cache and all others would be stripped, what is the purpose of VARNISH_KEEP_ALL_COOKIES than? Beside that how do we affect now on the negative rule changes since you don’t have a roll back to the previous stack. It would be much easier if this config would be working VARNISHD_VCL_SCRIPT but it’s not, why not (you told us to use the configs but this one does not work on the prod server)?

csandanov commented 5 years ago

if you change something that has a negative impact without a backwards compatibility than it must be a buggy approach -the way it was before it worked for us perfectly.

it's not an argument, we've made many changes without backward compatibility in our images and reflected in our changelog. It may be worked for you perfectly but it did not for others. Which is the main reason why we changed the caching logic in the first place, we now use the same approach for WP as we've been using in Drupal preset for years.

If you need to preserve additional WP cookies you can always override $VARNISH_WP_PRESERVED_COOKIES.

what is the purpose of VARNISH_KEEP_ALL_COOKIES than?

I've already replied here above, also simple search in README will help you.

It would be much easier if this config would be working VARNISHD_VCL_SCRIPT but it’s not

Why not? We do not limit usage of any environment variables. Or do you refer to the managed stack on Wodby platform? Please note it's a public repository of the docker image that people use outside of Wodby.

you told us to use the configs but this one does not work on the prod server

This is not the support channel for your application.

--

I'm going to lock this issue since I couldn't recognize any valid bug reports for this image.