httpcaddyfile: Make third-party app modules configurable via globals

[francislavoie]

This is an attempt at solving #3634 in the simplest way I can think of.

/cc @abiosoft and @vrongmeal since you both have Caddy apps (exec and git respectively) which could make use of this.

So the idea here is that to make a third-party Caddy app configurable via the Caddyfile, it would be in global options. For example, take this Caddyfile config (taking inspiration from the https://github.com/abiosoft/caddy-exec README):

{
	exec {
		command hugo {
			args "generate" "--destination=/home/user/site/public"
			at startup
			foreground
			timeout 10s
		}
		command echo {
			args "foobar"
			at startup
		}
	}
}

A package would invoke App("exec", parseExec), where parseExec has the signature func parseExec(d *caddyfile.Dispenser, _ interface{}) (interface{}, error), but actually returns a httpcaddyfile.App{AppKey: "exec", Value: json.RawMessage}, where the json.RawMessage is the marshaled app module config struct.

Then, when the Caddyfile adapter nears the end, it'll grab any options that are httpcaddyfile.App, and insert them as-is into the config.

This should do the trick, but I haven't tested it yet. Do you guys think this makes sense as an API?

Tested it with the caddy-exec lib in abiosoft/caddy-exec#8 and it works!

[francislavoie]

I think there might've been interest from @dunglas and @greenpau, so inviting you guys to take a look as well.

[francislavoie]

Another thing to note - nothing here stops users from "configuring" the app more than once in the global options block like this:

{
	exec {
		...
	}
	exec {
		...
	}
}

But due to the nature of how this is set up, i.e. a 1:1 mapping, only the last one would actually be kept. So any config for apps that might involve multiple options (like the command example above) will need to always be under the umbrella of a single global option for any given app.

[francislavoie]

Since all I got were 👍, I guess I'll just take this out of draft 🤷‍♂️

[francislavoie]

Alright, spent a bit of time this afternoon trying to implement global option support in caddy-exec (easiest one to try it with right now), and it works!!!! abiosoft/caddy-exec#8

[mholt]

Nice, I like how simple this.

It's definitely a hack though, the HTTP Caddyfile was not designed for this. 😓

I can see it being useful for features that need to accompany an HTTP app, but I wonder if a new kind of Caddyfile would be better, one where the outer-most block defines the app to use:

http {
    ...
}
exec {
    ...
}

If so, I wonder if that is a better direction to take, instead of this, which kind of abuses global options in spirit.

I'm also worried (slightly, not hugely) of namespace issues between app names and global option names.

What do you think of the multi-app Caddyfile idea?

[francislavoie]

I do like the look of that config idea with apps at the top-level. It would definitely feel better than shoving stuff in global options.

So really there's two ways we could take forwards that idea:

1. We could make a separate Caddyfile-like adapter which uses the httpcaddyfile just for the http app block. This adapter may or may not be standard (could be outside this repo) and need to be added as a plugin. It may also have an adapter name like app-caddyfile and use filenames like AppCaddyfile.

2. We could try to add this concept to the existing Caddyfile adapter (it is called httpcaddyfile but we could refactor things so that the app bits are in the caddyfile module and only call down to httpcaddyfile for the http app) while retaining backwards-compatibility via clever parsing to determine whether it looks like an app-wrapped Caddyfile or not; if it doesn't look app wrapped, parse as usual, as a pure HTTP Caddyfile.

   Note that the http app would still be able to break out of that mold, because it can generate config for logging and tls as well. Other apps probably would not get that kind of special treatment and would need to stay isolated. tls would continue to not be directly configurable, but maybe logging would become separately configurable. TBD.

The pros for the adapter idea is that we don't need to futz with backwards-compat since it would be its own thing. We may need to move some code around in this lib to make it properly work, but that's besides the point. And it would be more easily opt-in for those who need it.

In my opinion, the main con for the adapter idea is accessibility. All of our docs and defaults talk about or use the HTTP Caddyfile, like the systemd config, the docker image, etc. So if a user wants to use this adapter, they would need to override the defaults to get there. That's quite unfriendly to work with and I'm sure users would get confused. That's a layer deeper than they generally want to think.

The main con for enhancing the existing Caddyfile is that it may be quite difficult to implement backwards compatibility safely without bending over backwards. For example, what if exactly exec (or any other app module name) is actually a hostname someone wants to use for their HTTP app? Then the Caddyfile would end up being parsed inaccurately because the BC layer would fail to match the user's assumptions. An idea to solve that would be to use some syntactical prefix for all the app names, like maybe !http and !exec, but I think we agree that's less than ideal.

So, we have some 🤔 to do about this. This will affect how viable it is for third-party devs to make their own apps accessible to use, and whatever decision we go with we'll probably be stuck with. We could keep going with the idea in this PR since it's so simple, but if we do merge it, then yeah, we'll be stuck with it "forever" as soon as it's used any third-party modules.

[whitestrake]

The main con for enhancing the existing Caddyfile is that it may be quite difficult to implement backwards compatibility safely without bending over backwards. For example, what if exactly exec (or any other app module name) is actually a hostname someone wants to use for their HTTP app?

Could this be resolved for >99% of cases with some kind of try-else? Attempt HTTP Caddyfile parsing, and if there's a top-level error returned (not a directive error but a syntax error like unknown directive), try App Caddyfile parsing. If the App Caddyfile parsing also produces some kind of top level syntax error, return the HTTP Caddyfile error, otherwise return the App Caddyfile error.

In effect, check if it actually looks like a HTTP configuration first. If it works (or nearly works), run it like that. If it doesn't (and theoretically an App Caddyfile never would look like, or nearly look like, a HTTP Caddyfile), it might be an App Caddyfile instead.

Throw in an info message that tells the user exactly which way Caddy resolved to parse the Caddyfile, and a switch to let the user override/specify, and we're off to the races.

(This isn't necessarily a vote for that option, just a thought on mitigating this particular con.)

[mholt]

@whitestrake Thanks for your thoughts!

Could this be resolved for >99% of cases with some kind of try-else? Attempt HTTP Caddyfile parsing, and if there's a top-level error returned (not a directive error but a syntax error like unknown directive), try App Caddyfile parsing. If the App Caddyfile parsing also produces some kind of top level syntax error, return the HTTP Caddyfile error, otherwise return the App Caddyfile error.

This feels yucky... and not very robust/reliable. I think the Caddyfile is already too wishy-washy for its syntax/format, so to make it more like "Your Caddyfile can look like this, or this, or this ..." starts to get confusing. We should either cleanly delineate these different valid syntaxes with a different adapter, or find a way to justify shoving it into global options. (The implementation complexity of either of these options is nearly identical.)

[whitestrake]

Understandable. In that case, on its face, it would seem to make more sense to split this out to a separate adapter. Since the user is ostensibly already building with third party modules, it seems it would be feasible for this to be non-standard from a "user friction" standpoint, but if the adapter is non-standard I have to wonder how supported it would be by third party Caddy app developers in the first place.

For that purpose, to encourage adoption/availability, it seems like including this different adapter as standard would make the most sense to avoid this effort going to waste by developers not actually making configuration available to it. But if adoption is a primary concern, obviously being part of the global options is the most universally visible/available...

The way I see it, we've got a spectrum of options ranging from most compartmentalized/least available (a separate, non-standard adapter) to least compartmentalized/most available (global options) with the middle option being a separate, standard adapter. Does that sound accurate?

[mholt]

Yeah, I think I agree with that.

Most users will not be configuring multiple apps I think. If they are, they can easily specify --adapter on the command line (or Content-Type in their API call). It also keeps the global options limited to actual global config/options. I will think more on this, but that's the direction I'm leaning so far. Any preference @francislavoie ?

[francislavoie]

Of the options, I like a separate adapter the least. I don't like that it requires people to change their Docker command or systemd file etc to use it. That's just enough of a barrier that I think it will drive people away from wanting to play more with it. If we make it really easy to use then there's a better chance of it being more useful.

So to me my vote would be: augmented Caddyfile adapter > global options > new standard adapter > new non-standard adapter.

[francislavoie]

Honestly, I think at this point we should just merge this. I do still want to explore the idea of augmenting the Caddyfile adapter to support configuring apps at the top-level rather than in globals, but I don't think there's that much harm having the functionality this PR provides anyways. It's probably not too difficult for external apps to add support for some future API once they've already implemented app unmarshaling at a minimum.

Yes we'll be "stuck with it", but is it really that harmful to have one loop in the code, plus one struct added to the public interface? I think probably not.

[mholt]

Don't worry -- over the weekend I also came to the opinion that this should be accepted.

It makes sense to configure other apps in HTTP Caddyfile global options if the apps are in support of the HTTP server. For example, my dynamic DNS app can be used to ensure that a domain's A/AAAA records always point toward the running HTTP server.

It gets a little muddier when the app has nothing to do with HTTP serving; like the layer4 app or a process supervisor.

But I think there are clearly good use cases for this, so let's go ahead and see how much mileage we get out of this.

[mholt]

Thanks for this, Francis!