"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

@@ -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)

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)

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);

doc/repmgr-standby-clone.sgml

@@ -185,6 +185,15 @@
         </listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><option> --recovery-conf-only</option></term>
+        <listitem>
+          <para>
+            create <filename>recovery.conf</filename> file for a previously cloned instance
+          </para>
+        </listitem>
+      </varlistentry>
+
       <varlistentry>
         <term><option>--replication-user</option></term>
         <listitem>
@@ -194,6 +203,17 @@
         </listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><option>--superuser</option></term>
+        <listitem>
+          <para>
+            if the &repmgr; user is not a superuser, the name of a valid superuser must
+            be provided with this option
+          </para>
+        </listitem>
+      </varlistentry>
+
+
       <varlistentry>
         <term><option>--upstream-conninfo</option></term>
         <listitem>
@@ -221,15 +241,6 @@
         </listitem>
       </varlistentry>
 
-      <varlistentry>
-        <term><option> --recovery-conf-only</option></term>
-        <listitem>
-          <para>
-            create <filename>recovery.conf</filename> file for a previously cloned instance
-          </para>
-        </listitem>
-      </varlistentry>
-
     </variablelist>
   </refsect1>
 

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);
 

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,
+	initialize_conninfo_params(&conninfo_params, false);
-													  runtime_options.superuser,
+	conn_to_param_list(*conn, &conninfo_params);
-													  false);
+	param_set(&conninfo_params, "user", runtime_options.superuser);
+
+	*superuser_conn = establish_db_connection_by_params(&conninfo_params, false);
 
 	if (PQstatus(*superuser_conn) != CONNECTION_OK)
 	{