From e39fbdcb12e5fe1682528d397027925100531e5a Mon Sep 17 00:00:00 2001 From: Ken Barber Date: Mon, 12 Mar 2012 11:40:45 -0700 Subject: [PATCH] (#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. --- README.markdown | 3 ++- .../provider/firewallchain/iptables_chain.rb | 12 ++++++------ lib/puppet/type/firewallchain.rb | 10 +++++----- spec/unit/puppet/provider/iptables_chain_spec.rb | 2 +- spec/unit/puppet/type/firewallchain_spec.rb | 14 +++++++------- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/README.markdown b/README.markdown index 04c8865..b80d292 100644 --- 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 845792a..758d37c 100644 --- 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 830c68a..0bab44a 100644 --- 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 143e1e6..f8c013c 100755 --- 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 2bb031f..cbd3df1 100755 --- 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 -- 2.45.2