v1-0002-pg_ctl-Add-clear-environment-option.patch

text/plain

Filename: v1-0002-pg_ctl-Add-clear-environment-option.patch
Type: text/plain
Part: 1
Message: Re: libpq environment variables in the server

Patch

Format: format-patch
Series: patch v1-0002
Subject: pg_ctl: Add --clear-environment option
File+
doc/src/sgml/ref/pg_ctl-ref.sgml 12 0
src/bin/pg_ctl/pg_ctl.c 21 8
src/bin/pg_rewind/t/RewindTest.pm 2 2
src/test/ldap/t/001_auth.pl 3 0
src/test/perl/PostgresNode.pm 2 1
From 6e90d7fb54609cbe2806d3c10e0e36c49ab4f0c5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 18 Feb 2019 14:36:53 +0100
Subject: [PATCH v1 2/2] pg_ctl: Add --clear-environment option

This clears the environment for the newly started server process.
This should be the preferred practice.  Some init systems already do
this, but this option also gives that functionality when invoking
pg_ctl directly.

Also use this option in the TAP tests.  This revealed that a test in
the pg_rewrind suite relied on some environment variables being
inherited from the test environment.  Fix that case.
---
 doc/src/sgml/ref/pg_ctl-ref.sgml  | 12 ++++++++++++
 src/bin/pg_ctl/pg_ctl.c           | 29 +++++++++++++++++++++--------
 src/bin/pg_rewind/t/RewindTest.pm |  4 ++--
 src/test/ldap/t/001_auth.pl       |  3 +++
 src/test/perl/PostgresNode.pm     |  3 ++-
 5 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index e31275a04e..8f0f832fdb 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -280,6 +280,18 @@ <title>Options</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--clear-environment</option></term>
+      <listitem>
+       <para>
+        Clear all environment variables in the started server process.  This
+        is recommended for init scripts.  Note that this needs to be specified
+        for both the <literal>start</literal> and any
+        <literal>restart</literal> actions.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>-D <replaceable class="parameter">datadir</replaceable></option></term>
       <term><option>--pgdata=<replaceable class="parameter">datadir</replaceable></option></term>
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index c82a702ffa..2eaadba3b1 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -95,6 +95,7 @@ static char *register_password = NULL;
 static char *argv0 = NULL;
 static bool allow_core_files = false;
 static time_t start_time;
+static bool clear_environment = false;
 
 static char postopts_file[MAXPGPATH];
 static char version_file[MAXPGPATH];
@@ -103,6 +104,7 @@ static char backup_file[MAXPGPATH];
 static char promote_file[MAXPGPATH];
 static char logrotate_file[MAXPGPATH];
 
+static char pg_grandparent_pid_env_var[32];
 static volatile pgpid_t postmasterPID = -1;
 
 #ifdef WIN32
@@ -448,6 +450,8 @@ static pgpid_t
 start_postmaster(void)
 {
 	char		cmd[MAXPGPATH];
+	extern char **environ;
+	static char *clean_postmaster_env[] = {"LANG=C", NULL, NULL};
 
 #ifndef WIN32
 	pgpid_t		pm_pid;
@@ -486,6 +490,14 @@ start_postmaster(void)
 	}
 #endif
 
+	if (pg_grandparent_pid_env_var[0])
+	{
+		if (clear_environment)
+			clean_postmaster_env[1] = pg_grandparent_pid_env_var;
+		else
+			putenv(pg_grandparent_pid_env_var);
+	}
+
 	/*
 	 * Since there might be quotes to handle here, it is easier simply to pass
 	 * everything to a shell to process them.  Use exec so that the postmaster
@@ -499,7 +511,8 @@ start_postmaster(void)
 		snprintf(cmd, MAXPGPATH, "exec \"%s\" %s%s < \"%s\" 2>&1",
 				 exec_path, pgdata_opt, post_opts, DEVNULL);
 
-	(void) execl("/bin/sh", "/bin/sh", "-c", cmd, (char *) NULL);
+	(void) execle("/bin/sh", "/bin/sh", "-c", cmd, (char *) NULL,
+				  clear_environment ? clean_postmaster_env : environ);
 
 	/* exec failed */
 	write_stderr(_("%s: could not start server: %s\n"),
@@ -853,13 +866,8 @@ do_start(void)
 	 * getppid() unfortunately.
 	 */
 #ifndef WIN32
-	{
-		static char env_var[32];
-
-		snprintf(env_var, sizeof(env_var), "PG_GRANDPARENT_PID=%d",
-				 (int) getppid());
-		putenv(env_var);
-	}
+	snprintf(pg_grandparent_pid_env_var, sizeof(pg_grandparent_pid_env_var),
+			 "PG_GRANDPARENT_PID=%d", (int) getppid());
 #endif
 
 	pm_pid = start_postmaster();
@@ -2057,6 +2065,7 @@ do_help(void)
 #else
 	printf(_("  -c, --core-files       not applicable on this platform\n"));
 #endif
+	printf(_("      --clear-environment  clear environment variables in postgres process\n"));
 	printf(_("  -l, --log=FILENAME     write (or append) server log to FILENAME\n"));
 	printf(_("  -o, --options=OPTIONS  command line options to pass to postgres\n"
 			 "                         (PostgreSQL server executable) or initdb\n"));
@@ -2260,6 +2269,7 @@ main(int argc, char **argv)
 		{"core-files", no_argument, NULL, 'c'},
 		{"wait", no_argument, NULL, 'w'},
 		{"no-wait", no_argument, NULL, 'W'},
+		{"clear-environment", no_argument, NULL, 1},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -2415,6 +2425,9 @@ main(int argc, char **argv)
 				case 'c':
 					allow_core_files = true;
 					break;
+				case 1:
+					clear_environment = true;
+					break;
 				default:
 					/* getopt_long already issued a suitable error message */
 					do_advice();
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 85cae7e47b..ea0466cdce 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -268,10 +268,10 @@ sub run_pg_rewind
 		"unable to set permissions for $master_pgdata/postgresql.conf");
 
 	# Plug-in rewound node to the now-promoted standby node
-	my $port_standby = $node_standby->port;
+	my $connstr_standby = $node_standby->connstr();
 	$node_master->append_conf(
 		'postgresql.conf', qq(
-primary_conninfo='port=$port_standby'
+primary_conninfo='$connstr_standby'
 ));
 
 	$node_master->set_standby_mode();
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 431ad6442c..1fa8f7b554 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -269,6 +269,9 @@ sub test_access
 $node->append_conf('pg_hba.conf',
 	qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(uid=\$username)" ldaptls=1}
 );
+$node->append_conf('postgresql.conf',
+	qq{environment = 'LDAPCONF=$ldap_conf'}
+);
 $node->restart;
 
 $ENV{"PGPASSWORD"} = 'secret1';
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 8a2c6fc122..25aa8c38ea 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -701,7 +701,7 @@ sub start
 	BAIL_OUT("node \"$name\" is already running") if defined $self->{_pid};
 	print("### Starting node \"$name\"\n");
 	my $ret = TestLib::system_log('pg_ctl', '-D', $self->data_dir, '-l',
-		$self->logfile, 'start');
+		$self->logfile, '--clear-environment', 'start');
 
 	if ($ret != 0)
 	{
@@ -776,6 +776,7 @@ sub restart
 	my $name    = $self->name;
 	print "### Restarting node \"$name\"\n";
 	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
+		'--clear-environment',
 		'restart');
 	$self->_update_pid(1);
 	return;
-- 
2.20.1