]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
Improve support for EL7 and other related fixes
authorTrey Dockendorf <treydock@gmail.com>
Mon, 21 Jul 2014 18:55:24 +0000 (13:55 -0500)
committerHunter Haugen <hunter@puppetlabs.com>
Wed, 23 Jul 2014 00:30:21 +0000 (17:30 -0700)
* Support RHEL7 by removing firewalld before installing iptables-services
* Autorequire Package[iptables-services] for Firewall and Firewallchain types
* Ensure /etc/sysconfig/iptables exists before starting Service[iptables]

lib/puppet/type/firewall.rb
lib/puppet/type/firewallchain.rb
manifests/linux/redhat.pp
spec/unit/classes/firewall_linux_redhat_spec.rb
spec/unit/classes/firewall_linux_spec.rb
spec/unit/puppet/type/firewall_spec.rb
spec/unit/puppet/type/firewallchain_spec.rb

index e6be89e25c20ae943bf99229a73c9bdcebb9faf6..bf28f7339754b924b55892b08511c3297499fe9f 100644 (file)
@@ -22,8 +22,8 @@ Puppet::Type.newtype(:firewall) do
     `chain` or `jump` parameters, the firewall resource will autorequire
     those firewallchain resources.
 
-    If Puppet is managing the iptables or iptables-persistent packages, and
-    the provider is iptables or ip6tables, the firewall resource will
+    If Puppet is managing the iptables, iptables-persistent, or iptables-services packages,
+    and the provider is iptables or ip6tables, the firewall resource will
     autorequire those packages to ensure that any required binaries are
     installed.
   EOS
@@ -937,7 +937,7 @@ Puppet::Type.newtype(:firewall) do
   autorequire(:package) do
     case value(:provider)
     when :iptables, :ip6tables
-      %w{iptables iptables-persistent}
+      %w{iptables iptables-persistent iptables-services}
     else
       []
     end
index 3e3c5d1370696593ff844fbc14ca54363f0ce19e..b962a0a36b283d1dedfcca701099dec0c20cdeb8 100644 (file)
@@ -18,8 +18,8 @@ Puppet::Type.newtype(:firewallchain) do
     allow it.
 
     **Autorequires:**
-    If Puppet is managing the iptables or iptables-persistent packages, and
-    the provider is iptables_chain, the firewall resource will autorequire
+    If Puppet is managing the iptables, iptables-persistent, or iptables-services packages,
+    and the provider is iptables_chain, the firewall resource will autorequire
     those packages to ensure that any required binaries are installed.
   EOS
 
@@ -151,7 +151,7 @@ Puppet::Type.newtype(:firewallchain) do
   autorequire(:package) do
     case value(:provider)
     when :iptables_chain
-      %w{iptables iptables-persistent}
+      %w{iptables iptables-persistent iptables-services}
     else
       []
     end
index f697d211b9f660dfb0164798b68076d1b00f2d3f..b7a4d0e3f770c5bb3f63edf9d2b52253b4786638 100644 (file)
@@ -20,15 +20,16 @@ class firewall::linux::redhat (
   # RHEL 7 and later and Fedora 15 and later require the iptables-services
   # package, which provides the /usr/libexec/iptables/iptables.init used by
   # lib/puppet/util/firewall.rb.
-  if $::operatingsystem == RedHat and $::operatingsystemrelease >= 7 {
-    package { 'iptables-services':
-      ensure => present,
+  if   ($::operatingsystem != 'Fedora' and versioncmp($::operatingsystemrelease, '7.0') >= 0)
+    or ($::operatingsystem == 'Fedora' and versioncmp($::operatingsystemrelease, '15') >= 0) {
+    package { 'firewalld':
+      ensure  => absent,
+      before  => Package['iptables-services'],
     }
-  }
 
-  if ($::operatingsystem == 'Fedora' and (( $::operatingsystemrelease =~ /^\d+/ and $::operatingsystemrelease >= 15 ) or $::operatingsystemrelease == "Rawhide")) {
     package { 'iptables-services':
-      ensure => present,
+      ensure  => present,
+      before  => Service['iptables'],
     }
   }
 
@@ -36,5 +37,13 @@ class firewall::linux::redhat (
     ensure    => $ensure,
     enable    => $enable,
     hasstatus => true,
+    require   => File['/etc/sysconfig/iptables'],
+  }
+
+  file { '/etc/sysconfig/iptables':
+    ensure  => present,
+    owner   => 'root',
+    group   => 'root',
+    mode    => '0600',
   }
 }
index ea49d2b83b8be514fb7bb01116e7374681353236..43263fa6709f09c968171454d7787b270e48f30b 100644 (file)
@@ -1,22 +1,60 @@
 require 'spec_helper'
 
 describe 'firewall::linux::redhat', :type => :class do
-  it { should contain_service('iptables').with(
-    :ensure => 'running',
-    :enable => 'true'
-  )}
+  %w{RedHat CentOS Fedora}.each do |os|
+    oldreleases = (os == 'Fedora' ? ['14'] : ['6.5'])
+    newreleases = (os == 'Fedora' ? ['15','Rawhide'] : ['7.0.1406'])
 
-  context 'ensure => stopped' do
-    let(:params) {{ :ensure => 'stopped' }}
-    it { should contain_service('iptables').with(
-      :ensure => 'stopped'
-    )}
-  end
+    oldreleases.each do |osrel|
+      context "os #{os} and osrel #{osrel}" do
+        let(:facts) {{
+          :operatingsystem        => os,
+          :operatingsystemrelease => osrel
+        }}
+
+        it { should_not contain_package('firewalld') }
+        it { should_not contain_package('iptables-services') }
+      end
+    end
+
+    newreleases.each do |osrel|
+      context "os #{os} and osrel #{osrel}" do
+        let(:facts) {{
+          :operatingsystem        => os,
+          :operatingsystemrelease => osrel
+        }}
+
+        it { should contain_package('firewalld').with(
+          :ensure => 'absent',
+          :before => 'Package[iptables-services]'
+        )}
+
+        it { should contain_package('iptables-services').with(
+          :ensure => 'present',
+          :before => 'Service[iptables]'
+        )}
+      end
+    end
 
-  context 'enable => false' do
-    let(:params) {{ :enable => 'false' }}
-    it { should contain_service('iptables').with(
-      :enable => 'false'
-    )}
+    describe 'ensure' do
+      context 'default' do
+        it { should contain_service('iptables').with(
+          :ensure => 'running',
+          :enable => 'true'
+        )}
+      end
+      context 'ensure => stopped' do
+        let(:params) {{ :ensure => 'stopped' }}
+        it { should contain_service('iptables').with(
+          :ensure => 'stopped'
+        )}
+      end
+      context 'enable => false' do
+        let(:params) {{ :enable => 'false' }}
+        it { should contain_service('iptables').with(
+          :enable => 'false'
+        )}
+      end
+    end
   end
 end
index 42056c1b1af0528bd97c99a0c75506f5621293e4..e43c1e9f809d7ab5c0887e987074314c4d976190 100644 (file)
@@ -7,7 +7,7 @@ describe 'firewall::linux', :type => :class do
   context 'RedHat like' do
     %w{RedHat CentOS Fedora}.each do |os|
       context "operatingsystem => #{os}" do
-        releases = (os == 'Fedora' ? [14,15,'Rawhide'] : [6,7])
+        releases = (os == 'Fedora' ? ['14','15','Rawhide'] : ['6','7'])
         releases.each do |osrel|
           context "operatingsystemrelease => #{osrel}" do
             let(:facts) { facts_default.merge({ :operatingsystem => os,
index 368d1873f6285e72948f58b5ee18bb56275dca79..8e9ef5663c3d5240cbbad14a642921286a6a3ead 100755 (executable)
@@ -628,12 +628,13 @@ describe firewall do
         rel.target.ref.should == @resource.ref
       end
 
-      it "provider #{provider} should autorequire packages iptables and iptables-persistent" do
+      it "provider #{provider} should autorequire packages iptables, iptables-persistent, and iptables-services" do
         @resource[:provider] = provider
         @resource[:provider].should == provider
         packages = [
           Puppet::Type.type(:package).new(:name => 'iptables'),
-          Puppet::Type.type(:package).new(:name => 'iptables-persistent')
+          Puppet::Type.type(:package).new(:name => 'iptables-persistent'),
+          Puppet::Type.type(:package).new(:name => 'iptables-services')
         ]
         catalog = Puppet::Resource::Catalog.new
         catalog.add_resource @resource
index 3ce7768ce87bc44bb5cea6b117c4503024a0fddf..bd3095e0993f8fa69b592e179a687185046fe69a 100755 (executable)
@@ -121,11 +121,12 @@ describe firewallchain do
       rel.target.ref.should == resource.ref
     end
 
-    it "provider iptables_chain should autorequire packages iptables and iptables-persistent" do
+    it "provider iptables_chain should autorequire packages iptables, iptables-persistent, and iptables-services" do
       resource[:provider].should == :iptables_chain
       packages = [
         Puppet::Type.type(:package).new(:name => 'iptables'),
-        Puppet::Type.type(:package).new(:name => 'iptables-persistent')
+        Puppet::Type.type(:package).new(:name => 'iptables-persistent'),
+        Puppet::Type.type(:package).new(:name => 'iptables-services')
       ]
       catalog = Puppet::Resource::Catalog.new
       catalog.add_resource resource