(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

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 8c07a62df49a1bb92cb250a81aed4b38997d78c4..016305973ec72746e90872f465c7282ea18d647d 100644 (file)
--- 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 916cc1a9dd4f50141b1a772684292e0f47754ed4..261b4770e69bc4cad970a7ecdc3f4d25d0c6e5d0 100644 (file)
--- 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 d4dcea57cae33132ded9cd0d3b8ac1a37ff0b6e5..0ff81e3407951711f190fa28c3ae4a6c165d1922 100644 (file)
--- 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 ef16be161391b2ff1ec89a09c094d93294922d94..2fbfabd070a037385ef37e34f9b7d52da5afa3e9 100644 (file)
--- 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'
     }