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

Fix postgresql_set module if single-value param contains comma in value #400

Merged
merged 4 commits into from
Jan 26, 2023
Merged

Conversation

RealGreenDragon
Copy link
Contributor

@RealGreenDragon RealGreenDragon commented Jan 21, 2023

SUMMARY

The PR #357 fix the multi-value parameters management (issue #78), but the new check assumes as multi-value each parameter with a comma in the value, that is incorrect as there are single-value parameters with comma in value.

If a single-value parameter is treated as a multi-value, the 'param_set' function builds an ALTER SYSTEM SET command with multiple comma-separated values, that fails with the message:

ERROR:  SET log_line_prefix takes only one argument

Single-value parameters with possible comma in value are "_command" and "_prefix" that for exmple in v12 are:

  • ssl_passphrase_command
  • archive_command
  • restore_command
  • archive_cleanup_command
  • recovery_end_command
  • log_line_prefix

The most critical (that I add in tests) is 'log_line_prefix' because often it contains comma and a space at the end.

I simply fix the check to evaluate as single-value all parameters that ends with '_command' and '_prefix'.

If the fix is accepted, It can be back-ported also in stable branch.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

postgresql_set

@hunleyd
Copy link
Collaborator

hunleyd commented Jan 23, 2023

set pgaudit.log = 'write, ddl';
set pgaudit.log_relation = on;

would still be handled incorrectly, yes?

@RealGreenDragon
Copy link
Contributor Author

Hi hunleyd,

the parameters pgaudit.log and pgaudit.log_relation are not affected by my PR.
pgaudit.log_relation does not contains commas, so always go to the standard management.
pgaudit.log contains commas but is multi-value, so it is correctly not not affected by my PR (i.e. still go to specific management).

Small example with python3 and pg 12.13 (from postgres user):

$ python3
>>> value='role, ddl'
>>> ','.join(["'" + elem.strip() + "'" for elem in value.split(',')])
"'role','ddl'"
>>>

$ psql
postgres=# alter system set pgaudit.log = 'role','ddl';
ALTER SYSTEM

$ grep "pgaudit.log =" 12/data/postgresql.auto.conf
pgaudit.log = 'role, ddl'

Copy link
Collaborator

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealGreenDragon hi, welcome and thanks for the PR!

A couple of minor things from me.

plugins/modules/postgresql_set.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
@RealGreenDragon
Copy link
Contributor Author

@Andersson007 Suggestions applied, thank you very much!

@hunleyd hunleyd requested a review from Andersson007 January 25, 2023 20:46
@hunleyd
Copy link
Collaborator

hunleyd commented Jan 26, 2023

thx @RealGreenDragon !

@hunleyd hunleyd merged commit 939c2cd into ansible-collections:main Jan 26, 2023
@patchback
Copy link

patchback bot commented Jan 26, 2023

Backport to stable-1: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-1/939c2cd611e06017e234931afeadaab4a271b973/pr-400

Backported as #403

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 26, 2023
…ue (#400)

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
(cherry picked from commit 939c2cd)
@Andersson007
Copy link
Collaborator

@RealGreenDragon thanks for the contribution!

@hunleyd @jchancojr thanks for reviewing!

@RealGreenDragon if you're not in the #postgresql:ansible.com room on Matrix yet, welcome!

hunleyd pushed a commit that referenced this pull request Jan 26, 2023
…ue (#400) (#403)

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
(cherry picked from commit 939c2cd)

Co-authored-by: RealGreenDragon <14246920+RealGreenDragon@users.noreply.github.com>
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.

4 participants