Update Caddy reverse proxy config to exclude root path for static files

[jeromegamez]

Hey there 👋,

first and foremost: thanks for Framework X! I'm currently developing a website from scratch and it's like a fresh breeze compared to the "heavy" frameworks I'm used to, and I'm having fun using and exploring it - a feeling I haven't had for a long time.

While following the docs to let Caddy (also a new experience for me) serve the static assets on my local machine, the home page (/) didn't work for me until I excluded / in addition to *.php in the @static directive.

I'm proposing the change in a PR instead of starting a discussion because I'm hopeful this change was the correct choice (and although it's just one added char) 😅.

If the change tackles the problem from the wrong end, I'd be grateful for a hint in the right direction!

:octocat:

[SimonFrings]

Hey @jeromegamez, thanks for opening up this PR, always happy about contributions! 💪

first and foremost: thanks for Framework X! I'm currently developing a website from scratch and it's like a fresh breeze compared to the "heavy" frameworks I'm used to, and I'm having fun using and exploring it - a feeling I haven't had for a long time.

Glad you like the project, your comment in itself sounds like an awesome testimonial, do you think we could place this on our website? 😁

I'm proposing the change in a PR instead of starting a discussion because I'm hopeful this change was the correct choice (and although it's just one added char) sweat_smile.

I have to be honest here, I'm not the most experienced when it comes to Caddy, hard for me to judge on this one, but I'll try to look into it. @francislavoie filed the original PR for Caddy in #82, so I'm curious what he thinks about this one.

Would be much easier to decide if we'd had (acceptance) tests specifically for Caddy, maybe this is something we can look into a follow up PR. What do you think?

[jeromegamez]

Glad you like the project, your comment in itself sounds like an awesome testimonial, do you think we could place this on our website? 😁

Of course! I'll send you the link then once it's ready for public ☺️

I have to be honest here, I'm not the most experienced when it comes to Caddy, hard for me to judge on this one, but I'll try to look into it.

Same for me, but since it worked for me on my machine™ I thought "well, this must be it"!. I'm curious about what Francis will say as well! It has been a while since #82 after all…

I'll spin up an example repo and describe how I've set it up on my computer to (hopefully) confirm the behaviour, if Francis doesn't beat me to it with his reply 💪

[jeromegamez]

Now that I think about it, I'm not sure the acceptance tests should be part of the Framework - Caddy in the docs is just an example of an (infrastructure) implementation, not a dependency 🤔

[francislavoie]

The usual way to catch requests to static files in Caddy is with the file matcher, which will check the filesystem to see if that requested file exists. If so, you can serve it with file_server, else you can let everything else go to the reverse_proxy to let X handle it with its routing.

I'm on mobile right now so I can't write up an example config, but I'll come back to this later today, unless you want to take a crack at it yourself.

[francislavoie]

Alright, sorry, I was misreading this discussion when I got the notification this morning because I started with the comment I got @'d on which made me miss the whole point of this PR.

So yeah, I think path / *.php is probably fine. But even *.php might not even be relevant. It depends on how the X app is set up to route requests. If you're always using "clean" URLs with no *.php (because there's no reason to have .php, it's not file-based routing, probably) then it's not a useful condition.

Another thing to mention is the file matcher also provides a placeholder {file_match.type} which could be used to check if the matched file is actually a file and not a directory. That might be a more accurate way to fix this issue. https://caddyserver.com/docs/caddyfile/matchers#file

Also if we require Caddy v2.6.0 at minimum, then an expression matcher could be used to do this in a more programmatic looking way.

@static `file({path}) && {file_match.type} == "file"`
handle @static {
	file_server
}

Since the path is already the one we're matching, and we don't need to serve directories, then the rewrite can be removed, since it serves no purpose anymore.

I gave a talk in September about the expression matcher functionality, probably the best way for me to give you the backstory on how this works: https://youtu.be/QSerOHpMjgY?t=641

[francislavoie]

I just noticed there's a semicolon on the line of the root directive in the doc. That should be removed, the Caddyfile doesn't use semicolon delimiters.

So the config I'd put in the docs, altogether:

example.com {
	root * /var/www/html/public

	encode gzip

	@static `file({path}) && {file_match.type} == "file"`
	handle @static {
		file_server
	}

	handle {
		reverse_proxy localhost:8080
	}
}

[jeromegamez]

@francislavoie You're awesome, thank you for explaining everything in so much detail, and your talk was enjoyable as well!

I noticed that the index.php file was treated as a (plain-text) file as well and ended up with

example.com {
	root * /var/www/html/public

	encode gzip

	@static `!path("*.php") && file({path}) && {file_match.type} == "file"`

	handle @static {
		file_server
	}

	handle {
		reverse_proxy localhost:8080
	}
}

Would that be the way to go?

@SimonFrings If Francis signs off on the updated condition, would you like me to replace the config as he suggested, or would you prefer the original writing style? If I understand correctly, we wouldn't be able to differentiate between files and directories then, though.

[francislavoie]

Yeah, that makes sense. I wonder though, is there even a need for an index.php file to exist, if the X server was run directly? Couldn't that live outside the public directory in that case? My understanding is that it's only there for traditional (PHP-FPM/Apache mod_php) setups.

Minor nitpick, I tend to prefer not having a newline between named matchers and the directive it's attached to, to link them visually as related.

[jeromegamez]

You're absolutely right - in my case, I'm not interested in the traditional setup (and I believe that would also take from the point that Framework X is more performant on its own), so moved the server out of the public directory and it makes things indeed easier… but that's perhaps for a discussion in another tread 😅

For the moment, shall we perhaps go with just adding the / to the not path condition, and perhaps create a new PR to change the example altogether?

[SimonFrings]

Sounds like a plan, I think this PR is good as is :+1:

[SimonFrings]

@jeromegamez I know I already approved, but I talked a bit more with @clue about this ticket. I think this could be a good point to introduce some automated tests for Caddy, in order to assure everything works as expected. This would involve adding a Caddy job to our ci.yml, similar to the already existing nginx-webserver and Apache-webserver jobs. At the same time we would also have to add a new case to the tests/acceptance.sh, testing the behavior introduced in this PR.

What do you think?

[francislavoie]

Just got notified here, reminded me to follow up on https://github.com/clue/framework-x/pull/209#issuecomment-1374447690; in Caddy v2.6.3, we fixed the file matcher in expression to no longer need {path} for the base case. So file({path}) can be replaced with simply file() which reads a bit nicer.

Let me know if you need me to review the acceptance tests if you end up going that route!

[jeromegamez]

The PR is now updated with what we've discussed so far, thank you for all your input! :pray:

I have difficulties running the acceptance tests locally on MacOS, it starts with the system's grep not supporting the -P option; also, extending the current process is challenging because there is no pre-built php:caddy docker image.

In order to enable everyone (also Windows users) to execute the unit and acceptance tests without having all prerequisites set up on their local computer… how do you feel about migrating the existing tests to a docker(-compose)-based setup? I feel this would blow up the scope of this PR, though 🤔 (and I realize that I'm blowing it up by proposing this in the first place 😅)

[francislavoie]

If you'd like, you could use https://github.com/Baldinof/caddy-supervisor which is a plugin for Caddy that can run other processes like php-fpm, so Caddy acts as PID 1 in Docker, and you can have both Caddy and php-fpm in the same container. Takes a bit of setup though. Just an idea if it might be helpful.

[bpolaszek]

If you'd like, you could use https://github.com/Baldinof/caddy-supervisor which is a plugin for Caddy that can run other processes like php-fpm, so Caddy acts as PID 1 in Docker, and you can have both Caddy and php-fpm in the same container. Takes a bit of setup though. Just an idea if it might be helpful.

Just adding a little testimonial here: I've been using this (Framework X + Caddy Reverse proxy + Caddy Supervisor to spawn X servers) in production for a few months and it just works awesomely. The only hard part was to get it running on Docker for development as my low Docker skills didn't help me figure out how to make a caddy+php image - but in production, the whole stuff runs with just a few lines of config 👍

[francislavoie]

The only hard part was to get it running on Docker for development as my low Docker skills didn't help me figure out how to make a caddy+php image

Gist of it: do a multi-stage build; use the Caddy builder image, copy it into a php base image, make sure the image has some things set up like mailcap and ca-certificates and the env vars for Caddy, see https://github.com/caddyserver/caddy-docker/blob/82318569fbf9d281912802ece7c51b9a8cf8a033/2.6/alpine/Dockerfile#L3 for how the base image looks.

[SimonFrings]

@bpolaszek thanks for bringing in your testimonial here, we're always happy to hear about your success stories 😍

FYI, @clue is currently also looking into refactoring the examples and functional tests, so we have a solid foundation for easier test integrations in the future.

[SimonFrings]

@jeromegamez @francislavoie We just released the new v0.16.0 of Framework X containing the mentioned refactorings for the examples and functional tests (#250). We also added tests for the nginx and Apache configuration in #251, so I think we should have everything to bring this ticket here up to date and test the Caddy configuration file :+1: