puppetlabs-mysql

puppetlabs-mysql copied to clipboard

In some very special (read: stupid) configuration I need to supply a special configuration file so that puppet can log in.

The problem is: Some other package (plesk) will not allow to have /root/my.cnf in existence. Having the credentials in /etc/my.cnd.d/client.cfg break other stuff in the application. It will authenticate as a (non root) user, using a password from the ENV. The password in the client.cnf [mysql] section overrides this.

On top of this, I imagine someone would like to use a specific user for puppet to configure stuff, and/or disable/remove the root user

I really need help with creating a spec...

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: Heidistein
:x: LukasAud
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Hi! I would really like some feedback on this... should I continue this path or should I change to --defaults-group-suffix= option. Is there even a remote chance this will ever get merged?

Adding another method and more conditionals is more complexity than needed. I'd much prefer to simply extend the original method to return the most specific options file that exists. Something like

def self.defaults_file
  options = [
    "#{Facter.value(:root_home)}/.my.puppet.cnf",
    "#{Facter.value(:root_home)}/.my.cnf",
  ]
  found = options.find {|path| File.file? path}

  "--defaults-extra-file=#{found}" if found
end

I kind of hate doing this, but I do see the utility. (I think.)

The best I can tell, it's not that Plesk disallows /root/my.cnf but that for whatever reason, its installer fails if /root/my.cnf provides authentication options. There is precedent for similar config files, eg /etc/mysql/debian.cnf.

Adding another method and more conditionals is more complexity than needed. I'd much prefer to simply extend the original method to return the most specific options file that exists. Something like

def self.defaults_file
  options = [
    "#{Facter.value(:root_home)}/.my.puppet.cnf",
    "#{Facter.value(:root_home)}/.my.cnf",
  ]
  found = options.find {|path| File.file? path}

  "--defaults-extra-file=#{found}" if found
end

I kind of hate doing this, but I do see the utility. (I think.)

The best I can tell, it's not that Plesk disallows /root/my.cnf but that for whatever reason, its installer fails if /root/my.cnf provides authentication options. There is precedent for similar config files, eg /etc/mysql/debian.cnf.

Yes, your option is nicer, however. .my.cnf uses --defaults-extra-file= whereas .my.puppet.cnf uses --defaults-file=. This is on purpose. .my.cnf is just to leave backwards compatability (leave 'as is'). The .my.puppet.cnf user the "use only this file, and none other" option

This PR has been marked as stale because it has been open for a while and has had no recent activity. If this PR is still important to you please drop a comment below and we will add this to our backlog to complete. Otherwise, it will be closed in 7 days.

This still is relevant!

Hello! 👋

This pull request has been open for a while and has had no recent activity. We've labelled it with attention-needed so that we can get a clear view of which PRs need our attention.

If you are waiting on a response from us we will try and address your comments on a future Community Day.

Alternatively, if it is no longer relevant to you please close the PR with a comment.

Please note that if a pull request receives no update for 7 after it has been labelled, it will be closed. We are always happy to re-open pull request if they have been closed in error.

Hey @Heidistein, just wanted to follow up on this PR. We are investigating your PR as we currently are not sure whether this feature can be justified in terms of functionality/reachability/maintenance-cost.

We understand this PR has been sitting around for close to a year already, but it may take a bit longer still until we can assign the resources to properly look at it. Sorry for the inconvenience.

Apologies for that. It seems like our bot might be malfunctioning.

Hey @Heidistein, our team has finally been able to evaluate your proposed change. Unfortunately, after some discussions, we have decided that we will not merge this PR. While the change itself is valid and seems to work, this is not something that we are looking to implement right now.

We will be closing this PR now. However, we are willing to re-open this PR in the future if more people from the community share their interest in it. As such, I recommend you create a 'feature request' in our 'Issues' section and link this PR there so that the changes and discussions are not lost.