pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: libpq environment variables in the server
Date: Mon, 18 Feb 2019 14:58:09 +0100
Message-ID: <744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com> (raw)
In-Reply-To: <33383613-690e-6f1b-d5ba-4957ff40f6ce@2ndquadrant.com>
References: <33383613-690e-6f1b-d5ba-4957ff40f6ce@2ndquadrant.com>

On 2019-01-21 11:42, Peter Eisentraut wrote:
> Maybe pg_ctl should have some functionality to clear the environment,
> and maybe there could be a facility in postgresql.conf to set
> environment variables.

After pondering this, this seemed to be the best solution.

Many init systems already clear the environment by default, so that the
started services don't have random environment settings inherited from
the user.  However, you can't easily do this when using pg_ctl directly,
so having an option to clear the environment in pg_ctl seems generally
useful.  Then we can use it in the test suites and thus avoid
environment variables leaking in in unintended ways.

That revealed that we do need the second mechanism to set environment
variables after everything has been cleared.  One example in the test
suite is LDAPCONF.  But there are other cases that could be useful, such
as any environment settings that would support archive_command or
restore_command.  I think this would also be a nice feature in general,
so that you can keep all the configuration together in the configuration
files and don't have to rely on external mechanisms to inject these
environment variables.

Attached are two patches that implement these features.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From dd76033c87ddfe60c50e6bb397c123f489cadc1c 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 1/2] Add GUC parameter "environment"

This allows setting environment variables inside the server processes,
for use by third-party libraries, like LDAPCONF.  Previously, this had
to be done via external mechanisms such as init scripts.
---
 doc/src/sgml/config.sgml                      | 21 ++++++++++
 src/backend/utils/misc/guc.c                  | 40 +++++++++++++++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/utils/guc.h                       |  2 +
 4 files changed, 64 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8bd57f376b..acbcf5559b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8304,6 +8304,27 @@ <title>Other Defaults</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-environment" xreflabel="environment">
+      <term><varname>environment</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>environment</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        A comma-separated list of environment variable assignments to apply
+        inside the <productname>PostgreSQL</productname> server process.  This
+        can be used to supply configuration information or debugging settings
+        to third-party libraries or plugins.  Example:
+<programlisting>environment = 'LDAPCONF=/some/file,RSYNC_RSH=ssh'</programlisting>
+        This parameter can only be set in the
+        <filename>postgresql.conf</filename> file or on the server command
+        line.  Note that changing this parameter at run time will not unset
+        previously set environment variables.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-gin-fuzzy-search-limit" xreflabel="gin_fuzzy_search_limit">
       <term><varname>gin_fuzzy_search_limit</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 156d147c85..10c9528729 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -212,6 +212,7 @@ static bool check_recovery_target_lsn(char **newval, void **extra, GucSource sou
 static void assign_recovery_target_lsn(const char *newval, void *extra);
 static bool check_primary_slot_name(char **newval, void **extra, GucSource source);
 static bool check_default_with_oids(bool *newval, void **extra, GucSource source);
+static void assign_environment_settings(const char *newval, void *extra);
 
 /* Private functions in guc-file.l that need to be called from guc.c */
 static ConfigVariable *ProcessConfigFileInternal(GucContext context,
@@ -529,6 +530,8 @@ int			tcp_keepalives_idle;
 int			tcp_keepalives_interval;
 int			tcp_keepalives_count;
 
+char	   *environment_settings;
+
 /*
  * SSL renegotiation was been removed in PostgreSQL 9.5, but we tolerate it
  * being set to zero (meaning never renegotiate) for backward compatibility.
@@ -3516,6 +3519,17 @@ static struct config_string ConfigureNamesString[] =
 		check_client_encoding, assign_client_encoding, NULL
 	},
 
+	{
+		{"environment", PGC_SIGHUP, CLIENT_CONN_OTHER,
+			gettext_noop("Environment variables to be set."),
+			NULL,
+			GUC_LIST_INPUT | GUC_LIST_QUOTE
+		},
+		&environment_settings,
+		"",
+		NULL, assign_environment_settings, NULL
+	},
+
 	{
 		{"log_line_prefix", PGC_SIGHUP, LOGGING_WHAT,
 			gettext_noop("Controls information prefixed to each log line."),
@@ -11384,4 +11398,30 @@ check_default_with_oids(bool *newval, void **extra, GucSource source)
 	return true;
 }
 
+static void
+assign_environment_settings(const char *newval, void *extra)
+{
+	char *rawstring = pstrdup(newval);
+	List *elemlist;
+	ListCell *lc;
+
+	if (!SplitGUCList(rawstring, ',', &elemlist))
+	{
+		/* syntax error in list */
+		list_free_deep(elemlist);
+		pfree(rawstring);
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("invalid list syntax in parameter \"%s\"",
+						"parameter")));
+	}
+
+	foreach(lc, elemlist)
+	{
+		char	   *setting = lfirst(lc);
+
+		putenv(guc_strdup(ERROR, setting));
+	}
+}
+
 #include "guc-file.c"
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 194f312096..551a02d2aa 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -676,6 +676,7 @@
 # - Other Defaults -
 
 #dynamic_library_path = '$libdir'
+#environment = ''
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c07e7b945e..fdcbb90bb2 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -273,6 +273,8 @@ extern int	tcp_keepalives_count;
 extern bool trace_sort;
 #endif
 
+extern char *environment_settings;
+
 /*
  * Functions exported by guc.c
  */

base-commit: 050710b36964dee7e1b2bf6b5ef00041fd5d2787
-- 
2.20.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



Attachments:

  [text/plain] v1-0001-Add-GUC-parameter-environment.patch (5.0K, ../744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com/2-v1-0001-Add-GUC-parameter-environment.patch)
  download | inline diff:
From dd76033c87ddfe60c50e6bb397c123f489cadc1c 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 1/2] Add GUC parameter "environment"

This allows setting environment variables inside the server processes,
for use by third-party libraries, like LDAPCONF.  Previously, this had
to be done via external mechanisms such as init scripts.
---
 doc/src/sgml/config.sgml                      | 21 ++++++++++
 src/backend/utils/misc/guc.c                  | 40 +++++++++++++++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/utils/guc.h                       |  2 +
 4 files changed, 64 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8bd57f376b..acbcf5559b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8304,6 +8304,27 @@ <title>Other Defaults</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-environment" xreflabel="environment">
+      <term><varname>environment</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>environment</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        A comma-separated list of environment variable assignments to apply
+        inside the <productname>PostgreSQL</productname> server process.  This
+        can be used to supply configuration information or debugging settings
+        to third-party libraries or plugins.  Example:
+<programlisting>environment = 'LDAPCONF=/some/file,RSYNC_RSH=ssh'</programlisting>
+        This parameter can only be set in the
+        <filename>postgresql.conf</filename> file or on the server command
+        line.  Note that changing this parameter at run time will not unset
+        previously set environment variables.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-gin-fuzzy-search-limit" xreflabel="gin_fuzzy_search_limit">
       <term><varname>gin_fuzzy_search_limit</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 156d147c85..10c9528729 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -212,6 +212,7 @@ static bool check_recovery_target_lsn(char **newval, void **extra, GucSource sou
 static void assign_recovery_target_lsn(const char *newval, void *extra);
 static bool check_primary_slot_name(char **newval, void **extra, GucSource source);
 static bool check_default_with_oids(bool *newval, void **extra, GucSource source);
+static void assign_environment_settings(const char *newval, void *extra);
 
 /* Private functions in guc-file.l that need to be called from guc.c */
 static ConfigVariable *ProcessConfigFileInternal(GucContext context,
@@ -529,6 +530,8 @@ int			tcp_keepalives_idle;
 int			tcp_keepalives_interval;
 int			tcp_keepalives_count;
 
+char	   *environment_settings;
+
 /*
  * SSL renegotiation was been removed in PostgreSQL 9.5, but we tolerate it
  * being set to zero (meaning never renegotiate) for backward compatibility.
@@ -3516,6 +3519,17 @@ static struct config_string ConfigureNamesString[] =
 		check_client_encoding, assign_client_encoding, NULL
 	},
 
+	{
+		{"environment", PGC_SIGHUP, CLIENT_CONN_OTHER,
+			gettext_noop("Environment variables to be set."),
+			NULL,
+			GUC_LIST_INPUT | GUC_LIST_QUOTE
+		},
+		&environment_settings,
+		"",
+		NULL, assign_environment_settings, NULL
+	},
+
 	{
 		{"log_line_prefix", PGC_SIGHUP, LOGGING_WHAT,
 			gettext_noop("Controls information prefixed to each log line."),
@@ -11384,4 +11398,30 @@ check_default_with_oids(bool *newval, void **extra, GucSource source)
 	return true;
 }
 
+static void
+assign_environment_settings(const char *newval, void *extra)
+{
+	char *rawstring = pstrdup(newval);
+	List *elemlist;
+	ListCell *lc;
+
+	if (!SplitGUCList(rawstring, ',', &elemlist))
+	{
+		/* syntax error in list */
+		list_free_deep(elemlist);
+		pfree(rawstring);
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("invalid list syntax in parameter \"%s\"",
+						"parameter")));
+	}
+
+	foreach(lc, elemlist)
+	{
+		char	   *setting = lfirst(lc);
+
+		putenv(guc_strdup(ERROR, setting));
+	}
+}
+
 #include "guc-file.c"
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 194f312096..551a02d2aa 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -676,6 +676,7 @@
 # - Other Defaults -
 
 #dynamic_library_path = '$libdir'
+#environment = ''
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c07e7b945e..fdcbb90bb2 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -273,6 +273,8 @@ extern int	tcp_keepalives_count;
 extern bool trace_sort;
 #endif
 
+extern char *environment_settings;
+
 /*
  * Functions exported by guc.c
  */

base-commit: 050710b36964dee7e1b2bf6b5ef00041fd5d2787
-- 
2.20.1



  [text/plain] v1-0002-pg_ctl-Add-clear-environment-option.patch (6.7K, ../744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com/3-v1-0002-pg_ctl-Add-clear-environment-option.patch)
  download | inline diff:
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



view thread (10+ messages)  latest in thread

Message-ID: <744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com>
Permalink:  ../744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com/
Also on:    postgresql.org/message-id/744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: peter.eisentraut@2ndquadrant.com
  Subject: Re: libpq environment variables in the server
  In-Reply-To: <744962da-c27f-c992-4058-364463857ee0@2ndquadrant.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox