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

Improve error message when trying to define a global matcher #6332

Closed autoantwort closed 1 month ago

autoantwort commented 1 month ago

For me it was not obvious from the documentation that it is forbidden to write global matchers, so I wrote:

@rwth-netz client_ip 134.130.0.0/16 137.226.0.0/16 134.61.0.0/16 192.35.229.0/24 10.72.0.0/16 2a00:8a60::/32
@hilton-net client_ip 134.130.55.0/24 10.72.0.0/16 2a00:8a60:e005::0/48

(https_proxy) {
  reverse_proxy https://{args[0]} {
    transport http {
      tls_insecure_skip_verify
    }
  }
}

alt.xxx.de {
  import https_proxy 10.72.4.27
}

db.xxx.de {
  reverse_proxy @rwth-netz 10.72.4.31
}
... lot of rules using the matchers

But the error message I got was

╰─# caddy validate           
2024/05/21 13:56:17.288 INFO    using adjacent Caddyfile
2024/05/21 13:56:17.288 INFO    using provided configuration    {"config_file": "Caddyfile", "config_adapter": ""}
Error: adapting config using caddyfile: File to import not found: https_proxy, at Caddyfile:20

The error message is very misleading.

mholt commented 1 month ago

This might be a good one for @francislavoie if he has a moment :slightly_smiling_face:

francislavoie commented 1 month ago

Yep it's on my radar.

francislavoie commented 1 month ago

Ah actually we do have that kind of error (if you comment out import https_proxy you get this):

Error: Caddyfile:1: cannot define a matcher outside of a site block: '@rwth-netz'

But it's an order of operations problem. We try to replace import first before parsing, but the import doesn't work because we gave up reading snippets since we found something invalid before that.

So we'll need to shift the "global matchers" check to the same place we do snippet loading, I think.

Edit: Simple fix https://github.com/caddyserver/caddy/pull/6339