[7X] gpconfig does not escape '$' char

Issue:
gpconfig is not able to set the guc correctly if the value contains '$'.
It considers the string as a shell var and evaluates it, instead of escaping the '$' character

    [~/workspace/gpdb/gpMgmt: {main} ?]$ gpconfig --verbose -c dynamic_library_path -v '$libdir:/foo/bar'
    20230906:11:33:38:023885 gpconfig:-[INFO]:-completed successfully with parameters '--verbose -c dynamic_library_path -v '$libdir:/foo/bar''

    [~/workspace/gpdb/gpMgmt: {main} ?]$ gpstop -u
    20230906:11:35:17:024203 gpstop:xx-[INFO]:-Greenplum Version: 'postgres (Greenplum Database) 7.0.0-beta.4+dev.254.g1621aed51f build dev'
    20230906:11:35:17:024203 gpstop:xx-[INFO]:-Signalling all postmaster processes to reload

    [~/workspace/gpdb/gpMgmt: {main} ?]$ gpconfig -s dynamic_library_path
    Values on all segments are consistent
    GUC              : dynamic_library_path
    Coordinator value: :/foo/bar
    Segment     value: :/foo/bar

Reason: gpconfig does not encode the guc value when invoking the gpconfig_helper
script to add the guc parameter to postgressql.conf. Just quotes the value using
shlex.quote().Value is sent as qouted plain text hence the $ char is not escaped
and the value string is considered as shell variable. shlex.quote() is not
efficient enough to add escape char ('/') before '$' in guc value. This issue does
not exist in 6X as value is encoded using base64.urlsafe_b64encode() then sent to
segment hosts.

Fix: Encoding the value at coordinator and then decoding it on segment host will make
sure that value string is intact. Reverted the gpconfig changes done in commit
50b06308db1e3f0b4e310c25f4501e6f11884d6a which introduced the use of shlex.quote() to
qoute the guc values instead of encoding it.

After fix:
[~/workspace/gpdb/gpMgmt: {gpconfig-issue} ?]$ gpconfig -c dynamic_library_path -v '$libdir:/foo/bar'
20230915:16:55:55:054429 gpconfig:-[INFO]:-completed successfully with parameters '-c dynamic_library_path -v '$libdir:/foo/bar''

[~/workspace/gpdb/gpMgmt: {gpconfig-issue} ?]$ gpstop -u
20230915:16:56:11:054645 gpstop:-[INFO]:-Greenplum Version: 'postgres (Greenplum Database) 7.0.0-beta.4+dev.254.g1621aed51f build dev'
20230915:16:56:11:054645 gpstop:-[INFO]:-Signalling all postmaster processes to reload

[~/workspace/gpdb/gpMgmt: {gpconfig-issue} ?]$ gpconfig -s dynamic_library_path
Values on all segments are consistent
GUC              : dynamic_library_path
Coordinator value: $libdir:/foo/bar
Segment     value: $libdir:/foo/bar

(cherry picked from commit 9383cb7612dbd7e2e05100e85e0ac47c763fa03c)

gpMgmt/bin/gppylib/commands/gp.py

@@ -8,6 +8,8 @@
 """
 import json
 import os, time
+import base64
+import pickle
 import shlex
 import os.path
 import pipes
@@ -1101,7 +1103,7 @@ def __init__(self, command_name, postgresconf_dir, name, value=None, segInfo=Non
 
         addParameter = (not getParameter) and (not removeParameter)
         if addParameter:
-            args = "--add-parameter %s --value %s " % (name, shlex.quote(value))
+            args = "--add-parameter %s --value %s " % (name, base64.urlsafe_b64encode(pickle.dumps(value)).decode())
         if getParameter:
             args = "--get-parameter %s" % name
         if removeParameter:
@@ -1115,7 +1117,8 @@ def __init__(self, command_name, postgresconf_dir, name, value=None, segInfo=Non
 
     # FIXME: figure out how callers of this can handle exceptions here
     def get_value(self):
-        return self.get_results().stdout
+        raw_value = self.get_results().stdout
+        return pickle.loads(base64.urlsafe_b64decode(raw_value))
 
 
 #-----------------------------------------------

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

@@ -1,7 +1,7 @@
 import errno
 import imp
 import os
-import shlex
+import base64, pickle
 import shutil
 import sys
 import tempfile
@@ -289,11 +289,11 @@ def test_option_change_value_coordinator_separate_succeed(self):
         self.assertEqual(self.pool.addCommand.call_count, 5)
         segment_command = self.pool.addCommand.call_args_list[0][0][0]
         self.assertTrue("my_property_name" in segment_command.cmdStr)
-        value = shlex.quote("100")
+        value = base64.urlsafe_b64encode(pickle.dumps("100")).decode()
         self.assertTrue(value in segment_command.cmdStr)
         coordinator_command = self.pool.addCommand.call_args_list[4][0][0]
         self.assertTrue("my_property_name" in coordinator_command.cmdStr)
-        value = shlex.quote("20")
+        value = base64.urlsafe_b64encode(pickle.dumps("20")).decode()
         self.assertTrue(value in coordinator_command.cmdStr)
 
     def test_option_change_value_coordinatoronly_succeed(self):
@@ -312,7 +312,7 @@ def test_option_change_value_coordinatoronly_succeed(self):
         self.assertEqual(self.pool.addCommand.call_count, 1)
         coordinator_command = self.pool.addCommand.call_args_list[0][0][0]
         self.assertTrue(("my_property_name") in coordinator_command.cmdStr)
-        value = shlex.quote("100")
+        value = base64.urlsafe_b64encode(pickle.dumps("100")).decode()
         self.assertTrue(value in coordinator_command.cmdStr)
 
     def test_new_option_change_value_coordinatoronly_succeed(self):
@@ -331,7 +331,7 @@ def test_new_option_change_value_coordinatoronly_succeed(self):
         self.assertEqual(self.pool.addCommand.call_count, 1)
         coordinator_command = self.pool.addCommand.call_args_list[0][0][0]
         self.assertTrue(("my_property_name") in coordinator_command.cmdStr)
-        value = shlex.quote("100")
+        value = base64.urlsafe_b64encode(pickle.dumps("100")).decode()
         self.assertTrue(value in coordinator_command.cmdStr)
 
     def test_old_and_new_option_change_value_coordinatoronly_succeed(self):
@@ -350,7 +350,7 @@ def test_old_and_new_option_change_value_coordinatoronly_succeed(self):
         self.assertEqual(self.pool.addCommand.call_count, 1)
         coordinator_command = self.pool.addCommand.call_args_list[0][0][0]
         self.assertTrue(("my_property_name") in coordinator_command.cmdStr)
-        value = shlex.quote("100")
+        value = base64.urlsafe_b64encode(pickle.dumps("100")).decode()
         self.assertTrue(value in coordinator_command.cmdStr)
 
 
@@ -373,7 +373,7 @@ def test_option_change_value_hidden_guc_with_skipvalidation(self):
         self.assertTrue("my_hidden_guc_name" in segment_command.cmdStr)
         coordinator_command = self.pool.addCommand.call_args_list[1][0][0]
         self.assertTrue("my_hidden_guc_name" in coordinator_command.cmdStr)
-        value = shlex.quote("100")
+        value = base64.urlsafe_b64encode(pickle.dumps("100")).decode()
         self.assertTrue(value in coordinator_command.cmdStr)
 
     def test_option_change_value_hidden_guc_without_skipvalidation(self):
@@ -579,7 +579,7 @@ def validation_for_testing_quoting_string_values(self, expected_value):
             # In this case, we have an object as an argument to poo.addCommand
             # call_obj[1] returns a dict for all named arguments -> {key='arg3', key2='arg4'}
             gp_add_config_script_obj = call[0][0]
-            value = shlex.quote(expected_value)
+            value = base64.urlsafe_b64encode(pickle.dumps(expected_value)).decode()
             try:
                 self.assertTrue(value in gp_add_config_script_obj.cmdStr)
             except AssertionError as e:

gpMgmt/sbin/gpconfig_helper.py

@@ -16,6 +16,8 @@
     import shutil
     import sys
     import tempfile
+    import base64
+    import pickle
 
     from optparse import Option, OptionParser
     from gppylib.gpparseopts import OptParser, OptChecker
@@ -101,7 +103,7 @@ def add_parameter(filename, name, value):
         for line in lines:
             outfile.write(line)
             new_lines = new_lines + 1
-        outfile.write(name + '=' + value + os.linesep)
+        outfile.write(name + '=' + pickle.loads(base64.urlsafe_b64decode(value)) + os.linesep)
         new_lines = new_lines + 1
 
     if new_lines == len(lines) + 1:
@@ -122,7 +124,7 @@ def main():
     if options.get_parameter:
         try:
             value = get_parameter(options.file, options.get_parameter)
-            sys.stdout.write(value)
+            sys.stdout.write(base64.urlsafe_b64encode(pickle.dumps(value)).decode())
             return
         except Exception as err:
             sys.stderr.write("Failed to get value for parameter '%s' in file %s due to: %s" % (

gpMgmt/test/behave/mgmt_utils/gpconfig.feature

@@ -74,6 +74,7 @@ Feature: gpconfig integration tests
         | float                                       | checkpoint_completion_target | float      | 0.4        | 0.5       | 0.5        | 0.5        | 0.33                   | 0.33                        | 0.7               | 0.7                    | 0.7                    |
         | basic string                                | application_name             | string     | xxxxxx     | bodhi     | 'bodhi'    | bodhi      | lucy                   | 'lucy'                      | bengie            | 'bengie'               | bengie                 |
         | string with spaces                          | application_name             | string     | yyyyyy     | 'bod hi'  | 'bod hi'   | bod hi     | 'bod hi'               | 'bod hi'                    | 'bod hi'          | 'bod hi'               | bod hi                 |
+        | string with special characters              | application_name             | string     | yyyyyy     | '$li@d#r' | '$li@d#r'   | $li@d#r   | '$li@d#r'              | '$li@d#r'                   | '$li@d#r'         | '$li@d#r'              | $li@d#r                |
         | different value on coordinator and segments | application_name             | string     | yyyyyy     | 'bod hi'  | 'bod hi'   | bod hi     | 'lu cy'                | 'lu cy'                     | 'ben gie'         | 'ben gie'              | ben gie                |
         | empty string                                | application_name             | string     | zzzzzz     | ''        | ''         |            | ''                     | ''                          | ''                | ''                     |                        |
         | quoted double quotes                        | application_name             | string     | zzzzzz     | '"hi"'    | '"hi"'     | "hi"       | '"hi"'                 | '"hi"'                      | '"hi"'            | '"hi"'                 | "hi"                   |