This was previously accomplished b retrieving the full list of rules each time a rule was set in order to check it was unique. This was to allow the user to choose the response to a duplicate rule being found.
However this caused a massive slowdown within certain module runs and as such we have changed the the location of the check, so that it instead runs a check for any duplicates when retrieving the current rules prior to any updates being made.
As an effect of this the user is now unable to choose the response to a duplicate rule being found, however wee feel that this is a fair tradeoff for the increased speed and that the response that we have chosen is the correct one.
It is possible for an unmanaged rule to exist on the target system that has the same comment as the rule specified in the manifest. This configuration is not supported by the firewall module.
-In the event of a duplicate rule, the module will by default display a warning message notifying the user that it has found a duplicate but will continue to update the resource.
-
-This behaviour is configurable via the `onduplicaterulebehaviour` parameter. Users can choose from the following behaviours:
-
-* `ignore` - The duplicate rule is ignored and any updates to the resource will continue unaffected.
-* `warn` - The duplicate rule is logged as a warning and any updates to the resource will continue unaffected.
-* `error` - The duplicate rule is logged as an error and any updates to the resource will be skipped.
-
-With either the `ignore` or `warn` (default) behaviour, Puppet may create another duplicate rule.
-To prevent this behavior and report the resource as failing during the Puppet run, specify the `error` behaviour.
+In the event of a duplicate rule, the module will throw an error message notifying the user that it has found a duplicate and halt in it's update.
+This behaviour was previously configurable via the `onduplicaterulebehaviour` parameter. However the implementation of this resulted in a massive slowdown of the module runs and so this has been removed infavour of a simple error being thrown whenever a duplicate is detected.
### Additional information
Access the inline documentation:
* [`line`](#-firewall--line)
* [`name`](#-firewall--name)
-* [`onduplicaterulebehaviour`](#-firewall--onduplicaterulebehaviour)
* [`provider`](#-firewall--provider)
##### <a name="-firewall--line"></a>`line`
Depending on the provider, the name of the rule can be stored using
the comment feature of the underlying firewall subsystem.
-##### <a name="-firewall--onduplicaterulebehaviour"></a>`onduplicaterulebehaviour`
-
-Valid values: `ignore`, `warn`, `error`
-
-In certain situations it is possible for an unmanaged rule to exist
-on the target system that has the same comment as the rule
-specified in the manifest.
-
-This setting determines what happens when such a duplicate is found.
-
-It offers three options:
-
- * ignore - The duplicate rule is ignored and any updates to the resource will continue unaffected.
- * warn - The duplicate rule is logged as a warning and any updates to the resource will continue unaffected.
- * error - The duplicate rule is logged as an error and any updates to the resource will be skipped.
-
-Default value: `warn`
-
##### <a name="-firewall--provider"></a>`provider`
The specific backend to use for this `firewall` resource. You will seldom need to specify this --- Puppet will usually
end
def exists?
- if duplicate_rule?(@property_hash[:name])
- core_message = "Duplicate rule found for #{@property_hash[:name]}."
- case @resource.parameters[:onduplicaterulebehaviour].value
- when :error
- raise "#{core_message} Skipping update."
- when :warn
- warning "#{core_message}. This may add an additional rule to the system."
- when :ignore
- debug "#{core_message}. This may add an additional rule to the system."
- end
- end
-
properties[:ensure] != :absent
end
- def duplicate_rule?(rule)
- rules = self.class.instances
- system_rule_count = Hash.new(0)
- rules.each { |r| system_rule_count[r.name] += 1 }
- duplicate_rules = rules.select { |r| system_rule_count[r.name] > 1 }
- duplicate_rules.select { |r| r.name == rule }.any?
- end
-
# Flush the property hash once done.
def flush
debug('[flush]')
debug '[instances]'
table = nil
rules = []
+ # Used for detecting duplicate rules
+ duplicate_rules = []
counter = 1
# String#lines would be nice, but we need to support Ruby 1.8.5
else
hash = rule_to_hash(line, table, counter)
if hash
+ raise "Duplicate rule found for #{hash[:name]}. Skipping update." if duplicate_rules.include?(hash[:name])
+ duplicate_rules << hash[:name]
rules << new(hash)
counter += 1
end
newvalues(%r{^\d+[[:graph:][:space:]]+$})
end
- newparam(:onduplicaterulebehaviour) do
- desc <<-PUPPETCODE
- In certain situations it is possible for an unmanaged rule to exist
- on the target system that has the same comment as the rule
- specified in the manifest.
-
- This setting determines what happens when such a duplicate is found.
-
- It offers three options:
-
- * ignore - The duplicate rule is ignored and any updates to the resource will continue unaffected.
- * warn - The duplicate rule is logged as a warning and any updates to the resource will continue unaffected.
- * error - The duplicate rule is logged as an error and any updates to the resource will be skipped.
- PUPPETCODE
- newvalues(:ignore, :warn, :error)
- defaultto :warn
- end
-
newproperty(:action) do
desc <<-PUPPETCODE
This is the action to perform on a match. Can be one of:
require 'spec_helper_acceptance'
-def make_manifest(behaviour)
- pp = <<-PUPPETCODE
+describe 'firewall - duplicate comments' do
+ before(:all) do
+ if os[:family] == 'ubuntu' || os[:family] == 'debian'
+ update_profile_file
+ end
+ end
+
+ after(:each) do
+ iptables_flush_all_tables
+ end
+
+ context 'when a duplicate comment is found' do
+ pp = <<-PUPPETCODE
class { 'firewall': }
resources { 'firewall':
purge => true,
dport => '550',
action => accept,
destination => '192.168.2.0/24',
- onduplicaterulebehaviour => #{behaviour}
}
PUPPETCODE
- pp
-end
-
-describe 'firewall - duplicate comments' do
- before(:all) do
- if os[:family] == 'ubuntu' || os[:family] == 'debian'
- update_profile_file
- end
- end
-
- before(:each) do
- run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 551 -j ACCEPT -m comment --comment "550 destination"')
- end
-
- after(:each) do
- iptables_flush_all_tables
- end
-
- context 'when onduplicateerrorhevent is set to error' do
it 'raises an error' do
run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 551 -j ACCEPT -m comment --comment "550 destination"')
- pp = make_manifest('error')
-
- apply_manifest(pp) do |r|
- expect(r.stderr).to include('Error: /Stage[main]/Main/Firewall[550 destination]: Could not evaluate: Duplicate rule found for 550 destination. Skipping update.')
- end
- end
- end
-
- context 'when onduplicateerrorhevent is set to warn' do
- run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 551 -j ACCEPT -m comment --comment "550 destination"')
-
- it 'warns and continues' do
- run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 551 -j ACCEPT -m comment --comment "550 destination"')
- pp = make_manifest('warn')
-
- apply_manifest(pp) do |r|
- expect(r.stderr).to include('Warning: Firewall[550 destination](provider=iptables): Duplicate rule found for 550 destination.. This may add an additional rule to the system.')
- end
- end
- end
-
- context 'when onduplicateerrorhevent is set to ignore' do
- run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 551 -j ACCEPT -m comment --comment "550 destination"')
-
- it 'continues silently' do
- run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 551 -j ACCEPT -m comment --comment "550 destination"')
- pp = make_manifest('ignore')
+ run_shell('iptables -I INPUT -m state --state NEW -m tcp -p tcp --dport 552 -j ACCEPT -m comment --comment "550 destination"')
apply_manifest(pp) do |r|
- expect(r.stderr).to be_empty
+ expect(r.stderr).to include('Duplicate rule found for 550 destination. Skipping update.')
end
end
end
Code Review / puppet-modules / puppetlabs-firewall.git / commitdiff