From f2cfa270c920377466143ac335c8e3631ab070ce Mon Sep 17 00:00:00 2001
From: Onno Molenkamp <onno@flox.org>
Date: Mon, 13 Jun 2022 11:47:54 +0200
Subject: [PATCH 1/2] Add support for multiple values per key

---
 lib/puppet/provider/ini_subsetting/ruby.rb    |   2 +-
 lib/puppet/type/ini_setting.rb                |   2 +-
 lib/puppet/util/ini_file.rb                   |  49 ++++--
 lib/puppet/util/ini_file/section.rb           |  22 ++-
 .../puppet/provider/ini_setting/ruby_spec.rb  | 158 ++++++++++++++++--
 spec/unit/puppet/util/ini_file_spec.rb        |  88 ++++++----
 6 files changed, 255 insertions(+), 66 deletions(-)

diff --git a/lib/puppet/provider/ini_subsetting/ruby.rb b/lib/puppet/provider/ini_subsetting/ruby.rb
index 0f6b7721..f20bc6cd 100644
--- a/lib/puppet/provider/ini_subsetting/ruby.rb
+++ b/lib/puppet/provider/ini_subsetting/ruby.rb
@@ -89,7 +89,7 @@ def ini_file
 
   def setting_value
     @setting_value ||= Puppet::Util::SettingValue.new(
-      ini_file.get_value(section, setting),
+      Array(ini_file.get_value(section, setting))[0],
       subsetting_separator, quote_char, subsetting_key_val_separator
     )
   end
diff --git a/lib/puppet/type/ini_setting.rb b/lib/puppet/type/ini_setting.rb
index 474c4f4c..094c0139 100644
--- a/lib/puppet/type/ini_setting.rb
+++ b/lib/puppet/type/ini_setting.rb
@@ -85,7 +85,7 @@ def munge_boolean_md5(value)
     defaultto(' = ')
   end
 
-  newproperty(:value) do
+  newproperty(:value, :array_matching => :all) do
     desc 'The value of the setting to be defined.'
 
     munge do |value|
diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb
index 2eaf401a..459e4b90 100644
--- a/lib/puppet/util/ini_file.rb
+++ b/lib/puppet/util/ini_file.rb
@@ -67,6 +67,8 @@ def set_value(*args)
         (section_name, setting, separator, value) = args
       end
 
+      value = [*value]
+
       complete_setting = {
         setting: setting,
         separator: separator,
@@ -79,8 +81,17 @@ def set_value(*args)
       section = @sections_hash[section_name]
 
       if section.existing_setting?(setting)
+        existing_value = section.get_value(setting)
+        section_index = @section_names.index(section_name)
+
         update_line(section, setting, value)
         section.update_existing_setting(setting, value)
+
+        if value.length > existing_value.length
+          increment_section_line_numbers(section_index + 1, value.length - existing_value.length)
+        elsif value.length < existing_value.length
+          decrement_section_line_numbers(section_index + 1, existing_value.length - value.length)
+        end
       elsif find_commented_setting(section, setting)
         # So, this stanza is a bit of a hack.  What we're trying
         # to do here is this: for settings that don't already
@@ -101,7 +112,7 @@ def set_value(*args)
         # numbers for all of the sections *after* the one that
         # was modified.
         section_index = @section_names.index(section_name)
-        increment_section_line_numbers(section_index + 1)
+        increment_section_line_numbers(section_index + 1, value.length)
       elsif !setting.nil? || !value.nil?
         section.set_additional_setting(setting, value)
       end
@@ -110,6 +121,8 @@ def set_value(*args)
     def remove_setting(section_name, setting)
       section = @sections_hash[section_name]
       return unless section.existing_setting?(setting)
+      existing_value = section.get_value(setting)
+
       # If the setting is found, we have some work to do.
       # First, we remove the line from our array of lines:
       remove_line(section, setting)
@@ -122,7 +135,7 @@ def remove_setting(section_name, setting)
       # numbers for all of the sections *after* the one that
       # was modified.
       section_index = @section_names.index(section_name)
-      decrement_section_line_numbers(section_index + 1)
+      decrement_section_line_numbers(section_index + 1, existing_value.length)
 
       return unless section.empty?
       # By convention, it's time to remove this newly emptied out section
@@ -174,7 +187,9 @@ def save
 
           # write new settings, if there are any
           section.additional_settings.each_pair do |key, value|
-            fh.puts("#{@indent_char * (@indent_width || section.indentation || 0)}#{key}#{@key_val_separator}#{value}")
+            value.each do |v|
+              fh.puts("#{@indent_char * (@indent_width || section.indentation || 0)}#{key}#{@key_val_separator}#{v}")
+            end
           end
 
           if !whitespace_buffer.empty?
@@ -234,7 +249,7 @@ def read_section(name, start_line, line_iter)
           return Section.new(name, start_line, end_line_num, settings, min_indentation)
         end
         if (match = @setting_regex.match(line))
-          settings[match[2]] = match[4]
+          (settings[match[2]] ||= []).push(match[4])
           indentation = match[1].length
           min_indentation = [indentation, min_indentation || indentation].min
         end
@@ -245,16 +260,24 @@ def read_section(name, start_line, line_iter)
     end
 
     def update_line(section, setting, value)
-      (section.start_line..section.end_line).each do |line_num|
+      updated = false
+      section.end_line.downto section.start_line do |line_num|
         next unless (match = @setting_regex.match(lines[line_num]))
         if match[2] == setting
-          lines[line_num] = "#{match[1]}#{match[2]}#{match[3]}#{value}"
+          lines.delete_at(line_num)
+
+          if !updated
+            value.each_with_index do |v, i|
+              lines.insert(line_num + i, "#{match[1]}#{match[2]}#{match[3]}#{v}")
+            end
+            updated = true
+          end
         end
       end
     end
 
     def remove_line(section, setting)
-      (section.start_line..section.end_line).each do |line_num|
+      section.end_line.downto section.start_line do |line_num|
         next unless (match = @setting_regex.match(lines[line_num]))
         if match[2] == setting
           lines.delete_at(line_num)
@@ -307,26 +330,28 @@ def find_commented_setting(section, setting)
     def insert_inline_setting_line(result, section, complete_setting)
       line_num = result[:line_num]
       s = complete_setting
-      lines.insert(line_num + 1, "#{@indent_char * (@indent_width || section.indentation || 0)}#{s[:setting]}#{s[:separator]}#{s[:value]}")
+      s[:value].each_with_index do |v, i|
+        lines.insert(line_num + 1 + i, "#{@indent_char * (@indent_width || section.indentation || 0)}#{s[:setting]}#{s[:separator]}#{v}")
+      end
     end
 
     # Utility method; given a section index (index into the @section_names
     # array), decrement the start/end line numbers for that section and all
     # all of the other sections that appear *after* the specified section.
-    def decrement_section_line_numbers(section_index)
+    def decrement_section_line_numbers(section_index, n = 1)
       @section_names[section_index..(@section_names.length - 1)].each do |name|
         section = @sections_hash[name]
-        section.decrement_line_nums
+        section.decrement_line_nums(n)
       end
     end
 
     # Utility method; given a section index (index into the @section_names
     # array), increment the start/end line numbers for that section and all
     # all of the other sections that appear *after* the specified section.
-    def increment_section_line_numbers(section_index)
+    def increment_section_line_numbers(section_index, n = 1)
       @section_names[section_index..(@section_names.length - 1)].each do |name|
         section = @sections_hash[name]
-        section.increment_line_nums
+        section.increment_line_nums(n)
       end
     end
 
diff --git a/lib/puppet/util/ini_file/section.rb b/lib/puppet/util/ini_file/section.rb
index 75ea5448..7d2832b3 100644
--- a/lib/puppet/util/ini_file/section.rb
+++ b/lib/puppet/util/ini_file/section.rb
@@ -54,11 +54,17 @@ def empty?
     end
 
     def update_existing_setting(setting_name, value)
+      existing_value = @existing_settings[setting_name]
+
       @existing_settings[setting_name] = value
+
+      @end_line += value.length - existing_value.length
     end
 
     def remove_existing_setting(setting_name)
-      @end_line -= 1 if @existing_settings.delete(setting_name) && @end_line
+      existing_value = @existing_settings.delete(setting_name)
+
+      @end_line -= existing_value.length if existing_value && @end_line
     end
 
     # This is a hacky method; it's basically called when we need to insert
@@ -69,7 +75,7 @@ def remove_existing_setting(setting_name)
     # of the lines.
     def insert_inline_setting(setting_name, value)
       @existing_settings[setting_name] = value
-      @end_line += 1 if @end_line
+      @end_line += value.length if @end_line
     end
 
     def set_additional_setting(setting_name, value)
@@ -79,17 +85,17 @@ def set_additional_setting(setting_name, value)
     # Decrement the start and end line numbers for the section (if they are
     # defined); this is intended to be called when a setting is removed
     # from a section that comes before this section in the ini file.
-    def decrement_line_nums
-      @start_line -= 1 if @start_line
-      @end_line -= 1 if @end_line
+    def decrement_line_nums(n = 1)
+      @start_line -= n if @start_line
+      @end_line -= n if @end_line
     end
 
     # Increment the start and end line numbers for the section (if they are
     # defined); this is intended to be called when an inline setting is added
     # to a section that comes before this section in the ini file.
-    def increment_line_nums
-      @start_line += 1 if @start_line
-      @end_line += 1 if @end_line
+    def increment_line_nums(n = 1)
+      @start_line += n if @start_line
+      @end_line += n if @end_line
     end
   end
 end
diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
index d1f448eb..8a0933ff 100644
--- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
+++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
@@ -95,13 +95,13 @@ def self.file_path
         expect(child_three).to receive(:file_path).exactly(2).times.and_return(tmpfile)
         expect(child_three.instances.size).to eq(7)
         expected_array = [
-          { name: 'section1/foo', value: 'foovalue' },
-          { name: 'section1/bar', value: 'barvalue' },
-          { name: 'section1/main', value: 'true' },
-          { name: 'section2/foo', value: 'foovalue2' },
-          { name: 'section2/baz', value: 'bazvalue' },
-          { name: 'section2/url', value: 'http://192.168.1.1:8080' },
-          { name: 'section:sub/subby', value: 'bar' },
+          { name: 'section1/foo', value: ['foovalue'] },
+          { name: 'section1/bar', value: ['barvalue'] },
+          { name: 'section1/main', value: ['true'] },
+          { name: 'section2/foo', value: ['foovalue2'] },
+          { name: 'section2/baz', value: ['bazvalue'] },
+          { name: 'section2/url', value: ['http://192.168.1.1:8080'] },
+          { name: 'section:sub/subby', value: ['bar'] },
         ]
         real_array = []
         ensure_array = []
@@ -439,7 +439,7 @@ def self.file_path
     it 'modifies an existing setting with a different value - with colon in section' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section:sub', setting: 'subby', value: 'foo'))
       provider = described_class.new(resource)
-      expect(provider.value).to eq('bar')
+      expect(provider.value).to eq(['bar'])
       provider.value = 'foo'
       validate_file(expected_content_ten, tmpfile)
     end
@@ -468,7 +468,7 @@ def self.file_path
     it 'is 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)
-      expect(provider.value).to eq('http://192.168.1.1:8080')
+      expect(provider.value).to eq(['http://192.168.1.1:8080'])
       provider.value = 'http://192.168.0.1:8080'
       validate_file(expected_content_eleven, tmpfile)
     end
@@ -497,7 +497,7 @@ def self.file_path
     it 'is able to handle settings with pre/suffix with non alphanumbering settings' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'nonstandard', setting: 'shoes', value: 'http://192.168.0.1:8080', section_prefix: '-', section_suffix: '-'))
       provider = described_class.new(resource)
-      expect(provider.value).to eq('purple')
+      expect(provider.value).to eq(['purple'])
       provider.value = 'http://192.168.0.1:8080'
       validate_file(expected_content_twelve, tmpfile)
     end
@@ -654,7 +654,7 @@ def self.file_path
     it 'is able to handle variables of any type' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'main', value: true))
       provider = described_class.new(resource)
-      expect(provider.value).to eq('true')
+      expect(provider.value).to eq(['true'])
     end
   end
 
@@ -776,7 +776,7 @@ def self.file_path
     it 'modifies an existing setting with a different value' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: '', setting: 'foo', value: 'yippee'))
       provider = described_class.new(resource)
-      expect(provider.value).to eq('blah')
+      expect(provider.value).to eq(['blah'])
       provider.value = 'yippee'
       validate_file(expected_content_two, tmpfile)
     end
@@ -817,7 +817,7 @@ def self.file_path
     it 'modifies an existing setting' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section2', setting: 'foo', value: 'yippee'))
       provider = described_class.new(resource)
-      expect(provider.value).to eq('http://192.168.1.1:8080')
+      expect(provider.value).to eq(['http://192.168.1.1:8080'])
       provider.value = 'yippee'
       validate_file(expected_content_two, tmpfile)
     end
@@ -851,7 +851,7 @@ def self.file_path
     it 'modifies 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)
-      expect(provider.value).to eq('bar')
+      expect(provider.value).to eq(['bar'])
       provider.value = 'yippee'
       validate_file(expected_content_one, tmpfile)
     end
@@ -872,7 +872,7 @@ def self.file_path
     it 'modifies 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)
-      expect(provider.value).to eq('bar')
+      expect(provider.value).to eq(['bar'])
       provider.value = 'yippee'
       validate_file(expected_content_one, tmpfile)
     end
@@ -906,7 +906,7 @@ def self.file_path
     it 'modifies 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)
-      expect(provider.value).to eq('bar')
+      expect(provider.value).to eq(['bar'])
       provider.value = 'yippee'
       validate_file(expected_content_one, tmpfile)
     end
@@ -1586,5 +1586,131 @@ def self.file_path
       validate_file(expected_content_one, tmpfile)
     end
   end
+
+  context 'when using keys with multiple values' do
+    let(:orig_content) do
+        <<-EOS
+[section]
+foo = foovalue
+foo = foovalue2
+foo = foovalue3
+foo = foovalue4
+bar = barvalue
+
+[section2]
+foo = value
+bar = barvalue
+foo = value2
+
+[section3]
+        EOS
+    end
+
+    it 'retains an existing setting with setting the original value' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section', setting: 'foo', value: ['foovalue', 'foovalue2', 'foovalue3', 'foovalue4']))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be true
+      provider.create
+      validate_file(orig_content, tmpfile)
+    end
+
+    expected_content_one = <<-EOS
+[section]
+foo = foovalue
+bar = barvalue
+
+[section2]
+foo = value
+bar = barvalue
+foo = value2
+
+[section3]
+    EOS
+
+    it 'removes additional values from an existing setting' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section', setting: 'foo', value: 'foovalue'))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be true
+      provider.create
+      validate_file(expected_content_one, tmpfile)
+    end
+
+    expected_content_two = <<-EOS
+[section]
+foo = foovalue
+foo = foovalue2
+foo = foovalue3
+foo = foovalue4
+bar = barvalue
+
+[section2]
+bar = barvalue
+foo = value
+foo = value2
+foo = value3
+
+[section3]
+    EOS
+
+    it 'adds new values to an existing setting' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section2', setting: 'foo', value: ['value', 'value2', 'value3']))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be true
+      provider.create
+      validate_file(expected_content_two, tmpfile)
+    end
+
+    expected_content_three = <<-EOS
+[section]
+foo = foovalue2
+foo = foovalue4
+foo = foovalue
+foo = foovalue3
+bar = barvalue
+
+[section2]
+foo = value
+bar = barvalue
+foo = value2
+
+[section3]
+    EOS
+
+    it 'modifies an existing setting with values ordered differently' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section', setting: 'foo', value: ['foovalue2', 'foovalue4', 'foovalue', 'foovalue3']))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be true
+      provider.create
+      validate_file(expected_content_three, tmpfile)
+    end
+
+    expected_content_four = <<-EOS
+[section]
+foo = foovalue
+foo = foovalue2
+foo = foovalue3
+foo = foovalue4
+bar = barvalue
+
+[section2]
+foo = value
+bar = barvalue
+foo = value2
+
+[section3]
+foo = a
+foo = b
+foo = c
+    EOS
+
+    it 'adds a new setting' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section3', setting: 'foo', value: ['a', 'b', 'c']))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be false
+      provider.create
+      validate_file(expected_content_four, tmpfile)
+    end
+  end
+
   # rubocop:enable Layout/IndentHeredoc
 end
diff --git a/spec/unit/puppet/util/ini_file_spec.rb b/spec/unit/puppet/util/ini_file_spec.rb
index abf4a25a..1fd53c4f 100644
--- a/spec/unit/puppet/util/ini_file_spec.rb
+++ b/spec/unit/puppet/util/ini_file_spec.rb
@@ -49,16 +49,16 @@
     end
 
     it 'exposes settings for sections #section1' do
-      expect(ini_sub.get_settings('section1')).to eq('bar' => 'barvalue',
-                                                     'baz' => '',
-                                                     'foo' => 'foovalue')
+      expect(ini_sub.get_settings('section1')).to eq('bar' => ['barvalue'],
+                                                     'baz' => [''],
+                                                     'foo' => ['foovalue'])
     end
     it 'exposes settings for sections #section2' do
-      expect(ini_sub.get_settings('section2')).to eq('baz' => 'bazvalue',
-                                                     'foo' => 'foovalue2',
-                                                     'l' => 'git log',
-                                                     "xyzzy['thing1']['thing2']" => 'xyzzyvalue',
-                                                     'zot' => 'multi word value')
+      expect(ini_sub.get_settings('section2')).to eq('baz' => ['bazvalue'],
+                                                     'foo' => ['foovalue2'],
+                                                     'l' => ['git log'],
+                                                     "xyzzy['thing1']['thing2']" => ['xyzzyvalue'],
+                                                     'zot' => ['multi word value'])
     end
   end
 
@@ -83,7 +83,7 @@
     end
 
     it 'exposes settings for sections' do
-      expect(ini_sub.get_value('section1', 'foo')).to eq('foovalue')
+      expect(ini_sub.get_value('section1', 'foo')).to eq(['foovalue'])
     end
   end
 
@@ -109,10 +109,10 @@
     end
 
     it 'exposes settings for sections #bar' do
-      expect(ini_sub.get_value('', 'foo')).to eq('bar')
+      expect(ini_sub.get_value('', 'foo')).to eq(['bar'])
     end
     it 'exposes settings for sections #foovalue' do
-      expect(ini_sub.get_value('section1', 'foo')).to eq('foovalue')
+      expect(ini_sub.get_value('section1', 'foo')).to eq(['foovalue'])
     end
   end
 
@@ -136,42 +136,42 @@
 
     it 'properlies update uncommented values' do
       ini_sub.set_value('section1', 'foo', ' = ', 'foovalue')
-      expect(ini_sub.get_value('section1', 'foo')).to eq('foovalue')
+      expect(ini_sub.get_value('section1', 'foo')).to eq(['foovalue'])
     end
 
     it 'properlies update uncommented values without separator' do
       ini_sub.set_value('section1', 'foo', 'foovalue')
-      expect(ini_sub.get_value('section1', 'foo')).to eq('foovalue')
+      expect(ini_sub.get_value('section1', 'foo')).to eq(['foovalue'])
     end
 
     it 'properlies update commented value' do
       ini_sub.set_value('section1', 'bar', ' = ', 'barvalue')
-      expect(ini_sub.get_value('section1', 'bar')).to eq('barvalue')
+      expect(ini_sub.get_value('section1', 'bar')).to eq(['barvalue'])
     end
 
     it 'properlies update commented values' do
       ini_sub.set_value('section1', "xyzzy['thing1']['thing2']", ' = ', 'xyzzyvalue')
-      expect(ini_sub.get_value('section1', "xyzzy['thing1']['thing2']")).to eq('xyzzyvalue')
+      expect(ini_sub.get_value('section1', "xyzzy['thing1']['thing2']")).to eq(['xyzzyvalue'])
     end
 
     it 'properlies update commented value without separator' do
       ini_sub.set_value('section1', 'bar', 'barvalue')
-      expect(ini_sub.get_value('section1', 'bar')).to eq('barvalue')
+      expect(ini_sub.get_value('section1', 'bar')).to eq(['barvalue'])
     end
 
     it 'properlies update commented values without separator' do
       ini_sub.set_value('section1', "xyzzy['thing1']['thing2']", 'xyzzyvalue')
-      expect(ini_sub.get_value('section1', "xyzzy['thing1']['thing2']")).to eq('xyzzyvalue')
+      expect(ini_sub.get_value('section1', "xyzzy['thing1']['thing2']")).to eq(['xyzzyvalue'])
     end
 
     it 'properlies add new empty values' do
       ini_sub.set_value('section1', 'baz', ' = ', 'bazvalue')
-      expect(ini_sub.get_value('section1', 'baz')).to eq('bazvalue')
+      expect(ini_sub.get_value('section1', 'baz')).to eq(['bazvalue'])
     end
 
     it 'adds new empty values without separator' do
       ini_sub.set_value('section1', 'baz', 'bazvalue')
-      expect(ini_sub.get_value('section1', 'baz')).to eq('bazvalue')
+      expect(ini_sub.get_value('section1', 'baz')).to eq(['bazvalue'])
     end
   end
 
@@ -258,10 +258,10 @@
     end
 
     it 'exposes settings for sections #print' do
-      expect(ini_sub.get_value('khotkeys', '{5465e8c7-d608-4493-a48f-b99d99fdb508}')).to eq('Print,none,PrintScreen')
+      expect(ini_sub.get_value('khotkeys', '{5465e8c7-d608-4493-a48f-b99d99fdb508}')).to eq(['Print,none,PrintScreen'])
     end
     it 'exposes settings for sections #search' do
-      expect(ini_sub.get_value('khotkeys', '{d03619b6-9b3c-48cc-9d9c-a2aadb485550}')).to eq('Search,none,Search')
+      expect(ini_sub.get_value('khotkeys', '{d03619b6-9b3c-48cc-9d9c-a2aadb485550}')).to eq(['Search,none,Search'])
     end
   end
 
@@ -277,13 +277,13 @@
     end
 
     it 'exposes settings for sections #A' do
-      expect(ini_sub.get_value('Drive names', 'A:')).to eq '5.25" Floppy'
+      expect(ini_sub.get_value('Drive names', 'A:')).to eq ['5.25" Floppy']
     end
     it 'exposes settings for sections #B' do
-      expect(ini_sub.get_value('Drive names', 'B:')).to eq '3.5" Floppy'
+      expect(ini_sub.get_value('Drive names', 'B:')).to eq ['3.5" Floppy']
     end
     it 'exposes settings for sections #C' do
-      expect(ini_sub.get_value('Drive names', 'C:')).to eq 'Winchester'
+      expect(ini_sub.get_value('Drive names', 'C:')).to eq ['Winchester']
     end
   end
 
@@ -302,16 +302,48 @@
     end
 
     it 'exposes settings for sections #log' do
-      expect(ini_sub.get_value('global', 'log file')).to eq '/var/log/samba/log.%m'
+      expect(ini_sub.get_value('global', 'log file')).to eq ['/var/log/samba/log.%m']
     end
     it 'exposes settings for sections #kerberos' do
-      expect(ini_sub.get_value('global', 'kerberos method')).to eq 'system keytab'
+      expect(ini_sub.get_value('global', 'kerberos method')).to eq ['system keytab']
     end
     it 'exposes settings for sections #passdb' do
-      expect(ini_sub.get_value('global', 'passdb backend')).to eq 'tdbsam'
+      expect(ini_sub.get_value('global', 'passdb backend')).to eq ['tdbsam']
     end
     it 'exposes settings for sections #security' do
-      expect(ini_sub.get_value('global', 'security')).to eq 'ads'
+      expect(ini_sub.get_value('global', 'security')).to eq ['ads']
+    end
+  end
+
+  context 'when using keys with multiple values' do
+    let(:sample_content) do
+      template = <<-EOS
+        [section]
+        foo=value1
+        foo=value2
+        bar=barvalue
+      EOS
+      template.split("\n")
+    end
+
+    it 'exposes settings for section #section' do
+      expect(ini_sub.get_settings('section')).to eq('foo' => ['value1', 'value2'],
+                                                    'bar' => ['barvalue'])
+    end
+
+    it 'adds multiple values' do
+      ini_sub.set_value('section', 'baz', ['bazvalue', 'bazvalue2'])
+      expect(ini_sub.get_value('section', 'baz')).to eq(['bazvalue', 'bazvalue2'])
+    end
+
+    it 'updates multiple values' do
+      ini_sub.set_value('section', 'bar', ['barvalue', 'barvalue2'])
+      expect(ini_sub.get_value('section', 'bar')).to eq(['barvalue', 'barvalue2'])
+    end
+
+    it 'removes extra values' do
+      ini_sub.set_value('section', 'foo', 'value2')
+      expect(ini_sub.get_value('section', 'foo')).to eq(['value2'])
     end
   end
 end

From e8753c2b694eed8c44d51d79f6b81c9111bbe1d3 Mon Sep 17 00:00:00 2001
From: Onno Molenkamp <onno@flox.org>
Date: Mon, 20 Jun 2022 11:33:19 +0200
Subject: [PATCH 2/2] Fix RuboCop errors

---
 lib/puppet/type/ini_setting.rb                    |  2 +-
 lib/puppet/util/ini_file.rb                       | 15 +++++++--------
 .../unit/puppet/provider/ini_setting/ruby_spec.rb |  4 ++--
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/lib/puppet/type/ini_setting.rb b/lib/puppet/type/ini_setting.rb
index 094c0139..dbeda852 100644
--- a/lib/puppet/type/ini_setting.rb
+++ b/lib/puppet/type/ini_setting.rb
@@ -85,7 +85,7 @@ def munge_boolean_md5(value)
     defaultto(' = ')
   end
 
-  newproperty(:value, :array_matching => :all) do
+  newproperty(:value, array_matching: :all) do
     desc 'The value of the setting to be defined.'
 
     munge do |value|
diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb
index 459e4b90..b7029e6d 100644
--- a/lib/puppet/util/ini_file.rb
+++ b/lib/puppet/util/ini_file.rb
@@ -263,16 +263,15 @@ def update_line(section, setting, value)
       updated = false
       section.end_line.downto section.start_line do |line_num|
         next unless (match = @setting_regex.match(lines[line_num]))
-        if match[2] == setting
-          lines.delete_at(line_num)
+        next unless match[2] == setting
 
-          if !updated
-            value.each_with_index do |v, i|
-              lines.insert(line_num + i, "#{match[1]}#{match[2]}#{match[3]}#{v}")
-            end
-            updated = true
-          end
+        lines.delete_at(line_num)
+        next if updated
+
+        value.each_with_index do |v, i|
+          lines.insert(line_num + i, "#{match[1]}#{match[2]}#{match[3]}#{v}")
         end
+        updated = true
       end
     end
 
diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
index 8a0933ff..2e6666a2 100644
--- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
+++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
@@ -1589,7 +1589,7 @@ def self.file_path
 
   context 'when using keys with multiple values' do
     let(:orig_content) do
-        <<-EOS
+      <<-EOS
 [section]
 foo = foovalue
 foo = foovalue2
@@ -1603,7 +1603,7 @@ def self.file_path
 foo = value2
 
 [section3]
-        EOS
+      EOS
     end
 
     it 'retains an existing setting with setting the original value' do