From 2404ab90d1e4894008270707db512f47c68c599b Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Fri, 3 Jun 2022 22:11:47 +0200 Subject: [PATCH] rubocop: autofix --- .rubocop.yml | 2 + .rubocop_todo.yml | 34 +++++++++++ lib/facter/firewalld_version.rb | 2 + lib/puppet/provider/firewalld.rb | 13 ++--- .../firewalld_custom_service/firewall_cmd.rb | 58 ++++++++----------- .../firewalld_direct_chain/firewall_cmd.rb | 2 + .../firewall_cmd.rb | 2 + .../firewalld_direct_purge/firewall_cmd.rb | 8 ++- .../firewalld_direct_rule/firewall_cmd.rb | 2 + .../provider/firewalld_ipset/firewall_cmd.rb | 8 ++- .../provider/firewalld_port/firewall_cmd.rb | 2 + .../firewalld_rich_rule/firewall_cmd.rb | 14 +++-- .../firewalld_service/firewall_cmd.rb | 4 +- .../provider/firewalld_zone/firewall_cmd.rb | 26 +++++---- lib/puppet/type/firewalld_custom_service.rb | 14 +++-- lib/puppet/type/firewalld_direct_chain.rb | 2 + .../type/firewalld_direct_passthrough.rb | 2 + lib/puppet/type/firewalld_direct_purge.rb | 11 ++-- lib/puppet/type/firewalld_direct_rule.rb | 2 + lib/puppet/type/firewalld_ipset.rb | 10 ++-- lib/puppet/type/firewalld_port.rb | 2 + lib/puppet/type/firewalld_rich_rule.rb | 16 +++-- lib/puppet/type/firewalld_service.rb | 4 +- lib/puppet/type/firewalld_zone.rb | 21 ++++--- .../suites/default/00_default_spec.rb | 2 + spec/classes/init_spec.rb | 8 ++- spec/classes/reload/complete_spec.rb | 2 + spec/classes/reload_spec.rb | 2 + spec/defines/custom_service_spec.rb | 2 + spec/functions/safe_filename_spec.rb | 6 +- spec/spec_helper_acceptance.rb | 8 +-- .../provider/firewalld_custom_service_spec.rb | 2 + .../puppet/provider/firewalld_ipset_spec.rb | 7 +++ .../provider/firewalld_rich_rule_spec.rb | 3 + .../puppet/provider/firewalld_zone_spec.rb | 4 +- .../type/firewalld_custom_service_spec.rb | 18 +++--- .../type/firewalld_direct_chain_spec.rb | 10 +++- .../type/firewalld_direct_passthrough_spec.rb | 7 ++- .../puppet/type/firewalld_direct_rule_spec.rb | 6 +- spec/unit/puppet/type/firewalld_ipset_spec.rb | 35 ++++++----- spec/unit/puppet/type/firewalld_port_spec.rb | 6 +- .../puppet/type/firewalld_rich_rule_spec.rb | 37 +++++++----- .../puppet/type/firewalld_service_spec.rb | 6 +- spec/unit/puppet/type/firewalld_zone_spec.rb | 15 +++-- 44 files changed, 289 insertions(+), 158 deletions(-) create mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml index 53ac1898..ea22bff8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,4 +1,6 @@ --- +inherit_from: .rubocop_todo.yml + # Managed by modulesync - DO NOT EDIT # https://voxpupuli.org/docs/updating-files-managed-with-modulesync/ diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 00000000..1724acef --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,34 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2022-06-03 20:21:58 UTC using RuboCop version 1.22.3. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# Offense count: 1 +# Configuration parameters: AllowComments, AllowEmptyLambdas. +Lint/EmptyBlock: + Exclude: + - 'lib/puppet/type/firewalld_direct_purge.rb' + +# Offense count: 3 +# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames. +# AllowedNames: at, by, db, id, in, io, ip, of, on, os, pp, to +Naming/MethodParameterName: + Exclude: + - 'lib/puppet/provider/firewalld_zone/firewall_cmd.rb' + - 'lib/puppet/type/firewalld_ipset.rb' + +# Offense count: 1 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle. +# SupportedStyles: always, always_true, never +Style/FrozenStringLiteralComment: + Exclude: + - 'spec/unit/facter/firewalld_version_spec.rb' + +# Offense count: 1 +Style/MixinUsage: + Exclude: + - 'spec/spec_helper_acceptance.rb' diff --git a/lib/facter/firewalld_version.rb b/lib/facter/firewalld_version.rb index a0fc5196..a33b7a38 100644 --- a/lib/facter/firewalld_version.rb +++ b/lib/facter/firewalld_version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Return the version of firewalld that is installed Facter.add(:firewalld_version) do confine { Process.uid.zero? } diff --git a/lib/puppet/provider/firewalld.rb b/lib/puppet/provider/firewalld.rb index 017da166..9bb30fa5 100644 --- a/lib/puppet/provider/firewalld.rb +++ b/lib/puppet/provider/firewalld.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require 'puppet/type' require 'puppet/provider' @@ -6,8 +8,7 @@ class Puppet::Provider::Firewalld < Puppet::Provider @runstate = nil class << self - attr_accessor :running - attr_accessor :runstate + attr_accessor :running, :runstate end def state @@ -15,9 +16,7 @@ def state end def self.state - if Puppet::Provider::Firewalld.runstate.nil? - Puppet::Provider::Firewalld.runstate = check_running_state - end + Puppet::Provider::Firewalld.runstate = check_running_state if Puppet::Provider::Firewalld.runstate.nil? Puppet::Provider::Firewalld.runstate end @@ -39,7 +38,7 @@ def self.check_running_state # See: https://github.com/crayfishx/puppet-firewalld/issues/96 # debug('Could not determine state of firewalld because the executable is not available') - return nil + nil end # v3.0.0 @@ -83,7 +82,7 @@ def execute_firewall_cmd(args, zone = @resource[:zone], perm = true, failonfail # def parse_args(args) args = args.flatten.join(' ') if args.is_a?(Array) - args.split(%r{(\'[^\']*\'| )}).reject { |r| ['', ' '].include?(r) } + args.split(%r{('[^']*'| )}).reject { |r| ['', ' '].include?(r) } end # Occasionally we need to restart firewalld in a transient way between resources diff --git a/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb b/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb index 14ad0b9d..f2ea1a9b 100644 --- a/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') @@ -12,11 +14,9 @@ def exists? builtin = true - found_resource = execute_firewall_cmd(['--get-services'], nil).strip.split(' ').include?(@resource[:name]) + found_resource = execute_firewall_cmd(['--get-services'], nil).strip.split.include?(@resource[:name]) - if found_resource && execute_firewall_cmd(['--path-service', @resource[:name]], nil).start_with?('/etc') - builtin = false - end + builtin = false if found_resource && execute_firewall_cmd(['--path-service', @resource[:name]], nil).start_with?('/etc') return false if builtin && (@resource[:ensure] == :absent) @@ -92,14 +92,12 @@ def ports=(should) end end - to_remove .each do |entry| - begin - port_str = "#{entry['port']}/#{entry['protocol']}" + to_remove.each do |entry| + port_str = "#{entry['port']}/#{entry['protocol']}" - execute_firewall_cmd(['--service', @resource[:name], '--remove-port', port_str], nil) - rescue Puppet::ExecutionFailure => e - errors << "Could not remove port '#{port_str} from #{@resource[:name]}' => #{e}" - end + execute_firewall_cmd(['--service', @resource[:name], '--remove-port', port_str], nil) + rescue Puppet::ExecutionFailure => e + errors << "Could not remove port '#{port_str} from #{@resource[:name]}' => #{e}" end raise Puppet::ResourceError, errors.join("\n") unless errors.empty? @@ -120,27 +118,21 @@ def protocols=(should) else to_remove = @property_hash[:protocols] - should ports_protos = [] - unless @resource[:ports].include?(:unset) - ports_protos = Array(@resource[:ports]).select { |x| x['port'].nil? }.map { |x| x['protocol'] } - end + ports_protos = Array(@resource[:ports]).select { |x| x['port'].nil? }.map { |x| x['protocol'] } unless @resource[:ports].include?(:unset) to_add = (should + ports_protos) - @property_hash[:protocols] end errors = [] to_add.each do |entry| - begin - execute_firewall_cmd(['--service', @resource[:name], '--add-protocol', entry], nil) - rescue Puppet::ExecutionFailure => e - errors << "Could not add protocol '#{entry} to #{@resource[:name]}' => #{e}" - end + execute_firewall_cmd(['--service', @resource[:name], '--add-protocol', entry], nil) + rescue Puppet::ExecutionFailure => e + errors << "Could not add protocol '#{entry} to #{@resource[:name]}' => #{e}" end to_remove.each do |entry| - begin - execute_firewall_cmd(['--service', @resource[:name], '--remove-protocol', entry], nil) - rescue Puppet::ExecutionFailure => e - errors << "Could not remove protocol'#{entry} from #{@resource[:name]}' => #{e}" - end + execute_firewall_cmd(['--service', @resource[:name], '--remove-protocol', entry], nil) + rescue Puppet::ExecutionFailure => e + errors << "Could not remove protocol'#{entry} from #{@resource[:name]}' => #{e}" end raise Puppet::ResourceError, errors.join("\n") unless errors.empty? @@ -165,19 +157,15 @@ def modules=(should) errors = [] to_add.each do |entry| - begin - execute_firewall_cmd(['--service', @resource[:name], '--add-module', entry], nil) - rescue Puppet::ExecutionFailure => e - errors << "Could not add module '#{entry} to #{@resource[:name]}' => #{e}" - end + execute_firewall_cmd(['--service', @resource[:name], '--add-module', entry], nil) + rescue Puppet::ExecutionFailure => e + errors << "Could not add module '#{entry} to #{@resource[:name]}' => #{e}" end to_remove.each do |entry| - begin - execute_firewall_cmd(['--service', @resource[:name], '--remove-module', entry], nil) - rescue Puppet::ExecutionFailure => e - errors << "Could not remove module '#{entry} from #{@resource[:name]}' => #{e}" - end + execute_firewall_cmd(['--service', @resource[:name], '--remove-module', entry], nil) + rescue Puppet::ExecutionFailure => e + errors << "Could not remove module '#{entry} from #{@resource[:name]}' => #{e}" end raise Puppet::ResourceError, errors.join("\n") unless errors.empty? @@ -230,7 +218,7 @@ def destinations return @destinations if @destinations @destinations = execute_firewall_cmd(['--service', @resource[:name], '--get-destinations'], nil).strip.split(%r{\s+}) - @destinations = Hash[@destinations.map { |x| x.split(':', 2) }] + @destinations = @destinations.map { |x| x.split(':', 2) }.to_h @destinations end diff --git a/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb index bacad590..bdb59204 100644 --- a/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_direct_chain/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') diff --git a/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb index 3ca7130e..dc356f5c 100644 --- a/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_direct_passthrough/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') diff --git a/lib/puppet/provider/firewalld_direct_purge/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_purge/firewall_cmd.rb index daf3e194..c5a49a77 100644 --- a/lib/puppet/provider/firewalld_direct_purge/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_direct_purge/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') @@ -8,14 +10,16 @@ desc 'Meta provider to the firewalld_direct_purge type' def get_instances_of(restype) - raise Puppet::Error, "Unknown type #{restype}" unless [:chain, :passthrough, :rule].include?(restype) + raise Puppet::Error, "Unknown type #{restype}" unless %i[chain passthrough rule].include?(restype) + perm = execute_firewall_cmd(['--direct', "--get-all-#{restype}s"], nil).split(%r{\n}) curr = execute_firewall_cmd(['--direct', "--get-all-#{restype}s"], nil, false).split(%r{\n}) [perm, curr].flatten.uniq end def purge_resources(restype, args) - raise Puppet::Error, "Unknown type #{restype}" unless [:chain, :passthrough, :rule].include?(restype) + raise Puppet::Error, "Unknown type #{restype}" unless %i[chain passthrough rule].include?(restype) + execute_firewall_cmd(['--direct', "--remove-#{restype}", parse_args(args)], nil) end end diff --git a/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb index c0e2cd55..29fd2f03 100644 --- a/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_direct_rule/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') diff --git a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb b/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb index 69605b56..c9f671e7 100644 --- a/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') @@ -10,13 +12,13 @@ mk_resource_methods def self.instances - ipset_ids = execute_firewall_cmd(['--get-ipsets'], nil).split(' ') + ipset_ids = execute_firewall_cmd(['--get-ipsets'], nil).split ipset_ids.map do |ipset_id| ipset_raw = execute_firewall_cmd(["--info-ipset=#{ipset_id}"], nil) raw_options = ipset_raw.match(%r{options: (.*)}) options = {} if raw_options - raw_options[1].split(' ').each do |v| + raw_options[1].split.each do |v| k, v = v.split('=') options[k.to_sym] = v end @@ -61,7 +63,7 @@ def create @resource[:entries].each { |e| add_entry(e) } if @resource[:manage_entries] end - [:type, :maxelem, :family, :hashsize, :timeout].each do |method| + %i[type maxelem family hashsize timeout].each do |method| define_method("#{method}=") do |should| info("Destroying and creating ipset #{@resource[:name]}") destroy diff --git a/lib/puppet/provider/firewalld_port/firewall_cmd.rb b/lib/puppet/provider/firewalld_port/firewall_cmd.rb index 9e56457c..9b0fbc18 100644 --- a/lib/puppet/provider/firewalld_port/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_port/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') diff --git a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb index 3112f55b..c7217d19 100644 --- a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') @@ -26,6 +28,7 @@ def key_val_opt(opt, resource_param = opt) def eval_source args = [] return [] unless (addr = @resource[:source]) + invert = addr['invert'] ? ' NOT' : '' args << "source#{invert}" args << quote_keyval('address', addr['address']) @@ -36,6 +39,7 @@ def eval_source def eval_dest args = [] return [] unless (addr = @resource[:dest]) + invert = addr['invert'] ? ' NOT' : '' args << "destination#{invert}" args << quote_keyval('address', addr['address']) @@ -44,7 +48,7 @@ def eval_dest end def elements - [:service, :port, :protocol, :icmp_block, :icmp_type, :masquerade, :forward_port] + %i[service port protocol icmp_block icmp_type masquerade forward_port] end def eval_element @@ -76,6 +80,7 @@ def eval_element def eval_log return [] unless @resource[:log] + args = [] args << 'log' if @resource[:log].is_a?(Hash) @@ -88,16 +93,16 @@ def eval_log def eval_audit return [] unless @resource[:audit] + args = [] args << 'audit' - if @resource[:audit].is_a?(Hash) - args << quote_keyval('limit value', @resource[:log]['limit']) - end + args << quote_keyval('limit value', @resource[:log]['limit']) if @resource[:audit].is_a?(Hash) args end def eval_action return [] unless (action = @resource[:action]) + args = [] if action.is_a?(Hash) args << action[:action] @@ -109,6 +114,7 @@ def eval_action def build_rich_rule return @resource[:raw_rule] if @resource[:raw_rule] + rule = ['rule'] rule << [ key_val_opt('family'), diff --git a/lib/puppet/provider/firewalld_service/firewall_cmd.rb b/lib/puppet/provider/firewalld_service/firewall_cmd.rb index 8e98aa97..db0e7914 100644 --- a/lib/puppet/provider/firewalld_service/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_service/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') @@ -8,7 +10,7 @@ desc 'Interact with firewall-cmd' def exists? - execute_firewall_cmd(['--list-services']).split(' ').include?(@resource[:service]) + execute_firewall_cmd(['--list-services']).split.include?(@resource[:service]) end def create diff --git a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb index ab412a10..009d2b79 100644 --- a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require 'puppet/type' require File.join(File.dirname(__FILE__), '..', 'firewalld.rb') @@ -10,7 +12,7 @@ def exists? @resource[:zone] = @resource[:name] - execute_firewall_cmd(['--get-zones'], nil).split(' ').include?(@resource[:name]) + execute_firewall_cmd(['--get-zones'], nil).split.include?(@resource[:name]) end def create @@ -36,6 +38,7 @@ def target # %% depending on the version. See: # https://github.com/crayfishx/puppet-firewalld/issues/111 return @resource[:target] if @resource[:target].delete('%') == zone_target + zone_target end @@ -45,7 +48,7 @@ def target=(_t) end def interfaces - execute_firewall_cmd(['--list-interfaces']).chomp.split(' ') || [] + execute_firewall_cmd(['--list-interfaces']).chomp.split || [] end def interfaces=(new_interfaces) @@ -62,7 +65,7 @@ def interfaces=(new_interfaces) end def sources - execute_firewall_cmd(['--list-sources']).chomp.split(' ').sort || [] + execute_firewall_cmd(['--list-sources']).chomp.split.sort || [] end def sources=(new_sources) @@ -104,7 +107,7 @@ def icmp_blocks=(i) remove_blocks = [] case i - when Array then + when Array get_icmp_blocks.each do |remove_block| unless i.include?(remove_block) debug("removing block #{remove_block} from zone #{@resource[:name]}") @@ -114,6 +117,7 @@ def icmp_blocks=(i) i.each do |block| raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' unless block.is_a?(String) + if get_icmp_types.include?(block) debug("adding block #{block} to zone #{@resource[:name]}") set_blocks.push(block) @@ -122,7 +126,7 @@ def icmp_blocks=(i) raise Puppet::Error, "#{block} is not a valid icmp type on this system! Valid types are: #{valid_types}" end end - when String then + when String get_icmp_blocks.reject { |x| x == i }.each do |remove_block| debug("removing block #{remove_block} from zone #{@resource[:name]}") remove_blocks.push(remove_block) @@ -157,14 +161,14 @@ def get_rules end def get_services - perm = execute_firewall_cmd(['--list-services']).split(' ') - curr = execute_firewall_cmd(['--list-services'], @resource[:name], false).split(' ') + perm = execute_firewall_cmd(['--list-services']).split + curr = execute_firewall_cmd(['--list-services'], @resource[:name], false).split [perm, curr].flatten.uniq end def get_ports - perm = execute_firewall_cmd(['--list-ports']).split(' ') - curr = execute_firewall_cmd(['--list-ports'], @resource[:name], false).split(' ') + perm = execute_firewall_cmd(['--list-ports']).split + curr = execute_firewall_cmd(['--list-ports'], @resource[:name], false).split [perm, curr].flatten.uniq.map do |entry| port, protocol = entry.split(%r{/}) @@ -174,11 +178,11 @@ def get_ports end def get_icmp_blocks - execute_firewall_cmd(['--list-icmp-blocks']).split(' ').sort + execute_firewall_cmd(['--list-icmp-blocks']).split.sort end def get_icmp_types - execute_firewall_cmd(['--get-icmptypes'], nil).split(' ') + execute_firewall_cmd(['--get-icmptypes'], nil).split end # rubocop:enable Style/AccessorMethodName diff --git a/lib/puppet/type/firewalld_custom_service.rb b/lib/puppet/type/firewalld_custom_service.rb index d4dd7b6c..05f0e698 100644 --- a/lib/puppet/type/firewalld_custom_service.rb +++ b/lib/puppet/type/firewalld_custom_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' Puppet::Type.newtype(:firewalld_custom_service) do @@ -52,7 +54,7 @@ if value.is_a?(Hash) # Handle the legacy format from the module translate : to - - value = Hash[value.map { |k, v| [k, v.to_s.tr(':', '-')] }] + value = value.transform_values { |v| v.to_s.tr(':', '-') } else port, protocol = value.split('/') @@ -91,9 +93,7 @@ end allowed_protocols = %w[tcp udp sctp dccp] - unless allowed_protocols.include?(value['protocol']) - raise Puppet::ParseError, "The protocol must be one of '#{allowed_protocols.join(', ')}'. Got '#{value['protocol']}'" - end + raise Puppet::ParseError, "The protocol must be one of '#{allowed_protocols.join(', ')}'. Got '#{value['protocol']}'" unless allowed_protocols.include?(value['protocol']) end end @@ -165,13 +165,14 @@ def insync?(is) addr = IPAddr.new(value) raise Puppet::ParseError, "#{value} is not an IPv4 address" unless addr.ipv4? - rescue => e + rescue StandardError => e raise Puppet::ParseError, e.to_s end end def insync?(is) return true if should == :unset && is.empty? + is == should end end @@ -190,13 +191,14 @@ def insync?(is) addr = IPAddr.new(value) raise Puppet::ParseError, "#{value} is not an IPv6 address" unless addr.ipv6? - rescue => e + rescue StandardError => e raise Puppet::ParseError, e.to_s end end def insync?(is) return true if should == :unset && is.empty? + is == should end end diff --git a/lib/puppet/type/firewalld_direct_chain.rb b/lib/puppet/type/firewalld_direct_chain.rb index a0193f2b..aa3c715d 100644 --- a/lib/puppet/type/firewalld_direct_chain.rb +++ b/lib/puppet/type/firewalld_direct_chain.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' Puppet::Type.newtype(:firewalld_direct_chain) do diff --git a/lib/puppet/type/firewalld_direct_passthrough.rb b/lib/puppet/type/firewalld_direct_passthrough.rb index ccb4530a..7ac2c0a2 100644 --- a/lib/puppet/type/firewalld_direct_passthrough.rb +++ b/lib/puppet/type/firewalld_direct_passthrough.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' Puppet::Type.newtype(:firewalld_direct_passthrough) do diff --git a/lib/puppet/type/firewalld_direct_purge.rb b/lib/puppet/type/firewalld_direct_purge.rb index db2808a3..e47f4219 100644 --- a/lib/puppet/type/firewalld_direct_purge.rb +++ b/lib/puppet/type/firewalld_direct_purge.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require 'puppet/parameter/boolean' @@ -75,17 +77,14 @@ def purge_resources puppet_rules = [] catalog.resources.select { |r| r.is_a?(klass) }.each do |res| - unless res.provider.respond_to?(:generate_raw) - raise Puppet::Error, "Provider for #{resource_type} doesn't support generate_raw method" - end + raise Puppet::Error, "Provider for #{resource_type} doesn't support generate_raw method" unless res.provider.respond_to?(:generate_raw) + puppet_rules << res.provider.generate_raw.join(' ') end provider.get_instances_of(resource_type).reject { |i| puppet_rules.include?(i) }.each do |inst| @purge_resources << inst - unless Puppet.settings[:noop] || self[:noop] - provider.purge_resources(resource_type, inst.split(%r{ })) - end + provider.purge_resources(resource_type, inst.split(%r{ })) unless Puppet.settings[:noop] || self[:noop] end end end diff --git a/lib/puppet/type/firewalld_direct_rule.rb b/lib/puppet/type/firewalld_direct_rule.rb index a0d6fe0a..20b45a01 100644 --- a/lib/puppet/type/firewalld_direct_rule.rb +++ b/lib/puppet/type/firewalld_direct_rule.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' Puppet::Type.newtype(:firewalld_direct_rule) do diff --git a/lib/puppet/type/firewalld_ipset.rb b/lib/puppet/type/firewalld_ipset.rb index 6a019987..71fa63fb 100644 --- a/lib/puppet/type/firewalld_ipset.rb +++ b/lib/puppet/type/firewalld_ipset.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Puppet::Type.newtype(:firewalld_ipset) do @doc = " Configure IPsets in Firewalld @@ -25,7 +27,7 @@ def po2?(n) newparam(:name, namevar: true) do desc 'Name of the IPset' validate do |val| - raise Puppet::Error, 'IPset name must be a word with no spaces' unless val =~ %r{^[\w\.-]+$} + raise Puppet::Error, 'IPset name must be a word with no spaces' unless val =~ %r{^[\w.-]+$} end end @@ -71,7 +73,7 @@ def change_to_s(current, desire) desc 'Initial hash size of the IPSet' validate do |value| raise ArgumentError, 'hashsize must be an integer' unless Puppet::Pops::Types::TypeParser.singleton.parse('Init[Integer]').instance?(value) - raise ArgumentError, 'hashsize must be a positive integer' unless value.to_i > 0 + raise ArgumentError, 'hashsize must be a positive integer' unless value.to_i.positive? raise ArgumentError, 'hashsize must be a power of 2' unless @resource.po2?(value.to_i) end end @@ -92,9 +94,7 @@ def change_to_s(current, desire) end validate do - if !(self[:manage_entries]) && self[:entries] - raise(Puppet::Error, "Ipset should not declare entries if it doesn't manage entries") - end + raise(Puppet::Error, "Ipset should not declare entries if it doesn't manage entries") if !(self[:manage_entries]) && self[:entries] end autorequire(:service) do diff --git a/lib/puppet/type/firewalld_port.rb b/lib/puppet/type/firewalld_port.rb index 8d25f852..db9683dd 100644 --- a/lib/puppet/type/firewalld_port.rb +++ b/lib/puppet/type/firewalld_port.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' Puppet::Type.newtype(:firewalld_port) do diff --git a/lib/puppet/type/firewalld_rich_rule.rb b/lib/puppet/type/firewalld_rich_rule.rb index 447f3da8..2b4ecbef 100644 --- a/lib/puppet/type/firewalld_rich_rule.rb +++ b/lib/puppet/type/firewalld_rich_rule.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Puppet::Type.newtype(:firewalld_rich_rule) do @doc = "Manages firewalld rich rules. @@ -44,6 +46,7 @@ else errormsg = 'Only one source type address or ipset may be specified.' raise errormsg if value.key?('address') && value.key?('ipset') + value end end @@ -56,6 +59,7 @@ else errormsg = 'Only one source type address or ipset may be specified.' raise errormsg if value.key?('address') && value.key?('ipset') + value end end @@ -102,12 +106,12 @@ def _validate_action(value) raise Puppet::Error, "Authorized action values are `accept`, `reject`, `drop` or `mark`, got #{value}" unless %w[accept drop reject mark].include? value end validate do |value| - if value.is_a?(Hash) - if value.keys.sort != [:action, :type] - raise Puppet::Error, "Rule action hash should contain `action` and `type` keys. Use a string if you only want to declare the action to be `accept` or `reject`. Got #{value}" - end + case value + when Hash + raise Puppet::Error, "Rule action hash should contain `action` and `type` keys. Use a string if you only want to declare the action to be `accept` or `reject`. Got #{value}" if value.keys.sort != %i[action type] + _validate_action(value[:action]) - elsif value.is_a?(String) + when String _validate_action(value) end end @@ -119,7 +123,7 @@ def _validate_action(value) end def elements - [:service, :port, :protocol, :icmp_block, :icmp_type, :masquerade, :forward_port] + %i[service port protocol icmp_block icmp_type masquerade forward_port] end validate do diff --git a/lib/puppet/type/firewalld_service.rb b/lib/puppet/type/firewalld_service.rb index 3cc6bffb..b993512a 100644 --- a/lib/puppet/type/firewalld_service.rb +++ b/lib/puppet/type/firewalld_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' Puppet::Type.newtype(:firewalld_service) do @@ -53,6 +55,6 @@ end autorequire(:firewalld_custom_service) do - self[:service].gsub(%r{[^\w-]}, '_') if self[:service] + self[:service]&.gsub(%r{[^\w-]}, '_') end end diff --git a/lib/puppet/type/firewalld_zone.rb b/lib/puppet/type/firewalld_zone.rb index 49f63f4c..8b42e617 100644 --- a/lib/puppet/type/firewalld_zone.rb +++ b/lib/puppet/type/firewalld_zone.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require 'puppet/parameter/boolean' @@ -44,6 +46,7 @@ def generate return [] unless Puppet::Provider::Firewalld.available? + purge_rich_rules if self[:purge_rich_rules] == :true purge_services if self[:purge_services] == :true purge_ports if self[:purge_ports] == :true @@ -100,12 +103,12 @@ def insync?(is) end end - def is_to_s(value = []) # rubocop:disable Style/PredicateName - '[' + value.join(', ') + ']' + def is_to_s(value = []) + "[#{value.join(', ')}]" end def should_to_s(value = []) - '[' + value.join(', ') + ']' + "[#{value.join(', ')}]" end end @@ -133,6 +136,7 @@ def insync?(is) def retrieve return :false if @resource[:purge_rich_rules] == :false + provider.resource.rich_rules_purgable ? :purgable : :true end end @@ -148,6 +152,7 @@ def retrieve def retrieve return :false if @resource[:purge_services] == :false + provider.resource.services_purgable ? :purgable : :true end end @@ -162,15 +167,14 @@ def retrieve def retrieve return :false if @resource[:purge_ports] == :false + provider.resource.ports_purgable ? :purgable : :true end end validate do - [:zone, :name].each do |attr| - if self[attr] && (self[attr]).to_s.length > 17 - raise(Puppet::Error, "Zone identifier '#{attr}' must be less than 18 characters long") - end + %i[zone name].each do |attr| + raise(Puppet::Error, "Zone identifier '#{attr}' must be less than 18 characters long") if self[attr] && (self[attr]).to_s.length > 17 end end @@ -189,6 +193,7 @@ def purge_resource(res_type) def purge_rich_rules return [] unless provider.exists? + puppet_rules = [] catalog.resources.select { |r| r.is_a?(Puppet::Type::Firewalld_rich_rule) }.each do |fwr| debug("not purging puppet controlled rich rule #{fwr[:name]}") @@ -217,6 +222,7 @@ def purge_rich_rules def purge_services return [] unless provider.exists? + puppet_services = [] catalog.resources.select { |r| r.is_a?(Puppet::Type::Firewalld_service) }.each do |fws| if fws[:zone] == self[:name] @@ -240,6 +246,7 @@ def purge_services def purge_ports return [] unless provider.exists? + puppet_ports = [] catalog.resources.select { |r| r.is_a?(Puppet::Type::Firewalld_port) }.each do |fwp| if fwp[:zone] == self[:name] diff --git a/spec/acceptance/suites/default/00_default_spec.rb b/spec/acceptance/suites/default/00_default_spec.rb index 04dbd937..39aff73e 100644 --- a/spec/acceptance/suites/default/00_default_spec.rb +++ b/spec/acceptance/suites/default/00_default_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper_acceptance' describe 'firewalld', unless: UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb index 36417d88..9c59b6d2 100644 --- a/spec/classes/init_spec.rb +++ b/spec/classes/init_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'puppet/provider/firewalld' @@ -154,10 +156,10 @@ 'Accept SSH from Gondor' => { 'ensure' => 'present', - 'zone' => 'restricted', - 'source' => '192.162.1.0/22', + 'zone' => 'restricted', + 'source' => '192.162.1.0/22', 'service' => 'ssh', - 'action' => 'accept' + 'action' => 'accept' } } } diff --git a/spec/classes/reload/complete_spec.rb b/spec/classes/reload/complete_spec.rb index d66f718c..3e38e6a5 100644 --- a/spec/classes/reload/complete_spec.rb +++ b/spec/classes/reload/complete_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'puppet/provider/firewalld' diff --git a/spec/classes/reload_spec.rb b/spec/classes/reload_spec.rb index bfba7392..b030d82c 100644 --- a/spec/classes/reload_spec.rb +++ b/spec/classes/reload_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'puppet/provider/firewalld' diff --git a/spec/defines/custom_service_spec.rb b/spec/defines/custom_service_spec.rb index a629ab6a..7d3b07fc 100644 --- a/spec/defines/custom_service_spec.rb +++ b/spec/defines/custom_service_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe 'firewalld::custom_service' do diff --git a/spec/functions/safe_filename_spec.rb b/spec/functions/safe_filename_spec.rb index c04bc11f..4aa58d3b 100644 --- a/spec/functions/safe_filename_spec.rb +++ b/spec/functions/safe_filename_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe 'firewalld::safe_filename' do @@ -12,7 +14,7 @@ let(:invalid_filenames) do { 'This Should Work' => 'This_Should_Work', - 'this should work' => 'this_should_work', + 'this should work' => 'this_should_work', 'th1s$Sh0uld w0rk!!' => 'th1s_Sh0uld_w0rk__' } end @@ -20,7 +22,7 @@ let(:filenames_with_options) do { 'ThisShouldWork.test' => 'ThisShouldWork.test', - 'this_should_!work.xml' => 'this_should_--work--xml', + 'this_should_!work.xml' => 'this_should_--work--xml', 'th1s$Sh0uld w0rk!!.test' => 'th1s--Sh0uld--w0rk----.test' } end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index 1cef0730..09bdbdf9 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'beaker-rspec' require 'tmpdir' require 'yaml' @@ -26,9 +28,7 @@ # Configure all nodes in nodeset c.before :suite do - begin - # Install modules and dependencies from spec/fixtures/modules - copy_fixture_modules_to(hosts) - end + # Install modules and dependencies from spec/fixtures/modules + copy_fixture_modules_to(hosts) end end diff --git a/spec/unit/puppet/provider/firewalld_custom_service_spec.rb b/spec/unit/puppet/provider/firewalld_custom_service_spec.rb index 5454390a..98576d95 100644 --- a/spec/unit/puppet/provider/firewalld_custom_service_spec.rb +++ b/spec/unit/puppet/provider/firewalld_custom_service_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' provider_class = Puppet::Type.type(:firewalld_custom_service).provider(:firewall_cmd) diff --git a/spec/unit/puppet/provider/firewalld_ipset_spec.rb b/spec/unit/puppet/provider/firewalld_ipset_spec.rb index 54b1b886..345d55d0 100644 --- a/spec/unit/puppet/provider/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/provider/firewalld_ipset_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' provider_class = Puppet::Type.type(:firewalld_ipset).provider(:firewall_cmd) @@ -34,14 +36,17 @@ ipsets_names = provider.class.instances.map(&:name) expect(ipsets_names).to include('black', 'white') end + it 'with correct families' do ipsets_families = provider.class.instances.map(&:family) expect(ipsets_families).to include('inet', 'inet6') end + it 'with correct hashsizes' do ipsets_hashsize = provider.class.instances.map(&:hashsize) expect(ipsets_hashsize).to include('2048') end + it 'with correct maxelems' do ipsets_maxelem = provider.class.instances.map(&:maxelem) expect(ipsets_maxelem).to include('200', '400') @@ -91,6 +96,7 @@ provider.hashsize = 2048 end end + context 'entries' do it 'removes and add entries' do resource.expects(:[]).with(:name).returns('white').at_least_once @@ -111,6 +117,7 @@ provider.create provider.entries = ['192.168.14.0/24', '10.0.0.0/8'] end + it 'ignores entries when manage_entries is false' do resource.expects(:[]).with(:name).returns('white').at_least_once resource.expects(:[]).with(:type).returns('hash:net').at_least_once diff --git a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb index 1f671607..54553277 100644 --- a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' provider_class = Puppet::Type.type(:firewalld_rich_rule).provider(:firewall_cmd) @@ -43,6 +45,7 @@ expect(provider.build_rich_rule).to eq('rule family="ipv4" source service name="ssh" accept') end end + context 'with reject type' do it 'builds the rich rule' do resource.expects(:[]).with(:source).returns(nil).at_least_once diff --git a/spec/unit/puppet/provider/firewalld_zone_spec.rb b/spec/unit/puppet/provider/firewalld_zone_spec.rb index 15373b8b..4bf2bac6 100644 --- a/spec/unit/puppet/provider/firewalld_zone_spec.rb +++ b/spec/unit/puppet/provider/firewalld_zone_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' provider_class = Puppet::Type.type(:firewalld_zone).provider(:firewall_cmd) @@ -54,7 +56,7 @@ provider.expects(:execute_firewall_cmd).with(['--add-interface', 'eth0']) provider.expects(:execute_firewall_cmd).with(['--new-zone', 'white'], nil) provider.expects(:execute_firewall_cmd).with(['--set-short', 'little description'], 'white', true, false) - provider.expects(:execute_firewall_cmd).with(['--set-description', :"Better description"], 'white', true, false) + provider.expects(:execute_firewall_cmd).with(['--set-description', :'Better description'], 'white', true, false) provider.create provider.description = :'Better description' diff --git a/spec/unit/puppet/type/firewalld_custom_service_spec.rb b/spec/unit/puppet/type/firewalld_custom_service_spec.rb index 6391934a..e25a524f 100644 --- a/spec/unit/puppet/type/firewalld_custom_service_spec.rb +++ b/spec/unit/puppet/type/firewalld_custom_service_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_custom_service) do @@ -40,7 +42,7 @@ name: 'test', short: short ) - end. to raise_error(%r{Valid values match}) + end.to raise_error(%r{Valid values match}) end end @@ -64,7 +66,7 @@ name: 'test', description: description ) - end. to raise_error(%r{Valid values match}) + end.to raise_error(%r{Valid values match}) end end @@ -188,7 +190,7 @@ name: 'test', protocols: protocol ) - end. to raise_error(%r{Valid values match}) + end.to raise_error(%r{Valid values match}) end end end @@ -196,8 +198,8 @@ context ':modules validation' do it 'accepts valid modules' do - modules = ['nf_conntrack_ftp', 'thing', 'other_thing', 'new-thing'] - expected_modules = ['ftp', 'thing', 'other_thing', 'new-thing'] + modules = %w[nf_conntrack_ftp thing other_thing new-thing] + expected_modules = %w[ftp thing other_thing new-thing] resource = described_class.new( name: 'test', @@ -221,7 +223,7 @@ name: 'test', modules: mod ) - end. to raise_error(%r{Valid values match}) + end.to raise_error(%r{Valid values match}) end end end @@ -262,7 +264,7 @@ name: 'test', ipv4_destination: destination ) - end. to raise_error(%r{(invalid address|not an IPv4)}) + end.to raise_error(%r{(invalid address|not an IPv4)}) end end end @@ -304,7 +306,7 @@ name: 'test', ipv6_destination: destination ) - end. to raise_error(%r{(invalid address|not an IPv6)}) + end.to raise_error(%r{(invalid address|not an IPv6)}) end end end diff --git a/spec/unit/puppet/type/firewalld_direct_chain_spec.rb b/spec/unit/puppet/type/firewalld_direct_chain_spec.rb index 172f27b0..5e1a4572 100644 --- a/spec/unit/puppet/type/firewalld_direct_chain_spec.rb +++ b/spec/unit/puppet/type/firewalld_direct_chain_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_direct_chain) do @@ -7,8 +9,8 @@ context 'with no params' do describe 'when validating attributes' do - [:name, :inet_protocol, :table].each do |param| - it "should have a #{param} parameter" do + %i[name inet_protocol table].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end @@ -16,7 +18,7 @@ describe 'namevar validation' do it 'has :name, :inet_protocol and :table as its namevars' do - expect(described_class.key_attributes).to eq([:name, :inet_protocol, :table]) + expect(described_class.key_attributes).to eq(%i[name inet_protocol table]) end it 'uses the title as the name when non-delimited' do @@ -30,9 +32,11 @@ it 'sets resource `name` correctly' do expect(resource.name).to eq('LOG_DROPS') end + it 'sets resource `table` parameter correctly' do expect(resource[:table]).to eq('filter') end + it 'sets resource `inet_protocol` parameter correctly' do expect(resource[:inet_protocol]).to eq('ipv4') 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..b63e96a4 100644 --- a/spec/unit/puppet/type/firewalld_direct_passthrough_spec.rb +++ b/spec/unit/puppet/type/firewalld_direct_passthrough_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_direct_passthrough) do @@ -7,8 +9,8 @@ context 'with no params' do describe 'when validating attributes' do - [:inet_protocol, :args].each do |param| - it "should have a #{param} parameter" do + %i[inet_protocol args].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end @@ -46,6 +48,7 @@ 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 diff --git a/spec/unit/puppet/type/firewalld_direct_rule_spec.rb b/spec/unit/puppet/type/firewalld_direct_rule_spec.rb index 2e6df270..df9a9741 100644 --- a/spec/unit/puppet/type/firewalld_direct_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_direct_rule_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_direct_rule) do @@ -7,8 +9,8 @@ context 'with no params' do describe 'when validating attributes' do - [:inet_protocol, :args, :table, :chain, :priority].each do |param| - it "should have a #{param} parameter" do + %i[inet_protocol args table chain priority].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end diff --git a/spec/unit/puppet/type/firewalld_ipset_spec.rb b/spec/unit/puppet/type/firewalld_ipset_spec.rb index bf785851..a90fc036 100644 --- a/spec/unit/puppet/type/firewalld_ipset_spec.rb +++ b/spec/unit/puppet/type/firewalld_ipset_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_ipset) do @@ -7,66 +9,70 @@ describe 'type' do describe 'when validating attributes' do - [:name, :type, :options, :manage_entries].each do |param| - it "should have a #{param} parameter" do + %i[name type options manage_entries].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end - [:entries, :family, :hashsize, :maxelem, :timeout].each do |prop| - it "should have a #{prop} property" do + %i[entries family hashsize maxelem timeout].each do |prop| + it "has a #{prop} property" do expect(described_class.attrtype(prop)).to eq(:property) end end end + describe 'when validating attribute values' do describe 'hashsize' do [128, 256, 512, 1024, '1024'].each do |value| - it "should support #{value} as a value to hashsize" do + it "supports #{value} as a value to hashsize" do expect { described_class.new(name: 'test', hashsize: value) }.not_to raise_error end end ['foo', '3.5'].each do |value| - it "should not support #{value} as a value to hashsize" do + it "does not support #{value} as a value to hashsize" do expect { described_class.new(name: 'test', hashsize: value) }.to raise_error(Puppet::Error, %r{hashsize must be an integer}) end end [0, '0', -1, '-1'].each do |value| - it "should not support #{value} as a value to hashsize" do + it "does not support #{value} as a value to hashsize" do expect { described_class.new(name: 'test', hashsize: value) }.to raise_error(Puppet::Error, %r{hashsize must be a positive integer}) end end [5, 41, '99'].each do |value| - it "should not support #{value} as a value to hashsize" do + it "does not support #{value} as a value to hashsize" do expect { described_class.new(name: 'test', hashsize: value) }.to raise_error(Puppet::Error, %r{hashsize must be a power of 2}) end end end + describe 'maxelem' do [2048, '3000', 65_536].each do |value| - it "should support #{value} as a value to maxelem" do + it "supports #{value} as a value to maxelem" do expect { described_class.new(name: 'test', maxelem: value) }.not_to raise_error end end [0, 'foo', '3.5', -1, 0.6, '-1000.3'].each do |value| - it "should not support #{value} as a value to maxelem" do + it "does not support #{value} as a value to maxelem" do expect { described_class.new(name: 'test', maxelem: value) }.to raise_error(Puppet::Error, %r{Invalid value}) end end end + describe 'timeout' do [0, '0', 60, 3600, '2147483'].each do |value| - it "should support #{value} as a value to timeout" do + it "supports #{value} as a value to timeout" do expect { described_class.new(name: 'test', timeout: value) }.not_to raise_error end end ['foo', '3.5', -1, 0.6, '-1000.3'].each do |value| - it "should not support #{value} as a value to timeout" do + it "does not support #{value} as a value to timeout" do expect { described_class.new(name: 'test', timeout: value) }.to raise_error(Puppet::Error, %r{Invalid value}) end end end end + it 'raises an error if wrong name' do expect do described_class.new( @@ -75,6 +81,7 @@ ) end.to raise_error(%r{IPset name must be a word with no spaces}) end + it 'accept - in name' do expect do described_class.new( @@ -83,6 +90,7 @@ ) end.not_to raise_error end + it 'accept . in name' do expect do described_class.new( @@ -133,6 +141,7 @@ provider.entries = ['192.168.2.2', '10.72.1.100'] end end + context 'change in ipset members' do let(:resource) do Puppet::Type.type(:firewalld_ipset).new( @@ -147,7 +156,7 @@ end end - context 'validation when not managing ipset entries ' do + context 'validation when not managing ipset entries' do it 'raises an error if wrong type' do expect do Puppet::Type.type(:firewalld_ipset).new( diff --git a/spec/unit/puppet/type/firewalld_port_spec.rb b/spec/unit/puppet/type/firewalld_port_spec.rb index 7eaf1545..6ea74f7a 100644 --- a/spec/unit/puppet/type/firewalld_port_spec.rb +++ b/spec/unit/puppet/type/firewalld_port_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_port) do @@ -7,8 +9,8 @@ context 'with no params' do describe 'when validating attributes' do - [:name, :zone, :port, :protocol].each do |param| - it "should have a #{param} parameter" do + %i[name zone port protocol].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end diff --git a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb index 6fd166e4..a99eb8ce 100644 --- a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb @@ -1,28 +1,31 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_rich_rule) do before do Puppet::Provider::Firewalld.any_instance.stubs(:state).returns(:true) # rubocop:disable RSpec/AnyInstance end + context 'with no params' do describe 'when validating attributes' do - [ - :family, - :zone, - :source, - :service, - :action, - :protocol, - :icmp_block, - :icmp_type, - :masquerade, - :forward_port, - :log, - :audit, - :action, - :raw_rule + %i[ + family + zone + source + service + action + protocol + icmp_block + icmp_type + masquerade + forward_port + log + audit + action + raw_rule ].each do |param| - it "should have a #{param} parameter" do + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end @@ -38,6 +41,7 @@ ) end.to raise_error(%r{Authorized action values are `accept`, `reject`, `drop` or `mark`}) end + it 'raises an error if wrong action hash keys' do expect do described_class.new( @@ -46,6 +50,7 @@ ) end.to raise_error(%r{Rule action hash should contain `action` and `type` keys. Use a string if you only want to declare the action to be `accept` or `reject`}) end + it 'raises an error if wrong action hash values' do expect do described_class.new( diff --git a/spec/unit/puppet/type/firewalld_service_spec.rb b/spec/unit/puppet/type/firewalld_service_spec.rb index 5dd0a53f..d333a029 100644 --- a/spec/unit/puppet/type/firewalld_service_spec.rb +++ b/spec/unit/puppet/type/firewalld_service_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_service) do @@ -7,8 +9,8 @@ context 'with no params' do describe 'when validating attributes' do - [:name, :service, :zone].each do |param| - it "should have a #{param} parameter" do + %i[name service zone].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end diff --git a/spec/unit/puppet/type/firewalld_zone_spec.rb b/spec/unit/puppet/type/firewalld_zone_spec.rb index 8ad4ba15..26f024eb 100644 --- a/spec/unit/puppet/type/firewalld_zone_spec.rb +++ b/spec/unit/puppet/type/firewalld_zone_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_zone) do @@ -11,13 +13,13 @@ [ :name ].each do |param| - it "should have a #{param} parameter" do + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end - [:target, :icmp_blocks, :sources, :purge_rich_rules, :purge_services, :purge_ports].each do |param| - it "should have a #{param} parameter" do + %i[target icmp_blocks sources purge_rich_rules purge_services purge_ports].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:property) end end @@ -34,7 +36,7 @@ name: 'restricted', target: '%%REJECT%%', interfaces: ['eth0'], - icmp_blocks: ['redirect', 'router-advertisment'], + icmp_blocks: %w[redirect router-advertisment], sources: ['192.168.2.2', '10.72.1.100'] ) end @@ -66,7 +68,7 @@ provider.expects(:execute_firewall_cmd).with(['--new-zone', 'restricted'], nil) provider.expects(:execute_firewall_cmd).with(['--set-target', '%%REJECT%%']) - provider.expects(:icmp_blocks=).with(['redirect', 'router-advertisment']) + provider.expects(:icmp_blocks=).with(%w[redirect router-advertisment]) provider.expects(:sources).returns([]) provider.expects(:execute_firewall_cmd).with(['--add-source', '192.168.2.2']) @@ -123,7 +125,7 @@ it 'lists icmp types' do provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns('echo-reply echo-request') - expect(provider.get_icmp_types).to eq(['echo-reply', 'echo-request']) + expect(provider.get_icmp_types).to eq(%w[echo-reply echo-request]) end end @@ -153,6 +155,7 @@ provider.expects(:execute_firewall_cmd).with(['--query-masquerade'], 'public', true, false).returns("no\n") expect(provider.masquerade).to eq(:false) end + it 'gets masquerading state as true when set' do provider.expects(:execute_firewall_cmd).with(['--query-masquerade'], 'public', true, false).returns("yes\n") expect(provider.masquerade).to eq(:true)