Not use hardcoded LOCAL_HOST_NAME but citus.local_hostname to distinguish loopback connections (#7436)

Fixes a bug that breaks queries from non-maindbs when
citus.local_hostname is set to a value different than "localhost".

This is a very old bug doesn't cause a problem as long as Citus catalog
is available to FindWorkerNode(). And the catalog is always available
unless we're in non-main database, which might be the case on main but
not on older releases, hence not adding a `DESCRIPTION`. For this
reason, I don't see a reason to backport this.

Maybe we should totally refrain using LOCAL_HOST_NAME in all code-paths,
but not doing that in this PR as the other paths don't seem to be
breaking something that is user-facing.

```c
char *
GetAuthinfo(char *hostname, int32 port, char *user)
{
	char *authinfo = NULL;
	bool isLoopback = (strncmp(LOCAL_HOST_NAME, hostname, MAX_NODE_LENGTH) == 0 &&
					   PostPortNumber == port);

	if (IsTransactionState())
	{
		int64 nodeId = WILDCARD_NODE_ID;

		/* -1 is a special value for loopback connections (task tracker) */
		if (isLoopback)
		{
			nodeId = LOCALHOST_NODE_ID;
		}
		else
		{
			WorkerNode *worker = FindWorkerNode(hostname, port);
			if (worker != NULL)
			{
				nodeId = worker->nodeId;
			}
		}

		authinfo = GetAuthinfoViaCatalog(user, nodeId);
	}

	return (authinfo != NULL) ? authinfo : "";
}
```
pull/7449/head
Onur Tirtir 2024-01-24 15:58:55 +03:00 committed by GitHub
parent 8b48d6ab02
commit 1d096df7f4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 28 additions and 9 deletions

View File

@ -521,9 +521,23 @@ char *
GetAuthinfo(char *hostname, int32 port, char *user)
{
char *authinfo = NULL;
bool isLoopback = (strncmp(LOCAL_HOST_NAME, hostname, MAX_NODE_LENGTH) == 0 &&
bool isLoopback = (strncmp(LocalHostName, hostname, MAX_NODE_LENGTH) == 0 &&
PostPortNumber == port);
/*
* Citus will not be loaded when we run a global DDL command from a
* Citus non-main database.
*/
if (!CitusHasBeenLoaded())
{
/*
* We don't expect non-main databases to connect to a node other than
* the local one.
*/
Assert(isLoopback);
return "";
}
if (IsTransactionState())
{
int64 nodeId = WILDCARD_NODE_ID;

View File

@ -5723,14 +5723,6 @@ GetPoolinfoViaCatalog(int32 nodeId)
char *
GetAuthinfoViaCatalog(const char *roleName, int64 nodeId)
{
/*
* Citus will not be loaded when we run a global DDL command from a
* Citus non-main database.
*/
if (!CitusHasBeenLoaded())
{
return "";
}
char *authinfo = "";
Datum nodeIdDatumArray[2] = {
Int32GetDatum(nodeId),

View File

@ -71,9 +71,15 @@ SELECT citus_internal.execute_command_on_remote_nodes_as_user($$SELECT 'dangerou
ERROR: operation is not allowed
HINT: Run the command with a superuser.
\c other_db1
SET citus.local_hostname TO '127.0.0.1';
SET ROLE nonsuperuser;
-- Make sure that we don't try to access pg_dist_node.
-- Otherwise, we would get the following error:
-- ERROR: cache lookup failed for pg_dist_node, called too early?
CREATE USER other_db_user9;
RESET ROLE;
RESET citus.local_hostname;
RESET ROLE;
\c regression
SELECT usename FROM pg_user WHERE usename LIKE 'other\_db\_user%' ORDER BY 1;
usename

View File

@ -51,9 +51,16 @@ SET ROLE nonsuperuser;
SELECT citus_internal.execute_command_on_remote_nodes_as_user($$SELECT 'dangerous query'$$, 'postgres');
\c other_db1
SET citus.local_hostname TO '127.0.0.1';
SET ROLE nonsuperuser;
-- Make sure that we don't try to access pg_dist_node.
-- Otherwise, we would get the following error:
-- ERROR: cache lookup failed for pg_dist_node, called too early?
CREATE USER other_db_user9;
RESET ROLE;
RESET citus.local_hostname;
RESET ROLE;
\c regression
SELECT usename FROM pg_user WHERE usename LIKE 'other\_db\_user%' ORDER BY 1;