From e0f3746e4c8e9eb251a45dca68fae7d8db128266 Mon Sep 17 00:00:00 2001 From: Marc Tardif Date: Wed, 24 Apr 2013 14:46:26 -0400 Subject: [PATCH] (#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. --- manifests/init.pp | 17 ++++++++++++++-- manifests/linux.pp | 15 +++++++++++++- manifests/linux/archlinux.pp | 13 +++++++----- manifests/linux/debian.pp | 7 +++++-- manifests/linux/redhat.pp | 9 ++++++--- .../classes/firewall_linux_archlinux_spec.rb | 20 +++++++++++++++++++ .../classes/firewall_linux_debian_spec.rb | 7 +++++++ .../classes/firewall_linux_redhat_spec.rb | 14 +++++++++++++ spec/unit/classes/firewall_spec.rb | 19 +++++++++++++++++- 9 files changed, 107 insertions(+), 14 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 9730d3b..a1a65c6 100644 --- 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 92a0541..13ad762 100644 --- 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 9c04c2e..1ed1160 100644 --- 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 4a2242b..49769e6 100644 --- 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 e89feca..c7dcb94 100644 --- 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 30c0989..954d9ee 100644 --- 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 ed468f5..98285b6 100644 --- 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 f61e781..ea49d2b 100644 --- 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 8d1b7c2..87c3f2b 100644 --- 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 -- 2.45.2