ostark / upper

Integrates Edge Caches like Fastly, KeyCDN, Cloudflare and Varnish with Craft.
MIT License
102 stars 22 forks source link

adds support for varnish_ban_key #29

Open larsboldt opened 5 years ago

larsboldt commented 5 years ago

VARNISH_BAN_KEY lets you specify a unique id which you can pick up in your VCL and issue the correct bans. Very useful when you have multiple sites hosted on the same backends and you want to control what to ban.

ostark commented 5 years ago

I thought this is solved by the keyprefix already, isn't it?

larsboldt commented 5 years ago

Key Prefix prevents key collisions when you have multisites in CraftCMS or more CraftCMS installations on same varnish server.

Ban key lets you know which domains to ban (based on your own logic in regards to the ban key) when you want to clear the entire cache for a certain domain.

So they are different things, but both needed in a multisite setup.

larsboldt commented 5 years ago

The BAN is issued when using ./craft clear-caches/upper-purge-all at which point the key prefix is not in the picture. All it does now is issue a ban request to varnish, but you have no way of knowing what to ban, that is what the ban key lets you determine.

larsboldt commented 5 years ago

any chance this is getting in soon, need it in production asap? if not I'll just redirect to my fork

ostark commented 5 years ago

@larsboldt Depending on your vcl it should not hurt to include the Varnish_Ban_Key to any request, right?

// config
// ...
'varnish'    => [
            'tagHeaderName'   => 'XKEY',
            'purgeHeaderName' => 'XKEY-PURGE',
            'purgeUrl'        => getenv('VARNISH_URL') ?: 'http://127.0.0.1:80/',
            'headers'         =>['Host' => getenv('VARNISH_HOST'), 'Varnish_Ban_Key' =>getenv('VARNISH_BAN_KEY')] : []
        ],
larsboldt commented 5 years ago

@ostark I guess not, but it makes only sense when issuing the ban?

larsboldt commented 5 years ago

Would love to get this merged so I could stick to the official instead of a fork - what do you think?

timkelty commented 3 years ago

@larsboldt Seems like this should be a config var, not an explicit env check.