Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ linux/s390x
* `REAL_IP_FROM`: Trusted addresses that are known to send correct replacement addresses (default `0.0.0.0/32`)
* `REAL_IP_HEADER`: Request header field whose value will be used to replace the client address (default `X-Forwarded-For`)
* `LOG_IP_VAR`: Use another variable to retrieve the remote IP address for access [log_format](http://nginx.org/en/docs/http/ngx_http_log_module.html#log_format) on Nginx. (default `remote_addr`)
* `APP_TRUSTED_PROXIES`: Reverse proxy addresses trusted to forward the real client IP (default 172.17.0.0/16)
* `SESSION_DRIVER`: [Driver to use for session storage](https://github.com/librenms/librenms/blob/master/config/session.php) (default `file`)
* `CACHE_DRIVER`: [Driver to use for cache and locks](https://github.com/librenms/librenms/blob/master/config/cache.php) (default `database`)

Expand Down
1 change: 1 addition & 0 deletions examples/traefik/librenms.env
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ OPCACHE_MEM_SIZE=128
REAL_IP_FROM=0.0.0.0/32
REAL_IP_HEADER=X-Forwarded-For
LOG_IP_VAR=http_x_forwarded_for
APP_TRUSTED_PROXIES=172.17.0.0/16

CACHE_DRIVER=redis
SESSION_DRIVER=redis
Expand Down
2 changes: 2 additions & 0 deletions rootfs/etc/cont-init.d/03-config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ DB_USER=${DB_USER:-librenms}
DB_TIMEOUT=${DB_TIMEOUT:-30}

LIBRENMS_BASE_URL=${LIBRENMS_BASE_URL:-/}
APP_TRUSTED_PROXIES=${APP_TRUSTED_PROXIES:-"172.17.0.0/16"}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we think about this as a default? I'm unsure since it differs from upstream. But at least you document it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your are right this range is wrong, back in the old days I used to do exactly this: everything landed on the default bridge, so whitelisting 172.17.0.0/16 gave you a simple, ready-to-go path between your reverse proxy and any other container you spun up on the host. That's was the intent here.

Looking at the example in this repo, we're not even separating the reverse proxy from the LibreNMS server (Traefik and LibreNMS share one compose network), which will be 172.18.0.0/16 most of the time on a basic host. If we want a default to cover more than just the example scenario, it should be 172.16.0.0/12, which covers all the /16 networks Docker hands out on a host of this size.

That said, I don't have a strong stake in how this lands, I won't be using it myself (I run this in Kubernetes), I was just trying to make this easier for the most basic Docker deployments.

I appreciate your dedication to LibreNMS. Feel free to make whatever adjustments you want to this PR — or close it if that's easier. I don't plan to spend more time on it myself.

This wouldn't be the first fix I've submitted that stalled out in rounds of small tweaks. Honestly, I don't think senior contributors like you should have to wait on a drive-by contributor like me for changes you already know should be in there — just make them directly. I'm fine with whatever the result looks like.


# Timezone
echo "Setting timezone to ${TZ}..."
Expand Down Expand Up @@ -133,6 +134,7 @@ if [ -z "$DB_PASSWORD" ]; then
fi
cat >${LIBRENMS_PATH}/.env <<EOL
APP_URL=${LIBRENMS_BASE_URL}
APP_TRUSTED_PROXIES=${APP_TRUSTED_PROXIES}
DB_HOST=${DB_HOST}
DB_PORT=${DB_PORT}
DB_DATABASE=${DB_NAME}
Expand Down