Re: PATCH: Configurable file mode mask

From: David Steele <david(at)pgmasters(dot)net>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Configurable file mode mask
Date: 2018-03-02 02:32:58
Message-ID: bfe6dbce-ec0b-e1ce-99f5-3e63e31d0fb9@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Andres,

On 3/1/18 9:04 PM, Andres Freund wrote:
> On 2018-02-27 15:52:32 -0500, David Steele wrote:
>> Thanks for having a look at the patches.
>
> I'd personally appreciate if you'd add commit messages to the individual
> patches in a series. A brief explanation why something is done is good
> enough.
>
> I'd suggest just using git format-patch -v to format series.

Sure, I've been meaning to switch over to this format and just haven't
gotten around to it yet. I do recognize the value, however, and will do
it going forward.

> Independent of that, I'm not entirely clear on what the status of this
> patch is, without rereading the thread. Could you summarize?

Good question. This patch has been around for a year and has gone
though a number of iterations.

In summary, we started with a more rigid design that required a GUC to
enable group privs (actually, at first it was any mode you wanted).
Through feedback from Tom and others we realized that the front-end
tools could not read the GUCs and it would probably be best based on the
perms of PGDATA, as long as they were 700 or 750.

We decided the patch was too big, so broke it up a bit and got some of
the infrastructure committed in 2017-09 [1].

In 2018-01 we brought in a unified patch set that built on the ideas
from 2017-03. During that CF we taught all the front-end tools to check
PGDATA for permissions and wrote a lot of tests.

The current incarnation is a refinement with input from Michael and a
different split in the patches.

> E.g. here it's far from obvious why something is done with
> pg_resetwal.
[reordered by me for clarity]

Any front-end tool that writes into PGDATA is affected by this patch
because mode must be set correctly.

pg_resetwal had no tests, so I wrote a basic test suite to base the
group permissions tests on. I included the basic test suite as a
separate patch because I felt it should be committed even if the main
feature wasn't. It's far from a comprehensive test suite, but certainly
better than what we have currently.

Does that clear things up?

--
-David
david(at)pgmasters(dot)net

[1] https://commitfest.postgresql.org/14/1262/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2018-03-02 02:33:16 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Previous Message Daniel Gustafsson 2018-03-02 02:29:15 Re: [HACKERS] Support for Secure Transport SSL library on macOS as OpenSSL alternative