vcl: Make 'none' a reserved keyword and use it for probes

[dridi]

:warning: This is technically a VCL BREAKING CHANGE but we should make it. :warning:

This patch series does several things, but the effective goal is to enable this:

backend improbable {
        .host = "legacy.example.com";
        .probe = none;
}

Having backends we can't probe also prevents the use of a default probe, so my suggestion here is to allow none as a special value for the .probe field in backend definitions.

This is a breaking change, but I believe we should treat it as a bug fix and proceed without concern for broken VCL. In exchange, it should of course be one of the headlines in the release notes.

Once none becomes a reserved word, we can imagine solving the recurring confusion around unset values:

• https://github.com/varnishcache/varnish-cache/issues/2846
• https://github.com/varnishcache/varnish-cache/issues/3634
• https://github.com/varnishcache/varnish-cache/issues/3844

We could imagine the two if statements becoming equivalent:

if (req.http.foo) { }
if (req.http.foo != none) { }

In the :warning: ACTUALLY BREAKING :warning: department, some would like to change the behavior of set HEADER = HEADER;:

unset req.http.missing;
set req.http.assigned = req.http.missing;
# expect both headers to be 'none' here

I personally think it's an interesting question that should extend to set HEADER = STRING; for null strings. For comparisons in general, it could apply to any expression that can yield a null pointer. Something to consider for the next revision of VCL.

This pull requests stops after turning none into the keyword it should have become on day one, in hindsight, and then uses this keyword to explicitly circumvent the default probe.

[nigoroll]

So this looks very sensible to me.

Just one question: Is it really the best option to keep NULL for "default probe", or should NULL become "no probe" and instead introduce a magic value for "default probe"?

[dridi]

Just one question: Is it really the best option to keep NULL for "default probe", or should NULL become "no probe" and instead introduce a magic value for "default probe"?

This is what I tried first, but that magic value would live in varnishd and not be considered constant enough to appear in static initializers. Following this path would have broken the API, which I tried to preserve.

[nigoroll]

@dridi bugwash is go when https://github.com/varnishcache/varnish-cache/pull/4309#discussion_r2044014197 is addressed.

[dridi]

Am I understanding the scope correctly?

• make none a reserved keyword
• implement none probes as suggested in https://github.com/varnishcache/varnish-cache/pull/4309#discussion_r2044014197
• leave other none usage as a discussion for later

In other words the current patch series amended with https://github.com/varnishcache/varnish-cache/pull/4309#discussion_r2044014197 in mind.

My goal was to start with this scope in the first place, and move the other talking points to #3844.

[nigoroll]

@dridi your last comment aligns with my understanding.