Fix276

lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb

@@ -4,20 +4,31 @@
 Puppet::Type.type(:firewalld_direct_chain).provide(:firewall_cmd, parent: Puppet::Provider::Firewalld) do
   desc 'Provider for managing firewalld direct chains using firewall-cmd'
 
+  def initialize(value = {})
+    super(value)
+    @in_perm = false
+    @in_run = false
+  end
+
   def exists?
     @chain_args ||= generate_raw
-    output = execute_firewall_cmd(['--direct', '--query-chain', @chain_args], nil, true, false)
-    output.include?('yes')
+    @in_perm = execute_firewall_cmd(['--direct', '--query-chain', @chain_args], nil, true, false).include?('yes')
+    @in_run = execute_firewall_cmd(['--direct', '--query-chain', @chain_args], nil, false, false).include?('yes')
+    if @resource[:ensure] == :present
+      @in_perm && @in_run
+    else
+      @in_perm || @in_run
+    end
   end
 
   def create
     @chain_args ||= generate_raw
-    execute_firewall_cmd(['--direct', '--add-chain', @chain_args], nil)
+    execute_firewall_cmd(['--direct', '--add-chain', @chain_args], nil) unless @in_perm
   end
 
   def destroy
     @chain_args ||= generate_raw
-    execute_firewall_cmd(['--direct', '--remove-chain', @chain_args], nil)
+    execute_firewall_cmd(['--direct', '--remove-chain', @chain_args], nil) if @in_perm
   end
 
   def generate_raw

lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb

@@ -7,20 +7,31 @@
 ) do
   desc 'Interact with firewall-cmd'
 
+  def initialize(value = {})
+    super(value)
+    @in_perm = false
+    @in_run = false
+  end
+
   def exists?
     @passt_args ||= generate_raw
-    output = execute_firewall_cmd(['--direct', '--query-passthrough', @passt_args], nil, true, false)
-    output.include?('yes')
+    @in_perm = execute_firewall_cmd(['--direct', '--query-passthrough', @passt_args], nil, true, false).include?('yes')
+    @in_run = execute_firewall_cmd(['--direct', '--query-passthrough', @passt_args], nil, false, false).include?('yes')
+    if @resource[:ensure] == :present
+      @in_perm && @in_run
+    else
+      @in_perm || @in_run
+    end
   end
 
   def create
     @passt_args ||= generate_raw
-    execute_firewall_cmd(['--direct', '--add-passthrough', @passt_args], nil)
+    execute_firewall_cmd(['--direct', '--add-passthrough', @passt_args], nil) unless @in_perm
   end
 
   def destroy
     @passt_args ||= generate_raw
-    execute_firewall_cmd(['--direct', '--remove-passthrough', @passt_args], nil)
+    execute_firewall_cmd(['--direct', '--remove-passthrough', @passt_args], nil) if @in_perm
   end
 
   def generate_raw

lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb

@@ -7,20 +7,31 @@
 ) do
   desc 'Interact with firewall-cmd'
 
+  def initialize(value = {})
+    super(value)
+    @in_perm = false
+    @in_run = false
+  end
+
   def exists?
     @rule_args ||= generate_raw
-    output = execute_firewall_cmd(['--direct', '--query-rule', @rule_args], nil, true, false)
-    output.include?('yes')
+    @in_perm = execute_firewall_cmd(['--direct', '--query-rule', @rule_args], nil, true, false).include?('yes')
+    @in_run = execute_firewall_cmd(['--direct', '--query-rule', @rule_args], nil, false, false).include?('yes')
+    if @resource[:ensure] == :present
+      @in_perm && @in_run
+    else
+      @in_perm || @in_run
+    end
   end
 
   def create
     @rule_args ||= generate_raw
-    execute_firewall_cmd(['--direct', '--add-rule', @rule_args], nil)
+    execute_firewall_cmd(['--direct', '--add-rule', @rule_args], nil) unless @in_perm
   end
 
   def destroy
     @rule_args ||= generate_raw
-    execute_firewall_cmd(['--direct', '--remove-rule', @rule_args], nil)
+    execute_firewall_cmd(['--direct', '--remove-rule', @rule_args], nil) if @in_perm
   end
 
   def generate_raw

lib/puppet/provider/firewalld_ipset/firewall_cmd.rb

@@ -10,9 +10,9 @@
   mk_resource_methods
 
   def self.instances
-    ipset_ids = execute_firewall_cmd(['--get-ipsets'], nil).split(' ')
+    ipset_ids = execute_firewall_cmd(['--get-ipsets'], nil, false).split(' ')
     ipset_ids.map do |ipset_id|
-      ipset_raw = execute_firewall_cmd(["--info-ipset=#{ipset_id}"], nil)
+      ipset_raw = execute_firewall_cmd(["--info-ipset=#{ipset_id}"], nil, false)
       raw_options = ipset_raw.match(%r{options: (.*)})
       options = {}
       if raw_options
@@ -44,20 +44,23 @@ def exists?
   end
 
   def create
-    args = []
-    args << ["--new-ipset=#{@resource[:name]}"]
-    args << ["--type=#{@resource[:type]}"]
-    options = {
-      family: @resource[:family],
-      hashsize: @resource[:hashsize],
-      maxelem: @resource[:maxelem],
-      timeout: @resource[:timeout]
-    }
-    options = options.merge(@resource[:options]) if @resource[:options]
-    options.each do |option_name, value|
-      args << ["--option=#{option_name}=#{value}"] if value
+    ipsets_in_perm = execute_firewall_cmd(['--get-ipsets'], nil).split(' ')
+    unless ipsets_in_perm.include?(@resource[:name])
+      args = []
+      args << ["--new-ipset=#{@resource[:name]}"]
+      args << ["--type=#{@resource[:type]}"]
+      options = {
+        family: @resource[:family],
+        hashsize: @resource[:hashsize],
+        maxelem: @resource[:maxelem],
+        timeout: @resource[:timeout]
+      }
+      options = options.merge(@resource[:options]) if @resource[:options]
+      options.each do |option_name, value|
+        args << ["--option=#{option_name}=#{value}"] if value
+      end
+      execute_firewall_cmd(args.flatten, nil)
     end
-    execute_firewall_cmd(args.flatten, nil)
     @resource[:entries].each { |e| add_entry(e) } if @resource[:manage_entries]
   end
 
@@ -72,7 +75,7 @@ def create
 
   def entries
     if @resource[:manage_entries]
-      execute_firewall_cmd(["--ipset=#{@resource[:name]}", '--get-entries'], nil).split("\n").sort
+      execute_firewall_cmd(["--ipset=#{@resource[:name]}", '--get-entries'], nil, false).split("\n").sort
     else
       @resource[:entries]
     end
@@ -99,6 +102,7 @@ def entries=(should_entries)
   end
 
   def destroy
-    execute_firewall_cmd(["--delete-ipset=#{@resource[:name]}"], nil)
+    ipsets_in_perm = execute_firewall_cmd(['--get-ipsets'], nil).split(' ')
+    execute_firewall_cmd(["--delete-ipset=#{@resource[:name]}"], nil) if ipsets_in_perm.include?(@resource[:name])
   end
 end

lib/puppet/provider/firewalld_port/firewall_cmd.rb

@@ -7,12 +7,25 @@
 ) do
   desc 'Interact with firewall-cmd'
 
+  attr_accessor :in_perm, :in_run
+
   mk_resource_methods
 
+  def initialize(value = {})
+    super(value)
+    @in_perm = false
+    @in_run = false
+  end
+
   def exists?
     @rule_args ||= build_port_rule
-    output = execute_firewall_cmd(['--query-port', @rule_args], @resource[:zone], true, false)
-    output.exitstatus.zero?
+    @in_perm = execute_firewall_cmd(['--query-port', @rule_args], @resource[:zone], true, false).exitstatus.zero?
+    @in_run = execute_firewall_cmd(['--query-port', @rule_args], @resource[:zone], false, false).exitstatus.zero?
+    if @resource[:ensure] == :present
+      @in_perm && @in_run
+    else
+      @in_perm || @in_run
+    end
   end
 
   def quote_keyval(key, val)
@@ -32,10 +45,10 @@ def build_port_rule
   end
 
   def create
-    execute_firewall_cmd(['--add-port', build_port_rule])
+    execute_firewall_cmd(['--add-port', build_port_rule]) unless @in_perm
   end
 
   def destroy
-    execute_firewall_cmd(['--remove-port', build_port_rule])
+    execute_firewall_cmd(['--remove-port', build_port_rule]) if @in_perm
   end
 end

lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb

@@ -7,12 +7,25 @@
 ) do
   desc 'Interact with firewall-cmd'
 
+  attr_accessor :in_perm, :in_run
+
   mk_resource_methods
 
+  def initialize(value = {})
+    super(value)
+    @in_perm = false
+    @in_run = false
+  end
+
   def exists?
     @rule_args ||= build_rich_rule
-    output = execute_firewall_cmd(['--query-rich-rule', @rule_args], @resource[:zone], true, false)
-    output.exitstatus.zero?
+    @in_perm = execute_firewall_cmd(['--query-rich-rule', @rule_args], @resource[:zone], true, false).exitstatus.zero?
+    @in_run = execute_firewall_cmd(['--query-rich-rule', @rule_args], @resource[:zone], false, false).exitstatus.zero?
+    if @resource[:ensure] == :present
+      @in_perm && @in_run
+    else
+      @in_perm || @in_run
+    end
   end
 
   def quote_keyval(key, val)
@@ -124,10 +137,10 @@ def build_rich_rule
   end
 
   def create
-    execute_firewall_cmd(['--add-rich-rule', build_rich_rule])
+    execute_firewall_cmd(['--add-rich-rule', build_rich_rule]) unless @in_perm
   end
 
   def destroy
-    execute_firewall_cmd(['--remove-rich-rule', build_rich_rule])
+    execute_firewall_cmd(['--remove-rich-rule', build_rich_rule]) if @in_perm
   end
 end

manifests/init.pp

@@ -248,7 +248,6 @@
   Firewalld_ipset <||> ~> Class['firewalld::reload']
   Firewalld_port <||> ~> Class['firewalld::reload']
   Firewalld_rich_rule <||> ~> Class['firewalld::reload']
-  Firewalld_service <||> ~> Class['firewalld::reload']
 
   if $purge_unknown_ipsets {
     Firewalld_ipset <||>

spec/classes/init_spec.rb

@@ -141,7 +141,6 @@
       is_expected.to contain_firewalld_service('mysql').
         with_ensure('present').
         with_zone('public').
-        that_notifies('Class[firewalld::reload]').
         that_requires('Service[firewalld]')
     end
   end

spec/unit/puppet/provider/firewalld_direct_chain_spec.rb

@@ -0,0 +1,76 @@
+require 'spec_helper'
+
+provider_class = Puppet::Type.type(:firewalld_direct_chain).provider(:firewall_cmd)
+
+describe provider_class do
+  let(:resource) do
+    @resource = Puppet::Type.type(:firewalld_direct_chain).new(
+      ensure: :present,
+      name: 'LOG_DROPS',
+      inet_protocol: 'ipv4',
+      table: 'filter'
+    )
+  end
+  let(:provider) { resource.provider }
+  let(:chain_args) { [%w[ipv4 filter LOG_DROPS]] }
+
+  describe 'when creating' do
+    it 'direct rule should not be created if rule exists in permanent' do
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, true, false).returns('yes')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, false, false).returns('no')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--add-chain', chain_args]).never
+      provider.exists?
+      provider.create
+    end
+
+    it 'creates' do
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, true, false).returns('no')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, false, false).returns('no')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--add-chain', chain_args], nil)
+      provider.exists?
+      provider.create
+    end
+  end
+
+  describe 'when destroying' do
+    it 'destroys' do
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, true, false).returns('yes')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, false, false).returns('yes')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-chain', chain_args], nil)
+      provider.exists?
+      provider.destroy
+    end
+
+    it 'direct rule should not be destroyed if it doesnt exist in permanent' do
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, true, false).returns('no')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, false, false).returns('yes')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-chain', chain_args]).never
+      provider.exists?
+      provider.destroy
+    end
+  end
+
+  describe 'exists?' do
+    let(:resource_absent) do
+      @resource_absent = Puppet::Type.type(:firewalld_direct_chain).new(
+        ensure: :absent,
+        name: 'LOG_DROPS',
+        inet_protocol: 'ipv4',
+        table: 'filter'
+      )
+    end
+    let(:provider_absent) { resource_absent.provider }
+
+    it 'direct rule creation should be triggered if rule exists in runtime but not in permanent' do
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, true, false).returns('no')
+      provider.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, false, false).returns('yes')
+      expect(provider.exists?).to eq(false)
+    end
+
+    it 'direct rule deletion should be triggered if rule exists in runtime but not in permanent' do
+      provider_absent.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, true, false).returns('no')
+      provider_absent.expects(:execute_firewall_cmd).with(['--direct', '--query-chain', chain_args], nil, false, false).returns('yes')
+      expect(provider_absent.exists?).to eq(true)
+    end
+  end
+end