(SIMP-8691) Does not manage service when use_firewalld=true on EL6 (#87)

- Fixed a bug in which the iptables services and rules were not managed when iptables::use_firewalld was set to true on an EL6 system. - Maintenance - Removed unit test cruft left over from experimental firewalld support. - Split out firewalld suite into its own tests in the .gitlab-ci.yml so that individual test suite status could be more easily tracked. - Allow the default OEL nodeset to fail due to bugs in updating xt_recent with scanblock SIMP-8691 #close
simp · Nov 17, 2020 · 6aa59fc · 6aa59fc
1 parent 48b58e8
commit 6aa59fc
Show file tree

Hide file tree

Showing 26 changed files with 597 additions and 575 deletions.
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
@@ -208,10 +208,10 @@ variables:
     BEAKER_PUPPET_COLLECTION: 'puppet6'
     MATRIX_RUBY_VERSION: '2.5'
 
-.pup_6_16_0: &pup_6_16_0
+.pup_6_18_0: &pup_6_18_0
   image: 'ruby:2.5'
   variables:
-    PUPPET_VERSION: '6.16.0'
+    PUPPET_VERSION: '6.18.0'
     BEAKER_PUPPET_COLLECTION: 'puppet6'
     MATRIX_RUBY_VERSION: '2.5'
 
@@ -294,8 +294,8 @@ pup6-unit:
   <<: *unit_tests
   <<: *with_SIMP_SPEC_MATRIX_LEVEL_2
 
-pup6.16.0-unit:
-  <<: *pup_6_16_0
+pup6.18.0-unit:
+  <<: *pup_6_18_0
   <<: *unit_tests
 
 # ------------------------------------------------------------------------------
@@ -312,60 +312,96 @@ pup5.5.20:
   <<: *pup_5_5_20
   <<: *acceptance_base
   script:
-    - 'bundle exec rake beaker:suites'
+    - 'bundle exec rake beaker:suites[default,default]'
 
 pup5.5.20-fips:
   <<: *pup_5_5_20
   <<: *acceptance_base
   script:
-    - 'BEAKER_fips=yes bundle exec rake beaker:suites'
+    - 'BEAKER_fips=yes bundle exec rake beaker:suites[default,default]'
 
 pup5.5.20-oel:
+  allow_failure: true
   <<: *pup_5_5_20
   <<: *acceptance_base
+  <<: *with_SIMP_ACCEPTANCE_MATRIX_LEVEL_3
   script:
     - 'bundle exec rake beaker:suites[default,oel]'
 
 pup5.5.20-oel-fips:
+  allow_failure: true
   <<: *pup_5_5_20
   <<: *acceptance_base
   <<: *with_SIMP_ACCEPTANCE_MATRIX_LEVEL_3
   script:
     - 'BEAKER_fips=yes bundle exec rake beaker:suites[default,oel]'
 
+pup5.5.20-firewalld:
+  <<: *pup_5_5_20
+  <<: *acceptance_base
+  script:
+    - 'bundle exec rake beaker:suites[firewalld,default]'
+
+pup5.5.20-oel-firewalld:
+  <<: *pup_5_5_20
+  <<: *acceptance_base
+  script:
+    - 'bundle exec rake beaker:suites[firewalld,oel]'
+
 pup6:
   <<: *pup_6
   <<: *acceptance_base
   script:
-    - 'bundle exec rake beaker:suites'
+    - 'bundle exec rake beaker:suites[default,default]'
 
 pup6-fips:
   <<: *pup_6
   <<: *acceptance_base
   script:
-    - 'BEAKER_fips=yes bundle exec rake beaker:suites'
+    - 'BEAKER_fips=yes bundle exec rake beaker:suites[default,default]'
+
+pup6-firewalld:
+  <<: *pup_6
+  <<: *acceptance_base
+  script:
+    - 'bundle exec rake beaker:suites[firewalld,default]'
 
-pup6.16.0:
-  <<: *pup_6_16_0
+pup6.18.0:
+  <<: *pup_6_18_0
   <<: *acceptance_base
   script:
-    - 'bundle exec rake beaker:suites'
+    - 'bundle exec rake beaker:suites[default,default]'
 
-pup6.16.0-fips:
-  <<: *pup_6_16_0
+pup6.18.0-fips:
+  <<: *pup_6_18_0
   <<: *acceptance_base
   script:
-    - 'BEAKER_fips=yes bundle exec rake beaker:suites'
+    - 'BEAKER_fips=yes bundle exec rake beaker:suites[default,default]'
 
-pup6.16.0-oel:
-  <<: *pup_6_16_0
+pup6.18.0-oel:
+  allow_failure: true
+  <<: *pup_6_18_0
   <<: *acceptance_base
+  <<: *with_SIMP_ACCEPTANCE_MATRIX_LEVEL_3
   script:
     - 'bundle exec rake beaker:suites[default,oel]'
 
-pup6.16.0-oel-fips:
-  <<: *pup_6_16_0
+pup6.18.0-oel-fips:
+  allow_failure: true
+  <<: *pup_6_18_0
   <<: *acceptance_base
   <<: *with_SIMP_ACCEPTANCE_MATRIX_LEVEL_3
   script:
     - 'BEAKER_fips=yes bundle exec rake beaker:suites[default,oel]'
+
+pup6.18.0-firewalld:
+  <<: *pup_6_18_0
+  <<: *acceptance_base
+  script:
+    - 'bundle exec rake beaker:suites[firewalld,default]'
+
+pup6.18.0-oel-firewalld:
+  <<: *pup_6_18_0
+  <<: *acceptance_base
+  script:
+    - 'bundle exec rake beaker:suites[firewalld,oel]'
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,12 @@
+* Mon Nov 16 2020 Liz Nemsick <lnemsick.simp@gmail.com> - 6.5.4
+- Fixed a bug in which the iptables services and rules were not
+  managed when iptables::use_firewalld was set to true on an
+  EL6 system.
+- Fixed an ordering issue with setting `xt_recent` parameters that
+  could occur on OEL7 nodes. However, there are other issues
+  with `xt_recent` on OEL that may prevent this module from
+  working on OEL in some circumstances.
+
 * Fri Oct 23 2020 Trevor Vaughan <tvaughan@onyxpoint.com> - 6.5.3
 - Ensure that systems that do not have firewalld will not attempt to configure
   it.

diff --git a/manifests/init.pp b/manifests/init.pp
@@ -120,8 +120,9 @@
 
   simplib::assert_metadata($module_name)
 
+  $firewalld_mode = ( 'firewalld' in pick($facts['simplib__firewalls'], 'none') ) and $use_firewalld
   if $enable != 'ignore' {
-    if ( 'firewalld' in pick($facts['simplib__firewalls'], 'none') ) and $use_firewalld {
+    if $firewalld_mode {
       simplib::assert_optional_dependency($module_name, 'simp/simp_firewalld')
 
       include 'simp_firewalld'

diff --git a/manifests/listen/all.pp b/manifests/listen/all.pp
@@ -70,7 +70,7 @@
 ){
   include 'iptables'
 
-  if $iptables::use_firewalld {
+  if $iptables::firewalld_mode {
     simplib::assert_optional_dependency($module_name, 'simp/simp_firewalld')
 
     simp_firewalld::rule { "all_${name}":

diff --git a/manifests/listen/icmp.pp b/manifests/listen/icmp.pp
@@ -80,7 +80,7 @@
 ) {
   include 'iptables'
 
-  if $iptables::use_firewalld {
+  if $iptables::firewalld_mode {
     simplib::assert_optional_dependency($module_name, 'simp/simp_firewalld')
 
     simp_firewalld::rule { "icmp_${name}":

diff --git a/manifests/listen/tcp_stateful.pp b/manifests/listen/tcp_stateful.pp
@@ -77,7 +77,7 @@
 ) {
   include 'iptables'
 
-  if $iptables::use_firewalld {
+  if $iptables::firewalld_mode {
     simplib::assert_optional_dependency($module_name, 'simp/simp_firewalld')
 
     simp_firewalld::rule { "tcp_${name}":

diff --git a/manifests/listen/udp.pp b/manifests/listen/udp.pp
@@ -77,7 +77,7 @@
 ) {
   include 'iptables'
 
-  if $iptables::use_firewalld {
+  if $iptables::firewalld_mode {
     simplib::assert_optional_dependency($module_name, 'simp/simp_firewalld')
 
     simp_firewalld::rule { "udp_${name}":

diff --git a/manifests/rule.pp b/manifests/rule.pp
@@ -83,7 +83,7 @@
 ) {
   include iptables
 
-  if $iptables::use_firewalld {
+  if $iptables::firewalld_mode {
     $_caller = simplib::caller()
 
     notify { 'iptables::rule with firewalld':

diff --git a/manifests/rules/mod_recent.pp b/manifests/rules/mod_recent.pp
@@ -61,7 +61,6 @@
     refreshonly => true
   }
 
-  Xt_recent['/sys/module/xt_recent/parameters'] -> File['/etc/modprobe.d/xt_recent.conf']
   File['/etc/modprobe.d/xt_recent.conf'] ~> Exec['reload xt_recent']
 
   ### End workaround for kernel panic

diff --git a/manifests/service.pp b/manifests/service.pp
@@ -15,64 +15,62 @@
 ){
   simplib::assert_metadata($module_name)
 
-  unless $iptables::use_firewalld {
-    if $enable != 'ignore' {
-      if $enable {
-        $_ensure = 'running'
-        $_enable = true
-      }
-      else {
-        $_ensure = 'stopped'
-        $_enable = false
-      }
+  if $enable != 'ignore' {
+    if $enable {
+      $_ensure = 'running'
+      $_enable = true
+    }
+    else {
+      $_ensure = 'stopped'
+      $_enable = false
+    }
+
+    service { 'iptables':
+      ensure     => $_ensure,
+      enable     => $_enable,
+      hasrestart => false,
+      restart    => '/sbin/iptables-restore /etc/sysconfig/iptables || ( /sbin/iptables-restore /etc/sysconfig/iptables.bak && exit 3 )',
+      hasstatus  => true,
+      provider   => 'redhat'
+    }
+
+    service { 'iptables-retry':
+      enable   => $_enable,
+      provider => 'redhat'
+    }
 
-      service { 'iptables':
+    if $ipv6 and $facts['ipv6_enabled'] {
+      service { 'ip6tables':
         ensure     => $_ensure,
         enable     => $_enable,
         hasrestart => false,
-        restart    => '/sbin/iptables-restore /etc/sysconfig/iptables || ( /sbin/iptables-restore /etc/sysconfig/iptables.bak && exit 3 )',
+        restart    => '/sbin/ip6tables-restore /etc/sysconfig/ip6tables || ( /sbin/ip6tables-restore /etc/sysconfig/ip6tables.bak && exit 3 )',
         hasstatus  => true,
+        require    => File['/etc/init.d/ip6tables'],
         provider   => 'redhat'
       }
 
-      service { 'iptables-retry':
-        enable   => $_enable,
+      service { 'ip6tables-retry':
+        enable   => true,
+        require  => File['/etc/init.d/ip6tables-retry'],
         provider => 'redhat'
       }
+    }
 
-      if $ipv6 and $facts['ipv6_enabled'] {
-        service { 'ip6tables':
-          ensure     => $_ensure,
-          enable     => $_enable,
-          hasrestart => false,
-          restart    => '/sbin/ip6tables-restore /etc/sysconfig/ip6tables || ( /sbin/ip6tables-restore /etc/sysconfig/ip6tables.bak && exit 3 )',
-          hasstatus  => true,
-          require    => File['/etc/init.d/ip6tables'],
-          provider   => 'redhat'
-        }
-
-        service { 'ip6tables-retry':
-          enable   => true,
-          require  => File['/etc/init.d/ip6tables-retry'],
-          provider => 'redhat'
-        }
-      }
-
-      # firewalld should be disabled
-      service{ 'firewalld':
-        ensure => 'stopped',
-        enable => false
-      }
+    # firewalld should be disabled
+    service{ 'firewalld':
+      ensure => 'stopped',
+      enable => false
+    }
 
-      exec { 'fully stop firewalld':
-        command => 'pkill firewalld',
-        onlyif  => 'pgrep firewalld',
-        path    => [
-          '/bin',
-          '/usr/bin'
-        ],
-        require => Service['firewalld']
-      }
+    exec { 'fully stop firewalld':
+      command => 'pkill firewalld',
+      onlyif  => 'pgrep firewalld',
+      path    => [
+        '/bin',
+        '/usr/bin'
+      ],
+      require => Service['firewalld']
     }
   }
 }
diff --git a/metadata.json b/metadata.json
@@ -1,6 +1,6 @@
 {
   "name": "simp-iptables",
-  "version": "6.5.3",
+  "version": "6.5.4",
   "author": "SIMP Team",
   "summary": "Safely manages IPTables firewall rules",
   "license": "Apache-2.0",

diff --git a/spec/acceptance/nodesets/default.yml b/spec/acceptance/nodesets/default.yml
@@ -8,7 +8,6 @@
 HOSTS:
   el7:
     roles:
-      - server7
       - default
       - iptables
       - firewalld
@@ -18,16 +17,13 @@ HOSTS:
 
   el6:
     roles:
-      - server6
       - iptables
-      - firewalld
     platform: el-6-x86_64
     box: centos/6
     hypervisor: <%= hypervisor %>
 
   el8:
     roles:
-      - server8
       - firewalld
     platform: el-8-x86_64
     box: generic/centos8

diff --git a/spec/acceptance/nodesets/oel.yml b/spec/acceptance/nodesets/oel.yml
@@ -8,22 +8,23 @@
 HOSTS:
   oel7:
     roles:
-      - server7
       - default
+      - iptables
+      - firewalld
     platform: el-7-x86_64
     box: generic/oracle7
     hypervisor: <%= hypervisor %>
 
   oel6:
     roles:
-      - server6
+      - iptables
     platform: el-6-x86_64
     box: onyxpoint/oel-6-x86_64
     hypervisor: <%= hypervisor %>
 
   oel8:
     roles:
-      - server8
+      - firewalld
     platform: el-8-x86_64
     box: generic/oracle8
     hypervisor: <%= hypervisor %>

diff --git a/spec/acceptance/nodesets/rhel7.yml b/spec/acceptance/nodesets/rhel7.yml
@@ -8,10 +8,9 @@
 HOSTS:
   server-el7:
     roles:
-      - server
       - default
-      - master
-      - simp_server
+      - iptables
+      - firewalld
     platform:   el-7-x86_64
     box:        generic/rhel7
     hypervisor: <%= hypervisor %>