Merge pull request #350 from jcpunk/linter

Run rubocop linter

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.

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? }

lib/puppet/provider/firewalld.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'puppet'
 require 'puppet/type'
 require 'puppet/provider'
@@ -6,18 +8,15 @@ class Puppet::Provider::Firewalld < Puppet::Provider
   @runstate = nil
 
   class << self
-    attr_accessor :running
-    attr_accessor :runstate
+    attr_accessor :running, :runstate
   end
 
   def state
     self.class.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

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

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')
 

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')
 

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

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')
 

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

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
 

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')