From: Joe Julian <me@joejulian.name>
Date: Thu, 30 May 2013 03:42:29 +0000 (-0700)
Subject: Accept pre-existing rule with invalid name
X-Git-Tag: 0.3.1~4^2~1
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=371c04e46c956c97b1ad55503414ec70ce74e635;p=puppet-modules%2Fpuppetlabs-firewall.git

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 df8744b..4f03371 100644
--- 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 09f7084..091faae 100644
--- 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