Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cherry-pick] [7X] gpconfig does not escape '$' char #403

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

Ray-Eldath
Copy link
Contributor

@Ray-Eldath Ray-Eldath commented Apr 8, 2024

cherry-picked greenplum-db/gpdb#16449. conflicts are resolved and tested locally.

this is required by madlib support.

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)
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@my-ship-it my-ship-it merged commit be6897c into cloudberrydb:main Apr 10, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants