Allow setting of the umask for the consul daemon. #106
Conversation
|
I should note that I've tested the upstart and launchd versions of this, since I have Consul running on Ubuntu and OS X, but I haven't really verified that my systemd, sles, sysv, or debian init scripts work. |
| @@ -63,6 +63,7 @@ | |||
| $watches = {}, | |||
| $checks = {}, | |||
| $acls = {}, | |||
| $umask = 0022, | |||
There was a problem hiding this comment.
It would be a tad safer to put this in quotes as it is meant to be treated as a string
|
I've put quotes around the default, and rebased on top of master to fix the merge conflict with #105 |
|
hold on, syntax failures. |
|
Travis is happy again. |
| @@ -63,6 +63,7 @@ | |||
| $watches = {}, | |||
| $checks = {}, | |||
| $acls = {}, | |||
| $umask = '0022', | |||
| ) inherits consul::params { | |||
|
|
|||
There was a problem hiding this comment.
can you validate_re on this?
Also it would be a double standard for me to insist on tests from external people and not from contributors?
I would mostly want to be sure you got the name of the variable right (we don't run tests with strict variables, we probably should) also that each init script has the correct syntax for umask maybe?
There was a problem hiding this comment.
I could validate_re to be 0[0-7]{3} maybe; but some of these invocations (particularly the bash builtin umask) accept other formats, like o+rwx g+rwx
There was a problem hiding this comment.
Yea but, whatever your input in has to be readable by all the other outputs? Your numeric re there seems very sane to me.
Allow setting of the umask for the consul daemon.
| @@ -52,7 +52,7 @@ start() { | |||
| mkrundir | |||
| [ -f $PID_FILE ] && rm $PID_FILE | |||
| daemon --user=<%= scope.lookupvar('consul::user') %> \ | |||
| --pidfile="$PID_FILE" \ | |||
| --pidfile="$PID_FILE" --umask=<%= scope.lookupvar('consul::umask') %> \ | |||
There was a problem hiding this comment.
This breaks CentOS init scripts with the following error in the logs:
/etc/init.d/consul: Usage: daemon [+/-nicelevel] {program} [FAILED]
There was a problem hiding this comment.
Oops, sorry about that. I guess I'll have to just call the shell built in unmask before launching the daemon.
There was a problem hiding this comment.
yeah,I can confirm this breaks the init script on CentOS/RHEL
This allows setting of the umask before spawning the consul daemon. I'd like to be able to store secrets in Consul, but by default Consul creates files as 0644 and directories as 0755, which leaves these secrets readable to anyone on the system.
Consul does respect umask, however, so by setting it before launching, we can avoid creating these files as world-readable.
Currently this defaults to 0022, (not writeable by world or group), but in a secure environment it's probably good to set to 0027 or 0077. I could be convinced to change the default to one of those.