fly-apps / laravel-docker

Base Docker images for use with Laravel on Fly.io
38 stars 8 forks source link

nginx `log_format` Fly-Client-IP #8

Closed kingIZZZY closed 6 months ago

kingIZZZY commented 7 months ago

nginx access log prints the internal IP address for each request All requests in the access log output appear to come from the same origin IP address - which is only the fly router internal IP 🤦‍♂️ Can it please be configured to print the correct IP address whether Fly-Client-IP or X-Forwarded-For or whatever it needs to be 🙏

Example:

172.160.103.90 - - [01/Jan/2024:16:38:09 +0000] "GET / HTTP/1.1" 200 77575 "-" "Mozilla/5.0 (Linux; Android 10; Z5156CC Build/QP1A.190711.020; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/119.0.6045.163 Mobile Safari/537.36"

All access log entries have the same exact IP address no matter where the request comes from

fideloper commented 6 months ago

Thanks, - great point.

I'm whipping something up to resolve that.

fideloper commented 6 months ago

I'll merge in https://github.com/fly-apps/laravel-docker/pull/9 soon

fideloper commented 6 months ago

Two notes:

1️⃣ The log format created is this (modified from the default combined log format):

log_format  fly  '$remote_addr - $remote_user [$time_local] "$request" '
                 '$status $body_bytes_sent "$http_referer" '
                 '"$http_user_agent" "$forwarded_header_value"';

Which means the client IP will be last in the log string.

2️⃣ I'm re-building the Docker images now so this update makes it to Docker Hub, that'll take a few minutes.

kingIZZZY commented 6 months ago

Would it make sense to put the fly-client-ip at the beginning and maybe add the remote_addr at the end? In this context of a docker project specifically made for fly.io it would seem the remote_addr is guaranteed to always be some boring internal fly.io ip address, and the actual interesting IP address which we are most interested to see at the beginning of log lines is the fly-client-ip, no?

fideloper commented 6 months ago

I don't want to stray far from the standard combined log format, the format used here is like the 2nd most common log format (appending x-forwarded-for or similar after the combined log format stuff content), so in that sense I'd rather not do more.

kingIZZZY commented 6 months ago

I see Arguably this "most common format" also most commonly displays the most commonly desired IP address - the actual client IP and not the fly.io-specific IP.. 🤔 just my 2cents

How about - is there any way to open this up to be configured from the Dockerfile? So by default it can be configured as you implemented, but in the Dockerfile there can be some line to conveniently modify this log_format particularly for fly.io users as mentioned because of the "real IP" issue specific to such fly.io projects