diff --git a/dbutils.c b/dbutils.c index 6f3b5010..93b7449f 100644 --- a/dbutils.c +++ b/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); } diff --git a/dbutils.h b/dbutils.h index 875d9577..0da3bea6 100644 --- a/dbutils.h +++ b/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 */ diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index c41e904e..926eb4f7 100644 --- a/repmgr-action-standby.c +++ b/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 */ diff --git a/repmgr-client.c b/repmgr-client.c index affdb9dd..e3315c55 100644 --- a/repmgr-client.c +++ b/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, ' ');