gpconfig: add validation for readonly GUCs

-- metadata about gucs, like GUC_DISALLOW_IN_FILE, is not available via
sql. Work around that by parsing the C code that defines this metadata, and store
relevant information in a file, to be read by gpconfig when called to
change a GUC.

-- the Makefile in gpMgmt/bin will create a local file during 'make install', used by
gpconfig to validate that a given GUC can be changed
Signed-off-by: NC.J. Jameson <cjameson@pivotal.io>
Signed-off-by: NShoaib Lari <slari@pivotal.io>
Signed-off-by: NNadeem Ghani <nghani@pivotal.io>
Signed-off-by: NMarbin Tan <mtan@pivotal.io>

gpMgmt/bin/Makefile

@@ -29,6 +29,7 @@ LIB_DIR=$(SRC)/lib
 PYLIB_DIR=$(SRC)/ext
 
 core: pygresql subprocess32
+	python gpconfig_modules/parse_guc_metadata.py $(prefix)
 
 ifneq "$(wildcard $(CURDIR)/pythonSrc/ext/*.tar.gz)" ""
 all: core lockfile paramiko pycrypto stream psutil
@@ -246,6 +247,7 @@ clean :
 	@rm -rf $(STREAM_DIR)/stream lib/stream  
 	@rm -rf *.pyc lib/*.pyc
 	@rm -f gpcheckcatc gpcrondumpc gpexpandc gptransferc
+	@rm -f gpconfig_modules/gucs_disallowed_in_file.txt
 
 .PHONY: disclean
 distclean: clean

gpMgmt/bin/gpconfig

@@ -21,6 +21,7 @@ try:
     from gpconfig_modules.file_segment_guc import FileSegmentGuc
     from gpconfig_modules.guc_collection import GucCollection
     from gppylib.gpresgroup import GpResGroup
+    from gpconfig_modules.parse_guc_metadata import ParseGuc
 except ImportError as err:
     sys.exit('Cannot import modules.  Please check that you have sourced '
              'greenplum_path.sh.  Detail: ' + str(err))
@@ -29,6 +30,7 @@ EXECNAME = os.path.split(__file__)[-1]
 
 PROHIBITED_GUCS = set(["port", "listen_addresses"])
 SAMEVALUE_GUCS = set(["gp_default_storage_options"])
+read_only_gucs = set()  # populated at runtime
 LOGGER = get_default_logger()
 setup_tool_logging(EXECNAME, getLocalHostname(), getUserName())
 
@@ -75,6 +77,10 @@ def validate_mutual_options(options):
     if user is None or user == ' ':
         log_and_raise("USER environment variable must be set.")
 
+    gphome = os.getenv('GPHOME')
+    if not gphome:
+        log_and_raise("GPHOME environment variable must be set.")
+
     if options.file and not options.show:
         log_and_raise("'--file' option must accompany '--show' option")
     if options.file and options.file_compare and options.show:
@@ -273,7 +279,7 @@ def do_change(options):
         gp_array = GpArray.initFromCatalog(dbconn.DbURL(), utility=True)
 
         if not options.skipvalidation:
-            validate_options(options)
+            validate_change_options(options)
 
     except DatabaseError as ex:
         LOGGER.error(ex.__str__())
@@ -348,7 +354,33 @@ def do_change(options):
         LOGGER.info("completed successfully")
 
 
-def validate_options(options):
+def _is_guc_writeable(options):
+    """
+FYI, metadata about gucs, like GUC_DISALLOW_IN_FILE, is not available via
+sql. We work around that by making use of a file already created during 'make install'.
+(That file is created by parsing the C code that defines all GUCS,
+ storing all properties with GUC_DISALLOW_IN_FILE into a file.)
+    """
+
+    gphome = os.getenv('GPHOME')
+    gpconfig_modules_dir = os.path.join(gphome, ParseGuc.DESTINATION_DIR)
+    disallowed_guc_file = os.path.abspath(os.path.join(gpconfig_modules_dir, ParseGuc.DESTINATION_FILENAME))
+    if os.path.exists(disallowed_guc_file):
+        with open(disallowed_guc_file) as f:
+            lines = f.readlines()
+        read_only_gucs.update([guc.strip() for guc in lines])
+    else:
+        msg = "disallowed GUCs file missing: '%s'" % disallowed_guc_file
+        LOGGER.warning(msg)
+    return options.entry not in read_only_gucs
+
+
+def validate_change_options(options):
+    if not _is_guc_writeable(options):
+        msg = "not a modifiable GUC: '%s'" % options.entry
+        LOGGER.fatal(msg)
+        raise Exception(msg)
+
     conn = dbconn.connect(dbconn.DbURL(), True)
     guc = get_normal_guc(conn, options)
     if not guc:

gpMgmt/bin/gpconfig_modules/parse_guc_metadata.py

+import errno
+import os
+import re
+import sys
+
+# this finds all instances of a property name that is both surrounded with quotes and is adjacent
+# to the metadata "GUC_DISALLOW_IN_FILE", where adjacent means not separated by a closing } bracket,
+# assuming all the separate lines have been combined together into one string
+GUCS_DISALLOWED_IN_FILE_REGEX = r"\"(\w+)\"[^}]+GUC_DISALLOW_IN_FILE"
+
+
+class ParseGuc:
+    """
+    metadata about gucs, like GUC_DISALLOW_IN_FILE, is not available via
+sql. Work around that by parsing the C code that defines this metadata, and store
+relevant information in a file, to be read by gpconfig when called to
+change a GUC.  The Makefile in gpMgmt/bin will use this parser to
+ create a file during 'make install', used by gpconfig to validate that a given GUC can be changed
+    """
+
+    # hardcoded name for file looked up by gpconfig during its runtime
+    DESTINATION_FILENAME = "gucs_disallowed_in_file.txt"
+    DESTINATION_DIR = "share/greenplum"
+
+    def __init__(self):
+        self.disallowed_in_file_pattern = re.compile(GUCS_DISALLOWED_IN_FILE_REGEX)
+
+    @staticmethod
+    def get_source_c_file_paths():
+        mypath = os.path.realpath(__file__)
+        misc_dir = os.path.join(os.path.dirname(mypath), "../../../src/backend/utils/misc")
+        return [os.path.join(misc_dir, "guc.c"), os.path.join(misc_dir, "guc_gp.c")]
+
+    def parse_gucs(self, guc_path):
+        with open(guc_path) as f:
+            lines = f.readlines()
+        combined = " ".join([line.strip() for line in lines])
+        return self.disallowed_in_file_pattern.finditer(combined)
+
+    def write_gucs_to_file(self, filepath):
+        with open(filepath, 'w') as f:
+            paths = self.get_source_c_file_paths()
+            for guc_source in paths:
+                hits = self.parse_gucs(guc_source)
+                for match in hits:
+                    title = match.groups()
+                    f.write("%s\n" % title)
+
+    def main(self):
+        if len(sys.argv) < 2:
+            print("usage: parse_guc_metadata <prefix path to installation directory>")
+            sys.exit(1)
+        prefix = sys.argv[1]
+        dir_path = os.path.join(prefix, self.DESTINATION_DIR)
+        _mkdir_p(dir_path, 0755)
+        output_file = os.path.join(dir_path, self.DESTINATION_FILENAME)
+        self.write_gucs_to_file(output_file)
+
+
+def _mkdir_p(path, mode):
+    try:
+        os.makedirs(path, mode)
+    except OSError as exc:  # Python >2.5
+        if exc.errno == errno.EEXIST and os.path.isdir(path):
+            pass
+        else:
+            raise
+
+# --------------------------------------------------------------------------
+# Main
+# --------------------------------------------------------------------------
+if __name__ == '__main__':
+    ParseGuc().main()

gpMgmt/bin/gppylib/test/unit/test_unit_gpconfig.py

@@ -6,6 +6,8 @@ import sys
 import tempfile
 
 from StringIO import StringIO
+
+import errno
 from pygresql.pg import DatabaseError
 
 from gparray import GpDB, GpArray, Segment
@@ -13,15 +15,18 @@ import shutil
 from mock import *
 from gp_unittest import *
 from gphostcache import GpHost
+from gpconfig_modules.parse_guc_metadata import ParseGuc
 
 db_singleton_side_effect_list = []
 
 
 def singleton_side_effect(unused1, unused2):
     # this function replaces dbconn.execSQLForSingleton(conn, sql), conditionally raising exception
-    if db_singleton_side_effect_list[0] == "DatabaseError":
-        raise DatabaseError("mock exception")
-    return db_singleton_side_effect_list[0]
+    if len(db_singleton_side_effect_list) > 0:
+        if db_singleton_side_effect_list[0] == "DatabaseError":
+            raise DatabaseError("mock exception")
+        return db_singleton_side_effect_list[0]
+    return None
 
 
 class GpConfig(GpTestCase):
@@ -45,6 +50,7 @@ class GpConfig(GpTestCase):
 
         self.os_env = dict(USER="my_user")
         self.os_env["MASTER_DATA_DIRECTORY"] = self.temp_dir
+        self.os_env["GPHOME"] = self.temp_dir
         self.gparray = self._create_gparray_with_2_primary_2_mirrors()
         self.host_cache = Mock()
 
@@ -81,6 +87,12 @@ class GpConfig(GpTestCase):
         ])
         sys.argv = ["gpconfig"]  # reset to relatively empty args list
 
+        shared_dir = os.path.join(self.temp_dir, ParseGuc.DESTINATION_DIR)
+        _mkdir_p(shared_dir, 0755)
+        self.guc_disallowed_readonly_file = os.path.abspath(os.path.join(shared_dir, ParseGuc.DESTINATION_FILENAME))
+        with open(self.guc_disallowed_readonly_file, 'w') as f:
+            f.writelines("x\ny\n")
+
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
         super(GpConfig, self).tearDown()
@@ -350,6 +362,43 @@ class GpConfig(GpTestCase):
         self.assertIn("[context: -1] [dbid: 2] [name: my_property_name] [value: foo | file: bar]",
                       mock_stdout.getvalue())
 
+    def test_setting_guc_when_guc_is_readonly_will_fail(self):
+        self.subject.read_only_gucs.add("is_superuser")
+        sys.argv = ["gpconfig", "-c", "is_superuser", "-v", "on"]
+        with self.assertRaisesRegexp(Exception, "not a modifiable GUC: 'is_superuser'"):
+            self.subject.do_main()
+
+    def test_change_will_populate_read_only_gucs_set(self):
+        sys.argv = ["gpconfig", "--change", "foobar", "--value", "baz"]
+        try:
+            self.subject.do_main()
+        except Exception:
+            pass
+        self.assertEqual(len(self.subject.read_only_gucs), 2)
+
+    def test_change_when_disallowed_gucs_file_is_missing_gives_warning(self):
+        os.remove(self.guc_disallowed_readonly_file)
+        db_singleton_side_effect_list.append("some happy result")
+        entry = 'my_property_name'
+        sys.argv = ["gpconfig", "-c", entry, "-v", "100", "--masteronly"]
+        # 'SELECT name, setting, unit, short_desc, context, vartype, min_val, max_val FROM pg_settings'
+        self.cursor.set_result_for_testing([['my_property_name', 'setting', 'unit', 'short_desc',
+                                             'context', 'vartype', 'min_val', 'max_val']])
+        self.subject.do_main()
+
+        self.subject.LOGGER.info.assert_called_with("completed successfully")
+        target_warning = "disallowed GUCs file missing: '%s'" % self.guc_disallowed_readonly_file
+        self.subject.LOGGER.warning.assert_called_with(target_warning)
+
+    def test_when_gphome_env_unset_raises(self):
+        self.os_env['GPHOME'] = None
+        sys.argv = ["gpconfig", "-c", 'my_property_name', "-v", "100", "--masteronly"]
+
+        with self.assertRaisesRegexp(Exception, "GPHOME environment variable must be set"):
+            self.subject.do_main()
+
+
+
     @staticmethod
     def _create_gparray_with_2_primary_2_mirrors():
         master = GpDB.initFromString(
@@ -365,5 +414,15 @@ class GpConfig(GpTestCase):
         return GpArray([master, primary0, primary1, mirror0, mirror1])
 
 
+def _mkdir_p(path, mode):
+    try:
+        os.makedirs(path, mode)
+    except OSError as exc:  # Python >2.5
+        if exc.errno == errno.EEXIST and os.path.isdir(path):
+            pass
+        else:
+            raise
+
+
 if __name__ == '__main__':
     run_tests()

gpMgmt/bin/gppylib/test/unit/test_unit_parse_guc_metadata.py

+import os
+
+import sys
+
+from gp_unittest import *
+from gpconfig_modules.parse_guc_metadata import ParseGuc
+
+
+class TestParseGucMetadata(GpTestCase):
+    def setUp(self):
+        self.subject = ParseGuc()
+        self.DEST_DIR = "/tmp/test_parseguc/bar"
+
+    def test_main_writes_file(self):
+        sys.argv = ["parse_guc_metadata", self.DEST_DIR]
+
+        self.subject.main()
+
+        dest_file = os.path.join(self.DEST_DIR, "share/greenplum", self.subject.DESTINATION_FILENAME)
+        with open(dest_file, 'r') as f:
+            lines = f.readlines()
+        self.assertIn('is_superuser\n', lines)
+        self.assertIn('gp_session_id\n', lines)
+
+    def test_main_when_no_prefix_will_fail(self):
+        sys.argv = ["parse_guc_metadata"]
+
+        self.assertRaises(SystemExit, self.subject.main)
+
+
+if __name__ == '__main__':
+    run_tests()