From: Jonathan Tripathy <jonathan.tripathy@puppetlabs.com>
Date: Wed, 21 Jan 2015 08:48:12 +0000 (-0800)
Subject: Fix for MODULES-1688
X-Git-Tag: 1.4.0~11^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=19ea03665630ea7e9719b4610055c66d807bdd7f;p=puppet-modules%2Fpuppetlabs-firewall.git

Fix for MODULES-1688

Re-applying a manifest with an unchanged UID will now not re-apply
the rule unnecessarily.
---

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 88d1aaf..b2bccb9 100644
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -732,7 +732,25 @@ Puppet::Type.newtype(:firewall) do
     EOS
     def insync?(is)
       require 'etc'
-      return is.to_s == @should.first.to_s || Etc.getpwuid(Integer(is)).name == @should.first.to_s
+
+      # The following code allow us to take into consideration unix mappings
+      # between string usernames and UIDs (integers). We also need to ignore
+      # spaces as they are irrelevant with respect to rule sync.
+
+      is = is.gsub(/\s+/,'')
+
+      if is.start_with?('!')
+        lookup_id = is.gsub(/^!/,'')
+        negate = '!'
+      else
+        lookup_id = is
+        negate = ''
+      end
+
+      resolve = Etc.getpwuid(Integer(lookup_id)).name
+      resolve_with_negate = "#{negate}#{resolve}"
+
+      return is.to_s == @should.first.to_s.gsub(/\s+/,'') || resolve_with_negate == @should.first.to_s.gsub(/\s+/,'')
     end
 
   end
diff --git a/spec/acceptance/firewall_uid_spec.rb b/spec/acceptance/firewall_uid_spec.rb
new file mode 100644
index 0000000..50728b4
--- /dev/null
+++ b/spec/acceptance/firewall_uid_spec.rb
@@ -0,0 +1,117 @@
+require 'spec_helper_acceptance'
+
+describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do
+
+  describe 'reset' do
+    it 'deletes all rules' do
+      shell('iptables --flush; iptables -t nat --flush; iptables -t mangle --flush')
+    end
+    it 'deletes all ip6tables rules' do
+      shell('ip6tables --flush; ip6tables -t nat --flush; ip6tables -t mangle --flush')
+    end
+  end
+
+  describe "uid tests" do
+    context 'uid set to root' do
+      it 'applies' do
+        pp = <<-EOS
+          class { '::firewall': }
+          firewall { '801 - test':
+            chain => 'OUTPUT',
+            action => accept,
+            uid => 'root',
+            proto => 'all',
+          }
+        EOS
+
+        apply_manifest(pp, :catch_failures => true)
+        unless fact('selinux') == 'true'
+          apply_manifest(pp, :catch_changes => true)
+        end
+      end
+
+      it 'should contain the rule' do
+         shell('iptables-save') do |r|
+           expect(r.stdout).to match(/-A OUTPUT -m owner --uid-owner (0|root) -m comment --comment "801 - test" -j ACCEPT/)
+         end
+      end
+    end
+
+    context 'uid set to !root' do
+      it 'applies' do
+        pp = <<-EOS
+          class { '::firewall': }
+          firewall { '802 - test':
+            chain => 'OUTPUT',
+            action => accept,
+            uid => '!root',
+            proto => 'all',
+          }
+        EOS
+
+        apply_manifest(pp, :catch_failures => true)
+        unless fact('selinux') == 'true'
+          apply_manifest(pp, :catch_changes => true)
+        end
+      end
+
+      it 'should contain the rule' do
+         shell('iptables-save') do |r|
+           expect(r.stdout).to match(/-A OUTPUT -m owner ! --uid-owner (0|root) -m comment --comment "802 - test" -j ACCEPT/)
+         end
+      end
+    end
+
+    context 'uid set to 0' do
+      it 'applies' do
+        pp = <<-EOS
+          class { '::firewall': }
+          firewall { '803 - test':
+            chain => 'OUTPUT',
+            action => accept,
+            uid => '0',
+            proto => 'all',
+          }
+        EOS
+
+        apply_manifest(pp, :catch_failures => true)
+        unless fact('selinux') == 'true'
+          apply_manifest(pp, :catch_changes => true)
+        end
+      end
+
+      it 'should contain the rule' do
+         shell('iptables-save') do |r|
+           expect(r.stdout).to match(/-A OUTPUT -m owner --uid-owner (0|root) -m comment --comment "803 - test" -j ACCEPT/)
+         end
+      end
+    end
+
+    context 'uid set to !0' do
+      it 'applies' do
+        pp = <<-EOS
+          class { '::firewall': }
+          firewall { '804 - test':
+            chain => 'OUTPUT',
+            action => accept,
+            uid => '!0',
+            proto => 'all',
+          }
+        EOS
+
+        apply_manifest(pp, :catch_failures => true)
+        unless fact('selinux') == 'true'
+          apply_manifest(pp, :catch_changes => true)
+        end
+      end
+
+      it 'should contain the rule' do
+         shell('iptables-save') do |r|
+           expect(r.stdout).to match(/-A OUTPUT -m owner ! --uid-owner (0|root) -m comment --comment "804 - test" -j ACCEPT/)
+         end
+      end
+    end
+
+  end
+
+end