Merge pull request #152 from stepanstipl/feature-keep_secret2

Added keep_secret parameter feature

README.markdown

@@ -308,6 +308,17 @@ Determines whether the specified setting should exist. Valid options: 'present'
 
 *Required.* Designates a setting to manage within the specified INI file and section. Valid options: a string.
 
+##### `show_diff`
+
+*Optional.* Prevents outputting actual values to the logfile. Useful for handling of passwords and other sensitive information. Possible values are:
+  * `true`: This allows all values to be passed to logfiles. (default)
+  * `false`: The values in the logfiles will be replaced with `[redacted sensitive information]`. 
+  * `md5`: The values in the logfiles will be replaced with their md5 hash.
+
+Global show_diff configuraton takes priority over this one -
+[https://docs.puppetlabs.com/references/latest/configuration.html#showdiff]([https://docs.puppetlabs.com/references/latest/configuration.html#showdiff].
+). Default value: 'true'.
+
 ##### `value`
 
 *Optional.* Supplies a value for the specified setting. Valid options: a string. Default value: undefined.
@@ -352,6 +363,17 @@ Specifies whether the subsetting should be present. Valid options: 'present' and
 
 *Required.* Designates a setting within the specified section containing the subsetting to manage. Valid options: a string.
 
+##### `show_diff`
+
+*Optional.* Prevents outputting actual values to the logfile. Useful for handling of passwords and other sensitive information. Possible values are:
+  * `true`: This allows all values to be passed to logfiles. (default)
+  * `false`: The values in the logfiles will be replaced with `[redacted sensitive information]`. 
+  * `md5`: The values in the logfiles will be replaced with their md5 hash.
+
+Global show_diff configuraton takes priority over this one -
+[https://docs.puppetlabs.com/references/latest/configuration.html#showdiff]([https://docs.puppetlabs.com/references/latest/configuration.html#showdiff].
+). Default value: 'true'.
+
 ##### `subsetting`
 
 *Required.* Designates a subsetting to manage within the specified setting. Valid options: a string.

lib/puppet/type/ini_setting.rb

@@ -1,3 +1,5 @@
+require 'digest/md5'
+
 Puppet::Type.newtype(:ini_setting) do
 
   ensurable do
@@ -5,6 +7,19 @@ Puppet::Type.newtype(:ini_setting) do
     defaultto :present
   end
 
+  def munge_boolean_md5(value)
+    case value
+    when true, :true, 'true', :yes, 'yes'
+      :true
+    when false, :false, 'false', :no, 'no'
+      :false
+    when :md5, 'md5'
+      :md5
+    else
+      fail('expected a boolean value or :md5')
+    end
+  end
+
   newparam(:name, :namevar => true) do
     desc 'An arbitrary name used as the identity of the resource.'
   end
@@ -34,6 +49,18 @@ Puppet::Type.newtype(:ini_setting) do
     end
   end
 
+  newparam(:show_diff) do
+    desc 'Whether to display differences when the setting changes.'
+
+    defaultto :true
+
+    newvalues(:true, :md5, :false)
+
+    munge do |value|
+      @resource.munge_boolean_md5(value)
+    end
+  end
+
   newparam(:key_val_separator) do
     desc 'The separator string to use between each setting name and value. ' +
         'Defaults to " = ", but you could use this to override e.g. ": ", or' +
@@ -43,17 +70,31 @@ Puppet::Type.newtype(:ini_setting) do
 
   newproperty(:value) do
     desc 'The value of the setting to be defined.'
+
+    def should_to_s(newvalue)
+      if (@resource[:show_diff] == :true && Puppet[:show_diff]) then
+        return newvalue
+      elsif (@resource[:show_diff] == :md5 && Puppet[:show_diff]) then
+        return '{md5}' + Digest::MD5.hexdigest(newvalue.to_s)
+      else
+        return '[redacted sensitive information]'
+      end
+    end
+
+    def is_to_s(value)
+      should_to_s(value)
+    end
   end
 
   newparam(:section_prefix) do
     desc 'The prefix to the section name\'s header.' +
-        'Defaults to \'[\'.'
+      'Defaults to \'[\'.'
     defaultto('[')
   end
 
   newparam(:section_suffix) do
     desc 'The suffix to the section name\'s header.' +
-        'Defaults to \']\'.'
+      'Defaults to \']\'.'
     defaultto(']')
   end
 

lib/puppet/type/ini_subsetting.rb

@@ -1,3 +1,5 @@
+require 'digest/md5'
+
 Puppet::Type.newtype(:ini_subsetting) do
 
   ensurable do
@@ -5,6 +7,19 @@ Puppet::Type.newtype(:ini_subsetting) do
     defaultto :present
   end
 
+  def munge_boolean_md5(value)
+    case value
+    when true, :true, 'true', :yes, 'yes'
+      :true
+    when false, :false, 'false', :no, 'no'
+      :false
+    when :md5, 'md5'
+      :md5
+    else
+      fail('expected a boolean value or :md5')
+    end
+  end
+
   newparam(:name, :namevar => true) do
     desc 'An arbitrary name used as the identity of the resource.'
   end
@@ -37,6 +52,16 @@ Puppet::Type.newtype(:ini_subsetting) do
     end
   end
 
+  newparam(:show_diff) do
+    desc 'Whether to display differences when the setting changes.'
+    defaultto :true
+    newvalues(:true, :md5, :false)
+
+    munge do |value|
+      @resource.munge_boolean_md5(value)
+    end
+  end
+
   newparam(:key_val_separator) do
     desc 'The separator string to use between each setting name and value. ' +
         'Defaults to " = ", but you could use this to override e.g. ": ", or' +
@@ -64,6 +89,24 @@ Puppet::Type.newtype(:ini_subsetting) do
 
   newproperty(:value) do
     desc 'The value of the subsetting to be defined.'
+
+    def should_to_s(newvalue)
+      if (@resource[:show_diff] == :true && Puppet[:show_diff]) then
+        return newvalue
+      elsif (@resource[:show_diff] == :md5 && Puppet[:show_diff]) then
+        return '{md5}' + Digest::MD5.hexdigest(newvalue.to_s)
+      else
+        return '[redacted sensitive information]'
+      end
+    end
+
+    def is_to_s(value)
+      should_to_s(value)
+    end
+
+    def is_to_s(value)
+      should_to_s(value)
+    end
   end
 
 end

spec/acceptance/ini_setting_spec.rb

@@ -281,4 +281,40 @@ describe 'ini_setting resource' do
       end
     end
   end
+
+  describe 'show_diff parameter and logging:' do
+    [ {:value => "initial_value", :matcher => "created", :show_diff => true},
+      {:value => "public_value", :matcher => /initial_value.*public_value/, :show_diff => true},
+      {:value => "secret_value", :matcher => /redacted sensitive information.*redacted sensitive information/, :show_diff => false},
+      {:value => "md5_value", :matcher => /{md5}881671aa2bbc680bc530c4353125052b.*{md5}ed0903a7fa5de7886ca1a7a9ad06cf51/, :show_diff => :md5}
+    ].each do |i|
+      context "show_diff => #{i[:show_diff]}" do
+        pp = <<-EOS
+          ini_setting { 'test_show_diff':
+            ensure      => present,
+            section     => 'test',
+            setting     => 'something',
+            value       => '#{i[:value]}',
+            path        => "#{tmpdir}/test_show_diff.ini",
+            show_diff   => #{i[:show_diff]} 
+          }
+        EOS
+
+        it "applies manifest and expects changed value to be logged in proper form" do
+          config = {
+            'main' => {
+              'show_diff'   => true
+            }
+          }
+          configure_puppet_on(default, config)
+
+          res = apply_manifest(pp, :expect_changes => true)
+          expect(res.stdout).to match(i[:matcher])
+          expect(res.stdout).not_to match(i[:value]) unless (i[:show_diff] == true)
+
+        end
+      end
+    end
+  end
+
 end

spec/acceptance/ini_subsetting_spec.rb

@@ -197,4 +197,41 @@ describe 'ini_subsetting resource' do
       end
     end
   end
+
+  describe 'show_diff parameter and logging:' do
+    [ {:value => "initial_value", :matcher => "created", :show_diff => true},
+      {:value => "public_value", :matcher => /initial_value.*public_value/, :show_diff => true},
+      {:value => "secret_value", :matcher => /redacted sensitive information.*redacted sensitive information/, :show_diff => false},
+      {:value => "md5_value", :matcher => /{md5}881671aa2bbc680bc530c4353125052b.*{md5}ed0903a7fa5de7886ca1a7a9ad06cf51/, :show_diff => :md5}
+    ].each do |i|
+      context "show_diff => #{i[:show_diff]}" do
+        pp = <<-EOS
+          ini_subsetting { 'test_show_diff':
+            ensure      => present,
+            section     => 'test',
+            setting     => 'something',
+            subsetting  => 'xxx',
+            value       => '#{i[:value]}',
+            path        => "#{tmpdir}/test_show_diff.ini",
+            show_diff   => #{i[:show_diff]} 
+          }
+        EOS
+
+        it "applies manifest and expects changed value to be logged in proper form" do
+          config = {
+            'main' => {
+              'show_diff'   => true
+            }
+          }
+          configure_puppet_on(default, config)
+
+          res = apply_manifest(pp, :expect_changes => true)
+          expect(res.stdout).to match(i[:matcher])
+          expect(res.stdout).not_to match(i[:value]) unless (i[:show_diff] == true)
+
+        end
+      end
+    end
+  end
+
 end

spec/unit/puppet/type/ini_setting_spec.rb

@@ -0,0 +1,63 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+
+ini_setting = Puppet::Type.type(:ini_setting)
+
+describe ini_setting do
+
+  [true, false].product([true, false, "true", "false", "md5", :md5]).each do |cfg, param|
+    describe "when Puppet[:show_diff] is #{cfg} and show_diff => #{param}" do
+
+      before do
+        Puppet[:show_diff] = cfg
+        @value = described_class.new(:name => 'foo', :value => 'whatever', :show_diff => param).property(:value)
+      end
+
+      if (cfg and [true, "true"].include? param)
+        it "should display diff" do
+          expect(@value.change_to_s('not_secret','at_all')).to include('not_secret','at_all')
+        end
+
+        it "should tell current value" do
+          expect(@value.is_to_s('not_secret_at_all')).to eq('not_secret_at_all')
+        end
+
+        it "should tell new value" do
+          expect(@value.should_to_s('not_secret_at_all')).to eq('not_secret_at_all')
+        end
+      elsif (cfg and ["md5", :md5].include? param)
+        it "should tell correct md5 hash for value" do
+          expect(@value.change_to_s('not_secret','at_all')).to include('e9e8db547f8960ef32dbc34029735564','46cd73a9509ba78c39f05faf078a8cbe')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('not_secret')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('at_all')
+        end
+
+        it "should tell md5 of current value, but not value itself" do
+          expect(@value.is_to_s('not_secret_at_all')).to eq('{md5}218fde79f501b8ab8d212f1059bb857f')
+          expect(@value.is_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+
+        it "should tell md5 of new value, but not value itself" do
+          expect(@value.should_to_s('not_secret_at_all')).to eq('{md5}218fde79f501b8ab8d212f1059bb857f')
+          expect(@value.should_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+      else
+        it "should not tell any actual values" do
+          expect(@value.change_to_s('not_secret','at_all')).to include('[redacted sensitive information]')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('not_secret')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('at_all')
+        end
+
+        it "should not tell current value" do
+          expect(@value.is_to_s('not_secret_at_all')).to eq('[redacted sensitive information]')
+          expect(@value.is_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+
+        it "should not tell new value" do
+          expect(@value.should_to_s('not_secret_at_all')).to eq('[redacted sensitive information]')
+          expect(@value.should_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+      end
+    end
+  end
+end

spec/unit/puppet/type/ini_subetting_spec.rb

@@ -0,0 +1,62 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+
+ini_subsetting = Puppet::Type.type(:ini_subsetting)
+
+describe ini_subsetting do
+  [true, false].product([true, false, :md5]).each do |cfg, param|
+    describe "when Puppet[:show_diff] is #{cfg} and show_diff => #{param}" do
+
+      before do
+        Puppet[:show_diff] = cfg
+        @value = described_class.new(:name => 'foo', :value => 'whatever', :show_diff => param).property(:value)
+      end
+
+      if (cfg and param == true)
+        it "should display diff" do
+          expect(@value.change_to_s('not_secret','at_all')).to include('not_secret','at_all')
+        end
+
+        it "should tell current value" do
+          expect(@value.is_to_s('not_secret_at_all')).to eq('not_secret_at_all')
+        end
+
+        it "should tell new value" do
+          expect(@value.should_to_s('not_secret_at_all')).to eq('not_secret_at_all')
+        end
+      elsif (cfg and param == :md5)
+        it "should tell correct md5 hash for value" do
+          expect(@value.change_to_s('not_secret','at_all')).to include('e9e8db547f8960ef32dbc34029735564','46cd73a9509ba78c39f05faf078a8cbe')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('not_secret')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('at_all')
+        end
+
+        it "should tell md5 of current value, but not value itself" do
+          expect(@value.is_to_s('not_secret_at_all')).to eq('{md5}218fde79f501b8ab8d212f1059bb857f')
+          expect(@value.is_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+
+        it "should tell md5 of new value, but not value itself" do
+          expect(@value.should_to_s('not_secret_at_all')).to eq('{md5}218fde79f501b8ab8d212f1059bb857f')
+          expect(@value.should_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+      else
+        it "should not tell any actual values" do
+          expect(@value.change_to_s('not_secret','at_all')).to include('[redacted sensitive information]')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('not_secret')
+          expect(@value.change_to_s('not_secret','at_all')).not_to include('at_all')
+        end
+
+        it "should not tell current value" do
+          expect(@value.is_to_s('not_secret_at_all')).to eq('[redacted sensitive information]')
+          expect(@value.is_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+
+        it "should not tell new value" do
+          expect(@value.should_to_s('not_secret_at_all')).to eq('[redacted sensitive information]')
+          expect(@value.should_to_s('not_secret_at_all')).not_to include('not_secret_at_all')
+        end
+      end
+    end
+  end
+end