]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(GH-134) Autorequire iptables related packages
authorDan Carley <dan.carley@gmail.com>
Fri, 1 Mar 2013 18:55:32 +0000 (18:55 +0000)
committerDan Carley <dan.carley@gmail.com>
Fri, 1 Mar 2013 19:35:28 +0000 (19:35 +0000)
autorequires from firewall and firewallchain resources to iptables and
iptables-persistent packages, when the appropriate provider is selected and
the packages are managed in the catalog. This will prevent failed rule
creation and persistence on fresh nodes where the packages may not be
pre-installed.

lib/puppet/type/firewall.rb
lib/puppet/type/firewallchain.rb
spec/unit/puppet/type/firewall_spec.rb
spec/unit/puppet/type/firewallchain_spec.rb

index ce0f377f2eea7d123cabc9a77b337abe9d2e34d3..4f9ee4f27072692bf1d1d10fbde768a319d2bd77 100644 (file)
@@ -16,9 +16,16 @@ Puppet::Type.newtype(:firewall) do
     This type provides the capability to manage firewall rules within
     puppet.
 
-    **Autorequires:** If Puppet is managing the iptables or ip6tables chains
-    specified in the `chain` or `jump` parameters, the firewall resource
-    will autorequire those firewallchain resources.
+    **Autorequires:**
+
+    If Puppet is managing the iptables or ip6tables chains specified in the
+    `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
+    autorequire those packages to ensure that any required binaries are
+    installed.
   EOS
 
   feature :rate_limiting, "Rate limiting features."
@@ -569,6 +576,17 @@ Puppet::Type.newtype(:firewall) do
     reqs
   end
 
+  # Classes would be a better abstraction, pending:
+  # http://projects.puppetlabs.com/issues/19001
+  autorequire(:package) do
+    case value(:provider)
+    when :iptables, :ip6tables
+      %w{iptables iptables-persistent}
+    else
+      []
+    end
+  end
+
   validate do
     debug("[validate]")
 
index 7eade4bc8eedd1dbac659a81e5268d1742933244..2ed1e5b1eb003d2ffc858043708c1891cf7c65ff 100644 (file)
@@ -16,6 +16,11 @@ Puppet::Type.newtype(:firewallchain) do
     Currently this supports only iptables, ip6tables and ebtables on Linux. And
     provides support for setting the default policy on chains and tables that
     allow it.
+
+    **Autorequires:**
+    If Puppet is managing the iptables or iptables-persistent packages, and
+    the provider is iptables_chain, the firewall resource will autorequire
+    those packages to ensure that any required binaries are installed.
   EOS
 
   feature :iptables_chain, "The provider provides iptables chain features."
@@ -100,6 +105,17 @@ Puppet::Type.newtype(:firewallchain) do
     end
   end
 
+  # Classes would be a better abstraction, pending:
+  # http://projects.puppetlabs.com/issues/19001
+  autorequire(:package) do
+    case value(:provider)
+    when :iptables_chain
+      %w{iptables iptables-persistent}
+    else
+      []
+    end
+  end
+
   validate do
     debug("[validate]")
 
index d0a02a386ac2c58056e8fe1f74033a378d75639a..4ed4dcb9581ed560f57184ad3c3b1a03ceac4e85 100755 (executable)
@@ -496,4 +496,38 @@ describe firewall do
       lambda { @resource[:pkttype] = 'not valid' }.should raise_error(Puppet::Error)
     end
   end
+
+  describe 'autorequire packages' do
+    [:iptables, :ip6tables].each do |provider|
+      it "provider #{provider} should autorequire package iptables" do
+        @resource[:provider] = provider
+        @resource[:provider].should == provider
+        package = Puppet::Type.type(:package).new(:name => 'iptables')
+        catalog = Puppet::Resource::Catalog.new
+        catalog.add_resource @resource
+        catalog.add_resource package
+        rel = @resource.autorequire[0]
+        rel.source.ref.should == package.ref
+        rel.target.ref.should == @resource.ref
+      end
+
+      it "provider #{provider} should autorequire packages iptables and iptables-persistent" do
+        @resource[:provider] = provider
+        @resource[:provider].should == provider
+        packages = [
+          Puppet::Type.type(:package).new(:name => 'iptables'),
+          Puppet::Type.type(:package).new(:name => 'iptables-persistent')
+        ]
+        catalog = Puppet::Resource::Catalog.new
+        catalog.add_resource @resource
+        packages.each do |package|
+          catalog.add_resource package
+        end
+        packages.zip(@resource.autorequire) do |package, rel|
+          rel.source.ref.should == package.ref
+          rel.target.ref.should == @resource.ref
+        end
+      end
+    end
+  end
 end
index e9679275bb9019d64a7a002eef896109c120c9aa..de0035ea43669071b7dd6d107295d9273a280afe 100755 (executable)
@@ -104,4 +104,33 @@ describe firewallchain do
 
   end
 
+  describe 'autorequire packages' do
+    it "provider iptables_chain should autorequire package iptables" do
+      resource[:provider].should == :iptables_chain
+      package = Puppet::Type.type(:package).new(:name => 'iptables')
+      catalog = Puppet::Resource::Catalog.new
+      catalog.add_resource resource
+      catalog.add_resource package
+      rel = resource.autorequire[0]
+      rel.source.ref.should == package.ref
+      rel.target.ref.should == resource.ref
+    end
+
+    it "provider iptables_chain should autorequire packages iptables and iptables-persistent" do
+      resource[:provider].should == :iptables_chain
+      packages = [
+        Puppet::Type.type(:package).new(:name => 'iptables'),
+        Puppet::Type.type(:package).new(:name => 'iptables-persistent')
+      ]
+      catalog = Puppet::Resource::Catalog.new
+      catalog.add_resource resource
+      packages.each do |package|
+        catalog.add_resource package
+      end
+      packages.zip(resource.autorequire) do |package, rel|
+        rel.source.ref.should == package.ref
+        rel.target.ref.should == resource.ref
+      end
+    end
+  end
 end