Re: PATCH: Configurable file mode mask

Hi Michael, On 4/6/18 10:20 AM, Michael Paquier wrote: > On Fri, Apr 06, 2018 at 09:15:15AM -0400, Stephen Frost wrote: >> I'll reply to David's last email (where the latest set of patches were >> included) with my comments/suggestions and I expect we'll be able to get >> those addressed today

Re: PATCH: Configurable file mode mask

Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > (If not, this bodes ill for the Windows results.) Ah, there go the Windows builds... Missing a #else clause where we should have one for those systems, will fix. Thanks! Stephen signature.asc Description: PGP signature

Re: PATCH: Configurable file mode mask

Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=culicidae=2018-04-07%2021%3A50%3A02 > > > Yes, I'm taking a look at it. > > Since other animals

Re: PATCH: Configurable file mode mask

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=culicidae=2018-04-07%2021%3A50%3A02 > Yes, I'm taking a look at it. Since other animals are coming back successfully, my first suspicion lights on

Re: PATCH: Configurable file mode mask

Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > Time to watch the buildfarm, > > That didn't take long. > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=culicidae=2018-04-07%2021%3A50%3A02 Yes, I'm taking a look at it. Thanks! Stephen

Re: PATCH: Configurable file mode mask

Stephen Frost writes: > Time to watch the buildfarm, That didn't take long. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=culicidae=2018-04-07%2021%3A50%3A02 (I'm really starting to get a bit upset at the amount of stuff that's being pushed in on the very last

Re: PATCH: Configurable file mode mask

On 4/7/18 5:49 PM, Stephen Frost wrote: > * David Steele (da...@pgmasters.net) wrote: >> >> OK, one last review. I did't make any code changes, but I improved some >> comments, added documentation and fixed a test. > > Thanks! I took those and then added a bit more commentary around the >

Re: PATCH: Configurable file mode mask

David, * David Steele (da...@pgmasters.net) wrote: > On 4/6/18 10:22 PM, Stephen Frost wrote: > > * David Steele (da...@pgmasters.net) wrote: > >> On 4/6/18 6:04 PM, David Steele wrote: > >>> On 4/6/18 3:02 PM, Stephen Frost wrote: > > - Further discussion in the commit messages > >>> >

Re: PATCH: Configurable file mode mask

Hi Stephen, On 4/6/18 10:22 PM, Stephen Frost wrote: > > * David Steele (da...@pgmasters.net) wrote: >> On 4/6/18 6:04 PM, David Steele wrote: >>> On 4/6/18 3:02 PM, Stephen Frost wrote: - Further discussion in the commit messages >>> >>> Agreed, these need some more work.  I'm happy

Re: PATCH: Configurable file mode mask

David, * David Steele (da...@pgmasters.net) wrote: > On 4/6/18 6:04 PM, David Steele wrote: > >On 4/6/18 3:02 PM, Stephen Frost wrote: > >> > >>- Further discussion in the commit messages > > > >Agreed, these need some more work.  I'm happy to do that but I'll need a > >bit more time.  Have a

Re: PATCH: Configurable file mode mask

On 4/6/18 6:04 PM, David Steele wrote: On 4/6/18 3:02 PM, Stephen Frost wrote: - Further discussion in the commit messages Agreed, these need some more work.  I'm happy to do that but I'll need a bit more time.  Have a look at the new patches and I'll work on some better messages. I'm

Re: PATCH: Configurable file mode mask

Hi Stephen, On 4/6/18 3:02 PM, Stephen Frost wrote: Alright, changes I've made, since I got impatient and it didn't seem to make sense to bounce these back to David instead of just making them (I did discuss them with him on the phone today tho, just to be clear). - The PG_FILE_MODE_DEFAULT,

Re: PATCH: Configurable file mode mask

Michael, * Michael Paquier (mich...@paquier.xyz) wrote: > On Fri, Apr 06, 2018 at 09:15:15AM -0400, Stephen Frost wrote: > > I'll reply to David's last email (where the latest set of patches were > > included) with my comments/suggestions and I expect we'll be able to get > > those addressed

Re: PATCH: Configurable file mode mask

David, * David Steele (da...@pgmasters.net) wrote: > The GUC shows the current mode of the data directory, while the > variables in file_perm.c store the mode that should be used to create > new dirs/files. One is certainly based on the other but I thought it > best to split them for clarity.

Re: PATCH: Configurable file mode mask

On Fri, Apr 06, 2018 at 09:15:15AM -0400, Stephen Frost wrote: > I'll reply to David's last email (where the latest set of patches were > included) with my comments/suggestions and I expect we'll be able to get > those addressed today and have a final patch to post tonight, with an > eye towards

Re: PATCH: Configurable file mode mask

Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Thu, Apr 05, 2018 at 12:08:15PM -0400, David Steele wrote: > > On 4/5/18 2:55 AM, Michael Paquier wrote: > >> On Wed, Apr 04, 2018 at 08:03:54PM -0400, David Steele wrote: > >> > >>> Instead I have created variables in file_perm.c >

Re: PATCH: Configurable file mode mask

On Thu, Apr 05, 2018 at 12:08:15PM -0400, David Steele wrote: > On 4/5/18 2:55 AM, Michael Paquier wrote: >> On Wed, Apr 04, 2018 at 08:03:54PM -0400, David Steele wrote: >> >>> Instead I have created variables in file_perm.c >>> that hold the current file create mode, dir create mode, and mode

Re: PATCH: Configurable file mode mask

Hi Michael, On 4/5/18 2:55 AM, Michael Paquier wrote: > On Wed, Apr 04, 2018 at 08:03:54PM -0400, David Steele wrote: > >> Instead I have created variables in file_perm.c >> that hold the current file create mode, dir create mode, and mode mask. >> All call sites use those variables (e.g.

Re: PATCH: Configurable file mode mask

On Wed, Apr 04, 2018 at 08:03:54PM -0400, David Steele wrote: > On 4/2/18 2:28 AM, Michael Paquier wrote: >> The answer is no for me and likely the same for others, but I am raising >> the point for the archives. Should we relax >> check_ssl_key_file_permissions() for group permissions by the way

Re: PATCH: Configurable file mode mask

Hi Michael, On 4/2/18 2:28 AM, Michael Paquier wrote: > > @@ -285,7 +286,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size > request_size, >* returning. >*/ > flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0); > - if ((fd = shm_open(name, flags, 0600))

Re: PATCH: Configurable file mode mask

On Fri, Mar 30, 2018 at 01:27:11PM -0400, David Steele wrote: > I have replaced data_directory_group_access with data_directory_mode. That looks way better. Thanks for considering it. > I decided this made sense to do. It was only a few lines in initdb.c > using a very well established

Re: PATCH: Configurable file mode mask

On 3/29/18 11:04 PM, Michael Paquier wrote: > On Thu, Mar 29, 2018 at 10:59:59AM -0400, David Steele wrote: >> On 3/28/18 1:59 AM, Michael Paquier wrote: >> >>> In pg_backup_tar.c, we still have a 0600 hardcoded. You should use >>> PG_FILE_MODE_DEFAULT there as well. >> >> I'm starting to wonder

Re: PATCH: Configurable file mode mask

On Thu, Mar 29, 2018 at 10:59:59AM -0400, David Steele wrote: > On 3/28/18 1:59 AM, Michael Paquier wrote: >> On Tue, Mar 27, 2018 at 04:21:09PM -0400, David Steele wrote: >>> These updates address Michael's latest review and implement group access >>> for pg_basebackup, pg_receivewal, and

Re: PATCH: Configurable file mode mask

On 3/28/18 1:59 AM, Michael Paquier wrote: > On Tue, Mar 27, 2018 at 04:21:09PM -0400, David Steele wrote: >> These updates address Michael's latest review and implement group access >> for pg_basebackup, pg_receivewal, and pg_recvlogical. A new internal >> GUC, data_directory_group_access,

Re: PATCH: Configurable file mode mask

On Tue, Mar 27, 2018 at 04:21:09PM -0400, David Steele wrote: > These updates address Michael's latest review and implement group access > for pg_basebackup, pg_receivewal, and pg_recvlogical. A new internal > GUC, data_directory_group_access, allows remote processes to determine > the correct

Re: PATCH: Configurable file mode mask

On 3/20/18 11:14 PM, Michael Paquier wrote: > On Tue, Mar 20, 2018 at 05:44:22PM -0400, Stephen Frost wrote: >> * David Steele (da...@pgmasters.net) wrote: >>> On 3/16/18 11:12 AM, Stephen Frost wrote: >>> It seems to me that pg_basebackup and pg_receivexlog should have a -g >>> option to control

Re: PATCH: Configurable file mode mask

On Fri, Mar 23, 2018 at 12:26:47PM -0400, David Steele wrote: > I've attached a patch that integrates my tests with the current tests. > If you don't think they are worth adding then I'll just drop them from > my patchset. It seems to me that those tests have values (we can add more tests for

Re: PATCH: Configurable file mode mask

Hi Peter, On 3/23/18 10:36 AM, Peter Eisentraut wrote: > I have committed a basic pg_resetwal test suite as part of another > patch, so please adjust accordingly. > > It looks to me like your pg_resetwal tests contain a bit of useless > copy-and-paste. For example, you are not using use Config,

Re: PATCH: Configurable file mode mask

I have committed a basic pg_resetwal test suite as part of another patch, so please adjust accordingly. It looks to me like your pg_resetwal tests contain a bit of useless copy-and-paste. For example, you are not using use Config, nor $tempdir, and you run $node->stop right after $node->start.

Re: PATCH: Configurable file mode mask

On Tue, Mar 20, 2018 at 05:44:22PM -0400, Stephen Frost wrote: > * David Steele (da...@pgmasters.net) wrote: >> On 3/16/18 11:12 AM, Stephen Frost wrote: >> It seems to me that pg_basebackup and pg_receivexlog should have a -g >> option to control the mode of the files that they write to disk (not

Re: PATCH: Configurable file mode mask

David, * David Steele (da...@pgmasters.net) wrote: > On 3/16/18 11:12 AM, Stephen Frost wrote: > > > >>> Visibly there would be no need for a -g switch in > >>> pg_basebackup as it is possible to guess from the received untar'ed > >>> files what should be the permissions of the data based on

Re: PATCH: Configurable file mode mask

On 3/16/18 11:12 AM, Stephen Frost wrote: > >>> Visibly there would be no need for a -g switch in >>> pg_basebackup as it is possible to guess from the received untar'ed >>> files what should be the permissions of the data based on what is >>> received in pg_basebackup.c. It would also be

Re: PATCH: Configurable file mode mask

On Fri, Mar 16, 2018 at 11:12:44AM -0400, Stephen Frost wrote: > Given that we're already, as I recall, including the owner/group of the > file being backed up through pg_basebackup in the tarball, it seems like > we should be including whatever the current dir/file mode is too. Those > files may

Re: PATCH: Configurable file mode mask

Greetings David, Michael, all, * David Steele (da...@pgmasters.net) wrote: > On 3/15/18 3:17 AM, Michael Paquier wrote: > > On Wed, Mar 14, 2018 at 02:08:19PM -0400, David Steele wrote: > > > > When taking a base backup from a data folder which has group access, > > then the tar data, as well as

Re: PATCH: Configurable file mode mask

On 3/15/18 3:17 AM, Michael Paquier wrote: > On Wed, Mar 14, 2018 at 02:08:19PM -0400, David Steele wrote: > > When taking a base backup from a data folder which has group access, > then the tar data, as well as the untar'ed data, are still using > 0600 as umask for files and 0700 for folders.

Re: PATCH: Configurable file mode mask

On Wed, Mar 14, 2018 at 02:08:19PM -0400, David Steele wrote: > OK, that being the case a new patch set is attached that sets the mode > of postmaster.pid the same as other files in PGDATA. +1. > I also cleaned up / corrected / added comments in various places. Patches 1 and 2 look fine to me.

Re: PATCH: Configurable file mode mask

On Wed, Mar 14, 2018 at 02:15:03PM -0400, David Steele wrote: > Do you mean a separate patch in this patch set, or a separate patch > entirely? 02 depends on this logic, so I guess you mean create a new > patch between 01 and 02? Yes, one new patch that which can be applied on top of 1. > Are

Re: PATCH: Configurable file mode mask

Hi Michael, On 3/13/18 9:31 PM, Michael Paquier wrote: > On Tue, Mar 13, 2018 at 12:19:07PM -0400, David Steele wrote: >> I'll attach new patches in a reply to [1] once I have made the changes >> Tom requested. > > Cool, thanks for your patience. Looking forward to seeing those. I'll > spend

Re: PATCH: Configurable file mode mask

Hi, On 3/13/18 12:13 PM, Stephen Frost wrote: > * David Steele (da...@pgmasters.net) wrote: >> On 3/13/18 11:00 AM, Tom Lane wrote: >>> >>> FWIW, I took a quick look through this patch and don't have any problem >>> with the approach, which appears to be "use the data directory's >>> startup-time

Re: PATCH: Configurable file mode mask

On Tue, Mar 13, 2018 at 12:19:07PM -0400, David Steele wrote: > I'll attach new patches in a reply to [1] once I have made the changes > Tom requested. Cool, thanks for your patience. Looking forward to seeing those. I'll spend time on 0003 at the same time. Let's also put the additional umask

Re: PATCH: Configurable file mode mask

On Tue, Mar 13, 2018 at 01:28:17PM -0400, Tom Lane wrote: > David Steele writes: >> On 3/12/18 3:28 AM, Michael Paquier wrote: >>> In pg_rewind and pg_resetwal, isn't that also a portion which is not >>> necessary without the group access feature? > >> These seem like a good

Re: PATCH: Configurable file mode mask

Chapman, * Chapman Flack (c...@anastigmatix.net) wrote: > I'm not advocating the Sisyphean task of having PG incorporate > knowledge of all those possibilities. I'm advocating the conservative > approach: have PG be that well-behaved application that those extended > semantics are generally all

Re: PATCH: Configurable file mode mask

On 03/13/2018 02:47 PM, Tom Lane wrote: > Well, to be blunt, what we target is POSIX-compatible systems. If > you're telling us to worry about non-POSIX filesystem semantics, > and your name is not Microsoft, it's going to be a hard sell. > We have enough to do just keeping up with that scope of

Re: PATCH: Configurable file mode mask

On 03/13/2018 01:50 PM, Stephen Frost wrote: > Yes, that's true, but PG has never paid attention to POSIX ACLs or Linux > capabilities. Changing it to do so is quite a bit beyond the scope... I think we're largely in agreement here, as my aim was not to advocate that PG should work harder to

Re: PATCH: Configurable file mode mask

Greetings Chapman, * Chapman Flack (c...@anastigmatix.net) wrote: > On 03/13/2018 10:40 AM, Stephen Frost wrote: > > ... Ultimately, the default which makes sense here isn't a > > one-size-fits-all but is system dependent and the administrator should > > be able to choose what permissions they

Re: PATCH: Configurable file mode mask

On 03/13/2018 10:40 AM, Stephen Frost wrote: > * Michael Paquier (mich...@paquier.xyz) wrote: >> Well, one thing is that the current checks in the postmaster make sure >> that a data folder is never using anything else than 0700. From a >> security point of view, making it possible to allow a

Re: PATCH: Configurable file mode mask

David Steele writes: > On 3/12/18 3:28 AM, Michael Paquier wrote: >> In pg_rewind and pg_resetwal, isn't that also a portion which is not >> necessary without the group access feature? > These seem like a good idea to me with or without patch 03. Some of our > front-end

Re: PATCH: Configurable file mode mask

Hi Michael, On 3/12/18 3:28 AM, Michael Paquier wrote: > On Fri, Mar 09, 2018 at 01:51:14PM -0500, David Steele wrote: >> How about a GUC that enforces one mode or the other on startup? Default >> would be 700. The GUC can be set automatically by initdb based on the >> -g option. We had this

Re: PATCH: Configurable file mode mask

Greetings, * David Steele (da...@pgmasters.net) wrote: > On 3/13/18 11:00 AM, Tom Lane wrote: > > Stephen Frost writes: > >> * Michael Paquier (mich...@paquier.xyz) wrote: > >>> If the problem is parsing, it could as well be more portable to put that > >>> in the control

Re: PATCH: Configurable file mode mask

On 3/13/18 11:00 AM, Tom Lane wrote: > Stephen Frost writes: >> * Michael Paquier (mich...@paquier.xyz) wrote: >>> If the problem is parsing, it could as well be more portable to put that >>> in the control file, no? > >> Then we'd need a tool to allow changing it for people

Re: PATCH: Configurable file mode mask

Stephen Frost writes: > * Michael Paquier (mich...@paquier.xyz) wrote: >> If the problem is parsing, it could as well be more portable to put that >> in the control file, no? > Then we'd need a tool to allow changing it for people who do want to > change it. There's no

Re: PATCH: Configurable file mode mask

On 3/13/18 2:46 AM, Michael Paquier wrote: > On Mon, Mar 12, 2018 at 03:14:13PM -0400, Stephen Frost wrote: >> We already had a discussion about having a GUC for this and concluded, >> rightly in my view, that it's not sensible to have since we don't want >> all of the various tools having to read

Re: PATCH: Configurable file mode mask

Michael, * Michael Paquier (mich...@paquier.xyz) wrote: > On Mon, Mar 12, 2018 at 03:14:13PM -0400, Stephen Frost wrote: > > We already had a discussion about having a GUC for this and concluded, > > rightly in my view, that it's not sensible to have since we don't want > > all of the various

Re: PATCH: Configurable file mode mask

On Mon, Mar 12, 2018 at 03:14:13PM -0400, Stephen Frost wrote: > We already had a discussion about having a GUC for this and concluded, > rightly in my view, that it's not sensible to have since we don't want > all of the various tools having to read and parse out postgresql.conf. If the problem

Re: PATCH: Configurable file mode mask

Michael, David, * Michael Paquier (mich...@paquier.xyz) wrote: > On Fri, Mar 09, 2018 at 01:51:14PM -0500, David Steele wrote: > > How about a GUC that enforces one mode or the other on startup? Default > > would be 700. The GUC can be set automatically by initdb based on the > > -g option. We

Re: PATCH: Configurable file mode mask

On Fri, Mar 09, 2018 at 01:51:14PM -0500, David Steele wrote: > How about a GUC that enforces one mode or the other on startup? Default > would be 700. The GUC can be set automatically by initdb based on the > -g option. We had this GUC originally, but since the front-end tools > can't read it

Re: PATCH: Configurable file mode mask

Hi Michael, On 3/7/18 8:51 PM, Michael Paquier wrote: > On Wed, Mar 07, 2018 at 10:56:32AM -0500, David Steele wrote: >> On 3/6/18 10:04 PM, Michael Paquier wrote: >>> Seems like you forgot to re-add the chmod calls in initdb.c. >> >> Hmmm, I thought we were talking about moving the position of

Re: PATCH: Configurable file mode mask

On Wed, Mar 07, 2018 at 10:56:32AM -0500, David Steele wrote: > On 3/6/18 10:04 PM, Michael Paquier wrote: >> Seems like you forgot to re-add the chmod calls in initdb.c. > > Hmmm, I thought we were talking about moving the position of umask(). > > I don't think the chmod() calls are needed

Re: PATCH: Configurable file mode mask

On 3/6/18 10:04 PM, Michael Paquier wrote: > On Tue, Mar 06, 2018 at 01:32:49PM -0500, David Steele wrote: >> On 3/5/18 10:46 PM, Michael Paquier wrote: > >>> Those two are separate issues. Could you begin a new thread on the >>> matter? This will attract more attention. >> >> OK, I'll move it

Re: PATCH: Configurable file mode mask

On Tue, Mar 06, 2018 at 01:32:49PM -0500, David Steele wrote: > On 3/5/18 10:46 PM, Michael Paquier wrote: > Ah, I see what you mean now. Fixed using follow_fast. This variant > claims to be faster and it doesn't matter if files are occasionally > checked twice. Fine for me. I can see this

Re: PATCH: Configurable file mode mask

On 3/5/18 10:46 PM, Michael Paquier wrote: > On Mon, Mar 05, 2018 at 03:07:20PM -0500, David Steele wrote: >> On 2/28/18 2:28 AM, Michael Paquier wrote: >>> On Tue, Feb 27, 2018 at 03:52:32PM -0500, David Steele wrote: >>> I don't quite understand here. I have no objection into extending >>>

Re: PATCH: Configurable file mode mask

On Mon, Mar 05, 2018 at 03:07:20PM -0500, David Steele wrote: > On 2/28/18 2:28 AM, Michael Paquier wrote: >> On Tue, Feb 27, 2018 at 03:52:32PM -0500, David Steele wrote: >> I don't quite understand here. I have no objection into extending >> setup_cluster() with a group_access argument so as

Re: PATCH: Configurable file mode mask

On 3/5/18 8:03 PM, Michael Paquier wrote: > On Mon, Mar 05, 2018 at 05:11:29PM -0500, Tom Lane wrote: >> David Steele writes: >>> On 2/28/18 2:28 AM, Michael Paquier wrote: That's basically a recursive chmod, so chmod_recursive is more adapted? I could imagine that

Re: PATCH: Configurable file mode mask

On Mon, Mar 05, 2018 at 05:11:29PM -0500, Tom Lane wrote: > David Steele writes: >> On 2/28/18 2:28 AM, Michael Paquier wrote: >>> That's basically a recursive chmod, so chmod_recursive is more adapted? >>> I could imagine that this is useful as well for removing group >>>

Re: PATCH: Configurable file mode mask

On 3/5/18 5:51 PM, Tom Lane wrote: > David Steele writes: >> On 3/5/18 5:11 PM, Tom Lane wrote: >>> David Steele writes: I'm not sure what the protocol for introducing a new Perl module is? I couldn't find packages for the major OSes. Are we

Re: PATCH: Configurable file mode mask

David Steele writes: > On 3/5/18 5:11 PM, Tom Lane wrote: >> David Steele writes: >>> I'm not sure what the protocol for introducing a new Perl module is? I >>> couldn't find packages for the major OSes. Are we OK with using CPAN? >> I don't think

Re: PATCH: Configurable file mode mask

Tom, all, * Tom Lane (t...@sss.pgh.pa.us) wrote: > David Steele writes: > > On 2/28/18 2:28 AM, Michael Paquier wrote: > >> That's basically a recursive chmod, so chmod_recursive is more adapted? > >> I could imagine that this is useful as well for removing group > >>

Re: PATCH: Configurable file mode mask

Hi Tom, On 3/5/18 5:11 PM, Tom Lane wrote: > David Steele writes: >> On 2/28/18 2:28 AM, Michael Paquier wrote: >>> That's basically a recursive chmod, so chmod_recursive is more adapted? >>> I could imagine that this is useful as well for removing group >>> permissions, so

Re: PATCH: Configurable file mode mask

David Steele writes: > On 2/28/18 2:28 AM, Michael Paquier wrote: >> That's basically a recursive chmod, so chmod_recursive is more adapted? >> I could imagine that this is useful as well for removing group >> permissions, so the new mode could be specified as an argument. >

Re: PATCH: Configurable file mode mask

On 3/1/18 11:18 PM, Michael Paquier wrote: > > Based on my recent lookup at code level for this feature, the patch for > pg_resetwal (which could have been discussed on its own thread as well), > would be fine for commit. The thing could be extended a bit more but > there is nothing opposing

Re: PATCH: Configurable file mode mask

On 2/28/18 2:28 AM, Michael Paquier wrote: > On Tue, Feb 27, 2018 at 03:52:32PM -0500, David Steele wrote: >> On 1/30/18 3:01 AM, Michael Paquier wrote: > >>> +command_ok( >>> + ['chmod', "-R", 'g+rX', "$pgdata"], >>> + 'add group perms to PGDATA'); >>> >>> That would blow up on Windows. You

Re: PATCH: Configurable file mode mask

On Thu, Mar 01, 2018 at 09:32:58PM -0500, David Steele wrote: > On 3/1/18 9:04 PM, Andres Freund wrote: >> 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,

Re: PATCH: Configurable file mode mask

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

Re: PATCH: Configurable file mode mask

Hi, 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. E.g. here it's far from obvious why something

Re: PATCH: Configurable file mode mask

On Tue, Feb 27, 2018 at 03:52:32PM -0500, David Steele wrote: > On 1/30/18 3:01 AM, Michael Paquier wrote: > The purpose of this test to be ensure nothing else is in the directory. > As the tests get more complex I think keeping track of the state will be > important. In other words, this is

Re: PATCH: Configurable file mode mask

Hi Michael, Thanks for having a look at the patches. On 1/30/18 3:01 AM, Michael Paquier wrote: > On Mon, Jan 29, 2018 at 04:29:08PM -0500, David Steele wrote: >> >> Adds a *very* basic test suite for pg_resetwal. I was able to make this >> utility core dump (floating point errors) pretty

Re: PATCH: Configurable file mode mask

On Mon, Jan 29, 2018 at 04:29:08PM -0500, David Steele wrote: > OK, that being the case I have piggy-backed on the current pg_upgrade > tests in the same way that --wal-segsize did. > > There are now three patches: > > 1) 01-pgresetwal-test > > Adds a *very* basic test suite for pg_resetwal. I

Re: PATCH: Configurable file mode mask

On 1/19/18 4:43 PM, Peter Eisentraut wrote: > On 1/19/18 14:07, David Steele wrote: >> 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

Re: PATCH: Configurable file mode mask

On Tue, Jan 23, 2018 at 10:48:07PM -0500, David Steele wrote: > Sorry - it means "level of effort". I was trying to get an idea if it > is something that could be available in the PG11 development cycle, or > if I should be looking at other ways to get the testing for this patch > completed. I

Re: PATCH: Configurable file mode mask

On 1/23/18 9:22 PM, Michael Paquier wrote: > On Tue, Jan 23, 2018 at 09:18:51AM -0500, David Steele wrote: >> On 1/20/18 5:47 PM, Michael Paquier wrote: >>> Making this possible would require first some >>> refactoring of PostgresNode.pm so as a node is aware of the binary paths >>> it uses to be

Re: PATCH: Configurable file mode mask

On Tue, Jan 23, 2018 at 09:18:51AM -0500, David Steele wrote: > On 1/20/18 5:47 PM, Michael Paquier wrote: >> Making this possible would require first some >> refactoring of PostgresNode.pm so as a node is aware of the binary paths >> it uses to be able to manipulate multiple instances with

Re: PATCH: Configurable file mode mask

On 1/23/18 09:33, David Steele wrote: > What if I update pg_upgrade/test.sh to optionally allow group > permissions and we configure an animal to test it if it gets committed? > > It's not ideal, I know, but it would get the permissions patch over the > line and is consistent with how we

Re: PATCH: Configurable file mode mask

On 1/19/18 16:54, Alvaro Herrera wrote: > If the only problem is that buildfarm would run tests twice, then I > think we should just press forward with this regardless of that: it > seems a chicken-and-egg problem, because buildfarm cannot be upgraded to > use some new test method if the method

Re: PATCH: Configurable file mode mask

On 1/23/18 9:26 AM, Tom Lane wrote: > David Steele writes: >> Unless I read it wrong the buildfarm is not doing cross-version >> upgrades, but a developer/user can do so manually using the same script? > > The buildfarm isn't doing that *by default*, but Andrew has at least

Re: PATCH: Configurable file mode mask

David Steele writes: > Unless I read it wrong the buildfarm is not doing cross-version > upgrades, but a developer/user can do so manually using the same script? The buildfarm isn't doing that *by default*, but Andrew has at least one critter configured to do so (crake I

Re: PATCH: Configurable file mode mask

On 1/20/18 5:47 PM, Michael Paquier wrote: > On Fri, Jan 19, 2018 at 06:54:23PM -0300, Alvaro Herrera wrote: >> Peter Eisentraut wrote: >> If the only problem is that buildfarm would run tests twice, then I >> think we should just press forward with this regardless of that: it >> seems a

Re: PATCH: Configurable file mode mask

On 1/19/18 4:43 PM, Peter Eisentraut wrote: > On 1/19/18 14:07, David Steele wrote: >> 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

Re: PATCH: Configurable file mode mask

On Fri, Jan 19, 2018 at 06:54:23PM -0300, Alvaro Herrera wrote: > Peter Eisentraut wrote: > If the only problem is that buildfarm would run tests twice, then I > think we should just press forward with this regardless of that: it > seems a chicken-and-egg problem, because buildfarm cannot be

Re: PATCH: Configurable file mode mask

Peter Eisentraut wrote: > On 1/19/18 14:07, David Steele wrote: > > 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

Re: PATCH: Configurable file mode mask

On 1/19/18 14:07, David Steele wrote: > 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

Re: PATCH: Configurable file mode mask

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

Re: PATCH: Configurable file mode mask

On 1/10/18 12:37, David Steele wrote: > 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. Works for me. -- Peter Eisentraut

Re: PATCH: Configurable file mode mask

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

Re: PATCH: Configurable file mode mask

David, * David Steele (da...@pgmasters.net) wrote: > On 1/8/18 8:58 PM, Peter Eisentraut wrote: > > On 1/3/18 08:11, Robert Haas wrote: > >> On Tue, Jan 2, 2018 at 11:43 AM, David Steele wrote: > > I think MakeDirectory() is a good wrapper, but isn't >

Re: PATCH: Configurable file mode mask

On 1/8/18 8:58 PM, Peter Eisentraut wrote: > On 1/3/18 08:11, Robert Haas wrote: >> On Tue, Jan 2, 2018 at 11:43 AM, David Steele wrote: > I think MakeDirectory() is a good wrapper, but isn't MakeDirectoryPerm() sort of silly? >>> >>> There's one place in the backend

Re: PATCH: Configurable file mode mask

On Tue, Jan 2, 2018 at 11:43 AM, David Steele wrote: >> > I think MakeDirectory() is a good wrapper, but isn't >> MakeDirectoryPerm() sort of silly? > > There's one place in the backend (storage/ipc/ipc.c) that sets non-default > directory permissions. This function is

Re: PATCH: Configurable file mode mask

Hi Robert, Thanks for looking at the patches. On 12/31/17 1:27 PM, Robert Haas wrote: On Thu, Dec 28, 2017 at 2:36 PM, David Steele wrote: Attached is a new patch set that should address various concerns raised in this thread. 1) group-access-v3-01-mkdir.patch

Re: Re: PATCH: Configurable file mode mask

On 3/21/17 2:02 PM, David Steele wrote: On 3/18/17 3:57 PM, Robert Haas wrote: I think Tom's concerns about people doing insecure stuff are excessive.  People can do insecure stuff no matter what we do, and trying to prevent them often leads to them doing even-more-insecure stuff.  That having