(#11093) Improve log_level property so it converts names to numbers

Previously the log_level property was constantly reloading due to the fact
that iptables was converting names to numbers. So unless you were using
numbers in your log_level setting, it was constantly telling you it needed
to be changed.

Now we convert the names to numbers in the munge so when comparing it will
always hopefully match.

Also, the default value when the jump value is 'LOG' is now set to 4 (warn)
based on iptables own defaults.

diff --git a/examples/iptables/test.pp b/examples/iptables/test.pp
index 6dc91b8ba0dcefe19c99c7fa72e1ca0cfac0ba91..46930133a37592e716d19810454d5689e652aed0 100644 (file)
--- a/examples/iptables/test.pp
+++ b/examples/iptables/test.pp
@@ -4,6 +4,12 @@ firewall { '000 allow foo':
   proto => "tcp",
 }
 
+firewall { '975 log test':
+  state => 'NEW',
+  log_level => 'panic',
+  jump => 'LOG'
+}
+
 firewall { '001 allow boo':
   action => accept,
   iniface => "eth0",

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 4ecfea073818f1e4b314f2c18f9a480c397e3cb4..8bd1f62cc4e76d62099d1d17174bc3e1133164c9 100644 (file)
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -150,6 +150,12 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
       hash[:name] = "9999 #{Digest::MD5.hexdigest(line)}"
     end
 
+    # Iptables defaults to log_level '4', so it is omitted from the output of iptables-save.
+    # If the :jump value is LOG and you don't have a log-level set, we assume it to be '4'.
+    if hash[:jump] == 'LOG' && ! hash[:log_level]
+      hash[:log_level] = '4'
+    end
+
     hash[:line] = line
     hash[:provider] = self.name.to_s
     hash[:table] = table

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 6d12fabec8ff1d863e90ab0d5804b4bea886d00d..4cecb78fe3a60c53ce72d916645d07cc3b515cc0 100644 (file)
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -9,7 +9,6 @@
 $LOAD_PATH.unshift(File.join(File.dirname(__FILE__),"..",".."))
 require 'puppet/util/firewall'
 
-# Puppet Firewall type
 Puppet::Type.newtype(:firewall) do
   include Puppet::Util::Firewall
 
@@ -315,6 +314,19 @@ Puppet::Type.newtype(:firewall) do
       When combined with jump => "LOG" specifies the system log level to log
       to.
     EOS
+
+    munge do |value|
+      if value.kind_of?(String)
+        value = @resource.log_level_name_to_number(value)
+      else
+        value
+      end
+
+      if value == nil && value != ""
+        self.fail("Unable to determine log level")
+      end
+      value
+    end
   end
 
   newproperty(:log_prefix, :required_features => :log_prefix) do

diff --git a/lib/puppet/util/firewall.rb b/lib/puppet/util/firewall.rb
index 6804774ad9d159fb1542cb1734cf8327382102a7..cf7f659393e2902152db1316b4514a3a24383a13 100644 (file)
--- a/lib/puppet/util/firewall.rb
+++ b/lib/puppet/util/firewall.rb
@@ -27,6 +27,29 @@ module Puppet::Util::Firewall
     end
   end
 
+  # Convert log_level names to their respective numbers
+  def log_level_name_to_number(value)
+    #TODO make this 0-7 only
+    if value =~ /\d/
+      value
+    else
+      case value
+        when "panic" then "0"
+        when "alert" then "1"
+        when "crit" then "2"
+        when "err" then "3"
+        when "error" then "3"
+        when "warn" then "4"
+        when "warning" then "4"
+        when "not" then "5"
+        when "notice" then "5"
+        when "info" then "6"
+        when "debug" then "7"
+        else nil
+      end
+    end
+  end
+
   # This method takes a string and attempts to convert it to a port number
   # if valid.
   #

diff --git a/spec/fixtures/iptables/conversion_hash.rb b/spec/fixtures/iptables/conversion_hash.rb
index 0645a394d572aeb2cc41e78fb6457700ffcdcbc6..6f8ed6d096212d6f91a4d862f79dbd92e24c07f3 100644 (file)
--- a/spec/fixtures/iptables/conversion_hash.rb
+++ b/spec/fixtures/iptables/conversion_hash.rb
@@ -95,11 +95,29 @@ ARGS_TO_HASH = {
   },
   'comment_string_character_validation' => {
     :line => '-A INPUT -s 192.168.0.1 -m comment --comment "000 allow from 192.168.0.1, please"',
-    :tables => 'filter',
+    :table => 'filter',
     :params => {
       :source => '192.168.0.1',
     },
   },
+  'log_level_debug' => {
+    :line => '-A INPUT -m comment --comment "956 INPUT log-level" -m state --state NEW -j LOG --log-level 7',
+    :table => 'filter',
+    :params => {
+      :state => ['NEW'],
+      :log_level => '7',
+      :jump => 'LOG'
+    },
+  },
+  'log_level_warn' => {
+    :line => '-A INPUT -m comment --comment "956 INPUT log-level" -m state --state NEW -j LOG',
+    :table => 'filter',
+    :params => {
+      :state => ['NEW'],
+      :log_level => '4',
+      :jump => 'LOG'
+    },
+  },
   'load_limit_module' => {
     :line => '-A INPUT -m multiport --dports 123 -m comment --comment "057 INPUT limit NTP" -m limit --limit 15/hour',
     :table => 'filter',
@@ -250,6 +268,26 @@ HASH_TO_ARGS = {
     },
     :args => ['-t', :filter, '-p', :tcp, '-m', 'multiport', '--ports', '80', '-m', 'comment', '--comment', '001 port property'],
   },
+  'log_level_debug' => {
+    :params => {
+      :name => '956 INPUT log-level',
+      :table => 'filter',
+      :state => 'NEW',
+      :jump => 'LOG',
+      :log_level => 'debug'
+    },
+    :args => ['-t', :filter, '-p', :tcp, '-m', 'comment', '--comment', '956 INPUT log-level', '-m', 'state', '--state', 'NEW', '-j', 'LOG', '--log-level', '7'],
+  },
+  'log_level_warn' => {
+    :params => {
+      :name => '956 INPUT log-level',
+      :table => 'filter',
+      :state => 'NEW',
+      :jump => 'LOG',
+      :log_level => 'warn'
+    },
+    :args => ['-t', :filter, '-p', :tcp, '-m', 'comment', '--comment', '956 INPUT log-level', '-m', 'state', '--state', 'NEW', '-j', 'LOG', '--log-level', '4'],
+  },
   'load_limit_module' => {
     :params => {
       :name => '057 INPUT limit NTP',

diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 38ad5d4a0d15479d9359f8f98861355392daf83b..03c3a0366afa5d10cfefeff3beea455087a003e9 100755 (executable)
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -175,6 +175,35 @@ describe firewall do
     end
   end
 
+  describe ':log_level' do
+    values = {
+      'panic' => '0',
+      'alert' => '1',
+      'crit'  => '2',
+      'err'   => '3',
+      'warn'  => '4',
+      'warning' => '4',
+      'not'  => '5',
+      'notice' => '5',
+      'info' => '6',
+      'debug' => '7'
+    }
+
+    values.each do |k,v|
+      it {
+        @resource[:log_level] = k
+        @resource[:log_level].should == v
+      }
+
+      it {
+        @resource[:log_level] = 3
+        @resource[:log_level].should == 3
+      }
+
+      it { lambda { @resource[:log_level] = 'foo' }.should raise_error(Puppet::Error) }
+    end
+  end
+
   describe ':icmp' do
     values = {
       '0' => 'echo-reply',