From 6e90d7fb54609cbe2806d3c10e0e36c49ab4f0c5 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut 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 @@ Options + + + + + 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 start and any + restart actions. + + + + 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