Re: current_logfiles not following group access and instead follows log_file_mode permissions

Greetings,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Michael Paquier (michael(at)paquier(dot)xyz) wrote:
> >> On Tue, Jan 15, 2019 at 10:53:30AM -0500, Tom Lane wrote:
> >>> On reflection, maybe the problem is not that we're giving the file
> >>> the wrong permissions, but that we're putting it in the wrong place?
>
> > I'm not really sure putting it into the logfile directory is such a hot
> > idea as users might have set up external log file rotation of files in
> > that directory. Of course, in that case they'd probably signal PG right
> > afterwards and PG would go write out a new file, but it still seems
> > pretty awkward. I'm not terribly against solving this issue that way
> > either though, but I tend to think the originally proposed patch is more
> > sensible.
>
> I dunno, I think that the current design was made without any thought
> whatsoever about the log-files-outside-the-data-directory case. If
> you're trying to set things up that way, it's because you want to give
> logfile read access to people who shouldn't be able to look into the
> data directory proper. That makes current_logfiles pretty useless
> to such people, as it's now designed.

... or you just want to move the log files to a more sensible location
than the data directory. The justification for log_file_mode existing
is because you might want to have log files with different privileges,
but that's quite a different thing.

> Now, if the expectation is that current_logfiles is just an internal
> working file that users shouldn't access directly, then this argument
> is wrong --- but then why is it documented in user-facing docs?

I really couldn't say why it's documented in the user-facing docs, and
for my 2c I don't really think it should be- there's a function to get
that information. Sprinkling the data directory with files for users to
access directly doesn't exactly fit my view of what a good API looks
like.

The fact that there isn't any discussion about where that file actually
lives does make me suspect you're right that log files outside the data
directory wasn't really contemplated.

> If we're going to accept the patch as-is, then it logically follows
> that we should de-document current_logfiles, because we're taking the
> position that it's an internal temporary file not meant for user access.

... and hopefully we'd get rid of it one day entirely.

> I don't really believe your argument about log rotation: a rotator
> would presumably be configured either to pay attention to file name
> patterns (which current_logfiles wouldn't match) or to file age
> (which current_logfiles shouldn't satisfy either, since it's always
> rewritten when we switch logfiles).

Yes, a good pattern would avoid picking up on this file and most are
configured that way (though they are maybe not as specific as you might
think- the default here is just /var/log/postgresql/*.log).

> If we wanted to worry about that case, a possible solution is to make the
> current_logfiles pathname user-configurable so it could be put in some
> third directory. But I think that adds more complexity than is justified
> --- and not just for us, but for programs trying to find and use
> current_logfiles.

I'd much rather move to get rid of that file rather than increase its
visability- programs should be using the provided function.

Thanks!

Stephen