voxpupuli / puppet-collectd

Collectd module for Puppet
https://forge.puppet.com/puppet/collectd
Apache License 2.0
69 stars 272 forks source link

MySQL database plugin: allow no password; default Host, User, and Password to undef #970

Closed ph448 closed 3 years ago

ph448 commented 3 years ago

Pull Request (PR) description

This started off as a change to allow for not specifying a password, as my setup uses socket authentication, then I noticed #724 and thought to include the changes to fix that as well. Setting the default host to 'localhost' and not requiring a username or password is better in line with mysqlclient defaults, which the collectd mysql plugin ultimately uses to interact with the server. Also, the plugin supports specifying a socket, for which no username or password is required, so having them optional allows for configuring the plugin in correct ways that were not possible before the change.

This Pull Request (PR) fixes the following issues

Fixes #724

ph448 commented 3 years ago

@smortex empty strings are probably not valid for any of the string parameters in manifests/plugin/mysql/database.pp, but none of them have a minimum length requirement specified, making it look like a convention rather than an omission. that said, I'm happy to make the change, shall I change all of them?

smortex commented 3 years ago

This module has a lot of classes and a rather long history, it has some classes without data types at all, some with "relaxed" data types and some with "strict" data types. Tackling all this at once is not something appealing and fun, and I think that improving bits here and there is a better way to go, progressively improving the module validation.

If MySQL allow empty password the module must allow this (no idea, I am not a user of MySQL), for the two other parameters I would go for a minimum length of 1.

Thanks!

ph448 commented 3 years ago

OK, I've added the length restriction to both $username and $password as it doesn't make sense to set either to an empty string. Seeing that $port was already using a puppetlabs-stdlib type I've chosen to use Stdlib::Host for $hostname.