caddyserver / caddy

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
https://caddyserver.com
Apache License 2.0
55.48k stars 3.91k forks source link

cmd: fix auto-detetction of .caddyfile extension #6356

Closed mohammed90 closed 1 month ago

mohammed90 commented 1 month ago

Fix #6355

francislavoie commented 1 month ago

I think this existed to deal with https://github.com/caddyserver/caddy/pull/5919#discussion_r1425560197 i.e. Caddyfile.yaml if yaml is loaded as an adapter, should use that instead of the Caddyfile adapter.

mohammed90 commented 1 month ago

I think this existed to deal with #5919 (comment) i.e. Caddyfile.yaml if yaml is loaded as an adapter, should use that instead of the Caddyfile adapter.

They would have specified the --adapter flag. I've added a comment.

mholt commented 1 month ago

Are you sure though? The point of this is to presume when the Caddyfile is being used: Caddyfile.yaml is clearly ambiguous (and incorrect, since a yaml file isn't Caddyfile, and a Caddyfile isn't yaml) -- what happens if they don't use the --adapter flag? Do we presume Caddyfile or YAML?

mohammed90 commented 1 month ago

what happens if they don't use the --adapter flag? Do we presume Caddyfile or YAML?

I can add a condition for such pesky scenarios 🙂

if adapterName == "" && startsOrEndsInCaddyfile {
    return nil, "", fmt.Error("ambiguous... blablabbla")
}
mholt commented 1 month ago

Ok, yeah, that sounds better. Maybe an error is better than ignoring it like we were doing. (But yeah, it's still important that we don't proceed, either way.)

graphixillusion commented 1 month ago

With this new modification, in version 2.8.2 my Caddyfile isn't recognized anymore. I need to force it using -c Caddyfile --adapter caddyfile. If not i get ambigous config message error. Same Caddyfile works without problems in 2.8.1

mohammed90 commented 1 month ago

With this new modification, in version 2.8.2 my Caddyfile isn't recognized anymore. I need to force it using -c Caddyfile --adapter caddyfile. If not i get ambigous config message error. Same Caddyfile works without problems in 2.8.1

My bad. Let me work on the fix. I'm sorry.

cattyhouse commented 1 month ago

official website mentions Caddyfile as config name (note the cap C), but since this change 'Caddyfile' will produce error. You may need to modify the documents of your official website or add Caddyfile to this line startsOrEndsInCaddyfile := strings.HasPrefix(baseConfig, "caddyfile") || strings.HasSuffix(baseConfig, ".caddyfile")

mohammed90 commented 1 month ago

You may need to modify the documents of your official website

That's not necessary. It's a regression. Please see the linked PR.

avluis commented 1 month ago

Aha! I knew it; appreciate the very quick fix!