IBX-12173: Added Varnish 9 VCL - #84
Conversation
konradoboza
left a comment
konradoboza
left a comment
There was a problem hiding this comment.
Thank you @vidarl! Side note: since 5.0 we most likely need that in https://github.com/ibexa/cloud/blob/5.0/resources/upsun/common/5.0/.platform/varnish.vcl too.
bnowak
left a comment
bnowak
left a comment
There was a problem hiding this comment.
Approved to ublock topic, based on it's 1:1 copy between versions.
I don't know vcl much, so if you want to have more confidence, please wait for some more reviewers 😉
@konradoboza :PR created : ibexa/cloud#12 |
83221ba to
4e98423
Compare
|
Sonar Cloud complains about Github actions can be ignored, ref discussion on |
|
I am re-requesting reviews on this one, as I have added two more commits. |
barw4
left a comment
barw4
left a comment
There was a problem hiding this comment.
+1 but aren't we missing some yml config as CI shows?
|
| set req.backend_hint = ezplatform; | ||
|
|
||
| // Add a Surrogate-Capability header to announce ESI support. | ||
| set req.http.Surrogate-Capability = "abc=ESI/1.0"; |
There was a problem hiding this comment.
Can this be e.g. ibx instead of abc?
| sub vcl_recv { | ||
|
|
||
| // Set the backend | ||
| set req.backend_hint = ezplatform; |
There was a problem hiding this comment.
I'm assuming ezplatform is going to change in either 5.0 or 6.0 merge up?
| set req.url = std.querysort(req.url); | ||
|
|
||
| // Retrieve client user context hash and add it to the forwarded request. | ||
| call ez_user_context_hash; |
There was a problem hiding this comment.
Q: did we change this in 5.0 or 6.0? If so, then worth remembering on a merge up.
| // Handle purge | ||
| // You may add FOSHttpCacheBundle tagging rules | ||
| // See http://foshttpcache.readthedocs.org/en/latest/varnish-configuration.html#id4 | ||
| sub ez_purge { |
There was a problem hiding this comment.
Naming: Can this be ibx_purge instead? I'm fine with new code referencing the new namespace identifiers despite inconsistency. Distinguishes new vs legacy solution.
5 participants
Caution
DO-NOT-MERGE : PR contains TMP commit with dependencies.json for making CI green
Caution
This PR cannot be merged up as-is as vcl files are different in here vs 5.0. Code for 5.0 is prepared in separate branch:
IBX-12173_support_for_varnish9_50Caution
I based this on IBX-12244-varnish7-vcl-hit for the time being as it is dependent on #86 . However, this one should be merged into 4.6 after #86, not merged into IBX-12244-varnish7-vcl-hit.
Related PRs:
Description:
Adds
docs/varnish/vcl/varnish9.vclfor Varnish 9. VCL 4.1 syntax is unchanged between Varnish 7 and 9, so this file isidentical to
varnish7.vclapart from the header comment.Note:
The SonarCloud warnings are false positives, claiming full commit SHA hash should be used for GitHub actions. But they refer to ibexa repos and can be considered safe
For QA:
Documentation: