From d2c1e07e8088fb2372d258cfdd27ee4aab10cef1 Mon Sep 17 00:00:00 2001 From: Stephen Date: Tue, 14 Aug 2012 13:38:08 +0100 Subject: [PATCH 1/2] Fixed regex to match sections and settings with non alphanumeric characters. Fixed writing to file without any sections at all. Fixed exists checking for variable type by always casting to string and added all the tests for the above items. --- lib/puppet/provider/ini_setting/ruby.rb | 2 +- lib/puppet/util/ini_file.rb | 11 ++-- .../puppet/provider/ini_setting/ruby_spec.rb | 62 ++++++++++++++++++- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/lib/puppet/provider/ini_setting/ruby.rb b/lib/puppet/provider/ini_setting/ruby.rb index f04af06..9f13dff 100644 --- a/lib/puppet/provider/ini_setting/ruby.rb +++ b/lib/puppet/provider/ini_setting/ruby.rb @@ -2,7 +2,7 @@ require File.expand_path('../../../util/ini_file', __FILE__) Puppet::Type.type(:ini_setting).provide(:ruby) do def exists? - ini_file.get_value(resource[:section], resource[:setting]) == resource[:value] + ini_file.get_value(resource[:section], resource[:setting]) == resource[:value].to_s end def create diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index 51dfda7..0505462 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -5,14 +5,13 @@ module Puppet module Util class IniFile - SECTION_REGEX = /^\s*\[([\w\d\.]+)\]\s*$/ - SETTING_REGEX = /^\s*([\w\d\.]+)\s*=\s*([\w\d\.]+)\s*$/ + SECTION_REGEX = /^\s*\[([\S.]+)\]\s*$/ + SETTING_REGEX = /^\s*([\S]+)\s*=\s*([\S]+)\s*$/ def initialize(path) @path = path @section_names = [] @sections_hash = {} - parse_file end @@ -43,7 +42,8 @@ module Util def save File.open(@path, 'w') do |fh| first_section = @sections_hash[@section_names[0]] - (0..first_section.start_line - 1).each do |line_num| + first_section.start_line == nil ? start_line = 0 : start_line = first_section.start_line + (0..start_line - 1).each do |line_num| fh.puts(lines[line_num]) end @@ -93,7 +93,6 @@ module Util elsif (match = SETTING_REGEX.match(line)) settings[match[1]] = match[2] end - line_iter.next end end @@ -129,4 +128,4 @@ module Util end end -end +end \ No newline at end of file diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb index 91d3050..72a5da0 100644 --- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb @@ -6,6 +6,7 @@ describe provider_class do include PuppetlabsSpec::Files let(:tmpfile) { tmpfilename("ini_setting_test") } + let(:emptyfile) { tmpfilename("ini_setting_test_empty") } let(:orig_content) { <<-EOS # This is a comment @@ -14,16 +15,18 @@ describe provider_class do foo=foovalue bar = barvalue +master = true [section2] foo= foovalue2 baz=bazvalue +url = http://192.168.1.1:8080 #another comment ; yet another comment EOS } - def validate_file(expected_content) + def validate_file(expected_content,tmpfile = tmpfile) File.read(tmpfile).should == expected_content end @@ -32,6 +35,9 @@ baz=bazvalue File.open(tmpfile, 'w') do |fh| fh.write(orig_content) end + File.open(emptyfile, 'w') do |fh| + fh.write("") + end end context "when ensuring that a setting is present" do @@ -54,10 +60,12 @@ baz=bazvalue foo=foovalue bar = barvalue +master = true [section2] foo= foovalue2 baz=bazvalue +url = http://192.168.1.1:8080 #another comment ; yet another comment yahoo = yippee @@ -78,16 +86,44 @@ yahoo = yippee foo=foovalue bar = barvalue +master = true [section2] foo= foovalue2 baz = bazvalue2 +url = http://192.168.1.1:8080 #another comment ; yet another comment EOS ) end + it "should be able to handle settings with non alphanumbering settings " do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :setting => 'url', :value => 'http://192.168.0.1:8080')) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + + validate_file( <<-EOS +# This is a comment +[section1] +; This is also a comment +foo=foovalue + +bar = barvalue +master = true +[section2] + +foo= foovalue2 +baz=bazvalue +url = http://192.168.0.1:8080 + #another comment + ; yet another comment + EOS + ) + end + it "should recognize an existing setting with the specified value" do resource = Puppet::Type::Ini_setting.new(common_params.merge( :setting => 'baz', :value => 'bazvalue')) @@ -108,10 +144,12 @@ baz = bazvalue2 foo=foovalue bar = barvalue +master = true [section2] foo= foovalue2 baz=bazvalue +url = http://192.168.1.1:8080 #another comment ; yet another comment @@ -120,5 +158,27 @@ huzzah = shazaam EOS ) end + + it "should add a new section if no sections exists" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => "section1", :setting => 'setting1', :value => 'hellowworld', :path => emptyfile)) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + validate_file(" +[section1] +setting1 = hellowworld +", emptyfile) + end + + it "should be able to handle variables of any type" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => "section1", :setting => 'master', :value => true)) + provider = described_class.new(resource) + provider.exists?.should == true + provider.create + end + + end end From 8d2a8c859d928700350d48965dcf01d279289393 Mon Sep 17 00:00:00 2001 From: Stephen Date: Tue, 14 Aug 2012 19:30:14 +0100 Subject: [PATCH 2/2] make the regex less matchy --- lib/puppet/util/ini_file.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index 0505462..b951b3f 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -5,8 +5,8 @@ module Puppet module Util class IniFile - SECTION_REGEX = /^\s*\[([\S.]+)\]\s*$/ - SETTING_REGEX = /^\s*([\S]+)\s*=\s*([\S]+)\s*$/ + SECTION_REGEX = /^\s*\[([\w\d\.\\\/\-]+)\]\s*$/ + SETTING_REGEX = /^\s*([\w\d\.\\\/\-]+)\s*=\s*([\S]+)\s*$/ def initialize(path) @path = path