(#10162) Modify firewallchain name to be chain:table:protocol

We've decided to change the ordering of the namevar so that it is now:

    chain:table:protocol

So its closer to a linear hierachy ie. chain in table in protocol.

Previously this was table:chain:protocol which made less sense.

diff --git a/README.markdown b/README.markdown
index 04c8865154e2779893681d1cbd5c83a315bc3d10..b80d292c205e4e35aae313eb486cb92ac51076e9 100644 (file)
--- a/README.markdown
+++ b/README.markdown
@@ -107,7 +107,8 @@ Creating a new rule that forwards to a chain, then adding a rule to this chain:
       jump    => 'MY_CHAIN',
       require => Firewallchain["filter:MY_CHAIN:IPv4"],
     }
-    firewallchain { 'filter:MY_CHAIN:IPv4':
+    # The namevar here is in the format chain_name:table:protocol
+    firewallchain { 'MY_CHAIN:filter:IPv4':
       ensure  => present,
     }
     firewall { '100 my rule':

diff --git a/lib/puppet/provider/firewallchain/iptables_chain.rb b/lib/puppet/provider/firewallchain/iptables_chain.rb
index 845792a5ad9b344a3e324fcb3ffbcb0eebee8395..758d37c49fbc2596014b3939ece81642e60a84d6 100644 (file)
--- a/lib/puppet/provider/firewallchain/iptables_chain.rb
+++ b/lib/puppet/provider/firewallchain/iptables_chain.rb
@@ -36,14 +36,14 @@ Puppet::Type.type(:firewallchain).provide :iptables_chain do
   }
   InternalChains = /^(PREROUTING|POSTROUTING|BROUTING|INPUT|FORWARD|OUTPUT)$/
   Tables = 'nat|mangle|filter|raw|rawpost|broute'
-  Nameformat = /^(#{Tables}):(.+):(IP(v[46])?|ethernet)$/
+  Nameformat = /^(.+):(#{Tables}):(IP(v[46])?|ethernet)$/
 
   def create
     # can't create internal chains
     if @resource[:name] =~ InternalChains
       self.warn "Attempting to create internal chain #{@resource[:name]}"
     end
-    allvalidchains do |t, table, chain, protocol|
+    allvalidchains do |t, chain, table, protocol|
       if properties[:ensure] == protocol
         debug "Skipping Inserting chain #{chain} on table #{table} (#{protocol}) already exists"
       else
@@ -129,7 +129,7 @@ Puppet::Type.type(:firewallchain).provide :iptables_chain do
       begin
         c[:save].call.each_line do |line|
           if line =~ c[:re] then
-            name = (table == 'filter' ? 'filter' : table) + ':' + $1 + ':' + p.to_s
+            name = $1 + ':' + (table == 'filter' ? 'filter' : table) + ':' + p.to_s
             policy = $2 == '-' ? nil : $2.downcase.to_sym
 
             chains << new({
@@ -155,10 +155,10 @@ Puppet::Type.type(:firewallchain).provide :iptables_chain do
 
   def allvalidchains
     @resource[:name].match(Nameformat)
-    table = $1
-    chain = $2
+    chain = $1
+    table = $2
     protocol = $3
-    yield Mapping[protocol.to_sym][:tables],table,chain,protocol.to_sym
+    yield Mapping[protocol.to_sym][:tables],chain,table,protocol.to_sym
   end
 
 end

diff --git a/lib/puppet/type/firewallchain.rb b/lib/puppet/type/firewallchain.rb
index 830c68abac976c31ca0be9c496de89b95be0981e..0bab44a924ccab04aedf50bb45d17fc82cdc5212 100644 (file)
--- a/lib/puppet/type/firewallchain.rb
+++ b/lib/puppet/type/firewallchain.rb
@@ -26,10 +26,10 @@ Puppet::Type.newtype(:firewallchain) do
 
     validate do |value|
       if value !~ Nameformat then
-        raise ArgumentError, "Inbuilt chains must be in the form {table}:{chain}:{protocol} where {table} is one of FILTER, NAT, MANGLE, RAW, RAWPOST, BROUTE or empty (alias for filter), chain can be anything without colons or one of PREROUTING, POSTROUTING, BROUTING, INPUT, FORWARD, OUTPUT for the inbuilt chains, and {protocol} being IPv4, IPv6, ethernet (ethernet bridging) got '#{value}' table:'#{$1}' chain:'#{$2}' protocol:'#{$3}'"
+        raise ArgumentError, "Inbuilt chains must be in the form {chain}:{table}:{protocol} where {table} is one of FILTER, NAT, MANGLE, RAW, RAWPOST, BROUTE or empty (alias for filter), chain can be anything without colons or one of PREROUTING, POSTROUTING, BROUTING, INPUT, FORWARD, OUTPUT for the inbuilt chains, and {protocol} being IPv4, IPv6, ethernet (ethernet bridging) got '#{value}' table:'#{$1}' chain:'#{$2}' protocol:'#{$3}'"
       else
-        table = $1
-        chain = $2
+        chain = $1
+        table = $2
         protocol = $3
         case table
         when 'filter'
@@ -94,8 +94,8 @@ Puppet::Type.newtype(:firewallchain) do
     debug("[validate]")
 
     value(:name).match(Nameformat)
-    table = $1
-    chain = $2
+    chain = $1
+    table = $2
     protocol = $3
 
     # Check that we're not removing an internal chain

diff --git a/spec/unit/puppet/provider/iptables_chain_spec.rb b/spec/unit/puppet/provider/iptables_chain_spec.rb
index 143e1e63b45bf5196afbb10c6ddc7adf2fc0d979..f8c013c73889cb79cd1e8f1043c078667f2453b4 100755 (executable)
--- a/spec/unit/puppet/provider/iptables_chain_spec.rb
+++ b/spec/unit/puppet/provider/iptables_chain_spec.rb
@@ -38,7 +38,7 @@ describe 'iptables chain provider detection' do
 
     # Create a resource instance and make sure the provider is iptables
     resource = Puppet::Type.type(:firewallchain).new({
-      :name => 'filter:test:IPv4',
+      :name => 'test:filter:IPv4',
     })
     resource.provider.class.to_s.should == "Puppet::Type::Firewallchain::ProviderIptables_chain"
   end

diff --git a/spec/unit/puppet/type/firewallchain_spec.rb b/spec/unit/puppet/type/firewallchain_spec.rb
index 2bb031f80e350e78008b9876ec73ebac1c6f9bdd..cbd3df19b269b3cbfdfb24fe2aa1d71dfefbbe34 100755 (executable)
--- a/spec/unit/puppet/type/firewallchain_spec.rb
+++ b/spec/unit/puppet/type/firewallchain_spec.rb
@@ -13,7 +13,7 @@ describe firewallchain do
   }
   let(:resource) {
     Puppet::Type::Firewallchain.stubs(:defaultprovider).returns provider
-    klass.new({:name => 'filter:INPUT:IPv4', :policy => :accept })
+    klass.new({:name => 'INPUT:filter:IPv4', :policy => :accept })
   }
 
   it 'should have :name be its namevar' do
@@ -29,7 +29,7 @@ describe firewallchain do
     }.each_pair do |table, allowedinternalchains|
       ['IPv4', 'IPv6', 'ethernet'].each do |protocol|
         [ 'test', '$5()*&%\'"^$09):' ].each do |chainname|
-          name = "#{table}:#{chainname}:#{protocol}"
+          name = "#{chainname}:#{table}:#{protocol}"
           if table == 'nat' && protocol == 'IPv6'
             it "should fail #{name}" do
               expect { resource[:name] = name }.to raise_error(Puppet::Error)
@@ -48,7 +48,7 @@ describe firewallchain do
       end # protocol
 
       [ 'PREROUTING', 'POSTROUTING', 'BROUTING', 'INPUT', 'FORWARD', 'OUTPUT' ].each do |internalchain|
-        name = table + ':' + internalchain + ':'
+        name = internalchain + ':' + table + ':'
         if internalchain == 'BROUTING'
           name += 'ethernet'
         elsif table == 'nat'
@@ -71,11 +71,11 @@ describe firewallchain do
     end # table, allowedinternalchainnames
 
     it 'should fail with invalid table names' do
-      expect { resource[:name] = 'wrongtablename:test:' }.to raise_error(Puppet::Error)
+      expect { resource[:name] = 'wrongtablename:test:IPv4' }.to raise_error(Puppet::Error)
     end
 
     it 'should fail with invalid protocols names' do
-      expect { resource[:name] = ':test:IPv5' }.to raise_error(Puppet::Error)
+      expect { resource[:name] = 'test:filter:IPv5' }.to raise_error(Puppet::Error)
     end
 
   end
@@ -95,10 +95,10 @@ describe firewallchain do
 
     [:accept, :drop, :queue, :return].each do |policy|
       it "non-inbuilt chains should not accept policy #{policy}" do
-        expect { klass.new({:name => 'filter:testchain:IPv4', :policy => policy }) }.to raise_error(Puppet::Error)
+        expect { klass.new({:name => 'testchain:filter:IPv4', :policy => policy }) }.to raise_error(Puppet::Error)
       end
       it "non-inbuilt chains can accept policies on protocol = ethernet (policy #{policy})" do
-        klass.new({:name => 'filter:testchain:ethernet', :policy => policy }).should be_instance_of(@provider)
+        klass.new({:name => 'testchain:filter:ethernet', :policy => policy }).should be_instance_of(@provider)
       end
     end