From 6fbbe2a97a8d8f3a8483eedec446576e32ad668c Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Fri, 2 Mar 2018 14:46:27 +0900 Subject: [PATCH] "standby clone": fix --superuser handling get_superuser_connection() was erroneously using the local node record to connect to as a superuser, which works when registering the primary but obviously not when cloning a standby. Addresses GitHub #380. --- HISTORY | 1 + dbutils.c | 33 +-------------------------------- dbutils.h | 3 --- doc/repmgr-standby-clone.sgml | 29 ++++++++++++++++++++--------- repmgr-action-standby.c | 5 +++-- repmgr-client.c | 22 +++++++++++++++++----- 6 files changed, 42 insertions(+), 51 deletions(-) diff --git a/HISTORY b/HISTORY index 3ec54554..70eecf91 100644 --- a/HISTORY +++ b/HISTORY @@ -6,6 +6,7 @@ repmgr: remove restriction on replication slots when cloning from a Barman server; GitHub #379 (Ian) repmgr: ensure "node rejoin" honours "--dry-run" option; GitHub #383 (Ian) + repmgr: fix --superuser handling when cloning a standby; GitHub #380 (Ian) repmgrd: improve detection of status change from primary to standby (Ian) repmgrd: improve log output in various situations (Ian) repmgrd: improve reconnection to the local node after a failover (Ian) diff --git a/dbutils.c b/dbutils.c index 68ce05c8..eb263b00 100644 --- a/dbutils.c +++ b/dbutils.c @@ -219,8 +219,7 @@ establish_db_connection_quiet(const char *conninfo) } -PGconn - * +PGconn * establish_primary_db_connection(PGconn *conn, const bool exit_on_error) { @@ -237,36 +236,6 @@ establish_primary_db_connection(PGconn *conn, } -PGconn * -establish_db_connection_as_user(const char *conninfo, - const char *user, - const bool exit_on_error) -{ - PGconn *conn = NULL; - t_conninfo_param_list conninfo_params = T_CONNINFO_PARAM_LIST_INITIALIZER; - bool parse_success = false; - char *errmsg = NULL; - - initialize_conninfo_params(&conninfo_params, false); - - parse_success = parse_conninfo_string(conninfo, &conninfo_params, errmsg, true); - - if (parse_success == false) - { - log_error(_("unable to pass provided conninfo string:\n %s"), errmsg); - return NULL; - } - - param_set(&conninfo_params, "user", user); - - conn = establish_db_connection_by_params(&conninfo_params, false); - - return conn; -} - - - - PGconn * establish_db_connection_by_params(t_conninfo_param_list *param_list, const bool exit_on_error) diff --git a/dbutils.h b/dbutils.h index cecf4f63..d9b6b53b 100644 --- a/dbutils.h +++ b/dbutils.h @@ -343,9 +343,6 @@ bool atobool(const char *value); PGconn *establish_db_connection(const char *conninfo, const bool exit_on_error); PGconn *establish_db_connection_quiet(const char *conninfo); -PGconn *establish_db_connection_as_user(const char *conninfo, - const char *user, - const bool exit_on_error); PGconn *establish_db_connection_by_params(t_conninfo_param_list *param_list, const bool exit_on_error); diff --git a/doc/repmgr-standby-clone.sgml b/doc/repmgr-standby-clone.sgml index 4d942a2d..74cae20c 100644 --- a/doc/repmgr-standby-clone.sgml +++ b/doc/repmgr-standby-clone.sgml @@ -185,6 +185,15 @@ + + + + + create recovery.conf file for a previously cloned instance + + + + @@ -194,6 +203,17 @@ + + + + + if the &repmgr; user is not a superuser, the name of a valid superuser must + be provided with this option + + + + + @@ -221,15 +241,6 @@ - - - - - create recovery.conf file for a previously cloned instance - - - - diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index fa421865..e9872511 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -4385,7 +4385,7 @@ check_upstream_config(PGconn *conn, int server_version_num, t_node_info *node_in param_set(&repl_conninfo, "replication", "1"); - if (*runtime_options.replication_user) + if (runtime_options.replication_user[0] != '\0') { param_set(&repl_conninfo, "user", runtime_options.replication_user); } @@ -4440,12 +4440,13 @@ check_upstream_config(PGconn *conn, int server_version_num, t_node_info *node_in * XXX at this point we could check * current_setting('max_wal_senders) - COUNT(*) FROM * pg_stat_replication; if >= min_replication_connections we could - * infer possible authentication error. + * infer possible authentication error / lack of permissions. * * Alternatively call PQconnectStart() and poll for * presence/absence of CONNECTION_AUTH_OK ? */ log_error(_("unable to establish necessary replication connections")); + log_hint(_("increase \"max_wal_senders\" by at least %i"), min_replication_connections - possible_replication_connections); diff --git a/repmgr-client.c b/repmgr-client.c index 85d34f4e..403af559 100644 --- a/repmgr-client.c +++ b/repmgr-client.c @@ -1485,11 +1485,12 @@ check_cli_parameters(const int action) { case PRIMARY_REGISTER: case STANDBY_REGISTER: - break; case STANDBY_CLONE: + break; + case STANDBY_FOLLOW: item_list_append_format(&cli_warnings, - _("--replication-user ignored when executing %s)"), + _("--replication-user ignored when executing %s"), action_name(action)); default: item_list_append_format(&cli_warnings, @@ -2148,10 +2149,19 @@ local_command(const char *command, PQExpBufferData *outputbuf) } +/* + * get_superuser_connection() + * + * Check if provided connection "conn" is a superuser connection, if not attempt to + * make a superuser connection "superuser_conn" with the provided --superuser parameter. + * + * "privileged_conn" is set to whichever connection is the superuser connection. + */ void get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privileged_conn) { t_connection_user userinfo = T_CONNECTION_USER_INITIALIZER; + t_conninfo_param_list conninfo_params = T_CONNINFO_PARAM_LIST_INITIALIZER; bool is_superuser = false; /* this should never happen */ @@ -2177,9 +2187,11 @@ get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privil exit(ERR_BAD_CONFIG); } - *superuser_conn = establish_db_connection_as_user(config_file_options.conninfo, - runtime_options.superuser, - false); + initialize_conninfo_params(&conninfo_params, false); + conn_to_param_list(*conn, &conninfo_params); + param_set(&conninfo_params, "user", runtime_options.superuser); + + *superuser_conn = establish_db_connection_by_params(&conninfo_params, false); if (PQstatus(*superuser_conn) != CONNECTION_OK) {