(#171) Added ensure parameter to firewall class

This change adds the ensure parameter to the firewall class so that
the appropriate iptables service for the operating system can be
stopped. The reason for this change is to extend the class so that
it is not just limited to running the service.

The change to the manifests still made all the rspec tests pass,
so it should not introduce any regressions. More rspec tests were
also added to exercise the new code paths introduced.

This pull request fixes issue #171.

diff --git a/manifests/init.pp b/manifests/init.pp
index 9730d3b8eab91dfaf21424d5f95d17513e177d8a..a1a65c6822f406267bbe7bb6d2e6b22880c81644 100644 (file)
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -3,10 +3,23 @@
 # Manages the installation of packages for operating systems that are
 # currently supported by the firewall type.
 #
-class firewall {
+class firewall (
+  $ensure = running
+) {
+  case $ensure {
+    /^(running|stopped)$/: {
+      # Do nothing.
+    }
+    default: {
+      fail("${title}: Ensure value '${ensure}' is not supported")
+    }
+  }
+
   case $::kernel {
     'Linux': {
-      class { "${title}::linux": }
+      class { "${title}::linux":
+        ensure => $ensure,
+      }
     }
     default: {
       fail("${title}: Kernel '${::kernel}' is not currently supported")

diff --git a/manifests/linux.pp b/manifests/linux.pp
index 92a054158ca6f403ae435569f5e50e10752b36de..13ad762bd3ed2e9cb4aa99a224be817caf53cab6 100644 (file)
--- a/manifests/linux.pp
+++ b/manifests/linux.pp
@@ -1,4 +1,11 @@
-class firewall::linux {
+class firewall::linux (
+  $ensure = running
+) {
+  $enable = $ensure ? {
+    running => true,
+    stopped => false,
+  }
+
   package { 'iptables':
     ensure => present,
   }
@@ -6,16 +13,22 @@ class firewall::linux {
   case $::operatingsystem {
     'RedHat', 'CentOS', 'Fedora': {
       class { "${title}::redhat":
+        ensure  => $ensure,
+        enable  => $enable,
         require => Package['iptables'],
       }
     }
     'Debian', 'Ubuntu': {
       class { "${title}::debian":
+        ensure  => $ensure,
+        enable  => $enable,
         require => Package['iptables'],
       }
     }
     'Archlinux': {
       class { "${title}::archlinux":
+        ensure  => $ensure,
+        enable  => $enable,
         require => Package['iptables'],
       }
     }

diff --git a/manifests/linux/archlinux.pp b/manifests/linux/archlinux.pp
index 9c04c2eb9f0681026bcff3d4806d04cb813118bb..1ed11602d25affafd9fefa858caa6ab9a2b300d8 100644 (file)
--- a/manifests/linux/archlinux.pp
+++ b/manifests/linux/archlinux.pp
@@ -1,12 +1,15 @@
-class firewall::linux::archlinux {
+class firewall::linux::archlinux (
+  $ensure = 'running',
+  $enable = true
+) {
   service { 'iptables':
-    ensure => running,
-    enable => true,
+    ensure => $ensure,
+    enable => $enable,
   }
 
   service { 'ip6tables':
-    ensure => running,
-    enable => true,
+    ensure => $ensure,
+    enable => $enable,
   }
 
   file { '/etc/iptables/iptables.rules':

diff --git a/manifests/linux/debian.pp b/manifests/linux/debian.pp
index 4a2242bce88c39a2a4c0c22fadd4738e2c08d7cc..49769e662b3c8040e0249de55a6f39c5a1669721 100644 (file)
--- a/manifests/linux/debian.pp
+++ b/manifests/linux/debian.pp
@@ -1,4 +1,7 @@
-class firewall::linux::debian {
+class firewall::linux::debian (
+  $ensure = running,
+  $enable = true
+) {
   package { 'iptables-persistent':
     ensure => present,
   }
@@ -7,7 +10,7 @@ class firewall::linux::debian {
   # needs to be called on system boot.
   service { 'iptables-persistent':
     ensure  => undef,
-    enable  => true,
+    enable  => $enable,
     require => Package['iptables-persistent'],
   }
 }

diff --git a/manifests/linux/redhat.pp b/manifests/linux/redhat.pp
index e89feca7d47a9cd0fc908b4fb5d1c7ea772854ef..c7dcb947d1fda2ff19d27dccbeda7626e23c4279 100644 (file)
--- a/manifests/linux/redhat.pp
+++ b/manifests/linux/redhat.pp
@@ -1,6 +1,9 @@
-class firewall::linux::redhat {
+class firewall::linux::redhat (
+  $ensure = running,
+  $enable = true
+) {
   service { 'iptables':
-    ensure => running,
-    enable => true,
+    ensure => $ensure,
+    enable => $enable,
   }
 }

diff --git a/spec/unit/classes/firewall_linux_archlinux_spec.rb b/spec/unit/classes/firewall_linux_archlinux_spec.rb
index 30c09896bae072f026bad6c2000923b5e7c50bfe..954d9ee10d75141ac7cfe7b069d1d19495ddb05e 100644 (file)
--- a/spec/unit/classes/firewall_linux_archlinux_spec.rb
+++ b/spec/unit/classes/firewall_linux_archlinux_spec.rb
@@ -9,4 +9,24 @@ describe 'firewall::linux::archlinux', :type => :class do
     :ensure   => 'running',
     :enable   => 'true'
   )}
+
+  context 'ensure => stopped' do
+    let(:params) {{ :ensure => 'stopped' }}
+    it { should contain_service('iptables').with(
+      :ensure   => 'stopped'
+    )}
+    it { should contain_service('ip6tables').with(
+      :ensure   => 'stopped'
+    )}
+  end
+
+  context 'enable => false' do
+    let(:params) {{ :enable => 'false' }}
+    it { should contain_service('iptables').with(
+      :enable   => 'false'
+    )}
+    it { should contain_service('ip6tables').with(
+      :enable   => 'false'
+    )}
+  end
 end

diff --git a/spec/unit/classes/firewall_linux_debian_spec.rb b/spec/unit/classes/firewall_linux_debian_spec.rb
index ed468f55ab4205b9511b854045ae8e6041794fc4..98285b642cab22da0a89584f133ca8e674f23b35 100644 (file)
--- a/spec/unit/classes/firewall_linux_debian_spec.rb
+++ b/spec/unit/classes/firewall_linux_debian_spec.rb
@@ -9,4 +9,11 @@ describe 'firewall::linux::debian', :type => :class do
     :enable   => 'true',
     :require  => 'Package[iptables-persistent]'
   )}
+
+  context 'enable => false' do
+    let(:params) {{ :enable => 'false' }}
+    it { should contain_service('iptables-persistent').with(
+      :enable   => 'false'
+    )}
+  end
 end

diff --git a/spec/unit/classes/firewall_linux_redhat_spec.rb b/spec/unit/classes/firewall_linux_redhat_spec.rb
index f61e7815a9dd9efd1883327163ee9c91b93cb8de..ea49d2b83b8be514fb7bb01116e7374681353236 100644 (file)
--- a/spec/unit/classes/firewall_linux_redhat_spec.rb
+++ b/spec/unit/classes/firewall_linux_redhat_spec.rb
@@ -5,4 +5,18 @@ describe 'firewall::linux::redhat', :type => :class do
     :ensure => 'running',
     :enable => 'true'
   )}
+
+  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

diff --git a/spec/unit/classes/firewall_spec.rb b/spec/unit/classes/firewall_spec.rb
index 8d1b7c25cca239d44c75145346ef931957f98f10..87c3f2b8f4530bbc386307b6ce8f7e5eee5d5a3b 100644 (file)
--- a/spec/unit/classes/firewall_spec.rb
+++ b/spec/unit/classes/firewall_spec.rb
@@ -3,6 +3,23 @@ require 'spec_helper'
 describe 'firewall', :type => :class do
   context 'kernel => Linux' do
     let(:facts) {{ :kernel => 'Linux' }}
-    it { should include_class('firewall::linux') }
+    it { should contain_class('firewall::linux').with_ensure('running') }
+  end
+
+  context 'kernel => Windows' do
+    let(:facts) {{ :kernel => 'Windows' }}
+    it { expect { should include_class('firewall::linux') }.to raise_error(Puppet::Error) }
+  end
+
+  context 'ensure => stopped' do
+    let(:facts) {{ :kernel => 'Linux' }}
+    let(:params) {{ :ensure => 'stopped' }}
+    it { should contain_class('firewall::linux').with_ensure('stopped') }
+  end
+
+  context 'ensure => test' do
+    let(:facts) {{ :kernel => 'Linux' }}
+    let(:params) {{ :ensure => 'test' }}
+    it { expect { should include_class('firewall::linux') }.to raise_error(Puppet::Error) }
   end
 end