From 5e5743da5dc58045c9eb0f75134850f6c094c98b Mon Sep 17 00:00:00 2001 From: isqo Date: Tue, 31 Mar 2020 00:18:36 +0200 Subject: [PATCH 01/24] FIX #276 for firewalld_service --- .travis.yml | 5 +- .../firewalld_service/firewall_cmd.rb | 18 ++++- manifests/init.pp | 1 - spec/classes/init_spec.rb | 1 - .../puppet/provider/firewalld_service_spec.rb | 75 +++++++++++++++++++ 5 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 spec/unit/puppet/provider/firewalld_service_spec.rb diff --git a/.travis.yml b/.travis.yml index 2b897eff..8df2fc72 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,10 +22,7 @@ matrix: - rvm: 2.4.4 bundler_args: --without system_tests development release env: PUPPET_VERSION="~> 5.0" CHECK=build DEPLOY_TO_FORGE=yes -branches: - only: - - master - - /^v\d/ + notifications: email: false webhooks: https://voxpupu.li/incoming/travis diff --git a/lib/puppet/provider/firewalld_service/firewall_cmd.rb b/lib/puppet/provider/firewalld_service/firewall_cmd.rb index 8e98aa97..9bc34c32 100644 --- a/lib/puppet/provider/firewalld_service/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_service/firewall_cmd.rb @@ -7,13 +7,25 @@ ) do desc 'Interact with firewall-cmd' + def initialize(value = {}) + super(value) + @exists_in_perm = false + @exists_in_run = false + end + def exists? - execute_firewall_cmd(['--list-services']).split(' ').include?(@resource[:service]) + @exists_in_perm = execute_firewall_cmd(['--list-services']).split(' ').include?(@resource[:service]) + @exists_in_run = execute_firewall_cmd(['--list-services'], nil, false).split(' ').include?(@resource[:service]) + if @resource[:ensure] == :present + @exists_in_perm && @exists_in_run + else + @exists_in_perm || @exists_in_run + end end def create debug("Adding new service to firewalld: #{@resource[:service]}") - execute_firewall_cmd(['--add-service', @resource[:service]]) + execute_firewall_cmd(['--add-service', @resource[:service]]) unless @exists_in_perm reload_firewall end @@ -26,7 +38,7 @@ def destroy '--remove-service-from-zone' end - execute_firewall_cmd([flag, @resource[:service]]) + execute_firewall_cmd([flag, @resource[:service]]) if @exists_in_perm reload_firewall end end diff --git a/manifests/init.pp b/manifests/init.pp index fe1d0552..fb6aa5e1 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -253,7 +253,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 <||> diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb index 4fb4ba7a..45e99e2f 100644 --- a/spec/classes/init_spec.rb +++ b/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 diff --git a/spec/unit/puppet/provider/firewalld_service_spec.rb b/spec/unit/puppet/provider/firewalld_service_spec.rb new file mode 100644 index 00000000..77a432a1 --- /dev/null +++ b/spec/unit/puppet/provider/firewalld_service_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +provider_class = Puppet::Type.type(:firewalld_service).provider(:firewall_cmd) + +describe provider_class do + let(:resource) do + @resource = Puppet::Type.type(:firewalld_service).new( + ensure: :present, + name: 'Allow SSH from the external zone', + service: 'ssh', + zone: 'external' + ) + end + let(:provider) { resource.provider } + + describe 'when creating' do + it 'service should not be added if rule exists in permanent' do + provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('ssh http') + provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') + provider.expects(:execute_firewall_cmd).with(%w[--add-service ssh]).never + provider.exists? + provider.create + end + + it 'service should be added if rule does not exist in permanent' do + provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('') + provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('') + provider.expects(:execute_firewall_cmd).with(%w[--add-service ssh]).once + provider.exists? + provider.create + end + end + + describe 'when destroying' do + it 'service should not be deleted if rule does not exist in permanent' do + provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('') + provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('') + provider.expects(:execute_firewall_cmd).with(%w[--remove-service-from-zone ssh]).never + provider.exists? + provider.destroy + end + + it 'service should be deleted if rule exists in permanent' do + provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('ssh http') + provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') + provider.expects(:execute_firewall_cmd).with(%w[--remove-service-from-zone ssh]).once + provider.exists? + provider.destroy + end + end + + describe 'exists?' do + let(:resource_absent) do + @resource_absent = Puppet::Type.type(:firewalld_service).new( + ensure: :absent, + name: 'Allow SSH from the external zone', + service: 'ssh', + zone: 'external' + ) + end + let(:provider_absent) { resource_absent.provider } + + it 'service creation should be triggered if rule exists in runtime but not in permanent' do + provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('') + provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') + expect(provider.exists?).to eq(false) + end + + it 'service deletion should be triggered if rule exists in runtime but not in permanent' do + provider_absent.expects(:execute_firewall_cmd).with(['--list-services']).returns('') + provider_absent.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') + expect(provider_absent.exists?).to eq(true) + end + end +end From 45c82b19b5b9861fda1653c71ed359f55d8dc9eb Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 1 Apr 2020 20:50:51 +0200 Subject: [PATCH 02/24] FIX #276 for firewalld_direct_rule - Check direct rules existence in both runtime and permanent - Put all provider direct rules unit tests in firewalld_direct_rule_spec.rb --- .../firewalld_direct_rule/firewall_cmd.rb | 13 ++- .../provider/firewalld_direct_rule_spec.rb | 94 +++++++++++++++++++ .../puppet/type/firewalld_direct_rule_spec.rb | 38 -------- 3 files changed, 103 insertions(+), 42 deletions(-) create mode 100644 spec/unit/puppet/provider/firewalld_direct_rule_spec.rb diff --git a/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb index c0e2cd55..714a10a4 100644 --- a/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb @@ -9,18 +9,23 @@ 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 diff --git a/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb b/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb new file mode 100644 index 00000000..36ba2916 --- /dev/null +++ b/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +provider_class = Puppet::Type.type(:firewalld_direct_rule).provider(:firewall_cmd) + +describe provider_class do + let(:resource) do + @resource = Puppet::Type.type(:firewalld_direct_rule).new( + ensure: :present, + name: 'Allow outgoing SSH connection', + inet_protocol: 'ipv4', + table: 'filter', + chain: 'OUTPUT', + priority: 1, + args: '-p tcp --dport=22 -j ACCEPT', + ) + end + let(:provider) { resource.provider } + let(:rule_args) { %w[ipv4 filter OUTPUT 1 -p tcp --dport=22 -j ACCEPT] } + + 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-rule', rule_args], nil, true, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, false, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--add-rule', rule_args]).never + provider.exists? + provider.create + end + + it 'creates' do + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, true, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, false, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--add-rule', rule_args], nil) + provider.exists? + provider.create + end + end + + describe 'when destroying' do + it 'destroys' do + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, true, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, false, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-rule', rule_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-rule', rule_args], nil, true, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, false, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-rule', rule_args]).never + provider.exists? + provider.destroy + end + end + + describe 'exists?' do + let(:resource_absent) do + @resource_absent = Puppet::Type.type(:firewalld_direct_rule).new( + ensure: :absent, + name: 'Allow outgoing SSH connection', + inet_protocol: 'ipv4', + table: 'filter', + chain: 'OUTPUT', + priority: 1, + args: '-p tcp --dport=22 -j ACCEPT', + ) + 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-rule', rule_args], nil, true, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_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-rule', rule_args], nil, true, false).returns('no') + provider_absent.expects(:execute_firewall_cmd).with(['--direct', '--query-rule', rule_args], nil, false, false).returns('yes') + expect(provider_absent.exists?).to eq(true) + end + end + + context 'parsing arguments' do + it 'correctlies parse arguments into an array' do + args = '-p tcp --dport=22 -j ACCEPT' + expect(provider.parse_args(args)).to eq(['-p', 'tcp', '--dport=22', '-j', 'ACCEPT']) + end + + it 'correctlies parse arguments in quotes' do + args = "-j LOG --log-prefix '# IPTABLES DROPPED:'" + expect(provider.parse_args(args)).to eq(['-j', 'LOG', '--log-prefix', '\'# IPTABLES DROPPED:\'']) + end + end +end diff --git a/spec/unit/puppet/type/firewalld_direct_rule_spec.rb b/spec/unit/puppet/type/firewalld_direct_rule_spec.rb index 2e6df270..bf6f91f2 100644 --- a/spec/unit/puppet/type/firewalld_direct_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_direct_rule_spec.rb @@ -41,44 +41,6 @@ end end - describe 'provider' do - let(:resource) do - described_class.new( - name: 'allow ssh', - ensure: 'present', - inet_protocol: 'ipv4', - table: 'filter', - chain: 'OUTPUT', - priority: 4, - args: '-p tcp --dport=22 -j ACCEPT' - ) - end - - let(:provider) { resource.provider } - - it 'creates' do - provider.expects(:execute_firewall_cmd).with(['--direct', '--add-rule', ['ipv4', 'filter', 'OUTPUT', '4', '-p', 'tcp', '--dport=22', '-j', 'ACCEPT']], nil) - provider.create - end - - it 'destroys' do - provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-rule', ['ipv4', 'filter', 'OUTPUT', '4', '-p', 'tcp', '--dport=22', '-j', 'ACCEPT']], nil) - provider.destroy - end - - context 'parsing arguments' do - it 'correctlies parse arguments into an array' do - args = '-p tcp --dport=22 -j ACCEPT' - expect(provider.parse_args(args)).to eq(['-p', 'tcp', '--dport=22', '-j', 'ACCEPT']) - end - - it 'correctlies parse arguments in quotes' do - args = "-j LOG --log-prefix '# IPTABLES DROPPED:'" - expect(provider.parse_args(args)).to eq(['-j', 'LOG', '--log-prefix', '\'# IPTABLES DROPPED:\'']) - end - end - end - context 'autorequires' do # rubocop:disable RSpec/InstanceVariable before do From fb15e443cd7e61881d01229e5a26387469543fad Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 1 Apr 2020 20:54:49 +0200 Subject: [PATCH 03/24] fix offenses --- .../provider/firewalld_direct_rule/firewall_cmd.rb | 10 ++++++++-- .../unit/puppet/provider/firewalld_direct_rule_spec.rb | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb index 714a10a4..a9f5e1c7 100644 --- a/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb @@ -7,6 +7,12 @@ ) do desc 'Interact with firewall-cmd' + def initialize(value = {}) + super(value) + @in_perm = false + @in_run = false + end + def exists? @rule_args ||= generate_raw @in_perm = execute_firewall_cmd(['--direct', '--query-rule', @rule_args], nil, true, false).include?('yes') @@ -20,12 +26,12 @@ def exists? def create @rule_args ||= generate_raw - execute_firewall_cmd(['--direct', '--add-rule', @rule_args], nil) unless @in_perm + 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) if @in_perm + execute_firewall_cmd(['--direct', '--remove-rule', @rule_args], nil) if @in_perm end def generate_raw diff --git a/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb b/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb index 36ba2916..f0f99359 100644 --- a/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb +++ b/spec/unit/puppet/provider/firewalld_direct_rule_spec.rb @@ -11,7 +11,7 @@ table: 'filter', chain: 'OUTPUT', priority: 1, - args: '-p tcp --dport=22 -j ACCEPT', + args: '-p tcp --dport=22 -j ACCEPT' ) end let(:provider) { resource.provider } @@ -62,7 +62,7 @@ table: 'filter', chain: 'OUTPUT', priority: 1, - args: '-p tcp --dport=22 -j ACCEPT', + args: '-p tcp --dport=22 -j ACCEPT' ) end let(:provider_absent) { resource_absent.provider } From 653fe346a38b42a28b377006c35093c5ff44ddfa Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 1 Apr 2020 21:20:09 +0200 Subject: [PATCH 04/24] FIX #276 for firewalld_direct_chain - Check direct rules existence in both runtime and permanent --- .../firewalld_direct_chain/firewall_cmd.rb | 19 ++++- .../provider/firewalld_direct_chain_spec.rb | 76 +++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 spec/unit/puppet/provider/firewalld_direct_chain_spec.rb diff --git a/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb index bacad590..d405b924 100644 --- a/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb +++ b/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 diff --git a/spec/unit/puppet/provider/firewalld_direct_chain_spec.rb b/spec/unit/puppet/provider/firewalld_direct_chain_spec.rb new file mode 100644 index 00000000..2d91d664 --- /dev/null +++ b/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) { provider.generate_raw } + + 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 From 03a83b5f76bfc155c065bdf3cee8b77565475bd8 Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 1 Apr 2020 21:59:56 +0200 Subject: [PATCH 05/24] FIX #276 for firewalld_direct_passthrough - Check direct rules existence in both runtime and permanent - fix wrong chain_args value in direct chain spec - put all direct passthrough provider unit tests in its corresponding file --- .../firewall_cmd.rb | 19 ++++- .../provider/firewalld_direct_chain_spec.rb | 2 +- .../firewalld_direct_passthrough_spec.rb | 77 +++++++++++++++++++ .../type/firewalld_direct_passthrough_spec.rb | 22 ------ 4 files changed, 93 insertions(+), 27 deletions(-) create mode 100644 spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb diff --git a/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb index 3478e119..5cb10351 100644 --- a/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb +++ b/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 diff --git a/spec/unit/puppet/provider/firewalld_direct_chain_spec.rb b/spec/unit/puppet/provider/firewalld_direct_chain_spec.rb index 2d91d664..e2d62d28 100644 --- a/spec/unit/puppet/provider/firewalld_direct_chain_spec.rb +++ b/spec/unit/puppet/provider/firewalld_direct_chain_spec.rb @@ -12,7 +12,7 @@ ) end let(:provider) { resource.provider } - let(:chain_args) { provider.generate_raw } + 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 diff --git a/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb b/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb new file mode 100644 index 00000000..b23c744a --- /dev/null +++ b/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +provider_class = Puppet::Type.type(:firewalld_direct_passthrough).provider(:firewall_cmd) + +describe provider_class do + let(:resource) do + @resource = Puppet::Type.type(:firewalld_direct_passthrough).new( + ensure: :present, + name: 'Allow outgoing SSH connection', + inet_protocol: 'ipv4', + args: '-A OUTPUT -j OUTPUT_filter' + ) + end + let(:provider) { resource.provider } + let(:pass_args) { %w[ipv4 -A OUTPUT -j OUTPUT_filter] } + + 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-passthrough', pass_args], nil, true, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, false, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--add-passthrough', pass_args]).never + provider.exists? + provider.create + end + + it 'creates' do + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, true, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, false, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--add-passthrough', pass_args], nil) + provider.exists? + provider.create + end + end + + describe 'when destroying' do + it 'destroys' do + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, true, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, false, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-passthrough', pass_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-passthrough', pass_args], nil, true, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, false, false).returns('yes') + provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-passthrough', pass_args]).never + provider.exists? + provider.destroy + end + end + + describe 'exists?' do + let(:resource_absent) do + @resource_absent = Puppet::Type.type(:firewalld_direct_passthrough).new( + ensure: :absent, + name: 'Allow outgoing SSH connection', + inet_protocol: 'ipv4', + args: '-A OUTPUT -j OUTPUT_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-passthrough', pass_args], nil, true, false).returns('no') + provider.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_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-passthrough', pass_args], nil, true, false).returns('no') + provider_absent.expects(:execute_firewall_cmd).with(['--direct', '--query-passthrough', pass_args], nil, false, false).returns('yes') + expect(provider_absent.exists?).to eq(true) + end + end + +end diff --git a/spec/unit/puppet/type/firewalld_direct_passthrough_spec.rb b/spec/unit/puppet/type/firewalld_direct_passthrough_spec.rb index 88b5758a..2ece5ba8 100644 --- a/spec/unit/puppet/type/firewalld_direct_passthrough_spec.rb +++ b/spec/unit/puppet/type/firewalld_direct_passthrough_spec.rb @@ -30,28 +30,6 @@ end end - describe 'provider' do - let(:resource) do - described_class.new( - name: 'Forward OUTPUT', - ensure: 'present', - inet_protocol: 'ipv4', - args: '-A OUTPUT -j OUTPUT_filter' - ) - end - - let(:provider) { resource.provider } - - it 'creates' do - provider.expects(:execute_firewall_cmd).with(['--direct', '--add-passthrough', ['ipv4', '-A', 'OUTPUT', '-j', 'OUTPUT_filter']], nil) - provider.create - end - it 'destroys' do - provider.expects(:execute_firewall_cmd).with(['--direct', '--remove-passthrough', ['ipv4', '-A', 'OUTPUT', '-j', 'OUTPUT_filter']], nil) - provider.destroy - end - end - context 'autorequires' do # rubocop:disable RSpec/InstanceVariable before do From 07c2c4d82b328cebb9d709073af73709b5e903a0 Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 1 Apr 2020 22:05:06 +0200 Subject: [PATCH 06/24] FIX #276 fix all offences --- spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb b/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb index b23c744a..7c875643 100644 --- a/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb +++ b/spec/unit/puppet/provider/firewalld_direct_passthrough_spec.rb @@ -73,5 +73,4 @@ expect(provider_absent.exists?).to eq(true) end end - end From ae4561ac59665b411c88e7851725b4451ed62422 Mon Sep 17 00:00:00 2001 From: isqo Date: Fri, 3 Apr 2020 22:22:46 +0200 Subject: [PATCH 07/24] FIX 276 for firewalld_rich_rule - Check rich provider rules existence in both runtime and permanent - put all rich rule provider unit tests in provider spec file --- .../firewalld_rich_rule/firewall_cmd.rb | 21 ++- .../provider/firewalld_rich_rule_spec.rb | 126 ++++++++++++++++++ .../puppet/type/firewalld_rich_rule_spec.rb | 115 ---------------- 3 files changed, 143 insertions(+), 119 deletions(-) diff --git a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb index 08525f9f..d45638be 100644 --- a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb +++ b/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) @@ -122,10 +135,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 diff --git a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb index 807afc58..807207ef 100644 --- a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb @@ -24,6 +24,132 @@ end describe 'when creating' do + scenarios = { + ## Test source + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + zone: 'restricted', + source: { 'address' => '10.0.1.2/24' }, + service: 'ssh', + log: { 'level' => 'debug' }, + action: 'accept' + } => 'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" accept', + ## Test ipset + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + zone: 'restricted', + source: { 'ipset' => 'whitelist' }, + service: 'ssh', + log: { 'level' => 'debug' }, + action: 'accept' + } => 'rule family="ipv4" source ipset="whitelist" service name="ssh" log level="debug" accept', + + ## Test destination + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + zone: 'restricted', + dest: '10.0.1.2/24', + service: 'ssh', + log: { 'level' => 'debug' }, + action: 'accept' + } => 'rule family="ipv4" destination address="10.0.1.2/24" service name="ssh" log level="debug" accept', + + ## Test address invertion + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + zone: 'restricted', + source: { 'address' => '10.0.1.2/24', 'invert' => true }, + service: 'ssh', + log: { 'level' => 'debug' }, + action: 'accept' + } => 'rule family="ipv4" source NOT address="10.0.1.2/24" service name="ssh" log level="debug" accept', + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + zone: 'restricted', + dest: { 'address' => '10.0.1.2/24', 'invert' => true }, + service: 'ssh', + log: { 'level' => 'debug' }, + action: 'accept' + } => 'rule family="ipv4" destination NOT address="10.0.1.2/24" service name="ssh" log level="debug" accept', + + ## test port + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + zone: 'restricted', + dest: '10.0.1.2/24', + port: { 'port' => '22', 'protocol' => 'tcp' }, + log: { 'level' => 'debug' }, + action: 'accept' + } => 'rule family="ipv4" destination address="10.0.1.2/24" port port="22" protocol="tcp" log level="debug" accept', + + ## test forward port + { + name: 'accept ssh', + ensure: 'present', + family: 'ipv4', + forward_port: { 'port' => '8080', 'protocol' => 'tcp', 'to_addr' => '10.72.1.10', 'to_port' => '80' }, + zone: 'restricted', + log: { 'level' => 'debug' } + } => 'rule family="ipv4" forward-port port="8080" protocol="tcp" to-port="80" to-addr="10.72.1.10" log level="debug"' + + } + + scenarios.each do |attrs, rawrule| + context "for rule #{rawrule}" do + let(:resource) do + Puppet::Type.type(:firewalld_rich_rule).new(attrs) + end + let(:fakeclass) { Class.new } + let(:provider) { resource.provider } + let(:rawrule) do + 'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" accept' + end + + it 'queries the status' do + fakeclass.stubs(:exitstatus).returns(0) + provider.expects(:execute_firewall_cmd).with(['--query-rich-rule', rawrule], 'restricted', true, false).returns(fakeclass) + provider.expects(:execute_firewall_cmd).with(['--query-rich-rule', rawrule], 'restricted', false, false).returns(fakeclass) + expect(provider).to be_exists + end + + it 'add rich rule executed when rule does not exist in permanent' do + provider.in_perm = false + provider.expects(:execute_firewall_cmd).with(['--add-rich-rule', rawrule]) + provider.create + end + + it 'remove rich rule executed when rule does exist in permanent' do + provider.in_perm = true + provider.expects(:execute_firewall_cmd).with(['--remove-rich-rule', rawrule]) + provider.destroy + end + + it 'add rich rule does not execute when exist in permanent' do + provider.in_perm = true + provider.expects(:execute_firewall_cmd).with(['--add-rich-rule', rawrule]).never + provider.create + end + + it 'remove rich rule does not execute when rule does not exist in permanent' do + provider.in_perm = false + provider.expects(:execute_firewall_cmd).with(['--remove-rich-rule', rawrule]).never + provider.destroy + end + end + end + context 'with basic parameters' do it 'builds the rich rule' do resource.expects(:[]).with(:source).returns('192.168.1.2/32').at_least_once diff --git a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb index 160c8167..a456ae5a 100644 --- a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb @@ -90,121 +90,6 @@ end end - ## Many more scenarios needed! - # - describe 'provider' do - scenarios = { - ## Test source - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - zone: 'restricted', - source: { 'address' => '10.0.1.2/24' }, - service: 'ssh', - log: { 'level' => 'debug' }, - action: 'accept' - } => 'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" accept', - ## Test ipset - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - zone: 'restricted', - source: { 'ipset' => 'whitelist' }, - service: 'ssh', - log: { 'level' => 'debug' }, - action: 'accept' - } => 'rule family="ipv4" source ipset="whitelist" service name="ssh" log level="debug" accept', - - ## Test destination - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - zone: 'restricted', - dest: '10.0.1.2/24', - service: 'ssh', - log: { 'level' => 'debug' }, - action: 'accept' - } => 'rule family="ipv4" destination address="10.0.1.2/24" service name="ssh" log level="debug" accept', - - ## Test address invertion - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - zone: 'restricted', - source: { 'address' => '10.0.1.2/24', 'invert' => true }, - service: 'ssh', - log: { 'level' => 'debug' }, - action: 'accept' - } => 'rule family="ipv4" source NOT address="10.0.1.2/24" service name="ssh" log level="debug" accept', - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - zone: 'restricted', - dest: { 'address' => '10.0.1.2/24', 'invert' => true }, - service: 'ssh', - log: { 'level' => 'debug' }, - action: 'accept' - } => 'rule family="ipv4" destination NOT address="10.0.1.2/24" service name="ssh" log level="debug" accept', - - ## test port - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - zone: 'restricted', - dest: '10.0.1.2/24', - port: { 'port' => '22', 'protocol' => 'tcp' }, - log: { 'level' => 'debug' }, - action: 'accept' - } => 'rule family="ipv4" destination address="10.0.1.2/24" port port="22" protocol="tcp" log level="debug" accept', - - ## test forward port - { - name: 'accept ssh', - ensure: 'present', - family: 'ipv4', - forward_port: { 'port' => '8080', 'protocol' => 'tcp', 'to_addr' => '10.72.1.10', 'to_port' => '80' }, - zone: 'restricted', - log: { 'level' => 'debug' } - } => 'rule family="ipv4" forward-port port="8080" protocol="tcp" to-port="80" to-addr="10.72.1.10" log level="debug"' - - } - - scenarios.each do |attrs, rawrule| - context "for rule #{rawrule}" do - let(:resource) do - described_class.new(attrs) - end - let(:fakeclass) { Class.new } - let(:provider) { resource.provider } - let(:rawrule) do - 'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" accept' - end - - it 'queries the status' do - fakeclass.stubs(:exitstatus).returns(0) - provider.expects(:execute_firewall_cmd).with(['--query-rich-rule', rawrule], 'restricted', true, false).returns(fakeclass) - expect(provider).to be_exists - end - - it 'creates' do - provider.expects(:execute_firewall_cmd).with(['--add-rich-rule', rawrule]) - provider.create - end - - it 'destroys' do - provider.expects(:execute_firewall_cmd).with(['--remove-rich-rule', rawrule]) - provider.destroy - end - end - end - end - context 'autorequires' do # rubocop:disable RSpec/InstanceVariable before do From 0288630f20abf9e30c05af485a6aae487d718b8e Mon Sep 17 00:00:00 2001 From: isqo Date: Fri, 3 Apr 2020 22:27:26 +0200 Subject: [PATCH 08/24] FIX #276 for firewalld_rich_rule - Check rich provider rules existence in both runtime and permanent - put all rich rule provider unit tests in provider spec file --- lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb index d45638be..8c483516 100644 --- a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb @@ -7,7 +7,7 @@ ) do desc 'Interact with firewall-cmd' - attr_accessor :in_perm,:in_run + attr_accessor :in_perm, :in_run mk_resource_methods From 6e471be8c703caf43d4520913a54066bcc13af98 Mon Sep 17 00:00:00 2001 From: isqo Date: Fri, 3 Apr 2020 23:04:50 +0200 Subject: [PATCH 09/24] FIX #276 for firewalld_port - Check port's existence in both runtime and permanent --- .../provider/firewalld_port/firewall_cmd.rb | 21 ++++- .../puppet/provider/firewalld_port_spec.rb | 76 +++++++++++++++++++ 2 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 spec/unit/puppet/provider/firewalld_port_spec.rb diff --git a/lib/puppet/provider/firewalld_port/firewall_cmd.rb b/lib/puppet/provider/firewalld_port/firewall_cmd.rb index 9e56457c..5666551f 100644 --- a/lib/puppet/provider/firewalld_port/firewall_cmd.rb +++ b/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 diff --git a/spec/unit/puppet/provider/firewalld_port_spec.rb b/spec/unit/puppet/provider/firewalld_port_spec.rb new file mode 100644 index 00000000..baf3f89f --- /dev/null +++ b/spec/unit/puppet/provider/firewalld_port_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +provider_class = Puppet::Type.type(:firewalld_port).provider(:firewall_cmd) + +describe provider_class do + let(:resource) do + @resource = Puppet::Type.type(:firewalld_port).new( + ensure: :present, + name: 'Open port 8080 in the public zone', + zone: 'public', + port: '8080', + protocol: 'tcp', + ) + end + let(:provider) { resource.provider } + let(:fakeclassperm) { Class.new } + let(:fakeclassrun) { Class.new } + let(:args) { [['8080/tcp']] } + + describe 'when creating' do + it 'add port should not execute if rule exists in permanent' do + provider.in_perm = true + provider.expects(:execute_firewall_cmd).with(['--add-port', args]).never + provider.create + end + + it 'add port should execute if rule does not exist in permanent' do + provider.in_perm = false + provider.expects(:execute_firewall_cmd).with(['--add-port', args]) + provider.create + end + end + + describe 'when destroying' do + it 'remove port should execute if rule exists in permanent' do + provider.in_perm = true + provider.expects(:execute_firewall_cmd).with(['--remove-port', args]) + provider.destroy + end + + it 'remove port should not execute if rule does not exist in permanent' do + provider.in_perm=false + provider.expects(:execute_firewall_cmd).with(['--remove-port', args]).never + provider.destroy + end + end + + describe 'exists?' do + let(:resource_absent) do + @resource_absent = Puppet::Type.type(:firewalld_port).new( + ensure: :absent, + name: 'Open port 8080 in the public zone', + zone: 'public', + port: '8080', + protocol: 'tcp', + ) + end + let(:provider_absent) { resource_absent.provider } + + it 'port adding should be triggered if rule exists in runtime but not in permanent' do + fakeclassperm.stubs(:exitstatus).returns(0) + fakeclassrun.stubs(:exitstatus).returns(1) + provider.expects(:execute_firewall_cmd).with(['--query-port', args], 'public', false, false).returns(fakeclassperm) + provider.expects(:execute_firewall_cmd).with(['--query-port', args], 'public', true, false).returns(fakeclassrun) + expect(provider.exists?).to eq(false) + end + + it 'port deletion should be triggered if rule exists in runtime but not in permanent' do + fakeclassperm.stubs(:exitstatus).returns(0) + fakeclassrun.stubs(:exitstatus).returns(1) + provider_absent.expects(:execute_firewall_cmd).with(['--query-port', args], 'public', true, false).returns(fakeclassperm) + provider_absent.expects(:execute_firewall_cmd).with(['--query-port', args], 'public', false, false).returns(fakeclassrun) + expect(provider_absent.exists?).to eq(true) + end + end +end From 3d84bfecfe973dcd9fd2fef1b8b7c188dc4f1256 Mon Sep 17 00:00:00 2001 From: isqo Date: Fri, 3 Apr 2020 23:09:42 +0200 Subject: [PATCH 10/24] fix offences --- spec/unit/puppet/provider/firewalld_port_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/unit/puppet/provider/firewalld_port_spec.rb b/spec/unit/puppet/provider/firewalld_port_spec.rb index baf3f89f..a2835bc2 100644 --- a/spec/unit/puppet/provider/firewalld_port_spec.rb +++ b/spec/unit/puppet/provider/firewalld_port_spec.rb @@ -9,7 +9,7 @@ name: 'Open port 8080 in the public zone', zone: 'public', port: '8080', - protocol: 'tcp', + protocol: 'tcp' ) end let(:provider) { resource.provider } @@ -39,7 +39,7 @@ end it 'remove port should not execute if rule does not exist in permanent' do - provider.in_perm=false + provider.in_perm = false provider.expects(:execute_firewall_cmd).with(['--remove-port', args]).never provider.destroy end @@ -52,8 +52,8 @@ name: 'Open port 8080 in the public zone', zone: 'public', port: '8080', - protocol: 'tcp', - ) + protocol: 'tcp' + ) end let(:provider_absent) { resource_absent.provider } From e1c8ddc850741291f8c55bd646d1eccfe1c3e8ec Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Thu, 23 Apr 2020 17:33:03 +0200 Subject: [PATCH 11/24] ipsets should be considered present only if they are in runtime stage --- .../provider/firewalld_ipset/firewall_cmd.rb | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb b/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb index 69605b56..5dc45beb 100644 --- a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb +++ b/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,true).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,true) 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,true).split("\n").sort else @resource[:entries] end @@ -99,6 +102,9 @@ 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(' ') + if ipsets_in_perm.include?(@resource[:name]) + execute_firewall_cmd(["--delete-ipset=#{@resource[:name]}"], nil) + end end end From 176d1884aa16222ed21f88cdb694275f94809ce5 Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Thu, 23 Apr 2020 17:44:07 +0200 Subject: [PATCH 12/24] fix offenses --- lib/puppet/provider/firewalld_ipset/firewall_cmd.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb b/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb index 5dc45beb..2d71b77b 100644 --- a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb +++ b/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,true).split(' ') + ipset_ids = execute_firewall_cmd(['--get-ipsets'], nil, true).split(' ') ipset_ids.map do |ipset_id| - ipset_raw = execute_firewall_cmd(["--info-ipset=#{ipset_id}"], nil,true) + ipset_raw = execute_firewall_cmd(["--info-ipset=#{ipset_id}"], nil, true) raw_options = ipset_raw.match(%r{options: (.*)}) options = {} if raw_options @@ -75,7 +75,7 @@ def create def entries if @resource[:manage_entries] - execute_firewall_cmd(["--ipset=#{@resource[:name]}", '--get-entries'], nil,true).split("\n").sort + execute_firewall_cmd(["--ipset=#{@resource[:name]}", '--get-entries'], nil, true).split("\n").sort else @resource[:entries] end @@ -103,8 +103,6 @@ def entries=(should_entries) def destroy ipsets_in_perm = execute_firewall_cmd(['--get-ipsets'], nil).split(' ') - if ipsets_in_perm.include?(@resource[:name]) - execute_firewall_cmd(["--delete-ipset=#{@resource[:name]}"], nil) - end + execute_firewall_cmd(["--delete-ipset=#{@resource[:name]}"], nil) if ipsets_in_perm.include?(@resource[:name]) end end From ff6cf8af3787fd901738411776fb41f0d203ffce Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Thu, 23 Apr 2020 17:55:28 +0200 Subject: [PATCH 13/24] fix tests --- spec/unit/puppet/provider/firewalld_ipset_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index 54b1b886..e5e9d691 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -15,17 +15,18 @@ let(:provider) { resource.provider } before do + provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil,true).returns('white black') provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white black') provider.class.stubs(:execute_firewall_cmd).with(['--state'], nil, false, false, false).returns(Object.any_instance.stubs(exitstatus: 0)) # rubocop:disable RSpec/AnyInstance - provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=white'], nil).returns('white + provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=white'], nil, true).returns('white type: hash:ip options: maxelem=200 family=inet6 entries:') - provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=black'], nil).returns('black + provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=black'], nil, true).returns('black type: hash:ip options: maxelem=400 family=inet hashsize=2048 entries:') - provider.class.stubs(:execute_firewall_cmd).with(['--ipset=white', '--get-entries'], nil).returns('') + provider.class.stubs(:execute_firewall_cmd).with(['--ipset=white', '--get-entries'], nil, true).returns('') end describe 'self.instances' do From af9a84052b6f32ac307c686f276eed2e3df91e36 Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Fri, 24 Apr 2020 10:40:20 +0200 Subject: [PATCH 14/24] fix tests --- spec/unit/puppet/provider/firewalld_ipset_spec.rb | 5 +++++ spec/unit/puppet/type/firewalld_ipset_spec.rb | 2 ++ 2 files changed, 7 insertions(+) diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index e5e9d691..b4899143 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -62,6 +62,7 @@ resource.expects(:[]).with(:options).returns({}).at_least_once resource.expects(:[]).with(:manage_entries).returns(true) resource.expects(:[]).with(:entries).returns(['192.168.0/24', '10.0.0/8']) + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('') provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet', '--option=hashsize=1024', '--option=maxelem=65536'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0/24'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0/8'], nil) @@ -83,6 +84,8 @@ resource.expects(:[]).with(:options).returns({}).at_least_once resource.expects(:[]).with(:manage_entries).returns(true).at_least_once resource.expects(:[]).with(:entries).returns(['192.168.0/24', '10.0.0/8']).at_least_once + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('') + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white') provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet'], nil) provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet', '--option=hashsize=2048'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0/24'], nil).at_least_once @@ -104,6 +107,7 @@ resource.expects(:[]).with(:manage_entries).returns(true).at_least_once resource.expects(:[]).with(:entries).returns(['192.168.0.0/24', '10.0.0.0/8']).at_least_once provider.expects(:entries).returns(['192.168.0.0/24', '10.0.0.0/8']) + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('') provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0.0/24'], nil).at_least_once provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0.0/8'], nil).at_least_once @@ -121,6 +125,7 @@ resource.expects(:[]).with(:timeout).returns(nil).at_least_once resource.expects(:[]).with(:options).returns({}).at_least_once resource.expects(:[]).with(:manage_entries).returns(false).at_least_once + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('') provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet'], nil) provider.create provider.entries = ['192.168.14.0/24', '10.0.0.0/8'] diff --git a/spec/unit/puppet/type/firewalld_ipset_spec.rb b/spec/unit/puppet/type/firewalld_ipset_spec.rb index 84afdcc0..974c81a8 100644 --- a/spec/unit/puppet/type/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/type/firewalld_ipset_spec.rb @@ -99,6 +99,7 @@ end it 'creates' do + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('') provider.expects(:execute_firewall_cmd).with(['--new-ipset=whitelist', '--type=hash:ip'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=192.168.2.2'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=10.72.1.100'], nil) @@ -106,6 +107,7 @@ end it 'removes' do + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('whitelist') provider.expects(:execute_firewall_cmd).with(['--delete-ipset=whitelist'], nil) provider.destroy end From 28e3228da642c38d8ab568238c4aac6e8a59fffe Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Fri, 24 Apr 2020 10:57:54 +0200 Subject: [PATCH 15/24] fix tests --- spec/unit/puppet/provider/firewalld_ipset_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index b4899143..e937c896 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -74,6 +74,8 @@ describe 'when modifying' do context 'hashsize' do it 'removes and create a new ipset' do + ipsets_sequence = sequence('ipsets') + resource.expects(:[]).with(:name).returns('white').at_least_once resource.expects(:[]).with(:type).returns('hash:net').at_least_once resource.expects(:[]).with(:family).returns('inet').at_least_once @@ -84,12 +86,12 @@ resource.expects(:[]).with(:options).returns({}).at_least_once resource.expects(:[]).with(:manage_entries).returns(true).at_least_once resource.expects(:[]).with(:entries).returns(['192.168.0/24', '10.0.0/8']).at_least_once - provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('') - provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white') + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('').in_sequence(ipsets_sequence) provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet'], nil) provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet', '--option=hashsize=2048'], nil) provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0/24'], nil).at_least_once provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0/8'], nil).at_least_once + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white').in_sequence(ipsets_sequence) provider.expects(:execute_firewall_cmd).with(['--delete-ipset=white'], nil) provider.create provider.hashsize = 2048 From 27d3d022a8248aa4a63d9ec48be03a2d252ff84f Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Fri, 24 Apr 2020 11:03:03 +0200 Subject: [PATCH 16/24] fix tests --- spec/unit/puppet/provider/firewalld_ipset_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index e937c896..ae098d23 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -93,6 +93,7 @@ provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0/8'], nil).at_least_once provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white').in_sequence(ipsets_sequence) provider.expects(:execute_firewall_cmd).with(['--delete-ipset=white'], nil) + provider.expects(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('').in_sequence(ipsets_sequence) provider.create provider.hashsize = 2048 end From 3e97483b7c284b948e0faac5e83202a398bc954d Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Fri, 24 Apr 2020 11:06:51 +0200 Subject: [PATCH 17/24] fix offenses --- spec/unit/puppet/provider/firewalld_ipset_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index ae098d23..3a7f92e6 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -15,7 +15,7 @@ let(:provider) { resource.provider } before do - provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil,true).returns('white black') + provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil, true).returns('white black') provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white black') provider.class.stubs(:execute_firewall_cmd).with(['--state'], nil, false, false, false).returns(Object.any_instance.stubs(exitstatus: 0)) # rubocop:disable RSpec/AnyInstance provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=white'], nil, true).returns('white From ce81bd72c36334ead3cb3cf8b8f6e2e1f10e4553 Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Fri, 24 Apr 2020 11:15:31 +0200 Subject: [PATCH 18/24] revert service resource because it uses the transient reload --- .../firewalld_service/firewall_cmd.rb | 18 +---- .../puppet/provider/firewalld_service_spec.rb | 75 ------------------- 2 files changed, 3 insertions(+), 90 deletions(-) delete mode 100644 spec/unit/puppet/provider/firewalld_service_spec.rb diff --git a/lib/puppet/provider/firewalld_service/firewall_cmd.rb b/lib/puppet/provider/firewalld_service/firewall_cmd.rb index 9bc34c32..8e98aa97 100644 --- a/lib/puppet/provider/firewalld_service/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_service/firewall_cmd.rb @@ -7,25 +7,13 @@ ) do desc 'Interact with firewall-cmd' - def initialize(value = {}) - super(value) - @exists_in_perm = false - @exists_in_run = false - end - def exists? - @exists_in_perm = execute_firewall_cmd(['--list-services']).split(' ').include?(@resource[:service]) - @exists_in_run = execute_firewall_cmd(['--list-services'], nil, false).split(' ').include?(@resource[:service]) - if @resource[:ensure] == :present - @exists_in_perm && @exists_in_run - else - @exists_in_perm || @exists_in_run - end + execute_firewall_cmd(['--list-services']).split(' ').include?(@resource[:service]) end def create debug("Adding new service to firewalld: #{@resource[:service]}") - execute_firewall_cmd(['--add-service', @resource[:service]]) unless @exists_in_perm + execute_firewall_cmd(['--add-service', @resource[:service]]) reload_firewall end @@ -38,7 +26,7 @@ def destroy '--remove-service-from-zone' end - execute_firewall_cmd([flag, @resource[:service]]) if @exists_in_perm + execute_firewall_cmd([flag, @resource[:service]]) reload_firewall end end diff --git a/spec/unit/puppet/provider/firewalld_service_spec.rb b/spec/unit/puppet/provider/firewalld_service_spec.rb deleted file mode 100644 index 77a432a1..00000000 --- a/spec/unit/puppet/provider/firewalld_service_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -require 'spec_helper' - -provider_class = Puppet::Type.type(:firewalld_service).provider(:firewall_cmd) - -describe provider_class do - let(:resource) do - @resource = Puppet::Type.type(:firewalld_service).new( - ensure: :present, - name: 'Allow SSH from the external zone', - service: 'ssh', - zone: 'external' - ) - end - let(:provider) { resource.provider } - - describe 'when creating' do - it 'service should not be added if rule exists in permanent' do - provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('ssh http') - provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') - provider.expects(:execute_firewall_cmd).with(%w[--add-service ssh]).never - provider.exists? - provider.create - end - - it 'service should be added if rule does not exist in permanent' do - provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('') - provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('') - provider.expects(:execute_firewall_cmd).with(%w[--add-service ssh]).once - provider.exists? - provider.create - end - end - - describe 'when destroying' do - it 'service should not be deleted if rule does not exist in permanent' do - provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('') - provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('') - provider.expects(:execute_firewall_cmd).with(%w[--remove-service-from-zone ssh]).never - provider.exists? - provider.destroy - end - - it 'service should be deleted if rule exists in permanent' do - provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('ssh http') - provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') - provider.expects(:execute_firewall_cmd).with(%w[--remove-service-from-zone ssh]).once - provider.exists? - provider.destroy - end - end - - describe 'exists?' do - let(:resource_absent) do - @resource_absent = Puppet::Type.type(:firewalld_service).new( - ensure: :absent, - name: 'Allow SSH from the external zone', - service: 'ssh', - zone: 'external' - ) - end - let(:provider_absent) { resource_absent.provider } - - it 'service creation should be triggered if rule exists in runtime but not in permanent' do - provider.expects(:execute_firewall_cmd).with(['--list-services']).returns('') - provider.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') - expect(provider.exists?).to eq(false) - end - - it 'service deletion should be triggered if rule exists in runtime but not in permanent' do - provider_absent.expects(:execute_firewall_cmd).with(['--list-services']).returns('') - provider_absent.expects(:execute_firewall_cmd).with(['--list-services'], nil, false).returns('ssh http') - expect(provider_absent.exists?).to eq(true) - end - end -end From 558d8eeaac9930dce02df6e50ef1747485419b64 Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Fri, 24 Apr 2020 15:52:02 +0200 Subject: [PATCH 19/24] Fix getting ipsets from firewalld's runtime --- lib/puppet/provider/firewalld_ipset/firewall_cmd.rb | 6 +++--- spec/unit/puppet/provider/firewalld_ipset_spec.rb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb b/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb index 2d71b77b..5337c1c0 100644 --- a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb +++ b/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, true).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, true) + ipset_raw = execute_firewall_cmd(["--info-ipset=#{ipset_id}"], nil, false) raw_options = ipset_raw.match(%r{options: (.*)}) options = {} if raw_options @@ -75,7 +75,7 @@ def create def entries if @resource[:manage_entries] - execute_firewall_cmd(["--ipset=#{@resource[:name]}", '--get-entries'], nil, true).split("\n").sort + execute_firewall_cmd(["--ipset=#{@resource[:name]}", '--get-entries'], nil, false).split("\n").sort else @resource[:entries] end diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index 3a7f92e6..3e42f95e 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -15,18 +15,18 @@ let(:provider) { resource.provider } before do - provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil, true).returns('white black') + provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil, false).returns('white black') provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white black') provider.class.stubs(:execute_firewall_cmd).with(['--state'], nil, false, false, false).returns(Object.any_instance.stubs(exitstatus: 0)) # rubocop:disable RSpec/AnyInstance - provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=white'], nil, true).returns('white + provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=white'], nil, false).returns('white type: hash:ip options: maxelem=200 family=inet6 entries:') - provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=black'], nil, true).returns('black + provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=black'], nil, false).returns('black type: hash:ip options: maxelem=400 family=inet hashsize=2048 entries:') - provider.class.stubs(:execute_firewall_cmd).with(['--ipset=white', '--get-entries'], nil, true).returns('') + provider.class.stubs(:execute_firewall_cmd).with(['--ipset=white', '--get-entries'], nil, false).returns('') end describe 'self.instances' do From 2e364dd8dfdb7cbe19e6e0832f2cf41cfd470dcd Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Mon, 27 Apr 2020 11:44:50 +0200 Subject: [PATCH 20/24] Revert travis.yml to its master's content --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8df2fc72..6ab7aa23 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,10 @@ matrix: - rvm: 2.4.4 bundler_args: --without system_tests development release env: PUPPET_VERSION="~> 5.0" CHECK=build DEPLOY_TO_FORGE=yes - +branches: + only: + - master + - /^v\d/ notifications: email: false webhooks: https://voxpupu.li/incoming/travis From 37359fda9caa3d0953ffff1e0700f81bb48a76a7 Mon Sep 17 00:00:00 2001 From: iqouiqa Date: Mon, 27 Apr 2020 11:46:21 +0200 Subject: [PATCH 21/24] Revert travis.yml to its master's content --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6ab7aa23..2b897eff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,8 +24,8 @@ matrix: env: PUPPET_VERSION="~> 5.0" CHECK=build DEPLOY_TO_FORGE=yes branches: only: - - master - - /^v\d/ + - master + - /^v\d/ notifications: email: false webhooks: https://voxpupu.li/incoming/travis From 883703dea4b38adc7844127527396ef5fa26ae39 Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 8 Jul 2020 22:37:58 +0200 Subject: [PATCH 22/24] build all refs --- .travis.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2b897eff..8df2fc72 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,10 +22,7 @@ matrix: - rvm: 2.4.4 bundler_args: --without system_tests development release env: PUPPET_VERSION="~> 5.0" CHECK=build DEPLOY_TO_FORGE=yes -branches: - only: - - master - - /^v\d/ + notifications: email: false webhooks: https://voxpupu.li/incoming/travis From 36a0fb4cfb44fce4e620a3ba405c70c229154337 Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 8 Jul 2020 22:41:55 +0200 Subject: [PATCH 23/24] fix failing tests --- .../puppet/type/firewalld_rich_rule_spec.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb index 160c8167..3a77a8cd 100644 --- a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb @@ -189,18 +189,33 @@ it 'queries the status' do fakeclass.stubs(:exitstatus).returns(0) provider.expects(:execute_firewall_cmd).with(['--query-rich-rule', rawrule], 'restricted', true, false).returns(fakeclass) + provider.expects(:execute_firewall_cmd).with(['--query-rich-rule', rawrule], 'restricted', false, false).returns(fakeclass) expect(provider).to be_exists end - it 'creates' do + it 'add rich rule executed when rule does not exist in permanent' do + provider.in_perm = false provider.expects(:execute_firewall_cmd).with(['--add-rich-rule', rawrule]) provider.create end - it 'destroys' do + it 'remove rich rule executed when rule does exist in permanent' do + provider.in_perm = true provider.expects(:execute_firewall_cmd).with(['--remove-rich-rule', rawrule]) provider.destroy end + + it 'add rich rule does not execute when exist in permanent' do + provider.in_perm = true + provider.expects(:execute_firewall_cmd).with(['--add-rich-rule', rawrule]).never + provider.create + end + + it 'remove rich rule does not execute when rule does not exist in permanent' do + provider.in_perm = false + provider.expects(:execute_firewall_cmd).with(['--remove-rich-rule', rawrule]).never + provider.destroy + end end end end From 4fa4690862e5d08da65bce70db8226e2386118ed Mon Sep 17 00:00:00 2001 From: isqo Date: Wed, 8 Jul 2020 22:45:16 +0200 Subject: [PATCH 24/24] build only master and tags --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8df2fc72..2b897eff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,10 @@ matrix: - rvm: 2.4.4 bundler_args: --without system_tests development release env: PUPPET_VERSION="~> 5.0" CHECK=build DEPLOY_TO_FORGE=yes - +branches: + only: + - master + - /^v\d/ notifications: email: false webhooks: https://voxpupu.li/incoming/travis