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. The new umask calls in pg_rewind and
pg_resetwal could be split into a separate patch but that's a minor
issue.

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. Is that an expected
behavior? I would have imagined that sendFileWithContent() and
_tarWriteDir() should enforce the file mode to have group access if the
cluster has been initialized to work as such. Still as this is a
feature aimed at being used for custom backups, that's not really a
blocker I guess. 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 necessary to change the
permissions of pg_wal as this is created before receiving any files.

Speaking of which, we may want to switch the values used for st_mode to
what file_perm.h is giving in basebackup.c?

We should also replace the hardcoded 0700 value in pg_backup_directory.c
by what file_perm.h offers? I would recommend to not touch at mkdtemp.c
as this comes from NetBSD.

+=item $node->group_access()
+
+Does the data dir allow group access?
+
Nit: s/dir/directory/.

Indentation is weird in PostgresNode.pm for some of the chmod calls
(tabs not spaces please).
--
Michael