From 6e7923870a932fa038f6c3a430f000ba3bfa95ca Mon Sep 17 00:00:00 2001 From: Jonathan Boyett Date: Wed, 30 Nov 2011 18:52:35 -0800 Subject: [PATCH] (#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. --- examples/iptables/test.pp | 6 ++++ lib/puppet/provider/firewall/iptables.rb | 6 ++++ lib/puppet/type/firewall.rb | 14 +++++++- lib/puppet/util/firewall.rb | 23 +++++++++++++ spec/fixtures/iptables/conversion_hash.rb | 40 ++++++++++++++++++++++- spec/unit/puppet/type/firewall_spec.rb | 29 ++++++++++++++++ 6 files changed, 116 insertions(+), 2 deletions(-) diff --git a/examples/iptables/test.pp b/examples/iptables/test.pp index 6dc91b8..4693013 100644 --- 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 4ecfea0..8bd1f62 100644 --- 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 6d12fab..4cecb78 100644 --- 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 6804774..cf7f659 100644 --- 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 0645a39..6f8ed6d 100644 --- 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 38ad5d4..03c3a03 100755 --- 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', -- 2.45.2