Simplify and clean up the $NEON_AUTH_TOKEN stuff in compute

- Remove the neon.safekeeper_token_env GUC. It was used to set the
  name of an environment variable, which was then used in pageserver
  and safekeeper connection strings to in place of the
  password. Instead, always look up the environment variable called
  NEON_AUTH_TOKEN. That's what neon.safekeeper_token_env was always
  set to in practice, and I don't see the need for the extra level of
  indirection or configurability.

- Instead of substituting $NEON_AUTH_TOKEN in the connection strings,
  pass $NEON_AUTH_TOKEN "out-of-band" as the password, when we connect
  to the pageserver or safekeepers. That's simpler.

- Also use the password from $NEON_AUTH_TOKEN in compute_ctl, when it
  connects to the pageserver to get the "base backup".

pgxn/neon/libpagestore.c

@@ -46,8 +46,12 @@ PGconn	   *pageserver_conn = NULL;
  */
 WaitEventSet *pageserver_conn_wes = NULL;
 
-char	   *page_server_connstring_raw;
-char	   *safekeeper_token_env;
+/* GUCs */
+char	   *neon_timeline;
+char	   *neon_tenant;
+int32		max_cluster_size;
+char	   *page_server_connstring;
+char	   *neon_auth_token;
 
 int			n_unflushed_requests = 0;
 int			flush_every_n_requests = 8;
@@ -60,10 +64,37 @@ pageserver_connect(int elevel)
 {
 	char	   *query;
 	int			ret;
+	const char *keywords[3];
+	const char *values[3];
+	int			n;
 
 	Assert(!connected);
 
-	pageserver_conn = PQconnectdb(page_server_connstring);
+	/*
+	 * Connect using the connection string we got from the
+	 * neon.pageserver_connstring GUC. If the NEON_AUTH_TOKEN environment
+	 * variable was set, use that as the password.
+	 *
+	 * The connection options are parsed in the order they're given, so
+	 * when we set the password before the connection string, the
+	 * connection string can override the password from the env variable.
+	 * Seems useful, although we don't currently use that capability
+	 * anywhere.
+	 */
+	n = 0;
+	if (neon_auth_token)
+	{
+		keywords[n] = "password";
+		values[n] = neon_auth_token;
+		n++;
+	}
+	keywords[n] = "dbname";
+	values[n] = page_server_connstring;
+	n++;
+	keywords[n] = NULL;
+	values[n] = NULL;
+	n++;
+	pageserver_conn = PQconnectdbParams(keywords, values, 1);
 
 	if (PQstatus(pageserver_conn) == CONNECTION_BAD)
 	{
@@ -125,7 +156,7 @@ pageserver_connect(int elevel)
 		}
 	}
 
-	neon_log(LOG, "libpagestore: connected to '%s'", page_server_connstring_raw);
+	neon_log(LOG, "libpagestore: connected to '%s'", page_server_connstring);
 
 	connected = true;
 	return true;
@@ -354,105 +385,6 @@ check_neon_id(char **newval, void **extra, GucSource source)
 	return **newval == '\0' || HexDecodeString(id, *newval, 16);
 }
 
-static char *
-substitute_pageserver_password(const char *page_server_connstring_raw)
-{
-	char	   *host = NULL;
-	char	   *port = NULL;
-	char	   *user = NULL;
-	char	   *auth_token = NULL;
-	char	   *err = NULL;
-	char	   *page_server_connstring = NULL;
-	PQconninfoOption *conn_options;
-	PQconninfoOption *conn_option;
-	MemoryContext oldcontext;
-
-	/*
-	 * Here we substitute password in connection string with an environment
-	 * variable. To simplify things we construct a connection string back with
-	 * only known options. In particular: host port user and password. We do
-	 * not currently use other options and constructing full connstring in an
-	 * URI shape is quite messy.
-	 */
-
-	if (page_server_connstring_raw == NULL || page_server_connstring_raw[0] == '\0')
-		return NULL;
-
-	/* extract the auth token from the connection string */
-	conn_options = PQconninfoParse(page_server_connstring_raw, &err);
-	if (conn_options == NULL)
-	{
-		/* The error string is malloc'd, so we must free it explicitly */
-		char	   *errcopy = err ? pstrdup(err) : "out of memory";
-
-		PQfreemem(err);
-		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("invalid connection string syntax: %s", errcopy)));
-	}
-
-	/*
-	 * Trying to populate pageserver connection string with auth token from
-	 * environment. We are looking for password in with placeholder value like
-	 * $ENV_VAR_NAME, so if password field is present and starts with $ we try
-	 * to fetch environment variable value and fail loudly if it is not set.
-	 */
-	for (conn_option = conn_options; conn_option->keyword != NULL; conn_option++)
-	{
-		if (strcmp(conn_option->keyword, "host") == 0)
-		{
-			if (conn_option->val != NULL && conn_option->val[0] != '\0')
-				host = conn_option->val;
-		}
-		else if (strcmp(conn_option->keyword, "port") == 0)
-		{
-			if (conn_option->val != NULL && conn_option->val[0] != '\0')
-				port = conn_option->val;
-		}
-		else if (strcmp(conn_option->keyword, "user") == 0)
-		{
-			if (conn_option->val != NULL && conn_option->val[0] != '\0')
-				user = conn_option->val;
-		}
-		else if (strcmp(conn_option->keyword, "password") == 0)
-		{
-			if (conn_option->val != NULL && conn_option->val[0] != '\0')
-			{
-				/* ensure that this is a template */
-				if (strncmp(conn_option->val, "$", 1) != 0)
-					ereport(ERROR,
-							(errcode(ERRCODE_CONNECTION_EXCEPTION),
-							 errmsg("expected placeholder value in pageserver password starting from $ but found: %s", &conn_option->val[1])));
-
-				neon_log(LOG, "found auth token placeholder in pageserver conn string '%s'", &conn_option->val[1]);
-				auth_token = getenv(&conn_option->val[1]);
-				if (!auth_token)
-				{
-					ereport(ERROR,
-							(errcode(ERRCODE_CONNECTION_EXCEPTION),
-							 errmsg("cannot get auth token, environment variable %s is not set", &conn_option->val[1])));
-				}
-				else
-				{
-					neon_log(LOG, "using auth token from environment passed via env");
-				}
-			}
-		}
-	}
-
-	/*
-	 * allocate connection string in TopMemoryContext to make sure it is not
-	 * freed
-	 */
-	oldcontext = CurrentMemoryContext;
-	MemoryContextSwitchTo(TopMemoryContext);
-	page_server_connstring = psprintf("postgresql://%s:%s@%s:%s", user, auth_token ? auth_token : "", host, port);
-	MemoryContextSwitchTo(oldcontext);
-
-	PQconninfoFree(conn_options);
-	return page_server_connstring;
-}
-
 /*
  * Module initialization function
  */
@@ -462,21 +394,12 @@ pg_init_libpagestore(void)
 	DefineCustomStringVariable("neon.pageserver_connstring",
 							   "connection string to the page server",
 							   NULL,
-							   &page_server_connstring_raw,
+							   &page_server_connstring,
 							   "",
 							   PGC_POSTMASTER,
 							   0,	/* no flags required */
 							   NULL, NULL, NULL);
 
-    DefineCustomStringVariable("neon.safekeeper_token_env",
-                               "the environment variable containing JWT token for authentication with Safekeepers, the convention is to either unset or set to $NEON_AUTH_TOKEN",
-                               NULL,
-                               &safekeeper_token_env,
-                               NULL,
-                               PGC_POSTMASTER,
-                               0,	/* no flags required */
-                               NULL, NULL, NULL);
-
 	DefineCustomStringVariable("neon.timeline_id",
 							   "Neon timeline_id the server is running on",
 							   NULL,
@@ -533,26 +456,10 @@ pg_init_libpagestore(void)
 	neon_log(PageStoreTrace, "libpagestore already loaded");
 	page_server = &api;
 
-	/* substitute password in pageserver_connstring */
-	page_server_connstring = substitute_pageserver_password(page_server_connstring_raw);
-
-	/* retrieve the token for Safekeeper, if present */
-	if (safekeeper_token_env != NULL) {
-		if (safekeeper_token_env[0] != '$') {
-			ereport(ERROR,
-					(errcode(ERRCODE_CONNECTION_EXCEPTION),
-							errmsg("expected safekeeper auth token environment variable's name starting with $ but found: %s",
-								   safekeeper_token_env)));
-		}
-		neon_safekeeper_token = getenv(&safekeeper_token_env[1]);
-		if (!neon_safekeeper_token) {
-			ereport(ERROR,
-					(errcode(ERRCODE_CONNECTION_EXCEPTION),
-							errmsg("cannot get safekeeper auth token, environment variable %s is not set",
-								   &safekeeper_token_env[1])));
-		}
-		neon_log(LOG, "using safekeeper auth token from environment variable");
-	}
+	/* Retrieve the auth token to use when connecting to pageserver and safekeepers */
+	neon_auth_token = getenv("NEON_AUTH_TOKEN");
+	if (neon_auth_token)
+		neon_log(LOG, "using storage auth token from NEON_AUTH_TOKEN environment variable");
 
 	if (page_server_connstring && page_server_connstring[0])
 	{

pgxn/neon/libpqwalproposer.c

@@ -51,12 +51,39 @@ walprop_status(WalProposerConn *conn)
 }
 
 WalProposerConn *
-walprop_connect_start(char *conninfo)
+walprop_connect_start(char *conninfo, char *password)
 {
 	WalProposerConn *conn;
 	PGconn	   *pg_conn;
+	const char *keywords[3];
+	const char *values[3];
+	int			n;
 
-	pg_conn = PQconnectStart(conninfo);
+	/*
+	 * Connect using the given connection string. If the
+	 * NEON_AUTH_TOKEN environment variable was set, use that as
+	 * the password.
+	 *
+	 * The connection options are parsed in the order they're given, so
+	 * when we set the password before the connection string, the
+	 * connection string can override the password from the env variable.
+	 * Seems useful, although we don't currently use that capability
+	 * anywhere.
+	 */
+	n = 0;
+	if (password)
+	{
+		keywords[n] = "password";
+		values[n] = neon_auth_token;
+		n++;
+	}
+	keywords[n] = "dbname";
+	values[n] = conninfo;
+	n++;
+	keywords[n] = NULL;
+	values[n] = NULL;
+	n++;
+	pg_conn = PQconnectStartParams(keywords, values, 1);
 
 	/*
 	 * Allocation of a PQconn can fail, and will return NULL. We want to fully

pgxn/neon/neon.h

@@ -13,7 +13,7 @@
 #define NEON_H
 
 /* GUCs */
-extern char *neon_safekeeper_token;
+extern char *neon_auth_token;
 extern char *neon_timeline;
 extern char *neon_tenant;
 

pgxn/neon/pagestore_smgr.c

@@ -92,14 +92,6 @@ const int	SmgrTrace = DEBUG5;
 
 page_server_api *page_server;
 
-/* GUCs */
-char	   *page_server_connstring;
-
-/*with substituted password*/
-char	   *neon_timeline;
-char	   *neon_tenant;
-int32		max_cluster_size;
-
 /* unlogged relation build states */
 typedef enum
 {

pgxn/neon/walproposer.c

@@ -510,17 +510,9 @@ WalProposerInit(XLogRecPtr flushRecPtr, uint64 systemId)
 			Safekeeper *sk = &safekeeper[n_safekeepers];
 			int written = 0;
 
-			if (neon_safekeeper_token != NULL) {
-				written = snprintf((char *) &sk->conninfo, MAXCONNINFO,
-								   "host=%s port=%s password=%s dbname=replication options='-c timeline_id=%s tenant_id=%s'",
-								   sk->host, sk->port, neon_safekeeper_token, neon_timeline,
-								   neon_tenant);
-			} else {
-				written = snprintf((char *) &sk->conninfo, MAXCONNINFO,
-								   "host=%s port=%s dbname=replication options='-c timeline_id=%s tenant_id=%s'",
-								   sk->host, sk->port, neon_timeline, neon_tenant);
-			}
-
+			written = snprintf((char *) &sk->conninfo, MAXCONNINFO,
+							   "host=%s port=%s dbname=replication options='-c timeline_id=%s tenant_id=%s'",
+							   sk->host, sk->port, neon_timeline, neon_tenant);
 			if (written > MAXCONNINFO || written < 0)
 				elog(FATAL, "could not create connection string for safekeeper %s:%s", sk->host, sk->port);
 		}
@@ -696,7 +688,7 @@ ResetConnection(Safekeeper *sk)
 	/*
 	 * Try to establish new connection
 	 */
-	sk->conn = walprop_connect_start((char *) &sk->conninfo);
+	sk->conn = walprop_connect_start((char *) &sk->conninfo, neon_auth_token);
 
 	/*
 	 * "If the result is null, then libpq has been unable to allocate a new

pgxn/neon/walproposer.h

@@ -454,7 +454,7 @@ extern char *walprop_error_message(WalProposerConn *conn);
 extern WalProposerConnStatusType walprop_status(WalProposerConn *conn);
 
 /* Re-exported PQconnectStart */
-extern WalProposerConn * walprop_connect_start(char *conninfo);
+extern WalProposerConn * walprop_connect_start(char *conninfo, char *password);
 
 /* Re-exported PQconectPoll */
 extern WalProposerConnectPollStatusType walprop_connect_poll(WalProposerConn *conn);