On Sun, May 05, 2019 at 03:06:31PM +0200, Tim Düsterhus wrote: > William, > > Am 26.04.19 um 20:30 schrieb Tim Düsterhus: > > William, > > > > Am 26.04.19 um 14:56 schrieb William Lallemand: > >> On Fri, Apr 26, 2019 at 12:15:37AM +0200, Tim Duesterhus wrote: > >>> [Service] > >>> -Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" > >>> +Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" > >>> "MASTER_SOCKET=/run/haproxy-master.sock" > >>> ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q > >>> -ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE > >>> +ExecStart=@SBINDIR@/haproxy -Ws -S $MASTER_SOCKET -f $CONFIG -p $PIDFILE > >>> ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q > >> > >> In my opinion that's not a good idea to do it this way, because you can't > >> disable the master CLI by unsetting the MASTER_SOCKET variable. > >> > >> It's probably better to have "-S /run/haproxy-master.sock" in an $OPTIONS > >> variable at the end of the ExecStart line. > >> > > > > I'm not sure whether this is better. Yes you can disable the socket more > > easily then, but you have to remember to add it back when editing the > > 'OPTIONS' variable. > > > > I believe it boils to to this question: Does the user usually want to > > run with the socket enabled or not? My guess is that most users want to > > have this socket (having is better than needing!), they might just want > > to move it to a different location rather than outright disabling it. > > During my tests it was only accessible to root, so there does not appear > > a security issue in the default configuration either. > > > > On an unrelated note: Debian patches the unit file to add in such a > > variable, called 'EXTRAOPTS': > > https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch > > > > So if we want to go the 'OPTIONS' variable path we might want to re-use > > that variable name. Any change will break their patch anyway so they > > definitely notice. > > > > Best regards > > Tim Düsterhus > > > > I'd like to bump my reply to you once more. As outlined previously I > believe that the "default" expectation is having the socket, it can > still be disabled by overriding the ExecStart= declaration. > > Best regards > Tim Düsterhus >
Hi Tim, This socket gives full admin access to HAProxy without being in the configuration, so it might surprise the user if it's there after an upgrade, it should really be configurable. But I agree that it could be nice to have it by default. Regarding the overriding of ExecStart, I disagree. In my opinion this is a confusing solution for the user, when doing that the user won't have the update of the unit file in the package. Lots of people still prefer /etc/default/ and /etc/sysconfig/ to add some options. In this case we could indeed use the EXTRAOPTS variable, and add "EXTRAOPTS=-S /run/haproxy-master.sock" at the end of the Environment line. Regards, -- William Lallemand
