diff --git a/HISTORY b/HISTORY index 6f6853f7..52d6cf99 100644 --- a/HISTORY +++ b/HISTORY @@ -2,6 +2,8 @@ config: fix parsing of "replication_type"; GitHub #672 (Ian) standby clone: handle missing "postgresql.auto.conf" (Ian) standby clone: add option --recovery-min-apply-delay (Ian) + standby clone: fix data directory permissions handling for + PostgreSQL 11 and later (Ian) repmgrd: prevent termination when local node not available and standby_disconnect_on_failover; GitHub #675 (Ian) repmgrd: ensure reconnect_interval" is correctly handled; diff --git a/dirutil.c b/dirutil.c index 1ec7ed3c..5682572d 100644 --- a/dirutil.c +++ b/dirutil.c @@ -109,9 +109,56 @@ create_dir(const char *path) bool -set_dir_permissions(const char *path) +set_dir_permissions(const char *path, int server_version_num) { - return (chmod(path, 0700) != 0) ? false : true; + struct stat stat_buf; + bool no_group_access = + (server_version_num != UNKNOWN_SERVER_VERSION_NUM) && + (server_version_num < 110000); + /* + * At this point the path should exist, so this check is very + * much just-in-case. + */ + if (stat(path, &stat_buf) != 0) + { + if (errno == ENOENT) + { + log_warning(_("directory \"%s\" does not exist"), path); + } + else + { + log_warning(_("could not read permissions of directory \"%s\""), + path); + log_detail("%s", strerror(errno)); + } + + return false; + } + + /* + * If mode is not 0700 or 0750, attempt to change. + */ + if ((no_group_access == true && (stat_buf.st_mode & (S_IRWXG | S_IRWXO))) + || (no_group_access == false && (stat_buf.st_mode & (S_IWGRP | S_IRWXO)))) + { + /* + * Currently we default to 0700. + * There is no facility to override this directly, + * but the user can manually create the directory with + * the desired permissions. + */ + + if (chmod(path, 0700) != 0) { + log_error(_("unable to change permissions of directory \"%s\""), path); + log_detail("%s", strerror(errno)); + return false; + } + + return true; + } + + /* Leave as-is */ + return true; } @@ -303,7 +350,7 @@ create_pg_dir(const char *path, bool force) switch (check_dir(path)) { case DIR_NOENT: - /* directory does not exist, attempt to create it */ + /* Directory does not exist, attempt to create it. */ log_info(_("creating directory \"%s\"..."), path); if (!create_dir(path)) @@ -314,14 +361,23 @@ create_pg_dir(const char *path, bool force) } break; case DIR_EMPTY: - /* exists but empty, fix permissions and use it */ + /* + * Directory exists but empty, fix permissions and use it. + * + * Note that at this point the caller might not know the server + * version number, so in this case "set_dir_permissions()" will + * accept 0750 as a valid setting. As this is invalid in Pg10 and + * earlier, the caller should call "set_dir_permissions()" again + * when it has the number. + * + * We need to do the permissions check here in any case to catch + * fatal permissions early. + */ log_info(_("checking and correcting permissions on existing directory \"%s\""), path); - if (!set_dir_permissions(path)) + if (!set_dir_permissions(path, UNKNOWN_SERVER_VERSION_NUM)) { - log_error(_("unable to change permissions of directory \"%s\""), path); - log_detail("%s", strerror(errno)); return false; } break; diff --git a/dirutil.h b/dirutil.h index 8284d6e3..49d05700 100644 --- a/dirutil.h +++ b/dirutil.h @@ -35,7 +35,7 @@ typedef enum } PgDirState; extern int mkdir_p(char *path, mode_t omode); -extern bool set_dir_permissions(const char *path); +extern bool set_dir_permissions(const char *path, int server_version_num); extern DataDirState check_dir(const char *path); extern bool create_dir(const char *path); diff --git a/doc/appendix-release-notes.xml b/doc/appendix-release-notes.xml index 8873a61a..6a27b72c 100644 --- a/doc/appendix-release-notes.xml +++ b/doc/appendix-release-notes.xml @@ -46,12 +46,14 @@ Bug fixes - + + - Fix parsing of . GitHub #672. + Configuration: fix parsing of . GitHub #672. + repmgr standby clone: @@ -60,6 +62,14 @@ + + + repmgr standby clone: + in PostgreSQL 11 and later, an existing data directory's permissions + will not be changed to if they are already set to + . + + diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index 3aa9f2c9..0bce499b 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -846,6 +846,16 @@ do_standby_clone(void) break; } + /* + * Do a final check on the data directory permissions - if the user + * is cloning into an existing directory set to 0750, and the server + * is Pg10 or earlier, Pg will refuse to start. We might not have + * known the server version when creating the data directory + * (mainly if cloning from Barman with no upstream connection), hence + * the additional check here. + */ + set_dir_permissions(local_data_directory, source_server_version_num); + /* * TODO: It might be nice to provide an option to have repmgr start the * PostgreSQL server automatically