librenms / docker

LibreNMS Docker image
MIT License
662 stars 275 forks source link

env-config: allow setting AD config from docker environment #431

Closed MarlinMr closed 1 month ago

MarlinMr commented 5 months ago

This MR allows you to set Active Directory settings from the docker environment.

MarlinMr commented 5 months ago

Maybe having default values is a dumb idea? Maybe it should be blank by default? Perhaps that breaks current deployments of it's not blank?

crazy-max commented 5 months ago

I don't think we should have explicit envs for each LibreNMS config in this image but instead use live config as shown in https://github.com/librenms/docker?tab=readme-ov-file#live-config

In your case you could create a /data/config/ad.php file with your Active Directory config.

murrant commented 5 months ago

The problem is when you set stuff in config.php (via includes like this too), it completely disables the webui settings. Placing an if around them would fix that though and only set them if the env variables are set.

A word of caution too. There are no immediate plans, but LibreNMS may scrap it's custom auth code and use upstream auth code. This may affect the available options so take care not to make any env that are overly specific to LibreNMS' implementation.

MarlinMr commented 4 months ago

The reason this is my preferred method, is simply that it allows setting settings via docker environment, which in turn allows all settings to be deployed easily from central docker management software.

It completely disables webui settings, yes, but that's not needed as it is set in the docker env anyhow. That's ofc only "not a problem" for those who want to use docker env, yes. Putting it inside an if could be a good idea, sure.

murrant commented 4 months ago

There are already two methods to set config settings inside containers (without running lnms config:set) Perhaps making those work for your use case is better than expanding the environment variables to cover the thousands of settings that LibreNMS has.