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.

Sign up for GitHub

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

Jump to bottom

Fix postgresql_conf quote logic #297

Merged
merged 2 commits into from Apr 3, 2014
Merged

Fix postgresql_conf quote logic #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0573c15
Fix postgresql_conf quote logic
reidmv Oct 26, 2013
a99ad69
Add tests
reidmv Oct 27, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/puppet/provider/postgresql_conf/parsed.rb
View file
Open in desktop
Expand Up @@ -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.-]+)$/)
dontneedquote = h[:value].match(/^(\w+)$/)
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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+/.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

dontneedequal = h[:name].match(/^(include|include_if_exists)$/i)

str = h[:name].downcase # normalize case
Expand Down
18 changes: 18 additions & 0 deletions spec/acceptance/server/config_entry_spec.rb
View file
Open in desktop
Expand Up @@ -23,4 +23,22 @@ class { 'postgresql::server': }
expect(r.stderr).to eq('')
end
end

it 'should correctly set a quotes-required string' do
pp = <<-EOS.unindent
class { 'postgresql::server': }

postgresql::server::config_entry { 'listen_addresses':
value => '0.0.0.0',
}
EOS

apply_manifest(pp, :catch_failures => true)

psql('--command="show all" postgres') do |r|
r.stdout.should =~ /listen_adresses.+0\.0\.0\.0/
r.stderr.should be_empty
r.exit_code.should == 0
end
end
end