bool2nginx function

[3flex]

#453 will introduce ripienaar's module_data module. In YAML on and off are treated as boolean true and false, but this module expects strings, so on and off will have to be quoted in YAML.

Consider using an equivalent function to bool2httpd from puppetlabs-apache: puppetlabs/puppetlabs-apache@d2ad8ec

This would allow both booleans and strings to be used interchangeably, allowing users to choose whichever style suits them while avoiding this potential YAML gotcha.

[wyardley]

Now that module_data is not being used exactly, is this still needed?
I think in most cases where directives are actually 'on' / 'off', we'd want them to be quoted... seems like this could end up causing unpredictable behavior either way.

[3flex]

I still think it has value. Values passed through from Hiera face the same issue. I'd change the function used in puppetlabs-apache and only check for literal booleans, and convert those to on/off, and pass anything else through as is (the function in puppetlabs-apache also checks for strings with value "true" or "false" and tries to convert those too which I agree could be confusing. But there's no reason I can think of to attempt to pass a literal boolean as the value of an nginx directive in a configuration file, hence the reason for this function to try and be a little more user friendly).

It could also be used in places to clean up the templates a bit and remove a bunch of conditionals (like in https://github.com/puppetlabs/puppetlabs-apache/pull/1400/files)

That said, I'm not going to work on this, and it doesn't seem like anyone else will run with it, so no problem if you want to close it.

[wyardley]

Yeah, I guess that was my main worry -- that it would treat the strings 'true' / 'false' as booleans, which I think would be a little regressive, given how Puppet 4 stops doing that 'magic' conversion of booleans to strings in most cases.

When you say convert to on / off, do you mean you'd be using on / off internally?
In some cases, there are parameters that actually are booleans (i.e., things that are used internally), so would this function only be used for parameters that actually have a value of on / off in the nginx config?

I will leave this open for now... we will probably need something similar to this if we undertake another round of structural maintenance.

[3flex]

For a simple contrived example, if I wanted to set ssl_verify_client on; in the nginx template, I'd expect setting any of these in hiera to do so:

ssl_verify_client: on
ssl_verify_client: 'on'
ssl_verify_client: true

I would expect ssl_verify_client: 'true' would be passed through as ssl_verify_client true; in the config file. Which doesn't make sense in this example but means you can set ssl_verify_client: 'optional' and have that pass through as expected.

So yes, using on / off internally when the template is populated, and only for parameters that have on, off, and potentially other valid values in the nginx config (like ssl_verify_client).

[wyardley]

@igalic @bastelfreak Just mentioning this old thread again... It would be cool to have true / on and false / off work the same way, but maybe not at the cost of not being able to have things be undef.

Can an enum be undef?

[3flex]

Looks like you can use Optional for that:

The Optional data type wraps one other data type, and results in a data type that matches anything that type would match plus undef.

I'm not sure this is so relevant anymore though. Enum['on','off'] only allows the string on or off, it won't accept booleans, so there's a built-in validation in case anyone uses unquoted on or off in their YAML, which avoids the original issue.

Also nginx doesn't really treat on/off as boolean, since many directives accept on, off, or something else...

[wyardley]

Cool, let's close this then.