]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
Fix for MODULES-1688
authorJonathan Tripathy <jonathan.tripathy@puppetlabs.com>
Wed, 21 Jan 2015 08:48:12 +0000 (00:48 -0800)
committerJonathan Tripathy <jonathan.tripathy@puppetlabs.com>
Wed, 21 Jan 2015 08:48:12 +0000 (00:48 -0800)
Re-applying a manifest with an unchanged UID will now not re-apply
the rule unnecessarily.

lib/puppet/type/firewall.rb
spec/acceptance/firewall_uid_spec.rb [new file with mode: 0644]

index 88d1aaf533ce0bdf12a952910f96d9c790781855..b2bccb96dc68313a28553f81aec4fbb8f8d83796 100644 (file)
@@ -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 (file)
index 0000000..50728b4
--- /dev/null
@@ -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