diff --git a/README.md b/README.md index de1c21f2..e4e6094d 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,7 @@ will produce: * `ipv4_destination`: (Optional) A string specifying the destination network as a network IP address (optional with /mask), or a plain IP - address. + address. The use of hostnames is possible but not recommended, because these will only be resolved at service activation and transmitted to the kernel. @@ -448,7 +448,7 @@ will produce: * `ipv6_destination`: (Optional) A string specifying the destination network as a network IP address (optional with /mask), or a plain IP - address. + address. The use of hostnames is possible but not recommended, because these will only be resolved at service activation and transmitted to the kernel. 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 c3b74916..2ec0838d 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 @@ -88,7 +87,7 @@ def execute_firewall_cmd_policy(args, policy = @resource[:policy], perm = true, # 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_policy/firewall_cmd.rb b/lib/puppet/provider/firewalld_policy/firewall_cmd.rb index fa45ef25..7d488701 100644 --- a/lib/puppet/provider/firewalld_policy/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_policy/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[:policy] = @resource[:name] - execute_firewall_cmd_policy(['--get-policies'], nil).split(' ').include?(@resource[:name]) + execute_firewall_cmd_policy(['--get-policies'], nil).split.include?(@resource[:name]) end def create @@ -37,6 +39,7 @@ def target # %% depending on the version. See: # https://github.com/crayfishx/puppet-firewalld/issues/111 return @resource[:target] if @resource[:target].delete('%') == policy_target + policy_target end @@ -46,7 +49,7 @@ def target=(_t) end def ingress_zones - execute_firewall_cmd_policy(['--list-ingress-zones']).chomp.split(' ') || [] + execute_firewall_cmd_policy(['--list-ingress-zones']).chomp.split || [] end def ingress_zones=(new_ingress_zones) @@ -66,7 +69,7 @@ def ingress_zones=(new_ingress_zones) end def egress_zones - execute_firewall_cmd_policy(['--list-egress-zones']).chomp.split(' ') || [] + execute_firewall_cmd_policy(['--list-egress-zones']).chomp.split || [] end def egress_zones=(new_egress_zones) @@ -121,7 +124,7 @@ def icmp_blocks=(new_icmp_blocks) icmp_types = get_icmp_types case new_icmp_blocks - when Array then + when Array get_icmp_blocks.each do |remove_block| unless new_icmp_blocks.include?(remove_block) debug("removing block #{remove_block} from policy #{@resource[:name]}") @@ -131,6 +134,7 @@ def icmp_blocks=(new_icmp_blocks) new_icmp_blocks.each do |block| raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' unless block.is_a?(String) + if icmp_types.include?(block) debug("adding block #{block} to policy #{@resource[:name]}") set_blocks.push(block) @@ -139,7 +143,7 @@ def icmp_blocks=(new_icmp_blocks) 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 == new_icmp_blocks }.each do |remove_block| debug("removing block #{remove_block} from policy #{@resource[:name]}") remove_blocks.push(remove_block) @@ -174,14 +178,14 @@ def get_rules end def get_services - perm = execute_firewall_cmd_policy(['--list-services']).split(' ') - curr = execute_firewall_cmd_policy(['--list-services'], @resource[:name], false).split(' ') + perm = execute_firewall_cmd_policy(['--list-services']).split + curr = execute_firewall_cmd_policy(['--list-services'], @resource[:name], false).split [perm, curr].flatten.uniq end def get_ports - perm = execute_firewall_cmd_policy(['--list-ports']).split(' ') - curr = execute_firewall_cmd_policy(['--list-ports'], @resource[:name], false).split(' ') + perm = execute_firewall_cmd_policy(['--list-ports']).split + curr = execute_firewall_cmd_policy(['--list-ports'], @resource[:name], false).split [perm, curr].flatten.uniq.map do |entry| port, protocol = entry.split(%r{/}) @@ -191,11 +195,11 @@ def get_ports end def get_icmp_blocks - execute_firewall_cmd_policy(['--list-icmp-blocks']).split(' ').sort + execute_firewall_cmd_policy(['--list-icmp-blocks']).split.sort end def get_icmp_types - execute_firewall_cmd_policy(['--get-icmptypes'], nil).split(' ') + execute_firewall_cmd_policy(['--get-icmptypes'], nil).split end # rubocop:enable Style/AccessorMethodName diff --git a/lib/puppet/provider/firewalld_port/firewall_cmd.rb b/lib/puppet/provider/firewalld_port/firewall_cmd.rb index c12fe80a..41dd4eb4 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 fe0015e7..ed983c9d 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') @@ -33,12 +35,14 @@ def key_val_opt(opt, resource_param = opt) def eval_priority return [] unless (priority = @resource[:priority]) + quote_keyval('priority', priority) end def eval_source args = [] return [] unless (addr = @resource[:source]) + invert = addr['invert'] ? ' NOT' : '' args << "source#{invert}" args << quote_keyval('address', addr['address']) @@ -49,6 +53,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']) @@ -57,7 +62,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 @@ -89,6 +94,7 @@ def eval_element def eval_log return [] unless @resource[:log] + args = [] args << 'log' if @resource[:log].is_a?(Hash) @@ -101,16 +107,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'] @@ -122,6 +128,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 e3775782..b39d6302 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 self.instances - services = execute_firewall_cmd(['--get-services'], nil).split(' ') + services = execute_firewall_cmd(['--get-services'], nil).split services.map do |service| new( { @@ -29,9 +31,9 @@ def self.prefetch(resources) def exists? if @resource[:zone] == :unset - execute_firewall_cmd_policy(['--list-services']).split(' ').include?(@resource[:service]) + execute_firewall_cmd_policy(['--list-services']).split.include?(@resource[:service]) else - execute_firewall_cmd(['--list-services']).split(' ').include?(@resource[:service]) + execute_firewall_cmd(['--list-services']).split.include?(@resource[:service]) end end diff --git a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb index 09948da9..ac871c6b 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) @@ -106,7 +109,7 @@ def icmp_blocks=(new_icmp_blocks) icmp_types = get_icmp_types case new_icmp_blocks - when Array then + when Array get_icmp_blocks.each do |remove_block| unless new_icmp_blocks.include?(remove_block) debug("removing block #{remove_block} from zone #{@resource[:name]}") @@ -116,6 +119,7 @@ def icmp_blocks=(new_icmp_blocks) new_icmp_blocks.each do |block| raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' unless block.is_a?(String) + if icmp_types.include?(block) debug("adding block #{block} to zone #{@resource[:name]}") set_blocks.push(block) @@ -124,7 +128,7 @@ def icmp_blocks=(new_icmp_blocks) 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 == new_icmp_blocks }.each do |remove_block| debug("removing block #{remove_block} from zone #{@resource[:name]}") remove_blocks.push(remove_block) @@ -153,9 +157,9 @@ def icmp_blocks=(new_icmp_blocks) def icmp_block_inversion if execute_firewall_cmd(['--query-icmp-block-inversion'], @resource[:name], true, false).chomp == 'no' - return :false + :false else - return :true + :true end end @@ -176,14 +180,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{/}) @@ -193,11 +197,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_policy.rb b/lib/puppet/type/firewalld_policy.rb index bfa55aa4..e09568c6 100644 --- a/lib/puppet/type/firewalld_policy.rb +++ b/lib/puppet/type/firewalld_policy.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppet' require 'puppet/parameter/boolean' @@ -45,6 +47,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 @@ -82,7 +85,7 @@ def generate def insync?(is) case should when Array then should.sort == is.sort - when :unset then [] == is.sort + when :unset then is.sort == [] else raise Puppet::Error, "parameter #{self.class.name} must be an array of strings!" end end @@ -98,7 +101,7 @@ def insync?(is) def insync?(is) case should when Array then should.sort == is.sort - when :unset then [] == is.sort + when :unset then is.sort == [] else raise Puppet::Error, "parameter #{self.class.name} must be an array of strings!" end end @@ -119,13 +122,11 @@ def insync?(is) validate do |value| begin Integer(value) - rescue + rescue StandardError raise Puppet::Error, 'parameter priority must be a non zero integer' end - if Integer(value).zero? - raise Puppet::Error, 'parameter priority must be non zero' - end + raise Puppet::Error, 'parameter priority must be non zero' if Integer(value).zero? end end @@ -159,6 +160,7 @@ def insync?(is) def retrieve return :false if @resource[:purge_rich_rules] == :false + provider.resource.rich_rules_purgable ? :purgable : :true end end @@ -174,6 +176,7 @@ def retrieve def retrieve return :false if @resource[:purge_services] == :false + provider.resource.services_purgable ? :purgable : :true end end @@ -188,23 +191,20 @@ def retrieve def retrieve return :false if @resource[:purge_ports] == :false + provider.resource.ports_purgable ? :purgable : :true end end def validate_zone_list(attr) - if self[:ensure] == :absent and NilClass === self[attr] + if (self[:ensure] == :absent) && self[attr].is_a?(NilClass) self[attr] = [] return end - if not Array === self[attr] - raise Puppet::Error, "parameter #{attr} must be an array of strings!" - end + raise Puppet::Error, "parameter #{attr} must be an array of strings!" unless self[attr].is_a?(Array) - if self[attr].length.zero? - raise Puppet::Error, "parameter #{attr} must contain at least one zone!" - end + raise Puppet::Error, "parameter #{attr} must contain at least one zone!" if self[attr].length.zero? self[attr].each do |element| case element @@ -215,16 +215,12 @@ def validate_zone_list(attr) return if self[attr].length == 1 - if self[attr].include?('HOST') || self[attr].include?('ANY') - raise Puppet::Error, "parameter #{attr} must contain a single symbolic zone or one or more regular zones" - end + raise Puppet::Error, "parameter #{attr} must contain a single symbolic zone or one or more regular zones" if self[attr].include?('HOST') || self[attr].include?('ANY') end validate do - [:policy, :name].each do |attr| - if self[attr] && (self[attr]).to_s.length > 17 - raise(Puppet::Error, "Policy identifier '#{attr}' must be less than 18 characters long") - end + %i[policy name].each do |attr| + raise(Puppet::Error, "Policy identifier '#{attr}' must be less than 18 characters long") if self[attr] && (self[attr]).to_s.length > 17 end validate_zone_list(:ingress_zones) validate_zone_list(:egress_zones) @@ -235,7 +231,7 @@ def validate_zone_list(attr) end autorequire(:firewalld_zone) do - (self[:ingress_zones] != :unset ? self[:ingress_zones] : []) + (self[:egress_zones] != :unset ? self[:egress_zones] : []) + (self[:ingress_zones] == :unset ? [] : self[:ingress_zones]) + (self[:egress_zones] == :unset ? [] : self[:egress_zones]) end def purge_resource(res_type) @@ -249,6 +245,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| if fwr[:policy] == self[:name] @@ -279,6 +276,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[:policy] == self[:name] @@ -302,6 +300,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[:policy] == self[:name] diff --git a/lib/puppet/type/firewalld_port.rb b/lib/puppet/type/firewalld_port.rb index 051268ed..478217fd 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 @@ -56,13 +58,9 @@ end validate do - if self[:zone] != :unset && self[:policy] != :unset - raise Puppet::Error, 'only one of the parameters zone and policy may be supplied' - end + raise Puppet::Error, 'only one of the parameters zone and policy may be supplied' if self[:zone] != :unset && self[:policy] != :unset - if self[:zone] == :unset && self[:policy] == :unset - raise Puppet::Error, 'one of the parameters zone and policy must be supplied' - end + raise Puppet::Error, 'one of the parameters zone and policy must be supplied' if self[:zone] == :unset && self[:policy] == :unset end autorequire(:firewalld_zone) do diff --git a/lib/puppet/type/firewalld_rich_rule.rb b/lib/puppet/type/firewalld_rich_rule.rb index 170d6ba1..f75f428f 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. @@ -51,7 +53,7 @@ munge(&:to_s) validate do |value| - raise Puppet::Error, 'Priority must be between -32768 and 32767' unless value.to_i.to_s == value.to_s && (-32768..32767).include?(value.to_i) + raise Puppet::Error, 'Priority must be between -32768 and 32767' unless value.to_i.to_s == value.to_s && (-32_768..32_767).include?(value.to_i) end end @@ -63,6 +65,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 @@ -75,6 +78,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 @@ -121,12 +125,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 != %w[action type] + _validate_action(value['action']) - elsif value.is_a?(String) + when String _validate_action(value) end end @@ -139,20 +143,16 @@ 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 errormsg = "Only one element (#{elements.join(',')}) may be specified." raise errormsg if elements.select { |e| self[e] }.size > 1 - if self[:zone] != :unset && self[:policy] != :unset - raise Puppet::Error, 'only one of the parameters zone and policy may be supplied' - end + raise Puppet::Error, 'only one of the parameters zone and policy may be supplied' if self[:zone] != :unset && self[:policy] != :unset - if self[:zone] == :unset && self[:policy] == :unset - raise Puppet::Error, 'one of the parameters zone and policy must be supplied' - end + raise Puppet::Error, 'one of the parameters zone and policy must be supplied' if self[:zone] == :unset && self[:policy] == :unset end autorequire(:firewalld_zone) do diff --git a/lib/puppet/type/firewalld_service.rb b/lib/puppet/type/firewalld_service.rb index fea31471..639a341f 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 @@ -55,13 +57,9 @@ end validate do - if self[:zone] != :unset && self[:policy] != :unset - raise Puppet::Error, 'only one of the parameters zone and policy may be supplied' - end + raise Puppet::Error, 'only one of the parameters zone and policy may be supplied' if self[:zone] != :unset && self[:policy] != :unset - if self[:zone] == :unset && self[:policy] == :unset - raise Puppet::Error, 'one of the parameters zone and policy must be supplied' - end + raise Puppet::Error, 'one of the parameters zone and policy must be supplied' if self[:zone] == :unset && self[:policy] == :unset end autorequire(:firewalld_zone) do @@ -77,6 +75,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 329db79b..a611afc6 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' @@ -45,6 +47,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 @@ -101,12 +104,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 @@ -140,6 +143,7 @@ def insync?(is) def retrieve return :false if @resource[:purge_rich_rules] == :false + provider.resource.rich_rules_purgable ? :purgable : :true end end @@ -155,6 +159,7 @@ def retrieve def retrieve return :false if @resource[:purge_services] == :false + provider.resource.services_purgable ? :purgable : :true end end @@ -169,15 +174,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 @@ -196,6 +200,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| if fwr[:zone] == self[:name] @@ -226,6 +231,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] @@ -249,6 +255,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 08ba79a0..57b81911 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' @@ -12,7 +14,7 @@ } end -# it { pp catalogue.resources } + # it { pp catalogue.resources } context 'with defaults for all parameters' do it { is_expected.to contain_class('firewalld') } @@ -156,10 +158,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/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.rb b/spec/spec_helper.rb index 59041394..e911dbef 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This file is managed via modulesync # https://github.com/voxpupuli/modulesync # https://github.com/voxpupuli/modulesync_config @@ -8,15 +10,13 @@ # puppetlabs_spec_helper will set up coverage if the env variable is set. # We want to do this if lib exists and it hasn't been explicitly set. -ENV['COVERAGE'] ||= 'yes' if Dir.exist?(File.expand_path('../../lib', __FILE__)) +ENV['COVERAGE'] ||= 'yes' if Dir.exist?(File.expand_path('../lib', __dir__)) require 'voxpupuli/test/spec_helper' if File.exist?(File.join(__dir__, 'default_module_facts.yml')) facts = YAML.safe_load(File.read(File.join(__dir__, 'default_module_facts.yml'))) - if facts - facts.each do |name, value| - add_custom_fact name.to_sym, value - end + facts&.each do |name, value| + add_custom_fact name.to_sym, value end 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/facter/firewalld_version_spec.rb b/spec/unit/facter/firewalld_version_spec.rb index c4298b3e..bb943e13 100644 --- a/spec/unit/facter/firewalld_version_spec.rb +++ b/spec/unit/facter/firewalld_version_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe 'firewalld_version' do 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_policy_spec.rb b/spec/unit/puppet/provider/firewalld_policy_spec.rb index 725bf197..3fb0dc53 100644 --- a/spec/unit/puppet/provider/firewalld_policy_spec.rb +++ b/spec/unit/puppet/provider/firewalld_policy_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' provider_class = Puppet::Type.type(:firewalld_policy).provider(:firewall_cmd) @@ -25,7 +27,7 @@ describe 'when creating policy' do context 'with name public2restricted' do - it 'should execute firewall_cmd with new-policy' do + it 'executes firewall_cmd with new-policy' do resource.expects(:[]).with(:name).returns('public2restricted').at_least_once resource.expects(:[]).with(:target).returns(nil).at_least_once resource.expects(:[]).with(:ingress_zones).returns(['public']).at_least_once @@ -49,7 +51,7 @@ describe 'when modifying description' do context 'type' do - it 'should store updated description' do + it 'stores updated description' do resource.expects(:[]).with(:name).returns('public2restricted').at_least_once resource.expects(:[]).with(:target).returns(nil).at_least_once resource.expects(:[]).with(:ingress_zones).returns(['public']).at_least_once @@ -67,7 +69,7 @@ provider.create - provider.expects(:execute_firewall_cmd_policy).with(['--set-description', :"Modified description"], 'public2restricted', true, false) + provider.expects(:execute_firewall_cmd_policy).with(['--set-description', :'Modified description'], 'public2restricted', true, false) # Modify description provider.description = :'Modified description' diff --git a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb index 91b28349..f32a1123 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) @@ -44,6 +46,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(:priority).returns(nil) @@ -64,6 +67,7 @@ expect(provider.build_rich_rule).to eq('rule family="ipv4" destination address="192.168.0.1/32" service name="ssh" reject type="icmp-admin-prohibited"') end end + context 'with priority' do it 'builds the rich rule' do resource.expects(:[]).with(:priority).returns(1200) 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_policy_spec.rb b/spec/unit/puppet/type/firewalld_policy_spec.rb index c6ecf42a..56a1f461 100644 --- a/spec/unit/puppet/type/firewalld_policy_spec.rb +++ b/spec/unit/puppet/type/firewalld_policy_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Puppet::Type.type(:firewalld_policy) do @@ -8,16 +10,16 @@ describe 'type' do context 'with no params' do describe 'when validating attributes' do - [:name, :policy, :description, :short].each do |param| - it "should have a #{param} parameter" do + %i[name policy description short].each do |param| + it "has a #{param} parameter" do expect(described_class.attrtype(param)).to eq(:param) end end - [:target, :ingress_zones, :egress_zones, - :priority, :masquerade, :icmp_blocks, - :purge_rich_rules, :purge_services, :purge_ports].each do |prop| - it "should have a #{prop} property" do + %i[target ingress_zones egress_zones + priority masquerade icmp_blocks + purge_rich_rules purge_services purge_ports].each do |prop| + it "has a #{prop} property" do expect(described_class.attrtype(prop)).to eq(:property) end end @@ -25,90 +27,94 @@ end context 'validation' do - it "should reject empty ingress_zones" do + it 'rejects empty ingress_zones' do expect do - described_class.new(name: "empty iz", + described_class.new(name: 'empty iz', ingress_zones: [], - egress_zones: ["restricted"]) - end. to raise_error( - %r{parameter ingress_zones must contain at least one zone}) + egress_zones: ['restricted']) + end.to raise_error( + %r{parameter ingress_zones must contain at least one zone} + ) end - it "should reject empty egress_zones" do + it 'rejects empty egress_zones' do expect do - described_class.new(name: "empty ez", - ingress_zones: ["public"], + described_class.new(name: 'empty ez', + ingress_zones: ['public'], egress_zones: []) - end. to raise_error( - %r{parameter egress_zones must contain at least one zone}) + end.to raise_error( + %r{parameter egress_zones must contain at least one zone} + ) end - it "should reject unset ingress_zones when ensure is not absent" do + it 'rejects unset ingress_zones when ensure is not absent' do expect do - described_class.new(name: "unset iz", - egress_zones: ["restricted"]) - end. to raise_error( - %r{parameter ingress_zones must be an array of strings}) + described_class.new(name: 'unset iz', + egress_zones: ['restricted']) + end.to raise_error( + %r{parameter ingress_zones must be an array of strings} + ) end - it "should reject unset egress_zones when ensure is not absent" do + it 'rejects unset egress_zones when ensure is not absent' do expect do - described_class.new(name: "unset ez", - ingress_zones: ["public"]) - end. to raise_error( - %r{parameter egress_zones must be an array of strings}) + described_class.new(name: 'unset ez', + ingress_zones: ['public']) + end.to raise_error( + %r{parameter egress_zones must be an array of strings} + ) end - it "should not complain about unset iz/ez when ensure is absent" do + it 'does not complain about unset iz/ez when ensure is absent' do nozones = described_class.new(name: 'unset iz/ez', ensure: :absent) expect(nozones[:ingress_zones]).to eq([]) expect(nozones[:egress_zones]).to eq([]) end - it "should reject bad ingress_zones combinations" do + it 'rejects bad ingress_zones combinations' do expect do - ["ANY", "HOST"].each do |symbolic_zone| - described_class.new(name: "bad ingress_zones", - ingress_zones: [symbolic_zone, "public"], - egress_zones: ["restricted"]) - end. to raise_error(%r{parameter ingress_zones must contain a single symbolic zone or one or more regular zones}) + %w[ANY HOST].each do |symbolic_zone| + described_class.new(name: 'bad ingress_zones', + ingress_zones: [symbolic_zone, 'public'], + egress_zones: ['restricted']) + end.to raise_error(%r{parameter ingress_zones must contain a single symbolic zone or one or more regular zones}) end end - it "should reject bad egress_zones combinations" do + it 'rejects bad egress_zones combinations' do expect do - ["ANY", "HOST"].each do |symbolic_zone| - described_class.new(name: "bad egress_zones", - ingress_zones: ["public"], - egress_zones: [symbolic_zone, "restricted"]) - end. to raise_error(%r{parameter egress_zones must contain a single symbolic zone or one or more regular zones}) + %w[ANY HOST].each do |symbolic_zone| + described_class.new(name: 'bad egress_zones', + ingress_zones: ['public'], + egress_zones: [symbolic_zone, 'restricted']) + end.to raise_error(%r{parameter egress_zones must contain a single symbolic zone or one or more regular zones}) end end - it "should accept lone symbolic ingress_zones" do - ["ANY", "HOST"].each do |symbolic_zone| - izresource = described_class.new(name: "symbolic iz", + it 'accepts lone symbolic ingress_zones' do + %w[ANY HOST].each do |symbolic_zone| + izresource = described_class.new(name: 'symbolic iz', ingress_zones: [symbolic_zone], - egress_zones: ["restricted"]) + egress_zones: ['restricted']) expect(izresource[:ingress_zones]).to eq([symbolic_zone]) end end - it "should accept lone symbolic egress_zones" do - ["ANY", "HOST"].each do |symbolic_zone| - izresource = described_class.new(name: "symbolic iz", - ingress_zones: ["public"], + it 'accepts lone symbolic egress_zones' do + %w[ANY HOST].each do |symbolic_zone| + izresource = described_class.new(name: 'symbolic iz', + ingress_zones: ['public'], egress_zones: [symbolic_zone]) expect(izresource[:egress_zones]).to eq([symbolic_zone]) end end - it "should munge priority to string" do + it 'munges priority to string' do [-17, -1, 1, 17].each do |prio| - presource = described_class.new(name: "prio as numeric", - ingress_zones: ["public"], - egress_zones: ["restricted"], + presource = described_class.new(name: 'prio as numeric', + ingress_zones: ['public'], + egress_zones: ['restricted'], priority: prio) expect(presource[:priority]).to eq(prio.to_s) end @@ -126,39 +132,39 @@ target: '%%REJECT%%', ingress_zones: ['public'], egress_zones: ['restricted'], - icmp_blocks: ['redirect', 'router-advertisment'] + icmp_blocks: %w[redirect router-advertisment] ) end let(:provider) do resource.provider end - it 'should check if it exists' do + it 'checks if it exists' do provider.expects(:execute_firewall_cmd_policy).with(['--get-policies'], nil).returns('public2restricted other') expect(provider).to be_exists end - it 'should check if it doesnt exist' do + it 'checks if it doesnt exist' do provider.expects(:execute_firewall_cmd_policy).with(['--get-policies'], nil).returns('wrong other') expect(provider).not_to be_exists end - it 'should evalulate target' do + it 'evalulates target' do provider.expects(:execute_firewall_cmd_policy).with(['--get-target']).returns('%%REJECT%%') expect(provider.target).to eq('%%REJECT%%') end - it 'should evalulate target correctly when not surrounded with %%' do + it 'evalulates target correctly when not surrounded with %%' do provider.expects(:execute_firewall_cmd_policy).with(['--get-target']).returns('REJECT') expect(provider.target).to eq('%%REJECT%%') end - it 'should add policy when created' do + it 'adds policy when created' do provider.expects(:execute_firewall_cmd_policy).with(['--new-policy', 'public2restricted'], nil) provider.expects(:execute_firewall_cmd_policy).with(['--set-target', '%%REJECT%%']) provider.expects(:execute_firewall_cmd_policy).with(['--set-priority', '-1']) - provider.expects(:icmp_blocks=).with(['redirect', 'router-advertisment']) + provider.expects(:icmp_blocks=).with(%w[redirect router-advertisment]) provider.expects(:ingress_zones).returns([]) provider.expects(:execute_firewall_cmd_policy).with(['--add-ingress-zone', 'public']) @@ -168,34 +174,34 @@ provider.create end - it 'should delete policy when destroyed' do + it 'deletes policy when destroyed' do provider.expects(:execute_firewall_cmd_policy).with(['--delete-policy', 'public2restricted'], nil) provider.destroy end - it 'should set target' do + it 'sets target' do provider.expects(:execute_firewall_cmd_policy).with(['--set-target', '%%REJECT%%']) provider.target = '%%REJECT%%' end - it 'should get ingress zones' do + it 'gets ingress zones' do provider.expects(:execute_firewall_cmd_policy).with(['--list-ingress-zones']).returns('public') expect(provider.ingress_zones).to eq(['public']) end - it 'should get egress zones' do + it 'gets egress zones' do provider.expects(:execute_firewall_cmd_policy).with(['--list-egress-zones']).returns('restricted') expect(provider.egress_zones).to eq(['restricted']) end - it 'should get icmp_blocks' do + it 'gets icmp_blocks' do provider.expects(:execute_firewall_cmd_policy).with(['--list-icmp-blocks']).returns('val') expect(provider.icmp_blocks).to eq(['val']) end - it 'should list icmp types' do + it 'lists icmp types' do provider.expects(:execute_firewall_cmd_policy).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 @@ -213,21 +219,22 @@ resource.provider end - it 'should set masquerading' do + it 'sets masquerading' do provider.expects(:execute_firewall_cmd_policy).with(['--add-masquerade']) provider.masquerade = :true end - it 'should disable masquerading' do + it 'disables masquerading' do provider.expects(:execute_firewall_cmd_policy).with(['--remove-masquerade']) provider.masquerade = :false end - it 'should get masquerading state as false when not set' do + it 'gets masquerading state as false when not set' do provider.expects(:execute_firewall_cmd_policy).with(['--query-masquerade'], 'public2restricted', true, false).returns("no\n") expect(provider.masquerade).to eq(:false) end - it 'should get masquerading state as true when set' do + + it 'gets masquerading state as true when set' do provider.expects(:execute_firewall_cmd_policy).with(['--query-masquerade'], 'public2restricted', true, false).returns("yes\n") expect(provider.masquerade).to eq(:true) end @@ -242,11 +249,11 @@ @catalog.add_resource(firewalld_service) end - it 'should autorequire the firewalld service' do + it 'autorequires the firewalld service' do resource = described_class.new( name: 'public2restricted', - ingress_zones: ["public"], - egress_zones: ["restricted"] + ingress_zones: ['public'], + egress_zones: ['restricted'] ) @catalog.add_resource(resource) diff --git a/spec/unit/puppet/type/firewalld_port_spec.rb b/spec/unit/puppet/type/firewalld_port_spec.rb index a4096db9..122f9a94 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 a3b3f625..b04e49a8 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( @@ -101,24 +106,27 @@ ) end.to raise_error(%r{Priority must be between -32768 and 32767}) end + it 'raises an error if too low priority' do expect do described_class.new( title: 'SSH from barny', zone: 'restricted', - priority: -32769 + priority: -32_769 ) end.to raise_error(%r{Priority must be between -32768 and 32767}) end + it 'raises an error if too high priority' do expect do described_class.new( title: 'SSH from barny', zone: 'restricted', - priority: 32768 + priority: 32_768 ) end.to raise_error(%r{Priority must be between -32768 and 32767}) end + it 'does not raises an error if priority is valid' do expect do described_class.new( @@ -126,7 +134,7 @@ zone: 'restricted', priority: 10 ) - end.not_to raise_error() + end.not_to raise_error end end diff --git a/spec/unit/puppet/type/firewalld_service_spec.rb b/spec/unit/puppet/type/firewalld_service_spec.rb index 4ec8e0e5..1daeaef8 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 f2c060f0..dd6dd0b2 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, :icmp_block_inversion, :sources, :purge_rich_rules, :purge_services, :purge_ports].each do |param| - it "should have a #{param} parameter" do + %i[target icmp_blocks icmp_block_inversion 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], icmp_block_inversion: true, sources: ['192.168.2.2', '10.72.1.100'] ) @@ -67,7 +69,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']) @@ -119,7 +121,7 @@ it 'sets icmp_block_inversion' do provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'restricted', true, false).returns('no') - provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], "restricted", true, false) + provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], 'restricted', true, false) expect(provider.icmp_block_inversion).to eq(:false) end @@ -130,7 +132,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 @@ -160,6 +162,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)