Browse files

Merge branch 'ticket/9439-existing_rules'

* ticket/9439-existing_rules:
  (#9439) fix parsing and deleting existing rules

Reviewed-by: Ken Barber <ken@bob.sh>
  • Loading branch information...
2 parents 762f9bd + 960f430 commit c6807fae84e7bc32fff3613858482da61c64f9eb @kbarber kbarber committed Oct 11, 2011
View
39 lib/puppet/provider/firewall/iptables.rb
@@ -1,4 +1,5 @@
require 'puppet/provider/firewall'
+require 'digest/md5'
Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Firewall do
include Puppet::Util::Firewall
@@ -60,7 +61,7 @@ def update
def delete
debug 'Deleting rule %s' % resource[:name]
- iptables "-D", properties[:chain], insert_order
+ iptables delete_args
end
def exists?
@@ -119,15 +120,22 @@ def self.rule_to_hash(line, table, counter)
[:dport, :sport, :state].each do |prop|
hash[prop] = hash[prop].split(',') if ! hash[prop].nil?
end
-
+
+ # This forces all existing, commentless rules to be moved to the bottom of the stack.
+ # Puppet-firewall requires that all rules have comments (resource names) and will fail if
+ # a rule in iptables does not have a comment. We get around this by appending a high level
+ if ! hash[:name]
+ hash[:name] = "9999 #{Digest::MD5.hexdigest(line)}"
+ end
+
+ hash[:line] = line
hash[:provider] = self.name.to_s
hash[:table] = table
hash[:ensure] = :present
# Munge some vars here ...
# proto should equal 'all' if undefined
hash[:proto] = "all" if !hash.include?(:proto)
-
hash
end
@@ -145,6 +153,31 @@ def update_args
args
end
+ def delete_args
+ count = []
+ line = properties[:line].gsub(/\-A/, '-D').split
+
+ # Grab all comment indices
+ line.each do |v|
+ if v =~ /"/
+ count << line.index(v)
+ end
+ end
+
+ if ! count.empty?
+ # Remove quotes and set first comment index to full string
+ line[count.first] = line[count.first..count.last].join(' ').gsub(/"/, '')
+
+ # Make all remaining comment indices nil
+ ((count.first + 1)..count.last).each do |i|
+ line[i] = nil
+ end
+ end
+
+ # Return array without nils
+ line.compact
+ end
+
def general_args
debug "Current resource: %s" % resource.class
args = []
View
4 lib/puppet/type/firewall.rb
@@ -195,6 +195,10 @@ def should_to_s(value)
desc "Rate limiting burst value (per second)."
newvalue(/^\d+$/)
end
+
+ newparam(:line) do
+ desc 'Read-only property for caching the rule line'
+ end
validate do
debug("[validate]")
View
2 spec/spec_helper.rb
@@ -1,8 +1,6 @@
dir = File.expand_path(File.dirname(__FILE__))
$LOAD_PATH.unshift File.join(dir, 'lib')
-p dir
-
# Don't want puppet getting the command line arguments for rake or autotest
ARGV.clear
View
48 spec/unit/provider/iptables_prov_spec.rb
@@ -38,7 +38,6 @@
:name => '000 test foo' }) }.should raise_error(Puppet::DevError,
"Could not find a default provider for firewall")
end
-
end
describe 'iptables provider' do
@@ -81,14 +80,57 @@
end
end
- describe 'when modifying resources' do
+ describe 'when converting rules without comments to resources' do
+ before :each do
+ @rule = '-A INPUT -s 1.1.1.1 -d 1.1.1.1 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061, 7062 -j ACCEPT'
+ @resource = @provider.rule_to_hash(@rule, 'filter', 0)
+ @instance = @provider.new(@resource)
+ end
+
+ it 'rule name contains a MD5 sum of the line' do
+ @resource[:name].should == "9999 #{Digest::MD5.hexdigest(@resource[:line])}"
+ end
+ end
+
+ describe 'when creating resources' do
before :each do
@instance = @provider.new(@resource)
@provider.expects(:execute).with(['/sbin/iptables-save']).returns("")
end
- it 'should do something' do
+ it 'insert_args should be an array' do
@instance.insert_args.class.should == Array
end
end
+
+ describe 'when modifying resources' do
+ before :each do
+ @instance = @provider.new(@resource)
+ @provider.expects(:execute).with(['/sbin/iptables-save']).returns ""
+ end
+
+ it 'update_args should be an array' do
+ @instance.update_args.class.should == Array
+ end
+ end
+
+ describe 'when deleting resources' do
+ before :each do
+ @rule = '-A INPUT -s 1.1.1.1 -d 1.1.1.1 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061, 7062 -j ACCEPT'
+ @resource = @provider.rule_to_hash(@rule, 'filter', 0)
+ @instance = @provider.new(@resource)
+ end
+
+ it 'resource[:line] looks like the original rule' do
+ @resource[:line] == @rule
+ end
+
+ it 'delete_args is an array' do
+ @instance.delete_args.class.should == Array
+ end
+
+ it 'delete_args is the same as the rule string when joined' do
+ @instance.delete_args.join(' ').should == @rule.gsub(/\-A/, '-D')
+ end
+ end
end

0 comments on commit c6807fa

Please sign in to comment.