Fix/skip hba propagation (#609)

* Add --skip-pg-hba to pg_autoctl do tmux session.

This is usable from the Makefile thanks to the new CLUSTER_OPTS variable.

* Add OPTION_POSTGRESQL_HBA_LEVEL to expected KeeperConfig INI settings.

In passing, also fix several oddities in the pghba file editing and review
the comman line option parsing logic for the --auth --skip-pg-hba and
--pg-hba-kan options.

Author: DimCitus

Makefile

@@ -35,6 +35,7 @@ NODES_ASYNC ?= 0				# count of replication-quorum false nodes
 NODES_PRIOS ?= 50				# either "50", or "50,50", or "50,50,0" etc
 NODES_SYNC_SB ?= -1
 FIRST_PGPORT ?= 5500
+CLUSTER_OPTS = ""			# could be "--skip-pg-hba"
 
 TMUX_EXTRA_COMMANDS ?= ""
 TMUX_LAYOUT ?= even-vertical	# could be "tiled"
@@ -150,7 +151,8 @@ $(TMUX_SCRIPT): bin
          --async-nodes $(NODES_ASYNC)     \
          --node-priorities $(NODES_PRIOS) \
          --sync-standbys $(NODES_SYNC_SB) \
-         --layout $(TMUX_LAYOUT) > $@
+         $(CLUSTER_OPTS)                  \
+		 --layout $(TMUX_LAYOUT) > $@
 
 tmux-script: $(TMUX_SCRIPT) ;
 
@@ -168,6 +170,7 @@ cluster: install tmux-clean
          --async-nodes $(NODES_ASYNC)     \
          --node-priorities $(NODES_PRIOS) \
          --sync-standbys $(NODES_SYNC_SB) \
+         $(CLUSTER_OPTS)                  \
          --layout $(TMUX_LAYOUT)
 
 

src/bin/pg_autoctl/cli_common.c

@@ -193,14 +193,23 @@ cli_common_keeper_getopts(int argc, char **argv,
 			case 'A':
 			{
 				/* { "auth", required_argument, NULL, 'A' }, */
-				if (!IS_EMPTY_STRING_BUFFER(LocalOptionConfig.pgSetup.authMethod))
+				if (LocalOptionConfig.pgSetup.hbaLevel == HBA_EDIT_SKIP)
 				{
 					errors++;
 					log_error("Please use either --auth or --skip-pg-hba");
 				}
 
 				strlcpy(LocalOptionConfig.pgSetup.authMethod, optarg, NAMEDATALEN);
 				log_trace("--auth %s", LocalOptionConfig.pgSetup.authMethod);
+
+				if (LocalOptionConfig.pgSetup.hbaLevel == HBA_EDIT_UNKNOWN)
+				{
+					strlcpy(LocalOptionConfig.pgSetup.hbaLevelStr,
+							pgsetup_hba_level_to_string(HBA_EDIT_MINIMAL),
+							sizeof(LocalOptionConfig.pgSetup.hbaLevelStr));
+
+					LocalOptionConfig.pgSetup.hbaLevel = HBA_EDIT_MINIMAL;
+				}
 				break;
 			}
 
@@ -231,8 +240,7 @@ cli_common_keeper_getopts(int argc, char **argv,
 			case 'L':
 			{
 				/* { "pg-hba-lan", required_argument, NULL, 'L' }, */
-				if (LocalOptionConfig.pgSetup.hbaLevel != HBA_EDIT_UNKNOWN &&
-					LocalOptionConfig.pgSetup.hbaLevel != HBA_EDIT_LAN)
+				if (LocalOptionConfig.pgSetup.hbaLevel == HBA_EDIT_SKIP)
 				{
 					errors++;
 					log_error("Please use either --skip-pg-hba or --pg-hba-lan");
@@ -511,6 +519,10 @@ cli_common_keeper_getopts(int argc, char **argv,
 	/* the default HBA editing level is MINIMAL, time to install it */
 	if (LocalOptionConfig.pgSetup.hbaLevel == HBA_EDIT_UNKNOWN)
 	{
+		strlcpy(LocalOptionConfig.pgSetup.hbaLevelStr,
+				pgsetup_hba_level_to_string(HBA_EDIT_MINIMAL),
+				sizeof(LocalOptionConfig.pgSetup.hbaLevelStr));
+
 		LocalOptionConfig.pgSetup.hbaLevel = HBA_EDIT_MINIMAL;
 	}
 

src/bin/pg_autoctl/cli_do_root.c

@@ -243,21 +243,29 @@ CommandLine do_tmux_script =
 	make_command("script",
 				 "Produce a tmux script for a demo or a test case (debug only)",
 				 "[option ...]",
-				 "  --root          path where to create a cluster\n"
-				 "  --first-pgport  first Postgres port to use (5500)\n"
-				 "  --nodes         number of Postgres nodes to create (2)\n"
-				 "  --layout        tmux layout to use (even-vertical)",
+				 "  --root            path where to create a cluster\n"
+				 "  --first-pgport    first Postgres port to use (5500)\n"
+				 "  --nodes           number of Postgres nodes to create (2)\n"
+				 "  --async-nodes     number of async nodes within nodes (0)\n"
+				 "  --node-priorities list of nodes priorities (50)\n"
+				 "  --sync-standbys   number-sync-standbys to set (0 or 1)\n"
+				 "  --skip-pg-hba     use --skip-pg-hba when creating nodes\n"
+				 "  --layout          tmux layout to use (even-vertical)",
 				 cli_do_tmux_script_getopts,
 				 cli_do_tmux_script);
 
 CommandLine do_tmux_session =
 	make_command("session",
 				 "Run a tmux session for a demo or a test case",
 				 "[option ...]",
-				 "  --root          path where to create a cluster\n"
-				 "  --first-pgport  first Postgres port to use (5500)\n"
-				 "  --nodes         number of Postgres nodes to create (2)\n"
-				 "  --layout        tmux layout to use (even-vertical)",
+				 "  --root            path where to create a cluster\n"
+				 "  --first-pgport    first Postgres port to use (5500)\n"
+				 "  --nodes           number of Postgres nodes to create (2)\n"
+				 "  --async-nodes     number of async nodes within nodes (0)\n"
+				 "  --node-priorities list of nodes priorities (50)\n"
+				 "  --sync-standbys   number-sync-standbys to set (0 or 1)\n"
+				 "  --skip-pg-hba     use --skip-pg-hba when creating nodes\n"
+				 "  --layout          tmux layout to use (even-vertical)",
 				 cli_do_tmux_script_getopts,
 				 cli_do_tmux_session);
 
@@ -286,9 +294,14 @@ CommandLine do_tmux_wait =
 	make_command("wait",
 				 "Wait until a given node has been registered on the monitor",
 				 "[option ...] nodename [ targetState ]",
-				 "  --root          path where to create a cluster\n"
-				 "  --first-pgport  first Postgres port to use (5500)\n"
-				 "  --nodes         number of Postgres nodes to create (2)",
+				 "  --root            path where to create a cluster\n"
+				 "  --first-pgport    first Postgres port to use (5500)\n"
+				 "  --nodes           number of Postgres nodes to create (2)\n"
+				 "  --async-nodes     number of async nodes within nodes (0)\n"
+				 "  --node-priorities list of nodes priorities (50)\n"
+				 "  --sync-standbys   number-sync-standbys to set (0 or 1)\n"
+				 "  --skip-pg-hba     use --skip-pg-hba when creating nodes\n"
+				 "  --layout          tmux layout to use (even-vertical)",
 				 cli_do_tmux_script_getopts,
 				 cli_do_tmux_wait);
 

src/bin/pg_autoctl/cli_do_tmux.c

@@ -210,6 +210,7 @@ cli_do_tmux_script_getopts(int argc, char **argv)
 		{ "async-nodes", required_argument, NULL, 'a' },
 		{ "node-priorities", required_argument, NULL, 'P' },
 		{ "sync-standbys", required_argument, NULL, 's' },
+		{ "skip-pg-hba", required_argument, NULL, 'S' },
 		{ "layout", required_argument, NULL, 'l' },
 		{ "version", no_argument, NULL, 'V' },
 		{ "verbose", no_argument, NULL, 'v' },
@@ -225,6 +226,7 @@ cli_do_tmux_script_getopts(int argc, char **argv)
 	options.nodes = 2;
 	options.asyncNodes = 0;
 	options.numSync = -1;       /* use pg_autoctl defaults */
+	options.skipHBA = false;
 	strlcpy(options.root, "/tmp/pgaf/tmux", sizeof(options.root));
 	strlcpy(options.layout, "even-vertical", sizeof(options.layout));
 
@@ -329,6 +331,13 @@ cli_do_tmux_script_getopts(int argc, char **argv)
 				break;
 			}
 
+			case 'S':
+			{
+				options.skipHBA = true;
+				log_trace("--skip-pg-hba");
+				break;
+			}
+
 			case 'l':
 			{
 				strlcpy(options.layout, optarg, MAXPGPATH);
@@ -613,9 +622,13 @@ tmux_add_new_session(PQExpBuffer script, const char *root, int pgport)
 void
 tmux_pg_autoctl_create_monitor(PQExpBuffer script,
 							   const char *root,
-							   int pgport)
+							   int pgport,
+							   bool skipHBA)
 {
-	char *pg_ctl_opts = "--hostname localhost --ssl-self-signed --auth trust";
+	char *pg_ctl_opts =
+		skipHBA
+		? "--hostname localhost --ssl-self-signed --skip-pg-hba"
+		: "--hostname localhost --ssl-self-signed --auth trust";
 
 	tmux_add_send_keys_command(script, "export PGPORT=%d", pgport);
 
@@ -639,10 +652,14 @@ tmux_pg_autoctl_create_postgres(PQExpBuffer script,
 								int pgport,
 								const char *name,
 								bool replicationQuorum,
-								int candidatePriority)
+								int candidatePriority,
+								bool skipHBA)
 {
 	char monitor[BUFSIZE] = { 0 };
-	char *pg_ctl_opts = "--hostname localhost --ssl-self-signed --auth trust";
+	char *pg_ctl_opts =
+		skipHBA
+		? "--hostname localhost --ssl-self-signed --skip-pg-hba"
+		: "--hostname localhost --ssl-self-signed --auth trust --pg-hba-lan";
 
 	tmux_add_send_keys_command(script, "export PGPORT=%d", pgport);
 
@@ -661,7 +678,6 @@ tmux_pg_autoctl_create_postgres(PQExpBuffer script,
 							   "--monitor %s "
 							   "--name %s "
 							   "--dbname demo "
-							   "--pg-hba-lan "
 							   "--replication-quorum %s "
 							   "--candidate-priority %d "
 							   "--run",
@@ -696,7 +712,7 @@ prepare_tmux_script(TmuxOptions *options, PQExpBuffer script)
 
 	/* start a monitor */
 	(void) tmux_add_xdg_environment(script);
-	tmux_pg_autoctl_create_monitor(script, root, pgport++);
+	tmux_pg_autoctl_create_monitor(script, root, pgport++, options->skipHBA);
 
 	/* start the Postgres nodes, using the monitor URI */
 	sformat(previousName, sizeof(previousName), "monitor");
@@ -726,7 +742,8 @@ prepare_tmux_script(TmuxOptions *options, PQExpBuffer script)
 										node->pgport,
 										node->name,
 										node->replicationQuorum,
-										node->candidatePriority);
+										node->candidatePriority,
+										options->skipHBA);
 
 		strlcpy(previousName, node->name, sizeof(previousName));
 	}

src/bin/pg_autoctl/cli_do_tmux.h

@@ -36,6 +36,7 @@ typedef struct TmuxOptions
 	int asyncNodes;             /* number of async nodes, within the total */
 	int priorities[MAX_NODES];  /* node priorities */
 	int numSync;                /* number-sync-standbys */
+	bool skipHBA;               /* do we want to use --skip-pg-hba? */
 	char layout[BUFSIZE];
 } TmuxOptions;
 
@@ -77,14 +78,16 @@ bool tmux_prepare_XDG_environment(const char *root,
 
 void tmux_pg_autoctl_create_monitor(PQExpBuffer script,
 									const char *root,
-									int pgport);
+									int pgport,
+									bool skipHBA);
 
 void tmux_pg_autoctl_create_postgres(PQExpBuffer script,
 									 const char *root,
 									 int pgport,
 									 const char *name,
 									 bool replicationQuorum,
-									 int candidatePriority);
+									 int candidatePriority,
+									 bool skipHBA);
 
 bool tmux_start_server(const char *scriptName);
 bool pg_autoctl_getpid(const char *pgdata, pid_t *pid);

src/bin/pg_autoctl/keeper.c

@@ -1571,7 +1571,7 @@ keeper_update_group_hba(Keeper *keeper, NodeAddressArray *diffNodesArray)
 									   postgresSetup->dbname,
 									   PG_AUTOCTL_REPLICA_USERNAME,
 									   authMethod,
-									   postgresSetup->hbaLevel))
+									   keeper->config.pgSetup.hbaLevel))
 	{
 		log_error("Failed to edit HBA file \"%s\" to update rules to current "
 				  "list of nodes registered on the monitor",
@@ -1584,7 +1584,7 @@ keeper_update_group_hba(Keeper *keeper, NodeAddressArray *diffNodesArray)
 	 * edited the HBA and it's going to take effect at next restart of
 	 * Postgres, so we're good here.
 	 */
-	if (postgresSetup->hbaLevel > HBA_EDIT_SKIP &&
+	if (keeper->config.pgSetup.hbaLevel >= HBA_EDIT_MINIMAL &&
 		pg_setup_is_running(postgresSetup))
 	{
 		if (!pgsql_reload_conf(pgsql))

src/bin/pg_autoctl/keeper_config.c

@@ -195,6 +195,7 @@
 		OPTION_POSTGRESQL_PROXY_PORT(config), \
 		OPTION_POSTGRESQL_LISTEN_ADDRESSES(config), \
 		OPTION_POSTGRESQL_AUTH_METHOD(config), \
+		OPTION_POSTGRESQL_HBA_LEVEL(config), \
 		OPTION_SSL_ACTIVE(config), \
 		OPTION_SSL_MODE(config), \
 		OPTION_SSL_CA_FILE(config), \
@@ -213,6 +214,7 @@
 	}
 
 static bool keeper_config_init_nodekind(KeeperConfig *config);
+static bool keeper_config_init_hbalevel(KeeperConfig *config);
 static bool keeper_config_set_backup_directory(KeeperConfig *config, int nodeId);
 
 
@@ -289,6 +291,13 @@ keeper_config_init(KeeperConfig *config,
 		exit(EXIT_CODE_BAD_CONFIG);
 	}
 
+	if (!keeper_config_init_hbalevel(config))
+	{
+		log_error("Failed to initialize postgresql.hba_level");
+		log_error("Please review your setup options per above messages");
+		exit(EXIT_CODE_BAD_CONFIG);
+	}
+
 	if (!pg_setup_init(&pgSetup,
 					   &(config->pgSetup),
 					   missingPgdataIsOk,
@@ -405,9 +414,10 @@ keeper_config_read_file_skip_pgsetup(KeeperConfig *config,
 	/*
 	 * Turn the configuration string for hbaLevel into our enum value.
 	 */
-	if (IS_EMPTY_STRING_BUFFER(config->pgSetup.hbaLevelStr))
+	if (!keeper_config_init_hbalevel(config))
 	{
-		strlcpy(config->pgSetup.hbaLevelStr, "minimal", NAMEDATALEN);
+		log_error("Failed to initialize postgresql.hba_level");
+		return false;
 	}
 
 	/* set the ENUM value for hbaLevel */
@@ -724,6 +734,30 @@ keeper_config_init_nodekind(KeeperConfig *config)
 }
 
 
+/*
+ * keeper_config_init_hbalevel initializes the config->pgSetup.hbaLevel and
+ * hbaLevelStr when no command line option switch has been used that places a
+ * value (see --auth, --skip-pg-hba, and --pg-hba-lan).
+ */
+static bool
+keeper_config_init_hbalevel(KeeperConfig *config)
+{
+	/*
+	 * Turn the configuration string for hbaLevel into our enum value.
+	 */
+	if (IS_EMPTY_STRING_BUFFER(config->pgSetup.hbaLevelStr))
+	{
+		strlcpy(config->pgSetup.hbaLevelStr, "minimal", NAMEDATALEN);
+	}
+
+	/* set the ENUM value for hbaLevel */
+	config->pgSetup.hbaLevel =
+		pgsetup_parse_hba_level(config->pgSetup.hbaLevelStr);
+
+	return true;
+}
+
+
 /*
  * keeper_config_set_backup_directory sets the pg_basebackup target directory
  * to ${PGDATA}/../backup/${hostname} by default. Adding the local hostname