Code review comment for lp:~hloeung/charm-haproxy/bump-global-dh-param

Revision history for this message
Haw Loeung (hloeung) wrote :

> One comment about the description field.
>
> I also have a question about the fact that java 7 is the default on trusty.
> This will break new deployments on trusty that don't specify
> global_default_dh_param explicitly. I think our options are to:
>
> 1) Drop support for trusty
> 2) Update the code to fallback to 1024 if java 7 or lower is installed despite
> the setting of this config option
> 3) Leave it as is and essentially require anyone deploying with java 7 to set
> this to 1024 manually.

Actually, it's not if the Java package is installed on the same unit as HAProxy but rather if you think or know you want to support Java 7 and lower clients, then you need to make sure the dhparam here is 1024.

> I don't see any references to java or openjdk in the charm code - do we know
> why this is called out explicitly? What is java potentially being used for
> here? Looks like this description was added in revno 88 on 205-02-20.

The description looks like it's one taken straight out of the HAProxy manual:

| http://cbonte.github.io/haproxy-dconv/1.8/configuration.html#3.2-tune.ssl.default-dh-param

But it looks like that has been updated, default in HAProxy 2.2 is now 2048:

| http://cbonte.github.io/haproxy-dconv/2.2/configuration.html#3.2-tune.ssl.default-dh-param

« Back to merge proposal