From 3026ccb00df6d6cdcc700484d21c565cb525afc3 Mon Sep 17 00:00:00 2001
From: Hunter Haugen <hunter@puppetlabs.com>
Date: Tue, 28 Jan 2014 18:08:42 -0800
Subject: [PATCH] (MODULES-441) Helpfully fail when modifying chains

It is not intended for chains to be modified using the firewall
resource, but it would still try and result in obscure incorrect errors.
This raises a more helpful error
---
 lib/puppet/provider/firewall/iptables.rb   | 12 +++++++++--
 spec/acceptance/params_spec.rb             | 25 ++++++++++++++++++++++
 spec/unit/puppet/provider/iptables_spec.rb |  4 ++++
 spec/unit/puppet/util/firewall_spec.rb     |  2 +-
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 8c07a62..0163059 100644
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -92,8 +92,16 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
       @property_hash[property.to_sym]
     end
 
-    define_method "#{property}=" do |value|
-      @property_hash[:needs_change] = true
+    if property == :chain
+      define_method "#{property}=" do |value|
+        if @property_hash[:chain] != value
+          raise ArgumentError, "Modifying the chain for existing rules is not supported."
+        end
+      end
+    else
+      define_method "#{property}=" do |value|
+        @property_hash[:needs_change] = true
+      end
     end
   end
 
diff --git a/spec/acceptance/params_spec.rb b/spec/acceptance/params_spec.rb
index 916cc1a..261b477 100644
--- a/spec/acceptance/params_spec.rb
+++ b/spec/acceptance/params_spec.rb
@@ -85,6 +85,31 @@ firewall { '#{name}':
     expect(apply_manifest(ppm, :catch_failures => true).exit_code).to eq(2)
   end
 
+  it 'test chain - changing names' do
+    iptables_flush_all_tables
+
+    ppm1 = pp({
+      'name'  => '004 with a chain',
+      'chain' => 'INPUT',
+      'proto' => 'all',
+    })
+
+    ppm2 = pp({
+      'name'  => '004 with a chain',
+      'chain' => 'OUTPUT',
+      'proto' => 'all',
+    })
+
+    apply_manifest(ppm1, :expect_changes => true)
+
+    ppm = <<-EOS + "\n" + ppm2
+      resources { 'firewall':
+        purge => true,
+      }
+    EOS
+    expect(apply_manifest(ppm2, :expect_failures => true).stderr).to match(/is not supported/)
+  end
+
   it 'test log rule - idempotent' do
     iptables_flush_all_tables
 
diff --git a/spec/unit/puppet/provider/iptables_spec.rb b/spec/unit/puppet/provider/iptables_spec.rb
index d4dcea5..0ff81e3 100644
--- a/spec/unit/puppet/provider/iptables_spec.rb
+++ b/spec/unit/puppet/provider/iptables_spec.rb
@@ -180,6 +180,10 @@ describe 'iptables provider' do
     it 'update_args should be an array' do
       expect(instance.update_args.class).to eq(Array)
     end
+
+    it 'fails when modifying the chain' do
+      expect { instance.chain = "OUTPUT" }.to raise_error(/is not supported/)
+    end
   end
 
   describe 'when deleting resources' do
diff --git a/spec/unit/puppet/util/firewall_spec.rb b/spec/unit/puppet/util/firewall_spec.rb
index ef16be1..2fbfabd 100644
--- a/spec/unit/puppet/util/firewall_spec.rb
+++ b/spec/unit/puppet/util/firewall_spec.rb
@@ -28,7 +28,7 @@ describe 'Puppet::Util::Firewall' do
   describe '#host_to_mask' do
     subject { resource }
     specify {
-      expect(Resolv).to receive(:getaddress).any_number_of_times.with('puppetlabs.com').and_return('96.126.112.51')
+      expect(Resolv).to receive(:getaddress).at_least(:once).with('puppetlabs.com').and_return('96.126.112.51')
       subject.host_to_mask('puppetlabs.com').should == '96.126.112.51/32'
       subject.host_to_mask('!puppetlabs.com').should == '! 96.126.112.51/32'
     }
-- 
2.45.2