Enable configuration through environment variables by disabling php-fpm/clear_env #85
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for opening this pull request! Be sure to follow the pull request template!
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
Thanks for the PR. Can you please change the method to match what we do for Nextcloud? https://github.com/linuxserver/docker-nextcloud/blob/master/Dockerfile#L55-L56 |
By default php-fpm has `clear_env` option activated. This means that environment variables never will be passed on from the system environment to the actual worker process for a given request. This also means that Grocy will not be configureable via Environment variables. Which is very common behaviour. We can fix this by disabling the `clear_env`. You can test that it works by for example changing the currency via an environment variable: docker run -e GROCY_CURRENCY=NOK -p 8080:80 linuxserver/grocy:latest Then opening: http://localhost:8080/stockoverview And verifying that the app allows you to change the setting. There's more info on how Grocy handles configuration, here: https://github.com/grocy/grocy/blob/master/config-dist.php#L3
benedicteb
force-pushed
the
add-clear-env-no
branch
from
bdbabe2
to
b0f407b
Compare
|
Thanks for the review! 😊 I copied the sed/grep lines from the Nextcloud repo into both the Dockerfiles and performed the same tests again. It works well! Also squashed/rebased the commits to make a bit more sense. Let me know if there's anything else y'all need to get this merged 😊 |
|
I am a bot, here are the test results for this PR:
|
Description:
By default php-fpm has
clear_envoption activated.This means that environment variables never will be passed on from the system environment to the actual worker process for a given request.
And because of that it's not possible to configure Grocy with environment variables even though it's advocated for in the default configuration file. It seems other users already have struggled with this, with this image, for example in #12.
We can fix this by disabling the
clear_envin a.dfile forphp-fpm. This PR introduces that file, which will be copied automatically by theADDat the bottom ofDockerfile.Benefits of this PR and context:
The clear benefit is that you'll be able to run Grocy completely declarative without editing any files.
There are some reasons as to why
clear_env = yesis default inphp-fpm. For example a note on performance. That forwarding the environment variables has certain performance implications in high traffic situations. Yet I believe Grocy won't be an app usually run in such situations and that the performance impact isn't so big that you'll notice it very soon.There also might be some security implications. For example on multi-tenant setups. Yet that seems unlikely to apply here since we use containers for that kind of isolation.
It is also worth noting that in the official
phpdocker imageclear_envis actually disabled. See this thread for info.How Has This Been Tested?
I have tested this by building the image locally, running it and verifying it now passes on configuration via environment variables.
You can test it yourself by for example changing the currency via an environment variable:
Then opening:
http://localhost:8080/stockoverview
And verifying that the app actually has NOK as the currency. There's more info on how Grocy handles configuration, here:
https://github.com/grocy/grocy/blob/master/config-dist.php#L3
Source / References: