Standardize "primary_conninfo" generation

Previously repmgr would write all the default libpq parameters
into "primary_conninfo" on "standby clone", but not for
"standby follow", which is inconsistent.

For repmgr4 we'll determine that the upstream node's conninfo
must be canonical and contain all required connection parameters,
even if these are available as defaults or environment variables
in the local environment, as those are transient and may not
be available in all environments/situations.

recovery.conf's "primary_conninfo" will be generated using the
upstream's conninfo parameters, except for those specific
to the downstream node. These are:

  - "application_name": this will always be set to the
      "node_name"  of the downstream node
  - "passfile" and "servicefile": these, must of course
    reference files on the downstream node so will be extracted
    from the downstream node's conninfo, if set

dbutils.c

@@ -527,9 +527,14 @@ param_get(t_conninfo_param_list *param_list, const char *param)
  * Parse a conninfo string into a t_conninfo_param_list
  *
  * See conn_to_param_list() to do the same for a PQconn
+ *
+ * "ignore_local_params": ignores those parameters specific
+ * to a local installation, i.e. when parsing an upstream
+ * node's conninfo string for inclusion into "primary_conninfo",
+ * don't copy that node's values
  */
 bool
-parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg, bool ignore_application_name)
+parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg, bool ignore_local_params)
 {
 	PQconninfoOption *connOptions;
 	PQconninfoOption *option;
@@ -541,14 +546,21 @@ parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_lis
 
 	for (option = connOptions; option && option->keyword; option++)
 	{
-		/* Ignore non-set or blank parameter values*/
+		/* Ignore non-set or blank parameter values */
 		if ((option->val == NULL) ||
 		   (option->val != NULL && option->val[0] == '\0'))
 			continue;
 
 		/* Ignore application_name */
-		if (ignore_application_name == true && strcmp(option->keyword, "application_name") == 0)
-			continue;
+		if (ignore_local_params == true)
+		{
+			if (strcmp(option->keyword, "application_name") == 0)
+				continue;
+			if (strcmp(option->keyword, "passfile") == 0)
+				continue;
+			if (strcmp(option->keyword, "servicefile") == 0)
+				continue;
+		}
 
 		param_set(param_list, option->keyword, option->val);
 	}

dbutils.h

@@ -306,7 +306,7 @@ void		conn_to_param_list(PGconn *conn, t_conninfo_param_list *param_list);
 void		param_set(t_conninfo_param_list *param_list, const char *param, const char *value);
 void		param_set_ine(t_conninfo_param_list *param_list, const char *param, const char *value);
 char	   *param_get(t_conninfo_param_list *param_list, const char *param);
-bool		parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg, bool ignore_application_name);
+bool		parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg, bool ignore_local_params);
 char	   *param_list_to_string(t_conninfo_param_list *param_list);
 
 /* transaction functions */

repmgr-action-standby.c

@@ -155,39 +155,42 @@ do_standby_clone(void)
 	 * Initialise list of conninfo parameters which will later be used
 	 * to create the `primary_conninfo` string in recovery.conf .
 	 *
-	 * We'll initialise it with the default values as seen by libpq,
-	 * and overwrite them with the host settings specified on the command
+	 * We'll initialise it with the host settings specified on the command
 	 * line. As it's possible the standby will be cloned from a node different
 	 * to its intended upstream, we'll later attempt to fetch the
 	 * upstream node record and overwrite the values set here with
 	 * those from the upstream node record (excluding that record's
 	 * application_name)
 	 */
-	initialize_conninfo_params(&recovery_conninfo, true);
+	initialize_conninfo_params(&recovery_conninfo, false);
 
 	copy_conninfo_params(&recovery_conninfo, &source_conninfo);
 
-	/* Set the default application name to this node's name */
-	param_set(&recovery_conninfo, "application_name", config_file_options.node_name);
 
-	/*
-	 * If application_name is set in repmgr.conf's conninfo parameter, use
-	 * this value (if the source host was provided as a conninfo string, any
-	 * application_name values set there will be overridden; we assume the only
-	 * reason to pass an application_name via the command line is in the
-	 * rare corner case where a user wishes to clone a server without
-	 * providing repmgr.conf)
-	 */
-	if (strlen(config_file_options.conninfo))
+	/* Set the default application name to this node's name */
+	if (config_file_options.node_id != UNKNOWN_NODE_ID)
 	{
 		char application_name[MAXLEN] = "";
 
+		param_set(&recovery_conninfo, "application_name", config_file_options.node_name);
+
 		get_conninfo_value(config_file_options.conninfo, "application_name", application_name);
-		if (strlen(application_name))
+		if (strlen(application_name) && strncmp(application_name, config_file_options.node_name, MAXLEN) != 0)
 		{
-			param_set(&recovery_conninfo, "application_name", application_name);
+			log_notice(_("\"application_name\" is set in repmgr.conf but will be replaced by the node name"));
 		}
 	}
+	else
+	{
+		/* this will only happen in corner cases where the node is being
+		 * cloned without a configuration file; fall back to "repmgr" if no application_name
+		 * provided
+		 */
+		char *application_name = param_get(&source_conninfo, "application_name");
+
+		if (application_name == NULL)
+			param_set(&recovery_conninfo, "application_name", "repmgr");
+	}
 
 
 	/*
@@ -205,6 +208,8 @@ do_standby_clone(void)
 	if (*runtime_options.upstream_conninfo)
 		runtime_options.no_upstream_connection = false;
 
+
+
 	/* By default attempt to connect to the source server */
 	if (runtime_options.no_upstream_connection == false)
 	{
@@ -225,6 +230,7 @@ do_standby_clone(void)
 		check_source_server_via_barman();
 	}
 
+
 	/*
 	 * by this point we should know the target data directory - check
 	 * there's no running Pg instance
@@ -244,7 +250,7 @@ do_standby_clone(void)
 
 	if (upstream_record_found == true)
 	{
-		/*  parse returned upstream conninfo string to recovery primary_conninfo params*/
+		/* parse returned upstream conninfo string to recovery primary_conninfo params*/
 		char	   *errmsg = NULL;
 		bool	    parse_success;
 
@@ -253,6 +259,7 @@ do_standby_clone(void)
 		/* parse_conninfo_string() here will remove the upstream's `application_name`, if set */
 
 		parse_success = parse_conninfo_string(recovery_conninfo_str, &recovery_conninfo, errmsg, true);
+
 		if (parse_success == false)
 		{
 			log_error(_("unable to parse conninfo string \"%s\" for upstream node:\n  %s"),
@@ -2405,13 +2412,6 @@ check_source_server()
 		}
 	}
 
-	/*
-	 * Copy the source connection so that we have some default values,
-	 * particularly stuff like passwords extracted from PGPASSFILE;
-	 * these will be overridden from the upstream conninfo, if provided.
-	 */
-	conn_to_param_list(source_conn, &recovery_conninfo);
-
 	/*
 	 * Attempt to find the upstream node record
 	 */

repmgr-client.c

@@ -111,7 +111,7 @@ main(int argc, char **argv)
 	 * when the connection is made.
 	 */
 
-	initialize_conninfo_params(&source_conninfo, true);
+	initialize_conninfo_params(&source_conninfo, false);
 
 	for (c = 0; c < source_conninfo.size && source_conninfo.keywords[c]; c++)
 	{
@@ -2433,7 +2433,8 @@ copy_remote_files(char *host, char *remote_user, char *remote_path,
  * Creates a recovery.conf file for a standby
  *
  * A database connection pointer is required for escaping primary_conninfo
- * parameters. When cloning from Barman and --no-upstream-connection ) this might not be
+ * parameters. When cloning from Barman and --no-upstream-connection ) this
+ * might not be available.
  */
 bool
 create_recovery_file(t_node_info *node_record, t_conninfo_param_list *recovery_conninfo, const char *data_dir)
@@ -2572,12 +2573,15 @@ write_primary_conninfo(char *line, t_conninfo_param_list *param_list)
 		 */
 		if (strcmp(param_list->keywords[c], "dbname") == 0 ||
 		    strcmp(param_list->keywords[c], "replication") == 0 ||
-			(runtime_options.use_recovery_conninfo_password == false &&
-			 strcmp(param_list->keywords[c], "password") == 0) ||
 		    (param_list->values[c] == NULL) ||
 		    (param_list->values[c] != NULL && param_list->values[c][0] == '\0'))
 			continue;
 
+		/* only include "password" if explicitly requested */
+		if (runtime_options.use_recovery_conninfo_password == false &&
+			strcmp(param_list->keywords[c], "password") == 0)
+			continue;
+
 		if (conninfo_buf.len != 0)
 			appendPQExpBufferChar(&conninfo_buf, ' ');