Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow changing the mysql-config-file group-ownership #1284

Merged

Conversation

unki
Copy link
Contributor

@unki unki commented Feb 26, 2020

With my PR #1278 it's now possible to change the file-permission-mode of the mysql-config-file - e.g. my.cnf.

If you are trying now to apply a rather strict file-mode like 0600 - in combination with Puppet's default file-ownership root:root - it will lead to issues with MariaDB not being able to access the config-file after it has switched the processes to the service-user (e.g. mysql).

Therefor I would propose additionally using $mysql::server::mysql_group as the mysql-config-file's group-ownership. Then restricting access would be easy done with a 0640 file-mode.

@unki unki requested a review from a team as a code owner February 26, 2020 09:35
@unki unki force-pushed the adapt-group-owner-of-config-file branch 2 times, most recently from 4d5e2f8 to 97ef2ca Compare February 26, 2020 20:07
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@8b2591f). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1284   +/-   ##
=========================================
  Coverage          ?   14.53%           
=========================================
  Files             ?       19           
  Lines             ?      860           
  Branches          ?        0           
=========================================
  Hits              ?      125           
  Misses            ?      735           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b2591f...97ef2ca. Read the comment docs.

@unki unki force-pushed the adapt-group-owner-of-config-file branch 4 times, most recently from e7c8746 to bff135d Compare February 27, 2020 19:07
@michaeltlombardi
Copy link
Contributor

Hey @unki, thanks for this patch! Can you rebase your PR on latest master and repush? I believe that may fix your CI issue.

@unki unki force-pushed the adapt-group-owner-of-config-file branch from bff135d to e911dc0 Compare March 5, 2020 10:53
@michaeltlombardi michaeltlombardi changed the title allow changing the mysql-config-file group-ownership Allow changing the mysql-config-file group-ownership Mar 5, 2020
@michaeltlombardi michaeltlombardi merged commit 4750787 into puppetlabs:master Mar 5, 2020
@unki unki deleted the adapt-group-owner-of-config-file branch March 5, 2020 16:18
@TwizzyDizzy
Copy link

Hey @unki,

thanks also for this... seems like we had the same thoughts. What I don't get, is why you - in your second commit - decided not to use $mysql::server::root_group but introduce those new variables for mycnf-ownership. Did using $mysql::server::root_group have side effects that made those new variables necessary?

Cheers & thanks in advance :)
Thomas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants