Merge pull request #187 from tphoney/release_prep_1_4_2

Release prep 1 4 2

CHANGELOG.md

@@ -1,3 +1,12 @@
+## 2015-09-01 - Supported Release 1.4.2
+### Summary
+This release adds some bugfixes.
+
+####Bugfixes
+- MODULES-2212 Add use_exact_match parameter for subsettings
+- MODULES-1908 Munge the setting to ensure we always strip the whitespace
+- MODULES-2369 Support a space as a key_val_separator
+
 ## 2015-07-15 - Supported Release 1.4.1
 ### Summary
 This release bumps the metadata for PE up.

README.markdown

@@ -334,7 +334,7 @@ Specifies whether the subsetting should be present. Valid options: 'present' and
 
 ##### `key_val_separator`
 
-*Optional.* Specifies a string to use between subsetting name and value (e.g., to determine whether the separator includes whitespace). Valid options: a string. Default value: ' = '.
+*Optional.* Specifies a string to use between setting name and value (e.g., to determine whether the separator includes whitespace). Valid options: a string. Default value: ' = '.
 
 ##### `path`
 
@@ -360,6 +360,10 @@ Specifies whether the subsetting should be present. Valid options: 'present' and
 
 *Optional.* Specifies a string to use between subsettings. Valid options: a string. Default value: " ".
 
+##### `use_exact_match`
+
+*Optional.* Whether to use partial or exact matching for subsetting. Should be set to true if the subsettings do not have values. Valid options: true, false. Default value: false.
+
 ##### `value`
 
 *Optional.* Supplies a value for the specified subsetting. Valid options: a string. Default value: undefined.

lib/puppet/provider/ini_subsetting/ruby.rb

@@ -4,11 +4,11 @@ require File.expand_path('../../../util/setting_value', __FILE__)
 Puppet::Type.type(:ini_subsetting).provide(:ruby) do
 
   def exists?
-    setting_value.get_subsetting_value(subsetting)
+    setting_value.get_subsetting_value(subsetting, resource[:use_exact_match])
   end
 
   def create
-    setting_value.add_subsetting(subsetting, resource[:value])
+    setting_value.add_subsetting(subsetting, resource[:value], resource[:use_exact_match])
     ini_file.set_value(section, setting, setting_value.get_value)
     ini_file.save
     @ini_file = nil
@@ -16,7 +16,7 @@ Puppet::Type.type(:ini_subsetting).provide(:ruby) do
   end
 
   def destroy
-    setting_value.remove_subsetting(subsetting)
+    setting_value.remove_subsetting(subsetting, resource[:use_exact_match])
     ini_file.set_value(section, setting, setting_value.get_value)
     ini_file.save
     @ini_file = nil
@@ -28,7 +28,7 @@ Puppet::Type.type(:ini_subsetting).provide(:ruby) do
   end
 
   def value=(value)
-    setting_value.add_subsetting(subsetting, resource[:value])
+    setting_value.add_subsetting(subsetting, resource[:value], resource[:use_exact_match])
     ini_file.set_value(section, setting, setting_value.get_value)
     ini_file.save
   end

lib/puppet/type/ini_setting.rb

@@ -17,6 +17,12 @@ Puppet::Type.newtype(:ini_setting) do
 
   newparam(:setting) do
     desc 'The name of the setting to be defined.'
+    munge do |value|
+      if value =~ /(^\s|\s$)/
+        Puppet.warn("Settings should not have spaces in the value, we are going to strip the whitespace")
+      end
+      value.lstrip.rstrip
+    end
   end
 
   newparam(:path) do

lib/puppet/type/ini_subsetting.rb

@@ -56,6 +56,12 @@ Puppet::Type.newtype(:ini_subsetting) do
     end
   end
 
+  newparam(:use_exact_match) do
+    desc 'Set to true if your subsettings don\'t have values and you want to use exact matches to determine if the subsetting exists. See MODULES-2212'
+    newvalues(:true, :false)
+    defaultto(:false)
+  end
+
   newproperty(:value) do
     desc 'The value of the subsetting to be defined.'
   end

lib/puppet/util/ini_file.rb

@@ -7,7 +7,7 @@ module Util
 
     def initialize(path, key_val_separator = ' = ', section_prefix = '[', section_suffix = ']')
 
-      k_v_s = key_val_separator.strip
+      k_v_s = key_val_separator =~ /^\s+$/ ? ' ' : key_val_separator.strip
 
       @section_prefix = section_prefix
       @section_suffix = section_suffix

lib/puppet/util/setting_value.rb

@@ -1,95 +1,103 @@
 module Puppet
-module Util
+  module Util
 
-  class SettingValue
+    class SettingValue
 
-    def initialize(setting_value, subsetting_separator = ' ', default_quote_char = nil)
-      @setting_value = setting_value
-      @subsetting_separator = subsetting_separator
+      def initialize(setting_value, subsetting_separator = ' ', default_quote_char = nil)
+        @setting_value = setting_value
+        @subsetting_separator = subsetting_separator
 
-      default_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 = []
-        @quote_char = default_quote_char
-      end     
-    end
+        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 = []
+          @quote_char = default_quote_char
+        end
+      end
+
+      def unquote_setting_value(setting_value)
+        quote_char = ""
+        if (setting_value.start_with?('"') and setting_value.end_with?('"'))
+          quote_char = '"'
+        elsif (setting_value.start_with?("'") and setting_value.end_with?("'"))
+          quote_char = "'"
+        end
+
+        unquoted = setting_value
 
-    def unquote_setting_value(setting_value)
-      quote_char = ""
-      if (setting_value.start_with?('"') and setting_value.end_with?('"'))
-        quote_char = '"'
-      elsif (setting_value.start_with?("'") and setting_value.end_with?("'"))
-        quote_char = "'"
+        if (quote_char != "")
+          unquoted = setting_value[1, setting_value.length - 2]
+        end
+
+        [unquoted, quote_char]
       end
 
-      unquoted = setting_value
+      def get_value
+
+        result = ""
+        first = true
+
+        @subsetting_items.each { |item|
+          result << @subsetting_separator unless first
+          result << item
+          first = false
+        }
 
-      if (quote_char != "")
-        unquoted = setting_value[1, setting_value.length - 2]
+        @quote_char + result + @quote_char
       end
 
-      [unquoted, quote_char]
-    end
+      def get_subsetting_value(subsetting, use_exact_match=:false)
 
-    def get_value
-    
-      result = ""
-      first = true
-      
-      @subsetting_items.each { |item|
-        result << @subsetting_separator unless first
-        result << item        
-        first = false
-      }
-      
-      @quote_char + result + @quote_char
-    end
+        value = nil
+
+        @subsetting_items.each { |item|
+          if(use_exact_match == :false and item.start_with?(subsetting))
+            value = item[subsetting.length, item.length - subsetting.length]
+            break
+          elsif(use_exact_match == :true and item.eql?(subsetting))
+            return true
+          end
+        }
 
-    def get_subsetting_value(subsetting)
-    
-      value = nil
-      
-      @subsetting_items.each { |item|
-        if(item.start_with?(subsetting))
-          value = item[subsetting.length, item.length - subsetting.length]
-          break
+        value
+      end
+
+      def add_subsetting(subsetting, subsetting_value, use_exact_match=:false)
+
+        new_item = subsetting + (subsetting_value || '')
+        found = false
+
+        @subsetting_items.map! { |item|
+          if use_exact_match == :false and item.start_with?(subsetting)
+            value = new_item
+            found = true
+          elsif use_exact_match == :true and item.eql?(subsetting)
+            value = new_item
+            found = true
+          else
+            value = item
+          end
+
+          value
+        }
+
+        unless found
+          @subsetting_items.push(new_item)
         end
-      }
-      
-      value
-    end
-    
-    def add_subsetting(subsetting, subsetting_value)
-    
-      new_item = subsetting + (subsetting_value || '')
-      found = false
-
-      @subsetting_items.map! { |item|
-        if item.start_with?(subsetting)
-          value = new_item
-          found = true
+      end
+
+      def remove_subsetting(subsetting, use_exact_match=:false)
+        if use_exact_match == :false
+          @subsetting_items = @subsetting_items.map { |item| item.start_with?(subsetting) ? nil : item }.compact
         else
-          value = item
+          @subsetting_items = @subsetting_items.map { |item| item.eql?(subsetting) ? nil : item }.compact
         end
-        
-        value
-      }
-      
-      unless found
-        @subsetting_items.push(new_item)
       end
     end
-
-    def remove_subsetting(subsetting)   
-      @subsetting_items = @subsetting_items.map { |item| item.start_with?(subsetting) ? nil : item }.compact
-    end
-    
   end
 end
-end

metadata.json

@@ -1,15 +1,12 @@
 {
   "name": "puppetlabs-inifile",
-  "version": "1.4.1",
+  "version": "1.4.2",
   "author": "Puppet Labs",
   "summary": "Resource types for managing settings in INI files",
   "license": "Apache-2.0",
   "source": "https://github.com/puppetlabs/puppetlabs-inifile",
   "project_page": "https://github.com/puppetlabs/puppetlabs-inifile",
   "issues_url": "https://tickets.puppetlabs.com/browse/MODULES",
-  "dependencies": [
-  
-  ],
   "operatingsystem_support": [
     {
       "operatingsystem": "RedHat",
@@ -101,5 +98,8 @@
       "name": "puppet",
       "version_requirement": ">= 3.0.0 < 5.0.0"
     }
+  ],
+  "dependencies": [
+  
   ]
 }

spec/acceptance/ini_setting_spec.rb

@@ -262,6 +262,8 @@ describe 'ini_setting resource' do
         ""                             => /two = three/,
         "key_val_separator => '=',"    => /two=three/,
         "key_val_separator => ' =  '," => /two =  three/,
+        "key_val_separator => ' '," => /two three/,
+        "key_val_separator => '   '," => /two   three/,
     }.each do |parameter, content|
       context "with \"#{parameter}\" makes \"#{content}\"" do
         pp = <<-EOS

spec/unit/puppet/provider/ini_setting/ruby_spec.rb

@@ -783,6 +783,49 @@ bar: baz
 
   end
 
+  context "when overriding the separator to a space" do
+    let(:orig_content) {
+      <<-EOS
+[section2]
+foo bar
+      EOS
+    }
+
+    it "should modify an existing setting" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+                                                   :section           => 'section2',
+                                                   :setting           => 'foo',
+                                                   :value             => 'yippee',
+                                                   :key_val_separator => ' '))
+      provider = described_class.new(resource)
+      provider.exists?.should be true
+      provider.value.should == 'bar'
+      provider.value=('yippee')
+      validate_file(<<-EOS
+[section2]
+foo yippee
+      EOS
+      )
+    end
+
+    it "should add a new setting" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+                                                   :section           => 'section2',
+                                                   :setting           => 'bar',
+                                                   :value             => 'baz',
+                                                   :key_val_separator => ' '))
+      provider = described_class.new(resource)
+      provider.exists?.should be false
+      provider.create
+      validate_file(<<-EOS
+[section2]
+foo bar
+bar baz
+      EOS
+      )
+    end
+  end
+
   context "when ensuring that a setting is absent" do
     let(:orig_content) {
       <<-EOS

spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb

@@ -132,4 +132,50 @@ somenewsetting = puppetdb
     end
 
   end
+
+  context "when working with subsettings in files with use_exact_match" do
+    let(:common_params) { {
+        :title           => 'ini_setting_ensure_present_test',
+        :path            => tmpfile,
+        :section         => 'master',
+        :setting         => 'reports',
+        :use_exact_match => true,
+    } }
+
+    let(:orig_content) {
+      <<-EOS
+[master]
+
+reports = http,foo
+      EOS
+    }
+
+    it "should add a new subsetting when the 'parent' setting already exists" do
+      resource = Puppet::Type::Ini_subsetting.new(common_params.merge(
+          :subsetting => 'fo', :subsetting_separator => ','))
+      provider = described_class.new(resource)
+      provider.value=('')
+      validate_file(<<-eos
+[master]
+
+reports = http,foo,fo
+      eos
+      )
+    end
+
+    it "should not remove substring subsettings" do
+      resource = Puppet::Type::Ini_subsetting.new(common_params.merge(
+          :subsetting => 'fo', :subsetting_separator => ','))
+      provider = described_class.new(resource)
+      provider.value=('')
+      provider.destroy
+      validate_file(<<-EOS
+[master]
+
+reports = http,foo
+      EOS
+      )
+    end
+  end
+
 end