-
-
Notifications
You must be signed in to change notification settings - Fork 58
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
Change the user and group under which proxysql runs on Debian systems #44
Conversation
On Debian systems prosysql should be running under the proxysql user and group, instead of root. The configuration files should also be owned by this user and group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this with every debian package? I think this isn't in mainline Debian so this should be good.
manifests/params.pp
Outdated
@@ -24,13 +24,7 @@ | |||
$admin_listen_port = 6032 | |||
|
|||
case $::operatingsystem { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might make sense to change this to the osfamily instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Updating as suggested.
Correct. There is currently no |
manifests/params.pp
Outdated
@@ -23,20 +23,14 @@ | |||
$admin_listen_ip = '127.0.0.1' | |||
$admin_listen_port = 6032 | |||
|
|||
case $::operatingsystem { | |||
case $::osfamily { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please switch this to the new $facts hash? $facts['os']['family']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bastelfreak - I have updated the code as you requested and squashed the change.
The problem is that Percona rebuilds the package and changes the defaults that ProxySQL uses. Imho we should add this as the default repo to use, if you want to override the repo to the percona repo you should also override the user/group to run it under... |
@tullis @ekohl @bastelfreak see #47 that PR shows how I would update the module currently, I tested that PR on Ubuntu 16.04 and CentOS 7. i still need to review the testsuite and docs but I wanted to push this here already to give an idea... It's been a while since I wrote puppet-code, my new job doesn't include puppet anymore but I can still help and contribute when time allows... |
… instead of operatingsystem There is no longer any difference between the Debian and Ubuntu variants, in terms of permissions. This means that we can simplify the case statement by simply using the os::family fact instead of the operatingsystem fact.
Thanks all for your input. I'm happy to close this PR in favour of the default use of the new upstream repo. Does anyone know when the version on the forge might be updated with a new release? |
Closing in favour of #47 |
On Debian systems proxysql should be running under the
proxysql
user and group, instead ofroot
.The configuration files should also be owned by this user and group.
I initially started looking to see when the Debian version might have switched from using the
root
account toproxysql
, so that I could use a version check inparams.pp
but I could not find any time when it did. Therefore I thought it best to merge the case blocks for Ubuntu and Debian into one.