Fix postgresql_conf quote logic #297
Conversation
|
Can one of the admins verify this patch? |
|
Can you add system tests for this @reidmv? I think the pattern already reflected here in the current test could be mirrored for setting an ip address: https://github.com/puppetlabs/puppetlabs-postgresql/blob/master/spec/system/server/config_entry_spec.rb |
| @@ -18,7 +18,7 @@ | |||
| :to_line => proc { |h| | |||
|
|
|||
| # simple string and numeric values don't need to be enclosed in quotes | |||
| dontneedquote = h[:value].match(/^(\w+|[0-9.-]+)$/) | |||
There was a problem hiding this comment.
We really need to be careful here, and make sure what we end up with doesn't break something else. A few more tests like shared_buffers and other simple values is wise here, but we should probably verify the quoting rules ourselves.
There was a problem hiding this comment.
I've done some poking and prodding on a PostgreSQL 9.2 server with regards to quoting or not quoting data values that are strings, floating point numbers, and ip addresses. Unscientific results suggest that quoting is always acceptable, and is strictly required for values that don't match the ruby regex /\w+/.
There was a problem hiding this comment.
Well, we could change it. I think we'd want a few more tests around configuration items (using a similar pattern as we already have, perhaps even more reduced) but I think this could work. We do test this against Centos 5.9, so that gets the 8.1 coverage.
|
@reidmv were you going to change this default behaviour to quoting mate? |
|
The annoying thing is, that if there are quotes in the value, they also get added. I tried to get around this bug by doing |
|
@reidmv Can you rebase this and we'll see if the tests start passing? Also, if I understand it correctly, we now need beaker tests to merge stuff into this module. |
Things like IP addresses DO need to be in quotes. Fixing regex to reflect requirement.
|
@nibalizer rebased onto current master, and Travis CI looks happy. Looks like the tests that were failing were not related to this code. |
Fix postgresql_conf quote logic
…te_logic Fix postgresql_conf quote logic
Things like IP addresses DO need to be in quotes or PostgreSQL 9.2 won't start. Fixing regex to reflect requirement.
Can demonstrate failure pre-fix by passing an ip address in as the $listen_addresses parameter to postgresql::server.