Add DH param generator option

[panteparak]

With the integration of letsencrypt-nginx-proxy-companion. When both containers start at the same time, sometimes both containers are trying to generate Dh-Param at the same time which could be heavily taxing thus, adding an option to disable the generation of dh param and only let letsencrypt-nginx-proxy-companion do the generating, or offers an already generated dh param.

[panteparak]

@jwilder would it be possible to re-run tests? The failed tests was triggered when it was trying to generate dh param which could take time. I have run tests on my computer and it passed every single test cases

[kamermans]

This makes sense to me, as the contributor that wrote the DHPARAM generation code.

One suggestion I would make is to rename GENERATE_DHPARAM to DHPARAM_GENERATION so that the environment variables are sort of "namespaced", making it easier to see, at a glance, what feature an env var is related to. That having been said, I'm not sure if anyone else is following that practice :)

[panteparak]

@kamermans Yes I do agree to, it's always a good idea to be consistence. Will make that change in the future commit

[jessejoe]

Is there a reason this was never merged? I'm also using letsencrypt-nginx-proxy-companion and this adds an unnecessary delay.

[dfsoeten]

Not quite sure if this is the right place to ask this, but is the DHPARAM_GENERATION environment option working? If I set it to false the container still tries to generate a dhparam. 🤔

[jessejoe]

@iamdann yeah, looks like there's a small user experience bug here. If you provide DHPARAM_GENERATION you also have to provide DHPARAM_BITS, otherwise DHPARAM_GENERATION gets passed as the first argument to /app/generate-dhparam.sh which is expecting the DHPARAM_BITS -

$ docker run -it --rm -e "DHPARAM_GENERATION=false" testing /bin/bash
WARNING: /etc/nginx/dhparam/dhparam.pem was not found. A pre-generated dhparam.pem will be used for now while a new one
is being generated in the background.  Once the new dhparam.pem is in place, nginx will be reloaded.
dhparam: Can't parse "false" as a number

$ docker run -it --rm -e "DHPARAM_BITS=2048" -e "DHPARAM_GENERATION=false" testing /bin/bash
Skipping Diffie-Hellman parameters generation and Ignoring pre-generated dhparam.pem

https://github.com/jwilder/nginx-proxy/blob/8c590fc68ffb0551cfa99875f1a8fd1cdf6a3e66/generate-dhparam.sh#L4-L5

I think the better user experience would be to set both variables as default if they aren't set, not rely on positional arguments:

diff --git a/generate-dhparam.sh b/generate-dhparam.sh
index 4099dde..aed055e 100755
--- a/generate-dhparam.sh
+++ b/generate-dhparam.sh
@@ -4,2 +4,2 @@
-DHPARAM_BITS=${1:-2048}
-GENERATE_DHPARAM=${2:-true}
+DHPARAM_BITS=${DHPARAM_BITS:-2048}
+GENERATE_DHPARAM=${DHPARAM_GENERATION:-true

I'm also not sure why there's both DHPARAM_GENERATION and GENERATE_DHPARAM in different scripts. Seems like it should just be one or the other as far as naming.

@panteparak @kamermans if you agree I'd be happy to open a PR.

[kamermans]

That all makes sense to me, and a PR would be appreciated! You're welcome to fix the variable naming to make it consistent as well :)

[jessejoe]

@kamermans I opened #1263 ICYMI.