diff --git a/gpMgmt/bin/analyzedb b/gpMgmt/bin/analyzedb index ff7ced099f1..a0637d54a0a 100755 --- a/gpMgmt/bin/analyzedb +++ b/gpMgmt/bin/analyzedb @@ -167,12 +167,24 @@ SELECT schemaname, tablename FROM ORDER BY tableoid DESC; """ +def quote_literal(value): + """ + Quote a name for use inside a SQL string literal. + + Not pg.escape_string(): PyGreSQL's C implementation encodes its argument as + ASCII and raises UnicodeEncodeError on a schema, table or column name that + contains non-ASCII characters. With standard_conforming_strings on -- the + default -- doubling single quotes is the whole of the escaping needed. + """ + return value.replace("'", "''") + + def validate_schema_exists(pg_port, dbname, schema): conn = None try: dburl = dbconn.DbURL(port=pg_port, dbname=dbname) conn = dbconn.connect(dburl) - count = dbconn.querySingleton(conn, "select count(*) from pg_namespace where nspname='%s';" % pg.escape_string(schema)) + count = dbconn.querySingleton(conn, "select count(*) from pg_namespace where nspname='%s';" % quote_literal(schema)) if count == 0: raise ExceptionNoStackTraceNeeded("Schema %s does not exist in database %s." % (schema, dbname)) finally: @@ -982,7 +994,7 @@ def get_oid_str(table_list): def regclass_schema_tbl(schema, tbl): schema_tbl = "%s.%s" % (escape_identifier(schema), escape_identifier(tbl)) - return "to_regclass('%s')" % (pg.escape_string(schema_tbl)) + return "to_regclass('%s')" % quote_literal(schema_tbl) # Escape double-quotes in a string, so that the resulting string is suitable for @@ -1250,7 +1262,7 @@ def validate_tables(conn, tablenames): while curr_batch < nbatches: batch = tablenames[curr_batch * batch_size:(curr_batch + 1) * batch_size] - oid_str = ','.join(map((lambda x: "('%s')" % pg.escape_string(x)), batch)) + oid_str = ','.join(map((lambda x: "('%s')" % quote_literal(x)), batch)) if not oid_str: break @@ -1266,7 +1278,7 @@ def get_include_cols_from_exclude(conn, schema, table, exclude_cols): """ Given a list of excluded columns of a table, get the list of included columns """ - quoted_exclude_cols = ','.join(["'%s'" % pg.escape_string(x) for x in exclude_cols]) + quoted_exclude_cols = ','.join(["'%s'" % quote_literal(x) for x in exclude_cols]) oid_str = regclass_schema_tbl(schema, table) cols = run_sql(conn, GET_INCLUDED_COLUMNS_FROM_EXCLUDE_SQL % (oid_str, quoted_exclude_cols)) @@ -1282,7 +1294,7 @@ def validate_columns(conn, schema, table, column_list): return sql = VALIDATE_COLUMN_NAMES_SQL % (regclass_schema_tbl(schema, table), - ','.join(["'%s'" % pg.escape_string(x) for x in column_list])) + ','.join(["'%s'" % quote_literal(x) for x in column_list])) valid_col_count = dbconn.querySingleton(conn, sql) if int(valid_col_count) != len(column_list): diff --git a/gpMgmt/bin/gppylib/commands/unix.py b/gpMgmt/bin/gppylib/commands/unix.py index 4708b7ac574..b3f922bd2fe 100644 --- a/gpMgmt/bin/gppylib/commands/unix.py +++ b/gpMgmt/bin/gppylib/commands/unix.py @@ -55,16 +55,41 @@ def check_pid_on_remotehost(pid, host): def check_pid(pid): - """ Check For the existence of a unix pid. """ + """ + Check for the existence of a unix pid, meaning a process that is still + running. + + os.kill(pid, 0) on its own is not that question: it keeps succeeding for a + process that has exited and has not been reaped, and nothing reaps an orphan + in a container started without an init, so such a zombie answers yes for the + life of the container. Every caller here is deciding whether something is + still working -- a postmaster, a utility holding a lock, a segment that was + just signalled -- and for all of them a zombie has gone. + """ + + try: + pid = int(pid) + except (TypeError, ValueError): + # A lock file that was created but never written leaves an empty PID + # behind; whatever wrote it is not running either way, and the callers + # ask this question to decide whether to clean up. + return False if pid == 0: return False try: - os.kill(int(pid), signal.SIG_DFL) + os.kill(pid, signal.SIG_DFL) except OSError: return False - else: + + try: + with open('/proc/%d/stat' % pid) as statfile: + # The comm field is parenthesised and may itself contain spaces and + # parentheses, so state is the first field after the last ')'. + return statfile.read().rpartition(')')[2].split()[0] != 'Z' + except (IOError, OSError, IndexError, ValueError): + # No /proc to consult; the kill() above is the best answer available. return True diff --git a/gpMgmt/bin/gppylib/mainUtils.py b/gpMgmt/bin/gppylib/mainUtils.py index e947639591d..beaef5cd58f 100644 --- a/gpMgmt/bin/gppylib/mainUtils.py +++ b/gpMgmt/bin/gppylib/mainUtils.py @@ -307,13 +307,13 @@ def simple_main_internal(createOptionParserFn, createCommandFn, mainOptions): # at this point we have whatever lock we require try: - simple_main_locked(parserOptions, parserArgs, createCommandFn, mainOptions) + simple_main_locked(parser, parserOptions, parserArgs, createCommandFn, mainOptions) finally: if sml is not None: sml.release() -def simple_main_locked(parserOptions, parserArgs, createCommandFn, mainOptions): +def simple_main_locked(parser, parserOptions, parserArgs, createCommandFn, mainOptions): """ Not to be called externally -- use simple_main instead """ @@ -326,7 +326,6 @@ def simple_main_locked(parserOptions, parserArgs, createCommandFn, mainOptions): faultProberInterface.registerFaultProber(faultProberImplGpdb.GpFaultProberImplGpdb()) commandObject = None - parser = None forceQuiet = mainOptions is not None and mainOptions.get("forceQuietOutput") diff --git a/gpMgmt/bin/gppylib/test/unit/test_unit_seg_update_pg_hba.py b/gpMgmt/bin/gppylib/test/unit/test_unit_seg_update_pg_hba.py index 5ac6aee9249..5ddcbb17bd6 100644 --- a/gpMgmt/bin/gppylib/test/unit/test_unit_seg_update_pg_hba.py +++ b/gpMgmt/bin/gppylib/test/unit/test_unit_seg_update_pg_hba.py @@ -67,3 +67,87 @@ def test_remove_dup_with_spaces_add_entries(self): expected_out_entries = expected_block.\ format(ip_mirror1='10.0.0.1', ip_mirror2='10.0.0.2', ip_mirror3='10.0.0.3').split('\n') self.assertEqual(result_out_entries, expected_out_entries) + + def test_remove_stale_replication_entries(self): + existing_entries = """host replication gpadmin samehost trust +host all gpadmin 10.0.0.1/32 trust +host all gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.1/32 trust +host replication gpadmin 10.0.0.2/32 trust""".split('\n') + + input_entries = 'host replication gpadmin samehost trust\nhost all gpadmin 10.0.0.2/32 trust' \ + '\nhost replication gpadmin 10.0.0.2/32 trust\nhost replication gpadmin 10.0.0.3/32 trust' + unique_entries = seg_update_pg_hba.remove_dup_add_entries(existing_entries, input_entries) + result = seg_update_pg_hba.remove_stale_replication_entries(unique_entries, input_entries) + + expected = """host replication gpadmin samehost trust +host all gpadmin 10.0.0.1/32 trust +host all gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.3/32 trust""".split('\n') + self.assertEqual(result, expected) + + def test_pg_hba_with_no_replication_entries(self): + existing_entries = """# standby coordinator host ip addresses +host all gpadmin 10.0.0.2/32 trust +host all gpadmin 10.0.0.1/32 trust""".split('\n') + input_entries = '# standby coordinator host ip addresses\nhost all gpadmin 10.0.0.2/32 trust' + + result = seg_update_pg_hba.remove_stale_replication_entries(existing_entries, input_entries) + + self.assertEqual(result, existing_entries) + + def test_remove_stale_replication_entries_without_removing_localhost(self): + existing_entries = """# Allow replication connections from localhost, by a user with the +# replication privilege. +local replication all trust +host replication all 127.0.0.1/32 trust +host replication all ::1/128 trust +host replication gpadmin samehost trust +host all gpadmin 10.0.0.1/32 trust +host all gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.1/32 trust +host replication gpadmin 10.0.0.2/32 trust""".split('\n') + input_entries = 'host replication gpadmin samehost trust\nhost all gpadmin 10.0.0.2/32 trust\nhost replication gpadmin 10.0.0.2/32 trust' + + result = seg_update_pg_hba.remove_stale_replication_entries(existing_entries, input_entries) + + expected = """# Allow replication connections from localhost, by a user with the +# replication privilege. +local replication all trust +host replication all 127.0.0.1/32 trust +host replication all ::1/128 trust +host replication gpadmin samehost trust +host all gpadmin 10.0.0.1/32 trust +host all gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.2/32 trust""".split('\n') + + self.assertEqual(result, expected) + + def test_remove_stale_replication_entries_without_removing_samehost(self): + existing_entries = """# Allow replication connections from localhost, by a user with the +# replication privilege. +local replication all trust +host replication all 127.0.0.1/32 trust +host replication all ::1/128 trust +host replication gpadmin samehost trust +host all gpadmin 10.0.0.1/32 trust +host all gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.1/32 trust +host replication gpadmin 10.0.0.2/32 trust""".split('\n') + input_entries = 'host all gpadmin 10.0.0.2/32 trust\nhost replication gpadmin 10.0.0.2/32 trust' + + result = seg_update_pg_hba.remove_stale_replication_entries(existing_entries, input_entries) + + expected = """# Allow replication connections from localhost, by a user with the +# replication privilege. +local replication all trust +host replication all 127.0.0.1/32 trust +host replication all ::1/128 trust +host replication gpadmin samehost trust +host all gpadmin 10.0.0.1/32 trust +host all gpadmin 10.0.0.2/32 trust +host replication gpadmin 10.0.0.2/32 trust""".split('\n') + + self.assertEqual(result, expected) + diff --git a/gpMgmt/bin/gppylib/utils.py b/gpMgmt/bin/gppylib/utils.py index 1adb1e68cdc..42fca86592e 100644 --- a/gpMgmt/bin/gppylib/utils.py +++ b/gpMgmt/bin/gppylib/utils.py @@ -505,7 +505,12 @@ def escapeDoubleQuoteInSQLString(string, forceDoubleQuote=True): def Escape(query_str): - return pgdb.escape_string(query_str) + # pgdb.escape_string() is a thin wrapper around libpq's PQescapeString, + # and the str overload encodes with the ASCII codec, so any non-ASCII + # identifier or value raises UnicodeEncodeError. Hand it UTF-8 bytes + # instead: libpq escapes those byte-for-byte and the result decodes back + # to the original text. + return pgdb.escape_string(query_str.encode('utf-8')).decode('utf-8') def escapeArrayElement(query_str): diff --git a/gpMgmt/bin/gpstop b/gpMgmt/bin/gpstop index ab7655597c9..e4123261f0e 100755 --- a/gpMgmt/bin/gpstop +++ b/gpMgmt/bin/gpstop @@ -983,5 +983,16 @@ class GpStop: etcd_hosts=options.etcd_hosts, is_external_fts=external_fts) + # Cherry-picked from Greenplum Database (greenplum-db/gpdb-archive @ 482967c1b4), + # gpMgmt/bin/gpstop; the docstring is reworded. + @staticmethod + def mainOptions(): + """ + This method returns a dictionary instructing simple_main to check for + a gpstop.lock file under COORDINATOR_DATA_DIRECTORY in order to prevent + more than one gpstop running against the same cluster at a time. + """ + return {'pidlockpath': 'gpstop.lock', 'parentpidvar': 'GPSTOPPID'} + if __name__ == '__main__': - simple_main(GpStop.createParser, GpStop.createProgram) + simple_main(GpStop.createParser, GpStop.createProgram, GpStop.mainOptions()) diff --git a/gpMgmt/bin/minirepro b/gpMgmt/bin/minirepro index 79d4c9fbb31..9a93389b14c 100755 --- a/gpMgmt/bin/minirepro +++ b/gpMgmt/bin/minirepro @@ -74,6 +74,11 @@ sysnslist = "('pg_toast', 'pg_bitmapindex', 'pg_catalog', 'information_schema', # unset search path due to CVE-2018-1058 pgoptions = '-c optimizer=off -c gp_role=utility -c search_path=' + +def escape_sql_literal(value): + return value.replace("'", "''") + + class MRQuery(object): def __init__(self): self.schemas = [] @@ -136,7 +141,7 @@ def dump_query(connectionInfo, query_file): with open(query_file, 'r') as query_f: sql_text = query_f.read() - query = "select pg_catalog.gp_dump_query_oids('%s')" % Escape(sql_text) + query = "select pg_catalog.gp_dump_query_oids('%s')" % escape_sql_literal(sql_text) toolkit_sql = PATH_PREFIX + 'toolkit.sql' with open(toolkit_sql, 'w') as toolkit_f: @@ -165,6 +170,27 @@ def parse_oids(cursor, json_oids): if len(result.funcids) == 0: result.funcids = '0' + # In PG16, gp_dump_query_oids may return only view OIDs without their + # dependent table OIDs. Expand view dependencies so pg_dump includes + # the tables that views reference. + dep_query = "SELECT DISTINCT d.refobjid FROM pg_depend d " \ + "JOIN pg_class c ON d.classid = 'pg_rewrite'::regclass " \ + "JOIN pg_rewrite r ON d.objid = r.oid " \ + "WHERE r.ev_class IN (%s) AND d.refobjid != r.ev_class " \ + "AND d.refclassid = 'pg_class'::regclass " \ + "AND d.refobjid NOT IN (%s)" % (result.relids, result.relids) + try: + cursor.execute(dep_query) + dep_oids = [str(row[0]) for row in result_iter(cursor)] + if dep_oids: + result.relids = result.relids + ',' + ','.join(dep_oids) + except pgdb.DatabaseError as e: + # The dump is still usable without the view's base tables, so warn + # and carry on with the OIDs gp_dump_query_oids gave us. + sys.stderr.write('\nWarning: could not expand view dependencies; ' + 'the dump may be missing tables referenced by views.\n\n' + + str(e) + '\n\n') + cat_query = "SELECT distinct(nspname) FROM pg_class c, pg_namespace n WHERE " \ "c.relnamespace = n.oid AND c.oid IN (%s) " \ "AND n.nspname NOT IN %s" % (result.relids, sysnslist) diff --git a/gpMgmt/sbin/seg_update_pg_hba.py b/gpMgmt/sbin/seg_update_pg_hba.py index 8e8b4c800f2..6e076e52bcc 100755 --- a/gpMgmt/sbin/seg_update_pg_hba.py +++ b/gpMgmt/sbin/seg_update_pg_hba.py @@ -72,6 +72,36 @@ def remove_dup_add_entries(lines, entries): return out_entries +# remove_stale_replication_entries() and is_stale_replication_entry() are +# cherry-picked unmodified from Greenplum Database (greenplum-db/gpdb-archive @ 482967c1b4), +# gpMgmt/sbin/seg_update_pg_hba.py. +def remove_stale_replication_entries(existing_hba, entries): + final_entries = [] + replication_entry = "replication" in entries + + for line in existing_hba: + if line not in entries.split('\n'): + # check for stale replication entries on existing pg_hba.conf + if replication_entry and is_stale_replication_entry(line): + continue + final_entries.append(line.strip()) + + return final_entries + +def is_stale_replication_entry(line): + # Here considering all non-local replication entries as stale. + # This assumes 1:1 replication and will not work with 1:N replication. + + localhost_entries = ["127.0.0.1/32", "::1/128", "local", "samehost"] + # return False if line is a comment or not a replication entry + if line.startswith('#') or "replication" not in line: + return False + # keep replication entries present for localhost + if any(entry in line for entry in localhost_entries): + return False + + return True + def run_pg_ctl_reload(datadir): name = "pg_ctl reload" cmd_str = "$GPHOME/bin/pg_ctl reload -D %s" % datadir @@ -83,7 +113,8 @@ def main(): hba_filename = options.datadir +'/pg_hba.conf' lines, temp_hba_filename = read_from_hba_file_and_get_empty_tempfile(hba_filename) out_entries = remove_dup_add_entries(lines, options.entries) - write_entries(out_entries, temp_hba_filename, hba_filename) + final_entries = remove_stale_replication_entries(out_entries, options.entries) + write_entries(final_entries, temp_hba_filename, hba_filename) run_pg_ctl_reload(options.datadir) if __name__ == "__main__":