caddyfile: Add support for env var defaults, tests

[francislavoie]

Closes #2313

This adds support for an optional default in Caddyfile environment variable placeholders with a : separator. This is specifically for the ones like {$FOO}, not the ones like {env.FOO} which are replaced at runtime.

For example:

{$DOMAIN:localhost} {
	respond "Hello world!"
}

If the DOMAIN environment variable exists, the site label will be set to the value of that environment variable, but if not set, will be set to localhost.

FYI @mholt I also noticed while working on this that we were doing os.GetEnv and feeding that into os.ExpandEnv, which actually means that the env var will be replaced twice in chain. I retained this behaviour for now and added a test for it, but it feels like that wasn't intentional. It would technically be a minor breaking change to fix it so I want to wait for your input.

[mholt]

I guess I'm alright with this. Thanks!

Is the bash-ism :- just to keep things easier to remember so you don't have to learn a second syntax? (Because we're def. not bash)

As for the redundant ExpandEnv + Getenv, why is fixing it a breaking change?

Note the reviewdog comment

[francislavoie]

As for the redundant ExpandEnv + Getenv, why is fixing it a breaking change?

Someone could be specifically using that behaviour 🤷‍♂️

But if you're okay with fixing that, I can.

Is the bash-ism :- just to keep things easier to remember so you don't have to learn a second syntax? (Because we're def. not bash)

I guess. What alternative syntax would you suggest?

[mholt]

I am more familiar with || from several programming languages; JS actually uses that literally as a "otherwise" operator in some cases.

I'm not saying that's what we should do, it's just more intuitive to me personally, and it looks better.

[francislavoie]

How about ??, as in the commonly used null coalescing operator? I think it's more technically correct because we only apply the default if the env doesn't exists, not if it's "falsey".

[mholt]

I'd be fine with that too. Honestly I guess I don't care if it's :- or ??. Should we put it to a vote? :P

[francislavoie]

Alright decided to go ahead with ?? for now. Reached out on Slack to get some feedback (nothing yet) but I think we can probably be more comfortable with that.

I also removed the "chaining" behaviour since I really doubt anyone actually relies on it, frankly. I just wanted to be safe by preserving it but it should be fine.

We'll need to remember to note that whitespace around the ?? is not stripped so we'll need to make sure to document that. Or maybe we should trim either side of it? What do you think? What if someone wants leading spaces in their default value? 🤔

Also we have #3807 in the pipeline which uses ? for header defaults so I think ?? is likely natural to use here.

Anyways, according to https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10_02 it seems that a regexp for env vars could look like: [a-zA-Z_]+[a-zA-Z0-9_]* so maybe we could just make it ? instead of ?? because why not, but 🤷‍♂️ I don't think there's a whole lot of precedence for that.

[mholt]

I think ?? is fine.

Also we have #3807 in the pipeline which uses ? for header defaults so I think ?? is likely natural to use here.

lol, I also just reopened this thread with the intent of linking to that issue. My OCD is bothered that one uses ? and the other uses ?? but I'm not against it... still, might be worth asking... can we get them to be the same (?) -- would that make sense? Is it worth it? (You asked the same question, and now I'm asking it... hmm... a sign?)

[mholt]

Now I have another idea. Doesn't something like {$FOO:bar} read a little more naturally/easier to you? Using : as the default operator in this sense. I feel like it reads easier, and reminds me more of ternary statements in a lot of languages, i.e. A ? B : C where : C indicates the default or fallback value.

Once I settle my mind on the syntax I'm good with merging this.

[francislavoie]

Sure, that's fine to me. Just FYI, shells use :- because it can also do other things followed by :, see https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

[mholt]

K I've made up my mind, let's use : 😄 I'm done now, I promise we can merge this then.

[mholt]

Oh you already implemented it. K I guess we're good then.