Accept pre-existing rule with invalid name

This patch fixes up a pre-existing rule whose name does not
type-validate with a valid name (typically one without a numeric
prefix in the comment).

Fixes #116
Signed-off-by: Joe Julian <me@joejulian.name>

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index df8744ba80ef3ef29afe44ba7e3c401531c33b86..4f033712ef1e2d615cfc6845f75552d1b68f21f9 100644 (file)
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -228,9 +228,15 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
     # iptables-save and user supplied resources is consistent.
     hash[:state] = hash[:state].sort unless hash[:state].nil?
 
-    # This forces all existing, commentless rules to be moved to the bottom of the stack.
-    # Puppet-firewall requires that all rules have comments (resource names) and will fail if
-    # a rule in iptables does not have a comment. We get around this by appending a high level
+    # This forces all existing, commentless rules or rules with invalid comments to be moved 
+    # to the bottom of the stack.
+    # Puppet-firewall requires that all rules have comments (resource names) and match this 
+    # regex and will fail if a rule in iptables does not have a comment. We get around this 
+    # by appending a high level
+    if not /^\d+[[:alpha:][:digit:][:punct:][:space:]]+$/ =~ hash[:name]
+      num = 9000 + counter
+      hash[:name] = "#{num} #{/([[:alpha:][:digit:][:punct:][:space:]]+)/.match(hash[:name])[1]}"
+    end
     if ! hash[:name]
       num = 9000 + counter
       hash[:name] = "#{num} #{Digest::MD5.hexdigest(line)}"

diff --git a/spec/system/resource_cmd_spec.rb b/spec/system/resource_cmd_spec.rb
index 09f70844fbc52a18555876570e30f535da648310..091faae981fc4d86e16be10d8048afe1e5ec172f 100644 (file)
--- a/spec/system/resource_cmd_spec.rb
+++ b/spec/system/resource_cmd_spec.rb
@@ -22,4 +22,26 @@ describe 'puppet resource firewall command:' do
       r[:stdout].should == "\n"
     end
   end
+  
+  it 'accepts rules without comments' do
+    iptables_flush_all_tables
+    system_run('/sbin/iptables -A INPUT -j ACCEPT -p tcp --dport 80')
+
+    puppet_resource('firewall') do |r|
+      r[:exit_code].should == 0
+      # don't check stdout, testing preexisting rules, output is normal
+      r[:stderr].should == ''
+    end
+  end
+
+  it 'accepts rules with invalid comments' do
+    iptables_flush_all_tables
+    system_run('/sbin/iptables -A INPUT -j ACCEPT -p tcp --dport 80 -m comment --comment "http"')
+
+    puppet_resource('firewall') do |r|
+      r[:exit_code].should == 0
+      # don't check stdout, testing preexisting rules, output is normal
+      r[:stderr].should == ''
+    end
+  end
 end