From: Ken Barber <ken@bob.sh>
Date: Sun, 18 Sep 2011 21:28:11 +0000 (+0100)
Subject: (#9583) Fix provider detection for gentoo and unsupported linuxes for the iptables... 
X-Git-Tag: v0.0.1~4^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4ea078e3f3d599b732195a1b13eff19ad9aa9998;p=puppet-modules%2Fpuppetlabs-firewall.git

(#9583) Fix provider detection for gentoo and unsupported linuxes for the iptables provider.

Previously we had fairly specific confine settings for named distributions
of linux for the iptables provider. This was silly, since the commands
defined in the provider should be enough to confine the provider to Linux
only systems.

I've removed the confine, and replaced the defaultfor to be:

  :kernel => :linux

Which should avoid the need to keep adding extra Linux distributions.

I've also added some spec tests for provider detection which should help
catch any failures around the command based detection in the future.
---

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index e941090..a6efda8 100644
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -19,8 +19,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
   commands :iptables => '/sbin/iptables'
   commands :iptables_save => '/sbin/iptables-save'
 
-  defaultfor :operatingsystem => [:redhat, :debian, :ubuntu, :fedora, :suse, :centos, :sles, :oel, :ovm]
-  confine :operatingsystem => [:redhat, :debian, :ubuntu, :fedora, :suse, :centos, :sles, :oel, :ovm]
+  defaultfor :kernel => :linux
 
   @resource_map = {
     :burst => "--limit-burst",
diff --git a/spec/unit/provider/iptables_prov_spec.rb b/spec/unit/provider/iptables_prov_spec.rb
index 3b32132..aef682e 100644
--- a/spec/unit/provider/iptables_prov_spec.rb
+++ b/spec/unit/provider/iptables_prov_spec.rb
@@ -1,5 +1,46 @@
 require 'spec_helper'
 
+describe 'iptables provider detection' do
+  before :each do
+    require 'puppet/provider/confine/exists'
+    @exists = Puppet::Provider::Confine::Exists
+
+    # Reset the default provider
+    Puppet::Type.type(:firewall).defaultprovider = nil
+  end
+
+  it "should default to iptables provider if /sbin/iptables[-save] exists" do
+    # Stub lookup for /sbin/iptables & /sbin/iptables-save
+    @exists.any_instance.stubs(:which).with("/sbin/iptables").
+      returns "/sbin/iptables"
+    @exists.any_instance.stubs(:which).with("/sbin/iptables-save").
+      returns "/sbin/iptables-save"
+
+    # Every other command should return false so we don't pick up any
+    # other providers
+    @exists.any_instance.stubs(:which).with() { |value|
+      ! ["/sbin/iptables","/sbin/iptables-save"].include?(value)
+    }.returns false
+
+    # Create a resource instance and make sure the provider is iptables
+    resource = Puppet::Type.type(:firewall).new({
+      :name => '000 test foo',
+    })
+    resource.provider.class.to_s.should == "Puppet::Type::Firewall::ProviderIptables"
+  end
+
+  it "should raise a default provider error when there are no commands" do
+    # Stub all commands lookups so they return nothing 
+    @exists.any_instance.stubs(:which).returns false
+
+    # Instantiate a resource instance and make sure it raises an exception
+    lambda { resource = Puppet::Type.type(:firewall).new({ 
+      :name => '000 test foo' }) }.should raise_error(Puppet::DevError, 
+      "Could not find a default provider for firewall")
+  end
+
+end
+
 describe 'iptables provider' do
   before :each do
     @provider = Puppet::Type.type(:firewall).provider(:iptables)