From 560bbc622fd4253ec1a74e159ab6b656cc2e4dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20R=C5=AF=C5=BEi=C4=8Dka?= Date: Wed, 7 May 2014 17:04:43 +0200 Subject: [PATCH 1/2] Add quote_char parameter to the ini_subsetting resource type The quote_char is used to quote the entire setting when it is modified as a result of changes to some subsettings. For an example let's assume we have a setting of this form: JAVA_ARGS=-Xmx256m and we want to add the '-Xms' parameter to the setting, for that purpose we define a resource like this: ini_subsetting { '-Xms': ensure => present, path => '/some/config/file', section => '', setting => 'JAVA_ARGS', subsetting => '-Xms' value => '256m', } which results into the following setting: JAVA_ARGS=-Xmx256m -Xms256m But this is not what we intended - if this setting is read by the bash shell the '-Xms256m' parameter is interpreted as a command to be executed rather than a value to be assigned to the JAVA_ARGS variable. To fix this problem the quote_char parameter was added to the ini_subsetting resource type, and we'll take advantage of it to fix the problem in the above example like so: ini_subsetting { '-Xms': ensure => present, path => '/some/config/file', section => '', setting => 'JAVA_ARGS', quote_char => '"', subsetting => '-Xms' value => '256m', } which will result into: JAVA_ARGS="-Xmx256m -Xms256m" which is what we intended. --- lib/puppet/provider/ini_subsetting/ruby.rb | 7 +++++-- lib/puppet/type/ini_subsetting.rb | 12 ++++++++++++ lib/puppet/util/setting_value.rb | 10 ++++++---- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/lib/puppet/provider/ini_subsetting/ruby.rb b/lib/puppet/provider/ini_subsetting/ruby.rb index 49c0e49..764124d 100644 --- a/lib/puppet/provider/ini_subsetting/ruby.rb +++ b/lib/puppet/provider/ini_subsetting/ruby.rb @@ -57,14 +57,17 @@ Puppet::Type.type(:ini_subsetting).provide(:ruby) do resource[:key_val_separator] || '=' end + def quote_char + resource[:quote_char] + end + private def ini_file @ini_file ||= Puppet::Util::IniFile.new(file_path, separator) end - private def setting_value - @setting_value ||= Puppet::Util::SettingValue.new(ini_file.get_value(section, setting), subsetting_separator) + @setting_value ||= Puppet::Util::SettingValue.new(ini_file.get_value(section, setting), subsetting_separator, quote_char) end end diff --git a/lib/puppet/type/ini_subsetting.rb b/lib/puppet/type/ini_subsetting.rb index dd146c2..ff7232b 100644 --- a/lib/puppet/type/ini_subsetting.rb +++ b/lib/puppet/type/ini_subsetting.rb @@ -48,6 +48,18 @@ Puppet::Type.newtype(:ini_subsetting) do end end + newparam(:quote_char) do + desc 'The character used to quote the entire value of the setting. ' + + %q{Vaild vaules are '', '"' and "'". Defaults to ''.} + defaultto('') + + validate do |value| + unless value =~ /^["']?$/ + raise Puppet::Error, %q{:quote_char valid values are '', '"' and "'"} + end + end + end + newproperty(:value) do desc 'The value of the subsetting to be defined.' end diff --git a/lib/puppet/util/setting_value.rb b/lib/puppet/util/setting_value.rb index 42cd28e..d44330a 100644 --- a/lib/puppet/util/setting_value.rb +++ b/lib/puppet/util/setting_value.rb @@ -2,19 +2,21 @@ module Puppet module Util class SettingValue - - def initialize(setting_value, subsetting_separator = ' ') + + def initialize(setting_value, subsetting_separator = ' ', default_quote_char = nil) @setting_value = setting_value @subsetting_separator = subsetting_separator - @quote_char = "" + default_quote_char ||= '' if @setting_value unquoted, @quote_char = unquote_setting_value(setting_value) @subsetting_items = unquoted.scan(Regexp.new("(?:(?:[^\\#{@subsetting_separator}]|\\.)+)")) # an item can contain escaped separator @subsetting_items.map! { |item| item.strip } + @quote_char = default_quote_char if @quote_char.empty? else - @subsetting_items = [] + @subsetting_items = [] + @quote_char = default_quote_char end end From 77854d55f8694eefb21c9b6c03e1dbdfba194c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20R=C5=AF=C5=BEi=C4=8Dka?= Date: Wed, 7 May 2014 18:30:38 +0200 Subject: [PATCH 2/2] Tests for the quote_char parameter. --- spec/acceptance/ini_subsetting_spec.rb | 43 +++++++++++++++++++++ spec/unit/puppet/util/setting_value_spec.rb | 34 ++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/spec/acceptance/ini_subsetting_spec.rb b/spec/acceptance/ini_subsetting_spec.rb index 30517c3..e616853 100644 --- a/spec/acceptance/ini_subsetting_spec.rb +++ b/spec/acceptance/ini_subsetting_spec.rb @@ -145,4 +145,47 @@ describe 'ini_subsetting resource' do end end end + + describe 'quote_char' do + { + ['-Xmx'] => 'args=""', + ['-Xmx', '256m'] => 'args=-Xmx256m', + ['-Xmx', '512m'] => 'args="-Xmx512m"', + ['-Xms', '256m'] => 'args="-Xmx256m -Xms256m"', + }.each do |parameter, content| + context %Q{with '#{parameter.first}' #{parameter.length > 1 ? '=> \'' << parameter[1] << '\'' : 'absent'} makes '#{content}'} do + path = File.join(tmpdir, 'ini_subsetting.ini') + + before :all do + shell(%Q{echo '[java]\nargs=-Xmx256m' > #{path}}) + end + after :all do + shell("cat #{path}", :acceptable_exit_codes => [0,1,2]) + shell("rm #{path}", :acceptable_exit_codes => [0,1,2]) + end + + pp = <<-EOS + ini_subsetting { '#{parameter.first}': + ensure => #{parameter.length > 1 ? 'present' : 'absent'}, + path => '#{path}', + section => 'java', + setting => 'args', + quote_char => '"', + subsetting => '#{parameter.first}', + value => '#{parameter.length > 1 ? parameter[1] : ''}' + } + EOS + + it 'applies the manifest twice with no stderr' do + expect(apply_manifest(pp, :catch_failures => true).stderr).to eq("") + expect(apply_manifest(pp, :catch_changes => true).stderr).to eq("") + end + + describe file("#{tmpdir}/ini_subsetting.ini") do + it { should be_file } + it { should contain(content) } + end + end + end + end end diff --git a/spec/unit/puppet/util/setting_value_spec.rb b/spec/unit/puppet/util/setting_value_spec.rb index 6148396..8514724 100644 --- a/spec/unit/puppet/util/setting_value_spec.rb +++ b/spec/unit/puppet/util/setting_value_spec.rb @@ -66,4 +66,38 @@ describe Puppet::Util::SettingValue do @setting_value.get_subsetting_value("-Xmx").should == nil end end + + describe "quote_char parameter" do + QUOTE_CHAR = '"' + INIT_VALUE_UNQUOTED = '-Xmx192m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/var/log/pe-puppetdb/puppetdb-oom.hprof' + + it "should get quoted empty string if original value was empty" do + setting_value = Puppet::Util::SettingValue.new(nil, ' ', QUOTE_CHAR) + setting_value.get_value.should == QUOTE_CHAR * 2 + end + + it "should quote the setting when adding a value" do + setting_value = Puppet::Util::SettingValue.new(INIT_VALUE_UNQUOTED, ' ', QUOTE_CHAR) + setting_value.add_subsetting("-Xms", "256m") + + setting_value.get_subsetting_value("-Xms").should == "256m" + setting_value.get_value.should == QUOTE_CHAR + INIT_VALUE_UNQUOTED + ' -Xms256m' + QUOTE_CHAR + end + + it "should quote the setting when changing an existing value" do + setting_value = Puppet::Util::SettingValue.new(INIT_VALUE_UNQUOTED, ' ', QUOTE_CHAR) + setting_value.add_subsetting("-Xmx", "512m") + + setting_value.get_subsetting_value("-Xmx").should == "512m" + setting_value.get_value.should =~ /^#{Regexp.quote(QUOTE_CHAR)}.*#{Regexp.quote(QUOTE_CHAR)}$/ + end + + it "should quote the setting when removing an existing value" do + setting_value = Puppet::Util::SettingValue.new(INIT_VALUE_UNQUOTED, ' ', QUOTE_CHAR) + setting_value.remove_subsetting("-Xmx") + + setting_value.get_subsetting_value("-Xmx").should == nil + setting_value.get_value.should =~ /^#{Regexp.quote(QUOTE_CHAR)}.*#{Regexp.quote(QUOTE_CHAR)}$/ + end + end end