Thanks for your answers.

On Thu, Jan 09, 2020 at 06:17:29AM +0100, Willy Tarreau wrote:
> I had a look at how this currently works and am embarrassed by both the
> patch and the way things currently work. Indeed, the patch only makes
> use of the mode, uid and gid from the unix-bind statement and silently
> ignores the path. It would be tempting to decide that since it's a unix
> socket we should enfore all of unix-bind settings to the stats socket,
> but then the problem caused by unix-bind is that the path component is
> a mandatory prefix that is prepended before all socket paths. So if we
> enforce the path we'll break all configs already using unix-bind.
> 
> I tend to think that at some point we could decide to purposely break
> some of this stuff so that the stats socket is not special at all, but
> I fear that some configs could not be expressed anymore due to this,
> and typically users will place the stats socket into a location outside
> of the chroot so that it cannot be accessed by accident by the process.
> That's even more true for the master socket where the path is specified
> on the command line, regardless of any global setting.
> 
> So maybe in the end your approach is the most reasonable one. However
> in this case we should explicitly state in the doc what settings from
> the unix-bind directive are reused by the stats/master socket, because
> to be transparent, I wasn't aware of the other ones beyond "prefix" and
> that's the first thing I tried and was surprized not to see it work as
> I imagined it would.
> 
> What do you think ?

I was indeed puzzled for the exact same reasons you mentioned but I
badly felt lazy expressing them (shame on me).
I also think we should consider those sockets as any others, and so
take into account the prefix as well. This will make everything clearer
from the user point of view, even though this will come with a breaking
change. Indeed, when I started using master socket, I was a bit
frustrated to have to replicate the rights settings in two different
places instead of having one single coherent one.

On Thu, Jan 09, 2020 at 12:00:43PM +0100, William Lallemand wrote:
> In my opinion that's not a good idea for the master CLI as it shouldn't depend
> on the configuration file. As a reminder the master CLI must work when a
> configuration file is corrupted, in "waitpid" mode. In this mode there is no
> configuration and the master CLI is running only with the configuration used 
> in
> argument.

that's a good subject to talk about. While looking at this I also
wondered why it was not possible to configure it in the config file; to my
knowledge there is no explanation about it in the doc, nor in the code,
but I might have missed something.
I did not know about those concerns around being able to access CLI even
if the config file is corrupted, thanks for the clarification.

That being said, it makes my patch less likely to be acceptable, but I
still confirm the user point of view where:
- you don't get why you cannot configure the master CLI in the
  configuration file; that's probably a point to address in the doc,
  since the configuration doc mentioned the master cli, but you find how
  to set it up in the management doc.
- having to replicate same settings is frustrating even though our
  config is generated
- when reading "stats socket" doc, it is mentioned all settings in
  "bind" are supported, but while reading "unix-bind" it becomes
  confusing whether this will be taken into account or not.

-- 
William

Reply via email to