Re: PATCH: Configurable file mode mask
David Steele <david@pgmasters.net>
From: David Steele <david@pgmasters.net>
To: Michael Paquier <michael.paquier@gmail.com>,
Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
Robert Haas <robertmhaas@gmail.com>,
"Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>,
Tom Lane <tgl@sss.pgh.pa.us>, Stephen Frost <sfrost@snowman.net>,
Pg Hackers <pgsql-hackers@postgresql.org>,
Adam Brightwell <adam.brightwell@crunchydata.com>
Date: 2018-01-19T19:07:54Z
Lists: pgsql-hackers
Attachments
- group-access-v5-01-mkdir.patch (text/plain) patch v5
- group-access-v5-02-group.patch (text/plain) patch v5
On 1/19/18 3:08 AM, Michael Paquier wrote: > On Wed, Jan 10, 2018 at 03:19:46PM -0300, Alvaro Herrera wrote: >> David Steele wrote: >>> On 1/8/18 8:58 PM, Peter Eisentraut wrote: >> >>>> Yeah, I didn't like this aspect when this patch was originally >>>> submitted. We want to keep the code legible for future new >>>> contributors. Having these generic-sounding but specific-in-purpose >>>> wrapper functions can be pretty confusing. Let's use mkdir() when it's >>>> the appropriate function, and let's figure out a different name for >>>> "make a data directory subdirectory in a secure and robust way". >> >>> How about MakeDirectoryDefaultPerm()? That's what I'll go with if I >>> don't hear any other ideas. The single call to MakeDirectoryPerm() will >>> be reverted to mkdir() and I'll remove the function. >> >> I'd go with MakeDirectory, documenting exactly what it does and why, and >> be done with it. If your new function satisfies future users, great; if >> not, it can be patched (or not) once we know exactly what these callers >> need. > > After going through the thread, I would vote for making things simple by > just using MakeDirectory() and document precisely what it does. Anyway, > there is as well the approach of using MakeDirectoryDefaultPerm(), so > I'll be fine with the decision you make, David. The patchis moved to > "waiting on author". I ended up with MakeDirectoryDefaultPerm(), but I'm flexible. Attached is a new patch set that also adds the following to patch 02. 1) Move permission constants to a new file in common so they can be shared with the front end. 2) Update all client programs that write to PGDATA: initdb, pg_ctl, pg_resetwal, pg_rewind, pg_upgrade. 3) Add tests for initdb, pg_ctl, and pg_rewind. I have yet to add tests for pg_rewindwal and pg_upgrade. pg_rewindwal doesn't *have* any tests as far as I can tell and pg_upgrade has tests in a shell script -- it's not clear how I would extend it or reuse the Perl code for perm testing. Does anyone have suggestions on tests for pg_resetwal and pg_upgrade? Should I start from scratch? Thanks, -- -David david@pgmasters.net
Commits
-
Allow group access on PGDATA
- c37b3d08ca68 11.0 landed
-
Refactor dir/file permissions
- da9b580d8990 11.0 landed
-
Revert "Add basic TAP test setup for pg_upgrade"
- 58ffe141eb37 11.0 cited
-
Add basic TAP test setup for pg_upgrade
- f41e56c76e39 11.0 cited