Support data-ciphers and data-ciphers-fallback options

[rvm-peercode]

Pull Request (PR) description

Support data-ciphers and data-ciphers-fallback options for OpenVPN 2.5.

This Pull Request (PR) fixes the following issues: n/a

[smortex]

Some minor data types issues. I added in-line comments to address them.

I would suggest to stick to String in this PR to match the rest of the module for now, and if you are up to it work on improving the module so that OpenVPN use the default values (i.e. no parameter is output) when no explicit value is provided instead of when a '' is passed in another PR that will go in a new major release.

[smortex]

Optional with a default value cannot be forced to undef. Since we test the value against '', it make sense to only allow String:

Suggested change

	Optional[String] $data_ciphers = 'AES-256-GCM:AES-128-GCM',
	String $data_ciphers = 'AES-256-GCM:AES-128-GCM',

[smortex]

Since we test the value against '' bellow, it make sense to only allow String:

Suggested change

	Optional[String] $data_ciphers_fallback = undef,
	String $data_ciphers_fallback = '',

Admitedly this is inelegant, but it is the may this module currently duplicate all config.

[rvm-peercode]

I would suggest to stick to String in this PR to match the rest of the module for now

Just a quick question: if the vars are not Optional, how would that work with OpenVPN versions that don't support these yet? I added them as optional so as not to break config for OpenVPN 2.4 and before.

[smortex]

An optional parameter with a default value cannot be forced to "back" to undef: it will take the default value:

class foo (
  Optional[String] $x = 'foo',
) {
  warning("x=${x}")
}

class { 'foo':
  x => undef, # This will print "x=foo"
}

If the parameters are new, indeed adding them to the config may cause trouble with older versions. My recommendation then would be make sure these parameters are only output when the user provided a value for them. This would mean defaulting to '' (what the module seems to currently do) or undef (preferred because it is more correct semantically speaking and allow better type checking).

class foo (
  Optional[String[1]] $x = undef,
  String              $y = '',
) {
  # ...
}

and the corresponding template:

<% if $x { %>
x = <%= $x %>
<% } %>
<% if $y != '' { %>
y = <%= $y %>
<% } %>